Re: [PATCH 0/8] An effort to bring DT bindings compliance within U-boot

[Simon Glass]

Hi Sumit,

On Tue, Dec 26, 2023 at 10:06 AM Sumit Garg  wrote:
>
> Hi Simon,
>
> On Tue, 26 Dec 2023 at 15:17, Simon Glass  wrote:
> >
> > Hi Sumit,
> >
> > On Fri, Dec 22, 2023 at 5:34 AM Sumit Garg  wrote:
> > >
> > > On Thu, 21 Dec 2023 at 20:48, Simon Glass  wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Thu, Dec 21, 2023 at 3:03 PM Tom Rini  wrote:
> > > > >
> > > > > On Thu, Dec 14, 2023 at 07:20:55PM +0530, Sumit Garg wrote:
> > > > >
> > > > > > Prerquisite
> > > > > > ---
> > > > > >
> > > > > > This patch series requires devicetree-rebasing git repo to be added 
> > > > > > as a
> > > > > > subtree to the main U-boot repo via:
> > > > > >
> > > > > > $ git subtree add --prefix devicetree-rebasing \
> > > > > >   
> > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git
> > > > > >  \
> > > > > >   v6.6-dts --squash
> > > > >
> > > > > So, I've played with subtree a little and I think this is the right 
> > > > > way
> > > > > forward in these cases. If anyone wants to take a look at how this 
> > > > > works
> > > > > in practice, take a look at:
> > > > > https://source.denx.de/u-boot/u-boot/-/commits/WIP/u-boot-with-devicetree-rebasing-since-v6.1/?ref_type=heads
> > > > > In that tree I started with the v6.1-dts tag, sync'd all the configs 
> > > > > (to
> > > > > have an example of a normal commit) and then did a merge of each tag
> > > > > until v6.6-dts, so provide some history. And git log looks like what I
> > > > > want to see, the squash commit has clear references to what we are
> > > > > getting and I make a merge commit that says what I did. If you pull 
> > > > > the
> > > > > tree and checkout the branch, all the code is right there already,
> > > > > nothing further to do. Same with tarball releases. The only thing I
> > > > > don't like is the size growth there, but we'll reclaim some of it when
> > > > > we delete our obsolete bindings, and then obsolete dts files.
> > > >
> > > > I spent a bit of time with subtree as well, as part of reviewing this
> > > > series, using the instructions Sumit provided. It seems OK to me. We
> > > > have to accept that it adds code and there will be changes/churn, but
> > > > it is not too different to accepting patches on those files within
> > > > U-Boot. We will bring in files which U-Boot doesn't use, but U-Boot
> > > > does support a good proportion of the boards supported by Linux, so I
> > > > don't see that as a big cost.
> > > >
> > > > From my experimentation, subtrees seem to have no impact on buildman,
> > > > which is great. Am I missing anything?
> > >
> > > No it shouldn't cause any regression on existing tools/CI/build
> > > systems. It is just a version controlled way of importing third party
> > > source code as a tarball.
> > >
> > > >
> > > > I still worry about the board-level 'switch' between U-Boot DT and
> > > > upstream ones. I believe that should be at the SoC level instead.
> > > >
> > >
> > > Probably I wasn't clear enough in my earlier replies but this series
> > > motivates for a SoC level switch only. Patch #7 is essentially a
> > > switch for Amlogic meson-gxbb SoC and its derived boards.
> >
> > OK good...but that's not quite what I mean. You have a fragment like
> > this in multiple defconfig files:
> >
> > +CONFIG_OF_UPSTREAM=y
> > +CONFIG_DEFAULT_DEVICE_TREE="amlogic/meson-gxbb-odroidc2"
> >
> > instead, OF_UPSTREAM should be enabled via 'imply' for the SoC, in the
> > Kconfig.
>
> Okay I got your point. It makes sense to me. I will do that for v3.
>
> > We should not have to specify the filename like this. It is
> > laborious and error-prone.
>
> The only differentiation in naming here is just silicon vendor prefix
> addition (amlogic/ in this case). The reason for this being that I
> just want to mirror the whole silicon vendor directory from DT
> rebasing rather than mirroring individual DT files. Given this do you
> have any better alternatives?

My current opinion is that a better approach would be to move the
files first (in the U-Boot tree). That would be easier if we could
just copy the Linux arch/xxx/boot/dts Makefiles but for now that is
not possible. The Kconfig options for each SoC are similar but there
are various differences.

Having a different layout is just a pain and it will get worse, as
people add new boards, since they need to know to add the correct
directory.

I will see if I can devise a small series to point this in what I
consider to be the right direction, in line with Linux, and the U-Boot
commit 3284c8b8 ("dts: generate multiple device tree blobs").

Regards,
Simon

Re: [PATCH v2 3/8] scripts/Makefile.lib: Statically define *-u-boot.dtsi files location

[Simon Glass]

Hi Sumit,

On Tue, Dec 26, 2023 at 10:09 AM Sumit Garg  wrote:
>
> On Tue, 26 Dec 2023 at 15:17, Simon Glass  wrote:
> >
> > Hi Sumit,
> >
> > On Fri, Dec 22, 2023 at 6:12 AM Sumit Garg  wrote:
> > >
> > > Allow u-boot to build DTB from a different directory tree such that
> > > *-u-boot.dtsi files can be included from a common location. Currently
> > > that location is arch/$(ARCH)/dts/, so statically define that common
> > > location.
> > >
> > > This is needed for platform owners to start building DTB files from
> > > devicetree-rebasing directory but still being able to include
> > > *-u-boot.dtsi files.
> > >
> >
> > This is a v2 patch but I don't see a change log?
>
> I usually prefer to keep change logs in the cover letter [1].
>
> [1] 
> https://lore.kernel.org/all/20231222061208.3009970-1-sumit.g...@linaro.org/
>

As a reviewer I much prefer to see the changes with the patch. If you
use patman you can have both, I suppose.

> >
> > > Signed-off-by: Sumit Garg 
> > > ---
> > >  scripts/Makefile.lib | 25 ++---
> > >  1 file changed, 14 insertions(+), 11 deletions(-)
> >
> > Reviewed-by: Simon Glass 
> >

Regards,
Simon

vexpress and DM_SERIAL

[Simon Glass]

Hi Kristian,

Do you think it would be possible to migrate the vexpress_ca9x4 board
to use DM_SERIAL?

Thanks,
SImon

Re: Passing boot logs to Linux?

[Dragan Simic]

On 2023-12-26 23:38, Csókás Bence wrote:

On 2023. 12. 26. 19:09, Dragan Simic wrote:

On 2023-12-26 10:46, Simon Glass wrote:
On Thu, Dec 21, 2023 at 2:23 AM Dragan Simic  
wrote:

On 2023-12-21 02:44, Dragan Simic wrote:
> On 2023-12-21 02:37, Dragan Simic wrote:
>> On 2023-12-21 02:03, Daniel Golle wrote:
>>> On Thu, Dec 21, 2023 at 12:55:20AM +0100, Dragan Simic wrote:
 On 2023-12-21 00:27, Csókás Bence wrote:
 > Not every system has eMMC/uSD, and as you said, these
arguments don't
 > hold for a 4 MB SPI NAND, for example, one you might find in
an OpenWrt
 > router for example. Whereas RAM is quite cheap nowadays.

 I see, but I also wonder how many such OpenWrt routers are still
 used these
 days, and, even more importantly, how many of them are regularly
 updated and
 can be expected to actually use this new feature?
>>>
>>> Avoid flash writes is a very important matter, even on systems with
>>> 128 MiB of SPI-NAND flash which is by far the most common setup you
>>> find on off-the-shelf plastic routers and access points nowadays.
>>
>> I agree, writing something to the SPI chips all the time, no matter
>> how small the writes are, is a big no-no, which I already clearly
>> expressed in one of my earlier posts.
>>
>>> Especially also as those devices often come without a local console,
>>> having U-Boot's output prepended to dmesg on boot would be a very
big
>>> win.
>>
>> I was also thinking about that, but I'm not sure it would be accepted
>> to the Linux kernel.  Maybe we can try getting that accepted later.

Maybe, but just _maybe,_ it could be possible to add a new 
command-line
option to dmesg(1) that would display the last recorded console 
output,
fetched from the pstore.  That _might_ get accepted to util-linux, 
while

being perfectly fine from the usability standpoint.

I'm also willing to work on that, and I already contributed a few
dmesg(1) patches to util-linux.

 Please, don't get me wrong, I still support having both options
 available,
 but I'm also wondering about the target demographic.

 > > > Plus, I don't want the console subsystem to depend on any
file/disk
 > > > operations/drivers.
 > >
 > > Well, the console would still work as usual even if logging
to disk
 > > would fail for any reason, which is similar to the serial
console
 > > still
 > > working if the graphical console fails.  Moreover, if the
disk fails,
 > > the system isn't be able to boot, so any RAM-based console logs
 > > would be
 > > lost in that case.  All this makes the RAM-based logging no
more
 > > resilient to disk failures.
 >
 > Correction: if disk *reads* fail, as well as writes, then the
system
 > will not boot. However, typical failure of Flash media is that it
 > becomes read-only.

 That's a good point, but having a read-only root filesystem usually
 also
 means having a non-operational system that can only have its stored
 data
 salvaged.  Unless the system is specifically crafted to survive
such
 scenarios, of course.
>>>
>>> ... which holds true for any decent embedded OS, which at least
>>> allows
>>> limited remote access and some kind of recovery even in this
>>> situation.
>>
>> Perhaps.  I'm more into running general-purpose Linux
distributions on
>> single-board computers and derived embedded devices, which are on the
>> "thick" end of the embedded device spectrum, so to speak.
>>
 > > I still think that using disk-based pstore is a better
option.  Just
 > > as
 > > you don't want to wear out your flash disks with 30-40 KB of
data, I
 > > also don't want to waste 30-40 KB of RAM.
 >
 > As I said, you could just unload the log after you're done
processing
 > it. 40 KB RAM is less, than what `sshd` uses, for instance
(860k on my
 > laptop, but it can probably be less, maybe even 10x less, so
80-90k?),
 > so you could, in your init, process the in-RAM log, then
unload it, then
 > start your other services, thereby reclaiming that RAM.

 Using pstore should have that unloading already covered, and the
 already
 existing systemd service is there to perform the archiving to the
 primary
 filesystem, if desired so.  It would all need to be tested in
 detail, of
 course.
