Re: [PATCH v6 09/25] arm: xenguest_arm64: Add a fake devicetree file

[Simon Glass]

Hi Tom,

On Thu, 2 Dec 2021 at 10:56, Tom Rini  wrote:
>
> On Thu, Dec 02, 2021 at 05:40:46PM +, Oleksandr Andrushchenko wrote:
> > Hi, Simon!
> >
> > Sorry for being late to the party
> >
> > On 02.12.21 17:59, Simon Glass wrote:
> > > Add an empty file to prevent build errors when building with
> > > CONFIG_OF_SEPARATE enabled.
> > >
> > > The build instructions in U-Boot do not provide enough detail to build a
> > > useful devicetree, unfortunately.
> > Xen guest doesn't use any built-in device trees as the guest's device tree 
> > is provided
> > by the Xen hypervisor itself and is generated at the virtual machine 
> > creation time: it is
> > populated with memory size, number of CPUs etc. based on [1].
> > So, even if we provide some device tree here it must not be used by U-boot 
> > at
> > the end of the day. Thus, it might be a reasonable solution to provide an 
> > empty device
> > tree as you do, but put a comment that it is not used.
>
> So another example of why this series is taking things in the wrong
> direction.

Why? At least we might figure out where to get the DT and how to run
Xen with U-Boot. I don't see any down-side to having a basic DT in the
tree along with instructions on how to run Xen.

Regards,
Simon

Re: [PATCH v6 07/25] arm: rpi: Add a devicetree file for rpi_4

[François Ozog]

Hi Mark,

On Thu, 2 Dec 2021 at 18:34, Mark Kettenis  wrote:

> > From: Simon Glass 
> > Date: Thu,  2 Dec 2021 08:59:01 -0700
> >
> > Add this file, obtained from the Raspbian boot disk, so there is a
> > reference devicetree in the U-Boot tree. The same one is used for
> > 32- and 64-bit variants.
> >
> > Note that U-Boot does not normally need this at runtime, since
> > CONFIG_OF_BOARD is enabled. The previous firmware stage provides a
> > devicetree at runtime.
> >
> > Signed-off-by: Simon Glass 
> > ---
> >
> > (no changes since v1)
> >
> >  arch/arm/dts/Makefile|3 +-
> >  arch/arm/dts/bcm2711-rpi-4-b.dts | 1958 ++
> >  configs/rpi_4_32b_defconfig  |1 +
> >  configs/rpi_4_defconfig  |1 +
> >  configs/rpi_arm64_defconfig  |1 +
> >  5 files changed, 1963 insertions(+), 1 deletion(-)
> >  create mode 100644 arch/arm/dts/bcm2711-rpi-4-b.dts
> >
> > diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
> > index 2d92b2f940d..9cddab37207 100644
> > --- a/arch/arm/dts/Makefile
> > +++ b/arch/arm/dts/Makefile
> > @@ -1077,7 +1077,8 @@ dtb-$(CONFIG_ARCH_BCM283X) += \
> >   bcm2837-rpi-3-a-plus.dtb \
> >   bcm2837-rpi-3-b.dtb \
> >   bcm2837-rpi-3-b-plus.dtb \
> > - bcm2837-rpi-cm3-io3.dtb
> > + bcm2837-rpi-cm3-io3.dtb \
> > + bcm2711-rpi-4-b.dtb
> >
> >  dtb-$(CONFIG_ARCH_BCM63158) += \
> >   bcm963158.dtb
> > diff --git a/arch/arm/dts/bcm2711-rpi-4-b.dts
> b/arch/arm/dts/bcm2711-rpi-4-b.dts
> > new file mode 100644
> > index 000..425e9fb91c4
> > --- /dev/null
> > +++ b/arch/arm/dts/bcm2711-rpi-4-b.dts
> > @@ -0,0 +1,1958 @@
> > +// SPDX-License-Identifier: GPL-2.0+ OR MIT
> > +/*
> > + * Sample device tree for rpi_4
> > +
> > + * Copyright 2021 Google LLC
> > + */
>
> This is of course an utter lie.  This wasn't written from scratch by a
> Google employee.
>
> The original copyright and license for the dtb files you
> "disassembled" applies.  You don't specify exactly where you obtained
> the file from, but it probably came from here:
>
>   https://github.com/raspberrypi/firmware
>
> And the README.md for that repo states that:
>
>   "The dtbs, overlays and associated README are built from Linux
>kernel sources, released under the GPL (see boot/COPYING.linux)"
>
> That's quite a point! thanks for bringing this additional "legal"
standpoint.

> They're probably talking about their own fork of the Linux kernel
> sources here as there are still differences between their device trees
> and the the device trees in the official Linux tree.  But if you
> insist on having a device tree in the U-Boot tree, that's where you
> should look.


> > +
> > +/dts-v1/;
> > +
> > +/memreserve/ 0x 0x1000;
> > +/ {
> > + compatible = "raspberrypi,4-model-b\0brcm,bcm2838\0brcm,bcm2837";
> > + model = "Raspberry Pi 4 Model B";
> > + interrupt-parent = <0x01>;
> > + #address-cells = <0x02>;
> > + #size-cells = <0x01>;
> > +
> > + aliases {
> > + serial0 = "/soc/serial@7e215040";
> > + serial1 = "/soc/serial@7e201000";
> > + audio = "/soc/audio";
> > + aux = "/soc/aux@7e215000";
> > + sound = "/soc/sound";
> > + soc = "/soc";
> > + dma = "/soc/dma@7e007000";
> > + watchdog = "/soc/watchdog@7e10";
> > + random = "/soc/rng@7e104000";
> > + mailbox = "/soc/mailbox@7e00b880";
> > + gpio = "/soc/gpio@7e20";
> > + uart0 = "/soc/serial@7e201000";
> > + sdhost = "/soc/mmc@7e202000";
> > + mmc0 = "/soc/emmc2@7e34";
> > + i2s = "/soc/i2s@7e203000";
> > + spi0 = "/soc/spi@7e204000";
> > + i2c0 = "/soc/i2c@7e205000";
> > + uart1 = "/soc/serial@7e215040";
> > + spi1 = "/soc/spi@7e215080";
> > + spi2 = "/soc/spi@7e2150c0";
> > + mmc = "/soc/mmc@7e30";
> > + mmc1 = "/soc/mmcnr@7e30";
> > + i2c1 = "/soc/i2c@7e804000";
> > + i2c2 = "/soc/i2c@7e805000";
> > + usb = "/soc/usb@7e98";
> > + leds = "/leds";
> > + fb = "/soc/fb";
> > + thermal = "/soc/thermal@7d5d2200";
> > + axiperf = "/soc/axiperf";
> > + mmc2 = "/soc/mmc@7e202000";
> > + ethernet0 = "/scb/genet@7d58";
> > + };
> > +
> > + chosen {
> > + bootargs = "8250.nr_uarts=1 cma=64M";
> > + };
> > +
> > + thermal-zones {
> > +
> > + cpu-thermal {
> > + polling-delay-passive = <0x00>;
> > + polling-delay = <0x3e8>;
> > + thermal-sensors = <0x02>;
> > + coefficients = <0xfe19 0x641b8>;
> > + phandle = <0x2f>;
> > +
> > + cooling-maps {
> > + };
> > +   

Re: [PATCH v6 09/25] arm: xenguest_arm64: Add a fake devicetree file

[François Ozog]

Hi Simon

On Thu, 2 Dec 2021 at 19:29, Simon Glass  wrote:

> Hi François,
>
> On Thu, 2 Dec 2021 at 11:17, François Ozog 
> wrote:
> >
> > Hi Simon
> >
> > On Thu, 2 Dec 2021 at 19:05, Simon Glass  wrote:
> >>
> >> Hi Tom,
> >>
> >> On Thu, 2 Dec 2021 at 10:56, Tom Rini  wrote:
> >> >
> >> > On Thu, Dec 02, 2021 at 05:40:46PM +, Oleksandr Andrushchenko
> wrote:
> >> > > Hi, Simon!
> >> > >
> >> > > Sorry for being late to the party
> >> > >
> >> > > On 02.12.21 17:59, Simon Glass wrote:
> >> > > > Add an empty file to prevent build errors when building with
> >> > > > CONFIG_OF_SEPARATE enabled.
> >> > > >
> >> > > > The build instructions in U-Boot do not provide enough detail to
> build a
> >> > > > useful devicetree, unfortunately.
> >> > > Xen guest doesn't use any built-in device trees as the guest's
> device tree is provided
> >> > > by the Xen hypervisor itself and is generated at the virtual
> machine creation time: it is
> >> > > populated with memory size, number of CPUs etc. based on [1].
> >> > > So, even if we provide some device tree here it must not be used by
> U-boot at
> >> > > the end of the day. Thus, it might be a reasonable solution to
> provide an empty device
> >> > > tree as you do, but put a comment that it is not used.
> >> >
> >> > So another example of why this series is taking things in the wrong
> >> > direction.
> >>
> >> Why?
> >
> > I only had that comment in mind: "there is none so deaf as he who will
> not hear."
>
> Hey, stop the pile-on. It's not useful.
>
> I've guided U-Boot's use of devicetree for 10 years successfully. The
> current state is a mess and I just to straighten it out.
>
> I admire your talent and knowledge.
I know you are 99,99% of the time correct and spot on for your comments in
many meetings we were attending.
When you questioned a number of points I made, I first tried to understand
what I got wrong because you said it.
And you were right ;-)
For this topic, I made every effort to come to your position, but
definitively can't.


>>
> >> At least we might figure out where to get the DT and how to run
> >> Xen with U-Boot. I don't see any down-side to having a basic DT in the
> >> tree along with instructions on how to run Xen.
> >
> > If an EMPTY device is what is required to pass current build
> constraints, so be it, and let's correct that in a later patch. And EMPTY
> is not basic.
>
> The problem here is a difference in philosophy around device tree.
>
> Indeed!

> Regards,
> Simon
>


-- 
François-Frédéric Ozog | *Director Business Development*
T: +33.67221.6485
francois.o...@linaro.org | Skype: ffozog

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Simon Glass]

Hi Tom,

On Thu, 2 Dec 2021 at 11:34, Tom Rini  wrote:
>
> On Thu, Dec 02, 2021 at 11:17:38AM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 2 Dec 2021 at 11:03, Tom Rini  wrote:
> > >
> > > On Thu, Dec 02, 2021 at 10:07:13AM -0700, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Thu, 2 Dec 2021 at 09:59, Tom Rini  wrote:
> > > > >
> > > > > On Thu, Dec 02, 2021 at 09:49:51AM -0700, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Thu, 2 Dec 2021 at 09:38, Tom Rini  wrote:
> > > > > > >
> > > > > > > On Thu, Dec 02, 2021 at 05:33:53PM +0100, François Ozog wrote:
> > > > > > > > Hi Simon
> > > > > > > >
> > > > > > > > Le jeu. 2 déc. 2021 à 17:00, Simon Glass  a 
> > > > > > > > écrit :
> > > > > > > >
> > > > > > > > > With Ilias' efforts we have dropped OF_PRIOR_STAGE and 
> > > > > > > > > OF_HOSTFILE so
> > > > > > > > > there are only three ways to obtain a devicetree:
> > > > > > > > >
> > > > > > > > >- OF_SEPARATE - the normal way, where the devicetree is 
> > > > > > > > > built and
> > > > > > > > >   appended to U-Boot
> > > > > > > > >- OF_EMBED - for development purposes, the devicetree is 
> > > > > > > > > embedded in
> > > > > > > > >   the ELF file (also used for EFI)
> > > > > > > > >- OF_BOARD - the board figures it out on its own
> > > > > > > > >
> > > > > > > > > The last one is currently set up so that no devicetree is 
> > > > > > > > > needed at all
> > > > > > > > > in the U-Boot tree. Most boards do provide one, but some 
> > > > > > > > > don't. Some
> > > > > > > > > don't even provide instructions on how to boot on the board.
> > > > > > > > >
> > > > > > > > > The problems with this approach are documented in another 
> > > > > > > > > patch in this
> > > > > > > > > series: "doc: Add documentation about devicetree usage"
> > > > > > > > >
> > > > > > > > > In practice, OF_BOARD is not really distinct from 
> > > > > > > > > OF_SEPARATE. Any board
> > > > > > > > > can obtain its devicetree at runtime, even it is has a 
> > > > > > > > > devicetree built
> > > > > > > > > in U-Boot. This is because U-Boot may be a second-stage 
> > > > > > > > > bootloader and its
> > > > > > > > > caller may have a better idea about the hardware available in 
> > > > > > > > > the machine.
> > > > > > > > > This is the case with a few QEMU boards, for example.
> > > > > > > > >
> > > > > > > > > So it makes no sense to have OF_BOARD as a 'choice'. It 
> > > > > > > > > should be an
> > > > > > > > > option, available with either OF_SEPARATE or OF_EMBED.
> > > > > > > > >
> > > > > > > > > This series makes this change, adding various missing 
> > > > > > > > > devicetree files
> > > > > > > > > (and placeholders) to make the build work.
> > > > > > > > >
> > > > > > > > > Note: If board maintainers are able to add their own patch to 
> > > > > > > > > add the
> > > > > > > > > files, some patches in this series can be dropped.
> > > > > > > > >
> > > > > > > > > It also provides a few qemu clean-ups discovered along the 
> > > > > > > > > way. The
> > > > > > > > > qemu-riscv64_spl problem is fixed.
> > > > > > > > >
> > > > > > > > > [1]
> > > > > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20210919215111.3830278-3-...@chromium.org/
> > > > > > > > >
> > > > > > > > > Changes in v6:
> > > > > > > > > - Fix description of OF_BOARD so it refers just to the 
> > > > > > > > > current state
> > > > > > > > > - Explain that the 'two devicetrees' refers to two *control* 
> > > > > > > > > devicetrees
> > > > > > > > > - Expand the commit message based on comments
> > > > > > > > > - Expand the commit message based on comments
> > > > > > > >
> > > > > > > > You haven’t addressed any concerns expressed on the mailing 
> > > > > > > > list.so I am
> > > > > > > > not in favor of this new version either.
> > > > > > > > If you make a version without « fake DTs » as you name them, 
> > > > > > > > there are good
> > > > > > > > advances in the documentation and other areas that would be 
> > > > > > > > better in
> > > > > > > > mainline….
> > > > > > > > If I am the only one thinking this way and the patch can be 
> > > > > > > > accepted, I
> > > > > > > > would love there is a warning in capital letters at the top of 
> > > > > > > > the DTS fake
> > > > > > > > files that explains the intent of this fake DT, the possible 
> > > > > > > > outcomes of
> > > > > > > > not using the one provided by the platform and the right way of 
> > > > > > > > dealing
> > > > > > > > with DTs for the platform.
> > > > > > >
> > > > > > > This is the part that I too am still unhappy about.  I do not want
> > > > > > > reference or fake or whatever device trees in the U-Boot source 
> > > > > > > tree.
> > > > > > > We should be able to _remove_ the ones we have, that are not 
> > > > > > > required,
> > > > > > > with doc/board/...rst explaining how to get / view one.  Not 
> > > > > > > adding
> > > > > > > more.
> > > > > >
> > > > > > I understand 

Re: Please pull u-boot-net/next

[Tom Rini]

On Thu, Dec 02, 2021 at 10:25:19AM +0200, Ramon Fried wrote:

> Hi Tom,
> This pull request contains:
> * New Broadcom NetXtreme driver
> * Support for socat for netconsole
> * Felix switch soft reset fix
> 
> The following changes since commit fc47dbb26e9d86a688e69e198b2ed0749db16756:
> 
>   Merge branch '2021-12-01-Kconfig-migrations' into next (2021-12-01
> 13:32:35 -0500)
> 
> are available in the Git repository at:
> 
>   https://source.denx.de/u-boot/custodians/u-boot-net.git/
> 
> for you to fetch changes up to 300761b68da14fc77f3e236f35c459fb1a6769ce:
> 
>   board: brcm-ns3: Load netXtreme firmware (2021-12-02 08:34:01 +0200)
> 

Applied to u-boot/next, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH] pci: Disable I/O forwarding during autoconfiguration if unsupported

[Stefan Roese]

On 12/2/21 00:08, Pali Rohár wrote:

On Tuesday 30 November 2021 07:09:36 Stefan Roese wrote:

On 11/25/21 11:32, Pali Rohár wrote:

If U-Boot does not have any I/O resource for assignment then disable I/O
forwarding in PCI bridge autoconfiguration code. Default initial state of
PCI bridge IO registers is unspecified, therefore they can be in enabled if
U-Boot does not touch them.

Signed-off-by: Pali Rohár 

---
   drivers/pci/pci_auto.c | 8 
   1 file changed, 8 insertions(+)

diff --git a/drivers/pci/pci_auto.c b/drivers/pci/pci_auto.c
index 7e6ee54be087..6e5ed194f247 100644
--- a/drivers/pci/pci_auto.c
+++ b/drivers/pci/pci_auto.c
@@ -265,6 +265,14 @@ void dm_pciauto_prescan_setup_bridge(struct udevice *dev, 
int sub_bus)
  (pci_io->bus_lower & 0x) >> 16);
cmdstat |= PCI_COMMAND_IO;
+   } else {
+   /* Disable I/O if unsupported */
+   dm_pci_write_config8(dev, PCI_IO_BASE, 0xf0 | io_32);
+   dm_pci_write_config8(dev, PCI_IO_LIMIT, 0x0 | io_32);


Does it perhaps make sense to add / use a macro for this 0xf0 above?


For setting range bits we already have macro PCI_IO_RANGE_MASK.

So for setting all base range bits to one (which is this patch is
suppose to do) I can write something like this: (~0 & PCI_IO_RANGE_MASK)
or (0xff & PCI_IO_RANGE_MASK). (write_config8 denotes that we use 8bit
numbers...)

I/O forwarding is disabled when base address is higher than limit
address and it is common to choose base address as the highest possible
(hence all ones) and limit address to lowest possible (hence all zeros).

So question is, do we need macro which evaluates to number with all
zeros and another macro which evaluates to number with all ones?

I'm not sure. For me is "0xf0 | io_32" more readable as "the verbose
usage with macro" "(0xff & PCI_IO_RANGE_MASK) | io_32".


I have no hard feeling here to really change this - was just checking
here.


But this is all just about "naming conventions" and "coding style". If
you think that for consistency is better to create macro or use another
construction, please let me know other ideas. I would follow any style
which is expected here...


You already have my RB tag, so all is fine IMHO.

Thanks,
Stefan




Other than this:

Reviewed-by: Stefan Roese 

Thanks,
Stefan



+   if (io_32 == PCI_IO_RANGE_TYPE_32) {
+   dm_pci_write_config16(dev, PCI_IO_BASE_UPPER16, 0x0);
+   dm_pci_write_config16(dev, PCI_IO_LIMIT_UPPER16, 0x0);
+   }
}
/* Enable memory and I/O accesses, enable bus master */



Viele Grüße,
Stefan Roese

--
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de


Viele Grüße,
Stefan Roese

--
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de

[PATCH v6 08/25] arm: vexpress: Add a devicetree file for juno

[Simon Glass]

Add this file, obtained from the Linaro website[1], so there is a
reference file in the U-Boot tree.

Note that U-Boot does not normally need this at runtime, since
CONFIG_OF_BOARD is enabled. The previous firmware stage provides a
devicetree at runtime.


[1] https://releases.linaro.org/android/reference-lcr/juno/7.1-17.05/

Signed-off-by: Simon Glass 
---

(no changes since v1)

 arch/arm/dts/Makefile  |3 +
 arch/arm/dts/juno-r2.dts   | 1038 
 configs/vexpress_aemv8a_juno_defconfig |1 +
 3 files changed, 1042 insertions(+)
 create mode 100644 arch/arm/dts/juno-r2.dts

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 9cddab37207..d53bae2c350 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -1149,7 +1149,10 @@ dtb-$(CONFIG_TARGET_GE_BX50V3) += \
 dtb-$(CONFIG_TARGET_GE_B1X5V2) += imx6dl-b1x5v2.dtb
 dtb-$(CONFIG_TARGET_MX53PPD) += imx53-ppd.dtb
 
+# TODO(Linus Walleij ): Should us a single vexpress
+# Kconfig option to build all of these. See examples above.
 dtb-$(CONFIG_TARGET_VEXPRESS_CA9X4) += vexpress-v2p-ca9.dtb
+dtb-$(CONFIG_TARGET_VEXPRESS64_JUNO) += juno-r2.dtb
 
 dtb-$(CONFIG_TARGET_TOTAL_COMPUTE) += total_compute.dtb
 
