All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] ARM: imx: imx8mn-evk: generate a single bootable flash.bin
@ 2022-06-09 20:49 Heiko Thiery
  2022-06-09 20:49 ` [PATCH v2 2/2] ARM: imx: imx8mn-evk: use one common u-boot.dtsi for the evk boards Heiko Thiery
  2022-06-22 18:16 ` [PATCH v2 1/2] ARM: imx: imx8mn-evk: generate a single bootable flash.bin Alper Nebi Yasak
  0 siblings, 2 replies; 9+ messages in thread
From: Heiko Thiery @ 2022-06-09 20:49 UTC (permalink / raw)
  To: u-boot
  Cc: Ying-Chun Liu, Peng Fan, Fabio Estevam, Heiko Thiery,
	Marek Vasut, Marcel Ziswiler, Tim Harvey, Sean Anderson,
	Thomas Schäfer, Stefano Babic

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";
+		};
+	};
 };
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/2] ARM: imx: imx8mn-evk: use one common u-boot.dtsi for the evk boards
  2022-06-09 20:49 [PATCH v2 1/2] ARM: imx: imx8mn-evk: generate a single bootable flash.bin Heiko Thiery
@ 2022-06-09 20:49 ` Heiko Thiery
  2022-06-09 21:17   ` Tim Harvey
  2022-06-22 18:16   ` Alper Nebi Yasak
  2022-06-22 18:16 ` [PATCH v2 1/2] ARM: imx: imx8mn-evk: generate a single bootable flash.bin Alper Nebi Yasak
  1 sibling, 2 replies; 9+ messages in thread
From: Heiko Thiery @ 2022-06-09 20:49 UTC (permalink / raw)
  To: u-boot
  Cc: Ying-Chun Liu, Peng Fan, Fabio Estevam, Heiko Thiery,
	Marek Vasut, Marcel Ziswiler, Tim Harvey, Sean Anderson,
	Thomas Schäfer, Stefano Babic

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] ARM: imx: imx8mn-evk: use one common u-boot.dtsi for the evk boards
  2022-06-09 20:49 ` [PATCH v2 2/2] ARM: imx: imx8mn-evk: use one common u-boot.dtsi for the evk boards Heiko Thiery
@ 2022-06-09 21:17   ` Tim Harvey
  2022-06-09 21:38     ` Heiko Thiery
  2022-06-22 18:16   ` Alper Nebi Yasak
  1 sibling, 1 reply; 9+ messages in thread
From: Tim Harvey @ 2022-06-09 21:17 UTC (permalink / raw)
  To: Heiko Thiery
  Cc: u-boot, Ying-Chun Liu, Peng Fan, Fabio Estevam, Marek Vasut,
	Marcel Ziswiler, Sean Anderson, Thomas Schäfer,
	Stefano Babic

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

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] ARM: imx: imx8mn-evk: use one common u-boot.dtsi for the evk boards
  2022-06-09 21:17   ` Tim Harvey
@ 2022-06-09 21:38     ` Heiko Thiery
  2022-06-22 18:16       ` Alper Nebi Yasak
  0 siblings, 1 reply; 9+ messages in thread
From: Heiko Thiery @ 2022-06-09 21:38 UTC (permalink / raw)
  To: Tim Harvey
  Cc: u-boot, Ying-Chun Liu, Peng Fan, Fabio Estevam, Marek Vasut,
	Marcel Ziswiler, Sean Anderson, Thomas Schäfer,
	Stefano Babic, Simon Glass

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:
> 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?

I am not aware that there is a conventional here. But maybe simon can
give some hints here.

-- 
Heiko


--
Heiko

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/2] ARM: imx: imx8mn-evk: generate a single bootable flash.bin
  2022-06-09 20:49 [PATCH v2 1/2] ARM: imx: imx8mn-evk: generate a single bootable flash.bin Heiko Thiery
  2022-06-09 20:49 ` [PATCH v2 2/2] ARM: imx: imx8mn-evk: use one common u-boot.dtsi for the evk boards Heiko Thiery
@ 2022-06-22 18:16 ` Alper Nebi Yasak
  2022-06-26  9:49   ` Heiko Thiery
  1 sibling, 1 reply; 9+ messages in thread
