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 ?