Re: [PATCH 5/7] Makefile: Add a target for building capsules

2023-06-28 Thread Simon Glass
Hi Sughosh,

On Tue, 27 Jun 2023 at 18:42, Sughosh Ganu  wrote:
>
> hi Simon,
>
> On Tue, 27 Jun 2023 at 17:51, Simon Glass  wrote:
> >
> > Hi Sughosh,
> >
> > On Tue, 27 Jun 2023 at 13:08, Sughosh Ganu  wrote:
> > >
> > > hi Simon,
> > >
> > > On Tue, 27 Jun 2023 at 16:50, Simon Glass  wrote:
> > > >
> > > > Hi Sughosh,
> > > >
> > > > On Tue, 27 Jun 2023 at 05:57, Sughosh Ganu  
> > > > wrote:
> > > > >
> > > > > hi Simon,
> > > > >
> > > > > On Mon, 26 Jun 2023 at 17:43, Sughosh Ganu  
> > > > > wrote:
> > > > > >
> > > > > > hi Simon,
> > > > > >
> > > > > > On Mon, 26 Jun 2023 at 14:38, Simon Glass  wrote:
> > > > > > >
> > > > > > > Hi Sughosh,
> > > > > > >
> > > > > > > On Wed, 21 Jun 2023 at 05:26, Sughosh Ganu 
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > hi Simon,
> > > > > > > >
> > > > > > > > On Mon, 19 Jun 2023 at 18:07, Simon Glass  
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hi Sughosh,
> > > > > > > > >
> > > > > > > > > On Thu, 15 Jun 2023 at 17:25, Sughosh Ganu 
> > > > > > > > >  wrote:
> > > > > > > > > >
> > > > > > > > > > hi Simon,
> > > > > > > > > >
> > > > > > > > > > On Thu, 15 Jun 2023 at 14:44, Simon Glass 
> > > > > > > > > >  wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hi Sughosh,
> > > > > > > > > > >
> > > > > > > > > > > On Tue, 13 Jun 2023 at 11:39, Sughosh Ganu 
> > > > > > > > > > >  wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Add a target for building EFI capsules. The capsule 
> > > > > > > > > > > > parameters are
> > > > > > > > > > > > specified through a config file, and the path to the 
> > > > > > > > > > > > config file is
> > > > > > > > > > > > specified through CONFIG_EFI_CAPSULE_CFG_FILE. When the 
> > > > > > > > > > > > config file is
> > > > > > > > > > > > not specified, the command only builds tools.
> > > > > > > > > > > >
> > > > > > > > > > > > Signed-off-by: Sughosh Ganu 
> > > > > > > > > > > > ---
> > > > > > > > > > > >  Makefile | 9 +
> > > > > > > > > > > >  1 file changed, 9 insertions(+)
> > > > > > > > > > > >
> > > > > > > > > > > > diff --git a/Makefile b/Makefile
> > > > > > > > > > > > index 10bfaa52ad..96db29aa77 100644
> > > > > > > > > > > > --- a/Makefile
> > > > > > > > > > > > +++ b/Makefile
> > > > > > > > > > > > @@ -1151,6 +1151,15 @@ dtbs: dts/dt.dtb
> > > > > > > > > > > >  dts/dt.dtb: u-boot
> > > > > > > > > > > > $(Q)$(MAKE) $(build)=dts dtbs
> > > > > > > > > > > >
> > > > > > > > > > > > +quiet_cmd_mkeficapsule = MKEFICAPSULE $@
> > > > > > > > > > > > +cmd_mkeficapsule = $(objtree)/tools/mkeficapsule $@
> > > > > > > > > > > > +
> > > > > > > > > > > > +PHONY += capsule
> > > > > > > > > > > > +capsule: tools
> > > > > > > > > > > > +ifneq ($(CONFIG_EFI_CAPSULE_CFG_FILE),"")
> > > > > > > > > > > > +   $(call cmd,mkeficapsule)
> > > > > > > > > > > > +endif
> > > > > > > > > > > > +
> > > > > > > > > > > >  quiet_cmd_copy = COPY$@
> > > > > > > > > > > >cmd_copy = cp $< $@
> > > > > > > > > > > >
> > > > > > > > > > > > --
> > > > > > > > > > > > 2.34.1
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > We should be using binman to build images...you seem to 
> > > > > > > > > > > be building
> > > > > > > > > > > something in parallel with that. Can you please take a 
> > > > > > > > > > > look at binman?
> > > > > > > > > >
> > > > > > > > > > Again, I had explored using binman for this task. The one 
> > > > > > > > > > issue where
> > > > > > > > > > I find the above flow better is that I can simply build my 
> > > > > > > > > > payload
> > > > > > > > > > image(s) followed by 'make capsule' to generate the 
> > > > > > > > > > capsules for
> > > > > > > > > > earlier generated images. In it's current form, I don't see 
> > > > > > > > > > an easy
> > > > > > > > > > way to enforce this dependency in binman when I want to 
> > > > > > > > > > build the
> > > > > > > > > > payload followed by generation of capsules. I did see the 
> > > > > > > > > > mention of
> > > > > > > > > > encapsulating an entry within another dependent entry, but 
> > > > > > > > > > I think
> > > > > > > > > > that makes the implementation more complex than it ought to 
> > > > > > > > > > be.
> > > > > > > > > >
> > > > > > > > > > I think it is much easier to use the make flow to generate 
> > > > > > > > > > the images
> > > > > > > > > > followed by capsules, instead of tweaking the binman node 
> > > > > > > > > > to first
> > > > > > > > > > generate the payload images, followed by enabling the 
> > > > > > > > > > capsule node to
> > > > > > > > > > build the capsules. If there is an easy way of enforcing 
> > > > > > > > > > this
> > > > > > > > > > dependency, please let me know. Thanks
> > > > > > > > >
> > > > > > > > > Can you share your explorations? I think the capsule should 
> > > > > > > > > be created
> > > > > > > > > as part of the build, if enabled. Rather than changing the 
> > > > > > > > > input
> > > > > > > > > files, binm

Re: [PATCH] bootstd: USB devtype detection for script boot

2023-06-28 Thread Simon Glass
Hi John,