From: Alper Nebi Yasak @ 2022-06-22 18:16 UTC (permalink / raw)
  To: Heiko Thiery
  Cc: Ying-Chun Liu, Peng Fan, Fabio Estevam, Marek Vasut,
	Marcel Ziswiler, Tim Harvey, Sean Anderson, Thomas Schäfer,
	Stefano Babic, u-boot

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.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] ARM: imx: imx8mn-evk: use one common u-boot.dtsi for the evk boards
  2022-06-09 20:49 ` [PATCH v2 2/2] ARM: imx: imx8mn-evk: use one common u-boot.dtsi for the evk boards Heiko Thiery
  2022-06-09 21:17   ` Tim Harvey
@ 2022-06-22 18:16   ` Alper Nebi Yasak
  2022-06-26  9:53     ` Heiko Thiery
  1 sibling, 1 reply; 9+ messages in thread
From: Alper Nebi Yasak @ 2022-06-22 18:16 UTC (permalink / raw)
  To: Heiko Thiery
  Cc: Ying-Chun Liu, Fabio Estevam, Marek Vasut, Marcel Ziswiler,
	Tim Harvey, Sean Anderson, Thomas Schäfer, Stefano Babic,
	u-boot, Peng Fan

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] ARM: imx: imx8mn-evk: use one common u-boot.dtsi for the evk boards
  2022-06-09 21:38     ` Heiko Thiery
@ 2022-06-22 18:16       ` Alper Nebi Yasak
  0 siblings, 0 replies; 9+ messages in thread
From: Alper Nebi Yasak @ 2022-06-22 18:16 UTC (permalink / raw)
  To: Heiko Thiery, Tim Harvey
  Cc: u-boot, Ying-Chun Liu, Peng Fan, Fabio Estevam, Marek Vasut,
	Marcel Ziswiler, Sean Anderson, Thomas Schäfer,
	Stefano Babic, Simon Glass

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:
>> 1. 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)

>> 2. 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.
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 1/2] ARM: imx: imx8mn-evk: generate a single bootable flash.bin
  2022-06-22 18:16 ` [PATCH v2 1/2] ARM: imx: imx8mn-evk: generate a single bootable flash.bin Alper Nebi Yasak
@ 2022-06-26  9:49   ` Heiko Thiery
  0 siblings, 0 replies; 9+ messages in thread
From: Heiko Thiery @ 2022-06-26  9:49 UTC (permalink / raw)
  To: Alper Nebi Yasak
  Cc: Ying-Chun Liu, Peng Fan, Fabio Estevam, Marek Vasut,
	Marcel Ziswiler, Tim Harvey, Sean Anderson, Thomas Schäfer,
	Stefano Babic, u-boot

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] ARM: imx: imx8mn-evk: use one common u-boot.dtsi for the evk boards
  2022-06-22 18:16   ` Alper Nebi Yasak
@ 2022-06-26  9:53     ` Heiko Thiery
  0 siblings, 0 replies; 9+ messages in thread
From: Heiko Thiery @ 2022-06-26  9:53 UTC (permalink / raw)
  To: Alper Nebi Yasak
  Cc: Ying-Chun Liu, Fabio Estevam, Marek Vasut, Marcel Ziswiler,
	Tim Harvey, Sean Anderson, Thomas Schäfer, Stefano Babic,
	u-boot, Peng Fan

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
-- 
Heiko

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-06-26  9:53 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-06-09 20:49 [PATCH v2 1/2] ARM: imx: imx8mn-evk: generate a single bootable flash.bin Heiko Thiery
2022-06-09 20:49 ` [PATCH v2 2/2] ARM: imx: imx8mn-evk: use one common u-boot.dtsi for the evk boards Heiko Thiery
2022-06-09 21:17   ` Tim Harvey
2022-06-09 21:38     ` Heiko Thiery
2022-06-22 18:16       ` Alper Nebi Yasak
2022-06-22 18:16   ` Alper Nebi Yasak
2022-06-26  9:53     ` Heiko Thiery
2022-06-22 18:16 ` [PATCH v2 1/2] ARM: imx: imx8mn-evk: generate a single bootable flash.bin Alper Nebi Yasak
2022-06-26  9:49   ` Heiko Thiery

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.