diff --git a/arch/arm/dts/juno-r2.dts b/arch/arm/dts/juno-r2.dts
new file mode 100644
index 000..5a536d8100e
--- /dev/null
+++ b/arch/arm/dts/juno-r2.dts
@@ -0,0 +1,1038 @@
+// SPDX-License-Identifier: GPL-2.0+ OR MIT
+/*
+ * Sample device tree for juno
+
+ * Copyright 2021 Google LLC
+ */
+
+/dts-v1/;
+
+/ {
+   model = "ARM Juno development board (r2)";
+   compatible = "arm,juno-r2\0arm,juno";
+   interrupt-parent = <0x01>;
+   #address-cells = <0x02>;
+   #size-cells = <0x02>;
+
+   aliases {
+   serial0 = "/uart@7ff8";
+   };
+
+   chosen {
+   stdout-path = "serial0:115200n8";
+   };
+
+   psci {
+   compatible = "arm,psci-0.2";
+   method = "smc";
+   };
+
+   cpus {
+   #address-cells = <0x02>;
+   #size-cells = <0x00>;
+
+   cpu-map {
+
+   cluster0 {
+
+   core0 {
+   cpu = <0x02>;
+   };
+
+   core1 {
+   cpu = <0x03>;
+   };
+   };
+
+   cluster1 {
+
+   core0 {
+   cpu = <0x04>;
+   };
+
+   core1 {
+   cpu = <0x05>;
+   };
+
+   core2 {
+   cpu = <0x06>;
+   };
+
+   core3 {
+   cpu = <0x07>;
+   };
+   };
+   };
+
+   idle-states {
+   entry-method = "arm,psci";
+
+   cpu-sleep-0 {
+   compatible = "arm,idle-state";
+   arm,psci-suspend-param = <0x1>;
+   local-timer-stop;
+   entry-latency-us = <0x12c>;
+   exit-latency-us = <0x4b0>;
+   min-residency-us = <0x7d0>;
+   linux,phandle = <0x0a>;
+   phandle = <0x0a>;
+   };
+
+   cluster-sleep-0 {
+   compatible = "arm,idle-state";
+   arm,psci-suspend-param = <0x101>;
+   local-timer-stop;
+   entry-latency-us = <0x190>;
+   exit-latency-us = <0x4b0>;
+   min-residency-us = <0x9c4>;
+   linux,phandle = <0x0b>;
+   phandle = <0x0b>;
+   };
+   };
+
+   cpu@0 {
+   compatible = "arm,cortex-a72\0arm,armv8";
+   reg = <0x00 0x00>;
+   device_type = "cpu";
+   enable-method = "psci";
+   next-level-cache = <0x08>;
+   clocks = <0x09 0x00>;
+   cpu-idle-states = <0x0a 0x0b>;
+   sched-energy-costs = <0x0c 0x0d>;
+   #cooling-cells = <0x02>;
+   dynamic-power-coefficient = <0x1c2>;
+   linux,phandle = <0x02>;
+   phandle = <0x02>;
+   };
+
+   cpu@1 {
+   compatible = "arm,cortex-a72\0arm,armv8";
+

[PATCH v6 12/25] arm: bcm7xxx: Add a devicetree file

[Simon Glass]

Add a dummy devicetree file for these boards. It seems to be possible to
obtain a real one from another bootloader called 'bolt' but I will leave
this to the maintainer.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 arch/arm/dts/Makefile |  2 ++
 arch/arm/dts/bcm7xxx.dts  | 15 +++
 configs/bcm7260_defconfig |  1 +
 configs/bcm7445_defconfig |  1 +
 4 files changed, 19 insertions(+)
 create mode 100644 arch/arm/dts/bcm7xxx.dts

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index b07a1b3378e..8bb15b96cc7 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -1092,6 +1092,8 @@ dtb-$(CONFIG_ARCH_BCM6858) += \
 
 dtb-$(CONFIG_TARGET_BCMNS3) += ns3-board.dtb
 
+dtb-$(CONFIG_ARCH_BCMSTB) += bcm7xxx.dtb
+
 dtb-$(CONFIG_ASPEED_AST2500) += ast2500-evb.dtb
 dtb-$(CONFIG_ASPEED_AST2600) += ast2600-evb.dtb
 
diff --git a/arch/arm/dts/bcm7xxx.dts b/arch/arm/dts/bcm7xxx.dts
new file mode 100644
index 000..799cc9caad4
--- /dev/null
+++ b/arch/arm/dts/bcm7xxx.dts
@@ -0,0 +1,15 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Dummy devicetre file for bcm7260 board
+ *
+ * This is required to make the board build with CONFIG OF_SEPARATE
+ * In-tree document explains how to obtain a real devicetree using 'bolt' but
+ * I did not attempt this.
+ *
+ * Copyright 2021 Google LLC
+ */
+
+/dts-v1/;
+
+/ {
+};
diff --git a/configs/bcm7260_defconfig b/configs/bcm7260_defconfig
index 257d81052a8..2b527b65770 100644
--- a/configs/bcm7260_defconfig
+++ b/configs/bcm7260_defconfig
@@ -7,6 +7,7 @@ CONFIG_TARGET_BCM7260=y
 CONFIG_NR_DRAM_BANKS=1
 CONFIG_ENV_SIZE=0x1
 CONFIG_ENV_OFFSET=0x814800
+CONFIG_DEFAULT_DEVICE_TREE="bcm7xxx"
 CONFIG_ENV_OFFSET_REDUND=0x824800
 CONFIG_SYS_LOAD_ADDR=0x0200
 CONFIG_FIT=y
diff --git a/configs/bcm7445_defconfig b/configs/bcm7445_defconfig
index 9ffa436e454..3ae678ba56e 100644
--- a/configs/bcm7445_defconfig
+++ b/configs/bcm7445_defconfig
@@ -8,6 +8,7 @@ CONFIG_NR_DRAM_BANKS=3
 CONFIG_ENV_SIZE=0x1
 CONFIG_ENV_OFFSET=0x1E
 CONFIG_ENV_SECT_SIZE=0x1
+CONFIG_DEFAULT_DEVICE_TREE="bcm7xxx"
 CONFIG_ENV_OFFSET_REDUND=0x1F
 CONFIG_SYS_LOAD_ADDR=0x0200
 CONFIG_FIT=y
-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH v6 10/25] arm: octeontx: Add a fake devicetree file

[Simon Glass]

Add an empty file to prevent build errors when building with
CONFIG_OF_SEPARATE enabled.

Unfortunately there are no build instructions in the U-Boot tree to enable
a real file to be created.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 arch/arm/dts/Makefile|  3 +++
 arch/arm/dts/octeontx.dts| 14 ++
 configs/octeontx2_95xx_defconfig |  1 +
 configs/octeontx2_96xx_defconfig |  1 +
 configs/octeontx_81xx_defconfig  |  1 +
 configs/octeontx_83xx_defconfig  |  1 +
 6 files changed, 21 insertions(+)
 create mode 100644 arch/arm/dts/octeontx.dts

diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index f6345988c8c..91302118598 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -1142,6 +1142,9 @@ dtb-$(CONFIG_ARCH_MEDIATEK) += \
 
 dtb-$(CONFIG_XEN) += xenguest-arm64.dtb
 
+dtb-$(CONFIG_ARCH_OCTEONTX) += octeontx.dtb
+dtb-$(CONFIG_ARCH_OCTEONTX2) += octeontx.dtb
+
 dtb-$(CONFIG_TARGET_GE_BX50V3) += \
imx6q-bx50v3.dtb \
imx6q-b850v3.dtb \
diff --git a/arch/arm/dts/octeontx.dts b/arch/arm/dts/octeontx.dts
new file mode 100644
index 000..60a15f5df23
--- /dev/null
+++ b/arch/arm/dts/octeontx.dts
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Dummy devicetre file for octeontx2 boards
+ *
+ * This is required to make the board build with CONFIG OF_SEPARATE
+ * I could not find any in-tree documentation at all so this is a dummy file.
+ *
+ * Copyright 2021 Google LLC
+ */
+
+/dts-v1/;
+
+/ {
+};
diff --git a/configs/octeontx2_95xx_defconfig b/configs/octeontx2_95xx_defconfig
index e1b86a5a8b6..e3df390c94b 100644
--- a/configs/octeontx2_95xx_defconfig
+++ b/configs/octeontx2_95xx_defconfig
@@ -12,6 +12,7 @@ CONFIG_ENV_OFFSET=0xF0
 CONFIG_ENV_SECT_SIZE=0x1
 CONFIG_TARGET_OCTEONTX2_95XX=y
 CONFIG_DM_GPIO=y
+CONFIG_DEFAULT_DEVICE_TREE="octeontx"
 CONFIG_DEBUG_UART_BASE=0x87e02800
 CONFIG_DEBUG_UART_CLOCK=2400
 CONFIG_DEBUG_UART=y
diff --git a/configs/octeontx2_96xx_defconfig b/configs/octeontx2_96xx_defconfig
index ec03d959771..0478b3068a6 100644
--- a/configs/octeontx2_96xx_defconfig
+++ b/configs/octeontx2_96xx_defconfig
@@ -10,6 +10,7 @@ CONFIG_ENV_OFFSET=0xF0
 CONFIG_ENV_SECT_SIZE=0x1
 CONFIG_TARGET_OCTEONTX2_96XX=y
 CONFIG_DM_GPIO=y
+CONFIG_DEFAULT_DEVICE_TREE="octeontx"
 CONFIG_DEBUG_UART_BASE=0x87e02800
 CONFIG_DEBUG_UART_CLOCK=2400
 CONFIG_DEBUG_UART=y
diff --git a/configs/octeontx_81xx_defconfig b/configs/octeontx_81xx_defconfig
index d0728ac3c67..d871be8f812 100644
--- a/configs/octeontx_81xx_defconfig
+++ b/configs/octeontx_81xx_defconfig
@@ -12,6 +12,7 @@ CONFIG_ENV_OFFSET=0xF0
 CONFIG_ENV_SECT_SIZE=0x1
 CONFIG_TARGET_OCTEONTX_81XX=y
 CONFIG_DM_GPIO=y
+CONFIG_DEFAULT_DEVICE_TREE="octeontx"
 CONFIG_DEBUG_UART_BASE=0x87e02800
 CONFIG_DEBUG_UART_CLOCK=2400
 CONFIG_DEBUG_UART=y
diff --git a/configs/octeontx_83xx_defconfig b/configs/octeontx_83xx_defconfig
index 6c9609b0cd3..94b072bf6fa 100644
--- a/configs/octeontx_83xx_defconfig
+++ b/configs/octeontx_83xx_defconfig
@@ -10,6 +10,7 @@ CONFIG_ENV_OFFSET=0xF0
 CONFIG_ENV_SECT_SIZE=0x1
 CONFIG_TARGET_OCTEONTX_83XX=y
 CONFIG_DM_GPIO=y
+CONFIG_DEFAULT_DEVICE_TREE="octeontx"
 CONFIG_DEBUG_UART_BASE=0x87e02800
 CONFIG_DEBUG_UART_CLOCK=2400
 CONFIG_DEBUG_UART=y
-- 
2.34.0.rc2.393.gf8c9666880-goog

[PATCH v6 21/25] fdt: Drop #ifdef around board_fdt_blob_setup()

[Simon Glass]

This serves no purpose. Drop it.

Signed-off-by: Simon Glass 
---

(no changes since v1)

 lib/fdtdec.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/lib/fdtdec.c b/lib/fdtdec.c
index fbdc92c0813..299a2c3a32f 100644
--- a/lib/fdtdec.c
+++ b/lib/fdtdec.c
@@ -1203,7 +1203,6 @@ static int uncompress_blob(const void *src, ulong sz_src, 
void **dstp)
return 0;
 }
 
-#if defined(CONFIG_OF_BOARD) || defined(CONFIG_OF_SEPARATE)
 /*
  * For CONFIG_OF_SEPARATE, the board may optionally implement this to
  * provide and/or fixup the fdt.
@@ -1226,7 +1225,6 @@ __weak void *board_fdt_blob_setup(int *err)
 
return fdt_blob;
 }
-#endif
 
 int fdtdec_set_ethernet_mac_address(void *fdt, const u8 *mac, size_t size)
 {
-- 
2.34.0.rc2.393.gf8c9666880-goog

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Tom Rini]

On Thu, Dec 02, 2021 at 05:33:53PM +0100, François Ozog wrote:
> Hi Simon
> 
> Le jeu. 2 déc. 2021 à 17:00, Simon Glass  a écrit :
> 
> > With Ilias' efforts we have dropped OF_PRIOR_STAGE and OF_HOSTFILE so
> > there are only three ways to obtain a devicetree:
> >
> >- OF_SEPARATE - the normal way, where the devicetree is built and
> >   appended to U-Boot
> >- OF_EMBED - for development purposes, the devicetree is embedded in
> >   the ELF file (also used for EFI)
> >- OF_BOARD - the board figures it out on its own
> >
> > The last one is currently set up so that no devicetree is needed at all
> > in the U-Boot tree. Most boards do provide one, but some don't. Some
> > don't even provide instructions on how to boot on the board.
> >
> > The problems with this approach are documented in another patch in this
> > series: "doc: Add documentation about devicetree usage"
> >
> > In practice, OF_BOARD is not really distinct from OF_SEPARATE. Any board
> > can obtain its devicetree at runtime, even it is has a devicetree built
> > in U-Boot. This is because U-Boot may be a second-stage bootloader and its
> > caller may have a better idea about the hardware available in the machine.
> > This is the case with a few QEMU boards, for example.
> >
> > So it makes no sense to have OF_BOARD as a 'choice'. It should be an
> > option, available with either OF_SEPARATE or OF_EMBED.
> >
> > This series makes this change, adding various missing devicetree files
> > (and placeholders) to make the build work.
> >
> > Note: If board maintainers are able to add their own patch to add the
> > files, some patches in this series can be dropped.
> >
> > It also provides a few qemu clean-ups discovered along the way. The
> > qemu-riscv64_spl problem is fixed.
> >
> > [1]
> > https://patchwork.ozlabs.org/project/uboot/patch/20210919215111.3830278-3-...@chromium.org/
> >
> > Changes in v6:
> > - Fix description of OF_BOARD so it refers just to the current state
> > - Explain that the 'two devicetrees' refers to two *control* devicetrees
> > - Expand the commit message based on comments
> > - Expand the commit message based on comments
> 
> You haven’t addressed any concerns expressed on the mailing list.so I am
> not in favor of this new version either.
> If you make a version without « fake DTs » as you name them, there are good
> advances in the documentation and other areas that would be better in
> mainline….
> If I am the only one thinking this way and the patch can be accepted, I
> would love there is a warning in capital letters at the top of the DTS fake
> files that explains the intent of this fake DT, the possible outcomes of
> not using the one provided by the platform and the right way of dealing
> with DTs for the platform.

This is the part that I too am still unhappy about.  I do not want
reference or fake or whatever device trees in the U-Boot source tree.
We should be able to _remove_ the ones we have, that are not required,
with doc/board/...rst explaining how to get / view one.  Not adding
more.

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Heinrich Schuchardt]

On 12/2/21 16:58, Simon Glass wrote:

With Ilias' efforts we have dropped OF_PRIOR_STAGE and OF_HOSTFILE so
there are only three ways to obtain a devicetree:

- OF_SEPARATE - the normal way, where the devicetree is built and
   appended to U-Boot
- OF_EMBED - for development purposes, the devicetree is embedded in
   the ELF file (also used for EFI)
- OF_BOARD - the board figures it out on its own

The last one is currently set up so that no devicetree is needed at all
in the U-Boot tree. Most boards do provide one, but some don't. Some
don't even provide instructions on how to boot on the board.

The problems with this approach are documented in another patch in this
series: "doc: Add documentation about devicetree usage"

In practice, OF_BOARD is not really distinct from OF_SEPARATE. Any board
can obtain its devicetree at runtime, even it is has a devicetree built
in U-Boot. This is because U-Boot may be a second-stage bootloader and its
caller may have a better idea about the hardware available in the machine.
This is the case with a few QEMU boards, for example.

So it makes no sense to have OF_BOARD as a 'choice'. It should be an
option, available with either OF_SEPARATE or OF_EMBED.

This series makes this change, adding various missing devicetree files
(and placeholders) to make the build work.

Note: If board maintainers are able to add their own patch to add the
files, some patches in this series can be dropped.

It also provides a few qemu clean-ups discovered along the way. The
qemu-riscv64_spl problem is fixed.


Distros like Ubuntu are provided as preinstalled images using U-Boot to 
launch Linux for usage with QEMU. A single image must be able to be 
usable in the future irrespective of the QEMU command line device 
configuration.


This means that the devicetree coming from QEMU must be accurately 
parsed in U-Boot to setup the UEFI memory map. The number and type of 
CPUs and the NUMA configuration must be accurate. All devices enabled 
via the QEMU command line must be visible in the device-tree of Linux.


Please, observe that information like number of CPU cores, number and 
type of block devices, namespace IDs used for NVMe drives, etc. cannot 
be available at build time.


It this all guaranteed with this series? If not, this would 
unfortunately imply a NAK.


Best regards

Heinrich



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

Changes in v6:
- Fix description of OF_BOARD so it refers just to the current state
- Explain that the 'two devicetrees' refers to two *control* devicetrees
- Expand the commit message based on comments
- Expand the commit message based on comments

Changes in v5:
- Bring into the OF_BOARD series
- Rebase to master and drop mention of OF_PRIOR_STAGE, since removed
- Refer to the 'control' DTB in the first paragraph
- Use QEMU instead of qemu
- Merge RISC-V and ARM patches since they are similar
- Add new patches to clean up fdtdec_setup() and surrounds

Changes in v3:
- Clarify the 'bug' refered to at the top
- Reword 'This means that there' paragraph to explain U-Boot-specific things
- Move to doc/develop/devicetree now that OF_CONTROL is in the docs

Changes in v2:
- Fix typos per Sean (thank you!) and a few others
- Add a 'Use of U-Boot /config node' section
- Drop mention of dm-verity since that actually uses the kernel cmdline
- Explain that OF_BOARD will still work after these changes (in
   'Once this bug is fixed...' paragraph)
- Expand a bit on the reason why the 'Current situation' is bad
- Clarify in a second place that Linux and U-Boot use the same devicetree
   in 'To be clear, while U-Boot...'
- Expand on why we should have rules for other projects in
   'Devicetree in another project'
- Add a comment as to why devicetree in U-Boot is not 'bad design'
- Reword 'in-tree U-Boot devicetree' to 'devicetree source in U-Boot'
- Rewrite 'Devicetree generated on-the-fly in another project' to cover
   points raised on v1
- Add 'Why does U-Boot have its nodes and properties?'
- Add 'Why not have two devicetrees?'

Simon Glass (25):
   doc: Add documentation about devicetree usage
   arm: qemu: Mention -nographic in the docs
   arm: riscv: qemu: Explain how to extract the generated dt
   arm: qemu: Add a devicetree file for qemu_arm
   arm: qemu: Add a devicetree file for qemu_arm64
   riscv: qemu: Add devicetree files for qemu_riscv32/64
   arm: rpi: Add a devicetree file for rpi_4
   arm: vexpress: Add a devicetree file for juno
   arm: xenguest_arm64: Add a fake devicetree file
   arm: octeontx: Add a fake devicetree file
   arm: xilinx_versal_virt: Add a devicetree file
   arm: bcm7xxx: Add a devicetree file
   arm: qemu-ppce500: Add a devicetree file
   arm: highbank: Add a fake devicetree file
   fdt: Make OF_BOARD a bool option
   Drop CONFIG_BINMAN_STANDALONE_FDT
   doc: Update info on devicetree update
   fdt: Move MULTI_DTB_FIT handling out of fdtdec_setup()
   fdt: Drop 

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Simon Glass]

Hi Tom,

On Thu, 2 Dec 2021 at 09:38, Tom Rini  wrote:
>
> On Thu, Dec 02, 2021 at 05:33:53PM +0100, François Ozog wrote:
> > Hi Simon
> >
> > Le jeu. 2 déc. 2021 à 17:00, Simon Glass  a écrit :
> >
> > > With Ilias' efforts we have dropped OF_PRIOR_STAGE and OF_HOSTFILE so
> > > there are only three ways to obtain a devicetree:
> > >
> > >- OF_SEPARATE - the normal way, where the devicetree is built and
> > >   appended to U-Boot
> > >- OF_EMBED - for development purposes, the devicetree is embedded in
> > >   the ELF file (also used for EFI)
> > >- OF_BOARD - the board figures it out on its own
> > >
> > > The last one is currently set up so that no devicetree is needed at all
> > > in the U-Boot tree. Most boards do provide one, but some don't. Some
> > > don't even provide instructions on how to boot on the board.
> > >
> > > The problems with this approach are documented in another patch in this
> > > series: "doc: Add documentation about devicetree usage"
> > >
> > > In practice, OF_BOARD is not really distinct from OF_SEPARATE. Any board
> > > can obtain its devicetree at runtime, even it is has a devicetree built
> > > in U-Boot. This is because U-Boot may be a second-stage bootloader and its
> > > caller may have a better idea about the hardware available in the machine.
> > > This is the case with a few QEMU boards, for example.
> > >
> > > So it makes no sense to have OF_BOARD as a 'choice'. It should be an
> > > option, available with either OF_SEPARATE or OF_EMBED.
> > >
> > > This series makes this change, adding various missing devicetree files
> > > (and placeholders) to make the build work.
> > >
> > > Note: If board maintainers are able to add their own patch to add the
> > > files, some patches in this series can be dropped.
> > >
> > > It also provides a few qemu clean-ups discovered along the way. The
> > > qemu-riscv64_spl problem is fixed.
> > >
> > > [1]
> > > https://patchwork.ozlabs.org/project/uboot/patch/20210919215111.3830278-3-...@chromium.org/
> > >
> > > Changes in v6:
> > > - Fix description of OF_BOARD so it refers just to the current state
> > > - Explain that the 'two devicetrees' refers to two *control* devicetrees
> > > - Expand the commit message based on comments
> > > - Expand the commit message based on comments
> >
> > You haven’t addressed any concerns expressed on the mailing list.so I am
> > not in favor of this new version either.
> > If you make a version without « fake DTs » as you name them, there are good
> > advances in the documentation and other areas that would be better in
> > mainline….
> > If I am the only one thinking this way and the patch can be accepted, I
> > would love there is a warning in capital letters at the top of the DTS fake
> > files that explains the intent of this fake DT, the possible outcomes of
> > not using the one provided by the platform and the right way of dealing
> > with DTs for the platform.
>
> This is the part that I too am still unhappy about.  I do not want
> reference or fake or whatever device trees in the U-Boot source tree.
> We should be able to _remove_ the ones we have, that are not required,
> with doc/board/...rst explaining how to get / view one.  Not adding
> more.

I understand you don't like it and that others don't as well. I wish
it had not come to this.

However we are only talking about 10 boards, three of which don't even
have a devicetree anywhere I can find.

I think on balance this is a substantial clean-up. I am happy to add
whatever caveats and documentation people want to clarify what is
going on here. I'm happy to look at future options where the
devicetree is hosted elsewhere, so long as it is trivial to build it
within U-Boot for development purposes.

I'll also note that the bootstd series shows the devicetree source:

Core:  246 devices, 88 uclasses, devicetree: board

But for now, I still feel this is the best path forward.

Regards,
Simon

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Simon Glass]

Hi Heinrich,

On Thu, 2 Dec 2021 at 09:47, Heinrich Schuchardt
 wrote:
>
> On 12/2/21 16:58, Simon Glass wrote:
> > With Ilias' efforts we have dropped OF_PRIOR_STAGE and OF_HOSTFILE so
> > there are only three ways to obtain a devicetree:
> >
> > - OF_SEPARATE - the normal way, where the devicetree is built and
> >appended to U-Boot
> > - OF_EMBED - for development purposes, the devicetree is embedded in
> >the ELF file (also used for EFI)
> > - OF_BOARD - the board figures it out on its own
> >
> > The last one is currently set up so that no devicetree is needed at all
> > in the U-Boot tree. Most boards do provide one, but some don't. Some
> > don't even provide instructions on how to boot on the board.
> >
> > The problems with this approach are documented in another patch in this
> > series: "doc: Add documentation about devicetree usage"
> >
> > In practice, OF_BOARD is not really distinct from OF_SEPARATE. Any board
> > can obtain its devicetree at runtime, even it is has a devicetree built
> > in U-Boot. This is because U-Boot may be a second-stage bootloader and its
> > caller may have a better idea about the hardware available in the machine.
> > This is the case with a few QEMU boards, for example.
> >
> > So it makes no sense to have OF_BOARD as a 'choice'. It should be an
> > option, available with either OF_SEPARATE or OF_EMBED.
> >
> > This series makes this change, adding various missing devicetree files
> > (and placeholders) to make the build work.
> >
> > Note: If board maintainers are able to add their own patch to add the
> > files, some patches in this series can be dropped.
> >
> > It also provides a few qemu clean-ups discovered along the way. The
> > qemu-riscv64_spl problem is fixed.
>
> Distros like Ubuntu are provided as preinstalled images using U-Boot to
> launch Linux for usage with QEMU. A single image must be able to be
> usable in the future irrespective of the QEMU command line device
> configuration.
>
> This means that the devicetree coming from QEMU must be accurately
> parsed in U-Boot to setup the UEFI memory map. The number and type of
> CPUs and the NUMA configuration must be accurate. All devices enabled
> via the QEMU command line must be visible in the device-tree of Linux.
>
> Please, observe that information like number of CPU cores, number and
> type of block devices, namespace IDs used for NVMe drives, etc. cannot
> be available at build time.
>
> It this all guaranteed with this series? If not, this would
> unfortunately imply a NAK.

Yes, it is guaranteed and there is no change there.

Regards,
Simon

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Tom Rini]

On Thu, Dec 02, 2021 at 09:49:51AM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 2 Dec 2021 at 09:38, Tom Rini  wrote:
> >
> > On Thu, Dec 02, 2021 at 05:33:53PM +0100, François Ozog wrote:
> > > Hi Simon
> > >
> > > Le jeu. 2 déc. 2021 à 17:00, Simon Glass  a écrit :
> > >
> > > > With Ilias' efforts we have dropped OF_PRIOR_STAGE and OF_HOSTFILE so
> > > > there are only three ways to obtain a devicetree:
> > > >
> > > >- OF_SEPARATE - the normal way, where the devicetree is built and
> > > >   appended to U-Boot
> > > >- OF_EMBED - for development purposes, the devicetree is embedded in
> > > >   the ELF file (also used for EFI)
> > > >- OF_BOARD - the board figures it out on its own
> > > >
> > > > The last one is currently set up so that no devicetree is needed at all
> > > > in the U-Boot tree. Most boards do provide one, but some don't. Some
> > > > don't even provide instructions on how to boot on the board.
> > > >
> > > > The problems with this approach are documented in another patch in this
> > > > series: "doc: Add documentation about devicetree usage"
> > > >
> > > > In practice, OF_BOARD is not really distinct from OF_SEPARATE. Any board
> > > > can obtain its devicetree at runtime, even it is has a devicetree built
> > > > in U-Boot. This is because U-Boot may be a second-stage bootloader and 
> > > > its
> > > > caller may have a better idea about the hardware available in the 
> > > > machine.
> > > > This is the case with a few QEMU boards, for example.
> > > >
> > > > So it makes no sense to have OF_BOARD as a 'choice'. It should be an
> > > > option, available with either OF_SEPARATE or OF_EMBED.
> > > >
> > > > This series makes this change, adding various missing devicetree files
> > > > (and placeholders) to make the build work.
> > > >
> > > > Note: If board maintainers are able to add their own patch to add the
> > > > files, some patches in this series can be dropped.
> > > >
> > > > It also provides a few qemu clean-ups discovered along the way. The
> > > > qemu-riscv64_spl problem is fixed.
> > > >
> > > > [1]
> > > > https://patchwork.ozlabs.org/project/uboot/patch/20210919215111.3830278-3-...@chromium.org/
> > > >
> > > > Changes in v6:
> > > > - Fix description of OF_BOARD so it refers just to the current state
> > > > - Explain that the 'two devicetrees' refers to two *control* devicetrees
> > > > - Expand the commit message based on comments
> > > > - Expand the commit message based on comments
> > >
> > > You haven’t addressed any concerns expressed on the mailing list.so I am
> > > not in favor of this new version either.
> > > If you make a version without « fake DTs » as you name them, there are 
> > > good
> > > advances in the documentation and other areas that would be better in
> > > mainline….
> > > If I am the only one thinking this way and the patch can be accepted, I
> > > would love there is a warning in capital letters at the top of the DTS 
> > > fake
> > > files that explains the intent of this fake DT, the possible outcomes of
> > > not using the one provided by the platform and the right way of dealing
> > > with DTs for the platform.
> >
> > This is the part that I too am still unhappy about.  I do not want
> > reference or fake or whatever device trees in the U-Boot source tree.
> > We should be able to _remove_ the ones we have, that are not required,
> > with doc/board/...rst explaining how to get / view one.  Not adding
> > more.
> 
> I understand you don't like it and that others don't as well. I wish
> it had not come to this.
> 
> However we are only talking about 10 boards, three of which don't even
> have a devicetree anywhere I can find.
> 
> I think on balance this is a substantial clean-up. I am happy to add
> whatever caveats and documentation people want to clarify what is
> going on here. I'm happy to look at future options where the
> devicetree is hosted elsewhere, so long as it is trivial to build it
> within U-Boot for development purposes.
> 
> I'll also note that the bootstd series shows the devicetree source:
> 
> Core:  246 devices, 88 uclasses, devicetree: board
> 
> But for now, I still feel this is the best path forward.

I'm not sure how to proceed here.  The reviews are rather strongly
against the "include a device tree that won't be used".  The use case of
"but for development someone might need to modify the device tree" is
handled by platforms documenting where / how to get the real one.  We
should even update the Kconfig help to note that if you enable this your
board docs MUST explain where the device tree can be seen (or have some
legal reason you think it's OK to not...).

And yes, we're "only" talking about 10 platforms, which include things
like the "everyone" has one Pi family, the extraordinarily flexible (and
so easy for the reference device tree to be very wrong) QEMU families
and then platforms that are including a dts in-tree now because they
were told that was required.

How about 

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[François Ozog]

Hi Simon

Le jeu. 2 déc. 2021 à 17:40, Simon Glass  a écrit :

> Hi François,
>
> On Thu, 2 Dec 2021 at 09:34, François Ozog 
> wrote:
> >
> > Hi Simon
> >
> > Le jeu. 2 déc. 2021 à 17:00, Simon Glass  a écrit :
> >>
> >> With Ilias' efforts we have dropped OF_PRIOR_STAGE and OF_HOSTFILE so
> >> there are only three ways to obtain a devicetree:
> >>
> >>- OF_SEPARATE - the normal way, where the devicetree is built and
> >>   appended to U-Boot
> >>- OF_EMBED - for development purposes, the devicetree is embedded in
> >>   the ELF file (also used for EFI)
> >>- OF_BOARD - the board figures it out on its own
> >>
> >> The last one is currently set up so that no devicetree is needed at all
> >> in the U-Boot tree. Most boards do provide one, but some don't. Some
> >> don't even provide instructions on how to boot on the board.
> >>
> >> The problems with this approach are documented in another patch in this
> >> series: "doc: Add documentation about devicetree usage"
> >>
> >> In practice, OF_BOARD is not really distinct from OF_SEPARATE. Any board
> >> can obtain its devicetree at runtime, even it is has a devicetree built
> >> in U-Boot. This is because U-Boot may be a second-stage bootloader and
> its
> >> caller may have a better idea about the hardware available in the
> machine.
> >> This is the case with a few QEMU boards, for example.
> >>
> >> So it makes no sense to have OF_BOARD as a 'choice'. It should be an
> >> option, available with either OF_SEPARATE or OF_EMBED.
> >>
> >> This series makes this change, adding various missing devicetree files
> >> (and placeholders) to make the build work.
> >>
> >> Note: If board maintainers are able to add their own patch to add the
> >> files, some patches in this series can be dropped.
> >>
> >> It also provides a few qemu clean-ups discovered along the way. The
> >> qemu-riscv64_spl problem is fixed.
> >>
> >> [1]
> https://patchwork.ozlabs.org/project/uboot/patch/20210919215111.3830278-3-...@chromium.org/
> >>
> >> Changes in v6:
> >> - Fix description of OF_BOARD so it refers just to the current state
> >> - Explain that the 'two devicetrees' refers to two *control* devicetrees
> >> - Expand the commit message based on comments
> >> - Expand the commit message based on comments
> >
> > You haven’t addressed any concerns expressed on the mailing list.so I am
> not in favor of this new version either.
>
> Please see the change log. I have addressed everything except the
> fundamental disagreement we have.
>
> > If you make a version without « fake DTs » as you name them, there are
> good advances in the documentation and other areas that would be better in
> mainline….
> > If I am the only one thinking this way and the patch can be accepted, I
> would love there is a warning in capital letters at the top of the DTS fake
> files that explains the intent of this fake DT, the possible outcomes of
> not using the one provided by the platform and the right way of dealing
> with DTs for the platform.
>
> The word 'fake' applies to only three of the boards (highbank, bcm7xxx
> and octeontx),

Qemu is another one, and then you will have all SystemReady compliant
boards.

> where I could not even figure out where to get a
> devicetree. One might think this would be a significant problem!
>
> Anyway yes I can add a comment to all the files, but please try to ask
> for everything at once as there is a cost to continually reworking
> this series.

I proposed this as a last resort if I was the only one with concerns. But
sounds like I am not the only one strongly opinionated here.
Another trick would be to use a “.dts.doc” extension so that it is not
compiled unless someone is knowingly deciding to use the fake DT for debug
purposes (the warning message is still valid). (This is an effort to
advance on the good things of the patch)

>
>
> Regards,
> Simon
>
> >>
> >>
> >> Changes in v5:
> >> - Bring into the OF_BOARD series
> >> - Rebase to master and drop mention of OF_PRIOR_STAGE, since removed
> >> - Refer to the 'control' DTB in the first paragraph
> >> - Use QEMU instead of qemu
> >> - Merge RISC-V and ARM patches since they are similar
> >> - Add new patches to clean up fdtdec_setup() and surrounds
> >>
> >> Changes in v3:
> >> - Clarify the 'bug' refered to at the top
> >> - Reword 'This means that there' paragraph to explain U-Boot-specific
> things
> >> - Move to doc/develop/devicetree now that OF_CONTROL is in the docs
> >>
> >> Changes in v2:
> >> - Fix typos per Sean (thank you!) and a few others
> >> - Add a 'Use of U-Boot /config node' section
> >> - Drop mention of dm-verity since that actually uses the kernel cmdline
> >> - Explain that OF_BOARD will still work after these changes (in
> >>   'Once this bug is fixed...' paragraph)
> >> - Expand a bit on the reason why the 'Current situation' is bad
> >> - Clarify in a second place that Linux and U-Boot use the same
> devicetree
> >>   in 'To be clear, while U-Boot...'
> >> - Expand 

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Mark Kettenis]

> From: Ilias Apalodimas 
> Date: Thu, 2 Dec 2021 19:03:46 +0200
> 
> On Thu, 2 Dec 2021 at 18:38, Tom Rini  wrote:
> >
> > On Thu, Dec 02, 2021 at 05:33:53PM +0100, François Ozog wrote:
> > > Hi Simon
> > >
> > > Le jeu. 2 déc. 2021 à 17:00, Simon Glass  a écrit :
> > >
> > > > With Ilias' efforts we have dropped OF_PRIOR_STAGE and OF_HOSTFILE so
> > > > there are only three ways to obtain a devicetree:
> > > >
> > > >- OF_SEPARATE - the normal way, where the devicetree is built and
> > > >   appended to U-Boot
> > > >- OF_EMBED - for development purposes, the devicetree is embedded in
> > > >   the ELF file (also used for EFI)
> > > >- OF_BOARD - the board figures it out on its own
> > > >
> > > > The last one is currently set up so that no devicetree is needed at all
> > > > in the U-Boot tree. Most boards do provide one, but some don't. Some
> > > > don't even provide instructions on how to boot on the board.
> > > >
> > > > The problems with this approach are documented in another patch in this
> > > > series: "doc: Add documentation about devicetree usage"
> > > >
> > > > In practice, OF_BOARD is not really distinct from OF_SEPARATE. Any board
> > > > can obtain its devicetree at runtime, even it is has a devicetree built
> > > > in U-Boot. This is because U-Boot may be a second-stage bootloader and 
> > > > its
> > > > caller may have a better idea about the hardware available in the 
> > > > machine.
> > > > This is the case with a few QEMU boards, for example.
> > > >
> > > > So it makes no sense to have OF_BOARD as a 'choice'. It should be an
> > > > option, available with either OF_SEPARATE or OF_EMBED.
> > > >
> > > > This series makes this change, adding various missing devicetree files
> > > > (and placeholders) to make the build work.
> > > >
> > > > Note: If board maintainers are able to add their own patch to add the
> > > > files, some patches in this series can be dropped.
> > > >
> > > > It also provides a few qemu clean-ups discovered along the way. The
> > > > qemu-riscv64_spl problem is fixed.
> > > >
> > > > [1]
> > > > https://patchwork.ozlabs.org/project/uboot/patch/20210919215111.3830278-3-...@chromium.org/
> > > >
> > > > Changes in v6:
> > > > - Fix description of OF_BOARD so it refers just to the current state
> > > > - Explain that the 'two devicetrees' refers to two *control* devicetrees
> > > > - Expand the commit message based on comments
> > > > - Expand the commit message based on comments
> > >
> > > You haven’t addressed any concerns expressed on the mailing list.so I am
> > > not in favor of this new version either.
> > > If you make a version without « fake DTs » as you name them, there are 
> > > good
> > > advances in the documentation and other areas that would be better in
> > > mainline….
> > > If I am the only one thinking this way and the patch can be accepted, I
> > > would love there is a warning in capital letters at the top of the DTS 
> > > fake
> > > files that explains the intent of this fake DT, the possible outcomes of
> > > not using the one provided by the platform and the right way of dealing
> > > with DTs for the platform.
> >
> > This is the part that I too am still unhappy about.  I do not want
> > reference or fake or whatever device trees in the U-Boot source tree.
> > We should be able to _remove_ the ones we have, that are not required,
> > with doc/board/...rst explaining how to get / view one.  Not adding
> > more.
> 
> So this is a key point for me and the reason I completely disagree
> with this approach.  This proposal is working in the *exact* opposite
> direction and we'll never be able to get rid of device trees from
> U-Boot, even if at some point they move out of the kernel to a
> 'common' repo'.  I'll just repeat what I've been saying since v1.
> Personally I'd be way happier if we could figure out were the specific
> U-Boot config nodes are needed and when are they needed.  Based on
> what we figure out we could, pick up the device tree from a previous
> state bootloader and fix it up with our special nodes before we start
> using it, using internal DTS files (compiled to .dtbos or similar)
> that indeed belong in the u-boot tree.

I don't think it makes sense to put stuff in the DT that is specific
for U-Boot only to pull it out moments later.  Maybe it does make some
sense to do this to pass information between TPL/SPL and U-Boot
proper.  But otherwise you can just use global variables...

Now I just ran into an issue on Apple M1 that may have some relevance
here.  I'm adding support for power domains and the serial port
requires certain power domains to be on.  Since the serial port is
initialized in the pre-relocation phase this means that the device
tree nodes for the power domain controllers need to have the
"u-boot,dm-pre-reloc" property on them.  Otherwise the DM code won't
be able to bind the power domain controller driver in this phase and
binding the serial port driver itself will fail.  Which 

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Tom Rini]

On Thu, Dec 02, 2021 at 11:17:38AM -0700, Simon Glass wrote:
> Hi Tom,
> 
> On Thu, 2 Dec 2021 at 11:03, Tom Rini  wrote:
> >
> > On Thu, Dec 02, 2021 at 10:07:13AM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Thu, 2 Dec 2021 at 09:59, Tom Rini  wrote:
> > > >
> > > > On Thu, Dec 02, 2021 at 09:49:51AM -0700, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Thu, 2 Dec 2021 at 09:38, Tom Rini  wrote:
> > > > > >
> > > > > > On Thu, Dec 02, 2021 at 05:33:53PM +0100, François Ozog wrote:
> > > > > > > Hi Simon
> > > > > > >
> > > > > > > Le jeu. 2 déc. 2021 à 17:00, Simon Glass  a 
> > > > > > > écrit :
> > > > > > >
> > > > > > > > With Ilias' efforts we have dropped OF_PRIOR_STAGE and 
> > > > > > > > OF_HOSTFILE so
> > > > > > > > there are only three ways to obtain a devicetree:
> > > > > > > >
> > > > > > > >- OF_SEPARATE - the normal way, where the devicetree is 
> > > > > > > > built and
> > > > > > > >   appended to U-Boot
> > > > > > > >- OF_EMBED - for development purposes, the devicetree is 
> > > > > > > > embedded in
> > > > > > > >   the ELF file (also used for EFI)
> > > > > > > >- OF_BOARD - the board figures it out on its own
> > > > > > > >
> > > > > > > > The last one is currently set up so that no devicetree is 
> > > > > > > > needed at all
> > > > > > > > in the U-Boot tree. Most boards do provide one, but some don't. 
> > > > > > > > Some
> > > > > > > > don't even provide instructions on how to boot on the board.
> > > > > > > >
> > > > > > > > The problems with this approach are documented in another patch 
> > > > > > > > in this
> > > > > > > > series: "doc: Add documentation about devicetree usage"
> > > > > > > >
> > > > > > > > In practice, OF_BOARD is not really distinct from OF_SEPARATE. 
> > > > > > > > Any board
> > > > > > > > can obtain its devicetree at runtime, even it is has a 
> > > > > > > > devicetree built
> > > > > > > > in U-Boot. This is because U-Boot may be a second-stage 
> > > > > > > > bootloader and its
> > > > > > > > caller may have a better idea about the hardware available in 
> > > > > > > > the machine.
> > > > > > > > This is the case with a few QEMU boards, for example.
> > > > > > > >
> > > > > > > > So it makes no sense to have OF_BOARD as a 'choice'. It should 
> > > > > > > > be an
> > > > > > > > option, available with either OF_SEPARATE or OF_EMBED.
> > > > > > > >
> > > > > > > > This series makes this change, adding various missing 
> > > > > > > > devicetree files
> > > > > > > > (and placeholders) to make the build work.
> > > > > > > >
> > > > > > > > Note: If board maintainers are able to add their own patch to 
> > > > > > > > add the
> > > > > > > > files, some patches in this series can be dropped.
> > > > > > > >
> > > > > > > > It also provides a few qemu clean-ups discovered along the way. 
> > > > > > > > The
> > > > > > > > qemu-riscv64_spl problem is fixed.
> > > > > > > >
> > > > > > > > [1]
> > > > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20210919215111.3830278-3-...@chromium.org/
> > > > > > > >
> > > > > > > > Changes in v6:
> > > > > > > > - Fix description of OF_BOARD so it refers just to the current 
> > > > > > > > state
> > > > > > > > - Explain that the 'two devicetrees' refers to two *control* 
> > > > > > > > devicetrees
> > > > > > > > - Expand the commit message based on comments
> > > > > > > > - Expand the commit message based on comments
> > > > > > >
> > > > > > > You haven’t addressed any concerns expressed on the mailing 
> > > > > > > list.so I am
> > > > > > > not in favor of this new version either.
> > > > > > > If you make a version without « fake DTs » as you name them, 
> > > > > > > there are good
> > > > > > > advances in the documentation and other areas that would be 
> > > > > > > better in
> > > > > > > mainline….
> > > > > > > If I am the only one thinking this way and the patch can be 
> > > > > > > accepted, I
> > > > > > > would love there is a warning in capital letters at the top of 
> > > > > > > the DTS fake
> > > > > > > files that explains the intent of this fake DT, the possible 
> > > > > > > outcomes of
> > > > > > > not using the one provided by the platform and the right way of 
> > > > > > > dealing
> > > > > > > with DTs for the platform.
> > > > > >
> > > > > > This is the part that I too am still unhappy about.  I do not want
> > > > > > reference or fake or whatever device trees in the U-Boot source 
> > > > > > tree.
> > > > > > We should be able to _remove_ the ones we have, that are not 
> > > > > > required,
> > > > > > with doc/board/...rst explaining how to get / view one.  Not adding
> > > > > > more.
> > > > >
> > > > > I understand you don't like it and that others don't as well. I wish
> > > > > it had not come to this.
> > > > >
> > > > > However we are only talking about 10 boards, three of which don't even
> > > > > have a devicetree anywhere I can find.
> > > > >
> > > > > I think on balance this is a 

[PATCH] mkimage: fix segfault on MacOS arm64

[Sergey V. Lobanov]

mkimage segfaults due ASLR mechasim on MacOS arm64

It is required to use _dyld_get_image_vmaddr_slide()
to prevent segfault on MacOS arm64

This patch ased on the discussion
https://github.com/u-boot/u-boot/commit/3b142045e8a7f0ab17b6099e9226296af45967d0

Thanks to Ronny Kotzschmar and ptpt52 github user

Signed-off-by: Sergey V. Lobanov 
---
 tools/imagetool.h | 8 ++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/imagetool.h b/tools/imagetool.h
index e229a34ffc..13775ff9b3 100644
--- a/tools/imagetool.h
+++ b/tools/imagetool.h
@@ -271,11 +271,16 @@ int rockchip_copy_image(int fd, struct image_tool_params 
*mparams);
  *  b) we need a API call to get the respective section symbols */
 #if defined(__MACH__)
 #include 
+#include 
 
-#define INIT_SECTION(name)  do {   \
+#define INIT_SECTION(name) struct image_type_params\
+   **__cat(__start_, name), **__cat(__stop_, name);\
+   do {\
unsigned long name ## _len; \
char *__cat(pstart_, name) = getsectdata("__DATA",  \
#name, &__cat(name, _len)); \
+   __cat(pstart_, name) += \
+   _dyld_get_image_vmaddr_slide(0);\
char *__cat(pstop_, name) = __cat(pstart_, name) +  \
__cat(name, _len);  \
__cat(__start_, name) = (void *)__cat(pstart_, name);   \
@@ -283,7 +288,6 @@ int rockchip_copy_image(int fd, struct image_tool_params 
*mparams);
} while (0)
 #define SECTION(name)   __attribute__((section("__DATA, " #name)))
 
-struct image_type_params **__start_image_type, **__stop_image_type;
 #else
 #define INIT_SECTION(name) /* no-op for ELF */
 #define SECTION(name)   __attribute__((section(#name)))
-- 
2.30.1 (Apple Git-130)

Re: [PATCH 0/4] rockchip: Improve support for Bob chromebook and add support for Kevin

[Simon Glass]

Hi Alper,

On Thu, 25 Nov 2021 at 10:39, Alper Nebi Yasak  wrote:
>
> I have recently started testing booting U-Boot from SPI on my gru-kevin
> (as opposed to chainloading it from vendor coreboot + depthcharge) and
> brought it to a better working state based on an initial support patch
> from Marty [1][2] and some follow-up work by Simon [3].
>
> I tried to keep them as the git author when I took things from their
> work, but squashing other changes into those and rewriting commit
> messages makes things a bit weird in my opinion, especially for keeping
> their signoff. Do tell me if there is a better way to that.
>
> As the Kevin and Bob boards are very similar. I assumed the config and
> devicetree changes will be appropriate for Bob as well, and applied them
> to it first. I do not have a Bob, so could not test on one myself, but
> Simon did test an earlier version of this and it appears to work [4].
>
> Other useful things for these boards:
> - Patch to fix a hang when usb controllers exit [5]
> - Series to support HS400ES mode as HS400 training fails [6]
> - Hack to skip eMMC reinitialization so it keeps working [7]
>
> [1] https://patchwork.ozlabs.org/patch/1053386/
> [2] https://patchwork.ozlabs.org/comment/2488899/
> [3] https://github.com/sjg20/u-boot/commits/kevin
> [4] 
> https://lore.kernel.org/u-boot/capnjgz23jd92y9ni8yw1ftl0a1sjhgouoakx13zkokn6t-s...@mail.gmail.com/
> [5] 
> https://patchwork.ozlabs.org/project/uboot/patch/20210406151059.1187379-1-icen...@aosc.io/
> [6] https://patchwork.ozlabs.org/project/uboot/list/?series=269768
> [7] https://patchwork.ozlabs.org/comment/2779784/
>
>
> Alper Nebi Yasak (2):
>   rockchip: gru: Set up SoC IO domain registers
>   rockchip: bob: Enable more configs
>
> Marty E. Plummer (1):
>   rockchip: rk3399: Add support for chromebook_kevin
>
> Simon Glass (1):
>   rockchip: gru: Add more devicetree settings
>
>  arch/arm/dts/Makefile |   1 +
>  arch/arm/dts/rk3399-gru-kevin-u-boot.dtsi |  11 ++
>  arch/arm/dts/rk3399-gru-u-boot.dtsi   |  55 +
>  arch/arm/mach-rockchip/rk3399/Kconfig |  11 ++
>  arch/arm/mach-rockchip/rk3399/rk3399.c|   4 +-
>  arch/arm/mach-rockchip/spl.c  |   3 +-
>  board/google/gru/Kconfig  |  16 +++
>  board/google/gru/MAINTAINERS  |   8 ++
>  board/google/gru/gru.c|  56 -
>  configs/chromebook_bob_defconfig  |  27 +++-
>  configs/chromebook_kevin_defconfig| 116 ++
>  doc/board/rockchip/rockchip.rst   |   1 +
>  include/configs/gru.h |   3 +
>  include/dt-bindings/input/linux-event-codes.h |   3 +-
>  14 files changed, 309 insertions(+), 6 deletions(-)
>  create mode 100644 arch/arm/dts/rk3399-gru-kevin-u-boot.dtsi
>  create mode 100644 configs/chromebook_kevin_defconfig
>
> --
> 2.34.0
>

My test results below. It seems that MMC is broken (I think by a
previous change). The garbage on the console with kevin is new but may
be unrelated to your patches, not sure. So it seems that your patches
work for me on both.


Bob:

do-try-int.sh bob
Revision 68acce8a08beb24618343a1ce7ee6c0c4c234b97, board bob

Checking revision 68acce8a08beb24618343a1ce7ee6c0c4c234b97
/vid/software/devel/ubtest
tbot starting ...
├─Parameters:
│ rev= '68acce8a08beb24618343a1ce7ee6c0c4c234b97'
│ clean  = False
├─Calling uboot_build_and_flash ...
│   ├─bob is on port 9904 and uses /dev/pts/37
│   ├─Calling uboot_build ...
│   │   ├─Calling uboot_checkout ...
│   │   │   ├─Builder: bob
│   │   │   └─Done. (1.104s)
│   │   ├─Configuring build ...
│   │   ├─Calling uboot_make ...
│   │   │   └─Done. (10.289s)
│   │   └─Done. (11.767s)
│   ├─Calling uboot_flash ...
│   │   └─Done. (2.209s)
│   └─Done. (14.438s)
├─
└─SUCCESS (16.780s)
tbot starting ...
├─Calling interactive_board ...
│   ├─bob is on port 9904 and uses /dev/pts/37
│   ├─POWERON (bob)
│   ├─Entering interactive shell (CTRL+D to exit) ...
Channel 0: LPDDR3, 933MHz
BW=32 Col=10 Bk=8 CS0 Row=15 CS1 Row=15 CS=2 Die BW=16 Size=2048MB
Channel 1: LPDDR3, 933MHz
BW=32 Col=10 Bk=8 CS0 Row=15 CS1 Row=15 CS=2 Die BW=16 Size=2048MB
256B stride

U-Boot SPL 2022.01-rc3-00130-g68acce8a08b (Dec 02 2021 - 20:05:54 -0700)
Trying to boot from SPI
rockchip_rk3399_pinctrl pinctrl: pinctrl_select_state_full:
uclass_get_device_by_phandle_id: err=-19
rockchip_rk3399_pinctrl pinctrl: pinctrl_select_state_full:
uclass_get_device_by_phandle_id: err=-19
ns16550_serial serial@ff1a: pinctrl_select_state_full:
uclass_get_device_by_phandle_id: err=-19


U-Boot 2022.01-rc3-00130-g68acce8a08b (Dec 02 2021 - 20:05:54 -0700)

Model: Google Bob
DRAM:  3.9 GiB
MMC:   mmc@fe32: 1, mmc@fe33: 0
Loading Environment from MMC... Select HS400ES failed -524
unable to select a mode : -5
*** Warning - No block device, using default environment

In:cros-ec-keyb

Re: [PATCH v2 1/2] binman: Do not pollute source tree when build with `make O=...`

[Simon Glass]

Hi Andy,

On Tue, 30 Nov 2021 at 12:04, Andy Shevchenko
 wrote:
>
> Importing libraries in Python caches the bytecode by default.
> Since we run scripts in source tree it ignores the current directory
> settings, which is $(srctree), and creates cache just in the middle
> of the source tree. Move cache to the current directory.
>
> Signed-off-by: Andy Shevchenko 
> ---
> v2: reused our_path
>  tools/binman/main.py | 11 ++-
>  1 file changed, 10 insertions(+), 1 deletion(-)

This look useful, but we cannot rely on 'srcdir' being in the
environment. For example, most binman development is done just by
running 'binman test' in the source tre. So perhaps default to the
current directory is 'srcdir' is not set?

Regards,
Simon


>
> diff --git a/tools/binman/main.py b/tools/binman/main.py
> index 8c1e478d54ce..4d8b124c7468 100755
> --- a/tools/binman/main.py
> +++ b/tools/binman/main.py
> @@ -16,9 +16,18 @@ import sys
>  import traceback
>  import unittest
>
> +# Get the absolute path to this file at run-time
> +our_path = os.path.dirname(sys.argv[0])
> +
> +#
> +# Do not pollute source tree with cache files:
> +# https://stackoverflow.com/a/60024195/2511795
> +# https://bugs.python.org/issue33499
> +#
> +sys.pycache_prefix = os.path.relpath(our_path, os.environ['srctree'])
> +
>  # Bring in the patman and dtoc libraries (but don't override the first path
>  # in PYTHONPATH)
> -our_path = os.path.dirname(os.path.realpath(__file__))
>  sys.path.insert(2, os.path.join(our_path, '..'))
>
>  from patman import test_util
> --
> 2.33.0
>

Re: [PATCH 1/4] rockchip: gru: Set up SoC IO domain registers

[Simon Glass]

Hi Alper,

On Thu, 25 Nov 2021 at 10:40, Alper Nebi Yasak  wrote:
>
> The RK3399 SoC needs to know the voltage value provided by some
> regulators, which is done by setting relevant register bits. Configure
> these the way other RK3399 boards do, but with values set in coreboot.

What do you mean by values set in coreboot? We don't need that to run
here, do we?

>
> Signed-off-by: Alper Nebi Yasak 
> ---
> There is a driver for this on Rockchip's repo, I managed to forward-port
> it and get it working. If that's more desirable than doing it per-board
> like this I can send that upstream (but I'd prefer to do it after this).
>
>  board/google/gru/gru.c | 54 ++
>  1 file changed, 54 insertions(+)


>
> diff --git a/board/google/gru/gru.c b/board/google/gru/gru.c
> index 23080c1798b7..cddcb286a380 100644
> --- a/board/google/gru/gru.c
> +++ b/board/google/gru/gru.c
> @@ -6,6 +6,17 @@
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define GRF_IO_VSEL_BT656_SHIFT 0
> +#define GRF_IO_VSEL_AUDIO_SHIFT 1
> +#define PMUGRF_CON0_VSEL_SHIFT 8
> +#define PMUGRF_CON0_VOL_SHIFT 9
>
>  #ifdef CONFIG_SPL_BUILD
>  /* provided to defeat compiler optimisation in board_init_f() */
> @@ -54,3 +65,46 @@ int board_early_init_r(void)
> return 0;
>  }
>  #endif
> +
> +#ifdef CONFIG_MISC_INIT_R

Can you just drop this #ifdef? It doesn't seem necessasry.

> +static void setup_iodomain(void)
> +{
> +   struct rk3399_grf_regs *grf =
> +  syscon_get_first_range(ROCKCHIP_SYSCON_GRF);
> +   struct rk3399_pmugrf_regs *pmugrf =
> +  syscon_get_first_range(ROCKCHIP_SYSCON_PMUGRF);
> +
> +   /* BT656 and audio is in 1.8v domain */
> +   rk_setreg(>io_vsel, (1 << GRF_IO_VSEL_BT656_SHIFT |
> + 1 << GRF_IO_VSEL_AUDIO_SHIFT));
> +
> +   /*
> +* Set GPIO1 1.8v/3.0v source select to PMU1830_VOL
> +* and explicitly configure that PMU1830_VOL to be 1.8V
> +*/
> +   rk_setreg(>soc_con0, (1 << PMUGRF_CON0_VSEL_SHIFT |
> + 1 << PMUGRF_CON0_VOL_SHIFT));
> +}
> +
> +int misc_init_r(void)
> +{
> +   const u32 cpuid_offset = 0x7;
> +   const u32 cpuid_length = 0x10;
> +   u8 cpuid[cpuid_length];
> +   int ret;
> +
> +   setup_iodomain();
> +
> +   ret = rockchip_cpuid_from_efuse(cpuid_offset, cpuid_length, cpuid);
> +   if (ret)
> +   return ret;
> +
> +   ret = rockchip_cpuid_set(cpuid, cpuid_length);
> +   if (ret)
> +   return ret;
> +
> +   ret = rockchip_setup_macaddr();
> +
> +   return ret;
> +}
> +#endif
> --
> 2.34.0
>

Regards,
Simon

Re: [PATCH 4/4] rockchip: rk3399: Add support for chromebook_kevin

[Simon Glass]

Hi Alper,

On Thu, 25 Nov 2021 at 10:40, Alper Nebi Yasak  wrote:
>
> From: "Marty E. Plummer" 
>
> Add support for Kevin, an RK3399-based convertible chromebook that is
> very similar to Bob. This patch is mostly based on existing support for
> Bob, with only minor changes for Kevin-specific things.
>
> Unlike other Gru boards, coreboot sets Kevin's center logic to 925 mV,
> so adjust it here in the dts as well. The rk3399-gru-kevin devicetree
> has an unknown event code reference which has to be defined, set it
> to the Linux counterpart. The new defconfig is copied from Bob with the
> diffconfig:
>
>  DEFAULT_DEVICE_TREE "rk3399-gru-bob" -> "rk3399-gru-kevin"
>  DEFAULT_FDT_FILE "rockchip/rk3399-gru-bob.dtb" -> 
> "rockchip/rk3399-gru-kevin.dtb"
>  VIDEO_ROCKCHIP_MAX_XRES 1280 -> 2400
>  VIDEO_ROCKCHIP_MAX_YRES 800 -> 1600
> +TARGET_CHROMEBOOK_KEVIN y
>
> With this Kevin can boot from SPI flash to a usable U-Boot prompt on the
> display with the keyboard working, but cannot boot into Linux for
> unknown reasons.
>
> eMMC starts in a working state but fails to re-init, microSD card works
> but at a lower-than-expected speed, USB works but causes a hang on
> de-init. There are known workarounds to solve eMMC and USB issues.
>
> Cc: Marty E. Plummer 
> Cc: Simon Glass 
> [Alper: commit message, resync config with Bob, update MAINTAINERS,
> add to Rockchip doc, add Kconfig help message, set regulator]
> Co-developed-by: Alper Nebi Yasak 
> Signed-off-by: Alper Nebi Yasak 
> ---
> Marty had signed-off an earlier version of this [1], but not the updated
> version I continued on top of [2]. So I'm not sure if I can add his
> sign-off to this as is, and I hope he can reply to this with a sign-off.
>
> [1] https://patchwork.ozlabs.org/patch/1053386/
> [2] https://patchwork.ozlabs.org/comment/2488899/
>
>  arch/arm/dts/Makefile |   1 +
>  arch/arm/dts/rk3399-gru-kevin-u-boot.dtsi |  11 ++
>  arch/arm/mach-rockchip/rk3399/Kconfig |  11 ++
>  arch/arm/mach-rockchip/rk3399/rk3399.c|   4 +-
>  arch/arm/mach-rockchip/spl.c  |   3 +-
>  board/google/gru/Kconfig  |  16 +++
>  board/google/gru/MAINTAINERS  |   8 ++
>  board/google/gru/gru.c|   2 +-
>  configs/chromebook_kevin_defconfig| 116 ++
>  doc/board/rockchip/rockchip.rst   |   1 +
>  include/dt-bindings/input/linux-event-codes.h |   3 +-
>  11 files changed, 171 insertions(+), 5 deletions(-)
>  create mode 100644 arch/arm/dts/rk3399-gru-kevin-u-boot.dtsi
>  create mode 100644 configs/chromebook_kevin_defconfig

This needs a rebase on next, just for a change in rk3399.c but I
suppose it could be done when applying.

Regards,
Simon

Re: [PATCH u-boot-marvell RESEND 04/11] fdt_support: Remove FDT_STATUS_FAIL_ERROR_CODE

[Simon Glass]

On Fri, 26 Nov 2021 at 06:57, Marek Behún  wrote:
>
> From: Marek Behún 
>
> Since no one uses this feature and I am not aware of any parsers of this
> in Linux, remove it.
>
> Signed-off-by: Marek Behún 
> Reviewed-by: Stefan Roese 
> Cc: Simon Glass 
> Cc: Andy Shevchenko 
> Cc: Pratyush Yadav 
> Cc: Tim Harvey 
> Cc: Michael Walle 
> Cc: Priyanka Jain 
> ---
>  arch/arm/cpu/armv7/ls102xa/fdt.c |  6 +++---
>  board/gateworks/gw_ventana/common.c  |  3 +--
>  board/kontron/sl28/sl28.c|  2 +-
>  common/fdt_support.c | 20 +---
>  drivers/pci/pcie_layerscape_fixup.c  |  8 
>  drivers/pci/pcie_layerscape_gen4_fixup.c |  8 
>  include/fdt_support.h| 18 --
>  7 files changed, 26 insertions(+), 39 deletions(-)

Reviewed-by: Simon Glass 

OpenBSD?

Re: [PATCH u-boot-marvell RESEND 03/11] fdt_support: Remove fdt_alloc_phandle() in favor of fdt_generate_phandle()

[Simon Glass]

On Fri, 26 Nov 2021 at 06:57, Marek Behún  wrote:
>
> From: Marek Behún 
>
> Commit f0921f5098d ("fdt: Sync up to the latest libfdt") introduced
> fdt_generate_phandle() in libfdt, making fdt_alloc_phandle() obsolete in
> fdt_support.
>
> Signed-off-by: Marek Behún 
> Reviewed-by: Stefan Roese 
> Cc: Simon Glass 
> Cc: "hui.song" 
> Cc: Meenakshi Aggarwal 
> Cc: Priyanka Jain 
> Cc: Ioana Ciornei 
> ---
>  board/freescale/lx2160a/eth_lx2160aqds.c |  8 +--
>  board/freescale/lx2160a/eth_lx2162aqds.c |  8 +--
>  common/fdt_support.c | 28 +++-
>  include/fdt_support.h|  1 -
>  4 files changed, 20 insertions(+), 25 deletions(-)

Reviewed-by: Simon Glass 

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Andre Przywara]

On Thu, 2 Dec 2021 13:34:07 -0500
Tom Rini  wrote:

Hi,

> On Thu, Dec 02, 2021 at 11:17:38AM -0700, Simon Glass wrote:
> > Hi Tom,
> > 
> > On Thu, 2 Dec 2021 at 11:03, Tom Rini  wrote:  
> > >
> > > On Thu, Dec 02, 2021 at 10:07:13AM -0700, Simon Glass wrote:  
> > > > Hi Tom,
> > > >
> > > > On Thu, 2 Dec 2021 at 09:59, Tom Rini  wrote:  
> > > > >
> > > > > On Thu, Dec 02, 2021 at 09:49:51AM -0700, Simon Glass wrote:  
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Thu, 2 Dec 2021 at 09:38, Tom Rini  wrote:  
> > > > > > >
> > > > > > > On Thu, Dec 02, 2021 at 05:33:53PM +0100, François Ozog wrote:  
> > > > > > > > Hi Simon
> > > > > > > >
> > > > > > > > Le jeu. 2 déc. 2021 à 17:00, Simon Glass  a 
> > > > > > > > écrit :
> > > > > > > >  
> > > > > > > > > With Ilias' efforts we have dropped OF_PRIOR_STAGE and 
> > > > > > > > > OF_HOSTFILE so
> > > > > > > > > there are only three ways to obtain a devicetree:
> > > > > > > > >
> > > > > > > > >- OF_SEPARATE - the normal way, where the devicetree is 
> > > > > > > > > built and
> > > > > > > > >   appended to U-Boot
> > > > > > > > >- OF_EMBED - for development purposes, the devicetree is 
> > > > > > > > > embedded in
> > > > > > > > >   the ELF file (also used for EFI)
> > > > > > > > >- OF_BOARD - the board figures it out on its own
> > > > > > > > >
> > > > > > > > > The last one is currently set up so that no devicetree is 
> > > > > > > > > needed at all
> > > > > > > > > in the U-Boot tree. Most boards do provide one, but some 
> > > > > > > > > don't. Some
> > > > > > > > > don't even provide instructions on how to boot on the board.
> > > > > > > > >
> > > > > > > > > The problems with this approach are documented in another 
> > > > > > > > > patch in this
> > > > > > > > > series: "doc: Add documentation about devicetree usage"
> > > > > > > > >
> > > > > > > > > In practice, OF_BOARD is not really distinct from 
> > > > > > > > > OF_SEPARATE. Any board
> > > > > > > > > can obtain its devicetree at runtime, even it is has a 
> > > > > > > > > devicetree built
> > > > > > > > > in U-Boot. This is because U-Boot may be a second-stage 
> > > > > > > > > bootloader and its
> > > > > > > > > caller may have a better idea about the hardware available in 
> > > > > > > > > the machine.
> > > > > > > > > This is the case with a few QEMU boards, for example.
> > > > > > > > >
> > > > > > > > > So it makes no sense to have OF_BOARD as a 'choice'. It 
> > > > > > > > > should be an
> > > > > > > > > option, available with either OF_SEPARATE or OF_EMBED.
> > > > > > > > >
> > > > > > > > > This series makes this change, adding various missing 
> > > > > > > > > devicetree files
> > > > > > > > > (and placeholders) to make the build work.
> > > > > > > > >
> > > > > > > > > Note: If board maintainers are able to add their own patch to 
> > > > > > > > > add the
> > > > > > > > > files, some patches in this series can be dropped.
> > > > > > > > >
> > > > > > > > > It also provides a few qemu clean-ups discovered along the 
> > > > > > > > > way. The
> > > > > > > > > qemu-riscv64_spl problem is fixed.
> > > > > > > > >
> > > > > > > > > [1]
> > > > > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20210919215111.3830278-3-...@chromium.org/
> > > > > > > > >
> > > > > > > > > Changes in v6:
> > > > > > > > > - Fix description of OF_BOARD so it refers just to the 
> > > > > > > > > current state
> > > > > > > > > - Explain that the 'two devicetrees' refers to two *control* 
> > > > > > > > > devicetrees
> > > > > > > > > - Expand the commit message based on comments
> > > > > > > > > - Expand the commit message based on comments  
> > > > > > > >
> > > > > > > > You haven’t addressed any concerns expressed on the mailing 
> > > > > > > > list.so I am
> > > > > > > > not in favor of this new version either.
> > > > > > > > If you make a version without « fake DTs » as you name them, 
> > > > > > > > there are good
> > > > > > > > advances in the documentation and other areas that would be 
> > > > > > > > better in
> > > > > > > > mainline….
> > > > > > > > If I am the only one thinking this way and the patch can be 
> > > > > > > > accepted, I
> > > > > > > > would love there is a warning in capital letters at the top of 
> > > > > > > > the DTS fake
> > > > > > > > files that explains the intent of this fake DT, the possible 
> > > > > > > > outcomes of
> > > > > > > > not using the one provided by the platform and the right way of 
> > > > > > > > dealing
> > > > > > > > with DTs for the platform.  
> > > > > > >
> > > > > > > This is the part that I too am still unhappy about.  I do not want
> > > > > > > reference or fake or whatever device trees in the U-Boot source 
> > > > > > > tree.
> > > > > > > We should be able to _remove_ the ones we have, that are not 
> > > > > > > required,
> > > > > > > with doc/board/...rst explaining how to get / view one.  Not 
> > > > > > > adding
> > > > > > > more.  
> > > > > >
> > 

[PATCH] RFC: gitlab: x86: Add a coreboot test

[Simon Glass]

Coreboot supports U-Boot as a payload and this recently got a bit of a
facelist. Add a test for this.

For now this uses a binary build of coreboot (v4.15). Future work could
potentially build it from source, but we need to figure out the
toolchain problems first, since coreboot uses its own toolchain.

This needs some changes to the hooks scripts as well. An example build
is at https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/359687

Signed-off-by: Simon Glass 
---

 .gitlab-ci.yml | 28 +++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 4c89daeadcf..f5d7e0f77e2 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -14,7 +14,7 @@ stages:
   stage: test.py
   before_script:
 # Clone uboot-test-hooks
-- git clone --depth=1 https://source.denx.de/u-boot/u-boot-test-hooks 
/tmp/uboot-test-hooks
+- git clone -b try --depth=1 https://github.com/sjg20/uboot-test-hooks.git 
/tmp/uboot-test-hooks
 - ln -s travis-ci /tmp/uboot-test-hooks/bin/`hostname`
 - ln -s travis-ci /tmp/uboot-test-hooks/py/`hostname`
 - grub-mkimage --prefix="" -o ~/grub_x86.efi -O i386-efi normal  echo 
lsefimmap lsefi lsefisystab efinet tftp minicmd
@@ -52,6 +52,16 @@ stages:
 genimage --inputpath . --config 
board/sifive/unleashed/genimage_spi-nor.cfg;
 cp images/spi-nor.img ${UBOOT_TRAVIS_BUILD_DIR}/;
   fi
+- if [[ "${TEST_PY_BD}" == "coreboot" ]]; then
+wget -O -
+  
"https://drive.google.com/uc?id=1x6nrtWIyIRPLS2cQBwYTnT2TbOI8UjmM=download;
 |
+  xz -dc >${UBOOT_TRAVIS_BUILD_DIR}/coreboot.rom;
+wget -O -
+  
"https://drive.google.com/uc?id=149Cz-5SZXHNKpi9xg6R_5XITWohu348y=download;
 >
+  cbfstool;
+chmod a+x cbfstool;
+./cbfstool ${UBOOT_TRAVIS_BUILD_DIR}/coreboot.rom add-flat-binary -f 
${UBOOT_TRAVIS_BUILD_DIR}/u-boot.bin -n fallback/payload -c LZMA -l 0x111 
-e 0x111;
+  fi
 - virtualenv -p /usr/bin/python3 /tmp/venv
 - . /tmp/venv/bin/activate
 - pip install -r test/py/requirements.txt
@@ -61,6 +71,10 @@ stages:
   ./test/py/test.py -ra --bd ${TEST_PY_BD} ${TEST_PY_ID}
 ${TEST_PY_TEST_SPEC:+"-k ${TEST_PY_TEST_SPEC}"}
 --build-dir "$UBOOT_TRAVIS_BUILD_DIR"
+# It seems that the files in /tmp go away, so copy out what we need
+- if [[ "${TEST_PY_BD}" == "coreboot" ]]; then
+cp -v /tmp/coreboot/*.{html,css} .;
+  fi
 
 build all 32bit ARM platforms:
   stage: world build
@@ -366,3 +380,15 @@ xtfpga test.py:
 TEST_PY_TEST_SPEC: "not sleep"
 TEST_PY_ID: "--id qemu"
   <<: *buildman_and_testpy_dfn
+
+coreboot test.py:
+  variables:
+TEST_PY_BD: "coreboot"
+TEST_PY_TEST_SPEC: "not sleep"
+TEST_PY_ID: "--id qemu"
+  artifacts:
+paths:
+  - "*.html"
+  - "*.css"
+expire_in: 1 week
+  <<: *buildman_and_testpy_dfn
-- 
2.34.0.384.gca35af8252-goog

Re: [PATCH v2 2/2] binman: Use less hard coded magic when inserting new PATH

[Simon Glass]

Hi Andy,

On Tue, 30 Nov 2021 at 12:04, Andy Shevchenko
 wrote:
>
> Instead of joining hard coded '..' to the run-time path of the executable,
> take just a dirname out of it. Besides that, use $(srctree) where it makes
> sense.
>
> Signed-off-by: Andy Shevchenko 
> ---
> v2: new patch
>  tools/binman/main.py | 12 +++-
>  1 file changed, 7 insertions(+), 5 deletions(-)

Looks good, apart from my comment on the previous patch, which applies here too.

Regards,
Simon

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Simon Glass]

Hi Tom,

On Thu, 2 Dec 2021 at 15:47, Tom Rini  wrote:
>
> On Thu, Dec 02, 2021 at 12:12:16PM -0700, Simon Glass wrote:
> > Hi Tom,
> >
> > On Thu, 2 Dec 2021 at 11:34, Tom Rini  wrote:
> > >
> > > On Thu, Dec 02, 2021 at 11:17:38AM -0700, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Thu, 2 Dec 2021 at 11:03, Tom Rini  wrote:
> > > > >
> > > > > On Thu, Dec 02, 2021 at 10:07:13AM -0700, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Thu, 2 Dec 2021 at 09:59, Tom Rini  wrote:
> > > > > > >
> > > > > > > On Thu, Dec 02, 2021 at 09:49:51AM -0700, Simon Glass wrote:
> > > > > > > > Hi Tom,
> > > > > > > >
> > > > > > > > On Thu, 2 Dec 2021 at 09:38, Tom Rini  
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > On Thu, Dec 02, 2021 at 05:33:53PM +0100, François Ozog wrote:
> > > > > > > > > > Hi Simon
> > > > > > > > > >
> > > > > > > > > > Le jeu. 2 déc. 2021 à 17:00, Simon Glass 
> > > > > > > > > >  a écrit :
> > > > > > > > > >
> > > > > > > > > > > With Ilias' efforts we have dropped OF_PRIOR_STAGE and 
> > > > > > > > > > > OF_HOSTFILE so
> > > > > > > > > > > there are only three ways to obtain a devicetree:
> > > > > > > > > > >
> > > > > > > > > > >- OF_SEPARATE - the normal way, where the devicetree 
> > > > > > > > > > > is built and
> > > > > > > > > > >   appended to U-Boot
> > > > > > > > > > >- OF_EMBED - for development purposes, the devicetree 
> > > > > > > > > > > is embedded in
> > > > > > > > > > >   the ELF file (also used for EFI)
> > > > > > > > > > >- OF_BOARD - the board figures it out on its own
> > > > > > > > > > >
> > > > > > > > > > > The last one is currently set up so that no devicetree is 
> > > > > > > > > > > needed at all
> > > > > > > > > > > in the U-Boot tree. Most boards do provide one, but some 
> > > > > > > > > > > don't. Some
> > > > > > > > > > > don't even provide instructions on how to boot on the 
> > > > > > > > > > > board.
> > > > > > > > > > >
> > > > > > > > > > > The problems with this approach are documented in another 
> > > > > > > > > > > patch in this
> > > > > > > > > > > series: "doc: Add documentation about devicetree usage"
> > > > > > > > > > >
> > > > > > > > > > > In practice, OF_BOARD is not really distinct from 
> > > > > > > > > > > OF_SEPARATE. Any board
> > > > > > > > > > > can obtain its devicetree at runtime, even it is has a 
> > > > > > > > > > > devicetree built
> > > > > > > > > > > in U-Boot. This is because U-Boot may be a second-stage 
> > > > > > > > > > > bootloader and its
> > > > > > > > > > > caller may have a better idea about the hardware 
> > > > > > > > > > > available in the machine.
> > > > > > > > > > > This is the case with a few QEMU boards, for example.
> > > > > > > > > > >
> > > > > > > > > > > So it makes no sense to have OF_BOARD as a 'choice'. It 
> > > > > > > > > > > should be an
> > > > > > > > > > > option, available with either OF_SEPARATE or OF_EMBED.
> > > > > > > > > > >
> > > > > > > > > > > This series makes this change, adding various missing 
> > > > > > > > > > > devicetree files
> > > > > > > > > > > (and placeholders) to make the build work.
> > > > > > > > > > >
> > > > > > > > > > > Note: If board maintainers are able to add their own 
> > > > > > > > > > > patch to add the
> > > > > > > > > > > files, some patches in this series can be dropped.
> > > > > > > > > > >
> > > > > > > > > > > It also provides a few qemu clean-ups discovered along 
> > > > > > > > > > > the way. The
> > > > > > > > > > > qemu-riscv64_spl problem is fixed.
> > > > > > > > > > >
> > > > > > > > > > > [1]
> > > > > > > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20210919215111.3830278-3-...@chromium.org/
> > > > > > > > > > >
> > > > > > > > > > > Changes in v6:
> > > > > > > > > > > - Fix description of OF_BOARD so it refers just to the 
> > > > > > > > > > > current state
> > > > > > > > > > > - Explain that the 'two devicetrees' refers to two 
> > > > > > > > > > > *control* devicetrees
> > > > > > > > > > > - Expand the commit message based on comments
> > > > > > > > > > > - Expand the commit message based on comments
> > > > > > > > > >
> > > > > > > > > > You haven’t addressed any concerns expressed on the mailing 
> > > > > > > > > > list.so I am
> > > > > > > > > > not in favor of this new version either.
> > > > > > > > > > If you make a version without « fake DTs » as you name 
> > > > > > > > > > them, there are good
> > > > > > > > > > advances in the documentation and other areas that would be 
> > > > > > > > > > better in
> > > > > > > > > > mainline….
> > > > > > > > > > If I am the only one thinking this way and the patch can be 
> > > > > > > > > > accepted, I
> > > > > > > > > > would love there is a warning in capital letters at the top 
> > > > > > > > > > of the DTS fake
> > > > > > > > > > files that explains the intent of this fake DT, the 
> > > > > > > > > > possible outcomes of
> > > > > > > > > > not using the one 

[PATCH 1/2] board/sunxi/dram_sun4i_auto: use DRAM_MEMORY_TYPE_DDR3 instead of magic number 3

[Giulio Benetti]

Since DRAM_MEMORY_TYPE_DDR3 is defined let's use it instead of magic
number 3.

Signed-off-by: Giulio Benetti 
---
 board/sunxi/dram_sun4i_auto.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/sunxi/dram_sun4i_auto.c b/board/sunxi/dram_sun4i_auto.c
index e8bbee4ee5..547d1c0cb4 100644
--- a/board/sunxi/dram_sun4i_auto.c
+++ b/board/sunxi/dram_sun4i_auto.c
@@ -4,7 +4,7 @@
 
 static struct dram_para dram_para = {
.clock = CONFIG_DRAM_CLK,
-   .type = 3,
+   .type = DRAM_MEMORY_TYPE_DDR3,
.rank_num = 1,
.density = 0,
.io_width = 0,
-- 
2.25.1

[PATCH 2/2] board/sunxi/dram_sun5i_auto: use DRAM_MEMORY_TYPE_DDR3 instead of magic number 3

[Giulio Benetti]

Since DRAM_MEMORY_TYPE_DDR3 is defined let's use it instead of magic
number 3.

Signed-off-by: Giulio Benetti 
---
 board/sunxi/dram_sun5i_auto.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/board/sunxi/dram_sun5i_auto.c b/board/sunxi/dram_sun5i_auto.c
index a5f4f8b743..517506ccc4 100644
--- a/board/sunxi/dram_sun5i_auto.c
+++ b/board/sunxi/dram_sun5i_auto.c
@@ -7,7 +7,7 @@
 static struct dram_para dram_para = {
.clock = CONFIG_DRAM_CLK,
.mbus_clock = CONFIG_DRAM_MBUS_CLK,
-   .type = 3,
+   .type = DRAM_MEMORY_TYPE_DDR3,
.rank_num = 1,
.density = 0,
.io_width = 0,
-- 
2.25.1

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Andre Przywara]

On Thu, 2 Dec 2021 19:03:46 +0200
Ilias Apalodimas  wrote:

Hi,

> On Thu, 2 Dec 2021 at 18:38, Tom Rini  wrote:
> >
> > On Thu, Dec 02, 2021 at 05:33:53PM +0100, François Ozog wrote:  
> > > Hi Simon
> > >
> > > Le jeu. 2 déc. 2021 à 17:00, Simon Glass  a écrit :
> > >  
> > > > With Ilias' efforts we have dropped OF_PRIOR_STAGE and OF_HOSTFILE so
> > > > there are only three ways to obtain a devicetree:
> > > >
> > > >- OF_SEPARATE - the normal way, where the devicetree is built and
> > > >   appended to U-Boot
> > > >- OF_EMBED - for development purposes, the devicetree is embedded in
> > > >   the ELF file (also used for EFI)
> > > >- OF_BOARD - the board figures it out on its own
> > > >
> > > > The last one is currently set up so that no devicetree is needed at all
> > > > in the U-Boot tree. Most boards do provide one, but some don't. Some
> > > > don't even provide instructions on how to boot on the board.
> > > >
> > > > The problems with this approach are documented in another patch in this
> > > > series: "doc: Add documentation about devicetree usage"
> > > >
> > > > In practice, OF_BOARD is not really distinct from OF_SEPARATE. Any board
> > > > can obtain its devicetree at runtime, even it is has a devicetree built
> > > > in U-Boot. This is because U-Boot may be a second-stage bootloader and 
> > > > its
> > > > caller may have a better idea about the hardware available in the 
> > > > machine.
> > > > This is the case with a few QEMU boards, for example.
> > > >
> > > > So it makes no sense to have OF_BOARD as a 'choice'. It should be an
> > > > option, available with either OF_SEPARATE or OF_EMBED.
> > > >
> > > > This series makes this change, adding various missing devicetree files
> > > > (and placeholders) to make the build work.
> > > >
> > > > Note: If board maintainers are able to add their own patch to add the
> > > > files, some patches in this series can be dropped.
> > > >
> > > > It also provides a few qemu clean-ups discovered along the way. The
> > > > qemu-riscv64_spl problem is fixed.
> > > >
> > > > [1]
> > > > https://patchwork.ozlabs.org/project/uboot/patch/20210919215111.3830278-3-...@chromium.org/
> > > >
> > > > Changes in v6:
> > > > - Fix description of OF_BOARD so it refers just to the current state
> > > > - Explain that the 'two devicetrees' refers to two *control* devicetrees
> > > > - Expand the commit message based on comments
> > > > - Expand the commit message based on comments  
> > >
> > > You haven’t addressed any concerns expressed on the mailing list.so I am
> > > not in favor of this new version either.
> > > If you make a version without « fake DTs » as you name them, there are 
> > > good
> > > advances in the documentation and other areas that would be better in
> > > mainline….
> > > If I am the only one thinking this way and the patch can be accepted, I
> > > would love there is a warning in capital letters at the top of the DTS 
> > > fake
> > > files that explains the intent of this fake DT, the possible outcomes of
> > > not using the one provided by the platform and the right way of dealing
> > > with DTs for the platform.  
> >
> > This is the part that I too am still unhappy about.  I do not want
> > reference or fake or whatever device trees in the U-Boot source tree.
> > We should be able to _remove_ the ones we have, that are not required,
> > with doc/board/...rst explaining how to get / view one.  Not adding
> > more.  
> 
> So this is a key point for me and the reason I completely disagree
> with this approach.  This proposal is working in the *exact* opposite
> direction and we'll never be able to get rid of device trees from
> U-Boot, even if at some point they move out of the kernel to a
> 'common' repo'.  I'll just repeat what I've been saying since v1.
> Personally I'd be way happier if we could figure out were the specific
> U-Boot config nodes are needed and when are they needed.  Based on
> what we figure out we could, pick up the device tree from a previous
> state bootloader and fix it up with our special nodes before we start
> using it, using internal DTS files (compiled to .dtbos or similar)
> that indeed belong in the u-boot tree.

I agree on that, I always felt like U-Boot is abusing the DT here for
its own purposes. If it needs to convey some configuration information,
it should do it separately. It can use the DTB *format* (hierarchically
organised key/value pairs), but this should not really be mingled with
the hardware information. Trusted-Firmware is using this idea: they use
the already included libfdt for parsing, but load the various config
"DTB"s separately. The "hw_config" DTB (as they call the actual DTB) is
treated separately.

Cheers,
Andre

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Andre Przywara]

On Thu, 2 Dec 2021 11:17:38 -0700
Simon Glass  wrote:

> Hi Tom,
> 
> On Thu, 2 Dec 2021 at 11:03, Tom Rini  wrote:
> >
> > On Thu, Dec 02, 2021 at 10:07:13AM -0700, Simon Glass wrote:  
> > > Hi Tom,
> > >
> > > On Thu, 2 Dec 2021 at 09:59, Tom Rini  wrote:  
> > > >
> > > > On Thu, Dec 02, 2021 at 09:49:51AM -0700, Simon Glass wrote:  
> > > > > Hi Tom,
> > > > >
> > > > > On Thu, 2 Dec 2021 at 09:38, Tom Rini  wrote:  
> > > > > >
> > > > > > On Thu, Dec 02, 2021 at 05:33:53PM +0100, François Ozog wrote:  
> > > > > > > Hi Simon
> > > > > > >
> > > > > > > Le jeu. 2 déc. 2021 à 17:00, Simon Glass  a 
> > > > > > > écrit :
> > > > > > >  
> > > > > > > > With Ilias' efforts we have dropped OF_PRIOR_STAGE and 
> > > > > > > > OF_HOSTFILE so
> > > > > > > > there are only three ways to obtain a devicetree:
> > > > > > > >
> > > > > > > >- OF_SEPARATE - the normal way, where the devicetree is 
> > > > > > > > built and
> > > > > > > >   appended to U-Boot
> > > > > > > >- OF_EMBED - for development purposes, the devicetree is 
> > > > > > > > embedded in
> > > > > > > >   the ELF file (also used for EFI)
> > > > > > > >- OF_BOARD - the board figures it out on its own
> > > > > > > >
> > > > > > > > The last one is currently set up so that no devicetree is 
> > > > > > > > needed at all
> > > > > > > > in the U-Boot tree. Most boards do provide one, but some don't. 
> > > > > > > > Some
> > > > > > > > don't even provide instructions on how to boot on the board.
> > > > > > > >
> > > > > > > > The problems with this approach are documented in another patch 
> > > > > > > > in this
> > > > > > > > series: "doc: Add documentation about devicetree usage"
> > > > > > > >
> > > > > > > > In practice, OF_BOARD is not really distinct from OF_SEPARATE. 
> > > > > > > > Any board
> > > > > > > > can obtain its devicetree at runtime, even it is has a 
> > > > > > > > devicetree built
> > > > > > > > in U-Boot. This is because U-Boot may be a second-stage 
> > > > > > > > bootloader and its
> > > > > > > > caller may have a better idea about the hardware available in 
> > > > > > > > the machine.
> > > > > > > > This is the case with a few QEMU boards, for example.
> > > > > > > >
> > > > > > > > So it makes no sense to have OF_BOARD as a 'choice'. It should 
> > > > > > > > be an
> > > > > > > > option, available with either OF_SEPARATE or OF_EMBED.
> > > > > > > >
> > > > > > > > This series makes this change, adding various missing 
> > > > > > > > devicetree files
> > > > > > > > (and placeholders) to make the build work.
> > > > > > > >
> > > > > > > > Note: If board maintainers are able to add their own patch to 
> > > > > > > > add the
> > > > > > > > files, some patches in this series can be dropped.
> > > > > > > >
> > > > > > > > It also provides a few qemu clean-ups discovered along the way. 
> > > > > > > > The
> > > > > > > > qemu-riscv64_spl problem is fixed.
> > > > > > > >
> > > > > > > > [1]
> > > > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20210919215111.3830278-3-...@chromium.org/
> > > > > > > >
> > > > > > > > Changes in v6:
> > > > > > > > - Fix description of OF_BOARD so it refers just to the current 
> > > > > > > > state
> > > > > > > > - Explain that the 'two devicetrees' refers to two *control* 
> > > > > > > > devicetrees
> > > > > > > > - Expand the commit message based on comments
> > > > > > > > - Expand the commit message based on comments  
> > > > > > >
> > > > > > > You haven’t addressed any concerns expressed on the mailing 
> > > > > > > list.so I am
> > > > > > > not in favor of this new version either.
> > > > > > > If you make a version without « fake DTs » as you name them, 
> > > > > > > there are good
> > > > > > > advances in the documentation and other areas that would be 
> > > > > > > better in
> > > > > > > mainline….
> > > > > > > If I am the only one thinking this way and the patch can be 
> > > > > > > accepted, I
> > > > > > > would love there is a warning in capital letters at the top of 
> > > > > > > the DTS fake
> > > > > > > files that explains the intent of this fake DT, the possible 
> > > > > > > outcomes of
> > > > > > > not using the one provided by the platform and the right way of 
> > > > > > > dealing
> > > > > > > with DTs for the platform.  
> > > > > >
> > > > > > This is the part that I too am still unhappy about.  I do not want
> > > > > > reference or fake or whatever device trees in the U-Boot source 
> > > > > > tree.
> > > > > > We should be able to _remove_ the ones we have, that are not 
> > > > > > required,
> > > > > > with doc/board/...rst explaining how to get / view one.  Not adding
> > > > > > more.  
> > > > >
> > > > > I understand you don't like it and that others don't as well. I wish
> > > > > it had not come to this.
> > > > >
> > > > > However we are only talking about 10 boards, three of which don't even
> > > > > have a devicetree anywhere I can find.
> > > > >
> > > > > I think on balance 

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Simon Glass]

Hi Andre,

On Thu, 2 Dec 2021 at 18:59, Andre Przywara  wrote:
>
> On Thu, 2 Dec 2021 13:34:07 -0500
> Tom Rini  wrote:
>
> Hi,
>
> > On Thu, Dec 02, 2021 at 11:17:38AM -0700, Simon Glass wrote:
> > > Hi Tom,
> > >
> > > On Thu, 2 Dec 2021 at 11:03, Tom Rini  wrote:
> > > >
> > > > On Thu, Dec 02, 2021 at 10:07:13AM -0700, Simon Glass wrote:
> > > > > Hi Tom,
> > > > >
> > > > > On Thu, 2 Dec 2021 at 09:59, Tom Rini  wrote:
> > > > > >
> > > > > > On Thu, Dec 02, 2021 at 09:49:51AM -0700, Simon Glass wrote:
> > > > > > > Hi Tom,
> > > > > > >
> > > > > > > On Thu, 2 Dec 2021 at 09:38, Tom Rini  wrote:
> > > > > > > >
> > > > > > > > On Thu, Dec 02, 2021 at 05:33:53PM +0100, François Ozog wrote:
> > > > > > > > > Hi Simon
> > > > > > > > >
> > > > > > > > > Le jeu. 2 déc. 2021 à 17:00, Simon Glass  
> > > > > > > > > a écrit :
> > > > > > > > >
> > > > > > > > > > With Ilias' efforts we have dropped OF_PRIOR_STAGE and 
> > > > > > > > > > OF_HOSTFILE so
> > > > > > > > > > there are only three ways to obtain a devicetree:
> > > > > > > > > >
> > > > > > > > > >- OF_SEPARATE - the normal way, where the devicetree is 
> > > > > > > > > > built and
> > > > > > > > > >   appended to U-Boot
> > > > > > > > > >- OF_EMBED - for development purposes, the devicetree is 
> > > > > > > > > > embedded in
> > > > > > > > > >   the ELF file (also used for EFI)
> > > > > > > > > >- OF_BOARD - the board figures it out on its own
> > > > > > > > > >
> > > > > > > > > > The last one is currently set up so that no devicetree is 
> > > > > > > > > > needed at all
> > > > > > > > > > in the U-Boot tree. Most boards do provide one, but some 
> > > > > > > > > > don't. Some
> > > > > > > > > > don't even provide instructions on how to boot on the board.
> > > > > > > > > >
> > > > > > > > > > The problems with this approach are documented in another 
> > > > > > > > > > patch in this
> > > > > > > > > > series: "doc: Add documentation about devicetree usage"
> > > > > > > > > >
> > > > > > > > > > In practice, OF_BOARD is not really distinct from 
> > > > > > > > > > OF_SEPARATE. Any board
> > > > > > > > > > can obtain its devicetree at runtime, even it is has a 
> > > > > > > > > > devicetree built
> > > > > > > > > > in U-Boot. This is because U-Boot may be a second-stage 
> > > > > > > > > > bootloader and its
> > > > > > > > > > caller may have a better idea about the hardware available 
> > > > > > > > > > in the machine.
> > > > > > > > > > This is the case with a few QEMU boards, for example.
> > > > > > > > > >
> > > > > > > > > > So it makes no sense to have OF_BOARD as a 'choice'. It 
> > > > > > > > > > should be an
> > > > > > > > > > option, available with either OF_SEPARATE or OF_EMBED.
> > > > > > > > > >
> > > > > > > > > > This series makes this change, adding various missing 
> > > > > > > > > > devicetree files
> > > > > > > > > > (and placeholders) to make the build work.
> > > > > > > > > >
> > > > > > > > > > Note: If board maintainers are able to add their own patch 
> > > > > > > > > > to add the
> > > > > > > > > > files, some patches in this series can be dropped.
> > > > > > > > > >
> > > > > > > > > > It also provides a few qemu clean-ups discovered along the 
> > > > > > > > > > way. The
> > > > > > > > > > qemu-riscv64_spl problem is fixed.
> > > > > > > > > >
> > > > > > > > > > [1]
> > > > > > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20210919215111.3830278-3-...@chromium.org/
> > > > > > > > > >
> > > > > > > > > > Changes in v6:
> > > > > > > > > > - Fix description of OF_BOARD so it refers just to the 
> > > > > > > > > > current state
> > > > > > > > > > - Explain that the 'two devicetrees' refers to two 
> > > > > > > > > > *control* devicetrees
> > > > > > > > > > - Expand the commit message based on comments
> > > > > > > > > > - Expand the commit message based on comments
> > > > > > > > >
> > > > > > > > > You haven’t addressed any concerns expressed on the mailing 
> > > > > > > > > list.so I am
> > > > > > > > > not in favor of this new version either.
> > > > > > > > > If you make a version without « fake DTs » as you name them, 
> > > > > > > > > there are good
> > > > > > > > > advances in the documentation and other areas that would be 
> > > > > > > > > better in
> > > > > > > > > mainline….
> > > > > > > > > If I am the only one thinking this way and the patch can be 
> > > > > > > > > accepted, I
> > > > > > > > > would love there is a warning in capital letters at the top 
> > > > > > > > > of the DTS fake
> > > > > > > > > files that explains the intent of this fake DT, the possible 
> > > > > > > > > outcomes of
> > > > > > > > > not using the one provided by the platform and the right way 
> > > > > > > > > of dealing
> > > > > > > > > with DTs for the platform.
> > > > > > > >
> > > > > > > > This is the part that I too am still unhappy about.  I do not 
> > > > > > > > want
> > > > > > > > reference or fake or whatever device trees in the 

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Simon Glass]

Hi Andre,

On Thu, 2 Dec 2021 at 18:31, Andre Przywara  wrote:
>
> On Thu, 2 Dec 2021 11:17:38 -0700
> Simon Glass  wrote:
>
> > Hi Tom,
> >
> > On Thu, 2 Dec 2021 at 11:03, Tom Rini  wrote:
> > >
> > > On Thu, Dec 02, 2021 at 10:07:13AM -0700, Simon Glass wrote:
> > > > Hi Tom,
> > > >
> > > > On Thu, 2 Dec 2021 at 09:59, Tom Rini  wrote:
> > > > >
> > > > > On Thu, Dec 02, 2021 at 09:49:51AM -0700, Simon Glass wrote:
> > > > > > Hi Tom,
> > > > > >
> > > > > > On Thu, 2 Dec 2021 at 09:38, Tom Rini  wrote:
> > > > > > >
> > > > > > > On Thu, Dec 02, 2021 at 05:33:53PM +0100, François Ozog wrote:
> > > > > > > > Hi Simon
> > > > > > > >
> > > > > > > > Le jeu. 2 déc. 2021 à 17:00, Simon Glass  a 
> > > > > > > > écrit :
> > > > > > > >
> > > > > > > > > With Ilias' efforts we have dropped OF_PRIOR_STAGE and 
> > > > > > > > > OF_HOSTFILE so
> > > > > > > > > there are only three ways to obtain a devicetree:
> > > > > > > > >
> > > > > > > > >- OF_SEPARATE - the normal way, where the devicetree is 
> > > > > > > > > built and
> > > > > > > > >   appended to U-Boot
> > > > > > > > >- OF_EMBED - for development purposes, the devicetree is 
> > > > > > > > > embedded in
> > > > > > > > >   the ELF file (also used for EFI)
> > > > > > > > >- OF_BOARD - the board figures it out on its own
> > > > > > > > >
> > > > > > > > > The last one is currently set up so that no devicetree is 
> > > > > > > > > needed at all
> > > > > > > > > in the U-Boot tree. Most boards do provide one, but some 
> > > > > > > > > don't. Some
> > > > > > > > > don't even provide instructions on how to boot on the board.
> > > > > > > > >
> > > > > > > > > The problems with this approach are documented in another 
> > > > > > > > > patch in this
> > > > > > > > > series: "doc: Add documentation about devicetree usage"
> > > > > > > > >
> > > > > > > > > In practice, OF_BOARD is not really distinct from 
> > > > > > > > > OF_SEPARATE. Any board
> > > > > > > > > can obtain its devicetree at runtime, even it is has a 
> > > > > > > > > devicetree built
> > > > > > > > > in U-Boot. This is because U-Boot may be a second-stage 
> > > > > > > > > bootloader and its
> > > > > > > > > caller may have a better idea about the hardware available in 
> > > > > > > > > the machine.
> > > > > > > > > This is the case with a few QEMU boards, for example.
> > > > > > > > >
> > > > > > > > > So it makes no sense to have OF_BOARD as a 'choice'. It 
> > > > > > > > > should be an
> > > > > > > > > option, available with either OF_SEPARATE or OF_EMBED.
> > > > > > > > >
> > > > > > > > > This series makes this change, adding various missing 
> > > > > > > > > devicetree files
> > > > > > > > > (and placeholders) to make the build work.
> > > > > > > > >
> > > > > > > > > Note: If board maintainers are able to add their own patch to 
> > > > > > > > > add the
> > > > > > > > > files, some patches in this series can be dropped.
> > > > > > > > >
> > > > > > > > > It also provides a few qemu clean-ups discovered along the 
> > > > > > > > > way. The
> > > > > > > > > qemu-riscv64_spl problem is fixed.
> > > > > > > > >
> > > > > > > > > [1]
> > > > > > > > > https://patchwork.ozlabs.org/project/uboot/patch/20210919215111.3830278-3-...@chromium.org/
> > > > > > > > >
> > > > > > > > > Changes in v6:
> > > > > > > > > - Fix description of OF_BOARD so it refers just to the 
> > > > > > > > > current state
> > > > > > > > > - Explain that the 'two devicetrees' refers to two *control* 
> > > > > > > > > devicetrees
> > > > > > > > > - Expand the commit message based on comments
> > > > > > > > > - Expand the commit message based on comments
> > > > > > > >
> > > > > > > > You haven’t addressed any concerns expressed on the mailing 
> > > > > > > > list.so I am
> > > > > > > > not in favor of this new version either.
> > > > > > > > If you make a version without « fake DTs » as you name them, 
> > > > > > > > there are good
> > > > > > > > advances in the documentation and other areas that would be 
> > > > > > > > better in
> > > > > > > > mainline….
> > > > > > > > If I am the only one thinking this way and the patch can be 
> > > > > > > > accepted, I
> > > > > > > > would love there is a warning in capital letters at the top of 
> > > > > > > > the DTS fake
> > > > > > > > files that explains the intent of this fake DT, the possible 
> > > > > > > > outcomes of
> > > > > > > > not using the one provided by the platform and the right way of 
> > > > > > > > dealing
> > > > > > > > with DTs for the platform.
> > > > > > >
> > > > > > > This is the part that I too am still unhappy about.  I do not want
> > > > > > > reference or fake or whatever device trees in the U-Boot source 
> > > > > > > tree.
> > > > > > > We should be able to _remove_ the ones we have, that are not 
> > > > > > > required,
> > > > > > > with doc/board/...rst explaining how to get / view one.  Not 
> > > > > > > adding
> > > > > > > more.
> > > > > >
> > > > > > I 

[u-boot-test-hooks PATCH] travis-ci: Add tests for booting from coreboot

[Simon Glass]

Add a means of testing a coreboot + U-Boot build using qemu.

Signed-off-by: Simon Glass 
---

 bin/ellesmere/conf.coreboot_qemu |  1 +
 bin/travis-ci/conf.coreboot_qemu | 28 
 2 files changed, 29 insertions(+)
 create mode 12 bin/ellesmere/conf.coreboot_qemu
 create mode 100644 bin/travis-ci/conf.coreboot_qemu

diff --git a/bin/ellesmere/conf.coreboot_qemu b/bin/ellesmere/conf.coreboot_qemu
new file mode 12
index 000..c81eb3f
--- /dev/null
+++ b/bin/ellesmere/conf.coreboot_qemu
@@ -0,0 +1 @@
+../travis-ci/conf.coreboot_qemu
\ No newline at end of file
diff --git a/bin/travis-ci/conf.coreboot_qemu b/bin/travis-ci/conf.coreboot_qemu
new file mode 100644
index 000..76d6927
--- /dev/null
+++ b/bin/travis-ci/conf.coreboot_qemu
@@ -0,0 +1,28 @@
+# Copyright (c) 2016 Konsulko Group. All rights reserved.
+# Copyright 2021 Google LLC
+#
+# Permission is hereby granted, free of charge, to any person obtaining a
+# copy of this software and associated documentation files (the "Software"),
+# to deal in the Software without restriction, including without limitation
+# the rights to use, copy, modify, merge, publish, distribute, sublicense,
+# and/or sell copies of the Software, and to permit persons to whom the
+# Software is furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included in
+# all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+# THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+# FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+# DEALINGS IN THE SOFTWARE.
+
+console_impl=qemu
+qemu_machine="pc"
+qemu_binary="qemu-system-i386"
+qemu_extra_args="-nographic -cpu qemu32 -netdev 
user,id=net0,tftp=${UBOOT_TRAVIS_BUILD_DIR} -device e1000,netdev=net0"
+qemu_kernel_args="-bios ${U_BOOT_BUILD_DIR}/coreboot.rom"
+reset_impl=none
+flash_impl=none
-- 
2.34.0.384.gca35af8252-goog

[PATCH v2] RFC: gitlab: x86: Add a coreboot test

[Simon Glass]

Coreboot supports U-Boot as a payload and this recently got a bit of a
facelist. Add a test for this.

For now this uses a binary build of coreboot (v4.15). Future work could
potentially build it from source, but we need to figure out the
toolchain problems first, since coreboot uses its own toolchain.

This needs some changes to the hooks scripts as well. An example build
is at https://source.denx.de/u-boot/custodians/u-boot-dm/-/jobs/359687

Signed-off-by: Simon Glass 
---

Changes in v2:
- Drop the local CI changes; we need the real ones applied first anyway

 .gitlab-ci.yml | 26 ++
 1 file changed, 26 insertions(+)

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index 4c89daeadcf..065d8d6158e 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -52,6 +52,16 @@ stages:
 genimage --inputpath . --config 
board/sifive/unleashed/genimage_spi-nor.cfg;
 cp images/spi-nor.img ${UBOOT_TRAVIS_BUILD_DIR}/;
   fi
+- if [[ "${TEST_PY_BD}" == "coreboot" ]]; then
+wget -O -
+  
"https://drive.google.com/uc?id=1x6nrtWIyIRPLS2cQBwYTnT2TbOI8UjmM=download;
 |
+  xz -dc >${UBOOT_TRAVIS_BUILD_DIR}/coreboot.rom;
+wget -O -
+  
"https://drive.google.com/uc?id=149Cz-5SZXHNKpi9xg6R_5XITWohu348y=download;
 >
+  cbfstool;
+chmod a+x cbfstool;
+./cbfstool ${UBOOT_TRAVIS_BUILD_DIR}/coreboot.rom add-flat-binary -f 
${UBOOT_TRAVIS_BUILD_DIR}/u-boot.bin -n fallback/payload -c LZMA -l 0x111 
-e 0x111;
+  fi
 - virtualenv -p /usr/bin/python3 /tmp/venv
 - . /tmp/venv/bin/activate
 - pip install -r test/py/requirements.txt
@@ -61,6 +71,10 @@ stages:
   ./test/py/test.py -ra --bd ${TEST_PY_BD} ${TEST_PY_ID}
 ${TEST_PY_TEST_SPEC:+"-k ${TEST_PY_TEST_SPEC}"}
 --build-dir "$UBOOT_TRAVIS_BUILD_DIR"
+# It seems that the files in /tmp go away, so copy out what we need
+- if [[ "${TEST_PY_BD}" == "coreboot" ]]; then
+cp -v /tmp/coreboot/*.{html,css} .;
+  fi
 
 build all 32bit ARM platforms:
   stage: world build
@@ -366,3 +380,15 @@ xtfpga test.py:
 TEST_PY_TEST_SPEC: "not sleep"
 TEST_PY_ID: "--id qemu"
   <<: *buildman_and_testpy_dfn
+
+coreboot test.py:
+  variables:
+TEST_PY_BD: "coreboot"
+TEST_PY_TEST_SPEC: "not sleep"
+TEST_PY_ID: "--id qemu"
+  artifacts:
+paths:
+  - "*.html"
+  - "*.css"
+expire_in: 1 week
+  <<: *buildman_and_testpy_dfn
-- 
2.34.0.384.gca35af8252-goog

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Andre Przywara]

On Thu, 2 Dec 2021 09:39:52 -0700
Simon Glass  wrote:

Hi,

wow, this thread quickly exploded, jumping in here randomly to add my
thoughts: 

> Hi François,
> 
> On Thu, 2 Dec 2021 at 09:34, François Ozog  wrote:
> >
> > Hi Simon
> >
> > Le jeu. 2 déc. 2021 à 17:00, Simon Glass  a écrit :  
> >>
> >> With Ilias' efforts we have dropped OF_PRIOR_STAGE and OF_HOSTFILE so
> >> there are only three ways to obtain a devicetree:
> >>
> >>- OF_SEPARATE - the normal way, where the devicetree is built and
> >>   appended to U-Boot
> >>- OF_EMBED - for development purposes, the devicetree is embedded in
> >>   the ELF file (also used for EFI)
> >>- OF_BOARD - the board figures it out on its own
> >>
> >> The last one is currently set up so that no devicetree is needed at all
> >> in the U-Boot tree. Most boards do provide one, but some don't. Some
> >> don't even provide instructions on how to boot on the board.
> >>
> >> The problems with this approach are documented in another patch in this
> >> series: "doc: Add documentation about devicetree usage"
> >>
> >> In practice, OF_BOARD is not really distinct from OF_SEPARATE. Any board
> >> can obtain its devicetree at runtime, even it is has a devicetree built
> >> in U-Boot. This is because U-Boot may be a second-stage bootloader and its
> >> caller may have a better idea about the hardware available in the machine.
> >> This is the case with a few QEMU boards, for example.
> >>
> >> So it makes no sense to have OF_BOARD as a 'choice'. It should be an
> >> option, available with either OF_SEPARATE or OF_EMBED.
> >>
> >> This series makes this change, adding various missing devicetree files
> >> (and placeholders) to make the build work.
> >>
> >> Note: If board maintainers are able to add their own patch to add the
> >> files, some patches in this series can be dropped.
> >>
> >> It also provides a few qemu clean-ups discovered along the way. The
> >> qemu-riscv64_spl problem is fixed.
> >>
> >> [1] 
> >> https://patchwork.ozlabs.org/project/uboot/patch/20210919215111.3830278-3-...@chromium.org/
> >>
> >> Changes in v6:
> >> - Fix description of OF_BOARD so it refers just to the current state
> >> - Explain that the 'two devicetrees' refers to two *control* devicetrees
> >> - Expand the commit message based on comments
> >> - Expand the commit message based on comments  
> >
> > You haven’t addressed any concerns expressed on the mailing list.so I am 
> > not in favor of this new version either.  
> 
> Please see the change log. I have addressed everything except the
> fundamental disagreement we have.
> 
> > If you make a version without « fake DTs » as you name them, there are good 
> > advances in the documentation and other areas that would be better in 
> > mainline….
> > If I am the only one thinking this way and the patch can be accepted, I 
> > would love there is a warning in capital letters at the top of the DTS fake 
> > files that explains the intent of this fake DT, the possible outcomes of 
> > not using the one provided by the platform and the right way of dealing 
> > with DTs for the platform.  
> 
> The word 'fake' applies to only three of the boards (highbank, bcm7xxx
> and octeontx), where I could not even figure out where to get a
> devicetree. One might think this would be a significant problem!

But why? Actually it's the exact opposite, Highbank is somewhat of a
(sadly abandoned) poster book child of DT usage:
Before the Cortex-A cores boot, the DTB gets loaded by the management
processor (ECME) from NOR flash into DRAM. There the memory node gets
populated, based on what the ECME detected (it has a DIMM slot!). Then
this DTB gets used by U-Boot, which adds the command line and initrd
information, based on user choices. And this is eventually consumed by
a kernel.

So the base DTB here is provided by the system, and software just
reacts to it - how it should be. The (arm64) Linux kernel works on this
idea: exactly *one* binary image, and the drivers that are needed by the
hardware get loaded based on DT information. The fact the .dts files
live in the kernel tree is more for practical reasons, because it
started there, we enhance DTs and should have a repo holding them, plus
all the review knowledge is in the kernel community.

Cheers,
Andre

P.S. I started writing some documentation for Highbank, to be
posted soonish.

> 
> Anyway yes I can add a comment to all the files, but please try to ask
> for everything at once as there is a cost to continually reworking
> this series.
> 
> Regards,
> Simon
> 
> >>
> >>
> >> Changes in v5:
> >> - Bring into the OF_BOARD series
> >> - Rebase to master and drop mention of OF_PRIOR_STAGE, since removed
> >> - Refer to the 'control' DTB in the first paragraph
> >> - Use QEMU instead of qemu
> >> - Merge RISC-V and ARM patches since they are similar
> >> - Add new patches to clean up fdtdec_setup() and surrounds
> >>
> >> Changes in v3:
> >> - Clarify the 'bug' refered to at the top
> 

Re: [RFC Patch v2] binman: add support for creating dummy files for external blobs

[Simon Glass]

Hi Heiko,

On Mon, 29 Nov 2021 at 02:48, Heiko Thiery  wrote:
>
> While converting to binman for an imx8mq board, it has been found that
> building in the u-boot CI fails. This is because an imx8mq requires an
> external binary (signed_hdmi_imx8m.bin). If this file cannot be found
> mkimage fails.
> To be able to build this board in the u-boot CI a binman option
> (--fake-ext-blobs) is introduced that can be switched on via the u-boot
> makefile option BINMAN_FAKE_EXT_BLOBS. With that the needed dummy files are
> created.
>
> Signed-off-by: Heiko Thiery 
> ---
> v2:
>  - pass allow_fake_blobs to ProcessImage()
>  - set AllowAllowFakeBlob() to images/entries
>  - create fake blob in Entry_blot.ObtainContents() when file is missing and
>creation is allowed
>
>  still missing:
>   - unittest
>   - option to set BINMAN_FAKE_EXT_BLOBS in Makefile via environment
> variable. With that we could simply set this env variable in the CI
> (gitlab-ci.yml) with adding support to buildman.
>
>  Makefile   |  1 +
>  tools/binman/cmdline.py|  2 ++
>  tools/binman/control.py|  9 +++--
>  tools/binman/entry.py  | 11 +++
>  tools/binman/etype/blob.py |  7 +++
>  tools/binman/etype/blob_ext.py |  8 
>  tools/binman/etype/mkimage.py  |  9 +
>  tools/binman/etype/section.py  |  9 +
>  8 files changed, 54 insertions(+), 2 deletions(-)

This looks good to me! The only thing is that instead of the warning
you should just print a single line at the end saying which blobs were
faked. See missing_list in ProcessImage() for how that could work. You
can set self.fake_blob in your blob.ObtainContents() and then have a
similar thing to CheckMissing() to actually collect the list of
entries which were faked.

Also, for the real version can you please add a test (so 'binman test
-T' stays at 100% test coverage) and some docs in binman.rst ? You can
use testMissingBlob() as a template.

Regards,
Simon

Re: [PATCH 0/4] rockchip: Improve support for Bob chromebook and add support for Kevin

[Simon Glass]

Hi Peter,

On Wed, 1 Dec 2021 at 07:23, Peter Robinson  wrote:
>
> On Thu, Nov 25, 2021 at 5:39 PM Alper Nebi Yasak
>  wrote:
> >
> > I have recently started testing booting U-Boot from SPI on my gru-kevin
> > (as opposed to chainloading it from vendor coreboot + depthcharge) and
> > brought it to a better working state based on an initial support patch
> > from Marty [1][2] and some follow-up work by Simon [3].
> >
> > I tried to keep them as the git author when I took things from their
> > work, but squashing other changes into those and rewriting commit
> > messages makes things a bit weird in my opinion, especially for keeping
> > their signoff. Do tell me if there is a better way to that.
> >
> > As the Kevin and Bob boards are very similar. I assumed the config and
> > devicetree changes will be appropriate for Bob as well, and applied them
> > to it first. I do not have a Bob, so could not test on one myself, but
> > Simon did test an earlier version of this and it appears to work [4].
> >
> > Other useful things for these boards:
> > - Patch to fix a hang when usb controllers exit [5]
> > - Series to support HS400ES mode as HS400 training fails [6]
> > - Hack to skip eMMC reinitialization so it keeps working [7]
>
> Is there docs updates to be done in doc/chromium/ for this so people
> know how to use it ?

This is for bare-metal use though, so it isn't anything to do with
Chromium at present.

Regards,
Simon

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Ilias Apalodimas]

Hi Simon, 

> > > > >

[...]

> > > > > Changes in v6:
> > > > > - Fix description of OF_BOARD so it refers just to the current state
> > > > > - Explain that the 'two devicetrees' refers to two *control* 
> > > > > devicetrees
> > > > > - Expand the commit message based on comments
> > > > > - Expand the commit message based on comments
> > > >
> > > > You haven’t addressed any concerns expressed on the mailing list.so I am
> > > > not in favor of this new version either.
> > > > If you make a version without « fake DTs » as you name them, there are 
> > > > good
> > > > advances in the documentation and other areas that would be better in
> > > > mainline….
> > > > If I am the only one thinking this way and the patch can be accepted, I
> > > > would love there is a warning in capital letters at the top of the DTS 
> > > > fake
> > > > files that explains the intent of this fake DT, the possible outcomes of
> > > > not using the one provided by the platform and the right way of dealing
> > > > with DTs for the platform.
> > >
> > > This is the part that I too am still unhappy about.  I do not want
> > > reference or fake or whatever device trees in the U-Boot source tree.
> > > We should be able to _remove_ the ones we have, that are not required,
> > > with doc/board/...rst explaining how to get / view one.  Not adding
> > > more.
> >
> > So this is a key point for me and the reason I completely disagree
> > with this approach.  This proposal is working in the *exact* opposite
> > direction and we'll never be able to get rid of device trees from
> > U-Boot, even if at some point they move out of the kernel to a
> > 'common' repo'.  I'll just repeat what I've been saying since v1.
> > Personally I'd be way happier if we could figure out were the specific
> > U-Boot config nodes are needed and when are they needed.  Based on
> > what we figure out we could, pick up the device tree from a previous
> > state bootloader and fix it up with our special nodes before we start
> > using it, using internal DTS files (compiled to .dtbos or similar)
> > that indeed belong in the u-boot tree.
> 
> Eh? If the device tree files are actually common then there should be
> a single source. If U-Boot has a copy then it should be identical.
> 
> The special node thing makes no sense for U-Boot. We just need to
> upstream our bindings and I am working on that.

Yea we discussed this on v5.  If the device bindings are upstreamed things
gets substantially easier.  But that's a big if.

> 
> Are the device tree files moving out of Linux?

There was an effort from Linaro and iirc we tried to move a few and see
how things would work out, but there's nothing official.

Regards
/Ilias

Re: [PATCH v2 1/2] binman: Do not pollute source tree when build with `make O=...`

[Andy Shevchenko]

On Friday, December 3, 2021, Simon Glass  wrote:

> Hi Andy,
>
> On Tue, 30 Nov 2021 at 12:04, Andy Shevchenko
>  wrote:
> >
> > Importing libraries in Python caches the bytecode by default.
> > Since we run scripts in source tree it ignores the current directory
> > settings, which is $(srctree), and creates cache just in the middle
> > of the source tree. Move cache to the current directory.
> >
> > Signed-off-by: Andy Shevchenko 
> > ---
> > v2: reused our_path
> >  tools/binman/main.py | 11 ++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
>
> This look useful, but we cannot rely on 'srcdir' being in the
> environment.


True and code is aware of that. Nothing needs to be fixed.

>
> For example, most binman development is done just by
> running 'binman test' in the source tre. So perhaps default to the
> current directory is 'srcdir' is not set?
>
> Regards,
> Simon
>
>
> >
> > diff --git a/tools/binman/main.py b/tools/binman/main.py
> > index 8c1e478d54ce..4d8b124c7468 100755
> > --- a/tools/binman/main.py
> > +++ b/tools/binman/main.py
> > @@ -16,9 +16,18 @@ import sys
> >  import traceback
> >  import unittest
> >
> > +# Get the absolute path to this file at run-time
> > +our_path = os.path.dirname(sys.argv[0])
> > +
> > +#
> > +# Do not pollute source tree with cache files:
> > +# https://stackoverflow.com/a/60024195/2511795
> > +# https://bugs.python.org/issue33499
> > +#
> > +sys.pycache_prefix = os.path.relpath(our_path, os.environ['srctree'])
> > +
> >  # Bring in the patman and dtoc libraries (but don't override the first
> path
> >  # in PYTHONPATH)
> > -our_path = os.path.dirname(os.path.realpath(__file__))
> >  sys.path.insert(2, os.path.join(our_path, '..'))
> >
> >  from patman import test_util
> > --
> > 2.33.0
> >
>


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v7 00/12] efi_loader: capsule: improve capsule authentication support

[AKASHI Takahiro]

Heinrich,

On Thu, Nov 25, 2021 at 03:02:35PM +0900, AKASHI Takahiro wrote:
> Hi Heinrich
> 
> On Tue, Nov 16, 2021 at 01:32:26PM +0900, AKASHI Takahiro wrote:
> > As I proposed and discussed in [1] and [2], I have made a couple of
> > improvements on the current implementation of capsule update in this
> > patch set.
> 
> For this version(v7), I have seen your review comments only
> on patch#1 and #2.
> Please take your time to review the rest (the main part of
> commits) as well.
> I don't want to respin the patch series and post its new version
> which is almost the same as the old one(v7).

Ping.

-Takahiro Akashi

> -Takahiro Akashi
> 
> 
> > * add signing feature to mkeficapsule
> > * add "--guid" option to mkeficapsule
> > * add man page of mkeficapsule
> > * update uefi document regarding capsule update
> > * revise pytests
> > * (as RFC) add CONFIG_EFI_CAPSULE_KEY_PATH
> > 
> > # We have had some discussion about fdtsig.sh.
> > # So RFCs (patch#11,#12) are still included for further discussion
> > # if they are useful or not.
> > # For smooth merge, the rest (patch#1-10) should work without them.
> > 
> > [1] https://lists.denx.de/pipermail/u-boot/2021-April/447918.html
> > [2] https://lists.denx.de/pipermail/u-boot/2021-July/455292.html
> > 
> > Prerequisite patches
> > 
> > None
> > 
> > Test
> > 
> > * locally passed the pytest which is included in this patch series
> >   on sandbox built.
> >   (CONFIG_EFI_CAPSULE_AUTHENTICATE should explicitly be turned on
> >   in order to exercise the authentication code.)
> > 
> > Changes
> > ===
> > v7 (Nov 16, 2021)
> > * rebased on pre-v2022.01-rc2
> > * drop already-merged patch
> > * check for a size of firmware binary file (patch#1)
> > * enable mkeficapsule in tools-only_defconfig (patch#2)
> > * define eficapsule.h and include it from mkeficapsule (patch#3)
> >   Hopefully, the tool can now compile on non-linux host.
> > 
> > v6 (Nov 02, 2021)
> > * rebased on pre-v2022.01-rc1
> > * add patch#2 to rework/refactor the code for better readability (patch#2)
> > * use exit(EXIT_SUCCESS/FAILURE) (patch#3)
> > * truncate >80chars lines in pytest scripts (patch#6)
> > 
> > v5 (Oct 27, 2021)
> > * rebased on pre-v2022.01-rc1 (WIP/26Oct2021)
> > * drop already-merged patches
> > * drop __weak from efi_get_public_key_data() (patch#1)
> > * describe the format of public key node in device tree (patch#4)
> > * re-order patches by grouping closely-related patches (patch#6-8)
> > * modify pytest to make the test results correctly verified
> >   either with or without CONFIG_EFI_CAPSULE_AUTHENTICATE (patch#9)
> > * add RFCs for embedding public keys during the build process (patch#10,11)
> > 
> > v4 (Oct 7, 2021)
> > * rebased on v2021.10
> > * align with "Revert "efi_capsule: Move signature from DTB to .rodata""
> > * add more missing *revert* commits (patch#1,#2,#3)
> > * add fdtsig.sh, replacing dtb support in mkeficapsule (patch#4)
> > * update/revise the man/uefi doc (patch#6,#7)
> > * fix a bug in parsing guid string (patch#8)
> > * add a test for "--guid" option (patch#10)
> > * use dtb-based authentication test as done in v1 (patch#11)
> > 
> > v3 (Aug 31, 2021)
> > * rebased on v2021.10-rc3
> > * remove pytest-related patches
> > * add function descriptions in mkeficapsule.c
> > * correct format specifiers in printf()
> > * let main() return 0 or -1 only
> > * update doc/develop/uefi/uefi.rst for syntax change of mkeficapsule
> > 
> > v2 (July 28, 2021)
> > * rebased on v2021.10-rc*
> > * removed dependency on target's configuration
> > * removed fdtsig.sh and others
> > * add man page
> > * update the UEFI document
> > * add dedicate defconfig for testing on sandbox
> > * add gitlab CI support
> > * add "--guid" option to mkeficapsule
> >   (yet rather RFC)
> > 
> > Initial release (May 12, 2021)
> > * based on v2021.07-rc2
> > 
> > AKASHI Takahiro (12):
> >   tools: mkeficapsule: rework the code a little bit
> >   tools: build mkeficapsule with tools-only_defconfig
> >   tools: mkeficapsule: add firmwware image signing
> >   tools: mkeficapsule: add man page
> >   doc: update UEFI document for usage of mkeficapsule
> >   test/py: efi_capsule: add image authentication test
> >   tools: mkeficapsule: allow for specifying GUID explicitly
> >   test/py: efi_capsule: align with the syntax change of mkeficapsule
> >   test/py: efi_capsule: add a test for "--guid" option
> >   test/py: efi_capsule: check the results in case of
> > CAPSULE_AUTHENTICATE
> >   (RFC) tools: add fdtsig.sh
> >   (RFC) efi_loader, dts: add public keys for capsules to device tree
> > 
> >  MAINTAINERS   |   2 +
> >  configs/tools-only_defconfig  |   1 +
> >  doc/develop/uefi/uefi.rst | 143 ++--
> >  doc/mkeficapsule.1| 107 +++
> >  dts/Makefile  |  23 +-
> >  lib/efi_loader/Kconfig|   7 +
> >  

Re: [RFC 07/22] dm: blk: add UCLASS_PARTITION

[AKASHI Takahiro]

Heinrich,

On Tue, Nov 16, 2021 at 12:01:27PM +0900, AKASHI Takahiro wrote:
> On Tue, Nov 16, 2021 at 01:02:55AM +0100, Heinrich Schuchardt wrote:
> > On 11/16/21 00:51, AKASHI Takahiro wrote:
> > > > > Is the patch good enough to include in the series?
> > > > > 
> > > > > If not, you could reply to it with what needs doing.
> > > ? I have already replied to your patch:)
> > > Basically, it seems to be fine to me.
> > > 
> > > > > Regards,
> > > > > Simon
> > > > > 
> > > > The patch is not usable as is. It assumes only GPT partioning is used.
> > > @Heinrich
> > > I don't get your point. Either my patch or Simon's is not specific
> > > to GPT at all.
> > > 
> > > So I'm going to start respinning my patch for a next round of discussion.
> > 
> > A field name gpt_part_info obviously relates to GPT?
> 
> No.
> 
> IICU, the structure, disk_partition, is not particularly GPT-specific
> as such type of data are used over various partition drivers.
> In my patch series, I simply reuse "struct disk_part" as a structure
> holding a partition number and partition information (= disk_partition).

So do you agree that we won't have "partition-table" devices for now?

-Takahiro Akashi


> -Takahiro Akashi
> 
> > Up to now this string exists only in cmd/gpt.c.
> > 
> > Best regards
> > 
> > Heinrich
> > 
> > > 
> > > -Takahiro Akashi
> > > 
> > > > Instead all partition table drivers need to be converted to drivers for
> > > > the new uclass.
> > > > 
> > > > Best regards
> > > > 
> > > > Heinrich
> > 

Re: [PATCH v6 00/25] fdt: Make OF_BOARD a boolean option

[Ilias Apalodimas]

Hi Mark,

> > > > >

[...]

> > > > > Changes in v6:
> > > > > - Fix description of OF_BOARD so it refers just to the current state
> > > > > - Explain that the 'two devicetrees' refers to two *control* 
> > > > > devicetrees
> > > > > - Expand the commit message based on comments
> > > > > - Expand the commit message based on comments
> > > >
> > > > You haven’t addressed any concerns expressed on the mailing list.so I am
> > > > not in favor of this new version either.
> > > > If you make a version without « fake DTs » as you name them, there are 
> > > > good
> > > > advances in the documentation and other areas that would be better in
> > > > mainline….
> > > > If I am the only one thinking this way and the patch can be accepted, I
> > > > would love there is a warning in capital letters at the top of the DTS 
> > > > fake
> > > > files that explains the intent of this fake DT, the possible outcomes of
> > > > not using the one provided by the platform and the right way of dealing
> > > > with DTs for the platform.
> > >
> > > This is the part that I too am still unhappy about.  I do not want
> > > reference or fake or whatever device trees in the U-Boot source tree.
> > > We should be able to _remove_ the ones we have, that are not required,
> > > with doc/board/...rst explaining how to get / view one.  Not adding
> > > more.
> > 
> > So this is a key point for me and the reason I completely disagree
> > with this approach.  This proposal is working in the *exact* opposite
> > direction and we'll never be able to get rid of device trees from
> > U-Boot, even if at some point they move out of the kernel to a
> > 'common' repo'.  I'll just repeat what I've been saying since v1.
> > Personally I'd be way happier if we could figure out were the specific
> > U-Boot config nodes are needed and when are they needed.  Based on
> > what we figure out we could, pick up the device tree from a previous
> > state bootloader and fix it up with our special nodes before we start
> > using it, using internal DTS files (compiled to .dtbos or similar)
> > that indeed belong in the u-boot tree.
> 
> I don't think it makes sense to put stuff in the DT that is specific
> for U-Boot only to pull it out moments later.  Maybe it does make some
> sense to do this to pass information between TPL/SPL and U-Boot
> proper.  But otherwise you can just use global variables...

Last time we said we don't really have to remove them,  but I get the
point.

> 
> Now I just ran into an issue on Apple M1 that may have some relevance
> here.  I'm adding support for power domains and the serial port
> requires certain power domains to be on.  Since the serial port is
> initialized in the pre-relocation phase this means that the device
> tree nodes for the power domain controllers need to have the
> "u-boot,dm-pre-reloc" property on them.  Otherwise the DM code won't
> be able to bind the power domain controller driver in this phase and
> binding the serial port driver itself will fail.  Which makes U-Boot
> hang without any visible output on the serial console.

Very relevant indeed.  That's close to what I was afraid of when I said 
"if we could figure out were the specific U-Boot config nodes are needed 
and *when* are they needed".  Obviously this is a clear no go, since more
boards will have similar requirements in the future.

> 
> Within the Asahi Linux group we're currently discussing how to solve
> this.  We could just add the "u-boot,dm-pre-reloc" properties in the
> device trees that we're going to distribute as part of m1n1 (the
> "bootloader" than embeds U-Boot).  Or we can write some code that adds
> those properties to the device tree nodes that are dependencies for
> the serial port.

That might make sense for a project like m1n1 were you are dealing with a
handful of devices,  but I think it's going to be a pain on a larger scale,
unless of course the bindings are documented in upstream.  In that case we
could ask previous bootloaders to add them etc.

> 
> I don't think the suggestion of applying an overlay embedded in U-Boot
> would work here.  The code applying the overlay would need to run very
> early on in the pre-relocation phase.

Yep it wouldn't

> We'd also have to include
> overlays for all the models that Apple offers and pick the right one.
> And if a new model appears we can no longer just add a new device tree
> to m1n1.
> 
> But maybe there is a case where the overlay approach would make sense...

I think there is, for example I was thinking of TF-A doing all the hardware init
and then handover a DTB into u-boot on a register.  In that case U-boot
could fixup the DTB before initialing the rest of the subsystems and make DM
happy.  However as you pointed out that's not the case for all boards and
dealing with this in the early pre-relocation stage is close to
impossible, so let's drop that.


Thanks!
/Ilias

[PATCH 1/3] efi_loader: efi_tcg2_register returns appropriate error

[Masahisa Kojima]

This commit modify efi_tcg2_register() to return the
appropriate error.
With this fix, sandbox will not boot because efi_tcg2_register()
fails due to some missing feature in GetCapabilities.
So disable sandbox if EFI_TCG2_PROTOCOL is enabled.

UEFI secure boot variable measurement is not directly related
to TCG2 protocol installation, tcg2_measure_secure_boot_variable()
is moved to the separate function.

Signed-off-by: Masahisa Kojima 
---
 include/efi_loader.h   |  2 ++
 lib/efi_loader/Kconfig |  2 ++
 lib/efi_loader/efi_setup.c |  2 ++
 lib/efi_loader/efi_tcg2.c  | 65 +++---
 4 files changed, 53 insertions(+), 18 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index 67c40ca57a..f4860e87fc 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -525,6 +525,8 @@ efi_status_t efi_disk_register(void);
 efi_status_t efi_rng_register(void);
 /* Called by efi_init_obj_list() to install EFI_TCG2_PROTOCOL */
 efi_status_t efi_tcg2_register(void);
+/* Called by efi_init_obj_list() to do initial measurement */
+efi_status_t efi_tcg2_do_initial_measurement(void);
 /* measure the pe-coff image, extend PCR and add Event Log */
 efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
   struct efi_loaded_image_obj *handle,
diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 700dc838dd..24f9a2bb75 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -308,6 +308,8 @@ config EFI_TCG2_PROTOCOL
bool "EFI_TCG2_PROTOCOL support"
default y
depends on TPM_V2
+   # Sandbox TPM currently fails on GetCapabilities needed for TCG2
+   depends on !SANDBOX
select SHA1
select SHA256
select SHA384
diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
index 1aba71cd96..f58a4afa7f 100644
--- a/lib/efi_loader/efi_setup.c
+++ b/lib/efi_loader/efi_setup.c
@@ -241,6 +241,8 @@ efi_status_t efi_init_obj_list(void)
ret = efi_tcg2_register();
if (ret != EFI_SUCCESS)
goto out;
+
+   efi_tcg2_do_initial_measurement();
}
 
