On 4/8/20 12:09 PM, Patrick DELAUNAY wrote:
Hi,
Hi,
From: Marek Vasut marex@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 ?
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; ... }