[PATCH 3/4] [RFC] ARM: dts: stm32: Rework DDR DT inclusion

[Marek Vasut]

On 4/9/20 7:05 PM, Patrick DELAUNAY wrote:
> Hi Marek,
> 
>> From: Marek Vasut <marex at denx.de>
>> Sent: mercredi 8 avril 2020 15:54
>>
>> On 4/8/20 12:09 PM, Patrick DELAUNAY wrote:
>>> Hi,
>>
>> Hi,
>>
>>>> From: Marek Vasut <marex at denx.de>
>>>> Sent: mardi 7 avril 2020 22:01
>>>>
>>>> On 4/7/20 3:00 PM, Patrick DELAUNAY wrote:
>>>>> Dear Marek,
>>>>
>>>> Hi,
>>>>
> 
> [...]
> 
>>>>>
>>>>> 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"
>>>>> }
>>>>>
>>>>>
> 
> [...]
> 
>>>>
>>>> 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.
>>>
> 
> [....]
> 
>>>
>>> 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 ?
>>
>> I wonder, can't we have some sort of macro where you would specify a compatible
>> string for the DDR config (on which you can match in your
>> board_stm32mp1_ddr_config_name_match() and the dtsi file to be included, and
>> the macro would generate the necessary entries in the &ddr {} controller node ?
>>
>> E.g. like this:
>>
>> #include "stm32mp15-ddr.dtsi"
>> STM32MP15_DDR("vendor,board-1gib", stm32mp15-ddr3-2x4Gb-1066-binG.dtsi);
>> STM32MP15_DDR("vendor,board-2gib", stm32mp15-ddr3-4x4Gb-1066-binG.dtsi);
>>
>> and then in board_stm32mp1_ddr_config_name_match()
>> {
>>  if (!strcmp(..., "vendor,board-1gib"))
>>     return 0;
>>  ...
>> }
> 
> Yes, I agree, compatible is the better solution and the binding 
> 
> ./doc/device-tree-bindings/memory-controllers/st,stm32mp1-ddr.txt becomes
> 
> ddr: ddr at 0x5A003000{
> 	compatible = "st,stm32mp1-ddr";
> 	[...]
> 
> 	config-1 {
> 		compatible = "vendor,board-1gib";
> 
> 		st,mem-name = "..."
> 		[...]
> 		st,phy-timing = <...>
> 	}
> 	config-2 {
> 		compatible = "vendor,board-2gib";
> 		st,mem-name = "..."
> 		[...]
> 		st,phy-timing = <...>
> 	}
> 	status = "okay";
> }
> 
> And you match this configuration with compatible.
> 
> For the macro, it should be perfect, if it is not too complicate.
> 
> Because I afraid that "#include" in macro isn't allowed.

I'll send a V2 now. The compatible string is easy enough.