/* Secure boot */
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 5f71b188a0..6dbdd35f29 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -153,6 +153,15 @@ static u16 alg_to_len(u16 hash_alg)
return 0;
 }
 
+static bool is_tcg2_protocol_installed(void)
+{
+   struct efi_handler *handler;
+   efi_status_t ret;
+
+   ret = efi_search_protocol(efi_root, _guid_tcg2_protocol, );
+   return ret == EFI_SUCCESS;
+}
+
 static u32 tcg_event_final_size(struct tpml_digest_values *digest_list)
 {
u32 len;
@@ -1664,6 +1673,14 @@ void tcg2_uninit(void)
event_log.buffer = NULL;
efi_free_pool(event_log.final_buffer);
event_log.final_buffer = NULL;
+
+   if (!is_tcg2_protocol_installed())
+   return;
+
+   ret = efi_remove_protocol(efi_root, _guid_tcg2_protocol,
+ (void *)_tcg2_protocol);
+   if (ret != EFI_SUCCESS)
+   log_err("Failed to remove EFI TCG2 protocol\n");
 }
 
 /**
@@ -2345,12 +2362,37 @@ error:
return ret;
 }
 
+/**
+ * efi_tcg2_do_initial_measurement() - do initial measurement
+ *
+ * Return: status code
+ */
+efi_status_t efi_tcg2_do_initial_measurement(void)
+{
+   efi_status_t ret;
+   struct udevice *dev;
+
+   if (!is_tcg2_protocol_installed())
+   return EFI_SUCCESS;
+
+   ret = platform_get_tpm2_device();
+   if (ret != EFI_SUCCESS)
+   goto out;
+
+   ret = tcg2_measure_secure_boot_variable(dev);
+   if (ret != EFI_SUCCESS)
+   goto out;
+
+out:
+   return ret;
+}
+
 /**
  * efi_tcg2_register() - register EFI_TCG2_PROTOCOL
  *
  * If a TPM2 device is available, the TPM TCG2 Protocol is registered
  *
- * Return: An error status is only returned if adding the protocol fails.
+ * Return: status code
  */
 efi_status_t efi_tcg2_register(void)
 {
@@ -2373,8 +2415,10 @@ efi_status_t efi_tcg2_register(void)
}
 
