[PATCH 1/2] imx: Drop unneeded phandle in FIT template

Simon Glass sjg at chromium.org
Mon Aug 28 19:55:00 CEST 2023


Hi Tim,

On Mon, 28 Aug 2023 at 11:33, Tim Harvey <tharvey at gateworks.com> wrote:
>
> On Wed, Aug 23, 2023 at 6:18 PM Simon Glass <sjg at chromium.org> wrote:
> >
> > Adding a phandle to a template node is not allowed, since when the node is
> > instantiated multiple times, we end up with duplicate phandles.
> >
> > Drop this invalid constructs.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> >  arch/arm/dts/imx8mm-cl-iot-gate-optee-u-boot.dtsi | 2 ++
> >  arch/arm/dts/imx8mm-u-boot.dtsi                   | 2 +-
> >  arch/arm/dts/imx8mn-u-boot.dtsi                   | 2 +-
> >  arch/arm/dts/imx8mp-rsb3720-a1-u-boot.dtsi        | 2 ++
> >  arch/arm/dts/imx8mp-u-boot.dtsi                   | 4 ++--
> >  arch/arm/dts/imx8qm-u-boot.dtsi                   | 2 +-
> >  arch/arm/dts/k3-am65-iot2050-boot-image.dtsi      | 8 ++++----
> >  7 files changed, 13 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/arm/dts/imx8mm-cl-iot-gate-optee-u-boot.dtsi b/arch/arm/dts/imx8mm-cl-iot-gate-optee-u-boot.dtsi
> > index 484e31824b85..d93e1cbd8a71 100644
> > --- a/arch/arm/dts/imx8mm-cl-iot-gate-optee-u-boot.dtsi
> > +++ b/arch/arm/dts/imx8mm-cl-iot-gate-optee-u-boot.dtsi
> > @@ -41,9 +41,11 @@
> >         };
> >  };
> >
> > +/* This cannot work since it refers to a template node
> >  &binman_configuration {
> >         loadables = "atf", "fip";
> >  };
> > +*/
> >
> >  &fec1 {
> >         phy-reset-gpios = <&gpio4 22 GPIO_ACTIVE_LOW>;
> > diff --git a/arch/arm/dts/imx8mm-u-boot.dtsi b/arch/arm/dts/imx8mm-u-boot.dtsi
> > index 035282bf0b00..6085128e24ec 100644
> > --- a/arch/arm/dts/imx8mm-u-boot.dtsi
> > +++ b/arch/arm/dts/imx8mm-u-boot.dtsi
> > @@ -140,7 +140,7 @@
> >                         configurations {
> >                                 default = "@config-DEFAULT-SEQ";
> >
> > -                               binman_configuration: @config-SEQ {
> > +                               @config-SEQ {
> >                                         description = "NAME";
> >                                         fdt = "fdt-SEQ";
> >                                         firmware = "uboot";
> > diff --git a/arch/arm/dts/imx8mn-u-boot.dtsi b/arch/arm/dts/imx8mn-u-boot.dtsi
> > index 5046b38e4e29..bc57566a108f 100644
> > --- a/arch/arm/dts/imx8mn-u-boot.dtsi
> > +++ b/arch/arm/dts/imx8mn-u-boot.dtsi
> > @@ -204,7 +204,7 @@
> >                         configurations {
> >                                 default = "@config-DEFAULT-SEQ";
> >
> > -                               binman_configuration: @config-SEQ {
> > +                               @config-SEQ {
> >                                         description = "NAME";
> >                                         fdt = "fdt-SEQ";
> >                                         firmware = "uboot";
> > diff --git a/arch/arm/dts/imx8mp-rsb3720-a1-u-boot.dtsi b/arch/arm/dts/imx8mp-rsb3720-a1-u-boot.dtsi
> > index f3fb44046d5c..c4ea536b29bb 100644
> > --- a/arch/arm/dts/imx8mp-rsb3720-a1-u-boot.dtsi
> > +++ b/arch/arm/dts/imx8mp-rsb3720-a1-u-boot.dtsi
> > @@ -162,6 +162,8 @@
> >         };
> >  };
> >
> > +/* This cannot work since it refers to a template node
> >  &binman_configuration {
> >         loadables = "atf", "fip";
> >  };
> > +*/
> > diff --git a/arch/arm/dts/imx8mp-u-boot.dtsi b/arch/arm/dts/imx8mp-u-boot.dtsi
> > index 36e7444a627b..200938a98072 100644
> > --- a/arch/arm/dts/imx8mp-u-boot.dtsi
> > +++ b/arch/arm/dts/imx8mp-u-boot.dtsi
> > @@ -146,7 +146,7 @@
> >                                         type = "flat_dt";
> >                                         compression = "none";
> >
> > -                                       uboot_fdt_blob: blob-ext {
> > +                                       blob-ext {
> >                                                 filename = "u-boot.dtb";
> >                                         };
> >                                 };
> > @@ -155,7 +155,7 @@
> >                         configurations {
> >                                 default = "@config-DEFAULT-SEQ";
> >
> > -                               binman_configuration: @config-SEQ {
> > +                               @config-SEQ {
> >                                         description = "NAME";
> >                                         fdt = "fdt-SEQ";
> >                                         firmware = "uboot";
> > diff --git a/arch/arm/dts/imx8qm-u-boot.dtsi b/arch/arm/dts/imx8qm-u-boot.dtsi
> > index a3e0af48109b..d316e869516f 100644
> > --- a/arch/arm/dts/imx8qm-u-boot.dtsi
> > +++ b/arch/arm/dts/imx8qm-u-boot.dtsi
> > @@ -112,7 +112,7 @@
> >                         configurations {
> >                                 default = "@config-DEFAULT-SEQ";
> >
> > -                               binman_configuration: @config-SEQ {
> > +                               @config-SEQ {
> >                                         description = "NAME";
> >                                         fdt = "fdt-SEQ";
> >                                         firmware = "uboot";
> > diff --git a/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi b/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
> > index 3ecb461b0110..64318d09cf0a 100644
> > --- a/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
> > +++ b/arch/arm/dts/k3-am65-iot2050-boot-image.dtsi
> > @@ -41,7 +41,7 @@
> >                                         os = "arm-trusted-firmware";
> >                                         load = <CONFIG_K3_ATF_LOAD_ADDR>;
> >                                         entry = <CONFIG_K3_ATF_LOAD_ADDR>;
> > -                                       atf: atf-bl31 {
> > +                                       atf-bl31 {
> >                                         };
> >                                 };
> >
> > @@ -53,7 +53,7 @@
> >                                         os = "tee";
> >                                         load = <0x9e800000>;
> >                                         entry = <0x9e800000>;
> > -                                       tee: tee-os {
> > +                                       tee-os {
> >                                         };
> >                                 };
> >
> > @@ -78,7 +78,7 @@
> >                                         compression = "none";
> >                                         load = <CONFIG_SPL_TEXT_BASE>;
> >                                         entry = <CONFIG_SPL_TEXT_BASE>;
> > -                                       u_boot_spl_nodtb: blob-ext {
> > +                                       blob-ext {
> >                                                 filename = "spl/u-boot-spl-nodtb.bin";
> >                                         };
> >                                 };
> > @@ -88,7 +88,7 @@
> >                                         type = "flat_dt";
> >                                         arch = "arm";
> >                                         compression = "none";
> > -                                       spl_am65x_evm_dtb: blob-ext {
> > +                                       blob-ext {
> >                                                 filename = "spl/dts/k3-am65-iot2050-spl.dtb";
> >                                         };
> >                                 };
> > --
> > 2.42.0.rc1.204.g551eb34607-goog
> >
>
> Simon,
>
> Adding Ying-Chun Liu (PaulLiu) here as he is the maintainer for both
> boards being affected here.
>
> I'm not clear if the two affected boards here do have duplicate
> phandles due to only having a single dtb in their defconfigs. However
> I could not get imx8mm-cl-iot-gate-optee_defconfig,
> imx8mp_rsb3720a1_4G_defconfig, or imx8mp_rsb3720a1_6G_defconfig which
> use those two files to build with latest master to even check:
>   BINMAN  .binman_stamp
> Image 'itb' is missing external blobs and is non-functional: blob-ext
>
> /binman/itb/fit/images/fip/blob-ext (fip.bin):
>    Missing blob
>
> Some images are invalid
> make: *** [Makefile:1115: .binman_stamp] Error 103
>
> So it seems these boards are broken anyway and need some attention so
> for both patches:
>
> Acked-by: Tim Harvey <tharvey at gateworks.com>
>

Eek, OK. Well that's good to know. Thank you for digging into it.

> Is there a test that binman can make to catch any future occurrences
> of phandles being added to templates?

Yes in the FDT library's test_copy_node()  and binman's testTemplatePhandleDup()

Regards,
Simon


More information about the U-Boot mailing list