Hi Marek,
On Fri, 28 Jun 2024 at 03:48, Marek Vasut <ma...@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 <ma...@denx.de>
> ---
> Cc: Manoj Sai <abbaraju.manoj...@amarulasolutions.com>
> Cc: Sean Anderson <sean...@gmail.com>
> Cc: Simon Glass <s...@chromium.org>
> Cc: Suniel Mahesh <su...@amarulasolutions.com>
> Cc: Tom Rini <tr...@konsulko.com>
> Cc: u-b...@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
