[PATCH v2 1/2] ARM: imx: imx8mn-evk: generate a single bootable flash.bin
To have a flash.bin file that also contains the U-Boot and TF-A/ATF create this like already done for other imx8 boards.
Signed-off-by: Heiko Thiery heiko.thiery@gmail.com Reviewed-by: Fabio Estevam festevam@gmail.com Reviewed-by: Peng Fan peng.fan@nxp.com --- v2: sync with current master and fix merge conflict
arch/arm/dts/imx8mn-evk-u-boot.dtsi | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/arch/arm/dts/imx8mn-evk-u-boot.dtsi b/arch/arm/dts/imx8mn-evk-u-boot.dtsi index 3db46d4cbc..d1427941eb 100644 --- a/arch/arm/dts/imx8mn-evk-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-evk-u-boot.dtsi @@ -58,7 +58,9 @@ };
- flash { + spl { + filename = "spl.bin"; + mkimage { args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e 0x912000";
@@ -125,4 +127,19 @@ }; }; }; + + 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"; + }; + }; };
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 +++++++++++++++++++++++ 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 @@ &wdog1 { u-boot,dm-spl; }; - -&binman { - u-boot-spl-ddr { - filename = "u-boot-spl-ddr.bin"; - pad-byte = <0xff>; - align-size = <4>; - align = <4>; - - u-boot-spl { - align-end = <4>; - }; - - 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>; - }; - }; - - - 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"; - }; - }; - }; - - configurations { - default = "conf"; - - conf { - description = "NAME"; - 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"; - }; - }; -}; 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"
&i2c1 { @@ -24,122 +25,3 @@ &pinctrl_pmic { u-boot,dm-spl; }; - -&binman { - u-boot-spl-ddr { - filename = "u-boot-spl-ddr.bin"; - pad-byte = <0xff>; - align-size = <4>; - align = <4>; - - u-boot-spl { - align-end = <4>; - }; - - 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>; - }; - }; - - - 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"; - }; - }; - }; - - configurations { - default = "conf"; - - conf { - description = "NAME"; - 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"; - }; - }; -}; 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>; + }; + }; + + 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"; + }; + }; + }; + + configurations { + default = "conf"; + + conf { + description = "NAME"; + 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"; + }; + }; +}; + +#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>; + }; +}; +#else + #error "no valid ddr config selected" +#endif
On Thu, Jun 9, 2022 at 1:50 PM Heiko Thiery heiko.thiery@gmail.com 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 +++++++++++++++++++++++ 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 @@ &wdog1 { u-boot,dm-spl; };
-&binman {
u-boot-spl-ddr {filename = "u-boot-spl-ddr.bin";pad-byte = <0xff>;align-size = <4>;align = <4>;u-boot-spl {align-end = <4>;};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>;};};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";};};};configurations {default = "conf";conf {description = "NAME";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";};};-}; 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"
&i2c1 { @@ -24,122 +25,3 @@ &pinctrl_pmic { u-boot,dm-spl; };
-&binman {
u-boot-spl-ddr {filename = "u-boot-spl-ddr.bin";pad-byte = <0xff>;align-size = <4>;align = <4>;u-boot-spl {align-end = <4>;};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>;};};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";};};};configurations {default = "conf";conf {description = "NAME";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";};};-}; 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>;};};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";};};};configurations {default = "conf";conf {description = "NAME";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";};};+};
+#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>;};+}; +#else
#error "no valid ddr config selected"+#endif
2.20.1
Heiko,
You can add multi-dtb support to this so that it's usable by the other imx8mn boards with the following: diff --git a/arch/arm/dts/imx8mn-u-boot.dtsi b/arch/arm/dts/imx8mn-u-boot.dtsi index 7b591085a0be..af6697b1efbc 100644 --- a/arch/arm/dts/imx8mn-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-u-boot.dtsi @@ -38,6 +38,7 @@
fit { description = "Configuration to load ATF before U-Boot"; + fit,fdt-list = "of-list"; #address-cells = <1>; fit,external-offset = <CONFIG_FIT_EXTERNAL_OFFSET>;
@@ -67,7 +68,7 @@ }; };
- fdt { + @fdt-SEQ { description = "NAME"; type = "flat_dt"; compression = "none"; @@ -79,13 +80,13 @@ };
configurations { - default = "conf"; + default = "@config-DEFAULT-SEQ";
- conf { + binman_configuration: @config-SEQ { description = "NAME"; firmware = "uboot"; loadables = "atf"; - fdt = "fdt"; + fdt = "fdt-SEQ"; }; }; };
I don't mind sending this as a follow-up to your patch here.
It looks like there are only the following boards in mainline that would benefit from using this shared include: imx8mn-beacon-kit-u-boot.dtsi imx8mn-var-som-symphony-u-boot.dtsi imx8mn-venice-u-boot.dtsi
Have you compared the binman portions of imx8m{m,n,p}-u-boot.dtsi? There are a lot of differences due to different property ordering and label/node naming conventions. I would like to see these normalized but i'm not clear which is the best example to normalize to. Specifically I don't know: 1. what is the convention for property ordering in dt... is it simply alphabetical order? 2. have we settled on a convention for the blob naming, if so what is the best example?
Best Regards,
Tim
Hi Tim, Hi Simon,
[SNIP]
Heiko,
You can add multi-dtb support to this so that it's usable by the other imx8mn boards with the following: diff --git a/arch/arm/dts/imx8mn-u-boot.dtsi b/arch/arm/dts/imx8mn-u-boot.dtsi index 7b591085a0be..af6697b1efbc 100644 --- a/arch/arm/dts/imx8mn-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-u-boot.dtsi @@ -38,6 +38,7 @@
fit { description = "Configuration to load ATF before U-Boot";
fit,fdt-list = "of-list"; #address-cells = <1>; fit,external-offset = <CONFIG_FIT_EXTERNAL_OFFSET>;@@ -67,7 +68,7 @@ }; };
fdt {
@fdt-SEQ { description = "NAME"; type = "flat_dt"; compression = "none";@@ -79,13 +80,13 @@ };
configurations {
default = "conf";
default = "@config-DEFAULT-SEQ";
conf {
binman_configuration: @config-SEQ { description = "NAME"; firmware = "uboot"; loadables = "atf";
fdt = "fdt";
fdt = "fdt-SEQ"; }; }; };I don't mind sending this as a follow-up to your patch here.
Since this patch moves the parts from the 2 imx8mn-evk boards to one "common" file it would be better to do more changes on that in a separate patch.
It looks like there are only the following boards in mainline that would benefit from using this shared include: imx8mn-beacon-kit-u-boot.dtsi imx8mn-var-som-symphony-u-boot.dtsi imx8mn-venice-u-boot.dtsi
Have you compared the binman portions of imx8m{m,n,p}-u-boot.dtsi?
No not yet.
There are a lot of differences due to different property ordering and label/node naming conventions. I would like to see these normalized but i'm not clear which is the best example to normalize to. Specifically I don't know:
- what is the convention for property ordering in dt... is it simply
alphabetical order? 2. have we settled on a convention for the blob naming, if so what is the best example?
I am not aware that there is a conventional here. But maybe simon can give some hints here.
On 10/06/2022 00:38, Heiko Thiery wrote:
Hi Tim, Hi Simon,
[SNIP]
Heiko,
You can add multi-dtb support to this so that it's usable by the other imx8mn boards with the following:
[...]
I don't mind sending this as a follow-up to your patch here.
Since this patch moves the parts from the 2 imx8mn-evk boards to one "common" file it would be better to do more changes on that in a separate patch.
It looks like there are only the following boards in mainline that would benefit from using this shared include: imx8mn-beacon-kit-u-boot.dtsi imx8mn-var-som-symphony-u-boot.dtsi imx8mn-venice-u-boot.dtsi
There's also 'imx8mn-bsh-smm-s2-u-boot-common.dtsi'. Slightly different because only half of the blobs are there with IMX8M_DDR3L.
Have you compared the binman portions of imx8m{m,n,p}-u-boot.dtsi?
No not yet.
I looked a bit and they look very much alike. I suspect it's possible to eventually unify everything into a shared 'imx8m-u-boot.dtsi', but I didn't actually try.
There are a lot of differences due to different property ordering and label/node naming conventions. I would like to see these normalized but i'm not clear which is the best example to normalize to. Specifically I don't know:
- what is the convention for property ordering in dt... is it simply
alphabetical order?
AFAIK property order doesn't matter for binman. Node order is very significant though. For names, I think we should:
- Prefer hyphens to underscores - Prefer lowercase to uppercase - Prefer meaningful names to things like 'blob-ext@1' - Avoid first char being a number (because labels can't have that)
- have we settled on a convention for the blob naming, if so what is
the best example?
I had suggested 'ddr-1d-imem-fw' etc. on a series from Peng, see latest version of it [1].
[1] arm: dts: imx8m: update binman ddr firmware node name https://lore.kernel.org/u-boot/20220603071715.15212-4-peng.fan@oss.nxp.com/
I am not aware that there is a conventional here. But maybe simon can give some hints here.
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
Hi Alper.
Am Mi., 22. Juni 2022 um 20:18 Uhr schrieb Alper Nebi Yasak alpernebiyasak@gmail.com:
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.
I have implemented your comments to prepare the dtb files for uniform use and will send an updated version.
[SNIP]
Thanks
On 09/06/2022 23:49, Heiko Thiery wrote:
To have a flash.bin file that also contains the U-Boot and TF-A/ATF create this like already done for other imx8 boards.
Signed-off-by: Heiko Thiery heiko.thiery@gmail.com Reviewed-by: Fabio Estevam festevam@gmail.com Reviewed-by: Peng Fan peng.fan@nxp.com
v2: sync with current master and fix merge conflict
arch/arm/dts/imx8mn-evk-u-boot.dtsi | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/arch/arm/dts/imx8mn-evk-u-boot.dtsi b/arch/arm/dts/imx8mn-evk-u-boot.dtsi index 3db46d4cbc..d1427941eb 100644 --- a/arch/arm/dts/imx8mn-evk-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-evk-u-boot.dtsi @@ -58,7 +58,9 @@ };
- flash {
- spl {
filename = "spl.bin";- mkimage { args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e 0x912000";
This change got applied in the meantime via another patch [1].
[1] imx8mn_evk: Add the missing spl.bin entry https://lore.kernel.org/u-boot/20220503190304.413968-1-festevam@gmail.com/
@@ -125,4 +127,19 @@ }; }; };
- 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";};- };
};
This is already inherited from an #included dtsi, so not strictly necessary. I think it would be clearer to include this here as well, but since you're removing it in the next patch anyway you can drop this patch entirely.
Hi Alper
Am Mi., 22. Juni 2022 um 20:18 Uhr schrieb Alper Nebi Yasak alpernebiyasak@gmail.com:
On 09/06/2022 23:49, Heiko Thiery wrote:
To have a flash.bin file that also contains the U-Boot and TF-A/ATF create this like already done for other imx8 boards.
Signed-off-by: Heiko Thiery heiko.thiery@gmail.com Reviewed-by: Fabio Estevam festevam@gmail.com Reviewed-by: Peng Fan peng.fan@nxp.com
v2: sync with current master and fix merge conflict
arch/arm/dts/imx8mn-evk-u-boot.dtsi | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/arch/arm/dts/imx8mn-evk-u-boot.dtsi b/arch/arm/dts/imx8mn-evk-u-boot.dtsi index 3db46d4cbc..d1427941eb 100644 --- a/arch/arm/dts/imx8mn-evk-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-evk-u-boot.dtsi @@ -58,7 +58,9 @@ };
flash {
spl {filename = "spl.bin";mkimage { args = "-n spl/u-boot-spl.cfgout -T imx8mimage -e 0x912000";This change got applied in the meantime via another patch [1].
ok
[1] imx8mn_evk: Add the missing spl.bin entry https://lore.kernel.org/u-boot/20220503190304.413968-1-festevam@gmail.com/
@@ -125,4 +127,19 @@ }; }; };
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";};};};
This is already inherited from an #included dtsi, so not strictly necessary. I think it would be clearer to include this here as well, but since you're removing it in the next patch anyway you can drop this patch entirely.
You're right. This is not required. Therefore I set the state to rejected in patchwork.
Thanks Heiko
participants (3)
-
Alper Nebi Yasak -
Heiko Thiery -
Tim Harvey