[PATCH v2 12/35] rockchip: dts: Create a template for the FIT

[Simon Glass]

Hi Jonas,

On Mon, 10 Feb 2025 at 14:52, Jonas Karlman <jonas at kwiboo.se> wrote:
>
> Hi Simon,
>
> On 2025-02-09 22:14, Simon Glass wrote:
> > Move the FIT description into a template so that it can (later) be used
> > in multiple places in the image.
> >
> > Signed-off-by: Simon Glass <sjg at chromium.org>
> > ---
> >
> > (no changes since v1)
> >
> >  arch/arm/dts/rockchip-u-boot.dtsi | 57 ++++++++++++++++++-------------
> >  1 file changed, 34 insertions(+), 23 deletions(-)
> >
> > diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
> > index 64a6b57d5fd..ca666ba0cd4 100644
> > --- a/arch/arm/dts/rockchip-u-boot.dtsi
> > +++ b/arch/arm/dts/rockchip-u-boot.dtsi
> > @@ -19,6 +19,10 @@
> >  #define COMP "none"
> >  #endif
> >
> > +#if defined(CONFIG_SPL_FIT) && (defined(CONFIG_ARM64) || defined(CONFIG_SPL_OPTEE_IMAGE))
> > +#define HAS_FIT
> > +#endif
> > +
> >  / {
> >       binman: binman {
> >               multiple-images;
> > @@ -27,28 +31,9 @@
> >
> >  #ifdef CONFIG_SPL
> >  &binman {
> > -     simple-bin {
> > -             filename = "u-boot-rockchip.bin";
> > -             pad-byte = <0xff>;
> > -
> > -             mkimage {
> > -                     filename = "idbloader.img";
> > -                     args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
> > -                     multiple-data-files;
> > -
> > -#ifdef CONFIG_ROCKCHIP_EXTERNAL_TPL
> > -                     rockchip-tpl {
> > -                     };
> > -#elif defined(CONFIG_TPL)
> > -                     u-boot-tpl {
> > -                     };
> > -#endif
> > -                     u-boot-spl {
> > -                     };
> > -             };
> > -
> > -#if defined(CONFIG_SPL_FIT) && (defined(CONFIG_ARM64) || defined(CONFIG_SPL_OPTEE_IMAGE))
> > -             fit: fit {
> > +#ifdef HAS_FIT
> > +     common_part: template-1 {
> > +                     type = "fit";
> >  #ifdef CONFIG_ARM64
> >                       description = "FIT image for U-Boot with bl31 (TF-A)";
> >  #else
> > @@ -59,7 +44,6 @@
> >                       filename = "u-boot.itb";
>
> Please move the filename prop to the fit node in this patch.
>
> There are lots of guides and Linux image building scripts that depend on
> the u-boot.itb file, so we should continue to keep this file to be
> backward compatible for a few more releases.
>
> >                       fit,external-offset = <CONFIG_FIT_EXTERNAL_OFFSET>;
> >                       fit,align = <512>;
> > -                     offset = <CONFIG_SPL_PAD_TO>;
> >                       images {
> >                               u-boot {
> >                                       description = "U-Boot";
> > @@ -164,6 +148,33 @@
> >                                       fit,loadables;
> >                               };
> >                       };
> > +     };
> > +#endif /* HAS_FIT */
> > +
> > +     simple-bin {
> > +             filename = "u-boot-rockchip.bin";
> > +             pad-byte = <0xff>;
> > +
> > +             mkimage {
> > +                     filename = "idbloader.img";
> > +                     args = "-n", CONFIG_SYS_SOC, "-T", "rksd";
> > +                     multiple-data-files;
> > +
> > +#ifdef CONFIG_ROCKCHIP_EXTERNAL_TPL
> > +                     rockchip-tpl {
> > +                     };
> > +#elif defined(CONFIG_TPL)
> > +                     u-boot-tpl {
> > +                     };
> > +#endif
> > +                     u-boot-spl {
> > +                     };
> > +             };
> > +
> > +#ifdef HAS_FIT
> > +             fit {
> > +                     offset = <CONFIG_SPL_PAD_TO>;
> > +                     insert-template = <&common_part>;
> >               };
> >  #else
> >               u-boot-img {
>
> There is one more fit node in simple-bin-spi that should change to use
> #ifdef HAS_FIT. Also for consistency with fit/u-boot-img node in
> simple-bin-spi I recommend we move the #else/#endif to not include the
> offset and closing bracket.
>
> I suggest something like the following fixup to be included in this patch:
>
> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
> index c3013e75ed10..460bf15e0032 100644
> --- a/arch/arm/dts/rockchip-u-boot.dtsi
> +++ b/arch/arm/dts/rockchip-u-boot.dtsi
> @@ -41,7 +41,6 @@
>  #endif
>                         #address-cells = <1>;
>                         fit,fdt-list = "of-list";
> -                       filename = "u-boot.itb";
>                         fit,external-offset = <CONFIG_FIT_EXTERNAL_OFFSET>;
>                         fit,align = <512>;
>                         images {
> @@ -173,14 +172,13 @@
>
>  #ifdef HAS_FIT
>                 fit {
> -                       offset = <CONFIG_SPL_PAD_TO>;
> +                       filename = "u-boot.itb";
>                         insert-template = <&common_part>;
> -               };
>  #else
>                 u-boot-img {
> +#endif
>                         offset = <CONFIG_SPL_PAD_TO>;
>                 };
> -#endif
>
>                 fdtmap {
>                 };
> @@ -207,7 +205,7 @@
>                         };
>                 };
>
> -#if defined(CONFIG_ARM64) || defined(CONFIG_SPL_OPTEE_IMAGE)
> +#ifdef HAS_FIT
>                 fit {
>                         type = "blob";
>                         filename = "u-boot.itb";
>
>
> With something like that applied this is:

Thanks for the patch, that helped me figure out what you wanted with
the braces. I have tended to avoid #ifdef around open braces, but I
agree it seems easier this way in this case.

>
> Reviewed-by: Jonas Karlman <jonas at kwiboo.se>

Regards,
Simon