ret = efi_init_event_log();
-   if (ret != EFI_SUCCESS)
+   if (ret != EFI_SUCCESS) {
+   tcg2_uninit();
goto fail;
+   }
 
ret = efi_add_protocol(efi_root, _guid_tcg2_protocol,
   (void *)_tcg2_protocol);
@@ -2391,24 +2435,9 @@ efi_status_t efi_tcg2_register(void)
goto fail;
}
 
-   ret = tcg2_measure_secure_boot_variable(dev);
-   if (ret != EFI_SUCCESS) {
-   tcg2_uninit();
-   goto fail;
-   }
-
return ret;
 
 fail:
log_err("Cannot install EFI_TCG2_PROTOCOL\n");
-   /*
-* Return EFI_SUCCESS and don't stop the EFI subsystem.
-* That's done for 2 reasons
-* - If the protocol is not 

Re: [PATCH 01/15] i2c: muxes: pca954x: add PCA9847 variant

[Heiko Schocher]

Hello Vladimir,

On 02.12.21 15:53, Vladimir Oltean wrote:
> This seems to be very similar to the already existing PCA9547, save for
> the fact that it supports 0.8V and doesn't support 5V. In fact, it is so
> similar to the PCA9547 that the NXP LS1028A-RDB board has been driving
> this chip using a "nxp,pca9547" compatible string.
> 
> Create a new compatible for the PCA9847 (which is the same as in Linux)
> and define the same operating parameters for it as for PCA9547.
> 
> Cc: Heiko Schocher 
> Signed-off-by: Vladimir Oltean 
> ---
>  drivers/i2c/muxes/pca954x.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)

Reviewed-by: Heiko Schocher 

Thanks!

bye,
Heiko
-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: h...@denx.de

Re: [RESEND RFC PATCH 04/10] FWU: Add metadata access functions for GPT partitioned block devices

[Sughosh Ganu]

hi Simon,

On Thu, 2 Dec 2021 at 19:04, Simon Glass  wrote:

> Hi Sughosh,
>
> On Thu, 2 Dec 2021 at 01:05, Sughosh Ganu  wrote:
> >
> > hi Simon,
> >
> > On Wed, 1 Dec 2021 at 23:32, Simon Glass  wrote:
> >>
> >> Hi Sughosh,
> >>
> >> On Thu, 25 Nov 2021 at 00:13, Sughosh Ganu 
> wrote:
> >> >
> >> > In the FWU Multi Bank Update feature, the information about the
> >> > updatable images is stored as part of the metadata, on a separate
> >> > partition. Add functions for reading from and writing to the metadata
> >> > when the updatable images and the metadata are stored on a block
> >> > device which is formated with GPT based partition scheme.
> >> >
> >> > Signed-off-by: Sughosh Ganu 
> >> > ---
> >> >  lib/fwu_updates/fwu_metadata_gpt_blk.c | 716
> +
> >> >  1 file changed, 716 insertions(+)
> >> >  create mode 100644 lib/fwu_updates/fwu_metadata_gpt_blk.c
> >>
> >> Can we pick a better name than metadata? It is just so
> >> vague...everything is metadata.
> >
> >
> > The specification calls it metadata. And the functions in this file are
> for accessing the metadata on a GPT partitioned block device. Do you have
> any alternate name for this?
>
> Well lots of things call their metadata "metadata". That doesn't mean
> it is a good name for the code base. Which specification? Do you have
> a link?
>

I am referring to the FWU specification[1]. Please refer to section 4 in
the document.

-sughosh

[1] - https://developer.arm.com/documentation/den0118/a

[PATCH 3/3] efi_loader: correctly handle tcg2_measure_pe_image() error

[Masahisa Kojima]

When the TCG2 protocol is installed in efi_tcg2_register(),
TPM2 device must be present.
tcg2_measure_pe_image() expects that TCP2 protocol is installed
and TPM device is available. If TCG2 Protocol is installed but
TPM device is not found, tcg2_measure_pe_image() returns
EFI_SECURITY_VIOLATION efi_load_image() ends with failure.

Signed-off-by: Masahisa Kojima 
---
 lib/efi_loader/efi_image_loader.c | 11 +--
 lib/efi_loader/efi_tcg2.c |  2 +-
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/lib/efi_loader/efi_image_loader.c 
b/lib/efi_loader/efi_image_loader.c
index eb95580538..426f096574 100644
--- a/lib/efi_loader/efi_image_loader.c
+++ b/lib/efi_loader/efi_image_loader.c
@@ -934,9 +934,16 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj 
*handle,
 
 #if CONFIG_IS_ENABLED(EFI_TCG2_PROTOCOL)
/* Measure an PE/COFF image */
-   if (tcg2_measure_pe_image(efi, efi_size, handle,
- loaded_image_info))
+   ret = tcg2_measure_pe_image(efi, efi_size, handle, loaded_image_info);
+   if (ret == EFI_SECURITY_VIOLATION) {
+   /*
+* TCG2 Protocol is installed but no TPM device found,
+* this is not expected.
+*/
log_err("PE image measurement failed\n");
+   goto err;
+   }
+
 #endif
 
