[PATCH 1/4] [RFC] ARM: stm32: Implement board coding on AV96
The AV96 board does exist in multiple variants. To cater for all of them, implement board code handling. There are two GPIOs which code the type of the board, read them out and use the value to pick the correct device tree from an fitImage.
Signed-off-by: Marek Vasut marex@denx.de Cc: Manivannan Sadhasivam manivannan.sadhasivam@linaro.org Cc: Patrick Delaunay patrick.delaunay@st.com Cc: Patrice Chotard patrice.chotard@st.com --- arch/arm/dts/stm32mp15xx-dhcor-u-boot.dtsi | 9 +++ arch/arm/mach-stm32mp/spl.c | 2 +- board/dhelectronics/dh_stm32mp1/board.c | 84 ++++++++++++++++++++++ board/dhelectronics/dh_stm32mp1/u-boot.its | 39 ++++++++++ configs/stm32mp15_dhcor_basic_defconfig | 3 + 5 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 board/dhelectronics/dh_stm32mp1/u-boot.its
diff --git a/arch/arm/dts/stm32mp15xx-dhcor-u-boot.dtsi b/arch/arm/dts/stm32mp15xx-dhcor-u-boot.dtsi index 02dad81b0b..17a23ae21c 100644 --- a/arch/arm/dts/stm32mp15xx-dhcor-u-boot.dtsi +++ b/arch/arm/dts/stm32mp15xx-dhcor-u-boot.dtsi @@ -11,6 +11,15 @@ #include "stm32mp157-u-boot.dtsi" #include "stm32mp15-ddr3-2x4Gb-1066-binG.dtsi"
+/ { + u-boot,dm-pre-reloc; + config { + u-boot,dm-pre-reloc; + #gpio-cells = <2>; + dh,som-coding-gpios = <&gpioz 7 0>, <&gpiof 3 0>; + }; +}; + &i2c4 { u-boot,dm-pre-reloc; }; diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c index ca4231cd0d..5461572090 100644 --- a/arch/arm/mach-stm32mp/spl.c +++ b/arch/arm/mach-stm32mp/spl.c @@ -76,7 +76,7 @@ void spl_display_print(void) } #endif
-void board_init_f(ulong dummy) +__weak void board_init_f(ulong dummy) { struct udevice *dev; int ret; diff --git a/board/dhelectronics/dh_stm32mp1/board.c b/board/dhelectronics/dh_stm32mp1/board.c index a3458a2623..36b8652521 100644 --- a/board/dhelectronics/dh_stm32mp1/board.c +++ b/board/dhelectronics/dh_stm32mp1/board.c @@ -133,6 +133,90 @@ int checkboard(void) return 0; }
+#ifdef CONFIG_SPL_BUILD +static u8 somcode __section("data"); + +static void board_get_coding_straps(void) +{ + struct gpio_desc gpio; + ofnode node; + int i, ret; + + node = ofnode_path("/config"); + if (!ofnode_valid(node)) { + printf("%s: no /config node?\n", __func__); + return; + } + + for (i = 0; i < 2; i++) { + ret = gpio_request_by_name_nodev(node, "dh,som-coding-gpios", + i, &gpio, GPIOD_IS_IN); + if (ret) { + printf("%s: could not find a /config/dh,som-coding-gpios[%i]\n", + __func__, i); + return; + } + + somcode |= !!dm_gpio_get_value(&gpio) << i; + } + + printf("Code: SoM:%x\n", somcode); +} + +void board_init_f(ulong dummy) +{ + struct udevice *dev; + int ret; + + arch_cpu_init(); + + ret = spl_early_init(); + if (ret) { + debug("spl_early_init() failed: %d\n", ret); + hang(); + } + + ret = uclass_get_device(UCLASS_CLK, 0, &dev); + if (ret) { + debug("Clock init failed: %d\n", ret); + return; + } + + ret = uclass_get_device(UCLASS_RESET, 0, &dev); + if (ret) { + debug("Reset init failed: %d\n", ret); + return; + } + + ret = uclass_get_device(UCLASS_PINCTRL, 0, &dev); + if (ret) { + debug("%s: Cannot find pinctrl device\n", __func__); + return; + } + + /* enable console uart printing */ + preloader_console_init(); + + board_get_coding_straps(); + + ret = uclass_get_device(UCLASS_RAM, 0, &dev); + if (ret) { + printf("DRAM init failed: %d\n", ret); + hang(); + } +} + +#ifdef CONFIG_SPL_LOAD_FIT +int board_fit_config_name_match(const char *name) +{ + if (somcode == 0 && !strcmp(name, "586-100")) + return 0; + + return -EINVAL; +} +#endif +#endif + static void board_key_check(void) { #if defined(CONFIG_FASTBOOT) || defined(CONFIG_CMD_STM32PROG) diff --git a/board/dhelectronics/dh_stm32mp1/u-boot.its b/board/dhelectronics/dh_stm32mp1/u-boot.its new file mode 100644 index 0000000000..3ca3036f7e --- /dev/null +++ b/board/dhelectronics/dh_stm32mp1/u-boot.its @@ -0,0 +1,39 @@ +/dts-v1/; + +/ { + description = "U-Boot mainline"; + #address-cells = <1>; + + images { + uboot { + description = "U-Boot (32-bit)"; + data = /incbin/("../../../u-boot-nodtb.bin"); + type = "standalone"; + os = "U-Boot"; + arch = "arm"; + compression = "none"; + load = <0xc0100000>; + entry = <0xc0100000>; + }; + + fdt-1 { + description = ".dtb"; + data = /incbin/("../../../arch/arm/dts/stm32mp15xx-dhcor-avenger96.dtb"); + type = "flat_dt"; + arch = "arm"; + compression = "none"; + }; + }; + + configurations { + default = "config-1"; + + config-1 { + description = "586-100"; /* SoM model */ + loadables = "uboot"; + fdt = "fdt-1"; + }; + + /* Add 586-200..586-400 with fdt-2..fdt-4 here */ + }; +}; diff --git a/configs/stm32mp15_dhcor_basic_defconfig b/configs/stm32mp15_dhcor_basic_defconfig index 97e95bde7d..6c5ca31f40 100644 --- a/configs/stm32mp15_dhcor_basic_defconfig +++ b/configs/stm32mp15_dhcor_basic_defconfig @@ -11,7 +11,10 @@ CONFIG_SPL_SPI_SUPPORT=y CONFIG_SPL_TEXT_BASE=0x2FFC2500 CONFIG_DISTRO_DEFAULTS=y CONFIG_FIT=y +CONFIG_SPL_LOAD_FIT=y +CONFIG_SPL_FIT_SOURCE="board/dhelectronics/dh_stm32mp1/u-boot.its" CONFIG_BOOTCOMMAND="run bootcmd_stm32mp" +CONFIG_SPL_LEGACY_IMAGE_SUPPORT=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION=3 CONFIG_SPL_I2C_SUPPORT=y
Add support for multiple DRAM configuration subnodes, while retaining the support for a single flat DRAM configuration node. This is useful on systems which can be manufactured in multiple configurations and where the DRAM configuration can be determined at runtime.
The code is augmented by a function which can be overridden on board level, allowing a match on the configuration node name, very much like the fitImage configuration node name matching works. The default match is on the single top-level DRAM configuration, if matching on subnodes is required, then this board_stm32mp1_ddr_config_name_match() must be overridden.
Signed-off-by: Marek Vasut marex@denx.de Cc: Manivannan Sadhasivam manivannan.sadhasivam@linaro.org Cc: Patrick Delaunay patrick.delaunay@st.com Cc: Patrice Chotard patrice.chotard@st.com --- drivers/ram/stm32mp1/stm32mp1_ram.c | 36 +++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-)
diff --git a/drivers/ram/stm32mp1/stm32mp1_ram.c b/drivers/ram/stm32mp1/stm32mp1_ram.c index eb78f1198d..d6821eacd7 100644 --- a/drivers/ram/stm32mp1/stm32mp1_ram.c +++ b/drivers/ram/stm32mp1/stm32mp1_ram.c @@ -57,6 +57,33 @@ int stm32mp1_ddr_clk_enable(struct ddr_info *priv, uint32_t mem_speed) return 0; }
+__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; + + dev_for_each_subnode(node, dev) { + name = ofnode_get_name(node); + + if (!board_stm32mp1_ddr_config_name_match(dev, name)) + return node; + } + + return ofnode_null(); +} + static __maybe_unused int stm32mp1_ddr_setup(struct udevice *dev) { struct ddr_info *priv = dev_get_priv(dev); @@ -64,6 +91,7 @@ static __maybe_unused int stm32mp1_ddr_setup(struct udevice *dev) unsigned int idx; struct clk axidcg; struct stm32mp1_ddr_config config; + ofnode node = stm32mp1_ddr_get_ofnode(dev);
#define PARAM(x, y) \ { x,\ @@ -87,9 +115,9 @@ static __maybe_unused int stm32mp1_ddr_setup(struct udevice *dev) PHY_PARAM(cal) };
- config.info.speed = dev_read_u32_default(dev, "st,mem-speed", 0); - config.info.size = dev_read_u32_default(dev, "st,mem-size", 0); - config.info.name = dev_read_string(dev, "st,mem-name"); + config.info.speed = ofnode_read_u32_default(node, "st,mem-speed", 0); + config.info.size = ofnode_read_u32_default(node, "st,mem-size", 0); + config.info.name = ofnode_read_string(node, "st,mem-name"); if (!config.info.name) { debug("%s: no st,mem-name\n", __func__); return -EINVAL; @@ -97,7 +125,7 @@ static __maybe_unused int stm32mp1_ddr_setup(struct udevice *dev) printf("RAM: %s\n", config.info.name);
for (idx = 0; idx < ARRAY_SIZE(param); idx++) { - ret = dev_read_u32_array(dev, param[idx].name, + ret = ofnode_read_u32_array(node, param[idx].name, (void *)((u32)&config + param[idx].offset), param[idx].size);
Hi Marek,
From: Marek Vasut marex@denx.de Sent: mercredi 1 avril 2020 01:48
Add support for multiple DRAM configuration subnodes, while retaining the support for a single flat DRAM configuration node. This is useful on systems which can be manufactured in multiple configurations and where the DRAM configuration can be determined at runtime.
The code is augmented by a function which can be overridden on board level, allowing a match on the configuration node name, very much like the fitImage configuration node name matching works. The default match is on the single top- level DRAM configuration, if matching on subnodes is required, then this board_stm32mp1_ddr_config_name_match() must be overridden.
Signed-off-by: Marek Vasut marex@denx.de Cc: Manivannan Sadhasivam manivannan.sadhasivam@linaro.org Cc: Patrick Delaunay patrick.delaunay@st.com Cc: Patrice Chotard patrice.chotard@st.com
drivers/ram/stm32mp1/stm32mp1_ram.c | 36 +++++++++++++++++++++++++---
1 file changed, 32 insertions(+), 4 deletions(-)
diff --git a/drivers/ram/stm32mp1/stm32mp1_ram.c b/drivers/ram/stm32mp1/stm32mp1_ram.c index eb78f1198d..d6821eacd7 100644 --- a/drivers/ram/stm32mp1/stm32mp1_ram.c +++ b/drivers/ram/stm32mp1/stm32mp1_ram.c @@ -57,6 +57,33 @@ int stm32mp1_ddr_clk_enable(struct ddr_info *priv, uint32_t mem_speed) return 0; }
+__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;
- dev_for_each_subnode(node, dev) {
name = ofnode_get_name(node);if (!board_stm32mp1_ddr_config_name_match(dev, name))return node;- }
- return ofnode_null();
+}
static __maybe_unused int stm32mp1_ddr_setup(struct udevice *dev) { struct ddr_info *priv = dev_get_priv(dev); @@ -64,6 +91,7 @@ static __maybe_unused int stm32mp1_ddr_setup(struct udevice *dev) unsigned int idx; struct clk axidcg; struct stm32mp1_ddr_config config;
- ofnode node = stm32mp1_ddr_get_ofnode(dev);
#define PARAM(x, y) \ { x,\ @@ -87,9 +115,9 @@ static __maybe_unused int stm32mp1_ddr_setup(struct udevice *dev) PHY_PARAM(cal) };
- config.info.speed = dev_read_u32_default(dev, "st,mem-speed", 0);
- config.info.size = dev_read_u32_default(dev, "st,mem-size", 0);
- config.info.name = dev_read_string(dev, "st,mem-name");
- config.info.speed = ofnode_read_u32_default(node, "st,mem-speed", 0);
- config.info.size = ofnode_read_u32_default(node, "st,mem-size", 0);
- config.info.name = ofnode_read_string(node, "st,mem-name"); if (!config.info.name) { debug("%s: no st,mem-name\n", __func__); return -EINVAL;
@@ -97,7 +125,7 @@ static __maybe_unused int stm32mp1_ddr_setup(struct udevice *dev) printf("RAM: %s\n", config.info.name);
for (idx = 0; idx < ARRAY_SIZE(param); idx++) {
ret = dev_read_u32_array(dev, param[idx].name,
ret = ofnode_read_u32_array(node, param[idx].name, (void *)((u32)&config + param[idx].offset), param[idx].size);-- 2.25.1
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.
[...]
Hi Marek,
From: Marek Vasut marex@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.
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)
Patrick
[...]
On 4/8/20 12:17 PM, Patrick DELAUNAY wrote:
Hi Marek,
Hi,
From: Marek Vasut marex@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 ?
Adjust the DDR configuration dtsi such that they only generate the DRAM configuration node, the DDR controller node is moved into the stm32mp157-u-boot.dtsi itself. This permits including multiple DDR configuration dtsi files in board DT.
Signed-off-by: Marek Vasut marex@denx.de Cc: Manivannan Sadhasivam manivannan.sadhasivam@linaro.org Cc: Patrick Delaunay patrick.delaunay@st.com Cc: Patrice Chotard patrice.chotard@st.com --- arch/arm/dts/stm32mp15-ddr.dtsi | 357 +++++++++++------- .../dts/stm32mp15-ddr3-1x4Gb-1066-binG.dtsi | 1 + .../dts/stm32mp15-ddr3-2x4Gb-1066-binG.dtsi | 1 + arch/arm/dts/stm32mp157-u-boot.dtsi | 25 ++ 4 files changed, 246 insertions(+), 138 deletions(-)
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; +&ddr { + config@DDR_MEM_LABEL { + u-boot,dm-pre-reloc;
- compatible = "st,stm32mp1-ddr"; + st,mem-name = DDR_MEM_NAME; + st,mem-speed = <DDR_MEM_SPEED>; + st,mem-size = <DDR_MEM_SIZE>;
- reg = <0x5A003000 0x550 - 0x5A004000 0x234>; + st,ctl-reg = < + DDR_MSTR + DDR_MRCTRL0 + DDR_MRCTRL1 + DDR_DERATEEN + DDR_DERATEINT + DDR_PWRCTL + DDR_PWRTMG + DDR_HWLPCTL + DDR_RFSHCTL0 + DDR_RFSHCTL3 + DDR_CRCPARCTL0 + DDR_ZQCTL0 + DDR_DFITMG0 + DDR_DFITMG1 + DDR_DFILPCFG0 + DDR_DFIUPD0 + DDR_DFIUPD1 + DDR_DFIUPD2 + DDR_DFIPHYMSTR + DDR_ODTMAP + DDR_DBG0 + DDR_DBG1 + DDR_DBGCMD + DDR_POISONCFG + DDR_PCCFG + >;
- clocks = <&rcc AXIDCG>, - <&rcc DDRC1>, - <&rcc DDRC2>, - <&rcc DDRPHYC>, - <&rcc DDRCAPB>, - <&rcc DDRPHYCAPB>; + st,ctl-timing = < + DDR_RFSHTMG + DDR_DRAMTMG0 + DDR_DRAMTMG1 + DDR_DRAMTMG2 + DDR_DRAMTMG3 + DDR_DRAMTMG4 + DDR_DRAMTMG5 + DDR_DRAMTMG6 + DDR_DRAMTMG7 + DDR_DRAMTMG8 + DDR_DRAMTMG14 + DDR_ODTCFG + >;
- clock-names = "axidcg", - "ddrc1", - "ddrc2", - "ddrphyc", - "ddrcapb", - "ddrphycapb"; + st,ctl-map = < + DDR_ADDRMAP1 + DDR_ADDRMAP2 + DDR_ADDRMAP3 + DDR_ADDRMAP4 + DDR_ADDRMAP5 + DDR_ADDRMAP6 + DDR_ADDRMAP9 + DDR_ADDRMAP10 + DDR_ADDRMAP11 + >;
- st,mem-name = DDR_MEM_NAME; - st,mem-speed = <DDR_MEM_SPEED>; - st,mem-size = <DDR_MEM_SIZE>; + st,ctl-perf = < + DDR_SCHED + DDR_SCHED1 + DDR_PERFHPR1 + DDR_PERFLPR1 + DDR_PERFWR1 + DDR_PCFGR_0 + DDR_PCFGW_0 + DDR_PCFGQOS0_0 + DDR_PCFGQOS1_0 + DDR_PCFGWQOS0_0 + DDR_PCFGWQOS1_0 + DDR_PCFGR_1 + DDR_PCFGW_1 + DDR_PCFGQOS0_1 + DDR_PCFGQOS1_1 + DDR_PCFGWQOS0_1 + DDR_PCFGWQOS1_1 + >;
- st,ctl-reg = < - DDR_MSTR - DDR_MRCTRL0 - DDR_MRCTRL1 - DDR_DERATEEN - DDR_DERATEINT - DDR_PWRCTL - DDR_PWRTMG - DDR_HWLPCTL - DDR_RFSHCTL0 - DDR_RFSHCTL3 - DDR_CRCPARCTL0 - DDR_ZQCTL0 - DDR_DFITMG0 - DDR_DFITMG1 - DDR_DFILPCFG0 - DDR_DFIUPD0 - DDR_DFIUPD1 - DDR_DFIUPD2 - DDR_DFIPHYMSTR - DDR_ODTMAP - DDR_DBG0 - DDR_DBG1 - DDR_DBGCMD - DDR_POISONCFG - DDR_PCCFG - >; + st,phy-reg = < + DDR_PGCR + DDR_ACIOCR + DDR_DXCCR + DDR_DSGCR + DDR_DCR + DDR_ODTCR + DDR_ZQ0CR1 + DDR_DX0GCR + DDR_DX1GCR + DDR_DX2GCR + DDR_DX3GCR + >;
- st,ctl-timing = < - DDR_RFSHTMG - DDR_DRAMTMG0 - DDR_DRAMTMG1 - DDR_DRAMTMG2 - DDR_DRAMTMG3 - DDR_DRAMTMG4 - DDR_DRAMTMG5 - DDR_DRAMTMG6 - DDR_DRAMTMG7 - DDR_DRAMTMG8 - DDR_DRAMTMG14 - DDR_ODTCFG - >; + st,phy-timing = < + DDR_PTR0 + DDR_PTR1 + DDR_PTR2 + DDR_DTPR0 + DDR_DTPR1 + DDR_DTPR2 + DDR_MR0 + DDR_MR1 + DDR_MR2 + DDR_MR3 + >;
- st,ctl-map = < - DDR_ADDRMAP1 - DDR_ADDRMAP2 - DDR_ADDRMAP3 - DDR_ADDRMAP4 - DDR_ADDRMAP5 - DDR_ADDRMAP6 - DDR_ADDRMAP9 - DDR_ADDRMAP10 - DDR_ADDRMAP11 - >; + 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 + >;
- st,ctl-perf = < - DDR_SCHED - DDR_SCHED1 - DDR_PERFHPR1 - DDR_PERFLPR1 - DDR_PERFWR1 - DDR_PCFGR_0 - DDR_PCFGW_0 - DDR_PCFGQOS0_0 - DDR_PCFGQOS1_0 - DDR_PCFGWQOS0_0 - DDR_PCFGWQOS1_0 - DDR_PCFGR_1 - DDR_PCFGW_1 - DDR_PCFGQOS0_1 - DDR_PCFGQOS1_1 - DDR_PCFGWQOS0_1 - DDR_PCFGWQOS1_1 - >; - - st,phy-reg = < - DDR_PGCR - DDR_ACIOCR - DDR_DXCCR - DDR_DSGCR - DDR_DCR - DDR_ODTCR - DDR_ZQ0CR1 - DDR_DX0GCR - DDR_DX1GCR - DDR_DX2GCR - DDR_DX3GCR - >; - - st,phy-timing = < - DDR_PTR0 - DDR_PTR1 - DDR_PTR2 - DDR_DTPR0 - DDR_DTPR1 - DDR_DTPR2 - DDR_MR0 - DDR_MR1 - DDR_MR2 - DDR_MR3 - >; - - 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 - >; - - status = "okay"; - }; + status = "okay"; }; }; + +#undef DDR_MEM_LABEL +#undef DDR_MEM_NAME +#undef DDR_MEM_SPEED +#undef DDR_MEM_SIZE + +#undef DDR_MSTR +#undef DDR_MRCTRL0 +#undef DDR_MRCTRL1 +#undef DDR_DERATEEN +#undef DDR_DERATEINT +#undef DDR_PWRCTL +#undef DDR_PWRTMG +#undef DDR_HWLPCTL +#undef DDR_RFSHCTL0 +#undef DDR_RFSHCTL3 +#undef DDR_RFSHTMG +#undef DDR_CRCPARCTL0 +#undef DDR_DRAMTMG0 +#undef DDR_DRAMTMG1 +#undef DDR_DRAMTMG2 +#undef DDR_DRAMTMG3 +#undef DDR_DRAMTMG4 +#undef DDR_DRAMTMG5 +#undef DDR_DRAMTMG6 +#undef DDR_DRAMTMG7 +#undef DDR_DRAMTMG8 +#undef DDR_DRAMTMG14 +#undef DDR_ZQCTL0 +#undef DDR_DFITMG0 +#undef DDR_DFITMG1 +#undef DDR_DFILPCFG0 +#undef DDR_DFIUPD0 +#undef DDR_DFIUPD1 +#undef DDR_DFIUPD2 +#undef DDR_DFIPHYMSTR +#undef DDR_ADDRMAP1 +#undef DDR_ADDRMAP2 +#undef DDR_ADDRMAP3 +#undef DDR_ADDRMAP4 +#undef DDR_ADDRMAP5 +#undef DDR_ADDRMAP6 +#undef DDR_ADDRMAP9 +#undef DDR_ADDRMAP10 +#undef DDR_ADDRMAP11 +#undef DDR_ODTCFG +#undef DDR_ODTMAP +#undef DDR_SCHED +#undef DDR_SCHED1 +#undef DDR_PERFHPR1 +#undef DDR_PERFLPR1 +#undef DDR_PERFWR1 +#undef DDR_DBG0 +#undef DDR_DBG1 +#undef DDR_DBGCMD +#undef DDR_POISONCFG +#undef DDR_PCCFG +#undef DDR_PCFGR_0 +#undef DDR_PCFGW_0 +#undef DDR_PCFGQOS0_0 +#undef DDR_PCFGQOS1_0 +#undef DDR_PCFGWQOS0_0 +#undef DDR_PCFGWQOS1_0 +#undef DDR_PCFGR_1 +#undef DDR_PCFGW_1 +#undef DDR_PCFGQOS0_1 +#undef DDR_PCFGQOS1_1 +#undef DDR_PCFGWQOS0_1 +#undef DDR_PCFGWQOS1_1 +#undef DDR_PGCR +#undef DDR_PTR0 +#undef DDR_PTR1 +#undef DDR_PTR2 +#undef DDR_ACIOCR +#undef DDR_DXCCR +#undef DDR_DSGCR +#undef DDR_DCR +#undef DDR_DTPR0 +#undef DDR_DTPR1 +#undef DDR_DTPR2 +#undef DDR_MR0 +#undef DDR_MR1 +#undef DDR_MR2 +#undef DDR_MR3 +#undef DDR_ODTCR +#undef DDR_ZQ0CR1 +#undef DDR_DX0GCR +#undef DDR_DX0DLLCR +#undef DDR_DX0DQTR +#undef DDR_DX0DQSTR +#undef DDR_DX1GCR +#undef DDR_DX1DLLCR +#undef DDR_DX1DQTR +#undef DDR_DX1DQSTR +#undef DDR_DX2GCR +#undef DDR_DX2DLLCR +#undef DDR_DX2DQTR +#undef DDR_DX2DQSTR +#undef DDR_DX3GCR +#undef DDR_DX3DLLCR +#undef DDR_DX3DQTR +#undef DDR_DX3DQSTR diff --git a/arch/arm/dts/stm32mp15-ddr3-1x4Gb-1066-binG.dtsi b/arch/arm/dts/stm32mp15-ddr3-1x4Gb-1066-binG.dtsi index 11e8f2bef6..a63be8643c 100644 --- a/arch/arm/dts/stm32mp15-ddr3-1x4Gb-1066-binG.dtsi +++ b/arch/arm/dts/stm32mp15-ddr3-1x4Gb-1066-binG.dtsi @@ -16,6 +16,7 @@ * address mapping : RBC * Tc > + 85C : N */ +#define DDR_MEM_LABEL ddr3-1066-888-bin-g-1x4gb-533mhz #define DDR_MEM_NAME "DDR3-1066/888 bin G 1x4Gb 533MHz v1.45" #define DDR_MEM_SPEED 533000 #define DDR_MEM_SIZE 0x20000000 diff --git a/arch/arm/dts/stm32mp15-ddr3-2x4Gb-1066-binG.dtsi b/arch/arm/dts/stm32mp15-ddr3-2x4Gb-1066-binG.dtsi index 4b70b60554..6f059269af 100644 --- a/arch/arm/dts/stm32mp15-ddr3-2x4Gb-1066-binG.dtsi +++ b/arch/arm/dts/stm32mp15-ddr3-2x4Gb-1066-binG.dtsi @@ -16,6 +16,7 @@ * address mapping : RBC * Tc > + 85C : N */ +#define DDR_MEM_LABEL ddr3-1066-888-bin-g-2x4gb-533mhz #define DDR_MEM_NAME "DDR3-1066/888 bin G 2x4Gb 533MHz v1.45" #define DDR_MEM_SPEED 533000 #define DDR_MEM_SIZE 0x40000000 diff --git a/arch/arm/dts/stm32mp157-u-boot.dtsi b/arch/arm/dts/stm32mp157-u-boot.dtsi index 8f9535a4db..1279589a56 100644 --- a/arch/arm/dts/stm32mp157-u-boot.dtsi +++ b/arch/arm/dts/stm32mp157-u-boot.dtsi @@ -36,6 +36,31 @@
soc { u-boot,dm-pre-reloc; + + ddr: ddr@5a003000 { + u-boot,dm-pre-reloc; + + compatible = "st,stm32mp1-ddr"; + + reg = <0x5A003000 0x550 + 0x5A004000 0x234>; + + clocks = <&rcc AXIDCG>, + <&rcc DDRC1>, + <&rcc DDRC2>, + <&rcc DDRPHYC>, + <&rcc DDRCAPB>, + <&rcc DDRPHYCAPB>; + + clock-names = "axidcg", + "ddrc1", + "ddrc2", + "ddrphyc", + "ddrcapb", + "ddrphycapb"; + + status = "okay"; + }; }; };
Dear Marek,
From: Marek Vasut marex@denx.de Sent: mercredi 1 avril 2020 01:48
Adjust the DDR configuration dtsi such that they only generate the DRAM configuration node, the DDR controller node is moved into the stm32mp157-u- boot.dtsi itself. This permits including multiple DDR configuration dtsi files in board DT.
Signed-off-by: Marek Vasut marex@denx.de Cc: Manivannan Sadhasivam manivannan.sadhasivam@linaro.org Cc: Patrick Delaunay patrick.delaunay@st.com Cc: Patrice Chotard patrice.chotard@st.com
arch/arm/dts/stm32mp15-ddr.dtsi | 357 +++++++++++------- .../dts/stm32mp15-ddr3-1x4Gb-1066-binG.dtsi | 1 + .../dts/stm32mp15-ddr3-2x4Gb-1066-binG.dtsi | 1 + arch/arm/dts/stm32mp157-u-boot.dtsi | 25 ++ 4 files changed, 246 insertions(+), 138 deletions(-)
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...
Regards
Patrick
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.
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 ?
Patrick
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; ... }
Hi Marek,
From: Marek Vasut marex@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@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@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.
regards
Patrick
On 4/9/20 7:05 PM, Patrick DELAUNAY wrote:
Hi Marek,
From: Marek Vasut marex@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@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@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.
The DHCOR board does exist in multiple variants with different DDR3 DRAM sizes. To cater for all of them, implement DDR3 code handling. There are two GPIOs which code the DRAM size populated on the SoM, read them out and use the value to pick the correct DDR3 config.
Signed-off-by: Marek Vasut marex@denx.de Cc: Manivannan Sadhasivam manivannan.sadhasivam@linaro.org Cc: Patrick Delaunay patrick.delaunay@st.com Cc: Patrice Chotard patrice.chotard@st.com --- arch/arm/dts/stm32mp15xx-dhcor-u-boot.dtsi | 2 ++ board/dhelectronics/dh_stm32mp1/board.c | 26 +++++++++++++++++++++- 2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/arch/arm/dts/stm32mp15xx-dhcor-u-boot.dtsi b/arch/arm/dts/stm32mp15xx-dhcor-u-boot.dtsi index 17a23ae21c..6db4ac37e1 100644 --- a/arch/arm/dts/stm32mp15xx-dhcor-u-boot.dtsi +++ b/arch/arm/dts/stm32mp15xx-dhcor-u-boot.dtsi @@ -9,6 +9,7 @@
#include <dt-bindings/clock/stm32mp1-clksrc.h> #include "stm32mp157-u-boot.dtsi" +#include "stm32mp15-ddr3-1x4Gb-1066-binG.dtsi" #include "stm32mp15-ddr3-2x4Gb-1066-binG.dtsi"
/ { @@ -16,6 +17,7 @@ config { u-boot,dm-pre-reloc; #gpio-cells = <2>; + dh,ddr3-coding-gpios = <&gpiog 0 0>, <&gpiog 1 0>; dh,som-coding-gpios = <&gpioz 7 0>, <&gpiof 3 0>; }; }; diff --git a/board/dhelectronics/dh_stm32mp1/board.c b/board/dhelectronics/dh_stm32mp1/board.c index 36b8652521..a081a3f86b 100644 --- a/board/dhelectronics/dh_stm32mp1/board.c +++ b/board/dhelectronics/dh_stm32mp1/board.c @@ -134,6 +134,7 @@ int checkboard(void) }
#ifdef CONFIG_SPL_BUILD +static u8 ddr3code __section("data"); static u8 somcode __section("data");
static void board_get_coding_straps(void) @@ -149,6 +150,16 @@ static void board_get_coding_straps(void) }
for (i = 0; i < 2; i++) { + ret = gpio_request_by_name_nodev(node, "dh,ddr3-coding-gpios", + i, &gpio, GPIOD_IS_IN); + if (ret) { + printf("%s: could not find a /config/dh,ddr3-coding-gpios[%i]\n", + __func__, i); + return; + } + + ddr3code |= !!dm_gpio_get_value(&gpio) << i; + ret = gpio_request_by_name_nodev(node, "dh,som-coding-gpios", i, &gpio, GPIOD_IS_IN); if (ret) { @@ -160,7 +171,7 @@ static void board_get_coding_straps(void) somcode |= !!dm_gpio_get_value(&gpio) << i; }
- printf("Code: SoM:%x\n", somcode); + printf("Code: DDR3:%x SoM:%x\n", ddr3code, somcode); }
void board_init_f(ulong dummy) @@ -206,6 +217,19 @@ void board_init_f(ulong dummy) } }
+int board_stm32mp1_ddr_config_name_match(struct udevice *dev, + const char *name) +{ + if (ddr3code == 2 && + !strcmp(name, "config@ddr3-1066-888-bin-g-1x4gb-533mhz")) + + if (ddr3code == 3 && + !strcmp(name, "config@ddr3-1066-888-bin-g-2x4gb-533mhz")) + return 0; + + return -EINVAL; +} + #ifdef CONFIG_SPL_LOAD_FIT int board_fit_config_name_match(const char *name) {
Dear Marek,
From: Marek Vasut marex@denx.de Sent: mercredi 1 avril 2020 01:48
The DHCOR board does exist in multiple variants with different DDR3 DRAM sizes. To cater for all of them, implement DDR3 code handling. There are two GPIOs which code the DRAM size populated on the SoM, read them out and use the value to pick the correct DDR3 config.
Signed-off-by: Marek Vasut marex@denx.de Cc: Manivannan Sadhasivam manivannan.sadhasivam@linaro.org Cc: Patrick Delaunay patrick.delaunay@st.com Cc: Patrice Chotard patrice.chotard@st.com
It is ok for me.
Only check the name check with remarks on previous patch of the serie.
Reviewed-by: Patrick Delaunay patrick.delaunay@st.com
Thanks
Patrick
Dear Marek,
From: Marek Vasut marex@denx.de Sent: mercredi 1 avril 2020 01:48
The AV96 board does exist in multiple variants. To cater for all of them, implement board code handling. There are two GPIOs which code the type of the board, read them out and use the value to pick the correct device tree from an fitImage.
Nice.
Signed-off-by: Marek Vasut marex@denx.de Cc: Manivannan Sadhasivam manivannan.sadhasivam@linaro.org Cc: Patrick Delaunay patrick.delaunay@st.com Cc: Patrice Chotard patrice.chotard@st.com
arch/arm/dts/stm32mp15xx-dhcor-u-boot.dtsi | 9 +++ arch/arm/mach-stm32mp/spl.c | 2 +- board/dhelectronics/dh_stm32mp1/board.c | 84 ++++++++++++++++++++++ board/dhelectronics/dh_stm32mp1/u-boot.its | 39 ++++++++++ configs/stm32mp15_dhcor_basic_defconfig | 3 + 5 files changed, 136 insertions(+), 1 deletion(-) create mode 100644 board/dhelectronics/dh_stm32mp1/u-boot.its
[...]
diff --git a/arch/arm/mach-stm32mp/spl.c b/arch/arm/mach-stm32mp/spl.c index ca4231cd0d..5461572090 100644 --- a/arch/arm/mach-stm32mp/spl.c +++ b/arch/arm/mach-stm32mp/spl.c @@ -76,7 +76,7 @@ void spl_display_print(void) } #endif
-void board_init_f(ulong dummy) +__weak void board_init_f(ulong dummy) { struct udevice *dev; int ret; diff --git a/board/dhelectronics/dh_stm32mp1/board.c b/board/dhelectronics/dh_stm32mp1/board.c index a3458a2623..36b8652521 100644 --- a/board/dhelectronics/dh_stm32mp1/board.c +++ b/board/dhelectronics/dh_stm32mp1/board.c @@ -133,6 +133,90 @@ int checkboard(void) return 0; }
It is really need to add weak and duplicate all this function ?
Is it possible to define a weak function called just before DDR init ?
As board_early_init_f for example....done in mach-rockchip/spl.c Or at91_spl_board_init() in mach-at91/spl_at91.c
In ./arch/arm/mach-stm32mp/spl.c
+__weak int board_early_init_f(void) +{ + return 0; +}
void board_init_f(ulong dummy) { struct udevice *dev; int ret;
arch_cpu_init();
[....] /* enable console uart printing */ preloader_console_init();
+ board_early_init_f();
ret = uclass_get_device(UCLASS_RAM, 0, &dev); if (ret) { printf("DRAM init failed: %d\n", ret); hang(); } }
[....]
+void board_init_f(ulong dummy) +{
- struct udevice *dev;
- int ret;
- arch_cpu_init();
[....]
- /* enable console uart printing */
- preloader_console_init();
- board_get_coding_straps();
- ret = uclass_get_device(UCLASS_RAM, 0, &dev);
- if (ret) {
printf("DRAM init failed: %d\n", ret);hang();- }
+}
With my proposal:
__weak int board_early_init_f(void) { board_get_coding_straps();
return 0; }
+#ifdef CONFIG_SPL_LOAD_FIT +int board_fit_config_name_match(const char *name) {
- if (somcode == 0 && !strcmp(name, "586-100"))
return 0;- return -EINVAL;
+} +#endif +#endif
static void board_key_check(void) { #if defined(CONFIG_FASTBOOT) || defined(CONFIG_CMD_STM32PROG) diff -- git a/board/dhelectronics/dh_stm32mp1/u-boot.its b/board/dhelectronics/dh_stm32mp1/u-boot.its new file mode 100644 index 0000000000..3ca3036f7e --- /dev/null +++ b/board/dhelectronics/dh_stm32mp1/u-boot.its
[...]
diff --git a/configs/stm32mp15_dhcor_basic_defconfig b/configs/stm32mp15_dhcor_basic_defconfig index 97e95bde7d..6c5ca31f40 100644 --- a/configs/stm32mp15_dhcor_basic_defconfig +++ b/configs/stm32mp15_dhcor_basic_defconfig @@ -11,7 +11,10 @@ CONFIG_SPL_SPI_SUPPORT=y CONFIG_SPL_TEXT_BASE=0x2FFC2500 CONFIG_DISTRO_DEFAULTS=y CONFIG_FIT=y +CONFIG_SPL_LOAD_FIT=y +CONFIG_SPL_FIT_SOURCE="board/dhelectronics/dh_stm32mp1/u-boot.its" CONFIG_BOOTCOMMAND="run bootcmd_stm32mp" +CONFIG_SPL_LEGACY_IMAGE_SUPPORT=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION=3 CONFIG_SPL_I2C_SUPPORT=y
To include the needed target in "make all" you can also add
+ CONFIG_BUILD_TARGET="u-boot.itb"
Or change default in Kconfig (add ARCH_STM32MP for SPL_LOAD_FIT case)
-- 2.25.1
Regards
Patrick
On 4/7/20 9:51 AM, Patrick DELAUNAY wrote:
Dear Marek,
Hi,
[...]
b/board/dhelectronics/dh_stm32mp1/board.c index a3458a2623..36b8652521 100644 --- a/board/dhelectronics/dh_stm32mp1/board.c +++ b/board/dhelectronics/dh_stm32mp1/board.c @@ -133,6 +133,90 @@ int checkboard(void) return 0; }
It is really need to add weak and duplicate all this function ?
Is it possible to define a weak function called just before DDR init ?
As board_early_init_f for example....done in mach-rockchip/spl.c Or at91_spl_board_init() in mach-at91/spl_at91.c
In ./arch/arm/mach-stm32mp/spl.c
+__weak int board_early_init_f(void) +{
- return 0;
+}
void board_init_f(ulong dummy) { struct udevice *dev; int ret;
arch_cpu_init();
[....] /* enable console uart printing */ preloader_console_init();
- board_early_init_f();
[...]
diff --git a/configs/stm32mp15_dhcor_basic_defconfig b/configs/stm32mp15_dhcor_basic_defconfig index 97e95bde7d..6c5ca31f40 100644 --- a/configs/stm32mp15_dhcor_basic_defconfig +++ b/configs/stm32mp15_dhcor_basic_defconfig @@ -11,7 +11,10 @@ CONFIG_SPL_SPI_SUPPORT=y CONFIG_SPL_TEXT_BASE=0x2FFC2500 CONFIG_DISTRO_DEFAULTS=y CONFIG_FIT=y +CONFIG_SPL_LOAD_FIT=y +CONFIG_SPL_FIT_SOURCE="board/dhelectronics/dh_stm32mp1/u-boot.its" CONFIG_BOOTCOMMAND="run bootcmd_stm32mp" +CONFIG_SPL_LEGACY_IMAGE_SUPPORT=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_PARTITION=y CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION=3 CONFIG_SPL_I2C_SUPPORT=y
To include the needed target in "make all" you can also add
- CONFIG_BUILD_TARGET="u-boot.itb"
Or change default in Kconfig (add ARCH_STM32MP for SPL_LOAD_FIT case)
I very much have both in my tree already, so I'll send a non-RFC.
participants (2)
-
Marek Vasut -
Patrick DELAUNAY