> 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....@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 <thar...@gateworks.com> #imx8m[m,n,p]-venice
> > Signed-off-by: Peng Fan <peng....@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";