/* Copy PE headers */
diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 2b7b7cbbae..c19f73dc10 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -977,7 +977,7 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
 
ret = platform_get_tpm2_device();
if (ret != EFI_SUCCESS)
-   return ret;
+   return EFI_SECURITY_VIOLATION;
 
switch (handle->image_type) {
case IMAGE_SUBSYSTEM_EFI_APPLICATION:
-- 
2.17.1

[PATCH 0/3] fix TCG2 error handling

[Masahisa Kojima]

This series fix the efi_tcg2.c error handling.

Masahisa Kojima (3):
  efi_loader: efi_tcg2_register returns appropriate error
  efi_loader: check tcg2 protocol installation outside the TCG protocol
  efi_loader: correctly handle tcg2_measure_pe_image() error

 include/efi_loader.h  |  2 +
 lib/efi_loader/Kconfig|  2 +
 lib/efi_loader/efi_image_loader.c | 11 +++-
 lib/efi_loader/efi_setup.c|  2 +
 lib/efi_loader/efi_tcg2.c | 85 ---
 5 files changed, 81 insertions(+), 21 deletions(-)

-- 
2.17.1

[PATCH 2/3] efi_loader: check tcg2 protocol installation outside the TCG protocol

[Masahisa Kojima]

There are functions that calls tcg2_agile_log_append() outside
of the TCG protocol invocation (e.g tcg2_measure_pe_image).
These functions must to check that TCG2 protocol is installed.
If not, measurement shall be skipped.

Signed-off-by: Masahisa Kojima 
---
 lib/efi_loader/efi_tcg2.c | 18 ++
 1 file changed, 18 insertions(+)

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c
index 6dbdd35f29..2b7b7cbbae 100644
--- a/lib/efi_loader/efi_tcg2.c
+++ b/lib/efi_loader/efi_tcg2.c
@@ -972,6 +972,9 @@ efi_status_t tcg2_measure_pe_image(void *efi, u64 efi_size,
IMAGE_NT_HEADERS32 *nt;
struct efi_handler *handler;
 
+   if (!is_tcg2_protocol_installed())
+   return EFI_SUCCESS;
+
ret = platform_get_tpm2_device();
if (ret != EFI_SUCCESS)
return ret;
@@ -2189,6 +2192,9 @@ efi_status_t efi_tcg2_measure_efi_app_invocation(struct 
efi_loaded_image_obj *ha
u32 event = 0;
struct smbios_entry *entry;
 
+   if (!is_tcg2_protocol_installed())
+   return EFI_SUCCESS;
+
if (tcg2_efi_app_invoked)
return EFI_SUCCESS;
 
@@ -2239,6 +2245,9 @@ efi_status_t efi_tcg2_measure_efi_app_exit(void)
efi_status_t ret;
struct udevice *dev;
 
+   if (!is_tcg2_protocol_installed())
+   return EFI_SUCCESS;
+
ret = platform_get_tpm2_device();
if (ret != EFI_SUCCESS)
return ret;
@@ -2264,6 +2273,12 @@ efi_tcg2_notify_exit_boot_services(struct efi_event 
*event, void *context)
EFI_ENTRY("%p, %p", event, context);
 
event_log.ebs_called = true;
+
+   if (!is_tcg2_protocol_installed()) {
+   ret = EFI_SUCCESS;
+   goto out;
+   }
+
ret = platform_get_tpm2_device();
if (ret != EFI_SUCCESS)
goto out;
@@ -2293,6 +2308,9 @@ efi_status_t 
efi_tcg2_notify_exit_boot_services_failed(void)
struct udevice *dev;
efi_status_t ret;
 
+   if (!is_tcg2_protocol_installed())
+   return EFI_SUCCESS;
+
ret = platform_get_tpm2_device();
if (ret != EFI_SUCCESS)
goto out;
-- 
2.17.1

Re: [BUG] efi_loader: incorrect creation of device paths

[AKASHI Takahiro]

Heinrich,

On Thu, Nov 25, 2021 at 02:44:28PM +0900, AKASHI Takahiro wrote:
> Heinrich,
> 
> On Wed, Nov 24, 2021 at 12:10:32PM +0900, AKASHI Takahiro wrote:
> > On Sat, Nov 20, 2021 at 01:54:30PM +0100, Heinrich Schuchardt wrote:
> > > Hello Takahiro,
> > > 
> > > in a prior mail we have discussed the creation of device paths for USB
> > > mass storage devices.
> > > 
> > > On the sand boxyou get the following devices after 'usb start':
> > > 
> > >  Class Index  Probed  DriverName
> > > ---
> > >  usb   0  [ + ]   usb_sandbox   |-- usb@1
> > >  usb_hub   0  [ + ]   usb_hub   |   `-- hub
> > >  usb_mass_s0  [ + ]   usb_mass_storage  |   |--
> > > usb_mass_storage
> > >  blk   3  [   ]   usb_storage_blk   |   |   `--
> > > usb_mass_storage.lun0
> > >  usb_mass_s1  [ + ]   usb_mass_storage  |   |--
> > > usb_mass_storage
> > >  blk   4  [   ]   usb_storage_blk   |   |   `--
> > > usb_mass_storage.lun0
> > >  usb_mass_s2  [ + ]   usb_mass_storage  |   `--
> > > usb_mass_storage
> > >  blk   5  [   ]   usb_storage_blk   |   `--
> > > usb_mass_storage.lun0
> > > 
> > > For of these storage devices we try to create the same device path which
> > > is not allowable:
> > > 
> > > => usb start
> > > starting USB...
> > > Bus usb@1: scanning bus usb@1 for devices... 5 USB Device(s) found
> > >scanning usb for storage devices... 3 Storage Device(s) found
> > > =>
> > > => efidebug dh
> > > Scanning disk mmc2.blk...
> > > handle 15e34f00,
> > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(2)/SD(0)
> > > Scanning disk mmc1.blk...
> > > handle 15e36b30,
> > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(1)/SD(1)
> > > Scanning disk mmc0.blk...
> > > handle 15e35b00,
> > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(2)
> > > Scanning disk usb_mass_storage.lun0...
> > > handle 15e35c10,
> > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/UsbClass(0x1234,0x5678,0x9,0x0,0x0)/UsbClass(0x1234,0x5678,0x0,0x0,0x0)
> > > fs_devread read outside partition 2
> > > Failed to mount ext2 filesystem...
> > > BTRFS: superblock end 69632 is larger than device size 512
> > > Scanning disk usb_mass_storage.lun0...
> > > handle 15e361f0,
> > > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/UsbClass(0x1234,0x5678,0x9,0x0,0x0)/UsbClass(0x1234,0x5678,0x0,0x0,0x0)
> > > ERROR: failure to add disk device usb_mass_storage.lun0, r = 20
> > > Error: Cannot initialize UEFI sub-system, r = 20
> > > 
> > > I will provide a patch that will allow the first USB device to be used
> > > and avoids stopping the boot process. But we really have to walk the dm
> > > tree to create a device patch for USB devices based on port IDs.
> > > 
> > > We should add a new field to struct uclass_driver:
> > > 
> > > struct efi_device_path *get_node(udevice *dev);
> > > 
> > > This function shall return a pointer to an freshly allocated buffer with
> > > the device node for the device. To build the devicepath we can then walk
> > > the dm tree.
> > 
> > I'm not sure this is an acceptable solution.
> > 
> > Let me make sure:
> > The goal would be to create a device path like
> >.../USB(0x1,0x0)/HD(1,...)
> > instead of
> >.../UsbHub(0x0,0x0,0x0,0x3)/UsbMassStorage(0x46f4,0x1,0x0,0x0)/HD(1,...)
> > as we already see this format for SCSI:
> >.../Scsi(0,0)/HD(1,..)
> > 
> > Right?
> 
> Please try the tweak attached below.
> (I think what I did here is trivial.)
> 
> If you like, I will post it as a patch.
> (See "10.3.4.5.1 USB Device Path Example".)

Any comment on my proposal?

> -Takahiro Akashi
> 
> >>> From cda91e9d8144f89f0d73738b338289a7940bbe0e Mon Sep 17 00:00:00 2001
> >>> From: AKASHI Takahiro 
> >>> Date: Thu, 25 Nov 2021 14:20:08 +0900
> >>> Subject: [PATCH] efi_loader: disk: usb mass-storage
> 
> - use path_usb instead of path_usb_class for existing USB dp nodes
> - add a dp node for UCLASS_USB
> 
> => usb start
> starting USB...
> Bus xhci_pci: Register 8001040 NbrPorts 8
> Starting the controller
> USB XHCI 1.00
> scanning bus xhci_pci for devices... 4 USB Device(s) found
>scanning usb for storage devices... 2 Storage Device(s) found
> => dm tree
>  Class Index  Probed  DriverName
> ---
>  root  0  [ + ]   root_driver   root_driver
>  ...
>  pci   0  [ + ]   pci_generic_ecam  |-- pcie@1000
>  pci_generi0  [   ]   pci_generic_drv   |   |-- pci_0:0.0
>  virtio   32  [ + ]   virtio-pci.l  |   |-- virtio-pci.l#0
>  ethernet  0  [ + ]   virtio-net|   |   `-- virtio-net#32
>  usb   0  [ + ]   xhci_pci  |   `-- xhci_pci
>  usb_hub   0  [ + ]   usb_hub   |   `-- usb_hub
>  usb_dev_ge0  [ + ]   

Re: [PATCH v6 09/25] arm: xenguest_arm64: Add a fake devicetree file

[Oleksandr Andrushchenko]

Hi, Simon!

On 02.12.21 19:57, Simon Glass wrote:
> Hi Oleksandr,
>
> On Thu, 2 Dec 2021 at 10:40, Oleksandr Andrushchenko
>  wrote:
>> Hi, Simon!
>>
>> Sorry for being late to the party
>>
>> On 02.12.21 17:59, Simon Glass wrote:
>>> Add an empty file to prevent build errors when building with
>>> CONFIG_OF_SEPARATE enabled.
>>>
>>> The build instructions in U-Boot do not provide enough detail to build a
>>> useful devicetree, unfortunately.
>> Xen guest doesn't use any built-in device trees as the guest's device tree 
>> is provided
>> by the Xen hypervisor itself and is generated at the virtual machine 
>> creation time: it is
>> populated with memory size, number of CPUs etc. based on [1].
>> So, even if we provide some device tree here it must not be used by U-boot at
>> the end of the day. Thus, it might be a reasonable solution to provide an 
>> empty device
>> tree as you do, but put a comment that it is not used.
> OK we can go with an empty one if we have to, but where are the
> instructions to create the DT that is used?
You don't need to create the device tree yourself, but instead it is
provided by Xen and generated at run-time while creating a
virtual machine. So, it is up to Xen to provide one.
There are cases [1] when you may want providing a so called
partial device tree to better tune what a virtual machine gets.
But again, it is used by Xen toolstack outside of the virtual machine
and serves as a sort of overlay to the generated device tree.
So, we can provide some device tree to be embedded in U-boot,
but it will have no practical meaning and will make more harm than good
>
> I'm not even sure how to run U-Boot with Xen? The in-tree instructions
> don't help...
This is just a virtual machine from Xen POV, so U-boot is nothing
different here from Linux kernel or anything else.
Thus no specific instructions are needed nor provided

Thank you,
Oleksandr
>
> Regards,
> Simon
[1] https://xenbits.xen.org/docs/unstable/misc/arm/passthrough.txt

[PULL] u-boot-riscv/master

[Leo Liang]

Hi Tom,

The following changes since commit 4a14bfffd42f968ed9d72a780a8d44a9053c5b95:

  Merge https://source.denx.de/u-boot/custodians/u-boot-marvell (2021-11-30 
08:59:22 -0500)

are available in the Git repository at:

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

for you to fetch changes up to c0ffc12a701621dc72dfc896965cbfe5b0dbf9b4:

  riscv: Enable SPI flash env for SiFive Unmatched. (2021-12-02 16:43:56 +0800)

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


Leo Yu-Chi Liang (1):
  board: ae350: Support autoboot from RAM

Padmarao Begari (5):
  net: macb: Remove Microchip compatible string
  i2c: Add Microchip PolarFire SoC I2C driver
  riscv: dts: Split Microchip device tree
  riscv: Update Microchip MPFS Icicle Kit support
  doc: board: Update Microchip MPFS Icicle Kit doc

Thomas Skibo (2):
  riscv: Support booting SiFive Unmatched from SPI.
  riscv: Enable SPI flash env for SiFive Unmatched.

 arch/riscv/cpu/fu740/Kconfig   |  13 +
 arch/riscv/dts/hifive-unmatched-a00-u-boot.dtsi|  11 +
 arch/riscv/dts/microchip-mpfs-icicle-kit.dts   | 518 +--
 arch/riscv/dts/microchip-mpfs.dtsi | 571 +
 board/microchip/mpfs_icicle/Kconfig|   5 +
 board/microchip/mpfs_icicle/mpfs_icicle.c  |  17 +-
 board/sifive/unmatched/Kconfig |   1 +
 board/sifive/unmatched/spl.c   |   3 +
 configs/microchip_mpfs_icicle_defconfig|   1 -
 configs/sifive_unmatched_defconfig |   6 +
 doc/board/microchip/mpfs_icicle.rst|   7 +-
 doc/board/sifive/unmatched.rst |  31 ++
 drivers/i2c/Kconfig|   6 +
 drivers/i2c/Makefile   |   1 +
 drivers/i2c/i2c-microchip.c| 482 +
 drivers/net/macb.c |  18 +-
 include/configs/ax25-ae350.h   |  13 +-
 .../interrupt-controller/microchip-mpfs-plic.h | 196 +++
 .../dt-bindings/interrupt-controller/riscv-hart.h  |  17 +
 19 files changed, 1512 insertions(+), 405 deletions(-)
 create mode 100644 arch/riscv/dts/microchip-mpfs.dtsi
 create mode 100644 drivers/i2c/i2c-microchip.c
 create mode 100644 
include/dt-bindings/interrupt-controller/microchip-mpfs-plic.h
 create mode 100644 include/dt-bindings/interrupt-controller/riscv-hart.h

 Best regards,
 Leo

Re: [RESEND RFC PATCH 04/10] FWU: Add metadata access functions for GPT partitioned block devices

[Sughosh Ganu]

hi Simon,

On Wed, 1 Dec 2021 at 23:32, Simon Glass  wrote:

> Hi Sughosh,
>
> On Thu, 25 Nov 2021 at 00:13, Sughosh Ganu 
> wrote:
> >
> > In the FWU Multi Bank Update feature, the information about the
> > updatable images is stored as part of the metadata, on a separate
> > partition. Add functions for reading from and writing to the metadata
> > when the updatable images and the metadata are stored on a block
> > device which is formated with GPT based partition scheme.
> >
> > Signed-off-by: Sughosh Ganu 
> > ---
> >  lib/fwu_updates/fwu_metadata_gpt_blk.c | 716 +
> >  1 file changed, 716 insertions(+)
> >  create mode 100644 lib/fwu_updates/fwu_metadata_gpt_blk.c
>
> Can we pick a better name than metadata? It is just so
> vague...everything is metadata.
>

The specification calls it metadata. And the functions in this file are for
accessing the metadata on a GPT partitioned block device. Do you have any
alternate name for this?

-sughosh

Re: [PATCH] mtd: sf: Set SF parameters as env variables

[Pratyush Yadav]

Hi Marek,

On 28/11/21 10:56PM, Marek Vasut wrote:
> On 10/7/21 2:46 PM, Marek Vasut wrote:
> > On 10/7/21 2:40 PM, Pratyush Yadav wrote:
> > > On 23/09/21 10:00PM, Marek Vasut wrote:
> > > > On 9/23/21 8:53 PM, Pratyush Yadav wrote:
> > > > > On 14/09/21 05:28AM, Marek Vasut wrote:
> > > > > > Set the SF page size, erase block size and total size as
> > > > > > an environment
> > > > > > variable after "sf probe". This lets us discern boards with multiple
> > > > > > distinct SPI flash options and also e.g. set mtdparts accordingly.
> > > > > 
> > > > > I don't quite follow the rationale for making these environment
> > > > > variables. Wouldn't you be better off finding this info out via mtd or
> > > > > sf command?
> > > > 
> > > > I need to use that info in a script, that's why these env vars.
> > > 
> > > Ok. Honestly, it doesn't feel quite right to me.
> > > 
> > > I haven't played with U-Boot's shell too much but is there no way to
> > > assign variables from command outputs?
> > 
> > Not that I know of.
> > 
> > > For example, can we do something
> > > like `foo=$(cat a.txt)`? If that is possible, maybe add some new
> > > subcommands to "sf" that return this information?
> > 
> > What you would want to have is per-device runtime properties, kind-of
> > like reduced sysfs, which would need VFS in U-Boot, and we do not have
> > that yet.
> 
> Are there any news on this patch ?

All I would say is that I do not particularly like what this patch is 
doing, but I also can't give you an alternative solution to the problem 
unfortunately. So I drop my $0.02 here and leave it up to you and Jagan 
to figure it out.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

Please pull u-boot-net/master

[Ramon Fried]

Hi Tom,
The below contains two fixes from Marek for designware and mdio.

The following changes since commit 4a14bfffd42f968ed9d72a780a8d44a9053c5b95:

  Merge https://source.denx.de/u-boot/custodians/u-boot-marvell
(2021-11-30 08:59:22 -0500)

are available in the Git repository at:

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

for you to fetch changes up to 3fbd17aadf792735846a5f2052b3f68f7075201f:

  net: dwc_eth_qos: Enable clock in probe (2021-12-02 08:35:44 +0200)


Marek Vasut (2):
  net: eth-phy: Handle gpio_request_by_name() return value
  net: dwc_eth_qos: Enable clock in probe

 drivers/net/dwc_eth_qos.c| 22 +++---
 drivers/net/eth-phy-uclass.c |  2 +-
 2 files changed, 12 insertions(+), 12 deletions(-)

Thanks,
Ramon.

Re: [PATCH v3 2/2] riscv: Enable SPI flash env for SiFive Unmatched.

[Leo Liang]

On Wed, Nov 24, 2021 at 02:32:10PM -0800, Thomas Skibo wrote:
> Enable saving environment to SPI flash memory on SiFive
> Unmatched.
> 
> Signed-off-by: Thomas Skibo 
> ---
>  arch/riscv/cpu/fu740/Kconfig   | 13 +
>  board/sifive/unmatched/Kconfig |  1 +
>  2 files changed, 14 insertions(+)

Reviewed-by: Leo Yu-Chi Liang 

Re: [PATCH v2 1/5] net: macb: Remove Microchip compatible string

[Leo Liang]

On Wed, Nov 17, 2021 at 06:21:15PM +0530, Padmarao Begari wrote:
> Remove the microchip compatible string and default compatible "cdns,macb"
> support both 32-bit and 64-bit DMA access.
> 
> Signed-off-by: Padmarao Begari 
> ---
>  drivers/net/macb.c | 18 +++---
>  1 file changed, 7 insertions(+), 11 deletions(-)

Reviewed-by: Leo Yu-Chi Liang 

Re: [PATCH] mtd: cqspi: Wait for transfer completion

[Pratyush Yadav]

Hi Marek,

On 25/10/21 10:25PM, Marek Vasut wrote:
> On 10/25/21 9:53 PM, Pratyush Yadav wrote:
> > On 08/10/21 06:06PM, Jagan Teki wrote:
> > > On Wed, Sep 15, 2021 at 2:05 PM Marek Vasut  wrote:
> > > > 
> > > > On 9/15/21 10:28 AM, Pratyush Yadav wrote:
> > > > > On 14/09/21 08:22PM, Marek Vasut wrote:
> > > > > > On 9/14/21 7:42 PM, Pratyush Yadav wrote:
> > > > > > > On 14/09/21 05:22AM, Marek Vasut wrote:
> > > > > > > > Wait for the read/write transfer finish bit get actually 
> > > > > > > > cleared,
> > > > > > > > this does not happen immediately on at least SoCFPGA Gen5.
> > > > > > > > 
> > > > > > > > Signed-off-by: Marek Vasut 
> > > > > > > > Cc: Jagan Teki 
> > > > > > > > Cc: Vignesh R 
> > > > > > > > Cc: Pratyush Yadav 
> > > > > > > > ---
> > > > > > > > drivers/spi/cadence_qspi_apb.c | 17 +
> > > > > > > > 1 file changed, 17 insertions(+)
> > > > > > > > 
> > > > > > > > diff --git a/drivers/spi/cadence_qspi_apb.c 
> > > > > > > > b/drivers/spi/cadence_qspi_apb.c
> > > > > > > > index 429ee335db6..2cdf4c9c9f8 100644
> > > > > > > > --- a/drivers/spi/cadence_qspi_apb.c
> > > > > > > > +++ b/drivers/spi/cadence_qspi_apb.c
> > > > > > > > @@ -858,6 +858,14 @@ 
> > > > > > > > cadence_qspi_apb_indirect_read_execute(struct cadence_spi_plat 
> > > > > > > > *plat,
> > > > > > > > writel(CQSPI_REG_INDIRECTRD_DONE,
> > > > > > > >plat->regbase + CQSPI_REG_INDIRECTRD);
> > > > > > > > +  /* Check indirect done status */
> > > > > > > > +  ret = wait_for_bit_le32(plat->regbase + CQSPI_REG_INDIRECTRD,
> > > > > > > > +  CQSPI_REG_INDIRECTRD_DONE, 0, 10, 0);
> > > > > > > > +  if (ret) {
> > > > > > > > +  printf("Indirect read clear completion error 
> > > > > > > > (%i)\n", ret);
> > > > > > > > +  goto failrd;
> > > > > > > > +  }
> > > > > > > > +
> > > > > > > 
> > > > > > > Huh, this is strange. I would expect the bit to clear immediately 
> > > > > > > since
> > > > > > > it doesn't really do any operation on the flash. How long does it
> > > > > > > usually take to clear? If you don't wait for it to clear does 
> > > > > > > anything
> > > > > > > break?
> > > > > > 
> > > > > > Often it does clear immediately, but there were a few odd cases 
> > > > > > where it did
> > > > > > not happen right away, in which case I had transfer corruption.
> > > > > 
> > > > > By "transfer corruption" do you mean the current transfer gets 
> > > > > corrupted
> > > > > or the next one?
> > > > > 
> > > > > We get here _after_ the transfer is completed and this bit should have
> > > > > little to do with the data received. If the current transfer fails 
> > > > > then
> > > > > I suspect something else might be going wrong the this is just a 
> > > > > symptom
> > > > > of the problem.
> > > > 
> > > > As far as I recall, the problem was triggered when using UBI on the SPI
> > > > NOR, so that could very well be the next transfer.
> > > 
> > > Any further decisions here? shall I take it or v2?
> > 
> > I think we need more information here. I don't see why checking this bit
> > would interfere with the current transfer since that is already finished
> > by the time we get here. If it is the next transfer then this might just
> > be a symptom and the real problem might be somewhere else.
> 
> What additional information do you need ?

Sorry for the late reply. I dropped the ball on this one.

The additional information I needed was to know what exactly happens if 
we don't wait for this bit to be cleared. Does the _current_ transfer go 
though fine? If it does, what gets corrupted in the next transfer? Do 
you only get partial data? Do you get garbage data? Do you get no data 
at all?

All this information would be useful for understanding if this fix is 
the right one, and if so why it is the right one.

-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.

Please pull u-boot-net/next

[Ramon Fried]

Hi Tom,
This pull request contains:
* New Broadcom NetXtreme driver
* Support for socat for netconsole
* Felix switch soft reset fix

The following changes since commit fc47dbb26e9d86a688e69e198b2ed0749db16756:

  Merge branch '2021-12-01-Kconfig-migrations' into next (2021-12-01
13:32:35 -0500)

are available in the Git repository at:

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

for you to fetch changes up to 300761b68da14fc77f3e236f35c459fb1a6769ce:

  board: brcm-ns3: Load netXtreme firmware (2021-12-02 08:34:01 +0200)


Andy Shevchenko (1):
  tools/netconsole: Add support for socat

Bharat Gooty (2):
  net: brcm: netXtreme driver
  board: brcm-ns3: Load netXtreme firmware

Radu Bulie (1):
  drivers: net: Soft reset felix switch core

Ramon Fried (1):
  driver: net: Makefile: order file alphabetically

 board/broadcom/bcmns3/ns3.c |5 +-
 drivers/net/Kconfig |1 +
 drivers/net/Makefile|   79 +-
 drivers/net/bnxt/Kconfig|7 +
 drivers/net/bnxt/Makefile   |5 +
 drivers/net/bnxt/bnxt.c | 1708 +++
 drivers/net/bnxt/bnxt.h |  390 +++
 drivers/net/bnxt/bnxt_dbg.h |  536 ++
 drivers/net/bnxt/bnxt_hsi.h |  889 
 drivers/net/mscc_eswitch/felix_switch.c |   13 +-
 include/pci_ids.h   |3 +
 tools/netconsole|   12 +-
 12 files changed, 3605 insertions(+), 43 deletions(-)
 create mode 100644 drivers/net/bnxt/Kconfig
 create mode 100644 drivers/net/bnxt/Makefile
 create mode 100644 drivers/net/bnxt/bnxt.c
 create mode 100644 drivers/net/bnxt/bnxt.h
 create mode 100644 drivers/net/bnxt/bnxt_dbg.h
 create mode 100644 drivers/net/bnxt/bnxt_hsi.h

Thanks,
Ramon

Re: [PATCH v3 1/2] riscv: Support booting SiFive Unmatched from SPI.

[Leo Liang]

On Wed, Nov 24, 2021 at 02:32:09PM -0800, Thomas Skibo wrote:
> Configure SPI flash devices into SPL.  Add SPI boot option to spl.c.
> Document how to format flash for booting.
> 
> Signed-off-by: Thomas Skibo 
> ---
>  .../dts/hifive-unmatched-a00-u-boot.dtsi  | 11 +++
>  board/sifive/unmatched/spl.c  |  3 ++
>  configs/sifive_unmatched_defconfig|  6 
>  doc/board/sifive/unmatched.rst| 31 +++
>  4 files changed, 51 insertions(+)

Reviewed-by: Leo Yu-Chi Liang 

Re: [PATCH 0/2] binman: Add support for ATF Firmware Image Package (FIP)

[Sandrine Bailleux]

Hi Simon,

On 12/1/21 5:51 PM, Simon Glass wrote:
> Hi Sandrine,
>
> On Wed, 1 Dec 2021 at 03:32, Sandrine Bailleux
>  wrote:
>>
>> Hi everyone,
>>
>> I am Sandrine Bailleux, from the Trusted Firmware-A project. Ilias
>> Apalodimas CC'ed me on this thread.
>>
>> First of all, thanks for involving the TF-A developers in this thread
>> and my apologies for the delay in responding.
>
> Thank you for your response.
>
>>
>> On 11/25/21 6:01 PM, François Ozog wrote:
>>> Hi Simon,
>>>
>>> On Thu, 25 Nov 2021 at 17:49, Simon Glass >> > wrote:
>>>
>>> Hi François,
>>>
>>> On Thu, 25 Nov 2021 at 08:11, François Ozog
>>> mailto:francois.o...@linaro.org>> wrote:
>>> >
>>> > Hi Simon,
>>> >
>>> >
>>> >
>>> >
>>> > On Thu, 25 Nov 2021 at 15:47, Ilias Apalodimas
>>> mailto:ilias.apalodi...@linaro.org>>
>>> wrote:
>>> >>
>>> >> +cc Sandrine
>>> >>
>>> >> On Thu, 25 Nov 2021 at 11:42, Ilias Apalodimas
>>> >> >> > wrote:
>>> >> >
>>> >> > Hi Simon,
>>> >> >
>>> >> >
>>> >> > On Wed, 24 Nov 2021 at 06:09, Simon Glass >> > wrote:
>>> >> > >
>>> >> > >
>>> >> > > This series adds support for the FIP format as used by ARM
>>> Trusted
>>> >> > > Firmware (in particular TF-A).
>>> >> > >
>>> >
>>> > I will use a question you use often "what problem are you trying
>>> to solve?". If FIP format is used it means that TF-A/BL2 is going to
>>> parse it and verify the hashes/signatures according to TF-A scheme.
>>> >
>>> > The packager will embed in a FIP components like Secure Monitor,
>>> Secure hypervisor, Secure partitions code and manifests.
>>> >
>>> > All in all, U-Boot will be representing a small percentage of the
>>> functionality offered by secure firmware  as a whole and it feels
>>> odd to make another implementation that is more "accessory" rather
>>> than critical for the U-Boot community. It may be a good idea but I
>>> wish you could explain it.
>>>
>>> Here is a talk about Binman, its goals and how it works.
>>>
>>> https://www.youtube.com/watch?v=L84ujgUXBOQ
>>>
>>> Think of Binman as a separate tool that brings everything together. It
>>> has grown out of U-Boot, largely because U-Boot is the main firmware
>>> in most cases. Getting U-Boot to start up and boot successfully is the
>>> goal of this packaging process. There are lots of instructions in the
>>> tree and elsewhere about how to build an image comprising U-Boot and
>>> various binary blobs. Binman aims to provide documentation for that
>>> process and an image description that can be used to build an image
>>> reliably.
>>>
>>> We are slowly converting these text instructions into an in-tree image
>>> description.
>>>
>>> I believe that Binman can help bring order to the chaos that is
>>> otherwise only going to grow, with lots of shell scripts, manual
>>> instructions, obscure binary tools, etc. Binman brings everything
>>> together and makes it clear what is needing/missing to build an image.
>>>
>>> >
>>> >> > > This allows images to be created containing a FIP, which
>>> itself contains
>>> >> > > various binaries. With this, image creation can be handled
>>> from an in-tree
>>> >> > > image description instead of needing to perform a lot of
>>> manual steps or
>>> >> > > custom scripts to build the FIP.
>>> >> > >
>>> >
>>> > That's not my experience of building a fip.  Packaging even Linux
>>> as a BL33 (instead of U-Boot) is very simple.
>>>
>>> But not automatic. With Binman you don't need to worry about the
>>> packaging. It is done for you. You just need to find all the binary
>>> blobs that are needed.  This ability is quite important as firmware is
>>> fragmenting and getting very complicated these days.
>>>
>>> Binman runs twice...once in the U-Boot tree to do a build and again
>>> later to repackage, put in a final fdtmap, add signatures and any
>>> final pieces needed.
>>>
>>> See this patch for an example of complicated build instructions with
>>> Odroid-C2 (>10 binary blobs!) and how Binman can help (see the changes
>>> in the .rst file):
>>>
>>> 
>>> https://patchwork.ozlabs.org/project/uboot/patch/20211124040954.699821-8-...@chromium.org/
>>>
>>> That's indeed complicated! I can't comment whether this build process is
>>> "canonical" as per TF-A standards so I'll let TF-A community comment.
>>> Have you factored in the signature issues that Jan@Siemens has in the
>>> overall process?
>>
>> I am a bit confused by the ask here. What input would you like from TF-A
>> community? Are you asking for a code review or are you more interested
>> in getting an opinion about adding support for FIP files in binman?
>
> The latter.
>
>>
>> 

Re: [PATCH 1/2] scripts: remove CONFIG_IS_ENABLED and CONFIG_VAL in generated u_boot.cfg

[Tom Rini]

On Mon, Nov 08, 2021 at 10:21:21AM +0100, Patrick Delaunay wrote:

> The two helpers macros CONFIG_IS_ENABLED and CONFIG_VAL are defined in
> include/linux/kconfig.h but they are not real configurations; they can
> be safely removed in the generated configuration file "u-boot.cfg".
> 
> This patch simplifies the comparison of this U-Boot configuration file.
> 
> Signed-off-by: Patrick Delaunay 
> Acked-by: Simon Glass 

Applied to u-boot/next, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH] spl: fit: Skip attempting to load 0 length image

[Tom Rini]

On Tue, Oct 19, 2021 at 12:32:29PM -0500, Nishanth Menon wrote:

> When, for various reasons, a bad FIT image is used where a loadable
> image is marked as 0 length, attempt is made for a 0 length allocation and
> read of 0 byte read operation.
> 
> Instead provide warning in log and skip attempting to do such a load.
> 
> Signed-off-by: Nishanth Menon 
> Reviewed-by: Aswath Govindraju 

Applied to u-boot/next, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH 2/2] scripts: remove CONFIG_IS_ENABLED and CONFIG_VAL in config_whitelist.txt

[Tom Rini]

On Mon, Nov 08, 2021 at 10:21:22AM +0100, Patrick Delaunay wrote:

> The helper macro CONFIG_IS_ENABLED and CONFIG_VAL are not real
> configurations and they are no more present in u-boot.cfg so they can
> be removed in config_whitelist.txt.
> 
> Signed-off-by: Patrick Delaunay 
> Acked-by: Simon Glass 

Applied to u-boot/next, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [RESEND PATCH 2/2] dm: add debug message when failed to select the default pinctrl

[Tom Rini]

On Fri, Nov 19, 2021 at 10:02:27AM +0100, Patrick Delaunay wrote:

> Add a message on probe in driver model core when the default
> pinctrl selection failed.
> 
> This message is displayed only when the pinctrl API is
> implemented, i.e. when result is not ENOSYS.
> 
> Signed-off-by: Patrick Delaunay 
> Reviewed-by: Simon Glass 

Applied to u-boot/next, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [RESEND PATCH 1/2] pinctrl: change result for unsupported API

[Tom Rini]

On Fri, Nov 19, 2021 at 10:02:26AM +0100, Patrick Delaunay wrote:

> Use the return value ENOSYS for unsupported API
> - pinctrl_generic_set_state
> - pinctrl_select_state
> 
> Signed-off-by: Patrick Delaunay 
> Reviewed-by: Simon Glass 

Applied to u-boot/next, thanks!

-- 
Tom


signature.asc
Description: PGP signature

Re: [PATCH 5/5] clk: Add clk_get_by_name_optional

[Neil Armstrong]

On 01/12/2021 19:43, Sean Anderson wrote:
> This adds a helper function for clk_get_by_name in cases where the clock is
> optional. Hopefully this helps point driver writers in the right direction.
> Also convert some existing users.
> 
> Signed-off-by: Sean Anderson 
> ---
> 
>  drivers/clk/clk_zynq.c  |  5 +++--
>  drivers/rng/meson-rng.c |  4 ++--
>  include/clk.h   | 24 
>  3 files changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clk/clk_zynq.c b/drivers/clk/clk_zynq.c
> index 18915c3e04..e80500e382 100644
> --- a/drivers/clk/clk_zynq.c
> +++ b/drivers/clk/clk_zynq.c
> @@ -472,8 +472,9 @@ static int zynq_clk_probe(struct udevice *dev)
>  
>   for (i = 0; i < 2; i++) {
>   sprintf(name, "gem%d_emio_clk", i);
> - ret = clk_get_by_name(dev, name, >gem_emio_clk[i]);
> - if (ret < 0 && ret != -ENODATA) {
> + ret = clk_get_by_name_optional(dev, name,
> +>gem_emio_clk[i]);
> + if (ret) {
>   dev_err(dev, "failed to get %s clock\n", name);
>   return ret;
>   }
> diff --git a/drivers/rng/meson-rng.c b/drivers/rng/meson-rng.c
> index 5a4f45ad5a..e0a1e8c7e0 100644
> --- a/drivers/rng/meson-rng.c
> +++ b/drivers/rng/meson-rng.c
> @@ -91,8 +91,8 @@ static int meson_rng_of_to_plat(struct udevice *dev)
>   return -ENODEV;
>  
>   /* Get optional "core" clock */
> - err = clk_get_by_name(dev, "core", >clk);
> - if (err && err != -ENODATA)
> + err = clk_get_by_name_optional(dev, "core", >clk);
> + if (err)
>   return err;
>  
>   return 0;
> diff --git a/include/clk.h b/include/clk.h
> index 103ef16bf9..36721188d0 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -292,6 +292,30 @@ static inline int clk_release_all(struct clk *clk, int 
> count)
>  }
>  #endif
>  
> +/**
> + * clk_get_by_name_optional() - Get/request a optional clock by name.
> + * @dev: The client device.
> + * @name:The name of the clock to request, within the client's list of
> + *   clocks.
> + * @clk: A pointer to a clock struct to initialize.
> + *
> + * Behaves the same as clk_get_by_name(), except when there is no clock
> + * provider. In the latter case, return 0.
> + *
> + * Return: 0 if OK, or a negative error code.
> + */
> +static inline int clk_get_by_name_optional(struct udevice *dev,
> +const char *name, struct clk *clk)
> +{
> + int ret;
> +
> + ret = clk_get_by_name_optional(dev, name, clk);
> + if (ret == -ENODATA)
> + return 0;
> +
> + return ret;
> +}
> +
>  /**
>   * clk_get_by_name_nodev_optional - Get/request an optinonal clock by name
>   *   without a device.
> 


For meson-rng:
Reviewed-by: Neil Armstrong 

Re: [PATCH 5/5] clk: Add clk_get_by_name_optional

[Simon Glass]

Hi Sean,

On Wed, 1 Dec 2021 at 11:43, Sean Anderson  wrote:
>
> This adds a helper function for clk_get_by_name in cases where the clock is
> optional. Hopefully this helps point driver writers in the right direction.
> Also convert some existing users.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  drivers/clk/clk_zynq.c  |  5 +++--
>  drivers/rng/meson-rng.c |  4 ++--
>  include/clk.h   | 24 
>  3 files changed, 29 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/clk/clk_zynq.c b/drivers/clk/clk_zynq.c
> index 18915c3e04..e80500e382 100644
> --- a/drivers/clk/clk_zynq.c
> +++ b/drivers/clk/clk_zynq.c
> @@ -472,8 +472,9 @@ static int zynq_clk_probe(struct udevice *dev)
>
> for (i = 0; i < 2; i++) {
> sprintf(name, "gem%d_emio_clk", i);
> -   ret = clk_get_by_name(dev, name, >gem_emio_clk[i]);
> -   if (ret < 0 && ret != -ENODATA) {
> +   ret = clk_get_by_name_optional(dev, name,
> +  >gem_emio_clk[i]);
> +   if (ret) {
> dev_err(dev, "failed to get %s clock\n", name);
> return ret;
> }
> diff --git a/drivers/rng/meson-rng.c b/drivers/rng/meson-rng.c
> index 5a4f45ad5a..e0a1e8c7e0 100644
> --- a/drivers/rng/meson-rng.c
> +++ b/drivers/rng/meson-rng.c
> @@ -91,8 +91,8 @@ static int meson_rng_of_to_plat(struct udevice *dev)
> return -ENODEV;
>
> /* Get optional "core" clock */
> -   err = clk_get_by_name(dev, "core", >clk);
> -   if (err && err != -ENODATA)
> +   err = clk_get_by_name_optional(dev, "core", >clk);
> +   if (err)
> return err;
>
> return 0;
> diff --git a/include/clk.h b/include/clk.h
> index 103ef16bf9..36721188d0 100644
> --- a/include/clk.h
> +++ b/include/clk.h
> @@ -292,6 +292,30 @@ static inline int clk_release_all(struct clk *clk, int 
> count)
>  }
>  #endif
>
> +/**
> + * clk_get_by_name_optional() - Get/request a optional clock by name.

Can I suggest we rename this to ..._opt as it is shorter?

> + * @dev:   The client device.
> + * @name:  The name of the clock to request, within the client's list of
> + * clocks.
> + * @clk:   A pointer to a clock struct to initialize.
> + *
> + * Behaves the same as clk_get_by_name(), except when there is no clock
> + * provider. In the latter case, return 0.
> + *
> + * Return: 0 if OK, or a negative error code.
> + */
> +static inline int clk_get_by_name_optional(struct udevice *dev,
> +  const char *name, struct clk *clk)
> +{
> +   int ret;
> +
> +   ret = clk_get_by_name_optional(dev, name, clk);
> +   if (ret == -ENODATA)
> +   return 0;
> +
> +   return ret;
> +}
> +
>  /**
>   * clk_get_by_name_nodev_optional - Get/request an optinonal clock by name
>   * without a device.
> --
> 2.33.0
>

Regards,
SImon

Re: [PATCH 07/40] efi: Correct call to write_acpi_tables()

[Simon Glass]

Hi Heinrich,

On Wed, 1 Dec 2021 at 22:00, Heinrich Schuchardt  wrote:
>
> Am 1. Dezember 2021 22:12:09 MEZ schrieb Simon Glass :
> >Hi Heinrich,
> >
> >On Wed, 1 Dec 2021 at 12:57, Heinrich Schuchardt  wrote:
> >>
> >>
> >>
> >> On 12/1/21 20:32, Simon Glass wrote:
> >> > Hi Heinrich,
> >> >
> >> > On Wed, 1 Dec 2021 at 11:08, Heinrich Schuchardt  
> >> > wrote:
> >> >>
> >> >> On 12/1/21 17:02, Simon Glass wrote:
> >> >>> This must be passed a ulong, not a u64. Fix it to avoid LTO warnings on
> >> >>> sandbox.
> >> >>>
> >> >>> Signed-off-by: Simon Glass 
> >> >>> ---
> >> >>>
> >> >>>lib/efi_loader/efi_acpi.c | 2 +-
> >> >>>1 file changed, 1 insertion(+), 1 deletion(-)
> >> >>>
> >> >>> diff --git a/lib/efi_loader/efi_acpi.c b/lib/efi_loader/efi_acpi.c
> >> >>> index a62c34009cc..016bbf6db33 100644
> >> >>> --- a/lib/efi_loader/efi_acpi.c
> >> >>> +++ b/lib/efi_loader/efi_acpi.c
> >> >>> @@ -34,7 +34,7 @@ efi_status_t efi_acpi_register(void)
> >> >>> * a 4k-aligned address, so it is safe to assume that
> >> >>> * write_acpi_tables() will write the table at that address.
> >> >>> */
> >> >>> - write_acpi_tables(acpi);
> >> >>> + write_acpi_tables((ulong)acpi);
> >> >>
> >> >> This is wrong: acpi is not an address in the virtual address space of
> >> >> the sandbox. You must not pass it to use map_sysmem() to convert it to a
> >> >> pointer.
> >> >
> >> > I think you are referring to the other patch.
> >> >
> >> > Here the problem is that a u64 is being passed to a ulong. As you say,
> >> > this should be a global prototype and I will fix that in v2.
> >> >
> >> >>
> >> >> Please change the parameter of write_acpi_tables() to be a pointer and
> >> >> move all sandbox specific conversions to the sandbox code.
> >> >
> >> > Sorry, no, I don't want to do that as it would be a significant
> >> > departure from how U-Boot currently works. We use ulong everywhere for
> >> > addresses.
> >>
> >> We typically use pointers to pass references to memory and not ulong.
> >
> >(again, I think this discussion relates to the other patch)
> >
> >That is true in some internal code, but more often a ulong is used for
> >an address, e.g. with the boot flow, commands, ACPI, board init, etc.
> >etc. The ulong is the standard holder of an address in U-Boot and ACPI
> >uses addresses. Haven't we been through this before?
> >
> >> Relative addresses in the virtual address space of the sandbox are only
> >> relevant for testing on and user interaction with the sandbox.
> >>
> >> The location of the memory location for the ACPI table is not determined
> >> by the user.
> >>
> >> Please, have a look at the called function where the sandbox address is
> >> immediately converted back to a pointer. The parameter value is only
> >> used in a debug statement. This double conversion makes is a waste of
> >> memory and CPU cycles. Even for the debug output on the sandbox I would
> >> go for a pointer because that is the value that I will need to
> >> understand what is happening in the booted image.
> >
> >I think you are over-indexing on sandbox. The first thing to explain
> >is why a u64 is used to hold what I believe to be a void pointer.
>
> The UEFI API requires u64 for AllocatePages() and GetMemoryMap(). This is a 
> design choice of the API that I don't expect to be corrected in future API 
> versions.
>
> IMO
> >the API is not right. Or perhaps just its interaction with sandbox is
> >wrong. But somehow, we need to fix the bug in the other patch.
> >
> >The only thing that is useful to see with sandbox is the address. The
> >pointers are random numbers really, not easily related to the
> >addresses that they came from. For example, with 'md 0', I want to see
> >the address '0', not the pointer value which might be 0x33d2712 or
> >anything else.
>
> In U-Boot except for the sandbox pointers is all we need. Other defconfigs 
> neither need nor use a virtual address space.
>
> The only reason why the sandbox uses a virtual address space is ease of user 
> interaction and ease of imlementing unit tests. This is why offsets to the 
> sandbox memory start are passed around instead of addresses (pointers).

Where did you get that idea? U-Boot has always used addresses rather
than pointers. Sandbox happens to take advantage of that, that is all.

>
> Currently the sandbox requirements have spilled to a lot of different code 
> parts. An alternative design could have strictly restricted the 
> offset-address conversions to console input and output.
>
> I don't expect this design to be reverted. But we should not artificially 
> increase the number of places where we use offsets instead of pointers.
>
> If a called function can not use an offset but needs the pointer, passing an 
> offset as a parameter makes no sense.

It is not an offset, it is an address. You are getting tied up in the
sandbox side of this. Casting an address to a pointer should be done
using mapmem, for sandbox compatibility, 

Re: [PATCH] treewide: invaild -> invalid

[Simon Glass]

On Wed, 1 Dec 2021 at 12:27, Sean Anderson  wrote:
>
> Somewhere along the way, someone misspelt "invalid" and it got copied
> everywhere. Fix it.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  drivers/clk/clk-uclass.c   | 2 +-
>  drivers/clk/clk_stm32f.c   | 2 +-
>  drivers/clk/clk_stm32h7.c  | 2 +-
>  drivers/clk/clk_versaclock.c   | 2 +-
>  drivers/clk/renesas/clk-rcar-gen2.c| 2 +-
>  drivers/clk/renesas/clk-rcar-gen3.c| 2 +-
>  drivers/clk/ti/clk-ctrl.c  | 2 +-
>  drivers/dma/dma-uclass.c   | 2 +-
>  drivers/hwspinlock/hwspinlock-uclass.c | 2 +-
>  drivers/mailbox/k3-sec-proxy.c | 2 +-
>  drivers/mailbox/mailbox-uclass.c   | 2 +-
>  drivers/mailbox/tegra-hsp.c| 2 +-
>  drivers/misc/irq-uclass.c  | 2 +-
>  drivers/mux/mux-uclass.c   | 2 +-
>  drivers/phy/phy-uclass.c   | 2 +-
>  drivers/reset/reset-uclass.c   | 2 +-
>  16 files changed, 16 insertions(+), 16 deletions(-)
>

It's almost like a virus.

Reviewed-by: Simon Glass 

Re: [PATCH] clk: Remove no-op request and rfree callbacks

[Simon Glass]

On Wed, 1 Dec 2021 at 12:51, Sean Anderson  wrote:
>
> These callbacks are optional. Remove ones which do nothing.
>
> Signed-off-by: Sean Anderson 
> ---
>
>  drivers/clk/ics8n3qv01.c  |  6 --
>  drivers/clk/tegra/tegra-car-clk.c |  9 -
>  drivers/clk/ti/clk-sci.c  | 14 --
>  3 files changed, 29 deletions(-)

Reviewed-by: Simon Glass 

Re: [RESEND RFC PATCH 04/10] FWU: Add metadata access functions for GPT partitioned block devices

[Simon Glass]

Hi Sughosh,

On Thu, 2 Dec 2021 at 01:05, Sughosh Ganu  wrote:
>
> hi Simon,
>
> On Wed, 1 Dec 2021 at 23:32, Simon Glass  wrote:
>>
>> Hi Sughosh,
>>
>> On Thu, 25 Nov 2021 at 00:13, Sughosh Ganu  wrote:
>> >
>> > In the FWU Multi Bank Update feature, the information about the
>> > updatable images is stored as part of the metadata, on a separate
>> > partition. Add functions for reading from and writing to the metadata
>> > when the updatable images and the metadata are stored on a block
>> > device which is formated with GPT based partition scheme.
>> >
>> > Signed-off-by: Sughosh Ganu 
>> > ---
>> >  lib/fwu_updates/fwu_metadata_gpt_blk.c | 716 +
>> >  1 file changed, 716 insertions(+)
>> >  create mode 100644 lib/fwu_updates/fwu_metadata_gpt_blk.c
>>
>> Can we pick a better name than metadata? It is just so
>> vague...everything is metadata.
>
>
> The specification calls it metadata. And the functions in this file are for 
> accessing the metadata on a GPT partitioned block device. Do you have any 
> alternate name for this?

Well lots of things call their metadata "metadata". That doesn't mean
it is a good name for the code base. Which specification? Do you have
a link?

Regards,
Simon

Re: [PATCH 5/5] clk: Add clk_get_by_name_optional

[Sean Anderson]

Hi Simon,

On 12/2/21 8:43 AM, Simon Glass wrote:

Hi Sean,

On Wed, 1 Dec 2021 at 11:43, Sean Anderson  wrote:


This adds a helper function for clk_get_by_name in cases where the clock is
optional. Hopefully this helps point driver writers in the right direction.
Also convert some existing users.

Signed-off-by: Sean Anderson 
---

  drivers/clk/clk_zynq.c  |  5 +++--
  drivers/rng/meson-rng.c |  4 ++--
  include/clk.h   | 24 
  3 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/clk_zynq.c b/drivers/clk/clk_zynq.c
index 18915c3e04..e80500e382 100644
--- a/drivers/clk/clk_zynq.c
+++ b/drivers/clk/clk_zynq.c
@@ -472,8 +472,9 @@ static int zynq_clk_probe(struct udevice *dev)

 for (i = 0; i < 2; i++) {
 sprintf(name, "gem%d_emio_clk", i);
-   ret = clk_get_by_name(dev, name, >gem_emio_clk[i]);
-   if (ret < 0 && ret != -ENODATA) {
+   ret = clk_get_by_name_optional(dev, name,
+  >gem_emio_clk[i]);
+   if (ret) {
 dev_err(dev, "failed to get %s clock\n", name);
 return ret;
 }
diff --git a/drivers/rng/meson-rng.c b/drivers/rng/meson-rng.c
index 5a4f45ad5a..e0a1e8c7e0 100644
--- a/drivers/rng/meson-rng.c
+++ b/drivers/rng/meson-rng.c
@@ -91,8 +91,8 @@ static int meson_rng_of_to_plat(struct udevice *dev)
 return -ENODEV;

 /* Get optional "core" clock */
-   err = clk_get_by_name(dev, "core", >clk);
-   if (err && err != -ENODATA)
+   err = clk_get_by_name_optional(dev, "core", >clk);
+   if (err)
 return err;

 return 0;
diff --git a/include/clk.h b/include/clk.h
index 103ef16bf9..36721188d0 100644
--- a/include/clk.h
+++ b/include/clk.h
@@ -292,6 +292,30 @@ static inline int clk_release_all(struct clk *clk, int 
count)
  }
  #endif

+/**
+ * clk_get_by_name_optional() - Get/request a optional clock by name.


Can I suggest we rename this to ..._opt as it is shorter?


This follows the general trend of suffixing _optional. For example (and
several of these are borrowed from Linux):

clk_get_optional
reset_control_bulk_get_optional_exclusive
gpiod_get_optional
platform_get_irq_byname_optional

and particularly, the Linux clock subsystem uses _optional and not _opt
as a suffix. While some of these names can get fairly long-winded, I
think we should go for consistency here.

--Sean


+ * @dev:   The client device.
+ * @name:  The name of the clock to request, within the client's list of
+ * clocks.
+ * @clk:   A pointer to a clock struct to initialize.
+ *
+ * Behaves the same as clk_get_by_name(), except when there is no clock
+ * provider. In the latter case, return 0.
+ *
+ * Return: 0 if OK, or a negative error code.
+ */
+static inline int clk_get_by_name_optional(struct udevice *dev,
+  const char *name, struct clk *clk)
+{
+   int ret;
+
+   ret = clk_get_by_name_optional(dev, name, clk);
+   if (ret == -ENODATA)
+   return 0;
+
+   return ret;
+}
+
  /**
   * clk_get_by_name_nodev_optional - Get/request an optinonal clock by name
   * without a device.
--
2.33.0



Regards,
SImon