Re: [PATCH v9 2/2] arm64: boot: Support Flat Image Tree
Hi Yamada-san, On Thu, Dec 14, 2023 at 7:12 AM Masahiro Yamada wrote: > On Thu, Dec 14, 2023 at 1:03 PM Chen-Yu Tsai wrote: > > On Sun, Dec 10, 2023 at 1:31 AM Geert Uytterhoeven > > wrote: > > > On Sat, Dec 9, 2023 at 4:29 PM Laurent Pinchart > > > wrote: > > > > On Sat, Dec 09, 2023 at 10:13:59PM +0900, Chen-Yu Tsai wrote: > > > > > On Thu, Dec 7, 2023 at 11:38 PM Laurent Pinchart > > > > > wrote: > > > > > > On Thu, Dec 07, 2023 at 10:27:23PM +0800, Chen-Yu Tsai wrote: > > > > > > > On Sun, Dec 03, 2023 at 05:34:01PM +0200, Laurent Pinchart wrote: > > > > > > > > On Fri, Dec 01, 2023 at 08:54:42PM -0700, Simon Glass wrote: > > > > > > > > > Add a script which produces a Flat Image Tree (FIT), a single > > > > > > > > > file > > > > > > > > > containing the built kernel and associated devicetree files. > > > > > > > > > Compression defaults to gzip which gives a good balance of > > > > > > > > > size and > > > > > > > > > performance. > > > > > > > > > > > > > > > > > > The files compress from about 86MB to 24MB using this > > > > > > > > > approach. > > > > > > > > > > > > > > > > > > The FIT can be used by bootloaders which support it, such as > > > > > > > > > U-Boot > > > > > > > > > and Linuxboot. It permits automatic selection of the correct > > > > > > > > > devicetree, matching the compatible string of the running > > > > > > > > > board with > > > > > > > > > the closest compatible string in the FIT. There is no need for > > > > > > > > > filenames or other workarounds. > > > > > > > > > > > > > > > > > > Add a 'make image.fit' build target for arm64, as well. Use > > > > > > > > > FIT_COMPRESSION to select a different algorithm. > > > > > > > > > > > > > > > > > > The FIT can be examined using 'dumpimage -l'. > > > > > > > > > > > > > > > > > > This features requires pylibfdt (use 'pip install libfdt'). > > > > > > > > > It also > > > > > > > > > requires compression utilities for the algorithm being used. > > > > > > > > > Supported > > > > > > > > > compression options are the same as the Image.xxx files. For > > > > > > > > > now there > > > > > > > > > is no way to change the compression other than by editing the > > > > > > > > > rule for > > > > > > > > > $(obj)/image.fit > > > > > > > > > > > > > > > > > > While FIT supports a ramdisk / initrd, no attempt is made to > > > > > > > > > support > > > > > > > > > this here, since it must be built separately from the Linux > > > > > > > > > build. > > > > > > > > > > > > > > > > FIT images are very useful, so I think this is a very welcome > > > > > > > > addition > > > > > > > > to the kernel build system. It can get tricky though: given the > > > > > > > > versatile nature of FIT images, there can't be any > > > > > > > > one-size-fits-them-all solution to build them, and striking the > > > > > > > > right > > > > > > > > balance between what makes sense for the kernel and the > > > > > > > > features that > > > > > > > > users may request will probably lead to bikeshedding. As we all > > > > > > > > love > > > > > > > > bikeshedding, I thought I would start selfishly, with a > > > > > > > > personal use > > > > > > > > case :-) This isn't a yak-shaving request though, I don't see > > > > > > > > any reason > > > > > > > > to delay merging this series. > > > > > > > > > > > > > > > > Have you envisioned building FIT images with a subset of DTBs, > > > > > > > > or adding > > > > > > > > DTBOs ? Both would be fairly trivial extensions to this script > > > > > > > > by > > > > > > > > extending the supported command line arguments. It would > > > > > > > > perhaps be more > > > > > > > > difficult to integrate in the kernel build system though. This > > > > > > > > leads me > > > > > > > > to a second question: would you consider merging extensions to > > > > > > > > this > > > > > > > > script if they are not used by the kernel build system, but > > > > > > > > meant for > > > > > > > > users who manually invoke the script ? More generally, is the > > > > > > > > script > > > > > > > > > > > > > > We'd also be interested in some customization, though in a > > > > > > > different way. > > > > > > > We imagine having a rule file that says X compatible string > > > > > > > should map > > > > > > > to A base DTB, plus B and C DTBO for the configuration section. > > > > > > > The base > > > > > > > DTB would carry all common elements of some device, while the > > > > > > > DTBOs > > > > > > > carry all the possible second source components, like different > > > > > > > display > > > > > > > panels or MIPI cameras for instance. This could drastically > > > > > > > reduce the > > > > > > > size of FIT images in ChromeOS by deduplicating all the common > > > > > > > stuff. > > > > > > > > > > > > Do you envision the "mapping" compatible string mapping to a config > > > > > > section in the FIT image, that would bundle the base DTB and the > > > > > > DTBOs ? > > > > > > > > > > That's exactly the idea. The
Re: [PATCH v9 2/2] arm64: boot: Support Flat Image Tree
On Thu, Dec 14, 2023 at 3:12 PM Masahiro Yamada wrote: > > On Thu, Dec 14, 2023 at 1:03 PM Chen-Yu Tsai wrote: > > > > On Sun, Dec 10, 2023 at 1:31 AM Geert Uytterhoeven > > wrote: > > > > > > Hi Laurent, > > > > > > On Sat, Dec 9, 2023 at 4:29 PM Laurent Pinchart > > > wrote: > > > > On Sat, Dec 09, 2023 at 10:13:59PM +0900, Chen-Yu Tsai wrote: > > > > > On Thu, Dec 7, 2023 at 11:38 PM Laurent Pinchart > > > > > wrote: > > > > > > On Thu, Dec 07, 2023 at 10:27:23PM +0800, Chen-Yu Tsai wrote: > > > > > > > On Sun, Dec 03, 2023 at 05:34:01PM +0200, Laurent Pinchart wrote: > > > > > > > > On Fri, Dec 01, 2023 at 08:54:42PM -0700, Simon Glass wrote: > > > > > > > > > Add a script which produces a Flat Image Tree (FIT), a single > > > > > > > > > file > > > > > > > > > containing the built kernel and associated devicetree files. > > > > > > > > > Compression defaults to gzip which gives a good balance of > > > > > > > > > size and > > > > > > > > > performance. > > > > > > > > > > > > > > > > > > The files compress from about 86MB to 24MB using this > > > > > > > > > approach. > > > > > > > > > > > > > > > > > > The FIT can be used by bootloaders which support it, such as > > > > > > > > > U-Boot > > > > > > > > > and Linuxboot. It permits automatic selection of the correct > > > > > > > > > devicetree, matching the compatible string of the running > > > > > > > > > board with > > > > > > > > > the closest compatible string in the FIT. There is no need for > > > > > > > > > filenames or other workarounds. > > > > > > > > > > > > > > > > > > Add a 'make image.fit' build target for arm64, as well. Use > > > > > > > > > FIT_COMPRESSION to select a different algorithm. > > > > > > > > > > > > > > > > > > The FIT can be examined using 'dumpimage -l'. > > > > > > > > > > > > > > > > > > This features requires pylibfdt (use 'pip install libfdt'). > > > > > > > > > It also > > > > > > > > > requires compression utilities for the algorithm being used. > > > > > > > > > Supported > > > > > > > > > compression options are the same as the Image.xxx files. For > > > > > > > > > now there > > > > > > > > > is no way to change the compression other than by editing the > > > > > > > > > rule for > > > > > > > > > $(obj)/image.fit > > > > > > > > > > > > > > > > > > While FIT supports a ramdisk / initrd, no attempt is made to > > > > > > > > > support > > > > > > > > > this here, since it must be built separately from the Linux > > > > > > > > > build. > > > > > > > > > > > > > > > > FIT images are very useful, so I think this is a very welcome > > > > > > > > addition > > > > > > > > to the kernel build system. It can get tricky though: given the > > > > > > > > versatile nature of FIT images, there can't be any > > > > > > > > one-size-fits-them-all solution to build them, and striking the > > > > > > > > right > > > > > > > > balance between what makes sense for the kernel and the > > > > > > > > features that > > > > > > > > users may request will probably lead to bikeshedding. As we all > > > > > > > > love > > > > > > > > bikeshedding, I thought I would start selfishly, with a > > > > > > > > personal use > > > > > > > > case :-) This isn't a yak-shaving request though, I don't see > > > > > > > > any reason > > > > > > > > to delay merging this series. > > > > > > > > > > > > > > > > Have you envisioned building FIT images with a subset of DTBs, > > > > > > > > or adding > > > > > > > > DTBOs ? Both would be fairly trivial extensions to this script > > > > > > > > by > > > > > > > > extending the supported command line arguments. It would > > > > > > > > perhaps be more > > > > > > > > difficult to integrate in the kernel build system though. This > > > > > > > > leads me > > > > > > > > to a second question: would you consider merging extensions to > > > > > > > > this > > > > > > > > script if they are not used by the kernel build system, but > > > > > > > > meant for > > > > > > > > users who manually invoke the script ? More generally, is the > > > > > > > > script > > > > > > > > > > > > > > We'd also be interested in some customization, though in a > > > > > > > different way. > > > > > > > We imagine having a rule file that says X compatible string > > > > > > > should map > > > > > > > to A base DTB, plus B and C DTBO for the configuration section. > > > > > > > The base > > > > > > > DTB would carry all common elements of some device, while the > > > > > > > DTBOs > > > > > > > carry all the possible second source components, like different > > > > > > > display > > > > > > > panels or MIPI cameras for instance. This could drastically > > > > > > > reduce the > > > > > > > size of FIT images in ChromeOS by deduplicating all the common > > > > > > > stuff. > > > > > > > > > > > > Do you envision the "mapping" compatible string mapping to a config > > > > > > section in the FIT image, that would bundle the base DTB and the > > > > > > DTBOs ? > > > > > > > > > > That's
Re: [PATCH] efi_loader: eliminate efi_disk_obj structure
Am 14. Dezember 2023 02:55:27 MEZ schrieb Masahisa Kojima
:
>Hi Heinrich,
>
>On Wed, 13 Dec 2023 at 23:22, Heinrich Schuchardt wrote:
>>
>> On 13.12.23 09:57, Masahisa Kojima wrote:
>> > Current code uses struct efi_disk_obj to keep information
>> > about block devices and partitions. As the efi handle already
>> > has a field with the udevice, we should eliminate
>> > struct efi_disk_obj and use an pointer to struct efi_object
>> > for the handle.
>> >
>> > efi_link_dev() call is moved inside of efi_disk_add_dev() function
>> > to simplify the cleanup process in case of error.
>>
>> I agree that using struct efi_disk_obj as a container for protocols of a
>> block IO device is a bit messy.
>>
>> But we should keep looking up the handle easy. Currently we use
>>
>> diskobj = container_of(this, struct efi_disk_obj, ops);
>>
>> Instead we could append a private field with the handle to the
>> EFI_BLOCK_IO_PROTOCOL structure.
>>
>> >
>> > Signed-off-by: Masahisa Kojima
>> > ---
>> > lib/efi_loader/efi_disk.c | 209 +-
>> > 1 file changed, 116 insertions(+), 93 deletions(-)
>> >
>> > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
>> > index f0d76113b0..cfb7ace551 100644
>> > --- a/lib/efi_loader/efi_disk.c
>> > +++ b/lib/efi_loader/efi_disk.c
>> > @@ -27,27 +27,24 @@ struct efi_system_partition efi_system_partition = {
>> > const efi_guid_t efi_block_io_guid = EFI_BLOCK_IO_PROTOCOL_GUID;
>> > const efi_guid_t efi_system_partition_guid = PARTITION_SYSTEM_GUID;
>> >
>> > -/**
>> > - * struct efi_disk_obj - EFI disk object
>> > - *
>> > - * @header: EFI object header
>> > - * @ops: EFI disk I/O protocol interface
>> > - * @dev_index: device index of block device
>> > - * @media: block I/O media information
>> > - * @dp: device path to the block device
>> > - * @part:partition
>> > - * @volume: simple file system protocol of the partition
>> > - * @dev: associated DM device
>> > - */
>> > -struct efi_disk_obj {
>> > - struct efi_object header;
>> > - struct efi_block_io ops;
>> > - int dev_index;
>> > - struct efi_block_io_media media;
>> > - struct efi_device_path *dp;
>> > - unsigned int part;
>> > - struct efi_simple_file_system_protocol *volume;
>> > -};
>> > +static efi_handle_t efi_blkio_find_obj(struct efi_block_io *blkio)
>> > +{
>> > + efi_handle_t handle;
>> > +
>> > + list_for_each_entry(handle, _obj_list, link) {
>> > + efi_status_t ret;
>> > + struct efi_handler *handler;
>> > +
>> > + ret = efi_search_protocol(handle, _block_io_guid,
>> > );
>> > + if (ret != EFI_SUCCESS)
>> > + continue;
>> > +
>> > + if (blkio == handler->protocol_interface)
>> > + return handle;
>> > + }
>>
>> Depending on the number of handles and pointers this will take a
>> considerable time. A private field for the handle appended to struct
>> efi_block_io would allow a fast lookup.
>>
>> EDK II does the same. See the definition of RAM_DISK_PRIVATE_FROM_BLKIO
>> which uses macro BASE_CR() to find the private fields.
>
>This patch tries to address this issue[0].
>
>If I understand correctly, two options are suggested here.
> 1) a private field for the handle appended to struct efi_block_io
> 2) keep the private struct something like current struct
>efi_disk_obj, same as EDK II does
Edk II uses 1) as I indicated above.
The UEFI specification prescribes which fields must be in the struct. In both
1) and 2) you have private fields at a known offset to the structure.
EDK II can (this is configurable) additionally add a magic value to verify that
the overall structure was provided by EDK II.
Best regards
Heinrich
>
>struct efi_block_io is defined in UEFI spec, so probably option#2 is preferable
>and it is almost the same as the current implementation.
>I think we can proceed with the minor cleanup of struct efi_disk_obj
>as Akashi-san suggested.
>
>[0] https://source.denx.de/u-boot/custodians/u-boot-efi/-/issues/9
>
>Thanks,
>Masahisa Kojima
>
>>
>> Best regards
>>
>> Heinrich
>>
>> > +
>> > + return NULL;
>> > +}
>> >
>> > /**
>> >* efi_disk_reset() - reset block device
>> > @@ -107,13 +104,16 @@ static efi_status_t efi_disk_rw_blocks(struct
>> > efi_block_io *this,
>> > u32 media_id, u64 lba, unsigned long buffer_size,
>> > void *buffer, enum efi_disk_direction direction)
>> > {
>> > - struct efi_disk_obj *diskobj;
>> > + efi_handle_t handle;
>> > int blksz;
>> > int blocks;
>> > unsigned long n;
>> >
>> > - diskobj = container_of(this, struct efi_disk_obj, ops);
>> > - blksz = diskobj->media.block_size;
>> > + handle = efi_blkio_find_obj(this);
>> > + if (!handle)
>> > + return EFI_INVALID_PARAMETER;
>> > +
>> > + blksz = this->media->block_size;
>> > blocks =
Re: [PATCH 1/7] doc: Update documentation URL
Am 13. Dezember 2023 21:39:12 MEZ schrieb Simon Glass : >Update to use the new docs.u-boot.org URL for documentation. Who owns that domain? Can we let http://www.u-boot.org forward to https://docs.u-boot.org? Best regards Heinrich > >Signed-off-by: Simon Glass >--- > > MAINTAINERS | 2 +- > README| 2 +- > doc/build/documentation.rst | 2 +- > tools/binman/pyproject.toml | 2 +- > tools/buildman/pyproject.toml | 2 +- > tools/dtoc/pyproject.toml | 2 +- > tools/patman/pyproject.toml | 2 +- > tools/u_boot_pylib/pyproject.toml | 2 +- > 8 files changed, 8 insertions(+), 8 deletions(-) > >Applied to u-boot-dm/next, thanks!
Re: [PATCH v9 2/2] arm64: boot: Support Flat Image Tree
On Thu, Dec 14, 2023 at 1:03 PM Chen-Yu Tsai wrote:
>
> On Sun, Dec 10, 2023 at 1:31 AM Geert Uytterhoeven
> wrote:
> >
> > Hi Laurent,
> >
> > On Sat, Dec 9, 2023 at 4:29 PM Laurent Pinchart
> > wrote:
> > > On Sat, Dec 09, 2023 at 10:13:59PM +0900, Chen-Yu Tsai wrote:
> > > > On Thu, Dec 7, 2023 at 11:38 PM Laurent Pinchart
> > > > wrote:
> > > > > On Thu, Dec 07, 2023 at 10:27:23PM +0800, Chen-Yu Tsai wrote:
> > > > > > On Sun, Dec 03, 2023 at 05:34:01PM +0200, Laurent Pinchart wrote:
> > > > > > > On Fri, Dec 01, 2023 at 08:54:42PM -0700, Simon Glass wrote:
> > > > > > > > Add a script which produces a Flat Image Tree (FIT), a single
> > > > > > > > file
> > > > > > > > containing the built kernel and associated devicetree files.
> > > > > > > > Compression defaults to gzip which gives a good balance of size
> > > > > > > > and
> > > > > > > > performance.
> > > > > > > >
> > > > > > > > The files compress from about 86MB to 24MB using this approach.
> > > > > > > >
> > > > > > > > The FIT can be used by bootloaders which support it, such as
> > > > > > > > U-Boot
> > > > > > > > and Linuxboot. It permits automatic selection of the correct
> > > > > > > > devicetree, matching the compatible string of the running board
> > > > > > > > with
> > > > > > > > the closest compatible string in the FIT. There is no need for
> > > > > > > > filenames or other workarounds.
> > > > > > > >
> > > > > > > > Add a 'make image.fit' build target for arm64, as well. Use
> > > > > > > > FIT_COMPRESSION to select a different algorithm.
> > > > > > > >
> > > > > > > > The FIT can be examined using 'dumpimage -l'.
> > > > > > > >
> > > > > > > > This features requires pylibfdt (use 'pip install libfdt'). It
> > > > > > > > also
> > > > > > > > requires compression utilities for the algorithm being used.
> > > > > > > > Supported
> > > > > > > > compression options are the same as the Image.xxx files. For
> > > > > > > > now there
> > > > > > > > is no way to change the compression other than by editing the
> > > > > > > > rule for
> > > > > > > > $(obj)/image.fit
> > > > > > > >
> > > > > > > > While FIT supports a ramdisk / initrd, no attempt is made to
> > > > > > > > support
> > > > > > > > this here, since it must be built separately from the Linux
> > > > > > > > build.
> > > > > > >
> > > > > > > FIT images are very useful, so I think this is a very welcome
> > > > > > > addition
> > > > > > > to the kernel build system. It can get tricky though: given the
> > > > > > > versatile nature of FIT images, there can't be any
> > > > > > > one-size-fits-them-all solution to build them, and striking the
> > > > > > > right
> > > > > > > balance between what makes sense for the kernel and the features
> > > > > > > that
> > > > > > > users may request will probably lead to bikeshedding. As we all
> > > > > > > love
> > > > > > > bikeshedding, I thought I would start selfishly, with a personal
> > > > > > > use
> > > > > > > case :-) This isn't a yak-shaving request though, I don't see any
> > > > > > > reason
> > > > > > > to delay merging this series.
> > > > > > >
> > > > > > > Have you envisioned building FIT images with a subset of DTBs, or
> > > > > > > adding
> > > > > > > DTBOs ? Both would be fairly trivial extensions to this script by
> > > > > > > extending the supported command line arguments. It would perhaps
> > > > > > > be more
> > > > > > > difficult to integrate in the kernel build system though. This
> > > > > > > leads me
> > > > > > > to a second question: would you consider merging extensions to
> > > > > > > this
> > > > > > > script if they are not used by the kernel build system, but meant
> > > > > > > for
> > > > > > > users who manually invoke the script ? More generally, is the
> > > > > > > script
> > > > > >
> > > > > > We'd also be interested in some customization, though in a
> > > > > > different way.
> > > > > > We imagine having a rule file that says X compatible string should
> > > > > > map
> > > > > > to A base DTB, plus B and C DTBO for the configuration section. The
> > > > > > base
> > > > > > DTB would carry all common elements of some device, while the DTBOs
> > > > > > carry all the possible second source components, like different
> > > > > > display
> > > > > > panels or MIPI cameras for instance. This could drastically reduce
> > > > > > the
> > > > > > size of FIT images in ChromeOS by deduplicating all the common
> > > > > > stuff.
> > > > >
> > > > > Do you envision the "mapping" compatible string mapping to a config
> > > > > section in the FIT image, that would bundle the base DTB and the
> > > > > DTBOs ?
> > > >
> > > > That's exactly the idea. The mapping compatible string could be untied
> > > > from the base board's compatible string if needed (which we probably
> > > > do).
> > > >
> > > > So something like:
> > > >
> > > > config {
> > > > config-1 {
> > > > compatible = "google,krane-sku0";
> > > > fdt =
[PATCH v6 5/6] spi: zynqmp_gqspi: Add parallel memories support in GQSPI driver
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 v6 6/6] spi: zynq_qspi: Add parallel memories support in QSPI driver
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 v6 4/6] spi: spi-uclass: Read chipselect and restrict capabilities
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| 6 +++---
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 | 23 +++-
drivers/spi/stm32_qspi.c | 2 +-
drivers/spi/stm32_spi.c | 4 ++--
drivers/spi/ti_qspi.c| 14 ++--
drivers/spi/xilinx_spi.c | 4 ++--
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, 168 insertions(+), 149 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 at91_spi
[PATCH v6 2/6] mtd: spi-nor: Add parallel memories support for read_sr and read_fsr
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 1ebb7d5c15..30dd2764c9 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 v6 3/6] mtd: spi-nor: Add parallel and stacked memories support in read_bar and write_bar
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 30dd2764c9..2c6a203f82 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 v6 1/6] mtd: spi-nor: Add parallel and stacked memories support
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 | 296 +
include/linux/mtd/spi-nor.h| 12 ++
include/spi.h | 11 ++
3 files changed, 291 insertions(+), 28 deletions(-)
diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
index 9a1801ba93..1ebb7d5c15 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,62 @@ 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 v6 0/6] spi-nor: Add parallel and stacked memories support
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. 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 (2): spi: zynqmp_gqspi: Add parallel memories support in GQSPI driver spi: zynq_qspi: Add parallel memories support in QSPI driver drivers/mtd/spi/sandbox.c | 2 +- drivers/mtd/spi/spi-nor-core.c | 401 + 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 | 6 +- 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 | 23 +- drivers/spi/stm32_qspi.c | 2 +- drivers/spi/stm32_spi.c| 4 +- drivers/spi/ti_qspi.c | 14 +- drivers/spi/xilinx_spi.c | 4 +- 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 +- 44 files changed, 773 insertions(+), 226 deletions(-) -- 2.25.1
Re: [PATCH 02/17] video: dw_hdmi: Add Vendor PHY handling
Hi Simon,
On Thu, Dec 14, 2023 at 1:21 AM Simon Glass wrote:
>
> Hi Jagan,
>
> On Mon, 11 Dec 2023 at 02:00, Jagan Teki wrote:
> >
> > From: Jagan Teki
> >
> > DW HDMI support Vendor PHY like Rockchip RK3328 Inno HDMI PHY.
> >
> > Extend the vendor phy handling by adding platform phy hooks.
> >
> > Signed-off-by: Jagan Teki
> > ---
> > drivers/video/dw_hdmi.c | 29 +++-
> > drivers/video/meson/meson_dw_hdmi.c | 11 ++-
> > drivers/video/rockchip/rk3399_hdmi.c | 8 +++-
> > drivers/video/rockchip/rk_hdmi.c | 2 +-
> > drivers/video/sunxi/sunxi_dw_hdmi.c | 11 ++-
> > include/dw_hdmi.h| 14 +-
> > 6 files changed, 69 insertions(+), 6 deletions(-)
> >
>
> Isn't there a PHY framework we should use in U-Boot?
Yes, the driver is using generic PHY.
>
> Regards,
> Sim on
>
> > diff --git a/drivers/video/dw_hdmi.c b/drivers/video/dw_hdmi.c
> > index c4fbb18294..ea12a09407 100644
> > --- a/drivers/video/dw_hdmi.c
> > +++ b/drivers/video/dw_hdmi.c
> > @@ -988,7 +988,7 @@ int dw_hdmi_enable(struct dw_hdmi *hdmi, const struct
> > display_timing *edid)
> >
> > hdmi_av_composer(hdmi, edid);
> >
> > - ret = hdmi->phy_set(hdmi, edid->pixelclock.typ);
> > + ret = hdmi->ops->phy_set(hdmi, edid->pixelclock.typ);
> > if (ret)
> > return ret;
> >
> > @@ -1009,10 +1009,37 @@ int dw_hdmi_enable(struct dw_hdmi *hdmi, const
> > struct display_timing *edid)
> > return 0;
> > }
> >
> > +static const struct dw_hdmi_phy_ops dw_hdmi_synopsys_phy_ops = {
> > + .phy_set = dw_hdmi_phy_cfg,
> > +};
> > +
> > +static void dw_hdmi_detect_phy(struct dw_hdmi *hdmi)
> > +{
> > + if (!hdmi->data)
> > + return;
> > +
> > + /* hook Synopsys PHYs ops */
> > + if (!hdmi->data->phy_force_vendor) {
> > + hdmi->ops = _hdmi_synopsys_phy_ops;
> > + return;
> > + }
> > +
> > + /* Vendor HDMI PHYs must assign phy_ops in plat_data */
> > + if (!hdmi->data->phy_ops) {
> > + printf("Unsupported Vendor HDMI phy_ops\n");
> > + return;
> > + }
> > +
> > + /* hook Vendor HDMI PHYs ops */
> > + hdmi->ops = hdmi->data->phy_ops;
> > +}
> > +
> > void dw_hdmi_init(struct dw_hdmi *hdmi)
> > {
> > uint ih_mute;
> >
> > + dw_hdmi_detect_phy(hdmi);
> > +
> > /*
> > * boot up defaults are:
> > * hdmi_ih_mute = 0x03 (disabled)
> > diff --git a/drivers/video/meson/meson_dw_hdmi.c
> > b/drivers/video/meson/meson_dw_hdmi.c
> > index 5db01904b5..63ca3ac52e 100644
> > --- a/drivers/video/meson/meson_dw_hdmi.c
> > +++ b/drivers/video/meson/meson_dw_hdmi.c
> > @@ -375,6 +375,15 @@ static int meson_dw_hdmi_wait_hpd(struct dw_hdmi *hdmi)
> > return -ETIMEDOUT;
> > }
> >
> > +static const struct dw_hdmi_phy_ops dw_hdmi_meson_phy_ops = {
> > + .phy_set = meson_dw_hdmi_phy_cfg,
> > +};
> > +
> > +static const struct dw_hdmi_plat_data dw_hdmi_meson_plat_data = {
> > + .phy_force_vendor = true,
> > + .phy_ops = _hdmi_meson_phy_ops,
> > +};
> > +
> > static int meson_dw_hdmi_probe(struct udevice *dev)
> > {
> > struct meson_dw_hdmi *priv = dev_get_priv(dev);
> > @@ -397,7 +406,7 @@ static int meson_dw_hdmi_probe(struct udevice *dev)
> >
> > priv->hdmi.hdmi_data.enc_out_bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> > priv->hdmi.hdmi_data.enc_in_bus_format = MEDIA_BUS_FMT_YUV8_1X24;
> > - priv->hdmi.phy_set = meson_dw_hdmi_phy_init;
> > + priv->hdmi.data = _hdmi_meson_plat_data;
> > if (meson_hdmi_is_compatible(priv, HDMI_COMPATIBLE_G12A))
> > priv->hdmi.reg_io_width = 1;
> > else {
> > diff --git a/drivers/video/rockchip/rk3399_hdmi.c
> > b/drivers/video/rockchip/rk3399_hdmi.c
> > index 3041360c6e..b32139a8a6 100644
> > --- a/drivers/video/rockchip/rk3399_hdmi.c
> > +++ b/drivers/video/rockchip/rk3399_hdmi.c
> > @@ -64,8 +64,14 @@ static const struct dm_display_ops rk3399_hdmi_ops = {
> > .enable = rk3399_hdmi_enable,
> > };
> >
> > +static const struct dw_hdmi_plat_data rk3399_hdmi_drv_data = {
> > +};
> > +
> > static const struct udevice_id rk3399_hdmi_ids[] = {
> > - { .compatible = "rockchip,rk3399-dw-hdmi" },
> > + {
> > + .compatible = "rockchip,rk3399-dw-hdmi",
> > + .data = (ulong)_hdmi_drv_data
> > + },
> > { }
> > };
> >
> > diff --git a/drivers/video/rockchip/rk_hdmi.c
> > b/drivers/video/rockchip/rk_hdmi.c
> > index b75a174489..e34f532cd6 100644
> > --- a/drivers/video/rockchip/rk_hdmi.c
> > +++ b/drivers/video/rockchip/rk_hdmi.c
> > @@ -83,6 +83,7 @@ int rk_hdmi_of_to_plat(struct udevice *dev)
> > struct rk_hdmi_priv *priv = dev_get_priv(dev);
> > struct dw_hdmi *hdmi = >hdmi;
> >
> > + hdmi->data = (const struct dw_hdmi_plat_data
> > *)dev_get_driver_data(dev);
> >
Re: [PATCH v9 2/2] arm64: boot: Support Flat Image Tree
On Sun, Dec 10, 2023 at 1:31 AM Geert Uytterhoeven wrote:
>
> Hi Laurent,
>
> On Sat, Dec 9, 2023 at 4:29 PM Laurent Pinchart
> wrote:
> > On Sat, Dec 09, 2023 at 10:13:59PM +0900, Chen-Yu Tsai wrote:
> > > On Thu, Dec 7, 2023 at 11:38 PM Laurent Pinchart
> > > wrote:
> > > > On Thu, Dec 07, 2023 at 10:27:23PM +0800, Chen-Yu Tsai wrote:
> > > > > On Sun, Dec 03, 2023 at 05:34:01PM +0200, Laurent Pinchart wrote:
> > > > > > On Fri, Dec 01, 2023 at 08:54:42PM -0700, Simon Glass wrote:
> > > > > > > Add a script which produces a Flat Image Tree (FIT), a single file
> > > > > > > containing the built kernel and associated devicetree files.
> > > > > > > Compression defaults to gzip which gives a good balance of size
> > > > > > > and
> > > > > > > performance.
> > > > > > >
> > > > > > > The files compress from about 86MB to 24MB using this approach.
> > > > > > >
> > > > > > > The FIT can be used by bootloaders which support it, such as
> > > > > > > U-Boot
> > > > > > > and Linuxboot. It permits automatic selection of the correct
> > > > > > > devicetree, matching the compatible string of the running board
> > > > > > > with
> > > > > > > the closest compatible string in the FIT. There is no need for
> > > > > > > filenames or other workarounds.
> > > > > > >
> > > > > > > Add a 'make image.fit' build target for arm64, as well. Use
> > > > > > > FIT_COMPRESSION to select a different algorithm.
> > > > > > >
> > > > > > > The FIT can be examined using 'dumpimage -l'.
> > > > > > >
> > > > > > > This features requires pylibfdt (use 'pip install libfdt'). It
> > > > > > > also
> > > > > > > requires compression utilities for the algorithm being used.
> > > > > > > Supported
> > > > > > > compression options are the same as the Image.xxx files. For now
> > > > > > > there
> > > > > > > is no way to change the compression other than by editing the
> > > > > > > rule for
> > > > > > > $(obj)/image.fit
> > > > > > >
> > > > > > > While FIT supports a ramdisk / initrd, no attempt is made to
> > > > > > > support
> > > > > > > this here, since it must be built separately from the Linux build.
> > > > > >
> > > > > > FIT images are very useful, so I think this is a very welcome
> > > > > > addition
> > > > > > to the kernel build system. It can get tricky though: given the
> > > > > > versatile nature of FIT images, there can't be any
> > > > > > one-size-fits-them-all solution to build them, and striking the
> > > > > > right
> > > > > > balance between what makes sense for the kernel and the features
> > > > > > that
> > > > > > users may request will probably lead to bikeshedding. As we all love
> > > > > > bikeshedding, I thought I would start selfishly, with a personal use
> > > > > > case :-) This isn't a yak-shaving request though, I don't see any
> > > > > > reason
> > > > > > to delay merging this series.
> > > > > >
> > > > > > Have you envisioned building FIT images with a subset of DTBs, or
> > > > > > adding
> > > > > > DTBOs ? Both would be fairly trivial extensions to this script by
> > > > > > extending the supported command line arguments. It would perhaps be
> > > > > > more
> > > > > > difficult to integrate in the kernel build system though. This
> > > > > > leads me
> > > > > > to a second question: would you consider merging extensions to this
> > > > > > script if they are not used by the kernel build system, but meant
> > > > > > for
> > > > > > users who manually invoke the script ? More generally, is the script
> > > > >
> > > > > We'd also be interested in some customization, though in a different
> > > > > way.
> > > > > We imagine having a rule file that says X compatible string should map
> > > > > to A base DTB, plus B and C DTBO for the configuration section. The
> > > > > base
> > > > > DTB would carry all common elements of some device, while the DTBOs
> > > > > carry all the possible second source components, like different
> > > > > display
> > > > > panels or MIPI cameras for instance. This could drastically reduce the
> > > > > size of FIT images in ChromeOS by deduplicating all the common stuff.
> > > >
> > > > Do you envision the "mapping" compatible string mapping to a config
> > > > section in the FIT image, that would bundle the base DTB and the DTBOs ?
> > >
> > > That's exactly the idea. The mapping compatible string could be untied
> > > from the base board's compatible string if needed (which we probably do).
> > >
> > > So something like:
> > >
> > > config {
> > > config-1 {
> > > compatible = "google,krane-sku0";
> > > fdt = "krane-baseboard", "krane-sku0-overlay";
> > > };
> > > };
> > >
> > > With "krane-sku0-overlay" being an overlay that holds the differences
> > > between the SKUs, in this case the display panel and MIPI camera (not
> > > upstreamed) that applies to SKU0 in particular.
> >
> > The kernel DT makefiles already contain information on what overlays to
> > apply to what base boards, in order to test the
[patch v1] arm add initial support for the Phytium Pe2201 Board board：phytium：add pe2201 folder，It initializes pcie, ddr and other aspects of phytium pe2201 board dts：add phytium_pe2201.dts，it is dev
From: TracyMg_Li
Signed-off-by: TracyMg_Li
---
arch/arm/Kconfig | 8 ++
arch/arm/dts/Makefile| 1 +
arch/arm/dts/phytium-pe2201.dts | 43 +++
board/phytium/pe2201/Kconfig | 12 ++
board/phytium/pe2201/MAINTAINERS | 8 ++
board/phytium/pe2201/Makefile| 12 ++
board/phytium/pe2201/cpu.h | 64 +++
board/phytium/pe2201/ddr.c | 190 +++
board/phytium/pe2201/pcie.c | 60 ++
board/phytium/pe2201/pe2201.c| 92 +++
board/phytium/pe2201/pll.c | 75
board/phytium/pe2201/sec.c | 37 ++
configs/pe2201_defconfig | 40 +++
include/configs/pe2201.h | 32 ++
14 files changed, 674 insertions(+)
create mode 100644 arch/arm/dts/phytium-pe2201.dts
create mode 100644 board/phytium/pe2201/Kconfig
create mode 100644 board/phytium/pe2201/MAINTAINERS
create mode 100644 board/phytium/pe2201/Makefile
create mode 100644 board/phytium/pe2201/cpu.h
create mode 100644 board/phytium/pe2201/ddr.c
create mode 100644 board/phytium/pe2201/pcie.c
create mode 100644 board/phytium/pe2201/pe2201.c
create mode 100644 board/phytium/pe2201/pll.c
create mode 100644 board/phytium/pe2201/sec.c
create mode 100644 configs/pe2201_defconfig
create mode 100644 include/configs/pe2201.h
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index d812685c98..8a8f8805a0 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -2060,6 +2060,13 @@ config TARGET_POMELO
Support for pomelo platform.
It has 8GB Sdram, uart and pcie.
+config TARGET_PE2201
+bool "Support Phytium PE2201 Platform"
+select ARM64
+help
+ Support for pe2201 platform.
+ It has 2GB Sdram, uart and pcie.
+
config TARGET_PRESIDIO_ASIC
bool "Support Cortina Presidio ASIC Platform"
select ARM64
@@ -2336,6 +2343,7 @@ source "board/variscite/dart_6ul/Kconfig"
source "board/vscom/baltos/Kconfig"
source "board/phytium/durian/Kconfig"
source "board/phytium/pomelo/Kconfig"
+source "board/phytium/pe2201/Kconfig"
source "board/xen/xenguest_arm64/Kconfig"
source "arch/arm/Kconfig.debug"
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 5fc888680b..ce2fc626b0 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -1469,6 +1469,7 @@ dtb-$(CONFIG_TARGET_TOTAL_COMPUTE) += total_compute.dtb
dtb-$(CONFIG_TARGET_DURIAN) += phytium-durian.dtb
dtb-$(CONFIG_TARGET_POMELO) += phytium-pomelo.dtb
+dtb-$(CONFIG_TARGET_PE2201) += phytium-pe2201.dtb
dtb-$(CONFIG_TARGET_PRESIDIO_ASIC) += ca-presidio-engboard.dtb
diff --git a/arch/arm/dts/phytium-pe2201.dts b/arch/arm/dts/phytium-pe2201.dts
new file mode 100644
index 00..e661e80f06
--- /dev/null
+++ b/arch/arm/dts/phytium-pe2201.dts
@@ -0,0 +1,43 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * dts file for Phytium pe2201 board
+ * Copyright (C) 2023, Phytium Technology Co., Ltd.
+ * lixinde
+ * weichangzheng
+ */
+/dts-v1/;
+
+/ {
+ model = "Phytium pe2201 Board";
+ compatible = "phytium,pe2201";
+ #address-cells = <2>;
+ #size-cells = <2>;
+
+ aliases {
+ serial0 =
+ };
+
+ uart0: serial@2800c000 {
+ compatible = "arm,pl011", "arm,primecell";
+ reg = <0x0 0x2800c000 0x0 0x1000>;
+ clock = <1>;
+ };
+
+ soc {
+ compatible = "simple-bus";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ pcie@4000 {
+ compatible = "pci-host-ecam-generic";
+ device_type = "pci";
+ #address-cells = <3>;
+ #size-cells = <2>;
+ reg = <0x0 0x4000 0x0 0x1000>;
+ ranges = <0x0100 0x00 0x 0x0 0x5000 0x0
0x00F0>,
+ <0x0200 0x00 0x5800 0x0 0x5800 0x0
0x2800>,
+ <0x4300 0x10 0x 0x10 0x 0x10
0x>;
+ };
+ };
+};
diff --git a/board/phytium/pe2201/Kconfig b/board/phytium/pe2201/Kconfig
new file mode 100644
index 00..f2f222b5d3
--- /dev/null
+++ b/board/phytium/pe2201/Kconfig
@@ -0,0 +1,12 @@
+if TARGET_PE2201
+
+config SYS_BOARD
+ default "pe2201"
+
+config SYS_VENDOR
+ default "phytium"
+
+config SYS_CONFIG_NAME
+ default "pe2201"
+
+endif
diff --git a/board/phytium/pe2201/MAINTAINERS b/board/phytium/pe2201/MAINTAINERS
new file mode 100644
index 00..f583d631d2
--- /dev/null
+++ b/board/phytium/pe2201/MAINTAINERS
@@ -0,0 +1,8 @@
+PE2201 BOARD
+M: lixinde
+M: weichangzheng
+S: Maintained
+F: board/phytium/pe2201/*
+F: include/configs/pe2201.h
+F: configs/pe2201_defconfig
+F: arch/arm/dts/phytium-pe2201.dts
diff --git
RE: [EXT] Re: [PATCH v2 1/3] drivers: rtc: add pcf2131 rtc driver
Okay，Thanks your kind reminder! BR Joy Zou > -Original Message- > From: Fabio Estevam > Sent: 2023年12月14日 4:07 > To: Joy Zou > Cc: Fabio Estevam ; Chris Packham > ; Peng Fan ; > sap...@gmail.com; Stefano Babic ; Simon Glass > ; U-Boot-Denx ; dl-uboot-imx > ; Ye Li > Subject: [EXT] Re: [PATCH v2 1/3] drivers: rtc: add pcf2131 rtc driver > > Caution: This is an external email. Please take care when clicking links or > opening attachments. When in doubt, report the message using the 'Report > this email' button > > > Hi Joy, > > In Linux, the pcf2131 rtc is handled by the pcf2127 driver. > > I suggest we do the same in U-Boot. Thanks
Re: [PATCH 2/2 v3] smbios: Fallback to the default DT if sysinfo nodes are missing
Hi Tom, On Wed, 13 Dec 2023 at 16:05, Tom Rini wrote: > > On Wed, Dec 13, 2023 at 03:22:25PM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Wed, 13 Dec 2023 at 13:56, Tom Rini wrote: > > > > > > On Wed, Dec 13, 2023 at 01:41:04PM -0700, Simon Glass wrote: > > > > Hi Tom, > > > > > > > > On Wed, 13 Dec 2023 at 13:17, Tom Rini wrote: > > > > > > > > > > On Wed, Dec 13, 2023 at 12:51:33PM -0700, Simon Glass wrote: > > > > > > Hi Ilias, > > > > > > > > > > > > On Thu, 7 Dec 2023 at 02:19, Ilias Apalodimas > > > > > > wrote: > > > > > > > > > > > > > > In order to fill in the SMBIOS tables U-Boot currently relies on a > > > > > > > "u-boot,sysinfo-smbios" compatible node. This is fine for the > > > > > > > boards > > > > > > > that already include such nodes. However with some recent EFI > > > > > > > changes, > > > > > > > the majority of boards can boot up distros, which usually rely on > > > > > > > things like dmidecode etc for their reporting. For boards that > > > > > > > lack this special node the SMBIOS output looks like: > > > > > > > > > > > > > > System Information > > > > > > > Manufacturer: Unknown > > > > > > > Product Name: Unknown > > > > > > > Version: Unknown > > > > > > > Serial Number: Unknown > > > > > > > UUID: Not Settable > > > > > > > Wake-up Type: Reserved > > > > > > > SKU Number: Unknown > > > > > > > Family: Unknown > > > > > > > > > > > > > > This looks problematic since most of the info are "Unknown". The > > > > > > > DT spec > > > > > > > specifies standard properties containing relevant information like > > > > > > > 'model' and 'compatible' for which the suggested format is > > > > > > > . Unfortunately the 'model' string found in > > > > > > > DTs is > > > > > > > usually lacking the manufacturer so we can't use it for both > > > > > > > 'Manufacturer' and 'Product Name' SMBIOS entries reliably. > > > > > > > > > > > > > > So let's add a last resort to our current smbios parsing. If > > > > > > > none of > > > > > > > the sysinfo properties are found, scan for those information in > > > > > > > the > > > > > > > root node of the device tree. Use the 'model' to fill the 'Product > > > > > > > Name' and the first value of 'compatible' for the 'Manufacturer', > > > > > > > since > > > > > > > that always contains one. > > > > > > > > > > > > > > pre-patch: > > > > > > > Handle 0x0001, DMI type 1, 27 bytes > > > > > > > System Information > > > > > > > Manufacturer: Unknown > > > > > > > Product Name: Unknown > > > > > > > Version: Unknown > > > > > > > Serial Number: 1bb24ceb > > > > > > > UUID: 30303031-3030-3030-3061-613234636435 > > > > > > > Wake-up Type: Reserved > > > > > > > SKU Number: Unknown > > > > > > > Family: Unknown > > > > > > > [...] > > > > > > > > > > > > > > and post patch: > > > > > > > Handle 0x0001, DMI type 1, 27 bytes > > > > > > > System Information > > > > > > > Manufacturer: raspberrypi > > > > > > > Product Name: Raspberry Pi 4 Model B Rev 1.1 > > > > > > > Version: Unknown > > > > > > > Serial Number: 1bb24ceb > > > > > > > UUID: 30303031-3030-3030-3061-613234636435 > > > > > > > Wake-up Type: Reserved > > > > > > > SKU Number: Unknown > > > > > > > Family: Unknown > > > > > > > [...] > > > > > > > > > > > > > > Signed-off-by: Ilias Apalodimas > > > > > > > --- > > > > > > > Simon, I'll work with tou on the refactoring you wanted and > > > > > > > remove the EFI requirement of SMBIOS in !x86 platforms. > > > > > > > Since this code has no tests, I'll add some once the refactoring > > > > > > > is done > > > > > > > > > > > > > > Changes since v2: > > > > > > > - Spelling mistakes > > > > > > > - rebase on top of patch #1 changes > > > > > > > Changes since v1: > > > > > > > - Tokenize the DT node entry and use the appropriate value > > > > > > > instead of > > > > > > > the entire string > > > > > > > - Removed Peters tested/reviewed-by tags due to the above > > > > > > > lib/smbios.c | 94 > > > > > > > +--- > > > > > > > 1 file changed, 89 insertions(+), 5 deletions(-) > > > > > > > > > > > > Can we add a Kconfig to enable this? > > > > > > > > > > Why? This is the default option that we want moving forward in order > > > > > to > > > > > get the fields populated. > > > > > > > > The model name is fine. The manufacturer is in lower case (using the > > > > DT vendor name), so not in the right format. > > > > > > > > It only populates two fields, so far as I can tell. > > > > > > Maybe we need to turn this discussion on its head slightly. What do you > > > want to do to get SMBIOS fields to be widely populated with > > > generally-correct information? What we have been doing has seen very > > > little adoption so we need something else. > > > > Well, who or what is
[GIT PULL] u-boot-riscv/master
Hi Tom, The following changes since commit 20d0464300c25db673cfb5e4539aa3767606d151: Merge tag 'u-boot-imx-20231212' of https://gitlab.denx.de/u-boot/custodians/u-boot-imx (2023-12-12 16:33:57 -0500) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-riscv.git for you to fetch changes up to 8c785ddb7ae8d675faf558c81a29938cb0ec2b35: riscv: sifive: unmatched: migrate to text environment (2023-12-13 16:19:43 +0800) CI result shows no issue: https://source.denx.de/u-boot/custodians/u-boot-riscv/-/pipelines/18889 - VisionFive2: Enable CONFIG_SYSRESET - StarFive: Modify starfive timer driver - AMD/Xilinx: Add MicroBlaze V support - Unmatched: Migrate to text environment Jaehoon Chung (2): riscv: dts: jh7110: Add a gpio-restart node configs: visionfive2: Enable CONFIG_SYSRESET config Kuan Lim Lee (1): timer: starfive: Add Starfive timer support Michal Simek (1): riscv: Add support for AMD/Xilinx MicroBlaze V Yong-Xuan Wang (1): riscv: sifive: unmatched: migrate to text environment arch/riscv/Kconfig | 4 + arch/riscv/dts/Makefile | 2 + arch/riscv/dts/jh7110-starfive-visionfive-2.dtsi | 5 ++ arch/riscv/dts/xilinx-mbv32.dts | 106 +++ board/sifive/unmatched/unmatched.env | 19 board/xilinx/Kconfig | 3 +- board/xilinx/common/board.c | 5 ++ board/xilinx/mbv/Kconfig | 28 ++ board/xilinx/mbv/MAINTAINERS | 7 ++ board/xilinx/mbv/Makefile| 5 ++ board/xilinx/mbv/board.c | 11 +++ configs/sifive_unmatched_defconfig | 2 +- configs/starfive_visionfive2_defconfig | 1 + configs/xilinx_mbv32_defconfig | 30 +++ configs/xilinx_mbv32_smode_defconfig | 32 +++ drivers/timer/starfive-timer.c | 16 ++-- include/configs/sifive-unmatched.h | 37 include/configs/xilinx_mbv.h | 6 ++ 18 files changed, 273 insertions(+), 46 deletions(-) create mode 100644 arch/riscv/dts/xilinx-mbv32.dts create mode 100644 board/sifive/unmatched/unmatched.env create mode 100644 board/xilinx/mbv/Kconfig create mode 100644 board/xilinx/mbv/MAINTAINERS create mode 100644 board/xilinx/mbv/Makefile create mode 100644 board/xilinx/mbv/board.c create mode 100644 configs/xilinx_mbv32_defconfig create mode 100644 configs/xilinx_mbv32_smode_defconfig create mode 100644 include/configs/xilinx_mbv.h Best regards, Leo
Re: [PATCH v6 03/17] arm: mach-k3: Add basic support for J784S4 SoC definition
On December 7, 2023 thus sayeth Neha Malcom Francis:
> Hi Apurva,
>
> On 06/12/23 18:07, Apurva Nandan wrote:
> > Add J784S4 initialization files for initial SPL boot.
> >
> > Signed-off-by: Hari Nagalla
> > [ add firewall configurations and change the R5 MCU scratchpad ]
> > Signed-off-by: Manorit Chawdhry
> > Signed-off-by: Dasnavis Sabiya
> > Signed-off-by: Apurva Nandan
> > ---
> > arch/arm/mach-k3/Kconfig | 16 +-
> > arch/arm/mach-k3/Makefile | 2 +
> > arch/arm/mach-k3/arm64-mmu.c | 52 +++
> > arch/arm/mach-k3/include/mach/hardware.h | 4 +
> > .../mach-k3/include/mach/j784s4_hardware.h| 60
> > arch/arm/mach-k3/include/mach/j784s4_spl.h| 47 +++
> > arch/arm/mach-k3/include/mach/spl.h | 4 +
> > arch/arm/mach-k3/j784s4_fdt.c | 15 +
> > arch/arm/mach-k3/j784s4_init.c| 335 ++
> > arch/arm/mach-k3/r5/Makefile | 1 +
> > 10 files changed, 529 insertions(+), 7 deletions(-)
> > create mode 100644 arch/arm/mach-k3/include/mach/j784s4_hardware.h
> > create mode 100644 arch/arm/mach-k3/include/mach/j784s4_spl.h
> > create mode 100644 arch/arm/mach-k3/j784s4_fdt.c
> > create mode 100644 arch/arm/mach-k3/j784s4_init.c
> >
...
> > +
> > +void k3_mmc_stop_clock(void)
> > +{
> > + if (IS_ENABLED(CONFIG_K3_LOAD_SYSFW)) {
> > + if (spl_boot_device() == BOOT_DEVICE_MMC1) {
> > + struct mmc *mmc = find_mmc_device(0);
> > +
> > + if (!mmc)
> > + return;
> > +
> > + mmc->saved_clock = mmc->clock;
> > + mmc_set_clock(mmc, 0, true);
> > + }
> > + }
> > +}
> > +
> > +void k3_mmc_restart_clock(void)
> > +{
> > + if (IS_ENABLED(CONFIG_K3_LOAD_SYSFW)) {
> > + if (spl_boot_device() == BOOT_DEVICE_MMC1) {
> > + struct mmc *mmc = find_mmc_device(0);
> > +
> > + if (!mmc)
> > + return;
> > +
> > + mmc_set_clock(mmc, mmc->saved_clock, false);
> > + }
> > + }
> > +}
...
> > +void board_init_f(ulong dummy)
> > +{
> > + struct udevice *dev;
> > + int ret, ctr = 1;
> > +
> > + /*
> > +* Cannot delay this further as there is a chance that
> > +* K3_BOOT_PARAM_TABLE_INDEX can be over written by SPL MALLOC section.
> > +*/
> > + store_boot_info_from_rom();
> > +
> > + /* Make all control module registers accessible */
> > + ctrl_mmr_unlock();
> > +
> > + if (IS_ENABLED(CONFIG_CPU_V7R)) {
> > + disable_linefill_optimization();
> > + setup_k3_mpu_regions();
> > + }
> > +
> > + /* Init DM early */
> > + ret = spl_early_init();
> > +
> > + /* Prepare console output */
> > + preloader_console_init();
> > +
> > + if (IS_ENABLED(CONFIG_K3_LOAD_SYSFW)) {
> > + /*
> > +* Process pinctrl for the serial0 a.k.a. WKUP_UART0 module and
> > continue
> > +* regardless of the result of pinctrl. Do this without probing
> > the
> > +* device, but instead by searching the device that would
> > request the
> > +* given sequence number if probed. The UART will be used by
> > the system
> > +* firmware (SYSFW) image for various purposes and SYSFW
> > depends on us
> > +* to initialize its pin settings.
> > +*/
> > + ret = uclass_find_device_by_seq(UCLASS_SERIAL, 0, );
> > + if (!ret)
> > + pinctrl_select_state(dev, "default");
> > +
> > + /*
> > +* Load, start up, and configure system controller firmware.
> > Provide
> > +* the U-Boot console init function to the SYSFW post-PM
> > configuration
> > +* callback hook, effectively switching on (or over) the console
> > +* output.
> > +*/
> > + k3_sysfw_loader(is_rom_loaded_sysfw(),
> > + k3_mmc_stop_clock, k3_mmc_restart_clock);
> > +
>
> All these K3 specific code, should we push it out to a separate function
> like J721S2? (commit fca27ee8b993bd1a5752fc2156b1e86358fa8041 "arch:
> mach-k3: Update board specific API name to K3 generic API name")
>
> Else we can take this up in a separate clean up series.
>
Do we still have a need to support the split binary (sysfw.itb) boot
anymore now that binman is building tiboot3 for us? I don't think we
will ever be in a situation where ROM hasn't loaded the firmware for us.
> > +#ifdef CONFIG_SPL_OF_LIST
> > + if (IS_ENABLED(CONFIG_TI_I2C_BOARD_DETECT))
> > + do_board_detect();
> > +#endif
> > +
Should we tie CONFIG_SPL_OF_LIST and CONFIG_TI_I2C_BOARD_DETECT together
in Kconfig to avoid this?
> > + if (IS_ENABLED(CONFIG_SPL_CLK_K3)) {
> > + /*
> > +* Force probe of clk_k3
How to boot Fedora 39
Hi, I tried the instructions here with a rpi4: https://docs.fedoraproject.org/en-US/quick-docs/raspberry-pi/#_installing_fedora_on_a_raspberry_pi_for_linux_users xzcat Fedora-Workstation-39-1.5.aarch64.raw.xz | sudo dd status=progress bs=4M of=/dev/sdb 16843096064 bytes (17 GB, 16 GiB) copied, 151 s, 112 MB/s 0+2051288 records in 0+2051288 records out 17179869184 bytes (17 GB, 16 GiB) copied, 152.023 s, 113 MB/s Then booted: U-Boot 2023.07 (Oct 25 2023 - 00:00:00 +) DRAM: 992 MiB (effective 3.9 GiB) RPI 4 Model B (0xc03111) Core: 211 devices, 17 uclasses, devicetree: board MMC: mmcnr@7e30: 1, mmc@7e34: 0 Loading Environment from FAT... Unable to read "uboot.env" from mmc0:1... In:serial Out: vidconsole Err: vidconsole Net: eth0: ethernet@7d58 PCIe BRCM: link up, 5.0 Gbps x1 (SSC) starting USB... Bus usb@7e98: USB DWC2 Bus xhci_pci: Register 5000420 NbrPorts 5 Starting the controller USB XHCI 1.00 scanning bus usb@7e98 for devices... 1 USB Device(s) found scanning bus xhci_pci for devices... 3 USB Device(s) found scanning usb for storage devices... 0 Storage Device(s) found Hit any key to stop autoboot: 0 switch to partitions #0, OK mmc0 is current device Scanning mmc 0:1... Found EFI removable media binary efi/boot/bootaa64.efi 966711 bytes read in 105 ms (8.8 MiB/s) Card did not respond to voltage select! : -110 No EFI system partition No EFI system partition Failed to persist EFI variables Booting /efi\boot\bootaa64.efi No EFI system partition Failed to persist EFI variables No EFI system partition Failed to persist EFI variables No EFI system partition Failed to persist EFI variables error: ../../grub-core/commands/search.c:315:no such device: 77e306d4-532a-4d8c-b023-5bde5c8689ac. GRUB version 2.06 Minimal BASH-like line editing is supported. For the first word, TAB lists possible command completions. Anywhere else TAB lists possible device or file completions. grub> I looked at the partition list and don't see that GUID, but perhaps U-Boot is missing some config? U-Boot> part list mmc 0 Partition Map for MMC device 0 -- Partition Type: DOS PartStart SectorNum Sectors UUIDType 1 20481228800 7d51a49c-01 06 Boot 2 1230848 2097152 7d51a49c-02 83 3 3328000 302264327d51a49c-03 83 U-Boot> Regards, Simon
Re: [PATCH 1/1] pico-imx7d: add baseboard SD card boot detect
On Tue, Nov 7, 2023 at 7:16 PM wrote: > > From: Benjamin Szőke > > Technexion PICO-IMX7 SoM is supporting USDHC3 (eMMC or micro SD on SoM) > and USDHC1 (SD on carrier board) to use on any carrier board like > PICO-NYMPH. Based on the U-Boot version from Technexion it adds > baseboard SD card boot detect to able to boot from selected USDHC1 > or USDHC3 boot devices. > > - Add baseboard SD card boot detect from Technexion's U-Boot. > - Implement board_mmc_get_env_dev() for pico-imx7d SoM. > - Implement mmc_map_to_kernel_blk() for pico-imx7d SoM. > - Add board_late_mmc_env_init() to use from common freescale > functions to able to provide "mmcdev" and "mmcroot" variables > in environment of U-boot. > - Add "mmc1" alias to use for usdhc1 in imx7d-pico-pi-u-boot.dtsi. > - In default environment set "mmcautodetect=yes" to use boot detection. > > Signed-off-by: Benjamin Szőke Adjusted commit log, removed (void)(devno), and applied itto u-boot-imx next, thanks.
Re: [PATCH] imx: imx-hab: Select SPL_DRIVERS_MISC in the SPL case
On Mon, Dec 11, 2023 at 10:46 AM Fabio Estevam wrote:
>
> From: Fabio Estevam
>
> Selecting CONFIG_IMX_HAB=y on a SPL target, such as apalis_imx6_defconfig,
> for example, leads to the following build error:
>
> /usr/bin/arm-linux-gnueabihf-ld.bfd: arch/arm/mach-imx/hab.o: in function
> `imx_hab_is_enabled':
> arch/arm/mach-imx/hab.c:879: undefined reference to `fuse_read'
>
> fuse_read() comes from SPL_MXC_OCOTP, which depends on SPL_DRIVERS_MISC,
> since commit 251a3053b1e6 ("misc: imx: remove DM dependency for ocotp
> driver in SPL").
>
> Select SPL_DRIVERS_MISC in the SPL case to fix this build issue.
>
> Reported-by: Lisandro Pérez Meyer
> Signed-off-by: Fabio Estevam
Applied to u-boot-imx next, thanks.
Re: [PATCH] tools: mxsboot: pre-fill buffer with 0xff, not 0
On Sun, Dec 3, 2023 at 9:24 PM Alessandro Rubini wrote: > > The tool works for me, with imx28 and NAND memory, but the resulting > blocks are reported as bad, both by u-boot and the kernel. > > This makes it impossible to erase from Linux (for an upgrade without > console access, for example -- u-boot can "nand scrub" but linux can't). > > pre-filling with 0xff creates a proper boot loader image, but no > bad-block marker is there when written to flash. > > Signed-off-by: Alessandro Rubini Applied to u-boot-imx next, thanks
Re: [PATCH v2] arm: mxs: Clear CPSR V bit to activate low vectors
On Wed, Oct 18, 2023 at 3:52 PM Marek Vasut wrote: > > The MXS starts with CPSR V bit set, which makes the CPU jump to high vectors > in case of an exception. Those high vectors are located at 0x, which > is where the BootROM exception table is located as well. U-Boot should handle > exceptions on its own using its own exception handling code, which is located > at 0x0, i.e. at low vectors. Clear the CPSR V bit, so that the CPU would jump > to low vectors on exception instead, and therefore run the U-Boot exception > handling code. > > Signed-off-by: Marek Vasut Applied to u-boot-imx next, thanks.
Re: [PATCH] efi_loader: eliminate efi_disk_obj structure
Hi Heinrich,
On Wed, 13 Dec 2023 at 23:22, Heinrich Schuchardt wrote:
>
> On 13.12.23 09:57, Masahisa Kojima wrote:
> > Current code uses struct efi_disk_obj to keep information
> > about block devices and partitions. As the efi handle already
> > has a field with the udevice, we should eliminate
> > struct efi_disk_obj and use an pointer to struct efi_object
> > for the handle.
> >
> > efi_link_dev() call is moved inside of efi_disk_add_dev() function
> > to simplify the cleanup process in case of error.
>
> I agree that using struct efi_disk_obj as a container for protocols of a
> block IO device is a bit messy.
>
> But we should keep looking up the handle easy. Currently we use
>
> diskobj = container_of(this, struct efi_disk_obj, ops);
>
> Instead we could append a private field with the handle to the
> EFI_BLOCK_IO_PROTOCOL structure.
>
> >
> > Signed-off-by: Masahisa Kojima
> > ---
> > lib/efi_loader/efi_disk.c | 209 +-
> > 1 file changed, 116 insertions(+), 93 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> > index f0d76113b0..cfb7ace551 100644
> > --- a/lib/efi_loader/efi_disk.c
> > +++ b/lib/efi_loader/efi_disk.c
> > @@ -27,27 +27,24 @@ struct efi_system_partition efi_system_partition = {
> > const efi_guid_t efi_block_io_guid = EFI_BLOCK_IO_PROTOCOL_GUID;
> > const efi_guid_t efi_system_partition_guid = PARTITION_SYSTEM_GUID;
> >
> > -/**
> > - * struct efi_disk_obj - EFI disk object
> > - *
> > - * @header: EFI object header
> > - * @ops: EFI disk I/O protocol interface
> > - * @dev_index: device index of block device
> > - * @media: block I/O media information
> > - * @dp: device path to the block device
> > - * @part:partition
> > - * @volume: simple file system protocol of the partition
> > - * @dev: associated DM device
> > - */
> > -struct efi_disk_obj {
> > - struct efi_object header;
> > - struct efi_block_io ops;
> > - int dev_index;
> > - struct efi_block_io_media media;
> > - struct efi_device_path *dp;
> > - unsigned int part;
> > - struct efi_simple_file_system_protocol *volume;
> > -};
> > +static efi_handle_t efi_blkio_find_obj(struct efi_block_io *blkio)
> > +{
> > + efi_handle_t handle;
> > +
> > + list_for_each_entry(handle, _obj_list, link) {
> > + efi_status_t ret;
> > + struct efi_handler *handler;
> > +
> > + ret = efi_search_protocol(handle, _block_io_guid,
> > );
> > + if (ret != EFI_SUCCESS)
> > + continue;
> > +
> > + if (blkio == handler->protocol_interface)
> > + return handle;
> > + }
>
> Depending on the number of handles and pointers this will take a
> considerable time. A private field for the handle appended to struct
> efi_block_io would allow a fast lookup.
>
> EDK II does the same. See the definition of RAM_DISK_PRIVATE_FROM_BLKIO
> which uses macro BASE_CR() to find the private fields.
This patch tries to address this issue[0].
If I understand correctly, two options are suggested here.
1) a private field for the handle appended to struct efi_block_io
2) keep the private struct something like current struct
efi_disk_obj, same as EDK II does
struct efi_block_io is defined in UEFI spec, so probably option#2 is preferable
and it is almost the same as the current implementation.
I think we can proceed with the minor cleanup of struct efi_disk_obj
as Akashi-san suggested.
[0] https://source.denx.de/u-boot/custodians/u-boot-efi/-/issues/9
Thanks,
Masahisa Kojima
>
> Best regards
>
> Heinrich
>
> > +
> > + return NULL;
> > +}
> >
> > /**
> >* efi_disk_reset() - reset block device
> > @@ -107,13 +104,16 @@ static efi_status_t efi_disk_rw_blocks(struct
> > efi_block_io *this,
> > u32 media_id, u64 lba, unsigned long buffer_size,
> > void *buffer, enum efi_disk_direction direction)
> > {
> > - struct efi_disk_obj *diskobj;
> > + efi_handle_t handle;
> > int blksz;
> > int blocks;
> > unsigned long n;
> >
> > - diskobj = container_of(this, struct efi_disk_obj, ops);
> > - blksz = diskobj->media.block_size;
> > + handle = efi_blkio_find_obj(this);
> > + if (!handle)
> > + return EFI_INVALID_PARAMETER;
> > +
> > + blksz = this->media->block_size;
> > blocks = buffer_size / blksz;
> >
> > EFI_PRINT("blocks=%x lba=%llx blksz=%x dir=%d\n",
> > @@ -124,18 +124,16 @@ static efi_status_t efi_disk_rw_blocks(struct
> > efi_block_io *this,
> > return EFI_BAD_BUFFER_SIZE;
> >
> > if (CONFIG_IS_ENABLED(PARTITIONS) &&
> > - device_get_uclass_id(diskobj->header.dev) == UCLASS_PARTITION) {
> > + device_get_uclass_id(handle->dev) == UCLASS_PARTITION) {
> > if (direction == EFI_DISK_READ)
> > - n =
Re: [PATCH] efi_loader: eliminate efi_disk_obj structure
Hi Kojima-san,
On Wed, Dec 13, 2023 at 05:57:37PM +0900, Masahisa Kojima wrote:
> Current code uses struct efi_disk_obj to keep information
> about block devices and partitions. As the efi handle already
> has a field with the udevice, we should eliminate
> struct efi_disk_obj and use an pointer to struct efi_object
> for the handle.
I don't still understand what is an advantage of your approach.
> efi_link_dev() call is moved inside of efi_disk_add_dev() function
> to simplify the cleanup process in case of error.
>
> Signed-off-by: Masahisa Kojima
> ---
> lib/efi_loader/efi_disk.c | 209 +-
> 1 file changed, 116 insertions(+), 93 deletions(-)
>
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index f0d76113b0..cfb7ace551 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -27,27 +27,24 @@ struct efi_system_partition efi_system_partition = {
> const efi_guid_t efi_block_io_guid = EFI_BLOCK_IO_PROTOCOL_GUID;
> const efi_guid_t efi_system_partition_guid = PARTITION_SYSTEM_GUID;
>
> -/**
> - * struct efi_disk_obj - EFI disk object
> - *
> - * @header: EFI object header
> - * @ops: EFI disk I/O protocol interface
> - * @dev_index: device index of block device
> - * @media: block I/O media information
> - * @dp: device path to the block device
> - * @part:partition
> - * @volume: simple file system protocol of the partition
> - * @dev: associated DM device
> - */
> -struct efi_disk_obj {
> - struct efi_object header;
This is a handy way of making it ease to dereference from an internal
structure to an external reference as Heinrich mentioned.
> - struct efi_block_io ops;
Along with "header" and "media", this allows us to congregate
a couple of "malloc" calls, instead of your change, into one.
> - int dev_index;
> - struct efi_device_path *dp;
> - unsigned int part;
> - struct efi_simple_file_system_protocol *volume;
If we carefully look into the base code, we will no longer have to
have those fields in this structure because they are mostly used in
a single internal function (if I'm correct).
So we can simply replace them with local variables (with some extra
changes).
-Takahiro Akashi
> -};
> +static efi_handle_t efi_blkio_find_obj(struct efi_block_io *blkio)
> +{
> + efi_handle_t handle;
> +
> + list_for_each_entry(handle, _obj_list, link) {
> + efi_status_t ret;
> + struct efi_handler *handler;
> +
> + ret = efi_search_protocol(handle, _block_io_guid, );
> + if (ret != EFI_SUCCESS)
> + continue;
> +
> + if (blkio == handler->protocol_interface)
> + return handle;
> + }
> +
> + return NULL;
> +}
>
> /**
> * efi_disk_reset() - reset block device
> @@ -107,13 +104,16 @@ static efi_status_t efi_disk_rw_blocks(struct
> efi_block_io *this,
> u32 media_id, u64 lba, unsigned long buffer_size,
> void *buffer, enum efi_disk_direction direction)
> {
> - struct efi_disk_obj *diskobj;
> + efi_handle_t handle;
> int blksz;
> int blocks;
> unsigned long n;
>
> - diskobj = container_of(this, struct efi_disk_obj, ops);
> - blksz = diskobj->media.block_size;
> + handle = efi_blkio_find_obj(this);
> + if (!handle)
> + return EFI_INVALID_PARAMETER;
> +
> + blksz = this->media->block_size;
> blocks = buffer_size / blksz;
>
> EFI_PRINT("blocks=%x lba=%llx blksz=%x dir=%d\n",
> @@ -124,18 +124,16 @@ static efi_status_t efi_disk_rw_blocks(struct
> efi_block_io *this,
> return EFI_BAD_BUFFER_SIZE;
>
> if (CONFIG_IS_ENABLED(PARTITIONS) &&
> - device_get_uclass_id(diskobj->header.dev) == UCLASS_PARTITION) {
> + device_get_uclass_id(handle->dev) == UCLASS_PARTITION) {
> if (direction == EFI_DISK_READ)
> - n = disk_blk_read(diskobj->header.dev, lba, blocks,
> - buffer);
> + n = disk_blk_read(handle->dev, lba, blocks, buffer);
> else
> - n = disk_blk_write(diskobj->header.dev, lba, blocks,
> -buffer);
> + n = disk_blk_write(handle->dev, lba, blocks, buffer);
> } else {
> /* dev is a block device (UCLASS_BLK) */
> struct blk_desc *desc;
>
> - desc = dev_get_uclass_plat(diskobj->header.dev);
> + desc = dev_get_uclass_plat(handle->dev);
> if (direction == EFI_DISK_READ)
> n = blk_dread(desc, lba, blocks, buffer);
> else
> @@ -388,6 +386,7 @@ static int efi_fs_exists(struct blk_desc *desc, int part)
> * @part:partition
> * @disk:pointer to receive the created handle
> * @agent_handle:
Re: Please pull u-boot-dm/next
On Wed, Dec 13, 2023 at 01:50:57PM -0700, Simon Glass wrote: > Hi Tom, > > This is for the -next branch > > https://source.denx.de/u-boot/custodians/u-boot-dm/-/commit/39e7e039b02e1cf8b04ea668e2bfe2cd2ef0e14d > > https://dev.azure.com/simon0972/u-boot/_build/results?buildId=56=results > > > The following changes since commit 9565771076c2d4b0193f1741b3990695ac33c1f3: > > Merge patch series "bootm: Refactoring to reduce reliance on CMDLINE > (part A)" (2023-12-13 11:51:53 -0500) > > are available in the Git repository at: > > git://git.denx.de/u-boot-dm.git tags/dm-next-13dec23 > > for you to fetch changes up to 39e7e039b02e1cf8b04ea668e2bfe2cd2ef0e14d: > > test: vboot: Using variable 'old_dtb' before assignment (2023-12-13 > 13:20:20 -0700) > Applied to u-boot/next, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH 2/2 v3] smbios: Fallback to the default DT if sysinfo nodes are missing
> Date: Wed, 13 Dec 2023 15:56:34 -0500 > From: Tom Rini > > On Wed, Dec 13, 2023 at 01:41:04PM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Wed, 13 Dec 2023 at 13:17, Tom Rini wrote: > > > > > > On Wed, Dec 13, 2023 at 12:51:33PM -0700, Simon Glass wrote: > > > > Hi Ilias, > > > > > > > > On Thu, 7 Dec 2023 at 02:19, Ilias Apalodimas > > > > wrote: > > > > > > > > > > In order to fill in the SMBIOS tables U-Boot currently relies on a > > > > > "u-boot,sysinfo-smbios" compatible node. This is fine for the boards > > > > > that already include such nodes. However with some recent EFI > > > > > changes, > > > > > the majority of boards can boot up distros, which usually rely on > > > > > things like dmidecode etc for their reporting. For boards that > > > > > lack this special node the SMBIOS output looks like: > > > > > > > > > > System Information > > > > > Manufacturer: Unknown > > > > > Product Name: Unknown > > > > > Version: Unknown > > > > > Serial Number: Unknown > > > > > UUID: Not Settable > > > > > Wake-up Type: Reserved > > > > > SKU Number: Unknown > > > > > Family: Unknown > > > > > > > > > > This looks problematic since most of the info are "Unknown". The DT > > > > > spec > > > > > specifies standard properties containing relevant information like > > > > > 'model' and 'compatible' for which the suggested format is > > > > > . Unfortunately the 'model' string found in DTs is > > > > > usually lacking the manufacturer so we can't use it for both > > > > > 'Manufacturer' and 'Product Name' SMBIOS entries reliably. > > > > > > > > > > So let's add a last resort to our current smbios parsing. If none of > > > > > the sysinfo properties are found, scan for those information in the > > > > > root node of the device tree. Use the 'model' to fill the 'Product > > > > > Name' and the first value of 'compatible' for the 'Manufacturer', > > > > > since > > > > > that always contains one. > > > > > > > > > > pre-patch: > > > > > Handle 0x0001, DMI type 1, 27 bytes > > > > > System Information > > > > > Manufacturer: Unknown > > > > > Product Name: Unknown > > > > > Version: Unknown > > > > > Serial Number: 1bb24ceb > > > > > UUID: 30303031-3030-3030-3061-613234636435 > > > > > Wake-up Type: Reserved > > > > > SKU Number: Unknown > > > > > Family: Unknown > > > > > [...] > > > > > > > > > > and post patch: > > > > > Handle 0x0001, DMI type 1, 27 bytes > > > > > System Information > > > > > Manufacturer: raspberrypi > > > > > Product Name: Raspberry Pi 4 Model B Rev 1.1 > > > > > Version: Unknown > > > > > Serial Number: 1bb24ceb > > > > > UUID: 30303031-3030-3030-3061-613234636435 > > > > > Wake-up Type: Reserved > > > > > SKU Number: Unknown > > > > > Family: Unknown > > > > > [...] > > > > > > > > > > Signed-off-by: Ilias Apalodimas > > > > > --- > > > > > Simon, I'll work with tou on the refactoring you wanted and > > > > > remove the EFI requirement of SMBIOS in !x86 platforms. > > > > > Since this code has no tests, I'll add some once the refactoring > > > > > is done > > > > > > > > > > Changes since v2: > > > > > - Spelling mistakes > > > > > - rebase on top of patch #1 changes > > > > > Changes since v1: > > > > > - Tokenize the DT node entry and use the appropriate value instead of > > > > > the entire string > > > > > - Removed Peters tested/reviewed-by tags due to the above > > > > > lib/smbios.c | 94 > > > > > +--- > > > > > 1 file changed, 89 insertions(+), 5 deletions(-) > > > > > > > > Can we add a Kconfig to enable this? > > > > > > Why? This is the default option that we want moving forward in order to > > > get the fields populated. > > > > The model name is fine. The manufacturer is in lower case (using the > > DT vendor name), so not in the right format. > > > > It only populates two fields, so far as I can tell. > > Maybe we need to turn this discussion on its head slightly. What do you > want to do to get SMBIOS fields to be widely populated with > generally-correct information? What we have been doing has seen very > little adoption so we need something else. One possibility would be to have a script that maps the board compatible to a vendor name using Documentation/devicetree/bindings/vendor-prefixes.yaml That would solve populating the manufacturer field, but not the product_name unfortunately.
Re: [PATCH v6 08/17] board: ti: j784s4: Add board support for J784S4 EVM
On Wed, Dec 06, 2023 at 06:07:44PM +0530, Apurva Nandan wrote: > Add board files for J784S4 EVM. > > Signed-off-by: Hari Nagalla > [ add env and board specific yaml files for binman ] > Signed-off-by: Neha Malcom Francis > [ cleaned up the env files ] > Signed-off-by: Manorit Chawdhry > Signed-off-by: Dasnavis Sabiya > Signed-off-by: Apurva Nandan Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH v2] net: wget: Support non-default HTTP port
On Wed, Dec 13, 2023 at 10:11:13PM +0100, Marek Vasut wrote: > Currently the wget command is hard wired to HTTP port 80. This is > inconvenient, as it is extremely easy to start trivial HTTP server > as an unprivileged user using e.g. python http module to serve the > files, but such a server has to run on one of the higher ports: > " > $ python3 -m http.server -d $(pwd) 8080 > " > > Make it possible to configure HTTP server port the same way it is > possible to configure TFTP server port, using environment variable > 'httpdstp' (similar to 'tftpdstp'). Retain port 80 as the default > fallback port. This way, users can start their own trivial server > and conveniently download whatever files they need into U-Boot. > > Signed-off-by: Marek Vasut Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH v2] test/py: mii: Add tests for mii command
On Tue, Dec 05, 2023 at 05:32:17PM +0530, 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 Reviewed-by: Tom Rini -- Tom signature.asc Description: PGP signature
Re: [PATCH] maintainers: bcmns3: remove maintainer
On Tue, Dec 05, 2023 at 09:12:22AM +, Peter Robinson wrote: > Remove Bharat Gooty as a maintainer as his mail is > bouncing. > > Signed-off-by: Peter Robinson > Cc: Rayagonda Kokatanur Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] maintainers: rk3399: remove maintainer
On Fri, Nov 24, 2023 at 12:04:31PM +, Shantur Rathore wrote: > Remove Akash Gajjar from > MAINTAINERS as email is bouncing. > > Signed-off-by: Shantur Rathore > Reviewed-by: Kever Yang Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] MAINTAINERS: Fix ARCH_APPLE file paths
On Wed, Nov 22, 2023 at 04:21:14PM +, Moritz Fischer wrote:
> Fixes a filepath in MAINTAINERS file that wasn't updated when
> renaming the files to match the new SoC name.
>
> Fixes: a4bd5e4120d6 ('arm: apple: Change SoC name from "m1" into "apple"')
> Signed-off-by: Moritz Fischer
Applied to u-boot/master, thanks!
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH] drivers: misc: Kconfig: Fix SPL_FS_LOADER prompt
On Mon, Nov 20, 2023 at 08:21:51PM +, Alexander Gendin wrote: > Both FS_LOADER and SPL_FS_LOADER have the same menu prompt. > To avoid confusion, make prompt for SPL_FS_LOADER different. > > Signed-off-by: Alexander Gendin > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] lib/Kconfig: Correct typo about SYSINFO_SMBIOS in help message
On Mon, Nov 20, 2023 at 03:17:23PM -0500, Tom Rini wrote: > The correct symbol to enable to have SMBIOS populate fields based on the > device tree is SYSINFO_SMBIOS and not SMBIOS_SYSINFO. > > Signed-off-by: Tom Rini > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH 2/2 v3] smbios: Fallback to the default DT if sysinfo nodes are missing
On Wed, Dec 13, 2023 at 03:22:25PM -0700, Simon Glass wrote: > Hi Tom, > > On Wed, 13 Dec 2023 at 13:56, Tom Rini wrote: > > > > On Wed, Dec 13, 2023 at 01:41:04PM -0700, Simon Glass wrote: > > > Hi Tom, > > > > > > On Wed, 13 Dec 2023 at 13:17, Tom Rini wrote: > > > > > > > > On Wed, Dec 13, 2023 at 12:51:33PM -0700, Simon Glass wrote: > > > > > Hi Ilias, > > > > > > > > > > On Thu, 7 Dec 2023 at 02:19, Ilias Apalodimas > > > > > wrote: > > > > > > > > > > > > In order to fill in the SMBIOS tables U-Boot currently relies on a > > > > > > "u-boot,sysinfo-smbios" compatible node. This is fine for the > > > > > > boards > > > > > > that already include such nodes. However with some recent EFI > > > > > > changes, > > > > > > the majority of boards can boot up distros, which usually rely on > > > > > > things like dmidecode etc for their reporting. For boards that > > > > > > lack this special node the SMBIOS output looks like: > > > > > > > > > > > > System Information > > > > > > Manufacturer: Unknown > > > > > > Product Name: Unknown > > > > > > Version: Unknown > > > > > > Serial Number: Unknown > > > > > > UUID: Not Settable > > > > > > Wake-up Type: Reserved > > > > > > SKU Number: Unknown > > > > > > Family: Unknown > > > > > > > > > > > > This looks problematic since most of the info are "Unknown". The > > > > > > DT spec > > > > > > specifies standard properties containing relevant information like > > > > > > 'model' and 'compatible' for which the suggested format is > > > > > > . Unfortunately the 'model' string found in DTs > > > > > > is > > > > > > usually lacking the manufacturer so we can't use it for both > > > > > > 'Manufacturer' and 'Product Name' SMBIOS entries reliably. > > > > > > > > > > > > So let's add a last resort to our current smbios parsing. If none > > > > > > of > > > > > > the sysinfo properties are found, scan for those information in the > > > > > > root node of the device tree. Use the 'model' to fill the 'Product > > > > > > Name' and the first value of 'compatible' for the 'Manufacturer', > > > > > > since > > > > > > that always contains one. > > > > > > > > > > > > pre-patch: > > > > > > Handle 0x0001, DMI type 1, 27 bytes > > > > > > System Information > > > > > > Manufacturer: Unknown > > > > > > Product Name: Unknown > > > > > > Version: Unknown > > > > > > Serial Number: 1bb24ceb > > > > > > UUID: 30303031-3030-3030-3061-613234636435 > > > > > > Wake-up Type: Reserved > > > > > > SKU Number: Unknown > > > > > > Family: Unknown > > > > > > [...] > > > > > > > > > > > > and post patch: > > > > > > Handle 0x0001, DMI type 1, 27 bytes > > > > > > System Information > > > > > > Manufacturer: raspberrypi > > > > > > Product Name: Raspberry Pi 4 Model B Rev 1.1 > > > > > > Version: Unknown > > > > > > Serial Number: 1bb24ceb > > > > > > UUID: 30303031-3030-3030-3061-613234636435 > > > > > > Wake-up Type: Reserved > > > > > > SKU Number: Unknown > > > > > > Family: Unknown > > > > > > [...] > > > > > > > > > > > > Signed-off-by: Ilias Apalodimas > > > > > > --- > > > > > > Simon, I'll work with tou on the refactoring you wanted and > > > > > > remove the EFI requirement of SMBIOS in !x86 platforms. > > > > > > Since this code has no tests, I'll add some once the refactoring > > > > > > is done > > > > > > > > > > > > Changes since v2: > > > > > > - Spelling mistakes > > > > > > - rebase on top of patch #1 changes > > > > > > Changes since v1: > > > > > > - Tokenize the DT node entry and use the appropriate value instead > > > > > > of > > > > > > the entire string > > > > > > - Removed Peters tested/reviewed-by tags due to the above > > > > > > lib/smbios.c | 94 > > > > > > +--- > > > > > > 1 file changed, 89 insertions(+), 5 deletions(-) > > > > > > > > > > Can we add a Kconfig to enable this? > > > > > > > > Why? This is the default option that we want moving forward in order to > > > > get the fields populated. > > > > > > The model name is fine. The manufacturer is in lower case (using the > > > DT vendor name), so not in the right format. > > > > > > It only populates two fields, so far as I can tell. > > > > Maybe we need to turn this discussion on its head slightly. What do you > > want to do to get SMBIOS fields to be widely populated with > > generally-correct information? What we have been doing has seen very > > little adoption so we need something else. > > Well, who or what is actually using this info? Is the problem just > that it doesn't actually matter, so no one bothers? I'm just not sure. Users are using this information. Peter has explained that Fedora has been carrying Ilias' original version of this patch since it was posted so that bug reports say (something close enough and
Re: [PATCH 1/2 v3] smbios: Simplify reporting of unknown values
On Thu, 14 Dec 2023 at 00:22, Simon Glass wrote:
>
> Hi Ilias,
>
> On Wed, 13 Dec 2023 at 14:37, Ilias Apalodimas
> wrote:
> >
> > Hi Simon,
> >
> > On Wed, 13 Dec 2023 at 21:52, Simon Glass wrote:
> > >
> > > Hi Ilias,
> > >
> > > On Thu, 7 Dec 2023 at 02:19, Ilias Apalodimas
> > > wrote:
> > > >
> > > > If a value is not valid during the DT or SYSINFO parsing, we explicitly
> > > > set that to "Unknown Product" and "Unknown" for the product and
> > > > manufacturer respectively. It's cleaner if we move the checks insisde
> > > > smbios_add_prop_si() and provide an alternative string in case the
> > > > primary is NULL or empty
> > > >
> > > > pre-patch dmidecode
> > > >
> > > > Handle 0x0001, DMI type 1, 27 bytes
> > > > System Information
> > > > Manufacturer: Unknown
> > > > Product Name: Unknown Product
> > > > Version: Not Specified
> > > > Serial Number: Not Specified
> > > > UUID: Not Settable
> > > > Wake-up Type: Reserved
> > > > SKU Number: Not Specified
> > > > Family: Not Specified
> > > >
> > > > [...]
> > > >
> > > > post-patch dmidecode:
> > > >
> > > > Handle 0x0001, DMI type 1, 27 bytes
> > > > System Information
> > > > Manufacturer: Unknown
> > > > Product Name: Unknown
> > > > Version: Unknown
> > > > Serial Number: Unknown
> > > > UUID: Not Settable
> > > > Wake-up Type: Reserved
> > > > SKU Number: Unknown
> > > > Family: Unknown
> > > > [...]
> > > >
> > > > While at it make smbios_add_prop_si() add a string directly if the prop
> > > > node is NULL and replace smbios_add_string() calls with
> > > > smbios_add_prop_si(ctx, NULL, )
> > > >
> > > > Signed-off-by: Ilias Apalodimas
> > > > ---
> > > > Changes since v2:
> > > > - refactor even more code and remove the smbios_add_string calls from
> > > > the
> > > > main code. Instead use smbios_add_prop() foir everything and add a
> > > > default value, in case the parsed one is NULL or emtpy
> > > > Changes since v1:
> > > > - None
> > > >
> > > > lib/smbios.c | 73 +---
> > > > 1 file changed, 35 insertions(+), 38 deletions(-)
> > > >
> > >
> > > I actually think we should just stop here and first write a test for
> > > what we already have. I can take a crack at it if you like?
> >
> > I don't mind. As I said in a previous email I'd rather do it after
> > the refactoring you had in mind. But I don't see why that should stop
> > the current patch from going forward.
>
> OK, well if this series could be simplified, I don't mind either. But
> at present it is complicated and I think it really needs a test.
>
> >
> > >
> > > > diff --git a/lib/smbios.c b/lib/smbios.c
> > > > index d7f4999e8b2a..444aa245a273 100644
> > > > --- a/lib/smbios.c
> > > > +++ b/lib/smbios.c
> > > > @@ -102,9 +102,6 @@ static int smbios_add_string(struct smbios_ctx
> > > > *ctx, const char *str)
> > > > int i = 1;
> > > > char *p = ctx->eos;
> > > >
> > > > - if (!*str)
> > > > - str = "Unknown";
> > > > -
> > > > for (;;) {
> > > > if (!*p) {
> > > > ctx->last_str = p;
> > > > @@ -134,11 +131,18 @@ static int smbios_add_string(struct smbios_ctx
> > > > *ctx, const char *str)
> > > > *
> > > > * @ctx: context for writing the tables
> > > > * @prop: property to write
> > >
> > > which can now be NULL?
> >
> > No, it can't because smbios_add_string can't do that. See commit
> > 00a871d34e2f0a.
> > But instead of checking it here, I moved the code around and
> > smbios_add_string is only called from smbios_add_prop_si instead of
> > calling it on each failure of smbios_add_prop_si(). In that function,
> > we make sure the value isn't NULL, apart from the ->get_str sysinfo
> > calls -- none of these currently returns null though. I can move the
> > checking back to where it was if to shield us again dump 'get_str'
> > implementations, or in the future fix smbios_add_string properly, but
> > that's not the point of this patch.
>
> My point was that you have code to check for !prop and do something:
>
> if (!prop)
> return smbios_add_string(ctx, dval);
>
> Before, we needed prop (at least if OF_CONTROL), but now prop can be
> NULL, so you should update the comment to mention that and explain
> what it means.
Sure
>
> >
> > >
> > > > + * @dval: Default value to use if the string is not found or is
> > > > empty
> > > > * Return: 0 if not found, else SMBIOS string number (1 or more)
> > > > */
> > > > static int smbios_add_prop_si(struct smbios_ctx *ctx, const char *prop,
> > > > - int sysinfo_id)
> > > > + int sysinfo_id, const char *dval)
> > > > {
> > > > + if (!dval || !*dval)
> > > > + dval = "Unknown";
> > >
> > > You shouldn't need this, right? It is already the default
Re: [PATCH 2/2 v3] smbios: Fallback to the default DT if sysinfo nodes are missing
Hi Tom, On Wed, 13 Dec 2023 at 13:56, Tom Rini wrote: > > On Wed, Dec 13, 2023 at 01:41:04PM -0700, Simon Glass wrote: > > Hi Tom, > > > > On Wed, 13 Dec 2023 at 13:17, Tom Rini wrote: > > > > > > On Wed, Dec 13, 2023 at 12:51:33PM -0700, Simon Glass wrote: > > > > Hi Ilias, > > > > > > > > On Thu, 7 Dec 2023 at 02:19, Ilias Apalodimas > > > > wrote: > > > > > > > > > > In order to fill in the SMBIOS tables U-Boot currently relies on a > > > > > "u-boot,sysinfo-smbios" compatible node. This is fine for the boards > > > > > that already include such nodes. However with some recent EFI > > > > > changes, > > > > > the majority of boards can boot up distros, which usually rely on > > > > > things like dmidecode etc for their reporting. For boards that > > > > > lack this special node the SMBIOS output looks like: > > > > > > > > > > System Information > > > > > Manufacturer: Unknown > > > > > Product Name: Unknown > > > > > Version: Unknown > > > > > Serial Number: Unknown > > > > > UUID: Not Settable > > > > > Wake-up Type: Reserved > > > > > SKU Number: Unknown > > > > > Family: Unknown > > > > > > > > > > This looks problematic since most of the info are "Unknown". The DT > > > > > spec > > > > > specifies standard properties containing relevant information like > > > > > 'model' and 'compatible' for which the suggested format is > > > > > . Unfortunately the 'model' string found in DTs is > > > > > usually lacking the manufacturer so we can't use it for both > > > > > 'Manufacturer' and 'Product Name' SMBIOS entries reliably. > > > > > > > > > > So let's add a last resort to our current smbios parsing. If none of > > > > > the sysinfo properties are found, scan for those information in the > > > > > root node of the device tree. Use the 'model' to fill the 'Product > > > > > Name' and the first value of 'compatible' for the 'Manufacturer', > > > > > since > > > > > that always contains one. > > > > > > > > > > pre-patch: > > > > > Handle 0x0001, DMI type 1, 27 bytes > > > > > System Information > > > > > Manufacturer: Unknown > > > > > Product Name: Unknown > > > > > Version: Unknown > > > > > Serial Number: 1bb24ceb > > > > > UUID: 30303031-3030-3030-3061-613234636435 > > > > > Wake-up Type: Reserved > > > > > SKU Number: Unknown > > > > > Family: Unknown > > > > > [...] > > > > > > > > > > and post patch: > > > > > Handle 0x0001, DMI type 1, 27 bytes > > > > > System Information > > > > > Manufacturer: raspberrypi > > > > > Product Name: Raspberry Pi 4 Model B Rev 1.1 > > > > > Version: Unknown > > > > > Serial Number: 1bb24ceb > > > > > UUID: 30303031-3030-3030-3061-613234636435 > > > > > Wake-up Type: Reserved > > > > > SKU Number: Unknown > > > > > Family: Unknown > > > > > [...] > > > > > > > > > > Signed-off-by: Ilias Apalodimas > > > > > --- > > > > > Simon, I'll work with tou on the refactoring you wanted and > > > > > remove the EFI requirement of SMBIOS in !x86 platforms. > > > > > Since this code has no tests, I'll add some once the refactoring > > > > > is done > > > > > > > > > > Changes since v2: > > > > > - Spelling mistakes > > > > > - rebase on top of patch #1 changes > > > > > Changes since v1: > > > > > - Tokenize the DT node entry and use the appropriate value instead of > > > > > the entire string > > > > > - Removed Peters tested/reviewed-by tags due to the above > > > > > lib/smbios.c | 94 > > > > > +--- > > > > > 1 file changed, 89 insertions(+), 5 deletions(-) > > > > > > > > Can we add a Kconfig to enable this? > > > > > > Why? This is the default option that we want moving forward in order to > > > get the fields populated. > > > > The model name is fine. The manufacturer is in lower case (using the > > DT vendor name), so not in the right format. > > > > It only populates two fields, so far as I can tell. > > Maybe we need to turn this discussion on its head slightly. What do you > want to do to get SMBIOS fields to be widely populated with > generally-correct information? What we have been doing has seen very > little adoption so we need something else. Well, who or what is actually using this info? Is the problem just that it doesn't actually matter, so no one bothers? I'm just not sure. The fact that on ARM you cannot pass it to the kernel without EFI might be inhibiting its usefulness, too? Usefulness is the key question for me. Anyway, my suggestion (once we understand why we care about SMBIOS tables) is patches like we just saw from rockpro64, perhaps even with a few more fields filled out. This series seems like a backup for board maintainers that don't care, which is fine, but we should at least be able to disable it. Regards, Simon
Re: [PATCH v2 1/5] cyclic: Add a symbol for SPL
Hi Tom,
On Wed, 13 Dec 2023 at 13:59, Tom Rini wrote:
>
> On Wed, Dec 13, 2023 at 01:51:00PM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Wed, 13 Dec 2023 at 13:42, Tom Rini wrote:
> > >
> > > On Wed, Dec 13, 2023 at 12:50:02PM -0700, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Sat, 9 Dec 2023 at 08:48, Tom Rini wrote:
> > > > >
> > > > > On Sat, Dec 02, 2023 at 08:33:48AM -0700, Simon Glass wrote:
> > > > >
> > > > > > The cyclic subsystem is currently enabled either in all build phases
> > > > > > or none. For tools this should not be enabled, but since
> > > > > > lib/shc256.c
> > > > > > and other files include watchdog.h in the host build, we must make
> > > > > > sure that it is not enabled there.
> > > > >
> > > > > This part is what I see as the Wrong Direction. There's some code we
> > > > > really need to share with our user space tools, in order to not
> > > > > copy/paste the same code. In turn, this code must be as user-space
> > > > > friendly as possible. Maybe even we re-factor things a little more, if
> > > > > needed, so that we _just_ have the library functions in common files,
> > > > > and u-boot or user space only files have the make use of logic. I
> > > > > don't
> > > > > feel bad about tools/ needing:
> > > > > void sha256_csum_wd(const unsigned char *input, unsigned int ilen,
> > > > > unsigned char *output, unsigned int chunk_sz)
> > > > > {
> > > > > sha256_context ctx;
> > > > > sha256_starts();
> > > > > sha256_update(, input, ilen);
> > > > > sha256_finish(, output);
> > > > > }
> > > > >
> > > > > (and so on for other algos) as a duplicate bit of code. Much less so
> > > > > than I do about adding to a direct include list
> > > > > (since
> > > > > we should never be doing that) so that later on we can if
> > > > > (IS_ENABLED(..)) the existing code.
> > > >
> > > > Bear in mind that we have the CONFIG_TOOLS_... options entirely to
> > > > deal with the need for tools to enable features in common code. This
> > > > SHA thing is a very small part of the code, compared to common code in
> > > > boot/ for example.
> > > >
> > > > So is this really a win?
> > >
> > > I don't follow you here, sorry.
> >
> > I mean that we share a lot of code already, code which contains CONFIG
> > options. So does it matter avoiding adding one more?
>
> It's adding to files, we must not do that. And we
> don't need to if we don't have (and that in turn,
> cyclic.h) included outside of USE_HOSTCC :)
$ git grep kconfig.h
.azure-pipelines.yml: :^include/linux/kconfig.h
:^tools/ && exit 1 || exit 0
.gitlab-ci.yml::^include/linux/kconfig.h :^tools/ && exit 1 || exit 0
Makefile: -include $(srctree)/include/linux/kconfig.h
Makefile: -include linux/kconfig.h -include
include/config.h \
arch/arm/include/asm/arch-fsl-layerscape/config.h:#include
arch/arm/mach-rockchip/tpl.c:#include
arch/arm/mach-sunxi/dram_sun50i_h6.c:#include
arch/arm/mach-sunxi/dram_sun50i_h616.c:#include
arch/arm/mach-sunxi/dram_sunxi_dw.c:#include
boot/image-fit.c:#include
boot/image.c:#include
These are the sorts of ones which are a real pain to remove.
common/hash.c:#include
drivers/timer/dw-apb-timer.c:#include
env/embedded.c:#include
include/bootstage.h:#include
include/configs/at91-sama5_common.h:#include
include/configs/tqma6.h:#include
include/env_internal.h:#include
include/hash.h:#include
include/image.h:#include
include/u-boot/ecdsa.h:#include
lib/rsa/rsa-verify.c:#include
scripts/Makefile.autoconf: -include
$(srctree)/include/linux/kconfig.h
scripts/Makefile.autoconf: echo \#include \;
\
test/dm/scmi.c:#include
test/lib/kconfig.c: * Test of linux/kconfig.h macros
test/lib/kconfig_spl.c: * Test of linux/kconfig.h macros for SPL
tools/binman/test/blob_syms.c:#include
tools/binman/test/u_boot_binman_syms.c:#include
tools/binman/test/u_boot_binman_syms_size.c:#include
tools/env/fw_env_private.h:#include
tools/envcrc.c:#include
tools/mkeficapsule.c:#include
tools/printinitialenv.c:#include
I don't really mind what we do with sha, and it seems I am a bit too
rabid on the anti-#ifdef thing compared to others.
Regards,
Simon
Re: [PATCH 1/2 v3] smbios: Simplify reporting of unknown values
Hi Ilias,
On Wed, 13 Dec 2023 at 14:37, Ilias Apalodimas
wrote:
>
> Hi Simon,
>
> On Wed, 13 Dec 2023 at 21:52, Simon Glass wrote:
> >
> > Hi Ilias,
> >
> > On Thu, 7 Dec 2023 at 02:19, Ilias Apalodimas
> > wrote:
> > >
> > > If a value is not valid during the DT or SYSINFO parsing, we explicitly
> > > set that to "Unknown Product" and "Unknown" for the product and
> > > manufacturer respectively. It's cleaner if we move the checks insisde
> > > smbios_add_prop_si() and provide an alternative string in case the
> > > primary is NULL or empty
> > >
> > > pre-patch dmidecode
> > >
> > > Handle 0x0001, DMI type 1, 27 bytes
> > > System Information
> > > Manufacturer: Unknown
> > > Product Name: Unknown Product
> > > Version: Not Specified
> > > Serial Number: Not Specified
> > > UUID: Not Settable
> > > Wake-up Type: Reserved
> > > SKU Number: Not Specified
> > > Family: Not Specified
> > >
> > > [...]
> > >
> > > post-patch dmidecode:
> > >
> > > Handle 0x0001, DMI type 1, 27 bytes
> > > System Information
> > > Manufacturer: Unknown
> > > Product Name: Unknown
> > > Version: Unknown
> > > Serial Number: Unknown
> > > UUID: Not Settable
> > > Wake-up Type: Reserved
> > > SKU Number: Unknown
> > > Family: Unknown
> > > [...]
> > >
> > > While at it make smbios_add_prop_si() add a string directly if the prop
> > > node is NULL and replace smbios_add_string() calls with
> > > smbios_add_prop_si(ctx, NULL, )
> > >
> > > Signed-off-by: Ilias Apalodimas
> > > ---
> > > Changes since v2:
> > > - refactor even more code and remove the smbios_add_string calls from the
> > > main code. Instead use smbios_add_prop() foir everything and add a
> > > default value, in case the parsed one is NULL or emtpy
> > > Changes since v1:
> > > - None
> > >
> > > lib/smbios.c | 73 +---
> > > 1 file changed, 35 insertions(+), 38 deletions(-)
> > >
> >
> > I actually think we should just stop here and first write a test for
> > what we already have. I can take a crack at it if you like?
>
> I don't mind. As I said in a previous email I'd rather do it after
> the refactoring you had in mind. But I don't see why that should stop
> the current patch from going forward.
OK, well if this series could be simplified, I don't mind either. But
at present it is complicated and I think it really needs a test.
>
> >
> > > diff --git a/lib/smbios.c b/lib/smbios.c
> > > index d7f4999e8b2a..444aa245a273 100644
> > > --- a/lib/smbios.c
> > > +++ b/lib/smbios.c
> > > @@ -102,9 +102,6 @@ static int smbios_add_string(struct smbios_ctx *ctx,
> > > const char *str)
> > > int i = 1;
> > > char *p = ctx->eos;
> > >
> > > - if (!*str)
> > > - str = "Unknown";
> > > -
> > > for (;;) {
> > > if (!*p) {
> > > ctx->last_str = p;
> > > @@ -134,11 +131,18 @@ static int smbios_add_string(struct smbios_ctx
> > > *ctx, const char *str)
> > > *
> > > * @ctx: context for writing the tables
> > > * @prop: property to write
> >
> > which can now be NULL?
>
> No, it can't because smbios_add_string can't do that. See commit
> 00a871d34e2f0a.
> But instead of checking it here, I moved the code around and
> smbios_add_string is only called from smbios_add_prop_si instead of
> calling it on each failure of smbios_add_prop_si(). In that function,
> we make sure the value isn't NULL, apart from the ->get_str sysinfo
> calls -- none of these currently returns null though. I can move the
> checking back to where it was if to shield us again dump 'get_str'
> implementations, or in the future fix smbios_add_string properly, but
> that's not the point of this patch.
My point was that you have code to check for !prop and do something:
if (!prop)
return smbios_add_string(ctx, dval);
Before, we needed prop (at least if OF_CONTROL), but now prop can be
NULL, so you should update the comment to mention that and explain
what it means.
>
> >
> > > + * @dval: Default value to use if the string is not found or is
> > > empty
> > > * Return: 0 if not found, else SMBIOS string number (1 or more)
> > > */
> > > static int smbios_add_prop_si(struct smbios_ctx *ctx, const char *prop,
> > > - int sysinfo_id)
> > > + int sysinfo_id, const char *dval)
> > > {
> > > + if (!dval || !*dval)
> > > + dval = "Unknown";
> >
> > You shouldn't need this, right? It is already the default value.
>
> Unless someone misuses smbios_add_prop_si() with both the prop and
> dval being NULL. Also see above.
Don't allow that, then? It seems strange to have a default value for
the default value.
>
> >
> > > +
> > > + if (!prop)
> > > + return smbios_add_string(ctx, dval);
> > > +
> > > if
Re: [PATCH v3] mmc: Poll CD in case cyclic framework is enabled
Hi Marek,
On Wed, 13 Dec 2023 at 14:00, Marek Vasut wrote:
>
> On 12/13/23 20:49, Simon Glass wrote:
> > Hi Marek,
> >
> > On Wed, 13 Dec 2023 at 07:08, Marek Vasut wrote:
> >>
> >> On 12/12/23 15:05, Simon Glass wrote:
> >>> Hi Marek,
> >>>
> >>> On Mon, 11 Dec 2023 at 12:24, Marek Vasut wrote:
>
> On 12/11/23 18:52, Simon Glass wrote:
> > Hi Marek,
> >
> > On Sun, 10 Dec 2023 at 08:03, Marek Vasut
> > wrote:
> >>
> >> In case the cyclic framework is enabled, poll the card detect of
> >> already
> >> initialized cards and deinitialize them in case they are removed. Since
> >> the card initialization is a longer process and card initialization is
> >> done on first access to an uninitialized card anyway, avoid
> >> initializing
> >> newly detected uninitialized cards in the cyclic callback.
> >>
> >> Signed-off-by: Marek Vasut
> >> ---
> >> Cc: Jaehoon Chung
> >> Cc: Peng Fan
> >> Cc: Simon Glass
> >> ---
> >> V2: Move the cyclic registration/unregistration into mmc init/deinit
> >> V3: Replace if (CONFIG_IS_ENABLED(CYCLIC)...) with #if as the former
> >>does not work with structure members
> >> ---
> >> drivers/mmc/mmc.c | 36
> >> include/mmc.h | 5 +
> >> 2 files changed, 41 insertions(+)
> >>
> >> diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
> >> index eb5010c1465..a5686dbc12e 100644
> >> --- a/drivers/mmc/mmc.c
> >> +++ b/drivers/mmc/mmc.c
> >> @@ -2985,6 +2985,22 @@ static int mmc_complete_init(struct mmc *mmc)
> >>return err;
> >> }
> >>
> >> +#if CONFIG_IS_ENABLED(CYCLIC)
> >> +static void mmc_cyclic_cd_poll(void *ctx)
> >> +{
> >> + struct mmc *m = ctx;
> >> +
> >> + if (!m->has_init)
> >> + return;
> >> +
> >> + if (mmc_getcd(m))
> >> + return;
> >> +
> >> + mmc_deinit(m);
> >> + m->has_init = 0;
> >> +}
> >> +#endif
> >> +
> >> int mmc_init(struct mmc *mmc)
> >> {
> >>int err = 0;
> >> @@ -3007,6 +3023,19 @@ int mmc_init(struct mmc *mmc)
> >>if (err)
> >>pr_info("%s: %d, time %lu\n", __func__, err,
> >> get_timer(start));
> >>
> >> +#if CONFIG_IS_ENABLED(CYCLIC)
> >
> > We really shouldn't be adding new #ifdefs to the code.
> >
> > If you really want to make put ->cyclic behind an #ifdef then how
> > about creating an accessor as is done in global_data.h ?
>
> That's really just working around the underlying issue, which is that
> this does not compile:
>
> struct foo {
> #if CONFIG_IS_ENABLED(STUFF)
> type member;
> #endif
> ...
> };
>
> type fn() {
> ...
> if (CONFIG_IS_ENABLED(STUFF))
> access struct->member;
> ...
> }
>
> Accessor won't make it any better, would it ? It would only attempt to
> hide the error and make the code more fragile, i.e. this:
>
> accessor(struct)
> {
> type fn() {
> if (CONFIG_IS_ENABLED(STUFF))
> return access struct->member;
> else
> return 0;
> }
>
> }
>
> type fn() {
> ...
> accessor(struct);
> ...
> }
>
> I suspect it also won't compile for one thing, and for the other, it
> really just hides the issue.
> >>>
> >>> No, I mean like this:
> >>>
> >>> #ifdef CONFIG_ACPI
> >>> #define gd_acpi_ctx() gd->acpi_ctx
> >>> #define gd_acpi_start() gd->acpi_start
> >>> #define gd_set_acpi_start(addr) gd->acpi_start = addr
> >>> #else
> >>> #define gd_acpi_ctx() NULL
> >>> #define gd_acpi_start() 0UL
> >>> #define gd_set_acpi_start(addr)
> >>> #endif
> >>>
> >>> The header file has #ifdefs but not the C files.
> >>
> >> Hmmm, I can't say I like that either, that just adds more indirection
> >> into the code and makes it harder to read in my opinion .
> >
> > Sure, it isn't for everyone. But adding new #ifdefs is really grim
> > IMO. Maybe you will get used to it?
>
> I don't really want to get used to such workarounds which only obfuscate
> the code and decrease readability.
>
> > Maybe there is a better approach?
>
> I am open to suggestions . If there are none, I would say go with the
> ifdefs .
OK
Regards,
Simon
Re: [PATCH 2/2 v3] smbios: Fallback to the default DT if sysinfo nodes are missing
Hi Ilias,
On Wed, 13 Dec 2023 at 14:43, Ilias Apalodimas
wrote:
>
> Hi Simon,
>
> [...]
>
> >
> > Can we add a Kconfig to enable this?
>
> I don't see the point, and I am not the only one.
The information is not curated and isn't accurate. E.g. it can end up
putting the vendor, device and version all into a single field,
instead of using the fields correctly.
>
> > > + * @size: str buffer length
>
> [...]
>
> > > + int cnt = 0;
> > > + char *token;
> > > +
> > > + memset(str, 0, size);
> >
> > *str = '\0' should be enough, assuming size is not 0
>
> I generally prefer initializing all memory, it's not that big of a burden
> here.
OK, not a big deal, just odd. Also it is a character, so '\0' is better.
>
> [...]
>
> > > - str = ofnode_read_string(ctx->node, prop);
> > > + const char *str = NULL;
> > > + char str_dt[128] = { 0 };
> > > + /*
> > > +* If the node is not valid fallback and try the entire DT
> >
> > s/and/then/ ?
> >
>
> Sure
>
> > > +* so we can at least fill in manufacturer and board type
> > > +*/
> > > + if (ofnode_valid(ctx->node)) {
> > > + str = ofnode_read_string(ctx->node, prop);
> > > + } else {
> > > + const struct map_sysinfo *nprop;
> > > +
> > > + nprop = convert_sysinfo_to_dt(prop);
> > > + get_str_from_dt(nprop, str_dt, sizeof(str_dt));
> > > + str = (const char *)str_dt;
> >
> > can't you drop the case?
>
> What case?
Sorry, I meant 'cast'.
Regards,
Simon
Re: [PATCH 2/2 v3] smbios: Fallback to the default DT if sysinfo nodes are missing
Hi Simon,
[...]
>
> Can we add a Kconfig to enable this?
I don't see the point, and I am not the only one.
> > + * @size: str buffer length
[...]
> > + int cnt = 0;
> > + char *token;
> > +
> > + memset(str, 0, size);
>
> *str = '\0' should be enough, assuming size is not 0
I generally prefer initializing all memory, it's not that big of a burden here.
[...]
> > - str = ofnode_read_string(ctx->node, prop);
> > + const char *str = NULL;
> > + char str_dt[128] = { 0 };
> > + /*
> > +* If the node is not valid fallback and try the entire DT
>
> s/and/then/ ?
>
Sure
> > +* so we can at least fill in manufacturer and board type
> > +*/
> > + if (ofnode_valid(ctx->node)) {
> > + str = ofnode_read_string(ctx->node, prop);
> > + } else {
> > + const struct map_sysinfo *nprop;
> > +
> > + nprop = convert_sysinfo_to_dt(prop);
> > + get_str_from_dt(nprop, str_dt, sizeof(str_dt));
> > + str = (const char *)str_dt;
>
> can't you drop the case?
What case?
[...]
Thanks
/Ilias
Re: [PATCH 1/2 v3] smbios: Simplify reporting of unknown values
Hi Simon,
On Wed, 13 Dec 2023 at 21:52, Simon Glass wrote:
>
> Hi Ilias,
>
> On Thu, 7 Dec 2023 at 02:19, Ilias Apalodimas
> wrote:
> >
> > If a value is not valid during the DT or SYSINFO parsing, we explicitly
> > set that to "Unknown Product" and "Unknown" for the product and
> > manufacturer respectively. It's cleaner if we move the checks insisde
> > smbios_add_prop_si() and provide an alternative string in case the
> > primary is NULL or empty
> >
> > pre-patch dmidecode
> >
> > Handle 0x0001, DMI type 1, 27 bytes
> > System Information
> > Manufacturer: Unknown
> > Product Name: Unknown Product
> > Version: Not Specified
> > Serial Number: Not Specified
> > UUID: Not Settable
> > Wake-up Type: Reserved
> > SKU Number: Not Specified
> > Family: Not Specified
> >
> > [...]
> >
> > post-patch dmidecode:
> >
> > Handle 0x0001, DMI type 1, 27 bytes
> > System Information
> > Manufacturer: Unknown
> > Product Name: Unknown
> > Version: Unknown
> > Serial Number: Unknown
> > UUID: Not Settable
> > Wake-up Type: Reserved
> > SKU Number: Unknown
> > Family: Unknown
> > [...]
> >
> > While at it make smbios_add_prop_si() add a string directly if the prop
> > node is NULL and replace smbios_add_string() calls with
> > smbios_add_prop_si(ctx, NULL, )
> >
> > Signed-off-by: Ilias Apalodimas
> > ---
> > Changes since v2:
> > - refactor even more code and remove the smbios_add_string calls from the
> > main code. Instead use smbios_add_prop() foir everything and add a
> > default value, in case the parsed one is NULL or emtpy
> > Changes since v1:
> > - None
> >
> > lib/smbios.c | 73 +---
> > 1 file changed, 35 insertions(+), 38 deletions(-)
> >
>
> I actually think we should just stop here and first write a test for
> what we already have. I can take a crack at it if you like?
I don't mind. As I said in a previous email I'd rather do it after
the refactoring you had in mind. But I don't see why that should stop
the current patch from going forward.
>
> > diff --git a/lib/smbios.c b/lib/smbios.c
> > index d7f4999e8b2a..444aa245a273 100644
> > --- a/lib/smbios.c
> > +++ b/lib/smbios.c
> > @@ -102,9 +102,6 @@ static int smbios_add_string(struct smbios_ctx *ctx,
> > const char *str)
> > int i = 1;
> > char *p = ctx->eos;
> >
> > - if (!*str)
> > - str = "Unknown";
> > -
> > for (;;) {
> > if (!*p) {
> > ctx->last_str = p;
> > @@ -134,11 +131,18 @@ static int smbios_add_string(struct smbios_ctx *ctx,
> > const char *str)
> > *
> > * @ctx: context for writing the tables
> > * @prop: property to write
>
> which can now be NULL?
No, it can't because smbios_add_string can't do that. See commit 00a871d34e2f0a.
But instead of checking it here, I moved the code around and
smbios_add_string is only called from smbios_add_prop_si instead of
calling it on each failure of smbios_add_prop_si(). In that function,
we make sure the value isn't NULL, apart from the ->get_str sysinfo
calls -- none of these currently returns null though. I can move the
checking back to where it was if to shield us again dump 'get_str'
implementations, or in the future fix smbios_add_string properly, but
that's not the point of this patch.
>
> > + * @dval: Default value to use if the string is not found or is empty
> > * Return: 0 if not found, else SMBIOS string number (1 or more)
> > */
> > static int smbios_add_prop_si(struct smbios_ctx *ctx, const char *prop,
> > - int sysinfo_id)
> > + int sysinfo_id, const char *dval)
> > {
> > + if (!dval || !*dval)
> > + dval = "Unknown";
>
> You shouldn't need this, right? It is already the default value.
Unless someone misuses smbios_add_prop_si() with both the prop and
dval being NULL. Also see above.
>
> > +
> > + if (!prop)
> > + return smbios_add_string(ctx, dval);
> > +
> > if (sysinfo_id && ctx->dev) {
> > char val[SMBIOS_STR_MAX];
> > int ret;
> > @@ -151,8 +155,8 @@ static int smbios_add_prop_si(struct smbios_ctx *ctx,
> > const char *prop,
> > const char *str;
> >
> > str = ofnode_read_string(ctx->node, prop);
> > - if (str)
> > - return smbios_add_string(ctx, str);
> > +
> > + return smbios_add_string(ctx, str ? str : dval);
> > }
> >
> > return 0;
> > @@ -161,12 +165,15 @@ static int smbios_add_prop_si(struct smbios_ctx *ctx,
> > const char *prop,
> > /**
> > * smbios_add_prop() - Add a property from the devicetree
> > *
> > - * @prop: property to write
> > + * @prop: property to write. The default string will be written if
> > + *
[PATCH v2] net: wget: Support non-default HTTP port
Currently the wget command is hard wired to HTTP port 80. This is
inconvenient, as it is extremely easy to start trivial HTTP server
as an unprivileged user using e.g. python http module to serve the
files, but such a server has to run on one of the higher ports:
"
$ python3 -m http.server -d $(pwd) 8080
"
Make it possible to configure HTTP server port the same way it is
possible to configure TFTP server port, using environment variable
'httpdstp' (similar to 'tftpdstp'). Retain port 80 as the default
fallback port. This way, users can start their own trivial server
and conveniently download whatever files they need into U-Boot.
Signed-off-by: Marek Vasut
---
Cc: "Ying-Chun Liu (PaulLiu)"
Cc: Duncan Hare
Cc: Joe Hershberger
Cc: Ramon Fried
Cc: Simon Glass
Cc: Tom Rini
---
V2: Update documentation
---
doc/usage/cmd/wget.rst| 3 ++-
doc/usage/environment.rst | 4
include/net/wget.h| 1 -
net/wget.c| 14 ++
4 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/doc/usage/cmd/wget.rst b/doc/usage/cmd/wget.rst
index e1e7f8d8145..8e7383b6c60 100644
--- a/doc/usage/cmd/wget.rst
+++ b/doc/usage/cmd/wget.rst
@@ -16,7 +16,8 @@ Description
The wget command is used to download a file from an HTTP server.
wget command will use HTTP over TCP to download files from an HTTP server.
-Currently it can only download image from an HTTP server hosted on port 80.
+By default the destination port is 80 and the source port is pseudo-random.
+The environment variable *httpdstp* can be used to set the destination port.
address
memory address for the data downloaded
diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst
index c57b717caaf..82b6ea7b6e7 100644
--- a/doc/usage/environment.rst
+++ b/doc/usage/environment.rst
@@ -306,6 +306,10 @@ ethrotate
anything other than "no", U-Boot does go through all
available network interfaces.
+httpdstp
+If this is set, the value is used for HTTP's TCP
+destination port instead of the default port 80.
+
netretry
When set to "no" each network operation will
either succeed or fail without retrying.
diff --git a/include/net/wget.h b/include/net/wget.h
index da0920de118..6714f7ea573 100644
--- a/include/net/wget.h
+++ b/include/net/wget.h
@@ -17,6 +17,5 @@ enum wget_state {
};
#define DEBUG_WGET 0 /* Set to 1 for debug messages */
-#define SERVER_PORT80
#define WGET_RETRY_COUNT 30
#define WGET_TIMEOUT 2000UL
diff --git a/net/wget.c b/net/wget.c
index 6ae2237a0a8..817c5ebd5d0 100644
--- a/net/wget.c
+++ b/net/wget.c
@@ -19,6 +19,9 @@
DECLARE_GLOBAL_DATA_PTR;
+/* The default, change with environment variable 'httpdstp' */
+#define SERVER_PORT80
+
static const char bootfile1[] = "GET ";
static const char bootfile3[] = " HTTP/1.0\r\n\r\n";
static const char http_eom[] = "\r\n\r\n";
@@ -134,19 +137,22 @@ static void wget_send_stored(void)
int len = retry_len;
unsigned int tcp_ack_num = retry_tcp_seq_num + (len == 0 ? 1 : len);
unsigned int tcp_seq_num = retry_tcp_ack_num;
+ unsigned int server_port;
uchar *ptr, *offset;
+ server_port = env_get_ulong("httpdstp", 10, SERVER_PORT) & 0x;
+
switch (current_wget_state) {
case WGET_CLOSED:
debug_cond(DEBUG_WGET, "wget: send SYN\n");
current_wget_state = WGET_CONNECTING;
- net_send_tcp_packet(0, SERVER_PORT, our_port, action,
+ net_send_tcp_packet(0, server_port, our_port, action,
tcp_seq_num, tcp_ack_num);
packets = 0;
break;
case WGET_CONNECTING:
pkt_q_idx = 0;
- net_send_tcp_packet(0, SERVER_PORT, our_port, action,
+ net_send_tcp_packet(0, server_port, our_port, action,
tcp_seq_num, tcp_ack_num);
ptr = net_tx_packet + net_eth_hdr_size() +
@@ -161,14 +167,14 @@ static void wget_send_stored(void)
memcpy(offset, , strlen(bootfile3));
offset += strlen(bootfile3);
- net_send_tcp_packet((offset - ptr), SERVER_PORT, our_port,
+ net_send_tcp_packet((offset - ptr), server_port, our_port,
TCP_PUSH, tcp_seq_num, tcp_ack_num);
current_wget_state = WGET_CONNECTED;
break;
case WGET_CONNECTED:
case WGET_TRANSFERRING:
case WGET_TRANSFERRED:
- net_send_tcp_packet(0, SERVER_PORT, our_port, action,
+ net_send_tcp_packet(0, server_port, our_port, action,
tcp_seq_num, tcp_ack_num);
break;
}
--
2.42.0
Re: [PATCH 2/2 v2] smbios: Fallback to the default DT if sysinfo nodes are missing
> From: Simon Glass > Date: Wed, 13 Dec 2023 13:41:05 -0700 > > Hi Tom, > > On Wed, 13 Dec 2023 at 13:19, Tom Rini wrote: > > > > On Wed, Dec 13, 2023 at 12:50:03PM -0700, Simon Glass wrote: > > > Hi Ilias, > > > > > > On Mon, 11 Dec 2023 at 00:49, Ilias Apalodimas > > > wrote: > > > > > > > > Hi Tom, > > > > > > > > On Sat, 9 Dec 2023 at 20:49, Tom Rini wrote: > > > > > > > > > > On Wed, Dec 06, 2023 at 06:03:14PM +0200, Ilias Apalodimas wrote: > > > > > > Hi Simon, > > > > > > > > > > > > On Sat, 2 Dec 2023 at 20:28, Simon Glass wrote: > > > > > > > > > > > > > > Hi Ilias, > > > > > > > > > > > > > > On Wed, 29 Nov 2023 at 23:50, Ilias Apalodimas > > > > > > > wrote: > > > > > > > > > > > > > > > > Hi Simon, > > > > > > > > > > > > > > > > [...] > > > > > > > > > > > > > > > >> > Changes since v1: > > > > > > > >> > - Tokenize the DT node entry and use the appropriate value > > > > > > > >> > instead of > > > > > > > >> > the entire string > > > > > > > >> > - Removed Peters tested/reviewed-by tags due to the above > > > > > > > >> > lib/smbios.c | 94 > > > > > > > >> > +--- > > > > > > > >> > 1 file changed, 90 insertions(+), 4 deletions(-) > > > > > > > >> > > > > > > > > >> > > > > > > > >> Can this be put behind a Kconfig? It adds quite a bit of code > > > > > > > >> which > > > > > > > >> punishes those boards which do the right thing. > > > > > > > > > > > > > > > > > > > > > > > > Sure but OTOH the code increase should be really minimal. But I > > > > > > > > don't mind I can add a Kconfig > > > > > > > > > > > > > > > >> > > > > > > > >> > + > > > > > > > >> > + dt_str = ofnode_read_string(ofnode_root(), > > > > > > > >> > nprop->dt_str); > > > > > > > >> > > > > > > > >> Could this use ofnode_read_string_index() ? > > > > > > > > > > > > > > > > > > > > > > > > Maybe, I'll have a look and change it if that works > > > > > > > > > > > > Unless I am missing something this doesn't work. > > > > > > This is designed to return a string index from a DT property that's > > > > > > defined as > > > > > > foo_property = "value1", "value2" isn't it? > > > > > > > > > > > > The code above is trying to read an existing property (e.g > > > > > > compatible) > > > > > > and get the string after the comma delimiter. > > > > > > Perhaps I should add this in drivers/core/ofnode.c instead? > > > > > > > > > > > > > > > > > > > > > > [...] > > > > > > > > > > > > > > > >> > > > > > > > >> Any chance of a test for this code? > > > > > > > > > > > > > > > > > > > > > > > > Sure, but any suggestions on where to add the test? > > > > > > > > SMBIOS tables are populated on OS booting, do we have a test > > > > > > > > somewhere that boots an OS? > > > > > > > > > > > > > > They are written on startup, right? They should certainly be in > > > > > > > place > > > > > > > before U-Boot enters the command line. > > > > > > > > > > > > Not always. I am not sure if x86 does that, but on the rest of the > > > > > > architectures, they are only initialized when the efi smbios code > > > > > > runs. Wasn't this something you were trying to change? > > > > > > > > > > One of those things I keep repeating is that we don't know for sure > > > > > what > > > > > the right values here are until we load the DT the OS _will_ use. It's > > > > > quite valid to start U-Boot and pass it a generic "good enough" DT at > > > > > run time until U-Boot can see (GPIOs, EEPROM, whatever) what it's on > > > > > and > > > > > what the real DT to load before passing to the OS is. Since U-Boot > > > > > doesn't need SMBIOS tables itself, we should make these "late" not > > > > > "early". > > > > > > > > Fair enough, we can defer the init and testing of those late, just > > > > before we are about to boot. But this is irrelevant to what this patch > > > > does, can we get the fallback mechanism in first, assuming everyone is > > > > ok with the patch now? > > > > > > > > > > I would like this behind a Kconfig. Making it the default means people > > > are going to start relying on (in user space) and then discover later > > > that it is wrong. > > > > What do you mean wrong, exactly? > > "raspberrypi" instead of "Raspberry Pi", for example, or "friendlyarm" > instead of "FriendlyElec" Heh, well, even in the x86 world vendors can't even spell their own name consistently. > I just wonder what this information is actually used for. If it is > just for fun, that is fine, but I believe some tools do use dmidecode, > at which point it needs to be accurate and should not change later, > e.g. when someone decides to actually add the info. So the Linux kernel uses this information to apply quirks for specific machines. But I hope that on platforms that have a device tree folks will just use the board compatible string for that purpose. Otherwise I think this information is mostly used to populate system information UIs and asset databases.
Re: [PATCH v3] mmc: Poll CD in case cyclic framework is enabled
On 12/13/23 20:49, Simon Glass wrote:
Hi Marek,
On Wed, 13 Dec 2023 at 07:08, Marek Vasut wrote:
On 12/12/23 15:05, Simon Glass wrote:
Hi Marek,
On Mon, 11 Dec 2023 at 12:24, Marek Vasut wrote:
On 12/11/23 18:52, Simon Glass wrote:
Hi Marek,
On Sun, 10 Dec 2023 at 08:03, Marek Vasut
wrote:
In case the cyclic framework is enabled, poll the card detect of already
initialized cards and deinitialize them in case they are removed. Since
the card initialization is a longer process and card initialization is
done on first access to an uninitialized card anyway, avoid initializing
newly detected uninitialized cards in the cyclic callback.
Signed-off-by: Marek Vasut
---
Cc: Jaehoon Chung
Cc: Peng Fan
Cc: Simon Glass
---
V2: Move the cyclic registration/unregistration into mmc init/deinit
V3: Replace if (CONFIG_IS_ENABLED(CYCLIC)...) with #if as the former
does not work with structure members
---
drivers/mmc/mmc.c | 36
include/mmc.h | 5 +
2 files changed, 41 insertions(+)
diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
index eb5010c1465..a5686dbc12e 100644
--- a/drivers/mmc/mmc.c
+++ b/drivers/mmc/mmc.c
@@ -2985,6 +2985,22 @@ static int mmc_complete_init(struct mmc *mmc)
return err;
}
+#if CONFIG_IS_ENABLED(CYCLIC)
+static void mmc_cyclic_cd_poll(void *ctx)
+{
+ struct mmc *m = ctx;
+
+ if (!m->has_init)
+ return;
+
+ if (mmc_getcd(m))
+ return;
+
+ mmc_deinit(m);
+ m->has_init = 0;
+}
+#endif
+
int mmc_init(struct mmc *mmc)
{
int err = 0;
@@ -3007,6 +3023,19 @@ int mmc_init(struct mmc *mmc)
if (err)
pr_info("%s: %d, time %lu\n", __func__, err,
get_timer(start));
+#if CONFIG_IS_ENABLED(CYCLIC)
We really shouldn't be adding new #ifdefs to the code.
If you really want to make put ->cyclic behind an #ifdef then how
about creating an accessor as is done in global_data.h ?
That's really just working around the underlying issue, which is that
this does not compile:
struct foo {
#if CONFIG_IS_ENABLED(STUFF)
type member;
#endif
...
};
type fn() {
...
if (CONFIG_IS_ENABLED(STUFF))
access struct->member;
...
}
Accessor won't make it any better, would it ? It would only attempt to
hide the error and make the code more fragile, i.e. this:
accessor(struct)
{
type fn() {
if (CONFIG_IS_ENABLED(STUFF))
return access struct->member;
else
return 0;
}
}
type fn() {
...
accessor(struct);
...
}
I suspect it also won't compile for one thing, and for the other, it
really just hides the issue.
No, I mean like this:
#ifdef CONFIG_ACPI
#define gd_acpi_ctx() gd->acpi_ctx
#define gd_acpi_start() gd->acpi_start
#define gd_set_acpi_start(addr) gd->acpi_start = addr
#else
#define gd_acpi_ctx() NULL
#define gd_acpi_start() 0UL
#define gd_set_acpi_start(addr)
#endif
The header file has #ifdefs but not the C files.
Hmmm, I can't say I like that either, that just adds more indirection
into the code and makes it harder to read in my opinion .
Sure, it isn't for everyone. But adding new #ifdefs is really grim
IMO. Maybe you will get used to it?
I don't really want to get used to such workarounds which only obfuscate
the code and decrease readability.
Maybe there is a better approach?
I am open to suggestions . If there are none, I would say go with the
ifdefs .
Re: [PATCH v2 1/5] cyclic: Add a symbol for SPL
On Wed, Dec 13, 2023 at 01:51:00PM -0700, Simon Glass wrote:
> Hi Tom,
>
> On Wed, 13 Dec 2023 at 13:42, Tom Rini wrote:
> >
> > On Wed, Dec 13, 2023 at 12:50:02PM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Sat, 9 Dec 2023 at 08:48, Tom Rini wrote:
> > > >
> > > > On Sat, Dec 02, 2023 at 08:33:48AM -0700, Simon Glass wrote:
> > > >
> > > > > The cyclic subsystem is currently enabled either in all build phases
> > > > > or none. For tools this should not be enabled, but since lib/shc256.c
> > > > > and other files include watchdog.h in the host build, we must make
> > > > > sure that it is not enabled there.
> > > >
> > > > This part is what I see as the Wrong Direction. There's some code we
> > > > really need to share with our user space tools, in order to not
> > > > copy/paste the same code. In turn, this code must be as user-space
> > > > friendly as possible. Maybe even we re-factor things a little more, if
> > > > needed, so that we _just_ have the library functions in common files,
> > > > and u-boot or user space only files have the make use of logic. I don't
> > > > feel bad about tools/ needing:
> > > > void sha256_csum_wd(const unsigned char *input, unsigned int ilen,
> > > > unsigned char *output, unsigned int chunk_sz)
> > > > {
> > > > sha256_context ctx;
> > > > sha256_starts();
> > > > sha256_update(, input, ilen);
> > > > sha256_finish(, output);
> > > > }
> > > >
> > > > (and so on for other algos) as a duplicate bit of code. Much less so
> > > > than I do about adding to a direct include list (since
> > > > we should never be doing that) so that later on we can if
> > > > (IS_ENABLED(..)) the existing code.
> > >
> > > Bear in mind that we have the CONFIG_TOOLS_... options entirely to
> > > deal with the need for tools to enable features in common code. This
> > > SHA thing is a very small part of the code, compared to common code in
> > > boot/ for example.
> > >
> > > So is this really a win?
> >
> > I don't follow you here, sorry.
>
> I mean that we share a lot of code already, code which contains CONFIG
> options. So does it matter avoiding adding one more?
It's adding to files, we must not do that. And we
don't need to if we don't have (and that in turn,
cyclic.h) included outside of USE_HOSTCC :)
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH 2/2 v3] smbios: Fallback to the default DT if sysinfo nodes are missing
On Wed, Dec 13, 2023 at 01:41:04PM -0700, Simon Glass wrote: > Hi Tom, > > On Wed, 13 Dec 2023 at 13:17, Tom Rini wrote: > > > > On Wed, Dec 13, 2023 at 12:51:33PM -0700, Simon Glass wrote: > > > Hi Ilias, > > > > > > On Thu, 7 Dec 2023 at 02:19, Ilias Apalodimas > > > wrote: > > > > > > > > In order to fill in the SMBIOS tables U-Boot currently relies on a > > > > "u-boot,sysinfo-smbios" compatible node. This is fine for the boards > > > > that already include such nodes. However with some recent EFI changes, > > > > the majority of boards can boot up distros, which usually rely on > > > > things like dmidecode etc for their reporting. For boards that > > > > lack this special node the SMBIOS output looks like: > > > > > > > > System Information > > > > Manufacturer: Unknown > > > > Product Name: Unknown > > > > Version: Unknown > > > > Serial Number: Unknown > > > > UUID: Not Settable > > > > Wake-up Type: Reserved > > > > SKU Number: Unknown > > > > Family: Unknown > > > > > > > > This looks problematic since most of the info are "Unknown". The DT > > > > spec > > > > specifies standard properties containing relevant information like > > > > 'model' and 'compatible' for which the suggested format is > > > > . Unfortunately the 'model' string found in DTs is > > > > usually lacking the manufacturer so we can't use it for both > > > > 'Manufacturer' and 'Product Name' SMBIOS entries reliably. > > > > > > > > So let's add a last resort to our current smbios parsing. If none of > > > > the sysinfo properties are found, scan for those information in the > > > > root node of the device tree. Use the 'model' to fill the 'Product > > > > Name' and the first value of 'compatible' for the 'Manufacturer', since > > > > that always contains one. > > > > > > > > pre-patch: > > > > Handle 0x0001, DMI type 1, 27 bytes > > > > System Information > > > > Manufacturer: Unknown > > > > Product Name: Unknown > > > > Version: Unknown > > > > Serial Number: 1bb24ceb > > > > UUID: 30303031-3030-3030-3061-613234636435 > > > > Wake-up Type: Reserved > > > > SKU Number: Unknown > > > > Family: Unknown > > > > [...] > > > > > > > > and post patch: > > > > Handle 0x0001, DMI type 1, 27 bytes > > > > System Information > > > > Manufacturer: raspberrypi > > > > Product Name: Raspberry Pi 4 Model B Rev 1.1 > > > > Version: Unknown > > > > Serial Number: 1bb24ceb > > > > UUID: 30303031-3030-3030-3061-613234636435 > > > > Wake-up Type: Reserved > > > > SKU Number: Unknown > > > > Family: Unknown > > > > [...] > > > > > > > > Signed-off-by: Ilias Apalodimas > > > > --- > > > > Simon, I'll work with tou on the refactoring you wanted and > > > > remove the EFI requirement of SMBIOS in !x86 platforms. > > > > Since this code has no tests, I'll add some once the refactoring > > > > is done > > > > > > > > Changes since v2: > > > > - Spelling mistakes > > > > - rebase on top of patch #1 changes > > > > Changes since v1: > > > > - Tokenize the DT node entry and use the appropriate value instead of > > > > the entire string > > > > - Removed Peters tested/reviewed-by tags due to the above > > > > lib/smbios.c | 94 +--- > > > > 1 file changed, 89 insertions(+), 5 deletions(-) > > > > > > Can we add a Kconfig to enable this? > > > > Why? This is the default option that we want moving forward in order to > > get the fields populated. > > The model name is fine. The manufacturer is in lower case (using the > DT vendor name), so not in the right format. > > It only populates two fields, so far as I can tell. Maybe we need to turn this discussion on its head slightly. What do you want to do to get SMBIOS fields to be widely populated with generally-correct information? What we have been doing has seen very little adoption so we need something else. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 1/5] cyclic: Add a symbol for SPL
Hi Tom,
On Wed, 13 Dec 2023 at 13:42, Tom Rini wrote:
>
> On Wed, Dec 13, 2023 at 12:50:02PM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Sat, 9 Dec 2023 at 08:48, Tom Rini wrote:
> > >
> > > On Sat, Dec 02, 2023 at 08:33:48AM -0700, Simon Glass wrote:
> > >
> > > > The cyclic subsystem is currently enabled either in all build phases
> > > > or none. For tools this should not be enabled, but since lib/shc256.c
> > > > and other files include watchdog.h in the host build, we must make
> > > > sure that it is not enabled there.
> > >
> > > This part is what I see as the Wrong Direction. There's some code we
> > > really need to share with our user space tools, in order to not
> > > copy/paste the same code. In turn, this code must be as user-space
> > > friendly as possible. Maybe even we re-factor things a little more, if
> > > needed, so that we _just_ have the library functions in common files,
> > > and u-boot or user space only files have the make use of logic. I don't
> > > feel bad about tools/ needing:
> > > void sha256_csum_wd(const unsigned char *input, unsigned int ilen,
> > > unsigned char *output, unsigned int chunk_sz)
> > > {
> > > sha256_context ctx;
> > > sha256_starts();
> > > sha256_update(, input, ilen);
> > > sha256_finish(, output);
> > > }
> > >
> > > (and so on for other algos) as a duplicate bit of code. Much less so
> > > than I do about adding to a direct include list (since
> > > we should never be doing that) so that later on we can if
> > > (IS_ENABLED(..)) the existing code.
> >
> > Bear in mind that we have the CONFIG_TOOLS_... options entirely to
> > deal with the need for tools to enable features in common code. This
> > SHA thing is a very small part of the code, compared to common code in
> > boot/ for example.
> >
> > So is this really a win?
>
> I don't follow you here, sorry.
I mean that we share a lot of code already, code which contains CONFIG
options. So does it matter avoiding adding one more?
> We share lib/sha*.c with host tools for
> generic mkimage functionality. I'm fine with continuing to use
> USE_HOSTCC as the guard for U-Boot / userspace here. And I don't think
> switching these files from:
> #if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG)
> to:
> if (IS_ENABLED(CONFIG_HW_WATCHDOG) || IS_ENABLED(CONFIG_WATCHDOG)) {
> improves readability of the code.
Perhaps I am being too hard on the #ifdefs.
BTW we have a tools_build() function now.
Regards,
Simon
Please pull u-boot-dm/next
Hi Tom, This is for the -next branch https://source.denx.de/u-boot/custodians/u-boot-dm/-/commit/39e7e039b02e1cf8b04ea668e2bfe2cd2ef0e14d https://dev.azure.com/simon0972/u-boot/_build/results?buildId=56=results The following changes since commit 9565771076c2d4b0193f1741b3990695ac33c1f3: Merge patch series "bootm: Refactoring to reduce reliance on CMDLINE (part A)" (2023-12-13 11:51:53 -0500) are available in the Git repository at: git://git.denx.de/u-boot-dm.git tags/dm-next-13dec23 for you to fetch changes up to 39e7e039b02e1cf8b04ea668e2bfe2cd2ef0e14d: test: vboot: Using variable 'old_dtb' before assignment (2023-12-13 13:20:20 -0700) minor improvements to test, acpi updates for new PyPl release Dario Binacchi (1): binman: doc: fix typo Heinrich Schuchardt (11): acpi: move acpi_get_rsdp_addr() to acpi/acpi_table.h acpi: cannot have RSDT above 4 GiB acpi: simplify acpi_write_ssdt() acpi: consider XSDT in acpi_find_table() test: unit test for acpi_find_table() acpi: fix struct acpi_xsdt cmd: acpi: fix acpi list command cmd: check argc for acpi dump binman: elf: Using variable 'old_val' before assignment test: fit: Using variable 'old_dtb' before assignment test: vboot: Using variable 'old_dtb' before assignment Ilias Apalodimas (1): bootstd: Fix a memory leak in the efi manager bootflow Neha Malcom Francis (3): binman: etype: dm: Add entry type for TI DM arm: dts: k3-*-binman: Move to using ti-dm entry type doc: board: ti: k3: Mention TI_DM argument Simon Glass (28): test: Add a new suite for commands test: Add helper to skip to partial console line test: Make UT_LIB_ASN1 depend on sandbox test: Run bootstd tests only on sandbox test: Handle use of stack pointer in bdinfo test: bdinfo: Add missing asserts test: fdt: Add a special case for real boards test: font: Add dependencies on fonts test: event: Only run test_event_probe() on sandbox test: lmb: Move tests into the lib suite test: print: Skip test on x86 video: Add a function to clear the display sandbox: Add a dummy booti command bootstd: Add a menu option to bootflow scan boot: Drop size parameter from image_setup_libfdt() fdt: Check for a valid fdt in oftree_ensure() fdt: Improve the comment for fdt_shrink_to_minimum() fdt: ppc: Drop extra size for ramdisk boot: Move adding initrd earlier in image_setup_libfdt() fdt: Drop the confusing casts in lmb_free() fdt: Move ft_verify_fdt() before the final fixups doc: Update documentation URL u_boot_pylib: Correct readme formatting tools: Keep test_util and patman test files in the pip release u_boot_pylib: Correct files used for pip release tools/make_pip: Add mention of u_boot_pylib in tool list patman: Update the run script tools: Move python tools to version 0.0.6 MAINTAINERS | 2 +- Makefile | 1 + README| 2 +- arch/arm/dts/k3-am625-sk-binman.dtsi | 4 +- arch/arm/dts/k3-am625-verdin-wifi-dev-binman.dtsi | 4 +- arch/arm/dts/k3-am62a-sk-binman.dtsi | 4 +- arch/arm/dts/k3-j7200-binman.dtsi | 4 +- arch/arm/dts/k3-j721e-binman.dtsi | 4 +- arch/arm/dts/k3-j721s2-binman.dtsi| 4 +- arch/mips/lib/bootm.c | 4 +- arch/sandbox/lib/bootm.c | 7 +++ arch/x86/include/asm/acpi_table.h | 9 boot/bootmeth_efi_mgr.c | 2 + boot/fdt_support.c| 1 - boot/image-board.c| 2 +- boot/image-fdt.c | 27 ++-- cmd/Kconfig | 2 +- cmd/acpi.c| 67 - cmd/bootefi.c | 2 +- cmd/bootflow.c| 27 ++-- cmd/booti.c | 2 +- cmd/cls.c | 25 ++- common/console.c | 31 ++ configs/tools-only_defconfig | 1 + doc/board/ti/k3.rst | 7 +++ doc/build/documentation.rst | 2 +- doc/usage/cmd/bootflow.rst| 5 +++ drivers/core/ofnode.c | 5 +++ drivers/misc/qfw.c| 1 + include/acpi/acpi_table.h | 11
Re: [PATCH v2 1/5] cyclic: Add a symbol for SPL
On Wed, Dec 13, 2023 at 12:50:02PM -0700, Simon Glass wrote:
> Hi Tom,
>
> On Sat, 9 Dec 2023 at 08:48, Tom Rini wrote:
> >
> > On Sat, Dec 02, 2023 at 08:33:48AM -0700, Simon Glass wrote:
> >
> > > The cyclic subsystem is currently enabled either in all build phases
> > > or none. For tools this should not be enabled, but since lib/shc256.c
> > > and other files include watchdog.h in the host build, we must make
> > > sure that it is not enabled there.
> >
> > This part is what I see as the Wrong Direction. There's some code we
> > really need to share with our user space tools, in order to not
> > copy/paste the same code. In turn, this code must be as user-space
> > friendly as possible. Maybe even we re-factor things a little more, if
> > needed, so that we _just_ have the library functions in common files,
> > and u-boot or user space only files have the make use of logic. I don't
> > feel bad about tools/ needing:
> > void sha256_csum_wd(const unsigned char *input, unsigned int ilen,
> > unsigned char *output, unsigned int chunk_sz)
> > {
> > sha256_context ctx;
> > sha256_starts();
> > sha256_update(, input, ilen);
> > sha256_finish(, output);
> > }
> >
> > (and so on for other algos) as a duplicate bit of code. Much less so
> > than I do about adding to a direct include list (since
> > we should never be doing that) so that later on we can if
> > (IS_ENABLED(..)) the existing code.
>
> Bear in mind that we have the CONFIG_TOOLS_... options entirely to
> deal with the need for tools to enable features in common code. This
> SHA thing is a very small part of the code, compared to common code in
> boot/ for example.
>
> So is this really a win?
I don't follow you here, sorry. We share lib/sha*.c with host tools for
generic mkimage functionality. I'm fine with continuing to use
USE_HOSTCC as the guard for U-Boot / userspace here. And I don't think
switching these files from:
#if defined(CONFIG_HW_WATCHDOG) || defined(CONFIG_WATCHDOG)
to:
if (IS_ENABLED(CONFIG_HW_WATCHDOG) || IS_ENABLED(CONFIG_WATCHDOG)) {
improves readability of the code.
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v4] arm: dts: rockpro64: Add RockPro64 smbios
Hi Tom,
On Wed, 13 Dec 2023 at 13:29, Tom Rini wrote:
>
> On Wed, Dec 13, 2023 at 12:50:06PM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 11 Dec 2023 at 13:53, Tom Rini wrote:
> > >
> > > On Mon, Dec 11, 2023 at 08:42:19PM +, Shantur Rathore wrote:
> > > > Hi,
> > > >
> > > > On Mon, Nov 13, 2023 at 11:24 AM Shantur Rathore
> > > > wrote:
> > > > >
> > > > > Add smbios information for Pine64 RockPro64 board and enable in
> > > > > config
> > > > >
> > > > > Signed-off-by: Shantur Rathore
> > > > > ---
> > > > > Changes
> > > > > v4: Change PINE64 to Pine64
> > > > > v3: Enable SYSINFO and SYSINFO_SMBIOS in defconfig
> > > > >
> > > > > arch/arm/dts/rk3399-rockpro64-u-boot.dtsi | 22 ++
> > > > > configs/rockpro64-rk3399_defconfig| 2 ++
> > > > > 2 files changed, 24 insertions(+)
> > > > >
> > > > > diff --git a/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
> > > > > b/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
> > > > > index 732727d9b0..089732524a 100644
> > > > > --- a/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
> > > > > +++ b/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
> > > > > @@ -9,6 +9,28 @@
> > > > > chosen {
> > > > > u-boot,spl-boot-order = "same-as-spl", _flash,
> > > > > ,
> > > > > };
> > > > > +
> > > > > +smbios {
> > > > > +compatible = "u-boot,sysinfo-smbios";
> > > > > +smbios {
> > > > > +system {
> > > > > +manufacturer = "Pine64";
> > > > > +product = "RockPro64";
> > > > > +};
> [snip]
> > > Yes, we should just defer this and pickup the SMBIOS series that Ilias
> > > has posted.
> >
> > I don't think it is a suitable substitute, it is just a fallback.
> >
> > So I believe this patch should be applied.
>
> Please note that this patch is adding _less_ details than the top-level
> model field contains today for the platform. The only difference is
> "Pine64" vs "pine64".
The top-level model is "Pine64 RockPro64 v2.1", I believe. But:
- the first part of that is the manufacturer, not the product
- the second part is what we have in this patch
- the third part is the version
SMBIOS tries to split things up into separate fields.
So, perhaps a new version of this patch could add:
system {
version = "2.1";
};
Regards,
Simon
Re: [PATCH 2/2 v2] smbios: Fallback to the default DT if sysinfo nodes are missing
Hi Tom, On Wed, 13 Dec 2023 at 13:19, Tom Rini wrote: > > On Wed, Dec 13, 2023 at 12:50:03PM -0700, Simon Glass wrote: > > Hi Ilias, > > > > On Mon, 11 Dec 2023 at 00:49, Ilias Apalodimas > > wrote: > > > > > > Hi Tom, > > > > > > On Sat, 9 Dec 2023 at 20:49, Tom Rini wrote: > > > > > > > > On Wed, Dec 06, 2023 at 06:03:14PM +0200, Ilias Apalodimas wrote: > > > > > Hi Simon, > > > > > > > > > > On Sat, 2 Dec 2023 at 20:28, Simon Glass wrote: > > > > > > > > > > > > Hi Ilias, > > > > > > > > > > > > On Wed, 29 Nov 2023 at 23:50, Ilias Apalodimas > > > > > > wrote: > > > > > > > > > > > > > > Hi Simon, > > > > > > > > > > > > > > [...] > > > > > > > > > > > > > >> > Changes since v1: > > > > > > >> > - Tokenize the DT node entry and use the appropriate value > > > > > > >> > instead of > > > > > > >> > the entire string > > > > > > >> > - Removed Peters tested/reviewed-by tags due to the above > > > > > > >> > lib/smbios.c | 94 > > > > > > >> > +--- > > > > > > >> > 1 file changed, 90 insertions(+), 4 deletions(-) > > > > > > >> > > > > > > > >> > > > > > > >> Can this be put behind a Kconfig? It adds quite a bit of code > > > > > > >> which > > > > > > >> punishes those boards which do the right thing. > > > > > > > > > > > > > > > > > > > > > Sure but OTOH the code increase should be really minimal. But I > > > > > > > don't mind I can add a Kconfig > > > > > > > > > > > > > >> > > > > > > >> > + > > > > > > >> > + dt_str = ofnode_read_string(ofnode_root(), > > > > > > >> > nprop->dt_str); > > > > > > >> > > > > > > >> Could this use ofnode_read_string_index() ? > > > > > > > > > > > > > > > > > > > > > Maybe, I'll have a look and change it if that works > > > > > > > > > > Unless I am missing something this doesn't work. > > > > > This is designed to return a string index from a DT property that's > > > > > defined as > > > > > foo_property = "value1", "value2" isn't it? > > > > > > > > > > The code above is trying to read an existing property (e.g compatible) > > > > > and get the string after the comma delimiter. > > > > > Perhaps I should add this in drivers/core/ofnode.c instead? > > > > > > > > > > > > > > > > > > > [...] > > > > > > > > > > > > > >> > > > > > > >> Any chance of a test for this code? > > > > > > > > > > > > > > > > > > > > > Sure, but any suggestions on where to add the test? > > > > > > > SMBIOS tables are populated on OS booting, do we have a test > > > > > > > somewhere that boots an OS? > > > > > > > > > > > > They are written on startup, right? They should certainly be in > > > > > > place > > > > > > before U-Boot enters the command line. > > > > > > > > > > Not always. I am not sure if x86 does that, but on the rest of the > > > > > architectures, they are only initialized when the efi smbios code > > > > > runs. Wasn't this something you were trying to change? > > > > > > > > One of those things I keep repeating is that we don't know for sure what > > > > the right values here are until we load the DT the OS _will_ use. It's > > > > quite valid to start U-Boot and pass it a generic "good enough" DT at > > > > run time until U-Boot can see (GPIOs, EEPROM, whatever) what it's on and > > > > what the real DT to load before passing to the OS is. Since U-Boot > > > > doesn't need SMBIOS tables itself, we should make these "late" not > > > > "early". > > > > > > Fair enough, we can defer the init and testing of those late, just > > > before we are about to boot. But this is irrelevant to what this patch > > > does, can we get the fallback mechanism in first, assuming everyone is > > > ok with the patch now? > > > > > > > I would like this behind a Kconfig. Making it the default means people > > are going to start relying on (in user space) and then discover later > > that it is wrong. > > What do you mean wrong, exactly? "raspberrypi" instead of "Raspberry Pi", for example, or "friendlyarm" instead of "FriendlyElec" I just wonder what this information is actually used for. If it is just for fun, that is fine, but I believe some tools do use dmidecode, at which point it needs to be accurate and should not change later, e.g. when someone decides to actually add the info. Regards, Simon
Re: [PATCH 2/2 v3] smbios: Fallback to the default DT if sysinfo nodes are missing
Hi Tom, On Wed, 13 Dec 2023 at 13:17, Tom Rini wrote: > > On Wed, Dec 13, 2023 at 12:51:33PM -0700, Simon Glass wrote: > > Hi Ilias, > > > > On Thu, 7 Dec 2023 at 02:19, Ilias Apalodimas > > wrote: > > > > > > In order to fill in the SMBIOS tables U-Boot currently relies on a > > > "u-boot,sysinfo-smbios" compatible node. This is fine for the boards > > > that already include such nodes. However with some recent EFI changes, > > > the majority of boards can boot up distros, which usually rely on > > > things like dmidecode etc for their reporting. For boards that > > > lack this special node the SMBIOS output looks like: > > > > > > System Information > > > Manufacturer: Unknown > > > Product Name: Unknown > > > Version: Unknown > > > Serial Number: Unknown > > > UUID: Not Settable > > > Wake-up Type: Reserved > > > SKU Number: Unknown > > > Family: Unknown > > > > > > This looks problematic since most of the info are "Unknown". The DT spec > > > specifies standard properties containing relevant information like > > > 'model' and 'compatible' for which the suggested format is > > > . Unfortunately the 'model' string found in DTs is > > > usually lacking the manufacturer so we can't use it for both > > > 'Manufacturer' and 'Product Name' SMBIOS entries reliably. > > > > > > So let's add a last resort to our current smbios parsing. If none of > > > the sysinfo properties are found, scan for those information in the > > > root node of the device tree. Use the 'model' to fill the 'Product > > > Name' and the first value of 'compatible' for the 'Manufacturer', since > > > that always contains one. > > > > > > pre-patch: > > > Handle 0x0001, DMI type 1, 27 bytes > > > System Information > > > Manufacturer: Unknown > > > Product Name: Unknown > > > Version: Unknown > > > Serial Number: 1bb24ceb > > > UUID: 30303031-3030-3030-3061-613234636435 > > > Wake-up Type: Reserved > > > SKU Number: Unknown > > > Family: Unknown > > > [...] > > > > > > and post patch: > > > Handle 0x0001, DMI type 1, 27 bytes > > > System Information > > > Manufacturer: raspberrypi > > > Product Name: Raspberry Pi 4 Model B Rev 1.1 > > > Version: Unknown > > > Serial Number: 1bb24ceb > > > UUID: 30303031-3030-3030-3061-613234636435 > > > Wake-up Type: Reserved > > > SKU Number: Unknown > > > Family: Unknown > > > [...] > > > > > > Signed-off-by: Ilias Apalodimas > > > --- > > > Simon, I'll work with tou on the refactoring you wanted and > > > remove the EFI requirement of SMBIOS in !x86 platforms. > > > Since this code has no tests, I'll add some once the refactoring > > > is done > > > > > > Changes since v2: > > > - Spelling mistakes > > > - rebase on top of patch #1 changes > > > Changes since v1: > > > - Tokenize the DT node entry and use the appropriate value instead of > > > the entire string > > > - Removed Peters tested/reviewed-by tags due to the above > > > lib/smbios.c | 94 +--- > > > 1 file changed, 89 insertions(+), 5 deletions(-) > > > > Can we add a Kconfig to enable this? > > Why? This is the default option that we want moving forward in order to > get the fields populated. The model name is fine. The manufacturer is in lower case (using the DT vendor name), so not in the right format. It only populates two fields, so far as I can tell. Regards, Simon
Re: [PATCH] binman: doc: fix typo
On Thu, 23 Nov 2023 at 06:13, Michael Nazzareno Trimarchi wrote: > > Hi > > On Thu, Nov 23, 2023 at 2:10 PM Dario Binacchi > wrote: > > > > s/use set/set/ > > > > Signed-off-by: Dario Binacchi > > --- > > > > tools/binman/binman.rst | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > Applied to u-boot-dm/next, thanks!
Re: [PATCH 02/34] test: Add a new suite for commands
Add a new suite for 'cmd' tests, used for testing commands. These are kept in the test/cmd directory. For now it is empty, but it will be used for coreboot-command tests. Signed-off-by: Simon Glass --- include/test/cmd.h| 15 +++ include/test/suites.h | 1 + test/cmd/Makefile | 2 ++ test/cmd/cmd_ut_cmd.c | 21 + test/cmd_ut.c | 6 ++ 5 files changed, 45 insertions(+) create mode 100644 include/test/cmd.h create mode 100644 test/cmd/cmd_ut_cmd.c Applied to u-boot-dm/next, thanks!
Re: [PATCH 03/34] test: Add helper to skip to partial console line
Sometimes we need to skip to a line but it includes addresses or other information which can vary depending on the runtime conditions. Add a new ut_assert_skip_to_linen() which is similar to the existing ut_assert_skip_to_line() function but only checks that the console line matches up to the length of the provided string. Signed-off-by: Simon Glass --- include/test/ut.h | 30 ++ test/ut.c | 27 +++ 2 files changed, 57 insertions(+) Applied to u-boot-dm/next, thanks!
Re: [PATCH 04/34] test: Make UT_LIB_ASN1 depend on sandbox
This doesn't seem to work on a real board, so use the test on sandbox only. Signed-off-by: Simon Glass --- test/Kconfig | 1 + 1 file changed, 1 insertion(+) Applied to u-boot-dm/next, thanks!
Re: [PATCH 06/34] test: Handle use of stack pointer in bdinfo
This test assumes that the stack pointer is the same across two calls to lmb_init_and_reserve() but this is not the case on x86, for example. Add a special case to handle this, along with a detailed comment. Signed-off-by: Simon Glass --- test/cmd/bdinfo.c | 12 1 file changed, 12 insertions(+) Applied to u-boot-dm/next, thanks!
Re: [PATCH 05/34] test: Run bootstd tests only on sandbox
These make use of disk images which are not available on reak boards. Add a new Kconfig to ensure these tests only run where they are valid. Signed-off-by: Simon Glass --- test/Kconfig | 5 + test/Makefile | 2 +- test/cmd_ut.c | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) Applied to u-boot-dm/next, thanks!
Re: [PATCH 07/34] test: bdinfo: Add missing asserts
Calling into sub-test functions should be done using ut_assertok() so that the test exits immediately on failure. Add those which are missing. Signed-off-by: Simon Glass --- test/cmd/bdinfo.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) Applied to u-boot-dm/next, thanks!
Re: [PATCH 09/34] test: font: Add dependencies on fonts
The font test needs two fonts. If one is not available, skip out early, to avoid an error. Signed-off-by: Simon Glass --- test/cmd/font.c | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) Applied to u-boot-dm/next, thanks!
Re: [PATCH 10/34] test: event: Only run test_event_probe() on sandbox
This needs test devices which are only present on sandbox. Add a check for this and skip just this test if running on a real board. Signed-off-by: Simon Glass --- test/common/event.c | 3 +++ 1 file changed, 3 insertions(+) Applied to u-boot-dm/next, thanks!
Re: [PATCH 11/34] test: lmb: Move tests into the lib suite
These tests are marked as driver model tests, but have nothing to do with driver model. As a result, they are run as part of 'ut dm' which only exists for sandbox. Move them to the 'lib' suite and drop the requirement for initing devices, since they don't use devices. Also put the lib_test_lmb_max_regions() macro inside the same #ifdef as its function, to avoid a build error if the condition is false. Signed-off-by: Simon Glass --- test/lib/lmb.c | 36 1 file changed, 12 insertions(+), 24 deletions(-) Applied to u-boot-dm/next, thanks!
Re: [PATCH 12/34] test: print: Skip test on x86
These tests cannot work on x86 machines as memory at address zero is not writable. Add a condition to skip these. Signed-off-by: Simon Glass --- test/print_ut.c | 8 1 file changed, 8 insertions(+) Applied to u-boot-dm/next, thanks!
Re: [PATCH 13/34] video: Add a function to clear the display
Hi, On Sun, 1 Oct 2023 at 19:16, Simon Glass wrote: > > Move the code from the 'cls' command into the console file, so it can > be called from elsewhere. > > Signed-off-by: Simon Glass > --- > > cmd/cls.c | 25 +++-- > common/console.c | 31 +++ > include/console.h | 10 ++ > 3 files changed, 44 insertions(+), 22 deletions(-) > Any word on this patch, please? Heinrich, do you want to look at it? Regards, Simon Applied to u-boot-dm/next, thanks!
Re: [PATCH 14/34] sandbox: Add a dummy booti command
Add basic sandbox support for 'booti' so we can start to boot the test ARMbian image. This is helpful in checking that it is parsed correctly. Signed-off-by: Simon Glass --- arch/sandbox/lib/bootm.c | 7 +++ cmd/Kconfig | 2 +- cmd/booti.c | 2 +- configs/tools-only_defconfig | 3 ++- 4 files changed, 11 insertions(+), 3 deletions(-) Applied to u-boot-dm/next, thanks!
Re: [PATCH 15/34] bootstd: Add a menu option to bootflow scan
Allow showing a menu and automatically booting, with 'bootflow scan'. This is more convenient than using a script. Signed-off-by: Simon Glass --- cmd/bootflow.c | 27 +++-- doc/usage/cmd/bootflow.rst | 5 +++ test/boot/bootflow.c | 82 ++ 3 files changed, 110 insertions(+), 4 deletions(-) Applied to u-boot-dm/next, thanks!
Re: [PATCH] acpi: move acpi_get_rsdp_addr() to acpi/acpi_table.h
Hi Tom, Heinrich, On Thu, 9 Nov 2023 at 13:42, Tom Rini wrote: > > On Thu, Nov 09, 2023 at 12:28:26PM -0800, Heinrich Schuchardt wrote: > > On 11/9/23 11:24, Tom Rini wrote: > > > On Thu, Nov 09, 2023 at 09:23:02AM -0800, Heinrich Schuchardt wrote: > > > > > > > Function acpi_get_rsdp_addr() is needed on all architectures which > > > > write ACPI tables. Move the definition from the x86 include to an > > > > architecture independent one. > > > > > > > > Signed-off-by: Heinrich Schuchardt > > > > --- > > > > arch/x86/include/asm/acpi_table.h | 9 - > > > > drivers/misc/qfw.c| 1 + > > > > include/acpi/acpi_table.h | 9 + > > > > 3 files changed, 10 insertions(+), 9 deletions(-) > > > > > > My question here is, does this work right on non-x86? I know you can > > > have ACPI without UEFI on x86, but elsewhere doesn't the location have > > > to be provided in some manner by UEFI? I know I'm thinking that's the > > > case with SMBIOS (with a few exception). > > > > > > I'm assuming you're doing something here with qemu and its qfw interface > > > and risc-v. > > > > > > > Hello Tom, > > > > yes, I want to enable ACPI passthrough on QEMU RISC-V. There will be more > > patches needed. But the current change seemed obvious looking at the code so > > I did not want to pile up more patches first. > > > > Do you know what the status of ACPI passthrough is on ARM? > > On U-Boot? No. I know a while back I asked Simon to, and he did some > hacking such that on a Raspberry Pi you could do UEFI boot and pass > through some ACPI tables and it was somewhat functional (which was I > think an issue with the ACPI tables as much as anything else). And I Yes it worked OK, at least to a point. See [1] > mentioned to some Linaro/EBBR people that it was because in part I don't > object to U-Boot being able to do SystemReady ES level support either > (and to be clear, I don't mean instead of, or preferred over IR, just > that ES mandates ACPI and if we can pass it through and yes, > there's more to it than that, I know). This is what U-Boot does now when booted from coreboot. So it should be fairly easy to arrange. There is a blockage in Linux re booting with ACPI but without EFI, though. I suppose it doesn't affect U-Boot right now since it can boot with EFI, but it is causing problems with other projects. > > And yeah, I see now I had mis-interpreted the "rsdp" portion of the > function name. > > Reviewed-by: Tom Rini > > -- > Tom [1] https://patchwork.ozlabs.org/project/uboot/list/?series=274675=* Applied to u-boot-dm/next, thanks!
Re: [PATCH 1/7] boot: Drop size parameter from image_setup_libfdt()
On Sun, Nov 12, 2023 at 08:27:44AM -0700, Simon Glass wrote: > The of_size parameter is not used, so remove it. > > Signed-off-by: Simon Glass Reviewed-by: Tom Rini -- Tom Applied to u-boot-dm/next, thanks!
Re: [PATCH 4/7] fdt: ppc: Drop extra size for ramdisk
On Sun, Nov 12, 2023 at 08:27:47AM -0700, Simon Glass wrote: > This code dates from around 2008: > >56844a22b76 powerpc: Fix bootm to boot up again with a Ramdisk > > Since then we have added FDT relocation which provides enough space > for expansion. We have also added all sorts of fixups earlier in > image_setup_libfdt() which require more space, with ramdisk being the > least of them. > > Therefore this extra hack for ramdisk seems unnecessary. Drop it. > > Signed-off-by: Simon Glass Reviewed-by: Tom Rini -- Tom Applied to u-boot-dm/next, thanks!
Re: [PATCH 5/7] boot: Move adding initrd earlier in image_setup_libfdt()
This may as well happen before the general event is emitted, so move it. This will allow us to use the livetree for the event part, but the flattree for the earlier part. Signed-off-by: Simon Glass --- boot/image-fdt.c | 5 - include/fdt_support.h | 12 +++- 2 files changed, 15 insertions(+), 2 deletions(-) Applied to u-boot-dm/next, thanks!
Re: [PATCH 6/7] fdt: Drop the confusing casts in lmb_free()
Just use map_to_sysmem() instead of all the casting. Signed-off-by: Simon Glass --- boot/image-fdt.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) Applied to u-boot-dm/next, thanks!
Re: [PATCH v2 1/1] acpi: cannot have RSDT above 4 GiB
On Sun, 12 Nov 2023 at 16:54, Heinrich Schuchardt wrote: > > The field RsdtAddress has only 32 bit. The RSDT table cannot be located > beyond 4 GiB. > > Signed-off-by: Heinrich Schuchardt > --- > v2: > Avoid superfluous 0 assignment. RSDP is already zeroed out. > Use constants form linux/sizes.h > --- > lib/acpi/base.c | 23 --- > 1 file changed, 16 insertions(+), 7 deletions(-) Reviewed-by: Simon Glass Applied to u-boot-dm/next, thanks!
Re: [PATCH v2 2/2] test: unit test for acpi_find_table()
On Sat, 18 Nov 2023 at 14:57, Heinrich Schuchardt wrote: > > Provide a unit test for acpi_find_table() > > Signed-off-by: Heinrich Schuchardt > --- > v2: > new patch > --- > test/dm/acpi.c | 96 ++ > 1 file changed, 96 insertions(+) > Reviewed-by: Simon Glass Applied to u-boot-dm/next, thanks!
Re: [PATCH 1/1] acpi: simplify acpi_write_ssdt()
On Sat, 18 Nov 2023 at 14:52, Heinrich Schuchardt
wrote:
>
> * Converting to void * is superfluous when calling memset().
> * acpi_fill_header() already fills oem_table_id.
>
> Fixes: d953137526cc ("x86: Move SSDT table to a writer function")
> Signed-off-by: Heinrich Schuchardt
> ---
> lib/acpi/ssdt.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
Reviewed-by: Simon Glass
Applied to u-boot-dm/next, thanks!
Re: [PATCH v2 1/2] acpi: consider XSDT in acpi_find_table()
On Sun, 19 Nov 2023 at 02:45, Simon Glass wrote: > > On Sat, 18 Nov 2023 at 14:57, Heinrich Schuchardt > wrote: > > > > The RSDT table is deprecated and does not exist on all systems. > > > > By preference scan XSDT for the table to find. If no XSDT table exists, try > > to use the RSDT table. > > > > Signed-off-by: Heinrich Schuchardt > > --- > > v2: > > consider that map_sysmem(0, 0) != NULL > > --- > > lib/acpi/acpi.c | 20 > > 1 file changed, 16 insertions(+), 4 deletions(-) > > Reviewed-by: Simon Glass Reviewed-by: Ilias Apalodimas Applied to u-boot-dm/next, thanks!
Re: [PATCH 6/7] patman: Update the run script
Patman now has its main program in a function, so update the toml file to match. Signed-off-by: Simon Glass --- tools/patman/pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Applied to u-boot-dm/next, thanks!
Re: [PATCH v4 2/2] cmd: acpi: fix acpi list command
On Tue, 21 Nov 2023 at 07:41, Heinrich Schuchardt wrote: > > ACPI tables may comprise either RSDT, XSDT, or both. The current code fails > to check the presence of the RSDT table before accessing it. This leads to > an exception if the RSDT table is not provided. > > The XSDT table takes precedence over the RSDT table. > > The return values of list_rsdt() and list_rsdp() are always zero and not > checked. Remove the return values. > > Addresses in the XSDT table are 64-bit. Adjust the output accordingly. > > As the RSDT table has to be ignored if the XSDT command is present there is > no need to compare the tables in a display command. Anyway the > specification does not require that the sequence of addresses in the RSDT > and XSDT table are the same. > > The FACS table header does not provide revision information. Correct the > description of dump_hdr(). > > Adjust the ACPI test to match the changed output format of the 'acpi list' > command. > > Signed-off-by: Heinrich Schuchardt > --- > v4: > deduplicate code in list_rsdt() > v3: > consider map_sysmem(0, 0) != NULL on the sandbox > adjust signature of list_rsdt() and list_rsdp() > v2: > add unit test for offset of field Entry in RSDT, XSDT > merge patch 2 and 3 > remove leading zeros from address output > --- > cmd/acpi.c | 64 +++--- > test/dm/acpi.c | 18 +++--- > 2 files changed, 43 insertions(+), 39 deletions(-) Reviewed-by: Simon Glass I really don't like using 16 characters for a 64-bit address. I wonder if there is a better solution? But at least it is leading spaces, not zeroes. Applied to u-boot-dm/next, thanks!
Re: [PATCH 1/7] doc: Update documentation URL
Update to use the new docs.u-boot.org URL for documentation. Signed-off-by: Simon Glass --- MAINTAINERS | 2 +- README| 2 +- doc/build/documentation.rst | 2 +- tools/binman/pyproject.toml | 2 +- tools/buildman/pyproject.toml | 2 +- tools/dtoc/pyproject.toml | 2 +- tools/patman/pyproject.toml | 2 +- tools/u_boot_pylib/pyproject.toml | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) Applied to u-boot-dm/next, thanks!
Re: [PATCH v2 2/3] arm: dts: k3-*-binman: Move to using ti-dm entry type
On Tue, 5 Dec 2023 at 02:42, Neha Malcom Francis wrote: > > Move the DM entry in tispl.bin FIT image from default fetching an > external blob entry to fetching using ti-dm entry type. This way, the > DM entry will be populated by the TI_DM pathname if provided. Else it > will resort to the ti-dm.bin file. > > Signed-off-by: Neha Malcom Francis > Reviewed-by: Andrew Davis > --- > arch/arm/dts/k3-am625-sk-binman.dtsi | 4 ++-- > arch/arm/dts/k3-am625-verdin-wifi-dev-binman.dtsi | 4 ++-- > arch/arm/dts/k3-am62a-sk-binman.dtsi | 4 ++-- > arch/arm/dts/k3-j7200-binman.dtsi | 4 ++-- > arch/arm/dts/k3-j721e-binman.dtsi | 4 ++-- > arch/arm/dts/k3-j721s2-binman.dtsi| 4 ++-- > 6 files changed, 12 insertions(+), 12 deletions(-) Reviewed-by: Simon Glass Applied to u-boot-dm/next, thanks!
Re: [PATCH 5/7] tools/make_pip: Add mention of u_boot_pylib in tool list
This is not a tool but it is handled by the script, so update the help to include it. Signed-off-by: Simon Glass --- scripts/make_pip.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Applied to u-boot-dm/next, thanks!
Re: [PATCH v2 1/3] binman: etype: dm: Add entry type for TI DM
Hi Simon, On 06/12/23 09:24, Simon Glass wrote: > On Tue, 5 Dec 2023 at 02:42, Neha Malcom Francis wrote: >> >> K3 devices introduces the concept of centralized power, resource and >> security management to System Firmware. This is to overcome challenges >> by the traditional approach that implements system control functions on >> each of the processing units. >> >> The software interface for System Firmware is split into TIFS and DM. DM >> (Device Manager) is responsible for resource and power management from >> secure and non-secure hosts. This additional binary is necessary for >> specific platforms' ROM boot images and is to be packaged into tispl.bin >> >> Add an entry for DM. The entry can be used for the packaging of >> tispl.bin by binman along with ATF and TEE. >> >> Signed-off-by: Neha Malcom Francis >> Reviewed-by: Andrew Davis >> --- >> Makefile| 1 + >> tools/binman/entries.rst| 14 ++ >> tools/binman/etype/ti_dm.py | 22 ++ >> tools/binman/ftest.py | 7 +++ >> tools/binman/test/225_ti_dm.dts | 13 + >> 5 files changed, 57 insertions(+) >> create mode 100644 tools/binman/etype/ti_dm.py >> create mode 100644 tools/binman/test/225_ti_dm.dts >> > > Reviewed-by: Simon Glass > > Is there a doc/ update somewhere about TI_DM ? > Yes, it's patch 3/3 in this series. > Regards, > Simon -- Thanking You Neha Malcom Francis Applied to u-boot-dm/next, thanks!
Re: [PATCH 2/7] u_boot_pylib: Correct readme formatting
Correct a heading which is too short in the readme.
Fixes: 75554dfac29 ("patman: Add support for building a u_boot_tools...")
Signed-off-by: Simon Glass
---
tools/u_boot_pylib/README.rst | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Applied to u-boot-dm/next, thanks!
Re: [PATCH 3/7] tools: Keep test_util and patman test files in the pip release
The test_util module is actually imported by some tools, e.g. binman so include it in the pip release. The patman tool uses its test code when starting up, so keep that too. Show a list of deleted files so it is clear what is happening. Signed-off-by: Simon Glass --- scripts/make_pip.sh | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) Applied to u-boot-dm/next, thanks!
Re: [PATCH v4 1/2] acpi: fix struct acpi_xsdt
The size of the ACPI table header is not a multiple of 8. We have to mark struct acpi_xsdt as packed to correctly access field Entry. Add a unit test for the offsets of field Entry in the RSDT and XSDT tables. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- v4: no change v3: no change v2: add unit test for offset of field Entry in RSDT, XSDT --- include/acpi/acpi_table.h | 2 +- test/dm/acpi.c| 10 ++ 2 files changed, 11 insertions(+), 1 deletion(-) Applied to u-boot-dm/next, thanks!
Re: [PATCH 4/7] u_boot_pylib: Correct files used for pip release
The files list is incorrect and dates from a time when the script was run from a different directory. Update it to match all the other tools. Signed-off-by: Simon Glass --- tools/u_boot_pylib/pyproject.toml | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) Applied to u-boot-dm/next, thanks!
Re: [PATCH 7/7] tools: Move python tools to version 0.0.6
A new release has been done with this version, so update it. Use the version numbers in dependencies also. Signed-off-by: Simon Glass --- tools/binman/pyproject.toml | 4 ++-- tools/buildman/pyproject.toml | 4 ++-- tools/dtoc/pyproject.toml | 4 ++-- tools/patman/pyproject.toml | 4 ++-- tools/u_boot_pylib/pyproject.toml | 2 +- 5 files changed, 9 insertions(+), 9 deletions(-) Applied to u-boot-dm/next, thanks!
Re: [PATCH v2 3/3] doc: board: ti: k3: Mention TI_DM argument
Hi Neha, On 15:12-20231205, Neha Malcom Francis wrote: > Mention TI_DM argument can be used to fetch a custom DM binary in the > A72 build instructions for K3 devices. > > Signed-off-by: Neha Malcom Francis > Reviewed-by: Andrew Davis > --- > doc/board/ti/k3.rst | 7 +++ > 1 file changed, 7 insertions(+) > Applied to u-boot-dm/next, thanks!
Re: [PATCH v2 2/2] test: vboot: Using variable 'old_dtb' before assignment
old_dtb can only be assumed initialized in the finally block if it is assigned a value before the try statement. Avoid a pylint error reported by current pylint. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- v2: mention pylint in commit message --- test/py/tests/test_vboot.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) Applied to u-boot-dm/next, thanks!
Re: [PATCH v2 1/2] test: fit: Using variable 'old_dtb' before assignment
old_dtb can only be assumed initialized in the finally block if it is assigned a value before the try statement. Avoid a pylint error reported by current pylint. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- v2: mention pylint in commit message --- test/py/tests/test_fit.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) Applied to u-boot-dm/next, thanks!
Re: [PATCH v2] bootstd: Fix a memory leak in the efi manager bootflow
> From: Ilias Apalodimas
> Date: Thu, 7 Dec 2023 14:36:36 +0200
>
> efi_get_var() allocates memory which has to be freed after the value of
> the variable is consumed. Free the memory properly
>
> Fixes: f2bfa0cb1794 ("bootstd: Make efi_mgr bootmeth work for non-sandbox
> setups")
> Signed-off-by: Ilias Apalodimas
> ---
> Apologies for the quick resend but the previous patches wasn't compiling due
> to
> the missing #include
>
> Changes since v1
> - Add #include since it's needed for free()
>
> boot/bootmeth_efi_mgr.c | 2 ++
> 1 file changed, 2 insertions(+)
Oops!
Reviewed-by: Mark Kettenis
Applied to u-boot-dm/next, thanks!
Re: [PATCH 1/1] binman: elf: Using variable 'old_val' before assignment
On 11.12.23 18:52, Simon Glass wrote: > On Sat, 9 Dec 2023 at 11:50, Heinrich Schuchardt > wrote: >> >> old_val can only be assumed initialized in the finally block >> if it is assigned a value before the try statement. >> >> Signed-off-by: Heinrich Schuchardt >> --- >> tools/binman/elf_test.py | 8 >> 1 file changed, 4 insertions(+), 4 deletions(-) >> Applied to u-boot-dm/next, thanks!
Re: [PATCH 1/1] cmd: check argc for acpi dump
On Sat, 9 Dec 2023 at 10:05, Heinrich Schuchardt wrote: > > 'acpi dump' without parameter results in a NULL dereference. Check the > number of arguments. > > Signed-off-by: Heinrich Schuchardt > --- > cmd/acpi.c | 3 +++ > 1 file changed, 3 insertions(+) > Reviewed-by: Simon Glass This could have a test in test/dm/acpi.c if you like Applied to u-boot-dm/next, thanks!
Re: [PATCH v4] arm: dts: rockpro64: Add RockPro64 smbios
On Wed, Dec 13, 2023 at 12:50:06PM -0700, Simon Glass wrote:
> Hi Tom,
>
> On Mon, 11 Dec 2023 at 13:53, Tom Rini wrote:
> >
> > On Mon, Dec 11, 2023 at 08:42:19PM +, Shantur Rathore wrote:
> > > Hi,
> > >
> > > On Mon, Nov 13, 2023 at 11:24 AM Shantur Rathore wrote:
> > > >
> > > > Add smbios information for Pine64 RockPro64 board and enable in
> > > > config
> > > >
> > > > Signed-off-by: Shantur Rathore
> > > > ---
> > > > Changes
> > > > v4: Change PINE64 to Pine64
> > > > v3: Enable SYSINFO and SYSINFO_SMBIOS in defconfig
> > > >
> > > > arch/arm/dts/rk3399-rockpro64-u-boot.dtsi | 22 ++
> > > > configs/rockpro64-rk3399_defconfig| 2 ++
> > > > 2 files changed, 24 insertions(+)
> > > >
> > > > diff --git a/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
> > > > b/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
> > > > index 732727d9b0..089732524a 100644
> > > > --- a/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
> > > > +++ b/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi
> > > > @@ -9,6 +9,28 @@
> > > > chosen {
> > > > u-boot,spl-boot-order = "same-as-spl", _flash,
> > > > ,
> > > > };
> > > > +
> > > > +smbios {
> > > > +compatible = "u-boot,sysinfo-smbios";
> > > > +smbios {
> > > > +system {
> > > > +manufacturer = "Pine64";
> > > > +product = "RockPro64";
> > > > +};
[snip]
> > Yes, we should just defer this and pickup the SMBIOS series that Ilias
> > has posted.
>
> I don't think it is a suitable substitute, it is just a fallback.
>
> So I believe this patch should be applied.
Please note that this patch is adding _less_ details than the top-level
model field contains today for the platform. The only difference is
"Pine64" vs "pine64".
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH 2/2 v2] smbios: Fallback to the default DT if sysinfo nodes are missing
On Wed, Dec 13, 2023 at 12:50:03PM -0700, Simon Glass wrote: > Hi Ilias, > > On Mon, 11 Dec 2023 at 00:49, Ilias Apalodimas > wrote: > > > > Hi Tom, > > > > On Sat, 9 Dec 2023 at 20:49, Tom Rini wrote: > > > > > > On Wed, Dec 06, 2023 at 06:03:14PM +0200, Ilias Apalodimas wrote: > > > > Hi Simon, > > > > > > > > On Sat, 2 Dec 2023 at 20:28, Simon Glass wrote: > > > > > > > > > > Hi Ilias, > > > > > > > > > > On Wed, 29 Nov 2023 at 23:50, Ilias Apalodimas > > > > > wrote: > > > > > > > > > > > > Hi Simon, > > > > > > > > > > > > [...] > > > > > > > > > > > >> > Changes since v1: > > > > > >> > - Tokenize the DT node entry and use the appropriate value > > > > > >> > instead of > > > > > >> > the entire string > > > > > >> > - Removed Peters tested/reviewed-by tags due to the above > > > > > >> > lib/smbios.c | 94 > > > > > >> > +--- > > > > > >> > 1 file changed, 90 insertions(+), 4 deletions(-) > > > > > >> > > > > > > >> > > > > > >> Can this be put behind a Kconfig? It adds quite a bit of code which > > > > > >> punishes those boards which do the right thing. > > > > > > > > > > > > > > > > > > Sure but OTOH the code increase should be really minimal. But I > > > > > > don't mind I can add a Kconfig > > > > > > > > > > > >> > > > > > >> > + > > > > > >> > + dt_str = ofnode_read_string(ofnode_root(), > > > > > >> > nprop->dt_str); > > > > > >> > > > > > >> Could this use ofnode_read_string_index() ? > > > > > > > > > > > > > > > > > > Maybe, I'll have a look and change it if that works > > > > > > > > Unless I am missing something this doesn't work. > > > > This is designed to return a string index from a DT property that's > > > > defined as > > > > foo_property = "value1", "value2" isn't it? > > > > > > > > The code above is trying to read an existing property (e.g compatible) > > > > and get the string after the comma delimiter. > > > > Perhaps I should add this in drivers/core/ofnode.c instead? > > > > > > > > > > > > > > > > [...] > > > > > > > > > > > >> > > > > > >> Any chance of a test for this code? > > > > > > > > > > > > > > > > > > Sure, but any suggestions on where to add the test? > > > > > > SMBIOS tables are populated on OS booting, do we have a test > > > > > > somewhere that boots an OS? > > > > > > > > > > They are written on startup, right? They should certainly be in place > > > > > before U-Boot enters the command line. > > > > > > > > Not always. I am not sure if x86 does that, but on the rest of the > > > > architectures, they are only initialized when the efi smbios code > > > > runs. Wasn't this something you were trying to change? > > > > > > One of those things I keep repeating is that we don't know for sure what > > > the right values here are until we load the DT the OS _will_ use. It's > > > quite valid to start U-Boot and pass it a generic "good enough" DT at > > > run time until U-Boot can see (GPIOs, EEPROM, whatever) what it's on and > > > what the real DT to load before passing to the OS is. Since U-Boot > > > doesn't need SMBIOS tables itself, we should make these "late" not > > > "early". > > > > Fair enough, we can defer the init and testing of those late, just > > before we are about to boot. But this is irrelevant to what this patch > > does, can we get the fallback mechanism in first, assuming everyone is > > ok with the patch now? > > > > I would like this behind a Kconfig. Making it the default means people > are going to start relying on (in user space) and then discover later > that it is wrong. What do you mean wrong, exactly? -- Tom signature.asc Description: PGP signature
Re: [PATCH 2/2 v3] smbios: Fallback to the default DT if sysinfo nodes are missing
On Wed, Dec 13, 2023 at 12:51:33PM -0700, Simon Glass wrote: > Hi Ilias, > > On Thu, 7 Dec 2023 at 02:19, Ilias Apalodimas > wrote: > > > > In order to fill in the SMBIOS tables U-Boot currently relies on a > > "u-boot,sysinfo-smbios" compatible node. This is fine for the boards > > that already include such nodes. However with some recent EFI changes, > > the majority of boards can boot up distros, which usually rely on > > things like dmidecode etc for their reporting. For boards that > > lack this special node the SMBIOS output looks like: > > > > System Information > > Manufacturer: Unknown > > Product Name: Unknown > > Version: Unknown > > Serial Number: Unknown > > UUID: Not Settable > > Wake-up Type: Reserved > > SKU Number: Unknown > > Family: Unknown > > > > This looks problematic since most of the info are "Unknown". The DT spec > > specifies standard properties containing relevant information like > > 'model' and 'compatible' for which the suggested format is > > . Unfortunately the 'model' string found in DTs is > > usually lacking the manufacturer so we can't use it for both > > 'Manufacturer' and 'Product Name' SMBIOS entries reliably. > > > > So let's add a last resort to our current smbios parsing. If none of > > the sysinfo properties are found, scan for those information in the > > root node of the device tree. Use the 'model' to fill the 'Product > > Name' and the first value of 'compatible' for the 'Manufacturer', since > > that always contains one. > > > > pre-patch: > > Handle 0x0001, DMI type 1, 27 bytes > > System Information > > Manufacturer: Unknown > > Product Name: Unknown > > Version: Unknown > > Serial Number: 1bb24ceb > > UUID: 30303031-3030-3030-3061-613234636435 > > Wake-up Type: Reserved > > SKU Number: Unknown > > Family: Unknown > > [...] > > > > and post patch: > > Handle 0x0001, DMI type 1, 27 bytes > > System Information > > Manufacturer: raspberrypi > > Product Name: Raspberry Pi 4 Model B Rev 1.1 > > Version: Unknown > > Serial Number: 1bb24ceb > > UUID: 30303031-3030-3030-3061-613234636435 > > Wake-up Type: Reserved > > SKU Number: Unknown > > Family: Unknown > > [...] > > > > Signed-off-by: Ilias Apalodimas > > --- > > Simon, I'll work with tou on the refactoring you wanted and > > remove the EFI requirement of SMBIOS in !x86 platforms. > > Since this code has no tests, I'll add some once the refactoring > > is done > > > > Changes since v2: > > - Spelling mistakes > > - rebase on top of patch #1 changes > > Changes since v1: > > - Tokenize the DT node entry and use the appropriate value instead of > > the entire string > > - Removed Peters tested/reviewed-by tags due to the above > > lib/smbios.c | 94 +--- > > 1 file changed, 89 insertions(+), 5 deletions(-) > > Can we add a Kconfig to enable this? Why? This is the default option that we want moving forward in order to get the fields populated. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 1/3] drivers: rtc: add pcf2131 rtc driver
Hi Joy, In Linux, the pcf2131 rtc is handled by the pcf2127 driver. I suggest we do the same in U-Boot. Thanks
