On 4/8/20 12:17 PM, Patrick DELAUNAY wrote:
> Hi Marek,
Hi,
>> From: Marek Vasut <ma...@denx.de>
>> Sent: mardi 7 avril 2020 21:58
>>
>> On 4/7/20 3:04 PM, Patrick DELAUNAY wrote:
>>> Hi Marek,
>>
>> Hi,
>>
>> [...]
>>
>>>> +__weak int board_stm32mp1_ddr_config_name_match(struct udevice *dev,
>>>> + const char *name)
>>>> +{
>>>> + return 0; /* Always match */
>>>> +}
>>>> +
>>>> +static ofnode stm32mp1_ddr_get_ofnode(struct udevice *dev) {
>>>> + const char *name;
>>>> + ofnode node;
>>>> +
>>>> + node = dev_ofnode(dev);
>>>> + name = ofnode_get_name(node);
>>>> + if (!board_stm32mp1_ddr_config_name_match(dev, name))
>>>> + return node;
>>>
>>> Compare with name of the node or with name of DDR configuration ?
>>>
>>> For me " st,mem-name" is same than "description" in FIT config.
>>>
>>> name = ofnode_read_string(node, "st,mem-name");
>>>
>>> if (name && !board_stm32mp1_ddr_config_name_match(dev, name))
>>> return node;
>>
>> st,mem-name contains the version string, which makes it not very usable,
>> see:
>>
>> arch/arm/dts/stm32mp15-ddr3-1x4Gb-1066-binG.dtsi:#define DDR_MEM_NAME
>> "DDR3-1066/888 bin G 1x4Gb 533MHz v1.45"
>> arch/arm/dts/stm32mp15-ddr3-2x4Gb-1066-binG.dtsi:#define DDR_MEM_NAME
>> "DDR3-1066/888 bin G 2x4Gb 533MHz v1.45"
>>
>> That "v1.45" part.
>
> It is the version of first internal tools used to generated the ddr file ,
> which define register
> values according type, timing , size, frequency.
> I kept it with when I upstream the file (for U-Boot and TF-A) but it was a
> bad idea.
No, that's a good idea, at least we know which version is used. That
could be important.
> I align these 2 files with the files generated by the official tools = CubeMX
> and this version indication disappear (but DDR_MEM_NAME change)
> in http://patchwork.ozlabs.org/patch/1264835/
>
> [14/16] ARM: dts: stm32mp15: use DDR3 files generated by STM32CubeMX
>
> NB: You can also compare reg if you are OK with my proposal (config@2 /
> config@3)
That is absolutely error-prone. We need some unique unambiguous
identifier, maybe a compatible string for the memory config ?