On Tue, 27 Jun 2023 at 15:39, John Clark  wrote:
>
> Change the device type from "usb_mass_storage" to "usb" when
> booting a script.
>
> Before this change:
>   => printenv devtype
>   devtype=usb_mass_storage
>
> After this change:
>   => printenv devtype
>   devtype=usb
>
> Signed-off-by: John Clark 
> ---
>
>  boot/bootmeth_script.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/boot/bootmeth_script.c b/boot/bootmeth_script.c
> index 225eb18ee6..9fdadb3005 100644
> --- a/boot/bootmeth_script.c
> +++ b/boot/bootmeth_script.c
> @@ -187,10 +187,14 @@ static int script_set_bootflow(struct udevice *dev, 
> struct bootflow *bflow,
>  static int script_boot(struct udevice *dev, struct bootflow *bflow)
>  {
> struct blk_desc *desc = dev_get_uclass_plat(bflow->blk);
> +   const char *devtype = blk_get_devtype(bflow->blk);
> ulong addr;
> int ret;
>
> -   ret = env_set("devtype", blk_get_devtype(bflow->blk));
> +   if (!strcmp("usb_mass_storage", devtype))

I only just thought of this, but I think it is better to check
blk->uclass_id == UCLASS_USB instead, since it avoids a string
comparison.

> +   ret = env_set("devtype", "usb");
> +   else
> +   ret = env_set("devtype", devtype);
> if (!ret)
> ret = env_set_hex("devnum", desc->devnum);
> if (!ret)
> @@ -198,7 +202,7 @@ static int script_boot(struct udevice *dev, struct 
> bootflow *bflow)
> if (!ret)
> ret = env_set("prefix", bflow->subdir);
> if (!ret && IS_ENABLED(CONFIG_ARCH_SUNXI) &&
> -   !strcmp("mmc", blk_get_devtype(bflow->blk)))
> +   !strcmp("mmc", devtype))
> ret = env_set_hex("mmc_bootdev", desc->devnum);
> if (ret)
> return log_msg_ret("env", ret);
> --
> 2.39.2
>

Regards,
Simon


[RFC PATCH] riscv: sifive: fu70: downclock CPU clock for stability

2023-06-28 Thread Icenowy Zheng
When building the package `rustc` for AOSC OS on HiFive Unmatched,
random SIGSEGV prevents the package from getting correctly built.
Downclocking the CPU PLL clock seems to allow rustc to be built,
although taking much more time.

Downclock the CPU PLL frequency for stability.

Signed-off-by: Icenowy Zheng 
---
 arch/riscv/dts/fu740-c000-u-boot.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/dts/fu740-c000-u-boot.dtsi 
b/arch/riscv/dts/fu740-c000-u-boot.dtsi
index 706224b384..6b80cab588 100644
--- a/arch/riscv/dts/fu740-c000-u-boot.dtsi
+++ b/arch/riscv/dts/fu740-c000-u-boot.dtsi
@@ -8,7 +8,7 @@
 / {
cpus {
assigned-clocks = <&prci FU740_PRCI_CLK_COREPLL>;
-   assigned-clock-rates = <12>;
+   assigned-clock-rates = <98800>;
bootph-pre-ram;
cpu0: cpu@0 {
clocks = <&prci FU740_PRCI_CLK_COREPLL>;
-- 
2.39.1



RE: [PATCH v1 01/17] arch: arm: update kconfig for new platform agilex5

2023-06-28 Thread Chee, Tien Fong
Hi Jit Loon,

> -Original Message-
> From: Lim, Jit Loon 
> Sent: Wednesday, 21 June, 2023 11:16 AM
> To: u-boot@lists.denx.de
> Cc: Jagan Teki ; Vignesh R
> ; Vasut, Marek ; Simon
> ; Chee, Tien Fong
> ; Hea, Kok Kiang ;
> Lokanathan, Raaj ; Maniyam, Dinesh
> ; Ng, Boon Khai ;
> Yuslaimi, Alif Zakuan ; Chong, Teik Heng
> ; Zamri, Muhammad Hazim Izzat
> ; Lim, Jit Loon
> ; Tang, Sieu Mun 
> Subject: [PATCH v1 01/17] arch: arm: update kconfig for new platform agilex5
> 
> This is for new platform enablement for agilex5
> 
> Signed-off-by: Jit Loon Lim 
> ---
>  arch/arm/Kconfig | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index
> 99264a6478..8e36456fa8 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1093,6 +1093,8 @@ config ARCH_SOCFPGA
>   select SPL_LIBGENERIC_SUPPORT
>   select SPL_OF_CONTROL
>   select SPL_SEPARATE_BSS if TARGET_SOCFPGA_SOC64
> + select SPL_DRIVERS_MISC if TARGET_SOCFPGA_SOC64
> + select SPL_SOCFPGA_SEC_REG if TARGET_SOCFPGA_SOC64

Please exclude these changes for now because this driver is not in mainline 
yet. You can submit another patch
for these changes once the driver is accepted into mainline.

>   select SPL_SERIAL
>   select SPL_SYSRESET
>   select SPL_WATCHDOG
> @@ -1101,7 +1103,8 @@ config ARCH_SOCFPGA
>   select SYS_THUMB_BUILD if TARGET_SOCFPGA_GEN5 ||
> TARGET_SOCFPGA_ARRIA10
>   select SYSRESET
>   select SYSRESET_SOCFPGA if TARGET_SOCFPGA_GEN5 ||
> TARGET_SOCFPGA_ARRIA10
> - select SYSRESET_SOCFPGA_SOC64 if TARGET_SOCFPGA_SOC64
> + select SYSRESET_SOCFPGA_SOC64 if !TARGET_SOCFPGA_AGILEX5
> && TARGET_SOCFPGA_SOC64
> + imply SYSRESET_SOCFPGA_AGILEX5 if TARGET_SOCFPGA_AGILEX5

Please update your commit message, your message should be clear and accurate to 
reflect your changes here.

>   imply CMD_DM
>   imply CMD_MTDPARTS
>   imply CRC32_VERIFY
> --
> 2.26.2

Thanks and regards,
Tien Fong


Re: [PATCH 5/7] Makefile: Add a target for building capsules

2023-06-28 Thread Ilias Apalodimas
[...]

> > > > > The order of operation is supposed to be:
> > > > >
> > > > > 1. Various projects used to build their ouputs (e.g. TF-A)
> > > > > 2. Makefile used to build U-Boot:
> > > > > 2a. The build produces a set of files which serve as inputs to binman 
> > > > > (INPUTS-y)
> > > > > 2b. Binman runs on INPUTS-y, picking up all the bits and creating the
> > > > > final firmware image(s)
> > > > > 3. If necessary, separate from the U-Boot build, binman can be used
> > > > > separately to do signing or whatever is needed on the final image(s)
> > > > >
> > > > > I understand that the public key is available in a CONFIG, so it
> > > > > should be possible to embed it in the build as input, either as a
> > > > > .dtsi build using 2a, or as a binary file pulled in by binman in 2b.
> > > >
> > > > Using a dtsi would mean that every platform which wants to enable
> > > > capsule authentication would need to add a signature node to it's
> > > > dtsi. Instead, is it not simpler to just generate a dtbo and merge it
> > > > with the dtb being generated. That is what was being done in V1 [1].
> > >
> > > Why is it simpler? The .dtsi is where we are supposed to put
> > > devicetree properties. It seems a lot harder (to me) to add it later.
> > > It could be a #include, or even just put it in the .dtsi if it is the
> > > same for all boards?
> >
> > I was referring to the solution in V1 of the patch series [1]. All
> > that was needed there was the path to the public key ESL file, which
> > was being provided through the CONFIG_EFI_CAPSULE_ESL_FILE. The
> > embed_capsule_key.sh would take care of doing what you are suggesting
> > to be done via dtsi. When adding a node to the dtsi, the user will
> > still have to add a node to the dtsi and include the contents of the
> > ESL file. All that was being automated through the script in V1.
>
> Instead of all that complexity, can you check the irc where Kwiboo
> describes how to add a .dtsi fragment containing that CONFIG
>
> BTW the format you are using in the dtb looks to be binary. Have you
> thought about using real properties and values like the existing
> public key mechanism? Or is that binary format required by EFI?
>
> >
> > >
> > > > For your suggestion to pull it in as a binary file in binman, that
> > > > still does not fix the issue of not changing INPUTS-y.
> > > >
> > > > If you ask me, the embedding of the public-key into the dtb is not a
> > > > task suitable for binman. Why? Because this task is actually changing
> > > > one of the INPUTS-y file that feeds into binman. And yes, we can
> > > > generate a different set of files, like u-boot-capsule.dtb and
> > > > u-boot-capsule.bin -- implementing that is not at all difficult. But
> > > > like I had highlighted earlier, and also explained by Ilias, we
> > > > already have platforms that use capsule updates and which use
> > > > u-boot.dtb and u-boot.bin. Also, platforms would not want a separate
> > >
> > > Those platforms should change, IMO. But how can this be, when this
> > > functionality has not yet been added to U-Boot?
> > >
> > > > set of files, one for normal boot, one when using capsules. So I think
> > > > it is imperative that we generate the same set of files irrespective
> > > > of whether a platform enables capsule updates. So a proper design
> > > > would be to add/embed the public key into the dtb as the dtb is
> > > > getting built. Again, this is what was being done in V1.
> > >
> > > I completely disagree with this. A capsule update is not the same as
> > > the vanilla board build / binary. Please can you just give up on this
> > > idea? Many platforms generate their output in separate files, e.g. see
> > > u-boot-rockchip.bin - it just does not make sense to change the built
> > > binary after it is built.
> >
> > I completely agree with your last statement. Which is why I said that
> > if we want to use the same files which are otherwise
> > generated(u-boot.dtb, u-boot.bin), binman is not the right way to add
> > the public key to the platform's dtb -- that should be done when the
> > dtb is being built.
>
> OK, so perhaps that is some progress.
>
> My second point (perhaps lost above) is that the capsule file should
> be called u-boot-capsule.bin or something like that, not u-boot.bin,
> since that is the output from the build system.

+CC Jonas who proposed the idea.

So one reasonable way to plug this was what Jonas and I discussed over IRC.
Instead of having a custom script injecting the 'signature' node in
the .dts we could add
- u-boot-cap-key.dtsi (or similar)
- if authenticated capsule updates are selected then
CONFIG_DEVICE_TREE_INCLUDES can be automatically selected to include
the .dts that contains the public portion of the key

That would take care of the public key inclusion in the final binary,
without going through binman since this is already working in
makefiles.   We need to add a few sanity checks.  E.g if the .esl file
is empty we need to pop a build error, b

RE: [PATCH v1 02/17] arch: arm: dts: add dts and dtsi for new platform agilex5

2023-06-28 Thread Chee, Tien Fong
Hi Jit Loon,

> -Original Message-
> From: Lim, Jit Loon 
> Sent: Wednesday, 21 June, 2023 11:16 AM
> To: u-boot@lists.denx.de
> Cc: Jagan Teki ; Vignesh R
> ; Vasut, Marek ; Simon
> ; Chee, Tien Fong
> ; Hea, Kok Kiang ;
> Lokanathan, Raaj ; Maniyam, Dinesh
> ; Ng, Boon Khai ;
> Yuslaimi, Alif Zakuan ; Chong, Teik Heng
> ; Zamri, Muhammad Hazim Izzat
> ; Lim, Jit Loon
> ; Tang, Sieu Mun 
> Subject: [PATCH v1 02/17] arch: arm: dts: add dts and dtsi for new platform
> agilex5
> 
> This is for new platform enablement for agilex5.
> Add agilex5 dtsi and dts.
> Update checkpatch error for stratix10.

Why having checkpatch error for Stratix10?
This should be in a separate patch.

> 
> Signed-off-by: Jit Loon Lim 
> ---
>  arch/arm/dts/Makefile |   1 +
>  arch/arm/dts/socfpga_agilex5-u-boot.dtsi  | 459 +
>  arch/arm/dts/socfpga_agilex5.dtsi | 634 ++
>  .../arm/dts/socfpga_agilex5_socdk-u-boot.dtsi | 131 
>  arch/arm/dts/socfpga_agilex5_socdk.dts| 165 +
>  arch/arm/dts/socfpga_soc64_fit-u-boot.dtsi|  38 +-
>  arch/arm/dts/socfpga_soc64_u-boot.dtsi| 120 
>  arch/arm/dts/socfpga_stratix10.dtsi   |   0
>  .../dts/socfpga_stratix10_socdk-u-boot.dtsi   |   0
>  arch/arm/dts/socfpga_stratix10_socdk.dts  |   0
>  10 files changed, 1534 insertions(+), 14 deletions(-)  create mode 100644
> arch/arm/dts/socfpga_agilex5-u-boot.dtsi
>  create mode 100644 arch/arm/dts/socfpga_agilex5.dtsi  create mode 100644
> arch/arm/dts/socfpga_agilex5_socdk-u-boot.dtsi
>  create mode 100644 arch/arm/dts/socfpga_agilex5_socdk.dts
>  create mode 100644 arch/arm/dts/socfpga_soc64_u-boot.dtsi
>  mode change 100755 => 100644 arch/arm/dts/socfpga_stratix10.dtsi
>  mode change 100755 => 100644 arch/arm/dts/socfpga_stratix10_socdk-u-
> boot.dtsi
>  mode change 100755 => 100644 arch/arm/dts/socfpga_stratix10_socdk.dts
> 
> diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile index
> 480269fa60..2e4bc556e1 100644
> --- a/arch/arm/dts/Makefile
> +++ b/arch/arm/dts/Makefile
> @@ -456,6 +456,7 @@ dtb-$(CONFIG_TARGET_THUNDERX_88XX) +=
> thunderx-88xx.dtb
> 
>  dtb-$(CONFIG_ARCH_SOCFPGA) +=\
>   socfpga_agilex_socdk.dtb\
> + socfpga_agilex5_socdk.dtb   \
>   socfpga_arria5_secu1.dtb\
>   socfpga_arria5_socdk.dtb\
>   socfpga_arria10_chameleonv3_270_2.dtb   \
> diff --git a/arch/arm/dts/socfpga_agilex5-u-boot.dtsi
> b/arch/arm/dts/socfpga_agilex5-u-boot.dtsi
> new file mode 100644
> index 00..6a1299901a
> --- /dev/null
> +++ b/arch/arm/dts/socfpga_agilex5-u-boot.dtsi
> @@ -0,0 +1,459 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * U-Boot additions
> + *
> + * Copyright (C) 2019-2022 Intel Corporation   */
> +
> +#include "socfpga_soc64_u-boot.dtsi"
> +#include "socfpga_soc64_fit-u-boot.dtsi"
> +
> +/{
> + memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + bootph-all;
> + };
> +
> + soc {
> + bootph-all;
> +
> + socfpga_secreg: socfpga-secreg {
> + compatible = "intel,socfpga-secreg";
> + #address-cells = <1>;
> + #size-cells = <1>;
> + bootph-all;
> +
> + /* DSU */
> + i_ccu_caiu0@1c00 {
> + reg = <0x1c00 0x1000>;
> + intel,offset-settings =
> + /* CAIUAMIGR */
> + <0x03c0 0x0003 0x001f>,
> + /* CAIUMIFSR */
> + <0x03c4 0x
> 0x07070777>,
> + /* DII1_MPFEREGS */
> + <0x0414 0x00018000 0x>,
> + <0x0418 0x 0x00ff>,
> + <0x0410 0xc0e00200 0xc1f03e1f>,
> + /* DII2_GICREGS */
> + <0x0424 0x0001d000 0x>,
> + <0x0428 0x 0x00ff>,
> + <0x0420 0xc0800400 0xc1f03e1f>,
> + /* NCAIU0_LWSOC2FPGA */
> + <0x0444 0x0002 0x>,
> + <0x0448 0x 0x00ff>,
> + <0x0440 0xc116 0xc1f03e1f>,
> + /* NCAIU0_SOC2FPGA_1G */
> + <0x0454 0x0004 0x>,
> + <0x0458 0x 0x00ff>,
> + 

RE: [PATCH v1 03/17] arch: arm: mach-socfpga: add new platform agilex5 mach-socfpga enablement

2023-06-28 Thread Chee, Tien Fong
Hi Jit Loon,

> -Original Message-
> From: Lim, Jit Loon 
> Sent: Wednesday, 21 June, 2023 11:16 AM
> To: u-boot@lists.denx.de
> Cc: Jagan Teki ; Vignesh R
> ; Vasut, Marek ; Simon
> ; Chee, Tien Fong
> ; Hea, Kok Kiang ;
> Lokanathan, Raaj ; Maniyam, Dinesh
> ; Ng, Boon Khai ;
> Yuslaimi, Alif Zakuan ; Chong, Teik Heng
> ; Zamri, Muhammad Hazim Izzat
> ; Lim, Jit Loon
> ; Tang, Sieu Mun 
> Subject: [PATCH v1 03/17] arch: arm: mach-socfpga: add new platform
> agilex5 mach-socfpga enablement
> 
> This is for new platform enablement for agilex5.
> Add platform related files to enable new product.

You should not squash all IP drivers such as clock manager, reset manager into 
one patch, this is not the right way to generate the patch, very hard for me to 
review the codes.

> 
> Signed-off-by: Jit Loon Lim 
> ---
>  arch/arm/mach-socfpga/Kconfig |  37 +++
>  arch/arm/mach-socfpga/Makefile|  69 -
>  arch/arm/mach-socfpga/board.c |  65 -
>  arch/arm/mach-socfpga/clock_manager_agilex5.c |  82 ++
>  arch/arm/mach-socfpga/firewall.c  | 107 ---
>  arch/arm/mach-socfpga/lowlevel_init_agilex5.S |  61 
>  arch/arm/mach-socfpga/lowlevel_init_soc64.S   | 167 ++-
>  arch/arm/mach-socfpga/mailbox_s10.c   |  21 ++
>  arch/arm/mach-socfpga/misc.c  |  19 +-
>  arch/arm/mach-socfpga/misc_soc64.c|  33 ++-
>  arch/arm/mach-socfpga/mmu-arm64_s10.c |  43 ++-
>  arch/arm/mach-socfpga/reset_manager_s10.c | 271 +++--
> -
>  arch/arm/mach-socfpga/secure_reg_helper.c |   4 +-
>  arch/arm/mach-socfpga/smmu_agilex5.c  |  34 +++
>  arch/arm/mach-socfpga/smmu_s10.c  | 126 
>  arch/arm/mach-socfpga/spl_agilex5.c   | 180 
>  arch/arm/mach-socfpga/spl_soc64.c | 188 +++-
>  arch/arm/mach-socfpga/u-boot-spl-soc64.lds|  93 ++
>  arch/arm/mach-socfpga/wrap_handoff_soc64.c|   7 +-
>  19 files changed, 1429 insertions(+), 178 deletions(-)
>  create mode 100644 arch/arm/mach-socfpga/clock_manager_agilex5.c
>  delete mode 100644 arch/arm/mach-socfpga/firewall.c
>  create mode 100644 arch/arm/mach-socfpga/lowlevel_init_agilex5.S
>  create mode 100644 arch/arm/mach-socfpga/smmu_agilex5.c
>  create mode 100644 arch/arm/mach-socfpga/smmu_s10.c
>  create mode 100644 arch/arm/mach-socfpga/spl_agilex5.c
>  create mode 100644 arch/arm/mach-socfpga/u-boot-spl-soc64.lds
> 
> diff --git a/arch/arm/mach-socfpga/Kconfig b/arch/arm/mach-
> socfpga/Kconfig
> index 503c82d388..562c3796ec 100644
> --- a/arch/arm/mach-socfpga/Kconfig
> +++ b/arch/arm/mach-socfpga/Kconfig
> @@ -44,6 +44,15 @@ config TEXT_BASE
>   default 0x0140 if TARGET_SOCFPGA_ARRIA10
>   default 0x0140 if TARGET_SOCFPGA_GEN5
> 
> +config ARMV8_PSCI_NR_CPUS
> + default 4 if TARGET_SOCFPGA_SOC64
> +
> +config ARMV8_SECURE_BASE
> + default 0x1000 if TARGET_SOCFPGA_SOC64 && ARMV8_PSCI
> +
> +config SYS_HAS_ARMV8_SECURE_BASE
> + default y if TARGET_SOCFPGA_SOC64 && ARMV8_PSCI
> +
>  config TARGET_SOCFPGA_AGILEX
>   bool
>   select ARMV8_MULTIENTRY
> @@ -51,10 +60,31 @@ config TARGET_SOCFPGA_AGILEX
>   select BINMAN if SPL_ATF
>   select CLK
>   select FPGA_INTEL_SDM_MAILBOX
> + select GICV2
> + select NCORE_CACHE
> + select SPL_CLK if SPL
> + select TARGET_SOCFPGA_SOC64
> +
> +config TARGET_SOCFPGA_AGILEX5
> + bool
> + select BINMAN if SPL_ATF
> + select CLK
> + select FPGA_INTEL_SDM_MAILBOX
> + select GICV3
>   select NCORE_CACHE
>   select SPL_CLK if SPL
>   select TARGET_SOCFPGA_SOC64
> 
> +config TARGET_SOCFPGA_AGILEX5_EMU
> + bool "Enable build that bootable only on Agilex5 Emulator"
> + help
> +  This is to use for Agilex5 Emulator.
> +
> +config TARGET_SOCFPGA_AGILEX5_SIMICS
> + bool "Enable build that bootable only on Agilex5 Simics platform"
> + help
> +  This is to use for Agilex5 Simics.
> +
>  config TARGET_SOCFPGA_ARRIA5
>   bool
>   select TARGET_SOCFPGA_GEN5
> @@ -126,6 +156,10 @@ config TARGET_SOCFPGA_AGILEX_SOCDK
>   bool "Intel SOCFPGA SoCDK (Agilex)"
>   select TARGET_SOCFPGA_AGILEX
> 
> +config TARGET_SOCFPGA_AGILEX5_SOCDK
> + bool "Intel SOCFPGA SoCDK (Agilex5)"
> + select TARGET_SOCFPGA_AGILEX5
> +
>  config TARGET_SOCFPGA_ARIES_MCVEVK
>   bool "Aries MCVEVK (Cyclone V)"
>   select TARGET_SOCFPGA_CYCLONE5
> @@ -199,6 +233,7 @@ config TARGET_SOCFPGA_TERASIC_SOCKIT
>  endchoice
> 
>  config SYS_BOARD
> + default "agilex5-socdk" if TARGET_SOCFPGA_AGILEX5_SOCDK
>   default "agilex-socdk" if TARGET_SOCFPGA_AGILEX_SOCDK
>   default "arria5-socdk" if TARGET_SOCFPGA_ARRIA5_SOCDK
>   default "arria10-socdk" if TARGET_SOCFPGA_ARRIA10_SOCDK
> @@ -220,6 +255,7 @@ config SYS_BOARD
>   default "vining_fpga" if TARGET_SOCFPGA_SOFTING_VINING_FPGA
> 
>  config SYS_VENDOR
> + default "i

[GIT PULL] Please pull u-boot-amlogic-next-20230628

2023-06-28 Thread Neil Armstrong

Hi Tom,

A set of changes for next release, including:
- A1 SoC support with associated reference boards
- KII Pro board support based on GXBB SoC
- New Secure power domains driver for A1 SoC

The CI job is at 
https://source.denx.de/u-boot/custodians/u-boot-amlogic/pipelines/16718

Thanks,
Neil

The following changes since commit eef4a771e85fc30a18719316a23d0ad1476ae1a5:

  Merge branch '2023-06-21-fix-get_ram_size-with-cache-enabled' into next 
(2023-06-22 09:59:43 -0400)

are available in the Git repository at:

  https://source.denx.de/u-boot/custodians/u-boot-amlogic.git 
tags/u-boot-amlogic-next-20230628

for you to fetch changes up to 58edf5773adcc95105bbd814dcbe43b1d9804391:

  drivers: meson: introduce secure power controller driver (2023-06-28 10:05:34 
+0200)


- add support for Amlogic A1 SoC and ad401 board
- add support for Videostrong KII Pro
- introduce secure power domain for A1 SoC


Alexey Romanov (3):
  arch/arm: meson: sm: introduce power domain functions
  dt-bindings: power: add Meson A1 PWRC bindings
  drivers: meson: introduce secure power controller driver

Ferass El Hafidi (3):
  arm: dts: add support for Videostrong KII Pro
  boards: amlogic: add KII Pro defconfig
  doc: boards: amlogic: add documentation for KII Pro

Igor Prusov (5):
  ARM: dts: Add Amlogic Meson A1 DT from Linux 6.3-rc7
  ARM: dts: sync meson-a1-ad401 from Linux 6.3-rc7
  ARM: meson: add A1 support
  pinctrl: meson: add pinctrl driver for Amlogic A1
  board: amlogic: add support for AD401 board

 arch/arm/dts/Makefile   |   2 +
 arch/arm/dts/meson-a1-ad401.dts |  30 +
 arch/arm/dts/meson-a1.dtsi  | 161 ++
 arch/arm/dts/meson-gxbb-kii-pro-u-boot.dtsi |  13 +
 arch/arm/dts/meson-gxbb-kii-pro.dts | 140 +
 arch/arm/include/asm/arch-meson/a1.h|  20 +
 arch/arm/include/asm/arch-meson/sm.h|  30 +
 arch/arm/mach-meson/Kconfig |   7 +
 arch/arm/mach-meson/Makefile|   1 +
 arch/arm/mach-meson/board-a1.c  |  59 ++
 arch/arm/mach-meson/sm.c|  14 +
 board/amlogic/ad401/MAINTAINERS |   6 +
 board/amlogic/ad401/Makefile|   4 +
 board/amlogic/ad401/ad401.c |  15 +
 board/amlogic/p200/MAINTAINERS  |   2 +
 configs/ad401_defconfig |  54 ++
 configs/videostrong-kii-pro_defconfig   |  70 +++
 doc/board/amlogic/index.rst |   1 +
 doc/board/amlogic/videostrong-kii-pro.rst   | 112 
 drivers/pinctrl/meson/Kconfig   |   4 +
 drivers/pinctrl/meson/Makefile  |   1 +
 drivers/pinctrl/meson/pinctrl-meson-a1.c| 867 
 drivers/power/domain/Kconfig|   7 +
 drivers/power/domain/Makefile   |   1 +
 drivers/power/domain/meson-secure-pwrc.c| 160 +
 include/configs/meson64.h   |   3 +
 include/dt-bindings/gpio/meson-a1-gpio.h|  73 +++
 include/dt-bindings/power/meson-a1-power.h  |  32 +
 28 files changed, 1889 insertions(+)
 create mode 100644 arch/arm/dts/meson-a1-ad401.dts
 create mode 100644 arch/arm/dts/meson-a1.dtsi
 create mode 100644 arch/arm/dts/meson-gxbb-kii-pro-u-boot.dtsi
 create mode 100644 arch/arm/dts/meson-gxbb-kii-pro.dts
 create mode 100644 arch/arm/include/asm/arch-meson/a1.h
 create mode 100644 arch/arm/mach-meson/board-a1.c
 create mode 100644 board/amlogic/ad401/MAINTAINERS
 create mode 100644 board/amlogic/ad401/Makefile
 create mode 100644 board/amlogic/ad401/ad401.c
 create mode 100644 configs/ad401_defconfig
 create mode 100644 configs/videostrong-kii-pro_defconfig
 create mode 100644 doc/board/amlogic/videostrong-kii-pro.rst
 create mode 100644 drivers/pinctrl/meson/pinctrl-meson-a1.c
 create mode 100644 drivers/power/domain/meson-secure-pwrc.c
 create mode 100644 include/dt-bindings/gpio/meson-a1-gpio.h
 create mode 100644 include/dt-bindings/power/meson-a1-power.h




Re: [PATCH 5/7] Makefile: Add a target for building capsules

2023-06-28 Thread Sughosh Ganu
hi Simon,

On Wed, 28 Jun 2023 at 13:12, Simon Glass  wrote:
>
> Hi Sughosh,
>
> On Tue, 27 Jun 2023 at 18:42, Sughosh Ganu  wrote:
> >
> > hi Simon,
> >
> > On Tue, 27 Jun 2023 at 17:51, Simon Glass  wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Tue, 27 Jun 2023 at 13:08, Sughosh Ganu  
> > > wrote:
> > > >
> > > > hi Simon,
> > > >
> > > > On Tue, 27 Jun 2023 at 16:50, Simon Glass  wrote:
> > > > >
> > > > > Hi Sughosh,
> > > > >
> > > > > On Tue, 27 Jun 2023 at 05:57, Sughosh Ganu  
> > > > > wrote:
> > > > > >
> > > > > > hi Simon,
> > > > > >
> > > > > > On Mon, 26 Jun 2023 at 17:43, Sughosh Ganu 
> > > > > >  wrote:
> > > > > > >
> > > > > > > hi Simon,
> > > > > > >
> > > > > > > On Mon, 26 Jun 2023 at 14:38, Simon Glass  
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > Hi Sughosh,
> > > > > > > >
> > > > > > > > On Wed, 21 Jun 2023 at 05:26, Sughosh Ganu 
> > > > > > > >  wrote:
> > > > > > > > >
> > > > > > > > > hi Simon,
> > > > > > > > >
> > > > > > > > > On Mon, 19 Jun 2023 at 18:07, Simon Glass  
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Sughosh,
> > > > > > > > > >
> > > > > > > > > > On Thu, 15 Jun 2023 at 17:25, Sughosh Ganu 
> > > > > > > > > >  wrote:
> > > > > > > > > > >
> > > > > > > > > > > hi Simon,
> > > > > > > > > > >
> > > > > > > > > > > On Thu, 15 Jun 2023 at 14:44, Simon Glass 
> > > > > > > > > > >  wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Hi Sughosh,
> > > > > > > > > > > >
> > > > > > > > > > > > On Tue, 13 Jun 2023 at 11:39, Sughosh Ganu 
> > > > > > > > > > > >  wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > Add a target for building EFI capsules. The capsule 
> > > > > > > > > > > > > parameters are
> > > > > > > > > > > > > specified through a config file, and the path to the 
> > > > > > > > > > > > > config file is
> > > > > > > > > > > > > specified through CONFIG_EFI_CAPSULE_CFG_FILE. When 
> > > > > > > > > > > > > the config file is
> > > > > > > > > > > > > not specified, the command only builds tools.
> > > > > > > > > > > > >
> > > > > > > > > > > > > Signed-off-by: Sughosh Ganu 
> > > > > > > > > > > > > ---
> > > > > > > > > > > > >  Makefile | 9 +
> > > > > > > > > > > > >  1 file changed, 9 insertions(+)
> > > > > > > > > > > > >
> > > > > > > > > > > > > diff --git a/Makefile b/Makefile
> > > > > > > > > > > > > index 10bfaa52ad..96db29aa77 100644
> > > > > > > > > > > > > --- a/Makefile
> > > > > > > > > > > > > +++ b/Makefile
> > > > > > > > > > > > > @@ -1151,6 +1151,15 @@ dtbs: dts/dt.dtb
> > > > > > > > > > > > >  dts/dt.dtb: u-boot
> > > > > > > > > > > > > $(Q)$(MAKE) $(build)=dts dtbs
> > > > > > > > > > > > >
> > > > > > > > > > > > > +quiet_cmd_mkeficapsule = MKEFICAPSULE $@
> > > > > > > > > > > > > +cmd_mkeficapsule = $(objtree)/tools/mkeficapsule $@
> > > > > > > > > > > > > +
> > > > > > > > > > > > > +PHONY += capsule
> > > > > > > > > > > > > +capsule: tools
> > > > > > > > > > > > > +ifneq ($(CONFIG_EFI_CAPSULE_CFG_FILE),"")
> > > > > > > > > > > > > +   $(call cmd,mkeficapsule)
> > > > > > > > > > > > > +endif
> > > > > > > > > > > > > +
> > > > > > > > > > > > >  quiet_cmd_copy = COPY$@
> > > > > > > > > > > > >cmd_copy = cp $< $@
> > > > > > > > > > > > >
> > > > > > > > > > > > > --
> > > > > > > > > > > > > 2.34.1
> > > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > We should be using binman to build images...you seem to 
> > > > > > > > > > > > be building
> > > > > > > > > > > > something in parallel with that. Can you please take a 
> > > > > > > > > > > > look at binman?
> > > > > > > > > > >
> > > > > > > > > > > Again, I had explored using binman for this task. The one 
> > > > > > > > > > > issue where
> > > > > > > > > > > I find the above flow better is that I can simply build 
> > > > > > > > > > > my payload
> > > > > > > > > > > image(s) followed by 'make capsule' to generate the 
> > > > > > > > > > > capsules for
> > > > > > > > > > > earlier generated images. In it's current form, I don't 
> > > > > > > > > > > see an easy
> > > > > > > > > > > way to enforce this dependency in binman when I want to 
> > > > > > > > > > > build the
> > > > > > > > > > > payload followed by generation of capsules. I did see the 
> > > > > > > > > > > mention of
> > > > > > > > > > > encapsulating an entry within another dependent entry, 
> > > > > > > > > > > but I think
> > > > > > > > > > > that makes the implementation more complex than it ought 
> > > > > > > > > > > to be.
> > > > > > > > > > >
> > > > > > > > > > > I think it is much easier to use the make flow to 
> > > > > > > > > > > generate the images
> > > > > > > > > > > followed by capsules, instead of tweaking the binman node 
> > > > > > > > > > > to first
> > > > > > > > > > > generate the payload images, followed by enabling the 
> > > > > > > > > > > capsule node to
> > > > > > > > > > > build the capsules. If there is an easy way of enforcing 
> > > > > > > >

Re: [PATCH 5/7] Makefile: Add a target for building capsules

2023-06-28 Thread Simon Glass
Hi Sughosh,

On Wed, 28 Jun 2023 at 11:00, Sughosh Ganu  wrote:
>
> hi Simon,
>
> On Wed, 28 Jun 2023 at 13:12, Simon Glass  wrote:
> >
> > Hi Sughosh,
> >
> > On Tue, 27 Jun 2023 at 18:42, Sughosh Ganu  wrote:
> > >
> > > hi Simon,
> > >
> > > On Tue, 27 Jun 2023 at 17:51, Simon Glass  wrote:
> > > >
> > > > Hi Sughosh,
> > > >
> > > > On Tue, 27 Jun 2023 at 13:08, Sughosh Ganu  
> > > > wrote:
> > > > >
> > > > > hi Simon,
> > > > >
> > > > > On Tue, 27 Jun 2023 at 16:50, Simon Glass  wrote:
> > > > > >
> > > > > > Hi Sughosh,
> > > > > >
> > > > > > On Tue, 27 Jun 2023 at 05:57, Sughosh Ganu 
> > > > > >  wrote:
> > > > > > >
> > > > > > > hi Simon,
> > > > > > >
> > > > > > > On Mon, 26 Jun 2023 at 17:43, Sughosh Ganu 
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > hi Simon,
> > > > > > > >
> > > > > > > > On Mon, 26 Jun 2023 at 14:38, Simon Glass  
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > Hi Sughosh,
> > > > > > > > >
> > > > > > > > > On Wed, 21 Jun 2023 at 05:26, Sughosh Ganu 
> > > > > > > > >  wrote:
> > > > > > > > > >
> > > > > > > > > > hi Simon,
> > > > > > > > > >
> > > > > > > > > > On Mon, 19 Jun 2023 at 18:07, Simon Glass 
> > > > > > > > > >  wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hi Sughosh,
> > > > > > > > > > >
> > > > > > > > > > > On Thu, 15 Jun 2023 at 17:25, Sughosh Ganu 
> > > > > > > > > > >  wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > hi Simon,
> > > > > > > > > > > >
> > > > > > > > > > > > On Thu, 15 Jun 2023 at 14:44, Simon Glass 
> > > > > > > > > > > >  wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > Hi Sughosh,
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Tue, 13 Jun 2023 at 11:39, Sughosh Ganu 
> > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Add a target for building EFI capsules. The capsule 
> > > > > > > > > > > > > > parameters are
> > > > > > > > > > > > > > specified through a config file, and the path to 
> > > > > > > > > > > > > > the config file is
> > > > > > > > > > > > > > specified through CONFIG_EFI_CAPSULE_CFG_FILE. When 
> > > > > > > > > > > > > > the config file is
> > > > > > > > > > > > > > not specified, the command only builds tools.
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu 
> > > > > > > > > > > > > > 
> > > > > > > > > > > > > > ---
> > > > > > > > > > > > > >  Makefile | 9 +
> > > > > > > > > > > > > >  1 file changed, 9 insertions(+)
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > diff --git a/Makefile b/Makefile
> > > > > > > > > > > > > > index 10bfaa52ad..96db29aa77 100644
> > > > > > > > > > > > > > --- a/Makefile
> > > > > > > > > > > > > > +++ b/Makefile
> > > > > > > > > > > > > > @@ -1151,6 +1151,15 @@ dtbs: dts/dt.dtb
> > > > > > > > > > > > > >  dts/dt.dtb: u-boot
> > > > > > > > > > > > > > $(Q)$(MAKE) $(build)=dts dtbs
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > +quiet_cmd_mkeficapsule = MKEFICAPSULE $@
> > > > > > > > > > > > > > +cmd_mkeficapsule = $(objtree)/tools/mkeficapsule $@
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > > +PHONY += capsule
> > > > > > > > > > > > > > +capsule: tools
> > > > > > > > > > > > > > +ifneq ($(CONFIG_EFI_CAPSULE_CFG_FILE),"")
> > > > > > > > > > > > > > +   $(call cmd,mkeficapsule)
> > > > > > > > > > > > > > +endif
> > > > > > > > > > > > > > +
> > > > > > > > > > > > > >  quiet_cmd_copy = COPY$@
> > > > > > > > > > > > > >cmd_copy = cp $< $@
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > --
> > > > > > > > > > > > > > 2.34.1
> > > > > > > > > > > > > >
> > > > > > > > > > > > >
> > > > > > > > > > > > > We should be using binman to build images...you seem 
> > > > > > > > > > > > > to be building
> > > > > > > > > > > > > something in parallel with that. Can you please take 
> > > > > > > > > > > > > a look at binman?
> > > > > > > > > > > >
> > > > > > > > > > > > Again, I had explored using binman for this task. The 
> > > > > > > > > > > > one issue where
> > > > > > > > > > > > I find the above flow better is that I can simply build 
> > > > > > > > > > > > my payload
> > > > > > > > > > > > image(s) followed by 'make capsule' to generate the 
> > > > > > > > > > > > capsules for
> > > > > > > > > > > > earlier generated images. In it's current form, I don't 
> > > > > > > > > > > > see an easy
> > > > > > > > > > > > way to enforce this dependency in binman when I want to 
> > > > > > > > > > > > build the
> > > > > > > > > > > > payload followed by generation of capsules. I did see 
> > > > > > > > > > > > the mention of
> > > > > > > > > > > > encapsulating an entry within another dependent entry, 
> > > > > > > > > > > > but I think
> > > > > > > > > > > > that makes the implementation more complex than it 
> > > > > > > > > > > > ought to be.
> > > > > > > > > > > >
> > > > > > > > > > > > I think it is much easier to use the make flow to 
> > > > > > > > > > > > gene

[PATCH 00/12] binman: Simple templating feature and mkimage conversion

2023-06-28 Thread Simon Glass
This series converts the mkimage entry type to be a section, i.e. based on
the entry_Section class. This makes it more consistent in its behaviour,
e.g. allowing symbol writing and expanded entries.

A simple templating feature is also introduced, to reduce duplication
when a set of entries must be used in multiple images.

The templating implementation works by appending the template nodes to
the target node. It is probably better to insert the template nodes
before any subnodes in the target, so that the ordering of nodes in the
template is preserved. But that involves rewriting the Fdt classs, since
it can currently only add a subnode after the existing ones. This is left
for later.


Marek Vasut (1):
  binman: Convert mkimage to Entry_section

Simon Glass (11):
  binman: Init align_default in entry_Section
  binman: Use GetEntries() to obtain section contents
  binman: Read _multiple_data_files in the correct place
  binman: Allow disabling symbol writing
  stm32mp15: Avoid writing symbols in SPL
  binman: Provide a way to specific the fdt-list directly
  binman: Drop __bss_size variable in bss_data.c
  binman: Correct handling of zero bss size
  dtoc: Support copying the contents of a node into another
  dtoc: Allow inserting a list of nodes into another
  binman: Support simple templates

 arch/arm/dts/stm32mp15-u-boot.dtsi|   1 +
 tools/binman/binman.rst   |  87 ++
 tools/binman/control.py   |   9 ++
 tools/binman/elf_test.py  |   5 ++
 tools/binman/entries.rst  |   6 ++
 tools/binman/entry.py |  10 +--
 tools/binman/etype/blob_phase.py  |   5 ++
 tools/binman/etype/fit.py |   9 ++
 tools/binman/etype/mkimage.py |  79 ++---
 tools/binman/etype/section.py |  22 ++---
 tools/binman/etype/u_boot_spl_bss_pad.py  |   2 +-
 tools/binman/etype/u_boot_tpl_bss_pad.py  |   2 +-
 tools/binman/etype/u_boot_vpl_bss_pad.py  |   2 +-
 tools/binman/ftest.py | 103 +-
 tools/binman/test/282_symbols_disable.dts |  25 ++
 tools/binman/test/283_mkimage_special.dts |  24 +
 tools/binman/test/284_fit_fdt_list.dts|  58 
 tools/binman/test/285_spl_expand.dts  |  13 +++
 tools/binman/test/286_entry_template.dts  |  42 +
 tools/binman/test/Makefile|   5 +-
 tools/binman/test/bss_data.c  |   3 +-
 tools/binman/test/bss_data_zero.c |  16 
 tools/binman/test/bss_data_zero.lds   |  15 
 tools/binman/test/embed_data.lds  |   1 +
 tools/dtoc/fdt.py |  38 
 tools/dtoc/test/dtoc_test_simple.dts  |  13 ++-
 tools/dtoc/test_fdt.py|  61 +
 27 files changed, 599 insertions(+), 57 deletions(-)
 create mode 100644 tools/binman/test/282_symbols_disable.dts
 create mode 100644 tools/binman/test/283_mkimage_special.dts
 create mode 100644 tools/binman/test/284_fit_fdt_list.dts
 create mode 100644 tools/binman/test/285_spl_expand.dts
 create mode 100644 tools/binman/test/286_entry_template.dts
 create mode 100644 tools/binman/test/bss_data_zero.c
 create mode 100644 tools/binman/test/bss_data_zero.lds

-- 
2.41.0.162.gfafddb0af9-goog



[PATCH 01/12] binman: Init align_default in entry_Section

2023-06-28 Thread Simon Glass
This should be set up in the init function, to avoid a warning about a
property not set up there. Fix it.

Signed-off-by: Simon Glass 
---

 tools/binman/etype/section.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index c36edd13508b..77250a7525c6 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -168,6 +168,7 @@ class Entry_section(Entry):
 self._end_4gb = False
 self._ignore_missing = False
 self._filename = None
+self.align_default = 0
 
 def IsSpecialSubnode(self, node):
 """Check if a node is a special one used by the section itself
-- 
2.41.0.162.gfafddb0af9-goog



[PATCH 03/12] binman: Read _multiple_data_files in the correct place

2023-06-28 Thread Simon Glass
Move this to the ReadEntries() function where it belongs.

Signed-off-by: Simon Glass 
---

 tools/binman/etype/mkimage.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
index e028c4407081..cb3e10672ad7 100644
--- a/tools/binman/etype/mkimage.py
+++ b/tools/binman/etype/mkimage.py
@@ -121,7 +121,6 @@ class Entry_mkimage(Entry):
 """
 def __init__(self, section, etype, node):
 super().__init__(section, etype, node)
-self._multiple_data_files = fdt_util.GetBool(self._node, 
'multiple-data-files')
 self._mkimage_entries = OrderedDict()
 self._imagename = None
 self._filename = fdt_util.GetString(self._node, 'filename')
@@ -129,6 +128,8 @@ class Entry_mkimage(Entry):
 
 def ReadNode(self):
 super().ReadNode()
+self._multiple_data_files = fdt_util.GetBool(self._node,
+ 'multiple-data-files')
 self._args = fdt_util.GetArgs(self._node, 'args')
 self._data_to_imagename = fdt_util.GetBool(self._node,
'data-to-imagename')
-- 
2.41.0.162.gfafddb0af9-goog



[PATCH 02/12] binman: Use GetEntries() to obtain section contents

2023-06-28 Thread Simon Glass
Some section types don't have a simple _entries list. Use the GetEntries()
method in GetEntryContents() and other places to handle this.

This makes the behaviour more consistent.

Signed-off-by: Simon Glass 
---

 tools/binman/etype/section.py | 18 +-
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py
index 77250a7525c6..d56cc11d1023 100644
--- a/tools/binman/etype/section.py
+++ b/tools/binman/etype/section.py
@@ -720,7 +720,7 @@ class Entry_section(Entry):
 next_todo.append(entry)
 return entry
 
-todo = self._entries.values()
+todo = self.GetEntries().values()
 for passnum in range(3):
 threads = state.GetThreads()
 next_todo = []
@@ -893,7 +893,7 @@ class Entry_section(Entry):
 allow_missing: True if allowed, False if not allowed
 """
 self.allow_missing = allow_missing
-for entry in self._entries.values():
+for entry in self.GetEntries().values():
 entry.SetAllowMissing(allow_missing)
 
 def SetAllowFakeBlob(self, allow_fake):
@@ -903,7 +903,7 @@ class Entry_section(Entry):
 allow_fake: True if allowed, False if not allowed
 """
 super().SetAllowFakeBlob(allow_fake)
-for entry in self._entries.values():
+for entry in self.GetEntries().values():
 entry.SetAllowFakeBlob(allow_fake)
 
 def CheckMissing(self, missing_list):
@@ -915,7 +915,7 @@ class Entry_section(Entry):
 Args:
 missing_list: List of Entry objects to be added to
 """
-for entry in self._entries.values():
+for entry in self.GetEntries().values():
 entry.CheckMissing(missing_list)
 
 def CheckFakedBlobs(self, faked_blobs_list):
@@ -926,7 +926,7 @@ class Entry_section(Entry):
 Args:
 faked_blobs_list: List of Entry objects to be added to
 """
-for entry in self._entries.values():
+for entry in self.GetEntries().values():
 entry.CheckFakedBlobs(faked_blobs_list)
 
 def CheckOptional(self, optional_list):
@@ -937,7 +937,7 @@ class Entry_section(Entry):
 Args:
 optional_list (list): List of Entry objects to be added to
 """
-for entry in self._entries.values():
+for entry in self.GetEntries().values():
 entry.CheckOptional(optional_list)
 
 def check_missing_bintools(self, missing_list):
@@ -949,7 +949,7 @@ class Entry_section(Entry):
 missing_list: List of Bintool objects to be added to
 """
 super().check_missing_bintools(missing_list)
-for entry in self._entries.values():
+for entry in self.GetEntries().values():
 entry.check_missing_bintools(missing_list)
 
 def _CollectEntries(self, entries, entries_by_name, add_entry):
@@ -999,12 +999,12 @@ class Entry_section(Entry):
 entry.Raise(f'Missing required properties/entry args: {missing}')
 
 def CheckAltFormats(self, alt_formats):
-for entry in self._entries.values():
+for entry in self.GetEntries().values():
 entry.CheckAltFormats(alt_formats)
 
 def AddBintools(self, btools):
 super().AddBintools(btools)
-for entry in self._entries.values():
+for entry in self.GetEntries().values():
 entry.AddBintools(btools)
 
 def read_elf_segments(self):
-- 
2.41.0.162.gfafddb0af9-goog



[PATCH 04/12] binman: Allow disabling symbol writing

2023-06-28 Thread Simon Glass
Some boards don't use symbol writing but do access the symbols in SPL.
Provide an option to work around this.

Signed-off-by: Simon Glass 
---

 tools/binman/binman.rst   |  7 ++
 tools/binman/entry.py |  4 +++-
 tools/binman/etype/blob_phase.py  |  5 
 tools/binman/ftest.py | 28 +++
 tools/binman/test/282_symbols_disable.dts | 25 
 5 files changed, 64 insertions(+), 5 deletions(-)
 create mode 100644 tools/binman/test/282_symbols_disable.dts

diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
index 23cbb99b4b0b..a4b31fe5397b 100644
--- a/tools/binman/binman.rst
+++ b/tools/binman/binman.rst
@@ -831,6 +831,13 @@ write-symbols:
 binman. This is automatic for certain entry types, e.g. `u-boot-spl`. See
 binman_syms_ for more information.
 
+no-write-symbols:
+Disables symbol writing for this entry. This can be used in entry types
+where symbol writing is automatic. For example, if `u-boot-spl` refers to
+the `u_boot_any_image_pos` symbol but U-Boot is not available in the image
+containing SPL, this can be used to disable the writing. Quite likely this
+indicates a bug in your setup.
+
 elf-filename:
 Sets the file name of a blob's associated ELF file. For example, if the
 blob is `zephyr.bin` then the ELF file may be `zephyr.elf`. This allows
diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 39456906a477..328b5bc568a9 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -158,6 +158,7 @@ class Entry(object):
 self.offset_from_elf = None
 self.preserve = False
 self.build_done = False
+self.no_write_symbols = False
 
 @staticmethod
 def FindEntryClass(etype, expanded):
@@ -321,6 +322,7 @@ class Entry(object):
  'offset-from-elf')
 
 self.preserve = fdt_util.GetBool(self._node, 'preserve')
+self.no_write_symbols = fdt_util.GetBool(self._node, 
'no-write-symbols')
 
 def GetDefaultFilename(self):
 return None
@@ -695,7 +697,7 @@ class Entry(object):
 Args:
   section: Section containing the entry
 """
-if self.auto_write_symbols:
+if self.auto_write_symbols and not self.no_write_symbols:
 # Check if we are writing symbols into an ELF file
 is_elf = self.GetDefaultFilename() == self.elf_fname
 elf.LookupAndWriteSymbols(self.elf_fname, self, section.GetImage(),
diff --git a/tools/binman/etype/blob_phase.py b/tools/binman/etype/blob_phase.py
index b937158756fd..951d9934050e 100644
--- a/tools/binman/etype/blob_phase.py
+++ b/tools/binman/etype/blob_phase.py
@@ -52,3 +52,8 @@ class Entry_blob_phase(Entry_section):
 
 # Read entries again, now that we have some
 self.ReadEntries()
+
+# Propagate the no-write-symbols property
+if self.no_write_symbols:
+for entry in self._entries.values():
+entry.no_write_symbols = True
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 43b4f850a691..dabb3f689fdb 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -1452,7 +1452,7 @@ class TestFunctional(unittest.TestCase):
 self.assertEqual(U_BOOT_SPL_NODTB_DATA, 
data[:len(U_BOOT_SPL_NODTB_DATA)])
 
 def checkSymbols(self, dts, base_data, u_boot_offset, entry_args=None,
- use_expanded=False):
+ use_expanded=False, no_write_symbols=False):
 """Check the image contains the expected symbol values
 
 Args:
@@ -1481,9 +1481,14 @@ class TestFunctional(unittest.TestCase):
 sym_values = struct.pack(';
+   #size-cells = <1>;
+
+   binman {
+   pad-byte = <0xff>;
+   u-boot-spl {
+   no-write-symbols;
+   };
+
+   u-boot {
+   offset = <0x38>;
+   no-expanded;
+   };
+
+   u-boot-spl2 {
+   type = "u-boot-spl";
+   no-write-symbols;
+   };
+   };
+};
-- 
2.41.0.162.gfafddb0af9-goog



[PATCH 05/12] stm32mp15: Avoid writing symbols in SPL

2023-06-28 Thread Simon Glass
These boards use SPL in a mkimage entry and apparently access the symbol
containing the image position of U-Boot, but put U-Boot in another
image. This means that binman is unable to fill in the symbol correctly
in the SPL binary.

This doesn't matter at present since mkimage doesn't support symbol
writing. But with the upcoming conversion to a section, it will. So add
a property to disable symbol writing.

Signed-off-by: Simon Glass 
---

 arch/arm/dts/stm32mp15-u-boot.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/dts/stm32mp15-u-boot.dtsi 
b/arch/arm/dts/stm32mp15-u-boot.dtsi
index d872c6fc5679..573dd4d3ed56 100644
--- a/arch/arm/dts/stm32mp15-u-boot.dtsi
+++ b/arch/arm/dts/stm32mp15-u-boot.dtsi
@@ -226,6 +226,7 @@
mkimage {
args = "-T stm32image -a 0x2ffc2500 -e 0x2ffc2500";
u-boot-spl {
+   no-write-symbols;
};
};
};
-- 
2.41.0.162.gfafddb0af9-goog



[PATCH 06/12] binman: Convert mkimage to Entry_section

2023-06-28 Thread Simon Glass
From: Marek Vasut 

This is needed to handle mkimage with inner section located itself in a
section.

Signed-off-by: Marek Vasut 
Use BuildSectionData() instead of ObtainContents(), add tests and a few
other minor fixes:
Signed-off-by: Simon Glass 
---

 tools/binman/entry.py |  6 +-
 tools/binman/etype/mkimage.py | 76 ++-
 tools/binman/ftest.py | 45 +-
 tools/binman/test/283_mkimage_special.dts | 24 +++
 4 files changed, 117 insertions(+), 34 deletions(-)
 create mode 100644 tools/binman/test/283_mkimage_special.dts

diff --git a/tools/binman/entry.py b/tools/binman/entry.py
index 328b5bc568a9..8f06fea51ad4 100644
--- a/tools/binman/entry.py
+++ b/tools/binman/entry.py
@@ -1311,10 +1311,8 @@ features to produce new behaviours.
 """
 data = b''
 for entry in entries:
-# First get the input data and put it in a file. If not available,
-# try later.
-if not entry.ObtainContents(fake_size=fake_size):
-return None, None, None
+# First get the input data and put it in a file
+entry.ObtainContents(fake_size=fake_size)
 data += entry.GetData()
 uniq = self.GetUniqueName()
 fname = tools.get_output_filename(f'{prefix}.{uniq}')
diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
index cb3e10672ad7..8311fed59762 100644
--- a/tools/binman/etype/mkimage.py
+++ b/tools/binman/etype/mkimage.py
@@ -8,10 +8,11 @@
 from collections import OrderedDict
 
 from binman.entry import Entry
+from binman.etype.section import Entry_section
 from dtoc import fdt_util
 from u_boot_pylib import tools
 
-class Entry_mkimage(Entry):
+class Entry_mkimage(Entry_section):
 """Binary produced by mkimage
 
 Properties / Entry arguments:
@@ -121,10 +122,8 @@ class Entry_mkimage(Entry):
 """
 def __init__(self, section, etype, node):
 super().__init__(section, etype, node)
-self._mkimage_entries = OrderedDict()
 self._imagename = None
-self._filename = fdt_util.GetString(self._node, 'filename')
-self.align_default = None
+self._multiple_data_files = False
 
 def ReadNode(self):
 super().ReadNode()
@@ -135,41 +134,60 @@ class Entry_mkimage(Entry):
'data-to-imagename')
 if self._data_to_imagename and self._node.FindNode('imagename'):
 self.Raise('Cannot use both imagename node and data-to-imagename')
-self.ReadEntries()
 
 def ReadEntries(self):
 """Read the subnodes to find out what should go in this image"""
 for node in self._node.subnodes:
-entry = Entry.Create(self, node)
+if self.IsSpecialSubnode(node):
+continue
+entry = Entry.Create(self, node,
+ expanded=self.GetImage().use_expanded,
+ missing_etype=self.GetImage().missing_etype)
 entry.ReadNode()
+entry.SetPrefix(self._name_prefix)
 if entry.name == 'imagename':
 self._imagename = entry
 else:
-self._mkimage_entries[entry.name] = entry
+self._entries[entry.name] = entry
 
-def ObtainContents(self):
+def BuildSectionData(self, required):
+"""Build mkimage entry contents
+
+Runs mkimage to build the entry contents
+
+Args:
+required (bool): True if the data must be present, False if it is 
OK
+to return None
+
+Returns:
+bytes: Contents of the section
+"""
 # Use a non-zero size for any fake files to keep mkimage happy
 # Note that testMkimageImagename() relies on this 'mkimage' parameter
 fake_size = 1024
 if self._multiple_data_files:
 fnames = []
 uniq = self.GetUniqueName()
-for entry in self._mkimage_entries.values():
-if not entry.ObtainContents(fake_size=fake_size):
-return False
-if entry._pathname:
-fnames.append(entry._pathname)
+for entry in self._entries.values():
+entry.ObtainContents(fake_size=fake_size)
+
+# If this is a section, put the contents in a temporary file.
+# Otherwise, assume it is a blob and use the pathname
+if isinstance(entry, Entry_section):
+ename = f'mkimage-in-{uniq}-{entry.name}'
+fname = tools.get_output_filename(ename)
+tools.write_file(fname, entry.data)
+elif entry._pathname:
+fname = entry._pathname
+fnames.append(fname)
 input_fname = ":".join(fnames)
+data = b''
 else:
 data, inp

[PATCH 07/12] binman: Provide a way to specific the fdt-list directly

2023-06-28 Thread Simon Glass
Sometimes multiple boards are built with binman and it is useful to
specify a different FDT list for each. At present this is not possible
without providing multiple values of the of-list entryarg (which is not
supported in the U-Boot build system).

Allow a fit,fdt-list-val string-list property to be used instead.

Signed-off-by: Simon Glass 
---

 tools/binman/entries.rst   |  6 +++
 tools/binman/etype/fit.py  |  9 
 tools/binman/ftest.py  | 14 ++-
 tools/binman/test/284_fit_fdt_list.dts | 58 ++
 4 files changed, 86 insertions(+), 1 deletion(-)
 create mode 100644 tools/binman/test/284_fit_fdt_list.dts

diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
index b71af801fdad..b55f424620a3 100644
--- a/tools/binman/entries.rst
+++ b/tools/binman/entries.rst
@@ -615,6 +615,12 @@ The top-level 'fit' node supports the following special 
properties:
 `of-list` meaning that `-a of-list="dtb1 dtb2..."` should be passed
 to binman.
 
+fit,fdt-list-val
+As an alternative to fit,fdt-list the list of device tree files
+can be provided in this property as a string list, e.g.::
+
+fit,fdt-list-val = "dtb1", "dtb2";
+
 Substitutions
 ~
 
diff --git a/tools/binman/etype/fit.py b/tools/binman/etype/fit.py
index c395706ece5f..ef4d0667578d 100644
--- a/tools/binman/etype/fit.py
+++ b/tools/binman/etype/fit.py
@@ -81,6 +81,12 @@ class Entry_fit(Entry_section):
 `of-list` meaning that `-a of-list="dtb1 dtb2..."` should be passed
 to binman.
 
+fit,fdt-list-val
+As an alternative to fit,fdt-list the list of device tree files
+can be provided in this property as a string list, e.g.::
+
+fit,fdt-list-val = "dtb1", "dtb2";
+
 Substitutions
 ~
 
@@ -361,6 +367,9 @@ class Entry_fit(Entry_section):
 [EntryArg(self._fit_list_prop.value, str)])
 if fdts is not None:
 self._fdts = fdts.split()
+else:
+self._fdts = fdt_util.GetStringList(self._node, 'fit,fdt-list-val')
+
 self._fit_default_dt = self.GetEntryArgsOrProps([EntryArg('default-dt',
   str)])[0]
 
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 6d0ffda2f432..54691c420733 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -6739,6 +6739,18 @@ fdt fdtmapExtract the devicetree 
blob from the fdtmap
 # Just check that the data appears in the file somewhere
 self.assertIn(U_BOOT_DATA, data)
 
+def testFitFdtList(self):
+"""Test an image with an FIT with the fit,fdt-list-val option"""
+entry_args = {
+'default-dt': 'test-fdt2',
+}
+data = self._DoReadFileDtb(
+'284_fit_fdt_list.dts',
+entry_args=entry_args,
+extra_indirs=[os.path.join(self._indir, TEST_FDT_SUBDIR)])[0]
+self.assertEqual(U_BOOT_NODTB_DATA, data[-len(U_BOOT_NODTB_DATA):])
+fit_data = data[len(U_BOOT_DATA):-len(U_BOOT_NODTB_DATA)]
+
 
-if __name__ == "_s_main__":
+if __name__ == "__main__":
 unittest.main()
diff --git a/tools/binman/test/284_fit_fdt_list.dts 
b/tools/binman/test/284_fit_fdt_list.dts
new file mode 100644
index ..8885313f5b88
--- /dev/null
+++ b/tools/binman/test/284_fit_fdt_list.dts
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   binman {
+   u-boot {
+   };
+   fit {
+   description = "test-desc";
+   #address-cells = <1>;
+   fit,fdt-list-val = "test-fdt1", "test-fdt2";
+
+   images {
+   kernel {
+   description = "Vanilla Linux kernel";
+   type = "kernel";
+   arch = "ppc";
+   os = "linux";
+   compression = "gzip";
+   load = <>;
+   entry = <>;
+   hash-1 {
+   algo = "crc32";
+   };
+   hash-2 {
+   algo = "sha1";
+   };
+   u-boot {
+   };
+   };
+   @fdt-SEQ {
+   description = "fdt-NAME.dtb";
+   type = "flat_dt";
+   

[PATCH 10/12] dtoc: Support copying the contents of a node into another

2023-06-28 Thread Simon Glass
This permits implementation of a simple templating system, where a node
can be reused as a base for others.

For now this adds new subnodes after any existing ones.

Signed-off-by: Simon Glass 
---

 tools/dtoc/fdt.py| 22 ++
 tools/dtoc/test/dtoc_test_simple.dts |  3 ++
 tools/dtoc/test_fdt.py   | 43 
 3 files changed, 68 insertions(+)

diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py
index a8e05349a720..fcf229f83036 100644
--- a/tools/dtoc/fdt.py
+++ b/tools/dtoc/fdt.py
@@ -13,6 +13,7 @@ from dtoc import fdt_util
 import libfdt
 from libfdt import QUIET_NOTFOUND
 from u_boot_pylib import tools
+from u_boot_pylib import tout
 
 # This deals with a device tree, presenting it as an assortment of Node and
 # Prop objects, representing nodes and properties, respectively. This file
@@ -635,6 +636,27 @@ class Node:
 prop.Sync(auto_resize)
 return added
 
+def copy_node(self, src):
+"""Copy a node and all its subnodes into this node
+
+Args:
+src (Node): Node to copy
+
+This works recursively.
+
+The new node is put after all other nodes. If the node already
+exists, just its properties are copied. Properties which exist in the
+destination node already are not copied.
+"""
+dst = self.FindNode(src.name)
+if not dst:
+dst = self.AddSubnode(src.name)
+for name, src_prop in src.props.items():
+if name not in dst.props:
+dst.props[name] = Prop(dst, None, name, src_prop.bytes)
+for node in src.subnodes:
+dst.copy_node(node)
+
 
 class Fdt:
 """Provides simple access to a flat device tree blob using libfdts.
diff --git a/tools/dtoc/test/dtoc_test_simple.dts 
b/tools/dtoc/test/dtoc_test_simple.dts
index 08f667ee5a10..c51f1a5908ce 100644
--- a/tools/dtoc/test/dtoc_test_simple.dts
+++ b/tools/dtoc/test/dtoc_test_simple.dts
@@ -45,6 +45,9 @@
stringarray = "one";
longbytearray = [09 0a 0b 0c 0d 0e 0f 10];
maybe-empty-int = <1>;
+
+   first-node {
+   };
};
 
i2c@0 {
diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
index 4fe8d12c403a..5d9d99eb384b 100755
--- a/tools/dtoc/test_fdt.py
+++ b/tools/dtoc/test_fdt.py
@@ -306,6 +306,49 @@ class TestNode(unittest.TestCase):
 self.assertIn("Internal error, node '/spl-test' name mismatch 'i2c@0'",
   str(exc.exception))
 
+def test_copy_node(self):
+"""Test copy_node() function"""
+tmpl = self.dtb.GetNode('/i2c@0')
+dst = self.dtb.GetNode('/spl-test3')
+dst.copy_node(tmpl)
+
+self.assertEqual(['/spl-test3/first-node', '/spl-test3/i2c@0'],
+ [n.path for n in dst.subnodes])
+
+chk = self.dtb.GetNode('/spl-test3/i2c@0')
+self.assertTrue(chk)
+self.assertEqual(
+{'bootph-all', 'compatible', '#address-cells', '#size-cells'},
+chk.props.keys())
+
+# Check the first property
+prop = chk.props['bootph-all']
+self.assertEqual('bootph-all', prop.name)
+self.assertEqual(True, prop.value)
+self.assertIsNone(prop._offset)
+self.assertEqual(chk.path, prop._node.path)
+
+# Check the second property
+prop = chk.props['compatible']
+self.assertEqual('compatible', prop.name)
+self.assertEqual('sandbox,i2c', prop.value)
+self.assertIsNone(prop._offset)
+self.assertEqual(chk.path, prop._node.path)
+
+pmic = chk.FindNode('pmic@9')
+self.assertTrue(chk)
+
+pmic = self.dtb.GetNode('/spl-test3/i2c@0/pmic@9')
+self.assertTrue(pmic)
+self.assertEqual([pmic], chk.subnodes)
+self.assertEqual(chk, pmic.parent)
+self.assertIsNone(pmic._offset)
+self.assertEqual(
+{'bootph-all', 'compatible', 'reg', 'low-power'},
+pmic.props.keys())
+
+self.dtb.Sync(auto_resize=True)
+
 
 class TestProp(unittest.TestCase):
 """Test operation of the Prop class"""
-- 
2.41.0.162.gfafddb0af9-goog



[PATCH 08/12] binman: Drop __bss_size variable in bss_data.c

2023-06-28 Thread Simon Glass
This is not needed since the linker script sets it up. Drop the variable
to avoid confusion.

Fix the prototype for main() while we are here.

Signed-off-by: Simon Glass 
---

 tools/binman/test/bss_data.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/tools/binman/test/bss_data.c b/tools/binman/test/bss_data.c
index 4f9b64cef9ef..7047a3bb014d 100644
--- a/tools/binman/test/bss_data.c
+++ b/tools/binman/test/bss_data.c
@@ -7,9 +7,8 @@
  */
 
 int bss_data[10];
-int __bss_size = sizeof(bss_data);
 
-int main()
+int main(void)
 {
bss_data[2] = 2;
 
-- 
2.41.0.162.gfafddb0af9-goog



[PATCH 11/12] dtoc: Allow inserting a list of nodes into another

2023-06-28 Thread Simon Glass
Provide a way to specify a phandle list of nodes which are to be inserted
into an existing node.

Signed-off-by: Simon Glass 
---

 tools/dtoc/fdt.py| 16 
 tools/dtoc/test/dtoc_test_simple.dts | 10 --
 tools/dtoc/test_fdt.py   | 18 ++
 3 files changed, 42 insertions(+), 2 deletions(-)

diff --git a/tools/dtoc/fdt.py b/tools/dtoc/fdt.py
index fcf229f83036..4ff55a47b6f1 100644
--- a/tools/dtoc/fdt.py
+++ b/tools/dtoc/fdt.py
@@ -657,6 +657,22 @@ class Node:
 for node in src.subnodes:
 dst.copy_node(node)
 
+def copy_subnodes_from_phandles(self, phandle_list):
+"""Copy subnodes of a list of nodes into another node
+
+Args:
+phandle_list (list of int): List of phandles of nodes to copy
+
+For each node in the phandle list, its subnodes and their properties 
are
+copied recursively. Note that it does not copy the node itself, nor its
+properties.
+"""
+for phandle in phandle_list:
+parent = self.GetFdt().LookupPhandle(phandle)
+tout.debug(f'adding template {parent.path} to node {self.path}')
+for node in parent.subnodes:
+self.copy_node(node)
+
 
 class Fdt:
 """Provides simple access to a flat device tree blob using libfdts.
diff --git a/tools/dtoc/test/dtoc_test_simple.dts 
b/tools/dtoc/test/dtoc_test_simple.dts
index c51f1a5908ce..f7ad445574d2 100644
--- a/tools/dtoc/test/dtoc_test_simple.dts
+++ b/tools/dtoc/test/dtoc_test_simple.dts
@@ -50,7 +50,7 @@
};
};
 
-   i2c@0 {
+   i2c: i2c@0 {
compatible = "sandbox,i2c";
bootph-all;
#address-cells = <1>;
@@ -63,10 +63,16 @@
};
};
 
-   orig-node {
+   orig: orig-node {
orig = <1 23 4>;
args = "-n first", "second", "-p", "123,456", "-x";
args2 = "a space", "there";
args3 = "-n first second -p 123,456 -x";
+
+   copy-list = <&i2c &orig>;
+
+   subnode {
+   a-prop;
+   };
};
 };
diff --git a/tools/dtoc/test_fdt.py b/tools/dtoc/test_fdt.py
index 5d9d99eb384b..6d96270d539e 100755
--- a/tools/dtoc/test_fdt.py
+++ b/tools/dtoc/test_fdt.py
@@ -349,6 +349,24 @@ class TestNode(unittest.TestCase):
 
 self.dtb.Sync(auto_resize=True)
 
+def test_copy_subnodes_from_phandles(self):
+"""Test copy_node() function"""
+pmic = self.dtb.GetNode('/spl-test3/i2c@0/pmic@9')
+self.assertIsNone(pmic)
+
+orig = self.dtb.GetNode('/orig-node')
+node_list = fdt_util.GetPhandleList(orig, 'copy-list')
+
+dst = self.dtb.GetNode('/spl-test3')
+dst.copy_subnodes_from_phandles(node_list)
+
+pmic = self.dtb.GetNode('/spl-test3/pmic@9')
+self.assertTrue(pmic)
+
+subn = self.dtb.GetNode('/spl-test3/subnode')
+self.assertTrue(subn)
+self.assertEqual({'a-prop'}, subn.props.keys())
+
 
 class TestProp(unittest.TestCase):
 """Test operation of the Prop class"""
-- 
2.41.0.162.gfafddb0af9-goog



[PATCH 09/12] binman: Correct handling of zero bss size

2023-06-28 Thread Simon Glass
Fix the check for the __bss_size symbol, since it may be 0. Unfortunately
there was no test coverage for this.

Signed-off-by: Simon Glass 
---

 tools/binman/elf_test.py |  5 +
 tools/binman/etype/u_boot_spl_bss_pad.py |  2 +-
 tools/binman/etype/u_boot_tpl_bss_pad.py |  2 +-
 tools/binman/etype/u_boot_vpl_bss_pad.py |  2 +-
 tools/binman/ftest.py| 12 
 tools/binman/test/285_spl_expand.dts | 13 +
 tools/binman/test/Makefile   |  5 -
 tools/binman/test/bss_data_zero.c| 16 
 tools/binman/test/bss_data_zero.lds  | 15 +++
 tools/binman/test/embed_data.lds |  1 +
 10 files changed, 69 insertions(+), 4 deletions(-)
 create mode 100644 tools/binman/test/285_spl_expand.dts
 create mode 100644 tools/binman/test/bss_data_zero.c
 create mode 100644 tools/binman/test/bss_data_zero.lds

diff --git a/tools/binman/elf_test.py b/tools/binman/elf_test.py
index c98083961b53..84aa493663c3 100644
--- a/tools/binman/elf_test.py
+++ b/tools/binman/elf_test.py
@@ -369,6 +369,11 @@ class TestElf(unittest.TestCase):
 elf.GetSymbolOffset(fname, 'embed')
 self.assertIn('__image_copy_start', str(e.exception))
 
+def test_get_symbol_address(self):
+fname = self.ElfTestFile('embed_data')
+addr = elf.GetSymbolAddress(fname, 'region_size')
+self.assertEqual(0, addr)
+
 
 if __name__ == '__main__':
 unittest.main()
diff --git a/tools/binman/etype/u_boot_spl_bss_pad.py 
b/tools/binman/etype/u_boot_spl_bss_pad.py
index 1ffeb3911fd8..4af4045d3702 100644
--- a/tools/binman/etype/u_boot_spl_bss_pad.py
+++ b/tools/binman/etype/u_boot_spl_bss_pad.py
@@ -38,7 +38,7 @@ class Entry_u_boot_spl_bss_pad(Entry_blob):
 def ObtainContents(self):
 fname = tools.get_input_filename('spl/u-boot-spl')
 bss_size = elf.GetSymbolAddress(fname, '__bss_size')
-if not bss_size:
+if bss_size is None:
 self.Raise('Expected __bss_size symbol in spl/u-boot-spl')
 self.SetContents(tools.get_bytes(0, bss_size))
 return True
diff --git a/tools/binman/etype/u_boot_tpl_bss_pad.py 
b/tools/binman/etype/u_boot_tpl_bss_pad.py
index 29c6a9541296..46d2cd58f7e2 100644
--- a/tools/binman/etype/u_boot_tpl_bss_pad.py
+++ b/tools/binman/etype/u_boot_tpl_bss_pad.py
@@ -38,7 +38,7 @@ class Entry_u_boot_tpl_bss_pad(Entry_blob):
 def ObtainContents(self):
 fname = tools.get_input_filename('tpl/u-boot-tpl')
 bss_size = elf.GetSymbolAddress(fname, '__bss_size')
-if not bss_size:
+if bss_size is None:
 self.Raise('Expected __bss_size symbol in tpl/u-boot-tpl')
 self.SetContents(tools.get_bytes(0, bss_size))
 return True
diff --git a/tools/binman/etype/u_boot_vpl_bss_pad.py 
b/tools/binman/etype/u_boot_vpl_bss_pad.py
index bba38ccf9e93..12b286a71987 100644
--- a/tools/binman/etype/u_boot_vpl_bss_pad.py
+++ b/tools/binman/etype/u_boot_vpl_bss_pad.py
@@ -38,7 +38,7 @@ class Entry_u_boot_vpl_bss_pad(Entry_blob):
 def ObtainContents(self):
 fname = tools.get_input_filename('vpl/u-boot-vpl')
 bss_size = elf.GetSymbolAddress(fname, '__bss_size')
-if not bss_size:
+if bss_size is None:
 self.Raise('Expected __bss_size symbol in vpl/u-boot-vpl')
 self.SetContents(tools.get_bytes(0, bss_size))
 return True
diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
index 54691c420733..4db54c69682c 100644
--- a/tools/binman/ftest.py
+++ b/tools/binman/ftest.py
@@ -6751,6 +6751,18 @@ fdt fdtmapExtract the devicetree 
blob from the fdtmap
 self.assertEqual(U_BOOT_NODTB_DATA, data[-len(U_BOOT_NODTB_DATA):])
 fit_data = data[len(U_BOOT_DATA):-len(U_BOOT_NODTB_DATA)]
 
+def testSplEmptyBss(self):
+"""Test an expanded SPL with a zero-size BSS"""
+# ELF file with a '__bss_size' symbol
+self._SetupSplElf(src_fname='bss_data_zero')
+
+entry_args = {
+'spl-bss-pad': 'y',
+'spl-dtb': 'y',
+}
+data = self._DoReadFileDtb('285_spl_expand.dts',
+   use_expanded=True, entry_args=entry_args)[0]
+
 
 if __name__ == "__main__":
 unittest.main()
diff --git a/tools/binman/test/285_spl_expand.dts 
b/tools/binman/test/285_spl_expand.dts
new file mode 100644
index ..9c88ccb287b1
--- /dev/null
+++ b/tools/binman/test/285_spl_expand.dts
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/dts-v1/;
+
+/ {
+   #address-cells = <1>;
+   #size-cells = <1>;
+
+   binman {
+   u-boot-spl {
+   };
+   };
+};
diff --git a/tools/binman/test/Makefile b/tools/binman/test/Makefile
index cd66a3038be2..4d152eee9c09 100644
--- a/tools/binman/test/Makefile
+++ b/tools/binman/test/Makefile
@@ -32,7 +32,7 @@ LDS_BINMAN_EMBED := -T $(SRC)u_boot_binman_embed.lds
 LDS_EFL_

[PATCH 12/12] binman: Support simple templates

2023-06-28 Thread Simon Glass
Collections can used to collect the contents of other entries into a
single entry, but they result in a single entry, with the original entries
'left behind' in their old place.

It is useful to be able to specific a set of entries ones and have it used
in multiple images, or parts of an image.

Implement this mechanism.

Signed-off-by: Simon Glass 
---

 tools/binman/binman.rst  | 80 
 tools/binman/control.py  |  9 +++
 tools/binman/etype/section.py|  3 +-
 tools/binman/ftest.py|  8 +++
 tools/binman/test/286_entry_template.dts | 42 +
 5 files changed, 141 insertions(+), 1 deletion(-)
 create mode 100644 tools/binman/test/286_entry_template.dts

diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
index a4b31fe5397b..9be979ae1c5b 100644
--- a/tools/binman/binman.rst
+++ b/tools/binman/binman.rst
@@ -727,6 +727,13 @@ optional:
 Note that missing, optional blobs do not produce a non-zero exit code from
 binman, although it does show a warning about the missing external blob.
 
+insert-template:
+This is not strictly speaking an entry property, since it is processed 
early
+in Binman before the entries are read. It is a list of phandles of nodes to
+include in the current (target) node. For each node, its subnodes and their
+properties are brought into the target node. See Templates_ below for
+more information.
+
 The attributes supported for images and sections are described below. Several
 are similar to those for entries.
 
@@ -1172,6 +1179,79 @@ If you are having trouble figuring out what is going on, 
you can use
  arch/arm/dts/u-boot.dtsi ... found: "arch/arm/dts/juno-r2-u-boot.dtsi"
 
 
+Templates
+=
+
+Sometimes multiple images need to be created which have all have a common
+part. For example, a board may generate SPI and eMMC images which both include
+a FIT. Since the FIT includes many entries, it is tedious to repeat them twice
+in the image description.
+
+Templates provide a simple way to handle this::
+
+binman {
+multiple-images;
+common_part: template-1 {
+fit {
+... lots of entries in here
+};
+
+text {
+text = "base image";
+};
+};
+
+spi-image {
+filename = "image-spi.bin";
+insert-template = <&fit>;
+
+/* things specific to SPI follow */
+header {
+];
+
+text {
+text = "SPI image";
+};
+};
+
+mmc-image {
+filename = "image-mmc.bin";
+insert-template = <&fit>;
+
+/* things specific to MMC follow */
+header {
+];
+
+text {
+text = "MMC image";
+};
+};
+};
+
+The template node name must start with 'template', so it is not considered to 
be
+an image itself.
+
+The mechanism is very simple. For each phandle in the 'insert-templates'
+property, the source node is looked up. Then the subnodes of that source node
+are copied into the target node, i.e. the one containing the `insert-template`
+property.
+
+If the target node has a node with the same name as a template, its properties
+override corresponding properties in the template. This allows the template to
+be uses as a base, with the node providing updates to the properties as needed.
+The overriding happens recursively.
+
+At present there is an unpleasant limitation on this feature: it works by
+appending the template nodes after any existing subnodes to the target node.
+This means that if the target node includes any subnodes, these appear in order
+before the template node. In the above example, 'header' becomes the first
+subnode of each image, followed by `fit` and `text`. If this is not what is
+desired, there is no way to adjust it.
+
+Note: The above limitation will likely be removed in future, so that the
+template subnodes appear before the target subnodes.
+
+
 Updating an ELF file
 
 
diff --git a/tools/binman/control.py b/tools/binman/control.py
index 68597c4e7792..e7faca78e9aa 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -22,6 +22,7 @@ from binman import bintool
 from binman import cbfs_util
 from binman import elf
 from binman import entry
+from dtoc import fdt_util
 from u_boot_pylib import command
 from u_boot_pylib import tools
 from u_boot_pylib import tout
@@ -478,6 +479,12 @@ def SignEntries(image_fname, input_fname, 
privatekey_fname, algo, entry_paths,
 
 AfterReplace(image, allow_resize=True, write_map=write_map)
 
+def _ProcessTemplates(parent):
+for node in parent.subnodes:
+tmpl = fdt_util.GetPhandleList(node, 'insert-template')
+if tmpl:
+node.copy_subnodes_from_phandles(tmpl)
+
 def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt, us

Re: [PATCH 5/7] Makefile: Add a target for building capsules

2023-06-28 Thread Sughosh Ganu
hi Simon,

On Wed, 28 Jun 2023 at 15:49, Simon Glass  wrote:
>
> Hi Sughosh,
>
> On Wed, 28 Jun 2023 at 11:00, Sughosh Ganu  wrote:
> >
> > hi Simon,
> >
> > On Wed, 28 Jun 2023 at 13:12, Simon Glass  wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Tue, 27 Jun 2023 at 18:42, Sughosh Ganu  
> > > wrote:
> > > >
> > > > hi Simon,
> > > >
> > > > On Tue, 27 Jun 2023 at 17:51, Simon Glass  wrote:
> > > > >
> > > > > Hi Sughosh,
> > > > >
> > > > > On Tue, 27 Jun 2023 at 13:08, Sughosh Ganu  
> > > > > wrote:
> > > > > >
> > > > > > hi Simon,
> > > > > >
> > > > > > On Tue, 27 Jun 2023 at 16:50, Simon Glass  wrote:
> > > > > > >
> > > > > > > Hi Sughosh,
> > > > > > >
> > > > > > > On Tue, 27 Jun 2023 at 05:57, Sughosh Ganu 
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > hi Simon,
> > > > > > > >
> > > > > > > > On Mon, 26 Jun 2023 at 17:43, Sughosh Ganu 
> > > > > > > >  wrote:
> > > > > > > > >
> > > > > > > > > hi Simon,
> > > > > > > > >
> > > > > > > > > On Mon, 26 Jun 2023 at 14:38, Simon Glass  
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > Hi Sughosh,
> > > > > > > > > >
> > > > > > > > > > On Wed, 21 Jun 2023 at 05:26, Sughosh Ganu 
> > > > > > > > > >  wrote:
> > > > > > > > > > >
> > > > > > > > > > > hi Simon,
> > > > > > > > > > >
> > > > > > > > > > > On Mon, 19 Jun 2023 at 18:07, Simon Glass 
> > > > > > > > > > >  wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Hi Sughosh,
> > > > > > > > > > > >
> > > > > > > > > > > > On Thu, 15 Jun 2023 at 17:25, Sughosh Ganu 
> > > > > > > > > > > >  wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > hi Simon,
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Thu, 15 Jun 2023 at 14:44, Simon Glass 
> > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Hi Sughosh,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Tue, 13 Jun 2023 at 11:39, Sughosh Ganu 
> > > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Add a target for building EFI capsules. The 
> > > > > > > > > > > > > > > capsule parameters are
> > > > > > > > > > > > > > > specified through a config file, and the path to 
> > > > > > > > > > > > > > > the config file is
> > > > > > > > > > > > > > > specified through CONFIG_EFI_CAPSULE_CFG_FILE. 
> > > > > > > > > > > > > > > When the config file is
> > > > > > > > > > > > > > > not specified, the command only builds tools.
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu 
> > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > ---
> > > > > > > > > > > > > > >  Makefile | 9 +
> > > > > > > > > > > > > > >  1 file changed, 9 insertions(+)
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > diff --git a/Makefile b/Makefile
> > > > > > > > > > > > > > > index 10bfaa52ad..96db29aa77 100644
> > > > > > > > > > > > > > > --- a/Makefile
> > > > > > > > > > > > > > > +++ b/Makefile
> > > > > > > > > > > > > > > @@ -1151,6 +1151,15 @@ dtbs: dts/dt.dtb
> > > > > > > > > > > > > > >  dts/dt.dtb: u-boot
> > > > > > > > > > > > > > > $(Q)$(MAKE) $(build)=dts dtbs
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > +quiet_cmd_mkeficapsule = MKEFICAPSULE $@
> > > > > > > > > > > > > > > +cmd_mkeficapsule = $(objtree)/tools/mkeficapsule 
> > > > > > > > > > > > > > > $@
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > +PHONY += capsule
> > > > > > > > > > > > > > > +capsule: tools
> > > > > > > > > > > > > > > +ifneq ($(CONFIG_EFI_CAPSULE_CFG_FILE),"")
> > > > > > > > > > > > > > > +   $(call cmd,mkeficapsule)
> > > > > > > > > > > > > > > +endif
> > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > >  quiet_cmd_copy = COPY$@
> > > > > > > > > > > > > > >cmd_copy = cp $< $@
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > --
> > > > > > > > > > > > > > > 2.34.1
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > We should be using binman to build images...you 
> > > > > > > > > > > > > > seem to be building
> > > > > > > > > > > > > > something in parallel with that. Can you please 
> > > > > > > > > > > > > > take a look at binman?
> > > > > > > > > > > > >
> > > > > > > > > > > > > Again, I had explored using binman for this task. The 
> > > > > > > > > > > > > one issue where
> > > > > > > > > > > > > I find the above flow better is that I can simply 
> > > > > > > > > > > > > build my payload
> > > > > > > > > > > > > image(s) followed by 'make capsule' to generate the 
> > > > > > > > > > > > > capsules for
> > > > > > > > > > > > > earlier generated images. In it's current form, I 
> > > > > > > > > > > > > don't see an easy
> > > > > > > > > > > > > way to enforce this dependency in binman when I want 
> > > > > > > > > > > > > to build the
> > > > > > > > > > > > > payload followed by generation of capsules. I did see 
> > > > > > > > > > > > > the mention of
> > > > > > > > 

[PATCH] usb: dwc3-generic: Ensure reset GPIO is configured as an output

2023-06-28 Thread Peter Korsgaard
GPIOD_ACTIVE_LOW is not enough to configure a GPIO as an output, we need
GPIOD_IS_OUT as well.

Fixes: b252d79b0936d60b ("usb: dwc3: Add support to reset usb ULPI phy")
Signed-off-by: Peter Korsgaard 
---
 drivers/usb/dwc3/dwc3-generic.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/dwc3-generic.c b/drivers/usb/dwc3/dwc3-generic.c
index 66da5a8d6f..35e4b36a69 100644
--- a/drivers/usb/dwc3/dwc3-generic.c
+++ b/drivers/usb/dwc3/dwc3-generic.c
@@ -105,7 +105,7 @@ static int dwc3_generic_probe(struct udevice *dev,
if (CONFIG_IS_ENABLED(DM_GPIO) &&
device_is_compatible(dev->parent, "xlnx,zynqmp-dwc3")) {
priv->ulpi_reset = devm_gpiod_get_optional(dev->parent, "reset",
-   
GPIOD_ACTIVE_LOW);
+  GPIOD_IS_OUT | 
GPIOD_ACTIVE_LOW);
/* property is optional, don't return error! */
if (priv->ulpi_reset) {
/* Toggle ulpi to reset the phy. */
-- 
2.30.2



Trying to boot JH7110 RISCV-V CPU from MMC

2023-06-28 Thread Roland Ruckerbauer

Hi!

I am trying to use upstream u-boot + opensbi, to boot my visionfive2 SBC 
I got from external SD card. I had not much luck with the vendor
provided images / tools, they seem to have a lot of hard coded stuff and 
just won`t work correctly.


With upstream u-boot I followed the doc/board/starfive/visionfive2.rst 
to the point, but unfortunately it did not work.


First I ran into an issue, where the chip would pick up a random SPL 
somewhere else on the SD card, and so I could not test my build.
I fixed it by completely erasing the whole sdcard to make sure there can 
be no other SPL/Uboot on it. This worked, but unfortunately
the sd card I made with the steps from 
doc/board/starfive/visionfive2.rst still did not work.


I managed to figure out, that I need to make special modifications to 
the first 2 sectors of the sd card (protective MBR and GPT header), using
https://github.com/starfive-tech/Tools/tree/master/spl_tool. Calling 
spl_tool -i -f /dev/sdb on my sdcard patches the first sectors with an 
invalid
SPL, and also a offset to the backup SPL (which is the primary one I 
flashed according to the documentation).


Maybe this should be added to the docu, otherwise it will not work, or 
worse, load some other SPL also on the SD card and cause confusion.
There also is a mention in the docu, that the boot ROM searches for the 
SPL by looking for the offset of its partition in the GPT with a specific
GUID. Not sure where this information comes from, but testing showed, 
that its probably not true.



Now to my actual problem, hopefully someone can help:

The sdcard I built with u-boot and opensbi can now boot the spl, and 
also start opensbi and load u-boot. Unfortunately
the init_sequence_r now fails with an error: initcall sequence 
fffe0738 failed at call 40216240 (err=-19)


Digging through the source and some printf debugging revealed to me, 
that initr_dm_devices() fails, because it can not find

a timer device with in dm_timer_init() call.

For the JH7110 and pretty much any other RISCV chip the timer device 
should be provided by the SBI, which is functioning correctly as far as
I can tell. I searched for it in the u-boot code, and figured out that 
riscv_timer uboot driver should be probed, when the booting cpu is bound to

its driver in the riscv_cpu_bind() function.

Unfortunately it seems like this cpu driver bind function is never 
called before the initcall sequence crashed because of the missing timer 
device.


That is pretty much everything I could figure out on my own until now, 
but I am not sure what exactly is going wrong.
Hopefully it has nothing to do with how my local setup compiles the 
code. u-boot seems to be using many tricks like the U_BOOT_DRIVER() macro
and others, maybe some of them do not guarantee to create entries in a 
specific order, and the code accidentally depends on a specific order.


Maybe somebody can give me some pointers, I would appreciate it.

Greetings,
Roland Ruckerbauer



[PATCH] clk: starfive: pll: Fix to use postdiv1_mask

2023-06-28 Thread Hoegeun Kwon
There is a problem that the rates of PLL0 and PLL1 are set incorrectly
because the postdiv1_mask value is incorrectly entered when setting
the pll clk reg. Modify postdiv1's mask value to be put correctly.

Signed-off-by: Hoegeun Kwon 
---
 drivers/clk/starfive/clk-jh7110-pll.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/starfive/clk-jh7110-pll.c 
b/drivers/clk/starfive/clk-jh7110-pll.c
index 02e6d9000e29..7492b1f70dd4 100644
--- a/drivers/clk/starfive/clk-jh7110-pll.c
+++ b/drivers/clk/starfive/clk-jh7110-pll.c
@@ -185,7 +185,7 @@ static void jh7110_pll_set_rate(struct clk_jh7110_pllx *pll,
PLLX_SET(pll->offset->dsmpd, pll->offset->dsmpd_mask, 1);
PLLX_SET(pll->offset->prediv, pll->offset->prediv_mask, rate->prediv);
PLLX_SET(pll->offset->fbdiv, pll->offset->fbdiv_mask, rate->fbdiv);
-   PLLX_SET(pll->offset->postdiv1, pll->offset->postdiv1, 0);
+   PLLX_SET(pll->offset->postdiv1, pll->offset->postdiv1_mask, 0);
PLLX_SET(pll->offset->pd, pll->offset->pd_mask, PLL_PD_ON);
 
if (set) {
-- 
2.17.1



Re: [PATCH 5/7] Makefile: Add a target for building capsules

2023-06-28 Thread Simon Glass
Hi Sughosh,

On Wed, 28 Jun 2023 at 13:25, Sughosh Ganu  wrote:
>
> hi Simon,
>
> On Wed, 28 Jun 2023 at 15:49, Simon Glass  wrote:
> >
> > Hi Sughosh,
> >
> > On Wed, 28 Jun 2023 at 11:00, Sughosh Ganu  wrote:
> > >
> > > hi Simon,
> > >
> > > On Wed, 28 Jun 2023 at 13:12, Simon Glass  wrote:
> > > >
> > > > Hi Sughosh,
> > > >
> > > > On Tue, 27 Jun 2023 at 18:42, Sughosh Ganu  
> > > > wrote:
> > > > >
> > > > > hi Simon,
> > > > >
> > > > > On Tue, 27 Jun 2023 at 17:51, Simon Glass  wrote:
> > > > > >
> > > > > > Hi Sughosh,
> > > > > >
> > > > > > On Tue, 27 Jun 2023 at 13:08, Sughosh Ganu 
> > > > > >  wrote:
> > > > > > >
> > > > > > > hi Simon,
> > > > > > >
> > > > > > > On Tue, 27 Jun 2023 at 16:50, Simon Glass  
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > Hi Sughosh,
> > > > > > > >
> > > > > > > > On Tue, 27 Jun 2023 at 05:57, Sughosh Ganu 
> > > > > > > >  wrote:
> > > > > > > > >
> > > > > > > > > hi Simon,
> > > > > > > > >
> > > > > > > > > On Mon, 26 Jun 2023 at 17:43, Sughosh Ganu 
> > > > > > > > >  wrote:
> > > > > > > > > >
> > > > > > > > > > hi Simon,
> > > > > > > > > >
> > > > > > > > > > On Mon, 26 Jun 2023 at 14:38, Simon Glass 
> > > > > > > > > >  wrote:
> > > > > > > > > > >
> > > > > > > > > > > Hi Sughosh,
> > > > > > > > > > >
> > > > > > > > > > > On Wed, 21 Jun 2023 at 05:26, Sughosh Ganu 
> > > > > > > > > > >  wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > hi Simon,
> > > > > > > > > > > >
> > > > > > > > > > > > On Mon, 19 Jun 2023 at 18:07, Simon Glass 
> > > > > > > > > > > >  wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > Hi Sughosh,
> > > > > > > > > > > > >
> > > > > > > > > > > > > On Thu, 15 Jun 2023 at 17:25, Sughosh Ganu 
> > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > hi Simon,
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > On Thu, 15 Jun 2023 at 14:44, Simon Glass 
> > > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > Hi Sughosh,
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > On Tue, 13 Jun 2023 at 11:39, Sughosh Ganu 
> > > > > > > > > > > > > > >  wrote:
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Add a target for building EFI capsules. The 
> > > > > > > > > > > > > > > > capsule parameters are
> > > > > > > > > > > > > > > > specified through a config file, and the path 
> > > > > > > > > > > > > > > > to the config file is
> > > > > > > > > > > > > > > > specified through CONFIG_EFI_CAPSULE_CFG_FILE. 
> > > > > > > > > > > > > > > > When the config file is
> > > > > > > > > > > > > > > > not specified, the command only builds tools.
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu 
> > > > > > > > > > > > > > > > 
> > > > > > > > > > > > > > > > ---
> > > > > > > > > > > > > > > >  Makefile | 9 +
> > > > > > > > > > > > > > > >  1 file changed, 9 insertions(+)
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > diff --git a/Makefile b/Makefile
> > > > > > > > > > > > > > > > index 10bfaa52ad..96db29aa77 100644
> > > > > > > > > > > > > > > > --- a/Makefile
> > > > > > > > > > > > > > > > +++ b/Makefile
> > > > > > > > > > > > > > > > @@ -1151,6 +1151,15 @@ dtbs: dts/dt.dtb
> > > > > > > > > > > > > > > >  dts/dt.dtb: u-boot
> > > > > > > > > > > > > > > > $(Q)$(MAKE) $(build)=dts dtbs
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > +quiet_cmd_mkeficapsule = MKEFICAPSULE $@
> > > > > > > > > > > > > > > > +cmd_mkeficapsule = 
> > > > > > > > > > > > > > > > $(objtree)/tools/mkeficapsule $@
> > > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > > +PHONY += capsule
> > > > > > > > > > > > > > > > +capsule: tools
> > > > > > > > > > > > > > > > +ifneq ($(CONFIG_EFI_CAPSULE_CFG_FILE),"")
> > > > > > > > > > > > > > > > +   $(call cmd,mkeficapsule)
> > > > > > > > > > > > > > > > +endif
> > > > > > > > > > > > > > > > +
> > > > > > > > > > > > > > > >  quiet_cmd_copy = COPY$@
> > > > > > > > > > > > > > > >cmd_copy = cp $< $@
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > > --
> > > > > > > > > > > > > > > > 2.34.1
> > > > > > > > > > > > > > > >
> > > > > > > > > > > > > > >
> > > > > > > > > > > > > > > We should be using binman to build images...you 
> > > > > > > > > > > > > > > seem to be building
> > > > > > > > > > > > > > > something in parallel with that. Can you please 
> > > > > > > > > > > > > > > take a look at binman?
> > > > > > > > > > > > > >
> > > > > > > > > > > > > > Again, I had explored using binman for this task. 
> > > > > > > > > > > > > > The one issue where
> > > > > > > > > > > > > > I find the above flow better is that I can simply 
> > > > > > > > > > > > > > build my payload
> > > > > > > > > > > > > > image(s) followed by 'make capsule' to generate the 
> > > > > > > > > > > > > > capsules for
> > > > > > > > > > > > > > earlier generated images. In i

Re: [PATCH 1/2] disk: part: Add API to get partitions with specific driver

2023-06-28 Thread Joshua Watt
On Mon, Jun 26, 2023 at 4:07 AM Simon Glass  wrote:
>
> Hi Joshua,
>
> On Fri, 23 Jun 2023 at 21:01, Joshua Watt  wrote:
> >
> > Adds part_driver_get_type() API which can be used to force a specific
> > driver to be used when getting partition information instead of relying
> > on auto detection.
> >
> > Signed-off-by: Joshua Watt 
> > ---
> >  disk/part.c| 38 +++---
> >  include/part.h |  2 ++
> >  2 files changed, 33 insertions(+), 7 deletions(-)
> >
> > diff --git a/disk/part.c b/disk/part.c
> > index 35300df590..1f8c786ca5 100644
> > --- a/disk/part.c
> > +++ b/disk/part.c
> > @@ -26,6 +26,22 @@
> >  /* Check all partition types */
> >  #define PART_TYPE_ALL  -1
> >
> > +static struct part_driver *part_driver_get_type(int part_type)
> > +{
> > +   struct part_driver *drv =
> > +   ll_entry_start(struct part_driver, part_driver);
> > +   const int n_ents = ll_entry_count(struct part_driver, part_driver);
> > +   struct part_driver *entry;
> > +
> > +   for (entry = drv; entry != drv + n_ents; entry++) {
> > +   if (part_type == entry->part_type)
> > +   return entry;
> > +   }
> > +
> > +   /* Not found */
> > +   return NULL;
> > +}
> > +
> >  static struct part_driver *part_driver_lookup_type(struct blk_desc 
> > *dev_desc)
> >  {
> > struct part_driver *drv =
> > @@ -44,10 +60,7 @@ static struct part_driver 
> > *part_driver_lookup_type(struct blk_desc *dev_desc)
> > }
> > }
> > } else {
> > -   for (entry = drv; entry != drv + n_ents; entry++) {
> > -   if (dev_desc->part_type == entry->part_type)
> > -   return entry;
> > -   }
> > +   return part_driver_get_type(dev_desc->part_type);
> > }
> >
> > /* Not found */
> > @@ -306,8 +319,8 @@ void part_print(struct blk_desc *dev_desc)
> > drv->print(dev_desc);
> >  }
> >
> > -int part_get_info(struct blk_desc *dev_desc, int part,
> > -  struct disk_partition *info)
> > +int part_get_info_by_type(struct blk_desc *dev_desc, int part, int 
> > part_type,
> > + struct disk_partition *info)
> >  {
> > struct part_driver *drv;
> >
> > @@ -320,7 +333,12 @@ int part_get_info(struct blk_desc *dev_desc, int part,
> > info->type_guid[0] = 0;
> >  #endif
> >
> > -   drv = part_driver_lookup_type(dev_desc);
> > +   if (part_type == PART_TYPE_UNKNOWN) {
> > +   drv = part_driver_lookup_type(dev_desc);
> > +   } else {
> > +   drv = part_driver_get_type(part_type);
> > +   }
> > +
> > if (!drv) {
> > debug("## Unknown partition table type %x\n",
> >   dev_desc->part_type);
> > @@ -340,6 +358,12 @@ int part_get_info(struct blk_desc *dev_desc, int part,
> > return -ENOENT;
> >  }
> >
> > +int part_get_info(struct blk_desc *dev_desc, int part,
> > + struct disk_partition *info)
> > +{
> > +   return part_get_info_by_type(dev_desc, part, PART_TYPE_UNKNOWN, 
> > info);
> > +}
> > +
> >  int part_get_info_whole_disk(struct blk_desc *dev_desc,
> >  struct disk_partition *info)
> >  {
> > diff --git a/include/part.h b/include/part.h
> > index be75c73549..f150c84206 100644
> > --- a/include/part.h
> > +++ b/include/part.h
> > @@ -106,6 +106,8 @@ struct blk_desc *blk_get_dev(const char *ifname, int 
> > dev);
> >  struct blk_desc *mg_disk_get_dev(int dev);
> >
> >  /* disk/part.c */
> > +int part_get_info_by_type(struct blk_desc *dev_desc, int part, int 
> > part_type,
> > + struct disk_partition *info);
>
> Please can you add a full comment?
>
> Also please add a test to test/dm/part.c for your new function[1]

Any trick to getting the current test/dm/part.c tests to pass? When I
run them they fail with `** No device specified **` errors, which
doesn't give me hope that I can write tests that I could actually run
locally.

>
> >  int part_get_info(struct blk_desc *dev_desc, int part,
> >   struct disk_partition *info);
> >  /**
> > --
> > 2.33.0
> >
>
> Regards,
> Simon


Re: [PATCH 2/5] rockchip: rk3399-pinebook-pro: Fix SPL max size and SPI flash payload offset

2023-06-28 Thread Quentin Schulz

Hi Jonas,

On 6/27/23 21:10, Jonas Karlman wrote:

TPL max size is limited to 184 KB, SPL is loaded to 0x0 and TF-A is
loaded to 0x4, this limit SPL max size to 256 KB. With BootRom only
reading first 2 KB per 4 KB page of SPI flash, 880 KB may be needed for
TPL+SPL in a worst-case scenario. (184 KB + 256 KB) x 2 = 880 KB

Use 0xE (896 KB) as the payload offset in SPI flash, this allows
for a payload of 3168 KB before env offset start to overlap.

Also add CONFIG_ROCKCHIP_SPI_IMAGE=y to build a bootable SPI flash
image, u-boot-rockchip-spi.bin.

Signed-off-by: Jonas Karlman 
---
  arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi | 4 
  configs/pinebook-pro-rk3399_defconfig| 4 +++-
  2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi 
b/arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi
index ea7a5a17ae0f..88a77cad8d43 100644
--- a/arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi
+++ b/arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi
@@ -10,10 +10,6 @@
chosen {
u-boot,spl-boot-order = "same-as-spl", &sdhci, &spiflash, 
&sdmmc;
};
-
-   config {
-   u-boot,spl-payload-offset = <0x6>; /* @ 384KB */
-   };


Just a nitpick/question (and for the whole series): Is there any 
specific reason for going back to the Kconfig way 
(CONFIG_SYS_SPI_U_BOOT_OFFS) instead of just modifying the offset in the 
-u-boot DTSI?


On a different note, that also means that we're effectively breaking 
current setups by moving the environment some other location. I cannot 
talk for the maintainer(s) and user(s) but I thought it was worth 
mentioning.



  };
  
  &edp {

diff --git a/configs/pinebook-pro-rk3399_defconfig 
b/configs/pinebook-pro-rk3399_defconfig
index 58a8b91aa60f..1109ebb7387b 100644
--- a/configs/pinebook-pro-rk3399_defconfig
+++ b/configs/pinebook-pro-rk3399_defconfig
@@ -12,6 +12,7 @@ CONFIG_ENV_OFFSET=0x3F8000
  CONFIG_DEFAULT_DEVICE_TREE="rk3399-pinebook-pro"
  CONFIG_DM_RESET=y
  CONFIG_ROCKCHIP_RK3399=y
+CONFIG_ROCKCHIP_SPI_IMAGE=y
  CONFIG_TARGET_PINEBOOK_PRO_RK3399=y
  CONFIG_SPL_STACK=0x40
  CONFIG_DEBUG_UART_BASE=0xFF1A
@@ -26,7 +27,7 @@ CONFIG_USE_PREBOOT=y
  CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-pinebook-pro.dtb"
  CONFIG_DISPLAY_BOARDINFO_LATE=y
  CONFIG_MISC_INIT_R=y
-CONFIG_SPL_MAX_SIZE=0x2e000
+CONFIG_SPL_MAX_SIZE=0x4


nitpick: We *could* have this in another commit after we move the 
environment to another location later in the SPI flash.


Cheers,
Quentin


Re: [PATCH v3] dt-bindings: riscv: deprecate riscv,isa

2023-06-28 Thread Anup Patel
On Wed, Jun 28, 2023 at 7:46 PM Palmer Dabbelt  wrote:
>
> On Tue, 27 Jun 2023 08:49:06 PDT (-0700), Palmer Dabbelt wrote:
> > On Mon, 26 Jun 2023 23:52:06 PDT (-0700), apa...@ventanamicro.com wrote:
> >> On Tue, Jun 27, 2023 at 1:23 AM Palmer Dabbelt  wrote:
> >>>
> >>> On Mon, 26 Jun 2023 10:38:43 PDT (-0700), apa...@ventanamicro.com wrote:
> >>> > On Mon, Jun 26, 2023 at 3:42 PM Conor Dooley 
> >>> >  wrote:
> >>> >>
> >>> >> intro
> >>> >> =
> >>> >>
> >>> >> When the RISC-V dt-bindings were accepted upstream in Linux, the base
> >>> >> ISA etc had yet to be ratified. By the ratification of the base ISA,
> >>> >> incompatible changes had snuck into the specifications - for example 
> >>> >> the
> >>> >> Zicsr and Zifencei extensions were spun out of the base ISA.
> >>> >>
> >>> >> Fast forward to today, and the reason for this patch.
> >>> >> Currently the riscv,isa dt property permits only a specific subset of
> >>> >> the ISA string - in particular it excludes version numbering.
> >>> >> With the current constraints, it is not possible to discern whether
> >>> >> "rv64i" means that the hart supports the fence.i instruction, for
> >>> >> example.
> >>> >> Future systems may choose to implement their own instruction fencing,
> >>> >> perhaps using a vendor extension, or they may not implement the 
> >>> >> optional
> >>> >> counter extensions. Software needs a way to determine this.
> >>> >>
> >>> >> versioning schemes
> >>> >> ==
> >>> >>
> >>> >> "Use the extension versions that are described in the ISA manual" you
> >>> >> may say, and it's not like this has not been considered.
> >>> >> Firstly, software that parses the riscv,isa property at runtime will
> >>> >> need to contain a lookup table of some sort that maps arbitrary 
> >>> >> versions
> >>> >> to versions it understands. There is not a consistent application of
> >>> >> version number applied to extensions, with a higgledy-piggledy
> >>> >> collection of tags, "bare" and versioned documents awaiting the reader
> >>> >> on the "recently ratified extensions" page:
> >>> >> https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions
> >>> >>
> >>> >> As an aside, and this is reflected in the patch too, since many
> >>> >> extensions have yet to appear in a release of the ISA specs,
> >>> >> they are defined by commits in their respective "working draft"
> >>> >> repositories.
> >>> >>
> >>> >> Secondly, there is an issue of backwards compatibility, whereby 
> >>> >> allowing
> >>> >> numbers in the ISA string, some parsers may be broken. This would
> >>> >> require an additional property to be created to even use the versions 
> >>> >> in
> >>> >> this manner.
> >>> >>
> >>> >> ~boolean properties~ string array property
> >>> >> ==
> >>> >>
> >>> >> If a new property is needed, the whole approach may as well be looked 
> >>> >> at
> >>> >> from the bottom up. A string with limited character choices etc is
> >>> >> hardly the best approach for communicating extension information to
> >>> >> software.
> >>> >>
> >>> >> Switching to using properties that are defined on a per extension 
> >>> >> basis,
> >>> >> allows us to define explicit meanings for the DT representation of each
> >>> >> extension - rather than the current situation where different operating
> >>> >> systems or other bits of software may impart different meanings to
> >>> >> characters in the string.
> >>> >> Clearly the best source of meanings is the specifications themselves,
> >>> >> this just provides us the ability to choose at what point in time the
> >>> >> meaning is set. If an extension changes incompatibility in the future,
> >>> >> a new property will be required.
> >>> >>
> >>> >> Off-list, some of the RVI folks have committed to shoring up the 
> >>> >> wording
> >>> >> in either the ISA specifications, the riscv-isa-manual or
> >>> >> so that in the future, modifications to and additions or removals of
> >>> >> features will require a new extension. Codifying that assertion
> >>> >> somewhere would make it quite unlikely that compatibility would be
> >>> >> broken, but we have the tools required to deal with it, if & when it
> >>> >> crops up.
> >>> >> It is in our collective interest, as consumers of extension meanings, 
> >>> >> to
> >>> >> define a scheme that enforces compatibility.
> >>> >>
> >>> >> The use of individual properties, rather than elements in a single
> >>> >> string, will also permit validation that the properties have a meaning,
> >>> >> as well as potentially reject mutually exclusive combinations, or
> >>> >> enforce dependencies between extensions. That would not have be 
> >>> >> possible
> >>> >> with the current dt-schema infrastructure for arbitrary strings, as we
> >>> >> would need to add a riscv,isa parser to dt-validate!
> >>> >> That's not implemented in this patch, but rather left as future work 
> >>> >> (for
> >>> >> the brave, or 

Re: [PATCH v3] dt-bindings: riscv: deprecate riscv,isa

2023-06-28 Thread Palmer Dabbelt
On Tue, 27 Jun 2023 08:49:06 PDT (-0700), Palmer Dabbelt wrote:
> On Mon, 26 Jun 2023 23:52:06 PDT (-0700), apa...@ventanamicro.com wrote:
>> On Tue, Jun 27, 2023 at 1:23 AM Palmer Dabbelt  wrote:
>>>
>>> On Mon, 26 Jun 2023 10:38:43 PDT (-0700), apa...@ventanamicro.com wrote:
>>> > On Mon, Jun 26, 2023 at 3:42 PM Conor Dooley 
>>> >  wrote:
>>> >>
>>> >> intro
>>> >> =
>>> >>
>>> >> When the RISC-V dt-bindings were accepted upstream in Linux, the base
>>> >> ISA etc had yet to be ratified. By the ratification of the base ISA,
>>> >> incompatible changes had snuck into the specifications - for example the
>>> >> Zicsr and Zifencei extensions were spun out of the base ISA.
>>> >>
>>> >> Fast forward to today, and the reason for this patch.
>>> >> Currently the riscv,isa dt property permits only a specific subset of
>>> >> the ISA string - in particular it excludes version numbering.
>>> >> With the current constraints, it is not possible to discern whether
>>> >> "rv64i" means that the hart supports the fence.i instruction, for
>>> >> example.
>>> >> Future systems may choose to implement their own instruction fencing,
>>> >> perhaps using a vendor extension, or they may not implement the optional
>>> >> counter extensions. Software needs a way to determine this.
>>> >>
>>> >> versioning schemes
>>> >> ==
>>> >>
>>> >> "Use the extension versions that are described in the ISA manual" you
>>> >> may say, and it's not like this has not been considered.
>>> >> Firstly, software that parses the riscv,isa property at runtime will
>>> >> need to contain a lookup table of some sort that maps arbitrary versions
>>> >> to versions it understands. There is not a consistent application of
>>> >> version number applied to extensions, with a higgledy-piggledy
>>> >> collection of tags, "bare" and versioned documents awaiting the reader
>>> >> on the "recently ratified extensions" page:
>>> >> https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions
>>> >>
>>> >> As an aside, and this is reflected in the patch too, since many
>>> >> extensions have yet to appear in a release of the ISA specs,
>>> >> they are defined by commits in their respective "working draft"
>>> >> repositories.
>>> >>
>>> >> Secondly, there is an issue of backwards compatibility, whereby allowing
>>> >> numbers in the ISA string, some parsers may be broken. This would
>>> >> require an additional property to be created to even use the versions in
>>> >> this manner.
>>> >>
>>> >> ~boolean properties~ string array property
>>> >> ==
>>> >>
>>> >> If a new property is needed, the whole approach may as well be looked at
>>> >> from the bottom up. A string with limited character choices etc is
>>> >> hardly the best approach for communicating extension information to
>>> >> software.
>>> >>
>>> >> Switching to using properties that are defined on a per extension basis,
>>> >> allows us to define explicit meanings for the DT representation of each
>>> >> extension - rather than the current situation where different operating
>>> >> systems or other bits of software may impart different meanings to
>>> >> characters in the string.
>>> >> Clearly the best source of meanings is the specifications themselves,
>>> >> this just provides us the ability to choose at what point in time the
>>> >> meaning is set. If an extension changes incompatibility in the future,
>>> >> a new property will be required.
>>> >>
>>> >> Off-list, some of the RVI folks have committed to shoring up the wording
>>> >> in either the ISA specifications, the riscv-isa-manual or
>>> >> so that in the future, modifications to and additions or removals of
>>> >> features will require a new extension. Codifying that assertion
>>> >> somewhere would make it quite unlikely that compatibility would be
>>> >> broken, but we have the tools required to deal with it, if & when it
>>> >> crops up.
>>> >> It is in our collective interest, as consumers of extension meanings, to
>>> >> define a scheme that enforces compatibility.
>>> >>
>>> >> The use of individual properties, rather than elements in a single
>>> >> string, will also permit validation that the properties have a meaning,
>>> >> as well as potentially reject mutually exclusive combinations, or
>>> >> enforce dependencies between extensions. That would not have be possible
>>> >> with the current dt-schema infrastructure for arbitrary strings, as we
>>> >> would need to add a riscv,isa parser to dt-validate!
>>> >> That's not implemented in this patch, but rather left as future work (for
>>> >> the brave, or the foolish).
>>> >>
>>> >> acpi
>>> >> 
>>> >>
>>> >> The current ACPI ECR is based on having a single ISA string 
>>> >> unfortunately,
>>> >> but ideally ACPI will move to another method, perhaps GUIDs, that give
>>> >> explicit meaning to extensions.
>>> >
>>> > Drop this paragraph on ACPI.
>>> >
>>> > We clearly mentioned 

[PATCH v2 1/4] sysinfo: Add IDs for board id and revision

2023-06-28 Thread Detlev Casanova
These IDs will be used by the sysinfo command. The new IDs are:
 * SYSINFO_ID_BOARD_ID: The board ID as an integer
 * SYSINFO_ID_BOARD_REVISION: The board revision as a string

Signed-off-by: Detlev Casanova 
---
 include/sysinfo.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/include/sysinfo.h b/include/sysinfo.h
index b140d742e93..43d9e91212d 100644
--- a/include/sysinfo.h
+++ b/include/sysinfo.h
@@ -47,6 +47,11 @@ enum sysinfo_id {
/* For show_board_info() */
SYSINFO_ID_BOARD_MODEL,
 
+   /* For sysinfo command */
+   SYSINFO_ID_BOARD_ID,
+   SYSINFO_ID_BOARD_REVISION_MAJOR,
+   SYSINFO_ID_BOARD_REVISION_MINOR,
+
/* First value available for downstream/board used */
SYSINFO_ID_USER = 0x1000,
 };
-- 
2.39.3



[PATCH v2 0/4] Introduce the sysinfo command

2023-06-28 Thread Detlev Casanova
The command can be used to show various information that can be used to
identify the running system.

Currently supported subcommands are:
* model: A string representing the model
* id: The id of the board
* revision: The revision of this board.

I'm also not sure if a test should be added and where to start to test
this kind of command.

Changes since v1:
 - Removed shell function to select linux device tree. This will be
   distributions job.
 - Break revision in rev_major and rev_minor in the sysinfo driver.

Detlev Casanova (4):
  sysinfo: Add IDs for board id and revision
  cmd: Add a sysinfo command
  sysinfo: rcar3: Use int instead of char for revision
  sysinfo: rcar3: Implement BOARD_ID and BOARD_REVISION_*

 cmd/Kconfig |   6 ++
 cmd/Makefile|   1 +
 cmd/sysinfo.c   | 134 ++
 drivers/sysinfo/rcar3.c | 141 
 include/sysinfo.h   |   5 ++
 5 files changed, 245 insertions(+), 42 deletions(-)
 create mode 100644 cmd/sysinfo.c

-- 
2.39.3



[PATCH v2 3/4] sysinfo: rcar3: Use int instead of char for revision

2023-06-28 Thread Detlev Casanova
To be used with the sysinfo command, revision values must be considered
as integers, not chars as some boards will implement BOARD_REVISION_*
and might use numbers greater than 9.

Signed-off-by: Detlev Casanova 
---
 drivers/sysinfo/rcar3.c | 104 
 1 file changed, 62 insertions(+), 42 deletions(-)

diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c
index 7b127986da7..450a4c26773 100644
--- a/drivers/sysinfo/rcar3.c
+++ b/drivers/sysinfo/rcar3.c
@@ -68,90 +68,110 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv 
*priv)
bool salvator_xs = false;
bool ebisu_4d = false;
bool condor_i = false;
-   char rev_major = '?';
-   char rev_minor = '?';
+   char model[64];
+   char rev[4] = "?.?";
+   u8 rev_major = 0;
+   u8 rev_minor = 0;
 
switch (board_id) {
case BOARD_SALVATOR_XS:
salvator_xs = true;
fallthrough;
case BOARD_SALVATOR_X:
+   snprintf(model, sizeof(model),
+"Renesas Salvator-X%s board", salvator_xs ? "S" : "");
if (!(board_rev & ~1)) { /* Only rev 0 and 1 is valid */
-   rev_major = '1';
-   rev_minor = '0' + (board_rev & BIT(0));
+   rev_major = 1;
+   rev_minor = board_rev & BIT(0);
+   snprintf(rev, sizeof(rev), "%u.%u", rev_major, 
rev_minor);
}
-   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
-"Renesas Salvator-X%s board rev %c.%c",
-salvator_xs ? "S" : "", rev_major, rev_minor);
+
+   snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev 
%s",
+model, rev);
+
return;
case BOARD_STARTER_KIT:
+   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+"Renesas Starter Kit board");
if (!(board_rev & ~1)) { /* Only rev 0 and 1 is valid */
-   rev_major = (board_rev & BIT(0)) ? '3' : '1';
-   rev_minor = '0';
+   rev_major = (board_rev & BIT(0)) ? 3 : 1;
+   rev_minor = 0;
+   snprintf(rev, sizeof(rev), "%u.%u", rev_major, 
rev_minor);
}
-   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
-"Renesas Starter Kit board rev %c.%c",
-rev_major, rev_minor);
+   snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev 
%s",
+model, rev);
return;
case BOARD_STARTER_KIT_PRE:
+   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+"Renesas Starter Kit Premier board");
if (!(board_rev & ~3)) { /* Only rev 0..3 is valid */
-   rev_major = (board_rev & BIT(1)) ? '2' : '1';
-   rev_minor = (board_rev == 3) ? '1' : '0';
+   rev_major = (board_rev & BIT(1)) ? 2 : 1;
+   rev_minor = (board_rev == 3) ? 1 : 0;
+   snprintf(rev, sizeof(rev), "%u.%u", rev_major, 
rev_minor);
}
-   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
-"Renesas Starter Kit Premier board rev %c.%c",
-rev_major, rev_minor);
+   snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev 
%s",
+model, rev);
return;
case BOARD_EAGLE:
+   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+"Renesas Eagle board");
if (!board_rev) { /* Only rev 0 is valid */
-   rev_major = '1';
-   rev_minor = '0';
+   rev_major = 1;
+   rev_minor = 0;
+   snprintf(rev, sizeof(rev), "%u.%u", rev_major, 
rev_minor);
}
-   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
-"Renesas Eagle board rev %c.%c",
-rev_major, rev_minor);
+   snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev 
%s",
+model, rev);
return;
case BOARD_EBISU_4D:
ebisu_4d = true;
fallthrough;
case BOARD_EBISU:
+   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+"Renesas Ebisu%s board", ebisu_4d ? "-4D" : "");
if (!board_rev) { /* Only rev 0 is valid */
-   rev_major = '1';
-   rev_minor = '0';
+   rev_major = 1;
+   rev_minor = 0;
+   snprintf(rev, sizeof(rev), "%u.%u",

[PATCH v2 2/4] cmd: Add a sysinfo command

2023-06-28 Thread Detlev Casanova
The command is able to show different information for the running
system:
* Model name
* Board ID
* Revision

This command can be used by boot shell scripts to select configurations
depending on the specific running system.

Signed-off-by: Detlev Casanova 
---
 cmd/Kconfig   |   6 +++
 cmd/Makefile  |   1 +
 cmd/sysinfo.c | 134 ++
 3 files changed, 141 insertions(+)
 create mode 100644 cmd/sysinfo.c

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 02e54f1e50f..9fb778ce809 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -210,6 +210,12 @@ config CMD_SBI
help
  Display information about the SBI implementation.
 
+config CMD_SYSINFO
+   bool "sysinfo"
+   depends on SYSINFO
+   help
+ Display information about the system.
+
 endmenu
 
 menu "Boot commands"
diff --git a/cmd/Makefile b/cmd/Makefile
index 6c37521b4e2..ba4d6de9a1b 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -165,6 +165,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o
 obj-$(CONFIG_CMD_STRINGS) += strings.o
 obj-$(CONFIG_CMD_SMC) += smccc.o
 obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o
+obj-$(CONFIG_CMD_SYSINFO) += sysinfo.o
 obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o
 obj-$(CONFIG_CMD_TEMPERATURE) += temperature.o
 obj-$(CONFIG_CMD_TERMINAL) += terminal.o
diff --git a/cmd/sysinfo.c b/cmd/sysinfo.c
new file mode 100644
index 000..513ea0416a2
--- /dev/null
+++ b/cmd/sysinfo.c
@@ -0,0 +1,134 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright 2023
+ * Detlev Casanova 
+ */
+
+#include 
+#include 
+#include 
+#include 
+
+static int get_sysinfo(struct udevice **dev)
+{
+   int ret = sysinfo_get(dev);
+
+   if (ret) {
+   debug("Cannot get sysinfo: %d\n", ret);
+   return ret;
+   }
+
+   ret = sysinfo_detect(*dev);
+   if (ret) {
+   debug("Cannot detect sysinfo: %d\n", ret);
+   return ret;
+   }
+
+   return 0;
+}
+
+static int do_sysinfo_model(struct cmd_tbl *cmdtp, int flag, int argc,
+   char *const argv[])
+{
+   struct udevice *dev;
+   char model[64];
+   int ret = get_sysinfo(&dev);
+
+   if (ret)
+   return CMD_RET_FAILURE;
+
+   ret = sysinfo_get_str(dev,
+ SYSINFO_ID_BOARD_MODEL,
+ sizeof(model),
+ model);
+
+   if (ret) {
+   debug("Cannot get sysinfo str: %d\n", ret);
+   return CMD_RET_FAILURE;
+   }
+
+   if (argc == 2)
+   env_set(argv[1], model);
+   else
+   printf("%s\n", model);
+
+   return CMD_RET_SUCCESS;
+}
+
+static int do_sysinfo_id(struct cmd_tbl *cmdtp, int flag, int argc,
+char *const argv[])
+{
+   struct udevice *dev;
+   u32 board_id;
+   int ret = get_sysinfo(&dev);
+
+   if (ret)
+   return CMD_RET_FAILURE;
+
+   ret = sysinfo_get_int(dev,
+ SYSINFO_ID_BOARD_ID,
+ &board_id);
+
+   if (ret) {
+   debug("Cannot get sysinfo int: %d\n", ret);
+   return CMD_RET_FAILURE;
+   }
+
+   if (argc == 2)
+   env_set_hex(argv[1], board_id);
+   else
+   printf("0x%02x\n", board_id);
+
+   return CMD_RET_SUCCESS;
+}
+
+static int do_sysinfo_revision(struct cmd_tbl *cmdtp, int flag, int argc,
+  char *const argv[])
+{
+   struct udevice *dev;
+   int rev_major;
+   int rev_minor;
+   char rev[4];
+   int ret = get_sysinfo(&dev);
+
+   if (ret)
+   return CMD_RET_FAILURE;
+
+   ret = sysinfo_get_int(dev,
+ SYSINFO_ID_BOARD_REVISION_MAJOR,
+ &rev_major);
+
+   if (ret) {
+   debug("Cannot get sysinfo int: %d\n", ret);
+   return CMD_RET_FAILURE;
+   }
+
+   ret = sysinfo_get_int(dev,
+ SYSINFO_ID_BOARD_REVISION_MINOR,
+ &rev_minor);
+
+   if (ret) {
+   debug("Cannot get sysinfo int: %d\n", ret);
+   return CMD_RET_FAILURE;
+   }
+
+   snprintf(rev, sizeof(rev), "%d.%d", rev_major, rev_minor);
+
+   if (argc == 2)
+   env_set(argv[1], rev);
+   else
+   printf("%s\n", rev);
+
+   return CMD_RET_SUCCESS;
+}
+
+static char sysinfo_help_text[] =
+   "model  - Show or set the board model in varname\n"
+   "sysinfo id - Show or set the board id in varname (in 
format 0xHH)\n"
+   "sysinfo revision   - Show or set the board revision in 
varname";
+
+U_BOOT_CMD_WITH_SUBCMDS(sysinfo, "System information", sysinfo_help_text,
+   U_BOOT_SUBCMD_MKENT(model, 2, 1, do_sysinfo_model),
+   U_BOOT_SUBCMD_MKENT(id, 2, 1, do_sysinfo_id),
+   

[PATCH v2 4/4] sysinfo: rcar3: Implement BOARD_ID and BOARD_REVISION_*

2023-06-28 Thread Detlev Casanova
Expose that information to the command shell to let scripts select the
correct devicetree name.

Signed-off-by: Detlev Casanova 
---
 drivers/sysinfo/rcar3.c | 103 +++-
 1 file changed, 70 insertions(+), 33 deletions(-)

diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c
index 450a4c26773..1a99642bf75 100644
--- a/drivers/sysinfo/rcar3.c
+++ b/drivers/sysinfo/rcar3.c
@@ -32,6 +32,10 @@
  */
 struct sysinfo_rcar_priv {
charboardmodel[64];
+   u8  id;
+   u8  rev_major;
+   u8  rev_minor;
+   boolhas_rev;
u8  val;
 };
 
@@ -56,9 +60,33 @@ static int sysinfo_rcar_get_str(struct udevice *dev, int id, 
size_t size, char *
};
 }
 
+static int sysinfo_rcar_get_int(struct udevice *dev, int id, int *val)
+{
+   struct sysinfo_rcar_priv *priv = dev_get_priv(dev);
+
+   switch (id) {
+   case SYSINFO_ID_BOARD_ID:
+   *val = priv->id;
+   return 0;
+   case SYSINFO_ID_BOARD_REVISION_MAJOR:
+   if (!priv->has_rev)
+   return -EINVAL;
+   *val = priv->rev_major;
+   return 0;
+   case SYSINFO_ID_BOARD_REVISION_MINOR:
+   if (!priv->has_rev)
+   return -EINVAL;
+   *val = priv->rev_minor;
+   return 0;
+   default:
+   return -EINVAL;
+   };
+}
+
 static const struct sysinfo_ops sysinfo_rcar_ops = {
.detect = sysinfo_rcar_detect,
.get_str = sysinfo_rcar_get_str,
+   .get_int = sysinfo_rcar_get_int,
 };
 
 static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv)
@@ -70,8 +98,9 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv)
bool condor_i = false;
char model[64];
char rev[4] = "?.?";
-   u8 rev_major = 0;
-   u8 rev_minor = 0;
+
+   priv->id = board_id;
+   priv->has_rev = false;
 
switch (board_id) {
case BOARD_SALVATOR_XS:
@@ -81,9 +110,10 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv 
*priv)
snprintf(model, sizeof(model),
 "Renesas Salvator-X%s board", salvator_xs ? "S" : "");
if (!(board_rev & ~1)) { /* Only rev 0 and 1 is valid */
-   rev_major = 1;
-   rev_minor = board_rev & BIT(0);
-   snprintf(rev, sizeof(rev), "%u.%u", rev_major, 
rev_minor);
+   priv->rev_major = 1;
+   priv->rev_minor = board_rev & BIT(0);
+   priv->has_rev = true;
+   snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, 
priv->rev_minor);
}
 
snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev 
%s",
@@ -91,34 +121,37 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv 
*priv)
 
return;
case BOARD_STARTER_KIT:
-   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+   snprintf(model, sizeof(model),
 "Renesas Starter Kit board");
if (!(board_rev & ~1)) { /* Only rev 0 and 1 is valid */
-   rev_major = (board_rev & BIT(0)) ? 3 : 1;
-   rev_minor = 0;
-   snprintf(rev, sizeof(rev), "%u.%u", rev_major, 
rev_minor);
+   priv->rev_major = (board_rev & BIT(0)) ? 3 : 1;
+   priv->rev_minor = 0;
+   snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, 
priv->rev_minor);
+   priv->has_rev = true;
}
snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev 
%s",
 model, rev);
return;
case BOARD_STARTER_KIT_PRE:
-   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+   snprintf(model, sizeof(model),
 "Renesas Starter Kit Premier board");
if (!(board_rev & ~3)) { /* Only rev 0..3 is valid */
-   rev_major = (board_rev & BIT(1)) ? 2 : 1;
-   rev_minor = (board_rev == 3) ? 1 : 0;
-   snprintf(rev, sizeof(rev), "%u.%u", rev_major, 
rev_minor);
+   priv->rev_major = (board_rev & BIT(1)) ? 2 : 1;
+   priv->rev_minor = (board_rev == 3) ? 1 : 0;
+   snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, 
priv->rev_minor);
+   priv->has_rev = true;
}
snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev 
%s",
 model, rev);
return;
case BOARD_EAGLE:
-   snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+   snprintf(model, sizeof(model),
 "Renesas Eagle board");
if (!board_re

Re: [PATCH 2/5] rockchip: rk3399-pinebook-pro: Fix SPL max size and SPI flash payload offset

2023-06-28 Thread Jonas Karlman
Hi Quentin,

On 2023-06-28 15:49, Quentin Schulz wrote:
> Hi Jonas,
> 
> On 6/27/23 21:10, Jonas Karlman wrote:
>> TPL max size is limited to 184 KB, SPL is loaded to 0x0 and TF-A is
>> loaded to 0x4, this limit SPL max size to 256 KB. With BootRom only
>> reading first 2 KB per 4 KB page of SPI flash, 880 KB may be needed for
>> TPL+SPL in a worst-case scenario. (184 KB + 256 KB) x 2 = 880 KB
>>
>> Use 0xE (896 KB) as the payload offset in SPI flash, this allows
>> for a payload of 3168 KB before env offset start to overlap.
>>
>> Also add CONFIG_ROCKCHIP_SPI_IMAGE=y to build a bootable SPI flash
>> image, u-boot-rockchip-spi.bin.
>>
>> Signed-off-by: Jonas Karlman 
>> ---
>>   arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi | 4 
>>   configs/pinebook-pro-rk3399_defconfig| 4 +++-
>>   2 files changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi 
>> b/arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi
>> index ea7a5a17ae0f..88a77cad8d43 100644
>> --- a/arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi
>> +++ b/arch/arm/dts/rk3399-pinebook-pro-u-boot.dtsi
>> @@ -10,10 +10,6 @@
>>  chosen {
>>  u-boot,spl-boot-order = "same-as-spl", &sdhci, &spiflash, 
>> &sdmmc;
>>  };
>> -
>> -config {
>> -u-boot,spl-payload-offset = <0x6>; /* @ 384KB */
>> -};
> 
> Just a nitpick/question (and for the whole series): Is there any 
> specific reason for going back to the Kconfig way 
> (CONFIG_SYS_SPI_U_BOOT_OFFS) instead of just modifying the offset in the 
> -u-boot DTSI?

The main reason is that SYS_SPI_U_BOOT_OFFS is used as payload offset by
binman in rockchip-u-boot.dtsi, for building u-boot-rockchip-spi.bin.
And also to avoid any confusion on what value is used.

rockchip-u-boot.dtsi:
  offset = ;

> 
> On a different note, that also means that we're effectively breaking 
> current setups by moving the environment some other location. I cannot 
> talk for the maintainer(s) and user(s) but I thought it was worth 
> mentioning.

The environment should not be affected by these changes. All rk3399
boards using ENV_IS_IN_SPI_FLASH all use ENV_OFFSET=0x3F8000. u-boot.itb
or u-boot.img can be up to 3168 KB before it affects environment.

  0x00: idbloader.img (TPL+SPL)
  0x0E: u-boot.itb or u-boot.img (was before at 0x06)
  0x3F8000: environment, same as before

I am not expecting any issues for users, as long as the updated
instructions are followed when updating u-boot in SPI flash, i.e.
writing u-boot-rockchip-spi.bin at start of SPI flash.

Regards,
Jonas

> 
>>   };
>>   
>>   &edp {
>> diff --git a/configs/pinebook-pro-rk3399_defconfig 
>> b/configs/pinebook-pro-rk3399_defconfig
>> index 58a8b91aa60f..1109ebb7387b 100644
>> --- a/configs/pinebook-pro-rk3399_defconfig
>> +++ b/configs/pinebook-pro-rk3399_defconfig
>> @@ -12,6 +12,7 @@ CONFIG_ENV_OFFSET=0x3F8000
>>   CONFIG_DEFAULT_DEVICE_TREE="rk3399-pinebook-pro"
>>   CONFIG_DM_RESET=y
>>   CONFIG_ROCKCHIP_RK3399=y
>> +CONFIG_ROCKCHIP_SPI_IMAGE=y
>>   CONFIG_TARGET_PINEBOOK_PRO_RK3399=y
>>   CONFIG_SPL_STACK=0x40
>>   CONFIG_DEBUG_UART_BASE=0xFF1A
>> @@ -26,7 +27,7 @@ CONFIG_USE_PREBOOT=y
>>   CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-pinebook-pro.dtb"
>>   CONFIG_DISPLAY_BOARDINFO_LATE=y
>>   CONFIG_MISC_INIT_R=y
>> -CONFIG_SPL_MAX_SIZE=0x2e000
>> +CONFIG_SPL_MAX_SIZE=0x4
> 
> nitpick: We *could* have this in another commit after we move the 
> environment to another location later in the SPI flash.
> 
> Cheers,
> Quentin



[PATCH] net: ksz9477: add support for KSZ9893 GbE switch

2023-06-28 Thread Karsten Wiese
Copy and tweak the required code from the linux kernel.
Only the KSZ9893 has been tested.

Signed-off-by: Karsten Wiese 

---
 drivers/net/ksz9477.c | 103 --
 1 file changed, 89 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ksz9477.c b/drivers/net/ksz9477.c
index 6b59b5fcd2..43baa69961 100644
--- a/drivers/net/ksz9477.c
+++ b/drivers/net/ksz9477.c
@@ -16,6 +16,10 @@
 
 #include 
 
+/* Used with variable features to indicate capabilities. */
+#define NEW_XMII   BIT(1)
+#define IS_9893BIT(2)
+
 /* Global registers */
 
 /* Chip ID */
@@ -41,6 +45,13 @@
 #define PORT_RMII_SEL  0x1
 #define PORT_GMII_SEL  0x2
 #define PORT_MII_SEL   0x3
+/* S1 */
+#define PORT_MII_1000MBIT_S1   BIT(6)
+/* S1 */
+#define PORT_MII_SEL_S10x0
+#define PORT_RMII_SEL_S1   0x1
+#define PORT_GMII_SEL_S1   0x2
+#define PORT_RGMII_SEL_S1  0x3
 
 /* Port MSTP State Register */
 #define REG_PORT_MSTP_STATE0x0b04
@@ -62,6 +73,8 @@
 
 struct ksz_dsa_priv {
struct udevice *dev;
+
+   u32 features;   /* chip specific features */
 };
 
 static inline int ksz_read8(struct udevice *dev, u32 reg, u8 *val)
@@ -284,6 +297,60 @@ U_BOOT_DRIVER(ksz_mdio) = {
.plat_auto  = sizeof(struct mdio_perdev_priv),
 };
 
+static void ksz9477_set_gbit(struct ksz_dsa_priv *priv, bool gbit, u8 *data)
+{
+   if (priv->features & NEW_XMII) {
+   if (gbit)
+   *data &= ~PORT_MII_NOT_1GBIT;
+   else
+   *data |= PORT_MII_NOT_1GBIT;
+   } else {
+   if (gbit)
+   *data |= PORT_MII_1000MBIT_S1;
+   else
+   *data &= ~PORT_MII_1000MBIT_S1;
+   }
+}
+
+static void ksz9477_set_xmii(struct ksz_dsa_priv *priv, int mode, u8 *data)
+{
+   u8 xmii;
+
+   if (priv->features & NEW_XMII) {
+   switch (mode) {
+   case 0:
+   xmii = PORT_MII_SEL;
+   break;
+   case 1:
+   xmii = PORT_RMII_SEL;
+   break;
+   case 2:
+   xmii = PORT_GMII_SEL;
+   break;
+   default:
+   xmii = PORT_RGMII_SEL;
+   break;
+   }
+   } else {
+   switch (mode) {
+   case 0:
+   xmii = PORT_MII_SEL_S1;
+   break;
+   case 1:
+   xmii = PORT_RMII_SEL_S1;
+   break;
+   case 2:
+   xmii = PORT_GMII_SEL_S1;
+   break;
+   default:
+   xmii = PORT_RGMII_SEL_S1;
+   break;
+   }
+   }
+   *data &= ~PORT_MII_SEL_M;
+   *data |= xmii;
+}
+
 static int ksz_port_setup(struct udevice *dev, int port,
  phy_interface_t interface)
 {
@@ -293,9 +360,11 @@ static int ksz_port_setup(struct udevice *dev, int port,
dev_dbg(dev, "%s P%d %s\n", __func__, port + 1,
(port == pdata->cpu_port) ? "cpu" : "");
 
+   struct ksz_dsa_priv *priv = dev_get_priv(dev);
if (port != pdata->cpu_port) {
-   /* phy port: config errata and leds */
-   ksz_phy_errata_setup(dev, port);
+   if (priv->features & NEW_XMII)
+   /* phy port: config errata and leds */
+   ksz_phy_errata_setup(dev, port);
} else {
/* cpu port: configure MAC interface mode */
ksz_pread8(dev, port, REG_PORT_XMII_CTRL_1, &data8);
@@ -303,24 +372,20 @@ static int ksz_port_setup(struct udevice *dev, int port,
phy_string_for_interface(interface));
switch (interface) {
case PHY_INTERFACE_MODE_MII:
-   data8 &= ~PORT_MII_SEL_M;
-   data8 |= PORT_MII_SEL;
-   data8 |= PORT_MII_NOT_1GBIT;
+   ksz9477_set_xmii(priv, 0, &data8);
+   ksz9477_set_gbit(priv, false, &data8);
break;
case PHY_INTERFACE_MODE_RMII:
-   data8 &= ~PORT_MII_SEL_M;
-   data8 |= PORT_RMII_SEL;
-   data8 |= PORT_MII_NOT_1GBIT;
+   ksz9477_set_xmii(priv, 1, &data8);
+   ksz9477_set_gbit(priv, false, &data8);
break;
case PHY_INTERFACE_MODE_GMII:
-   data8 &= ~PORT_MII_SEL_M;
-   data8 |= PORT_GMII_SEL;
-   data8 &= ~PORT_MII_NOT_1GBIT;
+   ksz9477_se

EFI Secure boot default keys

2023-06-28 Thread Neil Jones
Please can someone describe the format of the file needed for the default / 
built-in EFI secure boot keys (ubootefi.var)

The only docs I have found suggest its best to enroll the keys from within 
u-boot onto some removable media, then copy this off and use this as the 
default, this is not very helpful and doesn't work for me:

=> fatload mmc 0:1 ${loadaddr} PK.aut
2053 bytes read in 18 ms (111.3 KiB/s)
=> setenv -e -nv -bs -rt -at -i ${loadaddr}:$filesize PK
setenv - set environment variables

Usage:
setenv setenv [-f] name value ...
- [forcibly] set environment variable 'name' to 'value ...'
setenv [-f] name
- [forcibly] delete environment variable 'name'

my setenv doesn't support all the extra switches ? This is with 2022.04, all 
other EFI options seem to be in this release and I can boot unsigned EFI images 
ok.

Cheers,

Neil





Re: [PATCH 06/12] binman: Convert mkimage to Entry_section

2023-06-28 Thread Jonas Karlman
Hi Simon,
On 2023-06-28 13:41, Simon Glass wrote:
> From: Marek Vasut 
> 
> This is needed to handle mkimage with inner section located itself in a
> section.
> 
> Signed-off-by: Marek Vasut 
> Use BuildSectionData() instead of ObtainContents(), add tests and a few
> other minor fixes:
> Signed-off-by: Simon Glass 
> ---
> 
>  tools/binman/entry.py |  6 +-
>  tools/binman/etype/mkimage.py | 76 ++-
>  tools/binman/ftest.py | 45 +-
>  tools/binman/test/283_mkimage_special.dts | 24 +++
>  4 files changed, 117 insertions(+), 34 deletions(-)
>  create mode 100644 tools/binman/test/283_mkimage_special.dts
> 
> diff --git a/tools/binman/entry.py b/tools/binman/entry.py
> index 328b5bc568a9..8f06fea51ad4 100644
> --- a/tools/binman/entry.py
> +++ b/tools/binman/entry.py
> @@ -1311,10 +1311,8 @@ features to produce new behaviours.
>  """
>  data = b''
>  for entry in entries:
> -# First get the input data and put it in a file. If not 
> available,
> -# try later.
> -if not entry.ObtainContents(fake_size=fake_size):
> -return None, None, None
> +# First get the input data and put it in a file
> +entry.ObtainContents(fake_size=fake_size)
>  data += entry.GetData()
>  uniq = self.GetUniqueName()
>  fname = tools.get_output_filename(f'{prefix}.{uniq}')
> diff --git a/tools/binman/etype/mkimage.py b/tools/binman/etype/mkimage.py
> index cb3e10672ad7..8311fed59762 100644
> --- a/tools/binman/etype/mkimage.py
> +++ b/tools/binman/etype/mkimage.py
> @@ -8,10 +8,11 @@
>  from collections import OrderedDict
>  
>  from binman.entry import Entry
> +from binman.etype.section import Entry_section
>  from dtoc import fdt_util
>  from u_boot_pylib import tools
>  
> -class Entry_mkimage(Entry):
> +class Entry_mkimage(Entry_section):
>  """Binary produced by mkimage
>  
>  Properties / Entry arguments:
> @@ -121,10 +122,8 @@ class Entry_mkimage(Entry):
>  """
>  def __init__(self, section, etype, node):
>  super().__init__(section, etype, node)
> -self._mkimage_entries = OrderedDict()
>  self._imagename = None
> -self._filename = fdt_util.GetString(self._node, 'filename')
> -self.align_default = None
> +self._multiple_data_files = False
>  
>  def ReadNode(self):
>  super().ReadNode()
> @@ -135,41 +134,60 @@ class Entry_mkimage(Entry):
> 'data-to-imagename')
>  if self._data_to_imagename and self._node.FindNode('imagename'):
>  self.Raise('Cannot use both imagename node and 
> data-to-imagename')
> -self.ReadEntries()
>  
>  def ReadEntries(self):
>  """Read the subnodes to find out what should go in this image"""
>  for node in self._node.subnodes:
> -entry = Entry.Create(self, node)
> +if self.IsSpecialSubnode(node):
> +continue
> +entry = Entry.Create(self, node,
> + expanded=self.GetImage().use_expanded,
> + missing_etype=self.GetImage().missing_etype)
>  entry.ReadNode()
> +entry.SetPrefix(self._name_prefix)
>  if entry.name == 'imagename':
>  self._imagename = entry
>  else:
> -self._mkimage_entries[entry.name] = entry
> +self._entries[entry.name] = entry
>  
> -def ObtainContents(self):
> +def BuildSectionData(self, required):
> +"""Build mkimage entry contents
> +
> +Runs mkimage to build the entry contents
> +
> +Args:
> +required (bool): True if the data must be present, False if it 
> is OK
> +to return None
> +
> +Returns:
> +bytes: Contents of the section
> +"""
>  # Use a non-zero size for any fake files to keep mkimage happy
>  # Note that testMkimageImagename() relies on this 'mkimage' parameter
>  fake_size = 1024
>  if self._multiple_data_files:
>  fnames = []
>  uniq = self.GetUniqueName()
> -for entry in self._mkimage_entries.values():
> -if not entry.ObtainContents(fake_size=fake_size):
> -return False
> -if entry._pathname:
> -fnames.append(entry._pathname)
> +for entry in self._entries.values():
> +entry.ObtainContents(fake_size=fake_size)
> +
> +# If this is a section, put the contents in a temporary file.
> +# Otherwise, assume it is a blob and use the pathname
> +if isinstance(entry, Entry_section):
> +ename = f'mkimage-in-{uniq}-{entry.name}'
> +fname = tools.get_output_file

Re: [GIT PULL] Please pull u-boot-amlogic-next-20230628

2023-06-28 Thread Tom Rini
On Wed, Jun 28, 2023 at 12:01:08PM +0200, Neil Armstrong wrote:

> Hi Tom,
> 
> A set of changes for next release, including:
> - A1 SoC support with associated reference boards
> - KII Pro board support based on GXBB SoC
> - New Secure power domains driver for A1 SoC
> 
> The CI job is at 
> https://source.denx.de/u-boot/custodians/u-boot-amlogic/pipelines/16718
> 
> Thanks,
> Neil
> 
> The following changes since commit eef4a771e85fc30a18719316a23d0ad1476ae1a5:
> 
>   Merge branch '2023-06-21-fix-get_ram_size-with-cache-enabled' into next 
> (2023-06-22 09:59:43 -0400)
> 
> are available in the Git repository at:
> 
>   https://source.denx.de/u-boot/custodians/u-boot-amlogic.git 
> tags/u-boot-amlogic-next-20230628
> 
> for you to fetch changes up to 58edf5773adcc95105bbd814dcbe43b1d9804391:
> 
>   drivers: meson: introduce secure power controller driver (2023-06-28 
> 10:05:34 +0200)
> 

Applied to u-boot/next, thanks!

-- 
Tom


signature.asc
Description: PGP signature


Re: EFI Secure boot default keys

2023-06-28 Thread AKASHI Takahiro
On Wed, Jun 28, 2023 at 04:26:58PM +, Neil Jones wrote:
> Please can someone describe the format of the file needed for the default / 
> built-in EFI secure boot keys (ubootefi.var)
> 
> The only docs I have found suggest its best to enroll the keys from within 
> u-boot onto some removable media, then copy this off and use this as the 
> default, this is not very helpful and doesn't work for me:
> 
> => fatload mmc 0:1 ${loadaddr} PK.aut
> 2053 bytes read in 18 ms (111.3 KiB/s)
> => setenv -e -nv -bs -rt -at -i ${loadaddr}:$filesize PK
> setenv - set environment variables
> 
> Usage:
> setenv setenv [-f] name value ...
> - [forcibly] set environment variable 'name' to 'value ...'
> setenv [-f] name
> - [forcibly] delete environment variable 'name'
> 
> my setenv doesn't support all the extra switches ? This is with 2022.04, all 
> other EFI options seem to be in this release and I can boot unsigned EFI 
> images ok.

Please turn on CONFIG_CMD_NVEDIT_EFI when building your U-Boot.

This option was disabled by the commit:
commit 3b728f8728fa (tag: efi-2020-01-rc1)
Author: Heinrich Schuchardt 
Date:   Sun Oct 6 15:44:22 2019 +0200

cmd: disable CMD_NVEDIT_EFI by default

The binary size of efi has grown much since in the past, though.

-Takahiro Akashi

> Cheers,
> 
> Neil
> 
> 
> 


[PATCH] cmd: efidebug: add missing efi_free_pool for dh subcommand

2023-06-28 Thread Masahisa Kojima
This adds the missing efi_free_pool call for dh subcommand.

Signed-off-by: Masahisa Kojima 
---
 cmd/efidebug.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index 9622430c47..0be3af3e76 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -486,6 +486,7 @@ static int do_efi_show_handles(struct cmd_tbl *cmdtp, int 
flag,
if (guidcmp(guid[j], &efi_guid_device_path))
printf("  %pUs\n", guid[j]);
}
+   efi_free_pool(guid);
}
 
efi_free_pool(handles);
-- 
2.34.1



Re: [RESEND PATCH v1 1/4] riscv: t-head: licheepi4a: initial support added

2023-06-28 Thread Leo Liang
Hi YiXun,
On Fri, May 26, 2023 at 08:41:04PM +0800, Yixun Lan wrote:
> Add support for Sipeed's Lichee Pi 4A board which based on
> T-HEAD's TH1520 SoC, only minimal device tree and serial onsole are enabled,
> so it's capable of chain booting from T-HEAD's vendor u-boot.
> 
> Reviewed-by: Wei Fu 
> Signed-off-by: Yixun Lan 
> ...
> diff --git a/board/thead/th1520_lpi4a/board.c 
> b/board/thead/th1520_lpi4a/board.c
> new file mode 100644
> index 00..378bab098b
> --- /dev/null
> +++ b/board/thead/th1520_lpi4a/board.c
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2023, Yixun Lan 
> + *
> + */
> +
> +#include 
> +
> +int board_init(void)
> +{
> + enable_caches();

There is a compilation warining here at "enable_caches"
(probably due to cpu_func.h not being included to provide this function)


Best regards,
Leo
> +
> + return 0;
> +}


Re: [RESEND PATCH v1 4/4] doc: t-head: lpi4a: document Lichee PI 4A board

2023-06-28 Thread Leo Liang
Hi YiXun,

On Fri, May 26, 2023 at 08:41:07PM +0800, Yixun Lan wrote:
> Reviewed-by: Wei Fu 
> Signed-off-by: Yixun Lan 
> ---
>  doc/board/index.rst   |   1 +
>  doc/board/thead/index.rst |   9 +++
>  doc/board/thead/lpi4a.rst | 112 ++
>  3 files changed, 122 insertions(+)
>  create mode 100644 doc/board/thead/index.rst
>  create mode 100644 doc/board/thead/lpi4a.rst
> 
> diff --git a/doc/board/index.rst b/doc/board/index.rst
> index 9ef25b1091..aadc90af89 100644
> --- a/doc/board/index.rst
> +++ b/doc/board/index.rst
> @@ -45,6 +45,7 @@ Board-specific doc
> starfive/index
> ste/index
> tbs/index
> +   thead/index
> ti/index
> toradex/index
> variscite/index
> diff --git a/doc/board/thead/index.rst b/doc/board/thead/index.rst
> new file mode 100644
> index 00..41566d3a36
> --- /dev/null
> +++ b/doc/board/thead/index.rst
> @@ -0,0 +1,9 @@
> +.. SPDX-License-Identifier: GPL-2.0+
> +
> +T-HEAD
> +
> +
> +.. toctree::
> +   :maxdepth: 1
> +
> +   lpi4a
> diff --git a/doc/board/thead/lpi4a.rst b/doc/board/thead/lpi4a.rst
> new file mode 100644
> index 00..3764e732af
> --- /dev/null
> +++ b/doc/board/thead/lpi4a.rst
> @@ -0,0 +1,112 @@
> +.. SPDX-License-Identifier: GPL-2.0+
> +
> +Sipeed's Lichee PI 4A based on T-HEAD TH1520 SoC
> +
> +

The underline is too short and would cause error when building docs.

Best regards,
Leo
> +TH1520 RISC-V SoC
> +-
> +The TH1520 is 4+2 core 64-bit RISC-V SoC from Alibaba T-HEAD, it's capable of
> +running highest CPU frequency at 2.5 GHz, integrate Imagination GPU for 
> graphics,
> +also with 4 TOPS NPU for AI acceleration.
> +
> +Mainline support
> +
> +
> +The support for following drivers are already enabled:
> +
> +1. ns16550 UART Driver.
> +
> +Building
> +
> +
> +1. Add the RISC-V toolchain to your PATH.
> +2. Setup ARCH & cross compilation environment variable:
> +
> +.. code-block:: none
> +
> +   export CROSS_COMPILE=
> +
> +The U-Boot is capable of running in M-Mode, so we can directly build it.
> +
> +.. code-block:: console
> +
> + cd 
> + make th1520_lpi4a_defconfig
> + make
> +
> +This will generate u-boot-dtb.bin
> +
> +Booting
> +~~~
> +
> +Currently, we rely on vendor u-boot to initialize the clock, pinctrl 
> subsystem,
> +and chain load the mainline u-boot image either via tftp or emmc storage,
> +then bootup from it.
> +
> +Sample boot log from Lichee PI 4A board via tftp
> +~~~
> +
> +.. code-block:: none
> +
> + brom_ver 8
> + [APP][E] protocol_connect failed, exit.
> +
> + U-Boot SPL 2020.01-00016-g8c870a6be8 (May 20 2023 - 01:04:49 +)
> + FM[1] lpddr4x dualrank freq=3733 64bit dbi_off=n sdram init
> + ddr initialized, jump to uboot
> + image has no header
> +
> +
> + U-Boot 2020.01-00016-g8c870a6be8 (May 20 2023 - 01:04:49 +)
> +
> + CPU:   rv64imafdcvsu
> + Model: T-HEAD c910 light
> + DRAM:  8 GiB
> + C910 CPU FREQ: 750MHz
> + AHB2_CPUSYS_HCLK FREQ: 250MHz
> + AHB3_CPUSYS_PCLK FREQ: 125MHz
> + PERISYS_AHB_HCLK FREQ: 250MHz
> + PERISYS_APB_PCLK FREQ: 62MHz
> + GMAC PLL POSTDIV FREQ: 1000MHZ
> + DPU0 PLL POSTDIV FREQ: 1188MHZ
> + DPU1 PLL POSTDIV FREQ: 1188MHZ
> + MMC:   sdhci@ffe708: 0, sd@ffe709: 1
> + Loading Environment from MMC... OK
> + Error reading output register
> + Warning: cannot get lcd-en GPIO
> + LCD panel cannot be found : -121
> + splash screen startup cost 16 ms
> + In:serial
> + Out:   serial
> + Err:   serial
> + Net:
> + Warning: ethernet@ffe707 using MAC address from ROM
> + eth0: ethernet@ffe707ethernet@ffe707:0 is connected to 
> ethernet@ffe707.  Reconnecting to ethernet@ffe706
> +
> + Warning: ethernet@ffe706 (eth1) using random MAC address - 
> 42:25:d4:16:5f:fc
> + , eth1: ethernet@ffe706
> + Hit any key to stop autoboot:  2
> + ethernet@ffe706 Waiting for PHY auto negotiation to complete.. done
> + Speed: 1000, full duplex
> + Using ethernet@ffe707 device
> + TFTP from server 192.168.8.50; our IP address is 192.168.8.45
> + Filename 'u-boot-dtb.bin'.
> + Load address: 0x1c0
> + Loading: * #
> +  8 MiB/s
> + done
> + Bytes transferred = 376686 (5bf6e hex)
> + ## Starting application at 0x01C0 ...
> +
> +U-Boot 2023.07-rc2-4-g1befbe31c1 (May 23 2023 - 18:40:01 +0800)
> +
> +CPU:   rv64imafdc
> +Model: Sipeed Lichee Pi 4A
> +DRAM:  8 GiB
> +Core:  13 devices, 6 uclasses, devicetree: separate
> +Loading Environment from ... OK
> +In:serial@ffe7014000
> +Out:   serial@ffe7014000
> +Err:   serial@ffe7014000
> +Model: Sipeed Lichee Pi 4A
> +LPI4A=>
> -- 
> 2.40

Re: [PATCH v3] dt-bindings: riscv: deprecate riscv,isa

2023-06-28 Thread Palmer Dabbelt

On Wed, 28 Jun 2023 08:31:32 PDT (-0700), apa...@ventanamicro.com wrote:

On Wed, Jun 28, 2023 at 7:46 PM Palmer Dabbelt  wrote:


On Tue, 27 Jun 2023 08:49:06 PDT (-0700), Palmer Dabbelt wrote:
> On Mon, 26 Jun 2023 23:52:06 PDT (-0700), apa...@ventanamicro.com wrote:
>> On Tue, Jun 27, 2023 at 1:23 AM Palmer Dabbelt  wrote:
>>>
>>> On Mon, 26 Jun 2023 10:38:43 PDT (-0700), apa...@ventanamicro.com wrote:
>>> > On Mon, Jun 26, 2023 at 3:42 PM Conor Dooley  
wrote:
>>> >>
>>> >> intro
>>> >> =
>>> >>
>>> >> When the RISC-V dt-bindings were accepted upstream in Linux, the base
>>> >> ISA etc had yet to be ratified. By the ratification of the base ISA,
>>> >> incompatible changes had snuck into the specifications - for example the
>>> >> Zicsr and Zifencei extensions were spun out of the base ISA.
>>> >>
>>> >> Fast forward to today, and the reason for this patch.
>>> >> Currently the riscv,isa dt property permits only a specific subset of
>>> >> the ISA string - in particular it excludes version numbering.
>>> >> With the current constraints, it is not possible to discern whether
>>> >> "rv64i" means that the hart supports the fence.i instruction, for
>>> >> example.
>>> >> Future systems may choose to implement their own instruction fencing,
>>> >> perhaps using a vendor extension, or they may not implement the optional
>>> >> counter extensions. Software needs a way to determine this.
>>> >>
>>> >> versioning schemes
>>> >> ==
>>> >>
>>> >> "Use the extension versions that are described in the ISA manual" you
>>> >> may say, and it's not like this has not been considered.
>>> >> Firstly, software that parses the riscv,isa property at runtime will
>>> >> need to contain a lookup table of some sort that maps arbitrary versions
>>> >> to versions it understands. There is not a consistent application of
>>> >> version number applied to extensions, with a higgledy-piggledy
>>> >> collection of tags, "bare" and versioned documents awaiting the reader
>>> >> on the "recently ratified extensions" page:
>>> >> https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions
>>> >>
>>> >> As an aside, and this is reflected in the patch too, since many
>>> >> extensions have yet to appear in a release of the ISA specs,
>>> >> they are defined by commits in their respective "working draft"
>>> >> repositories.
>>> >>
>>> >> Secondly, there is an issue of backwards compatibility, whereby allowing
>>> >> numbers in the ISA string, some parsers may be broken. This would
>>> >> require an additional property to be created to even use the versions in
>>> >> this manner.
>>> >>
>>> >> ~boolean properties~ string array property
>>> >> ==
>>> >>
>>> >> If a new property is needed, the whole approach may as well be looked at
>>> >> from the bottom up. A string with limited character choices etc is
>>> >> hardly the best approach for communicating extension information to
>>> >> software.
>>> >>
>>> >> Switching to using properties that are defined on a per extension basis,
>>> >> allows us to define explicit meanings for the DT representation of each
>>> >> extension - rather than the current situation where different operating
>>> >> systems or other bits of software may impart different meanings to
>>> >> characters in the string.
>>> >> Clearly the best source of meanings is the specifications themselves,
>>> >> this just provides us the ability to choose at what point in time the
>>> >> meaning is set. If an extension changes incompatibility in the future,
>>> >> a new property will be required.
>>> >>
>>> >> Off-list, some of the RVI folks have committed to shoring up the wording
>>> >> in either the ISA specifications, the riscv-isa-manual or
>>> >> so that in the future, modifications to and additions or removals of
>>> >> features will require a new extension. Codifying that assertion
>>> >> somewhere would make it quite unlikely that compatibility would be
>>> >> broken, but we have the tools required to deal with it, if & when it
>>> >> crops up.
>>> >> It is in our collective interest, as consumers of extension meanings, to
>>> >> define a scheme that enforces compatibility.
>>> >>
>>> >> The use of individual properties, rather than elements in a single
>>> >> string, will also permit validation that the properties have a meaning,
>>> >> as well as potentially reject mutually exclusive combinations, or
>>> >> enforce dependencies between extensions. That would not have be possible
>>> >> with the current dt-schema infrastructure for arbitrary strings, as we
>>> >> would need to add a riscv,isa parser to dt-validate!
>>> >> That's not implemented in this patch, but rather left as future work (for
>>> >> the brave, or the foolish).
>>> >>
>>> >> acpi
>>> >> 
>>> >>
>>> >> The current ACPI ECR is based on having a single ISA string 
unfortunately,
>>> >> but ideally ACPI will move to another method, perhaps GUIDs, that giv

Re: [PATCH v3] dt-bindings: riscv: deprecate riscv,isa

2023-06-28 Thread Anup Patel
On Thu, Jun 29, 2023 at 8:49 AM Palmer Dabbelt  wrote:
>
> On Wed, 28 Jun 2023 08:31:32 PDT (-0700), apa...@ventanamicro.com wrote:
> > On Wed, Jun 28, 2023 at 7:46 PM Palmer Dabbelt  wrote:
> >>
> >> On Tue, 27 Jun 2023 08:49:06 PDT (-0700), Palmer Dabbelt wrote:
> >> > On Mon, 26 Jun 2023 23:52:06 PDT (-0700), apa...@ventanamicro.com wrote:
> >> >> On Tue, Jun 27, 2023 at 1:23 AM Palmer Dabbelt  
> >> >> wrote:
> >> >>>
> >> >>> On Mon, 26 Jun 2023 10:38:43 PDT (-0700), apa...@ventanamicro.com 
> >> >>> wrote:
> >> >>> > On Mon, Jun 26, 2023 at 3:42 PM Conor Dooley 
> >> >>> >  wrote:
> >> >>> >>
> >> >>> >> intro
> >> >>> >> =
> >> >>> >>
> >> >>> >> When the RISC-V dt-bindings were accepted upstream in Linux, the 
> >> >>> >> base
> >> >>> >> ISA etc had yet to be ratified. By the ratification of the base ISA,
> >> >>> >> incompatible changes had snuck into the specifications - for 
> >> >>> >> example the
> >> >>> >> Zicsr and Zifencei extensions were spun out of the base ISA.
> >> >>> >>
> >> >>> >> Fast forward to today, and the reason for this patch.
> >> >>> >> Currently the riscv,isa dt property permits only a specific subset 
> >> >>> >> of
> >> >>> >> the ISA string - in particular it excludes version numbering.
> >> >>> >> With the current constraints, it is not possible to discern whether
> >> >>> >> "rv64i" means that the hart supports the fence.i instruction, for
> >> >>> >> example.
> >> >>> >> Future systems may choose to implement their own instruction 
> >> >>> >> fencing,
> >> >>> >> perhaps using a vendor extension, or they may not implement the 
> >> >>> >> optional
> >> >>> >> counter extensions. Software needs a way to determine this.
> >> >>> >>
> >> >>> >> versioning schemes
> >> >>> >> ==
> >> >>> >>
> >> >>> >> "Use the extension versions that are described in the ISA manual" 
> >> >>> >> you
> >> >>> >> may say, and it's not like this has not been considered.
> >> >>> >> Firstly, software that parses the riscv,isa property at runtime will
> >> >>> >> need to contain a lookup table of some sort that maps arbitrary 
> >> >>> >> versions
> >> >>> >> to versions it understands. There is not a consistent application of
> >> >>> >> version number applied to extensions, with a higgledy-piggledy
> >> >>> >> collection of tags, "bare" and versioned documents awaiting the 
> >> >>> >> reader
> >> >>> >> on the "recently ratified extensions" page:
> >> >>> >> https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions
> >> >>> >>
> >> >>> >> As an aside, and this is reflected in the patch too, since 
> >> >>> >> many
> >> >>> >> extensions have yet to appear in a release of the ISA specs,
> >> >>> >> they are defined by commits in their respective "working 
> >> >>> >> draft"
> >> >>> >> repositories.
> >> >>> >>
> >> >>> >> Secondly, there is an issue of backwards compatibility, whereby 
> >> >>> >> allowing
> >> >>> >> numbers in the ISA string, some parsers may be broken. This would
> >> >>> >> require an additional property to be created to even use the 
> >> >>> >> versions in
> >> >>> >> this manner.
> >> >>> >>
> >> >>> >> ~boolean properties~ string array property
> >> >>> >> ==
> >> >>> >>
> >> >>> >> If a new property is needed, the whole approach may as well be 
> >> >>> >> looked at
> >> >>> >> from the bottom up. A string with limited character choices etc is
> >> >>> >> hardly the best approach for communicating extension information to
> >> >>> >> software.
> >> >>> >>
> >> >>> >> Switching to using properties that are defined on a per extension 
> >> >>> >> basis,
> >> >>> >> allows us to define explicit meanings for the DT representation of 
> >> >>> >> each
> >> >>> >> extension - rather than the current situation where different 
> >> >>> >> operating
> >> >>> >> systems or other bits of software may impart different meanings to
> >> >>> >> characters in the string.
> >> >>> >> Clearly the best source of meanings is the specifications 
> >> >>> >> themselves,
> >> >>> >> this just provides us the ability to choose at what point in time 
> >> >>> >> the
> >> >>> >> meaning is set. If an extension changes incompatibility in the 
> >> >>> >> future,
> >> >>> >> a new property will be required.
> >> >>> >>
> >> >>> >> Off-list, some of the RVI folks have committed to shoring up the 
> >> >>> >> wording
> >> >>> >> in either the ISA specifications, the riscv-isa-manual or
> >> >>> >> so that in the future, modifications to and additions or removals of
> >> >>> >> features will require a new extension. Codifying that assertion
> >> >>> >> somewhere would make it quite unlikely that compatibility would be
> >> >>> >> broken, but we have the tools required to deal with it, if & when it
> >> >>> >> crops up.
> >> >>> >> It is in our collective interest, as consumers of extension 
> >> >>> >> meanings, to
> >> >>> >> define a scheme that enforces compatibility.
> >> >>> >>
> >> >>> >