>>>
>>> pstore/ramoops uses a statically assigned reserved memory region, so
>>> in
>>> the moment you want to use that feature you loose that amount of RAM
>>> (a
>>> few kB, so it doesn't really matter on modern systems).
>>> As in: there is *no* dynamic allocation.
>>>
>>> Imho using pstore/ramoops (which is a more or less Linux-specific
>>> debugging feature, meant to store one or more timestamped logs of
>>> crashes) might not be the most suitable choice. I understand the
>>> advantages of using existing infrastructure, but on the other hand
>>> we don't need most of the complexity of pstore for the task.
>>
>> 

Re: [PATCH 5/5] test: unit test for smbios command

[Simon Glass]

Hi Heinrich,

On Tue, Dec 26, 2023 at 10:11 AM Heinrich Schuchardt  wrote:
>
> On 12/26/23 10:47, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Sat, Dec 23, 2023 at 12:49 AM Heinrich Schuchardt  
> > wrote:
> >>
> >> Provide a unit test for the smbios command.
> >>
> >> Provide different test functions for QEMU, sandbox, and other systems.
> >>
> >> Signed-off-by: Heinrich Schuchardt 
> >> ---
> >>   test/py/tests/test_smbios.py | 47 
> >>   1 file changed, 47 insertions(+)
> >>   create mode 100644 test/py/tests/test_smbios.py
> >
> > Can you please write this test in C?
> >
> > Regards,
> > Simon
>
> According to our documentation we can use either Python or C tests.
> We generally use Python tests for testing the CLI.

Not really...there are lots of C tests for the CLI. It is actually
easier to code, in most cases.

>
> I cannot see any benefit in trying to write this in C.

I did go to some effort to document this:

https://docs.u-boot.org/en/latest/develop/tests_writing.html#test-types

Regards,
Simon

[PATCH v4 7/7] smbios: Require the caller to align the SMBIOS table

[Simon Glass]

All callers handle this alignment, so drop the unnecessary code. This
simplifies things a little.

Signed-off-by: Simon Glass 
Reviewed-by: Heinrich Schuchardt 
---

(no changes since v1)

 include/smbios.h | 5 +
 lib/smbios.c | 2 --
 2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/include/smbios.h b/include/smbios.h
index 77be58887a2..879b8a814b8 100644
--- a/include/smbios.h
+++ b/include/smbios.h
@@ -258,12 +258,9 @@ static inline void fill_smbios_header(void *table, int 
type,
  *
  * This writes SMBIOS table at a given address.
  *
- * @addr:  start address to write SMBIOS table. If this is not
- * 16-byte-aligned then it will be aligned before the table is
- * written.
+ * @addr:  start address to write SMBIOS table (must be 16-byte-aligned)
  * Return: end address of SMBIOS table (and start address for next entry)
  * or NULL in case of an error
- *
  */
 ulong write_smbios_table(ulong addr);
 
diff --git a/lib/smbios.c b/lib/smbios.c
index 7f79d969c92..cfd451e4088 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -563,8 +563,6 @@ ulong write_smbios_table(ulong addr)
ctx.dev = NULL;
}
 
-   /* 16 byte align the table address */
-   addr = ALIGN(addr, 16);
start_addr = addr;
 
/*
-- 
2.34.1

[PATCH v4 6/7] efi: Use the correct GUID for the SMBIOS table

[Simon Glass]

EFI does not use the 'anchor string' to determine the SMBIOS table
version, instead preferring to have two separate GUIDs. Use the correct
one, depending on the table version.

Call unmap_system() to balance to the use of map_sysmem()

Signed-off-by: Simon Glass 
Reviewed-by: Heinrich Schuchardt 
---

(no changes since v2)

Changes in v2:
- Add a note about why unmap_system() is called

 include/efi_api.h   |  4 
 lib/efi_loader/efi_smbios.c | 12 ++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/include/efi_api.h b/include/efi_api.h
index 0e92cb8a7f6..ab40b1b5ddf 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -433,6 +433,10 @@ struct efi_runtime_services {
EFI_GUID(0xeb9d2d31, 0x2d88, 0x11d3,  \
 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
 
+#define SMBIOS3_TABLE_GUID \
+   EFI_GUID(0xf2fd1544, 0x9794, 0x4a2c, \
+0x99, 0x2e, 0xe5, 0xbb, 0xcf, 0x20, 0xe3, 0x94)
+
 #define EFI_LOAD_FILE_PROTOCOL_GUID \
EFI_GUID(0x56ec3091, 0x954c, 0x11d2, \
 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c
index 49adc87e45a..5cbce6dc4eb 100644
--- a/lib/efi_loader/efi_smbios.c
+++ b/lib/efi_loader/efi_smbios.c
@@ -14,6 +14,8 @@
 #include 
 #include 
 
+const efi_guid_t smbios3_guid = SMBIOS3_TABLE_GUID;
+
 enum {
TABLE_SIZE  = SZ_4K,
 };
@@ -25,8 +27,10 @@ enum {
  */
 efi_status_t efi_smbios_register(void)
 {
+   const efi_guid_t *guid;
ulong addr;
efi_status_t ret;
+   void *buf;
 
addr = gd_smbios_start();
if (!addr) {
@@ -42,8 +46,12 @@ efi_status_t efi_smbios_register(void)
log_debug("EFI using SMBIOS tables at %lx\n", addr);
 
/* Install SMBIOS information as configuration table */
-   return efi_install_configuration_table(_guid,
-  map_sysmem(addr, 0));
+   buf = map_sysmem(addr, 0);
+   guid = !memcmp(buf, "_SM_", 4) ? _guid : _guid;
+   ret = efi_install_configuration_table(guid, buf);
+   unmap_sysmem(buf);
+
+   return ret;
 }
 
 static int install_smbios_table(void)
-- 
2.34.1

[PATCH v4 5/7] smbios: Correct gd_smbios_start()

[Simon Glass]

This should access arch-specific properties. Fix it and update the
existing usage.

Signed-off-by: Simon Glass 
Reviewed-by: Heinrich Schuchardt 
---

(no changes since v2)

Changes in v2:
- Add a new patch to correct gd_smbios_start()

 include/asm-generic/global_data.h | 2 +-
 lib/efi_loader/efi_smbios.c   | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/asm-generic/global_data.h 
b/include/asm-generic/global_data.h
index e8c6412e3f8..c1f7818817f 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -553,7 +553,7 @@ static_assert(sizeof(struct global_data) == GD_SIZE);
 #endif
 
 #ifdef CONFIG_SMBIOS
-#define gd_smbios_start()  gd->smbios_start
+#define gd_smbios_start()  gd->arch.smbios_start
 #define gd_set_smbios_start(addr)  gd->arch.smbios_start = addr
 #else
 #define gd_smbios_start()  0UL
diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c
index bbb8421ce14..49adc87e45a 100644
--- a/lib/efi_loader/efi_smbios.c
+++ b/lib/efi_loader/efi_smbios.c
@@ -28,7 +28,7 @@ efi_status_t efi_smbios_register(void)
ulong addr;
efi_status_t ret;
 
-   addr = gd->arch.smbios_start;
+   addr = gd_smbios_start();
if (!addr) {
log_err("No SMBIOS tables to install\n");
return EFI_NOT_FOUND;
-- 
2.34.1

[PATCH v4 4/7] smbios: Use SMBIOS 3.0 to support an address above 4GB

[Simon Glass]

When the SMBIOS table is written to an address above 4GB a 32-bit table
address is not large enough.

Use an SMBIOS3 table in that case.

Note that we cannot use efi_allocate_pages() since this function has
nothing to do with EFI. There is no equivalent function to allocate
memory below 4GB in U-Boot. One solution would be to create a separate
malloc() pool, or just always put the malloc() pool below 4GB.

- Use log_debug() for warning
- Rebase on Heinrich's smbios.h patch
- Set the checksum for SMBIOS3

Signed-off-by: Simon Glass 
---

Changes in v4:
- Check the start of the table rather than the end

Changes in v2:
- Check the end of the table rather than the start.

 include/smbios.h |  6 +-
 lib/smbios.c | 30 +-
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/include/smbios.h b/include/smbios.h
index e601283d293..77be58887a2 100644
--- a/include/smbios.h
+++ b/include/smbios.h
@@ -12,7 +12,7 @@
 
 /* SMBIOS spec version implemented */
 #define SMBIOS_MAJOR_VER   3
-#define SMBIOS_MINOR_VER   0
+#define SMBIOS_MINOR_VER   7
 
 enum {
SMBIOS_STR_MAX  = 64,   /* Maximum length allowed for a string */
@@ -80,6 +80,10 @@ struct __packed smbios3_entry {
u64 struct_table_address;
 };
 
+/* These two structures should use the same amount of 16-byte-aligned space */
+static_assert(ALIGN(16, sizeof(struct smbios_entry)) ==
+ ALIGN(16, sizeof(struct smbios3_entry)));
+
 /* BIOS characteristics */
 #define BIOS_CHARACTERISTICS_PCI_SUPPORTED (1 << 7)
 #define BIOS_CHARACTERISTICS_UPGRADEABLE   (1 << 11)
diff --git a/lib/smbios.c b/lib/smbios.c
index eea72670bd9..7f79d969c92 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -567,7 +567,11 @@ ulong write_smbios_table(ulong addr)
addr = ALIGN(addr, 16);
start_addr = addr;
 
-   addr += sizeof(struct smbios_entry);
+   /*
+* So far we don't know which struct will be used, but they both end
+* up using the same amount of 16-bit-aligned space
+*/
+   addr += max(sizeof(struct smbios_entry), sizeof(struct smbios3_entry));
addr = ALIGN(addr, 16);
tables = addr;
 
@@ -590,16 +594,32 @@ ulong write_smbios_table(ulong addr)
 * We must use a pointer here so things work correctly on sandbox. The
 * user of this table is not aware of the mapping of addresses to
 * sandbox's DRAM buffer.
+*
+* Check the address of the end of the tables. If it is above 4GB then
+* it is sensible to use SMBIOS3 even if the start of the table is below
+* 4GB (this case is very unlikely to happen in practice)
 */
table_addr = (ulong)map_sysmem(tables, 0);
-   if (sizeof(table_addr) > sizeof(u32) && table_addr > (ulong)UINT_MAX) {
+   if (sizeof(table_addr) > sizeof(u32) && addr >= (ulong)UINT_MAX) {
+   struct smbios3_entry *se;
/*
 * We need to put this >32-bit pointer into the table but the
 * field is only 32 bits wide.
 */
-   printf("WARNING: SMBIOS table_address overflow %llx\n",
-  (unsigned long long)table_addr);
-   addr = 0;
+   log_debug("WARNING: Using SMBIOS3.0 due to table-address 
overflow %lx\n",
+ table_addr);
+   se = map_sysmem(start_addr, sizeof(struct smbios_entry));
+   memset(se, '\0', sizeof(struct smbios_entry));
+   memcpy(se->anchor, "_SM3_", 5);
+   se->length = sizeof(struct smbios3_entry);
+   se->major_ver = SMBIOS_MAJOR_VER;
+   se->minor_ver = SMBIOS_MINOR_VER;
+   se->doc_rev = 0;
+   se->entry_point_rev = 1;
+   se->max_struct_size = len;
+   se->struct_table_address = table_addr;
+   se->checksum = table_compute_checksum(se,
+   sizeof(struct smbios3_entry));
} else {
struct smbios_entry *se;
 
-- 
2.34.1

[PATCH v4 3/7] smbios: SMBIOS 3.0 (64-bit) Entry Point structure

[Simon Glass]

From: Heinrich Schuchardt 

Add definition of the SMBIOS 3.0 (64-bit) Entry Point structure.

Signed-off-by: Heinrich Schuchardt 
Reviewed-by: Simon Glass 

Signed-off-by: Simon Glass 
---

Changes in v4:
- Bring in this patch from Heinrich's series

 include/smbios.h | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/include/smbios.h b/include/smbios.h
index c9df2706f5a..e601283d293 100644
--- a/include/smbios.h
+++ b/include/smbios.h
@@ -54,6 +54,32 @@ struct __packed smbios_entry {
u8 bcd_rev;
 };
 
+/**
+ * struct smbios3_entry - SMBIOS 3.0 (64-bit) Entry Point structure
+ */
+struct __packed smbios3_entry {
+   /** @anchor: anchor string */
+   u8 anchor[5];
+   /** @checksum: checksum of the entry point structure */
+   u8 checksum;
+   /** @length: length of the entry point structure */
+   u8 length;
+   /** @major_ver: major version of the SMBIOS specification */
+   u8 major_ver;
+   /** @minor_ver: minor version of the SMBIOS specification */
+   u8 minor_ver;
+   /** @docrev: revision of the SMBIOS specification */
+   u8 doc_rev;
+   /** @entry_point_rev: revision of the entry point structure */
+   u8 entry_point_rev;
+   /** @reserved: reserved */
+   u8 reserved;
+   /** maximum size of SMBIOS table */
+   u32 max_struct_size;
+   /** @struct_table_address: 64-bit physical starting address */
+   u64 struct_table_address;
+};
+
 /* BIOS characteristics */
 #define BIOS_CHARACTERISTICS_PCI_SUPPORTED (1 << 7)
 #define BIOS_CHARACTERISTICS_UPGRADEABLE   (1 << 11)
-- 
2.34.1

[PATCH v4 2/7] smbios: Move the rest of the SMBIOS2 code

[Simon Glass]

Move all of this logic into the else clause, since it will not be used
for SMBIOS3

Signed-off-by: Simon Glass 
Reviewed-by: Heinrich Schuchardt 
---

(no changes since v1)

 lib/smbios.c | 15 +++
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/lib/smbios.c b/lib/smbios.c
index b417f8a9057..eea72670bd9 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -544,9 +544,8 @@ static struct smbios_write_method smbios_write_funcs[] = {
 ulong write_smbios_table(ulong addr)
 {
ofnode parent_node = ofnode_null();
-   struct smbios_entry *se;
+   ulong table_addr, start_addr;
struct smbios_ctx ctx;
-   ulong table_addr;
ulong tables;
int len = 0;
int max_struct_size = 0;
@@ -566,9 +565,7 @@ ulong write_smbios_table(ulong addr)
 
/* 16 byte align the table address */
addr = ALIGN(addr, 16);
-
-   se = map_sysmem(addr, sizeof(struct smbios_entry));
-   memset(se, 0, sizeof(struct smbios_entry));
+   start_addr = addr;
 
addr += sizeof(struct smbios_entry);
addr = ALIGN(addr, 16);
@@ -603,8 +600,11 @@ ulong write_smbios_table(ulong addr)
printf("WARNING: SMBIOS table_address overflow %llx\n",
   (unsigned long long)table_addr);
addr = 0;
-   goto out;
} else {
+   struct smbios_entry *se;
+
+   se = map_sysmem(start_addr, sizeof(struct smbios_entry));
+   memset(se, '\0', sizeof(struct smbios_entry));
memcpy(se->anchor, "_SM_", 4);
se->length = sizeof(struct smbios_entry);
se->major_ver = SMBIOS_MAJOR_VER;
@@ -625,9 +625,8 @@ ulong write_smbios_table(ulong addr)
   isize);
se->checksum = table_compute_checksum(se,
  sizeof(struct smbios_entry));
+   unmap_sysmem(se);
}
-out:
-   unmap_sysmem(se);
 
return addr;
 }
-- 
2.34.1

[PATCH v4 1/7] smbios: Refactor 32-bit code into an else statement

[Simon Glass]

In preparation for adding support for SMBIOS3 move this code into an
else statement. There is no functional change.

Signed-off-by: Simon Glass 
Reviewed-by: Heinrich Schuchardt 
---

(no changes since v1)

 lib/smbios.c | 38 +-
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/lib/smbios.c b/lib/smbios.c
index 45480b01af4..b417f8a9057 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -589,14 +589,6 @@ ulong write_smbios_table(ulong addr)
len += tmp;
}
 
-   memcpy(se->anchor, "_SM_", 4);
-   se->length = sizeof(struct smbios_entry);
-   se->major_ver = SMBIOS_MAJOR_VER;
-   se->minor_ver = SMBIOS_MINOR_VER;
-   se->max_struct_size = max_struct_size;
-   memcpy(se->intermediate_anchor, "_DMI_", 5);
-   se->struct_table_length = len;
-
/*
 * We must use a pointer here so things work correctly on sandbox. The
 * user of this table is not aware of the mapping of addresses to
@@ -612,16 +604,28 @@ ulong write_smbios_table(ulong addr)
   (unsigned long long)table_addr);
addr = 0;
goto out;
+   } else {
+   memcpy(se->anchor, "_SM_", 4);
+   se->length = sizeof(struct smbios_entry);
+   se->major_ver = SMBIOS_MAJOR_VER;
+   se->minor_ver = SMBIOS_MINOR_VER;
+   se->max_struct_size = max_struct_size;
+   memcpy(se->intermediate_anchor, "_DMI_", 5);
+   se->struct_table_length = len;
+
+   se->struct_table_address = table_addr;
+
+   se->struct_count = handle;
+
+   /* calculate checksums */
+   istart = (char *)se + SMBIOS_INTERMEDIATE_OFFSET;
+   isize = sizeof(struct smbios_entry) -
+   SMBIOS_INTERMEDIATE_OFFSET;
+   se->intermediate_checksum = table_compute_checksum(istart,
+  isize);
+   se->checksum = table_compute_checksum(se,
+ sizeof(struct smbios_entry));
}
-   se->struct_table_address = table_addr;
-
-   se->struct_count = handle;
-
-   /* calculate checksums */
-   istart = (char *)se + SMBIOS_INTERMEDIATE_OFFSET;
-   isize = sizeof(struct smbios_entry) - SMBIOS_INTERMEDIATE_OFFSET;
-   se->intermediate_checksum = table_compute_checksum(istart, isize);
-   se->checksum = table_compute_checksum(se, sizeof(struct smbios_entry));
 out:
unmap_sysmem(se);
 
-- 
2.34.1

[PATCH v4 0/7] smbios: Deal with tables beyond 4GB

[Simon Glass]

When the malloc() region extends beyond 4GB on ARM we may end up with
an SMBIOS table in that region.

Add support for writing an SMBIOS3 table, which supports a 64-bit
address.

Note that this problem does not happen on x86 since it requires the
tables to be placed just below 1MB in memory, unless
CONFIG_BLOBLIST_TABLES is enabled.


Changes in v4:
- Bring in this patch from Heinrich's series
- Check the start of the table rather than the end

Changes in v2:
- Check the end of the table rather than the start.
- Add a new patch to correct gd_smbios_start()
- Add a note about why unmap_system() is called

Heinrich Schuchardt (1):
  smbios: SMBIOS 3.0 (64-bit) Entry Point structure

Simon Glass (6):
  smbios: Refactor 32-bit code into an else statement
  smbios: Move the rest of the SMBIOS2 code
  smbios: Use SMBIOS 3.0 to support an address above 4GB
  smbios: Correct gd_smbios_start()
  efi: Use the correct GUID for the SMBIOS table
  smbios: Require the caller to align the SMBIOS table

 include/asm-generic/global_data.h |  2 +-
 include/efi_api.h |  4 ++
 include/smbios.h  | 37 --
 lib/efi_loader/efi_smbios.c   | 14 +++--
 lib/smbios.c  | 85 +++
 5 files changed, 101 insertions(+), 41 deletions(-)

-- 
2.34.1

Re: [RFC PATCH 00/16] Introduce ICSSG Ethernet driver

[MD Danish Anwar]

On 22/12/23 6:13 pm, Roger Quadros wrote:
> 
> On 22/12/2023 12:26, MD Danish Anwar wrote:
>> Hi Roger,
>>
>> On 20/12/23 3:29 pm, Roger Quadros wrote:
>>> Hi,
>>>
>>> On 19/12/2023 12:11, MD Danish Anwar wrote:
 Introduce ICSSG PRUETH support in uboot. The ICSSG driver is used in TI
 AM654 SR2.0.

 The ICSSG PRU Sub-system runs on EMAC firmware. This series Introduces
 support for ICSSG driver in uboot. This series also adds the driver's
 dependencies.

 The ICSSG2 node is added in device tree overlay so that it remains in
 sync with linux kernel.

 The series introduces device tree and config changes and AM65x
 to enable ICSSG driver. The series also enables SPL_LOAD_FIT_APPLY_OVERLAY
 for AM65x in order to load overlay over spl.

 This series has been tested on AM65x SR2.0, and the ICSSG interface is
 able to ping / dhcp and boot kernel using tftp in uboot.

 To use ICSSG2 ethernet, the ICSSG firmware needs to be loaded to PRU RPROC
 cores and RPROC cores need to be booted with the firmware. This step is
 done inside driver in kernel as kernel supports APIs like
 rproc_set_firmware() and rproc_fw_boot(). But as u-boot doesn't have these
 APIs, the same needs to be done via u-boot cmds.

 To make sure icssg-eth works we need to do below steps.

 1. Initialize rproc cores i.e. rproc_init()
 2. Load $firmware_file from partition '1:2' (root) on device (mmc in this
example)
 3. Load the firmware file to rproc cores passing. i.e. rproc_load()
taking rproc_id, loadaddr and file size as arguments.
 4. Start rproc cores. i.e. rproc_start() taking rproc_id as arguments

 The above steps are done by running the below commands at u-boot prompt.

 => setenv start_icssg2 'rproc start 14; rproc start 15; rproc start 16; 
 rproc start 17; rproc start 18; rproc start 19'
 => setenv stop_icssg2 'rproc stop 14; rproc stop 15; rproc stop 16; rproc 
 stop 17; rproc stop 18; rproc stop 19'
 => setenv firmware_dir '/lib/firmware/ti-pruss'
 => setenv get_firmware_mmc 'load mmc ${bootpart} ${loadaddr} 
 ${firmware_dir}/${firmware_file}'

 => setenv init_icssg2 'setenv ethact icssg2-eth; setenv autoload no; rproc 
 init; setenv loadaddr 0x8000; \
 setenv firmware_file am65x-sr2-pru0-prueth-fw.elf; run 
 get_firmware_mmc;  rproc load 14 0x8000 ${filesize}; \
 setenv loadaddr 0x8900; setenv firmware_file 
 am65x-sr2-rtu0-prueth-fw.elf; run get_firmware_mmc; rproc load 15 
 0x8900 ${filesize}; \
 setenv loadaddr 0x9000; setenv firmware_file 
 am65x-sr2-txpru0-prueth-fw.elf; run get_firmware_mmc; rproc load 16 
 0x9000 ${filesize}; \
 setenv loadaddr 0x8000; setenv firmware_file 
 am65x-sr2-pru1-prueth-fw.elf; run get_firmware_mmc; rproc load 17 
 0x8000 ${filesize}; \
 setenv loadaddr 0x8900; setenv firmware_file 
 am65x-sr2-rtu1-prueth-fw.elf; run get_firmware_mmc; rproc load 18 
 0x8900 ${filesize}; \
 setenv loadaddr 0x9000; setenv firmware_file 
 am65x-sr2-txpru1-prueth-fw.elf; run get_firmware_mmc; rproc load 19 
 0x9000 ${filesize}; \
 run start_icssg2;'
>>>
>>> A whole bunch of commands are required to get ethernet functional.
>>> This is not at all user friendly and will be a maintenance nightmare.
>>> What worries me is tracking the 6 different rproc cores and the 6 different 
>>> firmware files to start 1 ethernet device.
>>> This will get even more interesting when we have to deal with different 
>>> ICSSG instances on different boards.
>>>
>>> What is preventing the driver from starting the rproc cores it needs so 
>>> user doesn't need to care about it?
>>> All the necessary information is in the Device tree. At least this is how 
>>> it is done on Linux.
>>>
>>
>> I tried removing the need for these commands and implementing them
>> inside the driver only. I am able to load the firmware from driver using
>> the fs_loader API request_firmware_into_buf(). It requires changes to
>> dt. A DT node called fs-loader needs to be added also CONFIG_FS_LOADER
>> needs to enabled. In the DT node we need to specify the storage media
>> that we are using i.e. mmc, ospi, usb. It's upto user to modify the
>> storage media, the driver will take the media from DT and try to laod
>> firmware from their.
>>
>> For loading firmwares to rproc cores, rproc_load() API is needed. Now
>> this API takes rproc_id, loadaddr and firmware_size as arguments.
>> loadaddr is fixed for all three pru cores. firmware_size is obtained
>> from request_firmware_into_buf() but I couldn't find a way to get the
>> rproc_id. For now based on the ICSSG instance and slice number I am
>> figuring out the rproc_id and passing it to rproc_load() and
>> rproc_start() APIs. Please let me know if you have any other / better
>> way of 

Re: [PATCH v4] fdt: Allow the devicetree to come from a bloblist

[Ilias Apalodimas]

Hi Tom,

On Tue, 26 Dec 2023 at 14:07, Tom Rini  wrote:
>
> On Tue, Dec 26, 2023 at 09:46:25AM +, Simon Glass wrote:
>
> > Standard passage provides for a bloblist to be passed from one firmware
> > phase to the next. That can be used to pass the devicetree along as well.
> > Add an option to support this.
> >
> > Tests for this will be added as part of the Universal Payload work.
> >
> > Signed-off-by: Simon Glass 
> > ---
> > The discussion on this was not resolved and is now important due to the
> > bloblist series from Raymond. So I am sending it again since I believe
> > this is a better starting point than building on OF_BOARD
>
> I really don't like adding another option for "DT is given to us". Why
> isn't adding another enum to fdt_source_t sufficient, and if we have
> bloblist enabled, that will look for and use if found? Maybe some other
> code needs to be restructured and cleaned up too?

Same here. On top of that the bloblist has a few items in there, e.g a
TPM eventlog. What are we going to do? Add a Kconfig for each item?

This has been going back and forth for a while. I've lost count of how
many times I repeated the same proposal, but here it goes again. We
have OF_BOARD and BLOBLIST options. The bloblist and its properties
are scannable at runtime. Can't we use the combination of these 2 can
be used to imply we expect things from a bloblist. If we want to be
stricter in the future and explicitly expect the DT from a bloblist,
we could add a Kconfig option failing the boot if that's missing.

Thanks
/Ilias
>
> --
> Tom

Re: [PATCH 0/8] An effort to bring DT bindings compliance within U-boot

[Sumit Garg]

Hi Tony,

On Wed, 27 Dec 2023 at 10:11, Tony Dinh  wrote:
>
> Hi Sumit
> Hi Simon,
>
> On Tue, Dec 26, 2023 at 2:13 AM Sumit Garg  wrote:
> >
> > Hi Simon,
> >
> > On Tue, 26 Dec 2023 at 15:17, Simon Glass  wrote:
> > >
> > > Hi Sumit,
> > >
> > > On Fri, Dec 22, 2023 at 5:34 AM Sumit Garg  wrote:
> > > >
> > > > On Thu, 21 Dec 2023 at 20:48, Simon Glass  wrote:
> > > > >
> > > > > Hi,
> > > > >
> > > > > On Thu, Dec 21, 2023 at 3:03 PM Tom Rini  wrote:
> > > > > >
> > > > > > On Thu, Dec 14, 2023 at 07:20:55PM +0530, Sumit Garg wrote:
> > > > > >
> > > > > > > Prerquisite
> > > > > > > ---
> > > > > > >
> > > > > > > This patch series requires devicetree-rebasing git repo to be 
> > > > > > > added as a
> > > > > > > subtree to the main U-boot repo via:
> > > > > > >
> > > > > > > $ git subtree add --prefix devicetree-rebasing \
> > > > > > >   
> > > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git
> > > > > > >  \
> > > > > > >   v6.6-dts --squash
> > > > > >
> > > > > > So, I've played with subtree a little and I think this is the right 
> > > > > > way
> > > > > > forward in these cases. If anyone wants to take a look at how this 
> > > > > > works
> > > > > > in practice, take a look at:
> > > > > > https://source.denx.de/u-boot/u-boot/-/commits/WIP/u-boot-with-devicetree-rebasing-since-v6.1/?ref_type=heads
> > > > > > In that tree I started with the v6.1-dts tag, sync'd all the 
> > > > > > configs (to
> > > > > > have an example of a normal commit) and then did a merge of each tag
> > > > > > until v6.6-dts, so provide some history. And git log looks like 
> > > > > > what I
> > > > > > want to see, the squash commit has clear references to what we are
> > > > > > getting and I make a merge commit that says what I did. If you pull 
> > > > > > the
> > > > > > tree and checkout the branch, all the code is right there already,
> > > > > > nothing further to do. Same with tarball releases. The only thing I
> > > > > > don't like is the size growth there, but we'll reclaim some of it 
> > > > > > when
> > > > > > we delete our obsolete bindings, and then obsolete dts files.
> > > > >
> > > > > I spent a bit of time with subtree as well, as part of reviewing this
> > > > > series, using the instructions Sumit provided. It seems OK to me. We
> > > > > have to accept that it adds code and there will be changes/churn, but
> > > > > it is not too different to accepting patches on those files within
> > > > > U-Boot. We will bring in files which U-Boot doesn't use, but U-Boot
> > > > > does support a good proportion of the boards supported by Linux, so I
> > > > > don't see that as a big cost.
> > > > >
> > > > > From my experimentation, subtrees seem to have no impact on buildman,
> > > > > which is great. Am I missing anything?
> > > >
> > > > No it shouldn't cause any regression on existing tools/CI/build
> > > > systems. It is just a version controlled way of importing third party
> > > > source code as a tarball.
> > > >
> > > > >
> > > > > I still worry about the board-level 'switch' between U-Boot DT and
> > > > > upstream ones. I believe that should be at the SoC level instead.
> > > > >
> > > >
> > > > Probably I wasn't clear enough in my earlier replies but this series
> > > > motivates for a SoC level switch only. Patch #7 is essentially a
> > > > switch for Amlogic meson-gxbb SoC and its derived boards.
> > >
> > > OK good...but that's not quite what I mean. You have a fragment like
> > > this in multiple defconfig files:
> > >
> > > +CONFIG_OF_UPSTREAM=y
> > > +CONFIG_DEFAULT_DEVICE_TREE="amlogic/meson-gxbb-odroidc2"
> > >
> > > instead, OF_UPSTREAM should be enabled via 'imply' for the SoC, in the
> > > Kconfig.
> >
> > Okay I got your point. It makes sense to me. I will do that for v3.
> >
> > > We should not have to specify the filename like this. It is
> > > laborious and error-prone.
> >
> > The only differentiation in naming here is just silicon vendor prefix
> > addition (amlogic/ in this case). The reason for this being that I
> > just want to mirror the whole silicon vendor directory from DT
> > rebasing rather than mirroring individual DT files. Given this do you
> > have any better alternatives?
> >
>
> So how do we handle the case where the DTS exists in u-boot, but not
> in Linux? For example, the DTS was developed for u-boot first and has
> not been accepted in the Linux device tree yet. Can we set in the
> board defconfig like:
>
> # CONFIG_OF_UPSTREAM is not set
>
> Would that allow the DTS to remain in arch//dts?

Yeah that is expected until DTS becomes available in the DT rebasing tree.

-Sumit

>
> Thanks,
> Tony

Re: [PATCH 0/8] An effort to bring DT bindings compliance within U-boot

[Tony Dinh]

Hi Sumit
Hi Simon,

On Tue, Dec 26, 2023 at 2:13 AM Sumit Garg  wrote:
>
> Hi Simon,
>
> On Tue, 26 Dec 2023 at 15:17, Simon Glass  wrote:
> >
> > Hi Sumit,
> >
> > On Fri, Dec 22, 2023 at 5:34 AM Sumit Garg  wrote:
> > >
> > > On Thu, 21 Dec 2023 at 20:48, Simon Glass  wrote:
> > > >
> > > > Hi,
> > > >
> > > > On Thu, Dec 21, 2023 at 3:03 PM Tom Rini  wrote:
> > > > >
> > > > > On Thu, Dec 14, 2023 at 07:20:55PM +0530, Sumit Garg wrote:
> > > > >
> > > > > > Prerquisite
> > > > > > ---
> > > > > >
> > > > > > This patch series requires devicetree-rebasing git repo to be added 
> > > > > > as a
> > > > > > subtree to the main U-boot repo via:
> > > > > >
> > > > > > $ git subtree add --prefix devicetree-rebasing \
> > > > > >   
> > > > > > git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git
> > > > > >  \
> > > > > >   v6.6-dts --squash
> > > > >
> > > > > So, I've played with subtree a little and I think this is the right 
> > > > > way
> > > > > forward in these cases. If anyone wants to take a look at how this 
> > > > > works
> > > > > in practice, take a look at:
> > > > > https://source.denx.de/u-boot/u-boot/-/commits/WIP/u-boot-with-devicetree-rebasing-since-v6.1/?ref_type=heads
> > > > > In that tree I started with the v6.1-dts tag, sync'd all the configs 
> > > > > (to
> > > > > have an example of a normal commit) and then did a merge of each tag
> > > > > until v6.6-dts, so provide some history. And git log looks like what I
> > > > > want to see, the squash commit has clear references to what we are
> > > > > getting and I make a merge commit that says what I did. If you pull 
> > > > > the
> > > > > tree and checkout the branch, all the code is right there already,
> > > > > nothing further to do. Same with tarball releases. The only thing I
> > > > > don't like is the size growth there, but we'll reclaim some of it when
> > > > > we delete our obsolete bindings, and then obsolete dts files.
> > > >
> > > > I spent a bit of time with subtree as well, as part of reviewing this
> > > > series, using the instructions Sumit provided. It seems OK to me. We
> > > > have to accept that it adds code and there will be changes/churn, but
> > > > it is not too different to accepting patches on those files within
> > > > U-Boot. We will bring in files which U-Boot doesn't use, but U-Boot
> > > > does support a good proportion of the boards supported by Linux, so I
> > > > don't see that as a big cost.
> > > >
> > > > From my experimentation, subtrees seem to have no impact on buildman,
> > > > which is great. Am I missing anything?
> > >
> > > No it shouldn't cause any regression on existing tools/CI/build
> > > systems. It is just a version controlled way of importing third party
> > > source code as a tarball.
> > >
> > > >
> > > > I still worry about the board-level 'switch' between U-Boot DT and
> > > > upstream ones. I believe that should be at the SoC level instead.
> > > >
> > >
> > > Probably I wasn't clear enough in my earlier replies but this series
> > > motivates for a SoC level switch only. Patch #7 is essentially a
> > > switch for Amlogic meson-gxbb SoC and its derived boards.
> >
> > OK good...but that's not quite what I mean. You have a fragment like
> > this in multiple defconfig files:
> >
> > +CONFIG_OF_UPSTREAM=y
> > +CONFIG_DEFAULT_DEVICE_TREE="amlogic/meson-gxbb-odroidc2"
> >
> > instead, OF_UPSTREAM should be enabled via 'imply' for the SoC, in the
> > Kconfig.
>
> Okay I got your point. It makes sense to me. I will do that for v3.
>
> > We should not have to specify the filename like this. It is
> > laborious and error-prone.
>
> The only differentiation in naming here is just silicon vendor prefix
> addition (amlogic/ in this case). The reason for this being that I
> just want to mirror the whole silicon vendor directory from DT
> rebasing rather than mirroring individual DT files. Given this do you
> have any better alternatives?
>

So how do we handle the case where the DTS exists in u-boot, but not
in Linux? For example, the DTS was developed for u-boot first and has
not been accepted in the Linux device tree yet. Can we set in the
board defconfig like:

# CONFIG_OF_UPSTREAM is not set

Would that allow the DTS to remain in arch//dts?

Thanks,
Tony

Re: [PATCH v3 3/4] net: tftp: remove explicit efi configuration dependency

[AKASHI Takahiro]

Hi Simon,

On Tue, Dec 26, 2023 at 09:47:03AM +, Simon Glass wrote:
> Hi Heinrich,
> 
> On Wed, Dec 20, 2023 at 9:17???AM Heinrich Schuchardt  
> wrote:
> >
> >
> >
> > Am 20. Dezember 2023 05:46:16 MEZ schrieb Simon Glass :
> > >Hi,
> > >
> > >On Mon, 18 Dec 2023 at 17:17, AKASHI Takahiro
> > > wrote:
> > >>
> > >> Hi Simon,
> > >>
> > >> On Mon, Dec 18, 2023 at 08:01:46AM -0700, Simon Glass wrote:
> > >> > Hi AKASHI,
> > >> >
> > >> > On Sun, 17 Dec 2023 at 19:39, AKASHI Takahiro
> > >> >  wrote:
> > >> > >
> > >> > > Now it is clear that the feature actually depends on efi interfaces,
> > >> > > not "bootefi" command. efi_set_bootdev() will automatically be 
> > >> > > nullified
> > >> > > if necessary efi component is disabled.
> > >> > >
> > >> > > Signed-off-by: AKASHI Takahiro 
> > >> > > ---
> > >> > >  net/tftp.c | 10 --
> > >> > >  1 file changed, 4 insertions(+), 6 deletions(-)
> > >> > >
> > >> >
> > >> > I have the same comment here as the 'fs' patch.
> > >> >
> > >> > > diff --git a/net/tftp.c b/net/tftp.c
> > >> > > index 88e71e67de35..2e335413492b 100644
> > >> > > --- a/net/tftp.c
> > >> > > +++ b/net/tftp.c
> > >> > > @@ -302,12 +302,10 @@ static void tftp_complete(void)
> > >> > > time_start * 1000, "/s");
> > >> > > }
> > >> > > puts("\ndone\n");
> > >> > > -   if (IS_ENABLED(CONFIG_CMD_BOOTEFI)) {
> > >> >
> > >> > Shouldn't this depend on your new CONFIG? What happens if EFI_LOADER
> > >> > is not enabled?
> > >>
> > >> The trick is in efi_loader.h.
> > >> If EFI_LOADER (more specifically CONFIG_EFI_BINARY_EXEC) is not defined,
> > >> this function gets voided.  See patch#1 in this version.
> > >>
> > >> I took this approach in order not to make users much worried about
> > >> what config be used as they are not familiar with UEFI implementation.
> > >
> > >OK, but we really need to delete this function, so what is the plan
> > >for that? The info that EFI needs should be passed to the bootefi()
> > >function, not set in a global.
> >
> > Hello Simon,
> >
> > Bootstd is not the only way to boot. Please, do not forget the shell.
> >
> > The user loads a file with tftpboot. At some later moment the user executes 
> > bootefi.
> >
> > We need a place where we store the device from which the image was loaded.
> 
> Yes, agreed. See my other reply on that.
> 
> >
> > In future we might have a register of loaded files. But that is beyond the 
> > scope of this patch series.
> 
> I believe we could just record the device and partition number (which
> is itself a device these days, I suppose). Then for EFI can do the
> translation at the start of the bootm cmd if not using bootstd.

Then, what is the difference between "record the device and partition
number (strictly it's not accurate because we also support tftp and others.)
and "call efi_set_bootdev()"?

The latter does the translation on the fly and saves the info
in bootefi_image_path and bootefi_device_path local variables.

I believe that both are essentially the same.

> Reviewed-by: Simon Glass 

Always thank you for your review.

-Takahiro Akashi

> 
> >
> > Best regards
> >
> > Heinrich
> >
> > >
> > >
> > >>
> > >> -Takahiro Akashi
> > >>
> > >> > > -   if (!tftp_put_active)
> > >> > > -   efi_set_bootdev("Net", "", tftp_filename,
> > >> > > -   map_sysmem(tftp_load_addr, 
> > >> > > 0),
> > >> > > -   net_boot_file_size);
> > >> > > -   }
> > >> > > +   if (!tftp_put_active)
> > >> > > +   efi_set_bootdev("Net", "", tftp_filename,
> > >> > > +   map_sysmem(tftp_load_addr, 0),
> > >> > > +   net_boot_file_size);
> > >> > > net_set_state(NETLOOP_SUCCESS);
> > >> > >  }
> > >> > >
> > >> > > --
> > >> > > 2.34.1
> > >
> > >Regards,
> 
> Regards,
> Simon

Re: [PATCH v3 1/4] efi_loader: split unrelated code from efi_bootmgr.c

[AKASHI Takahiro]

On Mon, Dec 25, 2023 at 10:17:06AM +0100, Heinrich Schuchardt wrote:
> On 12/18/23 03:38, AKASHI Takahiro wrote:
> > Some code moved from cmd/bootefi.c is actually necessary only for "bootefi
> > " command (starting an image manually loaded by a user using U-Boot
> > load commands or other methods (like JTAG debugger).
> > 
> > The code will never been opted out as unused code by a compiler which
> > doesn't know how EFI boot manager is implemented. So introduce a new
> > configuration, CONFIG_EFI_BINARY_EXEC, to enforce theem opted out
> > explicitly.
> 
> We build with -ffunction-sections. The linker removes unreferenced
> functions.

Yes, I know but I also think it would be better in terms of readability
and maintainability to add a new config option and separate EFI_BINARY_EXEC
portion from BOOTEFI_BOOTMGR as these two functions share almost nothing
(except efi_install_fdt()).

> > 
> > Signed-off-by: AKASHI Takahiro 
> > ---
> >   boot/Kconfig |   4 +-
> >   cmd/Kconfig  |   6 +-
> >   include/efi_loader.h |  28 +-
> >   lib/efi_loader/Kconfig   |   9 +
> >   lib/efi_loader/efi_bootmgr.c | 493 --
> >   lib/efi_loader/efi_device_path.c |   3 +-
> >   lib/efi_loader/efi_helper.c  | 499 ++-
> 
> We expect that after each patch we can compile the code. This requires
> that the Makefile change is in the same patch as the creation of
> efi_helper.c.

Please remember that efi_helper.c is not a new file.
If you like, as Simon suggested, I will move "499" lines of code
into a new file, efi_boot.c and then add:
   obj-$(CONFIG_EFI_BINARY_EXEC) := efi_boot.o

> >   7 files changed, 529 insertions(+), 513 deletions(-)
> > 
> > diff --git a/boot/Kconfig b/boot/Kconfig
> > index 987ca7314117..8ab7e6f63d34 100644
> > --- a/boot/Kconfig
> > +++ b/boot/Kconfig
> > @@ -523,7 +523,7 @@ config BOOTMETH_EXTLINUX_PXE
> > 
> >   config BOOTMETH_EFILOADER
> > bool "Bootdev support for EFI boot"
> > -   depends on BOOTEFI_BOOTMGR
> > +   depends on EFI_BINARY_EXEC
> 
> Why do we need a symbol CONFIG_EFI_BINARY_EXEC? CONFIG_EFI_LOADER=y
> without the ability to execute an EFI binary makes no sense to me.

It's up to users. It allows them to configure U-Boot with EFI_LOADER and
EFI_BOOTMGR only.

Thanks,
-Takahiro Akashi


> Best regards
> 
> Heinrich
> 
> > default y
> > help
> >   Enables support for EFI boot using bootdevs. This makes the
> > @@ -558,7 +558,7 @@ config BOOTMETH_DISTRO
> > select BOOTMETH_SCRIPT if CMDLINE # E.g. Armbian uses scripts
> > select BOOTMETH_EXTLINUX  # E.g. Debian uses these
> > select BOOTMETH_EXTLINUX_PXE if CMD_PXE && CMD_NET && DM_ETH
> > -   select BOOTMETH_EFILOADER if BOOTEFI_BOOTMGR # E.g. Ubuntu uses this
> > +   select BOOTMETH_EFILOADER if EFI_BINARY_EXEC # E.g. Ubuntu uses this
> > 
> >   config SPL_BOOTMETH_VBE
> > bool "Bootdev support for Verified Boot for Embedded (SPL)"
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index 24bfbe505722..2c993496b70e 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -273,7 +273,7 @@ config CMD_BOOTMETH
> > 
> >   config BOOTM_EFI
> > bool "Support booting UEFI FIT images"
> > -   depends on BOOTEFI_BOOTMGR && CMD_BOOTM && FIT
> > +   depends on EFI_BINARY_EXEC && CMD_BOOTM && FIT
> > default y
> > help
> >   Support booting UEFI FIT images via the bootm command.
> > @@ -365,7 +365,7 @@ config CMD_BOOTEFI
> >   if CMD_BOOTEFI
> >   config CMD_BOOTEFI_BINARY
> > bool "Allow booting an EFI binary directly"
> > -   depends on BOOTEFI_BOOTMGR
> > +   depends on EFI_BINARY_EXEC
> > default y
> > help
> >   Select this option to enable direct execution of binary at 'bootefi'.
> > @@ -395,7 +395,7 @@ config CMD_BOOTEFI_HELLO_COMPILE
> > 
> >   config CMD_BOOTEFI_HELLO
> > bool "Allow booting a standard EFI hello world for testing"
> > -   depends on CMD_BOOTEFI_HELLO_COMPILE
> > +   depends on CMD_BOOTEFI_BINARY && CMD_BOOTEFI_HELLO_COMPILE
> > default y if CMD_BOOTEFI_SELFTEST
> > help
> >   This adds a standard EFI hello world application to U-Boot so that
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index 34e7fbbf1840..484c9fad239f 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -90,11 +90,7 @@ efi_status_t efi_add_runtime_mmio(void *mmio_ptr, u64 
> > len);
> >* back to u-boot world
> >*/
> >   void efi_restore_gd(void);
> > -/* Call this to unset the current device name */
> > -void efi_clear_bootdev(void);
> > -/* Call this to set the current device name */
> > -void efi_set_bootdev(const char *dev, const char *devnr, const char *path,
> > -void *buffer, size_t buffer_size);
> > +
> >   /* Called by networking code to memorize the dhcp ack package */
> >   void efi_net_set_dhcp_ack(void *pkt, int len);
> >   /* Print information about all loaded images */
> > @@ -116,10 

Re: Passing boot logs to Linux?

[Csókás Bence]

On 2023. 12. 26. 19:09, Dragan Simic wrote:
> Hello,
> 
> On 2023-12-26 10:46, Simon Glass wrote:
>> On Thu, Dec 21, 2023 at 2:23 AM Dragan Simic  wrote:
>>> On 2023-12-21 02:44, Dragan Simic wrote:
>>> > On 2023-12-21 02:37, Dragan Simic wrote:
>>> >> On 2023-12-21 02:03, Daniel Golle wrote:
>>> >>> On Thu, Dec 21, 2023 at 12:55:20AM +0100, Dragan Simic wrote:
>>>  On 2023-12-21 00:27, Csókás Bence wrote:
>>>  > Not every system has eMMC/uSD, and as you said, these 
>>> arguments don't
>>>  > hold for a 4 MB SPI NAND, for example, one you might find in 
>>> an OpenWrt
>>>  > router for example. Whereas RAM is quite cheap nowadays.
>>> 
>>>  I see, but I also wonder how many such OpenWrt routers are still
>>>  used these
>>>  days, and, even more importantly, how many of them are regularly
>>>  updated and
>>>  can be expected to actually use this new feature?
>>> >>>
>>> >>> Avoid flash writes is a very important matter, even on systems with
>>> >>> 128 MiB of SPI-NAND flash which is by far the most common setup you
>>> >>> find on off-the-shelf plastic routers and access points nowadays.
>>> >>
>>> >> I agree, writing something to the SPI chips all the time, no matter
>>> >> how small the writes are, is a big no-no, which I already clearly
>>> >> expressed in one of my earlier posts.
>>> >>
>>> >>> Especially also as those devices often come without a local console,
>>> >>> having U-Boot's output prepended to dmesg on boot would be a very 
>>> big
>>> >>> win.
>>> >>
>>> >> I was also thinking about that, but I'm not sure it would be accepted
>>> >> to the Linux kernel.  Maybe we can try getting that accepted later.
>>>
>>> Maybe, but just _maybe,_ it could be possible to add a new command-line
>>> option to dmesg(1) that would display the last recorded console output,
>>> fetched from the pstore.  That _might_ get accepted to util-linux, while
>>> being perfectly fine from the usability standpoint.
>>>
>>> I'm also willing to work on that, and I already contributed a few
>>> dmesg(1) patches to util-linux.
>>>
>>>  Please, don't get me wrong, I still support having both options
>>>  available,
>>>  but I'm also wondering about the target demographic.
>>> 
>>>  > > > Plus, I don't want the console subsystem to depend on any 
>>> file/disk
>>>  > > > operations/drivers.
>>>  > >
>>>  > > Well, the console would still work as usual even if logging 
>>> to disk
>>>  > > would fail for any reason, which is similar to the serial 
>>> console
>>>  > > still
>>>  > > working if the graphical console fails.  Moreover, if the 
>>> disk fails,
>>>  > > the system isn't be able to boot, so any RAM-based console logs
>>>  > > would be
>>>  > > lost in that case.  All this makes the RAM-based logging no 
>>> more
>>>  > > resilient to disk failures.
>>>  >
>>>  > Correction: if disk *reads* fail, as well as writes, then the 
>>> system
>>>  > will not boot. However, typical failure of Flash media is that it
>>>  > becomes read-only.
>>> 
>>>  That's a good point, but having a read-only root filesystem usually
>>>  also
>>>  means having a non-operational system that can only have its stored
>>>  data
>>>  salvaged.  Unless the system is specifically crafted to survive 
>>> such
>>>  scenarios, of course.
>>> >>>
>>> >>> ... which holds true for any decent embedded OS, which at least
>>> >>> allows
>>> >>> limited remote access and some kind of recovery even in this
>>> >>> situation.
>>> >>
>>> >> Perhaps.  I'm more into running general-purpose Linux 
>>> distributions on
>>> >> single-board computers and derived embedded devices, which are on the
>>> >> "thick" end of the embedded device spectrum, so to speak.
>>> >>
>>>  > > I still think that using disk-based pstore is a better 
>>> option.  Just
>>>  > > as
>>>  > > you don't want to wear out your flash disks with 30-40 KB of 
>>> data, I
>>>  > > also don't want to waste 30-40 KB of RAM.
>>>  >
>>>  > As I said, you could just unload the log after you're done 
>>> processing
>>>  > it. 40 KB RAM is less, than what `sshd` uses, for instance 
>>> (860k on my
>>>  > laptop, but it can probably be less, maybe even 10x less, so 
>>> 80-90k?),
>>>  > so you could, in your init, process the in-RAM log, then 
>>> unload it, then
>>>  > start your other services, thereby reclaiming that RAM.
>>> 
>>>  Using pstore should have that unloading already covered, and the
>>>  already
>>>  existing systemd service is there to perform the archiving to the
>>>  primary
>>>  filesystem, if desired so.  It would all need to be tested in
>>>  detail, of
>>>  course.
>>> >>>
>>> >>> pstore/ramoops uses a statically assigned reserved memory region, so
>>> >>> in
>>> >>> the moment you want to use that feature you loose that amount of RAM
>>> >>> (a
>>> >>> few kB, so it 

Re: [PATCH] arm: apple: Disable SMBIOS again

[Tom Rini]

On Sun, 24 Dec 2023 19:12:55 +0100, Mark Kettenis wrote:

> Apple machines do not have memory below 4G.  Since U-Boot does
> not support the SMBIOS 3 header structures this means we can't
> support SMBIOS on these machines.  Unfortunately the refactoring
> of the SMBIOS code this cycle accidentally enabled it again.
> 
> 

Applied to u-boot/master, thanks!

-- 
Tom

Re: [PATCH v1] spi: npcm_pspi: Fix the wrong clock divider calculation

[Tom Rini]

On Tue, 26 Dec 2023 10:00:06 +0800, Jim Liu wrote:

> Fix the wrong clock divider calculation.
> 
> 

Applied to u-boot/master, thanks!

-- 
Tom

Re: Passing boot logs to Linux?

[Dragan Simic]

Hello,

On 2023-12-26 10:46, Simon Glass wrote:
On Thu, Dec 21, 2023 at 2:23 AM Dragan Simic  
wrote:

On 2023-12-21 02:44, Dragan Simic wrote:
> On 2023-12-21 02:37, Dragan Simic wrote:
>> On 2023-12-21 02:03, Daniel Golle wrote:
>>> On Thu, Dec 21, 2023 at 12:55:20AM +0100, Dragan Simic wrote:
 On 2023-12-21 00:27, Csókás Bence wrote:
 > Not every system has eMMC/uSD, and as you said, these arguments don't
 > hold for a 4 MB SPI NAND, for example, one you might find in an OpenWrt
 > router for example. Whereas RAM is quite cheap nowadays.

 I see, but I also wonder how many such OpenWrt routers are still
 used these
 days, and, even more importantly, how many of them are regularly
 updated and
 can be expected to actually use this new feature?
>>>
>>> Avoid flash writes is a very important matter, even on systems with
>>> 128 MiB of SPI-NAND flash which is by far the most common setup you
>>> find on off-the-shelf plastic routers and access points nowadays.
>>
>> I agree, writing something to the SPI chips all the time, no matter
>> how small the writes are, is a big no-no, which I already clearly
>> expressed in one of my earlier posts.
>>
>>> Especially also as those devices often come without a local console,
>>> having U-Boot's output prepended to dmesg on boot would be a very big
>>> win.
>>
>> I was also thinking about that, but I'm not sure it would be accepted
>> to the Linux kernel.  Maybe we can try getting that accepted later.

Maybe, but just _maybe,_ it could be possible to add a new 
command-line
option to dmesg(1) that would display the last recorded console 
output,
fetched from the pstore.  That _might_ get accepted to util-linux, 
while

being perfectly fine from the usability standpoint.

I'm also willing to work on that, and I already contributed a few
dmesg(1) patches to util-linux.

 Please, don't get me wrong, I still support having both options
 available,
 but I'm also wondering about the target demographic.

 > > > Plus, I don't want the console subsystem to depend on any file/disk
 > > > operations/drivers.
 > >
 > > Well, the console would still work as usual even if logging to disk
 > > would fail for any reason, which is similar to the serial console
 > > still
 > > working if the graphical console fails.  Moreover, if the disk fails,
 > > the system isn't be able to boot, so any RAM-based console logs
 > > would be
 > > lost in that case.  All this makes the RAM-based logging no more
 > > resilient to disk failures.
 >
 > Correction: if disk *reads* fail, as well as writes, then the system
 > will not boot. However, typical failure of Flash media is that it
 > becomes read-only.

 That's a good point, but having a read-only root filesystem usually
 also
 means having a non-operational system that can only have its stored
 data
 salvaged.  Unless the system is specifically crafted to survive such
 scenarios, of course.
>>>
>>> ... which holds true for any decent embedded OS, which at least
>>> allows
>>> limited remote access and some kind of recovery even in this
>>> situation.
>>
>> Perhaps.  I'm more into running general-purpose Linux distributions on
>> single-board computers and derived embedded devices, which are on the
>> "thick" end of the embedded device spectrum, so to speak.
>>
 > > I still think that using disk-based pstore is a better option.  Just
 > > as
 > > you don't want to wear out your flash disks with 30-40 KB of data, I
 > > also don't want to waste 30-40 KB of RAM.
 >
 > As I said, you could just unload the log after you're done processing
 > it. 40 KB RAM is less, than what `sshd` uses, for instance (860k on my
 > laptop, but it can probably be less, maybe even 10x less, so 80-90k?),
 > so you could, in your init, process the in-RAM log, then unload it, then
 > start your other services, thereby reclaiming that RAM.

 Using pstore should have that unloading already covered, and the
 already
 existing systemd service is there to perform the archiving to the
 primary
 filesystem, if desired so.  It would all need to be tested in
 detail, of
 course.
>>>
>>> pstore/ramoops uses a statically assigned reserved memory region, so
>>> in
>>> the moment you want to use that feature you loose that amount of RAM
>>> (a
>>> few kB, so it doesn't really matter on modern systems).
>>> As in: there is *no* dynamic allocation.
>>>
>>> Imho using pstore/ramoops (which is a more or less Linux-specific
>>> debugging feature, meant to store one or more timestamped logs of
>>> crashes) might not be the most suitable choice. I understand the
>>> advantages of using existing infrastructure, but on the other hand
>>> we don't need most of the complexity of pstore for the task.
>>
>> Hmm, let me research all that a bit more in the next couple of days,
>> please, 

[PATCH v2] rockchip: rk35xx: expand space for decompressed kernel

[Hugh Cole-Baker]

An uncompressed 6.7.0-rc1 Linux kernel Image built with the arm64
defconfig is about 40MB. This does not fit in to the space between
kernel_comp_addr_r and fdt_addr_r, so when uncompressing an Image.gz
to this size, the FDT will be overwritten. Rearrange addresses to have
128MiB for the kernel and its decompression buffer, then devicetree,
overlay and ramdisk at the end.

Signed-off-by: Hugh Cole-Baker 
---
Changes from v1:
Reorder things to have to have 128MiB for kernel and decompression space,
then devicetree, overlays and ramdisk.
Update kernel_comp_size to 128MiB.

 include/configs/rk3568_common.h | 12 ++--
 include/configs/rk3588_common.h | 12 ++--
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/include/configs/rk3568_common.h b/include/configs/rk3568_common.h
index 1b7d3437b1..48f93083de 100644
--- a/include/configs/rk3568_common.h
+++ b/include/configs/rk3568_common.h
@@ -20,12 +20,12 @@
"script_offset_f=0xffe000\0"\
"script_size_f=0x2000\0"\
"pxefile_addr_r=0x00e0\0"   \
-   "fdt_addr_r=0x0a10\0"   \
-   "fdtoverlay_addr_r=0x0200\0"\
-   "kernel_addr_r=0x0208\0"\
-   "ramdisk_addr_r=0x0a20\0"   \
-   "kernel_comp_addr_r=0x0800\0"   \
-   "kernel_comp_size=0x200\0"
+   "kernel_addr_r=0x0200\0"\
+   "kernel_comp_addr_r=0x0a00\0"   \
+   "fdt_addr_r=0x1200\0"   \
+   "fdtoverlay_addr_r=0x1210\0"\
+   "ramdisk_addr_r=0x1218\0"   \
+   "kernel_comp_size=0x800\0"
 
 #define CFG_EXTRA_ENV_SETTINGS \
ENV_MEM_LAYOUT_SETTINGS \
diff --git a/include/configs/rk3588_common.h b/include/configs/rk3588_common.h
index 46389d087d..70430612ef 100644
--- a/include/configs/rk3588_common.h
+++ b/include/configs/rk3588_common.h
@@ -19,12 +19,12 @@
"script_offset_f=0xffe000\0"\
"script_size_f=0x2000\0"\
"pxefile_addr_r=0x00e0\0"   \
-   "fdt_addr_r=0x0a10\0"   \
-   "fdtoverlay_addr_r=0x0200\0"\
-   "kernel_addr_r=0x0208\0"\
-   "ramdisk_addr_r=0x0a20\0"   \
-   "kernel_comp_addr_r=0x0800\0"   \
-   "kernel_comp_size=0x200\0"
+   "kernel_addr_r=0x0200\0"\
+   "kernel_comp_addr_r=0x0a00\0"   \
+   "fdt_addr_r=0x1200\0"   \
+   "fdtoverlay_addr_r=0x1210\0"\
+   "ramdisk_addr_r=0x1218\0"   \
+   "kernel_comp_size=0x800\0"
 
 #define CFG_EXTRA_ENV_SETTINGS \
"fdtfile=" CONFIG_DEFAULT_FDT_FILE "\0" \
-- 
2.39.3 (Apple Git-145)

Re: [PATHv3 8/8] Makefile: add dtbs to clean

[Tom Rini]

On Tue, Dec 26, 2023 at 09:46:19PM +0600, Maxim Uvarov wrote:

> CI test checks that generated dtb has to be cleaned up.
> Use the same clean procedure as Linux top level Makefile has.
> 
> Signed-off-by: Maxim Uvarov 

Reviewed-by: Tom Rini 

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v9 7/8] config: xea: Enable the SPL_FIT config option

[Tom Rini]

On Tue, Dec 26, 2023 at 05:14:52PM +0530, Venkatesh Yadav Abbarapu wrote:

> Enable the config SPL_FIT, as getting the below error
> ../arm-xilinx-linux-gnueabi-ld.bfd.real: drivers/core/ofnode.o: in function
> `ofnode_get_property':
> /u-boot/drivers/core/ofnode.c:1185: undefined reference to `fdt_getprop'
> make[1]: *** [scripts/Makefile.spl:527: spl/u-boot-spl] Error 1
> 
> Signed-off-by: Venkatesh Yadav Abbarapu 
> ---
>  configs/imx28_xea_defconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

This isn't a problem today and is a side effect of changes you're making
elsewhere in the series. Please figure out what that is, and then work
to rework things so that platforms like this one which don't (and can't
ever) make use of the new functionality don't see much in the way of
growth, thanks.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v9 8/8] axm/taurus: Disable the SPL_SPI config

