Hi,
> From: Marek Vasut <ma...@denx.de>
> Sent: mardi 7 avril 2020 22:01
>
> On 4/7/20 3:00 PM, Patrick DELAUNAY wrote:
> > Dear Marek,
>
> Hi,
>
> >> diff --git a/arch/arm/dts/stm32mp15-ddr.dtsi
> >> b/arch/arm/dts/stm32mp15-ddr.dtsi index 38f29bb789..50ca7092c4 100644
> >> --- a/arch/arm/dts/stm32mp15-ddr.dtsi
> >> +++ b/arch/arm/dts/stm32mp15-ddr.dtsi
> >> @@ -3,152 +3,233 @@
> >> * Copyright : STMicroelectronics 2018
> >> */
> >>
> >> -/ {
> >> - soc {
> >> - ddr: ddr@5a003000 {
> >> - u-boot,dm-pre-reloc;
> >
> > For information, binding file must be updated also
> > ./doc/device-tree-bindings/memory-controllers/st,stm32mp1-ddr.txt
> >
> > This binding and the helper file " stm32mp15-ddr.dtsi" is common with TF-A.
> >
> >> +&ddr {
> >> + config@DDR_MEM_LABEL {
> >
> > I think that "config@text"
> > don't respect the latest device tree rule and a warning is raised with
> > latest dtc version
> >
> > it is now mandatory to value after align @ with reg value
> >
> > config@<reg> {
> > reg = <reg>
> > }
> >
> > It is why I prefer a name without meaning here (as config-1 /
> > config-2), And selection is done on st,mem-name
> >
> > config-1 {
> > ....
> > }
> > config-2 {
> > ....
> > }
> >
> >
> > And I want to propose, for DH board with several configuration
> >
> > &ddr {
> > config-1 {
> > #include "stm32mp15-ddr3-1x4Gb-1066-binG.dtsi"
> > }
> > config-2 {
> > #include "stm32mp15-ddr3-2x4Gb-1066-binG.dtsi"
> > }
> > }
> >
> >
> > For ST board with only one configuration (don't change the device
> > tree, config at the same level) &ddr { #include
> > "stm32mp15-ddr3-1x4Gb-1066-binG.dtsi"
> > }
> >
> >
> > NB: I have a other idea, it is to use the "reg" as config identifier to
> > select
> configuration, as it is done with OTP with "opp-supported-hw"
> > in linux/Documentation/devicetree/bindings/opp/opp.txt
> >
> > And reg can be the identified with your GPIO setting
> >
> > &ddr {
> > config@2 {
> > reg = 2;
> > #include "stm32mp15-ddr3-1x4Gb-1066-binG.dtsi"
> > }
> > config@3 {
> > reg = 3;
> > #include "stm32mp15-ddr3-2x4Gb-1066-binG.dtsi"
> > }
> > }
> >
> > This proposal avoids to compare a hardcoded string in SPL...
>
> I would much rather prefer to avoid manually writing the config@<foo> parts,
> that
> should be handled by some macro magic instead. With my proposal, it is not
> necessary at all either.
Ok, but you should keep the support when DDR_MEM_LABEL is not available (the
ddr dtsi is generated by ST tools)
I can propose a intermediate solution for the macro :
./arch/arm/dts/stm32mp15-ddr.dtsi
&ddr {
#ifdef DDR_MEM_CONFIG
config@DDR_MEM_CONFIG {
reg = < DDR_MEM_CONFIG>
#endif
st,mem-name = DDR_MEM_NAME;
st,mem-speed = <DDR_MEM_SPEED>;
st,mem-size = <DDR_MEM_SIZE>;
[....]
st,phy-cal = <
DDR_DX0DLLCR
DDR_DX0DQTR
DDR_DX0DQSTR
DDR_DX1DLLCR
DDR_DX1DQTR
DDR_DX1DQSTR
DDR_DX2DLLCR
DDR_DX2DQTR
DDR_DX2DQSTR
DDR_DX3DLLCR
DDR_DX3DQTR
DDR_DX3DQSTR
>;
#ifdef DDR_MEM_CONFIG
}
#endif
}
#ifdef DDR_MEM_CONFIG
#undef DDR_MEM_CONFIG
#endif
#undef DDR_MEM_LABEL
#undef DDR_MEM_NAME
#undef DDR_MEM_SPEED
#undef DDR_MEM_SIZE
[....]
#undef DDR_DX3DQTR
#undef DDR_DX3DQSTR
So the file generate by CubeMX don't change =
stm32mp15-ddr3-1x4Gb-1066-binG.dtsi and stm32mp15-ddr3-2x4Gb-1066-binG.dtsi.
The ST board devicetree don't change: the DDR configuration is still in ddr
node (as in TF-A)
For your board, the device tree /arch/arm/dts/stm32mp15xx-dhcor-u-boot.dtsi
[...]
#define DDR_MEM_CONFIG 2
#include "stm32mp15-ddr3-1x4Gb-1066-binG.dtsi"
#define DDR_MEM_CONFIG 3
#include "stm32mp15-ddr3-2x4Gb-1066-binG.dtsi"
[...]
And you can directly compare reg value of sub node with ddr3code.
It is more acceptable ?
Patrick
