[PULL] u-boot-riscv/master

[Leo Liang]

Hi Tom,

The following changes since commit 15155ab0a3d1f839509bcac620bfb38f950bead6:

  Merge tag 'u-boot-imx-20230923' of 
https://source.denx.de/u-boot/custodians/u-boot-imx (2023-09-24 17:15:31 -0400)

are available in the Git repository at:

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

for you to fetch changes up to 16dbe3d9d45527f67d479535a22dc4054ae93e99:

  riscv: set fdtfile on VisionFive 2 (2023-09-26 10:43:02 +0800)

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

However, this patch has landed in the "next" branch.
Could we cherry-pick this commit to have this patch on master branch ?
 


+ Fix VisionFive2 booting issue by providing the correct FDT.


Heinrich Schuchardt (1):
  riscv: set fdtfile on VisionFive 2

 arch/riscv/Kconfig|  1 +
 board/starfive/visionfive2/starfive_visionfive2.c | 43 
+--
 2 files changed, 42 insertions(+), 2 deletions(-)

Best regards,
Leo

Re: [PATCH v2] riscv: enable multi-range memory layout

[Anup Patel]

On Thu, Sep 14, 2023 at 12:49 PM Heinrich Schuchardt  wrote:
>
> On 9/14/23 08:48, Wu, Fei wrote:
> > On 9/14/2023 2:05 PM, Heinrich Schuchardt wrote:
> >>
> >>
> >> Am 14. September 2023 07:30:55 MESZ schrieb Fei Wu :
> >>> In order to enable PCIe passthrough on qemu riscv, the physical memory
> >>> range between 3GB and 4GB is reserved. Therefore if guest has 4GB ram,
> >>> two ranges are created as [2G, 3G) and [4G, 7G), currently u-boot sets
> >>> ram_top to 4G - 1 if the gd->ram_top is above 4G in
> >>
> >> This should move to 7GiB - 1 in your example on riscv64.
> >>
> > I'm describing the current implementation of board_get_usable_ram_top()
> > in ./arch/riscv/cpu/generic/dram.c. Do you mean this function should be
> > changed? Is the comment about 32bit DMA device still valid?
> >
> > phys_size_t board_get_usable_ram_top(phys_size_t total_size)
> > {
> >  /*
> >   * Ensure that we run from first 4GB so that all
> >   * addresses used by U-Boot are 32bit addresses.
> >   *
> >   * This in-turn ensures that 32bit DMA capable
> >   * devices work fine because DMA mapping APIs will
> >   * provide 32bit DMA addresses only.
> >   */
> >  if (gd->ram_top >= SZ_4G)
> >  return SZ_4G - 1;
> >
> >  return gd->ram_top;
> > }
>
> The comment above says 32bit DMA is board specific and not architecture
> specific. So it is wrong to have this board_get_usable_ram_top()
> function on architecture level. It makes usage of devices with all
> memory above 4 GiB impossible.
>
> It tried to pass through a SATA controller but received an error:
>
> # modprobe vfio-pci
> # echo :06:00.0 > /sys/bus/pci/drivers/ahci/unbind
> # echo 1022 7091 > /sys/bus/pci/drivers/vfio-pci/new_id
> # qemu-system-riscv64 -kernel u-boot.bin -nographic -M virt -m 4G
> -device vfio-pci,host=:06:00.0
> qemu-system-riscv64: -device vfio-pci,host=:06:00.0: VFIO_MAP_DMA
> failed: Invalid argument
> qemu-system-riscv64: -device vfio-pci,host=:06:00.0: vfio
> :06:00.0: failed to setup container for group 5: memory listener
> initialization failed: Region riscv_virt_board.ram:
> vfio_dma_map(0x55adbde66f70, 0x8000, 0x1, 0x7fcd6fe0) =
> -22 (Invalid argument)
>
> With which version of QEMU were you able to use PCI pass through?

The original S-mode SiFive unleashed board support was using the
generic CPU under arch/riscv. We have Cadance MACB ethernet
device on this board which is a 32-bit device and can't access memory
beyond 4GB.

For more details, refer "git show 26f4fd1cb4f2"

I think we can wrap the board_get_usable_ram_top() implementation
in generic RISC-V CPU with a #ifdef Kconfig option which allows
certain boards (such as QEMU-virt) to implement their own
board_get_usable_ram_top().

Regards,
Anup

>
> Best regards
>
> Heinrich
>
> >
> >>> board_get_usable_ram_top(), but that address is not backed by ram. This
> >>> patch selects the lowest range instead.
> >>>
> >>> Signed-off-by: Fei Wu 
> >>> ---
> >>> arch/riscv/cpu/generic/dram.c| 2 +-
> >>> configs/qemu-riscv64_defconfig   | 2 +-
> >>> configs/qemu-riscv64_smode_defconfig | 2 +-
> >>> configs/qemu-riscv64_spl_defconfig   | 2 +-
> >>> 4 files changed, 4 insertions(+), 4 deletions(-)
> >>>
> >>> diff --git a/arch/riscv/cpu/generic/dram.c b/arch/riscv/cpu/generic/dram.c
> >>> index 44e11bd56c..fb53a57b4e 100644
> >>> --- a/arch/riscv/cpu/generic/dram.c
> >>> +++ b/arch/riscv/cpu/generic/dram.c
> >>> @@ -13,7 +13,7 @@ DECLARE_GLOBAL_DATA_PTR;
> >>>
> >>> int dram_init(void)
> >>> {
> >>> -   return fdtdec_setup_mem_size_base();
> >>> +   return fdtdec_setup_mem_size_base_lowest();
> >>
> >> Linaro is working on allowing to download a distro image via https and 
> >> installing from a RAM disk.
> >>
> >> We should not artificially reduce the RAM size available for U-Boot by 
> >> restricting ourselfs to 1 GiB.
> >>
> >> We must ensure that ram top is in the upper range.
> >>
> > How do they handle the case of >4GB? board_get_usable_ram_top() attached
> > above always returns <4GB. And it seems like
> > fdtdec_setup_mem_size_base() cannot ensure the upper one is picked, it
> > just picks up one, but which one is selected depending on fdt.
> >
> > Thanks,
> > Fei.
> >
> >> Best regards
> >>
> >> Heinrich
> >>
> >>> }
> >>>
> >>> int dram_init_banksize(void)
> >>> diff --git a/configs/qemu-riscv64_defconfig 
> >>> b/configs/qemu-riscv64_defconfig
> >>> index 9a8bbef192..aa55317d26 100644
> >>> --- a/configs/qemu-riscv64_defconfig
> >>> +++ b/configs/qemu-riscv64_defconfig
> >>> @@ -1,6 +1,6 @@
> >>> CONFIG_RISCV=y
> >>> CONFIG_SYS_MALLOC_LEN=0x80
> >>> -CONFIG_NR_DRAM_BANKS=1
> >>> +CONFIG_NR_DRAM_BANKS=2
> >>> CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> >>> CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x8020
> >>> CONFIG_ENV_SIZE=0x2
> >>> diff --git a/configs/qemu-riscv64_smode_defconfig 
> >>> b/configs/qemu-riscv64_smode_defconfig
> >>> index 

Re: Trying to boot custom kernel on Wink Hub (i.MX28)

[Rogan Dawes]

Hi Fabio,

That prints "LLC", but does not print "Pref".

Rogan


On Tue, 26 Sept 2023 at 02:15, Fabio Estevam  wrote:

> Hi Rogan,
>
> On Mon, Sep 25, 2023 at 4:27 PM Rogan Dawes  wrote:
> >
> > That outputs LLC about a second after the mxsldr terminates (kinda
> slowly, the individual characters can be seen being emitted one at a time),
> and then about 5 seconds later, "Pref". But nothing after that point.
> >
> > At least it looks like there is something happening, which is excellent!
>
> I pushed a change in my branch. Please give it a try.
>

Re: [PATCH v4 5/8] efi_loader: set EFI HTTP Boot download buffer as reserved

[Masahisa Kojima]

Hi Ilias,

On Mon, 25 Sept 2023 at 21:46, Ilias Apalodimas
 wrote:
>
> Kojima-san,
>
> [...]
> >  /* Carve out DT reserved memory ranges */
> >  void efi_carve_out_dt_rsv(void *fdt);
> >  /* Purge unused kaslr-seed */
> > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> > index 605be5041e..4991056946 100644
> > --- a/lib/efi_loader/efi_bootmgr.c
> > +++ b/lib/efi_loader/efi_bootmgr.c
> > @@ -326,6 +326,11 @@ static efi_status_t try_load_from_uri_path(struct 
> > efi_device_path_uri *uridp,
> >   return EFI_INVALID_PARAMETER;
> >
> >   ret = load_default_file_from_blk_dev(blk, handle);
> > + if (ret != EFI_SUCCESS)
> > + return ret;
> > +
> > + /* whole ramdisk must be reserved */
> > + efi_reserve_memory(image_addr, image_size, true);
>
> Why is this a different patch though?

No special reason, I will merge this into #4 "efi_loader: support boot
from URI device path" patch.

> My concern is code duplication when we add similar functionality in
> eficonfig.  Isn't there a better place to handle the memory reservation?

I think eficonfig will only provide add/edit/delete URI boot option,
efibootmgr is
responsible for handling the URI device path and reserving the memory.
So there will not be code duplication.

Thanks,
Masahisa Kojima

>
> [...]
>
> Thanks
> /Ilias
>

Re: [PATCH v4 7/8] doc: uefi: add HTTP Boot support

[Masahisa Kojima]

On Mon, 25 Sept 2023 at 21:12, Ilias Apalodimas
 wrote:
>
> On Fri, Sep 22, 2023 at 04:11:18PM +0900, Masahisa Kojima wrote:
> > This adds the description about HTTP Boot.
> >
> > Signed-off-by: Masahisa Kojima 
> > ---
> >  doc/develop/uefi/uefi.rst | 30 ++
> >  1 file changed, 30 insertions(+)
> >
> > diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
> > index a7a41f2fac..65eea89265 100644
> > --- a/doc/develop/uefi/uefi.rst
> > +++ b/doc/develop/uefi/uefi.rst
> > @@ -594,6 +594,36 @@ UEFI variables. Booting according to these variables 
> > is possible via::
> >  As of U-Boot v2020.10 UEFI variables cannot be set at runtime. The U-Boot
> >  command 'efidebug' can be used to set the variables.
> >
> > +UEFI HTTP Boot
> > +~~
> > +
> > +HTTP Boot provides the capability for system deployment and configuration
> > +over the network. HTTP Boot can be activated by specifying::
> > +
> > +CONFIG_CMD_DNS
> > +CONFIG_CMD_WGET
> > +CONFIG_BLKMAP
> > +
> > +Set up the load option specifying the target URI::
> > +
> > +efidebug boot add -u 1 netinst http://foo/bar
> > +
> > +When this load option is selected as boot selection, resolve the
> > +host ip address by dns, then download the file with wget.
> > +If the downloaded file extension is .iso or .img file, efibootmgr tries to
> > +mount the image and boot with the default file(e.g. EFI/BOOT/BOOTAA64.EFI).
> > +If the downloaded file is PE-COFF image, load the downloaded file and
> > +start it.
> > +
> > +There is a limitation that current implementation tries to resolve
>
> Remove the 'There is a limitation that', use The current implementation ...

OK.

>
> > +the IP address as a host name. If the uri is like 
> > "http://192.168.1.1/foobar;,
> > +the dns process tries to resolve the host "192.168.1.1" and it will
> > +end up with "host not found".
> > +
> > +We need to preset the "httpserverip" environment variable to proceed the 
> > wget::
> > +
> > +setenv httpserverip 192.168.1.1
> > +
> >  Executing the built in hello world application
> >  ~~
> >
> > --
> > 2.34.1
> >
>
> Other than the nit above
> Reviewed-by: Ilias Apalodimas 

Thanks,
Masahisa Kojima

>
>

Re: [PATCH v4 4/8] efi_loader: support boot from URI device path

[Masahisa Kojima]

Hi Ilias,

On Mon, 25 Sept 2023 at 21:37, Ilias Apalodimas
 wrote:
>
> On Fri, Sep 22, 2023 at 04:11:15PM +0900, Masahisa Kojima wrote:
> > This supports to boot from the URI device path.
> > When user selects the URI device path, bootmgr downloads
> > the file using wget into the address specified by loadaddr
> > env variable.
> > If the file is .iso or .img file, mount the image with blkmap
> > then try to boot with the default file(e.g. EFI/BOOT/BOOTAA64.EFI).
> > If the file is .efi file, load and start the downloaded file.
> >
> > Signed-off-by: Masahisa Kojima 
> > ---
> >  lib/efi_loader/efi_bootmgr.c | 189 +++
> >  1 file changed, 189 insertions(+)
> >
> > diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> > index a40762c74c..605be5041e 100644
> > --- a/lib/efi_loader/efi_bootmgr.c
> > +++ b/lib/efi_loader/efi_bootmgr.c
> > @@ -7,10 +7,14 @@
> >
> >  #define LOG_CATEGORY LOGC_EFI
> >
> > +#include 
> > +#include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  #include 
> > @@ -168,6 +172,182 @@ out:
> >   return ret;
> >  }
> >
> > +/**
> > + * mount_image() - mount the image with blkmap
> > + *
> > + * @lo_label u16 label string of load option
> > + * @image_addr:  image address
> > + * @image_size   image size
> > + * Return:   pointer to the UCLASS_BLK udevice, NULL if failed
> > + */
> > +static struct udevice *mount_image(u16 *lo_label, ulong image_addr, int 
> > image_size)
> > +{
> > + int err;
> > + struct blkmap *bm;
> > + struct udevice *bm_dev;
> > + char *label = NULL, *p;
> > +
> > + label = efi_alloc(utf16_utf8_strlen(lo_label) + 1);
> > + if (!label)
> > + return NULL;
> > +
> > + p = label;
> > + utf16_utf8_strcpy(, lo_label);
> > + err = blkmap_create_ramdisk(label, image_addr, image_size, _dev);
> > + if (err) {
> > + efi_free_pool(label);
> > + return NULL;
> > + }
> > + bm = dev_get_plat(bm_dev);
> > +
> > + efi_free_pool(label);
> > +
> > + return bm->blk;
> > +}
> > +
> > +/**
> > + * try_load_default_file() - try to load the default file
> > + *
> > + * Search the device having EFI_SIMPLE_FILE_SYSTEM_PROTOCOL,
> > + * then try to load with the default boot file(e.g. EFI/BOOT/BOOTAA64.EFI).
> > + *
> > + * @dev  pointer to the UCLASS_BLK or UCLASS_PARTITION 
> > udevice
> > + * @image_handle:pointer to handle for newly installed image
> > + * Return:   status code
> > + */
> > +static efi_status_t try_load_default_file(struct udevice *dev,
> > +   efi_handle_t *image_handle)
> > +{
>
> Maybe I misunderstood you on the previous series.  Wasn't the plan to merge
> the patch that adds boot options automatically when a disk is scanned? This

Adding the boot option automatically when a disk is scanned was sent
separately[1] from this series
since this feature is the matter of the timing of automatic boot
option creation and not directly related
to EFI HTTP Boot.
I also included the fixes of the efi_secboot python test failure.
Sorry for confusing you.

[1] 
https://patchwork.ozlabs.org/project/uboot/patch/20230920084121.1248590-1-masahisa.koj...@linaro.org/

> code could only look for a boot option with '1234567' in its load options
> instead of rescanning all devices with the EFI_SIMPLE_FILE_SYSTEM_PROTOCO
> installed

Now we can scan and load the default file(EFI/BOOT/BOOTAA64.EFI) on the fly with
load_default_file_from_blk_dev() function.
The patch #8 of this series "efi_loader: create BlockIo device boot
option" create the boot option
for the block device excluding the logical partition,

>
> > + efi_status_t ret;
> > + efi_handle_t handle;
> > + struct efi_handler *handler;
> > + struct efi_device_path *file_path;
> > + struct efi_device_path *device_path;
> > +
> > + if (dev_tag_get_ptr(dev, DM_TAG_EFI, (void **))) {
> > + log_warning("DM_TAG_EFI not found\n");
> > + return EFI_INVALID_PARAMETER;
> > + }
> > +
> > + ret = efi_search_protocol(handle,
> > +   _simple_file_system_protocol_guid, 
> > );
> > + if (ret != EFI_SUCCESS)
> > + return ret;
> > +
> > + ret = EFI_CALL(bs->open_protocol(handle, _guid_device_path,
> > +  (void **)_path, efi_root, 
> > NULL,
> > +  EFI_OPEN_PROTOCOL_GET_PROTOCOL));
> > + if (ret != EFI_SUCCESS)
> > + return ret;
> > +
> > + file_path = expand_media_path(device_path);
> > + ret = EFI_CALL(efi_load_image(true, efi_root, file_path, NULL, 0,
> > +   image_handle));
> > +
> > + efi_free_pool(file_path);
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * load_default_file_from_blk_dev() - load the default 

Re: Trying to boot custom kernel on Wink Hub (i.MX28)

[Fabio Estevam]

Hi Rogan,

On Mon, Sep 25, 2023 at 4:27 PM Rogan Dawes  wrote:
>
> That outputs LLC about a second after the mxsldr terminates (kinda slowly, 
> the individual characters can be seen being emitted one at a time), and then 
> about 5 seconds later, "Pref". But nothing after that point.
>
> At least it looks like there is something happening, which is excellent!

I pushed a change in my branch. Please give it a try.

Re: [PATCH] dt-bindings: mtd: Add a schema for binman

[Simon Glass]

Hi Rob,

On Mon, 25 Sept 2023 at 12:49, Rob Herring  wrote:
>
> On Mon, Sep 25, 2023 at 11:25 AM Simon Glass  wrote:
> >
> > Hi Miquel,
> >
> > On Mon, 25 Sept 2023 at 09:24, Miquel Raynal  
> > wrote:
> > >
> > > Hi Simon,
> > >
> > > > > > > > > > > > I was assuming that I should create a top-level 
> > > > > > > > > > > > compatible = "binman"
> > > > > > > > > > > > node, with subnodes like compatible = 
> > > > > > > > > > > > "binman,bl31-atf", for example.
> > > > > > > > > > > > I should use the compatible string to indicate the 
> > > > > > > > > > > > contents, right?
> > > > > > > > > > >
> > > > > > > > > > > Yes for subnodes, and we already have some somewhat 
> > > > > > > > > > > standard ones for
> > > > > > > > > > > "u-boot" and "u-boot-env". Though historically, "label" 
> > > > > > > > > > > was used.
> > > > > > > > > >
> > > > > > > > > > Binman has common properties for all entries, including 
> > > > > > > > > > "compress"
> > > > > > > > > > which sets the compression algorithm.
> > > > > > > > >
> > > > > > > > > I see no issue with adding that. It seems useful and 
> > > > > > > > > something missing
> > > > > > > > > in the existing partition schemas.
> > > > > > > >
> > > > > > > > OK I sent a patch with that.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > > So perhaps I should start by defining a new binman,bl31-atf 
> > > > > > > > > > which has
> > > > > > > > > > common properties from an "binman,entry" definition?
> > > > > > > > >
> > > > > > > > > I don't understand the binman prefix. The contents are ATF 
> > > > > > > > > (or TF-A
> > > > > > > > > now). Who wrote it to the flash image is not relevant.
> > > > > > > >
> > > > > > > > Are you suggesting just "atf-bl31", or "arm,atf-bl31" ? Or 
> > > > > > > > should we
> > > > > > > > change it to "tfa-bl31"?
> > > > > > >
> > > > > > > I don't really understand the relationship with TF-A here. Can't 
> > > > > > > we
> > > > > > > just have a kind of fixed-partitions with additional properties 
> > > > > > > like
> > > > > > > the compression?
> > > > > >
> > > > > > Binman needs to know what to put in there, which is the purpose of 
> > > > > > the
> > > > > > compatible string.
> > > > >
> > > > > But "what" should be put inside the partition is part of the input
> > > > > argument, not the output. You said (maybe I got this wrong) that the
> > > > > schema would apply to the output of binman. If you want to let user
> > > > > know what's inside, maybe it is worth adding a label, but otherwise I
> > > > > don't like the idea of a compatible for that, which for me would mean:
> > > > > "here is how to handle that specific portion of the flash/here is how
> > > > > the flash is organized".
> > > >
> > > > But I thought that the compatible string was for that purpose? See for
> > > > example "brcm,bcm4908-firmware" and "brcm,bcm963xx-imagetag" and
> > > > "linksys,ns-firmware".
> > >
> > > These three examples apparently need specific handling, the partitions
> > > contain meta-data that a parser needs to check or something like that.
> > > And finally it looks like partition names are set depending on the
> > > content that was discovered, so yes, the partition name is likely the
> > > good location to tell users/OSes what's inside.
> > >
> > > > > > > Also, I still don't understand the purpose of this schema. So 
> > > > > > > binman
> > > > > > > generates an image, you want to flash this image and you would 
> > > > > > > like the
> > > > > > > tool to generate the corresponding (partition) DT snippet 
> > > > > > > automatically.
> > > > > > > Do I get this right? I don't get why you would need new 
> > > > > > > compatibles for
> > > > > > > that.
> > > > > >
> > > > > > It is actually the other way around. The schema tells Binman how to
> > > > > > build the image (what goes in there and where). Then outputs an
> > > > > > updated DT which describes where everything ended up, for use by 
> > > > > > other
> > > > > > tools, e.g. firmware update. It is a closed loop in that sense. See
> > > > > > the references for more information.
> > > > >
> > > > > Maybe I fail to see why you would want these description to be
> > > > > introduced here, if they are not useful to the OS.
> > > >
> > > > Well I was asked to send them to Linux since they apparently don't
> > > > belong in dt-schema.
>
> That is not what I said. I said fixed-partitions should be extended. I
> prefer they are extended in-place before moving them rather than the
> other way around.

OK.

>
> > > > These are firmware bindings, as indicated, but I
> > > > took them out of the /firmware node since that is for a different
> > > > purpose. Rob suggested that partitions was a good place. We have fwupd
> > > > using DT to hold the firmware-update information, so I expect it will
> > > > move to use these bindings too.
> > >
> > > I would definitely use fixed partitions as that's what you need then:
> > > registering where everything starts and ends. 

[PATCH] mmc: Add SPL_MMC_PWRSEQ to fix link issue when building SPL

[Jonas Karlman]

With MMC_PWRSEQ enabled the following link issue may happen when
building SPL and SPL_PWRSEQ is not enabled.

  aarch64-linux-gnu-ld.bfd: drivers/mmc/meson_gx_mmc.o: in function 
`meson_mmc_probe':
  drivers/mmc/meson_gx_mmc.c:295: undefined reference to `pwrseq_set_power'

Fix this by adding a SPL_MMC_PWRSEQ Kconfig option used to enable mmc
pwrseq support in SPL.

Also add depends on DM_GPIO to fix following link issue:

  aarch64-linux-gnu-ld.bfd: drivers/mmc/mmc-pwrseq.o: in function 
`mmc_pwrseq_set_power':
  drivers/mmc/mmc-pwrseq.c:26: undefined reference to `gpio_request_by_name'
  aarch64-linux-gnu-ld.bfd: drivers/mmc/mmc-pwrseq.c:29: undefined reference to 
`dm_gpio_set_value'
  aarch64-linux-gnu-ld.bfd: drivers/mmc/mmc-pwrseq.c:31: undefined reference to 
`dm_gpio_set_value'

Signed-off-by: Jonas Karlman 
---
 drivers/mmc/Kconfig   | 10 +-
 drivers/mmc/Makefile  |  2 +-
 drivers/mmc/meson_gx_mmc.c|  2 +-
 drivers/mmc/rockchip_dw_mmc.c |  2 +-
 include/mmc.h |  4 ++--
 5 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/Kconfig b/drivers/mmc/Kconfig
index de01b9687bad..a9931d39412d 100644
--- a/drivers/mmc/Kconfig
+++ b/drivers/mmc/Kconfig
@@ -20,11 +20,19 @@ config MMC_WRITE
 
 config MMC_PWRSEQ
bool "HW reset support for eMMC"
-   depends on PWRSEQ
+   depends on PWRSEQ && DM_GPIO
help
  Ths select Hardware reset support aka pwrseq-emmc for eMMC
  devices.
 
+config SPL_MMC_PWRSEQ
+   bool "HW reset support for eMMC in SPL"
+   depends on SPL_PWRSEQ && SPL_DM_GPIO
+   default y if MMC_PWRSEQ
+   help
+ Ths select Hardware reset support aka pwrseq-emmc for eMMC
+ devices in SPL.
+
 config MMC_BROKEN_CD
bool "Poll for broken card detection case"
help
diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
index 2c65c4765ab2..0a79dd058bef 100644
--- a/drivers/mmc/Makefile
+++ b/drivers/mmc/Makefile
@@ -11,7 +11,7 @@ obj-$(CONFIG_$(SPL_TPL_)BOOTSTD) += mmc_bootdev.o
 endif
 
 obj-$(CONFIG_$(SPL_TPL_)MMC_WRITE) += mmc_write.o
-obj-$(CONFIG_MMC_PWRSEQ) += mmc-pwrseq.o
+obj-$(CONFIG_$(SPL_)MMC_PWRSEQ) += mmc-pwrseq.o
 obj-$(CONFIG_MMC_SDHCI_ADMA_HELPERS) += sdhci-adma.o
 
 ifndef CONFIG_$(SPL_)BLK
diff --git a/drivers/mmc/meson_gx_mmc.c b/drivers/mmc/meson_gx_mmc.c
index fcf4f03d1e24..0825c0a2a838 100644
--- a/drivers/mmc/meson_gx_mmc.c
+++ b/drivers/mmc/meson_gx_mmc.c
@@ -288,7 +288,7 @@ static int meson_mmc_probe(struct udevice *dev)
 
mmc_set_clock(mmc, cfg->f_min, MMC_CLK_ENABLE);
 
-#ifdef CONFIG_MMC_PWRSEQ
+#if CONFIG_IS_ENABLED(MMC_PWRSEQ)
/* Enable power if needed */
ret = mmc_pwrseq_get_power(dev, cfg);
if (!ret) {
diff --git a/drivers/mmc/rockchip_dw_mmc.c b/drivers/mmc/rockchip_dw_mmc.c
index 72c820ee6330..ad4529d6afa8 100644
--- a/drivers/mmc/rockchip_dw_mmc.c
+++ b/drivers/mmc/rockchip_dw_mmc.c
@@ -145,7 +145,7 @@ static int rockchip_dwmmc_probe(struct udevice *dev)
 
host->fifo_mode = priv->fifo_mode;
 
-#ifdef CONFIG_MMC_PWRSEQ
+#if CONFIG_IS_ENABLED(MMC_PWRSEQ)
/* Enable power if needed */
ret = mmc_pwrseq_get_power(dev, >cfg);
if (!ret) {
diff --git a/include/mmc.h b/include/mmc.h
index 1022db3ffa7c..9aef31ea5deb 100644
--- a/include/mmc.h
+++ b/include/mmc.h
@@ -590,7 +590,7 @@ struct mmc_config {
uint f_max;
uint b_max;
unsigned char part_type;
-#ifdef CONFIG_MMC_PWRSEQ
+#if CONFIG_IS_ENABLED(MMC_PWRSEQ)
struct udevice *pwr_dev;
 #endif
 };
@@ -808,7 +808,7 @@ int mmc_deinit(struct mmc *mmc);
  */
 int mmc_of_parse(struct udevice *dev, struct mmc_config *cfg);
 
-#ifdef CONFIG_MMC_PWRSEQ
+#if CONFIG_IS_ENABLED(MMC_PWRSEQ)
 /**
  * mmc_pwrseq_get_power() - get a power device from device tree
  *
-- 
2.42.0

Re: [PATCH v2 0/7] rockchip: rk3568: Fix use of PCIe bifurcation

[Jonas Karlman]

Hi Kever,

It would be nice to get some feedback and plans for this and the
following series :-)

rockchip: rk3568: Fix use of PCIe bifurcation (this series)
https://patchwork.ozlabs.org/cover/1816140/

rockchip: rk3568-nanopi-r5: Add missing PCIe options
https://patchwork.ozlabs.org/cover/1816147/

rockchip: Port IO-domain driver for RK3568 from linux
https://patchwork.ozlabs.org/cover/1823769/

I also plan to send a v2 with small update based on the little feedback
I got on the following:

rockchip: Add GMAC support for RK3568 and RK3588
https://patchwork.ozlabs.org/cover/1817469/

Regards,
Jonas

On 2023-08-02 21:04, Jonas Karlman wrote:
> This series add support for use of PCIe bifurcation on RK3568, and as a
> bonus support for the RK3588 PHY is also included. With PCIe bifurcation
> supported it is possible to enable PCIe on more RK3568 boards, e.g. on
> NanoPi R5C and NanoPi R5S. This series only include fixing the mini PCIe
> slot on Radxa E25.
> 
> Most parts of this series was imported almost 1:1 from mainline linux.
> 
> Patch 1 fixes configuration of number of lanes in pcie_dw_rockchip.
> Patch 2-3 refactor the snps-pcie3 phy driver.
> Patch 4 add bifurcation support for RK3568.
> Patch 5 add support for RK3588 to snps-pcie3 driver.
> Patch 6 fixes use of pcie2x1l0 on ROCK 5B.
> Patch 7 enables the mini PCIe slot on Radxa E25.
> 
> Changes in v2:
> - Fix use of signal from comb PHY on RK3588
> - Add fixes tag
> 
> The RK3588 PHY part was tested on a ROCK 5B together with device tree
> files picked from Sebastian Reichel's rk3588 branch at [1].
> 
> Patches in this series is also aviliable at [2].
> 
> [1] 
> https://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-misc.git/tree/?h=rk3588
> [2] 
> https://github.com/Kwiboo/u-boot-rockchip/commits/rk35xx-pcie-bifurcation-v2
> 
> Jonas Karlman (7):
>   pci: pcie_dw_rockchip: Configure number of lanes and link width speed
>   phy: rockchip: snps-pcie3: Refactor to use clk_bulk API
>   phy: rockchip: snps-pcie3: Refactor to use a phy_init ops
>   phy: rockchip: snps-pcie3: Add bifurcation support for RK3568
>   phy: rockchip: snps-pcie3: Add support for RK3588
>   phy: rockchip: naneng-combphy: Use signal from comb PHY on RK3588
>   rockchip: rk3568-radxa-e25: Enable pcie3x1 node
> 
>  arch/arm/dts/rk3568-radxa-e25-u-boot.dtsi |  11 +-
>  configs/radxa-e25-rk3568_defconfig|   1 -
>  drivers/pci/pcie_dw_rockchip.c|  58 -
>  .../rockchip/phy-rockchip-naneng-combphy.c|   6 +
>  .../phy/rockchip/phy-rockchip-snps-pcie3.c| 230 ++
>  5 files changed, 241 insertions(+), 65 deletions(-)
> 

Re: [PATCH v3 00/38] spl: Preparation for Universal Payload

[Tom Rini]

On Sun, Sep 24, 2023 at 01:24:45PM -0600, Simon Glass wrote:

> This series tidies up SPL a little and adds some core ofnode functions
> needed to support Universal Payload. It also includes a few minor fix-ups
> for sandbox.
> 
> For SPL the changes include CONFIG naming, removing various #ifdefs and
> tidying up the FIT code.
> 
> One notable piece of the ofnode improvements is support for flattening
> a livetree. This should be useful in future as we move FDT fixups to use
> the ofnode API.
> 
> Changes in v3:
> - Mention testing on qemu-ppce500
> - Add new patch to create a proper symbol for enabling the malloc() pool
> - Add new patch to enable CONFIG_SPL_SYS_MALLOC_F where needed
> - Add new patch to nable CONFIG_TPL_SYS_MALLOC_F where needed
> - Add new patch to use SYS_MALLOC_F instead of SYS_MALLOC_F_LEN
> - Add new patch to tidy up uses of CONFIG_SYS_MALLOC_F_LEN
> - Add new patch to clean up SYS_MALLOC_SIMPLE documentation
> - Add new patch to correct help in TPL_DM and VPL_DM
> - Rebase on the new patch
> 
> Changes in v2:
> - Rename based on Tom's feedback
> - Improve readability by moving the size part to the header file
> - Explicitly copy two maintainers as it seems only Mario was auto-cc'd
> - Change the approach to use the header file
> - Use the same condition for both pieces to avoid possible problems
> - No changes as it still seems unclear what should be done
> 
> Simon Glass (38):
>   spl: Use CONFIG_SPL... instead of CONFIG_..._SPL_...
>   spl: Rename SYS_SPL_ARGS_ADDR to SPL_PAYLOAD_ARGS_ADDR
>   spl: Avoid #ifdef with CONFIG_SPL_SYS_MALLOC
>   spl: mx6: powerpc: Drop the condition on timer_init()
>   spl: Drop #ifdefs for BOARD_INIT and watchdog
>   spl: Avoid #ifdef with CONFIG_SPL_PAYLOAD_ARGS_ADDR
>   spl: Drop the switch() statement for OS selection
>   spl: Create proper symbols for enabling the malloc() pool
>   spl: Enable CONFIG_SPL_SYS_MALLOC_F where needed
>   tpl: Enable CONFIG_TPL_SYS_MALLOC_F where needed
>   spl: Use SYS_MALLOC_F instead of SYS_MALLOC_F_LEN
>   Tidy up uses of CONFIG_SYS_MALLOC_F_LEN
>   doc: Clean up SYS_MALLOC_SIMPLE
>   dm: core: Correct help in TPL_DM and VPL_DM
>   spl: Avoid an #ifdef when printing gd->malloc_ptr
>   spl: Remove #ifdefs with BOOTSTAGE
>   spl: Rename spl_load_fit_image() to load_simple_fit()
>   spl: Move the full FIT code to spl_fit.c
>   spl: Use the correct FIT_..._PROP constants
>   spl: Move bloblist writing until the image is known
>   dm: core: Reverse the argument order in ofnode_copy_props()
>   dm: core: Ensure we run flattree tests on ofnode
>   dm: core: Tidy up comments in the ofnode tests
>   dm: core: Add a function to create an empty tree
>   dm: core: Add a way to copy a node
>   dm: core: Add a way to delete a node
>   dm: core: Add a way to convert a devicetree to a dtb
>   dm: core: Support writing a boolean
>   dm: core: Support writing a 64-bit value
>   dm: core: Add tests for oftree_path()
>   sandbox: Move reading the RAM buffer into a better place
>   sandbox: Init the EC properly even if no state file is available
>   sandbox: Only read the state if we have a state file
>   sandbox: Move the bloblist down a little in memory
>   bloblist: Support initing from multiple places
>   fdt: Allow the devicetree to come from a bloblist
>   command: Include a required header in command.h
>   pci: serial: Support reading PCI-register size with base

Specific feedback aside, generally this seems fine.

-- 
Tom


signature.asc
Description: PGP signature

[PATCH 1/3] net: Get pxe config file from dhcp option 209

[seanedmond]

From: Sean Edmond 

Allow dhcp server pass pxe config file full path by using option 209

Signed-off-by: Sean Edmond 
---
 cmd/Kconfig |  4 
 cmd/pxe.c   | 10 ++
 net/bootp.c | 21 +
 3 files changed, 35 insertions(+)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 43ca10f69c..25c1efc41e 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1817,6 +1817,10 @@ config BOOTP_PXE_CLIENTARCH
default 0x15 if ARM
default 0 if X86
 
+config BOOTP_PXE_DHCP_OPTION
+   bool "Request & store 'pxe_configfile' from BOOTP/DHCP server"
+   depends on BOOTP_PXE
+
 config BOOTP_VCI_STRING
string
depends on CMD_BOOTP
diff --git a/cmd/pxe.c b/cmd/pxe.c
index 677142520b..83c4ed5411 100644
--- a/cmd/pxe.c
+++ b/cmd/pxe.c
@@ -65,6 +65,8 @@ static int pxe_dhcp_option_path(struct pxe_context *ctx, 
unsigned long pxefile_a
int ret = get_pxe_file(ctx, pxelinux_configfile, pxefile_addr_r);
 
free(pxelinux_configfile);
+   /* set to NULL to avoid double-free if DHCP is tried again */
+   pxelinux_configfile = NULL;
 
return ret;
 }
@@ -141,6 +143,14 @@ int pxe_get(ulong pxefile_addr_r, char **bootdirp, ulong 
*sizep, bool use_ipv6)
  env_get("bootfile"), use_ipv6))
