Re: [PATCH] Add support for more XMC series

2023-08-14 Thread Clus Tom
Hi Simon,
I just want to apply:
https://patchwork.ozlabs.org/project/uboot/patch/20230812031846.3958-1-ssunk...@gmail.com

Thanks,
SSunk

Simon Glass  于2023年8月15日周二 06:43写道:

> Hi,
>
> On Sun, 13 Aug 2023 at 21:19, Clus Tom  wrote:
> >
> > Hi Simon,
> > I'm not quite sure what you mean by v2, if it's the previous email, it
> only removes the XM25QH128C part of the commit message compared to the
> previous one.
> >
>
> Right, so it is v2.
>
> Your emails seem to have generated 3 patches:
>
>
> https://patchwork.ozlabs.org/project/uboot/patch/20230811082000.4277-1-ssunk...@gmail.com/
>
>
> https://patchwork.ozlabs.org/project/uboot/patch/20230812031846.3958-1-ssunk...@gmail.com/
>
>
> https://patchwork.ozlabs.org/project/uboot/patch/20230812030731.3711-1-ssunk...@gmail.com/
>
> The version number helps the maintainer figure out which one to apply.
>
> Regards,
> Simon
>
>
>
> - Simon
>
>
> > Thanks,
> > SSunk
> >
> > Simon Glass  于2023年8月13日周日 21:36写道:
> >>
> >> On Fri, 11 Aug 2023 at 21:19, SSunk  wrote:
> >> >
> >> > Add XMC XM25QH256C/XM25QU256C/XM25QH512C/XM25QU512C
> >> > site: https://www.xmcwh.com/site/product
> >> >
> >> > Signed-off-by: Kankan Sun 
> >> > ---
> >> >  configs/evb-ast2600_defconfig | 1 +
> >> >  drivers/mtd/spi/spi-nor-ids.c | 4 
> >> >  2 files changed, 5 insertions(+)
> >>
> >> Reviewed-by: Simon Glass 
> >>
> >> I think this is v2 so it should have that as well as a change list.
> >> You can use 'patman' to help with this.
> >>
> >> Regards,
> >> Simon
> >>
> >>
> >>
> >> >
> >> > diff --git a/configs/evb-ast2600_defconfig
> b/configs/evb-ast2600_defconfig
> >> > index 9244654c82..f06c0e1fe1 100644
> >> > --- a/configs/evb-ast2600_defconfig
> >> > +++ b/configs/evb-ast2600_defconfig
> >> > @@ -100,6 +100,7 @@ CONFIG_SPI_FLASH_SPANSION=y
> >> >  CONFIG_SPI_FLASH_STMICRO=y
> >> >  CONFIG_SPI_FLASH_SST=y
> >> >  CONFIG_SPI_FLASH_WINBOND=y
> >> > +CONFIG_SPI_FLASH_XMC=y
> >> >  # CONFIG_SPI_FLASH_USE_4K_SECTORS is not set
> >> >  CONFIG_PHY_REALTEK=y
> >> >  CONFIG_PHY_NCSI=y
> >> > diff --git a/drivers/mtd/spi/spi-nor-ids.c
> b/drivers/mtd/spi/spi-nor-ids.c
> >> > index 4587215984..80d7678293 100644
> >> > --- a/drivers/mtd/spi/spi-nor-ids.c
> >> > +++ b/drivers/mtd/spi/spi-nor-ids.c
> >> > @@ -531,6 +531,10 @@ const struct flash_info spi_nor_ids[] = {
> >> > { INFO("XM25QH64A", 0x207017, 0, 64 * 1024, 128, SECT_4K |
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> >> > { INFO("XM25QH64C", 0x204017, 0, 64 * 1024, 128, SECT_4K |
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> >> > { INFO("XM25QH128A", 0x207018, 0, 64 * 1024, 256, SECT_4K |
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> >> > +   { INFO("XM25QH256C", 0x204019, 0, 64 * 1024, 512, SECT_4K |
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
> >> > +   { INFO("XM25QU256C", 0x204119, 0, 64 * 1024, 512, SECT_4K |
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
> >> > +   { INFO("XM25QH512C", 0x204020, 0, 64 * 1024, 1024, SECT_4K |
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
> >> > +   { INFO("XM25QU512C", 0x204120, 0, 64 * 1024, 1024, SECT_4K |
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
> >> >  #endif
> >> >  #ifdef CONFIG_SPI_FLASH_XTX
> >> > /* XTX Technology Limited */
> >> > --
> >> > 2.34.1
> >> >
>


RE: [PATCH] Add support for XMC XM25QH128C/XM25QH256C/XM25QU256C/XM25QH512C/XM25QU512C

2023-08-14 Thread Chin-Ting Kuo
> -Original Message-
> From: SSunk 
> Sent: Friday, August 11, 2023 4:20 PM
> Subject: [PATCH] Add support for XMC
> XM25QH128C/XM25QH256C/XM25QU256C/XM25QH512C/XM25QU512C
> 
> site: https://www.xmcwh.com/site/product
> 
> Signed-off-by: Kankan Sun 
> ---
>  configs/evb-ast2600_defconfig | 1 +
>  drivers/mtd/spi/spi-nor-ids.c | 4 
>  2 files changed, 5 insertions(+)
> 
> diff --git a/configs/evb-ast2600_defconfig b/configs/evb-ast2600_defconfig
> index 9244654c82..f06c0e1fe1 100644
> --- a/configs/evb-ast2600_defconfig
> +++ b/configs/evb-ast2600_defconfig
> @@ -100,6 +100,7 @@ CONFIG_SPI_FLASH_SPANSION=y
> CONFIG_SPI_FLASH_STMICRO=y  CONFIG_SPI_FLASH_SST=y
> CONFIG_SPI_FLASH_WINBOND=y
> +CONFIG_SPI_FLASH_XMC=y
>  # CONFIG_SPI_FLASH_USE_4K_SECTORS is not set  CONFIG_PHY_REALTEK=y
> CONFIG_PHY_NCSI=y diff --git a/drivers/mtd/spi/spi-nor-ids.c
> b/drivers/mtd/spi/spi-nor-ids.c index 4587215984..80d7678293 100644
> --- a/drivers/mtd/spi/spi-nor-ids.c
> +++ b/drivers/mtd/spi/spi-nor-ids.c
> @@ -531,6 +531,10 @@ const struct flash_info spi_nor_ids[] = {
>   { INFO("XM25QH64A", 0x207017, 0, 64 * 1024, 128, SECT_4K |
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>   { INFO("XM25QH64C", 0x204017, 0, 64 * 1024, 128, SECT_4K |
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>   { INFO("XM25QH128A", 0x207018, 0, 64 * 1024, 256, SECT_4K |
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> + { INFO("XM25QH256C", 0x204019, 0, 64 * 1024, 512, SECT_4K |
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
> + { INFO("XM25QU256C", 0x204119, 0, 64 * 1024, 512, SECT_4K |
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
> + { INFO("XM25QH512C", 0x204020, 0, 64 * 1024, 1024, SECT_4K |
> SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
> + { INFO("XM25QU512C", 0x204120, 0, 64 * 1024, 1024, SECT_4K |
> +SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
>  #endif
>  #ifdef CONFIG_SPI_FLASH_XTX
>   /* XTX Technology Limited */
> --
> 2.34.1

Reviewed-by: Chin-Ting Kuo 




Re: [PATCH 8/8] disk: Make blk_get_ops() internal to blk uclass

2023-08-14 Thread AKASHI Takahiro
On Mon, Aug 14, 2023 at 04:42:57PM -0600, Simon Glass wrote:
> On Sun, 13 Aug 2023 at 17:47, Marek Vasut
>  wrote:
> >
> > Move the macro into blk-uclass.c , since it is only used there.
> >
> > Signed-off-by: Marek Vasut 
> > ---
> > Cc: AKASHI Takahiro 
> > Cc: Abdellatif El Khlifi 
> > Cc: Bin Meng 
> > Cc: Heinrich Schuchardt 
> > Cc: Joshua Watt 
> > Cc: Michal Suchanek 
> > Cc: Simon Glass 
> > Cc: Tobias Waldekranz 
> > ---
> >  drivers/block/blk-uclass.c | 2 ++
> >  include/blk.h  | 2 --
> >  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> Reviewed-by: Simon Glass 
> 
> Unfortunately this does not stop people using the ops member directly.
> 
> For this series, I tried a patch myself [1] but I think I stuffed it
> up. So I will let Takahiro-San figure it out. I would very much like
> to see this clean-up go in.

At that time I thought that the necessary change was small and trivial:)

As for Marek's patch, let me first explain why I implement that way,
i.e. separating disk_blk_*() from part_disk_*():
- Initially I tried to implement disk_blk_*() work for both UCLASS_BLOCK
  and UCLASS_PARTITION, while this idea was rejected by Simon.
- Then, I implemented part_disk_*() with direct access to the devices,
  and part_disk_*(), as helper functions, with block caching.
  I thought that this approach was aligned with the implementation of
  block devices (blk_[read|wirte]).

If you don't think the second point makes sense, I can agree to Marek's
approach.

Thanks,
-Takahiro Akashi


> Regards,
> Simon
> 
> [1] 
> https://patchwork.ozlabs.org/project/uboot/patch/20230319192957.1084530-1-...@chromium.org/


Re: [PATCH v9 08/10] sandbox: capsule: Generate capsule related files through binman

2023-08-14 Thread Simon Glass
Hi Sughosh,

On Mon, 14 Aug 2023 at 02:17, Sughosh Ganu  wrote:
>
> hi Simon,
>
> On Sat, 12 Aug 2023 at 22:33, Simon Glass  wrote:
> >
> > Hi Sughosh,
> >
> > On Sat, 12 Aug 2023 at 09:40, Sughosh Ganu  wrote:
> > >
> > > hi Simon,
> > >
> > > On Sat, 12 Aug 2023 at 19:55, Simon Glass  wrote:
> > > >
> > > > On Fri, 11 Aug 2023 at 23:58, Sughosh Ganu  
> > > > wrote:
> > > > >
> > > > > The EFI capsule files can now be generated as part of u-boot
> > > > > build through binman. Add capsule entry nodes for the sandbox
> > > > > architecture for generating the capsules. These capsules are then used
> > > > > for testing the EFI capsule update functionality on the sandbox
> > > > > platforms.
> > > > >
> > > > > Remove the corresponding logic in the test setup which was used for
> > > > > generation of these capsule which is now superfluous.
> > > > >
> > > > > Signed-off-by: Sughosh Ganu 
> > > > > ---
> > > > > Changes since V8:
> > > > > * Build the capsules for all sandbox variants.
> > > > > * Remove the type property by renaming the capsule nodes as
> > > > >   'efi-capsule'.
> > > > >
> > > > >  arch/sandbox/dts/sandbox.dts  |   2 +
> > > > >  arch/sandbox/dts/sandbox_capsule.dtsi | 315 
> > > > > ++
> > > > >  arch/sandbox/dts/test.dts |   2 +
> > > > >  include/sandbox_efi_capsule.h |  21 ++
> > > > >  test/py/tests/test_efi_capsule/conftest.py| 155 +
> > > > >  .../tests/test_efi_capsule/uboot_bin_env.its  |  36 --
> > > > >  6 files changed, 356 insertions(+), 175 deletions(-)
> > > > >  create mode 100644 arch/sandbox/dts/sandbox_capsule.dtsi
> > > > >  create mode 100644 include/sandbox_efi_capsule.h
> > > > >  delete mode 100644 test/py/tests/test_efi_capsule/uboot_bin_env.its
> > > >
> > > > Reviewed-by: Simon Glass 
> > > >
> > > > One way to reduce the files is to put all the capsules in the same
> > > > binman image. You could have 'size = <0x1000>' so they are all the
> > > > same size.
> > > >
> > > > Then your test could check each one by iterating through the (single) 
> > > > file.
> > >
> > > This won't work, as the UEFI specification defines a particular format
> > > of the capsule file. For multiple capsules, the individual capsule
> > > files need to be present under the /EFI/UpdateCapsule/ directory on
> > > the EFI System Partition. The spec does define a format in which a
> > > single capsule file can have multiple payloads(input binaries). This
> > > is planned to be supported once support has been added for generating
> > > capsules through the config file.
> >
> > We continue to have challenges getting confused between testing and real 
> > world.
> >
> > For testing, we can do all sorts of things to make things simpler. We
> > don't even need an EFI partition, etc. We can have a test which checks
> > that U-Boot can decode an update and apply it, The current tests rely
> > far too much on an end-to-end, black-box approach, as I might have
> > mentioned.
>
> Okay, but I am stating the constraints on using a single combined file
> based on the way the tests work today. So if we were to generate a
> single file consisting of all the individual capsule files, that would
> break the tests. That was my point.

Sure...my point is that the tests could use a bit of rework to make
them easier and faster to run.

Regards,
Simon


Please pull u-boot-dm/next

2023-08-14 Thread Simon Glass
Hi Tom,

This is for the -next branch


The following changes since commit 321d7b4d875a77552a969dd6ea5bbed2644fcb0c:

  Merge branch '2023-08-09-misc-cleanups' into next (2023-08-09 13:15:51 -0400)

are available in the Git repository at:

  git://git.denx.de/u-boot-dm.git tags/dm-next-14aug23

for you to fetch changes up to daffb0be2c839f3abe431cd68c772fae0e7e49ca:

  bootstd: cros: Add ARM support (2023-08-11 07:33:40 -0600)


Enhance bootmeth_cros


Simon Glass (16):
  bootstd: cros: Correct reporting of I/O errors
  bootstd: cros: Move partition reading into a function
  bootstd: cros: Bring in some ChromiumOS structures
  bootstd: cros: Support a kernel on either partition
  bootstd: cros: Decode some kernel preamble fields
  bootstd: cros: Simplify setup and cmdline expressions
  bootstd: Move common zimage functions to bootm.h
  bootstd: cros: Add docs for the kernel layout
  bootstd: cros: Add private info for ChromiumOS
  bootstd: Add private bootmeth data to the bootflow
  bootstd: cros: Add a function to read info from partition
  bootstd: cros: Add a function to read a kernel
  bootstd: cros: Split up reading info and kernel
  bootstd: Allow display of the x86 setup information
  bootstd: Add a command to read all files for a bootflow
  bootstd: cros: Add ARM support

 arch/x86/include/asm/zimage.h |  37 -
 arch/x86/lib/zimage.c |   8 +-
 boot/Kconfig  |   4 +-
 boot/bootflow.c   |  17 ++
 boot/bootm.c  |  37 +
 boot/bootmeth-uclass.c|  12 ++
 boot/bootmeth_cros.c  | 367 +++---
 boot/bootmeth_cros.h  | 197 +++
 cmd/bootflow.c|  47 +-
 doc/usage/cmd/bootflow.rst| 139 +++-
 include/bootflow.h|  15 +-
 include/bootm.h   |  47 ++
 include/bootmeth.h|  25 ++-
 13 files changed, 845 insertions(+), 107 deletions(-)
 create mode 100644 boot/bootmeth_cros.h

Regards,
Simon


Re: [PATCH 2/2] configs: sandbox: test: dm: blk: Enable NVMXIP QSPI and update test

2023-08-14 Thread Marek Vasut

On 8/15/23 00:42, Simon Glass wrote:

On Sun, 13 Aug 2023 at 15:47, Marek Vasut
 wrote:


Enable NVMXIP QSPI driver on sandbox, since it is already enabled
on sandbox64. Update blk tests to match.

Signed-off-by: Marek Vasut 
---
Cc: Abdellatif El Khlifi 
Cc: Simon Glass 
---
  configs/sandbox_defconfig |  1 +
  test/dm/blk.c | 63 +++
  2 files changed, 52 insertions(+), 12 deletions(-)



Reviewed-by: Simon Glass 


NAK, this one has to wait. All of this NVMXIP stuff needs closer look, I 
think it is really weirdly broken and doesn't even pass sandbox and 
sandbox64 tests. Can Abdellatif try and check whether ALL sandbox and 
sandbox64 tests pass with this enabled ?


Re: [PATCH 8/8] disk: Make blk_get_ops() internal to blk uclass

2023-08-14 Thread Simon Glass
On Sun, 13 Aug 2023 at 17:47, Marek Vasut
 wrote:
>
> Move the macro into blk-uclass.c , since it is only used there.
>
> Signed-off-by: Marek Vasut 
> ---
> Cc: AKASHI Takahiro 
> Cc: Abdellatif El Khlifi 
> Cc: Bin Meng 
> Cc: Heinrich Schuchardt 
> Cc: Joshua Watt 
> Cc: Michal Suchanek 
> Cc: Simon Glass 
> Cc: Tobias Waldekranz 
> ---
>  drivers/block/blk-uclass.c | 2 ++
>  include/blk.h  | 2 --
>  2 files changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass 

Unfortunately this does not stop people using the ops member directly.

For this series, I tried a patch myself [1] but I think I stuffed it
up. So I will let Takahiro-San figure it out. I would very much like
to see this clean-up go in.

Regards,
Simon

[1] 
https://patchwork.ozlabs.org/project/uboot/patch/20230319192957.1084530-1-...@chromium.org/


Re: [PATCH 1/2] blk: Add bounce buffer support to read/write operations

2023-08-14 Thread Simon Glass
Hi Marek,

On Sun, 13 Aug 2023 at 17:50, Marek Vasut
 wrote:
>
> Some devices have limited DMA capabilities and require that the
> buffers passed to them fit specific properties. Add new optional
> callback which can be used at driver level to indicate whether a
> buffer alignment is suitable for the device DMA or not, and
> trigger use of generic bounce buffer implementation to help use
> of unsuitable buffers at the expense of performance degradation.
>
> Signed-off-by: Marek Vasut 
> ---
> Cc: Abdellatif El Khlifi 
> Cc: Bin Meng 
> Cc: Heinrich Schuchardt 
> Cc: Mattijs Korpershoek 
> Cc: Michal Suchanek 
> Cc: Simon Glass 
> Cc: Tobias Waldekranz 
> ---
>  drivers/block/blk-uclass.c | 62 --
>  include/blk.h  | 19 
>  2 files changed, 79 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c
> index 6aac92d9962..885513893f6 100644
> --- a/drivers/block/blk-uclass.c
> +++ b/drivers/block/blk-uclass.c
> @@ -446,6 +446,26 @@ int blk_get_device(int uclass_id, int devnum, struct 
> udevice **devp)
> return device_probe(*devp);
>  }
>
> +struct blk_bounce_buffer {
> +   struct udevice  *dev;
> +   struct bounce_bufferstate;
> +};
> +
> +static int blk_buffer_aligned(struct bounce_buffer *state)
> +{
> +#if IS_ENABLED(CONFIG_BOUNCE_BUFFER)

I think it is worth adding an accessor in the header file to allow you
to convert this #if to if

> +   struct blk_bounce_buffer *bbstate =
> +   container_of(state, struct blk_bounce_buffer, state);
> +   struct udevice *dev = bbstate->dev;
> +   const struct blk_ops *ops = blk_get_ops(dev);
> +
> +   if (ops->buffer_aligned)
> +   return ops->buffer_aligned(dev, state);
> +#endif /* CONFIG_BOUNCE_BUFFER */
> +
> +   return 1;   /* Default, any buffer is OK */
> +}
> +
>  long blk_read(struct udevice *dev, lbaint_t start, lbaint_t blkcnt, void 
> *buf)
>  {
> struct blk_desc *desc = dev_get_uclass_plat(dev);
> @@ -458,7 +478,25 @@ long blk_read(struct udevice *dev, lbaint_t start, 
> lbaint_t blkcnt, void *buf)
> if (blkcache_read(desc->uclass_id, desc->devnum,
>   start, blkcnt, desc->blksz, buf))
> return blkcnt;
> -   blks_read = ops->read(dev, start, blkcnt, buf);
> +
> +   if (IS_ENABLED(CONFIG_BOUNCE_BUFFER)) {
> +   struct blk_bounce_buffer bbstate = { .dev = dev };
> +   int ret;
> +
> +   ret = bounce_buffer_start_extalign(, buf,
> +  blkcnt * desc->blksz,
> +  GEN_BB_WRITE, desc->blksz,
> +  blk_buffer_aligned);
> +   if (ret)
> +   return ret;
> +
> +   blks_read = ops->read(dev, start, blkcnt, 
> bbstate.state.bounce_buffer);
> +
> +   bounce_buffer_stop();
> +   } else {
> +   blks_read = ops->read(dev, start, blkcnt, buf);
> +   }
> +
> if (blks_read == blkcnt)
> blkcache_fill(desc->uclass_id, desc->devnum, start, blkcnt,
>   desc->blksz, buf);
> @@ -471,13 +509,33 @@ long blk_write(struct udevice *dev, lbaint_t start, 
> lbaint_t blkcnt,
>  {
> struct blk_desc *desc = dev_get_uclass_plat(dev);
> const struct blk_ops *ops = blk_get_ops(dev);
> +   long blks_written;
>
> if (!ops->write)
> return -ENOSYS;
>
> blkcache_invalidate(desc->uclass_id, desc->devnum);
>
> -   return ops->write(dev, start, blkcnt, buf);
> +   if (IS_ENABLED(CONFIG_BOUNCE_BUFFER)) {
> +   struct blk_bounce_buffer bbstate = { .dev = dev };
> +   int ret;
> +
> +   ret = bounce_buffer_start_extalign(, (void 
> *)buf,
> +  blkcnt * desc->blksz,
> +  GEN_BB_READ, desc->blksz,
> +  blk_buffer_aligned);
> +   if (ret)
> +   return ret;
> +
> +   blks_written = ops->write(dev, start, blkcnt,
> + bbstate.state.bounce_buffer);
> +
> +   bounce_buffer_stop();
> +   } else {
> +   blks_written = ops->write(dev, start, blkcnt, buf);
> +   }
> +
> +   return blks_written;
>  }
>
>  long blk_erase(struct udevice *dev, lbaint_t start, lbaint_t blkcnt)
> diff --git a/include/blk.h b/include/blk.h
> index 8986e953e5a..b819f97c2f1 100644
> --- a/include/blk.h
> +++ b/include/blk.h
> @@ -7,6 +7,7 @@
>  #ifndef BLK_H
>  #define BLK_H
>
> +#include 
>  #include 
>  #include 
>
> @@ -260,6 +261,24 @@ struct blk_ops {
>  * @return 0 if OK, -ve on error
>  */
> int 

Re: [PATCH v2 4/4] doc: qemu: arm: Add a section on booting Linux distros

2023-08-14 Thread Simon Glass
Hi Alper,

On Mon, 14 Aug 2023 at 11:40, Alper Nebi Yasak  wrote:
>
> Add an example qemu-system-aarch64 command that can make U-Boot on QEMU
> boot into the Debian Installer, along with resulting console messages
> from U-Boot, based on the existing documentation section for the x86
> version.
>
> Signed-off-by: Alper Nebi Yasak 
> ---
> I actually want to put the root.img device first so that the VM can boot
> into the installed system when it reboots, but U-Boot can't find the
> bootflow on the second drive. I tried e.g. `bootdev list -p; bootflow
> scan -lab`, but it seems to only ever search the first virtio-blk.
> However, `eficonfig; bootefi bootmgr` makes it boot somehow.

Yes, this is annoying.

The problem is that the scanning is getting so complicated. The
boot_targets var lists things which can either be a uclass, or a
uclass with a device number. The currently implementation sees
"virtio" and moves on to the next thing in boot_targets once it has
processed one virtio device. That is obviously not correct, but...

Would it be possible to just drop the 'boot_targets' var? That is what
is stuffing it up, but we don't actually need it now. The defaults end
up doing the same thing, apart (perhaps) from the strangeness of
virtio which can be both a network and a blk device.

I believe it is possible to do the right thing, but I'll need to
create a better test mechanism to handle all the cases.

>
> Changes in v2:
> - Add new patch for doc section on booting Linux distros
>
>  doc/board/emulation/qemu-arm.rst | 68 
>  1 file changed, 68 insertions(+)
>

Regards,
Simon


Re: [PATCH v3 5/9] board_f: Fix corruption of relocaddr

2023-08-14 Thread Simon Glass
Hi Devarsh, Nikhil, Tom,

On Wed, 9 Aug 2023 at 09:29, Bin Meng  wrote:
>
> On Thu, Aug 3, 2023 at 7:03 PM Bin Meng  wrote:
> >
> > On Thu, Aug 3, 2023 at 6:37 PM Bin Meng  wrote:
> > >
> > > On Tue, Aug 1, 2023 at 12:00 AM Simon Glass  wrote:
> > > >
> > > > When the video framebuffer comes from the bloblist, we should not change
> > > > relocaddr to this address, since it interfers with the normal memory
> > >
> > > typo: interferes
> > >
> > > > allocation.
> > > >
> > > > This fixes a boot loop in qemu-x86_64
> > > >
> > > > Signed-off-by: Simon Glass 
> > > > Fixes: 5bc610a7d9d ("common: board_f: Pass frame buffer info from SPL 
> > > > to u-boot")
> > > > Suggested-by: Nikhil M Jain 
> > > > ---
> > > >
> > > > Changes in v3:
> > > > - Reword the Kconfig help as suggested
> > > >
> > > > Changes in v2:
> > > > - Add a Kconfig as the suggested conditional did not work
> > > >
> > > >  common/board_f.c  | 3 ++-
> > > >  drivers/video/Kconfig | 9 +
> > > >  2 files changed, 11 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/common/board_f.c b/common/board_f.c
> > > > index 7d2c380e91e..5173d0a0c2d 100644
> > > > --- a/common/board_f.c
> > > > +++ b/common/board_f.c
> > > > @@ -419,7 +419,8 @@ static int reserve_video(void)
> > > > if (!ho)
> > > > return log_msg_ret("blf", -ENOENT);
> > > > video_reserve_from_bloblist(ho);
> > > > -   gd->relocaddr = ho->fb;
> > > > +   if (IS_ENABLED(CONFIG_VIDEO_RESERVE_SPL))
> > > > +   gd->relocaddr = ho->fb;
> > > > } else if (CONFIG_IS_ENABLED(VIDEO)) {
> > > > ulong addr;
> > > > int ret;
> > > > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> > > > index b41dc60cec5..f2e56204d52 100644
> > > > --- a/drivers/video/Kconfig
> > > > +++ b/drivers/video/Kconfig
> > > > @@ -1106,6 +1106,15 @@ config SPL_VIDEO_REMOVE
> > > >   if this  option is enabled video driver will be removed at 
> > > > the end of
> > > >   SPL stage, beforeloading the next stage.
> > > >
> > > > +config VIDEO_RESERVE_SPL
> > > > +   bool
> > > > +   help
> > > > + This adjusts reserve_video() to redirect memory reservation 
> > > > when it
> > > > + sees a video handoff blob (BLOBLISTT_U_BOOT_VIDEO). This 
> > > > avoids the
> > > > + memory used for framebuffer from being allocated by U-Boot 
> > > > proper,
> > > > + thus preventing any further memory reservations done by 
> > > > U-Boot proper
> > > > + from overwriting the framebuffer.
> > > > +
> > > >  if SPL_SPLASH_SCREEN
> > > >
> > > >  config SPL_SPLASH_SCREEN_ALIGN
> > >
> > > Reviewed-by: Bin Meng 
> >
> > applied to u-boot-x86, thanks!
>
> Dropped this one from the x86 queue per the discussion.

I just wanted to come back to this discussion.

Do we have an agreed way forward? Who is waiting for who?

Regards,
Simon


Re: [PATCH 2/2] configs: sandbox: test: dm: blk: Enable NVMXIP QSPI and update test

2023-08-14 Thread Simon Glass
On Sun, 13 Aug 2023 at 15:47, Marek Vasut
 wrote:
>
> Enable NVMXIP QSPI driver on sandbox, since it is already enabled
> on sandbox64. Update blk tests to match.
>
> Signed-off-by: Marek Vasut 
> ---
> Cc: Abdellatif El Khlifi 
> Cc: Simon Glass 
> ---
>  configs/sandbox_defconfig |  1 +
>  test/dm/blk.c | 63 +++
>  2 files changed, 52 insertions(+), 12 deletions(-)
>

Reviewed-by: Simon Glass 


Re: [PATCH] test: cpu: Handle both 32bit and 64bit CPUs

2023-08-14 Thread Simon Glass
On Sun, 13 Aug 2023 at 13:52, Marek Vasut
 wrote:
>
> Handle both 32bit and 64bit systems, i.e. sandbox and sandbox64
> the same way drivers/cpu/cpu_sandbox.c does, that is in case
> CONFIG_PHYS_64BIT is enabled, assume 64bit address width, else
> assume 32bit address width. This fixes ut_dm_dm_test_cpu test
> failure on sandbox64.
>
> Signed-off-by: Marek Vasut 
> ---
> Cc: Simon Glass 
> ---
>  test/dm/cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass 


Re: [PATCH] disk: dos: Infer MBR partition sector size from underlying drive sector size

2023-08-14 Thread Simon Glass
On Sun, 13 Aug 2023 at 17:49, Marek Vasut
 wrote:
>
> Block devices with 4k sectors imply the MBR sectors are also 4k instead
> of regular 512B. Avoid hard-coding the 512B sector size and isntead read
> the current block device sector size from it, and if the sector size is
> larger than 512B, use the block device sector size.
>
> Signed-off-by: Marek Vasut 
> ---
> Cc: Simon Glass 
> ---
>  disk/part_dos.c | 17 ++---
>  1 file changed, 14 insertions(+), 3 deletions(-)

Reviewed-by: Simon Glass 


Re: [PATCH 1/1] doc: description of board_get_usable_ram_top()

2023-08-14 Thread Simon Glass
On Mon, 14 Aug 2023 at 00:47, Heinrich Schuchardt
 wrote:
>
> Improve the description of function board_get_usable_ram_top().
>
> Signed-off-by: Heinrich Schuchardt 
> ---
>  include/init.h | 15 ++-
>  1 file changed, 10 insertions(+), 5 deletions(-)
>

Reviewed-by: Simon Glass 


Re: [PATCH] Add support for more XMC series

2023-08-14 Thread Simon Glass
Hi,

On Sun, 13 Aug 2023 at 21:19, Clus Tom  wrote:
>
> Hi Simon,
> I'm not quite sure what you mean by v2, if it's the previous email, it only 
> removes the XM25QH128C part of the commit message compared to the previous 
> one.
>

Right, so it is v2.

Your emails seem to have generated 3 patches:

https://patchwork.ozlabs.org/project/uboot/patch/20230811082000.4277-1-ssunk...@gmail.com/

https://patchwork.ozlabs.org/project/uboot/patch/20230812031846.3958-1-ssunk...@gmail.com/

https://patchwork.ozlabs.org/project/uboot/patch/20230812030731.3711-1-ssunk...@gmail.com/

The version number helps the maintainer figure out which one to apply.

Regards,
Simon



- Simon


> Thanks,
> SSunk
>
> Simon Glass  于2023年8月13日周日 21:36写道:
>>
>> On Fri, 11 Aug 2023 at 21:19, SSunk  wrote:
>> >
>> > Add XMC XM25QH256C/XM25QU256C/XM25QH512C/XM25QU512C
>> > site: https://www.xmcwh.com/site/product
>> >
>> > Signed-off-by: Kankan Sun 
>> > ---
>> >  configs/evb-ast2600_defconfig | 1 +
>> >  drivers/mtd/spi/spi-nor-ids.c | 4 
>> >  2 files changed, 5 insertions(+)
>>
>> Reviewed-by: Simon Glass 
>>
>> I think this is v2 so it should have that as well as a change list.
>> You can use 'patman' to help with this.
>>
>> Regards,
>> Simon
>>
>>
>>
>> >
>> > diff --git a/configs/evb-ast2600_defconfig b/configs/evb-ast2600_defconfig
>> > index 9244654c82..f06c0e1fe1 100644
>> > --- a/configs/evb-ast2600_defconfig
>> > +++ b/configs/evb-ast2600_defconfig
>> > @@ -100,6 +100,7 @@ CONFIG_SPI_FLASH_SPANSION=y
>> >  CONFIG_SPI_FLASH_STMICRO=y
>> >  CONFIG_SPI_FLASH_SST=y
>> >  CONFIG_SPI_FLASH_WINBOND=y
>> > +CONFIG_SPI_FLASH_XMC=y
>> >  # CONFIG_SPI_FLASH_USE_4K_SECTORS is not set
>> >  CONFIG_PHY_REALTEK=y
>> >  CONFIG_PHY_NCSI=y
>> > diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
>> > index 4587215984..80d7678293 100644
>> > --- a/drivers/mtd/spi/spi-nor-ids.c
>> > +++ b/drivers/mtd/spi/spi-nor-ids.c
>> > @@ -531,6 +531,10 @@ const struct flash_info spi_nor_ids[] = {
>> > { INFO("XM25QH64A", 0x207017, 0, 64 * 1024, 128, SECT_4K | 
>> > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>> > { INFO("XM25QH64C", 0x204017, 0, 64 * 1024, 128, SECT_4K | 
>> > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>> > { INFO("XM25QH128A", 0x207018, 0, 64 * 1024, 256, SECT_4K | 
>> > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>> > +   { INFO("XM25QH256C", 0x204019, 0, 64 * 1024, 512, SECT_4K | 
>> > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
>> > +   { INFO("XM25QU256C", 0x204119, 0, 64 * 1024, 512, SECT_4K | 
>> > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
>> > +   { INFO("XM25QH512C", 0x204020, 0, 64 * 1024, 1024, SECT_4K | 
>> > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
>> > +   { INFO("XM25QU512C", 0x204120, 0, 64 * 1024, 1024, SECT_4K | 
>> > SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
>> >  #endif
>> >  #ifdef CONFIG_SPI_FLASH_XTX
>> > /* XTX Technology Limited */
>> > --
>> > 2.34.1
>> >


Re: [PATCH] configs: sandbox64: Enable clock CCF driver

2023-08-14 Thread Simon Glass
On Sun, 13 Aug 2023 at 13:50, Marek Vasut
 wrote:
>
> Align the sandbox64 defconfig with sandbox defconfig. Enable missing
> CCF and Sandbox CCF drivers. This fixes ut_dm_dm_test_clk_ccf test .
>
> Signed-off-by: Marek Vasut 
> ---
> Cc: Simon Glass 
> ---
>  configs/sandbox64_defconfig | 2 ++
>  1 file changed, 2 insertions(+)
>

Reviewed-by: Simon Glass 


Re: [PATCH] configs: sandbox64: Enable PCI register multi-entry support

2023-08-14 Thread Simon Glass
On Sun, 13 Aug 2023 at 13:52, Marek Vasut
 wrote:
>
> Align the sandbox64 defconfig with sandbox defconfig. Enable missing
> PCI register multi-entry support. This fixes ut_dm_dm_test_pci_bus_to_phys
> test .
>
> Signed-off-by: Marek Vasut 
> ---
> Cc: Simon Glass 
> ---
>  configs/sandbox64_defconfig | 1 +
>  1 file changed, 1 insertion(+)
>

Reviewed-by: Simon Glass 


Re: [PATCH] x86: Update cbmem driver

2023-08-14 Thread Simon Glass
Hi Alex,

On Sun, 13 Aug 2023 at 21:03, Alex Sadovsky
 wrote:
>
> Dear Simon and other developers,
> > - cursor = cbmem_console_p->buffer_cursor++;
> > - if (cursor < cbmem_console_p->buffer_size)
> > - cbmem_console_p->buffer_body[cursor] = data;
> > + pos = cons->cursor++;
> > + if (pos < cons->size)
> > + cons->body[pos] = data;
> While at it, is it OK to increment cons->cursor unconditionally,
> even when the buffer is full?
>
> It's better to do it after the check, isn't it? E.g.:
>
> if (cons->cursor < cons->size)
> cons->body[cons->cursor++] = data;

I believe the original intent was to indicate that the buffer had
overflowed. But prompted by your review I took a look at the coreboot
implementation and it now has an overflow flag.

So I will send a v2 incorporating this.

Thanks,
Simon


[PATCH 19/19] expo: doc: Update documentation for persistent settings

2023-08-14 Thread Simon Glass
Add mention of persistent settings in the documentation.

Signed-off-by: Simon Glass 
---

 doc/develop/cedit.rst | 15 +++
 doc/develop/expo.rst  |  1 -
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/doc/develop/cedit.rst b/doc/develop/cedit.rst
index 8f0a554ae918..63dff9d3f140 100644
--- a/doc/develop/cedit.rst
+++ b/doc/develop/cedit.rst
@@ -152,3 +152,18 @@ Themes
 
 The configuration editor uses simple expo themes. The theme is read from
 `/bootstd/cedit-theme` in the devicetree.
+
+
+Reading and writing settings
+
+
+Cedit provides several options for persistent settings:
+
+- Writing an FDT file to a filesystem
+- Writing to U-Boot's environment variables, which are then typically stored in
+  a persistent manner
+- Writing to CMOS RAM registers (common on x86 machines)
+
+For now, reading and writing settings is not automatic. See the
+:doc:`../usage/cmd/cedit` for how to do this on the command line or in a
+script.
diff --git a/doc/develop/expo.rst b/doc/develop/expo.rst
index 61b6855c72f4..f13761995d3f 100644
--- a/doc/develop/expo.rst
+++ b/doc/develop/expo.rst
@@ -485,7 +485,6 @@ Some ideas for future work:
 - Support unicode
 - Support curses for proper serial-terminal menus
 - Add support for large menus which need to scroll
-- Add support for reading and writing configuration settings with cedit
 - Update expo.py tool to check for overlapping names and CMOS locations
 
 .. Simon Glass 
-- 
2.41.0.694.ge786442a9b-goog



[PATCH 18/19] expo: cedit: Support reading settings from CMOS RAM

2023-08-14 Thread Simon Glass
Add a command to read edit settings from CMOS RAM, using the cedit
definition to indicate which registers and bits are used.

Signed-off-by: Simon Glass 
---

 boot/cedit.c| 99 +
 boot/scene_internal.h   | 12 +
 boot/scene_menu.c   | 16 +++
 cmd/cedit.c | 36 +++
 doc/usage/cmd/cedit.rst |  9 +++-
 include/cedit.h | 12 +
 test/boot/cedit.c   | 11 +
 7 files changed, 194 insertions(+), 1 deletion(-)

diff --git a/boot/cedit.c b/boot/cedit.c
index 725745aba55d..73645f70b6cb 100644
--- a/boot/cedit.c
+++ b/boot/cedit.c
@@ -39,6 +39,7 @@ enum {
  * @mask: Mask bits for the CMOS RAM. If a bit is set the byte containing it
  * will be written
  * @value: Value bits for CMOS RAM. This is the actual value written
+ * @dev: RTC device to write to
  */
 struct cedit_iter_priv {
struct abuf *buf;
@@ -46,6 +47,7 @@ struct cedit_iter_priv {
bool verbose;
u8 *mask;
u8 *value;
+   struct udevice *dev;
 };
 
 int cedit_arange(struct expo *exp, struct video_priv *vpriv, uint scene_id)
@@ -619,3 +621,100 @@ done:
free(priv.value);
return ret;
 }
+
+static int h_read_settings_cmos(struct scene_obj *obj, void *vpriv)
+{
+   struct cedit_iter_priv *priv = vpriv;
+   const struct scene_menitem *mi;
+   struct scene_obj_menu *menu;
+   int val, ret;
+   uint i;
+
+   if (obj->type != SCENEOBJT_MENU)
+   return 0;
+
+   menu = (struct scene_obj_menu *)obj;
+
+   /* figure out where to place this item */
+   if (!obj->bit_length)
+   return log_msg_ret("len", -EINVAL);
+   if (obj->start_bit + obj->bit_length > CMOS_MAX_BITS)
+   return log_msg_ret("bit", -E2BIG);
+
+   val = 0;
+   for (i = 0; i < obj->bit_length; i++) {
+   uint bitnum = obj->start_bit + i;
+   uint offset = CMOS_BYTE(bitnum);
+
+   /* read the byte if not already read */
+   if (!priv->mask[offset]) {
+   ret = rtc_read8(priv->dev, offset);
+   if (ret < 0)
+   return  log_msg_ret("rea", ret);
+   priv->value[offset] = ret;
+
+   /* mark it as read */
+   priv->mask[offset] = 0xff;
+   }
+
+   if (priv->value[offset] & BIT(CMOS_BIT(bitnum)))
+   val |= BIT(i);
+   log_debug("bit %x %x\n", bitnum, val);
+   }
+
+   /* update the current item */
+   mi = scene_menuitem_find_seq(menu, val);
+   if (!mi)
+   return log_msg_ret("seq", -ENOENT);
+
+   menu->cur_item_id = mi->id;
+   log_debug("Update menu %d cur_item_id %d\n", menu->obj.id, mi->id);
+
+   return 0;
+}
+
+int cedit_read_settings_cmos(struct expo *exp, struct udevice *dev,
+bool verbose)
+{
+   struct cedit_iter_priv priv;
+   int ret, i, count, first, last;
+
+   /* read in the items */
+   priv.mask = calloc(1, CMOS_MAX_BYTES);
+   if (!priv.mask)
+   return log_msg_ret("mas", -ENOMEM);
+   priv.value = calloc(1, CMOS_MAX_BYTES);
+   if (!priv.value) {
+   free(priv.mask);
+   return log_msg_ret("val", -ENOMEM);
+   }
+   priv.dev = dev;
+
+   ret = expo_iter_scene_objs(exp, h_read_settings_cmos, );
+   if (ret) {
+   log_debug("Failed to read CMOS (err=%d)\n", ret);
+   ret = log_msg_ret("set", ret);
+   goto done;
+   }
+
+   /* read the data to the RTC */
+   first = CMOS_MAX_BYTES;
+   last = -1;
+   for (i = 0, count = 0; i < CMOS_MAX_BYTES; i++) {
+   if (priv.mask[i]) {
+   log_debug("Read byte %x: %x\n", i, priv.value[i]);
+   count++;
+   first = min(first, i);
+   last = max(last, i);
+   }
+   }
+   if (verbose) {
+   printf("Read %d bytes from offset %x to %x\n", count, first,
+  last);
+   }
+
+done:
+   free(priv.mask);
+   free(priv.value);
+   return ret;
+}
diff --git a/boot/scene_internal.h b/boot/scene_internal.h
index 23e29cb349b0..695a907dc6af 100644
--- a/boot/scene_internal.h
+++ b/boot/scene_internal.h
@@ -234,4 +234,16 @@ int expo_iter_scene_objs(struct expo *exp, 
expo_scene_obj_iterator iter,
 struct scene_menitem *scene_menuitem_find(const struct scene_obj_menu *menu,
  int id);
 
+/**
+ * scene_menuitem_find_seq() - Find the menu item at a sequential position
+ *
+ * This numbers the items from 0 and returns the seq'th one
+ *
+ * @menu: Menu to check
+ * @seq: Sequence number to look for
+ * Return: menu item if found, else NULL
+ */
+struct scene_menitem *scene_menuitem_find_seq(const struct scene_obj_menu 

[PATCH 17/19] expo: cedit: Support writing settings to CMOS RAM

2023-08-14 Thread Simon Glass
Add a command to write cedit settings to CMOS RAM so that it can be
preserved across a reboot. This uses a simple bit-encoding, where each
field has a 'bit position' and a 'bit length' in the schema.

Signed-off-by: Simon Glass 
---

 boot/cedit.c| 137 +++-
 boot/expo_build.c   |   7 +-
 cmd/cedit.c |  36 +
 doc/develop/expo.rst|  13 +++
 doc/usage/cmd/cedit.rst |  22 +
 include/cedit.h |  13 +++
 include/expo.h  |   6 +-
 test/boot/cedit.c   |  30 +++
 test/boot/files/expo_layout.dts |   5 ++
 9 files changed, 266 insertions(+), 3 deletions(-)

diff --git a/boot/cedit.c b/boot/cedit.c
index e3f6dc003991..725745aba55d 100644
--- a/boot/cedit.c
+++ b/boot/cedit.c
@@ -15,22 +15,37 @@
 #include 
 #include 
 #include 
+#include 
 #include 
+#include 
 #include 
 #include 
 #include "scene_internal.h"
 
+enum {
+   CMOS_MAX_BITS   = 2048,
+   CMOS_MAX_BYTES  = CMOS_MAX_BITS / 8,
+};
+
+#define CMOS_BYTE(bit) ((bit) / 8)
+#define CMOS_BIT(bit)  ((bit) % 8)
+
 /**
  * struct cedit_iter_priv - private data for cedit operations
  *
  * @buf: Buffer to use when writing settings to the devicetree
  * @node: Node to read from when reading settings from devicetree
  * @verbose: true to show writing to environment variables
+ * @mask: Mask bits for the CMOS RAM. If a bit is set the byte containing it
+ * will be written
+ * @value: Value bits for CMOS RAM. This is the actual value written
  */
 struct cedit_iter_priv {
struct abuf *buf;
ofnode node;
bool verbose;
+   u8 *mask;
+   u8 *value;
 };
 
 int cedit_arange(struct expo *exp, struct video_priv *vpriv, uint scene_id)
@@ -445,7 +460,7 @@ static int h_read_settings_env(struct scene_obj *obj, void 
*vpriv)
struct cedit_iter_priv *priv = vpriv;
struct scene_obj_menu *menu;
char var[60];
-   int val, ret;
+   int val;
 
if (obj->type != SCENEOBJT_MENU)
return 0;
@@ -484,3 +499,123 @@ int cedit_read_settings_env(struct expo *exp, bool 
verbose)
 
return 0;
 }
+
+/**
+ * get_cur_menuitem_seq() - Get the sequence number of a menu's current item
+ *
+ * Enumerates the items of a menu (0, 1, 2) and returns the sequence number of
+ * the currently selected item. If the first item is selected, this returns 0;
+ * if the second, 1; etc.
+ *
+ * @menu: Menu to check
+ * Return: Sequence number on success, else -ve error value
+ */
+static int get_cur_menuitem_seq(const struct scene_obj_menu *menu)
+{
+   const struct scene_menitem *mi;
+   int seq, found;
+
+   seq = 0;
+   found = -1;
+   list_for_each_entry(mi, >item_head, sibling) {
+   if (mi->id == menu->cur_item_id) {
+   found = seq;
+   break;
+   }
+   seq++;
+   }
+
+   if (found == -1)
+   return log_msg_ret("nf", -ENOENT);
+
+   return found;
+}
+
+static int h_write_settings_cmos(struct scene_obj *obj, void *vpriv)
+{
+   const struct scene_obj_menu *menu;
+   struct cedit_iter_priv *priv = vpriv;
+   int val, ret;
+   uint i, seq;
+
+   if (obj->type != SCENEOBJT_MENU)
+   return 0;
+
+   menu = (struct scene_obj_menu *)obj;
+   val = menu->cur_item_id;
+
+   ret = get_cur_menuitem_seq(menu);
+   if (ret < 0)
+   return log_msg_ret("cur", ret);
+   seq = ret;
+   log_debug("%s: seq=%d\n", menu->obj.name, seq);
+
+   /* figure out where to place this item */
+   if (!obj->bit_length)
+   return log_msg_ret("len", -EINVAL);
+   if (obj->start_bit + obj->bit_length > CMOS_MAX_BITS)
+   return log_msg_ret("bit", -E2BIG);
+
+   for (i = 0; i < obj->bit_length; i++, seq >>= 1) {
+   uint bitnum = obj->start_bit + i;
+
+   priv->mask[CMOS_BYTE(bitnum)] |= 1 << CMOS_BIT(bitnum);
+   if (seq & 1)
+   priv->value[CMOS_BYTE(bitnum)] |= BIT(CMOS_BIT(bitnum));
+   log_debug("bit %x %x %x\n", bitnum,
+ priv->mask[CMOS_BYTE(bitnum)],
+ priv->value[CMOS_BYTE(bitnum)]);
+   }
+
+   return 0;
+}
+
+int cedit_write_settings_cmos(struct expo *exp, struct udevice *dev,
+ bool verbose)
+{
+   struct cedit_iter_priv priv;
+   int ret, i, count, first, last;
+
+   /* write out the items */
+   priv.mask = calloc(1, CMOS_MAX_BYTES);
+   if (!priv.mask)
+   return log_msg_ret("mas", -ENOMEM);
+   priv.value = calloc(1, CMOS_MAX_BYTES);
+   if (!priv.value) {
+   free(priv.mask);
+   return log_msg_ret("val", -ENOMEM);
+   }
+
+   ret = expo_iter_scene_objs(exp, h_write_settings_cmos, );
+   if (ret) {
+   

[PATCH 16/19] expo: cedit: Support reading settings from environment vars

2023-08-14 Thread Simon Glass
Add a command to read cedit settings from environment variables so that
they can be restored as part of the environment.

Signed-off-by: Simon Glass 
---

 boot/cedit.c| 45 +
 cmd/cedit.c | 22 
 doc/usage/cmd/cedit.rst | 19 +
 include/cedit.h |  8 
 test/boot/cedit.c   | 12 ++-
 5 files changed, 105 insertions(+), 1 deletion(-)

diff --git a/boot/cedit.c b/boot/cedit.c
index 9399c01cda96..e3f6dc003991 100644
--- a/boot/cedit.c
+++ b/boot/cedit.c
@@ -439,3 +439,48 @@ int cedit_write_settings_env(struct expo *exp, bool 
verbose)
 
return 0;
 }
+
+static int h_read_settings_env(struct scene_obj *obj, void *vpriv)
+{
+   struct cedit_iter_priv *priv = vpriv;
+   struct scene_obj_menu *menu;
+   char var[60];
+   int val, ret;
+
+   if (obj->type != SCENEOBJT_MENU)
+   return 0;
+
+   menu = (struct scene_obj_menu *)obj;
+   val = menu->cur_item_id;
+   snprintf(var, sizeof(var), "c.%s", obj->name);
+
+   val = env_get_ulong(var, 10, 0);
+   if (priv->verbose)
+   printf("%s=%d\n", var, val);
+   if (!val)
+   return log_msg_ret("get", -ENOENT);
+
+   /*
+* note that no validation is done here, to make sure the ID is valid
+* and actually points to a menu item
+*/
+   menu->cur_item_id = val;
+
+   return 0;
+}
+
+int cedit_read_settings_env(struct expo *exp, bool verbose)
+{
+   struct cedit_iter_priv priv;
+   int ret;
+
+   /* write out the items */
+   priv.verbose = verbose;
+   ret = expo_iter_scene_objs(exp, h_read_settings_env, );
+   if (ret) {
+   log_debug("Failed to read settings from env (err=%d)\n", ret);
+   return log_msg_ret("set", ret);
+   }
+
+   return 0;
+}
diff --git a/cmd/cedit.c b/cmd/cedit.c
index 85629f7b83cb..b2548f44b572 100644
--- a/cmd/cedit.c
+++ b/cmd/cedit.c
@@ -156,6 +156,26 @@ static int do_cedit_write_env(struct cmd_tbl *cmdtp, int 
flag, int argc,
return 0;
 }
 
+static int do_cedit_read_env(struct cmd_tbl *cmdtp, int flag, int argc,
+char *const argv[])
+{
+   bool verbose;
+   int ret;
+
+   if (check_cur_expo())
+   return CMD_RET_FAILURE;
+
+   verbose = argc > 1 && !strcmp(argv[1], "-v");
+
+   ret = cedit_read_settings_env(cur_exp, verbose);
+   if (ret) {
+   printf("Failed to read settings from environment: %dE\n", ret);
+   return CMD_RET_FAILURE;
+   }
+
+   return 0;
+}
+
 static int do_cedit_run(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
 {
@@ -187,6 +207,7 @@ static char cedit_help_text[] =
"load  - load config editor\n"
"cedit read_fdt- read settings\n"
"cedit write_fdt   - write settings\n"
+   "cedit read_env [-v]  - read settings from 
env vars\n"
"cedit write_env [-v] - write settings to 
env vars\n"
"cedit run- run config editor";
 #endif /* CONFIG_SYS_LONGHELP */
@@ -195,6 +216,7 @@ U_BOOT_CMD_WITH_SUBCMDS(cedit, "Configuration editor", 
cedit_help_text,
U_BOOT_SUBCMD_MKENT(load, 5, 1, do_cedit_load),
U_BOOT_SUBCMD_MKENT(read_fdt, 5, 1, do_cedit_read_fdt),
U_BOOT_SUBCMD_MKENT(write_fdt, 5, 1, do_cedit_write_fdt),
+   U_BOOT_SUBCMD_MKENT(read_env, 2, 1, do_cedit_read_env),
U_BOOT_SUBCMD_MKENT(write_env, 2, 1, do_cedit_write_env),
U_BOOT_SUBCMD_MKENT(run, 1, 1, do_cedit_run),
 );
diff --git a/doc/usage/cmd/cedit.rst b/doc/usage/cmd/cedit.rst
index 426470a82ac2..1f92b7306a7a 100644
--- a/doc/usage/cmd/cedit.rst
+++ b/doc/usage/cmd/cedit.rst
@@ -13,6 +13,7 @@ Synopis
 cedit write_fdt  
 cedit read_fdt  
 cedit write_env [-v]
+cedit read_env [-v]
 
 Description
 ---
@@ -53,6 +54,16 @@ cedit read_fdt
 Reads the user settings from a devicetree file and updates the cedit with those
 settings.
 
+cedit read_env
+~~
+
+Reads the settings from the environment variables. For each menu item ``,
+cedit looks for a variable called `c.` with the ID of the selected menu
+item.
+
+The `-v` flag enables verbose mode, where each variable is printed after it is
+read.
+
 cedit write_env
 ~~~
 
@@ -91,6 +102,10 @@ That results in::
 This shows settings being stored in the environment::
 
 => cedit write_env -v
+c.cpu-speed=7
+c.cpu-speed-str=2.5 GHz
+c.power-loss=12
+c.power-loss-str=Memory
 => print
 ...
 c.cpu-speed=6
@@ -98,3 +113,7 @@ This shows settings being stored in the environment::
 c.power-loss=10
 c.power-loss-str=Always Off
 ...
+
+=> cedit read_env -v
+c.cpu-speed=7
+c.power-loss=12
diff --git a/include/cedit.h 

[PATCH 15/19] expo: cedit: Support writing settings to environment vars

2023-08-14 Thread Simon Glass
Add a command to write cedit settings to environment variables so that
they can be stored with 'saveenv'.

Signed-off-by: Simon Glass 
---

 boot/cedit.c| 97 +++--
 cmd/cedit.c | 22 ++
 doc/usage/cmd/cedit.rst | 25 +++
 include/cedit.h |  9 
 test/boot/cedit.c   | 33 ++
 5 files changed, 172 insertions(+), 14 deletions(-)

diff --git a/boot/cedit.c b/boot/cedit.c
index 6a74a3809898..9399c01cda96 100644
--- a/boot/cedit.c
+++ b/boot/cedit.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -24,10 +25,12 @@
  *
  * @buf: Buffer to use when writing settings to the devicetree
  * @node: Node to read from when reading settings from devicetree
+ * @verbose: true to show writing to environment variables
  */
 struct cedit_iter_priv {
struct abuf *buf;
ofnode node;
+   bool verbose;
 };
 
 int cedit_arange(struct expo *exp, struct video_priv *vpriv, uint scene_id)
@@ -209,6 +212,30 @@ static int check_space(int ret, struct abuf *buf)
return 0;
 }
 
+static int get_cur_menuitem_text(const struct scene_obj_menu *menu,
+const char **strp)
+{
+   struct scene *scn = menu->obj.scene;
+   const struct scene_menitem *mi;
+   const struct scene_obj_txt *txt;
+   const char *str;
+
+   mi = scene_menuitem_find(menu, menu->cur_item_id);
+   if (!mi)
+   return log_msg_ret("mi", -ENOENT);
+
+   txt = scene_obj_find(scn, mi->label_id, SCENEOBJT_TEXT);
+   if (!txt)
+   return log_msg_ret("txt", -ENOENT);
+
+   str = expo_get_str(scn->expo, txt->str_id);
+   if (!str)
+   return log_msg_ret("str", -ENOENT);
+   *strp = str;
+
+   return 0;
+}
+
 static int h_write_settings(struct scene_obj *obj, void *vpriv)
 {
struct cedit_iter_priv *priv = vpriv;
@@ -221,9 +248,6 @@ static int h_write_settings(struct scene_obj *obj, void 
*vpriv)
break;
case SCENEOBJT_MENU: {
const struct scene_obj_menu *menu;
-   const struct scene_obj_txt *txt;
-   struct scene *scn = obj->scene;
-   const struct scene_menitem *mi;
const char *str;
char name[80];
int ret, i;
@@ -243,17 +267,9 @@ static int h_write_settings(struct scene_obj *obj, void 
*vpriv)
if (ret)
return log_msg_ret("wrt", -EFAULT);
 
-   mi = scene_menuitem_find(menu, menu->cur_item_id);
-   if (!mi)
-   return log_msg_ret("mi", -ENOENT);
-
-   txt = scene_obj_find(scn, mi->label_id, SCENEOBJT_TEXT);
-   if (!txt)
-   return log_msg_ret("txt", -ENOENT);
-
-   str = expo_get_str(scn->expo, txt->str_id);
-   if (!str)
-   return log_msg_ret("str", -ENOENT);
+   ret = get_cur_menuitem_text(menu, );
+   if (ret)
+   return log_msg_ret("mis", ret);
 
snprintf(name, sizeof(name), "%s-str", obj->name);
ret = -EAGAIN;
@@ -370,3 +386,56 @@ int cedit_read_settings(struct expo *exp, oftree tree)
 
return 0;
 }
+
+static int h_write_settings_env(struct scene_obj *obj, void *vpriv)
+{
+   const struct scene_obj_menu *menu;
+   struct cedit_iter_priv *priv = vpriv;
+   char name[80], var[60];
+   const char *str;
+   int val, ret;
+
+   if (obj->type != SCENEOBJT_MENU)
+   return 0;
+
+   menu = (struct scene_obj_menu *)obj;
+   val = menu->cur_item_id;
+   snprintf(var, sizeof(var), "c.%s", obj->name);
+
+   if (priv->verbose)
+   printf("%s=%d\n", var, val);
+
+   ret = env_set_ulong(var, val);
+   if (ret)
+   return log_msg_ret("set", ret);
+
+   ret = get_cur_menuitem_text(menu, );
+   if (ret)
+   return log_msg_ret("mis", ret);
+
+   snprintf(name, sizeof(name), "c.%s-str", obj->name);
+   if (priv->verbose)
+   printf("%s=%s\n", name, str);
+
+   ret = env_set(name, str);
+   if (ret)
+   return log_msg_ret("st2", ret);
+
+   return 0;
+}
+
+int cedit_write_settings_env(struct expo *exp, bool verbose)
+{
+   struct cedit_iter_priv priv;
+   int ret;
+
+   /* write out the items */
+   priv.verbose = verbose;
+   ret = expo_iter_scene_objs(exp, h_write_settings_env, );
+   if (ret) {
+   log_debug("Failed to write settings to env (err=%d)\n", ret);
+   return log_msg_ret("set", ret);
+   }
+
+   return 0;
+}
diff --git a/cmd/cedit.c b/cmd/cedit.c
index a155e080b1f2..85629f7b83cb 100644
--- a/cmd/cedit.c
+++ b/cmd/cedit.c
@@ -136,6 +136,26 @@ static int do_cedit_read_fdt(struct cmd_tbl *cmdtp, int 
flag, int argc,
   

[PATCH 14/19] expo: cedit: Support reading settings from a file

2023-08-14 Thread Simon Glass
Add a command to read cedit settings from a devicetree file.

Signed-off-by: Simon Glass 
---

 boot/cedit.c| 52 +
 cmd/cedit.c | 39 +++
 doc/usage/cmd/cedit.rst |  8 +++
 include/cedit.h | 13 +++
 test/boot/cedit.c   | 22 ++---
 5 files changed, 131 insertions(+), 3 deletions(-)

diff --git a/boot/cedit.c b/boot/cedit.c
index 4dd79a2263d6..6a74a3809898 100644
--- a/boot/cedit.c
+++ b/boot/cedit.c
@@ -23,9 +23,11 @@
  * struct cedit_iter_priv - private data for cedit operations
  *
  * @buf: Buffer to use when writing settings to the devicetree
+ * @node: Node to read from when reading settings from devicetree
  */
 struct cedit_iter_priv {
struct abuf *buf;
+   ofnode node;
 };
 
 int cedit_arange(struct expo *exp, struct video_priv *vpriv, uint scene_id)
@@ -318,3 +320,53 @@ int cedit_write_settings(struct expo *exp, struct abuf 
*buf)
 
return 0;
 }
+
+static int h_read_settings(struct scene_obj *obj, void *vpriv)
+{
+   struct cedit_iter_priv *priv = vpriv;
+   ofnode node = priv->node;
+
+   switch (obj->type) {
+   case SCENEOBJT_NONE:
+   case SCENEOBJT_IMAGE:
+   case SCENEOBJT_TEXT:
+   break;
+   case SCENEOBJT_MENU: {
+   struct scene_obj_menu *menu;
+   uint val;
+
+   if (ofnode_read_u32(node, obj->name, ))
+   return log_msg_ret("rd", -ENOENT);
+   menu = (struct scene_obj_menu *)obj;
+   menu->cur_item_id = val;
+
+   break;
+   }
+   }
+
+   return 0;
+}
+
+int cedit_read_settings(struct expo *exp, oftree tree)
+{
+   struct cedit_iter_priv priv;
+   ofnode root, node;
+   int ret;
+
+   root = oftree_root(tree);
+   if (!ofnode_valid(root))
+   return log_msg_ret("roo", -ENOENT);
+   node = ofnode_find_subnode(root, CEDIT_NODE_NAME);
+   if (!ofnode_valid(node))
+   return log_msg_ret("pat", -ENOENT);
+
+   /* read in the items */
+   priv.node = node;
+   ret = expo_iter_scene_objs(exp, h_read_settings, );
+   if (ret) {
+   log_debug("Failed to read settings (err=%d)\n", ret);
+   return log_msg_ret("set", ret);
+   }
+
+   return 0;
+}
diff --git a/cmd/cedit.c b/cmd/cedit.c
index 18cc8ba191bf..a155e080b1f2 100644
--- a/cmd/cedit.c
+++ b/cmd/cedit.c
@@ -99,6 +99,43 @@ static int do_cedit_write_fdt(struct cmd_tbl *cmdtp, int 
flag, int argc,
return 0;
 }
 
+static int do_cedit_read_fdt(struct cmd_tbl *cmdtp, int flag, int argc,
+char *const argv[])
+{
+   const char *fname;
+   void *buf;
+   oftree tree;
+   ulong size;
+   int ret;
+
+   if (argc < 4)
+   return CMD_RET_USAGE;
+   fname = argv[3];
+
+   ret = fs_load_alloc(argv[1], argv[2], argv[3], SZ_1M, 0, , );
+   if (ret) {
+   printf("File not found\n");
+   return CMD_RET_FAILURE;
+   }
+
+   tree = oftree_from_fdt(buf);
+   if (!oftree_valid(tree)) {
+   free(buf);
+   printf("Cannot create oftree\n");
+   return CMD_RET_FAILURE;
+   }
+
+   ret = cedit_read_settings(cur_exp, tree);
+   oftree_dispose(tree);
+   free(buf);
+   if (ret) {
+   printf("Failed to read settings: %dE\n", ret);
+   return CMD_RET_FAILURE;
+   }
+
+   return 0;
+}
+
 static int do_cedit_run(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
 {
@@ -128,12 +165,14 @@ static int do_cedit_run(struct cmd_tbl *cmdtp, int flag, 
int argc,
 #ifdef CONFIG_SYS_LONGHELP
 static char cedit_help_text[] =
"load  - load config editor\n"
+   "cedit read_fdt- read settings\n"
"cedit write_fdt   - write settings\n"
"cedit run- run config editor";
 #endif /* CONFIG_SYS_LONGHELP */
 
 U_BOOT_CMD_WITH_SUBCMDS(cedit, "Configuration editor", cedit_help_text,
U_BOOT_SUBCMD_MKENT(load, 5, 1, do_cedit_load),
+   U_BOOT_SUBCMD_MKENT(read_fdt, 5, 1, do_cedit_read_fdt),
U_BOOT_SUBCMD_MKENT(write_fdt, 5, 1, do_cedit_write_fdt),
U_BOOT_SUBCMD_MKENT(run, 1, 1, do_cedit_run),
 );
diff --git a/doc/usage/cmd/cedit.rst b/doc/usage/cmd/cedit.rst
index 0581594831fa..0a9f620b59b3 100644
--- a/doc/usage/cmd/cedit.rst
+++ b/doc/usage/cmd/cedit.rst
@@ -11,6 +11,7 @@ Synopis
 cedit load   
 cedit run
 cedit write_fdt  
+cedit read_fdt  
 
 Description
 ---
@@ -45,6 +46,11 @@ cedit write_fdt
 Writes the current user settings to a devicetree file. For each menu item the
 selected ID and its text string are written.
 
+cedit read_fdt
+~~
+
+Reads the user settings from a devicetree file and updates the cedit with those

[PATCH 13/19] expo: cedit: Support writing settings to a file

2023-08-14 Thread Simon Glass
Support writing settings from an expo into a file in FDT format. It
consists of a single node with a two properties for each sceneitem,
one with tag ID chosen by the user and another for its text value.

Signed-off-by: Simon Glass 
---

 boot/cedit.c| 136 
 cmd/cedit.c |  51 ++-
 doc/usage/cmd/cedit.rst |  19 ++
 include/cedit.h |  22 +++
 test/boot/cedit.c   |  45 +
 5 files changed, 270 insertions(+), 3 deletions(-)

diff --git a/boot/cedit.c b/boot/cedit.c
index 6c10b2114541..4dd79a2263d6 100644
--- a/boot/cedit.c
+++ b/boot/cedit.c
@@ -9,6 +9,7 @@
 #define LOG_CATEGORY LOGC_EXPO
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -18,6 +19,15 @@
 #include 
 #include "scene_internal.h"
 
+/**
+ * struct cedit_iter_priv - private data for cedit operations
+ *
+ * @buf: Buffer to use when writing settings to the devicetree
+ */
+struct cedit_iter_priv {
+   struct abuf *buf;
+};
+
 int cedit_arange(struct expo *exp, struct video_priv *vpriv, uint scene_id)
 {
struct scene_obj_txt *txt;
@@ -182,3 +192,129 @@ int cedit_run(struct expo *exp)
 
return 0;
 }
+
+static int check_space(int ret, struct abuf *buf)
+{
+   if (ret == -FDT_ERR_NOSPACE) {
+   if (!abuf_realloc_inc(buf, CEDIT_SIZE_INC))
+   return log_msg_ret("spc", -ENOMEM);
+   ret = fdt_resize(abuf_data(buf), abuf_data(buf),
+abuf_size(buf));
+   if (ret)
+   return log_msg_ret("res", -EFAULT);
+   }
+
+   return 0;
+}
+
+static int h_write_settings(struct scene_obj *obj, void *vpriv)
+{
+   struct cedit_iter_priv *priv = vpriv;
+   struct abuf *buf = priv->buf;
+
+   switch (obj->type) {
+   case SCENEOBJT_NONE:
+   case SCENEOBJT_IMAGE:
+   case SCENEOBJT_TEXT:
+   break;
+   case SCENEOBJT_MENU: {
+   const struct scene_obj_menu *menu;
+   const struct scene_obj_txt *txt;
+   struct scene *scn = obj->scene;
+   const struct scene_menitem *mi;
+   const char *str;
+   char name[80];
+   int ret, i;
+
+   menu = (struct scene_obj_menu *)obj;
+   ret = -EAGAIN;
+   for (i = 0; ret && i < 2; i++) {
+   ret = fdt_property_u32(abuf_data(buf), obj->name,
+  menu->cur_item_id);
+   if (!i) {
+   ret = check_space(ret, buf);
+   if (ret)
+   return log_msg_ret("res", -ENOMEM);
+   }
+   }
+   /* this should not happen */
+   if (ret)
+   return log_msg_ret("wrt", -EFAULT);
+
+   mi = scene_menuitem_find(menu, menu->cur_item_id);
+   if (!mi)
+   return log_msg_ret("mi", -ENOENT);
+
+   txt = scene_obj_find(scn, mi->label_id, SCENEOBJT_TEXT);
+   if (!txt)
+   return log_msg_ret("txt", -ENOENT);
+
+   str = expo_get_str(scn->expo, txt->str_id);
+   if (!str)
+   return log_msg_ret("str", -ENOENT);
+
+   snprintf(name, sizeof(name), "%s-str", obj->name);
+   ret = -EAGAIN;
+   for (i = 0; ret && i < 2; i++) {
+   ret = fdt_property_string(abuf_data(buf), name, str);
+   if (!i) {
+   ret = check_space(ret, buf);
+   if (ret)
+   return log_msg_ret("rs2", -ENOMEM);
+   }
+   }
+
+   /* this should not happen */
+   if (ret)
+   return log_msg_ret("wr2", -EFAULT);
+
+   break;
+   }
+   }
+
+   return 0;
+}
+
+int cedit_write_settings(struct expo *exp, struct abuf *buf)
+{
+   struct cedit_iter_priv priv;
+   void *fdt;
+   int ret;
+
+   abuf_init(buf);
+   if (!abuf_realloc(buf, CEDIT_SIZE_INC))
+   return log_msg_ret("buf", -ENOMEM);
+
+   fdt = abuf_data(buf);
+   ret = fdt_create(fdt, abuf_size(buf));
+   if (!ret)
+   ret = fdt_finish_reservemap(fdt);
+   if (!ret)
+   ret = fdt_begin_node(fdt, "");
+   if (!ret)
+   ret = fdt_begin_node(fdt, CEDIT_NODE_NAME);
+   if (ret) {
+   log_debug("Failed to start FDT (err=%d)\n", ret);
+   return log_msg_ret("sta", -EINVAL);
+   }
+
+   /* write out the items */
+   priv.buf = buf;
+   ret = expo_iter_scene_objs(exp, h_write_settings, );
+   if (ret) {
+   log_debug("Failed to write settings (err=%d)\n", ret);
+   

[PATCH 12/19] expo: Export scene_menuitem_find() for use in internal code

2023-08-14 Thread Simon Glass
Make this function available to other expo code so we can use it to look
up a menu item.

Signed-off-by: Simon Glass 
---

 boot/scene_internal.h | 12 
 boot/scene_menu.c |  4 ++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/boot/scene_internal.h b/boot/scene_internal.h
index 60b143440785..23e29cb349b0 100644
--- a/boot/scene_internal.h
+++ b/boot/scene_internal.h
@@ -222,4 +222,16 @@ int scene_iter_objs(struct scene *scn, 
expo_scene_obj_iterator iter,
 int expo_iter_scene_objs(struct expo *exp, expo_scene_obj_iterator iter,
 void *priv);
 
+/**
+ * scene_menuitem_find() - Find the menu item for an ID
+ *
+ * Looks up the menu to find the item with the given ID
+ *
+ * @menu: Menu to check
+ * @id: ID to look for
+ * Return: Menu item, or NULL if not found
+ */
+struct scene_menitem *scene_menuitem_find(const struct scene_obj_menu *menu,
+ int id);
+
 #endif /* __SCENE_INTERNAL_H */
diff --git a/boot/scene_menu.c b/boot/scene_menu.c
index 57ffb523ff3f..602fe24580af 100644
--- a/boot/scene_menu.c
+++ b/boot/scene_menu.c
@@ -33,8 +33,8 @@ void scene_menu_destroy(struct scene_obj_menu *menu)
scene_menuitem_destroy(item);
 }
 
-static struct scene_menitem *scene_menuitem_find(struct scene_obj_menu *menu,
-int id)
+struct scene_menitem *scene_menuitem_find(const struct scene_obj_menu *menu,
+ int id)
 {
struct scene_menitem *item;
 
-- 
2.41.0.694.ge786442a9b-goog



[PATCH 11/19] doc: Expand documentation for the cedit command

2023-08-14 Thread Simon Glass
Add a little information about each subcommand.

Signed-off-by: Simon Glass 
---

 doc/usage/cmd/cedit.rst | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/doc/usage/cmd/cedit.rst b/doc/usage/cmd/cedit.rst
index 3d815bd27af2..d34a220797ee 100644
--- a/doc/usage/cmd/cedit.rst
+++ b/doc/usage/cmd/cedit.rst
@@ -24,6 +24,21 @@ The description is in the form of a devicetree file, as 
documented at
 
 See :doc:`../../develop/cedit` for information about the configuration editor.
 
+cedit load
+~~
+
+Loads a configuration-editor description from a file. It creates a new cedit
+structure ready for use. Initially no settings are read, so default values are
+used for each object.
+
+cedit run
+~
+
+Runs the default configuration-editor event loop. This is very simple, just
+accepting character input and moving through the objects under user control.
+The implementation is at `cedit_run()`.
+
+
 Example
 ---
 
-- 
2.41.0.694.ge786442a9b-goog



[PATCH 10/19] expo: Move cedit theme under bootstd

2023-08-14 Thread Simon Glass
This is related to standard boot, so put it under the same node. This may
simplify schema upstreaming later.

Mention themes in the documentation while we are here.

Signed-off-by: Simon Glass 
---

 arch/sandbox/dts/sandbox.dtsi | 12 ++--
 arch/sandbox/dts/test.dts | 12 ++--
 cmd/cedit.c   |  2 +-
 doc/develop/cedit.rst |  7 +++
 4 files changed, 20 insertions(+), 13 deletions(-)

diff --git a/arch/sandbox/dts/sandbox.dtsi b/arch/sandbox/dts/sandbox.dtsi
index f0ee0b3481ac..4a0a5e847bef 100644
--- a/arch/sandbox/dts/sandbox.dtsi
+++ b/arch/sandbox/dts/sandbox.dtsi
@@ -16,12 +16,6 @@
stdout-path = "/serial";
};
 
-   cedit-theme {
-   font-size = <30>;
-   menu-inset = <3>;
-   menuitem-gap-y = <1>;
-   };
-
alarm_wdt: alarm-wdt {
compatible = "sandbox,alarm-wdt";
timeout-sec = <5>;
@@ -36,6 +30,12 @@
bootstd {
compatible = "u-boot,boot-std";
filename-prefixes = "./";
+
+   cedit-theme {
+   font-size = <30>;
+   menu-inset = <3>;
+   menuitem-gap-y = <1>;
+   };
};
 
buttons {
diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
index b5509eee8cfe..9641776c8ba8 100644
--- a/arch/sandbox/dts/test.dts
+++ b/arch/sandbox/dts/test.dts
@@ -100,6 +100,12 @@
menuitem-gap-y = <1>;
};
 
+   cedit-theme {
+   font-size = <30>;
+   menu-inset = <3>;
+   menuitem-gap-y = <1>;
+   };
+
/*
 * This is used for the VBE OS-request tests. A FAT filesystem
 * created in a partition with the VBE information appearing
@@ -144,12 +150,6 @@
cedit: cedit {
};
 
-   cedit-theme {
-   font-size = <30>;
-   menu-inset = <3>;
-   menuitem-gap-y = <1>;
-   };
-
fuzzing-engine {
compatible = "sandbox,fuzzing-engine";
};
diff --git a/cmd/cedit.c b/cmd/cedit.c
index 5f0e84403f5b..e98121b067b2 100644
--- a/cmd/cedit.c
+++ b/cmd/cedit.c
@@ -65,7 +65,7 @@ static int do_cedit_run(struct cmd_tbl *cmdtp, int flag, int 
argc,
return CMD_RET_FAILURE;
}
 
-   node = ofnode_path("/cedit-theme");
+   node = ofnode_path("/bootstd/cedit-theme");
if (ofnode_valid(node)) {
ret = expo_apply_theme(cur_exp, node);
if (ret)
diff --git a/doc/develop/cedit.rst b/doc/develop/cedit.rst
index 48262ee535e9..8f0a554ae918 100644
--- a/doc/develop/cedit.rst
+++ b/doc/develop/cedit.rst
@@ -145,3 +145,10 @@ Multiple scenes
 Expo supports multiple scenes but has no pre-determined way of moving between
 them. You could use selection of a menu item as a signal to change the scene,
 but this is not currently implemented in the cedit code (see `cedit_run()`).
+
+
+Themes
+--
+
+The configuration editor uses simple expo themes. The theme is read from
+`/bootstd/cedit-theme` in the devicetree.
-- 
2.41.0.694.ge786442a9b-goog



[PATCH 09/19] expo: Add documentation for the configuration editor

2023-08-14 Thread Simon Glass
This is mentioned in passing in the 'cedit' command. Its file format is
described under `expo`. But it would be better if it had its own entry
in the documentation.

Add a new 'cedit' entry with a few details about this feature.

Signed-off-by: Simon Glass 
---

 doc/develop/cedit.rst   | 147 
 doc/develop/expo.rst|   3 +
 doc/develop/index.rst   |   1 +
 doc/usage/cmd/cedit.rst |   2 +
 4 files changed, 153 insertions(+)
 create mode 100644 doc/develop/cedit.rst

diff --git a/doc/develop/cedit.rst b/doc/develop/cedit.rst
new file mode 100644
index ..48262ee535e9
--- /dev/null
+++ b/doc/develop/cedit.rst
@@ -0,0 +1,147 @@
+.. SPDX-License-Identifier: GPL-2.0+
+
+Configuration Editor
+
+
+Introduction
+
+
+U-Boot provides a configuration editor which allows settings to be changed in
+a GUI or text environment.
+
+
+This feature is still in development and has a number of limitations. For
+example, cedit only supports menu items (there is no numeric or text entry),
+provides no support for colour text and does not support scrolling. Still it is
+possible to use it for simple applications.
+
+
+Overview
+
+
+The configuration editor makes use of :doc:`expo` to build a description of the
+configuration screens and allow user to interact with it.
+
+To create a single-scene cedit for your application:
+
+#. Design the scene, i.e. the objects that need to be present and what their
+   possible values are
+
+#. Enter this in .dts format
+
+#. Create a header file containing the IDs
+
+#. Run the 'expo.py' tool to generate a .dtb file containing the layout, which
+   can be used by U-Boot
+
+#. Use the :doc:`../usage/cmd/cedit` to create the cedit, read the settings,
+   present the cedit to the user and save the settings afterwards.
+
+Each of these is described in a separate section. See :ref:`expo_example` for
+an example file.
+
+
+Design a scene
+--
+
+Using a piece of paper or a drawing tool, lay out the objects you want in your
+scene. Typically you will use the default layout engine, which simply puts 
items
+one after the other from top to bottom. So use a single column and show the
+prompt and value for each object.
+
+For menu items, show one of the values, but keep in mind what else you need.
+
+
+Create an expo-format file
+--
+
+The description is in the form of a devicetree file, as documented at
+:ref:`expo_format`. Since everything in an expo has an ID number (an integer
+greater than 1) the description is written terms of these IDs. They each have
+an enum value. which is typically taken care of by the `expo.py` tool.
+
+The expo should have a `scenes` node with a named scene as a subnode. Within 
the
+scene, add properties for the scene, then a subnode for each object in the
+scene.
+
+All object nodes require an `id` value and a `type` property. Other properties
+depend on the type. For example, a menu has a `title` and an `item-label` list
+proving the text for the menu items, as well as an `item-id` list providing the
+ID of each menu item, so it can be selected.
+
+Text properties may have two variants. For example `title` specifies the title
+of a menu, but you can instead use `title-id` to specify the string ID to use 
as
+the title. String are defined in a separate area, common to the whole expo,
+which contains a subnode for each string. Within that subnode are the ID and 
the
+`value` (i.e. the text). For now only English is supported, but in future it 
may
+be possible to append a language identifier to provide other values (e.g.
+'value-es' for Spanish).
+
+
+Create an ID header-file
+
+
+Expo needs to know the integer value to use for every ID referenced in your
+expo-format file. For example, if you have defined a `cpu-speed` node with an
+id of `ID_CPU_SPEED`, then Expo needs to know the value of `ID_CPU_SPEED`.
+
+When you write C code to use the expo, you may need to know the IDs. For
+example, to find which value the user selected in `cpu-speed` menu, you must
+use the `ID_CPU_SPEED` ID. The ID is the only way to refer to anything in Expo.
+
+Since we need a shared set of IDs, it is best to have a header file containing
+them. Expo supports doing this with an enum, where every ID is listed in the
+enum::
+
+enum {
+ZERO,
+
+ID_PROMPT,
+
+ID_SCENE1,
+ID_SCENE1_TITLE,
+...
+};
+
+The C compiler can parse this directly. The `expo.py` tool parses it for expo.
+
+Create a header file containing every ID mentioned in your expo. Try to group
+related things together.
+
+
+Build the expo layout
+-
+
+Use the `expo.py` tool to build a .dtb for your expo::
+
+./tools/expo.py -e expo_ids.h -l expo_layout.dts -o expo.dtb
+
+This uses the enum in the provided header file to get the ID numbers, grabs
+the `.dts` file, inserts the ID numbers and then uses the 

[PATCH 08/19] expo: Tidy up the expo.py tool and usage

2023-08-14 Thread Simon Glass
Tidy up this tool a little:

- define which arguments are needed
- split the enum values out into a header file
- warn if no enum values are found
- display the dtc error if something goes wrong
- avoid a Python traceback on error

Signed-off-by: Simon Glass 
---

 doc/develop/expo.rst| 37 +++--
 test/boot/files/expo_ids.h  | 25 ++
 test/boot/files/expo_layout.dts | 23 +---
 test/py/tests/test_ut.py|  4 +++-
 tools/expo.py   | 22 +---
 5 files changed, 65 insertions(+), 46 deletions(-)
 create mode 100644 test/boot/files/expo_ids.h

diff --git a/doc/develop/expo.rst b/doc/develop/expo.rst
index 2ac4af232da4..0643283ae481 100644
--- a/doc/develop/expo.rst
+++ b/doc/develop/expo.rst
@@ -367,22 +367,27 @@ strings are provided inline in the nodes where they are 
used.
 
 ::
 
-#define ID_PROMPT   1
-#define ID_SCENE1   2
-#define ID_SCENE1_TITLE 3
-
-#define ID_CPU_SPEED4
-#define ID_CPU_SPEED_TITLE  5
-#define ID_CPU_SPEED_1  6
-#define ID_CPU_SPEED_2  7
-#define ID_CPU_SPEED_3  8
-
-#define ID_POWER_LOSS   9
-#define ID_AC_OFF   10
-#define ID_AC_ON11
-#define ID_AC_MEMORY12
-
-#define ID_DYNAMIC_START13
+/* this comment is parsed by the expo.py tool to insert the values below
+
+enum {
+ZERO,
+ID_PROMPT,
+ID_SCENE1,
+ID_SCENE1_TITLE,
+
+ID_CPU_SPEED,
+ID_CPU_SPEED_TITLE,
+ID_CPU_SPEED_1,
+ID_CPU_SPEED_2,
+ID_CPU_SPEED_3,
+
+ID_POWER_LOSS,
+ID_AC_OFF,
+ID_AC_ON,
+ID_AC_MEMORY,
+
+ID_DYNAMIC_START,
+*/
 
  {
 dynamic-start = ;
diff --git a/test/boot/files/expo_ids.h b/test/boot/files/expo_ids.h
new file mode 100644
index ..027d44bf38c5
--- /dev/null
+++ b/test/boot/files/expo_ids.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Sample expo screen layout (ID numbers)
+ */
+
+enum {
+   ZERO,
+   ID_PROMPT,
+
+   ID_SCENE1,
+   ID_SCENE1_TITLE,
+
+   ID_CPU_SPEED,
+   ID_CPU_SPEED_TITLE,
+   ID_CPU_SPEED_1,
+   ID_CPU_SPEED_2,
+   ID_CPU_SPEED_3,
+
+   ID_POWER_LOSS,
+   ID_AC_OFF,
+   ID_AC_ON,
+   ID_AC_MEMORY,
+
+   ID_DYNAMIC_START,
+};
diff --git a/test/boot/files/expo_layout.dts b/test/boot/files/expo_layout.dts
index 55d5c910dd5e..913140bace98 100644
--- a/test/boot/files/expo_layout.dts
+++ b/test/boot/files/expo_layout.dts
@@ -5,28 +5,7 @@
 
 /dts-v1/;
 
-/*
-enum {
-   ZERO,
-   ID_PROMPT,
-
-   ID_SCENE1,
-   ID_SCENE1_TITLE,
-
-   ID_CPU_SPEED,
-   ID_CPU_SPEED_TITLE,
-   ID_CPU_SPEED_1,
-   ID_CPU_SPEED_2,
-   ID_CPU_SPEED_3,
-
-   ID_POWER_LOSS,
-   ID_AC_OFF,
-   ID_AC_ON,
-   ID_AC_MEMORY,
-
-   ID_DYNAMIC_START,
-};
-*/
+/* see expo_ids.h for the IDs */
 
 / {
dynamic-start = ;
diff --git a/test/py/tests/test_ut.py b/test/py/tests/test_ut.py
index aa1d477cd565..6a59c306322f 100644
--- a/test/py/tests/test_ut.py
+++ b/test/py/tests/test_ut.py
@@ -285,10 +285,12 @@ label Fedora-Workstation-armhfp-31-1.9 
(5.3.7-301.fc31.armv7hl)
 def setup_cedit_file(cons):
 infname = os.path.join(cons.config.source_dir,
'test/boot/files/expo_layout.dts')
+inhname = os.path.join(cons.config.source_dir,
+   'test/boot/files/expo_ids.h')
 expo_tool = os.path.join(cons.config.source_dir, 'tools/expo.py')
 outfname = 'cedit.dtb'
 u_boot_utils.run_and_log(
-cons, f'{expo_tool} -e {infname} -l {infname} -o {outfname}')
+cons, f'{expo_tool} -e {inhname} -l {infname} -o {outfname}')
 
 
 @pytest.mark.buildconfigspec('ut_dm')
diff --git a/tools/expo.py b/tools/expo.py
index c6eb87aec739..ea80c70f04e3 100755
--- a/tools/expo.py
+++ b/tools/expo.py
@@ -69,7 +69,10 @@ def calc_ids(fname):
 
 def run_expo(args):
 """Run the expo program"""
-ids = calc_ids(args.enum_fname)
+fname = args.enum_fname or args.layout
+ids = calc_ids(fname)
+if not ids:
+print(f"Warning: No enum ID values found in file '{fname}'")
 
 indata = tools.read_file(args.layout)
 
@@ -88,10 +91,10 @@ def run_expo(args):
 
 with open('/tmp/asc', 'wb') as outf:
 outf.write(data)
-proc = subprocess.run('dtc', input=data, capture_output=True, check=True)
+proc = subprocess.run('dtc', input=data, capture_output=True)
 edtb = proc.stdout
 if proc.stderr:
-print(proc.stderr)
+print(f"Devicetree compiler error:\n{proc.stderr.decode('utf-8')}")
 return 1
 tools.write_file(args.outfile, edtb)
 return 0
@@ -109,11 +112,13 @@ def parse_args(argv):
 args is a list of string arguments
 """
 parser = argparse.ArgumentParser()
+

[PATCH 07/19] expo: Add a function to prepare a cedit

2023-08-14 Thread Simon Glass
Split out the code which prepares the cedit for use, so we can call it
from a test.

Add a log category while we are here.

Signed-off-by: Simon Glass 
---

 boot/cedit.c| 32 ++--
 include/cedit.h | 15 +++
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/boot/cedit.c b/boot/cedit.c
index 2d16086bad63..6c10b2114541 100644
--- a/boot/cedit.c
+++ b/boot/cedit.c
@@ -6,6 +6,8 @@
  * Written by Simon Glass 
  */
 
+#define LOG_CATEGORY LOGC_EXPO
+
 #include 
 #include 
 #include 
@@ -47,18 +49,15 @@ int cedit_arange(struct expo *exp, struct video_priv 
*vpriv, uint scene_id)
return 0;
 }
 
-int cedit_run(struct expo *exp)
+int cedit_prepare(struct expo *exp, struct video_priv **vid_privp,
+ struct scene **scnp)
 {
-   struct cli_ch_state s_cch, *cch = _cch;
struct video_priv *vid_priv;
-   uint scene_id;
struct udevice *dev;
struct scene *scn;
-   bool done;
+   uint scene_id;
int ret;
 
-   cli_ch_init(cch);
-
/* For now we only support a video console */
ret = uclass_first_device_err(UCLASS_VIDEO, );
if (ret)
@@ -93,6 +92,27 @@ int cedit_run(struct expo *exp)
if (ret)
return log_msg_ret("dim", ret);
 
+   *vid_privp = vid_priv;
+   *scnp = scn;
+
+   return scene_id;
+}
+
+int cedit_run(struct expo *exp)
+{
+   struct cli_ch_state s_cch, *cch = _cch;
+   struct video_priv *vid_priv;
+   uint scene_id;
+   struct scene *scn;
+   bool done;
+   int ret;
+
+   cli_ch_init(cch);
+   ret = cedit_prepare(exp, _priv, );
+   if (ret < 0)
+   return log_msg_ret("prep", ret);
+   scene_id = ret;
+
done = false;
do {
struct expo_action act;
diff --git a/include/cedit.h b/include/cedit.h
index 21de12dfe7a9..851d8e83564a 100644
--- a/include/cedit.h
+++ b/include/cedit.h
@@ -8,6 +8,7 @@
 #define __CEDIT_H
 
 struct expo;
+struct scene;
 struct video_priv;
 
 /**
@@ -30,4 +31,18 @@ int cedit_arange(struct expo *exp, struct video_priv 
*vid_priv, uint scene_id);
  */
 int cedit_run(struct expo *exp);
 
+/**
+ * cedit_prepare() - Prepare to run a cedit
+ *
+ * Set up the video device, select the first scene and highlight the first 
item.
+ * This ensures that all menus have a selected item.
+ *
+ * @exp: Expo to use
+ * @vid_privp: Set to private data for the video device
+ * @scnp: Set to the first scene
+ * Return: scene ID of first scene if OK, -ve on error
+ */
+int cedit_prepare(struct expo *exp, struct video_priv **vid_privp,
+ struct scene **scnp);
+
 #endif /* __CEDIT_H */
-- 
2.41.0.694.ge786442a9b-goog



[PATCH 06/19] expo: Move cedit test into its own file and tidy

2023-08-14 Thread Simon Glass
Move this test out so it can have its own file. Rename the test to use
a cedit_ prefix.

This allows us to drop the check for CONFIG_CMD_CEDIT in the test.

Also we don't need driver model objects for this test, so drop them.

Signed-off-by: Simon Glass 
---

 test/boot/Makefile |  1 +
 test/boot/cedit.c  | 53 ++
 test/boot/expo.c   | 43 -
 3 files changed, 54 insertions(+), 43 deletions(-)
 create mode 100644 test/boot/cedit.c

diff --git a/test/boot/Makefile b/test/boot/Makefile
index 22ed61c8fa02..52947580ae60 100644
--- a/test/boot/Makefile
+++ b/test/boot/Makefile
@@ -6,6 +6,7 @@ obj-$(CONFIG_BOOTSTD) += bootdev.o bootstd_common.o bootflow.o 
bootmeth.o
 obj-$(CONFIG_FIT) += image.o
 
 obj-$(CONFIG_EXPO) += expo.o
+obj-$(CONFIG_CEDIT) += cedit.o
 
 ifdef CONFIG_OF_LIVE
 obj-$(CONFIG_BOOTMETH_VBE_SIMPLE) += vbe_simple.o
diff --git a/test/boot/cedit.c b/test/boot/cedit.c
new file mode 100644
index ..f3411f734fa2
--- /dev/null
+++ b/test/boot/cedit.c
@@ -0,0 +1,53 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2023 Google LLC
+ * Written by Simon Glass 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include "bootstd_common.h"
+#include 
+#include "../../boot/scene_internal.h"
+
+/* Check the cedit command */
+static int cedit_base(struct unit_test_state *uts)
+{
+   extern struct expo *cur_exp;
+   struct scene_obj_menu *menu;
+   struct scene_obj_txt *txt;
+   struct expo *exp;
+   struct scene *scn;
+
+   ut_assertok(run_command("cedit load hostfs - cedit.dtb", 0));
+
+   console_record_reset_enable();
+
+   /*
+* ^N  Move down to second menu
+* ^M  Open menu
+* ^N  Move down to second item
+* ^M  Select item
+* \e  Quit
+*/
+   console_in_puts("\x0e\x0d\x0e\x0d\e");
+   ut_assertok(run_command("cedit run", 0));
+
+   exp = cur_exp;
+   scn = expo_lookup_scene_id(exp, exp->scene_id);
+   ut_assertnonnull(scn);
+
+   menu = scene_obj_find(scn, scn->highlight_id, SCENEOBJT_NONE);
+   ut_assertnonnull(menu);
+
+   txt = scene_obj_find(scn, menu->title_id, SCENEOBJT_NONE);
+   ut_assertnonnull(txt);
+   ut_asserteq_str("AC Power", expo_get_str(exp, txt->str_id));
+
+   ut_asserteq(ID_AC_ON, menu->cur_item_id);
+
+   return 0;
+}
+BOOTSTD_TEST(cedit_base, 0);
diff --git a/test/boot/expo.c b/test/boot/expo.c
index 458e332440c3..90027409c817 100644
--- a/test/boot/expo.c
+++ b/test/boot/expo.c
@@ -714,46 +714,3 @@ static int expo_test_build(struct unit_test_state *uts)
return 0;
 }
 BOOTSTD_TEST(expo_test_build, UT_TESTF_DM);
-
-/* Check the cedit command */
-static int expo_cedit(struct unit_test_state *uts)
-{
-   extern struct expo *cur_exp;
-   struct scene_obj_menu *menu;
-   struct scene_obj_txt *txt;
-   struct expo *exp;
-   struct scene *scn;
-
-   if (!IS_ENABLED(CONFIG_CMD_CEDIT))
-   return -EAGAIN;
-
-   ut_assertok(run_command("cedit load hostfs - cedit.dtb", 0));
-
-   console_record_reset_enable();
-
-   /*
-* ^N  Move down to second menu
-* ^M  Open menu
-* ^N  Move down to second item
-* ^M  Select item
-* \e  Quit
-*/
-   console_in_puts("\x0e\x0d\x0e\x0d\e");
-   ut_assertok(run_command("cedit run", 0));
-
-   exp = cur_exp;
-   scn = expo_lookup_scene_id(exp, exp->scene_id);
-   ut_assertnonnull(scn);
-
-   menu = scene_obj_find(scn, scn->highlight_id, SCENEOBJT_NONE);
-   ut_assertnonnull(menu);
-
-   txt = scene_obj_find(scn, menu->title_id, SCENEOBJT_NONE);
-   ut_assertnonnull(txt);
-   ut_asserteq_str("AC Power", expo_get_str(exp, txt->str_id));
-
-   ut_asserteq(ID_AC_ON, menu->cur_item_id);
-
-   return 0;
-}
-BOOTSTD_TEST(expo_cedit, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
-- 
2.41.0.694.ge786442a9b-goog



[PATCH 05/19] expo: Split out cedit into its own header

2023-08-14 Thread Simon Glass
Before adding more functions to this interface, create a new header for
the configuration editor.

Fix up the expo header guard while we are here.

Signed-off-by: Simon Glass 
---

 boot/cedit.c|  1 +
 cmd/cedit.c |  1 +
 include/cedit.h | 33 +
 include/expo.h  | 27 +++
 4 files changed, 38 insertions(+), 24 deletions(-)
 create mode 100644 include/cedit.h

diff --git a/boot/cedit.c b/boot/cedit.c
index ee24658917bc..2d16086bad63 100644
--- a/boot/cedit.c
+++ b/boot/cedit.c
@@ -7,6 +7,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/cmd/cedit.c b/cmd/cedit.c
index 0cae304c4adc..5f0e84403f5b 100644
--- a/cmd/cedit.c
+++ b/cmd/cedit.c
@@ -7,6 +7,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/include/cedit.h b/include/cedit.h
new file mode 100644
index ..21de12dfe7a9
--- /dev/null
+++ b/include/cedit.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2023 Google LLC
+ * Written by Simon Glass 
+ */
+
+#ifndef __CEDIT_H
+#define __CEDIT_H
+
+struct expo;
+struct video_priv;
+
+/**
+ * cedit_arange() - Arrange objects in a configuration-editor scene
+ *
+ * @exp: Expo to update
+ * @vid_priv: Private info of the video device
+ * @scene_id: scene ID to arrange
+ * Returns: 0 if OK, -ve on error
+ */
+int cedit_arange(struct expo *exp, struct video_priv *vid_priv, uint scene_id);
+
+/**
+ * cedit_run() - Run a configuration editor
+ *
+ * This accepts input until the user quits with Escape
+ *
+ * @exp: Expo to use
+ * Returns: 0 if OK, -ve on error
+ */
+int cedit_run(struct expo *exp);
+
+#endif /* __CEDIT_H */
diff --git a/include/expo.h b/include/expo.h
index 0b1d944a169f..da151074d207 100644
--- a/include/expo.h
+++ b/include/expo.h
@@ -4,14 +4,13 @@
  * Written by Simon Glass 
  */
 
-#ifndef __SCENE_H
-#define __SCENE_H
+#ifndef __EXPO_H
+#define __EXPO_H
 
 #include 
 #include 
 
 struct udevice;
-struct video_priv;
 
 /**
  * enum expoact_type - types of actions reported by the expo
@@ -676,24 +675,4 @@ int expo_apply_theme(struct expo *exp, ofnode node);
  */
 int expo_build(ofnode root, struct expo **expp);
 
-/**
- * cedit_arange() - Arrange objects in a configuration-editor scene
- *
- * @exp: Expo to update
- * @vid_priv: Private info of the video device
- * @scene_id: scene ID to arrange
- * Returns: 0 if OK, -ve on error
- */
-int cedit_arange(struct expo *exp, struct video_priv *vid_priv, uint scene_id);
-
-/**
- * cedit_run() - Run a configuration editor
- *
- * This accepts input until the user quits with Escape
- *
- * @exp: Expo to use
- * Returns: 0 if OK, -ve on error
- */
-int cedit_run(struct expo *exp);
-
-#endif /*__SCENE_H */
+#endif /*__EXPO_H */
-- 
2.41.0.694.ge786442a9b-goog



[PATCH 04/19] expo: Refactor menu_build() to return the object created

2023-08-14 Thread Simon Glass
The caller reads the ID but menu_build() does this again. Add the ID as
a parameter to avoid this. Return the object created so that the caller
can adjust it.

Signed-off-by: Simon Glass 
---

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

diff --git a/boot/expo_build.c b/boot/expo_build.c
index 22f62eb54bc5..e8c4a40d3f09 100644
--- a/boot/expo_build.c
+++ b/boot/expo_build.c
@@ -214,22 +214,21 @@ static void list_strings(struct build_info *info)
  * @info: Build information
  * @node: Node containing the menu description
  * @scn: Scene to add the menu to
+ * @id: ID for the menu
+ * @objp: Returns the object pointer
  * Returns: 0 if OK, -ENOMEM if out of memory, -EINVAL if there is a format
  * error, -ENOENT if there is a references to a non-existent string
  */
-static int menu_build(struct build_info *info, ofnode node, struct scene *scn)
+static int menu_build(struct build_info *info, ofnode node, struct scene *scn,
+ uint id, struct scene_obj **objp)
 {
struct scene_obj_menu *menu;
uint title_id, menu_id;
const u32 *item_ids;
int ret, size, i;
const char *name;
-   u32 id;
 
name = ofnode_get_name(node);
-   ret = ofnode_read_u32(node, "id", );
-   if (ret)
-   return log_msg_ret("id", -EINVAL);
 
ret = scene_menu(scn, name, id, );
if (ret < 0)
@@ -275,12 +274,13 @@ static int menu_build(struct build_info *info, ofnode 
node, struct scene *scn)
if (ret < 0)
return log_msg_ret("mi", ret);
}
+   *objp = >obj;
 
return 0;
 }
 
 /**
- * menu_build() - Build an expo object and add it to a scene
+ * obj_build() - Build an expo object and add it to a scene
  *
  * See doc/developer/expo.rst for a description of the format
  *
@@ -292,6 +292,7 @@ static int menu_build(struct build_info *info, ofnode node, 
struct scene *scn)
  */
 static int obj_build(struct build_info *info, ofnode node, struct scene *scn)
 {
+   struct scene_obj *obj;
const char *type;
u32 id;
int ret;
@@ -306,7 +307,7 @@ static int obj_build(struct build_info *info, ofnode node, 
struct scene *scn)
return log_msg_ret("typ", -EINVAL);
 
if (!strcmp("menu", type))
-   ret = menu_build(info, node, scn);
+   ret = menu_build(info, node, scn, id, );
 else
ret = -EINVAL;
if (ret)
-- 
2.41.0.694.ge786442a9b-goog



[PATCH 03/19] expo: Provide a way to iterate through all scene objects

2023-08-14 Thread Simon Glass
For some operations it is necessary to process all objects in an expo.
Provide an iterator to handle this.

Signed-off-by: Simon Glass 
---

 boot/expo.c   | 15 +++
 boot/scene.c  | 16 +++
 boot/scene_internal.h | 24 +++
 test/boot/expo.c  | 45 +++
 4 files changed, 100 insertions(+)

diff --git a/boot/expo.c b/boot/expo.c
index db837f7b4924..139d684f8e6e 100644
--- a/boot/expo.c
+++ b/boot/expo.c
@@ -266,3 +266,18 @@ int expo_apply_theme(struct expo *exp, ofnode node)
 
return 0;
 }
+
+int expo_iter_scene_objs(struct expo *exp, expo_scene_obj_iterator iter,
+void *priv)
+{
+   struct scene *scn;
+   int ret;
+
+   list_for_each_entry(scn, >scene_head, sibling) {
+   ret = scene_iter_objs(scn, iter, priv);
+   if (ret)
+   return log_msg_ret("wr", ret);
+   }
+
+   return 0;
+}
diff --git a/boot/scene.c b/boot/scene.c
index b4c36c41702f..6c52948eb69c 100644
--- a/boot/scene.c
+++ b/boot/scene.c
@@ -681,3 +681,19 @@ int scene_set_open(struct scene *scn, uint id, bool open)
 
return 0;
 }
+
+int scene_iter_objs(struct scene *scn, expo_scene_obj_iterator iter,
+   void *priv)
+{
+   struct scene_obj *obj;
+
+   list_for_each_entry(obj, >obj_head, sibling) {
+   int ret;
+
+   ret = iter(obj, priv);
+   if (ret)
+   return log_msg_ret("itr", ret);
+   }
+
+   return 0;
+}
diff --git a/boot/scene_internal.h b/boot/scene_internal.h
index 1620d10a7778..60b143440785 100644
--- a/boot/scene_internal.h
+++ b/boot/scene_internal.h
@@ -9,6 +9,8 @@
 #ifndef __SCENE_INTERNAL_H
 #define __SCENE_INTERNAL_H
 
+typedef int (*expo_scene_obj_iterator)(struct scene_obj *obj, void *priv);
+
 /**
  * expo_lookup_scene_id() - Look up a scene ID
  *
@@ -198,4 +200,26 @@ int scene_menu_render_deps(struct scene *scn, struct 
scene_obj_menu *menu);
  */
 int scene_menu_calc_dims(struct scene_obj_menu *menu);
 
+/**
+ * scene_iter_objs() - Iterate through all scene objects
+ *
+ * @scn: Scene to process
+ * @iter: Iterator to call on each object
+ * @priv: Private data to pass to the iterator, in addition to the object
+ * Return: 0 if OK, -ve on error
+ */
+int scene_iter_objs(struct scene *scn, expo_scene_obj_iterator iter,
+   void *priv);
+
+/**
+ * expo_iter_scene_objects() - Iterate through all scene objects
+ *
+ * @exp: Expo to process
+ * @iter: Iterator to call on each object
+ * @priv: Private data to pass to the iterator, in addition to the object
+ * Return: 0 if OK, -ve on error
+ */
+int expo_iter_scene_objs(struct expo *exp, expo_scene_obj_iterator iter,
+void *priv);
+
 #endif /* __SCENE_INTERNAL_H */
diff --git a/test/boot/expo.c b/test/boot/expo.c
index 3898f853a751..458e332440c3 100644
--- a/test/boot/expo.c
+++ b/test/boot/expo.c
@@ -289,6 +289,33 @@ static int expo_object_attr(struct unit_test_state *uts)
 }
 BOOTSTD_TEST(expo_object_attr, UT_TESTF_DM | UT_TESTF_SCAN_FDT);
 
+/**
+ * struct test_iter_priv - private data for expo-iterator test
+ *
+ * @count: number of scene objects
+ * @menu_count: number of menus
+ * @fail_at: item ID at which to return an error
+ */
+struct test_iter_priv {
+   int count;
+   int menu_count;
+   int fail_at;
+};
+
+int h_test_iter(struct scene_obj *obj, void *vpriv)
+{
+   struct test_iter_priv *priv = vpriv;
+
+   if (priv->fail_at == obj->id)
+   return -EINVAL;
+
+   priv->count++;
+   if (obj->type == SCENEOBJT_MENU)
+   priv->menu_count++;
+
+   return 0;
+}
+
 /* Check creating a scene with a menu */
 static int expo_object_menu(struct unit_test_state *uts)
 {
@@ -296,6 +323,7 @@ static int expo_object_menu(struct unit_test_state *uts)
struct scene_menitem *item;
int id, label_id, desc_id, key_id, pointer_id, preview_id;
struct scene_obj_txt *ptr, *name1, *desc1, *key1, *tit, *prev1;
+   struct test_iter_priv priv;
struct scene *scn;
struct expo *exp;
ulong start_mem;
@@ -382,6 +410,23 @@ static int expo_object_menu(struct unit_test_state *uts)
ut_asserteq(menu->obj.dim.y + 32, prev1->obj.dim.y);
ut_asserteq(true, prev1->obj.flags & SCENEOF_HIDE);
 
+   /* check iterating through scene items */
+   memset(, '\0', sizeof(priv));
+   ut_assertok(expo_iter_scene_objs(exp, h_test_iter, ));
+   ut_asserteq(7, priv.count);
+   ut_asserteq(1, priv.menu_count);
+
+   /* check the iterator failing part way through iteration */
+   memset(, '\0', sizeof(priv));
+   priv.fail_at = key_id;
+   ut_asserteq(-EINVAL, expo_iter_scene_objs(exp, h_test_iter, ));
+
+   /* 2 items (preview_id and the menuitem) are after key_id, 7 - 2 = 5 */
+   ut_asserteq(5, priv.count);
+
+   /* menu is 

[PATCH 02/19] abuf: Allow incrementing the size

2023-08-14 Thread Simon Glass
Provide a convenience function to increment the size of the abuf.

Signed-off-by: Simon Glass 
---

 include/abuf.h  |  9 +
 lib/abuf.c  |  5 +
 test/lib/abuf.c | 25 +
 3 files changed, 39 insertions(+)

diff --git a/include/abuf.h b/include/abuf.h
index 9badda64e4fa..be98ec78c860 100644
--- a/include/abuf.h
+++ b/include/abuf.h
@@ -90,6 +90,15 @@ void abuf_map_sysmem(struct abuf *abuf, ulong addr, size_t 
size);
  */
 bool abuf_realloc(struct abuf *abuf, size_t new_size);
 
+/**
+ * abuf_realloc_inc() - Increment abuf size by a given amount
+ *
+ * @abuf: abuf to adjust
+ * @inc: Size incrmement to use (the buffer size will be increased by this 
much)
+ * Return: true if OK, false if out of memory
+ */
+bool abuf_realloc_inc(struct abuf *abuf, size_t inc);
+
 /**
  * abuf_uninit_move() - Return the allocated contents and uninit the abuf
  *
diff --git a/lib/abuf.c b/lib/abuf.c
index bd270467dd45..ce2cff53dc93 100644
--- a/lib/abuf.c
+++ b/lib/abuf.c
@@ -82,6 +82,11 @@ bool abuf_realloc(struct abuf *abuf, size_t new_size)
}
 }
 
+bool abuf_realloc_inc(struct abuf *abuf, size_t inc)
+{
+   return abuf_realloc(abuf, abuf->size + inc);
+}
+
 void *abuf_uninit_move(struct abuf *abuf, size_t *sizep)
 {
void *ptr;
diff --git a/test/lib/abuf.c b/test/lib/abuf.c
index 42ee4c175526..42803b20e2a1 100644
--- a/test/lib/abuf.c
+++ b/test/lib/abuf.c
@@ -155,6 +155,31 @@ static int lib_test_abuf_realloc_size(struct 
unit_test_state *uts)
 }
 LIB_TEST(lib_test_abuf_realloc_size, 0);
 
+/* Test abuf_realloc_inc() */
+static int lib_test_abuf_realloc_inc(struct unit_test_state *uts)
+{
+   struct abuf buf;
+   ulong start;
+
+   start = ut_check_free();
+
+   abuf_init();
+   ut_asserteq(0, buf.size);
+   ut_asserteq(false, buf.alloced);
+
+   abuf_realloc_inc(, 20);
+   ut_asserteq(20, buf.size);
+   ut_asserteq(true, buf.alloced);
+
+   abuf_uninit();
+
+   /* Check for memory leaks */
+   ut_assertok(ut_check_delta(start));
+
+   return 0;
+}
+LIB_TEST(lib_test_abuf_realloc_inc, 0);
+
 /* Test handling of buffers that are too large */
 static int lib_test_abuf_large(struct unit_test_state *uts)
 {
-- 
2.41.0.694.ge786442a9b-goog



[PATCH 01/19] expo: Make scene_obj_find() take a const scene

2023-08-14 Thread Simon Glass
This does not change the scene, so mark the pointer const.

Signed-off-by: Simon Glass 
---

 boot/scene.c  | 2 +-
 boot/scene_internal.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/boot/scene.c b/boot/scene.c
index e5271f9c..b4c36c41702f 100644
--- a/boot/scene.c
+++ b/boot/scene.c
@@ -79,7 +79,7 @@ int scene_obj_count(struct scene *scn)
return count;
 }
 
-void *scene_obj_find(struct scene *scn, uint id, enum scene_obj_t type)
+void *scene_obj_find(const struct scene *scn, uint id, enum scene_obj_t type)
 {
struct scene_obj *obj;
 
diff --git a/boot/scene_internal.h b/boot/scene_internal.h
index fb1ea5533b9a..1620d10a7778 100644
--- a/boot/scene_internal.h
+++ b/boot/scene_internal.h
@@ -38,7 +38,7 @@ uint resolve_id(struct expo *exp, uint id);
  * @type: Type of the object, or SCENEOBJT_NONE to match any type
  * Returns: Object found, or NULL if not found
  */
-void *scene_obj_find(struct scene *scn, uint id, enum scene_obj_t type);
+void *scene_obj_find(const struct scene *scn, uint id, enum scene_obj_t type);
 
 /**
  * scene_obj_find_by_name() - Find an object in a scene by name
-- 
2.41.0.694.ge786442a9b-goog



[PATCH 00/19] expo: Support a persistent configuration editor

2023-08-14 Thread Simon Glass
So far cedit does not support reading and writing the configuration.
This series add several features related to this:

First, it adds support for using a file on a filesystem. This is in
FDT format and provides enough information to reset the cedit back to
the saved settings.

Second, it adds support for using the U-Boot environment. Since the
environment is generally saved across reboots, this feature provides an
easy way of storing the state on most boards. The variables all have a
'c.' prefix to avoid confusion with other variables.

Finally it adds support for using CMOS RAM. This is commonly used on x86
devices to store BIOS settings. The expo schema provides information on
the register layout.

Some other minor tweaks and improvements are included along the way.


Simon Glass (19):
  expo: Make scene_obj_find() take a const scene
  abuf: Allow incrementing the size
  expo: Provide a way to iterate through all scene objects
  expo: Refactor menu_build() to return the object created
  expo: Split out cedit into its own header
  expo: Move cedit test into its own file and tidy
  expo: Add a function to prepare a cedit
  expo: Tidy up the expo.py tool and usage
  expo: Add documentation for the configuration editor
  expo: Move cedit theme under bootstd
  doc: Expand documentation for the cedit command
  expo: Export scene_menuitem_find() for use in internal code
  expo: cedit: Support writing settings to a file
  expo: cedit: Support reading settings from a file
  expo: cedit: Support writing settings to environment vars
  expo: cedit: Support reading settings from environment vars
  expo: cedit: Support writing settings to CMOS RAM
  expo: cedit: Support reading settings from CMOS RAM
  expo: doc: Update documentation for persistent settings

 arch/sandbox/dts/sandbox.dtsi   |  12 +-
 arch/sandbox/dts/test.dts   |  12 +-
 boot/cedit.c| 569 +++-
 boot/expo.c |  15 +
 boot/expo_build.c   |  22 +-
 boot/scene.c|  18 +-
 boot/scene_internal.h   |  50 ++-
 boot/scene_menu.c   |  20 +-
 cmd/cedit.c | 209 +++-
 doc/develop/cedit.rst   | 169 ++
 doc/develop/expo.rst|  48 ++-
 doc/develop/index.rst   |   1 +
 doc/usage/cmd/cedit.rst | 117 +++
 include/abuf.h  |   9 +
 include/cedit.h | 125 +++
 include/expo.h  |  33 +-
 lib/abuf.c  |   5 +
 test/boot/Makefile  |   1 +
 test/boot/cedit.c   | 198 +++
 test/boot/expo.c|  88 ++---
 test/boot/files/expo_ids.h  |  25 ++
 test/boot/files/expo_layout.dts |  28 +-
 test/lib/abuf.c |  25 ++
 test/py/tests/test_ut.py|   4 +-
 tools/expo.py   |  22 +-
 25 files changed, 1679 insertions(+), 146 deletions(-)
 create mode 100644 doc/develop/cedit.rst
 create mode 100644 include/cedit.h
 create mode 100644 test/boot/cedit.c
 create mode 100644 test/boot/files/expo_ids.h

-- 
2.41.0.694.ge786442a9b-goog



Re: [PATCH 12/17] ufs: Add Support for Qualcomm UFS HC driver

2023-08-14 Thread Marek Vasut

On 8/14/23 23:54, Bhupesh Sharma wrote:

Add Support for the Host Controller driver for UFS HC
present on Qualcomm Snapdragon SoCs.


[...]


+static void ufs_qcom_dev_ref_clk_ctrl(struct ufs_hba *hba, bool enable)
+{
+   struct ufs_qcom_priv *priv = dev_get_priv(hba->dev);
+
+   if (priv->dev_ref_clk_ctrl_mmio &&
+   (enable ^ priv->is_dev_ref_clk_enabled)) {


Invert condition to reduce indent.


+   u32 temp = readl_relaxed(priv->dev_ref_clk_ctrl_mmio);
+
+   if (enable)
+   temp |= priv->dev_ref_clk_en_mask;
+   else
+   temp &= ~priv->dev_ref_clk_en_mask;
+
+   /*
+* If we are here to disable this clock it might be immediately
+* after entering into hibern8 in which case we need to make
+* sure that device ref_clk is active for specific time after
+* hibern8 enter.
+*/
+   if (!enable)
+   udelay(10);
+
+   writel_relaxed(temp, priv->dev_ref_clk_ctrl_mmio);
+
+   /*
+* Make sure the write to ref_clk reaches the destination and
+* not stored in a Write Buffer (WB).
+*/
+   readl(priv->dev_ref_clk_ctrl_mmio);
+
+   /*
+* If we call hibern8 exit after this, we need to make sure that
+* device ref_clk is stable for at least 1us before the hibern8
+* exit command.
+*/
+   if (enable)
+   udelay(1);
+
+   priv->is_dev_ref_clk_enabled = enable;
+   }
+}


[...]


+static int ufs_qcom_check_hibern8(struct ufs_hba *hba)
+{
+   int err, retry_count = 50;
+   u32 tx_fsm_val = 0;
+
+   do {
+   err = ufshcd_dme_get(hba,
+   UIC_ARG_MIB_SEL(MPHY_TX_FSM_STATE,
+   UIC_ARG_MPHY_TX_GEN_SEL_INDEX(0)),
+   _fsm_val);
+   if (err || tx_fsm_val == TX_FSM_HIBERN8)
+   break;
+
+   /* max. 200us */
+   udelay(200);
+   retry_count--;
+   } while (retry_count != 0);


Is this some readx_poll_timeout() reimplementation ?


[PATCH] pci: pcie-brcmstb: bring over some robustness improvements from Linux

2023-08-14 Thread Sam Edwards
Since the initial U-Boot driver was ported here from Linux, the latter
has had a few changes for robustness/stability. This patch brings over
two of them:
- Do not attempt to access the configuration space of a PCIe device if
  the link has gone down, as that will result in an asynchronous SError
  interrupt which will crash U-Boot.
- Wait for the recommended 100ms after PERST# is deasserted.

I sent this patch while debugging a crash involving PCIe, but these
are unrelated improvements. I do not believe that this patch fixes any
real-world bug.

Signed-off-by: Sam Edwards 
---
 drivers/pci/pcie_brcmstb.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/pci/pcie_brcmstb.c b/drivers/pci/pcie_brcmstb.c
index 1de2802113..a159952822 100644
--- a/drivers/pci/pcie_brcmstb.c
+++ b/drivers/pci/pcie_brcmstb.c
@@ -223,6 +223,10 @@ static int brcm_pcie_config_address(const struct udevice 
*dev, pci_dev_t bdf,
return 0;
}
 
+   /* An access to our HW w/o link-up will cause a CPU Abort */
+   if (!brcm_pcie_link_up(pcie))
+   return -EINVAL;
+
/* For devices, write to the config space index register */
idx = PCIE_ECAM_OFFSET(pci_bus, pci_dev, pci_func, 0);
 
@@ -505,6 +509,12 @@ static int brcm_pcie_probe(struct udevice *dev)
clrbits_le32(pcie->base + PCIE_RGR1_SW_INIT_1,
 RGR1_SW_INIT_1_PERST_MASK);
 
+   /*
+* Wait for 100ms after PERST# deassertion; see PCIe CEM specification
+* sections 2.2, PCIe r5.0, 6.6.1.
+*/
+   mdelay(100);
+
/* Give the RC/EP time to wake up, before trying to configure RC.
 * Intermittently check status for link-up, up to a total of 100ms.
 */
-- 
2.41.0



Re: [PATCH 08/17] ufs: Expose 'ufshcd_ops_dbg_register_dump' vops to allow dumping debug registers

2023-08-14 Thread Marek Vasut

On 8/14/23 23:54, Bhupesh Sharma wrote:

Add more verbose debug capabilities and vops to allow dumping
UFS debug registers / regions, similar to how the UFS Linux driver
does it.

Signed-off-by: Bhupesh Sharma 
---
  drivers/ufs/ufs.c | 71 +++
  drivers/ufs/ufs.h |  9 ++
  2 files changed, 80 insertions(+)

diff --git a/drivers/ufs/ufs.c b/drivers/ufs/ufs.c
index b2c3af429e..15fa3832b9 100644
--- a/drivers/ufs/ufs.c
+++ b/drivers/ufs/ufs.c
@@ -59,10 +59,81 @@
  /* maximum bytes per request */
  #define UFS_MAX_BYTES (128 * 256 * 1024)
  
+#define ufshcd_hex_dump(prefix_str, buf, len) do {  \

+   size_t __len = (len);   \
+   print_hex_dump(prefix_str,  \
+  DUMP_PREFIX_OFFSET,  \
+  16, 4, buf, __len, false);   \
+} while (0)
+
  static inline bool ufshcd_is_hba_active(struct ufs_hba *hba);
  static inline void ufshcd_hba_stop(struct ufs_hba *hba);
  static int ufshcd_hba_enable(struct ufs_hba *hba);
  
+int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len,

+const char *prefix)
+{
+   u32 *regs;
+   size_t pos;
+
+   if (offset % 4 != 0 || len % 4 != 0) /* keep readl happy */
+   return -EINVAL;
+
+   regs = kzalloc(len, GFP_KERNEL);
+   if (!regs)
+   return -ENOMEM;
+
+   for (pos = 0; pos < len; pos += 4) {
+   if (offset == 0 &&
+   pos >= REG_UIC_ERROR_CODE_PHY_ADAPTER_LAYER &&
+   pos <= REG_UIC_ERROR_CODE_DME)
+   continue;
+   regs[pos / 4] = ufshcd_readl(hba, offset + pos);
+   }
+
+   ufshcd_hex_dump(prefix, regs, len);
+   kfree(regs);


Why not use variable on stack instead of this malloc-free cycle ?


Re: [PATCH 00/17] Enable UFS on DragonBoard845c

2023-08-14 Thread Marek Vasut

On 8/14/23 23:54, Bhupesh Sharma wrote:

This patchset enables the UFS controller on DragonBoard845c
board which houses Qualcomm SDM845 Snapdragon SoC.

In addition to enabling the UFS HC and UFS QMP PHY found
on this SoC this patchset also contains:
  * Patches to add 'reset' controller support for SDM845 SoC.
  * Minor UFS core framework fixes.
  * Patches to sync u-boot UFS driver flow with Linux UFS driver.
  * Patches which enable RESET, UFS and SCSI config options for
DragonBoard845c.


I submitted a couple of UFS related patches yesterday, they add handling 
for controllers which cannot handle 64bit memory and add support for 
cache management. Please rebase on top of those, otherwise there will be 
hard to resolve conflicts.


[PATCH 1/6] ufs: Add UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS
[PATCH 1/2] blk: Add bounce buffer support to read/write operations


Re: [PATCH 17/17] MAINTAINERS: Update UFS maintainer

2023-08-14 Thread Marek Vasut

On 8/14/23 23:54, Bhupesh Sharma wrote:

Since Faiz Abbas's email ID is no longer valid,
drop him from UFS Maintainer list.

Since I am using the UFS framework now extensively
on Qualcomm Snapdragon SoCs, so proposing myself as
the new UFS maintainer.

I have also been sending out u-boot UFS fixes in the
recent past.

Signed-off-by: Bhupesh Sharma 


I believe TI is working on something here, so please coordinate with them.


[PATCH 16/17] configs/dragonboard845c_defconfig: Enable UFS + SCSI related configs

2023-08-14 Thread Bhupesh Sharma
Signed-off-by: Bhupesh Sharma 
---
 configs/dragonboard845c_defconfig | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/configs/dragonboard845c_defconfig 
b/configs/dragonboard845c_defconfig
index e2676a37b5..0dd5ce6159 100644
--- a/configs/dragonboard845c_defconfig
+++ b/configs/dragonboard845c_defconfig
@@ -18,13 +18,23 @@ CONFIG_HUSH_PARSER=y
 CONFIG_SYS_MAXARGS=64
 CONFIG_SYS_CBSIZE=512
 CONFIG_CMD_GPIO=y
+CONFIG_CMD_PART=y
+CONFIG_CMD_UFS=y
+CONFIG_DOS_PARTITION=y
+CONFIG_EFI_PARTITION=y
 # CONFIG_NET is not set
 CONFIG_CLK=y
 CONFIG_MSM_GPIO=y
 CONFIG_QCOM_PMIC_GPIO=y
+CONFIG_PHY=y
+CONFIG_PHY_QCOM_QMP_UFS=y
 CONFIG_PINCTRL=y
 CONFIG_DM_PMIC=y
 CONFIG_PMIC_QCOM=y
+CONFIG_SCSI=y
+CONFIG_DM_SCSI=y
 CONFIG_MSM_GENI_SERIAL=y
 CONFIG_SPMI_MSM=y
+CONFIG_UFS=y
+CONFIG_QCOM_UFS=y
 CONFIG_LMB_MAX_REGIONS=64
-- 
2.38.1



[PATCH 17/17] MAINTAINERS: Update UFS maintainer

2023-08-14 Thread Bhupesh Sharma
Since Faiz Abbas's email ID is no longer valid,
drop him from UFS Maintainer list.

Since I am using the UFS framework now extensively
on Qualcomm Snapdragon SoCs, so proposing myself as
the new UFS maintainer.

I have also been sending out u-boot UFS fixes in the
recent past.

Signed-off-by: Bhupesh Sharma 
---
 MAINTAINERS | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 2db052961b..3b2acb5167 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1584,7 +1584,7 @@ T:git 
https://source.denx.de/u-boot/custodians/u-boot-ubi.git
 F: drivers/mtd/ubi/
 
 UFS
-M: Faiz Abbas 
+M: Bhupesh Sharma 
 S: Maintained
 F: drivers/ufs/
 
-- 
2.38.1



[PATCH 15/17] configs/dragonboard845c_defconfig: Enable DM_RESET by default

2023-08-14 Thread Bhupesh Sharma
Signed-off-by: Bhupesh Sharma 
---
 configs/dragonboard845c_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/dragonboard845c_defconfig 
b/configs/dragonboard845c_defconfig
index a69d82761a..e2676a37b5 100644
--- a/configs/dragonboard845c_defconfig
+++ b/configs/dragonboard845c_defconfig
@@ -4,6 +4,7 @@ CONFIG_COUNTER_FREQUENCY=1900
 CONFIG_POSITION_INDEPENDENT=y
 CONFIG_ARCH_SNAPDRAGON=y
 CONFIG_DEFAULT_DEVICE_TREE="dragonboard845c"
+CONFIG_DM_RESET=y
 CONFIG_TARGET_DRAGONBOARD845C=y
 CONFIG_IDENT_STRING="\nQualcomm-DragonBoard 845C"
 CONFIG_SYS_LOAD_ADDR=0x8000
-- 
2.38.1



[PATCH 14/17] arm: dts: qcom: sdm845: Add UFS HC and PHY nodes

2023-08-14 Thread Bhupesh Sharma
Add UFS HC and PHY nodes in Qualcomm SDM845 dtsi.

Signed-off-by: Bhupesh Sharma 
---
 arch/arm/dts/sdm845.dtsi | 62 
 1 file changed, 62 insertions(+)

diff --git a/arch/arm/dts/sdm845.dtsi b/arch/arm/dts/sdm845.dtsi
index 056a1674d5..9ef41e3c84 100644
--- a/arch/arm/dts/sdm845.dtsi
+++ b/arch/arm/dts/sdm845.dtsi
@@ -30,6 +30,68 @@
compatible = "qcom,gcc-reset-sdm845";
reg = <0x0010 0x1f>;
#reset-cells = <1>;
+
+   ufs_mem_hc: ufshc@1d84000 {
+   compatible = "qcom,sdm845-ufshc", "qcom,ufshc",
+"jedec,ufs-2.0";
+   reg = <0x01d84000 0x2500>;
+   phys = <_mem_phy_lanes>;
+   phy-names = "ufsphy";
+   lanes-per-direction = <2>;
+   #reset-cells = <1>;
+   resets = < GCC_UFS_PHY_BCR>;
+   reset-names = "rst";
+
+   power-domains = < UFS_PHY_GDSC>;
+
+   clock-names =
+   "core_clk",
+   "bus_aggr_clk",
+   "iface_clk",
+   "core_clk_unipro",
+   "tx_lane0_sync_clk",
+   "rx_lane0_sync_clk",
+   "rx_lane1_sync_clk",
+   "ice_core_clk";
+   clocks =
+   < GCC_UFS_PHY_AXI_CLK>,
+   < GCC_AGGRE_UFS_PHY_AXI_CLK>,
+   < GCC_UFS_PHY_AHB_CLK>,
+   < GCC_UFS_PHY_UNIPRO_CORE_CLK>,
+   < GCC_UFS_PHY_TX_SYMBOL_0_CLK>,
+   < GCC_UFS_PHY_RX_SYMBOL_0_CLK>,
+   < GCC_UFS_PHY_RX_SYMBOL_1_CLK>,
+   < GCC_UFS_PHY_ICE_CORE_CLK>;
+   freq-table-hz =
+   <5000 2>,
+   <0 0>,
+   <0 0>,
+   <3750 15000>,
+   <0 0>,
+   <0 0>,
+   <0 0>,
+   <0 3>;
+   };
+
+   ufs_mem_phy: phy@1d87000 {
+   compatible = "qcom,sdm845-qmp-ufs-phy";
+   reg = <0x01d87000 0x18c>;
+   #address-cells = <1>;
+   #size-cells = <1>;
+   ranges;
+   clock-names = "ref",
+ "ref_aux";
+   clocks = < GCC_UFS_MEM_CLKREF_CLK>,
+< GCC_UFS_PHY_PHY_AUX_CLK>;
+
+   ufs_mem_phy_lanes: phy@1d87400 {
+   reg = <0x01d87400 0x108>,
+ <0x01d87600 0x1e0>,
+ <0x01d87c00 0x1dc>,
+ <0x01d87800 0x108>,
+ <0x01d87a00 0x1e0>;
+   #phy-cells = <0>;
+   };
};
 
gpio_north: gpio_north@390 {
-- 
2.38.1



[PATCH 13/17] arm: dts: qcom: sdm845: Add 'reset' node

2023-08-14 Thread Bhupesh Sharma
Add 'reset' controller node in Qualcomm SDM845 dtsi.

Signed-off-by: Bhupesh Sharma 
---
 arch/arm/dts/sdm845.dtsi | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm/dts/sdm845.dtsi b/arch/arm/dts/sdm845.dtsi
index 3b86b9328f..056a1674d5 100644
--- a/arch/arm/dts/sdm845.dtsi
+++ b/arch/arm/dts/sdm845.dtsi
@@ -26,6 +26,12 @@
#power-domain-cells = <1>;
};
 
+   reset: gcc-reset@10 {
+   compatible = "qcom,gcc-reset-sdm845";
+   reg = <0x0010 0x1f>;
+   #reset-cells = <1>;
+   };
+
gpio_north: gpio_north@390 {
#gpio-cells = <2>;
compatible = "qcom,sdm845-pinctrl";
-- 
2.38.1



[PATCH 12/17] ufs: Add Support for Qualcomm UFS HC driver

2023-08-14 Thread Bhupesh Sharma
Add Support for the Host Controller driver for UFS HC
present on Qualcomm Snapdragon SoCs.

Signed-off-by: Bhupesh Sharma 
---
 drivers/ufs/Kconfig   |   7 +
 drivers/ufs/Makefile  |   1 +
 drivers/ufs/qcom-ufshcd.c | 880 ++
 drivers/ufs/ufs-qcom.h| 275 
 4 files changed, 1163 insertions(+)
 create mode 100644 drivers/ufs/qcom-ufshcd.c
 create mode 100644 drivers/ufs/ufs-qcom.h

diff --git a/drivers/ufs/Kconfig b/drivers/ufs/Kconfig
index 69ea18edf8..5dfe8048a8 100644
--- a/drivers/ufs/Kconfig
+++ b/drivers/ufs/Kconfig
@@ -15,6 +15,13 @@ config CADENCE_UFS
  This selects the platform driver for the Cadence UFS host
  controller present on present TI's J721e devices.
 
+config QCOM_UFS
+   bool "Qualcomm Host Controller driver for UFS"
+   depends on UFS && ARCH_SNAPDRAGON
+help
+ This selects the platform driver for the UFS host
+ controller present on Qualcomm Snapdragon SoCs.
+
 config TI_J721E_UFS
bool "Glue Layer driver for UFS on TI J721E devices"
help
diff --git a/drivers/ufs/Makefile b/drivers/ufs/Makefile
index 62ed016608..87944b1a7a 100644
--- a/drivers/ufs/Makefile
+++ b/drivers/ufs/Makefile
@@ -5,4 +5,5 @@
 
 obj-$(CONFIG_UFS) += ufs.o ufs-uclass.o
 obj-$(CONFIG_CADENCE_UFS) += cdns-platform.o
+obj-$(CONFIG_QCOM_UFS) += qcom-ufshcd.o
 obj-$(CONFIG_TI_J721E_UFS) += ti-j721e-ufs.o
diff --git a/drivers/ufs/qcom-ufshcd.c b/drivers/ufs/qcom-ufshcd.c
new file mode 100644
index 00..ca55c3e8f5
--- /dev/null
+++ b/drivers/ufs/qcom-ufshcd.c
@@ -0,0 +1,880 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2023 Bhupesh Sharma 
+ *
+ * Based on Linux driver
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+
+#include "ufs.h"
+#include "ufs-qcom.h"
+
+#define MSEC_PER_SEC   (1000L)
+#define USEC_PER_SEC   (100L)
+#define NSEC_PER_SEC   (10L)
+
+static int ufs_qcom_set_dme_vs_core_clk_ctrl_clear_div(struct ufs_hba *hba,
+  u32 clk_cycles);
+
+static int ufs_qcom_clk_get(struct udevice *dev,
+   const char *name, struct clk **clk_out, bool optional)
+{
+   struct clk *clk;
+   int err = 0;
+
+   clk = devm_clk_get(dev, name);
+   if (!IS_ERR(clk)) {
+   *clk_out = clk;
+   return 0;
+   }
+
+   err = PTR_ERR(clk);
+
+   if (optional && err == -ENOENT) {
+   *clk_out = NULL;
+   return 0;
+   }
+
+   if (err != -EPROBE_DEFER)
+   dev_err(dev, "failed to get %s err %d\n", name, err);
+
+   return err;
+}
+
+static int ufs_qcom_clk_enable(struct udevice *dev,
+   const char *name, struct clk *clk)
+{
+   int err = 0;
+
+   err = clk_prepare_enable(clk);
+   if (err)
+   dev_err(dev, "%s: %s enable failed %d\n", __func__, name, err);
+
+   return err;
+}
+
+static int ufs_qcom_enable_lane_clks(struct ufs_qcom_priv *priv)
+{
+   int err;
+   struct udevice *dev = priv->hba->dev;
+
+   if (priv->is_lane_clks_enabled)
+   return 0;
+
+   err = ufs_qcom_clk_enable(dev, "rx_lane0_sync_clk",
+   priv->rx_l0_sync_clk);
+   if (err)
+   return err;
+
+   err = ufs_qcom_clk_enable(dev, "tx_lane0_sync_clk",
+   priv->tx_l0_sync_clk);
+   if (err)
+   goto disable_rx_l0;
+
+   err = ufs_qcom_clk_enable(dev, "rx_lane1_sync_clk",
+   priv->rx_l1_sync_clk);
+   if (err)
+   goto disable_tx_l0;
+
+   priv->is_lane_clks_enabled = true;
+
+   return 0;
+
+disable_tx_l0:
+   clk_disable_unprepare(priv->tx_l0_sync_clk);
+disable_rx_l0:
+   clk_disable_unprepare(priv->rx_l0_sync_clk);
+
+   return err;
+}
+
+static int ufs_qcom_init_lane_clks(struct ufs_qcom_priv *priv)
+{
+   int err = 0;
+   struct udevice *dev = priv->hba->dev;
+
+   err = ufs_qcom_clk_get(dev, "rx_lane0_sync_clk",
+   >rx_l0_sync_clk, false);
+   if (err)
+   return err;
+
+   err = ufs_qcom_clk_get(dev, "tx_lane0_sync_clk",
+   >tx_l0_sync_clk, false);
+   if (err)
+   return err;
+
+   err = ufs_qcom_clk_get(dev, "rx_lane1_sync_clk",
+   >rx_l1_sync_clk, false);
+   if (err)
+   return err;
+
+   return 0;
+}
+
+static int ufs_qcom_enable_core_clks(struct ufs_qcom_priv *priv)
+{
+   int err;
+   struct udevice *dev = priv->hba->dev;
+
+   if (priv->is_core_clks_enabled)
+   return 0;
+
+   err = ufs_qcom_clk_enable(dev, "core_clk", priv->core_clk);
+   if (err)
+   return err;
+
+   err = ufs_qcom_clk_enable(dev, "bus_aggr_clk", priv->bus_aggr_clk);
+   if (err)
+   

[PATCH 11/17] ufs: Fix debug message in 'ufs_start'

2023-08-14 Thread Bhupesh Sharma
Minor typo fix and rewording of printf message
inside 'ufs_start' which announces the availability
of the UFS device.

Signed-off-by: Bhupesh Sharma 
---
 drivers/ufs/ufs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/ufs/ufs.c b/drivers/ufs/ufs.c
index a97c45a530..8e507e8378 100644
--- a/drivers/ufs/ufs.c
+++ b/drivers/ufs/ufs.c
@@ -1937,7 +1937,7 @@ int ufs_start(struct ufs_hba *hba)
return ret;
}
 
-   printf("Device at %s up at:", hba->dev->name);
+   printf("UFS Device %s is up!\n", hba->dev->name);
ufshcd_print_pwr_info(hba);
}
 
-- 
2.38.1



[PATCH 10/17] ufs: Add missing memory barriers

2023-08-14 Thread Bhupesh Sharma
Add missing wmb() and mb() barriers in the u-boot UFS core
framework driver to allow registers updates to happen before
follow-up read operations.

This makes the barrier placement similar to the Linux UFS driver.

Signed-off-by: Bhupesh Sharma 
---
 drivers/ufs/ufs.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/ufs/ufs.c b/drivers/ufs/ufs.c
index 15fa3832b9..a97c45a530 100644
--- a/drivers/ufs/ufs.c
+++ b/drivers/ufs/ufs.c
@@ -503,6 +503,12 @@ static int ufshcd_make_hba_operational(struct ufs_hba *hba)
ufshcd_writel(hba, upper_32_bits((dma_addr_t)hba->utmrdl),
  REG_UTP_TASK_REQ_LIST_BASE_H);
 
+   /*
+* Make sure base address and interrupt setup are updated before
+* enabling the run/stop registers below.
+*/
+   wmb();
+
/*
 * UCRDY, UTMRLDY and UTRLRDY bits must be 1
 */
@@ -892,6 +898,9 @@ static int ufshcd_send_command(struct ufs_hba *hba, 
unsigned int task_tag)
 
ufshcd_writel(hba, 1 << task_tag, REG_UTP_TRANSFER_REQ_DOOR_BELL);
 
+   /* Make sure doorbell reg is updated before reading interrupt status */
+   wmb();
+
start = get_timer(0);
do {
intr_status = ufshcd_readl(hba, REG_INTERRUPT_STATUS);
@@ -1989,6 +1998,8 @@ int ufshcd_probe(struct udevice *ufs_dev, struct 
ufs_hba_ops *hba_ops)
  REG_INTERRUPT_STATUS);
ufshcd_writel(hba, 0, REG_INTERRUPT_ENABLE);
 
+   mb();
+
err = ufshcd_hba_enable(hba);
if (err) {
dev_err(hba->dev, "Host controller enable failed\n");
-- 
2.38.1



[PATCH 09/17] ufs: Sync possible UFS Quirks with Linux UFS driver

2023-08-14 Thread Bhupesh Sharma
Sync u-boot UFS driver to add all possible UFS Quirks
as supported by Linux UFS driver as well.

Signed-off-by: Bhupesh Sharma 
---
 drivers/ufs/ufs.h | 129 +++---
 1 file changed, 121 insertions(+), 8 deletions(-)

diff --git a/drivers/ufs/ufs.h b/drivers/ufs/ufs.h
index b3d2bd0368..cdc88919b4 100644
--- a/drivers/ufs/ufs.h
+++ b/drivers/ufs/ufs.h
@@ -712,14 +712,127 @@ struct ufs_hba {
u32 version;
u32 intr_mask;
u32 quirks;
-/*
- * If UFS host controller is having issue in processing LCC (Line
- * Control Command) coming from device then enable this quirk.
- * When this quirk is enabled, host controller driver should disable
- * the LCC transmission on UFS device (by clearing TX_LCC_ENABLE
- * attribute of device to 0).
- */
-#define UFSHCD_QUIRK_BROKEN_LCC0x1
+
+   /* Interrupt aggregation support is broken */
+#define UFSHCD_QUIRK_BROKEN_INTR_AGGR  (1 << 0)
+
+   /*
+* delay before each dme command is required as the unipro
+* layer has shown instabilities
+*/
+#define UFSHCD_QUIRK_DELAY_BEFORE_DME_CMDS (1 << 1)
+
+   /*
+* If UFS host controller is having issue in processing LCC (Line
+* Control Command) coming from device then enable this quirk.
+* When this quirk is enabled, host controller driver should disable
+* the LCC transmission on UFS device (by clearing TX_LCC_ENABLE
+* attribute of device to 0).
+*/
+#define UFSHCD_QUIRK_BROKEN_LCC(1 << 2)
+
+   /*
+* The attribute PA_RXHSUNTERMCAP specifies whether or not the
+* inbound Link supports unterminated line in HS mode. Setting this
+* attribute to 1 fixes moving to HS gear.
+*/
+#define UFSHCD_QUIRK_BROKEN_PA_RXHSUNTERMCAP   (1 << 3)
+
+   /*
+* This quirk needs to be enabled if the host controller only allows
+* accessing the peer dme attributes in AUTO mode (FAST AUTO or
+* SLOW AUTO).
+*/
+#define UFSHCD_QUIRK_DME_PEER_ACCESS_AUTO_MODE (1 << 4)
+
+   /*
+* This quirk needs to be enabled if the host controller doesn't
+* advertise the correct version in UFS_VER register. If this quirk
+* is enabled, standard UFS host driver will call the vendor specific
+* ops (get_ufs_hci_version) to get the correct version.
+*/
+#define UFSHCD_QUIRK_BROKEN_UFS_HCI_VERSION(1 << 5)
+
+   /*
+* Clear handling for transfer/task request list is just opposite.
+*/
+#define UFSHCI_QUIRK_BROKEN_REQ_LIST_CLR   (1 << 6)
+
+   /*
+* This quirk needs to be enabled if host controller doesn't allow
+* that the interrupt aggregation timer and counter are reset by s/w.
+*/
+#define UFSHCI_QUIRK_SKIP_RESET_INTR_AGGR  (1 << 7)
+
+   /*
+* This quirks needs to be enabled if host controller cannot be
+* enabled via HCE register.
+*/
+#define UFSHCI_QUIRK_BROKEN_HCE(1 << 8)
+
+   /*
+* This quirk needs to be enabled if the host controller regards
+* resolution of the values of PRDTO and PRDTL in UTRD as byte.
+*/
+#define UFSHCD_QUIRK_PRDT_BYTE_GRAN(1 << 9)
+
+   /*
+* This quirk needs to be enabled if the host controller reports
+* OCS FATAL ERROR with device error through sense data
+*/
+#define UFSHCD_QUIRK_BROKEN_OCS_FATAL_ERROR(1 << 10)
+
+   /*
+* This quirk needs to be enabled if the host controller has
+* auto-hibernate capability but it doesn't work.
+*/
+#define UFSHCD_QUIRK_BROKEN_AUTO_HIBERN8   (1 << 11)
+
+   /*
+* This quirk needs to disable manual flush for write booster
+*/
+#define UFSHCI_QUIRK_SKIP_MANUAL_WB_FLUSH_CTRL (1 << 12)
+
+   /*
+* This quirk needs to disable unipro timeout values
+* before power mode change
+*/
+#define UFSHCD_QUIRK_SKIP_DEF_UNIPRO_TIMEOUT_SETTING   (1 << 13)
+
+   /*
+* Align DMA SG entries on a 4 KiB boundary.
+*/
+#define UFSHCD_QUIRK_4KB_DMA_ALIGNMENT (1 << 14)
+
+   /*
+* This quirk needs to be enabled if the host controller does not
+* support UIC command
+*/
+#define UFSHCD_QUIRK_BROKEN_UIC_CMD(1 << 15)
+
+   /*
+* This quirk needs to be enabled if the host controller cannot
+* support physical host configuration.
+*/
+#define UFSHCD_QUIRK_SKIP_PH_CONFIGURATION (1 << 16)
+
+   /*
+* This quirk needs to be enabled if the host controller has
+* 64-bit addressing supported capability but it doesn't work.
+*/
+#define UFSHCD_QUIRK_BROKEN_64BIT_ADDRESS  (1 << 17)
+
+   /*
+* This quirk needs to be enabled if the 

[PATCH 08/17] ufs: Expose 'ufshcd_ops_dbg_register_dump' vops to allow dumping debug registers

2023-08-14 Thread Bhupesh Sharma
Add more verbose debug capabilities and vops to allow dumping
UFS debug registers / regions, similar to how the UFS Linux driver
does it.

Signed-off-by: Bhupesh Sharma 
---
 drivers/ufs/ufs.c | 71 +++
 drivers/ufs/ufs.h |  9 ++
 2 files changed, 80 insertions(+)

diff --git a/drivers/ufs/ufs.c b/drivers/ufs/ufs.c
index b2c3af429e..15fa3832b9 100644
--- a/drivers/ufs/ufs.c
+++ b/drivers/ufs/ufs.c
@@ -59,10 +59,81 @@
 /* maximum bytes per request */
 #define UFS_MAX_BYTES  (128 * 256 * 1024)
 
+#define ufshcd_hex_dump(prefix_str, buf, len) do {  \
+   size_t __len = (len);   \
+   print_hex_dump(prefix_str,  \
+  DUMP_PREFIX_OFFSET,  \
+  16, 4, buf, __len, false);   \
+} while (0)
+
 static inline bool ufshcd_is_hba_active(struct ufs_hba *hba);
 static inline void ufshcd_hba_stop(struct ufs_hba *hba);
 static int ufshcd_hba_enable(struct ufs_hba *hba);
 
+int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len,
+const char *prefix)
+{
+   u32 *regs;
+   size_t pos;
+
+   if (offset % 4 != 0 || len % 4 != 0) /* keep readl happy */
+   return -EINVAL;
+
+   regs = kzalloc(len, GFP_KERNEL);
+   if (!regs)
+   return -ENOMEM;
+
+   for (pos = 0; pos < len; pos += 4) {
+   if (offset == 0 &&
+   pos >= REG_UIC_ERROR_CODE_PHY_ADAPTER_LAYER &&
+   pos <= REG_UIC_ERROR_CODE_DME)
+   continue;
+   regs[pos / 4] = ufshcd_readl(hba, offset + pos);
+   }
+
+   ufshcd_hex_dump(prefix, regs, len);
+   kfree(regs);
+
+   return 0;
+}
+
+void ufshcd_print_tr(struct ufs_hba *hba, int tag, bool pr_prdt)
+{
+   int prdt_length;
+   struct utp_transfer_req_desc *req_desc = hba->utrdl;
+
+   dev_info(hba->dev,
+   "UPIU[%d] - Transfer Request Descriptor phys@0x%llx\n",
+   tag, (u64)hba->utrdl);
+
+   ufshcd_hex_dump("UPIU TRD: ", hba->utrdl,
+   sizeof(struct utp_transfer_req_desc));
+   dev_info(hba->dev, "UPIU[%d] - Request UPIU phys@0x%llx\n", tag,
+   (u64)hba->ucd_req_ptr);
+   ufshcd_hex_dump("UPIU REQ: ", hba->ucd_req_ptr,
+   sizeof(struct utp_upiu_req));
+   dev_info(hba->dev, "UPIU[%d] - Response UPIU phys@0x%llx\n", tag,
+   (u64)hba->ucd_rsp_ptr);
+   ufshcd_hex_dump("UPIU RSP: ", hba->ucd_rsp_ptr,
+   sizeof(struct utp_upiu_rsp));
+
+   prdt_length = le16_to_cpu(req_desc->prd_table_length);
+
+   dev_info(hba->dev,
+   "UPIU[%d] - PRDT - %d entries  phys@0x%llx\n",
+   tag, prdt_length,
+   (u64)hba->ucd_prdt_ptr);
+
+   if (pr_prdt)
+   ufshcd_hex_dump("UPIU PRDT: ", hba->ucd_prdt_ptr,
+   sizeof(struct ufshcd_sg_entry) * prdt_length);
+}
+
+void ufshcd_dbg_register_dump(struct ufs_hba *hba)
+{
+   ufshcd_ops_dbg_register_dump(hba);
+}
+
 /*
  * ufshcd_wait_for_register - wait for register value to change
  */
diff --git a/drivers/ufs/ufs.h b/drivers/ufs/ufs.h
index c9320a905e..b3d2bd0368 100644
--- a/drivers/ufs/ufs.h
+++ b/drivers/ufs/ufs.h
@@ -695,6 +695,7 @@ struct ufs_dev_cmd {
 
 struct ufs_hba_ops {
int (*init)(struct ufs_hba *hba);
+   void (*dbg_register_dump)(struct ufs_hba *hba);
int (*hce_enable_notify)(struct ufs_hba *hba,
 enum ufs_notify_change_status);
int (*link_startup_notify)(struct ufs_hba *hba,
@@ -746,6 +747,12 @@ static inline int ufshcd_ops_init(struct ufs_hba *hba)
return 0;
 }
 
+static inline void ufshcd_ops_dbg_register_dump(struct ufs_hba *hba)
+{
+   if (hba->ops && hba->ops->dbg_register_dump)
+   hba->ops->dbg_register_dump(hba);
+}
+
 static inline int ufshcd_ops_hce_enable_notify(struct ufs_hba *hba,
bool status)
 {
@@ -931,5 +938,7 @@ static inline void ufshcd_rmwl(struct ufs_hba *hba, u32 
mask, u32 val, u32 reg)
 #define UTP_TASK_REQ_LIST_RUN_STOP_BIT 0x1
 
 int ufshcd_probe(struct udevice *dev, struct ufs_hba_ops *hba_ops);
+int ufshcd_dump_regs(struct ufs_hba *hba, size_t offset, size_t len,
+const char *prefix);
 
 #endif
-- 
2.38.1



[PATCH 07/17] ufs: Add support for probing UFS controllers newer than UFSHCI_VERSION_21

2023-08-14 Thread Bhupesh Sharma
UFS Host Controllers on Qualcomm Snapdragon SoCs support versions
newer/ greater than UFSHCI_VERSION_21. So, modify the driver
to just print the UFS HC version and not bail-out for newer versions.

Signed-off-by: Bhupesh Sharma 
---
 drivers/ufs/ufs.c | 7 +--
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/ufs/ufs.c b/drivers/ufs/ufs.c
index 25639a6d24..b2c3af429e 100644
--- a/drivers/ufs/ufs.c
+++ b/drivers/ufs/ufs.c
@@ -1894,12 +1894,7 @@ int ufshcd_probe(struct udevice *ufs_dev, struct 
ufs_hba_ops *hba_ops)
 
/* Get UFS version supported by the controller */
hba->version = ufshcd_get_ufs_version(hba);
-   if (hba->version != UFSHCI_VERSION_10 &&
-   hba->version != UFSHCI_VERSION_11 &&
-   hba->version != UFSHCI_VERSION_20 &&
-   hba->version != UFSHCI_VERSION_21)
-   dev_err(hba->dev, "invalid UFS version 0x%x\n",
-   hba->version);
+   dev_info(hba->dev, "UFS version : 0x%x\n", hba->version);
 
/* Get Interrupt bit mask per version */
hba->intr_mask = ufshcd_get_intr_mask(hba);
-- 
2.38.1



[PATCH 06/17] ufs: Clear UECPA once due to LINERESET has happened during LINK_STARTUP

2023-08-14 Thread Bhupesh Sharma
Clear UECPA once in u-boot UFS driver due to LINERESET has happened
during LINK_STARTUP. This makes the u-boot ufs driver behavior related
to UECPA similar to Linux UFS driver.

Signed-off-by: Bhupesh Sharma 
---
 drivers/ufs/ufs.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/ufs/ufs.c b/drivers/ufs/ufs.c
index 3bf1a95e7f..25639a6d24 100644
--- a/drivers/ufs/ufs.c
+++ b/drivers/ufs/ufs.c
@@ -504,6 +504,8 @@ link_startup:
if (ret)
goto out;
 
+   /* Clear UECPA once due to LINERESET has happened during LINK_STARTUP */
+   ufshcd_readl(hba, REG_UIC_ERROR_CODE_PHY_ADAPTER_LAYER);
ret = ufshcd_make_hba_operational(hba);
 out:
if (ret)
-- 
2.38.1



[PATCH 05/17] ufs/ufs.h: Add definition of 'ufshcd_rmwl()'

2023-08-14 Thread Bhupesh Sharma
Add definition of 'ufshcd_rmwl()' helper function
which would be later used by Qualcomm UFS driver
to read-modify-write registers.

Signed-off-by: Bhupesh Sharma 
---
 drivers/ufs/ufs.h | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/drivers/ufs/ufs.h b/drivers/ufs/ufs.h
index 8a38832b05..c9320a905e 100644
--- a/drivers/ufs/ufs.h
+++ b/drivers/ufs/ufs.h
@@ -2,6 +2,7 @@
 #ifndef __UFS_H
 #define __UFS_H
 
+#include 
 #include "unipro.h"
 
 struct udevice;
@@ -906,6 +907,23 @@ enum {
 #define ufshcd_readl(hba, reg) \
readl((hba)->mmio_base + (reg))
 
+/**
+ * ufshcd_rmwl - perform read/modify/write for a controller register
+ * @hba: per adapter instance
+ * @mask: mask to apply on read value
+ * @val: actual value to write
+ * @reg: register address
+ */
+static inline void ufshcd_rmwl(struct ufs_hba *hba, u32 mask, u32 val, u32 reg)
+{
+   u32 tmp;
+
+   tmp = ufshcd_readl(hba, reg);
+   tmp &= ~mask;
+   tmp |= (val & mask);
+   ufshcd_writel(hba, tmp, reg);
+}
+
 /* UTRLRSR - UTP Transfer Request Run-Stop Register 60h */
 #define UTP_TRANSFER_REQ_LIST_RUN_STOP_BIT 0x1
 
-- 
2.38.1



[PATCH 04/17] phy: qcom: Add QMP UFS PHY driver

2023-08-14 Thread Bhupesh Sharma
Add Qualcomm QMP UFS PHY driver which is available on the following
Snapdragon SoCs - SDM845, SM6115 and SM8250 SoCs.

Signed-off-by: Bhupesh Sharma 
---
 drivers/phy/qcom/Kconfig|   6 +
 drivers/phy/qcom/Makefile   |   1 +
 drivers/phy/qcom/phy-qcom-qmp-ufs.c | 996 
 drivers/phy/qcom/phy-qcom-qmp.h | 115 
 4 files changed, 1118 insertions(+)
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-ufs.c
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp.h

diff --git a/drivers/phy/qcom/Kconfig b/drivers/phy/qcom/Kconfig
index f4ca174805..38536566ca 100644
--- a/drivers/phy/qcom/Kconfig
+++ b/drivers/phy/qcom/Kconfig
@@ -12,6 +12,12 @@ config PHY_QCOM_IPQ4019_USB
help
  Support for the USB PHY-s on Qualcomm IPQ40xx SoC-s.
 
+config PHY_QCOM_QMP_UFS
+   tristate "Qualcomm QMP UFS PHY driver"
+   depends on PHY && ARCH_SNAPDRAGON
+   help
+ Enable this to support the UFS QMP PHY on various Qualcomm chipsets.
+
 config PHY_QCOM_USB_HS_28NM
tristate "Qualcomm 28nm High-Speed PHY"
depends on PHY && ARCH_SNAPDRAGON
diff --git a/drivers/phy/qcom/Makefile b/drivers/phy/qcom/Makefile
index 2113f178c0..e804f127b0 100644
--- a/drivers/phy/qcom/Makefile
+++ b/drivers/phy/qcom/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_PHY_QCOM_IPQ4019_USB) += phy-qcom-ipq4019-usb.o
 obj-$(CONFIG_MSM8916_USB_PHY) += msm8916-usbh-phy.o
+obj-$(CONFIG_PHY_QCOM_QMP_UFS) += phy-qcom-qmp-ufs.o
 obj-$(CONFIG_PHY_QCOM_USB_HS_28NM) += phy-qcom-usb-hs-28nm.o
 obj-$(CONFIG_PHY_QCOM_USB_SS) += phy-qcom-usb-ss.o
diff --git a/drivers/phy/qcom/phy-qcom-qmp-ufs.c 
b/drivers/phy/qcom/phy-qcom-qmp-ufs.c
new file mode 100644
index 00..16e95175bb
--- /dev/null
+++ b/drivers/phy/qcom/phy-qcom-qmp-ufs.c
@@ -0,0 +1,996 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2023 Bhupesh Sharma 
+ *
+ * Based on Linux driver
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "phy-qcom-qmp.h"
+#include "phy-qcom-qmp-pcs-ufs-v2.h"
+#include "phy-qcom-qmp-pcs-ufs-v3.h"
+#include "phy-qcom-qmp-pcs-ufs-v4.h"
+#include "phy-qcom-qmp-pcs-ufs-v5.h"
+#include "phy-qcom-qmp-pcs-ufs-v6.h"
+
+#include "phy-qcom-qmp-qserdes-com-v4.h"
+#include "phy-qcom-qmp-qserdes-txrx-v4.h"
+#include "phy-qcom-qmp-qserdes-txrx-ufs-v6.h"
+
+/* QPHY_SW_RESET bit */
+#define SW_RESET   BIT(0)
+/* QPHY_POWER_DOWN_CONTROL */
+#define SW_PWRDN   BIT(0)
+/* QPHY_START_CONTROL bits */
+#define SERDES_START   BIT(0)
+#define PCS_START  BIT(1)
+/* QPHY_PCS_READY_STATUS bit */
+#define PCS_READY  BIT(0)
+
+#define PHY_INIT_COMPLETE_TIMEOUT  (200 * 1)
+
+struct qmp_ufs_init_tbl {
+   unsigned int offset;
+   unsigned int val;
+   /*
+* mask of lanes for which this register is written
+* for cases when second lane needs different values
+*/
+   u8 lane_mask;
+};
+
+#define QMP_PHY_INIT_CFG(o, v) \
+   {   \
+   .offset = o,\
+   .val = v,   \
+   .lane_mask = 0xff,  \
+   }
+
+#define QMP_PHY_INIT_CFG_LANE(o, v, l) \
+   {   \
+   .offset = o,\
+   .val = v,   \
+   .lane_mask = l, \
+   }
+
+enum ufs_hs_gear_tag {
+   UFS_HS_DONT_CHANGE, /* Don't change Gear */
+   UFS_HS_G1,  /* HS Gear 1 (default for reset) */
+   UFS_HS_G2,  /* HS Gear 2 */
+   UFS_HS_G3,  /* HS Gear 3 */
+   UFS_HS_G4,  /* HS Gear 4 */
+   UFS_HS_G5   /* HS Gear 5 */
+};
+
+/* set of registers with offsets different per-PHY */
+enum qphy_reg_layout {
+   /* PCS registers */
+   QPHY_SW_RESET,
+   QPHY_START_CTRL,
+   QPHY_PCS_READY_STATUS,
+   QPHY_PCS_POWER_DOWN_CONTROL,
+   /* Keep last to ensure regs_layout arrays are properly initialized */
+   QPHY_LAYOUT_SIZE
+};
+
+static const unsigned int ufsphy_v2_regs_layout[QPHY_LAYOUT_SIZE] = {
+   [QPHY_START_CTRL]   = QPHY_V2_PCS_UFS_PHY_START,
+   [QPHY_PCS_READY_STATUS] = QPHY_V2_PCS_UFS_READY_STATUS,
+   [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V2_PCS_UFS_POWER_DOWN_CONTROL,
+};
+
+static const unsigned int ufsphy_v3_regs_layout[QPHY_LAYOUT_SIZE] = {
+   [QPHY_START_CTRL]   = QPHY_V3_PCS_UFS_PHY_START,
+   [QPHY_PCS_READY_STATUS] = QPHY_V3_PCS_UFS_READY_STATUS,
+   [QPHY_PCS_POWER_DOWN_CONTROL]   = QPHY_V3_PCS_UFS_POWER_DOWN_CONTROL,
+};
+
+static const unsigned int ufsphy_v4_regs_layout[QPHY_LAYOUT_SIZE] = {
+   [QPHY_START_CTRL]   = 

[PATCH 03/17] dt-bindings: clock: Import SM6115 and SM8250 related clock header files from Linux

2023-08-14 Thread Bhupesh Sharma
Import SM6115 and SM8250 related clock header files from Linux,
which would be included in the Qualcomm QMP PHY driver.

Signed-off-by: Bhupesh Sharma 
---
 include/dt-bindings/clock/qcom,gcc-sm6115.h | 201 +++
 include/dt-bindings/clock/qcom,gcc-sm8250.h | 271 
 2 files changed, 472 insertions(+)
 create mode 100644 include/dt-bindings/clock/qcom,gcc-sm6115.h
 create mode 100644 include/dt-bindings/clock/qcom,gcc-sm8250.h

diff --git a/include/dt-bindings/clock/qcom,gcc-sm6115.h 
b/include/dt-bindings/clock/qcom,gcc-sm6115.h
new file mode 100644
index 00..b91a7b4604
--- /dev/null
+++ b/include/dt-bindings/clock/qcom,gcc-sm6115.h
@@ -0,0 +1,201 @@
+/* SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) */
+/*
+ * Copyright (c) 2019-2021, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef _DT_BINDINGS_CLK_QCOM_GCC_SM6115_H
+#define _DT_BINDINGS_CLK_QCOM_GCC_SM6115_H
+
+/* GCC clocks */
+#define GPLL0  0
+#define GPLL0_OUT_AUX2 1
+#define GPLL0_OUT_MAIN 2
+#define GPLL10 3
+#define GPLL10_OUT_MAIN4
+#define GPLL11 5
+#define GPLL11_OUT_MAIN6
+#define GPLL3  7
+#define GPLL4  8
+#define GPLL4_OUT_MAIN 9
+#define GPLL6  10
+#define GPLL6_OUT_MAIN 11
+#define GPLL7  12
+#define GPLL7_OUT_MAIN 13
+#define GPLL8  14
+#define GPLL8_OUT_MAIN 15
+#define GPLL9  16
+#define GPLL9_OUT_MAIN 17
+#define GCC_CAMSS_CSI0PHYTIMER_CLK 18
+#define GCC_CAMSS_CSI0PHYTIMER_CLK_SRC 19
+#define GCC_CAMSS_CSI1PHYTIMER_CLK 20
+#define GCC_CAMSS_CSI1PHYTIMER_CLK_SRC 21
+#define GCC_CAMSS_CSI2PHYTIMER_CLK 22
+#define GCC_CAMSS_CSI2PHYTIMER_CLK_SRC 23
+#define GCC_CAMSS_MCLK0_CLK24
+#define GCC_CAMSS_MCLK0_CLK_SRC25
+#define GCC_CAMSS_MCLK1_CLK26
+#define GCC_CAMSS_MCLK1_CLK_SRC27
+#define GCC_CAMSS_MCLK2_CLK28
+#define GCC_CAMSS_MCLK2_CLK_SRC29
+#define GCC_CAMSS_MCLK3_CLK30
+#define GCC_CAMSS_MCLK3_CLK_SRC31
+#define GCC_CAMSS_NRT_AXI_CLK  32
+#define GCC_CAMSS_OPE_AHB_CLK  33
+#define GCC_CAMSS_OPE_AHB_CLK_SRC  34
+#define GCC_CAMSS_OPE_CLK  35
+#define GCC_CAMSS_OPE_CLK_SRC  36
+#define GCC_CAMSS_RT_AXI_CLK   37
+#define GCC_CAMSS_TFE_0_CLK38
+#define GCC_CAMSS_TFE_0_CLK_SRC39
+#define GCC_CAMSS_TFE_0_CPHY_RX_CLK40
+#define GCC_CAMSS_TFE_0_CSID_CLK   41
+#define GCC_CAMSS_TFE_0_CSID_CLK_SRC   42
+#define GCC_CAMSS_TFE_1_CLK43
+#define GCC_CAMSS_TFE_1_CLK_SRC44
+#define GCC_CAMSS_TFE_1_CPHY_RX_CLK45
+#define GCC_CAMSS_TFE_1_CSID_CLK   46
+#define GCC_CAMSS_TFE_1_CSID_CLK_SRC   47
+#define GCC_CAMSS_TFE_2_CLK48
+#define GCC_CAMSS_TFE_2_CLK_SRC49
+#define GCC_CAMSS_TFE_2_CPHY_RX_CLK50
+#define GCC_CAMSS_TFE_2_CSID_CLK   51
+#define GCC_CAMSS_TFE_2_CSID_CLK_SRC   52
+#define GCC_CAMSS_TFE_CPHY_RX_CLK_SRC  53
+#define GCC_CAMSS_TOP_AHB_CLK  54
+#define GCC_CAMSS_TOP_AHB_CLK_SRC  55
+#define GCC_CFG_NOC_USB3_PRIM_AXI_CLK  56
+#define GCC_CPUSS_AHB_CLK  57
+#define GCC_CPUSS_GNOC_CLK 60
+#define GCC_DISP_AHB_CLK 

[PATCH 02/17] phy: qcom: Import QMP phy related header files from Linux

2023-08-14 Thread Bhupesh Sharma
Import Qualcomm QMP phy related header files from Linux.

Signed-off-by: Bhupesh Sharma 
---
 drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v2.h|  25 ++
 drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v3.h|  21 ++
 drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v4.h|  31 +++
 drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v5.h|  32 +++
 drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v6.h|  31 +++
 drivers/phy/qcom/phy-qcom-qmp-pcs-v2.h|  43 
 drivers/phy/qcom/phy-qcom-qmp-pcs-v3.h| 145 +++
 drivers/phy/qcom/phy-qcom-qmp-pcs-v4.h| 135 ++
 .../phy/qcom/phy-qcom-qmp-qserdes-com-v3.h| 111 +
 .../phy/qcom/phy-qcom-qmp-qserdes-com-v4.h| 123 +
 drivers/phy/qcom/phy-qcom-qmp-qserdes-com.h   | 140 +++
 drivers/phy/qcom/phy-qcom-qmp-qserdes-pll.h   |  66 +
 .../qcom/phy-qcom-qmp-qserdes-txrx-ufs-v6.h   |  30 +++
 .../phy/qcom/phy-qcom-qmp-qserdes-txrx-v3.h   |  68 +
 .../phy/qcom/phy-qcom-qmp-qserdes-txrx-v4.h   | 233 ++
 drivers/phy/qcom/phy-qcom-qmp-qserdes-txrx.h  | 205 +++
 16 files changed, 1439 insertions(+)
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v2.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v3.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v4.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v5.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v6.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-pcs-v2.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-pcs-v3.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-pcs-v4.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-qserdes-com-v3.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-qserdes-com-v4.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-qserdes-com.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-qserdes-pll.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-qserdes-txrx-ufs-v6.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-qserdes-txrx-v3.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-qserdes-txrx-v4.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-qserdes-txrx.h

diff --git a/drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v2.h 
b/drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v2.h
new file mode 100644
index 00..a0803a8783
--- /dev/null
+++ b/drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v2.h
@@ -0,0 +1,25 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2017, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef QCOM_PHY_QMP_PCS_UFS_V2_H_
+#define QCOM_PHY_QMP_PCS_UFS_V2_H_
+
+#define QPHY_V2_PCS_UFS_PHY_START  0x000
+#define QPHY_V2_PCS_UFS_POWER_DOWN_CONTROL 0x004
+
+#define QPHY_V2_PCS_UFS_TX_LARGE_AMP_DRV_LVL   0x034
+#define QPHY_V2_PCS_UFS_TX_LARGE_AMP_POST_EMP_LVL  0x038
+#define QPHY_V2_PCS_UFS_TX_SMALL_AMP_DRV_LVL   0x03c
+#define QPHY_V2_PCS_UFS_TX_SMALL_AMP_POST_EMP_LVL  0x040
+
+#define QPHY_V2_PCS_UFS_RX_MIN_STALL_NOCONFIG_TIME_CAP 0x0cc
+#define QPHY_V2_PCS_UFS_RX_SYM_RESYNC_CTRL 0x13c
+#define QPHY_V2_PCS_UFS_RX_MIN_HIBERN8_TIME0x140
+#define QPHY_V2_PCS_UFS_RX_SIGDET_CTRL20x148
+#define QPHY_V2_PCS_UFS_RX_PWM_GEAR_BAND   0x154
+
+#define QPHY_V2_PCS_UFS_READY_STATUS   0x168
+
+#endif
diff --git a/drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v3.h 
b/drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v3.h
new file mode 100644
index 00..adea13c3a9
--- /dev/null
+++ b/drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v3.h
@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2017, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef QCOM_PHY_QMP_PCS_UFS_V3_H_
+#define QCOM_PHY_QMP_PCS_UFS_V3_H_
+
+#define QPHY_V3_PCS_UFS_PHY_START  0x000
+#define QPHY_V3_PCS_UFS_POWER_DOWN_CONTROL 0x004
+#define QPHY_V3_PCS_UFS_TX_LARGE_AMP_DRV_LVL   0x02c
+#define QPHY_V3_PCS_UFS_TX_SMALL_AMP_DRV_LVL   0x034
+#define QPHY_V3_PCS_UFS_RX_SYM_RESYNC_CTRL 0x134
+#define QPHY_V3_PCS_UFS_RX_MIN_HIBERN8_TIME0x138
+#define QPHY_V3_PCS_UFS_RX_SIGDET_CTRL10x13c
+#define QPHY_V3_PCS_UFS_RX_SIGDET_CTRL20x140
+#define QPHY_V3_PCS_UFS_READY_STATUS   0x160
+#define QPHY_V3_PCS_UFS_TX_MID_TERM_CTRL1  0x1bc
+#define QPHY_V3_PCS_UFS_MULTI_LANE_CTRL1   0x1c4
+
+#endif
diff --git a/drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v4.h 
b/drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v4.h
new file mode 100644
index 00..a1c7d3d171
--- /dev/null
+++ b/drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v4.h
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) 2017, The Linux Foundation. All rights reserved.
+ */
+
+#ifndef QCOM_PHY_QMP_PCS_UFS_V4_H_
+#define QCOM_PHY_QMP_PCS_UFS_V4_H_
+
+/* Only for QMP V4 PHY - UFS PCS registers */
+#define QPHY_V4_PCS_UFS_PHY_START  

[PATCH 01/17] reset: qcom: Add support for SDM845 SoC reset table

2023-08-14 Thread Bhupesh Sharma
Add support for SDM845 SoC reset table

Signed-off-by: Bhupesh Sharma 
---
 drivers/reset/reset-qcom.c | 33 +
 1 file changed, 33 insertions(+)

diff --git a/drivers/reset/reset-qcom.c b/drivers/reset/reset-qcom.c
index 94315e76d5..0d37305e10 100644
--- a/drivers/reset/reset-qcom.c
+++ b/drivers/reset/reset-qcom.c
@@ -131,6 +131,38 @@ static const struct qcom_reset_map gcc_qcom_resets[] = {
 };
 #endif
 
+#ifdef CONFIG_SDM845
+#include 
+static const struct qcom_reset_map gcc_qcom_resets[] = {
+   [GCC_MMSS_BCR] = { 0xb000 },
+   [GCC_PCIE_0_BCR] = { 0x6b000 },
+   [GCC_PCIE_1_BCR] = { 0x8d000 },
+   [GCC_PCIE_PHY_BCR] = { 0x6f000 },
+   [GCC_PDM_BCR] = { 0x33000 },
+   [GCC_PRNG_BCR] = { 0x34000 },
+   [GCC_QUPV3_WRAPPER_0_BCR] = { 0x17000 },
+   [GCC_QUPV3_WRAPPER_1_BCR] = { 0x18000 },
+   [GCC_QUSB2PHY_PRIM_BCR] = { 0x12000 },
+   [GCC_QUSB2PHY_SEC_BCR] = { 0x12004 },
+   [GCC_SDCC2_BCR] = { 0x14000 },
+   [GCC_SDCC4_BCR] = { 0x16000 },
+   [GCC_TSIF_BCR] = { 0x36000 },
+   [GCC_UFS_CARD_BCR] = { 0x75000 },
+   [GCC_UFS_PHY_BCR] = { 0x77000 },
+   [GCC_USB30_PRIM_BCR] = { 0xf000 },
+   [GCC_USB30_SEC_BCR] = { 0x1 },
+   [GCC_USB3_PHY_PRIM_BCR] = { 0x5 },
+   [GCC_USB3PHY_PHY_PRIM_BCR] = { 0x50004 },
+   [GCC_USB3_DP_PHY_PRIM_BCR] = { 0x50008 },
+   [GCC_USB3_PHY_SEC_BCR] = { 0x5000c },
+   [GCC_USB3PHY_PHY_SEC_BCR] = { 0x50010 },
+   [GCC_USB3_DP_PHY_SEC_BCR] = { 0x50014 },
+   [GCC_USB_PHY_CFG_AHB2PHY_BCR] = { 0x6a000 },
+   [GCC_PCIE_0_PHY_BCR] = { 0x6c01c },
+   [GCC_PCIE_1_PHY_BCR] = { 0x8e01c },
+};
+#endif
+
 static int qcom_reset_assert(struct reset_ctl *rst)
 {
struct qcom_reset_priv *priv = dev_get_priv(rst->dev);
@@ -171,6 +203,7 @@ static const struct reset_ops qcom_reset_ops = {
 static const struct udevice_id qcom_reset_ids[] = {
{ .compatible = "qcom,gcc-reset-ipq4019" },
{ .compatible = "qcom,gcc-reset-qcs404" },
+   { .compatible = "qcom,gcc-reset-sdm845" },
{ }
 };
 
-- 
2.38.1



[PATCH 00/17] Enable UFS on DragonBoard845c

2023-08-14 Thread Bhupesh Sharma
This patchset enables the UFS controller on DragonBoard845c
board which houses Qualcomm SDM845 Snapdragon SoC.

In addition to enabling the UFS HC and UFS QMP PHY found
on this SoC this patchset also contains:
 * Patches to add 'reset' controller support for SDM845 SoC.
 * Minor UFS core framework fixes.
 * Patches to sync u-boot UFS driver flow with Linux UFS driver.
 * Patches which enable RESET, UFS and SCSI config options for
   DragonBoard845c.

Also since the current UFS maintainer's email bounces, promote
myself as the new UFS maintainer to help address this work-area ->
review, test and fix UFS issues in u-boot framework. 

Bhupesh Sharma (17):
  reset: qcom: Add support for SDM845 SoC reset table
  phy: qcom: Import QMP phy related header files from Linux
  dt-bindings: clock: Import SM6115 and SM8250 related clock header
files from Linux
  phy: qcom: Add QMP UFS PHY driver
  ufs/ufs.h: Add definition of 'ufshcd_rmwl()'
  ufs: Clear UECPA once due to LINERESET has happened during
LINK_STARTUP
  ufs: Add support for probing UFS controllers newer than
UFSHCI_VERSION_21
  ufs: Expose 'ufshcd_ops_dbg_register_dump' vops to allow dumping debug
registers
  ufs: Sync possible UFS Quirks with Linux UFS driver
  ufs: Add missing memory barriers
  ufs: Fix debug message in 'ufs_start'
  ufs: Add Support for Qualcomm UFS HC driver
  arm: dts: qcom: sdm845: Add 'reset' node
  arm: dts: qcom: sdm845: Add UFS HC and PHY nodes
  configs/dragonboard845c_defconfig: Enable DM_RESET by default
  configs/dragonboard845c_defconfig: Enable UFS + SCSI related configs
  MAINTAINERS: Update UFS maintainer

 MAINTAINERS   |   2 +-
 arch/arm/dts/sdm845.dtsi  |  68 ++
 configs/dragonboard845c_defconfig |  11 +
 drivers/phy/qcom/Kconfig  |   6 +
 drivers/phy/qcom/Makefile |   1 +
 drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v2.h|  25 +
 drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v3.h|  21 +
 drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v4.h|  31 +
 drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v5.h|  32 +
 drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v6.h|  31 +
 drivers/phy/qcom/phy-qcom-qmp-pcs-v2.h|  43 +
 drivers/phy/qcom/phy-qcom-qmp-pcs-v3.h| 145 +++
 drivers/phy/qcom/phy-qcom-qmp-pcs-v4.h| 135 +++
 .../phy/qcom/phy-qcom-qmp-qserdes-com-v3.h| 111 ++
 .../phy/qcom/phy-qcom-qmp-qserdes-com-v4.h| 123 +++
 drivers/phy/qcom/phy-qcom-qmp-qserdes-com.h   | 140 +++
 drivers/phy/qcom/phy-qcom-qmp-qserdes-pll.h   |  66 ++
 .../qcom/phy-qcom-qmp-qserdes-txrx-ufs-v6.h   |  30 +
 .../phy/qcom/phy-qcom-qmp-qserdes-txrx-v3.h   |  68 ++
 .../phy/qcom/phy-qcom-qmp-qserdes-txrx-v4.h   | 233 
 drivers/phy/qcom/phy-qcom-qmp-qserdes-txrx.h  | 205 
 drivers/phy/qcom/phy-qcom-qmp-ufs.c   | 996 ++
 drivers/phy/qcom/phy-qcom-qmp.h   | 115 ++
 drivers/reset/reset-qcom.c|  33 +
 drivers/ufs/Kconfig   |   7 +
 drivers/ufs/Makefile  |   1 +
 drivers/ufs/qcom-ufshcd.c | 880 
 drivers/ufs/ufs-qcom.h| 275 +
 drivers/ufs/ufs.c |  93 +-
 drivers/ufs/ufs.h | 156 ++-
 include/dt-bindings/clock/qcom,gcc-sm6115.h   | 201 
 include/dt-bindings/clock/qcom,gcc-sm8250.h   | 271 +
 32 files changed, 4539 insertions(+), 16 deletions(-)
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v2.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v3.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v4.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v5.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-pcs-ufs-v6.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-pcs-v2.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-pcs-v3.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-pcs-v4.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-qserdes-com-v3.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-qserdes-com-v4.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-qserdes-com.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-qserdes-pll.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-qserdes-txrx-ufs-v6.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-qserdes-txrx-v3.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-qserdes-txrx-v4.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-qserdes-txrx.h
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp-ufs.c
 create mode 100644 drivers/phy/qcom/phy-qcom-qmp.h
 create mode 100644 drivers/ufs/qcom-ufshcd.c
 create mode 100644 drivers/ufs/ufs-qcom.h
 create mode 100644 include/dt-bindings/clock/qcom,gcc-sm6115.h
 create mode 100644 include/dt-bindings/clock/qcom,gcc-sm8250.h

-- 
2.38.1



Re: [PATCH 1/3] sunxi: psci: clean away preprocessor macros

2023-08-14 Thread Sam Edwards

On 8/14/23 15:05, Andre Przywara wrote:

Yes, I will add this to the header file, either defined as 0, or to its
actual address.


Gotcha; my v2 will also assume you've taken care of merging these guys:
+#define SUNXI_CPUX_BASE0x0901
+#define SUNXI_CPUCFG_BASE  0

(As I presume from your comments on 3/3 that it's better to consider 
"CPUX" simply an updated CPUCFG than to distinguish between them as 
fundamentally different concepts.)



Yes, but you handle both above explicitly.


No, I was passing a NULL clamp address in those cases too. So I'm noting 
that this must also be fixed in v2.


Thanks,
Sam


Re: [PATCH 2/5] drivers: security: Add TPM2 implementation of security devices

2023-08-14 Thread Sean Edmond



On 2023-08-14 1:39 a.m., Ilias Apalodimas wrote:

Hi Sean

On Sat, 12 Aug 2023 at 03:28,  wrote:

From: Stephen Carlson 

This implementation of the security uclass driver allows existing TPM2
devices declared in the device tree to be referenced for storing the OS
anti-rollback counter, using the TPM2 non-volatile storage API.

Signed-off-by: Stephen Carlson 
---
  MAINTAINERS |   1 +
  drivers/security/Makefile   |   1 +
  drivers/security/security-tpm.c | 173 
  include/tpm-v2.h|   1 +
  4 files changed, 176 insertions(+)
  create mode 100644 drivers/security/security-tpm.c

diff --git a/MAINTAINERS b/MAINTAINERS

[...]


+
+struct security_state {
+   u32 index_arbvn;
+   struct udevice *tpm_dev;
+};
+
+static int tpm_security_init(struct udevice *tpm_dev)
+{
+   int res;
+
+   /* Initialize TPM but allow reuse of existing session */
+   res = tpm_open(tpm_dev);
+   if (res == -EBUSY) {
+   log(UCLASS_SECURITY, LOGL_DEBUG,
+   "Existing TPM session found, reusing\n");
+   } else {
+   if (res) {
+   log(UCLASS_SECURITY, LOGL_ERR,
+   "TPM initialization failed (ret=%d)\n", res);
+   return res;
+   }
+
+   res = tpm2_startup(tpm_dev, TPM2_SU_CLEAR);
+   if (res) {
+   log(UCLASS_SECURITY, LOGL_ERR,
+   "TPM startup failed (ret=%d)\n", res);
+   return res;
+   }
+   }
+
+   return 0;
+}

There's nothing security related in that wrapper.  It looks like a
typical tpm startup sequence.  Any reason you can't use
tpm_auto_start()?


Good suggestion, I'll make this change.




+
+static int tpm_security_arbvn_get(struct udevice *dev, u64 *arbvn)
+{
+   struct security_state *priv = dev_get_priv(dev);
+   int ret;
+
+   if (!arbvn)
+   return -EINVAL;
+
+   ret = tpm2_nv_read_value(priv->tpm_dev, priv->index_arbvn, arbvn,
+sizeof(u64));
+   if (ret == TPM2_RC_NV_UNINITIALIZED) {
+   /* Expected if no OS image has been loaded before */
+   log(UCLASS_SECURITY, LOGL_INFO,
+   "No previous OS image, defaulting ARBVN to 0\n");
+   *arbvn = 0ULL;

Why aren't we returning an error here?  Looks like the code following
this is trying to reason with the validity of arbnv
On the first boot (before ARBVN has been set in NV memory), it's 
expected that the NV index hasn't been initialized/written yet.  In this 
case, TPM2_RC_NV_UNINITIALIZED is expected.  A value of 0 is returned to 
ensure that the anti-rollback check always passes (which it should since 
there's nothing to check on the first boot).



+   } else if (ret) {
+   log(UCLASS_SECURITY, LOGL_ERR,
+   "Unable to read ARBVN from TPM (ret=%d)\n", ret);
+   return ret;
+   }
+
+   return 0;
+}
+
+static int tpm_security_arbvn_set(struct udevice *dev, u64 arbvn)
+{
+   struct security_state *priv = dev_get_priv(dev);
+   struct udevice *tpm_dev = priv->tpm_dev;
+   u64 old_arbvn;
+   int ret;
+
+   ret = tpm_security_arbvn_get(dev, _arbvn);
+   if (ret)
+   return ret;
+
+   if (arbvn < old_arbvn)
+   return -EPERM;
+

What happens if they are equal ?

If they are equal, then we are booting the same OS that was previously 
booted (we are not moving the OS version forward or back).


Note the actual "anti-rollback" check is in fit_image_verify_arbvn().  
If it make things more clear, we could remove the value checks here 
completely and just write the value.



+   if (arbvn > old_arbvn) {

You just check for this and exited


+   ret = tpm2_nv_write_value(tpm_dev, priv->index_arbvn, ,
+ sizeof(u64));
+   if (ret) {
+   log(UCLASS_SECURITY, LOGL_ERR,
+   "Unable to write ARBVN to TPM (ret=%d)\n", ret);
+   return ret;
+   }
+   }
+
+   return 0;
+}
+
+static const struct dm_security_ops tpm_security_ops = {
+   .arbvn_get  = tpm_security_arbvn_get,
+   .arbvn_set  = tpm_security_arbvn_set,
+};
+
+static int tpm_security_probe(struct udevice *dev)
+{
+   struct security_state *priv = dev_get_priv(dev);
+   struct udevice *tpm_dev = priv->tpm_dev;
+   u32 index = priv->index_arbvn;
+   int ret;
+
+   if (!tpm_dev) {
+   log(UCLASS_SECURITY, LOGL_ERR,
+   "TPM device not defined in DTS\n");
+   return -EINVAL;
+   }
+
+   ret = tpm_security_init(tpm_dev);
+   if (ret)
+   return ret;
+
+   ret = tpm2_nv_define_space(tpm_dev, index, sizeof(u64), TPMA_NV_PPREAD |
+  

Re: [PATCH 1/3] sunxi: psci: clean away preprocessor macros

2023-08-14 Thread Andre Przywara
On Mon, 14 Aug 2023 12:10:25 -0600
Sam Edwards  wrote:

> Hi Andre,
> 
> On 8/14/23 10:37, Andre Przywara wrote:
> > So I think we can get rid of this:
> > - GEN_H6 never compiles this code here, as both H6 and H616 are arm64.  
> 
> Easy!
> 
> > - We can define SUNXI_PRCM_BASE for NCAT2, I believe Samuel once
> > mentioned that the D1/T113 does have such a block, actually.  
> 
> Will you be taking care of this in v2 of your T113s series, or should I 
> be adding it (in which case I'll need to know the location of the block)?

Yes, I will add this to the header file, either defined as 0, or to its
actual address.
 
> > - The non-existing cpu_pwr_clamp member should go away when you switch to
> > a BASE_ADDR + REG_OFFSET approach, I think.  
> 
> Less easy, but still can do.
> 
> > Shouldn't that be the opposite? In the existing code, sun6i and H3 DO
> > program the clamp (see the "-" section above).  
> 
> And sun7i and R40, as well.

Yes, but you handle both above explicitly.

> It appears I simply read the #if 
> defined(...) mess backwards. I'll fix that for v2. As a bonus, this 
> lends itself to a rather nice refactoring of sunxi_cpu_set_power() where 
> I can have the if block only determine the pwroff/clamp addresses, and 
> have a single tail-call to sunxi_power_switch() at the bottom. Since the 
> latter function is so simple, I may as well just inline it into 
> sunxi_cpu_set_power() (which I suspect might be more readable).

Yes, any further simplification is welcome, and probably somewhat
rewarding in this case ;-)

Cheers,
Andre


Re: [PATCH] sunxi: psci: remove redundant initialization from psci_arch_init

2023-08-14 Thread Andre Przywara
On Wed, 31 May 2023 14:15:20 -0600
Sam Edwards  wrote:

Hi,

(CC:ing Marc and Chen-Yu as the original authors)

sorry for the delay, found that mouldering in my Drafts folder.

> The nonsec code overrides/handles these:
> - Setting PMR to 0xFF happens earlier, in _nonsec_init, so this is
>   redundant.
> - The NS bit is not yet set: it gets set later in _secure_monitor.
>   Trying to clear it here is pointless and misleading.

So superficially this looks alright, but I am still somewhat reluctant
to apply this, see below ...

> Signed-off-by: Sam Edwards 
> ---
>  arch/arm/cpu/armv7/sunxi/psci.c | 4 
>  1 file changed, 4 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
> index e1d3638b5c..f866025c37 100644
> --- a/arch/arm/cpu/armv7/sunxi/psci.c
> +++ b/arch/arm/cpu/armv7/sunxi/psci.c
> @@ -300,14 +300,10 @@ void __secure psci_arch_init(void)
>   /* Set SGI15 priority to 0 */
>   writeb(0, GICD_BASE + GICD_IPRIORITYRn + 15);
>  
> - /* Be cool with non-secure */
> - writel(0xff, GICC_BASE + GICC_PMR);
> -
>   /* Switch FIQEn on */
>   setbits_le32(GICC_BASE + GICC_CTLR, BIT(3));
>  
>   reg = cp15_read_scr();
>   reg |= BIT(2);  /* Enable FIQ in monitor mode */
> - reg &= ~BIT(0); /* Secure mode */

I am scratching my head on this one: I see it deliberately being done in
the original patch by Marc[1], and wonder why. If I read the code and
the architecture correctly, this routine is called only(?) from the SMC
handler, which means we are in monitor mode, that only exists in secure
state. So the NS bit is irrelevant until we switch to another mode. And
we indeed set the bit later, before dropping back to non-secure. So I
agree that clearing the bit is pointless. Was it put in to be more
robust, for other potential callers of this function, for instance in
the boot path?

Does anyone remember?

Cheers,
Andre

[1]
https://source.denx.de/u-boot/u-boot/-/commit/d5db7024aafc5ea603f3a34f83bb29a1eaa3cbe7#f377950449a5dba076796d9e48fbd21f5d6ac545_0_65

>   cp15_write_scr(reg);
>  }



Re: [RFC PATCH 2/4] scripts/Makefile.lib: Embed capsule public key in platform's dtb

2023-08-14 Thread Tom Rini
On Tue, Aug 15, 2023 at 12:19:36AM +0530, Sughosh Ganu wrote:
> hi Tom,
> 
> On Mon, 14 Aug 2023 at 20:34, Tom Rini  wrote:
> >
> > On Mon, Aug 14, 2023 at 02:33:07PM +0530, Sughosh Ganu wrote:
> > > The EFI capsule authentication logic in u-boot expects the public key
> > > in the form of an EFI Signature List(ESL) to be provided as part of
> > > the platform's dtb. Currently, the embedding of the ESL file into the
> > > dtb needs to be done manually.
> > >
> > > Add a target for generating a dtsi file which contains the signature
> > > node with the ESL file included as a property under the signature
> > > node. Include the dtsi file in the dtb. This brings the embedding of
> > > the ESL in the dtb into the U-Boot build flow.
> > >
> > > The path to the ESL file is specified through the
> > > CONFIG_EFI_CAPSULE_ESL_FILE symbol.
> > >
> > > Signed-off-by: Sughosh Ganu 
> > > ---
> > >  lib/efi_loader/Kconfig |  9 +
> > >  lib/efi_loader/capsule_esl.dtsi.in | 11 +++
> > >  scripts/Makefile.lib   | 17 +
> > >  3 files changed, 37 insertions(+)
> > >  create mode 100644 lib/efi_loader/capsule_esl.dtsi.in
> > >
> > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > > index 9989e3f384..f40a62a0ba 100644
> > > --- a/lib/efi_loader/Kconfig
> > > +++ b/lib/efi_loader/Kconfig
> > > @@ -272,6 +272,15 @@ config EFI_CAPSULE_MAX
> > > Select the max capsule index value used for capsule report
> > > variables. This value is used to create CapsuleMax variable.
> > >
> > > +config EFI_CAPSULE_ESL_FILE
> > > + string "Path to the EFI Signature List File"
> > > + default ""
> >
> > No default.
> 
> I think Simon wanted to keep this so that it would not break the build
> if no option was provided.

No, that means that there's a missing dependency.  Blank defaults are
almost never right.  And "so that we don't break configs" means that
something else is wrong, generally a missing dependency.  And maybe I
cut out too much context here as yes, this option needs to depend on
some part of the capsule framework.

> > [snip]
> > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > > index f41b16781d..cf4eef0b05 100644
> > > --- a/scripts/Makefile.lib
> > > +++ b/scripts/Makefile.lib
> > > @@ -334,8 +334,25 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
> > >   ; \
> > >   sed "s:$(pre-tmp):$(<):" $(depfile).pre.tmp $(depfile).dtc.tmp > 
> > > $(depfile)
> > >
> > > +ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > > +quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@
> > > +cmd_capsule_esl_gen = \
> > > + $(shell sed "s:ESL_BIN_FILE:$(capsule_esl_path):" 
> > > $(capsule_esl_input_file) > $@)
> > > +
> > > +$(obj)/.capsule_esl.dtsi:
> > > + $(call cmd_capsule_esl_gen)
> > > +
> > > +capsule_esl_input_file=$(srctree)/lib/efi_loader/capsule_esl.dtsi.in
> > > +capsule_esl_dtsi = .capsule_esl.dtsi
> > > +capsule_esl_path=$(abspath $(srctree)/$(subst 
> > > $(quote),,$(CONFIG_EFI_CAPSULE_ESL_FILE)))
> >
> > This seems right.
> >
> > > +include_files += $(capsule_esl_dtsi)
> > > +
> > > +$(obj)/%.dtb: $(src)/%.dts $(DTC) $(obj)/.capsule_esl.dtsi FORCE
> > > + $(call if_changed_dep,dtc)
> > > +else
> > >  $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
> > >   $(call if_changed_dep,dtc)
> > > +endif
> >
> > I think this implies to me that we should have been depending on
> > $(include_files) (once renamed to be less vague) here already ?
> 
> Okay. That can be done. I had kept the .capsule_esl.dtsi as a
> dependency primarily because that file needs to be generated before it
> can be included. I suppose the same can apply to some other dtsi as
> well.

You shouldn't need to have it be explicit, if it's in dtsi_include_list
as there's a rule so make can resolve how to satisfy the dependency.
But since we dynamically #include the other files, I don't know that
they will be caught with the normal dependency logic.  But, it will
still be cleaner to have the dtb rules depend on the automagic #include
files too, rather than special casing the rule here I believe.

-- 
Tom


signature.asc
Description: PGP signature


[PATCH 2/2] irq: Fix typo in header comment

2023-08-14 Thread Paul Barker
Signed-off-by: Paul Barker 
---
 include/irq.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/irq.h b/include/irq.h
index 1d08cb858d0e..5638c10128e9 100644
--- a/include/irq.h
+++ b/include/irq.h
@@ -109,7 +109,7 @@ struct irq_ops {
 * xxx_xlate() call, or as the only step in implementing a client's
 * irq_request() call.
 *
-* @irq:The irq struct to request; this has been fille in by
+* @irq:The irq struct to request; this has been filled in by
 *  a previoux xxx_xlate() function call, or by the caller
 *  of irq_request().
 * @return 0 if OK, or a negative error code.
-- 
2.34.1



[PATCH 1/2] clk: Fix typo in header comment

2023-08-14 Thread Paul Barker
Signed-off-by: Paul Barker 
---
 include/clk-uclass.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/clk-uclass.h b/include/clk-uclass.h
index 65ebff9ed27b..a22f1a5d8489 100644
--- a/include/clk-uclass.h
+++ b/include/clk-uclass.h
@@ -64,7 +64,7 @@ int of_xlate(struct clk *clock, struct ofnode_phandle_args 
*args);
 
 /**
  * request() - Request a translated clock.
- * @clock: The clock struct to request; this has been fille in by
+ * @clock: The clock struct to request; this has been filled in by
  * a previoux xxx_xlate() function call, or by the caller
  * of clk_request().
  *
-- 
2.34.1



Re: [RFC PATCH 1/4] scripts/Makefile.lib: Collate all dtsi files for inclusion

2023-08-14 Thread Tom Rini
On Tue, Aug 15, 2023 at 12:12:57AM +0530, Sughosh Ganu wrote:
> hi Tom,
> 
> On Mon, 14 Aug 2023 at 20:34, Tom Rini  wrote:
> >
> > On Mon, Aug 14, 2023 at 02:33:06PM +0530, Sughosh Ganu wrote:
> >
> > > At the time of building a device-tree file, all the *u-boot.dtsi files
> > > are looked for, in a particular order, and the first file found is
> > > included. Then, the list of files specified in the
> > > CONFIG_DEVICE_TREE_INCLUDES symbol are included.
> > >
> > > Combine these files that are to be included into a variable, and then
> > > include all these files in one go.
> > >
> > > Signed-off-by: Sughosh Ganu 
> > > ---
> > >  scripts/Makefile.lib | 11 +++
> > >  1 file changed, 7 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > > index f5ab7af0f4..f41b16781d 100644
> > > --- a/scripts/Makefile.lib
> > > +++ b/scripts/Makefile.lib
> > > @@ -179,10 +179,13 @@ ifdef DEVICE_TREE_DEBUG
> > >  u_boot_dtsi_options_debug = $(warning $(u_boot_dtsi_options_raw))
> > >  endif
> > >
> > > -# We use the first match
> > > -u_boot_dtsi = $(strip $(u_boot_dtsi_options_debug) \
> > > +# We use the first match to be included
> > > +include_files = $(strip $(u_boot_dtsi_options_debug) \
> > >   $(notdir $(firstword $(u_boot_dtsi_options
> > >
> > > +# The CONFIG_DEVICE_TREE_INCLUDES also need to be included
> > > +include_files += $(CONFIG_DEVICE_TREE_INCLUDES)
> >
> > This is what I wanted, logic-wise, but I think include_files is too
> > vague.
> 
> Okay. How about dtsi_include_list, or dtsi_list? If not, can you
> suggest a name which you think would be apt. Thanks.

dtsi_include_list sounds good.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 1/3] fdt: common API to populate kaslr seed

2023-08-14 Thread Sean Edmond



On 2023-08-12 6:09 a.m., Simon Glass wrote:

Hi Sean,

On Fri, 11 Aug 2023 at 11:14, Sean Edmond 
wrote:


On 2023-08-09 6:49 p.m., Simon Glass wrote:

Hi Sean,

On Wed, 9 Aug 2023 at 16:35, Sean Edmond 

wrote:

On 2023-08-08 7:03 p.m., Simon Glass wrote:

Hi,

On Fri, 4 Aug 2023 at 17:34,  wrote:

From: Dhananjay Phadke 

fdt_fixup_kaslr_seed() will update given FDT with random seed value.
Source for random seed can be TPM or RNG driver in u-boot or sec
firmware (ARM).

Signed-off-by: Dhananjay Phadke 
---
arch/arm/cpu/armv8/sec_firmware.c | 32

+++

common/fdt_support.c  | 31

++

include/fdt_support.h |  3 +++
3 files changed, 41 insertions(+), 25 deletions(-)

We need to find a way to use the ofnode API here.


diff --git a/arch/arm/cpu/armv8/sec_firmware.c

b/arch/arm/cpu/armv8/sec_firmware.c

index c0e8726346..84ba49924e 100644
--- a/arch/arm/cpu/armv8/sec_firmware.c
+++ b/arch/arm/cpu/armv8/sec_firmware.c
@@ -411,46 +411,28 @@ int sec_firmware_init(const void

*sec_firmware_img,

/*
 * fdt_fix_kaslr - Add kalsr-seed node in Device tree
 * @fdt:   Device tree
- * @eret:  0 in case of error, 1 for success
+ * @eret:  0 for success
 */
int fdt_fixup_kaslr(void *fdt)

You could pass an oftree to this function, e.g. obtained with:

oftree_from_fdt(fdt)

The common API I added is fdt_fixup_kaslr_seed(), which was added to
"common/fdt_support.c".

There are 3 callers:
sec_firmware_init()->fdt_fixup_kaslr_seed()
do_kaslr_seed()->fdt_fixup_kaslr_seed()
image_setup_libfdt()->fdt_tpm_kaslr_seed->fdt_fixup_kaslr_seed()

I think the ask is to create a common API that uses the ofnode API.

So,

instead of fdt_fixup_kaslr_seed() I can create
ofnode_fixup_kaslr_seed()?  Where should it live?

If you like you could add common/ofnode_support.c ?

But it is OK to have it in the same file, I think.


Are you also wanting
the callers (eg. fdt_tpm_kaslr_seed, fdt_fixup_kaslr) to take oftree as
input too?

So far as you can go, yes. Also you may want to pass an ofnode (the
root node) so that things can deal with adding their stuff to any
node.

Regards,
Simon


I re-worked the API to use the ofnode API and tested it on our board.  I
was required to explicitly enable CONFIG_OFNODE_MULTI_TREE in order for
it to work.

I have concerns this will create a breaking change for users of the
kaslr fdt touch up.  In our case, if CONFIG_OFNODE_MULTI_TREE isn't set,
the control FDT gets touched up, not the kernel FDT as required.
Everything runs to completion, but "/proc/device-tree/chosen/kaslr-seed"
isn't present after boot.

Am I missing something?  Perhaps there's a way to modify the default
value for CONFIG_OFNODE_MULTI_TREE to ensure this works out-of-the-box?


Yes, perhaps we should enable this when fixups are used? Is there a way to
tell?
I don't think there's a way to tell unfortunately.  Fixups are always 
called if OF_LIBFDT is enabled, and if an FDT is found during bootm.


I'm having trouble understanding the intention of the current default 
for OFNODE_MULTI_TREE:

    default y if EVENT && !DM_DEV_READ_INLINE && !DM_INLINE_OFNODE
Could we simplify to this?
    default y if !OF_LIVE


Also, we should make it return an error when attempting to use a tree
without that option enabled. I would expect oftree_ensure() to provide that?

I'll add a check.


Regards,
Simon




Re: [PATCHv6 05/14] net/lwip: implement tftp cmd

2023-08-14 Thread Maxim Uvarov
On Mon, 14 Aug 2023 at 20:25, Ilias Apalodimas 
wrote:

> > +// SPDX-License-Identifier: GPL-2.0
> > +
> > +/*
> > + * (C) Copyright 2023 Linaro Ltd. 
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include "lwip/apps/tftp_client.h"
> > +#include "lwip/apps/tftp_server.h"
> > +#include 
> > +
> > +#include 
> > +
> > +#include 
> > +
> > +#if LWIP_UDP
>
> Why do we have this?  I dont think it makes sense to start reasoning about
> LWIP config options from within U-Boot code.  Instead U-Boot makefiles
> should enable all the LWIP features we need when a command is included
>
>
That is how example used it. Agree do drop this if.


> > +
> > +static ulong daddr;
> > +static ulong size;
> > +
> > +static void *tftp_open(const char *fname, const char *mode, u8_t
> is_write)
> > +{
> > + LWIP_UNUSED_ARG(mode);
> > + return NULL;
> > +}
> > +
> > +static void tftp_close(void *handle)
> > +{
> > + log_info("\ndone\n");
> > + log_info("Bytes transferred = %ld (0x%lx hex)\n", size, size);
> > +
> > + bootstage_mark_name(BOOTSTAGE_KERNELREAD_STOP, "tftp_done");
> > + env_set_ulong("filesize", size);
> > + ulwip_exit(0);
> > +}
> > +
> > +static int tftp_read(void *handle, void *buf, int bytes)
> > +{
> > + return 0;
> > +}
> > +
> > +static int tftp_write(void *handle, struct pbuf *p)
> > +{
> > + struct pbuf *q;
> > +
> > + for (q = p; q != NULL; q = q->next) {
> > + memcpy((void *)daddr, q->payload, q->len);
> > + daddr += q->len;
> > + size += q->len;
> > + log_info("#");
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void tftp_error(void *handle, int err, const char *msg, int size)
> > +{
> > + char message[100];
> > +
> > + LWIP_UNUSED_ARG(handle);
> > +
> > + memset(message, 0, sizeof(message));
> > + MEMCPY(message, msg, LWIP_MIN(sizeof(message)-1, (size_t)size));
> > +
> > + log_info("TFTP error: %d (%s)", err, message);
> > +}
> > +
> > +static const struct tftp_context tftp = {
> > + tftp_open,
> > + tftp_close,
> > + tftp_read,
> > + tftp_write,
> > + tftp_error
> > +};
> > +
> > +int ulwip_tftp(ulong addr, char *fname)
> > +{
> > + void *f = (void *)0x1; /* unused fake file handle*/
> > + err_t err;
> > + ip_addr_t srv;
> > + int ret;
> > + char *server_ip;
> > +
> > + if (!fname || addr == 0)
> > + return CMD_RET_FAILURE;
> > +
> > + size = 0;
> > + daddr = addr;
> > + server_ip = env_get("serverip");
> > + if (!server_ip) {
> > + log_err("error: serverip variable has to be set\n");
> > + return CMD_RET_FAILURE;
> > + }
> > +
> > + ret = ipaddr_aton(server_ip, );
> > + if (!ret) {
> > + log_err("error: ipaddr_aton\n");
> > + return CMD_RET_FAILURE;
> > + }
> > +
> > + log_info("TFTP from server %s; our IP address is %s\n",
> > +  server_ip, env_get("ipaddr"));
> > + log_info("Filename '%s'.\n", fname);
> > + log_info("Load address: 0x%lx\n", daddr);
> > + log_info("Loading:");
> > +
> > + bootstage_mark_name(BOOTSTAGE_KERNELREAD_START, "tftp_start");
> > +
> > + err = tftp_init_client();
> > + if (!(err == ERR_OK || err == ERR_USE))
> > + log_err("tftp_init_client err: %d\n", err);
> > +
> > + err = tftp_get(f, , TFTP_PORT, fname, TFTP_MODE_OCTET);
>
> Shouldn't this be part of tftp_read()?
>
>
tft_get() actually does some initialization. Then when the polling loop
works tftp_write() or tftp_read() callbacks will be called.
So here it should stay as is.


> > + /* might return different errors, like routing problems */
> > + if (err != ERR_OK) {
> > + log_err("tftp_get err=%d\n", err);
> > + return CMD_RET_FAILURE;
> > + }
> > +
> > + env_set_hex("fileaddr", addr);
> > + return err;
> > +}
> > +#else
> > +#error "UDP has to be supported"
> > +#endif /* LWIP_UDP */
> > --
> > 2.30.2
> >
>


Re: [RFC PATCH 2/4] scripts/Makefile.lib: Embed capsule public key in platform's dtb

2023-08-14 Thread Sughosh Ganu
hi Tom,

On Mon, 14 Aug 2023 at 20:34, Tom Rini  wrote:
>
> On Mon, Aug 14, 2023 at 02:33:07PM +0530, Sughosh Ganu wrote:
> > The EFI capsule authentication logic in u-boot expects the public key
> > in the form of an EFI Signature List(ESL) to be provided as part of
> > the platform's dtb. Currently, the embedding of the ESL file into the
> > dtb needs to be done manually.
> >
> > Add a target for generating a dtsi file which contains the signature
> > node with the ESL file included as a property under the signature
> > node. Include the dtsi file in the dtb. This brings the embedding of
> > the ESL in the dtb into the U-Boot build flow.
> >
> > The path to the ESL file is specified through the
> > CONFIG_EFI_CAPSULE_ESL_FILE symbol.
> >
> > Signed-off-by: Sughosh Ganu 
> > ---
> >  lib/efi_loader/Kconfig |  9 +
> >  lib/efi_loader/capsule_esl.dtsi.in | 11 +++
> >  scripts/Makefile.lib   | 17 +
> >  3 files changed, 37 insertions(+)
> >  create mode 100644 lib/efi_loader/capsule_esl.dtsi.in
> >
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index 9989e3f384..f40a62a0ba 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -272,6 +272,15 @@ config EFI_CAPSULE_MAX
> > Select the max capsule index value used for capsule report
> > variables. This value is used to create CapsuleMax variable.
> >
> > +config EFI_CAPSULE_ESL_FILE
> > + string "Path to the EFI Signature List File"
> > + default ""
>
> No default.

I think Simon wanted to keep this so that it would not break the build
if no option was provided.

>
> [snip]
> > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > index f41b16781d..cf4eef0b05 100644
> > --- a/scripts/Makefile.lib
> > +++ b/scripts/Makefile.lib
> > @@ -334,8 +334,25 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
> >   ; \
> >   sed "s:$(pre-tmp):$(<):" $(depfile).pre.tmp $(depfile).dtc.tmp > 
> > $(depfile)
> >
> > +ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> > +quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@
> > +cmd_capsule_esl_gen = \
> > + $(shell sed "s:ESL_BIN_FILE:$(capsule_esl_path):" 
> > $(capsule_esl_input_file) > $@)
> > +
> > +$(obj)/.capsule_esl.dtsi:
> > + $(call cmd_capsule_esl_gen)
> > +
> > +capsule_esl_input_file=$(srctree)/lib/efi_loader/capsule_esl.dtsi.in
> > +capsule_esl_dtsi = .capsule_esl.dtsi
> > +capsule_esl_path=$(abspath $(srctree)/$(subst 
> > $(quote),,$(CONFIG_EFI_CAPSULE_ESL_FILE)))
>
> This seems right.
>
> > +include_files += $(capsule_esl_dtsi)
> > +
> > +$(obj)/%.dtb: $(src)/%.dts $(DTC) $(obj)/.capsule_esl.dtsi FORCE
> > + $(call if_changed_dep,dtc)
> > +else
> >  $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
> >   $(call if_changed_dep,dtc)
> > +endif
>
> I think this implies to me that we should have been depending on
> $(include_files) (once renamed to be less vague) here already ?

Okay. That can be done. I had kept the .capsule_esl.dtsi as a
dependency primarily because that file needs to be generated before it
can be included. I suppose the same can apply to some other dtsi as
well.

-sughosh


Re: [RFC PATCH 1/4] scripts/Makefile.lib: Collate all dtsi files for inclusion

2023-08-14 Thread Sughosh Ganu
hi Tom,

On Mon, 14 Aug 2023 at 20:34, Tom Rini  wrote:
>
> On Mon, Aug 14, 2023 at 02:33:06PM +0530, Sughosh Ganu wrote:
>
> > At the time of building a device-tree file, all the *u-boot.dtsi files
> > are looked for, in a particular order, and the first file found is
> > included. Then, the list of files specified in the
> > CONFIG_DEVICE_TREE_INCLUDES symbol are included.
> >
> > Combine these files that are to be included into a variable, and then
> > include all these files in one go.
> >
> > Signed-off-by: Sughosh Ganu 
> > ---
> >  scripts/Makefile.lib | 11 +++
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > index f5ab7af0f4..f41b16781d 100644
> > --- a/scripts/Makefile.lib
> > +++ b/scripts/Makefile.lib
> > @@ -179,10 +179,13 @@ ifdef DEVICE_TREE_DEBUG
> >  u_boot_dtsi_options_debug = $(warning $(u_boot_dtsi_options_raw))
> >  endif
> >
> > -# We use the first match
> > -u_boot_dtsi = $(strip $(u_boot_dtsi_options_debug) \
> > +# We use the first match to be included
> > +include_files = $(strip $(u_boot_dtsi_options_debug) \
> >   $(notdir $(firstword $(u_boot_dtsi_options
> >
> > +# The CONFIG_DEVICE_TREE_INCLUDES also need to be included
> > +include_files += $(CONFIG_DEVICE_TREE_INCLUDES)
>
> This is what I wanted, logic-wise, but I think include_files is too
> vague.

Okay. How about dtsi_include_list, or dtsi_list? If not, can you
suggest a name which you think would be apt. Thanks.

-sughosh


Re: [PATCH 0/3] Allwinner R528/T113s PSCI

2023-08-14 Thread Sam Edwards

On 8/14/23 08:06, Andre Przywara wrote:

Hi Sam,

many many thanks for sending this, I especially like your clean up around
the #ifdef's!

The patches looks good on the first glance (apart from some regression in
patch 3/3), but I will reply to them individually.

Cheers,
Andre


Thanks for your attention to these! While you're in a PSCI headspace, 
could you also look at my patch titled:

sunxi: psci: remove redundant initialization from psci_arch_init

...dated May 31?

This one should be pretty uncontroversial and straightforward to review. :)

Many thanks,
Sam


Re: [PATCH 1/3] sunxi: psci: clean away preprocessor macros

2023-08-14 Thread Sam Edwards

Hi Andre,

On 8/14/23 10:37, Andre Przywara wrote:

So I think we can get rid of this:
- GEN_H6 never compiles this code here, as both H6 and H616 are arm64.


Easy!


- We can define SUNXI_PRCM_BASE for NCAT2, I believe Samuel once
mentioned that the D1/T113 does have such a block, actually.


Will you be taking care of this in v2 of your T113s series, or should I 
be adding it (in which case I'll need to know the location of the block)?



- The non-existing cpu_pwr_clamp member should go away when you switch to
a BASE_ADDR + REG_OFFSET approach, I think.


Less easy, but still can do.


Shouldn't that be the opposite? In the existing code, sun6i and H3 DO
program the clamp (see the "-" section above).


And sun7i and R40, as well. It appears I simply read the #if 
defined(...) mess backwards. I'll fix that for v2. As a bonus, this 
lends itself to a rather nice refactoring of sunxi_cpu_set_power() where 
I can have the if block only determine the pwroff/clamp addresses, and 
have a single tail-call to sunxi_power_switch() at the bottom. Since the 
latter function is so simple, I may as well just inline it into 
sunxi_cpu_set_power() (which I suspect might be more readable).



Cheers,
Andre


Likewise,
Sam


[PATCH v2 4/4] doc: qemu: arm: Add a section on booting Linux distros

2023-08-14 Thread Alper Nebi Yasak
Add an example qemu-system-aarch64 command that can make U-Boot on QEMU
boot into the Debian Installer, along with resulting console messages
from U-Boot, based on the existing documentation section for the x86
version.

Signed-off-by: Alper Nebi Yasak 
---
I actually want to put the root.img device first so that the VM can boot
into the installed system when it reboots, but U-Boot can't find the
bootflow on the second drive. I tried e.g. `bootdev list -p; bootflow
scan -lab`, but it seems to only ever search the first virtio-blk.
However, `eficonfig; bootefi bootmgr` makes it boot somehow.

Changes in v2:
- Add new patch for doc section on booting Linux distros

 doc/board/emulation/qemu-arm.rst | 68 
 1 file changed, 68 insertions(+)

diff --git a/doc/board/emulation/qemu-arm.rst b/doc/board/emulation/qemu-arm.rst
index 8ec5349fc9ea..78bcc3ee44c0 100644
--- a/doc/board/emulation/qemu-arm.rst
+++ b/doc/board/emulation/qemu-arm.rst
@@ -98,6 +98,74 @@ can be enabled with the following command line parameters:
 
 These have been tested in QEMU 2.9.0 but should work in at least 2.5.0 as well.
 
+Booting distros
+---
+
+It is possible to install and boot a standard Linux distribution using
+qemu_arm64 by setting up a root disk::
+
+qemu-img create root.img 20G
+
+then using the installer to install. For example, with Debian 12::
+
+qemu-system-aarch64 \
+  -machine virt -cpu cortex-a53 -m 4G -smp 4 \
+  -bios u-boot.bin \
+  -serial stdio -device VGA \
+  -nic user,model=virtio-net-pci \
+  -device virtio-rng-pci \
+  -device qemu-xhci,id=xhci \
+  -device usb-kbd -device usb-tablet \
+  -drive 
if=virtio,file=debian-12.0.0-arm64-netinst.iso,format=raw,readonly=on,media=cdrom
 \
+  -drive if=virtio,file=root.img,format=raw,media=disk
+
+The output will be something like this::
+
+U-Boot 2023.10-rc2-00075-gbe8fbe718e35 (Aug 11 2023 - 08:38:49 +)
+
+DRAM:  4 GiB
+Core:  51 devices, 14 uclasses, devicetree: board
+Flash: 64 MiB
+Loading Environment from Flash... *** Warning - bad CRC, using default 
environment
+
+In:serial,usbkbd
+Out:   serial,vidconsole
+Err:   serial,vidconsole
+Bus xhci_pci: Register 8001040 NbrPorts 8
+Starting the controller
+USB XHCI 1.00
+scanning bus xhci_pci for devices... 3 USB Device(s) found
+Net:   eth0: virtio-net#32
+Hit any key to stop autoboot:  0
+Scanning for bootflows in all bootdevs
+Seq  Method   State   UclassPart  Name  
Filename
+---  ---  --        

+Scanning global bootmeth 'efi_mgr':
+Scanning bootdev 'fw-cfg@902.bootdev':
+fatal: no kernel available
+scanning bus for devices...
+Scanning bootdev 'virtio-blk#34.bootdev':
+  0  efi  ready   virtio   2  virtio-blk#34.bootdev.par 
efi/boot/bootaa64.efi
+** Booting bootflow 'virtio-blk#34.bootdev.part_2' with efi
+Using prior-stage device tree
+Failed to load EFI variables
+Error: writing contents
+** Unable to write file ubootefi.var **
+Failed to persist EFI variables
+Missing TPMv2 device for EFI_TCG_PROTOCOL
+Booting /efi\boot\bootaa64.efi
+Error: writing contents
+** Unable to write file ubootefi.var **
+Failed to persist EFI variables
+Welcome to GRUB!
+
+Standard boot looks through various available devices and finds the virtio
+disks, then boots from the first one. After a second or so the grub menu 
appears
+and you can work through the installer flow normally.
+
+After the installation, you can boot into the installed system by running QEMU
+again without the drive argument corresponding to the installer CD image.
+
 Enabling TPMv2 support
 --
 
-- 
2.40.1



[PATCH v2 3/4] arm: qemu: Enable usb keyboard as an input device

2023-08-14 Thread Alper Nebi Yasak
Commit 02be57caf730 ("riscv: qemu: Enable usb keyboard as an input
device") adds PCI xHCI support to QEMU RISC-V virtual machines and
enables using a USB keyboard as one of the input devices. Similarly,
enable those for ARM virtual machines as well.

Signed-off-by: Alper Nebi Yasak 
Reviewed-by: Simon Glass 
Reviewed-by: Bin Meng 
---

Changes in v2:
- Add tag: "Reviewed-by: Simon Glass "
- Add tag: "Reviewed-by: Bin Meng "

 arch/arm/Kconfig  | 5 +
 board/emulation/qemu-arm/qemu-arm.c   | 5 +
 board/emulation/qemu-arm/qemu-arm.env | 2 +-
 configs/qemu_arm64_defconfig  | 2 --
 configs/qemu_arm_defconfig| 2 --
 doc/board/emulation/qemu-arm.rst  | 4 
 6 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 89b978797720..1fd3ccd1607f 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1041,6 +1041,11 @@ config ARCH_QEMU
imply SYS_WHITE_ON_BLACK
imply SYS_CONSOLE_IS_IN_ENV
imply PRE_CONSOLE_BUFFER
+   imply USB
+   imply USB_XHCI_HCD
+   imply USB_XHCI_PCI
+   imply USB_KEYBOARD
+   imply CMD_USB
 
 config ARCH_RMOBILE
bool "Renesas ARM SoCs"
diff --git a/board/emulation/qemu-arm/qemu-arm.c 
b/board/emulation/qemu-arm/qemu-arm.c
index dfea0d92a3c8..942f1fff5717 100644
--- a/board/emulation/qemu-arm/qemu-arm.c
+++ b/board/emulation/qemu-arm/qemu-arm.c
@@ -11,6 +11,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -114,6 +115,10 @@ int board_late_init(void)
 */
virtio_init();
 
+   /* start usb so that usb keyboard can be used as input device */
+   if (CONFIG_IS_ENABLED(USB_KEYBOARD))
+   usb_init();
+
return 0;
 }
 
diff --git a/board/emulation/qemu-arm/qemu-arm.env 
b/board/emulation/qemu-arm/qemu-arm.env
index 86a99a2e8713..fb4adef281ed 100644
--- a/board/emulation/qemu-arm/qemu-arm.env
+++ b/board/emulation/qemu-arm/qemu-arm.env
@@ -2,7 +2,7 @@
 
 /* environment for qemu-arm and qemu-arm64 */
 
-stdin=serial
+stdin=serial,usbkbd
 stdout=serial,vidconsole
 stderr=serial,vidconsole
 fdt_high=0x
diff --git a/configs/qemu_arm64_defconfig b/configs/qemu_arm64_defconfig
index 94bd96678443..f6b8ae530a4a 100644
--- a/configs/qemu_arm64_defconfig
+++ b/configs/qemu_arm64_defconfig
@@ -35,7 +35,6 @@ CONFIG_CMD_NVEDIT_EFI=y
 CONFIG_CMD_DFU=y
 CONFIG_CMD_MTD=y
 CONFIG_CMD_PCI=y
-CONFIG_CMD_USB=y
 CONFIG_CMD_TPM=y
 CONFIG_CMD_MTDPARTS=y
 CONFIG_ENV_IS_IN_FLASH=y
@@ -68,7 +67,6 @@ CONFIG_SYSRESET=y
 CONFIG_SYSRESET_CMD_POWEROFF=y
 CONFIG_SYSRESET_PSCI=y
 CONFIG_TPM2_MMIO=y
-CONFIG_USB=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_PCI=y
 CONFIG_TPM=y
diff --git a/configs/qemu_arm_defconfig b/configs/qemu_arm_defconfig
index 7cb1e9f037ff..1347b86f34b1 100644
--- a/configs/qemu_arm_defconfig
+++ b/configs/qemu_arm_defconfig
@@ -36,7 +36,6 @@ CONFIG_CMD_NVEDIT_EFI=y
 CONFIG_CMD_DFU=y
 CONFIG_CMD_MTD=y
 CONFIG_CMD_PCI=y
-CONFIG_CMD_USB=y
 CONFIG_CMD_TPM=y
 CONFIG_CMD_MTDPARTS=y
 CONFIG_ENV_IS_IN_FLASH=y
@@ -69,7 +68,6 @@ CONFIG_SYSRESET=y
 CONFIG_SYSRESET_CMD_POWEROFF=y
 CONFIG_SYSRESET_PSCI=y
 CONFIG_TPM2_MMIO=y
-CONFIG_USB=y
 CONFIG_USB_EHCI_HCD=y
 CONFIG_USB_EHCI_PCI=y
 CONFIG_TPM=y
diff --git a/doc/board/emulation/qemu-arm.rst b/doc/board/emulation/qemu-arm.rst
index 1108fe5f8184..8ec5349fc9ea 100644
--- a/doc/board/emulation/qemu-arm.rst
+++ b/doc/board/emulation/qemu-arm.rst
@@ -84,6 +84,10 @@ can be enabled with the following command line parameters:
 
 -device usb-ehci,id=ehci
 
+- To add a USB keyboard attached to an emulated xHCI controller, pass e.g.::
+
+-device qemu-xhci,id=xhci -device usb-kbd,bus=xhci.0
+
 - To add an NVMe disk, pass e.g.::
 
 -drive if=none,file=disk.img,id=mydisk -device nvme,drive=mydisk,serial=foo
-- 
2.40.1



[PATCH v2 2/4] arm: qemu: Enable PRE_CONSOLE_BUFFER

2023-08-14 Thread Alper Nebi Yasak
Commit 608b80b5b855 ("riscv: qemu: Enable PRE_CONSOLE_BUFFER") enables
buffering console messages for QEMU RISC-V virtual machines so those
printed before the video console is available will still show up on the
display. Similarly, enable it for ARM virtual machines as well.

Signed-off-by: Alper Nebi Yasak 
Reviewed-by: Simon Glass 
Reviewed-by: Bin Meng 
---
Here are select values from bdinfo and env:

DRAM bank   = 0x
-> start= 0x4000
-> size = 0x0800
relocaddr   = 0x476d6000
fdt_blob= 0x46595d90
lmb_dump_all:
 memory.cnt = 0x1 / max = 0x10
 memory[0]  [0x4000-0x47ff], 0x0800 bytes flags: 0
 reserved.cnt = 0x2 / max = 0x10
 reserved[0][0x45591000-0x47ff], 0x02a6f000 bytes flags: 0
 reserved[1][0x46591760-0x47ff], 0x01a6e8a0 bytes flags: 0
TLB addr= 0x47fe
irq_sp  = 0x46595d80
sp start= 0x46595d80

fdt_addr= 0x4000
scriptaddr  = 0x4020
loadaddr= 0x4020
pxefile_addr_r  = 0x4030
kernel_addr_r   = 0x4040
ramdisk_addr_r  = 0x4400
fdtcontroladdr  =   465e5ea0

Changes in v2:
- Add tag: "Reviewed-by: Simon Glass "
- Add tag: "Reviewed-by: Bin Meng "

 arch/arm/Kconfig | 1 +
 board/emulation/qemu-arm/Kconfig | 4 
 2 files changed, 5 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 0d4654fb9dfc..89b978797720 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1040,6 +1040,7 @@ config ARCH_QEMU
imply VIDEO_BOCHS
imply SYS_WHITE_ON_BLACK
imply SYS_CONSOLE_IS_IN_ENV
+   imply PRE_CONSOLE_BUFFER
 
 config ARCH_RMOBILE
bool "Renesas ARM SoCs"
diff --git a/board/emulation/qemu-arm/Kconfig b/board/emulation/qemu-arm/Kconfig
index ed9949651c4b..09c95413a541 100644
--- a/board/emulation/qemu-arm/Kconfig
+++ b/board/emulation/qemu-arm/Kconfig
@@ -12,6 +12,10 @@ config BOARD_SPECIFIC_OPTIONS # dummy
imply VIRTIO_NET
imply VIRTIO_BLK
 
+config PRE_CON_BUF_ADDR
+   hex
+   default 0x4010
+
 endif
 
 if TARGET_QEMU_ARM_64BIT && !TFABOOT
-- 
2.40.1



[PATCH v2 1/4] arm: qemu: Enable Bochs video support

2023-08-14 Thread Alper Nebi Yasak
Commit 716161663ec49 ("riscv: qemu: Enable Bochs video support") enables
a video console for QEMU RISC-V virtual machines using an emulated Bochs
VGA card. Similarly, enable it for ARM virtual machines as well.

Signed-off-by: Alper Nebi Yasak 
Reviewed-by: Bin Meng 
---

Changes in v2:
- Add tag: "Reviewed-by: Bin Meng "

 arch/arm/Kconfig  | 4 
 board/emulation/qemu-arm/qemu-arm.env | 3 +++
 doc/board/emulation/qemu-arm.rst  | 4 
 3 files changed, 11 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 97c25b4f146d..0d4654fb9dfc 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1036,6 +1036,10 @@ config ARCH_QEMU
imply DM_RTC
imply RTC_PL031
imply OF_HAS_PRIOR_STAGE
+   imply VIDEO
+   imply VIDEO_BOCHS
+   imply SYS_WHITE_ON_BLACK
+   imply SYS_CONSOLE_IS_IN_ENV
 
 config ARCH_RMOBILE
bool "Renesas ARM SoCs"
diff --git a/board/emulation/qemu-arm/qemu-arm.env 
b/board/emulation/qemu-arm/qemu-arm.env
index e658d5ee7d63..86a99a2e8713 100644
--- a/board/emulation/qemu-arm/qemu-arm.env
+++ b/board/emulation/qemu-arm/qemu-arm.env
@@ -2,6 +2,9 @@
 
 /* environment for qemu-arm and qemu-arm64 */
 
+stdin=serial
+stdout=serial,vidconsole
+stderr=serial,vidconsole
 fdt_high=0x
 initrd_high=0x
 fdt_addr=0x4000
diff --git a/doc/board/emulation/qemu-arm.rst b/doc/board/emulation/qemu-arm.rst
index b42d924cc66a..1108fe5f8184 100644
--- a/doc/board/emulation/qemu-arm.rst
+++ b/doc/board/emulation/qemu-arm.rst
@@ -67,6 +67,10 @@ Additional persistent U-Boot environment support can be 
added as follows:
 Additional peripherals that have been tested to work in both U-Boot and Linux
 can be enabled with the following command line parameters:
 
+- To add a video console, remove "-nographic" and add e.g.::
+
+-serial stdio -device VGA
+
 - To add a Serial ATA disk via an Intel ICH9 AHCI controller, pass e.g.::
 
 -drive if=none,file=disk.img,format=raw,id=mydisk \
-- 
2.40.1



[PATCH v2 0/4] arm: qemu: Enable Bochs, console buffering, USB keyboard

2023-08-14 Thread Alper Nebi Yasak
Now that the driver for the Bochs VGA card emulated by QEMU is no longer
limited to x86 architectures [1], this series enables it on arm and
arm64 virtual machines to provide a graphical interface. In line with
that series this also enables console buffering and USB keyboard.

Tested with the Debian 12 installer using GRUB EFI:

  $ tools/buildman/buildman -o build/qemu_arm64 --boards=qemu_arm64 -w
  $ cd build/qemu_arm64
  $ curl -L -o debian.img \
  
https://cdimage.debian.org/debian-cd/current/arm64/iso-cd/debian-12.0.0-arm64-netinst.iso
  $ qemu-system-aarch64 \
  -machine virt -cpu cortex-a53 -m 4G -smp 4 \
  -bios u-boot.bin \
  -serial stdio -device VGA \
  -nic user,model=virtio-net-pci \
  -device virtio-rng-pci \
  -device qemu-xhci,id=xhci -device usb-kbd -device usb-tablet \
  -drive if=virtio,file=debian.img,format=raw,readonly=on,media=cdrom

And with one using extlinux.conf:

$ [...]
$ curl -L -o head.img.gz \

https://deb.debian.org/debian/dists/bookworm/main/installer-arm64/current/images/netboot/SD-card-images/gtk/firmware.none.img.gz
$ curl -L -o partition.img.gz \

https://deb.debian.org/debian/dists/bookworm/main/installer-arm64/current/images/netboot/SD-card-images/gtk/partition.img.gz
$ zcat head.img.gz partition.img.gz >debian.img
$ [...]

Both can get to a graphical installer just fine, in addition to U-Boot
video console showing up in a GTK window.

[1] video: bochs: Remove the x86 limitation
https://lore.kernel.org/u-boot/20230723044041.1089804-1-bm...@tinylab.org/

Changes in v2:
- Add new patch for doc section on booting Linux distros
- Improve and simplify the qemu command line used for testing

v1: 
https://lore.kernel.org/u-boot/20230808191901.1268429-1-alpernebiya...@gmail.com/

Alper Nebi Yasak (4):
  arm: qemu: Enable Bochs video support
  arm: qemu: Enable PRE_CONSOLE_BUFFER
  arm: qemu: Enable usb keyboard as an input device
  doc: qemu: arm: Add a section on booting Linux distros

 arch/arm/Kconfig  | 10 
 board/emulation/qemu-arm/Kconfig  |  4 ++
 board/emulation/qemu-arm/qemu-arm.c   |  5 ++
 board/emulation/qemu-arm/qemu-arm.env |  3 ++
 configs/qemu_arm64_defconfig  |  2 -
 configs/qemu_arm_defconfig|  2 -
 doc/board/emulation/qemu-arm.rst  | 76 +++
 7 files changed, 98 insertions(+), 4 deletions(-)


base-commit: 832148f675e427060be074c276956962fa9b5cb6
-- 
2.40.1



Re: [PATCH v4 1/2] musb-new: ti-musb: Handle usb dual-role feature as peripheral

2023-08-14 Thread Tom Rini
On Thu, Jul 06, 2023 at 12:40:22PM +0200, Julien Panis wrote:

> This prevents from getting some 'No USB device found' error,
> in usb_ether_init() function for instance.
> 
> Signed-off-by: Julien Panis 
> Reviewed-by: Tony Lindgren 
> Reviewed-by: Nishanth Menon 
> ---
>  drivers/usb/musb-new/ti-musb.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/usb/musb-new/ti-musb.c b/drivers/usb/musb-new/ti-musb.c
> index 3be3f93dd85d..44697dc31387 100644
> --- a/drivers/usb/musb-new/ti-musb.c
> +++ b/drivers/usb/musb-new/ti-musb.c
> @@ -302,6 +302,7 @@ static int ti_musb_wrapper_bind(struct udevice *parent)
>   dr_mode = usb_get_dr_mode(node);
>   switch (dr_mode) {
>   case USB_DR_MODE_PERIPHERAL:
> + case USB_DR_MODE_OTG:
>   /* Bind MUSB device */
>   ret = device_bind_driver_to_node(parent,
>"ti-musb-peripheral",

Julien, why don't we support OTG mode here instead?

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v3 09/12] usb: cdns3: Handle otg mode as peripheral

2023-08-14 Thread Tom Rini
On Thu, Jul 13, 2023 at 03:45:42PM +0200, Julien Panis wrote:

> Override 'otg' to 'peripheral' mode, since 'otg' mode
> is not yet supported by u-boot.
> 
> Signed-off-by: Julien Panis 
> Suggested-by: Roger Quadros 
> ---
>  drivers/usb/cdns3/core.c | 4 
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/usb/cdns3/core.c b/drivers/usb/cdns3/core.c
> index 644a9791b9c9..bd763fc593e1 100644
> --- a/drivers/usb/cdns3/core.c
> +++ b/drivers/usb/cdns3/core.c
> @@ -149,6 +149,10 @@ static int cdns3_core_init_role(struct cdns3 *cdns)
>  
>   dr_mode = best_dr_mode;
>  
> + /* u-boot doesn't yet support OTG so limit to PERIPHERAL */
> + if (dr_mode == USB_DR_MODE_OTG)
> + dr_mode = USB_DR_MODE_PERIPHERAL;
> +
>  #if defined(CONFIG_SPL_USB_HOST) || !defined(CONFIG_SPL_BUILD)
>   if (dr_mode == USB_DR_MODE_OTG || dr_mode == USB_DR_MODE_HOST) {
>   ret = cdns3_host_init(cdns);

Julien, why don't we support otg mode here?

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v3 01/12] usb: dwc3: Handle unknown mode as otg

2023-08-14 Thread Tom Rini
On Thu, Jul 13, 2023 at 03:45:34PM +0200, Julien Panis wrote:

> In case dr_mode attribute is not passed via DT,
> USB DRD controllers should default to OTG.
> 
> Signed-off-by: Julien Panis 
> Acked-by: Roger Quadros 
> ---
>  drivers/usb/dwc3/dwc3-generic.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
> index 66da5a8d6f8c..857ae806e11d 100644
> --- a/drivers/usb/dwc3/dwc3-generic.c
> +++ b/drivers/usb/dwc3/dwc3-generic.c
> @@ -182,8 +182,8 @@ static int dwc3_generic_of_to_plat(struct udevice *dev)
>   node = dev_ofnode(dev->parent);
>   plat->dr_mode = usb_get_dr_mode(node);
>   if (plat->dr_mode == USB_DR_MODE_UNKNOWN) {
> - pr_err("Invalid usb mode setup\n");
> - return -ENODEV;
> + pr_info("No USB mode specified. Using OTG.\n");
> + plat->dr_mode = USB_DR_MODE_OTG;
>   }
>   }

Is this part OK with you Marek?

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH 1/3] sunxi: psci: clean away preprocessor macros

2023-08-14 Thread Andre Przywara
On Fri, 11 Aug 2023 18:30:53 -0600
Sam Edwards  wrote:

Hi Sam,

many thanks for that cleanup, that's very much welcome!

I am still comparing the outcome for the different SoC families, and
testing this on the boards I have, but two things I stumbled upon already:

> This patch restructures psci.c to get away from the "many different
> function definitions switched by #ifdef" paradigm to the preferred style
> of having a single function definition with `if (IS_ENABLED(...))` to
> make the optimizer include only the appropriate function bodies instead.
> 
> There are no functional changes here.
> 
> Signed-off-by: Sam Edwards 
> ---
>  arch/arm/cpu/armv7/sunxi/psci.c | 94 ++---
>  1 file changed, 41 insertions(+), 53 deletions(-)
> 
> diff --git a/arch/arm/cpu/armv7/sunxi/psci.c b/arch/arm/cpu/armv7/sunxi/psci.c
> index e1d3638b5c..7809b074f8 100644
> --- a/arch/arm/cpu/armv7/sunxi/psci.c
> +++ b/arch/arm/cpu/armv7/sunxi/psci.c
> @@ -76,28 +76,24 @@ static void __secure __mdelay(u32 ms)
>   isb();
>  }
>  
> -static void __secure clamp_release(u32 __maybe_unused *clamp)
> +static void __secure clamp_release(u32 *clamp)
>  {
> -#if defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN7I) || \
> - defined(CONFIG_MACH_SUN8I_H3) || \
> - defined(CONFIG_MACH_SUN8I_R40)
> - u32 tmp = 0x1ff;
> - do {
> - tmp >>= 1;
> - writel(tmp, clamp);
> - } while (tmp);
> -
> - __mdelay(10);
> -#endif
> + if (clamp) {
> + u32 tmp = 0x1ff;
> + do {
> + tmp >>= 1;
> + writel(tmp, clamp);
> + } while (tmp);
> +
> + __mdelay(10);
> + }
>  }
>  
> -static void __secure clamp_set(u32 __maybe_unused *clamp)
> +static void __secure clamp_set(u32 *clamp)
>  {
> -#if defined(CONFIG_MACH_SUN6I) || defined(CONFIG_MACH_SUN7I) || \
> - defined(CONFIG_MACH_SUN8I_H3) || \
> - defined(CONFIG_MACH_SUN8I_R40)
> - writel(0xff, clamp);
> -#endif
> + if (clamp) {
> + writel(0xff, clamp);
> + }
>  }
>  
>  static void __secure sunxi_power_switch(u32 *clamp, u32 *pwroff, bool on,
> @@ -118,53 +114,45 @@ static void __secure sunxi_power_switch(u32 *clamp, u32 
> *pwroff, bool on,
>   }
>  }
>  
> -#ifdef CONFIG_MACH_SUN8I_R40
> -/* secondary core entry address is programmed differently on R40 */
>  static void __secure sunxi_set_entry_address(void *entry)
>  {
> - writel((u32)entry,
> -SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
> -}
> -#else
> -static void __secure sunxi_set_entry_address(void *entry)
> -{
> - struct sunxi_cpucfg_reg *cpucfg =
> - (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
> + /* secondary core entry address is programmed differently on R40 */
> + if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
> + writel((u32)entry,
> +SUNXI_SRAMC_BASE + SUN8I_R40_SRAMC_SOFT_ENTRY_REG0);
> + } else {
> + struct sunxi_cpucfg_reg *cpucfg =
> + (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
>  
> - writel((u32)entry, >priv0);
> + writel((u32)entry, >priv0);
> + }
>  }
> -#endif
>  
> -#ifdef CONFIG_MACH_SUN7I
> -/* sun7i (A20) is different from other single cluster SoCs */
> -static void __secure sunxi_cpu_set_power(int __always_unused cpu, bool on)
> -{
> - struct sunxi_cpucfg_reg *cpucfg =
> - (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
> -
> - sunxi_power_switch(>cpu1_pwr_clamp, >cpu1_pwroff,
> -on, 0);
> -}
> -#elif defined CONFIG_MACH_SUN8I_R40
>  static void __secure sunxi_cpu_set_power(int cpu, bool on)
>  {
>   struct sunxi_cpucfg_reg *cpucfg =
>   (struct sunxi_cpucfg_reg *)SUNXI_CPUCFG_BASE;
>  
> - sunxi_power_switch((void *)cpucfg + SUN8I_R40_PWR_CLAMP(cpu),
> -(void *)cpucfg + SUN8I_R40_PWROFF,
> -on, cpu);
> -}
> -#else /* ! CONFIG_MACH_SUN7I && ! CONFIG_MACH_SUN8I_R40 */
> -static void __secure sunxi_cpu_set_power(int cpu, bool on)
> -{
> - struct sunxi_prcm_reg *prcm =
> - (struct sunxi_prcm_reg *)SUNXI_PRCM_BASE;
> + /* sun7i (A20) is different from other single cluster SoCs */
> + if (IS_ENABLED(CONFIG_MACH_SUN7I)) {
> + sunxi_power_switch(NULL, >cpu1_pwroff, on, 0);
> + } else if (IS_ENABLED(CONFIG_MACH_SUN8I_R40)) {
> + sunxi_power_switch(NULL, (void *)cpucfg + SUN8I_R40_PWROFF,
> +on, cpu);
> + } else {
> +#if !defined(CONFIG_SUN50I_GEN_H6) && !defined(CONFIG_SUNXI_GEN_NCAT2)

So I think we can get rid of this:
- GEN_H6 never compiles this code here, as both H6 and H616 are arm64.
- We can define SUNXI_PRCM_BASE for NCAT2, I believe Samuel once
mentioned that the D1/T113 does have such a block, actually.
- The non-existing cpu_pwr_clamp member should go away when you switch to
a BASE_ADDR + 

Re: [PATCH v8 0/7] Tegra: add ASUS/Google Nexus 7 (2012) support

2023-08-14 Thread Thierry Reding
On Mon, Aug 07, 2023 at 09:49:07AM -0400, Tom Rini wrote:
> On Mon, Jul 31, 2023 at 11:29:19AM -0400, Tom Rini wrote:
> > On Mon, Jul 31, 2023 at 03:19:08PM +0300, Svyatoslav Ryhel wrote:
> > > Hello!
> > > 
> > > It has been a month since the last patchset was sent. Should I re-send 
> > > them?
> > 
> > Not unless things don't apply.  Thierry, can you put together a pull
> > request soon? Thanks!
> 
> Thierry? I'm OK with taking these boards after -rc2 if that helps move
> things along, thanks.

Sorry, I've been out of office for the last couple of weeks. Let me put
this high up on my list of things to get back to once I'm all caught up
with email.

Thierry


signature.asc
Description: PGP signature


[PATCH 2/2] riscv: jh7110: enable riscv,timer in the device tree

2023-08-14 Thread Torsten Duwe
The JH7110 has the arhitectural CPU timer on all 5 rv64 cores.
Note that in the device tree.

Signed-off-by: Torsten Duwe 
---
 arch/riscv/dts/jh7110.dtsi | 9 +
 1 file changed, 9 insertions(+)

diff --git a/arch/riscv/dts/jh7110.dtsi b/arch/riscv/dts/jh7110.dtsi
index 081b81..ec237a46ff 100644
--- a/arch/riscv/dts/jh7110.dtsi
+++ b/arch/riscv/dts/jh7110.dtsi
@@ -163,6 +163,15 @@
};
};
 
+   timer {
+   compatible = "riscv,timer";
+   interrupts-extended = <_intc 5>,
+ <_intc 5>,
+ <_intc 5>,
+ <_intc 5>,
+ <_intc 5>;
+   };
+
osc: oscillator {
compatible = "fixed-clock";
clock-output-names = "osc";
-- 
2.35.3



[PATCH 1/2] riscv: allow riscv timer to be instantiated via device tree

2023-08-14 Thread Torsten Duwe
For the architectural timer on riscv, there already is a defined
device tree binding[1]. Allow timer instances to be created from
device tree matches, but for now retain the old mechanism, which
registers the timer biggy-back with the CPU.

[1] linux/Documentation/devicetree/bindings/timer/riscv,timer.yaml

Signed-off-by: Torsten Duwe 
---
 drivers/timer/riscv_timer.c | 28 ++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/timer/riscv_timer.c b/drivers/timer/riscv_timer.c
index 3627ed79b8..28a6a6870b 100644
--- a/drivers/timer/riscv_timer.c
+++ b/drivers/timer/riscv_timer.c
@@ -13,6 +13,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -53,9 +54,26 @@ u64 notrace timer_early_get_count(void)
 static int riscv_timer_probe(struct udevice *dev)
 {
struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
+   u32 rate;
 
-   /* clock frequency was passed from the cpu driver as driver data */
-   uc_priv->clock_rate = dev->driver_data;
+   /*  When this function was called from the CPU driver, clock
+*  frequency is passed as driver data.
+*/
+   rate = dev->driver_data;
+
+   /* When called from an FDT match, the rate needs to be looked up. */
+   if (!rate && gd->fdt_blob) {
+   rate = fdt_getprop_u32_default(gd->fdt_blob,
+  "/cpus", "timebase-frequency", 
0);
+   }
+
+   uc_priv->clock_rate = rate;
+
+   /* With rate==0, timer uclass post_probe might later fail with -EINVAL.
+* Give a hint at the cause for debugging.
+*/
+   if (!rate)
+   log_err("riscv_timer_probe with invalid clock rate 0!\n");
 
return 0;
 }
@@ -64,9 +82,15 @@ static const struct timer_ops riscv_timer_ops = {
.get_count = riscv_timer_get_count,
 };
 
+static const struct udevice_id riscv_timer_ids[] = {
+   { .compatible = "riscv,timer", },
+   { }
+};
+
 U_BOOT_DRIVER(riscv_timer) = {
.name = "riscv_timer",
.id = UCLASS_TIMER,
+   .of_match = of_match_ptr(riscv_timer_ids),
.probe = riscv_timer_probe,
.ops = _timer_ops,
.flags = DM_FLAG_PRE_RELOC,
-- 
2.35.3



[PATCH 0/2] riscv: jh7110: visionfive2: fix u-boot crash due to missing timer

2023-08-14 Thread Torsten Duwe
Hi all,

Since commit 55171aedda8, U-Boot on the visionfive2 stops at

| initcall sequence fffd76f8 failed at call 402192c8 (err=-19)
| ### ERROR ### Please RESET the board ###

This is init_sequence_r[initr_dm_devices] calling dm_timer_init, which
returns ENODEV, because the riscv architectural timer got initialised
at the ROM stage, but then discarded at relocation, and never again to
be re-registered. A workaround hack was to allow it to register again
in drivers/core/root.c line 439:

if (CONFIG_IS_ENABLED(DM_EVENT) /* && !(gd->flags & GD_FLG_RELOC) */ ) {

but that would defeat the purpose of the commit 55171aedda8 cleanup.

However, the timer has a defined device tree binding which, when used,
IMHO makes things a lot clearer. AFAIU, the timer roughly corresponds
to the x86 TSC, which is defined on many x86 platforms, compare
yourself:

grep -r tsc_timer.dtsi arch/x86/dts
vs.
grep -r '"riscv,timer"' arch/riscv/dts

At first I tried to create a series that converts all riscv platforms
to use the DT for the CPU timer and removes the calls from the CPU
driver, but this takes too long. Release early, release often -- these
two patches fix the visionfive2, for a start. Others may follow and
the CPU driver calls can be removed later when they are not needed any
longer.

Torsten

Fixes: 55171aedda8 ("dm: Emit the arch_cpu_init_dm() even only before 
relocation")
---
Torsten Duwe (2):
  riscv: allow riscv timer to be instantiated via device tree
  riscv: jh7110: enable riscv,timer in the device tree

 arch/riscv/dts/jh7110.dtsi  |  9 +
 drivers/timer/riscv_timer.c | 28 ++--
 2 files changed, 35 insertions(+), 2 deletions(-)

-- 
2.35.3



Re: [PATCHv6 04/14] net/lwip: implement dhcp cmd

2023-08-14 Thread Tom Rini
On Mon, Aug 14, 2023 at 09:18:19PM +0600, Maxim Uvarov wrote:
> On Mon, 14 Aug 2023 at 20:21, Ilias Apalodimas 
> wrote:
> 
> > On Mon, Aug 14, 2023 at 07:32:43PM +0600, Maxim Uvarov wrote:
> > > Implement function for dhcp command with lwIP variant. Usage and output
> > is
> > > the same as the original command. This code called by compatibility code
> > > between U-Boot and lwIP.
> >
> > Same as the dns command
> >
> > >
> > > Signed-off-by: Maxim Uvarov 
> > > ---
> > >  include/net/lwip.h | 10 +++
> > >  net/lwip/Makefile  |  1 +
> > >  net/lwip/apps/dhcp/lwip-dhcp.c | 51 ++
> > >  3 files changed, 62 insertions(+)
> > >  create mode 100644 net/lwip/apps/dhcp/lwip-dhcp.c
> > >
> > > diff --git a/include/net/lwip.h b/include/net/lwip.h
> > > index c83b5c8231..2f035280eb 100644
> > > --- a/include/net/lwip.h
> > > +++ b/include/net/lwip.h
> > > @@ -15,3 +15,13 @@ int do_lwip_dns(struct cmd_tbl *cmdtp, int flag, int
> > argc,
> > >  * Other value < 0, if error
> > >  */
> > >  int ulwip_dns(char *name, char *varname);
> > > +
> > > +/*
> > > +* This function creates the DHCP request to obtain IP address. If DHCP
> > server
> >
> > Sphinx needs something more, please check the existing functions
> >
> > > +* returns file name, this file will be downloaded with tftp.  After this
> > > +* function you need to invoke the polling loop to process network
> > communication.
> > > +*
> > > +* Return: 0 if success
> > > +* Other value < 0, if error
> > > +*/
> > > +int ulwip_dhcp(void);
> > > diff --git a/net/lwip/Makefile b/net/lwip/Makefile
> > > index 6d2c00605b..59323fb325 100644
> > > --- a/net/lwip/Makefile
> > > +
> > > +static struct dhcp dhcp;
> > > +
> > > +static int ulwip_dhcp_tmo(void)
> > > +{
> > > + switch (dhcp.state) {
> > > + case DHCP_STATE_BOUND:
> > > + env_set("bootfile", dhcp.boot_file_name);
> > > + env_set("ipaddr", ip4addr_ntoa(_ip_addr));
> > > + env_set("netmask", ip4addr_ntoa(_sn_mask));
> > > + env_set("serverip", ip4addr_ntoa(_ip_addr));
> > > + printf("DHCP client bound to address %s\n",
> > ip4addr_ntoa(_ip_addr));
> > > + break;
> > > + default:
> > > + return -1;
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +int ulwip_dhcp(void)
> > > +{
> > > + int err;
> > > + struct netif *netif;
> > > +
> > > + ulwip_set_tmo(ulwip_dhcp_tmo);
> > > + netif = netif_get_by_index(1);
> >
> > What's (1)?
> >
> >
> Only one lwip netif is registered. 1 - here is the index of netif. I don't
> think that there is any definition for that...
> 

And there's only ever going to be one interface (even if we have ipv4
and ipv60 ? If so, define it to something please, thanks.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCHv6 04/14] net/lwip: implement dhcp cmd

2023-08-14 Thread Maxim Uvarov
On Mon, 14 Aug 2023 at 20:21, Ilias Apalodimas 
wrote:

> On Mon, Aug 14, 2023 at 07:32:43PM +0600, Maxim Uvarov wrote:
> > Implement function for dhcp command with lwIP variant. Usage and output
> is
> > the same as the original command. This code called by compatibility code
> > between U-Boot and lwIP.
>
> Same as the dns command
>
> >
> > Signed-off-by: Maxim Uvarov 
> > ---
> >  include/net/lwip.h | 10 +++
> >  net/lwip/Makefile  |  1 +
> >  net/lwip/apps/dhcp/lwip-dhcp.c | 51 ++
> >  3 files changed, 62 insertions(+)
> >  create mode 100644 net/lwip/apps/dhcp/lwip-dhcp.c
> >
> > diff --git a/include/net/lwip.h b/include/net/lwip.h
> > index c83b5c8231..2f035280eb 100644
> > --- a/include/net/lwip.h
> > +++ b/include/net/lwip.h
> > @@ -15,3 +15,13 @@ int do_lwip_dns(struct cmd_tbl *cmdtp, int flag, int
> argc,
> >  * Other value < 0, if error
> >  */
> >  int ulwip_dns(char *name, char *varname);
> > +
> > +/*
> > +* This function creates the DHCP request to obtain IP address. If DHCP
> server
>
> Sphinx needs something more, please check the existing functions
>
> > +* returns file name, this file will be downloaded with tftp.  After this
> > +* function you need to invoke the polling loop to process network
> communication.
> > +*
> > +* Return: 0 if success
> > +* Other value < 0, if error
> > +*/
> > +int ulwip_dhcp(void);
> > diff --git a/net/lwip/Makefile b/net/lwip/Makefile
> > index 6d2c00605b..59323fb325 100644
> > --- a/net/lwip/Makefile
> > +
> > +static struct dhcp dhcp;
> > +
> > +static int ulwip_dhcp_tmo(void)
> > +{
> > + switch (dhcp.state) {
> > + case DHCP_STATE_BOUND:
> > + env_set("bootfile", dhcp.boot_file_name);
> > + env_set("ipaddr", ip4addr_ntoa(_ip_addr));
> > + env_set("netmask", ip4addr_ntoa(_sn_mask));
> > + env_set("serverip", ip4addr_ntoa(_ip_addr));
> > + printf("DHCP client bound to address %s\n",
> ip4addr_ntoa(_ip_addr));
> > + break;
> > + default:
> > + return -1;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int ulwip_dhcp(void)
> > +{
> > + int err;
> > + struct netif *netif;
> > +
> > + ulwip_set_tmo(ulwip_dhcp_tmo);
> > + netif = netif_get_by_index(1);
>
> What's (1)?
>
>
Only one lwip netif is registered. 1 - here is the index of netif. I don't
think that there is any definition for that...



> > +
> > + if (!netif_get_client_data(netif,
> LWIP_NETIF_CLIENT_DATA_INDEX_DHCP))
> > + dhcp_set_struct(netif, );
> > +
> > + err = dhcp_start(netif);
> > + if (err)
> > + printf("dhcp_start error %d\n", err);
> > +
> > + return err;
> > +}
> > --
> > 2.30.2
> >
>
> Thanks
> /Ilias
>


Re: [PATCH] odroid_xu3: Fix board environment variable

2023-08-14 Thread Tom Rini
On Mon, Aug 07, 2023 at 10:18:33PM -0400, Ben Wolsieffer wrote:
> On Wed, Jun 08, 2022 at 02:30:14PM -0400, Tom Rini wrote:
> > When migrating CONFIG_CONS_INDEX to Kconfig, on this platform we changed
> > what "board" evaluated to in the environment.  This in turn meant that
> > we would no longer try and find the correct fdtfile via the normal
> > distro boot logic.  Fix this by overriding board in the default
> > environment, as done on other platforms where CONFIG_SYS_BOARD is not
> > what we want to be in the board environment variable.
> > 
> > Fixes: f76750d11133 ("Convert CONFIG_CONS_INDEX et al to Kconfig")
> > Reported-by: Gabriel Hojda 
> > Tested-by: Gabriel Hojda 
> > Signed-off-by: Tom Rini 
> > ---
> >  include/configs/odroid_xu3.h | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/include/configs/odroid_xu3.h b/include/configs/odroid_xu3.h
> > index eb35d7b4ae2d..360815bc03ee 100644
> > --- a/include/configs/odroid_xu3.h
> > +++ b/include/configs/odroid_xu3.h
> > @@ -86,6 +86,7 @@
> > "rootfstype=ext4\0" \
> > "console=console=ttySAC2,115200n8\0" \
> > "fdtfile=exynos5422-odroidxu3.dtb\0" \
> > +   "board=odroid\0" \
> > "board_name=odroidxu3\0" \
> > "mmcbootdev=0\0" \
> > "mmcrootdev=0\0" \
> > -- 
> > 2.25.1
> > 
> 
> Hi Tom,
> 
> This patch doesn't appear to have fixed the issue. There is a report
> from Debian of it occuring even after this patch was applied [1], and I
> ran into it today as well with 2023.07.02.
> 
> I looked into it a bit, and the problem seems to be that
> set_board_info() constructs the fdtfile variable using CONFIG_SYS_BOARD,
> so setting the correct value of the board variable has no effect. I
> think either set_board_info() needs to construct fdtfile differently, or
> things need to be restructured to allow CONFIG_SYS_BOARD to be set as
> "odroid".
> 
> I would submit a patch, but I'm not sure of the right approach to take,
> and I would appreciate your help.

Thanks for the detailed report. I believe the problem is that
board/samsung/common/misc.c::set_board_info() should be doing a lot more
checks for if the environment variable is already set, before
over-riding them.  This is typically done around ethaddr (so there's a
number of examples to look at) and in this case I think everything other
than soc_rev / soc_id should have a check before blindly overwriting.
However, just checking for board_name / fdtfile already being set
before setting them is enough to fix this problem I think.

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCHv6 03/14] net/lwip: implement dns cmd

2023-08-14 Thread Maxim Uvarov
On Mon, 14 Aug 2023 at 20:19, Ilias Apalodimas 
wrote:

> On Mon, Aug 14, 2023 at 07:32:42PM +0600, Maxim Uvarov wrote:
> > Implement function for dns command with lwIP variant. Usage and output is
> > the same as the original command. This code called by compatibility code
> > between U-Boot and lwIP.
>
> What's compatibility code?
> I think something along the lines of
>
> 'U-Boot recently got support for an alternative network stack using LWIP.
> Replace  command with the LWIP variant while keeping the output and
> error messages identical"
>

ok, that message is a good one. By 'compatibility code' I meant code to
merge lwip applications and other U-Boot code.

So in general all net/lwip/apps/{dns,ping,wget} and etc. These applications
do not have U-Boot headers and can be compiled with
any other lwIP port. I.e. for example, you can take the linux userland port
with a tap device and directly compile these apps, run and debug.
This might be named lwIP apps code.

And code which does lwIP init, calls application initialization, then goes
to net polling loop - I named here 'a compatibility code'. Might be not the
base naming, but that was an idea.



>
> > +
> > +int do_lwip_dns(struct cmd_tbl *cmdtp, int flag, int argc,
> > + char *const argv[]);
> > +
> > +/*
> > +* This function creates the DNS request to resolve a domain host name.
> Function
>
> You need the name of the function as well.  Please have a look at how the
> rest of the code is documented.
>
> > +* can return immediately if previous request was cached or it might
> require
> > +* entering the polling loop for a request to a remote server.
> > +*
> > +* @name  dns name to resolve
>
> @name: etc
>
> > +* @varname (optional) U-Boot variable name to store the result
> > +* Return: ERR_OK(0) for fetching entry from the cache
> > +* ERR_INPROGRESS(-5) success, can go to the polling loop
> > +* Other value < 0, if error
> > +*/
> > +int ulwip_dns(char *name, char *varname);
> > diff --git a/net/lwip/Makefile b/net/lwip/Makefile
> > index d1161bea78..6d2c00605b 100644
> > --- a/net/lwip/Makefile
> > + */
> > +
> > +#include 
> > +#include 
> > +#include 
> > +
> > +#include 
> > +#include 
> > +
> > +#include 
> > +
> > +static void dns_found_cb(const char *name, const ip_addr_t *ipaddr,
> void *callback_arg)
> > +{
> > + char *varname = (char *)callback_arg;
> > +
> > + if (varname)
> > + env_set(varname, ip4addr_ntoa(ipaddr));
>
> Why do we have to set the varname to the resolved address?  Is this
> something the dns command already does?
>
> Exactly, original  command is:

U_BOOT_CMD(
dns, 3, 1, do_dns,
"lookup the IP of a hostname",
"hostname [envvar]"
);



> > +
> > + log_info("resolved %s to %s\n",  name, ip4addr_ntoa(ipaddr));
> > + ulwip_exit(0);
> > +}
> > +
> > +int ulwip_dns(char *name, char *varname)
> > +{
> > + int err;
> > + ip_addr_t ipaddr; /* not used */
> > + ip_addr_t dns1;
> > + ip_addr_t dns2;
> > +
> > + ipaddr_aton(env_get("dnsip"), );
> > + ipaddr_aton(env_get("dnsip2"), );
> > +
> > + dns_init();
> > + dns_setserver(0, );
> > + dns_setserver(1, );
> > +
> > + err = dns_gethostbyname(name, , dns_found_cb, varname);
> > + if (err == ERR_OK)
> > + dns_found_cb(name, , varname);
> > +
> > + return err;
> > +}
> > --
> > 2.30.2
> >
>
> Thanks
> /Ilias
>


Re: [PATCH 1/1] common: return type board_get_usable_ram_top

2023-08-14 Thread Tom Rini
On Mon, Aug 14, 2023 at 08:31:31AM +0200, Heinrich Schuchardt wrote:
> 
> 
> On 8/13/23 15:36, Simon Glass wrote:
> > Hi Heinrich,
> > 
> > On Sat, 12 Aug 2023 at 23:01, Heinrich Schuchardt
> >  wrote:
> > > 
> > > board_get_usable_ram_top() returns a physical address that is stored in
> > > gd->ram_top. The return type of the function should be phys_addr_t like 
> > > the
> > > current type of gd->ram_top.
> > > 
> > > Signed-off-by: Heinrich Schuchardt 
> > > ---
> > >   arch/arm/mach-imx/imx8m/soc.c   | 2 +-
> > >   arch/arm/mach-mvebu/arm64-common.c  | 2 +-
> > >   arch/arm/mach-rockchip/sdram.c  | 2 +-
> > >   arch/arm/mach-stm32mp/dram_init.c   | 2 +-
> > >   arch/arm/mach-sunxi/board.c | 2 +-
> > >   arch/arm/mach-tegra/board2.c| 2 +-
> > >   arch/mips/mach-jz47xx/jz4780/jz4780.c   | 2 +-
> > >   arch/mips/mach-octeon/dram.c| 2 +-
> > >   arch/riscv/cpu/fu540/dram.c | 2 +-
> > >   arch/riscv/cpu/fu740/dram.c | 2 +-
> > >   arch/riscv/cpu/generic/dram.c   | 2 +-
> > >   arch/riscv/cpu/jh7110/dram.c| 2 +-
> > >   arch/x86/cpu/broadwell/sdram.c  | 2 +-
> > >   arch/x86/cpu/coreboot/sdram.c   | 2 +-
> > >   arch/x86/cpu/efi/payload.c  | 2 +-
> > >   arch/x86/cpu/efi/sdram.c| 2 +-
> > >   arch/x86/cpu/ivybridge/sdram.c  | 2 +-
> > >   arch/x86/cpu/qemu/dram.c| 2 +-
> > >   arch/x86/cpu/quark/dram.c   | 2 +-
> > >   arch/x86/cpu/slimbootloader/sdram.c | 2 +-
> > >   arch/x86/cpu/tangier/sdram.c| 2 +-
> > >   arch/x86/include/asm/u-boot-x86.h   | 2 +-
> > >   arch/x86/lib/fsp1/fsp_dram.c| 2 +-
> > >   arch/x86/lib/fsp2/fsp_dram.c| 2 +-
> > >   board/broadcom/bcmns3/ns3.c | 2 +-
> > >   board/imgtec/boston/ddr.c   | 2 +-
> > >   board/menlo/m53menlo/m53menlo.c | 2 +-
> > >   board/raspberrypi/rpi/rpi.c | 2 +-
> > >   board/ti/am65x/evm.c| 2 +-
> > >   board/ti/j721e/evm.c| 2 +-
> > >   board/ti/j721s2/evm.c   | 2 +-
> > >   board/toradex/verdin-am62/verdin-am62.c | 2 +-
> > >   board/xilinx/common/board.c | 2 +-
> > >   common/board_f.c| 2 +-
> > >   include/init.h  | 2 +-
> > >   35 files changed, 35 insertions(+), 35 deletions(-)
> > 
> > While you are here, could you please fix the function comment?
> > 
> > [..]
> > 
> > > diff --git a/include/init.h b/include/init.h
> > > index 8873081685..b1e1451166 100644
> > > --- a/include/init.h
> > > +++ b/include/init.h
> > > @@ -304,7 +304,7 @@ int show_board_info(void);
> > >*
> > >* @param total_size   Size of U-Boot (unused?)
> > 
> > It seems confused about the param, and we should have a return value.
> 
> The parameter with the monitor length (size of U-Boot code) is not used
> anywhere (though it is passed on to functions like
> mrc_common_board_get_usable_ram_top() where it isn't used either).
> 
> We should try to eliminate the parameter in further patches.
> 
> > 
> > Also should explain what this function is used for.
> 
> That explanation exists.
> 
> I will put the documentation fix into a separate patch so we can apply it to
> 2023.10. I don't know if Tom wants to apply this patch to next or to
> 2023.10.

I'm thinking we should take this for v2023.10.

-- 
Tom


signature.asc
Description: PGP signature


Re: [RFC PATCH 2/4] scripts/Makefile.lib: Embed capsule public key in platform's dtb

2023-08-14 Thread Tom Rini
On Mon, Aug 14, 2023 at 02:33:07PM +0530, Sughosh Ganu wrote:
> The EFI capsule authentication logic in u-boot expects the public key
> in the form of an EFI Signature List(ESL) to be provided as part of
> the platform's dtb. Currently, the embedding of the ESL file into the
> dtb needs to be done manually.
> 
> Add a target for generating a dtsi file which contains the signature
> node with the ESL file included as a property under the signature
> node. Include the dtsi file in the dtb. This brings the embedding of
> the ESL in the dtb into the U-Boot build flow.
> 
> The path to the ESL file is specified through the
> CONFIG_EFI_CAPSULE_ESL_FILE symbol.
> 
> Signed-off-by: Sughosh Ganu 
> ---
>  lib/efi_loader/Kconfig |  9 +
>  lib/efi_loader/capsule_esl.dtsi.in | 11 +++
>  scripts/Makefile.lib   | 17 +
>  3 files changed, 37 insertions(+)
>  create mode 100644 lib/efi_loader/capsule_esl.dtsi.in
> 
> diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> index 9989e3f384..f40a62a0ba 100644
> --- a/lib/efi_loader/Kconfig
> +++ b/lib/efi_loader/Kconfig
> @@ -272,6 +272,15 @@ config EFI_CAPSULE_MAX
> Select the max capsule index value used for capsule report
> variables. This value is used to create CapsuleMax variable.
>  
> +config EFI_CAPSULE_ESL_FILE
> + string "Path to the EFI Signature List File"
> + default ""

No default.

[snip]
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index f41b16781d..cf4eef0b05 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -334,8 +334,25 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
>   ; \
>   sed "s:$(pre-tmp):$(<):" $(depfile).pre.tmp $(depfile).dtc.tmp > 
> $(depfile)
>  
> +ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
> +quiet_cmd_capsule_esl_gen = CAPSULE_ESL_GEN $@
> +cmd_capsule_esl_gen = \
> + $(shell sed "s:ESL_BIN_FILE:$(capsule_esl_path):" 
> $(capsule_esl_input_file) > $@)
> +
> +$(obj)/.capsule_esl.dtsi:
> + $(call cmd_capsule_esl_gen)
> +
> +capsule_esl_input_file=$(srctree)/lib/efi_loader/capsule_esl.dtsi.in
> +capsule_esl_dtsi = .capsule_esl.dtsi
> +capsule_esl_path=$(abspath $(srctree)/$(subst 
> $(quote),,$(CONFIG_EFI_CAPSULE_ESL_FILE)))

This seems right.

> +include_files += $(capsule_esl_dtsi)
> +
> +$(obj)/%.dtb: $(src)/%.dts $(DTC) $(obj)/.capsule_esl.dtsi FORCE
> + $(call if_changed_dep,dtc)
> +else
>  $(obj)/%.dtb: $(src)/%.dts $(DTC) FORCE
>   $(call if_changed_dep,dtc)
> +endif

I think this implies to me that we should have been depending on
$(include_files) (once renamed to be less vague) here already ?

-- 
Tom


signature.asc
Description: PGP signature


Re: [RFC PATCH 1/4] scripts/Makefile.lib: Collate all dtsi files for inclusion

2023-08-14 Thread Tom Rini
On Mon, Aug 14, 2023 at 02:33:06PM +0530, Sughosh Ganu wrote:

> At the time of building a device-tree file, all the *u-boot.dtsi files
> are looked for, in a particular order, and the first file found is
> included. Then, the list of files specified in the
> CONFIG_DEVICE_TREE_INCLUDES symbol are included.
> 
> Combine these files that are to be included into a variable, and then
> include all these files in one go.
> 
> Signed-off-by: Sughosh Ganu 
> ---
>  scripts/Makefile.lib | 11 +++
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> index f5ab7af0f4..f41b16781d 100644
> --- a/scripts/Makefile.lib
> +++ b/scripts/Makefile.lib
> @@ -179,10 +179,13 @@ ifdef DEVICE_TREE_DEBUG
>  u_boot_dtsi_options_debug = $(warning $(u_boot_dtsi_options_raw))
>  endif
>  
> -# We use the first match
> -u_boot_dtsi = $(strip $(u_boot_dtsi_options_debug) \
> +# We use the first match to be included
> +include_files = $(strip $(u_boot_dtsi_options_debug) \
>   $(notdir $(firstword $(u_boot_dtsi_options
>  
> +# The CONFIG_DEVICE_TREE_INCLUDES also need to be included
> +include_files += $(CONFIG_DEVICE_TREE_INCLUDES)

This is what I wanted, logic-wise, but I think include_files is too
vague.

-- 
Tom


signature.asc
Description: PGP signature


Re: Pull request: u-boot-rockchip-20230814

2023-08-14 Thread Tom Rini
On Mon, Aug 14, 2023 at 08:14:13PM +0800, Kever Yang wrote:

> Hi Tom,
> 
> Please pull the updates for rockchip platform:
> - Add board: rk3568 EmbedFire Lubancat 2
> - Fixes for rk3568 clock and pinctrl;
> - Fixes for rk3308 clock and uart;
> - rk3328 rock64 updates;
> - Video fix on veyron board;
> 
> CI:
> https://source.denx.de/u-boot/custodians/u-boot-rockchip/-/pipelines/17348
> 
> Thanks,
> - Kever
> 
> The following changes since commit a5899cc69a99379f01e8e2f9f98e0e09b24f1656:
> 
>   Merge https://source.denx.de/u-boot/custodians/u-boot-watchdog (2023-08-10 
> 11:40:09 -0400)
> 
> are available in the Git repository at:
> 
>   https://source.denx.de/u-boot/custodians/u-boot-rockchip.git 
> tags/u-boot-rockchip-20230814
> 
> for you to fetch changes up to d7009faa098169abd7ff0e4b41af89b17896a7da:
> 
>   pinctrl: rockchip: Fix drive and input schmitt on RK3568 (2023-08-14 
> 11:07:27 +0800)
> 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: Please pull u-boot-video

2023-08-14 Thread Tom Rini
On Mon, Aug 14, 2023 at 08:02:54AM +0200, Anatolij Gustschin wrote:

> Hi Tom,
> 
> please pull video driver fixes for for v2023.10.
> 
> CI: https://source.denx.de/u-boot/custodians/u-boot-video/-/pipelines/17338
> 
> Thanks,
> Anatolij
> 
> The following changes since commit a5899cc69a99379f01e8e2f9f98e0e09b24f1656:
> 
>   Merge https://source.denx.de/u-boot/custodians/u-boot-watchdog (2023-08-10 
> 11:40:09 -0400)
> 
> are available in the Git repository at:
> 
>   https://source.denx.de/u-boot/custodians/u-boot-video.git 
> tags/video-20230814
> 
> for you to fetch changes up to 04cc66c047d959dc1b22a625b7949a26793ac52b:
> 
>   rpi: set the correct parameter for simple framebuffer node (2023-08-13 
> 23:57:46 +0200)
> 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v2 2/2] doc: printf() codes: Fix format specifier for unsigned int

2023-08-14 Thread Tom Rini
On Mon, Aug 14, 2023 at 10:23:48AM +0530, Siddharth Vadapalli wrote:

> The format specifier for the "unsigned int" variable is documented as
> "%d". However, it should be "%u". Thus, fix it.
> 
> Fixes: f5e9035043fb ("doc: printf() codes")
> Reported-by: Tom Rini 
> Signed-off-by: Siddharth Vadapalli 

Reviewed-by: Tom Rini 

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH v2 1/2] net: Fix the displayed value of bytes transferred

2023-08-14 Thread Tom Rini
On Mon, Aug 14, 2023 at 10:23:47AM +0530, Siddharth Vadapalli wrote:

> In the case of NETLOOP_SUCCESS, the decimal value of the u32 variable
> "net_boot_file_size" is printed using "%d", resulting in negative values
> being reported for large file sizes. Fix this by using "%u" to print the
> decimal value corresponding to the bytes transferred.
> 
> Fixes: 1411157d8578 ("net: cosmetic: Fixup var names related to boot file")
> Signed-off-by: Siddharth Vadapalli 

Reviewed-by: Tom Rini 

-- 
Tom


signature.asc
Description: PGP signature


Re: [PATCH] board: stm32mp1: add splash screen with stmicroelectronics logo

2023-08-14 Thread Dario Binacchi
Patrice, All

On Mon, Aug 7, 2023 at 9:41 AM Patrice CHOTARD
 wrote:
>
>
>
> On 7/10/23 21:02, Dario Binacchi wrote:
> > Hi Patrick,
> >
> > On Mon, Jul 10, 2023 at 1:31 PM Patrick Delaunay
> >  wrote:
> >>
> >> Display the STMicroelectronics logo with features VIDEO_LOGO and
> >> SPLASH_SCREEN on STMicroelectronics boards.
> >>
> >> With CONFIG_SYS_VENDOR = "st", the logo st.bmp is selected, loaded at the
> >> address indicated by splashimage and centered with "splashpos=m,m".
> >>
> >> Signed-off-by: Patrick Delaunay 
> >> ---
> >>
> >>  MAINTAINERS   |   1 +
> >>  configs/stm32mp15_basic_defconfig |   3 +++
> >>  configs/stm32mp15_defconfig   |   3 +++
> >>  configs/stm32mp15_trusted_defconfig   |   3 +++
> >>  include/configs/stm32mp15_st_common.h |   4 +++-
> >>  tools/logos/st.bmp| Bin 0 -> 18244 bytes
> >>  6 files changed, 13 insertions(+), 1 deletion(-)
> >>  create mode 100644 tools/logos/st.bmp
> >>
> >> diff --git a/MAINTAINERS b/MAINTAINERS
> >> index d724b6467344..dfe9409bc7fe 100644
> >> --- a/MAINTAINERS
> >> +++ b/MAINTAINERS
> >> @@ -578,6 +578,7 @@ F:  include/dt-bindings/clock/stm32mp*
> >>  F: include/dt-bindings/pinctrl/stm32-pinfunc.h
> >>  F: include/dt-bindings/reset/stm32mp*
> >>  F: include/stm32_rcc.h
> >> +F: tools/logos/st.bmp
> >>  F: tools/stm32image.c
> >>  N: stm
> >>  N: stm32
> >> diff --git a/configs/stm32mp15_basic_defconfig 
> >> b/configs/stm32mp15_basic_defconfig
> >> index 424ae5dbdfaf..9ea5aaa7145a 100644
> >> --- a/configs/stm32mp15_basic_defconfig
> >> +++ b/configs/stm32mp15_basic_defconfig
> >> @@ -171,6 +171,7 @@ CONFIG_USB_GADGET_VENDOR_NUM=0x0483
> >>  CONFIG_USB_GADGET_PRODUCT_NUM=0x5720
> >>  CONFIG_USB_GADGET_DWC2_OTG=y
> >>  CONFIG_VIDEO=y
> >> +CONFIG_VIDEO_LOGO=y
> >>  CONFIG_BACKLIGHT_GPIO=y
> >>  CONFIG_VIDEO_LCD_ORISETECH_OTM8009A=y
> >>  CONFIG_VIDEO_LCD_RAYDIUM_RM68200=y
> >> @@ -178,6 +179,8 @@ CONFIG_VIDEO_STM32=y
> >>  CONFIG_VIDEO_STM32_DSI=y
> >>  CONFIG_VIDEO_STM32_MAX_XRES=1280
> >>  CONFIG_VIDEO_STM32_MAX_YRES=800
> >> +CONFIG_SPLASH_SCREEN=y
> >> +CONFIG_SPLASH_SCREEN_ALIGN=y
> >>  CONFIG_BMP_16BPP=y
> >>  CONFIG_BMP_24BPP=y
> >>  CONFIG_BMP_32BPP=y
> >> diff --git a/configs/stm32mp15_defconfig b/configs/stm32mp15_defconfig
> >> index 2700b5c49910..4d0a81f8a871 100644
> >> --- a/configs/stm32mp15_defconfig
> >> +++ b/configs/stm32mp15_defconfig
> >> @@ -147,6 +147,7 @@ CONFIG_USB_GADGET_VENDOR_NUM=0x0483
> >>  CONFIG_USB_GADGET_PRODUCT_NUM=0x5720
> >>  CONFIG_USB_GADGET_DWC2_OTG=y
> >>  CONFIG_VIDEO=y
> >> +CONFIG_VIDEO_LOGO=y
> >>  CONFIG_BACKLIGHT_GPIO=y
> >>  CONFIG_VIDEO_LCD_ORISETECH_OTM8009A=y
> >>  CONFIG_VIDEO_LCD_RAYDIUM_RM68200=y
> >> @@ -154,6 +155,8 @@ CONFIG_VIDEO_STM32=y
> >>  CONFIG_VIDEO_STM32_DSI=y
> >>  CONFIG_VIDEO_STM32_MAX_XRES=1280
> >>  CONFIG_VIDEO_STM32_MAX_YRES=800
> >> +CONFIG_SPLASH_SCREEN=y
> >> +CONFIG_SPLASH_SCREEN_ALIGN=y
> >>  CONFIG_BMP_16BPP=y
> >>  CONFIG_BMP_24BPP=y
> >>  CONFIG_BMP_32BPP=y
> >> diff --git a/configs/stm32mp15_trusted_defconfig 
> >> b/configs/stm32mp15_trusted_defconfig
> >> index 5b94e0c6d2e7..0a7d8624858d 100644
> >> --- a/configs/stm32mp15_trusted_defconfig
> >> +++ b/configs/stm32mp15_trusted_defconfig
> >> @@ -147,6 +147,7 @@ CONFIG_USB_GADGET_VENDOR_NUM=0x0483
> >>  CONFIG_USB_GADGET_PRODUCT_NUM=0x5720
> >>  CONFIG_USB_GADGET_DWC2_OTG=y
> >>  CONFIG_VIDEO=y
> >> +CONFIG_VIDEO_LOGO=y
> >>  CONFIG_BACKLIGHT_GPIO=y
> >>  CONFIG_VIDEO_LCD_ORISETECH_OTM8009A=y
> >>  CONFIG_VIDEO_LCD_RAYDIUM_RM68200=y
> >> @@ -154,6 +155,8 @@ CONFIG_VIDEO_STM32=y
> >>  CONFIG_VIDEO_STM32_DSI=y
> >>  CONFIG_VIDEO_STM32_MAX_XRES=1280
> >>  CONFIG_VIDEO_STM32_MAX_YRES=800
> >> +CONFIG_SPLASH_SCREEN=y
> >> +CONFIG_SPLASH_SCREEN_ALIGN=y
> >>  CONFIG_BMP_16BPP=y
> >>  CONFIG_BMP_24BPP=y
> >>  CONFIG_BMP_32BPP=y
> >> diff --git a/include/configs/stm32mp15_st_common.h 
> >> b/include/configs/stm32mp15_st_common.h
> >> index b45982a35b8c..60838cb0e3f0 100644
> >> --- a/include/configs/stm32mp15_st_common.h
> >> +++ b/include/configs/stm32mp15_st_common.h
> >> @@ -10,7 +10,9 @@
> >>
> >>  #define STM32MP_BOARD_EXTRA_ENV \
> >> "usb_pgood_delay=2000\0" \
> >> -   "console=ttySTM0\0"
> >> +   "console=ttySTM0\0" \
> >> +   "splashimage=" __stringify(CONFIG_SYS_LOAD_ADDR) "\0" \
> >> +   "splashpos=m,m\0"
> >>
> >>  #include 
> >>
> >> diff --git a/tools/logos/st.bmp b/tools/logos/st.bmp
> >> new file mode 100644
> >> index 
> >> ..f59d3c5cef6b8bce5213a1ef42a9cdaa3c5dbc58
> >> GIT binary patch
> >> literal 18244
> >> zcmeHvcUV-((s!LXVPJqEg9Hfz>WVo>42Tg_%$P;Qw63nQ=A5|ZOKv57>
> >> z1OZ9TpyV(E6ZYo#-mm%$0|+yFpYPxIdH1=!JLh!$s;j%JtE=i1cZ}cI@xcG%#Q^+>
> >> zzm`w{<=7}Nzy`p116Ueq+Hd$w+L-avH{yT(zy1-lp`PqUgI^a@nCfW@{=J-FdS5q~
> >> zIiL&79_$HohV_B~uf8yE)BsrEJs1}H41>kvy >> zm9wV6>eSFAjj8m&}6=feT>cvV{;5v=}x8FNMu30%6Ol
> >> 

  1   2   >