
On 09/06/2022 23:49, Heiko Thiery wrote:
To have only one place to describe the binman images us the imx8mn-u-boot.dtsi. To have support for different DDR firmwares this nodes are included dependent on the used DDR config option.
Signed-off-by: Heiko Thiery heiko.thiery@gmail.com Reviewed-by: Fabio Estevam festevam@gmail.com
v2: sync with current master and fix merge conflict
arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi | 125 +----------------- arch/arm/dts/imx8mn-evk-u-boot.dtsi | 120 +---------------- arch/arm/dts/imx8mn-u-boot.dtsi | 156 +++++++++++++++++++++++
I see most of this is moving existing definitions, but I'm adding comments in a general sense, partially for future reference.
3 files changed, 159 insertions(+), 242 deletions(-) create mode 100644 arch/arm/dts/imx8mn-u-boot.dtsi
diff --git a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi index 4d0ecb07d4..3d0e817313 100644 --- a/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-ddr4-evk-u-boot.dtsi @@ -3,11 +3,9 @@
- Copyright 2019, 2021 NXP
*/
-/ {
- binman: binman {
multiple-images;
- };
+#include "imx8mn-u-boot.dtsi"
+/ { wdt-reboot { compatible = "wdt-reboot"; wdt = <&wdog1>; @@ -143,122 +141,3 @@ [...] diff --git a/arch/arm/dts/imx8mn-evk-u-boot.dtsi b/arch/arm/dts/imx8mn-evk-u-boot.dtsi index d1427941eb..339c3dd681 100644 --- a/arch/arm/dts/imx8mn-evk-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-evk-u-boot.dtsi @@ -3,6 +3,7 @@
- Copyright 2019 NXP
*/
+#include "imx8mn-u-boot.dtsi" #include "imx8mn-ddr4-evk-u-boot.dtsi"
"imx8mn-ddr4-evk-u-boot.dtsi" already includes "imx8mn-u-boot.dtsi". I think it should only be included once.
I don't know why these are structured this way, but I think common parts of the two boards should've been in a "imx8mn-evk-common-u-boot.dtsi" or something which the board -u-boot.dtsi files would include.
&i2c1 { @@ -24,122 +25,3 @@ [...] diff --git a/arch/arm/dts/imx8mn-u-boot.dtsi b/arch/arm/dts/imx8mn-u-boot.dtsi new file mode 100644 index 0000000000..7b591085a0 --- /dev/null +++ b/arch/arm/dts/imx8mn-u-boot.dtsi @@ -0,0 +1,156 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright 2019 NXP
- */
+/ {
- binman: binman {
multiple-images;
- };
+};
+&binman {
u_boot_spl_ddr: u-boot-spl-ddr {
filename = "u-boot-spl-ddr.bin";
pad-byte = <0xff>;
align-size = <4>;
align = <4>;
u-boot-spl {
align-end = <4>;
};
Maybe it's better to add an empty 'ddr-fw' section here, and populate that below.
- };
- spl {
filename = "spl.bin";
mkimage {
args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e 0x912000";
blob {
filename = "u-boot-spl-ddr.bin";
};
};
- };
- itb {
filename = "u-boot.itb";
fit {
description = "Configuration to load ATF before U-Boot";
#address-cells = <1>;
fit,external-offset = <CONFIG_FIT_EXTERNAL_OFFSET>;
images {
uboot {
description = "U-Boot (64-bit)";
type = "standalone";
arch = "arm64";
compression = "none";
load = <CONFIG_SYS_TEXT_BASE>;
uboot_blob: blob-ext {
filename = "u-boot-nodtb.bin";
};
};
atf {
description = "ARM Trusted Firmware";
type = "firmware";
arch = "arm64";
compression = "none";
load = <0x960000>;
entry = <0x960000>;
atf_blob: blob-ext {
filename = "bl31.bin";
};
};
fdt {
description = "NAME";
type = "flat_dt";
compression = "none";
uboot_fdt_blob: blob-ext {
filename = "u-boot.dtb";
};
};
These three 'blob-ext's would normally be 'u-boot-nodtb', 'atf-bl31', and 'u-boot-dtb' entries respectively.
};
configurations {
default = "conf";
conf {
description = "NAME";
This 'NAME' (and the fdt one) shows up in `mkimage -l u-boot.itb` output. Perhaps they were originally copied from a @fdt-SEQ version (which replaces it with the actual dtb filename).
firmware = "uboot";
loadables = "atf";
fdt = "fdt";
};
};
};
- };
- imx-boot {
filename = "flash.bin";
pad-byte = <0x00>;
spl: blob-ext@1 {
offset = <0x0>;
filename = "spl.bin";
};
uboot: blob-ext@2 {
offset = <0x58000>;
filename = "u-boot.itb";
};
- };
I dislike having 'blob-ext's as names, they should be descriptive. 'imx8mm-u-boot.dtsi' has these labels as node names which is slightly better.
+};
+#ifdef CONFIG_IMX8M_DDR4 +&u_boot_spl_ddr {
- blob_1: blob-ext@1 {
filename = "ddr4_imem_1d_201810.bin";
size = <0x8000>;
- };
- blob_2: blob-ext@2 {
filename = "ddr4_dmem_1d_201810.bin";
size = <0x4000>;
- };
- blob_3: blob-ext@3 {
filename = "ddr4_imem_2d_201810.bin";
size = <0x8000>;
- };
- blob_4: blob-ext@4 {
filename = "ddr4_dmem_2d_201810.bin";
size = <0x4000>;
- };
+}; +#elif CONFIG_IMX8M_LPDDR4 +&u_boot_spl_ddr {
- blob_1: blob-ext@1 {
filename = "lpddr4_pmu_train_1d_imem.bin";
size = <0x8000>;
- };
- blob_2: blob-ext@2 {
filename = "lpddr4_pmu_train_1d_dmem.bin";
size = <0x4000>;
- };
- blob_3: blob-ext@3 {
filename = "lpddr4_pmu_train_2d_imem.bin";
size = <0x8000>;
- };
- blob_4: blob-ext@4 {
filename = "lpddr4_pmu_train_2d_dmem.bin";
size = <0x4000>;
- };
+};
These conflict a bit with one of Peng's series which changes the names and removes the size for these blobs, see [1] [2].
[1] arm: dts: imx8m: update binman ddr firmware node name https://lore.kernel.org/u-boot/20220603071715.15212-4-peng.fan@oss.nxp.com/
[2] arm: dts: imx8m: shrink ddr firmware size to actual file size https://lore.kernel.org/u-boot/20220603071715.15212-7-peng.fan@oss.nxp.com/
+#else
- #error "no valid ddr config selected"
There's also IMX8M_DDR3L, see imx8mn-bsh-smm-s2-u-boot-common.dtsi
+#endif