return -ENOMEM;
 
+   if (IS_ENABLED(CONFIG_BOOTP_PXE_DHCP_OPTION) &&
+   pxelinux_configfile && !use_ipv6) {
+   if (pxe_dhcp_option_path(, pxefile_addr_r) > 0)
+   goto done;
+
+   goto error_exit;
+   }
+
if (IS_ENABLED(CONFIG_DHCP6_PXE_DHCP_OPTION) &&
pxelinux_configfile && use_ipv6) {
if (pxe_dhcp_option_path(, pxefile_addr_r) > 0)
diff --git a/net/bootp.c b/net/bootp.c
index 8b1a4ae2ef..013d54c7ed 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -26,6 +26,7 @@
 #ifdef CONFIG_BOOTP_RANDOM_DELAY
 #include "net_rand.h"
 #endif
+#include 
 
 #define BOOTP_VENDOR_MAGIC 0x63825363  /* RFC1048 Magic Cookie */
 
@@ -604,6 +605,10 @@ static int dhcp_extended(u8 *e, int message_type, struct 
in_addr server_ip,
*e++  = 42;
*cnt += 1;
 #endif
+   if (IS_ENABLED(CONFIG_BOOTP_PXE_DHCP_OPTION)) {
+   *e++ = 209; /* PXELINUX Config File */
+   *cnt += 1;
+   }
/* no options, so back up to avoid sending an empty request list */
if (*cnt == 0)
e -= 2;
@@ -912,6 +917,22 @@ static void dhcp_process_options(uchar *popt, uchar *end)
net_boot_file_name[size] = 0;
}
break;
+   case 209:   /* PXELINUX Config File */
+   if (IS_ENABLED(CONFIG_BOOTP_PXE_DHCP_OPTION)) {
+   /* In case it has already been allocated when 
get DHCP Offer packet,
+* free first to avoid memory leak.
+*/
+   if (pxelinux_configfile)
+   free(pxelinux_configfile);
+
+   pxelinux_configfile = (char *)malloc((oplen + 
1) * sizeof(char));
+
+   if (pxelinux_configfile)
+   strlcpy(pxelinux_configfile, popt + 2, 
oplen + 1);
+   else
+   printf("Error: Failed to allocate 
pxelinux_configfile\n");
+   }
+   break;
default:
 #if defined(CONFIG_BOOTP_VENDOREX)
if (dhcp_vendorex_proc(popt))
-- 
2.40.0

[PATCH 3/3] net: bootp: add config option BOOTP_RANDOM_XID

[seanedmond]

From: Sean Edmond 

The new config option BOOTP_RANDOM_XID will randomize the transaction ID
for each new BOOT/DHCPv4 exchange.

Signed-off-by: Sean Edmond 
---
 cmd/Kconfig |  7 +++
 net/bootp.c | 31 +--
 2 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 25c1efc41e..4be5be8724 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1829,6 +1829,13 @@ config BOOTP_VCI_STRING
default "U-Boot.arm" if ARM
default "U-Boot"
 
+config BOOTP_RANDOM_XID
+   bool "Send random transaction ID to BOOTP/DHCP server"
+   depends on CMD_BOOTP
+   help
+ Selecting this will allow for a random transaction ID to get
+ selected for each BOOTP/DHCPv4 exchange.
+
 if CMD_DHCP6
 
 config DHCP6_PXE_CLIENTARCH
diff --git a/net/bootp.c b/net/bootp.c
index 7248536cc4..5053a1b870 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -832,22 +832,25 @@ void bootp_request(void)
 
/* Only generate a new transaction ID for each new BOOTP request */
if (bootp_try == 1) {
-   /*
-*  Bootp ID is the lower 4 bytes of our ethernet address
-*  plus the current time in ms.
-*/
-   bootp_id = ((u32)net_ethaddr[2] << 24)
-   | ((u32)net_ethaddr[3] << 16)
-   | ((u32)net_ethaddr[4] << 8)
-   | (u32)net_ethaddr[5];
-   bootp_id += get_timer(0);
-   bootp_id = htonl(bootp_id);
-   bootp_add_id(bootp_id);
-   net_copy_u32(>bp_id, _id);
-   } else {
-   net_copy_u32(>bp_id, _id);
+   if (IS_ENABLED(CONFIG_BOOTP_RANDOM_XID)) {
+   srand(get_ticks() + rand());
+   bootp_id = rand();
+   } else {
+   /*
+*  Bootp ID is the lower 4 bytes of our ethernet 
address
+*  plus the current time in ms.
+*/
+   bootp_id = ((u32)net_ethaddr[2] << 24)
+   | ((u32)net_ethaddr[3] << 16)
+   | ((u32)net_ethaddr[4] << 8)
+   | (u32)net_ethaddr[5];
+   bootp_id += get_timer(0);
+   bootp_id = htonl(bootp_id);
+   }
}
 
+   bootp_add_id(bootp_id);
+   net_copy_u32(>bp_id, _id);
/*
 * Calculate proper packet lengths taking into account the
 * variable size of the options field
-- 
2.40.0

[PATCH 2/3] net: bootp: BOOTP/DHCPv4 retransmission improvements

[seanedmond]

From: Sean Edmond 

This patch introduces 3 improvements to align with RFC 951:
- retransmission backoff interval maximum is configurable
- initial retranmission backoff interval is configurable
- transaction ID is kept the same for each BOOTP/DHCPv4 request

In applications where thousands of nodes are serviced by a single DHCP
server, maximizing the retransmission backoff interval at 2 seconds (the
current u-boot default) exerts high pressure on the DHCP server and
network layer.

RFC 951 “7.2. Client Retransmission Strategy” states that the
retransmission backoff interval should maximize at 60 seconds.  This
patch allows the interval to be configurable using the environment
variable "bootpretransmitperiodmax"

The initial retranmission backoff period defaults to 250ms, which is
also too small for these scenarios with many clients.  This patch makes
the initial retransmission interval to be configurable using the
environment variable "bootpretransmitperiodinit".

Also, on a retransmission it is not expected for the transaction ID to
change (only the 'secs' field should be updated). Let's save the
transaction ID and use the same transaction ID for each BOOTP/DHCPv4
exchange.

Signed-off-by: Sean Edmond 
---
 net/bootp.c | 63 +++--
 1 file changed, 47 insertions(+), 16 deletions(-)

diff --git a/net/bootp.c b/net/bootp.c
index 013d54c7ed..7248536cc4 100644
--- a/net/bootp.c
+++ b/net/bootp.c
@@ -42,6 +42,17 @@
  */
 #define TIMEOUT_MS ((3 + (CONFIG_NET_RETRY_COUNT * 5)) * 1000)
 
+/*
+ * According to rfc951 : 7.2. Client Retransmission Strategy
+ * "After the 'average' backoff reaches about 60 seconds, it should be
+ * increased no further, but still randomized."
+ *
+ * U-Boot has saturated this backoff at 2 seconds for a long time.
+ * To modify, set the environment variable "bootpretransmitperiodmax"
+ */
+#define RETRANSMIT_PERIOD_MAX_MS   2000
+#define RETRANSMIT_PERIOD_INIT_MS  250
+
 #define PORT_BOOTPS67  /* BOOTP server UDP port */
 #define PORT_BOOTPC68  /* BOOTP client UDP port */
 
@@ -56,6 +67,7 @@
 u32bootp_ids[CFG_BOOTP_ID_CACHE_SIZE];
 unsigned int   bootp_num_ids;
 intbootp_try;
+u32bootp_id;
 ulong  bootp_start;
 ulong  bootp_timeout;
 char net_nis_domain[32] = {0,}; /* Our NIS domain */
@@ -63,6 +75,7 @@ char net_hostname[32] = {0,}; /* Our hostname */
 char net_root_path[CONFIG_BOOTP_MAX_ROOT_PATH_LEN] = {0,}; /* Our bootpath */
 
 static ulong time_taken_max;
+static u32   retransmit_period_max_ms;
 
 #if defined(CONFIG_CMD_DHCP)
 static dhcp_state_t dhcp_state = INIT;
@@ -417,8 +430,8 @@ static void bootp_timeout_handler(void)
}
} else {
bootp_timeout *= 2;
-   if (bootp_timeout > 2000)
-   bootp_timeout = 2000;
+   if (bootp_timeout > retransmit_period_max_ms)
+   bootp_timeout = retransmit_period_max_ms;
net_set_timeout_handler(bootp_timeout, bootp_timeout_handler);
bootp_request();
}
@@ -714,10 +727,18 @@ static int bootp_extended(u8 *e)
 
 void bootp_reset(void)
 {
+   char *ep;  /* Environment pointer */
+
bootp_num_ids = 0;
bootp_try = 0;
bootp_start = get_timer(0);
-   bootp_timeout = 250;
+
+   ep = env_get("bootpretransmitperiodinit");
+   if (ep)
+   bootp_timeout = dectoul(ep, NULL);
+   else
+   bootp_timeout = RETRANSMIT_PERIOD_INIT_MS;
+
 }
 
 void bootp_request(void)
@@ -729,7 +750,6 @@ void bootp_request(void)
 #ifdef CONFIG_BOOTP_RANDOM_DELAY
ulong rand_ms;
 #endif
-   u32 bootp_id;
struct in_addr zero_ip;
struct in_addr bcast_ip;
char *ep;  /* Environment pointer */
@@ -745,6 +765,12 @@ void bootp_request(void)
else
time_taken_max = TIMEOUT_MS;
 
+   ep = env_get("bootpretransmitperiodmax");
+   if (ep)
+   retransmit_period_max_ms = dectoul(ep, NULL);
+   else
+   retransmit_period_max_ms = RETRANSMIT_PERIOD_MAX_MS;
+
 #ifdef CONFIG_BOOTP_RANDOM_DELAY   /* Random BOOTP delay */
if (bootp_try == 0)
srand_mac();
@@ -804,18 +830,23 @@ void bootp_request(void)
extlen = bootp_extended((u8 *)bp->bp_vend);
 #endif
 
-   /*
-*  Bootp ID is the lower 4 bytes of our ethernet address
-*  plus the current time in ms.
-*/
-   bootp_id = ((u32)net_ethaddr[2] << 24)
-   | ((u32)net_ethaddr[3] << 16)
-   | ((u32)net_ethaddr[4] << 8)
-   | (u32)net_ethaddr[5];
-   bootp_id += get_timer(0);
-   bootp_id = htonl(bootp_id);
-   bootp_add_id(bootp_id);
-   net_copy_u32(>bp_id, _id);
+   /* Only generate a new transaction ID for each new BOOTP request */
+   if (bootp_try == 1) {
+   /*
+   

[PATCH 0/3] BOOTP/DHCPv4 enhancements

[seanedmond]

From: Sean Edmond 

In our datacenter application, a single DHCP server is servicing 36000+ clients.
Improvements are required to the DHCPv4 retransmission behavior to align with
RFC and ensure less pressure is exerted on the server:
- retransmission backoff interval maximum is configurable 
  (environment variable bootpretransmitperiodmax)
- initial retransmission backoff interval is configurable 
  (environment variable bootpretransmitperiodinit)
- transaction ID is kept the same for each BOOTP/DHCPv4 request 
  (not recreated on each retry)

For our application we'll use:
- bootpretransmitperiodmax=16000
- bootpretransmitperiodinit=2000

A new configuration BOOTP_RANDOM_XID has been added to enable a randomized
BOOTP/DHCPv4 transaction ID.

Add functionality for DHCPv4 sending/parsing option 209 (PXE config file).
Enabled with Kconfig BOOTP_PXE_DHCP_OPTION.  Note, this patch was
submitted previously but this latest version has been enhanced to
avoid a possible double free().

Sean Edmond (3):
  net: Get pxe config file from dhcp option 209
  net: bootp: BOOTP/DHCPv4 retransmission improvements
  net: bootp: add config option BOOTP_RANDOM_XID

 cmd/Kconfig | 11 +++
 cmd/pxe.c   | 10 +++
 net/bootp.c | 85 +++--
 3 files changed, 91 insertions(+), 15 deletions(-)

-- 
2.40.0

Re: [PATCH v3 38/38] pci: serial: Support reading PCI-register size with base

[Tom Rini]

On Sun, Sep 24, 2023 at 01:25:23PM -0600, Simon Glass wrote:

> The PCI helpers read only the base address for a PCI region. In some cases
> the size is needed as well, e.g. to pass along to a driver which needs to
> know the size of its register area.
> 
> Update the functions to allow the size to be returned. For serial, record
> the information and provided it with the serial_info() call.
> 
> A limitation still exists in that the size is not available when OF_LIVE
> is enabled, so take account of that in the tests.
> 
> Signed-off-by: Simon Glass 

So on platforms that end up here, we see consistently:
eaidk-610-rk3399: all +224 spl/u-boot-spl:all +224 
spl/u-boot-spl:text +224 text +224 tpl/u-boot-tpl:all +224 tpl/u-boot-tpl:text 
+224
   u-boot: add: 2/0, grow: 3/0 bytes: 224/0 (224)
 function   old new   delta
 fdtdec_get_addr_size_auto_noparent   - 124+124
 devfdt_get_addr_size_index   -  72 +72
 ns16550_serial_of_to_plat  316 328 +12
 ns16550_serial_probe   132 140  +8
 ns16550_serial_getinfo  84  92  +8
   tpl-u-boot-tpl: add: 2/0, grow: 3/0 bytes: 224/0 (224)
 function   old new   delta
 fdtdec_get_addr_size_auto_noparent   - 124+124
 devfdt_get_addr_size_index   -  72 +72
 ns16550_serial_of_to_plat  316 328 +12
 ns16550_serial_probe   132 140  +8
 ns16550_serial_getinfo  84  92  +8
   spl-u-boot-spl: add: 2/0, grow: 3/0 bytes: 224/0 (224)
 function   old new   delta
 fdtdec_get_addr_size_auto_noparent   - 124+124
 devfdt_get_addr_size_index   -  72 +72
 ns16550_serial_of_to_plat  316 328 +12
 ns16550_serial_probe   132 140  +8
 ns16550_serial_getinfo  84  92  +8

Can we optimize this in any way? I'm less concerned with full U-Boot
growing by 224 bytes than I am by TPL growing by that much too.

-- 
Tom


signature.asc
Description: PGP signature

bootstd: Scanning for USB bootflow will remove existing SCSI bootflow

[Tony Dinh]

Hi Simon,

Here is an observation during testing the bootflow command.

If there is a SCSI bootflow, scanning for USB bootflow will remove that existing
SCSI bootflow. To bring it back, I scanned for SCSI bootflow again, and it was
back to normal. Perhaps there is some kind of indexing problem?

Please see the log below after the break.

All the best,
Tony

===


U-Boot 2023.10-rc4-tld-1-00044-g9c21b2a350-dirty (Sep 18 2023 - 12:16:59 -0700)
Thecus N2350



N2350 > env def -a
## Resetting to default environment

N2350 > bootflow scan scsi
pcie0.0: Link down
pcie1.0: Link down
scanning bus for devices...
SATA link 0 timeout.
Target spinup took 0 ms.
AHCI 0001. 32 slots 2 ports 6 Gbps 0x3 impl SATA mode
flags: 64bit ncq led only pmp fbss pio slum part sxs
  Device 0: (1:0) Vendor: ATA Prod.: ST750LX003-1AC15 Rev: SM12
Type: Hard Disk
Capacity: 715404.8 MB = 698.6 GB (1465149168 x 512)
** File not found /boot/boot.bmp **

N2350 > bootflow l
Showing all bootflows
Seq  Method   State   UclassPart  Name  Filename
---  ---  --      

  0  script   ready   scsi 1  ahci_scsi.id1lun0.bootdev
/boot/boot.scr
---  ---  --      

(1 bootflow, 1 valid)

N2350 > bootflow scan usb
Bus usb@58000: USB EHCI 1.00
Bus usb3@f: MVEBU XHCI INIT controller @ 0xf10f4000
Register 2000120 NbrPorts 2
Starting the controller
USB XHCI 1.00
Bus usb3@f8000: MVEBU XHCI INIT controller @ 0xf10fc000
Register 2000120 NbrPorts 2
Starting the controller
USB XHCI 1.00
scanning bus usb@58000 for devices... 1 USB Device(s) found
scanning bus usb3@f for devices... 1 USB Device(s) found
scanning bus usb3@f8000 for devices... 2 USB Device(s) found
** File not found /boot/boot.bmp **

N2350 > bootflow l
Showing all bootflows
Seq  Method   State   UclassPart  Name  Filename
---  ---  --      

  0  script   ready   usb_mass_1  usb_mass_storage.lun0.boo
/boot/boot.scr
---  ---  --      

(1 bootflow, 1 valid)

N2350 > bootflow scan scsi
** File not found /boot/boot.bmp **
** File not found /boot/boot.bmp **

N2350 > bootflow l
Showing all bootflows
Seq  Method   State   UclassPart  Name  Filename
---  ---  --      

  0  script   ready   scsi 1  ahci_scsi.id1lun0.bootdev
/boot/boot.scr
  1  script   ready   usb_mass_1  usb_mass_storage.lun0.boo
/boot/boot.scr
---  ---  --      

(2 bootflows, 2 valid)

Re: [PATCH v3 16/38] spl: Remove #ifdefs with BOOTSTAGE

[Tom Rini]

On Sun, Sep 24, 2023 at 01:25:01PM -0600, Simon Glass wrote:

> This feature has some helpers in its header file so that its functions
> resolve to nothing when the feature is disabled. Add a few more and use
> these to simplify the code.
> 
> With this there are no more #ifdefs in board_init_r()
> 
> Signed-off-by: Simon Glass 
> ---
> 
> (no changes since v1)
> 
>  common/spl/spl.c| 15 +++
>  include/bootstage.h | 26 ++
>  2 files changed, 29 insertions(+), 12 deletions(-)

This isn't equivalent:
   aarch64: (for 1/1 boards) spl/u-boot-spl:all +722.0 spl/u-boot-spl:rodata 
+14.0 spl/u-boot-spl:text +708.0
lion-rk3368: spl/u-boot-spl:all +722 spl/u-boot-spl:rodata +14 
spl/u-boot-spl:text +708
   spl-u-boot-spl: add: 3/0, grow: 2/0 bytes: 708/0 (708)
 function   old new   delta
 bootstage_unstash- 308+308
 bootstage_stash  - 284+284
 static.get_record_name   -  92 +92
 spl_common_init124 136 +12
 board_init_r   424 436 +12

Perhaps because this (and another platform) are ones that set:
$ grep BOOTSTAGE .config
CONFIG_BOOTSTAGE_STASH_ADDR=0x0
CONFIG_BOOTSTAGE=y
CONFIG_SPL_BOOTSTAGE=y
# CONFIG_TPL_BOOTSTAGE is not set
CONFIG_BOOTSTAGE_REPORT=y
CONFIG_BOOTSTAGE_RECORD_COUNT=30
CONFIG_SPL_BOOTSTAGE_RECORD_COUNT=5
CONFIG_BOOTSTAGE_FDT=y
# CONFIG_BOOTSTAGE_STASH is not set
CONFIG_BOOTSTAGE_STASH_SIZE=0x1000
CONFIG_CMD_BOOTSTAGE=y

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v2 30/32] fdt: Allow the devicetree to come from a bloblist

[Ilias Apalodimas]

Hi Simon,

I commented on the v1 thread, but let's continue the discussion here

On Thu, 21 Sept 2023 at 04:58, Simon Glass  wrote:
>
> Standard passage provides for a bloblist to be passed from one firmware
> phase to the next. That can be used to pass the devicetree along as well.
> Add an option to support this.
>
> Tests for this will be added as part of the Universal Payload work.
>
> Signed-off-by: Simon Glass 
> ---
>
> Changes in v2:
> - No changes as it still seems unclear what should be done
>

[...]

>
> /* BLOBLISTT_VENDOR_AREA */
> diff --git a/doc/develop/devicetree/control.rst 
> b/doc/develop/devicetree/control.rst
> index cbb65c9b177f..56e00090166f 100644
> --- a/doc/develop/devicetree/control.rst
> +++ b/doc/develop/devicetree/control.rst
> @@ -108,6 +108,9 @@ If CONFIG_OF_BOARD is defined, a board-specific routine 
> will provide the
>  devicetree at runtime, for example if an earlier bootloader stage creates
>  it and passes it to U-Boot.
>
> +If CONFIG_OF_BLOBLIST is defined, the devicetree comes from a bloblist passed
> +from a previous stage.

What I argued before is that we don't need to be this explicit.
The bloblist can carry a bunch of options that might be used by
U-Boot.  It would be better if we had a more generic approach instead
of adding Kconfig options per bloblist entry

[...]

>  #ifndef USE_HOSTCC
> +
> +#define LOG_CATEGORY   LOGC_DT
> +
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -87,6 +91,7 @@ static const char *const fdt_src_name[] = {
> [FDTSRC_BOARD] = "board",
> [FDTSRC_EMBED] = "embed",
> [FDTSRC_ENV] = "env",
> +   [FDTSRC_BLOBLIST] = "bloblist",
>  };
>
>  const char *fdtdec_get_srcname(void)
> @@ -1666,20 +1671,35 @@ int fdtdec_setup(void)
> int ret;
>
> /* The devicetree is typically appended to U-Boot */
> -   if (IS_ENABLED(CONFIG_OF_SEPARATE)) {
> -   gd->fdt_blob = fdt_find_separate();
> -   gd->fdt_src = FDTSRC_SEPARATE;
> -   } else { /* embed dtb in ELF file for testing / development */
> -   gd->fdt_blob = dtb_dt_embedded();
> -   gd->fdt_src = FDTSRC_EMBED;
> -   }
> -
> -   /* Allow the board to override the fdt address. */
> -   if (IS_ENABLED(CONFIG_OF_BOARD)) {
> -   gd->fdt_blob = board_fdt_blob_setup();
> +   if (CONFIG_IS_ENABLED(OF_BLOBLIST)) {
> +   ret = bloblist_maybe_init();
> if (ret)
> return ret;
> -   gd->fdt_src = FDTSRC_BOARD;

So, instead of adding OF_BLOBLIST, just move this code under OF_BOARD,
inside an IS_ENABLED(BLOBLIST) check. If a bloblist is required and
the previous stage loader is supposed to provide a DT we can just
throw an error and stop booting


> +   gd->fdt_blob = bloblist_find(BLOBLISTT_CONTROL_FDT, 0);
> +   if (!gd->fdt_blob) {
> +   printf("Not FDT found in bloblist\n");
> +   bloblist_show_list();
> +   return -ENOENT;
> +   }

[...]

Regards
/Ilias

Re: [PATCH v3 08/38] spl: Create proper symbols for enabling the malloc() pool

[Tom Rini]

On Sun, Sep 24, 2023 at 01:24:53PM -0600, Simon Glass wrote:

> For U-Boot proper we have CONFIG_SYS_MALLOC_F which indicates that a
> malloc() pool is available before relocation.
> 
> For SPL we only have CONFIG_SPL_SYS_MALLOC_F_LEN which indicates the
> size of the pool.
> 
> In various places we use CONFIG_SPL_SYS_MALLOC_F_LEN == 0 to indicate
> that there is no pool.
> 
> This differing approach is confusing. Add a new CONFIG_SPL_SYS_MALLOC_F
> symbol for SPL (and similarly for TPL and VPL). Tidy up the Kconfig
> help for clarity.
> 
> For now these symbols are not used. That is cleaned up in the following
> patches.
> 
> Signed-off-by: Simon Glass 

This introduces:

WARNING: unmet direct dependencies detected for SPL_SYS_MALLOC_F
  Depends on [n]: ARM [=y] && ARCH_SNAPDRAGON [=n] || ARM [=y] && ARCH_SOCFPGA 
[=n] || SPL_FRAMEWORK [=n] && SYS_MALLOC_F [=y] && SPL [=y]
  Selected by [y]:
  - SPL_DM_SERIAL [=y] && SERIAL [=y] && DM_SERIAL [=y] && SPL_DM [=y]

WARNING: unmet direct dependencies detected for SPL_SYS_MALLOC_F
  Depends on [n]: ARM [=y] && ARCH_SNAPDRAGON [=n] || ARM [=y] && ARCH_SOCFPGA 
[=n] || SPL_FRAMEWORK [=n] && SYS_MALLOC_F [=y] && SPL [=y]
  Selected by [y]:
  - SPL_DM_SERIAL [=y] && SERIAL [=y] && DM_SERIAL [=y] && SPL_DM [=y]

On mx28evk and imx28_xea_sb.

-- 
Tom


signature.asc
Description: PGP signature

Re: Trying to boot custom kernel on Wink Hub (i.MX28)

[Rogan Dawes]

That outputs LLC about a second after the mxsldr terminates (kinda slowly,
the individual characters can be seen being emitted one at a time), and
then about 5 seconds later, "Pref". But nothing after that point.

At least it looks like there is something happening, which is excellent!

Thank you!

Without CONFIG_SPL_MXS_PMU_ENABLE_4P2_LINEAR_REGULATOR=y, (i.e. all 3 PMU
options not set) I still get the LLC\nPref as previously.

With only CONFIG_SPL_MXS_PMU_MINIMAL_VDD5V_CURRENT=y, I get LLC\nPref
With only CONFIG_SPL_MXS_PMU_DISABLE_BATT_CHARGE=y, I get LLC\nPref
With only CONFIG_SPL_MXS_PMU_ENABLE_4P2_LINEAR_REGULATOR=y, I get LLC\nPref

With CONFIG_SPL_MXS_PMU_ENABLE_4P2_LINEAR_REGULATOR=y, AND:
- CONFIG_SPL_MXS_PMU_MINIMAL_VDD5V_CURRENT=y, I just get a single L.
- CONFIG_SPL_MXS_PMU_DISABLE_BATT_CHARGE=y, I get the LLC\nPref as
previously.

With CONFIG_SPL_MXS_PMU_MINIMAL_VDD5V_CURRENT=y AND
CONFIG_SPL_MXS_PMU_DISABLE_BATT_CHARGE=y, I get LLC\nPref

With all three =y, I just get L

Rogan

On Mon, 25 Sept 2023 at 17:50, Fabio Estevam  wrote:
>
> On Mon, Sep 25, 2023 at 11:00 AM Rogan Dawes  wrote:
>
> > I see absolutely nothing in the console.
> >
> > I’m wondering whether I can possibly execute the vendor SPL with a
modern u-boot?
>
> The old U-Boot vendor does not use SPL. It uses an old mechanism
> called bootlets.
>
> I suggest that we start from scratch with this imx28-wink-hub support
> that I have just added:
> https://github.com/fabioestevam/u-boot/commits/wink
>
> To build it:
>
> make imx28-wink-hub_defconfig
> make u-boot.sb
>
> Then try to load u-boot.sb via mxsldr.
>
> If it still fails, then I suspect that the power management unit (PMU)
> may be incorrectly configured causing the board to not power up.
>
> Try playing with the CONFIG_SPL_MXS_PMU_ options.
>
> From configs/imx28_xea_defconfig, for example:
>
> CONFIG_SPL_MXS_PMU_MINIMAL_VDD5V_CURRENT=y
> CONFIG_SPL_MXS_PMU_DISABLE_BATT_CHARGE=y
> # CONFIG_SPL_MXS_PMU_ENABLE_4P2_LINEAR_REGULATOR is not set

Re: [PATCH] smegw01: Fix inverted CONFIG_SYS_BOOT_LOCKED logic

[Tom Rini]

On Mon, Sep 25, 2023 at 01:32:59PM -0300, Fabio Estevam wrote:

> From: Eduard Strehlau 
> 
> CONFIG_SYS_BOOT_LOCKED means that a restricted boot environment will
> be used. In this case, hab_auth_img_or_fail should be called to prevent
> U-Boot to continue running when the fitImage authentication fails.
> 
> Fix the logic accordingly.
> 
> Additionally, select CONFIG_SYS_BOOT_LOCKED by default.
> 
> Signed-off-by: Eduard Strehlau 
> Signed-off-by: Fabio Estevam 
> ---
> Hi Tom,
> 
> We have just identified this bug.
> 
> Could you please pick this one directly for U-Boot 2023.10?

OK.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH] arm: mvebu: sata_mv: Add bootstd hook to enable sata_bootdev

[Tony Dinh]

Thanks Stefan!

All the best,
Tony

On Mon, Sep 25, 2023 at 1:14 AM Stefan Roese  wrote:
>
> On 9/6/23 07:22, Tony Dinh wrote:
> > Add hook in sata_mv probe to enable bootstd bootdev.
> >
> > Note: bootdev_setup_for_sibling_blk() invocation is a noop if bootsd is
> > not enabled for ahci sata yet.
> >
> > Signed-off-by: Tony Dinh 
>
> Reviewed-by: Stefan Roese 
>
> Thanks,
> Stefan
>
> > ---
> >
> >   drivers/ata/sata_mv.c | 8 +++-
> >   1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c
> > index 18c7a66db1..55a5365b5a 100644
> > --- a/drivers/ata/sata_mv.c
> > +++ b/drivers/ata/sata_mv.c
> > @@ -34,6 +34,7 @@
> >   #include 
> >   #include 
> >   #include 
> > +#include 
> >   #include 
> >   #include 
> >   #include 
> > @@ -1104,6 +1105,12 @@ static int sata_mv_probe(struct udevice *dev)
> >   /* TODO: undo create */
> >   continue;
> >
> > + ret = bootdev_setup_for_sibling_blk(blk, "sata_bootdev");
> > + if (ret) {
> > + printf("%s: Failed to create bootdev\n", __func__);
> > + continue;
> > + }
> > +
> >   /* If we got here, the current SATA port was probed
> >* successfully, so set the probe status to successful.
> >*/
> > @@ -1116,7 +1123,6 @@ static int sata_mv_probe(struct udevice *dev)
> >   static int sata_mv_scan(struct udevice *dev)
> >   {
> >   /* Nothing to do here */
> > -
> >   return 0;
> >   }
> >
>
> Viele Grüße,
> Stefan Roese
>
> --
> DENX Software Engineering GmbH,  Managing Director: Erika Unter
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de

Re: [PATCH] scripts/Makefile.lib: also consider $(CONFIG_SYS_BOARD)-u-boot.dtsi

[Rasmus Villemoes]

On 25/09/2023 20.19, Tom Rini wrote:
> On Mon, Sep 25, 2023 at 10:27:43AM +0200, Rasmus Villemoes wrote:
>> On 04/05/2023 14.35, Rasmus Villemoes wrote:
>>> On 03/05/2023 16.54, Tom Rini wrote:
>>
 The one last problem now is on stm32mp15_dhcor_basic which is a
 defconfig missing one from OF_LIST but including it in the its file, so
 the above is the patch we need.

>>
>> Hi Tom
>>
>> Can I persuade you to try something like
>> https://source.denx.de/u-boot/u-boot/-/commit/a05e0d0e6b9103542a1076f9cab0005f400fa072
>> again, but leaving the .dtbo targets in there?
>>
>> I could send a patch, but it's entirely mechanical, and not really meant
>> for being applied until we know if there's more to be cleaned up.
> 
> So what ended up being the problem I think is the case Simon pointed out
> where we do take the output from "make all" and concatenate one of the
> dtbs that was generated with u-boot.img or so, and it works.  But maybe
> that should just list all of the valid DTBs that it needs in the
> defconfig to start with? I don't quite know, it was a case I hadn't
> considered at the time.
> 

Re-reading the thread, I can't see where that was mentioned.

But yes, if some boards (still) need that, and have more than one
possible .dtb, the board can't set an OF_LIST different from the default
consisting of DEFAULT_DEVICE_TREE because changing OF_LIST requires
SPL_LOAD_FIT || MULTI_DTB_FIT.

How do we figure out if such boards even exist?

Rasmus

Re: [PATCH] dt-bindings: mtd: Add a schema for binman

[Rob Herring]

On Mon, Sep 25, 2023 at 11:25 AM Simon Glass  wrote:
>
> Hi Miquel,
>
> On Mon, 25 Sept 2023 at 09:24, Miquel Raynal  
> wrote:
> >
> > Hi Simon,
> >
> > > > > > > > > > > I was assuming that I should create a top-level 
> > > > > > > > > > > compatible = "binman"
> > > > > > > > > > > node, with subnodes like compatible = "binman,bl31-atf", 
> > > > > > > > > > > for example.
> > > > > > > > > > > I should use the compatible string to indicate the 
> > > > > > > > > > > contents, right?
> > > > > > > > > >
> > > > > > > > > > Yes for subnodes, and we already have some somewhat 
> > > > > > > > > > standard ones for
> > > > > > > > > > "u-boot" and "u-boot-env". Though historically, "label" was 
> > > > > > > > > > used.
> > > > > > > > >
> > > > > > > > > Binman has common properties for all entries, including 
> > > > > > > > > "compress"
> > > > > > > > > which sets the compression algorithm.
> > > > > > > >
> > > > > > > > I see no issue with adding that. It seems useful and something 
> > > > > > > > missing
> > > > > > > > in the existing partition schemas.
> > > > > > >
> > > > > > > OK I sent a patch with that.
> > > > > > >
> > > > > > > >
> > > > > > > > > So perhaps I should start by defining a new binman,bl31-atf 
> > > > > > > > > which has
> > > > > > > > > common properties from an "binman,entry" definition?
> > > > > > > >
> > > > > > > > I don't understand the binman prefix. The contents are ATF (or 
> > > > > > > > TF-A
> > > > > > > > now). Who wrote it to the flash image is not relevant.
> > > > > > >
> > > > > > > Are you suggesting just "atf-bl31", or "arm,atf-bl31" ? Or should 
> > > > > > > we
> > > > > > > change it to "tfa-bl31"?
> > > > > >
> > > > > > I don't really understand the relationship with TF-A here. Can't we
> > > > > > just have a kind of fixed-partitions with additional properties like
> > > > > > the compression?
> > > > >
> > > > > Binman needs to know what to put in there, which is the purpose of the
> > > > > compatible string.
> > > >
> > > > But "what" should be put inside the partition is part of the input
> > > > argument, not the output. You said (maybe I got this wrong) that the
> > > > schema would apply to the output of binman. If you want to let user
> > > > know what's inside, maybe it is worth adding a label, but otherwise I
> > > > don't like the idea of a compatible for that, which for me would mean:
> > > > "here is how to handle that specific portion of the flash/here is how
> > > > the flash is organized".
> > >
> > > But I thought that the compatible string was for that purpose? See for
> > > example "brcm,bcm4908-firmware" and "brcm,bcm963xx-imagetag" and
> > > "linksys,ns-firmware".
> >
> > These three examples apparently need specific handling, the partitions
> > contain meta-data that a parser needs to check or something like that.
> > And finally it looks like partition names are set depending on the
> > content that was discovered, so yes, the partition name is likely the
> > good location to tell users/OSes what's inside.
> >
> > > > > > Also, I still don't understand the purpose of this schema. So binman
> > > > > > generates an image, you want to flash this image and you would like 
> > > > > > the
> > > > > > tool to generate the corresponding (partition) DT snippet 
> > > > > > automatically.
> > > > > > Do I get this right? I don't get why you would need new compatibles 
> > > > > > for
> > > > > > that.
> > > > >
> > > > > It is actually the other way around. The schema tells Binman how to
> > > > > build the image (what goes in there and where). Then outputs an
> > > > > updated DT which describes where everything ended up, for use by other
> > > > > tools, e.g. firmware update. It is a closed loop in that sense. See
> > > > > the references for more information.
> > > >
> > > > Maybe I fail to see why you would want these description to be
> > > > introduced here, if they are not useful to the OS.
> > >
> > > Well I was asked to send them to Linux since they apparently don't
> > > belong in dt-schema.

That is not what I said. I said fixed-partitions should be extended. I
prefer they are extended in-place before moving them rather than the
other way around.

> > > These are firmware bindings, as indicated, but I
> > > took them out of the /firmware node since that is for a different
> > > purpose. Rob suggested that partitions was a good place. We have fwupd
> > > using DT to hold the firmware-update information, so I expect it will
> > > move to use these bindings too.
> >
> > I would definitely use fixed partitions as that's what you need then:
> > registering where everything starts and ends. If you have "in-band"
> > meta data you might require a compatible, but I don't think you
> > do, in this case you should probably carry the content through a label
> > (which will become the partition name) and we can discuss additional
> > properties if needed.
>
> I believe I am going to need a compatible string at the 'partitions'
> 

Re: [PATCH] scripts/Makefile.lib: also consider $(CONFIG_SYS_BOARD)-u-boot.dtsi

[Tom Rini]

On Mon, Sep 25, 2023 at 10:27:43AM +0200, Rasmus Villemoes wrote:
> On 04/05/2023 14.35, Rasmus Villemoes wrote:
> > On 03/05/2023 16.54, Tom Rini wrote:
> 
> >> The one last problem now is on stm32mp15_dhcor_basic which is a
> >> defconfig missing one from OF_LIST but including it in the its file, so
> >> the above is the patch we need.
> >>
> > 
> > Hm, well, for now I think at least all .dtbo targets need to be left in
> > the Makefiles, since nothing in the defconfig tells us to build those.
> 
> Hi Tom
> 
> Can I persuade you to try something like
> https://source.denx.de/u-boot/u-boot/-/commit/a05e0d0e6b9103542a1076f9cab0005f400fa072
> again, but leaving the .dtbo targets in there?
> 
> I could send a patch, but it's entirely mechanical, and not really meant
> for being applied until we know if there's more to be cleaned up.

So what ended up being the problem I think is the case Simon pointed out
where we do take the output from "make all" and concatenate one of the
dtbs that was generated with u-boot.img or so, and it works.  But maybe
that should just list all of the valid DTBs that it needs in the
defconfig to start with? I don't quite know, it was a case I hadn't
considered at the time.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH 0/3] common: Remove a few more header files

