RE: [PATCH V4 2/8] arm: dts: imx8m: update binman ddr firmware node name

Subject: Re: [PATCH V4 2/8] arm: dts: imx8m: update binman ddr firmware node name

On 20/05/2022 17:10, Peng Fan (OSS) wrote:

...

From: Peng Fan peng.fan@nxp.com

We are migrating to use BINMAN SYMBOLS, the current name is not a valid binman type, so update to use blob-ext@[1,2,3,4].

Tested-by: Tim Harvey tharvey@gateworks.com #imx8m[m,n,p]-venice Signed-off-by: Peng Fan peng.fan@nxp.com

---

arch/arm/dts/imx8mm-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi | 4 ++-- arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mn-venice-u-boot.dtsi | 8 ++++---- arch/arm/dts/imx8mq-u-boot.dtsi | 8 ++++---- 5 files changed, 18 insertions(+), 18 deletions(-)

I think descriptive entry names are better in general, so I prefer the old names to a bunch of 'blob-ext's. Symbol resolution is done using the entry names anyway, I don't see why this change would be necessary.

Binman understand blob-ext, it not understand 1d-imem. If keep id-imem, or else, need to add several new binman types which I would not prefer.

Thanks, Peng.

In any case, the names and labels should be consistent across the different dts files, assuming the blobs have the same purpose / meaning. I see that labels starting with digits cause a dtc parse error, so the entries could be `imem_1d: imem-1d { ... };` and so on.

Also, the blobs essentially look the same to me aside from their filenames, just duplicated over different imx8m* variants and boards. Maybe binman parts could be unified into a single dtsi file later on? (Filename differences can be handled by new 'blob_named_by_arg's btw.)

...

diff --git a/arch/arm/dts/imx8mm-u-boot.dtsi b/arch/arm/dts/imx8mm-u-boot.dtsi index 9f66cdb65a9..5de55a2d80b 100644 --- a/arch/arm/dts/imx8mm-u-boot.dtsi +++ b/arch/arm/dts/imx8mm-u-boot.dtsi @@ -39,25 +39,25 @@ filename = "u-boot-spl.bin"; };

• 1d-imem {
  

• imem_1d: blob-ext@1 {
  filename = "lpddr4_pmu_train_1d_imem.bin";
  size = <0x8000>;
  type = "blob-ext";
  

  };

• 1d-dmem {
  

• dmem_1d: blob-ext@2 {
  filename = "lpddr4_pmu_train_1d_dmem.bin";
  size = <0x4000>;
  type = "blob-ext";
  

  };

• 2d-imem {
  

• imem_2d: blob-ext@3 {
  filename = "lpddr4_pmu_train_2d_imem.bin";
  size = <0x8000>;
  type = "blob-ext";
  

  };

• 2d-dmem {
  

• dmem_2d: blob-ext@4 {
  filename = "lpddr4_pmu_train_2d_dmem.bin";
  size = <0x4000>;
  type = "blob-ext";
  

diff --git a/arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi b/arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi index 46a9d7fd78b..5a52b73d7e9 100644 --- a/arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi +++ b/arch/arm/dts/imx8mn-bsh-smm-s2-u-boot-common.dtsi @@ -111,13 +111,13 @@ filename = "u-boot-spl.bin"; };

• 1d-imem {
  

• imem_1d: blob-ext@1 {
  filename = "ddr3_imem_1d.bin";
  size = <0x8000>;
  type = "blob-ext";
  

  };

• 1d_dmem {
  

• dmem_1d: blob-ext@2 {
  filename = "ddr3_dmem_1d.bin";
  size = <0x4000>;
  type = "blob-ext";
  

diff --git a/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi b/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi index 6e37622cca7..001e725f568 100644 --- a/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-var-som-symphony-u-boot.dtsi @@ -130,25 +130,25 @@ filename = "u-boot-spl.bin"; };

• 1d-imem {
  

• blob_1: blob-ext@1 {
  filename = "ddr4_imem_1d.bin";
  size = <0x8000>;
  type = "blob-ext";
  

  };

• 1d_dmem {
  

• blob_2: blob-ext@2 {
  filename = "ddr4_dmem_1d.bin";
  size = <0x4000>;
  type = "blob-ext";
  

  };

• 2d_imem {
  

• blob_3: blob-ext@3 {
  filename = "ddr4_imem_2d.bin";
  size = <0x8000>;
  type = "blob-ext";
  

  };

• 2d_dmem {
  

• blob_4: blob-ext@4 {
  filename = "ddr4_dmem_2d.bin";
  size = <0x4000>;
  type = "blob-ext";
  

diff --git a/arch/arm/dts/imx8mn-venice-u-boot.dtsi b/arch/arm/dts/imx8mn-venice-u-boot.dtsi index 35819553879..67922146963 100644 --- a/arch/arm/dts/imx8mn-venice-u-boot.dtsi +++ b/arch/arm/dts/imx8mn-venice-u-boot.dtsi @@ -126,25 +126,25 @@ filename = "u-boot-spl.bin"; };

• 1d-imem {
  

• imem_1d: blob-ext@1 {
  filename = "lpddr4_pmu_train_1d_imem.bin";
  size = <0x8000>;
  type = "blob-ext";
  

  };

• 1d_dmem {
  

• dmem_1d: blob-ext@2 {
  filename = "lpddr4_pmu_train_1d_dmem.bin";
  size = <0x4000>;
  type = "blob-ext";
  

  };

• 2d_imem {
  

• imem_2d: blob-ext@3 {
  filename = "lpddr4_pmu_train_2d_imem.bin";
  size = <0x8000>;
  type = "blob-ext";
  

  };

• 2d_dmem {
  

• dmem_2d: blob-ext@4 {
  filename = "lpddr4_pmu_train_2d_dmem.bin";
  size = <0x4000>;
  type = "blob-ext";
  

diff --git a/arch/arm/dts/imx8mq-u-boot.dtsi b/arch/arm/dts/imx8mq-u-boot.dtsi index 912a3d4a356..389414ad26f 100644 --- a/arch/arm/dts/imx8mq-u-boot.dtsi +++ b/arch/arm/dts/imx8mq-u-boot.dtsi @@ -46,25 +46,25 @@ filename = "u-boot-spl.bin"; };

• 1d-imem {
  

• imem_1d: blob-ext@1 {
  filename = "lpddr4_pmu_train_1d_imem.bin";
  size = <0x8000>;
  type = "blob-ext";
  

  };

• 1d-dmem {
  

• dmem_1d: blob-ext@2 {
  filename = "lpddr4_pmu_train_1d_dmem.bin";
  size = <0x4000>;
  type = "blob-ext";
  

  };

• 2d-imem {
  

• imem_2d: blob-ext@3 {
  filename = "lpddr4_pmu_train_2d_imem.bin";
  size = <0x8000>;
  type = "blob-ext";
  

  };

• 2d-dmem {
  

• dmem_2d: blob-ext@4 {
  filename = "lpddr4_pmu_train_2d_dmem.bin";
  size = <0x4000>;
  type = "blob-ext";