[Tom Rini]

On Tue, Dec 26, 2023 at 05:14:53PM +0530, Venkatesh Yadav Abbarapu wrote:

> Disabling the CONFIG_SPL_SPI for the axm and taurus defconfigs,
> as facing the SPL size issue "SPL image too big".
> 
> Signed-off-by: Venkatesh Yadav Abbarapu 
> ---
>  board/siemens/taurus/taurus.c | 4 +++-
>  configs/axm_defconfig | 7 ++-
>  configs/taurus_defconfig  | 7 ++-
>  3 files changed, 7 insertions(+), 11 deletions(-)
> 
> diff --git a/board/siemens/taurus/taurus.c b/board/siemens/taurus/taurus.c
> index ad44a7c0d2..ac1ce8acf4 100644
> --- a/board/siemens/taurus/taurus.c
> +++ b/board/siemens/taurus/taurus.c
> @@ -135,11 +135,12 @@ void spl_board_init(void)
>  
>   /* check for recovery mode */
>   if (at91_is_recovery() == 1) {
> - struct spi_flash *flash;
>  
>   puts("Recovery button pressed\n");
>   nand_init();
>   spl_nand_erase_one(0, 0);
> +#if defined(CONFIG_SPL_SPI)
> + struct spi_flash *flash;
>   flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS,
>   0,
>   CONFIG_SF_DEFAULT_SPEED,
> @@ -151,6 +152,7 @@ void spl_board_init(void)
>   spi_flash_erase(flash, 0,
>   CFG_SYS_NAND_U_BOOT_SIZE);
>   }
> +#endif
>   }
>  }
>  
> diff --git a/configs/axm_defconfig b/configs/axm_defconfig
> index e1a01b24b8..4d439832d7 100644
> --- a/configs/axm_defconfig
> +++ b/configs/axm_defconfig
> @@ -3,7 +3,6 @@ CONFIG_SKIP_LOWLEVEL_INIT_ONLY=y
>  CONFIG_SPL_SKIP_LOWLEVEL_INIT_ONLY=y
>  CONFIG_SPL_SYS_DCACHE_OFF=y
>  CONFIG_ARCH_CPU_INIT=y
> -CONFIG_SYS_THUMB_BUILD=y
>  # CONFIG_SPL_USE_ARCH_MEMCPY is not set
>  # CONFIG_SPL_USE_ARCH_MEMSET is not set
>  CONFIG_ARCH_AT91=y
> @@ -26,14 +25,14 @@ CONFIG_SPL=y
>  CONFIG_DEBUG_UART_BASE=0xf200
>  CONFIG_DEBUG_UART_CLOCK=18432000
>  CONFIG_ENV_OFFSET_REDUND=0x18
> -CONFIG_SPL_SPI_FLASH_SUPPORT=y
> -CONFIG_SPL_SPI=y
>  CONFIG_SYS_LOAD_ADDR=0x2200
>  CONFIG_DEBUG_UART=y
>  CONFIG_NAND_BOOT=y
>  CONFIG_BOOTDELAY=3
>  CONFIG_USE_BOOTCOMMAND=y
>  CONFIG_BOOTCOMMAND="run flash_self"
> +CONFIG_SYS_CBSIZE=256
> +CONFIG_SYS_PBSIZE=281
>  # CONFIG_DISPLAY_BOARDINFO is not set
>  CONFIG_BOARD_EARLY_INIT_F=y
>  CONFIG_SPL_MAX_SIZE=0x3e00
> @@ -57,8 +56,6 @@ CONFIG_SPL_NAND_BASE=y
>  CONFIG_SPL_DM_SPI_FLASH=y
>  CONFIG_HUSH_PARSER=y
>  CONFIG_SYS_PROMPT="U-Boot> "
> -CONFIG_SYS_CBSIZE=256
> -CONFIG_SYS_PBSIZE=281
>  # CONFIG_CMD_BDI is not set
>  CONFIG_CMD_BOOTZ=y
>  # CONFIG_CMD_IMI is not set
> diff --git a/configs/taurus_defconfig b/configs/taurus_defconfig
> index 9be30c8d8a..7d71262d1a 100644
> --- a/configs/taurus_defconfig
> +++ b/configs/taurus_defconfig
> @@ -4,7 +4,6 @@ CONFIG_SPL_SKIP_LOWLEVEL_INIT_ONLY=y
>  CONFIG_SPL_SYS_ICACHE_OFF=y
>  CONFIG_SPL_SYS_DCACHE_OFF=y
>  CONFIG_ARCH_CPU_INIT=y
> -CONFIG_SYS_THUMB_BUILD=y
>  # CONFIG_SPL_USE_ARCH_MEMCPY is not set
>  # CONFIG_SPL_USE_ARCH_MEMSET is not set
>  CONFIG_ARCH_AT91=y
> @@ -28,8 +27,6 @@ CONFIG_SPL=y
>  CONFIG_DEBUG_UART_BASE=0xf200
>  CONFIG_DEBUG_UART_CLOCK=18432000
>  CONFIG_ENV_OFFSET_REDUND=0x18
> -CONFIG_SPL_SPI_FLASH_SUPPORT=y
> -CONFIG_SPL_SPI=y
>  CONFIG_SYS_LOAD_ADDR=0x2200
>  CONFIG_DEBUG_UART=y
>  CONFIG_NAND_BOOT=y
> @@ -38,6 +35,8 @@ CONFIG_USE_BOOTARGS=y
>  CONFIG_BOOTARGS="console=ttyS0,115200 earlyprintk 
> mtdparts=atmel_nand:256k(bootstrap)ro,512k(uboot)ro,256k(env),256k(env_redundant),256k(spare),512k(dtb),6M(kernel)ro,-(rootfs)
>  root=/dev/mtdblock7 rw rootfstype=jffs2"
>  CONFIG_USE_BOOTCOMMAND=y
>  CONFIG_BOOTCOMMAND="nand read 0x2200 0x20 0x30; bootm"
> +CONFIG_SYS_CBSIZE=256
> +CONFIG_SYS_PBSIZE=281
>  # CONFIG_DISPLAY_BOARDINFO is not set
>  CONFIG_BOARD_EARLY_INIT_F=y
>  CONFIG_SPL_MAX_SIZE=0x3e00
> @@ -61,8 +60,6 @@ CONFIG_SPL_NAND_BASE=y
>  CONFIG_SPL_DM_SPI_FLASH=y
>  CONFIG_HUSH_PARSER=y
>  CONFIG_SYS_PROMPT="U-Boot> "
> -CONFIG_SYS_CBSIZE=256
> -CONFIG_SYS_PBSIZE=281
>  # CONFIG_SYS_XTRACE is not set
>  # CONFIG_CMD_BDI is not set
>  CONFIG_CMD_BOOTZ=y

This patch seems wrong. You're disabling thumb (and thumb in SPL) which
increases size and then removing specifically enabled functionality.
Please figure out how to add the functionality you're wanting to add,
but allow it to be disabled so that we have minimal (or if possible, no)
size growth on platforms that can't and won't use that functionality,
thanks.

-- 
Tom


signature.asc
Description: PGP signature

[PATHv3 8/8] Makefile: add dtbs to clean

[Maxim Uvarov]

CI test checks that generated dtb has to be cleaned up.
Use the same clean procedure as Linux top level Makefile has.

Signed-off-by: Maxim Uvarov 
---
 Makefile | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Makefile b/Makefile
