Re: [PATCH v2] arm: mvebu: Espressobin: Set environment variable fdtfile

[Andre Heider]

On 02/10/2020 12:49, Pali Rohár wrote:

On Saturday 26 September 2020 11:09:59 Andre Heider wrote:

On 25/09/2020 09:46, Pali Rohár wrote:

On Friday 11 September 2020 06:35:10 Andre Heider wrote:

...

+#ifdef CONFIG_BOARD_LATE_INIT
+int board_late_init(void)
+{
+   bool ddr4, emmc;
+
+   if (env_get("fdtfile"))
+   return 0;
+
+   if (!of_machine_is_compatible("globalscale,espressobin"))
+   return 0;
+
+   /* If the memory controller has been configured for DDR4, we're running 
on v7 */
+   ddr4 = ((readl(A3700_CH0_MC_CTRL2_REG) >> 
A3700_MC_CTRL2_SDRAM_TYPE_OFFS)
+   & A3700_MC_CTRL2_SDRAM_TYPE_MASK) == 
A3700_MC_CTRL2_SDRAM_TYPE_DDR4;
+
+   emmc = of_machine_is_compatible("globalscale,espressobin-emmc");


Maybe stupid question, but are not we able to detect if eMMC is
connected or not at runtime? That could simplify setup and avoid usage
of additional special DTS file for eMMC in U-Boot.


At some point I wondered about the same.

IIRC armbian just enables it and uses one u-boot binary everywhere. A
non-existing chip won't get detected, so that shouldn't be a problem.

But I think it has more to do with enabling/powering up hardware blocks,
which is not wanted or may even problematic.


In U-Boot such functionality may be implemented in board_fix_fdt()
function which allows U-Boot to modify its Device tree prior using it.

You have already wrote code which is doing V5 vs V7 detection, so
detecting eMMC is the last thing which is missing to have just one
U-Boot DTS file and therefore only one U-Boot binary for Espressobin.


That implies setting status=okay for the emmc node, which then powers up 
that block. I don't know if that might be problematic. Can we just do that?


The emmc detection would also add some delay to the boot process.

At least it should be powered down if no emmc chip has been detected, 
but I didn't check if that happens right now.


Regards,
Andre

Re: [PATCH v2] arm: mvebu: Espressobin: Set environment variable fdtfile

[Pali Rohár]

On Saturday 03 October 2020 09:17:35 Andre Heider wrote:
> On 02/10/2020 12:49, Pali Rohár wrote:
> > On Saturday 26 September 2020 11:09:59 Andre Heider wrote:
> > > On 25/09/2020 09:46, Pali Rohár wrote:
> > > > On Friday 11 September 2020 06:35:10 Andre Heider wrote:
> > > ...
> > > > > +#ifdef CONFIG_BOARD_LATE_INIT
> > > > > +int board_late_init(void)
> > > > > +{
> > > > > + bool ddr4, emmc;
> > > > > +
> > > > > + if (env_get("fdtfile"))
> > > > > + return 0;
> > > > > +
> > > > > + if (!of_machine_is_compatible("globalscale,espressobin"))
> > > > > + return 0;
> > > > > +
> > > > > + /* If the memory controller has been configured for DDR4, we're 
> > > > > running on v7 */
> > > > > + ddr4 = ((readl(A3700_CH0_MC_CTRL2_REG) >> 
> > > > > A3700_MC_CTRL2_SDRAM_TYPE_OFFS)
> > > > > + & A3700_MC_CTRL2_SDRAM_TYPE_MASK) == 
> > > > > A3700_MC_CTRL2_SDRAM_TYPE_DDR4;
> > > > > +
> > > > > + emmc = of_machine_is_compatible("globalscale,espressobin-emmc");
> > > > 
> > > > Maybe stupid question, but are not we able to detect if eMMC is
> > > > connected or not at runtime? That could simplify setup and avoid usage
> > > > of additional special DTS file for eMMC in U-Boot.
> > > 
> > > At some point I wondered about the same.
> > > 
> > > IIRC armbian just enables it and uses one u-boot binary everywhere. A
> > > non-existing chip won't get detected, so that shouldn't be a problem.
> > > 
> > > But I think it has more to do with enabling/powering up hardware blocks,
> > > which is not wanted or may even problematic.
> > 
> > In U-Boot such functionality may be implemented in board_fix_fdt()
> > function which allows U-Boot to modify its Device tree prior using it.
> > 
> > You have already wrote code which is doing V5 vs V7 detection, so
> > detecting eMMC is the last thing which is missing to have just one
> > U-Boot DTS file and therefore only one U-Boot binary for Espressobin.
> 
> That implies setting status=okay for the emmc node, which then powers up
> that block. I don't know if that might be problematic. Can we just do that?

No. I mean to detect presence of eMMC in board_fix_fdt() and then set
tatus=okay only if check passed.

> The emmc detection would also add some delay to the boot process.
> 
> At least it should be powered down if no emmc chip has been detected, but I
> didn't check if that happens right now.
> 
> Regards,
> Andre

Fit images and EFI_LOAD_FILE2_PROTOCOL

[Heinrich Schuchardt]

Hello Ilias, hello Christian,

with commit ec80b4735a59 ("efi_loader: Implement FileLoad2 for initramfs
loading") Ilias provided the possibility to specify a device path
(CONFIG_EFI_INITRD_FILESPEC) from which an initial RAM disk can be
served via the EFI_FILE_LOAD2_PROTOCOL.

Ard extended the Linux EFI stub to allow loading the initial RAM disk
via the EFI_FILE_LOAD2_PROTOCOL with the utmost priority.

With commit ecc7fdaa9ef1 ("bootm: Add a bootm command for type
IH_OS_EFI") Cristian enabled signed FIT images that contain a device
tree and a UEFI binary (enabled by CONFIG_BOOTM_EFI=y).

In the DTE calls we have discussed that it is unfortunate that we do not
have a method to validate initial RAM images in the UEFI context.

To me it would look like a good path forward to combine the two ideas:

* Let the signed FIT image (of type IH_OS_EFI) contain a RAM disk
* Pass location and size to the UEFI subsystem and serve them via
  the EFI_FILE_LOAD2_PROTOCOL.

We could also extend the bootefi command to be callable as

   bootefi $kernel_addr_r $ramdisk_addr_r:$filesize $fdt_addr_r

like the booti command to serve an initial RAM disk.

What are your thoughts?

Best regards

Heinrich

Re: Fit images and EFI_LOAD_FILE2_PROTOCOL

[François Ozog]

Le sam. 3 oct. 2020 à 10:51, Heinrich Schuchardt  a
écrit :

> Hello Ilias, hello Christian,
>
>
>
> with commit ec80b4735a59 ("efi_loader: Implement FileLoad2 for initramfs
>
> loading") Ilias provided the possibility to specify a device path
>
> (CONFIG_EFI_INITRD_FILESPEC) from which an initial RAM disk can be
>
> served via the EFI_FILE_LOAD2_PROTOCOL.
>
>
>
> Ard extended the Linux EFI stub to allow loading the initial RAM disk
>
> via the EFI_FILE_LOAD2_PROTOCOL with the utmost priority.
>
>
>
> With commit ecc7fdaa9ef1 ("bootm: Add a bootm command for type
>
> IH_OS_EFI") Cristian enabled signed FIT images that contain a device
>
> tree and a UEFI binary (enabled by CONFIG_BOOTM_EFI=y).
>
>
>
> In the DTE calls we have discussed that it is unfortunate that we do not
>
> have a method to validate initial RAM images in the UEFI context.
>
>
>
> To me it would look like a good path forward to combine the two ideas:
>
>
>
> * Let the signed FIT image (of type IH_OS_EFI) contain a RAM disk
>
> * Pass location and size to the UEFI subsystem and serve them via
>
>   the EFI_FILE_LOAD2_PROTOCOL.
>
>
>
> We could also extend the bootefi command to be callable as
>
>
>
>bootefi $kernel_addr_r $ramdisk_addr_r:$filesize $fdt_addr_r
>
>
>
> like the booti command to serve an initial RAM disk.
>
>
>
> What are your thoughts?

that looks super interesting.
I propose something (in the latest desk preparing oct 14th) similar except
the an efi application boots the FIT.
I view UEFI as booting a PE coff and pass a set of config tables. Today we
have DTB, we could just add Initrd (you command line). Bootefi would be
responsible to valide the containing FIT before pushing initrd (and
dTB?)into the table. It would be the responsibility of the efi stub to get
the initrd from the config table (GUID to be defined).


>
>
>
> Best regards
>
>
>
> Heinrich
>
> --
François-Frédéric Ozog | *Director Linaro Edge & Fog Computing Group*
T: +33.67221.6485
francois.o...@linaro.org | Skype: ffozog

Re: Fit images and EFI_LOAD_FILE2_PROTOCOL

[François Ozog]

Le sam. 3 oct. 2020 à 13:14, François Ozog  a
écrit :

>
>
> Le sam. 3 oct. 2020 à 10:51, Heinrich Schuchardt  a
> écrit :
>
>> Hello Ilias, hello Christian,
>>
>>
>>
>> with commit ec80b4735a59 ("efi_loader: Implement FileLoad2 for initramfs
>>
>> loading") Ilias provided the possibility to specify a device path
>>
>> (CONFIG_EFI_INITRD_FILESPEC) from which an initial RAM disk can be
>>
>> served via the EFI_FILE_LOAD2_PROTOCOL.
>>
>>
>>
>> Ard extended the Linux EFI stub to allow loading the initial RAM disk
>>
>> via the EFI_FILE_LOAD2_PROTOCOL with the utmost priority.
>>
>>
>>
>> With commit ecc7fdaa9ef1 ("bootm: Add a bootm command for type
>>
>> IH_OS_EFI") Cristian enabled signed FIT images that contain a device
>>
>> tree and a UEFI binary (enabled by CONFIG_BOOTM_EFI=y).
>>
>>
>>
>> In the DTE calls we have discussed that it is unfortunate that we do not
>>
>> have a method to validate initial RAM images in the UEFI context.
>>
>>
>>
>> To me it would look like a good path forward to combine the two ideas:
>>
>>
>>
>> * Let the signed FIT image (of type IH_OS_EFI) contain a RAM disk
>>
>> * Pass location and size to the UEFI subsystem and serve them via
>>
>>   the EFI_FILE_LOAD2_PROTOCOL.
>>
>>
>>
>> We could also extend the bootefi command to be callable as
>>
>>
>>
>>bootefi $kernel_addr_r $ramdisk_addr_r:$filesize $fdt_addr_r
>>
>>
>>
>> like the booti command to serve an initial RAM disk.
>>
>>
>>
>> What are your thoughts?
>
> that looks super interesting.
> I propose something (in the latest desk preparing oct 14th) similar except
> the an efi application boots the FIT.
> I view UEFI as booting a PE coff and pass a set of config tables. Today we
> have DTB, we could just add Initrd (you command line). Bootefi would be
> responsible to valide the containing FIT before pushing initrd (and
> dTB?)into the table. It would be the responsibility of the efi stub to get
> the initrd from the config table (GUID to be defined).
>
the memory attributes of the initrd config table should be such that it can
be recovered for normal use. That may be tricky though.

>
>
>>
>>
>>
>> Best regards
>>
>>
>>
>> Heinrich
>>
>> --
> François-Frédéric Ozog | *Director Linaro Edge & Fog Computing Group*
> T: +33.67221.6485
> francois.o...@linaro.org | Skype: ffozog
>
>
>
> --
François-Frédéric Ozog | *Director Linaro Edge & Fog Computing Group*
T: +33.67221.6485
francois.o...@linaro.org | Skype: ffozog

[PATCH 1/4] efi_loader: description EFI_LOAD_FILE2_PROTOCOL

[Heinrich Schuchardt]

U-Boot offers a EFI_LOAD_FILE2_PROTOCOL which the Linux EFI stub can use to
load an initial RAM disk. Update the function comments of the
implementation.

Signed-off-by: Heinrich Schuchardt 
---
 lib/efi_loader/efi_load_initrd.c | 42 +---
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
index 574a83d7e3..ff69e6eb79 100644
--- a/lib/efi_loader/efi_load_initrd.c
+++ b/lib/efi_loader/efi_load_initrd.c
@@ -47,9 +47,9 @@ static const struct efi_initrd_dp dp = {
 /**
  * get_file_size() - retrieve the size of initramfs, set efi status on error
  *
- * @dev:   device to read from. i.e "mmc"
- * @part:  device partition. i.e "0:1"
- * @file:  name fo file
+ * @dev:   device to read from, e.g. "mmc"
+ * @part:  device partition, e.g. "0:1"
+ * @file:  name of file
  * @status:EFI exit code in case of failure
  *
  * Return: size of file
@@ -78,15 +78,16 @@ out:
 }

 /**
- * load_file2() - get information about random number generation
+ * efi_load_file2initrd() - load initial RAM disk
+ *
+ * This function implements the LoadFile service of the EFI_LOAD_FILE2_PROTOCOL
+ * in order to load an initial RAM disk requested by the Linux kernel stub.
  *
- * This function implement the LoadFile2() service in order to load an initram
- * disk requested by the Linux kernel stub.
  * See the UEFI spec for details.
  *
- * @this:  loadfile2 protocol instance
- * @file_path: relative path of the file. "" in this case
- * @boot_policy:   must be false for Loadfile2
+ * @this:  EFI_LOAD_FILE2_PROTOCOL instance
+ * @file_path: media device path of the file, "" in this case
+ * @boot_policy:   must be false
  * @buffer_size:   size of allocated buffer
  * @buffer:buffer to load the file
  *
@@ -128,7 +129,13 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this,
goto out;
}

-   /* expect something like 'mmc 0:1 initrd.cpio.gz' */
+   /*
+* expect a string with three space separated parts:
+*
+* * a block device type, e.g. "mmc"
+* * a device and partition identifier, e.g. "0:1"
+* * a file path on the block device, e.g. "/boot/initrd.cpio.gz"
+*/
dev = strsep(&s, " ");
if (!dev)
goto out;
@@ -168,23 +175,20 @@ out:
 }

 /**
- * efi_initrd_register() - Register a handle and loadfile2 protocol
+ * efi_initrd_register() - create handle for loading initial RAM disk
  *
- * This function creates a new handle and installs a linux specific GUID
- * to handle initram disk loading during boot.
- * See the UEFI spec for details.
+ * This function creates a new handle and installs a Linux specific vendor
+ * device path and an EFI_LOAD_FILE_2_PROTOCOL. Linux uses the device path
+ * to identify the handle and then calls the LoadFile service of the
+ * EFI_LOAD_FILE_2_PROTOCOL to read the initial RAM disk.
  *
- * Return: status code
+ * Return: status code
  */
 efi_status_t efi_initrd_register(void)
 {
efi_handle_t efi_initrd_handle = NULL;
efi_status_t ret;

-   /*
-* Set up the handle with the EFI_LOAD_FILE2_PROTOCOL which Linux may
-* use to load the initial ramdisk.
-*/
ret = EFI_CALL(efi_install_multiple_protocol_interfaces
   (&efi_initrd_handle,
/* initramfs */
--
2.28.0

[PATCH 0/4] efi_loader: fix EFI_LOAD_FILE2_PROTOCOL

[Heinrich Schuchardt]

The EFI_LOAD_FILE2_PROTOCOL is used to provide an initial RAM disk to the
Linux EFI stub.

The following problems in the implementation are addressed:

* illegal free()
* insufficient function descriptions
* CRC32 in unit test printed as decimal instead of hexadecimal

Heinrich Schuchardt (4):
  efi_loader: description EFI_LOAD_FILE2_PROTOCOL
  efi_loader: illegal free in EFI_LOAD_FILE2_PROTOCOL
  efi_selftest: enable printing hexadecimal numbers
  efi_selftest: print CRC32 of initrd as hexadecimal

 lib/efi_loader/efi_load_initrd.c| 59 +++--
 lib/efi_selftest/efi_selftest_console.c | 35 +++-
 lib/efi_selftest/efi_selftest_load_initrd.c |  2 +-
 3 files changed, 55 insertions(+), 41 deletions(-)

--
2.28.0

[PATCH 3/4] efi_selftest: enable printing hexadecimal numbers

[Heinrich Schuchardt]

Add code to use %x in efi_st_print().

Signed-off-by: Heinrich Schuchardt 
---
 lib/efi_selftest/efi_selftest_console.c | 35 -
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/lib/efi_selftest/efi_selftest_console.c 
b/lib/efi_selftest/efi_selftest_console.c
index 13f3ee6bc1..e3f4e39348 100644
--- a/lib/efi_selftest/efi_selftest_console.c
+++ b/lib/efi_selftest/efi_selftest_console.c
@@ -44,25 +44,28 @@ static void mac(void *pointer, u16 **buf)
 }

 /*
- * Print a pointer to an u16 string
+ * printx() - print hexadecimal number to an u16 string
  *
- * @pointer: pointer
- * @buf: pointer to buffer address
- * on return position of terminating zero word
+ * @pointer:   pointer
+ * @prec:  minimum number of digits to print
+ * @buf:   pointer to buffer address,
+ * on return position of terminating zero word
+ * @size:  size of value to be printed in bytes
  */
-static void pointer(void *pointer, u16 **buf)
+static void printx(u64 p, int prec, u16 **buf)
 {
int i;
u16 c;
-   uintptr_t p = (uintptr_t)pointer;
u16 *pos = *buf;

-   for (i = 8 * sizeof(p) - 4; i >= 0; i -= 4) {
-   c = (p >> i) & 0x0f;
-   c += '0';
-   if (c > '9')
-   c += 'a' - '9' - 1;
-   *pos++ = c;
+   for (i = 2 * sizeof(p) - 1; i >= 0; --i) {
+   c = (p >> (4 * i)) & 0x0f;
+   if (c || pos != *buf || !i || i < prec) {
+   c += '0';
+   if (c > '9')
+   c += 'a' - '9' - 1;
+   *pos++ = c;
+   }
}
*pos = 0;
*buf = pos;
@@ -212,7 +215,9 @@ void efi_st_printc(int color, const char *fmt, ...)
break;
default:
--c;
-   pointer(va_arg(args, void*), &pos);
+   printx((u64)va_arg(args, void *),
+  2 * sizeof(void *), &pos);
+   break;
}
break;
case 's':
@@ -223,6 +228,10 @@ void efi_st_printc(int color, const char *fmt, ...)
case 'u':
uint2dec(va_arg(args, u32), prec, &pos);
break;
+   case 'x':
+   printx((u64)va_arg(args, unsigned int),
+  prec, &pos);
+   break;
default:
break;
}
--
2.28.0

[PATCH 4/4] efi_selftest: print CRC32 of initrd as hexadecimal

[Heinrich Schuchardt]

Print the CRC32 loaded via the EFI_LOAD_FILE2_PROTOCOL as a hexadecimal
number.

Signed-off-by: Heinrich Schuchardt 
---
 lib/efi_selftest/efi_selftest_load_initrd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_selftest/efi_selftest_load_initrd.c 
b/lib/efi_selftest/efi_selftest_load_initrd.c
index e16163caca..fe060a6644 100644
--- a/lib/efi_selftest/efi_selftest_load_initrd.c
+++ b/lib/efi_selftest/efi_selftest_load_initrd.c
@@ -200,7 +200,7 @@ static int execute(void)
efi_st_error("Could not determine CRC32\n");
return EFI_ST_FAILURE;
}
-   efi_st_printf("CRC32 %u\n", (unsigned int)crc32);
+   efi_st_printf("CRC32 %.8x\n", (unsigned int)crc32);

status = boottime->free_pool(buf);
if (status != EFI_SUCCESS) {
--
2.28.0

[PATCH 2/4] efi_loader: illegal free in EFI_LOAD_FILE2_PROTOCOL

[Heinrich Schuchardt]

strsep() changes the address that its first argument points to.
We cannot use the changed address as argument of free().

Signed-off-by: Heinrich Schuchardt 
---
 lib/efi_loader/efi_load_initrd.c | 17 +
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/lib/efi_loader/efi_load_initrd.c b/lib/efi_loader/efi_load_initrd.c
index ff69e6eb79..d517d686c3 100644
--- a/lib/efi_loader/efi_load_initrd.c
+++ b/lib/efi_loader/efi_load_initrd.c
@@ -98,19 +98,20 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this,
  struct efi_device_path *file_path, bool boot_policy,
  efi_uintn_t *buffer_size, void *buffer)
 {
-   const char *filespec = CONFIG_EFI_INITRD_FILESPEC;
+   char *filespec;
efi_status_t status = EFI_NOT_FOUND;
loff_t file_sz = 0, read_sz = 0;
char *dev, *part, *file;
-   char *s;
+   char *pos;
int ret;

EFI_ENTRY("%p, %p, %d, %p, %p", this, file_path, boot_policy,
  buffer_size, buffer);

-   s = strdup(filespec);
-   if (!s)
+   filespec = strdup(CONFIG_EFI_INITRD_FILESPEC);
+   if (!filespec)
goto out;
+   pos = filespec;

if (!this || this != &efi_lf2_protocol ||
!buffer_size) {
@@ -136,13 +137,13 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this,
 * * a device and partition identifier, e.g. "0:1"
 * * a file path on the block device, e.g. "/boot/initrd.cpio.gz"
 */
-   dev = strsep(&s, " ");
+   dev = strsep(&pos, " ");
if (!dev)
goto out;
-   part = strsep(&s, " ");
+   part = strsep(&pos, " ");
if (!part)
goto out;
-   file = strsep(&s, " ");
+   file = strsep(&pos, " ");
if (!file)
goto out;

@@ -170,7 +171,7 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this,
}

 out:
-   free(s);
+   free(filespec);
return EFI_EXIT(status);
 }

--
2.28.0

Re: Fit images and EFI_LOAD_FILE2_PROTOCOL

[Ard Biesheuvel]

On Sat, 3 Oct 2020 at 13:16, François Ozog  wrote:

>
>
> Le sam. 3 oct. 2020 à 13:14, François Ozog  a
> écrit :
>
>>
>>
>> Le sam. 3 oct. 2020 à 10:51, Heinrich Schuchardt  a
>> écrit :
>>
>>> Hello Ilias, hello Christian,
>>>
>>>
>>>
>>> with commit ec80b4735a59 ("efi_loader: Implement FileLoad2 for initramfs
>>>
>>> loading") Ilias provided the possibility to specify a device path
>>>
>>> (CONFIG_EFI_INITRD_FILESPEC) from which an initial RAM disk can be
>>>
>>> served via the EFI_FILE_LOAD2_PROTOCOL.
>>>
>>>
>>>
>>> Ard extended the Linux EFI stub to allow loading the initial RAM disk
>>>
>>> via the EFI_FILE_LOAD2_PROTOCOL with the utmost priority.
>>>
>>>
>>>
>>> With commit ecc7fdaa9ef1 ("bootm: Add a bootm command for type
>>>
>>> IH_OS_EFI") Cristian enabled signed FIT images that contain a device
>>>
>>> tree and a UEFI binary (enabled by CONFIG_BOOTM_EFI=y).
>>>
>>>
>>>
>>> In the DTE calls we have discussed that it is unfortunate that we do not
>>>
>>> have a method to validate initial RAM images in the UEFI context.
>>>
>>>
>>>
>>> To me it would look like a good path forward to combine the two ideas:
>>>
>>>
>>>
>>> * Let the signed FIT image (of type IH_OS_EFI) contain a RAM disk
>>>
>>> * Pass location and size to the UEFI subsystem and serve them via
>>>
>>>   the EFI_FILE_LOAD2_PROTOCOL.
>>>
>>>
>>>
>>> We could also extend the bootefi command to be callable as
>>>
>>>
>>>
>>>bootefi $kernel_addr_r $ramdisk_addr_r:$filesize $fdt_addr_r
>>>
>>>
>>>
>>> like the booti command to serve an initial RAM disk.
>>>
>>>
>>>
>>> What are your thoughts?
>>
>> that looks super interesting.
>> I propose something (in the latest desk preparing oct 14th) similar
>> except the an efi application boots the FIT.
>> I view UEFI as booting a PE coff and pass a set of config tables. Today
>> we have DTB, we could just add Initrd (you command line). Bootefi would be
>> responsible to valide the containing FIT before pushing initrd (and
>> dTB?)into the table. It would be the responsibility of the efi stub to get
>> the initrd from the config table (GUID to be defined).
>>
> the memory attributes of the initrd config table should be such that it
> can be recovered for normal use. That may be tricky though.
>

The purpose of the EFI_FILE_LOAD2_PROTOCOL based initrd loading mechanism
is to allow the EFI stub (which is tightly coupled to the kernel
arch/version/etc) to allocate the memory for the initrd, and pass it into
the LoadFile2() request, using whichever policy it wants to adhere to for
alignment, offset and/or vicinity of the kernel image. It also ensures that
any measurement performed by the bootloader for attestation or
authentication can be delayed to the point where the booting kernel assumes
ownership of the initrd contents, preventing potential TOCTOU issues where
intermediate boot stages are involved (shim+grub etc)

Creating an initrd config table would mean that the bootloader decides
where to load the initrd in memory, and only passes the address and size.
This is exactly what we wanted to avoid, because now, the bootloader has to
know all these different rules that vary between kernel version,
configurations and architectures.

For uboot's implementation of FIT based EFI_FILE_LOAD2_PROTOCOL, this might
mean that the initrd is loaded into memory first, and copied to another
location (and [re-]authenticated) when LoadFile2() is invoked. I don't
think this is a problem in the general case, but we might think about ways
to avoid this if this turns out to be a problem for memory constrained
devices with huge initrds.

[PATCH 01/17] dtoc: Extract inner loop from output_node()

[Simon Glass]

This function is very long. Put the inner loop in a separate function
to enhance readability.

Signed-off-by: Simon Glass 
---

 tools/dtoc/dtb_platdata.py | 89 +-
 1 file changed, 49 insertions(+), 40 deletions(-)

diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py
index 579a6749c40..bd1daa4379a 100644
--- a/tools/dtoc/dtb_platdata.py
+++ b/tools/dtoc/dtb_platdata.py
@@ -560,6 +560,54 @@ class DtbPlatdata(object):
 Args:
 node: node to output
 """
+def _output_list(node, prop):
+"""Output the C code for a devicetree property that holds a list
+
+Args:
+node (fdt.Node): Node to output
+prop (fdt.Prop): Prop to output
+"""
+self.buf('{')
+vals = []
+# For phandles, output a reference to the platform data
+# of the target node.
+info = self.get_phandle_argc(prop, node.name)
+if info:
+# Process the list as pairs of (phandle, id)
+pos = 0
+item = 0
+for args in info.args:
+phandle_cell = prop.value[pos]
+phandle = fdt_util.fdt32_to_cpu(phandle_cell)
+target_node = self._fdt.phandle_to_node[phandle]
+name = conv_name_to_c(target_node.name)
+arg_values = []
+for i in range(args):
+
arg_values.append(str(fdt_util.fdt32_to_cpu(prop.value[pos + 1 + i])))
+pos += 1 + args
+# node member is filled with NULL as the real value
+# will be update at run-time during dm_init_and_scan()
+# by dm_populate_phandle_data()
+vals.append('\t{NULL, {%s}}' % (', '.join(arg_values)))
+var_node = '%s%s.%s[%d].node' % \
+(VAL_PREFIX, var_name, member_name, item)
+# Save the the link information to be use to define
+# dm_populate_phandle_data()
+self._links.append({'var_node': var_node, 'dev_name': 
name})
+item += 1
+for val in vals:
+self.buf('\n\t\t%s,' % val)
+else:
+for val in prop.value:
+vals.append(get_value(prop.type, val))
+
+# Put 8 values per line to avoid very long lines.
+for i in range(0, len(vals), 8):
+if i:
+self.buf(',\n\t\t')
+self.buf(', '.join(vals[i:i + 8]))
+self.buf('}')
+
 struct_name, _ = self.get_normalized_compat_name(node)
 var_name = conv_name_to_c(node.name)
 self.buf('static struct %s%s %s%s = {\n' %
@@ -573,46 +621,7 @@ class DtbPlatdata(object):
 
 # Special handling for lists
 if isinstance(prop.value, list):
-self.buf('{')
-vals = []
-# For phandles, output a reference to the platform data
-# of the target node.
-info = self.get_phandle_argc(prop, node.name)
-if info:
-# Process the list as pairs of (phandle, id)
-pos = 0
-item = 0
-for args in info.args:
-phandle_cell = prop.value[pos]
-phandle = fdt_util.fdt32_to_cpu(phandle_cell)
-target_node = self._fdt.phandle_to_node[phandle]
-name = conv_name_to_c(target_node.name)
-arg_values = []
-for i in range(args):
-
arg_values.append(str(fdt_util.fdt32_to_cpu(prop.value[pos + 1 + i])))
-pos += 1 + args
-# node member is filled with NULL as the real value
-# will be update at run-time during dm_init_and_scan()
-# by dm_populate_phandle_data()
-vals.append('\t{NULL, {%s}}' % (', '.join(arg_values)))
-var_node = '%s%s.%s[%d].node' % \
-(VAL_PREFIX, var_name, member_name, item)
-# Save the the link information to be use to define
-# dm_populate_phandle_data()
-self._links.append({'var_node': var_node, 'dev_name': 
name})
-item += 1
-for val in vals:
-self.buf('\n\t\t%s,' % val)
-else:
-for val in prop.value:
-vals.append(get_value(prop.type, val))
-
-# Put 8 values per line to avoid very long lines.
-for i in 

[PATCH 02/17] dtoc: Use a namedtuple for _links

[Simon Glass]

The use of strings to access a dict is a bit ugly. Use a namedtuple for
this instead.

Signed-off-by: Simon Glass 
---

 tools/dtoc/dtb_platdata.py | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py
index bd1daa4379a..1b52198a943 100644
--- a/tools/dtoc/dtb_platdata.py
+++ b/tools/dtoc/dtb_platdata.py
@@ -54,6 +54,13 @@ VAL_PREFIX = 'dtv_'
 # phandles is len(args). This is a list of integers.
 PhandleInfo = collections.namedtuple('PhandleInfo', ['max_args', 'args'])
 
+# Holds a single phandle link, allowing a C struct value to be assigned to 
point
+# to a device
+#
+# var_node: C variable to assign (e.g. 'dtv_mmc.clocks[0].node')
+# dev_name: Name of device to assign to (e.g. 'clock')
+PhandleLink = collections.namedtuple('PhandleLink', ['var_node', 'dev_name'])
+
 
 def conv_name_to_c(name):
 """Convert a device-tree name to a C identifier
@@ -146,7 +153,8 @@ class DtbPlatdata(object):
 key: Driver alias declared with
 U_BOOT_DRIVER_ALIAS(driver_alias, driver_name)
 value: Driver name declared with U_BOOT_DRIVER(driver_name)
-_links: List of links to be included in dm_populate_phandle_data()
+_links: List of links to be included in dm_populate_phandle_data(),
+each a PhandleLink
 _drivers_additional: List of additional drivers to use during scanning
 """
 def __init__(self, dtb_fname, include_disabled, warning_disabled,
@@ -593,7 +601,7 @@ class DtbPlatdata(object):
 (VAL_PREFIX, var_name, member_name, item)
 # Save the the link information to be use to define
 # dm_populate_phandle_data()
-self._links.append({'var_node': var_node, 'dev_name': 
name})
+self._links.append(PhandleLink(var_node, name))
 item += 1
 for val in vals:
 self.buf('\n\t\t%s,' % val)
@@ -669,9 +677,9 @@ class DtbPlatdata(object):
 # nodes using DM_GET_DEVICE
 # dtv_dmc_at_xxx.clocks[0].node = 
DM_GET_DEVICE(clock_controller_at_xxx)
 self.buf('void dm_populate_phandle_data(void) {\n')
-for l in self._links:
+for link in self._links:
 self.buf('\t%s = DM_GET_DEVICE(%s);\n' %
- (l['var_node'], l['dev_name']))
+ (link.var_node, link.dev_name))
 self.buf('}\n')
 
 self.out(''.join(self.get_buf()))
-- 
2.28.0.806.g8561365e88-goog

[PATCH 05/17] dm: Avoid using #ifdef for CONFIG_OF_LIVE

[Simon Glass]

At present this option results in a number of #ifdefs due to the presence
or absence of the global_data of_root member.

Add a few macros to global_data.h to work around this. Update the code
accordingly.

Signed-off-by: Simon Glass 
---

 common/board_r.c  | 19 +--
 drivers/core/Makefile |  2 +-
 drivers/core/root.c   | 27 +--
 include/asm-generic/global_data.h | 13 -
 include/dm/of.h   |  9 +
 test/dm/test-main.c   | 16 +---
 6 files changed, 37 insertions(+), 49 deletions(-)

diff --git a/common/board_r.c b/common/board_r.c
index 9b2fec701a5..1f37345f940 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -295,20 +295,21 @@ static int initr_noncached(void)
 }
 #endif
 
-#ifdef CONFIG_OF_LIVE
 static int initr_of_live(void)
 {
-   int ret;
+   if (CONFIG_IS_ENABLED(OF_LIVE)) {
+   int ret;
 
-   bootstage_start(BOOTSTAGE_ID_ACCUM_OF_LIVE, "of_live");
-   ret = of_live_build(gd->fdt_blob, (struct device_node **)&gd->of_root);
-   bootstage_accum(BOOTSTAGE_ID_ACCUM_OF_LIVE);
-   if (ret)
-   return ret;
+   bootstage_start(BOOTSTAGE_ID_ACCUM_OF_LIVE, "of_live");
+   ret = of_live_build(gd->fdt_blob,
+   (struct device_node **)gd_of_root_ptr());
+   bootstage_accum(BOOTSTAGE_ID_ACCUM_OF_LIVE);
+   if (ret)
+   return ret;
+   }
 
return 0;
 }
-#endif
 
 #ifdef CONFIG_DM
 static int initr_dm(void)
@@ -701,9 +702,7 @@ static init_fnc_t init_sequence_r[] = {
 #ifdef CONFIG_SYS_NONCACHED_MEMORY
initr_noncached,
 #endif
-#ifdef CONFIG_OF_LIVE
initr_of_live,
-#endif
 #ifdef CONFIG_DM
initr_dm,
 #endif
diff --git a/drivers/core/Makefile b/drivers/core/Makefile
index 10f4bece335..5edd4e41357 100644
--- a/drivers/core/Makefile
+++ b/drivers/core/Makefile
@@ -11,7 +11,7 @@ obj-$(CONFIG_SIMPLE_PM_BUS)   += simple-pm-bus.o
 obj-$(CONFIG_DM)   += dump.o
 obj-$(CONFIG_$(SPL_TPL_)REGMAP)+= regmap.o
 obj-$(CONFIG_$(SPL_TPL_)SYSCON)+= syscon-uclass.o
-obj-$(CONFIG_OF_LIVE) += of_access.o of_addr.o
+obj-$(CONFIG_$(SPL_)OF_LIVE) += of_access.o of_addr.o
 ifndef CONFIG_DM_DEV_READ_INLINE
 obj-$(CONFIG_OF_CONTROL) += read.o
 endif
diff --git a/drivers/core/root.c b/drivers/core/root.c
index 0726be6b795..de23161cff8 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -61,7 +61,7 @@ void fix_drivers(void)
for (entry = drv; entry != drv + n_ents; entry++) {
if (entry->of_match)
entry->of_match = (const struct udevice_id *)
-   ((u32)entry->of_match + gd->reloc_off);
+   ((ulong)entry->of_match + gd->reloc_off);
if (entry->bind)
entry->bind += gd->reloc_off;
if (entry->probe)
@@ -151,11 +151,9 @@ int dm_init(bool of_live)
if (ret)
return ret;
 #if CONFIG_IS_ENABLED(OF_CONTROL)
-# if CONFIG_IS_ENABLED(OF_LIVE)
-   if (of_live)
-   DM_ROOT_NON_CONST->node = np_to_ofnode(gd->of_root);
+   if (CONFIG_IS_ENABLED(OF_LIVE) && of_live)
+   DM_ROOT_NON_CONST->node = np_to_ofnode(gd_of_root());
else
-#endif
DM_ROOT_NON_CONST->node = offset_to_ofnode(0);
 #endif
ret = device_probe(DM_ROOT_NON_CONST);
@@ -196,7 +194,7 @@ int dm_scan_platdata(bool pre_reloc_only)
return ret;
 }
 
-#if CONFIG_IS_ENABLED(OF_LIVE)
+#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 static int dm_scan_fdt_live(struct udevice *parent,
const struct device_node *node_parent,
bool pre_reloc_only)
@@ -223,9 +221,7 @@ static int dm_scan_fdt_live(struct udevice *parent,
 
return ret;
 }
-#endif /* CONFIG_IS_ENABLED(OF_LIVE) */
 
-#if CONFIG_IS_ENABLED(OF_CONTROL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 /**
  * dm_scan_fdt_node() - Scan the device tree and bind drivers for a node
  *
@@ -272,24 +268,20 @@ int dm_scan_fdt_dev(struct udevice *dev)
if (!dev_of_valid(dev))
return 0;
 
-#if CONFIG_IS_ENABLED(OF_LIVE)
if (of_live_active())
return dm_scan_fdt_live(dev, dev_np(dev),
gd->flags & GD_FLG_RELOC ? false : true);
-   else
-#endif
+
return dm_scan_fdt_node(dev, gd->fdt_blob, dev_of_offset(dev),
gd->flags & GD_FLG_RELOC ? false : true);
 }
 
 int dm_scan_fdt(const void *blob, bool pre_reloc_only)
 {
-#if CONFIG_IS_ENABLED(OF_LIVE)
if (of_live_active())
-   return dm_scan_fdt_live(gd->dm_root, gd->of_root,
+   return dm_scan_fdt_live(gd->dm_root, gd_of_root(),
pre_reloc_only);
-   else
-#endif
+
 

[PATCH 04/17] dm: core: Avoid void * in the of-platdata structs

[Simon Glass]

These pointers point to drivers. Update the definition to make this clear.

Signed-off-by: Simon Glass 
---

 include/dt-structs.h | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/include/dt-structs.h b/include/dt-structs.h
index 924d51fc522..eed8273d18e 100644
--- a/include/dt-structs.h
+++ b/include/dt-structs.h
@@ -8,18 +8,20 @@
 
 /* These structures may only be used in SPL */
 #if CONFIG_IS_ENABLED(OF_PLATDATA)
+struct driver_info;
+
 struct phandle_0_arg {
-   const void *node;
+   const struct driver_info *node;
int arg[0];
 };
 
 struct phandle_1_arg {
-   const void *node;
+   const struct driver_info *node;
int arg[1];
 };
 
 struct phandle_2_arg {
-   const void *node;
+   const struct driver_info *node;
int arg[2];
 };
 #include 
-- 
2.28.0.806.g8561365e88-goog

[PATCH 03/17] dm: core: Expand the comment for DM_GET_DEVICE()

[Simon Glass]

The current documentation for this is not particularly enlightening. Add
a little more detail.

Signed-off-by: Simon Glass 
---

 include/dm/platdata.h | 17 -
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/include/dm/platdata.h b/include/dm/platdata.h
index cab93b071ba..25479b03d22 100644
--- a/include/dm/platdata.h
+++ b/include/dm/platdata.h
@@ -45,7 +45,22 @@ struct driver_info {
 #define U_BOOT_DEVICES(__name) \
ll_entry_declare_list(struct driver_info, __name, driver_info)
 
-/* Get a pointer to a given driver */
+/**
+ * Get a pointer to a given device info given its name
+ *
+ * With the declaration U_BOOT_DEVICE(name), DM_GET_DEVICE(name) will return a
+ * pointer to the struct driver_info created by that declaration.
+ *
+ * if OF_PLATDATA is enabled, from this it is possible to use the @dev member 
of
+ * struct driver_info to find the device pointer itself.
+ *
+ * TODO(s...@chromium.org): U_BOOT_DEVICE() tells U-Boot to create a device, so
+ * the naming seems sensible, but DM_GET_DEVICE() is a bit of misnomer, since 
it
+ * finds the driver_info record, not the device.
+ *
+ * @__name: Driver name (C identifier, not a string. E.g. gpio7_at_ff7e)
+ * @return struct driver_info * to the driver that created the device
+ */
 #define DM_GET_DEVICE(__name)  \
ll_entry_get(struct driver_info, __name, driver_info)
 
-- 
2.28.0.806.g8561365e88-goog

[PATCH 09/17] dm: test: Disable some tests that should not run in SPL

[Simon Glass]

Tests are easier to run in U-Boot proper. Running them in SPL does not add
test coverage in most cases. Also some tests use features that are not
available in SPL.

Update the build rules to disable these tests in SPL. We still need
test-main to be able to actually run SPL tests.

Signed-off-by: Simon Glass 
---

 test/Makefile|  9 ++---
 test/dm/Makefile | 11 +++
 2 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/test/Makefile b/test/Makefile
index ed3e882f7a7..c8e554206a5 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -7,11 +7,14 @@ obj-$(CONFIG_$(SPL_)CMDLINE) += cmd/
 obj-$(CONFIG_$(SPL_)CMDLINE) += cmd_ut.o
 obj-$(CONFIG_$(SPL_)CMDLINE) += command_ut.o
 obj-$(CONFIG_$(SPL_)CMDLINE) += compression.o
-obj-$(CONFIG_UNIT_TEST) += lib/
-obj-y += log/
 obj-y += dm/
+obj-y += log/
 obj-$(CONFIG_$(SPL_)CMDLINE) += print_ut.o
 obj-$(CONFIG_$(SPL_)CMDLINE) += str_ut.o
 obj-$(CONFIG_UT_TIME) += time_ut.o
-obj-$(CONFIG_UT_UNICODE) += unicode_ut.o
 obj-y += ut.o
+
+ifeq ($(CONFIG_SPL_BUILD),)
+obj-$(CONFIG_UNIT_TEST) += lib/
+obj-$(CONFIG_$(SPL_)UT_UNICODE) += unicode_ut.o
+endif
diff --git a/test/dm/Makefile b/test/dm/Makefile
index 70ba1b66953..387a4b81410 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -2,15 +2,16 @@
 #
 # Copyright (c) 2013 Google, Inc
 
+obj-$(CONFIG_UT_DM) += test-main.o
+
+# Tests for particular subsystems - when enabling driver model for a new
+# subsystem you must add sandbox tests here.
+ifeq ($(CONFIG_SPL_BUILD),)
 obj-$(CONFIG_UT_DM) += bus.o
-obj-$(CONFIG_UT_DM) += nop.o
 obj-$(CONFIG_UT_DM) += test-driver.o
 obj-$(CONFIG_UT_DM) += test-fdt.o
-obj-$(CONFIG_UT_DM) += test-main.o
 obj-$(CONFIG_UT_DM) += test-uclass.o
 
-# Tests for particular subsystems - when enabling driver model for a new
-# subsystem you must add sandbox tests here.
 obj-$(CONFIG_UT_DM) += core.o
 ifneq ($(CONFIG_SANDBOX),)
 obj-$(CONFIG_ACPIGEN) += acpi.o
@@ -35,6 +36,7 @@ obj-$(CONFIG_LED) += led.o
 obj-$(CONFIG_DM_MAILBOX) += mailbox.o
 obj-$(CONFIG_DM_MMC) += mmc.o
 obj-y += fdtdec.o
+obj-$(CONFIG_UT_DM) += nop.o
 obj-y += ofnode.o
 obj-y += ofread.o
 obj-$(CONFIG_OSD) += osd.o
@@ -82,3 +84,4 @@ obj-$(CONFIG_SIMPLE_PM_BUS) += simple-pm-bus.o
 obj-$(CONFIG_RESET_SYSCON) += syscon-reset.o
 obj-$(CONFIG_SCMI_FIRMWARE) += scmi.o
 endif
+endif # !SPL
-- 
2.28.0.806.g8561365e88-goog

[PATCH 08/17] dm: test: Make use of CONFIG_UNIT_TEST

[Simon Glass]

At present we always include test/dm from the main Makefile. We have a
CONFIG_UNIT_TEST that should control whether the test/ directory is built,
so rely on that instead.

Signed-off-by: Simon Glass 
---

 Makefile  | 2 +-
 test/Makefile | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 2de0527e2dd..dbe7e7b75c9 100644
--- a/Makefile
+++ b/Makefile
@@ -806,7 +806,7 @@ libs-$(CONFIG_API) += api/
 ifdef CONFIG_POST
 libs-y += post/
 endif
-libs-$(CONFIG_UNIT_TEST) += test/ test/dm/
+libs-$(CONFIG_UNIT_TEST) += test/
 libs-$(CONFIG_UT_ENV) += test/env/
 libs-$(CONFIG_UT_OPTEE) += test/optee/
 libs-$(CONFIG_UT_OVERLAY) += test/overlay/
diff --git a/test/Makefile b/test/Makefile
index 73bddb4f61f..ed3e882f7a7 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -9,8 +9,9 @@ obj-$(CONFIG_$(SPL_)CMDLINE) += command_ut.o
 obj-$(CONFIG_$(SPL_)CMDLINE) += compression.o
 obj-$(CONFIG_UNIT_TEST) += lib/
 obj-y += log/
+obj-y += dm/
 obj-$(CONFIG_$(SPL_)CMDLINE) += print_ut.o
 obj-$(CONFIG_$(SPL_)CMDLINE) += str_ut.o
 obj-$(CONFIG_UT_TIME) += time_ut.o
 obj-$(CONFIG_UT_UNICODE) += unicode_ut.o
-obj-$(CONFIG_UNIT_TEST) += ut.o
+obj-y += ut.o
-- 
2.28.0.806.g8561365e88-goog

[PATCH 07/17] dm: test: Update Makefile conditions

[Simon Glass]

At present most of the tests in test/Makefile are dependent on
CONFIG_SANDBOX. But this is not ideal since they rely on commands being
available and SPL does not support commands.

Use CONFIG_COMMAND instead. This has the dual purpose of allowing these
tests to be used on other boards and allowing SPL to skip them.

Signed-off-by: Simon Glass 
---

 test/Makefile | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/test/Makefile b/test/Makefile
index 13ee661f50e..73bddb4f61f 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -2,15 +2,15 @@
 #
 # (C) Copyright 2012 The Chromium Authors
 
-obj-$(CONFIG_SANDBOX) += bloblist.o
-obj-$(CONFIG_CMDLINE) += cmd/
-obj-$(CONFIG_UNIT_TEST) += cmd_ut.o
-obj-$(CONFIG_SANDBOX) += command_ut.o
-obj-$(CONFIG_SANDBOX) += compression.o
+obj-$(CONFIG_$(SPL_)CMDLINE) += bloblist.o
+obj-$(CONFIG_$(SPL_)CMDLINE) += cmd/
+obj-$(CONFIG_$(SPL_)CMDLINE) += cmd_ut.o
+obj-$(CONFIG_$(SPL_)CMDLINE) += command_ut.o
+obj-$(CONFIG_$(SPL_)CMDLINE) += compression.o
 obj-$(CONFIG_UNIT_TEST) += lib/
 obj-y += log/
-obj-$(CONFIG_SANDBOX) += print_ut.o
-obj-$(CONFIG_SANDBOX) += str_ut.o
+obj-$(CONFIG_$(SPL_)CMDLINE) += print_ut.o
+obj-$(CONFIG_$(SPL_)CMDLINE) += str_ut.o
 obj-$(CONFIG_UT_TIME) += time_ut.o
 obj-$(CONFIG_UT_UNICODE) += unicode_ut.o
 obj-$(CONFIG_UNIT_TEST) += ut.o
-- 
2.28.0.806.g8561365e88-goog

[PATCH 06/17] dm: test: Sort the Makefile

[Simon Glass]

Move everything into alphabetical order.

Signed-off-by: Simon Glass 
---

 test/Makefile | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/test/Makefile b/test/Makefile
index 7c4039964e1..13ee661f50e 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -5,12 +5,12 @@
 obj-$(CONFIG_SANDBOX) += bloblist.o
 obj-$(CONFIG_CMDLINE) += cmd/
 obj-$(CONFIG_UNIT_TEST) += cmd_ut.o
-obj-$(CONFIG_UNIT_TEST) += ut.o
 obj-$(CONFIG_SANDBOX) += command_ut.o
 obj-$(CONFIG_SANDBOX) += compression.o
+obj-$(CONFIG_UNIT_TEST) += lib/
+obj-y += log/
 obj-$(CONFIG_SANDBOX) += print_ut.o
 obj-$(CONFIG_SANDBOX) += str_ut.o
 obj-$(CONFIG_UT_TIME) += time_ut.o
 obj-$(CONFIG_UT_UNICODE) += unicode_ut.o
-obj-y += log/
-obj-$(CONFIG_UNIT_TEST) += lib/
+obj-$(CONFIG_UNIT_TEST) += ut.o
-- 
2.28.0.806.g8561365e88-goog

[PATCH 10/17] dm: test: Build tests for SPL

[Simon Glass]

We want to run unit tests in SPL. Add a new Kconfig to control this and
enable it for sandbox_spl

Signed-off-by: Simon Glass 
---

 configs/sandbox_spl_defconfig |  2 +-
 scripts/Makefile.spl  |  1 +
 test/Kconfig  | 10 ++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
index 6d8e827aebc..49060cab7a3 100644
--- a/configs/sandbox_spl_defconfig
+++ b/configs/sandbox_spl_defconfig
@@ -22,7 +22,6 @@ CONFIG_BOOTSTAGE_STASH=y
 CONFIG_BOOTSTAGE_STASH_SIZE=0x4096
 CONFIG_CONSOLE_RECORD=y
 CONFIG_CONSOLE_RECORD_OUT_SIZE=0x1000
-CONFIG_SILENT_CONSOLE=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_HANDOFF=y
 CONFIG_SPL_BOARD_INIT=y
@@ -220,5 +219,6 @@ CONFIG_TPM=y
 CONFIG_LZ4=y
 CONFIG_ERRNO_STR=y
 CONFIG_UNIT_TEST=y
+CONFIG_SPL_UNIT_TEST=y
 CONFIG_UT_TIME=y
 CONFIG_UT_DM=y
diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index d528c994ff2..2e3a443035c 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -99,6 +99,7 @@ libs-y += dts/
 libs-y += fs/
 libs-$(CONFIG_SPL_POST_MEM_SUPPORT) += post/drivers/
 libs-$(CONFIG_SPL_NET_SUPPORT) += net/
+libs-$(CONFIG_SPL_UNIT_TEST) += test/
 
 head-y := $(addprefix $(obj)/,$(head-y))
 libs-y := $(addprefix $(obj)/,$(libs-y))
diff --git a/test/Kconfig b/test/Kconfig
index 28704a25b61..2646e7d825a 100644
--- a/test/Kconfig
+++ b/test/Kconfig
@@ -6,6 +6,16 @@ menuconfig UNIT_TEST
  This does not require sandbox to be included, but it is most
  often used there.
 
+config SPL_UNIT_TEST
+   bool "Unit tests in SPL"
+   # We need to be able to unbind devices for tests to work
+   select SPL_DM_DEVICE_REMOVE
+   help
+ Select this to enable unit tests in SPL. Most test are designed for
+ running in U-Boot proper, but some are intended for SPL, such as
+ of-platdata and SPL handover. To run these tests with the sandbox_spl
+ board, use the -u (unit test) option.
+
 config UT_LIB
bool "Unit tests for library functions"
depends on UNIT_TEST
-- 
2.28.0.806.g8561365e88-goog

[PATCH 11/17] dm: test: Update the test runner to support of-platdata

[Simon Glass]

At present DM tests assume that a devicetree is available. This is not the
case with of-platadata.

Update the code to add this condition.

Signed-off-by: Simon Glass 
---

 test/dm/test-main.c | 27 ---
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/test/dm/test-main.c b/test/dm/test-main.c
index 995988723ae..5560572c7c6 100644
--- a/test/dm/test-main.c
+++ b/test/dm/test-main.c
@@ -30,7 +30,8 @@ static int dm_test_init(struct unit_test_state *uts, bool 
of_live)
 
memset(dms, '\0', sizeof(*dms));
gd->dm_root = NULL;
-   memset(dm_testdrv_op_count, '\0', sizeof(dm_testdrv_op_count));
+   if (!CONFIG_IS_ENABLED(OF_PLATDATA))
+   memset(dm_testdrv_op_count, '\0', sizeof(dm_testdrv_op_count));
state_reset_for_test(state_get_current());
 
/* Determine whether to make the live tree available */
@@ -91,7 +92,8 @@ static int dm_do_test(struct unit_test_state *uts, struct 
unit_test *test,
ut_assertok(dm_scan_platdata(false));
if (test->flags & UT_TESTF_PROBE_TEST)
ut_assertok(do_autoprobe(uts));
-   if (test->flags & UT_TESTF_SCAN_FDT)
+   if (!CONFIG_IS_ENABLED(OF_PLATDATA) &&
+   (test->flags & UT_TESTF_SCAN_FDT))
ut_assertok(dm_extended_scan_fdt(gd->fdt_blob, false));
 
/*
@@ -136,14 +138,16 @@ static int dm_test_main(const char *test_name)
uts->priv = &_global_priv_dm_test_state;
uts->fail_count = 0;
 
-   /*
-* If we have no device tree, or it only has a root node, then these
-* tests clearly aren't going to work...
-*/
-   if (!gd->fdt_blob || fdt_next_node(gd->fdt_blob, 0, NULL) < 0) {
-   puts("Please run with test device tree:\n"
-"./u-boot -d arch/sandbox/dts/test.dtb\n");
-   ut_assert(gd->fdt_blob);
+   if (!CONFIG_IS_ENABLED(OF_PLATDATA)) {
+   /*
+* If we have no device tree, or it only has a root node, then
+* these * tests clearly aren't going to work...
+*/
+   if (!gd->fdt_blob || fdt_next_node(gd->fdt_blob, 0, NULL) < 0) {
+   puts("Please run with test device tree:\n"
+"./u-boot -d arch/sandbox/dts/test.dtb\n");
+   ut_assert(gd->fdt_blob);
+   }
}
 
if (!test_name)
@@ -192,7 +196,8 @@ static int dm_test_main(const char *test_name)
gd->dm_root = NULL;
ut_assertok(dm_init(CONFIG_IS_ENABLED(OF_LIVE)));
dm_scan_platdata(false);
-   dm_scan_fdt(gd->fdt_blob, false);
+   if (!CONFIG_IS_ENABLED(OF_PLATDATA))
+   dm_scan_fdt(gd->fdt_blob, false);
 
return uts->fail_count ? CMD_RET_FAILURE : 0;
 }
-- 
2.28.0.806.g8561365e88-goog

[PATCH 12/17] dm: test: Add a way to run SPL tests

[Simon Glass]

Add a -u flag for U-Boot SPL which requests that unit tests be run. To
make this work, export dm_test_main() and update it to skip test features
that are not used with of-platdata.

To run the tests:

   $ spl/u-boot-spl -u
   U-Boot SPL 2020.10-rc5 (Oct 01 2020 - 07:35:39 -0600)
   Running 0 driver model tests
   Failures: 0

At present there are no SPL unit tests.

Note that there is one wrinkle with these tests. SPL has limited memory
available for allocation. Also malloc_simple does not free memory
(free() is a nop) and running tests repeatedly causes driver-model to
reinit multiple times and allocate memory. Therefore it is not possible
to run more than a few tests at a time. One solution is to increase the
amount of malloc space in sandbox_spl. This is not a problem for pytest,
since it runs each test individually, so for now this is left as is.

Signed-off-by: Simon Glass 
---

 arch/sandbox/cpu/spl.c   |  8 
 arch/sandbox/cpu/start.c |  9 +
 arch/sandbox/include/asm/state.h |  1 +
 include/test/test.h  | 11 +++
 test/dm/test-main.c  |  2 +-
 5 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/arch/sandbox/cpu/spl.c b/arch/sandbox/cpu/spl.c
index 7ab8919eb90..48fd1265afe 100644
--- a/arch/sandbox/cpu/spl.c
+++ b/arch/sandbox/cpu/spl.c
@@ -12,6 +12,7 @@
 #include 
 #include 
 #include 
+#include 
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -67,6 +68,13 @@ void spl_board_init(void)
 uclass_next_device(&dev))
;
}
+
+   if (state->run_unittests) {
+   int ret;
+
+   ret = dm_test_main(NULL);
+   /* continue execution into U-Boot */
+   }
 }
 
 void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
index c6a2bbe4689..f5e104b127b 100644
--- a/arch/sandbox/cpu/start.c
+++ b/arch/sandbox/cpu/start.c
@@ -374,6 +374,15 @@ static int sandbox_cmdline_cb_show_of_platdata(struct 
sandbox_state *state,
 }
 SANDBOX_CMDLINE_OPT(show_of_platdata, 0, "Show of-platdata in SPL");
 
+static int sandbox_cmdline_cb_unittests(struct sandbox_state *state,
+   const char *arg)
+{
+   state->run_unittests = true;
+
+   return 0;
+}
+SANDBOX_CMDLINE_OPT_SHORT(unittests, 'u', 0, "Run unit tests");
+
 static void setup_ram_buf(struct sandbox_state *state)
 {
/* Zero the RAM buffer if we didn't read it, to keep valgrind happy */
diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h
index 1bfad305f1a..f828d9d2447 100644
--- a/arch/sandbox/include/asm/state.h
+++ b/arch/sandbox/include/asm/state.h
@@ -92,6 +92,7 @@ struct sandbox_state {
int default_log_level;  /* Default log level for sandbox */
bool show_of_platdata;  /* Show of-platdata in SPL */
bool ram_buf_read;  /* true if we read the RAM buffer */
+   bool run_unittests; /* Run unit tests */
 
/* Pointer to information for each SPI bus/cs */
struct sandbox_spi_info spi[CONFIG_SANDBOX_SPI_MAX_BUS]
diff --git a/include/test/test.h b/include/test/test.h
index 67c7d69d488..03e29290bf4 100644
--- a/include/test/test.h
+++ b/include/test/test.h
@@ -94,4 +94,15 @@ enum {
TEST_DEVRES_SIZE3   = 37,
 };
 
+/**
+ * dm_test_main() - Run driver model tests
+ *
+ * Run all the available driver model tests, or a selection
+ *
+ * @test_name: Name of single test to run (e.g. "dm_test_fdt_pre_reloc" or just
+ * "fdt_pre_reloc"), or NULL to run all
+ * @return 0 if all tests passed, 1 if not
+ */
+int dm_test_main(const char *test_name);
+
 #endif /* __TEST_TEST_H */
diff --git a/test/dm/test-main.c b/test/dm/test-main.c
index 5560572c7c6..9d22df8c4dc 100644
--- a/test/dm/test-main.c
+++ b/test/dm/test-main.c
@@ -127,7 +127,7 @@ static bool dm_test_run_on_flattree(struct unit_test *test)
return !strstr(fname, "video") || strstr(test->name, "video_base");
 }
 
-static int dm_test_main(const char *test_name)
+int dm_test_main(const char *test_name)
 {
struct unit_test *tests = ll_entry_start(struct unit_test, dm_test);
const int n_ents = ll_entry_count(struct unit_test, dm_test);
-- 
2.28.0.806.g8561365e88-goog

[PATCH 14/17] Makefile: Generate a symbol file for u-boot-spl

[Simon Glass]

Add a rule to generate u-boot-spl.sym so that pytest can discover the
available unit tests.

Signed-off-by: Simon Glass 
---

 scripts/Makefile.spl | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/scripts/Makefile.spl b/scripts/Makefile.spl
index 2e3a443035c..9f1f7445d71 100644
--- a/scripts/Makefile.spl
+++ b/scripts/Makefile.spl
@@ -214,7 +214,7 @@ spl/boot.bin: $(obj)/$(SPL_BIN)-align.bin FORCE
$(call if_changed,mkimage)
 endif
 
-INPUTS-y   += $(obj)/$(SPL_BIN).bin
+INPUTS-y   += $(obj)/$(SPL_BIN).bin $(obj)/$(SPL_BIN).sym
 
 ifdef CONFIG_SAMSUNG
 INPUTS-y   += $(obj)/$(BOARD)-spl.bin
@@ -408,6 +408,11 @@ MKIMAGEFLAGS_u-boot-spl-mtk.bin = -T mtk_image \
 $(obj)/u-boot-spl-mtk.bin: $(obj)/u-boot-spl.bin FORCE
$(call if_changed,mkimage)
 
+quiet_cmd_sym ?= SYM $@
+  cmd_sym ?= $(OBJDUMP) -t $< > $@
+$(obj)/$(SPL_BIN).sym: $(obj)/$(SPL_BIN) FORCE
+   $(call if_changed,sym)
+
 # Rule to link u-boot-spl
 # May be overridden by arch/$(ARCH)/config.mk
 quiet_cmd_u-boot-spl ?= LD  $@
-- 
2.28.0.806.g8561365e88-goog

[PATCH 13/17] dm: test: Add a very simple of-platadata test

[Simon Glass]

At present we have a pytest that covers of-platadata. Add a very simple
unit test that just checks that a device can be found. This shows the
ability to write these tests in C.

Signed-off-by: Simon Glass 
---

 test/dm/Makefile  |  4 +++-
 test/dm/of_platdata.c | 19 +++
 2 files changed, 22 insertions(+), 1 deletion(-)
 create mode 100644 test/dm/of_platdata.c

diff --git a/test/dm/Makefile b/test/dm/Makefile
index 387a4b81410..620cdc5f2b4 100644
--- a/test/dm/Makefile
+++ b/test/dm/Makefile
@@ -6,7 +6,9 @@ obj-$(CONFIG_UT_DM) += test-main.o
 
 # Tests for particular subsystems - when enabling driver model for a new
 # subsystem you must add sandbox tests here.
-ifeq ($(CONFIG_SPL_BUILD),)
+ifeq ($(CONFIG_SPL_BUILD),y)
+obj-$(CONFIG_SPL_OF_PLATDATA) += of_platdata.o
+else
 obj-$(CONFIG_UT_DM) += bus.o
 obj-$(CONFIG_UT_DM) += test-driver.o
 obj-$(CONFIG_UT_DM) += test-fdt.o
diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c
new file mode 100644
index 000..7a864eb0fb3
--- /dev/null
+++ b/test/dm/of_platdata.c
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* Test that we can find a device using of-platdata */
+static int dm_test_of_platdata_base(struct unit_test_state *uts)
+{
+   struct udevice *dev;
+
+   ut_assertok(uclass_first_device_err(UCLASS_SERIAL, &dev));
+   ut_asserteq_str("sandbox_serial", dev->name);
+
+   return 0;
+}
+DM_TEST(dm_test_of_platdata_base, UT_TESTF_SCAN_PDATA);
-- 
2.28.0.806.g8561365e88-goog

[PATCH 15/17] pytest: Collect SPL unit tests

[Simon Glass]

Add a new test_spl fixture to handle running SPL unit tests.

Signed-off-by: Simon Glass 
---

 test/py/conftest.py   | 14 +-
 test/py/tests/test_spl.py | 29 +
 2 files changed, 38 insertions(+), 5 deletions(-)
 create mode 100644 test/py/tests/test_spl.py

diff --git a/test/py/conftest.py b/test/py/conftest.py
index 30920474b36..28fde347c00 100644
--- a/test/py/conftest.py
+++ b/test/py/conftest.py
@@ -227,7 +227,7 @@ def pytest_configure(config):
 console = u_boot_console_exec_attach.ConsoleExecAttach(log, ubconfig)
 
 re_ut_test_list = re.compile(r'_u_boot_list_2_(.*)_test_2_\1_test_(.*)\s*$')
-def generate_ut_subtest(metafunc, fixture_name):
+def generate_ut_subtest(metafunc, fixture_name, sym_path):
 """Provide parametrization for a ut_subtest fixture.
 
 Determines the set of unit tests built into a U-Boot binary by parsing the
@@ -237,12 +237,13 @@ def generate_ut_subtest(metafunc, fixture_name):
 Args:
 metafunc: The pytest test function.
 fixture_name: The fixture name to test.
+sym_path: Relative path to the symbol file with preceding '/'
+(e.g. '/u-boot.sym')
 
 Returns:
 Nothing.
 """
-
-fn = console.config.build_dir + '/u-boot.sym'
+fn = console.config.build_dir + sym_path
 try:
 with open(fn, 'rt') as f:
 lines = f.readlines()
@@ -317,10 +318,13 @@ def pytest_generate_tests(metafunc):
 Returns:
 Nothing.
 """
-
+#print('name', metafunc.fixturenames)
 for fn in metafunc.fixturenames:
 if fn == 'ut_subtest':
-generate_ut_subtest(metafunc, fn)
+generate_ut_subtest(metafunc, fn, '/u-boot.sym')
+continue
+if fn == 'ut_spl_subtest':
+generate_ut_subtest(metafunc, fn, '/spl/u-boot-spl.sym')
 continue
 generate_config(metafunc, fn)
 
diff --git a/test/py/tests/test_spl.py b/test/py/tests/test_spl.py
new file mode 100644
index 000..58a851e5ec8
--- /dev/null
+++ b/test/py/tests/test_spl.py
@@ -0,0 +1,29 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright 2020 Google LLC
+# Written by Simon Glass 
+
+import os.path
+import pytest
+
+def test_spl(u_boot_console, ut_spl_subtest):
+"""Execute a "ut" subtest.
+
+The subtests are collected in function generate_ut_subtest() from linker
+generated lists by applying a regular expression to the lines of file
+spl/u-boot-spl.sym. The list entries are created using the C macro
+UNIT_TEST().
+
+Strict naming conventions have to be followed to match the regular
+expression. Use UNIT_TEST(foo_test_bar, _flags, foo_test) for a test bar in
+test suite foo that can be executed via command 'ut foo bar' and is
+implemented in C function foo_test_bar().
+
+Args:
+u_boot_console (ConsoleBase): U-Boot console
+ut_subtest (str): test to be executed via command ut, e.g 'foo bar' to
+execute command 'ut foo bar'
+"""
+cons = u_boot_console
+cons.restart_uboot_with_flags(['-u', ut_spl_subtest])
+output = cons.get_spawn_output().replace('\r', '')
+assert 'Failures: 0' in output
-- 
2.28.0.806.g8561365e88-goog

[PATCH 17/17] Azure/GitLab/Travis: Add SPL unit tests

[Simon Glass]

Run SPL unit tests in all test environments.

Signed-off-by: Simon Glass 
---

 .azure-pipelines.yml | 2 +-
 .gitlab-ci.yml   | 2 +-
 .travis.yml  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index 473ddee3835..a78c8d61300 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -182,7 +182,7 @@ jobs:
   OVERRIDE: "-O clang-10"
 sandbox_spl:
   TEST_PY_BD: "sandbox_spl"
-  TEST_PY_TEST_SPEC: "test_ofplatdata or test_handoff"
+  TEST_PY_TEST_SPEC: "test_ofplatdata or test_handoff or test_spl"
 sandbox_flattree:
   TEST_PY_BD: "sandbox_flattree"
 evb_ast2500:
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 9ac2b336a11..b1e0b3bc9dd 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -198,7 +198,7 @@ sandbox_spl test.py:
   tags: [ 'all' ]
   variables:
 TEST_PY_BD: "sandbox_spl"
-TEST_PY_TEST_SPEC: "test_ofplatdata or test_handoff"
+TEST_PY_TEST_SPEC: "test_ofplatdata or test_handoff or test_spl"
   <<: *buildman_and_testpy_dfn
 
 evb-ast2500 test.py:
diff --git a/.travis.yml b/.travis.yml
index fb8f73157d7..cb48ff3023a 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -512,7 +512,7 @@ matrix:
 - name: "test/py sandbox_spl"
   env:
 - TEST_PY_BD="sandbox_spl"
-  TEST_PY_TEST_SPEC="test_ofplatdata or test_handoff"
+  TEST_PY_TEST_SPEC="test_ofplatdata or test_handoff or test_spl"
   TOOLCHAIN="i386"
   TEST_PY_TOOLS="yes"
 - name: "test/py sandbox_flattree"
-- 
2.28.0.806.g8561365e88-goog

[PATCH 16/17] test: Run SPL unit tests

[Simon Glass]

Update the 'run' script to include SPL unit tests.

Signed-off-by: Simon Glass 
---

 test/run | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/run b/test/run
index de87e7530b1..735628e7e37 100755
--- a/test/run
+++ b/test/run
@@ -28,7 +28,7 @@ fi
 
 # Run tests which require sandbox_spl
 run_test "sandbox_spl" ./test/py/test.py --bd sandbox_spl --build \
-   -k 'test_ofplatdata or test_handoff'
+   -k 'test_ofplatdata or test_handoff or test_spl'
 
 if [ -z "$tools_only" ]; then
# Run tests for the flat-device-tree version of sandbox. This is a 
special
-- 
2.28.0.806.g8561365e88-goog

Re: [PATCH v2 4/4] smbios: Add documentation and devicetree binding

[Simon Glass]

Hi Heinrich,

On Fri, 2 Oct 2020 at 08:57, Heinrich Schuchardt  wrote:
>
> On 02.10.20 16:23, Simon Glass wrote:
> > Add information about how to set SMBIOS properties using the devicetree.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> > (no changes since v1)
> >
> >  doc/arch/x86.rst |  8 +
> >  doc/device-tree-bindings/board/board_x86.txt | 36 
> >  2 files changed, 44 insertions(+)
> >  create mode 100644 doc/device-tree-bindings/board/board_x86.txt
> >
> > diff --git a/doc/arch/x86.rst b/doc/arch/x86.rst
> > index c6b70ce61a3..45f9ba308c7 100644
> > --- a/doc/arch/x86.rst
> > +++ b/doc/arch/x86.rst
> > @@ -740,6 +740,14 @@ Note that this is a development feature only. It is 
> > not intended for use in
> >  production environments. Also it is not currently part of the automated 
> > tests
> >  so may break in the future.
> >
> > +SMBIOS tables
> > +-
> > +
> > +To generate SMBIOS tables in U-Boot, for use by the OS, enable the
> > +CONFIG_GENERATE_SMBIOS_TABLE option. The easiest way to provide the values 
> > to
> > +use is via the device tree. For details see
> > +device-tree-bindings/board/board_x86.txt
> > +
> >  TODO List
> >  -
> >  - Audio
> > diff --git a/doc/device-tree-bindings/board/board_x86.txt 
> > b/doc/device-tree-bindings/board/board_x86.txt
> > new file mode 100644
> > index 000..3766751896a
> > --- /dev/null
> > +++ b/doc/device-tree-bindings/board/board_x86.txt
> > @@ -0,0 +1,36 @@
> > +x86 board driver
> > +
> > +
> > +This driver allows providing board-specific features such as power control
> > +GPIOs. In addition, the SMBIOS values can be specified in the device tree,
> > +as below:
> > +
> > +An optional 'smbios' subnode can be used to provide these properties.
> > +
> > +Optional properties:
> > +  - manufacturer: Product manufacturer for system / baseboard
> > +  - product:  Product name
> > +  - version:  Product version string
> > +  - serial:   Serial number for system (note that this can be 
> > overridden by
> > +  the serial# environment variable)
> > +  - sku:  Product SKU (Stock-Keeping Unit)
> > +  - family:   Product family
> > +  - asset-tag:Asset tag for the motherboard, sometimes used in 
> > organisations
> > +  to track devices
> > +
> > +
> > +Example:
> > +
> > + board {
> > + compatible = "google,coral";
> > +
> > + smbios {
>
> I think board is the wrong place for the new node.
>
> SMBIOS also includes chassis, processor and other information. We may
> opt to provide non-board information in future via the device-tree.

I think of board as the whole thing. Are you thinking of it just being
the circuit board?

>
> If this is for U-Boot internal only usage, we should indicate this in
> the label name.

All drivers are for U-Boot-internal use, right? Or are you saying you
don't want this information to appear in the upstream bindings?

>
> So it could be like:
>
> /u-boot-smbios {

compatible string?

>bios {};
>system {};
>board {
>   manufacturer = "Hardkernel Co., Ltd.";
>   product = "ODROID-C2";
>   ...
>};
>chassis {};
>processor {};
>boot {};
> };
>
> > + manufacturer = "Google";
> > + product = "Coral";
> > + version = "rev2";
> > + serial = "123456789";
> > + sku = "sku3";
> > + family = "Google_Coral";
> > + asset-tag = "ABC123";
> > + };
> > + };
> >
>
> SMBIOS is not x86 specific. On ARM we are also passing an SMBIOS table.
>
> On my Odroid C2 dmidecode shows the information provided by U-Boot:

Oh dear. Legacy never dies. It never occurred to me that this would be
used on ARM.

>
> 
> Handle 0x0001, DMI type 1, 27 bytes
> System Information
> Manufacturer: Hardkernel Co., Ltd.
> Product Name: ODROID-C2
> Version: Not Specified
> ...
> Wake-up Type: Reserved
> SKU Number: Not Specified
> Family: Not Specified
> 
>
>
> So doc/device-tree-bindings/board/board_x86.txt is the wrong place to
> document the new device tree node.
>
> Why not create a document doc/device-tree-bindings/smbios.rst for it?

OK I can do that. I'll await your thoughts on the above.

Regards,
Simon

[PATCH 00/17] dm: test: Add unit tests for SPL

[Simon Glass]

U-Boot has about 700 tests but only a handful of these target SPL. This
is partly because SPL has limited functionality and does not have a
command interface to initiate tests.

The current SPL tests are targeted at sandbox_spl and are initiated from
pytest.

With SPL tests written in C, we can check the behaviour of of-platdata
more easily, since the test can check things directly rather than printing
out information for pytest to check.

This series adds support for SPL tests written in C, targeted at the
sandbox_spl build.

It includes quite a bit of minor refactoring to get everything working.


Simon Glass (17):
  dtoc: Extract inner loop from output_node()
  dtoc: Use a namedtuple for _links
  dm: core: Expand the comment for DM_GET_DEVICE()
  dm: core: Avoid void * in the of-platdata structs
  dm: Avoid using #ifdef for CONFIG_OF_LIVE
  dm: test: Sort the Makefile
  dm: test: Update Makefile conditions
  dm: test: Make use of CONFIG_UNIT_TEST
  dm: test: Disable some tests that should not run in SPL
  dm: test: Build tests for SPL
  dm: test: Update the test runner to support of-platdata
  dm: test: Add a way to run SPL tests
  dm: test: Add a very simple of-platadata test
  Makefile: Generate a symbol file for u-boot-spl
  pytest: Collect SPL unit tests
  test: Run SPL unit tests
  Azure/GitLab/Travis: Add SPL unit tests

 .azure-pipelines.yml  |   2 +-
 .gitlab-ci.yml|   2 +-
 .travis.yml   |   2 +-
 Makefile  |   2 +-
 arch/sandbox/cpu/spl.c|   8 +++
 arch/sandbox/cpu/start.c  |   9 +++
 arch/sandbox/include/asm/state.h  |   1 +
 common/board_r.c  |  19 +++---
 configs/sandbox_spl_defconfig |   2 +-
 drivers/core/Makefile |   2 +-
 drivers/core/root.c   |  27 +++-
 include/asm-generic/global_data.h |  13 +++-
 include/dm/of.h   |   9 +--
 include/dm/platdata.h |  17 -
 include/dt-structs.h  |   8 ++-
 include/test/test.h   |  11 
 scripts/Makefile.spl  |   8 ++-
 test/Kconfig  |  10 +++
 test/Makefile |  24 ---
 test/dm/Makefile  |  13 ++--
 test/dm/of_platdata.c |  19 ++
 test/dm/test-main.c   |  45 +++--
 test/py/conftest.py   |  14 ++--
 test/py/tests/test_spl.py |  29 +
 test/run  |   2 +-
 tools/dtoc/dtb_platdata.py| 103 +-
 26 files changed, 267 insertions(+), 134 deletions(-)
 create mode 100644 test/dm/of_platdata.c
 create mode 100644 test/py/tests/test_spl.py

-- 
2.28.0.806.g8561365e88-goog

Re: Fit images and EFI_LOAD_FILE2_PROTOCOL

[Heinrich Schuchardt]

On 10/3/20 3:12 PM, Ard Biesheuvel wrote:
>
>
> On Sat, 3 Oct 2020 at 13:16, François Ozog  > wrote:
>
>
>
> Le sam. 3 oct. 2020 à 13:14, François Ozog  > a écrit :
>
>
>
> Le sam. 3 oct. 2020 à 10:51, Heinrich Schuchardt
> mailto:xypron.g...@gmx.de>> a écrit :
>
> Hello Ilias, hello Christian,
>
>
>
> with commit ec80b4735a59 ("efi_loader: Implement FileLoad2
> for initramfs
>
> loading") Ilias provided the possibility to specify a device
> path
>
> (CONFIG_EFI_INITRD_FILESPEC) from which an initial RAM disk
> can be
>
> served via the EFI_FILE_LOAD2_PROTOCOL.
>
>
>
> Ard extended the Linux EFI stub to allow loading the initial
> RAM disk
>
> via the EFI_FILE_LOAD2_PROTOCOL with the utmost priority.
>
>
>
> With commit ecc7fdaa9ef1 ("bootm: Add a bootm command for type
>
> IH_OS_EFI") Cristian enabled signed FIT images that contain
> a device
>
> tree and a UEFI binary (enabled by CONFIG_BOOTM_EFI=y).
>
>
>
> In the DTE calls we have discussed that it is unfortunate
> that we do not
>
> have a method to validate initial RAM images in the UEFI
> context.
>
>
>
> To me it would look like a good path forward to combine the
> two ideas:
>
>
>
> * Let the signed FIT image (of type IH_OS_EFI) contain a RAM
> disk
>
> * Pass location and size to the UEFI subsystem and serve
> them via
>
>   the EFI_FILE_LOAD2_PROTOCOL.
>
>
>
> We could also extend the bootefi command to be callable as
>
>
>
>    bootefi $kernel_addr_r $ramdisk_addr_r:$filesize $fdt_addr_r
>
>
>
> like the booti command to serve an initial RAM disk.
>
>
>
> What are your thoughts?
>
> that looks super interesting. 
> I propose something (in the latest desk preparing oct 14th)
> similar except the an efi application boots the FIT.
> I view UEFI as booting a PE coff and pass a set of config
> tables. Today we have DTB, we could just add Initrd (you command
> line). Bootefi would be responsible to valide the containing FIT
> before pushing initrd (and dTB?)into the table. It would be the
> responsibility of the efi stub to get the initrd from the config
> table (GUID to be defined).
>
> the memory attributes of the initrd config table should be such that
> it can be recovered for normal use. That may be tricky though.
>
>
> The purpose of the EFI_FILE_LOAD2_PROTOCOL based initrd loading
> mechanism is to allow the EFI stub (which is tightly coupled to the
> kernel arch/version/etc) to allocate the memory for the initrd, and pass
> it into the LoadFile2() request, using whichever policy it wants to
> adhere to for alignment, offset and/or vicinity of the kernel image. It
> also ensures that any measurement performed by the bootloader for
> attestation or authentication can be delayed to the point where the
> booting kernel assumes ownership of the initrd contents, preventing
> potential TOCTOU issues where intermediate boot stages are involved
> (shim+grub etc)

Any UEFI binary that you invoke can overwrite the complete system table
and replace the existing UEFI API by its own implementation which may be
malicious.

So the EFI_FILE_LOAD2_PROTOCOL does not provide any safety guarantees
whatsoever.

Either you have a chain of trust or not. If you have a chain of trust,
it is sufficient that user input, e.g. an initrd loaded from disk is
verified once.

>
> Creating an initrd config table would mean that the bootloader decides
> where to load the initrd in memory, and only passesthe address and size.
> This is exactly what we wanted to avoid, because now, the bootloader has
> to know all these different rules that vary between kernel version,
> configurations and architectures.
>
> For uboot's implementation of FIT based EFI_FILE_LOAD2_PROTOCOL, this
> might mean that the initrd is loaded into memory first, and copied to
> another location (and [re-]authenticated) when LoadFile2() is invoked. I
> don't think this is a problem in the general case, but we might think
> about ways to avoid this if this turns out to be a problem for memory
> constrained devices with huge initrds.
>
>

Securitywise this all makes no difference. See above.

Best regards

Heinrich

Re: Fit images and EFI_LOAD_FILE2_PROTOCOL

[Ard Biesheuvel]

On Sat, 3 Oct 2020 at 18:35, Heinrich Schuchardt  wrote:
>
> On 10/3/20 3:12 PM, Ard Biesheuvel wrote:
> >
> >
> > On Sat, 3 Oct 2020 at 13:16, François Ozog  > > wrote:
> >
> >
> >
> > Le sam. 3 oct. 2020 à 13:14, François Ozog  > > a écrit :
> >
> >
> >
> > Le sam. 3 oct. 2020 à 10:51, Heinrich Schuchardt
> > mailto:xypron.g...@gmx.de>> a écrit :
> >
> > Hello Ilias, hello Christian,
> >
> >
> >
> > with commit ec80b4735a59 ("efi_loader: Implement FileLoad2
> > for initramfs
> >
> > loading") Ilias provided the possibility to specify a device
> > path
> >
> > (CONFIG_EFI_INITRD_FILESPEC) from which an initial RAM disk
> > can be
> >
> > served via the EFI_FILE_LOAD2_PROTOCOL.
> >
> >
> >
> > Ard extended the Linux EFI stub to allow loading the initial
> > RAM disk
> >
> > via the EFI_FILE_LOAD2_PROTOCOL with the utmost priority.
> >
> >
> >
> > With commit ecc7fdaa9ef1 ("bootm: Add a bootm command for type
> >
> > IH_OS_EFI") Cristian enabled signed FIT images that contain
> > a device
> >
> > tree and a UEFI binary (enabled by CONFIG_BOOTM_EFI=y).
> >
> >
> >
> > In the DTE calls we have discussed that it is unfortunate
> > that we do not
> >
> > have a method to validate initial RAM images in the UEFI
> > context.
> >
> >
> >
> > To me it would look like a good path forward to combine the
> > two ideas:
> >
> >
> >
> > * Let the signed FIT image (of type IH_OS_EFI) contain a RAM
> > disk
> >
> > * Pass location and size to the UEFI subsystem and serve
> > them via
> >
> >   the EFI_FILE_LOAD2_PROTOCOL.
> >
> >
> >
> > We could also extend the bootefi command to be callable as
> >
> >
> >
> >bootefi $kernel_addr_r $ramdisk_addr_r:$filesize $fdt_addr_r
> >
> >
> >
> > like the booti command to serve an initial RAM disk.
> >
> >
> >
> > What are your thoughts?
> >
> > that looks super interesting.
> > I propose something (in the latest desk preparing oct 14th)
> > similar except the an efi application boots the FIT.
> > I view UEFI as booting a PE coff and pass a set of config
> > tables. Today we have DTB, we could just add Initrd (you command
> > line). Bootefi would be responsible to valide the containing FIT
> > before pushing initrd (and dTB?)into the table. It would be the
> > responsibility of the efi stub to get the initrd from the config
> > table (GUID to be defined).
> >
> > the memory attributes of the initrd config table should be such that
> > it can be recovered for normal use. That may be tricky though.
> >
> >
> > The purpose of the EFI_FILE_LOAD2_PROTOCOL based initrd loading
> > mechanism is to allow the EFI stub (which is tightly coupled to the
> > kernel arch/version/etc) to allocate the memory for the initrd, and pass
> > it into the LoadFile2() request, using whichever policy it wants to
> > adhere to for alignment, offset and/or vicinity of the kernel image. It
> > also ensures that any measurement performed by the bootloader for
> > attestation or authentication can be delayed to the point where the
> > booting kernel assumes ownership of the initrd contents, preventing
> > potential TOCTOU issues where intermediate boot stages are involved
> > (shim+grub etc)
>
> Any UEFI binary that you invoke can overwrite the complete system table
> and replace the existing UEFI API by its own implementation which may be
> malicious.
>

I don't see how this has anything to do with what I wrote above.

> So the EFI_FILE_LOAD2_PROTOCOL does not provide any safety guarantees
> whatsoever.
>
> Either you have a chain of trust or not. If you have a chain of trust,
> it is sufficient that user input, e.g. an initrd loaded from disk is
> verified once.
>
> >
> > Creating an initrd config table would mean that the bootloader decides
> > where to load the initrd in memory, and only passesthe address and size.
> > This is exactly what we wanted to avoid, because now, the bootloader has
> > to know all these different rules that vary between kernel version,
> > configurations and architectures.
> >
> > For uboot's implementation of FIT based EFI_FILE_LOAD2_PROTOCOL, this
> > might mean that the initrd is loaded into memory first, and copied to
> > another location (and [re-]authenticated) when LoadFile2() is invoked. I
> > don't think this is a problem in the general case, but we might think
> > about ways to avoid this if this turns out to be a problem for memory
> > constrained devices with huge initrds.
> >
> >
>
> Securitywise this all makes no difference. See above.
>

Yes it

[PATCH 01/20] sandbox: Drop ad-hoc device declarations in SPL

[Simon Glass]

Since sandbox's SPL is build with of-platadata, we should not use
U_BOOT_DEVICE() declarations as well. Drop them.

Signed-off-by: Simon Glass 
---

 board/sandbox/sandbox.c | 2 ++
 drivers/serial/sandbox.c| 3 +++
 drivers/sysreset/sysreset_sandbox.c | 2 ++
 3 files changed, 7 insertions(+)

diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c
index 937ce284111..18a605de026 100644
--- a/board/sandbox/sandbox.c
+++ b/board/sandbox/sandbox.c
@@ -21,10 +21,12 @@
  */
 gd_t *gd;
 
+#if !CONFIG_IS_ENABLED(OF_PLATDATA)
 /* Add a simple GPIO device */
 U_BOOT_DEVICE(gpio_sandbox) = {
.name = "sandbox_gpio",
 };
+#endif
 
 void flush_cache(unsigned long start, unsigned long size)
 {
diff --git a/drivers/serial/sandbox.c b/drivers/serial/sandbox.c
index f09d291e043..db2fbac6295 100644
--- a/drivers/serial/sandbox.c
+++ b/drivers/serial/sandbox.c
@@ -267,6 +267,7 @@ U_BOOT_DRIVER(sandbox_serial) = {
.flags = DM_FLAG_PRE_RELOC,
 };
 
+#if !CONFIG_IS_ENABLED(OF_PLATDATA)
 static const struct sandbox_serial_platdata platdata_non_fdt = {
.colour = -1,
 };
@@ -275,4 +276,6 @@ U_BOOT_DEVICE(serial_sandbox_non_fdt) = {
.name = "sandbox_serial",
.platdata = &platdata_non_fdt,
 };
+#endif
+
 #endif /* CONFIG_IS_ENABLED(OF_CONTROL) */
diff --git a/drivers/sysreset/sysreset_sandbox.c 
b/drivers/sysreset/sysreset_sandbox.c
index 69c22a70008..71cabd19568 100644
--- a/drivers/sysreset/sysreset_sandbox.c
+++ b/drivers/sysreset/sysreset_sandbox.c
@@ -130,7 +130,9 @@ U_BOOT_DRIVER(warm_sysreset_sandbox) = {
.ops= &sandbox_warm_sysreset_ops,
 };
 
+#if !CONFIG_IS_ENABLED(OF_PLATDATA)
 /* This is here in case we don't have a device tree */
 U_BOOT_DEVICE(sysreset_sandbox_non_fdt) = {
.name = "sysreset_sandbox",
 };
+#endif
-- 
2.28.0.806.g8561365e88-goog

[PATCH 00/20] dm: Enhance of-platdata to support parent devices

[Simon Glass]

At present dev_get_parent() always returns the root device with
of-platdata. This means that it is not possible to find the I2C bus for an
I2C device easily. In many cases this does not cause problems as there is
only a single I2C bus, but it is still inconsistent with U-Boot proper.

Worse is that because devices are not bound to the correct parent, they
will not have parent_platdata allocated correctly. Manual fix-ups are
required in the code.

This series adds support for parent devices with of-platdata.

Recent improvements to of-platadata have provided a function to obtain a
device based on its driver_info struct. This requires in generated C code
in dm_populate_phandle_data() which writes the udevice pointer into each
phandle value, since this cannot be known at build time.

This works well and fixes a big hole in of-platdata. But the
implementation has a few drawbacks: the need for generated code adds (very
slightly) to code size and it means that the SPL must sit in writable
memory. The latter can be considered a security risk and is not actually
supported on Intel Apollo Lake.

Another point is that in future of-platadata may support instantiation of
devices at build time. Pointer fix-ups could become quite taxing.

So this series adjusts the approach, storing an index (idx) to the
driver_info struct in the linker list instead. That index can be figured
out in dtoc and the value placed directly in the phandle struct. For
64-bit machines it is also smaller than a pointer. It requires a new table
(driver_rt) to be set up at runtime.

This series adds more SPL tests written in C, using the infrastructure in
the previous series (u-boot-dm/tes-working). A few clean-ups and a bug fix
resulting from these are included.

Finally, x86 is updated to use the parent support. For now, parent support
is behind a Kconfig option, but I expect this will become the default,
assuming that no major problems are found.

SPL size impact is fairly neural, slightly positive on chromebook_jerry
(Thumb-2) and slightly negative on rock64-rk3328 (aarch64):

  aarch64: spl/u-boot-spl:all +39.0 spl/u-boot-spl:rodata -5.0
   spl/u-boot-spl:text +44.0
  arm: spl/u-boot-spl:all -49.0 spl/u-boot-spl:data -80.0
   spl/u-boot-spl:text +36.0 spl/u-boot-spl:rodata -5.0

The increase in code size is due to lists_bind_drivers() having to do
multiple passes to ensure that parents are processed before children. This
could possible be reduced with a more complex linker list ordering
mechanism, but it would be a bit messy and that idea is not explored in
this series.


Simon Glass (20):
  sandbox: Drop ad-hoc device declarations in SPL
  dtoc: Document the return value of scan_structs()
  dtoc: Order the structures internally by name
  dm: core: Allow dm_warn() to be used in SPL
  dtoc: Fix widening of int to bytes
  dm: Add a C test for of-platdata properties
  sandbox: Allow selection of SPL unit tests
  dm: test: Drop of-platdata pytest
  dm: test: Add a check that all devices have a dev value
  dm: test: Add a test for of-platdata phandles
  dm: Use an allocated array for run-time device info
  sandbox: Fix up building for of-platdata
  dm: Support parent devices with of-platdata
  dm: Add a test for of-platdata parent information
  dm: core: Convert #ifdef to if() in root.c
  x86: apl: Enable SPI flash in TPL with APL_SPI_FLASH_BOOT
  x86: apl: Take advantage of the of-platdata parent support
  dm: Use driver_info index instead of pointer
  dm: Don't allow U_BOOT_DEVICE() when of-platdata is used
  dm: doc: Update the of-platadata documentation

 arch/sandbox/cpu/spl.c  |  14 +-
 arch/sandbox/cpu/start.c|  16 +--
 arch/sandbox/dts/sandbox.dts|   1 +
 arch/sandbox/dts/sandbox.dtsi   |  27 
 arch/sandbox/include/asm/state.h|   2 +-
 arch/x86/cpu/apollolake/Kconfig |   2 +
 arch/x86/cpu/apollolake/spl.c   |   3 +-
 board/sandbox/sandbox.c |   2 +
 configs/sandbox_spl_defconfig   |   5 +-
 doc/driver-model/of-plat.rst|  42 +++---
 drivers/clk/clk-uclass.c|   3 +-
 drivers/clk/clk_fixed_rate.c|   4 +-
 drivers/clk/clk_sandbox.c   |   4 +-
 drivers/core/Kconfig|  18 ++-
 drivers/core/device.c   |  23 +++-
 drivers/core/lists.c|  70 +-
 drivers/core/root.c |  29 ++--
 drivers/core/util.c |   2 +-
 drivers/i2c/Makefile|   2 +-
 drivers/i2c/i2c-emul-uclass.c   |   2 +
 drivers/i2c/sandbox_i2c.c   |   4 +-
 drivers/misc/irq-uclass.c   |   2 +-
 drivers/misc/p2sb-uclass.c  |  27 ++--
 drivers/misc/spltest_sandbox.c  |  35 -
 drivers/mmc/fsl_esdhc_imx.c |   7 +-
 drivers/rtc/rtc-uclass.c|   2 +
 drivers/rtc/sandbox_rtc.c   |   4 +-
 drivers/serial/sandbox.c|   3 +
 drivers/spi/ich.c   |   4 +-
 drivers/sysreset/sysreset_sandbox.c |   2 +

[PATCH 02/20] dtoc: Document the return value of scan_structs()

[Simon Glass]

Add documentation to this function as well as generate_structs(), where
the return value is ultimately passed in.

Signed-off-by: Simon Glass 
---

 tools/dtoc/dtb_platdata.py | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py
index 1b52198a943..72725bb6fa1 100644
--- a/tools/dtoc/dtb_platdata.py
+++ b/tools/dtoc/dtb_platdata.py
@@ -466,6 +466,13 @@ class DtbPlatdata(object):
 
 Once the widest property is determined, all other properties are
 updated to match that width.
+
+Returns:
+dict containing structures:
+key (str): Node name, as a C identifier
+value: dict containing structure fields:
+key (str): Field name
+value: Prop object with field information
 """
 structs = {}
 for node in self._valid_nodes:
@@ -536,6 +543,14 @@ class DtbPlatdata(object):
 This writes out the body of a header file consisting of structure
 definitions for node in self._valid_nodes. See the documentation in
 doc/driver-model/of-plat.rst for more information.
+
+Args:
+structs: dict containing structures:
+key (str): Node name, as a C identifier
+value: dict containing structure fields:
+key (str): Field name
+value: Prop object with field information
+
 """
 self.out_header()
 self.out('#include \n')
-- 
2.28.0.806.g8561365e88-goog

[PATCH 04/20] dm: core: Allow dm_warn() to be used in SPL

[Simon Glass]

At present this option is disabled in SPL, meaning that warnings are not
displayed. It is sometimes useful to see warnings in SPL for debugging
purposes.

Add a new Kconfig option to permit this.

Signed-off-by: Simon Glass 
---

 drivers/core/Kconfig   | 18 --
 drivers/core/util.c|  2 +-
 include/config_uncmd_spl.h |  1 -
 include/dm/util.h  |  2 +-
 4 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/core/Kconfig b/drivers/core/Kconfig
index 1ca5d66141b..603a1d27c0c 100644
--- a/drivers/core/Kconfig
+++ b/drivers/core/Kconfig
@@ -40,10 +40,24 @@ config DM_WARN
depends on DM
default y
help
+ Enable this to see warnings related to driver model.
+
+ Warnings may help with debugging, such as when expected devices do
+ not bind correctly. If the option is disabled, dm_warn() is compiled
+ out - it will do nothing when called.
+
+config SPL_DM_WARN
+   bool "Enable warnings in driver model wuth SPL"
+   depends on SPL_DM
+   help
+ Enable this to see warnings related to driver model in SPL
+
  The dm_warn() function can use up quite a bit of space for its
  strings. By default this is disabled for SPL builds to save space.
- This will cause dm_warn() to be compiled out - it will do nothing
- when called.
+
+ Warnings may help with debugging, such as when expected devices do
+ not bind correctly. If the option is disabled, dm_warn() is compiled
+ out - it will do nothing when called.
 
 config DM_DEBUG
bool "Enable debug messages in driver model core"
diff --git a/drivers/core/util.c b/drivers/core/util.c
index 25b0d76f430..91e93b0cf14 100644
--- a/drivers/core/util.c
+++ b/drivers/core/util.c
@@ -11,7 +11,7 @@
 #include 
 #include 
 
-#ifdef CONFIG_DM_WARN
+#if CONFIG_IS_ENABLED(DM_WARN)
 void dm_warn(const char *fmt, ...)
 {
va_list args;
diff --git a/include/config_uncmd_spl.h b/include/config_uncmd_spl.h
index 31da6215b3a..af7e3e49fdd 100644
--- a/include/config_uncmd_spl.h
+++ b/include/config_uncmd_spl.h
@@ -16,7 +16,6 @@
 #undef CONFIG_DM_SPI
 #endif
 
-#undef CONFIG_DM_WARN
 #undef CONFIG_DM_STDIO
 
 #endif /* CONFIG_SPL_BUILD */
diff --git a/include/dm/util.h b/include/dm/util.h
index 9773db6de17..01a044992f2 100644
--- a/include/dm/util.h
+++ b/include/dm/util.h
@@ -6,7 +6,7 @@
 #ifndef __DM_UTIL_H
 #define __DM_UTIL_H
 
-#ifdef CONFIG_DM_WARN
+#if CONFIG_IS_ENABLED(DM_WARN)
 void dm_warn(const char *fmt, ...);
 #else
 static inline void dm_warn(const char *fmt, ...)
-- 
2.28.0.806.g8561365e88-goog

[PATCH 05/20] dtoc: Fix widening of int to bytes

[Simon Glass]

At present an integer is converted to bytes incorrectly. The whole 32-bit
integer is inserted as the first element of the byte array, and the other
three bytes are skipped. This was not noticed because the unit test did
not check it, and the functional test was checking for wrong values.

Update the code to handle this as a special case. Add one more test to
cover all code paths.

Signed-off-by: Simon Glass 
---

 test/py/tests/test_ofplatdata.py |  2 +-
 tools/dtoc/dtoc_test_simple.dts  |  1 +
 tools/dtoc/fdt.py|  9 +
 tools/dtoc/test_dtoc.py  |  4 +++-
 tools/dtoc/test_fdt.py   | 10 ++
 5 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/test/py/tests/test_ofplatdata.py b/test/py/tests/test_ofplatdata.py
index 263334b0743..13154935b9b 100644
--- a/test/py/tests/test_ofplatdata.py
+++ b/test/py/tests/test_ofplatdata.py
@@ -20,7 +20,7 @@ byte 08
 bytearray 01 23 34
 int 3
 intarray 5 0 0 0
-longbytearray 09 00 00 00 00 00 00 00 00
+longbytearray 09 0a 0b 0c 00 00 00 00 00
 string message2
 stringarray "another" "multi-word" "message"
 of-platdata probe:
diff --git a/tools/dtoc/dtoc_test_simple.dts b/tools/dtoc/dtoc_test_simple.dts
index 11bfc4c47aa..fd168cb5931 100644
--- a/tools/dtoc/dtoc_test_simple.dts
+++ b/tools/dtoc/dtoc_test_simple.dts
@@ -41,6 +41,7 @@
u-boot,dm-pre-reloc;
compatible = "sandbox,spl-test";
stringarray = "one";
+   longbytearray = [09 0a 0b 0c 0d 0e 0f 10];
};
 
spl-test4 {
diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py
index d058c59e927..03b86773d5f 100644
--- a/tools/dtoc/fdt.py
+++ b/tools/dtoc/fdt.py
@@ -129,6 +129,15 @@ class Prop:
 specific.
 """
 if newprop.type < self.type:
+# Special handling to convert an int into bytes
+if self.type == TYPE_INT and newprop.type == TYPE_BYTE:
+if type(self.value) == list:
+new_value = []
+for val in self.value:
+new_value += [tools.ToChar(by) for by in val]
+else:
+new_value = [tools.ToChar(by) for by in self.value]
+self.value = new_value
 self.type = newprop.type
 
 if type(newprop.value) == list and type(self.value) != list:
diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py
index 857a98e435f..8dcac91ee70 100755
--- a/tools/dtoc/test_dtoc.py
+++ b/tools/dtoc/test_dtoc.py
@@ -255,7 +255,7 @@ static struct dtd_sandbox_spl_test dtv_spl_test2 = {
 \t.byteval\t\t= 0x8,
 \t.intarray\t\t= {0x5, 0x0, 0x0, 0x0},
 \t.intval\t\t\t= 0x3,
-\t.longbytearray\t\t= {0x9, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0, 0x0,
+\t.longbytearray\t\t= {0x9, 0xa, 0xb, 0xc, 0x0, 0x0, 0x0, 0x0,
 \t\t0x0},
 \t.stringarray\t\t= {"another", "multi-word", "message"},
 \t.stringval\t\t= "message2",
@@ -268,6 +268,8 @@ U_BOOT_DEVICE(spl_test2) = {
 
 /* Node /spl-test3 index 4 */
 static struct dtd_sandbox_spl_test dtv_spl_test3 = {
+\t.longbytearray\t\t= {0x9, 0xa, 0xb, 0xc, 0xd, 0xe, 0xf, 0x10,
+\t\t0x0},
 \t.stringarray\t\t= {"one", "", ""},
 };
 U_BOOT_DEVICE(spl_test3) = {
diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
index b4f9b7f498f..cfe3e04c7ad 100755
--- a/tools/dtoc/test_fdt.py
+++ b/tools/dtoc/test_fdt.py
@@ -298,6 +298,7 @@ class TestProp(unittest.TestCase):
 def testWiden(self):
 """Test widening of values"""
 node2 = self.dtb.GetNode('/spl-test2')
+node3 = self.dtb.GetNode('/spl-test3')
 prop = self.node.props['intval']
 
 # No action
@@ -316,11 +317,20 @@ class TestProp(unittest.TestCase):
 # byte array, it should turn into an array.
 prop = self.node.props['longbytearray']
 prop2 = node2.props['longbytearray']
+prop3 = node3.props['longbytearray']
 self.assertFalse(isinstance(prop2.value, list))
 self.assertEqual(4, len(prop2.value))
+self.assertEqual(b'\x09\x0a\x0b\x0c', prop2.value)
 prop2.Widen(prop)
 self.assertTrue(isinstance(prop2.value, list))
 self.assertEqual(9, len(prop2.value))
+self.assertEqual(['\x09', '\x0a', '\x0b', '\x0c', '\0',
+  '\0', '\0', '\0', '\0'], prop2.value)
+prop3.Widen(prop)
+self.assertTrue(isinstance(prop3.value, list))
+self.assertEqual(9, len(prop3.value))
+self.assertEqual(['\x09', '\x0a', '\x0b', '\x0c', '\x0d',
+  '\x0e', '\x0f', '\x10', '\0'], prop3.value)
 
 # Similarly for a string array
 prop = self.node.props['stringval']
-- 
2.28.0.806.g8561365e88-goog

[PATCH 03/20] dtoc: Order the structures internally by name

[Simon Glass]

At present the structures are written in name order, but parents have to
be written before their children, so the file does not end up being in
order. The order of nodes in _valid_nodes matches the order of the
devicetree.

Update the code so that _valid_nodes is in sorted order, by C name of
the structure. This allows us to assign a sequential ordering to each
U_BOOT_DEVICE() declaration.

U-Boot's linker lists are also ordered alphabetically, which means that
the order in the driver_info list will match the order used by dtoc. This
defines an index ('idx') for the U_BOOT_DEVICE declarations. They appear
in alphabetical order, numbered from 0 in _valid_nodes and in the
driver_info linker list.

Add a comment against each declaration, showing the idx value.

Signed-off-by: Simon Glass 
---

 tools/dtoc/dtb_platdata.py |  21 +---
 tools/dtoc/test_dtoc.py| 105 -
 2 files changed, 83 insertions(+), 43 deletions(-)

diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py
index 72725bb6fa1..31a9b3877ea 100644
--- a/tools/dtoc/dtb_platdata.py
+++ b/tools/dtoc/dtb_platdata.py
@@ -143,7 +143,8 @@ class DtbPlatdata(object):
 Properties:
 _fdt: Fdt object, referencing the device tree
 _dtb_fname: Filename of the input device tree binary file
-_valid_nodes: A list of Node object with compatible strings
+_valid_nodes: A list of Node object with compatible strings. The list
+is ordered by conv_name_to_c(node.name)
 _include_disabled: true to include nodes marked status = "disabled"
 _outfile: The current output file (sys.stdout or a real file)
 _warning_disabled: true to disable warnings about driver names not 
found
@@ -367,23 +368,24 @@ class DtbPlatdata(object):
 """
 self._fdt = fdt.FdtScan(self._dtb_fname)
 
-def scan_node(self, root):
+def scan_node(self, root, valid_nodes):
 """Scan a node and subnodes to build a tree of node and phandle info
 
 This adds each node to self._valid_nodes.
 
 Args:
 root: Root node for scan
+valid_nodes: List of Node objects to add to
 """
 for node in root.subnodes:
 if 'compatible' in node.props:
 status = node.props.get('status')
 if (not self._include_disabled and not status or
 status.value != 'disabled'):
-self._valid_nodes.append(node)
+valid_nodes.append(node)
 
 # recurse to handle any subnodes
-self.scan_node(node)
+self.scan_node(node, valid_nodes)
 
 def scan_tree(self):
 """Scan the device tree for useful information
@@ -392,8 +394,12 @@ class DtbPlatdata(object):
 _valid_nodes: A list of nodes we wish to consider include in the
 platform data
 """
-self._valid_nodes = []
-return self.scan_node(self._fdt.GetRoot())
+valid_nodes = []
+self.scan_node(self._fdt.GetRoot(), valid_nodes)
+self._valid_nodes = sorted(valid_nodes,
+   key=lambda x: conv_name_to_c(x.name))
+for idx, node in enumerate(self._valid_nodes):
+node.idx = idx
 
 @staticmethod
 def get_num_cells(node):
@@ -474,7 +480,7 @@ class DtbPlatdata(object):
 key (str): Field name
 value: Prop object with field information
 """
-structs = {}
+structs = collections.OrderedDict()
 for node in self._valid_nodes:
 node_name, _ = self.get_normalized_compat_name(node)
 fields = {}
@@ -633,6 +639,7 @@ class DtbPlatdata(object):
 
 struct_name, _ = self.get_normalized_compat_name(node)
 var_name = conv_name_to_c(node.name)
+self.buf('/* Node %s index %d */\n' % (node.path, node.idx))
 self.buf('static struct %s%s %s%s = {\n' %
  (STRUCT_PREFIX, struct_name, VAL_PREFIX, var_name))
 for pname in sorted(node.props):
diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py
index c2ff267de70..857a98e435f 100755
--- a/tools/dtoc/test_dtoc.py
+++ b/tools/dtoc/test_dtoc.py
@@ -209,6 +209,27 @@ struct dtd_sandbox_spl_test_2 {
 with open(output) as infile:
 data = infile.read()
 self._CheckStrings(C_HEADER + '''
+/* Node /i2c@0 index 0 */
+static struct dtd_sandbox_i2c_test dtv_i2c_at_0 = {
+};
+U_BOOT_DEVICE(i2c_at_0) = {
+\t.name\t\t= "sandbox_i2c_test",
+\t.platdata\t= &dtv_i2c_at_0,
+\t.platdata_size\t= sizeof(dtv_i2c_at_0),
+};
+
+/* Node /i2c@0/pmic@9 index 1 */
+static struct dtd_sandbox_pmic_test dtv_pmic_at_9 = {
+\t.low_power\t\t= true,
+\t.reg\t\t\t= {0x9, 0x0},
+};
+U_BOOT_DEVICE(pmic_at_9) = {
+\t.name\t\t= "sandbox_pmic_test",
+\t.platdata\t= &dtv_pmic_at_9,
+\t.platdata_size\t= sizeof(dtv_pmic_at_9),
+};
+
+/* Node /spl-test index 2 *

[PATCH 08/20] dm: test: Drop of-platdata pytest

[Simon Glass]

Now that we have a C version of this test, drop the Python implementation.

Signed-off-by: Simon Glass 
---

 arch/sandbox/cpu/spl.c   | 12 
 arch/sandbox/cpu/start.c |  9 --
 arch/sandbox/include/asm/state.h |  1 -
 drivers/misc/spltest_sandbox.c   | 35 
 test/py/tests/test_ofplatdata.py | 47 
 5 files changed, 104 deletions(-)

diff --git a/arch/sandbox/cpu/spl.c b/arch/sandbox/cpu/spl.c
index 81b217a1d70..9a77da15619 100644
--- a/arch/sandbox/cpu/spl.c
+++ b/arch/sandbox/cpu/spl.c
@@ -54,20 +54,8 @@ SPL_LOAD_IMAGE_METHOD("sandbox", 9, BOOT_DEVICE_BOARD, 
spl_board_load_image);
 void spl_board_init(void)
 {
struct sandbox_state *state = state_get_current();
-   struct udevice *dev;
 
preloader_console_init();
-   if (state->show_of_platdata) {
-   /*
-* Scan all the devices so that we can output their platform
-* data. See sandbox_spl_probe().
-*/
-   printf("Scanning misc devices\n");
-   for (uclass_first_device(UCLASS_MISC, &dev);
-dev;
-uclass_next_device(&dev))
-   ;
-   }
 
if (state->run_unittests) {
int ret;
diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
index 569dafbcfee..58ada13fba5 100644
--- a/arch/sandbox/cpu/start.c
+++ b/arch/sandbox/cpu/start.c
@@ -365,15 +365,6 @@ static int sandbox_cmdline_cb_log_level(struct 
sandbox_state *state,
 SANDBOX_CMDLINE_OPT_SHORT(log_level, 'L', 1,
  "Set log level (0=panic, 7=debug)");
 
-static int sandbox_cmdline_cb_show_of_platdata(struct sandbox_state *state,
-  const char *arg)
-{
-   state->show_of_platdata = true;
-
-   return 0;
-}
-SANDBOX_CMDLINE_OPT(show_of_platdata, 0, "Show of-platdata in SPL");
-
 static int sandbox_cmdline_cb_unittests(struct sandbox_state *state,
const char *arg)
 {
diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h
index 7547602dd1c..bca13069824 100644
--- a/arch/sandbox/include/asm/state.h
+++ b/arch/sandbox/include/asm/state.h
@@ -90,7 +90,6 @@ struct sandbox_state {
bool skip_delays;   /* Ignore any time delays (for test) */
bool show_test_output;  /* Don't suppress stdout in tests */
int default_log_level;  /* Default log level for sandbox */
-   bool show_of_platdata;  /* Show of-platdata in SPL */
bool ram_buf_read;  /* true if we read the RAM buffer */
bool run_unittests; /* Run unit tests */
const char *select_unittests;   /* Unit test to run */
diff --git a/drivers/misc/spltest_sandbox.c b/drivers/misc/spltest_sandbox.c
index 999031625b5..3ae6707593e 100644
--- a/drivers/misc/spltest_sandbox.c
+++ b/drivers/misc/spltest_sandbox.c
@@ -8,43 +8,8 @@
 #include 
 #include 
 
-static int sandbox_spl_probe(struct udevice *dev)
-{
-   struct dtd_sandbox_spl_test *plat = dev_get_platdata(dev);
-   int i;
-
-   printf("of-platdata probe:\n");
-   printf("bool %d\n", plat->boolval);
-
-   printf("byte %02x\n", plat->byteval);
-   printf("bytearray");
-   for (i = 0; i < sizeof(plat->bytearray); i++)
-   printf(" %02x", plat->bytearray[i]);
-   printf("\n");
-
-   printf("int %d\n", plat->intval);
-   printf("intarray");
-   for (i = 0; i < ARRAY_SIZE(plat->intarray); i++)
-   printf(" %d", plat->intarray[i]);
-   printf("\n");
-
-   printf("longbytearray");
-   for (i = 0; i < sizeof(plat->longbytearray); i++)
-   printf(" %02x", plat->longbytearray[i]);
-   printf("\n");
-
-   printf("string %s\n", plat->stringval);
-   printf("stringarray");
-   for (i = 0; i < ARRAY_SIZE(plat->stringarray); i++)
-   printf(" \"%s\"", plat->stringarray[i]);
-   printf("\n");
-
-   return 0;
-}
-
 U_BOOT_DRIVER(sandbox_spl_test) = {
.name   = "sandbox_spl_test",
.id = UCLASS_MISC,
.flags  = DM_FLAG_PRE_RELOC,
-   .probe  = sandbox_spl_probe,
 };
diff --git a/test/py/tests/test_ofplatdata.py b/test/py/tests/test_ofplatdata.py
index 13154935b9b..78837a3c008 100644
--- a/test/py/tests/test_ofplatdata.py
+++ b/test/py/tests/test_ofplatdata.py
@@ -4,53 +4,6 @@
 import pytest
 import u_boot_utils as util
 
-OF_PLATDATA_OUTPUT = '''
-of-platdata probe:
-bool 1
-byte 05
-bytearray 06 00 00
-int 1
-intarray 2 3 4 0
-longbytearray 09 0a 0b 0c 0d 0e 0f 10 11
-string message
-stringarray "multi-word" "message" ""
-of-platdata probe:
-bool 0
-byte 08
-bytearray 01 23 34
-int 3
-intarray 5 0 0 0
-longbytearray 09 0a 0b 0c 00 00 00 00 00
-string message2
-stringarray "another" "multi-word" "message"
-of-platdata probe:
-bool 0
-byte 00
-bytearray 00 00 0

[PATCH 06/20] dm: Add a C test for of-platdata properties

[Simon Glass]

At present properties are tested in a roundabout way. The driver's probe()
method writes out the values of the properties and the Python test checks
the output from U-Boot SPL.

Add a C test which checks these values more directly.

Signed-off-by: Simon Glass 
---

 test/dm/of_platdata.c | 69 +++
 1 file changed, 69 insertions(+)

diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c
index 7a864eb0fb3..8763005ea9a 100644
--- a/test/dm/of_platdata.c
+++ b/test/dm/of_platdata.c
@@ -2,6 +2,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -17,3 +18,71 @@ static int dm_test_of_platdata_base(struct unit_test_state 
*uts)
return 0;
 }
 DM_TEST(dm_test_of_platdata_base, UT_TESTF_SCAN_PDATA);
+
+/* Test that we can read properties from a device */
+static int dm_test_of_platdata_props(struct unit_test_state *uts)
+{
+   struct dtd_sandbox_spl_test *plat;
+   struct udevice *dev;
+   int i;
+
+   ut_assertok(uclass_first_device_err(UCLASS_MISC, &dev));
+   plat = dev_get_platdata(dev);
+   ut_assert(plat->boolval);
+   ut_asserteq(1, plat->intval);
+   ut_asserteq(4, ARRAY_SIZE(plat->intarray));
+   ut_asserteq(2, plat->intarray[0]);
+   ut_asserteq(3, plat->intarray[1]);
+   ut_asserteq(4, plat->intarray[2]);
+   ut_asserteq(0, plat->intarray[3]);
+   ut_asserteq(5, plat->byteval);
+   ut_asserteq(3, ARRAY_SIZE(plat->bytearray));
+   ut_asserteq(6, plat->bytearray[0]);
+   ut_asserteq(0, plat->bytearray[1]);
+   ut_asserteq(0, plat->bytearray[2]);
+   ut_asserteq(9, ARRAY_SIZE(plat->longbytearray));
+   for (i = 0; i < ARRAY_SIZE(plat->longbytearray); i++)
+   ut_asserteq(9 + i, plat->longbytearray[i]);
+   ut_asserteq_str("message", plat->stringval);
+   ut_asserteq(3, ARRAY_SIZE(plat->stringarray));
+   ut_asserteq_str("multi-word", plat->stringarray[0]);
+   ut_asserteq_str("message", plat->stringarray[1]);
+   ut_asserteq_str("", plat->stringarray[2]);
+
+   ut_assertok(uclass_next_device_err(&dev));
+   plat = dev_get_platdata(dev);
+   ut_assert(!plat->boolval);
+   ut_asserteq(3, plat->intval);
+   ut_asserteq(5, plat->intarray[0]);
+   ut_asserteq(0, plat->intarray[1]);
+   ut_asserteq(0, plat->intarray[2]);
+   ut_asserteq(0, plat->intarray[3]);
+   ut_asserteq(8, plat->byteval);
+   ut_asserteq(3, ARRAY_SIZE(plat->bytearray));
+   ut_asserteq(1, plat->bytearray[0]);
+   ut_asserteq(0x23, plat->bytearray[1]);
+   ut_asserteq(0x34, plat->bytearray[2]);
+   for (i = 0; i < ARRAY_SIZE(plat->longbytearray); i++)
+   ut_asserteq(i < 4 ? 9 + i : 0, plat->longbytearray[i]);
+   ut_asserteq_str("message2", plat->stringval);
+   ut_asserteq_str("another", plat->stringarray[0]);
+   ut_asserteq_str("multi-word", plat->stringarray[1]);
+   ut_asserteq_str("message", plat->stringarray[2]);
+
+   ut_assertok(uclass_next_device_err(&dev));
+   plat = dev_get_platdata(dev);
+   ut_assert(!plat->boolval);
+   ut_asserteq_str("one", plat->stringarray[0]);
+   ut_asserteq_str("", plat->stringarray[1]);
+   ut_asserteq_str("", plat->stringarray[2]);
+
+   ut_assertok(uclass_next_device_err(&dev));
+   plat = dev_get_platdata(dev);
+   ut_assert(!plat->boolval);
+   ut_asserteq_str("spl", plat->stringarray[0]);
+
+   ut_asserteq(-ENODEV, uclass_next_device_err(&dev));
+
+   return 0;
+}
+DM_TEST(dm_test_of_platdata_props, UT_TESTF_SCAN_PDATA);
-- 
2.28.0.806.g8561365e88-goog

[PATCH 09/20] dm: test: Add a check that all devices have a dev value

[Simon Glass]

With of-platdata, the driver_info struct is updated with the device
pointer when it is bound. This makes it easy for a device to be found by
its driver info with the device_get_by_driver_info() function.

Add a test that all devices (except the root device) have such an entry.
Fix a bug that the function does not set *devp to NULL on failure, which
the documentation asserts.

Signed-off-by: Simon Glass 
---

 drivers/core/device.c |  1 +
 test/dm/of_platdata.c | 81 +++
 2 files changed, 82 insertions(+)

diff --git a/drivers/core/device.c b/drivers/core/device.c
index 355dbd147a9..aacc6adf02d 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -745,6 +745,7 @@ int device_get_by_driver_info(const struct driver_info 
*info,
struct udevice *dev;
 
dev = info->dev;
+   *devp = NULL;
 
return device_get_device_tail(dev, dev ? 0 : -ENOENT, devp);
 }
diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c
index 8763005ea9a..80900e416ba 100644
--- a/test/dm/of_platdata.c
+++ b/test/dm/of_platdata.c
@@ -86,3 +86,84 @@ static int dm_test_of_platdata_props(struct unit_test_state 
*uts)
return 0;
 }
 DM_TEST(dm_test_of_platdata_props, UT_TESTF_SCAN_PDATA);
+
+/*
+ * find_driver_info - recursively find the driver_info for a device
+ *
+ * This sets found[idx] to true when it finds the driver_info record for a
+ * device, where idx is the index in the driver_info linker list.
+ *
+ * @uts: Test state
+ * @parent: Parent to search
+ * @found: bool array to update
+ * @return 0 if OK, non-zero on error
+ */
+static int find_driver_info(struct unit_test_state *uts, struct udevice 
*parent,
+   bool found[])
+{
+   struct udevice *dev;
+
+   /* If not the root device, find the entry that caused it to be bound */
+   if (parent->parent) {
+   const struct driver_info *info =
+   ll_entry_start(struct driver_info, driver_info);
+   const int n_ents =
+   ll_entry_count(struct driver_info, driver_info);
+   const struct driver_info *entry;
+   int idx = -1;
+
+   for (entry = info; entry != info + n_ents; entry++) {
+   if (entry->dev == parent) {
+   idx = entry - info;
+   found[idx] = true;
+   break;
+   }
+   }
+
+   ut_assert(idx != -1);
+   }
+
+   device_foreach_child(dev, parent) {
+   int ret;
+
+   ret = find_driver_info(uts, dev, found);
+   if (ret < 0)
+   return ret;
+   }
+
+   return 0;
+}
+
+/* Check that every device is recorded in its driver_info struct */
+static int dm_test_of_platdata_dev(struct unit_test_state *uts)
+{
+   const struct driver_info *info =
+   ll_entry_start(struct driver_info, driver_info);
+   const int n_ents = ll_entry_count(struct driver_info, driver_info);
+   bool found[n_ents];
+   uint i;
+
+   /* Record the indexes that are found */
+   memset(found, '\0', sizeof(found));
+   ut_assertok(find_driver_info(uts, gd->dm_root, found));
+
+   /* Make sure that the driver entries without devices have no ->dev */
+   for (i = 0; i < n_ents; i++) {
+   const struct driver_info *entry = info + i;
+   struct udevice *dev;
+
+   if (found[i]) {
+   /* Make sure we can find it */
+   ut_assertnonnull(entry->dev);
+   ut_assertok(device_get_by_driver_info(entry, &dev));
+   ut_asserteq_ptr(dev, entry->dev);
+   } else {
+   ut_assertnull(entry->dev);
+   ut_asserteq(-ENOENT,
+   device_get_by_driver_info(entry, &dev));
+   }
+   }
+
+   return 0;
+}
+DM_TEST(dm_test_of_platdata_dev, UT_TESTF_SCAN_PDATA);
-- 
2.28.0.806.g8561365e88-goog

[PATCH 07/20] sandbox: Allow selection of SPL unit tests

[Simon Glass]

Now that we have more than one test, add a way to select the test to run.

Signed-off-by: Simon Glass 
---

 arch/sandbox/cpu/spl.c   | 2 +-
 arch/sandbox/cpu/start.c | 9 +
 arch/sandbox/include/asm/state.h | 1 +
 3 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/sandbox/cpu/spl.c b/arch/sandbox/cpu/spl.c
index 48fd1265afe..81b217a1d70 100644
--- a/arch/sandbox/cpu/spl.c
+++ b/arch/sandbox/cpu/spl.c
@@ -72,7 +72,7 @@ void spl_board_init(void)
if (state->run_unittests) {
int ret;
 
-   ret = dm_test_main(NULL);
+   ret = dm_test_main(state->select_unittests);
/* continue execution into U-Boot */
}
 }
diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
index f5e104b127b..569dafbcfee 100644
--- a/arch/sandbox/cpu/start.c
+++ b/arch/sandbox/cpu/start.c
@@ -383,6 +383,15 @@ static int sandbox_cmdline_cb_unittests(struct 
sandbox_state *state,
 }
 SANDBOX_CMDLINE_OPT_SHORT(unittests, 'u', 0, "Run unit tests");
 
+static int sandbox_cmdline_cb_select_unittests(struct sandbox_state *state,
+  const char *arg)
+{
+   state->select_unittests = arg;
+
+   return 0;
+}
+SANDBOX_CMDLINE_OPT_SHORT(select_unittests, 'k', 1, "Select unit tests to 
run");
+
 static void setup_ram_buf(struct sandbox_state *state)
 {
/* Zero the RAM buffer if we didn't read it, to keep valgrind happy */
diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h
index f828d9d2447..7547602dd1c 100644
--- a/arch/sandbox/include/asm/state.h
+++ b/arch/sandbox/include/asm/state.h
@@ -93,6 +93,7 @@ struct sandbox_state {
bool show_of_platdata;  /* Show of-platdata in SPL */
bool ram_buf_read;  /* true if we read the RAM buffer */
bool run_unittests; /* Run unit tests */
+   const char *select_unittests;   /* Unit test to run */
 
/* Pointer to information for each SPI bus/cs */
struct sandbox_spi_info spi[CONFIG_SANDBOX_SPI_MAX_BUS]
-- 
2.28.0.806.g8561365e88-goog

[PATCH 10/20] dm: test: Add a test for of-platdata phandles

[Simon Glass]

We have a test in dtoc for this feature, but not one in U-Boot itself.
Add a simple test that checks that the information comes through
correctly.

Signed-off-by: Simon Glass 
---

 arch/sandbox/dts/sandbox.dtsi | 26 
 configs/sandbox_spl_defconfig |  1 +
 drivers/clk/clk_fixed_rate.c  |  4 ++--
 drivers/clk/clk_sandbox.c |  4 ++--
 test/dm/of_platdata.c | 37 +++
 5 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi
index c76ecc013c9..ca7049fc974 100644
--- a/arch/sandbox/dts/sandbox.dtsi
+++ b/arch/sandbox/dts/sandbox.dtsi
@@ -29,6 +29,32 @@
};
};
 
+   clk_fixed: clk-fixed {
+   u-boot,dm-pre-reloc;
+   compatible = "fixed-clock";
+   #clock-cells = <0>;
+   clock-frequency = <1234>;
+   };
+
+   clk_sandbox: clk-sbox {
+   u-boot,dm-pre-reloc;
+   compatible = "sandbox,clk";
+   #clock-cells = <1>;
+   assigned-clocks = <&clk_sandbox 3>;
+   assigned-clock-rates = <321>;
+   };
+
+   clk-test {
+   u-boot,dm-pre-reloc;
+   compatible = "sandbox,clk-test";
+   clocks = <&clk_fixed>,
+<&clk_sandbox 1>,
+<&clk_sandbox 0>,
+<&clk_sandbox 3>,
+<&clk_sandbox 2>;
+   clock-names = "fixed", "i2c", "spi", "uart2", "uart1";
+   };
+
gpio_a: gpios@0 {
u-boot,dm-pre-reloc;
gpio-controller;
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
index 49060cab7a3..91215d23a0f 100644
--- a/configs/sandbox_spl_defconfig
+++ b/configs/sandbox_spl_defconfig
@@ -104,6 +104,7 @@ CONFIG_ADC_SANDBOX=y
 CONFIG_AXI=y
 CONFIG_AXI_SANDBOX=y
 CONFIG_CLK=y
+CONFIG_SPL_CLK=y
 CONFIG_CPU=y
 CONFIG_DM_DEMO=y
 CONFIG_DM_DEMO_SIMPLE=y
diff --git a/drivers/clk/clk_fixed_rate.c b/drivers/clk/clk_fixed_rate.c
index 55e1f8caa52..f86b4a0e924 100644
--- a/drivers/clk/clk_fixed_rate.c
+++ b/drivers/clk/clk_fixed_rate.c
@@ -46,8 +46,8 @@ static const struct udevice_id clk_fixed_rate_match[] = {
{ /* sentinel */ }
 };
 
-U_BOOT_DRIVER(clk_fixed_rate) = {
-   .name = "fixed_rate_clock",
+U_BOOT_DRIVER(fixed_clock) = {
+   .name = "fixed_clock",
.id = UCLASS_CLK,
.of_match = clk_fixed_rate_match,
.ofdata_to_platdata = clk_fixed_rate_ofdata_to_platdata,
diff --git a/drivers/clk/clk_sandbox.c b/drivers/clk/clk_sandbox.c
index 768fbb7c520..0ff1b496338 100644
--- a/drivers/clk/clk_sandbox.c
+++ b/drivers/clk/clk_sandbox.c
@@ -124,8 +124,8 @@ static const struct udevice_id sandbox_clk_ids[] = {
{ }
 };
 
-U_BOOT_DRIVER(clk_sandbox) = {
-   .name   = "clk_sandbox",
+U_BOOT_DRIVER(sandbox_clk) = {
+   .name   = "sandbox_clk",
.id = UCLASS_CLK,
.of_match   = sandbox_clk_ids,
.ops= &sandbox_clk_ops,
diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c
index 80900e416ba..57f903611a6 100644
--- a/test/dm/of_platdata.c
+++ b/test/dm/of_platdata.c
@@ -26,7 +26,11 @@ static int dm_test_of_platdata_props(struct unit_test_state 
*uts)
struct udevice *dev;
int i;
 
+   /* Skip the clock */
ut_assertok(uclass_first_device_err(UCLASS_MISC, &dev));
+   ut_asserteq_str("sandbox_clk_test", dev->name);
+
+   ut_assertok(uclass_next_device_err(&dev));
plat = dev_get_platdata(dev);
ut_assert(plat->boolval);
ut_asserteq(1, plat->intval);
@@ -167,3 +171,36 @@ static int dm_test_of_platdata_dev(struct unit_test_state 
*uts)
return 0;
 }
 DM_TEST(dm_test_of_platdata_dev, UT_TESTF_SCAN_PDATA);
+
+/* Test handling of phandles that point to other devices */
+static int dm_test_of_platdata_phandle(struct unit_test_state *uts)
+{
+   struct dtd_sandbox_clk_test *plat;
+   struct udevice *dev, *clk;
+
+   ut_assertok(uclass_first_device_err(UCLASS_MISC, &dev));
+   ut_asserteq_str("sandbox_clk_test", dev->name);
+   plat = dev_get_platdata(dev);
+
+   ut_assertok(device_get_by_driver_info(plat->clocks[0].node, &clk));
+   ut_asserteq_str("fixed_clock", clk->name);
+
+   ut_assertok(device_get_by_driver_info(plat->clocks[1].node, &clk));
+   ut_asserteq_str("sandbox_clk", clk->name);
+   ut_asserteq(1, plat->clocks[1].arg[0]);
+
+   ut_assertok(device_get_by_driver_info(plat->clocks[2].node, &clk));
+   ut_asserteq_str("sandbox_clk", clk->name);
+   ut_asserteq(0, plat->clocks[2].arg[0]);
+
+   ut_assertok(device_get_by_driver_info(plat->clocks[3].node, &clk));
+   ut_asserteq_str("sandbox_clk", clk->name);
+   ut_asserteq(3, plat->clocks[3].arg[0]);
+
+   ut_assertok(device_get_by_driver_info(plat->clocks[4].node, &clk));
+ 

[PATCH 11/20] dm: Use an allocated array for run-time device info

[Simon Glass]

At present we update the driver_info struct with a pointer to the device
that it created (i.e. caused to be bound). This works fine when U-Boot SPL
is stored in read-write memory. But on some platforms, such as Intel
Apollo Lake, it is not possible to update the data memory.

In any case, it is bad form to put this information in a structure that is
in the data region, since it expands the size of the binary.

Create a new driver_rt structure which holds runtime information about
drivers. Update the code to store the device pointer in this instead.
Also update the test check that this works.

Signed-off-by: Simon Glass 
---

 drivers/core/device.c | 11 ++-
 drivers/core/lists.c  | 16 +++-
 drivers/core/root.c   | 11 +++
 include/asm-generic/global_data.h | 13 +
 include/dm/device-internal.h  |  2 +-
 include/dm/platdata.h | 14 --
 test/dm/of_platdata.c | 19 ++-
 7 files changed, 64 insertions(+), 22 deletions(-)

diff --git a/drivers/core/device.c b/drivers/core/device.c
index aacc6adf02d..c3338efc4fa 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -249,7 +249,7 @@ int device_bind_ofnode(struct udevice *parent, const struct 
driver *drv,
 }
 
 int device_bind_by_name(struct udevice *parent, bool pre_reloc_only,
-   struct driver_info *info, struct udevice **devp)
+   const struct driver_info *info, struct udevice **devp)
 {
struct driver *drv;
uint platdata_size = 0;
@@ -269,9 +269,6 @@ int device_bind_by_name(struct udevice *parent, bool 
pre_reloc_only,
 platdata_size, devp);
if (ret)
return ret;
-#if CONFIG_IS_ENABLED(OF_PLATDATA)
-   info->dev = *devp;
-#endif
 
return ret;
 }
@@ -742,9 +739,13 @@ int device_get_global_by_ofnode(ofnode ofnode, struct 
udevice **devp)
 int device_get_by_driver_info(const struct driver_info *info,
  struct udevice **devp)
 {
+   struct driver_info *info_base =
+   ll_entry_start(struct driver_info, driver_info);
+   int idx = info - info_base;
+   struct driver_rt *drt = gd_dm_driver_rt() + idx;
struct udevice *dev;
 
-   dev = info->dev;
+   dev = drt->dev;
*devp = NULL;
 
return device_get_device_tail(dev, dev ? 0 : -ENOENT, devp);
diff --git a/drivers/core/lists.c b/drivers/core/lists.c
index 5beba9181cc..2e6bd5006ce 100644
--- a/drivers/core/lists.c
+++ b/drivers/core/lists.c
@@ -56,14 +56,20 @@ int lists_bind_drivers(struct udevice *parent, bool 
pre_reloc_only)
struct driver_info *info =
ll_entry_start(struct driver_info, driver_info);
const int n_ents = ll_entry_count(struct driver_info, driver_info);
-   struct driver_info *entry;
-   struct udevice *dev;
int result = 0;
-   int ret;
+   uint idx;
+
+   for (idx = 0; idx < n_ents; idx++) {
+   const struct driver_info *entry = info + idx;
+   struct driver_rt *drt = gd_dm_driver_rt() + idx;
+   struct udevice *dev;
+   int ret;
 
-   for (entry = info; entry != info + n_ents; entry++) {
ret = device_bind_by_name(parent, pre_reloc_only, entry, &dev);
-   if (ret && ret != -EPERM) {
+   if (!ret) {
+   if (CONFIG_IS_ENABLED(OF_PLATDATA))
+   drt->dev = dev;
+   } else if (ret != -EPERM) {
dm_warn("No match for driver '%s'\n", entry->name);
if (!result || ret != -ENOENT)
result = ret;
diff --git a/drivers/core/root.c b/drivers/core/root.c
index de23161cff8..e8df5aebe84 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -185,6 +185,17 @@ int dm_scan_platdata(bool pre_reloc_only)
 {
int ret;
 
+   if (CONFIG_IS_ENABLED(OF_PLATDATA)) {
+   struct driver_rt *dyn;
+   int n_ents;
+
+   n_ents = ll_entry_count(struct driver_info, driver_info);
+   dyn = calloc(n_ents, sizeof(struct driver_rt));
+   if (!dyn)
+   return -ENOMEM;
+   gd_set_dm_driver_rt(dyn);
+   }
+
ret = lists_bind_drivers(DM_ROOT_NON_CONST, pre_reloc_only);
if (ret == -ENOENT) {
dm_warn("Some drivers were not found\n");
diff --git a/include/asm-generic/global_data.h 
b/include/asm-generic/global_data.h
index d6f562d90d4..7fc53e8deee 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -24,6 +24,8 @@
 #include 
 #include 
 
+struct driver_rt;
+
 typedef struct global_data {
struct bd_info *bd;
unsigned long flags;
@@ -67,6 +69,9 @@ typedef struct global_data {
struct udevice  *dm_root;   /* Root instance f

[PATCH 15/20] dm: core: Convert #ifdef to if() in root.c

[Simon Glass]

Convert a few conditions to use compile-time checks to reduce the number
of build paths.

Signed-off-by: Simon Glass 
---

 drivers/core/root.c | 18 +++---
 1 file changed, 7 insertions(+), 11 deletions(-)

diff --git a/drivers/core/root.c b/drivers/core/root.c
index e8df5aebe84..5f10d7a39c7 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -50,7 +50,6 @@ void dm_fixup_for_gd_move(struct global_data *new_gd)
}
 }
 
-#if defined(CONFIG_NEEDS_MANUAL_RELOC)
 void fix_drivers(void)
 {
struct driver *drv =
@@ -129,8 +128,6 @@ void fix_devices(void)
}
 }
 
-#endif
-
 int dm_init(bool of_live)
 {
int ret;
@@ -141,11 +138,11 @@ int dm_init(bool of_live)
}
INIT_LIST_HEAD(&DM_UCLASS_ROOT_NON_CONST);
 
-#if defined(CONFIG_NEEDS_MANUAL_RELOC)
-   fix_drivers();
-   fix_uclass();
-   fix_devices();
-#endif
+   if (IS_ENABLED(CONFIG_NEEDS_MANUAL_RELOC)) {
+   fix_drivers();
+   fix_uclass();
+   fix_devices();
+   }
 
ret = device_bind_by_name(NULL, false, &root_info, &DM_ROOT_NON_CONST);
if (ret)
@@ -350,9 +347,8 @@ int dm_init_and_scan(bool pre_reloc_only)
 {
int ret;
 
-#if CONFIG_IS_ENABLED(OF_PLATDATA)
-   dm_populate_phandle_data();
-#endif
+   if (CONFIG_IS_ENABLED(OF_PLATDATA))
+   dm_populate_phandle_data();
 
ret = dm_init(CONFIG_IS_ENABLED(OF_LIVE));
if (ret) {
-- 
2.28.0.806.g8561365e88-goog

[PATCH 12/20] sandbox: Fix up building for of-platdata

[Simon Glass]

There is no devicetree with of-platdata. Update a few uclasses to allow
them to be built for sandbox_spl. Also drop the i2c-gpio from SPL to avoid
build errors, since it does not support of-platdata.

Signed-off-by: Simon Glass 
---

 drivers/i2c/Makefile  | 2 +-
 drivers/i2c/i2c-emul-uclass.c | 2 ++
 drivers/rtc/rtc-uclass.c  | 2 ++
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index bd248cbf52b..b37198036c0 100644
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -6,7 +6,7 @@ obj-$(CONFIG_DM_I2C) += i2c-uclass.o
 ifdef CONFIG_ACPIGEN
 obj-$(CONFIG_DM_I2C) += acpi_i2c.o
 endif
-obj-$(CONFIG_DM_I2C_GPIO) += i2c-gpio.o
+obj-$(CONFIG_$(SPL_)DM_I2C_GPIO) += i2c-gpio.o
 obj-$(CONFIG_$(SPL_)I2C_CROS_EC_TUNNEL) += cros_ec_tunnel.o
 obj-$(CONFIG_$(SPL_)I2C_CROS_EC_LDO) += cros_ec_ldo.o
 
diff --git a/drivers/i2c/i2c-emul-uclass.c b/drivers/i2c/i2c-emul-uclass.c
index 1b70e140545..84b6a219d19 100644
--- a/drivers/i2c/i2c-emul-uclass.c
+++ b/drivers/i2c/i2c-emul-uclass.c
@@ -76,7 +76,9 @@ UCLASS_DRIVER(i2c_emul) = {
 UCLASS_DRIVER(i2c_emul_parent) = {
.id = UCLASS_I2C_EMUL_PARENT,
.name   = "i2c_emul_parent",
+#if !CONFIG_IS_ENABLED(OF_PLATDATA)
.post_bind  = dm_scan_fdt_dev,
+#endif
 };
 
 static const struct udevice_id i2c_emul_parent_ids[] = {
diff --git a/drivers/rtc/rtc-uclass.c b/drivers/rtc/rtc-uclass.c
index 8035f7fe9cc..b406bab32d1 100644
--- a/drivers/rtc/rtc-uclass.c
+++ b/drivers/rtc/rtc-uclass.c
@@ -174,5 +174,7 @@ int rtc_write32(struct udevice *dev, unsigned int reg, u32 
value)
 UCLASS_DRIVER(rtc) = {
.name   = "rtc",
.id = UCLASS_RTC,
+#if !CONFIG_IS_ENABLED(OF_PLATDATA)
.post_bind  = dm_scan_fdt_dev,
+#endif
 };
-- 
2.28.0.806.g8561365e88-goog

[PATCH 13/20] dm: Support parent devices with of-platdata

[Simon Glass]

At present of-platdata does not provide parent information. But this is
useful for I2C devices, for example, since it allows them to determine
which bus they are on.

Add support for setting the parent correctly, by storing the parent
driver_info index in dtoc and reading this in lists_bind_drivers(). This
needs multiple passes since we must process children after their parents
already have been bound.

Signed-off-by: Simon Glass 
---

 drivers/core/lists.c   | 54 --
 dts/Kconfig| 18 +
 include/dm/platdata.h  | 10 ++-
 tools/dtoc/dtb_platdata.py |  4 +++
 tools/dtoc/test_dtoc.py| 33 +++
 5 files changed, 116 insertions(+), 3 deletions(-)

diff --git a/drivers/core/lists.c b/drivers/core/lists.c
index 2e6bd5006ce..b23ee3030e5 100644
--- a/drivers/core/lists.c
+++ b/drivers/core/lists.c
@@ -51,21 +51,48 @@ struct uclass_driver *lists_uclass_lookup(enum uclass_id id)
return NULL;
 }
 
-int lists_bind_drivers(struct udevice *parent, bool pre_reloc_only)
+static int bind_drivers_pass(struct udevice *parent, bool pre_reloc_only)
 {
struct driver_info *info =
ll_entry_start(struct driver_info, driver_info);
const int n_ents = ll_entry_count(struct driver_info, driver_info);
+   bool missing_parent = false;
int result = 0;
uint idx;
 
+   /*
+* Do one iteration through the driver_info records. For of-platdata,
+* bind only devices whose parent is already bound. If we find any
+* device we can't bind, set missing_parent to true, which will cause
+* this function to be called again.
+*/
for (idx = 0; idx < n_ents; idx++) {
+   struct udevice *par = parent;
const struct driver_info *entry = info + idx;
struct driver_rt *drt = gd_dm_driver_rt() + idx;
struct udevice *dev;
int ret;
 
-   ret = device_bind_by_name(parent, pre_reloc_only, entry, &dev);
+   if (CONFIG_IS_ENABLED(OF_PLATDATA)) {
+   int parent_idx = driver_info_parent_id(entry);
+
+   if (drt->dev)
+   continue;
+
+   if (CONFIG_IS_ENABLED(OF_PLATDATA_PARENT) &&
+   parent_idx != -1) {
+   struct driver_rt *parent_drt;
+
+   parent_drt = gd_dm_driver_rt() + parent_idx;
+   if (!parent_drt->dev) {
+   missing_parent = true;
+   continue;
+   }
+
+   par = parent_drt->dev;
+   }
+   }
+   ret = device_bind_by_name(par, pre_reloc_only, entry, &dev);
if (!ret) {
if (CONFIG_IS_ENABLED(OF_PLATDATA))
drt->dev = dev;
@@ -76,6 +103,29 @@ int lists_bind_drivers(struct udevice *parent, bool 
pre_reloc_only)
}
}
 
+   return result ? result : missing_parent ? -EAGAIN : 0;
+}
+
+int lists_bind_drivers(struct udevice *parent, bool pre_reloc_only)
+{
+   int result = 0;
+   int pass;
+
+   /*
+* 10 passes is 10 levels deep in the devicetree, which is plenty. If
+* OF_PLATDATA_PARENT is not enabled, then bind_drivers_pass() will
+* always succeed on the first pass.
+*/
+   for (pass = 0; pass < 10; pass++) {
+   int ret;
+
+   ret = bind_drivers_pass(parent, pre_reloc_only);
+   if (!ret)
+   break;
+   if (ret != -EAGAIN && !result)
+   result = ret;
+   }
+
return result;
 }
 
diff --git a/dts/Kconfig b/dts/Kconfig
index 046a54a1736..616f8484280 100644
--- a/dts/Kconfig
+++ b/dts/Kconfig
@@ -355,6 +355,15 @@ config SPL_OF_PLATDATA
  compatible string, then adding platform data and U_BOOT_DEVICE
  declarations for each node. See of-plat.txt for more information.
 
+config SPL_OF_PLATDATA_PARENT
+   bool "Support parent information in devices"
+   depends on SPL_OF_PLATDATA
+   default y
+   help
+ Generally it is useful to be able to access the parent of a device
+ with of-platdata. To save space this can be disabled, but in that
+ case dev_get_parent() will always return NULL;
+
 config TPL_OF_PLATDATA
bool "Generate platform data for use in TPL"
depends on TPL_OF_CONTROL
@@ -376,6 +385,15 @@ config TPL_OF_PLATDATA
  compatible string, then adding platform data and U_BOOT_DEVICE
  declarations for each node. See of-plat.txt for more information.
 
+config TPL_OF_PLATDATA_PARENT
+   bool "Support parent information in devices"
+   depends on TPL_OF_PLATDATA
+   d

[PATCH 14/20] dm: Add a test for of-platdata parent information

[Simon Glass]

Add a simple test that we can obtain the correct parent for an I2C
device. This requires updating the driver names to match the compatible
strings, adding them to the devicetree and enabling a few options.

Signed-off-by: Simon Glass 
---

 arch/sandbox/dts/sandbox.dts  |  1 +
 arch/sandbox/dts/sandbox.dtsi |  1 +
 configs/sandbox_spl_defconfig |  4 +++-
 drivers/i2c/sandbox_i2c.c |  4 ++--
 drivers/rtc/sandbox_rtc.c |  4 ++--
 test/dm/of_platdata.c | 15 +++
 6 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/arch/sandbox/dts/sandbox.dts b/arch/sandbox/dts/sandbox.dts
index 20f68938297..8b50a402898 100644
--- a/arch/sandbox/dts/sandbox.dts
+++ b/arch/sandbox/dts/sandbox.dts
@@ -69,6 +69,7 @@
clock-frequency = <40>;
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_i2c0>;
+   u-boot,dm-pre-reloc;
};
 
pcic: pci@0 {
diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi
index ca7049fc974..2ec43b009bc 100644
--- a/arch/sandbox/dts/sandbox.dtsi
+++ b/arch/sandbox/dts/sandbox.dtsi
@@ -90,6 +90,7 @@
reg = <0x43>;
compatible = "sandbox-rtc";
sandbox,emul = <&emul0>;
+   u-boot,dm-pre-reloc;
};
sandbox_pmic: sandbox_pmic {
reg = <0x40>;
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
index 91215d23a0f..0cf8cf76577 100644
--- a/configs/sandbox_spl_defconfig
+++ b/configs/sandbox_spl_defconfig
@@ -26,6 +26,8 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_HANDOFF=y
 CONFIG_SPL_BOARD_INIT=y
 CONFIG_SPL_ENV_SUPPORT=y
+CONFIG_SPL_I2C_SUPPORT=y
+CONFIG_SPL_RTC_SUPPORT=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
 CONFIG_CMD_BOOTZ=y
@@ -120,7 +122,6 @@ CONFIG_I2C_CROS_EC_LDO=y
 CONFIG_DM_I2C_GPIO=y
 CONFIG_SYS_I2C_SANDBOX=y
 CONFIG_I2C_MUX=y
-CONFIG_SPL_I2C_MUX=y
 CONFIG_I2C_ARB_GPIO_CHALLENGE=y
 CONFIG_CROS_EC_KEYB=y
 CONFIG_I8042_KEYB=y
@@ -187,6 +188,7 @@ CONFIG_REMOTEPROC_SANDBOX=y
 CONFIG_DM_RESET=y
 CONFIG_SANDBOX_RESET=y
 CONFIG_DM_RTC=y
+CONFIG_SPL_DM_RTC=y
 CONFIG_SANDBOX_SERIAL=y
 CONFIG_SOUND=y
 CONFIG_SOUND_SANDBOX=y
diff --git a/drivers/i2c/sandbox_i2c.c b/drivers/i2c/sandbox_i2c.c
index 57b1c60fde6..2cbdaf9cc73 100644
--- a/drivers/i2c/sandbox_i2c.c
+++ b/drivers/i2c/sandbox_i2c.c
@@ -93,8 +93,8 @@ static const struct udevice_id sandbox_i2c_ids[] = {
{ }
 };
 
-U_BOOT_DRIVER(i2c_sandbox) = {
-   .name   = "i2c_sandbox",
+U_BOOT_DRIVER(sandbox_i2c) = {
+   .name   = "sandbox_i2c",
.id = UCLASS_I2C,
.of_match = sandbox_i2c_ids,
.ops= &sandbox_i2c_ops,
diff --git a/drivers/rtc/sandbox_rtc.c b/drivers/rtc/sandbox_rtc.c
index 852770a49cf..d0864b1df97 100644
--- a/drivers/rtc/sandbox_rtc.c
+++ b/drivers/rtc/sandbox_rtc.c
@@ -92,8 +92,8 @@ static const struct udevice_id sandbox_rtc_ids[] = {
{ }
 };
 
-U_BOOT_DRIVER(rtc_sandbox) = {
-   .name   = "rtc-sandbox",
+U_BOOT_DRIVER(sandbox_rtc) = {
+   .name   = "sandbox_rtc",
.id = UCLASS_RTC,
.of_match = sandbox_rtc_ids,
.ops= &sandbox_rtc_ops,
diff --git a/test/dm/of_platdata.c b/test/dm/of_platdata.c
index e827d45ffb7..bad733fbee0 100644
--- a/test/dm/of_platdata.c
+++ b/test/dm/of_platdata.c
@@ -205,3 +205,18 @@ static int dm_test_of_platdata_phandle(struct 
unit_test_state *uts)
return 0;
 }
 DM_TEST(dm_test_of_platdata_phandle, UT_TESTF_SCAN_PDATA);
+
+#if CONFIG_IS_ENABLED(OF_PLATDATA_PARENT)
+/* Test that device parents are correctly set up */
+static int dm_test_of_platdata_parent(struct unit_test_state *uts)
+{
+   struct udevice *rtc, *i2c;
+
+   ut_assertok(uclass_first_device_err(UCLASS_RTC, &rtc));
+   ut_assertok(uclass_first_device_err(UCLASS_I2C, &i2c));
+   ut_asserteq_ptr(i2c, dev_get_parent(rtc));
+
+   return 0;
+}
+DM_TEST(dm_test_of_platdata_parent, UT_TESTF_SCAN_PDATA);
+#endif
-- 
2.28.0.806.g8561365e88-goog

[PATCH 20/20] dm: doc: Update the of-platadata documentation

[Simon Glass]

Update this to incorporate the parent feature as well as other recent
developments.

Signed-off-by: Simon Glass 
---

 doc/driver-model/of-plat.rst | 42 
 1 file changed, 19 insertions(+), 23 deletions(-)

diff --git a/doc/driver-model/of-plat.rst b/doc/driver-model/of-plat.rst
index 1e3fad137be..58481665cec 100644
--- a/doc/driver-model/of-plat.rst
+++ b/doc/driver-model/of-plat.rst
@@ -66,12 +66,6 @@ strictly necessary. Notable problems include:
  normally also supports device tree it must use #ifdef to separate
  out this code, since the structures are only available in SPL.
 
-   - Correct relations between nodes are not implemented. This means that
- parent/child relations (like bus device iteration) do not work yet.
- Some phandles (those that are recognised as such) are converted into
- a pointer to struct driver_info. This pointer can be used to access
- the referenced device.
-
 
 How it works
 
@@ -134,10 +128,14 @@ the following C struct declaration:
 fdt32_t vmmc_supply;
 };
 
-and the following device declaration:
+and the following device declarations:
 
 .. code-block:: c
 
+/* Node /clock-controller@ff76 index 0 */
+...
+
+/* Node /dwmmc@ff0c index 2 */
 static struct dtd_rockchip_rk3288_dw_mshc dtv_dwmmc_at_ff0c = {
 .fifo_depth = 0x100,
 .cap_sd_highspeed   = true,
@@ -145,10 +143,10 @@ and the following device declaration:
 .clock_freq_min_max = {0x61a80, 0x8f0d180},
 .vmmc_supply= 0xb,
 .num_slots  = 0x1,
-.clocks = {{NULL, 456},
-   {NULL, 68},
-   {NULL, 114},
-   {NULL, 118}},
+.clocks = {{0, 456},
+   {0, 68},
+   {0, 114},
+   {0, 118}},
 .cap_mmc_highspeed  = true,
 .disable_wp = true,
 .bus_width  = 0x4,
@@ -161,13 +159,10 @@ and the following device declaration:
 .name   = "rockchip_rk3288_dw_mshc",
 .platdata   = &dtv_dwmmc_at_ff0c,
 .platdata_size  = sizeof(dtv_dwmmc_at_ff0c),
+.parent_idx = -1,
 };
 
 void dm_populate_phandle_data(void) {
-dtv_dwmmc_at_ff0c.clocks[0].node = 
DM_GET_DEVICE(clock_controller_at_ff76);
-dtv_dwmmc_at_ff0c.clocks[1].node = 
DM_GET_DEVICE(clock_controller_at_ff76);
-dtv_dwmmc_at_ff0c.clocks[2].node = 
DM_GET_DEVICE(clock_controller_at_ff76);
-dtv_dwmmc_at_ff0c.clocks[3].node = 
DM_GET_DEVICE(clock_controller_at_ff76);
 }
 
 The device is then instantiated at run-time and the platform data can be
@@ -193,6 +188,13 @@ In order to make this a bit more flexible 
U_BOOT_DRIVER_ALIAS macro can be
 used to declare an alias for a driver name, typically a 'compatible' string.
 This macro produces no code, but it is by dtoc tool.
 
+The parent_idx is the index of the parent driver_info structure within its
+linker list (instantiated by the U_BOOT_DEVICE() macro). This is used to 
support
+dev_get_parent(). The dm_populate_phandle_data() is included to allow for
+fix-ups required by dtoc. It is not currently used. The values in 'clocks' are
+the index of the driver_info for the target device followed by any phandle
+arguments. This is used to support device_get_by_driver_info_idx().
+
 During the build process dtoc parses both U_BOOT_DRIVER and U_BOOT_DRIVER_ALIAS
 to build a list of valid driver names and driver aliases. If the 'compatible'
 string used for a device does not not match a valid driver name, it will be
@@ -339,12 +341,7 @@ spl/dt-platdata.c. It additionally contains the definition 
of
 dm_populate_phandle_data() which is responsible of filling the phandle
 information by adding references to U_BOOT_DEVICE by using DM_GET_DEVICE
 
-The beginnings of a libfdt Python module are provided. So far this only
-implements a subset of the features.
-
-The 'swig' tool is needed to build the libfdt Python module. If this is not
-found then the Python model is not used and a fallback is used instead, which
-makes use of fdtget.
+The pylibfdt Python module is used to access the devicetree.
 
 
 Credits
@@ -357,11 +354,10 @@ Future work
 ---
 - Consider programmatically reading binding files instead of device tree
   contents
-- Complete the phandle feature
-- Move to using a full Python libfdt module
 
 
 .. Simon Glass 
 .. Google, Inc
 .. 6/6/16
 .. Updated Independence Day 2016
+.. Updated 1st October 2020
-- 
2.28.0.806.g8561365e88-goog

[PATCH 17/20] x86: apl: Take advantage of the of-platdata parent support

[Simon Glass]

Now that parent devices are supported with of-platadata, we don't need the
messy code to fix up the parent pointers and allocations on Apollo Lake.
Put the code behind a condition.

Signed-off-by: Simon Glass 
---

 arch/x86/cpu/apollolake/spl.c |  3 ++-
 drivers/misc/p2sb-uclass.c| 27 ++-
 2 files changed, 16 insertions(+), 14 deletions(-)

diff --git a/arch/x86/cpu/apollolake/spl.c b/arch/x86/cpu/apollolake/spl.c
index 5a53831dc6a..089b37c59f8 100644
--- a/arch/x86/cpu/apollolake/spl.c
+++ b/arch/x86/cpu/apollolake/spl.c
@@ -90,7 +90,8 @@ static int apl_flash_probe(struct udevice *dev)
  */
 static int apl_flash_bind(struct udevice *dev)
 {
-   if (CONFIG_IS_ENABLED(OF_PLATDATA)) {
+   if (CONFIG_IS_ENABLED(OF_PLATDATA) &&
+   !CONFIG_IS_ENABLED(OF_PLATDATA_PARENT)) {
struct dm_spi_slave_platdata *plat;
struct udevice *spi;
int ret;
diff --git a/drivers/misc/p2sb-uclass.c b/drivers/misc/p2sb-uclass.c
index b5219df46be..12abcff2da4 100644
--- a/drivers/misc/p2sb-uclass.c
+++ b/drivers/misc/p2sb-uclass.c
@@ -174,19 +174,20 @@ int p2sb_set_port_id(struct udevice *dev, int portid)
if (!CONFIG_IS_ENABLED(OF_PLATDATA))
return -ENOSYS;
 
-   uclass_find_first_device(UCLASS_P2SB, &ps2b);
-   if (!ps2b)
-   return -EDEADLK;
-   dev->parent = ps2b;
-
-   /*
-* We must allocate this, since when the device was bound it did not
-* have a parent.
-* TODO(s...@chromium.org): Add a parent pointer to child devices in 
dtoc
-*/
-   dev->parent_platdata = malloc(sizeof(*pplat));
-   if (!dev->parent_platdata)
-   return -ENOMEM;
+   if (!CONFIG_IS_ENABLED(OF_PLATDATA_PARENT)) {
+   uclass_find_first_device(UCLASS_P2SB, &ps2b);
+   if (!ps2b)
+   return -EDEADLK;
+   dev->parent = ps2b;
+
+   /*
+* We must allocate this, since when the device was bound it did
+* not have a parent.
+*/
+   dev->parent_platdata = malloc(sizeof(*pplat));
+   if (!dev->parent_platdata)
+   return -ENOMEM;
+   }
pplat = dev_get_parent_platdata(dev);
pplat->pid = portid;
 
-- 
2.28.0.806.g8561365e88-goog

[PATCH 16/20] x86: apl: Enable SPI flash in TPL with APL_SPI_FLASH_BOOT

[Simon Glass]

At present, enabling CONFIG_APL_SPI_FLASH_BOOT does not build since SPI
and SPI flash are not enabled for TPL. Add a condition to fix this and
tidy up a build warning in the SPI-flash driver.

Signed-off-by: Simon Glass 
---

 arch/x86/cpu/apollolake/Kconfig | 2 ++
 drivers/spi/ich.c   | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/cpu/apollolake/Kconfig b/arch/x86/cpu/apollolake/Kconfig
index 35a425cd1bc..c6c1350f4f0 100644
--- a/arch/x86/cpu/apollolake/Kconfig
+++ b/arch/x86/cpu/apollolake/Kconfig
@@ -85,6 +85,8 @@ config APL_SPI_FLASH_BOOT
bool "Support booting with SPI-flash driver instead memory-mapped SPI"
select TPL_SPI_FLASH_SUPPORT
select TPL_SPI_SUPPORT
+   select TPL_DM_SPI
+   select TPL_DM_SPI_FLASH
help
  This enables SPI and SPI flash in TPL. Without the this only
  available boot method is to use memory-mapped SPI. Since this is
diff --git a/drivers/spi/ich.c b/drivers/spi/ich.c
index e1336b89c5a..a91cb785680 100644
--- a/drivers/spi/ich.c
+++ b/drivers/spi/ich.c
@@ -615,6 +615,7 @@ static int ich_spi_exec_op(struct spi_slave *slave, const 
struct spi_mem_op *op)
return ret;
 }
 
+#if !CONFIG_IS_ENABLED(OF_PLATDATA)
 /**
  * ich_spi_get_basics() - Get basic information about the ICH device
  *
@@ -657,6 +658,7 @@ static int ich_spi_get_basics(struct udevice *bus, bool 
can_probe,
 
return ret;
 }
+#endif
 
 /**
  * ich_get_mmap_bus() - Handle the get_mmap() method for a bus
@@ -946,10 +948,10 @@ static int ich_spi_child_pre_probe(struct udevice *dev)
 static int ich_spi_ofdata_to_platdata(struct udevice *dev)
 {
struct ich_spi_platdata *plat = dev_get_platdata(dev);
-   int ret;
 
 #if !CONFIG_IS_ENABLED(OF_PLATDATA)
struct ich_spi_priv *priv = dev_get_priv(dev);
+   int ret;
 
ret = ich_spi_get_basics(dev, true, &priv->pch, &plat->ich_version,
 &plat->mmio_base);
-- 
2.28.0.806.g8561365e88-goog

[PATCH 19/20] dm: Don't allow U_BOOT_DEVICE() when of-platdata is used

[Simon Glass]

With of-platdata, the devicetree is supposed to specify all the devices
in the system. So far this hasn't really mattered since of-platdata still
works correctly.

However, new of-platdata features rely on numbering the devices in a
particular order so that they can be referenced by a single integer. It is
tricky to implement this efficiently when other devices are present in the
build.

To address this, disable use of U_BOOT_DEVICE() when of-platdata is
enabled. This seems acceptable as it is not supposed to be used at all,
except in SPL/TPL, where of-platdata is the recommended approach.

This breaks one non-compliant boards at present: mx6cuboxi

Signed-off-by: Simon Glass 
---

 include/dm/platdata.h  | 8 
 tools/dtoc/dtb_platdata.py | 3 +++
 tools/dtoc/test_dtoc.py| 3 +++
 3 files changed, 14 insertions(+)

diff --git a/include/dm/platdata.h b/include/dm/platdata.h
index f800a866dda..216efa8ef77 100644
--- a/include/dm/platdata.h
+++ b/include/dm/platdata.h
@@ -55,9 +55,17 @@ struct driver_rt {
  * NOTE: Avoid using these except in extreme circumstances, where device tree
  * is not feasible (e.g. serial driver in SPL where <8KB of SRAM is
  * available). U-Boot's driver model uses device tree for configuration.
+ *
+ * When of-platdata is in use, U_BOOT_DEVICE() cannot be used outside of the
+ * dt-platdata.c file created by dtoc
  */
+#if CONFIG_IS_ENABLED(OF_PLATDATA) && !defined(DT_PLATDATA_C)
+#define U_BOOT_DEVICE(__name)  _Static_assert(false, \
+   "Cannot use U_BOOT_DEVICE with of-platdata. Please use devicetree 
instead")
+#else
 #define U_BOOT_DEVICE(__name)  \
ll_entry_declare(struct driver_info, __name, driver_info)
+#endif
 
 /* Declare a list of devices. The argument is a driver_info[] array */
 #define U_BOOT_DEVICES(__name) \
diff --git a/tools/dtoc/dtb_platdata.py b/tools/dtoc/dtb_platdata.py
index 2be11fff6c2..9b27aecc140 100644
--- a/tools/dtoc/dtb_platdata.py
+++ b/tools/dtoc/dtb_platdata.py
@@ -673,6 +673,9 @@ class DtbPlatdata(object):
 information.
 """
 self.out_header()
+self.out('/* Allow use of U_BOOT_DEVICE() in this file */\n')
+self.out('#define DT_PLATDATA_C\n')
+self.out('\n')
 self.out('#include \n')
 self.out('#include \n')
 self.out('#include \n')
diff --git a/tools/dtoc/test_dtoc.py b/tools/dtoc/test_dtoc.py
index 8e16dc0f0fa..a5836e04b7a 100755
--- a/tools/dtoc/test_dtoc.py
+++ b/tools/dtoc/test_dtoc.py
@@ -44,6 +44,9 @@ C_HEADER = '''/*
  * This file was generated by dtoc from a .dtb (device tree binary) file.
  */
 
+/* Allow use of U_BOOT_DEVICE() in this file */
+#define DT_PLATDATA_C
+
 #include 
 #include 
 #include 
-- 
2.28.0.806.g8561365e88-goog

[PATCH 18/20] dm: Use driver_info index instead of pointer

[Simon Glass]

At present we use a 'node' pointer in the of-platadata phandle_n_arg
structs. This is a pointer to the struct driver_info for a particular
device, and we can use it to obtain the struct udevice pointer itself.

Since we don't know the struct udevice pointer until it is allocated in
memory, we have to fix up the phandle_n_arg.node at runtime. This is
annoying since it requires that SPL's data is writable and adds a small
amount of extra (generated) code in the dm_populate_phandle_data()
function.

Now that we can find a driver_info by its index, it is easier to put the
index in the phandle_n_arg structures.

Update dtoc to do this, add a new device_get_by_driver_info_idx() to look
up a device by drive_info index and update the tests to match.

Signed-off-by: Simon Glass 
---

 drivers/clk/clk-uclass.c|  3 +--
 drivers/core/device.c   | 11 +++
 drivers/misc/irq-uclass.c   |  2 +-
 drivers/mmc/fsl_esdhc_imx.c |  7 ++-
 include/dm/device.h | 14 ++
 include/dt-structs.h|  6 +++---
 test/dm/of_platdata.c   | 10 +-
 test/dm/test-main.c | 24 
 test/py/tests/test_spl.py   |  5 ++---
 tools/dtoc/dtb_platdata.py  | 20 
 tools/dtoc/test_dtoc.py | 33 +++--
 11 files changed, 74 insertions(+), 61 deletions(-)

diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
index 934cd5787a5..c56bc71e387 100644
--- a/drivers/clk/clk-uclass.c
+++ b/drivers/clk/clk-uclass.c
@@ -37,8 +37,7 @@ int clk_get_by_driver_info(struct udevice *dev, struct 
phandle_1_arg *cells,
 {
int ret;
 
-   ret = device_get_by_driver_info((struct driver_info *)cells->node,
-   &clk->dev);
+   ret = device_get_by_driver_info_idx(cells->idx, &clk->dev);
if (ret)
return ret;
clk->id = cells->arg[0];
diff --git a/drivers/core/device.c b/drivers/core/device.c
index c3338efc4fa..522d2ffe7e3 100644
--- a/drivers/core/device.c
+++ b/drivers/core/device.c
@@ -750,6 +750,17 @@ int device_get_by_driver_info(const struct driver_info 
*info,
 
return device_get_device_tail(dev, dev ? 0 : -ENOENT, devp);
 }
+
+int device_get_by_driver_info_idx(uint idx, struct udevice **devp)
+{
+   struct driver_rt *drt = gd_dm_driver_rt() + idx;
+   struct udevice *dev;
+
+   dev = drt->dev;
+   *devp = NULL;
+
+   return device_get_device_tail(dev, dev ? 0 : -ENOENT, devp);
+}
 #endif
 
 int device_find_first_child(const struct udevice *parent, struct udevice 
**devp)
diff --git a/drivers/misc/irq-uclass.c b/drivers/misc/irq-uclass.c
index 94fa233f193..24b27962a7d 100644
--- a/drivers/misc/irq-uclass.c
+++ b/drivers/misc/irq-uclass.c
@@ -69,7 +69,7 @@ int irq_get_by_driver_info(struct udevice *dev,
 {
int ret;
 
-   ret = device_get_by_driver_info(cells->node, &irq->dev);
+   ret = device_get_by_driver_info_idx(cells->idx, &irq->dev);
if (ret)
return ret;
irq->id = cells->arg[0];
diff --git a/drivers/mmc/fsl_esdhc_imx.c b/drivers/mmc/fsl_esdhc_imx.c
index 0c866b168f9..ed04d1f964c 100644
--- a/drivers/mmc/fsl_esdhc_imx.c
+++ b/drivers/mmc/fsl_esdhc_imx.c
@@ -1511,12 +1511,9 @@ static int fsl_esdhc_probe(struct udevice *dev)
 
if (CONFIG_IS_ENABLED(DM_GPIO) && !priv->non_removable) {
struct udevice *gpiodev;
-   struct driver_info *info;
-
-   info = (struct driver_info *)dtplat->cd_gpios->node;
-
-   ret = device_get_by_driver_info(info, &gpiodev);
 
+   ret = device_get_by_driver_info(dtplat->cd_gpios->idx,
+   &gpiodev);
if (ret)
return ret;
 
diff --git a/include/dm/device.h b/include/dm/device.h
index ac3b6c1b8a0..c00bee1ccb0 100644
--- a/include/dm/device.h
+++ b/include/dm/device.h
@@ -553,6 +553,20 @@ int device_get_global_by_ofnode(ofnode node, struct 
udevice **devp);
 int device_get_by_driver_info(const struct driver_info *info,
  struct udevice **devp);
 
+/**
+ * device_get_by_driver_info_idx() - Get a device based on driver_info index
+ *
+ * Locates a device by its struct driver_info, by using its index number which
+ * is written into the idx field of struct phandle_1_arg, etc.
+ *
+ * The device is probed to activate it ready for use.
+ *
+ * @idx: Index number of the driver_info structure (0=first)
+ * @devp: Returns pointer to device if found, otherwise this is set to NULL
+ * @return 0 if OK, -ve on error
+ */
+int device_get_by_driver_info_idx(uint idx, struct udevice **devp);
+
 /**
  * device_find_first_child() - Find the first child of a device
  *
diff --git a/include/dt-structs.h b/include/dt-structs.h
index eed8273d18e..f0e1c9cb901 100644
--- a/include/dt-structs.h
+++ b/include/dt-structs.h
@@ -11,17 +11,17 @@
 struct driver_info;
 
 struct phandle_0_arg {
-   co

Re: [PATCH v2 4/4] smbios: Add documentation and devicetree binding

[Heinrich Schuchardt]

On 10/3/20 5:40 PM, Simon Glass wrote:
> Hi Heinrich,
>
> On Fri, 2 Oct 2020 at 08:57, Heinrich Schuchardt  wrote:
>>
>> On 02.10.20 16:23, Simon Glass wrote:
>>> Add information about how to set SMBIOS properties using the devicetree.
>>>
>>> Signed-off-by: Simon Glass 
>>> ---
>>>
>>> (no changes since v1)
>>>
>>>  doc/arch/x86.rst |  8 +
>>>  doc/device-tree-bindings/board/board_x86.txt | 36 
>>>  2 files changed, 44 insertions(+)
>>>  create mode 100644 doc/device-tree-bindings/board/board_x86.txt
>>>
>>> diff --git a/doc/arch/x86.rst b/doc/arch/x86.rst
>>> index c6b70ce61a3..45f9ba308c7 100644
>>> --- a/doc/arch/x86.rst
>>> +++ b/doc/arch/x86.rst
>>> @@ -740,6 +740,14 @@ Note that this is a development feature only. It is 
>>> not intended for use in
>>>  production environments. Also it is not currently part of the automated 
>>> tests
>>>  so may break in the future.
>>>
>>> +SMBIOS tables
>>> +-
>>> +
>>> +To generate SMBIOS tables in U-Boot, for use by the OS, enable the
>>> +CONFIG_GENERATE_SMBIOS_TABLE option. The easiest way to provide the values 
>>> to
>>> +use is via the device tree. For details see
>>> +device-tree-bindings/board/board_x86.txt
>>> +
>>>  TODO List
>>>  -
>>>  - Audio
>>> diff --git a/doc/device-tree-bindings/board/board_x86.txt 
>>> b/doc/device-tree-bindings/board/board_x86.txt
>>> new file mode 100644
>>> index 000..3766751896a
>>> --- /dev/null
>>> +++ b/doc/device-tree-bindings/board/board_x86.txt
>>> @@ -0,0 +1,36 @@
>>> +x86 board driver
>>> +
>>> +
>>> +This driver allows providing board-specific features such as power control
>>> +GPIOs. In addition, the SMBIOS values can be specified in the device tree,
>>> +as below:
>>> +
>>> +An optional 'smbios' subnode can be used to provide these properties.
>>> +
>>> +Optional properties:
>>> +  - manufacturer: Product manufacturer for system / baseboard
>>> +  - product:  Product name
>>> +  - version:  Product version string
>>> +  - serial:   Serial number for system (note that this can be 
>>> overridden by
>>> +  the serial# environment variable)
>>> +  - sku:  Product SKU (Stock-Keeping Unit)
>>> +  - family:   Product family
>>> +  - asset-tag:Asset tag for the motherboard, sometimes used in 
>>> organisations
>>> +  to track devices
>>> +
>>> +
>>> +Example:
>>> +
>>> + board {
>>> + compatible = "google,coral";
>>> +
>>> + smbios {
>>
>> I think board is the wrong place for the new node.
>>
>> SMBIOS also includes chassis, processor and other information. We may
>> opt to provide non-board information in future via the device-tree.
>
> I think of board as the whole thing. Are you thinking of it just being
> the circuit board?

The whole I would call a system.

dmidecode shows bios, system, board, chassis, processor, boot. So board
seems to be only a small aspect of the SMBIOS system description.

>> If this is for U-Boot internal only usage, we should indicate this in
>> the label name.
>
> All drivers are for U-Boot-internal use, right? Or are you saying you
> don't want this information to appear in the upstream bindings?

I don't won't name collisions of U-Boot with Linux when using U-Boot's
device tree for booting an operating system on ARM and RISC-V systems.

>> So it could be like:
>>
>> /u-boot-smbios {
>
> compatible string?

Yes there should be one. So you know that this is the node you are
looking for.

compatible = "u-boot,smbios";

>
>>bios {};
>>system {};
>>board {
>>   manufacturer = "Hardkernel Co., Ltd.";
>>   product = "ODROID-C2";
>>   ...
>>};
>>chassis {};
>>processor {};
>>boot {};
>> };
>>
>>> + manufacturer = "Google";
>>> + product = "Coral";
>>> + version = "rev2";
>>> + serial = "123456789";
>>> + sku = "sku3";
>>> + family = "Google_Coral";
>>> + asset-tag = "ABC123";
>>> + };
>>> + };
>>>
>>
>> SMBIOS is not x86 specific. On ARM we are also passing an SMBIOS table.
>>
>> On my Odroid C2 dmidecode shows the information provided by U-Boot:
>
> Oh dear. Legacy never dies. It never occurred to me that this would be
> used on ARM.

Don't forget about RISC-V :)

https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.3.0.pdf:

"The following processor architectures are now supported:

* IA-32 (x86),386
* x64 (x86-64, Intel64, AMD64, EM64T),
* Intel Itanium architecture,
* 32-bit ARM (Aarch32),
* 64-bit ARM (Aarch64),
* RISC-V 32 (RV32),
* RISC-V 64 (RV64),
* RISC-V 128 (RV128)"

If you think of data centers, it may be valuable to be able to use the
same management tools for amd64, aarch64 and rv64.

Best regards

Heinrich

>
>>
>> 
>> Handle 0x0001, DMI type 1, 27 bytes
>> System Informati

Re: [PATCH] clk: renesas: r8a774a1-cpg-mssr: Add R8A774A1 RPC clock

[Marek Vasut]

On 9/29/20 12:09 PM, Biju Das wrote:
> Add RPC entry into the R8A774A1 clock driver tables.
> 
> Signed-off-by: Biju Das 

Applied to sh/next, thanks

Re: [PATCH v4 1/2] arm: rmobile: Add PRR CPU ID macros for RZ/G2[HMNE]

[Marek Vasut]

On 10/1/20 12:36 PM, Biju Das wrote:
[...]

> diff --git a/arch/arm/mach-rmobile/include/mach/rmobile.h 
> b/arch/arm/mach-rmobile/include/mach/rmobile.h
> index a50249dc96..bd4bd01b75 100644
> --- a/arch/arm/mach-rmobile/include/mach/rmobile.h
> +++ b/arch/arm/mach-rmobile/include/mach/rmobile.h
> @@ -27,6 +27,10 @@
>  /* PRR CPU IDs */
>  #define RMOBILE_CPU_TYPE_SH73A0  0x37
>  #define RMOBILE_CPU_TYPE_R8A7740 0x40
> +#define RMOBILE_CPU_TYPE_R8A774A10x52

The problem here is that this is the same as
#define RMOBILE_CPU_TYPE_R8A7796   0x52

So if you use that ^ in the code, you cannot discern it from
RMOBILE_CPU_TYPE_R8A774A1 .

We really need to find a way to tell these two apart first, e.g. by
checking the bootrom, and then use it in U-Boot all over the place.

Re: [PATCH v2] spi: renesas_rpc_spi: Add R-Car Gen3 and RZ/G2 fallback compatibility string

[Marek Vasut]

On 9/30/20 2:19 PM, Biju Das wrote:
> Add fallback compatibility string for R-Car Gen3 and RZ/G2.
> 
> Also sorted the compatible string as per SoC ID.
> 
> Signed-off-by: Biju Das 

Applied to sh/next, thanks

RE: [PATCH v4 1/2] arm: rmobile: Add PRR CPU ID macros for RZ/G2[HMNE]

[Biju Das]

Hi Marek,

Thanks for the feedback.

> Subject: Re: [PATCH v4 1/2] arm: rmobile: Add PRR CPU ID macros for
> RZ/G2[HMNE]
>
> On 10/1/20 12:36 PM, Biju Das wrote:
> [...]
>
> > diff --git a/arch/arm/mach-rmobile/include/mach/rmobile.h
> > b/arch/arm/mach-rmobile/include/mach/rmobile.h
> > index a50249dc96..bd4bd01b75 100644
> > --- a/arch/arm/mach-rmobile/include/mach/rmobile.h
> > +++ b/arch/arm/mach-rmobile/include/mach/rmobile.h
> > @@ -27,6 +27,10 @@
> >  /* PRR CPU IDs */
> >  #define RMOBILE_CPU_TYPE_SH73A00x37
> >  #define RMOBILE_CPU_TYPE_R8A77400x40
> > +#define RMOBILE_CPU_TYPE_R8A774A10x52
>
> The problem here is that this is the same as
> #define RMOBILE_CPU_TYPE_R8A7796   0x52
>
> So if you use that ^ in the code, you cannot discern it from
> RMOBILE_CPU_TYPE_R8A774A1 .

But this value is same as per the hardware manual, see the definitions in 
linux[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/soc/renesas/renesas-soc.c?h=v5.9-rc7#n114
Please correct me, if this macros are not as per hardware manual.

> We really need to find a way to tell these two apart first, e.g. by checking 
> the
> bootrom, and then use it in U-Boot all over the place.

Yes, I agree we need to differentiate them. Different ways I can think of is

1) Bootrom level
2) Checking some IP register which is present in RCar and not in RZ/G2
3) Compatible SoC string from TFA
4) TFA there will be separation for RZ/G2 SoC and RCar Gen3 SoC, So populate 
some dtfragment for RCar/RZG2 SoC similar to dram and u-boot handling it for 
SoC separation.
5) Use compile time macros in U-boot.

Please let me you know, which is the best way to go forward and also let me 
know, if you have any other ideas in your mind.

Regards,
Biju





Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, 
Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 
Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 
3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. 
no.: DE 14978647

RE: [PATCH v2] spi: renesas_rpc_spi: Add R-Car Gen3 and RZ/G2 fallback compatibility string

[Biju Das]

Hi Marek,

Thank you

Cheers,
Biju

> Subject: Re: [PATCH v2] spi: renesas_rpc_spi: Add R-Car Gen3 and RZ/G2
> fallback compatibility string
>
> On 9/30/20 2:19 PM, Biju Das wrote:
> > Add fallback compatibility string for R-Car Gen3 and RZ/G2.
> >
> > Also sorted the compatible string as per SoC ID.
> >
> > Signed-off-by: Biju Das 
>
> Applied to sh/next, thanks


Renesas Electronics Europe GmbH, Geschaeftsfuehrer/President: Carsten Jauch, 
Sitz der Gesellschaft/Registered office: Duesseldorf, Arcadiastrasse 10, 40472 
Duesseldorf, Germany, Handelsregister/Commercial Register: Duesseldorf, HRB 
3708 USt-IDNr./Tax identification no.: DE 119353406 WEEE-Reg.-Nr./WEEE reg. 
no.: DE 14978647

Re: [PATCH v2 4/4] smbios: Add documentation and devicetree binding

[Simon Glass]

Hi Heinrich,

On Sat, 3 Oct 2020 at 11:54, Heinrich Schuchardt  wrote:
>
> On 10/3/20 5:40 PM, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Fri, 2 Oct 2020 at 08:57, Heinrich Schuchardt  wrote:
> >>
> >> On 02.10.20 16:23, Simon Glass wrote:
> >>> Add information about how to set SMBIOS properties using the devicetree.
> >>>
> >>> Signed-off-by: Simon Glass 
> >>> ---
> >>>
> >>> (no changes since v1)
> >>>
> >>>  doc/arch/x86.rst |  8 +
> >>>  doc/device-tree-bindings/board/board_x86.txt | 36 
> >>>  2 files changed, 44 insertions(+)
> >>>  create mode 100644 doc/device-tree-bindings/board/board_x86.txt
> >>>
> >>> diff --git a/doc/arch/x86.rst b/doc/arch/x86.rst
> >>> index c6b70ce61a3..45f9ba308c7 100644
> >>> --- a/doc/arch/x86.rst
> >>> +++ b/doc/arch/x86.rst
> >>> @@ -740,6 +740,14 @@ Note that this is a development feature only. It is 
> >>> not intended for use in
> >>>  production environments. Also it is not currently part of the automated 
> >>> tests
> >>>  so may break in the future.
> >>>
> >>> +SMBIOS tables
> >>> +-
> >>> +
> >>> +To generate SMBIOS tables in U-Boot, for use by the OS, enable the
> >>> +CONFIG_GENERATE_SMBIOS_TABLE option. The easiest way to provide the 
> >>> values to
> >>> +use is via the device tree. For details see
> >>> +device-tree-bindings/board/board_x86.txt
> >>> +
> >>>  TODO List
> >>>  -
> >>>  - Audio
> >>> diff --git a/doc/device-tree-bindings/board/board_x86.txt 
> >>> b/doc/device-tree-bindings/board/board_x86.txt
> >>> new file mode 100644
> >>> index 000..3766751896a
> >>> --- /dev/null
> >>> +++ b/doc/device-tree-bindings/board/board_x86.txt
> >>> @@ -0,0 +1,36 @@
> >>> +x86 board driver
> >>> +
> >>> +
> >>> +This driver allows providing board-specific features such as power 
> >>> control
> >>> +GPIOs. In addition, the SMBIOS values can be specified in the device 
> >>> tree,
> >>> +as below:
> >>> +
> >>> +An optional 'smbios' subnode can be used to provide these properties.
> >>> +
> >>> +Optional properties:
> >>> +  - manufacturer: Product manufacturer for system / baseboard
> >>> +  - product:  Product name
> >>> +  - version:  Product version string
> >>> +  - serial:   Serial number for system (note that this can be 
> >>> overridden by
> >>> +  the serial# environment variable)
> >>> +  - sku:  Product SKU (Stock-Keeping Unit)
> >>> +  - family:   Product family
> >>> +  - asset-tag:Asset tag for the motherboard, sometimes used in 
> >>> organisations
> >>> +  to track devices
> >>> +
> >>> +
> >>> +Example:
> >>> +
> >>> + board {
> >>> + compatible = "google,coral";
> >>> +
> >>> + smbios {
> >>
> >> I think board is the wrong place for the new node.
> >>
> >> SMBIOS also includes chassis, processor and other information. We may
> >> opt to provide non-board information in future via the device-tree.
> >
> > I think of board as the whole thing. Are you thinking of it just being
> > the circuit board?
>
> The whole I would call a system.
>
> dmidecode shows bios, system, board, chassis, processor, boot. So board
> seems to be only a small aspect of the SMBIOS system description.

Well that is using smbios terminology. For U-Boot there is only the
board at present.

We could rename the board uclass to system, I suppose. But at least
for now, the terminology within U-Boot is to use the word 'board' to
describe a build target.

>
> >> If this is for U-Boot internal only usage, we should indicate this in
> >> the label name.
> >
> > All drivers are for U-Boot-internal use, right? Or are you saying you
> > don't want this information to appear in the upstream bindings?
>
> I don't won't name collisions of U-Boot with Linux when using U-Boot's
> device tree for booting an operating system on ARM and RISC-V systems.
>
> >> So it could be like:
> >>
> >> /u-boot-smbios {
> >
> > compatible string?
>
> Yes there should be one. So you know that this is the node you are
> looking for.
>
> compatible = "u-boot,smbios";

OK good.

>
> >
> >>bios {};
> >>system {};
> >>board {
> >>   manufacturer = "Hardkernel Co., Ltd.";
> >>   product = "ODROID-C2";
> >>   ...
> >>};
> >>chassis {};
> >>processor {};
> >>boot {};

I think this makes sense because here we are describing smbios tables
so we should use the same terminology.

> >> };
> >>
> >>> + manufacturer = "Google";
> >>> + product = "Coral";
> >>> + version = "rev2";
> >>> + serial = "123456789";
> >>> + sku = "sku3";
> >>> + family = "Google_Coral";
> >>> + asset-tag = "ABC123";
> >>> + };
> >>> + };
> >>>
> >>
> >> SMBIOS is not x86 specific. On ARM we are also passing an SMBIOS table.
> >>
> >> On my Odroid C2 dmi

Re: [PATCH 1/4] efi_loader: description EFI_LOAD_FILE2_PROTOCOL

[Ilias Apalodimas]

On Sat, Oct 03, 2020 at 01:57:13PM +0200, Heinrich Schuchardt wrote:
> U-Boot offers a EFI_LOAD_FILE2_PROTOCOL which the Linux EFI stub can use to
> load an initial RAM disk. Update the function comments of the
> implementation.
> 
> Signed-off-by: Heinrich Schuchardt 
> ---
>  lib/efi_loader/efi_load_initrd.c | 42 +---
>  1 file changed, 23 insertions(+), 19 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_load_initrd.c 
> b/lib/efi_loader/efi_load_initrd.c
> index 574a83d7e3..ff69e6eb79 100644
> --- a/lib/efi_loader/efi_load_initrd.c
> +++ b/lib/efi_loader/efi_load_initrd.c
> @@ -47,9 +47,9 @@ static const struct efi_initrd_dp dp = {
>  /**
>   * get_file_size() - retrieve the size of initramfs, set efi status on error
>   *
> - * @dev: device to read from. i.e "mmc"
> - * @part:device partition. i.e "0:1"
> - * @file:name fo file
> + * @dev: device to read from, e.g. "mmc"
> + * @part:device partition, e.g. "0:1"
> + * @file:name of file
>   * @status:  EFI exit code in case of failure
>   *
>   * Return:   size of file
> @@ -78,15 +78,16 @@ out:
>  }
> 
>  /**
> - * load_file2() - get information about random number generation
> + * efi_load_file2initrd() - load initial RAM disk
> + *
> + * This function implements the LoadFile service of the 
> EFI_LOAD_FILE2_PROTOCOL
> + * in order to load an initial RAM disk requested by the Linux kernel stub.
>   *
> - * This function implement the LoadFile2() service in order to load an 
> initram
> - * disk requested by the Linux kernel stub.
>   * See the UEFI spec for details.
>   *
> - * @this:loadfile2 protocol instance
> - * @file_path:   relative path of the file. "" in this 
> case
> - * @boot_policy: must be false for Loadfile2
> + * @this:EFI_LOAD_FILE2_PROTOCOL instance
> + * @file_path:   media device path of the file, "" in 
> this case
> + * @boot_policy: must be false
>   * @buffer_size: size of allocated buffer
>   * @buffer:  buffer to load the file
>   *
> @@ -128,7 +129,13 @@ efi_load_file2_initrd(struct efi_load_file_protocol 
> *this,
>   goto out;
>   }
> 
> - /* expect something like 'mmc 0:1 initrd.cpio.gz' */
> + /*
> +  * expect a string with three space separated parts:
> +  *
> +  * * a block device type, e.g. "mmc"
> +  * * a device and partition identifier, e.g. "0:1"
> +  * * a file path on the block device, e.g. "/boot/initrd.cpio.gz"
> +  */
>   dev = strsep(&s, " ");
>   if (!dev)
>   goto out;
> @@ -168,23 +175,20 @@ out:
>  }
> 
>  /**
> - * efi_initrd_register() - Register a handle and loadfile2 protocol
> + * efi_initrd_register() - create handle for loading initial RAM disk
>   *
> - * This function creates a new handle and installs a linux specific GUID
> - * to handle initram disk loading during boot.
> - * See the UEFI spec for details.
> + * This function creates a new handle and installs a Linux specific vendor
> + * device path and an EFI_LOAD_FILE_2_PROTOCOL. Linux uses the device path
> + * to identify the handle and then calls the LoadFile service of the
> + * EFI_LOAD_FILE_2_PROTOCOL to read the initial RAM disk.
>   *
> - * Return:   status code
> + * Return:   status code
>   */
>  efi_status_t efi_initrd_register(void)
>  {
>   efi_handle_t efi_initrd_handle = NULL;
>   efi_status_t ret;
> 
> - /*
> -  * Set up the handle with the EFI_LOAD_FILE2_PROTOCOL which Linux may
> -  * use to load the initial ramdisk.
> -  */
>   ret = EFI_CALL(efi_install_multiple_protocol_interfaces
>  (&efi_initrd_handle,
>   /* initramfs */
> --
> 2.28.0
> 

Reviewed-by: Ilias Apalodimas 

Re: [PATCH 2/4] efi_loader: illegal free in EFI_LOAD_FILE2_PROTOCOL

[Ilias Apalodimas]

On Sat, Oct 03, 2020 at 01:57:14PM +0200, Heinrich Schuchardt wrote:
> strsep() changes the address that its first argument points to.
> We cannot use the changed address as argument of free().
> 
> Signed-off-by: Heinrich Schuchardt 
> ---
>  lib/efi_loader/efi_load_initrd.c | 17 +
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/efi_loader/efi_load_initrd.c 
> b/lib/efi_loader/efi_load_initrd.c
> index ff69e6eb79..d517d686c3 100644
> --- a/lib/efi_loader/efi_load_initrd.c
> +++ b/lib/efi_loader/efi_load_initrd.c
> @@ -98,19 +98,20 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this,
> struct efi_device_path *file_path, bool boot_policy,
> efi_uintn_t *buffer_size, void *buffer)
>  {
> - const char *filespec = CONFIG_EFI_INITRD_FILESPEC;
> + char *filespec;
>   efi_status_t status = EFI_NOT_FOUND;
>   loff_t file_sz = 0, read_sz = 0;
>   char *dev, *part, *file;
> - char *s;
> + char *pos;
>   int ret;
> 
>   EFI_ENTRY("%p, %p, %d, %p, %p", this, file_path, boot_policy,
> buffer_size, buffer);
> 
> - s = strdup(filespec);
> - if (!s)
> + filespec = strdup(CONFIG_EFI_INITRD_FILESPEC);
> + if (!filespec)
>   goto out;
> + pos = filespec;
> 
>   if (!this || this != &efi_lf2_protocol ||
>   !buffer_size) {
> @@ -136,13 +137,13 @@ efi_load_file2_initrd(struct efi_load_file_protocol 
> *this,
>* * a device and partition identifier, e.g. "0:1"
>* * a file path on the block device, e.g. "/boot/initrd.cpio.gz"
>*/
> - dev = strsep(&s, " ");
> + dev = strsep(&pos, " ");
>   if (!dev)
>   goto out;
> - part = strsep(&s, " ");
> + part = strsep(&pos, " ");
>   if (!part)
>   goto out;
> - file = strsep(&s, " ");
> + file = strsep(&pos, " ");
>   if (!file)
>   goto out;
> 
> @@ -170,7 +171,7 @@ efi_load_file2_initrd(struct efi_load_file_protocol *this,
>   }
> 
>  out:
> - free(s);
> + free(filespec);
>   return EFI_EXIT(status);
>  }
> 
> --
> 2.28.0
> 

Not changing the variable names would make this an one liner to read.
The changes do make sense though so 

Reviewed-by: Ilias Apalodimas 

Re: [PATCH 4/4] efi_selftest: print CRC32 of initrd as hexadecimal

[Ilias Apalodimas]

On Sat, Oct 03, 2020 at 01:57:16PM +0200, Heinrich Schuchardt wrote:
> Print the CRC32 loaded via the EFI_LOAD_FILE2_PROTOCOL as a hexadecimal
> number.
> 
> Signed-off-by: Heinrich Schuchardt 
> ---
>  lib/efi_selftest/efi_selftest_load_initrd.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/efi_selftest/efi_selftest_load_initrd.c 
> b/lib/efi_selftest/efi_selftest_load_initrd.c
> index e16163caca..fe060a6644 100644
> --- a/lib/efi_selftest/efi_selftest_load_initrd.c
> +++ b/lib/efi_selftest/efi_selftest_load_initrd.c
> @@ -200,7 +200,7 @@ static int execute(void)
>   efi_st_error("Could not determine CRC32\n");
>   return EFI_ST_FAILURE;
>   }
> - efi_st_printf("CRC32 %u\n", (unsigned int)crc32);
> + efi_st_printf("CRC32 %.8x\n", (unsigned int)crc32);
> 
>   status = boottime->free_pool(buf);
>   if (status != EFI_SUCCESS) {
> --
> 2.28.0
> 

Reviewed-by: Ilias Apalodimas