[PATCH v2] spl: fit: List DTOs applied by SPL in U-Boot control DT
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(-)
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); 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. + 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
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 fromfitImage was applied on top of U-Boot control DT by the SPL fitImageloader. The single integer cell indicates in which order was the DTOapplied 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, theremay be gaps. This property can be used to determine which DTOs wereapplied by the SPL from running U-Boot by inspecting the U-Bootcontrol 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
On 6/28/24 9:32 AM, Simon Glass wrote:
Hi Marek,
Hi,
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?
It seems there is nothing which would do even basic tests for SPL fitImage DT handling in that test? Or am I reading the current test wrong ?
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?
fdt_increase_size() would fail if there isn't enough space, so that should cover the check.
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 fromfitImage was applied on top of U-Boot control DT by the SPL fitImageloader. The single integer cell indicates in which order was the DTOapplied 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, theremay be gaps. This property can be used to determine which DTOs wereapplied by the SPL from running U-Boot by inspecting the U-Bootcontrol 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()
It is easier to test for a presence of property from U-Boot shell, also the property is integer where the integer indicates the index of the DTO when it was applied.
Hi Marek,
On Sun, 30 Jun 2024 at 06:24, Marek Vasut marex@denx.de wrote:
On 6/28/24 9:32 AM, Simon Glass wrote:
Hi Marek,
Hi,
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?
It seems there is nothing which would do even basic tests for SPL fitImage DT handling in that test? Or am I reading the current test wrong ?
That test handles loading of a FIT, but doesn't check what happens to the DT. You could add more asserts to spl_load_test(), or create a new test. You can run it in the sandbox_spl build with:
sandbox_spl/spl/u-boot-spl -u -c "exit"
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?
fdt_increase_size() would fail if there isn't enough space, so that should cover the check.
Yes it does, but I meant the memory about the DT. Do we know how much space space there is to increase the DT into?
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 fromfitImage was applied on top of U-Boot control DT by the SPL fitImageloader. The single integer cell indicates in which order was the DTOapplied 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, theremay be gaps. This property can be used to determine which DTOs wereapplied by the SPL from running U-Boot by inspecting the U-Bootcontrol 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()
It is easier to test for a presence of property from U-Boot shell, also the property is integer where the integer indicates the index of the DTO when it was applied.
Right, but in my suggestion the ordering is obvious, from the stringlist.
Regards, Simon
participants (2)
-
Marek Vasut -
Simon Glass