Re: [PATCH u-boot 33/39] ARM: omap3: fix LTO for DM3730 (and possibly other omap3 boards)
On Sat, Mar 6, 2021 at 10:26 PM Marek Behún wrote: > > Adam Ford says that DM3730 needs board.c compiled without LTO flags. > > Also add clock.c, since it says in Makefile that it need different > flags. > I managed to get the am3517_evm tested with the latest series, and it boots just fine. am3517_evm Without LTO: SPL 57255 U-Boot 664922 With LTO: SPL 51497 U-Boot 642058 For a reduction in size of: SPL -5758 (-10.06%) U-Boot -22864 (-3.44%) Tested-by: Adam Ford #omap3_logic & am3517_evm > Signed-off-by: Marek Behún > Suggested-by: Adam Ford > --- > arch/arm/mach-omap2/omap3/Makefile | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/arch/arm/mach-omap2/omap3/Makefile > b/arch/arm/mach-omap2/omap3/Makefile > index 91ed8ebc9f..7d22f04bbf 100644 > --- a/arch/arm/mach-omap2/omap3/Makefile > +++ b/arch/arm/mach-omap2/omap3/Makefile > @@ -5,10 +5,12 @@ > > # If clock.c is compiled for Thumb2, then it fails on OMAP3530 > CFLAGS_clock.o += -marm > +CFLAGS_REMOVE_clock.o := $(LTO_CFLAGS) > > obj-y := lowlevel_init.o > > obj-y += board.o > +CFLAGS_REMOVE_board.o := $(LTO_CFLAGS) > obj-y += boot.o > obj-y += clock.o > obj-y += sys_info.o > -- > 2.26.2 >
Re: [PATCH u-boot 30/39] ARM: imx6m: fix imx_eqos_txclk_set_rate() type mismatch for LTO
THX
Re: [PATCH u-boot 14/39] lib: crc32: make the crc_table variable non-const
On Sun, 7 Mar 2021 06:02:16 +0100 Marek Vasut wrote: > On 3/7/21 5:58 AM, Marek Behun wrote: > > On Sun, 7 Mar 2021 05:46:24 +0100 > > Marek Vasut wrote: > > > >> On 3/7/21 5:25 AM, Marek Behún wrote: > >>> When compiling with LTO, the compiler fails with an error saying that > >>> `crc_table` causes a section type conflict with `efi_var_buf`. > >>> > >>> This is because both are declared to be in the same section (via macro > >>> `__efi_runtime_data`), but one is const while the other is not. > >>> > >>> Make this variable non-const in order to fix this. > >> > >> This does not look right, the crc32 array is constant. > >> Maybe what you want to do instead if create some __efi_constant_data > >> section ? > > > > Yes, this was the easier solution, and maybe is not ideal. > > > > I thought it would not be much of a problem since this array can be > > nonconstant (generated after boot) if CONFIG_DYNAMIC_CRC_TABLE is > > enabled. > > > > Anyway I don't much understand the EFI code so I wanted to poke into it > > as little as possible. > > Isn't the compiler capable of better optimization on constant stuff ? > That's pretty much what prompted my suggestion to add separate section. Yes, but - for this case I don't think the compiler actually can do any significat optimizations when declaring the table const, since it has to access tab[(crc ^ (x)) & 255] I tried with arm compiler just now, with -O2 and -Os, and it generated the same code either way - when using LTO, the compiler theoretically should be able to deduce that the table is never written to But if people insist on declaring the table const, I will look into this...
Re: [PATCH u-boot 14/39] lib: crc32: make the crc_table variable non-const
On Sunday 07 March 2021 13:26:36 Marek Behun wrote: > On Sun, 7 Mar 2021 06:02:16 +0100 > Marek Vasut wrote: > > > On 3/7/21 5:58 AM, Marek Behun wrote: > > > On Sun, 7 Mar 2021 05:46:24 +0100 > > > Marek Vasut wrote: > > > > > >> On 3/7/21 5:25 AM, Marek Behún wrote: > > >>> When compiling with LTO, the compiler fails with an error saying that > > >>> `crc_table` causes a section type conflict with `efi_var_buf`. > > >>> > > >>> This is because both are declared to be in the same section (via macro > > >>> `__efi_runtime_data`), but one is const while the other is not. > > >>> > > >>> Make this variable non-const in order to fix this. > > >> > > >> This does not look right, the crc32 array is constant. > > >> Maybe what you want to do instead if create some __efi_constant_data > > >> section ? > > > > > > Yes, this was the easier solution, and maybe is not ideal. > > > > > > I thought it would not be much of a problem since this array can be > > > nonconstant (generated after boot) if CONFIG_DYNAMIC_CRC_TABLE is > > > enabled. > > > > > > Anyway I don't much understand the EFI code so I wanted to poke into it > > > as little as possible. > > > > Isn't the compiler capable of better optimization on constant stuff ? > > That's pretty much what prompted my suggestion to add separate section. > > Yes, but > - for this case I don't think the compiler actually can do any > significat optimizations when declaring the table const, since it has > to access > tab[(crc ^ (x)) & 255] > I tried with arm compiler just now, with -O2 and -Os, and it > generated the same code either way > - when using LTO, the compiler theoretically should be able to deduce > that the table is never written to > > But if people insist on declaring the table const, I will look into > this... If you try to overwrite a const variable from the same code unit then compiler throw an error. So declaring read-only variable as a const could prevent people to unintentionally overwrite read-only variable. And detect possible bad code.
Re: [PATCH u-boot 14/39] lib: crc32: make the crc_table variable non-const
On Sun, 7 Mar 2021 13:31:11 +0100 Pali Rohár wrote: > On Sunday 07 March 2021 13:26:36 Marek Behun wrote: > > On Sun, 7 Mar 2021 06:02:16 +0100 > > Marek Vasut wrote: > > > > > On 3/7/21 5:58 AM, Marek Behun wrote: > > > > On Sun, 7 Mar 2021 05:46:24 +0100 > > > > Marek Vasut wrote: > > > > > > > >> On 3/7/21 5:25 AM, Marek Behún wrote: > > > >>> When compiling with LTO, the compiler fails with an error saying that > > > >>> `crc_table` causes a section type conflict with `efi_var_buf`. > > > >>> > > > >>> This is because both are declared to be in the same section (via macro > > > >>> `__efi_runtime_data`), but one is const while the other is not. > > > >>> > > > >>> Make this variable non-const in order to fix this. > > > >> > > > >> This does not look right, the crc32 array is constant. > > > >> Maybe what you want to do instead if create some __efi_constant_data > > > >> section ? > > > > > > > > Yes, this was the easier solution, and maybe is not ideal. > > > > > > > > I thought it would not be much of a problem since this array can be > > > > nonconstant (generated after boot) if CONFIG_DYNAMIC_CRC_TABLE is > > > > enabled. > > > > > > > > Anyway I don't much understand the EFI code so I wanted to poke into it > > > > as little as possible. > > > > > > Isn't the compiler capable of better optimization on constant stuff ? > > > That's pretty much what prompted my suggestion to add separate section. > > > > Yes, but > > - for this case I don't think the compiler actually can do any > > significat optimizations when declaring the table const, since it has > > to access > > tab[(crc ^ (x)) & 255] > > I tried with arm compiler just now, with -O2 and -Os, and it > > generated the same code either way > > - when using LTO, the compiler theoretically should be able to deduce > > that the table is never written to > > > > But if people insist on declaring the table const, I will look into > > this... > > If you try to overwrite a const variable from the same code unit then > compiler throw an error. So declaring read-only variable as a const > could prevent people to unintentionally overwrite read-only variable. > And detect possible bad code. OK it seems all linker scripts also mention .rodata.efi_runtime*, not just .data.efi_runtime*. I shall put it into the .rodata.efi_runtime section. Marek
[PATCH u-boot 14/39] lib: crc32: put the crc_table variable into efi_runtime_rodata section
When compiling with LTO, the compiler fails with an error saying that `crc_table` causes a section type conflict with `efi_var_buf`. This is because both are declared to be in the same section (via macro `__efi_runtime_data`), but one is const while the other is not. Put this variable into the section .rodata.efi_runtime, instead of .data.efi_runtime. Signed-off-by: Marek Behún --- include/efi_loader.h | 2 ++ lib/crc32.c | 3 ++- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/include/efi_loader.h b/include/efi_loader.h index a8281b3c95..b1e5d2e13e 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -671,6 +671,7 @@ ssize_t efi_dp_check_length(const struct efi_device_path *dp, * section and thus still be available when the OS is running */ #define __efi_runtime_data __section(".data.efi_runtime") +#define __efi_runtime_rodata __section(".rodata.efi_runtime") #define __efi_runtime __section(".text.efi_runtime") /* Indicate supported runtime services */ @@ -870,6 +871,7 @@ efi_status_t efi_launch_capsules(void); /* Without CONFIG_EFI_LOADER we don't have a runtime section, stub it out */ #define __efi_runtime_data +#define __efi_runtime_rodata #define __efi_runtime static inline efi_status_t efi_add_runtime_mmio(void *mmio_ptr, u64 len) { diff --git a/lib/crc32.c b/lib/crc32.c index e9be3bf386..f2acc107fe 100644 --- a/lib/crc32.c +++ b/lib/crc32.c @@ -26,6 +26,7 @@ #ifdef USE_HOSTCC #define __efi_runtime #define __efi_runtime_data +#define __efi_runtime_rodata #endif #define tole(x) cpu_to_le32(x) @@ -88,7 +89,7 @@ static void __efi_runtime make_crc_table(void) * Table of CRC-32's of all single-byte values (made by make_crc_table) */ -static const uint32_t __efi_runtime_data crc_table[256] = { +static const uint32_t __efi_runtime_rodata crc_table[256] = { tole(0xL), tole(0x77073096L), tole(0xee0e612cL), tole(0x990951baL), tole(0x076dc419L), tole(0x706af48fL), tole(0xe963a535L), tole(0x9e6495a3L), tole(0x0edb8832L), tole(0x79dcb8a4L), tole(0xe0d5e91eL), tole(0x97d2d988L), -- 2.26.2
Re: [PATCH u-boot 14/39] lib: crc32: put the crc_table variable into efi_runtime_rodata section
Hmm, maybe this should be split into 2 commits.
Re: Habv4 on imx8m
Hi all, On 04.03.2021 12:24, Stefano Babic wrote: On 04.03.21 12:18, Fabio Estevam wrote: Hi Frieder, On Thu, Mar 4, 2021 at 5:23 AM Frieder Schrempf wrote: Is this still on your list? It would be great if you could send a patch for this. Thanks for the reminder. I have just sent the patch. I forgot it as well, it is just to extend the defconfig. I pick up Fabio's patch, it will be merged into 2021.04 Stefano I tested the patch with CONFIG_IMX_HAB=y for an IMX8m and I got some compiler errors. A part of the output (in german): ... CC drivers/crypto/fsl/sec.o CC drivers/crypto/fsl/jr.o drivers/crypto/fsl/jr.c:28:21: Fehler: »CONFIG_SYS_FSL_MAX_NUM_OF_SEC« ist hier nicht deklariert (nicht in einer Funktion) 28 | uint32_t sec_offset[CONFIG_SYS_FSL_MAX_NUM_OF_SEC] = { | ^ drivers/crypto/fsl/jr.c: In Funktion »start_jr0«: drivers/crypto/fsl/jr.c:47:2: Fehler: unbekannter Typname: »ccsr_sec_t« 47 | ccsr_sec_t *sec = (void *)SEC_ADDR(sec_idx); | ^~ drivers/crypto/fsl/jr.c:37:4: Fehler: »CONFIG_SYS_FSL_SEC_ADDR« nicht deklariert (erstmalige Verwendung in dieser Funktion); meinten Sie »CONFIG_SYS_FSL_ESDHC_ADDR«? 37 | ((CONFIG_SYS_FSL_SEC_ADDR + sec_offset[idx])) |^~~ ... The list is long and mostly the compiler cannot found some defines: CONFIG_SYS_FSL_SEC_ADDR CONFIG_SYS_FSL_JR0_OFFSET CONFIG_SYS_FSL_SEC_OFFSET RDSTA_SKVN RTSDCTL_ENT_DLY_MIN RTSDCTL_ENT_DLY_MAX And this type: ccsr_sec_t I used for the test: - Das U-Boot master branch - Cross compiler: aarch64-linux-gnu- - This documentation: https://github.com/u-boot/u-boot/blob/master/doc/board/freescale/imx8mm_evk.rst - I enable in the menuconfig CONFIG_IMX_HAB and SYS_FSL_SEC_LE Florian
[PATCH] configs: add PineTab defconfig
From: Arnaud Ferraris The PineTab device-tree is already in u-boot, this commit adds the corresponding defconfig, based on pinephone_defconfig. Signed-off-by: Arnaud Ferraris --- a/board/sunxi/MAINTAINERS +++ b/board/sunxi/MAINTAINERS @@ -471,6 +471,11 @@ M: Samuel Holland S: Maintained F: configs/pinephone_defconfig +PINETAB BOARD +M: Arnaud Ferraris +S: Maintained +F: configs/pinetab_defconfig + R16 EVB PARROT BOARD M: Quentin Schulz S: Maintained --- /dev/null +++ b/configs/pinetab_defconfig @@ -0,0 +1,21 @@ +CONFIG_ARM=y +CONFIG_ARCH_SUNXI=y +CONFIG_SPL=y +CONFIG_IDENT_STRING="" +CONFIG_MACH_SUN50I=y +CONFIG_SUNXI_DRAM_LPDDR3_STOCK=y +CONFIG_DRAM_CLK=552 +CONFIG_DRAM_ZQ=3881949 +CONFIG_MMC_SUNXI_SLOT_EXTRA=2 +# CONFIG_VIDEO_DE2 is not set +CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-pinetab" +# CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set +CONFIG_BOOTDELAY=0 +CONFIG_SYS_CONSOLE_INFO_QUIET=y +# CONFIG_DISPLAY_CPUINFO is not set +# CONFIG_DISPLAY_BOARDINFO is not set +# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set +# CONFIG_SPL_BANNER_PRINT is not set +# CONFIG_SPL_POWER_SUPPORT is not set +# CONFIG_NET is not set +# CONFIG_EFI_LOADER is not set
Re: [PATCH] Nokia RX-51: Enable CONFIG_WDT to remove deprecation warning
On Sat, Mar 06, 2021 at 11:16:53PM +0100, Pali Rohár wrote: > Signed-off-by: Pali Rohár > --- > This patch increase u-boot.bin binary size above maximal limit. So this > patch cannot be applied yet. But it can be applied on top of the LTO > patches. So please put this patch into the queue and include it into > U-Boot after LTO patches are merged. > --- > configs/nokia_rx51_defconfig | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/configs/nokia_rx51_defconfig b/configs/nokia_rx51_defconfig > index 312ca3a1a991..2949c37f721b 100644 > --- a/configs/nokia_rx51_defconfig > +++ b/configs/nokia_rx51_defconfig > @@ -67,4 +67,6 @@ CONFIG_CFB_CONSOLE=y > CONFIG_CFB_CONSOLE_ANSI=y > # CONFIG_VGA_AS_SINGLE_DEVICE is not set > CONFIG_SPLASH_SCREEN=y > +# CONFIG_WATCHDOG is not set > +CONFIG_WDT=y > # CONFIG_GZIP is not set Please update include/configs/nokia_rx51.h to not set CONFIG_HW_WATCHDOG and then make sure the appropriate watchdog options are enabled. -- Tom signature.asc Description: PGP signature
Re: Broken build on OpenBSD
> Date: Mon, 1 Mar 2021 09:02:18 -0500 > From: Tom Rini > > On Tue, Feb 23, 2021 at 08:07:21PM +0100, Mark Kettenis wrote: > > > Hi Simon, > > > > Commit c5819701a3de61e2ba2ef7ad0b616565b32305e5 broke the build on > > OpenBSD and probably other non-Linux systems. ENODATA, which is now > > used in fit_check_format(), isn't defined. It isn't part of POSIX[1] > > and generally not available on BSD-derived systems. Could you pick > > another error code for this case? > > > > Thanks, > > > > Mark > > > > [1] It is mentioned in the Open Group Base Specification, however it > > is part of the obsolete XSI STREAMS extension which was never part > > of POSIX proper. > > Just for the record: > > https://man7.org/linux/man-pages/man3/errno.3.html: > > ENODATA No message is available on the STREAM head read queue > (POSIX.1-2001). > > So perhaps you want to also raise this with the Linux man pages project > folks? Looks like somebody made a mistake and put the XSI STREAMS option marker for it on ENOBUFS just above. I filed a bug report: https://bugzilla.kernel.org/show_bug.cgi?id=212103 Cheers, Mark
Re: [PATCH] configs: add PineTab defconfig
On Sun, Mar 7, 2021 at 1:24 PM Nicolas Boulenguez wrote: > > From: Arnaud Ferraris > > The PineTab device-tree is already in u-boot, this commit adds the > corresponding > defconfig, based on pinephone_defconfig. Should it have support to deal with the two variants where they have differing screens? > Signed-off-by: Arnaud Ferraris > > --- a/board/sunxi/MAINTAINERS > +++ b/board/sunxi/MAINTAINERS > @@ -471,6 +471,11 @@ M: Samuel Holland > S: Maintained > F: configs/pinephone_defconfig > > +PINETAB BOARD > +M: Arnaud Ferraris > +S: Maintained > +F: configs/pinetab_defconfig > + > R16 EVB PARROT BOARD > M: Quentin Schulz > S: Maintained > --- /dev/null > +++ b/configs/pinetab_defconfig > @@ -0,0 +1,21 @@ > +CONFIG_ARM=y > +CONFIG_ARCH_SUNXI=y > +CONFIG_SPL=y > +CONFIG_IDENT_STRING="" > +CONFIG_MACH_SUN50I=y > +CONFIG_SUNXI_DRAM_LPDDR3_STOCK=y > +CONFIG_DRAM_CLK=552 > +CONFIG_DRAM_ZQ=3881949 > +CONFIG_MMC_SUNXI_SLOT_EXTRA=2 > +# CONFIG_VIDEO_DE2 is not set > +CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-pinetab" > +# CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set > +CONFIG_BOOTDELAY=0 > +CONFIG_SYS_CONSOLE_INFO_QUIET=y > +# CONFIG_DISPLAY_CPUINFO is not set > +# CONFIG_DISPLAY_BOARDINFO is not set > +# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set > +# CONFIG_SPL_BANNER_PRINT is not set > +# CONFIG_SPL_POWER_SUPPORT is not set > +# CONFIG_NET is not set > +# CONFIG_EFI_LOADER is not set
Re: [PATCH u-boot 38/39] ARM: enable LTO for some boards
On Sat, Mar 6, 2021 at 10:26 PM Marek Behún wrote: > > Enable LTO for some boards that were tested by people on U-Boot Mailing > List. > I have also confirmed this works on the r8a774a1_beacon board as well and boots without issues. The r8a774b1_beacon and r8a774e1_beacon should be safe since the only real difference between them is the device tree reference. The RZ/G2 boards from Beacon don't use SPL, but wIthout LTO: U-Boot 784324 With LTO: U-Boot 757202 For a reduction of: -27122 (-3.46%) > Signed-off-by: Marek Behún > Tested-by: Adam Ford > Tested-by: Pali Rohár > --- > configs/da850evm_defconfig| 1 + > configs/da850evm_direct_nor_defconfig | 1 + > configs/da850evm_nand_defconfig | 1 + > configs/imx6q_logic_defconfig | 1 + > configs/imx8mn_beacon_2g_defconfig| 1 + > configs/imx8mn_beacon_defconfig | 1 + > configs/nokia_rx51_defconfig | 1 + > configs/turris_mox_defconfig | 1 + > configs/turris_omnia_defconfig| 1 + > 9 files changed, 9 insertions(+) > > diff --git a/configs/da850evm_defconfig b/configs/da850evm_defconfig > index 26e76a2929..6ff5e21bc6 100644 > --- a/configs/da850evm_defconfig > +++ b/configs/da850evm_defconfig > @@ -1,3 +1,4 @@ > +CONFIG_LTO=y > CONFIG_ARM=y > CONFIG_SYS_THUMB_BUILD=y > CONFIG_ARCH_DAVINCI=y > diff --git a/configs/da850evm_direct_nor_defconfig > b/configs/da850evm_direct_nor_defconfig > index d3860a963d..06c7ce7c47 100644 > --- a/configs/da850evm_direct_nor_defconfig > +++ b/configs/da850evm_direct_nor_defconfig > @@ -1,3 +1,4 @@ > +CONFIG_LTO=y > CONFIG_ARM=y > CONFIG_ARCH_CPU_INIT=y > CONFIG_ARCH_DAVINCI=y > diff --git a/configs/da850evm_nand_defconfig b/configs/da850evm_nand_defconfig > index 0d0e9a148d..be737564e1 100644 > --- a/configs/da850evm_nand_defconfig > +++ b/configs/da850evm_nand_defconfig > @@ -1,3 +1,4 @@ > +CONFIG_LTO=y > CONFIG_ARM=y > CONFIG_SYS_THUMB_BUILD=y > CONFIG_ARCH_DAVINCI=y > diff --git a/configs/imx6q_logic_defconfig b/configs/imx6q_logic_defconfig > index 36dc24d080..0f8aea6983 100644 > --- a/configs/imx6q_logic_defconfig > +++ b/configs/imx6q_logic_defconfig > @@ -1,3 +1,4 @@ > +CONFIG_LTO=y > CONFIG_ARM=y > CONFIG_ARCH_MX6=y > CONFIG_SYS_TEXT_BASE=0x1780 > diff --git a/configs/imx8mn_beacon_2g_defconfig > b/configs/imx8mn_beacon_2g_defconfig > index 58b8e49486..1c8cbc2c89 100644 > --- a/configs/imx8mn_beacon_2g_defconfig > +++ b/configs/imx8mn_beacon_2g_defconfig > @@ -1,3 +1,4 @@ > +CONFIG_LTO=y > CONFIG_ARM=y > CONFIG_ARCH_IMX8M=y > CONFIG_SYS_TEXT_BASE=0x4020 > diff --git a/configs/imx8mn_beacon_defconfig b/configs/imx8mn_beacon_defconfig > index d6a3385d8d..6457b9409a 100644 > --- a/configs/imx8mn_beacon_defconfig > +++ b/configs/imx8mn_beacon_defconfig > @@ -1,3 +1,4 @@ > +CONFIG_LTO=y > CONFIG_ARM=y > CONFIG_ARCH_IMX8M=y > CONFIG_SYS_TEXT_BASE=0x4020 > diff --git a/configs/nokia_rx51_defconfig b/configs/nokia_rx51_defconfig > index 312ca3a1a9..85ca627790 100644 > --- a/configs/nokia_rx51_defconfig > +++ b/configs/nokia_rx51_defconfig > @@ -1,3 +1,4 @@ > +CONFIG_LTO=y > CONFIG_ARM=y > # CONFIG_SYS_THUMB_BUILD is not set > CONFIG_ARCH_OMAP2PLUS=y > diff --git a/configs/turris_mox_defconfig b/configs/turris_mox_defconfig > index f938fbb475..2a351d9180 100644 > --- a/configs/turris_mox_defconfig > +++ b/configs/turris_mox_defconfig > @@ -1,3 +1,4 @@ > +CONFIG_LTO=y > CONFIG_ARM=y > CONFIG_ARCH_CPU_INIT=y > CONFIG_ARCH_MVEBU=y > diff --git a/configs/turris_omnia_defconfig b/configs/turris_omnia_defconfig > index 4b8843d7be..d7a2ec3592 100644 > --- a/configs/turris_omnia_defconfig > +++ b/configs/turris_omnia_defconfig > @@ -1,3 +1,4 @@ > +CONFIG_LTO=y > CONFIG_ARM=y > CONFIG_ARCH_CPU_INIT=y > CONFIG_SPL_SYS_THUMB_BUILD=y > -- > 2.26.2 >
[RFC] tests in test/dm/video.c are not independent
Hello Simon, looking at the tests in test/dm/video.c it seems that none of them clears the screen before adding text and testing the hash of the frame buffer. So it is hard to tell which output led to changes in subsequent tests. I think the tests should first clear the screen to start from a defined state. This will make the tests independent of each other. What is your view? Best regards Heinrich
rpi: "stdout=serial,vidconsole\0"
Hello Matthias, currently the UEFI sub-systems determines the width of the terminal according to the following rules: If stdout="vidconsole", use the size of the video console, otherwise use the terminal size from the serial console. So if stdout="serial,vidconsole" the size of the video is ignored. With a patch I am about to merge the size of vidconsole will be used if the first 10 characters of stdout are "vidconsole": * stdout="vidconsole": use video console size * stdout="vidconsole,serial": use video console size * stdout="serial,vidconsole": use serial terminal size * stdout="serial": use serial terminal size The difference can for instance be seen when using GRUB_TERMINAL=console. This setting allows GRUB to be displayed both on the serial console as well as on the video console. At least for the Raspberry Pi 400 stdout="vidconsole,serial" seems to be the best choice as it will typically be used with a monitor. Best regards Heinrich
[PATCH 0/2] ANATOP regulator driver
From: "Ying-Chun Liu (PaulLiu)" Anatop is an integrated regulator inside i.MX6 SoC. There are 3 digital regulators which controls PU, CORE (ARM), and SOC. And 3 analog regulators which controls 1P1, 2P5, 3P0 (USB). This patch adds the Anatop regulator driver. Ying-Chun Liu (PaulLiu) (2): power: regulator: add driver for ANATOP regulator doc: device-tree-bindings: regulator: anatop regulator .../regulator/fsl,anatop-regulator.txt| 45 +++ drivers/power/regulator/Kconfig | 10 + drivers/power/regulator/Makefile | 1 + drivers/power/regulator/anatop_regulator.c| 289 ++ 4 files changed, 345 insertions(+) create mode 100644 doc/device-tree-bindings/regulator/fsl,anatop-regulator.txt create mode 100644 drivers/power/regulator/anatop_regulator.c -- 2.30.1
[PATCH 1/2] power: regulator: add driver for ANATOP regulator
From: "Ying-Chun Liu (PaulLiu)" Anatop is an integrated regulator inside i.MX6 SoC. There are 3 digital regulators which controls PU, CORE (ARM), and SOC. And 3 analog regulators which controls 1P1, 2P5, 3P0 (USB). This patch adds the Anatop regulator driver. Signed-off-by: Ying-Chun Liu (PaulLiu) --- drivers/power/regulator/Kconfig| 10 + drivers/power/regulator/Makefile | 1 + drivers/power/regulator/anatop_regulator.c | 289 + 3 files changed, 300 insertions(+) create mode 100644 drivers/power/regulator/anatop_regulator.c diff --git a/drivers/power/regulator/Kconfig b/drivers/power/regulator/Kconfig index fbbea18c7d..1cd1f3f5ed 100644 --- a/drivers/power/regulator/Kconfig +++ b/drivers/power/regulator/Kconfig @@ -312,6 +312,16 @@ config DM_REGULATOR_STPMIC1 by the PMIC device. This driver is controlled by a device tree node which includes voltage limits. +config DM_REGULATOR_ANATOP + bool "Enable driver for ANATOP regulators" + depends on DM_REGULATOR + select REGMAP + select SYSCON + help + Enable support for the Freescale i.MX on-chip ANATOP LDOs + regulators. It is recommended that this option be enabled on + i.MX6 platform. + config SPL_DM_REGULATOR_STPMIC1 bool "Enable driver for STPMIC1 regulators in SPL" depends on SPL_DM_REGULATOR && PMIC_STPMIC1 diff --git a/drivers/power/regulator/Makefile b/drivers/power/regulator/Makefile index 9d58112dcb..e7198da911 100644 --- a/drivers/power/regulator/Makefile +++ b/drivers/power/regulator/Makefile @@ -30,3 +30,4 @@ obj-$(CONFIG_DM_REGULATOR_TPS65910) += tps65910_regulator.o obj-$(CONFIG_DM_REGULATOR_TPS62360) += tps62360_regulator.o obj-$(CONFIG_$(SPL_)DM_REGULATOR_STPMIC1) += stpmic1.o obj-$(CONFIG_DM_REGULATOR_TPS65941) += tps65941_regulator.o +obj-$(CONFIG_$(SPL_)DM_REGULATOR_ANATOP) += anatop_regulator.o diff --git a/drivers/power/regulator/anatop_regulator.c b/drivers/power/regulator/anatop_regulator.c new file mode 100644 index 00..2bb5cdbac5 --- /dev/null +++ b/drivers/power/regulator/anatop_regulator.c @@ -0,0 +1,289 @@ +// SPDX-License-Identifier: GPL-2.0+ +// +// Copyright (C) 2011 Freescale Semiconductor, Inc. All Rights Reserved. +// Copyright (C) 2021 Linaro + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define LDO_RAMP_UP_UNIT_IN_CYCLES 64 /* 64 cycles per step */ +#define LDO_RAMP_UP_FREQ_IN_MHZ 24 /* cycle based on 24M OSC */ + +#define LDO_POWER_GATE 0x00 +#define LDO_FET_FULL_ON0x1f + +struct anatop_regulator { + const char *name; + u32 control_reg; + u32 vol_bit_shift; + u32 vol_bit_width; + u32 min_bit_val; + u32 min_voltage; + u32 max_voltage; + u32 delay_reg; + u32 delay_bit_shift; + u32 delay_bit_width; +}; + +static u32 anatop_get_bits(struct udevice *dev, u32 addr, int bit_shift, + int bit_width) +{ + struct udevice *syscon; + struct regmap *regmap; + int err; + u32 val, mask; + + syscon = dev_get_parent(dev); + if (!syscon) { + dev_err(dev, "%s: unable to find syscon device\n", __func__); + return err; + } + + regmap = syscon_get_regmap(syscon); + if (IS_ERR(regmap)) { + dev_err(dev, "%s: unable to find regmap (%ld)\n", __func__, + PTR_ERR(regmap)); + return PTR_ERR(regmap); + } + + if (bit_width == 32) + mask = ~0; + else + mask = (1 << bit_width) - 1; + + err = regmap_read(regmap, addr, &val); + if (err) + return err; + + val = (val >> bit_shift) & mask; + + return val; +} + +void anatop_set_bits(struct udevice *dev, u32 addr, int bit_shift, +int bit_width, u32 data) +{ + struct udevice *syscon; + struct regmap *regmap; + int err; + u32 val, mask; + + syscon = dev_get_parent(dev); + if (!syscon) { + dev_err(dev, "%s: unable to find syscon device\n", __func__); + return; + } + + regmap = syscon_get_regmap(syscon); + if (IS_ERR(regmap)) { + dev_err(dev, + "%s: unable to find regmap (%ld)\n", __func__, + PTR_ERR(regmap)); + return; + } + + if (bit_width == 32) + mask = ~0; + else + mask = (1 << bit_width) - 1; + + err = regmap_read(regmap, addr, &val); + if (err) { + dev_err(dev, + "%s: cannot read reg (%d)\n", __func__, + err); + return; + } + val = val & ~(mask << bit_shift); + err = regmap_write(reg
[PATCH 2/2] doc: device-tree-bindings: regulator: anatop regulator
From: "Ying-Chun Liu (PaulLiu)" Document the bindings for fsl,anatop-regulator Signed-off-by: Ying-Chun Liu (PaulLiu) --- .../regulator/fsl,anatop-regulator.txt| 45 +++ 1 file changed, 45 insertions(+) create mode 100644 doc/device-tree-bindings/regulator/fsl,anatop-regulator.txt diff --git a/doc/device-tree-bindings/regulator/fsl,anatop-regulator.txt b/doc/device-tree-bindings/regulator/fsl,anatop-regulator.txt new file mode 100644 index 00..2a60e4941b --- /dev/null +++ b/doc/device-tree-bindings/regulator/fsl,anatop-regulator.txt @@ -0,0 +1,45 @@ +ANATOP REGULATOR + +Anatop is an integrated regulator inside i.MX6 SoC. + +Required properties: +- compatible: Must be "fsl,anatop-regulator". +- regulator-name: Name of the regulator +- anatop-reg-offset: u32 value representing the anatop MFD register offset. +- anatop-vol-bit-shift:u32 value representing the bit shift for the register. +- anatop-vol-bit-width:u32 value representing the number of bits used in the + register. +- anatop-min-bit-val: u32 value representing the minimum value of this + register. +- anatop-min-voltage: u32 value representing the minimum voltage of this + regulator. +- anatop-max-voltage: u32 value representing the maximum voltage of this + regulator. + +Optional properties: +- anatop-delay-reg-offset: u32 value representing the anatop MFD step time + register offset. +- anatop-delay-bit-shift: u32 value representing the bit shift for the step + time register. +- anatop-delay-bit-width: u32 value representing the number of bits used in + the step time register. +- anatop-enable-bit: u32 value representing regulator enable bit offset. +- vin-supply:input supply phandle. + +Example: + regulator-vddpu { + compatible = "fsl,anatop-regulator"; + regulator-name = "vddpu"; + regulator-min-microvolt = <725000>; + regulator-max-microvolt = <130>; + regulator-always-on; + anatop-reg-offset = <0x140>; + anatop-vol-bit-shift = <9>; + anatop-vol-bit-width = <5>; + anatop-delay-reg-offset = <0x170>; + anatop-delay-bit-shift = <24>; + anatop-delay-bit-width = <2>; + anatop-min-bit-val = <1>; + anatop-min-voltage = <725000>; + anatop-max-voltage = <130>; + }; -- 2.30.1
Re: Fastboot
Yes that was it! I had used `log help`, but misunderstood the output. My bad... Below is the debug output I receive, where I don't really get what's happening except for those two lines: ``` uclass_find_device_by_seq() - 1 -1 'usb@47401800' uclass_find_device_by_seq() - not found ``` But I don't really understand what it means. I spent a lot of time reading the available options in menuconfig, and at that point I cannot say what I am doing wrong. Note that I am using the `am335x_evm_defconfig` defconfig, which I changed to get more logging outputs. Here is the output of `usb start`, let me know if I am missing something obvious from there: ``` => usb start starting USB... Bus usb@47401800: __of_translate_address() OF: ** translation for device usb@47401800 ** __of_translate_address() OF: bus is default (na=1, ns=1) on usb@4740 __of_translate_address() OF: parent bus is default (na=1, ns=1) on ocp of_translate_one() OF: no ranges, 1:1 translation of_translate_one() OF: with offset: 1195382784 __of_translate_address() OF: parent bus is default (na=1, ns=1) on of_translate_one() OF: no ranges, 1:1 translation of_translate_one() OF: with offset: 1195382784 __of_translate_address() OF: reached root node fdtdec_lookup_phandle() fdtdec_lookup_phandle: phys fdtdec_lookup_phandle() fdtdec_lookup_phandle: ti,ctrl_mod fdtdec_get_addr_size_fixed() fdtdec_get_addr_size_fixed: reg: fdtdec_get_addr_size_fixed() addr=44e10620 fdtdec_get_int() fdtdec_get_int: mentor,multipoint: fdtdec_get_int() 0x1 (1) fdtdec_get_int() fdtdec_get_int: mentor,num-eps: fdtdec_get_int() 0x10 (16) fdtdec_get_int() fdtdec_get_int: mentor,ram-bits: fdtdec_get_int() 0xc (12) fdtdec_get_int() fdtdec_get_int: mentor,power: fdtdec_get_int() 0x1f4 (500) uclass_find_device_by_seq() 0 1 uclass_find_device_by_seq() - 1 -1 'usb@47401800' uclass_find_device_by_seq() - not found clk_set_defaults() clk_set_defaults(usb@47401800) clk_set_default_parents() clk_set_default_parents: could not read assigned-clock-parents for 9df24d30 ofnode_read_prop() ofnode_read_prop: assigned-clock-rates: ofnode_read_prop() musb_core_init() musb-hdrc: hw_ep 0shared, max 64 musb_core_init() musb-hdrc: hw_ep 1tx, max 512 musb_core_init() musb-hdrc: hw_ep 1rx, max 512 musb_core_init() musb-hdrc: hw_ep 2tx, max 512 musb_core_init() musb-hdrc: hw_ep 2rx, max 512 musb_core_init() musb-hdrc: hw_ep 3tx, max 512 musb_core_init() musb-hdrc: hw_ep 3rx, max 512 musb_core_init() musb-hdrc: hw_ep 4tx, max 512 musb_core_init() musb-hdrc: hw_ep 4rx, max 512 musb_core_init() musb-hdrc: hw_ep 5tx, max 512 musb_core_init() musb-hdrc: hw_ep 5rx, max 512 musb_core_init() musb-hdrc: hw_ep 6tx, max 512 musb_core_init() musb-hdrc: hw_ep 6rx, max 512 musb_core_init() musb-hdrc: hw_ep 7tx, max 512 musb_core_init() musb-hdrc: hw_ep 7rx, max 512 musb_core_init() musb-hdrc: hw_ep 8tx, max 512 musb_core_init() musb-hdrc: hw_ep 8rx, max 512 musb_core_init() musb-hdrc: hw_ep 9tx, max 512 musb_core_init() musb-hdrc: hw_ep 9rx, max 512 musb_core_init() musb-hdrc: hw_ep 10tx, max 256 musb_core_init() musb-hdrc: hw_ep 10rx, max 64 musb_core_init() musb-hdrc: hw_ep 11tx, max 256 musb_core_init() musb-hdrc: hw_ep 11rx, max 64 musb_core_init() musb-hdrc: hw_ep 12tx, max 256 musb_core_init() musb-hdrc: hw_ep 12rx, max 64 musb_core_init() musb-hdrc: hw_ep 13shared, max 4096 musb_core_init() musb-hdrc: hw_ep 14shared, max 1024 musb_core_init() musb-hdrc: hw_ep 15shared, max 1024 musb_start() <== devctl 80 musb_stop() HDRC disabled Port not available. usb_init() scan end ``` Best, On Sun, Mar 7, 2021 at 3:04 AM Sean Anderson wrote: > On 3/6/21 8:38 PM, Jonas Vautherin wrote: > > Thanks a lot Sean! > > > > I tried to enable logging by using the following CONFIG_: > > > > ``` > > CONFIG_LOG=y > > CONFIG_SPL_LOG=y > > CONFIG_TPL_LOG=y > > CONFIG_LOG_MAX_LEVEL=5 > > You need to increase this. Log levels are only compiled-in if they are > less than the max log level. > > > CONFIG_SPL_LOG_MAX_LEVEL=3 > > CONFIG_TPL_LOG_MAX_LEVEL=3 > > CONFIG_LOG_DEFAULT_LEVEL=7 > > CONFIG_LOG_CONSOLE=y > > CONFIG_SPL_LOG_CONSOLE=y > > CONFIG_TPL_LOG_CONSOLE=y > > > > CONFIG_CMD_LOG=y > > ``` > > > > And I got the "log" function indeed. But it did not have the effect I > was hoping for: > > > > ``` > > => log level set 7 > > Use the help function ;) > > The correct syntax is "log level 7" > > If you use a newer U-Boot (newer than 2021.01) then the output will be > more verbose. E.g. > > => log level > EMERG > ALERT > CRIT > ERR > WARNING > NOTICE > INFO (default) > DEBUG > > --Sean > > > => log level > > Default log level: 0 > > => usb start > > starting USB... > > Bus usb@47401800: Port not available. > > ``` > > > > First, I don't get why it says "Default log level: 0" after I set it (to > whatever number, apparently). And second, it does not seem to bring me more > output about the usb issue :-/. > > > > Am I missing something? > > > > Best, > > Jonas > > > > On Sat, Mar 6, 2021 at 3:20 AM Sean Anderson sean...@gmail.com>> w
[PATCH 0/2] drivers: usb: phy: add driver for usb-nop-xceiv
From: "Ying-Chun Liu (PaulLiu)" implement the generic NOP USB transceiver for all USB transceiver which are either built-in into USB IP or which are mostly autonomous. Ying-Chun Liu (PaulLiu) (2): drivers: usb: phy: add driver for usb-nop-xceiv doc: device-tree-bindings: phy: usb-nop-xceiv .../phy/usb-nop-xceiv.txt | 43 ++ drivers/usb/phy/Kconfig | 8 + drivers/usb/phy/Makefile | 1 + drivers/usb/phy/usb_nop_phy.c | 137 ++ 4 files changed, 189 insertions(+) create mode 100644 doc/device-tree-bindings/phy/usb-nop-xceiv.txt create mode 100644 drivers/usb/phy/usb_nop_phy.c -- 2.30.1
[PATCH 1/2] drivers: usb: phy: add driver for usb-nop-xceiv
From: "Ying-Chun Liu (PaulLiu)" implement the generic NOP USB transceiver for all USB transceiver which are either built-in into USB IP or which are mostly autonomous. Signed-off-by: Ying-Chun Liu (PaulLiu) --- drivers/usb/phy/Kconfig | 8 ++ drivers/usb/phy/Makefile | 1 + drivers/usb/phy/usb_nop_phy.c | 137 ++ 3 files changed, 146 insertions(+) create mode 100644 drivers/usb/phy/usb_nop_phy.c diff --git a/drivers/usb/phy/Kconfig b/drivers/usb/phy/Kconfig index 8741553d09..663b50a937 100644 --- a/drivers/usb/phy/Kconfig +++ b/drivers/usb/phy/Kconfig @@ -13,3 +13,11 @@ config OMAP_USB_PHY config ROCKCHIP_USB2_PHY bool "Rockchip USB2 PHY" + +config NOP_USB_XCEIV + bool "NOP USB Transceiver Driver" + depends on PHY + help + This driver is to be used by all the usb transceiver which are either + built-in with usb ip or which are autonomous and doesn't require any + phy programming such as ISP1x04 etc. diff --git a/drivers/usb/phy/Makefile b/drivers/usb/phy/Makefile index 20f7edf48d..079e24770f 100644 --- a/drivers/usb/phy/Makefile +++ b/drivers/usb/phy/Makefile @@ -6,3 +6,4 @@ obj-$(CONFIG_TWL4030_USB) += twl4030.o obj-$(CONFIG_OMAP_USB_PHY) += omap_usb_phy.o obj-$(CONFIG_ROCKCHIP_USB2_PHY) += rockchip_usb2_phy.o +obj-$(CONFIG_NOP_USB_XCEIV) += usb_nop_phy.o diff --git a/drivers/usb/phy/usb_nop_phy.c b/drivers/usb/phy/usb_nop_phy.c new file mode 100644 index 00..7f4133af39 --- /dev/null +++ b/drivers/usb/phy/usb_nop_phy.c @@ -0,0 +1,137 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * NOP USB transceiver for all USB transceiver which are either built-in + * into USB IP or which are mostly autonomous. + * + * Copyright (C) 2009 Texas Instruments Inc + * Copyright (C) 2017 Texas Instruments Incorporated - http://www.ti.com/ + * Copyright (C) 2021 Linaro + * Author: Ajay Kumar Gupta + * Jean-Jacques Hiblot + * Ying-Chun Liu (PaulLiu) + * Current status: + * This provides a "nop" transceiver for PHYs which are + * autonomous such as isp1504, isp1707, etc. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +struct usb_nop_phy_priv { + struct clk_bulk bulk; + struct udevice *vcc; + struct gpio_desc *gpiod_reset; + struct gpio_desc *gpiod_vbus; +}; + +static int usb_nop_phy_reset(struct phy *phy) +{ + int ret = 0; + struct usb_nop_phy_priv *priv = dev_get_priv(phy->dev); + + if (!priv->gpiod_reset) + return ret; + + ret = dm_gpio_set_value(priv->gpiod_reset, 1); + mdelay(20); + ret = dm_gpio_set_value(priv->gpiod_reset, 0); + + return ret; +} + +static int usb_nop_phy_init(struct phy *phy) +{ + struct usb_nop_phy_priv *priv = dev_get_priv(phy->dev); + int ret = 0; + + if (CONFIG_IS_ENABLED(DM_REGULATOR) && priv->vcc) { + ret = regulator_set_enable(priv->vcc, true); + if (ret < 0) { + dev_err(phy->dev, "Failed to enable power: %d\n", ret); + return ret; + } + } + + if (CONFIG_IS_ENABLED(CLK)) { + ret = clk_enable_bulk(&priv->bulk); + if (ret < 0) { + dev_err(phy->dev, "Failed to enable clk: %d\n", ret); + return ret; + } + } + + return ret; +} + +static int usb_nop_phy_probe(struct udevice *dev) +{ + struct usb_nop_phy_priv *priv = dev_get_priv(dev); + u32 clk_rate = 0; + int i, ret = 0; + + /* Get all clocks. Actually only main_clk should be assigned */ + if (CONFIG_IS_ENABLED(CLK)) { + ret = clk_get_bulk(dev, &priv->bulk); + if (ret < 0) + dev_info(dev, "failed to get clock: %d\n", ret); + } + + /* Get vcc-supply */ + if (CONFIG_IS_ENABLED(DM_REGULATOR)) { + ret = device_get_supply_regulator(dev, "vcc-supply", &priv->vcc); + if (ret < 0) { + dev_info(dev, "failed to get vcc-supply: %d\n", ret); + priv->vcc = NULL; + } + } + + ret = ofnode_read_u32(dev_ofnode(dev), "clock-frequency", &clk_rate); + if (ret < 0) + clk_rate = 0; + + priv->gpiod_reset = devm_gpiod_get_optional(dev, "reset", 0); + if (priv->gpiod_reset) + priv->gpiod_vbus = devm_gpiod_get_optional(dev, + "vbus-detect", + 0); + + if (clk_rate) + for (i = 0; i < priv->bulk.count; i++) { + ret = clk_set_rate(&priv->bulk.clks[i], clk_rate); + if (ret < 0) { + dev_err(dev, + "Failed to set
[PATCH 2/2] doc: device-tree-bindings: phy: usb-nop-xceiv
From: "Ying-Chun Liu (PaulLiu)" Document the bindings for usb-nop-xceiv Signed-off-by: Ying-Chun Liu (PaulLiu) --- .../phy/usb-nop-xceiv.txt | 43 +++ 1 file changed, 43 insertions(+) create mode 100644 doc/device-tree-bindings/phy/usb-nop-xceiv.txt diff --git a/doc/device-tree-bindings/phy/usb-nop-xceiv.txt b/doc/device-tree-bindings/phy/usb-nop-xceiv.txt new file mode 100644 index 00..4dc6a8ee30 --- /dev/null +++ b/doc/device-tree-bindings/phy/usb-nop-xceiv.txt @@ -0,0 +1,43 @@ +USB NOP PHY + +Required properties: +- compatible: should be usb-nop-xceiv +- #phy-cells: Must be 0 + +Optional properties: +- clocks: phandle to the PHY clock. Use as per Documentation/devicetree + /bindings/clock/clock-bindings.txt + This property is required if clock-frequency is specified. + +- clock-names: Should be "main_clk" + +- clock-frequency: the clock frequency (in Hz) that the PHY clock must + be configured to. + +- vcc-supply: phandle to the regulator that provides power to the PHY. + +- reset-gpios: Should specify the GPIO for reset. + +- vbus-detect-gpio: should specify the GPIO detecting a VBus insertion +(see Documentation/devicetree/bindings/gpio/gpio.txt) +- vbus-regulator : should specifiy the regulator supplying current drawn from + the VBus line (see Documentation/devicetree/bindings/regulator/regulator.txt). + +Example: + + hsusb1_phy { + compatible = "usb-nop-xceiv"; + clock-frequency = <1920>; + clocks = <&osc 0>; + clock-names = "main_clk"; + vcc-supply = <&hsusb1_vcc_regulator>; + reset-gpios = <&gpio1 7 GPIO_ACTIVE_LOW>; + vbus-detect-gpio = <&gpio2 13 GPIO_ACTIVE_HIGH>; + vbus-regulator = <&vbus_regulator>; + #phy-cells = <0>; + }; + +hsusb1_phy is a NOP USB PHY device that gets its clock from an oscillator +and expects that clock to be configured to 19.2MHz by the NOP PHY driver. +hsusb1_vcc_regulator provides power to the PHY and GPIO 7 controls RESET. +GPIO 13 detects VBus insertion, and accordingly notifies the vbus-regulator. -- 2.30.1
Pull request for efi-2021-04-rc3-3
Dear Tom, The following changes since commit e4dba4ba6f61e8128be0b4200ca2d8cebf62180b: Merge https://gitlab.denx.de/u-boot/custodians/u-boot-fsl-qoriq (2021-03-06 07:25:04 -0500) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-efi.git tags/efi-2021-04-rc3-3 for you to fetch changes up to 7d3eff3412886f277c724a9effcbe545c4cdd5b5: efi_loader: correct uboot_bin_env.its file format (2021-03-07 17:37:26 +0100) Gitlab CI showed no problems: https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/6683 Pull request for efi-2021-04-rc3-3 New: * Provide library functions for converting UTF-8 streams either to code page 437 or Unicode code points. Bug fixes: * Fix the capsule update unit tests. * Use the terminal size of the video console if it is the primary output. Heinrich Schuchardt (6): efi_loader: console size of vidconsole efi_loader: move codepage 437 table efi_loader: carve out utf_to_cp() lib/charset: utf8_get() should return error lib/charset: UTF-8 stream conversion efi_loader: correct uboot_bin_env.its file format include/charset.h| 34 +++ lib/charset.c| 96 +-- lib/efi_loader/efi_console.c | 2 +- lib/efi_loader/efi_unicode_collation.c | 21 + test/py/tests/test_efi_capsule/uboot_bin_env.its | 4 +- test/unicode_ut.c| 114 +++ 6 files changed, 242 insertions(+), 29 deletions(-)
[PATCH 01/20] binman: Allow extracting to current directory
Extracting files to the current directory is not normally a very friendly thing to do, but it can be warranted, e.g. in a new temporary dir. At present binman reports an error when such an attempt is made. Fix it. Signed-off-by: Simon Glass --- tools/binman/control.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/binman/control.py b/tools/binman/control.py index 1952b2abf48..9c0cafeafc4 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -241,7 +241,7 @@ def ExtractEntries(image_fname, output_fname, outdir, entry_paths, # If this entry has children, create a directory for it and put its # data in a file called 'root' in that directory if entry.GetEntries(): -if not os.path.exists(fname): +if fname and not os.path.exists(fname): os.makedirs(fname) fname = os.path.join(fname, 'root') tout.Notice("Write entry '%s' size %x to '%s'" % -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH 00/20] binman: Support devicetree update in all entries
U-Boot's various phase binaries actually comprise two or three pieces. For example, u-boot.bin has the executable followed by a devicetree. With binman we want to be able to update that devicetree with full image information so that it is accessible to the executable. This is tricky if it is not clear where the devicetree starts. At present this is handled manually, by creating two separate nodes in the binman definition, u-boot-nodtb and u-boot-dtb, so that binman sees them separately and can easily update the devicetree. But this is not very convenient. Of course we could try to detect the devicetree in the binary, which is easy enough for binman to do. But this approach rapidly gets out of hand. When listing an existing image, should we show the devicetree separately? If we don't then binman cannot extract it by itself, only the whole binary. With SPL and TPL we may have BSS padding and this is harder to detect reliably. Also, a design goal of binman is to provide full information about what is in the image. The detection approach goes against that. So instead it seems best to convert these multi-part binaries into a section with the component parts inside. We can do this automatically without affecting the operation of binman very much. One implementation option for this feature would be to update the entries to behave in two different ways, i.e. either to act as a section containing sub-entries, or as a simple blob. However this is tricky because each entry has to use a particular base class. It is not reasonably possible for an entry to choose between using one base class (e.g. blob) or another (e.g. section), depending on a flag. Instead, the approach taken in this series is to replace an entry type with an expanded version, so 'u-boot' becomes 'u-boot-expanded', for example. The new entry is a section containing two or three sub-entries. A similar approach could be take to reduce the complexity of the x86 binman definition. The microcode is either in TPL, SPL or U-Boot proper, so an entryarg could control which of those is expanded to include microcode. That possibility is left for future consideration. Simon Glass (20): binman: Allow extracting to current directory binman: Document ExpandEntries() in the base class binman: Update entry help for files-align binman: Tidy up underscores in entry documentation binman: Correct the documentation for u-boot-spl-bss-pad binman: Add support for u-boot-tpl-nodtb binman: Add support for u-boot-tpl-bss-bad binman: Allow using an an 'expanded' entry type binman: Allow a way to select expanded entries binman: Plumb expanded entries through fully binman: Automatically expand phase binaries into sections Makefile: Pass new entry args to binman x86: Make use of binman expanded entries x86: dts: Drop unused CONFIG_SPL doc: Move UEFI down a bit binman: doc: Add documentation to htmldocs binman: Rearrange documentation into headings binman: Incorporate entry documentation binman: Drop repetitive heading for each entry binman: Update various pieces of the documentation Makefile | 8 + arch/x86/dts/u-boot.dtsi | 17 +- doc/index.rst| 25 +- doc/package/binman | 1 + doc/package/fit.rst | 8 + doc/package/index.rst| 20 + tools/binman/{README => README.rst} | 955 ++- tools/binman/cmdline.py | 5 +- tools/binman/control.py | 28 +- tools/binman/{README.entries => entries.rst} | 318 -- tools/binman/entry.py| 71 +- tools/binman/entry_test.py | 12 + tools/binman/etype/atf_bl31.py | 2 +- tools/binman/etype/blob.py | 4 +- tools/binman/etype/blob_ext.py | 2 +- tools/binman/etype/blob_phase.py | 51 + tools/binman/etype/cbfs.py | 12 +- tools/binman/etype/fdtmap.py | 30 +- tools/binman/etype/files.py | 2 +- tools/binman/etype/fit.py| 21 +- tools/binman/etype/intel_cmc.py | 2 +- tools/binman/etype/intel_fsp.py | 2 +- tools/binman/etype/intel_fsp_m.py| 2 +- tools/binman/etype/intel_fsp_s.py| 2 +- tools/binman/etype/intel_fsp_t.py| 2 +- tools/binman/etype/intel_ifwi.py | 10 +- tools/binman/etype/intel_me.py | 2 +- tools/binman/etype/intel_mrc.py | 2 +- tools/binman/etype/intel_refcode.py | 2 +- tools/binman/etype/intel_vbt.py | 2 +- tools/binman/etype/intel_vga.py | 2 +- tools/binman/etype/mkimage.py| 4 +- tools/binman/etype/scp.py| 2 +- tools/binman/etype/section.py| 15 +-
[PATCH 03/20] binman: Update entry help for files-align
Regenerate the entry documentation, which step was missed when the files-align feature was added. Fixes: 6eb9932668f ("binman: Support alignment of files") Signed-off-by: Simon Glass --- tools/binman/README.entries | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 999b77690f0..7cca030409e 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -302,6 +302,7 @@ Properties / Entry arguments: - files-compress: Compression algorithm to use: none: No compression lz4: Use lz4 compression (via 'lz4' command-line utility) +- files-align: Align each file to the given alignment This entry reads a number of files and places each in a separate sub-entry within this entry. To access these you need to enable device-tree updates -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH 05/20] binman: Correct the documentation for u-boot-spl-bss-pad
The documentation for this entry indicates that the SPL binary is included along with the padding. It is not, so update it to correct the error. Signed-off-by: Simon Glass --- tools/binman/README.entries | 17 ++--- tools/binman/etype/u_boot_spl_bss_pad.py | 17 ++--- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 3fbc06d9264..5c6663e2c7c 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -960,13 +960,16 @@ Entry: u-boot-spl-bss-pad: U-Boot SPL binary padded with a BSS region Properties / Entry arguments: None -This is similar to u_boot_spl except that padding is added after the SPL -binary to cover the BSS (Block Started by Symbol) region. This region holds -the various used by SPL. It is set to 0 by SPL when it starts up. If you -want to append data to the SPL image (such as a device tree file), you must -pad out the BSS region to avoid the data overlapping with U-Boot variables. -This entry is useful in that case. It automatically pads out the entry size -to cover both the code, data and BSS. +This holds the padding added after the SPL binary to cover the BSS (Block +Started by Symbol) region. This region holds the various variables used by +SPL. It is set to 0 by SPL when it starts up. If you want to append data to +the SPL image (such as a device tree file), you must pad out the BSS region +to avoid the data overlapping with U-Boot variables. This entry is useful in +that case. It automatically pads out the entry size to cover both the code, +data and BSS. + +The contents of this entry will a certain number of zero bytes, determined +by __bss_size The ELF file 'spl/u-boot-spl' must also be available for this to work, since binman uses that to look up the BSS address. diff --git a/tools/binman/etype/u_boot_spl_bss_pad.py b/tools/binman/etype/u_boot_spl_bss_pad.py index df15cd24ce7..18c5596bd35 100644 --- a/tools/binman/etype/u_boot_spl_bss_pad.py +++ b/tools/binman/etype/u_boot_spl_bss_pad.py @@ -18,13 +18,16 @@ class Entry_u_boot_spl_bss_pad(Entry_blob): Properties / Entry arguments: None -This is similar to u_boot_spl except that padding is added after the SPL -binary to cover the BSS (Block Started by Symbol) region. This region holds -the various used by SPL. It is set to 0 by SPL when it starts up. If you -want to append data to the SPL image (such as a device tree file), you must -pad out the BSS region to avoid the data overlapping with U-Boot variables. -This entry is useful in that case. It automatically pads out the entry size -to cover both the code, data and BSS. +This holds the padding added after the SPL binary to cover the BSS (Block +Started by Symbol) region. This region holds the various variables used by +SPL. It is set to 0 by SPL when it starts up. If you want to append data to +the SPL image (such as a device tree file), you must pad out the BSS region +to avoid the data overlapping with U-Boot variables. This entry is useful in +that case. It automatically pads out the entry size to cover both the code, +data and BSS. + +The contents of this entry will a certain number of zero bytes, determined +by __bss_size The ELF file 'spl/u-boot-spl' must also be available for this to work, since binman uses that to look up the BSS address. -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH 04/20] binman: Tidy up underscores in entry documentation
Several entries currently use an underscore in the entry-type name, but in fact a hyphen is used. Update the docs to fix this as it might be confusing. Also simplify the 'filename' comment and fix the 'operation' typo. Signed-off-by: Simon Glass --- tools/binman/README.entries | 23 ++--- tools/binman/etype/blob.py | 2 +- tools/binman/etype/u_boot.py| 2 +- tools/binman/etype/u_boot_dtb_with_ucode.py | 4 ++-- tools/binman/etype/u_boot_nodtb.py | 6 +++--- tools/binman/etype/u_boot_spl_nodtb.py | 7 +++ tools/binman/etype/u_boot_with_ucode_ptr.py | 2 +- 7 files changed, 22 insertions(+), 24 deletions(-) diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 7cca030409e..3fbc06d9264 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -39,7 +39,7 @@ Properties / Entry arguments: This entry reads data from a file and places it in the entry. The default filename is often specified specified by the subclass. See for -example the 'u_boot' entry which provides the filename 'u-boot.bin'. +example the 'u-boot' entry which provides the filename 'u-boot.bin'. If compression is enabled, an extra 'uncomp-size' property is written to the node (if enabled with -u) which provides the uncompressed size of the @@ -840,7 +840,7 @@ Properties / Entry arguments: This is the U-Boot binary, containing relocation information to allow it to relocate itself at runtime. The binary typically includes a device tree -blob at the end of it. Use u_boot_nodtb if you want to package the device +blob at the end of it. Use u-boot-nodtb if you want to package the device tree separately. U-Boot can access binman symbols at runtime. See: @@ -876,9 +876,9 @@ See Entry_u_boot_ucode for full details of the three entries involved in this process. This entry provides the U-Boot device-tree file, which contains the microcode. If the microcode is not being collated into one place then the offset and size of the microcode is recorded by this entry, -for use by u_boot_with_ucode_ptr. If it is being collated, then this +for use by u-boot-with-ucode_ptr. If it is being collated, then this entry deletes the microcode from the device tree (to save space) and makes -it available to u_boot_ucode. +it available to u-boot-ucode. @@ -920,12 +920,12 @@ Entry: u-boot-nodtb: U-Boot flat binary without device tree appended Properties / Entry arguments: -- filename: Filename of u-boot.bin (default 'u-boot-nodtb.bin') +- filename: Filename to include (default 'u-boot-nodtb.bin') This is the U-Boot binary, containing relocation information to allow it to relocate itself at runtime. It does not include a device tree blob at -the end of it so normally cannot work without it. You can add a u_boot_dtb -entry after this one, or use a u_boot entry instead (which contains both +the end of it so normally cannot work without it. You can add a u-boot-dtb +entry after this one, or use a u-boot entry instead (which contains both U-Boot and the device tree). @@ -1000,13 +1000,12 @@ Entry: u-boot-spl-nodtb: SPL binary without device tree appended Properties / Entry arguments: -- filename: Filename of spl/u-boot-spl-nodtb.bin (default -'spl/u-boot-spl-nodtb.bin') +- filename: Filename to include (default 'spl/u-boot-spl-nodtb.bin') This is the U-Boot SPL binary, It does not include a device tree blob at the end of it so may not be able to work without it, assuming SPL needs -a device tree to operation on your platform. You can add a u_boot_spl_dtb -entry after this one, or use a u_boot_spl entry instead (which contains +a device tree to operate on your platform. You can add a u-boot-spl-dtb +entry after this one, or use a u-boot-spl entry instead (which contains both SPL and the device tree). @@ -1148,7 +1147,7 @@ Properties / Entry arguments: See Entry_u_boot_ucode for full details of the three entries involved in this process. This entry updates U-Boot with the offset and size of the microcode, to allow early x86 boot code to find it without doing anything -complicated. Otherwise it is the same as the u_boot entry. +complicated. Otherwise it is the same as the u-boot entry. diff --git a/tools/binman/etype/blob.py b/tools/binman/etype/blob.py index 81756c326d9..e609a8b253f 100644 --- a/tools/binman/etype/blob.py +++ b/tools/binman/etype/blob.py @@ -24,7 +24,7 @@ class Entry_blob(Entry): This entry reads data from a file and places it in the entry. The default filename is often specified specified by the subclass. See for -example the 'u_boot' entry which provides the filename 'u-boot.bin'. +example the 'u-boot' entry which provides the filename 'u-boot.bin'. If compression is enabled, an extra 'u
[PATCH 02/20] binman: Document ExpandEntries() in the base class
Move the documentation to the base method as it is with other methods. Also update it a little while we are here. Signed-off-by: Simon Glass --- tools/binman/entry.py | 11 +++ tools/binman/etype/section.py | 6 -- 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/tools/binman/entry.py b/tools/binman/entry.py index d58a730f3d5..507760e2a86 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -211,6 +211,17 @@ class Entry(object): return {} def ExpandEntries(self): +"""Expand out entries which produce other entries + +Some entries generate subnodes automatically, from which sub-entries +are then created. This method allows those to be added to the binman +definition for the current image. An entry which implements this method +should call state.AddSubnode() to add a subnode and can add properties +with state.AddString(), etc. + +An example is 'files', which produces a section containing a list of +files. +""" pass def AddMissingProperties(self, have_image_pos): diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 1ceadef13f3..2103919b0c3 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -126,12 +126,6 @@ class Entry_section(Entry): return True def ExpandEntries(self): -"""Expand out any entries which have calculated sub-entries - -Some entries are expanded out at runtime, e.g. 'files', which produces -a section containing a list of files. Process these entries so that -this information is added to the device tree. -""" super().ExpandEntries() for entry in self._entries.values(): entry.ExpandEntries() -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH 07/20] binman: Add support for u-boot-tpl-bss-bad
This entry holds the padding between the end of of TPL binary and the end of BSS. This region must be left empty so that the devicetree can be appended correctly and remain accessible without interfering with BSS. Signed-off-by: Simon Glass --- tools/binman/README.entries | 22 tools/binman/etype/u_boot_tpl_bss_pad.py | 44 tools/binman/ftest.py| 16 + tools/binman/test/193_tpl_bss_pad.dts| 19 ++ 4 files changed, 101 insertions(+) create mode 100644 tools/binman/etype/u_boot_tpl_bss_pad.py create mode 100644 tools/binman/test/193_tpl_bss_pad.dts diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 253c579a62d..cd15073e6df 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -1047,6 +1047,28 @@ binman uses that to look up symbols to write into the TPL binary. +Entry: u-boot-tpl-bss-pad: U-Boot TPL binary padded with a BSS region +- + +Properties / Entry arguments: +None + +This holds the padding added after the TPL binary to cover the BSS (Block +Started by Symbol) region. This region holds the various variables used by +TPL. It is set to 0 by TPL when it starts up. If you want to append data to +the TPL image (such as a device tree file), you must pad out the BSS region +to avoid the data overlapping with U-Boot variables. This entry is useful in +that case. It automatically pads out the entry size to cover both the code, +data and BSS. + +The contents of this entry will a certain number of zero bytes, determined +by __bss_size + +The ELF file 'tpl/u-boot-tpl' must also be available for this to work, since +binman uses that to look up the BSS address. + + + Entry: u-boot-tpl-dtb: U-Boot TPL device tree - diff --git a/tools/binman/etype/u_boot_tpl_bss_pad.py b/tools/binman/etype/u_boot_tpl_bss_pad.py new file mode 100644 index 000..521b24a384e --- /dev/null +++ b/tools/binman/etype/u_boot_tpl_bss_pad.py @@ -0,0 +1,44 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2021 Google LLC +# Written by Simon Glass +# +# Entry-type module for BSS padding for tpl/u-boot-tpl.bin. This padding +# can be added after the TPL binary to ensure that anything concatenated +# to it will appear to TPL to be at the end of BSS rather than the start. +# + +from binman import elf +from binman.entry import Entry +from binman.etype.blob import Entry_blob +from patman import tools + +class Entry_u_boot_tpl_bss_pad(Entry_blob): +"""U-Boot TPL binary padded with a BSS region + +Properties / Entry arguments: +None + +This holds the padding added after the TPL binary to cover the BSS (Block +Started by Symbol) region. This region holds the various variables used by +TPL. It is set to 0 by TPL when it starts up. If you want to append data to +the TPL image (such as a device tree file), you must pad out the BSS region +to avoid the data overlapping with U-Boot variables. This entry is useful in +that case. It automatically pads out the entry size to cover both the code, +data and BSS. + +The contents of this entry will a certain number of zero bytes, determined +by __bss_size + +The ELF file 'tpl/u-boot-tpl' must also be available for this to work, since +binman uses that to look up the BSS address. +""" +def __init__(self, section, etype, node): +super().__init__(section, etype, node) + +def ObtainContents(self): +fname = tools.GetInputFilename('tpl/u-boot-tpl') +bss_size = elf.GetSymbolAddress(fname, '__bss_size') +if not bss_size: +self.Raise('Expected __bss_size symbol in tpl/u-boot-tpl') +self.SetContents(tools.GetBytes(0, bss_size)) +return True diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index b989dd46caf..10edeab8784 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -4251,6 +4251,22 @@ class TestFunctional(unittest.TestCase): self.assertEqual(U_BOOT_TPL_NODTB_DATA, data[:len(U_BOOT_TPL_NODTB_DATA)]) +def testTplBssPad(self): +"""Test that we can pad TPL's BSS with zeros""" +# ELF file with a '__bss_size' symbol +self._SetupTplElf() +data = self._DoReadFile('193_tpl_bss_pad.dts') +self.assertEqual(U_BOOT_TPL_DATA + tools.GetBytes(0, 10) + U_BOOT_DATA, + data) + +def testTplBssPadMissing(self): +"""Test that a missing symbol is detected""" +self._SetupTplElf('u_boot_ucode_ptr') +with self.assertRaises(ValueError) as e: +self._DoReadFile('193_tpl_bss_pad.dts') +self.assertIn('Expected __bss_size symbol in tpl/u-boot-tpl', + str(e.exception)) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman
[PATCH 06/20] binman: Add support for u-boot-tpl-nodtb
Allow this entry type to be placed in an image. This is the TPL binary, without a devicetree appended. Signed-off-by: Simon Glass --- tools/binman/README.entries| 14 +++ tools/binman/etype/u_boot_tpl_nodtb.py | 27 ++ tools/binman/ftest.py | 6 + tools/binman/test/192_u_boot_tpl_nodtb.dts | 13 +++ 4 files changed, 60 insertions(+) create mode 100644 tools/binman/etype/u_boot_tpl_nodtb.py create mode 100644 tools/binman/test/192_u_boot_tpl_nodtb.dts diff --git a/tools/binman/README.entries b/tools/binman/README.entries index 5c6663e2c7c..253c579a62d 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -1080,6 +1080,20 @@ be relocated to any address for execution. +Entry: u-boot-tpl-nodtb: TPL binary without device tree appended + + +Properties / Entry arguments: +- filename: Filename to include (default 'tpl/u-boot-tpl-nodtb.bin') + +This is the U-Boot TPL binary, It does not include a device tree blob at +the end of it so may not be able to work without it, assuming TPL needs +a device tree to operate on your platform. You can add a u-boot-tpl-dtb +entry after this one, or use a u-boot-tpl entry instead (which contains +both TPL and the device tree). + + + Entry: u-boot-tpl-with-ucode-ptr: U-Boot TPL with embedded microcode pointer diff --git a/tools/binman/etype/u_boot_tpl_nodtb.py b/tools/binman/etype/u_boot_tpl_nodtb.py new file mode 100644 index 000..eaeebcadf77 --- /dev/null +++ b/tools/binman/etype/u_boot_tpl_nodtb.py @@ -0,0 +1,27 @@ +# SPDX-License-Identifier: GPL-2.0+ +# Copyright 2021 Google LLC +# Written by Simon Glass +# +# Entry-type module for 'u-boot-tpl-nodtb.bin' +# + +from binman.entry import Entry +from binman.etype.blob import Entry_blob + +class Entry_u_boot_tpl_nodtb(Entry_blob): +"""TPL binary without device tree appended + +Properties / Entry arguments: +- filename: Filename to include (default 'tpl/u-boot-tpl-nodtb.bin') + +This is the U-Boot TPL binary, It does not include a device tree blob at +the end of it so may not be able to work without it, assuming TPL needs +a device tree to operate on your platform. You can add a u-boot-tpl-dtb +entry after this one, or use a u-boot-tpl entry instead (which contains +both TPL and the device tree). +""" +def __init__(self, section, etype, node): +super().__init__(section, etype, node) + +def GetDefaultFilename(self): +return 'tpl/u-boot-tpl-nodtb.bin' diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 814e91d42e9..b989dd46caf 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -4245,6 +4245,12 @@ class TestFunctional(unittest.TestCase): self.assertEquals(U_BOOT_DATA, u_boot.ReadData()) +def testTplNoDtb(self): +"""Test that an image with tpl/u-boot-tpl-nodtb.bin can be created""" +data = self._DoReadFile('192_u_boot_tpl_nodtb.dts') +self.assertEqual(U_BOOT_TPL_NODTB_DATA, + data[:len(U_BOOT_TPL_NODTB_DATA)]) + if __name__ == "__main__": unittest.main() diff --git a/tools/binman/test/192_u_boot_tpl_nodtb.dts b/tools/binman/test/192_u_boot_tpl_nodtb.dts new file mode 100644 index 000..94cef395e89 --- /dev/null +++ b/tools/binman/test/192_u_boot_tpl_nodtb.dts @@ -0,0 +1,13 @@ +// SPDX-License-Identifier: GPL-2.0+ + +/dts-v1/; + +/ { + #address-cells = <1>; + #size-cells = <1>; + + binman { + u-boot-tpl-nodtb { + }; + }; +}; -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH 09/20] binman: Allow a way to select expanded entries
Add a new command-line option to disable expanded entries. This is needed for most tests, since it is much easier to 'factor out' this function into a separate test and keep the existing packing tests simple. Add the option and select it by default from tests. Signed-off-by: Simon Glass --- tools/binman/cmdline.py | 3 +++ tools/binman/ftest.py | 19 +-- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py index c007d0a036d..0c0f48951f6 100644 --- a/tools/binman/cmdline.py +++ b/tools/binman/cmdline.py @@ -56,6 +56,9 @@ controlled by a description in the board device tree.''' default=False, help='Output a map file for each image') build_parser.add_argument('-M', '--allow-missing', action='store_true', default=False, help='Allow external blobs to be missing') +build_parser.add_argument('-n', '--no-expanded', action='store_true', +help="Don't use 'expanded' versions of entries where available; " + "normally 'u-boot' becomes 'u-boot-expanded', for example") build_parser.add_argument('-O', '--outdir', type=str, action='store', help='Path to directory to use for intermediate and ' 'output files') diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py index 10edeab8784..2a0e0460daa 100644 --- a/tools/binman/ftest.py +++ b/tools/binman/ftest.py @@ -305,7 +305,8 @@ class TestFunctional(unittest.TestCase): def _DoTestFile(self, fname, debug=False, map=False, update_dtb=False, entry_args=None, images=None, use_real_dtb=False, -verbosity=None, allow_missing=False, extra_indirs=None): +use_expanded=False, verbosity=None, allow_missing=False, +extra_indirs=None): """Run binman with a given test file Args: @@ -322,6 +323,8 @@ class TestFunctional(unittest.TestCase): the u-boot-dtb entry. Normally this is not needed and the test contents (the U_BOOT_DTB_DATA string) can be used. But in some test we need the real contents. +use_expanded: True to use expanded entries where available, e.g. +'u-boot-expanded' instead of 'u-boot' verbosity: Verbosity level to use (0-3, None=don't set it) allow_missing: Set the '--allow-missing' flag so that missing external binaries just produce a warning instead of an error @@ -344,6 +347,8 @@ class TestFunctional(unittest.TestCase): args.append('-u') if not use_real_dtb: args.append('--fake-dtb') +if not use_expanded: +args.append('--no-expanded') if entry_args: for arg, value in entry_args.items(): args.append('-a%s=%s' % (arg, value)) @@ -403,9 +408,9 @@ class TestFunctional(unittest.TestCase): dtb.Pack() return dtb.GetContents() -def _DoReadFileDtb(self, fname, use_real_dtb=False, map=False, - update_dtb=False, entry_args=None, reset_dtbs=True, - extra_indirs=None): +def _DoReadFileDtb(self, fname, use_real_dtb=False, use_expanded=False, + map=False, update_dtb=False, entry_args=None, + reset_dtbs=True, extra_indirs=None): """Run binman and return the resulting image This runs binman with a given test file and then reads the resulting @@ -420,6 +425,8 @@ class TestFunctional(unittest.TestCase): the u-boot-dtb entry. Normally this is not needed and the test contents (the U_BOOT_DTB_DATA string) can be used. But in some test we need the real contents. +use_expanded: True to use expanded entries where available, e.g. +'u-boot-expanded' instead of 'u-boot' map: True to output map files for the images update_dtb: Update the offset and size of each entry in the device tree before packing it into the image @@ -454,7 +461,7 @@ class TestFunctional(unittest.TestCase): try: retcode = self._DoTestFile(fname, map=map, update_dtb=update_dtb, entry_args=entry_args, use_real_dtb=use_real_dtb, -extra_indirs=extra_indirs) +use_expanded=use_expanded, extra_indirs=extra_indirs) self.assertEqual(0, retcode) out_dtb_fname = tools.GetOutputFilename('u-boot.dtb.out') @@ -652,7 +659,7 @@ class TestFunctional(unittest.TestCase): """Test that we can run it with a specific board""" self._SetupDtb('005_simple.dts', 'sandbox/u-boot.dtb') TestFunctional._MakeInputFile('sandbox/u-boot.bin', U_BOOT_DATA) -result = self._DoBinman('build', '-b', 'sandbox') +result = self._DoBinman('build', '-n', '-b', 'sandbox') self.assertEqu
[PATCH 08/20] binman: Allow using an an 'expanded' entry type
As the first step in supporting expanded entries, add a way for binman to automatically select an 'expanded' version of an entry type, if requested. This is controlled by a class method. Signed-off-by: Simon Glass --- tools/binman/entry.py | 60 -- tools/binman/entry_test.py | 12 2 files changed, 63 insertions(+), 9 deletions(-) diff --git a/tools/binman/entry.py b/tools/binman/entry.py index 507760e2a86..1651a38255e 100644 --- a/tools/binman/entry.py +++ b/tools/binman/entry.py @@ -102,22 +102,30 @@ class Entry(object): self.allow_missing = False @staticmethod -def Lookup(node_path, etype): +def Lookup(node_path, etype, expanded): """Look up the entry class for a node. Args: node_node: Path name of Node object containing information about the entry to create (used for errors) etype: Entry type to use +expanded: Use the expanded version of etype Returns: -The entry class object if found, else None +The entry class object if found, else None if not found and expanded +is True + +Raise: +ValueError if expanded is False and the class is not found """ # Convert something like 'u-boot@0' to 'u_boot' since we are only # interested in the type. module_name = etype.replace('-', '_') + if '@' in module_name: module_name = module_name.split('@')[0] +if expanded: +module_name += '_expanded' module = modules.get(module_name) # Also allow entry-type modules to be brought in from the etype directory. @@ -127,6 +135,8 @@ class Entry(object): try: module = importlib.import_module('binman.etype.' + module_name) except ImportError as e: +if expanded: +return None raise ValueError("Unknown entry type '%s' in node '%s' (expected etype/%s.py, error '%s'" % (etype, node_path, module_name, e)) modules[module_name] = module @@ -135,21 +145,31 @@ class Entry(object): return getattr(module, 'Entry_%s' % module_name) @staticmethod -def Create(section, node, etype=None): +def Create(section, node, etype=None, expanded=False): """Create a new entry for a node. Args: -section: Section object containing this node -node:Node object containing information about the entry to - create -etype: Entry type to use, or None to work it out (used for tests) +section: Section object containing this node +node: Node object containing information about the entry to + create +etype:Entry type to use, or None to work it out (used for tests) +expanded: True to use expanded versions of entries, where available Returns: A new Entry object of the correct type (a subclass of Entry) """ if not etype: etype = fdt_util.GetString(node, 'type', node.name) -obj = Entry.Lookup(node.path, etype) +obj = Entry.Lookup(node.path, etype, expanded) +if obj and expanded: +# Check whether to use the expanded entry +new_etype = etype + '-expanded' +if obj.UseExpanded(node, etype, new_etype): +etype = new_etype +else: +obj = None +if not obj: +obj = Entry.Lookup(node.path, etype, False) # Call its constructor to get the object we want. return obj(section, etype, node) @@ -648,7 +668,7 @@ features to produce new behaviours. modules.remove('_testing') missing = [] for name in modules: -module = Entry.Lookup('WriteDocs', name) +module = Entry.Lookup('WriteDocs', name, False) docs = getattr(module, '__doc__') if test_missing == name: docs = None @@ -907,3 +927,25 @@ features to produce new behaviours. self.uncomp_size = len(indata) data = tools.Compress(indata, self.compress) return data + +@classmethod +def UseExpanded(cls, node, etype, new_etype): +"""Check whether to use an expanded entry type + +This is called by Entry.Create() when it finds an expanded version of +an entry type (e.g. 'u-boot-expanded'). If this method returns True then +it will be used (e.g. in place of 'u-boot'). If it returns False, it is +ignored. + +Args: +node: Node object containing information about the entry to + create +etype:Original entry type being used +new_etype: New entry type proposed + +Ret
[PATCH 10/20] binman: Plumb expanded entries through fully
Add support for this feature in the control, image and section modules, so that expanded entries will be selected by default. So far there are no expanded entry types, so this is a nop. Signed-off-by: Simon Glass --- tools/binman/control.py | 24 ++-- tools/binman/etype/section.py | 3 ++- tools/binman/image.py | 17 - 3 files changed, 36 insertions(+), 8 deletions(-) diff --git a/tools/binman/control.py b/tools/binman/control.py index 9c0cafeafc4..9709aa9a2b2 100644 --- a/tools/binman/control.py +++ b/tools/binman/control.py @@ -28,7 +28,7 @@ images = OrderedDict() #value: Text for the help missing_blob_help = {} -def _ReadImageDesc(binman_node): +def _ReadImageDesc(binman_node, use_expanded): """Read the image descriptions from the /binman node This normally produces a single Image object called 'image'. But if @@ -36,15 +36,17 @@ def _ReadImageDesc(binman_node): Args: binman_node: Node object of the /binman node +use_expanded: True if the FDT will be updated with the entry information Returns: OrderedDict of Image objects, each of which describes an image """ images = OrderedDict() if 'multiple-images' in binman_node.props: for node in binman_node.subnodes: -images[node.name] = Image(node.name, node) +images[node.name] = Image(node.name, node, + use_expanded=use_expanded) else: -images['image'] = Image('image', binman_node) +images['image'] = Image('image', binman_node, use_expanded=use_expanded) return images def _FindBinmanNode(dtb): @@ -399,7 +401,7 @@ def ReplaceEntries(image_fname, input_fname, indir, entry_paths, return image -def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt): +def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt, use_expanded): """Prepare the images to be processed and select the device tree This function: @@ -413,6 +415,9 @@ def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt): dtb_fname: Filename of the device tree file to use (.dts or .dtb) selected_images: List of images to output, or None for all update_fdt: True to update the FDT wth entry offsets, etc. +use_expanded: True to use expanded versions of entries, if available. +So if 'u-boot' is called for, we use 'u-boot-expanded' instead. This +is needed if update_fdt is True (although tests may disable it) Returns: OrderedDict of images: @@ -438,7 +443,7 @@ def PrepareImagesAndDtbs(dtb_fname, select_images, update_fdt): raise ValueError("Device tree '%s' does not have a 'binman' " "node" % dtb_fname) -images = _ReadImageDesc(node) +images = _ReadImageDesc(node, use_expanded) if select_images: skip = [] @@ -611,6 +616,13 @@ def Binman(args): elf.debug = args.debug cbfs_util.VERBOSE = args.verbosity > 2 state.use_fake_dtb = args.fake_dtb + +# Normally we replace the 'u-boot' etype with 'u-boot-expanded', etc. +# When running tests this can be disabled using this flag. When not +# updating the FDT in image, it is not needed by binman, but we use it +# for consistency, so that the images look the same to U-Boot at +# runtime. +use_expanded = not args.no_expanded try: tools.SetInputDirs(args.indir) tools.PrepareOutputDir(args.outdir, args.preserve) @@ -618,7 +630,7 @@ def Binman(args): state.SetEntryArgs(args.entry_arg) images = PrepareImagesAndDtbs(dtb_fname, args.image, - args.update_fdt) + args.update_fdt, use_expanded) missing = False for image in images.values(): missing |= ProcessImage(image, args.update_fdt, args.map, diff --git a/tools/binman/etype/section.py b/tools/binman/etype/section.py index 2103919b0c3..2f862bddf0f 100644 --- a/tools/binman/etype/section.py +++ b/tools/binman/etype/section.py @@ -84,7 +84,8 @@ class Entry_section(Entry): for node in self._node.subnodes: if node.name.startswith('hash') or node.name.startswith('signature'): continue -entry = Entry.Create(self, node) +entry = Entry.Create(self, node, + expanded=self.GetImage().use_expanded) entry.ReadNode() entry.SetPrefix(self._name_prefix) self._entries[node.name] = entry diff --git a/tools/binman/image.py b/tools/binman/image.py index e9494352413..10778f47fe9 100644 --- a/tools/binman/image.py +++ b/tools/binman/image.py @@ -47,9 +47,23 @@ class Image(section.Entry_section): exception). This should be used if the Image is being
[PATCH 12/20] Makefile: Pass new entry args to binman
To support the use of 'expanded' entries, binman needs to be told whether SPL and TPL have a devicetree and whether they need BSS padding. Add these to the Makefile. Signed-off-by: Simon Glass --- Makefile | 8 1 file changed, 8 insertions(+) diff --git a/Makefile b/Makefile index 655de412e9a..07e43a6b09f 100644 --- a/Makefile +++ b/Makefile @@ -1332,6 +1332,11 @@ u-boot.ldr: u-boot # Use 'make BINMAN_DEBUG=1' to enable debugging # Use 'make BINMAN_VERBOSE=3' to set vebosity level default_dt := $(if $(DEVICE_TREE),$(DEVICE_TREE),$(CONFIG_DEFAULT_DEVICE_TREE)) + +# Tell binman whether we have a devicetree for SPL and TPL +have_spl_dt := $(if $(CONFIG_SPL_OF_PLATDATA),,$(CONFIG_SPL_OF_CONTROL)) +have_tpl_dt := $(if $(CONFIG_TPL_OF_PLATDATA),,$(CONFIG_TPL_OF_CONTROL)) + quiet_cmd_binman = BINMAN $@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \ --toolpath $(objtree)/tools \ @@ -1342,6 +1347,9 @@ cmd_binman = $(srctree)/tools/binman/binman $(if $(BINMAN_DEBUG),-D) \ -a atf-bl31-path=${BL31} \ -a default-dt=$(default_dt) \ -a scp-path=$(SCP) \ + -a spl-bss-pad=$(if $(CONFIG_SPL_SEPARATE_BSS),,1) \ + -a tpl-bss-pad=$(if $(CONFIG_TPL_SEPARATE_BSS),,1) \ + -a spl-dtb=$(have_spl_dt) -a tpl-dtb=$(have_tpl_dt) \ $(BINMAN_$(@F)) OBJCOPYFLAGS_u-boot.ldr.hex := -I binary -O ihex -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH 14/20] x86: dts: Drop unused CONFIG_SPL
This cannot be used since the previous #elif has already dealt with SPL. Drop it. Signed-off-by: Simon Glass --- arch/x86/dts/u-boot.dtsi | 6 +- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/arch/x86/dts/u-boot.dtsi b/arch/x86/dts/u-boot.dtsi index 50134b2fe00..ca84d18ad90 100644 --- a/arch/x86/dts/u-boot.dtsi +++ b/arch/x86/dts/u-boot.dtsi @@ -55,11 +55,7 @@ offset = ; }; #else -# ifdef CONFIG_SPL - u-boot { - offset = ; - }; -# elif defined(CONFIG_HAVE_MICROCODE) +# ifdef CONFIG_HAVE_MICROCODE /* If there is no SPL then we need to put microcode in U-Boot */ u-boot-with-ucode-ptr { offset = ; -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH 13/20] x86: Make use of binman expanded entries
We don't need to spell out the separate pieces of U-Boot phase binaries anymore. Revert to using the simple entry and let binman do the expansion itself as needed. Signed-off-by: Simon Glass --- arch/x86/dts/u-boot.dtsi | 11 +-- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/arch/x86/dts/u-boot.dtsi b/arch/x86/dts/u-boot.dtsi index bf92f45f2d3..50134b2fe00 100644 --- a/arch/x86/dts/u-boot.dtsi +++ b/arch/x86/dts/u-boot.dtsi @@ -38,20 +38,11 @@ }; #endif spl { - type = "section"; + type = "u-boot-spl"; offset = ; - u-boot-spl { - }; - u-boot-spl-dtb { - }; }; u-boot { - type = "section"; offset = ; - u-boot-nodtb { - }; - u-boot-dtb { - }; }; #elif defined(CONFIG_SPL) u-boot-spl-with-ucode-ptr { -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH 15/20] doc: Move UEFI down a bit
This features a little too prominently in the contents at present. It seems more important to talk about driver model and the API (which includes some UEFI notes). Reposition it below those. Signed-off-by: Simon Glass --- doc/index.rst | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/doc/index.rst b/doc/index.rst index 4c44955d67f..cdcc0a9e60a 100644 --- a/doc/index.rst +++ b/doc/index.rst @@ -38,18 +38,6 @@ want to contribute to U-Boot. develop/index -Unified Extensible Firmware (UEFI) --- - -U-Boot provides an implementation of the UEFI API allowing to run UEFI -compliant software like Linux, GRUB, and iPXE. Furthermore U-Boot itself -can be run an UEFI payload. - -.. toctree:: - :maxdepth: 2 - - uefi/index - Driver-Model documentation -- @@ -76,6 +64,18 @@ needed). api/index +Unified Extensible Firmware (UEFI) +-- + +U-Boot provides an implementation of the UEFI API allowing to run UEFI +compliant software like Linux, GRUB, and iPXE. Furthermore U-Boot itself +can be run an UEFI payload. + +.. toctree:: + :maxdepth: 2 + + uefi/index + Architecture-specific doc - -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH 11/20] binman: Automatically expand phase binaries into sections
When creating an entry, check for an expanded version of that entry, then use it instead. This allows, for example use of: u-boot { }; instead of having to write out in full: u-boot { type = "section"; u-boot-nodtb { }; u-boot-dtb { }; }; Add an implementaion of this and associated documentation. Signed-off-by: Simon Glass --- tools/binman/README | 77 ++ tools/binman/README.entries | 96 +++- tools/binman/etype/blob_phase.py | 51 +++ tools/binman/etype/u_boot.py | 8 +- tools/binman/etype/u_boot_expanded.py | 24 +++ tools/binman/etype/u_boot_nodtb.py| 4 +- tools/binman/etype/u_boot_spl.py | 3 + tools/binman/etype/u_boot_spl_expanded.py | 45 ++ tools/binman/etype/u_boot_spl_nodtb.py| 5 +- tools/binman/etype/u_boot_tpl.py | 3 + tools/binman/etype/u_boot_tpl_expanded.py | 45 ++ tools/binman/etype/u_boot_tpl_nodtb.py| 5 +- tools/binman/ftest.py | 174 ++ tools/binman/state.py | 19 ++- tools/binman/test/194_fdt_incl.dts| 17 +++ tools/binman/test/195_fdt_incl_tpl.dts| 13 ++ 16 files changed, 571 insertions(+), 18 deletions(-) create mode 100644 tools/binman/etype/blob_phase.py create mode 100644 tools/binman/etype/u_boot_expanded.py create mode 100644 tools/binman/etype/u_boot_spl_expanded.py create mode 100644 tools/binman/etype/u_boot_tpl_expanded.py create mode 100644 tools/binman/test/194_fdt_incl.dts create mode 100644 tools/binman/test/195_fdt_incl_tpl.dts diff --git a/tools/binman/README b/tools/binman/README index 45f0a0c2cd3..1de703cc653 100644 --- a/tools/binman/README +++ b/tools/binman/README @@ -840,6 +840,83 @@ of the image) can be used to point to the FDT map. See fdtmap and image-header entries for more information. +Expanded entries + + +Binman automatically replaces 'u-boot' with an expanded version of that, i.e. +'u-boot-expanded'. This means that when you write: + +u-boot { +}; + +you actually get: + +u-boot { +type = "u-boot-expanded'; +}; + +which in turn expands to: + +u-boot { +type = "section"; + +u-boot-nodtb { +}; + +u-boot-dtb { +}; +}; + +U-Boot's various phase binaries actually comprise two or three pieces. +For example, u-boot.bin has the executable followed by a devicetree. + +With binman we want to be able to update that devicetree with full image +information so that it is accessible to the executable. This is tricky +if it is not clear where the devicetree starts. + +The above feature ensures that the devicetree is clearly separated from the +U-Boot executable and can be updated separately by binman as needed. It can be +disabled with the --no-expanded flag if required. + +The same applies for u-boot-spl and u-boot-spl. In those cases, the expansion +includes the BSS padding, so for example: + +spl { +type = "u-boot-spl" +}; + +you actually get: + +spl { +type = "u-boot-expanded'; +}; + +which in turn expands to: + +spl { +type = "section"; + +u-boot-spl-nodtb { +}; + +u-boot-spl-bss-pad { +}; + +u-boot-spl-dtb { +}; +}; + + +Of course we should not expand SPL if it has no devicetree. Also if the BSS +padding is not needed (because BSS is in RAM as with CONFIG_SPL_SEPARATE_BSS), +the 'u-boot-spl-bss-pad' subnode should not be created. The use of the expaned +entry type is controlled by the UseExpanded() method. In the SPL case it checks +the 'spl-dtb' entry arg, which is 'y' or '1' if SPL has a devicetree. + +For the BSS case, a 'spl-bss-pad' entry arg controls whether it is present. All +entry args are provided by the U-Boot Makefile. + + Compression --- diff --git a/tools/binman/README.entries b/tools/binman/README.entries index cd15073e6df..b5032381c7f 100644 --- a/tools/binman/README.entries +++ b/tools/binman/README.entries @@ -87,6 +87,15 @@ See cros_ec_rw for an example of this. +Entry: blob-phase: Section that holds a phase binary + + +This is a base class that should not normally be used directly. It is used +when converting a 'u-boot' entry automatically into a 'u-boot-expanded' +entry; similarly for SPL. + + + Entry: cbfs: Entry containing a Coreboot Filesystem (CBFS) -- @@ -840,8 +849,7 @@ Properties / Entry arguments: This is the U-Boot binary, containing relocation information to allow it to relocate itself at runtime. The binary typically includes a device tree -blob at the end of it. Use u-boot-nodtb if you want to package the device -tree separately. +blob at the end of it. U-Boot can access binman symbols at runtime. See: @@ -849,6 +857,9 @@ U-Boot can a
[PATCH 17/20] binman: Rearrange documentation into headings
Collect the material into different top-level headings to make it easier to read. Signed-off-by: Simon Glass --- tools/binman/README.rst | 523 1 file changed, 266 insertions(+), 257 deletions(-) diff --git a/tools/binman/README.rst b/tools/binman/README.rst index 8d06aa91287..9348a3e8e1d 100644 --- a/tools/binman/README.rst +++ b/tools/binman/README.rst @@ -2,7 +2,7 @@ # Copyright (c) 2016 Google, Inc Introduction - + Firmware often consists of several components which must be packaged together. For example, we may have SPL, U-Boot, a device tree and an environment area @@ -137,6 +137,9 @@ the boundaries between building input files (mkimage) and packaging then into a final image (binman). +Using binman + + Example use of binman in U-Boot --- @@ -230,8 +233,111 @@ You can use other, more specific CONFIG options - see 'Automatic .dtsi inclusion' below. +Access to binman entry offsets at run time (symbols) + + +Binman assembles images and determines where each entry is placed in the image. +This information may be useful to U-Boot at run time. For example, in SPL it +is useful to be able to find the location of U-Boot so that it can be executed +when SPL is finished. + +Binman allows you to declare symbols in the SPL image which are filled in +with their correct values during the build. For example:: + +binman_sym_declare(ulong, u_boot_any, image_pos); + +declares a ulong value which will be assigned to the image-pos of any U-Boot +image (u-boot.bin, u-boot.img, u-boot-nodtb.bin) that is present in the image. +You can access this value with something like:: + +ulong u_boot_offset = binman_sym(ulong, u_boot_any, image_pos); + +Thus u_boot_offset will be set to the image-pos of U-Boot in memory, assuming +that the whole image has been loaded, or is available in flash. You can then +jump to that address to start U-Boot. + +At present this feature is only supported in SPL and TPL. In principle it is +possible to fill in such symbols in U-Boot proper, as well, but a future C +library is planned for this instead, to read from the device tree. + +As well as image-pos, it is possible to read the size of an entry and its +offset (which is the start position of the entry within its parent). + +A small technical note: Binman automatically adds the base address of the image +(i.e. __image_copy_start) to the value of the image-pos symbol, so that when the +image is loaded to its linked address, the value will be correct and actually +point into the image. + +For example, say SPL is at the start of the image and linked to start at address +80108000. If U-Boot's image-pos is 0x8000 then binman will write an image-pos +for U-Boot of 8011 into the SPL binary, since it assumes the image is loaded +to 80108000, with SPL at 80108000 and U-Boot at 8011. + +For x86 devices (with the end-at-4gb property) this base address is not added +since it is assumed that images are XIP and the offsets already include the +address. + + +Access to binman entry offsets at run time (fdt) + + +Binman can update the U-Boot FDT to include the final position and size of +each entry in the images it processes. The option to enable this is -u and it +causes binman to make sure that the 'offset', 'image-pos' and 'size' properties +are set correctly for every entry. Since it is not necessary to specify these in +the image definition, binman calculates the final values and writes these to +the device tree. These can be used by U-Boot at run-time to find the location +of each entry. + +Alternatively, an FDT map entry can be used to add a special FDT containing +just the information about the image. This is preceded by a magic string so can +be located anywhere in the image. An image header (typically at the start or end +of the image) can be used to point to the FDT map. See fdtmap and image-header +entries for more information. + + +Map files +- + +The -m option causes binman to output a .map file for each image that it +generates. This shows the offset and size of each entry. For example:: + + Offset Size Name + 0028 main-section + 0010 section@0 + 0004 u-boot + 0010 0010 section@1 + 0004 u-boot + +This shows a hierarchical image with two sections, each with a single entry. The +offsets of the sections are absolute hex byte offsets within the image. The +offsets of the entries are relative to their respective sections. The size of +each entry is also shown, in bytes (hex). The indentation shows the entries +nested inside their sections. + + +Passing command-line arguments to entries +- + +Sometimes it is useful to pass binman the value of an entry proper
[PATCH 20/20] binman: Update various pieces of the documentation
A few sections are a little out of date now. Update them. Signed-off-by: Simon Glass --- tools/binman/README.rst | 72 - 1 file changed, 36 insertions(+), 36 deletions(-) diff --git a/tools/binman/README.rst b/tools/binman/README.rst index 5df5de4d8e3..680a1c38a8d 100644 --- a/tools/binman/README.rst +++ b/tools/binman/README.rst @@ -9,39 +9,43 @@ For example, we may have SPL, U-Boot, a device tree and an environment area grouped together and placed in MMC flash. When the system starts, it must be able to find these pieces. -So far U-Boot has not provided a way to handle creating such images in a -general way. Each SoC does what it needs to build an image, often packing or -concatenating images in the U-Boot build system. - -Binman aims to provide a mechanism for building images, from simple -SPL + U-Boot combinations, to more complex arrangements with many parts. +Building firmware should be separate from packaging it. Many of the complexities +of modern firmware build systems come from trying to do both at once. With +binman, you build all the pieces that are needed, using whatever assortment of +projects and build systems are needed, then use binman to stitch everything +together. What it does Binman reads your board's device tree and finds a node which describes the -required image layout. It uses this to work out what to place where. The -output file normally contains the device tree, so it is in principle possible -to read an image and extract its constituent parts. +required image layout. It uses this to work out what to place where. + +Binman provides a mechanism for building images, from simple SPL + U-Boot +combinations, to more complex arrangements with many parts. It also allows +users to inspect images, extract and replace binaries within them, repacking if +needed. Features -So far binman is pretty simple. It supports binary blobs, such as 'u-boot', -'spl' and 'fdt'. It supports empty entries (such as setting to 0xff). It can -place entries at a fixed location in the image, or fit them together with -suitable padding and alignment. It provides a way to process binaries before -they are included, by adding a Python plug-in. The device tree is available -to U-Boot at run-time so that the images can be interpreted. +Apart from basic padding, alignment and positioning features, Binman supports +hierarchical images, compression, hashing and dealing with the binary blobs +which are a sad trend in open-source firmware at present. -Binman can update the device tree with the final location of everything when it -is done. Entry positions can be provided to U-Boot SPL as run-time symbols, -avoiding device-tree code overhead. +Executable binaries can access the location of other binaries in an image by +using special linker symbols (zero-overhead but somewhat limited) or by reading +the devicetree description of the image. -Binman can also support incorporating filesystems in the image if required. -For example x86 platforms may use CBFS in some cases. +Binman is designed primarily for use with U-Boot and associated binaries such +as ARM Trusted Firmware, but it is suitable for use with other projects, such +as Zephyr. Binman also provides facilities useful in Chromium OS, such as CBFS, +vblocks and and the like. + +Binman provides a way to process binaries before they are included, by adding a +Python plug-in. Binman is intended for use with U-Boot but is designed to be general enough to be useful in other image-packaging situations. @@ -50,11 +54,11 @@ to be useful in other image-packaging situations. Motivation -- -Packaging of firmware is quite a different task from building the various -parts. In many cases the various binaries which go into the image come from -separate build systems. For example, ARM Trusted Firmware is used on ARMv8 -devices but is not built in the U-Boot tree. If a Linux kernel is included -in the firmware image, it is built elsewhere. +As mentioned above, packaging of firmware is quite a different task from +building the various parts. In many cases the various binaries which go into +the image come from separate build systems. For example, ARM Trusted Firmware +is used on ARMv8 devices but is not built in the U-Boot tree. If a Linux kernel +is included in the firmware image, it is built elsewhere. It is of course possible to add more and more build rules to the U-Boot build system to cover these cases. It can shell out to other Makefiles and @@ -215,17 +219,12 @@ Binman has a few other options which you can see by running 'binman -h'. Enabling binman for a board --- -At present binman is invoked from a rule in the main Makefile. Typically you -will have a rule like:: - -ifneq ($(CONFIG_ARCH_),) -u-boot-.bin: checkbinman FORCE -$(call if_changed,binman) -endif +At present binman is invoked from a rule in the main Ma
[PATCH 18/20] binman: Incorporate entry documentation
Update this to avoid sphinx warnings and incorporate it into the new documentaiton tree. Signed-off-by: Simon Glass --- tools/binman/README.rst | 11 ++- tools/binman/cmdline.py | 2 +- tools/binman/{README.entries => entries.rst} | 79 +++- tools/binman/etype/cbfs.py | 10 +-- tools/binman/etype/fdtmap.py | 30 tools/binman/etype/fit.py| 19 +++-- tools/binman/etype/intel_ifwi.py | 8 +- tools/binman/etype/mkimage.py| 2 +- tools/binman/etype/section.py| 6 +- tools/binman/etype/text.py | 6 +- tools/binman/setup.py| 2 +- 11 files changed, 95 insertions(+), 80 deletions(-) rename tools/binman/{README.entries => entries.rst} (97%) diff --git a/tools/binman/README.rst b/tools/binman/README.rst index 9348a3e8e1d..5df5de4d8e3 100644 --- a/tools/binman/README.rst +++ b/tools/binman/README.rst @@ -786,12 +786,17 @@ the 'warning' line in scripts/Makefile.lib to see what it has found:: Entry Documentation +=== For details on the various entry types supported by binman and how to use them, -see README.entries. This is generated from the source code using: +see entries.rst which is generated from the source code using: + +binman entry-docs >tools/binman/entries.rst + +.. toctree:: + :maxdepth: 2 -binman entry-docs >tools/binman/README.entries + entries.rst Managing images diff --git a/tools/binman/cmdline.py b/tools/binman/cmdline.py index 0c0f48951f6..95f9ba27fbd 100644 --- a/tools/binman/cmdline.py +++ b/tools/binman/cmdline.py @@ -69,7 +69,7 @@ controlled by a description in the board device tree.''' default=False, help='Update the binman node with offset/size info') entry_parser = subparsers.add_parser('entry-docs', -help='Write out entry documentation (see README.entries)') +help='Write out entry documentation (see entries.rst)') list_parser = subparsers.add_parser('ls', help='List files in an image') list_parser.add_argument('-i', '--image', type=str, required=True, diff --git a/tools/binman/README.entries b/tools/binman/entries.rst similarity index 97% rename from tools/binman/README.entries rename to tools/binman/entries.rst index b5032381c7f..cb89570d5b2 100644 --- a/tools/binman/README.entries +++ b/tools/binman/entries.rst @@ -106,7 +106,7 @@ is not used, it supports compression and storing ELF files. CBFS is used by coreboot as its way of orgnanising SPI-flash contents. -The contents of the CBFS are defined by subnodes of the cbfs entry, e.g.: +The contents of the CBFS are defined by subnodes of the cbfs entry, e.g.:: cbfs { size = <0x10>; @@ -122,7 +122,7 @@ This creates a CBFS 1MB in size two files in it: u-boot.bin and u-boot.dtb. Note that the size is required since binman does not support calculating it. The contents of each entry is just what binman would normally provide if it were not a CBFS node. A blob type can be used to import arbitrary files as -with the second subnode below: +with the second subnode below:: cbfs { size = <0x10>; @@ -168,7 +168,7 @@ cbfs-type: This is an ELF file that has been loaded (i.e. mapped to memory), so appears in the CBFS as a flat binary. The input file must be an ELF image, for example this puts "u-boot" (the ELF image) into a 'stage' -entry: +entry:: cbfs { size = <0x10>; @@ -178,7 +178,7 @@ cbfs-type: }; }; -You can use your own ELF file with something like: +You can use your own ELF file with something like:: cbfs { size = <0x10>; @@ -211,7 +211,7 @@ not support other file types (e.g. payload), adding multiple files (like the particular offset in the CBFS and a few other things. Of course binman can create images containing multiple CBFSs, simply by -defining these in the binman config: +defining these in the binman config:: binman { @@ -279,24 +279,24 @@ sizes are included. Note that the -u option must be provided to ensure that binman updates the FDT with the position of each entry. -Example output for a simple image with U-Boot and an FDT map: +Example output for a simple image with U-Boot and an FDT map:: -/ { -image-name = "binman"; -size = <0x0112>; -image-pos = <0x>; -offset = <0x>; -u-boot { -size = <0x0004>; +/ { +image-name = "binman"; +size = <0x0112>; image-pos = <0x>; offset = <0x>; +u-boot { +size = <0x0004>; +image-pos = <0x>; +offset = <0x>; +}; +fdtmap { +size = <0x010e>; +image-
[PATCH 16/20] binman: doc: Add documentation to htmldocs
Add a link to binman's documentation and adjust the files so that it is accessible. Use the name README.rst so it is easy to discover when binman is installed without U-Boot. Signed-off-by: Simon Glass --- doc/index.rst | 1 + doc/package/binman | 1 + doc/package/fit.rst | 8 + doc/package/index.rst | 20 ++ tools/binman/{README => README.rst} | 480 ++-- tools/binman/control.py | 2 +- tools/binman/ftest.py | 4 +- tools/binman/index.rst | 9 + tools/binman/setup.py | 2 +- 9 files changed, 284 insertions(+), 243 deletions(-) create mode 12 doc/package/binman create mode 100644 doc/package/fit.rst create mode 100644 doc/package/index.rst rename tools/binman/{README => README.rst} (73%) create mode 100644 tools/binman/index.rst diff --git a/doc/index.rst b/doc/index.rst index cdcc0a9e60a..26be4be5b3c 100644 --- a/doc/index.rst +++ b/doc/index.rst @@ -25,6 +25,7 @@ trying to get it to work optimally on a given system. :maxdepth: 2 build/index + package/index usage/index Developer-oriented documentation diff --git a/doc/package/binman b/doc/package/binman new file mode 12 index 000..94916117605 --- /dev/null +++ b/doc/package/binman @@ -0,0 +1 @@ +../../tools/binman \ No newline at end of file diff --git a/doc/package/fit.rst b/doc/package/fit.rst new file mode 100644 index 000..70374340577 --- /dev/null +++ b/doc/package/fit.rst @@ -0,0 +1,8 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +Flat Image Tree (FIT) += + +U-Boot uses Flat Image Tree (FIT) as a standard file format for packaging +images that it it reads and boots. Documentation about FIT is available at +doc/uImage.FIT diff --git a/doc/package/index.rst b/doc/package/index.rst new file mode 100644 index 000..ca1f7fe2f97 --- /dev/null +++ b/doc/package/index.rst @@ -0,0 +1,20 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +Package U-Boot +== + +U-Boot uses Flat Image Tree (FIT) as a standard file format for packaging +images that it it reads and boots. Documentation about FIT is available at +doc/uImage.FIT + +U-Boot also provides binman for cases not covered by FIT. Examples include +initial execution (since FIT itself does not have an executable header) and +dealing with device boundaries, such as the read-only/read-write separation in +SPI flash. + + +.. toctree:: + :maxdepth: 2 + + fit + binman/index diff --git a/tools/binman/README b/tools/binman/README.rst similarity index 73% rename from tools/binman/README rename to tools/binman/README.rst index 1de703cc653..8d06aa91287 100644 --- a/tools/binman/README +++ b/tools/binman/README.rst @@ -67,18 +67,19 @@ standard format, we can support making valid images for any board without manual effort, lots of READMEs, etc. Benefits: -- Each binary can have its own build system and tool chain without creating -any dependencies between them -- Avoids the need for a single-shot build: individual parts can be updated -and brought in as needed -- Provides for a standard image description available in the build and at -run-time -- SoC-specific image-signing tools can be accommodated -- Avoids cluttering the U-Boot build system with image-building code -- The image description is automatically available at run-time in U-Boot, -SPL. It can be made available to other software also -- The image description is easily readable (it's a text file in device-tree -format) and permits flexible packing of binaries + + - Each binary can have its own build system and tool chain without creating +any dependencies between them + - Avoids the need for a single-shot build: individual parts can be updated +and brought in as needed + - Provides for a standard image description available in the build and at +run-time + - SoC-specific image-signing tools can be accommodated + - Avoids cluttering the U-Boot build system with image-building code + - The image description is automatically available at run-time in U-Boot, +SPL. It can be made available to other software also + - The image description is easily readable (it's a text file in device-tree +format) and permits flexible packing of binaries Terminology @@ -144,14 +145,14 @@ build system. Consider sunxi. It has the following steps: -1. It uses a custom mksunxiboot tool to build an SPL image called -sunxi-spl.bin. This should probably move into mkimage. + #. It uses a custom mksunxiboot tool to build an SPL image called + sunxi-spl.bin. This should probably move into mkimage. -2. It uses mkimage to package U-Boot into a legacy image file (so that it can -hold the load and execution address) called u-boot.img. + #. It uses mkimage to package U-Boot into a legacy image file (so that it can + hold the load and execution address) called u-boot.img. -3. It builds a final output
[PATCH 19/20] binman: Drop repetitive heading for each entry
Many entries start 'Entry containing a'. This looks fine in the source code but is annoying when viewed in the htmldocs table of contents. Drop these unnecessary words. Signed-off-by: Simon Glass --- tools/binman/entries.rst| 76 ++--- tools/binman/etype/atf_bl31.py | 2 +- tools/binman/etype/blob.py | 2 +- tools/binman/etype/blob_ext.py | 2 +- tools/binman/etype/cbfs.py | 2 +- tools/binman/etype/files.py | 2 +- tools/binman/etype/fit.py | 2 +- tools/binman/etype/intel_cmc.py | 2 +- tools/binman/etype/intel_fsp.py | 2 +- tools/binman/etype/intel_fsp_m.py | 2 +- tools/binman/etype/intel_fsp_s.py | 2 +- tools/binman/etype/intel_fsp_t.py | 2 +- tools/binman/etype/intel_ifwi.py| 2 +- tools/binman/etype/intel_me.py | 2 +- tools/binman/etype/intel_mrc.py | 2 +- tools/binman/etype/intel_refcode.py | 2 +- tools/binman/etype/intel_vbt.py | 2 +- tools/binman/etype/intel_vga.py | 2 +- tools/binman/etype/mkimage.py | 2 +- tools/binman/etype/scp.py | 2 +- 20 files changed, 57 insertions(+), 57 deletions(-) diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst index cb89570d5b2..528087db6cd 100644 --- a/tools/binman/entries.rst +++ b/tools/binman/entries.rst @@ -11,8 +11,8 @@ features to produce new behaviours. -Entry: atf-bl31: Entry containing an ARM Trusted Firmware (ATF) BL31 blob -- +Entry: atf-bl31: ARM Trusted Firmware (ATF) BL31 blob +- Properties / Entry arguments: - atf-bl31-path: Filename of file to read into entry. This is typically @@ -25,8 +25,8 @@ about ATF. -Entry: blob: Entry containing an arbitrary binary blob --- +Entry: blob: Arbitrary binary blob +-- Note: This should not be used by itself. It is normally used as a parent class by other entry types. @@ -56,8 +56,8 @@ obtained from the list of available device-tree files, managed by the -Entry: blob-ext: Entry containing an externally built binary blob -- +Entry: blob-ext: Externally built binary blob +- Note: This should not be used by itself. It is normally used as a parent class by other entry types. @@ -96,8 +96,8 @@ entry; similarly for SPL. -Entry: cbfs: Entry containing a Coreboot Filesystem (CBFS) --- +Entry: cbfs: Coreboot Filesystem (CBFS) +--- A CBFS provides a way to group files into a group. It has a simple directory structure and allows the position of individual files to be set, since it is @@ -303,8 +303,8 @@ added as necessary. See the binman README. -Entry: files: Entry containing a set of files -- +Entry: files: A set of files arranged in a section +-- Properties / Entry arguments: - pattern: Filename pattern to match the files to include @@ -335,8 +335,8 @@ byte value of a region. -Entry: fit: Entry containing a FIT --- +Entry: fit: FIT +--- This calls mkimage to create a FIT (U-Boot Flat Image Tree) based on the input provided. @@ -491,8 +491,8 @@ first/last in the entry list. -Entry: intel-cmc: Entry containing an Intel Chipset Micro Code (CMC) file -- +Entry: intel-cmc: Intel Chipset Micro Code (CMC) file +- Properties / Entry arguments: - filename: Filename of file to read into entry @@ -544,8 +544,8 @@ This entry contains a pointer to the FIT. It is required to be at address -Entry: intel-fsp: Entry containing an Intel Firmware Support Package (FSP) file +Entry: intel-fsp: Intel Firmware Support Package (FSP) file +--- Properties / Entry arguments: - filename: Filename of file to read into entry @@ -561,8 +561,8 @@ See README.x86 for information about x86 binary blobs. -Entry: intel-fsp-m: Entry containing Intel Firmware Support Package (FSP) memory init -- +Entry: intel-fsp-m: Intel Firmware Support Package (FSP) memory init + Properties / Entry arguments: - filename: Filename of file to read into entry @@ -578,8 +578,8 @@ See README.x86 for information about x86 binary blobs. -Entry: in
Re: [PATCH 15/20] doc: Move UEFI down a bit
On 3/7/21 8:31 PM, Simon Glass wrote: This features a little too prominently in the contents at present. It seems more important to talk about driver model and the API (which includes some UEFI notes). The driver model is only relevant for developers. So it should be moved to "Develop U-Boot". The UEFI part should be split up into a development related part under /doc/develop/ and a usage related part under /doc/usage. But as a first step simply move it to /doc/develop together with the driver model, please. Best regards Heinrich Reposition it below those. Signed-off-by: Simon Glass --- doc/index.rst | 24 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/doc/index.rst b/doc/index.rst index 4c44955d67f..cdcc0a9e60a 100644 --- a/doc/index.rst +++ b/doc/index.rst @@ -38,18 +38,6 @@ want to contribute to U-Boot. develop/index -Unified Extensible Firmware (UEFI) --- - -U-Boot provides an implementation of the UEFI API allowing to run UEFI -compliant software like Linux, GRUB, and iPXE. Furthermore U-Boot itself -can be run an UEFI payload. - -.. toctree:: - :maxdepth: 2 - - uefi/index - Driver-Model documentation -- @@ -76,6 +64,18 @@ needed). api/index +Unified Extensible Firmware (UEFI) +-- + +U-Boot provides an implementation of the UEFI API allowing to run UEFI +compliant software like Linux, GRUB, and iPXE. Furthermore U-Boot itself +can be run an UEFI payload. + +.. toctree:: + :maxdepth: 2 + + uefi/index + Architecture-specific doc -
Re: [PATCH 16/20] binman: doc: Add documentation to htmldocs
On 3/7/21 8:31 PM, Simon Glass wrote: Add a link to binman's documentation and adjust the files so that it is accessible. Use the name README.rst so it is easy to discover when binman is installed without U-Boot. Signed-off-by: Simon Glass --- doc/index.rst | 1 + doc/package/binman | 1 + doc/package/fit.rst | 8 + This information is only of interest for developers, not for end-users. Please, place that information in /doc/develop/. doc/package/index.rst | 20 ++ tools/binman/{README => README.rst} | 480 ++-- tools/binman/ is the wrong path for documentation. Please, put the file in /doc/develop/binman.rst or /doc/develop/tools/binman.rst so that it becomes available online. Best regards Heinrich tools/binman/control.py | 2 +- tools/binman/ftest.py | 4 +- tools/binman/index.rst | 9 + tools/binman/setup.py | 2 +- 9 files changed, 284 insertions(+), 243 deletions(-) create mode 12 doc/package/binman create mode 100644 doc/package/fit.rst create mode 100644 doc/package/index.rst rename tools/binman/{README => README.rst} (73%) create mode 100644 tools/binman/index.rst diff --git a/doc/index.rst b/doc/index.rst index cdcc0a9e60a..26be4be5b3c 100644 --- a/doc/index.rst +++ b/doc/index.rst @@ -25,6 +25,7 @@ trying to get it to work optimally on a given system. :maxdepth: 2 build/index + package/index usage/index Developer-oriented documentation diff --git a/doc/package/binman b/doc/package/binman new file mode 12 index 000..94916117605 --- /dev/null +++ b/doc/package/binman @@ -0,0 +1 @@ +../../tools/binman \ No newline at end of file diff --git a/doc/package/fit.rst b/doc/package/fit.rst new file mode 100644 index 000..70374340577 --- /dev/null +++ b/doc/package/fit.rst @@ -0,0 +1,8 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +Flat Image Tree (FIT) += + +U-Boot uses Flat Image Tree (FIT) as a standard file format for packaging +images that it it reads and boots. Documentation about FIT is available at +doc/uImage.FIT diff --git a/doc/package/index.rst b/doc/package/index.rst new file mode 100644 index 000..ca1f7fe2f97 --- /dev/null +++ b/doc/package/index.rst @@ -0,0 +1,20 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +Package U-Boot +== + +U-Boot uses Flat Image Tree (FIT) as a standard file format for packaging +images that it it reads and boots. Documentation about FIT is available at +doc/uImage.FIT + +U-Boot also provides binman for cases not covered by FIT. Examples include +initial execution (since FIT itself does not have an executable header) and +dealing with device boundaries, such as the read-only/read-write separation in +SPI flash. + + +.. toctree:: + :maxdepth: 2 + + fit + binman/index diff --git a/tools/binman/README b/tools/binman/README.rst similarity index 73% rename from tools/binman/README rename to tools/binman/README.rst index 1de703cc653..8d06aa91287 100644 --- a/tools/binman/README +++ b/tools/binman/README.rst @@ -67,18 +67,19 @@ standard format, we can support making valid images for any board without manual effort, lots of READMEs, etc. Benefits: -- Each binary can have its own build system and tool chain without creating -any dependencies between them -- Avoids the need for a single-shot build: individual parts can be updated -and brought in as needed -- Provides for a standard image description available in the build and at -run-time -- SoC-specific image-signing tools can be accommodated -- Avoids cluttering the U-Boot build system with image-building code -- The image description is automatically available at run-time in U-Boot, -SPL. It can be made available to other software also -- The image description is easily readable (it's a text file in device-tree -format) and permits flexible packing of binaries + + - Each binary can have its own build system and tool chain without creating +any dependencies between them + - Avoids the need for a single-shot build: individual parts can be updated +and brought in as needed + - Provides for a standard image description available in the build and at +run-time + - SoC-specific image-signing tools can be accommodated + - Avoids cluttering the U-Boot build system with image-building code + - The image description is automatically available at run-time in U-Boot, +SPL. It can be made available to other software also + - The image description is easily readable (it's a text file in device-tree +format) and permits flexible packing of binaries Terminology @@ -144,14 +145,14 @@ build system. Consider sunxi. It has the following steps: -1. It uses a custom mksunxiboot tool to build an SPL image called -sunxi-spl.bin. This should probably move into mkimage. + #. It uses a custom mksunxiboot tool to build an SPL image ca
Re: [PATCH] configs: add PineTab defconfig
On 2021-03-07, Nicolas Boulenguez wrote: > From: Arnaud Ferraris > > The PineTab device-tree is already in u-boot, this commit adds the > corresponding > defconfig, based on pinephone_defconfig. > > Signed-off-by: Arnaud Ferraris ... > --- /dev/null > +++ b/configs/pinetab_defconfig ... > +CONFIG_BOOTDELAY=0 Setting bootdelay to 0 it almost impossible to debug issues in a running u-boot. The default of 2 seconds that distro_bootcmd uses tries to strike a balance between not slowing the boot down too much while still being reasonably able to get into a u-boot shell when something goes wrong. Individual users or vendors can set this value as they see fit, but this doesn't seem like a good default for mainline u-boot, at least to me. live well, vagrant signature.asc Description: PGP signature
Re: [PATCH u-boot 38/39] ARM: enable LTO for some boards
On Sun, 7 Mar 2021 10:14:07 -0600 Adam Ford wrote: > On Sat, Mar 6, 2021 at 10:26 PM Marek Behún wrote: > > > > Enable LTO for some boards that were tested by people on U-Boot Mailing > > List. > > > > I have also confirmed this works on the r8a774a1_beacon board as well > and boots without issues. The r8a774b1_beacon and r8a774e1_beacon > should be safe since the only real difference between them is the > device tree reference. pushed into my github branch
Re: [PATCH u-boot 14/39] lib: crc32: put the crc_table variable into efi_runtime_rodata section
On Sun, 7 Mar 2021 14:04:35 +0100 Marek Behun wrote: > Hmm, maybe this should be split into 2 commits. I have divided this into 2 commits and pushed into my github branch
Re: [PATCH v2 2/5] configs: ds414: Enable XHCI_PCI by default
On Sat, Mar 06, 2021 at 09:15:27AM +0100, Stefan Roese wrote: > On 05.03.21 21:03, Phil Sutter wrote: > > With the recent fixes in pci_mvebu and xhci-pci drivers, the two rear > > USB3 ports are finally usable and accessing them no longer hangs the > > system. Moreover, if Linux is booted without a prior call to 'pci enum' > > and 'usb start', the HCD is detected but attached devices are not > > usable: > > > > | xhci_hcd :02:00.0: xHCI Host Controller > > | xhci_hcd :02:00.0: new USB bus registered, assigned bus number 2 > > | xhci_hcd :02:00.0: hcc params 0x040040a5 hci version 0x100 quirks > > 0x00080490 > > | usb usb2: New USB device found, idVendor=1d6b, idProduct=0002, bcdDevice= > > 5.04 > > | usb usb2: New USB device strings: Mfr=3, Product=2, SerialNumber=1 > > | usb usb2: Product: xHCI Host Controller > > | usb usb2: Manufacturer: Linux 5.4.92-1 xhci-hcd > > | usb usb2: SerialNumber: :02:00.0 > > | hub 2-0:1.0: USB hub found > > | ata1: SATA link down (SStatus 0 SControl 300) > > | hub 2-0:1.0: 2 ports detected > > | xhci_hcd :02:00.0: xHCI Host Controller > > | xhci_hcd :02:00.0: new USB bus registered, assigned bus number 3 > > | xhci_hcd :02:00.0: Host supports USB 3.0 SuperSpeed > > | usb usb3: We don't know the algorithms for LPM for this host, disabling > > LPM. > > | usb usb3: New USB device found, idVendor=1d6b, idProduct=0003, bcdDevice= > > 5.04 > > | usb usb3: New USB device strings: Mfr=3, Product=2, SerialNumber=1 > > | usb usb3: Product: xHCI Host Controller > > | usb usb3: Manufacturer: Linux 5.4.92-1 xhci-hcd > > | usb usb3: SerialNumber: :02:00.0 > > | hub 3-0:1.0: USB hub found > > | hub 3-0:1.0: 2 ports detected > > [...] > > | xhci_hcd :02:00.0: Error while assigning device slot ID > > | xhci_hcd :02:00.0: Max number of devices this xHCI host supports is > > 64. > > | usb usb2-port2: couldn't allocate usb_device > > > > To avoid this problem, enumerate PCI (and USB) from PREBOOT. > > > > Signed-off-by: Phil Sutter > > --- > > Changes since v1: > > - Have to enable XHCI_HCD as well to fulfill Kconfig dependency. > > - Explicitly disable XHCI_MARVELL which defaults to enabled. > > - Prefix PREBOOT with 'pci enum'. > > - Update commit message accordingly. > > Hmmm, in general is should not be necessary to configure / setup any of > the devices in the bootloader so that it works correctly in Linux. It's > best practice, that Linux does not rely on any bootloader setup. If a > device, like PCI and/or USB does not work in Linux without this U-Boot > setup, then it should be fixed in Linux instead. > > Especially calling "usb start" adds a quite big delay to the bootup > time. I would really like to not add such changes. Perhaps you could > check with the maintainer(s) of the Linux PCI driver and/or the USB > PCI controller driver about this issue instead? Turns out I should have tried a more recent kernel first, the bug I was working around is quite certainly fixed by Linux commit 216f8e95aacc8 ("PCI: mvebu: Setup BAR0 in order to fix MSI"). I'll send a v3 without the fuss but with the Kconfig fix. Sorry for the noise, Phil
[PATCH v3 2/5] configs: ds414: Enable XHCI_PCI by default
With the recent fixes in pci_mvebu and xhci-pci drivers, the two rear USB3 ports are finally usable and accessing them no longer hangs the system. Signed-off-by: Phil Sutter --- Changes since v2: - Leave PREBOOT alone, with recent kernels XHCI HCD works fine even if not initialized. - Adjust commit message again. Changes since v1: - Have to enable XHCI_HCD as well to fulfill Kconfig dependency. - Explicitly disable XHCI_MARVELL which defaults to enabled. - Prefix PREBOOT with 'pci enum'. - Update commit message accordingly. --- configs/ds414_defconfig | 3 +++ 1 file changed, 3 insertions(+) diff --git a/configs/ds414_defconfig b/configs/ds414_defconfig index 3e6dcec3edde3..412559256e6ca 100644 --- a/configs/ds414_defconfig +++ b/configs/ds414_defconfig @@ -65,5 +65,8 @@ CONFIG_SYS_NS16550=y CONFIG_KIRKWOOD_SPI=y CONFIG_USB=y CONFIG_DM_USB=y +CONFIG_USB_XHCI_HCD=y +# CONFIG_USB_XHCI_MVEBU is not set +CONFIG_USB_XHCI_PCI=y CONFIG_USB_EHCI_HCD=y CONFIG_USB_STORAGE=y -- 2.30.1
[PATCH v3 3/5] board/Synology: Unify legacy kernel support
Move the relevant bits from ds109.{c,h} into common/ and adjust the code to fit both DS109 and DS414. Moreover: * Introduce syno_board_id() which translates CONFIG_MACH_TYPE into the expected board ID tag value. * Properly initialize isusbhost, mac and mtu fields from env variables. * Set the right bootargs/bootcmd to correctly boot legacy kernel out of the (DS414) box. Getting the ramdisk location right is a bit tedious. Cc: Walter Schweizer Signed-off-by: Phil Sutter -- Changes since v2: - Drop PREBOOT entirely, 'usb init' is not needed. Changes since v1: - Avoid deprecated common.h header include. - Remove 'sf probe' call from DS414 PREBOOT, bootcmd contains it now. --- board/Synology/common/Makefile | 5 +++ board/Synology/common/legacy.c | 76 ++ board/Synology/common/legacy.h | 33 +++ board/Synology/ds109/ds109.c | 32 -- board/Synology/ds109/ds109.h | 17 configs/ds414_defconfig| 5 +-- include/configs/ds109.h| 3 +- include/configs/ds414.h| 15 ++- 8 files changed, 132 insertions(+), 54 deletions(-) create mode 100644 board/Synology/common/Makefile create mode 100644 board/Synology/common/legacy.c create mode 100644 board/Synology/common/legacy.h diff --git a/board/Synology/common/Makefile b/board/Synology/common/Makefile new file mode 100644 index 0..62354cc2e82e6 --- /dev/null +++ b/board/Synology/common/Makefile @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright (C) 2021 Phil Sutter + +obj-y += legacy.o diff --git a/board/Synology/common/legacy.c b/board/Synology/common/legacy.c new file mode 100644 index 0..3c89e92ae7382 --- /dev/null +++ b/board/Synology/common/legacy.c @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2021 + * Walter Schweizer + * Phil Sutter + */ + +#include +#include +#include +#include +#include + +#include "legacy.h" + +static unsigned int syno_board_id(void) +{ + switch (CONFIG_MACH_TYPE) { + case 527: + return SYNO_DS109_ID; + case 3036: + return SYNO_AXP_4BAY_2BAY; + default: + return 0; + } +} + +static unsigned int usb_port_modes(void) +{ + unsigned int i, ret = 0; + char var[32], *val; + + for (i = 0; i < USBPORT_MAX; i++) { + snprintf(var, 32, "usb%dMode", i); + val = env_get(var); + + if (!val || strcasecmp(val, "host")) + continue; + + ret |= 1 << i; + } + return ret; +} + +/* Support old kernels */ +void setup_board_tags(struct tag **in_params) +{ + struct tag_mv_uboot *t; + struct tag *params; + int i; + + debug("Synology board tags...\n"); + + params = *in_params; + t = (struct tag_mv_uboot *)¶ms->u; + + t->uboot_version = VER_NUM | syno_board_id(); + t->tclk = CONFIG_SYS_TCLK; + t->sysclk = CONFIG_SYS_TCLK * 2; + t->isusbhost = usb_port_modes(); + + for (i = 0; i < ETHADDR_MAX; i++) { + char addrvar[16], mtuvar[16]; + + sprintf(addrvar, i ? "eth%daddr" : "ethaddr", i); + sprintf(mtuvar, i ? "eth%dmtu" : "ethmtu", i); + + eth_env_get_enetaddr(addrvar, t->macaddr[i]); + t->mtu[i] = env_get_ulong(mtuvar, 10, 0); + } + + params->hdr.tag = ATAG_MV_UBOOT; + params->hdr.size = tag_size(tag_mv_uboot); + params = tag_next(params); + *in_params = params; +} diff --git a/board/Synology/common/legacy.h b/board/Synology/common/legacy.h new file mode 100644 index 0..0a814324d0977 --- /dev/null +++ b/board/Synology/common/legacy.h @@ -0,0 +1,33 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (C) 2021 + * Walter Schweizer + * Phil Sutter + */ + +#ifndef __SYNO_LEGACY_H +#define __SYNO_LEGACY_H + +/* Marvell uboot parameters */ +#define ATAG_MV_UBOOT 0x41000403 +#define VER_NUM 0x03040400 /* 3.4.4 */ + +#define BOARD_ID_BASE 0x0 +#define SYNO_DS109_ID (BOARD_ID_BASE + 0x15) +#define SYNO_AXP_4BAY_2BAY (0xf + 1) + +#define ETHADDR_MAX4 +#define USBPORT_MAX3 + +struct tag_mv_uboot { + u32 uboot_version; + u32 tclk; + u32 sysclk; + u32 isusbhost; + u8 macaddr[ETHADDR_MAX][ETH_ALEN]; + u16 mtu[ETHADDR_MAX]; + u32 fw_image_base; + u32 fw_image_size; +}; + +#endif /* __SYNO_LEGACY_H */ diff --git a/board/Synology/ds109/ds109.c b/board/Synology/ds109/ds109.c index eaac95460c6e1..3914faaf37bb4 100644 --- a/board/Synology/ds109/ds109.c +++ b/board/Synology/ds109/ds109.c @@ -114,38 +114,6 @@ void reset_misc(void) SOFTWARE_REBOOT); } -/* Support old kernels */ -void setup_board_tags(struct tag **in_params) -{ - unsigned int boardId; - struct tag *params; - struct tag_mv_uboot *t; - int i; - - printf("Synology board
Re: [PATCH 16/20] binman: doc: Add documentation to htmldocs
Hi Heinrich, On Sun, 7 Mar 2021 at 15:02, Heinrich Schuchardt wrote: > > On 3/7/21 8:31 PM, Simon Glass wrote: > > Add a link to binman's documentation and adjust the files so that it is > > accessible. Use the name README.rst so it is easy to discover when binman > > is installed without U-Boot. > > > > Signed-off-by: Simon Glass > > --- > > > > doc/index.rst | 1 + > > doc/package/binman | 1 + > > doc/package/fit.rst | 8 + > > This information is only of interest for developers, not for end-users. > Please, place that information in /doc/develop/. What is the difference between this and building? The build stuff is at the top level. People presumably need to package U-Boot as well, in order to use it. > > > doc/package/index.rst | 20 ++ > > tools/binman/{README => README.rst} | 480 ++-- > > tools/binman/ is the wrong path for documentation. Please, put the file > in /doc/develop/binman.rst or /doc/develop/tools/binman.rst so that it > becomes available online. Does this mean that README.rst would not appear? Why not? See the commit message for why I chose this. I suppose I could add a separate README if needed. Regards, Simon
Re: [PATCH 16/20] binman: doc: Add documentation to htmldocs
On 3/7/21 10:22 PM, Simon Glass wrote: Hi Heinrich, On Sun, 7 Mar 2021 at 15:02, Heinrich Schuchardt wrote: On 3/7/21 8:31 PM, Simon Glass wrote: Add a link to binman's documentation and adjust the files so that it is accessible. Use the name README.rst so it is easy to discover when binman is installed without U-Boot. Signed-off-by: Simon Glass --- doc/index.rst | 1 + doc/package/binman | 1 + doc/package/fit.rst | 8 + This information is only of interest for developers, not for end-users. Please, place that information in /doc/develop/. What is the difference between this and building? The build stuff is at the top level. People presumably need to package U-Boot as well, in order to use it. I never needed to use binman directly to build U-Boot and deploy it. That is why I say it should be described in /doc/develop/ FIT images to containing a kernel are a different story. Those should be described in /doc/usage/ doc/package/index.rst | 20 ++ tools/binman/{README => README.rst} | 480 ++-- tools/binman/ is the wrong path for documentation. Please, put the file in /doc/develop/binman.rst or /doc/develop/tools/binman.rst so that it becomes available online. Does this mean that README.rst would not appear? Why not? All documentation should reside in doc -- You added a symbolic link doc/package/binman=>../../tools/binman including a whole bunch of unrelated files into doc. To me this is chaos. I want to have one directory with all documentation. A filename called README.rst does not convey what it is about. So it is unsuitable inside doc/ Binman is useless outside U-Boot as it needs the board device-trees coming with U-Boot. There is no reason to install it without U-Boot. The patch creates a bug --- $ tools/binman/binman -H more: cannot open tools/binman/README: No such file or directory The patch is not applicable to origin/master Applying: binman: doc: Add documentation to htmldocs error: patch failed: tools/binman/README:844 error: tools/binman/README: patch does not apply doc/package/binman/README.rst does not build Warning, treated as error: doc/package/binman/README.rst:851:Definition list ends without a blank line; unexpected unindent. So I suggest that you put a binman.rst into /doc/devel and fix binman to find its documentation. Best regards Heinrich See the commit message for why I chose this. I suppose I could add a separate README if needed. Regards, Simon
Re: [PATCH] configs: add PineTab defconfig
On Sun, 07 Mar 2021 12:34:14 -0800 Vagrant Cascadian wrote: > On 2021-03-07, Nicolas Boulenguez wrote: > > From: Arnaud Ferraris > > > > The PineTab device-tree is already in u-boot, this commit adds the > > corresponding > > defconfig, based on pinephone_defconfig. > > > > Signed-off-by: Arnaud Ferraris > ... > > --- /dev/null > > +++ b/configs/pinetab_defconfig > ... > > +CONFIG_BOOTDELAY=0 > > Setting bootdelay to 0 it almost impossible to debug issues in a running > u-boot. > > The default of 2 seconds that distro_bootcmd uses tries to strike a > balance between not slowing the boot down too much while still being > reasonably able to get into a u-boot shell when something goes wrong. > > Individual users or vendors can set this value as they see fit, but this > doesn't seem like a good default for mainline u-boot, at least to me. Yeah, we just had a similar discussion recently about the Pinephone. I think we keep the default of 2 seconds for the mainline defconfig, by not having any explicit entry in that file, so it reverts to the platform default. People can always change this in their .config, even with automated build systems, by using a simple sed command. Cheers, Andre
Re: [PATCH 16/20] binman: doc: Add documentation to htmldocs
Hi Heinrich, On Sun, 7 Mar 2021 at 17:13, Heinrich Schuchardt wrote: > > On 3/7/21 10:22 PM, Simon Glass wrote: > > Hi Heinrich, > > > > On Sun, 7 Mar 2021 at 15:02, Heinrich Schuchardt wrote: > >> > >> On 3/7/21 8:31 PM, Simon Glass wrote: > >>> Add a link to binman's documentation and adjust the files so that it is > >>> accessible. Use the name README.rst so it is easy to discover when binman > >>> is installed without U-Boot. > >>> > >>> Signed-off-by: Simon Glass > >>> --- > >>> > >>>doc/index.rst | 1 + > >>>doc/package/binman | 1 + > >>>doc/package/fit.rst | 8 + > >> > >> This information is only of interest for developers, not for end-users. > >> Please, place that information in /doc/develop/. > > > > What is the difference between this and building? The build stuff is > > at the top level. People presumably need to package U-Boot as well, in > > order to use it. > > I never needed to use binman directly to build U-Boot and deploy it. > That is why I say it should be described in /doc/develop/ > > FIT images to containing a kernel are a different story. Those should be > described in /doc/usage/ OK well I'll put it under develop. > > > > >> > >>>doc/package/index.rst | 20 ++ > >>>tools/binman/{README => README.rst} | 480 ++-- > >> > >> tools/binman/ is the wrong path for documentation. Please, put the file > >> in /doc/develop/binman.rst or /doc/develop/tools/binman.rst so that it > >> becomes available online. > > > > Does this mean that README.rst would not appear? Why not? > > All documentation should reside in doc > -- > > You added a symbolic link > > doc/package/binman=>../../tools/binman > > including a whole bunch of unrelated files into doc. > > To me this is chaos. I want to have one directory with all documentation. > > A filename called README.rst does not convey what it is about. So it is > unsuitable inside doc/ > > Binman is useless outside U-Boot as it needs the board device-trees > coming with U-Boot. There is no reason to install it without U-Boot. Actually it doesn't. We are using it with Zephyr, for example. I do want to support installing it on a system, just like patman. > > The patch creates a bug > --- > > $ tools/binman/binman -H > more: cannot open tools/binman/README: No such file or directory > > The patch is not applicable to origin/master It is based on us/nextthe background here is that a whole load of patches got punted to -next so everything I am doing now is on -next. > > > Applying: binman: doc: Add documentation to htmldocs > error: patch failed: tools/binman/README:844 > error: tools/binman/README: patch does not apply > > doc/package/binman/README.rst does not build > > > Warning, treated as error: > doc/package/binman/README.rst:851:Definition list ends without a blank > line; unexpected unindent. > > So I suggest that you put a binman.rst into /doc/devel and fix binman to > find its documentation. I cannot simply copy the docs into that directory. Then we end up with two copies. What exactly is wrong with a symlink? Regards, Simon
Re: [PATCH v4 39/42] doc: Move coccinelle into its own section
Hi Heinrich, On Thu, 4 Mar 2021 at 12:23, Heinrich Schuchardt wrote: > > On 04.03.21 14:51, Simon Glass wrote: > > This tool has nothing to do with testing. Create a new section for > > 'refactoring' and move it into there. It is likely that other topics may > > fall under the same heading, such as using moveconfig and search/replace > > tools. > > Coccinelle makes static code analysis. This has nothing to do with > refactoring. I saw it as static testing compared to the dynamic testing > done for instance by pytest. > > You might put it into one chapter with our continuous integration. I don't know where that is, so I'll drop this patch. I have only every used spatch, and that only for refactoring. Regards, Simon
Re: [PATCH 16/20] binman: doc: Add documentation to htmldocs
On 3/7/21 11:17 PM, Simon Glass wrote: Hi Heinrich, On Sun, 7 Mar 2021 at 17:13, Heinrich Schuchardt wrote: On 3/7/21 10:22 PM, Simon Glass wrote: Hi Heinrich, On Sun, 7 Mar 2021 at 15:02, Heinrich Schuchardt wrote: On 3/7/21 8:31 PM, Simon Glass wrote: Add a link to binman's documentation and adjust the files so that it is accessible. Use the name README.rst so it is easy to discover when binman is installed without U-Boot. Signed-off-by: Simon Glass --- doc/index.rst | 1 + doc/package/binman | 1 + doc/package/fit.rst | 8 + This information is only of interest for developers, not for end-users. Please, place that information in /doc/develop/. What is the difference between this and building? The build stuff is at the top level. People presumably need to package U-Boot as well, in order to use it. I never needed to use binman directly to build U-Boot and deploy it. That is why I say it should be described in /doc/develop/ FIT images to containing a kernel are a different story. Those should be described in /doc/usage/ OK well I'll put it under develop. doc/package/index.rst | 20 ++ tools/binman/{README => README.rst} | 480 ++-- tools/binman/ is the wrong path for documentation. Please, put the file in /doc/develop/binman.rst or /doc/develop/tools/binman.rst so that it becomes available online. Does this mean that README.rst would not appear? Why not? All documentation should reside in doc -- You added a symbolic link doc/package/binman=>../../tools/binman including a whole bunch of unrelated files into doc. To me this is chaos. I want to have one directory with all documentation. A filename called README.rst does not convey what it is about. So it is unsuitable inside doc/ Binman is useless outside U-Boot as it needs the board device-trees coming with U-Boot. There is no reason to install it without U-Boot. Actually it doesn't. We are using it with Zephyr, for example. I do The README says it uses the board device-trees. But I guess you know better where it could be used. Zephyr test management? https://www.getzephyr.com/ Zephyr RTOS? https://www.zephyrproject.org/ want to support installing it on a system, just like patman. The patch creates a bug --- $ tools/binman/binman -H more: cannot open tools/binman/README: No such file or directory The patch is not applicable to origin/master It is based on us/nextthe background here is that a whole load of patches got punted to -next so everything I am doing now is on -next. Applying: binman: doc: Add documentation to htmldocs error: patch failed: tools/binman/README:844 error: tools/binman/README: patch does not apply doc/package/binman/README.rst does not build Warning, treated as error: doc/package/binman/README.rst:851:Definition list ends without a blank line; unexpected unindent. So I suggest that you put a binman.rst into /doc/devel and fix binman to find its documentation. I cannot simply copy the docs into that directory. Then we end up with two copies. What exactly is wrong with a symlink? If you want to symlink, please, only symlink the single file and not the directory and in /doc call it binman.rst. Best regards Heinrich Regards, Simon
Re: [PATCH 16/20] binman: doc: Add documentation to htmldocs
Hi Heinrich, On Sun, 7 Mar 2021 at 17:31, Heinrich Schuchardt wrote: > > On 3/7/21 11:17 PM, Simon Glass wrote: > > Hi Heinrich, > > > > On Sun, 7 Mar 2021 at 17:13, Heinrich Schuchardt wrote: > >> > >> On 3/7/21 10:22 PM, Simon Glass wrote: > >>> Hi Heinrich, > >>> > >>> On Sun, 7 Mar 2021 at 15:02, Heinrich Schuchardt > >>> wrote: > > On 3/7/21 8:31 PM, Simon Glass wrote: > > Add a link to binman's documentation and adjust the files so that it is > > accessible. Use the name README.rst so it is easy to discover when > > binman > > is installed without U-Boot. > > > > Signed-off-by: Simon Glass > > --- > > > > doc/index.rst | 1 + > > doc/package/binman | 1 + > > doc/package/fit.rst | 8 + > > This information is only of interest for developers, not for end-users. > Please, place that information in /doc/develop/. > >>> > >>> What is the difference between this and building? The build stuff is > >>> at the top level. People presumably need to package U-Boot as well, in > >>> order to use it. > >> > >> I never needed to use binman directly to build U-Boot and deploy it. > >> That is why I say it should be described in /doc/develop/ > >> > >> FIT images to containing a kernel are a different story. Those should be > >> described in /doc/usage/ > > > > OK well I'll put it under develop. > > > >> > >>> > > > doc/package/index.rst | 20 ++ > > tools/binman/{README => README.rst} | 480 > > ++-- > > tools/binman/ is the wrong path for documentation. Please, put the file > in /doc/develop/binman.rst or /doc/develop/tools/binman.rst so that it > becomes available online. > >>> > >>> Does this mean that README.rst would not appear? Why not? > >> > >> All documentation should reside in doc > >> -- > >> > >> You added a symbolic link > >> > >> doc/package/binman=>../../tools/binman > >> > >> including a whole bunch of unrelated files into doc. > >> > >> To me this is chaos. I want to have one directory with all documentation. > >> > >> A filename called README.rst does not convey what it is about. So it is > >> unsuitable inside doc/ > >> > >> Binman is useless outside U-Boot as it needs the board device-trees > >> coming with U-Boot. There is no reason to install it without U-Boot. > > > > Actually it doesn't. We are using it with Zephyr, for example. I do > > The README says it uses the board device-trees. But I guess you know > better where it could be used. > > Zephyr test management? > https://www.getzephyr.com/ > > Zephyr RTOS? > https://www.zephyrproject.org/ That one. > > > want to support installing it on a system, just like patman. > > > >> > >> The patch creates a bug > >> --- > >> > >> $ tools/binman/binman -H > >> more: cannot open tools/binman/README: No such file or directory > >> > >> The patch is not applicable to origin/master > > > > It is based on us/nextthe background here is that a whole load of > > patches got punted to -next so everything I am doing now is on -next. > > > > > >> > >> > >> Applying: binman: doc: Add documentation to htmldocs > >> error: patch failed: tools/binman/README:844 > >> error: tools/binman/README: patch does not apply > >> > >> doc/package/binman/README.rst does not build > >> > >> > >> Warning, treated as error: > >> doc/package/binman/README.rst:851:Definition list ends without a blank > >> line; unexpected unindent. > >> > >> So I suggest that you put a binman.rst into /doc/devel and fix binman to > >> find its documentation. > > > > I cannot simply copy the docs into that directory. Then we end up with > > two copies. What exactly is wrong with a symlink? > > If you want to symlink, please, only symlink the single file and not the > directory and in /doc call it binman.rst. OK I can symlink each file individually. Regards, Simon
Re: [PATCH v4 06/42] doc: Explain how to run tests without pytest
Hi Heinrich, On Thu, 4 Mar 2021 at 11:13, Heinrich Schuchardt wrote: > > On 3/4/21 2:50 PM, Simon Glass wrote: > > Add details about how to run a sandbox test directly, without using > > pytest. This is more convenient for rapid development, since it is faster > > and allows easier use of a debugger. Also mention sandbox_flattree as an > > example of the different sandbox builds available. > > > > Signed-off-by: Simon Glass > > --- > > > > (no changes since v2) > > > > Changes in v2: > > - Put the docs in tests_sandbox since it is more related to sandbox > > - Put in a mention of tests_sandbox in the main testing docs > > > > doc/develop/index.rst | 1 + > > doc/develop/testing.rst | 9 > > doc/develop/tests_sandbox.rst | 79 +++ > > Thank you for adding this documentation. > > tests_sandbox.rst seems to be the wrong file name: But if you look at it, it is all about running tests using sandbox. > > We have three types of tests: > > * Python tests > * library tests started using the 'ut' shell commad > * UEFI related tests started using the 'bootefi selftest' command > > All three tpyes of tests are run both on real hardware and on the sandbox. > > Unfortunately a few Python tests require the sandbox. > > Some tests are not run on the sandbox by default though they could, e.g. > the UEFI tests run only on QEMU because you decided not to activate > CONFIG_EFI_SELFTEST on sandbox_defconfig. It was a long time ago, but I recall spending a year trying to upstream a bit of refactoring to allow it and then giving up. If you have a way to do it, please go ahead. > > Some tests can only be used interactively, e.g. > > setenv efi_selftest block image transfer > bootefi selftest > > Some tests cannot be run on the sandbox at all, e.g. > test/py/tests/test_fpga.py That is a bug :-) > > > 3 files changed, 89 insertions(+) > > create mode 100644 doc/develop/tests_sandbox.rst > > > > diff --git a/doc/develop/index.rst b/doc/develop/index.rst > > index ac57fdb8f30..50b1de3bdff 100644 > > --- a/doc/develop/index.rst > > +++ b/doc/develop/index.rst > > @@ -33,3 +33,4 @@ Testing > > coccinelle > > testing > > py_testing > > + tests_sandbox > > diff --git a/doc/develop/testing.rst b/doc/develop/testing.rst > > index f01ca4dc408..87c90eee271 100644 > > --- a/doc/develop/testing.rst > > +++ b/doc/develop/testing.rst > > @@ -36,6 +36,7 @@ U-Boot can be built as a user-space application (e.g. for > > Linux). This > > allows test to be executed without needing target hardware. The 'sandbox' > > target provides this feature and it is widely used in tests. > > > > +See :doc:`tests_sandbox` for more information. > > > > Pytest Suite > > > > @@ -51,8 +52,16 @@ You can run the tests on sandbox with:: > > > > This will produce HTML output in build-sandbox/test-log.html > > > > +Some tests run with other versions of sandbox. For example sandbox_flattree > > +runs the tests with livetree (the hierachical devicetree) disabled. You can > > +also select particular tests with -k:: > > + > > + ./test/py/test.py --bd sandbox_flattree --build -k hello > > + > > See test/py/README.md for more information about the pytest suite. > > > > +See :doc:`tests_sandbox` for how to run tests directly (not through > > pytest). > > + > > > > tbot > > > > diff --git a/doc/develop/tests_sandbox.rst b/doc/develop/tests_sandbox.rst > > new file mode 100644 > > index 000..85bbd4f6734 > > --- /dev/null > > +++ b/doc/develop/tests_sandbox.rst > > @@ -0,0 +1,79 @@ > > +.. SPDX-License-Identifier: GPL-2.0+ > > + > > +Tests Under the Hood > > + > > + > > +Running sandbox tests directly > > +-- > > + > > +Typically tests are run using the pytest suite. This is easy and always > > gets > > +things right. > > No, in the contrary: > > It is difficult to set up Python testing for real hardware. Running > non-Python tests from the console is trivial both on the sandbox and on > real hardware. OK I will reword it. > > > + > > +But it is also possible to run some sandbox tests directly. For example, > > this > > +runs the dm_test_gpio() test which you can find in test/dm/gpio.c:: > > + > > + $ ./u-boot -T -c "ut dm gpio" > This works only on the sandbox. To create examples assume that the shell > is already started: The whole point of this file is to describe using sandbox! > > => ut dm gpio > > and > > => bootefi selftest > > We should add a man-page for the ut command in /doc/usage/. OK, please go ahead. > > > + > > + > > + U-Boot 2021.01 > > + > > + Model: sandbox > > + DRAM: 128 MiB > > + WDT: Started with servicing (60s timeout) > > + MMC: mmc2: 2 (SD), mmc1: 1 (SD), mmc0: 0 (SD) > > + In:serial > > + Out: vidconsole > > + Err: vidconsole > > + Model: sandbox > > + SCSI: > > + Net: eth0: eth@10002000, eth5: eth@10003000, eth3: s
Re: [PATCH 1/2] power: regulator: add driver for ANATOP regulator
Dear Ying-Chun On 3/8/21 3:18 AM, Ying-Chun Liu wrote: > From: "Ying-Chun Liu (PaulLiu)" > > Anatop is an integrated regulator inside i.MX6 SoC. > There are 3 digital regulators which controls PU, CORE (ARM), and SOC. > And 3 analog regulators which controls 1P1, 2P5, 3P0 (USB). > This patch adds the Anatop regulator driver. > > Signed-off-by: Ying-Chun Liu (PaulLiu) > --- > drivers/power/regulator/Kconfig| 10 + > drivers/power/regulator/Makefile | 1 + > drivers/power/regulator/anatop_regulator.c | 289 + > 3 files changed, 300 insertions(+) > create mode 100644 drivers/power/regulator/anatop_regulator.c > > diff --git a/drivers/power/regulator/Kconfig b/drivers/power/regulator/Kconfig > index fbbea18c7d..1cd1f3f5ed 100644 > --- a/drivers/power/regulator/Kconfig > +++ b/drivers/power/regulator/Kconfig > @@ -312,6 +312,16 @@ config DM_REGULATOR_STPMIC1 > by the PMIC device. This driver is controlled by a device tree node > which includes voltage limits. > > +config DM_REGULATOR_ANATOP > + bool "Enable driver for ANATOP regulators" > + depends on DM_REGULATOR > + select REGMAP > + select SYSCON > + help > + Enable support for the Freescale i.MX on-chip ANATOP LDOs > + regulators. It is recommended that this option be enabled on > + i.MX6 platform. > + > config SPL_DM_REGULATOR_STPMIC1 > bool "Enable driver for STPMIC1 regulators in SPL" > depends on SPL_DM_REGULATOR && PMIC_STPMIC1 > diff --git a/drivers/power/regulator/Makefile > b/drivers/power/regulator/Makefile > index 9d58112dcb..e7198da911 100644 > --- a/drivers/power/regulator/Makefile > +++ b/drivers/power/regulator/Makefile > @@ -30,3 +30,4 @@ obj-$(CONFIG_DM_REGULATOR_TPS65910) += tps65910_regulator.o > obj-$(CONFIG_DM_REGULATOR_TPS62360) += tps62360_regulator.o > obj-$(CONFIG_$(SPL_)DM_REGULATOR_STPMIC1) += stpmic1.o > obj-$(CONFIG_DM_REGULATOR_TPS65941) += tps65941_regulator.o > +obj-$(CONFIG_$(SPL_)DM_REGULATOR_ANATOP) += anatop_regulator.o > diff --git a/drivers/power/regulator/anatop_regulator.c > b/drivers/power/regulator/anatop_regulator.c > new file mode 100644 > index 00..2bb5cdbac5 > --- /dev/null > +++ b/drivers/power/regulator/anatop_regulator.c > @@ -0,0 +1,289 @@ > +// SPDX-License-Identifier: GPL-2.0+ > +// > +// Copyright (C) 2011 Freescale Semiconductor, Inc. All Rights Reserved. > +// Copyright (C) 2021 Linaro > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include Well, i think that it can be removed more than now. > + > +#define LDO_RAMP_UP_UNIT_IN_CYCLES 64 /* 64 cycles per step */ > +#define LDO_RAMP_UP_FREQ_IN_MHZ 24 /* cycle based on 24M OSC */ > + > +#define LDO_POWER_GATE 0x00 > +#define LDO_FET_FULL_ON 0x1f > + > +struct anatop_regulator { > + const char *name; > + u32 control_reg; > + u32 vol_bit_shift; > + u32 vol_bit_width; > + u32 min_bit_val; > + u32 min_voltage; > + u32 max_voltage; > + u32 delay_reg; > + u32 delay_bit_shift; > + u32 delay_bit_width; > +}; > + > +static u32 anatop_get_bits(struct udevice *dev, u32 addr, int bit_shift, > +int bit_width) > +{ > + struct udevice *syscon; > + struct regmap *regmap; > + int err; > + u32 val, mask; > + > + syscon = dev_get_parent(dev); > + if (!syscon) { > + dev_err(dev, "%s: unable to find syscon device\n", __func__); > + return err; > + } > + > + regmap = syscon_get_regmap(syscon); > + if (IS_ERR(regmap)) { > + dev_err(dev, "%s: unable to find regmap (%ld)\n", __func__, > + PTR_ERR(regmap)); > + return PTR_ERR(regmap); > + } > + > + if (bit_width == 32) Use macro instead of 32, plz. > + mask = ~0; > + else > + mask = (1 << bit_width) - 1; > + > + err = regmap_read(regmap, addr, &val); > + if (err) > + return err; > + > + val = (val >> bit_shift) & mask; > + > + return val; > +} > + > +void anatop_set_bits(struct udevice *dev, u32 addr, int bit_shift, > + int bit_width, u32 data) static? doesn't it need to return int type? If there is a somehting failed, it seems that it needs to pass the error number. (get_bits is returend to error..but set_bits doesn't return. It's strange.) And How about passing the struct anatop_regulator instead of each values? anatop_get/set_bits(struct anatop_regulator *regulator) { .. } > +{ > + struct udevice *syscon; > + struct regmap *regmap; > + int err; > + u32 val, mask; > + > + syscon = dev_get_parent(dev); > + if (!syscon) { > + dev_err(dev, "%s: unable to find syscon device\n", __func__); > + return; > + } > + > +
Re: [PATCH] pico-imx6ul: Pass the PMIC I2C address in pmic_get()
On 3/6/21 7:11 AM, Fabio Estevam wrote: > Pass "pfuze3000@8" in pmic_get() so that the PMIC node can > be found in the devicetree. > > Signed-off-by: Fabio Estevam Reviewed-by: Jaehoon Chung Best Regards, Jaehoon Chung > --- > board/technexion/pico-imx6ul/pico-imx6ul.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/board/technexion/pico-imx6ul/pico-imx6ul.c > b/board/technexion/pico-imx6ul/pico-imx6ul.c > index 62a54d0c8eb5..682c88dee78d 100644 > --- a/board/technexion/pico-imx6ul/pico-imx6ul.c > +++ b/board/technexion/pico-imx6ul/pico-imx6ul.c > @@ -159,7 +159,7 @@ int power_init_board(void) > struct udevice *dev; > int ret, dev_id, rev_id; > > - ret = pmic_get("pfuze3000", &dev); > + ret = pmic_get("pfuze3000@8", &dev); > if (ret == -ENODEV) > return 0; > if (ret != 0) >
Re: [PATCH v4 42/42] doc: Explain briefly how to write new tests
Hi Heinrich, On Thu, 4 Mar 2021 at 13:14, Heinrich Schuchardt wrote: > > On 04.03.21 14:51, Simon Glass wrote: > > Add a second on writing tests, covering when to use Python and C, where > > to put the tests, etc. Add a link to the existing Python test > > documentation. > > > > Signed-off-by: Simon Glass > > --- > > > > Changes in v4: > > - Add correct SPDX header > > > > Changes in v2: > > - Add new patches to cover running an SPL test > > > > doc/develop/index.rst | 1 + > > doc/develop/py_testing.rst| 3 +- > > doc/develop/testing.rst | 2 + > > doc/develop/tests_sandbox.rst | 7 + > > doc/develop/tests_writing.rst | 339 ++ > > 5 files changed, 351 insertions(+), 1 deletion(-) > > create mode 100644 doc/develop/tests_writing.rst > > > > diff --git a/doc/develop/index.rst b/doc/develop/index.rst > > index 9208668a2d4..8f0a598b5f2 100644 > > --- a/doc/develop/index.rst > > +++ b/doc/develop/index.rst > > @@ -32,6 +32,7 @@ Testing > > > > testing > > py_testing > > + tests_writing > > tests_sandbox > > > > Refactoring > > diff --git a/doc/develop/py_testing.rst b/doc/develop/py_testing.rst > > index 7f01858cfda..c4cecc0a01b 100644 > > --- a/doc/develop/py_testing.rst > > +++ b/doc/develop/py_testing.rst > > @@ -13,7 +13,8 @@ results. Advantages of this approach are: > >U-Boot; there can be no disconnect. > > - There is no need to write or embed test-related code into U-Boot itself. > >It is asserted that writing test-related code in Python is simpler and > > more > > - flexible than writing it all in C. > > + flexible than writing it all in C. But see :doc:`tests_writing` for > > caveats > > + and more discussion / analysis. > > - It is reasonably simple to interact with U-Boot in this way. > > > > Requirements > > diff --git a/doc/develop/testing.rst b/doc/develop/testing.rst > > index b181c2e2e41..ced13ac8bb4 100644 > > --- a/doc/develop/testing.rst > > +++ b/doc/develop/testing.rst > > @@ -117,6 +117,8 @@ or is covered sparingly. So here are some suggestions: > >is much easier to add onto a test - writing a new large test can seem > >daunting to most contributors. > > > > +See doc:`tests_writing` for how to write tests. > > + > > > > Future work > > --- > > diff --git a/doc/develop/tests_sandbox.rst b/doc/develop/tests_sandbox.rst > > index 8c2eac4c5f4..7e5cc84e43c 100644 > > --- a/doc/develop/tests_sandbox.rst > > +++ b/doc/develop/tests_sandbox.rst > > @@ -199,3 +199,10 @@ linker_list:: > > 0001f240 D _u_boot_list_2_dm_test_2_dm_test_of_plat_parent > > 0001f260 D _u_boot_list_2_dm_test_2_dm_test_of_plat_phandle > > 0001f280 D _u_boot_list_2_dm_test_2_dm_test_of_plat_props > > + > > + > > +Writing tests > > +- > > + > > +See :doc:`tests_writing` for how to write new tests. > > + > > diff --git a/doc/develop/tests_writing.rst b/doc/develop/tests_writing.rst > > new file mode 100644 > > index 000..d926c00f6e1 > > --- /dev/null > > +++ b/doc/develop/tests_writing.rst > > @@ -0,0 +1,339 @@ > > +.. SPDX-License-Identifier: GPL-2.0+ > > +.. Copyright 2021 Google LLC > > +.. sectionauthor:: Simon Glass > > + > > +Writing Tests > > += > > + > > +This describes how to write tests in U-Boot and describes the possible > > options. > > + > > +Test types > > +-- > > + > > +There are two basic types of test in U-Boot: > > + > > + - Python tests, in test/py/tests > > + - C tests, in test/ and its subdirectories > > You missed lib/efi_selftest/ for the UEFI tests. OK I will mention that. But really it is something of a pain that they are separate. > > > + > > +Python tests talk to U-Boot via the command line. They support both > > sandbox and > > +real hardware. They typically do not require building test code into U-Boot > > +itself. They are fairly slow to run, due to the command-line interface and > > there > > +being two separate processes. Python tests are fairly easy to write. They > > can > > +be a little tricky to debug sometimes due to the voluminous output of > > pytest. > > + > > +C tests are written directly in U-Boot. While they can be used on boards, > > they > > +are more commonly used with sandbox, as they obviously add to U-Boot code > > size. > > +C tests are easy to write so long as the required facilities exist. Where > > they > > +do not it can involve refactoring or adding new features to sandbox. They > > are > > +fast to run and easy to debug. > > + > > +Regardless of which test type is used, all tests are collected and run by > > the > > +pytest framework, so there is typically no need to run them separately. > > This > > +means that C tests can be used when it makes sense, and Python tests when > > it > > +doesn't. > > + > > + > > +This table shows how to decide whether to write a C or Python test: > > + > > += === > >
Re: [RFC] tests in test/dm/video.c are not independent
Hi Heinrich, On Sun, 7 Mar 2021 at 11:49, Heinrich Schuchardt wrote: > > Hello Simon, > > looking at the tests in test/dm/video.c it seems that none of them > clears the screen before adding text and testing the hash of the frame > buffer. So it is hard to tell which output led to changes in subsequent > tests. > > I think the tests should first clear the screen to start from a defined > state. This will make the tests independent of each other. > > What is your view? Well we reinit driver model before every test, so in effect they are independent. In fact that are more independent than just with the display. Regards, Simon
Re: [PATCH] configs: add PineTab defconfig
On Sun, 7 Mar 2021 13:53:56 +0100 Nicolas Boulenguez wrote: Hi, > From: Arnaud Ferraris > > The PineTab device-tree is already in u-boot, this commit adds the > corresponding > defconfig, based on pinephone_defconfig. > > Signed-off-by: Arnaud Ferraris > > --- a/board/sunxi/MAINTAINERS > +++ b/board/sunxi/MAINTAINERS > @@ -471,6 +471,11 @@ M: Samuel Holland > S: Maintained > F: configs/pinephone_defconfig > > +PINETAB BOARD > +M: Arnaud Ferraris > +S: Maintained > +F: configs/pinetab_defconfig Arnaud, do you agree with this? Happy to take your patch via Nicolas, but for the maintainer entry I would like to have some confirmation. > + > R16 EVB PARROT BOARD > M: Quentin Schulz > S: Maintained > --- /dev/null > +++ b/configs/pinetab_defconfig > @@ -0,0 +1,21 @@ > +CONFIG_ARM=y > +CONFIG_ARCH_SUNXI=y > +CONFIG_SPL=y > +CONFIG_IDENT_STRING="" Having "Allwinner Technology" here is indeed weird and probably not really justified anymore, given the "support" we see from Allwinner. I wonder if we should scrap this for all boards. Also it makes the line longer than 80 characters. > +CONFIG_MACH_SUN50I=y > +CONFIG_SUNXI_DRAM_LPDDR3_STOCK=y > +CONFIG_DRAM_CLK=552 > +CONFIG_DRAM_ZQ=3881949 > +CONFIG_MMC_SUNXI_SLOT_EXTRA=2 > +# CONFIG_VIDEO_DE2 is not set > +CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-pinetab" > +# CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set > +CONFIG_BOOTDELAY=0 I answered in the other email about the boot delay already. So what is the reason for all those other options below? Is there any particular reason they were all disabled? I can buy CONFIG_NET, but the rest seems unnecessary. There doesn't seem to be a driver for the PineTab panel in U-Boot, so this is solely suppressing a few lines on the serial? Since this would be surely for debug only, I think it's useful to have them, normal users wouldn't see them anyway. > +CONFIG_SYS_CONSOLE_INFO_QUIET=y > +# CONFIG_DISPLAY_CPUINFO is not set > +# CONFIG_DISPLAY_BOARDINFO is not set > +# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set > +# CONFIG_SPL_BANNER_PRINT is not set > +# CONFIG_SPL_POWER_SUPPORT is not set > +# CONFIG_NET is not set > +# CONFIG_EFI_LOADER is not set We should definitely keep EFI_LOADER. Cheers, Andre
Re: [PATCH] configs: add PineTab defconfig
于 2021年3月8日 GMT+08:00 上午8:12:24, Andre Przywara 写到: >On Sun, 7 Mar 2021 13:53:56 +0100 >Nicolas Boulenguez wrote: > >Hi, > >> From: Arnaud Ferraris >> >> The PineTab device-tree is already in u-boot, this commit adds the >corresponding >> defconfig, based on pinephone_defconfig. >> >> Signed-off-by: Arnaud Ferraris >> >> --- a/board/sunxi/MAINTAINERS >> +++ b/board/sunxi/MAINTAINERS >> @@ -471,6 +471,11 @@ M: Samuel Holland >> S: Maintained >> F: configs/pinephone_defconfig >> >> +PINETAB BOARD >> +M: Arnaud Ferraris >> +S: Maintained >> +F: configs/pinetab_defconfig > >Arnaud, do you agree with this? >Happy to take your patch via Nicolas, but for the maintainer entry I >would like to have some confirmation. > >> + >> R16 EVB PARROT BOARD >> M: Quentin Schulz >> S: Maintained >> --- /dev/null >> +++ b/configs/pinetab_defconfig >> @@ -0,0 +1,21 @@ >> +CONFIG_ARM=y >> +CONFIG_ARCH_SUNXI=y >> +CONFIG_SPL=y >> +CONFIG_IDENT_STRING="" > >Having "Allwinner Technology" here is indeed weird and probably not >really justified anymore, given the "support" we see from Allwinner. >I wonder if we should scrap this for all boards. Also it makes the line >longer than 80 characters. But if we do so, it should be in Kconfig, not defconfig. > >> +CONFIG_MACH_SUN50I=y >> +CONFIG_SUNXI_DRAM_LPDDR3_STOCK=y >> +CONFIG_DRAM_CLK=552 >> +CONFIG_DRAM_ZQ=3881949 >> +CONFIG_MMC_SUNXI_SLOT_EXTRA=2 >> +# CONFIG_VIDEO_DE2 is not set >> +CONFIG_DEFAULT_DEVICE_TREE="sun50i-a64-pinetab" >> +# CONFIG_SYS_MALLOC_CLEAR_ON_INIT is not set >> +CONFIG_BOOTDELAY=0 > >I answered in the other email about the boot delay already. > >So what is the reason for all those other options below? >Is there any particular reason they were all disabled? >I can buy CONFIG_NET, but the rest seems unnecessary. There doesn't >seem to be a driver for the PineTab panel in U-Boot, so this is solely >suppressing a few lines on the serial? Since this would be surely for >debug only, I think it's useful to have them, normal users wouldn't see >them anyway. > >> +CONFIG_SYS_CONSOLE_INFO_QUIET=y >> +# CONFIG_DISPLAY_CPUINFO is not set >> +# CONFIG_DISPLAY_BOARDINFO is not set >> +# CONFIG_SPL_RAW_IMAGE_SUPPORT is not set >> +# CONFIG_SPL_BANNER_PRINT is not set >> +# CONFIG_SPL_POWER_SUPPORT is not set >> +# CONFIG_NET is not set >> +# CONFIG_EFI_LOADER is not set > >We should definitely keep EFI_LOADER. > >Cheers, >Andre
[PATCH v5 00/41] test: Refactor tests to have a single test runner
At present U-Boot has two broad sets of tests in the C code: driver model tests which do a lot of pre-/post-init and command tests which do not. This separation makes it slightly harder to write a test, since there are two different test-state structures and different rules for running the two different test types. At present these rules are determined by where the test is (actually its prefix). All unit tests can be run from the command line with the 'ut' command. Since SPL does not have commands, it currently calls the test runner directly and offers no control of which tests are run. This seems like a good time to refactor the tests into a unified test runner, allowing U-Boot proper and SPL to use the same path, perhaps with some different conditions along the way. This series sets up a unified runner called ut_run_list(), which runs a set of tests from a linker_list. Driver model tests are distinguished by a new UT_TESTF_DM flag so that the necessary init and cleanup can still be done. The runner is modified to support running SPL tests that are not solely for driver model. An example test for FIT loading is added as a demonstration. In addition, some documentation is added to explain how to write tests. This series is available at u-boot-dm/test-working Changes in v5: - Update the test so we don't need a flag - Update based on Heinrich's comments - Mention UEFI tests - Update language to make it clear when sandbox is discussed - Update table based on Heinrich's comments Changes in v4: - Fix 'of-pldata' typo - Rebase on master - Add correct SPDX header Changes in v3: - Add new patch to re-enable test_ofplatdata - Reword the SPL tests section for clarity - Use test_set_state() throughout test-main.c instead of direct assignment - Update the pytest collector as well Changes in v2: - Use correct rst format for 'Ad-hoc tests' section - Expand docs on how each type of test is marked - Put the docs in tests_sandbox since it is more related to sandbox - Put in a mention of tests_sandbox in the main testing docs - Add a note that SPL tests can in fact be run individualy - Document how to run all C tests with 'ut all' - Fix 'get list' typo - Fix conditions so non-DM SPL tests are actually run - Allow for prefix to be NULL, to match function comment - Add new patches to cover running an SPL test Simon Glass (41): dm: core: Fix allocation of empty of-platdata doc: Tidy up testing section doc: Document make tcheck sandbox: Drop the 'starting...' message test: Re-enable test_ofplatdata doc: Explain how to run tests without pytest doc: Document how sandbox_spl_tests are run test: Correct setexpr test prefix test: Mark all driver model tests with a flag test: Rename test-main.c to test-dm.c test: Add an overall test runner test: Create pre/post-run functions test: Call test_pre/post_run() from driver model tests test: Move dm_extended_scan() to test_pre_run() test: Move do_autoprobe() to test_pre_run() test: Move dm_scan_plat() to test_pre_run() test: Drop mallinfo() work-around test: Move console silencing to test_pre_run() test: Move delay skipping to test_pre_run() test: Handle driver model reinit in test_pre_run() test: Drop struct dm_test_state test: Move dm_test_init() into test-main.c test: Move dm_test_destroy() into test-main.c test: Move test running into a separate function test: Use ut_run_test() to run driver model tests test: Drop dm_do_test() test: Add ut_run_test_live_flat() to run tests twice test: Use a local variable for test state test: Run driver-model tests using ut_run_list() test: Use return values in dm_test_run() test: Move the devicetree check into ut_run_list() test: Move restoring of driver model state to ut_run_list() test: log: Rename log main test file to log_ut.c test: Add a macros for finding tests in linker_lists test: Rename all linker lists to have a ut_ prefix test: Allow SPL to run any available test sandbox: Update os_find_u_boot() to find the .img file spl: Convert spl_fit to work with sandbox spl: test: Add a test for spl_load_simple_fit() test: sandbox: Move sandbox test docs into doc/develop doc: Explain briefly how to write new tests arch/sandbox/cpu/os.c | 8 +- arch/sandbox/cpu/spl.c | 9 +- arch/sandbox/cpu/start.c | 1 - common/spl/spl.c | 3 +- common/spl/spl_fit.c | 27 +- configs/sandbox_spl_defconfig | 2 +- doc/arch/sandbox.rst | 48 +--- doc/develop/index.rst | 2 + doc/develop/py_testing.rst | 3 +- doc/develop/testing.rst| 46 +++- doc/develop/tests_sandbox.rst | 209 ++ doc/develop/tests_writing.rst | 346 +++ drivers/core/device.c | 17 +- include/dm/test.h | 20 +- include/os.h | 3 +- include/spl.h | 9 + inclu
[PATCH v5 01/41] dm: core: Fix allocation of empty of-platdata
With of-platdata we always have a dtv struct that holds the platform data provided by the driver_info record. However, this struct can be empty if there are no actual devicetree properties provided. The upshot of empty platform data is that it will end up as a zero-size member in the BSS section, which is fine. But if the driver specifies plat_auto then it expects the correct amount of space to be allocated. At present this does not happen, since device_bind() assumes that the platform-data size will always be >0. As a result we end up not allocating the space and just use the BSS region, overwriting whatever other contents are present. Fix this by removing the condition that platform data be non-empty, always allocating space if requested. This fixes a strange bug that has been lurking since of-platdata was implemented. It has likely never been noticed since devices normally have at least some devicetree properties, BSS is seldom used on SPL, the dtv structs are normally at the end of bss and the overwriting only happens if a driver changes its platform data. It was discovered using sandbox_spl, which exercises more features than a normal board might, and the critical global_data variable 'gd' happened to be at the end of BSS. Fixes: 9fa28190091 ("dm: core: Expand platdata for of-platdata devices") Signed-off-by: Simon Glass --- (no changes since v1) drivers/core/device.c | 17 + 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/drivers/core/device.c b/drivers/core/device.c index 625134921d6..d1098a3861a 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -92,15 +92,19 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv, if (auto_seq && !(uc->uc_drv->flags & DM_UC_FLAG_NO_AUTO_SEQ)) dev->seq_ = uclass_find_next_free_seq(uc); + /* Check if we need to allocate plat */ if (drv->plat_auto) { bool alloc = !plat; + /* +* For of-platdata, we try use the existing data, but if +* plat_auto is larger, we must allocate a new space +*/ if (CONFIG_IS_ENABLED(OF_PLATDATA)) { - if (of_plat_size) { + if (of_plat_size) dev_or_flags(dev, DM_FLAG_OF_PLATDATA); - if (of_plat_size < drv->plat_auto) - alloc = true; - } + if (of_plat_size < drv->plat_auto) + alloc = true; } if (alloc) { dev_or_flags(dev, DM_FLAG_ALLOC_PDATA); @@ -109,6 +113,11 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv, ret = -ENOMEM; goto fail_alloc1; } + + /* +* For of-platdata, copy the old plat into the new +* space +*/ if (CONFIG_IS_ENABLED(OF_PLATDATA) && plat) memcpy(ptr, plat, of_plat_size); dev_set_plat(dev, ptr); -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 02/41] doc: Tidy up testing section
Tweak this so the output looks a little better. Signed-off-by: Simon Glass Reviewed-by: Heinrich Schuchardt --- (no changes since v2) Changes in v2: - Use correct rst format for 'Ad-hoc tests' section doc/develop/testing.rst | 20 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/doc/develop/testing.rst b/doc/develop/testing.rst index 4bc9ca3a6ae..bc74eb53e35 100644 --- a/doc/develop/testing.rst +++ b/doc/develop/testing.rst @@ -8,14 +8,14 @@ tested and what tests you should write when adding a new feature. Running tests - -To run most tests on sandbox, type this: +To run most tests on sandbox, type this:: make check in the U-Boot directory. Note that only the pytest suite is run using this command. -Some tests take ages to run. To run just the quick ones, type this: +Some tests take ages to run. To run just the quick ones, type this:: make qcheck @@ -35,9 +35,9 @@ either on sandbox or on real hardware. It relies on the U-Boot console to inject test commands and check the result. It is slower to run than C code, but provides the ability to unify lots of tests and summarise their results. -You can run the tests on sandbox with: +You can run the tests on sandbox with:: - ./test/py/test.py --bd sandbox --build + ./test/py/test.py --bd sandbox --build This will produce HTML output in build-sandbox/test-log.html @@ -58,10 +58,14 @@ Ad-hoc tests There are several ad-hoc tests which run outside the pytest environment: - test/fs - File system test (shell script) - test/image - FIT and legacy image tests (shell script and Python) - test/stdint - A test that stdint.h can be used in U-Boot (shell script) - trace - Test for the tracing feature (shell script) +test/fs + File system test (shell script) +test/image + FIT and legacy image tests (shell script and Python) +test/stdint + A test that stdint.h can be used in U-Boot (shell script) +trace + Test for the tracing feature (shell script) TODO: Move these into pytest. -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 05/41] test: Re-enable test_ofplatdata
This was inadvertently disabled after a recent change. Re-enable it. Signed-off-by: Simon Glass --- (no changes since v3) Changes in v3: - Add new patch to re-enable test_ofplatdata test/py/tests/test_ofplatdata.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/py/tests/test_ofplatdata.py b/test/py/tests/test_ofplatdata.py index 92d09b7aa19..e9cce4daf48 100644 --- a/test/py/tests/test_ofplatdata.py +++ b/test/py/tests/test_ofplatdata.py @@ -4,7 +4,7 @@ import pytest import u_boot_utils as util -@pytest.mark.boardspec('sandbox') +@pytest.mark.boardspec('sandbox_spl') @pytest.mark.buildconfigspec('spl_of_platdata') def test_spl_devicetree(u_boot_console): """Test content of spl device-tree""" -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 03/41] doc: Document make tcheck
Add a comment about this option in the documentation. Also mention the script that runs these combinations. Signed-off-by: Simon Glass --- (no changes since v2) Changes in v2: - Expand docs on how each type of test is marked doc/develop/testing.rst | 12 +++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/doc/develop/testing.rst b/doc/develop/testing.rst index bc74eb53e35..f01ca4dc408 100644 --- a/doc/develop/testing.rst +++ b/doc/develop/testing.rst @@ -15,10 +15,20 @@ To run most tests on sandbox, type this:: in the U-Boot directory. Note that only the pytest suite is run using this command. -Some tests take ages to run. To run just the quick ones, type this:: +Some tests take ages to run and are marked with @pytest.mark.slow. To run just +the quick ones, type this:: make qcheck +It is also possible to run just the tests for tools (patman, binman, etc.). +Such tests are included with those tools, i.e. no actual U-Boot unit tests are +run. Type this:: + +make tcheck + +All of the above use the test/run script with a paremeter to select which tests +are run. + Sandbox --- -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 09/41] test: Mark all driver model tests with a flag
Add a flag for driver model tests, so we can do special processing for them. Signed-off-by: Simon Glass --- (no changes since v1) include/dm/test.h | 3 ++- include/test/test.h | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/include/dm/test.h b/include/dm/test.h index 6ac6672cd6f..dfbc82c756d 100644 --- a/include/dm/test.h +++ b/include/dm/test.h @@ -143,7 +143,8 @@ struct dm_test_state { }; /* Declare a new driver model test */ -#define DM_TEST(_name, _flags) UNIT_TEST(_name, _flags, dm_test) +#define DM_TEST(_name, _flags) \ + UNIT_TEST(_name, UT_TESTF_DM | (_flags), dm_test) /* * struct sandbox_sdl_plat - Platform data for the SDL video driver diff --git a/include/test/test.h b/include/test/test.h index 3fdaa2b5e51..27585507d8c 100644 --- a/include/test/test.h +++ b/include/test/test.h @@ -36,6 +36,8 @@ enum { UT_TESTF_FLAT_TREE = BIT(3), /* test needs flat DT */ UT_TESTF_LIVE_TREE = BIT(4), /* needs live device tree */ UT_TESTF_CONSOLE_REC= BIT(5), /* needs console recording */ + /* do extra driver model init and uninit */ + UT_TESTF_DM = BIT(6), }; /** -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 06/41] doc: Explain how to run tests without pytest
Add details about how to run a sandbox test directly, without using pytest. This is more convenient for rapid development, since it is faster and allows easier use of a debugger. Also mention sandbox_flattree as an example of the different sandbox builds available. Signed-off-by: Simon Glass --- Changes in v5: - Update based on Heinrich's comments Changes in v2: - Put the docs in tests_sandbox since it is more related to sandbox - Put in a mention of tests_sandbox in the main testing docs doc/develop/index.rst | 1 + doc/develop/testing.rst | 9 doc/develop/tests_sandbox.rst | 80 +++ 3 files changed, 90 insertions(+) create mode 100644 doc/develop/tests_sandbox.rst diff --git a/doc/develop/index.rst b/doc/develop/index.rst index ac57fdb8f30..50b1de3bdff 100644 --- a/doc/develop/index.rst +++ b/doc/develop/index.rst @@ -33,3 +33,4 @@ Testing coccinelle testing py_testing + tests_sandbox diff --git a/doc/develop/testing.rst b/doc/develop/testing.rst index f01ca4dc408..87c90eee271 100644 --- a/doc/develop/testing.rst +++ b/doc/develop/testing.rst @@ -36,6 +36,7 @@ U-Boot can be built as a user-space application (e.g. for Linux). This allows test to be executed without needing target hardware. The 'sandbox' target provides this feature and it is widely used in tests. +See :doc:`tests_sandbox` for more information. Pytest Suite @@ -51,8 +52,16 @@ You can run the tests on sandbox with:: This will produce HTML output in build-sandbox/test-log.html +Some tests run with other versions of sandbox. For example sandbox_flattree +runs the tests with livetree (the hierachical devicetree) disabled. You can +also select particular tests with -k:: + + ./test/py/test.py --bd sandbox_flattree --build -k hello + See test/py/README.md for more information about the pytest suite. +See :doc:`tests_sandbox` for how to run tests directly (not through pytest). + tbot diff --git a/doc/develop/tests_sandbox.rst b/doc/develop/tests_sandbox.rst new file mode 100644 index 000..42b64882cd9 --- /dev/null +++ b/doc/develop/tests_sandbox.rst @@ -0,0 +1,80 @@ +.. SPDX-License-Identifier: GPL-2.0+ + +Tests Under the Hood + + +Running sandbox tests directly +-- + +Typically tests are run using the pytest suite. Running pytests on sandbox is +easy and always gets things right. For example some tests require files to be +set up before they can work. + +But it is also possible to run some sandbox tests directly. For example, this +runs the dm_test_gpio() test which you can find in test/dm/gpio.c:: + + $ ./u-boot -T -c "ut dm gpio" + + + U-Boot 2021.01 + + Model: sandbox + DRAM: 128 MiB + WDT: Started with servicing (60s timeout) + MMC: mmc2: 2 (SD), mmc1: 1 (SD), mmc0: 0 (SD) + In:serial + Out: vidconsole + Err: vidconsole + Model: sandbox + SCSI: + Net: eth0: eth@10002000, eth5: eth@10003000, eth3: sbe5, eth6: eth@10004000 + Test: dm_test_gpio: gpio.c + Test: dm_test_gpio: gpio.c (flat tree) + Failures: 0 + +The -T option tells the U-Boot sandbox to run with the 'test' devicetree +(test.dts) instead of -D which selects the normal sandbox.dts - this is +necessary because many tests rely on nodes or properties in the test devicetree. +If you try running tests without -T then you may see failures, like:: + + $ ./u-boot -c "ut dm gpio" + + + U-Boot 2021.01 + + DRAM: 128 MiB + WDT: Not found! + MMC: + In:serial + Out: serial + Err: serial + SCSI: + Net: No ethernet found. + Please run with test device tree: + ./u-boot -d arch/sandbox/dts/test.dtb + Test: dm_test_gpio: gpio.c + test/dm/gpio.c:37, dm_test_gpio(): 0 == gpio_lookup_name("b4", &dev, &offset, &gpio): Expected 0x0 (0), got 0xffea (-22) + Test: dm_test_gpio: gpio.c (flat tree) + test/dm/gpio.c:37, dm_test_gpio(): 0 == gpio_lookup_name("b4", &dev, &offset, &gpio): Expected 0x0 (0), got 0xffea (-22) + Failures: 2 + +The message above should provide a hint if you forget to use the -T flag. Even +running with -D will produce different results. + +You can easily use gdb on these tests, without needing --gdbserver:: + + $ gdb u-boot --args -T -c "ut dm gpio" + ... + (gdb) break dm_test_gpio + Breakpoint 1 at 0x1415bd: file test/dm/gpio.c, line 37. + (gdb) run -T -c "ut dm gpio" + Starting program: u-boot -T -c "ut dm gpio" + Test: dm_test_gpio: gpio.c + + Breakpoint 1, dm_test_gpio (uts=0x558029a0 ) + at files/test/dm/gpio.c:37 + 37 ut_assertok(gpio_lookup_name("b4", &dev, &offset, &gpio)); + (gdb) + +You can then single-step and look at variables as needed. + -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 04/41] sandbox: Drop the 'starting...' message
This message is annoying since it is only useful for testing. Drop it and update the test to cope. Signed-off-by: Simon Glass --- Changes in v5: - Update the test so we don't need a flag arch/sandbox/cpu/start.c | 1 - test/py/tests/test_log.py | 1 - 2 files changed, 2 deletions(-) diff --git a/arch/sandbox/cpu/start.c b/arch/sandbox/cpu/start.c index c4c4128d465..e87365e800d 100644 --- a/arch/sandbox/cpu/start.c +++ b/arch/sandbox/cpu/start.c @@ -491,7 +491,6 @@ int main(int argc, char *argv[]) gd->reloc_off = (ulong)gd->arch.text_base; /* sandbox test: log functions called before log_init in board_init_f */ - log_info("sandbox: starting...\n"); log_debug("debug: %s\n", __func__); /* Do pre- and post-relocation init */ diff --git a/test/py/tests/test_log.py b/test/py/tests/test_log.py index f889120f2b3..140dcb9aa2b 100644 --- a/test/py/tests/test_log.py +++ b/test/py/tests/test_log.py @@ -45,5 +45,4 @@ def test_log_dropped(u_boot_console): cons = u_boot_console cons.restart_uboot() output = cons.get_spawn_output().replace('\r', '') -assert 'sandbox: starting...' in output assert (not 'debug: main' in output) -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 07/41] doc: Document how sandbox_spl_tests are run
Add a few notes about the sandbox_spl tests, since they are special. Signed-off-by: Simon Glass Acked-by: Pratyush Yadav --- (no changes since v4) Changes in v4: - Fix 'of-pldata' typo Changes in v3: - Reword the SPL tests section for clarity Changes in v2: - Add a note that SPL tests can in fact be run individualy - Document how to run all C tests with 'ut all' - Fix 'get list' typo doc/develop/testing.rst | 5 +++ doc/develop/tests_sandbox.rst | 82 +++ 2 files changed, 87 insertions(+) diff --git a/doc/develop/testing.rst b/doc/develop/testing.rst index 87c90eee271..b181c2e2e41 100644 --- a/doc/develop/testing.rst +++ b/doc/develop/testing.rst @@ -58,6 +58,11 @@ also select particular tests with -k:: ./test/py/test.py --bd sandbox_flattree --build -k hello +There are some special tests that run in SPL. For this you need the sandbox_spl +build:: + + ./test/py/test.py --bd sandbox_spl --build -k test_spl + See test/py/README.md for more information about the pytest suite. See :doc:`tests_sandbox` for how to run tests directly (not through pytest). diff --git a/doc/develop/tests_sandbox.rst b/doc/develop/tests_sandbox.rst index 42b64882cd9..2b2c4be2dd1 100644 --- a/doc/develop/tests_sandbox.rst +++ b/doc/develop/tests_sandbox.rst @@ -78,3 +78,85 @@ You can easily use gdb on these tests, without needing --gdbserver:: You can then single-step and look at variables as needed. + +Running sandbox_spl tests directly +-- + +SPL is the phase before U-Boot proper. It is present in the sandbox_spl build, +so you can run SPL like this:: + + ./spl/u-boot-spl + +SPL tests are special in that they run (only in the SPL phase, of course) if the +-u flag is given:: + + ./spl/u-boot-spl -u + + U-Boot SPL 2021.01-00723-g43c77b51be5-dirty (Jan 24 2021 - 16:38:24 -0700) + Running 5 driver model tests + Test: dm_test_of_plat_base: of_platdata.c (flat tree) + Test: dm_test_of_plat_dev: of_platdata.c (flat tree) + Test: dm_test_of_plat_parent: of_platdata.c (flat tree) + Test: dm_test_of_plat_phandle: of_platdata.c (flat tree) + Test: dm_test_of_plat_props: of_platdata.c (flat tree) + Failures: 0 + + + U-Boot 2021.01-00723-g43c77b51be5-dirty (Jan 24 2021 - 16:38:24 -0700) + + DRAM: 128 MiB + ... + +It is not possible to run SPL tests in U-Boot proper, firstly because they are +not built into U-Boot proper and secondly because the environment is very +different, e.g. some SPL tests rely on of-platdata which is only available in +SPL. + +Note that after running, SPL continues to boot into U-Boot proper. You can add +'-c exit' to make U-Boot quit without doing anything further. It is not +currently possible to run SPL tests and then stop, since the pytests require +that U-Boot produces the expected banner. + +You can use the -k flag to select which tests run:: + + ./spl/u-boot-spl -u -k dm_test_of_plat_parent + +Of course you can use gdb with sandbox_spl, just as with sandbox. + + +Running all tests directly +-- + +A fast way to run all sandbox tests is:: + + ./u-boot -T -c "ut all" + +It typically runs single-thread in 6 seconds on 2021 hardware, with 2s of that +to the delays in the time test. + +This should not be considered a substitute for 'make check', but can be helpful +for git bisect, etc. + + +What tests are built in? + + +Whatever sandbox build is used, which tests are present is determined by which +source files are built. For sandbox_spl, the of_platdata tests are built +because of the build rule in test/dm/Makefile:: + + ifeq ($(CONFIG_SPL_BUILD),y) + obj-$(CONFIG_SPL_OF_PLATDATA) += of_platdata.o + else + ...other tests for non-spl + endif + +You can get a list of tests in a U-Boot ELF file by looking for the +linker_list:: + + $ nm /tmp/b/sandbox_spl/spl/u-boot-spl |grep 2_dm_test + 0001f200 D _u_boot_list_2_dm_test_2_dm_test_of_plat_base + 0001f220 D _u_boot_list_2_dm_test_2_dm_test_of_plat_dev + 0001f240 D _u_boot_list_2_dm_test_2_dm_test_of_plat_parent + 0001f260 D _u_boot_list_2_dm_test_2_dm_test_of_plat_phandle + 0001f280 D _u_boot_list_2_dm_test_2_dm_test_of_plat_props -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 10/41] test: Rename test-main.c to test-dm.c
This is the main test function for driver model but not for other tests. Rename the file and the function so this is clear. Signed-off-by: Simon Glass --- (no changes since v1) arch/sandbox/cpu/spl.c | 2 +- include/test/test.h| 4 ++-- test/dm/Makefile | 2 +- test/dm/{test-main.c => test-dm.c} | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) rename test/dm/{test-main.c => test-dm.c} (98%) diff --git a/arch/sandbox/cpu/spl.c b/arch/sandbox/cpu/spl.c index 6926e244ca2..3779d58c3fe 100644 --- a/arch/sandbox/cpu/spl.c +++ b/arch/sandbox/cpu/spl.c @@ -65,7 +65,7 @@ void spl_board_init(void) if (state->run_unittests) { int ret; - ret = dm_test_main(state->select_unittests); + ret = dm_test_run(state->select_unittests); /* continue execution into U-Boot */ } } diff --git a/include/test/test.h b/include/test/test.h index 27585507d8c..d282cb2362d 100644 --- a/include/test/test.h +++ b/include/test/test.h @@ -106,7 +106,7 @@ enum { struct udevice *testbus_get_clear_removed(void); /** - * dm_test_main() - Run driver model tests + * dm_test_run() - Run driver model tests * * Run all the available driver model tests, or a selection * @@ -114,6 +114,6 @@ struct udevice *testbus_get_clear_removed(void); * "fdt_pre_reloc"), or NULL to run all * @return 0 if all tests passed, 1 if not */ -int dm_test_main(const char *test_name); +int dm_test_run(const char *test_name); #endif /* __TEST_TEST_H */ diff --git a/test/dm/Makefile b/test/dm/Makefile index fd1455109d4..f5cc5540e8a 100644 --- a/test/dm/Makefile +++ b/test/dm/Makefile @@ -2,7 +2,7 @@ # # Copyright (c) 2013 Google, Inc -obj-$(CONFIG_UT_DM) += test-main.o +obj-$(CONFIG_UT_DM) += test-dm.o # Tests for particular subsystems - when enabling driver model for a new # subsystem you must add sandbox tests here. diff --git a/test/dm/test-main.c b/test/dm/test-dm.c similarity index 98% rename from test/dm/test-main.c rename to test/dm/test-dm.c index 560f8d63ec6..71e9cf6e5da 100644 --- a/test/dm/test-main.c +++ b/test/dm/test-dm.c @@ -146,7 +146,7 @@ static bool test_matches(const char *test_name, const char *find_name) return false; } -int dm_test_main(const char *test_name) +int dm_test_run(const char *test_name) { struct unit_test *tests = ll_entry_start(struct unit_test, dm_test); const int n_ents = ll_entry_count(struct unit_test, dm_test); @@ -226,5 +226,5 @@ int do_ut_dm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) if (argc > 1) test_name = argv[1]; - return dm_test_main(test_name); + return dm_test_run(test_name); } -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 08/41] test: Correct setexpr test prefix
This prefix should be for setexpr, not mem. This means that trying to select just these tests to run does not work. Fix it. For some reason this provokes an assertion failure due to memory not being freed. Move the env_set() in setexpr_test_str() to before the malloc() heap size size is recorded and disable the rest in setexpr_test_str_oper(). Signed-off-by: Simon Glass --- (no changes since v1) test/cmd/setexpr.c | 23 +++ 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/test/cmd/setexpr.c b/test/cmd/setexpr.c index fd6d869c0ed..b483069ff0f 100644 --- a/test/cmd/setexpr.c +++ b/test/cmd/setexpr.c @@ -306,8 +306,8 @@ static int setexpr_test_str(struct unit_test_state *uts) ut_asserteq(1, run_command("setexpr.s fred 0", 0)); ut_assertok(ut_check_delta(start_mem)); - start_mem = ut_check_free(); ut_assertok(env_set("fred", "12345")); + start_mem = ut_check_free(); ut_assertok(run_command("setexpr.s fred *0", 0)); ut_asserteq_str("hello", env_get("fred")); ut_assertok(ut_check_delta(start_mem)); @@ -345,7 +345,22 @@ static int setexpr_test_str_oper(struct unit_test_state *uts) start_mem = ut_check_free(); ut_assertok(run_command("setexpr.s fred *0 + *10", 0)); ut_asserteq_str("hello there", env_get("fred")); - ut_assertok(ut_check_delta(start_mem)); + + /* +* This check does not work with sandbox_flattree, apparently due to +* memory allocations in env_set(). +* +* The truetype console produces lots of memory allocations even though +* the LCD display is not visible. But even without these, it does not +* work. +* +* A better test would be for dlmalloc to record the allocs and frees +* for a particular caller, but that is not supported. +* +* For now, drop this test. +* +* ut_assertok(ut_check_delta(start_mem)); +*/ unmap_sysmem(buf); @@ -379,6 +394,6 @@ int do_ut_setexpr(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) setexpr_test); const int n_ents = ll_entry_count(struct unit_test, setexpr_test); - return cmd_ut_category("cmd_setexpr", "cmd_mem_", tests, n_ents, argc, - argv); + return cmd_ut_category("cmd_setexpr", "setexpr_test_", tests, n_ents, + argc, argv); } -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 12/41] test: Create pre/post-run functions
Split out the test preparation into a separation function before expanding it. Add a post-run function as well, currently empty. Signed-off-by: Simon Glass --- (no changes since v1) include/test/ut.h | 20 test/test-main.c | 41 +++-- 2 files changed, 51 insertions(+), 10 deletions(-) diff --git a/include/test/ut.h b/include/test/ut.h index 88e75ab791c..7cb5e10f3af 100644 --- a/include/test/ut.h +++ b/include/test/ut.h @@ -356,6 +356,26 @@ void ut_silence_console(struct unit_test_state *uts); */ void ut_unsilence_console(struct unit_test_state *uts); +/** + * test_pre_run() - Handle any preparation needed to run a test + * + * @uts: Test state + * @test: Test to prepare for + * @return 0 if OK, -EAGAIN to skip this test since some required feature is not + * available, other -ve on error (meaning that testing cannot likely + * continue) + */ +int test_pre_run(struct unit_test_state *uts, struct unit_test *test); + +/** + * test_post_run() - Handle cleaning up after a test + * + * @uts: Test state + * @test: Test to clean up after + * @return 0 if OK, -ve on error (meaning that testing cannot likely continue) + */ +int test_post_run(struct unit_test_state *uts, struct unit_test *test); + /** * ut_run_tests() - Run a set of tests * diff --git a/test/test-main.c b/test/test-main.c index 376e7ebd3d2..7961fd8aa3e 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -8,6 +8,27 @@ #include #include +int test_pre_run(struct unit_test_state *uts, struct unit_test *test) +{ + uts->start = mallinfo(); + + if (test->flags & UT_TESTF_CONSOLE_REC) { + int ret = console_record_reset_enable(); + + if (ret) { + printf("Skipping: Console recording disabled\n"); + return -EAGAIN; + } + } + + return 0; +} + +int test_post_run(struct unit_test_state *uts, struct unit_test *test) +{ + return 0; +} + int ut_run_tests(struct unit_test_state *uts, const char *prefix, struct unit_test *tests, int count, const char *select_name) { @@ -17,6 +38,7 @@ int ut_run_tests(struct unit_test_state *uts, const char *prefix, for (test = tests; test < tests + count; test++) { const char *test_name = test->name; + int ret; /* Remove the prefix */ if (prefix && !strncmp(test_name, prefix, prefix_len)) @@ -27,18 +49,17 @@ int ut_run_tests(struct unit_test_state *uts, const char *prefix, printf("Test: %s\n", test_name); found++; - if (test->flags & UT_TESTF_CONSOLE_REC) { - int ret = console_record_reset_enable(); - - if (ret) { - printf("Skipping: Console recording disabled\n"); - continue; - } - } - - uts->start = mallinfo(); + ret = test_pre_run(uts, test); + if (ret == -EAGAIN) + continue; + if (ret) + return ret; test->func(uts); + + ret = test_post_run(uts, test); + if (ret) + return ret; } if (select_name && !found) return -ENOENT; -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 16/41] test: Move dm_scan_plat() to test_pre_run()
Move this step over to the pre-run function. Signed-off-by: Simon Glass --- (no changes since v1) test/dm/test-dm.c | 2 -- test/test-main.c | 3 +++ 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/test/dm/test-dm.c b/test/dm/test-dm.c index c2e1a1b9207..18877c7ae56 100644 --- a/test/dm/test-dm.c +++ b/test/dm/test-dm.c @@ -74,8 +74,6 @@ static int dm_do_test(struct unit_test_state *uts, struct unit_test *test, ut_assertok(dm_test_init(uts, of_live)); uts->start = mallinfo(); - if (test->flags & UT_TESTF_SCAN_PDATA) - ut_assertok(dm_scan_plat(false)); ut_assertok(test_pre_run(uts, test)); diff --git a/test/test-main.c b/test/test-main.c index bd2f08a2b42..fe96d739dc4 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -34,6 +34,9 @@ int test_pre_run(struct unit_test_state *uts, struct unit_test *test) if (!(test->flags & UT_TESTF_DM)) uts->start = mallinfo(); + if (test->flags & UT_TESTF_SCAN_PDATA) + ut_assertok(dm_scan_plat(false)); + if (test->flags & UT_TESTF_PROBE_TEST) ut_assertok(do_autoprobe(uts)); -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 15/41] test: Move do_autoprobe() to test_pre_run()
Move this step over to the pre-run function. Signed-off-by: Simon Glass --- (no changes since v1) test/dm/test-dm.c | 17 - test/test-main.c | 18 ++ 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/test/dm/test-dm.c b/test/dm/test-dm.c index 4cb0da13b7c..c2e1a1b9207 100644 --- a/test/dm/test-dm.c +++ b/test/dm/test-dm.c @@ -43,21 +43,6 @@ static int dm_test_init(struct unit_test_state *uts, bool of_live) return 0; } -/* Ensure all the test devices are probed */ -static int do_autoprobe(struct unit_test_state *uts) -{ - struct udevice *dev; - int ret; - - /* Scanning the uclass is enough to probe all the devices */ - for (ret = uclass_first_device(UCLASS_TEST, &dev); -dev; -ret = uclass_next_device(&dev)) - ; - - return ret; -} - static int dm_test_destroy(struct unit_test_state *uts) { int id; @@ -91,8 +76,6 @@ static int dm_do_test(struct unit_test_state *uts, struct unit_test *test, uts->start = mallinfo(); if (test->flags & UT_TESTF_SCAN_PDATA) ut_assertok(dm_scan_plat(false)); - if (test->flags & UT_TESTF_PROBE_TEST) - ut_assertok(do_autoprobe(uts)); ut_assertok(test_pre_run(uts, test)); diff --git a/test/test-main.c b/test/test-main.c index a971fe0e9c8..bd2f08a2b42 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -13,12 +13,30 @@ DECLARE_GLOBAL_DATA_PTR; +/* Ensure all the test devices are probed */ +static int do_autoprobe(struct unit_test_state *uts) +{ + struct udevice *dev; + int ret; + + /* Scanning the uclass is enough to probe all the devices */ + for (ret = uclass_first_device(UCLASS_TEST, &dev); +dev; +ret = uclass_next_device(&dev)) + ; + + return ret; +} + int test_pre_run(struct unit_test_state *uts, struct unit_test *test) { /* DM tests have already done this */ if (!(test->flags & UT_TESTF_DM)) uts->start = mallinfo(); + if (test->flags & UT_TESTF_PROBE_TEST) + ut_assertok(do_autoprobe(uts)); + if (!CONFIG_IS_ENABLED(OF_PLATDATA) && (test->flags & UT_TESTF_SCAN_FDT)) ut_assertok(dm_extended_scan(false)); -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 17/41] test: Drop mallinfo() work-around
This is not needed now. Drop it. Signed-off-by: Simon Glass --- (no changes since v1) test/dm/test-dm.c | 2 -- test/test-main.c | 4 +--- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/test/dm/test-dm.c b/test/dm/test-dm.c index 18877c7ae56..d1d83e34782 100644 --- a/test/dm/test-dm.c +++ b/test/dm/test-dm.c @@ -73,8 +73,6 @@ static int dm_do_test(struct unit_test_state *uts, struct unit_test *test, !of_live ? " (flat tree)" : ""); ut_assertok(dm_test_init(uts, of_live)); - uts->start = mallinfo(); - ut_assertok(test_pre_run(uts, test)); if (!state->show_test_output) diff --git a/test/test-main.c b/test/test-main.c index fe96d739dc4..db0d82e36c3 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -30,9 +30,7 @@ static int do_autoprobe(struct unit_test_state *uts) int test_pre_run(struct unit_test_state *uts, struct unit_test *test) { - /* DM tests have already done this */ - if (!(test->flags & UT_TESTF_DM)) - uts->start = mallinfo(); + uts->start = mallinfo(); if (test->flags & UT_TESTF_SCAN_PDATA) ut_assertok(dm_scan_plat(false)); -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 11/41] test: Add an overall test runner
Add a new test runner that will eventually be able to run any test. For now, have it run the 'command' unit tests, so that the functionality in cmd_ut_category() moves into it. Signed-off-by: Simon Glass --- (no changes since v1) include/test/ut.h | 42 ++ test/Makefile | 2 ++ test/cmd_ut.c | 38 --- test/test-main.c | 66 +++ 4 files changed, 115 insertions(+), 33 deletions(-) create mode 100644 test/test-main.c diff --git a/include/test/ut.h b/include/test/ut.h index 17400c73ea9..88e75ab791c 100644 --- a/include/test/ut.h +++ b/include/test/ut.h @@ -356,4 +356,46 @@ void ut_silence_console(struct unit_test_state *uts); */ void ut_unsilence_console(struct unit_test_state *uts); +/** + * ut_run_tests() - Run a set of tests + * + * This runs the tests, handling any preparation and clean-up needed. It prints + * the name of each test before running it. + * + * @uts: Test state to update. The caller should ensure that this is zeroed for + * the first call to this function. On exit, @uts->fail_count is + * incremented by the number of failures (0, one hopes) + * @prefix: String prefix for the tests. Any tests that have this prefix will be + * printed without the prefix, so that it is easier to see the unique part + * of the test name. If NULL, no prefix processing is done + * @tests: List of tests to run + * @count: Number of tests to run + * @select_name: Name of a single test to run (from the list provided). If NULL + * then all tests are run + * @return 0 if all tests passed, -ENOENT if test @select_name was not found, + * -EBADF if any failed + */ +int ut_run_tests(struct unit_test_state *uts, const char *prefix, +struct unit_test *tests, int count, const char *select_name); + +/** + * ut_run_tests() - Run a set of tests + * + * This runs the test, handling any preparation and clean-up needed. It prints + * the name of each test before running it. + * + * @category: Category of these tests. This is a string printed at the start to + * announce the the number of tests + * @prefix: String prefix for the tests. Any tests that have this prefix will be + * printed without the prefix, so that it is easier to see the unique part + * of the test name. If NULL, no prefix processing is done + * @tests: List of tests to run + * @count: Number of tests to run + * @select_name: Name of a single test to run (from the list provided). If NULL + * then all tests are run + * @return 0 if all tests passed, -1 if any failed + */ +int ut_run_list(const char *name, const char *prefix, struct unit_test *tests, + int count, const char *select_name); + #endif diff --git a/test/Makefile b/test/Makefile index 932e5173831..5cd284e322e 100644 --- a/test/Makefile +++ b/test/Makefile @@ -2,6 +2,8 @@ # # (C) Copyright 2012 The Chromium Authors +obj-y += test-main.o + ifneq ($(CONFIG_$(SPL_)BLOBLIST),) obj-$(CONFIG_$(SPL_)CMDLINE) += bloblist.o obj-$(CONFIG_$(SPL_)CMDLINE) += bootm.o diff --git a/test/cmd_ut.c b/test/cmd_ut.c index 8f3089890ea..157f6aa9767 100644 --- a/test/cmd_ut.c +++ b/test/cmd_ut.c @@ -9,6 +9,7 @@ #include #include #include +#include static int do_ut_all(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]); @@ -17,41 +18,12 @@ int cmd_ut_category(const char *name, const char *prefix, struct unit_test *tests, int n_ents, int argc, char *const argv[]) { - struct unit_test_state uts = { .fail_count = 0 }; - struct unit_test *test; - int prefix_len = prefix ? strlen(prefix) : 0; + int ret; - if (argc == 1) - printf("Running %d %s tests\n", n_ents, name); + ret = ut_run_list(name, prefix, tests, n_ents, + argc > 1 ? argv[1] : NULL); - for (test = tests; test < tests + n_ents; test++) { - const char *test_name = test->name; - - /* Remove the prefix */ - if (prefix && !strncmp(test_name, prefix, prefix_len)) - test_name += prefix_len; - - if (argc > 1 && strcmp(argv[1], test_name)) - continue; - printf("Test: %s\n", test->name); - - if (test->flags & UT_TESTF_CONSOLE_REC) { - int ret = console_record_reset_enable(); - - if (ret) { - printf("Skipping: Console recording disabled\n"); - continue; - } - } - - uts.start = mallinfo(); - - test->func(&uts); - } - - printf("Failures: %d\n", uts.fail_count); - - return uts.fail_count ? CMD_RET_FAILURE : 0; + return ret ? CMD_RET_FAILURE : 0; } static struct cmd_tbl cmd_ut_sub[] = { diff --git a/test/test-ma
[PATCH v5 14/41] test: Move dm_extended_scan() to test_pre_run()
Move this step over to the pre-run function. Signed-off-by: Simon Glass --- (no changes since v1) test/dm/test-dm.c | 3 --- test/test-main.c | 7 +++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/test/dm/test-dm.c b/test/dm/test-dm.c index 69a0349d04c..4cb0da13b7c 100644 --- a/test/dm/test-dm.c +++ b/test/dm/test-dm.c @@ -93,9 +93,6 @@ static int dm_do_test(struct unit_test_state *uts, struct unit_test *test, ut_assertok(dm_scan_plat(false)); if (test->flags & UT_TESTF_PROBE_TEST) ut_assertok(do_autoprobe(uts)); - if (!CONFIG_IS_ENABLED(OF_PLATDATA) && - (test->flags & UT_TESTF_SCAN_FDT)) - ut_assertok(dm_extended_scan(false)); ut_assertok(test_pre_run(uts, test)); diff --git a/test/test-main.c b/test/test-main.c index 9c600094740..a971fe0e9c8 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -6,7 +6,10 @@ #include #include +#include +#include #include +#include DECLARE_GLOBAL_DATA_PTR; @@ -16,6 +19,10 @@ int test_pre_run(struct unit_test_state *uts, struct unit_test *test) if (!(test->flags & UT_TESTF_DM)) uts->start = mallinfo(); + if (!CONFIG_IS_ENABLED(OF_PLATDATA) && + (test->flags & UT_TESTF_SCAN_FDT)) + ut_assertok(dm_extended_scan(false)); + if (test->flags & UT_TESTF_CONSOLE_REC) { int ret = console_record_reset_enable(); -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 13/41] test: Call test_pre/post_run() from driver model tests
Ultimately we want to get rid of the special driver model test init and use test_pre_run() and test_post_run() for all tests. As a first step, use those function to handle console recording. For now we need a special case for setting uts->start, but that wil go away once all init is in one place. Signed-off-by: Simon Glass --- (no changes since v1) include/dm/test.h | 2 +- test/dm/test-dm.c | 10 +- test/test-main.c | 8 +++- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/include/dm/test.h b/include/dm/test.h index dfbc82c756d..c0b463cc0f1 100644 --- a/include/dm/test.h +++ b/include/dm/test.h @@ -144,7 +144,7 @@ struct dm_test_state { /* Declare a new driver model test */ #define DM_TEST(_name, _flags) \ - UNIT_TEST(_name, UT_TESTF_DM | (_flags), dm_test) + UNIT_TEST(_name, UT_TESTF_DM | UT_TESTF_CONSOLE_REC | (_flags), dm_test) /* * struct sandbox_sdl_plat - Platform data for the SDL video driver diff --git a/test/dm/test-dm.c b/test/dm/test-dm.c index 71e9cf6e5da..69a0349d04c 100644 --- a/test/dm/test-dm.c +++ b/test/dm/test-dm.c @@ -97,14 +97,14 @@ static int dm_do_test(struct unit_test_state *uts, struct unit_test *test, (test->flags & UT_TESTF_SCAN_FDT)) ut_assertok(dm_extended_scan(false)); - /* -* Silence the console and rely on console recording to get -* our output. -*/ - console_record_reset_enable(); + ut_assertok(test_pre_run(uts, test)); + if (!state->show_test_output) gd->flags |= GD_FLG_SILENT; test->func(uts); + + ut_assertok(test_post_run(uts, test)); + gd->flags &= ~(GD_FLG_SILENT | GD_FLG_RECORD); state_set_skip_delays(false); diff --git a/test/test-main.c b/test/test-main.c index 7961fd8aa3e..9c600094740 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -8,9 +8,13 @@ #include #include +DECLARE_GLOBAL_DATA_PTR; + int test_pre_run(struct unit_test_state *uts, struct unit_test *test) { - uts->start = mallinfo(); + /* DM tests have already done this */ + if (!(test->flags & UT_TESTF_DM)) + uts->start = mallinfo(); if (test->flags & UT_TESTF_CONSOLE_REC) { int ret = console_record_reset_enable(); @@ -26,6 +30,8 @@ int test_pre_run(struct unit_test_state *uts, struct unit_test *test) int test_post_run(struct unit_test_state *uts, struct unit_test *test) { + gd->flags &= ~GD_FLG_RECORD; + return 0; } -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 20/41] test: Handle driver model reinit in test_pre_run()
For driver model tests we want to reinit the data structures so that everything is in a known state before the test runs. This avoids one test changing something that breaks a subsequent tests. Move the call for this into test_pre_run(). Signed-off-by: Simon Glass --- (no changes since v1) include/test/test.h | 2 ++ include/test/ut.h | 10 ++ test/dm/test-dm.c | 6 +++--- test/test-main.c| 3 +++ 4 files changed, 18 insertions(+), 3 deletions(-) diff --git a/include/test/test.h b/include/test/test.h index d282cb2362d..6997568cc07 100644 --- a/include/test/test.h +++ b/include/test/test.h @@ -15,6 +15,7 @@ * @fail_count: Number of tests that failed * @start: Store the starting mallinfo when doing leak test * @priv: A pointer to some other info some suites want to track + * @of_live: true to use livetree if available, false to use flattree * @of_root: Record of the livetree root node (used for setting up tests) * @expect_str: Temporary string used to hold expected string value * @actual_str: Temporary string used to hold actual string value @@ -24,6 +25,7 @@ struct unit_test_state { struct mallinfo start; void *priv; struct device_node *of_root; + bool of_live; char expect_str[256]; char actual_str[256]; }; diff --git a/include/test/ut.h b/include/test/ut.h index e5ec18e60b0..6e56ca99c31 100644 --- a/include/test/ut.h +++ b/include/test/ut.h @@ -387,6 +387,16 @@ int test_pre_run(struct unit_test_state *uts, struct unit_test *test); */ int test_post_run(struct unit_test_state *uts, struct unit_test *test); +/** + * dm_test_init() - Get ready to run a driver model test + * + * This clears out the driver model data structures. For sandbox it resets the + * state structure. + * + * @uts: Test state + */ +int dm_test_init(struct unit_test_state *uts); + /** * ut_run_tests() - Run a set of tests * diff --git a/test/dm/test-dm.c b/test/dm/test-dm.c index 569ffbbad93..ceeac3fd361 100644 --- a/test/dm/test-dm.c +++ b/test/dm/test-dm.c @@ -24,10 +24,10 @@ DECLARE_GLOBAL_DATA_PTR; struct unit_test_state global_dm_test_state; static struct dm_test_state _global_priv_dm_test_state; -/* Get ready for testing */ -static int dm_test_init(struct unit_test_state *uts, bool of_live) +int dm_test_init(struct unit_test_state *uts) { struct dm_test_state *dms = uts->priv; + bool of_live = uts->of_live; memset(dms, '\0', sizeof(*dms)); gd->dm_root = NULL; @@ -70,7 +70,7 @@ static int dm_do_test(struct unit_test_state *uts, struct unit_test *test, printf("Test: %s: %s%s\n", test->name, fname, !of_live ? " (flat tree)" : ""); - ut_assertok(dm_test_init(uts, of_live)); + uts->of_live = of_live; ut_assertok(test_pre_run(uts, test)); diff --git a/test/test-main.c b/test/test-main.c index 6f0d32f7e27..f14b7b09f79 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -30,6 +30,9 @@ static int do_autoprobe(struct unit_test_state *uts) int test_pre_run(struct unit_test_state *uts, struct unit_test *test) { + if (test->flags & UT_TESTF_DM) + ut_assertok(dm_test_init(uts)); + ut_set_skip_delays(uts, false); uts->start = mallinfo(); -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 21/41] test: Drop struct dm_test_state
Driver model is a core part of U-Boot. We don't really need to have a separate test structure for the driver model tests and it makes it harder to write a test if you have to think about which type of test it is. Subsume the fields from struct dm_test_state into struct unit_test_state and delete the former. Signed-off-by: Simon Glass --- (no changes since v4) Changes in v4: - Rebase on master include/dm/test.h | 17 - include/test/test.h | 10 ++-- test/dm/core.c| 58 ++- test/dm/test-dm.c | 10 test/dm/test-driver.c | 4 +-- test/dm/test-uclass.c | 3 +-- 6 files changed, 39 insertions(+), 63 deletions(-) diff --git a/include/dm/test.h b/include/dm/test.h index c0b463cc0f1..fe1cc2e278c 100644 --- a/include/dm/test.h +++ b/include/dm/test.h @@ -125,23 +125,6 @@ extern int dm_testdrv_op_count[DM_TEST_OP_COUNT]; extern struct unit_test_state global_dm_test_state; -/* - * struct dm_test_state - Entire state of dm test system - * - * This is often abreviated to dms. - * - * @root: Root device - * @testdev: Test device - * @force_fail_alloc: Force all memory allocs to fail - * @skip_post_probe: Skip uclass post-probe processing - */ -struct dm_test_state { - struct udevice *root; - struct udevice *testdev; - int force_fail_alloc; - int skip_post_probe; -}; - /* Declare a new driver model test */ #define DM_TEST(_name, _flags) \ UNIT_TEST(_name, UT_TESTF_DM | UT_TESTF_CONSOLE_REC | (_flags), dm_test) diff --git a/include/test/test.h b/include/test/test.h index 6997568cc07..5eeec35f525 100644 --- a/include/test/test.h +++ b/include/test/test.h @@ -14,18 +14,24 @@ * * @fail_count: Number of tests that failed * @start: Store the starting mallinfo when doing leak test - * @priv: A pointer to some other info some suites want to track * @of_live: true to use livetree if available, false to use flattree * @of_root: Record of the livetree root node (used for setting up tests) + * @root: Root device + * @testdev: Test device + * @force_fail_alloc: Force all memory allocs to fail + * @skip_post_probe: Skip uclass post-probe processing * @expect_str: Temporary string used to hold expected string value * @actual_str: Temporary string used to hold actual string value */ struct unit_test_state { int fail_count; struct mallinfo start; - void *priv; struct device_node *of_root; bool of_live; + struct udevice *root; + struct udevice *testdev; + int force_fail_alloc; + int skip_post_probe; char expect_str[256]; char actual_str[256]; }; diff --git a/test/dm/core.c b/test/dm/core.c index 35ca689d646..2210345dd14 100644 --- a/test/dm/core.c +++ b/test/dm/core.c @@ -117,14 +117,13 @@ int dm_leak_check_end(struct unit_test_state *uts) /* Test that binding with plat occurs correctly */ static int dm_test_autobind(struct unit_test_state *uts) { - struct dm_test_state *dms = uts->priv; struct udevice *dev; /* * We should have a single class (UCLASS_ROOT) and a single root * device with no children. */ - ut_assert(dms->root); + ut_assert(uts->root); ut_asserteq(1, list_count_items(gd->uclass_root)); ut_asserteq(0, list_count_items(&gd->dm_root->child_head)); ut_asserteq(0, dm_testdrv_op_count[DM_TEST_OP_POST_BIND]); @@ -207,7 +206,6 @@ DM_TEST(dm_test_autobind_uclass_pdata_valid, UT_TESTF_SCAN_PDATA); /* Test that autoprobe finds all the expected devices */ static int dm_test_autoprobe(struct unit_test_state *uts) { - struct dm_test_state *dms = uts->priv; int expected_base_add; struct udevice *dev; struct uclass *uc; @@ -221,7 +219,7 @@ static int dm_test_autoprobe(struct unit_test_state *uts) ut_asserteq(0, dm_testdrv_op_count[DM_TEST_OP_POST_PROBE]); /* The root device should not be activated until needed */ - ut_assert(dev_get_flags(dms->root) & DM_FLAG_ACTIVATED); + ut_assert(dev_get_flags(uts->root) & DM_FLAG_ACTIVATED); /* * We should be able to find the three test devices, and they should @@ -241,7 +239,7 @@ static int dm_test_autoprobe(struct unit_test_state *uts) /* Activating a device should activate the root device */ if (!i) - ut_assert(dev_get_flags(dms->root) & DM_FLAG_ACTIVATED); + ut_assert(dev_get_flags(uts->root) & DM_FLAG_ACTIVATED); } /* @@ -293,7 +291,6 @@ DM_TEST(dm_test_plat, UT_TESTF_SCAN_PDATA); /* Test that we can bind, probe, remove, unbind a driver */ static int dm_test_lifecycle(struct unit_test_state *uts) { - struct dm_test_state *dms = uts->priv; int op_count[DM_TEST_OP_COUNT]; struct udevice *dev, *test_dev; int pingret; @@ -301,7 +298,7 @@ static int dm_test_lifecycle(struct unit_test
[PATCH v5 24/41] test: Move test running into a separate function
Add a function to handle the preparation for running a test and the post-test clean-up. Signed-off-by: Simon Glass --- (no changes since v1) include/test/ut.h | 16 test/test-main.c | 32 +++- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/include/test/ut.h b/include/test/ut.h index 4e0aba9f700..98f699cbba2 100644 --- a/include/test/ut.h +++ b/include/test/ut.h @@ -387,6 +387,22 @@ int test_pre_run(struct unit_test_state *uts, struct unit_test *test); */ int test_post_run(struct unit_test_state *uts, struct unit_test *test); +/** + * ut_run_test() - Run a single test + * + * This runs the test, handling any preparation and clean-up needed. It prints + * the name of each test before running it. + * + * @uts: Test state to update. The caller should ensure that this is zeroed for + * the first call to this function. On exit, @uts->fail_count is + * incremented by the number of failures (0, one hopes) + * @test: Test to run + * @name: Name of test, possibly skipping a prefix that should not be displayed + * @return 0 if all tests passed, -EAGAIN if the test should be skipped, -1 if + * any failed + */ +int ut_run_test(struct unit_test_state *uts, struct unit_test *test, + const char *name); /** * ut_run_tests() - Run a set of tests diff --git a/test/test-main.c b/test/test-main.c index 3806c2ad89c..dee28d35d82 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -121,6 +121,28 @@ int test_post_run(struct unit_test_state *uts, struct unit_test *test) return 0; } +int ut_run_test(struct unit_test_state *uts, struct unit_test *test, + const char *test_name) +{ + int ret; + + printf("Test: %s\n", test_name); + + ret = test_pre_run(uts, test); + if (ret == -EAGAIN) + return -EAGAIN; + if (ret) + return ret; + + test->func(uts); + + ret = test_post_run(uts, test); + if (ret) + return ret; + + return 0; +} + int ut_run_tests(struct unit_test_state *uts, const char *prefix, struct unit_test *tests, int count, const char *select_name) { @@ -138,20 +160,12 @@ int ut_run_tests(struct unit_test_state *uts, const char *prefix, if (select_name && strcmp(select_name, test_name)) continue; - printf("Test: %s\n", test_name); + ret = ut_run_test(uts, test, test_name); found++; - - ret = test_pre_run(uts, test); if (ret == -EAGAIN) continue; if (ret) return ret; - - test->func(uts); - - ret = test_post_run(uts, test); - if (ret) - return ret; } if (select_name && !found) return -ENOENT; -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 25/41] test: Use ut_run_test() to run driver model tests
Instead of having a separate function for running driver model tests, use the common one. Make the pre/post-run functions private since we don't need these outside of test-main.c Signed-off-by: Simon Glass --- (no changes since v1) include/test/ut.h | 20 test/dm/test-dm.c | 11 +-- test/test-main.c | 26 +++--- 3 files changed, 24 insertions(+), 33 deletions(-) diff --git a/include/test/ut.h b/include/test/ut.h index 98f699cbba2..adef0b7e1cf 100644 --- a/include/test/ut.h +++ b/include/test/ut.h @@ -367,26 +367,6 @@ void ut_unsilence_console(struct unit_test_state *uts); */ void ut_set_skip_delays(struct unit_test_state *uts, bool skip_delays); -/** - * test_pre_run() - Handle any preparation needed to run a test - * - * @uts: Test state - * @test: Test to prepare for - * @return 0 if OK, -EAGAIN to skip this test since some required feature is not - * available, other -ve on error (meaning that testing cannot likely - * continue) - */ -int test_pre_run(struct unit_test_state *uts, struct unit_test *test); - -/** - * test_post_run() - Handle cleaning up after a test - * - * @uts: Test state - * @test: Test to clean up after - * @return 0 if OK, -ve on error (meaning that testing cannot likely continue) - */ -int test_post_run(struct unit_test_state *uts, struct unit_test *test); - /** * ut_run_test() - Run a single test * diff --git a/test/dm/test-dm.c b/test/dm/test-dm.c index df938395bb6..b01123c7409 100644 --- a/test/dm/test-dm.c +++ b/test/dm/test-dm.c @@ -25,17 +25,8 @@ struct unit_test_state global_dm_test_state; static int dm_do_test(struct unit_test_state *uts, struct unit_test *test, bool of_live) { - const char *fname = strrchr(test->file, '/') + 1; - - printf("Test: %s: %s%s\n", test->name, fname, - !of_live ? " (flat tree)" : ""); uts->of_live = of_live; - - ut_assertok(test_pre_run(uts, test)); - - test->func(uts); - - ut_assertok(test_post_run(uts, test)); + ut_assertok(ut_run_test(uts, test, test->name)); return 0; } diff --git a/test/test-main.c b/test/test-main.c index dee28d35d82..32c4d4b1996 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -80,7 +80,16 @@ static int do_autoprobe(struct unit_test_state *uts) return ret; } -int test_pre_run(struct unit_test_state *uts, struct unit_test *test) +/** + * test_pre_run() - Handle any preparation needed to run a test + * + * @uts: Test state + * @test: Test to prepare for + * @return 0 if OK, -EAGAIN to skip this test since some required feature is not + * available, other -ve on error (meaning that testing cannot likely + * continue) + */ +static int test_pre_run(struct unit_test_state *uts, struct unit_test *test) { if (test->flags & UT_TESTF_DM) ut_assertok(dm_test_pre_run(uts)); @@ -112,7 +121,14 @@ int test_pre_run(struct unit_test_state *uts, struct unit_test *test) return 0; } -int test_post_run(struct unit_test_state *uts, struct unit_test *test) +/** + * test_post_run() - Handle cleaning up after a test + * + * @uts: Test state + * @test: Test to clean up after + * @return 0 if OK, -ve on error (meaning that testing cannot likely continue) + */ +static int test_post_run(struct unit_test_state *uts, struct unit_test *test) { ut_unsilence_console(uts); if (test->flags & UT_TESTF_DM) @@ -124,9 +140,13 @@ int test_post_run(struct unit_test_state *uts, struct unit_test *test) int ut_run_test(struct unit_test_state *uts, struct unit_test *test, const char *test_name) { + const char *fname = strrchr(test->file, '/') + 1; + const char *note = ""; int ret; - printf("Test: %s\n", test_name); + if ((test->flags & UT_TESTF_DM) && !uts->of_live) + note = " (flat tree)"; + printf("Test: %s: %s%s\n", test_name, fname, note); ret = test_pre_run(uts, test); if (ret == -EAGAIN) -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 18/41] test: Move console silencing to test_pre_run()
We already have a function for silencing the console during tests. Use this from test_pre_run() and drop this code from the driver model tests. Signed-off-by: Simon Glass --- (no changes since v1) test/dm/test-dm.c | 4 test/test-main.c | 3 ++- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/test/dm/test-dm.c b/test/dm/test-dm.c index d1d83e34782..fdd35f663e4 100644 --- a/test/dm/test-dm.c +++ b/test/dm/test-dm.c @@ -66,7 +66,6 @@ static int dm_test_destroy(struct unit_test_state *uts) static int dm_do_test(struct unit_test_state *uts, struct unit_test *test, bool of_live) { - struct sandbox_state *state = state_get_current(); const char *fname = strrchr(test->file, '/') + 1; printf("Test: %s: %s%s\n", test->name, fname, @@ -75,13 +74,10 @@ static int dm_do_test(struct unit_test_state *uts, struct unit_test *test, ut_assertok(test_pre_run(uts, test)); - if (!state->show_test_output) - gd->flags |= GD_FLG_SILENT; test->func(uts); ut_assertok(test_post_run(uts, test)); - gd->flags &= ~(GD_FLG_SILENT | GD_FLG_RECORD); state_set_skip_delays(false); ut_assertok(dm_test_destroy(uts)); diff --git a/test/test-main.c b/test/test-main.c index db0d82e36c3..e273777b6e2 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -50,13 +50,14 @@ int test_pre_run(struct unit_test_state *uts, struct unit_test *test) return -EAGAIN; } } + ut_silence_console(uts); return 0; } int test_post_run(struct unit_test_state *uts, struct unit_test *test) { - gd->flags &= ~GD_FLG_RECORD; + ut_unsilence_console(uts); return 0; } -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 23/41] test: Move dm_test_destroy() into test-main.c
Move this function into the common test runner and rename it to dm_test_post_run() so that its purpose is clear. Signed-off-by: Simon Glass --- (no changes since v1) test/dm/test-dm.c | 22 -- test/test-main.c | 23 +++ 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/test/dm/test-dm.c b/test/dm/test-dm.c index d601e497522..df938395bb6 100644 --- a/test/dm/test-dm.c +++ b/test/dm/test-dm.c @@ -22,26 +22,6 @@ DECLARE_GLOBAL_DATA_PTR; struct unit_test_state global_dm_test_state; -static int dm_test_destroy(struct unit_test_state *uts) -{ - int id; - - for (id = 0; id < UCLASS_COUNT; id++) { - struct uclass *uc; - - /* -* If the uclass doesn't exist we don't want to create it. So -* check that here before we call uclass_find_device(). -*/ - uc = uclass_find(id); - if (!uc) - continue; - ut_assertok(uclass_destroy(uc)); - } - - return 0; -} - static int dm_do_test(struct unit_test_state *uts, struct unit_test *test, bool of_live) { @@ -57,8 +37,6 @@ static int dm_do_test(struct unit_test_state *uts, struct unit_test *test, ut_assertok(test_post_run(uts, test)); - ut_assertok(dm_test_destroy(uts)); - return 0; } diff --git a/test/test-main.c b/test/test-main.c index 8b0121bdcec..3806c2ad89c 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -10,6 +10,7 @@ #include #include #include +#include #include #include @@ -44,6 +45,26 @@ static int dm_test_pre_run(struct unit_test_state *uts) return 0; } +static int dm_test_post_run(struct unit_test_state *uts) +{ + int id; + + for (id = 0; id < UCLASS_COUNT; id++) { + struct uclass *uc; + + /* +* If the uclass doesn't exist we don't want to create it. So +* check that here before we call uclass_find_device(). +*/ + uc = uclass_find(id); + if (!uc) + continue; + ut_assertok(uclass_destroy(uc)); + } + + return 0; +} + /* Ensure all the test devices are probed */ static int do_autoprobe(struct unit_test_state *uts) { @@ -94,6 +115,8 @@ int test_pre_run(struct unit_test_state *uts, struct unit_test *test) int test_post_run(struct unit_test_state *uts, struct unit_test *test) { ut_unsilence_console(uts); + if (test->flags & UT_TESTF_DM) + ut_assertok(dm_test_post_run(uts)); return 0; } -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 22/41] test: Move dm_test_init() into test-main.c
Move this function into test-main so that all the init is in one place. Rename it so that its purpose is clearer. Signed-off-by: Simon Glass --- (no changes since v1) include/test/ut.h | 9 - test/dm/test-dm.c | 22 -- test/test-main.c | 33 - 3 files changed, 32 insertions(+), 32 deletions(-) diff --git a/include/test/ut.h b/include/test/ut.h index 6e56ca99c31..4e0aba9f700 100644 --- a/include/test/ut.h +++ b/include/test/ut.h @@ -387,15 +387,6 @@ int test_pre_run(struct unit_test_state *uts, struct unit_test *test); */ int test_post_run(struct unit_test_state *uts, struct unit_test *test); -/** - * dm_test_init() - Get ready to run a driver model test - * - * This clears out the driver model data structures. For sandbox it resets the - * state structure. - * - * @uts: Test state - */ -int dm_test_init(struct unit_test_state *uts); /** * ut_run_tests() - Run a set of tests diff --git a/test/dm/test-dm.c b/test/dm/test-dm.c index 15adc53f533..d601e497522 100644 --- a/test/dm/test-dm.c +++ b/test/dm/test-dm.c @@ -12,7 +12,6 @@ #include #include #include -#include #include #include #include @@ -23,27 +22,6 @@ DECLARE_GLOBAL_DATA_PTR; struct unit_test_state global_dm_test_state; -int dm_test_init(struct unit_test_state *uts) -{ - bool of_live = uts->of_live; - - uts->root = NULL; - uts->testdev = NULL; - uts->force_fail_alloc = false; - uts->skip_post_probe = false; - gd->dm_root = NULL; - if (!CONFIG_IS_ENABLED(OF_PLATDATA)) - memset(dm_testdrv_op_count, '\0', sizeof(dm_testdrv_op_count)); - state_reset_for_test(state_get_current()); - - /* Determine whether to make the live tree available */ - gd_set_of_root(of_live ? uts->of_root : NULL); - ut_assertok(dm_init(of_live)); - uts->root = dm_root(); - - return 0; -} - static int dm_test_destroy(struct unit_test_state *uts) { int id; diff --git a/test/test-main.c b/test/test-main.c index f14b7b09f79..8b0121bdcec 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -7,12 +7,43 @@ #include #include #include +#include #include +#include #include #include DECLARE_GLOBAL_DATA_PTR; +/** + * dm_test_pre_run() - Get ready to run a driver model test + * + * This clears out the driver model data structures. For sandbox it resets the + * state structure + * + * @uts: Test state + */ +static int dm_test_pre_run(struct unit_test_state *uts) +{ + bool of_live = uts->of_live; + + uts->root = NULL; + uts->testdev = NULL; + uts->force_fail_alloc = false; + uts->skip_post_probe = false; + gd->dm_root = NULL; + if (!CONFIG_IS_ENABLED(OF_PLATDATA)) + memset(dm_testdrv_op_count, '\0', sizeof(dm_testdrv_op_count)); + state_reset_for_test(state_get_current()); + + /* Determine whether to make the live tree available */ + gd_set_of_root(of_live ? uts->of_root : NULL); + ut_assertok(dm_init(of_live)); + uts->root = dm_root(); + + return 0; +} + /* Ensure all the test devices are probed */ static int do_autoprobe(struct unit_test_state *uts) { @@ -31,7 +62,7 @@ static int do_autoprobe(struct unit_test_state *uts) int test_pre_run(struct unit_test_state *uts, struct unit_test *test) { if (test->flags & UT_TESTF_DM) - ut_assertok(dm_test_init(uts)); + ut_assertok(dm_test_pre_run(uts)); ut_set_skip_delays(uts, false); -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 26/41] test: Drop dm_do_test()
In an effort to make use of a common test runner, use ut_run_test() directly to run driver model tests. Signed-off-by: Simon Glass --- (no changes since v1) test/dm/test-dm.c | 15 --- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/test/dm/test-dm.c b/test/dm/test-dm.c index b01123c7409..de41fc09db0 100644 --- a/test/dm/test-dm.c +++ b/test/dm/test-dm.c @@ -22,15 +22,6 @@ DECLARE_GLOBAL_DATA_PTR; struct unit_test_state global_dm_test_state; -static int dm_do_test(struct unit_test_state *uts, struct unit_test *test, - bool of_live) -{ - uts->of_live = of_live; - ut_assertok(ut_run_test(uts, test, test->name)); - - return 0; -} - /** * dm_test_run_on_flattree() - Check if we should run a test with flat DT * @@ -103,7 +94,8 @@ int dm_test_run(const char *test_name) runs = 0; if (CONFIG_IS_ENABLED(OF_LIVE)) { if (!(test->flags & UT_TESTF_FLAT_TREE)) { - ut_assertok(dm_do_test(uts, test, true)); + uts->of_live = true; + ut_assertok(ut_run_test(uts, test, test->name)); runs++; } } @@ -114,7 +106,8 @@ int dm_test_run(const char *test_name) */ if (!(test->flags & UT_TESTF_LIVE_TREE) && (!runs || dm_test_run_on_flattree(test))) { - ut_assertok(dm_do_test(uts, test, false)); + uts->of_live = false; + ut_assertok(ut_run_test(uts, test, test->name)); runs++; } found++; -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 27/41] test: Add ut_run_test_live_flat() to run tests twice
Driver model tests are generally run twice, once with livetree enable and again with it disabled. Add a function to handle this and call it from the driver model test runner. Make ut_run_test() private since it is not used outside test-main.c now. Signed-off-by: Simon Glass --- (no changes since v2) Changes in v2: - Fix conditions so non-DM SPL tests are actually run include/test/ut.h | 13 + test/dm/test-dm.c | 37 +- test/test-main.c | 67 --- 3 files changed, 73 insertions(+), 44 deletions(-) diff --git a/include/test/ut.h b/include/test/ut.h index adef0b7e1cf..d06bc5089be 100644 --- a/include/test/ut.h +++ b/include/test/ut.h @@ -368,10 +368,13 @@ void ut_unsilence_console(struct unit_test_state *uts); void ut_set_skip_delays(struct unit_test_state *uts, bool skip_delays); /** - * ut_run_test() - Run a single test + * ut_run_test_live_flat() - Run a test with both live and flat tree * - * This runs the test, handling any preparation and clean-up needed. It prints - * the name of each test before running it. + * This calls ut_run_test() with livetree enabled, which is the standard setup + * for runnig tests. Then, for driver model test, it calls it again with + * livetree disabled. This allows checking of flattree being used when OF_LIVE + * is enabled, as is the case in U-Boot proper before relocation, as well as in + * SPL. * * @uts: Test state to update. The caller should ensure that this is zeroed for * the first call to this function. On exit, @uts->fail_count is @@ -381,8 +384,8 @@ void ut_set_skip_delays(struct unit_test_state *uts, bool skip_delays); * @return 0 if all tests passed, -EAGAIN if the test should be skipped, -1 if * any failed */ -int ut_run_test(struct unit_test_state *uts, struct unit_test *test, - const char *name); +int ut_run_test_live_flat(struct unit_test_state *uts, struct unit_test *test, + const char *name); /** * ut_run_tests() - Run a set of tests diff --git a/test/dm/test-dm.c b/test/dm/test-dm.c index de41fc09db0..826b64565e8 100644 --- a/test/dm/test-dm.c +++ b/test/dm/test-dm.c @@ -22,21 +22,6 @@ DECLARE_GLOBAL_DATA_PTR; struct unit_test_state global_dm_test_state; -/** - * dm_test_run_on_flattree() - Check if we should run a test with flat DT - * - * This skips long/slow tests where there is not much value in running a flat - * DT test in addition to a live DT test. - * - * @return true to run the given test on the flat device tree - */ -static bool dm_test_run_on_flattree(struct unit_test *test) -{ - const char *fname = strrchr(test->file, '/') + 1; - - return !strstr(fname, "video") || strstr(test->name, "video_base"); -} - static bool test_matches(const char *test_name, const char *find_name) { if (!find_name) @@ -85,31 +70,11 @@ int dm_test_run(const char *test_name) uts->of_root = gd_of_root(); for (test = tests; test < tests + n_ents; test++) { const char *name = test->name; - int runs; if (!test_matches(name, test_name)) continue; - /* Run with the live tree if possible */ - runs = 0; - if (CONFIG_IS_ENABLED(OF_LIVE)) { - if (!(test->flags & UT_TESTF_FLAT_TREE)) { - uts->of_live = true; - ut_assertok(ut_run_test(uts, test, test->name)); - runs++; - } - } - - /* -* Run with the flat tree if we couldn't run it with live tree, -* or it is a core test. -*/ - if (!(test->flags & UT_TESTF_LIVE_TREE) && - (!runs || dm_test_run_on_flattree(test))) { - uts->of_live = false; - ut_assertok(ut_run_test(uts, test, test->name)); - runs++; - } + ut_assertok(ut_run_test_live_flat(uts, test, test->name)); found++; } diff --git a/test/test-main.c b/test/test-main.c index 32c4d4b1996..4e17c9edb28 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -80,6 +80,24 @@ static int do_autoprobe(struct unit_test_state *uts) return ret; } +/* + * ut_test_run_on_flattree() - Check if we should run a test with flat DT + * + * This skips long/slow tests where there is not much value in running a flat + * DT test in addition to a live DT test. + * + * @return true to run the given test on the flat device tree + */ +static bool ut_test_run_on_flattree(struct unit_test *test) +{ + const char *fname = strrchr(test->file, '/') + 1; + + if (!(test->flags & UT_TESTF_DM)) + return false; + + return !strstr(fname, "video") || strstr(test->name, "video_base"); +} + /** * test_pre_run() -
[PATCH v5 30/41] test: Use return values in dm_test_run()
Update this function to use the return value of ut_run_list() to check for success/failure, so that they are in sync. Also return a command success code so that the caller gets what it expects. Signed-off-by: Simon Glass --- (no changes since v1) test/dm/test-dm.c | 14 ++ 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/test/dm/test-dm.c b/test/dm/test-dm.c index 20af1c13b3c..54e6577b009 100644 --- a/test/dm/test-dm.c +++ b/test/dm/test-dm.c @@ -20,16 +20,13 @@ DECLARE_GLOBAL_DATA_PTR; -struct unit_test_state global_dm_test_state; - int dm_test_run(const char *test_name) { struct unit_test *tests = ll_entry_start(struct unit_test, dm_test); const int n_ents = ll_entry_count(struct unit_test, dm_test); struct unit_test_state uts_s = { .fail_count = 0 }, *uts = &uts_s; struct device_node *of_root; - - uts->fail_count = 0; + int ret; if (!CONFIG_IS_ENABLED(OF_PLATDATA)) { /* @@ -39,22 +36,23 @@ int dm_test_run(const char *test_name) if (!gd->fdt_blob || fdt_next_node(gd->fdt_blob, 0, NULL) < 0) { puts("Please run with test device tree:\n" "./u-boot -d arch/sandbox/dts/test.dtb\n"); - ut_assert(gd->fdt_blob); + return CMD_RET_FAILURE; } } of_root = gd_of_root(); - ut_run_list("driver model", "dm_test_", tests, n_ents, test_name); + ret = ut_run_list("driver model", "dm_test_", tests, n_ents, test_name); /* Put everything back to normal so that sandbox works as expected */ gd_set_of_root(of_root); gd->dm_root = NULL; - ut_assertok(dm_init(CONFIG_IS_ENABLED(OF_LIVE))); + if (dm_init(CONFIG_IS_ENABLED(OF_LIVE))) + return CMD_RET_FAILURE; dm_scan_plat(false); if (!CONFIG_IS_ENABLED(OF_PLATDATA)) dm_scan_fdt(false); - return uts->fail_count ? CMD_RET_FAILURE : 0; + return ret ? CMD_RET_FAILURE : 0; } int do_ut_dm(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 28/41] test: Use a local variable for test state
At present we use a global test state for all driver-model tests. Make use of a local struct like we do with the other tests. To make this work, add functions to get and set this state. When a test starts, the state is set (so it can be used in the test). When a test finishes, the state is unset, so it cannot be used by mistake. Signed-off-by: Simon Glass --- (no changes since v3) Changes in v3: - Use test_set_state() throughout test-main.c instead of direct assignment include/test/ut.h | 14 ++ test/dm/test-dm.c | 4 +--- test/dm/test-driver.c | 10 +- test/dm/test-uclass.c | 7 +-- test/test-main.c | 18 ++ 5 files changed, 47 insertions(+), 6 deletions(-) diff --git a/include/test/ut.h b/include/test/ut.h index d06bc5089be..bed0e6eb5f6 100644 --- a/include/test/ut.h +++ b/include/test/ut.h @@ -367,6 +367,20 @@ void ut_unsilence_console(struct unit_test_state *uts); */ void ut_set_skip_delays(struct unit_test_state *uts, bool skip_delays); +/** + * test_get_state() - Get the active test state + * + * @return the currently active test state, or NULL if none + */ +struct unit_test_state *test_get_state(void); + +/** + * test_set_state() - Set the active test state + * + * @uts: Test state to use as currently active test state, or NULL if none + */ +void test_set_state(struct unit_test_state *uts); + /** * ut_run_test_live_flat() - Run a test with both live and flat tree * diff --git a/test/dm/test-dm.c b/test/dm/test-dm.c index 826b64565e8..cdaf27bf987 100644 --- a/test/dm/test-dm.c +++ b/test/dm/test-dm.c @@ -20,8 +20,6 @@ DECLARE_GLOBAL_DATA_PTR; -struct unit_test_state global_dm_test_state; - static bool test_matches(const char *test_name, const char *find_name) { if (!find_name) @@ -44,7 +42,7 @@ int dm_test_run(const char *test_name) { struct unit_test *tests = ll_entry_start(struct unit_test, dm_test); const int n_ents = ll_entry_count(struct unit_test, dm_test); - struct unit_test_state *uts = &global_dm_test_state; + struct unit_test_state uts_s = { .fail_count = 0 }, *uts = &uts_s; struct unit_test *test; int found; diff --git a/test/dm/test-driver.c b/test/dm/test-driver.c index 63dc9d335ae..02cb974b0f7 100644 --- a/test/dm/test-driver.c +++ b/test/dm/test-driver.c @@ -18,7 +18,6 @@ #include int dm_testdrv_op_count[DM_TEST_OP_COUNT]; -static struct unit_test_state *uts = &global_dm_test_state; static int testdrv_ping(struct udevice *dev, int pingval, int *pingret) { @@ -37,6 +36,8 @@ static const struct test_ops test_ops = { static int test_bind(struct udevice *dev) { + struct unit_test_state *uts = test_get_state(); + /* Private data should not be allocated */ ut_assert(!dev_get_priv(dev)); @@ -46,6 +47,7 @@ static int test_bind(struct udevice *dev) static int test_probe(struct udevice *dev) { + struct unit_test_state *uts = test_get_state(); struct dm_test_priv *priv = dev_get_priv(dev); /* Private data should be allocated */ @@ -58,6 +60,8 @@ static int test_probe(struct udevice *dev) static int test_remove(struct udevice *dev) { + struct unit_test_state *uts = test_get_state(); + /* Private data should still be allocated */ ut_assert(dev_get_priv(dev)); @@ -67,6 +71,8 @@ static int test_remove(struct udevice *dev) static int test_unbind(struct udevice *dev) { + struct unit_test_state *uts = test_get_state(); + /* Private data should not be allocated */ ut_assert(!dev_get_priv(dev)); @@ -116,6 +122,8 @@ static int test_manual_bind(struct udevice *dev) static int test_manual_probe(struct udevice *dev) { + struct unit_test_state *uts = test_get_state(); + dm_testdrv_op_count[DM_TEST_OP_PROBE]++; if (!uts->force_fail_alloc) dev_set_priv(dev, calloc(1, sizeof(struct dm_test_priv))); diff --git a/test/dm/test-uclass.c b/test/dm/test-uclass.c index f4b540c9278..067701734a0 100644 --- a/test/dm/test-uclass.c +++ b/test/dm/test-uclass.c @@ -17,8 +17,6 @@ #include #include -static struct unit_test_state *uts = &global_dm_test_state; - int test_ping(struct udevice *dev, int pingval, int *pingret) { const struct test_ops *ops = device_get_ops(dev); @@ -31,6 +29,7 @@ int test_ping(struct udevice *dev, int pingval, int *pingret) static int test_post_bind(struct udevice *dev) { + struct unit_test_state *uts = test_get_state(); struct dm_test_perdev_uc_pdata *uc_pdata; dm_testdrv_op_count[DM_TEST_OP_POST_BIND]++; @@ -56,6 +55,7 @@ static int test_pre_unbind(struct udevice *dev) static int test_pre_probe(struct udevice *dev) { struct dm_test_uclass_perdev_priv *priv = dev_get_uclass_priv(dev); + struct unit_test_state *uts = test_get_state(); dm_testdrv_op_count[DM_TEST_OP_PRE_PROBE]++; ut_assert(priv); @@ -66,6 +66,7 @@ static int test_pre
[PATCH v5 29/41] test: Run driver-model tests using ut_run_list()
Use this function instead of implementing it separately for driver model. Make ut_run_tests() private since it is only used in test-main.c Signed-off-by: Simon Glass --- (no changes since v2) Changes in v2: - Allow for prefix to be NULL, to match function comment include/test/ut.h | 42 --- test/dm/test-dm.c | 45 +++- test/test-main.c | 87 +-- 3 files changed, 81 insertions(+), 93 deletions(-) diff --git a/include/test/ut.h b/include/test/ut.h index bed0e6eb5f6..fbbba286ee0 100644 --- a/include/test/ut.h +++ b/include/test/ut.h @@ -381,48 +381,6 @@ struct unit_test_state *test_get_state(void); */ void test_set_state(struct unit_test_state *uts); -/** - * ut_run_test_live_flat() - Run a test with both live and flat tree - * - * This calls ut_run_test() with livetree enabled, which is the standard setup - * for runnig tests. Then, for driver model test, it calls it again with - * livetree disabled. This allows checking of flattree being used when OF_LIVE - * is enabled, as is the case in U-Boot proper before relocation, as well as in - * SPL. - * - * @uts: Test state to update. The caller should ensure that this is zeroed for - * the first call to this function. On exit, @uts->fail_count is - * incremented by the number of failures (0, one hopes) - * @test: Test to run - * @name: Name of test, possibly skipping a prefix that should not be displayed - * @return 0 if all tests passed, -EAGAIN if the test should be skipped, -1 if - * any failed - */ -int ut_run_test_live_flat(struct unit_test_state *uts, struct unit_test *test, - const char *name); - -/** - * ut_run_tests() - Run a set of tests - * - * This runs the tests, handling any preparation and clean-up needed. It prints - * the name of each test before running it. - * - * @uts: Test state to update. The caller should ensure that this is zeroed for - * the first call to this function. On exit, @uts->fail_count is - * incremented by the number of failures (0, one hopes) - * @prefix: String prefix for the tests. Any tests that have this prefix will be - * printed without the prefix, so that it is easier to see the unique part - * of the test name. If NULL, no prefix processing is done - * @tests: List of tests to run - * @count: Number of tests to run - * @select_name: Name of a single test to run (from the list provided). If NULL - * then all tests are run - * @return 0 if all tests passed, -ENOENT if test @select_name was not found, - * -EBADF if any failed - */ -int ut_run_tests(struct unit_test_state *uts, const char *prefix, -struct unit_test *tests, int count, const char *select_name); - /** * ut_run_tests() - Run a set of tests * diff --git a/test/dm/test-dm.c b/test/dm/test-dm.c index cdaf27bf987..20af1c13b3c 100644 --- a/test/dm/test-dm.c +++ b/test/dm/test-dm.c @@ -20,31 +20,14 @@ DECLARE_GLOBAL_DATA_PTR; -static bool test_matches(const char *test_name, const char *find_name) -{ - if (!find_name) - return true; - - if (!strcmp(test_name, find_name)) - return true; - - /* All tests have this prefix */ - if (!strncmp(test_name, "dm_test_", 8)) - test_name += 8; - - if (!strcmp(test_name, find_name)) - return true; - - return false; -} +struct unit_test_state global_dm_test_state; int dm_test_run(const char *test_name) { struct unit_test *tests = ll_entry_start(struct unit_test, dm_test); const int n_ents = ll_entry_count(struct unit_test, dm_test); struct unit_test_state uts_s = { .fail_count = 0 }, *uts = &uts_s; - struct unit_test *test; - int found; + struct device_node *of_root; uts->fail_count = 0; @@ -60,29 +43,11 @@ int dm_test_run(const char *test_name) } } - if (!test_name) - printf("Running %d driver model tests\n", n_ents); - else - - found = 0; - uts->of_root = gd_of_root(); - for (test = tests; test < tests + n_ents; test++) { - const char *name = test->name; - - if (!test_matches(name, test_name)) - continue; - - ut_assertok(ut_run_test_live_flat(uts, test, test->name)); - found++; - } - - if (test_name && !found) - printf("Test '%s' not found\n", test_name); - else - printf("Failures: %d\n", uts->fail_count); + of_root = gd_of_root(); + ut_run_list("driver model", "dm_test_", tests, n_ents, test_name); /* Put everything back to normal so that sandbox works as expected */ - gd_set_of_root(uts->of_root); + gd_set_of_root(of_root); gd->dm_root = NULL; ut_assertok(dm_init(CONFIG_IS_ENABLED(OF_LIVE))); dm_scan_plat(false); diff --git a/test/test-main.c b/test/t
[PATCH v5 32/41] test: Move restoring of driver model state to ut_run_list()
Add this functionality to ut_run_list() so it can be removed from dm_test_run(). At this point all tests are run through ut_run_list(). Signed-off-by: Simon Glass --- (no changes since v1) test/dm/test-dm.c | 11 --- test/test-main.c | 30 +- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/test/dm/test-dm.c b/test/dm/test-dm.c index 8cb99ed80cc..cb4f99537df 100644 --- a/test/dm/test-dm.c +++ b/test/dm/test-dm.c @@ -24,21 +24,10 @@ int dm_test_run(const char *test_name) { struct unit_test *tests = ll_entry_start(struct unit_test, dm_test); const int n_ents = ll_entry_count(struct unit_test, dm_test); - struct device_node *of_root; int ret; - of_root = gd_of_root(); ret = ut_run_list("driver model", "dm_test_", tests, n_ents, test_name); - /* Put everything back to normal so that sandbox works as expected */ - gd_set_of_root(of_root); - gd->dm_root = NULL; - if (dm_init(CONFIG_IS_ENABLED(OF_LIVE))) - return CMD_RET_FAILURE; - dm_scan_plat(false); - if (!CONFIG_IS_ENABLED(OF_PLATDATA)) - dm_scan_fdt(false); - return ret ? CMD_RET_FAILURE : 0; } diff --git a/test/test-main.c b/test/test-main.c index 8138fb43875..6edd49f0b61 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -143,7 +143,7 @@ static bool test_matches(const char *prefix, const char *test_name, return false; } -/* +/** * ut_list_has_dm_tests() - Check if a list of tests has driver model ones * * @tests: List of tests to run @@ -162,6 +162,28 @@ static bool ut_list_has_dm_tests(struct unit_test *tests, int count) return false; } +/** + * dm_test_restore() Put things back to normal so sandbox works as expected + * + * @of_root: Value to set for of_root + * @return 0 if OK, -ve on error + */ +static int dm_test_restore(struct device_node *of_root) +{ + int ret; + + gd_set_of_root(of_root); + gd->dm_root = NULL; + ret = dm_init(CONFIG_IS_ENABLED(OF_LIVE)); + if (ret) + return ret; + dm_scan_plat(false); + if (!CONFIG_IS_ENABLED(OF_PLATDATA)) + dm_scan_fdt(false); + + return 0; +} + /** * test_pre_run() - Handle any preparation needed to run a test * @@ -359,10 +381,12 @@ int ut_run_list(const char *category, const char *prefix, struct unit_test *tests, int count, const char *select_name) { struct unit_test_state uts = { .fail_count = 0 }; + bool has_dm_tests = false; int ret; if (!CONFIG_IS_ENABLED(OF_PLATDATA) && ut_list_has_dm_tests(tests, count)) { + has_dm_tests = true; /* * If we have no device tree, or it only has a root node, then * these * tests clearly aren't going to work... @@ -385,5 +409,9 @@ int ut_run_list(const char *category, const char *prefix, else printf("Failures: %d\n", uts.fail_count); + /* Best efforts only...ignore errors */ + if (has_dm_tests) + dm_test_restore(uts.of_root); + return ret; } -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 31/41] test: Move the devicetree check into ut_run_list()
Add a check to ut_run_list() as to whether a list has driver model tests. Move the logic for the test devicetree into that function, in an effort to eventually remove all logic from dm_test_run(). Signed-off-by: Simon Glass --- (no changes since v1) test/dm/test-dm.c | 13 - test/test-main.c | 32 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/test/dm/test-dm.c b/test/dm/test-dm.c index 54e6577b009..8cb99ed80cc 100644 --- a/test/dm/test-dm.c +++ b/test/dm/test-dm.c @@ -24,22 +24,9 @@ int dm_test_run(const char *test_name) { struct unit_test *tests = ll_entry_start(struct unit_test, dm_test); const int n_ents = ll_entry_count(struct unit_test, dm_test); - struct unit_test_state uts_s = { .fail_count = 0 }, *uts = &uts_s; struct device_node *of_root; int ret; - if (!CONFIG_IS_ENABLED(OF_PLATDATA)) { - /* -* If we have no device tree, or it only has a root node, then -* these * tests clearly aren't going to work... -*/ - if (!gd->fdt_blob || fdt_next_node(gd->fdt_blob, 0, NULL) < 0) { - puts("Please run with test device tree:\n" -"./u-boot -d arch/sandbox/dts/test.dtb\n"); - return CMD_RET_FAILURE; - } - } - of_root = gd_of_root(); ret = ut_run_list("driver model", "dm_test_", tests, n_ents, test_name); diff --git a/test/test-main.c b/test/test-main.c index 16c0d13ea55..8138fb43875 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -143,6 +143,25 @@ static bool test_matches(const char *prefix, const char *test_name, return false; } +/* + * ut_list_has_dm_tests() - Check if a list of tests has driver model ones + * + * @tests: List of tests to run + * @count: Number of tests to ru + * @return true if any of the tests have the UT_TESTF_DM flag + */ +static bool ut_list_has_dm_tests(struct unit_test *tests, int count) +{ + struct unit_test *test; + + for (test = tests; test < tests + count; test++) { + if (test->flags & UT_TESTF_DM) + return true; + } + + return false; +} + /** * test_pre_run() - Handle any preparation needed to run a test * @@ -342,6 +361,19 @@ int ut_run_list(const char *category, const char *prefix, struct unit_test_state uts = { .fail_count = 0 }; int ret; + if (!CONFIG_IS_ENABLED(OF_PLATDATA) && + ut_list_has_dm_tests(tests, count)) { + /* +* If we have no device tree, or it only has a root node, then +* these * tests clearly aren't going to work... +*/ + if (!gd->fdt_blob || fdt_next_node(gd->fdt_blob, 0, NULL) < 0) { + puts("Please run with test device tree:\n" +"./u-boot -d arch/sandbox/dts/test.dtb\n"); + return CMD_RET_FAILURE; + } + } + if (!select_name) printf("Running %d %s tests\n", count, category); -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 33/41] test: log: Rename log main test file to log_ut.c
The current name is the same as the main test runner file. Rename it to avoid confusion. Signed-off-by: Simon Glass Reviewed-by: Heinrich Schuchardt --- (no changes since v1) test/log/Makefile | 2 +- test/log/{test-main.c => log_ut.c} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename test/log/{test-main.c => log_ut.c} (100%) diff --git a/test/log/Makefile b/test/log/Makefile index 3f09deb644b..a3dedace043 100644 --- a/test/log/Makefile +++ b/test/log/Makefile @@ -7,7 +7,7 @@ obj-$(CONFIG_CMD_LOG) += log_filter.o ifdef CONFIG_UT_LOG -obj-y += test-main.o +obj-y += log_ut.o ifdef CONFIG_SANDBOX obj-$(CONFIG_LOG_SYSLOG) += syslog_test.o diff --git a/test/log/test-main.c b/test/log/log_ut.c similarity index 100% rename from test/log/test-main.c rename to test/log/log_ut.c -- 2.30.1.766.gb4fecdf3b7-goog
[PATCH v5 36/41] test: Allow SPL to run any available test
At present SPL only runs driver model tests. Update it to run all available tests, i.e. in any test suite. Signed-off-by: Simon Glass --- (no changes since v1) arch/sandbox/cpu/spl.c | 7 +-- include/test/test.h| 16 +--- test/dm/test-dm.c | 11 ++- test/test-main.c | 17 + 4 files changed, 33 insertions(+), 18 deletions(-) diff --git a/arch/sandbox/cpu/spl.c b/arch/sandbox/cpu/spl.c index 3779d58c3fe..ce5aae87fb6 100644 --- a/arch/sandbox/cpu/spl.c +++ b/arch/sandbox/cpu/spl.c @@ -13,7 +13,7 @@ #include #include #include -#include +#include DECLARE_GLOBAL_DATA_PTR; @@ -63,9 +63,12 @@ void spl_board_init(void) preloader_console_init(); if (state->run_unittests) { + struct unit_test *tests = UNIT_TEST_ALL_START(); + const int count = UNIT_TEST_ALL_COUNT(); int ret; - ret = dm_test_run(state->select_unittests); + ret = ut_run_list("spl", NULL, tests, count, + state->select_unittests); /* continue execution into U-Boot */ } } diff --git a/include/test/test.h b/include/test/test.h index 3330dcc72d3..0b124edd601 100644 --- a/include/test/test.h +++ b/include/test/test.h @@ -99,6 +99,11 @@ struct unit_test { #define UNIT_TEST_SUITE_COUNT(_suite) \ ll_entry_count(struct unit_test, ut_ ## _suite) +/* Use ! and ~ so that all tests will be sorted between these two values */ +#define UNIT_TEST_ALL_START() ll_entry_start(struct unit_test, ut_!) +#define UNIT_TEST_ALL_END()ll_entry_start(struct unit_test, ut_~) +#define UNIT_TEST_ALL_COUNT() (UNIT_TEST_ALL_END() - UNIT_TEST_ALL_START()) + /* Sizes for devres tests */ enum { TEST_DEVRES_SIZE= 100, @@ -119,15 +124,4 @@ enum { */ struct udevice *testbus_get_clear_removed(void); -/** - * dm_test_run() - Run driver model tests - * - * Run all the available driver model tests, or a selection - * - * @test_name: Name of single test to run (e.g. "dm_test_fdt_pre_reloc" or just - * "fdt_pre_reloc"), or NULL to run all - * @return 0 if all tests passed, 1 if not - */ -int dm_test_run(const char *test_name); - #endif /* __TEST_TEST_H */ diff --git a/test/dm/test-dm.c b/test/dm/test-dm.c index 729becf57b2..9ba2ba23d5d 100644 --- a/test/dm/test-dm.c +++ b/test/dm/test-dm.c @@ -20,7 +20,16 @@ DECLARE_GLOBAL_DATA_PTR; -int dm_test_run(const char *test_name) +/** + * dm_test_run() - Run driver model tests + * + * Run all the available driver model tests, or a selection + * + * @test_name: Name of single test to run (e.g. "dm_test_fdt_pre_reloc" or just + * "fdt_pre_reloc"), or NULL to run all + * @return 0 if all tests passed, 1 if not + */ +static int dm_test_run(const char *test_name) { struct unit_test *tests = UNIT_TEST_SUITE_START(dm_test); const int n_ents = UNIT_TEST_SUITE_COUNT(dm_test); diff --git a/test/test-main.c b/test/test-main.c index 6edd49f0b61..e1b49e091ab 100644 --- a/test/test-main.c +++ b/test/test-main.c @@ -119,7 +119,8 @@ static bool ut_test_run_on_flattree(struct unit_test *test) * * @prefix: String prefix for the tests. Any tests that have this prefix will be * printed without the prefix, so that it is easier to see the unique part - * of the test name. If NULL, no prefix processing is done + * of the test name. If NULL, any suite name (xxx_test) is considered to be + * a prefix. * @test_name: Name of current test * @select_name: Name of test to run (or NULL for all) * @return true to run this test, false to skip it @@ -133,9 +134,17 @@ static bool test_matches(const char *prefix, const char *test_name, if (!strcmp(test_name, select_name)) return true; - /* All tests have this prefix */ - if (prefix && !strncmp(test_name, prefix, strlen(prefix))) - test_name += strlen(prefix); + if (!prefix) { + const char *p = strstr(test_name, "_test_"); + + /* convert xxx_test_yyy to yyy, i.e. remove the suite name */ + if (p) + test_name = p + 6; + } else { + /* All tests have this prefix */ + if (!strncmp(test_name, prefix, strlen(prefix))) + test_name += strlen(prefix); + } if (!strcmp(test_name, select_name)) return true; -- 2.30.1.766.gb4fecdf3b7-goog