[Tom Rini]

On Mon, Sep 25, 2023 at 10:18:55AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Mon, 25 Sept 2023 at 09:13, Tom Rini  wrote:
> >
> > On Mon, Sep 25, 2023 at 08:35:44AM -0600, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Mon, 25 Sept 2023 at 08:08, Tom Rini  wrote:
> > > >
> > > > On Sun, Sep 24, 2023 at 08:39:49PM -0600, Simon Glass wrote:
> > > >
> > > > > This series removes two more header files from the common.h header. It
> > > > > also tidies up code style for time.h functions, to simplify similar
> > > > > maintenance in future.
> > > > >
> > > > >
> > > > > Simon Glass (3):
> > > > >   Fix code style for time functions
> > > > >   common: Drop time.h from common header
> > > > >   common: Drop linux/string.h from common header
> > > > [snip]
> > > > >  1709 files changed, 1854 insertions(+), 34 deletions(-)
> > > >
> > > > This is where I start to get worried we're possibly taking the wrong
> > > > approach to the problem.  Perhaps it would be better to:
> > > > 1. Start finding headers which include , remove and fixup the
> > > > breakage directly.
> > > > 2. Move on to removing  from files, a directory at a time.
> > > >
> > >
> > > The bigger question is whether you want common.h gone or not?
> >
> > Yes, I'd like it gone.
> >
> > > I shared my script with you so you can see what it is doing. But if a
> > > board uses memcmp(), for example, then it needs linux/string.h
> > > included. We can discuss why so many files use string functions, or
> > > whether the script is not perfect, but ultimately that is the
> > > question.
> > >
> > > Just these simple queries give you a sense of the scale:
> > >
> > >  git grep -l strcat |wc
> > >  45  451134
> > > git grep -l memcpy |wc
> > > 834 834   21643
> > >
> > > If you are suggesting that we add fewer includes for now, relying on
> > > transitive includes in header files, then I don't think that is very
> > > useful in the end. I had a series a few years back which removed
> > > common.h entirely and it foundered on this same issue.
> >
> > The problem I see with the approach you're taking now is that it's quite
> > hard to provide meaningful review.  Looking at the time.h patch here, I
> > assume you agree it would be better to just remove
> > arch/arm/include/asm/arch-tegra/timer.h as it's just a single prototype
> > already found in include/time.h.  And the example where time.h was
> > included inside a comment block is a tooling issue.  But then looking at
> > the printk.h patch I did just apply, I had to drop a few files because
> > they introduced seemingly random changes to the resulting build on some
> > platforms.
> 
> That is very strange. But I think with the Kconfig thing we
> established that the build size can vary slightly for all sorts of
> reasons, nothing to do with actual functional changes, particularly
> with LTO. So as things standard, a build-size change isn't a good
> signal.

I think, LTO platforms aside, it's extremely important and correct.  If
you aren't intending a functional change, there shouldn't be a
functional change.  We need to be able to explain why a change is
happening if we're "just" changing headers.

> >  And something like that is much easier to deal with and
> > explain _why_ it's happening if we're removing the header file by file,
> > rather than world-wide move-a-header.
> 
> With the Kconfig series I was unable to explain the minor 4-8 byte
> changes, despite quite a bit of effort, as you know.
> 
> My suggestion is that we press ahead with the migration. If some
> maintainers want to do clean-up afterwards, then that would be great.

I'd rather go with what I said about removing common.h from files and
fixing them up as needed, and make any new code not include 
since it's long just been "add headers X/Y/Z" and no prototypes itself.
And it's exceedingly hard to review patches like this series has.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH] lmb: remove overlapping region with next range

[Kumar, Udit]

Thanks Simon

On 9/25/2023 9:39 PM, Simon Glass wrote:

Hi Udit,

On Sun, 24 Sept 2023 at 22:10, Kumar, Udit  wrote:

On 9/24/2023 7:21 PM, Heinrich Schuchardt wrote:

Am 24. September 2023 13:18:32 MESZ schrieb Udit Kumar :

[..]

Assumption


Thanks Heinrich  will fix this typo in v2.

Let me wait for other review feedback as well before proceeding for v2.



Regards Heinrich


[..]

Can you please add a test to test/lib which fails before your change
and passes after?


Sure, will add test case.



Regards,
Simon

Re: [PATCH] lmb: remove overlapping region with next range

[Kumar, Udit]

Hi Mark/Tom

On 9/25/2023 9:01 PM, Tom Rini wrote:

On Mon, Sep 25, 2023 at 05:27:26PM +0200, Mark Kettenis wrote:

Date: Mon, 25 Sep 2023 11:21:26 -0400
From: Tom Rini 

On Sun, Sep 24, 2023 at 04:48:32PM +0530, Udit Kumar wrote:


In case of new memory range to be added is coalesced
with any already added non last lmb region.

And there is possibility that, then region in which new memory
range added is not adjacent to next region. But have some
sections are overlapping.

So along with adjacency check with next lmb region,
check for overlap should be done.

In case overlap  is found, adjust and merge these two lmb
region into one.

Reported-by: Suman Anna 
Signed-off-by: Udit Kumar 

This is good!  I wonder if this addresses the issue that has caused
other platforms to need to set CONFIG_LMB_MAX_REGIONS=64 and I'm
wondering if any of the other maintainers I've cc'd here can drop back
to the default number of regions and then re-test their failure case?
Thanks!

Hi Tom,

For the Apple M1 systems I don't think it makes sense to reduce the
number of regions.  These systems don't have TPL or SPL, have plenty
of memory (at least 8GB) and are really fast.  And we anticipate that
the number of memory regions will only grow in the future.

This partly gets at what I see as part of the problem.  It's been
unclear to me if it's reasonable to just bump the default to 64 for
aarch64 (and riscv?) because the problem is that 16 is truly too small,
or given what we're doing we still don't need a ton of LMB regions as at
the end of the day things should be coalesced in to just a few and it's
been an underlying bug in an area everyone assumed wasn't buggy.


Did we observed overlapped memory regions added in lmb region ?

which leads to increase in default number of regions.

Re: [PATCH v3 0/3] enable power and clock Kconfig conditionally

[Kumar, Udit]

Hi Tom,

On 9/13/2023 10:09 AM, Udit Kumar wrote:

This series, updates enabling mutually exclusive power and clock configs
for TI SOCs.


Sorry to push clock and power changes in one series,
managed by two maintainers. But In my quick grep,
I am not able to active PR raise from both trees.
So copying maintainers of clock and power for review.

Change log
Changes in v3:
- Patch 1 and 2: Added reviewed by
- Patch 3: Corrected Fixes in commit message
link to v2:
https://lore.kernel.org/all/20230912130645.1202325-1-u-kum...@ti.com/

Changes in v2:
- Patch 1 and 2) Added conditional support to enable Kconfig
- Patch 3 No change
link to v1:
https://lore.kernel.org/all/2023091946.749270-1-u-kum...@ti.com/

Udit Kumar (3):
   power: domain: ti: Enable single config for power domain
   clk: ti: Add support to enable configs conditionally
   config: j7200: remove not needed power config


NAK,

One of boot mode (OSPI) is not working with this series on J7200,

Please don't merge this series,  Let me fix OSPI boot and re-roll this 
series


Thanks

Udit


  configs/j7200_evm_r5_defconfig | 1 -
  drivers/clk/ti/Kconfig | 8 
  drivers/power/domain/Kconfig   | 2 +-
  3 files changed, 5 insertions(+), 6 deletions(-)

Re: [PATCH v3 01/38] spl: Use CONFIG_SPL... instead of CONFIG_..._SPL_...

[Simon Glass]

Hi Felix,

On Mon, 25 Sept 2023 at 08:56, Felix Brack  wrote:
>
> Hi Simon,
>
> On 24.09.23 21:24, Simon Glass wrote:
> > We like to put the SPL first so it is clear that it relates to SPL. Rename
> > various malloc-related options which have crept in, to stick to this
> > convention.
> >
> > Signed-off-by: Simon Glass 
> > Reviewed-by: Marcel Ziswiler 
> > Reviewed-by: Martyn Welch 
> > Reviewed-by: Svyatoslav Ryhel 
> > ---
> >
> > (no changes since v1)
> > .
>
> I just tried to apply the patch to master for testing and got the
> following result:
>
> error: patch failed: configs/am335x_baltos_defconfig:18
> error: configs/am335x_baltos_defconfig: patch does not apply
> error: patch failed: configs/am335x_igep003x_defconfig:20
> error: configs/am335x_igep003x_defconfig: patch does not apply
> error: patch failed: configs/dh_imx6_defconfig:39
> error: configs/dh_imx6_defconfig: patch does not apply
> error: patch failed: configs/starfive_visionfive2_defconfig:47
> error: configs/starfive_visionfive2_defconfig: patch does not apply
>
> For what I want to test (changes for PDU001 board) this doesn't matter
> however. I'm just not sure: should I go ahead with testing or wait for
> an updated patch?

I had to resync the defconfigs before this patch, to avoid conflicts
when applied.

So you can use './tools/moveconfig -Cy' and then the series should apply OK.

Regards,
Simon

[PATCH] smegw01: Fix inverted CONFIG_SYS_BOOT_LOCKED logic

[Fabio Estevam]

From: Eduard Strehlau 

CONFIG_SYS_BOOT_LOCKED means that a restricted boot environment will
be used. In this case, hab_auth_img_or_fail should be called to prevent
U-Boot to continue running when the fitImage authentication fails.

Fix the logic accordingly.

Additionally, select CONFIG_SYS_BOOT_LOCKED by default.

Signed-off-by: Eduard Strehlau 
Signed-off-by: Fabio Estevam 
---
Hi Tom,

We have just identified this bug.

Could you please pick this one directly for U-Boot 2023.10?

Thanks

 board/storopack/smegw01/smegw01.env | 4 ++--
 configs/smegw01_defconfig   | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/board/storopack/smegw01/smegw01.env 
b/board/storopack/smegw01/smegw01.env
index 528310dd81..93de866910 100644
--- a/board/storopack/smegw01/smegw01.env
+++ b/board/storopack/smegw01/smegw01.env
@@ -67,9 +67,9 @@ mmcboot=
run altbootcmd;
fi;
 #ifdef CONFIG_SYS_BOOT_LOCKED
-   hab_auth_img ${fileaddr} ${filesize};
-#else
hab_auth_img_or_fail ${fileaddr} ${filesize};
+#else
+   hab_auth_img ${fileaddr} ${filesize};
 #endif
run mmcargs;
if bootm; then
diff --git a/configs/smegw01_defconfig b/configs/smegw01_defconfig
index 616038387e..03d403ddc8 100644
--- a/configs/smegw01_defconfig
+++ b/configs/smegw01_defconfig
@@ -7,7 +7,7 @@ CONFIG_ENV_OFFSET=0x10
 CONFIG_DM_GPIO=y
 CONFIG_DEFAULT_DEVICE_TREE="imx7d-smegw01"
 CONFIG_TARGET_SMEGW01=y
-# CONFIG_SYS_BOOT_LOCKED is not set
+CONFIG_SYS_BOOT_LOCKED=y
 CONFIG_ENV_OFFSET_REDUND=0x11
 CONFIG_ARMV7_BOOT_SEC_DEFAULT=y
 # CONFIG_ARMV7_VIRT is not set
-- 
2.41.0

Re: [PATCH] dt-bindings: mtd: Add a schema for binman

[Simon Glass]

Hi Miquel,

On Mon, 25 Sept 2023 at 09:24, Miquel Raynal  wrote:
>
> Hi Simon,
>
> > > > > > > > > > I was assuming that I should create a top-level compatible 
> > > > > > > > > > = "binman"
> > > > > > > > > > node, with subnodes like compatible = "binman,bl31-atf", 
> > > > > > > > > > for example.
> > > > > > > > > > I should use the compatible string to indicate the 
> > > > > > > > > > contents, right?
> > > > > > > > >
> > > > > > > > > Yes for subnodes, and we already have some somewhat standard 
> > > > > > > > > ones for
> > > > > > > > > "u-boot" and "u-boot-env". Though historically, "label" was 
> > > > > > > > > used.
> > > > > > > >
> > > > > > > > Binman has common properties for all entries, including 
> > > > > > > > "compress"
> > > > > > > > which sets the compression algorithm.
> > > > > > >
> > > > > > > I see no issue with adding that. It seems useful and something 
> > > > > > > missing
> > > > > > > in the existing partition schemas.
> > > > > >
> > > > > > OK I sent a patch with that.
> > > > > >
> > > > > > >
> > > > > > > > So perhaps I should start by defining a new binman,bl31-atf 
> > > > > > > > which has
> > > > > > > > common properties from an "binman,entry" definition?
> > > > > > >
> > > > > > > I don't understand the binman prefix. The contents are ATF (or 
> > > > > > > TF-A
> > > > > > > now). Who wrote it to the flash image is not relevant.
> > > > > >
> > > > > > Are you suggesting just "atf-bl31", or "arm,atf-bl31" ? Or should we
> > > > > > change it to "tfa-bl31"?
> > > > >
> > > > > I don't really understand the relationship with TF-A here. Can't we
> > > > > just have a kind of fixed-partitions with additional properties like
> > > > > the compression?
> > > >
> > > > Binman needs to know what to put in there, which is the purpose of the
> > > > compatible string.
> > >
> > > But "what" should be put inside the partition is part of the input
> > > argument, not the output. You said (maybe I got this wrong) that the
> > > schema would apply to the output of binman. If you want to let user
> > > know what's inside, maybe it is worth adding a label, but otherwise I
> > > don't like the idea of a compatible for that, which for me would mean:
> > > "here is how to handle that specific portion of the flash/here is how
> > > the flash is organized".
> >
> > But I thought that the compatible string was for that purpose? See for
> > example "brcm,bcm4908-firmware" and "brcm,bcm963xx-imagetag" and
> > "linksys,ns-firmware".
>
> These three examples apparently need specific handling, the partitions
> contain meta-data that a parser needs to check or something like that.
> And finally it looks like partition names are set depending on the
> content that was discovered, so yes, the partition name is likely the
> good location to tell users/OSes what's inside.
>
> > > > > Also, I still don't understand the purpose of this schema. So binman
> > > > > generates an image, you want to flash this image and you would like 
> > > > > the
> > > > > tool to generate the corresponding (partition) DT snippet 
> > > > > automatically.
> > > > > Do I get this right? I don't get why you would need new compatibles 
> > > > > for
> > > > > that.
> > > >
> > > > It is actually the other way around. The schema tells Binman how to
> > > > build the image (what goes in there and where). Then outputs an
> > > > updated DT which describes where everything ended up, for use by other
> > > > tools, e.g. firmware update. It is a closed loop in that sense. See
> > > > the references for more information.
> > >
> > > Maybe I fail to see why you would want these description to be
> > > introduced here, if they are not useful to the OS.
> >
> > Well I was asked to send them to Linux since they apparently don't
> > belong in dt-schema. These are firmware bindings, as indicated, but I
> > took them out of the /firmware node since that is for a different
> > purpose. Rob suggested that partitions was a good place. We have fwupd
> > using DT to hold the firmware-update information, so I expect it will
> > move to use these bindings too.
>
> I would definitely use fixed partitions as that's what you need then:
> registering where everything starts and ends. If you have "in-band"
> meta data you might require a compatible, but I don't think you
> do, in this case you should probably carry the content through a label
> (which will become the partition name) and we can discuss additional
> properties if needed.

I believe I am going to need a compatible string at the 'partitions'
level to indicate that this is the binman scheme. But we can leave
that until later.

So you are suggesting 'label' for the contents. Rob suggested
'compatible' [1], so what should I do?

With this schema, would every node be called 'partition@...' or is
there flexibility to use other names?

One other point to note is that some entry types will eventually need
other properties, which vary depending on the type. For example a

Re: [PATCH 0/3] common: Remove a few more header files

[Simon Glass]

Hi Tom,

On Mon, 25 Sept 2023 at 09:13, Tom Rini  wrote:
>
> On Mon, Sep 25, 2023 at 08:35:44AM -0600, Simon Glass wrote:
> > Hi Tom,
> >
> > On Mon, 25 Sept 2023 at 08:08, Tom Rini  wrote:
> > >
> > > On Sun, Sep 24, 2023 at 08:39:49PM -0600, Simon Glass wrote:
> > >
> > > > This series removes two more header files from the common.h header. It
> > > > also tidies up code style for time.h functions, to simplify similar
> > > > maintenance in future.
> > > >
> > > >
> > > > Simon Glass (3):
> > > >   Fix code style for time functions
> > > >   common: Drop time.h from common header
> > > >   common: Drop linux/string.h from common header
> > > [snip]
> > > >  1709 files changed, 1854 insertions(+), 34 deletions(-)
> > >
> > > This is where I start to get worried we're possibly taking the wrong
> > > approach to the problem.  Perhaps it would be better to:
> > > 1. Start finding headers which include , remove and fixup the
> > > breakage directly.
> > > 2. Move on to removing  from files, a directory at a time.
> > >
> >
> > The bigger question is whether you want common.h gone or not?
>
> Yes, I'd like it gone.
>
> > I shared my script with you so you can see what it is doing. But if a
> > board uses memcmp(), for example, then it needs linux/string.h
> > included. We can discuss why so many files use string functions, or
> > whether the script is not perfect, but ultimately that is the
> > question.
> >
> > Just these simple queries give you a sense of the scale:
> >
> >  git grep -l strcat |wc
> >  45  451134
> > git grep -l memcpy |wc
> > 834 834   21643
> >
> > If you are suggesting that we add fewer includes for now, relying on
> > transitive includes in header files, then I don't think that is very
> > useful in the end. I had a series a few years back which removed
> > common.h entirely and it foundered on this same issue.
>
> The problem I see with the approach you're taking now is that it's quite
> hard to provide meaningful review.  Looking at the time.h patch here, I
> assume you agree it would be better to just remove
> arch/arm/include/asm/arch-tegra/timer.h as it's just a single prototype
> already found in include/time.h.  And the example where time.h was
> included inside a comment block is a tooling issue.  But then looking at
> the printk.h patch I did just apply, I had to drop a few files because
> they introduced seemingly random changes to the resulting build on some
> platforms.

That is very strange. But I think with the Kconfig thing we
established that the build size can vary slightly for all sorts of
reasons, nothing to do with actual functional changes, particularly
with LTO. So as things standard, a build-size change isn't a good
signal.

>  And something like that is much easier to deal with and
> explain _why_ it's happening if we're removing the header file by file,
> rather than world-wide move-a-header.

With the Kconfig series I was unable to explain the minor 4-8 byte
changes, despite quite a bit of effort, as you know.

My suggestion is that we press ahead with the migration. If some
maintainers want to do clean-up afterwards, then that would be great.

Regards,
Simon

Re: [PATCH] lmb: remove overlapping region with next range

[Simon Glass]

Hi Udit,

