Re: [PATCH 10/15] efi_loader: Add support for logging EFI calls

[Heinrich Schuchardt]

Am 28. Oktober 2024 13:48:01 MEZ schrieb Simon Glass :
>The current logging system suffers from some disadvantages, mainly that
>it writes its output to the console and cannot be easily reviewed.
>
>Add a dedicated log, storing records in a binary format and including
>the result codes and any return values from each call. The log is built
>sequentially in memory and can be reviewed after any EFI operation. It
>could potentially be written to media for later review, but that is not
>implemented so far.

We already have log_debug.

We should use it in EFI_ENTRY, EFI_PRINT, EFI_EXIT.

The missing piece might be a log driver to save the log wherever you want it 
for tests.

Best regards

Heinrich

>
>Signed-off-by: Simon Glass 
>---
>
> MAINTAINERS  |   6 +
> include/bloblist.h   |   1 +
> include/efi.h|   1 +
> include/efi_log.h| 169 ++
> lib/efi_loader/Kconfig   |  19 +++
> lib/efi_loader/Makefile  |   1 +
> lib/efi_loader/efi_log.c | 256 +++
> 7 files changed, 453 insertions(+)
> create mode 100644 include/efi_log.h
> create mode 100644 lib/efi_loader/efi_log.c
>
>diff --git a/MAINTAINERS b/MAINTAINERS
>index 38c714cf46a..9ade0ca4bc3 100644
>--- a/MAINTAINERS
>+++ b/MAINTAINERS
>@@ -1067,6 +1067,12 @@ F:  lib/efi/efi_app.c
> F:scripts/build-efi.sh
> F:test/dm/efi_media.c
> 
>+EFI LOGGING
>+M:Simon Glass 
>+S:Maintained
>+F:include/efi_log.h
>+F:lib/efi_loader/efi_log.c
>+
> EFI PAYLOAD
> M:Heinrich Schuchardt 
> M:Ilias Apalodimas 
>diff --git a/include/bloblist.h b/include/bloblist.h
>index ff32d3fecfd..1e1ca34aa92 100644
>--- a/include/bloblist.h
>+++ b/include/bloblist.h
>@@ -153,6 +153,7 @@ enum bloblist_tag_t {
>   BLOBLISTT_U_BOOT_SPL_HANDOFF= 0xfff000, /* Hand-off info from SPL */
>   BLOBLISTT_VBE   = 0xfff001, /* VBE per-phase state */
>   BLOBLISTT_U_BOOT_VIDEO  = 0xfff002, /* Video info from SPL */
>+  BLOBLISTT_EFI_LOG   = 0xfff003, /* Log of EFI calls */
> };
> 
> /**
>diff --git a/include/efi.h b/include/efi.h
>index 84640cf7b25..f7419b80d4d 100644
>--- a/include/efi.h
>+++ b/include/efi.h
>@@ -127,6 +127,7 @@ static inline void *guidcpy(void *dst, const void *src)
> #define EFI_COMPROMISED_DATA  (EFI_ERROR_MASK | 33)
> #define EFI_IP_ADDRESS_CONFLICT   (EFI_ERROR_MASK | 34)
> #define EFI_HTTP_ERROR(EFI_ERROR_MASK | 35)
>+#define EFI_ERROR_COUNT   36
> 
> #define EFI_WARN_UNKNOWN_GLYPH1
> #define EFI_WARN_DELETE_FAILURE   2
>diff --git a/include/efi_log.h b/include/efi_log.h
>new file mode 100644
>index 000..1988e5f9df0
>--- /dev/null
>+++ b/include/efi_log.h
>@@ -0,0 +1,169 @@
>+/* SPDX-License-Identifier: GPL-2.0+ */
>+/*
>+ * Logging (to memory) of calls from an EFI app
>+ *
>+ * Copyright 2024 Google LLC
>+ * Written by Simon Glass 
>+ */
>+
>+#ifndef __EFI_LOG_H
>+#define __EFI_LOG_H
>+
>+#include 
>+#include 
>+
>+/**
>+ * enum efil_tag - Types of logging records which can be created
>+ */
>+enum efil_tag {
>+  EFILT_TESTING,
>+
>+  EFILT_COUNT,
>+};
>+
>+/**
>+ * struct efil_rec_hdr - Header for each logging record
>+ *
>+ * @tag: Tag which indicates the type of the record
>+ * @size: Size of the record in bytes
>+ * @ended: true if record has been completed (i.e. the function returned), 
>false
>+ *if it is still pending
>+ * @e_ret: Records the return function from the logged function
>+ */
>+struct efil_rec_hdr {
>+  enum efil_tag tag;
>+  int size;
>+  bool ended;
>+  efi_status_t e_ret;
>+};
>+
>+/**
>+ * struct efil_hdr - Holds the header for the log
>+ *
>+ * @upto: Offset at which to store the next log record
>+ * @size: Total size of the log in bytes
>+ */
>+struct efil_hdr {
>+  int upto;
>+  int size;
>+};
>+
>+enum efil_test_t {
>+  EFI_LOG_TEST0,
>+  EFI_LOG_TEST1,
>+
>+  EFI_LOG_TEST_COUNT,
>+};
>+
>+/**
>+ * struct efil_testing - used for testing the log
>+ */
>+struct efil_testing {
>+  enum efil_test_t enum_val;
>+  efi_uintn_t int_val;
>+  u64 *memory;
>+  void **buffer;
>+  u64 e_memory;
>+  void *e_buffer;
>+};
>+
>+/**
>+ * struct efil_allocate_pages - holds info from efi_allocate_pages() call
>+ *
>+ * @e_memory: Contains the value of *@memory on return from the EFI function
>+ */
>+struct efil_allocate_pages {
>+  enum efi_allocate_type type;
>+  enum efi_memory_type memory_type;
>+  efi_uintn_t pages;
>+  u64 *memory;
>+  u64 e_memory;
>+};
>+
>+/*
>+ * The functions below are in pairs, with a 'start' and 'end' call for each 
>EFI
>+ * function. The 'start' function (efi_logs_...) is called when the function 
>is
>+ * started. It records all the arguments. The 'end' function (efi_loge_...) is
>+ * called when the function is ready to return. It records any output 
>arguments
>+ * as wel

[GIT PULL] u-boot-riscv/master

[Leo Liang]

Hi Tom,

The following changes since commit bfdfc6c12e8ca68fff1a7ed3892c180143a6a0ef:

  Revert "acpi_table: Fix coverity defect in acpi_write_spcr" (2024-10-28 
20:53:34 -0600)

are available in the Git repository at:

  https://source.denx.de/u-boot/custodians/u-boot-riscv.git 

for you to fetch changes up to 239e4705099c7516f3d3cf958f3e540d635a4ed3:

  riscv: dts: mpfs: migrate to OF_UPSTREAM (2024-10-29 19:58:22 +0800)

CI result shows no issue: 
https://source.denx.de/u-boot/custodians/u-boot-riscv/-/pipelines/23080


- board: migrate PolarFire to use OF_UPSTREAM
- dts: align DT with QEMU amd-microblaze-v-virt platform
- riscv: fix resume utility

Anton Blanchard (1):
  riscv: resume needs to be a global

Conor Dooley (3):
  clk: microchip: mpfs: support new syscon based devicetree configuration
  board: mpfs_icicle: imply new clk driver dependencies
  riscv: dts: mpfs: migrate to OF_UPSTREAM

Michal Simek (1):
  riscv: mbv: Align DT with QEMU

 arch/riscv/dts/Makefile|   1 -
 arch/riscv/dts/mpfs-icicle-kit-fabric.dtsi |  71 
 arch/riscv/dts/mpfs-icicle-kit-u-boot.dtsi |  14 -
 arch/riscv/dts/mpfs-icicle-kit.dts | 208 
 arch/riscv/dts/mpfs.dtsi   | 511 -
 arch/riscv/dts/xilinx-mbv32.dts|  30 +-
 arch/riscv/include/asm/global_data.h   |   1 +
 arch/riscv/lib/interrupts.c|  10 +-
 board/microchip/mpfs_icicle/Kconfig|   2 +
 board/xilinx/mbv/Kconfig   |   6 +-
 configs/microchip_mpfs_icicle_defconfig|   4 +-
 configs/xilinx_mbv32_defconfig |  12 +-
 drivers/clk/microchip/Kconfig  |   2 +
 drivers/clk/microchip/mpfs_clk.c   |  63 +++-
 drivers/clk/microchip/mpfs_clk.h   |   5 +-
 drivers/clk/microchip/mpfs_clk_cfg.c   |  16 +-
 drivers/clk/microchip/mpfs_clk_periph.c|  37 +--
 dts/upstream/src/riscv/Makefile|   6 +
 18 files changed, 115 insertions(+), 884 deletions(-)
 delete mode 100644 arch/riscv/dts/mpfs-icicle-kit-fabric.dtsi
 delete mode 100644 arch/riscv/dts/mpfs-icicle-kit-u-boot.dtsi
 delete mode 100644 arch/riscv/dts/mpfs-icicle-kit.dts
 delete mode 100644 arch/riscv/dts/mpfs.dtsi
 create mode 100644 dts/upstream/src/riscv/Makefile

Best regards,
Leo

Re: [PATCH] lmb: Correctly unmap and free memory on errors

[Ilias Apalodimas]

Hi Heinrich

On Mon, 28 Oct 2024 at 08:40, Heinrich Schuchardt  wrote:
>
> On 10/24/24 12:46, Ilias Apalodimas wrote:
> > We never free and unmap the memory on errors and we never unmap it when
> > freeing it. This won't cause any problems on actual hardware but it will
> > on sandbox
> >
> > Fixes: commit 22f2c9ed9f53 ("efi: memory: use the lmb API's for allocating 
> > and freeing memory")
> > Signed-off-by: Ilias Apalodimas 
> > ---
> >   lib/efi_loader/efi_memory.c | 15 ++-
> >   1 file changed, 10 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> > index 9f3f08769977..84be5532a655 100644
> > --- a/lib/efi_loader/efi_memory.c
> > +++ b/lib/efi_loader/efi_memory.c
> > @@ -451,7 +451,7 @@ efi_status_t efi_allocate_pages(enum efi_allocate_type 
> > type,
> >   enum efi_memory_type memory_type,
> >   efi_uintn_t pages, uint64_t *memory)
> >   {
> > - u64 len;
> > + u64 efi_addr, len;
> >   uint flags;
> >   efi_status_t ret;
> >   phys_addr_t addr;
> > @@ -499,14 +499,17 @@ efi_status_t efi_allocate_pages(enum 
> > efi_allocate_type type,
> >   return EFI_INVALID_PARAMETER;
> >   }
> >
> > - addr = (u64)(uintptr_t)map_sysmem(addr, 0);
> > + efi_addr = (u64)(uintptr_t)map_sysmem(addr, 0);
> >   /* Reserve that map in our memory maps */
> > - ret = efi_add_memory_map_pg(addr, pages, memory_type, true);
> > - if (ret != EFI_SUCCESS)
> > + ret = efi_add_memory_map_pg(efi_addr, pages, memory_type, true);
> > + if (ret != EFI_SUCCESS) {
> >   /* Map would overlap, bail out */
> > + lmb_free_flags(addr, (u64)pages << EFI_PAGE_SHIFT, flags);
> > + unmap_sysmem((void *)(uintptr_t)efi_addr);
> >   return  EFI_OUT_OF_RESOURCES;
> > + }
> >
> > - *memory = addr;
> > + *memory = efi_addr;
> >
> >   return EFI_SUCCESS;
> >   }
> > @@ -548,6 +551,8 @@ efi_status_t efi_free_pages(uint64_t memory, 
> > efi_uintn_t pages)
> >   if (ret != EFI_SUCCESS)
> >   return EFI_NOT_FOUND;
> >
> > + unmap_sysmem((void *)(uintptr_t)memory);
> > +
> >   return ret;
> >   }
> >
> > --
> > 2.45.2
> >
>
> AllocatePages() only provides access to EfiConventionalMemory.
>
> We can safely call map_sysmem() with len = 0 and not use unmap_sysmem().

I am not sure I am following you here. We are calling it with len = 0.
But map/unmap_sysmem keep track of internal ref counters for sandbox
and the special tag case. Why shouldn't we call unmap_sysmem()?

Thanks
/Ilias
>
> Best regards
>
> Heinrich

Re: [PATCH] lmb: Correctly unmap memory after notifications

[Ilias Apalodimas]

Hi Heinrich

On Tue, 29 Oct 2024 at 10:00, Ilias Apalodimas
 wrote:
>
> Hi Heinrich
>
> On Mon, 28 Oct 2024 at 08:37, Heinrich Schuchardt  wrote:
> >
> > On 10/24/24 12:46, Ilias Apalodimas wrote:
> > > We never unmap the memory used to update the EFI memory map after
> > > notifications
> > >
> > > Fixes: commit 2f6191526a13 ("lmb: notify of any changes to the LMB memory 
> > > map")
> > > Signed-off-by: Ilias Apalodimas 
> > > ---
> > >   lib/lmb.c | 1 +
> > >   1 file changed, 1 insertion(+)
> > >
> > > diff --git a/lib/lmb.c b/lib/lmb.c
> > > index 890e2cbfdf6b..38c6e1d5ba8d 100644
> > > --- a/lib/lmb.c
> > > +++ b/lib/lmb.c
> > > @@ -65,6 +65,7 @@ static int __maybe_unused 
> > > lmb_map_update_notify(phys_addr_t addr,
> > >   status & ~EFI_ERROR_MASK);
> > >   return -1;
> > >   }
> > > + unmap_sysmem((void *)(uintptr_t)efi_addr);
> >
> > If PCI memory was ever mapped via pci_map_physmem(), I don't think that
> > we want to unmap it here.
>
> Ok, so looking at it again, not calling unmap_sysmem() here won't
> break anything, so I guess we can drop this patch.

Looking at it again, for special sandbox tags, we need to correctly
decrease the refcnt. So why do you think this isnt needed here?

Thanks
/Ilias
>
> >
> > Why do we map the memory in this notification function first hand?
>
> Because the memory was added as such in efi_allocate_pages(). So for
> sandbox we need to find the correct virtual memory that was added.
>
> > Can't
> > we use len = 0 when calling map_sysmem()?
>
> We are using len = 0. But I am not following up on how could this
> help. All you will get is a warning for sanbox if a pci device mapped
> more that 0.
>
> Thanks
> /Ilias
>
> >
> > I guess we have to critically review all map_sysmem() calls.
> >
> > Best regards
> >
> > Heinrich
> >
> > >
> > >   return 0;
> > >   }
> >

Re: [RFC PATCH 2/2] apple: dart: use driver specific instance of LMB

[Sughosh Ganu]

On Tue, 29 Oct 2024 at 14:02, Janne Grunau  wrote:
>
> Hej,
>
> On Tue, Oct 29, 2024 at 12:48:46PM +0530, Sughosh Ganu wrote:
> > The LMB module is typically used for managing the platform's RAM
> > memory. RAM memory gets added to the module's memory map, and
> > subsequently allocated through various LMB API's.
> >
> > The IOMMU driver for the apple platforms also uses the LMB API's for
> > allocating device virtual addresses(VA). These addresses are different
> > from the RAM addresses, and cannot be added to the RAM memory map. Add
> > a separate instance of LMB memory map for the device VA's, which gets
> > managed by the IOMMU driver. Use lmb_push() and lmb_pop() functions to
> > set-up the relevant lmb instance.
>
> thanks, this fixes the initial brokenness when setting up dma mappings
> but I still see SError due to translation fault. I don't have time right
> now to look into it so it could be unrelated to the iommu.

Good to know. Thanks for testing the changes.