index 220411a293..b443831a93 100644
--- a/Makefile
+++ b/Makefile
@@ -2195,6 +2195,8 @@ clean: $(clean-dirs)
@find $(if $(KBUILD_EXTMOD), $(KBUILD_EXTMOD), .) $(RCS_FIND_IGNORE) \
\( -name '*.[oas]' -o -name '*.ko' -o -name '.*.cmd' \
-o -name '*.ko.*' -o -name '*.su' -o -name '*.pyc' \
+   -o -name '*.dtb' -o -name '*.dtbo' \
+   -o -name '*.dtb.S' -o -name '*.dtbo.S' \
-o -name '.*.d' -o -name '.*.tmp' -o -name '*.mod.c' \
-o -name '*.lex.c' -o -name '*.tab.[ch]' \
-o -name '*.asn1.[ch]' \
-- 
2.30.2

[PATHv3 7/8] omap3: use device specific naming for mem_init

[Maxim Uvarov]

Use device specific naming for functions so as to not overlap
with common function names.

Signed-off-by: Maxim Uvarov 
Reviewed-by: Tom Rini 
---
 arch/arm/include/asm/arch-omap3/mem.h | 2 +-
 arch/arm/mach-omap2/omap3/board.c | 2 +-
 arch/arm/mach-omap2/omap3/emif4.c | 4 ++--
 arch/arm/mach-omap2/omap3/sdrc.c  | 6 +++---
 4 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/arm/include/asm/arch-omap3/mem.h 
b/arch/arm/include/asm/arch-omap3/mem.h
index 569779c55e..fce3568eca 100644
--- a/arch/arm/include/asm/arch-omap3/mem.h
+++ b/arch/arm/include/asm/arch-omap3/mem.h
@@ -475,7 +475,7 @@ enum {
 #ifndef __ASSEMBLY__
 
 /* Function prototypes */
-void mem_init(void);
+void omap3_mem_init(void);
 
 u32 is_mem_sdr(void);
 u32 mem_ok(u32 cs);
diff --git a/arch/arm/mach-omap2/omap3/board.c 
b/arch/arm/mach-omap2/omap3/board.c
index 8b70251457..c76a95dd5d 100644
--- a/arch/arm/mach-omap2/omap3/board.c
+++ b/arch/arm/mach-omap2/omap3/board.c
@@ -216,7 +216,7 @@ void s_init(void)
 void board_init_f(ulong dummy)
 {
early_system_init();
-   mem_init();
+   omap3_mem_init();
/*
* Save the boot parameters passed from romcode.
* We cannot delay the saving further than this,
diff --git a/arch/arm/mach-omap2/omap3/emif4.c 
b/arch/arm/mach-omap2/omap3/emif4.c
index 7e5a281922..4fbfb387ab 100644
--- a/arch/arm/mach-omap2/omap3/emif4.c
+++ b/arch/arm/mach-omap2/omap3/emif4.c
@@ -159,10 +159,10 @@ int dram_init_banksize(void)
 }
 
 /*
- * mem_init() -
+ * omap3_mem_init() -
  *  - Initialize memory subsystem
  */
-void mem_init(void)
+void omap3_mem_init(void)
 {
do_emif4_init();
 }
diff --git a/arch/arm/mach-omap2/omap3/sdrc.c b/arch/arm/mach-omap2/omap3/sdrc.c
index 5d43e7c9cf..4d27d82c78 100644
--- a/arch/arm/mach-omap2/omap3/sdrc.c
+++ b/arch/arm/mach-omap2/omap3/sdrc.c
@@ -4,7 +4,7 @@
  *
  * This file has been created after exctracting and consolidating
  * the SDRC related content from mem.c and board.c, also created
- * generic init function (mem_init).
+ * generic init function (omap3_mem_init).
  *
  * Copyright (C) 2004-2010
  * Texas Instruments Incorporated - https://www.ti.com/
@@ -232,11 +232,11 @@ int dram_init_banksize(void)
 }
 
 /*
- * mem_init -
+ * omap3_mem_init -
  *  - Init the sdrc chip,
  *  - Selects CS0 and CS1,
  */
-void mem_init(void)
+void omap3_mem_init(void)
 {
/* only init up first bank here */
do_sdrc_init(CS0, EARLY_INIT);
-- 
2.30.2

[PATHv3 6/8] bcm_ns3: use device specific naming for variables

[Maxim Uvarov]

Use device specific naming for variables so as to not overlap
with common function names.

Signed-off-by: Maxim Uvarov 
Reviewed-by: Tom Rini 
Reviewed-by: Peter Robinson 
---
 include/configs/bcm_ns3.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/configs/bcm_ns3.h b/include/configs/bcm_ns3.h
index 47de4bc201..7c6e0725a6 100644
--- a/include/configs/bcm_ns3.h
+++ b/include/configs/bcm_ns3.h
@@ -45,11 +45,11 @@
 #define PCIE_ARGS "pcie_args=pci=pcie_bus_safe pcie_ports=native 
vfio_pci.disable_idle_d3=1\0"
 
 #ifdef CONFIG_BCM_SF2_ETH
-#define ETH_ADDR "ethaddr=00:0A:F7:95:65:A4\0"
+#define BCM_ETH_ADDR "ethaddr=00:0A:F7:95:65:A4\0"
 #define NET_ARGS "bgmac_platform.ethaddr=${ethaddr} " \
"ip=${ipaddr}::${gatewayip}:${netmask}::${ethif}:off"
 #else
-#define ETH_ADDR
+#define BMC_ETH_ADDR
 #define NET_ARGS
 #endif
 
@@ -749,7 +749,7 @@
OS_LOG_LEVEL \
EXTRA_ARGS \
PCIE_ARGS \
-   ETH_ADDR \
+   BMC_ETH_ADDR \
RESERVED_MEM \
SETBOOTARGS \
UPDATEME_FLASH_PARAMS \
-- 
2.30.2

[PATHv3 5/8] mach-socfpga: do not overlap defines with lwip

[Maxim Uvarov]

Fix compilation issue with overlapping lwip and march defines.

Signed-off-by: Maxim Uvarov 
Reviewed-by: Simon Glass 
---
 arch/arm/mach-socfpga/include/mach/handoff_soc64.h | 6 --
 arch/arm/mach-socfpga/wrap_handoff_soc64.c | 9 +
 2 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/arch/arm/mach-socfpga/include/mach/handoff_soc64.h 
b/arch/arm/mach-socfpga/include/mach/handoff_soc64.h
index 902fc6bfb5..9b85e5865b 100644
--- a/arch/arm/mach-socfpga/include/mach/handoff_soc64.h
+++ b/arch/arm/mach-socfpga/include/mach/handoff_soc64.h
@@ -78,12 +78,6 @@
 
 #ifndef __ASSEMBLY__
 #include 
-enum endianness {
-   LITTLE_ENDIAN = 0,
-   BIG_ENDIAN,
-   UNKNOWN_ENDIANNESS
-};
-
 int socfpga_get_handoff_size(void *handoff_address);
 int socfpga_handoff_read(void *handoff_address, void *table, u32 table_len);
 #endif
diff --git a/arch/arm/mach-socfpga/wrap_handoff_soc64.c 
b/arch/arm/mach-socfpga/wrap_handoff_soc64.c
index e7cb5ea89c..df0701ec85 100644
--- a/arch/arm/mach-socfpga/wrap_handoff_soc64.c
+++ b/arch/arm/mach-socfpga/wrap_handoff_soc64.c
@@ -10,6 +10,15 @@
 #include 
 #include "log.h"
 
+#ifndef __ASSEMBLY__
+#include 
+enum endianness {
+   LITTLE_ENDIAN = 0,
+   BIG_ENDIAN,
+   UNKNOWN_ENDIANNESS
+};
+#endif
+
 static enum endianness check_endianness(u32 handoff)
 {
switch (handoff) {
-- 
2.30.2

[PATHv3 4/8] driver/net/rtl8139: remove debug print

[Maxim Uvarov]

debug print delays reset of the driver. Finally I see
bunch of "rx error " errors in the screen. CI can
not handle many prints. While network works fine there

Reproduced with:
make CROSS_COMPILE=sh2-linux- r2dplus_defconfig all
qemu-system-sh4 -M r2d -nographic -serial null \
-serial mon:stdio -net user,tftp=`pwd` \
-net nic,model=rtl8139 -kernel ./u-boot.bin

Signed-off-by: Maxim Uvarov 
Reviewed-by: Simon Glass 
---
 drivers/net/rtl8139.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/rtl8139.c b/drivers/net/rtl8139.c
index 106bc1c7ae..d8f24ec81a 100644
--- a/drivers/net/rtl8139.c
+++ b/drivers/net/rtl8139.c
@@ -453,7 +453,7 @@ static int rtl8139_recv_common(struct rtl8139_priv *priv, 
unsigned char *rxdata,
  RTL_STS_RXBADALIGN)) ||
(rx_size < ETH_ZLEN) ||
(rx_size > ETH_FRAME_LEN + 4)) {
-   printf("rx error %hX\n", rx_status);
+   debug("rx error %hX\n", rx_status);
/* this clears all interrupts still pending */
rtl8139_reset(priv);
return 0;
-- 
2.30.2

[PATHv3 3/8] sandbox: eth-raw-os: successful return code is 0

[Maxim Uvarov]

all network drivers return 0 on the successful
transmission.

Signed-off-by: Maxim Uvarov 
Reviewed-by: Simon Glass 
---
 arch/sandbox/cpu/eth-raw-os.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/sandbox/cpu/eth-raw-os.c b/arch/sandbox/cpu/eth-raw-os.c
index e59b96be5f..92c35aed95 100644
--- a/arch/sandbox/cpu/eth-raw-os.c
+++ b/arch/sandbox/cpu/eth-raw-os.c
@@ -256,7 +256,7 @@ int sandbox_eth_raw_os_send(void *packet, int length,
   strerror(errno));
return -errno;
}
-   return retval;
+   return 0;
 }
 
 int sandbox_eth_raw_os_recv(void *packet, int *length,
-- 
2.30.2

[PATHv3 2/8] net/smc911x: fix return from smc911x_send

[Maxim Uvarov]

return value of smc911x_send is ignored, but on sucesseful
send we need return 0 and or error -ETIMEOUT, not opposite.

Signed-off-by: Maxim Uvarov 
Reviewed-by: Tom Rini 
Reviewed-by: Simon Glass 
---
 drivers/net/smc911x.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/smc911x.c b/drivers/net/smc911x.c
index 5c5ad8b84a..616b7ce174 100644
--- a/drivers/net/smc911x.c
+++ b/drivers/net/smc911x.c
@@ -403,7 +403,7 @@ static int smc911x_send(struct udevice *dev, void *packet, 
int length)
 
ret = smc911x_send_common(priv, packet, length);
 
-   return ret ? 0 : -ETIMEDOUT;
+   return ret ? -ETIMEDOUT : 0;
 }
 
 static int smc911x_recv(struct udevice *dev, int flags, uchar **packetp)
-- 
2.30.2

[PATHv3 1/8] test_net: print out net list

[Maxim Uvarov]

Printing net list is useful in CI log files.

Signed-off-by: Maxim Uvarov 
Reviewed-by: Simon Glass 
---
 test/py/tests/test_net.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/test/py/tests/test_net.py b/test/py/tests/test_net.py
index b2241ae6a4..cd5b791a6a 100644
--- a/test/py/tests/test_net.py
+++ b/test/py/tests/test_net.py
@@ -96,6 +96,8 @@ def test_net_pre_commands(u_boot_console):
 if init_pci:
 u_boot_console.run_command('pci enum')
 
+u_boot_console.run_command('net list')
+
 @pytest.mark.buildconfigspec('cmd_dhcp')
 def test_net_dhcp(u_boot_console):
 """Test the dhcp command.
-- 
2.30.2

[PATHv3 0/8] net fixes prior lwip

[Maxim Uvarov]

Add small net fixes prior lwip patches.

v3: - some small review fixes.
v2: - rebase on the top of master tree with descriptions fixes
  suggested in review comments.
Maxim Uvarov (8):
  test_net: print out net list
  net/smc911x: fix return from smc911x_send
  sandbox: eth-raw-os: successful return code is 0
  driver/net/rtl8139: remove debug print
  mach-socfpga: do not overlap defines with lwip
  bcm_ns3: use device specific naming for variables
  omap3: use device specific naming for mem_init
  Makefile: add dtbs to clean

 Makefile   | 2 ++
 arch/arm/include/asm/arch-omap3/mem.h  | 2 +-
 arch/arm/mach-omap2/omap3/board.c  | 2 +-
 arch/arm/mach-omap2/omap3/emif4.c  | 4 ++--
 arch/arm/mach-omap2/omap3/sdrc.c   | 6 +++---
 arch/arm/mach-socfpga/include/mach/handoff_soc64.h | 6 --
 arch/arm/mach-socfpga/wrap_handoff_soc64.c | 9 +
 arch/sandbox/cpu/eth-raw-os.c  | 2 +-
 drivers/net/rtl8139.c  | 2 +-
 drivers/net/smc911x.c  | 2 +-
 include/configs/bcm_ns3.h  | 6 +++---
 test/py/tests/test_net.py  | 2 ++
 12 files changed, 26 insertions(+), 19 deletions(-)

-- 
2.30.2

Re: [PATCH v2 6/6] smbios: Require the caller to align the SMBIOS table

[Heinrich Schuchardt]

On 10/15/23 04:45, Simon Glass wrote:

All callers handle this alignment, so drop the unnecessary code. This
simplifies things a little.

Signed-off-by: Simon Glass 


Reviewed: Heinrich Schuchardt 


---

(no changes since v1)

  include/smbios.h | 5 +
  lib/smbios.c | 2 --
  2 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/include/smbios.h b/include/smbios.h
index ddabb558299e..31d997287588 100644
--- a/include/smbios.h
+++ b/include/smbios.h
@@ -248,12 +248,9 @@ static inline void fill_smbios_header(void *table, int 
type,
   *
   * This writes SMBIOS table at a given address.
   *
- * @addr:  start address to write SMBIOS table. If this is not
- * 16-byte-aligned then it will be aligned before the table is
- * written.
+ * @addr:  start address to write SMBIOS table (must be 16-byte-aligned)
   * Return:end address of SMBIOS table (and start address for next entry)
   *or NULL in case of an error
- *
   */
  ulong write_smbios_table(ulong addr);

diff --git a/lib/smbios.c b/lib/smbios.c
index 92e98388084f..5c9f108496d6 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -483,8 +483,6 @@ ulong write_smbios_table(ulong addr)
ctx.dev = NULL;
}

-   /* 16 byte align the table address */
-   addr = ALIGN(addr, 16);
start_addr = addr;

/*

Re: [PATCH v2 5/6] efi: Use the correct GUID for the SMBIOS table

[Heinrich Schuchardt]

On 10/15/23 04:45, Simon Glass wrote:

EFI does not use the 'anchor string' to determine the SMBIOS table
version, instead preferring to have two separate GUIDs. Use the correct
one, depending on the table version.

Call unmap_system() to balance to the use of map_sysmem()

Signed-off-by: Simon Glass 


Reviewed-by: Heinrich Schuchardt 


---

Changes in v2:
- Add a note about why unmap_system() is called

  include/efi_api.h   |  4 
  lib/efi_loader/efi_smbios.c | 12 ++--
  2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/include/efi_api.h b/include/efi_api.h
index 8f5ef5f680f3..1abac2ed7563 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -430,6 +430,10 @@ struct efi_runtime_services {
EFI_GUID(0xeb9d2d31, 0x2d88, 0x11d3,  \
 0x9a, 0x16, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)

+#define SMBIOS3_TABLE_GUID \
+   EFI_GUID(0xf2fd1544, 0x9794, 0x4a2c, \
+0x99, 0x2e, 0xe5, 0xbb, 0xcf, 0x20, 0xe3, 0x94)
+
  #define EFI_LOAD_FILE_PROTOCOL_GUID \
EFI_GUID(0x56ec3091, 0x954c, 0x11d2, \
 0x8e, 0x3f, 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c
index 4ae4e4a7c236..80f38d78d877 100644
--- a/lib/efi_loader/efi_smbios.c
+++ b/lib/efi_loader/efi_smbios.c
@@ -15,6 +15,8 @@
  #include 
  #include 

+const efi_guid_t smbios3_guid = SMBIOS3_TABLE_GUID;
+
  enum {
TABLE_SIZE  = SZ_4K,
  };
@@ -26,8 +28,10 @@ enum {
   */
  efi_status_t efi_smbios_register(void)
  {
+   const efi_guid_t *guid;
ulong addr;
efi_status_t ret;
+   void *buf;

addr = gd_smbios_start();
if (!addr) {
@@ -43,8 +47,12 @@ efi_status_t efi_smbios_register(void)
log_debug("EFI using SMBIOS tables at %lx\n", addr);

/* Install SMBIOS information as configuration table */
-   return efi_install_configuration_table(_guid,
-  map_sysmem(addr, 0));
+   buf = map_sysmem(addr, 0);
+   guid = !memcmp(buf, "_SM_", 4) ? _guid : _guid;
+   ret = efi_install_configuration_table(guid, buf);
+   unmap_sysmem(buf);
+
+   return ret;
  }

  static int install_smbios_table(void)

Re: [PATCH v2 4/6] smbios: Correct gd_smbios_start()

[Heinrich Schuchardt]

On 10/15/23 04:45, Simon Glass wrote:

This should access arch-specific properties. Fix it and update the
existing usage.

Signed-off-by: Simon Glass 


Reviewed-by: Heinrich Schuchardt 


---

Changes in v2:
- Add a new patch to correct gd_smbios_start()

  include/asm-generic/global_data.h | 2 +-
  lib/efi_loader/efi_smbios.c   | 2 +-
  2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/asm-generic/global_data.h 
b/include/asm-generic/global_data.h
index e8c6412e3f8d..c1f7818817f2 100644
--- a/include/asm-generic/global_data.h
+++ b/include/asm-generic/global_data.h
@@ -553,7 +553,7 @@ static_assert(sizeof(struct global_data) == GD_SIZE);
  #endif

  #ifdef CONFIG_SMBIOS
-#define gd_smbios_start()  gd->smbios_start
+#define gd_smbios_start()  gd->arch.smbios_start
  #define gd_set_smbios_start(addr) gd->arch.smbios_start = addr
  #else
  #define gd_smbios_start() 0UL
diff --git a/lib/efi_loader/efi_smbios.c b/lib/efi_loader/efi_smbios.c
index 48446f654d9b..4ae4e4a7c236 100644
--- a/lib/efi_loader/efi_smbios.c
+++ b/lib/efi_loader/efi_smbios.c
@@ -29,7 +29,7 @@ efi_status_t efi_smbios_register(void)
ulong addr;
efi_status_t ret;

-   addr = gd->arch.smbios_start;
+   addr = gd_smbios_start();
if (!addr) {
log_err("No SMBIOS tables to install\n");
return EFI_NOT_FOUND;

Re: [PATCH v8 7/8] config: xea: Enable the SPL_FIT config option

[Fabio Estevam]

Please run /scripts/get_maintainer.pl and add the board maintainer on Cc.

I have added Lukasz.

On Tue, Dec 26, 2023 at 6:43 AM Venkatesh Yadav Abbarapu
 wrote:
>
> Enable the config SPL_FIT.

Please improve the commit log by explaining why SPL_FIT is being enabled.

Re: Fwd: [scr1564375] your CVE ID requests

[Tom Rini]

On Tue, Dec 26, 2023 at 08:07:20AM +0300, sploitem wrote:
> -- Forwarded message -
> От: 
> Date: вт, 19 дек. 2023 г. в 20:39
> Subject: Re: [scr1564375] your CVE ID requests
> To: 
> Cc: 
> 
> 
> -BEGIN PGP SIGNED MESSAGE-
> Hash: SHA256
> 
> 
> > [Suggested description]
> > Integer underflow in ipv6 net component when processing incoming packets.
> udp->udp_len is not verified before substruction leading to large number in
> len parameter (unsigned int). This can lead to DoS or code execution.

Would you please submit a patch to address the issue as well? Thanks.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATHv2 9/9] Makefile: add dtbs to clean

[Maxim Uvarov]

On Tue, 26 Dec 2023 at 15:50, Simon Glass  wrote:

> Hi Maxim,
>
> On Mon, Dec 25, 2023 at 3:42 PM Maxim Uvarov 
> wrote:
> >
> > CI test checks that generated dtb has to be cleaned up.
> >
> > Signed-off-by: Maxim Uvarov 
> > ---
> >  Makefile | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index 220411a293..ab603191ad 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -2165,7 +2165,8 @@ CLEAN_FILES += include/autoconf.mk*
> include/bmp_logo.h include/bmp_logo_data.h \
> >mkimage-out.spl.mkimage mkimage.spl.mkimage imx-boot.map \
> >itb.fit.fit itb.fit.itb itb.map spl.map
> mkimage-out.rom.mkimage \
> >mkimage.rom.mkimage mkimage-in-simple-bin* rom.map
> simple-bin* \
> > -  idbloader-spi.img lib/efi_loader/helloworld_efi.S *.itb
> > +  idbloader-spi.img lib/efi_loader/helloworld_efi.S *.itb \
> > +  ./*/*.dtb *.dtb
> >
> >  # Directories & files removed with 'make mrproper'
> >  MRPROPER_DIRS  += include/config include/generated spl tpl vpl \
> > --
> > 2.30.2
> >
>
> Shouldn't this be handled with CLEAN_FILES in arch/xxx/dts ?
>
> Regards,
> Simon
>

I don't know. It looks like CLEAN_FILES is almost not used inside ./arch
directory. Only 1 place where it appears:

fgrep -r CLEAN_FILES ./arch/
./arch/arm/mach-rmobile/Makefile:CLEAN_FILES += u-boot-spl.scif

BR,
Maxim.

Re: [PATHv2 9/9] Makefile: add dtbs to clean

[Maxim Uvarov]

On Tue, 26 Dec 2023 at 20:26, Tom Rini  wrote:

> On Tue, Dec 26, 2023 at 06:27:58PM +0600, Maxim Uvarov wrote:
> > On Tue, 26 Dec 2023 at 15:50, Simon Glass  wrote:
> >
> > > Hi Maxim,
> > >
> > > On Mon, Dec 25, 2023 at 3:42 PM Maxim Uvarov 
> > > wrote:
> > > >
> > > > CI test checks that generated dtb has to be cleaned up.
> > > >
> > > > Signed-off-by: Maxim Uvarov 
> > > > ---
> > > >  Makefile | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/Makefile b/Makefile
> > > > index 220411a293..ab603191ad 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -2165,7 +2165,8 @@ CLEAN_FILES += include/autoconf.mk*
> > > include/bmp_logo.h include/bmp_logo_data.h \
> > > >mkimage-out.spl.mkimage mkimage.spl.mkimage
> imx-boot.map \
> > > >itb.fit.fit itb.fit.itb itb.map spl.map
> > > mkimage-out.rom.mkimage \
> > > >mkimage.rom.mkimage mkimage-in-simple-bin* rom.map
> > > simple-bin* \
> > > > -  idbloader-spi.img lib/efi_loader/helloworld_efi.S
> *.itb
> > > > +  idbloader-spi.img lib/efi_loader/helloworld_efi.S
> *.itb \
> > > > +  ./*/*.dtb *.dtb
> > > >
> > > >  # Directories & files removed with 'make mrproper'
> > > >  MRPROPER_DIRS  += include/config include/generated spl tpl vpl \
> > > > --
> > > > 2.30.2
> > > >
> > >
> > > Shouldn't this be handled with CLEAN_FILES in arch/xxx/dts ?
> > >
> > > Regards,
> > > Simon
> > >
> >
> > I don't know. It looks like CLEAN_FILES is almost not used inside ./arch
> > directory. Only 1 place where it appears:
> >
> > fgrep -r CLEAN_FILES ./arch/
> > ./arch/arm/mach-rmobile/Makefile:CLEAN_FILES += u-boot-spl.scif
>
> I see that our top-level clean target is missing dtb related targets
> like the linux kernel has, and that's how this should be fixed.
>
> --
> Tom
>

If I understand right you just refers to kernel's top level Makefile
clean:
find
-o -name '*.dtb' -o -name '*.dtbo' \

-o -name '*.dtb.S' -o -name '*.dtbo.S' \
| xargs rm -rf

 I will fix it in that way in v3.

BR,
Maxim.

Re: CVE-2019-14194 patch bypass

[Tom Rini]

On Wed, Aug 02, 2023 at 11:16:17PM +0300, sploit em wrote:

> Hello.
> 
> I was analyzing CVE-2019-14194 and I think the patch can be bypassed.
> patch
> 
> 
> In nfs_read_reply, rlen is signed int. So we can make it less than zero.
> The value less than zero can bypass the check at line 756. This will lead
> to overflow due that assign has an unsigned int len parameter.
> 
> For example -1 will become 0x.
> 
> [image: nfs_read_reply_cve_2019_14194.png]
> 
> 
> I've constructed some code to prove this.
> 
> [image: nfs_read_reply_cve_2019_14194_bypass.png]
> 
> [image: nfs_read_reply_cve_2019_14194_3.png]
> 
> Thanks.

Good find.  A patch to resolve this issue, by someone with an eye
towards other ways the problem could be exploited again so that this
time we don't have other avenues of attack open, would be appreciated.
Thanks.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v3 4/4] fs: remove explicit efi configuration dependency

[Tom Rini]

On Tue, Dec 26, 2023 at 09:46:52AM +, Simon Glass wrote:
> Hi,
> 
> On Wed, Dec 20, 2023 at 1:25 PM Tom Rini  wrote:
> >
> > On Tue, Dec 19, 2023 at 09:46:27PM -0700, Simon Glass wrote:
> > > Hi Heinrich,
> > >
> > > On Mon, 18 Dec 2023 at 15:16, Heinrich Schuchardt  
> > > wrote:
> > > >
> > > >
> > > >
> > > > Am 18. Dezember 2023 16:01:40 MEZ schrieb Simon Glass 
> > > > :
> > > > >Hi AKASHI,
> > > > >
> > > > >On Sun, 17 Dec 2023 at 19:39, AKASHI Takahiro
> > > > > wrote:
> > > > >>
> > > > >> Now it is clear that the feature actually depends on efi interfaces,
> > > > >> not "bootefi" command. efi_set_bootdev() will automatically be 
> > > > >> nullified
> > > > >> if necessary efi component is disabled.
> > > > >>
> > > > >> Signed-off-by: AKASHI Takahiro 
> > > > >> ---
> > > > >>  fs/fs.c | 7 +++
> > > > >>  1 file changed, 3 insertions(+), 4 deletions(-)
> > > > >>
> > > > >> diff --git a/fs/fs.c b/fs/fs.c
> > > > >> index f33b85f92b61..82ee03b160e9 100644
> > > > >> --- a/fs/fs.c
> > > > >> +++ b/fs/fs.c
> > > > >> @@ -791,10 +791,9 @@ int do_load(struct cmd_tbl *cmdtp, int flag, 
> > > > >> int argc, char *const argv[],
> > > > >> return 1;
> > > > >> }
> > > > >>
> > > > >> -   if (IS_ENABLED(CONFIG_CMD_BOOTEFI))
> > > > >> -   efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
> > > > >> -   (argc > 4) ? argv[4] : "", 
> > > > >> map_sysmem(addr, 0),
> > > > >> -   len_read);
> > > > >> +   efi_set_bootdev(argv[1], (argc > 2) ? argv[2] : "",
> > > > >> +   (argc > 4) ? argv[4] : "", map_sysmem(addr, 
> > > > >> 0),
> > > > >> +   len_read);
> > > > >
> > > > >As I understand it, this is setting the boot device so that (if it
> > > > >happens to be an efi application) it will know which device it came
> > > > >from. But this is a hack. For bootstd, the device is known as it loads
> > > > >the kernel.
> > > >
> > > > Please, consider what happens when the user interactively executes the 
> > > > load and bootefi command from the console.
> 
> Yes, that case needs to be handled.
> 
> > > >
> > > > >
> > > > >Also it does not deal with memory allocation (nor can it).
> > > > >
> > > > >Where are we using the 'load' command to load a kernel? The distro
> > > > >scripts are deprecated.
> > > >
> > > > Some users use boot.scr scripts
> 
> We will need to figure out that. But we should make bootstd do the right 
> thing.
> 
> > > >
> > > >
> > > > >
> > > > >At some point this code should be removed. Is it too early for that?
> > > >
> > > > Yes, as long as we allow users to invoke the bootefi command with an 
> > > > address pointer.
> > >
> > > So how about we create the new path, with the info passed in as part
> > > of the bootefi call? Then we can remove the call from bootstd, at
> > > least.
> >
> > Why? It's not clear to me we'd have this information available at that
> > point unless we stored things to pass along at that point too.
> 
> For the bootstd case, we know the device where the kernel came from.
> See distro_efi_boot() for this code. It calls efiload_read_file()
> which calls set_efi_bootdev().
> 
> We should instead pass the device information to efi_binary_run().
> 
> With that done, we can perhaps make the 'load' and 'tftp' commands
> record the device information somewhere, so that bootm can pick it up
> when it starts and put it in 'images'. Then at some point we can clean
> up the 'devnr' parameter, which should really be a partition number,
> since the 'dev' parameter tells you the device.
> 
> Anyway, this series is fine, so long as we can agree to the above.

I'm getting very worried that we're spending a lot of time deciding how
users are going to use the system, without getting input from them on
what their use cases are, and are not, and why they've chosen what
they've chosen. Talking with some of the Debian people about why they're
using script + extlinux, and Armbian people about what they're doing
(which is just scripts?) should be the first priority before we decide
how booting things is going to work, outside of the EFI cases (which to
be clear, are having their interests / needs represented already I
believe).

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATHv2 9/9] Makefile: add dtbs to clean

[Tom Rini]

On Tue, Dec 26, 2023 at 06:27:58PM +0600, Maxim Uvarov wrote:
> On Tue, 26 Dec 2023 at 15:50, Simon Glass  wrote:
> 
> > Hi Maxim,
> >
> > On Mon, Dec 25, 2023 at 3:42 PM Maxim Uvarov 
> > wrote:
> > >
> > > CI test checks that generated dtb has to be cleaned up.
> > >
> > > Signed-off-by: Maxim Uvarov 
> > > ---
> > >  Makefile | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/Makefile b/Makefile
> > > index 220411a293..ab603191ad 100644
> > > --- a/Makefile
> > > +++ b/Makefile
> > > @@ -2165,7 +2165,8 @@ CLEAN_FILES += include/autoconf.mk*
> > include/bmp_logo.h include/bmp_logo_data.h \
> > >mkimage-out.spl.mkimage mkimage.spl.mkimage imx-boot.map \
> > >itb.fit.fit itb.fit.itb itb.map spl.map
> > mkimage-out.rom.mkimage \
> > >mkimage.rom.mkimage mkimage-in-simple-bin* rom.map
> > simple-bin* \
> > > -  idbloader-spi.img lib/efi_loader/helloworld_efi.S *.itb
> > > +  idbloader-spi.img lib/efi_loader/helloworld_efi.S *.itb \
> > > +  ./*/*.dtb *.dtb
> > >
> > >  # Directories & files removed with 'make mrproper'
> > >  MRPROPER_DIRS  += include/config include/generated spl tpl vpl \
> > > --
> > > 2.30.2
> > >
> >
> > Shouldn't this be handled with CLEAN_FILES in arch/xxx/dts ?
> >
> > Regards,
> > Simon
> >
> 
> I don't know. It looks like CLEAN_FILES is almost not used inside ./arch
> directory. Only 1 place where it appears:
> 
> fgrep -r CLEAN_FILES ./arch/
> ./arch/arm/mach-rmobile/Makefile:CLEAN_FILES += u-boot-spl.scif

I see that our top-level clean target is missing dtb related targets
like the linux kernel has, and that's how this should be fixed.

-- 
Tom


signature.asc
Description: PGP signature

Fwd: [scr1564375] your CVE ID requests

[sploitem]

-- Forwarded message -
От: 
Date: вт, 19 дек. 2023 г. в 20:39
Subject: Re: [scr1564375] your CVE ID requests
To: 
Cc: 


-BEGIN PGP SIGNED MESSAGE-
Hash: SHA256


> [Suggested description]
> Integer underflow in ipv6 net component when processing incoming packets.
udp->udp_len is not verified before substruction leading to large number in
len parameter (unsigned int). This can lead to DoS or code execution.
>
> --
>
> [Vulnerability Type]
> Integer Overflow
>
> --
>
> [Vendor of Product]
> U-Boot
>
> --
>
> [Affected Product Code Base]
> U-boot - <= v2024.01-rc3
>
> --
>
> [Affected Component]
> u-boot/net/net6.c
>
> --
>
> [Attack Type]
> Remote
>
> --
>
> [Impact Code execution]
> true
>
> --
>
> [Impact Denial of Service]
> true
>
> --
>
> [Attack Vectors]
> Crafted ipv6 udp packet.
>
> --
>
> [Reference]
> https://github.com/u-boot/u-boot/blob/master/net/net6.c#L442C18-L442C18
>
> --
>
> [Discoverer]
> sploitem

This request did not receive a CVE ID assignment as CVEs are not assigned
to Release Candidate (rc) versions of products.



- --
CVE Assignment Team
M/S M300, 202 Burlington Road, Bedford, MA 01730 USA
 [ A PGP key is available for encrypted communications at
   https://cve.mitre.org/cve/request_id.html ]
-BEGIN PGP SIGNATURE-
Version: GnuPG v1

iQIcBAEBCAAGBQJlgdSnAAoJENiPHH3233OGtbYQAKDjsoR/MIuN6txsUarIqDx9
KzoNx5MR0Ze+HIyZ3vBV1+eUJcnHfStLDaIMRNGR7497+A474X54vn8OVRTqgh+W
CPQiTqx2PpJTYikxBAUrbh6uzwFHlb+iv7Dt2mqyZ4Eg+sX8X507YfyyIsWr6Npw
VYiXSt53Sy9hQDski7H+Nl3keO3km5G29MdavcvNiAwH0g/a9f+NtRvQvi68X+n7
4nWRa2bzH3zChvOVJ0TRvua9ptwU+svM4wjL9vloEMO8sDO3CyFIoiGhyhseERcT
Zv/NpPdqpqlKwRcvY6vW/GMGU6pVhwpIer9jrX4yolviN1d92/J9sIfJXchl5yNh
fUnX0NYlBbBKH1Hy/ttOXuOBPeNgFv1VpryJvjyOxmTIpOZUhm1iiehzEGA2pFdO
FUHAKmfrugVTr8Gp3HL/tQ3MN08nB64LHkowD+j0+XGN1EEIurWSnwRwdXmEn26Y
MSkGbXzT0GiibnkhwpdRxcZW4p33NC+idNBZtb4K8TIcdUdh2P4ZzeyTMYmxMKgp
WVHxxKvLEom+E17SAb996Wesia4+gRo+2wK4cXCtnEn7nqz59y1mqBf1uALeTDLG
D2/9N3nqGYZEu8WeMjSNYPmUvLMHKvffj9Z81+dBSq/IngZMYuXr6m+UGTce66N1
jmb+4NApCJi3mhF4dUK6
=T698
-END PGP SIGNATURE-

[PATCH v9 8/8] axm/taurus: Disable the SPL_SPI config

[Venkatesh Yadav Abbarapu]

Disabling the CONFIG_SPL_SPI for the axm and taurus defconfigs,
as facing the SPL size issue "SPL image too big".

Signed-off-by: Venkatesh Yadav Abbarapu 
---
 board/siemens/taurus/taurus.c | 4 +++-
 configs/axm_defconfig | 7 ++-
 configs/taurus_defconfig  | 7 ++-
 3 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/board/siemens/taurus/taurus.c b/board/siemens/taurus/taurus.c
index ad44a7c0d2..ac1ce8acf4 100644
--- a/board/siemens/taurus/taurus.c
+++ b/board/siemens/taurus/taurus.c
@@ -135,11 +135,12 @@ void spl_board_init(void)
 
/* check for recovery mode */
if (at91_is_recovery() == 1) {
-   struct spi_flash *flash;
 
puts("Recovery button pressed\n");
nand_init();
spl_nand_erase_one(0, 0);
+#if defined(CONFIG_SPL_SPI)
+   struct spi_flash *flash;
flash = spi_flash_probe(CONFIG_SF_DEFAULT_BUS,
0,
CONFIG_SF_DEFAULT_SPEED,
@@ -151,6 +152,7 @@ void spl_board_init(void)
spi_flash_erase(flash, 0,
CFG_SYS_NAND_U_BOOT_SIZE);
}
+#endif
}
 }
 
diff --git a/configs/axm_defconfig b/configs/axm_defconfig
index e1a01b24b8..4d439832d7 100644
--- a/configs/axm_defconfig
+++ b/configs/axm_defconfig
@@ -3,7 +3,6 @@ CONFIG_SKIP_LOWLEVEL_INIT_ONLY=y
 CONFIG_SPL_SKIP_LOWLEVEL_INIT_ONLY=y
 CONFIG_SPL_SYS_DCACHE_OFF=y
 CONFIG_ARCH_CPU_INIT=y
-CONFIG_SYS_THUMB_BUILD=y
 # CONFIG_SPL_USE_ARCH_MEMCPY is not set
 # CONFIG_SPL_USE_ARCH_MEMSET is not set
 CONFIG_ARCH_AT91=y
@@ -26,14 +25,14 @@ CONFIG_SPL=y
 CONFIG_DEBUG_UART_BASE=0xf200
 CONFIG_DEBUG_UART_CLOCK=18432000
 CONFIG_ENV_OFFSET_REDUND=0x18
-CONFIG_SPL_SPI_FLASH_SUPPORT=y
-CONFIG_SPL_SPI=y
 CONFIG_SYS_LOAD_ADDR=0x2200
 CONFIG_DEBUG_UART=y
 CONFIG_NAND_BOOT=y
 CONFIG_BOOTDELAY=3
 CONFIG_USE_BOOTCOMMAND=y
 CONFIG_BOOTCOMMAND="run flash_self"
+CONFIG_SYS_CBSIZE=256
+CONFIG_SYS_PBSIZE=281
 # CONFIG_DISPLAY_BOARDINFO is not set
 CONFIG_BOARD_EARLY_INIT_F=y
 CONFIG_SPL_MAX_SIZE=0x3e00
@@ -57,8 +56,6 @@ CONFIG_SPL_NAND_BASE=y
 CONFIG_SPL_DM_SPI_FLASH=y
 CONFIG_HUSH_PARSER=y
 CONFIG_SYS_PROMPT="U-Boot> "
-CONFIG_SYS_CBSIZE=256
-CONFIG_SYS_PBSIZE=281
 # CONFIG_CMD_BDI is not set
 CONFIG_CMD_BOOTZ=y
 # CONFIG_CMD_IMI is not set
diff --git a/configs/taurus_defconfig b/configs/taurus_defconfig
index 9be30c8d8a..7d71262d1a 100644
--- a/configs/taurus_defconfig
+++ b/configs/taurus_defconfig
@@ -4,7 +4,6 @@ CONFIG_SPL_SKIP_LOWLEVEL_INIT_ONLY=y
 CONFIG_SPL_SYS_ICACHE_OFF=y
 CONFIG_SPL_SYS_DCACHE_OFF=y
 CONFIG_ARCH_CPU_INIT=y
-CONFIG_SYS_THUMB_BUILD=y
 # CONFIG_SPL_USE_ARCH_MEMCPY is not set
 # CONFIG_SPL_USE_ARCH_MEMSET is not set
 CONFIG_ARCH_AT91=y
@@ -28,8 +27,6 @@ CONFIG_SPL=y
 CONFIG_DEBUG_UART_BASE=0xf200
 CONFIG_DEBUG_UART_CLOCK=18432000
 CONFIG_ENV_OFFSET_REDUND=0x18
-CONFIG_SPL_SPI_FLASH_SUPPORT=y
-CONFIG_SPL_SPI=y
 CONFIG_SYS_LOAD_ADDR=0x2200
 CONFIG_DEBUG_UART=y
 CONFIG_NAND_BOOT=y
@@ -38,6 +35,8 @@ CONFIG_USE_BOOTARGS=y
 CONFIG_BOOTARGS="console=ttyS0,115200 earlyprintk 
mtdparts=atmel_nand:256k(bootstrap)ro,512k(uboot)ro,256k(env),256k(env_redundant),256k(spare),512k(dtb),6M(kernel)ro,-(rootfs)
 root=/dev/mtdblock7 rw rootfstype=jffs2"
 CONFIG_USE_BOOTCOMMAND=y
 CONFIG_BOOTCOMMAND="nand read 0x2200 0x20 0x30; bootm"
+CONFIG_SYS_CBSIZE=256
+CONFIG_SYS_PBSIZE=281
 # CONFIG_DISPLAY_BOARDINFO is not set
 CONFIG_BOARD_EARLY_INIT_F=y
 CONFIG_SPL_MAX_SIZE=0x3e00
@@ -61,8 +60,6 @@ CONFIG_SPL_NAND_BASE=y
 CONFIG_SPL_DM_SPI_FLASH=y
 CONFIG_HUSH_PARSER=y
 CONFIG_SYS_PROMPT="U-Boot> "
-CONFIG_SYS_CBSIZE=256
-CONFIG_SYS_PBSIZE=281
 # CONFIG_SYS_XTRACE is not set
 # CONFIG_CMD_BDI is not set
 CONFIG_CMD_BOOTZ=y
-- 
2.25.1

[PATCH v9 7/8] config: xea: Enable the SPL_FIT config option

[Venkatesh Yadav Abbarapu]

Enable the config SPL_FIT, as getting the below error
../arm-xilinx-linux-gnueabi-ld.bfd.real: drivers/core/ofnode.o: in function
`ofnode_get_property':
/u-boot/drivers/core/ofnode.c:1185: undefined reference to `fdt_getprop'
make[1]: *** [scripts/Makefile.spl:527: spl/u-boot-spl] Error 1

Signed-off-by: Venkatesh Yadav Abbarapu 
---
 configs/imx28_xea_defconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configs/imx28_xea_defconfig b/configs/imx28_xea_defconfig
index c1b0487f7e..882b6093c9 100644
--- a/configs/imx28_xea_defconfig
+++ b/configs/imx28_xea_defconfig
@@ -29,6 +29,7 @@ CONFIG_SYS_LOAD_ADDR=0x4200
 CONFIG_SPL_PAYLOAD="u-boot.img"
 CONFIG_TIMESTAMP=y
 CONFIG_FIT=y
+CONFIG_SPL_FIT=y
 # CONFIG_BOOTMETH_EXTLINUX is not set
 # CONFIG_BOOTMETH_VBE is not set
 CONFIG_OF_BOARD_SETUP=y
@@ -123,4 +124,3 @@ CONFIG_DM_SERIAL=y
 CONFIG_SPI=y
 CONFIG_DM_SPI=y
 CONFIG_MXS_SPI=y
-# CONFIG_SPL_OF_LIBFDT is not set
-- 
2.25.1

[PATCH v9 4/8] spi: spi-uclass: Read chipselect and restrict capabilities

[Venkatesh Yadav Abbarapu]

From: Ashok Reddy Soma 

Read chipselect properties from DT which are populated using 'reg'
property and save it in plat->cs[] array for later use.

Also read multi chipselect capability which is used for
parallel-memories and return errors if they are passed on using DT but
driver is not capable of handling it.

Signed-off-by: Ashok Reddy Soma 
Signed-off-by: Venkatesh Yadav Abbarapu 
---
 drivers/mtd/spi/sandbox.c|  2 +-
 drivers/spi/altera_spi.c |  4 ++--
 drivers/spi/atcspi200_spi.c  |  2 +-
 drivers/spi/ath79_spi.c  |  2 +-
 drivers/spi/atmel_spi.c  |  6 +++---
 drivers/spi/bcm63xx_hsspi.c  | 42 ++--
 drivers/spi/bcm63xx_spi.c|  6 +++---
 drivers/spi/bcmbca_hsspi.c   | 34 ++---
 drivers/spi/cf_spi.c |  6 +++---
 drivers/spi/davinci_spi.c|  8 +++
 drivers/spi/fsl_dspi.c   | 18 
 drivers/spi/fsl_espi.c   |  4 ++--
 drivers/spi/fsl_qspi.c   |  4 ++--
 drivers/spi/gxp_spi.c|  2 +-
 drivers/spi/mpc8xx_spi.c |  4 ++--
 drivers/spi/mpc8xxx_spi.c| 10 -
 drivers/spi/mscc_bb_spi.c|  4 ++--
 drivers/spi/mxc_spi.c|  6 +++---
 drivers/spi/npcm_fiu_spi.c   | 14 ++--
 drivers/spi/nxp_fspi.c   |  2 +-
 drivers/spi/octeon_spi.c |  2 +-
 drivers/spi/omap3_spi.c  |  4 ++--
 drivers/spi/pic32_spi.c  |  2 +-
 drivers/spi/rk_spi.c |  4 ++--
 drivers/spi/rockchip_sfc.c   |  2 +-
 drivers/spi/spi-aspeed-smc.c | 28 
 drivers/spi/spi-mxic.c   |  6 +++---
 drivers/spi/spi-qup.c|  4 ++--
 drivers/spi/spi-sifive.c |  6 +++---
 drivers/spi/spi-sn-f-ospi.c  |  2 +-
 drivers/spi/spi-sunxi.c  |  6 +++---
 drivers/spi/spi-synquacer.c  |  4 ++--
 drivers/spi/spi-uclass.c | 33 +++-
 drivers/spi/stm32_qspi.c |  2 +-
 drivers/spi/stm32_spi.c  |  4 ++--
 drivers/spi/ti_qspi.c| 14 ++--
 drivers/spi/xilinx_spi.c |  6 +++---
 drivers/spi/zynq_qspi.c  |  6 +++---
 drivers/spi/zynq_spi.c   |  6 +++---
 include/spi.h|  8 ++-
 lib/acpi/acpi_device.c   |  2 +-
 41 files changed, 180 insertions(+), 151 deletions(-)

diff --git a/drivers/mtd/spi/sandbox.c b/drivers/mtd/spi/sandbox.c
index 4fe547171a..72036d5a88 100644
--- a/drivers/mtd/spi/sandbox.c
+++ b/drivers/mtd/spi/sandbox.c
@@ -139,7 +139,7 @@ static int sandbox_sf_probe(struct udevice *dev)
return ret;
}
slave_plat = dev_get_parent_plat(dev);
-   cs = slave_plat->cs;
+   cs = slave_plat->cs[0];
debug("found at cs %d\n", cs);
 
if (!pdata->filename) {
diff --git a/drivers/spi/altera_spi.c b/drivers/spi/altera_spi.c
index 989679e881..48782f81c1 100644
--- a/drivers/spi/altera_spi.c
+++ b/drivers/spi/altera_spi.c
@@ -96,7 +96,7 @@ static int altera_spi_xfer(struct udevice *dev, unsigned int 
bitlen,
uint32_t reg, data, start;
 
debug("%s: bus:%i cs:%i bitlen:%i bytes:%i flags:%lx\n", __func__,
- dev_seq(bus), slave_plat->cs, bitlen, bytes, flags);
+ dev_seq(bus), slave_plat->cs[0], bitlen, bytes, flags);
 
if (bitlen == 0)
goto done;
@@ -111,7 +111,7 @@ static int altera_spi_xfer(struct udevice *dev, unsigned 
int bitlen,
readl(>rxdata);
 
if (flags & SPI_XFER_BEGIN)
-   spi_cs_activate(dev, slave_plat->cs);
+   spi_cs_activate(dev, slave_plat->cs[0]);
 
while (bytes--) {
if (txp)
diff --git a/drivers/spi/atcspi200_spi.c b/drivers/spi/atcspi200_spi.c
index de9c14837c..acee743653 100644
--- a/drivers/spi/atcspi200_spi.c
+++ b/drivers/spi/atcspi200_spi.c
@@ -321,7 +321,7 @@ static int atcspi200_spi_claim_bus(struct udevice *dev)
struct udevice *bus = dev->parent;
struct nds_spi_slave *ns = dev_get_priv(bus);
 
-   if (slave_plat->cs >= ns->num_cs) {
+   if (slave_plat->cs[0] >= ns->num_cs) {
printf("Invalid SPI chipselect\n");
return -EINVAL;
}
diff --git a/drivers/spi/ath79_spi.c b/drivers/spi/ath79_spi.c
index 205567ef54..ad10cec2a6 100644
--- a/drivers/spi/ath79_spi.c
+++ b/drivers/spi/ath79_spi.c
@@ -74,7 +74,7 @@ static int ath79_spi_xfer(struct udevice *dev, unsigned int 
bitlen,
if (restbits)
bytes++;
 
-   out = AR71XX_SPI_IOC_CS_ALL & ~(AR71XX_SPI_IOC_CS(slave->cs));
+   out = AR71XX_SPI_IOC_CS_ALL & ~(AR71XX_SPI_IOC_CS(slave->cs[0]));
while (bytes > 0) {
bytes--;
curbyte = 0;
diff --git a/drivers/spi/atmel_spi.c b/drivers/spi/atmel_spi.c
index aec6f4eca9..e2de39d1ef 100644
--- a/drivers/spi/atmel_spi.c
+++ b/drivers/spi/atmel_spi.c
@@ -126,7 +126,7 @@ static int atmel_spi_claim_bus(struct udevice *dev)
struct atmel_spi_priv *priv = dev_get_priv(bus);
struct dm_spi_slave_plat *slave_plat = dev_get_parent_plat(dev);
struct 

[PATCH v9 5/8] spi: zynqmp_gqspi: Add parallel memories support in GQSPI driver

[Venkatesh Yadav Abbarapu]

Add support for parallel memories in zynqmp_gqspi.c driver. In case of
parallel memories STRIPE bit is set and sent to the qspi ip, which will
send data bits to both the flashes in parallel. However for few commands
we should not use stripe, instead send same data to both the flashes.
Those commands are exclueded by using zynqmp_qspi_update_stripe().

Also update copyright info for this file.

Signed-off-by: Ashok Reddy Soma 
Signed-off-by: Venkatesh Yadav Abbarapu 
---
 drivers/spi/zynqmp_gqspi.c | 141 -
 include/spi.h  |   4 ++
 2 files changed, 129 insertions(+), 16 deletions(-)

diff --git a/drivers/spi/zynqmp_gqspi.c b/drivers/spi/zynqmp_gqspi.c
index a323994fb2..dedf8270a8 100644
--- a/drivers/spi/zynqmp_gqspi.c
+++ b/drivers/spi/zynqmp_gqspi.c
@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- * (C) Copyright 2018 Xilinx
- *
+ * (C) Copyright 2013 - 2022, Xilinx, Inc.
+ * (C) Copyright 2023, Advanced Micro Devices, Inc.
  * Xilinx ZynqMP Generic Quad-SPI(QSPI) controller driver(master mode only)
  */
 
@@ -25,6 +25,8 @@
 #include 
 #include 
 #include 
+#include 
+#include "../mtd/spi/sf_internal.h"
 #include 
 
 #define GQSPI_GFIFO_STRT_MODE_MASK BIT(29)
@@ -88,6 +90,9 @@
 #define SPI_XFER_ON_LOWER  1
 #define SPI_XFER_ON_UPPER  2
 
+#define GQSPI_SELECT_LOWER_CS  BIT(0)
+#define GQSPI_SELECT_UPPER_CS  BIT(1)
+
 #define GQSPI_DMA_ALIGN0x4
 #define GQSPI_MAX_BAUD_RATE_VAL7
 #define GQSPI_DFLT_BAUD_RATE_VAL   2
@@ -183,13 +188,14 @@ struct zynqmp_qspi_priv {
int bytes_to_transfer;
int bytes_to_receive;
const struct spi_mem_op *op;
+   unsigned int is_parallel;
+   unsigned int u_page;
+   unsigned int bus;
+   unsigned int stripe;
+   unsigned int flags;
+   u32 max_hz;
 };
 
-__weak int zynqmp_mmio_write(const u32 address, const u32 mask, const u32 
value)
-{
-   return 0;
-}
-
 static int zynqmp_qspi_of_to_plat(struct udevice *bus)
 {
struct zynqmp_qspi_plat *plat = dev_get_plat(bus);
@@ -234,8 +240,30 @@ static u32 zynqmp_qspi_bus_select(struct zynqmp_qspi_priv 
*priv)
 {
u32 gqspi_fifo_reg = 0;
 
-   gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS |
-GQSPI_GFIFO_CS_LOWER;
+   if (priv->is_parallel) {
+   if (priv->bus == SPI_XFER_ON_BOTH)
+   gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS |
+GQSPI_GFIFO_UP_BUS |
+GQSPI_GFIFO_CS_UPPER |
+GQSPI_GFIFO_CS_LOWER;
+   else if (priv->bus == SPI_XFER_ON_LOWER)
+   gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS |
+GQSPI_GFIFO_CS_UPPER |
+GQSPI_GFIFO_CS_LOWER;
+   else if (priv->bus == SPI_XFER_ON_UPPER)
+   gqspi_fifo_reg = GQSPI_GFIFO_UP_BUS |
+GQSPI_GFIFO_CS_LOWER |
+GQSPI_GFIFO_CS_UPPER;
+   else
+   debug("Wrong Bus selection:0x%x\n", priv->bus);
+   } else {
+   if (priv->u_page)
+   gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS |
+GQSPI_GFIFO_CS_UPPER;
+   else
+   gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS |
+GQSPI_GFIFO_CS_LOWER;
+   }
 
return gqspi_fifo_reg;
 }
@@ -295,8 +323,15 @@ static void zynqmp_qspi_chipselect(struct zynqmp_qspi_priv 
*priv, int is_on)
gqspi_fifo_reg |= GQSPI_SPI_MODE_SPI |
  GQSPI_IMD_DATA_CS_ASSERT;
} else {
-   gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS;
-   gqspi_fifo_reg |= GQSPI_IMD_DATA_CS_DEASSERT;
+   if (priv->is_parallel) {
+   gqspi_fifo_reg = GQSPI_GFIFO_UP_BUS |
+GQSPI_GFIFO_LOW_BUS;
+   } else if (priv->u_page) {
+   gqspi_fifo_reg = GQSPI_GFIFO_UP_BUS;
+   } else {
+   gqspi_fifo_reg = GQSPI_GFIFO_LOW_BUS;
+   gqspi_fifo_reg |= GQSPI_IMD_DATA_CS_DEASSERT;
+   }
}
 
zynqmp_qspi_fill_gen_fifo(priv, gqspi_fifo_reg);
@@ -366,12 +401,13 @@ static int zynqmp_qspi_set_speed(struct udevice *bus, 
uint speed)
 
log_debug("%s, Speed: %d, Max: %d\n", __func__, speed, plat->frequency);
 
-   if (speed > plat->frequency)
-   speed = plat->frequency;
+   /*
+* If speed == 0 or speed > max freq, then set speed to highest
+*/
+   if (!speed || speed > priv->max_hz)
+   speed = priv->max_hz;
 
if (plat->speed_hz != speed) {
-   /* Set the 

[PATCH v9 3/8] mtd: spi-nor: Add parallel and stacked memories support in read_bar and write_bar

[Venkatesh Yadav Abbarapu]

From: Ashok Reddy Soma 

Add support for parallel memories and stacked memories configuration
in read_bar and write_bar functions.

Signed-off-by: Ashok Reddy Soma 
Signed-off-by: Venkatesh Yadav Abbarapu 
---
 drivers/mtd/spi/spi-nor-core.c | 55 +-
 1 file changed, 47 insertions(+), 8 deletions(-)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index 6d1bd4d54a..6ca43aa6e8 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -885,12 +885,32 @@ static int clean_bar(struct spi_nor *nor)
 
 static int write_bar(struct spi_nor *nor, u32 offset)
 {
-   u8 cmd, bank_sel;
+   u8 cmd, bank_sel, upage_curr;
int ret;
+   struct mtd_info *mtd = >mtd;
+
+   /* Wait until previous write command is finished */
+   if (spi_nor_wait_till_ready(nor))
+   return 1;
+
+   if (nor->flags & (SNOR_F_HAS_PARALLEL | SNOR_F_HAS_STACKED) &&
+   mtd->size <= SZ_32M)
+   return 0;
+
+   if (mtd->size <= SZ_16M)
+   return 0;
+
+   offset = offset % (u32)mtd->size;
+   bank_sel = offset >> 24;
 
-   bank_sel = offset / SZ_16M;
-   if (bank_sel == nor->bank_curr)
-   goto bar_end;
+   upage_curr = nor->spi->flags & SPI_XFER_U_PAGE;
+
+   if (!(nor->flags & SNOR_F_HAS_STACKED) && bank_sel == nor->bank_curr)
+   return 0;
+   else if (upage_curr == nor->upage_prev && bank_sel == nor->bank_curr)
+   return 0;
+
+   nor->upage_prev = upage_curr;
 
cmd = nor->bank_write_cmd;
write_enable(nor);
@@ -900,15 +920,19 @@ static int write_bar(struct spi_nor *nor, u32 offset)
return ret;
}
 
-bar_end:
nor->bank_curr = bank_sel;
-   return nor->bank_curr;
+
+   return write_disable(nor);
 }
 
 static int read_bar(struct spi_nor *nor, const struct flash_info *info)
 {
u8 curr_bank = 0;
int ret;
+   struct mtd_info *mtd = >mtd;
+
+   if (mtd->size <= SZ_16M)
+   return 0;
 
switch (JEDEC_MFR(info)) {
case SNOR_MFR_SPANSION:
@@ -920,15 +944,30 @@ static int read_bar(struct spi_nor *nor, const struct 
flash_info *info)
nor->bank_write_cmd = SPINOR_OP_WREAR;
}
 
+   if (nor->flags & SNOR_F_HAS_PARALLEL)
+   nor->spi->flags |= SPI_XFER_LOWER;
+
ret = nor->read_reg(nor, nor->bank_read_cmd,
-   _bank, 1);
+   _bank, 1);
if (ret) {
debug("SF: fail to read bank addr register\n");
return ret;
}
nor->bank_curr = curr_bank;
 
-   return 0;
+   // Make sure both chips use the same BAR
+   if (nor->flags & SNOR_F_HAS_PARALLEL) {
+   write_enable(nor);
+   ret = nor->write_reg(nor, nor->bank_write_cmd, _bank, 1);
+   if (ret)
+   return ret;
+
+   ret = write_disable(nor);
+   if (ret)
+   return ret;
+   }
+
+   return ret;
 }
 #endif
 
-- 
2.25.1

[PATCH v9 2/8] mtd: spi-nor: Add parallel memories support for read_sr and read_fsr

[Venkatesh Yadav Abbarapu]

From: Ashok Reddy Soma 

Add support for parallel memories flash configuration in read status
register and read flag status register functions.

Signed-off-by: Ashok Reddy Soma 
Signed-off-by: Venkatesh Yadav Abbarapu 
---
 drivers/mtd/spi/spi-nor-core.c | 50 --
 1 file changed, 36 insertions(+), 14 deletions(-)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index 9d72dff85e..6d1bd4d54a 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -438,8 +438,9 @@ static ssize_t spi_nor_write_data(struct spi_nor *nor, 
loff_t to, size_t len,
 }
 
 /*
- * Read the status register, returning its value in the location
- * Return the status register value.
+ * Return the status register value. If the chip is parallel, then the
+ * read will be striped, so we should read 2 bytes to get the sr
+ * register value from both of the parallel chips.
  * Returns negative if error occurred.
  */
 static int read_sr(struct spi_nor *nor)
@@ -471,18 +472,29 @@ static int read_sr(struct spi_nor *nor)
if (spi_nor_protocol_is_dtr(nor->reg_proto))
op.data.nbytes = 2;
 
-   ret = spi_nor_read_write_reg(nor, , val);
-   if (ret < 0) {
-   pr_debug("error %d reading SR\n", (int)ret);
-   return ret;
+   if (nor->flags & SNOR_F_HAS_PARALLEL) {
+   op.data.nbytes = 2;
+   ret = spi_nor_read_write_reg(nor, , [0]);
+   if (ret < 0) {
+   pr_debug("error %d reading SR\n", (int)ret);
+   return ret;
+   }
+   val[0] |= val[1];
+   } else {
+   ret = spi_nor_read_write_reg(nor, , [0]);
+   if (ret < 0) {
+   pr_debug("error %d reading SR\n", (int)ret);
+   return ret;
+   }
}
 
-   return *val;
+   return val[0];
 }
 
 /*
- * Read the flag status register, returning its value in the location
- * Return the status register value.
+ * Return the flag status register value. If the chip is parallel, then
+ * the read will be striped, so we should read 2 bytes to get the fsr
+ * register value from both of the parallel chips.
  * Returns negative if error occurred.
  */
 static int read_fsr(struct spi_nor *nor)
@@ -514,13 +526,23 @@ static int read_fsr(struct spi_nor *nor)
if (spi_nor_protocol_is_dtr(nor->reg_proto))
op.data.nbytes = 2;
 
-   ret = spi_nor_read_write_reg(nor, , val);
-   if (ret < 0) {
-   pr_debug("error %d reading FSR\n", ret);
-   return ret;
+   if (nor->flags & SNOR_F_HAS_PARALLEL) {
+   op.data.nbytes = 2;
+   ret = spi_nor_read_write_reg(nor, , [0]);
+   if (ret < 0) {
+   pr_debug("error %d reading SR\n", (int)ret);
+   return ret;
+   }
+   val[0] &= val[1];
+   } else {
+   ret = spi_nor_read_write_reg(nor, , [0]);
+   if (ret < 0) {
+   pr_debug("error %d reading FSR\n", ret);
+   return ret;
+   }
}
 
-   return *val;
+   return val[0];
 }
 
 /*
-- 
2.25.1

[PATCH v9 1/8] mtd: spi-nor: Add parallel and stacked memories support

[Venkatesh Yadav Abbarapu]

From: Ashok Reddy Soma 

In parallel mode, the current implementation assumes that a maximum of
two flashes are connected. The QSPI controller splits the data evenly
between both the flashes so, both the flashes that are connected in
parallel mode should be identical.
During each operation SPI-NOR sets 0th bit for CS0 & 1st bit for CS1 in
nor->flags.

In stacked mode the current implementation assumes that a maximum of two
flashes are connected and both the flashes are of same make but can
differ in sizes. So, except the sizes all other flash parameters of both
the flashes are identical

Spi-nor will pass on the appropriate flash select flag to low level
driver, and it will select pass all the data to that particular flash.

Write operation in parallel mode are performed in page size * 2 chunks as
each write operation results in writing both the flashes. For doubling
the address space each operation is performed at addr/2 flash offset,
where addr is the address specified by the user.

Similarly for read and erase operations it will read from both flashes,
so size and offset are divided by 2 and send to flash.

Signed-off-by: Ashok Reddy Soma 
Signed-off-by: Venkatesh Yadav Abbarapu 
---
 drivers/mtd/spi/spi-nor-core.c | 293 +
 include/linux/mtd/spi-nor.h|  12 ++
 include/spi.h  |  11 ++
 3 files changed, 288 insertions(+), 28 deletions(-)

diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index 3f5f3c89ac..9d72dff85e 100644
--- a/drivers/mtd/spi/spi-nor-core.c
+++ b/drivers/mtd/spi/spi-nor-core.c
@@ -639,12 +639,17 @@ static u8 spi_nor_convert_3to4_erase(u8 opcode)
 static void spi_nor_set_4byte_opcodes(struct spi_nor *nor,
  const struct flash_info *info)
 {
+   bool shift = 0;
+
+   if (nor->flags & SNOR_F_HAS_PARALLEL)
+   shift = 1;
+
/* Do some manufacturer fixups first */
switch (JEDEC_MFR(info)) {
case SNOR_MFR_SPANSION:
/* No small sector erase for 4-byte command set */
nor->erase_opcode = SPINOR_OP_SE;
-   nor->mtd.erasesize = info->sector_size;
+   nor->mtd.erasesize = info->sector_size << shift;
break;
 
default:
@@ -965,8 +970,8 @@ static int spi_nor_erase_sector(struct spi_nor *nor, u32 
addr)
 static int spi_nor_erase(struct mtd_info *mtd, struct erase_info *instr)
 {
struct spi_nor *nor = mtd_to_spi_nor(mtd);
+   u32 addr, len, rem, offset;
bool addr_known = false;
-   u32 addr, len, rem;
int ret, err;
 
dev_dbg(nor->dev, "at 0x%llx, len %lld\n", (long long)instr->addr,
@@ -991,6 +996,19 @@ static int spi_nor_erase(struct mtd_info *mtd, struct 
erase_info *instr)
ret = -EINTR;
goto erase_err;
}
+
+   offset = addr;
+   if (nor->flags & SNOR_F_HAS_PARALLEL)
+   offset /= 2;
+
+   if (nor->flags & SNOR_F_HAS_STACKED) {
+   if (offset >= (mtd->size / 2)) {
+   offset = offset - (mtd->size / 2);
+   nor->spi->flags |= SPI_XFER_U_PAGE;
+   } else {
+   nor->spi->flags &= ~SPI_XFER_U_PAGE;
+   }
+   }
 #ifdef CONFIG_SPI_FLASH_BAR
ret = write_bar(nor, addr);
if (ret < 0)
@@ -1396,6 +1414,9 @@ static const struct flash_info *spi_nor_read_id(struct 
spi_nor *nor)
u8  id[SPI_NOR_MAX_ID_LEN];
const struct flash_info *info;
 
+   if (nor->flags & SNOR_F_HAS_PARALLEL)
+   nor->spi->flags |= SPI_XFER_LOWER;
+
tmp = nor->read_reg(nor, SPINOR_OP_RDID, id, SPI_NOR_MAX_ID_LEN);
if (tmp < 0) {
dev_dbg(nor->dev, "error %d reading JEDEC ID\n", tmp);
@@ -1420,28 +1441,66 @@ static int spi_nor_read(struct mtd_info *mtd, loff_t 
from, size_t len,
 {
struct spi_nor *nor = mtd_to_spi_nor(mtd);
int ret;
+   u32 offset = from;
+   u32 stack_shift = 0;
+   u32 read_len = 0;
+   u32 rem_bank_len = 0;
+   u8 bank;
+   u8 is_ofst_odd = 0;
 
dev_dbg(nor->dev, "from 0x%08x, len %zd\n", (u32)from, len);
 
+   if ((nor->flags & SNOR_F_HAS_PARALLEL) && (offset & 1)) {
+   /* We can hit this case when we use file system like ubifs */
+   from = (loff_t)(from - 1);
+   len = (size_t)(len + 1);
+   is_ofst_odd = 1;
+   }
+
while (len) {
-   loff_t addr = from;
-   size_t read_len = len;
+   if (nor->addr_width == 3) {
+   if (nor->flags & SNOR_F_HAS_PARALLEL) {
+   bank = (u32)from / (SZ_16M << 0x01);
+   rem_bank_len = ((SZ_16M << 0x01) *
+

[PATCH v9 6/8] spi: zynq_qspi: Add parallel memories support in QSPI driver

[Venkatesh Yadav Abbarapu]

Add support for parallel memories in zynq_qspi.c driver. In case of
parallel memories STRIPE bit is set and sent to the qspi ip, which will
send data bits to both the flashes in parallel. However for few commands
we should not use stripe, instead send same data to both the flashes.
Those commands are exclueded by using zynqmp_qspi_update_stripe().

Also update copyright info for this file.

Signed-off-by: Ashok Reddy Soma 
Signed-off-by: Venkatesh Yadav Abbarapu 
---
 drivers/spi/zynq_qspi.c | 113 
 1 file changed, 102 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/zynq_qspi.c b/drivers/spi/zynq_qspi.c
index bc82acd0b6..41f7ae2ab2 100644
--- a/drivers/spi/zynq_qspi.c
+++ b/drivers/spi/zynq_qspi.c
@@ -1,7 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- * (C) Copyright 2013 Xilinx, Inc.
+ * (C) Copyright 2013 - 2022, Xilinx, Inc.
  * (C) Copyright 2015 Jagan Teki 
+ * (C) Copyright 2023, Advanced Micro Devices, Inc.
  *
  * Xilinx Zynq Quad-SPI(QSPI) controller driver (master mode only)
  */
@@ -13,10 +14,12 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include "../mtd/spi/sf_internal.h"
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -42,6 +45,21 @@ DECLARE_GLOBAL_DATA_PTR;
 #define ZYNQ_QSPI_TXD_00_01_OFFSET 0x80/* Transmit 1-byte inst */
 #define ZYNQ_QSPI_TXD_00_10_OFFSET 0x84/* Transmit 2-byte inst */
 #define ZYNQ_QSPI_TXD_00_11_OFFSET 0x88/* Transmit 3-byte inst */
+#define ZYNQ_QSPI_FR_QOUT_CODE 0x6B/* read instruction code */
+
+#define QSPI_SELECT_LOWER_CSBIT(0)
+#define QSPI_SELECT_UPPER_CSBIT(1)
+
+/*
+ * QSPI Linear Configuration Register
+ *
+ * It is named Linear Configuration but it controls other modes when not in
+ * linear mode also.
+ */
+#define ZYNQ_QSPI_LCFG_TWO_MEM_MASK 0x4000 /* QSPI Enable Bit Mask */
+#define ZYNQ_QSPI_LCFG_SEP_BUS_MASK 0x2000 /* QSPI Enable Bit Mask */
+#define ZYNQ_QSPI_LCFG_U_PAGE   0x1000 /* QSPI Upper memory set */
+#define ZYNQ_QSPI_LCFG_DUMMY_SHIFT  8
 
 #define ZYNQ_QSPI_TXFIFO_THRESHOLD 1   /* Tx FIFO threshold level*/
 #define ZYNQ_QSPI_RXFIFO_THRESHOLD 32  /* Rx FIFO threshold level */
@@ -101,7 +119,11 @@ struct zynq_qspi_priv {
int bytes_to_transfer;
int bytes_to_receive;
unsigned int is_inst;
+   unsigned int is_parallel;
+   unsigned int is_stacked;
+   unsigned int u_page;
unsigned cs_change:1;
+   unsigned is_strip:1;
 };
 
 static int zynq_qspi_of_to_plat(struct udevice *bus)
@@ -112,7 +134,6 @@ static int zynq_qspi_of_to_plat(struct udevice *bus)
 
plat->regs = (struct zynq_qspi_regs *)fdtdec_get_addr(blob,
  node, "reg");
-
return 0;
 }
 
@@ -147,6 +168,9 @@ static void zynq_qspi_init_hw(struct zynq_qspi_priv *priv)
/* Disable Interrupts */
writel(ZYNQ_QSPI_IXR_ALL_MASK, >idr);
 
+   /* Disable linear mode as the boot loader may have used it */
+   writel(0x0, >lqspicfg);
+
/* Clear the TX and RX threshold reg */
writel(ZYNQ_QSPI_TXFIFO_THRESHOLD, >txftr);
writel(ZYNQ_QSPI_RXFIFO_THRESHOLD, >rxftr);
@@ -164,12 +188,11 @@ static void zynq_qspi_init_hw(struct zynq_qspi_priv *priv)
confr |= ZYNQ_QSPI_CR_IFMODE_MASK | ZYNQ_QSPI_CR_MCS_MASK |
ZYNQ_QSPI_CR_PCS_MASK | ZYNQ_QSPI_CR_FW_MASK |
ZYNQ_QSPI_CR_MSTREN_MASK;
-   writel(confr, >cr);
 
-   /* Disable the LQSPI feature */
-   confr = readl(>lqspicfg);
-   confr &= ~ZYNQ_QSPI_LQSPICFG_LQMODE_MASK;
-   writel(confr, >lqspicfg);
+   if (priv->is_stacked)
+   confr |= 0x10;
+
+   writel(confr, >cr);
 
/* Enable SPI */
writel(ZYNQ_QSPI_ENR_SPI_EN_MASK, >enr);
@@ -181,6 +204,7 @@ static int zynq_qspi_child_pre_probe(struct udevice *bus)
struct zynq_qspi_priv *priv = dev_get_priv(bus->parent);
 
priv->max_hz = slave->max_hz;
+   slave->multi_cs_cap = true;
 
return 0;
 }
@@ -363,8 +387,8 @@ static void zynq_qspi_fill_tx_fifo(struct zynq_qspi_priv 
*priv, u32 size)
unsigned len, offset;
struct zynq_qspi_regs *regs = priv->regs;
static const unsigned offsets[4] = {
-   ZYNQ_QSPI_TXD_00_00_OFFSET, ZYNQ_QSPI_TXD_00_01_OFFSET,
-   ZYNQ_QSPI_TXD_00_10_OFFSET, ZYNQ_QSPI_TXD_00_11_OFFSET };
+   ZYNQ_QSPI_TXD_00_01_OFFSET, ZYNQ_QSPI_TXD_00_10_OFFSET,
+   ZYNQ_QSPI_TXD_00_11_OFFSET, ZYNQ_QSPI_TXD_00_00_OFFSET };
 
while ((fifocount < size) &&
(priv->bytes_to_transfer > 0)) {
@@ -386,7 +410,11 @@ static void zynq_qspi_fill_tx_fifo(struct zynq_qspi_priv 
*priv, u32 size)
return;
len = priv->bytes_to_transfer;
zynq_qspi_write_data(priv, , len);
-  

[PATCH v9 0/8] spi-nor: Add parallel and stacked memories support

[Venkatesh Yadav Abbarapu]

This series adds support for Xilinx qspi parallel and stacked memeories.

In parallel mode, the current implementation assumes that a maximum of
two flashes are connected. The QSPI controller splits the data evenly
between both the flashes so, both the flashes that are connected in
parallel mode should be identical.
During each operation SPI-NOR sets 0th bit for CS0 & 1st bit for CS1 in
nor->flags.

In stacked mode the current implementation assumes that a maximum of two
flashes are connected and both the flashes are of same make but can
differ in sizes. So, except the sizes all other flash parameters of both
the flashes are identical.

Spi-nor will pass on the appropriate flash select flag to low level driver,
and it will select pass all the data to that particular flash.

Write operation in parallel mode are performed in page size * 2 chunks as
each write operation results in writing both the flashes. For doubling the
address space each operation is performed at addr/2 flash offset, where addr
is the address specified by the user.

Similarly for read and erase operations it will read from both flashes, so
size and offset are divided by 2 and send to flash.

Changes in v2:
- Fixed the compilation issues.
Changes in v3:
- Fixed the CI issues.
Changes in v4:
- Removed the dio,dummy_bytes variables from zynq_qspi driver.
- Fix the compilation issue by including the DM_SPI config.
Changes in v5:
- Fixed the issue reported by buildman.
Changes in v6:
- Fixed the issues reported while running the sandbox test cases.
Changes in v7:
- Fixed the issues reported while running these da850evm_defconfig,
  imx28_xea_defconfig configs.
- Fixed the issue when DM_SPI config is disabled.
- Fixed the issue while running the sandbox_noinst_defconfig with spl
  ./spl/u-boot-spl -d arch/sandbox/dts/test.dtb
   jedec_spi_nor spi.bin@0: has no valid 'reg' property (-12)
   jedec_spi_nor spi.bin@1: has no valid 'reg' property (-12)
   ### ERROR ### Please RESET the board ###
Changes in v8:
- Fixed the compilation issue with imx28_xea_defconfig.
- Fixed the SPL size issue with the axm and taurus defconfigs.
- Rebased the patches on top of next branch.
Changes in v9:
- Updated the commit log why SPL_FIT is being enabled.

Ashok Reddy Soma (4):
  mtd: spi-nor: Add parallel and stacked memories support
  mtd: spi-nor: Add parallel memories support for read_sr and read_fsr
  mtd: spi-nor: Add parallel and stacked memories support in read_bar
and write_bar
  spi: spi-uclass: Read chipselect and restrict capabilities

Venkatesh Yadav Abbarapu (4):
  spi: zynqmp_gqspi: Add parallel memories support in GQSPI driver
  spi: zynq_qspi: Add parallel memories support in QSPI driver
  config: xea: Enable the SPL_FIT config option
  axm/taurus: Disable the SPL_SPI config

 board/siemens/taurus/taurus.c  |   4 +-
 configs/axm_defconfig  |   7 +-
 configs/imx28_xea_defconfig|   2 +-
 configs/taurus_defconfig   |   7 +-
 drivers/mtd/spi/sandbox.c  |   2 +-
 drivers/mtd/spi/spi-nor-core.c | 398 -
 drivers/spi/altera_spi.c   |   4 +-
 drivers/spi/atcspi200_spi.c|   2 +-
 drivers/spi/ath79_spi.c|   2 +-
 drivers/spi/atmel_spi.c|   6 +-
 drivers/spi/bcm63xx_hsspi.c|  42 ++--
 drivers/spi/bcm63xx_spi.c  |   6 +-
 drivers/spi/bcmbca_hsspi.c |  34 +--
 drivers/spi/cf_spi.c   |   6 +-
 drivers/spi/davinci_spi.c  |   8 +-
 drivers/spi/fsl_dspi.c |  18 +-
 drivers/spi/fsl_espi.c |   4 +-
 drivers/spi/fsl_qspi.c |   4 +-
 drivers/spi/gxp_spi.c  |   2 +-
 drivers/spi/mpc8xx_spi.c   |   4 +-
 drivers/spi/mpc8xxx_spi.c  |  10 +-
 drivers/spi/mscc_bb_spi.c  |   4 +-
 drivers/spi/mxc_spi.c  |   6 +-
 drivers/spi/npcm_fiu_spi.c |  14 +-
 drivers/spi/nxp_fspi.c |   2 +-
 drivers/spi/octeon_spi.c   |   2 +-
 drivers/spi/omap3_spi.c|   4 +-
 drivers/spi/pic32_spi.c|   2 +-
 drivers/spi/rk_spi.c   |   4 +-
 drivers/spi/rockchip_sfc.c |   2 +-
 drivers/spi/spi-aspeed-smc.c   |  28 +--
 drivers/spi/spi-mxic.c |   6 +-
 drivers/spi/spi-qup.c  |   4 +-
 drivers/spi/spi-sifive.c   |   6 +-
 drivers/spi/spi-sn-f-ospi.c|   2 +-
 drivers/spi/spi-sunxi.c|   6 +-
 drivers/spi/spi-synquacer.c|   4 +-
 drivers/spi/spi-uclass.c   |  33 ++-
 drivers/spi/stm32_qspi.c   |   2 +-
 drivers/spi/stm32_spi.c|   4 +-
 drivers/spi/ti_qspi.c  |  14 +-
 drivers/spi/xilinx_spi.c   |   6 +-
 drivers/spi/zynq_qspi.c| 119 --
 drivers/spi/zynq_spi.c |   6 +-
 drivers/spi/zynqmp_gqspi.c | 141 ++--
 include/linux/mtd/spi-nor.h|  12 +
 include/spi.h  |  23 +-
 lib/acpi/acpi_device.c |   2 +-
 48 files changed, 790 insertions(+), 240 deletions(-)

-- 
2.25.1

Re: [PATCH v4] fdt: Allow the devicetree to come from a bloblist

[Tom Rini]

On Tue, Dec 26, 2023 at 09:46:25AM +, Simon Glass wrote:

> Standard passage provides for a bloblist to be passed from one firmware
> phase to the next. That can be used to pass the devicetree along as well.
> Add an option to support this.
> 
> Tests for this will be added as part of the Universal Payload work.
> 
> Signed-off-by: Simon Glass 
> ---
> The discussion on this was not resolved and is now important due to the
> bloblist series from Raymond. So I am sending it again since I believe
> this is a better starting point than building on OF_BOARD

I really don't like adding another option for "DT is given to us". Why
isn't adding another enum to fdt_source_t sufficient, and if we have
bloblist enabled, that will look for and use if found? Maybe some other
code needs to be restructured and cleaned up too?

-- 
Tom


signature.asc
Description: PGP signature

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

[Heinrich Schuchardt]

On 10/15/23 04:45, Simon Glass wrote:

When the SMBIOS table is written to an address above 4GB a 32-bit table
address is not large enough.

Use an SMBIOS3 table in that case.

Note that we cannot use efi_allocate_pages() since this function has
nothing to do with EFI. There is no equivalent function to allocate
memory below 4GB in U-Boot. One solution would be to create a separate
malloc() pool, or just always put the malloc() pool below 4GB.

Signed-off-by: Simon Glass 
---

Changes in v2:
- Check the end of the table rather than the start.

  include/smbios.h | 22 +-
  lib/smbios.c | 24 +++-
  2 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/include/smbios.h b/include/smbios.h
index c9df2706f5a6..ddabb558299e 100644
--- a/include/smbios.h
+++ b/include/smbios.h
@@ -12,7 +12,8 @@

  /* SMBIOS spec version implemented */
  #define SMBIOS_MAJOR_VER  3
-#define SMBIOS_MINOR_VER   0
+#define SMBIOS_MINOR_VER   7
+

  enum {
SMBIOS_STR_MAX  = 64,   /* Maximum length allowed for a string */
@@ -54,6 +55,25 @@ struct __packed smbios_entry {
u8 bcd_rev;
  };

+struct __packed smbios3_entry {
+   u8 anchor[5];
+   u8 checksum;
+   u8 length;
+   u8 major_ver;
+


This empty line is superfluous.


+   u8 minor_ver;
+   u8 docrev;


'doc_rev' would be more consistent with the other fields.


+   u8 entry_point_rev;
+   u8 reserved;
+   u32 max_struct_size;
+


This empty line is superfluous.

Please, consider copying the comments from my patch

[PATCH v2,1/2] smbios: SMBIOS 3.0 (64-bit) Entry Point structure
https://patchwork.ozlabs.org/project/uboot/patch/20231223010334.248291-2-xypron.g...@gmx.de/

Otherwise looks good to me.

Heinrich


+   u64 struct_table_address;
+};
+
+/* These two structures should use the same amount of 16-byte-aligned space */
+static_assert(ALIGN(16, sizeof(struct smbios_entry)) ==
+ ALIGN(16, sizeof(struct smbios3_entry)));
+
  /* BIOS characteristics */
  #define BIOS_CHARACTERISTICS_PCI_SUPPORTED(1 << 7)
  #define BIOS_CHARACTERISTICS_UPGRADEABLE  (1 << 11)
diff --git a/lib/smbios.c b/lib/smbios.c
index c7a557bc9b7b..92e98388084f 100644
--- a/lib/smbios.c
+++ b/lib/smbios.c
@@ -487,7 +487,11 @@ ulong write_smbios_table(ulong addr)
addr = ALIGN(addr, 16);
start_addr = addr;

-   addr += sizeof(struct smbios_entry);
+   /*
+* So far we don't know which struct will be used, but they both end
+* up using the same amount of 16-bit-aligned space
+*/
+   addr += max(sizeof(struct smbios_entry), sizeof(struct smbios3_entry));
addr = ALIGN(addr, 16);
tables = addr;

@@ -512,14 +516,24 @@ ulong write_smbios_table(ulong addr)
 * sandbox's DRAM buffer.
 */
table_addr = (ulong)map_sysmem(tables, 0);
-   if (sizeof(table_addr) > sizeof(u32) && table_addr > (ulong)UINT_MAX) {
+   if (sizeof(table_addr) > sizeof(u32) && addr >= (ulong)UINT_MAX) {
+   struct smbios3_entry *se;
/*
 * We need to put this >32-bit pointer into the table but the
 * field is only 32 bits wide.
 */
-   printf("WARNING: SMBIOS table_address overflow %llx\n",
-  (unsigned long long)table_addr);
-   addr = 0;
+   printf("WARNING: Using SMBIOS3.0 due to table-address overflow 
%lx\n",
+  table_addr);
+   se = map_sysmem(start_addr, sizeof(struct smbios_entry));
+   memset(se, '\0', sizeof(struct smbios_entry));
+   memcpy(se->anchor, "_SM3_", 5);
+   se->length = sizeof(struct smbios3_entry);
+   se->major_ver = SMBIOS_MAJOR_VER;
+   se->minor_ver = SMBIOS_MINOR_VER;
+   se->docrev = 0;
+   se->entry_point_rev = 1;
+   se->max_struct_size = len;
+   se->struct_table_address = table_addr;
} else {
struct smbios_entry *se;

Re: [PATCH v2 2/6] smbios: Move the rest of the SMBIOS2 code

[Heinrich Schuchardt]

On 10/15/23 04:45, Simon Glass wrote:

Move all of this logic into the else clause, since it will not be used
for SMBIOS3

Signed-off-by: Simon Glass


Reviewed-by: Heinrich Schuchardt 

Re: [PATCH v2 1/6] smbios: Refactor 32-bit code into an else statement

[Heinrich Schuchardt]

On 10/15/23 04:45, Simon Glass wrote:

In preparation for adding support for SMBIOS3 move this code into an
else statement. There is no functional change.

Signed-off-by: Simon Glass 


Reviewed-by: Heinrich Schuchardt 

[PATCH v2 1/1] lib: smbios: verify_checksum() is duplicate

[Heinrich Schuchardt]

The function verify_checksum() duplicates what table_compute_checksum()
does. Replace it. table_compute_checksum() is always compiled.

Fixes: 415eab0655a8 ("smbios: add parsing API")
Signed-off-by: Heinrich Schuchardt 
Reviewed-by: Simon Glass 
---
v2:
mention that table_compute_checksum() is always compiled
---
 lib/smbios-parser.c | 18 ++
 1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/lib/smbios-parser.c b/lib/smbios-parser.c
index b578c30840..110670343d 100644
--- a/lib/smbios-parser.c
+++ b/lib/smbios-parser.c
@@ -5,23 +5,9 @@
 
 #define LOG_CATEGORY   LOGC_BOOT
 
+#include 
 #include 
 
-static inline int verify_checksum(const struct smbios_entry *e)
-{
-   /*
-* Checksums for SMBIOS tables are calculated to have a value, so that
-* the sum over all bytes yields zero (using unsigned 8 bit arithmetic).
-*/
-   u8 *byte = (u8 *)e;
-   u8 sum = 0;
-
-   for (int i = 0; i < e->length; i++)
-   sum += byte[i];
-
-   return sum;
-}
-
 const struct smbios_entry *smbios_entry(u64 address, u32 size)
 {
const struct smbios_entry *entry = (struct smbios_entry 
*)(uintptr_t)address;
@@ -32,7 +18,7 @@ const struct smbios_entry *smbios_entry(u64 address, u32 size)
if (memcmp(entry->anchor, "_SM_", 4))
return NULL;
 
-   if (verify_checksum(entry))
+   if (table_compute_checksum(entry, entry->length))
return NULL;
 
return entry;
-- 
2.43.0

Re: [PATCH 4/8] dts: Add alternative location for upstream DTB builds

[Sumit Garg]

On Tue, 26 Dec 2023 at 15:17, Simon Glass  wrote:
>
> Hi Sumit,
>
> On Wed, Dec 20, 2023 at 12:01 PM Sumit Garg  wrote:
> >
> > Hi Simon,
> >
> > On Wed, 20 Dec 2023 at 10:17, Simon Glass  wrote:
> > >
> > > Hi Sumit,
> > >
> > > On Thu, 14 Dec 2023 at 06:52, Sumit Garg  wrote:
> > > >
> > > > Allow platform owners to mirror devicetree files from devitree-rebasing
> > > > directory into dts/arch/$(ARCH) (special case for dts/arch/arm64). Then
> > > > build then along with any *-u-boot.dtsi file present in arch/$(ARCH)/dts
> > > > directory.
> > >
> > > Also add a new Makefile for arm64.
> >
> > I suppose you are referring to dts/arch/arm64/Makefile which has been
> > added by this patch.
>
> Yes, I just mean that you should mention this in the commit message
>

Okay, I will append the commit message.

> >
> > >
> > > >
> > > > This will help easy migration for platforms which currently are 
> > > > compliant
> > > > with upstream Linux kernel devicetree files.
> > > >
> > > > Signed-off-by: Sumit Garg 
> > > > ---
> > > >  dts/Kconfig | 11 +++
> > > >  dts/Makefile| 17 ++---
> > > >  dts/arch/arm64/Makefile | 14 ++
> > > >  3 files changed, 39 insertions(+), 3 deletions(-)
> > > >  create mode 100644 dts/arch/arm64/Makefile
> > > >
> > > > diff --git a/dts/Kconfig b/dts/Kconfig
> > > > index 00c0aeff893..96396f12b67 100644
> > > > --- a/dts/Kconfig
> > > > +++ b/dts/Kconfig
> > > > @@ -85,6 +85,17 @@ config OF_LIVE
> > > >   enables a live tree which is available after relocation,
> > > >   and can be adjusted as needed.
> > > >
> > > > +config OF_UPSTREAM
> > > > +   bool "Enable use of devicetree imported from Linux kernel 
> > > > release"
> > > > +   help
> > > > + Traditionally, U-boot platforms used to have their custom 
> > > > devicetree
> > >
> > > U-Boot
> > >
> > > (I think I mentioned this, but you thought I said U-boot...but it
> > > really is U-Boot). Perhaps grep your patches next time.
> >
> > Ah, I see. It looks like I missed that bit, and will take care of it
> > in the next revision.
> >
> > >
> > > > + files or copy devicetree files from Linux kernel which are 
> > > > hard to
> > > > + maintain and can usually get out-of-sync from Linux kernel. 
> > > > This
> > > > + option enables platforms to migrate to devicetree-rebasing 
> > > > repo where
> > > > + a regular sync will be maintained every major Linux kernel 
> > > > release
> > > > + cycle. However, platforms can still have some custom u-boot 
> > > > specific
> > > > + bits maintained as part of *-u-boot.dtsi files.
> > >
> > > I'm a bit nervous about this. It means that boards chose between this
> > > dir of the local files. It means there is some effort to switch.
> > >
> > > I wonder if we could default to using the rebasing thing, with boards
> > > having to 'opt out'? So this should be 'default y' ?
> >
> > Ideally that should be our migration path going forward once we get
> > enough boards migrated to use rebasing repo. But at this initial stage
> > we have to put some effort on migrating boards to use rebasing
> > subtree, although effort should be just adding (given DT bindings
> > compliance) following to defconfig (see patch #7):
> >
> > CONFIG_OF_UPSTREAM=y
> > CONFIG_DEFAULT_DEVICE_TREE="amlogic/meson-gxbb-nanopi-k2"
> >
> > and create a mirrored copy of:
> > ../../../devicetree-rebasing/src/arm64/amlogic/ into
> > dts/arch/arm64/amlogic/ with modifications to add targets to
> > dts/arch/arm64/Makefile.
> >
> > I am happy to put in that effort but certainly for new board support
> > that would like to import DT from Linux it should be the default. That
> > stands true for the Qcom platform series for which Caleb is currently
> > working on.
>
> The problem I see is that this is board-by-board, so will never
> happen. Changing one board (e.g. rockpro64-rk3399) typically involves
> changing the .dtsi files it includes (e.g. rk3399.dtsi) so it is
> hard/impossible to do one board without doing all.
>
> So this is why I believe the setting should be on an SoC basis, not on
> a board basis.

Sure, I will enable OF_UPSTREAM on a per SoC basis.

>
> >
> > >
> > >
> > > > +
> > > >  choice
> > > > prompt "Provider of DTB for DT control"
> > > > depends on OF_CONTROL
> > > > diff --git a/dts/Makefile b/dts/Makefile
> > > > index 3437e54033d..8098bf8191a 100644
> > > > --- a/dts/Makefile
> > > > +++ b/dts/Makefile
> > > > @@ -10,10 +10,20 @@ ifeq ($(DEVICE_TREE),)
> > > >  DEVICE_TREE := unset
> > > >  endif
> > > >
> > > > +ifeq ($(CONFIG_OF_UPSTREAM),y)
> > > > +ifeq ($(CONFIG_ARM64),y)
> > > > +DEVICE_TREE_LOC := dts/arch/arm64
> > >
> > > dt_dir ?
> >
> > Okay I will rename it.
> >
> > >
> > > Should we move the arm64 DTs to arch/arm64 ?
> >
> > IMO, the migration path should be to use rebasing subtree via
> > dts/arch/arm64/ and dropping DTs except *-u-boot.dtsi from
> > 

Re: [PATCH 5/5] test: unit test for smbios command

[Heinrich Schuchardt]

On 12/26/23 10:47, Simon Glass wrote:

Hi Heinrich,

On Sat, Dec 23, 2023 at 12:49 AM Heinrich Schuchardt  wrote:


Provide a unit test for the smbios command.

Provide different test functions for QEMU, sandbox, and other systems.

Signed-off-by: Heinrich Schuchardt 
---
  test/py/tests/test_smbios.py | 47 
  1 file changed, 47 insertions(+)
  create mode 100644 test/py/tests/test_smbios.py


Can you please write this test in C?

Regards,
Simon


According to our documentation we can use either Python or C tests.
We generally use Python tests for testing the CLI.

I cannot see any benefit in trying to write this in C.

Best regards

Heinrich

Re: [PATCH v2 3/8] scripts/Makefile.lib: Statically define *-u-boot.dtsi files location

[Sumit Garg]

On Tue, 26 Dec 2023 at 15:17, Simon Glass  wrote:
>
> Hi Sumit,
>
> On Fri, Dec 22, 2023 at 6:12 AM Sumit Garg  wrote:
> >
> > Allow u-boot to build DTB from a different directory tree such that
> > *-u-boot.dtsi files can be included from a common location. Currently
> > that location is arch/$(ARCH)/dts/, so statically define that common
> > location.
> >
> > This is needed for platform owners to start building DTB files from
> > devicetree-rebasing directory but still being able to include
> > *-u-boot.dtsi files.
> >
>
> This is a v2 patch but I don't see a change log?

I usually prefer to keep change logs in the cover letter [1].

[1] https://lore.kernel.org/all/20231222061208.3009970-1-sumit.g...@linaro.org/

>
> > Signed-off-by: Sumit Garg 
> > ---
> >  scripts/Makefile.lib | 25 ++---
> >  1 file changed, 14 insertions(+), 11 deletions(-)
>
> Reviewed-by: Simon Glass 
>

Thanks.
-Sumit

> Regards,
> Simon

Re: [PATCH 0/8] An effort to bring DT bindings compliance within U-boot

[Sumit Garg]

Hi Simon,

On Tue, 26 Dec 2023 at 15:17, Simon Glass  wrote:
>
> Hi Sumit,
>
> On Fri, Dec 22, 2023 at 5:34 AM Sumit Garg  wrote:
> >
> > On Thu, 21 Dec 2023 at 20:48, Simon Glass  wrote:
> > >
> > > Hi,
> > >
> > > On Thu, Dec 21, 2023 at 3:03 PM Tom Rini  wrote:
> > > >
> > > > On Thu, Dec 14, 2023 at 07:20:55PM +0530, Sumit Garg wrote:
> > > >
> > > > > Prerquisite
> > > > > ---
> > > > >
> > > > > This patch series requires devicetree-rebasing git repo to be added 
> > > > > as a
> > > > > subtree to the main U-boot repo via:
> > > > >
> > > > > $ git subtree add --prefix devicetree-rebasing \
> > > > >   
> > > > > git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git
> > > > >  \
> > > > >   v6.6-dts --squash
> > > >
> > > > So, I've played with subtree a little and I think this is the right way
> > > > forward in these cases. If anyone wants to take a look at how this works
> > > > in practice, take a look at:
> > > > https://source.denx.de/u-boot/u-boot/-/commits/WIP/u-boot-with-devicetree-rebasing-since-v6.1/?ref_type=heads
> > > > In that tree I started with the v6.1-dts tag, sync'd all the configs (to
> > > > have an example of a normal commit) and then did a merge of each tag
> > > > until v6.6-dts, so provide some history. And git log looks like what I
> > > > want to see, the squash commit has clear references to what we are
> > > > getting and I make a merge commit that says what I did. If you pull the
> > > > tree and checkout the branch, all the code is right there already,
> > > > nothing further to do. Same with tarball releases. The only thing I
> > > > don't like is the size growth there, but we'll reclaim some of it when
> > > > we delete our obsolete bindings, and then obsolete dts files.
> > >
> > > I spent a bit of time with subtree as well, as part of reviewing this
> > > series, using the instructions Sumit provided. It seems OK to me. We
> > > have to accept that it adds code and there will be changes/churn, but
> > > it is not too different to accepting patches on those files within
> > > U-Boot. We will bring in files which U-Boot doesn't use, but U-Boot
> > > does support a good proportion of the boards supported by Linux, so I
> > > don't see that as a big cost.
> > >
> > > From my experimentation, subtrees seem to have no impact on buildman,
> > > which is great. Am I missing anything?
> >
> > No it shouldn't cause any regression on existing tools/CI/build
> > systems. It is just a version controlled way of importing third party
> > source code as a tarball.
> >
> > >
> > > I still worry about the board-level 'switch' between U-Boot DT and
> > > upstream ones. I believe that should be at the SoC level instead.
> > >
> >
> > Probably I wasn't clear enough in my earlier replies but this series
> > motivates for a SoC level switch only. Patch #7 is essentially a
> > switch for Amlogic meson-gxbb SoC and its derived boards.
>
> OK good...but that's not quite what I mean. You have a fragment like
> this in multiple defconfig files:
>
> +CONFIG_OF_UPSTREAM=y
> +CONFIG_DEFAULT_DEVICE_TREE="amlogic/meson-gxbb-odroidc2"
>
> instead, OF_UPSTREAM should be enabled via 'imply' for the SoC, in the
> Kconfig.

Okay I got your point. It makes sense to me. I will do that for v3.

> We should not have to specify the filename like this. It is
> laborious and error-prone.

The only differentiation in naming here is just silicon vendor prefix
addition (amlogic/ in this case). The reason for this being that I
just want to mirror the whole silicon vendor directory from DT
rebasing rather than mirroring individual DT files. Given this do you
have any better alternatives?

-Sumit

Re: ACPI: usage of sandbox virtual addresses

[Heinrich Schuchardt]

On 12/26/23 10:50, Simon Glass wrote:

Hi Heinrich,

On Tue, Dec 26, 2023 at 8:56 AM Heinrich Schuchardt  wrote:


Hello Simon,

currently we use sandbox virtual addresses in all ACPI tables. This
means that an application started by the U-Boot sandbox consuming these
tables will crash due to accessing invalid addresses.

Shouldn't the ACPI tables be migrated to use valid pointers?


The confusion arises due to the difference between addresses and
pointers. If we store addresses in the ACPI tables, then things should
work.

My approach has been to avoid using casts, but instead use
map_sysmem() and mem_to_sysmem().

Which particular piece of code is wrong in this case?


Tables DSDT, XSDT, RSDP, FADT in the sandbox contain sandbox virtual
addresses instead of pointers to real host memory.

All code referring to these tables should be changed.

Best regards

Heinrich

Re: [PATCH 2/5] smbios: type2: contained object handles

[Heinrich Schuchardt]

On 12/26/23 10:50, Simon Glass wrote:

Hi Heinrich,

On Sat, Dec 23, 2023 at 12:49 AM Heinrich Schuchardt  wrote:


The type 2 structure must include information about the contained objects.
It is fine to set the number of contained object handles to 0.

Add the missing field.

Fixes: 721e992a8af5 ("x86: Add SMBIOS table support")
Signed-off-by: Heinrich Schuchardt 
---
  include/smbios.h | 1 +
  1 file changed, 1 insertion(+)

diff --git a/include/smbios.h b/include/smbios.h
index e601283d29..88c19ae062 100644
--- a/include/smbios.h
+++ b/include/smbios.h
@@ -139,6 +139,7 @@ struct __packed smbios_type2 {
 u8 chassis_location;
 u16 chassis_handle;
 u8 board_type;
+   u8 number_contained_objects;
 char eos[SMBIOS_STRUCT_EOS_BYTES];
  };

--
2.43.0


I thought I already saw this patch elsewhere, but this one is not a v2?


I should have marked it as re-sent.

Best regards

Heinrich

Re: [PATCH 4/5] doc: man-page for smbios command

[Simon Glass]

On Sat, Dec 23, 2023 at 12:49 AM Heinrich Schuchardt  wrote:
>
> Provide a man-page for the smbios command.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  doc/usage/cmd/smbios.rst | 93 
>  doc/usage/index.rst  |  1 +
>  2 files changed, 94 insertions(+)
>  create mode 100644 doc/usage/cmd/smbios.rst
>

Reviewed-by: Simon Glass 

> diff --git a/doc/usage/cmd/smbios.rst b/doc/usage/cmd/smbios.rst
> new file mode 100644
> index 00..1ffd706d7d
> --- /dev/null
> +++ b/doc/usage/cmd/smbios.rst
> @@ -0,0 +1,93 @@
> +.. SPDX-License-Identifier: GPL-2.0-or-later:
> +
> +smbios command
> +==
> +
> +Synopsis
> +
> +
> +::
> +
> +smbios
> +
> +Description
> +---
> +
> +The smbios command displays information from the SMBIOS tables.
> +
> +Examples
> +
> +
> +The example below shows an example output of the smbios command.
> +
> +::

It doesn't matter, but can't that :: go on the previous line?

> +
> +=> smbios
> +SMBIOS 2.8.0 present.
> +8 structures occupying 81 bytes
> +Table at 0x6d35018
> +
> +Handle 0x0100, DMI type 1, 27 bytes at 0x6d35018
> +System Information
> +Manufacturer: QEMU
> +Product Name: Standard PC (i440FX + PIIX, 1996)
> +Version: pc-i440fx-2.5
> +Serial Number:
> +UUID ----
> +Wake Up Type:
> +Serial Number:
> +SKU Number:
> +
> +Handle 0x0300, DMI type 3, 22 bytes at 0x6d35069
> +Header and Data:
> +: 03 16 00 03 01 01 02 00 00 03 03 03 02 00 00 00
> +0010: 00 00 00 00 00 00
> +Strings:
> +String 1: QEMU
> +String 2: pc-i440fx-2.5
> +
> +Handle 0x0400, DMI type 4, 42 bytes at 0x6d35093
> +Header and Data:
> +: 04 2a 00 04 01 03 01 02 63 06 00 00 fd ab 81 07
> +0010: 03 00 00 00 d0 07 d0 07 41 01 ff ff ff ff ff ff
> +0020: 00 00 00 01 01 01 02 00 01 00
> +Strings:
> +String 1: CPU 0
> +String 2: QEMU
> +String 3: pc-i440fx-2.5
> +
> +Handle 0x1000, DMI type 16, 23 bytes at 0x6d350d7
> +Header and Data:
> +: 10 17 00 10 01 03 06 00 00 02 00 fe ff 01 00 00
> +0010: 00 00 00 00 00 00 00
> +
> +Handle 0x1100, DMI type 17, 40 bytes at 0x6d350f0
> +Header and Data:
> +: 11 28 00 11 00 10 fe ff ff ff ff ff 80 00 09 00
> +0010: 01 00 07 02 00 00 00 02 00 00 00 00 00 00 00 00
> +0020: 00 00 00 00 00 00 00 00
> +Strings:
> +String 1: DIMM 0
> +String 2: QEMU
> +
> +Handle 0x1300, DMI type 19, 31 bytes at 0x6d35125
> +Header and Data:
> +: 13 1f 00 13 00 00 00 00 ff ff 01 00 00 10 01 00
> +0010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> +
> +Handle 0x2000, DMI type 32, 11 bytes at 0x6d35146
> +Header and Data:
> +: 20 0b 00 20 00 00 00 00 00 00 00
> +
> +Handle 0x7f00, DMI type 127, 4 bytes at 0x6d35153
> +End Of Table
> +
> +Configuration
> +-
> +
> +The command is only available if CONFIG_CMD_SMBIOS=y.
> +
> +Return value
> +
> +
> +The return value $? is 0 (true) on success, 1 (false) otherwise.
> diff --git a/doc/usage/index.rst b/doc/usage/index.rst
> index 1a626c03c2..eb2b2311f8 100644
> --- a/doc/usage/index.rst
> +++ b/doc/usage/index.rst
> @@ -102,6 +102,7 @@ Shell commands
> cmd/size
> cmd/sleep
> cmd/sm
> +   cmd/smbios
> cmd/sound
> cmd/source
> cmd/temperature
> --
> 2.43.0
>

Regards,
Simon

Re: [PATCH 1/1] lib: smbios: remove redundant next_header()

[Simon Glass]

On Fri, Dec 22, 2023 at 5:59 PM Heinrich Schuchardt  wrote:
>
> next_header() and get_next_header() only differ in how the const attribute
> is used. One function taking a const parameter and returning a non-const is
> good enough.
>
> Fixes: 3d49ee8510d3 ("efi_loader: add SMBIOS table measurement")

Yes, please drop that

> Signed-off-by: Heinrich Schuchardt 
> ---
>  lib/smbios-parser.c | 11 ++-
>  1 file changed, 2 insertions(+), 9 deletions(-)

Reviewed-by: Simon Glass 

Re: [PATCH v2 1/2] smbios: SMBIOS 3.0 (64-bit) Entry Point structure

[Simon Glass]

Hi Heinrich,

On Sat, Dec 23, 2023 at 1:03 AM Heinrich Schuchardt  wrote:
>
> From: Heinrich Schuchardt 
>
> Add definition of the SMBIOS 3.0 (64-bit) Entry Point structure.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
> v2:
> no change
> ---
>  include/smbios.h | 26 ++
>  1 file changed, 26 insertions(+)

We have this struct elsewhere, as mentioned, but this one adds
comments so is nicer.

Reviewed-by: Simon Glass 

>
> diff --git a/include/smbios.h b/include/smbios.h
> index c9df2706f5..e601283d29 100644
> --- a/include/smbios.h
> +++ b/include/smbios.h
> @@ -54,6 +54,32 @@ struct __packed smbios_entry {
> u8 bcd_rev;
>  };
>
> +/**
> + * struct smbios3_entry - SMBIOS 3.0 (64-bit) Entry Point structure
> + */
> +struct __packed smbios3_entry {
> +   /** @anchor: anchor string */
> +   u8 anchor[5];
> +   /** @checksum: checksum of the entry point structure */
> +   u8 checksum;
> +   /** @length: length of the entry point structure */
> +   u8 length;
> +   /** @major_ver: major version of the SMBIOS specification */
> +   u8 major_ver;
> +   /** @minor_ver: minor version of the SMBIOS specification */
> +   u8 minor_ver;
> +   /** @docrev: revision of the SMBIOS specification */
> +   u8 doc_rev;
> +   /** @entry_point_rev: revision of the entry point structure */
> +   u8 entry_point_rev;
> +   /** @reserved: reserved */
> +   u8 reserved;
> +   /** maximum size of SMBIOS table */
> +   u32 max_struct_size;
> +   /** @struct_table_address: 64-bit physical starting address */
> +   u64 struct_table_address;
> +};
> +
>  /* BIOS characteristics */
>  #define BIOS_CHARACTERISTICS_PCI_SUPPORTED (1 << 7)
>  #define BIOS_CHARACTERISTICS_UPGRADEABLE   (1 << 11)
> --
> 2.43.0
>

Regards,
Simon

Re: [PATCH v3 00/14] Support Firmware Handoff spec via bloblist

[Simon Glass]

Hi,

On Mon, Dec 18, 2023 at 6:19 PM Raymond Mao  wrote:
>
> Major changes:
>
> Update bloblist to align to Firmware Handoff spec v0.9 (up to commit #3592349 
> of the spec)
> (https://github.com/FirmwareHandoff/firmware_handoff).
>
> Includes:
> - Align bloblist tags with the FW handoff spec
> - Add an explicit alignment field in the header
> - Update bloblist magic and version
> - Use a packed format for blob record header
> - Change the checksum alorigthm
> - Use a void entry to handle the alignment
> - Adjust the headers of bloblist and blob record
> - Align the bloblist record data section start address
>
> In v3, the implementation from boot arguments to bloblist and how to load the
> FDT from the bloblist are moved to a forthcoming patch serie.
>
> Raymond Mao (1):
>   bloblist: Align bloblist used_size and total_size to spec
>
> Simon Glass (13):
>   bloblist: Update the tag numbering
>   bloblist: Adjust API to align in powers of 2
>   bloblist: Change the magic value
>   bloblist: Set version to 1
>   bloblist: Access record hdr_size and tag via a function
>   bloblist: Drop spare value from bloblist record
>   bloblist: Change the checksum algorithm
>   bloblist: Checksum the entire bloblist
>   bloblist: Handle alignment with a void entry
>   bloblist: Reduce blob-header size
>   bloblist: Adjust the bloblist header
>   bloblist: Add alignment to bloblist_new()
>   bloblist: Update documentation and header comment
>
>  arch/x86/lib/tables.c|   3 +-
>  common/bloblist.c| 205 ---
>  doc/develop/bloblist.rst |   4 +-
>  include/bloblist.h   | 166 ++-
>  test/bloblist.c  | 105 +++-
>  5 files changed, 287 insertions(+), 196 deletions(-)
>
> --
> 2.25.1
>

Just to say that this series LGTM

Reviewed-by: Simon Glass 

Regards,
Simonn

Re: [PATCH 2/5] smbios: type2: contained object handles

[Simon Glass]

Hi Heinrich,

On Sat, Dec 23, 2023 at 12:49 AM Heinrich Schuchardt  wrote:
>
> The type 2 structure must include information about the contained objects.
> It is fine to set the number of contained object handles to 0.
>
> Add the missing field.
>
> Fixes: 721e992a8af5 ("x86: Add SMBIOS table support")
> Signed-off-by: Heinrich Schuchardt 
> ---
>  include/smbios.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/smbios.h b/include/smbios.h
> index e601283d29..88c19ae062 100644
> --- a/include/smbios.h
> +++ b/include/smbios.h
> @@ -139,6 +139,7 @@ struct __packed smbios_type2 {
> u8 chassis_location;
> u16 chassis_handle;
> u8 board_type;
> +   u8 number_contained_objects;
> char eos[SMBIOS_STRUCT_EOS_BYTES];
>  };
>
> --
> 2.43.0

I thought I already saw this patch elsewhere, but this one is not a v2?

Regards,
Simon

Re: [PATHv2 9/9] Makefile: add dtbs to clean

[Simon Glass]

Hi Maxim,

On Mon, Dec 25, 2023 at 3:42 PM Maxim Uvarov  wrote:
>
> CI test checks that generated dtb has to be cleaned up.
>
> Signed-off-by: Maxim Uvarov 
> ---
>  Makefile | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Makefile b/Makefile
> index 220411a293..ab603191ad 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -2165,7 +2165,8 @@ CLEAN_FILES += include/autoconf.mk* include/bmp_logo.h 
> include/bmp_logo_data.h \
>mkimage-out.spl.mkimage mkimage.spl.mkimage imx-boot.map \
>itb.fit.fit itb.fit.itb itb.map spl.map 
> mkimage-out.rom.mkimage \
>mkimage.rom.mkimage mkimage-in-simple-bin* rom.map simple-bin* 
> \
> -  idbloader-spi.img lib/efi_loader/helloworld_efi.S *.itb
> +  idbloader-spi.img lib/efi_loader/helloworld_efi.S *.itb \
> +  ./*/*.dtb *.dtb
>
>  # Directories & files removed with 'make mrproper'
>  MRPROPER_DIRS  += include/config include/generated spl tpl vpl \
> --
> 2.30.2
>

Shouldn't this be handled with CLEAN_FILES in arch/xxx/dts ?

Regards,
Simon

Re: [PATHv2 6/9] mach-socfpga: do not overlap defines with lwip

[Simon Glass]

On Mon, Dec 25, 2023 at 3:42 PM Maxim Uvarov  wrote:
>
> Fix compilation issue with overlapping lwip and march defines.
>
> Signed-off-by: Maxim Uvarov 
> ---
>  arch/arm/mach-socfpga/include/mach/handoff_soc64.h | 6 --
>  arch/arm/mach-socfpga/wrap_handoff_soc64.c | 9 +
>  2 files changed, 9 insertions(+), 6 deletions(-)
>

Reviewed-by: Simon Glass 

Re: [PATHv2 3/9] net/smc911x: fix return from smc911x_send

[Simon Glass]

On Mon, Dec 25, 2023 at 3:41 PM Maxim Uvarov  wrote:
>
> return value of smc911x_send is ignored, but on sucesseful
> send we need return 0 and or error -ETIMEOUT, not opposite.
>
> Signed-off-by: Maxim Uvarov 
> Reviewed-by: Tom Rini 
> ---
>  drivers/net/smc911x.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Simon Glass 

Re: [PATHv2 5/9] driver/net/rtl8139: remove debug print

[Simon Glass]

On Mon, Dec 25, 2023 at 3:42 PM Maxim Uvarov  wrote:
>
> debug print delays reset of the driver. Finally I see
> bunch of "rx error " errors in the screen. CI can
> not handle many prints. While network works fine there
>
> Reproduced with:
> make CROSS_COMPILE=sh2-linux- r2dplus_defconfig all
> qemu-system-sh4 -M r2d -nographic -serial null \
> -serial mon:stdio -net user,tftp=`pwd` \
> -net nic,model=rtl8139 -kernel ./u-boot.bin
>
> Signed-off-by: Maxim Uvarov 
> ---
>  drivers/net/rtl8139.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Reviewed-by: Simon Glass 

Re: [PATHv2 1/9] test_net: print out net list

[Simon Glass]

On Mon, Dec 25, 2023 at 3:41 PM Maxim Uvarov  wrote:
>
> Printing net list is useful in CI log files.
>
> Signed-off-by: Maxim Uvarov 
> ---
>  test/py/tests/test_net.py | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Simon Glass 

Re: ACPI: usage of sandbox virtual addresses

[Simon Glass]

Hi Heinrich,

On Tue, Dec 26, 2023 at 8:56 AM Heinrich Schuchardt  wrote:
>
> Hello Simon,
>
> currently we use sandbox virtual addresses in all ACPI tables. This
> means that an application started by the U-Boot sandbox consuming these
> tables will crash due to accessing invalid addresses.
>
> Shouldn't the ACPI tables be migrated to use valid pointers?

The confusion arises due to the difference between addresses and
pointers. If we store addresses in the ACPI tables, then things should
work.

My approach has been to avoid using casts, but instead use
map_sysmem() and mem_to_sysmem().

Which particular piece of code is wrong in this case?

Regards,
Simon

Re: Fw: Upgrade u-boot to use a redundant environment

[Simon Glass]

Hi Adrien,

On Thu, Dec 7, 2023 at 12:53 PM Hernot, Adrien  wrote:
>
>
> Hi,
>
> We are using u-boot and until now we are using a single env storage on the 
> eMMC.
>
> We want to change to using a redundant environment, and the way to do that is 
> well documented (thanks for that).
>
> The problem we are facing is transitioning from one single to redundant, as 
> the format of the env storage is different (redund has one byte flags right 
> ater the 4 bytes CRC, at offset 4).
> After the updating the u-boot binary and restarting the machine, u-boot CRC 
> check will fail and all environment will be lost, due to the format change.
>
> What is the ussual way to deal with this?
> Is there a tool to upgrade the environment blob format?

I am not sure about that. If you can suggest a solution you could send
a patch to code or docs?

Regards,
Simon

Re: [PATCH 2/5] linux: bitmap.h: add 'for_each_set_bitrange' iteration macro

[Simon Glass]

On Mon, Dec 11, 2023 at 12:21 PM  wrote:
>
> From: Lukas Funke 
>
> Add 'for_each_set_bitrange' (from Linux kernel) in order to iterate
> over each set bitrange of a bitmap. This becomes handy if one wants
> to generate a cpu list i.e. for isolcpu or nohz_full.
>
> Signed-off-by: Lukas Funke 
> ---
>
>  include/linux/bitmap.h | 7 +++
>  1 file changed, 7 insertions(+)
>

Reviewed-by: Simon Glass 

Re: [PATCH 1/5] sandbox: add generic find_next_zero_bit implementation

[Simon Glass]

On Mon, Dec 11, 2023 at 12:21 PM  wrote:
>
> From: Lukas Funke 
>
> Add generic 'find_next_zero_bit' implementation in order to enable the
> use of the 'for_each_set_bitrange' macro. The implementation is currently
> missing for the sandbox-arch and using the function results in a linker
> error.
>
> There are more efficient implementations in the architecture specific
> implementations. However, for the sandbox the implementation should be
> simple and portable.
>
> Signed-off-by: Lukas Funke 
> ---
>
>  arch/sandbox/include/asm/bitops.h | 16 ++--
>  1 file changed, 14 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass 

Could you add a test to test/lib perhaps?

Re: [PATCH v6 5/8] binman: j721s2: Add firewall configurations

[Simon Glass]

Hi Manorit,

On Wed, Dec 6, 2023 at 9:51 AM Manorit Chawdhry  wrote:
>
> The following commits adds the configuration of firewalls required to
> protect ATF and OP-TEE memory region from non-secure reads and
> writes using master and slave firewalls present in our K3 SOCs.
>
> Signed-off-by: Manorit Chawdhry 
> ---
>  arch/arm/dts/k3-j721s2-binman.dtsi | 217 
> +
>  1 file changed, 217 insertions(+)
>

Just a minor point, but could you please reserve the 'binman' tag for
patches in tools/binman ?

Thanks,
Simon

Re: [PATCH 1/5] smbios: correct definition of gd_smbios_start() macro

[Simon Glass]

Hi Heinrich,

On Sat, Dec 23, 2023 at 12:44 AM Heinrich Schuchardt  wrote:
>
> gd_smbios_start() currently points to a non-existent field.
> It should return the field written by gd_set_smbios_start().
>
> Fixes: 50834884a815 ("Record the position of the SMBIOS tables")
> Signed-off-by: Heinrich Schuchardt 
> ---
>  include/asm-generic/global_data.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

See:

https://patchwork.ozlabs.org/project/uboot/patch/20231015024511.3995580-5-...@chromium.org/

> diff --git a/include/asm-generic/global_data.h 
> b/include/asm-generic/global_data.h
> index e8c6412e3f..c1f7818817 100644
> --- a/include/asm-generic/global_data.h
> +++ b/include/asm-generic/global_data.h
> @@ -553,7 +553,7 @@ static_assert(sizeof(struct global_data) == GD_SIZE);
>  #endif
>
>  #ifdef CONFIG_SMBIOS
> -#define gd_smbios_start()  gd->smbios_start
> +#define gd_smbios_start()  gd->arch.smbios_start
>  #define gd_set_smbios_start(addr)  gd->arch.smbios_start = addr
>  #else
>  #define gd_smbios_start()  0UL
> --
> 2.43.0
>

Regards,
SImon

Re: [PATCH v3 2/2] tools: fix build without LIBCRYPTO support

[Simon Glass]

On Thu, Dec 21, 2023 at 7:26 AM Alexander Dahl  wrote:
>
> From: Paul-Erwan Rio 
>
> Commit cb9faa6f98ae ("tools: Use a single target-independent config to
> enable OpenSSL") introduced a target-independent configuration to build
> crypto features in host tools.
>
> But since commit 2c21256b27d7 ("hash: Use Kconfig to enable hashing in
> host tools and SPL") the build without OpenSSL is broken, due to FIT
> signature/encryption features. Add missing conditional compilation
> tokens to fix this.
>
> Signed-off-by: Paul-Erwan Rio 
> Tested-by: Alexander Dahl 
> Cc: Simon Glass 
> Reviewed-by: Tom Rini 
> ---
>
> Notes:
> Added another guard around the header includes and slightly reworded the
> commit message.  Otherwise it's the same patch as before, so I kept the
> author as is and only added my Tested-by:
> I removed the Reviewed-by: from Simon from this patch, because of the
> changes mentioned and because the patch was based on an U-Boot three or
> four releases ago.
>
> v1 -> v2:
> * collected tags
>
>  include/image.h| 2 +-
>  tools/Kconfig  | 1 +
>  tools/fit_image.c  | 2 +-
>  tools/image-host.c | 4 
>  tools/mkimage.c| 5 +++--
>  5 files changed, 10 insertions(+), 4 deletions(-)
>

Reviewed-by: Simon Glass 

Re: [PATCH v3 7/9] bloblist: Load the bloblist from the previous loader

[Simon Glass]

Hi Raymond,

On Fri, Dec 22, 2023 at 9:32 PM Raymond Mao  wrote:
>
> During bloblist initialization, when CONFIG_OF_BOARD is defined,
> invoke the platform custom function to load the bloblist via boot
> arguments from the previous loader.
> If the bloblist exists, copy it into the fixed bloblist memory region.
>
> Signed-off-by: Raymond Mao 
> ---
>  common/bloblist.c  | 47 --
>  include/bloblist.h | 16 
>  2 files changed, 45 insertions(+), 18 deletions(-)
>

We should not be using OF_BOARD for this...particularly as I
understood our intent was to make it go away.

For now I would like to have OF_BLOBLIST, with OF_BOARD being the
fallback for when we need board-specific things. See [1]. It is also
very important that U-Boot reports that the DT came from a bloblist.

Perhaps I should send just that patch so that Ilias and I can figure it out.

> diff --git a/common/bloblist.c b/common/bloblist.c
> index 5ad1db137a..e66afcde64 100644
> --- a/common/bloblist.c
> +++ b/common/bloblist.c
> @@ -492,31 +492,38 @@ int bloblist_init(void)
> bool fixed = IS_ENABLED(CONFIG_BLOBLIST_FIXED);
> int ret = -ENOENT;
> ulong addr, size;
> -   bool expected;
> -
> -   /**
> -* We don't expect to find an existing bloblist in the first phase of
> -* U-Boot that runs. Also we have no way to receive the address of an
> -* allocated bloblist from a previous stage, so it must be at a fixed
> +   /*
> +* If U-Boot is not in the first phase, an existing bloblist must be
> +* at a fixed address.

BTW there is now BLOBLIST_ALLOC

> +*/
> +   bool from_addr = fixed && !u_boot_first_phase();
> +   /*
> +* If U-Boot is in the first phase that a board specific routine 
> should
> +* install the bloblist passed from previous loader to this fixed
>  * address.
>  */
> -   expected = fixed && !u_boot_first_phase();
> +   bool from_board = fixed && IS_ENABLED(CONFIG_OF_BOARD) &&
> + u_boot_first_phase();
> +
> if (spl_prev_phase() == PHASE_TPL && !IS_ENABLED(CONFIG_TPL_BLOBLIST))
> -   expected = false;
> +   from_addr = false;
> if (fixed)
> addr = IF_ENABLED_INT(CONFIG_BLOBLIST_FIXED,
>   CONFIG_BLOBLIST_ADDR);
> size = CONFIG_BLOBLIST_SIZE;
> -   if (expected) {
> +
> +   if (from_board)
> +   ret = board_bloblist_from_boot_arg(addr, size);
> +   else if (from_addr)
> ret = bloblist_check(addr, size);
> -   if (ret) {
> -   log_warning("Expected bloblist at %lx not found 
> (err=%d)\n",
> -   addr, ret);
> -   } else {
> -   /* Get the real size, if it is not what we expected */
> -   size = gd->bloblist->total_size;
> -   }
> -   }
> +
> +   if (ret)
> +   log_warning("Bloblist at %lx not found (err=%d)\n",
> +   addr, ret);
> +   else
> +   /* Get the real size */
> +   size = gd->bloblist->total_size;
> +
> if (ret) {
> if (CONFIG_IS_ENABLED(BLOBLIST_ALLOC)) {
> void *ptr = memalign(BLOBLIST_ALIGN, size);
> @@ -525,7 +532,8 @@ int bloblist_init(void)
> return log_msg_ret("alloc", -ENOMEM);
> addr = map_to_sysmem(ptr);
> } else if (!fixed) {
> -   return log_msg_ret("!fixed", ret);
> +   return log_msg_ret("BLOBLIST_FIXED is not enabled",
> +  ret);
> }
> log_debug("Creating new bloblist size %lx at %lx\n", size,
>   addr);
> @@ -538,6 +546,9 @@ int bloblist_init(void)
> return log_msg_ret("ini", ret);
> gd->flags |= GD_FLG_BLOBLIST_READY;
>
> +   bloblist_show_stats();
> +   bloblist_show_list();

Can you put those two lines behind a BLOBLIST_DEBUG option, perhaps?

> +
> return 0;
>  }
>
> diff --git a/include/bloblist.h b/include/bloblist.h
> index c1dab11f78..2f4246576e 100644
> --- a/include/bloblist.h
> +++ b/include/bloblist.h
> @@ -445,6 +445,22 @@ int bloblist_reloc(void *to, uint to_size);
>   */
>  int bloblist_init(void);
>
> +#if (IS_ENABLED(CONFIG_ARCH_QEMU) && IS_ENABLED(CONFIG_ARM))
> +/* Board custom function for qemu-arm */
> +int board_bloblist_from_boot_arg(unsigned long addr, unsigned long size);
> +#else
> +/*
> + * A board need to implement this custom function if it needs to retrieve
> + * bloblist from a previous loader
> + */
> +static inline
> +int board_bloblist_from_boot_arg(unsigned long __always_unused addr,
> +unsigned long 

Re: [PATCH v3 1/9] bloblist: add API to check the register conventions

[Simon Glass]

Hi Raymond,

On Fri, Dec 22, 2023 at 9:31 PM Raymond Mao  wrote:
>
> Add bloblist_check_reg_conv() to check whether the bloblist is compliant
> to the register conventions defined in Firmware Handoff specification.
> This API can be used for all Arm platforms.
>
> Signed-off-by: Raymond Mao 
> ---
> Changes in v2
> - Refactor of bloblist_check_reg_conv().
> Changes in v3
> - bloblist_check_reg_conv() returns -ENOENT if OF_BOARD is disabled.
>
>  common/bloblist.c  | 13 +
>  include/bloblist.h | 12 
>  2 files changed, 25 insertions(+)
>
> diff --git a/common/bloblist.c b/common/bloblist.c
> index 625e480f6b..ba17dd851a 100644
> --- a/common/bloblist.c
> +++ b/common/bloblist.c
> @@ -542,3 +542,16 @@ int bloblist_maybe_init(void)
>
> return 0;
>  }
> +
> +int bloblist_check_reg_conv(ulong rfdt, ulong rzero)
> +{
> +   if (!IS_ENABLED(CONFIG_OF_BOARD))
> +   return -ENOENT;
> +
> +   if (rzero || rfdt != (ulong)bloblist_find(BLOBLISTT_CONTROL_FDT, 0)) {
> +   gd->bloblist = NULL;  /* Reset the gd bloblist pointer */
> +   return -EIO;
> +   }

Where does the magic 4a0fb10b value get checked?

> +
> +   return 0;
> +}
> diff --git a/include/bloblist.h b/include/bloblist.h
> index 84fc943819..bd32e38a06 100644
> --- a/include/bloblist.h
> +++ b/include/bloblist.h
> @@ -461,4 +461,16 @@ static inline int bloblist_maybe_init(void)
>  }
>  #endif /* BLOBLIST */
>
> +/**
> + * bloblist_check_reg_conv() - Check whether the bloblist is compliant to
> + *the register conventions according to the
> + *Firmware Handoff spec.
> + *
> + * @rfdt:  Register that holds the FDT base address.
> + * @rzero: Register that must be zero.
> + * Return: 0 if OK, -EIO if the bloblist is not compliant to the register
> + *conventions, -ENOENT if OF_BOARD is disabled.
> + */
> +int bloblist_check_reg_conv(ulong rfdt, ulong rzero);
> +
>  #endif /* __BLOBLIST_H */
> --
> 2.25.1
>

Regards,
Simon

Re: [PATCH v3 4/9] arm: armv7: save boot arguments

[Simon Glass]

Hi Raymond,

On Fri, Dec 22, 2023 at 9:31 PM Raymond Mao  wrote:
>
> Save boot arguments r[0-3] into an array for handover of bloblist from
> previous boot stage.
>
> Signed-off-by: Raymond Mao 
> ---
> Changes in v2
> - New patch file created for v2.
> Changes in v3
> - Swap value of r0 with r2.
>
>  arch/arm/cpu/armv7/start.S | 13 +
>  1 file changed, 13 insertions(+)
>
> diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S
> index 69e281b086..2ca63ca32c 100644
> --- a/arch/arm/cpu/armv7/start.S
> +++ b/arch/arm/cpu/armv7/start.S
> @@ -152,9 +152,22 @@ ENDPROC(c_runtime_cpu_setup)
>   *
>   */
>  WEAK(save_boot_params)
> +#if (IS_ENABLED(CONFIG_OF_BOARD) && IS_ENABLED(CONFIG_BLOBLIST))
> +   ldr r12, =saved_args
> +   /* Intentionally swapping r0 with r2 */
> +   stm r12, {r2, r1, r0, r3}

I thought the instruction was a bitmap, so what does the 'swapping'
actually change?

> +#endif
> b   save_boot_params_ret@ back to my caller
>  ENDPROC(save_boot_params)
>
> +.section .data
> +.global saved_args
> +saved_args:
> +   .rept 4
> +   .word 0
> +   .endr
> +END(saved_args)
> +
>  #ifdef CONFIG_ARMV7_LPAE
>  WEAK(switch_to_hypervisor)
> b   switch_to_hypervisor_ret
> --
> 2.25.1
>

Regards,
Simon

Re: [PATCH v5 0/8] Move framebuffer reservation for SPL to RAM end

[Simon Glass]

On Wed, Dec 13, 2023 at 2:56 PM Devarsh Thakkar  wrote:
>
> Hi Antoine,
>
> On 05/12/23 21:25, Devarsh Thakkar wrote:
> > Move video memory reservation for SPL at end of RAM so that it does
> > not interefere with reservations for next stage so that the next stage
> > need not have holes in between for passed regions and instead it can
> > maintain continuity in reservations.
> >
> > Also catch the bloblist before starting reservations to avoid the same
> > problem.
> >
> > While at it, also fill missing fields in video handoff struct before
> > passing it to next stage.
> >
> > This is as per discussions at :
> > For moving SPL framebuffer reservation at end of RAM:
> >
> > https://lore.kernel.org/all/capnjgz3xsoe_g3yrqwuavoivjufz+yqgkor0ztvxgt9vk8t...@mail.gmail.com/
> >
> > For filling missing video handoff fields :
> > https://lore.kernel.org/all/CAPnjgZ1Hs0rNf0JDirp6YPsOQ5=qqqsp9g9qrwlooasuv8a...@mail.gmail.com/
> >
> > Changelog:
> > V2:
> > - Make a generic function to reserve video memory at SPL stage.
> > - Add debug prints while skipping framebuffer allocation at uboot.
> > - Correct commenting style as suggested.
> >
> > V3:
> > - Change spl_reserve_video to spl_reserve_video_from_ram_top
> >   which enforce framebuffer reservation from end of RAM.
> > - Use gd->ram_top instead of local ram_top and update
> >   gd->reloc_addr after each reservation.
> > - Print error message on framebuffer reservation.
> > - Update SPL doc with spl splash screen specific info.
> >
> > V4:
> > - Split patches into atomic commits.
> > - Remove duplicate check for video blob passed addresses and error out
> >   if invalid address/size received from blob.
> > - Improve SPL documentation memory reservation scheme and print message
> >   for video memory reservation from bloblist.
> > - Add Reviewed-By.
> >
> > V5:
> >  - Add comment for filling video handoff size with aligned size
> >  - Use PHASE_BOARD_F while checking for U-boot proper stage
> >  - Add Reviewed-by
> >
>
> Is it possible to pull this series in if no further comments ?

Yes please, this has been pending for a while.

>
> Regards
> Devarsh
>
> > Test logs:
> > https://gist.github.com/devarsht/30a3c1591270c9ebae00714b48d33058
> >
> > Devarsh Thakkar (8):
> >   spl: Enforce framebuffer reservation from end of RAM
> >   arm: mach-k3: common: Reserve video memory from end of the RAM
> >   board: ti: am62x: evm: Remove video_setup from spl_board_init
> >   common/board_f: Catch bloblist before starting reservations
> >   video: Skip framebuffer reservation if already reserved
> >   video: Fill video handoff in video post probe
> >   doc: spl: Add info for missing Kconfigs
> >   doc: spl: Add info regarding memory reservation
> >
> >  arch/arm/mach-k3/common.c| 17 ++-
> >  board/ti/am62x/evm.c | 18 
> >  common/board_f.c | 41 +++-
> >  common/spl/spl.c | 19 +
> >  doc/develop/spl.rst  | 37 
> >  drivers/video/video-uclass.c | 40 ---
> >  include/spl.h| 10 +
> >  7 files changed, 142 insertions(+), 40 deletions(-)
> >

Regards,
Simon

Re: [PATCH v2] test/py: mii: Add tests for mii command

[Simon Glass]

Hi Love,

On Tue, Dec 5, 2023 at 12:03 PM Love Kumar  wrote:
>
> Add below test cases for mii commands:
> mii_info -To display MII PHY info
> mii_list - To list MII devices
> mii_set_device - To set MII device
> mii_read - To reads register from MII PHY address
> mii_dump - To display data from MII PHY address
>
> Signed-off-by: Love Kumar 
> ---
> Changes in v2:
>   - Get MII device list from env instead of auto-detecting it
>   - Set the MII device to its current device after testing so that it
>   won't impact next cases
> ---
>  test/py/tests/test_mii.py | 92 +++
>  1 file changed, 92 insertions(+)
>  create mode 100644 test/py/tests/test_mii.py

I'll just note that we can write tests in C now, even if they need
Python setup. See [1].

Some nits below, with which:

Reviewed-by: Simon Glass 

>
> diff --git a/test/py/tests/test_mii.py b/test/py/tests/test_mii.py
> new file mode 100644
> index ..e9bdbadffc8e
> --- /dev/null
> +++ b/test/py/tests/test_mii.py
> @@ -0,0 +1,92 @@
> +# SPDX-License-Identifier: GPL-2.0
> +# (C) Copyright 2023, Advanced Micro Devices, Inc.
> +
> +import pytest
> +import re
> +
> +"""
> +Note: This test doesn't rely on boardenv_* configuration value but they can
> +change test behavior.
> +
> +For example:
> +
> +# Setup env__mii_deive_test_skip to True if tests with ethernet PHY devices
> +# should be skipped. For example: Missing PHY device
> +env__mii_device_test_skip = True
> +
> +# Setup env__mii_device_test to set the MII device names. Test will be 
> skipped
> +# if env_mii_device_test is not set
> +env__mii_device_test = {
> +"device_list": ["eth0", "eth1"],

Please be sure to use single quotes throughout, for consistency.

> +}
> +"""
> +
> +@pytest.mark.buildconfigspec("cmd_mii")
> +def test_mii_info(u_boot_console):
> +if u_boot_console.config.env.get("env__mii_device_test_skip", False):
> +pytest.skip("MII device test is not enabled!")
> +expected_output = "PHY"
> +output = u_boot_console.run_command("mii info")
> +if not re.search(r"PHY (.+?):", output):
> +pytest.skip("PHY device does not exist!")
> +assert expected_output in output
> +
> +@pytest.mark.buildconfigspec("cmd_mii")
> +def test_mii_list(u_boot_console):
> +if u_boot_console.config.env.get("env__mii_device_test_skip", False):
> +pytest.skip("MII device test is not enabled!")
> +
> +f = u_boot_console.config.env.get("env__mii_device_test", None)
> +if not f:
> +pytest.skip("No MII device to test!")
> +
> +dev_list = f.get("device_list")
> +if len(dev_list) == 0:

if not dev_list:

> +pytest.fail("No MII device list provided via env__mii_device_test!")
> +
> +expected_output = "Current device"
> +output = u_boot_console.run_command("mii device")
> +mii_devices = (
> +re.search(r"MII devices: '(.+)'", output).groups()[0].replace("'", 
> "").split()
> +)
> +
> +assert len([x for x in dev_list if x in mii_devices]) == len(dev_list)
> +assert expected_output in output
> +
> +@pytest.mark.buildconfigspec("cmd_mii")
> +def test_mii_set_device(u_boot_console):
> +test_mii_list(u_boot_console)
> +f = u_boot_console.config.env.get("env__mii_device_test", None)
> +dev_list = f.get("device_list")
> +output = u_boot_console.run_command("mii device")
> +current_dev = re.search(r"Current device: '(.+?)'", output).groups()[0]
> +
> +for dev in dev_list:
> +u_boot_console.run_command(f"mii device {dev}")
> +output = u_boot_console.run_command("echo $?")
> +assert output.endswith("0")
> +
> +u_boot_console.run_command(f"mii device {current_dev}")
> +output = u_boot_console.run_command("mii device")
> +dev = re.search(r"Current device: '(.+?)'", output).groups()[0]
> +assert current_dev == dev
> +
> +@pytest.mark.buildconfigspec("cmd_mii")
> +def test_mii_read(u_boot_console):
> +test_mii_list(u_boot_console)
> +output = u_boot_console.run_command("mii info")
> +eth_addr = hex(int(re.search(r"PHY (.+?):", output).groups()[0], 16))
> +u_boot_console.run_command(f"mii read {eth_addr} 0")
> +output = u_boot_console.run_command("echo $?")
> +assert output.endswith("0")
> +
> +@pytest.mark.buildconfigspec("cmd_mii")
> +def test_mii_dump(u_boot_console):
> +test_mii_list(u_boot_console)
> +expected_response = "PHY control register"
> +output = u_boot_console.run_command("mii info")
> +eth_addr = hex(int(re.search(r"PHY (.+?):", output).groups()[0], 16))
> +response = u_boot_console.run_command(f"mii dump {eth_addr} 0")
> +assert expected_response in response
> +output = u_boot_console.run_command("echo $?")
> +assert output.endswith("0")
> --
> 2.25.1
>

Regards,
Simon


[1] 
https://docs.u-boot.org/en/latest/develop/tests_writing.html#mixing-python-and-c

Re: [PATCH v2 3/3] efi: Avoid using dm_scan_other()

[Simon Glass]

Hi Heinrich,

On Mon, Dec 18, 2023 at 3:02 PM Simon Glass  wrote:
>
> Hi Heinrich,
>
> On Sat, 16 Dec 2023 at 14:02, Heinrich Schuchardt  wrote:
> >
> > On 12/16/23 21:46, Simon Glass wrote:
> > > Hi,
> > >
> > > On Tue, 21 Nov 2023 at 06:21, Tom Rini  wrote:
> > >>
> > >> On Tue, Nov 21, 2023 at 01:18:09PM +0100, Heinrich Schuchardt wrote:
> > >>> On 11/12/23 21:55, Simon Glass wrote:
> >  This function is defined by bootstd so using it precludes using that
> >  feature. Use the board_early_init_r() feature instead.
> > 
> >  This requires moving quite a lot of code into the board directory, butt
> >  this is the normal place for code called by board_early_init_r()
> > 
> >  Signed-off-by: Simon Glass 
> >  ---
> > 
> >  Changes in v2:
> >  - Drop duplicate acpi_xsdt patch
> >  - Put the board_early_init_r code into board/
> > 
> > board/efi/efi-x86_app/Makefile  |   5 +
> > board/efi/efi-x86_app/efi_app.c | 205 
> >  
> > >>>
> > >>> Our target should be to enable building the EFI app on all 
> > >>> architectures.
> > >>>
> > >>> Only x86 specific code should be added to 
> > >>> board/efi/efi-x86_app/efi_app.c.
> > >>
> > >> A later enhancement to make U-Boot as an EFI app more generic would be
> > >> good, but outside the scope of this patch which is moving generic code
> > >> out from "lib" and in to "board".
> >
> > I cannot see that this patch is moving any code our of lib/.
> >
> > But why should we move generic code into board?
>
> It isn't really generic
>
> >
> > >
> > > This patch was marked as old /archived in patchwork so has been
> > > forgotten. I have marked it new and non-archived in the hope that it
> > > can be applied to -master soon.
> >
> > There is nothing x86 specific about the code. Generic code should be in
> > lib/. Please, provide a new version of the patch.
> >
> > >
> > > I believe that having the EFI app support bootstd could be a useful 
> > > addition.
> >
> > That was not disputed.

OK, I see now that your objection was putting it into board/ so I sent
a new patch for that.

It is very strange to have board_early_init_r() calling code in lib
though. We should think of a better way.

Regards,
SImon

Re: [PATCH v3 9/9] qemu-arm: get FDT from bloblist

[Simon Glass]

Hi,

On Fri, Dec 22, 2023 at 9:32 PM Raymond Mao  wrote:
>
> Get devicetree from a bloblist if it exists.
> If not, fallback to get FDT from the specified memory address.
>
> Signed-off-by: Raymond Mao 
> ---
> Changes in v2
> - Refactor of board_fdt_blob_setup().
>
>  board/emulation/qemu-arm/qemu-arm.c | 12 ++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>

I believe this patch will not be needed, since we have fdtdec_setup()
to do this.

> diff --git a/board/emulation/qemu-arm/qemu-arm.c 
> b/board/emulation/qemu-arm/qemu-arm.c
> index e225011bf0..d326668caf 100644
> --- a/board/emulation/qemu-arm/qemu-arm.c
> +++ b/board/emulation/qemu-arm/qemu-arm.c
> @@ -149,9 +149,17 @@ int dram_init_banksize(void)
>
>  void *board_fdt_blob_setup(int *err)
>  {
> +   void *fdt = NULL;
> *err = 0;
> -   /* QEMU loads a generated DTB for us at the start of RAM. */
> -   return (void *)CFG_SYS_SDRAM_BASE;
> +
> +   /* Check if a DTB exists in bloblist */
> +   if (IS_ENABLED(CONFIG_BLOBLIST) && !bloblist_maybe_init())
> +   fdt = bloblist_find(BLOBLISTT_CONTROL_FDT, 0);
> +   if (!fdt)
> +   /* QEMU loads a generated DTB for us at the start of RAM. */
> +   return (void *)CFG_SYS_SDRAM_BASE;
> +
> +   return fdt;
>  }
>
>  int board_bloblist_from_boot_arg(unsigned long addr, unsigned long size)
> --
> 2.25.1
>

Regards,
Simon

Re: [PATCH v2 2/2] smbios: copy QEMU tables

[Simon Glass]

Hi Heinrich,

On Sat, Dec 23, 2023 at 1:03 AM Heinrich Schuchardt  wrote:
>
> From: Heinrich Schuchardt 
>
> QEMU provides SMBIOS tables with detailed information. We should not try to
> replicate them in U-Boot.
>
> If we want to inform about U-Boot, we can add a Firmware Inventory
> Information (type 45) table in future.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
> v2:
> fix parsing of SMBIOS anchor
> enable copying on x86 and 32bit ARM/RISC-V
> ---
>  arch/x86/lib/tables.c   |   2 +-
>  drivers/misc/Kconfig|   7 ++
>  drivers/misc/Makefile   |   1 +
>  drivers/misc/qfw_smbios.c   | 197 
>  lib/efi_loader/efi_smbios.c |   4 +-
>  5 files changed, 209 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/misc/qfw_smbios.c
>
> diff --git a/arch/x86/lib/tables.c b/arch/x86/lib/tables.c
> index 5b5070f7ca..914d9de0cb 100644
> --- a/arch/x86/lib/tables.c
> +++ b/arch/x86/lib/tables.c
> @@ -61,7 +61,7 @@ static struct table_info table_list[] = {
>  #ifdef CONFIG_GENERATE_ACPI_TABLE
> { "acpi", write_acpi_tables, BLOBLISTT_ACPI_TABLES, 0x1, 0x1000},
>  #endif
> -#ifdef CONFIG_GENERATE_SMBIOS_TABLE
> +#if defined(CONFIG_GENERATE_SMBIOS_TABLE) && !defined(CONFIG_QFW_SMBIOS)
> { "smbios", write_smbios_table, BLOBLISTT_SMBIOS_TABLES, 0x1000, 
> 0x100},
>  #endif
>  };
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index e8e4400516..d2536c54e0 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -561,6 +561,13 @@ config QFW_MMIO
>   Hidden option to enable MMIO QEMU fw_cfg interface. This will be
>   selected by the appropriate QEMU board.
>
> +config QFW_SMBIOS
> +   bool
> +   default y
> +   depends on QFW && SMBIOS && !SANDBOX
> +   help
> + Hidden option to read SMBIOS tables from QEMU.
> +
>  config I2C_EEPROM
> bool "Enable driver for generic I2C-attached EEPROMs"
> depends on MISC
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index cda701d38e..64bea92f51 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -66,6 +66,7 @@ obj-y += qfw.o
>  obj-$(CONFIG_QFW_ACPI) += qfw_acpi.o
>  obj-$(CONFIG_QFW_PIO) += qfw_pio.o
>  obj-$(CONFIG_QFW_MMIO) += qfw_mmio.o
> +obj-$(CONFIG_QFW_SMBIOS) += qfw_smbios.o
>  obj-$(CONFIG_SANDBOX) += qfw_sandbox.o
>  endif
>  obj-$(CONFIG_ROCKCHIP_EFUSE) += rockchip-efuse.o
> diff --git a/drivers/misc/qfw_smbios.c b/drivers/misc/qfw_smbios.c
> new file mode 100644
> index 00..9019345783
> --- /dev/null
> +++ b/drivers/misc/qfw_smbios.c
> @@ -0,0 +1,197 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * (C) Copyright 2023 Heinrich Schuchardt 
> + */
> +
> +#define LOG_CATEGORY UCLASS_QFW
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +/**
> + * qfw_load_smbios_table() - load a QEMU firmware file
> + *
> + * @dev:   QEMU firmware device
> + * @size:  parameter to return the size of the loaded table
> + * @name:  name of the table to load
> + * Return: address of the loaded table, NULL on error
> + */
> +static void *qfw_load_smbios_table(struct udevice *dev, uint32_t *size,
> +  char *name)
> +{
> +   struct fw_file *file;
> +   struct bios_linker_entry *table;
> +
> +   file = qfw_find_file(dev, name);
> +   if (!file) {
> +   log_debug("Can't find %s\n", name);
> +   return NULL;
> +   }
> +
> +   *size = be32_to_cpu(file->cfg.size);
> +
> +   table = malloc(*size);
> +   if (!table) {
> +   log_err("Out of memory\n");
> +   return NULL;
> +   }
> +
> +   qfw_read_entry(dev, be16_to_cpu(file->cfg.select), *size, table);
> +
> +   return table;
> +}
> +
> +/**
> + * qfw_parse_smbios_anchor() - parse QEMU's SMBIOS anchor
> + *
> + * @dev:   QEMU firmware device
> + * @entry: SMBIOS 3 structure to be filled from QEMU's anchor
> + * Return: 0 for success, -ve on error
> + */
> +static int qfw_parse_smbios_anchor(struct udevice *dev,
> +  struct smbios3_entry *entry)
> +{
> +   void *table;
> +   uint32_t size;
> +   struct smbios_entry *entry2;
> +   struct smbios3_entry *entry3;
> +   const char smbios_sig[] = "_SM_";
> +   const char smbios3_sig[] = "_SM3_";
> +   int ret = 0;
> +
> +   table = qfw_load_smbios_table(dev, , "etc/smbios/smbios-anchor");
> +   if (!table)
> +   return -ENOMEM;
> +   if (!memcmp(table, smbios3_sig, sizeof(smbios3_sig) - 1)) {
> +   entry3 = table;
> +   if (entry3->length != sizeof(struct smbios3_entry)) {
> +   ret = -ENOENT;
> +   goto out;
> +   }
> +   memcpy(entry, entry3, 

Re: [PATCH 5/5] test: unit test for smbios command

[Simon Glass]

Hi Heinrich,

On Sat, Dec 23, 2023 at 12:49 AM Heinrich Schuchardt  wrote:
>
> Provide a unit test for the smbios command.
>
> Provide different test functions for QEMU, sandbox, and other systems.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  test/py/tests/test_smbios.py | 47 
>  1 file changed, 47 insertions(+)
>  create mode 100644 test/py/tests/test_smbios.py

Can you please write this test in C?

Regards,
Simon

Re: [PATCH v3 8/9] fdt: update the document and Kconfig description

[Simon Glass]

Hi Raymond,

On Fri, Dec 22, 2023 at 9:32 PM Raymond Mao  wrote:
>
> Update the document and Kconfig to describe the behavior of board
> specific custom functions when CONFIG_OF_BOARD is defined.
>
> Signed-off-by: Raymond Mao 
> ---
>  doc/develop/devicetree/control.rst | 6 +++---
>  dts/Kconfig| 7 +--
>  2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/doc/develop/devicetree/control.rst 
> b/doc/develop/devicetree/control.rst
> index cbb65c9b17..e061f4e812 100644
> --- a/doc/develop/devicetree/control.rst
> +++ b/doc/develop/devicetree/control.rst
> @@ -104,9 +104,9 @@ in u-boot.bin so you can still just flash u-boot.bin onto 
> your board. If you are
>  using CONFIG_SPL_FRAMEWORK, then u-boot.img will be built to include the 
> device
>  tree binary.
>
> -If CONFIG_OF_BOARD is defined, a board-specific routine will provide the
> -devicetree at runtime, for example if an earlier bootloader stage creates
> -it and passes it to U-Boot.
> +If CONFIG_OF_BOARD is defined, board-specific routines will provide the
> +bloblist and devicetree at runtime, for example if an earlier bootloader 
> stage
> +creates it and passes it to U-Boot.
>
>  If CONFIG_SANDBOX is defined, then it will be read from a file on
>  startup. Use the -d flag to U-Boot to specify the file to read, -D for the
> diff --git a/dts/Kconfig b/dts/Kconfig
> index 00c0aeff89..12d61dc748 100644
> --- a/dts/Kconfig
> +++ b/dts/Kconfig
> @@ -110,8 +110,11 @@ config OF_BOARD
> default y if SANDBOX || OF_HAS_PRIOR_STAGE
> help
>   If this option is enabled, the device tree is provided at runtime by
> - a custom function called board_fdt_blob_setup(). The board must
> - implement this function if it wishes to provide special behaviour.
> + a custom function called board_fdt_blob_setup().
> + If this option is enabled, bloblist is provided at runtime by a
> + custom function called board_bloblist_from_boot_arg().
> + The board must implement these functions if it wishes to provide
> + special behaviour.

Again, this spec is not board-specific, so it should work on any ARM
board, without board-specific code, just by enabling the feature.

>
>   With this option, the device tree build by U-Boot may be overridden 
> or
>   ignored. See OF_HAS_PRIOR_STAGE.
> --
> 2.25.1
>

Regards,
SImon

Re: [PATCH v3 6/9] qemu-arm: Get bloblist from boot arguments

[Simon Glass]

Hi Raymond,

On Fri, Dec 22, 2023 at 9:32 PM Raymond Mao  wrote:
>
> Add platform custom function to get bloblist from boot arguments.
> Check whether boot arguments aligns with the register conventions
> defined in FW Handoff spec v0.9.
> Add bloblist related options into qemu default config.
>
> Signed-off-by: Raymond Mao 
> ---
> Changes in v2
> - Remove low level code for copying boot arguments.
> - Refactor board_fdt_blob_setup() and remove direct access of gd->bloblist.
> Changes in v3
> - Optimize board_bloblist_from_boot_arg().
>
>  board/emulation/qemu-arm/qemu-arm.c | 30 +
>  configs/qemu_arm64_defconfig|  3 +++
>  2 files changed, 33 insertions(+)
>
> diff --git a/board/emulation/qemu-arm/qemu-arm.c 
> b/board/emulation/qemu-arm/qemu-arm.c
> index 942f1fff57..e225011bf0 100644
> --- a/board/emulation/qemu-arm/qemu-arm.c
> +++ b/board/emulation/qemu-arm/qemu-arm.c
> @@ -4,6 +4,7 @@
>   */
>
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -102,6 +103,15 @@ static struct mm_region qemu_arm64_mem_map[] = {
>  struct mm_region *mem_map = qemu_arm64_mem_map;
>  #endif
>
> +/*
> + * Boot parameters saved from start.S
> + * saved_args[0]: FDT base address
> + * saved_args[1]: Bloblist signature
> + * saved_args[2]: must be 0
> + * saved_args[3]: Bloblist base address
> + */
> +extern unsigned long saved_args[];
> +
>  int board_init(void)
>  {
> return 0;
> @@ -144,6 +154,26 @@ void *board_fdt_blob_setup(int *err)
> return (void *)CFG_SYS_SDRAM_BASE;
>  }
>
> +int board_bloblist_from_boot_arg(unsigned long addr, unsigned long size)
> +{
> +   int ret = -ENOENT;
> +
> +   if (!IS_ENABLED(CONFIG_OF_BOARD) || !IS_ENABLED(CONFIG_BLOBLIST))
> +   return -ENOENT;
> +
> +   ret = bloblist_check(saved_args[3], size);
> +   if (ret)
> +   return ret;

What about saved_args[1] ?

> +
> +   /* Check the register conventions */
> +   ret = bloblist_check_reg_conv(saved_args[0], saved_args[2]);
> +   if (!ret)
> +   /* Relocate the bloblist to the fixed address */
> +   ret = bloblist_reloc((void *)addr, CONFIG_BLOBLIST_SIZE);
> +
> +   return ret;
> +}
> +

This should be a generic function, e.g. in arch/arm

>  void enable_caches(void)
>  {
>  icache_enable();
> diff --git a/configs/qemu_arm64_defconfig b/configs/qemu_arm64_defconfig
> index c010c25a92..418f48001c 100644
> --- a/configs/qemu_arm64_defconfig
> +++ b/configs/qemu_arm64_defconfig
> @@ -69,3 +69,6 @@ CONFIG_USB_EHCI_HCD=y
>  CONFIG_USB_EHCI_PCI=y
>  CONFIG_SEMIHOSTING=y
>  CONFIG_TPM=y
> +CONFIG_BLOBLIST=y
> +CONFIG_BLOBLIST_ADDR=0x40004000
> +CONFIG_BLOBLIST_SIZE=0x4000
> --
> 2.25.1
>

Regards,
Simon

Re: [PATCH v3 5/9] arm: armv8: save boot arguments

[Simon Glass]

On Fri, Dec 22, 2023 at 9:32 PM Raymond Mao  wrote:
>
> Save boot arguments x[0-3] into an array for handover of bloblist from
> previous boot stage.
>
> Signed-off-by: Raymond Mao 
> ---
> Changes in v2
> - New patch file created for v2.
>
>  arch/arm/cpu/armv8/start.S | 14 ++
>  1 file changed, 14 insertions(+)
>

Reviewed-by: Simon Glass 

> diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S
> index 6cc1d26e5e..8e704f590e 100644
> --- a/arch/arm/cpu/armv8/start.S
> +++ b/arch/arm/cpu/armv8/start.S
> @@ -370,5 +370,19 @@ ENTRY(c_runtime_cpu_setup)
>  ENDPROC(c_runtime_cpu_setup)
>
>  WEAK(save_boot_params)
> +#if (IS_ENABLED(CONFIG_OF_BOARD) && IS_ENABLED(CONFIG_BLOBLIST))
> +   adr x9, saved_args
> +   stp x0, x1, [x9]
> +   /* Increment the address by 16 bytes for the next pair of values */

That seems obvious, but I suppose a comment here is fine.

> +   stp x2, x3, [x9, #16]
> +#endif
> b   save_boot_params_ret/* back to my caller */
>  ENDPROC(save_boot_params)
> +
> +.section .data
> +.global saved_args
> +saved_args:
> +   .rept 4
> +   .xword 0
> +   .endr
> +END(saved_args)
> --
> 2.25.1
>

Regards,
Simon

Re: [PATCH v3 3/9] bloblist: refactor of bloblist_reloc()

[Simon Glass]

On Fri, Dec 22, 2023 at 9:31 PM Raymond Mao  wrote:
>
> The current bloblist pointer and size can be retrieved from global
> data, so we don't need to pass them from the function arguments.
> This change also help to remove all external access of gd->bloblist
> outside of bloblist module.
>
> Signed-off-by: Raymond Mao 
> ---
> Changes in v2
> - New patch file created for v2.
> Changes in v3
> - Check the space size before copying the bloblist.
> - Add return code of bloblist_reloc().
>
>  common/bloblist.c  | 10 --
>  common/board_f.c   |  8 ++--
>  include/bloblist.h |  8 
>  test/bloblist.c|  6 ++
>  4 files changed, 16 insertions(+), 16 deletions(-)

Reviewed-by: Simon Glass 

Re: [PATCH v3 2/9] bloblist: check bloblist with specified buffer size

[Simon Glass]

Hi Raymond,

On Fri, Dec 22, 2023 at 9:31 PM Raymond Mao  wrote:
>
> Instead of expecting the bloblist total size to be the same as the
> pre-allocated buffer size, practically we are more interested in
> whether the pre-allocated buffer size is bigger than the bloblist
> total size.
>
> Signed-off-by: Raymond Mao 
> Reviewed-by: Ilias Apalodimas 
> ---
> Changes in v2
> - New patch file created for v2.
>
>  common/bloblist.c | 2 +-
>  test/bloblist.c   | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/common/bloblist.c b/common/bloblist.c
> index ba17dd851a..db3fbb20cf 100644
> --- a/common/bloblist.c
> +++ b/common/bloblist.c
> @@ -384,7 +384,7 @@ int bloblist_check(ulong addr, uint size)

Can you please also update the function comment for bloblist_check() ?

> return log_msg_ret("Bad magic", -ENOENT);
> if (hdr->version != BLOBLIST_VERSION)
> return log_msg_ret("Bad version", -EPROTONOSUPPORT);
> -   if (!hdr->total_size || (size && hdr->total_size != size))
> +   if (!hdr->total_size || (size && hdr->total_size > size))
> return log_msg_ret("Bad total size", -EFBIG);
> if (hdr->used_size > hdr->total_size)
> return log_msg_ret("Bad used size", -ENOENT);
> diff --git a/test/bloblist.c b/test/bloblist.c
> index 17d9dd03d0..7dab9addf8 100644
> --- a/test/bloblist.c
> +++ b/test/bloblist.c
> @@ -207,7 +207,7 @@ static int bloblist_test_checksum(struct unit_test_state 
> *uts)
> hdr->flags++;
>
> hdr->total_size--;
> -   ut_asserteq(-EFBIG, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
> +   ut_asserteq(-EIO, bloblist_check(TEST_ADDR, TEST_BLOBLIST_SIZE));
> hdr->total_size++;
>
> hdr->spare++;
> --
> 2.25.1
>

Regards,
Simon

Re: [PATCH 3/5] cmd: provide command to display SMBIOS information

[Simon Glass]

On Sat, Dec 23, 2023 at 12:49 AM Heinrich Schuchardt  wrote:
>
> U-Boot can either generated an SMBIOS table or copy it from a prior boot
> stage, e.g. QEMU.
>
> Provide a command to display the SMBIOS information.
>
> Currently only type 1 and 2 are translated to human readable text.
> Other types may be added later. Currently only a hexdump and the list of
> strings is provided for these.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  cmd/Kconfig  |   7 ++
>  cmd/Makefile |   1 +
>  cmd/smbios.c | 191 +++
>  3 files changed, 199 insertions(+)
>  create mode 100644 cmd/smbios.c

Reviewed-by: Simon Glass 

Re: [PATCH v3 3/9] arm: add ACPI fields to global data

[Simon Glass]

On Tue, Dec 19, 2023 at 12:24 PM Heinrich Schuchardt
 wrote:
>
> Add fields for the location of ACPI tables to the global data.
>
> Signed-off-by: Heinrich Schuchardt 
> ---
> v3:
> let tables in global data depend on CONFIG_(SPL_)ACPI
> v2:
> new patch
> ---
>  arch/arm/include/asm/global_data.h | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>

Reviewed-by: Simon Glass