On Sun, 24 Sept 2023 at 22:10, Kumar, Udit  wrote:
>
> On 9/24/2023 7:21 PM, Heinrich Schuchardt wrote:
> >
> > Am 24. September 2023 13:18:32 MESZ schrieb Udit Kumar :
> >> In case of new memory range to be added is coalesced
> >> with any already added non last lmb region.
> >>
> >> And there is possibility that, then region in which new memory
> >> range added is not adjacent to next region. But have some
> >> sections are overlapping.
> >>
> >> So along with adjacency check with next lmb region,
> >> check for overlap should be done.
> >>
> >> In case overlap  is found, adjust and merge these two lmb
> >> region into one.
> >>
> >> Reported-by: Suman Anna 
> >> Signed-off-by: Udit Kumar 
> >> ---
> >> logs
> >> https://gist.github.com/uditkumarti/5d08c34442235ad270cfa863792ebcdc
> >> seqeunce : line 1 to 13
> >> without fix : line 96-100
> >> with fix : line 199-202
> >>
> >> lib/lmb.c | 37 +
> >> 1 file changed, 33 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/lib/lmb.c b/lib/lmb.c
> >> index b2c233edb6..2580d01d90 100644
> >> --- a/lib/lmb.c
> >> +++ b/lib/lmb.c
> >> @@ -74,6 +74,16 @@ static long lmb_addrs_adjacent(phys_addr_t base1, 
> >> phys_size_t size1,
> >>  return 0;
> >> }
> >>
> >> +static long lmb_regions_overlap(struct lmb_region *rgn, unsigned long r1,
> >> +unsigned long r2)
> >> +{
> >> +phys_addr_t base1 = rgn->region[r1].base;
> >> +phys_size_t size1 = rgn->region[r1].size;
> >> +phys_addr_t base2 = rgn->region[r2].base;
> >> +phys_size_t size2 = rgn->region[r2].size;
> >> +
> >> +return lmb_addrs_overlap(base1, size1, base2, size2);
> >> +}
> >> static long lmb_regions_adjacent(struct lmb_region *rgn, unsigned long r1,
> >>   unsigned long r2)
> >> {
> >> @@ -81,7 +91,6 @@ static long lmb_regions_adjacent(struct lmb_region *rgn, 
> >> unsigned long r1,
> >>  phys_size_t size1 = rgn->region[r1].size;
> >>  phys_addr_t base2 = rgn->region[r2].base;
> >>  phys_size_t size2 = rgn->region[r2].size;
> >> -
> >>  return lmb_addrs_adjacent(base1, size1, base2, size2);
> >> }
> >>
> >> @@ -105,6 +114,23 @@ static void lmb_coalesce_regions(struct lmb_region 
> >> *rgn, unsigned long r1,
> >>  lmb_remove_region(rgn, r2);
> >> }
> >>
> >> +/*Assmptuon : base addr of region 1 < base addr of region 2*/
> > Assumption
>
>
> Thanks Heinrich  will fix this typo in v2.
>
> Let me wait for other review feedback as well before proceeding for v2.
>
>
> > Regards Heinrich
> >
> >> +static void lmb_fix_over_lap_regions(struct lmb_region *rgn, unsigned 
> >> long r1,
> >> + unsigned long r2)
> >> +{
> >> +phys_addr_t base1 = rgn->region[r1].base;
> >> +phys_size_t size1 = rgn->region[r1].size;
> >> +phys_addr_t base2 = rgn->region[r2].base;
> >> +phys_size_t size2 = rgn->region[r2].size;
> >> +
> >> +if (base1 + size1 > base2 + size2) {
> >> +printf("This will not be a case any time\n");
> >> +return;
> >> +}
> >> +rgn->region[r1].size = base2 + size2 - base1;
> >> +lmb_remove_region(rgn, r2);
> >> +}
> >> +
> >> void lmb_init(struct lmb *lmb)
> >> {
> >> #if IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
> >> @@ -249,7 +275,6 @@ static long lmb_add_region_flags(struct lmb_region 
> >> *rgn, phys_addr_t base,
> >>  phys_size_t rgnflags = rgn->region[i].flags;
> >>  phys_addr_t end = base + size - 1;
> >>  phys_addr_t rgnend = rgnbase + rgnsize - 1;
> >> -
> >>  if (rgnbase <= base && end <= rgnend) {
> >>  if (flags == rgnflags)
> >>  /* Already have this region, so we're done */
> >> @@ -278,10 +303,14 @@ static long lmb_add_region_flags(struct lmb_region 
> >> *rgn, phys_addr_t base,
> >>  }
> >>  }
> >>
> >> -if ((i < rgn->cnt - 1) && lmb_regions_adjacent(rgn, i, i + 1)) {
> >> -if (rgn->region[i].flags == rgn->region[i + 1].flags) {
> >> +if (i < rgn->cnt - 1 && rgn->region[i].flags == rgn->region[i + 
> >> 1].flags)  {
> >> +if (lmb_regions_adjacent(rgn, i, i + 1)) {
> >>  lmb_coalesce_regions(rgn, i, i + 1);
> >>  coalesced++;
> >> +} else if (lmb_regions_overlap(rgn, i, i + 1)) {
> >> +/* fix overlapping area */
> >> +lmb_fix_over_lap_regions(rgn, i, i + 1);
> >> +coalesced++;
> >>  }
> >>  }
> >>

Can you please add a test to test/lib which fails before your change
and passes after?

Regards,
Simon

Re: Config fragments

[Simon Glass]

Hi,

On Mon, 25 Sept 2023 at 09:12, Svyatoslav Ryhel  wrote:
>
> пн, 25 вер. 2023 р. о 18:05 Tom Rini  пише:
> >
> > On Mon, Sep 25, 2023 at 04:18:45PM +0300, Svyatoslav Ryhel wrote:
> > >
> > >
> > > 23 серпня 2023 р. 18:55:58 GMT+03:00, Tom Rini  
> > > написав(-ла):
> > > >On Wed, Aug 23, 2023 at 09:30:31AM -0600, Simon Glass wrote:
> > > >
> > > >> Hi,
> > > >>
> > > >> Up until 2023.04 it has been possible to build all the defconfigs but
> > > >> with 2023.07 that changed. Tom mentioned this to me recently.
> > > >
> > > >I'm adding Svyatoslav who is the person that brought in all of the
> > > >config fragments that are going to be in v2023.07.
> > > >
> > > >> Up until 2023.04 we can enumerate all the board configs that can be
> > > >> built. We can build any of them using a single name and a single
> > > >> defconfig. The boards.cfg file which buildman creates contains a full
> > > >> list of things that can be built.
> > > >>
> > > >> From 2023.07 this changes and we now have random .config files
> > > >> sprinkled about the place. I say random because they are not connected
> > > >> to anything. For example, here is
> > > >
> > > >They aren't random.  They're, just for v2023.07, in configs/ and then
> > > >moving forward, they're in the board directory.  I would like to see
> > > >them in more places possibly, but that's not something I'm sure enough
> > > >on right now.
> > > >
> > > >> board/ti/am62x/MAINTAINERS:
> > > >>
> > > >> AM62x BOARD
> > > >> M: Dave Gerlach 
> > > >> M: Tom Rini 
> > > >> S: Maintained
> > > >> F: board/ti/am62x/
> > > >> F: include/configs/am62x_evm.h
> > > >> F: configs/am62x_evm_r5_defconfig
> > > >> F: configs/am62x_evm_a53_defconfig
> > > >>
> > > >> BEAGLEPLAY BOARD
> > > >> M: Nishanth Menon 
> > > >> M: Robert Nelson 
> > > >> M: Tom Rini 
> > > >> S: Maintained
> > > >> N: beagleplay
> > > >>
> > > >> In most cases the MAINTAINERS file tells us about each board and until
> > > >> [1] I had assumed that was the case. With that patch reverted, these
> > > >> are the only 'orphaned' MAINTAINERS entries (buildman
> > > >> --maintainer-check):
> > > >>
> > > >> WARNING: orphaned defconfig in board/armltd/vexpress64/MAINTAINERS
> > > >> ending at line 8
> > > >> WARNING: orphaned defconfig in board/google/veyron/MAINTAINERS ending 
> > > >> at line 44
> > > >> WARNING: orphaned defconfig in
> > > >> board/mikrotik/crs3xx-98dx3236/MAINTAINERS ending at line 7
> > > >> WARNING: orphaned defconfig in board/st/common/MAINTAINERS ending at 
> > > >> line 6
> > > >> WARNING: orphaned defconfig in board/ti/am62x/MAINTAINERS ending at 
> > > >> line 15
> > > >>
> > > >> But Tom advised me that MAINTAINERS is not intended for this purpose,
> > > >> so the check has been dropped. I am not suggesting we bring it back,
> > > >> necessarily, but if we did, we could use it to specify the .config and
> > > >> defconfig files which are used by each board.
> > > >
> > > >Note that the "N" syntax has been in use, for defconfigs for a lot
> > > >longer, and is why the CI check for unmaintained / orphaned boards
> > > >stopped working.
> > > >
> > > >> *Failing that*, I suggest a new 'name.brd' file in the board directory
> > > >> to contain this information, e.g.
> > > >>
> > > >> # Contents of board/ti/am62x/beagleplay.brd:
> > > >> beagleplay-r5: am62x_evm_r5_defconfig beagleplay_r5.config
> > > >> beagleplay-a53: am62x_evm_a53_defconfig beagleplay_a53.config
> > > >>
> > > >> This could be parsed by buildman and other tools, to produce a list of
> > > >> available boards and to build each using a single name (e.g. make
> > > >> beagleplay-r5_defconfig)
> > > >>
> > > >> What do people think?
> > > >
> > > >I'm not sure I like this direction honestly.  Do we want CI to cover
> > > >these configurations?  Maybe.  But it's just for CI.  Using the TI
> > > >examples, I don't know that you can use buildman today to generate a
> > > >functional binary (and if you can, if --keep will retain the right
> > > >files).
> > > >
> > > >Maybe we just need to make it clear that if you use config fragments
> > > >then you're opting out of CI for that specific platform.  That should be
> > > >fine for example for the TI ones that will be built in OE a bunch
> > > >anyhow so accidental failure to builds will be caught.  I don't know
> > > >about the Tegra platforms.
> > > >
> > >
> > > Greetings! Are there any progress regards fragments move or this will
> > > hang in mailing list indefinitely?
> >
> > I don't follow, sorry.  Yes, please send a patch on top of next to move
> > the fragments for Tegra platforms to match how they're done for TI
> > platforms now.
> >
> > --
> > Tom
>
> Oh, so it was merged already into next, sorry did not check the next branch.
> Tegra fragments move patch was already sent a month ago alongside
> other board improvements, waiting for Thierry now. Thanks!

Re dealing with fragments, we have a proposal as above, but I have not
seen any other 

[PATCH v2 2/2] clk: Add clock driver for Amlogic A1

[Igor Prusov]

This patch adds basic clock driver for Amlogic A1 Family which supports
enabling/disabling some gates, getting frequencies and setting rate
with limited reparenting.

Signed-off-by: Igor Prusov 
Reviewed-by: Simon Glass 
---
 arch/arm/include/asm/arch-meson/clock-a1.h |  23 +
 drivers/clk/meson/Kconfig  |   8 +
 drivers/clk/meson/Makefile |   1 +
 drivers/clk/meson/a1.c | 723 +
 4 files changed, 755 insertions(+)
 create mode 100644 arch/arm/include/asm/arch-meson/clock-a1.h
 create mode 100644 drivers/clk/meson/a1.c

diff --git a/arch/arm/include/asm/arch-meson/clock-a1.h 
b/arch/arm/include/asm/arch-meson/clock-a1.h
new file mode 100644
index 00..f6795f5e0c
--- /dev/null
+++ b/arch/arm/include/asm/arch-meson/clock-a1.h
@@ -0,0 +1,23 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Copyright 2018 - AmLogic, Inc.
+ * Copyright 2023 (C) SberDevices, Inc.
+ */
+
+#ifndef _ARCH_MESON_CLOCK_A1_H_
+#define _ARCH_MESON_CLOCK_A1_H_
+
+/*
+ * Clock controller register offsets
+ */
+#define A1_SYS_OSCIN_CTRL  0x0
+#define A1_SYS_CLK_CTRL0   0x10
+#define A1_SYS_CLK_EN0 0x1c
+#define A1_SAR_ADC_CLK_CTR 0xc0
+#define A1_SPIFC_CLK_CTRL  0xd8
+#define A1_USB_BUSCLK_CTRL 0xdc
+#define A1_SD_EMMC_CLK_CTRL0xe0
+
+#define A1_ANACTRL_FIXPLL_CTRL00x0
+
+#endif /* _ARCH_MESON_CLOCK_A1_H_ */
diff --git a/drivers/clk/meson/Kconfig b/drivers/clk/meson/Kconfig
index 994b44ad7a..cdc9d6f76c 100644
--- a/drivers/clk/meson/Kconfig
+++ b/drivers/clk/meson/Kconfig
@@ -21,3 +21,11 @@ config CLK_MESON_G12A
help
  Enable clock support for the Amlogic G12A SoC family, such as
  the S905X/D2
+
+config CLK_MESON_A1
+   bool "Enable clock support for Amlogic A1"
+   depends on CLK && ARCH_MESON
+   default MESON_A1
+   help
+ Enable clock support for the Amlogic A1 SoC family, such as
+ the A113L
diff --git a/drivers/clk/meson/Makefile b/drivers/clk/meson/Makefile
index a486b13e9c..d975f07aab 100644
--- a/drivers/clk/meson/Makefile
+++ b/drivers/clk/meson/Makefile
@@ -8,3 +8,4 @@ obj-$(CONFIG_CLK_MESON_AXG) += axg.o
 obj-$(CONFIG_CLK_MESON_AXG) += axg-ao.o
 obj-$(CONFIG_CLK_MESON_G12A) += g12a.o
 obj-$(CONFIG_CLK_MESON_G12A) += g12a-ao.o
+obj-$(CONFIG_CLK_MESON_A1) += a1.o
diff --git a/drivers/clk/meson/a1.c b/drivers/clk/meson/a1.c
new file mode 100644
index 00..3aec42f33b
--- /dev/null
+++ b/drivers/clk/meson/a1.c
@@ -0,0 +1,723 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * (C) Copyright 2023 SberDevices, Inc.
+ * Author: Igor Prusov 
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include "clk_meson.h"
+
+/*
+ * This driver supports both PLL and peripherals clock sources.
+ * Following operations are supported:
+ * - calculating clock frequency on a limited tree
+ * - reading muxes and dividers
+ * - enabling/disabling gates without propagation
+ * - reparenting without rate propagation, only on muxes
+ * - setting rates with limited reparenting, only on dividers with mux parent
+ */
+
+#define NR_CLKS154
+#define NR_PLL_CLKS11
+
+/* External clock IDs. Those should not overlap with regular IDs */
+#define EXTERNAL_XTAL  (NR_CLKS + 0)
+#define EXTERNAL_FCLK_DIV2 (NR_CLKS + 1)
+#define EXTERNAL_FCLK_DIV3 (NR_CLKS + 2)
+#define EXTERNAL_FCLK_DIV5 (NR_CLKS + 3)
+#define EXTERNAL_FCLK_DIV7 (NR_CLKS + 4)
+
+#define EXTERNAL_FIXPLL_IN (NR_PLL_CLKS + 1)
+
+#define SET_PARM_VALUE(_priv, _parm, _val) \
+   regmap_update_bits((_priv)->map, (_parm)->reg_off,  \
+  SETPMASK((_parm)->width, (_parm)->shift),\
+  (_val) << (_parm)->shift)
+
+#define GET_PARM_VALUE(_priv, _parm)   \
+({ \
+   uint _reg;  \
+   regmap_read((_priv)->map, (_parm)->reg_off, &_reg); \
+   PARM_GET((_parm)->width, (_parm)->shift, _reg); \
+})
+
+struct meson_clk {
+   struct regmap *map;
+};
+
+/**
+ * enum meson_clk_type - The type of clock
+ * @MESON_CLK_ANY: Special value that matches any clock type
+ * @MESON_CLK_GATE: This clock is a gate
+ * @MESON_CLK_MUX: This clock is a multiplexer
+ * @MESON_CLK_DIV: This clock is a configurable divider
+ * @MESON_CLK_FIXED_DIV: This clock is a configurable divider
+ * @MESON_CLK_EXTERNAL: This is an external clock from different clock provider
+ * @MESON_CLK_PLL: This is a PLL
+ */
+enum meson_clk_type {
+   MESON_CLK_ANY = 0,
+   MESON_CLK_GATE,
+   MESON_CLK_MUX,
+   MESON_CLK_DIV,
+   MESON_CLK_FIXED_DIV,
+  

[PATCH v2 1/2] dt-bindings: clock: Add Amlogic A1 clock bindings

[Igor Prusov]

Add clock bindings for Amlogic A1 from linux-next next-20230821.

Signed-off-by: Igor Prusov 
Reviewed-by: Simon Glass 
---
 .../clock/amlogic,a1-peripherals-clkc.h   | 168 ++
 .../dt-bindings/clock/amlogic,a1-pll-clkc.h   |  25 +++
 2 files changed, 193 insertions(+)
 create mode 100644 include/dt-bindings/clock/amlogic,a1-peripherals-clkc.h
 create mode 100644 include/dt-bindings/clock/amlogic,a1-pll-clkc.h

diff --git a/include/dt-bindings/clock/amlogic,a1-peripherals-clkc.h 
b/include/dt-bindings/clock/amlogic,a1-peripherals-clkc.h
new file mode 100644
index 00..06f198ee76
--- /dev/null
+++ b/include/dt-bindings/clock/amlogic,a1-peripherals-clkc.h
@@ -0,0 +1,168 @@
+/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
+/*
+ * Copyright (c) 2019 Amlogic, Inc. All rights reserved.
+ * Author: Jian Hu 
+ *
+ * Copyright (c) 2023, SberDevices. All Rights Reserved.
+ * Author: Dmitry Rokosov 
+ */
+
+#ifndef __A1_PERIPHERALS_CLKC_H
+#define __A1_PERIPHERALS_CLKC_H
+
+#define CLKID_XTAL_IN  0
+#define CLKID_FIXPLL_IN1
+#define CLKID_USB_PHY_IN   2
+#define CLKID_USB_CTRL_IN  3
+#define CLKID_HIFIPLL_IN   4
+#define CLKID_SYSPLL_IN5
+#define CLKID_DDS_IN   6
+#define CLKID_SYS  7
+#define CLKID_CLKTREE  8
+#define CLKID_RESET_CTRL   9
+#define CLKID_ANALOG_CTRL  10
+#define CLKID_PWR_CTRL 11
+#define CLKID_PAD_CTRL 12
+#define CLKID_SYS_CTRL 13
+#define CLKID_TEMP_SENSOR  14
+#define CLKID_AM2AXI_DIV   15
+#define CLKID_SPICC_B  16
+#define CLKID_SPICC_A  17
+#define CLKID_MSR  18
+#define CLKID_AUDIO19
+#define CLKID_JTAG_CTRL20
+#define CLKID_SARADC_EN21
+#define CLKID_PWM_EF   22
+#define CLKID_PWM_CD   23
+#define CLKID_PWM_AB   24
+#define CLKID_CEC  25
+#define CLKID_I2C_S26
+#define CLKID_IR_CTRL  27
+#define CLKID_I2C_M_D  28
+#define CLKID_I2C_M_C  29
+#define CLKID_I2C_M_B  30
+#define CLKID_I2C_M_A  31
+#define CLKID_ACODEC   32
+#define CLKID_OTP  33
+#define CLKID_SD_EMMC_A34
+#define CLKID_USB_PHY  35
+#define CLKID_USB_CTRL 36
+#define CLKID_SYS_DSPB 37
+#define CLKID_SYS_DSPA 38
+#define CLKID_DMA  39
+#define CLKID_IRQ_CTRL 40
+#define CLKID_NIC  41
+#define CLKID_GIC  42
+#define CLKID_UART_C   43
+#define CLKID_UART_B   44
+#define CLKID_UART_A   45
+#define CLKID_SYS_PSRAM46
+#define CLKID_RSA  47
+#define CLKID_CORESIGHT48
+#define CLKID_AM2AXI_VAD   49
+#define CLKID_AUDIO_VAD50
+#define CLKID_AXI_DMC  51
+#define CLKID_AXI_PSRAM52
+#define CLKID_RAMB 53
+#define CLKID_RAMA 54
+#define CLKID_AXI_SPIFC55
+#define CLKID_AXI_NIC  56
+#define CLKID_AXI_DMA  57
+#define CLKID_CPU_CTRL 58
+#define CLKID_ROM  59
+#define CLKID_PROC_I2C 60
+#define CLKID_DSPA_SEL 61
+#define CLKID_DSPB_SEL 62
+#define CLKID_DSPA_EN  63
+#define CLKID_DSPA_EN_NIC  64
+#define CLKID_DSPB_EN  65
+#define CLKID_DSPB_EN_NIC  66
+#define CLKID_RTC  67
+#define CLKID_CECA_32K 68
+#define CLKID_CECB_32K 69
+#define CLKID_24M  70
+#define CLKID_12M  71
+#define CLKID_FCLK_DIV2_DIVN   72
+#define CLKID_GEN  73
+#define CLKID_SARADC_SEL   74
+#define CLKID_SARADC   75
+#define CLKID_PWM_A76
+#define CLKID_PWM_B77
+#define CLKID_PWM_C78
+#define CLKID_PWM_D79
+#define CLKID_PWM_E80
+#define CLKID_PWM_F81
+#define CLKID_SPICC82
+#define CLKID_TS   83
+#define CLKID_SPIFC84
+#define CLKID_USB_BUS  85
+#define CLKID_SD_EMMC  86
+#define CLKID_PSRAM87
+#define CLKID_DMC  88
+#define CLKID_SYS_A_SEL89
+#define CLKID_SYS_A_DIV90
+#define CLKID_SYS_A91
+#define CLKID_SYS_B_SEL92
+#define CLKID_SYS_B_DIV93
+#define CLKID_SYS_B94
+#define CLKID_DSPA_A_SEL   95
+#define CLKID_DSPA_A_DIV   96
+#define CLKID_DSPA_A   97
+#define CLKID_DSPA_B_SEL   98
+#define CLKID_DSPA_B_DIV   99
+#define CLKID_DSPA_B   100
+#define CLKID_DSPB_A_SEL   101
+#define CLKID_DSPB_A_DIV   102
+#define CLKID_DSPB_A   103
+#define CLKID_DSPB_B_SEL   104
+#define CLKID_DSPB_B_DIV   105
+#define CLKID_DSPB_B   106
+#define CLKID_RTC_32K_IN   107
+#define CLKID_RTC_32K_DIV  108
+#define CLKID_RTC_32K_XTAL 109
+#define 

[PATCH v2 0/2] clk: amlogic: a1: Add Amlogic A1 clock driver

[Igor Prusov]

This series adds dt-bindings and driver implementation for Amlogic A1
PLL and Peripherals clock controllers.

V1: 
https://lore.kernel.org/all/20230917101308.1250-1-ivpru...@salutedevices.com/

V1 -> V2:
 - Add more verbose comments for driver

Igor Prusov (2):
  dt-bindings: clock: Add Amlogic A1 clock bindings
  clk: Add clock driver for Amlogic A1

 arch/arm/include/asm/arch-meson/clock-a1.h|  23 +
 drivers/clk/meson/Kconfig |   8 +
 drivers/clk/meson/Makefile|   1 +
 drivers/clk/meson/a1.c| 723 ++
 .../clock/amlogic,a1-peripherals-clkc.h   | 168 
 .../dt-bindings/clock/amlogic,a1-pll-clkc.h   |  25 +
 6 files changed, 948 insertions(+)
 create mode 100644 arch/arm/include/asm/arch-meson/clock-a1.h
 create mode 100644 drivers/clk/meson/a1.c
 create mode 100644 include/dt-bindings/clock/amlogic,a1-peripherals-clkc.h
 create mode 100644 include/dt-bindings/clock/amlogic,a1-pll-clkc.h

-- 
2.34.1

Re: Trying to boot custom kernel on Wink Hub (i.MX28)

[Fabio Estevam]

On Mon, Sep 25, 2023 at 11:00 AM Rogan Dawes  wrote:

> I see absolutely nothing in the console.
>
> I’m wondering whether I can possibly execute the vendor SPL with a modern 
> u-boot?

The old U-Boot vendor does not use SPL. It uses an old mechanism
called bootlets.

I suggest that we start from scratch with this imx28-wink-hub support
that I have just added:
https://github.com/fabioestevam/u-boot/commits/wink

To build it:

make imx28-wink-hub_defconfig
make u-boot.sb

Then try to load u-boot.sb via mxsldr.

If it still fails, then I suspect that the power management unit (PMU)
may be incorrectly configured causing the board to not power up.

Try playing with the CONFIG_SPL_MXS_PMU_ options.

>From configs/imx28_xea_defconfig, for example:

CONFIG_SPL_MXS_PMU_MINIMAL_VDD5V_CURRENT=y
CONFIG_SPL_MXS_PMU_DISABLE_BATT_CHARGE=y
# CONFIG_SPL_MXS_PMU_ENABLE_4P2_LINEAR_REGULATOR is not set

Re: [PATCH] lmb: remove overlapping region with next range

[Tom Rini]

On Mon, Sep 25, 2023 at 05:27:26PM +0200, Mark Kettenis wrote:
> > Date: Mon, 25 Sep 2023 11:21:26 -0400
> > From: Tom Rini 
> > 
> > On Sun, Sep 24, 2023 at 04:48:32PM +0530, Udit Kumar wrote:
> > 
> > > In case of new memory range to be added is coalesced
> > > with any already added non last lmb region.
> > > 
> > > And there is possibility that, then region in which new memory
> > > range added is not adjacent to next region. But have some
> > > sections are overlapping.
> > > 
> > > So along with adjacency check with next lmb region,
> > > check for overlap should be done.
> > > 
> > > In case overlap  is found, adjust and merge these two lmb
> > > region into one.
> > > 
> > > Reported-by: Suman Anna 
> > > Signed-off-by: Udit Kumar 
> > 
> > This is good!  I wonder if this addresses the issue that has caused
> > other platforms to need to set CONFIG_LMB_MAX_REGIONS=64 and I'm
> > wondering if any of the other maintainers I've cc'd here can drop back
> > to the default number of regions and then re-test their failure case?
> > Thanks!
> 
> Hi Tom,
> 
> For the Apple M1 systems I don't think it makes sense to reduce the
> number of regions.  These systems don't have TPL or SPL, have plenty
> of memory (at least 8GB) and are really fast.  And we anticipate that
> the number of memory regions will only grow in the future.

This partly gets at what I see as part of the problem.  It's been
unclear to me if it's reasonable to just bump the default to 64 for
aarch64 (and riscv?) because the problem is that 16 is truly too small,
or given what we're doing we still don't need a ton of LMB regions as at
the end of the day things should be coalesced in to just a few and it's
been an underlying bug in an area everyone assumed wasn't buggy.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH] lmb: remove overlapping region with next range

[Mark Kettenis]

> Date: Mon, 25 Sep 2023 11:21:26 -0400
> From: Tom Rini 
> 
> On Sun, Sep 24, 2023 at 04:48:32PM +0530, Udit Kumar wrote:
> 
> > In case of new memory range to be added is coalesced
> > with any already added non last lmb region.
> > 
> > And there is possibility that, then region in which new memory
> > range added is not adjacent to next region. But have some
> > sections are overlapping.
> > 
> > So along with adjacency check with next lmb region,
> > check for overlap should be done.
> > 
> > In case overlap  is found, adjust and merge these two lmb
> > region into one.
> > 
> > Reported-by: Suman Anna 
> > Signed-off-by: Udit Kumar 
> 
> This is good!  I wonder if this addresses the issue that has caused
> other platforms to need to set CONFIG_LMB_MAX_REGIONS=64 and I'm
> wondering if any of the other maintainers I've cc'd here can drop back
> to the default number of regions and then re-test their failure case?
> Thanks!

Hi Tom,

For the Apple M1 systems I don't think it makes sense to reduce the
number of regions.  These systems don't have TPL or SPL, have plenty
of memory (at least 8GB) and are really fast.  And we anticipate that
the number of memory regions will only grow in the future.

Cheers,

Mark

> > ---
> > logs
> > https://gist.github.com/uditkumarti/5d08c34442235ad270cfa863792ebcdc
> > seqeunce : line 1 to 13
> > without fix : line 96-100
> > with fix : line 199-202
> > 
> >  lib/lmb.c | 37 +
> >  1 file changed, 33 insertions(+), 4 deletions(-)
> > 
> > diff --git a/lib/lmb.c b/lib/lmb.c
> > index b2c233edb6..2580d01d90 100644
> > --- a/lib/lmb.c
> > +++ b/lib/lmb.c
> > @@ -74,6 +74,16 @@ static long lmb_addrs_adjacent(phys_addr_t base1, 
> > phys_size_t size1,
> > return 0;
> >  }
> >  
> > +static long lmb_regions_overlap(struct lmb_region *rgn, unsigned long r1,
> > +   unsigned long r2)
> > +{
> > +   phys_addr_t base1 = rgn->region[r1].base;
> > +   phys_size_t size1 = rgn->region[r1].size;
> > +   phys_addr_t base2 = rgn->region[r2].base;
> > +   phys_size_t size2 = rgn->region[r2].size;
> > +
> > +   return lmb_addrs_overlap(base1, size1, base2, size2);
> > +}
> >  static long lmb_regions_adjacent(struct lmb_region *rgn, unsigned long r1,
> >  unsigned long r2)
> >  {
> > @@ -81,7 +91,6 @@ static long lmb_regions_adjacent(struct lmb_region *rgn, 
> > unsigned long r1,
> > phys_size_t size1 = rgn->region[r1].size;
> > phys_addr_t base2 = rgn->region[r2].base;
> > phys_size_t size2 = rgn->region[r2].size;
> > -
> > return lmb_addrs_adjacent(base1, size1, base2, size2);
> >  }
> >  
> > @@ -105,6 +114,23 @@ static void lmb_coalesce_regions(struct lmb_region 
> > *rgn, unsigned long r1,
> > lmb_remove_region(rgn, r2);
> >  }
> >  
> > +/*Assmptuon : base addr of region 1 < base addr of region 2*/
> > +static void lmb_fix_over_lap_regions(struct lmb_region *rgn, unsigned long 
> > r1,
> > +unsigned long r2)
> > +{
> > +   phys_addr_t base1 = rgn->region[r1].base;
> > +   phys_size_t size1 = rgn->region[r1].size;
> > +   phys_addr_t base2 = rgn->region[r2].base;
> > +   phys_size_t size2 = rgn->region[r2].size;
> > +
> > +   if (base1 + size1 > base2 + size2) {
> > +   printf("This will not be a case any time\n");
> > +   return;
> > +   }
> > +   rgn->region[r1].size = base2 + size2 - base1;
> > +   lmb_remove_region(rgn, r2);
> > +}
> > +
> >  void lmb_init(struct lmb *lmb)
> >  {
> >  #if IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
> > @@ -249,7 +275,6 @@ static long lmb_add_region_flags(struct lmb_region 
> > *rgn, phys_addr_t base,
> > phys_size_t rgnflags = rgn->region[i].flags;
> > phys_addr_t end = base + size - 1;
> > phys_addr_t rgnend = rgnbase + rgnsize - 1;
> > -
> > if (rgnbase <= base && end <= rgnend) {
> > if (flags == rgnflags)
> > /* Already have this region, so we're done */
> > @@ -278,10 +303,14 @@ static long lmb_add_region_flags(struct lmb_region 
> > *rgn, phys_addr_t base,
> > }
> > }
> >  
> > -   if ((i < rgn->cnt - 1) && lmb_regions_adjacent(rgn, i, i + 1)) {
> > -   if (rgn->region[i].flags == rgn->region[i + 1].flags) {
> > +   if (i < rgn->cnt - 1 && rgn->region[i].flags == rgn->region[i + 
> > 1].flags)  {
> > +   if (lmb_regions_adjacent(rgn, i, i + 1)) {
> > lmb_coalesce_regions(rgn, i, i + 1);
> > coalesced++;
> > +   } else if (lmb_regions_overlap(rgn, i, i + 1)) {
> > +   /* fix overlapping area */
> > +   lmb_fix_over_lap_regions(rgn, i, i + 1);
> > +   coalesced++;
> > }
> > }
> >  
> > -- 
> > 2.34.1
> > 
> 
> -- 
> Tom
> 
> [2:application/pgp-signature Show Save:signature.asc (659B)]
> 

Re: [PATCH] dt-bindings: mtd: Add a schema for binman

[Miquel Raynal]

Hi Simon,

> > > > > > > > > I was assuming that I should create a top-level compatible = 
> > > > > > > > > "binman"
> > > > > > > > > node, with subnodes like compatible = "binman,bl31-atf", for 
> > > > > > > > > example.
> > > > > > > > > I should use the compatible string to indicate the contents, 
> > > > > > > > > right?  
> > > > > > > >
> > > > > > > > Yes for subnodes, and we already have some somewhat standard 
> > > > > > > > ones for
> > > > > > > > "u-boot" and "u-boot-env". Though historically, "label" was 
> > > > > > > > used.  
> > > > > > >
> > > > > > > Binman has common properties for all entries, including "compress"
> > > > > > > which sets the compression algorithm.  
> > > > > >
> > > > > > I see no issue with adding that. It seems useful and something 
> > > > > > missing
> > > > > > in the existing partition schemas.  
> > > > >
> > > > > OK I sent a patch with that.
> > > > >  
> > > > > >  
> > > > > > > So perhaps I should start by defining a new binman,bl31-atf which 
> > > > > > > has
> > > > > > > common properties from an "binman,entry" definition?  
> > > > > >
> > > > > > I don't understand the binman prefix. The contents are ATF (or TF-A
> > > > > > now). Who wrote it to the flash image is not relevant.  
> > > > >
> > > > > Are you suggesting just "atf-bl31", or "arm,atf-bl31" ? Or should we
> > > > > change it to "tfa-bl31"?  
> > > >
> > > > I don't really understand the relationship with TF-A here. Can't we
> > > > just have a kind of fixed-partitions with additional properties like
> > > > the compression?  
> > >
> > > Binman needs to know what to put in there, which is the purpose of the
> > > compatible string.  
> >
> > But "what" should be put inside the partition is part of the input
> > argument, not the output. You said (maybe I got this wrong) that the
> > schema would apply to the output of binman. If you want to let user
> > know what's inside, maybe it is worth adding a label, but otherwise I
> > don't like the idea of a compatible for that, which for me would mean:
> > "here is how to handle that specific portion of the flash/here is how
> > the flash is organized".  
> 
> But I thought that the compatible string was for that purpose? See for
> example "brcm,bcm4908-firmware" and "brcm,bcm963xx-imagetag" and
> "linksys,ns-firmware".

These three examples apparently need specific handling, the partitions
contain meta-data that a parser needs to check or something like that.
And finally it looks like partition names are set depending on the
content that was discovered, so yes, the partition name is likely the
good location to tell users/OSes what's inside.

> > > > Also, I still don't understand the purpose of this schema. So binman
> > > > generates an image, you want to flash this image and you would like the
> > > > tool to generate the corresponding (partition) DT snippet automatically.
> > > > Do I get this right? I don't get why you would need new compatibles for
> > > > that.  
> > >
> > > It is actually the other way around. The schema tells Binman how to
> > > build the image (what goes in there and where). Then outputs an
> > > updated DT which describes where everything ended up, for use by other
> > > tools, e.g. firmware update. It is a closed loop in that sense. See
> > > the references for more information.  
> >
> > Maybe I fail to see why you would want these description to be
> > introduced here, if they are not useful to the OS.  
> 
> Well I was asked to send them to Linux since they apparently don't
> belong in dt-schema. These are firmware bindings, as indicated, but I
> took them out of the /firmware node since that is for a different
> purpose. Rob suggested that partitions was a good place. We have fwupd
> using DT to hold the firmware-update information, so I expect it will
> move to use these bindings too.

I would definitely use fixed partitions as that's what you need then:
registering where everything starts and ends. If you have "in-band"
meta data you might require a compatible, but I don't think you
do, in this case you should probably carry the content through a label
(which will become the partition name) and we can discuss additional
properties if needed.

> > > [1] https://u-boot.readthedocs.io/en/latest/develop/package/index.html
> > > [2] 
> > > https://pretalx.com/media/osfc2019/submissions/Y7EN9V/resources/Binman_-_A_data-controlled_firmware_packer_for_U-B_pFU3n2K.pdf
> > > [3] https://www.youtube.com/watch?v=L84ujgUXBOQ  
> >  
> 
> Regards,
> Simon


Thanks,
Miquèl

Re: [PATCH] lmb: remove overlapping region with next range

[Tom Rini]

On Sun, Sep 24, 2023 at 04:48:32PM +0530, Udit Kumar wrote:

> In case of new memory range to be added is coalesced
> with any already added non last lmb region.
> 
> And there is possibility that, then region in which new memory
> range added is not adjacent to next region. But have some
> sections are overlapping.
> 
> So along with adjacency check with next lmb region,
> check for overlap should be done.
> 
> In case overlap  is found, adjust and merge these two lmb
> region into one.
> 
> Reported-by: Suman Anna 
> Signed-off-by: Udit Kumar 

This is good!  I wonder if this addresses the issue that has caused
other platforms to need to set CONFIG_LMB_MAX_REGIONS=64 and I'm
wondering if any of the other maintainers I've cc'd here can drop back
to the default number of regions and then re-test their failure case?
Thanks!

> ---
> logs
> https://gist.github.com/uditkumarti/5d08c34442235ad270cfa863792ebcdc
> seqeunce : line 1 to 13
> without fix : line 96-100
> with fix : line 199-202
> 
>  lib/lmb.c | 37 +
>  1 file changed, 33 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/lmb.c b/lib/lmb.c
> index b2c233edb6..2580d01d90 100644
> --- a/lib/lmb.c
> +++ b/lib/lmb.c
> @@ -74,6 +74,16 @@ static long lmb_addrs_adjacent(phys_addr_t base1, 
> phys_size_t size1,
>   return 0;
>  }
>  
> +static long lmb_regions_overlap(struct lmb_region *rgn, unsigned long r1,
> + unsigned long r2)
> +{
> + phys_addr_t base1 = rgn->region[r1].base;
> + phys_size_t size1 = rgn->region[r1].size;
> + phys_addr_t base2 = rgn->region[r2].base;
> + phys_size_t size2 = rgn->region[r2].size;
> +
> + return lmb_addrs_overlap(base1, size1, base2, size2);
> +}
>  static long lmb_regions_adjacent(struct lmb_region *rgn, unsigned long r1,
>unsigned long r2)
>  {
> @@ -81,7 +91,6 @@ static long lmb_regions_adjacent(struct lmb_region *rgn, 
> unsigned long r1,
>   phys_size_t size1 = rgn->region[r1].size;
>   phys_addr_t base2 = rgn->region[r2].base;
>   phys_size_t size2 = rgn->region[r2].size;
> -
>   return lmb_addrs_adjacent(base1, size1, base2, size2);
>  }
>  
> @@ -105,6 +114,23 @@ static void lmb_coalesce_regions(struct lmb_region *rgn, 
> unsigned long r1,
>   lmb_remove_region(rgn, r2);
>  }
>  
> +/*Assmptuon : base addr of region 1 < base addr of region 2*/
> +static void lmb_fix_over_lap_regions(struct lmb_region *rgn, unsigned long 
> r1,
> +  unsigned long r2)
> +{
> + phys_addr_t base1 = rgn->region[r1].base;
> + phys_size_t size1 = rgn->region[r1].size;
> + phys_addr_t base2 = rgn->region[r2].base;
> + phys_size_t size2 = rgn->region[r2].size;
> +
> + if (base1 + size1 > base2 + size2) {
> + printf("This will not be a case any time\n");
> + return;
> + }
> + rgn->region[r1].size = base2 + size2 - base1;
> + lmb_remove_region(rgn, r2);
> +}
> +
>  void lmb_init(struct lmb *lmb)
>  {
>  #if IS_ENABLED(CONFIG_LMB_USE_MAX_REGIONS)
> @@ -249,7 +275,6 @@ static long lmb_add_region_flags(struct lmb_region *rgn, 
> phys_addr_t base,
>   phys_size_t rgnflags = rgn->region[i].flags;
>   phys_addr_t end = base + size - 1;
>   phys_addr_t rgnend = rgnbase + rgnsize - 1;
> -
>   if (rgnbase <= base && end <= rgnend) {
>   if (flags == rgnflags)
>   /* Already have this region, so we're done */
> @@ -278,10 +303,14 @@ static long lmb_add_region_flags(struct lmb_region 
> *rgn, phys_addr_t base,
>   }
>   }
>  
> - if ((i < rgn->cnt - 1) && lmb_regions_adjacent(rgn, i, i + 1)) {
> - if (rgn->region[i].flags == rgn->region[i + 1].flags) {
> + if (i < rgn->cnt - 1 && rgn->region[i].flags == rgn->region[i + 
> 1].flags)  {
> + if (lmb_regions_adjacent(rgn, i, i + 1)) {
>   lmb_coalesce_regions(rgn, i, i + 1);
>   coalesced++;
> + } else if (lmb_regions_overlap(rgn, i, i + 1)) {
> + /* fix overlapping area */
> + lmb_fix_over_lap_regions(rgn, i, i + 1);
> + coalesced++;
>   }
>   }
>  
> -- 
> 2.34.1
> 

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH 4/4] binman: update documentation for fit,align property

[Jonas Karlman]

Hi Rasmus,

On 2023-09-19 13:37, Rasmus Villemoes wrote:
> Eliminate the repetition "what alignment to use ... and provides the
> alignment to use", and indicate that fit,align can both be used by
> itself and together with fit,external-offset.
> 
> Signed-off-by: Rasmus Villemoes 
> ---
>  tools/binman/entries.rst | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
> index e7dfe6b2a3..f9ad27ce8c 100644
> --- a/tools/binman/entries.rst
> +++ b/tools/binman/entries.rst
> @@ -691,9 +691,8 @@ The top-level 'fit' node supports the following special 
> properties:
>  external offset. This is passed to mkimage via the -E and -p flags.
>  
>  fit,align
> -Indicates what alignment to use for the FIT and its external data,
> -and provides the alignment to use. This is passed to mkimage via
> -the -B flag.
> +Indicates what alignment to use for the FIT and, if applicable,
> +its external data. This is passed to mkimage via the -B flag.

This only updates entries.rst, please update tools/binman/etype/fit.py
and re-generate entries.rst from output of running binman entry-docs.

Regards,
Jonas

>  
>  fit,fdt-list
>  Indicates the entry argument which provides the list of device tree

Re: [PATCH 0/3] common: Remove a few more header files

[Tom Rini]

On Mon, Sep 25, 2023 at 08:35:44AM -0600, Simon Glass wrote:
> Hi Tom,
> 
> On Mon, 25 Sept 2023 at 08:08, Tom Rini  wrote:
> >
> > On Sun, Sep 24, 2023 at 08:39:49PM -0600, Simon Glass wrote:
> >
> > > This series removes two more header files from the common.h header. It
> > > also tidies up code style for time.h functions, to simplify similar
> > > maintenance in future.
> > >
> > >
> > > Simon Glass (3):
> > >   Fix code style for time functions
> > >   common: Drop time.h from common header
> > >   common: Drop linux/string.h from common header
> > [snip]
> > >  1709 files changed, 1854 insertions(+), 34 deletions(-)
> >
> > This is where I start to get worried we're possibly taking the wrong
> > approach to the problem.  Perhaps it would be better to:
> > 1. Start finding headers which include , remove and fixup the
> > breakage directly.
> > 2. Move on to removing  from files, a directory at a time.
> >
> 
> The bigger question is whether you want common.h gone or not?

Yes, I'd like it gone.

> I shared my script with you so you can see what it is doing. But if a
> board uses memcmp(), for example, then it needs linux/string.h
> included. We can discuss why so many files use string functions, or
> whether the script is not perfect, but ultimately that is the
> question.
> 
> Just these simple queries give you a sense of the scale:
> 
>  git grep -l strcat |wc
>  45  451134
> git grep -l memcpy |wc
> 834 834   21643
> 
> If you are suggesting that we add fewer includes for now, relying on
> transitive includes in header files, then I don't think that is very
> useful in the end. I had a series a few years back which removed
> common.h entirely and it foundered on this same issue.

The problem I see with the approach you're taking now is that it's quite
hard to provide meaningful review.  Looking at the time.h patch here, I
assume you agree it would be better to just remove 
arch/arm/include/asm/arch-tegra/timer.h as it's just a single prototype
already found in include/time.h.  And the example where time.h was
included inside a comment block is a tooling issue.  But then looking at
the printk.h patch I did just apply, I had to drop a few files because
they introduced seemingly random changes to the resulting build on some
platforms.  And something like that is much easier to deal with and
explain _why_ it's happening if we're removing the header file by file,
rather than world-wide move-a-header.

-- 
Tom


signature.asc
Description: PGP signature

Re: Config fragments

[Svyatoslav Ryhel]

пн, 25 вер. 2023 р. о 18:05 Tom Rini  пише:
>
> On Mon, Sep 25, 2023 at 04:18:45PM +0300, Svyatoslav Ryhel wrote:
> >
> >
> > 23 серпня 2023 р. 18:55:58 GMT+03:00, Tom Rini  
> > написав(-ла):
> > >On Wed, Aug 23, 2023 at 09:30:31AM -0600, Simon Glass wrote:
> > >
> > >> Hi,
> > >>
> > >> Up until 2023.04 it has been possible to build all the defconfigs but
> > >> with 2023.07 that changed. Tom mentioned this to me recently.
> > >
> > >I'm adding Svyatoslav who is the person that brought in all of the
> > >config fragments that are going to be in v2023.07.
> > >
> > >> Up until 2023.04 we can enumerate all the board configs that can be
> > >> built. We can build any of them using a single name and a single
> > >> defconfig. The boards.cfg file which buildman creates contains a full
> > >> list of things that can be built.
> > >>
> > >> From 2023.07 this changes and we now have random .config files
> > >> sprinkled about the place. I say random because they are not connected
> > >> to anything. For example, here is
> > >
> > >They aren't random.  They're, just for v2023.07, in configs/ and then
> > >moving forward, they're in the board directory.  I would like to see
> > >them in more places possibly, but that's not something I'm sure enough
> > >on right now.
> > >
> > >> board/ti/am62x/MAINTAINERS:
> > >>
> > >> AM62x BOARD
> > >> M: Dave Gerlach 
> > >> M: Tom Rini 
> > >> S: Maintained
> > >> F: board/ti/am62x/
> > >> F: include/configs/am62x_evm.h
> > >> F: configs/am62x_evm_r5_defconfig
> > >> F: configs/am62x_evm_a53_defconfig
> > >>
> > >> BEAGLEPLAY BOARD
> > >> M: Nishanth Menon 
> > >> M: Robert Nelson 
> > >> M: Tom Rini 
> > >> S: Maintained
> > >> N: beagleplay
> > >>
> > >> In most cases the MAINTAINERS file tells us about each board and until
> > >> [1] I had assumed that was the case. With that patch reverted, these
> > >> are the only 'orphaned' MAINTAINERS entries (buildman
> > >> --maintainer-check):
> > >>
> > >> WARNING: orphaned defconfig in board/armltd/vexpress64/MAINTAINERS
> > >> ending at line 8
> > >> WARNING: orphaned defconfig in board/google/veyron/MAINTAINERS ending at 
> > >> line 44
> > >> WARNING: orphaned defconfig in
> > >> board/mikrotik/crs3xx-98dx3236/MAINTAINERS ending at line 7
> > >> WARNING: orphaned defconfig in board/st/common/MAINTAINERS ending at 
> > >> line 6
> > >> WARNING: orphaned defconfig in board/ti/am62x/MAINTAINERS ending at line 
> > >> 15
> > >>
> > >> But Tom advised me that MAINTAINERS is not intended for this purpose,
> > >> so the check has been dropped. I am not suggesting we bring it back,
> > >> necessarily, but if we did, we could use it to specify the .config and
> > >> defconfig files which are used by each board.
> > >
> > >Note that the "N" syntax has been in use, for defconfigs for a lot
> > >longer, and is why the CI check for unmaintained / orphaned boards
> > >stopped working.
> > >
> > >> *Failing that*, I suggest a new 'name.brd' file in the board directory
> > >> to contain this information, e.g.
> > >>
> > >> # Contents of board/ti/am62x/beagleplay.brd:
> > >> beagleplay-r5: am62x_evm_r5_defconfig beagleplay_r5.config
> > >> beagleplay-a53: am62x_evm_a53_defconfig beagleplay_a53.config
> > >>
> > >> This could be parsed by buildman and other tools, to produce a list of
> > >> available boards and to build each using a single name (e.g. make
> > >> beagleplay-r5_defconfig)
> > >>
> > >> What do people think?
> > >
> > >I'm not sure I like this direction honestly.  Do we want CI to cover
> > >these configurations?  Maybe.  But it's just for CI.  Using the TI
> > >examples, I don't know that you can use buildman today to generate a
> > >functional binary (and if you can, if --keep will retain the right
> > >files).
> > >
> > >Maybe we just need to make it clear that if you use config fragments
> > >then you're opting out of CI for that specific platform.  That should be
> > >fine for example for the TI ones that will be built in OE a bunch
> > >anyhow so accidental failure to builds will be caught.  I don't know
> > >about the Tegra platforms.
> > >
> >
> > Greetings! Are there any progress regards fragments move or this will
> > hang in mailing list indefinitely?
>
> I don't follow, sorry.  Yes, please send a patch on top of next to move
> the fragments for Tegra platforms to match how they're done for TI
> platforms now.
>
> --
> Tom

Oh, so it was merged already into next, sorry did not check the next branch.
Tegra fragments move patch was already sent a month ago alongside
other board improvements, waiting for Thierry now. Thanks!

Re: Config fragments

[Tom Rini]

On Mon, Sep 25, 2023 at 04:18:45PM +0300, Svyatoslav Ryhel wrote:
> 
> 
> 23 серпня 2023 р. 18:55:58 GMT+03:00, Tom Rini  
> написав(-ла):
> >On Wed, Aug 23, 2023 at 09:30:31AM -0600, Simon Glass wrote:
> >
> >> Hi,
> >> 
> >> Up until 2023.04 it has been possible to build all the defconfigs but
> >> with 2023.07 that changed. Tom mentioned this to me recently.
> >
> >I'm adding Svyatoslav who is the person that brought in all of the
> >config fragments that are going to be in v2023.07.
> >
> >> Up until 2023.04 we can enumerate all the board configs that can be
> >> built. We can build any of them using a single name and a single
> >> defconfig. The boards.cfg file which buildman creates contains a full
> >> list of things that can be built.
> >> 
> >> From 2023.07 this changes and we now have random .config files
> >> sprinkled about the place. I say random because they are not connected
> >> to anything. For example, here is
> >
> >They aren't random.  They're, just for v2023.07, in configs/ and then
> >moving forward, they're in the board directory.  I would like to see
> >them in more places possibly, but that's not something I'm sure enough
> >on right now.
> >
> >> board/ti/am62x/MAINTAINERS:
> >>
> >> AM62x BOARD
> >> M: Dave Gerlach 
> >> M: Tom Rini 
> >> S: Maintained
> >> F: board/ti/am62x/
> >> F: include/configs/am62x_evm.h
> >> F: configs/am62x_evm_r5_defconfig
> >> F: configs/am62x_evm_a53_defconfig
> >> 
> >> BEAGLEPLAY BOARD
> >> M: Nishanth Menon 
> >> M: Robert Nelson 
> >> M: Tom Rini 
> >> S: Maintained
> >> N: beagleplay
> >> 
> >> In most cases the MAINTAINERS file tells us about each board and until
> >> [1] I had assumed that was the case. With that patch reverted, these
> >> are the only 'orphaned' MAINTAINERS entries (buildman
> >> --maintainer-check):
> >> 
> >> WARNING: orphaned defconfig in board/armltd/vexpress64/MAINTAINERS
> >> ending at line 8
> >> WARNING: orphaned defconfig in board/google/veyron/MAINTAINERS ending at 
> >> line 44
> >> WARNING: orphaned defconfig in
> >> board/mikrotik/crs3xx-98dx3236/MAINTAINERS ending at line 7
> >> WARNING: orphaned defconfig in board/st/common/MAINTAINERS ending at line 6
> >> WARNING: orphaned defconfig in board/ti/am62x/MAINTAINERS ending at line 15
> >> 
> >> But Tom advised me that MAINTAINERS is not intended for this purpose,
> >> so the check has been dropped. I am not suggesting we bring it back,
> >> necessarily, but if we did, we could use it to specify the .config and
> >> defconfig files which are used by each board.
> >
> >Note that the "N" syntax has been in use, for defconfigs for a lot
> >longer, and is why the CI check for unmaintained / orphaned boards
> >stopped working.
> >
> >> *Failing that*, I suggest a new 'name.brd' file in the board directory
> >> to contain this information, e.g.
> >> 
> >> # Contents of board/ti/am62x/beagleplay.brd:
> >> beagleplay-r5: am62x_evm_r5_defconfig beagleplay_r5.config
> >> beagleplay-a53: am62x_evm_a53_defconfig beagleplay_a53.config
> >> 
> >> This could be parsed by buildman and other tools, to produce a list of
> >> available boards and to build each using a single name (e.g. make
> >> beagleplay-r5_defconfig)
> >> 
> >> What do people think?
> >
> >I'm not sure I like this direction honestly.  Do we want CI to cover
> >these configurations?  Maybe.  But it's just for CI.  Using the TI
> >examples, I don't know that you can use buildman today to generate a
> >functional binary (and if you can, if --keep will retain the right
> >files).
> >
> >Maybe we just need to make it clear that if you use config fragments
> >then you're opting out of CI for that specific platform.  That should be
> >fine for example for the TI ones that will be built in OE a bunch
> >anyhow so accidental failure to builds will be caught.  I don't know
> >about the Tegra platforms.
> >
> 
> Greetings! Are there any progress regards fragments move or this will
> hang in mailing list indefinitely?

I don't follow, sorry.  Yes, please send a patch on top of next to move
the fragments for Tegra platforms to match how they're done for TI
platforms now.

-- 
Tom


signature.asc
Description: PGP signature

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

[Simon Glass]

Hi Rob,

On Thu, 21 Sept 2023 at 10:45, Simon Glass  wrote:
>
> Hi Rob,
>
> On Thu, 21 Sept 2023 at 09:20, Rob Herring  wrote:
> >
> > On Thu, Sep 21, 2023 at 9:38 AM Simon Glass  wrote:
> > >
> > > Hi Rob,
> > >
> > > On Thu, 21 Sept 2023 at 08:25, Rob Herring  wrote:
> > > >
> > > > On Thu, Sep 7, 2023 at 4:40 PM Simon Glass  wrote:
> > > > >
> > > > > It is common to split firmware into 'Platform Init', which does the
> > > > > initial hardware setup and a "Payload" which selects the OS to be 
> > > > > booted.
> > > > > Thus an handover interface is required between these two pieces.
> > > > >
> > > > > This aims to provide an small schema addition for the memory mapping
> > > > > needed to keep these two pieces working together well.
> > > > >
> > > > > Signed-off-by: Simon Glass 
> > > > > ---
> > > > >
> > > > > Changes in v6:
> > > > > - Drop mention of UEFI
> > > > > - Use compatible strings instead of node names
> > > > >
> > > > > Changes in v5:
> > > > > - Drop the memory-map node (should have done that in v4)
> > > > > - Tidy up schema a bit
> > > > >
> > > > > Changes in v4:
> > > > > - Make use of the reserved-memory node instead of creating a new one
> > > > >
> > > > > Changes in v3:
> > > > > - Reword commit message again
> > > > > - cc a lot more people, from the FFI patch
> > > > > - Split out the attributes into the /memory nodes
> > > > >
> > > > > Changes in v2:
> > > > > - Reword commit message
> > > > >
> > > > >  .../reserved-memory/common-reserved.yaml  | 71 
> > > > > +++
> > > > >  1 file changed, 71 insertions(+)
> > > > >  create mode 100644 
> > > > > dtschema/schemas/reserved-memory/common-reserved.yaml
> > > > >
> > > > > diff --git a/dtschema/schemas/reserved-memory/common-reserved.yaml 
> > > > > b/dtschema/schemas/reserved-memory/common-reserved.yaml
> > > > > new file mode 100644
> > > > > index 000..4889f59
> > > > > --- /dev/null
> > > > > +++ b/dtschema/schemas/reserved-memory/common-reserved.yaml
> > > > > @@ -0,0 +1,71 @@
> > > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > > > > +%YAML 1.2
> > > > > +---
> > > > > +$id: 
> > > > > http://devicetree.org/schemas/reserved-memory/common-reserved.yaml#
> > > > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > > > +
> > > > > +title: Common memory reservations
> > > > > +
> > > > > +description: |
> > > > > +  Specifies that the reserved memory region can be used for the 
> > > > > purpose
> > > > > +  indicated by its compatible string.
> > > > > +
> > > > > +  Clients may reuse this reserved memory if they understand what it 
> > > > > is for,
> > > > > +  subject to the notes below.
> > > > > +
> > > > > +maintainers:
> > > > > +  - Simon Glass 
> > > > > +
> > > > > +allOf:
> > > > > +  - $ref: reserved-memory.yaml
> > > > > +
> > > > > +properties:
> > > > > +  compatible:
> > > > > +description: |
> > > > > +  This describes some common memory reservations:
> > > > > +
> > > > > + acpi-reclaim: Contains ACPI tables; memory may be reclaimed 
> > > > > when the
> > > > > +   tables are no-longer needed
> > > >
> > > > I think you are mixing 2 things with the name here. What the memory
> > > > contains and what to do with it. You don't need the latter. The
> > > > consumer of the region will know what to do with it if anything based
> > > > on knowing what is in the region. For example, The DTB passed to the
> > > > OS is typically in a reserved region (probably still /mem-reserve/
> > > > though). The DTB may remain there forever or the OS could copy it
> > > > somewhere else and free the reserved region. The Linux kernel does
> > > > both depending on the arch. (Of course there is no "dtb" compatible
> > > > because we have to pass the location of the dtb to even find the
> > > > reserved regions in the first place.)
> > > >
> > > > So the question here is whether just "acpi" (or "acpi-tables"?) would
> > > > be explicit enough?
> > >
> > > Yes acpi-tables would be enough.
> > >
> > > >
> > > > > + acpi-nvs: Contains ACPI Non-volatile-storage data; memory 
> > > > > may be
> > > > > +   reclaimed when the tables are no-longer needed
> > > >
> > > > No need to say anything about reclaiming.
> > >
> > > OK...so what about all that discussion about being able to reclaim the
> > > memory if you know what it is for? Where should that be written? Or is
> > > it somewhere else already?
> >
> > Reclaiming is a policy of the consumer of the data. It belongs in the
> > documentation of the consumer if you are going to document it.
> >
> > The global policy is you can't use reserved regions and you can't
> > reclaim them if you don't know what they are. If you want to add
> > something to that effect in reserved-memory.yaml or the spec, that's
> > fine, but that's not a per compatible thing.
>
> OK I will do that.
>
> >
> > > > I know some ACPIisms (e.g. DSDT), but I don't know what NVS or
> > > > "Non-volatile-storage data" is 

Re: [PATCH v3 01/38] spl: Use CONFIG_SPL... instead of CONFIG_..._SPL_...

[Felix Brack]

Hi Simon,

On 24.09.23 21:24, Simon Glass wrote:
> We like to put the SPL first so it is clear that it relates to SPL. Rename
> various malloc-related options which have crept in, to stick to this
> convention.
> 
> Signed-off-by: Simon Glass 
> Reviewed-by: Marcel Ziswiler 
> Reviewed-by: Martyn Welch 
> Reviewed-by: Svyatoslav Ryhel 
> ---
> 
> (no changes since v1)
> .

I just tried to apply the patch to master for testing and got the
following result:

error: patch failed: configs/am335x_baltos_defconfig:18
error: configs/am335x_baltos_defconfig: patch does not apply
error: patch failed: configs/am335x_igep003x_defconfig:20
error: configs/am335x_igep003x_defconfig: patch does not apply
error: patch failed: configs/dh_imx6_defconfig:39
error: configs/dh_imx6_defconfig: patch does not apply
error: patch failed: configs/starfive_visionfive2_defconfig:47
error: configs/starfive_visionfive2_defconfig: patch does not apply

For what I want to test (changes for PDU001 board) this doesn't matter
however. I'm just not sure: should I go ahead with testing or wait for
an updated patch?

regards Felix

Re: [PATCH] dt-bindings: mtd: Add a schema for binman

[Simon Glass]

Hi Miquel,

On Mon, 25 Sept 2023 at 08:47, Miquel Raynal  wrote:
>
> Hi Simon,
>
> s...@chromium.org wrote on Mon, 25 Sep 2023 06:33:14 -0600:
>
> > Hi Miquel,
> >
> > On Mon, 25 Sept 2023 at 01:21, Miquel Raynal  
> > wrote:
> > >
> > > Hi Simon,
> > >
> > > s...@chromium.org wrote on Fri, 22 Sep 2023 13:51:14 -0600:
> > >
> > > > Hi Rob,
> > > >
> > > > On Fri, 22 Sept 2023 at 13:43, Rob Herring  wrote:
> > > > >
> > > > > On Fri, Sep 22, 2023 at 1:12 PM Simon Glass  wrote:
> > > > > >
> > > > > > Hi Rob,
> > > > > >
> > > > > > On Fri, 22 Sept 2023 at 11:46, Rob Herring  wrote:
> > > > > > >
> > > > > > > On Fri, Sep 22, 2023 at 11:01:18AM -0600, Simon Glass wrote:
> > > > > > > > Hi Rob,
> > > > > > > >
> > > > > > > > On Fri, 22 Sept 2023 at 10:00, Rob Herring  
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > On Thu, Sep 21, 2023 at 1:45 PM Simon Glass 
> > > > > > > > >  wrote:
> > > > > > > > > >
> > > > > > > > > > Binman[1] is a tool for creating firmware images. It allows 
> > > > > > > > > > you to
> > > > > > > > > > combine various binaries and place them in an output file.
> > > > > > > > > >
> > > > > > > > > > Binman uses a DT schema to describe an image, in enough 
> > > > > > > > > > detail that
> > > > > > > > > > it can be automatically built from component parts, 
> > > > > > > > > > disassembled,
> > > > > > > > > > replaced, listed, etc.
> > > > > > > > > >
> > > > > > > > > > Images are typically stored in flash, which is why this 
> > > > > > > > > > binding is
> > > > > > > > > > targeted at mtd. Previous discussion is at [2] [3].
> > > > > > > > > >
> > > > > > > > > > [1] 
> > > > > > > > > > https://u-boot.readthedocs.io/en/stable/develop/package/binman.html
> > > > > > > > > > [2] 
> > > > > > > > > > https://lore.kernel.org/u-boot/20230821180220.2724080-3-...@chromium.org/
> > > > > > > > > > [3] https://www.spinics.net/lists/devicetree/msg626149.html
> > > > > > > > >
> > > > > > > > > You missed:
> > > > > > > > >
> > > > > > > > > https://github.com/devicetree-org/dt-schema/pull/110
> > > > > > > > >
> > > > > > > > > where I said: We certainly shouldn't duplicate the existing 
> > > > > > > > > partitions
> > > > > > > > > bindings. What's missing from them (I assume we're mostly 
> > > > > > > > > talking
> > > > > > > > > about "fixed-partitions" which has been around forever I 
> > > > > > > > > think (before
> > > > > > > > > me))?
> > > > > > > > >
> > > > > > > > > To repeat, unless there is some reason binman partitions 
> > > > > > > > > conflict with
> > > > > > > > > fixed-partitions, you need to start there and extend it. 
> > > > > > > > > >From what's
> > > > > > > > > posted here, it neither conflicts nor needs extending.
> > > > > > > >
> > > > > > > > I think at this point I am just hopelessly confused. Have you 
> > > > > > > > taken a
> > > > > > > > look at the binman schema? [1]
> > > > > > >
> > > > > > > Why do I need to? That's used for some tool and has nothing to do 
> > > > > > > with a
> > > > > > > device's DTB. However, I thought somewhere in this discussion you 
> > > > > > > showed
> > > > > > > it under a flash device node.
> > > > > >
> > > > > > Yes, that is the intent (under a flash node).
> > > > > >
> > > > > > > Then I care because then it overlaps with
> > > > > > > what we already have for partitions. If I misunderstood that, 
> > > > > > > then just
> > > > > > > put your schema with your tool. Only users of the tool should 
> > > > > > > care about
> > > > > > > the tool's schema.
> > > > > >
> > > > > > OK. I believe that binman will fit into both camps, since its input 
> > > > > > is
> > > > > > not necessarily fully formed. E.g. if you don't specify the offset 
> > > > > > of
> > > > > > an entry, then it will be packed automatically. But the output is
> > > > > > fully formed, in that Binman now knows the offset so can write it to
> > > > > > the DT.
> > > > >
> > > > > I suppose it could take its own format as input and then write out
> > > > > something different for the "on the device" format (i.e.
> > > > > fixed-partitions). At least for the dynamic offsets, we may need
> > > > > something allowed for binman input, but not allowed on device. In
> > > > > general, there is support for partitions without addresses/offsets,
> > > > > but only for partitions that have some other way to figure that out
> > > > > (on disk partition info).
> > > > >
> > > > > There's also the image filename which doesn't really belong in the on
> > > > > device partitions. So maybe the input and output schemas should be
> > > > > separate.
> > > >
> > > > OK, I'll focus on the output schema for now. I suspect this will be a
> > > > grey area though.
> > > >
> > > > As an example, if you replace a binary in the firmware, Binman can
> > > > repack the firmware to make room, respecting the alignment and size
> > > > constraints. So these need to be in the output schema somehow.
> > > >
> > > > >
> > > > > > > > I saw this file, which 

Re: [PATCH] dt-bindings: mtd: Add a schema for binman

[Miquel Raynal]

Hi Simon,

s...@chromium.org wrote on Mon, 25 Sep 2023 06:33:14 -0600:

> Hi Miquel,
> 
> On Mon, 25 Sept 2023 at 01:21, Miquel Raynal  
> wrote:
> >
> > Hi Simon,
> >
> > s...@chromium.org wrote on Fri, 22 Sep 2023 13:51:14 -0600:
> >  
> > > Hi Rob,
> > >
> > > On Fri, 22 Sept 2023 at 13:43, Rob Herring  wrote:  
> > > >
> > > > On Fri, Sep 22, 2023 at 1:12 PM Simon Glass  wrote:  
> > > > >
> > > > > Hi Rob,
> > > > >
> > > > > On Fri, 22 Sept 2023 at 11:46, Rob Herring  wrote:  
> > > > > >
> > > > > > On Fri, Sep 22, 2023 at 11:01:18AM -0600, Simon Glass wrote:  
> > > > > > > Hi Rob,
> > > > > > >
> > > > > > > On Fri, 22 Sept 2023 at 10:00, Rob Herring  
> > > > > > > wrote:  
> > > > > > > >
> > > > > > > > On Thu, Sep 21, 2023 at 1:45 PM Simon Glass  
> > > > > > > > wrote:  
> > > > > > > > >
> > > > > > > > > Binman[1] is a tool for creating firmware images. It allows 
> > > > > > > > > you to
> > > > > > > > > combine various binaries and place them in an output file.
> > > > > > > > >
> > > > > > > > > Binman uses a DT schema to describe an image, in enough 
> > > > > > > > > detail that
> > > > > > > > > it can be automatically built from component parts, 
> > > > > > > > > disassembled,
> > > > > > > > > replaced, listed, etc.
> > > > > > > > >
> > > > > > > > > Images are typically stored in flash, which is why this 
> > > > > > > > > binding is
> > > > > > > > > targeted at mtd. Previous discussion is at [2] [3].
> > > > > > > > >
> > > > > > > > > [1] 
> > > > > > > > > https://u-boot.readthedocs.io/en/stable/develop/package/binman.html
> > > > > > > > > [2] 
> > > > > > > > > https://lore.kernel.org/u-boot/20230821180220.2724080-3-...@chromium.org/
> > > > > > > > > [3] https://www.spinics.net/lists/devicetree/msg626149.html  
> > > > > > > >
> > > > > > > > You missed:
> > > > > > > >
> > > > > > > > https://github.com/devicetree-org/dt-schema/pull/110
> > > > > > > >
> > > > > > > > where I said: We certainly shouldn't duplicate the existing 
> > > > > > > > partitions
> > > > > > > > bindings. What's missing from them (I assume we're mostly 
> > > > > > > > talking
> > > > > > > > about "fixed-partitions" which has been around forever I think 
> > > > > > > > (before
> > > > > > > > me))?
> > > > > > > >
> > > > > > > > To repeat, unless there is some reason binman partitions 
> > > > > > > > conflict with
> > > > > > > > fixed-partitions, you need to start there and extend it. From 
> > > > > > > > what's
> > > > > > > > posted here, it neither conflicts nor needs extending.  
> > > > > > >
> > > > > > > I think at this point I am just hopelessly confused. Have you 
> > > > > > > taken a
> > > > > > > look at the binman schema? [1]  
> > > > > >
> > > > > > Why do I need to? That's used for some tool and has nothing to do 
> > > > > > with a
> > > > > > device's DTB. However, I thought somewhere in this discussion you 
> > > > > > showed
> > > > > > it under a flash device node.  
> > > > >
> > > > > Yes, that is the intent (under a flash node).
> > > > >  
> > > > > > Then I care because then it overlaps with
> > > > > > what we already have for partitions. If I misunderstood that, then 
> > > > > > just
> > > > > > put your schema with your tool. Only users of the tool should care 
> > > > > > about
> > > > > > the tool's schema.  
> > > > >
> > > > > OK. I believe that binman will fit into both camps, since its input is
> > > > > not necessarily fully formed. E.g. if you don't specify the offset of
> > > > > an entry, then it will be packed automatically. But the output is
> > > > > fully formed, in that Binman now knows the offset so can write it to
> > > > > the DT.  
> > > >
> > > > I suppose it could take its own format as input and then write out
> > > > something different for the "on the device" format (i.e.
> > > > fixed-partitions). At least for the dynamic offsets, we may need
> > > > something allowed for binman input, but not allowed on device. In
> > > > general, there is support for partitions without addresses/offsets,
> > > > but only for partitions that have some other way to figure that out
> > > > (on disk partition info).
> > > >
> > > > There's also the image filename which doesn't really belong in the on
> > > > device partitions. So maybe the input and output schemas should be
> > > > separate.  
> > >
> > > OK, I'll focus on the output schema for now. I suspect this will be a
> > > grey area though.
> > >
> > > As an example, if you replace a binary in the firmware, Binman can
> > > repack the firmware to make room, respecting the alignment and size
> > > constraints. So these need to be in the output schema somehow.
> > >  
> > > >  
> > > > > > > I saw this file, which seems to extend a partition.
> > > > > > >
> > > > > > > Documentation/devicetree/bindings/mtd/partitions/brcm,bcm4908-partitions.yaml
> > > > > > >   
> > > > > >
> > > > > > IIRC, that's a different type where partition locations are stored 
> > > > > > in
> > > > > > the flash, so we 

Re: [PATCH v2 3/3] trace: Fix alignment logic in flyrecord header

[Michal Simek]

On 9/25/23 16:28, Tom Rini wrote:

On Mon, Sep 25, 2023 at 08:01:41AM -0600, Simon Glass wrote:

Hi Michal,

On Mon, 25 Sept 2023 at 07:38, Michal Simek  wrote:




On 9/25/23 15:10, Simon Glass wrote:

Hi Michal,

On Mon, 25 Sept 2023 at 00:06, Michal Simek  wrote:


Hi Simon,


On 9/23/23 20:13, Simon Glass wrote:

Current alignment which is using 16 bytes is not correct in connection to
trace_clocks description and it's length.
That's why use start_addr variable and record proper size based on used
entries.

Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format").
Signed-off-by: Michal Simek 
Reviewed-by: Simon Glass 
---

Changes in v2:
- s/start_addr/start_ofs/g'

tools/proftool.c | 31 +--
1 file changed, 29 insertions(+), 2 deletions(-)

Applied to u-boot-dm, thanks!


FYI: I have merged it to my tree and already sent pull request to Tom.
Without it I couldn't pass CI loop to get all reviewed features in.

https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c...@amd.com/


Ah OK, well that's fine. It was in my patchwork queue still, which
suggests that the patches were not set to 'applied'?


I am not using patchwork. But I expect my reply to cover letter was recorded 
there.


Probably. If you reply to each patch, it shows up in the patch, but
the cover letter is hidden somewhere else.


Patchwork doesn't, but b4 does, handle tags sent to the cover letter
rather than individually.


For tags yes it works nicely with b4. Pretty much just reply to cover letter and 
b4 copy it to all patches.

But this was more about that you can't see my Applied reply there.
I don't think that b4 will be able to catch it but I would love to be wrong.

M

Re: [PATCH 0/3] common: Remove a few more header files

[Simon Glass]

Hi Tom,

On Mon, 25 Sept 2023 at 08:08, Tom Rini  wrote:
>
> On Sun, Sep 24, 2023 at 08:39:49PM -0600, Simon Glass wrote:
>
> > This series removes two more header files from the common.h header. It
> > also tidies up code style for time.h functions, to simplify similar
> > maintenance in future.
> >
> >
> > Simon Glass (3):
> >   Fix code style for time functions
> >   common: Drop time.h from common header
> >   common: Drop linux/string.h from common header
> [snip]
> >  1709 files changed, 1854 insertions(+), 34 deletions(-)
>
> This is where I start to get worried we're possibly taking the wrong
> approach to the problem.  Perhaps it would be better to:
> 1. Start finding headers which include , remove and fixup the
> breakage directly.
> 2. Move on to removing  from files, a directory at a time.
>

The bigger question is whether you want common.h gone or not?

I shared my script with you so you can see what it is doing. But if a
board uses memcmp(), for example, then it needs linux/string.h
included. We can discuss why so many files use string functions, or
whether the script is not perfect, but ultimately that is the
question.

Just these simple queries give you a sense of the scale:

 git grep -l strcat |wc
 45  451134
git grep -l memcpy |wc
834 834   21643

If you are suggesting that we add fewer includes for now, relying on
transitive includes in header files, then I don't think that is very
useful in the end. I had a series a few years back which removed
common.h entirely and it foundered on this same issue.

Regards,
Simon

Re: [PATCH v2 3/3] trace: Fix alignment logic in flyrecord header

[Tom Rini]

On Mon, Sep 25, 2023 at 04:21:17PM +0200, Michal Simek wrote:
> 
> 
> On 9/25/23 16:19, Tom Rini wrote:
> > On Mon, Sep 25, 2023 at 04:10:38PM +0200, Michal Simek wrote:
> > > Hi Simon,
> > > 
> > > On 9/25/23 16:01, Simon Glass wrote:
> > > > Hi Michal,
> > > > 
> > > > On Mon, 25 Sept 2023 at 07:38, Michal Simek  
> > > > wrote:
> > > > > 
> > > > > 
> > > > > 
> > > > > On 9/25/23 15:10, Simon Glass wrote:
> > > > > > Hi Michal,
> > > > > > 
> > > > > > On Mon, 25 Sept 2023 at 00:06, Michal Simek  
> > > > > > wrote:
> > > > > > > 
> > > > > > > Hi Simon,
> > > > > > > 
> > > > > > > 
> > > > > > > On 9/23/23 20:13, Simon Glass wrote:
> > > > > > > > Current alignment which is using 16 bytes is not correct in 
> > > > > > > > connection to
> > > > > > > > trace_clocks description and it's length.
> > > > > > > > That's why use start_addr variable and record proper size based 
> > > > > > > > on used
> > > > > > > > entries.
> > > > > > > > 
> > > > > > > > Fixes: be16fc81b2ed ("trace: Update proftool to use new binary 
> > > > > > > > format").
> > > > > > > > Signed-off-by: Michal Simek 
> > > > > > > > Reviewed-by: Simon Glass 
> > > > > > > > ---
> > > > > > > > 
> > > > > > > > Changes in v2:
> > > > > > > > - s/start_addr/start_ofs/g'
> > > > > > > > 
> > > > > > > >  tools/proftool.c | 31 +--
> > > > > > > >  1 file changed, 29 insertions(+), 2 deletions(-)
> > > > > > > > 
> > > > > > > > Applied to u-boot-dm, thanks!
> > > > > > > 
> > > > > > > FYI: I have merged it to my tree and already sent pull request to 
> > > > > > > Tom.
> > > > > > > Without it I couldn't pass CI loop to get all reviewed features 
> > > > > > > in.
> > > > > > > 
> > > > > > > https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c...@amd.com/
> > > > > > 
> > > > > > Ah OK, well that's fine. It was in my patchwork queue still, which
> > > > > > suggests that the patches were not set to 'applied'?
> > > > > 
> > > > > I am not using patchwork. But I expect my reply to cover letter was 
> > > > > recorded there.
> > > > 
> > > > Probably. If you reply to each patch, it shows up in the patch, but
> > > > the cover letter is hidden somewhere else.
> > > 
> > > I have never started to like patchwork. I installed that client long time
> > > ago, I also have account for quite a long time.
> > > 
> > > > If you are not using patchwork, how come you are a custodian? Is
> > > > someone else dealing with patchwork for you?
> > > 
> > > Not really. I am just keep track on it via emails.
> > > 
> > > DT folks did wire CI loop on every patch which they get. I am not aware
> > > about any feature like this which would bring me something. That's why I 
> > > am
> > > considering patchwork as unneeded layer. And I also don't think that I 
> > > have
> > > read anywhere that all custodians should be using patchwork.
> > 
> > Right, patchwork isn't required, but can be helpful.  Part of how
> > patchwork is maintained for everyone (in U-Boot) is that I have a script
> > that will update the status of patches to accepted and add the githash,
> > based on the "patchwork hash" of a given commit.  There's a number of
> > automated tooling things that other projects use which could be helpful
> > here, but due to lack of time/resources, we haven't tried them here.
> 
> Can you share that script? I am happy to run it and pretty much close my list.
> I am using b4 for applying patches that's why all message-ids are listed in
> the history which will uniquely identify that patches.

If you like, yes, you can run the following.  Note that when I run it
myself between tags, it will still re-update things.  This requires
having patchwork cloned from git as well and is a slight modification of
both tools/patchwork-update-commits and tools/post-receive.hook:

#!/bin/bash

# Patchwork - automated patch tracking system
# Copyright (C) 2010 martin f. krafft 
#
# SPDX-License-Identifier: GPL-2.0-or-later

# Git post-receive hook to update Patchwork patches after Git pushes
set -u

PW_DIR=/home/trini/work/u-boot/patchwork/patchwork

#TODO: the state map should really live in the repo's git-config
STATE_MAP=".git/refs/heads/master:Accepted"

# ignore all commits already present in these refs
# e.g.,
#   EXCLUDE="refs/heads/upstream refs/heads/other-project"
EXCLUDE=""

do_exit=0
trap "do_exit=1" INT

get_patchwork_hash() {
local hash
hash=$(git diff "$1~..$1" | python3 $PW_DIR/hasher.py)
echo "$hash"
test -n "$hash"
}

get_patchwork_hash_harder() {
local hash
hash=$(git diff "$1~..$1" | sed -e 's/^ $//g' | python3 $PW_DIR/hasher.py)
echo "$hash"
test -n "$hash"
}

get_patch_id() {
local id
id=$(curl -s 
"http://patchwork.ozlabs.org/api/patches/?project=uboot=$1; | \
 jq '.[-1] | .id')
echo "$id"
}

set_patch_state() {
pwclient update -s "$2" -c "$3" "$1" 2>&1
}

update_patches() {
local cnt; cnt=0
for rev in $(git rev-parse --not ${EXCLUDE} |

Re: [PATCH v2 3/3] trace: Fix alignment logic in flyrecord header

[Tom Rini]

On Mon, Sep 25, 2023 at 08:01:41AM -0600, Simon Glass wrote:
> Hi Michal,
> 
> On Mon, 25 Sept 2023 at 07:38, Michal Simek  wrote:
> >
> >
> >
> > On 9/25/23 15:10, Simon Glass wrote:
> > > Hi Michal,
> > >
> > > On Mon, 25 Sept 2023 at 00:06, Michal Simek  wrote:
> > >>
> > >> Hi Simon,
> > >>
> > >>
> > >> On 9/23/23 20:13, Simon Glass wrote:
> > >>> Current alignment which is using 16 bytes is not correct in connection 
> > >>> to
> > >>> trace_clocks description and it's length.
> > >>> That's why use start_addr variable and record proper size based on used
> > >>> entries.
> > >>>
> > >>> Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format").
> > >>> Signed-off-by: Michal Simek 
> > >>> Reviewed-by: Simon Glass 
> > >>> ---
> > >>>
> > >>> Changes in v2:
> > >>> - s/start_addr/start_ofs/g'
> > >>>
> > >>>tools/proftool.c | 31 +--
> > >>>1 file changed, 29 insertions(+), 2 deletions(-)
> > >>>
> > >>> Applied to u-boot-dm, thanks!
> > >>
> > >> FYI: I have merged it to my tree and already sent pull request to Tom.
> > >> Without it I couldn't pass CI loop to get all reviewed features in.
> > >>
> > >> https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c...@amd.com/
> > >
> > > Ah OK, well that's fine. It was in my patchwork queue still, which
> > > suggests that the patches were not set to 'applied'?
> >
> > I am not using patchwork. But I expect my reply to cover letter was 
> > recorded there.
> 
> Probably. If you reply to each patch, it shows up in the patch, but
> the cover letter is hidden somewhere else.

Patchwork doesn't, but b4 does, handle tags sent to the cover letter
rather than individually.  Patchwork does have a link to the cover
letter, on individual patch links under the "expand" link on the
"Series" line.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH] bmips: Add Inteno XG6846 board

[Tom Rini]

On Mon, Sep 25, 2023 at 03:56:07PM +0200, Linus Walleij wrote:
> On Thu, Sep 21, 2023 at 7:25 PM Tom Rini  wrote:
> > On Thu, Sep 21, 2023 at 04:00:24PM +0200, Daniel Schwierzeck wrote:
> >
> > [snip]
> > > I just tested it, you can simply add an empty 
> > > board/inteno/xg6846/Makefile and
> > > remove board/inteno/xg6846/xg6846.c. But you can also remove the Makefile.
> > > Just the Kconfig and MAINTAINERS file are needed.
> >
> > An empty Makefile is OK, but no Makefile requires a bit more as we first
> > try and include it unconditionally and then try and link in built-in.o.
> > I'm poking at this now.
> 
> Did you get anywhere with this? If it's on the master I'll just rebase and
> be done with it, else I can go with an empty Makefile simply.

Please rebase on top of:
https://patchwork.ozlabs.org/project/uboot/list/?series=374302

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v2 3/3] trace: Fix alignment logic in flyrecord header

[Michal Simek]

On 9/25/23 16:19, Tom Rini wrote:

On Mon, Sep 25, 2023 at 04:10:38PM +0200, Michal Simek wrote:

Hi Simon,

On 9/25/23 16:01, Simon Glass wrote:

Hi Michal,

On Mon, 25 Sept 2023 at 07:38, Michal Simek  wrote:




On 9/25/23 15:10, Simon Glass wrote:

Hi Michal,

On Mon, 25 Sept 2023 at 00:06, Michal Simek  wrote:


Hi Simon,


On 9/23/23 20:13, Simon Glass wrote:

Current alignment which is using 16 bytes is not correct in connection to
trace_clocks description and it's length.
That's why use start_addr variable and record proper size based on used
entries.

Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format").
Signed-off-by: Michal Simek 
Reviewed-by: Simon Glass 
---

Changes in v2:
- s/start_addr/start_ofs/g'

 tools/proftool.c | 31 +--
 1 file changed, 29 insertions(+), 2 deletions(-)

Applied to u-boot-dm, thanks!


FYI: I have merged it to my tree and already sent pull request to Tom.
Without it I couldn't pass CI loop to get all reviewed features in.

https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c...@amd.com/


Ah OK, well that's fine. It was in my patchwork queue still, which
suggests that the patches were not set to 'applied'?


I am not using patchwork. But I expect my reply to cover letter was recorded 
there.


Probably. If you reply to each patch, it shows up in the patch, but
the cover letter is hidden somewhere else.


I have never started to like patchwork. I installed that client long time
ago, I also have account for quite a long time.


If you are not using patchwork, how come you are a custodian? Is
someone else dealing with patchwork for you?


Not really. I am just keep track on it via emails.

DT folks did wire CI loop on every patch which they get. I am not aware
about any feature like this which would bring me something. That's why I am
considering patchwork as unneeded layer. And I also don't think that I have
read anywhere that all custodians should be using patchwork.


Right, patchwork isn't required, but can be helpful.  Part of how
patchwork is maintained for everyone (in U-Boot) is that I have a script
that will update the status of patches to accepted and add the githash,
based on the "patchwork hash" of a given commit.  There's a number of
automated tooling things that other projects use which could be helpful
here, but due to lack of time/resources, we haven't tried them here.


Can you share that script? I am happy to run it and pretty much close my list.
I am using b4 for applying patches that's why all message-ids are listed in the 
history which will uniquely identify that patches.


Thanks,
Michal

Re: [PATCH v2 3/3] trace: Fix alignment logic in flyrecord header

[Tom Rini]

On Mon, Sep 25, 2023 at 04:10:38PM +0200, Michal Simek wrote:
> Hi Simon,
> 
> On 9/25/23 16:01, Simon Glass wrote:
> > Hi Michal,
> > 
> > On Mon, 25 Sept 2023 at 07:38, Michal Simek  wrote:
> > > 
> > > 
> > > 
> > > On 9/25/23 15:10, Simon Glass wrote:
> > > > Hi Michal,
> > > > 
> > > > On Mon, 25 Sept 2023 at 00:06, Michal Simek  
> > > > wrote:
> > > > > 
> > > > > Hi Simon,
> > > > > 
> > > > > 
> > > > > On 9/23/23 20:13, Simon Glass wrote:
> > > > > > Current alignment which is using 16 bytes is not correct in 
> > > > > > connection to
> > > > > > trace_clocks description and it's length.
> > > > > > That's why use start_addr variable and record proper size based on 
> > > > > > used
> > > > > > entries.
> > > > > > 
> > > > > > Fixes: be16fc81b2ed ("trace: Update proftool to use new binary 
> > > > > > format").
> > > > > > Signed-off-by: Michal Simek 
> > > > > > Reviewed-by: Simon Glass 
> > > > > > ---
> > > > > > 
> > > > > > Changes in v2:
> > > > > > - s/start_addr/start_ofs/g'
> > > > > > 
> > > > > > tools/proftool.c | 31 +--
> > > > > > 1 file changed, 29 insertions(+), 2 deletions(-)
> > > > > > 
> > > > > > Applied to u-boot-dm, thanks!
> > > > > 
> > > > > FYI: I have merged it to my tree and already sent pull request to Tom.
> > > > > Without it I couldn't pass CI loop to get all reviewed features in.
> > > > > 
> > > > > https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c...@amd.com/
> > > > 
> > > > Ah OK, well that's fine. It was in my patchwork queue still, which
> > > > suggests that the patches were not set to 'applied'?
> > > 
> > > I am not using patchwork. But I expect my reply to cover letter was 
> > > recorded there.
> > 
> > Probably. If you reply to each patch, it shows up in the patch, but
> > the cover letter is hidden somewhere else.
> 
> I have never started to like patchwork. I installed that client long time
> ago, I also have account for quite a long time.
> 
> > If you are not using patchwork, how come you are a custodian? Is
> > someone else dealing with patchwork for you?
> 
> Not really. I am just keep track on it via emails.
> 
> DT folks did wire CI loop on every patch which they get. I am not aware
> about any feature like this which would bring me something. That's why I am
> considering patchwork as unneeded layer. And I also don't think that I have
> read anywhere that all custodians should be using patchwork.

Right, patchwork isn't required, but can be helpful.  Part of how
patchwork is maintained for everyone (in U-Boot) is that I have a script
that will update the status of patches to accepted and add the githash,
based on the "patchwork hash" of a given commit.  There's a number of
automated tooling things that other projects use which could be helpful
here, but due to lack of time/resources, we haven't tried them here.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v2 3/3] trace: Fix alignment logic in flyrecord header

[Michal Simek]

Hi Simon,

On 9/25/23 16:01, Simon Glass wrote:

Hi Michal,

On Mon, 25 Sept 2023 at 07:38, Michal Simek  wrote:




On 9/25/23 15:10, Simon Glass wrote:

Hi Michal,

On Mon, 25 Sept 2023 at 00:06, Michal Simek  wrote:


Hi Simon,


On 9/23/23 20:13, Simon Glass wrote:

Current alignment which is using 16 bytes is not correct in connection to
trace_clocks description and it's length.
That's why use start_addr variable and record proper size based on used
entries.

Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format").
Signed-off-by: Michal Simek 
Reviewed-by: Simon Glass 
---

Changes in v2:
- s/start_addr/start_ofs/g'

tools/proftool.c | 31 +--
1 file changed, 29 insertions(+), 2 deletions(-)

Applied to u-boot-dm, thanks!


FYI: I have merged it to my tree and already sent pull request to Tom.
Without it I couldn't pass CI loop to get all reviewed features in.

https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c...@amd.com/


Ah OK, well that's fine. It was in my patchwork queue still, which
suggests that the patches were not set to 'applied'?


I am not using patchwork. But I expect my reply to cover letter was recorded 
there.


Probably. If you reply to each patch, it shows up in the patch, but
the cover letter is hidden somewhere else.


I have never started to like patchwork. I installed that client long time ago, I 
also have account for quite a long time.



If you are not using patchwork, how come you are a custodian? Is
someone else dealing with patchwork for you?


Not really. I am just keep track on it via emails.

DT folks did wire CI loop on every patch which they get. I am not aware about 
any feature like this which would bring me something. That's why I am 
considering patchwork as unneeded layer. And I also don't think that I have read 
anywhere that all custodians should be using patchwork.


Thanks,
Michal

Re: [PATCH 0/3] common: Remove a few more header files

[Tom Rini]

On Sun, Sep 24, 2023 at 08:39:49PM -0600, Simon Glass wrote:

> This series removes two more header files from the common.h header. It
> also tidies up code style for time.h functions, to simplify similar
> maintenance in future.
> 
> 
> Simon Glass (3):
>   Fix code style for time functions
>   common: Drop time.h from common header
>   common: Drop linux/string.h from common header
[snip]
>  1709 files changed, 1854 insertions(+), 34 deletions(-)

This is where I start to get worried we're possibly taking the wrong
approach to the problem.  Perhaps it would be better to:
1. Start finding headers which include , remove and fixup the
breakage directly.
2. Move on to removing  from files, a directory at a time.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v3 1/6] arm: mach-k3: j721e: dev-data: Add mcu_timer0 ID

[Nishanth Menon]

On 16:09-20230922, Neha Malcom Francis wrote:
> U-Boot uses mcu_timer0 as the tick-timer, so add it to device list.
> 
> Signed-off-by: Neha Malcom Francis 
> Reviewed-by: Manorit Chawdhry 
Reviewed-by: Nishanth Menon 

[...]

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 
849D 1736 249D

Re: [PATCH v3 2/6] arm: mach-k3: j721e_init: Move clk_k3 probe before loading TIFS

[Nishanth Menon]

On 16:09-20230922, Neha Malcom Francis wrote:
> When setting boot media to load the TIFS binary in legacy boot flow
> (followed by J721E), get_timer() is called which eventually calls
> dm_timer_init() to grab the tick-timer, which is mcu_timer0. Since we
> need to set up the clocks before using the timer, move clk_k3 driver
> probe before k3_sysfw_loader to ensure we have all necessary clocks set
> up before.
> 
> Signed-off-by: Neha Malcom Francis 
Reviewed-by: Nishanth Menon 

> ---
>  arch/arm/mach-k3/j721e_init.c | 24 
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/mach-k3/j721e_init.c b/arch/arm/mach-k3/j721e_init.c
> index b6164575b7..b1f7e25ed0 100644
> --- a/arch/arm/mach-k3/j721e_init.c
> +++ b/arch/arm/mach-k3/j721e_init.c
> @@ -228,6 +228,18 @@ void board_init_f(ulong dummy)
>   if (!ret)
>   pinctrl_select_state(dev, "default");
>  
> + /*
> +  * Force probe of clk_k3 driver here to ensure basic default clock
> +  * configuration is always done.
> +  */
> + if (IS_ENABLED(CONFIG_SPL_CLK_K3)) {
> + ret = uclass_get_device_by_driver(UCLASS_CLK,
> +   DM_DRIVER_GET(ti_clk),
> +   );
> + if (ret)
> + panic("Failed to initialize clk-k3!\n");
> + }
> +
>   /*
>* Load, start up, and configure system controller firmware. Provide
>* the U-Boot console init function to the SYSFW post-PM configuration
> @@ -241,18 +253,6 @@ void board_init_f(ulong dummy)
>   do_dt_magic();
>  #endif
>  
> - /*
> -  * Force probe of clk_k3 driver here to ensure basic default clock
> -  * configuration is always done.
> -  */
> - if (IS_ENABLED(CONFIG_SPL_CLK_K3)) {
> - ret = uclass_get_device_by_driver(UCLASS_CLK,
> -   DM_DRIVER_GET(ti_clk),
> -   );
> - if (ret)
> - panic("Failed to initialize clk-k3!\n");
> - }
> -
>   /* Prepare console output */
>   preloader_console_init();
>  
> -- 
> 2.34.1
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 
849D 1736 249D

Re: [PATCH v3 3/6] drivers: firmware: ti_sci: Get SCI revision only if TIFS/SYSFW is up

[Nishanth Menon]

On 16:09-20230922, Neha Malcom Francis wrote:
> When setting up boot media to load the TIFS binary in legacy boot flow
> (followed by J721E), get_timer() is called which calls dm_timer_init()
> which then gets the tick-timer: mcu_timer0. mcu_timer0 uses k3_clks
> (clock controller) and k3_pds (power controller) from the dmsc node that
> forces probe of the ti_sci driver of TIFS that hasn't been loaded yet!
> Running ti_sci_cmd_get_revision from the probe leads to panic since no
> TIFS and board config binaries have been loaded yet. Resolve this by
> moving ti_sci_cmd_get_revision to ti_sci_get_handle_from_sysfw as a
> common point of invocation for both legacy and combined boot flows.
> 
> Before doing this, it is important to go through whether any sync points
> exist where revision is needed before ti_sci_get_handle_from_sysfw is
> invoked. Going through the code along with boot tests on both flows
> ensures that there are none.

Please provide bootlogs for one board of the following SoCs in the
diffstat for us to be sure.

* AM65 (sysfw gen1), J721e(tifs gen1), AM64(sysfw gen2-Sitara), J7200(tifs
  gen2-Jacinto), J721s2(tifs-dm/Jacinto), AM62x(tifs-dm/Sitara)

> 
> Signed-off-by: Neha Malcom Francis 
> ---
>  drivers/firmware/ti_sci.c | 11 ---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/firmware/ti_sci.c b/drivers/firmware/ti_sci.c
> index 72f572d824..45406e24d2 100644
> --- a/drivers/firmware/ti_sci.c
> +++ b/drivers/firmware/ti_sci.c
> @@ -2690,6 +2690,8 @@ static void ti_sci_setup_ops(struct ti_sci_info *info)
>  const
>  struct ti_sci_handle *ti_sci_get_handle_from_sysfw(struct udevice *sci_dev)
>  {
> + int ret;
> +
>   if (!sci_dev)
>   return ERR_PTR(-EINVAL);
>  
> @@ -2703,6 +2705,11 @@ struct ti_sci_handle 
> *ti_sci_get_handle_from_sysfw(struct udevice *sci_dev)
>   if (!handle)
>   return ERR_PTR(-EINVAL);
>  
> + ret = ti_sci_cmd_get_revision(handle);
> +
> + if (ret)
> + return ret;
> +
>   return handle;
>  }
>  
> @@ -2825,11 +2832,9 @@ static int ti_sci_probe(struct udevice *dev)
>   list_add_tail(>list, _sci_list);
>   ti_sci_setup_ops(info);
>  
> - ret = ti_sci_cmd_get_revision(>handle);
> -
>   INIT_LIST_HEAD(>dev_list);
>  
> - return ret;
> + return 0;
>  }
>  
>  /**
> -- 
> 2.34.1
> 

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 
849D 1736 249D

Re: [PATCH 2/3] common: Drop time.h from common header

[Tom Rini]

On Sun, Sep 24, 2023 at 08:39:51PM -0600, Simon Glass wrote:

> Move this out of the common header and include it only where needed.
> 
> Signed-off-by: Simon Glass 
[snip]
> diff --git a/cmd/cyclic.c b/cmd/cyclic.c
> index 946f1d78184d..f12edfa6b727 100644
> --- a/cmd/cyclic.c
> +++ b/cmd/cyclic.c
> @@ -1,5 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  /*
> +#include 
>   * A general-purpose cyclic execution infrastructure, to allow "small"
>   * (run-time wise) functions to be executed at a specified frequency.
>   * Things like LED blinking or watchdog triggering are examples for such

This is wrong.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH 2/3] common: Drop time.h from common header

[Tom Rini]

On Sun, Sep 24, 2023 at 08:39:51PM -0600, Simon Glass wrote:

> Move this out of the common header and include it only where needed.
> 
> Signed-off-by: Simon Glass 
[snip]
> diff --git a/arch/arm/include/asm/arch-tegra/timer.h 
> b/arch/arm/include/asm/arch-tegra/timer.h
> index 1c4decacd338..c5e9d841031a 100644
> --- a/arch/arm/include/asm/arch-tegra/timer.h
> +++ b/arch/arm/include/asm/arch-tegra/timer.h
> @@ -9,6 +9,7 @@
>  #define _TEGRA_TIMER_H
>  
>  /* returns the current monotonic timer value in microseconds */
> +#include 
>  unsigned long timer_get_us(void);
>  
>  #endif

This should be replaced with timer.h being used directly.

-- 
Tom


signature.asc
Description: PGP signature

Re: Pull request: please pull u-boot-imx-20230905

[Tom Rini]

On Sat, Sep 23, 2023 at 08:56:14PM +0200, Stefano Babic wrote:

> Hi Tom,
> 
> some fixes for the release, please pull from u-boot-imx, thanks !
> 
> The following changes since commit 4cb31a9f3560b293670de95e76c1f3cf2f9e1ca8:
> 
>   Merge branch '2023-09-22-assorted-bugfixes' (2023-09-22 18:25:37 -0400)
> 
> are available in the Git repository at:
> 
>   https://gitlab.denx.de/u-boot/custodians/u-boot-imx.git
> tags/u-boot-imx-20230923
> 
> for you to fetch changes up to 62a3c66a7c1ee733695b46542c4a9034f3a663a4:
> 
>   smegw01: Use CONFIG_SYS_LOAD_ADDR for loading fitImage (2023-09-23
> 18:45:34 +0200)
> 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: Please pull u-boot-dm

[Tom Rini]

On Sat, Sep 23, 2023 at 12:16:40PM -0600, Simon Glass wrote:

> Hi Tom,
> 
> https://source.denx.de/u-boot/custodians/u-boot-dm/-/pipelines/17830
> https://dev.azure.com/simon0972/u-boot/_build/results?buildId=48=results
> 
> 
> The following changes since commit 4cb31a9f3560b293670de95e76c1f3cf2f9e1ca8:
> 
>   Merge branch '2023-09-22-assorted-bugfixes' (2023-09-22 18:25:37 -0400)
> 
> are available in the Git repository at:
> 
>   git://git.denx.de/u-boot-dm.git tags/dm-pull-23sep23
> 
> for you to fetch changes up to e278ad9a2ea5e4a089773a8fd79a5ea0e8572316:
> 
>   trace: Fix alignment logic in flyrecord header (2023-09-23 09:00:37 -0600)
> 

Applied to u-boot/master, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v3 6/6] arm: dts: k3-j721e: Sync with v6.6-rc1

[Nishanth Menon]

On 16:09-20230922, Neha Malcom Francis wrote:
> Sync k3-j721e DTS with kernel.org v6.6-rc1.

Give a summary of changes here - for example hbmc was disabled.

> 
> Signed-off-by: Neha Malcom Francis 
> ---
>  .../k3-j721e-common-proc-board-u-boot.dtsi|  153 +--
>  arch/arm/dts/k3-j721e-common-proc-board.dts   |  513 ++---
>  arch/arm/dts/k3-j721e-main.dtsi   | 1018 +++--
>  arch/arm/dts/k3-j721e-mcu-wakeup.dtsi |  305 -
>  .../arm/dts/k3-j721e-r5-common-proc-board.dts |  325 +-
>  arch/arm/dts/k3-j721e-r5-sk.dts   |  529 +
>  arch/arm/dts/k3-j721e-sk-u-boot.dtsi  |  184 +--
>  arch/arm/dts/k3-j721e-sk.dts  |  673 +++
>  arch/arm/dts/k3-j721e-som-p0.dtsi |  217 ++--
>  arch/arm/dts/k3-j721e-thermal.dtsi|   75 ++
>  arch/arm/dts/k3-j721e.dtsi|   32 +-
>  11 files changed, 2380 insertions(+), 1644 deletions(-)
>  create mode 100644 arch/arm/dts/k3-j721e-thermal.dtsi
> 
> diff --git a/arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi 
> b/arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi
> index 540c847eb3..64908c2056 100644
> --- a/arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi
> +++ b/arch/arm/dts/k3-j721e-common-proc-board-u-boot.dtsi
> @@ -3,83 +3,42 @@
>   * Copyright (C) 2018 Texas Instruments Incorporated - http://www.ti.com/
>   */

[...]

> +_uart0_pins_default {
> + bootph-pre-ram;
> +};
> +
> +_uart0_pins_default {
> + bootph-pre-ram;
> +};

You need this only for R5, correct? then, why not move those two to R5
dts?

> +
> +_uart0_pins_default {
> + bootph-pre-ram;
>  };
>  
>  _usbss0_pins_default {
> @@ -148,19 +114,6 @@
>   bootph-pre-ram;
>  };
>  
> -_cpsw {
> - reg = <0x0 0x4600 0x0 0x20>,
> -   <0x0 0x40f00200 0x0 0x2>;
> - reg-names = "cpsw_nuss", "mac_efuse";
> - /delete-property/ ranges;
> -
> - cpsw-phy-sel@40f04040 {
> - compatible = "ti,am654-cpsw-phy-sel";
> - reg= <0x0 0x40f04040 0x0 0x4>;
> - reg-names = "gmii-sel";
> - };
> -};
> -
>  _mmc1_pins_default {
>   bootph-pre-ram;
>  };
> @@ -169,8 +122,14 @@
>   bootph-pre-ram;
>  };
>  
> +_uart0 {
> + bootph-pre-ram;
> + status = "okay";
> +};
> +
>  _i2c0 {
>   bootph-pre-ram;
> + status = "okay";
>  };
>  
>  _i2c0 {
> @@ -181,6 +140,10 @@
>   bootph-pre-ram;
>  };
>  
> +_esm {
> + bootph-pre-ram;
> +};
> +
>   {
>   bootph-pre-ram;
>  };
> @@ -194,11 +157,7 @@
>  };
>  
>   {
> - bootph-pre-ram;
> -
> - flash@0,0 {
> - bootph-pre-ram;
> - };
> + status = "disabled";
OK - but then why have the node? or _mux or the mux?
>  };
>  
>  _mux {
> @@ -244,31 +203,3 @@
>  _r5fss1 {
>   ti,cluster-mode = <0>;

I don't understand the override here.


[...]

> diff --git a/arch/arm/dts/k3-j721e-r5-common-proc-board.dts 
> b/arch/arm/dts/k3-j721e-r5-common-proc-board.dts
> index 7bb5ce775c..950264ee16 100644
> --- a/arch/arm/dts/k3-j721e-r5-common-proc-board.dts
> +++ b/arch/arm/dts/k3-j721e-r5-common-proc-board.dts
> @@ -9,19 +9,17 @@
>  #include "k3-j721e-ddr-evm-lp4-4266.dtsi"
>  #include "k3-j721e-ddr.dtsi"
>  #include "k3-j721e-common-proc-board-u-boot.dtsi"
> -#include 
>  
[...]
>  
> -_main {
> - main_esm: esm@70 {
> - compatible = "ti,j721e-esm";
> - reg = <0x0 0x70 0x0 0x1000>;
> - ti,esm-pins = <344>, <345>;
> - bootph-pre-ram;
> - };
> +_timer0 {
> + status = "okay";
> + clock-frequency = <25000>;
Document why clock-frequency

> + bootph-pre-ram;
>  };
>  
>  

[...]

>  _uart0 {
> - /delete-property/ power-domains;
> - /delete-property/ clocks;
> - /delete-property/ clock-names;
> - pinctrl-names = "default";
> - pinctrl-0 = <_uart0_pins_default>;
> - status = "okay";
>   clock-frequency = <4800>;

I have'nt seen us needing this elsewhere.

>  };
>  
> -_uart0 {
> - pinctrl-names = "default";
> - pinctrl-0 = <_uart0_pins_default>;
> - status = "okay";
> - power-domains = <_pds 146 TI_SCI_PD_SHARED>;
> -};
> -
>  _sdhci0 {
> - /delete-property/ power-domains;
> - /delete-property/ assigned-clocks;
> - /delete-property/ assigned-clock-parents;
> - clock-names = "clk_xin";
> - clocks = <_200mhz>;
> - ti,driver-strength-ohm = <50>;
> - non-removable;
> - bus-width = <8>;
> + clock-frequency = <2>;

I have'nt seen us needing this elsewhere.

>  };
>  
>  _sdhci1 {
> - /delete-property/ power-domains;
> - /delete-property/ assigned-clocks;
> - /delete-property/ assigned-clock-parents;
> - pinctrl-names = "default";
> - pinctrl-0 = <_mmc1_pins_default>;
> - clock-names = "clk_xin";
> - clocks = <_200mhz>;
> - ti,driver-strength-ohm = <50>;
> + clock-frequency = <1920>;

I have'nt seen us needing this elsewhere.

>  };
>  
>  

Re: [PATCH 1/3] Fix code style for time functions

[Tom Rini]

On Sun, Sep 24, 2023 at 08:39:50PM -0600, Simon Glass wrote:

> Fix the code style used for some time functions.
> 
> Signed-off-by: Simon Glass 

Reviewed-by: Tom Rini 

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v2 3/3] trace: Fix alignment logic in flyrecord header

[Simon Glass]

Hi Michal,

On Mon, 25 Sept 2023 at 07:38, Michal Simek  wrote:
>
>
>
> On 9/25/23 15:10, Simon Glass wrote:
> > Hi Michal,
> >
> > On Mon, 25 Sept 2023 at 00:06, Michal Simek  wrote:
> >>
> >> Hi Simon,
> >>
> >>
> >> On 9/23/23 20:13, Simon Glass wrote:
> >>> Current alignment which is using 16 bytes is not correct in connection to
> >>> trace_clocks description and it's length.
> >>> That's why use start_addr variable and record proper size based on used
> >>> entries.
> >>>
> >>> Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format").
> >>> Signed-off-by: Michal Simek 
> >>> Reviewed-by: Simon Glass 
> >>> ---
> >>>
> >>> Changes in v2:
> >>> - s/start_addr/start_ofs/g'
> >>>
> >>>tools/proftool.c | 31 +--
> >>>1 file changed, 29 insertions(+), 2 deletions(-)
> >>>
> >>> Applied to u-boot-dm, thanks!
> >>
> >> FYI: I have merged it to my tree and already sent pull request to Tom.
> >> Without it I couldn't pass CI loop to get all reviewed features in.
> >>
> >> https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c...@amd.com/
> >
> > Ah OK, well that's fine. It was in my patchwork queue still, which
> > suggests that the patches were not set to 'applied'?
>
> I am not using patchwork. But I expect my reply to cover letter was recorded 
> there.

Probably. If you reply to each patch, it shows up in the patch, but
the cover letter is hidden somewhere else.

If you are not using patchwork, how come you are a custodian? Is
someone else dealing with patchwork for you?

Regards,
Simon

Re: Trying to boot custom kernel on Wink Hub (i.MX28)

[Rogan Dawes]

Hi Fabio,

On Mon, 25 Sep 2023 at 15:04, Fabio Estevam  wrote:

> Hi Rogan,
>
> On Mon, Sep 25, 2023 at 6:52 AM Rogan Dawes  wrote:
> >
> > Hi Fabio,
> >
> > I used the following diff with "make mx28evk_defconfig", but
> unfortunately still get absolutely nothing on the DUART when running
> "mxsloader u-boot.sb". In fact, mxsloader returns immediately on trying
> to load my own u-boot.sb, whereas it hangs when running the vendor
> u-boot.sb, I assume because the CALL never returns. Additionally, there
> is no need to reset the board between attempting a custom u-boot and the
> vendor u-boot, indicating that the ROM-based SDP protocol handler is still
> running. This suggests that there is something wrong with the format of the
> u-boot.sb file which the ROM-based SDP protocol handler is rejecting, I
> would think?
>
> I just tested loading u-boot.sb via mxsldr on my imx28-evk.
>
> I configured the boot jumpers to USB boot, turned on the board and
> then saw that it gets recognized by the host PC:
>
> $ dmesg
> 
> [1710347.828895] usb 1-1: new high-speed USB device number 96 using
> xhci_hcd
> [1710347.977657] usb 1-1: New USB device found, idVendor=15a2,
> idProduct=004f, bcdDevice= 0.01
> [1710347.977672] usb 1-1: New USB device strings: Mfr=1, Product=2,
> SerialNumber=0
> [1710347.977678] usb 1-1: Product: ROM Recovery
> [1710347.977682] usb 1-1: Manufacturer: Freescale,Inc.
> [1710347.979708] hid-generic 0003:15A2:004F.0014: hiddev0,hidraw1: USB
> HID v1.10 Device [Freescale,Inc. ROM Recovery] on
> usb-:00:14.0-1/input0
>
> After that, I loaded the u-boot.sb via mxsldr:
>
> $ sudo ./mxsldr u-boot.sb
> Detected: i.MX28
> Chip ID:  0x2800
> Chip Revision:0x0001
> ROM Version:  0x0101
> Protocol Version: 0x0100
>
> Do you get these messages above (both the dmesg as well the messages
> after running mxsldr)?


Yes, I get both the dmesg output showing the usb device, and the output
from mxsldr.

>
>
> And in the console, I do see the U-Boot messages coming up:
>

I see absolutely nothing in the console.

I’m wondering whether I can possibly execute the vendor SPL with a modern
u-boot?

And if that works, try and narrow down the differences.

Rogan

Re: [PATCH] bmips: Add Inteno XG6846 board

[Linus Walleij]

On Thu, Sep 21, 2023 at 7:25 PM Tom Rini  wrote:
> On Thu, Sep 21, 2023 at 04:00:24PM +0200, Daniel Schwierzeck wrote:
>
> [snip]
> > I just tested it, you can simply add an empty board/inteno/xg6846/Makefile 
> > and
> > remove board/inteno/xg6846/xg6846.c. But you can also remove the Makefile.
> > Just the Kconfig and MAINTAINERS file are needed.
>
> An empty Makefile is OK, but no Makefile requires a bit more as we first
> try and include it unconditionally and then try and link in built-in.o.
> I'm poking at this now.

Did you get anywhere with this? If it's on the master I'll just rebase and
be done with it, else I can go with an empty Makefile simply.

Yours,
Linus Walleij

Re: [PATCH 0/2] arm64: ti: k3-j7: Add the ESM nodes

[Nishanth Menon]

On 18:56-20230925, Keerthy wrote:
> Hi Tom,
> 
> https://lore.kernel.org/linux-arm-kernel/5187c590-ee9a-4c46-b326-655f4c371...@linaro.org/T/#me178708007a6b3b9695ae0ff84475fa4f49f283c
> 
> I have posted the ESM patches to the linux kernel device tree mailing
> list. These are neeeded in R5 SPL to enable watchdog support.
> 
> 
> Keerthy (2):
>   arm64: dts: ti: k3-j721s2: Add ESM instances
>   arm64: dts: ti: k3-j7200: Add MCU domain ESM instance
> 
>  arch/arm/dts/k3-j7200-mcu-wakeup.dtsi  |  7 +++
>  arch/arm/dts/k3-j721s2-main.dtsi   |  7 +++
>  arch/arm/dts/k3-j721s2-mcu-wakeup.dtsi | 14 ++
>  3 files changed, 28 insertions(+)

Get this as part of kernel dt sync.

-- 
Regards,
Nishanth Menon
Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 
849D 1736 249D

Re: [PATCH v2 3/3] trace: Fix alignment logic in flyrecord header

[Michal Simek]

On 9/25/23 15:10, Simon Glass wrote:

Hi Michal,

On Mon, 25 Sept 2023 at 00:06, Michal Simek  wrote:


Hi Simon,


On 9/23/23 20:13, Simon Glass wrote:

Current alignment which is using 16 bytes is not correct in connection to
trace_clocks description and it's length.
That's why use start_addr variable and record proper size based on used
entries.

Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format").
Signed-off-by: Michal Simek 
Reviewed-by: Simon Glass 
---

Changes in v2:
- s/start_addr/start_ofs/g'

   tools/proftool.c | 31 +--
   1 file changed, 29 insertions(+), 2 deletions(-)

Applied to u-boot-dm, thanks!


FYI: I have merged it to my tree and already sent pull request to Tom.
Without it I couldn't pass CI loop to get all reviewed features in.

https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c...@amd.com/


Ah OK, well that's fine. It was in my patchwork queue still, which
suggests that the patches were not set to 'applied'?


I am not using patchwork. But I expect my reply to cover letter was recorded 
there.

Thanks,
Michal

Re: [PATCH v3] env: ti: ti_common.env: Fix get_overlaystring for FIT Image

[Andrew Davis]

On 9/25/23 1:23 AM, Manorit Chawdhry wrote:

After the refactor with conf- nodes in fitImage, overlaystring wasn't
didn't handle the new conf- nodes in FIT Booting. Fix get_overlaystring
to handle conf- nodes.

Fixes: 837833a724b7 ("environment: ti: Add get_fit_config command to get FIT config 
string")
Reported-by: Aniket Limaye 
Signed-off-by: Manorit Chawdhry 
---


Reviewed-by: Andrew Davis 


Test Logs:
=> setenv name_overlays ti/k3-dt.dtbo ti/k3-dt1.dtbo
=> run get_overlaystring
=> printenv overlaystring
overlaystring=#conf-ti_k3-dt.dtbo#conf-ti_k3-dt1.dtbo
---
Changes in v3:
- Refactor to remove dependency on boot_fit ( Andrew )
- Rename it to get_fit_overlaystring as it is not used anywhere else
- Link to v2: 
https://lore.kernel.org/r/20230921-b4-upstream-overlaystring-v2-1-22e0b71e3...@ti.com
---
  include/env/ti/ti_common.env | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/env/ti/ti_common.env b/include/env/ti/ti_common.env
index e87a41a6590b..f5d84216e3ce 100644
--- a/include/env/ti/ti_common.env
+++ b/include/env/ti/ti_common.env
@@ -15,10 +15,10 @@ boot_fit=0
  addr_fit=0x9000
  name_fit=fitImage
  update_to_fit=setenv loadaddr ${addr_fit}; setenv bootfile ${name_fit}
-get_overlaystring=
-   for overlay in $name_overlays;
-   do;
-   setenv overlaystring ${overlaystring}'#'${overlay};
+get_fit_overlaystring=
+   for overlay in $name_overlays; do;
+   setexpr name_fit_overlay gsub / _ conf-${overlay};
+   setenv overlaystring ${overlaystring}'#'${name_fit_overlay};
done;
  get_fit_config=setexpr name_fit_config gsub / _ conf-${fdtfile}
  run_fit=run get_fit_config; bootm 
${addr_fit}#${name_fit_config}${overlaystring}
@@ -28,7 +28,7 @@ bootcmd_ti_mmc=
run main_cpsw0_qsgmii_phyinit; run boot_rprocs;
  #endif
if test ${boot_fit} -eq 1;
-   then run get_fit_${boot}; run get_overlaystring; run run_fit;
+   then run get_fit_${boot}; run get_fit_overlaystring; run 
run_fit;
else;
run get_kern_${boot}; run get_fdt_${boot}; run 
get_overlay_${boot}; run run_kern;
fi;

---
base-commit: 2fe4b54556ea6271237b35de68dc458bfceab94c
change-id: 20230915-b4-upstream-overlaystring-207e28b8c5fb

Best regards,

[PATCH 2/2] arm64: dts: ti: k3-j7200: Add MCU domain ESM instance

[Keerthy]

Patch adds the ESM instance for MCU domian of j7200.

Signed-off-by: Keerthy 
---
 arch/arm/dts/k3-j7200-mcu-wakeup.dtsi | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/arch/arm/dts/k3-j7200-mcu-wakeup.dtsi 
b/arch/arm/dts/k3-j7200-mcu-wakeup.dtsi
index 1044ec6c4b..2b185fa350 100644
--- a/arch/arm/dts/k3-j7200-mcu-wakeup.dtsi
+++ b/arch/arm/dts/k3-j7200-mcu-wakeup.dtsi
@@ -375,4 +375,11 @@
ti,loczrama = <1>;
};
};
+
+   mcu_esm: esm@4080 {
+   compatible = "ti,j721e-esm";
+   reg = <0x00 0x4080 0x00 0x1000>;
+   ti,esm-pins = <95>;
+   bootph-pre-ram;
+   };
 };
-- 
2.17.1

[PATCH 1/2] arm64: dts: ti: k3-j721s2: Add ESM instances

[Keerthy]

Patch adds the ESM instances for j721s2. It has 3 instances.
One in the main domain and two in the mcu-wakeup domian.

Signed-off-by: Keerthy 
---
 arch/arm/dts/k3-j721s2-main.dtsi   |  7 +++
 arch/arm/dts/k3-j721s2-mcu-wakeup.dtsi | 14 ++
 2 files changed, 21 insertions(+)

diff --git a/arch/arm/dts/k3-j721s2-main.dtsi b/arch/arm/dts/k3-j721s2-main.dtsi
index 976ba1e95a..859b5124ae 100644
--- a/arch/arm/dts/k3-j721s2-main.dtsi
+++ b/arch/arm/dts/k3-j721s2-main.dtsi
@@ -934,4 +934,11 @@
interrupt-names = "int0", "int1";
bosch,mram-cfg = <0x0 128 64 64 64 64 32 32>;
};
+
+   main_esm: esm@70 {
+   compatible = "ti,j721e-esm";
+   reg = <0x00 0x70 0x00 0x1000>;
+   ti,esm-pins = <688>, <689>;
+   bootph-pre-ram;
+   };
 };
diff --git a/arch/arm/dts/k3-j721s2-mcu-wakeup.dtsi 
b/arch/arm/dts/k3-j721s2-mcu-wakeup.dtsi
index 7521963719..f28cf7df50 100644
--- a/arch/arm/dts/k3-j721s2-mcu-wakeup.dtsi
+++ b/arch/arm/dts/k3-j721s2-mcu-wakeup.dtsi
@@ -299,4 +299,18 @@
ti,cpts-periodic-outputs = <2>;
};
};
+
+   mcu_esm: esm@4080 {
+   compatible = "ti,j721e-esm";
+   reg = <0x00 0x4080 0x00 0x1000>;
+   ti,esm-pins = <95>;
+   bootph-pre-ram;
+   };
+
+   wkup_esm: esm@4208 {
+   compatible = "ti,j721e-esm";
+   reg = <0x00 0x4208 0x00 0x1000>;
+   ti,esm-pins = <63>;
+   bootph-pre-ram;
+   };
 };
-- 
2.17.1

[PATCH 0/2] arm64: ti: k3-j7: Add the ESM nodes

[Keerthy]

Hi Tom,

https://lore.kernel.org/linux-arm-kernel/5187c590-ee9a-4c46-b326-655f4c371...@linaro.org/T/#me178708007a6b3b9695ae0ff84475fa4f49f283c

I have posted the ESM patches to the linux kernel device tree mailing
list. These are neeeded in R5 SPL to enable watchdog support.


Keerthy (2):
  arm64: dts: ti: k3-j721s2: Add ESM instances
  arm64: dts: ti: k3-j7200: Add MCU domain ESM instance

 arch/arm/dts/k3-j7200-mcu-wakeup.dtsi  |  7 +++
 arch/arm/dts/k3-j721s2-main.dtsi   |  7 +++
 arch/arm/dts/k3-j721s2-mcu-wakeup.dtsi | 14 ++
 3 files changed, 28 insertions(+)

-- 
2.17.1

Re: [PATCH 1/4] mkimage: also honour -B even without external data

[Rasmus Villemoes]

On 25/09/2023 15.10, Simon Glass wrote:
> Hi Rasmus,
> 
> On Mon, 25 Sept 2023 at 02:47, Rasmus Villemoes
>  wrote:

>> Since patches 2,3,4 touch binman code, could you take all four?
> 
> Yes, will do. I didn't pick them up in the most recent PR as I try to
> have things sit for a week before applying.

Oh, absolutely, no rush :)

Rasmus

Re: Config fragments

[Svyatoslav Ryhel]

23 серпня 2023 р. 18:55:58 GMT+03:00, Tom Rini  
написав(-ла):
>On Wed, Aug 23, 2023 at 09:30:31AM -0600, Simon Glass wrote:
>
>> Hi,
>> 
>> Up until 2023.04 it has been possible to build all the defconfigs but
>> with 2023.07 that changed. Tom mentioned this to me recently.
>
>I'm adding Svyatoslav who is the person that brought in all of the
>config fragments that are going to be in v2023.07.
>
>> Up until 2023.04 we can enumerate all the board configs that can be
>> built. We can build any of them using a single name and a single
>> defconfig. The boards.cfg file which buildman creates contains a full
>> list of things that can be built.
>> 
>> From 2023.07 this changes and we now have random .config files
>> sprinkled about the place. I say random because they are not connected
>> to anything. For example, here is
>
>They aren't random.  They're, just for v2023.07, in configs/ and then
>moving forward, they're in the board directory.  I would like to see
>them in more places possibly, but that's not something I'm sure enough
>on right now.
>
>> board/ti/am62x/MAINTAINERS:
>>
>> AM62x BOARD
>> M: Dave Gerlach 
>> M: Tom Rini 
>> S: Maintained
>> F: board/ti/am62x/
>> F: include/configs/am62x_evm.h
>> F: configs/am62x_evm_r5_defconfig
>> F: configs/am62x_evm_a53_defconfig
>> 
>> BEAGLEPLAY BOARD
>> M: Nishanth Menon 
>> M: Robert Nelson 
>> M: Tom Rini 
>> S: Maintained
>> N: beagleplay
>> 
>> In most cases the MAINTAINERS file tells us about each board and until
>> [1] I had assumed that was the case. With that patch reverted, these
>> are the only 'orphaned' MAINTAINERS entries (buildman
>> --maintainer-check):
>> 
>> WARNING: orphaned defconfig in board/armltd/vexpress64/MAINTAINERS
>> ending at line 8
>> WARNING: orphaned defconfig in board/google/veyron/MAINTAINERS ending at 
>> line 44
>> WARNING: orphaned defconfig in
>> board/mikrotik/crs3xx-98dx3236/MAINTAINERS ending at line 7
>> WARNING: orphaned defconfig in board/st/common/MAINTAINERS ending at line 6
>> WARNING: orphaned defconfig in board/ti/am62x/MAINTAINERS ending at line 15
>> 
>> But Tom advised me that MAINTAINERS is not intended for this purpose,
>> so the check has been dropped. I am not suggesting we bring it back,
>> necessarily, but if we did, we could use it to specify the .config and
>> defconfig files which are used by each board.
>
>Note that the "N" syntax has been in use, for defconfigs for a lot
>longer, and is why the CI check for unmaintained / orphaned boards
>stopped working.
>
>> *Failing that*, I suggest a new 'name.brd' file in the board directory
>> to contain this information, e.g.
>> 
>> # Contents of board/ti/am62x/beagleplay.brd:
>> beagleplay-r5: am62x_evm_r5_defconfig beagleplay_r5.config
>> beagleplay-a53: am62x_evm_a53_defconfig beagleplay_a53.config
>> 
>> This could be parsed by buildman and other tools, to produce a list of
>> available boards and to build each using a single name (e.g. make
>> beagleplay-r5_defconfig)
>> 
>> What do people think?
>
>I'm not sure I like this direction honestly.  Do we want CI to cover
>these configurations?  Maybe.  But it's just for CI.  Using the TI
>examples, I don't know that you can use buildman today to generate a
>functional binary (and if you can, if --keep will retain the right
>files).
>
>Maybe we just need to make it clear that if you use config fragments
>then you're opting out of CI for that specific platform.  That should be
>fine for example for the TI ones that will be built in OE a bunch
>anyhow so accidental failure to builds will be caught.  I don't know
>about the Tegra platforms.
>

Greetings! Are there any progress regards fragments move or this will hang in 
mailing list indefinitely?

Best regards,
Svyatoslav R.

Re: [PATCH 0/2] make CONFIG_SPL_SYS_MALLOC_SIMPLE && CONFIG_SYS_SPL_MALLOC actually work

[Simon Glass]

Hi Rasmus,

On Mon, 25 Sept 2023 at 02:54, Rasmus Villemoes
 wrote:
>
> On 21/09/2023 03.02, Simon Glass wrote:
> > Hi Rasmus,
> >
> > On Fri, 15 Sept 2023 at 11:51, Rasmus Villemoes
> >  wrote:
> >>
> >> Currently, setting both CONFIG_SPL_SYS_MALLOC_SIMPLE and
> >> CONFIG_SYS_SPL_MALLOC (but not CONFIG_SPL_STACK_R) doesn't work as
> >> expected: The SIMPLE option means that all malloc etc. calls are
> >> directed at build-time to the implementation in malloc_simple.c, but
> >
> > Sort-of. It is control by both a CONFIG and GD_FLG_FULL_MALLOC_INIT.
> >
>
> Eh, no? The CONFIG option completely preempts any run-time gd flag checking
>
> #if CONFIG_IS_ENABLED(SYS_MALLOC_SIMPLE)
> #define malloc malloc_simple
> #define realloc realloc_simple
> #define memalign memalign_simple
>
> Yes, _without_ that CONFIG option, the "real" malloc() functions do
> check that gd flag and if not set fall back to the _simple variants. But
> what I wrote is not merely "sort-of" correct.

OK I see what you mean, yes.

Regards,
Simon

Re: [PATCH v2] arm: rpi: remove boot_targets for default standard boot

[Simon Glass]

On Sat, 23 Sept 2023 at 20:59, Date Huang  wrote:
>
> Raspberry pi supports standard boot without boot_targets
>
> Signed-off-by: Date Huang 
> ---
>  board/raspberrypi/rpi/rpi.env | 2 --
>  1 file changed, 2 deletions(-)

Reviewed-by: Simon Glass 


>
> diff --git a/board/raspberrypi/rpi/rpi.env b/board/raspberrypi/rpi/rpi.env
> index 30228285ed..8fb8d5d5f7 100644
> --- a/board/raspberrypi/rpi/rpi.env
> +++ b/board/raspberrypi/rpi/rpi.env
> @@ -73,5 +73,3 @@ scriptaddr=0x0240
>  pxefile_addr_r=0x0250
>  fdt_addr_r=0x0260
>  ramdisk_addr_r=0x0270
> -
> -boot_targets=mmc usb pxe dhcp
> --
> 2.34.1
>

Re: [PATCH 4/5] sandbox: rename overlay sources to .dtso

[Simon Glass]

On Mon, 25 Sept 2023 at 02:10, Rasmus Villemoes
 wrote:
>
> Distinguish more clearly between source files meant for producing .dtb
> from those meant for producing .dtbo. No functional change, as we
> currently have rules for producing a foo.dtbo from either foo.dts or
> foo.dtso.
>
> Note that in the linux tree, all device tree overlay sources have been
> renamed to .dtso, and the .dts->.dtbo rule is gone since v6.5 (commit
> 81d362732bac). So this is also a step towards staying closer to linux
> with respect to both Kbuild and device tree sources.
>
> Signed-off-by: Rasmus Villemoes 
> ---
>  arch/sandbox/dts/{overlay0.dts => overlay0.dtso} | 0
>  arch/sandbox/dts/{overlay1.dts => overlay1.dtso} | 0
>  2 files changed, 0 insertions(+), 0 deletions(-)
>  rename arch/sandbox/dts/{overlay0.dts => overlay0.dtso} (100%)
>  rename arch/sandbox/dts/{overlay1.dts => overlay1.dtso} (100%)

Reviewed-by: Simon Glass 

Re: [PATCH 5/5] doc: use .dtso as extension for device tree overlay sources

[Simon Glass]

On Mon, 25 Sept 2023 at 02:10, Rasmus Villemoes
 wrote:
>
> Moving towards using .dtso for overlay sources, update the
> documentation examples to follow that pattern.
>
> Signed-off-by: Rasmus Villemoes 
> ---
>  doc/develop/uefi/uefi.rst  | 4 ++--
>  doc/usage/fdt_overlays.rst | 4 ++--
>  2 files changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass 

Re: [PATCH 1/4] mkimage: also honour -B even without external data

[Simon Glass]

Hi Rasmus,

On Mon, 25 Sept 2023 at 02:47, Rasmus Villemoes
 wrote:
>
> On 22/09/2023 17.26, Simon Glass wrote:
>
> >>> Shouldn't this be fdt_open_into()?
> >>
> >> I honestly just copy-pasted fit_extract_data() and shaved it down to the
> >> part that does the "align the FDT part of the file".
> >>
> >> I don't really understand your question. Are you saying this doesn't
> >> work (or maybe doesn't work in some cases), or are you saying that
> >> there's a simpler way to do this? For the latter, sure, one doesn't
> >> really need to parse the whole FDT; we could just
> >>
> >>   open()
> >>   pread() length from FDT header, convert to cpu-endianness
> >>   length = ALIGN(length)
> >>   pwrite() the new length in fdt-endianness
> >>   ftruncate()
> >>   close()
> >>
> >> but I thought it was better to stay closer to how fit_extract_data() was
> >> done.
> >
> > I mean that fdt_open_into() does more than just set the size (from
> > what I can tell). But looking further I see other code which calls
> > fdt_set_totalsize() so perhaps it is fine.
>
> Yes, I think it's as it should be - as a I said, this is really just a
> trimmed-down copy of the function which moves the data externally, and
> also needs to make the size of the base fdt structure aligned.
>
> Since patches 2,3,4 touch binman code, could you take all four?

Yes, will do. I didn't pick them up in the most recent PR as I try to
have things sit for a week before applying.

Regards,
Simon

Re: [PATCH v2 3/3] trace: Fix alignment logic in flyrecord header

[Simon Glass]

Hi Michal,

On Mon, 25 Sept 2023 at 00:06, Michal Simek  wrote:
>
> Hi Simon,
>
>
> On 9/23/23 20:13, Simon Glass wrote:
> > Current alignment which is using 16 bytes is not correct in connection to
> > trace_clocks description and it's length.
> > That's why use start_addr variable and record proper size based on used
> > entries.
> >
> > Fixes: be16fc81b2ed ("trace: Update proftool to use new binary format").
> > Signed-off-by: Michal Simek 
> > Reviewed-by: Simon Glass 
> > ---
> >
> > Changes in v2:
> > - s/start_addr/start_ofs/g'
> >
> >   tools/proftool.c | 31 +--
> >   1 file changed, 29 insertions(+), 2 deletions(-)
> >
> > Applied to u-boot-dm, thanks!
>
> FYI: I have merged it to my tree and already sent pull request to Tom.
> Without it I couldn't pass CI loop to get all reviewed features in.
>
> https://lore.kernel.org/all/ab72c480-e9f8-416e-adf5-726f7d40c...@amd.com/

Ah OK, well that's fine. It was in my patchwork queue still, which
suggests that the patches were not set to 'applied'?

Regards,
Simon

Re: [PATCH 1/2] serial: serial-uclass.c: move definition of _serial_flush up a bit

[Simon Glass]

On Mon, 25 Sept 2023 at 05:01, Rasmus Villemoes
 wrote:
>
> Preparation for next patch.
>
> Signed-off-by: Rasmus Villemoes 
> ---
>  drivers/serial/serial-uclass.c | 24 
>  1 file changed, 12 insertions(+), 12 deletions(-)

Reviewed-by: Simon Glass 

Re: Trying to boot custom kernel on Wink Hub (i.MX28)

[Fabio Estevam]

Hi Rogan,

On Mon, Sep 25, 2023 at 6:52 AM Rogan Dawes  wrote:
>
> Hi Fabio,
>
> I used the following diff with "make mx28evk_defconfig", but unfortunately 
> still get absolutely nothing on the DUART when running "mxsloader u-boot.sb". 
> In fact, mxsloader returns immediately on trying to load my own u-boot.sb, 
> whereas it hangs when running the vendor u-boot.sb, I assume because the CALL 
> never returns. Additionally, there is no need to reset the board between 
> attempting a custom u-boot and the vendor u-boot, indicating that the 
> ROM-based SDP protocol handler is still running. This suggests that there is 
> something wrong with the format of the u-boot.sb file which the ROM-based SDP 
> protocol handler is rejecting, I would think?

I just tested loading u-boot.sb via mxsldr on my imx28-evk.

I configured the boot jumpers to USB boot, turned on the board and
then saw that it gets recognized by the host PC:

$ dmesg

[1710347.828895] usb 1-1: new high-speed USB device number 96 using xhci_hcd
[1710347.977657] usb 1-1: New USB device found, idVendor=15a2,
idProduct=004f, bcdDevice= 0.01
[1710347.977672] usb 1-1: New USB device strings: Mfr=1, Product=2,
SerialNumber=0
[1710347.977678] usb 1-1: Product: ROM Recovery
[1710347.977682] usb 1-1: Manufacturer: Freescale,Inc.
[1710347.979708] hid-generic 0003:15A2:004F.0014: hiddev0,hidraw1: USB
HID v1.10 Device [Freescale,Inc. ROM Recovery] on
usb-:00:14.0-1/input0

After that, I loaded the u-boot.sb via mxsldr:

$ sudo ./mxsldr u-boot.sb
Detected: i.MX28
Chip ID:  0x2800
Chip Revision:0x0001
ROM Version:  0x0101
Protocol Version: 0x0100

Do you get these messages above (both the dmesg as well the messages
after running mxsldr)?

And in the console, I do see the U-Boot messages coming up:

HTLLCLLC

U-Boot 2023.10-rc4-00057-g4cb31a9f3560 (Sep 25 2023 - 09:49:04 -0300)

CPU:   Freescale i.MX28 rev1.2 at 454 MHz
BOOT:  USB #0
DRAM:  128 MiB
Core:  89 devices, 12 uclasses, devicetree: separate
NAND:  0 MiB
MMC:   MXS MMC: 0
Loading Environment from MMC... *** Warning - No block device, using
default environment

In:serial
Out:   serial
Err:   serial
Hit any key to stop autoboot:  0

Re: [PATCH v4 5/8] efi_loader: set EFI HTTP Boot download buffer as reserved

[Ilias Apalodimas]

Kojima-san,

[...]
>  /* Carve out DT reserved memory ranges */
>  void efi_carve_out_dt_rsv(void *fdt);
>  /* Purge unused kaslr-seed */
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index 605be5041e..4991056946 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -326,6 +326,11 @@ static efi_status_t try_load_from_uri_path(struct 
> efi_device_path_uri *uridp,
>   return EFI_INVALID_PARAMETER;
>
>   ret = load_default_file_from_blk_dev(blk, handle);
> + if (ret != EFI_SUCCESS)
> + return ret;
> +
> + /* whole ramdisk must be reserved */
> + efi_reserve_memory(image_addr, image_size, true);

Why is this a different patch though?
My concern is code duplication when we add similar functionality in
eficonfig.  Isn't there a better place to handle the memory reservation?

[...]

Thanks
/Ilias

Re: [PATCH v4 4/8] efi_loader: support boot from URI device path

[Ilias Apalodimas]

On Fri, Sep 22, 2023 at 04:11:15PM +0900, Masahisa Kojima wrote:
> This supports to boot from the URI device path.
> When user selects the URI device path, bootmgr downloads
> the file using wget into the address specified by loadaddr
> env variable.
> If the file is .iso or .img file, mount the image with blkmap
> then try to boot with the default file(e.g. EFI/BOOT/BOOTAA64.EFI).
> If the file is .efi file, load and start the downloaded file.
>
> Signed-off-by: Masahisa Kojima 
> ---
>  lib/efi_loader/efi_bootmgr.c | 189 +++
>  1 file changed, 189 insertions(+)
>
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index a40762c74c..605be5041e 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -7,10 +7,14 @@
>
>  #define LOG_CATEGORY LOGC_EFI
>
> +#include 
> +#include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -168,6 +172,182 @@ out:
>   return ret;
>  }
>
> +/**
> + * mount_image() - mount the image with blkmap
> + *
> + * @lo_label u16 label string of load option
> + * @image_addr:  image address
> + * @image_size   image size
> + * Return:   pointer to the UCLASS_BLK udevice, NULL if failed
> + */
> +static struct udevice *mount_image(u16 *lo_label, ulong image_addr, int 
> image_size)
> +{
> + int err;
> + struct blkmap *bm;
> + struct udevice *bm_dev;
> + char *label = NULL, *p;
> +
> + label = efi_alloc(utf16_utf8_strlen(lo_label) + 1);
> + if (!label)
> + return NULL;
> +
> + p = label;
> + utf16_utf8_strcpy(, lo_label);
> + err = blkmap_create_ramdisk(label, image_addr, image_size, _dev);
> + if (err) {
> + efi_free_pool(label);
> + return NULL;
> + }
> + bm = dev_get_plat(bm_dev);
> +
> + efi_free_pool(label);
> +
> + return bm->blk;
> +}
> +
> +/**
> + * try_load_default_file() - try to load the default file
> + *
> + * Search the device having EFI_SIMPLE_FILE_SYSTEM_PROTOCOL,
> + * then try to load with the default boot file(e.g. EFI/BOOT/BOOTAA64.EFI).
> + *
> + * @dev  pointer to the UCLASS_BLK or UCLASS_PARTITION 
> udevice
> + * @image_handle:pointer to handle for newly installed image
> + * Return:   status code
> + */
> +static efi_status_t try_load_default_file(struct udevice *dev,
> +   efi_handle_t *image_handle)
> +{

Maybe I misunderstood you on the previous series.  Wasn't the plan to merge
the patch that adds boot options automatically when a disk is scanned? This
code could only look for a boot option with '1234567' in its load options
instead of rescanning all devices with the EFI_SIMPLE_FILE_SYSTEM_PROTOCO
installed

> + efi_status_t ret;
> + efi_handle_t handle;
> + struct efi_handler *handler;
> + struct efi_device_path *file_path;
> + struct efi_device_path *device_path;
> +
> + if (dev_tag_get_ptr(dev, DM_TAG_EFI, (void **))) {
> + log_warning("DM_TAG_EFI not found\n");
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + ret = efi_search_protocol(handle,
> +   _simple_file_system_protocol_guid, 
> );
> + if (ret != EFI_SUCCESS)
> + return ret;
> +
> + ret = EFI_CALL(bs->open_protocol(handle, _guid_device_path,
> +  (void **)_path, efi_root, NULL,
> +  EFI_OPEN_PROTOCOL_GET_PROTOCOL));
> + if (ret != EFI_SUCCESS)
> + return ret;
> +
> + file_path = expand_media_path(device_path);
> + ret = EFI_CALL(efi_load_image(true, efi_root, file_path, NULL, 0,
> +   image_handle));
> +
> + efi_free_pool(file_path);
> +
> + return ret;
> +}
> +
> +/**
> + * load_default_file_from_blk_dev() - load the default file
> + *
> + * @blk  pointer to the UCLASS_BLK udevice
> + * @handle:  pointer to handle for newly installed image
> + * Return:   status code
> + */
> +static efi_status_t load_default_file_from_blk_dev(struct udevice *blk,
> +efi_handle_t *handle)
> +{
> + efi_status_t ret;
> + struct udevice *partition;
> +
> + /* image that has no partition table but a file system */
> + ret = try_load_default_file(blk, handle);
> + if (ret == EFI_SUCCESS)
> + return ret;
> +
> + /* try the partitions */
> + device_foreach_child(partition, blk) {
> + enum uclass_id id;
> +
> + id = device_get_uclass_id(partition);
> + if (id != UCLASS_PARTITION)
> + continue;
> +
> + ret = try_load_default_file(partition, handle);
> + if (ret == EFI_SUCCESS)
> + return ret;
> + }
> +
> + return EFI_NOT_FOUND;
> +}
> +
> +/**
> + * 

Re: [PATCH] dt-bindings: mtd: Add a schema for binman

[Simon Glass]

Hi Miquel,

On Mon, 25 Sept 2023 at 01:21, Miquel Raynal  wrote:
>
> Hi Simon,
>
> s...@chromium.org wrote on Fri, 22 Sep 2023 13:51:14 -0600:
>
> > Hi Rob,
> >
> > On Fri, 22 Sept 2023 at 13:43, Rob Herring  wrote:
> > >
> > > On Fri, Sep 22, 2023 at 1:12 PM Simon Glass  wrote:
> > > >
> > > > Hi Rob,
> > > >
> > > > On Fri, 22 Sept 2023 at 11:46, Rob Herring  wrote:
> > > > >
> > > > > On Fri, Sep 22, 2023 at 11:01:18AM -0600, Simon Glass wrote:
> > > > > > Hi Rob,
> > > > > >
> > > > > > On Fri, 22 Sept 2023 at 10:00, Rob Herring  wrote:
> > > > > > >
> > > > > > > On Thu, Sep 21, 2023 at 1:45 PM Simon Glass  
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > Binman[1] is a tool for creating firmware images. It allows you 
> > > > > > > > to
> > > > > > > > combine various binaries and place them in an output file.
> > > > > > > >
> > > > > > > > Binman uses a DT schema to describe an image, in enough detail 
> > > > > > > > that
> > > > > > > > it can be automatically built from component parts, 
> > > > > > > > disassembled,
> > > > > > > > replaced, listed, etc.
> > > > > > > >
> > > > > > > > Images are typically stored in flash, which is why this binding 
> > > > > > > > is
> > > > > > > > targeted at mtd. Previous discussion is at [2] [3].
> > > > > > > >
> > > > > > > > [1] 
> > > > > > > > https://u-boot.readthedocs.io/en/stable/develop/package/binman.html
> > > > > > > > [2] 
> > > > > > > > https://lore.kernel.org/u-boot/20230821180220.2724080-3-...@chromium.org/
> > > > > > > > [3] https://www.spinics.net/lists/devicetree/msg626149.html
> > > > > > >
> > > > > > > You missed:
> > > > > > >
> > > > > > > https://github.com/devicetree-org/dt-schema/pull/110
> > > > > > >
> > > > > > > where I said: We certainly shouldn't duplicate the existing 
> > > > > > > partitions
> > > > > > > bindings. What's missing from them (I assume we're mostly talking
> > > > > > > about "fixed-partitions" which has been around forever I think 
> > > > > > > (before
> > > > > > > me))?
> > > > > > >
> > > > > > > To repeat, unless there is some reason binman partitions conflict 
> > > > > > > with
> > > > > > > fixed-partitions, you need to start there and extend it. From 
> > > > > > > what's
> > > > > > > posted here, it neither conflicts nor needs extending.
> > > > > >
> > > > > > I think at this point I am just hopelessly confused. Have you taken 
> > > > > > a
> > > > > > look at the binman schema? [1]
> > > > >
> > > > > Why do I need to? That's used for some tool and has nothing to do 
> > > > > with a
> > > > > device's DTB. However, I thought somewhere in this discussion you 
> > > > > showed
> > > > > it under a flash device node.
> > > >
> > > > Yes, that is the intent (under a flash node).
> > > >
> > > > > Then I care because then it overlaps with
> > > > > what we already have for partitions. If I misunderstood that, then 
> > > > > just
> > > > > put your schema with your tool. Only users of the tool should care 
> > > > > about
> > > > > the tool's schema.
> > > >
> > > > OK. I believe that binman will fit into both camps, since its input is
> > > > not necessarily fully formed. E.g. if you don't specify the offset of
> > > > an entry, then it will be packed automatically. But the output is
> > > > fully formed, in that Binman now knows the offset so can write it to
> > > > the DT.
> > >
> > > I suppose it could take its own format as input and then write out
> > > something different for the "on the device" format (i.e.
> > > fixed-partitions). At least for the dynamic offsets, we may need
> > > something allowed for binman input, but not allowed on device. In
> > > general, there is support for partitions without addresses/offsets,
> > > but only for partitions that have some other way to figure that out
> > > (on disk partition info).
> > >
> > > There's also the image filename which doesn't really belong in the on
> > > device partitions. So maybe the input and output schemas should be
> > > separate.
> >
> > OK, I'll focus on the output schema for now. I suspect this will be a
> > grey area though.
> >
> > As an example, if you replace a binary in the firmware, Binman can
> > repack the firmware to make room, respecting the alignment and size
> > constraints. So these need to be in the output schema somehow.
> >
> > >
> > > > > > I saw this file, which seems to extend a partition.
> > > > > >
> > > > > > Documentation/devicetree/bindings/mtd/partitions/brcm,bcm4908-partitions.yaml
> > > > >
> > > > > IIRC, that's a different type where partition locations are stored in
> > > > > the flash, so we don't need location and size in DT.
> > > >
> > > > OK.
> > > >
> > > > >
> > > > > >
> > > > > > I was assuming that I should create a top-level compatible = 
> > > > > > "binman"
> > > > > > node, with subnodes like compatible = "binman,bl31-atf", for 
> > > > > > example.
> > > > > > I should use the compatible string to indicate the contents, right?
> > > > >
> > > > > Yes for subnodes, and we 

[PATCH] fastboot: fix CRC32 chunk size checking

[Wojciech Nizinski]

genimage create android-sparse file with CRC32 chunk at end. When
U-Boot's fastboot receives this chunk it returns error message:
`Fail Bogus chunk size for chunk type Dont Care`

According to reference implementation of Android's sparse file format:



the chunk_header.total_sz is CHUNK_HEADER_LEN + 4 (CRC32 size).

Signed-off-by: Wojciech Nizinski 
---

 lib/image-sparse.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/image-sparse.c b/lib/image-sparse.c
index 8f8a67e158..323aad981c 100644
--- a/lib/image-sparse.c
+++ b/lib/image-sparse.c
@@ -289,8 +289,8 @@ int write_sparse_image(struct sparse_storage *info,
 
case CHUNK_TYPE_CRC32:
if (chunk_header->total_sz !=
-   sparse_header->chunk_hdr_sz) {
-   info->mssg("Bogus chunk size for chunk type 
Dont Care",
+   sparse_header->chunk_hdr_sz + sizeof(uint32_t)) {
+   info->mssg("Bogus chunk size for chunk type 
CRC32",
   response);
return -1;
}
-- 
2.39.2

Re: [PATCH v4 7/8] doc: uefi: add HTTP Boot support

[Ilias Apalodimas]

On Fri, Sep 22, 2023 at 04:11:18PM +0900, Masahisa Kojima wrote:
> This adds the description about HTTP Boot.
>
> Signed-off-by: Masahisa Kojima 
> ---
>  doc/develop/uefi/uefi.rst | 30 ++
>  1 file changed, 30 insertions(+)
>
> diff --git a/doc/develop/uefi/uefi.rst b/doc/develop/uefi/uefi.rst
> index a7a41f2fac..65eea89265 100644
> --- a/doc/develop/uefi/uefi.rst
> +++ b/doc/develop/uefi/uefi.rst
> @@ -594,6 +594,36 @@ UEFI variables. Booting according to these variables is 
> possible via::
>  As of U-Boot v2020.10 UEFI variables cannot be set at runtime. The U-Boot
>  command 'efidebug' can be used to set the variables.
>
> +UEFI HTTP Boot
> +~~
> +
> +HTTP Boot provides the capability for system deployment and configuration
> +over the network. HTTP Boot can be activated by specifying::
> +
> +CONFIG_CMD_DNS
> +CONFIG_CMD_WGET
> +CONFIG_BLKMAP
> +
> +Set up the load option specifying the target URI::
> +
> +efidebug boot add -u 1 netinst http://foo/bar
> +
> +When this load option is selected as boot selection, resolve the
> +host ip address by dns, then download the file with wget.
> +If the downloaded file extension is .iso or .img file, efibootmgr tries to
> +mount the image and boot with the default file(e.g. EFI/BOOT/BOOTAA64.EFI).
> +If the downloaded file is PE-COFF image, load the downloaded file and
> +start it.
> +
> +There is a limitation that current implementation tries to resolve

Remove the 'There is a limitation that', use The current implementation ...

> +the IP address as a host name. If the uri is like 
> "http://192.168.1.1/foobar;,
> +the dns process tries to resolve the host "192.168.1.1" and it will
> +end up with "host not found".
> +
> +We need to preset the "httpserverip" environment variable to proceed the 
> wget::
> +
> +setenv httpserverip 192.168.1.1
> +
>  Executing the built in hello world application
>  ~~
>
> --
> 2.34.1
>

Other than the nit above
Reviewed-by: Ilias Apalodimas 

Re: [PATCH 2/2] serial: introduce CONFIG_CONSOLE_FLUSH_ON_NEWLINE

[Michal Suchánek]

Hello,

On Mon, Sep 25, 2023 at 01:01:01PM +0200, Rasmus Villemoes wrote:
> When debugging, one sometimes only gets partial output lines or
> nothing at all from the last printf, because the uart has a largish
> buffer, and the code after the printf() may cause the CPU to hang
> before the uart IP has time to actually emit all the characters. That
> can be very confusing, because one doesn't then know exactly where the
> hang happens.
> 
> Introduce a config knob allowing one to wait for the uart fifo to
> drain whenever a newline character is printed, roughly corresponding
> to the effect of setvbuf(..., _IOLBF, ...) in ordinary C programs.
> 
> Signed-off-by: Rasmus Villemoes 
> ---
>  common/Kconfig | 11 +++
>  drivers/serial/serial-uclass.c |  8 ++--
>  2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/common/Kconfig b/common/Kconfig
> index cdb77a6a7d..49130f2a2b 100644
> --- a/common/Kconfig
> +++ b/common/Kconfig
> @@ -224,6 +224,17 @@ config CONSOLE_FLUSH_SUPPORT
>   help
> This enables compilation of flush() function for console flush 
> support.
>  
> +config CONSOLE_FLUSH_ON_NEWLINE
> + bool "Flush console buffer on every newline character"
> + depends on CONSOLE_FLUSH_SUPPORT

Maybe selects would be more appropriate here if it can be arbitrarily
enabled on any console

> + depends on DM_SERIAL
> + help
> +   This makes the serial core code flush the console device
> +   whenever a newline (\n) character has been emitted. This can
> +   be especially useful when "printf debugging", as otherwise
> +   lots of output could still be in the UART's FIFO by the time
> +   one hits the code which causes the CPU to hang or reset.
> +
>  config CONSOLE_MUX
>   bool "Enable console multiplexing"
>   default y if VIDEO || LCD
> diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
> index 2f75ff878c..b41e3ebe7f 100644
> --- a/drivers/serial/serial-uclass.c
> +++ b/drivers/serial/serial-uclass.c
> @@ -181,7 +181,6 @@ int serial_initialize(void)
>   return serial_init();
>  }
>  
> -#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT

This should not be removed. CONSOLE_FLUSH_ON_NEWLINE will only ever be
configured when CONFIG_CONSOLE_FLUSH_SUPPORT is also enabled so there is
no need to remove, either.

Thanks

Michal

>  static void _serial_flush(struct udevice *dev)
>  {
>   struct dm_serial_ops *ops = serial_get_ops(dev);
> @@ -191,7 +190,6 @@ static void _serial_flush(struct udevice *dev)
>   while (ops->pending(dev, false) > 0)
>   ;
>  }
> -#endif
>  
>  static void _serial_putc(struct udevice *dev, char ch)
>  {
> @@ -204,6 +202,9 @@ static void _serial_putc(struct udevice *dev, char ch)
>   do {
>   err = ops->putc(dev, ch);
>   } while (err == -EAGAIN);
> +
> + if (IS_ENABLED(CONSOLE_FLUSH_ON_NEWLINE) && ch == '\n')
> + _serial_flush(dev);
>  }
>  
>  static int __serial_puts(struct udevice *dev, const char *str, size_t len)
> @@ -242,6 +243,9 @@ static void _serial_puts(struct udevice *dev, const char 
> *str)
>   if (*newline && __serial_puts(dev, "\r\n", 2))
>   return;
>  
> + if (IS_ENABLED(CONSOLE_FLUSH_ON_NEWLINE) && *newline)
> + _serial_flush(dev);
> +
>   str += len + !!*newline;
>   } while (*str);
>  }
> -- 
> 2.37.2
> 

Re: [PATCH v4 3/8] blk: blkmap: add ramdisk creation utility function

[Ilias Apalodimas]

On Fri, Sep 22, 2023 at 04:11:14PM +0900, Masahisa Kojima wrote:
> User needs to call several functions to create the ramdisk
> with blkmap.
> This adds the utility function to create blkmap device and
> mount the ramdisk.
>
> Signed-off-by: Masahisa Kojima 
> Reviewed-by: Simon Glass 
> ---
>  drivers/block/Makefile|  1 +
>  drivers/block/blkmap.c| 15 --
>  drivers/block/blkmap_helper.c | 53 +++
>  include/blkmap.h  | 29 +++
>  4 files changed, 83 insertions(+), 15 deletions(-)
>  create mode 100644 drivers/block/blkmap_helper.c
>
> diff --git a/drivers/block/Makefile b/drivers/block/Makefile
> index a161d145fd..c3ccfc03e5 100644
> --- a/drivers/block/Makefile
> +++ b/drivers/block/Makefile
> @@ -15,6 +15,7 @@ endif
>  obj-$(CONFIG_SANDBOX) += sandbox.o host-uclass.o host_dev.o
>  obj-$(CONFIG_$(SPL_TPL_)BLOCK_CACHE) += blkcache.o
>  obj-$(CONFIG_BLKMAP) += blkmap.o
> +obj-$(CONFIG_BLKMAP) += blkmap_helper.o
>
>  obj-$(CONFIG_EFI_MEDIA) += efi-media-uclass.o
>  obj-$(CONFIG_EFI_MEDIA_SANDBOX) += sb_efi_media.o
> diff --git a/drivers/block/blkmap.c b/drivers/block/blkmap.c
> index 2bb0acc20f..4e95997f61 100644
> --- a/drivers/block/blkmap.c
> +++ b/drivers/block/blkmap.c
> @@ -66,21 +66,6 @@ struct blkmap_slice {
>   void (*destroy)(struct blkmap *bm, struct blkmap_slice *bms);
>  };
>
> -/**
> - * struct blkmap - Block map
> - *
> - * Data associated with a blkmap.
> - *
> - * @label: Human readable name of this blkmap
> - * @blk: Underlying block device
> - * @slices: List of slices associated with this blkmap
> - */
> -struct blkmap {
> - char *label;
> - struct udevice *blk;
> - struct list_head slices;
> -};
> -
>  static bool blkmap_slice_contains(struct blkmap_slice *bms, lbaint_t blknr)
>  {
>   return (blknr >= bms->blknr) && (blknr < (bms->blknr + bms->blkcnt));
> diff --git a/drivers/block/blkmap_helper.c b/drivers/block/blkmap_helper.c
> new file mode 100644
> index 00..0f80035f57
> --- /dev/null
> +++ b/drivers/block/blkmap_helper.c
> @@ -0,0 +1,53 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * blkmap helper function
> + *
> + * Copyright (c) 2023, Linaro Limited
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +int blkmap_create_ramdisk(const char *label, ulong image_addr, int 
> image_size,
> +   struct udevice **devp)
> +{
> + int ret;
> + lbaint_t blknum;
> + struct blkmap *bm;
> + struct blk_desc *desc;
> + struct udevice *bm_dev;
> +
> + ret = blkmap_create(label, _dev);
> + if (ret) {
> + log_err("failed to create blkmap\n");
> + return ret;
> + }
> +
> + bm = dev_get_plat(bm_dev);
> + desc = dev_get_uclass_plat(bm->blk);
> + blknum = image_size >> desc->log2blksz;
> + ret = blkmap_map_pmem(bm_dev, 0, blknum, image_addr);
> + if (ret) {
> + log_err("Unable to map %#llx at block %d : %d\n",
> + (unsigned long long)image_addr, 0, ret);
> + goto err;
> + }
> + log_info("Block %d+0x" LBAF " mapped to %#llx\n", 0, blknum,
> +  (unsigned long long)image_addr);
> +
> + ret = device_probe(bm->blk);
> + if (ret)
> + goto err;
> +
> + if (devp)
> + *devp = bm_dev;
> +
> + return 0;
> +
> +err:
> + blkmap_destroy(bm_dev);
> +
> + return ret;
> +}
> diff --git a/include/blkmap.h b/include/blkmap.h
> index af54583c7d..0d87e6db6b 100644
> --- a/include/blkmap.h
> +++ b/include/blkmap.h
> @@ -7,6 +7,23 @@
>  #ifndef _BLKMAP_H
>  #define _BLKMAP_H
>
> +#include 
> +
> +/**
> + * struct blkmap - Block map
> + *
> + * Data associated with a blkmap.
> + *
> + * @label: Human readable name of this blkmap
> + * @blk: Underlying block device
> + * @slices: List of slices associated with this blkmap
> + */
> +struct blkmap {
> + char *label;
> + struct udevice *blk;
> + struct list_head slices;
> +};
> +
>  /**
>   * blkmap_map_linear() - Map region of other block device
>   *
> @@ -74,4 +91,16 @@ int blkmap_create(const char *label, struct udevice 
> **devp);
>   */
>  int blkmap_destroy(struct udevice *dev);
>
> +/**
> + * blkmap_create_ramdisk() - Create new ramdisk with blkmap
> + *
> + * @label: Label of the new blkmap
> + * @image_addr: Target memory start address of this mapping
> + * @image_size: Target memory size of this mapping
> + * @devp: Updated with the address of the created blkmap device
> + * Returns: 0 on success, negative error code on failure
> + */
> +int blkmap_create_ramdisk(const char *label, ulong image_addr, int 
> image_size,
> +   struct udevice **devp);
> +
>  #endif   /* _BLKMAP_H */
> --
> 2.34.1
>

Reviewed-by: Ilias Apalodimas 

Re: [PATCH 1/1] riscv: set fdtfile on VisionFive 2

[Leo Liang]

On Fri, Sep 22, 2023 at 09:54:23PM +0800, Shengyu Qu wrote:
> Hello Leo,
> 
> This patch seems only landed in next branch, not master. It is seriously
> 
> needed to make visionfive 2 working properly. Could you merge it to
> 
> master branch?
> 
> Best regards,
> 
> Shengyu
> 

Hi Shengyu, 

Got it!
Will do ASAP!

Best regards,
Leo

> > Multiple revisions of the StarFive VisionFive 2 board exist. They can be
> > identified by reading their EEPROM.
> > 
> > Linux uses two differently named device-tree files. To load the correct
> > device-tree we need to set $fdtfile to the device-tree file name that
> > matches the board revision.
> > 
> > Signed-off-by: Heinrich Schuchardt 
> > Reviewed-by: Leo Yu-Chi Liang 
> > Tested-by: Milan P. Stanić 
> > ---
> >   arch/riscv/Kconfig|  1 +
> >   .../visionfive2/starfive_visionfive2.c| 43 ++-
> >   2 files changed, 42 insertions(+), 2 deletions(-)
> > 
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 6771d8d919..1c62c2345b 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -26,6 +26,7 @@ config TARGET_SIFIVE_UNMATCHED
> >   config TARGET_STARFIVE_VISIONFIVE2
> > bool "Support StarFive VisionFive2 Board"
> > +   select BOARD_LATE_INIT
> >   config TARGET_TH1520_LPI4A
> > bool "Support Sipeed's TH1520 Lichee PI 4A Board"
> > diff --git a/board/starfive/visionfive2/starfive_visionfive2.c 
> > b/board/starfive/visionfive2/starfive_visionfive2.c
> > index d609262b67..05d8d2d657 100644
> > --- a/board/starfive/visionfive2/starfive_visionfive2.c
> > +++ b/board/starfive/visionfive2/starfive_visionfive2.c
> > @@ -5,14 +5,20 @@
> >*/
> >   #include 
> > -#include 
> > -#include 
> >   #include 
> >   #include 
> > +#include 
> > +#include 
> > +#include 
> > +#include 
> >   #include 
> >   #define JH7110_L2_PREFETCHER_BASE_ADDR0x203
> >   #define JH7110_L2_PREFETCHER_HART_OFFSET  0x2000
> > +#define FDTFILE_VISIONFIVE2_1_2A \
> > +   "starfive/jh7110-starfive-visionfive-2-v1.2a.dtb"
> > +#define FDTFILE_VISIONFIVE2_1_3B \
> > +   "starfive/jh7110-starfive-visionfive-2-v1.3b.dtb"
> >   /* enable U74-mc hart1~hart4 prefetcher */
> >   static void enable_prefetcher(void)
> > @@ -33,6 +39,31 @@ static void enable_prefetcher(void)
> > }
> >   }
> > +/**
> > + * set_fdtfile() - set the $fdtfile variable based on the board revision
> > + */
> > +static void set_fdtfile(void)
> > +{
> > +   u8 version;
> > +   const char *fdtfile;
> > +
> > +   version = get_pcb_revision_from_eeprom();
> > +   switch (version) {
> > +   case 'a':
> > +   case 'A':
> > +   fdtfile = FDTFILE_VISIONFIVE2_1_2A;
> > +   break;
> > +
> > +   case 'b':
> > +   case 'B':
> > +   default:
> > +   fdtfile = FDTFILE_VISIONFIVE2_1_3B;
> > +   break;
> > +   };
> > +
> > +   env_set("fdtfile", fdtfile);
> > +}
> > +
> >   int board_init(void)
> >   {
> > enable_caches();
> > @@ -41,6 +72,14 @@ int board_init(void)
> > return 0;
> >   }
> > +int board_late_init(void)
> > +{
> > +   if (CONFIG_IS_ENABLED(ID_EEPROM))
> > +   set_fdtfile();
> > +
> > +   return 0;
> > +}
> > +
> >   void *board_fdt_blob_setup(int *err)
> >   {
> > *err = 0;

Re: [PATCH 1/1] starfive: visionfive2: add mmc0 and nvme boot targets

[Leo Liang]

On Sat, Sep 23, 2023 at 01:53:30PM -0600, Simon Glass wrote:
> Hi Leo,
> 
> On Wed, 20 Sept 2023 at 06:46, Leo Liang  wrote:
> >
> > On Mon, Sep 18, 2023 at 10:32:29AM +0200, Milan P. Stanić wrote:
> > > boot from SDIO3.0 (mmc sdcard) first if it is plugged.
> > > If mmc is not plugged try to boot from emmc if it is plugged.
> > > If emmc is not plugged then try to boot from nvme.
> > >
> > > Signed-off-by: Milan P. Stanić 
> > > ---
> > >  include/configs/starfive-visionfive2.h | 2 ++
> > >  1 file changed, 2 insertions(+)
> >
> > Reviewed-by: Leo Yu-Chi Liang 
> 
> How about converting to standard boot and then this will be automatic?

Hi Simon,

Do you mean that we should implement this feature in a more general manner,
instead of implementing this in only on vf2 board ?

Best regards,
Leo

> 
> Regards,
> Simon

[PATCH 2/2] serial: introduce CONFIG_CONSOLE_FLUSH_ON_NEWLINE

[Rasmus Villemoes]

When debugging, one sometimes only gets partial output lines or
nothing at all from the last printf, because the uart has a largish
buffer, and the code after the printf() may cause the CPU to hang
before the uart IP has time to actually emit all the characters. That
can be very confusing, because one doesn't then know exactly where the
hang happens.

Introduce a config knob allowing one to wait for the uart fifo to
drain whenever a newline character is printed, roughly corresponding
to the effect of setvbuf(..., _IOLBF, ...) in ordinary C programs.

Signed-off-by: Rasmus Villemoes 
---
 common/Kconfig | 11 +++
 drivers/serial/serial-uclass.c |  8 ++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/common/Kconfig b/common/Kconfig
index cdb77a6a7d..49130f2a2b 100644
--- a/common/Kconfig
+++ b/common/Kconfig
@@ -224,6 +224,17 @@ config CONSOLE_FLUSH_SUPPORT
help
  This enables compilation of flush() function for console flush 
support.
 
+config CONSOLE_FLUSH_ON_NEWLINE
+   bool "Flush console buffer on every newline character"
+   depends on CONSOLE_FLUSH_SUPPORT
+   depends on DM_SERIAL
+   help
+ This makes the serial core code flush the console device
+ whenever a newline (\n) character has been emitted. This can
+ be especially useful when "printf debugging", as otherwise
+ lots of output could still be in the UART's FIFO by the time
+ one hits the code which causes the CPU to hang or reset.
+
 config CONSOLE_MUX
bool "Enable console multiplexing"
default y if VIDEO || LCD
diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index 2f75ff878c..b41e3ebe7f 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -181,7 +181,6 @@ int serial_initialize(void)
return serial_init();
 }
 
-#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
 static void _serial_flush(struct udevice *dev)
 {
struct dm_serial_ops *ops = serial_get_ops(dev);
@@ -191,7 +190,6 @@ static void _serial_flush(struct udevice *dev)
while (ops->pending(dev, false) > 0)
;
 }
-#endif
 
 static void _serial_putc(struct udevice *dev, char ch)
 {
@@ -204,6 +202,9 @@ static void _serial_putc(struct udevice *dev, char ch)
do {
err = ops->putc(dev, ch);
} while (err == -EAGAIN);
+
+   if (IS_ENABLED(CONSOLE_FLUSH_ON_NEWLINE) && ch == '\n')
+   _serial_flush(dev);
 }
 
 static int __serial_puts(struct udevice *dev, const char *str, size_t len)
@@ -242,6 +243,9 @@ static void _serial_puts(struct udevice *dev, const char 
*str)
if (*newline && __serial_puts(dev, "\r\n", 2))
return;
 
+   if (IS_ENABLED(CONSOLE_FLUSH_ON_NEWLINE) && *newline)
+   _serial_flush(dev);
+
str += len + !!*newline;
} while (*str);
 }
-- 
2.37.2

[PATCH 1/2] serial: serial-uclass.c: move definition of _serial_flush up a bit

[Rasmus Villemoes]

Preparation for next patch.

Signed-off-by: Rasmus Villemoes 
---
 drivers/serial/serial-uclass.c | 24 
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/serial/serial-uclass.c b/drivers/serial/serial-uclass.c
index 067fae2614..2f75ff878c 100644
--- a/drivers/serial/serial-uclass.c
+++ b/drivers/serial/serial-uclass.c
@@ -181,6 +181,18 @@ int serial_initialize(void)
return serial_init();
 }
 
+#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
+static void _serial_flush(struct udevice *dev)
+{
+   struct dm_serial_ops *ops = serial_get_ops(dev);
+
+   if (!ops->pending)
+   return;
+   while (ops->pending(dev, false) > 0)
+   ;
+}
+#endif
+
 static void _serial_putc(struct udevice *dev, char ch)
 {
struct dm_serial_ops *ops = serial_get_ops(dev);
@@ -234,18 +246,6 @@ static void _serial_puts(struct udevice *dev, const char 
*str)
} while (*str);
 }
 
-#ifdef CONFIG_CONSOLE_FLUSH_SUPPORT
-static void _serial_flush(struct udevice *dev)
-{
-   struct dm_serial_ops *ops = serial_get_ops(dev);
-
-   if (!ops->pending)
-   return;
-   while (ops->pending(dev, false) > 0)
-   ;
-}
-#endif
-
 static int __serial_getc(struct udevice *dev)
 {
struct dm_serial_ops *ops = serial_get_ops(dev);
-- 
2.37.2