Re: [PATCH v4 1/8] binman: ti-secure: Add support for firewalling entities

2023-10-11 Thread Manorit Chawdhry
Hi Simon,

On 20:41-20231011, Simon Glass wrote:
> Hi Manorit,
> 
> On Tue, 10 Oct 2023 at 23:25, Manorit Chawdhry  wrote:
> >
> > We can now firewall entities while loading them through our secure
> > entity TIFS, the required information should be present in the
> > certificate that is being parsed by TIFS.
> >
> > The following commit adds the support to enable the certificates to be
> > generated if the firewall configurations are present in the binman dtsi
> > nodes.
> >
> > Signed-off-by: Manorit Chawdhry 
> > ---
> >  tools/binman/btool/openssl.py   | 16 +++-
> >  tools/binman/etype/ti_secure.py | 90 
> > +
> >  tools/binman/etype/x509_cert.py |  3 +-
> >  3 files changed, 106 insertions(+), 3 deletions(-)
> >
> 
> Reviewed-by: Simon Glass 
> 
> I'm still a little worried about the error reporting if the user
> leaves out a property. Does it do the right thing?
> 

I did make a test also along the lines that would check all the
firewalling properties and just auth-in-place isn't checked at this
stage and people could end up adding the firewalling node without
auth-in-place. Do you want to cover that test case as well? Regarding
the other firewalling properties ( immitating the test by removing
"id"), We get this: 

[..]
  AR  spl/common/spl/built-in.o
  LD  spl/u-boot-spl
  OBJCOPY spl/u-boot-spl-nodtb.bin
  SYM spl/u-boot-spl.sym
  CAT spl/u-boot-spl-dtb.bin
  COPYspl/u-boot-spl.bin
  BINMAN  .binman_stamp
binman: id can't be None in firewall node

> Regards,
> Simon


[PATCH] riscv: andes: Rearrange Andes PLICSW to single-bit-per-hart strategy

2023-10-11 Thread Randolph
Source hart information is not necessary in IPI, so we could
use single-bit-per-hart strategy to rearrange PLICSW mapping.

Bit 0 of Interrupt Pending Bits is hardwired to 0.
Therefore, we use bit 1 to send IPI to hart 0,
bit 2 to hart 1, ..., and so on.

Signed-off-by: Randolph 
---
 arch/riscv/lib/andes_plicsw.c | 24 +++-
 1 file changed, 11 insertions(+), 13 deletions(-)

diff --git a/arch/riscv/lib/andes_plicsw.c b/arch/riscv/lib/andes_plicsw.c
index 7518408089..6fd49e873b 100644
--- a/arch/riscv/lib/andes_plicsw.c
+++ b/arch/riscv/lib/andes_plicsw.c
@@ -22,7 +22,7 @@
 #include 
 
 /* pending register */
-#define PENDING_REG(base, hart)((ulong)(base) + 0x1000 + ((hart) / 4) 
* 4)
+#define PENDING_REG(base)  ((ulong)(base) + 0x1000)
 /* enable register */
 #define ENABLE_REG(base, hart) ((ulong)(base) + 0x2000 + (hart) * 0x80)
 /* claim register */
@@ -30,10 +30,11 @@
 /* priority register */
 #define PRIORITY_REG(base) ((ulong)(base) + PLICSW_PRIORITY_BASE)
 
-#define ENABLE_HART_IPI (0x01010101)
-#define SEND_IPI_TO_HART(hart)  (0x1 << (hart))
+/* Bit 0 of PLIC-SW pending array is hardwired to zero, so we start from bit 1 
*/
+#define FIRST_AVAILABLE_BIT0x2
+#define SEND_IPI_TO_HART(hart) (FIRST_AVAILABLE_BIT << (hart))
 #define PLICSW_PRIORITY_BASE0x4
-#define PLICSW_INTERRUPT_PER_HART   0x8
+#define PLICSW_INTERRUPT_PER_HART   0x1
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -41,9 +42,8 @@ static int enable_ipi(int hart)
 {
unsigned int en;
 
-   en = ENABLE_HART_IPI << hart;
+   en = FIRST_AVAILABLE_BIT << hart;
writel(en, (void __iomem *)ENABLE_REG(gd->arch.plicsw, hart));
-   writel(en, (void __iomem *)ENABLE_REG(gd->arch.plicsw + 0x4, hart));
 
return 0;
 }
@@ -75,7 +75,7 @@ int riscv_init_ipi(void)
ret = uclass_find_first_device(UCLASS_CPU, );
if (ret)
return ret;
-   else if (!dev)
+   if (!dev)
return -ENODEV;
 