>
> > Signed-off-by: Sughosh Ganu 
> > ---
> >  drivers/iommu/apple_dart.c | 67 +-
> >  include/lmb.h  |  2 ++
> >  lib/lmb.c  |  1 -
> >  3 files changed, 68 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/iommu/apple_dart.c b/drivers/iommu/apple_dart.c
> > index 611ac7cd6de..55f60287b0e 100644
> > --- a/drivers/iommu/apple_dart.c
> > +++ b/drivers/iommu/apple_dart.c
> > @@ -3,6 +3,7 @@
> >   * Copyright (C) 2021 Mark Kettenis 
> >   */
> >
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -70,6 +71,8 @@
> >
> >  struct apple_dart_priv {
> >   void *base;
> > + struct lmb apple_dart_lmb;
> > + struct lmb ram_lmb;
> >   u64 *l1, *l2;
> >   int bypass, shift;
> >
> > @@ -87,6 +90,56 @@ struct apple_dart_priv {
> >   void (*flush_tlb)(struct apple_dart_priv *priv);
> >  };
> >
> > +static int apple_dart_lmb_init(struct apple_dart_priv *priv)
> > +{
> > + bool ret;
> > + struct lmb *almb = &priv->apple_dart_lmb;
> > +
> > + ret = alist_init(&almb->free_mem, sizeof(struct lmb_region),
> > +  (uint)LMB_ALIST_INITIAL_SIZE);
> > + if (!ret) {
> > + log_debug("Unable to initialise the list for LMB free 
> > memory\n");
> > + return -ENOMEM;
> > + }
> > +
> > + ret = alist_init(&almb->used_mem, sizeof(struct lmb_region),
> > +  (uint)LMB_ALIST_INITIAL_SIZE);
> > + if (!ret) {
> > + log_debug("Unable to initialise the list for LMB used 
> > memory\n");
> > + return -ENOMEM;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void apple_dart_lmb_uninit(struct apple_dart_priv *priv)
> > +{
> > + struct lmb *almb = &priv->apple_dart_lmb;
> > +
> > + alist_uninit(&almb->free_mem);
> > + alist_uninit(&almb->used_mem);
> > +}
> > +
> > +static void apple_dart_lmb_setup(struct apple_dart_priv *priv)
> > +{
> > + struct lmb *almb = &priv->apple_dart_lmb;
> > + struct lmb *rlmb = &priv->ram_lmb;
> > +
> > + lmb_push(rlmb);
> > +
> > + lmb_pop(almb);
>
> This doesn't look look like a good solution to me. We're conflating two
> different concepts into the global lmb. This looks error prone to me. I
> was looking into adding iomb_* entry points into the lmb points which
> take a pointer.

Yes, even I was trying to avoid having another instance of LMB as much
as possible, but given that this is breaking the platform, I thought
this would be the quickest to fix the break. Personally, I would
prefer LMB to have a consistent window of RAM addresses for all
platforms, i.e. ram_base to ram_top. Also, apart from this peculiar
scenario, I am not sure if there would be other uses of having to
allocate anything other than RAM addresses by LMB.

-sughosh


>
> Janne

[PATCH 3/4] board/BuR/common: split br_resetc_bmode function

[Bernhard Messerklinger]

Split br_resetc_bmode function to add support for reading of reset
reason in board code with br_resetc_bmode_get.

Signed-off-by: Bernhard Messerklinger 
---

 board/BuR/common/br_resetc.c | 129 ---
 board/BuR/common/br_resetc.h |   1 +
 2 files changed, 74 insertions(+), 56 deletions(-)

diff --git a/board/BuR/common/br_resetc.c b/board/BuR/common/br_resetc.c
index 827ea6ba35..28fa87033f 100644
--- a/board/BuR/common/br_resetc.c
+++ b/board/BuR/common/br_resetc.c
@@ -113,9 +113,73 @@ int br_resetc_bmode(void)
 {
int rc = 0;
u16 regw;
+   unsigned int bmode = 0;
+
+   if (!resetc.i2cdev)
+   rc = resetc_init();
+
+   if (rc != 0)
+   return rc;
+
+   board_boot_led(1);
+
+   rc = br_resetc_bmode_get(&bmode);
+   if (rc != 0)
+   return rc;
+
+   LCD_SETCURSOR(1, 8);
+
+   switch (bmode) {
+   case BMODE_PME:
+   LCD_PUTS("entering PME-Mode (netscript). ");
+   regw = 0x0C0C;
+   break;
+   case BMODE_DEFAULTAR:
+   LCD_PUTS("entering BOOT-mode.");
+   regw = 0x;
+   break;
+   case BMODE_DIAG:
+   LCD_PUTS("entering DIAGNOSE-mode.");
+   regw = 0x0F0F;
+   break;
+   case BMODE_SERVICE:
+   LCD_PUTS("entering SERVICE mode. ");
+   regw = 0xB4B4;
+   break;
+   case BMODE_RUN:
+   LCD_PUTS("loading OS...  ");
+   regw = 0x0404;
+   break;
+   }
+
+   board_boot_led(0);
+
+   if (resetc.is_psoc)
+   rc = dm_i2c_write(resetc.i2cdev, RSTCTRL_SCRATCHREG0,
+ (u8 *)®w, 2);
+   else
+   rc = dm_i2c_write(resetc.i2cdev, RSTCTRL_SCRATCHREG0,
+ (u8 *)®w, 1);
+
+   if (rc != 0)
+   printf("WARN: cannot write into resetcontroller!\n");
+
+   if (resetc.is_psoc)
+   printf("Reset: PSOC controller\n");
+   else
+   printf("Reset: STM32 controller\n");
+
+   printf("Mode:  %s\n", bootmodeascii[regw & 0x0F]);
+   env_set_ulong("b_mode", regw & 0x0F);
+
+   return rc;
+}
+
+int br_resetc_bmode_get(unsigned int *bmode)
+{
+   int rc = 0;
u8 regb, scr;
int cnt;
-   unsigned int bmode = 0;
 
if (!resetc.i2cdev)
rc = resetc_init();
@@ -135,13 +199,11 @@ int br_resetc_bmode(void)
return -1;
}
 
-   board_boot_led(1);
-
/* special bootmode from resetcontroller */
if (regb & 0x4) {
-   bmode = BMODE_DIAG;
+   *bmode = BMODE_DIAG;
} else if (regb & 0x8) {
-   bmode = BMODE_DEFAULTAR;
+   *bmode = BMODE_DEFAULTAR;
} else if (board_boot_key() != 0) {
cnt = 4;
do {
@@ -168,68 +230,23 @@ int br_resetc_bmode(void)
 
switch (cnt) {
case 0:
-   bmode = BMODE_PME;
+   *bmode = BMODE_PME;
break;
case 1:
-   bmode = BMODE_DEFAULTAR;
+   *bmode = BMODE_DEFAULTAR;
break;
case 2:
-   bmode = BMODE_DIAG;
+   *bmode = BMODE_DIAG;
break;
case 3:
-   bmode = BMODE_SERVICE;
+   *bmode = BMODE_SERVICE;
break;
}
} else if ((regb & 0x1) || scr == 0xCC) {
-   bmode = BMODE_PME;
+   *bmode = BMODE_PME;
} else {
-   bmode = BMODE_RUN;
+   *bmode = BMODE_RUN;
}
 
-   LCD_SETCURSOR(1, 8);
-
-   switch (bmode) {
-   case BMODE_PME:
-   LCD_PUTS("entering PME-Mode (netscript). ");
-   regw = 0x0C0C;
-   break;
-   case BMODE_DEFAULTAR:
-   LCD_PUTS("entering BOOT-mode.");
-   regw = 0x;
-   break;
-   case BMODE_DIAG:
-   LCD_PUTS("entering DIAGNOSE-mode.");
-   regw = 0x0F0F;
-   break;
-   case BMODE_SERVICE:
-   LCD_PUTS("entering SERVICE mode. ");
-   regw = 0xB4B4;
-   break;
-   case BMODE_RUN:
-   LCD_PUTS("loading OS...  ");
-   regw = 0x0404;
-   break;
-   }
-
-   board_boot_led(0);
-
-   if (resetc.is_psoc)
-   rc = dm_i2c_write(resetc.i2cdev, RSTCTRL_SCRATCHREG0,
- (u8 *)®w, 2);
-   else
-   rc = dm_i2c_write(resetc.i2cdev, RSTCT

[PATCH 1/4] board/BuR/common: use strlcpy instead of strncpy

[Bernhard Messerklinger]

Now strlcpy is used to copy the defip string to the corresponding
environment variable. This preserves memory for the NULL termination.

Signed-off-by: Bernhard Messerklinger 
---

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

diff --git a/board/BuR/common/common.c b/board/BuR/common/common.c
index 8aff821cfe..153761dd81 100644
--- a/board/BuR/common/common.c
+++ b/board/BuR/common/common.c
@@ -68,7 +68,7 @@ int brdefaultip_setup(int bus, int chip)
 "if test -r ${ipaddr}; then; else setenv ipaddr 
192.168.60.%d; setenv serverip 192.168.60.254; setenv gatewayip 192.168.60.254; 
setenv netmask 255.255.255.0; fi;",
 u8buf);
else
-   strncpy(defip,
+   strlcpy(defip,
"if test -r ${ipaddr}; then; else setenv ipaddr 
192.168.60.1; setenv serverip 192.168.60.254; setenv gatewayip 192.168.60.254; 
setenv netmask 255.255.255.0; fi;",
sizeof(defip));
 
-- 
2.46.1

[PATCH 0/4] Update board/BuR/common to keep current mainline boards compatible

[Bernhard Messerklinger]

Bernhard Messerklinger (4):
  board/BuR/common: use strlcpy instead of strncpy
  board/BuR/common: add parameter for reset controller I2C bus selection
  board/BuR/common: split br_resetc_bmode function
  board/BuR/zynq: initial commit

 arch/arm/dts/Makefile |   8 +-
 arch/arm/dts/zynq-brcp1-u-boot.dtsi   | 110 +++
 arch/arm/dts/zynq-brcp1.dtsi  | 146 +
 arch/arm/dts/zynq-brcp150-u-boot.dtsi | 129 
 arch/arm/dts/zynq-brcp150.dts | 187 
 arch/arm/dts/zynq-brcp170-u-boot.dtsi | 113 +++
 arch/arm/dts/zynq-brcp170.dts | 153 ++
 arch/arm/dts/zynq-brcp1_1r.dts|  29 ++
 arch/arm/dts/zynq-brcp1_1r_switch.dts |  30 ++
 arch/arm/dts/zynq-brcp1_2r.dts|  22 ++
 arch/arm/dts/zynq-brsmarc2-u-boot.dtsi| 110 +++
 arch/arm/dts/zynq-brsmarc2.dts| 172 +++
 arch/arm/mach-zynq/Kconfig|   1 +
 board/BuR/common/br_resetc.c  | 138 +
 board/BuR/common/br_resetc.h  |   1 +
 board/BuR/common/common.c |   2 +-
 board/BuR/zynq/Kconfig|  13 +
 board/BuR/zynq/MAINTAINERS|  13 +
 board/BuR/zynq/Makefile   |  15 +
 board/BuR/zynq/brcp150/board.c|   6 +
 board/BuR/zynq/brcp150/ps7_init_gpl.c | 284 ++
 board/BuR/zynq/brcp170/board.c|   6 +
 board/BuR/zynq/brcp170/ps7_init_gpl.c | 280 +
 board/BuR/zynq/brcp1_1r/board.c   |   6 +
 board/BuR/zynq/brcp1_1r/ps7_init_gpl.c| 280 +
 board/BuR/zynq/brcp1_1r_switch/board.c|   6 +
 board/BuR/zynq/brcp1_1r_switch/ps7_init_gpl.c | 276 +
 board/BuR/zynq/brcp1_2r/board.c   |   6 +
 board/BuR/zynq/brcp1_2r/ps7_init_gpl.c| 283 +
 board/BuR/zynq/brsmarc2/board.c   |  32 ++
 board/BuR/zynq/brsmarc2/ps7_init_gpl.c| 282 +
 board/BuR/zynq/common/board.c | 239 +++
 board/BuR/zynq/env/brcp1.env  | 106 +++
 board/BuR/zynq/env/brcp150.env| 116 +++
 configs/brcp150_defconfig | 123 
 configs/brcp170_defconfig | 122 
 configs/brcp1_1r_defconfig| 122 
 configs/brcp1_1r_switch_defconfig | 123 
 configs/brcp1_2r_defconfig| 122 
 configs/brsmarc2_defconfig| 122 
 include/configs/brzynq.h  |  21 ++
 41 files changed, 4295 insertions(+), 60 deletions(-)
 create mode 100644 arch/arm/dts/zynq-brcp1-u-boot.dtsi
 create mode 100644 arch/arm/dts/zynq-brcp1.dtsi
 create mode 100644 arch/arm/dts/zynq-brcp150-u-boot.dtsi
 create mode 100644 arch/arm/dts/zynq-brcp150.dts
 create mode 100644 arch/arm/dts/zynq-brcp170-u-boot.dtsi
 create mode 100644 arch/arm/dts/zynq-brcp170.dts
 create mode 100644 arch/arm/dts/zynq-brcp1_1r.dts
 create mode 100644 arch/arm/dts/zynq-brcp1_1r_switch.dts
 create mode 100644 arch/arm/dts/zynq-brcp1_2r.dts
 create mode 100644 arch/arm/dts/zynq-brsmarc2-u-boot.dtsi
 create mode 100644 arch/arm/dts/zynq-brsmarc2.dts
 create mode 100644 board/BuR/zynq/Kconfig
 create mode 100644 board/BuR/zynq/MAINTAINERS
 create mode 100644 board/BuR/zynq/Makefile
 create mode 100644 board/BuR/zynq/brcp150/board.c
 create mode 100644 board/BuR/zynq/brcp150/ps7_init_gpl.c
 create mode 100644 board/BuR/zynq/brcp170/board.c
 create mode 100644 board/BuR/zynq/brcp170/ps7_init_gpl.c
 create mode 100644 board/BuR/zynq/brcp1_1r/board.c
 create mode 100644 board/BuR/zynq/brcp1_1r/ps7_init_gpl.c
 create mode 100644 board/BuR/zynq/brcp1_1r_switch/board.c
 create mode 100644 board/BuR/zynq/brcp1_1r_switch/ps7_init_gpl.c
 create mode 100644 board/BuR/zynq/brcp1_2r/board.c
 create mode 100644 board/BuR/zynq/brcp1_2r/ps7_init_gpl.c
 create mode 100644 board/BuR/zynq/brsmarc2/board.c
 create mode 100644 board/BuR/zynq/brsmarc2/ps7_init_gpl.c
 create mode 100644 board/BuR/zynq/common/board.c
 create mode 100644 board/BuR/zynq/env/brcp1.env
 create mode 100644 board/BuR/zynq/env/brcp150.env
 create mode 100644 configs/brcp150_defconfig
 create mode 100644 configs/brcp170_defconfig
 create mode 100644 configs/brcp1_1r_defconfig
 create mode 100644 configs/brcp1_1r_switch_defconfig
 create mode 100644 configs/brcp1_2r_defconfig
 create mode 100644 configs/brsmarc2_defconfig
 create mode 100644 include/configs/brzynq.h

-- 
2.46.1

Re: [GIT PULL] u-boot-riscv/next

[Michal Simek]

út 29. 10. 2024 v 10:35 odesílatel Leo Liang  napsal:
>
> Hi Tom,
>
> On Mon, Oct 28, 2024 at 09:20:10AM -0600, Tom Rini wrote:
> > [EXTERNAL MAIL]
> > Date: Mon, 28 Oct 2024 09:20:10 -0600
> > From: Tom Rini 
> > To: Leo Liang 
> > Cc: u-boot@lists.denx.de, r...@andestech.com
> > Subject: Re: [GIT PULL] u-boot-riscv/next
> >
> > On Mon, Oct 28, 2024 at 08:25:55PM +0800, Leo Liang wrote:
> > > Hi Tom,
> > >
> > > The following changes since commit 
> > > 28dc47038edc4e93f32d75a357131bcf01a18d85:
> > >
> > >   Merge branch 'u-boot-nand-20241005' of 
> > > https://gitlab.denx.de/u-boot/custodians/u-boot-nand-flash into next 
> > > (2024-10-05 11:19:24 -0600)
> > >
> > > are available in the Git repository at:
> > >
> > >   https://source.denx.de/u-boot/custodians/u-boot-riscv.git next
> > >
> > > for you to fetch changes up to f07daa5967f65771f90221ee9bfad9814e549647:
> > >
> > >   riscv: mbv: Align DT with QEMU (2024-10-28 17:49:25 +0800)
> > >
> > > CI result shows no issue: 
> > > https://source.denx.de/u-boot/custodians/u-boot-riscv/-/pipelines/23049
> > >
> > > 
> > > Michal Simek (1):
> > >   riscv: mbv: Align DT with QEMU
> > >
> > >  arch/riscv/dts/xilinx-mbv32.dts | 30 ++
> > >  board/xilinx/mbv/Kconfig|  6 +++---
> > >  configs/xilinx_mbv32_defconfig  | 12 ++--
> > >  3 files changed, 19 insertions(+), 29 deletions(-)
> >
> > Why is this for -next and not master? We're not so far in to the cycle
> > that next is open, and this seems clear enough to pull in today. Thanks.
> >
> > --
> > Tom
>
>
> Michal said that this patch might have to wait for its counterpart in QEMU be 
> merged.
> (https://lore.kernel.org/all/20241017072507.4033413-1-sai.pavan.bo...@amd.com/)
> But like you said, this seems clear enough for master. I will create another 
> PR for this patch.

Likely the address map won't change.

Thanks,
Michal

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal SoCs

Re: [PATCH] riscv: resume needs to be a global

[Leo Liang]

On Thu, Aug 08, 2024 at 02:14:17AM +, Anton Blanchard wrote:
> If we take an exception before u-boot is relocated, there's a good
> chance we will end up in an endless loop of exceptions because resume is
> invalid until after relocation.
> 
> Signed-off-by: Anton Blanchard 
> ---
>  arch/riscv/include/asm/global_data.h |  1 +
>  arch/riscv/lib/interrupts.c  | 10 --
>  2 files changed, 5 insertions(+), 6 deletions(-)

Reviewed-by: Leo Yu-Chi Liang 

[PATCH] test/py: spi: prevent overwriting relocation memory

[Padmarao Begari]

Update spi negative test case to prevent SF command
from overwriting relocation memory area.

Signed-off-by: Padmarao Begari 
---
 test/py/tests/test_spi.py | 12 
 1 file changed, 12 insertions(+)

diff --git a/test/py/tests/test_spi.py b/test/py/tests/test_spi.py
index 3160d58540..caca930327 100644
--- a/test/py/tests/test_spi.py
+++ b/test/py/tests/test_spi.py
@@ -693,4 +693,16 @@ def test_spi_negative(u_boot_console):
 u_boot_console, 'read', offset, size, addr, 1, error_msg, 
EXPECTED_READ
 )
 
+# Read to relocation address
+output = u_boot_console.run_command('bdinfo')
+m = re.search('relocaddr\s*= (.+)', output)
+res_area = int(m.group(1), 16)
+
+start = 0
+size = 0x2000
+error_msg = 'ERROR: trying to overwrite reserved memory'
+flash_ops(
+u_boot_console, 'read', start, size, res_area, 1, error_msg, 
EXPECTED_READ
+)
+
 i = i + 1
-- 
2.25.1

Re: [PATCH v2 2/3] board: mpfs_icicle: imply new clk driver dependencies

[Leo Liang]

On Wed, Oct 23, 2024 at 11:17:53AM +0100, Conor Dooley wrote:
> From: Conor Dooley 
> 
> The clock driver for PolarFire SoC now requires syscon and regmap
> features, so imply them to preserve implication of the clock driver.
> 
> Signed-off-by: Conor Dooley 
> ---
>  board/microchip/mpfs_icicle/Kconfig | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Leo Yu-Chi Liang 

Re: [PATCH v2 1/3] clk: microchip: mpfs: support new syscon based devicetree configuration

[Leo Liang]

On Wed, Oct 23, 2024 at 11:17:52AM +0100, Conor Dooley wrote:
> From: Conor Dooley 
> 
> Why get a devicetree description wrong once when you can get it wrong
> twice? The original mistake, which the driver supports was failing to
> describe the main PLL that the "cfg" and "periph" clocks parented by.
> The second mistake was describing the "cfg" and "periph" clocks a
> reg region within the clock controller, rather as two registers within
> a syscon region that also contains pinctrl, interrupt muxing controls
> and other functions.
> 
> Make up for lost time and describe these regions as they should have
> been originally, preserving support for the existing two configurations
> for the sake of existing systems with firmware-provided devicetrees.
> 
> Signed-off-by: Conor Dooley 
> ---
>  drivers/clk/microchip/Kconfig   |  2 +
>  drivers/clk/microchip/mpfs_clk.c| 63 -
>  drivers/clk/microchip/mpfs_clk.h|  5 +-
>  drivers/clk/microchip/mpfs_clk_cfg.c| 16 +++
>  drivers/clk/microchip/mpfs_clk_periph.c | 37 +++
>  5 files changed, 81 insertions(+), 42 deletions(-)

Reviewed-by: Leo Yu-Chi Liang 

Re: [PATCH] lmb: Correctly unmap memory after notifications

[Ilias Apalodimas]

Hi Heinrich

On Mon, 28 Oct 2024 at 08:37, Heinrich Schuchardt  wrote:
>
> On 10/24/24 12:46, Ilias Apalodimas wrote:
> > We never unmap the memory used to update the EFI memory map after
> > notifications
> >
> > Fixes: commit 2f6191526a13 ("lmb: notify of any changes to the LMB memory 
> > map")
> > Signed-off-by: Ilias Apalodimas 
> > ---
> >   lib/lmb.c | 1 +
> >   1 file changed, 1 insertion(+)
> >
> > diff --git a/lib/lmb.c b/lib/lmb.c
> > index 890e2cbfdf6b..38c6e1d5ba8d 100644
> > --- a/lib/lmb.c
> > +++ b/lib/lmb.c
> > @@ -65,6 +65,7 @@ static int __maybe_unused 
> > lmb_map_update_notify(phys_addr_t addr,
> >   status & ~EFI_ERROR_MASK);
> >   return -1;
> >   }
> > + unmap_sysmem((void *)(uintptr_t)efi_addr);
>
> If PCI memory was ever mapped via pci_map_physmem(), I don't think that
> we want to unmap it here.

Ok, so looking at it again, not calling unmap_sysmem() here won't
break anything, so I guess we can drop this patch.

>
> Why do we map the memory in this notification function first hand?

Because the memory was added as such in efi_allocate_pages(). So for
sandbox we need to find the correct virtual memory that was added.

> Can't
> we use len = 0 when calling map_sysmem()?

We are using len = 0. But I am not following up on how could this
help. All you will get is a warning for sanbox if a pci device mapped
more that 0.

Thanks
/Ilias

>
> I guess we have to critically review all map_sysmem() calls.
>
> Best regards
>
> Heinrich
>
> >
> >   return 0;
> >   }
>

Re: [PATCH 2/2] ARM: renesas: Drop old unused power DT headers

[Adam Ford]

On Sun, Oct 27, 2024 at 5:58 AM Paul Barker
 wrote:
>
> On 27/10/2024 02:04, Marek Vasut wrote:
> > Renesas R-Car systems use mainline Linux DTs for U-Boot via OF_UPSTREAM,
> > which also includes headers from dts/upstream/include/dt-bindings/power .
> > Remove unused legacy DT header files from include/dt-bindings/power .
> >
> > Signed-off-by: Marek Vasut 

LGTM, Thanks for doing this.

Reviewed-by:  Adam Ford 
> > ---
> > Cc: "Cogent Embedded, Inc." 
> > Cc: Adam Ford 
> > Cc: Biju Das 
> > Cc: Hai Pham 
> > Cc: Heinrich Schuchardt 
> > Cc: Lad Prabhakar 
> > Cc: Masakazu Mochizuki 
> > Cc: Nobuhiro Iwamatsu 
> > Cc: Paul Barker 
> > Cc: Sumit Garg 
> > Cc: Tom Rini 
> > Cc: u-boot@lists.denx.de
>
> Reviewed-by: Paul Barker 
>
> Thanks!
>
> --
> Paul Barker

Re: [PATCH 00/15] efi_loader: Add support for logging to a buffer

[Simon Glass]

Hi Ilias,

On Tue, 29 Oct 2024 at 10:58, Ilias Apalodimas
 wrote:
>
> Hi Simon,
>
> On Mon, 28 Oct 2024 at 14:48, Simon Glass  wrote:
> >
> > It is a bit of a pain to log EFI boot-services calls at present. The
> > output goes to the console so cannot easily be inspected later. Also it
> > would be useful to be able to store the log and review it later, perhaps
> > after something has gone wrong.
> >
> > This series makes a start on implementing a log-to-buffer feature. It
> > provides a simple 'efidebug log' command to inspect the buffer. For now,
> > only memory allocations are logged.
>
> Why is this problem specific to EFI and no U-Boot in general? Do we
> have a similar machinery for malloc()?

Mostly because an app can make EFI calls and we want to know what they
are, e.g. to debug them and figure out what might be wrong when
something doesn't boot.

Regards,
Simon


>
> Thanks
> /Ilias
> >
> > This feature makes it possible to add tests to check which EFI calls are
> > made by U-Boot itself. It may also make it easier to figure out what is
> > needed for booting Windows.
> >
> > Some patches to help with debugging sandbox memory-mapping are included,
> > but for now EFI's use of mapping is not adjusted.
> >
> >
> > Simon Glass (15):
> >   log: Add a new category for tests
> >   test: Allow saving and restoring the bloblist
> >   bloblist: test: Mark tests with UTF_BLOBLIST
> >   sandbox: Convert sb command to use new macro
> >   doc: sandbox: Add docs for the sb command
> >   sandbox: Add a way to show the sandbox memory-mapping
> >   sandbox: Fix comment for nomap_sysmem() function
> >   lmb: Drop extra 16KB of stack space
> >   efi_loader: Fix free in ..._media_device_boot_option()
> >   efi_loader: Add support for logging EFI calls
> >   efi_loader: Create the log on startup
> >   efi_loader: Add a command to show the EFI log
> >   test: efi_loader: Add a simple test for the EFI log
> >   efi_loader: Use the log with memory-related functions
> >   efi_loader: Add documentation for the EFI log
> >
> >  MAINTAINERS|   7 +
> >  arch/sandbox/cpu/cpu.c |  13 +
> >  arch/sandbox/include/asm/cpu.h |   3 +
> >  arch/sandbox/include/asm/io.h  |   2 +-
> >  cmd/efidebug.c |  53 +++-
> >  cmd/sb.c   |  42 ++--
> >  common/log.c   |   1 +
> >  configs/sandbox_defconfig  |   3 +
> >  doc/develop/uefi/uefi.rst  |  22 ++
> >  doc/usage/cmd/efidebug.rst | 109 
> >  doc/usage/cmd/sb.rst   |  79 ++
> >  doc/usage/index.rst|   2 +
> >  include/bloblist.h |   1 +
> >  include/efi.h  |   1 +
> >  include/efi_log.h  | 316 +++
> >  include/log.h  |   2 +
> >  include/test/test.h|   3 +
> >  lib/efi_loader/Kconfig |  19 ++
> >  lib/efi_loader/Makefile|   1 +
> >  lib/efi_loader/efi_bootmgr.c   |   3 +-
> >  lib/efi_loader/efi_log.c   | 444 +
> >  lib/efi_loader/efi_memory.c| 119 ++---
> >  lib/efi_loader/efi_setup.c |   7 +
> >  lib/lmb.c  |   2 -
> >  test/common/bloblist.c |  28 +--
> >  test/lib/Makefile  |   1 +
> >  test/lib/efi_log.c |  93 +++
> >  test/test-main.c   |  13 +
> >  28 files changed, 1305 insertions(+), 84 deletions(-)
> >  create mode 100644 doc/usage/cmd/efidebug.rst
> >  create mode 100644 doc/usage/cmd/sb.rst
> >  create mode 100644 include/efi_log.h
> >  create mode 100644 lib/efi_loader/efi_log.c
> >  create mode 100644 test/lib/efi_log.c
> >
> > --
> > 2.43.0
> >

Re: [PATCH 00/15] efi_loader: Add support for logging to a buffer

[Ilias Apalodimas]

On Tue, 29 Oct 2024 at 17:45, Simon Glass  wrote:
>
> Hi Ilias,
>
> On Tue, 29 Oct 2024 at 10:58, Ilias Apalodimas
>  wrote:
> >
> > Hi Simon,
> >
> > On Mon, 28 Oct 2024 at 14:48, Simon Glass  wrote:
> > >
> > > It is a bit of a pain to log EFI boot-services calls at present. The
> > > output goes to the console so cannot easily be inspected later. Also it
> > > would be useful to be able to store the log and review it later, perhaps
> > > after something has gone wrong.
> > >
> > > This series makes a start on implementing a log-to-buffer feature. It
> > > provides a simple 'efidebug log' command to inspect the buffer. For now,
> > > only memory allocations are logged.
> >
> > Why is this problem specific to EFI and no U-Boot in general? Do we
> > have a similar machinery for malloc()?
>
> Mostly because an app can make EFI calls and we want to know what they
> are, e.g. to debug them and figure out what might be wrong when
> something doesn't boot.

EFI_PRINT() has been proven pretty useful for this. I don't personally
see the point of adding ~1300 lines of code to replace a print.
What would make more sense is teach EFI_PRINT to log errors in a buffer.


Thanks
/Ilias

/Ilias
>
> Regards,
> Simon
>
>
> >
> > Thanks
> > /Ilias
> > >
> > > This feature makes it possible to add tests to check which EFI calls are
> > > made by U-Boot itself. It may also make it easier to figure out what is
> > > needed for booting Windows.
> > >
> > > Some patches to help with debugging sandbox memory-mapping are included,
> > > but for now EFI's use of mapping is not adjusted.
> > >
> > >
> > > Simon Glass (15):
> > >   log: Add a new category for tests
> > >   test: Allow saving and restoring the bloblist
> > >   bloblist: test: Mark tests with UTF_BLOBLIST
> > >   sandbox: Convert sb command to use new macro
> > >   doc: sandbox: Add docs for the sb command
> > >   sandbox: Add a way to show the sandbox memory-mapping
> > >   sandbox: Fix comment for nomap_sysmem() function
> > >   lmb: Drop extra 16KB of stack space
> > >   efi_loader: Fix free in ..._media_device_boot_option()
> > >   efi_loader: Add support for logging EFI calls
> > >   efi_loader: Create the log on startup
> > >   efi_loader: Add a command to show the EFI log
> > >   test: efi_loader: Add a simple test for the EFI log
> > >   efi_loader: Use the log with memory-related functions
> > >   efi_loader: Add documentation for the EFI log
> > >
> > >  MAINTAINERS|   7 +
> > >  arch/sandbox/cpu/cpu.c |  13 +
> > >  arch/sandbox/include/asm/cpu.h |   3 +
> > >  arch/sandbox/include/asm/io.h  |   2 +-
> > >  cmd/efidebug.c |  53 +++-
> > >  cmd/sb.c   |  42 ++--
> > >  common/log.c   |   1 +
> > >  configs/sandbox_defconfig  |   3 +
> > >  doc/develop/uefi/uefi.rst  |  22 ++
> > >  doc/usage/cmd/efidebug.rst | 109 
> > >  doc/usage/cmd/sb.rst   |  79 ++
> > >  doc/usage/index.rst|   2 +
> > >  include/bloblist.h |   1 +
> > >  include/efi.h  |   1 +
> > >  include/efi_log.h  | 316 +++
> > >  include/log.h  |   2 +
> > >  include/test/test.h|   3 +
> > >  lib/efi_loader/Kconfig |  19 ++
> > >  lib/efi_loader/Makefile|   1 +
> > >  lib/efi_loader/efi_bootmgr.c   |   3 +-
> > >  lib/efi_loader/efi_log.c   | 444 +
> > >  lib/efi_loader/efi_memory.c| 119 ++---
> > >  lib/efi_loader/efi_setup.c |   7 +
> > >  lib/lmb.c  |   2 -
> > >  test/common/bloblist.c |  28 +--
> > >  test/lib/Makefile  |   1 +
> > >  test/lib/efi_log.c |  93 +++
> > >  test/test-main.c   |  13 +
> > >  28 files changed, 1305 insertions(+), 84 deletions(-)
> > >  create mode 100644 doc/usage/cmd/efidebug.rst
> > >  create mode 100644 doc/usage/cmd/sb.rst
> > >  create mode 100644 include/efi_log.h
> > >  create mode 100644 lib/efi_loader/efi_log.c
> > >  create mode 100644 test/lib/efi_log.c
> > >
> > > --
> > > 2.43.0
> > >

Re: [PATCH v7 01/11] test: Allow signaling that U-Boot is ready

[Tom Rini]

On Wed, Oct 09, 2024 at 07:51:35PM -0600, Simon Glass wrote:

> When Labgrid is used, it can get U-Boot ready for running tests. It
> prints a message when it has done so.
> 
> Add logic to detect this message and accept it.
> 
> Note that this does not change pytest, which still (also) looks for the
> U-Boot banner. This change merely makes it possible for pytest to
> believe Labgrid when it says that the board is ready for use.
> 
> In several cases, the board starts up and Labgrid receives some initial
> output, then pytest starts and misses some of that output, because it
> came in while Labgrid had the console open. Then pytest fails because
> it doesn't see the expected banners.
> 
> With this change, Labgrid handles getting U-Boot to a prompt, in a
> fully reliable manner. Then pytest starts up and can simply start
> running its tests.
> 
> But, again, this does not prevent pytest from handling a banner if one
> is provided (e.g. if not using the Labgrid integration).
> 
> Signed-off-by: Simon Glass 

I _may_ see this myself sometimes, but I'm not sure. But I'm not sure
where the bug would be between us and labgrid. Looking at
test/py/u_boot_console_exec_attach.py we grab the console and then reset
the target. If the spawn hasn't completed, especially before the power
cycle has completed (which in labgrid defaults to 2 seconds) we perhaps
need our own delay. But as I'll point out further on, NOT counting the
banners means we aren't able to verify other functionality. Even if they
are tests that I think you don't like because they verify the
functionality of resetting the device.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v7 10/11] test: Support testing with two board-builds

[Tom Rini]

On Wed, Oct 09, 2024 at 07:51:44PM -0600, Simon Glass wrote:

> The Beagleplay board uses two entirely separate builds to produce an
> image, rather than using an SPL build for this purpose.

JFTR, this is an incorrect explanation of what's going on. The TI K3
families have both a Cortex-R core and a Cortex-A core and the R core
needs to come up before the A core. But in both cases we have U-Boot SPL
and then U-Boot proper being used.

This is aside from having come back to re-review the series and still
feel like this is taking the wrong approach to the problem. We wouldn't
add a --optee-build-directory for going off and building OP-TEE and so
forth. It's just the case that we need assorted binaries for the system
and some of them come from building U-Boot for another platform.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v7 05/11] test: Introduce lab mode

[Tom Rini]

On Wed, Oct 09, 2024 at 07:51:39PM -0600, Simon Glass wrote:

> There is quite a bit of code in pytest to try to start up U-Boot on a
> board, with timeouts, expects, etc.
> 
> This is tedious to maintain and is peripheral to the test system's
> purpose. It seems better to put this logic in the lab itself, where is
> can provide such support.
> 
> With Labgrid we can use the UbootStrategy class to get the board into a
> useful state, however it needs to do it. Then it can report to pytest
> by writing a suitable string along with the U-Boot version it detected.
> 
> Add support for detecting 'lab mode' and simply assume that all is well
> in that case. Collect the version string when Labgrid says it is ready.
> 
> Signed-off-by: Simon Glass 

First, I worry that this kind of change will make verifying boards can
reset harder, not easier. But second, taking the "tedious" part and
putting in another project's config file instead makes it no less
"tedious". If I follow the labgrid code right, it's got the same kind of
"check for a string" that we have, except it's not caring about the
correct number / order of banner prints. Which I find worrisome and a
problem because of the times I've caught "oops, didn't actually test the
right stuff" because that was wrong.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v7 03/11] test: Allow connecting to a running board

[Tom Rini]

On Wed, Oct 09, 2024 at 07:51:37PM -0600, Simon Glass wrote:

> Sometimes we know that the board is already running the right software,
> so provide an option to allow running of tests directly, without first
> resetting the board.
> 
> This saves time when re-running a test where only the Python code is
> changing.
> 
> Signed-off-by: Simon Glass 

I'm not NAK'ing this, but this is not a open to errors kind of change
that I like. It also feels like an unrelated "I find this change useful"
kind of change.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v7 02/11] test: Release board after tests complete

[Tom Rini]

On Wed, Oct 09, 2024 at 07:51:36PM -0600, Simon Glass wrote:

> When a board is finished with, the lab may want to power it off, or
> perform some other function. Add a new script which is called when tests
> are complete.
> 
> Signed-off-by: Simon Glass 

Reviewed-by: Tom Rini 

But please note I don't see an up to date version of the
u-boot-test-hooks side of this which would include the release wrapper,
and presumably files.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v7 09/11] test: Add a section for closing the connection

[Tom Rini]

On Wed, Oct 09, 2024 at 07:51:43PM -0600, Simon Glass wrote:

> This can take a while and involve multiple steps (e.g. turning the board
> back off). Add a section for it and show the output.
> 
> Signed-off-by: Simon Glass 

Logging can be useful, so this is fine.

Reviewed-by: Tom Rini 

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v7 06/11] test: Improve handling of sending commands

[Tom Rini]

On Wed, Oct 09, 2024 at 07:51:40PM -0600, Simon Glass wrote:

> We expect commands to be echoed and this should happen quite quickly,
> since U-Boot is sitting at the prompt waiting for a command.
> 
> Reduce the timeout for this situation. Try to produce a more useful
> error message when something goes wrong. Also handle the case where the
> connection has gone away since the last command was issued.
> 
> Signed-off-by: Simon Glass 
> ---
> 
> (no changes since v1)
> 
>  test/py/u_boot_console_base.py | 35 --
>  1 file changed, 21 insertions(+), 14 deletions(-)

This looks good and I wish you had refactored the series to include this
(as well as patch #7) in the generic test/py improvements series.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v7 08/11] test: Try to shut down the lab console gracefully

[Tom Rini]

On Wed, Oct 09, 2024 at 07:51:42PM -0600, Simon Glass wrote:

> Send the Labgrid quit characters to ask it to exit gracefully. This
> typically allows it to power off the board being used. Only do this when
> labgrid is being used (detected with an env var).
> 
> If that doesn't work, try the less graceful approach.
> 
> Signed-off-by: Simon Glass 

I forget why we need this, and not just have the release hook power off
the board, as part of releasing it.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v9 02/11] efi_loader: Add a test app

[Tom Rini]

On Tue, Oct 29, 2024 at 08:22:10PM +0100, Simon Glass wrote:

> Add a simple app to use for testing. This is intended to do whatever it
> needs to for testing purposes. For now it just prints a message and
> exits boot services.
> 
> There was a considerable amount of discussion about whether it is OK to
> call exit-boot-services and then return to U-Boot. This is not normally
> done in a real application, since exit-boot-services is used to
> completely disconnect from U-Boot. However, since this is a test, we
> need to check the results of running the app, so returning is necessary.
> It works correctly and it provides a nice model of how to test the EFI
> code in a simple way.
> 
> Signed-off-by: Simon Glass 

This approach has been specifically rejected with an explained
rationale: It breaks how UEFI applications work and you cannot run
further UEFI tests in sandbox without resetting.

Since as you note, you can't reset in a C-based test, rework this to be
a python test where we can safely reset the system and verify that. I
believe Heinrich even noted that a test which checks ExitBootServices()
working as expected would be helpful as we only have a watchdog test
currently.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v9 03/11] sandbox: Add a -N flag to control on-host behaviour

[Tom Rini]

On Tue, Oct 29, 2024 at 08:22:11PM +0100, Simon Glass wrote:

> Sandbox is its own architecture, but sometimes we want to mimic the host
> architecture, e.g. when running an EFI app not built by U-Boot.
> 
> Add a -N/--native flag which tells sandbox to reflect the architecture
> of the host.
> 
> Signed-off-by: Simon Glass 
> ---
> 
> (no changes since v8)

I believe Heinrich asked that you invert the logic here and add a flag
for when you need the unexpected behavior.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v9 08/11] efi_loader: Drop sandbox PXE architecture

[Tom Rini]

On Tue, Oct 29, 2024 at 08:22:16PM +0100, Simon Glass wrote:

> Rather than returning 0, just return an error, since sandbox is not used
> with PXE at present.
> 
> Signed-off-by: Simon Glass 

Reviewed-by: Tom Rini 

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v1 0/3] Support Aspeed SGPIO controller

[Tom Rini]

On Wed, 16 Oct 2024 16:59:52 +0800, Billy Tsai wrote:

> AST2600 SoC has 2 SGPIO master interfaces one with 128 pins another one
> with 80 pins, AST2500/AST2400 SoC has 1 SGPIO master interface that
> supports up to 80 pins.
> 
> Billy Tsai (3):
>   gpio: Add Aspeed SGPIO driver
>   ARM: dts: ast2500: Add SGPIO to device tree
>   ARM: dts: ast2600: Add SGPIO to device tree
> 
> [...]

Applied to u-boot/master, thanks!

-- 
Tom

[PATCH] test/cmd/mem_copy.c: Use CONFIG_SYS_LOAD_ADDR for base

[Tom Rini]

When reading/writing to memory we cannot assume that a base address of
0x0 is correct and functional. So use CONFIG_SYS_LOAD_ADDR as the base
from which we add a bit more padding and being our tests.

Signed-off-by: Tom Rini 
---
 test/cmd/mem_copy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/cmd/mem_copy.c b/test/cmd/mem_copy.c
