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 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()

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.

Reply via email to