ofnode_for_each_subnode(node, dev_ofnode(dev->parent)) {
@@ -105,10 +105,9 @@ int riscv_init_ipi(void)
 
 int riscv_send_ipi(int hart)
 {
-   unsigned int ipi = (SEND_IPI_TO_HART(hart) << (8 * gd->arch.boot_hart));
+   unsigned int ipi = SEND_IPI_TO_HART(hart);
 
-   writel(ipi, (void __iomem *)PENDING_REG(gd->arch.plicsw,
-   gd->arch.boot_hart));
+   writel(ipi, (void __iomem *)PENDING_REG(gd->arch.plicsw));
 
return 0;
 }
@@ -125,10 +124,9 @@ int riscv_clear_ipi(int hart)
 
 int riscv_get_ipi(int hart, int *pending)
 {
-   unsigned int ipi = (SEND_IPI_TO_HART(hart) << (8 * gd->arch.boot_hart));
+   unsigned int ipi = SEND_IPI_TO_HART(hart);
 
-   *pending = readl((void __iomem *)PENDING_REG(gd->arch.plicsw,
-gd->arch.boot_hart));
+   *pending = readl((void __iomem *)PENDING_REG(gd->arch.plicsw));
*pending = !!(*pending & ipi);
 
return 0;
-- 
2.34.1



[PATCH] efi_loader: use well-known guid for auto-created boot option

2023-10-11 Thread Masahisa Kojima
The boot option automatically created by efibootmgr is identified
by the special guid appended in the optional data of boot option.
The same mechanism is implemented in the EDK II reference
implementation, it uses the different guid from the one currently
U-Boot uses.
The guid indicating auto-created boot option is not defined in the
UEFI specification, but some userspace tools such as 'efivar' package
are aware of the guid used in EDK II as auto-created boot option.

So let's use the same guid as EDK II reference implementation.

Signed-off-by: Masahisa Kojima 
---
 include/efi_loader.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index c4207edc91..106006127b 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -149,8 +149,8 @@ static inline efi_status_t efi_launch_capsules(void)
 
 /* GUID for the auto generated boot menu entry */
 #define EFICONFIG_AUTO_GENERATED_ENTRY_GUID \
-   EFI_GUID(0x38c1acc1, 0x9fc0, 0x41f0, \
-0xb9, 0x01, 0xfa, 0x74, 0xd6, 0xd6, 0xe4, 0xde)
+   EFI_GUID(0x8108ac4e, 0x9f11, 0x4d59, \
+0x85, 0x0e, 0xe2, 0x1a, 0x52, 0x2c, 0x59, 0xb2)
 
 /* Use internal device tree when starting UEFI application */
 #define EFI_FDT_USE_INTERNAL NULL
-- 
2.34.1



Re: [PATCH 01/26] spl: legacy: Fix referencing _image_binary_end

2023-10-11 Thread Sean Anderson

On 10/11/23 23:41, Simon Glass wrote:

On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:


On non-arm architectures, _image_binary_end is defined as a ulong and not a
char[]. Dereference it when accessing it, which is correct for both.


Is 'dereference' the right word?


Yeah...

"Take the address of it when accessing it"?

--Sean



Fixes: 1b8a1be1a1f ("spl: spl_legacy: Fix spl_end address")
Signed-off-by: Sean Anderson 
---

  common/spl/spl_legacy.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)


Reviewed-by: Simon Glass 




diff --git a/common/spl/spl_legacy.c b/common/spl/spl_legacy.c
index 095443c63d8..e9564e5c2a5 100644
--- a/common/spl/spl_legacy.c
+++ b/common/spl/spl_legacy.c
@@ -19,7 +19,7 @@
  static void spl_parse_legacy_validate(uintptr_t start, uintptr_t size)
  {
 uintptr_t spl_start = (uintptr_t)_start;
-   uintptr_t spl_end = (uintptr_t)_image_binary_end;
+   uintptr_t spl_end = (uintptr_t)&_image_binary_end;
 uintptr_t end = start + size;

 if ((start >= spl_start && start < spl_end) ||
--
2.37.1





Re: [PATCH 03/26] spl: fit: Fix entry point for SPL_LOAD_FIT_FULL

2023-10-11 Thread Sean Anderson

On 10/11/23 23:41, Simon Glass wrote:

On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:


The entry point is not always the same as the load address. Use the value
of the entrypoint property if it exists and is nonzero (following the
example of spl_load_simple_fit).

Fixes: 8a9dc16e4d0 ("spl: Add full fitImage support")
Signed-off-by: Sean Anderson 
---

  common/spl/spl_fit.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)


Reviewed-by: Simon Glass 

The check for 0 makes me uneasy, but I can't imagine it being valid in practice.


This is mostly to match spl_load_simple_fit:

/*
 * If a platform does not provide CONFIG_SYS_UBOOT_START, U-Boot's
 * Makefile will set it to 0 and it will end up as the entry point
 * here. What it actually means is: use the load address.
 */

SYS_UBOOT_START doesn't seem to be set very often and defaults to TEXT_BASE. 
That
appears to be undefined on

efi-x86_app64 efi-x86_app32 xtfpga 3c120 10m50

but none of these platforms define SPL_LOAD_FIT. So maybe this is moot?

--Sean





diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index ce6b8aa370a..e3cdf8e5c05 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -884,8 +884,10 @@ int spl_load_fit_image(struct spl_image_info *spl_image,
 return ret;

 spl_image->size = fw_len;
-   spl_image->entry_point = fw_data;
 spl_image->load_addr = fw_data;
+   if (fit_image_get_entry(header, ret, _image->entry_point) ||
+   !spl_image->entry_point)
+   spl_image->entry_point = fw_data;
 if (fit_image_get_os(header, ret, _image->os))
 spl_image->os = IH_OS_INVALID;
 spl_image->name = genimg_get_os_name(spl_image->os);
--
2.37.1





Re: [PATCH 09/26] spl: Allow enabling SPL_OF_REAL and SPL_OF_PLATDATA at the same time

2023-10-11 Thread Sean Anderson

On 10/11/23 23:41, Simon Glass wrote:

Hi Sean,

On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:


Sandbox unit tests in U-Boot proper load a test device tree to have some
devices to work with. In order to do the same in SPL, we must enable
SPL_OF_REAL. However, we already have SPL_OF_PLATDATA enabled. When
generating platdata from a devicetree, it is expected that we will not need
devicetree access functions (even though SPL_OF_CONTROL is enabled). This
expectation does not hold for sandbox, so allow user control of
SPL_OF_REAL.

There are several places in the tree where conditions involving OF_PLATDATA
or OF_REAL no longer function correctly when both of these options can be
selected at the same time. Adjust these conditions accordingly.

Signed-off-by: Sean Anderson 
---

  drivers/core/Makefile   | 1 +
  drivers/i2c/i2c-emul-uclass.c   | 2 +-
  drivers/serial/sandbox.c| 2 +-
  drivers/sysreset/sysreset_sandbox.c | 2 +-
  dts/Kconfig | 8 +---
  test/test-main.c| 2 +-
  6 files changed, 10 insertions(+), 7 deletions(-)


Reviewed-by: Simon Glass 

with a due sense of foreboding

I wonder whether this might create confusion?


Yeah, I was a bit worried about that as well.



diff --git a/drivers/core/Makefile b/drivers/core/Makefile
index bce0a3f65cb..acbd2bf2cef 100644
--- a/drivers/core/Makefile
+++ b/drivers/core/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_$(SPL_)OF_LIVE) += of_access.o of_addr.o
  ifndef CONFIG_DM_DEV_READ_INLINE
  obj-$(CONFIG_OF_CONTROL) += read.o
  endif
+obj-$(CONFIG_$(SPL_)OF_PLATDATA) += read.o
  obj-$(CONFIG_OF_CONTROL) += of_extra.o ofnode.o read_extra.o

  ccflags-$(CONFIG_DM_DEBUG) += -DDEBUG
diff --git a/drivers/i2c/i2c-emul-uclass.c b/drivers/i2c/i2c-emul-uclass.c
index 1107cf309fc..d421ddfcbe2 100644
--- a/drivers/i2c/i2c-emul-uclass.c
+++ b/drivers/i2c/i2c-emul-uclass.c
@@ -46,7 +46,7 @@ int i2c_emul_find(struct udevice *dev, struct udevice **emulp)
 struct udevice *emul;
 int ret;

-   if (!CONFIG_IS_ENABLED(OF_PLATDATA)) {
+   if (CONFIG_IS_ENABLED(OF_REAL)) {
 ret = uclass_find_device_by_phandle(UCLASS_I2C_EMUL, dev,
 "sandbox,emul", );
 } else {
diff --git a/drivers/serial/sandbox.c b/drivers/serial/sandbox.c
index f4003811ee7..f6ac3d22852 100644
--- a/drivers/serial/sandbox.c
+++ b/drivers/serial/sandbox.c
@@ -280,7 +280,7 @@ U_BOOT_DRIVER(sandbox_serial) = {
 .flags = DM_FLAG_PRE_RELOC,
  };

-#if CONFIG_IS_ENABLED(OF_REAL)
+#if CONFIG_IS_ENABLED(OF_REAL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
  static const struct sandbox_serial_plat platdata_non_fdt = {
 .colour = -1,
  };
diff --git a/drivers/sysreset/sysreset_sandbox.c 
b/drivers/sysreset/sysreset_sandbox.c
index 3750c60b9b9..f485a135299 100644
--- a/drivers/sysreset/sysreset_sandbox.c
+++ b/drivers/sysreset/sysreset_sandbox.c
@@ -132,7 +132,7 @@ U_BOOT_DRIVER(warm_sysreset_sandbox) = {
 .ops= _warm_sysreset_ops,
  };

-#if CONFIG_IS_ENABLED(OF_REAL)
+#if CONFIG_IS_ENABLED(OF_REAL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
  /* This is here in case we don't have a device tree */
  U_BOOT_DRVINFO(sysreset_sandbox_non_fdt) = {
 .name = "sysreset_sandbox",
diff --git a/dts/Kconfig b/dts/Kconfig
index 9152f5885e9..c6fb193ca89 100644
--- a/dts/Kconfig
+++ b/dts/Kconfig
@@ -410,12 +410,14 @@ config SPL_OF_PLATDATA
   declarations for each node. See of-plat.txt for more information.

  config SPL_OF_REAL
-   bool
+   bool "Support a real devicetree in SPL"


To avoid the user doing something silly, I wonder if it would be
better to keep this option hidden, but enable it for sandbox_spl via
Kconfig?


So

visible if SANDBOX

?


+   depends on SPL_OF_CONTROL
+   select SPL_OF_LIBFDT
 help
   Indicates that a real devicetree is available which can be accessed
   at runtime. This means that dev_read_...() functions can be used to
- read data from the devicetree for each device. This is true if
- SPL_OF_CONTROL is enabled and not SPL_OF_PLATDATA
+ read data from the devicetree for each device. You do not need to
+ enable this option if you have enabled SPL_OF_PLATDATA.

  if SPL_OF_PLATDATA

diff --git a/test/test-main.c b/test/test-main.c
index edb20bc4b9c..b7015d9f38d 100644
--- a/test/test-main.c
+++ b/test/test-main.c
@@ -303,7 +303,7 @@ static int test_pre_run(struct unit_test_state *uts, struct 
unit_test *test)
 if (test->flags & UT_TESTF_PROBE_TEST)
 ut_assertok(do_autoprobe(uts));

-   if (!CONFIG_IS_ENABLED(OF_PLATDATA) &&
+   if (CONFIG_IS_ENABLED(OF_REAL) &&
 (test->flags & UT_TESTF_SCAN_FDT)) {
 /*
  * only set this if we know the ethernet uclass will be created
--
2.37.1



Regards,
Simon




Re: [PATCH 16/26] spl: Don't cache devices when UNIT_TEST is enabled

2023-10-11 Thread Sean Anderson

On 10/11/23 23:41, Simon Glass wrote:

Hi Sean,

On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:


Several SPL functions try to avoid performing initialization twice by
caching devices. This is fine for regular boot, but does not work with
UNIT_TEST, since all devices are torn down after each test. Disable caching
so we don't use stale devices.

Signed-off-by: Sean Anderson 
---

  common/spl/spl_fat.c | 2 +-
  common/spl/spl_mmc.c | 3 ++-
  2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
index c6e2526ade1..8bec9cce5ca 100644
--- a/common/spl/spl_fat.c
+++ b/common/spl/spl_fat.c
@@ -24,7 +24,7 @@ static int spl_register_fat_device(struct blk_desc 
*block_dev, int partition)
  {
 int err = 0;

-   if (fat_registered)
+   if (!CONFIG_IS_ENABLED(UNIT_TEST) && fat_registered)


These sort of things worry me, since we are bringing test code /
conditions into the 'real' code. Is it possible to pass this as a
parameter, or adjust the value of fat_registered?


At the moment these variables are static, and I would like to keep them that way
to avoid size growth. We really are doing an unusual thing with UNIT_TEST where
devices which were previously valid can be free'd and then recreated later. This
is why I decided on using UNIT_TEST as the determiner here, since nothing else
should cause this situation.

--Sean


 return err;

 err = fat_register_device(block_dev, partition);
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index 67c7ae34a58..a8579e29dee 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -417,7 +417,8 @@ int spl_mmc_load(struct spl_image_info *spl_image,

 /* Perform peripheral init only once for an mmc device */
 mmc_dev = spl_mmc_get_device_index(bootdev->boot_device);
-   if (!mmc || spl_mmc_get_mmc_devnum(mmc) != mmc_dev) {
+   if (CONFIG_IS_ENABLED(UNIT_TEST) || !mmc ||
+   spl_mmc_get_mmc_devnum(mmc) != mmc_dev) {
 err = spl_mmc_find_device(, bootdev->boot_device);
 if (err)
 return err;
--
2.37.1



Regards,
Simon




Re: [PATCH 25/26] test: spl: Add a test for the NOR load method

2023-10-11 Thread Sean Anderson

On 10/11/23 23:41, Simon Glass wrote:

Hi Sean,

On Wed, 11 Oct 2023 at 18:57, Sean Anderson  wrote:


Add a test for the NOR load method. Since NOR is memory-mapped we can
substitute a buffer instead. The only major complication is testing LZMA
decompression.  It's too complex to implement LZMA compression in a test,
so we just include some pre-compressed data.


How about using the in-tree compression code?


We have that for LZMA? From what I could tell we only have decompression
in-tree.



Signed-off-by: Sean Anderson 
---

  arch/sandbox/include/asm/spl.h   |   1 +
  configs/sandbox_noinst_defconfig |   2 +
  configs/sandbox_spl_defconfig|   2 +
  include/configs/sandbox.h|   3 +
  include/test/spl.h   |   5 +
  test/image/Makefile  |   1 +
  test/image/spl_load.c| 269 ++-
  test/image/spl_load_nor.c|  39 +
  8 files changed, 316 insertions(+), 6 deletions(-)
  create mode 100644 test/image/spl_load_nor.c


Reviewed-by: Simon Glass 



diff --git a/arch/sandbox/include/asm/spl.h b/arch/sandbox/include/asm/spl.h
index ab9475567e0..cf16af5278a 100644
--- a/arch/sandbox/include/asm/spl.h
+++ b/arch/sandbox/include/asm/spl.h
@@ -13,6 +13,7 @@ enum {
 BOOT_DEVICE_BOARD,
 BOOT_DEVICE_VBE,
 BOOT_DEVICE_CPGMAC,
+   BOOT_DEVICE_NOR,
  };

  /**
diff --git a/configs/sandbox_noinst_defconfig b/configs/sandbox_noinst_defconfig
index 57cbadedb7d..085cc30c1e2 100644
--- a/configs/sandbox_noinst_defconfig
+++ b/configs/sandbox_noinst_defconfig
@@ -49,6 +49,7 @@ CONFIG_SPL_FS_EXT4=y
  CONFIG_SPL_I2C=y
  CONFIG_SPL_MMC_WRITE=y
  CONFIG_SPL_NET=y
+CONFIG_SPL_NOR_SUPPORT=y
  CONFIG_SPL_RTC=y
  CONFIG_CMD_CPU=y
  CONFIG_CMD_LICENSE=y
@@ -257,6 +258,7 @@ CONFIG_RSA_VERIFY_WITH_PKEY=y
  CONFIG_TPM=y
  CONFIG_LZ4=y
  CONFIG_ZSTD=y
+CONFIG_SPL_LZMA=y
  CONFIG_ERRNO_STR=y
  CONFIG_EFI_CAPSULE_ON_DISK=y
  CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
index b578cc8e443..56072b15ad2 100644
--- a/configs/sandbox_spl_defconfig
+++ b/configs/sandbox_spl_defconfig
@@ -41,6 +41,7 @@ CONFIG_SPL_SYS_MALLOC_SIZE=0x400
  CONFIG_SPL_ENV_SUPPORT=y
  CONFIG_SPL_FPGA=y
  CONFIG_SPL_I2C=y
+CONFIG_SPL_NOR_SUPPORT=y
  CONFIG_SPL_RTC=y
  CONFIG_CMD_CPU=y
  CONFIG_CMD_LICENSE=y
@@ -249,6 +250,7 @@ CONFIG_TPM=y
  CONFIG_SPL_CRC8=y
  CONFIG_LZ4=y
  CONFIG_ZSTD=y
+CONFIG_SPL_LZMA=y
  CONFIG_ERRNO_STR=y
  CONFIG_SPL_HEXDUMP=y
  CONFIG_EFI_CAPSULE_ON_DISK=y
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
index 4e5653dc886..2372485c84e 100644
--- a/include/configs/sandbox.h
+++ b/include/configs/sandbox.h
@@ -18,4 +18,7 @@
  #define CFG_SYS_BAUDRATE_TABLE {4800, 9600, 19200, 38400, 57600,\
 115200}

+/* Unused but necessary to build */
+#define CFG_SYS_UBOOT_BASE CONFIG_TEXT_BASE
+
  #endif
diff --git a/include/test/spl.h b/include/test/spl.h
index cfb52c90855..82325702d4a 100644
--- a/include/test/spl.h
+++ b/include/test/spl.h
@@ -27,12 +27,14 @@ void generate_data(char *data, size_t size, const char 
*test_name);
  /**
   * enum spl_test_image - Image types for testing
   * @LEGACY: "Legacy" uImages
+ * @LEGACY_LZMA: "Legacy" uImages, LZMA compressed
   * @IMX8: i.MX8 Container images
   * @FIT_INTERNAL: FITs with internal data
   * @FIT_EXTERNAL: FITs with external data
   */
  enum spl_test_image {
 LEGACY,
+   LEGACY_LZMA,
 IMX8,
 FIT_INTERNAL,
 FIT_EXTERNAL,
@@ -118,6 +120,9 @@ int do_spl_test_load(struct unit_test_state *uts, const 
char *test_name,
  static inline bool image_supported(enum spl_test_image type)
  {
 switch (type) {
+   case LEGACY_LZMA:
+   if (!IS_ENABLED(CONFIG_SPL_LZMA))
+   return false;
 case LEGACY:
 return IS_ENABLED(CONFIG_SPL_LEGACY_IMAGE_FORMAT);
 case IMX8:
diff --git a/test/image/Makefile b/test/image/Makefile
index ddbc39bf959..41b29995993 100644
--- a/test/image/Makefile
+++ b/test/image/Makefile
@@ -5,4 +5,5 @@
  obj-$(CONFIG_SPL_UT_LOAD) += spl_load.o
  obj-$(CONFIG_SPL_UT_LOAD_FS) += spl_load_fs.o
  obj-$(CONFIG_SPL_UT_LOAD_NET) += spl_load_net.o
+obj-$(CONFIG_SPL_NOR_SUPPORT) += spl_load_nor.o
  obj-$(CONFIG_SPL_UT_LOAD_OS) += spl_load_os.o
diff --git a/test/image/spl_load.c b/test/image/spl_load.c
index 06249044f9b..3f69e652630 100644
--- a/test/image/spl_load.c
+++ b/test/image/spl_load.c
@@ -43,6 +43,7 @@ void *board_spl_fit_buffer_addr(ulong fit_size, int sectors, 
int bl_len)

  /* Local flags for spl_image; start from the "top" to avoid conflicts */
  #define SPL_IMX_CONTAINER  0x8000
+#define SPL_COMP_LZMA  0x4000

  void generate_data(char *data, size_t size, const char *test_name)
  {
@@ -79,7 +80,8 @@ static size_t create_legacy(void *dst, struct spl_image_info 
*spl_image,
 image_set_os(hdr, spl_image->os);
 

Re: [PATCH 23/26] test: spl: Add a test for the MMC load method

2023-10-11 Thread Sean Anderson

On 10/11/23 23:41, Simon Glass wrote:

Hi Sean,

On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:


Add a test for the MMC load method. This shows the general shape of tests
to come: The main test function calls do_spl_test_load with an appropriate
callback to write the image to the medium.

Signed-off-by: Sean Anderson 
---

  configs/sandbox_noinst_defconfig |  2 +
  include/spl.h|  4 ++
  include/test/spl.h   | 30 +++
  test/image/Kconfig   |  1 +
  test/image/spl_load.c| 36 +
  test/image/spl_load_fs.c | 66 +---
  6 files changed, 134 insertions(+), 5 deletions(-)

diff --git a/configs/sandbox_noinst_defconfig b/configs/sandbox_noinst_defconfig
index 11be2dccf7d..4f16d9860d2 100644
--- a/configs/sandbox_noinst_defconfig
+++ b/configs/sandbox_noinst_defconfig
@@ -41,6 +41,8 @@ CONFIG_SPL_SYS_MALLOC=y
  CONFIG_SPL_HAS_CUSTOM_MALLOC_START=y
  CONFIG_SPL_CUSTOM_SYS_MALLOC_ADDR=0xa00
  CONFIG_SPL_SYS_MALLOC_SIZE=0x400
+CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
+CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x0
  CONFIG_SPL_ENV_SUPPORT=y
  CONFIG_SPL_FS_EXT4=y
  CONFIG_SPL_I2C=y
diff --git a/include/spl.h b/include/spl.h
index 7d30fb57dac..8229d40adab 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -673,6 +673,10 @@ static inline const char *spl_loader_name(const struct 
spl_image_loader *loader)
 }
  #endif

+#define SPL_LOAD_IMAGE_GET(_priority, _boot_device, _method) \
+   ll_entry_get(struct spl_image_loader, \
+_boot_device ## _priority ## _method, spl_image_loader)
+
  /* SPL FAT image functions */
  int spl_load_image_fat(struct spl_image_info *spl_image,
struct spl_boot_device *bootdev,
diff --git a/include/test/spl.h b/include/test/spl.h
index 7ae32a1020b..cfb52c90855 100644
--- a/include/test/spl.h
+++ b/include/test/spl.h
@@ -79,6 +79,36 @@ size_t create_image(void *dst, enum spl_test_image type,
  int check_image_info(struct unit_test_state *uts, struct spl_image_info 
*info1,
  struct spl_image_info *info2);

+/**
+ * typedef write_image_t - Callback for writing an image
+ * @uts: Current unit test state
+ * @img: Image to write
+ * @size: Size of @img
+ *
+ * Write @img to a location which will be read by a  spl_image_loader.
+ *
+ * Return: 0 on success or -1 on failure
+ */
+typedef int write_image_t(struct unit_test_state *its, void *img, size_t size);
+
+/**
+ * do_spl_test_load() - Test loading with an SPL image loader
+ * @uts: Current unit test state
+ * @test_name: Name of the current test
+ * @type: Type of image to try loading
+ * @loader: The loader to test
+ * @write_image: Callback to write the image to the backing storage
+ *
+ * Test @loader, performing the common tasks of setting up the image and
+ * checking it was loaded correctly. The caller must supply a @write_image
+ * callback to write the image to a location which will be read by @loader.
+ *
+ * Return: 0 on success or -1 on failure


Do you mean 1 ?

Reviewed-by: Simon Glass 


Huh... I guess I do.

--Sean


Re: [PATCH 21/26] test: spl: Add functions to create filesystems

2023-10-11 Thread Sean Anderson

On 10/11/23 23:41, Simon Glass wrote:

Hi Sean,

On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:


Add some functions for creating fat/ext2 filesystems with a single file and
a test for them. Filesystems require block devices, and it is easiest to
just use MMC for this. To get an MMC, we must also pull in the test device
tree.

Signed-off-by: Sean Anderson 
---

  arch/sandbox/cpu/start.c |   9 +-
  configs/sandbox_noinst_defconfig |   7 +
  include/ext4fs.h |   1 +
  include/ext_common.h |  14 ++
  include/test/spl.h   |   3 +
  test/image/Kconfig   |  11 ++
  test/image/Makefile  |   1 +
  test/image/spl_load.c|   1 +
  test/image/spl_load_fs.c | 305 +++
  9 files changed, 350 insertions(+), 2 deletions(-)
  create mode 100644 test/image/spl_load_fs.c


Reviewed-by: Simon Glass 

Wow there is a lot going on in this patch. It might be good to split it.


OK.



diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
index 2c8a72590b5..f5728e6e7ee 100644
--- a/arch/sandbox/cpu/start.c
+++ b/arch/sandbox/cpu/start.c
@@ -13,6 +13,7 @@
  #include 
  #include 
  #include 
+#include 
  #include 
  #include 
  #include 
@@ -202,10 +203,14 @@ static int sandbox_cmdline_cb_test_fdt(struct 
sandbox_state *state,
  {
 char buf[256];
 char *fname;
+   char *relname;
 int len;

-   len = state_get_rel_filename("arch/sandbox/dts/test.dtb", buf,
-sizeof(buf));
+   if (spl_phase() < PHASE_BOARD_F)


I think <= PHASE_SPL is better since you care about whether it is in
SPL or not. I did send a patch to check for SPL, but I don't think it
is merged yet.


Fine by me.


+   relname = "../arch/sandbox/dts/test.dtb";
+   else
+   relname = "arch/sandbox/dts/test.dtb";
+   len = state_get_rel_filename(relname, buf, sizeof(buf));
 if (len < 0)
 return len;

diff --git a/configs/sandbox_noinst_defconfig b/configs/sandbox_noinst_defconfig
index 908155be8a3..0a542cfb6aa 100644
--- a/configs/sandbox_noinst_defconfig
+++ b/configs/sandbox_noinst_defconfig
@@ -6,10 +6,12 @@ CONFIG_NR_DRAM_BANKS=1
  CONFIG_ENV_SIZE=0x2000
  CONFIG_DEFAULT_DEVICE_TREE="sandbox"
  CONFIG_DM_RESET=y
+CONFIG_SPL_MMC=y
  CONFIG_SPL_SERIAL=y
  CONFIG_SPL_DRIVERS_MISC=y
  CONFIG_SPL_SYS_MALLOC_F_LEN=0x8000
  CONFIG_SPL=y
+CONFIG_SPL_FS_FAT=y
  CONFIG_SYS_LOAD_ADDR=0x0
  CONFIG_PCI=y
  CONFIG_SANDBOX_SPL=y
@@ -39,7 +41,9 @@ CONFIG_SPL_HAS_CUSTOM_MALLOC_START=y
  CONFIG_SPL_CUSTOM_SYS_MALLOC_ADDR=0xa00
  CONFIG_SPL_SYS_MALLOC_SIZE=0x400
  CONFIG_SPL_ENV_SUPPORT=y
+CONFIG_SPL_FS_EXT4=y
  CONFIG_SPL_I2C=y
+CONFIG_SPL_MMC_WRITE=y
  CONFIG_SPL_RTC=y
  CONFIG_CMD_CPU=y
  CONFIG_CMD_LICENSE=y
@@ -97,6 +101,7 @@ CONFIG_AMIGA_PARTITION=y
  CONFIG_OF_CONTROL=y
  CONFIG_SPL_OF_CONTROL=y
  CONFIG_SPL_OF_PLATDATA=y
+CONFIG_SPL_OF_REAL=y
  CONFIG_ENV_IS_NOWHERE=y
  CONFIG_ENV_IS_IN_EXT4=y
  CONFIG_ENV_EXT4_INTERFACE="host"
@@ -159,6 +164,7 @@ CONFIG_CROS_EC_SPI=y
  CONFIG_P2SB=y
  CONFIG_PWRSEQ=y
  CONFIG_SPL_PWRSEQ=y
+CONFIG_FS_LOADER=y
  CONFIG_MMC_SANDBOX=y
  CONFIG_SPI_FLASH_SANDBOX=y
  CONFIG_SPI_FLASH_ATMEL=y
@@ -220,6 +226,7 @@ CONFIG_SYSRESET=y
  CONFIG_SPL_SYSRESET=y
  CONFIG_DM_THERMAL=y
  CONFIG_TIMER=y
+CONFIG_SPL_TIMER=y
  CONFIG_TIMER_EARLY=y
  CONFIG_SANDBOX_TIMER=y
  CONFIG_USB=y


I think these would be better in their own patch


Surprisingly, MMC_WRITE actually depends on SPL_TIMER. I didn't check on the 
exact
dependency, but presumably there is some poll/timeout loop in the MMC write 
code.


diff --git a/include/ext4fs.h b/include/ext4fs.h
index cb5d9cc0a5c..dd66d27f776 100644
--- a/include/ext4fs.h
+++ b/include/ext4fs.h
@@ -31,6 +31,7 @@
  struct disk_partition;

  #define EXT4_INDEX_FL  0x1000 /* Inode uses hash tree index */
+#define EXT4_TOPDIR_FL 0x0002 /* Top of directory hierarchies*/
  #define EXT4_EXTENTS_FL0x0008 /* Inode uses extents */
  #define EXT4_EXT_MAGIC 0xf30a
  #define EXT4_FEATURE_RO_COMPAT_GDT_CSUM0x0010
diff --git a/include/ext_common.h b/include/ext_common.h
index 30a0c248414..b09bbde116a 100644
--- a/include/ext_common.h
+++ b/include/ext_common.h
@@ -35,6 +35,16 @@ struct cmd_tbl;
  #define EXT2_PATH_MAX  4096
  /* Maximum nesting of symlinks, used to prevent a loop.  */
  #defineEXT2_MAX_SYMLINKCNT 8
+/* Maximum file name length */
+#define EXT2_NAME_LEN 255
+
+/*
+ * Revision levels
+ */
+#define EXT2_GOOD_OLD_REV  0   /* The good old (original) format */
+#define EXT2_DYNAMIC_REV   1   /* V2 format w/ dynamic inode sizes */
+
+#define EXT2_GOOD_OLD_INODE_SIZE 128

  /* Filetype used in directory entry.  */
  #defineFILETYPE_UNKNOWN0
@@ -48,6 +58,10 @@ struct cmd_tbl;
  #define FILETYPE_INO_DIRECTORY 

Re: [PATCH 24/26] test: spl: Add a test for the NET load method

2023-10-11 Thread Sean Anderson

On 10/11/23 23:41, Simon Glass wrote:

On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:


Add a test for loading U-Boot over TFTP. As with other sandbox net
routines, we need to initialize our packets manually since things like
net_set_ether and net_set_udp_header always use "our" addresses. We use
BOOTP instead of DHCP, since DHCP has a tag/length-based format which is
harder to parse. Our TFTP implementation doesn't define as many constants
as I'd like, so I create some here. Note that the TFTP block size is
one-based, but offsets are zero-based.

In order to avoid address errors, we need to set up/define some additional
address information settings. dram_init_banksize would be a good candidate
for settig up bi_dram, but it gets called too late in board_init_r.

Signed-off-by: Sean Anderson 
---

  arch/sandbox/cpu/spl.c   |   3 +
  arch/sandbox/include/asm/spl.h   |   1 +
  configs/sandbox_noinst_defconfig |   6 +-
  test/image/Kconfig   |   9 ++
  test/image/Makefile  |   1 +
  test/image/spl_load_net.c| 252 +++
  6 files changed, 271 insertions(+), 1 deletion(-)
  create mode 100644 test/image/spl_load_net.c



Reviewed-by: Simon Glass 


diff --git a/arch/sandbox/cpu/spl.c b/arch/sandbox/cpu/spl.c
index 09e3d10d6a5..8153df18d68 100644
--- a/arch/sandbox/cpu/spl.c
+++ b/arch/sandbox/cpu/spl.c
@@ -126,6 +126,9 @@ void spl_board_init(void)
  {
 struct sandbox_state *state = state_get_current();

+   gd->bd->bi_dram[0].start = gd->ram_base;
+   gd->bd->bi_dram[0].size = get_effective_memsize();


These could use a common as to why they are needed here.


OK.

--Sean


Re: [PATCH 20/26] test: spl: Add functions to create images

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:
>
> This add some basic functions to create images, and a test for said
> functions. This is not intended to be a test of the image parsing
> functions, but rather a framework for creating minimal images for testing
> load methods. That said, it does do an OK job at finding bugs in the image
> parsing directly.
>
> Since we have two methods for loading/parsing FIT images, add LOAD_FIT_FULL
> as a separate CI run.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  .azure-pipelines.yml |   4 +
>  .gitlab-ci.yml   |   7 +
>  arch/sandbox/cpu/u-boot-spl.lds  |   2 +
>  configs/sandbox_noinst_defconfig |   6 +
>  configs/sandbox_spl_defconfig|   6 +
>  include/test/spl.h   | 117 ++
>  test/image/spl_load.c| 352 +++
>  test/image/spl_load_os.c |   5 +-
>  8 files changed, 495 insertions(+), 4 deletions(-)
>  create mode 100644 include/test/spl.h

Reviewed-by: Simon Glass 

The error handling for the FDT creation is a bit unusual, but it is
test code, so if it fails all bets are off. So OK.


Re: [PATCH] riscv: binman: Fix compilation error

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 08:30, Mayuresh Chitale
 wrote:
>
> Some platforms may not have any DDR memory below 4G and for such platforms
> the TEXT_BASE and LOAD addresses etc are all 64 bit addresses due to
> which the u-boot build fails with below error:
>
> u-boot/arch/riscv/dts/binman.dtsi:30.14-25
> Value out of range for 32-bit array element
> u-boot/arch/riscv/dts/binman.dtsi:43.14-25
> Value out of range for 32-bit array element
> u-boot/arch/riscv/dts/binman.dtsi:44.15-26
> Value out of range for 32-bit array element
> FATAL ERROR: Syntax error parsing input tree
>
> Fix by setting the address-cells property to 2 and converting load
> addresses to 64 bit values.
>
> Signed-off-by: Mayuresh Chitale 
> ---
>  arch/riscv/dts/binman.dtsi | 14 ++
>  1 file changed, 10 insertions(+), 4 deletions(-)
>

Reviewed-by: Simon Glass 


Re: [PATCH v5] bootstd: sata: Add bootstd support for ahci sata

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 13:27, Tony Dinh  wrote:
>
> Add ahci sata bootdev and corresponding hunting function.
>
> Signed-off-by: Tony Dinh 
> ---
>
> Changes in v5:
> - In bootmeth_script script_boot(), it's unnecessary to check for ret so
> remove it. While we're here, also initialize ret in declaration.
>
> Changes in v4:
> - Revise logic in bootmeth_script() to set devtype to sata for non-scsi
> SATA device
> - Rewrite sata_rescan() logic to properly remove all devices before probing
> - Add description to sata_rescan() header
>
> Changes in v3:
> - Correct drivers/ata/Makefile to compile sata_bootdev only if
> ahci sata is enabled.
>
> Changes in v2:
> - set devtype to sata in bootmeth_script for non-scsi SATA device.
>
>  boot/bootmeth_script.c | 16 +++---
>  drivers/ata/Makefile   |  2 +-
>  drivers/ata/sata.c | 32 
>  drivers/ata/sata_bootdev.c | 62 ++
>  include/sata.h |  6 
>  5 files changed, 113 insertions(+), 5 deletions(-)
>  create mode 100644 drivers/ata/sata_bootdev.c

Reviewed-by: Simon Glass 


Re: [PATCH v2 6/6] binman: capsule: Add support for generating EFI empty capsules

2023-10-11 Thread Simon Glass
On Tue, 10 Oct 2023 at 02:12, Sughosh Ganu  wrote:
>
> Add support in binman for generating EFI empty capsules. These
> capsules are used in the FWU A/B update feature. Also add test cases
> in binman for the corresponding code coverage.
>
> Signed-off-by: Sughosh Ganu 
> ---
> Changes since V1:
> * Instead of using two separate boolean values, use a 'capsule-type'
>   property for indicating generation of an accept/revert capsule.
> * Make corresponding changes in the sanity checks, documentation and
>   tests based on the above change.
> * Use lower case characters for GUIDs.
> * Call get_binman_test_guid() from the efi_capsule entry module.
> * Add the documentation entry for the empty capsules in entries.rst.
> * Remove the #[address,size]-cells properties from the test dts' for
>   empty capsules.
>
>  tools/binman/entries.rst  | 44 ++
>  tools/binman/etype/efi_empty_capsule.py   | 86 +++
>  tools/binman/ftest.py | 57 
>  tools/binman/test/319_capsule_accept.dts  | 13 +++
>  tools/binman/test/320_capsule_revert.dts  | 11 +++
>  .../test/321_capsule_accept_missing_guid.dts  | 11 +++
>  .../test/322_empty_capsule_type_missing.dts   | 12 +++
>  .../323_capsule_accept_revert_missing.dts | 13 +++
>  8 files changed, 247 insertions(+)
>  create mode 100644 tools/binman/etype/efi_empty_capsule.py
>  create mode 100644 tools/binman/test/319_capsule_accept.dts
>  create mode 100644 tools/binman/test/320_capsule_revert.dts
>  create mode 100644 tools/binman/test/321_capsule_accept_missing_guid.dts
>  create mode 100644 tools/binman/test/322_empty_capsule_type_missing.dts
>  create mode 100644 tools/binman/test/323_capsule_accept_revert_missing.dts
>

Reviewed-by: Simon Glass 


Re: coreboot 4.11 + U-boot master

2023-10-11 Thread Simon Glass
Hi Jay,

On Wed, 11 Oct 2023 at 19:03, Jay Talbott
 wrote:
>
> Hi Simon,
>
>
>
> So I gave it a whirl to do a build of coreboot (on the 4.11 branch) with the 
> U-boot payload, specifying U-boot master to get the latest version of U-boot.
>
>
>
> Unfortunately, it failed to build the payload:
>
>
>
> Cloning U-Boot from Git
>
> Cloning into 'u-boot'...
>
> warning: redirecting to https://source.denx.de/u-boot/u-boot.git/
>
> remote: Enumerating objects: 945927, done.
>
> remote: Counting objects: 100% (6622/6622), done.
>
> remote: Compressing objects: 100% (2418/2418), done.
>
> remote: Total 945927 (delta 4232), reused 6496 (delta 4159), pack-reused 
> 939305
>
> Receiving objects: 100% (945927/945927), 190.29 MiB | 6.01 MiB/s, done.
>
> Resolving deltas: 100% (793720/793720), done.
>
> Updating files: 100% (20136/20136), done.
>
> Fetching new commits from the U-Boot git repo
>
> Checking out U-Boot revision origin/master
>
> Already on 'master'
>
> Your branch is up to date with 'origin/master'.
>
> Branch 'coreboot' set up to track remote branch 'master' from 'origin'.
>
> Switched to a new branch 'coreboot'
>
> MAKE   U-Boot origin/master
>
> In file included from tools/imagetool.h:24,
>
>  from tools/aisimage.c:7:
>
> include/image.h:1396:12: fatal error: openssl/evp.h: No such file or directory
>
> 1396 | #  include 
>
>   |^~~
>
> compilation terminated.
>
> make[3]: *** [scripts/Makefile.host:112: tools/aisimage.o] Error 1
>
> make[2]: *** [Makefile:1858: tools] Error 2
>
> make[1]: *** [Makefile:75: build] Error 2
>
> make: *** [payloads/external/Makefile.inc:187: 
> payloads/external/U-Boot/u-boot/u-boot-dtb.bin] Error 2
>
>
>
> Looks like it needs openssl to get pulled in as part of the build. Is there 
> something I’m missing that I need to do to get that to happen automatically? 
> Or is there a manual step I need to do to get it pulled in?

For Debian this is:

sudo apt install libssl-dev

You can see all the pre-reqs at [1]


>
>
>
> Also, in coreboot’s make menuconfig, there’s a place for specifying a U-boot 
> config file. For the first go at it I didn’t specify one, but I’m guessing 
> that maybe I need to at some point. If so, can you point me to what I’ll need 
> to put in there?

Nothing in particular. At some point if you want to build a particular
U-Boot version from a separate checkout, you can manually replace
U-Boot in coreboot.rom:

# update u-boot
cbfstool $BUILD_DIR/coreboot.rom remove -n fallback/payload || true
cbfstool $BUILD_DIR/coreboot.rom add-flat-binary \
   -f u-boot.bin \
-n fallback/payload -c LZMA -l 0x111 -e 0x111

If you want the 64-bit U-Boot then instead of u-boot.bin you need
u-boot-x86-with-spl.bin (and you must use 'make coreboot64_defconfig'
to configure U-Boot). In fact, that option is not available in
coreboot yet, so I should send a patch!


[1] https://u-boot.readthedocs.io/en/latest/build/gcc.html


Re: [PATCH 3/5] smbios: Use SMBIOS 3.0 to support an address above 4GB

2023-10-11 Thread Simon Glass
Hi Heinrich,

On Sun, 8 Oct 2023 at 19:48, Heinrich Schuchardt  wrote:
>
> On 10/8/23 23:36, Simon Glass wrote:
> > When the SMBIOS table is written to an address above 4GB a 32-bit table
> > address is not large enough.
> >
> > Use an SMBIOS3 table in that case.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> >   include/smbios.h | 22 +-
> >   lib/smbios.c | 22 ++
> >   2 files changed, 39 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/smbios.h b/include/smbios.h
> > index c9df2706f5a6..ddabb558299e 100644
> > --- a/include/smbios.h
> > +++ b/include/smbios.h
> > @@ -12,7 +12,8 @@
> >
> >   /* SMBIOS spec version implemented */
> >   #define SMBIOS_MAJOR_VER3
> > -#define SMBIOS_MINOR_VER 0
> > +#define SMBIOS_MINOR_VER 7
> > +
> >
> >   enum {
> >   SMBIOS_STR_MAX  = 64,   /* Maximum length allowed for a string */
> > @@ -54,6 +55,25 @@ struct __packed smbios_entry {
> >   u8 bcd_rev;
> >   };
> >
> > +struct __packed smbios3_entry {
> > + u8 anchor[5];
> > + u8 checksum;
> > + u8 length;
> > + u8 major_ver;
> > +
> > + u8 minor_ver;
> > + u8 docrev;
> > + u8 entry_point_rev;
> > + u8 reserved;
> > + u32 max_struct_size;
> > +
> > + u64 struct_table_address;
> > +};
> > +
> > +/* These two structures should use the same amount of 16-byte-aligned 
> > space */
> > +static_assert(ALIGN(16, sizeof(struct smbios_entry)) ==
> > +   ALIGN(16, sizeof(struct smbios3_entry)));
> > +
> >   /* BIOS characteristics */
> >   #define BIOS_CHARACTERISTICS_PCI_SUPPORTED  (1 << 7)
> >   #define BIOS_CHARACTERISTICS_UPGRADEABLE(1 << 11)
> > diff --git a/lib/smbios.c b/lib/smbios.c
> > index c7a557bc9b7b..82e259f31791 100644
> > --- a/lib/smbios.c
> > +++ b/lib/smbios.c
> > @@ -487,7 +487,11 @@ ulong write_smbios_table(ulong addr)
> >   addr = ALIGN(addr, 16);
> >   start_addr = addr;
> >
> > - addr += sizeof(struct smbios_entry);
> > + /*
> > +  * So far we don't know which struct will be used, but they both end
> > +  * up using the same amount of 16-bit-aligned space
> > +  */
> > + addr += max(sizeof(struct smbios_entry), sizeof(struct 
> > smbios3_entry));
> >   addr = ALIGN(addr, 16);
> >   tables = addr;
> >
> > @@ -513,13 +517,23 @@ ulong write_smbios_table(ulong addr)
> >*/
> >   table_addr = (ulong)map_sysmem(tables, 0);
> >   if (sizeof(table_addr) > sizeof(u32) && table_addr > (ulong)UINT_MAX) 
> > {
>
> You have to check the end address of the table not the start address here.
>
> The SMBIOS specification says that you should always supply a 32bit
> SMBIOS entry point. Of course this is not possible on boards that don't
> have low memory.
>
> In install_smbios_table() this can be achieved by calling
> efi_allocate_pages() with EFI_MAX_ALLOCATE_TYPE and a maximum address of
> 4 GiB - 1. Only if this fails use high memory.

Hmm perhaps we need a way to allocate memory below 4GB in U-Boot? How
does this EFI function work?

I don't think EFI has been set up by the time this function is called.

Regards,
Simon


Re: [PATCH v6 02/14] firmware: scmi: use a protocol's own channel if assigned

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 03:07, AKASHI Takahiro
 wrote:
>
> SCMI specification allows any protocol to have its own channel for
> the transport. While the current SCMI driver may assign its channel
> from a device tree, the core function, devm_scmi_process_msg(), doesn't
> use a protocol's channel, but always use an agent's channel.
>
> With this commit, devm_scmi_process_msg() tries to find and use
> a protocol's channel. If it doesn't exist, use an agent's.
>
> Signed-off-by: AKASHI Takahiro 
> Reviewed-by: Etienne Carriere 
> ---
> v5
> * new commit (fixing a potential bug)
> ---
>  drivers/firmware/scmi/mailbox_agent.c | 5 +++--
>  drivers/firmware/scmi/optee_agent.c   | 5 +++--
>  drivers/firmware/scmi/scmi_agent-uclass.c | 7 ---
>  drivers/firmware/scmi/smccc_agent.c   | 5 +++--
>  include/scmi_agent-uclass.h   | 8 +---
>  5 files changed, 18 insertions(+), 12 deletions(-)

Reviewed-by: Simon Glass 


Re: [PATCH 19/26] test: spl: Fix spl_test_load not failing if fname doesn't exist

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:
>
> Returning a negative value from a unit test doesn't automatically fail the
> test.  We have to fail an assertion. Modify the test to do so.
>
> This now causes the test to count as a failure on VPL. This is because the
> fname of SPL (and U-Boot) is generated with make_exec in os_jump_to_image.
> The original name of SPL is gone, and we can't determine the name of U-Boot
> from the generated name.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  configs/sandbox_vpl_defconfig | 1 +
>  test/image/spl_load_os.c  | 6 ++
>  2 files changed, 3 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass 


Re: [PATCH v2 5/6] btool: mkeficapsule: Add support for EFI empty capsule generation

2023-10-11 Thread Simon Glass
On Tue, 10 Oct 2023 at 02:12, Sughosh Ganu  wrote:
>
> Add a method to the mkeficapsule bintool to generate empty
> capsules. These are capsules needed for the FWU A/B update feature.
>
> Signed-off-by: Sughosh Ganu 
> ---
> Changes since V1:
> * Use a single boolean value to indicate the generation of either of
>   accept/revert capsule.
> * Move the parameters added to the list on the same line in a couple
>   of places.
>
>  tools/binman/btool/mkeficapsule.py | 26 ++
>  1 file changed, 26 insertions(+)
>

Reviewed-by: Simon Glass 


Re: [PATCH 23/26] test: spl: Add a test for the MMC load method

2023-10-11 Thread Simon Glass
Hi Sean,

On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:
>
> Add a test for the MMC load method. This shows the general shape of tests
> to come: The main test function calls do_spl_test_load with an appropriate
> callback to write the image to the medium.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  configs/sandbox_noinst_defconfig |  2 +
>  include/spl.h|  4 ++
>  include/test/spl.h   | 30 +++
>  test/image/Kconfig   |  1 +
>  test/image/spl_load.c| 36 +
>  test/image/spl_load_fs.c | 66 +---
>  6 files changed, 134 insertions(+), 5 deletions(-)
>
> diff --git a/configs/sandbox_noinst_defconfig 
> b/configs/sandbox_noinst_defconfig
> index 11be2dccf7d..4f16d9860d2 100644
> --- a/configs/sandbox_noinst_defconfig
> +++ b/configs/sandbox_noinst_defconfig
> @@ -41,6 +41,8 @@ CONFIG_SPL_SYS_MALLOC=y
>  CONFIG_SPL_HAS_CUSTOM_MALLOC_START=y
>  CONFIG_SPL_CUSTOM_SYS_MALLOC_ADDR=0xa00
>  CONFIG_SPL_SYS_MALLOC_SIZE=0x400
> +CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
> +CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x0
>  CONFIG_SPL_ENV_SUPPORT=y
>  CONFIG_SPL_FS_EXT4=y
>  CONFIG_SPL_I2C=y
> diff --git a/include/spl.h b/include/spl.h
> index 7d30fb57dac..8229d40adab 100644
> --- a/include/spl.h
> +++ b/include/spl.h
> @@ -673,6 +673,10 @@ static inline const char *spl_loader_name(const struct 
> spl_image_loader *loader)
> }
>  #endif
>
> +#define SPL_LOAD_IMAGE_GET(_priority, _boot_device, _method) \
> +   ll_entry_get(struct spl_image_loader, \
> +_boot_device ## _priority ## _method, spl_image_loader)
> +
>  /* SPL FAT image functions */
>  int spl_load_image_fat(struct spl_image_info *spl_image,
>struct spl_boot_device *bootdev,
> diff --git a/include/test/spl.h b/include/test/spl.h
> index 7ae32a1020b..cfb52c90855 100644
> --- a/include/test/spl.h
> +++ b/include/test/spl.h
> @@ -79,6 +79,36 @@ size_t create_image(void *dst, enum spl_test_image type,
>  int check_image_info(struct unit_test_state *uts, struct spl_image_info 
> *info1,
>  struct spl_image_info *info2);
>
> +/**
> + * typedef write_image_t - Callback for writing an image
> + * @uts: Current unit test state
> + * @img: Image to write
> + * @size: Size of @img
> + *
> + * Write @img to a location which will be read by a  spl_image_loader.
> + *
> + * Return: 0 on success or -1 on failure
> + */
> +typedef int write_image_t(struct unit_test_state *its, void *img, size_t 
> size);
> +
> +/**
> + * do_spl_test_load() - Test loading with an SPL image loader
> + * @uts: Current unit test state
> + * @test_name: Name of the current test
> + * @type: Type of image to try loading
> + * @loader: The loader to test
> + * @write_image: Callback to write the image to the backing storage
> + *
> + * Test @loader, performing the common tasks of setting up the image and
> + * checking it was loaded correctly. The caller must supply a @write_image
> + * callback to write the image to a location which will be read by @loader.
> + *
> + * Return: 0 on success or -1 on failure

Do you mean 1 ?

Reviewed-by: Simon Glass 


Re: [PATCH 16/26] spl: Don't cache devices when UNIT_TEST is enabled

2023-10-11 Thread Simon Glass
Hi Sean,

On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:
>
> Several SPL functions try to avoid performing initialization twice by
> caching devices. This is fine for regular boot, but does not work with
> UNIT_TEST, since all devices are torn down after each test. Disable caching
> so we don't use stale devices.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  common/spl/spl_fat.c | 2 +-
>  common/spl/spl_mmc.c | 3 ++-
>  2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
> index c6e2526ade1..8bec9cce5ca 100644
> --- a/common/spl/spl_fat.c
> +++ b/common/spl/spl_fat.c
> @@ -24,7 +24,7 @@ static int spl_register_fat_device(struct blk_desc 
> *block_dev, int partition)
>  {
> int err = 0;
>
> -   if (fat_registered)
> +   if (!CONFIG_IS_ENABLED(UNIT_TEST) && fat_registered)

These sort of things worry me, since we are bringing test code /
conditions into the 'real' code. Is it possible to pass this as a
parameter, or adjust the value of fat_registered?

> return err;
>
> err = fat_register_device(block_dev, partition);
> diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
> index 67c7ae34a58..a8579e29dee 100644
> --- a/common/spl/spl_mmc.c
> +++ b/common/spl/spl_mmc.c
> @@ -417,7 +417,8 @@ int spl_mmc_load(struct spl_image_info *spl_image,
>
> /* Perform peripheral init only once for an mmc device */
> mmc_dev = spl_mmc_get_device_index(bootdev->boot_device);
> -   if (!mmc || spl_mmc_get_mmc_devnum(mmc) != mmc_dev) {
> +   if (CONFIG_IS_ENABLED(UNIT_TEST) || !mmc ||
> +   spl_mmc_get_mmc_devnum(mmc) != mmc_dev) {
> err = spl_mmc_find_device(, bootdev->boot_device);
> if (err)
> return err;
> --
> 2.37.1
>

Regards,
Simon


Re: [PATCH 17/26] spl: Use map_sysmem where appropriate

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:
>
> All "physical" addresses in SPL must be converted to virtual addresses
> before access in order for sandbox to work. Add some calls to map_sysmem in
> appropriate places. We do not generally call unmap_sysmem, since we need
> the image memory to still be mapped when we jump to the image. This doesn't
> matter at the moment since unmap_sysmem is a no-op.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  common/spl/spl.c   |  4 +++-
>  common/spl/spl_blk_fs.c|  6 --
>  common/spl/spl_ext.c   |  4 +++-
>  common/spl/spl_fat.c   | 11 +++
>  common/spl/spl_fit.c   | 36 +-
>  common/spl/spl_imx_container.c |  4 +++-
>  common/spl/spl_legacy.c|  6 --
>  common/spl/spl_mmc.c   |  4 +++-
>  common/spl/spl_net.c   | 10 +++---
>  common/spl/spl_nor.c   |  5 +++--
>  common/spl/spl_spi.c   | 14 +
>  11 files changed, 69 insertions(+), 35 deletions(-)

Reviewed-by: Simon Glass 


Re: [PATCH v2 6/9] ufs: Allow mmio registers on the PCI bus

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 06:16, Bin Meng  wrote:
>
> Check if the UFS controller is on the PCI bus, and get its register
> base address accordingly.
>
> Signed-off-by: Bin Meng 
> ---
>
> (no changes since v1)
>
>  drivers/ufs/ufs.c | 10 +-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>

Reviewed-by: Simon Glass 


Re: [PATCH 02/26] spl: nor: Don't allocate header on stack

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:
>
> spl_image_info.name contains a reference to legacy_img_hdr. If we allocate
> the latter on the stack, it will be clobbered after we return. This was
> addressed for NAND back in 06377c5a1fc ("spl: spl_legacy: Fix NAND boot on
> OMAP3 BeagleBoard"), but that commit didn't fix NOR.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  common/spl/spl_nor.c | 11 ---
>  1 file changed, 4 insertions(+), 7 deletions(-)

Reviewed-by: Simon Glass 


Re: [PATCH 25/26] test: spl: Add a test for the NOR load method

2023-10-11 Thread Simon Glass
Hi Sean,

On Wed, 11 Oct 2023 at 18:57, Sean Anderson  wrote:
>
> Add a test for the NOR load method. Since NOR is memory-mapped we can
> substitute a buffer instead. The only major complication is testing LZMA
> decompression.  It's too complex to implement LZMA compression in a test,
> so we just include some pre-compressed data.

How about using the in-tree compression code?

>
> Signed-off-by: Sean Anderson 
> ---
>
>  arch/sandbox/include/asm/spl.h   |   1 +
>  configs/sandbox_noinst_defconfig |   2 +
>  configs/sandbox_spl_defconfig|   2 +
>  include/configs/sandbox.h|   3 +
>  include/test/spl.h   |   5 +
>  test/image/Makefile  |   1 +
>  test/image/spl_load.c| 269 ++-
>  test/image/spl_load_nor.c|  39 +
>  8 files changed, 316 insertions(+), 6 deletions(-)
>  create mode 100644 test/image/spl_load_nor.c

Reviewed-by: Simon Glass 

>
> diff --git a/arch/sandbox/include/asm/spl.h b/arch/sandbox/include/asm/spl.h
> index ab9475567e0..cf16af5278a 100644
> --- a/arch/sandbox/include/asm/spl.h
> +++ b/arch/sandbox/include/asm/spl.h
> @@ -13,6 +13,7 @@ enum {
> BOOT_DEVICE_BOARD,
> BOOT_DEVICE_VBE,
> BOOT_DEVICE_CPGMAC,
> +   BOOT_DEVICE_NOR,
>  };
>
>  /**
> diff --git a/configs/sandbox_noinst_defconfig 
> b/configs/sandbox_noinst_defconfig
> index 57cbadedb7d..085cc30c1e2 100644
> --- a/configs/sandbox_noinst_defconfig
> +++ b/configs/sandbox_noinst_defconfig
> @@ -49,6 +49,7 @@ CONFIG_SPL_FS_EXT4=y
>  CONFIG_SPL_I2C=y
>  CONFIG_SPL_MMC_WRITE=y
>  CONFIG_SPL_NET=y
> +CONFIG_SPL_NOR_SUPPORT=y
>  CONFIG_SPL_RTC=y
>  CONFIG_CMD_CPU=y
>  CONFIG_CMD_LICENSE=y
> @@ -257,6 +258,7 @@ CONFIG_RSA_VERIFY_WITH_PKEY=y
>  CONFIG_TPM=y
>  CONFIG_LZ4=y
>  CONFIG_ZSTD=y
> +CONFIG_SPL_LZMA=y
>  CONFIG_ERRNO_STR=y
>  CONFIG_EFI_CAPSULE_ON_DISK=y
>  CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
> index b578cc8e443..56072b15ad2 100644
> --- a/configs/sandbox_spl_defconfig
> +++ b/configs/sandbox_spl_defconfig
> @@ -41,6 +41,7 @@ CONFIG_SPL_SYS_MALLOC_SIZE=0x400
>  CONFIG_SPL_ENV_SUPPORT=y
>  CONFIG_SPL_FPGA=y
>  CONFIG_SPL_I2C=y
> +CONFIG_SPL_NOR_SUPPORT=y
>  CONFIG_SPL_RTC=y
>  CONFIG_CMD_CPU=y
>  CONFIG_CMD_LICENSE=y
> @@ -249,6 +250,7 @@ CONFIG_TPM=y
>  CONFIG_SPL_CRC8=y
>  CONFIG_LZ4=y
>  CONFIG_ZSTD=y
> +CONFIG_SPL_LZMA=y
>  CONFIG_ERRNO_STR=y
>  CONFIG_SPL_HEXDUMP=y
>  CONFIG_EFI_CAPSULE_ON_DISK=y
> diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
> index 4e5653dc886..2372485c84e 100644
> --- a/include/configs/sandbox.h
> +++ b/include/configs/sandbox.h
> @@ -18,4 +18,7 @@
>  #define CFG_SYS_BAUDRATE_TABLE {4800, 9600, 19200, 38400, 57600,\
> 115200}
>
> +/* Unused but necessary to build */
> +#define CFG_SYS_UBOOT_BASE CONFIG_TEXT_BASE
> +
>  #endif
> diff --git a/include/test/spl.h b/include/test/spl.h
> index cfb52c90855..82325702d4a 100644
> --- a/include/test/spl.h
> +++ b/include/test/spl.h
> @@ -27,12 +27,14 @@ void generate_data(char *data, size_t size, const char 
> *test_name);
>  /**
>   * enum spl_test_image - Image types for testing
>   * @LEGACY: "Legacy" uImages
> + * @LEGACY_LZMA: "Legacy" uImages, LZMA compressed
>   * @IMX8: i.MX8 Container images
>   * @FIT_INTERNAL: FITs with internal data
>   * @FIT_EXTERNAL: FITs with external data
>   */
>  enum spl_test_image {
> LEGACY,
> +   LEGACY_LZMA,
> IMX8,
> FIT_INTERNAL,
> FIT_EXTERNAL,
> @@ -118,6 +120,9 @@ int do_spl_test_load(struct unit_test_state *uts, const 
> char *test_name,
>  static inline bool image_supported(enum spl_test_image type)
>  {
> switch (type) {
> +   case LEGACY_LZMA:
> +   if (!IS_ENABLED(CONFIG_SPL_LZMA))
> +   return false;
> case LEGACY:
> return IS_ENABLED(CONFIG_SPL_LEGACY_IMAGE_FORMAT);
> case IMX8:
> diff --git a/test/image/Makefile b/test/image/Makefile
> index ddbc39bf959..41b29995993 100644
> --- a/test/image/Makefile
> +++ b/test/image/Makefile
> @@ -5,4 +5,5 @@
>  obj-$(CONFIG_SPL_UT_LOAD) += spl_load.o
>  obj-$(CONFIG_SPL_UT_LOAD_FS) += spl_load_fs.o
>  obj-$(CONFIG_SPL_UT_LOAD_NET) += spl_load_net.o
> +obj-$(CONFIG_SPL_NOR_SUPPORT) += spl_load_nor.o
>  obj-$(CONFIG_SPL_UT_LOAD_OS) += spl_load_os.o
> diff --git a/test/image/spl_load.c b/test/image/spl_load.c
> index 06249044f9b..3f69e652630 100644
> --- a/test/image/spl_load.c
> +++ b/test/image/spl_load.c
> @@ -43,6 +43,7 @@ void *board_spl_fit_buffer_addr(ulong fit_size, int 
> sectors, int bl_len)
>
>  /* Local flags for spl_image; start from the "top" to avoid conflicts */
>  #define SPL_IMX_CONTAINER  0x8000
> +#define SPL_COMP_LZMA  0x4000
>
>  void generate_data(char *data, size_t size, const char *test_name)
>  {
> @@ -79,7 +80,8 @@ static size_t create_legacy(void *dst, struct 
> 

Re: [PATCH 03/26] spl: fit: Fix entry point for SPL_LOAD_FIT_FULL

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:
>
> The entry point is not always the same as the load address. Use the value
> of the entrypoint property if it exists and is nonzero (following the
> example of spl_load_simple_fit).
>
> Fixes: 8a9dc16e4d0 ("spl: Add full fitImage support")
> Signed-off-by: Sean Anderson 
> ---
>
>  common/spl/spl_fit.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass 

The check for 0 makes me uneasy, but I can't imagine it being valid in practice.


>
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index ce6b8aa370a..e3cdf8e5c05 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -884,8 +884,10 @@ int spl_load_fit_image(struct spl_image_info *spl_image,
> return ret;
>
> spl_image->size = fw_len;
> -   spl_image->entry_point = fw_data;
> spl_image->load_addr = fw_data;
> +   if (fit_image_get_entry(header, ret, _image->entry_point) ||
> +   !spl_image->entry_point)
> +   spl_image->entry_point = fw_data;
> if (fit_image_get_os(header, ret, _image->os))
> spl_image->os = IH_OS_INVALID;
> spl_image->name = genimg_get_os_name(spl_image->os);
> --
> 2.37.1
>


Re: [PATCH v2 5/9] pci_ids: Add Red Hat vendor and device IDs

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 06:16, Bin Meng  wrote:
>
> Red Hat, Inc. donates a part of its device ID range [1] to QEMU,
> to be used for virtual devices. This commit adds several typical
> devices that are useful in U-Boot.
>
> [1] https://www.qemu.org/docs/master/specs/pci-ids.html
>
> Signed-off-by: Bin Meng 
> ---
>
> (no changes since v1)
>
>  include/pci_ids.h | 7 +++
>  1 file changed, 7 insertions(+)

Reviewed-by: Simon Glass 


Re: [PATCH 14/26] net: bootp: Move port numbers to header

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:
>
> These defines are useful when testing bootp.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  net/bootp.c | 3 ---
>  net/bootp.h | 3 +++
>  2 files changed, 3 insertions(+), 3 deletions(-)
>

Reviewed-by: Simon Glass 


Re: [PATCH 18/26] test: spl: Split tests up and use some configs

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:
>
> In order to make adding new spl unit tests easier, especially when they may
> have many dependencies, add some Kconfigs for the existing image test.
> Split it into the parts which are generic (such as callbacks) and the
> test-specific parts.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  test/Kconfig |  1 +
>  test/Makefile|  5 +--
>  test/image/Kconfig   | 20 ++
>  test/image/Makefile  |  3 +-
>  test/image/spl_load.c| 76 +
>  test/image/spl_load_os.c | 81 
>  6 files changed, 107 insertions(+), 79 deletions(-)
>  create mode 100644 test/image/Kconfig
>  create mode 100644 test/image/spl_load_os.c
>

Reviewed-by: Simon Glass 


Re: [PATCH 00/26] test: spl: Test some load methods

2023-10-11 Thread Simon Glass
Hi Sean,

On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:
>
> This series adds some tests for various SPL load methods, with the intent of
> helping debug v6 of [1]. With that in mind, notable omissions include NAND and
> ROMAPI, which both lack sandbox implementations, and OS_BOOT, which I have
> deferred due to its complexity. Semihosting is also omitted, but I think we 
> can
> test that with qemu.
>
> In order to test all of these methods, we must first generate suitable images,
> possibly on filesystems. While other tests have historically generated these
> images using external tools (e.g. mkimage, mkfs, etc.), I have chosen to
> generate them on the fly. This is for a few reasons:
>
> - By removing external dependencies on pytest to create certain files, the 
> tests
>   become self-contained. This makes them easier to iterate on and debug.
> - By generating tests at runtime, we can dynamically vary the content. This
>   helps detect test failures, as even if tests are loaded to the same 
> location,
>   the expected content will be different.
> - We are not testing the image parsers themselves (e.g. spl_load_simple_fit or
>   fs_read) but rather the load methods (e.g. spl_mmc_load_image). It is
>   unnecessary to exercise full functionality or generate 100% correct images.
> - By reducing functionality to only what is necessary, the complexity of 
> various
>   formats can often be greatly reduced.

This makes sense to me. Also the code to generate the images is
relatively small.

>
> This series depends on [2-3], which are small fixes identified through this
> patch set. The organization of patches in this series is as follows:
>
> - General fixes for bugs which are unlikely to be triggered outside of this
>   series
> - Changes to IMX8 container images to facilitate testing
> - General prep. work, particularly regarding linker issues
> - The tests themselves
>
> Mostly-passing CI at [4]; I have since fixed the typo/missing cast.
>
> [1] 
> https://lore.kernel.org/all/20230731224304.111081-1-sean.ander...@seco.com/
> [2] https://lore.kernel.org/all/20230930204246.515254-1-sean...@gmail.com/
> [3] https://lore.kernel.org/all/20231008014748.1987840-1-sean...@gmail.com/
> [4] https://source.denx.de/u-boot/custodians/u-boot-clk/-/pipelines/18091
>
>
> Sean Anderson (26):
>   spl: legacy: Fix referencing _image_binary_end
>   spl: nor: Don't allocate header on stack
>   spl: fit: Fix entry point for SPL_LOAD_FIT_FULL
>   arm: imx: Fix i.MX8 container load address
>   arm: imx: Add newlines after error messages
>   arm: imx: Add function to validate i.MX8 containers
>   arm: imx: Check header before calling spl_load_imx_container
>   Move i.MX8 container image loading support to common/spl
>   spl: Allow enabling SPL_OF_REAL and SPL_OF_PLATDATA at the same time
>   lib: acpi: Fix linking SPL when ACPIGEN is enabled
>   fs: ext4: Fix building ext4 in SPL if write is enabled
>   fs: Compile in sandbox filesystem in SPL if it is enabled
>   net: Fix compiling SPL when fastboot is enabled
>   net: bootp: Move port numbers to header
>   net: bootp: Fall back to BOOTP from DHCP when unit testing
>   spl: Don't cache devices when UNIT_TEST is enabled
>   spl: Use map_sysmem where appropriate
>   test: spl: Split tests up and use some configs
>   test: spl: Fix spl_test_load not failing if fname doesn't exist
>   test: spl: Add functions to create images
>   test: spl: Add functions to create filesystems
>   test: spl: Add a test for spl_blk_load_image
>   test: spl: Add a test for the MMC load method
>   test: spl: Add a test for the NET load method
>   test: spl: Add a test for the NOR load method
>   test: spl: Add a test for the SPI load method
>

Reviewed-by: Simon Glass 

Yeoman's work, that.

Regards,
Simon


Re: [PATCH 26/26] test: spl: Add a test for the SPI load method

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 18:57, Sean Anderson  wrote:
>
> Add test for the SPI load method. This one is pretty straightforward. We
> can't enable FIT_EXTERNAL with LOAD_FIT_FULL because spl_spi_load_image
> doesn't know the total image size and has to guess from fdt_totalsize. This
> doesn't include external data, so loading it will fail.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  arch/sandbox/include/asm/spl.h   |  1 +
>  configs/sandbox_noinst_defconfig |  6 +
>  include/spl.h| 10 
>  test/image/Kconfig   |  8 +++
>  test/image/Makefile  |  1 +
>  test/image/spl_load.c|  1 +
>  test/image/spl_load_spi.c| 41 
>  test/py/tests/test_spl.py| 10 
>  8 files changed, 78 insertions(+)
>  create mode 100644 test/image/spl_load_spi.c

Reviewed-by: Simon Glass 


Re: [PATCH v4 2/8] binman: ftest: Add test for ti-secure firewall node

2023-10-11 Thread Simon Glass
On Tue, 10 Oct 2023 at 23:25, Manorit Chawdhry  wrote:
>
> Add test for TI firewalling node in ti-secure.
>
> Signed-off-by: Manorit Chawdhry 
> ---
>  tools/binman/ftest.py  | 22 +
>  tools/binman/test/319_ti_secure_firewall.dts   | 28 
> ++
>  .../320_ti_secure_firewall_missing_property.dts| 28 
> ++
>  3 files changed, 78 insertions(+)

Reviewed-by: Simon Glass 


Re: [PATCH v2 4/9] cmd: ufs: Correct the help text

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 06:16, Bin Meng  wrote:
>
> Remove the additional space and use "sub-system" for consistency
> with other commands like "scsi" and "usb".
>
> Signed-off-by: Bin Meng 
> ---
>
> (no changes since v1)
>
>  cmd/ufs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass 


Re: [PATCH v2 9/9] qemu: riscv: Enable UFS support

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 06:16, Bin Meng  wrote:
>
> This enables UFS support for QEMU RISC-V 'virt' machine.
>
> Signed-off-by: Bin Meng 
>
> ---
>
> (no changes since v1)
>
>  board/emulation/qemu-riscv/Kconfig | 2 ++
>  doc/board/emulation/qemu-riscv.rst | 8 +++-
>  2 files changed, 9 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass 


Re: [PATCH v2 7/9] ufs: Add a PCI based UFS controller driver

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 06:16, Bin Meng  wrote:
>
> This adds a simple PCI based UFS controller driver with a QEMU
> emulated UFS controller on the PCI bus.
>
> Requiring QEMU v8.2+.
>
> Signed-off-by: Bin Meng 
>
> ---
>
> Changes in v2:
> - fix a build warning
>
>  drivers/ufs/Kconfig   | 11 +++
>  drivers/ufs/Makefile  |  1 +
>  drivers/ufs/ufs-pci.c | 45 +++
>  3 files changed, 57 insertions(+)
>  create mode 100644 drivers/ufs/ufs-pci.c

Reviewed-by: Simon Glass 


Re: [PATCH v2 8/9] ufs: Handle UFS 3.1 controllers

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 06:16, Bin Meng  wrote:
>
> Extend the version check to handle UFS 3.1 controllers as well.
> Tested on QEMU emulated UFS 3.1 controller.
>
> Signed-off-by: Bin Meng 
> ---
>
> (no changes since v1)
>
>  drivers/ufs/ufs.c | 3 ++-
>  drivers/ufs/ufs.h | 1 +
>  2 files changed, 3 insertions(+), 1 deletion(-)
>

Reviewed-by: Simon Glass 


Re: [PATCH 12/26] fs: Compile in sandbox filesystem in SPL if it is enabled

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:
>
> fs.c thinks that the sandbox filesystem is available if SANDBOX is enabled,
> but it is not in SPL. Compile it in SPL to avoid linker errors.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  fs/Makefile | 1 +
>  1 file changed, 1 insertion(+)
>

Reviewed-by: Simon Glass 


Re: [PATCH v2 3/6] binman: capsule: Remove superfluous [address,size]-cells properties

2023-10-11 Thread Simon Glass
On Tue, 10 Oct 2023 at 02:12, Sughosh Ganu  wrote:
>
> The #address-cells and #size-cells are not needed for running the
> capsule generation binman tests. Remove the superfluous properties.
>
> Signed-off-by: Sughosh Ganu 
> ---
> Changes since V1:
> * New patch based on review comment from Simon.
>
>  tools/binman/test/311_capsule.dts   | 3 ---
>  tools/binman/test/312_capsule_signed.dts| 3 ---
>  tools/binman/test/313_capsule_version.dts   | 3 ---
>  tools/binman/test/314_capsule_signed_ver.dts| 3 ---
>  tools/binman/test/315_capsule_oemflags.dts  | 3 ---
>  tools/binman/test/316_capsule_missing_key.dts   | 3 ---
>  tools/binman/test/317_capsule_missing_index.dts | 3 ---
>  tools/binman/test/318_capsule_missing_guid.dts  | 3 ---
>  8 files changed, 24 deletions(-)
>

Reviewed-by: Simon Glass 


Re: [PATCH v2 4/6] binman: capsule: Use dumped capsule header contents for verification

2023-10-11 Thread Simon Glass
On Tue, 10 Oct 2023 at 02:12, Sughosh Ganu  wrote:
>
> The various fields of a generated capsule are currently verified
> through hard-coded offsets. Use the dump-capsule feature for dumping
> the capsule header contents and use those for capsule verification.
>
> Signed-off-by: Sughosh Ganu 
> ---
> Changes since V1:
> * Move the get_binman_test_guid() function outside the
>   Entry_efi_capsule class so that it can be called from outside the
>   module.
> * Use lowercase characters in the GUID values
> * Add comments for the _GetCapsuleHeaders() function.
> * Use a simple dict in _GetCapsuleHeaders() for storing the capsule
>   header values dumped by the mkeficapsule tool.
>
>  tools/binman/etype/efi_capsule.py |  24 +--
>  tools/binman/ftest.py | 105 ++
>  2 files changed, 82 insertions(+), 47 deletions(-)

Reviewed-by: Simon Glass 


Re: [PATCH v2 1/9] ufs: Correct the UFS terminlogy

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 06:16, Bin Meng  wrote:
>
> UFS stands for Universal Flash Storage, not Subsytem.
>
> Signed-off-by: Bin Meng 
> ---
>
> (no changes since v1)
>
>  cmd/Kconfig  | 2 +-
>  drivers/ufs/ufs-uclass.c | 2 +-
>  drivers/ufs/ufs.c| 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass 


Re: [PATCH v2 3/9] cmd: kconfig: Make ufs prompt look similar to other commands

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 06:16, Bin Meng  wrote:
>
> At present the 'ufs' command prompt does not look similar like other
> commands. Update it.
>
> Signed-off-by: Bin Meng 
> ---
>
> (no changes since v1)
>
>  cmd/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Simon Glass 


Re: [PATCH v2 2/9] ufs: Add a line feed to the end of some dev_xxx() messages

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 06:16, Bin Meng  wrote:
>
> Add a line feed to improve readability of some dev_xxx() messages.
>
> Signed-off-by: Bin Meng 
> ---
>
> (no changes since v1)
>
>  drivers/ufs/ufs.c | 16 
>  1 file changed, 8 insertions(+), 8 deletions(-)

Reviewed-by: Simon Glass 


Re: [PATCH 24/26] test: spl: Add a test for the NET load method

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:
>
> Add a test for loading U-Boot over TFTP. As with other sandbox net
> routines, we need to initialize our packets manually since things like
> net_set_ether and net_set_udp_header always use "our" addresses. We use
> BOOTP instead of DHCP, since DHCP has a tag/length-based format which is
> harder to parse. Our TFTP implementation doesn't define as many constants
> as I'd like, so I create some here. Note that the TFTP block size is
> one-based, but offsets are zero-based.
>
> In order to avoid address errors, we need to set up/define some additional
> address information settings. dram_init_banksize would be a good candidate
> for settig up bi_dram, but it gets called too late in board_init_r.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  arch/sandbox/cpu/spl.c   |   3 +
>  arch/sandbox/include/asm/spl.h   |   1 +
>  configs/sandbox_noinst_defconfig |   6 +-
>  test/image/Kconfig   |   9 ++
>  test/image/Makefile  |   1 +
>  test/image/spl_load_net.c| 252 +++
>  6 files changed, 271 insertions(+), 1 deletion(-)
>  create mode 100644 test/image/spl_load_net.c
>

Reviewed-by: Simon Glass 

> diff --git a/arch/sandbox/cpu/spl.c b/arch/sandbox/cpu/spl.c
> index 09e3d10d6a5..8153df18d68 100644
> --- a/arch/sandbox/cpu/spl.c
> +++ b/arch/sandbox/cpu/spl.c
> @@ -126,6 +126,9 @@ void spl_board_init(void)
>  {
> struct sandbox_state *state = state_get_current();
>
> +   gd->bd->bi_dram[0].start = gd->ram_base;
> +   gd->bd->bi_dram[0].size = get_effective_memsize();

These could use a common as to why they are needed here.

[..]

Regards,
Simon


Re: [PATCH 1/9] ufs: Correct the UFS terminlogy

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 06:05, Bin Meng  wrote:
>
> UFS stands for Universal Flash Storage, not Subsytem.
>
> Signed-off-by: Bin Meng 
> ---
>
>  cmd/Kconfig  | 2 +-
>  drivers/ufs/ufs-uclass.c | 2 +-
>  drivers/ufs/ufs.c| 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass 


Re: [PATCH 01/26] spl: legacy: Fix referencing _image_binary_end

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:
>
> On non-arm architectures, _image_binary_end is defined as a ulong and not a
> char[]. Dereference it when accessing it, which is correct for both.

Is 'dereference' the right word?

>
> Fixes: 1b8a1be1a1f ("spl: spl_legacy: Fix spl_end address")
> Signed-off-by: Sean Anderson 
> ---
>
>  common/spl/spl_legacy.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass 


>
> diff --git a/common/spl/spl_legacy.c b/common/spl/spl_legacy.c
> index 095443c63d8..e9564e5c2a5 100644
> --- a/common/spl/spl_legacy.c
> +++ b/common/spl/spl_legacy.c
> @@ -19,7 +19,7 @@
>  static void spl_parse_legacy_validate(uintptr_t start, uintptr_t size)
>  {
> uintptr_t spl_start = (uintptr_t)_start;
> -   uintptr_t spl_end = (uintptr_t)_image_binary_end;
> +   uintptr_t spl_end = (uintptr_t)&_image_binary_end;
> uintptr_t end = start + size;
>
> if ((start >= spl_start && start < spl_end) ||
> --
> 2.37.1
>


Re: [PATCH 09/26] spl: Allow enabling SPL_OF_REAL and SPL_OF_PLATDATA at the same time

2023-10-11 Thread Simon Glass
Hi Sean,

On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:
>
> Sandbox unit tests in U-Boot proper load a test device tree to have some
> devices to work with. In order to do the same in SPL, we must enable
> SPL_OF_REAL. However, we already have SPL_OF_PLATDATA enabled. When
> generating platdata from a devicetree, it is expected that we will not need
> devicetree access functions (even though SPL_OF_CONTROL is enabled). This
> expectation does not hold for sandbox, so allow user control of
> SPL_OF_REAL.
>
> There are several places in the tree where conditions involving OF_PLATDATA
> or OF_REAL no longer function correctly when both of these options can be
> selected at the same time. Adjust these conditions accordingly.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  drivers/core/Makefile   | 1 +
>  drivers/i2c/i2c-emul-uclass.c   | 2 +-
>  drivers/serial/sandbox.c| 2 +-
>  drivers/sysreset/sysreset_sandbox.c | 2 +-
>  dts/Kconfig | 8 +---
>  test/test-main.c| 2 +-
>  6 files changed, 10 insertions(+), 7 deletions(-)

Reviewed-by: Simon Glass 

with a due sense of foreboding

I wonder whether this might create confusion?

>
> diff --git a/drivers/core/Makefile b/drivers/core/Makefile
> index bce0a3f65cb..acbd2bf2cef 100644
> --- a/drivers/core/Makefile
> +++ b/drivers/core/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_$(SPL_)OF_LIVE) += of_access.o of_addr.o
>  ifndef CONFIG_DM_DEV_READ_INLINE
>  obj-$(CONFIG_OF_CONTROL) += read.o
>  endif
> +obj-$(CONFIG_$(SPL_)OF_PLATDATA) += read.o
>  obj-$(CONFIG_OF_CONTROL) += of_extra.o ofnode.o read_extra.o
>
>  ccflags-$(CONFIG_DM_DEBUG) += -DDEBUG
> diff --git a/drivers/i2c/i2c-emul-uclass.c b/drivers/i2c/i2c-emul-uclass.c
> index 1107cf309fc..d421ddfcbe2 100644
> --- a/drivers/i2c/i2c-emul-uclass.c
> +++ b/drivers/i2c/i2c-emul-uclass.c
> @@ -46,7 +46,7 @@ int i2c_emul_find(struct udevice *dev, struct udevice 
> **emulp)
> struct udevice *emul;
> int ret;
>
> -   if (!CONFIG_IS_ENABLED(OF_PLATDATA)) {
> +   if (CONFIG_IS_ENABLED(OF_REAL)) {
> ret = uclass_find_device_by_phandle(UCLASS_I2C_EMUL, dev,
> "sandbox,emul", );
> } else {
> diff --git a/drivers/serial/sandbox.c b/drivers/serial/sandbox.c
> index f4003811ee7..f6ac3d22852 100644
> --- a/drivers/serial/sandbox.c
> +++ b/drivers/serial/sandbox.c
> @@ -280,7 +280,7 @@ U_BOOT_DRIVER(sandbox_serial) = {
> .flags = DM_FLAG_PRE_RELOC,
>  };
>
> -#if CONFIG_IS_ENABLED(OF_REAL)
> +#if CONFIG_IS_ENABLED(OF_REAL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
>  static const struct sandbox_serial_plat platdata_non_fdt = {
> .colour = -1,
>  };
> diff --git a/drivers/sysreset/sysreset_sandbox.c 
> b/drivers/sysreset/sysreset_sandbox.c
> index 3750c60b9b9..f485a135299 100644
> --- a/drivers/sysreset/sysreset_sandbox.c
> +++ b/drivers/sysreset/sysreset_sandbox.c
> @@ -132,7 +132,7 @@ U_BOOT_DRIVER(warm_sysreset_sandbox) = {
> .ops= _warm_sysreset_ops,
>  };
>
> -#if CONFIG_IS_ENABLED(OF_REAL)
> +#if CONFIG_IS_ENABLED(OF_REAL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
>  /* This is here in case we don't have a device tree */
>  U_BOOT_DRVINFO(sysreset_sandbox_non_fdt) = {
> .name = "sysreset_sandbox",
> diff --git a/dts/Kconfig b/dts/Kconfig
> index 9152f5885e9..c6fb193ca89 100644
> --- a/dts/Kconfig
> +++ b/dts/Kconfig
> @@ -410,12 +410,14 @@ config SPL_OF_PLATDATA
>   declarations for each node. See of-plat.txt for more information.
>
>  config SPL_OF_REAL
> -   bool
> +   bool "Support a real devicetree in SPL"

To avoid the user doing something silly, I wonder if it would be
better to keep this option hidden, but enable it for sandbox_spl via
Kconfig?

> +   depends on SPL_OF_CONTROL
> +   select SPL_OF_LIBFDT
> help
>   Indicates that a real devicetree is available which can be accessed
>   at runtime. This means that dev_read_...() functions can be used to
> - read data from the devicetree for each device. This is true if
> - SPL_OF_CONTROL is enabled and not SPL_OF_PLATDATA
> + read data from the devicetree for each device. You do not need to
> + enable this option if you have enabled SPL_OF_PLATDATA.
>
>  if SPL_OF_PLATDATA
>
> diff --git a/test/test-main.c b/test/test-main.c
> index edb20bc4b9c..b7015d9f38d 100644
> --- a/test/test-main.c
> +++ b/test/test-main.c
> @@ -303,7 +303,7 @@ static int test_pre_run(struct unit_test_state *uts, 
> struct unit_test *test)
> if (test->flags & UT_TESTF_PROBE_TEST)
> ut_assertok(do_autoprobe(uts));
>
> -   if (!CONFIG_IS_ENABLED(OF_PLATDATA) &&
> +   if (CONFIG_IS_ENABLED(OF_REAL) &&
> (test->flags & UT_TESTF_SCAN_FDT)) {
> /*
>  * only set this if we know the ethernet uclass will be 
> created
> --
> 2.37.1
>

Regards,

Re: [PATCH] mmc: pci: Drop the superfluous cast

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 04:05, Bin Meng  wrote:
>
> dm_pci_map_bar() return a value of (void *) already, hence no need
> to cast it again before assigning to host->ioaddr.
>
> Signed-off-by: Bin Meng 
> ---
>
>  drivers/mmc/pci_mmc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>

Reviewed-by: Simon Glass 


Re: [PATCH 10/26] lib: acpi: Fix linking SPL when ACPIGEN is enabled

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:
>
> lib/acpi/acpigen.o is only compiled into SPL when SPL_ACPIGEN is enabled.
> Update several files which reference these functions accordingly.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  drivers/core/root.c  | 2 +-
>  drivers/i2c/Makefile | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>

Reviewed-by: Simon Glass 


Re: [PATCH 21/26] test: spl: Add functions to create filesystems

2023-10-11 Thread Simon Glass
Hi Sean,

On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:
>
> Add some functions for creating fat/ext2 filesystems with a single file and
> a test for them. Filesystems require block devices, and it is easiest to
> just use MMC for this. To get an MMC, we must also pull in the test device
> tree.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  arch/sandbox/cpu/start.c |   9 +-
>  configs/sandbox_noinst_defconfig |   7 +
>  include/ext4fs.h |   1 +
>  include/ext_common.h |  14 ++
>  include/test/spl.h   |   3 +
>  test/image/Kconfig   |  11 ++
>  test/image/Makefile  |   1 +
>  test/image/spl_load.c|   1 +
>  test/image/spl_load_fs.c | 305 +++
>  9 files changed, 350 insertions(+), 2 deletions(-)
>  create mode 100644 test/image/spl_load_fs.c

Reviewed-by: Simon Glass 

Wow there is a lot going on in this patch. It might be good to split it.

>
> diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
> index 2c8a72590b5..f5728e6e7ee 100644
> --- a/arch/sandbox/cpu/start.c
> +++ b/arch/sandbox/cpu/start.c
> @@ -13,6 +13,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -202,10 +203,14 @@ static int sandbox_cmdline_cb_test_fdt(struct 
> sandbox_state *state,
>  {
> char buf[256];
> char *fname;
> +   char *relname;
> int len;
>
> -   len = state_get_rel_filename("arch/sandbox/dts/test.dtb", buf,
> -sizeof(buf));
> +   if (spl_phase() < PHASE_BOARD_F)

I think <= PHASE_SPL is better since you care about whether it is in
SPL or not. I did send a patch to check for SPL, but I don't think it
is merged yet.

> +   relname = "../arch/sandbox/dts/test.dtb";
> +   else
> +   relname = "arch/sandbox/dts/test.dtb";
> +   len = state_get_rel_filename(relname, buf, sizeof(buf));
> if (len < 0)
> return len;
>
> diff --git a/configs/sandbox_noinst_defconfig 
> b/configs/sandbox_noinst_defconfig
> index 908155be8a3..0a542cfb6aa 100644
> --- a/configs/sandbox_noinst_defconfig
> +++ b/configs/sandbox_noinst_defconfig
> @@ -6,10 +6,12 @@ CONFIG_NR_DRAM_BANKS=1
>  CONFIG_ENV_SIZE=0x2000
>  CONFIG_DEFAULT_DEVICE_TREE="sandbox"
>  CONFIG_DM_RESET=y
> +CONFIG_SPL_MMC=y
>  CONFIG_SPL_SERIAL=y
>  CONFIG_SPL_DRIVERS_MISC=y
>  CONFIG_SPL_SYS_MALLOC_F_LEN=0x8000
>  CONFIG_SPL=y
> +CONFIG_SPL_FS_FAT=y
>  CONFIG_SYS_LOAD_ADDR=0x0
>  CONFIG_PCI=y
>  CONFIG_SANDBOX_SPL=y
> @@ -39,7 +41,9 @@ CONFIG_SPL_HAS_CUSTOM_MALLOC_START=y
>  CONFIG_SPL_CUSTOM_SYS_MALLOC_ADDR=0xa00
>  CONFIG_SPL_SYS_MALLOC_SIZE=0x400
>  CONFIG_SPL_ENV_SUPPORT=y
> +CONFIG_SPL_FS_EXT4=y
>  CONFIG_SPL_I2C=y
> +CONFIG_SPL_MMC_WRITE=y
>  CONFIG_SPL_RTC=y
>  CONFIG_CMD_CPU=y
>  CONFIG_CMD_LICENSE=y
> @@ -97,6 +101,7 @@ CONFIG_AMIGA_PARTITION=y
>  CONFIG_OF_CONTROL=y
>  CONFIG_SPL_OF_CONTROL=y
>  CONFIG_SPL_OF_PLATDATA=y
> +CONFIG_SPL_OF_REAL=y
>  CONFIG_ENV_IS_NOWHERE=y
>  CONFIG_ENV_IS_IN_EXT4=y
>  CONFIG_ENV_EXT4_INTERFACE="host"
> @@ -159,6 +164,7 @@ CONFIG_CROS_EC_SPI=y
>  CONFIG_P2SB=y
>  CONFIG_PWRSEQ=y
>  CONFIG_SPL_PWRSEQ=y
> +CONFIG_FS_LOADER=y
>  CONFIG_MMC_SANDBOX=y
>  CONFIG_SPI_FLASH_SANDBOX=y
>  CONFIG_SPI_FLASH_ATMEL=y
> @@ -220,6 +226,7 @@ CONFIG_SYSRESET=y
>  CONFIG_SPL_SYSRESET=y
>  CONFIG_DM_THERMAL=y
>  CONFIG_TIMER=y
> +CONFIG_SPL_TIMER=y
>  CONFIG_TIMER_EARLY=y
>  CONFIG_SANDBOX_TIMER=y
>  CONFIG_USB=y

I think these would be better in their own patch

> diff --git a/include/ext4fs.h b/include/ext4fs.h
> index cb5d9cc0a5c..dd66d27f776 100644
> --- a/include/ext4fs.h
> +++ b/include/ext4fs.h
> @@ -31,6 +31,7 @@
>  struct disk_partition;
>
>  #define EXT4_INDEX_FL  0x1000 /* Inode uses hash tree index */
> +#define EXT4_TOPDIR_FL 0x0002 /* Top of directory hierarchies*/
>  #define EXT4_EXTENTS_FL0x0008 /* Inode uses extents */
>  #define EXT4_EXT_MAGIC 0xf30a
>  #define EXT4_FEATURE_RO_COMPAT_GDT_CSUM0x0010
> diff --git a/include/ext_common.h b/include/ext_common.h
> index 30a0c248414..b09bbde116a 100644
> --- a/include/ext_common.h
> +++ b/include/ext_common.h
> @@ -35,6 +35,16 @@ struct cmd_tbl;
>  #define EXT2_PATH_MAX  4096
>  /* Maximum nesting of symlinks, used to prevent a loop.  */
>  #defineEXT2_MAX_SYMLINKCNT 8
> +/* Maximum file name length */
> +#define EXT2_NAME_LEN 255
> +
> +/*
> + * Revision levels
> + */
> +#define EXT2_GOOD_OLD_REV  0   /* The good old (original) format */
> +#define EXT2_DYNAMIC_REV   1   /* V2 format w/ dynamic inode sizes */
> +
> +#define EXT2_GOOD_OLD_INODE_SIZE 128
>
>  /* Filetype used in directory entry.  */
>  #defineFILETYPE_UNKNOWN0
> @@ -48,6 +58,10 @@ struct cmd_tbl;
>  #define FILETYPE_INO_DIRECTORY 004
>  #define 

Re: [PATCH v3 2/3] dt-bindings: mtd: binman-partition: Add binman compatibles

2023-10-11 Thread Simon Glass
Hi Rob,

On Tue, 10 Oct 2023 at 11:06, Rob Herring  wrote:
>
> On Mon, Oct 09, 2023 at 04:02:40PM -0600, Simon Glass wrote:
> > Hi Rob,
> >
> > On Mon, 9 Oct 2023 at 15:18, Rob Herring  wrote:
> > >
> > >
> > > On Mon, 09 Oct 2023 14:10:00 -0600, Simon Glass wrote:
> > > > Add two compatible for binman entries, as a starting point for the
> > > > schema.
> > > >
> > > > Note that, after discussion on v2, we decided to keep the existing
> > > > meaning of label so as not to require changes to existing userspace
> > > > software when moving to use binman nodes to specify the firmware
> > > > layout.
> > > >
> > > > Signed-off-by: Simon Glass 
> > > > ---
> > > >
> > > > Changes in v3:
> > > > - Drop fixed-partitions from the example
> > > > - Use compatible instead of label
> > > >
> > > > Changes in v2:
> > > > - Use plain partition@xxx for the node name
> > > >
> > > >  .../mtd/partitions/binman-partition.yaml  | 48 +++
> > > >  1 file changed, 48 insertions(+)
> > > >  create mode 100644 
> > > > Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml
> > > >
> > >
> > > My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
> > > on your patch (DT_CHECKER_FLAGS is new in v5.13):
> > >
> > > yamllint warnings/errors:
> > >
> > > dtschema/dtc warnings/errors:
> > > /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/mtd/partitions/binman-partition.yaml:
> > >  properties:compatible:items: {'enum': ['u-boot', 'atf-bl31']} is not of 
> > > type 'array'
> > > from schema $id: 
> > > http://devicetree.org/meta-schemas/string-array.yaml#
> > >
> > > doc reference errors (make refcheckdocs):
> > >
> > > See 
> > > https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20231009201005.1964794-2-...@chromium.org
> > >
> > > The base for the series is generally the latest rc1. A different 
> > > dependency
> > > should be noted in *this* patch.
> > >
> > > If you already ran 'make dt_binding_check' and didn't see the above
> > > error(s), then make sure 'yamllint' is installed and dt-schema is up to
> > > date:
> > >
> > > pip3 install dtschema --upgrade
> > >
> > > Please check and re-submit after running the above command yourself. Note
> > > that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> > > your schema. However, it must be unset to test all examples with your 
> > > schema.
> > >
> >
> > Oh dear, I didn't notice that output but I see it now. Could the check
> > return a non-zero exit code if something goes wrong?
>
> No, because things go wrong too often and then it breaks for everyone.
> It's better now, but only because of the above reports and 3
> maintainers.
>
> Also, it is not fatal. The schemas are checked against the DT
> meta-schema, but are used for validation if they pass just draft2019-09
> meta-schema. That allows new DT meta-schema checks to not start
> excluding previously used schema.

OK I see. Another suggestion would be to remove non-warning output.
Perhaps I can do that with make -s though.

Regards,
Simon


Re: [PATCH 22/26] test: spl: Add a test for spl_blk_load_image

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:
>
> Add a test for spl_blk_load_image, currently used only by NVMe. Because
> there is no sandbox NVMe driver, just use MMC instead. Avoid falling back
> to raw images to make failures more obvious.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  configs/sandbox_noinst_defconfig |  2 ++
>  test/image/Kconfig   |  3 +-
>  test/image/spl_load_fs.c | 62 
>  3 files changed, 66 insertions(+), 1 deletion(-)

Reviewed-by: Simon Glass 


Re: [PATCH 13/26] net: Fix compiling SPL when fastboot is enabled

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:
>
> When fastboot is enabled in U-Boot proper and SPL_NET is enabled, we will
> try to (unsuccessfully) reference it in SPL. Fix these linker errors by
> conditioning on SPL_UDP/TCP_FUNCTION_FASTBOOT.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  net/Makefile | 4 ++--
>  net/net.c| 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)
>

Reviewed-by: Simon Glass 


Re: [PATCH 11/26] fs: ext4: Fix building ext4 in SPL if write is enabled

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:
>
> If EXT4_WRITE is enabled, write capabilities will be compiled into SPL, but
> not CRC16. Add an option to enable CRC16 to avoid linker errors.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  common/spl/Kconfig | 1 +
>  lib/Kconfig| 6 ++
>  lib/Makefile   | 1 +
>  3 files changed, 8 insertions(+)

Reviewed-by: Simon Glass 


Re: [PATCH 15/26] net: bootp: Fall back to BOOTP from DHCP when unit testing

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 18:56, Sean Anderson  wrote:
>
> If we sent a DHCP packet and get a BOOTP response from the server, we
> shouldn't try to send a DHCPREQUEST packet, since it won't be DHCPACKed.
> Transition straight to BIND. This is only enabled for UNIT_TEST to avoid
> bloat, since I suspect the number of BOOTP servers in the wild is
> vanishingly small.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  net/bootp.c | 6 ++
>  1 file changed, 6 insertions(+)
>

Reviewed-by: Simon Glass 


Re: [PATCH] net: e1000: Drop e1000_eth_ids[]

2023-10-11 Thread Simon Glass
On Wed, 11 Oct 2023 at 03:59, Bin Meng  wrote:
>
> e1000_eth_ids holds compatible strings for e1000 devices, but it
> is meaningless as e1000 is a PCI device and there is no such
> compatible string assigned to e1000 by the DT bindings community.
>
> Drop it.
>
> Signed-off-by: Bin Meng 
> ---
>
>  drivers/net/e1000.c | 6 --
>  1 file changed, 6 deletions(-)

Reviewed-by: Simon Glass 


Re: [PATCH v4 1/8] binman: ti-secure: Add support for firewalling entities

2023-10-11 Thread Simon Glass
Hi Manorit,

On Tue, 10 Oct 2023 at 23:25, Manorit Chawdhry  wrote:
>
> We can now firewall entities while loading them through our secure
> entity TIFS, the required information should be present in the
> certificate that is being parsed by TIFS.
>
> The following commit adds the support to enable the certificates to be
> generated if the firewall configurations are present in the binman dtsi
> nodes.
>
> Signed-off-by: Manorit Chawdhry 
> ---
>  tools/binman/btool/openssl.py   | 16 +++-
>  tools/binman/etype/ti_secure.py | 90 
> +
>  tools/binman/etype/x509_cert.py |  3 +-
>  3 files changed, 106 insertions(+), 3 deletions(-)
>

Reviewed-by: Simon Glass 

I'm still a little worried about the error reporting if the user
leaves out a property. Does it do the right thing?

Regards,
Simon


Re: [PATCH v2 2/5] configs: npcm: support more uart baud rate

2023-10-11 Thread Jim Liu
Hi Tom

Thanks for your review.
Google and other customers need to change baud rate so I added this table.

I will modify the name to CFG_SYS_BAUDRATE_TABLE.

Best regards,
Jim


On Wed, Oct 11, 2023 at 7:20 AM Tom Rini  wrote:
>
> On Wed, Oct 11, 2023 at 04:45:30PM +0800, Jim Liu wrote:
> > add uart baud rate table to arbel(npcm8xx) and poleg(npcm7xx)
> >
> > Signed-off-by: Jim Liu 
> > ---
> >  include/configs/arbel.h | 5 -
> >  include/configs/poleg.h | 2 ++
> >  2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/configs/arbel.h b/include/configs/arbel.h
> > index 891257bc93..c45b2ac137 100644
> > --- a/include/configs/arbel.h
> > +++ b/include/configs/arbel.h
> > @@ -7,10 +7,13 @@
> >  #define __CONFIG_ARBEL_H
> >
> >  #define CFG_SYS_SDRAM_BASE   0x0
> > -#define CFG_SYS_BOOTMAPSZ(30 << 20)
> > +#define CFG_SYS_BOOTMAPSZ(128 << 20)
> >  #define CFG_SYS_BOOTM_LEN(20 << 20)
> >  #define CFG_SYS_INIT_RAM_ADDRCFG_SYS_SDRAM_BASE
> >  #define CFG_SYS_INIT_RAM_SIZE0x8000
> > +#define CONFIG_SYS_BAUDRATE_TABLE{ 9600, 14400, 19200, 38400, 57600, 
> > 115200, 230400, \
> > +   380400, 460800, 921600 }
> > +
> >
> >  /* Default environemnt variables */
> >  #define CFG_EXTRA_ENV_SETTINGS   "uimage_flash_addr=8040\0"   \
> > diff --git a/include/configs/poleg.h b/include/configs/poleg.h
> > index 1e96e838be..fd0e9a7362 100644
> > --- a/include/configs/poleg.h
> > +++ b/include/configs/poleg.h
> > @@ -13,6 +13,8 @@
> >  #define CFG_SYS_BOOTMAPSZ(0x30 << 20)
> >  #define CFG_SYS_SDRAM_BASE   0x0
> >
> > +#define CONFIG_SYS_BAUDRATE_TABLE{ 57600, 115200, 230400, 460800 }
> > +
> >  /* Default environemnt variables */
> >  #define CFG_EXTRA_ENV_SETTINGS   "uimage_flash_addr=8020\0"   \
> >   "stdin=serial\0"   \
>
> This is now CFG_SYS_BAUDRATE_TABLE, do you really need something other
> than the default as well?
>
> --
> Tom


Re: [PATCH 5/4] mkimage: update man page and -h output

2023-10-11 Thread Simon Glass
Hi,

On Wed, 11 Oct 2023 at 12:33, Tom Rini  wrote:
>
> On Wed, Oct 11, 2023 at 09:07:11PM +0200, Rasmus Villemoes wrote:
> > On 11/10/2023 20.37, Tom Rini wrote:
> > > On Thu, Sep 28, 2023 at 10:02:57AM +0200, Rasmus Villemoes wrote:
> > >
> > >> The man page correctly said that -B was ignored without -E, while the
> > >> `mkimage -h` output suggested otherwise. Now that -B can actually be
> > >> used by itself, update the man page.
> > >>
> > >> While at it, also amend the `mkimage -h` line to mention the
> > >> connection with -E.
> > >>
> > >> The FDT header is a fixed 40 bytes, so its size cannot (and is not)
> > >> modified, while its alignment is a property of the address in RAM one
> > >> loads the FIT to, so not something mkimage can affect in any way. (In
> > >> the file itself, the header is of course at offset 0, which has all
> > >> possible alignments already.)
> > >>
> > >> Reported-by: Sean Anderson 
> > >> Signed-off-by: Rasmus Villemoes 
> > >> Reviewed-by: Simon Glass 
> > >
> > > Applied to u-boot/master, thanks!
> > >
> >
> > Thanks, but I'm afraid that was premature.
> >
> > The original series which this was a fixup/followup for hasn't been
> > applied, and Sean had reservations. I'm leaving it to Simon or Tom or
> > whoever has final say to decide if they should eventually go in, but it
> > would probably be good to get a verdict soonish (it really shouldn't be
> > too controversial), and if it's a no, this should just be reverted.
>
> Ugh, thanks.  I'll push the revert with the next round of changes.

My intent was to apply it, but I haven't got to it yet.

Regards,
Simon


Re: [PATCH] ARM: stm32: Power cycle Buck3 in reset on DHSOM

2023-10-11 Thread Marek Vasut

On 8/16/23 15:28, Marek Vasut wrote:

On 8/16/23 15:22, Patrice CHOTARD wrote:



On 7/10/23 23:43, Marek Vasut wrote:

On 6/17/23 02:36, Marek Vasut wrote:

On 6/16/23 15:04, Patrick DELAUNAY wrote:

Hi,


Hi,


[   39.426015] Disabling non-boot CPUs ...
[   39.448635] Retrying again to check for CPU kill
[   39.451909] CPU1 killed.
U-Boot SPL 2023.07-rc4-8-g2f4664f5c3e (Jun 15 2023 - 08:36:52 
+0200)

RAM: DDR3-DDR3L 32bits 533000kHz
DDR invalid size : 0x4, expected 0x4000
DRAM init failed: -22
### ERROR ### Please RESET the board ###

Press RESET button

U-Boot SPL 2023.07-rc4-8-g2f4664f5c3e (Jun 15 2023 - 08:36:52 
+0200)

RAM: DDR3-DDR3L 32bits 533000kHz
DDR invalid size : 0x4, expected 0x4000
DRAM init failed: -22
### ERROR ### Please RESET the board ###



I try it with the latest STMicroelectronics OSS image.

I just change in U-Boot config to be aligned the expected SD-Card 
partionning


configs/stm32mp15_basic_defconfig:

-CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION=3
+CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_PARTITION=5

But low power is not supported in this downstream config :-<


Use multi_v7_defconfig or some such ?


I got the error:


...
U-Boot SPL 2023.07-rc4-8-g2f4664f5c3ed-dirty (Jun 16 2023 - 
11:37:52 +0200)

RAM: DDR3-DDR3L 32bits 533000kHz
WDT:   Started watchdog@5a002000 with servicing every 1000ms (32s 
timeout)

image entry point: 0xc010


U-Boot 2023.07-rc4-8-g2f4664f5c3ed-dirty (Jun 16 2023 - 
11:37:52 +0200)


CPU: STM32MP157FAA Rev.Z
Model: STMicroelectronics STM32MP157C eval daughter on eval mother
Board: stm32mp1 in basic mode (st,stm32mp157c-ev1)
Board: MB1263 Var4.0 Rev.C-03
DRAM:  1 GiB
Clocks:
- MPU : 800 MHz
- MCU : 208.878 MHz
- AXI : 266.500 MHz
- PER : 24 MHz
- DDR : 533 MHz
Core:  288 devices, 42 uclasses, devicetree: separate
WDT:   Started watchdog@5a002000 with servicing every 1000ms (32s 
timeout)

NAND:  1024 MiB
MMC:   STM32 SD/MMC: 0, STM32 SD/MMC: 1
Loading Environment from MMC... Invalid ENV offset in MMC, copy=0
In:    serial
Out:   serial
Err:   serial
Net:   eth0: ethernet@5800a000
Hit any key to stop autoboot:  0


[    0.00] Booting Linux on physical CPU 0x0
[    0.00] Linux version 6.4.0-rc6 (oe-user@oe-host) 
(arm-ostl-linux-gnueabi-gcc (GCC) 12.2.0, GNU ld (GNU Binutils) 
2.40.20230119) #1 SMP PREEMPT Sun Jun 11 21:35:30 UTC 2023


root@stm32mp1-disco-oss:~# while true ; do rtcwake -s 100 -m mem ; 
done

rtcwake: unrecognized suspend state 'mem'


Please fix your kernel config and enable suspend to mem, I am sure 
that is not difficult.



I check also with downstream (OpenSTLinux V4.0),


This is not relevant to this discussion.

and I can't reproduced the issue but we are using TF-A  / OP-TEE / 
SCMI to support all the low power modes.



And this low power support (in TF-A/ OP-TEE / Linux with SCMI) is 
not yet up streamed.



PS: if you are not able to restart even after a RESET,
    I assume something is wrong in the PMIC configuration

    (for example in NVM or in initial regulator configuration)

    so you have no power cycle on DDR during reset...

         => something is wrong in PMIC configuration in linux ?


Possibly, but then it is also something wrong on STM32MP157C EV1, 
because I can reproduce the failure on EV1 too. I specifically did 
check this on the EV1. Please fix your kernel config and try again, 
then you should be able to see it yourself.


Has there been any news on this defect of EV1 or has this been 
ignored by ST ?


Hi Marek


Hi,


Sorry for the delay,


No worries.

What I am more concerned about is -- why is this problem present on EV1 
too and how to solve it there ? (and no, "add more unnecessary software 
to the stack to cover this up" is not the answer)


I ran into this defect again, it seems the EV1 problem is ignored by ST, 
or are there any news ?


Re: [PATCH 4/4] usb: dwc3-generic: Use combined glue and ctrl node for RK3588

2023-10-11 Thread Kever Yang



On 2023/10/11 06:23, Jonas Karlman wrote:

Like Rockchip RK3328 and RK3568, the RK3588 also have single node to
represent the glue and ctrl for USB 3.0.

Use rk_ops as driver data to select correct ctrl node for RK3588 DWC3.

Signed-off-by: Jonas Karlman 

Reviewed-by: Kever Yang 

Thanks,
- Kever

---
  drivers/usb/dwc3/dwc3-generic.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
index 744fde806948..6fb2de8a5ace 100644
--- a/drivers/usb/dwc3/dwc3-generic.c
+++ b/drivers/usb/dwc3/dwc3-generic.c
@@ -610,6 +610,7 @@ static const struct udevice_id dwc3_glue_ids[] = {
{ .compatible = "rockchip,rk3328-dwc3", .data = (ulong)_ops },
{ .compatible = "rockchip,rk3399-dwc3" },
{ .compatible = "rockchip,rk3568-dwc3", .data = (ulong)_ops },
+   { .compatible = "rockchip,rk3588-dwc3", .data = (ulong)_ops },
{ .compatible = "qcom,dwc3" },
{ .compatible = "fsl,imx8mp-dwc3", .data = (ulong)_ops },
{ .compatible = "fsl,imx8mq-dwc3" },


Re: [PATCH 5/4] mkimage: update man page and -h output

2023-10-11 Thread Sean Anderson

Hi Rasmus,

On 10/11/23 15:07, Rasmus Villemoes wrote:

On 11/10/2023 20.37, Tom Rini wrote:

On Thu, Sep 28, 2023 at 10:02:57AM +0200, Rasmus Villemoes wrote:


The man page correctly said that -B was ignored without -E, while the
`mkimage -h` output suggested otherwise. Now that -B can actually be
used by itself, update the man page.

While at it, also amend the `mkimage -h` line to mention the
connection with -E.

The FDT header is a fixed 40 bytes, so its size cannot (and is not)
modified, while its alignment is a property of the address in RAM one
loads the FIT to, so not something mkimage can affect in any way. (In
the file itself, the header is of course at offset 0, which has all
possible alignments already.)

Reported-by: Sean Anderson 
Signed-off-by: Rasmus Villemoes 
Reviewed-by: Simon Glass 


Applied to u-boot/master, thanks!



Thanks, but I'm afraid that was premature.

The original series which this was a fixup/followup for hasn't been
applied, and Sean had reservations. I'm leaving it to Simon or Tom or
whoever has final say to decide if they should eventually go in, but it
would probably be good to get a verdict soonish (it really shouldn't be
too controversial), and if it's a no, this should just be reverted.


I was hoping you would respond to my most-recent email regarding this series.
In particular:

| Why does mkimage have to do this? Can't you just use truncate or, in a
| binman context, align-size?

Presumably you have some reason for wanting this in mkimage rather than using
existing tooling.

--Sean


Re: [PATCH 2/4] rockchip: rk3588-rock-5b: Enable support for PCIe SATA cards

2023-10-11 Thread Kever Yang



On 2023/10/11 06:23, Jonas Karlman wrote:

Enable support for PCIe SATA cards and the on-board SATA controller.

This also revert use of CONFIG_PCI_INIT_R in order to speed up boot from
eMMC or SD-cards. Standard boot will initialize pci after faster boot
media have been enumerated.

Signed-off-by: Jonas Karlman 

Reviewed-by: Kever Yang 

Thanks,
- Kever

---
Cc: Christopher Obbard 
---
  configs/rock5b-rk3588_defconfig | 6 +-
  1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/configs/rock5b-rk3588_defconfig b/configs/rock5b-rk3588_defconfig
index 447913faccc4..ec22e70033a0 100644
--- a/configs/rock5b-rk3588_defconfig
+++ b/configs/rock5b-rk3588_defconfig
@@ -26,6 +26,7 @@ CONFIG_SPL_SPI=y
  CONFIG_SYS_LOAD_ADDR=0xc00800
  CONFIG_PCI=y
  CONFIG_DEBUG_UART=y
+CONFIG_AHCI=y
  CONFIG_FIT=y
  CONFIG_FIT_VERBOSE=y
  CONFIG_SPL_FIT_SIGNATURE=y
@@ -35,7 +36,6 @@ CONFIG_OF_BOARD_SETUP=y
  CONFIG_DEFAULT_FDT_FILE="rockchip/rk3588-rock-5b.dtb"
  # CONFIG_DISPLAY_CPUINFO is not set
  CONFIG_DISPLAY_BOARDINFO_LATE=y
-CONFIG_PCI_INIT_R=y
  CONFIG_SPL_MAX_SIZE=0x4
  CONFIG_SPL_PAD_TO=0x7f8000
  CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
@@ -63,6 +63,8 @@ CONFIG_OF_SPL_REMOVE_PROPS="clock-names interrupt-parent 
assigned-clocks assigne
  CONFIG_SPL_DM_SEQ_ALIAS=y
  CONFIG_SPL_REGMAP=y
  CONFIG_SPL_SYSCON=y
+CONFIG_AHCI_PCI=y
+CONFIG_DWC_AHCI=y
  CONFIG_SPL_CLK=y
  # CONFIG_USB_FUNCTION_FASTBOOT is not set
  CONFIG_ROCKCHIP_GPIO=y
@@ -89,6 +91,8 @@ CONFIG_SPL_PINCTRL=y
  CONFIG_REGULATOR_PWM=y
  CONFIG_PWM_ROCKCHIP=y
  CONFIG_SPL_RAM=y
+CONFIG_SCSI=y
+CONFIG_DM_SCSI=y
  CONFIG_BAUDRATE=150
  CONFIG_DEBUG_UART_SHIFT=2
  CONFIG_SYS_NS16550_MEM32=y


Re: [PATCH 1/4] rockchip: rk3588: Sync device tree from linux maintainer tree

2023-10-11 Thread Kever Yang



On 2023/10/11 06:23, Jonas Karlman wrote:

Sync rk3588 device tree from linux maintainer tree (v6.7-armsoc/dts64).
Adds PCIe nodes to rk3588-evb1-v10 and rk3588-rock-5b boards. Also
remove includes from u-boot.dtsi-files that is no longer needed.

Linux commits:
42145b7a8235 ("arm64: dts: rockchip: add PCIe network controller to rock-5b")
199cbd5f195a ("arm64: dts: rockchip: add PCIe for M.2 M-key to rock-5b")
da447ec38780 ("arm64: dts: rockchip: add PCIe for M.2 E-Key to rock-5b")
86a2024d95e2 ("arm64: dts: rockchip: add PCIe2 network controller to 
rk3588-evb1")
46bb398ea1d8 ("arm64: dts: rockchip: add PCIe3 bus to rk3588-evb1")
1c9a53ff7ece ("arm64: dts: rockchip: Add sdio node to rock-5b")
3eaf2abd11aa ("arm64: dts: rockchip: Add sfc node to rk3588s")
bf012368bb0a ("arm64: dts: rockchip: Add I2S2 M0 pin definitions to rk3588s")
3d77a3e51b0f ("arm64: dts: rockchip: Add UART9 M0 pin definitions to rk3588s")

Signed-off-by: Jonas Karlman 

Reviewed-by: Kever Yang 

Thanks,
- Kever

---
  arch/arm/dts/rk3588-evb1-v10-u-boot.dtsi |  11 +-
  arch/arm/dts/rk3588-evb1-v10.dts |  98 
  arch/arm/dts/rk3588-rock-5b-u-boot.dtsi  |  60 --
  arch/arm/dts/rk3588-rock-5b.dts  | 140 +++
  arch/arm/dts/rk3588-u-boot.dtsi  |   1 -
  arch/arm/dts/rk3588s-pinctrl.dtsi|  44 +++
  arch/arm/dts/rk3588s-rock-5a-u-boot.dtsi |   4 -
  arch/arm/dts/rk3588s-u-boot.dtsi |  10 --
  arch/arm/dts/rk3588s.dtsi|  11 ++
  9 files changed, 294 insertions(+), 85 deletions(-)

diff --git a/arch/arm/dts/rk3588-evb1-v10-u-boot.dtsi 
b/arch/arm/dts/rk3588-evb1-v10-u-boot.dtsi
index bd2e25948633..e8566785e965 100644
--- a/arch/arm/dts/rk3588-evb1-v10-u-boot.dtsi
+++ b/arch/arm/dts/rk3588-evb1-v10-u-boot.dtsi
@@ -6,16 +6,7 @@
  #include "rk3588-u-boot.dtsi"
  
  / {

-   aliases {
-   mmc0 = 
-   mmc1 = 
-   };
-
chosen {
-   u-boot,spl-boot-order = 
+   u-boot,spl-boot-order = "same-as-spl", 
};
  };
-
- {
-   bootph-all;
-};
diff --git a/arch/arm/dts/rk3588-evb1-v10.dts b/arch/arm/dts/rk3588-evb1-v10.dts
index 229a9111f5eb..c3fe58e39e99 100644
--- a/arch/arm/dts/rk3588-evb1-v10.dts
+++ b/arch/arm/dts/rk3588-evb1-v10.dts
@@ -29,6 +29,46 @@
pwms = < 0 25000 0>;
};
  
+	pcie20_avdd0v85: pcie20-avdd0v85-regulator {

+   compatible = "regulator-fixed";
+   regulator-name = "pcie20_avdd0v85";
+   regulator-always-on;
+   regulator-boot-on;
+   regulator-min-microvolt = <85>;
+   regulator-max-microvolt = <85>;
+   vin-supply = <_0v85_s0>;
+   };
+
+   pcie20_avdd1v8: pcie20-avdd1v8-regulator {
+   compatible = "regulator-fixed";
+   regulator-name = "pcie20_avdd1v8";
+   regulator-always-on;
+   regulator-boot-on;
+   regulator-min-microvolt = <180>;
+   regulator-max-microvolt = <180>;
+   vin-supply = <_1v8_s0>;
+   };
+
+   pcie30_avdd0v75: pcie30-avdd0v75-regulator {
+   compatible = "regulator-fixed";
+   regulator-name = "pcie30_avdd0v75";
+   regulator-always-on;
+   regulator-boot-on;
+   regulator-min-microvolt = <75>;
+   regulator-max-microvolt = <75>;
+   vin-supply = <_0v75_s0>;
+   };
+
+   pcie30_avdd1v8: pcie30-avdd1v8-regulator {
+   compatible = "regulator-fixed";
+   regulator-name = "pcie30_avdd1v8";
+   regulator-always-on;
+   regulator-boot-on;
+   regulator-min-microvolt = <180>;
+   regulator-max-microvolt = <180>;
+   vin-supply = <_1v8_s0>;
+   };
+
vcc12v_dcin: vcc12v-dcin-regulator {
compatible = "regulator-fixed";
regulator-name = "vcc12v_dcin";
@@ -38,6 +78,19 @@
regulator-max-microvolt = <1200>;
};
  
+	vcc3v3_pcie30: vcc3v3-pcie30-regulator {

+   compatible = "regulator-fixed";
+   regulator-name = "vcc3v3_pcie30";
+   regulator-min-microvolt = <330>;
+   regulator-max-microvolt = <330>;
+   enable-active-high;
+   gpios = < RK_PC3 GPIO_ACTIVE_HIGH>;
+   startup-delay-us = <5000>;
+   vin-supply = <_dcin>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_pcie30_en>;
+   };
+
vcc5v0_host: vcc5v0-host-regulator {
compatible = "regulator-fixed";
regulator-name = "vcc5v0_host";
@@ -87,6 +140,10 @@
status = "okay";
  };
  
+_psu {

+   status = "okay";
+};
+
  _b0 {
cpu-supply = <_cpu_big0_s0>;
  };
@@ -163,7 +220,32 @@
};
  };
  
+ {

+   reset-gpios = < RK_PA2 

[PATCH 26/26] test: spl: Add a test for the SPI load method

2023-10-11 Thread Sean Anderson
Add test for the SPI load method. This one is pretty straightforward. We
can't enable FIT_EXTERNAL with LOAD_FIT_FULL because spl_spi_load_image
doesn't know the total image size and has to guess from fdt_totalsize. This
doesn't include external data, so loading it will fail.

Signed-off-by: Sean Anderson 
---

 arch/sandbox/include/asm/spl.h   |  1 +
 configs/sandbox_noinst_defconfig |  6 +
 include/spl.h| 10 
 test/image/Kconfig   |  8 +++
 test/image/Makefile  |  1 +
 test/image/spl_load.c|  1 +
 test/image/spl_load_spi.c| 41 
 test/py/tests/test_spl.py| 10 
 8 files changed, 78 insertions(+)
 create mode 100644 test/image/spl_load_spi.c

diff --git a/arch/sandbox/include/asm/spl.h b/arch/sandbox/include/asm/spl.h
index cf16af5278a..f349ea19971 100644
--- a/arch/sandbox/include/asm/spl.h
+++ b/arch/sandbox/include/asm/spl.h
@@ -14,6 +14,7 @@ enum {
BOOT_DEVICE_VBE,
BOOT_DEVICE_CPGMAC,
BOOT_DEVICE_NOR,
+   BOOT_DEVICE_SPI,
 };
 
 /**
diff --git a/configs/sandbox_noinst_defconfig b/configs/sandbox_noinst_defconfig
index 085cc30c1e2..db05e630832 100644
--- a/configs/sandbox_noinst_defconfig
+++ b/configs/sandbox_noinst_defconfig
@@ -4,6 +4,7 @@ CONFIG_SPL_LIBCOMMON_SUPPORT=y
 CONFIG_SPL_LIBGENERIC_SUPPORT=y
 CONFIG_NR_DRAM_BANKS=1
 CONFIG_ENV_SIZE=0x2000
+CONFIG_SPL_DM_SPI=y
 CONFIG_DEFAULT_DEVICE_TREE="sandbox"
 CONFIG_DM_RESET=y
 CONFIG_SPL_MMC=y
@@ -12,6 +13,8 @@ CONFIG_SPL_DRIVERS_MISC=y
 CONFIG_SPL_SYS_MALLOC_F_LEN=0x8000
 CONFIG_SPL=y
 CONFIG_SPL_FS_FAT=y
+CONFIG_SPL_SPI_FLASH_SUPPORT=y
+CONFIG_SPL_SPI=y
 CONFIG_SYS_LOAD_ADDR=0x100
 CONFIG_PCI=y
 CONFIG_SANDBOX_SPL=y
@@ -48,9 +51,12 @@ CONFIG_SPL_ETH=y
 CONFIG_SPL_FS_EXT4=y
 CONFIG_SPL_I2C=y
 CONFIG_SPL_MMC_WRITE=y
+CONFIG_SPL_DM_SPI_FLASH=y
 CONFIG_SPL_NET=y
 CONFIG_SPL_NOR_SUPPORT=y
 CONFIG_SPL_RTC=y
+# CONFIG_SPL_SPI_FLASH_TINY is not set
+CONFIG_SPL_SPI_LOAD=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
 CONFIG_CMD_BOOTZ=y
diff --git a/include/spl.h b/include/spl.h
index 8229d40adab..cc343ada1ec 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -416,6 +416,16 @@ int spl_load_imx_container(struct spl_image_info 
*spl_image,
 void preloader_console_init(void);
 u32 spl_boot_device(void);
 
+struct spi_flash;
+
+/**
+ * spl_spi_get_uboot_offs() - Lookup function for the SPI boot offset
+ * @flash: The spi flash to boot from
+ *
+ * Return: The offset of U-Boot within the SPI flash
+ */
+unsigned int spl_spi_get_uboot_offs(struct spi_flash *flash);
+
 /**
  * spl_spi_boot_bus() - Lookup function for the SPI boot bus source.
  *
diff --git a/test/image/Kconfig b/test/image/Kconfig
index 99c50787816..8f9e6ae036b 100644
--- a/test/image/Kconfig
+++ b/test/image/Kconfig
@@ -32,6 +32,14 @@ config SPL_UT_LOAD_NET
help
  Test loading images over TFTP using the NET image load method.
 
+config SPL_UT_LOAD_SPI
+   bool "Test loading from SPI Flash"
+   depends on SANDBOX && SPL_OF_REAL
+   depends on SPL_SPI_LOAD
+   default y
+   help
+ Test the SPI flash image load metod.
+
 config SPL_UT_LOAD_OS
bool "Test loading from the host OS"
depends on SANDBOX && SPL_LOAD_FIT
diff --git a/test/image/Makefile b/test/image/Makefile
index 41b29995993..4cb4f96cedc 100644
--- a/test/image/Makefile
+++ b/test/image/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_SPL_UT_LOAD_FS) += spl_load_fs.o
 obj-$(CONFIG_SPL_UT_LOAD_NET) += spl_load_net.o
 obj-$(CONFIG_SPL_NOR_SUPPORT) += spl_load_nor.o
 obj-$(CONFIG_SPL_UT_LOAD_OS) += spl_load_os.o
+obj-$(CONFIG_SPL_UT_LOAD_SPI) += spl_load_spi.o
diff --git a/test/image/spl_load.c b/test/image/spl_load.c
index 3f69e652630..7df411edafd 100644
--- a/test/image/spl_load.c
+++ b/test/image/spl_load.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/test/image/spl_load_spi.c b/test/image/spl_load_spi.c
new file mode 100644
index 000..8f9b6e0139b
--- /dev/null
+++ b/test/image/spl_load_spi.c
@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2023 Sean Anderson 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static int spl_test_spi_write_image(struct unit_test_state *uts, void *img,
+   size_t img_size)
+{
+   struct spi_flash *flash;
+
+   flash = spi_flash_probe(spl_spi_boot_bus(), spl_spi_boot_cs(),
+   CONFIG_SF_DEFAULT_SPEED,
+   CONFIG_SF_DEFAULT_MODE);
+   ut_assertnonnull(flash);
+   ut_assertok(spi_flash_write(flash, spl_spi_get_uboot_offs(flash),
+   img_size, img));
+
+   return 0;
+}
+
+static int spl_test_spi(struct unit_test_state *uts, const char *test_name,
+   enum spl_test_image type)
+{
+   return do_spl_test_load(uts, test_name, type,
+  

[PATCH 18/26] test: spl: Split tests up and use some configs

2023-10-11 Thread Sean Anderson
In order to make adding new spl unit tests easier, especially when they may
have many dependencies, add some Kconfigs for the existing image test.
Split it into the parts which are generic (such as callbacks) and the
test-specific parts.

Signed-off-by: Sean Anderson 
---

 test/Kconfig |  1 +
 test/Makefile|  5 +--
 test/image/Kconfig   | 20 ++
 test/image/Makefile  |  3 +-
 test/image/spl_load.c| 76 +
 test/image/spl_load_os.c | 81 
 6 files changed, 107 insertions(+), 79 deletions(-)
 create mode 100644 test/image/Kconfig
 create mode 100644 test/image/spl_load_os.c

diff --git a/test/Kconfig b/test/Kconfig
index 830245b6f9a..ca648d23376 100644
--- a/test/Kconfig
+++ b/test/Kconfig
@@ -101,6 +101,7 @@ config UT_UNICODE
 
 source "test/dm/Kconfig"
 source "test/env/Kconfig"
+source "test/image/Kconfig"
 source "test/lib/Kconfig"
 source "test/optee/Kconfig"
 source "test/overlay/Kconfig"
diff --git a/test/Makefile b/test/Makefile
index 178773647a8..8e1fed2c28b 100644
--- a/test/Makefile
+++ b/test/Makefile
@@ -3,9 +3,6 @@
 # (C) Copyright 2012 The Chromium Authors
 
 obj-y += test-main.o
-ifdef CONFIG_SPL_LOAD_FIT
-obj-$(CONFIG_SANDBOX) += image/
-endif
 
 ifneq ($(CONFIG_$(SPL_)BLOBLIST),)
 obj-$(CONFIG_$(SPL_)CMDLINE) += bloblist.o
@@ -30,4 +27,6 @@ obj-$(CONFIG_UNIT_TEST) += boot/
 obj-$(CONFIG_UNIT_TEST) += common/
 obj-y += log/
 obj-$(CONFIG_$(SPL_)UT_UNICODE) += unicode_ut.o
+else
+obj-$(CONFIG_SPL_UT_LOAD) += image/
 endif
diff --git a/test/image/Kconfig b/test/image/Kconfig
new file mode 100644
index 000..70ffe0ff276
--- /dev/null
+++ b/test/image/Kconfig
@@ -0,0 +1,20 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright (C) 2023 Sean Anderson 
+
+config SPL_UT_LOAD
+   bool "Unit tests for SPL load methods"
+   depends on SPL_UNIT_TEST
+   default y if SANDBOX
+   help
+ Test various SPL load methods.
+
+if SPL_UT_LOAD
+
+config SPL_UT_LOAD_OS
+   bool "Test loading from the host OS"
+   depends on SANDBOX && SPL_LOAD_FIT
+   default y
+   help
+ Smoke test to ensure that loading U-boot works in sandbox.
+
+endif
diff --git a/test/image/Makefile b/test/image/Makefile
index c4039df707f..1f62d54453c 100644
--- a/test/image/Makefile
+++ b/test/image/Makefile
@@ -2,4 +2,5 @@
 #
 # Copyright 2021 Google LLC
 
-obj-$(CONFIG_SPL_BUILD) += spl_load.o
+obj-$(CONFIG_SPL_UT_LOAD) += spl_load.o
+obj-$(CONFIG_SPL_UT_LOAD_OS) += spl_load_os.o
diff --git a/test/image/spl_load.c b/test/image/spl_load.c
index 4e27ff460ab..1a57bf846d2 100644
--- a/test/image/spl_load.c
+++ b/test/image/spl_load.c
@@ -1,48 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- * Copyright 2021 Google LLC
- * Written by Simon Glass 
+ * Copyright (C) 2023 Sean Anderson 
  */
 
 #include 
-#include 
 #include 
-#include 
-#include 
-#include 
-
-/* Declare a new SPL test */
-#define SPL_TEST(_name, _flags)UNIT_TEST(_name, _flags, 
spl_test)
-
-/* Context used for this test */
-struct text_ctx {
-   int fd;
-};
-
-static ulong read_fit_image(struct spl_load_info *load, ulong sector,
-   ulong count, void *buf)
-{
-   struct text_ctx *text_ctx = load->priv;
-   off_t offset, ret;
-   ssize_t res;
-
-   offset = sector * load->bl_len;
-   ret = os_lseek(text_ctx->fd, offset, OS_SEEK_SET);
-   if (ret != offset) {
-   printf("Failed to seek to %zx, got %zx (errno=%d)\n", offset,
-  ret, errno);
-   return 0;
-   }
-
-   res = os_read(text_ctx->fd, buf, count * load->bl_len);
-   if (res == -1) {
-   printf("Failed to read %lx bytes, got %ld (errno=%d)\n",
-  count * load->bl_len, res, errno);
-   return 0;
-   }
-
-   return count;
-}
 
 int board_fit_config_name_match(const char *name)
 {
@@ -53,39 +15,3 @@ struct legacy_img_hdr *spl_get_load_buffer(ssize_t offset, 
size_t size)
 {
return map_sysmem(0x10, 0);
 }
-
-static int spl_test_load(struct unit_test_state *uts)
-{
-   struct spl_image_info image;
-   struct legacy_img_hdr *header;
-   struct text_ctx text_ctx;
-   struct spl_load_info load;
-   char fname[256];
-   int ret;
-   int fd;
-
-   memset(, '\0', sizeof(load));
-   load.bl_len = 512;
-   load.read = read_fit_image;
-
-   ret = sandbox_find_next_phase(fname, sizeof(fname), true);
-   if (ret) {
-   printf("(%s not found, error %d)\n", fname, ret);
-   return ret;
-   }
-   load.filename = fname;
-
-   header = spl_get_load_buffer(-sizeof(*header), sizeof(*header));
-
-   fd = os_open(fname, OS_O_RDONLY);
-   ut_assert(fd >= 0);
-   ut_asserteq(512, os_read(fd, header, 512));
-   text_ctx.fd = fd;
-
-   load.priv = _ctx;
-
-   

[PATCH 16/26] spl: Don't cache devices when UNIT_TEST is enabled

2023-10-11 Thread Sean Anderson
Several SPL functions try to avoid performing initialization twice by
caching devices. This is fine for regular boot, but does not work with
UNIT_TEST, since all devices are torn down after each test. Disable caching
so we don't use stale devices.

Signed-off-by: Sean Anderson 
---

 common/spl/spl_fat.c | 2 +-
 common/spl/spl_mmc.c | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
index c6e2526ade1..8bec9cce5ca 100644
--- a/common/spl/spl_fat.c
+++ b/common/spl/spl_fat.c
@@ -24,7 +24,7 @@ static int spl_register_fat_device(struct blk_desc 
*block_dev, int partition)
 {
int err = 0;
 
-   if (fat_registered)
+   if (!CONFIG_IS_ENABLED(UNIT_TEST) && fat_registered)
return err;
 
err = fat_register_device(block_dev, partition);
diff --git a/common/spl/spl_mmc.c b/common/spl/spl_mmc.c
index 67c7ae34a58..a8579e29dee 100644
--- a/common/spl/spl_mmc.c
+++ b/common/spl/spl_mmc.c
@@ -417,7 +417,8 @@ int spl_mmc_load(struct spl_image_info *spl_image,
 
/* Perform peripheral init only once for an mmc device */
mmc_dev = spl_mmc_get_device_index(bootdev->boot_device);
-   if (!mmc || spl_mmc_get_mmc_devnum(mmc) != mmc_dev) {
+   if (CONFIG_IS_ENABLED(UNIT_TEST) || !mmc ||
+   spl_mmc_get_mmc_devnum(mmc) != mmc_dev) {
err = spl_mmc_find_device(, bootdev->boot_device);
if (err)
return err;
-- 
2.37.1



[PATCH 20/26] test: spl: Add functions to create images

2023-10-11 Thread Sean Anderson
This add some basic functions to create images, and a test for said
functions. This is not intended to be a test of the image parsing
functions, but rather a framework for creating minimal images for testing
load methods. That said, it does do an OK job at finding bugs in the image
parsing directly.

Since we have two methods for loading/parsing FIT images, add LOAD_FIT_FULL
as a separate CI run.

Signed-off-by: Sean Anderson 
---

 .azure-pipelines.yml |   4 +
 .gitlab-ci.yml   |   7 +
 arch/sandbox/cpu/u-boot-spl.lds  |   2 +
 configs/sandbox_noinst_defconfig |   6 +
 configs/sandbox_spl_defconfig|   6 +
 include/test/spl.h   | 117 ++
 test/image/spl_load.c| 352 +++
 test/image/spl_load_os.c |   5 +-
 8 files changed, 495 insertions(+), 4 deletions(-)
 create mode 100644 include/test/spl.h

diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
index 7985ff5523c..6f91553e861 100644
--- a/.azure-pipelines.yml
+++ b/.azure-pipelines.yml
@@ -299,6 +299,10 @@ stages:
 sandbox_noinst:
   TEST_PY_BD: "sandbox_noinst"
   TEST_PY_TEST_SPEC: "test_ofplatdata or test_handoff or test_spl"
+sandbox_noinst_load_fit_full:
+  TEST_PY_BD: "sandbox_noinst"
+  TEST_PY_TEST_SPEC: "test_ofplatdata or test_handoff or test_spl"
+  OVERRIDE: "-a CONFIG_SPL_LOAD_FIT_FULL=y"
 sandbox_flattree:
   TEST_PY_BD: "sandbox_flattree"
 sandbox_trace:
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 129234ba3db..6decdfdee33 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -293,6 +293,13 @@ sandbox_noinst_test.py:
 TEST_PY_TEST_SPEC: "test_ofplatdata or test_handoff or test_spl"
   <<: *buildman_and_testpy_dfn
 
+sandbox_noinst with LOAD_FIT_FULL test.py:
+  variables:
+TEST_PY_BD: "sandbox_noinst"
+TEST_PY_TEST_SPEC: "test_ofplatdata or test_handoff or test_spl"
+OVERRIDE: "-a CONFIG_SPL_LOAD_FIT_FULL=y"
+  <<: *buildman_and_testpy_dfn
+
 sandbox_vpl test.py:
   variables:
 TEST_PY_BD: "sandbox_vpl"
diff --git a/arch/sandbox/cpu/u-boot-spl.lds b/arch/sandbox/cpu/u-boot-spl.lds
index ef885fd0cb0..a81d66a6f2e 100644
--- a/arch/sandbox/cpu/u-boot-spl.lds
+++ b/arch/sandbox/cpu/u-boot-spl.lds
@@ -26,6 +26,8 @@ SECTIONS
KEEP(*(_u_boot_sandbox_getopt))
*(_u_boot_sandbox_getopt_end)
}
+
+   _image_binary_end = .;
 }
 
 INSERT AFTER .data;
diff --git a/configs/sandbox_noinst_defconfig b/configs/sandbox_noinst_defconfig
index d39e54f98d2..908155be8a3 100644
--- a/configs/sandbox_noinst_defconfig
+++ b/configs/sandbox_noinst_defconfig
@@ -32,6 +32,12 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_SPL_NO_BSS_LIMIT=y
 CONFIG_HANDOFF=y
 CONFIG_SPL_BOARD_INIT=y
+CONFIG_SPL_LEGACY_IMAGE_FORMAT=y
+CONFIG_SPL_LOAD_IMX_CONTAINER=y
+CONFIG_SPL_SYS_MALLOC=y
+CONFIG_SPL_HAS_CUSTOM_MALLOC_START=y
+CONFIG_SPL_CUSTOM_SYS_MALLOC_ADDR=0xa00
+CONFIG_SPL_SYS_MALLOC_SIZE=0x400
 CONFIG_SPL_ENV_SUPPORT=y
 CONFIG_SPL_I2C=y
 CONFIG_SPL_RTC=y
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
index 4a67af2f088..b578cc8e443 100644
--- a/configs/sandbox_spl_defconfig
+++ b/configs/sandbox_spl_defconfig
@@ -32,6 +32,12 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_SPL_NO_BSS_LIMIT=y
 CONFIG_HANDOFF=y
 CONFIG_SPL_BOARD_INIT=y
+CONFIG_SPL_LEGACY_IMAGE_FORMAT=y
+CONFIG_SPL_LOAD_IMX_CONTAINER=y
+CONFIG_SPL_SYS_MALLOC=y
+CONFIG_SPL_HAS_CUSTOM_MALLOC_START=y
+CONFIG_SPL_CUSTOM_SYS_MALLOC_ADDR=0xa00
+CONFIG_SPL_SYS_MALLOC_SIZE=0x400
 CONFIG_SPL_ENV_SUPPORT=y
 CONFIG_SPL_FPGA=y
 CONFIG_SPL_I2C=y
diff --git a/include/test/spl.h b/include/test/spl.h
new file mode 100644
index 000..a2f8d77b88f
--- /dev/null
+++ b/include/test/spl.h
@@ -0,0 +1,117 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright (C) 2023 Sean Anderson 
+ */
+
+#ifndef TEST_SPL_H
+#define TEST_SPL_H
+
+struct unit_test_state;
+struct spl_image_info;
+
+/* Declare a new SPL test */
+#define SPL_TEST(_name, _flags)UNIT_TEST(_name, _flags, 
spl_test)
+
+/**
+ * generate_data() - Generate some test payload data
+ * @data: The location to fill
+ * @size: The size of @data
+ * @test_name: The seed for the data
+ *
+ * Fill @data with data. The upper nibbles will be an incrementing counter
+ * (0x00, 0x10, 0x20...) to make the data identifiable in a hex dump. The lower
+ * nibbles are random bits seeded with @test_name.
+ */
+void generate_data(char *data, size_t size, const char *test_name);
+
+/**
+ * enum spl_test_image - Image types for testing
+ * @LEGACY: "Legacy" uImages
+ * @IMX8: i.MX8 Container images
+ * @FIT_INTERNAL: FITs with internal data
+ * @FIT_EXTERNAL: FITs with external data
+ */
+enum spl_test_image {
+   LEGACY,
+   IMX8,
+   FIT_INTERNAL,
+   FIT_EXTERNAL,
+};
+
+/**
+ * create_image() - Create an image for testing
+ * @dst: The location to create the image at
+ * @type: 

[PATCH 21/26] test: spl: Add functions to create filesystems

2023-10-11 Thread Sean Anderson
Add some functions for creating fat/ext2 filesystems with a single file and
a test for them. Filesystems require block devices, and it is easiest to
just use MMC for this. To get an MMC, we must also pull in the test device
tree.

Signed-off-by: Sean Anderson 
---

 arch/sandbox/cpu/start.c |   9 +-
 configs/sandbox_noinst_defconfig |   7 +
 include/ext4fs.h |   1 +
 include/ext_common.h |  14 ++
 include/test/spl.h   |   3 +
 test/image/Kconfig   |  11 ++
 test/image/Makefile  |   1 +
 test/image/spl_load.c|   1 +
 test/image/spl_load_fs.c | 305 +++
 9 files changed, 350 insertions(+), 2 deletions(-)
 create mode 100644 test/image/spl_load_fs.c

diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
index 2c8a72590b5..f5728e6e7ee 100644
--- a/arch/sandbox/cpu/start.c
+++ b/arch/sandbox/cpu/start.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -202,10 +203,14 @@ static int sandbox_cmdline_cb_test_fdt(struct 
sandbox_state *state,
 {
char buf[256];
char *fname;
+   char *relname;
int len;
 
-   len = state_get_rel_filename("arch/sandbox/dts/test.dtb", buf,
-sizeof(buf));
+   if (spl_phase() < PHASE_BOARD_F)
+   relname = "../arch/sandbox/dts/test.dtb";
+   else
+   relname = "arch/sandbox/dts/test.dtb";
+   len = state_get_rel_filename(relname, buf, sizeof(buf));
if (len < 0)
return len;
 
diff --git a/configs/sandbox_noinst_defconfig b/configs/sandbox_noinst_defconfig
index 908155be8a3..0a542cfb6aa 100644
--- a/configs/sandbox_noinst_defconfig
+++ b/configs/sandbox_noinst_defconfig
@@ -6,10 +6,12 @@ CONFIG_NR_DRAM_BANKS=1
 CONFIG_ENV_SIZE=0x2000
 CONFIG_DEFAULT_DEVICE_TREE="sandbox"
 CONFIG_DM_RESET=y
+CONFIG_SPL_MMC=y
 CONFIG_SPL_SERIAL=y
 CONFIG_SPL_DRIVERS_MISC=y
 CONFIG_SPL_SYS_MALLOC_F_LEN=0x8000
 CONFIG_SPL=y
+CONFIG_SPL_FS_FAT=y
 CONFIG_SYS_LOAD_ADDR=0x0
 CONFIG_PCI=y
 CONFIG_SANDBOX_SPL=y
@@ -39,7 +41,9 @@ CONFIG_SPL_HAS_CUSTOM_MALLOC_START=y
 CONFIG_SPL_CUSTOM_SYS_MALLOC_ADDR=0xa00
 CONFIG_SPL_SYS_MALLOC_SIZE=0x400
 CONFIG_SPL_ENV_SUPPORT=y
+CONFIG_SPL_FS_EXT4=y
 CONFIG_SPL_I2C=y
+CONFIG_SPL_MMC_WRITE=y
 CONFIG_SPL_RTC=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
@@ -97,6 +101,7 @@ CONFIG_AMIGA_PARTITION=y
 CONFIG_OF_CONTROL=y
 CONFIG_SPL_OF_CONTROL=y
 CONFIG_SPL_OF_PLATDATA=y
+CONFIG_SPL_OF_REAL=y
 CONFIG_ENV_IS_NOWHERE=y
 CONFIG_ENV_IS_IN_EXT4=y
 CONFIG_ENV_EXT4_INTERFACE="host"
@@ -159,6 +164,7 @@ CONFIG_CROS_EC_SPI=y
 CONFIG_P2SB=y
 CONFIG_PWRSEQ=y
 CONFIG_SPL_PWRSEQ=y
+CONFIG_FS_LOADER=y
 CONFIG_MMC_SANDBOX=y
 CONFIG_SPI_FLASH_SANDBOX=y
 CONFIG_SPI_FLASH_ATMEL=y
@@ -220,6 +226,7 @@ CONFIG_SYSRESET=y
 CONFIG_SPL_SYSRESET=y
 CONFIG_DM_THERMAL=y
 CONFIG_TIMER=y
+CONFIG_SPL_TIMER=y
 CONFIG_TIMER_EARLY=y
 CONFIG_SANDBOX_TIMER=y
 CONFIG_USB=y
diff --git a/include/ext4fs.h b/include/ext4fs.h
index cb5d9cc0a5c..dd66d27f776 100644
--- a/include/ext4fs.h
+++ b/include/ext4fs.h
@@ -31,6 +31,7 @@
 struct disk_partition;
 
 #define EXT4_INDEX_FL  0x1000 /* Inode uses hash tree index */
+#define EXT4_TOPDIR_FL 0x0002 /* Top of directory hierarchies*/
 #define EXT4_EXTENTS_FL0x0008 /* Inode uses extents */
 #define EXT4_EXT_MAGIC 0xf30a
 #define EXT4_FEATURE_RO_COMPAT_GDT_CSUM0x0010
diff --git a/include/ext_common.h b/include/ext_common.h
index 30a0c248414..b09bbde116a 100644
--- a/include/ext_common.h
+++ b/include/ext_common.h
@@ -35,6 +35,16 @@ struct cmd_tbl;
 #define EXT2_PATH_MAX  4096
 /* Maximum nesting of symlinks, used to prevent a loop.  */
 #defineEXT2_MAX_SYMLINKCNT 8
+/* Maximum file name length */
+#define EXT2_NAME_LEN 255
+
+/*
+ * Revision levels
+ */
+#define EXT2_GOOD_OLD_REV  0   /* The good old (original) format */
+#define EXT2_DYNAMIC_REV   1   /* V2 format w/ dynamic inode sizes */
+
+#define EXT2_GOOD_OLD_INODE_SIZE 128
 
 /* Filetype used in directory entry.  */
 #defineFILETYPE_UNKNOWN0
@@ -48,6 +58,10 @@ struct cmd_tbl;
 #define FILETYPE_INO_DIRECTORY 004
 #define FILETYPE_INO_SYMLINK   012
 #define EXT2_ROOT_INO  2 /* Root inode */
+#define EXT2_BOOT_LOADER_INO   5 /* Boot loader inode */
+
+/* First non-reserved inode for old ext2 filesystems */
+#define EXT2_GOOD_OLD_FIRST_INO11
 
 /* The size of an ext2 block in bytes.  */
 #define EXT2_BLOCK_SIZE(data) (1 << LOG2_BLOCK_SIZE(data))
diff --git a/include/test/spl.h b/include/test/spl.h
index a2f8d77b88f..7ae32a1020b 100644
--- a/include/test/spl.h
+++ b/include/test/spl.h
@@ -114,4 +114,7 @@ SPL_TEST(func##_##type, flags)
 /* More than a couple blocks, and will not be aligned to anything */
 #define 

Re: [PATCH 00/26] test: spl: Test some load methods

2023-10-11 Thread Sean Anderson

On 10/11/23 21:56, Sean Anderson wrote:

This series adds some tests for various SPL load methods, with the intent of
helping debug v6 of [1]. With that in mind, notable omissions include NAND and
ROMAPI, which both lack sandbox implementations, and OS_BOOT, which I have
deferred due to its complexity. Semihosting is also omitted, but I think we can
test that with qemu.

In order to test all of these methods, we must first generate suitable images,
possibly on filesystems. While other tests have historically generated these
images using external tools (e.g. mkimage, mkfs, etc.), I have chosen to
generate them on the fly. This is for a few reasons:

- By removing external dependencies on pytest to create certain files, the tests
   become self-contained. This makes them easier to iterate on and debug.
- By generating tests at runtime, we can dynamically vary the content. This
   helps detect test failures, as even if tests are loaded to the same location,
   the expected content will be different.
- We are not testing the image parsers themselves (e.g. spl_load_simple_fit or
   fs_read) but rather the load methods (e.g. spl_mmc_load_image). It is
   unnecessary to exercise full functionality or generate 100% correct images.
- By reducing functionality to only what is necessary, the complexity of various
   formats can often be greatly reduced.

This series depends on [2-3], which are small fixes identified through this
patch set. The organization of patches in this series is as follows:

- General fixes for bugs which are unlikely to be triggered outside of this
   series
- Changes to IMX8 container images to facilitate testing
- General prep. work, particularly regarding linker issues
- The tests themselves

Mostly-passing CI at [4]; I have since fixed the typo/missing cast.


CI run for the series as-sent:

https://source.denx.de/u-boot/custodians/u-boot-clk/-/pipelines/18092

(hopefully by the time you read this it will have passed)

--Sean


[PATCH 15/26] net: bootp: Fall back to BOOTP from DHCP when unit testing

2023-10-11 Thread Sean Anderson
If we sent a DHCP packet and get a BOOTP response from the server, we
shouldn't try to send a DHCPREQUEST packet, since it won't be DHCPACKed.
Transition straight to BIND. This is only enabled for UNIT_TEST to avoid
bloat, since I suspect the number of BOOTP servers in the wild is
vanishingly small.

Signed-off-by: Sean Anderson 
---

 net/bootp.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/net/bootp.c b/net/bootp.c
index 2053cce88c6..7b0f45e18a9 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -1073,6 +1073,11 @@ static void dhcp_handler(uchar *pkt, unsigned dest, 
struct in_addr sip,
CONFIG_SYS_BOOTFILE_PREFIX,
strlen(CONFIG_SYS_BOOTFILE_PREFIX)) == 0) {
 #endif /* CONFIG_SYS_BOOTFILE_PREFIX */
+   if (CONFIG_IS_ENABLED(UNIT_TEST) &&
+   dhcp_message_type((u8 *)bp->bp_vend) == -1) {
+   debug("got BOOTP response; transitioning to 
BOUND\n");
+   goto dhcp_got_bootp;
+   }
dhcp_packet_process_options(bp);
if (CONFIG_IS_ENABLED(EFI_LOADER) &&
IS_ENABLED(CONFIG_NETDEVICES))
@@ -1099,6 +1104,7 @@ static void dhcp_handler(uchar *pkt, unsigned dest, 
struct in_addr sip,
debug("DHCP State: REQUESTING\n");
 
if (dhcp_message_type((u8 *)bp->bp_vend) == DHCP_ACK) {
+dhcp_got_bootp:
dhcp_packet_process_options(bp);
/* Store net params from reply */
store_net_params(bp);
-- 
2.37.1



[PATCH 08/26] Move i.MX8 container image loading support to common/spl

2023-10-11 Thread Sean Anderson
To facilitate testing loading i.MX8 container images, move the
parse-container code to common/spl.

Signed-off-by: Sean Anderson 
---

 MAINTAINERS|  1 +
 arch/arm/mach-imx/Kconfig  | 13 -
 arch/arm/mach-imx/Makefile |  2 +-
 common/spl/Kconfig | 14 ++
 common/spl/Makefile|  1 +
 .../spl/spl_imx_container.c|  0
 6 files changed, 17 insertions(+), 14 deletions(-)
 rename arch/arm/mach-imx/parse-container.c => common/spl/spl_imx_container.c 
(100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 35209e73af5..dd6bb558dc4 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -299,6 +299,7 @@ F:  arch/arm/include/asm/arch-vf610/
 F: arch/arm/include/asm/mach-imx/
 F: board/freescale/*mx*/
 F: board/freescale/common/
+F: common/spl/spl_imx_container.c
 F: drivers/serial/serial_mxc.c
 F: include/imx_container.h
 
diff --git a/arch/arm/mach-imx/Kconfig b/arch/arm/mach-imx/Kconfig
index 266bb20df9d..08ab7069187 100644
--- a/arch/arm/mach-imx/Kconfig
+++ b/arch/arm/mach-imx/Kconfig
@@ -194,19 +194,6 @@ config IMX_DCD_ADDR
  This information is shared with the user via mkimage -l just so the
  image can be signed.
 
-config SPL_LOAD_IMX_CONTAINER
-   bool "Enable SPL loading U-Boot as a i.MX Container image"
-   depends on SPL
-   help
- This is to let SPL could load i.MX Container image
-
-config IMX_CONTAINER_CFG
-   string "i.MX Container config file"
-   depends on SPL
-   help
- This is to specific the cfg file for generating container
- image which will be loaded by SPL.
-
 config IOMUX_LPSR
bool
 
diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile
index aebfa6517bd..7c4e03278e3 100644
--- a/arch/arm/mach-imx/Makefile
+++ b/arch/arm/mach-imx/Makefile
@@ -79,7 +79,7 @@ obj-$(CONFIG_CMD_NANDBCB) += cmd_nandbcb.o
 endif
 
 ifeq ($(CONFIG_SPL_BUILD),y)
-obj-$(CONFIG_SPL_LOAD_IMX_CONTAINER) += image-container.o parse-container.o
+obj-$(CONFIG_SPL_LOAD_IMX_CONTAINER) += image-container.o
 endif
 
 ifeq ($(SOC),$(filter $(SOC),imx8ulp imx9))
diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 46323597942..ad574a600e3 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -330,6 +330,20 @@ config SPL_LEGACY_IMAGE_CRC_CHECK
  If disabled, Legacy images are booted if the image magic and size
  are correct, without further integrity checks.
 
+config SPL_LOAD_IMX_CONTAINER
+   bool "Enable SPL loading and booting of i.MX8 Containers"
+   depends on SPL
+   help
+ Support booting U-Boot from an i.MX8 container image. If you are not
+ using i.MX8, say 'n'.
+
+config IMX_CONTAINER_CFG
+   string "i.MX8 Container config file"
+   depends on SPL && SPL_LOAD_IMX_CONTAINER
+   help
+ Specify the cfg file for generating the container image which will be
+ loaded by SPL.
+
 config SPL_SYS_MALLOC_SIMPLE
bool "Only use malloc_simple functions in the SPL"
help
diff --git a/common/spl/Makefile b/common/spl/Makefile
index bad2bbf6cf1..4f8eb2ec0ca 100644
--- a/common/spl/Makefile
+++ b/common/spl/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_$(SPL_TPL_)OPENSBI) += spl_opensbi.o
 obj-$(CONFIG_$(SPL_TPL_)USB_STORAGE) += spl_usb.o
 obj-$(CONFIG_$(SPL_TPL_)FS_FAT) += spl_fat.o
 obj-$(CONFIG_$(SPL_TPL_)FS_EXT4) += spl_ext.o
+obj-$(CONFIG_$(SPL_TPL_)LOAD_IMX_CONTAINER) += spl_imx_container.o
 obj-$(CONFIG_$(SPL_TPL_)SATA) += spl_sata.o
 obj-$(CONFIG_$(SPL_TPL_)NVME) += spl_nvme.o
 obj-$(CONFIG_$(SPL_TPL_)SEMIHOSTING) += spl_semihosting.o
diff --git a/arch/arm/mach-imx/parse-container.c 
b/common/spl/spl_imx_container.c
similarity index 100%
rename from arch/arm/mach-imx/parse-container.c
rename to common/spl/spl_imx_container.c
-- 
2.37.1



[PATCH 14/26] net: bootp: Move port numbers to header

2023-10-11 Thread Sean Anderson
These defines are useful when testing bootp.

Signed-off-by: Sean Anderson 
---

 net/bootp.c | 3 ---
 net/bootp.h | 3 +++
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/bootp.c b/net/bootp.c
index 8b1a4ae2ef8..2053cce88c6 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -41,9 +41,6 @@
  */
 #define TIMEOUT_MS ((3 + (CONFIG_NET_RETRY_COUNT * 5)) * 1000)
 
-#define PORT_BOOTPS67  /* BOOTP server UDP port */
-#define PORT_BOOTPC68  /* BOOTP client UDP port */
-
 #ifndef CFG_DHCP_MIN_EXT_LEN   /* minimal length of extension list */
 #define CFG_DHCP_MIN_EXT_LEN 64
 #endif
diff --git a/net/bootp.h b/net/bootp.h
index 567340ec5d4..4e32b19d424 100644
--- a/net/bootp.h
+++ b/net/bootp.h
@@ -15,6 +15,9 @@
 
 /**/
 
+#define PORT_BOOTPS67  /* BOOTP server UDP port */
+#define PORT_BOOTPC68  /* BOOTP client UDP port */
+
 /*
  * BOOTP header.
  */
-- 
2.37.1



[PATCH 11/26] fs: ext4: Fix building ext4 in SPL if write is enabled

2023-10-11 Thread Sean Anderson
If EXT4_WRITE is enabled, write capabilities will be compiled into SPL, but
not CRC16. Add an option to enable CRC16 to avoid linker errors.

Signed-off-by: Sean Anderson 
---

 common/spl/Kconfig | 1 +
 lib/Kconfig| 6 ++
 lib/Makefile   | 1 +
 3 files changed, 8 insertions(+)

diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index ad574a600e3..6bc4066fad7 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -657,6 +657,7 @@ config SPL_ETH
 
 config SPL_FS_EXT4
bool "Support EXT filesystems"
+   select SPL_CRC16 if EXT4_WRITE
help
  Enable support for EXT2/3/4 filesystems with SPL. This permits
  U-Boot (or Linux in Falcon mode) to be loaded from an EXT
diff --git a/lib/Kconfig b/lib/Kconfig
index 79cf9ef0fa3..f6ca559897e 100644
--- a/lib/Kconfig
+++ b/lib/Kconfig
@@ -687,6 +687,12 @@ config SPL_CRC8
  checksum with feedback to produce an 8-bit result. The code is small
  and it does not require a lookup table (unlike CRC32).
 
+config SPL_CRC16
+   bool "Support CRC16 in SPL"
+   depends on SPL
+   help
+ Enables CRC16 support in SPL. This is not normally required.
+
 config CRC32
def_bool y
help
diff --git a/lib/Makefile b/lib/Makefile
index 1c31ad9531e..2a76acf100d 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -64,6 +64,7 @@ obj-$(CONFIG_TPM_V2) += tpm-v2.o
 endif
 
 obj-$(CONFIG_$(SPL_TPL_)CRC8) += crc8.o
+obj-$(CONFIG_$(SPL_TPL_)CRC16) += crc16.o
 
 obj-y += crypto/
 
-- 
2.37.1



[PATCH 24/26] test: spl: Add a test for the NET load method

2023-10-11 Thread Sean Anderson
Add a test for loading U-Boot over TFTP. As with other sandbox net
routines, we need to initialize our packets manually since things like
net_set_ether and net_set_udp_header always use "our" addresses. We use
BOOTP instead of DHCP, since DHCP has a tag/length-based format which is
harder to parse. Our TFTP implementation doesn't define as many constants
as I'd like, so I create some here. Note that the TFTP block size is
one-based, but offsets are zero-based.

In order to avoid address errors, we need to set up/define some additional
address information settings. dram_init_banksize would be a good candidate
for settig up bi_dram, but it gets called too late in board_init_r.

Signed-off-by: Sean Anderson 
---

 arch/sandbox/cpu/spl.c   |   3 +
 arch/sandbox/include/asm/spl.h   |   1 +
 configs/sandbox_noinst_defconfig |   6 +-
 test/image/Kconfig   |   9 ++
 test/image/Makefile  |   1 +
 test/image/spl_load_net.c| 252 +++
 6 files changed, 271 insertions(+), 1 deletion(-)
 create mode 100644 test/image/spl_load_net.c

diff --git a/arch/sandbox/cpu/spl.c b/arch/sandbox/cpu/spl.c
index 09e3d10d6a5..8153df18d68 100644
--- a/arch/sandbox/cpu/spl.c
+++ b/arch/sandbox/cpu/spl.c
@@ -126,6 +126,9 @@ void spl_board_init(void)
 {
struct sandbox_state *state = state_get_current();
 
+   gd->bd->bi_dram[0].start = gd->ram_base;
+   gd->bd->bi_dram[0].size = get_effective_memsize();
+
if (state->run_unittests) {
struct unit_test *tests = UNIT_TEST_ALL_START();
const int count = UNIT_TEST_ALL_COUNT();
diff --git a/arch/sandbox/include/asm/spl.h b/arch/sandbox/include/asm/spl.h
index 2f8b5fcfcfe..ab9475567e0 100644
--- a/arch/sandbox/include/asm/spl.h
+++ b/arch/sandbox/include/asm/spl.h
@@ -12,6 +12,7 @@ enum {
BOOT_DEVICE_MMC2_2,
BOOT_DEVICE_BOARD,
BOOT_DEVICE_VBE,
+   BOOT_DEVICE_CPGMAC,
 };
 
 /**
diff --git a/configs/sandbox_noinst_defconfig b/configs/sandbox_noinst_defconfig
index 4f16d9860d2..57cbadedb7d 100644
--- a/configs/sandbox_noinst_defconfig
+++ b/configs/sandbox_noinst_defconfig
@@ -12,7 +12,7 @@ CONFIG_SPL_DRIVERS_MISC=y
 CONFIG_SPL_SYS_MALLOC_F_LEN=0x8000
 CONFIG_SPL=y
 CONFIG_SPL_FS_FAT=y
-CONFIG_SYS_LOAD_ADDR=0x0
+CONFIG_SYS_LOAD_ADDR=0x100
 CONFIG_PCI=y
 CONFIG_SANDBOX_SPL=y
 CONFIG_DEBUG_UART=y
@@ -44,9 +44,11 @@ CONFIG_SPL_SYS_MALLOC_SIZE=0x400
 CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
 CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x0
 CONFIG_SPL_ENV_SUPPORT=y
+CONFIG_SPL_ETH=y
 CONFIG_SPL_FS_EXT4=y
 CONFIG_SPL_I2C=y
 CONFIG_SPL_MMC_WRITE=y
+CONFIG_SPL_NET=y
 CONFIG_SPL_RTC=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
@@ -109,6 +111,8 @@ CONFIG_ENV_IS_NOWHERE=y
 CONFIG_ENV_IS_IN_EXT4=y
 CONFIG_ENV_EXT4_INTERFACE="host"
 CONFIG_ENV_EXT4_DEVICE_AND_PART="0:0"
+CONFIG_USE_BOOTFILE=y
+CONFIG_BOOTFILE="uImage"
 CONFIG_BOOTP_SEND_HOSTNAME=y
 CONFIG_NETCONSOLE=y
 CONFIG_IP_DEFRAG=y
diff --git a/test/image/Kconfig b/test/image/Kconfig
index e6be1b829f3..99c50787816 100644
--- a/test/image/Kconfig
+++ b/test/image/Kconfig
@@ -23,6 +23,15 @@ config SPL_UT_LOAD_FS
help
  Test filesystems and the various load methods which use them.
 
+config SPL_UT_LOAD_NET
+   bool "Test loading over TFTP"
+   depends on SANDBOX && SPL_OF_REAL
+   depends on SPL_ETH
+   depends on USE_BOOTFILE
+   default y
+   help
+ Test loading images over TFTP using the NET image load method.
+
 config SPL_UT_LOAD_OS
bool "Test loading from the host OS"
depends on SANDBOX && SPL_LOAD_FIT
diff --git a/test/image/Makefile b/test/image/Makefile
index 9427e69bd3b..ddbc39bf959 100644
--- a/test/image/Makefile
+++ b/test/image/Makefile
@@ -4,4 +4,5 @@
 
 obj-$(CONFIG_SPL_UT_LOAD) += spl_load.o
 obj-$(CONFIG_SPL_UT_LOAD_FS) += spl_load_fs.o
+obj-$(CONFIG_SPL_UT_LOAD_NET) += spl_load_net.o
 obj-$(CONFIG_SPL_UT_LOAD_OS) += spl_load_os.o
diff --git a/test/image/spl_load_net.c b/test/image/spl_load_net.c
new file mode 100644
index 000..f570cef163f
--- /dev/null
+++ b/test/image/spl_load_net.c
@@ -0,0 +1,252 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2023 Sean Anderson 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "../../net/bootp.h"
+
+/*
+ * sandbox_eth_bootp_req_to_reply()
+ *
+ * Check if a BOOTP request was sent. If so, inject a reply
+ *
+ * returns 0 if injected, -EAGAIN if not
+ */
+static int sandbox_eth_bootp_req_to_reply(struct udevice *dev, void *packet,
+ unsigned int len)
+{
+   struct eth_sandbox_priv *priv = dev_get_priv(dev);
+   struct ethernet_hdr *eth = packet;
+   struct ip_udp_hdr *ip;
+   struct bootp_hdr *bp;
+   struct ethernet_hdr *eth_recv;
+   struct ip_udp_hdr *ipr;
+   struct bootp_hdr *bpr;
+
+   if (ntohs(eth->et_protlen) != PROT_IP)
+   return -EAGAIN;

[PATCH 22/26] test: spl: Add a test for spl_blk_load_image

2023-10-11 Thread Sean Anderson
Add a test for spl_blk_load_image, currently used only by NVMe. Because
there is no sandbox NVMe driver, just use MMC instead. Avoid falling back
to raw images to make failures more obvious.

Signed-off-by: Sean Anderson 
---

 configs/sandbox_noinst_defconfig |  2 ++
 test/image/Kconfig   |  3 +-
 test/image/spl_load_fs.c | 62 
 3 files changed, 66 insertions(+), 1 deletion(-)

diff --git a/configs/sandbox_noinst_defconfig b/configs/sandbox_noinst_defconfig
index 0a542cfb6aa..11be2dccf7d 100644
--- a/configs/sandbox_noinst_defconfig
+++ b/configs/sandbox_noinst_defconfig
@@ -34,6 +34,7 @@ CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_SPL_NO_BSS_LIMIT=y
 CONFIG_HANDOFF=y
 CONFIG_SPL_BOARD_INIT=y
+# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set
 CONFIG_SPL_LEGACY_IMAGE_FORMAT=y
 CONFIG_SPL_LOAD_IMX_CONTAINER=y
 CONFIG_SPL_SYS_MALLOC=y
@@ -123,6 +124,7 @@ CONFIG_ADC=y
 CONFIG_ADC_SANDBOX=y
 CONFIG_AXI=y
 CONFIG_AXI_SANDBOX=y
+CONFIG_SPL_BLK_FS=y
 CONFIG_SYS_IDE_MAXBUS=1
 CONFIG_SYS_ATA_BASE_ADDR=0x100
 CONFIG_SYS_ATA_STRIDE=4
diff --git a/test/image/Kconfig b/test/image/Kconfig
index 963c86cc290..a52766b77d4 100644
--- a/test/image/Kconfig
+++ b/test/image/Kconfig
@@ -14,12 +14,13 @@ config SPL_UT_LOAD_FS
bool "Unit tests for filesystems"
depends on SANDBOX && SPL_OF_REAL
depends on FS_LOADER
+   depends on SPL_BLK_FS
depends on SPL_FS_FAT
depends on SPL_FS_EXT4
depends on SPL_MMC_WRITE
default y
help
- Test filesystems in SPL.
+ Test filesystems and the various load methods which use them.
 
 config SPL_UT_LOAD_OS
bool "Test loading from the host OS"
diff --git a/test/image/spl_load_fs.c b/test/image/spl_load_fs.c
index 8cd90b73518..45059f91999 100644
--- a/test/image/spl_load_fs.c
+++ b/test/image/spl_load_fs.c
@@ -10,6 +10,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -303,3 +304,64 @@ static int spl_test_fat(struct unit_test_state *uts)
return spl_test_fs(uts, __func__, create_fat);
 }
 SPL_TEST(spl_test_fat, DM_FLAGS);
+
+static int spl_test_mmc_fs(struct unit_test_state *uts, const char *test_name,
+  enum spl_test_image type, create_fs_t create_fs)
+{
+   const char *filename = CONFIG_SPL_FS_LOAD_PAYLOAD_NAME;
+   struct blk_desc *dev_desc;
+   size_t fs_size, fs_data, img_size, img_data,
+  data_size = SPL_TEST_DATA_SIZE;
+   struct spl_image_info info_write = {
+   .name = test_name,
+   .size = data_size,
+   }, info_read = { };
+   struct disk_partition part = {
+   .start = 1,
+   .sys_ind = 0x83,
+   };
+   struct spl_boot_device bootdev = { };
+   void *fs;
+   char *data;
+
+   img_size = create_image(NULL, type, _write, _data);
+   ut_assert(img_size);
+   fs_size = create_fs(NULL, img_size, filename, _data);
+   ut_assert(fs_size);
+   fs = calloc(fs_size, 1);
+   ut_assertnonnull(fs);
+
+   data = fs + fs_data + img_data;
+   generate_data(data, data_size, test_name);
+   ut_asserteq(img_size, create_image(fs + fs_data, type, _write,
+  NULL));
+   ut_asserteq(fs_size, create_fs(fs, img_size, filename, NULL));
+
+   dev_desc = blk_get_devnum_by_uclass_id(UCLASS_MMC, 0);
+   ut_assertnonnull(dev_desc);
+
+   ut_asserteq(512, dev_desc->blksz);
+   part.size = fs_size / dev_desc->blksz;
+   ut_assertok(write_mbr_partitions(dev_desc, , 1, 0));
+   ut_asserteq(part.size, blk_dwrite(dev_desc, part.start, part.size, fs));
+
+   ut_assertok(spl_blk_load_image(_read, , UCLASS_MMC, 0, 1));
+   if (check_image_info(uts, _write, _read))
+   return CMD_RET_FAILURE;
+   ut_asserteq_mem(data, phys_to_virt(info_write.load_addr), data_size);
+
+   free(fs);
+   return 0;
+}
+
+static int spl_test_blk(struct unit_test_state *uts, const char *test_name,
+   enum spl_test_image type)
+{
+   if (spl_test_mmc_fs(uts, test_name, type, create_fat))
+   return CMD_RET_FAILURE;
+
+   return spl_test_mmc_fs(uts, test_name, type, create_ext2);
+}
+SPL_IMG_TEST(spl_test_blk, LEGACY, DM_FLAGS);
+SPL_IMG_TEST(spl_test_blk, FIT_EXTERNAL, DM_FLAGS);
+SPL_IMG_TEST(spl_test_blk, FIT_INTERNAL, DM_FLAGS);
-- 
2.37.1



[PATCH 25/26] test: spl: Add a test for the NOR load method

2023-10-11 Thread Sean Anderson
Add a test for the NOR load method. Since NOR is memory-mapped we can
substitute a buffer instead. The only major complication is testing LZMA
decompression.  It's too complex to implement LZMA compression in a test,
so we just include some pre-compressed data.

Signed-off-by: Sean Anderson 
---

 arch/sandbox/include/asm/spl.h   |   1 +
 configs/sandbox_noinst_defconfig |   2 +
 configs/sandbox_spl_defconfig|   2 +
 include/configs/sandbox.h|   3 +
 include/test/spl.h   |   5 +
 test/image/Makefile  |   1 +
 test/image/spl_load.c| 269 ++-
 test/image/spl_load_nor.c|  39 +
 8 files changed, 316 insertions(+), 6 deletions(-)
 create mode 100644 test/image/spl_load_nor.c

diff --git a/arch/sandbox/include/asm/spl.h b/arch/sandbox/include/asm/spl.h
index ab9475567e0..cf16af5278a 100644
--- a/arch/sandbox/include/asm/spl.h
+++ b/arch/sandbox/include/asm/spl.h
@@ -13,6 +13,7 @@ enum {
BOOT_DEVICE_BOARD,
BOOT_DEVICE_VBE,
BOOT_DEVICE_CPGMAC,
+   BOOT_DEVICE_NOR,
 };
 
 /**
diff --git a/configs/sandbox_noinst_defconfig b/configs/sandbox_noinst_defconfig
index 57cbadedb7d..085cc30c1e2 100644
--- a/configs/sandbox_noinst_defconfig
+++ b/configs/sandbox_noinst_defconfig
@@ -49,6 +49,7 @@ CONFIG_SPL_FS_EXT4=y
 CONFIG_SPL_I2C=y
 CONFIG_SPL_MMC_WRITE=y
 CONFIG_SPL_NET=y
+CONFIG_SPL_NOR_SUPPORT=y
 CONFIG_SPL_RTC=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
@@ -257,6 +258,7 @@ CONFIG_RSA_VERIFY_WITH_PKEY=y
 CONFIG_TPM=y
 CONFIG_LZ4=y
 CONFIG_ZSTD=y
+CONFIG_SPL_LZMA=y
 CONFIG_ERRNO_STR=y
 CONFIG_EFI_CAPSULE_ON_DISK=y
 CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
index b578cc8e443..56072b15ad2 100644
--- a/configs/sandbox_spl_defconfig
+++ b/configs/sandbox_spl_defconfig
@@ -41,6 +41,7 @@ CONFIG_SPL_SYS_MALLOC_SIZE=0x400
 CONFIG_SPL_ENV_SUPPORT=y
 CONFIG_SPL_FPGA=y
 CONFIG_SPL_I2C=y
+CONFIG_SPL_NOR_SUPPORT=y
 CONFIG_SPL_RTC=y
 CONFIG_CMD_CPU=y
 CONFIG_CMD_LICENSE=y
@@ -249,6 +250,7 @@ CONFIG_TPM=y
 CONFIG_SPL_CRC8=y
 CONFIG_LZ4=y
 CONFIG_ZSTD=y
+CONFIG_SPL_LZMA=y
 CONFIG_ERRNO_STR=y
 CONFIG_SPL_HEXDUMP=y
 CONFIG_EFI_CAPSULE_ON_DISK=y
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
index 4e5653dc886..2372485c84e 100644
--- a/include/configs/sandbox.h
+++ b/include/configs/sandbox.h
@@ -18,4 +18,7 @@
 #define CFG_SYS_BAUDRATE_TABLE {4800, 9600, 19200, 38400, 57600,\
115200}
 
+/* Unused but necessary to build */
+#define CFG_SYS_UBOOT_BASE CONFIG_TEXT_BASE
+
 #endif
diff --git a/include/test/spl.h b/include/test/spl.h
index cfb52c90855..82325702d4a 100644
--- a/include/test/spl.h
+++ b/include/test/spl.h
@@ -27,12 +27,14 @@ void generate_data(char *data, size_t size, const char 
*test_name);
 /**
  * enum spl_test_image - Image types for testing
  * @LEGACY: "Legacy" uImages
+ * @LEGACY_LZMA: "Legacy" uImages, LZMA compressed
  * @IMX8: i.MX8 Container images
  * @FIT_INTERNAL: FITs with internal data
  * @FIT_EXTERNAL: FITs with external data
  */
 enum spl_test_image {
LEGACY,
+   LEGACY_LZMA,
IMX8,
FIT_INTERNAL,
FIT_EXTERNAL,
@@ -118,6 +120,9 @@ int do_spl_test_load(struct unit_test_state *uts, const 
char *test_name,
 static inline bool image_supported(enum spl_test_image type)
 {
switch (type) {
+   case LEGACY_LZMA:
+   if (!IS_ENABLED(CONFIG_SPL_LZMA))
+   return false;
case LEGACY:
return IS_ENABLED(CONFIG_SPL_LEGACY_IMAGE_FORMAT);
case IMX8:
diff --git a/test/image/Makefile b/test/image/Makefile
index ddbc39bf959..41b29995993 100644
--- a/test/image/Makefile
+++ b/test/image/Makefile
@@ -5,4 +5,5 @@
 obj-$(CONFIG_SPL_UT_LOAD) += spl_load.o
 obj-$(CONFIG_SPL_UT_LOAD_FS) += spl_load_fs.o
 obj-$(CONFIG_SPL_UT_LOAD_NET) += spl_load_net.o
+obj-$(CONFIG_SPL_NOR_SUPPORT) += spl_load_nor.o
 obj-$(CONFIG_SPL_UT_LOAD_OS) += spl_load_os.o
diff --git a/test/image/spl_load.c b/test/image/spl_load.c
index 06249044f9b..3f69e652630 100644
--- a/test/image/spl_load.c
+++ b/test/image/spl_load.c
@@ -43,6 +43,7 @@ void *board_spl_fit_buffer_addr(ulong fit_size, int sectors, 
int bl_len)
 
 /* Local flags for spl_image; start from the "top" to avoid conflicts */
 #define SPL_IMX_CONTAINER  0x8000
+#define SPL_COMP_LZMA  0x4000
 
 void generate_data(char *data, size_t size, const char *test_name)
 {
@@ -79,7 +80,8 @@ static size_t create_legacy(void *dst, struct spl_image_info 
*spl_image,
image_set_os(hdr, spl_image->os);
image_set_arch(hdr, IH_ARCH_DEFAULT);
image_set_type(hdr, IH_TYPE_FIRMWARE);
-   image_set_comp(hdr, IH_COMP_NONE);
+   image_set_comp(hdr, spl_image->flags & SPL_COMP_LZMA ? IH_COMP_LZMA :
+  IH_COMP_NONE);
image_set_name(hdr, spl_image->name);
  

[PATCH 19/26] test: spl: Fix spl_test_load not failing if fname doesn't exist

2023-10-11 Thread Sean Anderson
Returning a negative value from a unit test doesn't automatically fail the
test.  We have to fail an assertion. Modify the test to do so.

This now causes the test to count as a failure on VPL. This is because the
fname of SPL (and U-Boot) is generated with make_exec in os_jump_to_image.
The original name of SPL is gone, and we can't determine the name of U-Boot
from the generated name.

Signed-off-by: Sean Anderson 
---

 configs/sandbox_vpl_defconfig | 1 +
 test/image/spl_load_os.c  | 6 ++
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/configs/sandbox_vpl_defconfig b/configs/sandbox_vpl_defconfig
index 8d76f19729b..5bd0281796d 100644
--- a/configs/sandbox_vpl_defconfig
+++ b/configs/sandbox_vpl_defconfig
@@ -262,3 +262,4 @@ CONFIG_UNIT_TEST=y
 CONFIG_SPL_UNIT_TEST=y
 CONFIG_UT_TIME=y
 CONFIG_UT_DM=y
+# CONFIG_SPL_UT_LOAD_OS is not set
diff --git a/test/image/spl_load_os.c b/test/image/spl_load_os.c
index 06daa3700b8..500867ffd49 100644
--- a/test/image/spl_load_os.c
+++ b/test/image/spl_load_os.c
@@ -59,10 +59,8 @@ static int spl_test_load(struct unit_test_state *uts)
load.read = read_fit_image;
 
ret = sandbox_find_next_phase(fname, sizeof(fname), true);
-   if (ret) {
-   printf("(%s not found, error %d)\n", fname, ret);
-   return ret;
-   }
+   if (ret)
+   ut_assertf(0, "%s not found, error %d\n", fname, ret);
load.filename = fname;
 
header = spl_get_load_buffer(-sizeof(*header), sizeof(*header));
-- 
2.37.1



[PATCH 23/26] test: spl: Add a test for the MMC load method

2023-10-11 Thread Sean Anderson
Add a test for the MMC load method. This shows the general shape of tests
to come: The main test function calls do_spl_test_load with an appropriate
callback to write the image to the medium.

Signed-off-by: Sean Anderson 
---

 configs/sandbox_noinst_defconfig |  2 +
 include/spl.h|  4 ++
 include/test/spl.h   | 30 +++
 test/image/Kconfig   |  1 +
 test/image/spl_load.c| 36 +
 test/image/spl_load_fs.c | 66 +---
 6 files changed, 134 insertions(+), 5 deletions(-)

diff --git a/configs/sandbox_noinst_defconfig b/configs/sandbox_noinst_defconfig
index 11be2dccf7d..4f16d9860d2 100644
--- a/configs/sandbox_noinst_defconfig
+++ b/configs/sandbox_noinst_defconfig
@@ -41,6 +41,8 @@ CONFIG_SPL_SYS_MALLOC=y
 CONFIG_SPL_HAS_CUSTOM_MALLOC_START=y
 CONFIG_SPL_CUSTOM_SYS_MALLOC_ADDR=0xa00
 CONFIG_SPL_SYS_MALLOC_SIZE=0x400
+CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR=y
+CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x0
 CONFIG_SPL_ENV_SUPPORT=y
 CONFIG_SPL_FS_EXT4=y
 CONFIG_SPL_I2C=y
diff --git a/include/spl.h b/include/spl.h
index 7d30fb57dac..8229d40adab 100644
--- a/include/spl.h
+++ b/include/spl.h
@@ -673,6 +673,10 @@ static inline const char *spl_loader_name(const struct 
spl_image_loader *loader)
}
 #endif
 
+#define SPL_LOAD_IMAGE_GET(_priority, _boot_device, _method) \
+   ll_entry_get(struct spl_image_loader, \
+_boot_device ## _priority ## _method, spl_image_loader)
+
 /* SPL FAT image functions */
 int spl_load_image_fat(struct spl_image_info *spl_image,
   struct spl_boot_device *bootdev,
diff --git a/include/test/spl.h b/include/test/spl.h
index 7ae32a1020b..cfb52c90855 100644
--- a/include/test/spl.h
+++ b/include/test/spl.h
@@ -79,6 +79,36 @@ size_t create_image(void *dst, enum spl_test_image type,
 int check_image_info(struct unit_test_state *uts, struct spl_image_info *info1,
 struct spl_image_info *info2);
 
+/**
+ * typedef write_image_t - Callback for writing an image
+ * @uts: Current unit test state
+ * @img: Image to write
+ * @size: Size of @img
+ *
+ * Write @img to a location which will be read by a  spl_image_loader.
+ *
+ * Return: 0 on success or -1 on failure
+ */
+typedef int write_image_t(struct unit_test_state *its, void *img, size_t size);
+
+/**
+ * do_spl_test_load() - Test loading with an SPL image loader
+ * @uts: Current unit test state
+ * @test_name: Name of the current test
+ * @type: Type of image to try loading
+ * @loader: The loader to test
+ * @write_image: Callback to write the image to the backing storage
+ *
+ * Test @loader, performing the common tasks of setting up the image and
+ * checking it was loaded correctly. The caller must supply a @write_image
+ * callback to write the image to a location which will be read by @loader.
+ *
+ * Return: 0 on success or -1 on failure
+ */
+int do_spl_test_load(struct unit_test_state *uts, const char *test_name,
+enum spl_test_image type, struct spl_image_loader *loader,
+write_image_t write_image);
+
 /**
  * image_supported() - Determine whether an image type is supported
  * @type: The image type to check
diff --git a/test/image/Kconfig b/test/image/Kconfig
index a52766b77d4..e6be1b829f3 100644
--- a/test/image/Kconfig
+++ b/test/image/Kconfig
@@ -18,6 +18,7 @@ config SPL_UT_LOAD_FS
depends on SPL_FS_FAT
depends on SPL_FS_EXT4
depends on SPL_MMC_WRITE
+   depends on SYS_MMCSD_RAW_MODE_U_BOOT_USE_SECTOR
default y
help
  Test filesystems and the various load methods which use them.
diff --git a/test/image/spl_load.c b/test/image/spl_load.c
index 4338da417ce..06249044f9b 100644
--- a/test/image/spl_load.c
+++ b/test/image/spl_load.c
@@ -368,3 +368,39 @@ SPL_IMG_TEST(spl_test_image, LEGACY, 0);
 SPL_IMG_TEST(spl_test_image, IMX8, 0);
 SPL_IMG_TEST(spl_test_image, FIT_INTERNAL, 0);
 SPL_IMG_TEST(spl_test_image, FIT_EXTERNAL, 0);
+
+int do_spl_test_load(struct unit_test_state *uts, const char *test_name,
+enum spl_test_image type, struct spl_image_loader *loader,
+int (*write_image)(struct unit_test_state *, void *, 
size_t))
+{
+   size_t img_size, img_data, plain_size = SPL_TEST_DATA_SIZE;
+   struct spl_image_info info_write = {
+   .name = test_name,
+   .size = plain_size,
+   }, info_read = { };
+   struct spl_boot_device bootdev = {
+   .boot_device = loader->boot_device,
+   };
+   char *plain;
+   void *img;
+
+   img_size = create_image(NULL, type, _write, _data);
+   ut_assert(img_size);
+   img = calloc(img_size, 1);
+   ut_assertnonnull(img);
+
+   plain = img + img_data;
+   generate_data(plain, plain_size, test_name);
+   ut_asserteq(img_size, create_image(img, type, _write, NULL));
+
+   if 

[PATCH 09/26] spl: Allow enabling SPL_OF_REAL and SPL_OF_PLATDATA at the same time

2023-10-11 Thread Sean Anderson
Sandbox unit tests in U-Boot proper load a test device tree to have some
devices to work with. In order to do the same in SPL, we must enable
SPL_OF_REAL. However, we already have SPL_OF_PLATDATA enabled. When
generating platdata from a devicetree, it is expected that we will not need
devicetree access functions (even though SPL_OF_CONTROL is enabled). This
expectation does not hold for sandbox, so allow user control of
SPL_OF_REAL.

There are several places in the tree where conditions involving OF_PLATDATA
or OF_REAL no longer function correctly when both of these options can be
selected at the same time. Adjust these conditions accordingly.

Signed-off-by: Sean Anderson 
---

 drivers/core/Makefile   | 1 +
 drivers/i2c/i2c-emul-uclass.c   | 2 +-
 drivers/serial/sandbox.c| 2 +-
 drivers/sysreset/sysreset_sandbox.c | 2 +-
 dts/Kconfig | 8 +---
 test/test-main.c| 2 +-
 6 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/core/Makefile b/drivers/core/Makefile
index bce0a3f65cb..acbd2bf2cef 100644
--- a/drivers/core/Makefile
+++ b/drivers/core/Makefile
@@ -15,6 +15,7 @@ obj-$(CONFIG_$(SPL_)OF_LIVE) += of_access.o of_addr.o
 ifndef CONFIG_DM_DEV_READ_INLINE
 obj-$(CONFIG_OF_CONTROL) += read.o
 endif
+obj-$(CONFIG_$(SPL_)OF_PLATDATA) += read.o
 obj-$(CONFIG_OF_CONTROL) += of_extra.o ofnode.o read_extra.o
 
 ccflags-$(CONFIG_DM_DEBUG) += -DDEBUG
diff --git a/drivers/i2c/i2c-emul-uclass.c b/drivers/i2c/i2c-emul-uclass.c
index 1107cf309fc..d421ddfcbe2 100644
--- a/drivers/i2c/i2c-emul-uclass.c
+++ b/drivers/i2c/i2c-emul-uclass.c
@@ -46,7 +46,7 @@ int i2c_emul_find(struct udevice *dev, struct udevice **emulp)
struct udevice *emul;
int ret;
 
-   if (!CONFIG_IS_ENABLED(OF_PLATDATA)) {
+   if (CONFIG_IS_ENABLED(OF_REAL)) {
ret = uclass_find_device_by_phandle(UCLASS_I2C_EMUL, dev,
"sandbox,emul", );
} else {
diff --git a/drivers/serial/sandbox.c b/drivers/serial/sandbox.c
index f4003811ee7..f6ac3d22852 100644
--- a/drivers/serial/sandbox.c
+++ b/drivers/serial/sandbox.c
@@ -280,7 +280,7 @@ U_BOOT_DRIVER(sandbox_serial) = {
.flags = DM_FLAG_PRE_RELOC,
 };
 
-#if CONFIG_IS_ENABLED(OF_REAL)
+#if CONFIG_IS_ENABLED(OF_REAL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 static const struct sandbox_serial_plat platdata_non_fdt = {
.colour = -1,
 };
diff --git a/drivers/sysreset/sysreset_sandbox.c 
b/drivers/sysreset/sysreset_sandbox.c
index 3750c60b9b9..f485a135299 100644
--- a/drivers/sysreset/sysreset_sandbox.c
+++ b/drivers/sysreset/sysreset_sandbox.c
@@ -132,7 +132,7 @@ U_BOOT_DRIVER(warm_sysreset_sandbox) = {
.ops= _warm_sysreset_ops,
 };
 
-#if CONFIG_IS_ENABLED(OF_REAL)
+#if CONFIG_IS_ENABLED(OF_REAL) && !CONFIG_IS_ENABLED(OF_PLATDATA)
 /* This is here in case we don't have a device tree */
 U_BOOT_DRVINFO(sysreset_sandbox_non_fdt) = {
.name = "sysreset_sandbox",
diff --git a/dts/Kconfig b/dts/Kconfig
index 9152f5885e9..c6fb193ca89 100644
--- a/dts/Kconfig
+++ b/dts/Kconfig
@@ -410,12 +410,14 @@ config SPL_OF_PLATDATA
  declarations for each node. See of-plat.txt for more information.
 
 config SPL_OF_REAL
-   bool
+   bool "Support a real devicetree in SPL"
+   depends on SPL_OF_CONTROL
+   select SPL_OF_LIBFDT
help
  Indicates that a real devicetree is available which can be accessed
  at runtime. This means that dev_read_...() functions can be used to
- read data from the devicetree for each device. This is true if
- SPL_OF_CONTROL is enabled and not SPL_OF_PLATDATA
+ read data from the devicetree for each device. You do not need to
+ enable this option if you have enabled SPL_OF_PLATDATA.
 
 if SPL_OF_PLATDATA
 
diff --git a/test/test-main.c b/test/test-main.c
index edb20bc4b9c..b7015d9f38d 100644
--- a/test/test-main.c
+++ b/test/test-main.c
@@ -303,7 +303,7 @@ static int test_pre_run(struct unit_test_state *uts, struct 
unit_test *test)
if (test->flags & UT_TESTF_PROBE_TEST)
ut_assertok(do_autoprobe(uts));
 
-   if (!CONFIG_IS_ENABLED(OF_PLATDATA) &&
+   if (CONFIG_IS_ENABLED(OF_REAL) &&
(test->flags & UT_TESTF_SCAN_FDT)) {
/*
 * only set this if we know the ethernet uclass will be created
-- 
2.37.1



[PATCH 13/26] net: Fix compiling SPL when fastboot is enabled

2023-10-11 Thread Sean Anderson
When fastboot is enabled in U-Boot proper and SPL_NET is enabled, we will
try to (unsuccessfully) reference it in SPL. Fix these linker errors by
conditioning on SPL_UDP/TCP_FUNCTION_FASTBOOT.

Signed-off-by: Sean Anderson 
---

 net/Makefile | 4 ++--
 net/net.c| 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/net/Makefile b/net/Makefile
index 3e2d061338d..5ea58eef7e4 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -27,8 +27,8 @@ obj-$(CONFIG_CMD_PCAP) += pcap.o
 obj-$(CONFIG_CMD_RARP) += rarp.o
 obj-$(CONFIG_CMD_SNTP) += sntp.o
 obj-$(CONFIG_CMD_TFTPBOOT) += tftp.o
-obj-$(CONFIG_UDP_FUNCTION_FASTBOOT)  += fastboot_udp.o
-obj-$(CONFIG_TCP_FUNCTION_FASTBOOT)  += fastboot_tcp.o
+obj-$(CONFIG_$(SPL_)UDP_FUNCTION_FASTBOOT)  += fastboot_udp.o
+obj-$(CONFIG_$(SPL_)TCP_FUNCTION_FASTBOOT)  += fastboot_tcp.o
 obj-$(CONFIG_CMD_WOL)  += wol.o
 obj-$(CONFIG_PROT_UDP) += udp.o
 obj-$(CONFIG_PROT_TCP) += tcp.o
diff --git a/net/net.c b/net/net.c
index e6f61f0f8f6..8357f084101 100644
--- a/net/net.c
+++ b/net/net.c
@@ -511,12 +511,12 @@ restart:
tftp_start_server();
break;
 #endif
-#if defined(CONFIG_UDP_FUNCTION_FASTBOOT)
+#if CONFIG_IS_ENABLED(UDP_FUNCTION_FASTBOOT)
case FASTBOOT_UDP:
fastboot_udp_start_server();
break;
 #endif
-#if defined(CONFIG_TCP_FUNCTION_FASTBOOT)
+#if CONFIG_IS_ENABLED(TCP_FUNCTION_FASTBOOT)
case FASTBOOT_TCP:
fastboot_tcp_start_server();
break;
-- 
2.37.1



[PATCH 10/26] lib: acpi: Fix linking SPL when ACPIGEN is enabled

2023-10-11 Thread Sean Anderson
lib/acpi/acpigen.o is only compiled into SPL when SPL_ACPIGEN is enabled.
Update several files which reference these functions accordingly.

Signed-off-by: Sean Anderson 
---

 drivers/core/root.c  | 2 +-
 drivers/i2c/Makefile | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/core/root.c b/drivers/core/root.c
index 126b3140666..d4ae652bcfb 100644
--- a/drivers/core/root.c
+++ b/drivers/core/root.c
@@ -426,7 +426,7 @@ void dm_get_mem(struct dm_stats *stats)
stats->tag_size;
 }
 
-#ifdef CONFIG_ACPIGEN
+#if CONFIG_IS_ENABLED(ACPIGEN)
 static int root_acpi_get_name(const struct udevice *dev, char *out_name)
 {
return acpi_copy_name(out_name, "\\_SB");
diff --git a/drivers/i2c/Makefile b/drivers/i2c/Makefile
index d5b85f398db..a96a8c7e955 100644
--- a/drivers/i2c/Makefile
+++ b/drivers/i2c/Makefile
@@ -3,7 +3,7 @@
 # (C) Copyright 2000-2007
 # Wolfgang Denk, DENX Software Engineering, w...@denx.de.
 obj-$(CONFIG_$(SPL_)DM_I2C) += i2c-uclass.o
-ifdef CONFIG_ACPIGEN
+ifdef CONFIG_$(SPL_)ACPIGEN
 obj-$(CONFIG_$(SPL_)DM_I2C) += acpi_i2c.o
 endif
 obj-$(CONFIG_$(SPL_)DM_I2C_GPIO) += i2c-gpio.o
-- 
2.37.1



[PATCH 17/26] spl: Use map_sysmem where appropriate

2023-10-11 Thread Sean Anderson
All "physical" addresses in SPL must be converted to virtual addresses
before access in order for sandbox to work. Add some calls to map_sysmem in
appropriate places. We do not generally call unmap_sysmem, since we need
the image memory to still be mapped when we jump to the image. This doesn't
matter at the moment since unmap_sysmem is a no-op.

Signed-off-by: Sean Anderson 
---

 common/spl/spl.c   |  4 +++-
 common/spl/spl_blk_fs.c|  6 --
 common/spl/spl_ext.c   |  4 +++-
 common/spl/spl_fat.c   | 11 +++
 common/spl/spl_fit.c   | 36 +-
 common/spl/spl_imx_container.c |  4 +++-
 common/spl/spl_legacy.c|  6 --
 common/spl/spl_mmc.c   |  4 +++-
 common/spl/spl_net.c   | 10 +++---
 common/spl/spl_nor.c   |  5 +++--
 common/spl/spl_spi.c   | 14 +
 11 files changed, 69 insertions(+), 35 deletions(-)

diff --git a/common/spl/spl.c b/common/spl/spl.c
index 66eeea41a34..732d90d39e6 100644
--- a/common/spl/spl.c
+++ b/common/spl/spl.c
@@ -653,7 +653,9 @@ void board_init_r(gd_t *dummy1, ulong dummy2)
spl_set_bd();
 
if (IS_ENABLED(CONFIG_SPL_SYS_MALLOC)) {
-   mem_malloc_init(SPL_SYS_MALLOC_START, SPL_SYS_MALLOC_SIZE);
+   mem_malloc_init((ulong)map_sysmem(SPL_SYS_MALLOC_START,
+ SPL_SYS_MALLOC_SIZE),
+   SPL_SYS_MALLOC_SIZE);
gd->flags |= GD_FLG_FULL_MALLOC_INIT;
}
if (!(gd->flags & GD_FLG_SPL_INIT)) {
diff --git a/common/spl/spl_blk_fs.c b/common/spl/spl_blk_fs.c
index ea5d1a51d9f..63825d620d1 100644
--- a/common/spl/spl_blk_fs.c
+++ b/common/spl/spl_blk_fs.c
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 
 struct blk_dev {
const char *ifname;
@@ -29,7 +30,8 @@ static ulong spl_fit_read(struct spl_load_info *load, ulong 
file_offset,
return ret;
}
 
-   ret = fs_read(load->filename, (ulong)buf, file_offset, size, );
+   ret = fs_read(load->filename, virt_to_phys(buf), file_offset, size,
+ );
if (ret < 0) {
printf("spl: error reading image %s. Err - %d\n",
   load->filename, ret);
@@ -69,7 +71,7 @@ int spl_blk_load_image(struct spl_image_info *spl_image,
goto out;
}
 
-   ret = fs_read(filename, (ulong)header, 0,
+   ret = fs_read(filename, virt_to_phys(header), 0,
  sizeof(struct legacy_img_hdr), );
if (ret) {
printf("spl: unable to read file %s. Err - %d\n", filename,
diff --git a/common/spl/spl_ext.c b/common/spl/spl_ext.c
index 902564a6077..af836ca15b8 100644
--- a/common/spl/spl_ext.c
+++ b/common/spl/spl_ext.c
@@ -2,6 +2,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -53,7 +54,8 @@ int spl_load_image_ext(struct spl_image_info *spl_image,
goto end;
}
 
-   err = ext4fs_read((char *)spl_image->load_addr, 0, filelen, );
+   err = ext4fs_read(map_sysmem(spl_image->load_addr, filelen), 0, filelen,
+ );
 
 end:
 #ifdef CONFIG_SPL_LIBCOMMON_SUPPORT
diff --git a/common/spl/spl_fat.c b/common/spl/spl_fat.c
index 8bec9cce5ca..ff9892356f6 100644
--- a/common/spl/spl_fat.c
+++ b/common/spl/spl_fat.c
@@ -11,6 +11,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -74,11 +75,13 @@ int spl_load_image_fat(struct spl_image_info *spl_image,
 
if (IS_ENABLED(CONFIG_SPL_LOAD_FIT_FULL) &&
image_get_magic(header) == FDT_MAGIC) {
-   err = file_fat_read(filename, (void *)CONFIG_SYS_LOAD_ADDR, 0);
+   err = file_fat_read(filename,
+   map_sysmem(CONFIG_SYS_LOAD_ADDR, 0), 0);
if (err <= 0)
goto end;
err = spl_parse_image_header(spl_image, bootdev,
-   (struct legacy_img_hdr *)CONFIG_SYS_LOAD_ADDR);
+map_sysmem(CONFIG_SYS_LOAD_ADDR,
+   err));
if (err == -EAGAIN)
return err;
if (err == 0)
@@ -99,8 +102,8 @@ int spl_load_image_fat(struct spl_image_info *spl_image,
if (err)
goto end;
 
-   err = file_fat_read(filename,
-   (u8 *)(uintptr_t)spl_image->load_addr, 0);
+   err = file_fat_read(filename, map_sysmem(spl_image->load_addr,
+spl_image->size), 0);
}
 
 end:
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index e3cdf8e5c05..f397cb40277 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -16,6 +16,7 @@
 #include 
 #include 
 #include 
+#include 
 

[PATCH 12/26] fs: Compile in sandbox filesystem in SPL if it is enabled

2023-10-11 Thread Sean Anderson
fs.c thinks that the sandbox filesystem is available if SANDBOX is enabled,
but it is not in SPL. Compile it in SPL to avoid linker errors.

Signed-off-by: Sean Anderson 
---

 fs/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/Makefile b/fs/Makefile
index 4bed2ff2d99..592c7542bde 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -9,6 +9,7 @@ obj-$(CONFIG_FS_LOADER) += fs.o
 obj-$(CONFIG_SPL_FS_FAT) += fat/
 obj-$(CONFIG_SPL_FS_EXT4) += ext4/
 obj-$(CONFIG_SPL_FS_CBFS) += cbfs/
+obj-$(CONFIG_SANDBOX) += sandbox/
 obj-$(CONFIG_SPL_FS_SQUASHFS) += squashfs/
 else
 obj-y  += fs.o
-- 
2.37.1



[PATCH 07/26] arm: imx: Check header before calling spl_load_imx_container

2023-10-11 Thread Sean Anderson
Make sure we have an IMX header before calling spl_load_imx_container,
since if we don't it will fail with -ENOENT. This allows us to fall back to
legacy/raw images if they are also enabled.

To avoid too much bloat, Legacy/Raw images are disabled for the four
configs which only boot from raw MMC.

Future work could include merging imx_container.h with imx8image.h, since
they appear to define mostly the same structures.

Signed-off-by: Sean Anderson 
---

 MAINTAINERS  | 1 +
 arch/arm/include/asm/mach-imx/ahab.h | 2 +-
 arch/arm/mach-imx/cmd_dek.c  | 4 ++--
 arch/arm/mach-imx/ele_ahab.c | 2 +-
 arch/arm/mach-imx/image-container.c  | 2 +-
 arch/arm/mach-imx/imx8/ahab.c| 2 +-
 arch/arm/mach-imx/parse-container.c  | 2 +-
 arch/arm/mach-imx/spl_imx_romapi.c   | 5 +++--
 common/spl/spl_mmc.c | 4 +++-
 common/spl/spl_nand.c| 4 +++-
 common/spl/spl_nor.c | 4 +++-
 common/spl/spl_spi.c | 4 +++-
 configs/deneb_defconfig  | 2 ++
 configs/giedi_defconfig  | 2 ++
 configs/imx8qm_mek_defconfig | 2 ++
 configs/imx8qxp_mek_defconfig| 2 ++
 drivers/usb/gadget/f_sdp.c   | 4 +++-
 .../include/asm/mach-imx/image.h => include/imx_container.h  | 0
 18 files changed, 34 insertions(+), 14 deletions(-)
 rename arch/arm/include/asm/mach-imx/image.h => include/imx_container.h (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index 7d5d05320c0..35209e73af5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -300,6 +300,7 @@ F:  arch/arm/include/asm/mach-imx/
 F: board/freescale/*mx*/
 F: board/freescale/common/
 F: drivers/serial/serial_mxc.c
+F: include/imx_container.h
 
 ARM HISILICON
 M: Peter Griffin 
diff --git a/arch/arm/include/asm/mach-imx/ahab.h 
b/arch/arm/include/asm/mach-imx/ahab.h
index 4222e3db278..4884f056251 100644
--- a/arch/arm/include/asm/mach-imx/ahab.h
+++ b/arch/arm/include/asm/mach-imx/ahab.h
@@ -6,7 +6,7 @@
 #ifndef __IMX_AHAB_H__
 #define __IMX_AHAB_H__
 
-#include 
+#include 
 
 int ahab_auth_cntr_hdr(struct container_hdr *container, u16 length);
 int ahab_auth_release(void);
diff --git a/arch/arm/mach-imx/cmd_dek.c b/arch/arm/mach-imx/cmd_dek.c
index 6fa5b41fcd3..2f389dbe8df 100644
--- a/arch/arm/mach-imx/cmd_dek.c
+++ b/arch/arm/mach-imx/cmd_dek.c
@@ -18,12 +18,12 @@
 #include 
 #include 
 #ifdef CONFIG_IMX_SECO_DEK_ENCAP
+#include 
 #include 
-#include 
 #endif
 #ifdef CONFIG_IMX_ELE_DEK_ENCAP
+#include 
 #include 
-#include 
 #endif
 
 #include 
diff --git a/arch/arm/mach-imx/ele_ahab.c b/arch/arm/mach-imx/ele_ahab.c
index 6a1ad198f89..295c055ad0a 100644
--- a/arch/arm/mach-imx/ele_ahab.c
+++ b/arch/arm/mach-imx/ele_ahab.c
@@ -6,12 +6,12 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
diff --git a/arch/arm/mach-imx/image-container.c 
b/arch/arm/mach-imx/image-container.c
index eff9e0c4597..ebc8021d7cc 100644
--- a/arch/arm/mach-imx/image-container.c
+++ b/arch/arm/mach-imx/image-container.c
@@ -5,6 +5,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -12,7 +13,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 
diff --git a/arch/arm/mach-imx/imx8/ahab.c b/arch/arm/mach-imx/imx8/ahab.c
index 44ea63584aa..994becccefd 100644
--- a/arch/arm/mach-imx/imx8/ahab.c
+++ b/arch/arm/mach-imx/imx8/ahab.c
@@ -6,6 +6,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -13,7 +14,6 @@
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include "u-boot/sha256.h"
diff --git a/arch/arm/mach-imx/parse-container.c 
b/arch/arm/mach-imx/parse-container.c
index 0a3d41f411e..126ab7c57a1 100644
--- a/arch/arm/mach-imx/parse-container.c
+++ b/arch/arm/mach-imx/parse-container.c
@@ -6,9 +6,9 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
-#include 
 #ifdef CONFIG_AHAB_BOOT
 #include 
 #endif
diff --git a/arch/arm/mach-imx/spl_imx_romapi.c 
b/arch/arm/mach-imx/spl_imx_romapi.c
index b51061b987b..8816566b364 100644
--- a/arch/arm/mach-imx/spl_imx_romapi.c
+++ b/arch/arm/mach-imx/spl_imx_romapi.c
@@ -6,11 +6,11 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
-#include 
 #include 
 
 DECLARE_GLOBAL_DATA_PTR;
@@ -111,7 +111,8 @@ static int spl_romapi_load_image_seekable(struct 
spl_image_info *spl_image,
load.read = spl_romapi_read_seekable;
load.priv = 
return 

[PATCH 06/26] arm: imx: Add function to validate i.MX8 containers

2023-10-11 Thread Sean Anderson
Add a function to abstract the common task of validating i.MX8 container
image headers.

Signed-off-by: Sean Anderson 
---

 arch/arm/include/asm/mach-imx/image.h | 9 +
 arch/arm/mach-imx/ele_ahab.c  | 2 +-
 arch/arm/mach-imx/image-container.c   | 2 +-
 arch/arm/mach-imx/imx8/ahab.c | 2 +-
 arch/arm/mach-imx/parse-container.c   | 2 +-
 arch/arm/mach-imx/spl_imx_romapi.c| 3 ++-
 6 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/arch/arm/include/asm/mach-imx/image.h 
b/arch/arm/include/asm/mach-imx/image.h
index ee67ca96f4c..54cd684e35d 100644
--- a/arch/arm/include/asm/mach-imx/image.h
+++ b/arch/arm/include/asm/mach-imx/image.h
@@ -18,6 +18,9 @@
 #define CONTAINER_HDR_QSPI_OFFSET SZ_4K
 #define CONTAINER_HDR_NAND_OFFSET SZ_128M
 
+#define CONTAINER_HDR_TAG 0x87
+#define CONTAINER_HDR_VERSION 0
+
 struct container_hdr {
u8 version;
u8 length_lsb;
@@ -66,4 +69,10 @@ struct generate_key_blob_hdr {
 } __packed;
 
 int get_container_size(ulong addr, u16 *header_length);
+
+static inline bool valid_container_hdr(struct container_hdr *container)
+{
+   return container->tag == CONTAINER_HDR_TAG &&
+  container->version == CONTAINER_HDR_VERSION;
+}
 #endif
diff --git a/arch/arm/mach-imx/ele_ahab.c b/arch/arm/mach-imx/ele_ahab.c
index 785b0d6ec3c..6a1ad198f89 100644
--- a/arch/arm/mach-imx/ele_ahab.c
+++ b/arch/arm/mach-imx/ele_ahab.c
@@ -343,7 +343,7 @@ int authenticate_os_container(ulong addr)
}
 
phdr = (struct container_hdr *)addr;
-   if (phdr->tag != 0x87 || phdr->version != 0x0) {
+   if (!valid_container_hdr(phdr)) {
printf("Error: Wrong container header\n");
return -EFAULT;
}
diff --git a/arch/arm/mach-imx/image-container.c 
b/arch/arm/mach-imx/image-container.c
index 5f188ab32d1..eff9e0c4597 100644
--- a/arch/arm/mach-imx/image-container.c
+++ b/arch/arm/mach-imx/image-container.c
@@ -50,7 +50,7 @@ int get_container_size(ulong addr, u16 *header_length)
u32 max_offset = 0, img_end;
 
phdr = (struct container_hdr *)addr;
-   if (phdr->tag != 0x87 || phdr->version != 0x0) {
+   if (!valid_container_hdr(phdr)) {
debug("Wrong container header\n");
return -EFAULT;
}
diff --git a/arch/arm/mach-imx/imx8/ahab.c b/arch/arm/mach-imx/imx8/ahab.c
index b58b14ca9b4..44ea63584aa 100644
--- a/arch/arm/mach-imx/imx8/ahab.c
+++ b/arch/arm/mach-imx/imx8/ahab.c
@@ -146,7 +146,7 @@ int authenticate_os_container(ulong addr)
}
 
phdr = (struct container_hdr *)addr;
-   if (phdr->tag != 0x87 && phdr->version != 0x0) {
+   if (!valid_container_hdr(phdr)) {
printf("Error: Wrong container header\n");
return -EFAULT;
}
diff --git a/arch/arm/mach-imx/parse-container.c 
b/arch/arm/mach-imx/parse-container.c
index c5df78d1c58..0a3d41f411e 100644
--- a/arch/arm/mach-imx/parse-container.c
+++ b/arch/arm/mach-imx/parse-container.c
@@ -84,7 +84,7 @@ static int read_auth_container(struct spl_image_info 
*spl_image,
goto end;
}
 
-   if (container->tag != 0x87 && container->version != 0x0) {
+   if (!valid_container_hdr(container)) {
printf("Wrong container header\n");
ret = -ENOENT;
goto end;
diff --git a/arch/arm/mach-imx/spl_imx_romapi.c 
b/arch/arm/mach-imx/spl_imx_romapi.c
index 4af41699678..b51061b987b 100644
--- a/arch/arm/mach-imx/spl_imx_romapi.c
+++ b/arch/arm/mach-imx/spl_imx_romapi.c
@@ -184,7 +184,8 @@ static u8 *search_container_header(u8 *p, int size)
 
for (i = 0; i < size; i += 4) {
hdr = p + i;
-   if (*(hdr + 3) == 0x87 && *hdr == 0 && (*(hdr + 1) != 0 || 
*(hdr + 2) != 0))
+   if (valid_container_hdr((void *)hdr) &&
+   (*(hdr + 1) != 0 || *(hdr + 2) != 0))
return p + i;
}
 
-- 
2.37.1



[PATCH 05/26] arm: imx: Add newlines after error messages

2023-10-11 Thread Sean Anderson
These error messages are missing newlines. Add them.

Fixes: 6e81ca220e0 ("imx: parse-container: Use malloc for container processing")
Signed-off-by: Sean Anderson 
---

 arch/arm/mach-imx/parse-container.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-imx/parse-container.c 
b/arch/arm/mach-imx/parse-container.c
index d7275a58c17..c5df78d1c58 100644
--- a/arch/arm/mach-imx/parse-container.c
+++ b/arch/arm/mach-imx/parse-container.c
@@ -85,13 +85,13 @@ static int read_auth_container(struct spl_image_info 
*spl_image,
}
 
if (container->tag != 0x87 && container->version != 0x0) {
-   printf("Wrong container header");
+   printf("Wrong container header\n");
ret = -ENOENT;
goto end;
}
 
if (!container->num_images) {
-   printf("Wrong container, no image found");
+   printf("Wrong container, no image found\n");
ret = -ENOENT;
goto end;
}
-- 
2.37.1



[PATCH 02/26] spl: nor: Don't allocate header on stack

2023-10-11 Thread Sean Anderson
spl_image_info.name contains a reference to legacy_img_hdr. If we allocate
the latter on the stack, it will be clobbered after we return. This was
addressed for NAND back in 06377c5a1fc ("spl: spl_legacy: Fix NAND boot on
OMAP3 BeagleBoard"), but that commit didn't fix NOR.

Signed-off-by: Sean Anderson 
---

 common/spl/spl_nor.c | 11 ---
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/common/spl/spl_nor.c b/common/spl/spl_nor.c
index 79d4f1d7aa8..c141a9ae629 100644
--- a/common/spl/spl_nor.c
+++ b/common/spl/spl_nor.c
@@ -26,7 +26,7 @@ unsigned long __weak spl_nor_get_uboot_base(void)
 static int spl_nor_load_image(struct spl_image_info *spl_image,
  struct spl_boot_device *bootdev)
 {
-   __maybe_unused const struct legacy_img_hdr *header;
+   struct legacy_img_hdr *header;
__maybe_unused struct spl_load_info load;
 
/*
@@ -41,7 +41,7 @@ static int spl_nor_load_image(struct spl_image_info 
*spl_image,
 * Load Linux from its location in NOR flash to its defined
 * location in SDRAM
 */
-   header = (const struct legacy_img_hdr *)CONFIG_SYS_OS_BASE;
+   header = (void *)CONFIG_SYS_OS_BASE;
 #ifdef CONFIG_SPL_LOAD_FIT
if (image_get_magic(header) == FDT_MAGIC) {
int ret;
@@ -91,8 +91,8 @@ static int spl_nor_load_image(struct spl_image_info 
*spl_image,
 * Load real U-Boot from its location in NOR flash to its
 * defined location in SDRAM
 */
-#ifdef CONFIG_SPL_LOAD_FIT
header = (const struct legacy_img_hdr *)spl_nor_get_uboot_base();
+#ifdef CONFIG_SPL_LOAD_FIT
if (image_get_magic(header) == FDT_MAGIC) {
debug("Found FIT format U-Boot\n");
load.bl_len = 1;
@@ -111,14 +111,11 @@ static int spl_nor_load_image(struct spl_image_info 
*spl_image,
 
/* Legacy image handling */
if (IS_ENABLED(CONFIG_SPL_LEGACY_IMAGE_FORMAT)) {
-   struct legacy_img_hdr hdr;
-
load.bl_len = 1;
load.read = spl_nor_load_read;
-   spl_nor_load_read(, spl_nor_get_uboot_base(), sizeof(hdr), 
);
return spl_load_legacy_img(spl_image, bootdev, ,
   spl_nor_get_uboot_base(),
-  );
+  header);
}
 
return -EINVAL;
-- 
2.37.1



[PATCH 03/26] spl: fit: Fix entry point for SPL_LOAD_FIT_FULL

2023-10-11 Thread Sean Anderson
The entry point is not always the same as the load address. Use the value
of the entrypoint property if it exists and is nonzero (following the
example of spl_load_simple_fit).

Fixes: 8a9dc16e4d0 ("spl: Add full fitImage support")
Signed-off-by: Sean Anderson 
---

 common/spl/spl_fit.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index ce6b8aa370a..e3cdf8e5c05 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -884,8 +884,10 @@ int spl_load_fit_image(struct spl_image_info *spl_image,
return ret;
 
spl_image->size = fw_len;
-   spl_image->entry_point = fw_data;
spl_image->load_addr = fw_data;
+   if (fit_image_get_entry(header, ret, _image->entry_point) ||
+   !spl_image->entry_point)
+   spl_image->entry_point = fw_data;
if (fit_image_get_os(header, ret, _image->os))
spl_image->os = IH_OS_INVALID;
spl_image->name = genimg_get_os_name(spl_image->os);
-- 
2.37.1



[PATCH 00/26] test: spl: Test some load methods

2023-10-11 Thread Sean Anderson
This series adds some tests for various SPL load methods, with the intent of
helping debug v6 of [1]. With that in mind, notable omissions include NAND and
ROMAPI, which both lack sandbox implementations, and OS_BOOT, which I have
deferred due to its complexity. Semihosting is also omitted, but I think we can
test that with qemu.

In order to test all of these methods, we must first generate suitable images,
possibly on filesystems. While other tests have historically generated these
images using external tools (e.g. mkimage, mkfs, etc.), I have chosen to
generate them on the fly. This is for a few reasons:

- By removing external dependencies on pytest to create certain files, the tests
  become self-contained. This makes them easier to iterate on and debug.
- By generating tests at runtime, we can dynamically vary the content. This
  helps detect test failures, as even if tests are loaded to the same location,
  the expected content will be different.
- We are not testing the image parsers themselves (e.g. spl_load_simple_fit or
  fs_read) but rather the load methods (e.g. spl_mmc_load_image). It is
  unnecessary to exercise full functionality or generate 100% correct images.
- By reducing functionality to only what is necessary, the complexity of various
  formats can often be greatly reduced.

This series depends on [2-3], which are small fixes identified through this
patch set. The organization of patches in this series is as follows:

- General fixes for bugs which are unlikely to be triggered outside of this
  series
- Changes to IMX8 container images to facilitate testing
- General prep. work, particularly regarding linker issues
- The tests themselves

Mostly-passing CI at [4]; I have since fixed the typo/missing cast.

[1] https://lore.kernel.org/all/20230731224304.111081-1-sean.ander...@seco.com/
[2] https://lore.kernel.org/all/20230930204246.515254-1-sean...@gmail.com/
[3] https://lore.kernel.org/all/20231008014748.1987840-1-sean...@gmail.com/
[4] https://source.denx.de/u-boot/custodians/u-boot-clk/-/pipelines/18091


Sean Anderson (26):
  spl: legacy: Fix referencing _image_binary_end
  spl: nor: Don't allocate header on stack
  spl: fit: Fix entry point for SPL_LOAD_FIT_FULL
  arm: imx: Fix i.MX8 container load address
  arm: imx: Add newlines after error messages
  arm: imx: Add function to validate i.MX8 containers
  arm: imx: Check header before calling spl_load_imx_container
  Move i.MX8 container image loading support to common/spl
  spl: Allow enabling SPL_OF_REAL and SPL_OF_PLATDATA at the same time
  lib: acpi: Fix linking SPL when ACPIGEN is enabled
  fs: ext4: Fix building ext4 in SPL if write is enabled
  fs: Compile in sandbox filesystem in SPL if it is enabled
  net: Fix compiling SPL when fastboot is enabled
  net: bootp: Move port numbers to header
  net: bootp: Fall back to BOOTP from DHCP when unit testing
  spl: Don't cache devices when UNIT_TEST is enabled
  spl: Use map_sysmem where appropriate
  test: spl: Split tests up and use some configs
  test: spl: Fix spl_test_load not failing if fname doesn't exist
  test: spl: Add functions to create images
  test: spl: Add functions to create filesystems
  test: spl: Add a test for spl_blk_load_image
  test: spl: Add a test for the MMC load method
  test: spl: Add a test for the NET load method
  test: spl: Add a test for the NOR load method
  test: spl: Add a test for the SPI load method

 .azure-pipelines.yml  |   4 +
 .gitlab-ci.yml|   7 +
 MAINTAINERS   |   2 +
 arch/arm/include/asm/mach-imx/ahab.h  |   2 +-
 arch/arm/mach-imx/Kconfig |  13 -
 arch/arm/mach-imx/Makefile|   2 +-
 arch/arm/mach-imx/cmd_dek.c   |   4 +-
 arch/arm/mach-imx/ele_ahab.c  |   4 +-
 arch/arm/mach-imx/image-container.c   |   4 +-
 arch/arm/mach-imx/imx8/ahab.c |   4 +-
 arch/arm/mach-imx/spl_imx_romapi.c|   8 +-
 arch/sandbox/cpu/spl.c|   3 +
 arch/sandbox/cpu/start.c  |   9 +-
 arch/sandbox/cpu/u-boot-spl.lds   |   2 +
 arch/sandbox/include/asm/spl.h|   3 +
 common/spl/Kconfig|  15 +
 common/spl/Makefile   |   1 +
 common/spl/spl.c  |   4 +-
 common/spl/spl_blk_fs.c   |   6 +-
 common/spl/spl_ext.c  |   4 +-
 common/spl/spl_fat.c  |  13 +-
 common/spl/spl_fit.c  |  40 +-
 .../spl/spl_imx_container.c   |  12 +-
 common/spl/spl_legacy.c   |   8 +-
 common/spl/spl_mmc.c  |  11 +-
 common/spl/spl_nand.c |   4 +-
 common/spl/spl_net.c  |  10 +-
 common/spl/spl_nor.c  |  18 +-
 

[PATCH 04/26] arm: imx: Fix i.MX8 container load address

2023-10-11 Thread Sean Anderson
We should load images to their destination, not their entry point.

Fixes: 7b86cd4274e ("imx8: support parsing i.MX8 Container file")
Signed-off-by: Sean Anderson 
---

 arch/arm/mach-imx/parse-container.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-imx/parse-container.c 
b/arch/arm/mach-imx/parse-container.c
index e2a9e2b2732..d7275a58c17 100644
--- a/arch/arm/mach-imx/parse-container.c
+++ b/arch/arm/mach-imx/parse-container.c
@@ -45,7 +45,7 @@ static struct boot_img_t *read_auth_image(struct 
spl_image_info *spl_image,
debug("%s: container: %p sector: %lu sectors: %u\n", __func__,
  container, sector, sectors);
if (info->read(info, sector, sectors,
-  (void *)images[image_index].entry) != sectors) {
+  (void *)images[image_index].dst) != sectors) {
printf("%s wrong\n", __func__);
return NULL;
}
-- 
2.37.1



[PATCH 01/26] spl: legacy: Fix referencing _image_binary_end

2023-10-11 Thread Sean Anderson
On non-arm architectures, _image_binary_end is defined as a ulong and not a
char[]. Dereference it when accessing it, which is correct for both.

Fixes: 1b8a1be1a1f ("spl: spl_legacy: Fix spl_end address")
Signed-off-by: Sean Anderson 
---

 common/spl/spl_legacy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/spl/spl_legacy.c b/common/spl/spl_legacy.c
index 095443c63d8..e9564e5c2a5 100644
--- a/common/spl/spl_legacy.c
+++ b/common/spl/spl_legacy.c
@@ -19,7 +19,7 @@
 static void spl_parse_legacy_validate(uintptr_t start, uintptr_t size)
 {
uintptr_t spl_start = (uintptr_t)_start;
-   uintptr_t spl_end = (uintptr_t)_image_binary_end;
+   uintptr_t spl_end = (uintptr_t)&_image_binary_end;
uintptr_t end = start + size;
 
if ((start >= spl_start && start < spl_end) ||
-- 
2.37.1



[PATCH v4 4/4] sunxi: psci: implement PSCI on R528

2023-10-11 Thread Sam Edwards
This patch adds the necessary code to make nonsec booting and PSCI
secondary core management functional on the R528/T113.

Signed-off-by: Sam Edwards 
Tested-by: Maksim Kiselev 
Tested-by: Kevin Amadiva 
---
 arch/arm/cpu/armv7/Kconfig  |  3 ++-
 arch/arm/cpu/armv7/sunxi/psci.c | 47 -
 arch/arm/mach-sunxi/Kconfig |  4 +++
 3 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/arch/arm/cpu/armv7/Kconfig b/arch/arm/cpu/armv7/Kconfig
index f015d133cb..4eb34b7b44 100644
--- a/arch/arm/cpu/armv7/Kconfig
+++ b/arch/arm/cpu/armv7/Kconfig
@@ -61,8 +61,9 @@ config ARMV7_SECURE_MAX_SIZE
 config ARM_GIC_BASE_ADDRESS
hex
depends on ARMV7_NONSEC
-   depends on ARCH_EXYNOS5
+   depends on ARCH_EXYNOS5 || MACH_SUN8I_R528
default 0x1048 if ARCH_EXYNOS5
+   default 0x0302 if MACH_SUN8I_R528
help
  Override the GIC base address if the Arm Cortex defined
  CBAR/PERIPHBASE system register holds the wrong value.
diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
index 207aa6bc4b..b03a865483 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.c
+++ b/arch/arm/cpu/armv7/sunxi/psci.c
@@ -47,6 +47,19 @@
 #define SUN8I_R40_PWR_CLAMP(cpu)   (0x120 + (cpu) * 0x4)
 #define SUN8I_R40_SRAMC_SOFT_ENTRY_REG0(0xbc)
 
+/*
+ * R528 is also different, as it has both cores powered up (but held in reset
+ * state) after the SoC is reset. Like the R40, it uses a "soft" entry point
+ * address register, but unlike the R40, it uses a newer "CPUX" block to manage
+ * CPU state, rather than the older CPUCFG system.
+ */
+#define SUN8I_R528_SOFT_ENTRY  (0x1c8)
+#define SUN8I_R528_C0_RST_CTRL (0x)
+#define SUN8I_R528_C0_CTRL_REG0(0x0010)
+#define SUN8I_R528_C0_CPU_STATUS   (0x0080)
+
+#define SUN8I_R528_C0_STATUS_STANDBYWFI(16)
+
 static void __secure cp15_write_cntp_tval(u32 tval)
 {
asm volatile ("mcr p15, 0, %0, c14, c2, 0" : : "r" (tval));
@@ -103,10 +116,12 @@ static void __secure clamp_set(u32 *clamp)
 
 static void __secure sunxi_cpu_set_entry(int __always_unused cpu, void *entry)
 {
-   /* secondary core entry address is programmed differently on R40 */
if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
writel((u32)entry,
   SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
+   } else if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+   writel((u32)entry,
+  SUNXI_R_CPUCFG_BASE + SUN8I_R528_SOFT_ENTRY);
} else {
writel((u32)entry, SUNXI_CPUCFG_BASE + SUNXI_PRIV0);
}
@@ -125,6 +140,9 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
} else if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
clamp = (void *)SUNXI_CPUCFG_BASE + SUN8I_R40_PWR_CLAMP(cpu);
pwroff = (void *)SUNXI_CPUCFG_BASE + SUN8I_R40_PWROFF;
+   } else if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+   /* R528 leaves both cores powered up, manages them via reset */
+   return;
} else {
if (IS_ENABLED(CONFIG_MACH_SUN6I) ||
IS_ENABLED(CONFIG_MACH_SUN8I_H3))
@@ -152,11 +170,27 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
 
 static void __secure sunxi_cpu_set_reset(int cpu, bool reset)
 {
+   if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+   if (reset)
+   clrbits_le32(SUNXI_CPUCFG_BASE + SUN8I_R528_C0_RST_CTRL,
+BIT(cpu));
+   else
+   setbits_le32(SUNXI_CPUCFG_BASE + SUN8I_R528_C0_RST_CTRL,
+BIT(cpu));
+
+   return;
+   }
+
writel(reset ? 0b00 : 0b11, SUNXI_CPUCFG_BASE + SUNXI_CPU_RST(cpu));
 }
 
 static void __secure sunxi_cpu_set_locking(int cpu, bool lock)
 {
+   if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+   /* Not required on R528 */
+   return;
+   }
+
if (lock)
clrbits_le32(SUNXI_CPUCFG_BASE + SUNXI_DBG_CTRL1, BIT(cpu));
else
@@ -165,11 +199,22 @@ static void __secure sunxi_cpu_set_locking(int cpu, bool 
lock)
 
 static bool __secure sunxi_cpu_poll_wfi(int cpu)
 {
+   if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+   return !!(readl(SUNXI_CPUCFG_BASE + SUN8I_R528_C0_CPU_STATUS) &
+ BIT(SUN8I_R528_C0_STATUS_STANDBYWFI + cpu));
+   }
+
return !!(readl(SUNXI_CPUCFG_BASE + SUNXI_CPU_STATUS(cpu)) & BIT(2));
 }
 
 static void __secure sunxi_cpu_invalidate_cache(int cpu)
 {
+   if (IS_ENABLED(CONFIG_MACH_SUN8I_R528)) {
+   clrbits_le32(SUNXI_CPUCFG_BASE + SUN8I_R528_C0_CTRL_REG0,
+BIT(cpu));
+   return;
+   }
+
clrbits_le32(SUNXI_CPUCFG_BASE + SUNXI_GEN_CTRL, BIT(cpu));
 

[PATCH v4 2/4] sunxi: psci: refactor register access to separate functions

2023-10-11 Thread Sam Edwards
This is to prepare for R528, which does not have the typical
"CPUCFG" block; it has a "CPUX" block which provides these
same functions but is organized differently.

Moving the hardware-access bits to their own functions separates the
logic from the hardware so we can reuse the same logic.

Signed-off-by: Sam Edwards 
Reviewed-by: Andre Przywara 
---
 arch/arm/cpu/armv7/sunxi/psci.c | 66 +++--
 1 file changed, 47 insertions(+), 19 deletions(-)

diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
index 69fa3f3c2e..27ca9c39e1 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.c
+++ b/arch/arm/cpu/armv7/sunxi/psci.c
@@ -92,7 +92,7 @@ static void __secure clamp_set(u32 *clamp)
writel(0xff, clamp);
 }
 
-static void __secure sunxi_set_entry_address(void *entry)
+static void __secure sunxi_cpu_set_entry(int __always_unused cpu, void *entry)
 {
/* secondary core entry address is programmed differently on R40 */
if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
@@ -149,30 +149,60 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
}
 }
 
-void __secure sunxi_cpu_power_off(u32 cpuid)
+static void __secure sunxi_cpu_set_reset(int cpu, bool reset)
 {
struct sunxi_cpucfg_reg *cpucfg =
(struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+
+   writel(reset ? 0b00 : 0b11, >cpu[cpu].rst);
+}
+
+static void __secure sunxi_cpu_set_locking(int cpu, bool lock)
+{
+   struct sunxi_cpucfg_reg *cpucfg =
+   (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+
+   if (lock)
+   clrbits_le32(>dbg_ctrl1, BIT(cpu));
+   else
+   setbits_le32(>dbg_ctrl1, BIT(cpu));
+}
+
+static bool __secure sunxi_cpu_poll_wfi(int cpu)
+{
+   struct sunxi_cpucfg_reg *cpucfg =
+   (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+
+   return !!(readl(>cpu[cpu].status) & BIT(2));
+}
+
+static void __secure sunxi_cpu_invalidate_cache(int cpu)
+{
+   struct sunxi_cpucfg_reg *cpucfg =
+   (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+
+   clrbits_le32(>gen_ctrl, BIT(cpu));
+}
+
+static void __secure sunxi_cpu_power_off(u32 cpuid)
+{
u32 cpu = cpuid & 0x3;
 
/* Wait for the core to enter WFI */
-   while (1) {
-   if (readl(>cpu[cpu].status) & BIT(2))
-   break;
+   while (!sunxi_cpu_poll_wfi(cpu))
__mdelay(1);
-   }
 
/* Assert reset on target CPU */
-   writel(0, >cpu[cpu].rst);
+   sunxi_cpu_set_reset(cpu, true);
 
/* Lock CPU (Disable external debug access) */
-   clrbits_le32(>dbg_ctrl1, BIT(cpu));
+   sunxi_cpu_set_locking(cpu, true);
 
/* Power down CPU */
sunxi_cpu_set_power(cpuid, false);
 
-   /* Unlock CPU (Disable external debug access) */
-   setbits_le32(>dbg_ctrl1, BIT(cpu));
+   /* Unlock CPU (Reenable external debug access) */
+   sunxi_cpu_set_locking(cpu, false);
 }
 
 static u32 __secure cp15_read_scr(void)
@@ -229,33 +259,31 @@ out:
 int __secure psci_cpu_on(u32 __always_unused unused, u32 mpidr, u32 pc,
 u32 context_id)
 {
-   struct sunxi_cpucfg_reg *cpucfg =
-   (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
u32 cpu = (mpidr & 0x3);
 
/* store target PC and context id */
psci_save(cpu, pc, context_id);
 
/* Set secondary core power on PC */
-   sunxi_set_entry_address(_cpu_entry);
+   sunxi_cpu_set_entry(cpu, _cpu_entry);
 
/* Assert reset on target CPU */
-   writel(0, >cpu[cpu].rst);
+   sunxi_cpu_set_reset(cpu, true);
 
/* Invalidate L1 cache */
-   clrbits_le32(>gen_ctrl, BIT(cpu));
+   sunxi_cpu_invalidate_cache(cpu);
 
/* Lock CPU (Disable external debug access) */
-   clrbits_le32(>dbg_ctrl1, BIT(cpu));
+   sunxi_cpu_set_locking(cpu, true);
 
/* Power up target CPU */
sunxi_cpu_set_power(cpu, true);
 
/* De-assert reset on target CPU */
-   writel(BIT(1) | BIT(0), >cpu[cpu].rst);
+   sunxi_cpu_set_reset(cpu, false);
 
-   /* Unlock CPU (Disable external debug access) */
-   setbits_le32(>dbg_ctrl1, BIT(cpu));
+   /* Unlock CPU (Reenable external debug access) */
+   sunxi_cpu_set_locking(cpu, false);
 
return ARM_PSCI_RET_SUCCESS;
 }
-- 
2.41.0



[PATCH v4 3/4] sunxi: psci: stop modeling register layout with C structs

2023-10-11 Thread Sam Edwards
Since the sunxi support nowadays generally prefers #defined register
offsets instead of modeling register layouts using C structs, now is a
good time to do this for PSCI as well. This patch moves away from using
the structs `sunxi_cpucfg_reg` and `sunxi_prcm_reg` in psci.c.

The former struct and its associated header file existed only to support
PSCI code, so also delete them altogether.

Signed-off-by: Sam Edwards 
---
 arch/arm/cpu/armv7/sunxi/psci.c  | 57 
 arch/arm/include/asm/arch-sunxi/cpucfg.h | 67 
 2 files changed, 23 insertions(+), 101 deletions(-)
 delete mode 100644 arch/arm/include/asm/arch-sunxi/cpucfg.h

diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
index 27ca9c39e1..207aa6bc4b 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.c
+++ b/arch/arm/cpu/armv7/sunxi/psci.c
@@ -11,8 +11,6 @@
 #include 
 
 #include 
-#include 
-#include 
 #include 
 #include 
 #include 
@@ -27,6 +25,17 @@
 #defineGICD_BASE   (SUNXI_GIC400_BASE + GIC_DIST_OFFSET)
 #defineGICC_BASE   (SUNXI_GIC400_BASE + GIC_CPU_OFFSET_A15)
 
+/*
+ * Offsets into the CPUCFG block applicable to most SUNXIs.
+ */
+#define SUNXI_CPU_RST(cpu) (0x40 + (cpu) * 0x40 + 0x0)
+#define SUNXI_CPU_STATUS(cpu)  (0x40 + (cpu) * 0x40 + 0x8)
+#define SUNXI_GEN_CTRL (0x184)
+#define SUNXI_PRIV0(0x1a4)
+#define SUN7I_CPU1_PWR_CLAMP   (0x1b0)
+#define SUN7I_CPU1_PWROFF  (0x1b4)
+#define SUNXI_DBG_CTRL1(0x1e4)
+
 /*
  * R40 is different from other single cluster SoCs.
  *
@@ -99,10 +108,7 @@ static void __secure sunxi_cpu_set_entry(int 
__always_unused cpu, void *entry)
writel((u32)entry,
   SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
} else {
-   struct sunxi_cpucfg_reg *cpucfg =
-   (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
-
-   writel((u32)entry, >priv0);
+   writel((u32)entry, SUNXI_CPUCFG_BASE + SUNXI_PRIV0);
}
 }
 
@@ -110,26 +116,21 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
 {
u32 *clamp = NULL;
u32 *pwroff;
-   struct sunxi_cpucfg_reg *cpucfg =
-   (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
 
/* sun7i (A20) is different from other single cluster SoCs */
if (IS_ENABLED(CONFIG_MACH_SUN7I)) {
-   clamp = >cpu1_pwr_clamp;
-   pwroff = >cpu1_pwroff;
+   clamp = (void *)SUNXI_CPUCFG_BASE + SUN7I_CPU1_PWR_CLAMP;
+   pwroff = (void *)SUNXI_CPUCFG_BASE + SUN7I_CPU1_PWROFF;
cpu = 0;
} else if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
-   clamp = (void *)cpucfg + SUN8I_R40_PWR_CLAMP(cpu);
-   pwroff = (void *)cpucfg + SUN8I_R40_PWROFF;
+   clamp = (void *)SUNXI_CPUCFG_BASE + SUN8I_R40_PWR_CLAMP(cpu);
+   pwroff = (void *)SUNXI_CPUCFG_BASE + SUN8I_R40_PWROFF;
} else {
-   struct sunxi_prcm_reg *prcm =
-   (struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
-
if (IS_ENABLED(CONFIG_MACH_SUN6I) ||
IS_ENABLED(CONFIG_MACH_SUN8I_H3))
-   clamp = >cpu_pwr_clamp[cpu];
+   clamp = (void *)SUNXI_PRCM_BASE + 0x140 + cpu * 0x4;
 
-   pwroff = >cpu_pwroff;
+   pwroff = (void *)SUNXI_PRCM_BASE + 0x100;
}
 
if (on) {
@@ -151,37 +152,25 @@ static void __secure sunxi_cpu_set_power(int cpu, bool on)
 
 static void __secure sunxi_cpu_set_reset(int cpu, bool reset)
 {
-   struct sunxi_cpucfg_reg *cpucfg =
-   (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
-
-   writel(reset ? 0b00 : 0b11, >cpu[cpu].rst);
+   writel(reset ? 0b00 : 0b11, SUNXI_CPUCFG_BASE + SUNXI_CPU_RST(cpu));
 }
 
 static void __secure sunxi_cpu_set_locking(int cpu, bool lock)
 {
-   struct sunxi_cpucfg_reg *cpucfg =
-   (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
-
if (lock)
-   clrbits_le32(>dbg_ctrl1, BIT(cpu));
+   clrbits_le32(SUNXI_CPUCFG_BASE + SUNXI_DBG_CTRL1, BIT(cpu));
else
-   setbits_le32(>dbg_ctrl1, BIT(cpu));
+   setbits_le32(SUNXI_CPUCFG_BASE + SUNXI_DBG_CTRL1, BIT(cpu));
 }
 
 static bool __secure sunxi_cpu_poll_wfi(int cpu)
 {
-   struct sunxi_cpucfg_reg *cpucfg =
-   (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
-
-   return !!(readl(>cpu[cpu].status) & BIT(2));
+   return !!(readl(SUNXI_CPUCFG_BASE + SUNXI_CPU_STATUS(cpu)) & BIT(2));
 }
 
 static void __secure sunxi_cpu_invalidate_cache(int cpu)
 {
-   struct sunxi_cpucfg_reg *cpucfg =
-   (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
-
-   clrbits_le32(>gen_ctrl, BIT(cpu));
+   

[PATCH v4 1/4] sunxi: psci: clean away preprocessor macros

2023-10-11 Thread Sam Edwards
This patch restructures psci.c to get away from the "many different
function definitions switched by #ifdef" paradigm to the preferred style
of having a single function definition with `if (IS_ENABLED(...))` to
make the optimizer include only the appropriate function bodies instead.

There are no functional changes here.

Signed-off-by: Sam Edwards 
Reviewed-by: Andre Przywara 
---
 arch/arm/cpu/armv7/sunxi/psci.c | 103 +---
 1 file changed, 43 insertions(+), 60 deletions(-)

diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
index e1d3638b5c..69fa3f3c2e 100644
--- a/arch/arm/cpu/armv7/sunxi/psci.c
+++ b/arch/arm/cpu/armv7/sunxi/psci.c
@@ -76,11 +76,8 @@ static void __secure __mdelay(u32 ms)
isb();
 }
 
-static void __secure clamp_release(u32 __maybe_unused *clamp)
+static void __secure clamp_release(u32 *clamp)
 {
-#if defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN7I) || \
-   defined(CONFIG_MACH_SUN8I_H3) || \
-   defined(CONFIG_MACH_SUN8I_R40)
u32 tmp = 0x1ff;
do {
tmp >>= 1;
@@ -88,83 +85,69 @@ static void __secure clamp_release(u32 __maybe_unused 
*clamp)
} while (tmp);
 
__mdelay(10);
-#endif
 }
 
-static void __secure clamp_set(u32 __maybe_unused *clamp)
+static void __secure clamp_set(u32 *clamp)
 {
-#if defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN7I) || \
-   defined(CONFIG_MACH_SUN8I_H3) || \
-   defined(CONFIG_MACH_SUN8I_R40)
writel(0xff, clamp);
-#endif
 }
 
-static void __secure sunxi_power_switch(u32 *clamp, u32 *pwroff, bool on,
-   int cpu)
+static void __secure sunxi_set_entry_address(void *entry)
 {
-   if (on) {
-   /* Release power clamp */
-   clamp_release(clamp);
-
-   /* Clear power gating */
-   clrbits_le32(pwroff, BIT(cpu));
+   /* secondary core entry address is programmed differently on R40 */
+   if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
+   writel((u32)entry,
+  SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
} else {
-   /* Set power gating */
-   setbits_le32(pwroff, BIT(cpu));
+   struct sunxi_cpucfg_reg *cpucfg =
+   (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
 
-   /* Activate power clamp */
-   clamp_set(clamp);
+   writel((u32)entry, >priv0);
}
 }
 
-#ifdef CONFIG_MACH_SUN8I_R40
-/* secondary core entry address is programmed differently on R40 */
-static void __secure sunxi_set_entry_address(void *entry)
-{
-   writel((u32)entry,
-  SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
-}
-#else
-static void __secure sunxi_set_entry_address(void *entry)
+static void __secure sunxi_cpu_set_power(int cpu, bool on)
 {
+   u32 *clamp = NULL;
+   u32 *pwroff;
struct sunxi_cpucfg_reg *cpucfg =
(struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
 
-   writel((u32)entry, >priv0);
-}
-#endif
+   /* sun7i (A20) is different from other single cluster SoCs */
+   if (IS_ENABLED(CONFIG_MACH_SUN7I)) {
+   clamp = >cpu1_pwr_clamp;
+   pwroff = >cpu1_pwroff;
+   cpu = 0;
+   } else if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
+   clamp = (void *)cpucfg + SUN8I_R40_PWR_CLAMP(cpu);
+   pwroff = (void *)cpucfg + SUN8I_R40_PWROFF;
+   } else {
+   struct sunxi_prcm_reg *prcm =
+   (struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
 
-#ifdef CONFIG_MACH_SUN7I
-/* sun7i (A20) is different from other single cluster SoCs */
-static void __secure sunxi_cpu_set_power(int __always_unused cpu, bool on)
-{
-   struct sunxi_cpucfg_reg *cpucfg =
-   (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+   if (IS_ENABLED(CONFIG_MACH_SUN6I) ||
+   IS_ENABLED(CONFIG_MACH_SUN8I_H3))
+   clamp = >cpu_pwr_clamp[cpu];
 
-   sunxi_power_switch(>cpu1_pwr_clamp, >cpu1_pwroff,
-  on, 0);
-}
-#elif defined CONFIG_MACH_SUN8I_R40
-static void __secure sunxi_cpu_set_power(int cpu, bool on)
-{
-   struct sunxi_cpucfg_reg *cpucfg =
-   (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
+   pwroff = >cpu_pwroff;
+   }
 
-   sunxi_power_switch((void *)cpucfg + SUN8I_R40_PWR_CLAMP(cpu),
-  (void *)cpucfg + SUN8I_R40_PWROFF,
-  on, cpu);
-}
-#else /* ! CONFIG_MACH_SUN7I && ! CONFIG_MACH_SUN8I_R40 */
-static void __secure sunxi_cpu_set_power(int cpu, bool on)
-{
-   struct sunxi_prcm_reg *prcm =
-   (struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
+   if (on) {
+   /* Release power clamp */
+   if (clamp)
+   clamp_release(clamp);
 
-   sunxi_power_switch(>cpu_pwr_clamp[cpu], 

[PATCH v4 0/4] Allwinner R528/T113s PSCI

2023-10-11 Thread Sam Edwards
Hi list,

This is the third, and hopefully final, version of my patchset for PSCI support
on R528/T113-s3. This one, as always, depends on Andre Przywara's T113s support
series (v2), available on the list and also located on a Git branch at:
https://source.denx.de/u-boot/custodians/u-boot-sunxi.git t113s-v2

NOTE THAT THIS ALSO depends on the following commit on master:
3d5e52bd97 ("ARM: psci: move GIC address override to Kconfig")

For testing: I can confirm that patch 2/4 results in no change to machine code
whatsoever on any supported target. Patch 1/4 results in a minor machine code
change on R40 only. Patch 3/4 will likely require testing on each of the 4
"special case" sunxi targets (sun6i, sun7i, R40, H3) to ensure that register
offsets are kept consistent. Patch 4/4 needs testing on R528 only.

Warm regards,
Sam

Changes v3->v4:
- The GIC base address override is done through Kconfig, instead of in
  sunxi-common.h

Changes v2->v3:
- Fix missing `cpu=0;` for the sun7i power management case.
- Make sunxi_cpu_power_off() a static function, per feedback on v2.
- Kevin Amadiva of MEC electronics got in touch with me off-list, reported
  success bringing up CPU1 of a T113 with this patchset, and kindly provided
  me with a Tested-by tag for patch 4/4.
- Remove a comment about R40/R528 being special, per feedback on v2.
- Simplify an if statement, per feedback on v2.
- Add missing 'select' directives to the R528 Kconfig, per feedback on v2.

Changes v1->v2:
- Power clamp is now adjusted ONLY on sun{6,7}i, H3, R40. The previous version
  was mistakenly doing this EXCEPT on those machines.
- Flattened sunxi_power_switch() into sunxi_cpu_set_power() for simplicity's
  sake.
- Moved the "power clamp is not NULL" conditional into sunxi_cpu_set_power().
- Removed unnecessary H6 special-case, since H6 is actually ARM64.
- Renamed SUNXI_CPUX_BASE to SUNXI_CPUCFG_BASE, to mirror expected changes in
  Andre's v2 of the R528 series (we decided against using a new name for this
  block).
- Removed sunxi_cpucfg_reg struct, and stopped using the PRCM struct in psci.c.

Sam Edwards (4):
  sunxi: psci: clean away preprocessor macros
  sunxi: psci: refactor register access to separate functions
  sunxi: psci: stop modeling register layout with C structs
  sunxi: psci: implement PSCI on R528

 arch/arm/cpu/armv7/Kconfig   |   3 +-
 arch/arm/cpu/armv7/sunxi/psci.c  | 185 ++-
 arch/arm/include/asm/arch-sunxi/cpucfg.h |  67 
 arch/arm/mach-sunxi/Kconfig  |   4 +
 4 files changed, 121 insertions(+), 138 deletions(-)
 delete mode 100644 arch/arm/include/asm/arch-sunxi/cpucfg.h

-- 
2.41.0



Re: [PATCH v6 08/14] firmware: scmi: add a version check against base protocol

2023-10-11 Thread AKASHI Takahiro
Hi Etienne,

Thank you again for your review.

On Wed, Oct 11, 2023 at 03:44:36PM +, Etienne CARRIERE - foss wrote:
> > From: U-Boot  on behalf of AKASHI Takahiro 
> > 
> > Sent: Wednesday, October 11, 2023 12:07 PM
> > 
> > In SCMI base protocol version 2 (0x2), new interfaces,
> > BASE_SET_DEVICE_PERMISSIONS/BASE_SET_PROTOCOL_PERMISSIONS/
> > BASE_RESET_AGENT_CONFIGURATION, were added. Moreover, the api of
> > BASE_DISCOVER_AGENT was changed to support self-agent discovery.
> > 
> > So the driver expects SCMI firmware support version 2 of base protocol.
> > 
> > Signed-off-by: AKASHI Takahiro 
> > ---
> > v6
> > * new commit
> > ---
> >  drivers/firmware/scmi/base.c | 8 
> >  1 file changed, 8 insertions(+)
> > 
> > diff --git a/drivers/firmware/scmi/base.c b/drivers/firmware/scmi/base.c
> > index ee84e261945a..1d41a8a98fc6 100644
> > --- a/drivers/firmware/scmi/base.c
> > +++ b/drivers/firmware/scmi/base.c
> > @@ -481,6 +481,7 @@ static int 
> > scmi_base_reset_agent_configuration_int(struct udevice *dev,
> >   */
> >  static int scmi_base_probe(struct udevice *dev)
> >  {
> > +   u32 version;
> > int ret;
> > 
> > ret = devm_scmi_of_get_channel(dev);
> > @@ -488,6 +489,13 @@ static int scmi_base_probe(struct udevice *dev)
> > dev_err(dev, "get_channel failed\n");
> > return ret;
> > }
> > +   ret = scmi_base_protocol_version_int(dev, );
> > +   if (ret) {
> > +   dev_err(dev, "getting protocol version failed\n");
> > +   return ret;
> > +   }
> > +   if (version < SCMI_BASE_PROTOCOL_VERSION)
> > +   return -EINVAL;
> 
> LGTM. The open source SCMI server implementations I'm aware of (scp-firmware, 
> tf-a and optee-os) all report SCMI Base protocol version v2.0.

Yeah, I also confirmed this.

> Reviewed-by: Etienne Carriere 
> 
> That said, maybe a more flexible implementation would support both version 
> v1.0 (0x1) and v2.0

SCMI v2.0 (not the base protocol, but the specification itself)
introduced base protocol v2.0 and the spec was published in 2019.
Along with the status of the existing SCMI server implementation (as you
mentioned above), I doubt any chance that there comes up any new SCMI with
the base protocol v1.0 in the future.

> but disable permission commands for v1.0 compliant servers. Maybe in a later 
> change, if there is a need for.

Yes, we can make a tweak later. But anyhow "permission" commands are
categorized as optional even in v2.0 (i.e. the firmware may still return
SCMI_NOT_SUPPORTED) and U-Boot has no framework to utilize them for now.

BTW, I will not add a version check against all the existing protocols
(you implemented) as they utilize only the interfaces defined in each one's 
v1.0.

Thanks,
-Takahiro Akashi

> I fear using such strict minima protocol version values for other SCMI 
> procotols make U-Boot client too restrictive.
> 
> BR,
> Etienne
> 
> > 
> > return ret;
> >  }
> > --
> > 2.34.1
> > 
> > 


Re: [PATCH v7 2/2] schemas: Add some common reserved-memory usages

2023-10-11 Thread Ard Biesheuvel
On Sat, 7 Oct 2023 at 02:03, Simon Glass  wrote:
>
> Hi Ard,
>
> On Fri, 6 Oct 2023 at 17:00, Ard Biesheuvel  wrote:
> >
> > On Fri, 6 Oct 2023 at 20:17, Simon Glass  wrote:
> > >
> > > Hi Ard,
> > >
> > > On Fri, 6 Oct 2023 at 11:33, Ard Biesheuvel  wrote:
> > > >
> > > > On Mon, 2 Oct 2023 at 19:54, Simon Glass  wrote:
> > > > >
> > > > > Hi Rob,
> > > > >
> > > > > On Tue, 26 Sept 2023 at 13:42, Simon Glass  wrote:
> > > > > >
> > > > > > It is common to split firmware into 'Platform Init', which does the
> > > > > > initial hardware setup and a "Payload" which selects the OS to be 
> > > > > > booted.
> > > > > > Thus an handover interface is required between these two pieces.
> > > > > >
> > > > > > Where UEFI boot-time services are not available, but UEFI firmware 
> > > > > > is
> > > > > > present on either side of this interface, information about memory 
> > > > > > usage
> > > > > > and attributes must be presented to the "Payload" in some form.
> > > > > >
> > > > > > This aims to provide an small schema addition for the memory mapping
> > > > > > needed to keep these two pieces working together well.
> > > > > >
> > > > > > Signed-off-by: Simon Glass 
> > > > > > ---
> > > > > >
> > > > > > Changes in v7:
> > > > > > - Rename acpi-reclaim to acpi
> > > > > > - Drop individual mention of when memory can be reclaimed
> > > > > > - Rewrite the item descriptions
> > > > > > - Add back the UEFI text (with trepidation)
> > > > >
> > > > > I am again checking on this series. Can it be applied, please?
> > > > >
> > > >
> > > > Apologies for the delay in response. I have been away.
> > >
> > > OK, I hope you had a nice trip.
> > >
> >
> > Thanks, it was wonderful!
> >
> > > >
> > > > >
> > > > > >
> > > > > > Changes in v6:
> > > > > > - Drop mention of UEFI
> > > > > > - Use compatible strings instead of node names
> > > > > >
> > > > > > Changes in v5:
> > > > > > - Drop the memory-map node (should have done that in v4)
> > > > > > - Tidy up schema a bit
> > > > > >
> > > > > > Changes in v4:
> > > > > > - Make use of the reserved-memory node instead of creating a new one
> > > > > >
> > > > > > Changes in v3:
> > > > > > - Reword commit message again
> > > > > > - cc a lot more people, from the FFI patch
> > > > > > - Split out the attributes into the /memory nodes
> > > > > >
> > > > > > Changes in v2:
> > > > > > - Reword commit message
> > > > > >
> > > > > >  .../reserved-memory/common-reserved.yaml  | 71 
> > > > > > +++
> > > > > >  1 file changed, 71 insertions(+)
> > > > > >  create mode 100644 
> > > > > > dtschema/schemas/reserved-memory/common-reserved.yaml
> > > > > >
> > > > > > diff --git a/dtschema/schemas/reserved-memory/common-reserved.yaml 
> > > > > > b/dtschema/schemas/reserved-memory/common-reserved.yaml
> > > > > > new file mode 100644
> > > > > > index 000..f7fbdfd
> > > > > > --- /dev/null
> > > > > > +++ b/dtschema/schemas/reserved-memory/common-reserved.yaml
> > > > > > @@ -0,0 +1,71 @@
> > > > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > > > > +%YAML 1.2
> > > > > > +---
> > > > > > +$id: 
> > > > > > http://devicetree.org/schemas/reserved-memory/common-reserved.yaml#
> > > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > > +
> > > > > > +title: Common memory reservations
> > > > > > +
> > > > > > +description: |
> > > > > > +  Specifies that the reserved memory region can be used for the 
> > > > > > purpose
> > > > > > +  indicated by its compatible string.
> > > > > > +
> > > > > > +  Clients may reuse this reserved memory if they understand what 
> > > > > > it is for,
> > > > > > +  subject to the notes below.
> > > > > > +
> > > > > > +maintainers:
> > > > > > +  - Simon Glass 
> > > > > > +
> > > > > > +allOf:
> > > > > > +  - $ref: reserved-memory.yaml
> > > > > > +
> > > > > > +properties:
> > > > > > +  compatible:
> > > > > > +description: |
> > > > > > +  This describes some common memory reservations, with the 
> > > > > > compatible
> > > > > > +  string indicating what it is used for:
> > > > > > +
> > > > > > + acpi: Advanced Configuration and Power Interface (ACPI) 
> > > > > > tables
> > > > > > + acpi-nvs: ACPI Non-Volatile-Sleeping Memory (NVS). This 
> > > > > > is reserved by
> > > > > > +   the firmware for its use and is required to be saved 
> > > > > > and restored
> > > > > > +   across an NVS sleep
> > > > > > + boot-code: Contains code used for booting which is not 
> > > > > > needed by the OS
> > > > > > + boot-code: Contains data used for booting which is not 
> > > > > > needed by the OS
> > > > > > + runtime-code: Contains code used for interacting with the 
> > > > > > system when
> > > > > > +   running the OS
> > > > > > + runtime-data: Contains data used for interacting with the 
> > > > > > system when
> > > > > > +   running the OS
> > > > > > +
> > > > > > +enum:
> > > 

[PATCH v5] bootstd: sata: Add bootstd support for ahci sata

2023-10-11 Thread Tony Dinh
Add ahci sata bootdev and corresponding hunting function.

Signed-off-by: Tony Dinh 
---

Changes in v5:
- In bootmeth_script script_boot(), it's unnecessary to check for ret so
remove it. While we're here, also initialize ret in declaration.

Changes in v4:
- Revise logic in bootmeth_script() to set devtype to sata for non-scsi
SATA device
- Rewrite sata_rescan() logic to properly remove all devices before probing
- Add description to sata_rescan() header

Changes in v3:
- Correct drivers/ata/Makefile to compile sata_bootdev only if
ahci sata is enabled.

Changes in v2:
- set devtype to sata in bootmeth_script for non-scsi SATA device.

 boot/bootmeth_script.c | 16 +++---
 drivers/ata/Makefile   |  2 +-
 drivers/ata/sata.c | 32 
 drivers/ata/sata_bootdev.c | 62 ++
 include/sata.h |  6 
 5 files changed, 113 insertions(+), 5 deletions(-)
 create mode 100644 drivers/ata/sata_bootdev.c

diff --git a/boot/bootmeth_script.c b/boot/bootmeth_script.c
index 345114dabf..06340e43d2 100644
--- a/boot/bootmeth_script.c
+++ b/boot/bootmeth_script.c
@@ -188,12 +188,20 @@ static int script_boot(struct udevice *dev, struct 
bootflow *bflow)
 {
struct blk_desc *desc = dev_get_uclass_plat(bflow->blk);
ulong addr;
-   int ret;
+   int ret = 0;
 
-   if (desc->uclass_id == UCLASS_USB)
+   if (desc->uclass_id == UCLASS_USB) {
ret = env_set("devtype", "usb");
-   else
-   ret = env_set("devtype", blk_get_devtype(bflow->blk));
+   } else {
+   /* If the uclass is AHCI, but the driver is ATA
+* (not scsi), set devtype to sata
+*/
+   if (IS_ENABLED(CONFIG_SATA) &&
+   desc->uclass_id == UCLASS_AHCI)
+   ret = env_set("devtype", "sata");
+   else
+   ret = env_set("devtype", blk_get_devtype(bflow->blk));
+   }
if (!ret)
ret = env_set_hex("devnum", desc->devnum);
if (!ret)
diff --git a/drivers/ata/Makefile b/drivers/ata/Makefile
index 6e30180b8b..0b6f91098a 100644
--- a/drivers/ata/Makefile
+++ b/drivers/ata/Makefile
@@ -10,7 +10,7 @@ obj-$(CONFIG_SCSI_AHCI) += ahci.o
 obj-$(CONFIG_DWC_AHSATA) += dwc_ahsata.o
 obj-$(CONFIG_FSL_SATA) += fsl_sata.o
 obj-$(CONFIG_LIBATA) += libata.o
-obj-$(CONFIG_SATA) += sata.o
+obj-$(CONFIG_SATA) += sata.o sata_bootdev.o
 obj-$(CONFIG_SATA_CEVA) += sata_ceva.o
 obj-$(CONFIG_SATA_MV) += sata_mv.o
 obj-$(CONFIG_SATA_SIL) += sata_sil.o
diff --git a/drivers/ata/sata.c b/drivers/ata/sata.c
index ce3e9b5a40..f126b84e05 100644
--- a/drivers/ata/sata.c
+++ b/drivers/ata/sata.c
@@ -15,6 +15,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 #ifndef CONFIG_AHCI
 struct blk_desc sata_dev_desc[CONFIG_SYS_SATA_MAX_DEVICE];
@@ -50,6 +52,36 @@ int sata_scan(struct udevice *dev)
return ops->scan(dev);
 }
 
+int sata_rescan(bool verbose)
+{
+   int ret;
+   struct udevice *dev;
+
+   if (verbose)
+   printf("Removing devices on SATA bus...\n");
+
+   blk_unbind_all(UCLASS_AHCI);
+
+   ret = uclass_find_first_device(UCLASS_AHCI, );
+   if (ret || !dev) {
+   printf("Cannot find SATA device (err=%d)\n", ret);
+   return -ENOSYS;
+   }
+
+   ret = device_remove(dev, DM_REMOVE_NORMAL);
+   if (ret) {
+   printf("Cannot remove SATA device '%s' (err=%d)\n", dev->name, 
ret);
+   return -ENOSYS;
+   }
+
+   if (verbose)
+   printf("Rescanning SATA bus for devices...\n");
+
+   ret = uclass_probe_all(UCLASS_AHCI);
+
+   return ret;
+}
+
 #ifndef CONFIG_AHCI
 #ifdef CONFIG_PARTITIONS
 struct blk_desc *sata_get_dev(int dev)
diff --git a/drivers/ata/sata_bootdev.c b/drivers/ata/sata_bootdev.c
new file mode 100644
index 00..f638493ce0
--- /dev/null
+++ b/drivers/ata/sata_bootdev.c
@@ -0,0 +1,62 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Bootdev for sata
+ *
+ * Copyright 2023 Tony Dinh 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+static int sata_bootdev_bind(struct udevice *dev)
+{
+   struct bootdev_uc_plat *ucp = dev_get_uclass_plat(dev);
+
+   ucp->prio = BOOTDEVP_4_SCAN_FAST;
+
+   return 0;
+}
+
+static int sata_bootdev_hunt(struct bootdev_hunter *info, bool show)
+{
+   int ret;
+
+   if (IS_ENABLED(CONFIG_PCI)) {
+   ret = pci_init();
+   if (ret)
+   return ret;
+   }
+
+   ret = sata_rescan(true);
+   if (ret)
+   return ret;
+
+   return 0;
+}
+
+struct bootdev_ops sata_bootdev_ops = {
+};
+
+static const struct udevice_id sata_bootdev_ids[] = {
+   { .compatible = "u-boot,bootdev-sata" },
+   { }
+};
+
+U_BOOT_DRIVER(sata_bootdev) = {
+   .name   = "sata_bootdev",
+   .id = UCLASS_BOOTDEV,

  1   2   3   >