Re: [PATCH 1/1] riscv: support building double-float modules
Hi Heinrich, On Sat, Oct 08, 2022 at 11:17:57AM +0200, Heinrich Schuchardt wrote: > The riscv32 toolchain for GCC-12 provided by kernel.org contains libgcc.a > compiled for double-float. To link to it we have to adjust how we build > U-Boot. > > As U-Boot actually does not use floating point at all this should not > make a significant difference for the produced binaries. > > Signed-off-by: Heinrich Schuchardt > --- > arch/riscv/Kconfig | 14 ++ > arch/riscv/Makefile | 17 ++--- > 2 files changed, 28 insertions(+), 3 deletions(-) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig > index 32a90b83b5..8add95c8ef 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -152,6 +152,20 @@ config RISCV_ISA_C > when building U-Boot, which results in compressed instructions in the > U-Boot binary. > > +config RISCV_ISA_F > + bool "Standard extension for Single-Precision Floating Point" > + default y > + help > + Adds "F" to the ISA string passed to the compiler. > + > +config RISCV_ISA_D > + bool "Standard extension forr Double-Precision Floating Point" typo "forr" > + depends on RISCV_ISA_F > + default y > + help > + Adds "D" to the ISA string passed to the compiler and changes the > + riscv32 ABI from ilp32 to ipl32d. typo "ipl32d" I think we do not need to specify that the option is created for rv32. > + > config RISCV_ISA_A > def_bool y > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile > index 53d1194ffb..d1b6e86dd8 100644 > --- a/arch/riscv/Makefile > +++ b/arch/riscv/Makefile > @@ -5,15 +5,24 @@ > > ifeq ($(CONFIG_ARCH_RV64I),y) > ARCH_BASE = rv64im > - ABI = lp64 > + ABI_BASE = lp64 > endif > ifeq ($(CONFIG_ARCH_RV32I),y) > ARCH_BASE = rv32im > - ABI = ilp32 > + ABI_BASE = ilp32 > endif > ifeq ($(CONFIG_RISCV_ISA_A),y) > ARCH_A = a > endif > +ifeq ($(CONFIG_RISCV_ISA_F),y) > + ARCH_F = f > +endif > +ifeq ($(CONFIG_RISCV_ISA_D),y) > + ARCH_D = d > +ifeq ($(CONFIG_ARCH_RV32I),y) I think we do not need this if statement as Rick stated. The default libgcc.a of both rv32 and rv64 toolchains from kernel.org are built with double float abi, so this CONFIG_RISCV_ISA_D could be applied to rv64 setup as well. Other than the above, Looks good to me. Reviewed-by: Leo Yu-Chi Liang > + ABI_D = d > +endif > +endif > ifeq ($(CONFIG_RISCV_ISA_C),y) > ARCH_C = c > endif > @@ -24,7 +33,9 @@ ifeq ($(CONFIG_CMODEL_MEDANY),y) > CMODEL = medany > endif > > -RISCV_MARCH = $(ARCH_BASE)$(ARCH_A)$(ARCH_C) > + > +RISCV_MARCH = $(ARCH_BASE)$(ARCH_A)$(ARCH_F)$(ARCH_D)$(ARCH_C) > +ABI = $(ABI_BASE)$(ABI_D) > > # Newer binutils versions default to ISA spec version 20191213 which moves > some > # instructions from the I extension to the Zicsr and Zifencei extensions. > -- > 2.37.2 > Thanks for catching rv32 compilation issue. Best regards, Leo
[PATCH 3/3] timer: xilinx-timer: use timer_conv_64() to fix timer wrap around
Current xilinx_timer_get_count() implementation does not take into account
the periodic 32-bit wrap arounds, as it directly returns the 32-bit counter
register value. The roll-overs cause problems in the upper timer layers, as
generic timer code expects an incrementing 64-bit value from get_count() to
work correctly.
Add the missing 64-bit up-conversion to fix random hangs/delays in
__udelay().
Fixes: a36d86720f ("microblaze: Convert axi timer to DM driver")
Signed-off-by: Ovidiu Panait
---
drivers/timer/xilinx-timer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/timer/xilinx-timer.c b/drivers/timer/xilinx-timer.c
index 75b4473b63..172fd9f929 100644
--- a/drivers/timer/xilinx-timer.c
+++ b/drivers/timer/xilinx-timer.c
@@ -40,7 +40,7 @@ static u64 xilinx_timer_get_count(struct udevice *dev)
regmap_read(priv->regs, TIMER_COUNTER_OFFSET, );
- return value;
+ return timer_conv_64(value);
}
static int xilinx_timer_probe(struct udevice *dev)
--
2.25.1
[PATCH 2/3] timer-uclass: relocate ops pointers for CONFIG_NEEDS_MANUAL_RELOC
Relocate timer_ops pointers when CONFIG_NEEDS_MANUAL_RELOC is enabled.
The (gd->flags & GD_FLG_RELOC) check was added to make sure the reloc_done
logic works for drivers that use DM_FLAG_PRE_RELOC.
Signed-off-by: Ovidiu Panait
---
drivers/timer/timer-uclass.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c
index bdc77b3822..bb71979213 100644
--- a/drivers/timer/timer-uclass.c
+++ b/drivers/timer/timer-uclass.c
@@ -18,6 +18,7 @@
#include
#include
#include
+#include
DECLARE_GLOBAL_DATA_PTR;
@@ -50,6 +51,19 @@ unsigned long notrace timer_get_rate(struct udevice *dev)
static int timer_pre_probe(struct udevice *dev)
{
+ if (IS_ENABLED(CONFIG_NEEDS_MANUAL_RELOC) &&
+ (gd->flags & GD_FLG_RELOC)) {
+ struct timer_ops *ops = timer_get_ops(dev);
+ static int reloc_done;
+
+ if (!reloc_done) {
+ if (ops->get_count)
+ MANUAL_RELOC(ops->get_count);
+
+ reloc_done++;
+ }
+ }
+
if (CONFIG_IS_ENABLED(OF_REAL)) {
struct timer_dev_priv *uc_priv = dev_get_uclass_priv(dev);
struct clk timer_clk;
--
2.25.1
[PATCH 1/3] timer-uclass: add timer_get_ops() macro
Align timer uclass with the other subsystems and provide a timer_get_ops()
convenience macro.
Using this instead of the generic device_get_ops() also prevents
-Wdiscarded-qualifiers warnings when used with non-const variables.
Signed-off-by: Ovidiu Panait
---
drivers/timer/timer-uclass.c | 2 +-
include/timer.h | 2 ++
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/timer/timer-uclass.c b/drivers/timer/timer-uclass.c
index cbc3647698..bdc77b3822 100644
--- a/drivers/timer/timer-uclass.c
+++ b/drivers/timer/timer-uclass.c
@@ -32,7 +32,7 @@ DECLARE_GLOBAL_DATA_PTR;
int notrace timer_get_count(struct udevice *dev, u64 *count)
{
- const struct timer_ops *ops = device_get_ops(dev);
+ struct timer_ops *ops = timer_get_ops(dev);
if (!ops->get_count)
return -ENOSYS;
diff --git a/include/timer.h b/include/timer.h
index a044cb034e..d33a26e28f 100644
--- a/include/timer.h
+++ b/include/timer.h
@@ -6,6 +6,8 @@
#ifndef _TIMER_H_
#define _TIMER_H_
+#define timer_get_ops(dev) ((struct timer_ops *)(dev)->driver->ops)
+
/**
* dm_timer_init() - initialize a timer for time keeping. On success
* initializes gd->timer so that lib/timer can use it for future
--
2.25.1
Re: [PATCH 1/1] riscv: support building double-float modules
> From: Heinrich Schuchardt > Sent: Saturday, October 08, 2022 5:18 PM > To: Rick Jian-Zhi Chen(陳建志) ; Leo Yu-Chi Liang(梁育齊) > > Cc: Tom Rini ; u-boot@lists.denx.de; Heinrich Schuchardt > > Subject: [PATCH 1/1] riscv: support building double-float modules > > The riscv32 toolchain for GCC-12 provided by kernel.org contains libgcc.a > compiled for double-float. To link to it we have to adjust how we build > U-Boot. > > As U-Boot actually does not use floating point at all this should not make a > significant difference for the produced binaries. > > Signed-off-by: Heinrich Schuchardt > --- > arch/riscv/Kconfig | 14 ++ arch/riscv/Makefile | 17 > ++--- > 2 files changed, 28 insertions(+), 3 deletions(-) > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index > 32a90b83b5..8add95c8ef 100644 > --- a/arch/riscv/Kconfig > +++ b/arch/riscv/Kconfig > @@ -152,6 +152,20 @@ config RISCV_ISA_C > when building U-Boot, which results in compressed instructions in > the > U-Boot binary. > > +config RISCV_ISA_F > + bool "Standard extension for Single-Precision Floating Point" > + default y Shall this default y need to depend on RV32 ? Reviewed-by: Rick Chen > + help > + Adds "F" to the ISA string passed to the compiler. > + > +config RISCV_ISA_D > + bool "Standard extension forr Double-Precision Floating Point" > + depends on RISCV_ISA_F > + default y > + help > + Adds "D" to the ISA string passed to the compiler and changes the > + riscv32 ABI from ilp32 to ipl32d. > + > config RISCV_ISA_A > def_bool y > > diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile index > 53d1194ffb..d1b6e86dd8 100644 > --- a/arch/riscv/Makefile > +++ b/arch/riscv/Makefile > @@ -5,15 +5,24 @@ > > ifeq ($(CONFIG_ARCH_RV64I),y) > ARCH_BASE = rv64im > - ABI = lp64 > + ABI_BASE = lp64 > endif > ifeq ($(CONFIG_ARCH_RV32I),y) > ARCH_BASE = rv32im > - ABI = ilp32 > + ABI_BASE = ilp32 > endif > ifeq ($(CONFIG_RISCV_ISA_A),y) > ARCH_A = a > endif > +ifeq ($(CONFIG_RISCV_ISA_F),y) > + ARCH_F = f > +endif > +ifeq ($(CONFIG_RISCV_ISA_D),y) > + ARCH_D = d > +ifeq ($(CONFIG_ARCH_RV32I),y) > + ABI_D = d > +endif > +endif > ifeq ($(CONFIG_RISCV_ISA_C),y) > ARCH_C = c > endif > @@ -24,7 +33,9 @@ ifeq ($(CONFIG_CMODEL_MEDANY),y) > CMODEL = medany > endif > > -RISCV_MARCH = $(ARCH_BASE)$(ARCH_A)$(ARCH_C) > + > +RISCV_MARCH = $(ARCH_BASE)$(ARCH_A)$(ARCH_F)$(ARCH_D)$(ARCH_C) > +ABI = $(ABI_BASE)$(ABI_D) > > # Newer binutils versions default to ISA spec version 20191213 which moves > some # instructions from the I extension to the Zicsr and Zifencei > extensions. > --
Re: [PATCH v5 2/3] arch/riscv: add semihosting support for RISC-V
> From: Kautuk Consul > Sent: Friday, September 23, 2022 3:03 PM > To: Rayagonda Kokatanur ; Sean Anderson > ; Rick Jian-Zhi Chen(陳建志) ; Leo > Yu-Chi Liang(梁育齊) ; Bin Meng ; > Simon Glass ; Ilias Apalodimas > ; Alexandru Gagniuc ; > Philippe Reynes ; Heinrich Schuchardt > ; Rasmus Villemoes ; Eugen > Hristev ; Stefan Roese ; Loic > Poulain ; Peng Fan ; Michal Simek > > Cc: u-boot@lists.denx.de; Kautuk Consul ; Anup > Patel > Subject: [PATCH v5 2/3] arch/riscv: add semihosting support for RISC-V > > We add RISC-V semihosting based serial console for JTAG based early debugging. > > The RISC-V semihosting specification is available at: > https://github.com/riscv/riscv-semihosting-spec/blob/main/riscv-semihosting-spec.adoc > > Signed-off-by: Anup Patel > Signed-off-by: Kautuk Consul > --- > arch/riscv/include/asm/spl.h | 1 + > arch/riscv/lib/Makefile | 2 ++ > arch/riscv/lib/interrupts.c | 25 + > arch/riscv/lib/semihosting.c | 24 > lib/Kconfig | 10 +- > 5 files changed, 57 insertions(+), 5 deletions(-) create mode 100644 > arch/riscv/lib/semihosting.c Reviewed-by: Rick Chen
Re: [PATCH v5 3/3] board: qemu-riscv: enable semihosting
> From: Kautuk Consul > Sent: Friday, September 23, 2022 3:03 PM > To: Rayagonda Kokatanur ; Sean Anderson > ; Rick Jian-Zhi Chen(陳建志) ; Leo > Yu-Chi Liang(梁育齊) ; Bin Meng ; > Simon Glass ; Ilias Apalodimas > ; Alexandru Gagniuc ; > Philippe Reynes ; Heinrich Schuchardt > ; Rasmus Villemoes ; Eugen > Hristev ; Stefan Roese ; Loic > Poulain ; Peng Fan ; Michal Simek > > Cc: u-boot@lists.denx.de; Kautuk Consul > Subject: [PATCH v5 3/3] board: qemu-riscv: enable semihosting > > To enable semihosting we also need to enable the following configs in > defconfigs: > CONFIG_SEMIHOSTING > CONFIG_SPL_SEMIHOSTING > CONFIG_SEMIHOSTING_SERIAL > CONFIG_SERIAL_PROBE_ALL > CONFIG_SPL_FS_EXT4 > CONFIG_SPL_FS_FAT > > Signed-off-by: Kautuk Consul > --- > configs/qemu-riscv32_defconfig | 4 > configs/qemu-riscv32_smode_defconfig | 4 > configs/qemu-riscv32_spl_defconfig | 7 +++ > configs/qemu-riscv64_defconfig | 4 > configs/qemu-riscv64_smode_defconfig | 4 > configs/qemu-riscv64_spl_defconfig | 7 +++ > 6 files changed, 30 insertions(+) Reviewed-by: Rick Chen
Re: [PATCH 1/1] dm: fix blk_get_devnum_by_uclass_idname()
On Tue, Oct 11, 2022 at 03:54:32PM -0600, Simon Glass wrote: > Hi Heinrich, > > On Tue, 11 Oct 2022 at 14:17, Heinrich Schuchardt > wrote: > > > > On 10/11/22 16:16, Simon Glass wrote: > > > Hi Heinrich, > > > > > > On Tue, 11 Oct 2022 at 04:38, Heinrich Schuchardt > > > wrote: > > >> > > >> > > >> > > >> On 10/11/22 07:46, Heinrich Schuchardt wrote: > > >>> > > >>> > > >>> On 10/11/22 01:49, Simon Glass wrote: > > Hi Heinrich, > > > > On Thu, 6 Oct 2022 at 14:05, Heinrich Schuchardt > > wrote: > > > > > > On 10/3/22 18:44, Simon Glass wrote: > > >> Hi Heinrich, > > >> > > >> On Mon, 3 Oct 2022 at 10:33, Heinrich Schuchardt > > >> wrote: > > >>> > > >>> > > >>> > > >>> On 10/3/22 16:57, Simon Glass wrote: > > Hi Heinrich, > > > > On Mon, 3 Oct 2022 at 03:36, Heinrich Schuchardt > > wrote: > > > > > > On the sandbox I run: > > > > > > => setenv efi_selftest block device > > > => bootefi selftest > > > > > > and see the following output: > > > > > > ** Bad device specification host 0 ** > > > Couldn't find partition host 0:0 > > > Cannot read EFI system partition > > > > > > Running > > > > > > => lsblk > > > > > > yields > > > > > > Block Driver Devices > > > - > > > efi_blk : efiloader 0 > > > ide_blk : > > > mmc_blk : mmc 2, mmc 1, mmc 0 > > > nvme-blk: > > > sandbox_host_blk: > > > scsi_blk: > > > usb_storage_blk : > > > virtio-blk : > > > > > > So a efi_blk device was mistaken for a host device. > > > > > > I continue with > > > > > > => host bind 0 ../sandbox.img > > > => ls host 0:1 > > > > > > and get the following output: > > > > > >13 hello.txt > > > 7 u-boot.txt > > > > > > 2 file(s), 0 dir(s) > > > > > > This is the content of efiblock 0:1 and not of host 0:1 (sic!). > > > > > > The uclass of the parent device is irrelevant for the > > > determination of the > > > uclass of the block device. We must use the uclass stored in the > > > block > > > device descriptor. > > > > > > This issue has been raised repeatedly: > > > > > > [PATCH 1/1] block: fix blk_get_devnum_by_typename() > > > https://lore.kernel.org/u-boot/20220802094933.69170-1-heinrich.schucha...@canonical.com/ > > > [PATCH 1/1] blk: simplify blk_get_devnum_by_typename() > > > https://lore.kernel.org/u-boot/20211023140647.7661-1-heinrich.schucha...@canonical.com/ > > > > Yes and you were not able/willing to take on the required work, so > > this carried on longer than it should have. I finally did this > > myself > > and it is now in -next. > > >>> > > >>> The refactoring was orthogonal to the problem that I reported and > > >>> which > > >>> you unfortunately did not consider in the process. > > >> > > >> Well it involved using if_type to work around a problem, just making > > >> it harder to get rid of. Overall I am in favour of a faster pace of > > >> migration that we have been following and it would help if people > > >> took > > >> on some of this, instead of fixing their little issue. > > >> > > >>> > > > > So we might finally be able to fix this problem properly, since > > if_type is mostly just a work-around concept in -next, with just > > the > > fake uclass_id being used at present. > > > > Can you use if_type_to_uclass_id() here, which is the work-around > > function for now? > > >>> > > >>> This function does not exist in origin/next. We won't apply this > > >>> patch > > >>> in the 2022-10 cycle. > > >> > > >> I think I mean conv_uclass_id() which is the new name. > > >> > > >>> > > >>> Let's fix the bug first before thinking about future refactoring. > > >>> > > >>> You may determine the uclass ID for field bdev in struct blk_desc > > >>> using > > >>> function device_get_uclass_id() when refactoring. > > >> > > >> So if you call conv_uclass_id() (without any other refactoring) does > > >> that fix the problem? > > > > > > Except for UCLASS_USB that function is a NOP. How could it help to > > > differentiate between devices with the
Re: [PATCH 1/1] efi_loader: CloseProtocol in efi_fmp_find
On Tue, Oct 11, 2022 at 01:12:18PM +0200, Heinrich Schuchardt wrote:
>
>
> On 10/11/22 13:08, Heinrich Schuchardt wrote:
> >
> >
> > On 10/11/22 09:35, AKASHI Takahiro wrote:
> > > On Tue, Oct 11, 2022 at 07:58:11AM +0200, Heinrich Schuchardt wrote:
> > > >
> > > >
> > > > On 10/11/22 02:49, AKASHI Takahiro wrote:
> > > > > The commit message is not accurate.
> > > > >
> > > > > On Fri, Oct 07, 2022 at 04:06:23PM +0200, Heinrich Schuchardt wrote:
> > > > > > The CloseProtocol() boot service requires a handle as first
> > > > > > argument.
> > > > > > Passing the protocol interface is incorrect.
> > > > >
> > > > > Correct, but
> > > > >
> > > > > > CloseProtocol() only has an effect if called with a non-zero value
> > > > > > for
> > > > > > agent_handle. HandleProtocol() uses an opaque
> > > > > > agent_handle when invoking
> > > > > > OpenProtocol() (currently NULL).
> > > > >
> > > > > No. OpenProtocol() is called with efi_root as an agent handle.
> > > > > So, calling CloseProtocol() is a right thing at the end.
> > > >
> > > > Typically an agent handle is used to relate to a driver exposing
> > > > the driver
> > > > binding protocol.
> > >
> > > Why can't we, other than a driver, call HandleProtocol()
> > > as a convenient way of accessing an interface?
> >
> > The description of HandleProtocol() clearly says that it is deprecated.
> >
> > The assumption that the UEFI specification makes in it is example code
> > that you never be able to close a protocol opened with HandleProtocol.
> >
> > After the first usage of handle protocol the open protocol information
> > with the opaque agent handle will block the protocol interface from ever
> > being removed by the driver exposing it.
> >
> > >
> > > > The root node does not expose the driver binding protocol.
> > >
> > > So do you mean the current implementation of HandleProtocol() is wrong?
> >
> > Yes. If you ever install a boot time driver, it might remove a protocol
> > interface which is actually still in use.
>
> Since 755d42d4209e ("efi_loader: correct HandleProtocol()") we set agent
> handle = efi_root in the implementation of HandleProtocol(). So this part is
> ok.
That is why I said using HandleProtocl() is valid and that
your commit message is not accurate.
-Takahiro Akashi
> Best regards
>
> Heirnich
>
> >
> > >
> > > > Why would you want to create an open protocol information entry here?
> > >
> > > To access get_image_info() quickly.
> >
> > This is not related to an open protocol information (see the UEFI spec
> > description of OpenProtocolInformation()).
> >
> > Best regards
> >
> > Heinrich
> >
> > >
> > > > Do you think anything with the code after the patch is wrong?
> > >
> > > No reason to replace handle_protocol().
> > >
> > > Another example is here:
> > > efi_load_image_from_path()
> > > efi_handle_protocol(device, guid, (void **)_file_protocol));
> > > ...
> > > efi_close_protocol(device, guid, efi_root, NULL);
> > >
> > > I believe that this function is anything but a driver.
> > > I think using HandleProtocol() (or preferably OpenProtocol()) and
> > > CloseProtocol()
> > > in pair seems totally sane.
> > >
> > > -Takahiro Akashi
> > >
> > >
> > >
> > > > Best regards
> > > >
> > > > Heinrich
> > > >
> > > > >
> > > > > > Therefore HandleProtocol() should be
> > > > > > avoided.
> > > > > >
> > > > > > * Replace the LocateHandle() call by efi_search_protocol().
> > > > >
> > > > > LocateHandle() -> efi_handle_protocol()
> > > > >
> > > > > So you could have fixed this way:
> > > > > EFI_CALL(efi_close_protocol(handle, ..., _root, NULL);
> > > > >
> > > > > I preferred to use EFI_CALL() over this file as you can see.
> > > > >
> > > > > -Takahiro Akashi
> > > > >
> > > > > > * Remove the CloseProtocol() call.
> > > > > >
> > > > > > Fixes: 8d99026f0697 ("efi_loader: capsule: support firmware update")
> > > > > > Signed-off-by: Heinrich Schuchardt
> > > > > >
> > > > > > ---
> > > > > > lib/efi_loader/efi_capsule.c | 14 ++
> > > > > > 1 file changed, 6 insertions(+), 8 deletions(-)
> > > > > >
> > > > > > diff --git a/lib/efi_loader/efi_capsule.c
> > > > > > b/lib/efi_loader/efi_capsule.c
> > > > > > index b6bd2d6af8..397e393a18 100644
> > > > > > --- a/lib/efi_loader/efi_capsule.c
> > > > > > +++ b/lib/efi_loader/efi_capsule.c
> > > > > > @@ -159,12 +159,14 @@ efi_fmp_find(efi_guid_t
> > > > > > *image_type, u8 image_index, u64 instance,
> > > > > > efi_status_t ret;
> > > > > > for (i = 0, handle = handles; i < no_handles; i++, handle++)
> > > > > > {
> > > > > > - ret = EFI_CALL(efi_handle_protocol(
> > > > > > - *handle,
> > > > > > - _guid_firmware_management_protocol,
> > > > > > - (void **)));
> > > > > > + struct efi_handler *fmp_handler;
> > > > > > +
> > > > > > + ret = efi_search_protocol(
> > > > > > + *handle,
Re: [PATCH 1/1] efi_loader: CloseProtocol in efi_fmp_find
On Tue, Oct 11, 2022 at 01:08:26PM +0200, Heinrich Schuchardt wrote:
>
>
> On 10/11/22 09:35, AKASHI Takahiro wrote:
> > On Tue, Oct 11, 2022 at 07:58:11AM +0200, Heinrich Schuchardt wrote:
> > >
> > >
> > > On 10/11/22 02:49, AKASHI Takahiro wrote:
> > > > The commit message is not accurate.
> > > >
> > > > On Fri, Oct 07, 2022 at 04:06:23PM +0200, Heinrich Schuchardt wrote:
> > > > > The CloseProtocol() boot service requires a handle as first argument.
> > > > > Passing the protocol interface is incorrect.
> > > >
> > > > Correct, but
> > > >
> > > > > CloseProtocol() only has an effect if called with a non-zero value for
> > > > > agent_handle. HandleProtocol() uses an opaque agent_handle when
> > > > > invoking
> > > > > OpenProtocol() (currently NULL).
> > > >
> > > > No. OpenProtocol() is called with efi_root as an agent handle.
> > > > So, calling CloseProtocol() is a right thing at the end.
> > >
> > > Typically an agent handle is used to relate to a driver exposing the
> > > driver
> > > binding protocol.
> >
> > Why can't we, other than a driver, call HandleProtocol()
> > as a convenient way of accessing an interface?
>
> The description of HandleProtocol() clearly says that it is deprecated.
>
> The assumption that the UEFI specification makes in it is example code that
> you never be able to close a protocol opened with HandleProtocol.
I don't understand this statement.
I never find any description of "you never be able to close a protocol opened
with HandleProtocol" in the spec.
> After the first usage of handle protocol the open protocol information with
> the opaque agent handle will block the protocol interface from ever being
> removed by the driver exposing it.
>
> >
> > > The root node does not expose the driver binding protocol.
> >
> > So do you mean the current implementation of HandleProtocol() is wrong?
>
> Yes. If you ever install a boot time driver, it might remove a protocol
> interface which is actually still in use.
Again, what about efi_load_image_from_path() that I gave in another example?
I think that you have something to do *before* you submit this patch.
Anyhow, your commit message is not accurate.
-Takahiro Akashi
> >
> > > Why would you want to create an open protocol information entry here?
> >
> > To access get_image_info() quickly.
>
> This is not related to an open protocol information (see the UEFI spec
> description of OpenProtocolInformation()).
>
> Best regards
>
> Heinrich
>
> >
> > > Do you think anything with the code after the patch is wrong?
> >
> > No reason to replace handle_protocol().
> >
> > Another example is here:
> > efi_load_image_from_path()
> > efi_handle_protocol(device, guid, (void **)_file_protocol));
> > ...
> > efi_close_protocol(device, guid, efi_root, NULL);
> >
> > I believe that this function is anything but a driver.
> > I think using HandleProtocol() (or preferably OpenProtocol()) and
> > CloseProtocol()
> > in pair seems totally sane.
> >
> > -Takahiro Akashi
> >
> >
> >
> > > Best regards
> > >
> > > Heinrich
> > >
> > > >
> > > > > Therefore HandleProtocol() should be
> > > > > avoided.
> > > > >
> > > > > * Replace the LocateHandle() call by efi_search_protocol().
> > > >
> > > > LocateHandle() -> efi_handle_protocol()
> > > >
> > > > So you could have fixed this way:
> > > > EFI_CALL(efi_close_protocol(handle, ..., _root, NULL);
> > > >
> > > > I preferred to use EFI_CALL() over this file as you can see.
> > > >
> > > > -Takahiro Akashi
> > > >
> > > > > * Remove the CloseProtocol() call.
> > > > >
> > > > > Fixes: 8d99026f0697 ("efi_loader: capsule: support firmware update")
> > > > > Signed-off-by: Heinrich Schuchardt
> > > > > ---
> > > > >lib/efi_loader/efi_capsule.c | 14 ++
> > > > >1 file changed, 6 insertions(+), 8 deletions(-)
> > > > >
> > > > > diff --git a/lib/efi_loader/efi_capsule.c
> > > > > b/lib/efi_loader/efi_capsule.c
> > > > > index b6bd2d6af8..397e393a18 100644
> > > > > --- a/lib/efi_loader/efi_capsule.c
> > > > > +++ b/lib/efi_loader/efi_capsule.c
> > > > > @@ -159,12 +159,14 @@ efi_fmp_find(efi_guid_t *image_type, u8
> > > > > image_index, u64 instance,
> > > > > efi_status_t ret;
> > > > > for (i = 0, handle = handles; i < no_handles; i++, handle++) {
> > > > > - ret = EFI_CALL(efi_handle_protocol(
> > > > > - *handle,
> > > > > - _guid_firmware_management_protocol,
> > > > > - (void **)));
> > > > > + struct efi_handler *fmp_handler;
> > > > > +
> > > > > + ret = efi_search_protocol(
> > > > > + *handle,
> > > > > _guid_firmware_management_protocol,
> > > > > + _handler);
> > > > > if (ret != EFI_SUCCESS)
> > > > > continue;
> > > > > + fmp = fmp_handler->protocol_interface;
> > >
[PATCH] image: fit: Fix not verifying data configuration
When reading data from a FIT image, we must verify the configuration we
get it from. This is because when we have a key with required = "conf",
the image does not need any particular signature or hash. The
configuration is the only required verification, so we must verify it.
Users of fit_get_data_node are liable to load unsigned data unless the
user has set required = "image". Even then, they are vulnerable to
mix-and-match attacks. This also affects other callers of
fit_image_verify which don't first call fit_config_verify, such as
source and imxtract. I don't think there is a backwards-compatible way
to fix these interfaces. Fundamentally, selecting data by image when
images are not required to be verified is unsafe.
Fixes: 37feaf2f727 ("image: fit: Add some helpers for getting data")
Signed-off-by: Sean Anderson
---
boot/image-fit.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/boot/image-fit.c b/boot/image-fit.c
index 9c04ff78a15..632fd405e29 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -1948,7 +1948,14 @@ int fit_get_data_node(const void *fit, const char
*image_uname,
int fit_get_data_conf_prop(const void *fit, const char *prop_name,
const void **data, size_t *size)
{
- int noffset = fit_conf_get_node(fit, NULL);
+ int ret, noffset = fit_conf_get_node(fit, NULL);
+
+ if (noffset < 0)
+ return noffset;
+
+ ret = fit_config_verify(fit, noffset);
+ if (ret)
+ return ret;
noffset = fit_conf_get_prop_node(fit, noffset, prop_name);
return fit_get_data_tail(fit, noffset, data, size);
--
2.35.1.1320.gc452695387.dirty
Re: [PATCH 0/3] arm64: layerscape: Various small size reductions for SPL
Hi NXP Folks, On 10/11/22 5:51 PM, Sean Anderson wrote: > This series groups a few small size reductions I found while trying to > reduce my SPL size. They are all independent of each other. > > > Sean Anderson (3): > arm: layerscape: Don't select FSL_IFC when booting from SD card > arm: layerscape: Disable unused parts of ICID tables > arm: fsl: csu: Reduce size of ns_dev > > arch/arm/cpu/armv8/fsl-layerscape/Kconfig| 4 ++-- > arch/arm/cpu/armv8/fsl-layerscape/icid.c | 2 ++ > .../include/asm/arch-fsl-layerscape/fsl_icid.h | 16 ++-- > include/fsl_csu.h| 4 ++-- > 4 files changed, 20 insertions(+), 6 deletions(-) > I noticed when sending this series that Rajesh's email bounces. Does he still work at NXP? He seems to maintain a significant number of Layerscape boards. $ git grep "rajesh.bha...@nxp.com" board/freescale/ls1012afrdm/MAINTAINERS:M: Rajesh Bhagat board/freescale/ls1012aqds/MAINTAINERS:M: Rajesh Bhagat board/freescale/ls1012ardb/MAINTAINERS:M: Rajesh Bhagat board/freescale/ls1028a/MAINTAINERS:M: Rajesh Bhagat board/freescale/ls1028a/MAINTAINERS:M: Rajesh Bhagat board/freescale/ls1043aqds/MAINTAINERS:M: Rajesh Bhagat board/freescale/ls1043ardb/MAINTAINERS:M: Rajesh Bhagat board/freescale/ls1046aqds/MAINTAINERS:M: Rajesh Bhagat board/freescale/ls1046ardb/MAINTAINERS:M: Rajesh Bhagat board/freescale/ls1088a/MAINTAINERS:M: Rajesh Bhagat board/freescale/ls1088a/MAINTAINERS:M: Rajesh Bhagat board/freescale/ls2080aqds/MAINTAINERS:M: Rajesh Bhagat board/freescale/ls2080ardb/MAINTAINERS:M: Rajesh Bhagat --Sean
Re: [PATCH 1/1] dm: fix blk_get_devnum_by_uclass_idname()
Hi Heinrich, On Tue, 11 Oct 2022 at 14:17, Heinrich Schuchardt wrote: > > On 10/11/22 16:16, Simon Glass wrote: > > Hi Heinrich, > > > > On Tue, 11 Oct 2022 at 04:38, Heinrich Schuchardt > > wrote: > >> > >> > >> > >> On 10/11/22 07:46, Heinrich Schuchardt wrote: > >>> > >>> > >>> On 10/11/22 01:49, Simon Glass wrote: > Hi Heinrich, > > On Thu, 6 Oct 2022 at 14:05, Heinrich Schuchardt > wrote: > > > > On 10/3/22 18:44, Simon Glass wrote: > >> Hi Heinrich, > >> > >> On Mon, 3 Oct 2022 at 10:33, Heinrich Schuchardt > >> wrote: > >>> > >>> > >>> > >>> On 10/3/22 16:57, Simon Glass wrote: > Hi Heinrich, > > On Mon, 3 Oct 2022 at 03:36, Heinrich Schuchardt > wrote: > > > > On the sandbox I run: > > > > => setenv efi_selftest block device > > => bootefi selftest > > > > and see the following output: > > > > ** Bad device specification host 0 ** > > Couldn't find partition host 0:0 > > Cannot read EFI system partition > > > > Running > > > > => lsblk > > > > yields > > > > Block Driver Devices > > - > > efi_blk : efiloader 0 > > ide_blk : > > mmc_blk : mmc 2, mmc 1, mmc 0 > > nvme-blk: > > sandbox_host_blk: > > scsi_blk: > > usb_storage_blk : > > virtio-blk : > > > > So a efi_blk device was mistaken for a host device. > > > > I continue with > > > > => host bind 0 ../sandbox.img > > => ls host 0:1 > > > > and get the following output: > > > >13 hello.txt > > 7 u-boot.txt > > > > 2 file(s), 0 dir(s) > > > > This is the content of efiblock 0:1 and not of host 0:1 (sic!). > > > > The uclass of the parent device is irrelevant for the > > determination of the > > uclass of the block device. We must use the uclass stored in the > > block > > device descriptor. > > > > This issue has been raised repeatedly: > > > > [PATCH 1/1] block: fix blk_get_devnum_by_typename() > > https://lore.kernel.org/u-boot/20220802094933.69170-1-heinrich.schucha...@canonical.com/ > > [PATCH 1/1] blk: simplify blk_get_devnum_by_typename() > > https://lore.kernel.org/u-boot/20211023140647.7661-1-heinrich.schucha...@canonical.com/ > > Yes and you were not able/willing to take on the required work, so > this carried on longer than it should have. I finally did this myself > and it is now in -next. > >>> > >>> The refactoring was orthogonal to the problem that I reported and > >>> which > >>> you unfortunately did not consider in the process. > >> > >> Well it involved using if_type to work around a problem, just making > >> it harder to get rid of. Overall I am in favour of a faster pace of > >> migration that we have been following and it would help if people took > >> on some of this, instead of fixing their little issue. > >> > >>> > > So we might finally be able to fix this problem properly, since > if_type is mostly just a work-around concept in -next, with just the > fake uclass_id being used at present. > > Can you use if_type_to_uclass_id() here, which is the work-around > function for now? > >>> > >>> This function does not exist in origin/next. We won't apply this patch > >>> in the 2022-10 cycle. > >> > >> I think I mean conv_uclass_id() which is the new name. > >> > >>> > >>> Let's fix the bug first before thinking about future refactoring. > >>> > >>> You may determine the uclass ID for field bdev in struct blk_desc > >>> using > >>> function device_get_uclass_id() when refactoring. > >> > >> So if you call conv_uclass_id() (without any other refactoring) does > >> that fix the problem? > > > > Except for UCLASS_USB that function is a NOP. How could it help to > > differentiate between devices with the same parent device? > > It can't. But the root node should not have UCLASS_BLK children. I > think I mentioned that a few months back? > > > > > Would you agree that blk_get_devnum_by_uclass_idname() should not look > > at the parent but on the actual device? > > No, looking at the parent is exactly what it should do. A block device
[PATCH] mkimage: fit: Fix signing of configs with external data
Just like we exclude data-size, data-position, and data-offset from
fit_config_check_sig, we must exclude them while signing as well.
Fixes: 8edecd3110e ("fit: Fix verification of images with external data")
Fixes: c522949a29d ("rsa: sig: fix config signature check for fit with padding")
Signed-off-by: Sean Anderson
---
tools/image-host.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/image-host.c b/tools/image-host.c
index 698adfb3e1d..5ba6e3bbce0 100644
--- a/tools/image-host.c
+++ b/tools/image-host.c
@@ -917,7 +917,12 @@ static int fit_config_get_regions(const void *fit, int
conf_noffset,
int *region_countp, char **region_propp,
int *region_proplen)
{
- char * const exc_prop[] = {"data"};
+ char * const exc_prop[] = {
+ "data",
+ "data-size",
+ "data-position",
+ "data-offset"
+ };
struct strlist node_inc;
struct image_region *region;
struct fdt_region fdt_regions[100];
--
2.35.1.1320.gc452695387.dirty
[PATCH 3/3] arm: fsl: csu: Reduce size of ns_dev
None of the values in this struct are larger than 256, so we can reduce
the members to u8s. This saves around 1K.
Signed-off-by: Sean Anderson
---
include/fsl_csu.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/fsl_csu.h b/include/fsl_csu.h
index 0e59ac3c324..40c29687979 100644
--- a/include/fsl_csu.h
+++ b/include/fsl_csu.h
@@ -24,8 +24,8 @@ enum csu_cslx_access {
};
struct csu_ns_dev {
- unsigned long ind;
- uint32_t val;
+ u8 ind;
+ u8 val;
};
void enable_layerscape_ns_access(void);
--
2.35.1.1320.gc452695387.dirty
[PATCH 1/3] arm: layerscape: Don't select FSL_IFC when booting from SD card
FSL_IFC should only be selected when booting from NAND flash (or when NAND_FSL_IFC is enabled). The existing logic does this correctly when QSPI is also enabled, but not when just booting from SD. Signed-off-by: Sean Anderson --- arch/arm/cpu/armv8/fsl-layerscape/Kconfig | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig index 80a1642447d..10eef84f8d9 100644 --- a/arch/arm/cpu/armv8/fsl-layerscape/Kconfig +++ b/arch/arm/cpu/armv8/fsl-layerscape/Kconfig @@ -62,7 +62,7 @@ config ARCH_LS1043A bool select ARMV8_SET_SMPEN select ARM_ERRATA_855873 if !TFABOOT - select FSL_IFC if TFABOOT || (!QSPI_BOOT && !SD_BOOT_QSPI) + select FSL_IFC if TFABOOT || (!QSPI_BOOT && !SD_BOOT_QSPI || !SD_BOOT) select FSL_LAYERSCAPE select FSL_LSCH2 select GICV2 @@ -98,7 +98,7 @@ config ARCH_LS1043A config ARCH_LS1046A bool select ARMV8_SET_SMPEN - select FSL_IFC if TFABOOT || (!QSPI_BOOT && !SD_BOOT_QSPI) + select FSL_IFC if TFABOOT || (!QSPI_BOOT && !SD_BOOT_QSPI && !SD_BOOT) select FSL_LAYERSCAPE select FSL_LSCH2 select GICV2 -- 2.35.1.1320.gc452695387.dirty
[PATCH 2/3] arm: layerscape: Disable unused parts of ICID tables
Several parts of the ICID table are only necessary for U-Boot proper.
Disable them in SPL. This saves around 500 bytes.
Signed-off-by: Sean Anderson
---
arch/arm/cpu/armv8/fsl-layerscape/icid.c | 2 ++
.../include/asm/arch-fsl-layerscape/fsl_icid.h | 16 ++--
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/arch/arm/cpu/armv8/fsl-layerscape/icid.c
b/arch/arm/cpu/armv8/fsl-layerscape/icid.c
index 25cd82f16eb..2d87281ec21 100644
--- a/arch/arm/cpu/armv8/fsl-layerscape/icid.c
+++ b/arch/arm/cpu/armv8/fsl-layerscape/icid.c
@@ -46,6 +46,7 @@ void set_icids(void)
#endif
}
+#ifndef CONFIG_SPL_BUILD
int fdt_set_iommu_prop(void *blob, int off, int smmu_ph, u32 *ids, int num_ids)
{
int i, ret;
@@ -190,3 +191,4 @@ void fdt_fixup_icid(void *blob)
fdt_fixup_fman_icids(blob, smmu_ph);
#endif
}
+#endif
diff --git a/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
b/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
index 3c06a55cb85..8af0d35d27b 100644
--- a/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
+++ b/arch/arm/include/asm/arch-fsl-layerscape/fsl_icid.h
@@ -12,11 +12,15 @@
#include
struct icid_id_table {
+#ifndef CONFIG_SPL_BUILD
const char *compat;
- u32 id;
- u32 reg;
phys_addr_t compat_addr;
+#endif
phys_addr_t reg_addr;
+ u32 reg;
+#ifndef CONFIG_SPL_BUILD
+ u32 id;
+#endif
bool le;
};
@@ -31,6 +35,13 @@ int fdt_set_iommu_prop(void *blob, int off, int smmu_ph, u32
*ids, int num_ids);
void set_icids(void);
void fdt_fixup_icid(void *blob);
+#ifdef CONFIG_SPL_BUILD
+#define SET_ICID_ENTRY(name, idA, regA, addr, compataddr, _le) \
+ { .reg = regA, \
+ .reg_addr = addr, \
+ .le = _le \
+ }
+#else
#define SET_ICID_ENTRY(name, idA, regA, addr, compataddr, _le) \
{ .compat = name, \
.id = idA, \
@@ -39,6 +50,7 @@ void fdt_fixup_icid(void *blob);
.reg_addr = addr, \
.le = _le \
}
+#endif
#ifdef CONFIG_SYS_FSL_SEC_LE
#define SEC_IS_LE true
--
2.35.1.1320.gc452695387.dirty
[PATCH 0/3] arm64: layerscape: Various small size reductions for SPL
This series groups a few small size reductions I found while trying to reduce my SPL size. They are all independent of each other. Sean Anderson (3): arm: layerscape: Don't select FSL_IFC when booting from SD card arm: layerscape: Disable unused parts of ICID tables arm: fsl: csu: Reduce size of ns_dev arch/arm/cpu/armv8/fsl-layerscape/Kconfig| 4 ++-- arch/arm/cpu/armv8/fsl-layerscape/icid.c | 2 ++ .../include/asm/arch-fsl-layerscape/fsl_icid.h | 16 ++-- include/fsl_csu.h| 4 ++-- 4 files changed, 20 insertions(+), 6 deletions(-) -- 2.35.1.1320.gc452695387.dirty
Re: [PATCH v3 4/4] test: cmd: add test for temperature command
On Tue, Sep 06, 2022 at 01:30:36PM +0200, Robert Marko wrote: > Add simple test for the temperature command. > > Signed-off-by: Robert Marko > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v3 3/4] thermal: add sandbox driver
On Tue, Sep 06, 2022 at 01:30:35PM +0200, Robert Marko wrote: > Provide a simple sandbox driver for the thermal uclass. > It simply registers and returns 100 degrees C if requested. > > Signed-off-by: Robert Marko > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v3 2/4] doc: cmd: temperature: add documentation
On Tue, Sep 06, 2022 at 01:30:34PM +0200, Robert Marko wrote: > Add documentation for the temperature command. > > Signed-off-by: Robert Marko > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v3 1/4] cmd: add temperature command
On Tue, Sep 06, 2022 at 01:30:33PM +0200, Robert Marko wrote: > Currently, there is no way for users to check the readings from thermal > sensors from U-boot console, only some boards print it during boot. > > So, lets add a simple "temperature" command that allows listing thermal > uclass devices and getting their value. > > Note that the thermal devices are intenionally probed if list is used as > almost always they will not get probed otherwise and there is no way for > users to manually call probe on a certain device from console. > > Assumption is made that temperature is returned in degrees C and not > milidegrees like in Linux as this is what most drivers seem to return. > > Signed-off-by: Robert Marko > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v2] cmd: xxd: add new command
On Sat, Sep 03, 2022 at 01:15:04PM +, Roger Knecht wrote: > Add xxd command to print file content as hexdump to standard out > > Reviewed-by: Simon Glass > Signed-off-by: Roger Knecht Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v6] cmd: cat: add new command
On Sat, Sep 03, 2022 at 11:34:53AM +, Roger Knecht wrote: > Add cat command to print file content to standard out > > Reviewed-by: Simon Glass > Signed-off-by: Roger Knecht Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] pinctrl: fix buffer size for pinctrl_generic_set_state_prefix()
On Wed, Sep 07, 2022 at 12:11:42PM +0100, John Keeping wrote:
> This buffer has the concatenated prefix and name written into it, so it
> must be large enough to cover both strings plus the terminating NUL.
>
> Fixes: 92c4a95ec7 ("pinctrl: Add new function
> pinctrl_generic_set_state_prefix()")
> Signed-off-by: John Keeping
> Reviewed-by: Pali Rohár
> Reviewed-by: Simon Glass
Applied to u-boot/master, thanks!
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v1] pinctrl: nuvoton: fix set persist error
On Tue, Sep 13, 2022 at 02:23:15PM +0800, Jim Liu wrote: > CA9C is cortex A9 watchdog reset control bit. > if device set persist mode, it shouldn't set this bit. > > Signed-off-by: Jim Liu Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH 2/2] boot: image-pre-load: Check environment for location of signature info
On Wed, Sep 14, 2022 at 08:57:28PM +0200, Steven Lawrance wrote: > Setting an alternative signature info node in "pre_load_sig_info_path" > allows verification of an image using the bootm pre-load mechanism with > a different key, e.g.: setenv pre_load_sig_info_path "/alt/sig" ; bootm > preload [addr] > > Signed-off-by: Steven Lawrance > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH 1/2] image-pre-load: Move macros/definitions to image.h
On Wed, Sep 14, 2022 at 08:57:27PM +0200, Steven Lawrance wrote: > Putting these definitions in a header will allow signatures to be > validated independently of bootm. > > Signed-off-by: Steven Lawrance > Reviewed-by: Simon Glass Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] bootcount: pmic: Correct misleading comment
On Mon, Sep 19, 2022 at 10:11:00AM +0200, Philip Oberfichtner wrote: > Fix a copy-paste error I did when inserting the comment. > > Signed-off-by: Philip Oberfichtner > Reviewed-by: Jaehoon Chung Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] lib: crypt: fix selecting a non-existent option
On Mon, Sep 19, 2022 at 12:14:49PM +0300, Oleksandr Suvorov wrote:
> The option SHA256_ALGO does not exist. Remove selecting it.
>
> Fixes: 26dd9936574 ("lib: add crypt subsystem")
> Signed-off-by: Oleksandr Suvorov
Applied to u-boot/master, thanks!
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v2] cmd: pxe: add alias devicetree-overlay for fdtoverlays
On Wed, Sep 21, 2022 at 03:26:33PM +0200, Edoardo Tomelleri wrote: > This adds keyword devicetree-overlay as an alias for fdtoverlays in > extlinux (sysboot) and pxe to better follow the Boot Loader Specification > [1], improves documentation around them by adding an example for both > fdtoverlays and devicetree-overlay and the environment variable required > for this feature. The link for the spec is updated to the current one. > > [1] https://systemd.io/BOOT_LOADER_SPECIFICATION/ > > Signed-off-by: Edoardo Tomelleri > Reviewed-by: Neil Armstrong > Reviewed-by: Ramon Fried Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH 4/5] binman: Use an exit code when blobs are missing
On Mon, Oct 10, 2022 at 04:25:36PM -0600, Simon Glass wrote: > Hi Tom, > > On Mon, 10 Oct 2022 at 14:41, Tom Rini wrote: > > > > On Mon, Oct 10, 2022 at 02:00:31PM -0600, Simon Glass wrote: > > > > > At present binman returns success when told to handle missing blobs. > > > This is confusing this in fact the resulting image cannot work. > > > > > > Use exit code 103 to signal this problem, with a -W option to convert > > > it to a warning. > > > > > > Signed-off-by: Simon Glass > > > > I'm still not sure I like this, rather than changing the default "make" > > behavior. > > I did change that, in the sense that 'make' now fails if there are > missing blobs. > > > And then it gets me a bit worried that we have CI not doing > > some other things "right" as we don't want to ignore warnings in CI, we > > want warnings to become errors so that new warnings don't make it > > in-tree. > > That would be quite a big effort, and unrelated to this series. Here > are some warning types I'm aware of: > > - migration > - device tree > - compiler > - missing blobs > > Do we need a per-board whitelist for warnings? It seems pretty tricky to me. > > It is true that warnings are ignored in CI and this does create > problems...I'd love to make them into errors if we can. This is I guess also a related concern. When I say warnings, I mean C compiler warnings. That we have flags to suppress "warnings" that are other kinds of valid warnings is at least a little confusing. -- Tom signature.asc Description: PGP signature
[PATCH] ARM: dts: stm32: Fix and expand PLL configuration comments
Fix the frequencies listed in PLL configuration comments to match
the actual frequencies programmed into hardware. Furthermore, add
a comment which explains how those frequencies are calculated, so
it won't be necessary to look it up all over the datasheet and
make more mistakes in the calculation in the future.
Signed-off-by: Marek Vasut
Cc: Patrice Chotard
Cc: Patrick Delaunay
---
arch/arm/dts/stm32mp15xx-dhcom-u-boot.dtsi | 17 -
arch/arm/dts/stm32mp15xx-dhcor-u-boot.dtsi | 17 -
2 files changed, 32 insertions(+), 2 deletions(-)
diff --git a/arch/arm/dts/stm32mp15xx-dhcom-u-boot.dtsi
b/arch/arm/dts/stm32mp15xx-dhcom-u-boot.dtsi
index 8a7156c93bf..b72a2f63f16 100644
--- a/arch/arm/dts/stm32mp15xx-dhcom-u-boot.dtsi
+++ b/arch/arm/dts/stm32mp15xx-dhcom-u-boot.dtsi
@@ -190,6 +190,21 @@
CLK_LPTIM45_LSE
>;
+ /*
+* cfg = < DIVM1 DIVN P Q R PQR(p,q,r) >;
+* frac = < f >;
+*
+* PRQ(p,q,r) ... for p,q,r: 0-output disabled / 1-output enabled
+* DIVN ... actually multiplier, but RCC_PLL1CFGR1 calls the field DIVN
+* m ... for PLL1,2: m=2 ; for PLL3,4: m=1
+* XTAL = 24 MHz
+*
+* VCO = ( XTAL / (DIVM1 + 1) ) * m * ( DIVN + 1 + ( f / 8192 ) )
+* P = VCO / (P + 1)
+* Q = VCO / (Q + 1)
+* R = VCO / (R + 1)
+*/
+
/* VCO = 1066.0 MHz => P = 266 (AXI), Q = 533 (GPU), R = 533 (DDR) */
pll2: st,pll@1 {
compatible = "st,stm32mp1-pll";
@@ -208,7 +223,7 @@
u-boot,dm-pre-reloc;
};
- /* VCO = 600.0 MHz => P = 50, Q = 50, R = 50 */
+ /* VCO = 600.0 MHz => P = 100, Q = 50, R = 50 */
pll4: st,pll@3 {
compatible = "st,stm32mp1-pll";
reg = <3>;
diff --git a/arch/arm/dts/stm32mp15xx-dhcor-u-boot.dtsi
b/arch/arm/dts/stm32mp15xx-dhcor-u-boot.dtsi
index 19f4221f876..25a288b0475 100644
--- a/arch/arm/dts/stm32mp15xx-dhcor-u-boot.dtsi
+++ b/arch/arm/dts/stm32mp15xx-dhcor-u-boot.dtsi
@@ -144,6 +144,21 @@
CLK_LPTIM45_LSE
>;
+ /*
+* cfg = < DIVM1 DIVN P Q R PQR(p,q,r) >;
+* frac = < f >;
+*
+* PRQ(p,q,r) ... for p,q,r: 0-output disabled / 1-output enabled
+* DIVN ... actually multiplier, but RCC_PLL1CFGR1 calls the field DIVN
+* m ... for PLL1,2: m=2 ; for PLL3,4: m=1
+* XTAL = 24 MHz
+*
+* VCO = ( XTAL / (DIVM1 + 1) ) * m * ( DIVN + 1 + ( f / 8192 ) )
+* P = VCO / (P + 1)
+* Q = VCO / (Q + 1)
+* R = VCO / (R + 1)
+*/
+
/* VCO = 1066.0 MHz => P = 266 (AXI), Q = 533 (GPU), R = 533 (DDR) */
pll2: st,pll@1 {
compatible = "st,stm32mp1-pll";
@@ -162,7 +177,7 @@
u-boot,dm-pre-reloc;
};
- /* VCO = 600.0 MHz => P = 99, Q = 74, R = 99 */
+ /* VCO = 594.0 MHz => P = 99, Q = 74, R = 99 */
pll4: st,pll@3 {
compatible = "st,stm32mp1-pll";
reg = <3>;
--
2.35.1
Re: [PATCH] avb: Extend support to non-eMMC interfaces
On Mon, Oct 10, 2022 at 7:53 AM Mattijs Korpershoek
wrote:
>
> Hi Alistair,
>
> Thank you for your patch.
>
> On lun., sept. 26, 2022 at 22:02, Alistair Delva wrote:
>
> > From: Jiyong Park
> >
> > Previously Android AVB supported block devices only on eMMC. This change
> > eliminates the restriction by using the generic block driver model.
> >
> > The `avb init' command is modified to accept another parameter which
> > specifies the interface type. e.g., `avb init virtio 0' initializes
> > avb for the first (0) disk that is accessible via the virtio interface.
> >
> > [adelva: The "avb init" command is updated directly, as this is
> > considered a "debug command" that can't be usefully used in u-boot
> > scripts.]
>
> It seems that:
> * include/configs/ti_omap5_common.h
> * include/configs/meson64_android.h
> * configs/total_compute_defconfig
>
> All call "avb init"
>
> Aren't we breaking these boot scripts with this change?
>
> Since all of them used mmc before, it should be trivial to patch these
> environments as well.
> If you do so, please cc me in next version so I can give this a try on
> the khadas vim3l board.
Sure, I'll do that and send a v2.
> Maybe those devices are doing the wrong thing though. since this is
> considered a debug command I imagine we should not be calling this at
> all.
> If so, do you have any alternatives in mind?
Yes, sorry. My rationale was confusing. We have more patches to
upstream which will explain the reasoning better:
"data from boot and vendor_boot partitions are loaded only once for
the verification. After the verification is done, the read data is
directly copied to the load addresses instead of doing the I/O again
from the disk. This is to prevent a TOCTOU issue where attacker provides
different data for verification and actual booting."
So yes, what these env files do for now isn't safe, but there isn't a
good upstream alternative. Anyway, this problem isn't related to what
this patch is doing. So, I should update them for now.
> >
> > Signed-off-by: Alistair Delva
> > Cc: Igor Opaniuk
> > Cc: Ram Muthiah
> > Cc: Jiyong Park
> > Cc: Simon Glass
> > ---
> > cmd/avb.c| 16 ---
> > common/Kconfig | 1 -
> > common/avb_verify.c | 105 +--
> > include/avb_verify.h | 31 -
>
> Should we also update the documentation in doc/android/avb2.rst ?
Will do.
> I also think this might break:
> test/py/tests/test_android/test_avb.py
I'll update it.
> > 4 files changed, 69 insertions(+), 84 deletions(-)
> >
> > diff --git a/cmd/avb.c b/cmd/avb.c
> > index 783f51b816..8bffe49011 100644
> > --- a/cmd/avb.c
> > +++ b/cmd/avb.c
> > @@ -10,24 +10,25 @@
> > #include
> > #include
> > #include
> > -#include
> >
> > #define AVB_BOOTARGS "avb_bootargs"
> > static struct AvbOps *avb_ops;
> >
> > int do_avb_init(struct cmd_tbl *cmdtp, int flag, int argc, char *const
> > argv[])
> > {
> > - unsigned long mmc_dev;
> > + const char *iface;
> > + const char *devnum;
> >
> > - if (argc != 2)
> > + if (argc != 3)
> > return CMD_RET_USAGE;
> >
> > - mmc_dev = hextoul(argv[1], NULL);
> > + iface = argv[1];
> > + devnum = argv[2];
> >
> > if (avb_ops)
> > avb_ops_free(avb_ops);
> >
> > - avb_ops = avb_ops_alloc(mmc_dev);
> > + avb_ops = avb_ops_alloc(iface, devnum);
> > if (avb_ops)
> > return CMD_RET_SUCCESS;
> >
> > @@ -419,7 +420,7 @@ int do_avb_write_pvalue(struct cmd_tbl *cmdtp, int
> > flag, int argc,
> > }
> >
> > static struct cmd_tbl cmd_avb[] = {
> > - U_BOOT_CMD_MKENT(init, 2, 0, do_avb_init, "", ""),
> > + U_BOOT_CMD_MKENT(init, 3, 0, do_avb_init, "", ""),
> > U_BOOT_CMD_MKENT(read_rb, 2, 0, do_avb_read_rb, "", ""),
> > U_BOOT_CMD_MKENT(write_rb, 3, 0, do_avb_write_rb, "", ""),
> > U_BOOT_CMD_MKENT(is_unlocked, 1, 0, do_avb_is_unlocked, "", ""),
> > @@ -455,7 +456,8 @@ static int do_avb(struct cmd_tbl *cmdtp, int flag, int
> > argc, char *const argv[])
> > U_BOOT_CMD(
> > avb, 29, 0, do_avb,
> > "Provides commands for testing Android Verified Boot 2.0
> > functionality",
> > - "init - initialize avb2 for \n"
> > + "init - initialize avb2 for the disk
> > which\n"
> > + "is on the interface \n"
> > "avb read_rb - read rollback index at location \n"
> > "avb write_rb - write rollback index to \n"
> > "avb is_unlocked - returns unlock status of the device\n"
> > diff --git a/common/Kconfig b/common/Kconfig
> > index ebee856e56..a66060767c 100644
> > --- a/common/Kconfig
> > +++ b/common/Kconfig
> > @@ -703,7 +703,6 @@ config HASH
> > config AVB_VERIFY
> > bool "Build Android Verified Boot operations"
> > depends on LIBAVB
> > - depends on MMC
> > depends on PARTITION_UUIDS
> > help
> > This option enables compilation of bootloader-dependent operations,
> > diff --git
Re: [PATCH 1/1] dm: fix blk_get_devnum_by_uclass_idname()
On 10/11/22 16:16, Simon Glass wrote:
Hi Heinrich,
On Tue, 11 Oct 2022 at 04:38, Heinrich Schuchardt
wrote:
On 10/11/22 07:46, Heinrich Schuchardt wrote:
On 10/11/22 01:49, Simon Glass wrote:
Hi Heinrich,
On Thu, 6 Oct 2022 at 14:05, Heinrich Schuchardt
wrote:
On 10/3/22 18:44, Simon Glass wrote:
Hi Heinrich,
On Mon, 3 Oct 2022 at 10:33, Heinrich Schuchardt
wrote:
On 10/3/22 16:57, Simon Glass wrote:
Hi Heinrich,
On Mon, 3 Oct 2022 at 03:36, Heinrich Schuchardt
wrote:
On the sandbox I run:
=> setenv efi_selftest block device
=> bootefi selftest
and see the following output:
** Bad device specification host 0 **
Couldn't find partition host 0:0
Cannot read EFI system partition
Running
=> lsblk
yields
Block Driver Devices
-
efi_blk : efiloader 0
ide_blk :
mmc_blk : mmc 2, mmc 1, mmc 0
nvme-blk:
sandbox_host_blk:
scsi_blk:
usb_storage_blk :
virtio-blk :
So a efi_blk device was mistaken for a host device.
I continue with
=> host bind 0 ../sandbox.img
=> ls host 0:1
and get the following output:
13 hello.txt
7 u-boot.txt
2 file(s), 0 dir(s)
This is the content of efiblock 0:1 and not of host 0:1 (sic!).
The uclass of the parent device is irrelevant for the
determination of the
uclass of the block device. We must use the uclass stored in the
block
device descriptor.
This issue has been raised repeatedly:
[PATCH 1/1] block: fix blk_get_devnum_by_typename()
https://lore.kernel.org/u-boot/20220802094933.69170-1-heinrich.schucha...@canonical.com/
[PATCH 1/1] blk: simplify blk_get_devnum_by_typename()
https://lore.kernel.org/u-boot/20211023140647.7661-1-heinrich.schucha...@canonical.com/
Yes and you were not able/willing to take on the required work, so
this carried on longer than it should have. I finally did this myself
and it is now in -next.
The refactoring was orthogonal to the problem that I reported and
which
you unfortunately did not consider in the process.
Well it involved using if_type to work around a problem, just making
it harder to get rid of. Overall I am in favour of a faster pace of
migration that we have been following and it would help if people took
on some of this, instead of fixing their little issue.
So we might finally be able to fix this problem properly, since
if_type is mostly just a work-around concept in -next, with just the
fake uclass_id being used at present.
Can you use if_type_to_uclass_id() here, which is the work-around
function for now?
This function does not exist in origin/next. We won't apply this patch
in the 2022-10 cycle.
I think I mean conv_uclass_id() which is the new name.
Let's fix the bug first before thinking about future refactoring.
You may determine the uclass ID for field bdev in struct blk_desc
using
function device_get_uclass_id() when refactoring.
So if you call conv_uclass_id() (without any other refactoring) does
that fix the problem?
Except for UCLASS_USB that function is a NOP. How could it help to
differentiate between devices with the same parent device?
It can't. But the root node should not have UCLASS_BLK children. I
think I mentioned that a few months back?
Would you agree that blk_get_devnum_by_uclass_idname() should not look
at the parent but on the actual device?
No, looking at the parent is exactly what it should do. A block device
is generic, to the extent possible. Its methods are implemented in the
parent uclass and are tightly bound to it. See for example
U_BOOT_DRIVER(mmc_blk) in the MMC uclass.
Let's look at an MMC device
root_driver/soc/mmc@1c0f000/m...@1c0f000.blk is a block device.
What do we need to find out that it can be addressed as mmc 0? The
driver is mmc_blk and its index is 0. We don't need any information
about the parent device at all.
If blk is the MMC block device, the fact that is mmc 0 is determined
by dev_seq(dev_get_parent(blk)). We are not parsing strings to find
that out. It is part of the design.
Unfortunately this confusion is my fault since I used the root device
for the sandbox block devices. That was a convenience and a way to
reduce somewhat the crushing load of driver model migration. But the
time for that convenience is gone and we should create a sandbox host
parent node for the sandbox block devices and tidy up EFI too.
The only confusion is in the current blk_get_devnum_by_uclass_idname()
code looking into the parent device.
The parent device is totally irrelevant here. Stop using it.
See below.
You already noted when writing conv_uclass_id() that using the uclass
name does not work properly to find out the CLI name of a devie.
Can we put the CLI name for device types ("mmc", "scsi" ...) into struct
blk_ops? Then
[PATCH v2] rtc: Add fallbacks for dm functions
This adds fallbacks for the various dm_rtc_* functions. This allows
common code to use these functions without ifdefs.
Fixes: c8ce7ba87d1 ("misc: Add support for nvmem cells")
Reviewed-by: Simon Glass
Signed-off-by: Sean Anderson
---
Changes in v2:
- Include linux/errno.h for ENOSYS. This is needed for some mips boards
when CONFIG_BLK is enabled for whatever reason.
include/rtc.h | 32 +---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/include/rtc.h b/include/rtc.h
index 10104e3bf5a..b6fdbb60dc2 100644
--- a/include/rtc.h
+++ b/include/rtc.h
@@ -15,13 +15,12 @@
#include
#include
+#include
typedef int64_t time64_t;
-
-#ifdef CONFIG_DM_RTC
-
struct udevice;
+#if CONFIG_IS_ENABLED(DM_RTC)
struct rtc_ops {
/**
* get() - get the current time
@@ -222,6 +221,33 @@ int rtc_enable_32khz_output(int busnum, int chip_addr);
#endif
#else
+static inline int dm_rtc_get(struct udevice *dev, struct rtc_time *time)
+{
+ return -ENOSYS;
+}
+
+static inline int dm_rtc_set(struct udevice *dev, struct rtc_time *time)
+{
+ return -ENOSYS;
+}
+
+static inline int dm_rtc_reset(struct udevice *dev)
+{
+ return -ENOSYS;
+}
+
+static inline int dm_rtc_read(struct udevice *dev, unsigned int reg, u8 *buf,
+ unsigned int len)
+{
+ return -ENOSYS;
+}
+
+static inline int dm_rtc_write(struct udevice *dev, unsigned int reg,
+ const u8 *buf, unsigned int len)
+{
+ return -ENOSYS;
+}
+
int rtc_get (struct rtc_time *);
int rtc_set (struct rtc_time *);
void rtc_reset (void);
--
2.35.1.1320.gc452695387.dirty
Re: [PATCH] rtc: Add fallbacks for dm functions
On Fri, Sep 23, 2022 at 02:30:16PM -0400, Sean Anderson wrote:
> This adds fallbacks for the various dm_rtc_* functions. This allows
> common code to use these functions without ifdefs.
>
> Fixes: c8ce7ba87d1 ("misc: Add support for nvmem cells")
> Signed-off-by: Sean Anderson
> Reviewed-by: Simon Glass
This breaks a number of platforms such as malta64el:
+include/rtc.h: In function 'dm_rtc_get':
+include/rtc.h:225:17: error: 'ENOSYS' undeclared (first use in this function)
+ 225 | return -ENOSYS;
+ | ^~
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v2 2/2] mach-meson: g12a: reset usb controller in reset_misc()
On 11/10/2022 18:36, Mattijs Korpershoek wrote:
On Tue, Oct 11, 2022 at 17:39, Marek Vasut wrote:
On 10/11/22 09:12, neil.armstr...@linaro.org wrote:
On 10/10/2022 19:16, Marek Vasut wrote:
On 10/10/22 18:22, Neil Armstrong wrote:
Hi,
Hi,
On 10/10/2022 18:09, Marek Vasut wrote:
On 10/7/22 11:38, Mattijs Korpershoek wrote:
On some g12a boards like the VIM3L and the SEI610, with some
USB cables/hosts, there is a long (5s) delay before
between "fastboot reboot" and the host detecting a USB reset.
This breaks tools relying on "fastboot reboot fastboot" which assume
that 1s after the command send, the board should disconnect on usb.
To reproduce, enable fastboot in U-Boot console:
=> fastboot usb 0
Then, on the host, run:
# echo "running fastboot reboot bootloader" > /dev/kmsg &&
fastboot reboot bootloader
Rebooting into bootloader OKAY [ 0.003s]
Finished. Total time: 3.033s
[54074.251551] running fastboot reboot bootloader
... there is a delay of 5s before we detect a disconnection ...
[54079.041238] usb 1-7.4: USB disconnect, device number 72
[54079.239625] usb 1-7.4: new high-speed USB device number 73
using xhci_hcd
[54079.359103] usb 1-7.4: New USB device found, idVendor=1b8e,
idProduct=fada, bcdDevice= 2.27
[54079.359110] usb 1-7.4: New USB device strings: Mfr=1,
Product=2, SerialNumber=3
[54079.359112] usb 1-7.4: Product: USB download gadget
[54079.359114] usb 1-7.4: Manufacturer: U-Boot
[54079.359116] usb 1-7.4: SerialNumber: C8631470CC41
Note: this does not happen when we use the RST button on the board.
To fix this, re-implement a platform reset which calls
board_usb_cleanup() before resetting the board.
Shouldn't that call happen somewhere in drivers/usb/ .remove()
callback instead ?
No since dwc2 isn't DM yet, handling is done in arch/arm/mac-meson
board_usb_*() for now
Seems DWC2 is DM:
$ git grep U_BOOT_DRIVER drivers/usb/ | grep dwc2
drivers/usb/gadget/dwc2_udc_otg.c:U_BOOT_DRIVER(dwc2_udc_otg) = {
drivers/usb/host/dwc2.c:U_BOOT_DRIVER(usb_dwc2) = {
My bad, seems I missed the dwc2 otg DM wagon...
We will need to switch to this now then, thanks,
Thanks. I picked 1/2 at least, so you can only focus on 2/2 and send
just that.
Yes, thank you for picking 1/2
Neil, do you want me to look into converting the meson arch which uses
DWC2 to DM?
I'm not sure I'm confident to do these changes properly but I'm willing
to give it a try.
If you plan to do it yourself, fine by me. I'll respin 2/2 after that.
Sure, give a try, if you don't manage I can help!
Neil
Re: [PATCH v2 2/2] mach-meson: g12a: reset usb controller in reset_misc()
On Tue, Oct 11, 2022 at 17:39, Marek Vasut wrote:
> On 10/11/22 09:12, neil.armstr...@linaro.org wrote:
>> On 10/10/2022 19:16, Marek Vasut wrote:
>>> On 10/10/22 18:22, Neil Armstrong wrote:
Hi,
>>>
>>> Hi,
>>>
On 10/10/2022 18:09, Marek Vasut wrote:
> On 10/7/22 11:38, Mattijs Korpershoek wrote:
>> On some g12a boards like the VIM3L and the SEI610, with some
>> USB cables/hosts, there is a long (5s) delay before
>> between "fastboot reboot" and the host detecting a USB reset.
>>
>> This breaks tools relying on "fastboot reboot fastboot" which assume
>> that 1s after the command send, the board should disconnect on usb.
>>
>> To reproduce, enable fastboot in U-Boot console:
>> => fastboot usb 0
>>
>> Then, on the host, run:
>> # echo "running fastboot reboot bootloader" > /dev/kmsg &&
>> fastboot reboot bootloader
>> Rebooting into bootloader OKAY [ 0.003s]
>> Finished. Total time: 3.033s
>>
>> [54074.251551] running fastboot reboot bootloader
>> ... there is a delay of 5s before we detect a disconnection ...
>> [54079.041238] usb 1-7.4: USB disconnect, device number 72
>> [54079.239625] usb 1-7.4: new high-speed USB device number 73
>> using xhci_hcd
>> [54079.359103] usb 1-7.4: New USB device found, idVendor=1b8e,
>> idProduct=fada, bcdDevice= 2.27
>> [54079.359110] usb 1-7.4: New USB device strings: Mfr=1,
>> Product=2, SerialNumber=3
>> [54079.359112] usb 1-7.4: Product: USB download gadget
>> [54079.359114] usb 1-7.4: Manufacturer: U-Boot
>> [54079.359116] usb 1-7.4: SerialNumber: C8631470CC41
>>
>> Note: this does not happen when we use the RST button on the board.
>>
>> To fix this, re-implement a platform reset which calls
>> board_usb_cleanup() before resetting the board.
>
> Shouldn't that call happen somewhere in drivers/usb/ .remove()
> callback instead ?
No since dwc2 isn't DM yet, handling is done in arch/arm/mac-meson
board_usb_*() for now
>>>
>>> Seems DWC2 is DM:
>>>
>>> $ git grep U_BOOT_DRIVER drivers/usb/ | grep dwc2
>>> drivers/usb/gadget/dwc2_udc_otg.c:U_BOOT_DRIVER(dwc2_udc_otg) = {
>>> drivers/usb/host/dwc2.c:U_BOOT_DRIVER(usb_dwc2) = {
>>>
>>
>> My bad, seems I missed the dwc2 otg DM wagon...
>>
>> We will need to switch to this now then, thanks,
>
> Thanks. I picked 1/2 at least, so you can only focus on 2/2 and send
> just that.
Yes, thank you for picking 1/2
Neil, do you want me to look into converting the meson arch which uses
DWC2 to DM?
I'm not sure I'm confident to do these changes properly but I'm willing
to give it a try.
If you plan to do it yourself, fine by me. I'll respin 2/2 after that.
Re: [PATCH 2/2] buildman: Add --allow-missing-binaries flag to build with BINMAN_ALLOW_MISSING=1
On Mon, Oct 10, 2022 at 05:48:55PM -0600, Simon Glass wrote:
> Hi Tom,
>
> On Mon, 10 Oct 2022 at 09:18, Tom Rini wrote:
> >
> > Add a new flag to buildman so that we will in turn pass
> > BINMAN_ALLOW_MISSING=1 to 'make'. Make use of this flag in CI.
> >
> > Cc: Rasmus Villemoes
> > Cc: Simon Glass
> > Signed-off-by: Tom Rini
> > ---
> > .azure-pipelines.yml| 2 +-
> > .gitlab-ci.yml | 6 +++---
> > tools/buildman/builder.py | 5 -
> > tools/buildman/builderthread.py | 2 ++
> > tools/buildman/cmdline.py | 3 +++
> > tools/buildman/control.py | 3 ++-
> > 6 files changed, 15 insertions(+), 6 deletions(-)
> >
> > diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml
> > index f200b40dbb24..c932c2b3c619 100644
> > --- a/.azure-pipelines.yml
> > +++ b/.azure-pipelines.yml
> > @@ -553,7 +553,7 @@ stages:
> >cat << "EOF" >> build.sh
> >if [[ "${BUILDMAN}" != "" ]]; then
> >ret=0;
> > - tools/buildman/buildman -o /tmp -P -E -W ${BUILDMAN}
> > ${OVERRIDE} || ret=$?;
> > + tools/buildman/buildman -o /tmp -P -E -W
> > --allow-missing-binaries ${BUILDMAN} ${OVERRIDE} || ret=$?;
>
> This is fine for CI.
I think one of the issues here is we need to agree on the common use
cases. I don't think most people use buildman to build, they use make.
Of the people that do use buildman, how many aren't already using a
wrapper script? I know I always do. Can we also not just deal with
setting this flag in ~/.buildman ? Would it Just Work as:
[builder]
allow-missing-binaries = True
Or is more code needed?
> But having to provide a flag to do build testing seems like the tail
> is wagging the dog. Boards should be discouraged to use blobs and we
> don't want to make it hard for everyone else (who doesn't have the
> blobs) to test whether their patch breaks a build.
I'm not sure this ends up being a common case. If someone is build
testing for something they can't run test, the error is going to be
after whatever they compile-tested was compiled/linked, and if they're
testing everything it's via CI.
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH 0/3] clang-14 fixes
On Tue, Oct 11, 2022 at 08:14:53AM -0600, Simon Glass wrote: > Hi Tom, > > On Sat, 8 Oct 2022 at 21:33, Simon Glass wrote: > > > > This series contains a few clang-14 fixes to partly resolve the problems > > shown by [1]. > > > > The event_dump problem is that the symbol name is not printed by > > addr2line even though it seems to be present: > > > > Without LTO: > > > >$ addr2line -e u-boot.clang14.no-lto 4a37f > >addr2line: DWARF error: invalid or unhandled FORM value: 0x23 > >vbe_simple.c:? > > > >$ nm u-boot.clang14.no-lto |grep 4a37f > >0004a37f t bootmeth_vbe_simple_ft_fixup > > > > With LTO is even worse: > > > >$ addr2line -e u-boot.clang14.lto 2d2ee0 > >addr2line: DWARF error: invalid or unhandled FORM value: 0x23 > >??:0 > > > >$ nm u-boot.clang14.lto |grep 4dad1 > >0004dad1 t bootmeth_vbe_simple_ft_fixup > > > > Perhaps this is a bug in addr2line or the clang DWARF output? > > > > [1] https://source.denx.de/u-boot/u-boot/-/jobs/510282#L287 > > I should have mentioned that I can respin the series to deal with > this, perhaps by not requiring the function name to be present when > clang is used (or just at all?) > > Plus I have one more patch to add for the LTO stuff. > > Let me know what you think and I can respin this this. Maybe it's worth raising a question on one of the LLVM lists? This seems fairly odd. I would like to move to LLVM-14 / gcc-12.2 for CI, for v2023.01, but there's still other warnings to be fixed too, so we have a little time yet. -- Tom signature.asc Description: PGP signature
Re: [GIT PULL] xilinx patches for v2023.01-rc1 (round 3)
On Tue, Oct 11, 2022 at 01:16:05PM +0200, Michal Simek wrote: > Hi Tom, > > please pull the following patches to your tree. Based on discussion with > Simon I have also include fpga uclass series which was reviewed by him. > There is also one gcc12 patch we discussed over IRC which was fixed by > Heinrich. > > Thanks, > Michal > > The following changes since commit 2d4591353452638132d711551fec3495b7644731: > > Merge branch 'next' (2022-10-03 15:39:46 -0400) > > are available in the Git repository at: > > g...@source.denx.de:u-boot/custodians/u-boot-microblaze.git > tags/xilinx-for-v2023.01-rc1-v3 > > for you to fetch changes up to 63c46e028c14254f28332b3bd57fc3202e26b10a: > > fpga: virtex2: Use logging feature instead of FPGA_DEBUG (2022-10-10 > 12:28:08 +0200) > Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] fastboot: mmc: switch to user hwpart after erase/flash
On 10/11/22 8:07 AM, Mattijs Korpershoek wrote:
> Hi Sean,
>
> Thank you for your review.
>
> On Mon, Oct 10, 2022 at 11:38, Sean Anderson wrote:
>
>> On 10/10/22 4:05 AM, Mattijs Korpershoek wrote:
>>> Before erasing (or flashing) an mmc hardware partition, such as
>>> mmc0boot0 or mmc0boot1, we select it using blk_dselect_hwpart().
>>>
>>> However, we don't select back the normal (userdata) hwpartition
>>> afterwards.
>>>
>>> For example, the following sequence is broken:
>>>
>>> $ fastboot erase mmc2boot1# switch to hwpart 1
>>> $ fastboot reboot bootloader # attempts to read GPT from hwpart 1
>>>
writing 128 blocks starting at 8064...
wrote 65536 bytes to 'mmc0boot1'
GUID Partition Table Header signature is wrong: 0xFB4EC30FC5B7E5B2 !=
0x5452415020494645
find_valid_gpt: *** ERROR: Invalid GPT ***
GUID Partition Table Header signature is wrong: 0x0 != 0x5452415020494645
find_valid_gpt: *** ERROR: Invalid Backup GPT ***
Error: mmc 2:misc read failed (-2)
>>>
>>> The GPT is stored in hwpart0 (userdata), not hwpart1 (mmc0boot1)
>>
>> This seems like a problem with your boot script. You should run `mmc dev
>> ${mmcdev}` (and `mmc rescan`) before trying to access any MMC
>> partitions. This is especially important after fastbooting, since the
>> partition layout (or active hardware partition) may have changed.
>
> I don't think I can fix this in my boot script.
> My boot script runs => fastboot usb 0 which will start a loop to receive
> fastboot commands from the host.
>
> The full u-boot env i'm using is in include/configs/meson64_android.h
>
> This loop will go on until "fastboot reboot " is called.
>
> I do can "work around" this by using the following fastboot sequence:
> $ fastboot erase mmc2boot1
> $ fastboot oem format # switch backs to hwpart 0 (USER)
> $ fastboot reboot bootloader
>
> But that's not a good option to me. From a user's perspective, it should
> not matter in which order we issue fastboot commands to the board.
>
> Maybe I should fix bcb.c instead to make sure that it selects hwpart0
> before reading/writing into the bcb partition ?
Yes. The bcb command should be using something like
part_get_info_by_dev_and_name_or_num instead of using its own bespoke
parsing. Unfortunately, command syntax is part of the API, but at the
very least you can fall back to something sane if argv[2] ("devnum")
doesn't parse as an int (aka it's the ifname).
--Sean
>>
>>> As the blk documentation states:
Once selected only the region of the device
covered by that partition is accessible.
>>>
>>> Add a cleanup function, fastboot_mmc_select_default_hwpart() which
>>> selects back the default hwpartition (userdata).
>>>
>>> Note: this is more visible since
>>> commit a362ce214f03 ("fastboot: Implement generic fastboot_set_reboot_flag")
>>> because "fastboot reboot bootloader" will access the "misc" partition.
>>>
>>> Signed-off-by: Mattijs Korpershoek
>>> ---
>>> This fix has been used for over a year in the AOSP tree at [1]
>>>
>>> [1]
>>> https://android.googlesource.com/device/amlogic/yukawa/+/c008ddaec0943adda394b229e83e147f6165773c
>>> ---
>>> drivers/fastboot/fb_mmc.c | 20 +---
>>> include/mmc.h | 1 +
>>> 2 files changed, 18 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c
>>> index 033c510bc096..dedef7c4deb1 100644
>>> --- a/drivers/fastboot/fb_mmc.c
>>> +++ b/drivers/fastboot/fb_mmc.c
>>> @@ -199,6 +199,12 @@ static void write_raw_image(struct blk_desc *dev_desc,
>>> fastboot_okay(NULL, response);
>>> }
>>>
>>> +static void fastboot_mmc_select_default_hwpart(struct blk_desc *dev_desc)
>>> +{
>>> + if (blk_dselect_hwpart(dev_desc, MMC_PART_USERDATA))
>>
>> If you really want to do this, we shoud save the current partition and
>> restore if after fastbooting. Always switching to hardware partition 0
>> is just as broken as the current behavior.
>
> ACK. This patch just replaces one broken behaviour by another one. I
> will see if I can save the hardware partition state instead or patch the
> bcb command.
>
>
>>
>> --Sean
>>
>
[PATCH v2 14/14] vbe: Add a test for VBE device tree fixups
When a FIT includes some OS requests, U-Boot should process these and add
the requested info to corresponding subnodes of the /chosen node. Add a
pytest for this, which sets up the FIT, runs bootm and then uses a C
unit test to check that everything looks OK.
The test needs to run on sandbox_flattree since we don't support
device tree fixups on sandbox (live tree) yet. So enable BOOTMETH_VBE and
disable bootflow_system(), since EFI is not supported on
sandbox_flattree.
Add a link to the initial documentation.
Signed-off-by: Simon Glass
---
(no changes since v1)
configs/sandbox_flattree_defconfig | 2 +-
doc/develop/vbe.rst| 3 +-
test/boot/Makefile | 1 +
test/boot/bootflow.c | 2 +
test/boot/vbe_fixup.c | 59 ++
test/py/tests/fit_util.py | 5 +-
test/py/tests/test_vbe.py | 123 +
7 files changed, 192 insertions(+), 3 deletions(-)
create mode 100644 test/boot/vbe_fixup.c
create mode 100644 test/py/tests/test_vbe.py
diff --git a/configs/sandbox_flattree_defconfig
b/configs/sandbox_flattree_defconfig
index a2eb7afcf9b..0d8e3132320 100644
--- a/configs/sandbox_flattree_defconfig
+++ b/configs/sandbox_flattree_defconfig
@@ -11,7 +11,6 @@ CONFIG_DISTRO_DEFAULTS=y
CONFIG_FIT=y
CONFIG_FIT_SIGNATURE=y
CONFIG_FIT_VERBOSE=y
-# CONFIG_BOOTMETH_VBE is not set
CONFIG_BOOTSTAGE=y
CONFIG_BOOTSTAGE_REPORT=y
CONFIG_BOOTSTAGE_FDT=y
@@ -83,6 +82,7 @@ CONFIG_REGMAP=y
CONFIG_SYSCON=y
CONFIG_DEVRES=y
CONFIG_DEBUG_DEVRES=y
+CONFIG_OFNODE_MULTI_TREE=y
CONFIG_ADC=y
CONFIG_ADC_SANDBOX=y
CONFIG_AXI=y
diff --git a/doc/develop/vbe.rst b/doc/develop/vbe.rst
index 8f147fd9360..cca193c8fd4 100644
--- a/doc/develop/vbe.rst
+++ b/doc/develop/vbe.rst
@@ -19,8 +19,9 @@ listing methods and getting the status for a method.
For a detailed overview of VBE, see vbe-intro_. A fuller description of
bootflows is at vbe-bootflows_ and the firmware-update mechanism is described
at
-vbe-fwupdate_.
+vbe-fwupdate_. VBE OS requests are described at vbe-osrequests_.
.. _vbe-intro:
https://docs.google.com/document/d/e/2PACX-1vQjXLPWMIyVktaTMf8edHZYDrEvMYD_iNzIj1FgPmKF37fpglAC47Tt5cvPBC5fvTdoK-GA5Zv1wifo/pub
.. _vbe-bootflows:
https://docs.google.com/document/d/e/2PACX-1vR0OzhuyRJQ8kdeOibS3xB1rVFy3J4M_QKTM5-3vPIBNcdvR0W8EXu9ymG-yWfqthzWoM4JUNhqwydN/pub
.. _vbe-fwupdate:
https://docs.google.com/document/d/e/2PACX-1vTnlIL17vVbl6TVoTHWYMED0bme7oHHNk-g5VGxblbPiKIdGDALE1HKId8Go5f0g1eziLsv4h9bocbk/pub
+.. _vbe-osrequests:
https://docs.google.com/document/d/e/2PACX-1vTHhxX7WSZe68i9rAkW-DHdx6koU-jxYHhamLhZn9GQ9QT2_epSBosMV1_r7yPHOXZccx71rF_t0PXL/pub
diff --git a/test/boot/Makefile b/test/boot/Makefile
index 9e9d5ae21f3..5bb3f889759 100644
--- a/test/boot/Makefile
+++ b/test/boot/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_BOOTSTD) += bootdev.o bootstd_common.o bootflow.o
bootmeth.o
ifdef CONFIG_OF_LIVE
obj-$(CONFIG_BOOTMETH_VBE_SIMPLE) += vbe_simple.o
endif
+obj-$(CONFIG_BOOTMETH_VBE) += vbe_fixup.o
diff --git a/test/boot/bootflow.c b/test/boot/bootflow.c
index 85305234e01..1e8ea754bcd 100644
--- a/test/boot/bootflow.c
+++ b/test/boot/bootflow.c
@@ -329,6 +329,8 @@ static int bootflow_system(struct unit_test_state *uts)
{
struct udevice *dev;
+ if (!IS_ENABLED(CONFIG_CMD_BOOTEFI_BOOTMGR))
+ return 0;
ut_assertok(uclass_get_device_by_name(UCLASS_BOOTMETH, "efi_mgr",
));
sandbox_set_fake_efi_mgr_dev(dev, true);
diff --git a/test/boot/vbe_fixup.c b/test/boot/vbe_fixup.c
new file mode 100644
index 000..1b488e25ab6
--- /dev/null
+++ b/test/boot/vbe_fixup.c
@@ -0,0 +1,59 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Test for VBE device tree fix-ups
+ *
+ * Copyright 2022 Google LLC
+ * Written by Simon Glass
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include "bootstd_common.h"
+
+/* Basic test of reading nvdata and updating a fwupd node in the device tree */
+static int vbe_test_fixup(struct unit_test_state *uts)
+{
+ ofnode chosen, node;
+ const char *data;
+ oftree tree;
+ int size;
+
+ /*
+* This test works when called from test_vbe.py and it must use the
+* flat tree, since device tree fix-ups do not yet support live tree.
+*/
+ if (!working_fdt)
+ return 0;
+
+ tree = oftree_from_fdt(working_fdt);
+ ut_assert(oftree_valid(tree));
+
+ chosen = oftree_path(tree, "/chosen");
+ ut_assert(ofnode_valid(chosen));
+
+ /* check the things set up for the FIT in test_vbe.py */
+ node = ofnode_find_subnode(chosen, "random");
+
+ /* ignore if this test is run on its own */
+ if (!ofnode_valid(node))
+ return 0;
+ data = ofnode_read_prop(node, "data", );
+ ut_asserteq(0x40, size);
+
+ node = ofnode_find_subnode(chosen, "aslr2");
+
[PATCH v2 13/14] dm: core: Update docs about oftree_from_fdt()
Update this function's comment and also the livetree documentation, so it is clear when to use the function. Signed-off-by: Simon Glass --- Changes in v2: - Add new patch to update docs about oftree_from_fdt() doc/develop/driver-model/livetree.rst | 2 +- include/dm/ofnode.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/doc/develop/driver-model/livetree.rst b/doc/develop/driver-model/livetree.rst index 55aa3eac929..579eef5ca9f 100644 --- a/doc/develop/driver-model/livetree.rst +++ b/doc/develop/driver-model/livetree.rst @@ -255,7 +255,7 @@ So long as OF_LIVE is disabled, it is possible to do fixups using the ofnode interface. The OF_LIVE support required addition of the flattening step at the end. -See dm_test_ofnode_root() for some examples. The ofnode_path_root() function +See dm_test_ofnode_root() for some examples. The oftree_from_fdt() function causes a flat device tree to be 'registered' such that it can be used by the ofnode interface. diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h index 7aae2c29ef1..fa9865602d8 100644 --- a/include/dm/ofnode.h +++ b/include/dm/ofnode.h @@ -59,6 +59,9 @@ __attribute_const__ int ofnode_to_offset(ofnode node); /** * oftree_from_fdt() - Returns an oftree from a flat device tree pointer * + * If @fdt is not already registered in the list of current device trees, it is + * added to the list. + * * @fdt: Device tree to use * * Returns: reference to the given node -- 2.38.0.rc1.362.ged0d419d3c-goog
[PATCH v2 12/14] vbe: Add fixups for a basic set of OS requests
As a starting point, add support for providing random data, if requested
by the OS. Also add ASLR, as a placeholder for now.
Signed-off-by: Simon Glass
---
(no changes since v1)
boot/Makefile| 2 +-
boot/vbe_fixup.c | 233 +++
test/py/tests/test_event_dump.py | 1 +
3 files changed, 235 insertions(+), 1 deletion(-)
create mode 100644 boot/vbe_fixup.c
diff --git a/boot/Makefile b/boot/Makefile
index 67e335255f1..dd45d786f8c 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -47,5 +47,5 @@ ifdef CONFIG_SPL_BUILD
obj-$(CONFIG_SPL_LOAD_FIT) += common_fit.o
endif
-obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE) += vbe.o
+obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE) += vbe.o vbe_fixup.o
obj-$(CONFIG_$(SPL_TPL_)BOOTMETH_VBE_SIMPLE) += vbe_simple.o
diff --git a/boot/vbe_fixup.c b/boot/vbe_fixup.c
new file mode 100644
index 000..8cdff6972b1
--- /dev/null
+++ b/boot/vbe_fixup.c
@@ -0,0 +1,233 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Verified Boot for Embedded (VBE) device tree fixup functions
+ *
+ * Copyright 2022 Google LLC
+ * Written by Simon Glass
+ */
+
+#define LOG_CATEGORY LOGC_BOOT
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define VBE_PREFIX "vbe,"
+#define VBE_PREFIX_LEN (sizeof(VBE_PREFIX) - 1)
+#define VBE_ERR_STR_LEN128
+#define VBE_MAX_RAND_SIZE 256
+
+struct vbe_result {
+ int errnum;
+ char err_str[VBE_ERR_STR_LEN];
+};
+
+typedef int (*vbe_req_func)(ofnode node, struct vbe_result *result);
+
+static int handle_random_req(ofnode node, int default_size,
+struct vbe_result *result)
+{
+ char buf[VBE_MAX_RAND_SIZE];
+ struct udevice *dev;
+ u32 size;
+ int ret;
+
+ if (!IS_ENABLED(CONFIG_DM_RNG))
+ return -ENOTSUPP;
+
+ if (ofnode_read_u32(node, "vbe,size", )) {
+ if (!default_size) {
+ snprintf(result->err_str, VBE_ERR_STR_LEN,
+"Missing vbe,size property");
+ return log_msg_ret("byt", -EINVAL);
+ }
+ size = default_size;
+ }
+ if (size > VBE_MAX_RAND_SIZE) {
+ snprintf(result->err_str, VBE_ERR_STR_LEN,
+"vbe,size %#x exceeds max size %#x", size,
+VBE_MAX_RAND_SIZE);
+ return log_msg_ret("siz", -E2BIG);
+ }
+ ret = uclass_first_device_check(UCLASS_RNG, );
+ if (ret) {
+ snprintf(result->err_str, VBE_ERR_STR_LEN,
+"Cannot find random-number device (err=%d)", ret);
+ return log_msg_ret("wr", ret);
+ }
+ ret = dm_rng_read(dev, buf, size);
+ if (ret) {
+ snprintf(result->err_str, VBE_ERR_STR_LEN,
+"Failed to read random-number device (err=%d)", ret);
+ return log_msg_ret("rd", ret);
+ }
+ ret = ofnode_write_prop(node, "data", buf, size, true);
+ if (ret)
+ return log_msg_ret("wr", -EINVAL);
+
+ return 0;
+}
+
+static int vbe_req_random_seed(ofnode node, struct vbe_result *result)
+{
+ return handle_random_req(node, 0, result);
+}
+
+static int vbe_req_aslr_move(ofnode node, struct vbe_result *result)
+{
+ return -ENOTSUPP;
+}
+
+static int vbe_req_aslr_rand(ofnode node, struct vbe_result *result)
+{
+ return handle_random_req(node, 4, result);
+}
+
+static int vbe_req_efi_runtime_rand(ofnode node, struct vbe_result *result)
+{
+ return handle_random_req(node, 4, result);
+}
+
+static struct vbe_req {
+ const char *compat;
+ vbe_req_func func;
+} vbe_reqs[] = {
+ /* address space layout randomization - move the OS in memory */
+ { "aslr-move", vbe_req_aslr_move },
+
+ /* provide random data for address space layout randomization */
+ { "aslr-rand", vbe_req_aslr_rand },
+
+ /* provide random data for EFI-runtime-services address */
+ { "efi-runtime-rand", vbe_req_efi_runtime_rand },
+
+ /* generate random data bytes to see the OS's rand generator */
+ { "random-rand", vbe_req_random_seed },
+
+};
+
+static int vbe_process_request(ofnode node, struct vbe_result *result)
+{
+ const char *compat, *req_name;
+ int i;
+
+ compat = ofnode_read_string(node, "compatible");
+ if (!compat)
+ return 0;
+
+ if (strlen(compat) <= VBE_PREFIX_LEN ||
+ strncmp(compat, VBE_PREFIX, VBE_PREFIX_LEN))
+ return -EINVAL;
+
+ req_name = compat + VBE_PREFIX_LEN; /* drop "vbe," prefix */
+ for (i = 0; i < ARRAY_SIZE(vbe_reqs); i++) {
+ if (!strcmp(vbe_reqs[i].compat, req_name)) {
+ int ret;
+
+ ret = vbe_reqs[i].func(node, result);
+ if (ret)
+
[PATCH v2 11/14] test: Move common FIT code into a separate fit_util file
To avoid duplicating code, create a new fit_util module which provides
various utility functions for FIT. Move this code out from the existing
test_fit.py and refactor it with addition parameters.
Fix up pylint warnings in the conversion.
This involves no functional change.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/boot/bootstd_common.c | 21 -
test/py/tests/fit_util.py | 90 ++
test/py/tests/test_fit.py | 79 -
3 files changed, 110 insertions(+), 80 deletions(-)
create mode 100644 test/py/tests/fit_util.py
diff --git a/test/boot/bootstd_common.c b/test/boot/bootstd_common.c
index 59d46bef0c5..7a40836507a 100644
--- a/test/boot/bootstd_common.c
+++ b/test/boot/bootstd_common.c
@@ -17,6 +17,7 @@
#include
#include "bootstd_common.h"
+/* tracks whether bootstd_setup_for_tests() has been run yet */
bool vbe_setup_done;
/* set up MMC for VBE tests */
@@ -27,6 +28,9 @@ int bootstd_setup_for_tests(void)
struct blk_desc *desc;
int ret;
+ if (vbe_setup_done)
+ return 0;
+
/* Set up the version string */
ret = uclass_get_device(UCLASS_MMC, 1, );
if (ret)
@@ -46,6 +50,8 @@ int bootstd_setup_for_tests(void)
if (blk_dwrite(desc, NVDATA_START_BLK, 1, buf) != 1)
return log_msg_ret("wr2", -EIO);
+ vbe_setup_done = true;
+
return 0;
}
@@ -65,17 +71,12 @@ int do_ut_bootstd(struct cmd_tbl *cmdtp, int flag, int
argc, char *const argv[])
{
struct unit_test *tests = UNIT_TEST_SUITE_START(bootstd_test);
const int n_ents = UNIT_TEST_SUITE_COUNT(bootstd_test);
+ int ret;
- if (!vbe_setup_done) {
- int ret;
-
- ret = bootstd_setup_for_tests();
- if (ret) {
- printf("Failed to set up for bootstd tests (err=%d)\n",
- ret);
- return CMD_RET_FAILURE;
- }
- vbe_setup_done = true;
+ ret = bootstd_setup_for_tests();
+ if (ret) {
+ printf("Failed to set up for bootstd tests (err=%d)\n", ret);
+ return CMD_RET_FAILURE;
}
return cmd_ut_category("bootstd", "bootstd_test_",
diff --git a/test/py/tests/fit_util.py b/test/py/tests/fit_util.py
new file mode 100644
index 000..fcec4c43c3c
--- /dev/null
+++ b/test/py/tests/fit_util.py
@@ -0,0 +1,90 @@
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright 2022 Google LLC
+
+"""Common utility functions for FIT tests"""
+
+import os
+
+import u_boot_utils as util
+
+def make_fname(cons, basename):
+"""Make a temporary filename
+
+Args:
+cons (ConsoleBase): u_boot_console to use
+basename (str): Base name of file to create (within temporary
directory)
+Return:
+Temporary filename
+"""
+
+return os.path.join(cons.config.build_dir, basename)
+
+def make_its(cons, base_its, params, basename='test.its'):
+"""Make a sample .its file with parameters embedded
+
+Args:
+cons (ConsoleBase): u_boot_console to use
+base_its (str): Template text for the .its file, typically containing
+%() references
+params (dict of str): Parameters to embed in the %() strings
+basename (str): base name to write to (will be placed in the temp dir)
+Returns:
+str: Filename of .its file created
+"""
+its = make_fname(cons, basename)
+with open(its, 'w', encoding='utf-8') as outf:
+print(base_its % params, file=outf)
+return its
+
+def make_fit(cons, mkimage, base_its, params, basename='test.fit'):
+"""Make a sample .fit file ready for loading
+
+This creates a .its script with the selected parameters and uses mkimage to
+turn this into a .fit image.
+
+Args:
+cons (ConsoleBase): u_boot_console to use
+mkimage (str): Filename of 'mkimage' utility
+base_its (str): Template text for the .its file, typically containing
+%() references
+params (dict of str): Parameters to embed in the %() strings
+basename (str): base name to write to (will be placed in the temp dir)
+Return:
+Filename of .fit file created
+"""
+fit = make_fname(cons, basename)
+its = make_its(cons, base_its, params)
+util.run_and_log(cons, [mkimage, '-f', its, fit])
+return fit
+
+def make_kernel(cons, basename, text):
+"""Make a sample kernel with test data
+
+Args:
+cons (ConsoleBase): u_boot_console to use
+basename (str): base name to write to (will be placed in the temp dir)
+text (str): Contents of the kernel file (will be repeated 100 times)
+Returns:
+str: Full path and filename of the kernel it created
+"""
+fname = make_fname(cons, basename)
+data = ''
+for i in range(100):
+data += f'this {text} {i} is unlikely to
[PATCH v2 09/14] boot: Pass the correct FDT to the EVT_FT_FIXUP event
Now that we support multiple device trees with the ofnode interface, we
can pass the correct FDT to this event. This allows the 'working' FDT to
be fixed up, as expected, so long as OFNODE_MULTI_TREE is enabled.
Also make sure we don't try to do this with livetree, which does not
support fixups yet.
Signed-off-by: Simon Glass
---
(no changes since v1)
boot/image-fdt.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/boot/image-fdt.c b/boot/image-fdt.c
index f651940d9b4..b830a0ab418 100644
--- a/boot/image-fdt.c
+++ b/boot/image-fdt.c
@@ -665,15 +665,18 @@ int image_setup_libfdt(struct bootm_headers *images, void
*blob,
goto err;
}
}
- if (CONFIG_IS_ENABLED(EVENT)) {
+ if (!of_live_active() && CONFIG_IS_ENABLED(EVENT)) {
struct event_ft_fixup fixup;
- fixup.tree = oftree_default();
+ fixup.tree = oftree_from_fdt(blob);
fixup.images = images;
- ret = event_notify(EVT_FT_FIXUP, , sizeof(fixup));
- if (ret) {
- printf("ERROR: fdt fixup event failed: %d\n", ret);
- goto err;
+ if (oftree_valid(fixup.tree)) {
+ ret = event_notify(EVT_FT_FIXUP, , sizeof(fixup));
+ if (ret) {
+ printf("ERROR: fdt fixup event failed: %d\n",
+ ret);
+ goto err;
+ }
}
}
--
2.38.0.rc1.362.ged0d419d3c-goog
[PATCH v2 10/14] boot: Tidy up logging and naming in vbe_simple
Make sure the log_msg_ret() values are unique so that the log trace is
unambiguous with LOG_ERROR_RETURN. Also avoid reusing the 'node' variable
for two different nodes in bootmeth_vbe_simple_ft_fixup(), since this is
confusing.
Signed-off-by: Simon Glass
---
(no changes since v1)
boot/vbe_simple.c | 16 +---
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/boot/vbe_simple.c b/boot/vbe_simple.c
index 61b6322ebe2..076b650c25a 100644
--- a/boot/vbe_simple.c
+++ b/boot/vbe_simple.c
@@ -6,6 +6,8 @@
* Written by Simon Glass
*/
+#define LOG_CATEGORY LOGC_BOOT
+
#include
#include
#include
@@ -199,17 +201,17 @@ int vbe_simple_fixup_node(ofnode node, struct
simple_state *state)
version = strdup(state->fw_version);
if (!version)
- return log_msg_ret("ver", -ENOMEM);
+ return log_msg_ret("dup", -ENOMEM);
ret = ofnode_write_string(node, "cur-version", version);
if (ret)
return log_msg_ret("ver", ret);
ret = ofnode_write_u32(node, "cur-vernum", state->fw_vernum);
if (ret)
- return log_msg_ret("ver", ret);
+ return log_msg_ret("num", ret);
ret = ofnode_write_string(node, "bootloader-version", version_string);
if (ret)
- return log_msg_ret("fix", ret);
+ return log_msg_ret("bl", ret);
return 0;
}
@@ -233,7 +235,7 @@ static int bootmeth_vbe_simple_ft_fixup(void *ctx, struct
event *event)
*/
for (vbe_find_first_device(); dev; vbe_find_next_device()) {
struct simple_state state;
- ofnode node;
+ ofnode node, subnode;
int ret;
if (strcmp("vbe_simple", dev->driver->name))
@@ -243,8 +245,8 @@ static int bootmeth_vbe_simple_ft_fixup(void *ctx, struct
event *event)
node = oftree_path(tree, "/chosen/fwupd");
if (!ofnode_valid(node))
continue;
- node = ofnode_find_subnode(node, dev->name);
- if (!ofnode_valid(node))
+ subnode = ofnode_find_subnode(node, dev->name);
+ if (!ofnode_valid(subnode))
continue;
log_debug("Fixing up: %s\n", dev->name);
@@ -255,7 +257,7 @@ static int bootmeth_vbe_simple_ft_fixup(void *ctx, struct
event *event)
if (ret)
return log_msg_ret("read", ret);
- ret = vbe_simple_fixup_node(node, );
+ ret = vbe_simple_fixup_node(subnode, );
if (ret)
return log_msg_ret("fix", ret);
}
--
2.38.0.rc1.362.ged0d419d3c-goog
[PATCH v2 08/14] sandbox: Support FDT fixups
Add support for doing device tree fixups in sandbox. This allows us to
test that functionality in CI.
Signed-off-by: Simon Glass
---
(no changes since v1)
arch/sandbox/lib/bootm.c | 17 +
1 file changed, 17 insertions(+)
diff --git a/arch/sandbox/lib/bootm.c b/arch/sandbox/lib/bootm.c
index c1742f94de7..28f4a746fb6 100644
--- a/arch/sandbox/lib/bootm.c
+++ b/arch/sandbox/lib/bootm.c
@@ -50,8 +50,25 @@ int bootz_setup(ulong image, ulong *start, ulong *end)
return ret;
}
+/* Subcommand: PREP */
+static int boot_prep_linux(struct bootm_headers *images)
+{
+ int ret;
+
+ if (CONFIG_IS_ENABLED(LMB)) {
+ ret = image_setup_linux(images);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
int do_bootm_linux(int flag, int argc, char *argv[], struct bootm_headers
*images)
{
+ if (flag & BOOTM_STATE_OS_PREP)
+ return boot_prep_linux(images);
+
if (flag & (BOOTM_STATE_OS_GO | BOOTM_STATE_OS_FAKE_GO)) {
bootstage_mark(BOOTSTAGE_ID_RUN_OS);
printf("## Transferring control to Linux (at address
%08lx)...\n",
--
2.38.0.rc1.362.ged0d419d3c-goog
[PATCH v2 05/14] fs: Quieten down the filesystems more
When looking for a filesystem on a partition we should do so quietly. At
present if the filesystem is very small (e.g. 512 bytes) we get a host of
messages.
Update these to only show when debugging.
Signed-off-by: Simon Glass
---
(no changes since v1)
disk/part_efi.c | 15 +++
fs/btrfs/disk-io.c| 7 ---
fs/ext4/ext4_common.c | 2 +-
fs/fs_internal.c | 3 +--
4 files changed, 13 insertions(+), 14 deletions(-)
diff --git a/disk/part_efi.c b/disk/part_efi.c
index ad94504ed90..26738a57d5d 100644
--- a/disk/part_efi.c
+++ b/disk/part_efi.c
@@ -264,20 +264,19 @@ int part_get_info_efi(struct blk_desc *dev_desc, int part,
/* "part" argument must be at least 1 */
if (part < 1) {
- printf("%s: Invalid Argument(s)\n", __func__);
- return -1;
+ log_debug("Invalid Argument(s)\n");
+ return -EINVAL;
}
/* This function validates AND fills in the GPT header and PTE */
if (find_valid_gpt(dev_desc, gpt_head, _pte) != 1)
- return -1;
+ return -EINVAL;
if (part > le32_to_cpu(gpt_head->num_partition_entries) ||
!is_pte_valid(_pte[part - 1])) {
- debug("%s: *** ERROR: Invalid partition number %d ***\n",
- __func__, part);
+ log_debug("*** ERROR: Invalid partition number %d ***\n", part);
free(gpt_pte);
- return -1;
+ return -EPERM;
}
/* The 'lbaint_t' casting may limit the maximum disk size to 2 TB */
@@ -300,8 +299,8 @@ int part_get_info_efi(struct blk_desc *dev_desc, int part,
info->type_guid, UUID_STR_FORMAT_GUID);
#endif
- debug("%s: start 0x" LBAF ", size 0x" LBAF ", name %s\n", __func__,
- info->start, info->size, info->name);
+ log_debug("start 0x" LBAF ", size 0x" LBAF ", name %s\n", info->start,
+ info->size, info->name);
/* Remember to free pte */
free(gpt_pte);
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index c80f8e80283..3f0d9f1c113 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0+
#include
#include
+#include
#include
#include
#include "kernel-shared/btrfs_tree.h"
@@ -910,9 +911,9 @@ static int btrfs_scan_fs_devices(struct blk_desc *desc,
if (round_up(BTRFS_SUPER_INFO_SIZE + BTRFS_SUPER_INFO_OFFSET,
desc->blksz) > (part->size << desc->log2blksz)) {
- error("superblock end %u is larger than device size " LBAFU,
- BTRFS_SUPER_INFO_SIZE + BTRFS_SUPER_INFO_OFFSET,
- part->size << desc->log2blksz);
+ log_debug("superblock end %u is larger than device size " LBAFU,
+ BTRFS_SUPER_INFO_SIZE + BTRFS_SUPER_INFO_OFFSET,
+ part->size << desc->log2blksz);
return -EINVAL;
}
diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index d49ba4a9954..1185cb2c046 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -2415,7 +2415,7 @@ int ext4fs_mount(unsigned part_length)
return 1;
fail:
- printf("Failed to mount ext2 filesystem...\n");
+ log_debug("Failed to mount ext2 filesystem...\n");
fail_noerr:
free(data);
ext4fs_root = NULL;
diff --git a/fs/fs_internal.c b/fs/fs_internal.c
index ae1cb8584c7..111f91b355d 100644
--- a/fs/fs_internal.c
+++ b/fs/fs_internal.c
@@ -29,8 +29,7 @@ int fs_devread(struct blk_desc *blk, struct disk_partition
*partition,
/* Check partition boundaries */
if ((sector + ((byte_offset + byte_len - 1) >> log2blksz))
>= partition->size) {
- log_err("%s read outside partition " LBAFU "\n", __func__,
- sector);
+ log_debug("read outside partition " LBAFU "\n", sector);
return 0;
}
--
2.38.0.rc1.362.ged0d419d3c-goog
[PATCH v2 07/14] bootstd: Move VBE setup into a shared function
This information needs to be set up by the bootstd tests as well. Move it
into a common function and ensure it is executed before any bootstd test
is run.
Make sure the 'images' parameter is set correctly for fixups.
Signed-off-by: Simon Glass
---
(no changes since v1)
test/boot/bootstd_common.c | 48 ++
test/boot/bootstd_common.h | 16 +
test/boot/vbe_simple.c | 34 ---
3 files changed, 68 insertions(+), 30 deletions(-)
diff --git a/test/boot/bootstd_common.c b/test/boot/bootstd_common.c
index 05347d87106..59d46bef0c5 100644
--- a/test/boot/bootstd_common.c
+++ b/test/boot/bootstd_common.c
@@ -9,10 +9,46 @@
#include
#include
#include
+#include
+#include
+#include
#include
#include
+#include
#include "bootstd_common.h"
+bool vbe_setup_done;
+
+/* set up MMC for VBE tests */
+int bootstd_setup_for_tests(void)
+{
+ ALLOC_CACHE_ALIGN_BUFFER(u8, buf, MMC_MAX_BLOCK_LEN);
+ struct udevice *mmc;
+ struct blk_desc *desc;
+ int ret;
+
+ /* Set up the version string */
+ ret = uclass_get_device(UCLASS_MMC, 1, );
+ if (ret)
+ return log_msg_ret("mmc", -EIO);
+ desc = blk_get_by_device(mmc);
+
+ memset(buf, '\0', MMC_MAX_BLOCK_LEN);
+ strcpy(buf, TEST_VERSION);
+ if (blk_dwrite(desc, VERSION_START_BLK, 1, buf) != 1)
+ return log_msg_ret("wr1", -EIO);
+
+ /* Set up the nvdata */
+ memset(buf, '\0', MMC_MAX_BLOCK_LEN);
+ buf[1] = ilog2(0x40) << 4 | 1;
+ *(u32 *)(buf + 4) = TEST_VERNUM;
+ buf[0] = crc8(0, buf + 1, 0x3f);
+ if (blk_dwrite(desc, NVDATA_START_BLK, 1, buf) != 1)
+ return log_msg_ret("wr2", -EIO);
+
+ return 0;
+}
+
int bootstd_test_drop_bootdev_order(struct unit_test_state *uts)
{
struct bootstd_priv *priv;
@@ -30,6 +66,18 @@ int do_ut_bootstd(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
struct unit_test *tests = UNIT_TEST_SUITE_START(bootstd_test);
const int n_ents = UNIT_TEST_SUITE_COUNT(bootstd_test);
+ if (!vbe_setup_done) {
+ int ret;
+
+ ret = bootstd_setup_for_tests();
+ if (ret) {
+ printf("Failed to set up for bootstd tests (err=%d)\n",
+ ret);
+ return CMD_RET_FAILURE;
+ }
+ vbe_setup_done = true;
+ }
+
return cmd_ut_category("bootstd", "bootstd_test_",
tests, n_ents, argc, argv);
}
diff --git a/test/boot/bootstd_common.h b/test/boot/bootstd_common.h
index 676ef0a57f9..c5e0fd1ceab 100644
--- a/test/boot/bootstd_common.h
+++ b/test/boot/bootstd_common.h
@@ -9,10 +9,17 @@
#ifndef __bootstd_common_h
#define __bootstd_common_h
+#include
+
/* Declare a new bootdev test */
#define BOOTSTD_TEST(_name, _flags) \
UNIT_TEST(_name, _flags, bootstd_test)
+#define NVDATA_START_BLK ((0x400 + 0x400) / MMC_MAX_BLOCK_LEN)
+#define VERSION_START_BLK ((0x400 + 0x800) / MMC_MAX_BLOCK_LEN)
+#define TEST_VERSION "U-Boot v2022.04-local2"
+#define TEST_VERNUM0x00010002
+
struct unit_test_state;
/**
@@ -24,4 +31,13 @@ struct unit_test_state;
*/
int bootstd_test_drop_bootdev_order(struct unit_test_state *uts);
+/**
+ * bootstd_setup_for_tests() - Set up MMC data for VBE tests
+ *
+ * Some data is needed for VBE tests to work. This function sets that up.
+ *
+ * @return 0 if OK, -ve on error
+ */
+int bootstd_setup_for_tests(void);
+
#endif
diff --git a/test/boot/vbe_simple.c b/test/boot/vbe_simple.c
index 8acd777f4cd..faba9e8f90b 100644
--- a/test/boot/vbe_simple.c
+++ b/test/boot/vbe_simple.c
@@ -10,54 +10,27 @@
#include
#include
#include
-#include
-#include
#include
#include
-#include
-#include
#include
#include
-#include
#include "bootstd_common.h"
-#define NVDATA_START_BLK ((0x400 + 0x400) / MMC_MAX_BLOCK_LEN)
-#define VERSION_START_BLK ((0x400 + 0x800) / MMC_MAX_BLOCK_LEN)
-#define TEST_VERSION "U-Boot v2022.04-local2"
-#define TEST_VERNUM0x00010002
-
/* Basic test of reading nvdata and updating a fwupd node in the device tree */
static int vbe_simple_test_base(struct unit_test_state *uts)
{
- ALLOC_CACHE_ALIGN_BUFFER(u8, buf, MMC_MAX_BLOCK_LEN);
const char *version, *bl_version;
struct event_ft_fixup fixup;
- struct udevice *dev, *mmc;
+ struct udevice *dev;
struct device_node *np;
- struct blk_desc *desc;
char fdt_buf[0x400];
char info[100];
int node_ofs;
ofnode node;
u32 vernum;
- /* Set up the version string */
- ut_assertok(uclass_get_device(UCLASS_MMC, 1, ));
- desc = blk_get_by_device(mmc);
- ut_assertnonnull(desc);
-
- memset(buf, '\0', MMC_MAX_BLOCK_LEN);
- strcpy(buf,
[PATCH v2 06/14] fdt: Show a message when the working FDT changes
The working FDT is the one which comes from the OS and is fixed up by
U-Boot. When the bootm command runs, it sets up the working FDT to be the
one it is about to pass to the OS, so that fixups can happen.
This seems like an important step, so add a message indicating that the
working FDT has changed. This is shown during the running of the bootm
command.
Signed-off-by: Simon Glass
---
(no changes since v1)
cmd/fdt.c | 1 +
doc/usage/cmd/fdt.rst | 1 +
test/cmd/fdt.c| 11 ++-
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/cmd/fdt.c b/cmd/fdt.c
index 6fbd9205d38..4b2dcfec863 100644
--- a/cmd/fdt.c
+++ b/cmd/fdt.c
@@ -40,6 +40,7 @@ void set_working_fdt_addr(ulong addr)
{
void *buf;
+ printf("Working FDT set to %lx\n", addr);
buf = map_sysmem(addr, 0);
working_fdt = buf;
env_set_hex("fdtaddr", addr);
diff --git a/doc/usage/cmd/fdt.rst b/doc/usage/cmd/fdt.rst
index 07fed732e45..36b8230877c 100644
--- a/doc/usage/cmd/fdt.rst
+++ b/doc/usage/cmd/fdt.rst
@@ -60,6 +60,7 @@ The second word shows the size of the FDT. Now set the
working FDT to that
address and expand it to 0xf000 in size::
=> fdt addr 1 f000
+Working FDT set to 1
=> md 1 4
0001: edfe0dd0 00f0 7800 7c27 ...x..'|
diff --git a/test/cmd/fdt.c b/test/cmd/fdt.c
index 100a7ef5ebf..ba9eaa42c14 100644
--- a/test/cmd/fdt.c
+++ b/test/cmd/fdt.c
@@ -55,6 +55,7 @@ static int fdt_test_addr(struct unit_test_state *uts)
/* The working fdt is not set, so this should fail */
set_working_fdt_addr(0);
+ ut_assert_nextline("Working FDT set to 0");
ut_asserteq(CMD_RET_FAILURE, run_command("fdt addr", 0));
ut_assert_nextline("libfdt fdt_check_header(): FDT_ERR_BADMAGIC");
ut_assertok(ut_check_console_end(uts));
@@ -63,18 +64,22 @@ static int fdt_test_addr(struct unit_test_state *uts)
ut_assertok(make_test_fdt(uts, fdt, sizeof(fdt)));
addr = map_to_sysmem(fdt);
set_working_fdt_addr(addr);
+ ut_assert_nextline("Working FDT set to %lx", addr);
ut_assertok(run_command("fdt addr", 0));
ut_assert_nextline("Working fdt: %08lx", (ulong)map_to_sysmem(fdt));
ut_assertok(ut_check_console_end(uts));
/* Set the working FDT */
set_working_fdt_addr(0);
+ ut_assert_nextline("Working FDT set to 0");
ut_assertok(run_commandf("fdt addr %08x", addr));
+ ut_assert_nextline("Working FDT set to %lx", addr);
ut_asserteq(addr, map_to_sysmem(working_fdt));
ut_assertok(ut_check_console_end(uts));
set_working_fdt_addr(0);
+ ut_assert_nextline("Working FDT set to 0");
- /* Set the working FDT */
+ /* Set the control FDT */
fdt_blob = gd->fdt_blob;
gd->fdt_blob = NULL;
ret = run_commandf("fdt addr -c %08x", addr);
@@ -93,6 +98,7 @@ static int fdt_test_addr(struct unit_test_state *uts)
/* Test detecting an invalid FDT */
fdt[0] = 123;
set_working_fdt_addr(addr);
+ ut_assert_nextline("Working FDT set to %lx", addr);
ut_asserteq(1, run_commandf("fdt addr"));
ut_assert_nextline("libfdt fdt_check_header(): FDT_ERR_BADMAGIC");
ut_assertok(ut_check_console_end(uts));
@@ -115,16 +121,19 @@ static int fdt_test_resize(struct unit_test_state *uts)
/* Test setting and resizing the working FDT to a larger size */
ut_assertok(console_record_reset_enable());
ut_assertok(run_commandf("fdt addr %08x %x", addr, newsize));
+ ut_assert_nextline("Working FDT set to %lx", addr);
ut_assertok(ut_check_console_end(uts));
/* Try shrinking it */
ut_assertok(run_commandf("fdt addr %08x %x", addr, sizeof(fdt) / 4));
+ ut_assert_nextline("Working FDT set to %lx", addr);
ut_assert_nextline("New length %d < existing length %d, ignoring",
(int)sizeof(fdt) / 4, newsize);
ut_assertok(ut_check_console_end(uts));
/* ...quietly */
ut_assertok(run_commandf("fdt addr -q %08x %x", addr, sizeof(fdt) / 4));
+ ut_assert_nextline("Working FDT set to %lx", addr);
ut_assertok(ut_check_console_end(uts));
/* We cannot easily provoke errors in fdt_open_into(), so ignore that */
--
2.38.0.rc1.362.ged0d419d3c-goog
[PATCH v2 04/14] boot: Correct handling of addresses in boot_relocate_fdt()
This code uses casts between addresses and pointers, so does not work with
sandbox. Update it so we can allow sandbox to do device tree fixups.
Signed-off-by: Simon Glass
---
(no changes since v1)
boot/image-fdt.c | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/boot/image-fdt.c b/boot/image-fdt.c
index 884e089f2d8..f651940d9b4 100644
--- a/boot/image-fdt.c
+++ b/boot/image-fdt.c
@@ -186,24 +186,25 @@ int boot_relocate_fdt(struct lmb *lmb, char
**of_flat_tree, ulong *of_size)
/* If fdt_high is set use it to select the relocation address */
fdt_high = env_get("fdt_high");
if (fdt_high) {
- void *desired_addr = (void *)hextoul(fdt_high, NULL);
+ ulong desired_addr = hextoul(fdt_high, NULL);
+ ulong addr;
- if (((ulong) desired_addr) == ~0UL) {
+ if (desired_addr == ~0UL) {
/* All ones means use fdt in place */
of_start = fdt_blob;
- lmb_reserve(lmb, (ulong)of_start, of_len);
+ lmb_reserve(lmb, map_to_sysmem(of_start), of_len);
disable_relocation = 1;
} else if (desired_addr) {
- of_start =
- (void *)(ulong) lmb_alloc_base(lmb, of_len, 0x1000,
- (ulong)desired_addr);
+ addr = lmb_alloc_base(lmb, of_len, 0x1000,
+ desired_addr);
+ of_start = map_sysmem(addr, of_len);
if (of_start == NULL) {
puts("Failed using fdt_high value for Device
Tree");
goto error;
}
} else {
- of_start =
- (void *)(ulong) lmb_alloc(lmb, of_len, 0x1000);
+ addr = lmb_alloc(lmb, of_len, 0x1000);
+ of_start = map_sysmem(addr, of_len);
}
} else {
mapsize = env_get_bootm_mapsize();
@@ -224,9 +225,8 @@ int boot_relocate_fdt(struct lmb *lmb, char **of_flat_tree,
ulong *of_size)
* At least part of this DRAM bank is usable, try
* using it for LMB allocation.
*/
- of_start =
- (void *)(ulong) lmb_alloc_base(lmb, of_len, 0x1000,
- start + usable);
+ of_start = map_sysmem((ulong)lmb_alloc_base(lmb,
+ of_len, 0x1000, start + usable), of_len);
/* Allocation succeeded, use this block. */
if (of_start != NULL)
break;
--
2.38.0.rc1.362.ged0d419d3c-goog
[PATCH v2 02/14] bootm: Avoid returning error codes from command
Functions which implement commands must return a CMD_RET_... error code.
At present bootm can return a negative errno value in some cases, thus
causing strange behaviour such as trying to exit the shell and printing
usage information.
Fix this by returning the correct value.
Signed-off-by: Simon Glass
---
(no changes since v1)
cmd/bootm.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/cmd/bootm.c b/cmd/bootm.c
index d764a27002d..f09b41c2c16 100644
--- a/cmd/bootm.c
+++ b/cmd/bootm.c
@@ -111,7 +111,7 @@ static int do_bootm_subcommand(struct cmd_tbl *cmdtp, int
flag, int argc,
bootm_get_addr(argc, argv) + image_load_offset);
#endif
- return ret;
+ return ret ? CMD_RET_FAILURE : 0;
}
/***/
@@ -120,6 +120,8 @@ static int do_bootm_subcommand(struct cmd_tbl *cmdtp, int
flag, int argc,
int do_bootm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
{
+ int ret;
+
#ifdef CONFIG_NEEDS_MANUAL_RELOC
static int relocated = 0;
@@ -152,7 +154,7 @@ int do_bootm(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
return do_bootm_subcommand(cmdtp, flag, argc, argv);
}
- return do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START |
+ ret = do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START |
BOOTM_STATE_FINDOS | BOOTM_STATE_PRE_LOAD |
BOOTM_STATE_FINDOTHER |
BOOTM_STATE_LOADOS |
#ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH
@@ -163,6 +165,8 @@ int do_bootm(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
#endif
BOOTM_STATE_OS_PREP | BOOTM_STATE_OS_FAKE_GO |
BOOTM_STATE_OS_GO, , 1);
+
+ return ret ? CMD_RET_FAILURE : 0;
}
int bootm_maybe_autostart(struct cmd_tbl *cmdtp, const char *cmd)
--
2.38.0.rc1.362.ged0d419d3c-goog
[PATCH v2 03/14] bootm: Drop #ifdef from do_bootm()
Drop the #ifdefs from this command by using a variable to hold the states
that should be executed.
Signed-off-by: Simon Glass
---
(no changes since v1)
cmd/bootm.c | 19 +--
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/cmd/bootm.c b/cmd/bootm.c
index f09b41c2c16..37c2af96e08 100644
--- a/cmd/bootm.c
+++ b/cmd/bootm.c
@@ -120,6 +120,7 @@ static int do_bootm_subcommand(struct cmd_tbl *cmdtp, int
flag, int argc,
int do_bootm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[])
{
+ int states;
int ret;
#ifdef CONFIG_NEEDS_MANUAL_RELOC
@@ -154,17 +155,15 @@ int do_bootm(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])
return do_bootm_subcommand(cmdtp, flag, argc, argv);
}
- ret = do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START |
- BOOTM_STATE_FINDOS | BOOTM_STATE_PRE_LOAD |
BOOTM_STATE_FINDOTHER |
- BOOTM_STATE_LOADOS |
-#ifdef CONFIG_SYS_BOOT_RAMDISK_HIGH
- BOOTM_STATE_RAMDISK |
-#endif
-#if defined(CONFIG_PPC) || defined(CONFIG_MIPS)
- BOOTM_STATE_OS_CMDLINE |
-#endif
+ states = BOOTM_STATE_START | BOOTM_STATE_FINDOS | BOOTM_STATE_PRE_LOAD |
+ BOOTM_STATE_FINDOTHER | BOOTM_STATE_LOADOS |
BOOTM_STATE_OS_PREP | BOOTM_STATE_OS_FAKE_GO |
- BOOTM_STATE_OS_GO, , 1);
+ BOOTM_STATE_OS_GO;
+ if (IS_ENABLED(CONFIG_SYS_BOOT_RAMDISK_HIGH))
+ states |= BOOTM_STATE_RAMDISK;
+ if (IS_ENABLED(CONFIG_PPC) || IS_ENABLED(CONFIG_MIPS))
+ states |= BOOTM_STATE_OS_CMDLINE;
+ ret = do_bootm_states(cmdtp, flag, argc, argv, states, , 1);
return ret ? CMD_RET_FAILURE : 0;
}
--
2.38.0.rc1.362.ged0d419d3c-goog
[PATCH v2 01/14] bootm: Change incorrect 'unsupported' error
At present when bootm fails, it says:
subcommand not supported
and then prints help for the bootm command. This is not very useful, since
generally the error is related to something else, such as fixups failing.
It is quite confusing to see this in a test run.
Change the error and show the error code.
We could update the OS functions to return -ENOSYS when they do not
support the bootm subcommand. But this involves some thought since this is
arch-specific code and proper errno error codes are not always returned.
Also, with the code as is, all required subcommands are of course
supported - a problem would only come if someone added a new one or
removed support for one from an existing OS. Therefore it seems better to
leave that sort of effort for when our bootm tests are improved.
Note: v1 of this patch generated a discussion[1] about printing error
strings automatically using printf(). That is outside the scope of this
patch but will be dealt with separately.
[1]
https://patchwork.ozlabs.org/project/uboot/patch/20220909151801.336551-3-...@chromium.org/
Signed-off-by: Simon Glass
---
(no changes since v1)
boot/bootm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/boot/bootm.c b/boot/bootm.c
index 5b20b418dba..a4c0870c0fe 100644
--- a/boot/bootm.c
+++ b/boot/bootm.c
@@ -790,7 +790,7 @@ int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int
argc,
/* Check for unsupported subcommand. */
if (ret) {
- puts("subcommand not supported\n");
+ printf("subcommand failed (err=%d)\n", ret);
return ret;
}
--
2.38.0.rc1.362.ged0d419d3c-goog
[PATCH v2 00/14] vbe: Support device tree fixups for OS requests
VBE provides the ability for an OS to request that information be passed to it when it is booted. This is added into the /chosen node, in addition to things like the bootargs for Linux, for example. VBE's OS requests are intended to replace the need for the EFI 'boot-time services'. This works via a 'stub' which runs before Linux, collects the information from U-Boot, writes it to the device tree (mostly) and then jumps to Linux with the updated device tree. Rather than just jumping into Linux and waiting for it to request things from U-Boot, we can look at the requests in the FIT and process them before jumping to Linux. This is simpler and easier to test. It is also more deterministic, since we can tell whether we might lack something needed by Linux, before jumping to it. This series adds initial support for OS requests, with just a few simple ones provided. Further work will expand these out. Changes in v2: - Add new patch to update docs about oftree_from_fdt() Simon Glass (14): bootm: Change incorrect 'unsupported' error bootm: Avoid returning error codes from command bootm: Drop #ifdef from do_bootm() boot: Correct handling of addresses in boot_relocate_fdt() fs: Quieten down the filesystems more fdt: Show a message when the working FDT changes bootstd: Move VBE setup into a shared function sandbox: Support FDT fixups boot: Pass the correct FDT to the EVT_FT_FIXUP event boot: Tidy up logging and naming in vbe_simple test: Move common FIT code into a separate fit_util file vbe: Add fixups for a basic set of OS requests dm: core: Update docs about oftree_from_fdt() vbe: Add a test for VBE device tree fixups arch/sandbox/lib/bootm.c | 17 ++ boot/Makefile | 2 +- boot/bootm.c | 2 +- boot/image-fdt.c | 37 ++-- boot/vbe_fixup.c | 233 ++ boot/vbe_simple.c | 16 +- cmd/bootm.c | 25 +-- cmd/fdt.c | 1 + configs/sandbox_flattree_defconfig| 2 +- disk/part_efi.c | 15 +- doc/develop/driver-model/livetree.rst | 2 +- doc/develop/vbe.rst | 3 +- doc/usage/cmd/fdt.rst | 1 + fs/btrfs/disk-io.c| 7 +- fs/ext4/ext4_common.c | 2 +- fs/fs_internal.c | 3 +- include/dm/ofnode.h | 3 + test/boot/Makefile| 1 + test/boot/bootflow.c | 2 + test/boot/bootstd_common.c| 49 ++ test/boot/bootstd_common.h| 16 ++ test/boot/vbe_fixup.c | 59 +++ test/boot/vbe_simple.c| 34 +--- test/cmd/fdt.c| 11 +- test/py/tests/fit_util.py | 93 ++ test/py/tests/test_event_dump.py | 1 + test/py/tests/test_fit.py | 79 + test/py/tests/test_vbe.py | 123 ++ 28 files changed, 684 insertions(+), 155 deletions(-) create mode 100644 boot/vbe_fixup.c create mode 100644 test/boot/vbe_fixup.c create mode 100644 test/py/tests/fit_util.py create mode 100644 test/py/tests/test_vbe.py -- 2.38.0.rc1.362.ged0d419d3c-goog
Re: [PATCH v2 2/2] mach-meson: g12a: reset usb controller in reset_misc()
On 10/11/22 09:12, neil.armstr...@linaro.org wrote:
On 10/10/2022 19:16, Marek Vasut wrote:
On 10/10/22 18:22, Neil Armstrong wrote:
Hi,
Hi,
On 10/10/2022 18:09, Marek Vasut wrote:
On 10/7/22 11:38, Mattijs Korpershoek wrote:
On some g12a boards like the VIM3L and the SEI610, with some
USB cables/hosts, there is a long (5s) delay before
between "fastboot reboot" and the host detecting a USB reset.
This breaks tools relying on "fastboot reboot fastboot" which assume
that 1s after the command send, the board should disconnect on usb.
To reproduce, enable fastboot in U-Boot console:
=> fastboot usb 0
Then, on the host, run:
# echo "running fastboot reboot bootloader" > /dev/kmsg &&
fastboot reboot bootloader
Rebooting into bootloader OKAY [ 0.003s]
Finished. Total time: 3.033s
[54074.251551] running fastboot reboot bootloader
... there is a delay of 5s before we detect a disconnection ...
[54079.041238] usb 1-7.4: USB disconnect, device number 72
[54079.239625] usb 1-7.4: new high-speed USB device number 73
using xhci_hcd
[54079.359103] usb 1-7.4: New USB device found, idVendor=1b8e,
idProduct=fada, bcdDevice= 2.27
[54079.359110] usb 1-7.4: New USB device strings: Mfr=1,
Product=2, SerialNumber=3
[54079.359112] usb 1-7.4: Product: USB download gadget
[54079.359114] usb 1-7.4: Manufacturer: U-Boot
[54079.359116] usb 1-7.4: SerialNumber: C8631470CC41
Note: this does not happen when we use the RST button on the board.
To fix this, re-implement a platform reset which calls
board_usb_cleanup() before resetting the board.
Shouldn't that call happen somewhere in drivers/usb/ .remove()
callback instead ?
No since dwc2 isn't DM yet, handling is done in arch/arm/mac-meson
board_usb_*() for now
Seems DWC2 is DM:
$ git grep U_BOOT_DRIVER drivers/usb/ | grep dwc2
drivers/usb/gadget/dwc2_udc_otg.c:U_BOOT_DRIVER(dwc2_udc_otg) = {
drivers/usb/host/dwc2.c:U_BOOT_DRIVER(usb_dwc2) = {
My bad, seems I missed the dwc2 otg DM wagon...
We will need to switch to this now then, thanks,
Thanks. I picked 1/2 at least, so you can only focus on 2/2 and send
just that.
Re: [u-boot][PATCH 10/14] mtd: rawnand: omap_gpmc: support u-boot driver model
On Tue, Oct 11, 2022 at 6:52 AM Roger Quadros wrote:
>
> Adds driver model support.
>
> We need to be able to self initialize the NAND controller/chip
> at probe and so enable CONFIG_SYS_NAND_SELF_INIT.
>
> Doing so requires nand_register() API which is provided by nand.c
> and needs to be enabled during SPL build via CONFIG_SPL_NAND_INIT.
> But nand.c also provides nand_init() so we need to get rid of nand_init()
> in omap_gpmc driver if CONFIG_SPL_NAND_INIT is set.
>
> Signed-off-by: Roger Quadros
> ---
> drivers/mtd/nand/raw/Kconfig | 1 +
> drivers/mtd/nand/raw/omap_gpmc.c | 55 +++-
> 2 files changed, 55 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
> index bc5cabdfc2..1d23144ce4 100644
> --- a/drivers/mtd/nand/raw/Kconfig
> +++ b/drivers/mtd/nand/raw/Kconfig
> @@ -190,6 +190,7 @@ config NAND_LPC32XX_SLC
> config NAND_OMAP_GPMC
> bool "Support OMAP GPMC NAND controller"
> depends on ARCH_OMAP2PLUS || ARCH_KEYSTONE || ARCH_K3
> + select SYS_NAND_SELF_INIT if ARCH_K3
I have a question about this down below.
> help
> Enables omap_gpmc.c driver for OMAPx and AM platforms.
> GPMC controller is used for parallel NAND flash devices, and can
> diff --git a/drivers/mtd/nand/raw/omap_gpmc.c
> b/drivers/mtd/nand/raw/omap_gpmc.c
> index e772a914c8..7192ca9e5a 100644
> --- a/drivers/mtd/nand/raw/omap_gpmc.c
> +++ b/drivers/mtd/nand/raw/omap_gpmc.c
> @@ -7,6 +7,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> #ifdef CONFIG_ARCH_OMAP2PLUS
> @@ -1121,7 +1122,7 @@ int __maybe_unused omap_nand_switch_ecc(uint32_t
> hardware, uint32_t eccstrength)
> * nand_scan about special functionality. See the defines for further
> * explanation
> */
> -int board_nand_init(struct nand_chip *nand)
> +int gpmc_nand_init(struct nand_chip *nand)
> {
> int32_t gpmc_config = 0;
> int cs = cs_next++;
> @@ -1201,3 +1202,55 @@ int board_nand_init(struct nand_chip *nand)
>
> return 0;
> }
> +
> +static struct nand_chip *nand_chip;/* First NAND chip for SPL use only */
> +
> +#if CONFIG_IS_ENABLED(SYS_NAND_SELF_INIT)
> +
> +static int gpmc_nand_probe(struct udevice *dev)
> +{
> + struct nand_chip *nand = dev_get_priv(dev);
> + struct mtd_info *mtd = nand_to_mtd(nand);
> + int ret;
> +
> + gpmc_nand_init(nand);
> +
> + ret = nand_scan(mtd, CONFIG_SYS_NAND_MAX_CHIPS);
> + if (ret)
> + return ret;
> +
> + ret = nand_register(0, mtd);
> + if (ret)
> + return ret;
> +
> + if (!nand_chip)
> + nand_chip = nand;
> +
> + return 0;
> +}
> +
> +static const struct udevice_id gpmc_nand_ids[] = {
> + { .compatible = "ti,am64-nand" },
> + { .compatible = "ti,omap2-nand" },
The gpmc_nand_ids reference to omap2, but it's encapsulated inside the
SYS_NAND_SELF_INIT ifdef which appears to only be set if K3. Should
this code be expected to work on OMAP2? I don't think K3 is set for
OMAP2+. If so, should the SYS_NAND_SELF_INIT be selected if OMAP2 is
selected?
I have a DM3730 that I can test with this. Do you have a repo I can
point to to test? If not, I'll pull the series from patchwork, but I
need to know what branch to use as a starting point.
thanks,
adam
> + { }
> +};
> +
> +U_BOOT_DRIVER(gpmc_nand) = {
> + .name = "gpmc-nand",
> + .id = UCLASS_MTD,
> + .of_match = gpmc_nand_ids,
> + .probe = gpmc_nand_probe,
> + .priv_auto = sizeof(struct nand_chip),
> +};
> +
> +void board_nand_init(void)
> +{
> + struct udevice *dev;
> + int ret;
> +
> + ret = uclass_get_device_by_driver(UCLASS_MTD,
> + DM_DRIVER_GET(gpmc_nand), );
> + if (ret && ret != -ENODEV)
> + pr_err("%s: Failed to get GPMC device: %d\n", __func__, ret);
> +}
> +#endif /* CONFIG_SYS_NAND_SELF_INIT */
> --
> 2.17.1
>
[PATCH] stm32mp: fix compilation issue with DEBUG_UART
Fix the compilation issue when CONFIG_DEBUG_UART is activated
drivers/serial/serial_stm32.o: in function `debug_uart_init':
drivers/serial/serial_stm32.c:291: undefined reference to \
`board_debug_uart_init'
The board_debug_uart_init is needed for SPL boot, called in
cpu.c::mach_cpu_init(); it is defined in board/st/stm32mp1/spl.c.
But with the removal #ifdefs patch, the function debug_uart_init() is
always compiled even if not present in the final U-Boot image.
This patch adds a file to provided this function when DEBUG_UART and SPL
are activated.
Fixes: c8b2eef52b6c ("stm32mp15: tidy up #ifdefs in cpu.c")
Signed-off-by: Patrick Delaunay
---
arch/arm/mach-stm32mp/Kconfig.15x | 2 +-
board/engicam/stm32mp1/Makefile | 2 ++
board/engicam/stm32mp1/spl.c | 25 -
board/st/stm32mp1/Makefile| 2 ++
board/st/stm32mp1/debug_uart.c| 29 +
board/st/stm32mp1/spl.c | 28
6 files changed, 34 insertions(+), 54 deletions(-)
create mode 100644 board/st/stm32mp1/debug_uart.c
diff --git a/arch/arm/mach-stm32mp/Kconfig.15x
b/arch/arm/mach-stm32mp/Kconfig.15x
index d516270292a..5bd9b53a5d8 100644
--- a/arch/arm/mach-stm32mp/Kconfig.15x
+++ b/arch/arm/mach-stm32mp/Kconfig.15x
@@ -117,7 +117,7 @@ endif
if DEBUG_UART
config DEBUG_UART_BOARD_INIT
- default y
+ default y if SPL
# debug on UART4 by default
config DEBUG_UART_BASE
diff --git a/board/engicam/stm32mp1/Makefile b/board/engicam/stm32mp1/Makefile
index 65560df2900..155d33f9eec 100644
--- a/board/engicam/stm32mp1/Makefile
+++ b/board/engicam/stm32mp1/Makefile
@@ -8,3 +8,5 @@ obj-y += spl.o
else
obj-y += stm32mp1.o
endif
+
+obj-$(CONFIG_DEBUG_UART_BOARD_INIT) += ../../st/stm32mp1/debug_uart.o
diff --git a/board/engicam/stm32mp1/spl.c b/board/engicam/stm32mp1/spl.c
index 3aa738b3faa..2b7779cc01d 100644
--- a/board/engicam/stm32mp1/spl.c
+++ b/board/engicam/stm32mp1/spl.c
@@ -6,7 +6,6 @@
*/
#include
-#include
/* board early initialisation in board_f: need to use global variable */
static u32 opp_voltage_mv __section(".data");
@@ -22,27 +21,3 @@ int board_early_init_f(void)
return 0;
}
-#ifdef CONFIG_DEBUG_UART_BOARD_INIT
-void board_debug_uart_init(void)
-{
-#if (CONFIG_DEBUG_UART_BASE == STM32_UART4_BASE)
-
-#define RCC_MP_APB1ENSETR (STM32_RCC_BASE + 0x0A00)
-#define RCC_MP_AHB4ENSETR (STM32_RCC_BASE + 0x0A28)
-
- /* UART4 clock enable */
- setbits_le32(RCC_MP_APB1ENSETR, BIT(16));
-
-#define GPIOG_BASE 0x50008000
- /* GPIOG clock enable */
- writel(BIT(6), RCC_MP_AHB4ENSETR);
- /* GPIO configuration for ST boards: Uart4 TX = G11 */
- writel(0xffbf, GPIOG_BASE + 0x00);
- writel(0x6000, GPIOG_BASE + 0x24);
-#else
-
-#error("CONFIG_DEBUG_UART_BASE: not supported value")
-
-#endif
-}
-#endif
diff --git a/board/st/stm32mp1/Makefile b/board/st/stm32mp1/Makefile
index 65560df2900..f2d720b67b3 100644
--- a/board/st/stm32mp1/Makefile
+++ b/board/st/stm32mp1/Makefile
@@ -8,3 +8,5 @@ obj-y += spl.o
else
obj-y += stm32mp1.o
endif
+
+obj-$(CONFIG_DEBUG_UART_BOARD_INIT) += debug_uart.o
diff --git a/board/st/stm32mp1/debug_uart.c b/board/st/stm32mp1/debug_uart.c
new file mode 100644
index 000..24e3f9f2201
--- /dev/null
+++ b/board/st/stm32mp1/debug_uart.c
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0-or-later OR BSD-3-Clause
+/*
+ * Copyright (C) 2022, STMicroelectronics - All Rights Reserved
+ */
+
+#include
+#include
+#include
+#include
+#include
+
+#define RCC_MP_APB1ENSETR (STM32_RCC_BASE + 0x0A00)
+#define RCC_MP_AHB4ENSETR (STM32_RCC_BASE + 0x0A28)
+
+#define GPIOG_BASE 0x50008000
+
+void board_debug_uart_init(void)
+{
+ if (CONFIG_DEBUG_UART_BASE == STM32_UART4_BASE) {
+ /* UART4 clock enable */
+ setbits_le32(RCC_MP_APB1ENSETR, BIT(16));
+
+ /* GPIOG clock enable */
+ writel(BIT(6), RCC_MP_AHB4ENSETR);
+ /* GPIO configuration for ST boards: Uart4 TX = G11 */
+ writel(0xffbf, GPIOG_BASE + 0x00);
+ writel(0x6000, GPIOG_BASE + 0x24);
+ }
+}
diff --git a/board/st/stm32mp1/spl.c b/board/st/stm32mp1/spl.c
index 8e4549a1b35..747ec7e445a 100644
--- a/board/st/stm32mp1/spl.c
+++ b/board/st/stm32mp1/spl.c
@@ -5,11 +5,7 @@
#include
#include
-#include
-#include
#include
-#include
-#include
#include "../common/stpmic1.h"
/* board early initialisation in board_f: need to use global variable */
@@ -29,27 +25,3 @@ int board_early_init_f(void)
return 0;
}
-#ifdef CONFIG_DEBUG_UART_BOARD_INIT
-void board_debug_uart_init(void)
-{
-#if (CONFIG_DEBUG_UART_BASE == STM32_UART4_BASE)
-
-#define RCC_MP_APB1ENSETR (STM32_RCC_BASE + 0x0A00)
-#define RCC_MP_AHB4ENSETR (STM32_RCC_BASE + 0x0A28)
-
- /* UART4 clock enable */
- setbits_le32(RCC_MP_APB1ENSETR, BIT(16));
-
-#define GPIOG_BASE 0x50008000
- /*
Re: [PATCH v2 2/5] Makefile: Correct a missing FORCE on the binman rule
On Tue, Oct 11, 2022 at 11:16 PM Simon Glass wrote: > > This is required for if_changed to work correctly. Add it. > > Signed-off-by: Simon Glass > Reviewed-by: Pali Rohár > --- > > (no changes since v1) > > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index 3866cc62f9a..d28e8b4e316 100644 > --- a/Makefile > +++ b/Makefile > @@ -,7 +,7 @@ endef > PHONY += inputs > inputs: $(INPUTS-y) > > -all: .binman_stamp inputs > +all: .binman_stamp inputs FORCE > ifeq ($(CONFIG_BINMAN),y) > $(call if_changed,binman) 'all' is usually used as a phony target. I think this went wrong. if_changed should never be used for a phony target. In other words, if_changed should produce a build artifact. FORCE is never used for a phony target because it is added to 'PHONY'. PHONY += all > endif > -- > 2.38.0.rc1.362.ged0d419d3c-goog >
Re: [PATCH 1/2] global: Do not default to faking missing binaries for buildman
Hi Rasmus, On Tue, 11 Oct 2022 at 00:41, Rasmus Villemoes wrote: > > On 10/10/2022 17.18, Tom Rini wrote: > > While it is possible and documented on how to re-run buildman to replace > > faked required binary files after the fact, this behavior ends up being > > more confusing than helpful in practice. Switch to requiring > > BINMAN_ALLOW_MISSING=1 to be passed on the 'make' line to enable this > > behavior. > > Recreating the bitbake scenario that bit me with this applied correctly > failed to build. Thanks. > > Tested-by: Rasmus Villemoes If you have time, can you try this one instead (or the series): https://patchwork.ozlabs.org/project/uboot/patch/20221010200032.73483-5-...@chromium.org/ Regards, Simon
Re: [PATCH 1/1] dm: fix blk_get_devnum_by_uclass_idname()
Hi Heinrich, On Tue, 11 Oct 2022 at 04:38, Heinrich Schuchardt wrote: > > > > On 10/11/22 07:46, Heinrich Schuchardt wrote: > > > > > > On 10/11/22 01:49, Simon Glass wrote: > >> Hi Heinrich, > >> > >> On Thu, 6 Oct 2022 at 14:05, Heinrich Schuchardt > >> wrote: > >>> > >>> On 10/3/22 18:44, Simon Glass wrote: > Hi Heinrich, > > On Mon, 3 Oct 2022 at 10:33, Heinrich Schuchardt > wrote: > > > > > > > > On 10/3/22 16:57, Simon Glass wrote: > >> Hi Heinrich, > >> > >> On Mon, 3 Oct 2022 at 03:36, Heinrich Schuchardt > >> wrote: > >>> > >>> On the sandbox I run: > >>> > >>>=> setenv efi_selftest block device > >>>=> bootefi selftest > >>> > >>> and see the following output: > >>> > >>>** Bad device specification host 0 ** > >>>Couldn't find partition host 0:0 > >>>Cannot read EFI system partition > >>> > >>> Running > >>> > >>>=> lsblk > >>> > >>> yields > >>> > >>>Block Driver Devices > >>>- > >>>efi_blk : efiloader 0 > >>>ide_blk : > >>>mmc_blk : mmc 2, mmc 1, mmc 0 > >>>nvme-blk: > >>>sandbox_host_blk: > >>>scsi_blk: > >>>usb_storage_blk : > >>>virtio-blk : > >>> > >>> So a efi_blk device was mistaken for a host device. > >>> > >>> I continue with > >>> > >>>=> host bind 0 ../sandbox.img > >>>=> ls host 0:1 > >>> > >>> and get the following output: > >>> > >>> 13 hello.txt > >>>7 u-boot.txt > >>> > >>>2 file(s), 0 dir(s) > >>> > >>> This is the content of efiblock 0:1 and not of host 0:1 (sic!). > >>> > >>> The uclass of the parent device is irrelevant for the > >>> determination of the > >>> uclass of the block device. We must use the uclass stored in the > >>> block > >>> device descriptor. > >>> > >>> This issue has been raised repeatedly: > >>> > >>> [PATCH 1/1] block: fix blk_get_devnum_by_typename() > >>> https://lore.kernel.org/u-boot/20220802094933.69170-1-heinrich.schucha...@canonical.com/ > >>> [PATCH 1/1] blk: simplify blk_get_devnum_by_typename() > >>> https://lore.kernel.org/u-boot/20211023140647.7661-1-heinrich.schucha...@canonical.com/ > >> > >> Yes and you were not able/willing to take on the required work, so > >> this carried on longer than it should have. I finally did this myself > >> and it is now in -next. > > > > The refactoring was orthogonal to the problem that I reported and > > which > > you unfortunately did not consider in the process. > > Well it involved using if_type to work around a problem, just making > it harder to get rid of. Overall I am in favour of a faster pace of > migration that we have been following and it would help if people took > on some of this, instead of fixing their little issue. > > > > >> > >> So we might finally be able to fix this problem properly, since > >> if_type is mostly just a work-around concept in -next, with just the > >> fake uclass_id being used at present. > >> > >> Can you use if_type_to_uclass_id() here, which is the work-around > >> function for now? > > > > This function does not exist in origin/next. We won't apply this patch > > in the 2022-10 cycle. > > I think I mean conv_uclass_id() which is the new name. > > > > > Let's fix the bug first before thinking about future refactoring. > > > > You may determine the uclass ID for field bdev in struct blk_desc > > using > > function device_get_uclass_id() when refactoring. > > So if you call conv_uclass_id() (without any other refactoring) does > that fix the problem? > >>> > >>> Except for UCLASS_USB that function is a NOP. How could it help to > >>> differentiate between devices with the same parent device? > >> > >> It can't. But the root node should not have UCLASS_BLK children. I > >> think I mentioned that a few months back? > >> > >>> > >>> Would you agree that blk_get_devnum_by_uclass_idname() should not look > >>> at the parent but on the actual device? > >> > >> No, looking at the parent is exactly what it should do. A block device > >> is generic, to the extent possible. Its methods are implemented in the > >> parent uclass and are tightly bound to it. See for example > >> U_BOOT_DRIVER(mmc_blk) in the MMC uclass. > > > > Let's look at an MMC device > > > > root_driver/soc/mmc@1c0f000/m...@1c0f000.blk is a block device. > > > > What do we need to find out that it can be addressed as mmc 0? The > > driver is mmc_blk and its index is 0. We
[PATCH v2 4/5] binman: Use an exit code when blobs are missing
At present binman returns success when told to handle missing blobs.
This is confusing this in fact the resulting image cannot work.
Use exit code 103 to signal this problem, with a -W option to convert
it to a warning.
Signed-off-by: Simon Glass
---
Changes in v2:
- Fix missing usage of the msg variable
- Simplify code to remove the 'else'
tools/binman/binman.rst | 4
tools/binman/cmdline.py | 3 +++
tools/binman/control.py | 6 +-
3 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
index 4ee6f41f35e..c0512e68e67 100644
--- a/tools/binman/binman.rst
+++ b/tools/binman/binman.rst
@@ -1458,6 +1458,10 @@ space-separated list of directories to search for binary
blobs::
odroid-c4/build/board/hardkernel/odroidc4/firmware \
odroid-c4/build/scp_task" binman ...
+Note that binman fails with error code 103 when there are missing blobs. If you
+wish to continue anyway, you can pass `-W` to binman.
+
+
Code coverage
-
diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py
index 1d1ca43993d..144c0c916a2 100644
--- a/tools/binman/cmdline.py
+++ b/tools/binman/cmdline.py
@@ -128,6 +128,9 @@ controlled by a description in the board device tree.'''
default=False, help='Update the binman node with offset/size info')
build_parser.add_argument('--update-fdt-in-elf', type=str,
help='Update an ELF file with the output dtb:
infile,outfile,begin_sym,end_sym')
+build_parser.add_argument(
+'-W', '--ignore-missing-blobs', action='store_true', default=False,
+help='Return success even if there are missing blobs')
subparsers.add_parser(
'bintool-docs', help='Write out bintool documentation (see
bintool.rst)')
diff --git a/tools/binman/control.py b/tools/binman/control.py
index bfe63a15204..da52abad077 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -742,7 +742,11 @@ def Binman(args):
elf.UpdateFile(*elf_params, data)
if invalid:
-tout.warning("\nSome images are invalid")
+msg = '\nSome images are invalid'
+if not args.ignore_missing_blobs:
+tout.error(msg)
+return 103
+tout.warning(msg)
# Use this to debug the time take to pack the image
#state.TimingShow()
--
2.38.0.rc1.362.ged0d419d3c-goog
[PATCH v2 3/5] doc: Correct the path to the Makefile documentation
This is out-of-date now. Fix it. Signed-off-by: Simon Glass --- (no changes since v1) scripts/Kbuild.include | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include index 9c14310ad40..62e0207f91b 100644 --- a/scripts/Kbuild.include +++ b/scripts/Kbuild.include @@ -229,7 +229,7 @@ objectify = $(foreach o,$(1),$(if $(filter /%,$(o)),$(o),$(obj)/$(o))) # if_changed_dep - as if_changed, but uses fixdep to reveal dependencies # including used config symbols # if_changed_rule - as if_changed but execute rule instead -# See Documentation/kbuild/makefiles.txt for more info +# See doc/develop/makefiles.rst for more info ifneq ($(KBUILD_NOCMDDEP),1) # Check if both arguments are the same including their order. Result is empty -- 2.38.0.rc1.362.ged0d419d3c-goog
[PATCH v2 5/5] buildman: Detect binman reporting missing blobs
Buildman should consider a build as a success (with warnings) if missing
blobs have been dealt with by binman, even though buildman itself returns
and error code overall. This is how other warnings are dealt with.
We cannot easily access the 103 exit code, so detect the problem in the
output.
With this change, missing blobs result in an exit code of 101, although
they still indicate failure.
Signed-off-by: Simon Glass
---
(no changes since v1)
tools/buildman/builderthread.py | 6 +-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/tools/buildman/builderthread.py b/tools/buildman/builderthread.py
index 6240e08c767..065d836d68c 100644
--- a/tools/buildman/builderthread.py
+++ b/tools/buildman/builderthread.py
@@ -288,10 +288,14 @@ class BuilderThread(threading.Thread):
args.append('cfg')
result = self.Make(commit, brd, 'build', cwd, *args,
env=env)
+if (result.return_code == 2 and
+('Some images are invalid' in result.stderr)):
+# This is handled later by the check for output in
+# stderr
+result.return_code = 0
if adjust_cfg:
errs = cfgutil.check_cfg_file(cfg_file, adjust_cfg)
if errs:
-print('errs', errs)
result.stderr += errs
result.return_code = 1
result.stderr = result.stderr.replace(src_dir + '/', '')
--
2.38.0.rc1.362.ged0d419d3c-goog
[PATCH v2 2/5] Makefile: Correct a missing FORCE on the binman rule
This is required for if_changed to work correctly. Add it. Signed-off-by: Simon Glass Reviewed-by: Pali Rohár --- (no changes since v1) Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 3866cc62f9a..d28e8b4e316 100644 --- a/Makefile +++ b/Makefile @@ -,7 +,7 @@ endef PHONY += inputs inputs: $(INPUTS-y) -all: .binman_stamp inputs +all: .binman_stamp inputs FORCE ifeq ($(CONFIG_BINMAN),y) $(call if_changed,binman) endif -- 2.38.0.rc1.362.ged0d419d3c-goog
[PATCH v2 1/5] buildman: Handle the MAINTAINERS 'N' tag
This is needed for some soon-to-be-applied patches. Scan the configs/
directory to see if any of the files match.
Signed-off-by: Simon Glass
Tested-by: Tom Rini
Suggested-by: Tom Rini
---
(no changes since v1)
tools/buildman/boards.py | 11 +++
1 file changed, 11 insertions(+)
diff --git a/tools/buildman/boards.py b/tools/buildman/boards.py
index cdc4d9ffd27..0bb0723b18e 100644
--- a/tools/buildman/boards.py
+++ b/tools/buildman/boards.py
@@ -368,6 +368,17 @@ class MaintainersDatabase:
targets.append(front)
elif tag == 'S:':
status = rest
+elif tag == 'N:':
+# Just scan the configs directory since that's all we care
+# about
+for dirpath, _, fnames in os.walk('configs'):
+for fname in fnames:
+path = os.path.join(dirpath, fname)
+front, match, rear = path.partition('configs/')
+if not front and match:
+front, match, rear =
rear.rpartition('_defconfig')
+if match and not rear:
+targets.append(front)
elif line == '\n':
for target in targets:
self.database[target] = (status, maintainers)
--
2.38.0.rc1.362.ged0d419d3c-goog
[PATCH v2 0/5] Makefile: Deal with missing blobs consistently
Missing blobs should cause the build (with make) to fail, but at present success is returned. This is because binman currently produces an exit code of 0 in this case. Of course this is not correct, since the images cannot actually be used. This series fixes that and adjusts buildman to deal sensibly with the situation. Changes in v2: - Fix missing usage of the msg variable - Simplify code to remove the 'else' Simon Glass (5): buildman: Handle the MAINTAINERS 'N' tag Makefile: Correct a missing FORCE on the binman rule doc: Correct the path to the Makefile documentation binman: Use an exit code when blobs are missing buildman: Detect binman reporting missing blobs Makefile| 2 +- scripts/Kbuild.include | 2 +- tools/binman/binman.rst | 4 tools/binman/cmdline.py | 3 +++ tools/binman/control.py | 6 +- tools/buildman/boards.py| 11 +++ tools/buildman/builderthread.py | 6 +- 7 files changed, 30 insertions(+), 4 deletions(-) -- 2.38.0.rc1.362.ged0d419d3c-goog
Re: [PATCH 0/3] clang-14 fixes
Hi Tom, On Sat, 8 Oct 2022 at 21:33, Simon Glass wrote: > > This series contains a few clang-14 fixes to partly resolve the problems > shown by [1]. > > The event_dump problem is that the symbol name is not printed by > addr2line even though it seems to be present: > > Without LTO: > >$ addr2line -e u-boot.clang14.no-lto 4a37f >addr2line: DWARF error: invalid or unhandled FORM value: 0x23 >vbe_simple.c:? > >$ nm u-boot.clang14.no-lto |grep 4a37f >0004a37f t bootmeth_vbe_simple_ft_fixup > > With LTO is even worse: > >$ addr2line -e u-boot.clang14.lto 2d2ee0 >addr2line: DWARF error: invalid or unhandled FORM value: 0x23 >??:0 > >$ nm u-boot.clang14.lto |grep 4dad1 >0004dad1 t bootmeth_vbe_simple_ft_fixup > > Perhaps this is a bug in addr2line or the clang DWARF output? > > [1] https://source.denx.de/u-boot/u-boot/-/jobs/510282#L287 I should have mentioned that I can respin the series to deal with this, perhaps by not requiring the function name to be present when clang is used (or just at all?) Plus I have one more patch to add for the LTO stuff. Let me know what you think and I can respin this this. > > > Simon Glass (3): > dm: core: Fix lists_bind_fdt() using non-existent of_match > event: Drop the path when checking event-list filenames > test: Drop unwanted option in event_dump.py > > drivers/core/lists.c | 4 +++- > scripts/event_dump.py| 2 -- > test/py/tests/test_event_dump.py | 4 ++-- > 3 files changed, 5 insertions(+), 5 deletions(-) > > -- > 2.38.0.rc1.362.ged0d419d3c-goog > Regards, Simon
Re: [PATCH] spl: atf: Fix clang -Wasm-operand-widths warning
On Mon, Sep 26, 2022 at 08:47:55PM +, Alistair Delva wrote:
> common/spl/spl_atf.c:187:51: warning: value size does not match register
> size specified by the constraint and modifier [-Wasm-operand-widths]
> __asm__ __volatile__("msr DAIF, %0\n\t" : : "r" (daif) : "memory");
> ^
> common/spl/spl_atf.c:187:34: note: use constraint modifier "w"
> __asm__ __volatile__("msr DAIF, %0\n\t" : : "r" (daif) : "memory");
> ^~
> %w0
>
> Use %x0 to match what Linux does in write_sysreg().
>
> Signed-off-by: Alistair Delva
> Cc: Kever Yang
> Cc: Michael Walle
> Cc: Simon Glass
> Cc: Tom Rini
> Cc: Nick Desaulniers
Applied to u-boot/master, thanks!
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH] Makefile: apply dynamic relocations for LLD
On Mon, Sep 26, 2022 at 08:47:40PM +, Alistair Delva wrote: > From: Nick Desaulniers > > It seems that for aarch64, unless we apply dynamic relocations to the > location being relocated, we fail to boot. > > As Fangrui notes: > For dynamic relocations using the RELA format (readelf -Wr), GNU ld > sets the initial content to r_addend; ld.lld doesn't do that by > default (needs --apply-dynamic-relocs). > > Otherwise .rodata appears to be full of NUL-bytes before relocation, > causing crashes when trying to invoke the function pointers in > init_sequence_f from initcall_run_list(). > > Link: https://reviews.llvm.org/D42797 > Suggested-by: Fangrui Song > Signed-off-by: Nick Desaulniers > Signed-off-by: Alistair Delva > Cc: Simon Glass > Cc: Tom Rini > Cc: Nick Desaulniers Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] examples: standalone: Fix build with LLVM toolchain
On Mon, Sep 26, 2022 at 08:47:10PM +, Alistair Delva wrote: > When building the standalone example with llvm, the link step fails: > > examples/standalone/libstubs.o: In function `dummy': > include/_exports.h:10: undefined reference to `jt' > include/_exports.h:11: undefined reference to `jt' > include/_exports.h:12: undefined reference to `jt' > include/_exports.h:13: undefined reference to `jt' > include/_exports.h:14: undefined reference to `jt' > examples/standalone/libstubs.o:include/_exports.h:15: > more undefined references to `jt' follow > > Indeed, the standalone libstubs.o does use the jt symbol, but it was > marked 'static' in stubs.c. It's strange how gcc builds are working. > > Signed-off-by: Alistair Delva > Cc: Rick Chen > Cc: Simon Glass > Cc: Tom Rini > Cc: Nick Desaulniers Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH] fastboot: mmc: switch to user hwpart after erase/flash
Hi Sean,
Thank you for your review.
On Mon, Oct 10, 2022 at 11:38, Sean Anderson wrote:
> On 10/10/22 4:05 AM, Mattijs Korpershoek wrote:
>> Before erasing (or flashing) an mmc hardware partition, such as
>> mmc0boot0 or mmc0boot1, we select it using blk_dselect_hwpart().
>>
>> However, we don't select back the normal (userdata) hwpartition
>> afterwards.
>>
>> For example, the following sequence is broken:
>>
>> $ fastboot erase mmc2boot1# switch to hwpart 1
>> $ fastboot reboot bootloader # attempts to read GPT from hwpart 1
>>
>>> writing 128 blocks starting at 8064...
>>> wrote 65536 bytes to 'mmc0boot1'
>>> GUID Partition Table Header signature is wrong: 0xFB4EC30FC5B7E5B2 !=
>>> 0x5452415020494645
>>> find_valid_gpt: *** ERROR: Invalid GPT ***
>>> GUID Partition Table Header signature is wrong: 0x0 != 0x5452415020494645
>>> find_valid_gpt: *** ERROR: Invalid Backup GPT ***
>>> Error: mmc 2:misc read failed (-2)
>>
>> The GPT is stored in hwpart0 (userdata), not hwpart1 (mmc0boot1)
>
> This seems like a problem with your boot script. You should run `mmc dev
> ${mmcdev}` (and `mmc rescan`) before trying to access any MMC
> partitions. This is especially important after fastbooting, since the
> partition layout (or active hardware partition) may have changed.
I don't think I can fix this in my boot script.
My boot script runs => fastboot usb 0 which will start a loop to receive
fastboot commands from the host.
The full u-boot env i'm using is in include/configs/meson64_android.h
This loop will go on until "fastboot reboot " is called.
I do can "work around" this by using the following fastboot sequence:
$ fastboot erase mmc2boot1
$ fastboot oem format # switch backs to hwpart 0 (USER)
$ fastboot reboot bootloader
But that's not a good option to me. From a user's perspective, it should
not matter in which order we issue fastboot commands to the board.
Maybe I should fix bcb.c instead to make sure that it selects hwpart0
before reading/writing into the bcb partition ?
>
>> As the blk documentation states:
>>> Once selected only the region of the device
>>> covered by that partition is accessible.
>>
>> Add a cleanup function, fastboot_mmc_select_default_hwpart() which
>> selects back the default hwpartition (userdata).
>>
>> Note: this is more visible since
>> commit a362ce214f03 ("fastboot: Implement generic fastboot_set_reboot_flag")
>> because "fastboot reboot bootloader" will access the "misc" partition.
>>
>> Signed-off-by: Mattijs Korpershoek
>> ---
>> This fix has been used for over a year in the AOSP tree at [1]
>>
>> [1]
>> https://android.googlesource.com/device/amlogic/yukawa/+/c008ddaec0943adda394b229e83e147f6165773c
>> ---
>> drivers/fastboot/fb_mmc.c | 20 +---
>> include/mmc.h | 1 +
>> 2 files changed, 18 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/fastboot/fb_mmc.c b/drivers/fastboot/fb_mmc.c
>> index 033c510bc096..dedef7c4deb1 100644
>> --- a/drivers/fastboot/fb_mmc.c
>> +++ b/drivers/fastboot/fb_mmc.c
>> @@ -199,6 +199,12 @@ static void write_raw_image(struct blk_desc *dev_desc,
>> fastboot_okay(NULL, response);
>> }
>>
>> +static void fastboot_mmc_select_default_hwpart(struct blk_desc *dev_desc)
>> +{
>> +if (blk_dselect_hwpart(dev_desc, MMC_PART_USERDATA))
>
> If you really want to do this, we shoud save the current partition and
> restore if after fastbooting. Always switching to hardware partition 0
> is just as broken as the current behavior.
ACK. This patch just replaces one broken behaviour by another one. I
will see if I can save the hardware partition state instead or patch the
bcb command.
>
> --Sean
>
[u-boot][PATCH 14/14] mtd: rawnand: omap_elm: u-boot driver model support
Support u-boot driver model. We still retain
support legacy way of doing things if ELM_BASE
is defined in
We could completely get rid of that if all
platforms defining ELM_BASE get rid of that definition
and enable CONFIG_SYS_NAND_SELF_INIT and are verified
to work.
Signed-off-by: Roger Quadros
---
drivers/mtd/nand/raw/omap_elm.c | 33 ++-
.../mtd => drivers/mtd/nand/raw}/omap_elm.h | 6
drivers/mtd/nand/raw/omap_gpmc.c | 12 ++-
3 files changed, 49 insertions(+), 2 deletions(-)
rename {include/linux/mtd => drivers/mtd/nand/raw}/omap_elm.h (97%)
diff --git a/drivers/mtd/nand/raw/omap_elm.c b/drivers/mtd/nand/raw/omap_elm.c
index 35c6dd1f1b..7f4721f617 100644
--- a/drivers/mtd/nand/raw/omap_elm.c
+++ b/drivers/mtd/nand/raw/omap_elm.c
@@ -15,9 +15,14 @@
#include
#include
#include
-#include
#include
+#include
+#include
+#include
+
+#include "omap_elm.h"
+
#define DRIVER_NAME"omap-elm"
#define ELM_DEFAULT_POLY (0)
@@ -180,6 +185,7 @@ void elm_reset(void)
;
}
+#ifdef ELM_BASE
/**
* elm_init - Initialize ELM module
*
@@ -191,3 +197,28 @@ void elm_init(void)
elm_cfg = (struct elm *)ELM_BASE;
elm_reset();
}
+#endif
+
+static int elm_probe(struct udevice *dev)
+{
+ struct resource res;
+
+ dev_read_resource(dev, 0, );
+ elm_cfg = devm_ioremap(dev, res.start, resource_size());
+ elm_reset();
+
+ return 0;
+}
+
+static const struct udevice_id elm_ids[] = {
+ { .compatible = "ti,am3352-elm" },
+ { .compatible = "ti,am64-elm" },
+ { }
+};
+
+U_BOOT_DRIVER(gpmc_elm) = {
+ .name = DRIVER_NAME,
+ .id = UCLASS_MTD,
+ .of_match = elm_ids,
+ .probe = elm_probe,
+};
diff --git a/include/linux/mtd/omap_elm.h b/drivers/mtd/nand/raw/omap_elm.h
similarity index 97%
rename from include/linux/mtd/omap_elm.h
rename to drivers/mtd/nand/raw/omap_elm.h
index f3db00d55d..a7f7bacb15 100644
--- a/include/linux/mtd/omap_elm.h
+++ b/drivers/mtd/nand/raw/omap_elm.h
@@ -74,6 +74,12 @@ int elm_check_error(u8 *syndrome, enum bch_level bch_type,
u32 *error_count,
u32 *error_locations);
int elm_config(enum bch_level level);
void elm_reset(void);
+#ifdef ELM_BASE
void elm_init(void);
+#else
+static inline void elm_init(void)
+{
+}
+#endif
#endif /* __ASSEMBLY__ */
#endif /* __ASM_ARCH_ELM_H */
diff --git a/drivers/mtd/nand/raw/omap_gpmc.c b/drivers/mtd/nand/raw/omap_gpmc.c
index 79b14ce297..68f8d48e87 100644
--- a/drivers/mtd/nand/raw/omap_gpmc.c
+++ b/drivers/mtd/nand/raw/omap_gpmc.c
@@ -20,7 +20,8 @@
#include
#include
#include
-#include
+
+#include "omap_elm.h"
#ifndef GPMC_MAX_CS
#define GPMC_MAX_CS4
@@ -1248,6 +1249,15 @@ void board_nand_init(void)
struct udevice *dev;
int ret;
+#ifdef CONFIG_NAND_OMAP_ELM
+ ret = uclass_get_device_by_driver(UCLASS_MTD,
+ DM_DRIVER_GET(gpmc_elm), );
+ if (ret && ret != -ENODEV) {
+ pr_err("%s: Failed to get ELM device: %d\n", __func__, ret);
+ return;
+ }
+#endif
+
ret = uclass_get_device_by_driver(UCLASS_MTD,
DM_DRIVER_GET(gpmc_nand), );
if (ret && ret != -ENODEV)
--
2.17.1
[u-boot][PATCH 13/14] dt-bindings: mtd: Add ti, elm DT binding documentation
Adds DT binding documentation for the TI Error Location Module.
This is picked up from the Linux Kernel.
Signed-off-by: Roger Quadros
---
doc/device-tree-bindings/mtd/ti,elm.yaml | 72
1 file changed, 72 insertions(+)
create mode 100644 doc/device-tree-bindings/mtd/ti,elm.yaml
diff --git a/doc/device-tree-bindings/mtd/ti,elm.yaml
b/doc/device-tree-bindings/mtd/ti,elm.yaml
new file mode 100644
index 00..87128c0045
--- /dev/null
+++ b/doc/device-tree-bindings/mtd/ti,elm.yaml
@@ -0,0 +1,72 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/ti,elm.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments Error Location Module (ELM).
+
+maintainers:
+ - Roger Quadros
+
+description:
+ ELM module is used together with GPMC and NAND Flash to detect
+ errors and the location of the error based on BCH algorithms
+ so they can be corrected if possible.
+
+properties:
+ compatible:
+enum:
+ - ti,am3352-elm
+ - ti,am64-elm
+
+ reg:
+maxItems: 1
+
+ interrupts:
+maxItems: 1
+
+ clocks:
+maxItems: 1
+description: Functional clock.
+
+ clock-names:
+items:
+ - const: fck
+
+ power-domains:
+maxItems: 1
+
+ ti,hwmods:
+description:
+ Name of the HWMOD associated with ELM. This is for legacy
+ platforms only.
+$ref: /schemas/types.yaml#/definitions/string
+deprecated: true
+
+required:
+ - compatible
+ - reg
+ - interrupts
+
+allOf:
+ - if:
+ properties:
+compatible:
+ contains:
+const: ti,am64-elm
+then:
+ required:
+- clocks
+- clock-names
+- power-domains
+
+additionalProperties: false
+
+examples:
+ - |
+elm: ecc@0 {
+compatible = "ti,am3352-elm";
+reg = <0x0 0x2000>;
+interrupts = <4>;
+};
--
2.17.1
[u-boot][PATCH 11/14] mtd: rawnand: omap_gpmc: Add SPL NAND support
Enables SPL NAND support for ARCH_K3 by enabling
SPL_NAND_INIT and SPL_SYS_NAND_SELF_INIT.
Legacy OMAP2plus platforms still rely on SPL_NAND_AM33XX_BCH
instead.
Signed-off-by: Roger Quadros
---
drivers/mtd/nand/raw/Kconfig | 5
drivers/mtd/nand/raw/Makefile| 2 +-
drivers/mtd/nand/raw/omap_gpmc.c | 40
3 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index 1d23144ce4..b803759166 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -26,6 +26,9 @@ config TPL_SYS_NAND_SELF_INIT
config TPL_NAND_INIT
bool
+config SPL_NAND_INIT
+ bool
+
config SYS_NAND_DRIVER_ECC_LAYOUT
bool "Omit standard ECC layouts to save space"
help
@@ -191,6 +194,8 @@ config NAND_OMAP_GPMC
bool "Support OMAP GPMC NAND controller"
depends on ARCH_OMAP2PLUS || ARCH_KEYSTONE || ARCH_K3
select SYS_NAND_SELF_INIT if ARCH_K3
+ select SPL_NAND_INIT if ARCH_K3
+ select SPL_SYS_NAND_SELF_INIT if ARCH_K3
help
Enables omap_gpmc.c driver for OMAPx and AM platforms.
GPMC controller is used for parallel NAND flash devices, and can
diff --git a/drivers/mtd/nand/raw/Makefile b/drivers/mtd/nand/raw/Makefile
index a398aa9d88..6fe33d2485 100644
--- a/drivers/mtd/nand/raw/Makefile
+++ b/drivers/mtd/nand/raw/Makefile
@@ -18,7 +18,7 @@ obj-$(CONFIG_SPL_NAND_BASE) += nand_base.o nand_amd.o
nand_hynix.o \
nand_macronix.o nand_micron.o \
nand_samsung.o nand_toshiba.o
obj-$(CONFIG_SPL_NAND_IDENT) += nand_ids.o nand_timings.o
-obj-$(CONFIG_TPL_NAND_INIT) += nand.o
+obj-$(CONFIG_$(SPL_TPL_)NAND_INIT) += nand.o
ifeq ($(CONFIG_SPL_ENV_SUPPORT),y)
obj-$(CONFIG_ENV_IS_IN_NAND) += nand_util.o
endif
diff --git a/drivers/mtd/nand/raw/omap_gpmc.c b/drivers/mtd/nand/raw/omap_gpmc.c
index 7192ca9e5a..79b14ce297 100644
--- a/drivers/mtd/nand/raw/omap_gpmc.c
+++ b/drivers/mtd/nand/raw/omap_gpmc.c
@@ -1254,3 +1254,43 @@ void board_nand_init(void)
pr_err("%s: Failed to get GPMC device: %d\n", __func__, ret);
}
#endif /* CONFIG_SYS_NAND_SELF_INIT */
+
+#if defined(CONFIG_SPL_NAND_INIT)
+
+/* nand_init() is provided by nand.c */
+
+/* Unselect after operation */
+void nand_deselect(void)
+{
+ struct mtd_info *mtd = nand_to_mtd(nand_chip);
+
+ if (nand_chip->select_chip)
+ nand_chip->select_chip(mtd, -1);
+}
+
+static int nand_is_bad_block(int block)
+{
+ struct mtd_info *mtd = nand_to_mtd(nand_chip);
+
+ loff_t ofs = block * CONFIG_SYS_NAND_BLOCK_SIZE;
+
+ return nand_chip->block_bad(mtd, ofs);
+}
+
+static int nand_read_page(int block, int page, uchar *dst)
+{
+ int page_addr = block * CONFIG_SYS_NAND_PAGE_COUNT + page;
+ loff_t ofs = page_addr * CONFIG_SYS_NAND_PAGE_SIZE;
+ int ret;
+ size_t len = CONFIG_SYS_NAND_PAGE_SIZE;
+ struct mtd_info *mtd = nand_to_mtd(nand_chip);
+
+ ret = nand_read(mtd, ofs, , dst);
+ if (ret)
+ printf("nand_read failed %d\n", ret);
+
+ return ret;
+}
+
+#include "nand_spl_loaders.c"
+#endif /* CONFIG_SPL_NAND_INIT */
--
2.17.1
[u-boot][PATCH 12/14] mtd: rawnand: omap_gpmc: Enable SYS_NAND_PAGE_COUNT for OMAP_GPMC
The symbol is required for NAND support in SPL when using OMAP_GPMC driver. Signed-off-by: Roger Quadros --- drivers/mtd/nand/raw/Kconfig | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig index b803759166..95fe27c283 100644 --- a/drivers/mtd/nand/raw/Kconfig +++ b/drivers/mtd/nand/raw/Kconfig @@ -566,7 +566,8 @@ config SYS_NAND_ONFI_DETECTION config SYS_NAND_PAGE_COUNT hex "NAND chip page count" depends on SPL_NAND_SUPPORT && (NAND_ATMEL || NAND_MXC || \ - SPL_NAND_AM33XX_BCH || SPL_NAND_LOAD || SPL_NAND_SIMPLE) + SPL_NAND_AM33XX_BCH || SPL_NAND_LOAD || SPL_NAND_SIMPLE || \ + NAND_OMAP_GPMC) help Number of pages in the NAND chip. -- 2.17.1
[u-boot][PATCH 09/14] dt-bindings: mtd: Add ti, gpmc-nand DT binding documentation
Add DT binding documentation for the TI GPMC NAND controller.
This is picked up from the Linux Kernel.
Signed-off-by: Roger Quadros
---
.../mtd/ti,gpmc-nand.yaml | 129 ++
1 file changed, 129 insertions(+)
create mode 100644 doc/device-tree-bindings/mtd/ti,gpmc-nand.yaml
diff --git a/doc/device-tree-bindings/mtd/ti,gpmc-nand.yaml
b/doc/device-tree-bindings/mtd/ti,gpmc-nand.yaml
new file mode 100644
index 00..4ac198814b
--- /dev/null
+++ b/doc/device-tree-bindings/mtd/ti,gpmc-nand.yaml
@@ -0,0 +1,129 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mtd/ti,gpmc-nand.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments GPMC NAND Flash controller.
+
+maintainers:
+ - Tony Lindgren
+ - Roger Quadros
+
+description:
+ GPMC NAND controller/Flash is represented as a child of the
+ GPMC controller node.
+
+properties:
+ compatible:
+items:
+ - enum:
+ - ti,am64-nand
+ - ti,omap2-nand
+
+ reg:
+maxItems: 1
+
+ interrupts:
+items:
+ - description: Interrupt for fifoevent
+ - description: Interrupt for termcount
+
+ "#address-cells": true
+
+ "#size-cells": true
+
+ ti,nand-ecc-opt:
+description: Desired ECC algorithm
+$ref: /schemas/types.yaml#/definitions/string
+enum: [sw, ham1, bch4, bch8, bch16]
+
+ ti,nand-xfer-type:
+description: Data transfer method between controller and chip.
+$ref: /schemas/types.yaml#/definitions/string
+enum: [prefetch-polled, polled, prefetch-dma, prefetch-irq]
+default: prefetch-polled
+
+ ti,elm-id:
+description:
+ phandle to the ELM (Error Location Module).
+$ref: /schemas/types.yaml#/definitions/phandle
+
+ nand-bus-width:
+description:
+ Bus width to the NAND chip
+$ref: /schemas/types.yaml#/definitions/uint32
+enum: [8, 16]
+default: 8
+
+ rb-gpios:
+description:
+ GPIO connection to R/B signal from NAND chip
+maxItems: 1
+
+patternProperties:
+ "@[0-9a-f]+$":
+$ref: "/schemas/mtd/partitions/partition.yaml"
+
+allOf:
+ - $ref: "/schemas/memory-controllers/ti,gpmc-child.yaml"
+
+required:
+ - compatible
+ - reg
+ - ti,nand-ecc-opt
+
+unevaluatedProperties: false
+
+examples:
+ - |
+#include
+#include
+
+gpmc: memory-controller@5000 {
+ compatible = "ti,am3352-gpmc";
+ dmas = < 52 0>;
+ dma-names = "rxtx";
+ clocks = <_gclk>;
+ clock-names = "fck";
+ reg = <0x5000 0x2000>;
+ interrupts = ;
+ gpmc,num-cs = <7>;
+ gpmc,num-waitpins = <2>;
+ #address-cells = <2>;
+ #size-cells = <1>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ gpio-controller;
+ #gpio-cells = <2>;
+
+ ranges = <0 0 0x0800 0x0100>; /* CS0 space. Min partition =
16MB */
+ nand@0,0 {
+compatible = "ti,omap2-nand";
+reg = <0 0 4>; /* device IO registers */
+interrupt-parent = <>;
+interrupts = <0 IRQ_TYPE_NONE>, /* fifoevent */
+ <1 IRQ_TYPE_NONE>; /* termcount */
+ti,nand-xfer-type = "prefetch-dma";
+ti,nand-ecc-opt = "bch16";
+ti,elm-id = <>;
+#address-cells = <1>;
+#size-cells = <1>;
+
+/* NAND generic properties */
+nand-bus-width = <8>;
+rb-gpios = < 0 GPIO_ACTIVE_HIGH>; /* gpmc_wait0 */
+
+/* GPMC properties*/
+gpmc,device-width = <1>;
+
+partition@0 {
+ label = "NAND.SPL";
+ reg = <0x 0x0004>;
+};
+partition@1 {
+ label = "NAND.SPL.backup1";
+ reg = <0x0004 0x0004>;
+};
+ };
+};
--
2.17.1
[u-boot][PATCH 10/14] mtd: rawnand: omap_gpmc: support u-boot driver model
Adds driver model support.
We need to be able to self initialize the NAND controller/chip
at probe and so enable CONFIG_SYS_NAND_SELF_INIT.
Doing so requires nand_register() API which is provided by nand.c
and needs to be enabled during SPL build via CONFIG_SPL_NAND_INIT.
But nand.c also provides nand_init() so we need to get rid of nand_init()
in omap_gpmc driver if CONFIG_SPL_NAND_INIT is set.
Signed-off-by: Roger Quadros
---
drivers/mtd/nand/raw/Kconfig | 1 +
drivers/mtd/nand/raw/omap_gpmc.c | 55 +++-
2 files changed, 55 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig
index bc5cabdfc2..1d23144ce4 100644
--- a/drivers/mtd/nand/raw/Kconfig
+++ b/drivers/mtd/nand/raw/Kconfig
@@ -190,6 +190,7 @@ config NAND_LPC32XX_SLC
config NAND_OMAP_GPMC
bool "Support OMAP GPMC NAND controller"
depends on ARCH_OMAP2PLUS || ARCH_KEYSTONE || ARCH_K3
+ select SYS_NAND_SELF_INIT if ARCH_K3
help
Enables omap_gpmc.c driver for OMAPx and AM platforms.
GPMC controller is used for parallel NAND flash devices, and can
diff --git a/drivers/mtd/nand/raw/omap_gpmc.c b/drivers/mtd/nand/raw/omap_gpmc.c
index e772a914c8..7192ca9e5a 100644
--- a/drivers/mtd/nand/raw/omap_gpmc.c
+++ b/drivers/mtd/nand/raw/omap_gpmc.c
@@ -7,6 +7,7 @@
#include
#include
#include
+#include
#include
#ifdef CONFIG_ARCH_OMAP2PLUS
@@ -1121,7 +1122,7 @@ int __maybe_unused omap_nand_switch_ecc(uint32_t
hardware, uint32_t eccstrength)
* nand_scan about special functionality. See the defines for further
* explanation
*/
-int board_nand_init(struct nand_chip *nand)
+int gpmc_nand_init(struct nand_chip *nand)
{
int32_t gpmc_config = 0;
int cs = cs_next++;
@@ -1201,3 +1202,55 @@ int board_nand_init(struct nand_chip *nand)
return 0;
}
+
+static struct nand_chip *nand_chip;/* First NAND chip for SPL use only */
+
+#if CONFIG_IS_ENABLED(SYS_NAND_SELF_INIT)
+
+static int gpmc_nand_probe(struct udevice *dev)
+{
+ struct nand_chip *nand = dev_get_priv(dev);
+ struct mtd_info *mtd = nand_to_mtd(nand);
+ int ret;
+
+ gpmc_nand_init(nand);
+
+ ret = nand_scan(mtd, CONFIG_SYS_NAND_MAX_CHIPS);
+ if (ret)
+ return ret;
+
+ ret = nand_register(0, mtd);
+ if (ret)
+ return ret;
+
+ if (!nand_chip)
+ nand_chip = nand;
+
+ return 0;
+}
+
+static const struct udevice_id gpmc_nand_ids[] = {
+ { .compatible = "ti,am64-nand" },
+ { .compatible = "ti,omap2-nand" },
+ { }
+};
+
+U_BOOT_DRIVER(gpmc_nand) = {
+ .name = "gpmc-nand",
+ .id = UCLASS_MTD,
+ .of_match = gpmc_nand_ids,
+ .probe = gpmc_nand_probe,
+ .priv_auto = sizeof(struct nand_chip),
+};
+
+void board_nand_init(void)
+{
+ struct udevice *dev;
+ int ret;
+
+ ret = uclass_get_device_by_driver(UCLASS_MTD,
+ DM_DRIVER_GET(gpmc_nand), );
+ if (ret && ret != -ENODEV)
+ pr_err("%s: Failed to get GPMC device: %d\n", __func__, ret);
+}
+#endif /* CONFIG_SYS_NAND_SELF_INIT */
--
2.17.1
[u-boot][PATCH 08/14] mtd: rawnand: omap_gpmc: Reduce .bss usage
Allocate omap_ecclayout on the heap as we have
limited .bss space on AM64 R5 SPL configuration.
Reduces .bss usage by 2984 bytes.
Signed-off-by: Roger Quadros
---
drivers/mtd/nand/raw/omap_gpmc.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/mtd/nand/raw/omap_gpmc.c b/drivers/mtd/nand/raw/omap_gpmc.c
index b5ad66ad49..e772a914c8 100644
--- a/drivers/mtd/nand/raw/omap_gpmc.c
+++ b/drivers/mtd/nand/raw/omap_gpmc.c
@@ -40,7 +40,6 @@ static u8 bch8_polynomial[] = {0xef, 0x51, 0x2e, 0x09, 0xed,
0x93, 0x9a, 0xc2,
0x97, 0x79, 0xe5, 0x24, 0xb5};
#endif
static uint8_t cs_next;
-static __maybe_unused struct nand_ecclayout omap_ecclayout;
#if defined(CONFIG_NAND_OMAP_GPMC_WSCFG)
static const int8_t wscfg[CONFIG_SYS_MAX_NAND_DEVICE] =
@@ -874,7 +873,7 @@ static void __maybe_unused omap_free_bch(struct mtd_info
*mtd)
static int omap_select_ecc_scheme(struct nand_chip *nand,
enum omap_ecc ecc_scheme, unsigned int pagesize, unsigned int oobsize) {
struct omap_nand_info *info =
nand_get_controller_data(nand);
- struct nand_ecclayout *ecclayout = _ecclayout;
+ struct nand_ecclayout *ecclayout = nand->ecc.layout;
int eccsteps = pagesize / SECTOR_BYTES;
int i;
@@ -1167,7 +1166,9 @@ int board_nand_init(struct nand_chip *nand)
nand->cmd_ctrl = omap_nand_hwcontrol;
nand->options |= NAND_NO_PADDING | NAND_CACHEPRG;
nand->chip_delay = 100;
- nand->ecc.layout = _ecclayout;
+ nand->ecc.layout = kzalloc(sizeof(*nand->ecc.layout), GFP_KERNEL);
+ if (!nand->ecc.layout)
+ return -ENOMEM;
/* configure driver and controller based on NAND device bus-width */
gpmc_config = readl(_cfg->cs[cs].config1);
--
2.17.1
[u-boot][PATCH 05/14] mtd: rawnand: omap_gpmc: Fix BCH6/16 HW based correction
The BCH detection hardware can generate ECC bytes for multiple
sectors in one go. Use that feature.
correct() only corrects one sector at a time so we need to call it
repeatedly for each sector.
Signed-off-by: Roger Quadros
---
drivers/mtd/nand/raw/omap_gpmc.c | 325 +--
1 file changed, 223 insertions(+), 102 deletions(-)
diff --git a/drivers/mtd/nand/raw/omap_gpmc.c b/drivers/mtd/nand/raw/omap_gpmc.c
index b36fe762b3..b5ad66ad49 100644
--- a/drivers/mtd/nand/raw/omap_gpmc.c
+++ b/drivers/mtd/nand/raw/omap_gpmc.c
@@ -27,6 +27,9 @@
#define BADBLOCK_MARKER_LENGTH 2
#define SECTOR_BYTES 512
+#define ECCSIZE0_SHIFT 12
+#define ECCSIZE1_SHIFT 22
+#define ECC1RESULTSIZE 0x1
#define ECCCLEAR (0x1 << 8)
#define ECCRESULTREG1 (0x1 << 0)
/* 4 bit padding to make byte aligned, 56 = 52 + 4 */
@@ -187,72 +190,35 @@ static int __maybe_unused omap_correct_data(struct
mtd_info *mtd, uint8_t *dat,
__maybe_unused
static void omap_enable_hwecc(struct mtd_info *mtd, int32_t mode)
{
- struct nand_chip*nand = mtd_to_nand(mtd);
- struct omap_nand_info *info = nand_get_controller_data(nand);
+ struct nand_chip *nand = mtd_to_nand(mtd);
+ struct omap_nand_info *info = nand_get_controller_data(nand);
unsigned int dev_width = (nand->options & NAND_BUSWIDTH_16) ? 1 : 0;
- unsigned int ecc_algo = 0;
- unsigned int bch_type = 0;
- unsigned int eccsize1 = 0x00, eccsize0 = 0x00, bch_wrapmode = 0x00;
- u32 ecc_size_config_val = 0;
- u32 ecc_config_val = 0;
- int cs = info->cs;
+ u32 val;
- /* configure GPMC for specific ecc-scheme */
- switch (info->ecc_scheme) {
- case OMAP_ECC_HAM1_CODE_SW:
- return;
- case OMAP_ECC_HAM1_CODE_HW:
- ecc_algo = 0x0;
- bch_type = 0x0;
- bch_wrapmode = 0x00;
- eccsize0 = 0xFF;
- eccsize1 = 0xFF;
+ /* Clear ecc and enable bits */
+ writel(ECCCLEAR | ECCRESULTREG1, _cfg->ecc_control);
+
+ /* program ecc and result sizes */
+ val = nand->ecc.size >> 1) - 1) << ECCSIZE1_SHIFT) |
+ ECC1RESULTSIZE);
+ writel(val, _cfg->ecc_size_config);
+
+ switch (mode) {
+ case NAND_ECC_READ:
+ case NAND_ECC_WRITE:
+ writel(ECCCLEAR | ECCRESULTREG1, _cfg->ecc_control);
break;
- case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
- case OMAP_ECC_BCH8_CODE_HW:
- ecc_algo = 0x1;
- bch_type = 0x1;
- if (mode == NAND_ECC_WRITE) {
- bch_wrapmode = 0x01;
- eccsize0 = 0; /* extra bits in nibbles per sector */
- eccsize1 = 28; /* OOB bits in nibbles per sector */
- } else {
- bch_wrapmode = 0x01;
- eccsize0 = 26; /* ECC bits in nibbles per sector */
- eccsize1 = 2; /* non-ECC bits in nibbles per sector */
- }
- break;
- case OMAP_ECC_BCH16_CODE_HW:
- ecc_algo = 0x1;
- bch_type = 0x2;
- if (mode == NAND_ECC_WRITE) {
- bch_wrapmode = 0x01;
- eccsize0 = 0; /* extra bits in nibbles per sector */
- eccsize1 = 52; /* OOB bits in nibbles per sector */
- } else {
- bch_wrapmode = 0x01;
- eccsize0 = 52; /* ECC bits in nibbles per sector */
- eccsize1 = 0; /* non-ECC bits in nibbles per sector */
- }
+ case NAND_ECC_READSYN:
+ writel(ECCCLEAR, _cfg->ecc_control);
break;
default:
- return;
+ printf("%s: error: unrecognized Mode[%d]!\n", __func__, mode);
+ break;
}
- /* Clear ecc and enable bits */
- writel(ECCCLEAR | ECCRESULTREG1, _cfg->ecc_control);
- /* Configure ecc size for BCH */
- ecc_size_config_val = (eccsize1 << 22) | (eccsize0 << 12);
- writel(ecc_size_config_val, _cfg->ecc_size_config);
-
- /* Configure device details for BCH engine */
- ecc_config_val = ((ecc_algo << 16) | /* HAM1 | BCHx */
- (bch_type << 12)| /* BCH4/BCH8/BCH16 */
- (bch_wrapmode << 8) | /* wrap mode */
- (dev_width << 7)| /* bus width */
- (0x0 << 4) | /* number of sectors */
- (cs << 1) | /* ECC CS */
- (0x1)); /* enable ECC */
- writel(ecc_config_val, _cfg->ecc_config);
+
+ /* (ECC 16 or 8 bit col) | ( CS ) | ECC Enable */
+ val = (dev_width << 7) | (info->cs << 1) | (0x1);
+ writel(val,
[u-boot][PATCH 07/14] mtd: rawnand: nand_spl_loaders: Fix cast type build warning
Fixes the below build warning on 64-bit platforms.
drivers/mtd/nand/raw/nand_spl_loaders.c:26:21: warning: cast from pointer to
integer of different size [-Wpointer-to-int-cast]
dst = (void *)((int)dst - page_offset);
Signed-off-by: Roger Quadros
---
drivers/mtd/nand/raw/nand_spl_loaders.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/nand_spl_loaders.c
b/drivers/mtd/nand/raw/nand_spl_loaders.c
index 4befc75c04..156b44d835 100644
--- a/drivers/mtd/nand/raw/nand_spl_loaders.c
+++ b/drivers/mtd/nand/raw/nand_spl_loaders.c
@@ -23,7 +23,7 @@ int nand_spl_load_image(uint32_t offs, unsigned int size,
void *dst)
if (unlikely(page_offset)) {
memmove(dst, dst + page_offset,
CONFIG_SYS_NAND_PAGE_SIZE);
- dst = (void *)((int)dst - page_offset);
+ dst = (void *)(dst - page_offset);
page_offset = 0;
}
dst += CONFIG_SYS_NAND_PAGE_SIZE;
--
2.17.1
[u-boot][PATCH 06/14] mtd: rawnand: nand_base: Allow base driver to be used in SPL without nand_bbt
nand_bbt.c is not being built with the nand_base driver during SPL
build. This results in build failures if we try to access any nand_bbt
related functions.
Don't use any nand_bbt functions for SPL build.
Signed-off-by: Roger Quadros
---
drivers/mtd/nand/raw/nand_base.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 4b09a11288..826ae633ce 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -447,7 +447,10 @@ static int nand_default_block_markbad(struct mtd_info
*mtd, loff_t ofs)
static int nand_block_markbad_lowlevel(struct mtd_info *mtd, loff_t ofs)
{
struct nand_chip *chip = mtd_to_nand(mtd);
- int res, ret = 0;
+ int ret = 0;
+#ifndef CONFIG_SPL_BUILD
+ int res;
+#endif
if (!(chip->bbt_options & NAND_BBT_NO_OOB_BBM)) {
struct erase_info einfo;
@@ -465,12 +468,14 @@ static int nand_block_markbad_lowlevel(struct mtd_info
*mtd, loff_t ofs)
nand_release_device(mtd);
}
+#ifndef CONFIG_SPL_BUILD
/* Mark block bad in BBT */
if (chip->bbt) {
res = nand_markbad_bbt(mtd, ofs);
if (!ret)
ret = res;
}
+#endif
if (!ret)
mtd->ecc_stats.badblocks++;
@@ -517,7 +522,11 @@ static int nand_block_isreserved(struct mtd_info *mtd,
loff_t ofs)
if (!chip->bbt)
return 0;
/* Return info from the table */
+#ifndef CONFIG_SPL_BUILD
return nand_isreserved_bbt(mtd, ofs);
+#else
+ return 0;
+#endif
}
/**
@@ -543,7 +552,11 @@ static int nand_block_checkbad(struct mtd_info *mtd,
loff_t ofs, int allowbbt)
return chip->block_bad(mtd, ofs);
/* Return info from the table */
+#ifndef CONFIG_SPL_BUILD
return nand_isbad_bbt(mtd, ofs, allowbbt);
+#else
+ return 0;
+#endif
}
/**
@@ -3752,8 +3765,11 @@ static void nand_set_defaults(struct nand_chip *chip,
int busw)
chip->write_byte = busw ? nand_write_byte16 : nand_write_byte;
if (!chip->read_buf || chip->read_buf == nand_read_buf)
chip->read_buf = busw ? nand_read_buf16 : nand_read_buf;
+
+#ifndef CONFIG_SPL_BUILD
if (!chip->scan_bbt)
chip->scan_bbt = nand_default_bbt;
+#endif
if (!chip->controller) {
chip->controller = >hwcontrol;
--
2.17.1
[u-boot][PATCH 04/14] mtd: rawnand: omap_gpmc: Optimize NAND reads
Rename omap_nand_read() to omap_nand_read_buf() to reflect
actual behaviour.
Use FIFO read address instead of raw read address for reads.
The GPMC automatically converts 32-bit/16-bit reads to NAND
device specific reads (8/16 bit). Use the largest possible
read granularity size for more efficient reads.
Signed-off-by: Roger Quadros
---
drivers/mtd/nand/raw/omap_gpmc.c | 49 ++--
1 file changed, 28 insertions(+), 21 deletions(-)
diff --git a/drivers/mtd/nand/raw/omap_gpmc.c b/drivers/mtd/nand/raw/omap_gpmc.c
index d62c3e6fce..b36fe762b3 100644
--- a/drivers/mtd/nand/raw/omap_gpmc.c
+++ b/drivers/mtd/nand/raw/omap_gpmc.c
@@ -55,6 +55,7 @@ struct omap_nand_info {
enum omap_ecc ecc_scheme;
uint8_t cs;
uint8_t ws; /* wait status pin (0,1) */
+ void __iomem *fifo;
};
/* We are wasting a bit of memory but al least we are safe */
@@ -350,6 +351,20 @@ static int omap_calculate_ecc(struct mtd_info *mtd, const
uint8_t *dat,
return 0;
}
+static inline void omap_nand_read_buf(struct mtd_info *mtd, uint8_t *buf, int
len)
+{
+ struct nand_chip *chip = mtd_to_nand(mtd);
+ struct omap_nand_info *info = nand_get_controller_data(chip);
+ u32 alignment = ((uintptr_t)buf | len) & 3;
+
+ if (alignment & 1)
+ readsb(info->fifo, buf, len);
+ else if (alignment & 3)
+ readsw(info->fifo, buf, len >> 1);
+ else
+ readsl(info->fifo, buf, len >> 2);
+}
+
#ifdef CONFIG_NAND_OMAP_GPMC_PREFETCH
#define PREFETCH_CONFIG1_CS_SHIFT 24
@@ -415,7 +430,7 @@ static int __read_prefetch_aligned(struct nand_chip *chip,
uint32_t *buf, int le
cnt = PREFETCH_STATUS_FIFO_CNT(cnt);
for (i = 0; i < cnt / 4; i++) {
- *buf++ = readl(CONFIG_SYS_NAND_BASE);
+ *buf++ = readl(info->fifo);
len -= 4;
}
} while (len);
@@ -425,16 +440,6 @@ static int __read_prefetch_aligned(struct nand_chip *chip,
uint32_t *buf, int le
return 0;
}
-static inline void omap_nand_read(struct mtd_info *mtd, uint8_t *buf, int len)
-{
- struct nand_chip *chip = mtd_to_nand(mtd);
-
- if (chip->options & NAND_BUSWIDTH_16)
- nand_read_buf16(mtd, buf, len);
- else
- nand_read_buf(mtd, buf, len);
-}
-
static void omap_nand_read_prefetch(struct mtd_info *mtd, uint8_t *buf, int
len)
{
int ret;
@@ -447,7 +452,7 @@ static void omap_nand_read_prefetch(struct mtd_info *mtd,
uint8_t *buf, int len)
*/
head = ((uintptr_t)buf) % 4;
if (head) {
- omap_nand_read(mtd, buf, head);
+ omap_nand_read_buf(mtd, buf, head);
buf += head;
len -= head;
}
@@ -461,10 +466,10 @@ static void omap_nand_read_prefetch(struct mtd_info *mtd,
uint8_t *buf, int len)
ret = __read_prefetch_aligned(chip, (uint32_t *)buf, len - tail);
if (ret < 0) {
/* fallback in case the prefetch engine is busy */
- omap_nand_read(mtd, buf, len);
+ omap_nand_read_buf(mtd, buf, len);
} else if (tail) {
buf += len - tail;
- omap_nand_read(mtd, buf, tail);
+ omap_nand_read_buf(mtd, buf, tail);
}
}
#endif /* CONFIG_NAND_OMAP_GPMC_PREFETCH */
@@ -1001,6 +1006,8 @@ int board_nand_init(struct nand_chip *nand)
int32_t gpmc_config = 0;
int cs = cs_next++;
int err = 0;
+ struct omap_nand_info *info;
+
/*
* xloader/Uboot's gpmc configuration would have configured GPMC for
* nand type of memory. The following logic scans and latches on to the
@@ -1029,9 +1036,12 @@ int board_nand_init(struct nand_chip *nand)
nand->IO_ADDR_R = (void __iomem *)_cfg->cs[cs].nand_dat;
nand->IO_ADDR_W = (void __iomem *)_cfg->cs[cs].nand_cmd;
- omap_nand_info[cs].control = NULL;
- omap_nand_info[cs].cs = cs;
- omap_nand_info[cs].ws = wscfg[cs];
+
+ info = _nand_info[cs];
+ info->control = NULL;
+ info->cs = cs;
+ info->ws = wscfg[cs];
+ info->fifo = (void __iomem *)CONFIG_SYS_NAND_BASE;
nand_set_controller_data(nand, _nand_info[cs]);
nand->cmd_ctrl = omap_nand_hwcontrol;
nand->options |= NAND_NO_PADDING | NAND_CACHEPRG;
@@ -1062,10 +1072,7 @@ int board_nand_init(struct nand_chip *nand)
#ifdef CONFIG_NAND_OMAP_GPMC_PREFETCH
nand->read_buf = omap_nand_read_prefetch;
#else
- if (nand->options & NAND_BUSWIDTH_16)
- nand->read_buf = nand_read_buf16;
- else
- nand->read_buf = nand_read_buf;
+ nand->read_buf = omap_nand_read_buf;
#endif
nand->dev_ready = omap_dev_ready;
--
2.17.1
[u-boot][PATCH 03/14] mtd: rawnand: omap_gpmc: Fix build warning on 64-bit platforms
Pointer size cannot be assumed to be 32-bit, so use
use uintptr_t instead of uint32_t.
Fixes the below build warning on 64-bit builds.
drivers/mtd/nand/raw/omap_gpmc.c:439:10: warning: cast from pointer to integer
of different size [-Wpointer-to-int-cast]
head = ((uint32_t) buf) % 4;
Signed-off-by: Roger Quadros
---
drivers/mtd/nand/raw/omap_gpmc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/mtd/nand/raw/omap_gpmc.c b/drivers/mtd/nand/raw/omap_gpmc.c
index 7e9ccf7878..d62c3e6fce 100644
--- a/drivers/mtd/nand/raw/omap_gpmc.c
+++ b/drivers/mtd/nand/raw/omap_gpmc.c
@@ -438,14 +438,14 @@ static inline void omap_nand_read(struct mtd_info *mtd,
uint8_t *buf, int len)
static void omap_nand_read_prefetch(struct mtd_info *mtd, uint8_t *buf, int
len)
{
int ret;
- uint32_t head, tail;
+ uintptr_t head, tail;
struct nand_chip *chip = mtd_to_nand(mtd);
/*
* If the destination buffer is unaligned, start with reading
* the overlap byte-wise.
*/
- head = ((uint32_t) buf) % 4;
+ head = ((uintptr_t)buf) % 4;
if (head) {
omap_nand_read(mtd, buf, head);
buf += head;
--
2.17.1
[u-boot][PATCH 02/14] mtd: rawnand: omap_gpmc: Enable build for K2/K3 platforms
The GPMC module is present on some K2 and K3 SoCs. Enable building GPMC NAND driver for K2/K3 platforms. Signed-off-by: Roger Quadros --- drivers/mtd/nand/raw/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/mtd/nand/raw/Kconfig b/drivers/mtd/nand/raw/Kconfig index ce67d1abde..bc5cabdfc2 100644 --- a/drivers/mtd/nand/raw/Kconfig +++ b/drivers/mtd/nand/raw/Kconfig @@ -189,7 +189,7 @@ config NAND_LPC32XX_SLC config NAND_OMAP_GPMC bool "Support OMAP GPMC NAND controller" - depends on ARCH_OMAP2PLUS + depends on ARCH_OMAP2PLUS || ARCH_KEYSTONE || ARCH_K3 help Enables omap_gpmc.c driver for OMAPx and AM platforms. GPMC controller is used for parallel NAND flash devices, and can -- 2.17.1
[u-boot][PATCH 01/14] mtd: rawnand: omap_gpmc: Deprecate asm/arch/mem.h
We want to get rid of so don't enforce it for new platforms. This also means GPMC_MAX CS doesn't have to be defined by platform code. Define it locally here for now. Signed-off-by: Roger Quadros --- drivers/mtd/nand/raw/omap_gpmc.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/mtd/nand/raw/omap_gpmc.c b/drivers/mtd/nand/raw/omap_gpmc.c index 8b9ff4de18..7e9ccf7878 100644 --- a/drivers/mtd/nand/raw/omap_gpmc.c +++ b/drivers/mtd/nand/raw/omap_gpmc.c @@ -8,7 +8,11 @@ #include #include #include + +#ifdef CONFIG_ARCH_OMAP2PLUS #include +#endif + #include #include #include @@ -17,6 +21,10 @@ #include #include +#ifndef GPMC_MAX_CS +#define GPMC_MAX_CS4 +#endif + #define BADBLOCK_MARKER_LENGTH 2 #define SECTOR_BYTES 512 #define ECCCLEAR (0x1 << 8) -- 2.17.1
[u-boot][PATCH 00/14] rawnand: omap_gpmc: driver model support
Hi,
This series adds driver model support for rawnand: omap_gpmc
and omap_elm drivers.
This will enable the driver to be used on K2/K3 platforms as well.
cheers,
-roger
Roger Quadros (14):
mtd: rawnand: omap_gpmc: Deprecate asm/arch/mem.h
mtd: rawnand: omap_gpmc: Enable build for K2/K3 platforms
mtd: rawnand: omap_gpmc: Fix build warning on 64-bit platforms
mtd: rawnand: omap_gpmc: Optimize NAND reads
mtd: rawnand: omap_gpmc: Fix BCH6/16 HW based correction
mtd: rawnand: nand_base: Allow base driver to be used in SPL without
nand_bbt
mtd: rawnand: nand_spl_loaders: Fix cast type build warning
mtd: rawnand: omap_gpmc: Reduce .bss usage
dt-bindings: mtd: Add ti,gpmc-nand DT binding documentation
mtd: rawnand: omap_gpmc: support u-boot driver model
mtd: rawnand: omap_gpmc: Add SPL NAND support
mtd: rawnand: omap_gpmc: Enable SYS_NAND_PAGE_COUNT for OMAP_GPMC
dt-bindings: mtd: Add ti,elm DT binding documentation
mtd: rawnand: omap_elm: u-boot driver model support
doc/device-tree-bindings/mtd/ti,elm.yaml | 72 +++
.../mtd/ti,gpmc-nand.yaml | 129 +
drivers/mtd/nand/raw/Kconfig | 11 +-
drivers/mtd/nand/raw/Makefile | 2 +-
drivers/mtd/nand/raw/nand_base.c | 18 +-
drivers/mtd/nand/raw/nand_spl_loaders.c | 2 +-
drivers/mtd/nand/raw/omap_elm.c | 33 +-
.../mtd => drivers/mtd/nand/raw}/omap_elm.h | 6 +
drivers/mtd/nand/raw/omap_gpmc.c | 500 +-
9 files changed, 637 insertions(+), 136 deletions(-)
create mode 100644 doc/device-tree-bindings/mtd/ti,elm.yaml
create mode 100644 doc/device-tree-bindings/mtd/ti,gpmc-nand.yaml
rename {include/linux/mtd => drivers/mtd/nand/raw}/omap_elm.h (97%)
--
2.17.1
[GIT PULL] xilinx patches for v2023.01-rc1 (round 3)
Hi Tom, please pull the following patches to your tree. Based on discussion with Simon I have also include fpga uclass series which was reviewed by him. There is also one gcc12 patch we discussed over IRC which was fixed by Heinrich. Thanks, Michal The following changes since commit 2d4591353452638132d711551fec3495b7644731: Merge branch 'next' (2022-10-03 15:39:46 -0400) are available in the Git repository at: g...@source.denx.de:u-boot/custodians/u-boot-microblaze.git tags/xilinx-for-v2023.01-rc1-v3 for you to fetch changes up to 63c46e028c14254f28332b3bd57fc3202e26b10a: fpga: virtex2: Use logging feature instead of FPGA_DEBUG (2022-10-10 12:28:08 +0200) Xilinx changes for v2023.01-rc1 (round 3) fpga: - Create new uclass - Get rid of FPGA_DEBUG and use logging infrastructure zynq: - Enable early EEPROM decoding - Some DT updates zynqmp: - Use OCM_BANK_0 to check config loading permission - Change config object loading in SPL - Some DT updates net: - emaclite: Enable driver for RISC-V xilinx: - Fix static checker warnings - Fix GCC12 warning sdhci: - Read PD id from DT Alexander Dahl (11): dm: fpga: Introduce new uclass fpga: Add missing Kconfig symbols for old FPGA drivers fpga: spartan2: Fix printf arguments warning fpga: spartan3: Fix printf arguments warning fpga: virtex2: Fix printf format string warnings fpga: altera: Use logging feature instead of FPGA_DEBUG fpga: cyclon2: Use logging feature instead of FPGA_DEBUG fpga: ACEX1K: Use logging feature instead of FPGA_DEBUG fpga: spartan2: Use logging feature instead of FPGA_DEBUG fpga: spartan3: Use logging feature instead of FPGA_DEBUG fpga: virtex2: Use logging feature instead of FPGA_DEBUG Ashok Reddy Soma (4): firmware: zynqmp: Change loadable config object from APU_0 to OCM_BANK_0 mmc: zynq_sdhci: Change node_id prototype to u32 mmc: zynq_sdhci: Read power-domains id from DT and use arm64: dts: Remove unused property device_id Heinrich Schuchardt (1): xilinx: common: fix board_late_init_xilinx() Michal Simek (4): xilinx: zynq: Enable early eeprom decoding ARM: zynq: Point via nvmem0 alias to eeprom on zc702/zc706 ARM: zynq: Define rtc alias on zc702/zc706 xilinx: zynqmp: Load pmufw configuration before checking access Samuel Obuch (3): net: emaclite: enable for more architectures net: emaclite: fix xemaclite_alignedread/write functions net: emaclite: fix handling for IP packets with specific lengths Venkatesh Yadav Abbarapu (8): xilinx: common: Fix static checker warnings net: Fix static checker warnings arm64: zynqmp: Fix compiler warnings in mp.c spi: zynqmp_qspi: Mark zynqmp_qspi_set_tapdelay() as static xilinx: common: Add print_cpuinfo() declaration soc: xilinx: zynqmp: Mark soc_xilinx_zynqmp_get_machine() as static xilinx: zynqmp: change the type of multiboot variable clk: versal: Mark versal_clock_setup() as static MAINTAINERS| 1 + arch/arm/dts/versal-mini-emmc0.dts | 1 - arch/arm/dts/versal-mini-emmc1.dts | 1 - arch/arm/dts/zynq-zc702.dts| 6 +- arch/arm/dts/zynq-zc706.dts| 6 +- arch/arm/dts/zynqmp-mini-emmc0.dts | 1 - arch/arm/dts/zynqmp-mini-emmc1.dts | 1 - arch/arm/dts/zynqmp.dtsi | 2 - arch/arm/mach-zynqmp/mp.c | 6 +- arch/sandbox/dts/test.dts | 4 + board/xilinx/common/board.c| 9 +-- board/xilinx/common/cpu-info.c | 1 + board/xilinx/zynq/board.c | 3 + board/xilinx/zynqmp/zynqmp.c | 14 ++-- drivers/clk/clk_versal.c | 2 +- drivers/firmware/firmware-zynqmp.c | 11 ++- drivers/fpga/ACEX1K.c | 37 - drivers/fpga/Kconfig | 31 drivers/fpga/Makefile | 3 + drivers/fpga/altera.c | 11 +-- drivers/fpga/cyclon2.c | 38 -- drivers/fpga/fpga-uclass.c | 11 +++ drivers/fpga/sandbox.c | 17 + drivers/fpga/spartan2.c| 80 +--- drivers/fpga/spartan3.c| 80 +--- drivers/fpga/virtex2.c | 69 - drivers/mmc/zynq_sdhci.c | 53 ++--- drivers/net/xilinx_axi_emac.c | 5 ++ drivers/net/xilinx_emaclite.c | 20 ++--- drivers/net/zynq_gem.c | 2 +- drivers/soc/soc_xilinx_zynqmp.c| 2 +- drivers/spi/zynqmp_gqspi.c | 2 +- include/dm/uclass-id.h | 1 + test/dm/Makefile | 1 + test/dm/fpga.c | 20 + 35 files changed, 310 insertions(+), 242 deletions(-) create mode 100644 drivers/fpga/fpga-uclass.c create mode 100644 drivers/fpga/sandbox.c create mode 100644 test/dm/fpga.c -- Michal
Re: [PATCH 1/1] efi_loader: CloseProtocol in efi_fmp_find
On 10/11/22 13:08, Heinrich Schuchardt wrote:
On 10/11/22 09:35, AKASHI Takahiro wrote:
On Tue, Oct 11, 2022 at 07:58:11AM +0200, Heinrich Schuchardt wrote:
On 10/11/22 02:49, AKASHI Takahiro wrote:
The commit message is not accurate.
On Fri, Oct 07, 2022 at 04:06:23PM +0200, Heinrich Schuchardt wrote:
The CloseProtocol() boot service requires a handle as first argument.
Passing the protocol interface is incorrect.
Correct, but
CloseProtocol() only has an effect if called with a non-zero value for
agent_handle. HandleProtocol() uses an opaque agent_handle when
invoking
OpenProtocol() (currently NULL).
No. OpenProtocol() is called with efi_root as an agent handle.
So, calling CloseProtocol() is a right thing at the end.
Typically an agent handle is used to relate to a driver exposing the
driver
binding protocol.
Why can't we, other than a driver, call HandleProtocol()
as a convenient way of accessing an interface?
The description of HandleProtocol() clearly says that it is deprecated.
The assumption that the UEFI specification makes in it is example code
that you never be able to close a protocol opened with HandleProtocol.
After the first usage of handle protocol the open protocol information
with the opaque agent handle will block the protocol interface from ever
being removed by the driver exposing it.
The root node does not expose the driver binding protocol.
So do you mean the current implementation of HandleProtocol() is wrong?
Yes. If you ever install a boot time driver, it might remove a protocol
interface which is actually still in use.
Since 755d42d4209e ("efi_loader: correct HandleProtocol()") we set agent
handle = efi_root in the implementation of HandleProtocol(). So this
part is ok.
Best regards
Heirnich
Why would you want to create an open protocol information entry here?
To access get_image_info() quickly.
This is not related to an open protocol information (see the UEFI spec
description of OpenProtocolInformation()).
Best regards
Heinrich
Do you think anything with the code after the patch is wrong?
No reason to replace handle_protocol().
Another example is here:
efi_load_image_from_path()
efi_handle_protocol(device, guid, (void **)_file_protocol));
...
efi_close_protocol(device, guid, efi_root, NULL);
I believe that this function is anything but a driver.
I think using HandleProtocol() (or preferably OpenProtocol()) and
CloseProtocol()
in pair seems totally sane.
-Takahiro Akashi
Best regards
Heinrich
Therefore HandleProtocol() should be
avoided.
* Replace the LocateHandle() call by efi_search_protocol().
LocateHandle() -> efi_handle_protocol()
So you could have fixed this way:
EFI_CALL(efi_close_protocol(handle, ..., _root, NULL);
I preferred to use EFI_CALL() over this file as you can see.
-Takahiro Akashi
* Remove the CloseProtocol() call.
Fixes: 8d99026f0697 ("efi_loader: capsule: support firmware update")
Signed-off-by: Heinrich Schuchardt
---
lib/efi_loader/efi_capsule.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/lib/efi_loader/efi_capsule.c
b/lib/efi_loader/efi_capsule.c
index b6bd2d6af8..397e393a18 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -159,12 +159,14 @@ efi_fmp_find(efi_guid_t *image_type, u8
image_index, u64 instance,
efi_status_t ret;
for (i = 0, handle = handles; i < no_handles; i++, handle++) {
- ret = EFI_CALL(efi_handle_protocol(
- *handle,
- _guid_firmware_management_protocol,
- (void **)));
+ struct efi_handler *fmp_handler;
+
+ ret = efi_search_protocol(
+ *handle, _guid_firmware_management_protocol,
+ _handler);
if (ret != EFI_SUCCESS)
continue;
+ fmp = fmp_handler->protocol_interface;
/* get device's image info */
info_size = 0;
@@ -215,10 +217,6 @@ efi_fmp_find(efi_guid_t *image_type, u8
image_index, u64 instance,
skip:
efi_free_pool(package_version_name);
free(image_info);
- EFI_CALL(efi_close_protocol(
- (efi_handle_t)fmp,
- _guid_firmware_management_protocol,
- NULL, NULL));
if (found)
return fmp;
}
--
2.37.2
Re: [PATCH 1/1] efi_loader: CloseProtocol in efi_fmp_find
On 10/11/22 09:35, AKASHI Takahiro wrote:
On Tue, Oct 11, 2022 at 07:58:11AM +0200, Heinrich Schuchardt wrote:
On 10/11/22 02:49, AKASHI Takahiro wrote:
The commit message is not accurate.
On Fri, Oct 07, 2022 at 04:06:23PM +0200, Heinrich Schuchardt wrote:
The CloseProtocol() boot service requires a handle as first argument.
Passing the protocol interface is incorrect.
Correct, but
CloseProtocol() only has an effect if called with a non-zero value for
agent_handle. HandleProtocol() uses an opaque agent_handle when invoking
OpenProtocol() (currently NULL).
No. OpenProtocol() is called with efi_root as an agent handle.
So, calling CloseProtocol() is a right thing at the end.
Typically an agent handle is used to relate to a driver exposing the driver
binding protocol.
Why can't we, other than a driver, call HandleProtocol()
as a convenient way of accessing an interface?
The description of HandleProtocol() clearly says that it is deprecated.
The assumption that the UEFI specification makes in it is example code
that you never be able to close a protocol opened with HandleProtocol.
After the first usage of handle protocol the open protocol information
with the opaque agent handle will block the protocol interface from ever
being removed by the driver exposing it.
The root node does not expose the driver binding protocol.
So do you mean the current implementation of HandleProtocol() is wrong?
Yes. If you ever install a boot time driver, it might remove a protocol
interface which is actually still in use.
Why would you want to create an open protocol information entry here?
To access get_image_info() quickly.
This is not related to an open protocol information (see the UEFI spec
description of OpenProtocolInformation()).
Best regards
Heinrich
Do you think anything with the code after the patch is wrong?
No reason to replace handle_protocol().
Another example is here:
efi_load_image_from_path()
efi_handle_protocol(device, guid, (void **)_file_protocol));
...
efi_close_protocol(device, guid, efi_root, NULL);
I believe that this function is anything but a driver.
I think using HandleProtocol() (or preferably OpenProtocol()) and
CloseProtocol()
in pair seems totally sane.
-Takahiro Akashi
Best regards
Heinrich
Therefore HandleProtocol() should be
avoided.
* Replace the LocateHandle() call by efi_search_protocol().
LocateHandle() -> efi_handle_protocol()
So you could have fixed this way:
EFI_CALL(efi_close_protocol(handle, ..., _root, NULL);
I preferred to use EFI_CALL() over this file as you can see.
-Takahiro Akashi
* Remove the CloseProtocol() call.
Fixes: 8d99026f0697 ("efi_loader: capsule: support firmware update")
Signed-off-by: Heinrich Schuchardt
---
lib/efi_loader/efi_capsule.c | 14 ++
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index b6bd2d6af8..397e393a18 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -159,12 +159,14 @@ efi_fmp_find(efi_guid_t *image_type, u8 image_index, u64
instance,
efi_status_t ret;
for (i = 0, handle = handles; i < no_handles; i++, handle++) {
- ret = EFI_CALL(efi_handle_protocol(
- *handle,
- _guid_firmware_management_protocol,
- (void **)));
+ struct efi_handler *fmp_handler;
+
+ ret = efi_search_protocol(
+ *handle, _guid_firmware_management_protocol,
+ _handler);
if (ret != EFI_SUCCESS)
continue;
+ fmp = fmp_handler->protocol_interface;
/* get device's image info */
info_size = 0;
@@ -215,10 +217,6 @@ efi_fmp_find(efi_guid_t *image_type, u8 image_index, u64
instance,
skip:
efi_free_pool(package_version_name);
free(image_info);
- EFI_CALL(efi_close_protocol(
- (efi_handle_t)fmp,
- _guid_firmware_management_protocol,
- NULL, NULL));
if (found)
return fmp;
}
--
2.37.2
Re: [PATCH 1/1] dm: fix blk_get_devnum_by_uclass_idname()
On 10/11/22 07:46, Heinrich Schuchardt wrote:
On 10/11/22 01:49, Simon Glass wrote:
Hi Heinrich,
On Thu, 6 Oct 2022 at 14:05, Heinrich Schuchardt
wrote:
On 10/3/22 18:44, Simon Glass wrote:
Hi Heinrich,
On Mon, 3 Oct 2022 at 10:33, Heinrich Schuchardt
wrote:
On 10/3/22 16:57, Simon Glass wrote:
Hi Heinrich,
On Mon, 3 Oct 2022 at 03:36, Heinrich Schuchardt
wrote:
On the sandbox I run:
=> setenv efi_selftest block device
=> bootefi selftest
and see the following output:
** Bad device specification host 0 **
Couldn't find partition host 0:0
Cannot read EFI system partition
Running
=> lsblk
yields
Block Driver Devices
-
efi_blk : efiloader 0
ide_blk :
mmc_blk : mmc 2, mmc 1, mmc 0
nvme-blk :
sandbox_host_blk :
scsi_blk :
usb_storage_blk :
virtio-blk :
So a efi_blk device was mistaken for a host device.
I continue with
=> host bind 0 ../sandbox.img
=> ls host 0:1
and get the following output:
13 hello.txt
7 u-boot.txt
2 file(s), 0 dir(s)
This is the content of efiblock 0:1 and not of host 0:1 (sic!).
The uclass of the parent device is irrelevant for the
determination of the
uclass of the block device. We must use the uclass stored in the
block
device descriptor.
This issue has been raised repeatedly:
[PATCH 1/1] block: fix blk_get_devnum_by_typename()
https://lore.kernel.org/u-boot/20220802094933.69170-1-heinrich.schucha...@canonical.com/
[PATCH 1/1] blk: simplify blk_get_devnum_by_typename()
https://lore.kernel.org/u-boot/20211023140647.7661-1-heinrich.schucha...@canonical.com/
Yes and you were not able/willing to take on the required work, so
this carried on longer than it should have. I finally did this myself
and it is now in -next.
The refactoring was orthogonal to the problem that I reported and
which
you unfortunately did not consider in the process.
Well it involved using if_type to work around a problem, just making
it harder to get rid of. Overall I am in favour of a faster pace of
migration that we have been following and it would help if people took
on some of this, instead of fixing their little issue.
So we might finally be able to fix this problem properly, since
if_type is mostly just a work-around concept in -next, with just the
fake uclass_id being used at present.
Can you use if_type_to_uclass_id() here, which is the work-around
function for now?
This function does not exist in origin/next. We won't apply this patch
in the 2022-10 cycle.
I think I mean conv_uclass_id() which is the new name.
Let's fix the bug first before thinking about future refactoring.
You may determine the uclass ID for field bdev in struct blk_desc
using
function device_get_uclass_id() when refactoring.
So if you call conv_uclass_id() (without any other refactoring) does
that fix the problem?
Except for UCLASS_USB that function is a NOP. How could it help to
differentiate between devices with the same parent device?
It can't. But the root node should not have UCLASS_BLK children. I
think I mentioned that a few months back?
Would you agree that blk_get_devnum_by_uclass_idname() should not look
at the parent but on the actual device?
No, looking at the parent is exactly what it should do. A block device
is generic, to the extent possible. Its methods are implemented in the
parent uclass and are tightly bound to it. See for example
U_BOOT_DRIVER(mmc_blk) in the MMC uclass.
Let's look at an MMC device
root_driver/soc/mmc@1c0f000/m...@1c0f000.blk is a block device.
What do we need to find out that it can be addressed as mmc 0? The
driver is mmc_blk and its index is 0. We don't need any information
about the parent device at all.
Unfortunately this confusion is my fault since I used the root device
for the sandbox block devices. That was a convenience and a way to
reduce somewhat the crushing load of driver model migration. But the
time for that convenience is gone and we should create a sandbox host
parent node for the sandbox block devices and tidy up EFI too.
The only confusion is in the current blk_get_devnum_by_uclass_idname()
code looking into the parent device.
The parent device is totally irrelevant here. Stop using it.
You already noted when writing conv_uclass_id() that using the uclass
name does not work properly to find out the CLI name of a devie.
Can we put the CLI name for device types ("mmc", "scsi" ...) into struct
blk_ops? Then we have a clear separation of the block device from the
parent device.
Best regards
Heinrich
[PATCH v1] pinctrl: nuvoton: Add NPCM8xx pinctrl driver
Add Nuvoton BMC NPCM845 Pinmux and Pinconf support.
Signed-off-by: Jim Liu
Signed-off-by: Stanley Chu
---
drivers/pinctrl/nuvoton/Kconfig | 13 +
drivers/pinctrl/nuvoton/Makefile | 1 +
drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c | 995 ++
3 files changed, 1009 insertions(+)
create mode 100644 drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c
diff --git a/drivers/pinctrl/nuvoton/Kconfig b/drivers/pinctrl/nuvoton/Kconfig
index 07f65f7637..e55a0261ad 100644
--- a/drivers/pinctrl/nuvoton/Kconfig
+++ b/drivers/pinctrl/nuvoton/Kconfig
@@ -5,3 +5,16 @@ config PINCTRL_NPCM7XX
help
Say Y here to enable pin controller and GPIO support
for Nuvoton NPCM750/730/715/705 SoCs.
+
+config PINCTRL_NPCM8XX
+ bool "Pinctrl driver for Nuvoton NPCM8XX"
+ depends on DM && PINCTRL_GENERIC && ARCH_NPCM8XX
+ help
+ Support pin muxing and pin configuration on
+ Nuvoton NPCM8XX SoC.
+
+ The NPCM8XX contains 256 GPIO pins. Most of them are
+ multiplexed with other system functions. These pins can
+ be configured as either GPIO pin or alternate function.
+ It also supports basic configurations such as pull up/down,
+ drive-strength, and slew rate control for some of the pins.
diff --git a/drivers/pinctrl/nuvoton/Makefile b/drivers/pinctrl/nuvoton/Makefile
index 886d00784c..d03e2b5824 100644
--- a/drivers/pinctrl/nuvoton/Makefile
+++ b/drivers/pinctrl/nuvoton/Makefile
@@ -2,3 +2,4 @@
# Nuvoton pinctrl support
obj-$(CONFIG_PINCTRL_NPCM7XX) += pinctrl-npcm7xx.o
+obj-$(CONFIG_PINCTRL_NPCM8XX) += pinctrl-npcm8xx.o
diff --git a/drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c
b/drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c
new file mode 100644
index 00..c6ffa89f77
--- /dev/null
+++ b/drivers/pinctrl/nuvoton/pinctrl-npcm8xx.c
@@ -0,0 +1,995 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2022 Nuvoton Technology Corp.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/* GCR register offsets */
+#define WD0RCR 0x38
+#define WD1RCR 0x3c
+#define WD2RCR 0x40
+#define SWRSTC10x44
+#define SWRSTC20x48
+#define SWRSTC30x4c
+#define SWRSTC40x50
+#define CORSTC 0x5c
+#define FLOCKR10x74
+#define INTCR4 0xc0
+#define I2CSEGSEL 0xe0
+#define MFSEL1 0x260
+#define MFSEL2 0x264
+#define MFSEL3 0x268
+#define MFSEL4 0x26c
+#define MFSEL5 0x270
+#define MFSEL6 0x274
+#define MFSEL7 0x278
+
+/* GPIO register offsets */
+#define GPIO_POL 0x08 /* Polarity */
+#define GPIO_DOUT 0x0c /* Data OUT */
+#define GPIO_OTYP 0x14 /* Output Type */
+#define GPIO_PU0x1c /* Pull-up */
+#define GPIO_PD0x20 /* Pull-down */
+#define GPIO_DBNC 0x24 /* Debounce */
+#define GPIO_EVEN 0x40 /* Event Enable */
+#define GPIO_EVST 0x4c /* Event Status */
+#define GPIO_IEM 0x58 /* Input Enable */
+#define GPIO_OSRC 0x5c /* Output Slew-Rate Control */
+#define GPIO_ODSC 0x60 /* Output Drive Strength Control */
+#define GPIO_OES 0x70 /* Output Enable Set */
+#define GPIO_OEC 0x74 /* Output Enable Clear */
+
+#define NPCM8XX_GPIO_PER_BANK 32
+#define GPIOX_OFFSET 16
+
+struct npcm8xx_pinctrl_priv {
+ void __iomem *gpio_base;
+ struct regmap *gcr_regmap;
+ struct regmap *rst_regmap;
+};
+
+/*
+ * Function table
+ * name, register, enable bit, pin list
+ */
+#define FUNC_LIST \
+ FUNC(smb3, MFSEL1, 0, 30, 31) \
+ FUNC(smb4, MFSEL1, 1, 28, 29) \
+ FUNC(smb5, MFSEL1, 2, 26, 27) \
+ FUNC(spi0cs1, MFSEL1, 3, 32) \
+ FUNC(hsi1c, MFSEL1, 4, 45, 46, 47, 61) \
+ FUNC(hsi2c, MFSEL1, 5, 52, 53, 54, 55) \
+ FUNC(smb0, MFSEL1, 6, 114, 115) \
+ FUNC(smb1, MFSEL1, 7, 116, 117) \
+ FUNC(smb2, MFSEL1, 8, 118, 119) \
+ FUNC(bmcuart0a, MFSEL1, 9, 41, 42) \
+ FUNC(hsi1a, MFSEL1, 10, 43, 63) \
+ FUNC(hsi2a, MFSEL1, 11, 48, 49) \
+ FUNC(r1err, MFSEL1, 12, 56) \
+ FUNC(r1md, MFSEL1, 13, 57, 58) \
+ FUNC(r2, MFSEL1, 14, 84, 85, 86, 87, 88, 89, 200) \
+ FUNC(r2err, MFSEL1, 15, 90) \
+ FUNC(r2md, MFSEL1, 16, 91, 92) \
+ FUNC(ga20kbc, MFSEL1, 17, 93, 94) \
+ FUNC(clkout, MFSEL1, 21, 160) \
+ FUNC(sci, MFSEL1, 22, 170) \
+ FUNC(gspi, MFSEL1, 24, 12, 13, 14, 15) \
+ FUNC(lpc, MFSEL1, 26, 95, 161, 163, 164, 165, 166, 167) \
+ FUNC(hsi1b, MFSEL1, 28, 44, 62) \
+ FUNC(hsi2b, MFSEL1, 29, 50, 51) \
+ FUNC(iox1, MFSEL1, 30, 0, 1, 2, 3) \
+ FUNC(serirq, MFSEL1, 31, 168) \
+ FUNC(fanin0, MFSEL2, 0, 64) \
+ FUNC(fanin1, MFSEL2, 1, 65) \
+ FUNC(fanin2, MFSEL2, 2, 66) \
+ FUNC(fanin3, MFSEL2, 3, 67) \
+ FUNC(fanin4, MFSEL2, 4, 68) \
+
Re: [PATCH 4/5] binman: Use an exit code when blobs are missing
Hi Simon,
On 10/10/22 22:00, Simon Glass wrote:
At present binman returns success when told to handle missing blobs.
This is confusing this in fact the resulting image cannot work.
Use exit code 103 to signal this problem, with a -W option to convert
it to a warning.
Signed-off-by: Simon Glass
---
tools/binman/binman.rst | 4
tools/binman/cmdline.py | 3 +++
tools/binman/control.py | 7 ++-
3 files changed, 13 insertions(+), 1 deletion(-)
diff --git a/tools/binman/binman.rst b/tools/binman/binman.rst
index 4ee6f41f35e..c0512e68e67 100644
--- a/tools/binman/binman.rst
+++ b/tools/binman/binman.rst
@@ -1458,6 +1458,10 @@ space-separated list of directories to search for binary
blobs::
odroid-c4/build/board/hardkernel/odroidc4/firmware \
odroid-c4/build/scp_task" binman ...
+Note that binman fails with error code 103 when there are missing blobs. If you
+wish to continue anyway, you can pass `-W` to binman.
+
+
Code coverage
-
diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py
index 1d1ca43993d..144c0c916a2 100644
--- a/tools/binman/cmdline.py
+++ b/tools/binman/cmdline.py
@@ -128,6 +128,9 @@ controlled by a description in the board device tree.'''
default=False, help='Update the binman node with offset/size info')
build_parser.add_argument('--update-fdt-in-elf', type=str,
help='Update an ELF file with the output dtb:
infile,outfile,begin_sym,end_sym')
+build_parser.add_argument(
+'-W', '--ignore-missing-blobs', action='store_true', default=False,
+help='Return success even if there are missing blobs')
subparsers.add_parser(
'bintool-docs', help='Write out bintool documentation (see
bintool.rst)')
diff --git a/tools/binman/control.py b/tools/binman/control.py
index bfe63a15204..119b1176b7a 100644
--- a/tools/binman/control.py
+++ b/tools/binman/control.py
@@ -742,7 +742,12 @@ def Binman(args):
elf.UpdateFile(*elf_params, data)
if invalid:
-tout.warning("\nSome images are invalid")
+msg = '\nSome images are invalid'
+if args.ignore_missing_blobs:
+tout.warning(msg)
+else:
+tout.error("\nSome images are invalid")
I think you meant to have msg variable used here.
Also, you could flatten this by having a if not
args.ingore_missing_blobs which has the return and then the new
tout.warning left on the same indentation level of the current one.
Cheers,
Quentin
Re: [PATCH 1/1] efi_loader: CloseProtocol in efi_fmp_find
On Tue, Oct 11, 2022 at 07:58:11AM +0200, Heinrich Schuchardt wrote:
>
>
> On 10/11/22 02:49, AKASHI Takahiro wrote:
> > The commit message is not accurate.
> >
> > On Fri, Oct 07, 2022 at 04:06:23PM +0200, Heinrich Schuchardt wrote:
> > > The CloseProtocol() boot service requires a handle as first argument.
> > > Passing the protocol interface is incorrect.
> >
> > Correct, but
> >
> > > CloseProtocol() only has an effect if called with a non-zero value for
> > > agent_handle. HandleProtocol() uses an opaque agent_handle when invoking
> > > OpenProtocol() (currently NULL).
> >
> > No. OpenProtocol() is called with efi_root as an agent handle.
> > So, calling CloseProtocol() is a right thing at the end.
>
> Typically an agent handle is used to relate to a driver exposing the driver
> binding protocol.
Why can't we, other than a driver, call HandleProtocol()
as a convenient way of accessing an interface?
> The root node does not expose the driver binding protocol.
So do you mean the current implementation of HandleProtocol() is wrong?
> Why would you want to create an open protocol information entry here?
To access get_image_info() quickly.
> Do you think anything with the code after the patch is wrong?
No reason to replace handle_protocol().
Another example is here:
efi_load_image_from_path()
efi_handle_protocol(device, guid, (void **)_file_protocol));
...
efi_close_protocol(device, guid, efi_root, NULL);
I believe that this function is anything but a driver.
I think using HandleProtocol() (or preferably OpenProtocol()) and
CloseProtocol()
in pair seems totally sane.
-Takahiro Akashi
> Best regards
>
> Heinrich
>
> >
> > > Therefore HandleProtocol() should be
> > > avoided.
> > >
> > > * Replace the LocateHandle() call by efi_search_protocol().
> >
> > LocateHandle() -> efi_handle_protocol()
> >
> > So you could have fixed this way:
> > EFI_CALL(efi_close_protocol(handle, ..., _root, NULL);
> >
> > I preferred to use EFI_CALL() over this file as you can see.
> >
> > -Takahiro Akashi
> >
> > > * Remove the CloseProtocol() call.
> > >
> > > Fixes: 8d99026f0697 ("efi_loader: capsule: support firmware update")
> > > Signed-off-by: Heinrich Schuchardt
> > > ---
> > > lib/efi_loader/efi_capsule.c | 14 ++
> > > 1 file changed, 6 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
> > > index b6bd2d6af8..397e393a18 100644
> > > --- a/lib/efi_loader/efi_capsule.c
> > > +++ b/lib/efi_loader/efi_capsule.c
> > > @@ -159,12 +159,14 @@ efi_fmp_find(efi_guid_t *image_type, u8
> > > image_index, u64 instance,
> > > efi_status_t ret;
> > > for (i = 0, handle = handles; i < no_handles; i++, handle++) {
> > > - ret = EFI_CALL(efi_handle_protocol(
> > > - *handle,
> > > - _guid_firmware_management_protocol,
> > > - (void **)));
> > > + struct efi_handler *fmp_handler;
> > > +
> > > + ret = efi_search_protocol(
> > > + *handle, _guid_firmware_management_protocol,
> > > + _handler);
> > > if (ret != EFI_SUCCESS)
> > > continue;
> > > + fmp = fmp_handler->protocol_interface;
> > > /* get device's image info */
> > > info_size = 0;
> > > @@ -215,10 +217,6 @@ efi_fmp_find(efi_guid_t *image_type, u8 image_index,
> > > u64 instance,
> > > skip:
> > > efi_free_pool(package_version_name);
> > > free(image_info);
> > > - EFI_CALL(efi_close_protocol(
> > > - (efi_handle_t)fmp,
> > > - _guid_firmware_management_protocol,
> > > - NULL, NULL));
> > > if (found)
> > > return fmp;
> > > }
> > > --
> > > 2.37.2
> > >
Re: Broken watchdog in u-boot master branch
On Tuesday 11 October 2022 09:18:56 Rasmus Villemoes wrote: > On 10/10/2022 15.55, Tom Rini wrote: > > On Sun, Oct 09, 2022 at 09:12:25PM +0200, Pali Rohár wrote: > > > >> Hello! Watchdog code seems to be broken in u-boot master branch. > >> On Nokia N900 I'm getting following message in qemu: > >> > >> cyclic function rx51_watchdog took too long: 1us vs 1000us max, > >> disabling > >> > >> Seems that watchdog core code is not prepared for "slower" watchdogs > >> which communicate over slower i2c bus, like it is the case for N900. > >> > >> Disabling slower watchdog is a bad idea as it would result in reboot > >> loop instead of slower - but working code. > > So, a few thoughts. > > First, I assume that that board has a very coarse-grained tick, probably > just 1000Hz. IIRC i2c is running at 100kHz. But u-boot i2c driver contains more udelay() calls and more busy loops. And I was told that there are also some hw erratas. So at the end if some other driver calls u-boot dm function for i2c transfer, it can spend more time than expected... Also there is another issue is that udelay() itself calls watchdog reset function. So if watchdog is on i2c which driver calls udelay() it means that watchdog reset function may be called in infinite loop. For this reason n900 watchdog driver has lock which prevents and eliminate this loop (if somebody calls reset during active lock then reset does nothing). > Otherwise it would be pretty amazing for cpu_time to come > out as 10ms exactly. That's not the board's fault, of course, just an > observation, but it is something we need to bear in mind. If the > resolution is merely 100Hz, so 10ms is simply the granularity, we cannot > really meaningfully compare the cpu_time to anything less than that, > because every once in a while it _will_ happen that we sample "now" just > before the tick, run the function, then sample again just after, and it > may only have taken 17us, yet the diff comes out as 10ms. > > Second, perhaps the threshold should not be a compile-time constant, but > instead a fraction of the requested call frequency (say 1.5%, 1/64). > I.e., if we've registered a function to be called every 10 seconds, we'd > check if its runtime exceeded (1000 >> 6) us. Preferably per above > that bound is rounded up to a multiple of the timer's granularity (we > can get that, right?) > > Third, perhaps we shouldn't disable it, but just print a (one-time) > warning. Adding a "already-warned" field to struct cyclic_info is > certainly simple enough. > > Rasmus
Re: [PATCH] arm64: versal: Add zynq_board_read_rom_ethaddr()
On 10/7/22 14:25, Venkatesh Yadav Abbarapu wrote:
zynq_gem.c invokes zynq_board_read_rom_ethaddr(), but neglected to
declare its declaration, causing the following sparse and
compile time warnings:
drivers/net/zynq_gem.c:662:12:
warning: no previous prototype for 'zynq_board_read_rom_ethaddr'
[-Wmissing-prototypes]
662 | __weak int zynq_board_read_rom_ethaddr(unsigned char *ethaddr)
Include the missing declaration in asm/arch/sys_proto.h.
Signed-off-by: Venkatesh Yadav Abbarapu
---
arch/arm/mach-versal/include/mach/sys_proto.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/arm/mach-versal/include/mach/sys_proto.h
b/arch/arm/mach-versal/include/mach/sys_proto.h
index 8e5712e0c9..9eb91b23d9 100644
--- a/arch/arm/mach-versal/include/mach/sys_proto.h
+++ b/arch/arm/mach-versal/include/mach/sys_proto.h
@@ -12,6 +12,7 @@ enum {
void tcm_init(u8 mode);
void mem_map_fill(void);
+int zynq_board_read_rom_ethaddr(unsigned char *ethaddr);
static inline int zynqmp_mmio_write(const u32 address, const u32 mask, const u32 value)
{
I think we should remove zynq_board_read_rom_ethaddr() completely as I wrote
here.
https://lore.kernel.org/u-boot/cahtx3djvmqdv2pcbuvp7snwhok1p0nms8anyho_sjdnrp92...@mail.gmail.com/T/#t
Thanks,
Michal
Re: [PATCH v4 00/10] Use logging feature instead of FPGA_DEBUG
On 10/7/22 14:19, Alexander Dahl wrote: Hei hei, while working on FPGA support for a new device I discovered debug logging in some FPGA drivers is still done as in the old days. Bring that to what I thougt would be the currently preferred approach. Notes: Adding those Kconfig symbols in patch 1 is just to be able to build two of the old drivers. All drivers touched were build tested with sandbox_defconfig and GCC 8 on Debian GNU/Linux 10 (buster). Lines with other possibly questionable output were not touched, only what seemed to be designated debug output, and only for FPGA drivers having that ancient FPGA_DEBUG / PRINTF macros, so there's room for future improvements. Changelog: v3 -> v4: - Reordered patches, Kconfig patch comes first now (made it easier to build and test the series step by step) - Added three patches fixing printf compiler warnings first, before changing to the new logging framework (so CI should not fail anymore with -Werror) v2 -> v3: - Patch introducing FPGA uclass was completely reworked, sent independently from this series, and applied already, thus removed - Because requiring that new FPGA uclass changes, rebased on Michal's microblaze branch '20221005' - Removed '"%s …", __func__' and '"%d …", __line__' from log messages, because log framework can add those (enabled by CONFIG_LOGF_FUNC and CONFIG_LOGF_LINE) v1 -> v2: - Rebased on master - Added patch to introduce new FPGA uclass in front of the other patches - Use that new uclass as log category - Slightly reworded cover letter Greets Alex Cc: Michal Simek Alexander Dahl (10): fpga: Add missing Kconfig symbols for old FPGA drivers fpga: spartan2: Fix printf arguments warning fpga: spartan3: Fix printf arguments warning fpga: virtex2: Fix printf format string warnings fpga: altera: Use logging feature instead of FPGA_DEBUG fpga: cyclon2: Use logging feature instead of FPGA_DEBUG fpga: ACEX1K: Use logging feature instead of FPGA_DEBUG fpga: spartan2: Use logging feature instead of FPGA_DEBUG fpga: spartan3: Use logging feature instead of FPGA_DEBUG fpga: virtex2: Use logging feature instead of FPGA_DEBUG drivers/fpga/ACEX1K.c | 37 +-- drivers/fpga/Kconfig| 12 +++ drivers/fpga/altera.c | 11 +++--- drivers/fpga/cyclon2.c | 38 +--- drivers/fpga/spartan2.c | 80 +++-- drivers/fpga/spartan3.c | 80 +++-- drivers/fpga/virtex2.c | 69 --- 7 files changed, 152 insertions(+), 175 deletions(-) base-commit: 2d8cf392a77815f062446ef441f1078958dc1b2a Applied. M
