Re: [PATCH v2] spl: fit: List DTOs applied by SPL in U-Boot control DT

Hi Marek,

On Fri, 28 Jun 2024 at 03:48, Marek Vasut marex@denx.de wrote:

Insert /u-boot,spl-applied-dto-<dto-name> = <index> property into the U-Boot control DT during SPL DTO application process. This can be used by user to inspect which DTOs got applied by the SPL and in which order from running U-Boot.

Example:

u-boot=> fdt addr $fdtcontroladdr && fdt list /
Working FDT set to aee9aeb0
/ {
        u-boot,spl-applied-dto-fdt-dto-imx8mp-dhcom-pdk3-overlay-rev100 = <0x00000005>;
        u-boot,spl-applied-dto-fdt-dto-imx8mp-dhcom-som-overlay-rev100 = <0x00000004>;
        u-boot,spl-applied-dto-fdt-dto-imx8mp-dhcom-pdk-overlay-eth2xfast = <0x00000003>;
        u-boot,spl-applied-dto-fdt-dto-imx8mp-dhcom-som-overlay-eth2xfast = <0x00000002>;
...

Signed-off-by: Marek Vasut marex@denx.de

Cc: Manoj Sai abbaraju.manojsai@amarulasolutions.com Cc: Sean Anderson seanga2@gmail.com Cc: Simon Glass sjg@chromium.org Cc: Suniel Mahesh sunil@amarulasolutions.com Cc: Tom Rini trini@konsulko.com Cc: u-boot@dh-electronics.com Cc: u-boot@lists.denx.de

---

V2: - Expand the DT by one more byte to cater for trailing NUL byte - Update comment related to the expansion of DT - Flip the !ret conditional, which was incorrect and missed - Expand prefix to u-boot,spl-applied-dto- - Document the binding

---

common/spl/spl_fit.c | 29 +++++++++++++++++++++++++++-- doc/device-tree-bindings/config.txt | 11 +++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-)

Once this is figured out, can you extend test/image/spl_load_os.c to test this code?

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c index 988125be008..0a6b5ea8212 100644 --- a/common/spl/spl_fit.c +++ b/common/spl/spl_fit.c @@ -363,6 +363,7 @@ static int spl_fit_append_fdt(struct spl_image_info *spl_image, { struct spl_image_info image_info; int node, ret = 0, index = 0;

•   char dtoname[256];
  
    /*
     * Use the address following the image as target address for the
  

@@ -448,9 +449,13 @@ static int spl_fit_append_fdt(struct spl_image_info *spl_image, if (ret < 0) break;

•                   /* Make room in FDT for changes from the overlay */
  

•                   /*
  

•                    * Make room in FDT for changes from the overlay,
  

•                    * the overlay name and the trailing NUL byte in
  

•                    * that name.
  

•                    */
                    ret = fdt_increase_size(spl_image->fdt_addr,
  

•                                           image_info.size);
  

•                                           image_info.size + strlen(str) + 1);
  

Oh and I missed the room for the property, sorry. It needs something like this:

ALIGN(strlen(str) + 1, 4) + 12 + 4

the first bit is the string-table size increase

12 is sizeof(struct fdt_property) 4 is the u32 size

Alos, is there any way to check that there is actually enough space to increase the size?

                    if (ret < 0)
                            break;

@@ -464,6 +469,26 @@ static int spl_fit_append_fdt(struct spl_image_info *spl_image,

                    debug("%s: DT overlay %s applied\n", __func__,
                          fit_get_name(ctx->fit, node, NULL));

•                   /*
  

•                    * Insert /u-boot,<dto-name> = <index> property into
  

•                    * the U-Boot control DT. This can be used by user
  

•                    * to inspect which DTOs got applied by the SPL from
  

•                    * a running U-Boot.
  

•                    */
  

•                   snprintf(dtoname, sizeof(dtoname), "u-boot,spl-applied-dto-%s", str);
  

•                   ret = fdt_setprop_u32(spl_image->fdt_addr, 0, dtoname,
  

•                                         index);
  

•                   if (ret) {
  

•                           /*
  

•                            * The DTO itself was applied, do not treat the
  

•                            * insertion of /u-boot,<dto-name> as an error
  

•                            * so the system can possibly boot somehow.
  

•                            */
  

•                           debug("%s: DT overlay %s name not inserted into / node (%d)\n",
  

•                                 __func__,
  

•                                 fit_get_name(ctx->fit, node, NULL), ret);
  

•                   }
            }
            free(tmpbuffer);
            if (ret)
  

diff --git a/doc/device-tree-bindings/config.txt b/doc/device-tree-bindings/config.txt index f50c68bbdc3..7aa6d9a72c6 100644 --- a/doc/device-tree-bindings/config.txt +++ b/doc/device-tree-bindings/config.txt @@ -95,6 +95,17 @@ rootdisk-offset (int) silent-console (int) If present and non-zero, the console is silenced by default on boot.

+u-boot,spl-applied-dto-* (int)

•   Emitted by SPL into U-Boot control DT root node in case a DTO from
  

•   fitImage was applied on top of U-Boot control DT by the SPL fitImage
  

•   loader. The single integer cell indicates in which order was the DTO
  

•   applied by the SPL and matches the index of the DTO in the fitImage.
  

•   DTOs in fitImage may be skipped using board_spl_fit_append_fdt_skip(),
  

•   therefore the integers might not be monotonically incrementing, there
  

•   may be gaps. This property can be used to determine which DTOs were
  

•   applied by the SPL from running U-Boot by inspecting the U-Boot
  

•   control DT.
  

Should we send a binding with this? I wonder if it would be better to make the filename a property value rather than including it in the property name/string table? That way you would not need the integers...the ordering would be enough.

E.g.

u-boot,spl-applied-dtos = "fdt-dto-imx8mp-dhcom-som-overlay-eth2xfast", "fdt-dto-imx8mp-dhcom-pdk-overlay-eth2xfast";

But that might be more annoying to construct as you would probably need fdt_setprop_placeholder()

u-boot,spl-payload-offset (int) If present (and SPL is controlled by the device-tree), this allows to override the CONFIG_SYS_SPI_U_BOOT_OFFS setting using a value -- 2.43.0

Regards, Simon