index 1ba0cebbbe04..67eca328777a 100644
--- a/test/cmd/mem_copy.c
+++ b/test/cmd/mem_copy.c
@@ -21,7 +21,7 @@ struct param {
 static int do_test(struct unit_test_state *uts,
   const char *suffix, int d, int s, int count)
 {
-   const long addr = 0x1000;
+   const long addr = CONFIG_SYS_LOAD_ADDR + 0x1000;
u8 shadow[BUF_SIZE];
u8 *buf;
int i, w, bytes;
-- 
2.43.0

Re: [PATCH 09/15] efi_loader: Fix free in ..._media_device_boot_option()

[Heinrich Schuchardt]

Am 29. Oktober 2024 16:45:29 MEZ schrieb Simon Glass :
>Hi Ilias,
>
>On Tue, 29 Oct 2024 at 11:02, Ilias Apalodimas
> wrote:
>>
>> Hi Simon,
>>
>> On Mon, 28 Oct 2024 at 14:48, Simon Glass  wrote:
>> >
>> > Freeing a NULL pointer is an error in EFI, so check the pointer first,
>> > before freeing it.
>> >
>> > Signed-off-by: Simon Glass 
>> > ---
>> >
>> >  lib/efi_loader/efi_bootmgr.c | 3 ++-
>> >  1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
>> > index a3aa2b8d1b9..431a38704e9 100644
>> > --- a/lib/efi_loader/efi_bootmgr.c
>> > +++ b/lib/efi_loader/efi_bootmgr.c
>> > @@ -1180,7 +1180,8 @@ out:
>> > free(opt[i].lo);
>> > }
>> > free(opt);
>> > -   efi_free_pool(handles);
>> > +   if (handles)
>> > +   efi_free_pool(handles);
>>
>> We don't need this, efi_free_pool() checks the pointer already.
>
>Yes, but it then returns an error (EFI_INVALID_PARAMETER), which gets
>logged, with this series.

So this is not a problem of the existing code but of your patch series which 
creates a superfluous log message.

Best regards

Heinrich

>
>Regards,
>Simon

Re: [PATCH 10/15] efi_loader: Add support for logging EFI calls

[Heinrich Schuchardt]

Am 28. Oktober 2024 13:48:01 MEZ schrieb Simon Glass :
>The current logging system suffers from some disadvantages, mainly that
>it writes its output to the console and cannot be easily reviewed.
>
>Add a dedicated log, storing records in a binary format and including
>the result codes and any return values from each call. The log is built
>sequentially in memory and can be reviewed after any EFI operation. It
>could potentially be written to media for later review, but that is not
>implemented so far.

An EFI specific solution is not a good approach as it does not scale to other 
parts of the code.  Please, implement a log driver to collect the messages that 
you are interested in.

Best regards

Heinrich

>
>Signed-off-by: Simon Glass 
>---
>
> MAINTAINERS  |   6 +
> include/bloblist.h   |   1 +
> include/efi.h|   1 +
> include/efi_log.h| 169 ++
> lib/efi_loader/Kconfig   |  19 +++
> lib/efi_loader/Makefile  |   1 +
> lib/efi_loader/efi_log.c | 256 +++
> 7 files changed, 453 insertions(+)
> create mode 100644 include/efi_log.h
> create mode 100644 lib/efi_loader/efi_log.c
>
>diff --git a/MAINTAINERS b/MAINTAINERS
>index 38c714cf46a..9ade0ca4bc3 100644
>--- a/MAINTAINERS
>+++ b/MAINTAINERS
>@@ -1067,6 +1067,12 @@ F:  lib/efi/efi_app.c
> F:scripts/build-efi.sh
> F:test/dm/efi_media.c
> 
>+EFI LOGGING
>+M:Simon Glass 
>+S:Maintained
>+F:include/efi_log.h
>+F:lib/efi_loader/efi_log.c
>+
> EFI PAYLOAD
> M:Heinrich Schuchardt 
> M:Ilias Apalodimas 
>diff --git a/include/bloblist.h b/include/bloblist.h
>index ff32d3fecfd..1e1ca34aa92 100644
>--- a/include/bloblist.h
>+++ b/include/bloblist.h
>@@ -153,6 +153,7 @@ enum bloblist_tag_t {
>   BLOBLISTT_U_BOOT_SPL_HANDOFF= 0xfff000, /* Hand-off info from SPL */
>   BLOBLISTT_VBE   = 0xfff001, /* VBE per-phase state */
>   BLOBLISTT_U_BOOT_VIDEO  = 0xfff002, /* Video info from SPL */
>+  BLOBLISTT_EFI_LOG   = 0xfff003, /* Log of EFI calls */
> };
> 
> /**
>diff --git a/include/efi.h b/include/efi.h
>index 84640cf7b25..f7419b80d4d 100644
>--- a/include/efi.h
>+++ b/include/efi.h
>@@ -127,6 +127,7 @@ static inline void *guidcpy(void *dst, const void *src)
> #define EFI_COMPROMISED_DATA  (EFI_ERROR_MASK | 33)
> #define EFI_IP_ADDRESS_CONFLICT   (EFI_ERROR_MASK | 34)
> #define EFI_HTTP_ERROR(EFI_ERROR_MASK | 35)
>+#define EFI_ERROR_COUNT   36
> 
> #define EFI_WARN_UNKNOWN_GLYPH1
> #define EFI_WARN_DELETE_FAILURE   2
>diff --git a/include/efi_log.h b/include/efi_log.h
>new file mode 100644
>index 000..1988e5f9df0
>--- /dev/null
>+++ b/include/efi_log.h
>@@ -0,0 +1,169 @@
>+/* SPDX-License-Identifier: GPL-2.0+ */
>+/*
>+ * Logging (to memory) of calls from an EFI app
>+ *
>+ * Copyright 2024 Google LLC
>+ * Written by Simon Glass 
>+ */
>+
>+#ifndef __EFI_LOG_H
>+#define __EFI_LOG_H
>+
>+#include 
>+#include 
>+
>+/**
>+ * enum efil_tag - Types of logging records which can be created
>+ */
>+enum efil_tag {
>+  EFILT_TESTING,
>+
>+  EFILT_COUNT,
>+};
>+
>+/**
>+ * struct efil_rec_hdr - Header for each logging record
>+ *
>+ * @tag: Tag which indicates the type of the record
>+ * @size: Size of the record in bytes
>+ * @ended: true if record has been completed (i.e. the function returned), 
>false
>+ *if it is still pending
>+ * @e_ret: Records the return function from the logged function
>+ */
>+struct efil_rec_hdr {
>+  enum efil_tag tag;
>+  int size;
>+  bool ended;
>+  efi_status_t e_ret;
>+};
>+
>+/**
>+ * struct efil_hdr - Holds the header for the log
>+ *
>+ * @upto: Offset at which to store the next log record
>+ * @size: Total size of the log in bytes
>+ */
>+struct efil_hdr {
>+  int upto;
>+  int size;
>+};
>+
>+enum efil_test_t {
>+  EFI_LOG_TEST0,
>+  EFI_LOG_TEST1,
>+
>+  EFI_LOG_TEST_COUNT,
>+};
>+
>+/**
>+ * struct efil_testing - used for testing the log
>+ */
>+struct efil_testing {
>+  enum efil_test_t enum_val;
>+  efi_uintn_t int_val;
>+  u64 *memory;
>+  void **buffer;
>+  u64 e_memory;
>+  void *e_buffer;
>+};
>+
>+/**
>+ * struct efil_allocate_pages - holds info from efi_allocate_pages() call
>+ *
>+ * @e_memory: Contains the value of *@memory on return from the EFI function
>+ */
>+struct efil_allocate_pages {
>+  enum efi_allocate_type type;
>+  enum efi_memory_type memory_type;
>+  efi_uintn_t pages;
>+  u64 *memory;
>+  u64 e_memory;
>+};
>+
>+/*
>+ * The functions below are in pairs, with a 'start' and 'end' call for each 
>EFI
>+ * function. The 'start' function (efi_logs_...) is called when the function 
>is
>+ * started. It records all the arguments. The 'end' function (efi_loge_...) is
>+ * called when the function is ready to return. It records any output 
>argument

Re: [PATCH] dm: core: downgrade some dm_warn messages to log_debug()

[Tom Rini]

On Tue, 15 Oct 2024 16:32:14 +0200, Quentin Schulz wrote:

> People complained that enabling (SPL_)DM_WARN was now totally unusable
> due to the amount of messages printed on the console.
> 
> Let's downgrade the log level of some messages that are clearly not on
> the error path.
> 
> Note that there's one pr_debug in there, because it is followed by
> pr_cont so it made sense to reuse the same family of functions.
> 
> [...]

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCH 1/1] cmd: simplify network definitions in Makefile

[Tom Rini]

On Sat, 19 Oct 2024 12:24:45 +0200, Heinrich Schuchardt wrote:

> /Makefile already adds lib include paths to UBOOTINCLUDE. There is no point
> in adding the same paths again.
> 
> Clearly separate the lines relating to NET and to NET_LWIP.
> 
> 

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCH] lmb: add a check to prevent memory overrun

[Tom Rini]

On Mon, 21 Oct 2024 22:48:20 +0530, Sughosh Ganu wrote:

> When printing the LMB flags for a memory region, there is a need to
> check that the array index that is computed is a sane value. Put a
> noisy assert in case this check fails, as that implies something with
> the LMB code is not working as expected.
> 
> 

Applied to u-boot/master, thanks!

-- 
Tom

Re: (subset) [PATCH 1/3] test/cmd: Make some "ut dm" tests only available on sandbox

[Tom Rini]

On Mon, 28 Oct 2024 10:48:42 -0600, Tom Rini wrote:

> Currently, the "dm" suite in unit tests (ut) is only available on
> sandbox. Make sure that all cmd tests that are part of this suite are
> only available on sandbox and not attempted to be run on hardware (where
> it will fail to be able to be started).
> 
> 

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCH] fdt: lmb: add reserved regions as no-overwrite

[Tom Rini]

On Mon, 21 Oct 2024 22:54:33 +0530, Sughosh Ganu wrote:

> The boot_fdt_add_mem_rsv_regions() function reserves the memreserve and
> reserved-memory regions. These regions are being set with the LMB_NONE
> flag which allows overwriting and re-using the regions. This was fine
> earlier when the LMB memory map was local and not enforced
> globally. But that is no longer the case. Mark these regions as
> LMB_NOOVERWRITE so that they cannot be used.
> 
> [...]

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCH] board_r: Remove duplicate headers

[Tom Rini]

On Wed, 23 Oct 2024 08:27:50 +0300, Ilias Apalodimas wrote:

> efi_loader.h is included twice. Remove one and move the other in
> alphabetical order
> 
> 

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCH v2] lmb: Remove lmb_alloc_flags()

[Tom Rini]

On Wed, 23 Oct 2024 18:26:36 +0300, Ilias Apalodimas wrote:

> lmb_alloc_flags() & lmb_alloc_base_flags() are just a wrappers for
> _lmb_alloc_base(). Since the only difference is the max address of the
> allowed allocation which _lmb_alloc_base() already supports with the
> LMB_ALLOC_ANYWHERE flag, remove one of them.
> 
> Keep the lmb_alloc_base_flags() which also prints an error on failures
> and adjust efi_allocate_pages() to only use one of them.
> 
> [...]

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCHv2] Kconfig: Remove TARGET_TRICORDER references

[Tom Rini]

On Sat, 26 Oct 2024 08:09:59 -0600, Tom Rini wrote:

> These were missed when removing the rest of the tricorder platform.
> 
> 

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCH] disk: Mark static functions in part_efi.c

[Tom Rini]

On Sat, 26 Oct 2024 11:05:53 +0300, Ilias Apalodimas wrote:

> Mark all the functions that are only defined locally as static and
> quiesce W=1 warnings
> 
> 

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCH] cmd: fat: Make do_fat_size static

[Tom Rini]

On Sat, 26 Oct 2024 10:33:09 +0300, Ilias Apalodimas wrote:

> This is only used locally,so make it static
> 
> 

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCH 1/2] lmb: Fix lmb_add_region_flags() return codes and testing

[Tom Rini]

On Wed, 23 Oct 2024 18:22:00 +0300, Ilias Apalodimas wrote:

> The function description says this should return 0 or -1 on failures.
> When regions coalesce though this returns the number of coalescedregions
> which is confusing and requires special handling of the return code.
> On top of that no one is using the number of coalesced regions.
> 
> So let's just return 0 on success and adjust our selftests accordingly
> 
> [...]

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCHv2 1/2] CONFIG_SYS_NONCACHED_MEMORY: Move prototypes to include/cpu_func.h for consistency

[Tom Rini]

On Tue, 22 Oct 2024 10:31:17 -0600, Tom Rini wrote:

> Currently, a number of generic cache related functions have their common
> prototype declared in include/cpu_func.h. Move the current set of
> noncached functions there as well to match.
> 
> 

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCH] libfdt: Fix build with swig 4.3.0

[David Gibson]

On Tue, Oct 29, 2024 at 08:52:52PM +0800, Xi Ruoyao wrote:
> On Sat, 2024-10-26 at 12:34 +, Rudi Heitbaum wrote:
> > Call SWIG_AppendOutput instead of SWIG_Python_AppendOutput so that
> > is_void is handled within swig.
> > 
> > Link: 
> > https://github.com/swig/swig/commit/cd39cf132c96a0887be07c826b80804d7677a701
> > 
> > Signed-off-by: Rudi Heitbaum 
> 
> Dtc is a separate project at
> https://git.kernel.org/pub/scm/utils/dtc/dtc.git, so the patch should be
> rebased against it and sent to devicetree-compi...@vger.kernel.org.

Looks like someone independently fixed it upstream:
https://github.com/dgibson/dtc/pull/154

I'm doing final tests now and expect to merge shortly.

> 
> > ---
> >  scripts/dtc/pylibfdt/libfdt.i_shipped | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/scripts/dtc/pylibfdt/libfdt.i_shipped 
> > b/scripts/dtc/pylibfdt/libfdt.i_shipped
> > index 56cc5d48f4..e4659489a9 100644
> > --- a/scripts/dtc/pylibfdt/libfdt.i_shipped
> > +++ b/scripts/dtc/pylibfdt/libfdt.i_shipped
> > @@ -1037,7 +1037,7 @@ typedef uint32_t fdt32_t;
> >     fdt_string(fdt1, fdt32_to_cpu($1->nameoff)));
> >     buff = PyByteArray_FromStringAndSize(
> >     (const char *)($1 + 1), fdt32_to_cpu($1->len));
> > -   resultobj = SWIG_Python_AppendOutput(resultobj, buff);
> > +   resultobj = SWIG_AppendOutput(resultobj, buff);
> >     }
> >  }
> >  
> > @@ -1076,7 +1076,7 @@ typedef uint32_t fdt32_t;
> >  
> >  %typemap(argout) int *depth {
> >  PyObject *val = Py_BuildValue("i", *arg$argnum);
> > -    resultobj = SWIG_Python_AppendOutput(resultobj, val);
> > +    resultobj = SWIG_AppendOutput(resultobj, val);
> >  }
> >  
> >  %apply int *depth { int *depth };
> > @@ -1092,7 +1092,7 @@ typedef uint32_t fdt32_t;
> >     if (PyTuple_GET_SIZE(resultobj) == 0)
> >    resultobj = val;
> >     else
> > -  resultobj = SWIG_Python_AppendOutput(resultobj, val);
> > +  resultobj = SWIG_AppendOutput(resultobj, val);
> >  }
> >  }
> >  
> 

-- 
David Gibson (he or they)   | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you, not the other way
| around.
http://www.ozlabs.org/~dgibson


signature.asc
Description: PGP signature

[PATCH] lib: uuid: fix 32-bit support

[Caleb Connolly]

In 22c48a92cdce (lib: uuid: supporting building as part of host tools),
some instances of simple_strtoull() were accidentally replaced with
strtoul() instead (spot the difference!).

To keep compatibility with building as part of host tools, re-implement
this via a new hextoull macro to handle both cases.

Fixes: 22c48a92cdce (lib: uuid: supporting building as part of host tools)
Signed-off-by: Caleb Connolly 
Reported-by: Patrick DELAUNAY 
---
I've build-tested this but don't have the hardware set up to easily test.
---
 lib/uuid.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/uuid.c b/lib/uuid.c
index c6a27b7d044d..59f07a9a5834 100644
--- a/lib/uuid.c
+++ b/lib/uuid.c
@@ -34,8 +34,11 @@
 
 #ifdef USE_HOSTCC
 /* polyfill hextoul to avoid pulling in strto.c */
 #define hextoul(cp, endp) strtoul(cp, endp, 16)
+#define hextoull(cp, endp) strtoull(cp, endp, 16)
+#else
+#define hextoull(cp, endp) simple_strtoull(cp, endp, 16)
 #endif
 
 int uuid_str_valid(const char *uuid)
 {
@@ -289,9 +292,9 @@ int uuid_str_to_bin(const char *uuid_str, unsigned char 
*uuid_bin,
return -EINVAL;
}
 
if (str_format == UUID_STR_FORMAT_STD) {
-   tmp32 = cpu_to_be32(hextoul(uuid_str, NULL));
+   tmp32 = cpu_to_be32(hextoull(uuid_str, NULL));
memcpy(uuid_bin, &tmp32, 4);
 
tmp16 = cpu_to_be16(hextoul(uuid_str + 9, NULL));
memcpy(uuid_bin + 4, &tmp16, 2);
@@ -326,9 +329,9 @@ int uuid_str_to_le_bin(const char *uuid_str, unsigned char 
*uuid_bin)
 
if (!uuid_str_valid(uuid_str) || !uuid_bin)
return -EINVAL;
 
-   tmp32 = cpu_to_le32(hextoul(uuid_str, NULL));
+   tmp32 = cpu_to_le32(hextoull(uuid_str, NULL));
memcpy(uuid_bin, &tmp32, 4);
 
tmp16 = cpu_to_le16(hextoul(uuid_str + 9, NULL));
memcpy(uuid_bin + 4, &tmp16, 2);
-- 
2.47.0

Re: [PATCH] lib: uuid: fix 32-bit support

[Tom Rini]

On Wed, Oct 30, 2024 at 01:32:55AM +0100, Caleb Connolly wrote:
> In 22c48a92cdce (lib: uuid: supporting building as part of host tools),
> some instances of simple_strtoull() were accidentally replaced with
> strtoul() instead (spot the difference!).
> 
> To keep compatibility with building as part of host tools, re-implement
> this via a new hextoull macro to handle both cases.
> 
> Fixes: 22c48a92cdce (lib: uuid: supporting building as part of host tools)
> Signed-off-by: Caleb Connolly 
> Reported-by: Patrick DELAUNAY 
> ---
> I've build-tested this but don't have the hardware set up to easily test.
> ---
>  lib/uuid.c | 7 +--
>  1 file changed, 5 insertions(+), 2 deletions(-)

Close, but not quite:
=== FAILURES ===
 test_ut[ut_lib_lib_test_dynamic_uuid] _
test/py/tests/test_ut.py:592: in test_ut
assert output.endswith('Failures: 0')
E   assert False
E+  where False = ('Failures: 0')
E+where  = 
'Test: lib_test_dynamic_uuid: uuid.c\r\ntest/lib/uuid.c:114, 
lib_test_dynamic_uuid_case(): expected_uuid = uuid_str: E...-8e063312964b", got 
"92a09169-9f8a-5f94-8635-0d9c2e830a92"\r\nTest lib_test_dynamic_uuid failed 1 
times\r\nFailures: 1'.endswith
- Captured stdout call -
U-Boot> ut lib lib_test_dynamic_uuid
Test: lib_test_dynamic_uuid: uuid.c
test/lib/uuid.c:114, lib_test_dynamic_uuid_case(): expected_uuid = uuid_str: 
Expected "985f2937-7c2e-5e9a-8a5e-8e063312964b", got 
"92a09169-9f8a-5f94-8635-0d9c2e830a92"
Test lib_test_dynamic_uuid failed 1 times
Failures: 1
U-Boot>

-- 
Tom


signature.asc
Description: PGP signature

[GIT PULL] Please pull u-boot-imx-master-20241029

[Fabio Estevam]

Hi Tom,

Please pull from u-boot-imx/master, thanks.

The following changes since commit dfe9e29f8381213c377bfa1de4d1ba7587c3b1c3:

  Merge https://source.denx.de/u-boot/custodians/u-boot-sh (2024-10-29 10:05:30 
-0600)

are available in the Git repository at:

  https://gitlab.denx.de/u-boot/custodians/u-boot-imx.git 
tags/u-boot-imx-master-20241029

for you to fetch changes up to d88bcd6d247a2b5d1683e393d8c9dc0259cd29f0:

  net: dwc_eth_qos: Remove obsolete imx8 includes (2024-10-29 16:25:53 -0300)

u-boot-imx-master-20241029
--

CI: https://source.denx.de/u-boot/custodians/u-boot-imx/-/pipelines/23092

- Implement i.MX93 erratum fix on the dwc_eth_qos driver to fix RMII.
- Add support for Emcraft Systems NavQ+.
- Fix the size of IVT + CSF blob tacked on to u-boot.itb.

Erik Schumacher (3):
  net: dwc_eth_qos: Add support for platform specific reset
  net: dwc_eth_qos_imx: Add platform specific reset for i.MX93
  net: dwc_eth_qos: Remove obsolete imx8 includes

Gilles Talis (1):
  board: emcraft: Add support for Emcraft Systems NavQ+

Marek Vasut (1):
  arm64: imx: Fix 0Xnn to 0xnn

Rasmus Villemoes (1):
  imx: hab: fix size of IVT+CSF blob tacked on to u-boot.itb

 arch/arm/dts/imx8mp-navqp-u-boot.dtsi  |  110 ++
 arch/arm/include/asm/arch-imx8m/imx-regs.h |2 +-
 arch/arm/mach-imx/imx8m/Kconfig|8 +
 board/emcraft/imx8mp_navqp/Kconfig |   15 +
 board/emcraft/imx8mp_navqp/MAINTAINERS |8 +
 board/emcraft/imx8mp_navqp/Makefile|   13 +
 board/emcraft/imx8mp_navqp/imx8mp_navqp.c  |   10 +
 board/emcraft/imx8mp_navqp/imx8mp_navqp.env|   18 +
 board/emcraft/imx8mp_navqp/imximage-8mp-lpddr4.cfg |8 +
 board/emcraft/imx8mp_navqp/lpddr4_timing.c | 1842 
 board/emcraft/imx8mp_navqp/spl.c   |  132 ++
 configs/imx8mp_navqp_defconfig |  104 ++
 doc/board/emcraft/imx8mp-navqp.rst |   65 +
 doc/board/emcraft/index.rst|9 +
 doc/board/index.rst|1 +
 drivers/net/dwc_eth_qos.c  |7 +-
 drivers/net/dwc_eth_qos.h  |1 +
 drivers/net/dwc_eth_qos_imx.c  |   22 +
 include/configs/imx8mp_navqp.h |   37 +
 tools/binman/etype/nxp_imx8mcst.py |2 +
 20 files changed, 2409 insertions(+), 5 deletions(-)
 create mode 100644 arch/arm/dts/imx8mp-navqp-u-boot.dtsi
 create mode 100644 board/emcraft/imx8mp_navqp/Kconfig
 create mode 100644 board/emcraft/imx8mp_navqp/MAINTAINERS
 create mode 100644 board/emcraft/imx8mp_navqp/Makefile
 create mode 100644 board/emcraft/imx8mp_navqp/imx8mp_navqp.c
 create mode 100644 board/emcraft/imx8mp_navqp/imx8mp_navqp.env
 create mode 100644 board/emcraft/imx8mp_navqp/imximage-8mp-lpddr4.cfg
 create mode 100644 board/emcraft/imx8mp_navqp/lpddr4_timing.c
 create mode 100644 board/emcraft/imx8mp_navqp/spl.c
 create mode 100644 configs/imx8mp_navqp_defconfig
 create mode 100644 doc/board/emcraft/imx8mp-navqp.rst
 create mode 100644 doc/board/emcraft/index.rst
 create mode 100644 include/configs/imx8mp_navqp.h

Re: [PATCH 1/3] net: dwc_eth_qos: Add support for platform specific reset

[Fabio Estevam]

On Mon, Oct 28, 2024 at 6:42 PM Erik Schumacher
 wrote:
>
> This patch adds support for optional platform specific reset logic in
> the dwc_eth_qos driver. This new function 'eqos_fix_soc_reset' is called
> after the EQOS_DMA_MODE_SWR is set and before the driver waits for this
> bit to clear.
>
> Signed-off-by: Erik Schumacher 

Applied all, thanks.

Re: [PATCH] arm64: imx: Fix 0Xnn to 0xnn

[Fabio Estevam]

On Sat, Oct 26, 2024 at 5:15 PM Marek Vasut  wrote:
>
> Use lowercase 0x prefix for hexadecimal number to be consistent
> No functional change.
>
> Signed-off-by: Marek Vasut 

Applied, thanks.

Re: [PATCH v3] board: emcraft: Add support for Emcraft Systems NavQ+

[Fabio Estevam]

On Sun, Oct 27, 2024 at 6:30 PM Fabio Estevam  wrote:

> This version looks good, thanks:
>
> Reviewed-by: Fabio Estevam 

Applied, thanks.

Re: [PATCH v8 06/11] lib: uuid: supporting building as part of host tools

[Caleb Connolly]

Hey Patrick,

Wow thanks for tracking this one down.

On 28/10/2024 15:14, Patrick DELAUNAY wrote:
> Hi,
> 
> 
> This patch seens cause problemes for STM32MP1 platforms, AARCH32
> 
> For FIP UUID configuration.
> 
> 
> On 8/30/24 14:34, Caleb Connolly wrote:
>> Adjust the UUID library code so that it can be compiled as part of a
>> host tool.
>>
>> This removes the one redundant log_debug() call, as well as the
>> incorrectly defined LOG_CATEGORY.
>>
>> In general this is a fairly trivial change, just adjusting includes and
>> disabling list_guid.
>>
>> This will be used by a new genguid tool to generate v5 GUIDs that match
>> those generated by U-Boot at runtime.
>>
>> Acked-by: Ilias Apalodimas 
>> Signed-off-by: Caleb Connolly 
>> ---
>>   include/uuid.h |  4 ++--
>>   lib/uuid.c | 44 ++--
>>   2 files changed, 32 insertions(+), 16 deletions(-)
>>
>> diff --git a/include/uuid.h b/include/uuid.h
>> index 1f4fa103b5e9..7f8414dc906c 100644
>> --- a/include/uuid.h
>> +++ b/include/uuid.h
>> @@ -69,10 +69,10 @@ struct uuid {
>>   } __packed;
>>     /* Bits of a bitmask specifying the output format for GUIDs */
>>   #define UUID_STR_FORMAT_STD    0
>> -#define UUID_STR_FORMAT_GUID    BIT(0)
>> -#define UUID_STR_UPPER_CASE    BIT(1)
>> +#define UUID_STR_FORMAT_GUID    0x1
>> +#define UUID_STR_UPPER_CASE    0x2
>>     /* Use UUID_STR_LEN + 1 for string space */
>>   #define UUID_STR_LEN    36
>>   #define UUID_BIN_LEN    sizeof(struct uuid)
>> diff --git a/lib/uuid.c b/lib/uuid.c
>> index c9dfdf007a18..6fdae7997702 100644
>> --- a/lib/uuid.c
>> +++ b/lib/uuid.c
>> @@ -6,25 +6,38 @@
>>    * Authors:
>>    *   Abdellatif El Khlifi 
>>    */
>>   -#define LOG_CATEGOT LOGC_CORE
>> -
>> +#ifndef USE_HOSTCC
>>   #include 
>>   #include 
>>   #include 
>>   #include 
>>   #include 
>> -#include 
>> -#include 
>> -#include 
>>   #include 
>>   #include 
>>   #include 
>>   #include 
>>   #include 
>> +#include 
>> +#include 
>> +#else
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>> +#endif
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>>   #include 
>>   +#ifdef USE_HOSTCC
>> +/* polyfill hextoul to avoid pulling in strto.c */
>> +#define hextoul(cp, endp) strtoul(cp, endp, 16)
>> +#endif
>> +
>>   int uuid_str_valid(const char *uuid)
>>   {
>>   int i, valid;
>>   @@ -51,8 +64,9 @@ int uuid_str_valid(const char *uuid)
>>   static const struct {
>>   const char *string;
>>   efi_guid_t guid;
>>   } list_guid[] = {
>> +#ifndef USE_HOSTCC
>>   #ifdef CONFIG_PARTITION_TYPE_GUID
>>   {"system",    PARTITION_SYSTEM_GUID},
>>   {"mbr",    LEGACY_MBR_PARTITION_GUID},
>>   {"msft",    PARTITION_MSFT_RESERVED_GUID},
>> @@ -231,8 +245,9 @@ static const struct {
>>   { "EFI_MEMORY_TYPE", EFI_MEMORY_TYPE },
>>   { "EFI_MEM_STATUS_CODE_REC", EFI_MEM_STATUS_CODE_REC },
>>   { "EFI_GUID_EFI_ACPI1", EFI_GUID_EFI_ACPI1 },
>>   #endif
>> +#endif /* !USE_HOSTCC */
>>   };
>>     int uuid_guid_get_bin(const char *guid_str, unsigned char *guid_bin)
>>   {
>> @@ -266,9 +281,8 @@ int uuid_str_to_bin(const char *uuid_str, unsigned
>> char *uuid_bin,
>>   uint32_t tmp32;
>>   uint64_t tmp64;
>>     if (!uuid_str_valid(uuid_str)) {
>> -    log_debug("not valid\n");
>>   #ifdef CONFIG_PARTITION_TYPE_GUID
>>   if (!uuid_guid_get_bin(uuid_str, uuid_bin))
>>   return 0;
>>   #endif
>> @@ -297,19 +311,19 @@ int uuid_str_to_bin(const char *uuid_str,
>> unsigned char *uuid_bin,
>>     tmp16 = cpu_to_be16(hextoul(uuid_str + 19, NULL));
>>   memcpy(uuid_bin + 8, &tmp16, 2);
>>   -    tmp64 = cpu_to_be64(simple_strtoull(uuid_str + 24, NULL, 16));
>> +    tmp64 = cpu_to_be64(hextoul(uuid_str + 24, NULL));
>>   memcpy(uuid_bin + 10, (char *)&tmp64 + 2, 6);
>>     return 0;
>>   }
>>     int uuid_str_to_le_bin(const char *uuid_str, unsigned char *uuid_bin)
>>   {
>> -    u16 tmp16;
>> -    u32 tmp32;
>> -    u64 tmp64;
>> +    uint16_t tmp16;
>> +    uint32_t tmp32;
>> +    uint64_t tmp64;
>>     if (!uuid_str_valid(uuid_str) || !uuid_bin)
>>   return -EINVAL;
>>   @@ -324,22 +338,22 @@ int uuid_str_to_le_bin(const char *uuid_str,
>> unsigned char *uuid_bin)
>>     tmp16 = cpu_to_le16(hextoul(uuid_str + 19, NULL));
>>   memcpy(uuid_bin + 8, &tmp16, 2);
>>   -    tmp64 = cpu_to_le64(simple_strtoull(uuid_str + 24, NULL, 16));
>> +    tmp64 = cpu_to_le64(hextoul(uuid_str + 24, NULL));
> 
> Here you change
> 
> simple_strtoull  => return unsigned long long
> 
> to
> 
> hextoul => return unsigned long
> 
> for 32bits system that can the cause of issue I think

Yep that would do it, damn! I think I seriously just misread "strtoull"
as "strtoul" and didn't notice :(

I've sent
https://lore.kernel.org/u-boot/20241030003317.2901772-2-caleb.conno...@linaro.org
which should address this. I would greatly appreciate it if you could
give it a test.

Kind regards,
> 
> with UUID defined in 

[PATCH v9 11/11] test: efi: boot: Add a test for the efi bootmeth

[Simon Glass]

Add a simple test of booting with the EFI bootmeth, which runs the app
and checks that it can call 'exit boot-services' (to check that all the
device-removal code doesn't break anything) and then exit back to
U-Boot.

This uses a disk image containing the testapp, ready for execution by
sandbox when needed.

Signed-off-by: Simon Glass 

---
Note that this uses the same mechanism as for the other images which are
created. Once this series lands I will update[1] and revisit.

[1] https://patchwork.ozlabs.org/project/uboot/patch/
20240802093322.15240-1-rich...@nod.at/

Changes in v9:
- Fix 'sevices' typo

Changes in v7:
- Drop patches already applied
- Drop patch 'Disable ANSI output for tests'
- Rebase on -master

Changes in v6:
- Deal with sandbox CONFIG_LOGF_FUNC
- Rebase on -next
- Drop patches previously applied
- Drop mention of helloworld since it is no-longer used by this test

Changes in v4:
- Add efi_loader tag to some patches
- Split out non-EFI patches into a different series

Changes in v2:
- Add many new patches to resolve all the outstanding test issues

 test/boot/bootflow.c | 60 
 1 file changed, 60 insertions(+)

diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index d7e94c8cc59..8d4cbbb0702 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #ifdef CONFIG_SANDBOX
 #include 
@@ -31,6 +32,9 @@ extern U_BOOT_DRIVER(bootmeth_android);
 extern U_BOOT_DRIVER(bootmeth_cros);
 extern U_BOOT_DRIVER(bootmeth_2script);
 
+/* Use this as the vendor for EFI to tell the app to exit boot services */
+static u16 __efi_runtime_data test_vendor[] = u"U-Boot testing";
+
 static int inject_response(struct unit_test_state *uts)
 {
/*
@@ -1217,3 +1221,59 @@ static int bootflow_android(struct unit_test_state *uts)
return 0;
 }
 BOOTSTD_TEST(bootflow_android, UTF_CONSOLE);
+
+/* Test EFI bootmeth */
+static int bootflow_efi(struct unit_test_state *uts)
+{
+   /* disable ethernet since the hunter will run dhcp */
+   test_set_eth_enable(false);
+
+   /* make USB scan without delays */
+   test_set_skip_delays(true);
+
+   bootstd_reset_usb();
+
+   ut_assertok(bootstd_test_drop_bootdev_order(uts));
+   ut_assertok(run_command("bootflow scan", 0));
+   ut_assert_skip_to_line(
+   "Bus usb@1: scanning bus usb@1 for devices... 5 USB Device(s) 
found");
+
+   ut_assertok(run_command("bootflow list", 0));
+
+   ut_assert_nextlinen("Showing all");
+   ut_assert_nextlinen("Seq");
+   ut_assert_nextlinen("---");
+   ut_assert_nextlinen("  0  extlinux");
+   ut_assert_nextlinen(
+   "  1  efi  ready   usb_mass_1  
usb_mass_storage.lun0.boo /EFI/BOOT/BOOTSBOX.EFI");
+   ut_assert_nextlinen("---");
+   ut_assert_skip_to_line("(2 bootflows, 2 valid)");
+   ut_assert_console_end();
+
+   ut_assertok(run_command("bootflow select 1", 0));
+   ut_assert_console_end();
+
+   systab.fw_vendor = test_vendor;
+
+   ut_asserteq(1, run_command("bootflow boot", 0));
+   ut_assert_nextline(
+   "** Booting bootflow 'usb_mass_storage.lun0.bootdev.part_1' 
with efi");
+   if (IS_ENABLED(CONFIG_LOGF_FUNC))
+   ut_assert_skip_to_line("   efi_run_image() Booting 
/\\EFI\\BOOT\\BOOTSBOX.EFI");
+   else
+   ut_assert_skip_to_line("Booting /\\EFI\\BOOT\\BOOTSBOX.EFI");
+
+   /* TODO: Why the \r ? */
+   ut_assert_nextline("U-Boot test app for EFI_LOADER\r");
+   ut_assert_nextline("Exiting boot services");
+   if (IS_ENABLED(CONFIG_LOGF_FUNC))
+   ut_assert_nextline(" do_bootefi_exec() ## Application 
failed, r = 5");
+   else
+   ut_assert_nextline("## Application failed, r = 5");
+   ut_assert_nextline("Boot failed (err=-22)");
+
+   ut_assert_console_end();
+
+   return 0;
+}
+BOOTSTD_TEST(bootflow_efi, UTF_CONSOLE);
-- 
2.43.0

RE: Issue in probing QSPI flash in Cyclone V SOC

[Santhosh Kumar Janardhanam]

Hi All,
Has Anyone faced this issue, any suggestion to resolve the below issue would be 
very helpful?

Thanks,
Santhosh
From: Santhosh Kumar Janardhanam
Sent: Friday, October 25, 2024 7:42 PM
To: u-boot@lists.denx.de
Subject: Issue in probing QSPI flash in Cyclone V SOC

Hi All,
we are testing QSPI flash in Cyclone V custom board. The QSPI pins used are 
same like cyclone V sockit board.we are using uboot 2024.01
when we do sf probe, we got the below error. we have verified the pin mux 
configuration and also enabled the clock.

=> sf probe 0:0
jedec_spi_nor flash@0: unrecognized JEDEC id bytes: ff, ff, ff
Failed to initialize SPI flash at 0:0 (error -2)

The DTS file contains the following entries

&qspi {
  status = "okay";

  flash: flash@0 {
 #address-cells = <1>;
 #size-cells = <1>;
 compatible = "n25q256a";
 reg = <0>;
 spi-max-frequency = <1>;

 m25p,fast-read;
 cdns,page-size = <256>;
 cdns,block-size = <16>;
 cdns,read-delay = <4>;
 cdns,tshsl-ns = <50>;
 cdns,tsd2d-ns = <50>;
 cdns,tchsh-ns = <4>;
 cdns,tslch-ns = <4>;
  };
};
The flash chip is connected in bus 0 and chipselect 0.
As anyone faced this issue earlier in uboot with cyclone V? can anyone guide 
what needs to be done to make sf probe working?

Regards,
Santhosh
::DISCLAIMER::

The contents of this e-mail and any attachment(s) are confidential and intended 
for the named recipient(s) only. E-mail transmission is not guaranteed to be 
secure or error-free as information could be intercepted, corrupted, lost, 
destroyed, arrive late or incomplete, or may contain viruses in transmission. 
The e mail and its contents (with or without referred errors) shall therefore 
not attach any liability on the originator or HCL or its affiliates. Views or 
opinions, if any, presented in this email are solely those of the author and 
may not necessarily reflect the views or opinions of HCL or its affiliates. Any 
form of reproduction, dissemination, copying, disclosure, modification, 
distribution and / or publication of this message without the prior written 
consent of authorized representative of HCL is strictly prohibited. If you have 
received this email in error please delete it and notify the sender 
immediately. Before opening any email and/or attachments, please check them for 
viruses and other defects.

Re: [PATCH 00/15] efi_loader: Add support for logging to a buffer

[Ilias Apalodimas]

Hi Simon,

On Mon, 28 Oct 2024 at 14:48, Simon Glass  wrote:
>
> It is a bit of a pain to log EFI boot-services calls at present. The
> output goes to the console so cannot easily be inspected later. Also it
> would be useful to be able to store the log and review it later, perhaps
> after something has gone wrong.
>
> This series makes a start on implementing a log-to-buffer feature. It
> provides a simple 'efidebug log' command to inspect the buffer. For now,
> only memory allocations are logged.

Why is this problem specific to EFI and no U-Boot in general? Do we
have a similar machinery for malloc()?

Thanks
/Ilias
>
> This feature makes it possible to add tests to check which EFI calls are
> made by U-Boot itself. It may also make it easier to figure out what is
> needed for booting Windows.
>
> Some patches to help with debugging sandbox memory-mapping are included,
> but for now EFI's use of mapping is not adjusted.
>
>
> Simon Glass (15):
>   log: Add a new category for tests
>   test: Allow saving and restoring the bloblist
>   bloblist: test: Mark tests with UTF_BLOBLIST
>   sandbox: Convert sb command to use new macro
>   doc: sandbox: Add docs for the sb command
>   sandbox: Add a way to show the sandbox memory-mapping
>   sandbox: Fix comment for nomap_sysmem() function
>   lmb: Drop extra 16KB of stack space
>   efi_loader: Fix free in ..._media_device_boot_option()
>   efi_loader: Add support for logging EFI calls
>   efi_loader: Create the log on startup
>   efi_loader: Add a command to show the EFI log
>   test: efi_loader: Add a simple test for the EFI log
>   efi_loader: Use the log with memory-related functions
>   efi_loader: Add documentation for the EFI log
>
>  MAINTAINERS|   7 +
>  arch/sandbox/cpu/cpu.c |  13 +
>  arch/sandbox/include/asm/cpu.h |   3 +
>  arch/sandbox/include/asm/io.h  |   2 +-
>  cmd/efidebug.c |  53 +++-
>  cmd/sb.c   |  42 ++--
>  common/log.c   |   1 +
>  configs/sandbox_defconfig  |   3 +
>  doc/develop/uefi/uefi.rst  |  22 ++
>  doc/usage/cmd/efidebug.rst | 109 
>  doc/usage/cmd/sb.rst   |  79 ++
>  doc/usage/index.rst|   2 +
>  include/bloblist.h |   1 +
>  include/efi.h  |   1 +
>  include/efi_log.h  | 316 +++
>  include/log.h  |   2 +
>  include/test/test.h|   3 +
>  lib/efi_loader/Kconfig |  19 ++
>  lib/efi_loader/Makefile|   1 +
>  lib/efi_loader/efi_bootmgr.c   |   3 +-
>  lib/efi_loader/efi_log.c   | 444 +
>  lib/efi_loader/efi_memory.c| 119 ++---
>  lib/efi_loader/efi_setup.c |   7 +
>  lib/lmb.c  |   2 -
>  test/common/bloblist.c |  28 +--
>  test/lib/Makefile  |   1 +
>  test/lib/efi_log.c |  93 +++
>  test/test-main.c   |  13 +
>  28 files changed, 1305 insertions(+), 84 deletions(-)
>  create mode 100644 doc/usage/cmd/efidebug.rst
>  create mode 100644 doc/usage/cmd/sb.rst
>  create mode 100644 include/efi_log.h
>  create mode 100644 lib/efi_loader/efi_log.c
>  create mode 100644 test/lib/efi_log.c
>
> --
> 2.43.0
>

Re: [PATCH] libfdt: Fix build with swig 4.3.0

[Xi Ruoyao]

On Sat, 2024-10-26 at 12:34 +, Rudi Heitbaum wrote:
> Call SWIG_AppendOutput instead of SWIG_Python_AppendOutput so that
> is_void is handled within swig.
> 
> Link: 
> https://github.com/swig/swig/commit/cd39cf132c96a0887be07c826b80804d7677a701
> 
> Signed-off-by: Rudi Heitbaum 

Dtc is a separate project at
https://git.kernel.org/pub/scm/utils/dtc/dtc.git, so the patch should be
rebased against it and sent to devicetree-compi...@vger.kernel.org.

> ---
>  scripts/dtc/pylibfdt/libfdt.i_shipped | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/dtc/pylibfdt/libfdt.i_shipped 
> b/scripts/dtc/pylibfdt/libfdt.i_shipped
> index 56cc5d48f4..e4659489a9 100644
> --- a/scripts/dtc/pylibfdt/libfdt.i_shipped
> +++ b/scripts/dtc/pylibfdt/libfdt.i_shipped
> @@ -1037,7 +1037,7 @@ typedef uint32_t fdt32_t;
>   fdt_string(fdt1, fdt32_to_cpu($1->nameoff)));
>   buff = PyByteArray_FromStringAndSize(
>   (const char *)($1 + 1), fdt32_to_cpu($1->len));
> - resultobj = SWIG_Python_AppendOutput(resultobj, buff);
> + resultobj = SWIG_AppendOutput(resultobj, buff);
>   }
>  }
>  
> @@ -1076,7 +1076,7 @@ typedef uint32_t fdt32_t;
>  
>  %typemap(argout) int *depth {
>  PyObject *val = Py_BuildValue("i", *arg$argnum);
> -    resultobj = SWIG_Python_AppendOutput(resultobj, val);
> +    resultobj = SWIG_AppendOutput(resultobj, val);
>  }
>  
>  %apply int *depth { int *depth };
> @@ -1092,7 +1092,7 @@ typedef uint32_t fdt32_t;
>     if (PyTuple_GET_SIZE(resultobj) == 0)
>    resultobj = val;
>     else
> -  resultobj = SWIG_Python_AppendOutput(resultobj, val);
> +  resultobj = SWIG_AppendOutput(resultobj, val);
>  }
>  }
>  

-- 
Xi Ruoyao 
School of Aerospace Science and Technology, Xidian University

Re: [PATCH v2 3/8] sysinfo: Add sysinfo driver and data structure for smbios

[Simon Glass]

Hi Raymond,

On Mon, 28 Oct 2024 at 20:44, Raymond Mao  wrote:
>
> Hi Simon,
>
> On Mon, 28 Oct 2024 at 13:04, Simon Glass  wrote:
>>
>> Hi Raymond,
>>
>> On Tue, 22 Oct 2024 at 22:06, Raymond Mao  wrote:
>> >
>> > Add sysinfo driver to retrieve smbios information (Type 4 and 7).
>> > So that the smbios library can use it for getting values from the
>> > hardware platform instead of device tree.
>> >
>> > Signed-off-by: Raymond Mao 
>> > ---
>> > Changes in v2
>> > - Move the changes to smbios.c instead of creating new file.
>> > - Move the headfile to include dir.
>> > - Combine with #6(v1) patch.
>> > - Clean-up the private data structures.
>> > - Clean-up the operations of the strings and common values.
>> >
>> >  drivers/sysinfo/smbios.c | 228 +++
>> >  include/smbios.h |  60 +++
>> >  include/smbios_plat.h|  79 ++
>> >  include/sysinfo.h|  95 +++-
>> >  4 files changed, 461 insertions(+), 1 deletion(-)
>> >  create mode 100644 include/smbios_plat.h
>> >
>> > diff --git a/drivers/sysinfo/smbios.c b/drivers/sysinfo/smbios.c
>> > index a7ac8e3f072..3980845b3ba 100644
>> > --- a/drivers/sysinfo/smbios.c
>> > +++ b/drivers/sysinfo/smbios.c
>> > @@ -5,14 +5,240 @@
>> >   */
>> >
>> >  #include 
>> > +#include 
>> >  #include 
>> >
>> > +/* platform information storage */
>> > +struct processor_info processor_info;
>> > +struct cache_info cache_info[SYSINFO_CACHE_LVL_MAX];
>> > +struct sysinfo_plat sysinfo_smbios_p = {
>> > +   /* Processor Information */
>> > +   .processor = &processor_info,
>> > +   /* Cache Information */
>> > +   .cache = &cache_info[0],
>> > +};
>> > +
>> > +/* structure for smbios private data storage */
>> > +struct sysinfo_plat_priv {
>> > +   struct processor_info *t4;
>> > +   struct smbios_type7 t7[SYSINFO_CACHE_LVL_MAX];
>> > +   u16 cache_handles[SYSINFO_CACHE_LVL_MAX];
>> > +   u8 cache_level;
>> > +};
>> > +
>> > +static void smbios_cache_info_dump(struct smbios_type7 *cache_info)
>> > +{
>> > +   log_debug("SMBIOS Type 7 (Cache Information):\n");
>> > +   log_debug("Cache Configuration: 0x%04x\n", 
>> > cache_info->config.data);
>> > +   log_debug("Maximum Cache Size: %u KB\n", 
>> > cache_info->max_size.data);
>> > +   log_debug("Installed Size: %u KB\n", cache_info->inst_size.data);
>> > +   log_debug("Supported SRAM Type: 0x%04x\n",
>>
>> %#04x
>>
>> > + cache_info->supp_sram_type.data);
>> > +   log_debug("Current SRAM Type: 0x%04x\n",
>> > + cache_info->curr_sram_type.data);
>> > +   log_debug("Cache Speed: %u\n", cache_info->speed);
>> > +   log_debug("Error Correction Type: %u\n", 
>> > cache_info->err_corr_type);
>> > +   log_debug("System Cache Type: %u\n", cache_info->sys_cache_type);
>> > +   log_debug("Associativity: %u\n", cache_info->associativity);
>> > +   log_debug("Maximum Cache Size 2: %u KB\n", 
>> > cache_info->max_size2.data);
>> > +   log_debug("Installed Cache Size 2: %u KB\n",
>> > + cache_info->inst_size2.data);
>> > +}
>> > +
>> > +/* weak function for the platforms not yet supported */
>> > +__weak int sysinfo_get_cache_info(u8 level, struct cache_info *cache_info)
>> > +{
>> > +   return -ENOSYS;
>> > +}
>> > +
>> > +__weak int sysinfo_get_processor_info(struct processor_info *pinfo)
>> > +{
>> > +   return -ENOSYS;
>> > +}
>> > +
>> > +void sysinfo_cache_info_default(struct cache_info *ci)
>> > +{
>> > +   memset(ci, 0, sizeof(*ci));
>> > +   ci->config.data = SMBIOS_CACHE_LOCATE_UNKNOWN | 
>> > SMBIOS_CACHE_OP_UND;
>> > +   ci->supp_sram_type.fields.unknown = 1;
>> > +   ci->curr_sram_type.fields.unknown = 1;
>> > +   ci->speed = SMBIOS_CACHE_SPEED_UNKNOWN;
>> > +   ci->err_corr_type = SMBIOS_CACHE_ERRCORR_UNKNOWN;
>> > +   ci->cache_type = SMBIOS_CACHE_SYSCACHE_TYPE_UNKNOWN;
>> > +}
>> > +
>> > +static int sysinfo_plat_detect(struct udevice *dev)
>> > +{
>> > +   return 0;
>> > +}
>> > +
>> > +static int sysinfo_plat_get_str(struct udevice *dev, int id,
>> > +   size_t size, char *val)
>> > +{
>> > +   struct sysinfo_plat_priv *priv = dev_get_priv(dev);
>> > +   const char *str = NULL;
>> > +
>> > +   switch (id) {
>> > +   case SYSINFO_ID_SMBIOS_PROCESSOR_MANUFACT:
>>
>> These are getting too long.
>>
>> How about SYSINFOSM_PROC_MANUF ?
>>
>> We can use SYSINFOSM as short for SYSINFO_ID_SMBIOS
>>
> Other existing macros are starting from SYSINFO_ID_, so I guess something
> reasonable can be SYSINFO_ID_SMB_PROC_MANU.

That is certainly better. Perhaps SYSINFO_ID_ should become SYSID_...?

[..]

Regards,
Simon

Re: Pincontrol driver

[Simon Glass]

Hi Jayaramudu,

On Thu, 24 Oct 2024 at 06:58, Jayaramudu .D  wrote:
>
> Hi,
>
> I am working on a board which AMD soc, trying to enable gpio at uboot, but
> I did not see AMD pincontrol driver in U-boot 2024. Could you please
> suggest any info.

I would suggest you could look at the MAINTAINERS file and cc the
people involved. Also it is a good idea to mention which SoC it is.

Regards,
Simon

Re: [PATCH v3 2/2] binman: expand test coverage to nxp_imx8mcst

[Simon Glass]

Hi Brian,

On Mon, 21 Oct 2024 at 09:38, Brian Ruley  wrote:
>
> Add coverage for IMX8M code siging. Create PKI tree and other assets
> required by `cst' using `hab4_pki_tree.sh' script and `srktool' in
> `cst_3.4.1' [1].
>
> [1] https://www.nxp.com/webapp/Download?colCode=IMX_CST_TOOL_NEW
>
> Signed-off-by: Brian Ruley 
> ---
> Changes for v2:
> - Added missing *.pem files
> - Rebased on top of "[PATCH v4 2/2] binman: add fast authentication
>   method for i.MX8M signing"
> - Included a test for fast authentication
> Changes for v3:
> - Fixed relative path for SRK table and *.pem files in
>   341_nxp_imx8mcst.dts
>
>  tools/binman/ftest.py |  11 ++
>  tools/binman/test/340_nxp_imx8mcst.dts|  58 +
>  .../test/341_nxp_imx8mcst_fast_auth.dts   |  18 +++
>  .../CSF1_1_sha256_4096_65537_v3_usr_crt.pem   | 121 ++
>  .../IMG1_1_sha256_4096_65537_v3_usr_crt.pem   | 121 ++
>  .../SRK1_sha256_4096_65537_v3_usr_crt.pem | 121 ++
>  tools/binman/test/cst/crts/SRK_table.bin  | Bin 0 -> 531 bytes
>  .../test/cst/crts/SRK_table_fast_auth.bin | Bin 0 -> 531 bytes
>  .../CSF1_1_sha256_4096_65537_v3_usr_key.pem   |  54 
>  .../IMG1_1_sha256_4096_65537_v3_usr_key.pem   |  54 
>  .../SRK1_sha256_4096_65537_v3_usr_key.pem |  54 
>  tools/binman/test/cst/keys/key_pass.txt   |   2 +
>  12 files changed, 614 insertions(+)
>  create mode 100644 tools/binman/test/340_nxp_imx8mcst.dts
>  create mode 100644 tools/binman/test/341_nxp_imx8mcst_fast_auth.dts
>  create mode 100644 
> tools/binman/test/cst/crts/CSF1_1_sha256_4096_65537_v3_usr_crt.pem
>  create mode 100644 
> tools/binman/test/cst/crts/IMG1_1_sha256_4096_65537_v3_usr_crt.pem
>  create mode 100644 
> tools/binman/test/cst/crts/SRK1_sha256_4096_65537_v3_usr_crt.pem
>  create mode 100644 tools/binman/test/cst/crts/SRK_table.bin
>  create mode 100644 tools/binman/test/cst/crts/SRK_table_fast_auth.bin
>  create mode 100644 
> tools/binman/test/cst/keys/CSF1_1_sha256_4096_65537_v3_usr_key.pem
>  create mode 100644 
> tools/binman/test/cst/keys/IMG1_1_sha256_4096_65537_v3_usr_key.pem
>  create mode 100644 
> tools/binman/test/cst/keys/SRK1_sha256_4096_65537_v3_usr_key.pem
>  create mode 100644 tools/binman/test/cst/keys/key_pass.txt

I am still seeing some sort of problem here:

==
ERROR: testNxpImx8mCst (binman.ftest.TestFunctional.testNxpImx8mCst)
Test that binman can sign an iMX8M image
--
ValueError: Filename 'cst/crts/SRK_table.bin' not found in input path
(/tmp/binmant.tryjm0q0) (cwd='/home/sglass/files.local/u-boot')

==
ERROR: testNxpImx8mCstFastAuth
(binman.ftest.TestFunctional.testNxpImx8mCstFastAuth)
Test that binman can sign an iMX8M image using fast authentication
--
ValueError: Filename 'cst/crts/SRK_table_fast_auth.bin' not found in
input path (/tmp/binmant.tryjm0q0)
(cwd='/home/sglass/files.local/u-boot')


but it could be because I had trouble applying it:

 git am 
~/Downloads/v3-1-2-binman-nxp_imx8mcst-read-certificates-from-input-path.patch
Applying: binman: nxp_imx8mcst: read certificates from input path
Applying: binman: expand test coverage to nxp_imx8mcst
.git/rebase-apply/patch:210: trailing whitespace.
X509v3 Basic Constraints:
.git/rebase-apply/patch:212: trailing whitespace.
Netscape Comment:
.git/rebase-apply/patch:214: trailing whitespace.
X509v3 Subject Key Identifier:
.git/rebase-apply/patch:216: trailing whitespace.
X509v3 Authority Key Identifier:
.git/rebase-apply/patch:337: trailing whitespace.
X509v3 Basic Constraints:
error: patch failed: tools/binman/ftest.py:7804
error: tools/binman/ftest.py: patch does not apply
Patch failed at 0002 binman: expand test coverage to nxp_imx8mcst
hint: Use 'git am --show-current-patch=diff' to see the failed patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
sglass@okaro:~/u$ pm
patching file tools/binman/ftest.py
Hunk #2 merged at 7906-7912.
patching file tools/binman/test/340_nxp_imx8mcst.dts
patching file tools/binman/test/341_nxp_imx8mcst_fast_auth.dts
patching file tools/binman/test/cst/crts/CSF1_1_sha256_4096_65537_v3_usr_crt.pem
patching file tools/binman/test/cst/crts/IMG1_1_sha256_4096_65537_v3_usr_crt.pem
patching file tools/binman/test/cst/crts/SRK1_sha256_4096_65537_v3_usr_crt.pem
File tools/binman/test/cst/crts/SRK_table.bin: git binary diffs are
not supported.
File tools/binman/test/cst/crts/SRK_table_fast_auth.bin: git binary
diffs are not supported.
patching file tools/binman/test/cst/

Re: [PATCH v8 2/8] efi_loader: Add a test app

[Simon Glass]

Hi Heinrich,

On Mon, 28 Oct 2024 at 19:43, Heinrich Schuchardt  wrote:
>
> On 10/28/24 18:00, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Mon, 28 Oct 2024 at 07:11, Heinrich Schuchardt  > > wrote:
> >  >
> >  > On 10/22/24 14:00, Simon Glass wrote:
> >  > > Add a simple app to use for testing. This is intended to do whatever it
> >  > > needs to for testing purposes. For now it just prints a message and
> >  > > exits boot services.
> >  > >
> >  > > There was a considerable amount of discussion about whether it is OK to
> >  > > call exit-boot-services and then return to U-Boot. This is not normally
> >  > > done in a real application, since exit-boot-services is used to
> >  > > completely disconnect from U-Boot. However, since this is a test, we
> >  > > need to check the results of running the app, so returning is
> > necessary.
> >  > > It works correctly and it provides a nice model of how to test the EFI
> >  > > code in a simple way.
> >  > >
> >  > > Signed-off-by: Simon Glass  > >
> >  > > ---
> >  > >
> >  > > (no changes since v7)
> >  > >
> >  > > Changes in v7:
> >  > > - Update commit message
> >  > >
> >  > >   lib/efi_loader/Kconfig   | 10 ++
> >  > >   lib/efi_loader/Makefile  |  1 +
> >  > >   lib/efi_loader/testapp.c | 68 +++
> > +
> >  > >   3 files changed, 79 insertions(+)
> >  > >   create mode 100644 lib/efi_loader/testapp.c
> >  > >
> >  > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> >  > > index 69b2c9360d8..6ced29da719 100644
> >  > > --- a/lib/efi_loader/Kconfig
> >  > > +++ b/lib/efi_loader/Kconfig
> >  > > @@ -565,6 +565,16 @@ config BOOTEFI_HELLO_COMPILE
> >  > > No additional space will be required in the resulting U-
> > Boot binary
> >  > > when this option is enabled.
> >  > >
> >  > > +config BOOTEFI_TESTAPP_COMPILE
> >  > > + bool "Compile an EFI test app for testing"
> >  > > + default y
> >  > > + help
> >  > > +   This compiles an app designed for testing. It is packed
> > into an image
> >  > > +   by the test.py testing frame in the setup_efi_image() function.
> >  > > +
> >  > > +   No additional space will be required in the resulting U-
> > Boot binary
> >  > > +   when this option is enabled.
> >  > > +
> >  > >   endif
> >  > >
> >  > >   source "lib/efi/Kconfig"
> >  > > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> >  > > index 00d18966f9e..87131ab911d 100644
> >  > > --- a/lib/efi_loader/Makefile
> >  > > +++ b/lib/efi_loader/Makefile
> >  > > @@ -20,6 +20,7 @@ apps-$(CONFIG_EFI_LOAD_FILE2_INITRD) += initrddump
> >  > >   ifeq ($(CONFIG_GENERATE_ACPI_TABLE),)
> >  > >   apps-y += dtbdump
> >  > >   endif
> >  > > +apps-$(CONFIG_BOOTEFI_TESTAPP_COMPILE) += testapp
> >  > >
> >  > >   obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
> >  > >   obj-$(CONFIG_EFI_BOOTMGR) += efi_bootmgr.o
> >  > > diff --git a/lib/efi_loader/testapp.c b/lib/efi_loader/testapp.c
> >  > > new file mode 100644
> >  > > index 000..feb444c92e9
> >  > > --- /dev/null
> >  > > +++ b/lib/efi_loader/testapp.c
> >  > > @@ -0,0 +1,68 @@
> >  > > +// SPDX-License-Identifier: GPL-2.0+
> >  >
> >  > This is not a valid SPDX identifier. Please, use GPL-2.0-or-later.
> >  >
> >  > > +/*
> >  > > + * Hello world EFI application
> >  > > + *
> >  > > + * Copyright 2024 Google LLC
> >  > > + * Written by Simon Glass  > >
> >  > > + *
> >  > > + * This test program is used to test the invocation of an EFI
> > application.
> >  > > + * It writes a few messages to the console and then exits boot
> > services
> >  > > + */
> >  > > +
> >  > > +#include 
> >  > > +
> >  > > +static const efi_guid_t loaded_image_guid =
> > EFI_LOADED_IMAGE_PROTOCOL_GUID;
> >  > > +
> >  > > +static struct efi_system_table *systable;
> >  > > +static struct efi_boot_services *boottime;
> >  > > +static struct efi_simple_text_output_protocol *con_out;
> >  > > +
> >  > > +/**
> >  > > + * efi_main() - entry point of the EFI application.
> >  > > + *
> >  > > + * @handle:  handle of the loaded image
> >  > > + * @systab:  system table
> >  > > + * Return:   status code
> >  > > + */
> >  > > +efi_status_t EFIAPI efi_main(efi_handle_t handle,
> >  > > +  struct efi_system_table *systab)
> >  > > +{
> >  > > + struct efi_loaded_image *loaded_image;
> >  > > + efi_status_t ret;
> >  > > + efi_uintn_t map_size;
> >  > > + efi_uintn_t map_key;
> >  > > + efi_uintn_t desc_size;
> >  > > + u32 desc_version;
> >  > > +
> >  > > + systable = systab;
> >  > > + boottime = systable->boottime;
> >  > > + con_out = systable->con_out;
> >  > > +
> >  > > + /* Get the loaded image protocol */
> >  > > + ret = boottime->open_protocol(handle, &loaded_image_guid,
> >  > > +   (void **)&loaded_image, NULL, NULL,
> >  > > +   

Re: [PATCH 03/21] bloblist: test: doc: Move into the common suite

[Simon Glass]

Hi Tom,

On Mon, 28 Oct 2024 at 20:33, Tom Rini  wrote:
>
> On Mon, Oct 28, 2024 at 01:41:08PM +0100, Simon Glass wrote:
>
> > There is no particular need for bloblist to have its own test suite.
> > Move it into the common suite instead.
> >
> > Add the missing help for 'common' and update the docs.
> >
> > Signed-off-by: Simon Glass 
>
> This feels both like churn and weird. I would think "common" tests are
> for things that are kinda legacy and too catch-all to have their own
> subgroup.

It's just chosen so that the test for common/ is in test/common - i.e.
it makes it easier for people to find it.

BTW this series removes all tests from test/*.c and moves them into
appropriate subdirs. Some of the tests are quite old so have just sat
there all these years.

Regards,
Simon

Re: [PATCH] test/py: spi: prevent overwriting relocation memory

[Tom Rini]

On Tue, Oct 29, 2024 at 05:17:09PM +0530, Padmarao Begari wrote:

> Update spi negative test case to prevent SF command
> from overwriting relocation memory area.
> 
> Signed-off-by: Padmarao Begari 
> ---
>  test/py/tests/test_spi.py | 12 
>  1 file changed, 12 insertions(+)
> 
> diff --git a/test/py/tests/test_spi.py b/test/py/tests/test_spi.py
> index 3160d58540..caca930327 100644
> --- a/test/py/tests/test_spi.py
> +++ b/test/py/tests/test_spi.py
> @@ -693,4 +693,16 @@ def test_spi_negative(u_boot_console):
>  u_boot_console, 'read', offset, size, addr, 1, error_msg, 
> EXPECTED_READ
>  )
>  
> +# Read to relocation address
> +output = u_boot_console.run_command('bdinfo')
> +m = re.search('relocaddr\s*= (.+)', output)
> +res_area = int(m.group(1), 16)
> +
> +start = 0
> +size = 0x2000
> +error_msg = 'ERROR: trying to overwrite reserved memory'
> +flash_ops(
> +u_boot_console, 'read', start, size, res_area, 1, error_msg, 
> EXPECTED_READ
> +)
> +
>  i = i + 1

This got me looking at test/py/tests/test_spi.py more closely and this
test is fine, so:

Reviewed-by: Tom Rini 

However, the error message 'No env file to read for SPI family device
test' should really say that it's no test device configured (and similar
change for env__spi_lock_unlock). A follow-up to rephrase those error
messages would be appreciated.

-- 
Tom


signature.asc
Description: PGP signature

[PATCH v1 1/1] drivers: bootcount: Add ZynqMP specific bootcount support

[Vasileios Amoiridis]

From: Vasileios Amoiridis 

Add native support of the bootcount mechanism in the ZynqMP by utilising 
internal
PMU registers. The Persistent Global Storage Registers of the Platform 
Management
Unit can keep their value during reboot cycles unless there is a POR reset, 
making
them appropriate for the bootcount mechanism.

Signed-off-by: Vasileios Amoiridis 
---
 drivers/bootcount/Kconfig|  4 
 drivers/bootcount/Makefile   |  1 +
 drivers/bootcount/bootcount_zynqmp.c | 28 
 3 files changed, 33 insertions(+)
 create mode 100644 drivers/bootcount/bootcount_zynqmp.c

diff --git a/drivers/bootcount/Kconfig b/drivers/bootcount/Kconfig
index fa6d8e7128..95b6a9541a 100644
--- a/drivers/bootcount/Kconfig
+++ b/drivers/bootcount/Kconfig
@@ -82,6 +82,10 @@ config BOOTCOUNT_AT91
bool "Boot counter for Atmel AT91SAM9XE"
depends on AT91SAM9XE
 
+config BOOTCOUNT_ZYNQMP
+   bool "Boot counter for Zynq UltraScale+ MPSoC"
+   depends on ARCH_ZYNQMP
+
 config DM_BOOTCOUNT
 bool "Boot counter in a device-model device"
help
diff --git a/drivers/bootcount/Makefile b/drivers/bootcount/Makefile
index 245f879633..487adc1212 100644
--- a/drivers/bootcount/Makefile
+++ b/drivers/bootcount/Makefile
@@ -3,6 +3,7 @@
 obj-$(CONFIG_BOOTCOUNT_GENERIC)+= bootcount.o
 obj-$(CONFIG_BOOTCOUNT_MEM)+= bootcount.o
 obj-$(CONFIG_BOOTCOUNT_AT91)   += bootcount_at91.o
+obj-$(CONFIG_BOOTCOUNT_ZYNQMP)  += bootcount_zynqmp.o
 obj-$(CONFIG_BOOTCOUNT_AM33XX) += bootcount_davinci.o
 obj-$(CONFIG_BOOTCOUNT_RAM)+= bootcount_ram.o
 obj-$(CONFIG_BOOTCOUNT_ENV)+= bootcount_env.o
diff --git a/drivers/bootcount/bootcount_zynqmp.c 
b/drivers/bootcount/bootcount_zynqmp.c
new file mode 100644
index 00..9bb801e188
--- /dev/null
+++ b/drivers/bootcount/bootcount_zynqmp.c
@@ -0,0 +1,28 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+// SPDX-FileCopyrightText: 2024 CERN (home.cern)
+
+#include 
+#include 
+#include 
+
+#define PMU_PERS_GLOB_GEN_STORAGE0 0x00FFD80050
+
+void bootcount_store(ulong a)
+{
+   int ret;
+
+   ret = zynqmp_mmio_write(PMU_PERS_GLOB_GEN_STORAGE0, 0xFF, a);
+   if (ret)
+   printf("Failed: mmio write\n");
+}
+
+ulong bootcount_load(void)
+{
+   int ret;
+   u32 val;
+
+   ret = zynqmp_mmio_read(PMU_PERS_GLOB_GEN_STORAGE0, &val);
+   if (ret)
+   printf("Failed: mmio read\n");
+   return val;
+}
-- 
2.34.1

Re: (subset) [PATCH v2 00/13] boards: siemens: iot2050: SM variant, sysinfo support, fixes & cleanups

[Tom Rini]

On Tue, 22 Oct 2024 08:04:17 +0200, Jan Kiszka wrote:

> Changes in v2:
>  - rebased over master, adding the missing 6.12 cherry-picks from
>devicetree-rebasing
> 
> This adds support for the new IOT2050 SM variant, introduces a sysinfo
> driver which also permits SMBIOS support and switches the board to
> OF_UPSTREAM. There are some further fixes for the boards included as well.
> 
> [...]

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCH v8 2/8] efi_loader: Add a test app

[Tom Rini]

On Tue, Oct 29, 2024 at 04:46:16PM +0100, Simon Glass wrote:
> Hi Heinrich,
> 
> On Mon, 28 Oct 2024 at 19:43, Heinrich Schuchardt  wrote:
> >
> > On 10/28/24 18:00, Simon Glass wrote:
> > > Hi Heinrich,
> > >
> > > On Mon, 28 Oct 2024 at 07:11, Heinrich Schuchardt  > > > wrote:
> > >  >
> > >  > On 10/22/24 14:00, Simon Glass wrote:
> > >  > > Add a simple app to use for testing. This is intended to do whatever 
> > > it
> > >  > > needs to for testing purposes. For now it just prints a message and
> > >  > > exits boot services.
> > >  > >
> > >  > > There was a considerable amount of discussion about whether it is OK 
> > > to
> > >  > > call exit-boot-services and then return to U-Boot. This is not 
> > > normally
> > >  > > done in a real application, since exit-boot-services is used to
> > >  > > completely disconnect from U-Boot. However, since this is a test, we
> > >  > > need to check the results of running the app, so returning is
> > > necessary.
> > >  > > It works correctly and it provides a nice model of how to test the 
> > > EFI
> > >  > > code in a simple way.
> > >  > >
> > >  > > Signed-off-by: Simon Glass  > > >
> > >  > > ---
> > >  > >
> > >  > > (no changes since v7)
> > >  > >
> > >  > > Changes in v7:
> > >  > > - Update commit message
> > >  > >
> > >  > >   lib/efi_loader/Kconfig   | 10 ++
> > >  > >   lib/efi_loader/Makefile  |  1 +
> > >  > >   lib/efi_loader/testapp.c | 68 +++
> > > +
> > >  > >   3 files changed, 79 insertions(+)
> > >  > >   create mode 100644 lib/efi_loader/testapp.c
> > >  > >
> > >  > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > >  > > index 69b2c9360d8..6ced29da719 100644
> > >  > > --- a/lib/efi_loader/Kconfig
> > >  > > +++ b/lib/efi_loader/Kconfig
> > >  > > @@ -565,6 +565,16 @@ config BOOTEFI_HELLO_COMPILE
> > >  > > No additional space will be required in the resulting U-
> > > Boot binary
> > >  > > when this option is enabled.
> > >  > >
> > >  > > +config BOOTEFI_TESTAPP_COMPILE
> > >  > > + bool "Compile an EFI test app for testing"
> > >  > > + default y
> > >  > > + help
> > >  > > +   This compiles an app designed for testing. It is packed
> > > into an image
> > >  > > +   by the test.py testing frame in the setup_efi_image() 
> > > function.
> > >  > > +
> > >  > > +   No additional space will be required in the resulting U-
> > > Boot binary
> > >  > > +   when this option is enabled.
> > >  > > +
> > >  > >   endif
> > >  > >
> > >  > >   source "lib/efi/Kconfig"
> > >  > > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> > >  > > index 00d18966f9e..87131ab911d 100644
> > >  > > --- a/lib/efi_loader/Makefile
> > >  > > +++ b/lib/efi_loader/Makefile
> > >  > > @@ -20,6 +20,7 @@ apps-$(CONFIG_EFI_LOAD_FILE2_INITRD) += initrddump
> > >  > >   ifeq ($(CONFIG_GENERATE_ACPI_TABLE),)
> > >  > >   apps-y += dtbdump
> > >  > >   endif
> > >  > > +apps-$(CONFIG_BOOTEFI_TESTAPP_COMPILE) += testapp
> > >  > >
> > >  > >   obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
> > >  > >   obj-$(CONFIG_EFI_BOOTMGR) += efi_bootmgr.o
> > >  > > diff --git a/lib/efi_loader/testapp.c b/lib/efi_loader/testapp.c
> > >  > > new file mode 100644
> > >  > > index 000..feb444c92e9
> > >  > > --- /dev/null
> > >  > > +++ b/lib/efi_loader/testapp.c
> > >  > > @@ -0,0 +1,68 @@
> > >  > > +// SPDX-License-Identifier: GPL-2.0+
> > >  >
> > >  > This is not a valid SPDX identifier. Please, use GPL-2.0-or-later.
> > >  >
> > >  > > +/*
> > >  > > + * Hello world EFI application
> > >  > > + *
> > >  > > + * Copyright 2024 Google LLC
> > >  > > + * Written by Simon Glass  > > >
> > >  > > + *
> > >  > > + * This test program is used to test the invocation of an EFI
> > > application.
> > >  > > + * It writes a few messages to the console and then exits boot
> > > services
> > >  > > + */
> > >  > > +
> > >  > > +#include 
> > >  > > +
> > >  > > +static const efi_guid_t loaded_image_guid =
> > > EFI_LOADED_IMAGE_PROTOCOL_GUID;
> > >  > > +
> > >  > > +static struct efi_system_table *systable;
> > >  > > +static struct efi_boot_services *boottime;
> > >  > > +static struct efi_simple_text_output_protocol *con_out;
> > >  > > +
> > >  > > +/**
> > >  > > + * efi_main() - entry point of the EFI application.
> > >  > > + *
> > >  > > + * @handle:  handle of the loaded image
> > >  > > + * @systab:  system table
> > >  > > + * Return:   status code
> > >  > > + */
> > >  > > +efi_status_t EFIAPI efi_main(efi_handle_t handle,
> > >  > > +  struct efi_system_table *systab)
> > >  > > +{
> > >  > > + struct efi_loaded_image *loaded_image;
> > >  > > + efi_status_t ret;
> > >  > > + efi_uintn_t map_size;
> > >  > > + efi_uintn_t map_key;
> > >  > > + efi_uintn_t desc_size;
> > >  > > + u32 desc_version;
> > >  > > +
> > >  > > 

Re: [PATCH v3 2/2] binman: expand test coverage to nxp_imx8mcst

[Fabio Estevam]

Hi Brian,

On Tue, Oct 29, 2024 at 12:52 PM Simon Glass  wrote:

> Could you please rebase on -master and resend?

Please copy me on v4, thanks.

Re: [PATCH 03/21] bloblist: test: doc: Move into the common suite

[Tom Rini]

On Tue, Oct 29, 2024 at 04:45:40PM +0100, Simon Glass wrote:
> Hi Tom,
> 
> On Mon, 28 Oct 2024 at 20:33, Tom Rini  wrote:
> >
> > On Mon, Oct 28, 2024 at 01:41:08PM +0100, Simon Glass wrote:
> >
> > > There is no particular need for bloblist to have its own test suite.
> > > Move it into the common suite instead.
> > >
> > > Add the missing help for 'common' and update the docs.
> > >
> > > Signed-off-by: Simon Glass 
> >
> > This feels both like churn and weird. I would think "common" tests are
> > for things that are kinda legacy and too catch-all to have their own
> > subgroup.
> 
> It's just chosen so that the test for common/ is in test/common - i.e.
> it makes it easier for people to find it.

Then I really don't understand renaming bloblist from something
descriptive to common which is not.

> BTW this series removes all tests from test/*.c and moves them into
> appropriate subdirs. Some of the tests are quite old so have just sat
> there all these years.

This is good, yes. But perhaps needs a bit more thought then about how
strong a mapping between filename/directory structure and test suite
name there needs to be.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH 2/4] board/BuR/common: add parameter for reset controller I2C bus selection

[Tom Rini]

On Tue, Oct 29, 2024 at 09:52:10AM +0100, Bernhard Messerklinger wrote:

> Normally B&R reset controllers are located at I2C bus 0. This patch adds
> the possibility to change this bus number with BR_RESETC_I2CBUS.
> 
> Signed-off-by: Bernhard Messerklinger 
> 
> ---
> 
>  board/BuR/common/br_resetc.c | 9 +++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/board/BuR/common/br_resetc.c b/board/BuR/common/br_resetc.c
> index f5d09fef3d..827ea6ba35 100644
> --- a/board/BuR/common/br_resetc.c
> +++ b/board/BuR/common/br_resetc.c
> @@ -52,10 +52,15 @@ static int resetc_init(void)
>  {
>   struct udevice *i2cbus;
>   int rc;
> +#if !defined(BR_RESETC_I2CBUS)
> + int busno = 0;
> +#else
> + int busno = CONFIG_BR_RESETC_I2CBUS;
> +#endif

You need to introduce board/BuR/common/Kconfig and add the symbol
BR_RESETC_I2CBUS there.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PULL] u-boot-sh/master

[Tom Rini]

On Tue, Oct 29, 2024 at 02:05:47AM +0100, Marek Vasut wrote:

> The following changes since commit 3df6145db0ed3430a2af089db5a82372bea3f4d5:
> 
>   x86: Missed removal of CMD_BOOTEFI_HELLO_COMPILE (2024-10-27 20:11:36 -0600)
> 
> are available in the Git repository at:
> 
>   https://source.denx.de/u-boot/custodians/u-boot-sh.git master
> 
> for you to fetch changes up to 400d06cb86a074dbd23cbb7f1210dad26f392807:
> 
>   mmc: renesas-sdhi: Add compatible string for rzg2l-sdhi (2024-10-28 
> 16:27:34 +0100)
> 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [GIT PULL] u-boot-riscv/master

[Tom Rini]

On Tue, Oct 29, 2024 at 08:33:54PM +0800, Leo Liang wrote:

> Hi Tom,
> 
> The following changes since commit bfdfc6c12e8ca68fff1a7ed3892c180143a6a0ef:
> 
>   Revert "acpi_table: Fix coverity defect in acpi_write_spcr" (2024-10-28 
> 20:53:34 -0600)
> 
> are available in the Git repository at:
> 
>   https://source.denx.de/u-boot/custodians/u-boot-riscv.git 
> 
> for you to fetch changes up to 239e4705099c7516f3d3cf958f3e540d635a4ed3:
> 
>   riscv: dts: mpfs: migrate to OF_UPSTREAM (2024-10-29 19:58:22 +0800)
> 
> CI result shows no issue: 
> https://source.denx.de/u-boot/custodians/u-boot-riscv/-/pipelines/23080
> 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v8 5/8] sandbox: Report host default-filename in native mode

[Tom Rini]

On Tue, Oct 29, 2024 at 04:45:54PM +0100, Simon Glass wrote:
> Hi Heinrich,
> 
> On Mon, 28 Oct 2024 at 20:16, Heinrich Schuchardt  wrote:
> >
> > On 10/25/24 11:56, Ilias Apalodimas wrote:
> > > Hi Simon,
> > >
> > >
> > > On Tue, 22 Oct 2024 at 15:00, Simon Glass  wrote:
> > >>
> > >> When the --native flag is given, pretend to be running the host
> > >> architecture rather than sandbox.
> > >>
> > >> Add an 'efidebug filename' command to report it.
> > >
> > > Heinrich does this allow you to continue your testing in native archs?
> > >
> > >>
> > >> Signed-off-by: Simon Glass 
> > >> ---
> > >>
> > >> Changes in v8:
> > >> - Add new patch to report host default-filename in native mode
> > >>
> > >>   boot/bootmeth_efi.c | 25 ++--
> > >>   boot/efi_fname.c| 57 +
> > >>   cmd/efidebug.c  | 25 
> > >>   include/efi.h   | 25 
> > >>   4 files changed, 94 insertions(+), 38 deletions(-)
> > >>
> > >> diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
> > >> index 371b36d550b..f836aa655f5 100644
> > >> --- a/boot/bootmeth_efi.c
> > >> +++ b/boot/bootmeth_efi.c
> > >> @@ -25,32 +25,11 @@
> > >>
> > >>   #define EFI_DIRNAME"/EFI/BOOT/"
> > >>
> > >> -static int get_efi_pxe_arch(void)
> > >> -{
> > >> -   /* 
> > >> http://www.iana.org/assignments/dhcpv6-parameters/dhcpv6-parameters.xml 
> > >> */
> > >> -   if (IS_ENABLED(CONFIG_ARM64))
> > >> -   return 0xb;
> > >> -   else if (IS_ENABLED(CONFIG_ARM))
> > >> -   return 0xa;
> > >> -   else if (IS_ENABLED(CONFIG_X86_64))
> > >> -   return 0x6;
> > >> -   else if (IS_ENABLED(CONFIG_X86))
> > >> -   return 0x7;
> > >> -   else if (IS_ENABLED(CONFIG_ARCH_RV32I))
> > >> -   return 0x19;
> > >> -   else if (IS_ENABLED(CONFIG_ARCH_RV64I))
> > >> -   return 0x1b;
> > >> -   else if (IS_ENABLED(CONFIG_SANDBOX))
> > >> -   return 0;   /* not used */
> > >> -
> > >> -   return -EINVAL;
> > >> -}
> > >> -
> > >>   static int get_efi_pxe_vci(char *str, int max_len)
> > >>   {
> > >>  int ret;
> > >>
> > >> -   ret = get_efi_pxe_arch();
> > >> +   ret = efi_get_pxe_arch();
> > >>  if (ret < 0)
> > >>  return ret;
> > >>
> > >> @@ -239,7 +218,7 @@ static int distro_efi_read_bootflow_net(struct 
> > >> bootflow *bflow)
> > >>  ret = get_efi_pxe_vci(str, sizeof(str));
> > >>  if (ret)
> > >>  return log_msg_ret("vci", ret);
> > >> -   ret = get_efi_pxe_arch();
> > >> +   ret = efi_get_pxe_arch();
> > >>  if (ret < 0)
> > >>  return log_msg_ret("arc", ret);
> > >>  arch = ret;
> > >> diff --git a/boot/efi_fname.c b/boot/efi_fname.c
> > >> index a6b11383bba..790f9e2fa36 100644
> > >> --- a/boot/efi_fname.c
> > >> +++ b/boot/efi_fname.c
> > >> @@ -9,29 +9,34 @@
> > >>*/
> > >>
> > >>   #include 
> > >> +#include 
> > >>   #include 
> > >>
> > >> -#ifdef CONFIG_SANDBOX
> > >> -
> > >>   #if HOST_ARCH == HOST_ARCH_X86_64
> > >> -#define BOOTEFI_NAME "BOOTX64.EFI"
> > >> +#define HOST_BOOTEFI_NAME "BOOTX64.EFI"
> > >> +#define HOST_PXE_ARCH 0x6
> > >>   #elif HOST_ARCH == HOST_ARCH_X86
> > >> -#define BOOTEFI_NAME "BOOTIA32.EFI"
> > >> +#define HOST_BOOTEFI_NAME "BOOTIA32.EFI"
> > >> +#define HOST_PXE_ARCH 0x7
> > >>   #elif HOST_ARCH == HOST_ARCH_AARCH64
> > >> -#define BOOTEFI_NAME "BOOTAA64.EFI"
> > >> +#define HOST_BOOTEFI_NAME "BOOTAA64.EFI"
> > >> +#define HOST_PXE_ARCH 0xb
> > >>   #elif HOST_ARCH == HOST_ARCH_ARM
> > >> -#define BOOTEFI_NAME "BOOTARM.EFI"
> > >> +#define HOST_BOOTEFI_NAME "BOOTARM.EFI"
> > >> +#define HOST_PXE_ARCH 0xa
> > >>   #elif HOST_ARCH == HOST_ARCH_RISCV32
> > >> -#define BOOTEFI_NAME "BOOTRISCV32.EFI"
> > >> +#define HOST_BOOTEFI_NAME "BOOTRISCV32.EFI"
> > >> +#define HOST_PXE_ARCH 0x19
> > >>   #elif HOST_ARCH == HOST_ARCH_RISCV64
> > >> -#define BOOTEFI_NAME "BOOTRISCV64.EFI"
> > >> +#define HOST_BOOTEFI_NAME "BOOTRISCV64.EFI"
> > >> +#define HOST_PXE_ARCH 0x1b
> > >>   #else
> > >> -#error Unsupported UEFI architecture
> > >> +#error Unsupported Host architecture
> > >>   #endif
> > >>
> > >> -#else
> > >> -
> > >> -#if defined(CONFIG_ARM64)
> > >> +#if defined(CONFIG_SANDBOX)
> > >> +#define BOOTEFI_NAME "BOOTSBOX.EFI"
> > >> +#elif defined(CONFIG_ARM64)
> > >>   #define BOOTEFI_NAME "BOOTAA64.EFI"
> > >>   #elif defined(CONFIG_ARM)
> > >>   #define BOOTEFI_NAME "BOOTARM.EFI"
> > >> @@ -47,9 +52,31 @@
> > >>   #error Unsupported UEFI architecture
> >  >>   #endif>>
> > >> -#endif
> > >> -
> > >>   const char *efi_get_basename(void)
> > >>   {
> > >> -   return BOOTEFI_NAME;
> > >> +   return efi_use_host_arch() ? HOST_BOOTEFI_NAME : BOOTEFI_NAME;
> > >> +}
> > >> +
> > >> +int efi_get_pxe_arch(void)
> > >> +{
> > >> +   if (efi_use_host_arch())
> > >> +   return HOST_PX

[PATCH v9 02/11] efi_loader: Add a test app

[Simon Glass]

Add a simple app to use for testing. This is intended to do whatever it
needs to for testing purposes. For now it just prints a message and
exits boot services.

There was a considerable amount of discussion about whether it is OK to
call exit-boot-services and then return to U-Boot. This is not normally
done in a real application, since exit-boot-services is used to
completely disconnect from U-Boot. However, since this is a test, we
need to check the results of running the app, so returning is necessary.
It works correctly and it provides a nice model of how to test the EFI
code in a simple way.

Signed-off-by: Simon Glass 
---

Changes in v9:
- Update license
- Fix 'sevices' typo

Changes in v7:
- Update commit message

 lib/efi_loader/Kconfig   | 10 ++
 lib/efi_loader/Makefile  |  1 +
 lib/efi_loader/testapp.c | 68 
 3 files changed, 79 insertions(+)
 create mode 100644 lib/efi_loader/testapp.c

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 37572c82f54..44319729bda 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -584,6 +584,16 @@ config BOOTEFI_HELLO_COMPILE
  No additional space will be required in the resulting U-Boot binary
  when this option is enabled.
 
+config BOOTEFI_TESTAPP_COMPILE
+   bool "Compile an EFI test app for testing"
+   default y
+   help
+ This compiles an app designed for testing. It is packed into an image
+ by the test.py testing frame in the setup_efi_image() function.
+
+ No additional space will be required in the resulting U-Boot binary
+ when this option is enabled.
+
 endif
 
 source "lib/efi/Kconfig"
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index 8ec240cb864..7c8b2dd1ad6 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -20,6 +20,7 @@ apps-$(CONFIG_EFI_LOAD_FILE2_INITRD) += initrddump
 ifeq ($(CONFIG_GENERATE_ACPI_TABLE),)
 apps-y += dtbdump
 endif
+apps-$(CONFIG_BOOTEFI_TESTAPP_COMPILE) += testapp
 
 obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
 obj-$(CONFIG_EFI_BOOTMGR) += efi_bootmgr.o
diff --git a/lib/efi_loader/testapp.c b/lib/efi_loader/testapp.c
new file mode 100644
index 000..a28fc17ce5b
--- /dev/null
+++ b/lib/efi_loader/testapp.c
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Hello world EFI application
+ *
+ * Copyright 2024 Google LLC
+ * Written by Simon Glass 
+ *
+ * This test program is used to test the invocation of an EFI application.
+ * It writes a few messages to the console and then exits boot services
+ */
+
+#include 
+
+static const efi_guid_t loaded_image_guid = EFI_LOADED_IMAGE_PROTOCOL_GUID;
+
+static struct efi_system_table *systable;
+static struct efi_boot_services *boottime;
+static struct efi_simple_text_output_protocol *con_out;
+
+/**
+ * efi_main() - entry point of the EFI application.
+ *
+ * @handle:handle of the loaded image
+ * @systab:system table
+ * Return: status code
+ */
+efi_status_t EFIAPI efi_main(efi_handle_t handle,
+struct efi_system_table *systab)
+{
+   struct efi_loaded_image *loaded_image;
+   efi_status_t ret;
+   efi_uintn_t map_size;
+   efi_uintn_t map_key;
+   efi_uintn_t desc_size;
+   u32 desc_version;
+
+   systable = systab;
+   boottime = systable->boottime;
+   con_out = systable->con_out;
+
+   /* Get the loaded image protocol */
+   ret = boottime->open_protocol(handle, &loaded_image_guid,
+ (void **)&loaded_image, NULL, NULL,
+ EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+   if (ret != EFI_SUCCESS) {
+   con_out->output_string
+   (con_out, u"Cannot open loaded image protocol\r\n");
+   goto out;
+   }
+
+   /* UEFI requires CR LF */
+   con_out->output_string(con_out, u"U-Boot test app for EFI_LOADER\r\n");
+
+out:
+   map_size = 0;
+   ret = boottime->get_memory_map(&map_size, NULL, &map_key, &desc_size,
+  &desc_version);
+   con_out->output_string(con_out, u"Exiting boot services\n");
+
+   /* exit boot services so that this part of U-Boot can be tested */
+   boottime->exit_boot_services(handle, map_key);
+
+   /* now exit for real */
+   ret = boottime->exit(handle, ret, 0, NULL);
+
+   /* We should never arrive here */
+   return ret;
+}
-- 
2.43.0

[PATCH v9 00/11] efi: Add a test for EFI bootmeth

[Simon Glass]

The test coverage for the EFI bootmeth is incomplete since it does not
actually boot the application.

This series creates a simple test for this purpose. It includes a few
patches to make this work:

- Revert of an unreviewed patch to change the sandbox efi filename
- Hang in sandbox virtio due to EFI probing all block devices

Other necessary fixes have already been applied.

Changes in v9:
- Update license
- Fix 'sevices' typo
- Move the function into efi_helper.c
- Split out into a separate change
- Split out into its own patch
- Separate into separate patches
- Add new patch to drop sandbox PXE architecture
- Mark the image as complete after writing it
- Fix 'sevices' typo

Changes in v8:
- Add new patch to control on-host behaviour
- Add new patch to move default filename to a function
- Add new patch to report host default-filename in native mode
- Add new patch to report host default-filename in native mode

Changes in v7:
- Update commit message
- Drop patches already applied
- Drop patch 'Disable ANSI output for tests'
- Rebase on -master

Changes in v6:
- Drop the patch to disable sandbox virtio blk with EFI
- Add new patch to disable the sandbox virtio blk device
- Deal with sandbox CONFIG_LOGF_FUNC
- Rebase on -next
- Drop patches previously applied
- Drop mention of helloworld since it is no-longer used by this test

Changes in v4:
- Add efi_loader tag to some patches
- Split out non-EFI patches into a different series

Changes in v2:
- Add many new patches to resolve all the outstanding test issues

Simon Glass (11):
  test: boot: Update bootflow_iter() for console checking
  efi_loader: Add a test app
  sandbox: Add a -N flag to control on-host behaviour
  efi: Move default filename to a function
  efi_loader: Move get_efi_pxe_arch() to efi_helper
  efi_loader: Allow reporting the host defaults
  sandbox: Report host default-filename in native mode
  efi_loader: Drop sandbox PXE architecture
  sandbox: virtio: Disable the sandbox virtio blk device
  test: efi: boot: Set up an image suitable for EFI testing
  test: efi: boot: Add a test for the efi bootmeth

 arch/Kconfig|   3 +-
 arch/sandbox/cpu/start.c|  10 
 arch/sandbox/dts/test.dts   |   2 +-
 arch/sandbox/include/asm/state.h|   1 +
 boot/bootmeth_efi.c |  29 ++--
 cmd/efidebug.c  |  25 ++
 include/efi.h   |  34 +
 include/efi_default_filename.h  |  56 --
 lib/efi_loader/Kconfig  |  10 
 lib/efi_loader/Makefile |   1 +
 lib/efi_loader/efi_bootmgr.c|  10 ++--
 lib/efi_loader/efi_helper.c |  71 
 lib/efi_loader/testapp.c|  68 ++
 test/boot/bootdev.c |  18 ++-
 test/boot/bootflow.c|  71 ++--
 test/py/tests/bootstd/flash1.img.xz | Bin 0 -> 4980 bytes
 test/py/tests/test_ut.py|  53 ++---
 17 files changed, 364 insertions(+), 98 deletions(-)
 delete mode 100644 include/efi_default_filename.h
 create mode 100644 lib/efi_loader/testapp.c
 create mode 100644 test/py/tests/bootstd/flash1.img.xz

-- 
2.43.0

[PATCH v9 03/11] sandbox: Add a -N flag to control on-host behaviour

[Simon Glass]

Sandbox is its own architecture, but sometimes we want to mimic the host
architecture, e.g. when running an EFI app not built by U-Boot.

Add a -N/--native flag which tells sandbox to reflect the architecture
of the host.

Signed-off-by: Simon Glass 
---

(no changes since v8)

Changes in v8:
- Add new patch to control on-host behaviour

 arch/sandbox/cpu/start.c | 10 ++
 arch/sandbox/include/asm/state.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c
index 81752edc9f8..2940768cd1c 100644
--- a/arch/sandbox/cpu/start.c
+++ b/arch/sandbox/cpu/start.c
@@ -449,6 +449,16 @@ static void setup_ram_buf(struct sandbox_state *state)
gd->ram_size = state->ram_size;
 }
 
+static int sandbox_cmdline_cb_native(struct sandbox_state *state,
+const char *arg)
+{
+   state->native = true;
+
+   return 0;
+}
+SANDBOX_CMDLINE_OPT_SHORT(native, 'N', 0,
+ "Use native mode (host-based EFI boot filename)");
+
 void state_show(struct sandbox_state *state)
 {
char **p;
diff --git a/arch/sandbox/include/asm/state.h b/arch/sandbox/include/asm/state.h
index e7dc01759e8..dc21a623106 100644
--- a/arch/sandbox/include/asm/state.h
+++ b/arch/sandbox/include/asm/state.h
@@ -101,6 +101,7 @@ struct sandbox_state {
bool disable_eth;   /* Disable Ethernet devices */
bool disable_sf_bootdevs;   /* Don't bind SPI flash bootdevs */
bool upl;   /* Enable Universal Payload (UPL) */
+   bool native;/* Adjust to reflect host arch */
 
/* Pointer to information for each SPI bus/cs */
struct sandbox_spi_info spi[CONFIG_SANDBOX_SPI_MAX_BUS]
-- 
2.43.0

[PATCH v9 01/11] test: boot: Update bootflow_iter() for console checking

[Simon Glass]

This test checks console output so should have the UTF_CONSOLE flag. Add
it.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 test/boot/bootflow.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index 0d4e966892e..841729bd1ac 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -370,7 +370,7 @@ static int bootflow_iter(struct unit_test_state *uts)
 
return 0;
 }
-BOOTSTD_TEST(bootflow_iter, UTF_DM | UTF_SCAN_FDT);
+BOOTSTD_TEST(bootflow_iter, UTF_DM | UTF_SCAN_FDT | UTF_CONSOLE);
 
 #if defined(CONFIG_SANDBOX) && defined(CONFIG_BOOTMETH_GLOBAL)
 /* Check using the system bootdev */
-- 
2.43.0

Re: [PATCH v1 0/1] bootcount: zynqmp: Add bootcount API

[Tom Rini]

On Tue, Oct 29, 2024 at 07:58:13PM +0100, Vasileios Amoiridis wrote:
> From: Vasileios Amoiridis 
> 
> Add support for the bootcount API by utilizing the internal Persistent
> Global General Registers of the PMU. These registers get reset whenever
> there is a Power-On-Reset so it makes them suitable for the API.
> 
> Questions towards reviewers:
> 
>   1) Is the licensing of the file ok?

Yes.

>   2) How am I supposed to add a maintainer for this file?

This should be caught already under the zynqmp regex in the top-level
MAINTAINERS file.

The code itself looks fine, but I'll leave it to Michal to further
review.

-- 
Tom


signature.asc
Description: PGP signature

[PATCH v9 06/11] efi_loader: Allow reporting the host defaults

[Simon Glass]

Add an 'efidebug filename' command to report the default filename and
PXE architecture.

Signed-off-by: Simon Glass 
---

Changes in v9:
- Split out into its own patch

Changes in v8:
- Add new patch to report host default-filename in native mode

 cmd/efidebug.c | 25 +
 1 file changed, 25 insertions(+)

diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index bba984b2b75..ff6d118876d 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -536,6 +536,27 @@ static int do_efi_show_log(struct cmd_tbl *cmdtp, int flag,
return CMD_RET_FAILURE;
}
 
+   return 0;
+}
+
+/**
+ * do_efi_show_defaults() - show UEFI default filename and PXE architecture
+ *
+ * @cmdtp: Command table
+ * @flag:  Command flag
+ * @argc:  Number of arguments
+ * @argv:  Argument array
+ * Return: CMD_RET_SUCCESS on success, CMD_RET_RET_FAILURE on failure
+ *
+ * Implement efidebug "defaults" sub-command.
+ * Shows the default EFI filename and PXE architecture
+ */
+static int do_efi_show_defaults(struct cmd_tbl *cmdtp, int flag,
+   int argc, char *const argv[])
+{
+   printf("Default boot path: EFI\\BOOT\\%s\n", efi_get_basename());
+   printf("PXE arch: 0x%02x\n", efi_get_pxe_arch());
+
return CMD_RET_SUCCESS;
 }
 
@@ -1589,6 +1610,8 @@ static struct cmd_tbl cmd_efidebug_sub[] = {
 "", ""),
U_BOOT_CMD_MKENT(dh, CONFIG_SYS_MAXARGS, 1, do_efi_show_handles,
 "", ""),
+   U_BOOT_CMD_MKENT(defaults, CONFIG_SYS_MAXARGS, 1, do_efi_show_defaults,
+"", ""),
U_BOOT_CMD_MKENT(images, CONFIG_SYS_MAXARGS, 1, do_efi_show_images,
 "", ""),
U_BOOT_CMD_MKENT(log, CONFIG_SYS_MAXARGS, 1, do_efi_show_log, "", ""),
@@ -1688,6 +1711,8 @@ U_BOOT_LONGHELP(efidebug,
"  - show UEFI drivers\n"
"efidebug dh\n"
"  - show UEFI handles\n"
+   "efidebug defaults\n"
+   "  - show default EFI filename and PXE architecture\n"
"efidebug images\n"
"  - show loaded images\n"
"efidebug log\n"
-- 
2.43.0

[PATCH v9 04/11] efi: Move default filename to a function

[Simon Glass]

Use a function to obtain the device EFI filename, so that we can control
how sandbox behaves.

Signed-off-by: Simon Glass 
---

Changes in v9:
- Move the function into efi_helper.c

Changes in v8:
- Add new patch to move default filename to a function

 boot/bootmeth_efi.c|  4 +--
 include/efi.h  |  9 ++
 include/efi_default_filename.h | 56 --
 lib/efi_loader/efi_bootmgr.c   | 10 --
 lib/efi_loader/efi_helper.c| 45 +++
 test/boot/bootflow.c   |  7 +++--
 6 files changed, 67 insertions(+), 64 deletions(-)
 delete mode 100644 include/efi_default_filename.h

diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
index 2ad6d3b4ace..371b36d550b 100644
--- a/boot/bootmeth_efi.c
+++ b/boot/bootmeth_efi.c
@@ -13,7 +13,7 @@
 #include 
 #include 
 #include 
-#include 
+#include 
 #include 
 #include 
 #include 
@@ -168,7 +168,7 @@ static int distro_efi_try_bootflow_files(struct udevice 
*dev,
}
 
strcpy(fname, EFI_DIRNAME);
-   strcat(fname, BOOTEFI_NAME);
+   strcat(fname, efi_get_basename());
 
if (bflow->blk)
 desc = dev_get_uclass_plat(bflow->blk);
diff --git a/include/efi.h b/include/efi.h
index f7419b80d4d..fef88e9c5d6 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -670,4 +670,13 @@ int efi_get_mmap(struct efi_mem_desc **descp, int *sizep, 
uint *keyp,
  */
 void efi_show_tables(struct efi_system_table *systab);
 
+/**
+ * efi_get_basename() - Get the default filename to use when loading
+ *
+ * E.g. this function returns BOOTAA64.EFI for an aarch target
+ *
+ * Return: Default EFI filename
+ */
+const char *efi_get_basename(void);
+
 #endif /* _LINUX_EFI_H */
diff --git a/include/efi_default_filename.h b/include/efi_default_filename.h
deleted file mode 100644
index 77932984b55..000
--- a/include/efi_default_filename.h
+++ /dev/null
@@ -1,56 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
-/*
- * When a boot option does not provide a file path the EFI file to be
- * booted is \EFI\BOOT\$(BOOTEFI_NAME).EFI. The architecture specific
- * file name is defined in this include.
- *
- * Copyright (c) 2022, Heinrich Schuchardt 
- * Copyright (c) 2022, Linaro Limited
- */
-
-#ifndef _EFI_DEFAULT_FILENAME_H
-#define _EFI_DEFAULT_FILENAME_H
-
-#include 
-
-#undef BOOTEFI_NAME
-
-#ifdef CONFIG_SANDBOX
-
-#if HOST_ARCH == HOST_ARCH_X86_64
-#define BOOTEFI_NAME "BOOTX64.EFI"
-#elif HOST_ARCH == HOST_ARCH_X86
-#define BOOTEFI_NAME "BOOTIA32.EFI"
-#elif HOST_ARCH == HOST_ARCH_AARCH64
-#define BOOTEFI_NAME "BOOTAA64.EFI"
-#elif HOST_ARCH == HOST_ARCH_ARM
-#define BOOTEFI_NAME "BOOTARM.EFI"
-#elif HOST_ARCH == HOST_ARCH_RISCV32
-#define BOOTEFI_NAME "BOOTRISCV32.EFI"
-#elif HOST_ARCH == HOST_ARCH_RISCV64
-#define BOOTEFI_NAME "BOOTRISCV64.EFI"
-#else
-#error Unsupported UEFI architecture
-#endif
-
-#else
-
-#if defined(CONFIG_ARM64)
-#define BOOTEFI_NAME "BOOTAA64.EFI"
-#elif defined(CONFIG_ARM)
-#define BOOTEFI_NAME "BOOTARM.EFI"
-#elif defined(CONFIG_X86_64)
-#define BOOTEFI_NAME "BOOTX64.EFI"
-#elif defined(CONFIG_X86)
-#define BOOTEFI_NAME "BOOTIA32.EFI"
-#elif defined(CONFIG_ARCH_RV32I)
-#define BOOTEFI_NAME "BOOTRISCV32.EFI"
-#elif defined(CONFIG_ARCH_RV64I)
-#define BOOTEFI_NAME "BOOTRISCV64.EFI"
-#else
-#error Unsupported UEFI architecture
-#endif
-
-#endif
-
-#endif
diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index 431a38704e9..2ea20909491 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -11,10 +11,10 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
-#include 
 #include 
 #include 
 #include 
@@ -82,8 +82,12 @@ struct efi_device_path *expand_media_path(struct 
efi_device_path *device_path)
 &efi_simple_file_system_protocol_guid, &rem);
if (handle) {
if (rem->type == DEVICE_PATH_TYPE_END) {
-   full_path = efi_dp_from_file(device_path,
-"/EFI/BOOT/" BOOTEFI_NAME);
+   char fname[30];
+
+   snprintf(fname, sizeof(fname), "/EFI/BOOT/%s",
+efi_get_basename());
+   full_path = efi_dp_from_file(device_path, fname);
+
} else {
full_path = efi_dp_dup(device_path);
}
diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c
index 00167bd2a10..51e0c4852c5 100644
--- a/lib/efi_loader/efi_helper.c
+++ b/lib/efi_loader/efi_helper.c
@@ -12,18 +12,63 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
+#ifdef CONFIG_SANDBOX
+
+#if HOST_ARCH == HOST_ARCH_X86_64
+#define BOOTEFI_NAME "BOOTX64.EFI"
+#elif HOST_ARCH == HOST_ARCH_X86
+#define BOOTEFI_NAME "BOOTIA32.EFI"
+#elif HOST_ARCH == HOST_ARCH_AARCH64
+#define BOOTEFI_NAME "B

[PATCH v9 10/11] test: efi: boot: Set up an image suitable for EFI testing

[Simon Glass]

Create a new disk for use with tests, which contains the new 'testapp'
EFI app specifically intended for testing the EFI loader.

Attach it to the USB device, since most testing is currently done with
mmc.

Initially this image will be used to test the EFI bootmeth.

Fix a stale comment in prep_mmc_bootdev() while we are here.

For now this uses sudo and a compressed fallback file, like all the
other bootstd tests. Once this series is in, the patch which moves
this to use user-space tools will be cleaned up and re-submitted.

Signed-off-by: Simon Glass 

---
Here is the patch to avoid sudo and CI fallback:

[1] https://patchwork.ozlabs.org/project/uboot/patch/
20240802093322.15240-1-rich...@nod.at/

Changes in v9:
- Mark the image as complete after writing it

 arch/sandbox/dts/test.dts   |   2 +-
 test/boot/bootdev.c |  18 +-
 test/boot/bootflow.c|   2 +-
 test/py/tests/bootstd/flash1.img.xz | Bin 0 -> 4980 bytes
 test/py/tests/test_ut.py|  53 
 5 files changed, 66 insertions(+), 9 deletions(-)
 create mode 100644 test/py/tests/bootstd/flash1.img.xz

diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index 3017b33d67b..dee280184b1 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -1515,7 +1515,7 @@
flash-stick@1 {
reg = <1>;
compatible = "sandbox,usb-flash";
-   sandbox,filepath = "testflash1.bin";
+   sandbox,filepath = "flash1.img";
};
 
flash-stick@2 {
diff --git a/test/boot/bootdev.c b/test/boot/bootdev.c
index 369105ca4cf..c892854b227 100644
--- a/test/boot/bootdev.c
+++ b/test/boot/bootdev.c
@@ -221,6 +221,10 @@ static int bootdev_test_order(struct unit_test_state *uts)
/* Use the environment variable to override it */
ut_assertok(env_set("boot_targets", "mmc1 mmc2 usb"));
ut_assertok(bootflow_scan_first(NULL, NULL, &iter, 0, &bflow));
+
+   /* get the usb device which has a backing file (flash1.img) */
+   ut_asserteq(0, bootflow_scan_next(&iter, &bflow));
+
ut_asserteq(-ENODEV, bootflow_scan_next(&iter, &bflow));
ut_asserteq(5, iter.num_devs);
ut_asserteq_str("mmc1.bootdev", iter.dev_used[0]->name);
@@ -260,7 +264,11 @@ static int bootdev_test_order(struct unit_test_state *uts)
ut_assertok(bootflow_scan_first(NULL, NULL, &iter, 0, &bflow));
ut_asserteq(2, iter.num_devs);
 
-   /* Now scan past mmc1 and make sure that the 3 USB devices show up */
+   /*
+* Now scan past mmc1 and make sure that the 3 USB devices show up. The
+* first one has a backing file so returns success
+*/
+   ut_asserteq(0, bootflow_scan_next(&iter, &bflow));
ut_asserteq(-ENODEV, bootflow_scan_next(&iter, &bflow));
ut_asserteq(6, iter.num_devs);
ut_asserteq_str("mmc2.bootdev", iter.dev_used[0]->name);
@@ -322,6 +330,10 @@ static int bootdev_test_prio(struct unit_test_state *uts)
 
/* 3 MMC and 3 USB bootdevs: MMC should come before USB */
ut_assertok(bootflow_scan_first(NULL, NULL, &iter, 0, &bflow));
+
+   /* get the usb device which has a backing file (flash1.img) */
+   ut_asserteq(0, bootflow_scan_next(&iter, &bflow));
+
ut_asserteq(-ENODEV, bootflow_scan_next(&iter, &bflow));
ut_asserteq(6, iter.num_devs);
ut_asserteq_str("mmc2.bootdev", iter.dev_used[0]->name);
@@ -339,6 +351,10 @@ static int bootdev_test_prio(struct unit_test_state *uts)
bootflow_iter_uninit(&iter);
ut_assertok(bootflow_scan_first(NULL, NULL, &iter, BOOTFLOWIF_HUNT,
&bflow));
+
+   /* get the usb device which has a backing file (flash1.img) */
+   ut_asserteq(0, bootflow_scan_next(&iter, &bflow));
+
ut_asserteq(-ENODEV, bootflow_scan_next(&iter, &bflow));
ut_asserteq(7, iter.num_devs);
ut_asserteq_str("usb_mass_storage.lun0.bootdev",
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index 7cfb217088e..d7e94c8cc59 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -534,7 +534,7 @@ static int prep_mmc_bootdev(struct unit_test_state *uts, 
const char *mmc_dev,
 
order[2] = mmc_dev;
 
-   /* Enable the mmc4 node since we need a second bootflow */
+   /* Enable the requested mmc node since we need a second bootflow */
root = oftree_root(oftree_default());
node = ofnode_find_subnode(root, mmc_dev);
ut_assert(ofnode_valid(node));
diff --git a/test/py/tests/bootstd/flash1.img.xz 
b/test/py/tests/bootstd/flash1.img.xz
new file mode 100644
index 
..0efd9cbe146db6e8f422d9d66f11a772a493ce29
GIT binary patch
literal 4980
zcmeI0=Tj4i7R3

[PATCH v9 07/11] sandbox: Report host default-filename in native mode

[Simon Glass]

When the --native flag is given, pretend to be running the host
architecture rather than sandbox.

Allow the same control for PXE too.

Signed-off-by: Simon Glass 
---

Changes in v9:
- Separate into separate patches

Changes in v8:
- Add new patch to report host default-filename in native mode

 include/efi.h   | 15 +++
 lib/efi_loader/efi_helper.c | 35 ---
 2 files changed, 35 insertions(+), 15 deletions(-)

diff --git a/include/efi.h b/include/efi.h
index 633f5b4acb5..817bc56c835 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -679,6 +679,21 @@ void efi_show_tables(struct efi_system_table *systab);
  */
 const char *efi_get_basename(void);
 
+#ifdef CONFIG_SANDBOX
+#include 
+#endif
+
+static inline bool efi_use_host_arch(void)
+{
+#ifdef CONFIG_SANDBOX
+   struct sandbox_state *state = state_get_current();
+
+   return state->native;
+#else
+   return false;
+#endif
+}
+
 /**
  * efi_get_pxe_arch() - Get the architecture value for PXE
  *
diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c
index b8ece1c2e0c..ab5678eb66c 100644
--- a/lib/efi_loader/efi_helper.c
+++ b/lib/efi_loader/efi_helper.c
@@ -23,27 +23,31 @@
 
 #undef BOOTEFI_NAME
 
-#ifdef CONFIG_SANDBOX
-
 #if HOST_ARCH == HOST_ARCH_X86_64
-#define BOOTEFI_NAME "BOOTX64.EFI"
+#define HOST_BOOTEFI_NAME "BOOTX64.EFI"
+#define HOST_PXE_ARCH 0x6
 #elif HOST_ARCH == HOST_ARCH_X86
-#define BOOTEFI_NAME "BOOTIA32.EFI"
+#define HOST_BOOTEFI_NAME "BOOTIA32.EFI"
+#define HOST_PXE_ARCH 0x7
 #elif HOST_ARCH == HOST_ARCH_AARCH64
-#define BOOTEFI_NAME "BOOTAA64.EFI"
+#define HOST_BOOTEFI_NAME "BOOTAA64.EFI"
+#define HOST_PXE_ARCH 0xb
 #elif HOST_ARCH == HOST_ARCH_ARM
-#define BOOTEFI_NAME "BOOTARM.EFI"
+#define HOST_BOOTEFI_NAME "BOOTARM.EFI"
+#define HOST_PXE_ARCH 0xa
 #elif HOST_ARCH == HOST_ARCH_RISCV32
-#define BOOTEFI_NAME "BOOTRISCV32.EFI"
+#define HOST_BOOTEFI_NAME "BOOTRISCV32.EFI"
+#define HOST_PXE_ARCH 0x19
 #elif HOST_ARCH == HOST_ARCH_RISCV64
-#define BOOTEFI_NAME "BOOTRISCV64.EFI"
+#define HOST_BOOTEFI_NAME "BOOTRISCV64.EFI"
+#define HOST_PXE_ARCH 0x1b
 #else
-#error Unsupported UEFI architecture
+#error Unsupported Host architecture
 #endif
 
-#else
-
-#if defined(CONFIG_ARM64)
+#if defined(CONFIG_SANDBOX)
+#define BOOTEFI_NAME "BOOTSBOX.EFI"
+#elif defined(CONFIG_ARM64)
 #define BOOTEFI_NAME "BOOTAA64.EFI"
 #elif defined(CONFIG_ARM)
 #define BOOTEFI_NAME "BOOTARM.EFI"
@@ -59,8 +63,6 @@
 #error Unsupported UEFI architecture
 #endif
 
-#endif
-
 #if defined(CONFIG_CMD_EFIDEBUG) || defined(CONFIG_EFI_LOAD_FILE2_INITRD)
 /* GUID used by Linux to identify the LoadFile2 protocol with the initrd */
 const efi_guid_t efi_lf2_initrd_guid = EFI_INITRD_MEDIA_GUID;
@@ -68,11 +70,14 @@ const efi_guid_t efi_lf2_initrd_guid = 
EFI_INITRD_MEDIA_GUID;
 
 const char *efi_get_basename(void)
 {
-   return BOOTEFI_NAME;
+   return efi_use_host_arch() ? HOST_BOOTEFI_NAME : BOOTEFI_NAME;
 }
 
 int efi_get_pxe_arch(void)
 {
+   if (efi_use_host_arch())
+   return HOST_PXE_ARCH;
+
/* 
http://www.iana.org/assignments/dhcpv6-parameters/dhcpv6-parameters.xml */
if (IS_ENABLED(CONFIG_ARM64))
return 0xb;
-- 
2.43.0

[PATCH v9 05/11] efi_loader: Move get_efi_pxe_arch() to efi_helper

[Simon Glass]

Move this function from the EFI bootmeth to the common efi_helper file.
No functional change is intended.

Signed-off-by: Simon Glass 
---

Changes in v9:
- Split out into a separate change

 boot/bootmeth_efi.c | 25 ++---
 include/efi.h   | 10 ++
 lib/efi_loader/efi_helper.c | 23 +++
 3 files changed, 35 insertions(+), 23 deletions(-)

diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
index 371b36d550b..f836aa655f5 100644
--- a/boot/bootmeth_efi.c
+++ b/boot/bootmeth_efi.c
@@ -25,32 +25,11 @@
 
 #define EFI_DIRNAME"/EFI/BOOT/"
 
-static int get_efi_pxe_arch(void)
-{
-   /* 
http://www.iana.org/assignments/dhcpv6-parameters/dhcpv6-parameters.xml */
-   if (IS_ENABLED(CONFIG_ARM64))
-   return 0xb;
-   else if (IS_ENABLED(CONFIG_ARM))
-   return 0xa;
-   else if (IS_ENABLED(CONFIG_X86_64))
-   return 0x6;
-   else if (IS_ENABLED(CONFIG_X86))
-   return 0x7;
-   else if (IS_ENABLED(CONFIG_ARCH_RV32I))
-   return 0x19;
-   else if (IS_ENABLED(CONFIG_ARCH_RV64I))
-   return 0x1b;
-   else if (IS_ENABLED(CONFIG_SANDBOX))
-   return 0;   /* not used */
-
-   return -EINVAL;
-}
-
 static int get_efi_pxe_vci(char *str, int max_len)
 {
int ret;
 
-   ret = get_efi_pxe_arch();
+   ret = efi_get_pxe_arch();
if (ret < 0)
return ret;
 
@@ -239,7 +218,7 @@ static int distro_efi_read_bootflow_net(struct bootflow 
*bflow)
ret = get_efi_pxe_vci(str, sizeof(str));
if (ret)
return log_msg_ret("vci", ret);
-   ret = get_efi_pxe_arch();
+   ret = efi_get_pxe_arch();
if (ret < 0)
return log_msg_ret("arc", ret);
arch = ret;
diff --git a/include/efi.h b/include/efi.h
index fef88e9c5d6..633f5b4acb5 100644
--- a/include/efi.h
+++ b/include/efi.h
@@ -679,4 +679,14 @@ void efi_show_tables(struct efi_system_table *systab);
  */
 const char *efi_get_basename(void);
 
+/**
+ * efi_get_pxe_arch() - Get the architecture value for PXE
+ *
+ * See:
+ * http://www.iana.org/assignments/dhcpv6-parameters/dhcpv6-parameters.xml
+ *
+ * Return: Architecture value
+ */
+int efi_get_pxe_arch(void);
+
 #endif /* _LINUX_EFI_H */
diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c
index 51e0c4852c5..b8ece1c2e0c 100644
--- a/lib/efi_loader/efi_helper.c
+++ b/lib/efi_loader/efi_helper.c
@@ -21,6 +21,8 @@
 #include 
 #include 
 
+#undef BOOTEFI_NAME
+
 #ifdef CONFIG_SANDBOX
 
 #if HOST_ARCH == HOST_ARCH_X86_64
@@ -69,6 +71,27 @@ const char *efi_get_basename(void)
return BOOTEFI_NAME;
 }
 
+int efi_get_pxe_arch(void)
+{
+   /* 
http://www.iana.org/assignments/dhcpv6-parameters/dhcpv6-parameters.xml */
+   if (IS_ENABLED(CONFIG_ARM64))
+   return 0xb;
+   else if (IS_ENABLED(CONFIG_ARM))
+   return 0xa;
+   else if (IS_ENABLED(CONFIG_X86_64))
+   return 0x6;
+   else if (IS_ENABLED(CONFIG_X86))
+   return 0x7;
+   else if (IS_ENABLED(CONFIG_ARCH_RV32I))
+   return 0x19;
+   else if (IS_ENABLED(CONFIG_ARCH_RV64I))
+   return 0x1b;
+   else if (IS_ENABLED(CONFIG_SANDBOX))
+   return 0;   /* not used */
+
+   return -EINVAL;
+}
+
 /**
  * efi_create_current_boot_var() - Return Boot name were  is replaced 
by
  *the value of BootCurrent
-- 
2.43.0

[PATCH v9 09/11] sandbox: virtio: Disable the sandbox virtio blk device

[Simon Glass]

This is not implemented so cannot actually be used to read blocks.
Disable it until it is implemented, to avoid causing a hang with EFI,
which probes every available BLK device.

Signed-off-by: Simon Glass 
Reviewed-by: Tom Rini 
Issue: https://source.denx.de/u-boot/u-boot/-/issues/37
---

(no changes since v6)

Changes in v6:
- Drop the patch to disable sandbox virtio blk with EFI
- Add new patch to disable the sandbox virtio blk device

 arch/Kconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/Kconfig b/arch/Kconfig
index 8f1f4667012..c39efb4d0a2 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -212,7 +212,8 @@ config SANDBOX
imply VIRTIO_MMIO
imply VIRTIO_PCI
imply VIRTIO_SANDBOX
-   imply VIRTIO_BLK
+   # Re-enable this when fully implemented
+   # imply VIRTIO_BLK
imply VIRTIO_NET
imply DM_SOUND
imply PCI_SANDBOX_EP
-- 
2.43.0

[PATCH v9 08/11] efi_loader: Drop sandbox PXE architecture

[Simon Glass]

Rather than returning 0, just return an error, since sandbox is not used
with PXE at present.

Signed-off-by: Simon Glass 
---

Changes in v9:
- Add new patch to drop sandbox PXE architecture

 lib/efi_loader/efi_helper.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c
index ab5678eb66c..bf96f61d3d0 100644
--- a/lib/efi_loader/efi_helper.c
+++ b/lib/efi_loader/efi_helper.c
@@ -91,8 +91,6 @@ int efi_get_pxe_arch(void)
return 0x19;
else if (IS_ENABLED(CONFIG_ARCH_RV64I))
return 0x1b;
-   else if (IS_ENABLED(CONFIG_SANDBOX))
-   return 0;   /* not used */
 
return -EINVAL;
 }
-- 
2.43.0

[PATCH v1 0/1] bootcount: zynqmp: Add bootcount API

[Vasileios Amoiridis]

From: Vasileios Amoiridis 

Add support for the bootcount API by utilizing the internal Persistent
Global General Registers of the PMU. These registers get reset whenever
there is a Power-On-Reset so it makes them suitable for the API.

Questions towards reviewers:

1) Is the licensing of the file ok?
2) How am I supposed to add a maintainer for this file?
3) Is this the correct place for this macro? (Maybe this one
   goes more towards Michal.)

Cheers,
Vasilis

Vasileios Amoiridis (1):
  drivers: bootcount: Add ZynqMP specific bootcount support

 drivers/bootcount/Kconfig|  4 
 drivers/bootcount/Makefile   |  1 +
 drivers/bootcount/bootcount_zynqmp.c | 28 
 3 files changed, 33 insertions(+)
 create mode 100644 drivers/bootcount/bootcount_zynqmp.c


base-commit: 3df6145db0ed3430a2af089db5a82372bea3f4d5
-- 
2.34.1

Re: [PATCH 06/15] sandbox: Add a way to show the sandbox memory-mapping

[Simon Glass]

Hi Ilias,

On Tue, 29 Oct 2024 at 10:59, Ilias Apalodimas
 wrote:
>
> Hi Simon
>
> This seems completely unrelated to the series.
>
> Please send it as a separate patch

I've moved the initial patches in this series to my queue in patchwork, for now.

Regards,
Simon


>
> Thanks
> /Ilias
>
> On Mon, 28 Oct 2024 at 14:48, Simon Glass  wrote:
> >
> > This is mostly hidden in the background, but it is sometimes useful to
> > look at it. Add a function to allow this.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> >  arch/sandbox/cpu/cpu.c | 13 +
> >  arch/sandbox/include/asm/cpu.h |  3 +++
> >  cmd/sb.c   | 11 +++
> >  doc/usage/cmd/sb.rst   | 25 +
> >  4 files changed, 52 insertions(+)
> >
> > diff --git a/arch/sandbox/cpu/cpu.c b/arch/sandbox/cpu/cpu.c
> > index 06f8c13fab9..d1c4dcf0764 100644
> > --- a/arch/sandbox/cpu/cpu.c
> > +++ b/arch/sandbox/cpu/cpu.c
> > @@ -253,6 +253,19 @@ phys_addr_t map_to_sysmem(const void *ptr)
> > return mentry->tag;
> >  }
> >
> > +void sandbox_map_list(void)
> > +{
> > +   struct sandbox_mapmem_entry *mentry;
> > +   struct sandbox_state *state = state_get_current();
> > +
> > +   printf("Sandbox memory-mapping\n");
> > +   printf("%8s  %16s  %6s\n", "Addr", "Mapping", "Refcnt");
> > +   list_for_each_entry(mentry, &state->mapmem_head, sibling_node) {
> > +   printf("%8lx  %p  %6d\n", mentry->tag, mentry->ptr,
> > +  mentry->refcnt);
> > +   }
> > +}
> > +
> >  unsigned long sandbox_read(const void *addr, enum sandboxio_size_t size)
> >  {
> > struct sandbox_state *state = state_get_current();
> > diff --git a/arch/sandbox/include/asm/cpu.h b/arch/sandbox/include/asm/cpu.h
> > index c97ac7ba95b..682bb3376d1 100644
> > --- a/arch/sandbox/include/asm/cpu.h
> > +++ b/arch/sandbox/include/asm/cpu.h
> > @@ -8,4 +8,7 @@
> >
> >  void cpu_sandbox_set_current(const char *name);
> >
> > +/* show the mapping of sandbox addresses to pointers */
> > +void sandbox_map_list(void);
> > +
> >  #endif /* __SANDBOX_CPU_H */
> > diff --git a/cmd/sb.c b/cmd/sb.c
> > index 9dbb53275b3..9245052492e 100644
> > --- a/cmd/sb.c
> > +++ b/cmd/sb.c
> > @@ -7,6 +7,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >
> > @@ -29,6 +30,14 @@ static int do_sb_handoff(struct cmd_tbl *cmdtp, int 
> > flag, int argc,
> >  #endif
> >  }
> >
> > +static int do_sb_map(struct cmd_tbl *cmdtp, int flag, int argc,
> > +char *const argv[])
> > +{
> > +   sandbox_map_list();
> > +
> > +   return 0;
> > +}
> > +
> >  static int do_sb_state(struct cmd_tbl *cmdtp, int flag, int argc,
> >char *const argv[])
> >  {
> > @@ -42,8 +51,10 @@ static int do_sb_state(struct cmd_tbl *cmdtp, int flag, 
> > int argc,
> >
> >  U_BOOT_LONGHELP(sb,
> > "handoff - Show handoff data received from SPL\n"
> > +   "sb map - Show mapped memory\n"
> > "sb state   - Show sandbox state");
> >
> >  U_BOOT_CMD_WITH_SUBCMDS(sb, "Sandbox status commands", sb_help_text,
> > U_BOOT_SUBCMD_MKENT(handoff, 1, 1, do_sb_handoff),
> > +   U_BOOT_SUBCMD_MKENT(map, 1, 1, do_sb_map),
> > U_BOOT_SUBCMD_MKENT(state, 1, 1, do_sb_state));
> > diff --git a/doc/usage/cmd/sb.rst b/doc/usage/cmd/sb.rst
> > index 6f54f9d9eb7..37431aff7c8 100644
> > --- a/doc/usage/cmd/sb.rst
> > +++ b/doc/usage/cmd/sb.rst
> > @@ -12,6 +12,7 @@ Synopsis
> >  ::
> >
> >  sb handoff
> > +sb map
> >  sb state
> >
> >  Description
> > @@ -26,6 +27,24 @@ sb handoff
> >  This shows information about any handoff information received from SPL. If
> >  U-Boot is started from an SPL build, it shows a valid magic number.
> >
> > +sb map
> > +~~
> > +
> > +This shows any mappings between sandbox's emulated RAM and the underlying 
> > host
> > +address-space.
> > +
> > +Fields shown are:
> > +
> > +Addr
> > +Address in emulated RAM
> > +
> > +Mapping
> > +Equivalent address in the host address-space. While sandbox requests 
> > address
> > +``0x1000`` from the OS, this is not always available.
> > +
> > +Refcnt
> > +Shows the number of references to this mapping.
> > +
> >  sb state
> >  
> >
> > @@ -42,6 +61,12 @@ as ``sandbox_spl``::
> >  => sb handoff
> >  SPL handoff magic 14f93c7b
> >
> > +This shows output from the *sb map* subcommand, with a single mapping::
> > +
> > +Sandbox memory-mapping
> > +Addr   Mapping  Refcnt
> > +ff00  56185b46d6d0   2
> > +
> >  This shows output from the *sb state* subcommand::
> >
> >  => sb state
> > --
> > 2.43.0
> >

Re: [PATCH v2 6/8] armv8: Add generic smbios information into the device tree

[Simon Glass]

On Mon, 28 Oct 2024 at 21:00, Raymond Mao  wrote:
>
> Hi Simon,
>
> On Mon, 28 Oct 2024 at 13:05, Simon Glass  wrote:
>>
>> Hi Raymond,
>>
>> On Tue, 22 Oct 2024 at 22:07, Raymond Mao  wrote:
>> >
>> > Add common smbios information that can be used by all armv8
>> > platforms and set it as default for qemu-arm64.
>> > From now smbios library can load values from here for those fields
>> > doesn't exist in the sysinfo driver.
>> >
>> > To run this with QEMU arm64, please dump the generated DTB
>> > from QEMU first, merge it with the one we build and then re-run
>> > QEMU with the merged DTB.
>> > ```
>> > qemu-system-arm -machine virt -machine dumpdtb=qemu.dtb
>> > cat  <(dtc -I dtb qemu.dtb) <(dtc -I dtb ./dts/dt.dtb | \
>> >   grep -v /dts-v1/) | dtc - -o merged.dtb
>> > qemu-system-arm -machine virt -nographic -bios u-boot.bin \
>> >   -dtb merged.dtb
>>
>> and please talk to Peter about accepting my patch[1]
>>
>>
>> > ```
>> > For details please take reference on dt_qemu.rst
>> >
>> > Signed-off-by: Raymond Mao 
>> > ---
>> > Changes in v2
>> > - Initial patch.
>> >
>> >  arch/arm/dts/qemu-arm64.dts  |  4 ++
>> >  arch/arm/dts/smbios_generic.dtsi | 82 
>> >  2 files changed, 86 insertions(+)
>> >  create mode 100644 arch/arm/dts/smbios_generic.dtsi
>> >
>> > diff --git a/arch/arm/dts/qemu-arm64.dts b/arch/arm/dts/qemu-arm64.dts
>> > index 096b3910728..95fcf53ed74 100644
>> > --- a/arch/arm/dts/qemu-arm64.dts
>> > +++ b/arch/arm/dts/qemu-arm64.dts
>> > @@ -7,5 +7,9 @@
>> >
>> >  /dts-v1/;
>> >
>> > +#if defined(CONFIG_SYSINFO_SMBIOS) && !defined(QFW_SMBIOS)
>> > +#include "smbios_generic.dtsi"
>> > +#endif
>> > +
>> >  / {
>> >  };
>> > diff --git a/arch/arm/dts/smbios_generic.dtsi 
>> > b/arch/arm/dts/smbios_generic.dtsi
>> > new file mode 100644
>> > index 000..c9f07283403
>> > --- /dev/null
>> > +++ b/arch/arm/dts/smbios_generic.dtsi
>> > @@ -0,0 +1,82 @@
>> > +// SPDX-License-Identifier: GPL-2.0+
>> > +/*
>> > + * Default SMBIOS information for Arm64 platforms
>> > + *
>> > + * Copyright (c) 2024 Linaro Limited
>> > + * Author: Raymond Mao 
>> > + */
>> > +#include 
>> > +#include 
>> > +
>> > +/ {
>> > +   smbios {
>> > +   compatible = "u-boot,sysinfo-smbios";
>> > +
>> > +   smbios {
>> > +   system {
>> > +   manufacturer = CONFIG_SYS_VENDOR;
>> > +   product = CONFIG_SYS_BOARD;
>> > +   version = "Not Specified";
>> > +   serial = "Not Specified";
>>
>> Does it make sense to add these strings? Perhaps if the property is missing 
>> in the DT, it should be missing in the SMBIOS table? Is 'Not Specified' a 
>> special string in the SMBIO spec?
>
>
> This is not defined in the spec and actually those strings can be anything.
> As currently this is for qemu_arm64 only and we don't have real 
> vendor-defined values,
> "Not Specified" is used as a placeholder to show an example for other vendors 
> if they
> want to add similar properties.

OK I see.

Reviewed-by: Simon Glass 

Re: [PATCH 09/15] efi_loader: Fix free in ..._media_device_boot_option()

[Simon Glass]

Hi Ilias,

On Tue, 29 Oct 2024 at 11:02, Ilias Apalodimas
 wrote:
>
> Hi Simon,
>
> On Mon, 28 Oct 2024 at 14:48, Simon Glass  wrote:
> >
> > Freeing a NULL pointer is an error in EFI, so check the pointer first,
> > before freeing it.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> >  lib/efi_loader/efi_bootmgr.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> > index a3aa2b8d1b9..431a38704e9 100644
> > --- a/lib/efi_loader/efi_bootmgr.c
> > +++ b/lib/efi_loader/efi_bootmgr.c
> > @@ -1180,7 +1180,8 @@ out:
> > free(opt[i].lo);
> > }
> > free(opt);
> > -   efi_free_pool(handles);
> > +   if (handles)
> > +   efi_free_pool(handles);
>
> We don't need this, efi_free_pool() checks the pointer already.

Yes, but it then returns an error (EFI_INVALID_PARAMETER), which gets
logged, with this series.

Regards,
Simon

Re: [PATCH v8 5/8] sandbox: Report host default-filename in native mode

[Simon Glass]

Hi Heinrich,

On Mon, 28 Oct 2024 at 20:16, Heinrich Schuchardt  wrote:
>
> On 10/25/24 11:56, Ilias Apalodimas wrote:
> > Hi Simon,
> >
> >
> > On Tue, 22 Oct 2024 at 15:00, Simon Glass  wrote:
> >>
> >> When the --native flag is given, pretend to be running the host
> >> architecture rather than sandbox.
> >>
> >> Add an 'efidebug filename' command to report it.
> >
> > Heinrich does this allow you to continue your testing in native archs?
> >
> >>
> >> Signed-off-by: Simon Glass 
> >> ---
> >>
> >> Changes in v8:
> >> - Add new patch to report host default-filename in native mode
> >>
> >>   boot/bootmeth_efi.c | 25 ++--
> >>   boot/efi_fname.c| 57 +
> >>   cmd/efidebug.c  | 25 
> >>   include/efi.h   | 25 
> >>   4 files changed, 94 insertions(+), 38 deletions(-)
> >>
> >> diff --git a/boot/bootmeth_efi.c b/boot/bootmeth_efi.c
> >> index 371b36d550b..f836aa655f5 100644
> >> --- a/boot/bootmeth_efi.c
> >> +++ b/boot/bootmeth_efi.c
> >> @@ -25,32 +25,11 @@
> >>
> >>   #define EFI_DIRNAME"/EFI/BOOT/"
> >>
> >> -static int get_efi_pxe_arch(void)
> >> -{
> >> -   /* 
> >> http://www.iana.org/assignments/dhcpv6-parameters/dhcpv6-parameters.xml */
> >> -   if (IS_ENABLED(CONFIG_ARM64))
> >> -   return 0xb;
> >> -   else if (IS_ENABLED(CONFIG_ARM))
> >> -   return 0xa;
> >> -   else if (IS_ENABLED(CONFIG_X86_64))
> >> -   return 0x6;
> >> -   else if (IS_ENABLED(CONFIG_X86))
> >> -   return 0x7;
> >> -   else if (IS_ENABLED(CONFIG_ARCH_RV32I))
> >> -   return 0x19;
> >> -   else if (IS_ENABLED(CONFIG_ARCH_RV64I))
> >> -   return 0x1b;
> >> -   else if (IS_ENABLED(CONFIG_SANDBOX))
> >> -   return 0;   /* not used */
> >> -
> >> -   return -EINVAL;
> >> -}
> >> -
> >>   static int get_efi_pxe_vci(char *str, int max_len)
> >>   {
> >>  int ret;
> >>
> >> -   ret = get_efi_pxe_arch();
> >> +   ret = efi_get_pxe_arch();
> >>  if (ret < 0)
> >>  return ret;
> >>
> >> @@ -239,7 +218,7 @@ static int distro_efi_read_bootflow_net(struct 
> >> bootflow *bflow)
> >>  ret = get_efi_pxe_vci(str, sizeof(str));
> >>  if (ret)
> >>  return log_msg_ret("vci", ret);
> >> -   ret = get_efi_pxe_arch();
> >> +   ret = efi_get_pxe_arch();
> >>  if (ret < 0)
> >>  return log_msg_ret("arc", ret);
> >>  arch = ret;
> >> diff --git a/boot/efi_fname.c b/boot/efi_fname.c
> >> index a6b11383bba..790f9e2fa36 100644
> >> --- a/boot/efi_fname.c
> >> +++ b/boot/efi_fname.c
> >> @@ -9,29 +9,34 @@
> >>*/
> >>
> >>   #include 
> >> +#include 
> >>   #include 
> >>
> >> -#ifdef CONFIG_SANDBOX
> >> -
> >>   #if HOST_ARCH == HOST_ARCH_X86_64
> >> -#define BOOTEFI_NAME "BOOTX64.EFI"
> >> +#define HOST_BOOTEFI_NAME "BOOTX64.EFI"
> >> +#define HOST_PXE_ARCH 0x6
> >>   #elif HOST_ARCH == HOST_ARCH_X86
> >> -#define BOOTEFI_NAME "BOOTIA32.EFI"
> >> +#define HOST_BOOTEFI_NAME "BOOTIA32.EFI"
> >> +#define HOST_PXE_ARCH 0x7
> >>   #elif HOST_ARCH == HOST_ARCH_AARCH64
> >> -#define BOOTEFI_NAME "BOOTAA64.EFI"
> >> +#define HOST_BOOTEFI_NAME "BOOTAA64.EFI"
> >> +#define HOST_PXE_ARCH 0xb
> >>   #elif HOST_ARCH == HOST_ARCH_ARM
> >> -#define BOOTEFI_NAME "BOOTARM.EFI"
> >> +#define HOST_BOOTEFI_NAME "BOOTARM.EFI"
> >> +#define HOST_PXE_ARCH 0xa
> >>   #elif HOST_ARCH == HOST_ARCH_RISCV32
> >> -#define BOOTEFI_NAME "BOOTRISCV32.EFI"
> >> +#define HOST_BOOTEFI_NAME "BOOTRISCV32.EFI"
> >> +#define HOST_PXE_ARCH 0x19
> >>   #elif HOST_ARCH == HOST_ARCH_RISCV64
> >> -#define BOOTEFI_NAME "BOOTRISCV64.EFI"
> >> +#define HOST_BOOTEFI_NAME "BOOTRISCV64.EFI"
> >> +#define HOST_PXE_ARCH 0x1b
> >>   #else
> >> -#error Unsupported UEFI architecture
> >> +#error Unsupported Host architecture
> >>   #endif
> >>
> >> -#else
> >> -
> >> -#if defined(CONFIG_ARM64)
> >> +#if defined(CONFIG_SANDBOX)
> >> +#define BOOTEFI_NAME "BOOTSBOX.EFI"
> >> +#elif defined(CONFIG_ARM64)
> >>   #define BOOTEFI_NAME "BOOTAA64.EFI"
> >>   #elif defined(CONFIG_ARM)
> >>   #define BOOTEFI_NAME "BOOTARM.EFI"
> >> @@ -47,9 +52,31 @@
> >>   #error Unsupported UEFI architecture
>  >>   #endif>>
> >> -#endif
> >> -
> >>   const char *efi_get_basename(void)
> >>   {
> >> -   return BOOTEFI_NAME;
> >> +   return efi_use_host_arch() ? HOST_BOOTEFI_NAME : BOOTEFI_NAME;
> >> +}
> >> +
> >> +int efi_get_pxe_arch(void)
> >> +{
> >> +   if (efi_use_host_arch())
> >> +   return HOST_PXE_ARCH;
> >> +
> >> +   /* 
> >> http://www.iana.org/assignments/dhcpv6-parameters/dhcpv6-parameters.xml */
> >> +   if (IS_ENABLED(CONFIG_ARM64))
> >> +   return 0xb;
> >> +   else if (IS_ENABLED(CONFIG_ARM))
> >> +   return 0xa;
> >> +   else if (IS_ENABLED(CONFIG_X86_64))
> >> +   return 0x6;
> >> +

Re: [PATCH] menu: fix the logic checking whether ESC key is pressed

[高惟杰]

On Mon, 2024-10-28 at 18:01 +0100, Simon Glass wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> Hi Weijie,
> 
> On Mon, 28 Oct 2024 at 03:58, Weijie Gao 
> wrote:
> >
> > On Mon, 2024-10-28 at 03:33 +0100, Marek Vasut wrote:
> > > External email : Please do not click links or open attachments
> until
> > > you have verified the sender or the content.
> > >
> > >
> > > On 10/28/24 1:25 AM, Weijie Gao wrote:
> > > > Hi Daniel,
> > > >
> > > > On Sat, 2024-10-26 at 20:23 +0100, Daniel Golle wrote:
> > > > > External email : Please do not click links or open
> attachments
> > > > > until
> > > > > you have verified the sender or the content.
> > > > >
> > > > >
> > > > > Hi!
> > > > >
> > > > > On Mon, Oct 21, 2024 at 08:56:42AM +0800, Weijie Gao wrote:
> > > > > > It's observed that the bootmenu on a serial console
> sometimes
> > > > > > incorrectly quitted with superfluous characters filled to
> > > > > > command
> > > > > > line input:
> > > > > >
> > > > > > >   *** U-Boot Boot Menu ***
> > > > > > >
> > > > > > >   1. Startup system (Default)
> > > > > > >   2. Upgrade firmware
> > > > > > >   3. Upgrade ATF BL2
> > > > > > >   4. Upgrade ATF FIP
> > > > > > >   5. Load image
> > > > > > >   0. U-Boot console
> > > > > > >
> > > > > > >
> > > > > > >   Press UP/DOWN to move, ENTER to select, ESC to quit
> > > > > > > MT7988> [B
> > > > > >
> > > > > > Analysis shows it was caused by the wrong logic of
> > > > > > bootmenu_loop:
> > > > > >
> > > > > > At first the bootmenu_loop received the first ESC char
> > > > > > correctly.
> > > > > >
> > > > > > However, during the second call to bootmenu_loop, there's
> no
> > > > > > data
> > > > > > in the UART Rx FIFO. Due to the low baudrate, the second
> char
> > > > > > of
> > > > > > the down array key sequence hasn't be fully received.
> > > > > >
> > > > > > But bootmenu_loop just did a mdelay(10), and then treated
> it as
> > > > > > a
> > > > > > single ESC key press event. It didn't even try tstc() again
> > > > > > after
> > > > > > the 10ms timeout.
> > > > > >
> > > > > > This patch fixes this issue by letting bootmenu_loop check
> > > > > > tstc()
> > > > > > twice.
> > > > >
> > > > > The patch helps to fix the problem when dealing with single
> key
> > > > > presses. However, apparently this is not enough to fix the
> issue
> > > > > in
> > > > > all situations. I still manage to drop into U-Boot console
> with
> > > > > an
> > > > > extra '[A' when simply holding down the arrow-up key for a
> while.
> > > > >
> > > >
> > > > Holding down the arrow-up key for a while can cause the PC to
> send
> > > > large amounts of data to the UART Rx FIFO. Since filogic UARTs
> > > > usually
> > > > have only 32 bytes FIFO, it's very easy to cause Rx FIFO
> overflow,
> > > > and then you'll still be dropped into U-Boot console due to
> data
> > > > loss.
> > > > I've already confirmed this during solving this issue and this
> can
> > > > only
> > > > be solved by increasing the baudrate.
> > >
> > > Does CONFIG_SERIAL_RX_BUFFER help with those overflows ?
> >
> > CONFIG_SERIAL_RX_BUFFER can't fully elimate the overflow. I can
> still
> > reproduce this issue by holding down the arrow-up key for more than
> > 15 seconds.
> 
> Is this due to serial, or the delays caused by drawing to the
> display?
> 
> If the serial buffer is always full, one option might be to detect
> this (tstc() returning non-zero 10 times in a row) and discard all
> output until a key is not pressed for 200ms or so.
> 

I can confirm that the input data grows faster that the consumption
from bootmenu (mainly due to the menu being repainted too frequently).
So no matter how large the buffer is, FIFO overrun will eventually
happen.

A long time key (up/down) press will change the active menu item to
either the first or the last one, and then the menu does not need a
reprint.

So I decide to add an extra function for the common menu part to checkif the 
menu needs reprint.
> Regards,
> Simon

Re: [RFC PATCH 2/2] apple: dart: use driver specific instance of LMB

[Janne Grunau]

Hej,

On Tue, Oct 29, 2024 at 12:48:46PM +0530, Sughosh Ganu wrote:
> The LMB module is typically used for managing the platform's RAM
> memory. RAM memory gets added to the module's memory map, and
> subsequently allocated through various LMB API's.
> 
> The IOMMU driver for the apple platforms also uses the LMB API's for
> allocating device virtual addresses(VA). These addresses are different
> from the RAM addresses, and cannot be added to the RAM memory map. Add
> a separate instance of LMB memory map for the device VA's, which gets
> managed by the IOMMU driver. Use lmb_push() and lmb_pop() functions to
> set-up the relevant lmb instance.

thanks, this fixes the initial brokenness when setting up dma mappings
but I still see SError due to translation fault. I don't have time right
now to look into it so it could be unrelated to the iommu.

> Signed-off-by: Sughosh Ganu 
> ---
>  drivers/iommu/apple_dart.c | 67 +-
>  include/lmb.h  |  2 ++
>  lib/lmb.c  |  1 -
>  3 files changed, 68 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/apple_dart.c b/drivers/iommu/apple_dart.c
> index 611ac7cd6de..55f60287b0e 100644
> --- a/drivers/iommu/apple_dart.c
> +++ b/drivers/iommu/apple_dart.c
> @@ -3,6 +3,7 @@
>   * Copyright (C) 2021 Mark Kettenis 
>   */
>  
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -70,6 +71,8 @@
>  
>  struct apple_dart_priv {
>   void *base;
> + struct lmb apple_dart_lmb;
> + struct lmb ram_lmb;
>   u64 *l1, *l2;
>   int bypass, shift;
>  
> @@ -87,6 +90,56 @@ struct apple_dart_priv {
>   void (*flush_tlb)(struct apple_dart_priv *priv);
>  };
>  
> +static int apple_dart_lmb_init(struct apple_dart_priv *priv)
> +{
> + bool ret;
> + struct lmb *almb = &priv->apple_dart_lmb;
> +
> + ret = alist_init(&almb->free_mem, sizeof(struct lmb_region),
> +  (uint)LMB_ALIST_INITIAL_SIZE);
> + if (!ret) {
> + log_debug("Unable to initialise the list for LMB free 
> memory\n");
> + return -ENOMEM;
> + }
> +
> + ret = alist_init(&almb->used_mem, sizeof(struct lmb_region),
> +  (uint)LMB_ALIST_INITIAL_SIZE);
> + if (!ret) {
> + log_debug("Unable to initialise the list for LMB used 
> memory\n");
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> +static void apple_dart_lmb_uninit(struct apple_dart_priv *priv)
> +{
> + struct lmb *almb = &priv->apple_dart_lmb;
> +
> + alist_uninit(&almb->free_mem);
> + alist_uninit(&almb->used_mem);
> +}
> +
> +static void apple_dart_lmb_setup(struct apple_dart_priv *priv)
> +{
> + struct lmb *almb = &priv->apple_dart_lmb;
> + struct lmb *rlmb = &priv->ram_lmb;
> +
> + lmb_push(rlmb);
> +
> + lmb_pop(almb);

This doesn't look look like a good solution to me. We're conflating two
different concepts into the global lmb. This looks error prone to me. I
was looking into adding iomb_* entry points into the lmb points which
take a pointer.

Janne

[RFC PATCH 2/2] apple: dart: use driver specific instance of LMB

[Sughosh Ganu]

The LMB module is typically used for managing the platform's RAM
memory. RAM memory gets added to the module's memory map, and
subsequently allocated through various LMB API's.

The IOMMU driver for the apple platforms also uses the LMB API's for
allocating device virtual addresses(VA). These addresses are different
from the RAM addresses, and cannot be added to the RAM memory map. Add
a separate instance of LMB memory map for the device VA's, which gets
managed by the IOMMU driver. Use lmb_push() and lmb_pop() functions to
set-up the relevant lmb instance.

Signed-off-by: Sughosh Ganu 
---
 drivers/iommu/apple_dart.c | 67 +-
 include/lmb.h  |  2 ++
 lib/lmb.c  |  1 -
 3 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/apple_dart.c b/drivers/iommu/apple_dart.c
index 611ac7cd6de..55f60287b0e 100644
--- a/drivers/iommu/apple_dart.c
+++ b/drivers/iommu/apple_dart.c
@@ -3,6 +3,7 @@
  * Copyright (C) 2021 Mark Kettenis 
  */
 
+#include 
 #include 
 #include 
 #include 
@@ -70,6 +71,8 @@
 
 struct apple_dart_priv {
void *base;
+   struct lmb apple_dart_lmb;
+   struct lmb ram_lmb;
u64 *l1, *l2;
int bypass, shift;
 
@@ -87,6 +90,56 @@ struct apple_dart_priv {
void (*flush_tlb)(struct apple_dart_priv *priv);
 };
 
+static int apple_dart_lmb_init(struct apple_dart_priv *priv)
+{
+   bool ret;
+   struct lmb *almb = &priv->apple_dart_lmb;
+
+   ret = alist_init(&almb->free_mem, sizeof(struct lmb_region),
+(uint)LMB_ALIST_INITIAL_SIZE);
+   if (!ret) {
+   log_debug("Unable to initialise the list for LMB free 
memory\n");
+   return -ENOMEM;
+   }
+
+   ret = alist_init(&almb->used_mem, sizeof(struct lmb_region),
+(uint)LMB_ALIST_INITIAL_SIZE);
+   if (!ret) {
+   log_debug("Unable to initialise the list for LMB used 
memory\n");
+   return -ENOMEM;
+   }
+
+   return 0;
+}
+
+static void apple_dart_lmb_uninit(struct apple_dart_priv *priv)
+{
+   struct lmb *almb = &priv->apple_dart_lmb;
+
+   alist_uninit(&almb->free_mem);
+   alist_uninit(&almb->used_mem);
+}
+
+static void apple_dart_lmb_setup(struct apple_dart_priv *priv)
+{
+   struct lmb *almb = &priv->apple_dart_lmb;
+   struct lmb *rlmb = &priv->ram_lmb;
+
+   lmb_push(rlmb);
+
+   lmb_pop(almb);
+}
+
+static void apple_dart_lmb_pop(struct apple_dart_priv *priv)
+{
+   struct lmb *almb = &priv->apple_dart_lmb;
+   struct lmb *rlmb = &priv->ram_lmb;
+
+   lmb_push(almb);
+
+   lmb_pop(rlmb);
+}
+
 static void apple_dart_t8020_flush_tlb(struct apple_dart_priv *priv)
 {
dsb();
@@ -123,7 +176,9 @@ static dma_addr_t apple_dart_map(struct udevice *dev, void 
*addr, size_t size)
off = (phys_addr_t)addr - paddr;
psize = ALIGN(size + off, DART_PAGE_SIZE);
 
+   apple_dart_lmb_setup(priv);
dva = lmb_alloc(psize, DART_PAGE_SIZE);
+   apple_dart_lmb_pop(priv);
 
idx = dva / DART_PAGE_SIZE;
for (i = 0; i < psize / DART_PAGE_SIZE; i++) {
@@ -159,7 +214,9 @@ static void apple_dart_unmap(struct udevice *dev, 
dma_addr_t addr, size_t size)
   (unsigned long)&priv->l2[idx + i]);
priv->flush_tlb(priv);
 
+   apple_dart_lmb_setup(priv);
lmb_free(dva, psize);
+   apple_dart_lmb_pop(priv);
 }
 
 static struct iommu_ops apple_dart_ops = {
@@ -173,7 +230,7 @@ static int apple_dart_probe(struct udevice *dev)
dma_addr_t addr;
phys_addr_t l2;
int ntte, nl1, nl2;
-   int sid, i;
+   int sid, i, ret;
u32 params2, params4;
 
priv->base = dev_read_addr_ptr(dev);
@@ -212,7 +269,13 @@ static int apple_dart_probe(struct udevice *dev)
priv->dvabase = DART_PAGE_SIZE;
priv->dvaend = SZ_4G - DART_PAGE_SIZE;
 
+   ret = apple_dart_lmb_init(priv);
+   if (ret)
+   return ret;
+
+   apple_dart_lmb_setup(priv);
lmb_add(priv->dvabase, priv->dvaend - priv->dvabase);
+   apple_dart_lmb_pop(priv);
 
/* Disable translations. */
for (sid = 0; sid < priv->nsid; sid++)
@@ -294,6 +357,8 @@ static int apple_dart_remove(struct udevice *dev)
}
priv->flush_tlb(priv);
 
+   apple_dart_lmb_uninit(priv);
+
return 0;
 }
 
diff --git a/include/lmb.h b/include/lmb.h
index 72d7093023a..a39b48a9a97 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -14,6 +14,8 @@
  * Copyright (C) 2001 Peter Bergner, IBM Corp.
  */
 
+#define LMB_ALIST_INITIAL_SIZE 4
+
 /**
  * enum lmb_flags - definition of memory region attributes
  * @LMB_NONE: no special request
diff --git a/lib/lmb.c b/lib/lmb.c
index 2960c05abfc..959236f6e5a 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -28,7 +28,6 @@ DECLARE_GLOBAL_DATA_PTR;
 #define MAP_OP_ADD (u8)0x3
 
 #define LMB_ALLOC_ANYWHERE 0
-#defin

[RFC PATCH 0/2] apple: dart: Add driver specific instance of LMB

[Sughosh Ganu]

Sughosh Ganu (2):
  lmb: refactor lmb push and pop functions
  apple: dart: use driver specific instance of LMB

 drivers/iommu/apple_dart.c | 67 +-
 include/lmb.h  |  6 +++-
 lib/lmb.c  | 21 +---
 test/lib/lmb.c | 18 +-
 4 files changed, 96 insertions(+), 16 deletions(-)

-- 
2.34.1

Re: [RFC PATCH] apple: dart: add logic to allocate dva addresses

[Sughosh Ganu]

On Tue, 29 Oct 2024 at 09:09, Sughosh Ganu  wrote:
>
> The Apple IOMMU driver uses LMB API's to allocate virtual addresses
> for IO devices. These virtual addresses fall in the first 4GB address
> range. Currently, the driver obtains these virtual addresses through
> the LMB API's. This no longer works with the global LMB map. Add a
> function apple_dart_dvaalloc() which mimics what was being done by the
> LMB allocation function.
>
> Signed-off-by: Sughosh Ganu 
> ---
>
> Note: I have only build tested this on the m1 defconfig.

NAK. To be discarded.

>
>  drivers/iommu/apple_dart.c | 24 ++--
>  1 file changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/apple_dart.c b/drivers/iommu/apple_dart.c
> index 611ac7cd6de..8e2acf50a06 100644
> --- a/drivers/iommu/apple_dart.c
> +++ b/drivers/iommu/apple_dart.c
> @@ -6,7 +6,6 @@
>  #include 
>  #include 
>  #include 
> -#include 
>  #include 
>  #include 
>
> @@ -75,6 +74,7 @@ struct apple_dart_priv {
>
> dma_addr_t dvabase;
> dma_addr_t dvaend;
> +   dma_addr_t dvaalloc;
>
> int nsid;
> int nttbr;
> @@ -109,6 +109,21 @@ static void apple_dart_t8110_flush_tlb(struct 
> apple_dart_priv *priv)
> continue;
>  }
>
> +static dma_addr_t apple_dart_dvaalloc(struct udevice *dev, phys_size_t size)
> +{
> +   dma_addr_t dva;
> +   struct apple_dart_priv *priv = dev_get_priv(dev);
> +
> +   dva = priv->dvaalloc - size;
> +   dva &= ~(DART_PAGE_SIZE - 1);
> +
> +   assert(dva > priv->dvabase);
> +
> +   priv->dvaalloc = dva;
> +
> +   return priv->dvaalloc;
> +}
> +
>  static dma_addr_t apple_dart_map(struct udevice *dev, void *addr, size_t 
> size)
>  {
> struct apple_dart_priv *priv = dev_get_priv(dev);
> @@ -123,7 +138,7 @@ static dma_addr_t apple_dart_map(struct udevice *dev, 
> void *addr, size_t size)
> off = (phys_addr_t)addr - paddr;
> psize = ALIGN(size + off, DART_PAGE_SIZE);
>
> -   dva = lmb_alloc(psize, DART_PAGE_SIZE);
> +   dva = apple_dart_dvaalloc(dev, psize);
>
> idx = dva / DART_PAGE_SIZE;
> for (i = 0; i < psize / DART_PAGE_SIZE; i++) {
> @@ -158,8 +173,6 @@ static void apple_dart_unmap(struct udevice *dev, 
> dma_addr_t addr, size_t size)
> flush_dcache_range((unsigned long)&priv->l2[idx],
>(unsigned long)&priv->l2[idx + i]);
> priv->flush_tlb(priv);
> -
> -   lmb_free(dva, psize);
>  }
>
>  static struct iommu_ops apple_dart_ops = {
> @@ -211,8 +224,7 @@ static int apple_dart_probe(struct udevice *dev)
>
> priv->dvabase = DART_PAGE_SIZE;
> priv->dvaend = SZ_4G - DART_PAGE_SIZE;
> -
> -   lmb_add(priv->dvabase, priv->dvaend - priv->dvabase);
> +   priv->dvaalloc = priv->dvaend;
>
> /* Disable translations. */
> for (sid = 0; sid < priv->nsid; sid++)
> --
> 2.34.1
>

[RFC PATCH 1/2] lmb: refactor lmb push and pop functions

[Sughosh Ganu]

The LMB module uses a separate instance of the lmb structure when
running tests. Setting up the correct instance is done using the
lmb_push() and lmb_pop() functions. In addition to pushing and popping
the lmb instances, these functions are also doing some test specific
operations. Move these operations to lmb_test_push() and
lmb_test_pop() functions, so that lmb_push() and lmb_pop() can be
called from elsewhere.

Signed-off-by: Sughosh Ganu 
---
 include/lmb.h  |  4 +++-
 lib/lmb.c  | 20 
 test/lib/lmb.c | 18 +-
 3 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/include/lmb.h b/include/lmb.h
index e46abf400c6..72d7093023a 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -171,8 +171,10 @@ void lmb_dump_all_force(void);
 void lmb_arch_add_memory(void);
 
 struct lmb *lmb_get(void);
-int lmb_push(struct lmb *store);
+void lmb_push(struct lmb *store);
 void lmb_pop(struct lmb *store);
+int lmb_test_push(struct lmb *store);
+void lmb_test_pop(struct lmb *store);
 
 static inline int lmb_read_check(phys_addr_t addr, phys_size_t len)
 {
diff --git a/lib/lmb.c b/lib/lmb.c
index eec99c185ee..2960c05abfc 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -892,12 +892,23 @@ struct lmb *lmb_get(void)
return &lmb;
 }
 
+void lmb_push(struct lmb *store)
+{
+   *store = lmb;
+}
+
+void lmb_pop(struct lmb *store)
+{
+   lmb = *store;
+}
+
 #if CONFIG_IS_ENABLED(UNIT_TEST)
-int lmb_push(struct lmb *store)
+int lmb_test_push(struct lmb *store)
 {
int ret;
 
-   *store = lmb;
+   lmb_push(store);
+
ret = lmb_setup(true);
if (ret)
return ret;
@@ -905,10 +916,11 @@ int lmb_push(struct lmb *store)
return 0;
 }
 
-void lmb_pop(struct lmb *store)
+void lmb_test_pop(struct lmb *store)
 {
alist_uninit(&lmb.free_mem);
alist_uninit(&lmb.used_mem);
-   lmb = *store;
+
+   lmb_pop(store);
 }
 #endif /* UNIT_TEST */
diff --git a/test/lib/lmb.c b/test/lib/lmb.c
index b2c54fb4bcb..64e619eff22 100644
--- a/test/lib/lmb.c
+++ b/test/lib/lmb.c
@@ -63,7 +63,7 @@ static int setup_lmb_test(struct unit_test_state *uts, struct 
lmb *store,
 {
struct lmb *lmb;
 
-   ut_assertok(lmb_push(store));
+   ut_assertok(lmb_test_push(store));
lmb = lmb_get();
*mem_lstp = &lmb->free_mem;
*used_lstp = &lmb->used_mem;
@@ -194,7 +194,7 @@ static int test_multi_alloc(struct unit_test_state *uts, 
const phys_addr_t ram,
ut_asserteq(mem[0].size, ram_size);
}
 
-   lmb_pop(&store);
+   lmb_test_pop(&store);
 
return 0;
 }
@@ -293,7 +293,7 @@ static int test_bigblock(struct unit_test_state *uts, const 
phys_addr_t ram)
ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, alloc_64k_addr, 0x1,
   0, 0, 0, 0);
 
-   lmb_pop(&store);
+   lmb_test_pop(&store);
 
return 0;
 }
@@ -373,7 +373,7 @@ static int test_noreserved(struct unit_test_state *uts, 
const phys_addr_t ram,
ut_asserteq(ret, 0);
ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 0, 0, 0, 0, 0, 0, 0);
 
-   lmb_pop(&store);
+   lmb_test_pop(&store);
 
return 0;
 }
@@ -446,7 +446,7 @@ static int lib_test_lmb_at_0(struct unit_test_state *uts)
ut_asserteq(ret, 0);
ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 0, 0, 0, 0, 0, 0, 0);
 
-   lmb_pop(&store);
+   lmb_test_pop(&store);
 
return 0;
 }
@@ -499,7 +499,7 @@ static int lib_test_lmb_overlapping_reserve(struct 
unit_test_state *uts)
ASSERT_LMB(mem_lst, used_lst, ram, ram_size, 1, 0x4000, 0x4,
   0, 0, 0, 0);
 
-   lmb_pop(&store);
+   lmb_test_pop(&store);
 
return 0;
 }
@@ -619,7 +619,7 @@ static int test_alloc_addr(struct unit_test_state *uts, 
const phys_addr_t ram)
ut_asserteq(ret, 0);
}
 
-   lmb_pop(&store);
+   lmb_test_pop(&store);
 
return 0;
 }
@@ -691,7 +691,7 @@ static int test_get_unreserved_size(struct unit_test_state 
*uts,
s = lmb_get_free_size(ram_end - 4);
ut_asserteq(s, 4);
 
-   lmb_pop(&store);
+   lmb_test_pop(&store);
 
return 0;
 }
@@ -797,7 +797,7 @@ static int lib_test_lmb_flags(struct unit_test_state *uts)
ut_asserteq(lmb_is_nomap(&used[1]), 0);
ut_asserteq(lmb_is_nomap(&used[2]), 1);
 
-   lmb_pop(&store);
+   lmb_test_pop(&store);
 
return 0;
 }
-- 
2.34.1

Re: [PATCH 4/4] efi: add helper functions to insert pmem node for DT fixup

[Ilias Apalodimas]

Hi Heinrich

On Mon, 28 Oct 2024 at 08:57, Heinrich Schuchardt  wrote:
>
> On 10/25/24 13:14, Sughosh Ganu wrote:
> > The EFI HTTP boot puts the iso installer image at some location in
> > memory which needs to be reserved in the devicetree as persistent
> > memory (pmem). Add helper functions which add this pmem node when the
> > EFI_DT_FIXUP protocol's fixup callback is invoked.
> >
> > Signed-off-by: Sughosh Ganu 
> > ---
> >   boot/image-fdt.c |  9 +
> >   include/efi_loader.h | 17 +
> >   lib/efi_loader/efi_bootmgr.c | 21 +
> >   lib/efi_loader/efi_helper.c  | 12 
> >   4 files changed, 59 insertions(+)
> >
> > diff --git a/boot/image-fdt.c b/boot/image-fdt.c
> > index 8eda521693..b39e81ad30 100644
> > --- a/boot/image-fdt.c
> > +++ b/boot/image-fdt.c
> > @@ -11,6 +11,7 @@
> >   #include 
> >   #include 
> >   #include 
> > +#include 
> >   #include 
> >   #include 
> >   #include 
> > @@ -648,6 +649,14 @@ int image_setup_libfdt(struct bootm_headers *images, 
> > void *blob, bool lmb)
> >   if (!ft_verify_fdt(blob))
> >   goto err;
> >
> > + if (CONFIG_IS_ENABLED(EFI_HTTP_BOOT)) {
> > + fdt_ret = fdt_efi_pmem_setup(blob);
>
> Having EFI_HTTP_BOOT is not the only way to load an image to memory. You
> could have downloaded it via TFTP or taken it from a block device. So
> this config check looks wrong. Why don't you check for the presence of
> memory block devices here?

If it's an existing block device (e.g a USB), there's no need to
preserve the .iso. The installer will scan the disks and find it
again.
TFTP might make sense, but this only applies to booting EFI images, so
scanning the memory block devices is not enough.

Sughosh can you take a look and see if scanning memory blocks is doable?

>
> You need to check if the board is using ACPI, if yes, provide the ACPI
> table.

The ACPI table is out of scope on these patches. We need to add a
check and only do the fixup if we are booting with a DT, but we dont
currently plan on fixing NFIT ACPI tables.

Thanks
/Ilias

> Best regards
>
> Heinrich
>
> > + if (fdt_ret) {
> > + printf("ERROR: HTTP boot pmem fixup failed\n");
> > + goto err;
> > + }
> > + }
> > +
> >   /* after here we are using a livetree */
> >   if (!of_live_active() && CONFIG_IS_ENABLED(EVENT)) {
> >   struct event_ft_fixup fixup;
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index d450e304c6..031de18746 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -748,6 +748,15 @@ bool efi_varname_is_load_option(u16 *var_name16, int 
> > *index);
> >   efi_status_t efi_next_variable_name(efi_uintn_t *size, u16 **buf,
> >   efi_guid_t *guid);
> >
> > +/**
> > + * fdt_efi_pmem_setup() - Setup the pmem node in the devicetree
> > + *
> > + * @fdt: Pointer to the devicetree
> > + *
> > + * Return:   0 on success, negative on failure
> > + */
> > +int fdt_efi_pmem_setup(void *fdt);
> > +
> >   /**
> >* efi_size_in_pages() - convert size in bytes to size in pages
> >*
> > @@ -964,6 +973,14 @@ efi_status_t efi_set_load_options(efi_handle_t handle,
> > void *load_options);
> >   efi_status_t efi_bootmgr_load(efi_handle_t *handle, void **load_options);
> >
> > +/**
> > + * efi_bootmgr_pmem_setup() - Put a pmem node for UEFI HTTP installers
> > + *
> > + * @fdt: Pointer to the DT blob
> > + * Return:   status code
> > + */
> > +efi_status_t efi_bootmgr_pmem_setup(void *fdt);
> > +
> >   /**
> >* struct efi_image_regions - A list of memory regions
> >*
> > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> > index 16f7f6..1d9246be61 100644
> > --- a/lib/efi_loader/efi_bootmgr.c
> > +++ b/lib/efi_loader/efi_bootmgr.c
> > @@ -41,6 +41,8 @@ struct uridp_context {
> >   efi_handle_t mem_handle;
> >   };
> >
> > +static struct uridp_context *uctx;
> > +
> >   const efi_guid_t efi_guid_bootmenu_auto_generated =
> >   EFICONFIG_AUTO_GENERATED_ENTRY_GUID;
> >
> > @@ -423,6 +425,7 @@ efi_status_t efi_bootmgr_release_uridp(struct 
> > uridp_context *ctx)
> >
> >   efi_free_pool(ctx->loaded_dp);
> >   free(ctx);
> > + uctx = NULL;
> >
> >   return ret == EFI_SUCCESS ? ret2 : ret;
> >   }
> > @@ -443,6 +446,23 @@ static void EFIAPI efi_bootmgr_http_return(struct 
> > efi_event *event,
> >   EFI_EXIT(ret);
> >   }
> >
> > +/**
> > + * efi_bootmgr_pmem_setup() - Put a pmem node for UEFI HTTP installers
> > + *
> > + * @fdt: Pointer to the DT blob
> > + * Return:   status code
> > + */
> > +efi_status_t efi_bootmgr_pmem_setup(void *fdt)
> > +{
> > + if (!uctx) {
> > + log_warning("No EFI HTTP boot context found\n");
> > + return EFI_SUCCESS;
> > + }
> > +
> > + return !fdt_fixup_pmem_region(fdt, u

Re: [PATCH 1/1] lmb: remove __maybe_unused from lmb_map_update_notify

[Ilias Apalodimas]

On Mon, 28 Oct 2024 at 08:21, Heinrich Schuchardt
 wrote:
>
> Function lmb_map_update_notify() is always referenced.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  lib/lmb.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/lib/lmb.c b/lib/lmb.c
> index eec99c185ee..802fdb84489 100644
> --- a/lib/lmb.c
> +++ b/lib/lmb.c
> @@ -38,9 +38,7 @@ static bool lmb_should_notify(enum lmb_flags flags)
> CONFIG_IS_ENABLED(EFI_LOADER);
>  }
>
> -static int __maybe_unused lmb_map_update_notify(phys_addr_t addr,
> -   phys_size_t size,
> -   u8 op)
> +static int lmb_map_update_notify(phys_addr_t addr, phys_size_t size, u8 op)
>  {
> u64 efi_addr;
> u64 pages;
> --
> 2.45.2
>

Reviewed-by: Ilias Apalodimas 

[PATCH v2 3/3] bootmenu: add reprint check

[Weijie Gao]

Record the last active menu item and check if it equals to the
current selected item before reprint.

Signed-off-by: Weijie Gao 
---
 cmd/bootmenu.c | 16 +++-
 include/menu.h |  1 +
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
index c99605f3398..ffa63a4628d 100644
--- a/cmd/bootmenu.c
+++ b/cmd/bootmenu.c
@@ -103,11 +103,13 @@ static char *bootmenu_choice_entry(void *data)
 
switch (key) {
case BKEY_UP:
+   menu->last_active = menu->active;
if (menu->active > 0)
--menu->active;
/* no menu key selected, regenerate menu */
return NULL;
case BKEY_DOWN:
+   menu->last_active = menu->active;
if (menu->active < menu->count - 1)
++menu->active;
/* no menu key selected, regenerate menu */
@@ -133,6 +135,17 @@ static char *bootmenu_choice_entry(void *data)
return NULL;
 }
 
+static bool bootmenu_need_reprint(void *data)
+{
+   struct bootmenu_data *menu = data;
+   bool need_reprint;
+
+   need_reprint = menu->last_active != menu->active;
+   menu->last_active = menu->active;
+
+   return need_reprint;
+}
+
 static void bootmenu_destroy(struct bootmenu_data *menu)
 {
struct bootmenu_entry *iter = menu->first;
@@ -332,6 +345,7 @@ static struct bootmenu_data *bootmenu_create(int delay)
 
menu->delay = delay;
menu->active = 0;
+   menu->last_active = -1;
menu->first = NULL;
 
default_str = env_get("bootmenu_default");
@@ -506,7 +520,7 @@ static enum bootmenu_ret bootmenu_show(int delay)
 
menu = menu_create(NULL, bootmenu->delay, 1, menu_display_statusline,
   bootmenu_print_entry, bootmenu_choice_entry,
-  NULL, bootmenu);
+  bootmenu_need_reprint, bootmenu);
if (!menu) {
bootmenu_destroy(bootmenu);
return BOOTMENU_RET_FAIL;
diff --git a/include/menu.h b/include/menu.h
index 79643af272b..6cede89b950 100644
--- a/include/menu.h
+++ b/include/menu.h
@@ -40,6 +40,7 @@ int menu_show(int bootdelay);
 struct bootmenu_data {
int delay;  /* delay for autoboot */
int active; /* active menu entry */
+   int last_active;/* last active menu entry */
int count;  /* total count of menu entries */
struct bootmenu_entry *first;   /* first menu entry */
 };
-- 
2.45.2

[PATCH v2 2/3] menu: add support to check if menu needs to be reprinted

[Weijie Gao]

This patch adds a new callback named need_reprint for menu.
The need_reprint will be called before printing the menu. If the
callback exists and returns FALSE, menu printing will be canceled.

This is very useful if the menu was not changed. It can save time
for serial-based menu to handle more input data.

Signed-off-by: Weijie Gao 
---
 boot/pxe_utils.c |  2 +-
 cmd/bootmenu.c   |  2 +-
 cmd/eficonfig.c  |  2 +-
 common/menu.c| 11 +++
 include/menu.h   |  1 +
 5 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/boot/pxe_utils.c b/boot/pxe_utils.c
index d6a4b2cb859..3ae17553c6d 100644
--- a/boot/pxe_utils.c
+++ b/boot/pxe_utils.c
@@ -1474,7 +1474,7 @@ static struct menu *pxe_menu_to_menu(struct pxe_menu *cfg)
 * Create a menu and add items for all the labels.
 */
m = menu_create(cfg->title, DIV_ROUND_UP(cfg->timeout, 10),
-   cfg->prompt, NULL, label_print, NULL, NULL);
+   cfg->prompt, NULL, label_print, NULL, NULL, NULL);
if (!m)
return NULL;
 
diff --git a/cmd/bootmenu.c b/cmd/bootmenu.c
index 977a04b7d76..c99605f3398 100644
--- a/cmd/bootmenu.c
+++ b/cmd/bootmenu.c
@@ -506,7 +506,7 @@ static enum bootmenu_ret bootmenu_show(int delay)
 
menu = menu_create(NULL, bootmenu->delay, 1, menu_display_statusline,
   bootmenu_print_entry, bootmenu_choice_entry,
-  bootmenu);
+  NULL, bootmenu);
if (!menu) {
bootmenu_destroy(bootmenu);
return BOOTMENU_RET_FAIL;
diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index bea09e4ecc7..498653b778d 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -443,7 +443,7 @@ efi_status_t eficonfig_process_common(struct efimenu 
*efi_menu,
efi_menu->menu_desc = menu_desc;
 
menu = menu_create(NULL, 0, 1, display_statusline, item_data_print,
-  item_choice, efi_menu);
+  item_choice, NULL, efi_menu);
if (!menu)
return EFI_INVALID_PARAMETER;
 
diff --git a/common/menu.c b/common/menu.c
index 48ab7f0f398..5a2126aa01a 100644
--- a/common/menu.c
+++ b/common/menu.c
@@ -43,6 +43,7 @@ struct menu {
void (*display_statusline)(struct menu *);
void (*item_data_print)(void *);
char *(*item_choice)(void *);
+   bool (*need_reprint)(void *);
void *item_choice_data;
struct list_head items;
int item_cnt;
@@ -117,6 +118,11 @@ static inline void *menu_item_destroy(struct menu *m,
  */
 static inline void menu_display(struct menu *m)
 {
+   if (m->need_reprint) {
+   if (!m->need_reprint(m->item_choice_data))
+   return;
+   }
+
if (m->title) {
puts(m->title);
putc('\n');
@@ -362,6 +368,9 @@ int menu_item_add(struct menu *m, char *item_key, void 
*item_data)
  * item. Returns a key string corresponding to the chosen item or NULL if
  * no item has been selected.
  *
+ * need_reprint - If not NULL, will be called before printing the menu.
+ * Returning FALSE means the menu does not need reprint.
+ *
  * item_choice_data - Will be passed as the argument to the item_choice 
function
  *
  * Returns a pointer to the menu if successful, or NULL if there is
@@ -371,6 +380,7 @@ struct menu *menu_create(char *title, int timeout, int 
prompt,
void (*display_statusline)(struct menu *),
void (*item_data_print)(void *),
char *(*item_choice)(void *),
+   bool (*need_reprint)(void *),
void *item_choice_data)
 {
struct menu *m;
@@ -386,6 +396,7 @@ struct menu *menu_create(char *title, int timeout, int 
prompt,
m->display_statusline = display_statusline;
m->item_data_print = item_data_print;
m->item_choice = item_choice;
+   m->need_reprint = need_reprint;
m->item_choice_data = item_choice_data;
m->item_cnt = 0;
 
diff --git a/include/menu.h b/include/menu.h
index 6571c39b143..79643af272b 100644
--- a/include/menu.h
+++ b/include/menu.h
@@ -13,6 +13,7 @@ struct menu *menu_create(char *title, int timeout, int prompt,
void (*display_statusline)(struct menu *),
void (*item_data_print)(void *),
char *(*item_choice)(void *),
+   bool (*need_reprint)(void *),
void *item_choice_data);
 int menu_default_set(struct menu *m, char *item_key);
 int menu_get_choice(struct menu *m, void **choice);
-- 
2.45.2

Re: [PATCH 09/15] efi_loader: Fix free in ..._media_device_boot_option()

[Ilias Apalodimas]

Hi Simon,

On Mon, 28 Oct 2024 at 14:48, Simon Glass  wrote:
>
> Freeing a NULL pointer is an error in EFI, so check the pointer first,
> before freeing it.
>
> Signed-off-by: Simon Glass 
> ---
>
>  lib/efi_loader/efi_bootmgr.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index a3aa2b8d1b9..431a38704e9 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -1180,7 +1180,8 @@ out:
> free(opt[i].lo);
> }
> free(opt);
> -   efi_free_pool(handles);
> +   if (handles)
> +   efi_free_pool(handles);

We don't need this, efi_free_pool() checks the pointer already.

Thanks
/Ilias
>
> if (ret == EFI_NOT_FOUND)
> return EFI_SUCCESS;
> --
> 2.43.0
>

Re: [PATCH v2 4/5] arm: mach-k3: j721e-init.c: J7200: Add support for CONFIG_K3_OPP_LOW

[Manorit Chawdhry]

Hi Aniket,

On 18:27-20241023, Aniket Limaye wrote:
> From: Reid Tonking 
> 
> The default j7200 devicetree and k3_avs driver set 2GHz/1GHz frequency
> for A72/MSMC clks and the OPP_NOM voltage.
> 
> J7200 SOCs may support OPP_LOW Operating Performance Point:
> 1GHz/500MHz clks for A72/MSMC and OPP_LOW AVS voltage read from efuse.
> 
> Hence, add a config check to select OPP_LOW specs:
> - Check if OPP_LOW AVS voltage read from efuse is valid.
> - Update the clock frequencies in devicetree.
> - Program the OPP_LOW AVS voltage for VDD_CPU.
> 
> Signed-off-by: Reid Tonking 
> Signed-off-by: Aniket Limaye 
> 
> ---
> v2:
> - Fix indentation in fix_freq()
> - Remove the efuse data check addition from this commit, as it's not
>   related to adding support for CONFIG_K3_OPP_LOW. The same addition
>   was moved to the previous patch in this series.
> - Link to v1: 
> https://lore.kernel.org/u-boot/20241017062911.2241167-5-a-lim...@ti.com/
> ---
>  arch/arm/mach-k3/j721e/j721e_init.c | 45 -
>  1 file changed, 44 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-k3/j721e/j721e_init.c 
> b/arch/arm/mach-k3/j721e/j721e_init.c
> index e9ed8cb267c..de10517bb27 100644
> --- a/arch/arm/mach-k3/j721e/j721e_init.c
> +++ b/arch/arm/mach-k3/j721e/j721e_init.c
> @@ -19,6 +19,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  
>  #include "../sysfw-loader.h"
>  #include "../common.h"
> @@ -147,6 +148,32 @@ static void setup_navss_nb(void)
>   writel(NB_THREADMAP_BIT1, (uintptr_t)NAVSS0_NBSS_NB1_CFG_NB_THREADMAP);
>  }
>  
> +int fix_freq(const void *fdt)
> +{
> + int node, ret;
> + u32 opp_low_freq[3];
> +
> + node = fdt_node_offset_by_compatible(fdt, -1, "ti,am654-rproc");
> + if (node < 0) {
> + printf("%s: A72 not found\n", __func__);
> + return node;
> + }
> +
> + /* j7200 opp low values according to data sheet */
> + opp_low_freq[0] = cpu_to_fdt32(10); /* 202-2 -> 
> A72SS0_CORE0_0_ARM_CLK */
> + opp_low_freq[1] = cpu_to_fdt32(2); /* 61-1 -> GTC0_GTC_CLK */
> + opp_low_freq[2] = cpu_to_fdt32(5); /* 4-1 -> 
> A72SS0_CORE0_MSMC_CLK */
> +
> + ret = fdt_setprop((void *)fdt, node, "assigned-clock-rates",
> +   opp_low_freq, sizeof(opp_low_freq));
> + if (ret) {
> + printf("%s: Can not set value\n", __func__);
> + return ret;
> + }
> +
> + return 0;
> +}
> +
>  /*
>   * This uninitialized global variable would normal end up in the .bss 
> section,
>   * but the .bss is cleared between writing and reading this variable, so move
> @@ -301,8 +328,24 @@ void board_init_f(ulong dummy)
>  #if defined(CONFIG_CPU_V7R) && defined(CONFIG_K3_AVS0)
>   ret = uclass_get_device_by_driver(UCLASS_MISC, DM_DRIVER_GET(k3_avs),
> &dev);
> - if (ret)
> + if (!ret) {
> + if (IS_ENABLED(CONFIG_K3_OPP_LOW)) {

These ifs can be combined into one ig. 

> + ret = k3_check_opp(dev, J721E_VDD_MPU, AM6_OPP_LOW);
> + if (!ret) {
> + ret = fix_freq(gd->fdt_blob);
> + if (ret)
> + printf("Failed to set OPP_LOW 
> frequency\n");
> +
> + ret = k3_avs_set_opp(dev, J721E_VDD_MPU, 
> AM6_OPP_LOW);

Even if fix_freq fails, do we want to cascade and still run
k3_avs_set_opp? Not sure what the best way to handle this is but seeing
a lot of nesting so wondering if there is any better way to handle this
control flow..

Also, better to print ret as well in these cases btw.

Regards,
Manorit

> + if (ret)
> + printf("Failed to set OPP_LOW 
> voltage\n");
> + } else {
> + printf("Failed to enable K3_OPP_LOW\n");
> + }
> + }
> + } else {
>   printf("AVS init failed: %d\n", ret);
> + }
>  #endif
>  
>  #if defined(CONFIG_K3_J721E_DDRSS)
> -- 
> 2.34.1
>