[PATCH 3/3] ram: ast2600: Align the RL and WL setting
Use macro to represent the RL and WL setting to ensure the PHY and
controller setting are aligned.
Signed-off-by: Dylan Hung
---
arch/arm/include/asm/arch-aspeed/sdram_ast2600.h | 4
drivers/ram/aspeed/sdram_ast2600.c | 9 +
2 files changed, 13 insertions(+)
diff --git a/arch/arm/include/asm/arch-aspeed/sdram_ast2600.h
b/arch/arm/include/asm/arch-aspeed/sdram_ast2600.h
index d2408c0020f8..b0a91ae40d44 100644
--- a/arch/arm/include/asm/arch-aspeed/sdram_ast2600.h
+++ b/arch/arm/include/asm/arch-aspeed/sdram_ast2600.h
@@ -104,6 +104,10 @@
#define SDRAM_FORCE_PRECHARGE_EN BIT(4)
#define SDRAM_REFRESH_EN BIT(0)
+/* MCR14 */
+#define SDRAM_WL_SETTING GENMASK(23, 20)
+#define SDRAM_CL_SETTING GENMASK(19, 16)
+
#define SDRAM_TEST_LEN_SHIFT 4
#define SDRAM_TEST_LEN_MASK0xf
#define SDRAM_TEST_START_ADDR_SHIFT24
diff --git a/drivers/ram/aspeed/sdram_ast2600.c
b/drivers/ram/aspeed/sdram_ast2600.c
index bda02d062900..5d426088be3e 100644
--- a/drivers/ram/aspeed/sdram_ast2600.c
+++ b/drivers/ram/aspeed/sdram_ast2600.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#define DDR_PHY_TBL_CHG_ADDR0xaeeddeea
@@ -935,6 +936,7 @@ static void ast2600_sdrammc_lock(struct dram_info *info)
static void ast2600_sdrammc_common_init(struct ast2600_sdrammc_regs *regs)
{
int i;
+ u32 reg;
writel(MCR34_MREQI_DIS | MCR34_RESETN_DIS, >power_ctrl);
writel(SDRAM_VIDEO_UNLOCK_KEY, >gm_protection_key);
@@ -969,6 +971,13 @@ static void ast2600_sdrammc_common_init(struct
ast2600_sdrammc_regs *regs)
for (i = 0; i < ARRAY_SIZE(ddr4_ac_timing); ++i)
writel(ddr4_ac_timing[i], >ac_timing[i]);
+ /* update CL and WL */
+ reg = readl(>ac_timing[1]);
+ reg &= ~(SDRAM_WL_SETTING | SDRAM_CL_SETTING);
+ reg |= FIELD_PREP(SDRAM_WL_SETTING, CONFIG_WL - 5) |
+ FIELD_PREP(SDRAM_CL_SETTING, CONFIG_RL - 5);
+ writel(reg, >ac_timing[1]);
+
writel(DDR4_MR01_MODE, >mr01_mode_setting);
writel(DDR4_MR23_MODE, >mr23_mode_setting);
writel(DDR4_MR45_MODE, >mr45_mode_setting);
--
2.25.1
[PATCH 1/3] ram: ast2600: Fix incorrect statement of the register polling
The condition "~data" in the if-statement is a typo. The original
intention is to poll if SDRAM_PHYCTRL0_INIT bit equals to 0. So use
"data == 0" for instead.
Besides, the bit[1] of "phy_status" register is hardwired to
SDRAM_PHYCTRL0_INIT (with inverse logic). Since SDRAM_PHYCTRL0_INIT has
already done, remove the unnecessary checking of phy_status[1].
Fixes: fde93143469f ("ram: aspeed: Add AST2600 DRAM control support")
Signed-off-by: Dylan Hung
---
drivers/ram/aspeed/sdram_ast2600.c | 7 +--
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/ram/aspeed/sdram_ast2600.c
b/drivers/ram/aspeed/sdram_ast2600.c
index 9ad398d24155..b09232a30413 100644
--- a/drivers/ram/aspeed/sdram_ast2600.c
+++ b/drivers/ram/aspeed/sdram_ast2600.c
@@ -449,7 +449,7 @@ static void ast2600_sdramphy_kick_training(struct dram_info
*info)
while (1) {
data = readl(>phy_ctrl[0]) & SDRAM_PHYCTRL0_INIT;
- if (~data)
+ if (data == 0)
break;
}
}
@@ -984,11 +984,6 @@ static int ast2600_sdrammc_probe(struct udevice *dev)
L_ast2600_sdramphy_train:
ast2600_sdrammc_init_ddr4(priv);
- /* make sure DDR-PHY is ready before access */
- do {
- reg = readl(priv->phy_status) & BIT(1);
- } while (reg == 0);
-
if (ast2600_sdramphy_check_status(priv) != 0) {
printf("DDR4 PHY training fail, retrain\n");
goto L_ast2600_sdramphy_train;
--
2.25.1
[PATCH 2/3] ram: ast2600: Improve ddr4 timing and signal quality
Adjust the following settings to get better timing and signal quality.
1. write DQS/DQ delay
- 1e6e2304[0]
- 1e6e2304[15:8]
2. read DQS/DQ delay
- 0x1e6e0298[0]
- 0x1e6e0298[15:8]
3. CLK/CA timing
- 0x1e6e01a8[31]
4. Read and write termination
- change RTT_ROM from 40 ohm to 48 ohm (MR1[10:8])
- change RTT_PARK from disable to 48 ohm (MR5[8:6])
- change RTT_WR from 120 ohm to disable (MR2[11:9])
- change PHY ODT from 40 ohm to 80 ohm (0x1e6e0130[10:8])
Note1: Both DDR-PHY and DDR controller have their own registers for DDR4
Mode Registers (MR0~MR6). This patch introduces macros to synchronize
the MR value on both sides.
Note2: the waveform meansurement can be found in item #21 of Aspeed
AST26x0 Application note (AP note).
Signed-off-by: Dylan Hung
---
drivers/ram/aspeed/sdram_ast2600.c | 163 -
1 file changed, 138 insertions(+), 25 deletions(-)
diff --git a/drivers/ram/aspeed/sdram_ast2600.c
b/drivers/ram/aspeed/sdram_ast2600.c
index b09232a30413..bda02d062900 100644
--- a/drivers/ram/aspeed/sdram_ast2600.c
+++ b/drivers/ram/aspeed/sdram_ast2600.c
@@ -20,6 +20,119 @@
#define DDR_PHY_TBL_CHG_ADDR0xaeeddeea
#define DDR_PHY_TBL_END 0xaeededed
+/**
+ * phyr030[18:16] - Ron PU (PHY side)
+ * phyr030[14:12] - Ron PD (PHY side)
+ * b'000 : disable
+ * b'001 : 240 ohm
+ * b'010 : 120 ohm
+ * b'011 : 80 ohm
+ * b'100 : 60 ohm
+ * b'101 : 48 ohm
+ * b'110 : 40 ohm
+ * b'111 : 34 ohm (default)
+ */
+#define PHY_RON((0x7 << 16) | (0x7 << 12))
+
+/**
+ * phyr030[10:8] - ODT configuration (PHY side)
+ * b'000 : ODT disabled
+ * b'001 : 240 ohm
+ * b'010 : 120 ohm
+ * b'011 : 80 ohm (default)
+ * b'100 : 60 ohm
+ * b'101 : 48 ohm
+ * b'110 : 40 ohm
+ * b'111 : 34 ohm
+ */
+#define PHY_ODT(0x3 << 8)
+
+/**
+ * MR1[2:1] output driver impedance
+ * b'00 : 34 ohm (default)
+ * b'01 : 48 ohm
+ */
+#define DRAM_RON (0x0 << 1)
+
+/**
+ * DRAM ODT - synchronous ODT mode
+ * RTT_WR: disable
+ * RTT_NOM = RTT_PARK
+ *
+ * MR1[10:8] RTT_NOM
+ * b'000 : RTT_NOM disable
+ * b'001 : 60 ohm
+ * b'010 : 120 ohm
+ * b'011 : 40 ohm
+ * b'100 : 240 ohm
+ * b'101 : 48 ohm (default)
+ * b'110 : 80 ohm
+ * b'111 : 34 ohm
+ *
+ * MR5[8:6] RTT_PARK
+ * b'000 : RTT_PARK disable
+ * b'001 : 60 ohm
+ * b'010 : 120 ohm
+ * b'011 : 40 ohm
+ * b'100 : 240 ohm
+ * b'101 : 48 ohm (default)
+ * b'110 : 80 ohm
+ * b'111 : 34 ohm
+ *
+ * MR2[11:9] RTT_WR
+ * b'000 : Dynamic ODT off (default)
+ * b'001 : 120 ohm
+ * b'010 : 240 ohm
+ * b'011 : Hi-Z
+ * b'100 : 80 ohm
+ */
+#define RTT_WR (0x0 << 9)
+#define RTT_NOM(0x5 << 8)
+#define RTT_PARK (0x5 << 6)
+
+/**
+ * MR6[6] VrefDQ training range
+ * b'0 : range 1
+ * b'1 : range 2 (default)
+ */
+#define VREFDQ_RANGE_2 BIT(6)
+
+/**
+ * Latency setting:
+ * AL = PL = 0 (hardware fixed setting)
+ * -> WL = AL + CWL + PL = CWL
+ * -> RL = AL + CL + PL = CL
+ */
+#define CONFIG_WL 9
+#define CONFIG_RL 12
+#define T_RDDATA_EN((CONFIG_RL - 2) << 8)
+#define T_PHY_WRLAT(CONFIG_WL - 2)
+
+/* MR0 */
+#define MR0_CL_12 (BIT(4) | BIT(2))
+#define MR0_WR12_RTP6 BIT(9)
+#define MR0_DLL_RESET BIT(8)
+#define MR0_VAL(MR0_CL_12 | MR0_WR12_RTP6 |
MR0_DLL_RESET)
+
+/* MR1 */
+#define MR1_VAL(0x0001 | RTT_NOM | DRAM_RON)
+
+/* MR2 */
+#define MR2_CWL_9 0
+#define MR2_VAL(0x | RTT_WR | MR2_CWL_9)
+
+/* MR3 ~ MR6 */
+#define MR3_VAL0x
+#define MR4_VAL0x
+#define MR5_VAL(0x0400 | RTT_PARK)
+#define MR6_VAL0x0400
+
+/**
+ * The offset value applied to the DDR PHY write data eye training result
+ * to fine-tune the write DQ/DQS alignment
+ */
+#define WR_DATA_EYE_OFFSET (0x10 << 8)
+
#if defined(CONFIG_ASPEED_DDR4_800)
u32 ast2600_sdramphy_config[165] = {
0x1e6e0100, // start address
@@ -35,7 +148,7 @@ u32 ast2600_sdramphy_config[165] = {
0x2000, // phyr024
0x0008, // phyr028
0x, // phyr02c
- 0x00077600, // phyr030
+ (PHY_RON | PHY_ODT),/* phyr030 */
0x, // phyr034
0x, // phyr038
0x2000, // phyr03c
@@ -44,18 +157,18 @@ u32 ast2600_sdramphy_config[165] = {
0x2f07, // phyr048
0x3080, // phyr04c
0x0400, // phyr050
- 0x0200, // phyr054
- 0x03140201, // phyr058
- 0x0480, //
[PATCH 0/3] Improve AST26x0 DDR4 timing and signal quality
This patch series fine-tunes the read & write DQS/DQ timing, CLK/CA timing and termination (RTT_NOM, RTT_PARK and RTT_WR) for Aspeed AST26x0 SOC to get better signal quality and hence improve the stability. Also, a typing error of the DDR-PHY status polling is fixed. Dylan Hung (3): ram: ast2600: Fix incorrect statement of the register polling ram: ast2600: Improve ddr4 timing and signal quality ram: ast2600: Align the RL and WL setting .../include/asm/arch-aspeed/sdram_ast2600.h | 4 + drivers/ram/aspeed/sdram_ast2600.c| 179 +++--- 2 files changed, 152 insertions(+), 31 deletions(-) -- 2.25.1
Coverity report for EFI subsystem
Hi Tom, Heinirch forwarded me a coverity report a few days ago regarding some changes in the EFI subsystem and specifically the protocol management functions. The issues reported were: - CID 376213 - CID 376206 - CID 376203 - CID 376200 - CID 376199 - CID 376195 The last one has been already fixed here [0] and you should get a pull request soon. The rest should be marked as false positives. We are initializing the va_args outside the reported functions and I think coverity gets confused. If someone thinks otherwise please shout [0] https://source.denx.de/u-boot/custodians/u-boot-efi/-/commit/1c1c275363242c81442e0eba523d532dbe03ac45 Regards /Ilias
Re: [PATCH 04/22] sunxi: Hide image type selection if SPL is disabled
On Tue, 1 Nov 2022 00:08:16 -0500 Samuel Holland wrote: > This choice is meaningless when SPL is disabled. Hide it to avoid any > possible confusion. > > Signed-off-by: Samuel Holland Reviewed-by: Andre Przywara Cheers, Andre > --- > > board/sunxi/Kconfig | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/board/sunxi/Kconfig b/board/sunxi/Kconfig > index 084a8b0c6c..42f61df5c5 100644 > --- a/board/sunxi/Kconfig > +++ b/board/sunxi/Kconfig > @@ -1,5 +1,6 @@ > choice > prompt "SPL Image Type" > + depends on SPL > default SPL_IMAGE_TYPE_SUNXI_EGON > > config SPL_IMAGE_TYPE_SUNXI_EGON
Re: [PATCH 03/22] sunxi: Add missing dependencies to Kconfig selections
On Tue, 1 Nov 2022 00:08:15 -0500 Samuel Holland wrote: > Some of the selected symbols have a user-visible dependency. Make the > selections conditional on that dependency to avoid creating invalid > configurations. No change in any of the generated configs, so looks fine: > Signed-off-by: Samuel Holland Reviewed-by: Andre Przywara Thanks, Andre > --- > > arch/arm/Kconfig | 16 > 1 file changed, 8 insertions(+), 8 deletions(-) > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 939f76867e..4e5daa9e7f 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -1134,30 +1134,30 @@ config ARCH_SOCFPGA > config ARCH_SUNXI > bool "Support sunxi (Allwinner) SoCs" > select BINMAN > - select CMD_GPIO > + select CMD_GPIO if GPIO > select CMD_MMC if MMC > select CMD_USB if DISTRO_DEFAULTS && USB_HOST > select CLK > select DM > - select DM_ETH > - select DM_GPIO > + select DM_ETH if NET > + select DM_GPIO if GPIO > select DM_I2C if I2C > + select DM_SCSI if BLK && SCSI > + select DM_SERIAL if SERIAL > select DM_SPI if SPI > select DM_SPI_FLASH if SPI > - select DM_SCSI if SCSI > - select DM_SERIAL > select GPIO_EXTRA_HEADER > select OF_BOARD_SETUP > select OF_CONTROL > select OF_SEPARATE > select PINCTRL > - select SPECIFY_CONSOLE_INDEX > + select SPECIFY_CONSOLE_INDEX if SERIAL > select SPL_SEPARATE_BSS if SPL > select SPL_STACK_R if SPL > select SPL_SYS_MALLOC_SIMPLE if SPL > select SPL_SYS_THUMB_BUILD if !ARM64 > - select SUNXI_GPIO > - select SYS_NS16550 > + select SUNXI_GPIO if GPIO > + select SYS_NS16550 if SERIAL > select SYS_THUMB_BUILD if !ARM64 > select USB if DISTRO_DEFAULTS > select USB_KEYBOARD if DISTRO_DEFAULTS && USB_HOST
Re: [PATCH 02/22] sunxi: Remove unnecessary Kconfig selections
On Tue, 1 Nov 2022 00:08:14 -0500 Samuel Holland wrote: > Two of these selections are redundant and have no effect: > - DM_KEYBOARD is selected by USB_KEYBOARD > - DM_MMC is selected by MMC > > This selection has no effect by default and is unnecessarily strong: > - USB_STORAGE is implied by DISTRO_DEFAULTS With patch 01/22 removed (for now), this disables DM_KEYBOARD on 15 boards, it looks like for those without USB. I wouldn't be aware of any other keyboard type supported on Allwinner boards, so the change is fine: > Signed-off-by: Samuel Holland Reviewed-by: Andre Przywara Cheers, Andre > --- > > arch/arm/Kconfig | 3 --- > 1 file changed, 3 deletions(-) > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index d3a1f03b36..939f76867e 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -1144,8 +1144,6 @@ config ARCH_SUNXI > select DM_I2C if I2C > select DM_SPI if SPI > select DM_SPI_FLASH if SPI > - select DM_KEYBOARD > - select DM_MMC if MMC > select DM_SCSI if SCSI > select DM_SERIAL > select GPIO_EXTRA_HEADER > @@ -1163,7 +1161,6 @@ config ARCH_SUNXI > select SYS_THUMB_BUILD if !ARM64 > select USB if DISTRO_DEFAULTS > select USB_KEYBOARD if DISTRO_DEFAULTS && USB_HOST > - select USB_STORAGE if DISTRO_DEFAULTS && USB_HOST > select SPL_USE_TINY_PRINTF > select USE_PREBOOT > select SYS_RELOC_GD_ENV_ADDR
Re: [PATCH 01/22] sunxi: Fix default-enablement of USB host drivers
On Tue, 1 Nov 2022 00:08:13 -0500 Samuel Holland wrote: Hi Samuel, > We tried to enable USB_EHCI_GENERIC and USB_OHCI_GENERIC by default. Well, I am not really sure that was the real intention, looking at commit 29d280c88a1ff3, and the fact that those symbols are still explicitly defined in many board config files. My main concern is that this change enables USB on many boards which had it disabled - at least some of them probably on purpose. I believe for the Pinephone for instance it's off to decrease boot time. > This did not work because those symbols depend on USB_EHCI_HCD and > USB_OHCI_HCD, which were not enabled. Fix this by implying all four. The change itself looks correct, but as mentioned, 47 out of 161 boards now come with USB newly enabled. 25 of those don't have EHCI or OHCI nodes in their DT, so enabling USB doesn't make much sense there. For the others I don't know if USB was disabled on purpose, on some tablets for instance it seems that the only USB HCI port is connected to an on-board WiFi chip, which we don't support in U-Boot. So in a first round I would like to skip this patch. If we want to have it (I am not against it, since the majority of boards have USB), this should be paired with: # USB_EHCI_HCD is not set # USB_OHCI_HCD is not set in the defconfigs at least for those boards without HCI DT nodes (I have a list), to keep it disabled there. If someone cares, and it has been disabled by mistake, they can fix that with an extra patch. For the other boards we should remove the ?HCI symbols from their defconfigs, since they would be redundant then. Cheers, Andre > Signed-off-by: Samuel Holland > --- > > arch/arm/Kconfig | 4 > drivers/usb/host/Kconfig | 2 -- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig > index 710f171f87..d3a1f03b36 100644 > --- a/arch/arm/Kconfig > +++ b/arch/arm/Kconfig > @@ -1185,7 +1185,11 @@ config ARCH_SUNXI > imply SYSRESET > imply SYSRESET_WATCHDOG > imply SYSRESET_WATCHDOG_AUTO > + imply USB_EHCI_GENERIC > + imply USB_EHCI_HCD > imply USB_GADGET > + imply USB_OHCI_GENERIC > + imply USB_OHCI_HCD > imply WDT > > config ARCH_U8500 > diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig > index 1aabe062fb..a4d62bc9e8 100644 > --- a/drivers/usb/host/Kconfig > +++ b/drivers/usb/host/Kconfig > @@ -280,7 +280,6 @@ config USB_EHCI_ZYNQ > config USB_EHCI_GENERIC > bool "Support for generic EHCI USB controller" > depends on DM_USB > - default ARCH_SUNXI > ---help--- > Enables support for generic EHCI controller. > > @@ -342,7 +341,6 @@ config USB_OHCI_PCI > > config USB_OHCI_GENERIC > bool "Support for generic OHCI USB controller" > - default ARCH_SUNXI > ---help--- > Enables support for generic OHCI controller. >
Re: [PATCH 1/1] riscv: clarify meaning of CONFIG_SBI_V02
> From: Heinrich Schuchardt > Sent: Tuesday, November 08, 2022 11:14 PM > To: Bin Meng > Cc: Rick Jian-Zhi Chen(陳建志) ; Leo Yu-Chi Liang(梁育齊) > ; Conor Dooley ; > u-boot@lists.denx.de > Subject: Re: [PATCH 1/1] riscv: clarify meaning of CONFIG_SBI_V02 > > On 11/8/22 16:05, Bin Meng wrote: > > Hi Heinrich, > > > > On Tue, Nov 8, 2022 at 10:53 PM Heinrich Schuchardt > > wrote: > >> > >> Describe that CONFIG_SBI_V02=y does not mean SBI specification v0.2 > >> but v0.2 or later. > >> > >> Signed-off-by: Heinrich Schuchardt > >> > >> --- > >> arch/riscv/Kconfig | 14 +++--- > >> 1 file changed, 7 insertions(+), 7 deletions(-) Reviewed-by: Rick Chen
Re: [PATCH 1/1] riscv: clarify meaning of CONFIG_SBI_V02
Hi Heinrich > From: Heinrich Schuchardt > Sent: Tuesday, November 08, 2022 11:14 PM > To: Bin Meng > Cc: Rick Jian-Zhi Chen(陳建志) ; Leo Yu-Chi Liang(梁育齊) > ; Conor Dooley ; > u-boot@lists.denx.de > Subject: Re: [PATCH 1/1] riscv: clarify meaning of CONFIG_SBI_V02 > > On 11/8/22 16:05, Bin Meng wrote: > > Hi Heinrich, > > > > On Tue, Nov 8, 2022 at 10:53 PM Heinrich Schuchardt > > wrote: > >> > >> Describe that CONFIG_SBI_V02=y does not mean SBI specification v0.2 > >> but v0.2 or later. > >> > >> Signed-off-by: Heinrich Schuchardt > >> > >> --- > >> arch/riscv/Kconfig | 14 +++--- > >> 1 file changed, 7 insertions(+), 7 deletions(-) > >> > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index > >> 4d64e9be3f..ebc4bef220 100644 > >> --- a/arch/riscv/Kconfig > >> +++ b/arch/riscv/Kconfig > >> @@ -257,16 +257,16 @@ config SBI_V01 > >>deprecated in future once legacy M-mode software are no longer > >> in use. > >> > >> config SBI_V02 > >> - bool "SBI v0.2 support" > >> + bool "SBI v0.2 or later support" > >> depends on SBI > >> help > >> - This config allows kernel to use SBI v0.2 APIs. SBI v0.2 is more > >> - scalable and extendable to handle future needs for RISC-V > >> supervisor > >> - interfaces. For example, with SBI v0.2 HSM extension, only a > >> single > >> - hart need to boot and enter operating system. The booting hart > >> can > >> - bring up secondary harts one by one afterwards. > >> + The SBI specification introduced the concept of extensions in > >> version > >> + v0.2. With this configuration option U-Boot can detect and use > >> SBI > >> + extensions. With the HSM extension introduced in SBI 0.2, only a > >> + single hart needs to boot and enter the operating system. The > >> booting > >> + hart can bring up secondary harts one by one afterwards. > >> > >> - Choose this option if OpenSBI v0.7 or above release is used > >> together > >> + Choose this option if OpenSBI release v0.7 or above is used > >> + together > >>with U-Boot. > >> > >> endchoice > > > > I remember this option was borrowed from the Linux kernel. Do you plan > > to send patch to Linux kernel too? > > Linux uses a single configuration symbol > CONFIG_RISCV_SBI_V01 "SBI v0.1 support" > > So there is no need for a change there. > > SBI v0.1 is really obsolete. Can we drop support for it in U-Boot? I am fine to remove it. Thanks, Rick
Re: [RFC 1/1] tools: Dockerfile for armv7
On Thu, Nov 10, 2022 at 01:40:35PM -0700, Simon Glass wrote: > Hi, > > On Fri, 28 Oct 2022 at 13:19, Tom Rini wrote: > > > > On Fri, Oct 28, 2022 at 09:14:13PM +0200, Heinrich Schuchardt wrote: > > > On 10/28/22 21:04, Tom Rini wrote: > > > > On Sun, Oct 16, 2022 at 12:03:30PM +0200, Heinrich Schuchardt wrote: > > > > > On 10/16/22 09:43, Heinrich Schuchardt wrote: > > > > > > Provide an armv7 docker environment to build and test the sandbox. > > > > > > > > > > > > Signed-off-by: Heinrich > > > > > > Schuchardt > > > > > > --- > > > > > > I have been discussing with Simon, if we can test sandbox_defconfig > > > > > > build on arm in a docker container. This RFC is just a small step on > > > > > > the path to it. > > > > > > > > > > We also need a separate requirements.txt with pygit2==1.6.1 due the > > > > > different version of libgit in the armhf version of Ubuntu Jammy. > > > > > > > > So, what are the general best practices to have a Dockerfile that can be > > > > / is used for generating multiple platforms worth of images? We should > > > > aim for a single Dockerfile that can be used for x86-64, armv7 and > > > > aarch64. > > > > > > As we execute bash commands in the Dockerfile we could use if/fi and > > > case/esac to work around differences between architectures. > > > > For unavoidable differences, yes, OK. > > > > > Except on amd64 we only want to execute the sandbox. So there is no need > > > to > > > build GRUB, QEMU, and swtmp/libtmps. > > > > Why? There's no reason one shouldn't be able to run the whole of CI on > > aarch64 hosts, unless there's QEMU bugs, or similar issues. The first > > use might be just to do sandbox runs but I don't want to limit things > > without strong reason ($X doesn't work, would be a good reason to > > limit). > > I'm not sure where this is going, but how do we build sandbox on ARM? > Do we need to use the cross compiler? I'd love to see this happen. I'm going to keep an eye on the billing tab, but, I believe I have now signed up for a free and reasonably spec'd arm64 oracle cloud compute image and installed the gitlab runner on it. We'll want to put some additional logic in the gitlab-ci.yml file so that only certain jobs would run on runners with the lets say aarch64 tag, but, here's how we do it I believe. -- Tom signature.asc Description: PGP signature
Re: [PATCH] env: Allow string CONFIG options in the text environment
Hi Tom,
On Mon, 7 Nov 2022 at 10:29, Tom Rini wrote:
>
> On Mon, Nov 07, 2022 at 04:33:28PM +, Holger Brunck wrote:
> > Hi Tom,
> >
> > > > > On Fri, 4 Nov 2022 at 08:20, Holger Brunck
> > > > > wrote:
> > > > >>
> > > > >> Hi Simon,
> > > > >> I got no time to try it yet but I have a general comment.
> > > > >>
> > > > >>>
> > > > >>> Sometimes it is useful to include a CONFIG option that contains a
> > > > >>> string.
> > > > >>> This is hard to do in general, since in many cases it is useful to
> > > > >>> have the quotes around the string so that, for example:
> > > > >>>
> > > > >>
> > > > >> wouldn't it be cleaner to always convert a Kconfig option which is
> > > > >> defined as a string to a string without the double quotes? If
> > > > >> someone needs them he could explicitly add them with
> > > > >>
> > > > >> bootcmd=run "CONFIG_BOARD_CMD"
> > > > >>
> > > > >> Because in my case I have some options I use them to build
> > > > >> together the kernel command line I pass to the kernel. Ok I could
> > > > >> store them before in an own variable and them use them with
> > > > >> ${variable} in the command line. But I think it would be cleaner to
> > > > >> always convert a string defined in Kconfig in a string without the
> > > > >> quotes.
> > > What do you think?
> > > > >
> > > > > Yes I would prefer that to. I'm not sure how to implement it though.
> > > > > Any thoughts?
> > > >
> > > > I agree that special-casing the RHS containing a single qouted string
> > > > is a bad idea, it's really hard to understand and explain what the
> > > > rules are.
> > > >
> > > > Unfortunately, I don't think we can just create a separate version of
> > > > the config.h header where the quotes are removed and then as Holger
> > > > suggests rely on including the double quotes when needed, because then
> > > > the C preprocessor would see "CONFIG_BOARD_CMD" as a string literal,
> > > > inside which macro expansion is not done.
> > > >
> > > > What we really want is to separate the two uses of the config values:
> > > > "control" and "data". One use is on conditional lines
> > > >
> > > > #if IS_ENABLED(CONFIG_FOO)
> > > >
> > > > and another is the case of substituting values into RHS values.
> > > >
> > > > It is really convenient to use the C preprocessor for the former. But
> > > > for the latter, it's kind of overkill, and we could probably just as
> > > > well implement a simple perl script (or python or whatnot) that would
> > > > do what we want, including stripping of double quotes from string
> > > > items before substitution. But adding that as a pre-pre-processor step
> > > > (only doing substitution on lines not beginning with a #) would break
> > > > down if anybody uses #include directives, and it's also an annoying
> > > > extra step and extra script to maintain.
> > > >
> > > > tl;dr: no, I don't have any good ideas.
> > >
> > > This frankly also gets back to an ongoing discussion I'm having with Pali
> > > which is
> > > that having a Kconfig option to set a string used in the environment is
> > > well, not a
> > > great way to use the tool.
> > >
> > > What I kind of want, or at least keep leaning towards, is that nothing
> > > that sets
> > > the environment directly should be in Kconfig, and things should be
> > > pulled from
> > > the text based .env files.
> > >
> > > I don't have a complete idea / solution right now, but I'm not sure
> > > entering more
> > > strings in Kconfig, that end up in the environment is moving in the right
> > > direction.
> > >
> >
> > Ok. I was not aware that a Kconfig string should not be used in the
> > environment
> > header. If this is the case I agree that we maybe then should not support
> > it.
> >
> > But currently there are several usecases for it e.g.:
> > cmd/Kconfig:config MTDPARTS_DEFAULT
> > this is a string ends up in
> > include/env_default.h: "mtdparts=" CONFIG_MTDPARTS_DEFAULT "\0"
> >
> > So all these get obsolete in the future and should be defined in a
> > board_env.txt
> > file?
> >
> > If this is the case I will go into this direction and remove the strings I
> > have for my
> > boards coming from Kconfig and move them into a board_env.txt
>
> What I was trying to say was that yes, we do it in a ton of places
> today, and it's problematic. Pali's example was that nokia_rx51 had both
> preboot= in CONFIG_EXTRA_ENV_SETTINGS and then from how CONFIG_PREBOOT
> is set. I'd gone through and tried to find and fix all of the cases
> where preboot, mtdparts, etc, were both set in CONFIG_EXTRA_ENV_SETTINGS
> and then from env_default.h but missed at least that one case. And
> nothing but manual review will catch new cases of this issue from
> happening in the future. What I'm not sure of yet is how to solve this
> in a good way going forward.
If you are warning to stop the use of CONFIG strings in the
environment, the current implementation probably helps with that.
We could also grep for CONFIG options and make
Re: [RFC 1/1] tools: Dockerfile for armv7
Hi, On Fri, 28 Oct 2022 at 13:19, Tom Rini wrote: > > On Fri, Oct 28, 2022 at 09:14:13PM +0200, Heinrich Schuchardt wrote: > > On 10/28/22 21:04, Tom Rini wrote: > > > On Sun, Oct 16, 2022 at 12:03:30PM +0200, Heinrich Schuchardt wrote: > > > > On 10/16/22 09:43, Heinrich Schuchardt wrote: > > > > > Provide an armv7 docker environment to build and test the sandbox. > > > > > > > > > > Signed-off-by: Heinrich Schuchardt > > > > > --- > > > > > I have been discussing with Simon, if we can test sandbox_defconfig > > > > > build on arm in a docker container. This RFC is just a small step on > > > > > the path to it. > > > > > > > > We also need a separate requirements.txt with pygit2==1.6.1 due the > > > > different version of libgit in the armhf version of Ubuntu Jammy. > > > > > > So, what are the general best practices to have a Dockerfile that can be > > > / is used for generating multiple platforms worth of images? We should > > > aim for a single Dockerfile that can be used for x86-64, armv7 and > > > aarch64. > > > > As we execute bash commands in the Dockerfile we could use if/fi and > > case/esac to work around differences between architectures. > > For unavoidable differences, yes, OK. > > > Except on amd64 we only want to execute the sandbox. So there is no need to > > build GRUB, QEMU, and swtmp/libtmps. > > Why? There's no reason one shouldn't be able to run the whole of CI on > aarch64 hosts, unless there's QEMU bugs, or similar issues. The first > use might be just to do sandbox runs but I don't want to limit things > without strong reason ($X doesn't work, would be a good reason to > limit). I'm not sure where this is going, but how do we build sandbox on ARM? Do we need to use the cross compiler? I'd love to see this happen. Regards, Simon
Re: [PATCH v5 02/16] Makefile: Correct the binman rule
Hi Pali, On Thu, 10 Nov 2022 at 01:01, Pali Rohár wrote: > > On Wednesday 09 November 2022 19:14:40 Simon Glass wrote: > > This currently uses if_changed on a phony target. Use a real file as the > > target and add FORCE at the end, as required. Drop the 'inputs' phony > > since it is not needed. > > Hello! Just one small note. It is quite surprising that .buildman_stamp > target is called also when CONFIG_BINMAN is disabled. Not sure if this > is expected but seems it should not cause any issue. I don't think it does. Bit by bit, binman will become the final step for many boards. There is just so much complexity in building firmware images these days. Regards, Simon
Re: [PATCH] sandbox: Move the capsule GUID declarations to board file
On Thu, 10 Nov 2022 at 10:04, Sughosh Ganu wrote: > > The sandbox config file is to be removed. Move the GUID declarations > needed for capsule update functionality to the board file where they > are used. > > Signed-off-by: Sughosh Ganu > --- > board/sandbox/sandbox.c | 13 + > include/configs/sandbox.h | 13 - > 2 files changed, 13 insertions(+), 13 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH 0/8] env: mmc: improvements and corrections
Hi Patrick,
On Thu, 10 Nov 2022 at 03:49, Patrick Delaunay
wrote:
>
>
> Update in U-Boot env mmc backend with several cosmetic changes or
> corrections and 2 new features:
>
> 1/ CONFIG_ENV_MMC_USE_DT = no more use CONFIG_ENV_OFFSET
>in the mmc ENV backend when this config is activated.
>
>Requested by the STM32MP STMicroelectronics boards which activate
>several ENV_IS_IN_XXX; the value of CONFIG_ENV_OFFSET is invalid for
>SD-Card / eMMC boot; this offset should only used in SPIFlash backend
>(sf.c) for SPI-NOR boot.
>
>If this offset is used on mmc backend, when partition name in GPT is
>not aligned with U-Boot DT: "u-boot,mmc-env-partition", the behavior
>is difficult to debug: a partition is corrupted on 'env save' command.
>
> 2/ selects the GPT env partition by the "u-boot-env" type GUID introduced
>by the commit c0364ce1c695 ("doc/README.gpt: define partition
>type GUID for U-Boot environment")
>
>This feature can also avoid issue when 'u-boot-env' partition name
>change in GPT partitioning but not in the U-Boot DT with
>"u-boot,mmc-env-partition"
>
> Few check patch warnings remained in the series,
> but after check I can't remove them :
>
> - IS_ENABLED(ENV_MMC_HWPART_REDUND) is normally used as
> IS_ENABLED(CONFIG_ENV_MMC_HWPART_REDUND)
> => ENV_MMC_HWPART_REDUND is locally defined in this file it is not
> a real CONFIG but I can use the IS_ENABLED() macro as it is defined
> to 1
>
> - Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where
> possible
> + CONFIG_PARTITION_TYPE_GUID => info.type_guid existence
> + CONFIG_ENV_OFFSET_REDUND and CONFIG_ENV_MMC_USE_DT => only for define
>
> As I miss the merge window, not targeted for v2023.01 but for next
> v2023.04.
Shouldn't this all move to device tree? Using CONFIG options is such a
mess. We have the devices in DT so can indicate which ones have an
environment and what the parameters are for each.
Regards,
Simon
Re: [PATCH 01/21] Convert CONFIG_SYS_I2C_INIT_BOARD to Kconfig
On Fri, Oct 28, 2022 at 08:26:54PM -0400, Tom Rini wrote: > This converts the following to Kconfig: >CONFIG_SYS_I2C_INIT_BOARD > > Signed-off-by: Tom Rini > Reviewed-by: Simon Glass For the series, applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
[PATCH] net: macb: Fix race caused by flushing unwanted descriptors
The rx descriptor list is in cached memory, and there may be multiple
descriptors per cache-line. After reclaim_rx_buffers marks a descriptor
as unused it does a cache flush, which causes the entire cache-line to
be written to memory, which may override other descriptors in the same
cache-line that the controller may have written to.
The fix skips freeing descriptors that are not the last in a cache-line,
and if the freed descriptor is the last one in a cache-line, it marks
all the descriptors in the cache-line as unused.
This is similarly to what is done in drivers/net/fec_mxc.c
In my case this bug caused tftpboot to fail some times when other
packets are sent to u-boot in addition to the ongoing tftp (e.g. ping).
The driver would stop receiving new packets because it is waiting
on a descriptor that is marked unused, when in reality the descriptor
contains a new unprocessed packet but while freeing the previous buffer
descriptor & flushing the cache, the driver accidentally marked the
descriptor as unused.
Signed-off-by: Yaron Micher
---
drivers/net/macb.c | 51 +++---
1 file changed, 39 insertions(+), 12 deletions(-)
diff --git a/drivers/net/macb.c b/drivers/net/macb.c
index e02a57b411..65ec1f24ad 100644
--- a/drivers/net/macb.c
+++ b/drivers/net/macb.c
@@ -98,6 +98,9 @@ struct macb_dma_desc_64 {
#define MACB_RX_DMA_DESC_SIZE (DMA_DESC_BYTES(MACB_RX_RING_SIZE))
#define MACB_TX_DUMMY_DMA_DESC_SIZE(DMA_DESC_BYTES(1))
+#define DESC_PER_CACHELINE_32 (ARCH_DMA_MINALIGN/sizeof(struct macb_dma_desc))
+#define DESC_PER_CACHELINE_64 (ARCH_DMA_MINALIGN/DMA_DESC_SIZE)
+
#define RXBUF_FRMLEN_MASK 0x0fff
#define TXBUF_FRMLEN_MASK 0x07ff
@@ -401,32 +404,56 @@ static int _macb_send(struct macb_device *macb, const
char *name, void *packet,
return 0;
}
+static void reclaim_rx_buffer(struct macb_device *macb,
+ unsigned int idx)
+{
+ unsigned int mask;
+ unsigned int shift;
+ unsigned int i;
+
+ /*
+* There may be multiple descriptors per CPU cacheline,
+* so a cache flush would flush the whole line, meaning the content of
other descriptors
+* in the cacheline would also flush. If one of the other descriptors
had been
+* written to by the controller, the flush would cause those changes to
be lost.
+*
+* To circumvent this issue, we do the actual freeing only when we need
to free
+* the last descriptor in the current cacheline. When the current
descriptor is the
+* last in the cacheline, we free all the descriptors that belong to
that cacheline.
+*/
+ if (macb->config->hw_dma_cap & HW_DMA_CAP_64B) {
+ mask = DESC_PER_CACHELINE_64 - 1;
+ shift = 1;
+ } else {
+ mask = DESC_PER_CACHELINE_32 - 1;
+ shift = 0;
+ }
+
+ /* we exit without freeing if idx is not the last descriptor in the
cacheline */
+ if ((idx & mask) != mask)
+ return;
+
+ for (i = idx & (~mask); i <= idx; i++)
+ macb->rx_ring[i << shift].addr &= ~MACB_BIT(RX_USED);
+}
+
static void reclaim_rx_buffers(struct macb_device *macb,
unsigned int new_tail)
{
unsigned int i;
- unsigned int count;
i = macb->rx_tail;
macb_invalidate_ring_desc(macb, RX);
while (i > new_tail) {
- if (macb->config->hw_dma_cap & HW_DMA_CAP_64B)
- count = i * 2;
- else
- count = i;
- macb->rx_ring[count].addr &= ~MACB_BIT(RX_USED);
+ reclaim_rx_buffer(macb, i);
i++;
- if (i > MACB_RX_RING_SIZE)
+ if (i >= MACB_RX_RING_SIZE)
i = 0;
}
while (i < new_tail) {
- if (macb->config->hw_dma_cap & HW_DMA_CAP_64B)
- count = i * 2;
- else
- count = i;
- macb->rx_ring[count].addr &= ~MACB_BIT(RX_USED);
+ reclaim_rx_buffer(macb, i);
i++;
}
--
2.17.1
Re: [PATCH v4] schemas: Add schema for U-Boot driver model 'phase tags'
On Thu, Nov 10, 2022 at 10:59 AM Simon Glass wrote:
>
> Hi Rob,
>
> On Tue, 8 Nov 2022 at 10:19, Rob Herring wrote:
> >
> > On Tue, Nov 1, 2022 at 10:13 PM Simon Glass wrote:
> > >
> > > U-Boot has some particular challenges with device tree and devices:
> > >
> > > - U-Boot has multiple build phases, such as a Secondary Program Loader
> > > (SPL) phase which typically runs in a pre-SDRAM environment where code
> > > and data space are limited. In particular, there may not be enough
> > > space for the full device tree blob. U-Boot uses various automated
> > > techniques to reduce the size from perhaps 40KB to 3KB. It is not
> > > always possible to handle these tags entirely at build time, since
> > > U-Boot proper must have the full device tree, even though we do not
> > > want it to process all nodes until after relocation.
> > > - Some U-Boot phases needs to run before the clocks are properly set up,
> > > where the CPU may be running very slowly. Therefore it is important to
> > > bind only those devices which are actually needed in that phase
> > > - U-Boot uses lazy initialisation for its devices, with 'bind' and
> > > 'probe' being separate steps. Even if a device is bound, it is not
> > > actually probed until it is used. This is necessary to keep the boot
> > > time reasonable, e.g. to under a second
> > >
> > > The phases of U-Boot in order are: TPL, VPL, SPL, U-Boot (first
> > > pre-relocation, then post-relocation). ALl but the last two are optional.
> > >
> > > For the above reasons, U-Boot only includes the full device tree in the
> > > final 'U-Boot proper' build. Even then, before relocation U-Boot only
> > > processes nodes which are marked as being needed.
> > >
> > > For this to work, U-Boot's driver model[1] provides a way to mark device
> > > tree nodes as applicable for a particular phase. This works by adding a
> > > tag to the node, e.g.:
> > >
> > >cru: clock-controller@ff76 {
> > > phase,all;
> > > compatible = "rockchip,rk3399-cru";
> > > reg = <0x0 0xff76 0x0 0x1000>;
> > > rockchip,grf = <>;
> > > #clock-cells = <1>;
> > > #reset-cells = <1>;
> > > ...
> > >};
> > >
> > > Here the "phase,all" tag indicates that the node must be present in all
> > > phases, since the clock driver is required.
> > >
> > > There has been discussion over the years about whether this could be done
> > > in a property instead, e.g.
> > >
> > >options {
> > > phase,all = <> <_a> ...;
> > > ...
> > >};
> > >
> > > Some problems with this:
> > >
> > > - we need to be able to merge several such tags from different .dtsi files
> > > since many boards have their own specific requirements
> > > - it is hard to find and cross-reference the affected nodes
> > > - it is more error-prone
> > > - it requires significant tool rework in U-Boot, including fdtgrep and
> > > the build system
> > > - is harder (slower, more code) to process since it involves scanning
> > > another node/property to find out what to do with a particular node
> > > - we don't want to add phandle arguments to the above since we are
> > > referring, e.g., to the clock device as a whole, not a paricular clock
> > > - the of-platdata feature[2], which converts device tree to C for even
> > > more constrained environments, would need to become aware of the
> > > /options node
> > >
> > > There is also the question about whether this needs to be U-Boot-specific,
> > > or whether the tags could be generic. From what I can tell, U-Boot is the
> > > only bootloader which seriously attempts to use a runtime device tree in
> > > all cases. For this version, an attempt is made to name the phases in a
> > > generic manner.
> > >
> > > It should also be noted that the approach provided here has stood the test
> > > of time, used in U-Boot for 8 years so far.
> > >
> > > So add the schema for this. This will allow a major class of schema
> > > exceptions to be dropped from the U-Boot source tree.
> > >
> > > This being sent to the mailing list since it might attract more review.
> > > A PR will be sent when this has had some review. That is why the file
> > > path is set up for https://github.com/devicetree-org/dt-schema rather
> > > than the Linux kernel.
> > >
> > > [1]
> > > https://u-boot.readthedocs.io/en/latest/develop/driver-model/index.html
> > > [2]
> > > https://u-boot.readthedocs.io/en/latest/develop/driver-model/of-plat.html
> > >
> > > Signed-off-by: Simon Glass
> > > ---
> > >
> > > Changes in v4:
> > > - Drop some unnecessary context from the commit message
> > > - Explain why parent nodes do not automatically inherit their children's
> > > tags
> > > - Rename the tags to use a phase,xxx format, explaining each one
> > >
> > > Changes in v3:
> > > - Fix an incorrect schema path in $id
> > >
> > > Changes in v2:
> > > - Expand docs to include a description of each tag
> > > - Fix some typos and unclear wording
> > >
Re: [PATCH] Revert "serial: mxc: have putc use the TXFIFO"
On Tue, Nov 8, 2022 at 3:39 AM Fabio Estevam wrote:
>
> From: Fabio Estevam
>
> This reverts commit c7878a0483c59c48a730123bc0f4659fd40921bf.
>
> Since commit c7878a0483c5 ("serial: mxc: have putc use the TXFIFO"),
> serial console corruption can be seen when priting inside board_init().
>
> Revert it to avoid the regression.
>
> Reported-by: Tim Harvey
> Signed-off-by: Fabio Estevam
> ---
> drivers/serial/serial_mxc.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/serial/serial_mxc.c b/drivers/serial/serial_mxc.c
> index 4cf79c1ca24f..82c0d84628d5 100644
> --- a/drivers/serial/serial_mxc.c
> +++ b/drivers/serial/serial_mxc.c
> @@ -311,7 +311,7 @@ static int mxc_serial_putc(struct udevice *dev, const
> char ch)
> struct mxc_serial_plat *plat = dev_get_plat(dev);
> struct mxc_uart *const uart = plat->reg;
>
> - if (readl(>ts) & UTS_TXFULL)
> + if (!(readl(>ts) & UTS_TXEMPTY))
> return -EAGAIN;
>
> writel(ch, >txd);
> --
> 2.25.1
>
Fabio,
Thanks - yes please let's get this revert in.
Acked-by: Tim Harvey
Tim
Re: [PATCH] efi_loader: add comments on efi_file_from_path() regarding alignment
On 11/10/22 14:31, Ilias Apalodimas wrote: UEFI specification requires pointers that are passed to protocol member functions to be aligned. There's a u16_strdup in that function which doesn't make sense otherwise Add a comment so no one removes it accidentally Signed-off-by: Ilias Apalodimas --- lib/efi_loader/efi_file.c | 5 + 1 file changed, 5 insertions(+) diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c index 8480ed3007c7..5c254ccdd64d 100644 --- a/lib/efi_loader/efi_file.c +++ b/lib/efi_loader/efi_file.c @@ -1135,6 +1135,11 @@ struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp) return NULL; } + /* +* UEFI specification requires pointers that are passed to +* protocol member functions to be aligned. So memcpy it +* unconditionally +*/ filename = u16_strdup(fdp->str); if (!filename) return NULL; Reviewed-by: Heinrich Schuchardt
[PATCH] sandbox: Move the capsule GUID declarations to board file
The sandbox config file is to be removed. Move the GUID declarations
needed for capsule update functionality to the board file where they
are used.
Signed-off-by: Sughosh Ganu
---
board/sandbox/sandbox.c | 13 +
include/configs/sandbox.h | 13 -
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/board/sandbox/sandbox.c b/board/sandbox/sandbox.c
index 4d89f9be1c..4c655dfd49 100644
--- a/board/sandbox/sandbox.c
+++ b/board/sandbox/sandbox.c
@@ -30,6 +30,19 @@
gd_t *gd;
#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+/* GUIDs for capsule updatable firmware images */
+#define SANDBOX_UBOOT_IMAGE_GUID \
+ EFI_GUID(0x09d7cf52, 0x0720, 0x4710, 0x91, 0xd1, \
+0x08, 0x46, 0x9b, 0x7f, 0xe9, 0xc8)
+
+#define SANDBOX_UBOOT_ENV_IMAGE_GUID \
+ EFI_GUID(0x5a7021f5, 0xfef2, 0x48b4, 0xaa, 0xba, \
+0x83, 0x2e, 0x77, 0x74, 0x18, 0xc0)
+
+#define SANDBOX_FIT_IMAGE_GUID \
+ EFI_GUID(0x3673b45d, 0x6a7c, 0x46f3, 0x9e, 0x60, \
+0xad, 0xab, 0xb0, 0x3f, 0x79, 0x37)
+
struct efi_fw_image fw_images[] = {
#if defined(CONFIG_EFI_CAPSULE_FIRMWARE_RAW)
{
diff --git a/include/configs/sandbox.h b/include/configs/sandbox.h
index 5168e2fa35..0dcb2ebc31 100644
--- a/include/configs/sandbox.h
+++ b/include/configs/sandbox.h
@@ -10,19 +10,6 @@
#define CONFIG_MALLOC_F_ADDR 0x001
-/* GUIDs for capsule updatable firmware images */
-#define SANDBOX_UBOOT_IMAGE_GUID \
- EFI_GUID(0x09d7cf52, 0x0720, 0x4710, 0x91, 0xd1, \
-0x08, 0x46, 0x9b, 0x7f, 0xe9, 0xc8)
-
-#define SANDBOX_UBOOT_ENV_IMAGE_GUID \
- EFI_GUID(0x5a7021f5, 0xfef2, 0x48b4, 0xaa, 0xba, \
-0x83, 0x2e, 0x77, 0x74, 0x18, 0xc0)
-
-#define SANDBOX_FIT_IMAGE_GUID \
- EFI_GUID(0x3673b45d, 0x6a7c, 0x46f3, 0x9e, 0x60, \
-0xad, 0xab, 0xb0, 0x3f, 0x79, 0x37)
-
/* Size of our emulated memory */
#define SB_CONCAT(x, y) x ## y
#define SB_TO_UL(s) SB_CONCAT(s, UL)
--
2.34.1
Re: [PATCH v4] schemas: Add schema for U-Boot driver model 'phase tags'
Hi Rob,
On Tue, 8 Nov 2022 at 10:19, Rob Herring wrote:
>
> On Tue, Nov 1, 2022 at 10:13 PM Simon Glass wrote:
> >
> > U-Boot has some particular challenges with device tree and devices:
> >
> > - U-Boot has multiple build phases, such as a Secondary Program Loader
> > (SPL) phase which typically runs in a pre-SDRAM environment where code
> > and data space are limited. In particular, there may not be enough
> > space for the full device tree blob. U-Boot uses various automated
> > techniques to reduce the size from perhaps 40KB to 3KB. It is not
> > always possible to handle these tags entirely at build time, since
> > U-Boot proper must have the full device tree, even though we do not
> > want it to process all nodes until after relocation.
> > - Some U-Boot phases needs to run before the clocks are properly set up,
> > where the CPU may be running very slowly. Therefore it is important to
> > bind only those devices which are actually needed in that phase
> > - U-Boot uses lazy initialisation for its devices, with 'bind' and
> > 'probe' being separate steps. Even if a device is bound, it is not
> > actually probed until it is used. This is necessary to keep the boot
> > time reasonable, e.g. to under a second
> >
> > The phases of U-Boot in order are: TPL, VPL, SPL, U-Boot (first
> > pre-relocation, then post-relocation). ALl but the last two are optional.
> >
> > For the above reasons, U-Boot only includes the full device tree in the
> > final 'U-Boot proper' build. Even then, before relocation U-Boot only
> > processes nodes which are marked as being needed.
> >
> > For this to work, U-Boot's driver model[1] provides a way to mark device
> > tree nodes as applicable for a particular phase. This works by adding a
> > tag to the node, e.g.:
> >
> >cru: clock-controller@ff76 {
> > phase,all;
> > compatible = "rockchip,rk3399-cru";
> > reg = <0x0 0xff76 0x0 0x1000>;
> > rockchip,grf = <>;
> > #clock-cells = <1>;
> > #reset-cells = <1>;
> > ...
> >};
> >
> > Here the "phase,all" tag indicates that the node must be present in all
> > phases, since the clock driver is required.
> >
> > There has been discussion over the years about whether this could be done
> > in a property instead, e.g.
> >
> >options {
> > phase,all = <> <_a> ...;
> > ...
> >};
> >
> > Some problems with this:
> >
> > - we need to be able to merge several such tags from different .dtsi files
> > since many boards have their own specific requirements
> > - it is hard to find and cross-reference the affected nodes
> > - it is more error-prone
> > - it requires significant tool rework in U-Boot, including fdtgrep and
> > the build system
> > - is harder (slower, more code) to process since it involves scanning
> > another node/property to find out what to do with a particular node
> > - we don't want to add phandle arguments to the above since we are
> > referring, e.g., to the clock device as a whole, not a paricular clock
> > - the of-platdata feature[2], which converts device tree to C for even
> > more constrained environments, would need to become aware of the
> > /options node
> >
> > There is also the question about whether this needs to be U-Boot-specific,
> > or whether the tags could be generic. From what I can tell, U-Boot is the
> > only bootloader which seriously attempts to use a runtime device tree in
> > all cases. For this version, an attempt is made to name the phases in a
> > generic manner.
> >
> > It should also be noted that the approach provided here has stood the test
> > of time, used in U-Boot for 8 years so far.
> >
> > So add the schema for this. This will allow a major class of schema
> > exceptions to be dropped from the U-Boot source tree.
> >
> > This being sent to the mailing list since it might attract more review.
> > A PR will be sent when this has had some review. That is why the file
> > path is set up for https://github.com/devicetree-org/dt-schema rather
> > than the Linux kernel.
> >
> > [1] https://u-boot.readthedocs.io/en/latest/develop/driver-model/index.html
> > [2]
> > https://u-boot.readthedocs.io/en/latest/develop/driver-model/of-plat.html
> >
> > Signed-off-by: Simon Glass
> > ---
> >
> > Changes in v4:
> > - Drop some unnecessary context from the commit message
> > - Explain why parent nodes do not automatically inherit their children's
> > tags
> > - Rename the tags to use a phase,xxx format, explaining each one
> >
> > Changes in v3:
> > - Fix an incorrect schema path in $id
> >
> > Changes in v2:
> > - Expand docs to include a description of each tag
> > - Fix some typos and unclear wording
> >
> > dtschema/lib.py | 5 +++
> > dtschema/schemas/phase.yaml | 73 +
> > test/phases.dts | 26 +
> > 3 files changed, 104 insertions(+)
> > create mode 100644 dtschema/schemas/phase.yaml
> > create mode
[PATCH v5] schemas: Add schema for U-Boot driver model 'phase tags'
U-Boot has some particular challenges with device tree and devices:
- U-Boot has multiple build phases, such as a Secondary Program Loader
(SPL) phase which typically runs in a pre-SDRAM environment where code
and data space are limited. In particular, there may not be enough
space for the full device tree blob. U-Boot uses various automated
techniques to reduce the size from perhaps 40KB to 3KB. It is not
always possible to handle these tags entirely at build time, since
U-Boot proper must have the full device tree, even though we do not
want it to process all nodes until after relocation.
- Some U-Boot phases needs to run before the clocks are properly set up,
where the CPU may be running very slowly. Therefore it is important to
bind only those devices which are actually needed in that phase
- U-Boot uses lazy initialisation for its devices, with 'bind' and
'probe' being separate steps. Even if a device is bound, it is not
actually probed until it is used. This is necessary to keep the boot
time reasonable, e.g. to under a second
The phases of U-Boot in order are: TPL, VPL, SPL, U-Boot (first
pre-relocation, then post-relocation). ALl but the last two are optional.
For the above reasons, U-Boot only includes the full device tree in the
final 'U-Boot proper' build. Even then, before relocation U-Boot only
processes nodes which are marked as being needed.
For this to work, U-Boot's driver model[1] provides a way to mark device
tree nodes as applicable for a particular phase. This works by adding a
tag to the node, e.g.:
cru: clock-controller@ff76 {
phase-all;
compatible = "rockchip,rk3399-cru";
reg = <0x0 0xff76 0x0 0x1000>;
rockchip,grf = <>;
#clock-cells = <1>;
#reset-cells = <1>;
...
};
Here the "phase-all" tag indicates that the node must be present in all
phases, since the clock driver is required.
There has been discussion over the years about whether this could be done
in a property instead, e.g.
options {
phase-all = <> <_a> ...;
...
};
Some problems with this:
- we need to be able to merge several such tags from different .dtsi files
since many boards have their own specific requirements
- it is hard to find and cross-reference the affected nodes
- it is more error-prone
- it requires significant tool rework in U-Boot, including fdtgrep and
the build system
- is harder (slower, more code) to process since it involves scanning
another node/property to find out what to do with a particular node
- we don't want to add phandle arguments to the above since we are
referring, e.g., to the clock device as a whole, not a paricular clock
- the of-platdata feature[2], which converts device tree to C for even
more constrained environments, would need to become aware of the
/options node
There is also the question about whether this needs to be U-Boot-specific,
or whether the tags could be generic. From what I can tell, U-Boot is the
only bootloader which seriously attempts to use a runtime device tree in
all cases. For this version, an attempt is made to name the phases in a
generic manner.
It should also be noted that the approach provided here has stood the test
of time, used in U-Boot for 8 years so far.
So add the schema for this. This will allow a major class of schema
exceptions to be dropped from the U-Boot source tree.
This being sent to the mailing list since it might attract more review.
A PR will be sent when this has had some review. That is why the file
path is set up for https://github.com/devicetree-org/dt-schema rather
than the Linux kernel.
[1] https://u-boot.readthedocs.io/en/latest/develop/driver-model/index.html
[2] https://u-boot.readthedocs.io/en/latest/develop/driver-model/of-plat.html
Signed-off-by: Simon Glass
---
Changes in v5:
- Fix instructions to run test
- Update binding title
- Use 'phase-' instead of 'phase,'
Changes in v4:
- Drop some unnecessary context from the commit message
- Explain why parent nodes do not automatically inherit their children's
tags
- Rename the tags to use a phase,xxx format, explaining each one
Changes in v3:
- Fix an incorrect schema path in $id
Changes in v2:
- Expand docs to include a description of each tag
- Fix some typos and unclear wording
dtschema/lib.py | 5 +++
dtschema/schemas/phase.yaml | 74 +
test/phases.dts | 23
3 files changed, 102 insertions(+)
create mode 100644 dtschema/schemas/phase.yaml
create mode 100644 test/phases.dts
diff --git a/dtschema/lib.py b/dtschema/lib.py
index 1bce070..d65a0fc 100644
--- a/dtschema/lib.py
+++ b/dtschema/lib.py
@@ -514,6 +514,11 @@ def fixup_node_props(schema):
schema['properties'].setdefault('status', True)
schema['properties'].setdefault('secure-status', True)
schema['properties'].setdefault('$nodename', True)
+schema['properties'].setdefault('phase-pre-sram', True)
+
Re: [PATCH v2 8/8] imx: imx8: apalis: switch to binman
On 09/11/22, Marcel Ziswiler wrote: > Hi Oliver > > Thanks for working on this. > > On Fri, 2022-11-04 at 16:03 +0100, Oliver Graute wrote: > > Switch to use binman to pack images > > > > Signed-off-by: Oliver Graute > > --- > > Changes for v2 > > - use common imx8qm-u-boot.dtsi > > - guard SPL nodes with CONFIG_SPL > > I don't think it is that simple. Even after guarding them SPL nodes the > flash.bin one still references them, > not? And yes, so far we never used SPL for them 8/8X as that honestly does > not really make much sense given the > SCFW does the actual RAM initialisation. Anyway, I am not very clear on > how/what exactly that all means now. On imx8qm-rom7720 we use the SCFW for RAM initalisation too but still use SPL. > > Another topic is that flash.bin is now just the SPL whereas previously > u-boot-dtb.imx was the entire thing. But > again, that could have been related to us not using SPL. yes, thats right, if I guard the SPL subnode in imx-boot node I get a 0 byte flash.bin. At the moment I don't know the best way here. I would be grateful for some hints here. > > CONFIG_FIT=y +CONFIG_FIT_EXTERNAL_OFFSET=0x3000 > > Also not quite sure what exactly that is now. its introduced by commit 3814fcba12323b9f30f39ee5455f3f9a7e955c64 Best regards, Oliver
Re: [PATCH v2 0/8] imx8: switch missing boards to binman
On Wed, Nov 09, 2022 at 02:45:10PM -0300, Fabio Estevam wrote: > Hi Oliver and Stefano, > > On Wed, Nov 9, 2022 at 1:19 PM Oliver Graute wrote: > > > For my imx8qm boards I need to manually place these imx firmware blobs with > > expected names for sucessfull image creation. > > > > As I understood the CI creates somes fakes. But I don't know where this > > is happening. > > CI is falling with your series applied: > > https://source.denx.de/u-boot/custodians/u-boot-imx/-/jobs/525787 > > It is easy to reproduce the failure locally. Just remove > ahab-container.img from the > U-Boot tree and try to build imx8qxp_mek_defconfig. > > Prior to the binman conversion: > > WARNING './ahab-container.img' not found, resulting binary is not-functional > make[1]: Nothing to be done for 'SPL'. > > Only a warning is issued. > > After the binman conversion: > > WARNING './ahab-container.img' not found, resulting binary is not-functional > make[1]: Nothing to be done for 'SPL'. > BINMAN all > binman: Error 1 running 'mkimage -d ./mkimage.spl.mkimage -n > spl/u-boot-spl.cfgout -T imx8image -e 0x10 > ./mkimage-out.spl.mkimage': Fail open first container file > ahab-container.img > > make: *** [Makefile:1116: all] Error 1 > > Maybe someone has any suggestions? So, this is coming from board/freescale/imx8qxp_mek/imximage.cfg (and there's another board or two doing the same / simialr thing), shouldn't this be wholly in binman now? -- Tom signature.asc Description: PGP signature
Re: [PATCH] efi_loader: add comments on efi_file_from_path() regarding alignment
On 11/10/22 15:09, Ilias Apalodimas wrote: Hi Heinrich On Thu, 10 Nov 2022 at 15:46, Heinrich Schuchardt wrote: On 11/10/22 14:31, Ilias Apalodimas wrote: UEFI specification requires pointers that are passed to protocol member functions to be aligned. There's a u16_strdup in that function which doesn't make sense otherwise Add a comment so no one removes it accidentally Signed-off-by: Ilias Apalodimas --- lib/efi_loader/efi_file.c | 5 + 1 file changed, 5 insertions(+) diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c index 8480ed3007c7..5c254ccdd64d 100644 --- a/lib/efi_loader/efi_file.c +++ b/lib/efi_loader/efi_file.c @@ -1135,6 +1135,11 @@ struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp) return NULL; } + /* + * UEFI specification requires pointers that are passed to + * protocol member functions to be aligned. So memcpy it + * unconditionally + */ filename = u16_strdup(fdp->str); On ARM we enable unaligned access which is supported by the CPU. On RISC-V unaligned access is emulated by OpenSBI which is very slow. Therefore copying make sense. u16_strdup() calls u16_strsize() which itself is not caring about alignment. So this u16_strdup does not resolve all alignment issues. We could use the length field of the file path node to determine the length of the string to be copied and invoke malloc(fdp->length - 4). memcpy(,, fdp->length - 4). This would be better performance wise on RISC-V. Sure that makes sense. But the comment is for EFI functions that have that string as an argument. Will you pick the comment and I can send that on a followup patch? Thanks /Ilias Reviewed-by: Heinrich Schuchardt Best regards Heinrich if (!filename) return NULL;
Re: [PATCH] efi_loader: add comments on efi_file_from_path() regarding alignment
Hi Heinrich On Thu, 10 Nov 2022 at 15:46, Heinrich Schuchardt wrote: > > On 11/10/22 14:31, Ilias Apalodimas wrote: > > UEFI specification requires pointers that are passed to protocol member > > functions to be aligned. There's a u16_strdup in that function which > > doesn't make sense otherwise Add a comment so no one removes it > > accidentally > > > > Signed-off-by: Ilias Apalodimas > > --- > > lib/efi_loader/efi_file.c | 5 + > > 1 file changed, 5 insertions(+) > > > > diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c > > index 8480ed3007c7..5c254ccdd64d 100644 > > --- a/lib/efi_loader/efi_file.c > > +++ b/lib/efi_loader/efi_file.c > > @@ -1135,6 +1135,11 @@ struct efi_file_handle *efi_file_from_path(struct > > efi_device_path *fp) > > return NULL; > > } > > > > + /* > > + * UEFI specification requires pointers that are passed to > > + * protocol member functions to be aligned. So memcpy it > > + * unconditionally > > + */ > > filename = u16_strdup(fdp->str); > > On ARM we enable unaligned access which is supported by the CPU. On > RISC-V unaligned access is emulated by OpenSBI which is very slow. > Therefore copying make sense. > > u16_strdup() calls u16_strsize() which itself is not caring about > alignment. So this u16_strdup does not resolve all alignment issues. > > We could use the length field of the file path node to determine the > length of the string to be copied and invoke > > malloc(fdp->length - 4). > memcpy(,, fdp->length - 4). > > This would be better performance wise on RISC-V. Sure that makes sense. But the comment is for EFI functions that have that string as an argument. Will you pick the comment and I can send that on a followup patch? Thanks /Ilias > > Best regards > > Heinrich > > > if (!filename) > > return NULL; >
Re: [PATCH v3 1/2] arm: mediatek: add mt8195 SOC support
On Thu, 2022-11-10 at 15:34 +0800, Macpaul Lin wrote:
> From: Fabien Parent
>
> The MediaTek MT8195 is a ARM64-based SoC with a quad-core Cortex-A73
> and
> a quad-core Cortex-A53. It is including UART, SPI, USB3.0 device and
> hosts,
> SD and MMC cards, UFS, PWM, I2C, I2S, S/PDIF, and several LPDDR3
> and LPDDR4 options.
>
> Signed-off-by: Fabien Parent
> Signed-off-by: Macpaul Lin
>
> ---
> MAINTAINERS| 2 +
> arch/arm/dts/mt8195.dtsi | 370
> +
> arch/arm/mach-mediatek/Kconfig | 13 +-
> arch/arm/mach-mediatek/Makefile| 1 +
> arch/arm/mach-mediatek/mt8195/Makefile | 3 +
> arch/arm/mach-mediatek/mt8195/init.c | 81 ++
> 6 files changed, 469 insertions(+), 1 deletion(-)
> create mode 100644 arch/arm/dts/mt8195.dtsi
> create mode 100644 arch/arm/mach-mediatek/mt8195/Makefile
> create mode 100644 arch/arm/mach-mediatek/mt8195/init.c
>
> Changes for v2:
> - Correct node name to t-phy for u3phy0.
> - Add platform compatible string "mediatek,mt8195-tphy" to all usb
> phy nodes.
> - remove clock nodes that software cannot controlled in phy nodes.
> - Test and add back "mac" for HOST only xhci nodes.
>
> Changes for v3:
> - Revise device node name from "xhciX: xhciX@" to "xhciX: xhci@".
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 1cf99c1393..5528dd28c3 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -362,8 +362,10 @@ ARM MEDIATEK
> M: Ryder Lee
> M: Weijie Gao
> M: Chunfeng Yun
> +M: Macpaul Lin
> R: GSS_MTK_Uboot_upstream
> S: Maintained
> +F: arch/arm/dts/mt8195.dtsi
> F: arch/arm/mach-mediatek/
> F: arch/arm/include/asm/arch-mediatek/
> F: board/mediatek/
> diff --git a/arch/arm/dts/mt8195.dtsi b/arch/arm/dts/mt8195.dtsi
> new file mode 100644
> index 00..33282d21d1
> --- /dev/null
> +++ b/arch/arm/dts/mt8195.dtsi
> @@ -0,0 +1,370 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +/*
> + * Copyright (C) 2022 MediaTek Inc.
> + * Copyright (C) 2022 BayLibre, SAS
> + * Author: Ben Ho
> + * Erin Lo
> + * Fabien Parent
> + * Macpaul Lin
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +/ {
> + compatible = "mediatek,mt8195";
> + interrupt-parent = <>;
> + #address-cells = <2>;
> + #size-cells = <2>;
> +
> + cpus {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + cpu-map {
> + cluster0 {
> + core0 {
> + cpu = <>;
> + };
> + core1 {
> + cpu = <>;
> + };
> + core2 {
> + cpu = <>;
> + };
> + core3 {
> + cpu = <>;
> + };
> + };
> +
> + cluster1 {
> + core0 {
> + cpu = <>;
> + };
> + core1 {
> + cpu = <>;
> + };
> + core2 {
> + cpu = <>;
> + };
> + core3 {
> + cpu = <>;
> + };
> + };
> + };
> +
> + cpu0: cpu@0 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x000>;
> + enable-method = "psci";
> + capacity-dmips-mhz = <741>;
> + };
> +
> + cpu1: cpu@1 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x001>;
> + enable-method = "psci";
> + capacity-dmips-mhz = <741>;
> + };
> +
> + cpu2: cpu@2 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x002>;
> + enable-method = "psci";
> + capacity-dmips-mhz = <741>;
> + };
> +
> + cpu3: cpu@3 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a53";
> + reg = <0x003>;
> + enable-method = "psci";
> + capacity-dmips-mhz = <741>;
> + };
> +
> + cpu4: cpu@100 {
> + device_type = "cpu";
> + compatible = "arm,cortex-a73";
> + reg = <0x100>;
> +
Re: [PATCH] efi_loader: add comments on efi_file_from_path() regarding alignment
On 11/10/22 14:31, Ilias Apalodimas wrote: UEFI specification requires pointers that are passed to protocol member functions to be aligned. There's a u16_strdup in that function which doesn't make sense otherwise Add a comment so no one removes it accidentally Signed-off-by: Ilias Apalodimas --- lib/efi_loader/efi_file.c | 5 + 1 file changed, 5 insertions(+) diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c index 8480ed3007c7..5c254ccdd64d 100644 --- a/lib/efi_loader/efi_file.c +++ b/lib/efi_loader/efi_file.c @@ -1135,6 +1135,11 @@ struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp) return NULL; } + /* +* UEFI specification requires pointers that are passed to +* protocol member functions to be aligned. So memcpy it +* unconditionally +*/ filename = u16_strdup(fdp->str); On ARM we enable unaligned access which is supported by the CPU. On RISC-V unaligned access is emulated by OpenSBI which is very slow. Therefore copying make sense. u16_strdup() calls u16_strsize() which itself is not caring about alignment. So this u16_strdup does not resolve all alignment issues. We could use the length field of the file path node to determine the length of the string to be copied and invoke malloc(fdp->length - 4). memcpy(,, fdp->length - 4). This would be better performance wise on RISC-V. Best regards Heinrich if (!filename) return NULL;
Re: [PATCH 1/1] efi_loader: improve description of efi_file_from_path()
On Thu, 10 Nov 2022 at 15:14, Heinrich Schuchardt
wrote:
>
> Provide a description of the function's logic.
>
> Signed-off-by: Heinrich Schuchardt
> ---
> lib/efi_loader/efi_file.c | 9 +
> 1 file changed, 9 insertions(+)
>
> diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c
> index c96a7f7ca3..ae0ca6101b 100644
> --- a/lib/efi_loader/efi_file.c
> +++ b/lib/efi_loader/efi_file.c
> @@ -1098,6 +1098,15 @@ static const struct efi_file_handle
> efi_file_handle_protocol = {
> /**
> * efi_file_from_path() - open file via device path
> *
> + * The device path @fp consist of the device path of the handle with the
consists of
> + * simple file system protocol and one or more file path device path nodes.
> + * The concatenation of all file path names provides the total file path.
> + *
> + * The code starts at the first file path node and tries to open that file or
> + * directory. If there is a succeding file path node, the code opens it
> relative
> + * to this directory and continues iterating until reaching the last file
> path
> + * node.
> + *
> * @fp:device path
> * Return: EFI_FILE_PROTOCOL for the file or NULL
> */
> --
> 2.37.2
>
Reviewed-by: Ilias Apalodimas
[PATCH] efi_loader: add comments on efi_file_from_path() regarding alignment
UEFI specification requires pointers that are passed to protocol member functions to be aligned. There's a u16_strdup in that function which doesn't make sense otherwise Add a comment so no one removes it accidentally Signed-off-by: Ilias Apalodimas --- lib/efi_loader/efi_file.c | 5 + 1 file changed, 5 insertions(+) diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c index 8480ed3007c7..5c254ccdd64d 100644 --- a/lib/efi_loader/efi_file.c +++ b/lib/efi_loader/efi_file.c @@ -1135,6 +1135,11 @@ struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp) return NULL; } + /* +* UEFI specification requires pointers that are passed to +* protocol member functions to be aligned. So memcpy it +* unconditionally +*/ filename = u16_strdup(fdp->str); if (!filename) return NULL; -- 2.38.1
Re: [PATCH] efi_loader: remove EFI_CALL from efi_file_from_path()
On 11/10/22 14:11, Ilias Apalodimas wrote:
We now have internal functions for OpenVolume, EFI_FILE_PROTOCOL.Open()
and EFI_FILE_PROTOCOL.Close(). So use these instead of wrapping the
callsites with EFI_CALL()
Signed-off-by: Ilias Apalodimas
---
lib/efi_loader/efi_file.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c
index c96a7f7ca371..8480ed3007c7 100644
--- a/lib/efi_loader/efi_file.c
+++ b/lib/efi_loader/efi_file.c
@@ -,7 +,7 @@ struct efi_file_handle *efi_file_from_path(struct
efi_device_path *fp)
if (!v)
return NULL;
- EFI_CALL(ret = v->open_volume(v, ));
+ ret = efi_open_volume_int(v, );
if (ret != EFI_SUCCESS)
return NULL;
@@ -1131,22 +1131,21 @@ struct efi_file_handle *efi_file_from_path(struct
efi_device_path *fp)
if (!EFI_DP_TYPE(fp, MEDIA_DEVICE, FILE_PATH)) {
printf("bad file path!\n");
- f->close(f);
+ efi_file_close_int(f);
return NULL;
}
filename = u16_strdup(fdp->str);
if (!filename)
return NULL;
- EFI_CALL(ret = f->open(f, , filename,
- EFI_FILE_MODE_READ, 0));
+ ret = efi_file_open_int(f, , filename, EFI_FILE_MODE_READ,
0);
free(filename);
if (ret != EFI_SUCCESS)
return NULL;
fp = efi_dp_next(fp);
- EFI_CALL(f->close(f));
+ efi_file_close_int(f);
We should not assume that the FileProtocol() is implemented by U-Boot.
An EFI binary that we execute may have implement its own simple file
system protocol and file protocol and then call LoadImage() on it.
We do exactly this in lib/efi_selftest/efi_selftest_loadimage.c.
Best regards
Heinrich
f = f2;
}
[PATCH 1/1] efi_loader: improve description of efi_file_from_path()
Provide a description of the function's logic.
Signed-off-by: Heinrich Schuchardt
---
lib/efi_loader/efi_file.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c
index c96a7f7ca3..ae0ca6101b 100644
--- a/lib/efi_loader/efi_file.c
+++ b/lib/efi_loader/efi_file.c
@@ -1098,6 +1098,15 @@ static const struct efi_file_handle
efi_file_handle_protocol = {
/**
* efi_file_from_path() - open file via device path
*
+ * The device path @fp consist of the device path of the handle with the
+ * simple file system protocol and one or more file path device path nodes.
+ * The concatenation of all file path names provides the total file path.
+ *
+ * The code starts at the first file path node and tries to open that file or
+ * directory. If there is a succeding file path node, the code opens it
relative
+ * to this directory and continues iterating until reaching the last file path
+ * node.
+ *
* @fp:device path
* Return: EFI_FILE_PROTOCOL for the file or NULL
*/
--
2.37.2
Re: [PATCH] efi_loader: simplify efi_load_from_path
Heinrich
Please ignore this. As we discussed although this cleans up things
considerably, it's not exactly what the spec describes. So let's
keep the existing function.
I've sent another patch cleaning up the EFI_CALL() iterations
Thanks
/Ilias
On Thu, 10 Nov 2022 at 13:36, Ilias Apalodimas
wrote:
>
> The current implementation efi_load_from_path is a bit confusing.
> First of all it tries to check the device path to make sure it contains
> the path to the device plus the media path with the filename and nothing in
> between. But that should already be valid since U-Boot constructs those
> device paths.
> On top of that it tries to traverse the device path nodes and acquire the
> file part by stepping through the nodes of the directory path until the
> file is reached. We already have efi_dp_split_file_path() for that so
> rewrite the function and clean it up to use existing code.
>
> Signed-off-by: Ilias Apalodimas
> ---
> lib/efi_loader/efi_file.c | 52 ---
> 1 file changed, 16 insertions(+), 36 deletions(-)
>
> diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c
> index c96a7f7ca371..e1695ba309e4 100644
> --- a/lib/efi_loader/efi_file.c
> +++ b/lib/efi_loader/efi_file.c
> @@ -1104,53 +1104,33 @@ static const struct efi_file_handle
> efi_file_handle_protocol = {
> struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp)
> {
> struct efi_simple_file_system_protocol *v;
> - struct efi_file_handle *f;
> + struct efi_device_path_file_path *mdp;
> + struct efi_device_path *dev_dp, *file_dp;
> + struct efi_file_handle *vol_handle, *file_handle = NULL;
> efi_status_t ret;
>
> v = efi_fs_from_path(fp);
> if (!v)
> return NULL;
>
> - EFI_CALL(ret = v->open_volume(v, ));
> + ret = efi_dp_split_file_path(fp, _dp, _dp);
> if (ret != EFI_SUCCESS)
> return NULL;
> + efi_free_pool(dev_dp);
>
> - /* Skip over device-path nodes before the file path. */
> - while (fp && !EFI_DP_TYPE(fp, MEDIA_DEVICE, FILE_PATH))
> - fp = efi_dp_next(fp);
> -
> - /*
> -* Step through the nodes of the directory path until the actual file
> -* node is reached which is the final node in the device path.
> -*/
> - while (fp) {
> - struct efi_device_path_file_path *fdp =
> - container_of(fp, struct efi_device_path_file_path,
> dp);
> - struct efi_file_handle *f2;
> - u16 *filename;
> -
> - if (!EFI_DP_TYPE(fp, MEDIA_DEVICE, FILE_PATH)) {
> - printf("bad file path!\n");
> - f->close(f);
> - return NULL;
> - }
> -
> - filename = u16_strdup(fdp->str);
> - if (!filename)
> - return NULL;
> - EFI_CALL(ret = f->open(f, , filename,
> - EFI_FILE_MODE_READ, 0));
> - free(filename);
> - if (ret != EFI_SUCCESS)
> - return NULL;
> -
> - fp = efi_dp_next(fp);
> -
> - EFI_CALL(f->close(f));
> - f = f2;
> + ret = efi_open_volume_int(v, _handle);
> + if (ret != EFI_SUCCESS) {
> + efi_free_pool(file_dp);
> + return NULL;
> }
>
> - return f;
> + mdp = (struct efi_device_path_file_path *)file_dp;
> + /* we don't really care about the ret here *file_handle will be NULL
> */
> + ret = efi_file_open_int(vol_handle, _handle, mdp->str,
> EFI_FILE_MODE_READ, 0);
> + efi_free_pool(file_dp);
> + efi_file_close_int(vol_handle);
> +
> + return file_handle;
> }
>
> efi_status_t efi_open_volume_int(struct efi_simple_file_system_protocol
> *this,
> --
> 2.38.1
>
[PATCH] efi_loader: remove EFI_CALL from efi_file_from_path()
We now have internal functions for OpenVolume, EFI_FILE_PROTOCOL.Open()
and EFI_FILE_PROTOCOL.Close(). So use these instead of wrapping the
callsites with EFI_CALL()
Signed-off-by: Ilias Apalodimas
---
lib/efi_loader/efi_file.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c
index c96a7f7ca371..8480ed3007c7 100644
--- a/lib/efi_loader/efi_file.c
+++ b/lib/efi_loader/efi_file.c
@@ -,7 +,7 @@ struct efi_file_handle *efi_file_from_path(struct
efi_device_path *fp)
if (!v)
return NULL;
- EFI_CALL(ret = v->open_volume(v, ));
+ ret = efi_open_volume_int(v, );
if (ret != EFI_SUCCESS)
return NULL;
@@ -1131,22 +1131,21 @@ struct efi_file_handle *efi_file_from_path(struct
efi_device_path *fp)
if (!EFI_DP_TYPE(fp, MEDIA_DEVICE, FILE_PATH)) {
printf("bad file path!\n");
- f->close(f);
+ efi_file_close_int(f);
return NULL;
}
filename = u16_strdup(fdp->str);
if (!filename)
return NULL;
- EFI_CALL(ret = f->open(f, , filename,
- EFI_FILE_MODE_READ, 0));
+ ret = efi_file_open_int(f, , filename, EFI_FILE_MODE_READ,
0);
free(filename);
if (ret != EFI_SUCCESS)
return NULL;
fp = efi_dp_next(fp);
- EFI_CALL(f->close(f));
+ efi_file_close_int(f);
f = f2;
}
--
2.38.1
Re: [PATCH v2] riscv: Fix detecting FPU support in standard extension
> On Sat, 2022-11-05 at 14:02 +0800, Yu Chien Peter Lin wrote:
>
> We should check the string until it hits underscore, in case it
> searches for the letters in the custom extension. For example,
> "rv64imac_xandes" will be treated as D extension support since
> there is a "d" in "andes", resulting illegal instruction caused
> by initializing FCSR.
>
> Signed-off-by: Yu Chien Peter Lin
> ---
> arch/riscv/cpu/cpu.c | 14 +++---
> 1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/cpu/cpu.c b/arch/riscv/cpu/cpu.c
> index 52ab02519f..d34c8efce0 100644
> --- a/arch/riscv/cpu/cpu.c
> +++ b/arch/riscv/cpu/cpu.c
> @@ -36,6 +36,7 @@ static inline bool supports_extension(char ext)
> #ifdef CONFIG_CPU
> struct udevice *dev;
> char desc[32];
> + int i;
>
> uclass_find_first_device(UCLASS_CPU, );
> if (!dev) {
> @@ -43,9 +44,16 @@ static inline bool supports_extension(char ext)
> return false;
> }
> if (!cpu_get_desc(dev, desc, sizeof(desc))) {
> - /* skip the first 4 characters (rv32|rv64) */
> - if (strchr(desc + 4, ext))
> - return true;
> + /*
> +* skip the first 4 characters (rv32|rv64) and
> +* check until underscore
> +*/
> + for (i = 4; i < sizeof(desc); i++) {
> + if (desc[i] == '_' || desc[i] == '\0')
> + break;
> + if (desc[i] == ext)
> + return true;
> + }
>
Reviewed-by: Padmarao Begari
> }
>
> return false;
> --
> 2.34.1
>
Re: [PATCH 1/1] cmd: remove superfluous if in eficonfig_edit_boot_option
On Thu, 10 Nov 2022 at 13:37, Heinrich Schuchardt
wrote:
>
> Goto for an immediately succeeding label is superfluous.
>
> Fixes: 87d791423ac6 ("eficonfig: menu-driven addition of UEFI boot option")
> Addresses-Coverity: 376202 ("Identical code for different branches")
> Signed-off-by: Heinrich Schuchardt
> ---
> cmd/eficonfig.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> index 2595dd9563..0dc6e7b01e 100644
> --- a/cmd/eficonfig.c
> +++ b/cmd/eficonfig.c
> @@ -1527,8 +1527,6 @@ static efi_status_t eficonfig_edit_boot_option(u16
> *varname, struct eficonfig_bo
> }
>
> ret = eficonfig_set_boot_option(varname, final_dp, final_dp_size,
> bo->description, tmp);
> - if (ret != EFI_SUCCESS)
> - goto out;
> out:
> free(tmp);
> free(bo->optional_data);
> --
> 2.37.2
>
Reviewed-by: Ilias Apalodimas
[PATCH 1/1] cmd: remove superfluous if in eficonfig_edit_boot_option
Goto for an immediately succeeding label is superfluous.
Fixes: 87d791423ac6 ("eficonfig: menu-driven addition of UEFI boot option")
Addresses-Coverity: 376202 ("Identical code for different branches")
Signed-off-by: Heinrich Schuchardt
---
cmd/eficonfig.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index 2595dd9563..0dc6e7b01e 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -1527,8 +1527,6 @@ static efi_status_t eficonfig_edit_boot_option(u16
*varname, struct eficonfig_bo
}
ret = eficonfig_set_boot_option(varname, final_dp, final_dp_size,
bo->description, tmp);
- if (ret != EFI_SUCCESS)
- goto out;
out:
free(tmp);
free(bo->optional_data);
--
2.37.2
Re: [PATCH 1/1] MAINTAINERS: add UEFI commands to EFI PAYLOAD
On Thu, 10 Nov 2022 at 13:29, Heinrich Schuchardt wrote: > > Add the following files to EFI PAYLOAD: > > - cmd/bootefi.c > - cmd/efidebug.c > - cmd/eficonfig.c > - cmd/nvedit_efi.c > > Signed-off-by: Heinrich Schuchardt > --- > MAINTAINERS | 4 > 1 file changed, 4 insertions(+) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 8d3d528650..141a00edf1 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -880,6 +880,10 @@ M: Ilias Apalodimas > S: Maintained > T: git https://source.denx.de/u-boot/custodians/u-boot-efi.git > F: arch/arm/lib/*_efi.* > +F: cmd/bootefi.c > +F: cmd/eficonfig.c > +F: cmd/efidebug.c > +F: cmd/nvedit_efi.c > F: doc/api/efi.rst > F: doc/develop/uefi/* > F: doc/mkeficapsule.1 > -- > 2.37.2 > Reviewed-by: Ilias Apalodimas
[PATCH] efi_loader: simplify efi_load_from_path
The current implementation efi_load_from_path is a bit confusing.
First of all it tries to check the device path to make sure it contains
the path to the device plus the media path with the filename and nothing in
between. But that should already be valid since U-Boot constructs those
device paths.
On top of that it tries to traverse the device path nodes and acquire the
file part by stepping through the nodes of the directory path until the
file is reached. We already have efi_dp_split_file_path() for that so
rewrite the function and clean it up to use existing code.
Signed-off-by: Ilias Apalodimas
---
lib/efi_loader/efi_file.c | 52 ---
1 file changed, 16 insertions(+), 36 deletions(-)
diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c
index c96a7f7ca371..e1695ba309e4 100644
--- a/lib/efi_loader/efi_file.c
+++ b/lib/efi_loader/efi_file.c
@@ -1104,53 +1104,33 @@ static const struct efi_file_handle
efi_file_handle_protocol = {
struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp)
{
struct efi_simple_file_system_protocol *v;
- struct efi_file_handle *f;
+ struct efi_device_path_file_path *mdp;
+ struct efi_device_path *dev_dp, *file_dp;
+ struct efi_file_handle *vol_handle, *file_handle = NULL;
efi_status_t ret;
v = efi_fs_from_path(fp);
if (!v)
return NULL;
- EFI_CALL(ret = v->open_volume(v, ));
+ ret = efi_dp_split_file_path(fp, _dp, _dp);
if (ret != EFI_SUCCESS)
return NULL;
+ efi_free_pool(dev_dp);
- /* Skip over device-path nodes before the file path. */
- while (fp && !EFI_DP_TYPE(fp, MEDIA_DEVICE, FILE_PATH))
- fp = efi_dp_next(fp);
-
- /*
-* Step through the nodes of the directory path until the actual file
-* node is reached which is the final node in the device path.
-*/
- while (fp) {
- struct efi_device_path_file_path *fdp =
- container_of(fp, struct efi_device_path_file_path, dp);
- struct efi_file_handle *f2;
- u16 *filename;
-
- if (!EFI_DP_TYPE(fp, MEDIA_DEVICE, FILE_PATH)) {
- printf("bad file path!\n");
- f->close(f);
- return NULL;
- }
-
- filename = u16_strdup(fdp->str);
- if (!filename)
- return NULL;
- EFI_CALL(ret = f->open(f, , filename,
- EFI_FILE_MODE_READ, 0));
- free(filename);
- if (ret != EFI_SUCCESS)
- return NULL;
-
- fp = efi_dp_next(fp);
-
- EFI_CALL(f->close(f));
- f = f2;
+ ret = efi_open_volume_int(v, _handle);
+ if (ret != EFI_SUCCESS) {
+ efi_free_pool(file_dp);
+ return NULL;
}
- return f;
+ mdp = (struct efi_device_path_file_path *)file_dp;
+ /* we don't really care about the ret here *file_handle will be NULL */
+ ret = efi_file_open_int(vol_handle, _handle, mdp->str,
EFI_FILE_MODE_READ, 0);
+ efi_free_pool(file_dp);
+ efi_file_close_int(vol_handle);
+
+ return file_handle;
}
efi_status_t efi_open_volume_int(struct efi_simple_file_system_protocol *this,
--
2.38.1
[PATCH 1/1] MAINTAINERS: add UEFI commands to EFI PAYLOAD
Add the following files to EFI PAYLOAD: - cmd/bootefi.c - cmd/efidebug.c - cmd/eficonfig.c - cmd/nvedit_efi.c Signed-off-by: Heinrich Schuchardt --- MAINTAINERS | 4 1 file changed, 4 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index 8d3d528650..141a00edf1 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -880,6 +880,10 @@ M: Ilias Apalodimas S: Maintained T: git https://source.denx.de/u-boot/custodians/u-boot-efi.git F: arch/arm/lib/*_efi.* +F: cmd/bootefi.c +F: cmd/eficonfig.c +F: cmd/efidebug.c +F: cmd/nvedit_efi.c F: doc/api/efi.rst F: doc/develop/uefi/* F: doc/mkeficapsule.1 -- 2.37.2
[PATCH] mtd: spi-nor-ids: Add identity for GigaDevice GD25LQ128E
Add the Gigadevice GD25LQ128E identifers so it can be properly
used.
Datasheet: https://www.gigadevice.com/datasheet/gd25lq128e/
Signed-off-by: Peter Robinson
---
drivers/mtd/spi/spi-nor-ids.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
index 4fe8b0d92c4..d9dea7463f0 100644
--- a/drivers/mtd/spi/spi-nor-ids.c
+++ b/drivers/mtd/spi/spi-nor-ids.c
@@ -123,6 +123,11 @@ const struct flash_info spi_nor_ids[] = {
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
},
+ {
+ INFO("gd25lq128e", 0x257018, 0, 64 * 1024, 256,
+ SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
+ SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
+ },
{
INFO("gd25lq256d", 0xc86019, 0, 64 * 1024, 512,
SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
--
2.38.1
[PATCH 7/8] env: mmc: add debug message when mmc-env-partition is not found
Add a debug message to indicate a potential issue when
"u-boot,mmc-env-partition" is present in config node of device tree
but this partition name is not found in the mmc device.
Signed-off-by: Patrick Delaunay
---
env/mmc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/env/mmc.c b/env/mmc.c
index bd7d51e6b633..8941e0f5ff39 100644
--- a/env/mmc.c
+++ b/env/mmc.c
@@ -120,6 +120,7 @@ static inline s64 mmc_offset(int copy)
err = mmc_offset_try_partition(str, copy, );
if (!err)
return val;
+ debug("env partition '%s' not found (%d)", str, err);
}
/* try the GPT partition with "U-Boot ENV" TYPE GUID */
--
2.25.1
[PATCH 8/8] env: mmc: cosmetic: remove unused macro STR(X)
Remove the unused macro STR(X) since the commit 2b2f727500dc ("env: mmc:
allow support of mmc_get_env_dev with OF_CONTROL")
Signed-off-by: Patrick Delaunay
---
env/mmc.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/env/mmc.c b/env/mmc.c
index 8941e0f5ff39..85761417f283 100644
--- a/env/mmc.c
+++ b/env/mmc.c
@@ -21,9 +21,6 @@
#include
#include
-#define __STR(X) #X
-#define STR(X) __STR(X)
-
#define ENV_MMC_INVALID_OFFSET ((s64)-1)
#if defined(CONFIG_ENV_MMC_USE_DT)
--
2.25.1
[PATCH 6/8] env: mmc: select GPT env partition by type guid
Since commit c0364ce1c695 ("doc/README.gpt: define partition type GUID for
U-Boot environment"), a specific type GUID can be used to indicate
the U-Boot environment partition on the device with GPT partition table.
This patch uses this type GUID to found the env partition as fallback
when the partition name property "u-boot,mmc-env-partition" is not present
in config node or if the indicated partition name is not found.
The mmc_offset_try_partition() function is reused, it selects the first
partition with the correct type GUID when the parameter 'str' is NULL.
Signed-off-by: Patrick Delaunay
---
env/mmc.c | 19 ++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/env/mmc.c b/env/mmc.c
index 1894b6483220..bd7d51e6b633 100644
--- a/env/mmc.c
+++ b/env/mmc.c
@@ -74,8 +74,18 @@ static inline int mmc_offset_try_partition(const char *str,
int copy, s64 *val)
if (ret < 0)
return ret;
- if (!strncmp((const char *)info.name, str, sizeof(info.name)))
+ if (str && !strncmp((const char *)info.name, str,
sizeof(info.name)))
break;
+#ifdef CONFIG_PARTITION_TYPE_GUID
+ if (!str) {
+ const efi_guid_t env_guid =
PARTITION_U_BOOT_ENVIRONMENT;
+ efi_guid_t type_guid;
+
+ uuid_str_to_bin(info.type_guid, type_guid.b,
UUID_STR_FORMAT_GUID);
+ if (!memcmp(_guid, _guid, sizeof(efi_guid_t)))
+ break;
+ }
+#endif
}
/* round up to info.blksz */
@@ -112,6 +122,13 @@ static inline s64 mmc_offset(int copy)
return val;
}
+ /* try the GPT partition with "U-Boot ENV" TYPE GUID */
+ if (IS_ENABLED(CONFIG_PARTITION_TYPE_GUID)) {
+ err = mmc_offset_try_partition(NULL, copy, );
+ if (!err)
+ return val;
+ }
+
defvalue = ENV_MMC_OFFSET;
propname = dt_prop.offset;
--
2.25.1
[PATCH 5/8] configs: stm32mp: activate CONFIG_ENV_MMC_USE_DT
Activate by default CONFIG_ENV_MMC_USE_DT as "u-boot,mmc-env-partition" should be always use in STMicroelectronics boards device tree to locate the environment for mmc backend. The 2 defines: CONFIG_ENV_OFFSET=0x28 CONFIG_ENV_OFFSET_REDUND=0x2C are only valid for spi-nor and not for SD-Card or eMMC. Signed-off-by: Patrick Delaunay --- configs/stm32mp13_defconfig | 1 + configs/stm32mp15_basic_defconfig | 1 + configs/stm32mp15_defconfig | 1 + configs/stm32mp15_trusted_defconfig | 1 + 4 files changed, 4 insertions(+) diff --git a/configs/stm32mp13_defconfig b/configs/stm32mp13_defconfig index af6b1839d039..4cab07647349 100644 --- a/configs/stm32mp13_defconfig +++ b/configs/stm32mp13_defconfig @@ -46,6 +46,7 @@ CONFIG_ENV_IS_IN_MMC=y CONFIG_SYS_REDUNDAND_ENVIRONMENT=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_SYS_MMC_ENV_DEV=-1 +CONFIG_ENV_MMC_USE_DT=y CONFIG_CLK_SCMI=y CONFIG_GPIO_HOG=y CONFIG_DM_I2C=y diff --git a/configs/stm32mp15_basic_defconfig b/configs/stm32mp15_basic_defconfig index 86ebbef0a6c8..4a96ad22bcc8 100644 --- a/configs/stm32mp15_basic_defconfig +++ b/configs/stm32mp15_basic_defconfig @@ -91,6 +91,7 @@ CONFIG_ENV_UBI_VOLUME="uboot_config" CONFIG_ENV_UBI_VOLUME_REDUND="uboot_config_r" CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_SYS_MMC_ENV_DEV=-1 +CONFIG_ENV_MMC_USE_DT=y # CONFIG_SPL_ENV_IS_NOWHERE is not set # CONFIG_SPL_ENV_IS_IN_SPI_FLASH is not set CONFIG_TFTP_TSIZE=y diff --git a/configs/stm32mp15_defconfig b/configs/stm32mp15_defconfig index caa79e68834f..151981849de9 100644 --- a/configs/stm32mp15_defconfig +++ b/configs/stm32mp15_defconfig @@ -65,6 +65,7 @@ CONFIG_ENV_UBI_VOLUME="uboot_config" CONFIG_ENV_UBI_VOLUME_REDUND="uboot_config_r" CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_SYS_MMC_ENV_DEV=-1 +CONFIG_ENV_MMC_USE_DT=y CONFIG_TFTP_TSIZE=y CONFIG_STM32_ADC=y CONFIG_CLK_SCMI=y diff --git a/configs/stm32mp15_trusted_defconfig b/configs/stm32mp15_trusted_defconfig index 3309c2e79246..098eedc9b727 100644 --- a/configs/stm32mp15_trusted_defconfig +++ b/configs/stm32mp15_trusted_defconfig @@ -66,6 +66,7 @@ CONFIG_ENV_UBI_VOLUME="uboot_config" CONFIG_ENV_UBI_VOLUME_REDUND="uboot_config_r" CONFIG_SYS_RELOC_GD_ENV_ADDR=y CONFIG_SYS_MMC_ENV_DEV=-1 +CONFIG_ENV_MMC_USE_DT=y CONFIG_TFTP_TSIZE=y CONFIG_STM32_ADC=y CONFIG_CLK_SCMI=y -- 2.25.1
[PATCH 4/8] env: mmc: add CONFIG_ENV_MMC_USE_DT
Add a new config CONFIG_ENV_MMC_USE_DT to force configuration of the U-Boot environment offset with device tree config node. This patch avoids issues when several CONFIG_ENV_IS_IN_XXX are activated, the defconfig file uses the same value for CONFIG_ENV_OFFSET or CONFIG_ENV_OFFSET_REDUND for the several ENV backends (SPI_FLASH, EEPROM NAND, SATA, MMC). After this patch a bad offset value is not possible when the selected partition in device tree is not found. Signed-off-by: Patrick Delaunay --- env/Kconfig | 16 env/mmc.c | 7 +++ 2 files changed, 23 insertions(+) diff --git a/env/Kconfig b/env/Kconfig index 24111dfaf47b..f8ee99052b97 100644 --- a/env/Kconfig +++ b/env/Kconfig @@ -242,6 +242,13 @@ config ENV_IS_IN_MMC This value is also in units of bytes, but must also be aligned to an MMC sector boundary. + CONFIG_ENV_MMC_USE_DT (optional): + + These define forces the configuration by the config node in device + tree with partition name: "u-boot,mmc-env-partition" or with + offset: "u-boot,mmc-env-offset", "u-boot,mmc-env-offset-redundant". + CONFIG_ENV_OFFSET and CONFIG_ENV_OFFSET_REDUND are not used. + config ENV_IS_IN_NAND bool "Environment in a NAND device" depends on !CHAIN_OF_TRUST @@ -650,6 +657,15 @@ config SYS_MMC_ENV_PART partition 0 or the first boot partition, which is 1 or some other defined partition. +config ENV_MMC_USE_DT + bool "Read partition name and offset in DT" + depends on ENV_IS_IN_MMC && OF_CONTROL + help + Only use the device tree to get the environment location in MMC + device, with partition name or with offset. + The 2 defines CONFIG_ENV_OFFSET, CONFIG_ENV_OFFSET_REDUND + are not used as fallback. + config USE_DEFAULT_ENV_FILE bool "Create default environment from file" help diff --git a/env/mmc.c b/env/mmc.c index 661a268ea07d..1894b6483220 100644 --- a/env/mmc.c +++ b/env/mmc.c @@ -26,6 +26,12 @@ #define ENV_MMC_INVALID_OFFSET ((s64)-1) +#if defined(CONFIG_ENV_MMC_USE_DT) +/* ENV offset is invalid when not defined in Device Tree */ +#define ENV_MMC_OFFSET ENV_MMC_INVALID_OFFSET +#define ENV_MMC_OFFSET_REDUND ENV_MMC_INVALID_OFFSET + +#else /* Default ENV offset when not defined in Device Tree */ #define ENV_MMC_OFFSET CONFIG_ENV_OFFSET @@ -34,6 +40,7 @@ #else #define ENV_MMC_OFFSET_REDUND ENV_MMC_INVALID_OFFSET #endif +#endif DECLARE_GLOBAL_DATA_PTR; -- 2.25.1
[PATCH 2/8] env: mcc: Drop unnecessary #ifdefs
This file has a lot of conditional code and much of it is unnecessary.
Clean this up to reduce the number of build combinations.
This patch replaces the test on CONFIG_ENV_OFFSET_REDUND for the
more coherent CONFIG_SYS_REDUNDAND_ENVIRONMENT.
This patch also corrects a compilation issue in init_mmc_for_env()
when CONFIG_SYS_MMC_ENV_PART is not activated, env_mmc_orig_hwpart is
not defined.
Signed-off-by: Patrick Delaunay
---
env/mmc.c | 120 +-
1 file changed, 64 insertions(+), 56 deletions(-)
diff --git a/env/mmc.c b/env/mmc.c
index 42bcf7e775cc..b36bd9ad77ee 100644
--- a/env/mmc.c
+++ b/env/mmc.c
@@ -46,7 +46,7 @@ DECLARE_GLOBAL_DATA_PTR;
#if (defined(CONFIG_SYS_REDUNDAND_ENVIRONMENT) && \
(CONFIG_SYS_MMC_ENV_PART == 1) && \
(CONFIG_ENV_OFFSET == CONFIG_ENV_OFFSET_REDUND))
-#define ENV_MMC_HWPART_REDUND
+#define ENV_MMC_HWPART_REDUND 1
#endif
#if CONFIG_IS_ENABLED(OF_CONTROL)
@@ -108,12 +108,11 @@ static inline s64 mmc_offset(int copy)
defvalue = ENV_MMC_OFFSET;
propname = dt_prop.offset;
-#if defined(CONFIG_ENV_OFFSET_REDUND)
- if (copy) {
+ if (IS_ENABLED(CONFIG_SYS_REDUNDAND_ENVIRONMENT) && copy) {
defvalue = ENV_MMC_OFFSET_REDUND;
propname = dt_prop.offset_redund;
}
-#endif
+
return ofnode_conf_read_int(propname, defvalue);
}
#else
@@ -121,10 +120,9 @@ static inline s64 mmc_offset(int copy)
{
s64 offset = ENV_MMC_OFFSET;
-#if defined(CONFIG_ENV_OFFSET_REDUND)
- if (copy)
+ if (IS_ENABLED(CONFIG_SYS_REDUNDAND_ENVIRONMENT) && copy)
offset = ENV_MMC_OFFSET_REDUND;
-#endif
+
return offset;
}
#endif
@@ -165,8 +163,24 @@ static int mmc_set_env_part(struct mmc *mmc, uint part)
return ret;
}
+
+static bool mmc_set_env_part_init(struct mmc *mmc)
+{
+ env_mmc_orig_hwpart = mmc_get_blk_desc(mmc)->hwpart;
+ if (mmc_set_env_part(mmc, mmc_get_env_part(mmc)))
+ return false;
+
+ return true;
+}
+
+static int mmc_set_env_part_restore(struct mmc *mmc)
+{
+ return mmc_set_env_part(mmc, env_mmc_orig_hwpart);
+}
#else
static inline int mmc_set_env_part(struct mmc *mmc, uint part) {return 0; };
+static bool mmc_set_env_part_init(struct mmc *mmc) {return true; }
+static inline int mmc_set_env_part_restore(struct mmc *mmc) {return 0; };
#endif
static const char *init_mmc_for_env(struct mmc *mmc)
@@ -183,8 +197,7 @@ static const char *init_mmc_for_env(struct mmc *mmc)
if (mmc_init(mmc))
return "MMC init failed";
#endif
- env_mmc_orig_hwpart = mmc_get_blk_desc(mmc)->hwpart;
- if (mmc_set_env_part(mmc, mmc_get_env_part(mmc)))
+ if (!mmc_set_env_part_init(mmc))
return "MMC partition switch failed";
return NULL;
@@ -192,11 +205,7 @@ static const char *init_mmc_for_env(struct mmc *mmc)
static void fini_mmc_for_env(struct mmc *mmc)
{
-#ifdef CONFIG_SYS_MMC_ENV_PART
- int dev = mmc_get_env_dev();
-
- blk_select_hwpart_devnum(UCLASS_MMC, dev, env_mmc_orig_hwpart);
-#endif
+ mmc_set_env_part_restore(mmc);
}
#if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_SPL_BUILD)
@@ -233,21 +242,20 @@ static int env_mmc_save(void)
if (ret)
goto fini;
-#ifdef CONFIG_ENV_OFFSET_REDUND
- if (gd->env_valid == ENV_VALID)
- copy = 1;
-
-#ifdef ENV_MMC_HWPART_REDUND
- ret = mmc_set_env_part(mmc, copy + 1);
- if (ret)
- goto fini;
-#endif
+ if (IS_ENABLED(CONFIG_SYS_REDUNDAND_ENVIRONMENT)) {
+ if (gd->env_valid == ENV_VALID)
+ copy = 1;
-#endif
+ if (IS_ENABLED(ENV_MMC_HWPART_REDUND)) {
+ ret = mmc_set_env_part(mmc, copy + 1);
+ if (ret)
+ goto fini;
+ }
- if (mmc_get_env_addr(mmc, copy, )) {
- ret = 1;
- goto fini;
+ if (mmc_get_env_addr(mmc, copy, )) {
+ ret = 1;
+ goto fini;
+ }
}
printf("Writing to %sMMC(%d)... ", copy ? "redundant " : "", dev);
@@ -259,12 +267,12 @@ static int env_mmc_save(void)
ret = 0;
-#ifdef CONFIG_ENV_OFFSET_REDUND
- gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND;
-#endif
+ if (IS_ENABLED(CONFIG_SYS_REDUNDAND_ENVIRONMENT))
+ gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID :
ENV_REDUND;
fini:
fini_mmc_for_env(mmc);
+
return ret;
}
@@ -308,22 +316,22 @@ static int env_mmc_erase(void)
printf("\n");
ret = erase_env(mmc, CONFIG_ENV_SIZE, offset);
-#ifdef CONFIG_ENV_OFFSET_REDUND
- copy = 1;
+ if (IS_ENABLED(CONFIG_SYS_REDUNDAND_ENVIRONMENT)) {
+ copy = 1;
-#ifdef ENV_MMC_HWPART_REDUND
- ret =
[PATCH 3/8] env: mcc: fix compilation error with ENV_IS_EMBEDDED
When ENV_IS_EMBEDDED is enabled, ret is not defined but is used as a
return value in env_mmc_load().
This patch correct this issue and simplify the existing code, test only
one time #if defined(ENV_IS_EMBEDDED) and not in the function.
Signed-off-by: Patrick Delaunay
---
env/mmc.c | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/env/mmc.c b/env/mmc.c
index b36bd9ad77ee..661a268ea07d 100644
--- a/env/mmc.c
+++ b/env/mmc.c
@@ -353,10 +353,14 @@ static inline int read_env(struct mmc *mmc, unsigned long
size,
return (n == blk_cnt) ? 0 : -1;
}
-#if defined(CONFIG_SYS_REDUNDAND_ENVIRONMENT)
+#if defined(ENV_IS_EMBEDDED)
+static int env_mmc_load(void)
+{
+ return 0;
+}
+#elif defined(CONFIG_SYS_REDUNDAND_ENVIRONMENT)
static int env_mmc_load(void)
{
-#if !defined(ENV_IS_EMBEDDED)
struct mmc *mmc;
u32 offset1, offset2;
int read1_fail = 0, read2_fail = 0;
@@ -408,13 +412,11 @@ err:
if (ret)
env_set_default(errmsg, 0);
-#endif
return ret;
}
#else /* ! CONFIG_SYS_REDUNDAND_ENVIRONMENT */
static int env_mmc_load(void)
{
-#if !defined(ENV_IS_EMBEDDED)
ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE);
struct mmc *mmc;
u32 offset;
@@ -453,7 +455,7 @@ fini:
err:
if (ret)
env_set_default(errmsg, 0);
-#endif
+
return ret;
}
#endif /* CONFIG_SYS_REDUNDAND_ENVIRONMENT */
--
2.25.1
[PATCH 1/8] env: mmc: introduced ENV_MMC_OFFSET
Introduce ENV_MMC_OFFSET defines.
It is a preliminary step to the next patches to simplify the code.
Signed-off-by: Patrick Delaunay
---
env/mmc.c | 24
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/env/mmc.c b/env/mmc.c
index c28f4c6c6dc0..42bcf7e775cc 100644
--- a/env/mmc.c
+++ b/env/mmc.c
@@ -24,6 +24,17 @@
#define __STR(X) #X
#define STR(X) __STR(X)
+#define ENV_MMC_INVALID_OFFSET ((s64)-1)
+
+/* Default ENV offset when not defined in Device Tree */
+#define ENV_MMC_OFFSET CONFIG_ENV_OFFSET
+
+#if defined(CONFIG_ENV_OFFSET_REDUND)
+#define ENV_MMC_OFFSET_REDUND CONFIG_ENV_OFFSET_REDUND
+#else
+#define ENV_MMC_OFFSET_REDUND ENV_MMC_INVALID_OFFSET
+#endif
+
DECLARE_GLOBAL_DATA_PTR;
/*
@@ -94,12 +105,12 @@ static inline s64 mmc_offset(int copy)
return val;
}
- defvalue = CONFIG_ENV_OFFSET;
+ defvalue = ENV_MMC_OFFSET;
propname = dt_prop.offset;
#if defined(CONFIG_ENV_OFFSET_REDUND)
if (copy) {
- defvalue = CONFIG_ENV_OFFSET_REDUND;
+ defvalue = ENV_MMC_OFFSET_REDUND;
propname = dt_prop.offset_redund;
}
#endif
@@ -108,11 +119,11 @@ static inline s64 mmc_offset(int copy)
#else
static inline s64 mmc_offset(int copy)
{
- s64 offset = CONFIG_ENV_OFFSET;
+ s64 offset = ENV_MMC_OFFSET;
#if defined(CONFIG_ENV_OFFSET_REDUND)
if (copy)
- offset = CONFIG_ENV_OFFSET_REDUND;
+ offset = ENV_MMC_OFFSET_REDUND;
#endif
return offset;
}
@@ -122,6 +133,11 @@ __weak int mmc_get_env_addr(struct mmc *mmc, int copy, u32
*env_addr)
{
s64 offset = mmc_offset(copy);
+ if (offset == ENV_MMC_INVALID_OFFSET) {
+ printf("Invalid ENV offset in MMC, copy=%d\n", copy);
+ return -ENOENT;
+ }
+
if (offset < 0)
offset += mmc->capacity;
--
2.25.1
[PATCH 0/8] env: mmc: improvements and corrections
Update in U-Boot env mmc backend with several cosmetic changes or
corrections and 2 new features:
1/ CONFIG_ENV_MMC_USE_DT = no more use CONFIG_ENV_OFFSET
in the mmc ENV backend when this config is activated.
Requested by the STM32MP STMicroelectronics boards which activate
several ENV_IS_IN_XXX; the value of CONFIG_ENV_OFFSET is invalid for
SD-Card / eMMC boot; this offset should only used in SPIFlash backend
(sf.c) for SPI-NOR boot.
If this offset is used on mmc backend, when partition name in GPT is
not aligned with U-Boot DT: "u-boot,mmc-env-partition", the behavior
is difficult to debug: a partition is corrupted on 'env save' command.
2/ selects the GPT env partition by the "u-boot-env" type GUID introduced
by the commit c0364ce1c695 ("doc/README.gpt: define partition
type GUID for U-Boot environment")
This feature can also avoid issue when 'u-boot-env' partition name
change in GPT partitioning but not in the U-Boot DT with
"u-boot,mmc-env-partition"
Few check patch warnings remained in the series,
but after check I can't remove them :
- IS_ENABLED(ENV_MMC_HWPART_REDUND) is normally used as
IS_ENABLED(CONFIG_ENV_MMC_HWPART_REDUND)
=> ENV_MMC_HWPART_REDUND is locally defined in this file it is not
a real CONFIG but I can use the IS_ENABLED() macro as it is defined
to 1
- Use 'if (IS_ENABLED(CONFIG...))' instead of '#if or #ifdef' where
possible
+ CONFIG_PARTITION_TYPE_GUID => info.type_guid existence
+ CONFIG_ENV_OFFSET_REDUND and CONFIG_ENV_MMC_USE_DT => only for define
As I miss the merge window, not targeted for v2023.01 but for next
v2023.04.
Regards
Patrick
Patrick Delaunay (8):
env: mmc: introduced ENV_MMC_OFFSET
env: mcc: Drop unnecessary #ifdefs
env: mcc: fix compilation error with ENV_IS_EMBEDDED
env: mmc: add CONFIG_ENV_MMC_USE_DT
configs: stm32mp: activate CONFIG_ENV_MMC_USE_DT
env: mmc: select GPT env partition by type guid
env: mmc: add debug message when mmc-env-partition is not found
env: mmc: cosmetic: remove unused macro STR(X)
configs/stm32mp13_defconfig | 1 +
configs/stm32mp15_basic_defconfig | 1 +
configs/stm32mp15_defconfig | 1 +
configs/stm32mp15_trusted_defconfig | 1 +
env/Kconfig | 16 +++
env/mmc.c | 182 ++--
6 files changed, 135 insertions(+), 67 deletions(-)
--
2.25.1
[PATCH v5 3/3] rockpi4: capsule: Enable UEFI capsule update on RockPi4 boards
Enable the UEFI capsule update functionality on the RockPi4B and RockPi4C boards. Support is being enabled for updating the idbloader and u-boot firmware images residing on GPT partitioned uSD card storage device. Signed-off-by: Sughosh Ganu Reviewed-by: Kever Yang --- Changes since V5: None configs/rock-pi-4-rk3399_defconfig | 8 configs/rock-pi-4c-rk3399_defconfig | 8 2 files changed, 16 insertions(+) diff --git a/configs/rock-pi-4-rk3399_defconfig b/configs/rock-pi-4-rk3399_defconfig index 83721cedf3..b46f3f8c76 100644 --- a/configs/rock-pi-4-rk3399_defconfig +++ b/configs/rock-pi-4-rk3399_defconfig @@ -30,6 +30,7 @@ CONFIG_SPL_STACK_R=y CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x1 CONFIG_TPL=y CONFIG_CMD_BOOTZ=y +CONFIG_CMD_DFU=y CONFIG_CMD_GPT=y CONFIG_CMD_MMC=y CONFIG_CMD_PCI=y @@ -40,6 +41,7 @@ CONFIG_SPL_OF_CONTROL=y CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clock-names interrupt-parent assigned-clocks assigned-clock-rates assigned-clock-parents" CONFIG_ENV_IS_IN_MMC=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y +CONFIG_DFU_MMC=y CONFIG_ROCKCHIP_GPIO=y CONFIG_SYS_I2C_ROCKCHIP=y CONFIG_MISC=y @@ -84,3 +86,9 @@ CONFIG_VIDEO_ROCKCHIP=y CONFIG_DISPLAY_ROCKCHIP_HDMI=y CONFIG_SPL_TINY_MEMSET=y CONFIG_ERRNO_STR=y +CONFIG_CMD_NVEDIT_EFI=y +CONFIG_CMD_EFIDEBUG=y +CONFIG_TOOLS_MKEFICAPSULE=y +CONFIG_HEXDUMP=y +CONFIG_EFI_CAPSULE_ON_DISK=y +CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y diff --git a/configs/rock-pi-4c-rk3399_defconfig b/configs/rock-pi-4c-rk3399_defconfig index ac9a3f9830..7075286a21 100644 --- a/configs/rock-pi-4c-rk3399_defconfig +++ b/configs/rock-pi-4c-rk3399_defconfig @@ -30,6 +30,7 @@ CONFIG_SPL_STACK_R=y CONFIG_SPL_STACK_R_MALLOC_SIMPLE_LEN=0x1 CONFIG_TPL=y CONFIG_CMD_BOOTZ=y +CONFIG_CMD_DFU=y CONFIG_CMD_GPT=y CONFIG_CMD_MMC=y CONFIG_CMD_PCI=y @@ -40,6 +41,7 @@ CONFIG_SPL_OF_CONTROL=y CONFIG_OF_SPL_REMOVE_PROPS="pinctrl-0 pinctrl-names clock-names interrupt-parent assigned-clocks assigned-clock-rates assigned-clock-parents" CONFIG_ENV_IS_IN_MMC=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y +CONFIG_DFU_MMC=y CONFIG_ROCKCHIP_GPIO=y CONFIG_SYS_I2C_ROCKCHIP=y CONFIG_MISC=y @@ -84,3 +86,9 @@ CONFIG_VIDEO_ROCKCHIP=y CONFIG_DISPLAY_ROCKCHIP_HDMI=y CONFIG_SPL_TINY_MEMSET=y CONFIG_ERRNO_STR=y +CONFIG_CMD_NVEDIT_EFI=y +CONFIG_CMD_EFIDEBUG=y +CONFIG_TOOLS_MKEFICAPSULE=y +CONFIG_HEXDUMP=y +CONFIG_EFI_CAPSULE_ON_DISK=y +CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y -- 2.34.1
[PATCH v5 2/3] rockpi4: board: Add firmware image information for capsule updates
Add information that will be needed for enabling the UEFI capsule
update feature on the RockPi4 boards. With the feature enabled, it
would be possible to update the idbloader and u-boot.itb images on the
RockPi4B and RockPi4C variants.
Signed-off-by: Sughosh Ganu
Reviewed-by: Kever Yang
---
Changes since V5:
* Mark fw_images array as a static variable
* Populate num_image_type_guids through ROCKPI4_UPDATABLE_IMAGES
arch/arm/include/asm/arch-rockchip/misc.h | 1 +
board/rockchip/evb_rk3399/evb-rk3399.c| 55 ++-
include/configs/rk3399_common.h | 16 +++
3 files changed, 71 insertions(+), 1 deletion(-)
diff --git a/arch/arm/include/asm/arch-rockchip/misc.h
b/arch/arm/include/asm/arch-rockchip/misc.h
index b6b03c934e..4155af8c3b 100644
--- a/arch/arm/include/asm/arch-rockchip/misc.h
+++ b/arch/arm/include/asm/arch-rockchip/misc.h
@@ -11,3 +11,4 @@ int rockchip_cpuid_from_efuse(const u32 cpuid_offset,
u8 *cpuid);
int rockchip_cpuid_set(const u8 *cpuid, const u32 cpuid_length);
int rockchip_setup_macaddr(void);
+void rockchip_capsule_update_board_setup(void);
diff --git a/board/rockchip/evb_rk3399/evb-rk3399.c
b/board/rockchip/evb_rk3399/evb-rk3399.c
index abb76585cf..f56b379b93 100644
--- a/board/rockchip/evb_rk3399/evb-rk3399.c
+++ b/board/rockchip/evb_rk3399/evb-rk3399.c
@@ -5,11 +5,25 @@
#include
#include
+#include
#include
#include
#include
+#include
#include
+#define ROCKPI4_UPDATABLE_IMAGES 2
+
+#if CONFIG_IS_ENABLED(EFI_HAVE_CAPSULE_SUPPORT)
+static struct efi_fw_image fw_images[ROCKPI4_UPDATABLE_IMAGES] = {0};
+
+struct efi_capsule_update_info update_info = {
+ .images = fw_images,
+};
+
+u8 num_image_type_guids = ROCKPI4_UPDATABLE_IMAGES;
+#endif
+
#ifndef CONFIG_SPL_BUILD
int board_early_init_f(void)
{
@@ -29,4 +43,43 @@ int board_early_init_f(void)
out:
return 0;
}
-#endif
+
+#if defined(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) && defined(CONFIG_EFI_PARTITION)
+static bool board_is_rockpi_4b(void)
+{
+ return CONFIG_IS_ENABLED(TARGET_EVB_RK3399) &&
+ of_machine_is_compatible("radxa,rockpi4b");
+}
+
+static bool board_is_rockpi_4c(void)
+{
+ return CONFIG_IS_ENABLED(TARGET_EVB_RK3399) &&
+ of_machine_is_compatible("radxa,rockpi4c");
+}
+
+void rockchip_capsule_update_board_setup(void)
+{
+ if (board_is_rockpi_4b()) {
+ efi_guid_t idbldr_image_type_guid =
+ ROCKPI_4B_IDBLOADER_IMAGE_GUID;
+ efi_guid_t uboot_image_type_guid = ROCKPI_4B_UBOOT_IMAGE_GUID;
+
+ guidcpy(_images[0].image_type_id, _image_type_guid);
+ guidcpy(_images[1].image_type_id, _image_type_guid);
+
+ fw_images[0].fw_name = u"ROCKPI4B-IDBLOADER";
+ fw_images[1].fw_name = u"ROCKPI4B-UBOOT";
+ } else if (board_is_rockpi_4c()) {
+ efi_guid_t idbldr_image_type_guid =
+ ROCKPI_4C_IDBLOADER_IMAGE_GUID;
+ efi_guid_t uboot_image_type_guid = ROCKPI_4C_UBOOT_IMAGE_GUID;
+
+ guidcpy(_images[0].image_type_id, _image_type_guid);
+ guidcpy(_images[1].image_type_id, _image_type_guid);
+
+ fw_images[0].fw_name = u"ROCKPI4C-IDBLOADER";
+ fw_images[1].fw_name = u"ROCKPI4C-UBOOT";
+ }
+}
+#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT && CONFIG_EFI_PARTITION */
+#endif /* !CONFIG_SPL_BUILD */
diff --git a/include/configs/rk3399_common.h b/include/configs/rk3399_common.h
index 2f9aee5819..f0a9ab8f83 100644
--- a/include/configs/rk3399_common.h
+++ b/include/configs/rk3399_common.h
@@ -24,6 +24,22 @@
#define CONFIG_SYS_SDRAM_BASE 0
#define SDRAM_MAX_SIZE 0xf800
+#define ROCKPI_4B_IDBLOADER_IMAGE_GUID \
+ EFI_GUID(0x02f4d760, 0xcfd5, 0x43bd, 0x8e, 0x2d, \
+0xa4, 0x2a, 0xcb, 0x33, 0xc6, 0x60)
+
+#define ROCKPI_4B_UBOOT_IMAGE_GUID \
+ EFI_GUID(0x4ce292da, 0x1dd8, 0x428d, 0xa1, 0xc2, \
+0x77, 0x74, 0x3e, 0xf8, 0xb9, 0x6e)
+
+#define ROCKPI_4C_IDBLOADER_IMAGE_GUID \
+ EFI_GUID(0xfd68510c, 0x12d3, 0x4f0a, 0xb8, 0xd3, \
+0xd8, 0x79, 0xe1, 0xd3, 0xa5, 0x40)
+
+#define ROCKPI_4C_UBOOT_IMAGE_GUID \
+ EFI_GUID(0xb81fb4ae, 0xe4f3, 0x471b, 0x99, 0xb4, \
+0x0b, 0x3d, 0xa5, 0x49, 0xce, 0x13)
+
#ifndef CONFIG_SPL_BUILD
#define ENV_MEM_LAYOUT_SETTINGS \
--
2.34.1
[PATCH v5 1/3] rockchip: capsule: Add functions for supporting capsule updates
Add functions needed to support the UEFI capsule update feature on
rockchip boards. Currently, the feature is being enabled on the
RockPi4 boards with firmware images residing on GPT partitioned
storage media.
Signed-off-by: Sughosh Ganu
Reviewed-by: Kever Yang
---
Changes since V4:
* Get the pointer to struct efi_fw_image through struct
efi_capsule_update_info
arch/arm/mach-rockchip/Kconfig | 1 +
arch/arm/mach-rockchip/board.c | 153 +
2 files changed, 154 insertions(+)
diff --git a/arch/arm/mach-rockchip/Kconfig b/arch/arm/mach-rockchip/Kconfig
index 69d51ff378..4898260017 100644
--- a/arch/arm/mach-rockchip/Kconfig
+++ b/arch/arm/mach-rockchip/Kconfig
@@ -246,6 +246,7 @@ config ROCKCHIP_RK3399
select DM_PMIC
select DM_REGULATOR_FIXED
select BOARD_LATE_INIT
+ imply PARTITION_TYPE_GUID
imply PRE_CONSOLE_BUFFER
imply ROCKCHIP_COMMON_BOARD
imply ROCKCHIP_SDRAM_COMMON
diff --git a/arch/arm/mach-rockchip/board.c b/arch/arm/mach-rockchip/board.c
index cbe00d646c..6e05a8f76e 100644
--- a/arch/arm/mach-rockchip/board.c
+++ b/arch/arm/mach-rockchip/board.c
@@ -6,11 +6,15 @@
#include
#include
#include
+#include
#include
#include
#include
+#include
+#include
#include
#include
+#include
#include
#include
#include
@@ -22,8 +26,157 @@
DECLARE_GLOBAL_DATA_PTR;
+#if defined(CONFIG_EFI_HAVE_CAPSULE_SUPPORT) && defined(CONFIG_EFI_PARTITION)
+
+#define DFU_ALT_BUF_LENSZ_1K
+
+static struct efi_fw_image *fw_images;
+
+static bool updatable_image(struct disk_partition *info)
+{
+ int i;
+ bool ret = false;
+ efi_guid_t image_type_guid;
+
+ uuid_str_to_bin(info->type_guid, image_type_guid.b,
+ UUID_STR_FORMAT_GUID);
+
+ for (i = 0; i < num_image_type_guids; i++) {
+ if (!guidcmp(_images[i].image_type_id, _type_guid)) {
+ ret = true;
+ break;
+ }
+ }
+
+ return ret;
+}
+
+static void set_image_index(struct disk_partition *info, int index)
+{
+ int i;
+ efi_guid_t image_type_guid;
+
+ uuid_str_to_bin(info->type_guid, image_type_guid.b,
+ UUID_STR_FORMAT_GUID);
+
+ for (i = 0; i < num_image_type_guids; i++) {
+ if (!guidcmp(_images[i].image_type_id, _type_guid)) {
+ fw_images[i].image_index = index;
+ break;
+ }
+ }
+}
+
+static int get_mmc_desc(struct blk_desc **desc)
+{
+ int ret;
+ struct mmc *mmc;
+ struct udevice *dev;
+
+ /*
+* For now the firmware images are assumed to
+* be on the SD card
+*/
+ ret = uclass_get_device(UCLASS_MMC, 1, );
+ if (ret)
+ return -1;
+
+ mmc = mmc_get_mmc_dev(dev);
+ if (!mmc)
+ return -ENODEV;
+
+ if ((ret = mmc_init(mmc)))
+ return ret;
+
+ *desc = mmc_get_blk_desc(mmc);
+ if (!*desc)
+ return -1;
+
+ return 0;
+}
+
+void set_dfu_alt_info(char *interface, char *devstr)
+{
+ const char *name;
+ bool first = true;
+ int p, len, devnum, ret;
+ char buf[DFU_ALT_BUF_LEN];
+ struct disk_partition info;
+ struct blk_desc *desc = NULL;
+
+ ret = get_mmc_desc();
+ if (ret) {
+ log_err("Unable to get mmc desc\n");
+ return;
+ }
+
+ memset(buf, 0, sizeof(buf));
+ name = blk_get_uclass_name(desc->uclass_id);
+ devnum = desc->devnum;
+ len = strlen(buf);
+
+ len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
+"%s %d=", name, devnum);
+
+ for (p = 1; p <= MAX_SEARCH_PARTITIONS; p++) {
+ if (part_get_info(desc, p, ))
+ continue;
+
+ /* Add entry to dfu_alt_info only for updatable images */
+ if (updatable_image()) {
+ if (!first)
+ len += snprintf(buf + len,
+ DFU_ALT_BUF_LEN - len, ";");
+
+ len += snprintf(buf + len, DFU_ALT_BUF_LEN - len,
+ "%s%d_%s part %d %d",
+ name, devnum, info.name, devnum, p);
+ first = false;
+ }
+ }
+
+ log_debug("dfu_alt_info => %s\n", buf);
+ env_set("dfu_alt_info", buf);
+}
+
+static void gpt_capsule_update_setup(void)
+{
+ int p, i, ret;
+ struct disk_partition info;
+ struct blk_desc *desc = NULL;
+
+ fw_images = update_info.images;
+ rockchip_capsule_update_board_setup();
+
+ ret = get_mmc_desc();
+ if (ret) {
+ log_err("Unable to get mmc desc\n");
+ return;
+ }
+
+ for (p = 1, i = 1; p <=
[PATCH v5 0/3] rockpi4: Add capsule update support
Add capsule update support for the RockPi4B and RockPi4C boards. Support is being added for updating the idbloader.img and u-boot.itb firmware images on configurations with the firmware images stored on GPT partitioned uSD card device. Changes since V5: * Get the pointer to struct efi_fw_image through struct efi_capsule_update_info * Mark fw_images array as a static variable * Populate num_image_type_guids through ROCKPI4_UPDATABLE_IMAGES Sughosh Ganu (3): rockchip: capsule: Add functions for supporting capsule updates rockpi4: board: Add firmware image information for capsule updates rockpi4: capsule: Enable UEFI capsule update on RockPi4 boards arch/arm/include/asm/arch-rockchip/misc.h | 1 + arch/arm/mach-rockchip/Kconfig| 1 + arch/arm/mach-rockchip/board.c| 153 ++ board/rockchip/evb_rk3399/evb-rk3399.c| 55 +++- configs/rock-pi-4-rk3399_defconfig| 8 ++ configs/rock-pi-4c-rk3399_defconfig | 8 ++ include/configs/rk3399_common.h | 16 +++ 7 files changed, 241 insertions(+), 1 deletion(-) -- 2.34.1
[PATCH v8 5/5] eficonfig: add "Show Signature Database" menu entry
This commit adds the menu-driven interface to show the
signature list content.
Signed-off-by: Masahisa Kojima
---
No update since v7
Changes in v7:
- remove delete signature list feature
user can clear the signature database with signed null key
- rename function name to avoid confusion
- update commit message
Changes in v6:
- update comment
Changes in v2:
- integrate show and delete signature database menu
- add confirmation message before delete
- add function comment
cmd/eficonfig_sbkey.c | 236 ++
1 file changed, 236 insertions(+)
diff --git a/cmd/eficonfig_sbkey.c b/cmd/eficonfig_sbkey.c
index de1d1e8426..8a372b9960 100644
--- a/cmd/eficonfig_sbkey.c
+++ b/cmd/eficonfig_sbkey.c
@@ -17,6 +17,13 @@
#include
#include
+struct eficonfig_sig_data {
+ struct efi_signature_list *esl;
+ struct efi_signature_data *esd;
+ struct list_head list;
+ u16 *varname;
+};
+
enum efi_sbkey_signature_type {
SIG_TYPE_X509 = 0,
SIG_TYPE_HASH,
@@ -157,8 +164,237 @@ out:
return ret;
}
+/**
+ * eficonfig_process_show_siglist() - show signature list content
+ *
+ * @data: pointer to the data for each entry
+ * Return: status code
+ */
+static efi_status_t eficonfig_process_show_siglist(void *data)
+{
+ u32 i;
+ struct eficonfig_sig_data *sg = data;
+
+ puts(ANSI_CURSOR_HIDE);
+ puts(ANSI_CLEAR_CONSOLE);
+ printf(ANSI_CURSOR_POSITION, 1, 1);
+
+ printf("\n ** Show Signature Database (%ls) **\n\n"
+ "Owner GUID:\n"
+ " %pUL\n",
+ sg->varname, sg->esd->signature_owner.b);
+
+ for (i = 0; i < ARRAY_SIZE(sigtype_to_str); i++) {
+ if (!guidcmp(>esl->signature_type,
_to_str[i].sig_type)) {
+ printf("Signature Type:\n"
+ " %s\n", sigtype_to_str[i].str);
+
+ switch (sigtype_to_str[i].type) {
+ case SIG_TYPE_X509:
+ {
+ struct x509_certificate *cert_tmp;
+
+ cert_tmp =
x509_cert_parse(sg->esd->signature_data,
+
sg->esl->signature_size);
+ printf("Subject:\n"
+ " %s\n"
+ "Issuer:\n"
+ " %s\n",
+ cert_tmp->subject, cert_tmp->issuer);
+ break;
+ }
+ case SIG_TYPE_CRL:
+ {
+ u32 hash_size = sg->esl->signature_size -
sizeof(efi_guid_t) -
+ sizeof(struct efi_time);
+ struct efi_time *time =
+ (struct efi_time *)((u8
*)sg->esd->signature_data +
+ hash_size);
+
+ printf("ToBeSignedHash:\n");
+ print_hex_dump(" ", DUMP_PREFIX_NONE, 16,
1,
+ sg->esd->signature_data,
hash_size, false);
+ printf("TimeOfRevocation:\n"
+ " %d-%d-%d %02d:%02d:%02d\n",
+ time->year, time->month, time->day,
+ time->hour, time->minute, time->second);
+ break;
+ }
+ case SIG_TYPE_HASH:
+ {
+ u32 hash_size = sg->esl->signature_size -
sizeof(efi_guid_t);
+
+ printf("Hash:\n");
+ print_hex_dump(" ", DUMP_PREFIX_NONE, 16,
1,
+ sg->esd->signature_data,
hash_size, false);
+ break;
+ }
+ default:
+ eficonfig_print_msg("ERROR! Unsupported
format.");
+ return EFI_INVALID_PARAMETER;
+ }
+ }
+ }
+
+ while (tstc())
+ getchar();
+
+ printf("\n\n Press any key to continue");
+ getchar();
+
+ return EFI_SUCCESS;
+}
+
+/**
+ * prepare_signature_list_menu() - create the signature list menu entry
+ *
+ * @efimenu: pointer to the efimenu structure
+ * @varname: pointer to the variable name
+ * @db:pointer to the variable raw data
+ * @db_size: variable data size
+ * @func: callback of each entry
+ * Return: status code
+ */
+static efi_status_t prepare_signature_list_menu(struct efimenu *efi_menu, void
*varname,
+
[PATCH v8 4/5] eficonfig: add UEFI Secure Boot Key enrollment interface
This commit adds the menu-driven UEFI Secure Boot Key
enrollment interface. User can enroll PK, KEK, db
and dbx by selecting file.
Only the signed EFI Signature List(s) with an authenticated
header, typically '.auth' file, is accepted.
To clear the PK, KEK, db and dbx, user needs to enroll the null key
signed by PK or KEK.
Signed-off-by: Masahisa Kojima
---
Changes in v8:
- fix missing efi_file_close_int() call
Changes in v7:
- only accept .auth file.
- remove creating time based authenticated variable
- update commit message
- use efi_file_size()
Changes in v6:
- use efi_secure_boot_enabled()
- replace with WIN_CERT_REVISION_2_0 from pe.h
- call efi_build_signature_store() to check the valid EFI Signature List
- update comment
Changes in v4:
- add CONFIG_EFI_MM_COMM_TEE dependency
- fix error handling
Changes in v3:
- fix error handling
Changes in v2:
- allow to enroll .esl file
- fix typos
- add function comments
cmd/Makefile | 5 +
cmd/eficonfig.c | 3 +
cmd/eficonfig_sbkey.c | 244 ++
include/efi_config.h | 5 +
4 files changed, 257 insertions(+)
create mode 100644 cmd/eficonfig_sbkey.c
diff --git a/cmd/Makefile b/cmd/Makefile
index 2444d116c0..0b6a96c1d9 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -66,6 +66,11 @@ obj-$(CONFIG_CMD_EEPROM) += eeprom.o
obj-$(CONFIG_EFI) += efi.o
obj-$(CONFIG_CMD_EFIDEBUG) += efidebug.o
obj-$(CONFIG_CMD_EFICONFIG) += eficonfig.o
+ifdef CONFIG_CMD_EFICONFIG
+ifdef CONFIG_EFI_MM_COMM_TEE
+obj-$(CONFIG_EFI_SECURE_BOOT) += eficonfig_sbkey.o
+endif
+endif
obj-$(CONFIG_CMD_ELF) += elf.o
obj-$(CONFIG_CMD_EROFS) += erofs.o
obj-$(CONFIG_HUSH_PARSER) += exit.o
diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index 12babb76c2..d79194794b 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -2435,6 +2435,9 @@ static const struct eficonfig_item
maintenance_menu_items[] = {
{"Edit Boot Option", eficonfig_process_edit_boot_option},
{"Change Boot Order", eficonfig_process_change_boot_order},
{"Delete Boot Option", eficonfig_process_delete_boot_option},
+#if (CONFIG_IS_ENABLED(EFI_SECURE_BOOT) && CONFIG_IS_ENABLED(EFI_MM_COMM_TEE))
+ {"Secure Boot Configuration", eficonfig_process_secure_boot_config},
+#endif
{"Quit", eficonfig_process_quit},
};
diff --git a/cmd/eficonfig_sbkey.c b/cmd/eficonfig_sbkey.c
new file mode 100644
index 00..de1d1e8426
--- /dev/null
+++ b/cmd/eficonfig_sbkey.c
@@ -0,0 +1,244 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Menu-driven UEFI Secure Boot Key Maintenance
+ *
+ * Copyright (c) 2022 Masahisa Kojima, Linaro Limited
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+enum efi_sbkey_signature_type {
+ SIG_TYPE_X509 = 0,
+ SIG_TYPE_HASH,
+ SIG_TYPE_CRL,
+ SIG_TYPE_RSA2048,
+};
+
+struct eficonfig_sigtype_to_str {
+ efi_guid_t sig_type;
+ char *str;
+ enum efi_sbkey_signature_type type;
+};
+
+static const struct eficonfig_sigtype_to_str sigtype_to_str[] = {
+ {EFI_CERT_X509_GUID,"X509", SIG_TYPE_X509},
+ {EFI_CERT_SHA256_GUID, "SHA256", SIG_TYPE_HASH},
+ {EFI_CERT_X509_SHA256_GUID, "X509_SHA256 CRL", SIG_TYPE_CRL},
+ {EFI_CERT_X509_SHA384_GUID, "X509_SHA384 CRL", SIG_TYPE_CRL},
+ {EFI_CERT_X509_SHA512_GUID, "X509_SHA512 CRL", SIG_TYPE_CRL},
+ /* U-Boot does not support the following signature types */
+/* {EFI_CERT_RSA2048_GUID, "RSA2048",
SIG_TYPE_RSA2048}, */
+/* {EFI_CERT_RSA2048_SHA256_GUID, "RSA2048_SHA256",
SIG_TYPE_RSA2048}, */
+/* {EFI_CERT_SHA1_GUID,"SHA1", SIG_TYPE_HASH},
*/
+/* {EFI_CERT_RSA2048_SHA_GUID, "RSA2048_SHA",
SIG_TYPE_RSA2048 }, */
+/* {EFI_CERT_SHA224_GUID, "SHA224", SIG_TYPE_HASH},
*/
+/* {EFI_CERT_SHA384_GUID, "SHA384", SIG_TYPE_HASH},
*/
+/* {EFI_CERT_SHA512_GUID, "SHA512", SIG_TYPE_HASH},
*/
+};
+
+/**
+ * file_have_auth_header() - check file has EFI_VARIABLE_AUTHENTICATION_2
header
+ * @buf: pointer to file
+ * @size: file size
+ * Return: true if file has auth header, false otherwise
+ */
+static bool file_have_auth_header(void *buf, efi_uintn_t size)
+{
+ struct efi_variable_authentication_2 *auth = buf;
+
+ if (auth->auth_info.hdr.wCertificateType != WIN_CERT_TYPE_EFI_GUID)
+ return false;
+
+ if (guidcmp(>auth_info.cert_type, _guid_cert_type_pkcs7))
+ return false;
+
+ return true;
+}
+
+/**
+ * eficonfig_process_enroll_key() - enroll key into signature database
+ *
+ * @data: pointer to the data for each entry
+ * Return: status code
+ */
+static efi_status_t eficonfig_process_enroll_key(void *data)
+{
+ u32
[PATCH v8 3/5] eficonfig: refactor change boot order implementation
All the eficonfig menus other than "Change Boot Order"
use 'eficonfig_entry' structure for each menu entry.
This commit refactors change boot order implementation
to use 'eficonfig_entry' structure same as other menus
to have consistent menu handling.
This commit also simplifies the data->active handling when
KEY_SPACE is pressed, and sizeof() parameter.
Signed-off-by: Masahisa Kojima
Acked-by: Ilias Apalodimas
---
No update since v7
Changes in v7:
- simplify the data->active handling
- update sizeof() parameter
- update commit message
Changes in v5:
- remove direct access mode
newly created in v4
cmd/eficonfig.c | 129 +---
1 file changed, 67 insertions(+), 62 deletions(-)
diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index b392de7954..12babb76c2 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -93,20 +93,14 @@ struct eficonfig_boot_selection_data {
};
/**
- * struct eficonfig_boot_order - structure to be used to update BootOrder
variable
+ * struct eficonfig_boot_order_data - structure to be used to update BootOrder
variable
*
- * @num: index in the menu entry
- * @description: pointer to the description string
* @boot_index:boot option index
* @active:flag to include the boot option into BootOrder variable
- * @list: list structure
*/
-struct eficonfig_boot_order {
- u32 num;
- u16 *description;
+struct eficonfig_boot_order_data {
u32 boot_index;
bool active;
- struct list_head list;
};
/**
@@ -1802,7 +1796,7 @@ static void eficonfig_display_change_boot_order(struct
efimenu *efi_menu)
{
bool reverse;
struct list_head *pos, *n;
- struct eficonfig_boot_order *entry;
+ struct eficonfig_entry *entry;
printf(ANSI_CLEAR_CONSOLE ANSI_CURSOR_POSITION
"\n ** Change Boot Order **\n"
@@ -1818,7 +1812,7 @@ static void eficonfig_display_change_boot_order(struct
efimenu *efi_menu)
/* draw boot option list */
list_for_each_safe(pos, n, _menu->list) {
- entry = list_entry(pos, struct eficonfig_boot_order, list);
+ entry = list_entry(pos, struct eficonfig_entry, list);
reverse = (entry->num == efi_menu->active);
printf(ANSI_CURSOR_POSITION, entry->num + 4, 7);
@@ -1827,13 +1821,13 @@ static void eficonfig_display_change_boot_order(struct
efimenu *efi_menu)
puts(ANSI_COLOR_REVERSE);
if (entry->num < efi_menu->count - 2) {
- if (entry->active)
+ if (((struct eficonfig_boot_order_data
*)entry->data)->active)
printf("[*] ");
else
printf("[ ] ");
}
- printf("%ls", entry->description);
+ printf("%s", entry->title);
if (reverse)
puts(ANSI_COLOR_RESET);
@@ -1850,9 +1844,8 @@ static efi_status_t
eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
{
int esc = 0;
struct list_head *pos, *n;
- struct eficonfig_boot_order *tmp;
enum bootmenu_key key = KEY_NONE;
- struct eficonfig_boot_order *entry;
+ struct eficonfig_entry *entry, *tmp;
while (1) {
bootmenu_loop(NULL, , );
@@ -1861,11 +1854,11 @@ static efi_status_t
eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
case KEY_PLUS:
if (efi_menu->active > 0) {
list_for_each_safe(pos, n, _menu->list) {
- entry = list_entry(pos, struct
eficonfig_boot_order, list);
+ entry = list_entry(pos, struct
eficonfig_entry, list);
if (entry->num == efi_menu->active)
break;
}
- tmp = list_entry(pos->prev, struct
eficonfig_boot_order, list);
+ tmp = list_entry(pos->prev, struct
eficonfig_entry, list);
entry->num--;
tmp->num++;
list_del(>list);
@@ -1879,11 +1872,11 @@ static efi_status_t
eficonfig_choice_change_boot_order(struct efimenu *efi_menu)
case KEY_MINUS:
if (efi_menu->active < efi_menu->count - 3) {
list_for_each_safe(pos, n, _menu->list) {
- entry = list_entry(pos, struct
eficonfig_boot_order, list);
+ entry = list_entry(pos, struct
eficonfig_entry, list);
if (entry->num == efi_menu->active)
[PATCH v8 2/5] eficonfig: expose append entry function
Following commits are adding support for UEFI variable management
via the eficonfig menu. Those functions needs to use
append_entry() and append_quit_entry() to construct the
menu, so move them out of their static declarations.
Signed-off-by: Masahisa Kojima
Reviewed-by: Ilias Apalodimas
---
No update since v7
Changes in v7:
- update commit message
newly created in v2
cmd/eficonfig.c | 32 +---
include/efi_config.h | 5 +
2 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index 571e2b9ac0..b392de7954 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -263,7 +263,7 @@ efi_status_t eficonfig_process_quit(void *data)
}
/**
- * append_entry() - append menu item
+ * eficonfig_append_menu_entry() - append menu item
*
* @efi_menu: pointer to the efimenu structure
* @title: pointer to the entry title
@@ -271,8 +271,9 @@ efi_status_t eficonfig_process_quit(void *data)
* @data: pointer to the data to be passed to each entry callback
* Return: status code
*/
-static efi_status_t append_entry(struct efimenu *efi_menu,
-char *title, eficonfig_entry_func func, void
*data)
+efi_status_t eficonfig_append_menu_entry(struct efimenu *efi_menu,
+char *title, eficonfig_entry_func func,
+void *data)
{
struct eficonfig_entry *entry;
@@ -295,12 +296,12 @@ static efi_status_t append_entry(struct efimenu *efi_menu,
}
/**
- * append_quit_entry() - append quit entry
+ * eficonfig_append_quit_entry() - append quit entry
*
* @efi_menu: pointer to the efimenu structure
* Return: status code
*/
-static efi_status_t append_quit_entry(struct efimenu *efi_menu)
+efi_status_t eficonfig_append_quit_entry(struct efimenu *efi_menu)
{
char *title;
efi_status_t ret;
@@ -309,7 +310,7 @@ static efi_status_t append_quit_entry(struct efimenu
*efi_menu)
if (!title)
return EFI_OUT_OF_RESOURCES;
- ret = append_entry(efi_menu, title, eficonfig_process_quit, NULL);
+ ret = eficonfig_append_menu_entry(efi_menu, title,
eficonfig_process_quit, NULL);
if (ret != EFI_SUCCESS)
free(title);
@@ -341,7 +342,7 @@ void *eficonfig_create_fixed_menu(const struct
eficonfig_item *items, int count)
if (!title)
goto out;
- ret = append_entry(efi_menu, title, iter->func, iter->data);
+ ret = eficonfig_append_menu_entry(efi_menu, title, iter->func,
iter->data);
if (ret != EFI_SUCCESS) {
free(title);
goto out;
@@ -634,14 +635,15 @@ static efi_status_t eficonfig_select_volume(struct
eficonfig_select_file_info *f
info->v = v;
info->dp = device_path;
info->file_info = file_info;
- ret = append_entry(efi_menu, devname,
eficonfig_volume_selected, info);
+ ret = eficonfig_append_menu_entry(efi_menu, devname,
eficonfig_volume_selected,
+ info);
if (ret != EFI_SUCCESS) {
free(info);
goto out;
}
}
- ret = append_quit_entry(efi_menu);
+ ret = eficonfig_append_quit_entry(efi_menu);
if (ret != EFI_SUCCESS)
goto out;
@@ -745,8 +747,8 @@ eficonfig_create_file_entry(struct efimenu *efi_menu, u32
count,
(int (*)(const void *, const void *))sort_file);
for (i = 0; i < entry_num; i++) {
- ret = append_entry(efi_menu, tmp_infos[i]->file_name,
- eficonfig_file_selected, tmp_infos[i]);
+ ret = eficonfig_append_menu_entry(efi_menu,
tmp_infos[i]->file_name,
+ eficonfig_file_selected,
tmp_infos[i]);
if (ret != EFI_SUCCESS)
goto out;
}
@@ -815,7 +817,7 @@ static efi_status_t eficonfig_show_file_selection(struct
eficonfig_select_file_i
if (ret != EFI_SUCCESS)
goto err;
- ret = append_quit_entry(efi_menu);
+ ret = eficonfig_append_quit_entry(efi_menu);
if (ret != EFI_SUCCESS)
goto err;
@@ -1206,7 +1208,7 @@ static efi_status_t create_boot_option_entry(struct
efimenu *efi_menu, char *tit
utf16_utf8_strcpy(, val);
}
- return append_entry(efi_menu, buf, func, data);
+ return eficonfig_append_menu_entry(efi_menu, buf, func, data);
}
/**
@@ -1665,7 +1667,7 @@ static efi_status_t
eficonfig_add_boot_selection_entry(struct efimenu *efi_menu,
utf16_utf8_strcpy(, lo.label);
info->boot_index = boot_index;
info->selected =
[PATCH v8 1/5] eficonfig: refactor file selection handling
eficonfig_select_file_handler() is commonly used to select the
file. eficonfig_display_select_file_option() adds an additional
menu to clear the selected file.
eficonfig_display_select_file_option() is not always necessary
for the file selection process, so it must be outside of
eficonfig_select_file_handler().
This commit also renames the following functions to avoid confusion.
eficonfig_select_file_handler() -> eficonfig_process_select_file()
eficonfig_select_file() -> eficonfig_show_file_selection()
eficonfig_display_select_file_option() -> eficonfig_process_show_file_option()
Finally, test_eficonfig.py need to be updated to get aligned with
the above modification.
Signed-off-by: Masahisa Kojima
Reviewed-by: Ilias Apalodimas
---
No update since v7
Changes in v7:
- rename functio name to avoid confusion
- remove unused function
- update commit message
newly created in v2
cmd/eficonfig.c | 37 ++-
include/efi_config.h | 2 +-
.../py/tests/test_eficonfig/test_eficonfig.py | 1 +
3 files changed, 13 insertions(+), 27 deletions(-)
diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index 2595dd9563..571e2b9ac0 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -756,14 +756,14 @@ out:
}
/**
- * eficonfig_select_file() - construct the file selection menu
+ * eficonfig_show_file_selection() - construct the file selection menu
*
* @file_info: pointer to the file selection structure
* @root: pointer to the file handle
* Return: status code
*/
-static efi_status_t eficonfig_select_file(struct eficonfig_select_file_info
*file_info,
- struct efi_file_handle *root)
+static efi_status_t eficonfig_show_file_selection(struct
eficonfig_select_file_info *file_info,
+ struct efi_file_handle *root)
{
u32 count = 0, i;
efi_uintn_t len;
@@ -938,17 +938,6 @@ static efi_status_t eficonfig_boot_edit_save(void *data)
return EFI_SUCCESS;
}
-/**
- * eficonfig_process_select_file() - callback function for "Select File" entry
- *
- * @data: pointer to the data
- * Return: status code
- */
-efi_status_t eficonfig_process_select_file(void *data)
-{
- return EFI_SUCCESS;
-}
-
/**
* eficonfig_process_clear_file_selection() - callback function for "Clear"
entry
*
@@ -973,19 +962,19 @@ static struct eficonfig_item select_file_menu_items[] = {
{"Quit", eficonfig_process_quit},
};
-
/**
- * eficonfig_display_select_file_option() - display select file option
+ * eficonfig_process_show_file_option() - display select file option
*
* @file_info: pointer to the file information structure
* Return: status code
*/
-efi_status_t eficonfig_display_select_file_option(struct
eficonfig_select_file_info *file_info)
+efi_status_t eficonfig_process_show_file_option(void *data)
{
efi_status_t ret;
struct efimenu *efi_menu;
- select_file_menu_items[1].data = file_info;
+ select_file_menu_items[0].data = data;
+ select_file_menu_items[1].data = data;
efi_menu = eficonfig_create_fixed_menu(select_file_menu_items,
ARRAY_SIZE(select_file_menu_items));
if (!efi_menu)
@@ -1001,12 +990,12 @@ efi_status_t eficonfig_display_select_file_option(struct
eficonfig_select_file_i
}
/**
- * eficonfig_select_file_handler() - handle user file selection
+ * eficonfig_process_select_file() - handle user file selection
*
* @data: pointer to the data
* Return: status code
*/
-efi_status_t eficonfig_select_file_handler(void *data)
+efi_status_t eficonfig_process_select_file(void *data)
{
size_t len;
efi_status_t ret;
@@ -1016,10 +1005,6 @@ efi_status_t eficonfig_select_file_handler(void *data)
struct eficonfig_select_file_info *tmp = NULL;
struct eficonfig_select_file_info *file_info = data;
- ret = eficonfig_display_select_file_option(file_info);
- if (ret != EFI_SUCCESS)
- return ret;
-
tmp = calloc(1, sizeof(struct eficonfig_select_file_info));
if (!tmp)
return EFI_OUT_OF_RESOURCES;
@@ -1046,7 +1031,7 @@ efi_status_t eficonfig_select_file_handler(void *data)
if (ret != EFI_SUCCESS)
goto out;
- ret = eficonfig_select_file(tmp, root);
+ ret = eficonfig_show_file_selection(tmp, root);
if (ret == EFI_ABORTED)
continue;
if (ret != EFI_SUCCESS)
@@ -1284,7 +1269,7 @@ static efi_status_t prepare_file_selection_entry(struct
efimenu *efi_menu, char
utf8_utf16_strcpy(, devname);
u16_strlcat(file_name, file_info->current_path, len);
ret = create_boot_option_entry(efi_menu, title, file_name,
- eficonfig_select_file_handler,
[PATCH v8 0/5] eficonfig: add UEFI Secure Boot key maintenance interface
This series adds the UEFI Secure Boot key maintenance interface to the eficonfig command. User can enroll PK, KEK, db and dbx. Source code can be cloned with: $ git clone https://git.linaro.org/people/masahisa.kojima/u-boot.git -b kojima/eficonfig_sbkey_v8 Masahisa Kojima (5): eficonfig: refactor file selection handling eficonfig: expose append entry function eficonfig: refactor change boot order implementation eficonfig: add UEFI Secure Boot Key enrollment interface eficonfig: add "Show Signature Database" menu entry cmd/Makefile | 5 + cmd/eficonfig.c | 201 cmd/eficonfig_sbkey.c | 480 ++ include/efi_config.h | 12 +- .../py/tests/test_eficonfig/test_eficonfig.py | 1 + 5 files changed, 595 insertions(+), 104 deletions(-) create mode 100644 cmd/eficonfig_sbkey.c -- 2.17.1
Re: [PATCH v3] efi_loader: initialize return values in efi_uninstall_multiple_protocol_interfaces_int()
On 11/10/22 09:21, Ilias Apalodimas wrote:
If the va_list we got handed over contains no protocols we must return
EFI_SUCCESS. However in that case the current code just returns
an unintialized value.
Fix that by setting the return value in the variable definition
Addresses-Coverity: CID 376195: ("Uninitialized variables (UNINIT)")
Signed-off-by: Ilias Apalodimas
---
changes since v2:
- Don't return immediately on NULL protocols. Instead go through the list of
already uninstalled protocols and reinstall them
changes since v1:
- return EFI_SUCCESS instead of EFI_INVALID_PARAMETER
lib/efi_loader/efi_boottime.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index a56021559bbf..253f9f75ef63 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -2754,7 +2754,7 @@
efi_uninstall_multiple_protocol_interfaces_int(efi_handle_t handle,
{
const efi_guid_t *protocol;
void *protocol_interface;
- efi_status_t ret;
+ efi_status_t ret = EFI_SUCCESS;
size_t i = 0;
efi_va_list argptr_copy;
Reviewed by: Heinrich Schuchardt
[PATCH v3] efi_loader: initialize return values in efi_uninstall_multiple_protocol_interfaces_int()
If the va_list we got handed over contains no protocols we must return
EFI_SUCCESS. However in that case the current code just returns
an unintialized value.
Fix that by setting the return value in the variable definition
Addresses-Coverity: CID 376195: ("Uninitialized variables (UNINIT)")
Signed-off-by: Ilias Apalodimas
---
changes since v2:
- Don't return immediately on NULL protocols. Instead go through the list of
already uninstalled protocols and reinstall them
changes since v1:
- return EFI_SUCCESS instead of EFI_INVALID_PARAMETER
lib/efi_loader/efi_boottime.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index a56021559bbf..253f9f75ef63 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -2754,7 +2754,7 @@
efi_uninstall_multiple_protocol_interfaces_int(efi_handle_t handle,
{
const efi_guid_t *protocol;
void *protocol_interface;
- efi_status_t ret;
+ efi_status_t ret = EFI_SUCCESS;
size_t i = 0;
efi_va_list argptr_copy;
--
2.38.1
Re: [PATCH v2] efi_loader: initialize return values in efi_uninstall_multiple_protocol_interfaces_int()
Hi Heinrich
On Thu, 10 Nov 2022 at 10:15, Heinrich Schuchardt wrote:
>
> On 11/10/22 09:11, Ilias Apalodimas wrote:
> > If the va_list we got handed over contains no protocols we must return
> > EFI_SUCCESS. However in that case the current code just returns
> > an uninitialized value.
> > Fix that by setting the return value in the variable definition
> >
> > Addresses-Coverity: CID 376195: ("Uninitialized variables (UNINIT)")
> > Signed-off-by: Ilias Apalodimas
> > ---
> > changes since v1:
> > - return EFI_SUCCESS instead of EFI_INVALID_PARAMETER
> >
> > lib/efi_loader/efi_boottime.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> > index a56021559bbf..f52d086d17bf 100644
> > --- a/lib/efi_loader/efi_boottime.c
> > +++ b/lib/efi_loader/efi_boottime.c
> > @@ -2754,7 +2754,7 @@
> > efi_uninstall_multiple_protocol_interfaces_int(efi_handle_t handle,
> > {
> > const efi_guid_t *protocol;
> > void *protocol_interface;
> > - efi_status_t ret;
> > + efi_status_t ret = EFI_SUCCESS;
>
> OK
>
> > size_t i = 0;
> > efi_va_list argptr_copy;
> >
> > @@ -2765,7 +2765,7 @@
> > efi_uninstall_multiple_protocol_interfaces_int(efi_handle_t handle,
> > for (;;) {
> > protocol = efi_va_arg(argptr, efi_guid_t*);
> > if (!protocol)
> > - break;
> > + goto out;
>
> You may reach this point after actually uninstalling protocols. You have
> to check if the last protocol is uninstalled. Please, drop this change.
Yes good catch. Let me send a v3
Thanks
/Ilias
>
> Best regards
>
> Heinrich
>
> > protocol_interface = efi_va_arg(argptr, void*);
> > ret = efi_uninstall_protocol(handle, protocol,
> >protocol_interface);
>
Re: [PATCH v2] efi_loader: initialize return values in efi_uninstall_multiple_protocol_interfaces_int()
On 11/10/22 09:11, Ilias Apalodimas wrote:
If the va_list we got handed over contains no protocols we must return
EFI_SUCCESS. However in that case the current code just returns
an uninitialized value.
Fix that by setting the return value in the variable definition
Addresses-Coverity: CID 376195: ("Uninitialized variables (UNINIT)")
Signed-off-by: Ilias Apalodimas
---
changes since v1:
- return EFI_SUCCESS instead of EFI_INVALID_PARAMETER
lib/efi_loader/efi_boottime.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index a56021559bbf..f52d086d17bf 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -2754,7 +2754,7 @@
efi_uninstall_multiple_protocol_interfaces_int(efi_handle_t handle,
{
const efi_guid_t *protocol;
void *protocol_interface;
- efi_status_t ret;
+ efi_status_t ret = EFI_SUCCESS;
OK
size_t i = 0;
efi_va_list argptr_copy;
@@ -2765,7 +2765,7 @@
efi_uninstall_multiple_protocol_interfaces_int(efi_handle_t handle,
for (;;) {
protocol = efi_va_arg(argptr, efi_guid_t*);
if (!protocol)
- break;
+ goto out;
You may reach this point after actually uninstalling protocols. You have
to check if the last protocol is uninstalled. Please, drop this change.
Best regards
Heinrich
protocol_interface = efi_va_arg(argptr, void*);
ret = efi_uninstall_protocol(handle, protocol,
protocol_interface);
Re: [PATCH] efi_loader: initialize return values in efi_uninstall_multiple_protocol_interfaces_int()
On 11/10/22 08:35, Ilias Apalodimas wrote:
If the va_list we got handed over contains no protocols we must return
EFI_INVALID_PARAMETER. However in that case the current code just returns
an unintialized value.
Addresses-Coverity: CID 376195: ("Uninitialized variables (UNINIT)")
---
lib/efi_loader/efi_boottime.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index a56021559bbf..96aa36ef85de 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -2754,7 +2754,7 @@
efi_uninstall_multiple_protocol_interfaces_int(efi_handle_t handle,
{
const efi_guid_t *protocol;
void *protocol_interface;
- efi_status_t ret;
+ efi_status_t ret = EFI_INVALID_PARAMETER;
This should be EFI_SUCCESS as the UEFI spec does not explicitly forbid
to call the function without any protocol. EDK II does the same.
Best regards
Heinrich
size_t i = 0;
efi_va_list argptr_copy;
@@ -2765,7 +2765,7 @@
efi_uninstall_multiple_protocol_interfaces_int(efi_handle_t handle,
for (;;) {
protocol = efi_va_arg(argptr, efi_guid_t*);
if (!protocol)
- break;
+ goto out;
protocol_interface = efi_va_arg(argptr, void*);
ret = efi_uninstall_protocol(handle, protocol,
protocol_interface);
[PATCH v2] efi_loader: initialize return values in efi_uninstall_multiple_protocol_interfaces_int()
If the va_list we got handed over contains no protocols we must return
EFI_SUCCESS. However in that case the current code just returns
an uninitialized value.
Fix that by setting the return value in the variable definition
Addresses-Coverity: CID 376195: ("Uninitialized variables (UNINIT)")
Signed-off-by: Ilias Apalodimas
---
changes since v1:
- return EFI_SUCCESS instead of EFI_INVALID_PARAMETER
lib/efi_loader/efi_boottime.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index a56021559bbf..f52d086d17bf 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -2754,7 +2754,7 @@
efi_uninstall_multiple_protocol_interfaces_int(efi_handle_t handle,
{
const efi_guid_t *protocol;
void *protocol_interface;
- efi_status_t ret;
+ efi_status_t ret = EFI_SUCCESS;
size_t i = 0;
efi_va_list argptr_copy;
@@ -2765,7 +2765,7 @@
efi_uninstall_multiple_protocol_interfaces_int(efi_handle_t handle,
for (;;) {
protocol = efi_va_arg(argptr, efi_guid_t*);
if (!protocol)
- break;
+ goto out;
protocol_interface = efi_va_arg(argptr, void*);
ret = efi_uninstall_protocol(handle, protocol,
protocol_interface);
--
2.38.1
Re: [PATCH v5 02/16] Makefile: Correct the binman rule
On Wednesday 09 November 2022 19:14:40 Simon Glass wrote: > This currently uses if_changed on a phony target. Use a real file as the > target and add FORCE at the end, as required. Drop the 'inputs' phony > since it is not needed. Hello! Just one small note. It is quite surprising that .buildman_stamp target is called also when CONFIG_BINMAN is disabled. Not sure if this is expected but seems it should not cause any issue. > Signed-off-by: Simon Glass > --- > > (no changes since v2) > > Changes in v2: > - Use a separate rule for running binman > > Makefile | 11 --- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/Makefile b/Makefile > index d6699a54dbb..93d5c064f4c 100644 > --- a/Makefile > +++ b/Makefile > @@ -1108,18 +1108,15 @@ define deprecated > > endef > > -PHONY += inputs > -inputs: $(INPUTS-y) > - > -all: .binman_stamp inputs > +# Timestamp file to make sure that binman always runs > +.binman_stamp: $(INPUTS-y) FORCE > ifeq ($(CONFIG_BINMAN),y) > $(call if_changed,binman) > endif > - > -# Timestamp file to make sure that binman always runs > -.binman_stamp: FORCE > @touch $@ > > +all: .binman_stamp > + > ifeq ($(CONFIG_DEPRECATED),y) > $(warning "You have deprecated configuration options enabled in your > .config! Please check your configuration.") > endif > -- > 2.38.1.431.g37b22c650d-goog >
