Re: [PATCH] i2c: avoid dynamic stack use in dm_i2c_write
Hello Rasmus, On 07.07.22 15:12, Rasmus Villemoes wrote: > The size of the dynamic stack allocation here is bounded by the if() > statement. However, just allocating the maximum size up-front and > doing malloc() if necessary avoids code duplication (the > i2c_setup_offset() until the invocation of ->xfer), and generates much > better (smaller) code: > > bloat-o-meter drivers/i2c/i2c-uclass.o.{0,1} > add/remove: 0/0 grow/shrink: 0/1 up/down: 0/-144 (-144) > Function old new delta > dm_i2c_write 552 408-144 > Total: Before=3828, After=3684, chg -3.76% > > It also makes static analysis of maximum stack usage (using the .su > files that are automatically generated during build) easier if there > are no lines saying "dynamic". > > [This is not entirely equivalent to the existing code; this now uses > the stack for len <= 64 rather than len <= 63, but that seems like a > more natural limit.] > > Signed-off-by: Rasmus Villemoes > --- > drivers/i2c/i2c-uclass.c | 30 -- > 1 file changed, 12 insertions(+), 18 deletions(-) Looks good to me, nice catch! Reviewed-by: Heiko Schocher bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: h...@denx.de
Re: [PATCH v4] i2c: nuvoton: Add NPCM7xx i2c driver
Hello Jim, On 23.06.22 07:31, Jim Liu wrote: > Add Nuvoton BMC NPCM750 i2c driver > > Signed-off-by: Jim Liu > --- > changes for v4: >- remove i2c doc > changes for v3: >- add i2c doc > Changes for v2: >- use debug output in reset function >- use clr/setbits_8 > --- > drivers/i2c/Kconfig| 5 + > drivers/i2c/Makefile | 1 + > drivers/i2c/npcm-i2c.c | 631 + > 3 files changed, 637 insertions(+) > create mode 100644 drivers/i2c/npcm-i2c.c Reviewed-by: Heiko Schocher bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-52 Fax: +49-8142-66989-80 Email: h...@denx.de
Re: [PATCH sunxi/next] spi: sunxi: use XCH status to detect in-progress transfer
On Tue, 28 Jun 2022 14:49:24 +0800 Icenowy Zheng wrote: Hi Icenowy, > The current detection of RX FIFO depth seems to be not reliable, and > XCH will self-clear when a transfer is done. many thanks for sending this, indeed what I put in -next is broken, probably for everything except the F1C100 ;-) Digging a bit deeper this gets more interesting, though: I chased the issue down to my very first commit, that is (now properly!) setting the SPI bus frequency in the SPI controller's CCR register. It turns out that there are more issues in this driver, which lead to an actual frequency limit of 1 MHz[1]. So my commit now actually programs this value, and apparently it's too slow(?) for the code? Raising the default 1 to 4 MHz makes it work again (even without your patch). The previous timeout is generous, though, but by looking at the FIFO status register it just seemed to be stuck after clocking out one byte only, with the RX buf staying at 0. Reading FSR after your new loop reveals that the condition holds (RX FIFO level == nbytes), so there is something quite weird going on. Without your patch, but with some udelay(1000) after(!) the loop it also works, interestingly. As for the actual code change: looking at the XCH bit is probably indeed the most robust and clever method of checking for the end of a transfer, so I am tempted to take this change. However there are more things broken, apparently, and I would like to get to the bottom of those issues, before trying to paper over them. Cheers, Andre [1] The driver (as most U-Boot SPI drivers, actually) tries to read spi-max-frequency from the SPI's *controller* DT node, although this is a SPI *slave* property. It (correctly) doesn't find anything in there, so falls back to some (assumed) safe value of 1 MHz. > Check XCH bit when polling for transfer finish. > > Signed-off-by: Icenowy Zheng > --- > drivers/spi/spi-sunxi.c | 14 +- > 1 file changed, 5 insertions(+), 9 deletions(-) > > diff --git a/drivers/spi/spi-sunxi.c b/drivers/spi/spi-sunxi.c > index 2f7725..a424c6a98e 100644 > --- a/drivers/spi/spi-sunxi.c > +++ b/drivers/spi/spi-sunxi.c > @@ -83,7 +83,7 @@ DECLARE_GLOBAL_DATA_PTR; > #endif > #define SUN4I_SPI_MIN_RATE 3000 > #define SUN4I_SPI_DEFAULT_RATE 100 > -#define SUN4I_SPI_TIMEOUT_US 100 > +#define SUN4I_SPI_TIMEOUT_MS 1000 > > #define SPI_REG(priv, reg) ((priv)->base + \ > (priv)->variant->regs[reg]) > @@ -326,7 +326,6 @@ static int sun4i_spi_xfer(struct udevice *dev, unsigned > int bitlen, > struct dm_spi_slave_plat *slave_plat = dev_get_parent_plat(dev); > > u32 len = bitlen / 8; > - u32 rx_fifocnt; > u8 nbytes; > int ret; > > @@ -364,13 +363,10 @@ static int sun4i_spi_xfer(struct udevice *dev, unsigned > int bitlen, > setbits_le32(SPI_REG(priv, SPI_TCR), >SPI_BIT(priv, SPI_TCR_XCH)); > > - /* Wait till RX FIFO to be empty */ > - ret = readl_poll_timeout(SPI_REG(priv, SPI_FSR), > - rx_fifocnt, > - (((rx_fifocnt & > - SPI_BIT(priv, SPI_FSR_RF_CNT_MASK)) >> > - SUN4I_FIFO_STA_RF_CNT_BITS) >= nbytes), > - SUN4I_SPI_TIMEOUT_US); > + /* Wait for the transfer to be done */ > + ret = wait_for_bit_le32((const void *)SPI_REG(priv, SPI_TCR), > + SPI_BIT(priv, SPI_TCR_XCH), > + false, SUN4I_SPI_TIMEOUT_MS, false); > if (ret < 0) { > printf("ERROR: sun4i_spi: Timeout transferring data\n"); > sun4i_spi_set_cs(bus, slave_plat->cs, false);
Re: [PATCH 1/1] configs: sandbox_defconfig: CONFIG_LOG_MAX_LEVEL=9
On 7/10/22 21:31, Heinrich Schuchardt wrote: Without setting CONFIG_LOG_MAX_LEVEL to a value above 6 we will not detect NULL dereferences and other errors in log_debug() calls. Signed-off-by: Heinrich Schuchardt --- configs/sandbox_defconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index c509a924e6..4b87d96c76 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -31,6 +31,8 @@ CONFIG_CONSOLE_RECORD=y CONFIG_CONSOLE_RECORD_OUT_SIZE=0x6000 CONFIG_PRE_CONSOLE_BUFFER=y CONFIG_LOG=y +CONFIG_LOG_MAX_LEVEL=9 +CONFIG_LOG_DEFAULT_LEVEL=6 CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_STACKPROTECTOR=y CONFIG_ANDROID_AB=y We will also have to adjust the following tests to consider the value of LOG_MAX_LEVEL: FAILED test/py/tests/test_bind.py::test_bind_unbind_with_node - OSError: [Err... FAILED test/py/tests/test_bind.py::test_bind_unbind_with_uclass - AssertionEr... FAILED test/py/tests/test_ut.py::test_ut[ut_log_log_test_dropped] - Assertion... FAILED test/py/tests/test_ut.py::test_ut[ut_log_log_test_helpers] - assert False FAILED test/py/tests/test_ut.py::test_ut[ut_log_log_test_level_deny] - assert... Best regards Heinrich
[PATCH 1/1] configs: sandbox_defconfig: CONFIG_LOG_MAX_LEVEL=9
Without setting CONFIG_LOG_MAX_LEVEL to a value above 6 we will not detect NULL dereferences and other errors in log_debug() calls. Signed-off-by: Heinrich Schuchardt --- configs/sandbox_defconfig | 2 ++ 1 file changed, 2 insertions(+) diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index c509a924e6..4b87d96c76 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -31,6 +31,8 @@ CONFIG_CONSOLE_RECORD=y CONFIG_CONSOLE_RECORD_OUT_SIZE=0x6000 CONFIG_PRE_CONSOLE_BUFFER=y CONFIG_LOG=y +CONFIG_LOG_MAX_LEVEL=9 +CONFIG_LOG_DEFAULT_LEVEL=6 CONFIG_DISPLAY_BOARDINFO_LATE=y CONFIG_STACKPROTECTOR=y CONFIG_ANDROID_AB=y -- 2.36.1
RE: [TF-A] [RFC] Proposed location to host the firmware handoff specification.
Hi Julius, Thanks for reviewing the draft proposal, and for your comments. Last year’s discussion concluded with the agreement that, as a next step, we would draft a proposal of the data structure [1]. That draft is the DEN0135 document [2]. I realise that I haven’t made it sufficiently explicit in this thread that the DEN0135 document [2] is still at draft quality (see ALP maturity called out in the title page and footers). Please do not consider this a finished document . We’ve been iterating on this pdf as we gather feedback from the open-source communities. It's becoming clear that we need to move this debate to a more suitable forum. That’s why we’re proposing, in this thread, to move the document to trustedfirmware.org. There (and addressing the bottom half of your 4th point), the intent is to take new tag proposals as patch submissions. As you point out, this creates a low-barrier for new tags to be requested, suiting the needs of the open source communities that do adopt this data structure. First, we need to gather consensus on the initial document release. We want to use the repo in tf.org as a medium to mature the data structure definition and track the feedback/discussions. You raise very good points. Some do clash with feedback we've received so far. That further motivates the need to host this document in a repo. Once that's done, we'll have a suitable forum to discuss these distinct perspectives. > 1. Where did the requirement for 16-byte alignment come from? The requirement originated in feedback from the u-boot community, please see [3]. The argument for 16-byte alignment is that some data-structures have to be 16-byte aligned. We ought to meet that requirement if we ever want to carry those structures. >It seems a bit > excessive considering that no data field is actually larger than > 4 bytes (and it looks like it influenced several other choices in the spec, > e.g. > unnecessarily large transfer entry header because the bytes to the alignment > boundary have to be filled anyway). None of the current standard entries contain a field larger than 4-bytes. It's plausible that in the future there may be standard entries with a 64-bit field. Hence, I believe we need entries to be at least 8-byte aligned. > 2. The table entry header seems unnecessarily large. What's the point of > including a "header length" field when that field will always contain a 16? The "header length" enables the table entry header to be extended if we ever need that in the future. Changes to the TE header are backwards compatible. A consumer obtains the address of the data by adding the "header length" to the entry's base address. > While we're at it, the whole table header is also a bit large... > do we really expect to need > 2^32 possible versions? You could easily shrink that down to 16 > bytes.) > > 3. It's not exactly clear what the table header's "version" field is supposed > to > mean. What exactly would cause us to increase the version? The TL header version is incremented when we do a new release of the document. At a new document release, we can introduce one or more new standard tags and also append fields to the TL header. > What should a reader do when it reads an unknown version field? It would > probably be a good idea for the final document to specify that in more detail, > and I'd maybe consider splitting the version number in a major version and > minor version byte (major version = fully incompatible to older readers; minor > version = added meaning to previously reserved fields, but older readers can > continue to read the fields they know about). Agreed. I see this point requires further discussion and agreement between all involved. > > 4. I don't think a strict definition of entry type ranges is a good idea, > particularly if "standard" and "OEM" are the only two options. As mentioned above, the standard tag range ought to have a low-barrier for new tags to be requested. The OEM range allows FW integrators to carry platform-specific entries that may not be generalizable. > > 5. Embedding large data structures like a full FDT or ACPI table directly by > value > in this structure seems like a bad idea and too inflexible in practice. Note that the FDT can still be pointed to by X0/R2 without having to be included by value in the TL. The feedback we received so far is: data should be included in the TL as that simplifies procedures, such as relocating the data, without having to worry about stale pointers. Having said that, maybe we can debate this point and find a compromise that works for all! Regards, Jose [1] https://lists.trustedfirmware.org/archives/list/t...@lists.trustedfirmware.org/message/3S3GVT5RLIP2P4YCHC3MR4X2JKQXDE7I/ [2] https://developer.arm.com/documentation/den0135/a [3] https://www.mail-archive.com/u-boot@lists.denx.de/msg441605.html
Re: [PATCH] mtd: spi-nor-ids: add winbond w25q512nw family support
On Sun, Jul 10, 2022 at 8:05 PM Jae Hyun Yoo wrote: > > Hello Jagan, > > On 7/9/2022 10:20 PM, Jagan Teki wrote: > > On Sat, Jul 9, 2022 at 12:10 AM Jae Hyun Yoo > > wrote: > >> > >> Add Winbond w25q512nwiq/in and w25q512nwim support. > >> > >> datasheet: > >> https://www.winbond.com/resource-files/W25Q512NW%20RevB%2007192021.pdf > >> > >> Signed-off-by: Jae Hyun Yoo > >> --- > > > > Applied to u-boot-spi/master > > Can you please apply v2 instead? > https://lore.kernel.org/all/20220708190319.2730480-1-quic_jaeh...@quicinc.com/ I did v2. It looks like I replied for v1. It's Ok. https://patchwork.ozlabs.org/project/uboot/patch/20220708190319.2730480-1-quic_jaeh...@quicinc.com/ Jagan.
Re: [PATCH 2/2] arm: apple: Add initial Apple M2 support
> From: Janne Grunau > Date: Fri, 1 Jul 2022 00:06:17 +0200 > > Apple's M2 SoC very similar to the M1 and can use the same memory map. > The keyboard/trackpad on the MacBook Pro (13-inch, M2, 2022) uses > "dockchannel" as transport instead of SPI and needs a new driver. > USB, NVMe, uart, framebuffer and watchdog are working with the existing > drivers. > > Signed-off-by: Janne Grunau Reviewed-by: Mark Kettenis > --- > arch/arm/mach-apple/board.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/mach-apple/board.c b/arch/arm/mach-apple/board.c > index 1525a9edee8e..16046423128c 100644 > --- a/arch/arm/mach-apple/board.c > +++ b/arch/arm/mach-apple/board.c > @@ -16,7 +16,7 @@ > > DECLARE_GLOBAL_DATA_PTR; > > -/* Apple M1 */ > +/* Apple M1/M2 */ > > static struct mm_region t8103_mem_map[] = { > { > @@ -376,7 +376,8 @@ void build_mem_map(void) > fdt_size_t size; > int i; > > - if (of_machine_is_compatible("apple,t8103")) > + if (of_machine_is_compatible("apple,t8103") || > + of_machine_is_compatible("apple,t8112")) > mem_map = t8103_mem_map; > else if (of_machine_is_compatible("apple,t6000")) > mem_map = t6000_mem_map; > -- > 2.35.1 > >
Re: [PATCH 1/2] iommu: Add M2 support to Apple DART driver
> From: Janne Grunau > Date: Fri, 1 Jul 2022 00:06:16 +0200 > > "apple,t8112-dart" uses an incompatible register interface but still > offers the same functionality. This DART is found on the M2 and M1 > Pro/Max/Ultra SoCs. > > Signed-off-by: Janne Grunau Reviewed-by: Mark Kettenis > --- > drivers/iommu/apple_dart.c | 18 +- > 1 file changed, 17 insertions(+), 1 deletion(-) > > diff --git a/drivers/iommu/apple_dart.c b/drivers/iommu/apple_dart.c > index f2e170096072..2faacb8f3b6f 100644 > --- a/drivers/iommu/apple_dart.c > +++ b/drivers/iommu/apple_dart.c > @@ -24,6 +24,12 @@ > #define DART_TTBR_VALID BIT(31) > #define DART_TTBR_SHIFT 12 > > +#define DART_T8110_TCR(sid) (0x1000 + 4 * (sid)) > +#define DART_T8110_TCR_BYPASS_DAPF BIT(2) > +#define DART_T8110_TCR_BYPASS_DART BIT(1) > +#define DART_T8110_TCR_TRANSLATE_ENABLE BIT(0) > +#define DART_T8110_TTBR(sid) (0x1400 + 4 * (sid)) > + > static int apple_dart_probe(struct udevice *dev) > { > void *base; > @@ -34,7 +40,16 @@ static int apple_dart_probe(struct udevice *dev) > return -EINVAL; > > u32 params2 = readl(base + DART_PARAMS2); > - if (params2 & DART_PARAMS2_BYPASS_SUPPORT) { > + if (!(params2 & DART_PARAMS2_BYPASS_SUPPORT)) > + return 0; > + > + if (device_is_compatible(dev, "apple,t8112-dart")) { > + for (sid = 0; sid < 256; sid++) { > + writel(DART_T8110_TCR_BYPASS_DART | > DART_T8110_TCR_BYPASS_DAPF, > +base + DART_T8110_TCR(sid)); > + writel(0, base + DART_T8110_TTBR(sid)); > + } > + } else { > for (sid = 0; sid < 16; sid++) { > writel(DART_TCR_BYPASS_DART | DART_TCR_BYPASS_DAPF, > base + DART_TCR(sid)); > @@ -49,6 +64,7 @@ static int apple_dart_probe(struct udevice *dev) > static const struct udevice_id apple_dart_ids[] = { > { .compatible = "apple,t8103-dart" }, > { .compatible = "apple,t6000-dart" }, > + { .compatible = "apple,t8112-dart" }, > { /* sentinel */ } > }; > > -- > 2.35.1 > >
Re: [PATCH 3/3] doc: environment: Further expand on Image locations and provide example
On Sun, Jul 10, 2022 at 02:26:04PM +0200, Heinrich Schuchardt wrote: > On 6/30/22 12:06, Simon Glass wrote: > > On Mon, 20 Jun 2022 at 08:32, Tom Rini wrote: > > > > > > Start by elaborating on what some of our constraints tend to be with > > > image location values, and document where these external constraints > > > come from. Provide a new subsection, an example based on the TI ARMv7 > > > OMAP2PLUS families of chips, that gives sample values and explains why > > > we use these particular values. This is based on what is in > > > include/configs/ti_armv7_common.h as of fb3ad9bd923d ("TI: Add, use a > > > DEFAULT_LINUX_BOOT_ENV environment string") as this contains just the > > > values referenced in this document now and not some of the further > > > additions that are less generic. > > > > > > Signed-off-by: Tom Rini > > > --- > > > doc/usage/environment.rst | 39 +++ > > > 1 file changed, 39 insertions(+) > > > > Reviewed-by: Simon Glass > > Below you want a change? Yes, often Simon does that (and it's fine) to both offer a tag but if another iteration is needed to make a minor adjustment to some wording or another, or to make when applying. Which is fine with me. > > > diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst > > > index a9a4702632d2..f70ccd6a58ee 100644 > > > --- a/doc/usage/environment.rst > > > +++ b/doc/usage/environment.rst > > > @@ -404,6 +404,42 @@ device tree blob fdtfilefdt_addr_r > > > fdt_addr > > > ramdisk ramdiskfileramdisk_addr_r ramdisk_addr > > > = == == > > > > > > +When setting the RAM addresses for `kernel_addr_r`, `fdt_addr_r` and > > > +`ramdisk_addr_r` there are several constraints to keep in mind. When > > > booting > > > +Linux, the `Booting ARM Linux`_ and `Booting AArch64 Linux`_ documents > > > lay out > > > +the requirements for booting all ARM platforms, including both alignment > > > and > > > +where within memory various things must be. These guidelines tend to > > > also be > > > +correct for other OSes and unless specifically contradicted by > > > documentation > > What makes you think that BSD or Haiku have the same constraints as Linux? Because of what I said, and experience? Now, one may be a subset of another, but that still means it will work for both. This is intended to be general best practices. If you follow this then it's likely anything else will work too. The danger comes from trying to optimize the sizes to be as small as possible, rather than as large/flexible as will likely work anywhere. I will try and expand on that idea in the next iteration. > > > +specific to another architecture, are good rules to follow for other > > > +architectures as well. > > No. RISC-V does not have the same requirements as ARM. E.g. the initrd > can be located anywhere in memory. Please point to documentation that confirms that, and some otherwise bad examples that actually work. > > > + > > > +Example Image locations > > > +^^^ > > You seem not to refer to a file 'Image'. > > %s/Image/image/ OK. > > > + > > > +If we take the Texas Instruments OMAP2PLUS family of ARMv7 processors as > > > an > > > +example for the above listed variables, we would do:: > > %s/we would do/we chose/ ? Either? I don't see it mattering either way. > > > + > > > +loadaddr=0x8200 > > > +kernel_addr_r=${loadaddr} > > > +fdt_addr_r=0x8800 > > > +ramdisk_addr_r=0x8808 > > > +bootm_size=0x1000 > > > + > > > +To explain this, we start by noting that DRAM starts at 0x8000. A > > > 32MiB > > > > Should it say 'We use a 32MiB' ? > > Please, mention that MAX_UNCOMP_KERNEL_SIZE = 32 MiB is ARMv7 specific. Sorry? As I understood it last, the maximum size was something like 96MiB before you have to employ some funky tricks. > > > +buffer from the start of memory as our default load address, and so > > > where the > > > +kernel would also be loaded to. This will hopefully allow for us to > > > have the > > %s/allow for us/allow/ > > > > +whole of the compressed kernel image exist in memory above where the > > > whole of > > > +the decompressed kernel image will be, and allow for a quicker boot. > > > Next, we We use 32MiB for the reason I said here. Which is only a slight rewording of the arm32 Linux booting document, and the section starts out by saying this is an example for ARMv7 platforms. > Please, mention that decompressor code otherwise will have to relocate > the compressed kernel. I'm not sure. Perhaps it would be good to also link to some of the articles expanding upon how Linux on ARM32 boots, as part of more general documentation, rather than a specific example here. > > > +say that the device tree will be placed at 128MiB offset from the start > > > of > > Please, mention that initrd must be > > * within 512 MiB
[PATCH 4/4] MIPS: convert CONFIG_SYS_MIPS_TIMER_FREQ to Kconfig
This converts the following to Kconfig: CONFIG_SYS_MIPS_TIMER_REQ Signed-off-by: Daniel Schwierzeck --- arch/mips/Kconfig | 18 ++ configs/ap121_defconfig| 1 + configs/ap143_defconfig| 1 + configs/ap152_defconfig| 1 + configs/bcm968380gerg_ram_defconfig| 1 + configs/boston32r2_defconfig | 1 + configs/boston32r2el_defconfig | 1 + configs/boston32r6_defconfig | 1 + configs/boston32r6el_defconfig | 1 + configs/boston64r2_defconfig | 1 + configs/boston64r2el_defconfig | 1 + configs/boston64r6_defconfig | 1 + configs/boston64r6el_defconfig | 1 + configs/ci20_mmc_defconfig | 1 + configs/comtrend_ar5315u_ram_defconfig | 1 + configs/comtrend_ar5387un_ram_defconfig| 1 + configs/comtrend_ct5361_ram_defconfig | 1 + configs/comtrend_vr3032u_ram_defconfig | 1 + configs/comtrend_wap5813n_ram_defconfig| 1 + configs/gardena-smart-gateway-mt7688_defconfig | 1 + configs/huawei_hg556a_ram_defconfig| 1 + configs/imgtec_xilfpga_defconfig | 1 + configs/linkit-smart-7688_defconfig| 1 + configs/malta64_defconfig | 1 + configs/malta64el_defconfig| 1 + configs/malta_defconfig| 1 + configs/maltael_defconfig | 1 + configs/mscc_jr2_defconfig | 1 + configs/mscc_luton_defconfig | 1 + configs/mscc_ocelot_defconfig | 1 + configs/mscc_serval_defconfig | 1 + configs/mscc_servalt_defconfig | 1 + configs/mt7620_mt7530_rfb_defconfig| 1 + configs/mt7620_rfb_defconfig | 1 + configs/mt7621_nand_rfb_defconfig | 1 + configs/mt7621_rfb_defconfig | 1 + configs/mt7628_rfb_defconfig | 1 + configs/netgear_cg3100d_ram_defconfig | 1 + configs/netgear_dgnd3700v2_ram_defconfig | 1 + configs/pic32mzdask_defconfig | 1 + configs/sagem_f@st1704_ram_defconfig | 1 + configs/sfr_nb4-ser_ram_defconfig | 1 + configs/tplink_wdr4300_defconfig | 1 + configs/vocore2_defconfig | 1 + include/configs/ap121.h| 2 -- include/configs/ap143.h| 2 -- include/configs/ap152.h| 2 -- include/configs/bmips_bcm3380.h| 3 --- include/configs/bmips_bcm6318.h| 3 --- include/configs/bmips_bcm63268.h | 3 --- include/configs/bmips_bcm6328.h| 3 --- include/configs/bmips_bcm6338.h| 3 --- include/configs/bmips_bcm6348.h| 3 --- include/configs/bmips_bcm6358.h| 3 --- include/configs/bmips_bcm6362.h| 3 --- include/configs/bmips_bcm6368.h| 3 --- include/configs/bmips_bcm6838.h| 3 --- include/configs/boston.h | 1 - include/configs/ci20.h | 3 --- include/configs/gardena-smart-gateway-mt7688.h | 3 --- include/configs/imgtec_xilfpga.h | 2 -- include/configs/linkit-smart-7688.h| 3 --- include/configs/malta.h| 1 - include/configs/mt7620.h | 2 -- include/configs/mt7621.h | 2 -- include/configs/mt7628.h | 2 -- include/configs/pic32mzdask.h | 2 -- include/configs/tplink_wdr4300.h | 2 -- include/configs/vcoreiii.h | 5 - include/configs/vocore2.h | 3 --- scripts/config_whitelist.txt | 1 - 71 files changed, 61 insertions(+), 68 deletions(-) diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index 8bef63cbb7..9af0133f10 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -14,6 +14,7 @@ choice config TARGET_MALTA bool "Support malta" + select HAS_FIXED_TIMER_FREQUENCY select BOARD_EARLY_INIT_R select DM select DM_SERIAL @@ -41,17 +42,20 @@ config TARGET_MALTA config ARCH_ATH79 bool "Support QCA/Atheros ath79" + select HAS_FIXED_TIMER_FREQUENCY select DM select OF_CONTROL imply CMD_DM config ARCH_MSCC bool "Support MSCC VCore-III" + select HAS_FIXED_TIMER_FREQUENCY select OF_CONTROL select DM config ARCH_BMIPS bool "Support BMIPS SoCs" + select HAS_FIXED_TIMER_FREQUENCY select CLK select CPU select DM @@ -62,6 +66,7 @@ config ARCH_BMIPS
[PATCH 2/4] MIPS: remove CONFIG_SYS_MHZ
Resolve all uses of CONFIG_SYS_MHZ with the currently defined value. Remove code which depends on CONFIG_SYS_MHZ but where no board configs actually use that code. Signed-off-by: Daniel Schwierzeck --- arch/mips/mach-jz47xx/include/mach/jz4780.h | 2 +- arch/mips/mach-jz47xx/jz4780/pll.c | 6 +- board/imgtec/ci20/ci20.c| 4 include/configs/ap121.h | 3 +-- include/configs/ap143.h | 3 +-- include/configs/ap152.h | 3 +-- include/configs/ci20.h | 3 +-- include/configs/malta.h | 3 +-- include/configs/tplink_wdr4300.h| 3 +-- scripts/config_whitelist.txt| 1 - 10 files changed, 8 insertions(+), 23 deletions(-) diff --git a/arch/mips/mach-jz47xx/include/mach/jz4780.h b/arch/mips/mach-jz47xx/include/mach/jz4780.h index 4422e503ed..880445dac3 100644 --- a/arch/mips/mach-jz47xx/include/mach/jz4780.h +++ b/arch/mips/mach-jz47xx/include/mach/jz4780.h @@ -60,7 +60,7 @@ /* PLL setup */ #define JZ4780_SYS_EXTAL 4800 -#define JZ4780_SYS_MEM_SPEED (CONFIG_SYS_MHZ * 100) +#define JZ4780_SYS_MEM_SPEED (1200 * 100) #define JZ4780_SYS_MEM_DIV 3 #define JZ4780_SYS_AUDIO_SPEED (768 * 100) diff --git a/arch/mips/mach-jz47xx/jz4780/pll.c b/arch/mips/mach-jz47xx/jz4780/pll.c index 323c634fb3..4519b478cc 100644 --- a/arch/mips/mach-jz47xx/jz4780/pll.c +++ b/arch/mips/mach-jz47xx/jz4780/pll.c @@ -399,11 +399,7 @@ static void cpu_mux_select(int pll) ((2 - 1) << CPM_CPCCR_L2DIV_BIT) | ((1 - 1) << CPM_CPCCR_CDIV_BIT); - if (CONFIG_SYS_MHZ >= 1000) - clk_ctrl |= (12 - 1) << CPM_CPCCR_PDIV_BIT; - else - clk_ctrl |= (6 - 1) << CPM_CPCCR_PDIV_BIT; - + clk_ctrl |= (12 - 1) << CPM_CPCCR_PDIV_BIT; clrsetbits_le32(cpm_regs + CPM_CPCCR, 0x00ff, clk_ctrl); while (readl(cpm_regs + CPM_CPCSR) & (CPM_CPCSR_CDIV_BUSY | diff --git a/board/imgtec/ci20/ci20.c b/board/imgtec/ci20/ci20.c index 7cbe49abd9..89f5e7ad79 100644 --- a/board/imgtec/ci20/ci20.c +++ b/board/imgtec/ci20/ci20.c @@ -350,10 +350,6 @@ static const struct jz4780_ddr_config H5TQ2G83CFR_48_config = { .pulldn = 0x0e, }; -#if (CONFIG_SYS_MHZ != 1200) -#error No DDR configuration for CPU speed -#endif - const struct jz4780_ddr_config *jz4780_get_ddr_config(void) { const int board_revision = ci20_revision(); diff --git a/include/configs/ap121.h b/include/configs/ap121.h index 099aac5421..61cc073a8a 100644 --- a/include/configs/ap121.h +++ b/include/configs/ap121.h @@ -6,8 +6,7 @@ #ifndef __CONFIG_H #define __CONFIG_H -#define CONFIG_SYS_MHZ 200 -#define CONFIG_SYS_MIPS_TIMER_FREQ (CONFIG_SYS_MHZ * 100) +#define CONFIG_SYS_MIPS_TIMER_FREQ 2 #define CONFIG_SYS_SDRAM_BASE 0x8000 diff --git a/include/configs/ap143.h b/include/configs/ap143.h index 60b9e779fa..579b9b4f2c 100644 --- a/include/configs/ap143.h +++ b/include/configs/ap143.h @@ -6,8 +6,7 @@ #ifndef __CONFIG_H #define __CONFIG_H -#define CONFIG_SYS_MHZ 325 -#define CONFIG_SYS_MIPS_TIMER_FREQ (CONFIG_SYS_MHZ * 100) +#define CONFIG_SYS_MIPS_TIMER_FREQ 32500 #define CONFIG_SYS_SDRAM_BASE 0x8000 diff --git a/include/configs/ap152.h b/include/configs/ap152.h index d165ead7bb..283762fd22 100644 --- a/include/configs/ap152.h +++ b/include/configs/ap152.h @@ -6,8 +6,7 @@ #ifndef __CONFIG_H #define __CONFIG_H -#define CONFIG_SYS_MHZ 375 -#define CONFIG_SYS_MIPS_TIMER_FREQ (CONFIG_SYS_MHZ * 100) +#define CONFIG_SYS_MIPS_TIMER_FREQ 37500 #define CONFIG_SYS_SDRAM_BASE 0x8000 diff --git a/include/configs/ci20.h b/include/configs/ci20.h index 192da015e1..7e8a9fcb80 100644 --- a/include/configs/ci20.h +++ b/include/configs/ci20.h @@ -10,8 +10,7 @@ #define __CONFIG_CI20_H__ /* Ingenic JZ4780 clock configuration. */ -#define CONFIG_SYS_MHZ 1200 -#define CONFIG_SYS_MIPS_TIMER_FREQ (CONFIG_SYS_MHZ * 100) +#define CONFIG_SYS_MIPS_TIMER_FREQ 12 /* Memory configuration */ #define CONFIG_SYS_MONITOR_LEN (512 * 1024) diff --git a/include/configs/malta.h b/include/configs/malta.h index c8b230ab21..717867d12a 100644 --- a/include/configs/malta.h +++ b/include/configs/malta.h @@ -18,8 +18,7 @@ /* * CPU Configuration */ -#define CONFIG_SYS_MHZ 250 /* arbitrary value */ -#define CONFIG_SYS_MIPS_TIMER_FREQ (CONFIG_SYS_MHZ * 100) +#define CONFIG_SYS_MIPS_TIMER_FREQ 25000 /* * Memory map diff --git a/include/configs/tplink_wdr4300.h b/include/configs/tplink_wdr4300.h index f5466fd509..1400a211e3 100644 --- a/include/configs/tplink_wdr4300.h +++ b/include/configs/tplink_wdr4300.h @@ -6,8 +6,7 @@ #ifndef __CONFIG_H #define __CONFIG_H -#define
[PATCH 1/4] MIPS: remove deprecated TARGET_VCT option
This board has been removed a long time ago. Signed-off-by: Daniel Schwierzeck --- arch/mips/Kconfig | 8 1 file changed, 8 deletions(-) diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig index 2e0793a7a7..8bef63cbb7 100644 --- a/arch/mips/Kconfig +++ b/arch/mips/Kconfig @@ -39,14 +39,6 @@ config TARGET_MALTA select SWAP_IO_SPACE imply CMD_DM -config TARGET_VCT - bool "Support vct" - select ROM_EXCEPTION_VECTORS - select SUPPORTS_BIG_ENDIAN - select SUPPORTS_CPU_MIPS32_R1 - select SUPPORTS_CPU_MIPS32_R2 - select SYS_MIPS_CACHE_INIT_RAM_LOAD - config ARCH_ATH79 bool "Support QCA/Atheros ath79" select DM -- 2.37.0
[PATCH 3/4] MIPS: mscc: remove unused CPU_CLOCK_RATE
CPU_CLOCK_RATE is just used once for CONFIG_SYS_MIPS_TIMER_FREQ which is migrated to Kconfig in the next patch. Signed-off-by: Daniel Schwierzeck --- include/configs/vcoreiii.h | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/include/configs/vcoreiii.h b/include/configs/vcoreiii.h index 78a62a8b02..5c5036b8be 100644 --- a/include/configs/vcoreiii.h +++ b/include/configs/vcoreiii.h @@ -13,11 +13,9 @@ #define CONFIG_SYS_INIT_SP_OFFSET 0x40 #if defined(CONFIG_SOC_LUTON) || defined(CONFIG_SOC_SERVAL) -#define CPU_CLOCK_RATE 41666 /* Clock for the MIPS core */ #define CONFIG_SYS_MIPS_TIMER_FREQ 20833 #else -#define CPU_CLOCK_RATE 5 /* Clock for the MIPS core */ -#define CONFIG_SYS_MIPS_TIMER_FREQ (CPU_CLOCK_RATE / 2) +#define CONFIG_SYS_MIPS_TIMER_FREQ 25000 #endif #define CONFIG_SYS_NS16550_CLK CONFIG_SYS_MIPS_TIMER_FREQ -- 2.37.0
[PATCH 0/4] Convert CONFIG_SYS_MIPS_TIMER_FREQ to Kconfig
This removes CONFIG_SYS_HZ and converts CONFIG_SYS_MIPS_TIMER_FREQ to Kconfig. The series only applies on u-boot-mips/next (based on u-boot/next and the Mediatek MT7621 series). Daniel Schwierzeck (4): MIPS: remove deprecated TARGET_VCT option MIPS: remove CONFIG_SYS_MHZ MIPS: mscc: remove unused CPU_CLOCK_RATE MIPS: convert CONFIG_SYS_MIPS_TIMER_FREQ to Kconfig arch/mips/Kconfig | 26 +-- arch/mips/mach-jz47xx/include/mach/jz4780.h | 2 +- arch/mips/mach-jz47xx/jz4780/pll.c| 6 + board/imgtec/ci20/ci20.c | 4 --- configs/ap121_defconfig | 1 + configs/ap143_defconfig | 1 + configs/ap152_defconfig | 1 + configs/bcm968380gerg_ram_defconfig | 1 + configs/boston32r2_defconfig | 1 + configs/boston32r2el_defconfig| 1 + configs/boston32r6_defconfig | 1 + configs/boston32r6el_defconfig| 1 + configs/boston64r2_defconfig | 1 + configs/boston64r2el_defconfig| 1 + configs/boston64r6_defconfig | 1 + configs/boston64r6el_defconfig| 1 + configs/ci20_mmc_defconfig| 1 + configs/comtrend_ar5315u_ram_defconfig| 1 + configs/comtrend_ar5387un_ram_defconfig | 1 + configs/comtrend_ct5361_ram_defconfig | 1 + configs/comtrend_vr3032u_ram_defconfig| 1 + configs/comtrend_wap5813n_ram_defconfig | 1 + .../gardena-smart-gateway-mt7688_defconfig| 1 + configs/huawei_hg556a_ram_defconfig | 1 + configs/imgtec_xilfpga_defconfig | 1 + configs/linkit-smart-7688_defconfig | 1 + configs/malta64_defconfig | 1 + configs/malta64el_defconfig | 1 + configs/malta_defconfig | 1 + configs/maltael_defconfig | 1 + configs/mscc_jr2_defconfig| 1 + configs/mscc_luton_defconfig | 1 + configs/mscc_ocelot_defconfig | 1 + configs/mscc_serval_defconfig | 1 + configs/mscc_servalt_defconfig| 1 + configs/mt7620_mt7530_rfb_defconfig | 1 + configs/mt7620_rfb_defconfig | 1 + configs/mt7621_nand_rfb_defconfig | 1 + configs/mt7621_rfb_defconfig | 1 + configs/mt7628_rfb_defconfig | 1 + configs/netgear_cg3100d_ram_defconfig | 1 + configs/netgear_dgnd3700v2_ram_defconfig | 1 + configs/pic32mzdask_defconfig | 1 + configs/sagem_f@st1704_ram_defconfig | 1 + configs/sfr_nb4-ser_ram_defconfig | 1 + configs/tplink_wdr4300_defconfig | 1 + configs/vocore2_defconfig | 1 + include/configs/ap121.h | 3 --- include/configs/ap143.h | 3 --- include/configs/ap152.h | 3 --- include/configs/bmips_bcm3380.h | 3 --- include/configs/bmips_bcm6318.h | 3 --- include/configs/bmips_bcm63268.h | 3 --- include/configs/bmips_bcm6328.h | 3 --- include/configs/bmips_bcm6338.h | 3 --- include/configs/bmips_bcm6348.h | 3 --- include/configs/bmips_bcm6358.h | 3 --- include/configs/bmips_bcm6362.h | 3 --- include/configs/bmips_bcm6368.h | 3 --- include/configs/bmips_bcm6838.h | 3 --- include/configs/boston.h | 1 - include/configs/ci20.h| 4 --- .../configs/gardena-smart-gateway-mt7688.h| 3 --- include/configs/imgtec_xilfpga.h | 2 -- include/configs/linkit-smart-7688.h | 3 --- include/configs/malta.h | 2 -- include/configs/mt7620.h | 2 -- include/configs/mt7621.h | 2 -- include/configs/mt7628.h | 2 -- include/configs/pic32mzdask.h | 2 -- include/configs/tplink_wdr4300.h | 3 --- include/configs/vcoreiii.h| 7 - include/configs/vocore2.h | 3 --- scripts/config_whitelist.txt | 2 -- 74 files changed, 63 insertions(+), 95 deletions(-) -- 2.37.0
Re: [PATCH] mtd: spi-nor-ids: add winbond w25q512nw family support
Hello Jagan, On 7/9/2022 10:20 PM, Jagan Teki wrote: On Sat, Jul 9, 2022 at 12:10 AM Jae Hyun Yoo wrote: Add Winbond w25q512nwiq/in and w25q512nwim support. datasheet: https://www.winbond.com/resource-files/W25Q512NW%20RevB%2007192021.pdf Signed-off-by: Jae Hyun Yoo --- Applied to u-boot-spi/master Can you please apply v2 instead? https://lore.kernel.org/all/20220708190319.2730480-1-quic_jaeh...@quicinc.com/ Thank you! -Jae
[PATCH 1/1] efi_loader: memory leak in efi_set_bootdev()
efi_dp_str() allocates memory which should be released after use. Use %pD printf code. Adjust message wording. Fixes: d837cb1e3b6b ("efi: Add debugging to efi_set_bootdev()") Signed-off-by: Heinrich Schuchardt --- cmd/bootefi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 827fcd97df..e32385cf56 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -107,9 +107,9 @@ void efi_set_bootdev(const char *dev, const char *devnr, const char *path, efi_free_pool(image_tmp); } bootefi_image_path = image; - log_debug("- recorded device %ls\n", efi_dp_str(device)); + log_debug("- boot device %pD\n", device); if (image) - log_debug("- and image %ls\n", efi_dp_str(image)); + log_debug("- image %pD\n", image); } else { log_debug("- efi_dp_from_name() failed, err=%lx\n", ret); efi_clear_bootdev(); -- 2.36.1
Re: [PATCH 2/3] doc: environment: Expand on fdt_addr, initrd_addr and loadaddr
On Sun, Jul 10, 2022 at 01:02:28PM +0200, Heinrich Schuchardt wrote: > On 6/20/22 16:31, Tom Rini wrote: > > - Explain why fdt_addr and initrd_addr should not be set to disable > >relocation normally. > > - Provide some advice on the typical loadaddr default value. > > > > Signed-off-by: Tom Rini > > --- > > doc/usage/environment.rst | 15 --- > > 1 file changed, 12 insertions(+), 3 deletions(-) > > > > diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst > > index a3f23d4e4e22..a9a4702632d2 100644 > > --- a/doc/usage/environment.rst > > +++ b/doc/usage/environment.rst > > @@ -204,7 +204,9 @@ fdt_high > > to work it must reside in writable memory, have > > sufficient padding on the end of it for u-boot to > > add the information it needs into it, and the memory > > -must be accessible by the kernel. > > +must be accessible by the kernel. This usage is strongly discouraged > > +however as it also stops U-Boot from ensuring the device tree starting > > +address is properly aligned and a misaligned tree will cause OS > > failures. > > fdt_addr_r is typically 4 byte aligned. Is there a bug in some part of > the code? It must be 8 byte aligned. And we don't know the alignment inside of FIT images. Do not disable device tree relocation except under the most calculated of circumstances. > > fdtcontroladdr > > if set this is the address of the control flattened > > @@ -240,14 +242,21 @@ initrd_high > > memory. In this case U-Boot will NOT COPY the > > ramdisk at all. This may be useful to reduce the > > boot time on your system, but requires that this > > -feature is supported by your Linux kernel. > > +feature is supported by your Linux kernel. This usage however requires > > +that the user ensure that there will be no overlap with other parts of > > the > > It is not the user but the developer who define $kernel_addr_r, > $initr_addr_r, and $fdt_addr_r. No, it can be either. See stackoverflow and assorted blog posts. Person who picked up an SBC and just wants the thing to work will find various posts saying to do ... something ... here. > Relocating initrd does not stop the user from loading the kernel to > initrd_high. The user is always responsible for avoiding overlap. In > this regard there is nothing special about initrd_high=~0UL. And we're responsible for providing reasonable defaults. Do not disable initrd relocation by default as it gains little and can break the OS. > > +iamge such as the Linux kernel BSS. It should not be enabled by > > default > > %s/iamge/image Please fix when applying. > > +and only done as part of optimizing a deployment. > > Relocating initrd and fdt is using boot time. On many boards it is > simply superfluous. On these boards I cannot see any reason not to > disable it by default. The way to avoid "superfluous" time here is to set reasonable default locations, not disabling relocation. Please note this is what the policy is, not something new. -- Tom signature.asc Description: PGP signature
[PATCH 1/1] dm: avoid NULL dereference in add_item()
acpi_add_other_item() passes dev = NULL. Instead of dev->name write the string "other" to the debug log: ACPI: Writing ACPI tables at 1fd3000 0base: writing table '' * other: Added type 3, 11fd4000, size 240 1facs: writing table 'FACS' * other: Added type 3, 11fd4240, size 40 5csrt: writing table 'CSRT' * other: Added type 3, 11fd4280, size 30 Signed-off-by: Heinrich Schuchardt --- drivers/core/acpi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/core/acpi.c b/drivers/core/acpi.c index 0df58dbc0d..8457733edb 100644 --- a/drivers/core/acpi.c +++ b/drivers/core/acpi.c @@ -159,8 +159,8 @@ static int add_item(struct acpi_ctx *ctx, struct udevice *dev, memcpy(item->buf, start, item->size); } item_count++; - log_debug("* %s: Added type %d, %p, size %x\n", dev->name, type, start, - item->size); + log_debug("* %s: Added type %d, %p, size %x\n", + dev ? dev->name : "other", type, start, item->size); return 0; } -- 2.36.1
Re: [PATCH 3/3] doc: environment: Further expand on Image locations and provide example
On 6/30/22 12:06, Simon Glass wrote: On Mon, 20 Jun 2022 at 08:32, Tom Rini wrote: Start by elaborating on what some of our constraints tend to be with image location values, and document where these external constraints come from. Provide a new subsection, an example based on the TI ARMv7 OMAP2PLUS families of chips, that gives sample values and explains why we use these particular values. This is based on what is in include/configs/ti_armv7_common.h as of fb3ad9bd923d ("TI: Add, use a DEFAULT_LINUX_BOOT_ENV environment string") as this contains just the values referenced in this document now and not some of the further additions that are less generic. Signed-off-by: Tom Rini --- doc/usage/environment.rst | 39 +++ 1 file changed, 39 insertions(+) Reviewed-by: Simon Glass Below you want a change? diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst index a9a4702632d2..f70ccd6a58ee 100644 --- a/doc/usage/environment.rst +++ b/doc/usage/environment.rst @@ -404,6 +404,42 @@ device tree blob fdtfilefdt_addr_r fdt_addr ramdisk ramdiskfileramdisk_addr_r ramdisk_addr = == == +When setting the RAM addresses for `kernel_addr_r`, `fdt_addr_r` and +`ramdisk_addr_r` there are several constraints to keep in mind. When booting +Linux, the `Booting ARM Linux`_ and `Booting AArch64 Linux`_ documents lay out +the requirements for booting all ARM platforms, including both alignment and +where within memory various things must be. These guidelines tend to also be +correct for other OSes and unless specifically contradicted by documentation What makes you think that BSD or Haiku have the same constraints as Linux? +specific to another architecture, are good rules to follow for other +architectures as well. No. RISC-V does not have the same requirements as ARM. E.g. the initrd can be located anywhere in memory. + +Example Image locations +^^^ You seem not to refer to a file 'Image'. %s/Image/image/ + +If we take the Texas Instruments OMAP2PLUS family of ARMv7 processors as an +example for the above listed variables, we would do:: %s/we would do/we chose/ ? + +loadaddr=0x8200 +kernel_addr_r=${loadaddr} +fdt_addr_r=0x8800 +ramdisk_addr_r=0x8808 +bootm_size=0x1000 + +To explain this, we start by noting that DRAM starts at 0x8000. A 32MiB Should it say 'We use a 32MiB' ? Please, mention that MAX_UNCOMP_KERNEL_SIZE = 32 MiB is ARMv7 specific. +buffer from the start of memory as our default load address, and so where the +kernel would also be loaded to. This will hopefully allow for us to have the %s/allow for us/allow/ +whole of the compressed kernel image exist in memory above where the whole of +the decompressed kernel image will be, and allow for a quicker boot. Next, we Please, mention that decompressor code otherwise will have to relocate the compressed kernel. +say that the device tree will be placed at 128MiB offset from the start of Please, mention that initrd must be * within 512 MiB (0x2000) of the memory start on arm (which restricts initrd_high) * in a a 1 GB aligned region of size '1UL << (VA_BITS_MIN - 1)' that includes the kernel on arm64 On RISC-V such a limitation does not exist. +memory. This is suggested by the kernel documment as it is exceedingly %s/documment/document +unlikely to be overwritten by the kernel itself given other architectural +constraints. We then allow for the device tree to be up to 512KiB in size +before placing the ramdisk in memory. We then say that everything should be +within the first 256MiB of memory so that U-Boot can relocate things as needed +to ensure proper alignment. We pick 256MiB as our value here because we know If the load address ranges occupy the first 256 MiB, where will be the space for relocation on a 256 MiB board? Without mentioning initrd_high this paragraph is incomplete. +there are very few platforms on in this family with less memory. It could be +as high as 768MiB and still ensure that everything would be visible to the +kernel, but again we go with what we assume is the safest assumption. A section per architecture clearly naming the architecture specific restrictions of Linux would be much more helpful. Best regards Heinrich Automatically updated variables --- @@ -472,3 +508,6 @@ Implementation -- See :doc:`../develop/environment` for internal development details. + +.. _`Booting ARM Linux`: https://www.kernel.org/doc/html/latest/arm/booting.html +.. _`Booting AArch64 Linux`: https://www.kernel.org/doc/html/latest/arm64/booting.html -- 2.25.1
[PATCH 2/2] Nokia RX-51: Remove CONFIG_PREBOOT from defconfig
CONFIG_PREBOOT just cause putting "preboot=CONFIG_PREBOOT" into env list. Value CONFIG_PREBOOT="run preboot" in defconfig is just nonsense and does not do anything useful (it is infinite recursion). Config file for this board already contains default preboot= env variable with correct value, which has higher priority than CONFIG_PREBOOT and this is reason why nonsense CONFIG_PREBOOT is ignored. Remove nonsense and unused CONFIG_PREBOOT from nokia_rx51_defconfig file. Signed-off-by: Pali Rohár --- configs/nokia_rx51_defconfig | 1 - 1 file changed, 1 deletion(-) diff --git a/configs/nokia_rx51_defconfig b/configs/nokia_rx51_defconfig index 309cf28269c1..46b794f168d9 100644 --- a/configs/nokia_rx51_defconfig +++ b/configs/nokia_rx51_defconfig @@ -24,7 +24,6 @@ CONFIG_AUTOBOOT_MENU_SHOW=y CONFIG_USE_BOOTCOMMAND=y CONFIG_BOOTCOMMAND="run sdboot;run emmcboot;run attachboot;echo" CONFIG_USE_PREBOOT=y -CONFIG_PREBOOT="run preboot" # CONFIG_SYS_DEVICE_NULLDEV is not set CONFIG_HUSH_PARSER=y CONFIG_SYS_PROMPT="Nokia RX-51 # " -- 2.20.1
[PATCH 1/2] Fix usage of CONFIG_PREBOOT
Due to usage of PREBOOT in Kconfig, macro CONFIG_PREBOOT is always defined when CONFIG_USE_PREBOOT is enabled. In case CONFIG_PREBOOT is not explicitly enabled it is set to empty C string and therefore '#ifdef CONFIG_PREBOOT' guard does not work. Fix this issue by introducing a new Kconfig symbol PREBOOT_DEFINED which cause to define new C macro CONFIG_PREBOOT_DEFINED only when CONFIG_PREBOOT is really defined. Change usage of '#ifdef CONFIG_PREBOOT' by '#ifdef CONFIG_USE_PREBOOT' for code which checks if preboot code would be called and by '#ifdef CONFIG_PREBOOT_DEFINED' for defining preboot code. Signed-off-by: Pali Rohár --- board/boundary/nitrogen6x/nitrogen6x.c | 4 ++-- boot/Kconfig | 4 include/env_default.h | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/board/boundary/nitrogen6x/nitrogen6x.c b/board/boundary/nitrogen6x/nitrogen6x.c index 83bb445d481a..382c01ddf4e0 100644 --- a/board/boundary/nitrogen6x/nitrogen6x.c +++ b/board/boundary/nitrogen6x/nitrogen6x.c @@ -929,7 +929,7 @@ U_BOOT_CMD( "Returns 0 (true) to shell if key is pressed." ); -#ifdef CONFIG_PREBOOT +#ifdef CONFIG_USE_PREBOOT static char const kbd_magic_prefix[] = "key_magic"; static char const kbd_command_prefix[] = "key_cmd"; @@ -989,7 +989,7 @@ int misc_init_r(void) gpio_request(IMX_GPIO_NR(2, 3), "search"); gpio_request(IMX_GPIO_NR(7, 13), "volup"); gpio_request(IMX_GPIO_NR(4, 5), "voldown"); -#ifdef CONFIG_PREBOOT +#ifdef CONFIG_USE_PREBOOT preboot_keys(); #endif diff --git a/boot/Kconfig b/boot/Kconfig index 08451c65a56b..5e7ae61d5116 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -1309,6 +1309,10 @@ config PREBOOT help This is the default of "preboot" environment variable. +config PREBOOT_DEFINED + bool + default y if PREBOOT != "" + config DEFAULT_FDT_FILE string "Default fdt file" help diff --git a/include/env_default.h b/include/env_default.h index 7004a6fef29b..62a73b939cf2 100644 --- a/include/env_default.h +++ b/include/env_default.h @@ -62,7 +62,7 @@ const char default_environment[] = { #ifdef CONFIG_SYS_AUTOLOAD "autoload=" CONFIG_SYS_AUTOLOAD "\0" #endif -#ifdef CONFIG_PREBOOT +#ifdef CONFIG_PREBOOT_DEFINED "preboot=" CONFIG_PREBOOT "\0" #endif #ifdef CONFIG_ROOTPATH -- 2.20.1
[PATCH 2/2] ubifs: Use U-Boot assert() from in UBI/UBIFS code
U-Boot already provides assert function, so it use also in ubi and ubifs code. Signed-off-by: Pali Rohár --- drivers/mtd/ubi/debug.h | 9 ++--- fs/ubifs/debug.h| 9 ++--- 2 files changed, 4 insertions(+), 14 deletions(-) diff --git a/drivers/mtd/ubi/debug.h b/drivers/mtd/ubi/debug.h index 2c2faaf1b4dc..9c8ce51636b1 100644 --- a/drivers/mtd/ubi/debug.h +++ b/drivers/mtd/ubi/debug.h @@ -27,13 +27,8 @@ void ubi_dump_vid_hdr(const struct ubi_vid_hdr *vid_hdr); }\ } while (0) #else -#define ubi_assert(expr) do { \ - if (unlikely(!(expr))) { \ - pr_debug("UBI assert failed in %s at %u\n", \ - __func__, __LINE__); \ - dump_stack();\ - }\ -} while (0) +#include +#define ubi_assert(expr) assert(expr) #endif #define ubi_dbg_print_hex_dump(ps, pt, r, g, b, len, a) \ diff --git a/fs/ubifs/debug.h b/fs/ubifs/debug.h index d69f1e6ac70b..0ecc2e0c8e98 100644 --- a/fs/ubifs/debug.h +++ b/fs/ubifs/debug.h @@ -165,14 +165,9 @@ struct ubifs_global_debug_info { dbg_snprintf_key(c, key, __tmp_key_buf, DBG_KEY_BUF_LEN));\ } while (0) #else -#define ubifs_assert(expr) do { \ - if (unlikely(!(expr))) { \ - pr_debug("UBIFS assert failed in %s at %u\n", \ - __func__, __LINE__);\ - dump_stack(); \ - } \ -} while (0) +#include +#define ubifs_assert(expr) assert(expr) #define ubifs_assert_cmt_locked(c) do { } while (0) #define ubifs_dbg_msg(type, fmt, ...) \ -- 2.20.1
[PATCH 1/2] ubifs: Fix ubifs_assert_cmt_locked()
U-Boot does not implement down_write_trylock() and its stub always returns true that lock was acquired. Therefore ubifs_assert_cmt_locked() assert currently always fails. Fix this issue by redefining ubifs_assert_cmt_locked() to just empty stub as there is nothing to assert. Signed-off-by: Pali Rohár --- fs/ubifs/debug.h | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/fs/ubifs/debug.h b/fs/ubifs/debug.h index 5f6e12702de8..d69f1e6ac70b 100644 --- a/fs/ubifs/debug.h +++ b/fs/ubifs/debug.h @@ -173,13 +173,7 @@ struct ubifs_global_debug_info { } \ } while (0) -#define ubifs_assert_cmt_locked(c) do { \ - if (unlikely(down_write_trylock(&(c)->commit_sem))) { \ - up_write(&(c)->commit_sem);\ - pr_debug("commit lock is not locked!\n"); \ - ubifs_assert(0); \ - } \ -} while (0) +#define ubifs_assert_cmt_locked(c) do { } while (0) #define ubifs_dbg_msg(type, fmt, ...) \ pr_debug("UBIFS DBG " type ": " fmt "\n", \ -- 2.20.1
Re: [PATCH 2/3] doc: environment: Expand on fdt_addr, initrd_addr and loadaddr
On 6/20/22 16:31, Tom Rini wrote: - Explain why fdt_addr and initrd_addr should not be set to disable relocation normally. - Provide some advice on the typical loadaddr default value. Signed-off-by: Tom Rini --- doc/usage/environment.rst | 15 --- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst index a3f23d4e4e22..a9a4702632d2 100644 --- a/doc/usage/environment.rst +++ b/doc/usage/environment.rst @@ -204,7 +204,9 @@ fdt_high to work it must reside in writable memory, have sufficient padding on the end of it for u-boot to add the information it needs into it, and the memory -must be accessible by the kernel. +must be accessible by the kernel. This usage is strongly discouraged +however as it also stops U-Boot from ensuring the device tree starting +address is properly aligned and a misaligned tree will cause OS failures. fdt_addr_r is typically 4 byte aligned. Is there a bug in some part of the code? fdtcontroladdr if set this is the address of the control flattened @@ -240,14 +242,21 @@ initrd_high memory. In this case U-Boot will NOT COPY the ramdisk at all. This may be useful to reduce the boot time on your system, but requires that this -feature is supported by your Linux kernel. +feature is supported by your Linux kernel. This usage however requires +that the user ensure that there will be no overlap with other parts of the It is not the user but the developer who define $kernel_addr_r, $initr_addr_r, and $fdt_addr_r. Relocating initrd does not stop the user from loading the kernel to initrd_high. The user is always responsible for avoiding overlap. In this regard there is nothing special about initrd_high=~0UL. +iamge such as the Linux kernel BSS. It should not be enabled by default %s/iamge/image +and only done as part of optimizing a deployment. Relocating initrd and fdt is using boot time. On many boards it is simply superfluous. On these boards I cannot see any reason not to disable it by default. Best regards Heinrich ipaddr IP address; needed for tftpboot command loadaddr Default load address for commands like "bootp", -"rarpboot", "tftpboot", "loadb" or "diskboot" +"rarpboot", "tftpboot", "loadb" or "diskboot". Note that the optimal +default values here will vary between architectures. On 32bit ARM for +example, some offset from start of memory is used as the Linux kernel +zImage has a self decompressor and it's best if we stay out of where that +will be working. loads_echo see CONFIG_LOADS_ECHO
Re: [PATCH 1/3] doc: environment: Drop u-boot_addr_r
On 6/20/22 16:31, Tom Rini wrote: This variable is never set nor explained why it would be set, drop it. Signed-off-by: Tom Rini Reviewed-by: Heinrich Schuchardt --- doc/usage/environment.rst | 1 - 1 file changed, 1 deletion(-) diff --git a/doc/usage/environment.rst b/doc/usage/environment.rst index 28a8952b7528..a3f23d4e4e22 100644 --- a/doc/usage/environment.rst +++ b/doc/usage/environment.rst @@ -390,7 +390,6 @@ in U-Boot code. = == == Image File Name RAM Address Flash Location = == == -u-bootu-boot u-boot_addr_ru-boot_addr Linux kernel bootfile kernel_addr_rkernel_addr device tree blob fdtfilefdt_addr_r fdt_addr ramdisk ramdiskfileramdisk_addr_r ramdisk_addr
Re: [PATCH] mmc: fsl_esdhc: Fix 'Internal clock never stabilised.' error
PING? On Thursday 23 June 2022 18:11:37 Pali Rohár wrote: > On Tuesday 14 June 2022 11:25:18 Jaehoon Chung wrote: > > Hi, > > > > On 6/12/22 18:12, Pali Rohár wrote: > > > PING? > > > > Sorry for too late. > > When will be this fix patch processed and merged? > > > > > > > On Friday 29 April 2022 20:27:34 Pali Rohár wrote: > > >> Only newer eSDHC controllers set PRSSTAT_SDSTB flag. So do not wait until > > >> flag PRSSTAT_SDSTB is set on old pre-2.2 controllers. Instead sleep for > > >> fixed amount of time like it was before commit 6f883e501b65 ("mmc: > > >> fsl_esdhc: Add emmc hs200 support"). > > >> > > >> This change fixes error 'Internal clock never stabilised.' which is > > >> printed > > >> on P2020 board at every access to SD card. > > >> > > >> Fixes: 6f883e501b65 ("mmc: fsl_esdhc: Add emmc hs200 support") > > >> Signed-off-by: Pali Rohár > > > > Reviewed-by: Jaehoon Chung > > > > Best Regards, > > Jaehoon Chung > > > > >> --- > > >> drivers/mmc/fsl_esdhc.c | 17 + > > >> 1 file changed, 17 insertions(+) > > >> > > >> diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c > > >> index fdf2cc290e06..3b3587bd8d72 100644 > > >> --- a/drivers/mmc/fsl_esdhc.c > > >> +++ b/drivers/mmc/fsl_esdhc.c > > >> @@ -503,6 +503,7 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, > > >> struct mmc *mmc, uint clock) > > >> u32 time_out; > > >> u32 value; > > >> uint clk; > > >> +u32 hostver; > > >> > > >> if (clock < mmc->cfg->f_min) > > >> clock = mmc->cfg->f_min; > > >> @@ -543,6 +544,14 @@ static void set_sysctl(struct fsl_esdhc_priv *priv, > > >> struct mmc *mmc, uint clock) > > >> > > >> esdhc_clrsetbits32(>sysctl, SYSCTL_CLOCK_MASK, clk); > > >> > > >> +/* Only newer eSDHC controllers set PRSSTAT_SDSTB flag */ > > >> +hostver = esdhc_read32(>esdhc_regs->hostver); > > >> +if (HOSTVER_VENDOR(hostver) <= VENDOR_V_22) { > > >> +udelay(1); > > >> +esdhc_setbits32(>sysctl, SYSCTL_PEREN | > > >> SYSCTL_CKEN); > > >> +return; > > >> +} > > >> + > > >> time_out = 20; > > >> value = PRSSTAT_SDSTB; > > >> while (!(esdhc_read32(>prsstat) & value)) { > > >> @@ -562,6 +571,7 @@ static void esdhc_clock_control(struct > > >> fsl_esdhc_priv *priv, bool enable) > > >> struct fsl_esdhc *regs = priv->esdhc_regs; > > >> u32 value; > > >> u32 time_out; > > >> +u32 hostver; > > >> > > >> value = esdhc_read32(>sysctl); > > >> > > >> @@ -572,6 +582,13 @@ static void esdhc_clock_control(struct > > >> fsl_esdhc_priv *priv, bool enable) > > >> > > >> esdhc_write32(>sysctl, value); > > >> > > >> +/* Only newer eSDHC controllers set PRSSTAT_SDSTB flag */ > > >> +hostver = esdhc_read32(>esdhc_regs->hostver); > > >> +if (HOSTVER_VENDOR(hostver) <= VENDOR_V_22) { > > >> +udelay(1); > > >> +return; > > >> +} > > >> + > > >> time_out = 20; > > >> value = PRSSTAT_SDSTB; > > >> while (!(esdhc_read32(>prsstat) & value)) { > > >> -- > > >> 2.20.1 > > >> > > > > >
Re: [RFC PATCH 3/3] eficonfig: add "Delete Key" menu entry
On 6/19/22 07:20, Masahisa Kojima wrote: This commit add the menu-driven interface to delete the signature database entry. EFI Signature Lists can contain the multiple signature entries, this menu can delete the indivisual entry. If the PK is enrolled and UEFI Secure Boot is in User Mode, Why don't you mention Deployed Mode? user can not delete the existing signature lists since the signature lists must be signed by KEK or PK but signing information is not stored in the signature database. Updating PK with an empty value must be possible if if the new value is signed with the old PK. Signed-off-by: Masahisa Kojima --- cmd/eficonfig_sbkey.c | 218 +- 1 file changed, 217 insertions(+), 1 deletion(-) diff --git a/cmd/eficonfig_sbkey.c b/cmd/eficonfig_sbkey.c index 02ab8f8218..142bb4cef5 100644 --- a/cmd/eficonfig_sbkey.c +++ b/cmd/eficonfig_sbkey.c @@ -54,6 +54,29 @@ static const struct eficonfig_sigtype_to_str sigtype_to_str[] = { /*{EFI_CERT_SHA512_GUID, "SHA512", SIG_TYPE_HASH}, */ }; Please, add documentation to all functions. +static int eficonfig_console_yes_no(void) +{ + int esc = 0; + enum bootmenu_key key = KEY_NONE; + + while (1) { + bootmenu_loop(NULL, , ); + + switch (key) { + case KEY_SELECT: + return 1; + case KEY_QUIT: + return 0; + default: + break; + } + } + + /* never happens */ + debug("eficonfig: this should not happen"); + return 0; If this code is unreachable, remove it. +} + static void eficonfig_console_wait_enter(void) { int esc = 0; @@ -72,7 +95,19 @@ static void eficonfig_console_wait_enter(void) /* never happens */ debug("eficonfig: this should not happen"); - return; Please remove unreachable code. +} + +static bool is_setupmode(void) +{ + efi_status_t ret; + u8 setup_mode; + efi_uintn_t size; + + size = sizeof(setup_mode); + ret = efi_get_variable_int(u"SetupMode", _global_variable_guid, + NULL, , _mode, NULL); + + return setup_mode == 1; } static bool is_secureboot_enabled(void) @@ -254,6 +289,103 @@ static efi_status_t eficonfig_process_sigdata_show(void *data) return EFI_SUCCESS; } +static efi_status_t eficonfig_process_sigdata_delete(void *data) +{ + int yes_no; + struct eficonfig_sig_data *sg = data; + + display_sigdata_header(sg, "Delete"); + display_sigdata_info(sg); + + printf("\n\n Press ENTER to delete, ESC/CTRL+C to quit"); + yes_no = eficonfig_console_yes_no(); + if (!yes_no) + return EFI_NOT_READY; + + return EFI_SUCCESS; +} + +static void delete_selected_signature_data(void *db, efi_uintn_t *db_size, + struct eficonfig_sig_data *target, + struct list_head *siglist_list) +{ + u8 *dest, *start; + struct list_head *pos, *n; + u32 remain; + u32 size = *db_size; + u8 *end = (u8 *)db + size; + struct eficonfig_sig_data *sg; + + list_for_each_safe(pos, n, siglist_list) { + sg = list_entry(pos, struct eficonfig_sig_data, list); + if (sg->esl == target->esl && sg->esd == target->esd) { + remain = sg->esl->signature_list_size - +(sizeof(struct efi_signature_list) - +sg->esl->signature_header_size) - +sg->esl->signature_size; + if (remain > 0) { + /* only delete the single signature data */ + sg->esl->signature_list_size -= sg->esl->signature_size; + size -= sg->esl->signature_size; + dest = (u8 *)sg->esd; + start = (u8 *)sg->esd + sg->esl->signature_size; + } else { + /* delete entire signature list */ + dest = (u8 *)sg->esl; + start = (u8 *)sg->esl + sg->esl->signature_list_size; + size -= sg->esl->signature_list_size; + } + memmove(dest, start, (end - start)); + } + } + + *db_size = size; +} + +static efi_status_t create_time_based_payload(void *db, void **new_db, efi_uintn_t *size) +{ + efi_status_t ret; + struct efi_time time; + efi_uintn_t total_size; + struct efi_variable_authentication_2 *auth; + + *new_db = NULL; + + /* +* SetVariable() call with EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS +*
Re: [PULL] u-boot-mips for u-boot/next (v2022.10)
Hi Daniel, This is my personal email address. I found that my previous mail send from my company was still be marked as spam by gmail. I've tested the latest fixups on u-boot-mips/next, and all is ok. thx. Best Regards, Weijie Daniel Schwierzeck 于2022年7月10日周日 00:23写道: > > > > On 09.07.22 14:43, Tom Rini wrote: > > On Sat, Jul 09, 2022 at 02:01:01PM +0200, Daniel Schwierzeck wrote: > >> Hi Tom, > >> > >> On 08.07.22 18:50, Tom Rini wrote: > >>> On Fri, Jul 08, 2022 at 05:21:48PM +0200, Daniel SchwierzeckHi Tom, wrote: > > Gitlab CI: > > https://source.denx.de/u-boot/custodians/u-boot-mips/-/pipelines/12656 > > Azure: > > https://dev.azure.com/danielschwierzeck/u-boot/_build/results?buildId=30=results > > > The following changes since commit > 2d2c61ff0460740d9ec5a44dbef9255a8c690696: > > Merge tag 'efi-2022-07-rc7' of > https://source.denx.de/u-boot/custodians/u-boot-efi (2022-07-06 09:17:08 > -0400) > > are available in the Git repository at: > > https://source.denx.de/u-boot/custodians/u-boot-mips.git/ > tags/mips-pull-2022-07-08 > > for you to fetch changes up to e5fc4022af3cfd59e3459276305671a595ac5ff0: > > MAINTAINERS: update maintainer for MediaTek MIPS platform > (2022-07-08 15:13:29 +0200) > > > - MIPS: add drivers and board support for Mediatek MT7621 SoC > >>> > >>> OK, we need a few changes here, sorry. There's now migrated CONFIG > >>> symbols, most of which are easy to do and I was about to, and then I saw > >>> this: > >>> #ifdef CONFIG_TPL_BUILD > >>> #define CONFIG_SPL_START_S_PATH "arch/mips/mach-mtmips/mt7621/tpl" > >>> /* .bss will not be used by TPL */ > >>> #define CONFIG_SPL_BSS_START_ADDR 0x8000 > >>> #define CONFIG_SPL_BSS_MAX_SIZE 0 > >>> #else > >>> #define CONFIG_SPL_START_S_PATH "arch/mips/mach-mtmips/mt7621/spl" > >>> #define CONFIG_SPL_BSS_START_ADDR 0x8014 > >>> #define CONFIG_SPL_BSS_MAX_SIZE 0x8 > >>> #define CONFIG_SPL_MAX_SIZE 0x3 > >>> #endif > >>> > >>> No, you cannot abuse CONFIG_TPL_BUILD to set CONFIG_SPL_foo. Those need > >>> to become CONFIG_TPL_foo, and set appropriately. And then for > >>> [ST]PL_START_S_PATH, you need to set head-$(CONFIG_ARCH_xxx) to the > >>> right file, for SPL/TPL instead. > >>> > >> > >> do you already have patches for converting stuff like > >> CONFIG_SPL_BSS_START_ADDR prepared? Than I would wait with the pull request > >> until those patches are applied to mainline and I would adapt the MT7621 > >> patches. > >> > >> I could also assist with converting CONFIG_SPL_START_S_PATH because that's > >> only used on MIPS and one ARM board. > > > > See what's in -next already? SPL_BSS_START_ADDR is migrated, but there > > were no TPL_BSS_START_ADDR cases. For START_S_PATH, the platform just > > needs to be reworked as I suggested above I believe, to achieve the > > desired result. > > > > sorry, didn't check the latest updates in -next and the series was too > long on the list ;) > > TPL_BSS_START_ADDR shouldn't be necessary because MT7621 doesn't use BSS > in TPL, the defined values where just dummy values. > > I rechecked and removed all migrated Kconfig options from mt7621.h and > pushed an update to u-boot-mips/next. Weijie could you verify that? If > all is okay, I'll prepare a new pull request, otherwise please send me a > v7 patch series. > > -- > - Daniel -- ___ lede-devel mailing list lede-de...@lists.infradead.org
Re: [RFC PATCH 1/3] eficonfig: add UEFI Secure Boot Key enrollment interface
On 7/8/22 11:14, Ilias Apalodimas wrote: On Sun, Jun 19, 2022 at 02:20:20PM +0900, Masahisa Kojima wrote: This commit adds the menu-driven UEFI Secure Boot Key enrollment interface. User can enroll the PK, KEK, db and dbx by selecting EFI Signature Lists file. After the PK is enrolled, UEFI Secure Boot is enabled and EFI Signature Lists file must be signed by KEK or PK. Signed-off-by: Masahisa Kojima --- cmd/Makefile | 3 + cmd/eficonfig.c | 3 + cmd/eficonfig_sbkey.c | 202 ++ include/efi_config.h | 3 + 4 files changed, 211 insertions(+) create mode 100644 cmd/eficonfig_sbkey.c diff --git a/cmd/Makefile b/cmd/Makefile index 0afa687e94..9d87b639fc 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -64,6 +64,9 @@ 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 +obj-$(CONFIG_EFI_SECURE_BOOT) += eficonfig_sbkey.o +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 e62f5e41a4..e6d2cba9c5 100644 --- a/cmd/eficonfig.c +++ b/cmd/eficonfig.c @@ -1832,6 +1832,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)) + {"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..a5c0dbe9b3 --- /dev/null +++ b/cmd/eficonfig_sbkey.c @@ -0,0 +1,202 @@ +// 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 + Please, provide function descriptions. +static bool is_secureboot_enabled(void) +{ + efi_status_t ret; + u8 secure_boot; + efi_uintn_t size; + + size = sizeof(secure_boot); + ret = efi_get_variable_int(u"SecureBoot", _global_variable_guid, + NULL, , _boot, NULL); + + return secure_boot == 1; +} + +static efi_status_t eficonfig_process_enroll_key(void *data) +{ + u32 attr; + char *buf = NULL; + efi_uintn_t size; + efi_status_t ret; + struct efi_file_handle *f; + struct efi_file_handle *root; + struct eficonfig_select_file_info file_info; + + file_info.current_path = calloc(1, EFICONFIG_FILE_PATH_BUF_SIZE); + if (!file_info.current_path) + goto out; + + ret = eficonfig_select_file_handler(_info); + if (ret != EFI_SUCCESS) + goto out; + + ret = efi_open_volume_int(file_info.current_volume, ); + if (ret != EFI_SUCCESS) + goto out; + + ret = efi_file_open_int(root, , file_info.current_path, EFI_FILE_MODE_READ, 0); + if (ret != EFI_SUCCESS) + goto out; + + size = 0; + ret = EFI_CALL(f->getinfo(f, _file_info_guid, , NULL)); + if (ret != EFI_BUFFER_TOO_SMALL) + goto out; + + buf = calloc(1, size); + if (!buf) { + ret = EFI_OUT_OF_RESOURCES; + goto out; + } + ret = EFI_CALL(f->getinfo(f, _file_info_guid, , buf)); + if (ret != EFI_SUCCESS) + goto out; + + size = ((struct efi_file_info *)buf)->file_size; + free(buf); You should set buf to NULL here. Assigning NULL would have no effect. The variable is reassigned in the next line. + + buf = calloc(1, size); + if (!buf) + goto out; + + ret = efi_file_read_int(f, , buf); + if (ret != EFI_SUCCESS || size == 0) + goto out; + + attr = EFI_VARIABLE_NON_VOLATILE | + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS | + EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS; + /* PK can enroll only one certificate */ + if (u16_strcmp(data, u"PK")) { + efi_uintn_t db_size = 0; + + /* check the variable exists. If exists, add APPEND_WRITE attribute */ + ret = efi_get_variable_int(data, efi_auth_var_get_guid(data), NULL, + _size, NULL, NULL); + if (ret == EFI_BUFFER_TOO_SMALL) + attr |= EFI_VARIABLE_APPEND_WRITE; + } + Why are we appending? Shouldn't we always overwrite the platform key? The UEFI specification says: "The PK variable contains *the*
Re: [PATCH v8 1/9] efi_loader: expose END device path node
On 6/19/22 06:55, Masahisa Kojima wrote: This commit exposes the END device path node. Signed-off-by: Masahisa Kojima Reviewed-by: Heinrich Schuchardt --- No change in v8 Newly created in v7 include/efi_loader.h | 3 +++ lib/efi_loader/efi_device_path.c | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/include/efi_loader.h b/include/efi_loader.h index f6651e2c60..c6df29993c 100644 --- a/include/efi_loader.h +++ b/include/efi_loader.h @@ -798,6 +798,9 @@ ssize_t efi_dp_check_length(const struct efi_device_path *dp, (((_dp)->type == DEVICE_PATH_TYPE_##_type) && \ ((_dp)->sub_type == DEVICE_PATH_SUB_TYPE_##_subtype)) +/* template END node: */ +extern const struct efi_device_path END; + /* Indicate supported runtime services */ efi_status_t efi_init_runtime_supported(void); diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c index 50a988c561..4798cec622 100644 --- a/lib/efi_loader/efi_device_path.c +++ b/lib/efi_loader/efi_device_path.c @@ -30,7 +30,7 @@ const efi_guid_t efi_guid_virtio_dev = U_BOOT_VIRTIO_DEV_GUID; #endif /* template END node: */ -static const struct efi_device_path END = { +const struct efi_device_path END = { .type = DEVICE_PATH_TYPE_END, .sub_type = DEVICE_PATH_SUB_TYPE_END, .length = sizeof(END),
Re: [PATCH v8 4/9] menu: add KEY_PLUS and KEY_MINUS handling
On 6/19/22 06:56, Masahisa Kojima wrote: This is preparation to support menu-driven UEFI BootOrder variable updated by KEY_PLUS and KEY_MINUS. Signed-off-by: Masahisa Kojima Reviewed-by: Heinrich Schuchardt
Re: [PATCH v8 3/9] eficonfig: add "Edit Boot Option" menu entry
On 6/19/22 06:56, Masahisa Kojima wrote: This commit adds the menu entry to edit the existing BOOT variable contents. User selects the item from the boot option list, then user can edit the description, file path and optional_data. Note that automatically generated boot option entry by bootmenu to suppport the removable media device is filtered out and user can not edit the automatically generated entry. Signed-off-by: Masahisa Kojima --- Changes in v8: - fix menu header string - fix function and structure prefix to "eficonfig" Newly created in v7 cmd/eficonfig.c | 172 1 file changed, 172 insertions(+) diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c index 20747db115..0a58b83ea3 100644 --- a/cmd/eficonfig.c +++ b/cmd/eficonfig.c @@ -1197,6 +1197,177 @@ out: return ret; } Please, provide function descriptions. Best regards Heinrich +static efi_status_t eficonfig_process_boot_selected(void *data) +{ + struct eficonfig_boot_selection_data *info = data; + + if (info) + *info->selected = info->bootorder_index; + + return EFI_SUCCESS; +} + +static efi_status_t eficonfig_show_boot_selection(u16 *bootorder, efi_uintn_t count, + int *selected) +{ + u32 i; + efi_status_t ret; + efi_uintn_t size, actual_count = 0; + void *load_option; + struct efi_load_option lo; + u16 varname[] = u"Boot"; + struct eficonfig_item *menu_item, *iter; + + menu_item = calloc(count + 1, sizeof(struct eficonfig_item)); + if (!menu_item) { + ret = EFI_OUT_OF_RESOURCES; + goto out; + } + + iter = menu_item; + for (i = 0; i < count; i++) { + efi_create_indexed_name(varname, sizeof(varname), + "Boot", bootorder[i]); + load_option = efi_get_var(varname, _global_variable_guid, ); + if (!load_option) + continue; + + ret = efi_deserialize_load_option(, load_option, ); + if (ret != EFI_SUCCESS) { + log_warning("Invalid load option for %ls\n", varname); + free(load_option); + continue; + } + + if (size >= sizeof(efi_guid_t) && + !guidcmp(lo.optional_data, _guid_bootmenu_auto_generated)) { + /* +* auto generated entry has GUID in optional_data, +* skip auto generated entry because it will be generated +* again even if it is edited or deleted. +*/ + free(load_option); + continue; + } + + if (lo.attributes & LOAD_OPTION_ACTIVE) { + char *buf, *p; + struct eficonfig_boot_selection_data *info; + + info = calloc(1, sizeof(struct eficonfig_boot_selection_data)); + if (!info) { + free(load_option); + ret = EFI_OUT_OF_RESOURCES; + goto out; + } + + buf = calloc(1, utf16_utf8_strlen(lo.label) + 1); + if (!buf) { + free(load_option); + free(info); + ret = EFI_OUT_OF_RESOURCES; + goto out; + } + p = buf; + utf16_utf8_strcpy(, lo.label); + info->bootorder_index = i; + info->selected = selected; + iter->title = buf; + iter->func = eficonfig_process_boot_selected; + iter->data = info; + iter++; + actual_count++; + } + free(load_option); + } + + /* add "Quit" entry */ + iter->title = strdup("Quit"); + iter->func = eficonfig_process_quit; + iter->data = NULL; + actual_count += 1; + + ret = eficonfig_process_common(menu_item, actual_count, " ** Select Boot Option **"); + +out: + iter = menu_item; + for (i = 0; i < actual_count; i++, iter++) { + free(iter->title); + free(iter->data); + } + + free(menu_item); + + return ret; +} + +static efi_status_t eficonfig_process_edit_boot_option(void *data) +{ + u16 *bootorder; + efi_status_t ret; + efi_uintn_t num, size; + struct eficonfig_boot_option *bo = NULL; + + bootorder = efi_get_var(u"BootOrder", _global_variable_guid, ); + if (!bootorder) { + eficonfig_print_msg("BootOrder is not
Re: [PATCH v8 2/9] eficonfig: menu-driven addition of UEFI boot option
On 6/19/22 06:56, Masahisa Kojima wrote: This commit add the "eficonfig" command. The "eficonfig" command implements the menu-driven UEFI boot option maintenance feature. This commit implements the addition of new boot option. User can select the block device volume having efi_simple_file_system_protocol and select the file corresponding to the Boot variable. User can also enter the description and optional_data of the BOOT variable in utf8. This commit adds "include/efi_config.h", it contains the common definition to be used from other menus such as UEFI Secure Boot key management. Signed-off-by: Masahisa Kojima --- Changes in v8: - command name is change from "efimenu" to "eficonfig" - function and struct prefixes is changed to "eficonfig" - fix menu header string Changes in v7: - add "efimenu" command and uefi variable maintenance code moved into cmd/efimenu.c - create include/efimenu.h to define the common definition for the other menu such as UEFI Secure Boot key management - update boot option edit UI, user can select description, file, and optional_data to edit in the same menu like following. ** Edit Boot Option ** Description: debian File: virtio 0:1/EFI\debian\grubaa64.efi Optional Data: test Save Quit - remove exit parameter from efimenu_process_common() - menu title type is changed from u16 to char - efimenu_process_common() add menu title string - reduce printf/puts function call for displaying the menu - efi_console_get_u16_string() accept 0 length to allow optional_data is empty - efi_console_get_u16_string() the "size" parameter name is changes to "count" - efimenu is now designed to maintain the UEFI variables, remove autoboot related code - remove one empty line before "Quit" entry - efimenu_init() processes only the first time Changes in v6: - fix typos - modify volume name to match U-Boot syntax - compile in CONFIG_EFI_LOADER=n and CONFIG_CMD_BOOTEFI_BOOTMGR=n - simplify u16_strncmp() usage - support "a\b.efi" file path, use link list to handle filepath - modify length check condition - UEFI related menu items only appears with CONFIG_AUTOBOOT_MENU_SHOW=y Changes in v5: - remove forward declarations - add const qualifier for menu items - fix the possible unaligned access for directory info access - split into three commit 1)add boot option 2) delete boot option 3)change boot order This commit is 1)add boot option. - fix file name buffer allocation size, it should be EFI_BOOTMENU_FILE_PATH_MAX * sizeof(u16) - fix wrong size checking for file selection Chanes in v4: - UEFI boot option maintenance menu is integrated into bootmenu - display the simplified volume name(e.g. usb0:1, nvme1:2) for the volume selection - instead of extending lib/efi_loader/efi_bootmgr.c, newly create lib/efi_loader/efi_bootmenu_maintenance.c and implement boot variable maintenance into it. Changes in RFC v3: not included in v3 series Changes in RFC v2: - enable utf8 user input for boot option name - create lib/efi_loader/efi_console.c::efi_console_get_u16_string() for utf8 user input handling - use u16_strlcat instead of u16_strcat - remove the EFI_CALLs, and newly create or expose the following xxx_int() functions. efi_locate_handle_buffer_int(), efi_open_volume_int(), efi_file_open_int(), efi_file_close_int(), efi_file_read_int() and efi_file_setpos_int(). Note that EFI_CALLs still exist for EFI_DEVICE_PATH_TO_TEXT_PROTOCOL and EFI_SIMPLE_TEXT_INPUT/OUTPUT_PROTOCOL - use efi_search_protocol() instead of calling locate_protocol() to get the device_path_to_text_protocol interface. - remove unnecessary puts(ANSI_CLEAR_LINE), this patch is still depends on puts(ANSI_CLEAR_CONSOLE) - skip SetVariable() if the bootorder is not changed cmd/Kconfig |7 + cmd/Makefile |1 + cmd/eficonfig.c | 1270 + include/efi_config.h | 91 +++ include/efi_loader.h | 40 ++ lib/efi_loader/efi_boottime.c | 52 +- lib/efi_loader/efi_console.c | 78 ++ lib/efi_loader/efi_disk.c | 11 + lib/efi_loader/efi_file.c | 75 +- 9 files changed, 1578 insertions(+), 47 deletions(-) create mode 100644 cmd/eficonfig.c create mode 100644 include/efi_config.h diff --git a/cmd/Kconfig b/cmd/Kconfig index 09193b61b9..bb7f1d0463 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1870,6 +1870,13 @@ config CMD_EFIDEBUG particularly for managing boot parameters as well as examining various EFI status for debugging. +config CMD_EFICONFIG + bool "eficonfig - provide menu-driven uefi variables maintenance interface" + depends on CMD_BOOTEFI_BOOTMGR + help + Enable the 'eficonfig' command which provides the menu-driven UEFI + variable maintenance interface. + config CMD_EXCEPTION bool "exception - raise exception" depends on ARM ||
[PATCH v1] arm: relocate: Replace ADR instruction with non-pseudo-instruction
In Binutils 2.37 the ADR instruction has changed use alternate instructions. The change causes armv7-m to not boot. Signed-off-by: Jesse Taube --- arch/arm/lib/relocate.S | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/arch/arm/lib/relocate.S b/arch/arm/lib/relocate.S index 14b7f61c1a..22c419534f 100644 --- a/arch/arm/lib/relocate.S +++ b/arch/arm/lib/relocate.S @@ -78,7 +78,13 @@ ENDPROC(relocate_vectors) */ ENTRY(relocate_code) - adr r3, relocate_code +/* + * Binutils doesn't comply with the arm docs for adr in thumb2 + * from commit d3e52e120b68bf19552743fbc078e0a759f48cb7 onward + * to remove ambiguity explicitly define the pseudo-instruction + */ + mov r3, pc + subs r3, #4 ldr r1, _image_copy_start_ofs add r1, r3 /* r1 <- Run &__image_copy_start */ subsr4, r0, r1 /* r4 <- Run to copy offset */ -- 2.36.1