Re: [PATCH v2 0/2] Fix 'no USB device found' error.
On 13:59-20230623, Tom Rini wrote: > On Fri, Jun 23, 2023 at 09:42:01AM +0200, Julien Panis wrote: > > > > > > On 6/22/23 17:49, Tom Rini wrote: > > > On Thu, Jun 22, 2023 at 04:34:34PM +0200, Julien Panis wrote: > > > > This series fixes usb0 dr_mode for am335x-icev2 > > > > and am335x-evmsk. It must be set to 'peripheral' > > > > in order to avoid 'no USB device found' error, > > > > in usb_ether_init() function. > > > > > > > > Signed-off-by: Julien Panis > > > > --- > > > > Changes in v2: > > > > - Drop the modification made in arch/arm/mach-omap2/am33xx/board.c > > > > - Configure usb0 dr_mode as peripheral in 'am335x-icev2-u-boot.dtsi' > > > >and 'am335x-evmsk-u-boot.dtsi' device trees. > > > > - Link to v1: > > > > https://lore.kernel.org/r/20230621-fix_usb_ether_init-v1-1-215692399...@baylibre.com > > > > > > > > --- > > > > Julien Panis (2): > > > >arm: dts: am335x-icev2-u-boot: Configure peripheral mode for usb0 > > > >arm: dts: am335x-evmsk-u-boot: Configure peripheral mode for usb0 > > > > > > > > arch/arm/dts/am335x-evmsk-u-boot.dtsi | 4 > > > > arch/arm/dts/am335x-icev2-u-boot.dtsi | 4 > > > > 2 files changed, 8 insertions(+) > > > I'll ask the first question that Nishanth might also ask, which is why > > > don't these belong in the kernel dts files? Thanks! > > > > > > > That's a good question. :) Looping Nishanth... > > usb0 dr_mode property is already overlayed for am335x-evm, > > in 'am335x-evm-u-boot.dtsi'. So, it appeared more consistent > > to me to do the same thing for am335x-icev2 and am335x-evmsk. > > I guess that the goal is to configure usb0 as host by default at > > kernel boot, since we do not necessarily want to use this usb0 > > as peripheral from userspace. > > Is it the right explanation @Vignesh @Nishanth ? > > It's I think even more likely that the am335x_evm fragment needs to go > upstream too. Adding Tony to the thread, but I think it is better to send the changes to upstream kernel. -- Regards Nishanth Menon
[PATCH 1/6] mips: cpu: Use plain puts() in restart handler
This removes dependency on fprintf() , which is not available
in SPL unless full printf support is enabled.
Signed-off-by: Marek Vasut
---
Cc: Daniel Schwierzeck
Cc: Marek Vasut
---
arch/mips/cpu/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/mips/cpu/cpu.c b/arch/mips/cpu/cpu.c
index b304026a67c..f0e20da28f7 100644
--- a/arch/mips/cpu/cpu.c
+++ b/arch/mips/cpu/cpu.c
@@ -15,7 +15,7 @@
#if !CONFIG_IS_ENABLED(SYSRESET)
void __weak _machine_restart(void)
{
- fprintf(stderr, "*** reset failed ***\n");
+ puts("*** reset failed ***\n");
while (1)
/* NOP */;
--
2.40.1
[PATCH 2/6] ARM: at91: Switch sama5d2_icp_mmc to simple malloc in SPL
To avoid SRAM overflow in the SPL build, use simple malloc implementation. Signed-off-by: Marek Vasut --- Cc: Eugen Hristev --- configs/sama5d2_icp_mmc_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/configs/sama5d2_icp_mmc_defconfig b/configs/sama5d2_icp_mmc_defconfig index 6b7e308680e..185694d4319 100644 --- a/configs/sama5d2_icp_mmc_defconfig +++ b/configs/sama5d2_icp_mmc_defconfig @@ -41,6 +41,7 @@ CONFIG_SPL_MAX_SIZE=0x1 CONFIG_SPL_HAS_BSS_LINKER_SECTION=y CONFIG_SPL_BSS_START_ADDR=0x2000 CONFIG_SPL_BSS_MAX_SIZE=0x8 +CONFIG_SPL_SYS_MALLOC_SIMPLE=y # CONFIG_SPL_SHARES_INIT_SP_ADDR is not set CONFIG_SYS_SPL_MALLOC=y CONFIG_SYS_SPL_MALLOC_SIZE=0x8 -- 2.40.1
[PATCH 3/6] ARM: imx: Add weak default reset_cpu()
Add weak default reset_cpu() implementation needed by e.g. panic().
Signed-off-by: Marek Vasut
---
Cc: "NXP i.MX U-Boot Team"
Cc: Fabio Estevam
Cc: Stefano Babic
---
arch/arm/mach-imx/cpu.c | 4
1 file changed, 4 insertions(+)
diff --git a/arch/arm/mach-imx/cpu.c b/arch/arm/mach-imx/cpu.c
index 702cfc33275..488638c9058 100644
--- a/arch/arm/mach-imx/cpu.c
+++ b/arch/arm/mach-imx/cpu.c
@@ -510,3 +510,7 @@ char nxp_board_rev_string(void)
return (*rev + nxp_board_rev() - 1);
}
#endif
+
+__weak void reset_cpu(void)
+{
+}
--
2.40.1
[PATCH 4/6] spl: spl_legacy: Add extra address checks
Check whether the loaded image or entry point does not overlap SPL.
Signed-off-by: Marek Vasut
---
Cc: "NXP i.MX U-Boot Team"
Cc: Fabio Estevam
Cc: Heiko Schocher
Cc: Heinrich Schuchardt
Cc: Rasmus Villemoes
Cc: Simon Glass
Cc: Stefano Babic
Cc: Tom Rini
Cc: Ye Li
---
V2: - Use _start instead of __image_copy_start and __bss_end for end
- Define SYS_BOOTM_LEN for LEGACY_IMAGE_FORMAT || SPL_LEGACY_IMAGE_FORMAT
---
cmd/Kconfig | 3 ++-
common/spl/spl_legacy.c | 20
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 365371fb511..02e54f1e50f 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -362,7 +362,8 @@ config BOOTM_VXWORKS
config SYS_BOOTM_LEN
hex "Maximum size of a decompresed OS image"
- depends on CMD_BOOTM || CMD_BOOTI || CMD_BOOTZ
+ depends on CMD_BOOTM || CMD_BOOTI || CMD_BOOTZ || \
+ LEGACY_IMAGE_FORMAT || SPL_LEGACY_IMAGE_FORMAT
default 0x400 if PPC || ARM64
default 0x100 if X86 || ARCH_MX6 || ARCH_MX7
default 0x80
diff --git a/common/spl/spl_legacy.c b/common/spl/spl_legacy.c
index 16851c55eb5..d34bc5492e8 100644
--- a/common/spl/spl_legacy.c
+++ b/common/spl/spl_legacy.c
@@ -7,6 +7,7 @@
#include
#include
#include
+#include
#include
#include
@@ -15,6 +16,22 @@
#define LZMA_LEN (1 << 20)
+static void spl_parse_legacy_validate(uintptr_t start, uintptr_t size)
+{
+ uintptr_t spl_start = (uintptr_t)_start;
+ uintptr_t spl_end = (uintptr_t)__bss_end;
+ uintptr_t end = start + size;
+
+ if ((start >= spl_start && start < spl_end) ||
+ (end > spl_start && end <= spl_end) ||
+ (start < spl_start && end >= spl_end) ||
+ (start > end && end > spl_start))
+ panic("SPL: Image overlaps SPL\n");
+
+ if (size > CONFIG_SYS_BOOTM_LEN)
+ panic("SPL: Image too large\n");
+}
+
int spl_parse_legacy_header(struct spl_image_info *spl_image,
const struct legacy_img_hdr *header)
{
@@ -58,6 +75,9 @@ int spl_parse_legacy_header(struct spl_image_info *spl_image,
"payload image: %32s load addr: 0x%lx size: %d\n",
spl_image->name, spl_image->load_addr, spl_image->size);
+ spl_parse_legacy_validate(spl_image->load_addr, spl_image->size);
+ spl_parse_legacy_validate(spl_image->entry_point, 0);
+
return 0;
}
--
2.40.1
[PATCH 5/6] imx: hab: Fix a couple of build warnings with DEBUG enabled
In case the DEBUG is enabled, these three lines warn about cast of
pointer to integer of different size, add the missing casts to fix
the warnings.
Signed-off-by: Marek Vasut
---
Cc: "NXP i.MX U-Boot Team"
Cc: Fabio Estevam
Cc: Heiko Schocher
Cc: Heinrich Schuchardt
Cc: Rasmus Villemoes
Cc: Simon Glass
Cc: Stefano Babic
Cc: Tom Rini
Cc: Ye Li
---
V2: No change
---
arch/arm/mach-imx/hab.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/arm/mach-imx/hab.c b/arch/arm/mach-imx/hab.c
index c6747b257c4..439cdaf07a7 100644
--- a/arch/arm/mach-imx/hab.c
+++ b/arch/arm/mach-imx/hab.c
@@ -932,10 +932,10 @@ int imx_hab_authenticate_image(uint32_t ddr_start,
uint32_t image_size,
printf("ivt entry = 0x%08x, dcd = 0x%08x, csf = 0x%08x\n", ivt->entry,
ivt->dcd, ivt->csf);
puts("Dumping IVT\n");
- print_buffer(ivt_addr, (void *)(ivt_addr), 4, 0x8, 0);
+ print_buffer(ivt_addr, (void *)(uintptr_t)(ivt_addr), 4, 0x8, 0);
puts("Dumping CSF Header\n");
- print_buffer(ivt->csf, (void *)(ivt->csf), 4, 0x10, 0);
+ print_buffer(ivt->csf, (void *)(uintptr_t)(ivt->csf), 4, 0x10, 0);
#if !defined(CONFIG_SPL_BUILD)
get_hab_status();
@@ -944,7 +944,7 @@ int imx_hab_authenticate_image(uint32_t ddr_start, uint32_t
image_size,
puts("\nCalling authenticate_image in ROM\n");
printf("\tivt_offset = 0x%x\n", ivt_offset);
printf("\tstart = 0x%08lx\n", start);
- printf("\tbytes = 0x%x\n", bytes);
+ printf("\tbytes = 0x%lx\n", (ulong)bytes);
#endif
#ifndef CONFIG_ARM64
--
2.40.1
[PATCH 6/6] imx: hab: Simplify the mechanism
The current mechanism is unnecessarily complex. Simplify the whole mechanism
such that the entire fitImage is signed, IVT is placed at the end, followed
by CSF, and this entire bundle is also authenticated. This makes the signing
scripting far simpler.
Signed-off-by: Marek Vasut
---
Cc: "NXP i.MX U-Boot Team"
Cc: Fabio Estevam
Cc: Heiko Schocher
Cc: Heinrich Schuchardt
Cc: Rasmus Villemoes
Cc: Simon Glass
Cc: Stefano Babic
Cc: Tom Rini
Cc: Ye Li
---
V2: Use CONFIG_TEXT_BASE + CONFIG_SYS_BOOTM_LEN if CONFIG_SPL_LOAD_FIT_ADDRESS
is not defined
---
arch/arm/dts/imx8mm-u-boot.dtsi | 2 +
arch/arm/dts/imx8mn-u-boot.dtsi | 2 +
arch/arm/dts/imx8mp-u-boot.dtsi | 2 +
arch/arm/dts/imx8mq-u-boot.dtsi | 2 +
arch/arm/mach-imx/spl.c | 67 ++---
common/spl/spl_fit.c | 7 --
doc/imx/habv4/csf_examples/mx8m/csf.sh| 28 ++-
doc/imx/habv4/csf_examples/mx8m/csf_fit.txt | 10 +--
doc/imx/habv4/guides/mx8m_spl_secure_boot.txt | 74 ---
include/spl.h | 6 --
10 files changed, 54 insertions(+), 146 deletions(-)
diff --git a/arch/arm/dts/imx8mm-u-boot.dtsi b/arch/arm/dts/imx8mm-u-boot.dtsi
index 9c8ec1cf5b1..391fb10ed96 100644
--- a/arch/arm/dts/imx8mm-u-boot.dtsi
+++ b/arch/arm/dts/imx8mm-u-boot.dtsi
@@ -81,7 +81,9 @@
fit {
description = "Configuration to load ATF before U-Boot";
+#ifndef CONFIG_IMX_HAB
fit,external-offset = ;
+#endif
fit,fdt-list = "of-list";
#address-cells = <1>;
diff --git a/arch/arm/dts/imx8mn-u-boot.dtsi b/arch/arm/dts/imx8mn-u-boot.dtsi
index cef20dab468..5046b38e4e2 100644
--- a/arch/arm/dts/imx8mn-u-boot.dtsi
+++ b/arch/arm/dts/imx8mn-u-boot.dtsi
@@ -145,7 +145,9 @@
fit {
description = "Configuration to load ATF before U-Boot";
+#ifndef CONFIG_IMX_HAB
fit,external-offset = ;
+#endif
fit,fdt-list = "of-list";
#address-cells = <1>;
diff --git a/arch/arm/dts/imx8mp-u-boot.dtsi b/arch/arm/dts/imx8mp-u-boot.dtsi
index 18d1728e1d2..68cd0e10f02 100644
--- a/arch/arm/dts/imx8mp-u-boot.dtsi
+++ b/arch/arm/dts/imx8mp-u-boot.dtsi
@@ -103,7 +103,9 @@
fit {
description = "Configuration to load ATF before U-Boot";
+#ifndef CONFIG_IMX_HAB
fit,external-offset = ;
+#endif
fit,fdt-list = "of-list";
#address-cells = <1>;
diff --git a/arch/arm/dts/imx8mq-u-boot.dtsi b/arch/arm/dts/imx8mq-u-boot.dtsi
index b3fef862b49..90b2274754b 100644
--- a/arch/arm/dts/imx8mq-u-boot.dtsi
+++ b/arch/arm/dts/imx8mq-u-boot.dtsi
@@ -97,7 +97,9 @@
fit {
description = "Configuration to load ATF before U-Boot";
+#ifndef CONFIG_IMX_HAB
fit,external-offset = ;
+#endif
#address-cells = <1>;
images {
diff --git a/arch/arm/mach-imx/spl.c b/arch/arm/mach-imx/spl.c
index cb9801b7a13..6c13b00ef10 100644
--- a/arch/arm/mach-imx/spl.c
+++ b/arch/arm/mach-imx/spl.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
DECLARE_GLOBAL_DATA_PTR;
@@ -315,47 +316,21 @@ ulong board_spl_fit_size_align(ulong size)
size = ALIGN(size, 0x1000);
size += CONFIG_CSF_SIZE;
- return size;
-}
+ if (size > CONFIG_SYS_BOOTM_LEN)
+ panic("spl: ERROR: image too big\n");
-void board_spl_fit_post_load(const void *fit)
-{
- u32 offset = ALIGN(fdt_totalsize(fit), 0x1000);
-
- if (imx_hab_authenticate_image((uintptr_t)fit,
- offset + IVT_SIZE + CSF_PAD_SIZE,
- offset)) {
- panic("spl: ERROR: image authentication unsuccessful\n");
- }
+ return size;
}
#endif
void *board_spl_fit_buffer_addr(ulong fit_size, int sectors, int bl_len)
{
- int align_len = ARCH_DMA_MINALIGN - 1;
-
- /* Some devices like SDP, NOR, NAND, SPI are using bl_len =1, so their
fit address
-* is different with SD/MMC, this cause mismatch with signed address.
Thus, adjust
-* the bl_len to align with SD/MMC.
-*/
- if (bl_len < 512)
- bl_len = 512;
-
- return (void *)((CONFIG_TEXT_BASE - fit_size - bl_len -
- align_len) & ~align_len);
-}
+#if defined(CONFIG_SPL_LOAD_FIT_ADDRESS)
+ return (void *)CONFIG_SPL_LOAD_FIT_ADDRESS;
+#else
+ return (void *)(CONFIG_TEXT_BASE + CONFIG_SYS_BOOTM_LEN);
#endif
-
-#if defined(CONFIG_MX6) && defined(CONFIG_SPL_OS_BOOT)
-int dram_init_banksize(void)
-{
- gd->bd->bi_dram[0].start = CFG_SYS_SDRAM_BASE;
- gd->bd->bi_dram[0].size = imx_ddr_size();
-
RE: [PATCH v1] cache_v8: agilex5: Disable Dcache in the SPL
> -Original Message- > From: Marek Vasut > Sent: Wednesday, 21 June, 2023 10:20 PM > To: Marc Zyngier ; Lim, Jit Loon > Cc: u-boot@lists.denx.de; Jagan Teki ; Simon > ; Chee, Tien Fong > ; Hea, Kok Kiang ; > Lokanathan, Raaj ; Maniyam, Dinesh > ; Ng, Boon Khai ; > Yuslaimi, Alif Zakuan ; Chong, Teik Heng > ; Zamri, Muhammad Hazim Izzat > ; Tang, Sieu Mun > ; Ying-Chun Liu ; Lee, Kah > Jing > Subject: Re: [PATCH v1] cache_v8: agilex5: Disable Dcache in the SPL > > On 6/21/23 16:15, Marc Zyngier wrote: > > On Wed, 21 Jun 2023 15:06:51 +0100, > > Jit Loon Lim wrote: > >> > >> From: Kah Jing Lee > >> > >> Dcache feature is not enabled in SPL and enable it will cause ISR > >> exception. Since the Dcache is not supported in SPL, new > >> CONFIG_SPL_SYS_DISABLE_DCACHE_OPS is added to Kconfig to disable > >> Dcache in SPL. > >> > >> Signed-off-by: Kah Jing Lee > > > > This is missing your own SoB. > > > > Now, I'd like to understand what you are actually trying to fix. What > > is this 'ISR' exception? This isn't something the architecture > > describes. Unless you are using CMOs on something that isn't memory or > > for which you don't have a mapping, this should never generate an > > exception. > > You beat me to it, indeed, thanks ! The intention of doing this is because when we init SDMMC driver, the driver will call the invalidate_dcache_range. However, during that time, the dcache is not available in SPL yet thus causing exception. https://elixir.bootlin.com/u-boot/latest/source/drivers/mmc/sdhci.c#L181 -> https://elixir.bootlin.com/u-boot/latest/source/include/linux/dma-mapping.h#L55 -> https://elixir.bootlin.com/u-boot/latest/source/arch/arm/cpu/armv8/cache_v8.c#L475
Re: [PATCH 2/6] ARM: at91: Switch sama5d2_icp_mmc to simple malloc in SPL
On 6/25/23 04:37, Tom Rini wrote: On Sat, Jun 24, 2023 at 12:34:54AM +0200, Marek Vasut wrote: To avoid SRAM overflow in the SPL build, use simple malloc implementation. Signed-off-by: Marek Vasut Applied to u-boot/master, thanks! Why is this applied to master and not through at91 tree ?
Re: [PATCH] ARM: imx: romapi: Fix signed integer bitwise ops misuse
On 6/26/23 03:12, Peng Fan wrote: On 6/25/2023 6:34 PM, Marek Vasut wrote: Bitwise operations on signed integers are not defined, replace then with per-call checks. You mean C99 standard? I think U-Boot is C99, why ?
Re: [PATCH 1/1] doc: bind man-page
Hi Heinrich, On Thu, 22 Jun 2023 at 16:38, Heinrich Schuchardt wrote: > > provide a man-page for the bind command > > Signed-off-by: Heinrich Schuchardt > --- > doc/usage/cmd/bind.rst | 103 + > doc/usage/index.rst| 1 + > 2 files changed, 104 insertions(+) > create mode 100644 doc/usage/cmd/bind.rst Reviewed-by: Simon Glass Good that you found an example. This works because the DT fills in the info, I suppose. Regards, Simon
Re: [PATCH 2/6] ARM: at91: Switch sama5d2_icp_mmc to simple malloc in SPL
On 6/26/23 11:52, Marek Vasut wrote: To avoid SRAM overflow in the SPL build, use simple malloc implementation. Signed-off-by: Marek Vasut --- Cc: Eugen Hristev --- configs/sama5d2_icp_mmc_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/configs/sama5d2_icp_mmc_defconfig b/configs/sama5d2_icp_mmc_defconfig index 6b7e308680e..185694d4319 100644 --- a/configs/sama5d2_icp_mmc_defconfig +++ b/configs/sama5d2_icp_mmc_defconfig @@ -41,6 +41,7 @@ CONFIG_SPL_MAX_SIZE=0x1 CONFIG_SPL_HAS_BSS_LINKER_SECTION=y CONFIG_SPL_BSS_START_ADDR=0x2000 CONFIG_SPL_BSS_MAX_SIZE=0x8 +CONFIG_SPL_SYS_MALLOC_SIMPLE=y # CONFIG_SPL_SHARES_INIT_SP_ADDR is not set CONFIG_SYS_SPL_MALLOC=y CONFIG_SYS_SPL_MALLOC_SIZE=0x8 why is the email patch arriving after patch was already applied? not to say that it was applied so fast and without going through at91 tree.
Re: [PATCH 1/1] cmd: loads man-page
On Sun, 25 Jun 2023 at 22:27, Heinrich Schuchardt wrote: > > Provide a man-page for the loads command. > > Signed-off-by: Heinrich Schuchardt > --- > doc/usage/cmd/loads.rst | 96 + > doc/usage/index.rst | 1 + > 2 files changed, 97 insertions(+) > create mode 100644 doc/usage/cmd/loads.rst Reviewed-by: Simon Glass
Re: [PATCH v2 6/8] Makefile: Add a target for building capsules
On Sat, 24 Jun 2023 at 14:42, Sughosh Ganu wrote: > > Add a target for building EFI capsules. The capsule parameters are > specified through a config file, and the path to the config file is > specified through CONFIG_EFI_CAPSULE_CFG_FILE. When the config file is > not specified, the command only builds tools. > > Signed-off-by: Sughosh Ganu > --- > Changes since V1: > * Call the mkeficapsule utility with the cfg-file parameter when > building capsules via the config file. > > Makefile | 9 + > 1 file changed, 9 insertions(+) NAK, please do not add new output-file rules to Makefile
Re: [PATCH 1/1] doc: fix typo loady in loadb man-page
On Sun, 25 Jun 2023 at 22:24, Heinrich Schuchardt wrote: > > %s/loady/loadb/ > > Signed-off-by: Heinrich Schuchardt > --- > doc/usage/cmd/loadb.rst | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > Reviewed-by: Simon Glass
Re: [PATCH 1/1] doc: saves man-page
Hi Heinrich, On Sun, 25 Jun 2023 at 14:17, Heinrich Schuchardt wrote: > > Provide a man-page for the saves command. > > Signed-off-by: Heinrich Schuchardt > --- > doc/usage/cmd/saves.rst | 88 + > doc/usage/index.rst | 1 + > 2 files changed, 89 insertions(+) > create mode 100644 doc/usage/cmd/saves.rst Reviewed-by: Simon Glass > > diff --git a/doc/usage/cmd/saves.rst b/doc/usage/cmd/saves.rst > new file mode 100644 > index 00..57299029ff > --- /dev/null > +++ b/doc/usage/cmd/saves.rst > @@ -0,0 +1,88 @@ > +.. SPDX-License-Identifier: GPL-2.0+: > + > +saves command > += > + > +Synopsis > + > + > +:: > + > +saves [offset [size [baud]]] > + > +Description > +--- > + > +The *saves* command is used to transfer a file from the device via the serial > +line using the Motorola S-record file format. > + > +offset > +start address of memory area to save, defaults to 0x0 > + > +size > +size of memory area to save, defaults to 0x0 > + > +baud > +baud rate to use for download. This parameter is only available if Would 'upload' be better in this case? > +CONFIG_SYS_LOADS_BAUD_CHANGE=y > + > +Example > +--- > + > +In the example the *screen* command is used to connect to the U-Boot serial > +console. > + > +In a first screen session a file is loaded form the SD-card and the *saves* from > +command is invoked. is used to kill the screen session. > + > +A new screen session is started which logs the output to a file and the > + key is hit to start the file output. is issued to kill > the > +screen session. > + > +The log file is converted to a binary file using the *srec_cat* command. > +A negative offset of -1337982976 (= -0x4c00) is applied to compensate for We might be better to use: -4c00 (decimal xxx) since U-Boot does default to hex > +the offset used in the *saves* command. > + > +.. code-block:: > + > +$ screen /dev/ttyUSB0 115200 > +=> echo $scriptaddr > +0x4FC0 > +=> load mmc 0:1 $scriptaddr boot.txt > +124 bytes read in 1 ms (121.1 KiB/s) > +=> saves $scriptaddr $filesize > +## Ready for S-Record upload, press ENTER to proceed ... > +Really kill this window [y/n] > +$ screen -Logfile out.srec -L /dev/ttyUSB0 115200 > +S003FC > +S3154FC0736574656E76206175746F6C6F616420AD > +S3154FC000106E6F0A646863700A6C6F6164206D6D633E > +S3154FC0002020303A3120246664745F616464725F72B3 > +S3154FC00030206474620A6C6F6164206D6D6320303AC0 > +S3154FC000403120246B65726E656C5F616464725F72DA > +S3154FC0005020736E702E6566690A626F6F74656669C6 > +S3154FC0006020246B65726E656C5F616464725F7220CB > +S3114FC00070246664745F616464725F720A38 > +S705FA > +## S-Record upload complete > +=> > +Really kill this window [y/n] > +$ srec_cat out.srec -offset -1337982976 -Output out.txt -binary > 2>/dev/null > +$ cat out.txt > +setenv autoload no > +dhcp > +load mmc 0:1 $fdt_addr_r dtb > +load mmc 0:1 $kernel_addr_r snp.efi > +bootefi $kernel_addr_r $fdt_addr_r > +$ > + > +Configuration > +- > + > +The command is only available if CONFIG_CMD_SAVES=y. The parameter to set the > +baud rate is only available if CONFIG_SYS_LOADS_BAUD_CHANGE=y > + > +Return value > + > + > +The return value $? is 0 (true) on success, 1 (false) otherwise. > diff --git a/doc/usage/index.rst b/doc/usage/index.rst > index ef3e87fed0..388e59f173 100644 > --- a/doc/usage/index.rst > +++ b/doc/usage/index.rst > @@ -85,6 +85,7 @@ Shell commands > cmd/read > cmd/reset > cmd/rng > + cmd/saves > cmd/sbi > cmd/sf > cmd/scp03 > -- > 2.40.1 > Regards, Simon
Re: [PATCH] sql: Kconfig: TPL_BANNER_PRINT depends on DEBUG_UART && TPL_SERIAL
On Sun, 25 Jun 2023 at 13:35, wrote: > > From: Ying Sun > > As implemented in the arch/arm/mach-rockchip/tpl.c file, > the CONFIG_TPL_BANNER_PRINT option will not work > if either of these options is not enabled. > > Add dependency constraints to the CONFIG_TPL_BANNER_PRINT option > definition to prevent configuration problems > where option is enabled but do not take effect. > > Suggested-by: Yanjie Ren > Signed-off-by: Ying Sun > --- > common/spl/Kconfig.tpl | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/common/spl/Kconfig.tpl b/common/spl/Kconfig.tpl > index 1874f9db4f..3d6cf1e59f 100644 > --- a/common/spl/Kconfig.tpl > +++ b/common/spl/Kconfig.tpl > @@ -43,6 +43,7 @@ config TPL_FRAMEWORK > > config TPL_BANNER_PRINT > bool "Enable output of the TPL banner 'U-Boot TPL ...'" > + depends on DEBUG_UART && TPL_SERIAL I don't think it should depend on DEBUG_UART since it is possible to enable the normal UART in TPL. TPL_SERIAL looks right though. Perhaps the condition should be || ? > default y > help > If this option is enabled, TPL will print the banner with version > -- > 2.17.1 > Regards, Simon
Re: [PATCH 1/1] cmd: fix loads, saves on sandbox
Hi Heinrich,
On Sun, 25 Jun 2023 at 10:54, Heinrich Schuchardt
wrote:
>
> The loads and saves commands crash on the sandbox due to illegal memory
> access.
>
> For command line arguments the sandbox uses a virtual address space which
> does not equal the addresses of the memory allocated with memmap(). Add the
> missing address translations for the loads and saves commands.
>
> Signed-off-by: Heinrich Schuchardt
> ---
> cmd/load.c | 16
> 1 file changed, 12 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass
>
> diff --git a/cmd/load.c b/cmd/load.c
> index 5c4f34781d..2715cf5957 100644
> --- a/cmd/load.c
> +++ b/cmd/load.c
> @@ -181,13 +181,17 @@ static ulong load_serial(long offset)
> } else
> #endif
> {
> + void *dst;
> +
> ret = lmb_reserve(&lmb, store_addr, binlen);
> if (ret) {
> printf("\nCannot overwrite reserved area
> (%08lx..%08lx)\n",
> store_addr, store_addr + binlen);
> return ret;
> }
> - memcpy((char *)(store_addr), binbuf, binlen);
> + dst = map_sysmem(store_addr, binlen);
> + memcpy(dst, binbuf, binlen);
> + unmap_sysmem(dst);
> lmb_free(&lmb, store_addr, binlen);
> }
> if ((store_addr) < start_addr)
> @@ -350,15 +354,19 @@ static int save_serial(ulong address, ulong count)
> if(write_record(SREC3_START)) /* write the header */
> return (-1);
> do {
> - if(count) { /*
> collect hex data in the buffer */
> - c = *(volatile uchar*)(address + reclen); /*
> get one byte*/
> - checksum += c;
> /* accumulate checksum */
> + volatile uchar *src;
> +
> + src = map_sysmem(address, count);
> + if (count) {/* collect hex data
> in the buffer */
> + c = src[reclen];/* get one byte */
> + checksum += c; /* accumulate
> checksum */
> data[2*reclen] = hex[(c>>4)&0x0f];
> data[2*reclen+1] = hex[c & 0x0f];
> data[2*reclen+2] = '\0';
> ++reclen;
> --count;
> }
> + unmap_sysmem((void *)src);
nit: You should not need the cast.
> if(reclen == SREC_BYTES_PER_RECORD || count == 0) {
> /* enough data collected for one record: dump it */
> if(reclen) {/* build & write a data record: */
> --
> 2.40.1
>
Regards,
Simon
Re: [PATCH] cmd: CONFIG_CMD_SAVES depends on CONFIG_CMD_LOADS
On Sun, 25 Jun 2023 at 09:25, wrote: > > From: Ying Sun > > CONFIG_CMD_SAVES is used to enable support for the "saveenv" command > and is only implemented in cmd/load.c > when "#if defined(CONFIG_CMD_LOADS)" is met. > > It is recommended to add dependency constraints to its definition. > Prevents "saveenv" command from not being supported > when "CONFIG_CMD_SAVES=y CONFIG_CMD_LOADS=n". > > Suggested-by: Yanjie Ren > Signed-off-by: Ying Sun > --- > cmd/Kconfig | 1 + > 1 file changed, 1 insertion(+) > Reviewed-by: Simon Glass
Re: [PATCH 0/6] nokia_nx51: Attempts to debug keyboard
Hi Pali, On Sat, 24 Jun 2023 at 09:44, Pali Rohár wrote: > > On Saturday 17 June 2023 11:49:47 Simon Glass wrote: > > This series is an attempt to get the keyboard working properly with > > bootmenu. > > > > It fixes the board's tstc() function, which should be in drivers/input > > > > It also adjusts stopping of the menu autodelay - it should only stop when > > a whole escape sequence is received, not when the first escape character > > is received. That is pretty minor, so we could drop that patch. > > > > This series also adds some debugging output. This seems to make things > > work correctly, suggesting that there is some other problem. > > > > I also see this message fairly often: > > > > cyclic function rx51_watchdog took too long: 1us vs 1000us max > > > > > > Simon Glass (6): > > menu: Re-enable the ANSI codes > > nokia_rx51: Correct tstc() implementation > > bootmenu: Cancel delay only when a real key is pressed > > boobtmenu: Add debugging > > getch: debugging > > menu: Add debugging > > > > board/nokia/rx51/rx51.c | 3 +++ > > cmd/bootmenu.c | 19 --- > > common/cli_getch.c | 23 --- > > common/menu.c | 7 ++- > > 4 files changed, 41 insertions(+), 11 deletions(-) > > > > -- > > 2.41.0.162.gfafddb0af9-goog > > > > Hello, it looks like that this patch series is not finish yet (contains > some commented code, etc.). Anyway, I have tested it and it makes it > even worse. For example PAGE DOWN key in bootmenu on emulated UART > terminal completely stopped working and do nothing. So this patch series > still does not fix this problem. Indeed, but I was hoping it would help you to find the problem. The keyboard driver is currently returning 0 from tst() even when characters are available (i.e. a call to getc() will return valid chars but tstc() returns 0. I attempted to fix this, but perhaps you can do a proper fix? With that bug in place, it cannot work properly with the menu. See the docs for tstc: /** * tstc() - check if a key is available * * @dev: Device to check * @return 0 if no key is available, 1 if a key is available, -ve on *error */ int (*tstc)(struct udevice *dev); Regards, Simon
Re: [PATCH 1/2] disk: part: Add API to get partitions with specific driver
Hi Joshua,
On Fri, 23 Jun 2023 at 21:01, Joshua Watt wrote:
>
> Adds part_driver_get_type() API which can be used to force a specific
> driver to be used when getting partition information instead of relying
> on auto detection.
>
> Signed-off-by: Joshua Watt
> ---
> disk/part.c| 38 +++---
> include/part.h | 2 ++
> 2 files changed, 33 insertions(+), 7 deletions(-)
>
> diff --git a/disk/part.c b/disk/part.c
> index 35300df590..1f8c786ca5 100644
> --- a/disk/part.c
> +++ b/disk/part.c
> @@ -26,6 +26,22 @@
> /* Check all partition types */
> #define PART_TYPE_ALL -1
>
> +static struct part_driver *part_driver_get_type(int part_type)
> +{
> + struct part_driver *drv =
> + ll_entry_start(struct part_driver, part_driver);
> + const int n_ents = ll_entry_count(struct part_driver, part_driver);
> + struct part_driver *entry;
> +
> + for (entry = drv; entry != drv + n_ents; entry++) {
> + if (part_type == entry->part_type)
> + return entry;
> + }
> +
> + /* Not found */
> + return NULL;
> +}
> +
> static struct part_driver *part_driver_lookup_type(struct blk_desc *dev_desc)
> {
> struct part_driver *drv =
> @@ -44,10 +60,7 @@ static struct part_driver *part_driver_lookup_type(struct
> blk_desc *dev_desc)
> }
> }
> } else {
> - for (entry = drv; entry != drv + n_ents; entry++) {
> - if (dev_desc->part_type == entry->part_type)
> - return entry;
> - }
> + return part_driver_get_type(dev_desc->part_type);
> }
>
> /* Not found */
> @@ -306,8 +319,8 @@ void part_print(struct blk_desc *dev_desc)
> drv->print(dev_desc);
> }
>
> -int part_get_info(struct blk_desc *dev_desc, int part,
> - struct disk_partition *info)
> +int part_get_info_by_type(struct blk_desc *dev_desc, int part, int part_type,
> + struct disk_partition *info)
> {
> struct part_driver *drv;
>
> @@ -320,7 +333,12 @@ int part_get_info(struct blk_desc *dev_desc, int part,
> info->type_guid[0] = 0;
> #endif
>
> - drv = part_driver_lookup_type(dev_desc);
> + if (part_type == PART_TYPE_UNKNOWN) {
> + drv = part_driver_lookup_type(dev_desc);
> + } else {
> + drv = part_driver_get_type(part_type);
> + }
> +
> if (!drv) {
> debug("## Unknown partition table type %x\n",
> dev_desc->part_type);
> @@ -340,6 +358,12 @@ int part_get_info(struct blk_desc *dev_desc, int part,
> return -ENOENT;
> }
>
> +int part_get_info(struct blk_desc *dev_desc, int part,
> + struct disk_partition *info)
> +{
> + return part_get_info_by_type(dev_desc, part, PART_TYPE_UNKNOWN, info);
> +}
> +
> int part_get_info_whole_disk(struct blk_desc *dev_desc,
> struct disk_partition *info)
> {
> diff --git a/include/part.h b/include/part.h
> index be75c73549..f150c84206 100644
> --- a/include/part.h
> +++ b/include/part.h
> @@ -106,6 +106,8 @@ struct blk_desc *blk_get_dev(const char *ifname, int dev);
> struct blk_desc *mg_disk_get_dev(int dev);
>
> /* disk/part.c */
> +int part_get_info_by_type(struct blk_desc *dev_desc, int part, int part_type,
> + struct disk_partition *info);
Please can you add a full comment?
Also please add a test to test/dm/part.c for your new function[1]
> int part_get_info(struct blk_desc *dev_desc, int part,
> struct disk_partition *info);
> /**
> --
> 2.33.0
>
Regards,
Simon
Re: [PATCH 1/7] capsule: authenticate: Embed capsule public key in platform's dtb
Hi Sughosh,
On Wed, 21 Jun 2023 at 05:20, Sughosh Ganu wrote:
>
> hi Simon,
>
> On Mon, 19 Jun 2023 at 18:07, Simon Glass wrote:
> >
> > Hi Sughosh,
> >
> > On Thu, 15 Jun 2023 at 17:11, Sughosh Ganu wrote:
> > >
> > > hi Simon,
> > >
> > > On Thu, 15 Jun 2023 at 14:44, Simon Glass wrote:
> > > >
> > > > Hi Sughosh,
> > > >
> > > > On Tue, 13 Jun 2023 at 11:41, Sughosh Ganu
> > > > wrote:
> > > > >
> > > > > The EFI capsule authentication logic in u-boot expects the public key
> > > > > in the form of an EFI Signature List(ESL) to be provided as part of
> > > > > the platform's dtb. Currently, the embedding of the ESL file into the
> > > > > dtb needs to be done manually.
> > > > >
> > > > > Add a script for embedding the ESL used for capsule authentication in
> > > > > the platform's dtb, and call this as part of building the dtb(s). This
> > > > > brings the embedding of the ESL in the dtb into the u-boot build flow.
> > > > >
> > > > > The path to the ESL file is specified through the
> > > > > CONFIG_EFI_CAPSULE_ESL_FILE symbol.
> > > > >
> > > > > Signed-off-by: Sughosh Ganu
> > > > > ---
> > > > > lib/efi_loader/Kconfig | 11 +++
> > > > > scripts/Makefile.lib | 8
> > > > > scripts/embed_capsule_key.sh | 25 +
> > > > > 3 files changed, 44 insertions(+)
> > > > > create mode 100755 scripts/embed_capsule_key.sh
> > > > >
> > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > > > > index c5835e6ef6..1326a1d109 100644
> > > > > --- a/lib/efi_loader/Kconfig
> > > > > +++ b/lib/efi_loader/Kconfig
> > > > > @@ -234,6 +234,17 @@ config EFI_CAPSULE_MAX
> > > > > Select the max capsule index value used for capsule report
> > > > > variables. This value is used to create CapsuleMax variable.
> > > > >
> > > > > +config EFI_CAPSULE_ESL_FILE
> > > > > + string "Path to the EFI Signature List File"
> > > > > + default ""
> > > > > + depends on EFI_CAPSULE_AUTHENTICATE
> > > > > + help
> > > > > + Provides the absolute path to the EFI Signature List
> > > > > + file which will be embedded in the platform's device
> > > > > + tree and used for capsule authentication at the time
> > > > > + of capsule update.
> > > > > +
> > > > > +
> > > > > config EFI_DEVICE_PATH_TO_TEXT
> > > > > bool "Device path to text protocol"
> > > > > default y
> > > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
> > > > > index 7b27224b5d..a4083d0a26 100644
> > > > > --- a/scripts/Makefile.lib
> > > > > +++ b/scripts/Makefile.lib
> > > > > @@ -192,6 +192,8 @@ dtc_cpp_flags = -Wp,-MD,$(depfile).pre.tmp
> > > > > -nostdinc\
> > > > > -D__ASSEMBLY__
> > > > >\
> > > > > -undef -D__DTS__
> > > > >
> > > > > +export dtc_cpp_flags
> > > > > +
> > > > > # Finds the multi-part object the current object will be linked into
> > > > > modname-multi = $(sort $(foreach m,$(multi-used),\
> > > > > $(if $(filter $(subst $(obj)/,,$*.o),
> > > > > $($(m:.o=-objs)) $($(m:.o=-y))),$(m:.o=
> > > > > @@ -315,6 +317,9 @@ ifeq ($(CONFIG_OF_LIBFDT_OVERLAY),y)
> > > > > DTC_FLAGS += -@
> > > > > endif
> > > > >
> > > > > +quiet_cmd_embedcapsulekey = EMBEDCAPSULEKEY $@
> > > > > +cmd_embedcapsulekey = $(srctree)/scripts/embed_capsule_key.sh $@
> > > > > +
> > > > > quiet_cmd_dtc = DTC $@
> > > > > # Modified for U-Boot
> > > > > # Bring in any U-Boot-specific include at the end of the file
> > > > > @@ -333,6 +338,9 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \
> > > > >
> > > > > $(obj)/%.dtb: $(src)/%.dts FORCE
> > > > > $(call if_changed_dep,dtc)
> > > > > +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y)
> > > > > + $(call cmd,embedcapsulekey,$@)
> > > > > +endif
> > > > >
> > > > > pre-tmp = $(subst $(comma),_,$(dot-target).pre.tmp)
> > > > > dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp)
> > > > > diff --git a/scripts/embed_capsule_key.sh
> > > > > b/scripts/embed_capsule_key.sh
> > > > > new file mode 100755
> > > > > index 00..1c2e45f758
> > > > > --- /dev/null
> > > > > +++ b/scripts/embed_capsule_key.sh
> > > > > @@ -0,0 +1,25 @@
> > > > > +#! /bin/bash
> > > > > +# SPDX-License-Identifier: GPL-2.0+
> > > > > +#
> > > > > +# Copyright (C) 2023, Linaro Limited
> > > > > +#
> > > > > +
> > > > > +gen_capsule_signature_file() {
> > > > > +cat >> $1 << EOF
> > > > > +/dts-v1/;
> > > > > +/plugin/;
> > > > > +
> > > > > +&{/} {
> > > > > + signature {
> > > > > + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > > > > + };
> > > > > +};
> > > > > +EOF
> > > > > +}
> > > > > +
> > > > > +gen_capsule_signature_file signature.$$.dts > /dev/null 2>&1
> > > > > +$CPP $dtc_cpp_flags -x assembler-with-cpp -o signature.$$.tmp
> > > > > signature.$$.dts > /dev/null 2>&1
> > > > > +dtc -@ -O d
Re: [PATCH 5/7] Makefile: Add a target for building capsules
Hi Sughosh, On Wed, 21 Jun 2023 at 05:26, Sughosh Ganu wrote: > > hi Simon, > > On Mon, 19 Jun 2023 at 18:07, Simon Glass wrote: > > > > Hi Sughosh, > > > > On Thu, 15 Jun 2023 at 17:25, Sughosh Ganu wrote: > > > > > > hi Simon, > > > > > > On Thu, 15 Jun 2023 at 14:44, Simon Glass wrote: > > > > > > > > Hi Sughosh, > > > > > > > > On Tue, 13 Jun 2023 at 11:39, Sughosh Ganu > > > > wrote: > > > > > > > > > > Add a target for building EFI capsules. The capsule parameters are > > > > > specified through a config file, and the path to the config file is > > > > > specified through CONFIG_EFI_CAPSULE_CFG_FILE. When the config file is > > > > > not specified, the command only builds tools. > > > > > > > > > > Signed-off-by: Sughosh Ganu > > > > > --- > > > > > Makefile | 9 + > > > > > 1 file changed, 9 insertions(+) > > > > > > > > > > diff --git a/Makefile b/Makefile > > > > > index 10bfaa52ad..96db29aa77 100644 > > > > > --- a/Makefile > > > > > +++ b/Makefile > > > > > @@ -1151,6 +1151,15 @@ dtbs: dts/dt.dtb > > > > > dts/dt.dtb: u-boot > > > > > $(Q)$(MAKE) $(build)=dts dtbs > > > > > > > > > > +quiet_cmd_mkeficapsule = MKEFICAPSULE $@ > > > > > +cmd_mkeficapsule = $(objtree)/tools/mkeficapsule $@ > > > > > + > > > > > +PHONY += capsule > > > > > +capsule: tools > > > > > +ifneq ($(CONFIG_EFI_CAPSULE_CFG_FILE),"") > > > > > + $(call cmd,mkeficapsule) > > > > > +endif > > > > > + > > > > > quiet_cmd_copy = COPY$@ > > > > >cmd_copy = cp $< $@ > > > > > > > > > > -- > > > > > 2.34.1 > > > > > > > > > > > > > We should be using binman to build images...you seem to be building > > > > something in parallel with that. Can you please take a look at binman? > > > > > > Again, I had explored using binman for this task. The one issue where > > > I find the above flow better is that I can simply build my payload > > > image(s) followed by 'make capsule' to generate the capsules for > > > earlier generated images. In it's current form, I don't see an easy > > > way to enforce this dependency in binman when I want to build the > > > payload followed by generation of capsules. I did see the mention of > > > encapsulating an entry within another dependent entry, but I think > > > that makes the implementation more complex than it ought to be. > > > > > > I think it is much easier to use the make flow to generate the images > > > followed by capsules, instead of tweaking the binman node to first > > > generate the payload images, followed by enabling the capsule node to > > > build the capsules. If there is an easy way of enforcing this > > > dependency, please let me know. Thanks > > > > Can you share your explorations? I think the capsule should be created > > as part of the build, if enabled. Rather than changing the input > > files, binman should produce new output files. > > This is an issue of handling dependencies in binman, and not changing > input files. We do not have support for telling binman "build/generate > this particular image first before you proceed to build the capsules > using the earlier built images". I am not sure if this can be done in > a generic manner in binman, so that irrespective of the image being > generated, it can be specified to build capsules once the capsule > input images have been generated. I'm just not sure what you are getting out here. See INPUTS-y for the input files to binman. Then binman uses these to generate output files. It does not mess with the input files, nor should it. Please read the top part of the Binman motivation to understand how all this works. At the risk of repeating myself, we want the Makefile to focus on building U-Boot, with Binman handling the laterprocessing of those files. Binman may run as part of the U-Boot build, and/or can be run later, with the input files. > > > > > We are trying to remove most of the output logic in Makefile. It > > should just be producing input files for binman. > > I understand. However, like I mentioned above, as of now, we don't > have a way of handling dependencies in binman, at least in a generic > manner. Once this support gets added, I know that it would be trivial > to add support for building capsules in binman. What dependencies do you need? Please describe it in a simple manner so I can understand. It cannot involve change the binman input files. Regards, Simon
Re: [PATCH v1] cache_v8: agilex5: Disable Dcache in the SPL
On Mon, 26 Jun 2023 10:00:09 +0100, "Lim, Jit Loon" wrote: > > > > > -Original Message- > > From: Marek Vasut > > Sent: Wednesday, 21 June, 2023 10:20 PM > > To: Marc Zyngier ; Lim, Jit Loon > > Cc: u-boot@lists.denx.de; Jagan Teki ; Simon > > ; Chee, Tien Fong > > ; Hea, Kok Kiang ; > > Lokanathan, Raaj ; Maniyam, Dinesh > > ; Ng, Boon Khai ; > > Yuslaimi, Alif Zakuan ; Chong, Teik Heng > > ; Zamri, Muhammad Hazim Izzat > > ; Tang, Sieu Mun > > ; Ying-Chun Liu ; Lee, Kah > > Jing > > Subject: Re: [PATCH v1] cache_v8: agilex5: Disable Dcache in the SPL > > > > On 6/21/23 16:15, Marc Zyngier wrote: > > > On Wed, 21 Jun 2023 15:06:51 +0100, > > > Jit Loon Lim wrote: > > >> > > >> From: Kah Jing Lee > > >> > > >> Dcache feature is not enabled in SPL and enable it will cause ISR > > >> exception. Since the Dcache is not supported in SPL, new > > >> CONFIG_SPL_SYS_DISABLE_DCACHE_OPS is added to Kconfig to disable > > >> Dcache in SPL. > > >> > > >> Signed-off-by: Kah Jing Lee > > > > > > This is missing your own SoB. > > > > > > Now, I'd like to understand what you are actually trying to fix. What > > > is this 'ISR' exception? This isn't something the architecture > > > describes. Unless you are using CMOs on something that isn't memory or > > > for which you don't have a mapping, this should never generate an > > > exception. > > > > You beat me to it, indeed, thanks ! > > The intention of doing this is because when we init SDMMC driver, > the driver will call the invalidate_dcache_range. However, during > that time, the dcache is not available in SPL yet thus causing > exception. Again: which exception? There is no such thing as "the dcache isn't available". It is always valid to invalidate it, irrespective of it being enabled or not. My impression is that you are papering over another bug (such as feeding non-memory to the CMOs), and are getting an SError back, which would be entirely justified. M. -- Without deviation from the norm, progress is not possible.
Re: [PATCH 1/7] capsule: authenticate: Embed capsule public key in platform's dtb
Hi Simon, [...] > > > > > > + > > > > > > +gen_capsule_signature_file signature.$$.dts > /dev/null 2>&1 > > > > > > +$CPP $dtc_cpp_flags -x assembler-with-cpp -o signature.$$.tmp > > > > > > signature.$$.dts > /dev/null 2>&1 > > > > > > +dtc -@ -O dtb -o signature.$$.dtbo signature.$$.tmp > /dev/null > > > > > > 2>&1 > > > > > > +fdtoverlay -i $1 -o temp.$$.dtb -v signature.$$.dtbo > /dev/null > > > > > > 2>&1 > > > > > > +mv temp.$$.dtb $1 > /dev/null 2>&1 > > > > > > +rm -f signature.$$.* > /dev/null 2>&1 > > > > > > -- > > > > > > 2.34.1 > > > > > > > > > > > > > > > > Can you please add this to binman instead? > > > > > > > > I had looked at using binman for this work earlier because I very much > > > > expected this comment from you :). Having said that, I am very much > > > > open to using binman instead if it turns out to be the better way of > > > > achieving this. What this patch does is that, with capsule > > > > authentication enabled, it embeds the public key esl file into the > > > > dtb's as they get built. As per my understanding, binman gets called > > > > at the end of the u-boot build, once the constituent images( e..g > > > > u-boot.bin = u-boot-no-dtb.bin + dtb) have been generated. So, if we > > > > call binman _after_ the requisite image(s) have been generated, we > > > > would need to 1) identify the dtb's in which the esl needs to be > > > > embedded, and then 2) generate the final image all over again. Don't > > > > you think this is non optimal? Or is there a way of generating the > > > > constituent images(including the dtb's) through binman instead? > > > > > > The best way to do that IMO is to generate a second file, .e.g. > > > u-boot-capsule.bin > > This make no sense to me whatsoever. Do we have an example in u-boot generating multiple dtb versions for other reasons/subsystems? > > That would break the scripts for platforms which might be using > > u-boot.bin as the image to boot from. I know that the ST platform > > which does enable capsule updates uses the u-boot-nodtb.bin as the > > BL33 image and the u-boot.dtb as BL33_CFG. Hence my question, if we > > have to use binman, is there a way to 1) modify the u-boot.dtb and > > then 2) regenerate u-boot.bin image. > > > > I know this is software, and everything can be done in a hacky way. > > But I was exploring using the u-boot node as a section entry, so that > > the u-boot.dtb can be modified and then binman would > > repackage/regenerate the u-boot.bin. But this is not working. > > NO, please do not do that. You should create a new file, not modify > u-boot.bin or u-boot.dtb. Please just don't mess around with this, it > will lead to all sorts of confusion. > > I thought we already had this discussion a while back? No we haven't. In fact I am struggling to see the confusion part. It's fine for the u-boot dtb to include all the internal nodes DM needs, but suddenly having the capsule signature is problematic? In the past the .esl file was part of the U-Boot binary and things were working perfectly fine. In fact you could update/downgrade u-boot and the signatures naturally followed along instead of having to update u-boot *and* the dtb, which we have to do today. You could also build a capsule way easier without injecting/removing signatures to the dtb. You were the one that insisted on reverting that and instead adding it on the dtb. We explained most of the downsides back then, along with some security concerns. We also mentioned that the signature in the dtb makes little sense since it's difference *per class of boards* and it's not something we could include in static dtb files, but that lead nowhere... As Sughosh already said there are platforms that use the generated u-boot dtb and the raw binary to assemble a FIP image. So why exactly adding the capsule signature to the default dtb is wrong? Why should we add an *extra* .dtb with one additional node -- which is very much needed by the design you proposed. This will only lead to extra confusion. Thanks /Ilias > > > > > > > > > I don't think it is a good idea to add other junk to u-boot.bin. It > > > should just be U-Boot + dtb. > > > > No junk is being added to u-boot.bin. Just that, as the platform > > builds dtb's, the ESL file gets embedded into them as a property under > > the signature node. There is no additional image being added to the > > the u-boot.bin. > > This needs to be done in a separte file, as above. > > Regards, > Simon
[PATCH] efi_loader: make efi_delete_handle() follow the EFI spec
The EFI doesn't allow removal of handles, unless all hosted protocols
are cleanly removed. Our efi_delete_handle() is a bit intrusive.
Although it does try to delete protocols before removing a handle,
it doesn't care if that fails. Instead it only returns an error if the
handle is invalid. On top of that none of the callers of that function
check the return code.
So let's rewrite this in a way that fits the EFI spec better. Instead
of forcing the handle removal, gracefully uninstall all the handle
protocols. According to the EFI spec when the last protocol is removed
the handle will be deleted. Also switch all the callers and check the
return code. Some callers can't do anything useful apart from reporting
an error. The disk related functions on the other hand, can prevent a
medium that is being used by EFI from removal.
The only function that doesn't check the result is efi_delete_image().
But that function needs a bigger rework anyway, so we can clean it up in
the future
Signed-off-by: Ilias Apalodimas
---
Heinrich this needs to be applied on top of
https://lore.kernel.org/u-boot/20230620061932.113292-1-ilias.apalodi...@linaro.org/
cmd/bootefi.c | 19 ---
include/efi_loader.h | 2 +-
lib/efi_driver/efi_uclass.c | 13 ++---
lib/efi_loader/efi_boottime.c | 26 ++
lib/efi_loader/efi_disk.c | 17 +
5 files changed, 42 insertions(+), 35 deletions(-)
diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 8aa15a64c836..60b9e950ffdc 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -606,20 +606,6 @@ failure:
return ret;
}
-/**
- * bootefi_run_finish() - finish up after running an EFI test
- *
- * @loaded_image_info: Pointer to a struct which holds the loaded image info
- * @image_obj: Pointer to a struct which holds the loaded image object
- */
-static void bootefi_run_finish(struct efi_loaded_image_obj *image_obj,
- struct efi_loaded_image *loaded_image_info)
-{
- efi_restore_gd();
- free(loaded_image_info->load_options);
- efi_delete_handle(&image_obj->header);
-}
-
/**
* do_efi_selftest() - execute EFI selftest
*
@@ -638,7 +624,10 @@ static int do_efi_selftest(void)
/* Execute the test */
ret = EFI_CALL(efi_selftest(&image_obj->header, &systab));
- bootefi_run_finish(image_obj, loaded_image_info);
+ efi_restore_gd();
+ free(loaded_image_info->load_options);
+ if (efi_delete_handle(&image_obj->header) != EFI_SUCCESS)
+ log_err("Failed to delete loaded image handle\n");
return ret != EFI_SUCCESS;
}
diff --git a/include/efi_loader.h b/include/efi_loader.h
index e530bcf15f76..43ccf49abec4 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -618,7 +618,7 @@ void efi_add_handle(efi_handle_t obj);
/* Create handle */
efi_status_t efi_create_handle(efi_handle_t *handle);
/* Delete handle */
-void efi_delete_handle(efi_handle_t obj);
+efi_status_t efi_delete_handle(efi_handle_t obj);
/* Call this to validate a handle and find the EFI object for it */
struct efi_object *efi_search_obj(const efi_handle_t handle);
/* Locate device_path handle */
diff --git a/lib/efi_driver/efi_uclass.c b/lib/efi_driver/efi_uclass.c
index 45f935198874..d8755541fc14 100644
--- a/lib/efi_driver/efi_uclass.c
+++ b/lib/efi_driver/efi_uclass.c
@@ -285,10 +285,8 @@ static efi_status_t efi_add_driver(struct driver *drv)
bp->ops = ops;
ret = efi_create_handle(&bp->bp.driver_binding_handle);
- if (ret != EFI_SUCCESS) {
- free(bp);
- goto out;
- }
+ if (ret != EFI_SUCCESS)
+ goto err;
bp->bp.image_handle = bp->bp.driver_binding_handle;
ret = efi_add_protocol(bp->bp.driver_binding_handle,
&efi_guid_driver_binding_protocol, bp);
@@ -299,11 +297,12 @@ static efi_status_t efi_add_driver(struct driver *drv)
if (ret != EFI_SUCCESS)
goto err;
}
-out:
- return ret;
+ return ret;
err:
- efi_delete_handle(bp->bp.driver_binding_handle);
+ if (bp->bp.driver_binding_handle &&
+ efi_delete_handle(bp->bp.driver_binding_handle) != EFI_SUCCESS)
+ log_err("Failed to delete handle\n");
free(bp);
return ret;
}
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index d75a3336e3f1..5123055d7f5d 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -59,6 +59,10 @@ static efi_handle_t current_image;
static volatile gd_t *efi_gd, *app_gd;
#endif
+static efi_status_t efi_uninstall_protocol
+ (efi_handle_t handle, const efi_guid_t *protocol,
+void *protocol_interface);
+
/* 1 if inside U-Boot code, 0 if inside EFI payload code */
static int entry_count = 1;
static int nesting_level;
@@ -610,8 +
[PATCH v3] dt-bindings: riscv: deprecate riscv,isa
intro = When the RISC-V dt-bindings were accepted upstream in Linux, the base ISA etc had yet to be ratified. By the ratification of the base ISA, incompatible changes had snuck into the specifications - for example the Zicsr and Zifencei extensions were spun out of the base ISA. Fast forward to today, and the reason for this patch. Currently the riscv,isa dt property permits only a specific subset of the ISA string - in particular it excludes version numbering. With the current constraints, it is not possible to discern whether "rv64i" means that the hart supports the fence.i instruction, for example. Future systems may choose to implement their own instruction fencing, perhaps using a vendor extension, or they may not implement the optional counter extensions. Software needs a way to determine this. versioning schemes == "Use the extension versions that are described in the ISA manual" you may say, and it's not like this has not been considered. Firstly, software that parses the riscv,isa property at runtime will need to contain a lookup table of some sort that maps arbitrary versions to versions it understands. There is not a consistent application of version number applied to extensions, with a higgledy-piggledy collection of tags, "bare" and versioned documents awaiting the reader on the "recently ratified extensions" page: https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions As an aside, and this is reflected in the patch too, since many extensions have yet to appear in a release of the ISA specs, they are defined by commits in their respective "working draft" repositories. Secondly, there is an issue of backwards compatibility, whereby allowing numbers in the ISA string, some parsers may be broken. This would require an additional property to be created to even use the versions in this manner. ~boolean properties~ string array property == If a new property is needed, the whole approach may as well be looked at from the bottom up. A string with limited character choices etc is hardly the best approach for communicating extension information to software. Switching to using properties that are defined on a per extension basis, allows us to define explicit meanings for the DT representation of each extension - rather than the current situation where different operating systems or other bits of software may impart different meanings to characters in the string. Clearly the best source of meanings is the specifications themselves, this just provides us the ability to choose at what point in time the meaning is set. If an extension changes incompatibility in the future, a new property will be required. Off-list, some of the RVI folks have committed to shoring up the wording in either the ISA specifications, the riscv-isa-manual or so that in the future, modifications to and additions or removals of features will require a new extension. Codifying that assertion somewhere would make it quite unlikely that compatibility would be broken, but we have the tools required to deal with it, if & when it crops up. It is in our collective interest, as consumers of extension meanings, to define a scheme that enforces compatibility. The use of individual properties, rather than elements in a single string, will also permit validation that the properties have a meaning, as well as potentially reject mutually exclusive combinations, or enforce dependencies between extensions. That would not have be possible with the current dt-schema infrastructure for arbitrary strings, as we would need to add a riscv,isa parser to dt-validate! That's not implemented in this patch, but rather left as future work (for the brave, or the foolish). acpi The current ACPI ECR is based on having a single ISA string unfortunately, but ideally ACPI will move to another method, perhaps GUIDs, that give explicit meaning to extensions. parser simplicity = Many systems that parse DT at runtime already implement an function that can check for the presence of a string in an array of string, as it is similar to the process for parsing a list of compatible strings, so a bunch of new, custom, DT parsing should not be needed. Getting rid of "riscv,isa" parsing would be a nice simplification, but unfortunately for backwards compatibility with old dtbs, existing parsers may not be removable - which may greatly simplify dt parsing code. In Linux, for example, checking for whether a hart supports an extension becomes as simple as: of_property_match_string(node, "riscv,isa-extensions", "zicbom") vendor extensions = Compared to riscv,isa, this proposed scheme promotes vendor extensions, oft touted as the strength of RISC-V, to first-class citizens. At present, extensions are defined as meaning what the RISC-V ISA specifications say they do. There is no realistic way of using that interface to provi
[PATCH] mtd: spi-nor: Add support for w25q256jwm
Add support for Winbond 256M-bit flash w25q256jwm.
Performed basic erase/write/readback operations on
ZynqMP zc1751+dc1 board.
Signed-off-by: Venkatesh Yadav Abbarapu
---
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 3f8b796789..53a743a038 100644
--- a/drivers/mtd/spi/spi-nor-ids.c
+++ b/drivers/mtd/spi/spi-nor-ids.c
@@ -446,6 +446,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("w25q256jwm", 0xef8019, 0, 64 * 1024, 512,
+ SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
+ SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
+ },
{ INFO("w25x64", 0xef3017, 0, 64 * 1024, 128, SECT_4K) },
{
INFO("w25q64dw", 0xef6017, 0, 64 * 1024, 128,
--
2.17.1
Re:[PATCH] mmc:Remove thr legacy mode clock setting operation
Hi，Peng.：
Has you meet any issues without removing this line? or this is just code
inpsection?
In practical use, when some EMMC particles switch to the hs mode
below, an error message cmd8 may appear. The reason for this is that the clock
operation of setting the legacy mode was performed before switching to the hs
mode, which caused the clock to switch too early. At this time, the
communication between the host and card is abnormal
That is to say, switching the clocks of the host and card to high speed before
switching to hs mode can cause communication issues between the host and card
BTW the upper "else" will met issue if you directly remove this line.
Delete this line directly. There will indeed be issues with the else above. I
did not find it on my end because I did not open the compilation macro
definition above. This is the patch I resubmitted
Best wishes
Thanks
At 2023-06-26 14:54:47, xf_...@163.com wrote:
>From: xiefei
>
>Due to the need to read the register value before
>switching to hs mode, the standard protocol does
>not explicitly specify that the setting before
>switching to hs mode is in legacy mode. Therefore,
>the code at this point may cause communication
>abnormalities between the host and card
>
>Signed-off-by: xiefei
>---
> drivers/mmc/mmc.c | 2 --
> 1 file changed, 2 deletions(-)
>
>diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>index 1af6af82e6..96cc7e7332 100644
>--- a/drivers/mmc/mmc.c
>+++ b/drivers/mmc/mmc.c
>@@ -2136,9 +2136,7 @@ static int mmc_select_mode_and_width(struct mmc *mmc,
>uint card_caps)
> mmc->selected_mode == MMC_HS_400 ||
> mmc->selected_mode == MMC_HS_400_ES)
> mmc_set_card_speed(mmc, MMC_HS, true);
>- else
> #endif
>- mmc_set_clock(mmc, mmc->legacy_speed, MMC_CLK_ENABLE);
>
> for_each_mmc_mode_by_pref(card_caps, mwt) {
> for_each_supported_width(card_caps & mwt->widths,
>--
>2.17.1
Re:[PATCH] mmc:Remove thr legacy mode clock setting operation
Hi，Peng.：
Has you meet any issues without removing this line? or this is just code
inpsection?
In practical use, when some EMMC particles switch to the hs mode
below, an error message cmd8 may appear. The reason for this is that the clock
operation of setting the legacy mode was performed before switching to the hs
mode, which caused the clock to switch too early. At this time, the
communication between the host and card is abnormal
That is to say, switching the clocks of the host and card to high speed before
switching to hs mode can cause communication issues between the host and card
BTW the upper "else" will met issue if you directly remove this line.
Delete this line directly. There will indeed be issues with the else above. I
did not find it on my end because I did not open the compilation macro
definition above. This is the patch I resubmitted
Best wishes
Thanks
在 2023-06-26 16:59:42，xf_...@163.com 写道：
>From: xiefei
>
>Due to the need to read the register value before
>switching to hs mode, the standard protocol does
>not explicitly specify that the setting before
>switching to hs mode is in legacy mode. Therefore,
>the code at this point may cause communication
>abnormalities between the host and card
>
>Signed-off-by: xiefei
>---
> drivers/mmc/mmc.c | 2 --
> 1 file changed, 2 deletions(-)
>
>diff --git a/drivers/mmc/mmc.c b/drivers/mmc/mmc.c
>index 1af6af82e6..96cc7e7332 100644
>--- a/drivers/mmc/mmc.c
>+++ b/drivers/mmc/mmc.c
>@@ -2136,9 +2136,7 @@ static int mmc_select_mode_and_width(struct mmc *mmc,
>uint card_caps)
> mmc->selected_mode == MMC_HS_400 ||
> mmc->selected_mode == MMC_HS_400_ES)
> mmc_set_card_speed(mmc, MMC_HS, true);
>- else
> #endif
>- mmc_set_clock(mmc, mmc->legacy_speed, MMC_CLK_ENABLE);
>
> for_each_mmc_mode_by_pref(card_caps, mwt) {
> for_each_supported_width(card_caps & mwt->widths,
>--
>2.17.1
Re: [PATCH 1/1] cmd: fix loads, saves on sandbox
On 6/26/23 11:07, Simon Glass wrote:
Hi Heinrich,
On Sun, 25 Jun 2023 at 10:54, Heinrich Schuchardt
wrote:
The loads and saves commands crash on the sandbox due to illegal memory
access.
For command line arguments the sandbox uses a virtual address space which
does not equal the addresses of the memory allocated with memmap(). Add the
missing address translations for the loads and saves commands.
Signed-off-by: Heinrich Schuchardt
---
cmd/load.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
Reviewed-by: Simon Glass
diff --git a/cmd/load.c b/cmd/load.c
index 5c4f34781d..2715cf5957 100644
--- a/cmd/load.c
+++ b/cmd/load.c
@@ -181,13 +181,17 @@ static ulong load_serial(long offset)
} else
#endif
{
+ void *dst;
+
ret = lmb_reserve(&lmb, store_addr, binlen);
if (ret) {
printf("\nCannot overwrite reserved area
(%08lx..%08lx)\n",
store_addr, store_addr + binlen);
return ret;
}
- memcpy((char *)(store_addr), binbuf, binlen);
+ dst = map_sysmem(store_addr, binlen);
+ memcpy(dst, binbuf, binlen);
+ unmap_sysmem(dst);
lmb_free(&lmb, store_addr, binlen);
}
if ((store_addr) < start_addr)
@@ -350,15 +354,19 @@ static int save_serial(ulong address, ulong count)
if(write_record(SREC3_START)) /* write the header */
return (-1);
do {
- if(count) { /*
collect hex data in the buffer */
- c = *(volatile uchar*)(address + reclen); /* get
one byte*/
- checksum += c;
/* accumulate checksum */
+ volatile uchar *src;
+
+ src = map_sysmem(address, count);
+ if (count) {/* collect hex data in
the buffer */
+ c = src[reclen];/* get one byte */
+ checksum += c; /* accumulate checksum
*/
data[2*reclen] = hex[(c>>4)&0x0f];
data[2*reclen+1] = hex[c & 0x0f];
data[2*reclen+2] = '\0';
++reclen;
--count;
}
+ unmap_sysmem((void *)src);
nit: You should not need the cast.
Without the cast I get a build warning:
cmd/load.c: In function ‘save_serial’:
cmd/load.c:369:30: warning: passing argument 1 of ‘unmap_sysmem’
discards ‘volatile’ qualifier from pointer target type
[-Wdiscarded-qualifiers]
369 | unmap_sysmem(src);
| ^~~
In file included from include/mapmem.h:14,
from cmd/load.c:22:
./arch/sandbox/include/asm/io.h:40:45: note: expected ‘const void *’ but
argument is of type ‘volatile uchar *’ {aka ‘volatile unsigned char *’}
40 | static inline void unmap_sysmem(const void *vaddr)
| ^
Why Wolfgang introduced volatile in the original code is unclear to me.
Would anybody try to save register contents as S-records?
See commit fe8c2806cdba ("Initial revision")
common/cmd_boot.c:484:
c = *(volatile uchar*)(address + reclen); /* get one byte*/
Best regards
Heinrich
if(reclen == SREC_BYTES_PER_RECORD || count == 0) {
/* enough data collected for one record: dump it */
if(reclen) {/* build & write a data record: */
--
2.40.1
Regards,
Simon
[PATCH v2] board: rockchip: add Radxa ROCK5A Rk3588 board
ROCK 5A is a Rockchip RK3588S based SBC (Single Board Computer) by Radxa.
There are tree variants depending on the DRAM size : 4G, 8G and 16G.
Specifications:
Rockchip Rk3588S SoC
4x ARM Cortex-A76, 4x ARM Cortex-A55
4/8/16GB memory LPDDR4x
Mali G610MC4 GPU
MIPI CSI 2 multiple lanes connector
4-lane MIPI DSI connector
Audio – 3.5mm earphone jack
eMMC module connector
uSD slot (up to 128GB)
2x USB 2.0, 2x USB 3.0
2x micro HDMI 2.1 ports, one up to 8Kp60, the other up to 4Kp60
Gigabit Ethernet RJ45 with optional PoE support
40-pin IO header including UART, SPI, I2C and 5V DC power in
USB PD over USB Type-C
Size: 85mm x 56mm (Raspberry Pi 4 form factor)
Kernel commits:
d1824cf95799 ("arm64: dts: rockchip: Add rock-5a board")
991f136c9f8d ("arm64: dts: rockchip: Update sdhci alias for rock-5a")
304c8a759953 ("arm64: dts: rockchip: Remove empty line from rock-5a")
cda0c2ea65a0 ("arm64: dts: rockchip: Fix RX delay for ethernet phy on
rk3588s-rock5a")
Signed-off-by: Eugen Hristev
---
Changes in v2:
-fixed wrong Kconfig text (rk3588s instead of rk3588)
-changed doc name (rk3588s instead of rk3588)
arch/arm/dts/Makefile | 1 +
arch/arm/dts/rk3588s-rock-5a-u-boot.dtsi| 90 +
arch/arm/dts/rk3588s-rock-5a.dts| 73 +
arch/arm/mach-rockchip/rk3588/Kconfig | 28 +++
board/radxa/rock5a-rk3588s/Kconfig | 15
board/radxa/rock5a-rk3588s/MAINTAINERS | 6 ++
board/radxa/rock5a-rk3588s/Makefile | 6 ++
board/radxa/rock5a-rk3588s/rock5a-rk3588s.c | 39 +
doc/board/rockchip/rockchip.rst | 1 +
9 files changed, 259 insertions(+)
create mode 100644 arch/arm/dts/rk3588s-rock-5a-u-boot.dtsi
create mode 100644 arch/arm/dts/rk3588s-rock-5a.dts
create mode 100644 board/radxa/rock5a-rk3588s/Kconfig
create mode 100644 board/radxa/rock5a-rk3588s/MAINTAINERS
create mode 100644 board/radxa/rock5a-rk3588s/Makefile
create mode 100644 board/radxa/rock5a-rk3588s/rock5a-rk3588s.c
diff --git a/arch/arm/dts/Makefile b/arch/arm/dts/Makefile
index 480269fa6065..cd9b96c5ba7c 100644
--- a/arch/arm/dts/Makefile
+++ b/arch/arm/dts/Makefile
@@ -174,6 +174,7 @@ dtb-$(CONFIG_ROCKCHIP_RK3568) += \
dtb-$(CONFIG_ROCKCHIP_RK3588) += \
rk3588-edgeble-neu6a-io.dtb \
rk3588-evb1-v10.dtb \
+ rk3588s-rock-5a.dtb \
rk3588-rock-5b.dtb
dtb-$(CONFIG_ROCKCHIP_RV1108) += \
diff --git a/arch/arm/dts/rk3588s-rock-5a-u-boot.dtsi
b/arch/arm/dts/rk3588s-rock-5a-u-boot.dtsi
new file mode 100644
index ..a546f9e4dcc2
--- /dev/null
+++ b/arch/arm/dts/rk3588s-rock-5a-u-boot.dtsi
@@ -0,0 +1,90 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2023 Collabora Ltd.
+ */
+
+#include "rk3588s-u-boot.dtsi"
+#include
+#include
+#include
+#include
+
+/ {
+ aliases {
+ mmc1 = &sdmmc;
+ };
+
+ chosen {
+ u-boot,spl-boot-order = "same-as-spl", &sdmmc, &sdhci;
+ };
+};
+
+&emmc_bus8 {
+ bootph-all;
+};
+
+&emmc_clk {
+ bootph-all;
+};
+
+&emmc_cmd {
+ bootph-all;
+};
+
+&emmc_data_strobe {
+ bootph-all;
+};
+
+&emmc_rstnout {
+ bootph-all;
+};
+
+&pinctrl {
+ bootph-all;
+};
+
+&pcfg_pull_none {
+ bootph-all;
+};
+
+&pcfg_pull_up_drv_level_2 {
+ bootph-all;
+};
+
+&pcfg_pull_up {
+ bootph-all;
+};
+
+&sdmmc {
+ bus-width = <4>;
+ status = "okay";
+};
+
+&sdmmc_bus4 {
+ bootph-all;
+};
+
+&sdmmc_clk {
+ bootph-all;
+};
+
+&sdmmc_cmd {
+ bootph-all;
+};
+
+&sdmmc_det {
+ bootph-all;
+};
+
+&sdhci {
+ cap-mmc-highspeed;
+ mmc-ddr-1_8v;
+ mmc-hs200-1_8v;
+ pinctrl-names = "default";
+ pinctrl-0 = <&emmc_bus8 &emmc_clk &emmc_cmd &emmc_data_strobe
&emmc_rstnout>;
+};
+
+&uart2m0_xfer {
+ bootph-all;
+};
+
diff --git a/arch/arm/dts/rk3588s-rock-5a.dts b/arch/arm/dts/rk3588s-rock-5a.dts
new file mode 100644
index ..901825514f9d
--- /dev/null
+++ b/arch/arm/dts/rk3588s-rock-5a.dts
@@ -0,0 +1,73 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+
+/dts-v1/;
+
+#include
+#include
+#include "rk3588s.dtsi"
+
+/ {
+ model = "Radxa ROCK 5 Model A";
+ compatible = "radxa,rock-5a", "rockchip,rk3588s";
+
+ aliases {
+ mmc0 = &sdhci;
+ serial2 = &uart2;
+ };
+
+ chosen {
+ stdout-path = "serial2:150n8";
+ };
+};
+
+&gmac1 {
+ clock_in_out = "output";
+ phy-handle = <&rgmii_phy1>;
+ phy-mode = "rgmii";
+ pinctrl-0 = <&gmac1_miim
+&gmac1_tx_bus2
+&gmac1_rx_bus2
+&gmac1_rgmii_clk
+&gmac1_rgmii_bus>;
+ pinctrl-names = "default";
+ tx_delay = <0x3a>;
+ rx_delay = <0x3e>;
+ status = "okay";
+};
+
+&mdio1 {
+ rgmii_phy1: ethe
Re: [PATCH 1/1] doc: saves man-page
On 6/26/23 11:07, Simon Glass wrote: Hi Heinrich, On Sun, 25 Jun 2023 at 14:17, Heinrich Schuchardt wrote: Provide a man-page for the saves command. Signed-off-by: Heinrich Schuchardt --- doc/usage/cmd/saves.rst | 88 + doc/usage/index.rst | 1 + 2 files changed, 89 insertions(+) create mode 100644 doc/usage/cmd/saves.rst Reviewed-by: Simon Glass diff --git a/doc/usage/cmd/saves.rst b/doc/usage/cmd/saves.rst new file mode 100644 index 00..57299029ff --- /dev/null +++ b/doc/usage/cmd/saves.rst @@ -0,0 +1,88 @@ +.. SPDX-License-Identifier: GPL-2.0+: + +saves command += + +Synopsis + + +:: + +saves [offset [size [baud]]] + +Description +--- + +The *saves* command is used to transfer a file from the device via the serial +line using the Motorola S-record file format. + +offset +start address of memory area to save, defaults to 0x0 + +size +size of memory area to save, defaults to 0x0 + +baud +baud rate to use for download. This parameter is only available if Would 'upload' be better in this case? Copy and paste error. I will fix it. +CONFIG_SYS_LOADS_BAUD_CHANGE=y + +Example +--- + +In the example the *screen* command is used to connect to the U-Boot serial +console. + +In a first screen session a file is loaded form the SD-card and the *saves* from ok +command is invoked. is used to kill the screen session. + +A new screen session is started which logs the output to a file and the + key is hit to start the file output. is issued to kill the +screen session. + +The log file is converted to a binary file using the *srec_cat* command. +A negative offset of -1337982976 (= -0x4c00) is applied to compensate for We might be better to use: -4c00 (decimal xxx) Thanks for reviewing. Maybe I had a typo when trying negative hexadecimal numbers. But it seems to work. I will adjust the man-page. Best regards Heinrich since U-Boot does default to hex +the offset used in the *saves* command. + +.. code-block:: + +$ screen /dev/ttyUSB0 115200 +=> echo $scriptaddr +0x4FC0 +=> load mmc 0:1 $scriptaddr boot.txt +124 bytes read in 1 ms (121.1 KiB/s) +=> saves $scriptaddr $filesize +## Ready for S-Record upload, press ENTER to proceed ... +Really kill this window [y/n] +$ screen -Logfile out.srec -L /dev/ttyUSB0 115200 +S003FC +S3154FC0736574656E76206175746F6C6F616420AD +S3154FC000106E6F0A646863700A6C6F6164206D6D633E +S3154FC0002020303A3120246664745F616464725F72B3 +S3154FC00030206474620A6C6F6164206D6D6320303AC0 +S3154FC000403120246B65726E656C5F616464725F72DA +S3154FC0005020736E702E6566690A626F6F74656669C6 +S3154FC0006020246B65726E656C5F616464725F7220CB +S3114FC00070246664745F616464725F720A38 +S705FA +## S-Record upload complete +=> +Really kill this window [y/n] +$ srec_cat out.srec -offset -1337982976 -Output out.txt -binary 2>/dev/null +$ cat out.txt +setenv autoload no +dhcp +load mmc 0:1 $fdt_addr_r dtb +load mmc 0:1 $kernel_addr_r snp.efi +bootefi $kernel_addr_r $fdt_addr_r +$ + +Configuration +- + +The command is only available if CONFIG_CMD_SAVES=y. The parameter to set the +baud rate is only available if CONFIG_SYS_LOADS_BAUD_CHANGE=y + +Return value + + +The return value $? is 0 (true) on success, 1 (false) otherwise. diff --git a/doc/usage/index.rst b/doc/usage/index.rst index ef3e87fed0..388e59f173 100644 --- a/doc/usage/index.rst +++ b/doc/usage/index.rst @@ -85,6 +85,7 @@ Shell commands cmd/read cmd/reset cmd/rng + cmd/saves cmd/sbi cmd/sf cmd/scp03 -- 2.40.1 Regards, Simon
[PATCH v2 1/1] doc: saves man-page
Provide a man-page for the saves command. Signed-off-by: Heinrich Schuchardt Reviewed-by: Simon Glass --- v2: fix typos use a hexadecimal offset when calling srec_cat --- doc/usage/cmd/saves.rst | 88 + doc/usage/index.rst | 1 + 2 files changed, 89 insertions(+) create mode 100644 doc/usage/cmd/saves.rst diff --git a/doc/usage/cmd/saves.rst b/doc/usage/cmd/saves.rst new file mode 100644 index 00..10419cf25f --- /dev/null +++ b/doc/usage/cmd/saves.rst @@ -0,0 +1,88 @@ +.. SPDX-License-Identifier: GPL-2.0+: + +saves command += + +Synopsis + + +:: + +saves [offset [size [baud]]] + +Description +--- + +The *saves* command is used to transfer a file from the device via the serial +line using the Motorola S-record file format. + +offset +start address of memory area to save, defaults to 0x0 + +size +size of memory area to save, defaults to 0x0 + +baud +baud rate to use for upload. This parameter is only available if +CONFIG_SYS_LOADS_BAUD_CHANGE=y + +Example +--- + +In the example the *screen* command is used to connect to the U-Boot serial +console. + +In a first screen session a file is loaded from the SD-card and the *saves* +command is invoked. is used to kill the screen session. + +A new screen session is started which logs the output to a file and the + key is hit to start the file output. is issued to kill the +screen session. + +The log file is converted to a binary file using the *srec_cat* command. A +negative offset of -0x4c00 is applied to compensate for the offset used in +the *saves* command. + +.. code-block:: + +$ screen /dev/ttyUSB0 115200 +=> echo $scriptaddr +0x4FC0 +=> load mmc 0:1 $scriptaddr boot.txt +124 bytes read in 1 ms (121.1 KiB/s) +=> saves $scriptaddr $filesize +## Ready for S-Record upload, press ENTER to proceed ... +Really kill this window [y/n] +$ screen -Logfile out.srec -L /dev/ttyUSB0 115200 +S003FC +S3154FC0736574656E76206175746F6C6F616420AD +S3154FC000106E6F0A646863700A6C6F6164206D6D633E +S3154FC0002020303A3120246664745F616464725F72B3 +S3154FC00030206474620A6C6F6164206D6D6320303AC0 +S3154FC000403120246B65726E656C5F616464725F72DA +S3154FC0005020736E702E6566690A626F6F74656669C6 +S3154FC0006020246B65726E656C5F616464725F7220CB +S3114FC00070246664745F616464725F720A38 +S705FA +## S-Record upload complete +=> +Really kill this window [y/n] +$ srec_cat out.srec -offset -0x4FC0 -Output out.txt -binary 2>/dev/null +$ cat out.txt +setenv autoload no +dhcp +load mmc 0:1 $fdt_addr_r dtb +load mmc 0:1 $kernel_addr_r snp.efi +bootefi $kernel_addr_r $fdt_addr_r +$ + +Configuration +- + +The command is only available if CONFIG_CMD_SAVES=y. The parameter to set the +baud rate is only available if CONFIG_SYS_LOADS_BAUD_CHANGE=y + +Return value + + +The return value $? is 0 (true) on success, 1 (false) otherwise. diff --git a/doc/usage/index.rst b/doc/usage/index.rst index 29ae8a176e..563d109f8c 100644 --- a/doc/usage/index.rst +++ b/doc/usage/index.rst @@ -84,6 +84,7 @@ Shell commands cmd/read cmd/reset cmd/rng + cmd/saves cmd/sbi cmd/sf cmd/scp03 -- 2.40.1
Re: [PATCH 1/7] capsule: authenticate: Embed capsule public key in platform's dtb
Hi Ilias, On Mon, 26 Jun 2023 at 10:53, Ilias Apalodimas wrote: > > Hi Simon, > > [...] > > > > > > > > + > > > > > > > +gen_capsule_signature_file signature.$$.dts > /dev/null 2>&1 > > > > > > > +$CPP $dtc_cpp_flags -x assembler-with-cpp -o signature.$$.tmp > > > > > > > signature.$$.dts > /dev/null 2>&1 > > > > > > > +dtc -@ -O dtb -o signature.$$.dtbo signature.$$.tmp > /dev/null > > > > > > > 2>&1 > > > > > > > +fdtoverlay -i $1 -o temp.$$.dtb -v signature.$$.dtbo > /dev/null > > > > > > > 2>&1 > > > > > > > +mv temp.$$.dtb $1 > /dev/null 2>&1 > > > > > > > +rm -f signature.$$.* > /dev/null 2>&1 > > > > > > > -- > > > > > > > 2.34.1 > > > > > > > > > > > > > > > > > > > Can you please add this to binman instead? > > > > > > > > > > I had looked at using binman for this work earlier because I very much > > > > > expected this comment from you :). Having said that, I am very much > > > > > open to using binman instead if it turns out to be the better way of > > > > > achieving this. What this patch does is that, with capsule > > > > > authentication enabled, it embeds the public key esl file into the > > > > > dtb's as they get built. As per my understanding, binman gets called > > > > > at the end of the u-boot build, once the constituent images( e..g > > > > > u-boot.bin = u-boot-no-dtb.bin + dtb) have been generated. So, if we > > > > > call binman _after_ the requisite image(s) have been generated, we > > > > > would need to 1) identify the dtb's in which the esl needs to be > > > > > embedded, and then 2) generate the final image all over again. Don't > > > > > you think this is non optimal? Or is there a way of generating the > > > > > constituent images(including the dtb's) through binman instead? > > > > > > > > The best way to do that IMO is to generate a second file, .e.g. > > > > u-boot-capsule.bin > > > > > This make no sense to me whatsoever. Do we have an example in u-boot > generating multiple dtb versions for other reasons/subsystems? > > > > That would break the scripts for platforms which might be using > > > u-boot.bin as the image to boot from. I know that the ST platform > > > which does enable capsule updates uses the u-boot-nodtb.bin as the > > > BL33 image and the u-boot.dtb as BL33_CFG. Hence my question, if we > > > have to use binman, is there a way to 1) modify the u-boot.dtb and > > > then 2) regenerate u-boot.bin image. > > > > > > I know this is software, and everything can be done in a hacky way. > > > But I was exploring using the u-boot node as a section entry, so that > > > the u-boot.dtb can be modified and then binman would > > > repackage/regenerate the u-boot.bin. But this is not working. > > > > NO, please do not do that. You should create a new file, not modify > > u-boot.bin or u-boot.dtb. Please just don't mess around with this, it > > will lead to all sorts of confusion. > > > > I thought we already had this discussion a while back? > > No we haven't. In fact I am struggling to see the confusion part. It's > fine for the u-boot dtb to include all the internal nodes DM needs, but > suddenly having the capsule signature is problematic? > > In the past the .esl file was part of the U-Boot binary and things were > working perfectly fine. In fact you could update/downgrade u-boot and the > signatures naturally followed along instead of having to update u-boot > *and* the dtb, which we have to do today. You could also build a capsule > way easier without injecting/removing signatures to the dtb. > You were the one that insisted on reverting that and instead adding it on > the dtb. We explained most of the downsides back then, along with some > security concerns. We also mentioned that the signature in the dtb makes > little sense since it's difference *per class of boards* and it's not > something we could include in static dtb files, but that lead nowhere... > > As Sughosh already said there are platforms that use the generated u-boot > dtb and the raw binary to assemble a FIP image. So why exactly adding the > capsule signature to the default dtb is wrong? Why should we add an > *extra* .dtb with one additional node -- which is very much needed by the > design you proposed. This will only lead to extra confusion. 1. I thought a capsule update was going to be a separate file, e.g. u-boot-capture.bin ? 2. You can't put the signature into U-Boot. It needs to be in the capsule update so that U-Boot can check it. If you are talking about the public key, then yes that needs to be in U-Boot 3. Where does the public key come from? With the normal verified boot flow it is created (and added to the dtb) as a separate signing step after U-Boot is built. 4. Off topic, but re FIP, can someone just kill that off and use FIT? It is such a shame that a new format was invented... Regards, Simon
Re: [PATCH] menu: Re-enable the ANSI codes
Hi Frank,
On Mon, 26 Jun 2023 at 11:44, Frank Wunderlich wrote:
>
> Hi,
>
> what exact bug does the Patch fix? as i'm listed in reported-By...i remember
> only the problem with missing line-breaks after "normal" menuitems and the
> "u-boot-console" i reported in ML. But i don't think this is related to ANSI
> processing.
It fixes placement of the prompt to select an option. You can ignore
this patch if it isn't related to your report.
Regards,
Simon
>
> regards Frank
>
>
> > Gesendet: Montag, 12. Juni 2023 um 22:14 Uhr
> > Von: "Simon Glass"
> > An: "U-Boot Mailing List"
> > Cc: "Simon Glass" , "Pali Rohár" ,
> > "Mark Kettenis" , "Frank Wunderlich"
> > , "Heinrich Schuchardt" ,
> > "Ilias Apalodimas" , "Masahisa Kojima"
> > , "Stefan Roese"
> > Betreff: [PATCH] menu: Re-enable the ANSI codes
> >
> > The intent here was to allow ANSI codes to be disabled, since it was
> > proving impoosible to test operation of the menu code when it kept moving
> > the cursor. Unfortunately this ended up in the patch.
> >
> > Correct this by enabling ANSI again.
> >
> > Signed-off-by: Simon Glass
> > Reported-by: Pali Rohár
> > Reported-by: Mark Kettenis
> > Reported-by: Frank Wunderlich
> > Fixes: 32bab0eae51b ("menu: Make use of CLI character processing")
> > ---
> >
> > common/menu.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/common/menu.c b/common/menu.c
> > index 94514177e4e9..b55cf7b99967 100644
> > --- a/common/menu.c
> > +++ b/common/menu.c
> > @@ -15,7 +15,7 @@
> >
> > #include "menu.h"
> >
> > -#define ansi 0
> > +#define ansi 1
> >
> > /*
> > * Internally, each item in a menu is represented by a struct menu_item.
> > --
> > 2.41.0.162.gfafddb0af9-goog
> >
> >
Re: [PATCH 1/1] cmd: fix loads, saves on sandbox
Hi Heinrich,
On Mon, 26 Jun 2023 at 11:35, Heinrich Schuchardt
wrote:
>
> On 6/26/23 11:07, Simon Glass wrote:
> > Hi Heinrich,
> >
> > On Sun, 25 Jun 2023 at 10:54, Heinrich Schuchardt
> > wrote:
> >>
> >> The loads and saves commands crash on the sandbox due to illegal memory
> >> access.
> >>
> >> For command line arguments the sandbox uses a virtual address space which
> >> does not equal the addresses of the memory allocated with memmap(). Add the
> >> missing address translations for the loads and saves commands.
> >>
> >> Signed-off-by: Heinrich Schuchardt
> >> ---
> >> cmd/load.c | 16
> >> 1 file changed, 12 insertions(+), 4 deletions(-)
> >
> > Reviewed-by: Simon Glass
> >
> >>
> >> diff --git a/cmd/load.c b/cmd/load.c
> >> index 5c4f34781d..2715cf5957 100644
> >> --- a/cmd/load.c
> >> +++ b/cmd/load.c
> >> @@ -181,13 +181,17 @@ static ulong load_serial(long offset)
> >> } else
> >> #endif
> >> {
> >> + void *dst;
> >> +
> >> ret = lmb_reserve(&lmb, store_addr, binlen);
> >> if (ret) {
> >> printf("\nCannot overwrite reserved area
> >> (%08lx..%08lx)\n",
> >> store_addr, store_addr + binlen);
> >> return ret;
> >> }
> >> - memcpy((char *)(store_addr), binbuf, binlen);
> >> + dst = map_sysmem(store_addr, binlen);
> >> + memcpy(dst, binbuf, binlen);
> >> + unmap_sysmem(dst);
> >> lmb_free(&lmb, store_addr, binlen);
> >> }
> >> if ((store_addr) < start_addr)
> >> @@ -350,15 +354,19 @@ static int save_serial(ulong address, ulong count)
> >> if(write_record(SREC3_START)) /* write the
> >> header */
> >> return (-1);
> >> do {
> >> - if(count) { /*
> >> collect hex data in the buffer */
> >> - c = *(volatile uchar*)(address + reclen); /*
> >> get one byte*/
> >> - checksum += c;
> >> /* accumulate checksum */
> >> + volatile uchar *src;
> >> +
> >> + src = map_sysmem(address, count);
> >> + if (count) {/* collect hex
> >> data in the buffer */
> >> + c = src[reclen];/* get one byte */
> >> + checksum += c; /* accumulate
> >> checksum */
> >> data[2*reclen] = hex[(c>>4)&0x0f];
> >> data[2*reclen+1] = hex[c & 0x0f];
> >> data[2*reclen+2] = '\0';
> >> ++reclen;
> >> --count;
> >> }
> >> + unmap_sysmem((void *)src);
> >
> > nit: You should not need the cast.
>
> Without the cast I get a build warning:
>
> cmd/load.c: In function ‘save_serial’:
> cmd/load.c:369:30: warning: passing argument 1 of ‘unmap_sysmem’
> discards ‘volatile’ qualifier from pointer target type
> [-Wdiscarded-qualifiers]
>369 | unmap_sysmem(src);
>| ^~~
> In file included from include/mapmem.h:14,
> from cmd/load.c:22:
> ./arch/sandbox/include/asm/io.h:40:45: note: expected ‘const void *’ but
> argument is of type ‘volatile uchar *’ {aka ‘volatile unsigned char *’}
> 40 | static inline void unmap_sysmem(const void *vaddr)
>| ^
>
> Why Wolfgang introduced volatile in the original code is unclear to me.
> Would anybody try to save register contents as S-records?
>
> See commit fe8c2806cdba ("Initial revision")
> common/cmd_boot.c:484:
> c = *(volatile uchar*)(address + reclen); /* get one byte*/
Ah yes, the const problem. Perhaps we should have an
unmap_sysmem_const() as a work-around? But this patch is fine.
Regards,
Simon
Re: [PATCH 5/7] Makefile: Add a target for building capsules
hi Simon, On Mon, 26 Jun 2023 at 14:38, Simon Glass wrote: > > Hi Sughosh, > > On Wed, 21 Jun 2023 at 05:26, Sughosh Ganu wrote: > > > > hi Simon, > > > > On Mon, 19 Jun 2023 at 18:07, Simon Glass wrote: > > > > > > Hi Sughosh, > > > > > > On Thu, 15 Jun 2023 at 17:25, Sughosh Ganu > > > wrote: > > > > > > > > hi Simon, > > > > > > > > On Thu, 15 Jun 2023 at 14:44, Simon Glass wrote: > > > > > > > > > > Hi Sughosh, > > > > > > > > > > On Tue, 13 Jun 2023 at 11:39, Sughosh Ganu > > > > > wrote: > > > > > > > > > > > > Add a target for building EFI capsules. The capsule parameters are > > > > > > specified through a config file, and the path to the config file is > > > > > > specified through CONFIG_EFI_CAPSULE_CFG_FILE. When the config file > > > > > > is > > > > > > not specified, the command only builds tools. > > > > > > > > > > > > Signed-off-by: Sughosh Ganu > > > > > > --- > > > > > > Makefile | 9 + > > > > > > 1 file changed, 9 insertions(+) > > > > > > > > > > > > diff --git a/Makefile b/Makefile > > > > > > index 10bfaa52ad..96db29aa77 100644 > > > > > > --- a/Makefile > > > > > > +++ b/Makefile > > > > > > @@ -1151,6 +1151,15 @@ dtbs: dts/dt.dtb > > > > > > dts/dt.dtb: u-boot > > > > > > $(Q)$(MAKE) $(build)=dts dtbs > > > > > > > > > > > > +quiet_cmd_mkeficapsule = MKEFICAPSULE $@ > > > > > > +cmd_mkeficapsule = $(objtree)/tools/mkeficapsule $@ > > > > > > + > > > > > > +PHONY += capsule > > > > > > +capsule: tools > > > > > > +ifneq ($(CONFIG_EFI_CAPSULE_CFG_FILE),"") > > > > > > + $(call cmd,mkeficapsule) > > > > > > +endif > > > > > > + > > > > > > quiet_cmd_copy = COPY$@ > > > > > >cmd_copy = cp $< $@ > > > > > > > > > > > > -- > > > > > > 2.34.1 > > > > > > > > > > > > > > > > We should be using binman to build images...you seem to be building > > > > > something in parallel with that. Can you please take a look at binman? > > > > > > > > Again, I had explored using binman for this task. The one issue where > > > > I find the above flow better is that I can simply build my payload > > > > image(s) followed by 'make capsule' to generate the capsules for > > > > earlier generated images. In it's current form, I don't see an easy > > > > way to enforce this dependency in binman when I want to build the > > > > payload followed by generation of capsules. I did see the mention of > > > > encapsulating an entry within another dependent entry, but I think > > > > that makes the implementation more complex than it ought to be. > > > > > > > > I think it is much easier to use the make flow to generate the images > > > > followed by capsules, instead of tweaking the binman node to first > > > > generate the payload images, followed by enabling the capsule node to > > > > build the capsules. If there is an easy way of enforcing this > > > > dependency, please let me know. Thanks > > > > > > Can you share your explorations? I think the capsule should be created > > > as part of the build, if enabled. Rather than changing the input > > > files, binman should produce new output files. > > > > This is an issue of handling dependencies in binman, and not changing > > input files. We do not have support for telling binman "build/generate > > this particular image first before you proceed to build the capsules > > using the earlier built images". I am not sure if this can be done in > > a generic manner in binman, so that irrespective of the image being > > generated, it can be specified to build capsules once the capsule > > input images have been generated. > > I'm just not sure what you are getting out here. > > See INPUTS-y for the input files to binman. Then binman uses these to > generate output files. It does not mess with the input files, nor > should it. Please read the top part of the Binman motivation to > understand how all this works. > > At the risk of repeating myself, we want the Makefile to focus on > building U-Boot, with Binman handling the laterprocessing of those > files. Binman may run as part of the U-Boot build, and/or can be run > later, with the input files. > > > > > > > > > We are trying to remove most of the output logic in Makefile. It > > > should just be producing input files for binman. > > > > I understand. However, like I mentioned above, as of now, we don't > > have a way of handling dependencies in binman, at least in a generic > > manner. Once this support gets added, I know that it would be trivial > > to add support for building capsules in binman. > > What dependencies do you need? Please describe it in a simple manner > so I can understand. It cannot involve change the binman input files. Consider the following scenarios. One board, say Board A uses fip.bin as the input file(payload) for generating the capsule file. The fip.bin is being generated through binman. A binman entry is also added for generating the capsule(say fip.capule). Now, binman has to generate fip.bin *and subsequently* fip.capsule, as the capsule fi
Re: [PATCH] board: freescale: imx93_evk: Fix MMC environment offset boot conflict.
From: Peng Fan Sent: Thursday, April 20, 2023 9:46 AM To: Ken Sloat ; Peng Fan ; u-boot@lists.denx.de Cc: Nate Drude Subject: Re: [PATCH] board: freescale: imx93_evk: Fix MMC environment offset boot conflict. Hi Stefano, On 4/12/2023 4:40 AM, Ken Sloat wrote: > From 734bfa4e4556976fe85c741f13ae94161cae97e6 Mon Sep 17 00:00:00 2001 > From: Ken Sloat > Date: Tue, 11 Apr 2023 16:05:31 -0400 > > Currently, the imx93_evk is configured with CONFIG_ENV_IS_IN_MMC and the > chosen environment offset in the config is 0x40. Unless the user > programs the associated fuses, this offset is the default secondary boot > image offset used by the i.MX 93 ROM bootloader. With certain > combinations of environmental variables, the CRC and beginning of the > environment can potentially falsely appear as a valid boot image > container header. If the expected "sw_version" offset within this > mistaken boot image container is greater than the primary's, the ROM > bootloader can skip booting of the primary image altogether and attempt > to boot with the content of the environment data. This will then hang > the system. > > To fix this, move the environment from 0x40 to 0x70 reserving up > to 3 MB at 0x40 for any actual secondary user image container. > > Signed-off-by: Ken Sloat Reviewed-by: Peng Fan > --- > configs/imx93_11x11_evk_defconfig | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/configs/imx93_11x11_evk_defconfig > b/configs/imx93_11x11_evk_defconfig > index 4f8777161e..db94bbfdd1 100644 > --- a/configs/imx93_11x11_evk_defconfig > +++ b/configs/imx93_11x11_evk_defconfig > @@ -7,7 +7,7 @@ CONFIG_SPL_LIBCOMMON_SUPPORT=y > CONFIG_SPL_LIBGENERIC_SUPPORT=y > CONFIG_NR_DRAM_BANKS=2 > CONFIG_ENV_SIZE=0x4000 > -CONFIG_ENV_OFFSET=0x40 > +CONFIG_ENV_OFFSET=0x70 > CONFIG_DM_GPIO=y > CONFIG_DEFAULT_DEVICE_TREE="imx93-11x11-evk" > CONFIG_SPL_TEXT_BASE=0x2049A000 Looks like this may have been forgotten about? Just checking... Thanks Sincerely, Ken Sloat
Re: [PATCH] board: freescale: imx93_evk: Fix MMC environment offset boot conflict.
On Tue, Apr 11, 2023 at 8:55 PM Ken Sloat wrote: > > From 734bfa4e4556976fe85c741f13ae94161cae97e6 Mon Sep 17 00:00:00 2001 > From: Ken Sloat > Date: Tue, 11 Apr 2023 16:05:31 -0400 > > Currently, the imx93_evk is configured with CONFIG_ENV_IS_IN_MMC and the > chosen environment offset in the config is 0x40. Unless the user > programs the associated fuses, this offset is the default secondary boot > image offset used by the i.MX 93 ROM bootloader. With certain > combinations of environmental variables, the CRC and beginning of the > environment can potentially falsely appear as a valid boot image > container header. If the expected "sw_version" offset within this > mistaken boot image container is greater than the primary's, the ROM > bootloader can skip booting of the primary image altogether and attempt > to boot with the content of the environment data. This will then hang > the system. > > To fix this, move the environment from 0x40 to 0x70 reserving up > to 3 MB at 0x40 for any actual secondary user image container. > > Signed-off-by: Ken Sloat Reviewed-by: Fabio Estevam
[PATCH] smegw01: Fix duplicate bootcmd
From: Eduard Strehlau
Two conflicting bootcmds were included in the environment.
Streamline to defining the bootcmd only in the env file.
Signed-off-by: Eduard Strehlau
Signed-off-by: Fabio Estevam
---
Tom,
Stefano is out this week. Could you please apply this one
directly so that it gets included in 2023.07?
Thanks
board/storopack/smegw01/smegw01.env | 11 ++-
configs/smegw01_defconfig | 2 --
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/board/storopack/smegw01/smegw01.env
b/board/storopack/smegw01/smegw01.env
index 25bc7cdbd28b..1263ddac8ed0 100644
--- a/board/storopack/smegw01/smegw01.env
+++ b/board/storopack/smegw01/smegw01.env
@@ -29,7 +29,16 @@ altbootcmd=
run bootcmd;
boot_emmc=setenv mmcdev_wanted 1; run persist_mmcdev; run bootcmd;
boot_sd=setenv mmcdev_wanted 0; run persist_mmcdev; run bootcmd;
-bootcmd=run finduuid; run distro_bootcmd
+bootcmd=
+ if test "${bootcount}" -gt "${bootlimit}"; then
+ run altbootcmd;
+ else
+ if test "${ustate}" = 1; then
+ setenv upgrade_available 1;
+ saveenv;
+ fi;
+ run mmcboot;
+ fi;
bootdelay=2
bootlimit=3
bootm_size=0x1000
diff --git a/configs/smegw01_defconfig b/configs/smegw01_defconfig
index 3d43c9b996e0..7f1b2bee8e16 100644
--- a/configs/smegw01_defconfig
+++ b/configs/smegw01_defconfig
@@ -21,8 +21,6 @@ CONFIG_FIT_VERBOSE=y
# CONFIG_BOOTSTD is not set
CONFIG_AUTOBOOT_MENU_SHOW=y
CONFIG_BOOTMENU_DISABLE_UBOOT_CONSOLE=y
-CONFIG_USE_BOOTCOMMAND=y
-CONFIG_BOOTCOMMAND="if test \"${bootcount}\" -gt \"${bootlimit}\"; then run
altbootcmd; else if test \"${ustate}\" = 1; then setenv upgrade_available 1;
saveenv; fi; run mmcboot; fi;"
CONFIG_USE_PREBOOT=y
CONFIG_PREBOOT="run setup_boot_menu;"
CONFIG_HUSH_PARSER=y
--
2.34.1
Re: [PATCH] smegw01: Fix duplicate bootcmd
On Mon, Jun 26, 2023 at 11:31:36AM -0300, Fabio Estevam wrote: > From: Eduard Strehlau > > Two conflicting bootcmds were included in the environment. > Streamline to defining the bootcmd only in the env file. > > Signed-off-by: Eduard Strehlau > Signed-off-by: Fabio Estevam Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
[ANN] U-Boot v2023.07-rc4 released
Hey all, I've put out v2023.07-rc5 today, and it's a very small delta between this and -rc4. I'm hoping things stay very quiet between now and the release next week. In terms of a changelog, git log --merges v2023.07-rc4..v2023.07-rc5 contains what I've pulled but as always, better PR messages and tags will provide better results here. I plan to do the final release on July 3rd, 2023. Thanks all! -- Tom signature.asc Description: PGP signature
U-Boot v2023.07-rc5 released
On Mon, Jun 26, 2023 at 11:45:18AM -0400, Tom Rini wrote: > Hey all, > > I've put out v2023.07-rc5 today, and it's a very small delta between > this and -rc4. I'm hoping things stay very quiet between now and the > release next week. > > In terms of a changelog, > git log --merges v2023.07-rc4..v2023.07-rc5 > contains what I've pulled but as always, better PR messages and tags > will provide better results here. > > I plan to do the final release on July 3rd, 2023. Thanks all! I got the tag right at least, just the subject wrong, oops. -- Tom signature.asc Description: PGP signature
Re: [PATCH] riscv: Fix alignment of RELA sections in the linker scripts
On Wed, Jun 21, 2023 at 11:07 PM Bin Meng wrote:
>
> In current linker script both .efi_runtime_rel and .rela.dyn sections
> are of RELA type whose entry size is either 12 (RV32) or 24 (RV64).
> These two are arranged as an continuous region on purpose so that the
a continuous
> prelink-riscv executable can fix up the PIE addresses in one loop.
>
> However there is an 'ALIGN(8)' between these 2 sections which might
> cause a gap to be inserted between thesse 2 sections to satify the
typo: these, satisfy
> alignment requirement on RV32. This would break the assumption of
> the prelink process and generate an unbootable image.
>
> Fixes: 9a6569a043d3 ("riscv: Update alignment for some sections in linker
> scripts")
> Signed-off-by: Bin Meng
>
> ---
> This fix should go into the v2023.07 release.
>
Rick, ping?
Re: [PATCH v3] dt-bindings: riscv: deprecate riscv,isa
On Mon, 26 Jun 2023 11:10:46 +0100, Conor Dooley wrote: > intro > = > > When the RISC-V dt-bindings were accepted upstream in Linux, the base > ISA etc had yet to be ratified. By the ratification of the base ISA, > incompatible changes had snuck into the specifications - for example the > Zicsr and Zifencei extensions were spun out of the base ISA. > > Fast forward to today, and the reason for this patch. > Currently the riscv,isa dt property permits only a specific subset of > the ISA string - in particular it excludes version numbering. > With the current constraints, it is not possible to discern whether > "rv64i" means that the hart supports the fence.i instruction, for > example. > Future systems may choose to implement their own instruction fencing, > perhaps using a vendor extension, or they may not implement the optional > counter extensions. Software needs a way to determine this. > > versioning schemes > == > > "Use the extension versions that are described in the ISA manual" you > may say, and it's not like this has not been considered. > Firstly, software that parses the riscv,isa property at runtime will > need to contain a lookup table of some sort that maps arbitrary versions > to versions it understands. There is not a consistent application of > version number applied to extensions, with a higgledy-piggledy > collection of tags, "bare" and versioned documents awaiting the reader > on the "recently ratified extensions" page: > https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions > > As an aside, and this is reflected in the patch too, since many > extensions have yet to appear in a release of the ISA specs, > they are defined by commits in their respective "working draft" > repositories. > > Secondly, there is an issue of backwards compatibility, whereby allowing > numbers in the ISA string, some parsers may be broken. This would > require an additional property to be created to even use the versions in > this manner. > > ~boolean properties~ string array property > == > > If a new property is needed, the whole approach may as well be looked at > from the bottom up. A string with limited character choices etc is > hardly the best approach for communicating extension information to > software. > > Switching to using properties that are defined on a per extension basis, > allows us to define explicit meanings for the DT representation of each > extension - rather than the current situation where different operating > systems or other bits of software may impart different meanings to > characters in the string. > Clearly the best source of meanings is the specifications themselves, > this just provides us the ability to choose at what point in time the > meaning is set. If an extension changes incompatibility in the future, > a new property will be required. > > Off-list, some of the RVI folks have committed to shoring up the wording > in either the ISA specifications, the riscv-isa-manual or > so that in the future, modifications to and additions or removals of > features will require a new extension. Codifying that assertion > somewhere would make it quite unlikely that compatibility would be > broken, but we have the tools required to deal with it, if & when it > crops up. > It is in our collective interest, as consumers of extension meanings, to > define a scheme that enforces compatibility. > > The use of individual properties, rather than elements in a single > string, will also permit validation that the properties have a meaning, > as well as potentially reject mutually exclusive combinations, or > enforce dependencies between extensions. That would not have be possible > with the current dt-schema infrastructure for arbitrary strings, as we > would need to add a riscv,isa parser to dt-validate! > That's not implemented in this patch, but rather left as future work (for > the brave, or the foolish). > > acpi > > > The current ACPI ECR is based on having a single ISA string unfortunately, > but ideally ACPI will move to another method, perhaps GUIDs, that give > explicit meaning to extensions. > > parser simplicity > = > > Many systems that parse DT at runtime already implement an function that > can check for the presence of a string in an array of string, as it is > similar to the process for parsing a list of compatible strings, so a > bunch of new, custom, DT parsing should not be needed. > Getting rid of "riscv,isa" parsing would be a nice simplification, but > unfortunately for backwards compatibility with old dtbs, existing > parsers may not be removable - which may greatly simplify > dt parsing code. In Linux, for example, checking for whether a hart > supports an extension becomes as simple as: > of_property_match_string(node, "riscv,isa-extensions", "zicbom") > > vendor extensions > = > > Compared to riscv,isa, this propo
Re: [PATCH v3] dt-bindings: riscv: deprecate riscv,isa
On 26 Jun 2023, at 11:10, Conor Dooley wrote: > > intro > = > > When the RISC-V dt-bindings were accepted upstream in Linux, the base > ISA etc had yet to be ratified. By the ratification of the base ISA, > incompatible changes had snuck into the specifications - for example the > Zicsr and Zifencei extensions were spun out of the base ISA. > > Fast forward to today, and the reason for this patch. > Currently the riscv,isa dt property permits only a specific subset of > the ISA string - in particular it excludes version numbering. > With the current constraints, it is not possible to discern whether > "rv64i" means that the hart supports the fence.i instruction, for > example. > Future systems may choose to implement their own instruction fencing, > perhaps using a vendor extension, or they may not implement the optional > counter extensions. Software needs a way to determine this. > > versioning schemes > == > > "Use the extension versions that are described in the ISA manual" you > may say, and it's not like this has not been considered. > Firstly, software that parses the riscv,isa property at runtime will > need to contain a lookup table of some sort that maps arbitrary versions > to versions it understands. There is not a consistent application of > version number applied to extensions, with a higgledy-piggledy > collection of tags, "bare" and versioned documents awaiting the reader > on the "recently ratified extensions" page: > https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions > > As an aside, and this is reflected in the patch too, since many > extensions have yet to appear in a release of the ISA specs, > they are defined by commits in their respective "working draft" > repositories. > > Secondly, there is an issue of backwards compatibility, whereby allowing > numbers in the ISA string, some parsers may be broken. This would > require an additional property to be created to even use the versions in > this manner. > > ~boolean properties~ string array property > == > > If a new property is needed, the whole approach may as well be looked at > from the bottom up. A string with limited character choices etc is > hardly the best approach for communicating extension information to > software. > > Switching to using properties that are defined on a per extension basis, > allows us to define explicit meanings for the DT representation of each > extension - rather than the current situation where different operating > systems or other bits of software may impart different meanings to > characters in the string. > Clearly the best source of meanings is the specifications themselves, > this just provides us the ability to choose at what point in time the > meaning is set. If an extension changes incompatibility in the future, > a new property will be required. > > Off-list, some of the RVI folks have committed to shoring up the wording > in either the ISA specifications, the riscv-isa-manual or > so that in the future, modifications to and additions or removals of > features will require a new extension. Codifying that assertion > somewhere would make it quite unlikely that compatibility would be > broken, but we have the tools required to deal with it, if & when it > crops up. > It is in our collective interest, as consumers of extension meanings, to > define a scheme that enforces compatibility. > > The use of individual properties, rather than elements in a single > string, will also permit validation that the properties have a meaning, > as well as potentially reject mutually exclusive combinations, or > enforce dependencies between extensions. That would not have be possible > with the current dt-schema infrastructure for arbitrary strings, as we > would need to add a riscv,isa parser to dt-validate! > That's not implemented in this patch, but rather left as future work (for > the brave, or the foolish). > > acpi > > > The current ACPI ECR is based on having a single ISA string unfortunately, > but ideally ACPI will move to another method, perhaps GUIDs, that give > explicit meaning to extensions. Who’s assigning GUIDs to extensions in that world? And I really don’t want to see DT and ACPI diverge in basics like _describing what ISA your processor has_. As a non-Linux OS developer I am unhappy with this churn. Jess > parser simplicity > = > > Many systems that parse DT at runtime already implement an function that > can check for the presence of a string in an array of string, as it is > similar to the process for parsing a list of compatible strings, so a > bunch of new, custom, DT parsing should not be needed. > Getting rid of "riscv,isa" parsing would be a nice simplification, but > unfortunately for backwards compatibility with old dtbs, existing > parsers may not be removable - which may greatly simplify > dt parsing code. In Linux, for example, checking for whether a hart > support
Re: [PATCH v3] dt-bindings: riscv: deprecate riscv,isa
On Mon, Jun 26, 2023 at 3:42 PM Conor Dooley wrote: > > intro > = > > When the RISC-V dt-bindings were accepted upstream in Linux, the base > ISA etc had yet to be ratified. By the ratification of the base ISA, > incompatible changes had snuck into the specifications - for example the > Zicsr and Zifencei extensions were spun out of the base ISA. > > Fast forward to today, and the reason for this patch. > Currently the riscv,isa dt property permits only a specific subset of > the ISA string - in particular it excludes version numbering. > With the current constraints, it is not possible to discern whether > "rv64i" means that the hart supports the fence.i instruction, for > example. > Future systems may choose to implement their own instruction fencing, > perhaps using a vendor extension, or they may not implement the optional > counter extensions. Software needs a way to determine this. > > versioning schemes > == > > "Use the extension versions that are described in the ISA manual" you > may say, and it's not like this has not been considered. > Firstly, software that parses the riscv,isa property at runtime will > need to contain a lookup table of some sort that maps arbitrary versions > to versions it understands. There is not a consistent application of > version number applied to extensions, with a higgledy-piggledy > collection of tags, "bare" and versioned documents awaiting the reader > on the "recently ratified extensions" page: > https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions > > As an aside, and this is reflected in the patch too, since many > extensions have yet to appear in a release of the ISA specs, > they are defined by commits in their respective "working draft" > repositories. > > Secondly, there is an issue of backwards compatibility, whereby allowing > numbers in the ISA string, some parsers may be broken. This would > require an additional property to be created to even use the versions in > this manner. > > ~boolean properties~ string array property > == > > If a new property is needed, the whole approach may as well be looked at > from the bottom up. A string with limited character choices etc is > hardly the best approach for communicating extension information to > software. > > Switching to using properties that are defined on a per extension basis, > allows us to define explicit meanings for the DT representation of each > extension - rather than the current situation where different operating > systems or other bits of software may impart different meanings to > characters in the string. > Clearly the best source of meanings is the specifications themselves, > this just provides us the ability to choose at what point in time the > meaning is set. If an extension changes incompatibility in the future, > a new property will be required. > > Off-list, some of the RVI folks have committed to shoring up the wording > in either the ISA specifications, the riscv-isa-manual or > so that in the future, modifications to and additions or removals of > features will require a new extension. Codifying that assertion > somewhere would make it quite unlikely that compatibility would be > broken, but we have the tools required to deal with it, if & when it > crops up. > It is in our collective interest, as consumers of extension meanings, to > define a scheme that enforces compatibility. > > The use of individual properties, rather than elements in a single > string, will also permit validation that the properties have a meaning, > as well as potentially reject mutually exclusive combinations, or > enforce dependencies between extensions. That would not have be possible > with the current dt-schema infrastructure for arbitrary strings, as we > would need to add a riscv,isa parser to dt-validate! > That's not implemented in this patch, but rather left as future work (for > the brave, or the foolish). > > acpi > > > The current ACPI ECR is based on having a single ISA string unfortunately, > but ideally ACPI will move to another method, perhaps GUIDs, that give > explicit meaning to extensions. Drop this paragraph on ACPI. We clearly mentioned previously that ACPI will follow specs defined by RVI. There are scalability issues in using GUIDs for each ISA extension. Regards, Anup > > parser simplicity > = > > Many systems that parse DT at runtime already implement an function that > can check for the presence of a string in an array of string, as it is > similar to the process for parsing a list of compatible strings, so a > bunch of new, custom, DT parsing should not be needed. > Getting rid of "riscv,isa" parsing would be a nice simplification, but > unfortunately for backwards compatibility with old dtbs, existing > parsers may not be removable - which may greatly simplify > dt parsing code. In Linux, for example, checking for whether a hart > supports an exten
Re: [PATCH v3] dt-bindings: riscv: deprecate riscv,isa
On Mon, Jun 26, 2023 at 11:08:43PM +0530, Anup Patel wrote: > On Mon, Jun 26, 2023 at 3:42 PM Conor Dooley > wrote: > > acpi > > > > > > The current ACPI ECR is based on having a single ISA string unfortunately, > > but ideally ACPI will move to another method, perhaps GUIDs, that give > > explicit meaning to extensions. > > Drop this paragraph on ACPI. Sure. signature.asc Description: PGP signature
R: [PATCH v2 1/1] arm64: dts: rockchip: rk3308: Add Radxa ROCK Pi S support
Hi,
I've tried to build u-boot for Rock Pi S, but without success. With master and
2023.04, too.
Could you clarify which is the support provided with this commit?
I had a look at doc/board/rockchip/rockchip.rst where Rock Pi S is not listed.
Anyway, looking at what is described in doc/README.rockchip for roc-cc-rk3308
board, it seems U-Boot source code is not enough to build an SPL as complete
idbloader for RK3308, but instead it needs an external TPL: isn't it? Therefore
I would have expected a CONFIG_ROCKCHIP_EXTERNAL_TPL in the defconfig. Which
one of the ddr binaries provided in the rkbin Rockchip repository need to be
used as TPL?
And which binary to be used as BL31? As far as I know TF-A code is not
available as open source for RK3308.
Is there any difference (in TPL and BL31 binaries selection) if Rock Pi S is
equipped with RK3308B or RK3308B-S chip?
A few lines in the documentation would really help. Thanks in advance.
Best regards,
Massimo
> -Messaggio originale-
> Da: U-Boot Per conto di Kever Yang
> Inviato: giovedì 16 febbraio 2023 09:41
> A: Akash Gajjar ; na...@radxa.com
> Cc: Simon Glass ; Philipp Tomsich
> ; Andy Yan ; u-
> b...@lists.denx.de
> Oggetto: Re: [PATCH v2 1/1] arm64: dts: rockchip: rk3308: Add Radxa ROCK Pi
> S support
>
>
> On 2023/2/14 23:31, Akash Gajjar wrote:
> > Add Radxa ROCK 3 Model A support. sync rk3308-rock-pi-s.dts from Linux
> > 6.2.0-rc7.
> >
> > ROCK Pi S is RK3308 based SBC from radxa.com. ROCK Pi S has a,
> > - 256MB/512MB DDR3 RAM
> > - SD, NAND flash (optional on board 1/2/4/8Gb)
> > - 100MB ethernet, PoE (optional)
> > - Onboard 802.11 b/g/n wifi + Bluetooth 4.0 Module
> > - USB2.0 Type-A HOST x1
> > - USB3.0 Type-C OTG x1
> > - 26-pin expansion header
> > - USB Type-C DC 5V Power Supply
> >
> > Linux commit commit for the same,
> > <2e04c25b1320> ("arm64: dts: rockchip: add ROCK Pi S DTS support")
> >
> > Signed-off-by: Akash Gajjar
> Reviewed-by: Kever Yang
>
> Thanks,
> - Kever
> > ---
> > Changes in v2:
> > - Add MAINTAINER for the board
> > ---
> > arch/arm/dts/rk3308-rock-pi-s-u-boot.dtsi | 17 ++
> > arch/arm/dts/rk3308-rock-pi-s.dts | 228 ++
> > board/rockchip/evb_rk3308/MAINTAINERS | 7 +
> > configs/rock-pi-s-rk3308_defconfig| 89 +
> > 4 files changed, 341 insertions(+)
> > create mode 100644 arch/arm/dts/rk3308-rock-pi-s-u-boot.dtsi
> > create mode 100644 arch/arm/dts/rk3308-rock-pi-s.dts
> > create mode 100644 configs/rock-pi-s-rk3308_defconfig
> >
> > diff --git a/arch/arm/dts/rk3308-rock-pi-s-u-boot.dtsi
> > b/arch/arm/dts/rk3308-rock-pi-s-u-boot.dtsi
> > new file mode 100644
> > index 00..27735c49dd
> > --- /dev/null
> > +++ b/arch/arm/dts/rk3308-rock-pi-s-u-boot.dtsi
> > @@ -0,0 +1,17 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * (C) Copyright 2018-2019 Rockchip Electronics Co., Ltd */ #include
> > +"rk3308-u-boot.dtsi"
> > +
> > +/ {
> > + chosen {
> > + u-boot,spl-boot-order = "same-as-spl", &emmc;
> > + };
> > +};
> > +
> > +&uart0 {
> > + u-boot,dm-pre-reloc;
> > + clock-frequency = <2400>;
> > + status = "okay";
> > +};
> > diff --git a/arch/arm/dts/rk3308-rock-pi-s.dts
> > b/arch/arm/dts/rk3308-rock-pi-s.dts
> > new file mode 100644
> > index 00..b5a8691b3f
> > --- /dev/null
> > +++ b/arch/arm/dts/rk3308-rock-pi-s.dts
> > @@ -0,0 +1,228 @@
> > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> > +/*
> > + * Copyright (c) 2019 Fuzhou Rockchip Electronics Co., Ltd
> > + * Copyright (C) 2023 Akash Gajjar
> > + * Copyright (c) 2023 Jagan Teki */
> > +
> > +/dts-v1/;
> > +#include
> > +#include "rk3308.dtsi"
> > +
> > +/ {
> > + model = "Radxa ROCK Pi S";
> > + compatible = "radxa,rockpis", "rockchip,rk3308";
> > +
> > + aliases {
> > + ethernet0 = &mac;
> > + mmc0 = &emmc;
> > + mmc1 = &sdmmc;
> > + };
> > +
> > + chosen {
> > + stdout-path = "serial0:150n8";
> > + };
> > +
> > + leds {
> > + compatible = "gpio-leds";
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&green_led_gio>, <&heartbeat_led_gpio>;
> > +
> > + green-led {
> > + default-state = "on";
> > + gpios = <&gpio0 RK_PA6 GPIO_ACTIVE_HIGH>;
> > + label = "rockpis:green:power";
> > + linux,default-trigger = "default-on";
> > + };
> > +
> > + blue-led {
> > + default-state = "on";
> > + gpios = <&gpio0 RK_PA5 GPIO_ACTIVE_HIGH>;
> > + label = "rockpis:blue:user";
> > + linux,default-trigger = "heartbeat";
> > + };
> > + };
> > +
> > + sdio_pwrseq: sdio-pwrseq {
> > + compatible = "mmc-pwrseq-simple";
> > + pinctrl-0 = <&wifi_enable_h>;
> > + pinctrl-names = "default";
> > + reset-gpios = <&gpio0 RK_PA2 GPIO_ACTI
[PATCH] arm: imx: imx8m: add optee configuration to ft_system_setup
If optee is detected configure it in the Linux device-tree:
- add /firmware/optee node
- add /reserved-memory nodes for optee_core and optee_shm
Signed-off-by: Tim Harvey
---
arch/arm/mach-imx/imx8m/soc.c | 78 ++-
1 file changed, 77 insertions(+), 1 deletion(-)
diff --git a/arch/arm/mach-imx/imx8m/soc.c b/arch/arm/mach-imx/imx8m/soc.c
index 5ffdcabbb513..a46542bf9e16 100644
--- a/arch/arm/mach-imx/imx8m/soc.c
+++ b/arch/arm/mach-imx/imx8m/soc.c
@@ -1245,6 +1245,82 @@ static int fixup_thermal_trips(void *blob, const char
*name)
return 0;
}
+#define OPTEE_SHM_SIZE 0x0040
+static int ft_add_optee_node(void *fdt, struct bd_info *bd)
+{
+ struct fdt_memory carveout;
+ const char *path, *subpath;
+ phys_addr_t optee_start;
+ size_t optee_size;
+ int offs;
+ int ret;
+
+ /*
+* No TEE space allocated indicating no TEE running, so no
+* need to add optee node in dts
+*/
+ if (!rom_pointer[1])
+ return 0;
+
+ optee_start = (phys_addr_t)rom_pointer[0];
+ optee_size = rom_pointer[1] - OPTEE_SHM_SIZE;
+
+ offs = fdt_increase_size(fdt, 512);
+ if (offs) {
+ printf("No Space for dtb\n");
+ return 1;
+ }
+
+ path = "/firmware";
+ offs = fdt_path_offset(fdt, path);
+ if (offs < 0) {
+ path = "/";
+ offs = fdt_path_offset(fdt, path);
+
+ if (offs < 0) {
+ printf("Could not find root node.\n");
+ return offs;
+ }
+
+ subpath = "firmware";
+ offs = fdt_add_subnode(fdt, offs, subpath);
+ if (offs < 0) {
+ printf("Could not create %s node.\n", subpath);
+ return offs;
+ }
+ }
+
+ subpath = "optee";
+ offs = fdt_add_subnode(fdt, offs, subpath);
+ if (offs < 0) {
+ printf("Could not create %s node.\n", subpath);
+ return offs;
+ }
+
+ fdt_setprop_string(fdt, offs, "compatible", "linaro,optee-tz");
+ fdt_setprop_string(fdt, offs, "method", "smc");
+
+ carveout.start = optee_start,
+ carveout.end = optee_start + optee_size - 1,
+ ret = fdtdec_add_reserved_memory(fdt, "optee_core", &carveout, NULL, 0,
+NULL, FDTDEC_RESERVED_MEMORY_NO_MAP);
+ if (ret < 0) {
+ printf("Could not create optee_core node.\n");
+ return ret;
+ }
+
+ carveout.start = optee_start + optee_size;
+ carveout.end = optee_start + optee_size + OPTEE_SHM_SIZE - 1;
+ ret = fdtdec_add_reserved_memory(fdt, "optee_shm", &carveout, NULL, 0,
+NULL, FDTDEC_RESERVED_MEMORY_NO_MAP);
+ if (ret < 0) {
+ printf("Could not create optee_shm node.\n");
+ return ret;
+ }
+
+ return 0;
+}
+
int ft_system_setup(void *blob, struct bd_info *bd)
{
#ifdef CONFIG_IMX8MQ
@@ -1394,7 +1470,7 @@ usb_modify_speed:
fixup_thermal_trips(blob, "soc-thermal"))
printf("Failed to update soc-thermal trip(s)");
- return 0;
+ return ft_add_optee_node(blob, bd);
}
#endif
--
2.25.1
[PATCH] CI: Azure: Split keymile jobs out
Currently the PowerPC build job in Azure will hit the maximum time limit for a build and stop. Looking at the job, the easiest path to reducing it is to move Keymile vendor boards to their own job and exclude them from the PowerPC one (and while at this, the ls102 job). Signed-off-by: Tom Rini --- Cc: Aleksandar Gerasimovski Cc: Heiko Schocher Cc: Holger Brunck Cc: Niel Fourie Cc: Rainer Boschung --- .azure-pipelines.yml | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/.azure-pipelines.yml b/.azure-pipelines.yml index 3c1846a5bc3d..96b2ab4d75e6 100644 --- a/.azure-pipelines.yml +++ b/.azure-pipelines.yml @@ -492,7 +492,7 @@ stages: nxp_ls101x: BUILDMAN: "freescale&ls101" nxp_ls102x: - BUILDMAN: "freescale&ls102" + BUILDMAN: "freescale&ls102 -x keymile" nxp_ls104x: BUILDMAN: "freescale&ls104" nxp_ls108x: @@ -507,6 +507,8 @@ stages: BUILDMAN: "mx -x mx6,imx8,freescale,technexion,toradex" imx8_imx9: BUILDMAN: "imx8 imx9" +keymile: + BUILDMAN: "keymile" keystone2_keystone3: BUILDMAN: "k2 k3" sandbox_asan: @@ -548,7 +550,7 @@ stages: mips: BUILDMAN: "mips" powerpc: - BUILDMAN: "powerpc" + BUILDMAN: "powerpc -x keymile" siemens: BUILDMAN: "siemens" tegra: -- 2.34.1
[PATCH] rockchip: Restore support for boot scripts in legacy image format
Use of CONFIG_SPL_FIT_SIGNATURE=y cause CONFIG_LEGACY_IMAGE_FORMAT=n as
default, this prevent boot scripts in legacy image format from working
and was an unintended change in the listed fixes commits:
Wrong image format for "source" command
Add CONFIG_LEGACY_IMAGE_FORMAT=y to defconfig for affected boards to
restore support for boot scripts in legacy image format.
Fixes: 3bf8e4080763 ("board: rockchip: add Radxa ROCK5B Rk3588 board")
Fixes: cf777572ca31 ("rockchip: rockpro64: Use SDMA to boost eMMC performance")
Fixes: 6e2b8344d60c ("rockchip: rock-pi-4: Use SDMA to boost eMMC performance")
Fixes: 1bf49d5a4a7c ("rockchip: rk3566-radxa-cm3-io: Update defconfig")
Fixes: 703c170b40f2 ("rockchip: rk3568-evb: Update defconfig")
Fixes: 68000f750acd ("rockchip: rk3568-rock-3a: Update defconfig")
Fixes: 6fb02589a608 ("rockchip: rk3588-evb: Update defconfig")
Signed-off-by: Jonas Karlman
---
configs/evb-rk3568_defconfig | 1 +
configs/evb-rk3588_defconfig | 1 +
configs/radxa-cm3-io-rk3566_defconfig | 1 +
configs/rock-3a-rk3568_defconfig | 1 +
configs/rock-pi-4-rk3399_defconfig| 1 +
configs/rock5b-rk3588_defconfig | 1 +
configs/rockpro64-rk3399_defconfig| 1 +
7 files changed, 7 insertions(+)
diff --git a/configs/evb-rk3568_defconfig b/configs/evb-rk3568_defconfig
index 07819d105441..5f3fab7304c2 100644
--- a/configs/evb-rk3568_defconfig
+++ b/configs/evb-rk3568_defconfig
@@ -22,6 +22,7 @@ CONFIG_FIT=y
CONFIG_FIT_VERBOSE=y
CONFIG_SPL_FIT_SIGNATURE=y
CONFIG_SPL_LOAD_FIT=y
+CONFIG_LEGACY_IMAGE_FORMAT=y
CONFIG_DEFAULT_FDT_FILE="rockchip/rk3568-evb.dtb"
# CONFIG_DISPLAY_CPUINFO is not set
CONFIG_DISPLAY_BOARDINFO_LATE=y
diff --git a/configs/evb-rk3588_defconfig b/configs/evb-rk3588_defconfig
index d5f1c4b9ebc7..f49c2ca686a8 100644
--- a/configs/evb-rk3588_defconfig
+++ b/configs/evb-rk3588_defconfig
@@ -23,6 +23,7 @@ CONFIG_FIT=y
CONFIG_FIT_VERBOSE=y
CONFIG_SPL_FIT_SIGNATURE=y
CONFIG_SPL_LOAD_FIT=y
+CONFIG_LEGACY_IMAGE_FORMAT=y
CONFIG_OF_BOARD_SETUP=y
CONFIG_DEFAULT_FDT_FILE="rockchip/rk3588-evb1-v10.dtb"
# CONFIG_DISPLAY_CPUINFO is not set
diff --git a/configs/radxa-cm3-io-rk3566_defconfig
b/configs/radxa-cm3-io-rk3566_defconfig
index 56802d85cc25..488723dfaa30 100644
--- a/configs/radxa-cm3-io-rk3566_defconfig
+++ b/configs/radxa-cm3-io-rk3566_defconfig
@@ -22,6 +22,7 @@ CONFIG_FIT=y
CONFIG_FIT_VERBOSE=y
CONFIG_SPL_FIT_SIGNATURE=y
CONFIG_SPL_LOAD_FIT=y
+CONFIG_LEGACY_IMAGE_FORMAT=y
CONFIG_DEFAULT_FDT_FILE="rockchip/rk3566-radxa-cm3-io.dtb"
# CONFIG_DISPLAY_CPUINFO is not set
CONFIG_DISPLAY_BOARDINFO_LATE=y
diff --git a/configs/rock-3a-rk3568_defconfig b/configs/rock-3a-rk3568_defconfig
index 616499f2f82b..753d03914d90 100644
--- a/configs/rock-3a-rk3568_defconfig
+++ b/configs/rock-3a-rk3568_defconfig
@@ -27,6 +27,7 @@ CONFIG_FIT=y
CONFIG_FIT_VERBOSE=y
CONFIG_SPL_FIT_SIGNATURE=y
CONFIG_SPL_LOAD_FIT=y
+CONFIG_LEGACY_IMAGE_FORMAT=y
CONFIG_DEFAULT_FDT_FILE="rockchip/rk3568-rock-3a.dtb"
# CONFIG_DISPLAY_CPUINFO is not set
CONFIG_DISPLAY_BOARDINFO_LATE=y
diff --git a/configs/rock-pi-4-rk3399_defconfig
b/configs/rock-pi-4-rk3399_defconfig
index 4b984adc6ef8..466868d80b03 100644
--- a/configs/rock-pi-4-rk3399_defconfig
+++ b/configs/rock-pi-4-rk3399_defconfig
@@ -20,6 +20,7 @@ CONFIG_PCI=y
CONFIG_DEBUG_UART=y
# CONFIG_ANDROID_BOOT_IMAGE is not set
CONFIG_SPL_FIT_SIGNATURE=y
+CONFIG_LEGACY_IMAGE_FORMAT=y
CONFIG_DEFAULT_FDT_FILE="rockchip/rk3399-rock-pi-4a.dtb"
CONFIG_DISPLAY_BOARDINFO_LATE=y
CONFIG_MISC_INIT_R=y
diff --git a/configs/rock5b-rk3588_defconfig b/configs/rock5b-rk3588_defconfig
index c1155c20efa8..17205a56cd99 100644
--- a/configs/rock5b-rk3588_defconfig
+++ b/configs/rock5b-rk3588_defconfig
@@ -29,6 +29,7 @@ CONFIG_FIT=y
CONFIG_FIT_VERBOSE=y
CONFIG_SPL_FIT_SIGNATURE=y
CONFIG_SPL_LOAD_FIT=y
+CONFIG_LEGACY_IMAGE_FORMAT=y
CONFIG_OF_BOARD_SETUP=y
CONFIG_DEFAULT_FDT_FILE="rockchip/rk3588-rock-5b.dtb"
# CONFIG_DISPLAY_CPUINFO is not set
diff --git a/configs/rockpro64-rk3399_defconfig
b/configs/rockpro64-rk3399_defconfig
index f41c03067903..dc4392c7451c 100644
--- a/configs/rockpro64-rk3399_defconfig
+++ b/configs/rockpro64-rk3399_defconfig
@@ -23,6 +23,7 @@ CONFIG_PCI=y
CONFIG_DEBUG_UART=y
CONFIG_LTO=y
CONFIG_SPL_FIT_SIGNATURE=y
+CONFIG_LEGACY_IMAGE_FORMAT=y
CONFIG_BOOTSTAGE=y
CONFIG_BOOTSTAGE_REPORT=y
CONFIG_USE_PREBOOT=y
--
2.41.0
Re: [PATCH] riscv: Fix alignment of RELA sections in the linker scripts
> From: Bin Meng
> Sent: Wednesday, June 21, 2023 11:07 PM
> To: u-boot@lists.denx.de
> Cc: Andrew Scull ; Leo Yu-Chi Liang(梁育齊)
> ; Rick Jian-Zhi Chen(陳建志) ; Simon
> Glass
> Subject: [PATCH] riscv: Fix alignment of RELA sections in the linker scripts
>
> In current linker script both .efi_runtime_rel and .rela.dyn sections are of
> RELA type whose entry size is either 12 (RV32) or 24 (RV64).
> These two are arranged as an continuous region on purpose so that the
> prelink-riscv executable can fix up the PIE addresses in one loop.
>
> However there is an 'ALIGN(8)' between these 2 sections which might cause a
> gap to be inserted between thesse 2 sections to satify the alignment
> requirement on RV32. This would break the assumption of the prelink process
> and generate an unbootable image.
>
> Fixes: 9a6569a043d3 ("riscv: Update alignment for some sections in linker
> scripts")
> Signed-off-by: Bin Meng
>
> ---
> This fix should go into the v2023.07 release.
>
> arch/riscv/cpu/u-boot.lds | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
Reviewed-by: Rick Chen
Hi Leo,
Please help to push this patch ASAP.
Thanks,
Rick
Re: [PATCH] riscv: Fix alignment of RELA sections in the linker scripts
On Tue, Jun 27, 2023 at 8:50 AM Rick Chen wrote:
>
> > From: Bin Meng
> > Sent: Wednesday, June 21, 2023 11:07 PM
> > To: u-boot@lists.denx.de
> > Cc: Andrew Scull ; Leo Yu-Chi Liang(梁育齊)
> > ; Rick Jian-Zhi Chen(陳建志) ;
> > Simon Glass
> > Subject: [PATCH] riscv: Fix alignment of RELA sections in the linker scripts
> >
> > In current linker script both .efi_runtime_rel and .rela.dyn sections are
> > of RELA type whose entry size is either 12 (RV32) or 24 (RV64).
> > These two are arranged as an continuous region on purpose so that the
> > prelink-riscv executable can fix up the PIE addresses in one loop.
> >
> > However there is an 'ALIGN(8)' between these 2 sections which might cause a
> > gap to be inserted between thesse 2 sections to satify the alignment
> > requirement on RV32. This would break the assumption of the prelink process
> > and generate an unbootable image.
> >
> > Fixes: 9a6569a043d3 ("riscv: Update alignment for some sections in linker
> > scripts")
> > Signed-off-by: Bin Meng
> >
> > ---
> > This fix should go into the v2023.07 release.
> >
> > arch/riscv/cpu/u-boot.lds | 4 +---
> > 1 file changed, 1 insertion(+), 3 deletions(-)
>
> Reviewed-by: Rick Chen
>
> Hi Leo,
>
> Please help to push this patch ASAP.
>
Thanks Rick. I will respin a v2 to fix the typos in the commit message.
Regards,
Bin
[PATCH v2] riscv: Fix alignment of RELA sections in the linker scripts
In current linker script both .efi_runtime_rel and .rela.dyn sections
are of RELA type whose entry size is either 12 (RV32) or 24 (RV64).
These two are arranged as a continuous region on purpose so that the
prelink-riscv executable can fix up the PIE addresses in one loop.
However there is an 'ALIGN(8)' between these 2 sections which might
cause a gap to be inserted between these 2 sections to satisfy the
alignment requirement on RV32. This would break the assumption of
the prelink process and generate an unbootable image.
Fixes: 9a6569a043d3 ("riscv: Update alignment for some sections in linker
scripts")
Signed-off-by: Bin Meng
Reviewed-by: Rick Chen
---
Changes in v2:
- fix typos in the commit message
arch/riscv/cpu/u-boot.lds | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/arch/riscv/cpu/u-boot.lds b/arch/riscv/cpu/u-boot.lds
index 15b5cbc585..2ffe6ba3c8 100644
--- a/arch/riscv/cpu/u-boot.lds
+++ b/arch/riscv/cpu/u-boot.lds
@@ -48,7 +48,7 @@ SECTIONS
KEEP(*(SORT(__u_boot_list*)));
}
- . = ALIGN(4);
+ . = ALIGN(8);
.efi_runtime_rel : {
__efi_runtime_rel_start = .;
@@ -57,8 +57,6 @@ SECTIONS
__efi_runtime_rel_stop = .;
}
- . = ALIGN(8);
-
/DISCARD/ : { *(.rela.plt*) }
.rela.dyn : {
__rel_dyn_start = .;
--
2.25.1
Re: [PATCH] arm: imx: imx8m: add optee configuration to ft_system_setup
On 6/27/2023 2:25 AM, Tim Harvey wrote: If optee is detected configure it in the Linux device-tree: - add /firmware/optee node - add /reserved-memory nodes for optee_core and optee_shm Signed-off-by: Tim Harvey Reviewed-by: Peng Fan
Re: [PATCH] CI: Azure: Split keymile jobs out
Hello Tom, On 26.06.23 21:19, Tom Rini wrote: > Currently the PowerPC build job in Azure will hit the maximum time limit > for a build and stop. Looking at the job, the easiest path to reducing > it is to move Keymile vendor boards to their own job and exclude them > from the PowerPC one (and while at this, the ls102 job). > > Signed-off-by: Tom Rini > --- > Cc: Aleksandar Gerasimovski > Cc: Heiko Schocher > Cc: Holger Brunck > Cc: Niel Fourie > Cc: Rainer Boschung > --- > .azure-pipelines.yml | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) Thanks! Reviewed-by: Heiko Schocher bye, Heiko -- DENX Software Engineering GmbH, Managing Director: Erika Unter 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 5/7] Makefile: Add a target for building capsules
hi Simon, On Mon, 26 Jun 2023 at 17:43, Sughosh Ganu wrote: > > hi Simon, > > On Mon, 26 Jun 2023 at 14:38, Simon Glass wrote: > > > > Hi Sughosh, > > > > On Wed, 21 Jun 2023 at 05:26, Sughosh Ganu wrote: > > > > > > hi Simon, > > > > > > On Mon, 19 Jun 2023 at 18:07, Simon Glass wrote: > > > > > > > > Hi Sughosh, > > > > > > > > On Thu, 15 Jun 2023 at 17:25, Sughosh Ganu > > > > wrote: > > > > > > > > > > hi Simon, > > > > > > > > > > On Thu, 15 Jun 2023 at 14:44, Simon Glass wrote: > > > > > > > > > > > > Hi Sughosh, > > > > > > > > > > > > On Tue, 13 Jun 2023 at 11:39, Sughosh Ganu > > > > > > wrote: > > > > > > > > > > > > > > Add a target for building EFI capsules. The capsule parameters are > > > > > > > specified through a config file, and the path to the config file > > > > > > > is > > > > > > > specified through CONFIG_EFI_CAPSULE_CFG_FILE. When the config > > > > > > > file is > > > > > > > not specified, the command only builds tools. > > > > > > > > > > > > > > Signed-off-by: Sughosh Ganu > > > > > > > --- > > > > > > > Makefile | 9 + > > > > > > > 1 file changed, 9 insertions(+) > > > > > > > > > > > > > > diff --git a/Makefile b/Makefile > > > > > > > index 10bfaa52ad..96db29aa77 100644 > > > > > > > --- a/Makefile > > > > > > > +++ b/Makefile > > > > > > > @@ -1151,6 +1151,15 @@ dtbs: dts/dt.dtb > > > > > > > dts/dt.dtb: u-boot > > > > > > > $(Q)$(MAKE) $(build)=dts dtbs > > > > > > > > > > > > > > +quiet_cmd_mkeficapsule = MKEFICAPSULE $@ > > > > > > > +cmd_mkeficapsule = $(objtree)/tools/mkeficapsule $@ > > > > > > > + > > > > > > > +PHONY += capsule > > > > > > > +capsule: tools > > > > > > > +ifneq ($(CONFIG_EFI_CAPSULE_CFG_FILE),"") > > > > > > > + $(call cmd,mkeficapsule) > > > > > > > +endif > > > > > > > + > > > > > > > quiet_cmd_copy = COPY$@ > > > > > > >cmd_copy = cp $< $@ > > > > > > > > > > > > > > -- > > > > > > > 2.34.1 > > > > > > > > > > > > > > > > > > > We should be using binman to build images...you seem to be building > > > > > > something in parallel with that. Can you please take a look at > > > > > > binman? > > > > > > > > > > Again, I had explored using binman for this task. The one issue where > > > > > I find the above flow better is that I can simply build my payload > > > > > image(s) followed by 'make capsule' to generate the capsules for > > > > > earlier generated images. In it's current form, I don't see an easy > > > > > way to enforce this dependency in binman when I want to build the > > > > > payload followed by generation of capsules. I did see the mention of > > > > > encapsulating an entry within another dependent entry, but I think > > > > > that makes the implementation more complex than it ought to be. > > > > > > > > > > I think it is much easier to use the make flow to generate the images > > > > > followed by capsules, instead of tweaking the binman node to first > > > > > generate the payload images, followed by enabling the capsule node to > > > > > build the capsules. If there is an easy way of enforcing this > > > > > dependency, please let me know. Thanks > > > > > > > > Can you share your explorations? I think the capsule should be created > > > > as part of the build, if enabled. Rather than changing the input > > > > files, binman should produce new output files. > > > > > > This is an issue of handling dependencies in binman, and not changing > > > input files. We do not have support for telling binman "build/generate > > > this particular image first before you proceed to build the capsules > > > using the earlier built images". I am not sure if this can be done in > > > a generic manner in binman, so that irrespective of the image being > > > generated, it can be specified to build capsules once the capsule > > > input images have been generated. > > > > I'm just not sure what you are getting out here. > > > > See INPUTS-y for the input files to binman. Then binman uses these to > > generate output files. It does not mess with the input files, nor > > should it. Please read the top part of the Binman motivation to > > understand how all this works. > > > > At the risk of repeating myself, we want the Makefile to focus on > > building U-Boot, with Binman handling the laterprocessing of those > > files. Binman may run as part of the U-Boot build, and/or can be run > > later, with the input files. > > > > > > > > > > > > > We are trying to remove most of the output logic in Makefile. It > > > > should just be producing input files for binman. > > > > > > I understand. However, like I mentioned above, as of now, we don't > > > have a way of handling dependencies in binman, at least in a generic > > > manner. Once this support gets added, I know that it would be trivial > > > to add support for building capsules in binman. > > > > What dependencies do you need? Please describe it in a simple manner > > so I can understand. It cannot involve change the binman input files. > > Consider the
Re: U-Boot OMAP GPMC ECC change
Hi Michael, On Mon, Jun 26, 2023 at 07:03:19AM +0200, Michael Nazzareno Trimarchi wrote: > Hi all > > Il sab 20 mag 2023, 19:28 Colin Foster ha > scritto: > > > On Fri, May 19, 2023 at 03:41:34PM +0300, Roger Quadros wrote: > > > Hi Colin, > > > > > > On 19/05/2023 02:19, Colin Foster wrote: > > > > Hi Roger, > > > > > > > >> Can you please share your spl/u-boot.cfg? > > > > > > > > Attached > > > > > > Couple of questions there > > > > > > 1) CONFIG_MTDPARTS_DEFAULT > > "mtdparts=nandflash:0x2(xload_raw),0x18(u-boot),0x18(u-boot-2),0x1fce(main)" > > > Is this correct and matches with what kernel sees? > > > I couldn't see the NAND partition table in the Kernel Device tree patch. > > > > Yes, this is correct. I intentionally left my MTD Partitions out of the > > kernel patch, since I don't want any changes I might make to the flash > > partitions to require further patches. I'm currently at this structure > > (SPL, 2x U-Boot, and main UBI with A/B partitions and 2x U-Boot Envs) > > > > The SD Boot version of U-Boot doesn't use NAND, so it might have a stale > > partition layout that I'll need to remove / modify. > > > > > Was any end up here? I got pulled away from this before I could figure it out, but I have an idea. I suspect the ECC calculation is working as expected. It is just that now it is pre-calculating the ECC for regions that my flash writing was simply ignroing (just leaving erased). Previously the ECC would only be calculated for pages that were correctly written, so wouldn't cause any issues. I do need to verify this hypothesis, but admittedly don't know how soon that'll be. (I won't be near hardware for at least a week) > > Michael
Re: CFP open for RISC-V MC at Linux Plumbers Conference 2023
On Mon, Jun 26, 2023 at 9:08 PM Drew Fustini wrote: > > On Mon, Jun 19, 2023 at 12:55:39PM -0700, Atish Patra wrote: > > The CFP for topic proposals for the RISC-V micro conference[1] 2023 is open > > now. > > Please submit your proposal before it's too late! > > Thanks for posting. I am looking forward to it! > > > The Linux plumbers event will be both in person and remote > > (hybrid)virtual this year. More details can be found here [2]. > > > > We will start accepting proposals after 15th August 2023 to avoid last > > minute visa/travel hassles. > > The conference registration sells out pretty quickly too. Thus, we > > would prefer to sort out the schedule sooner than later. > > > > The final deadline is 15th October 2023 though. > > > > [1] https://lpc.events/event/16/contributions/1148/ > > I think that page is from 2022 and that this would be a better link: Copy paste error :( > https://lpc.events/blog/current/index.php/2023/06/23/risc-v-microconference-cfp/ > Thanks. > Drew -- Regards, Atish
Re: [PATCH v3] dt-bindings: riscv: deprecate riscv,isa
On Mon, Jun 26, 2023 at 5:40 PM Stefan O'Rear wrote: > > On Mon, Jun 26, 2023, at 6:10 AM, Conor Dooley wrote: > > intro > > = > > > > When the RISC-V dt-bindings were accepted upstream in Linux, the base > > ISA etc had yet to be ratified. By the ratification of the base ISA, > > incompatible changes had snuck into the specifications - for example the > > Zicsr and Zifencei extensions were spun out of the base ISA. > > > > Fast forward to today, and the reason for this patch. > > Currently the riscv,isa dt property permits only a specific subset of > > the ISA string - in particular it excludes version numbering. > > With the current constraints, it is not possible to discern whether > > "rv64i" means that the hart supports the fence.i instruction, for > > example. > > Future systems may choose to implement their own instruction fencing, > > perhaps using a vendor extension, or they may not implement the optional > > counter extensions. Software needs a way to determine this. > > > > versioning schemes > > == > > > > "Use the extension versions that are described in the ISA manual" you > > may say, and it's not like this has not been considered. > > Firstly, software that parses the riscv,isa property at runtime will > > need to contain a lookup table of some sort that maps arbitrary versions > > to versions it understands. There is not a consistent application of > > version number applied to extensions, with a higgledy-piggledy > > collection of tags, "bare" and versioned documents awaiting the reader > > on the "recently ratified extensions" page: > > https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions > > > > As an aside, and this is reflected in the patch too, since many > > extensions have yet to appear in a release of the ISA specs, > > they are defined by commits in their respective "working draft" > > repositories. > > > > Secondly, there is an issue of backwards compatibility, whereby allowing > > numbers in the ISA string, some parsers may be broken. This would > > require an additional property to be created to even use the versions in > > this manner. > > > > ~boolean properties~ string array property > > == > > > > If a new property is needed, the whole approach may as well be looked at > > from the bottom up. A string with limited character choices etc is > > hardly the best approach for communicating extension information to > > software. > > > > Switching to using properties that are defined on a per extension basis, > > allows us to define explicit meanings for the DT representation of each > > extension - rather than the current situation where different operating > > systems or other bits of software may impart different meanings to > > characters in the string. > > Clearly the best source of meanings is the specifications themselves, > > this just provides us the ability to choose at what point in time the > > meaning is set. If an extension changes incompatibility in the future, > > a new property will be required. > > > > Off-list, some of the RVI folks have committed to shoring up the wording > > in either the ISA specifications, the riscv-isa-manual or > > so that in the future, modifications to and additions or removals of > > features will require a new extension. Codifying that assertion > > somewhere would make it quite unlikely that compatibility would be > > broken, but we have the tools required to deal with it, if & when it > > crops up. > > It is in our collective interest, as consumers of extension meanings, to > > define a scheme that enforces compatibility. > > > > The use of individual properties, rather than elements in a single > > no longer individual properties > > > string, will also permit validation that the properties have a meaning, > > as well as potentially reject mutually exclusive combinations, or > > enforce dependencies between extensions. That would not have be possible > > Under what circumstances is a device tree which declares support for a > superset extension (e.g. m) required to also declare support for its subsets > (e.g. zmmul)? There are compatibility issues in both directions. > > Proposal: If an extension X is a superset of an extension Y and X is present > in riscv,isa-extensions, Y must also be present if Y was ratified or added > to the schema before X, but need not also be present if Y was ratified after > or at the same time as X. If X "depends on" Y, then Y must be present in > riscv,isa-extensions even if X and Y were ratified at the same time. > > > with the current dt-schema infrastructure for arbitrary strings, as we > > would need to add a riscv,isa parser to dt-validate! > > That's not implemented in this patch, but rather left as future work (for > > the brave, or the foolish). > > > > acpi > > > > > > The current ACPI ECR is based on having a single ISA string unfortunately, > > but ideally ACPI will move to another method,
Re: [PATCH v3] dt-bindings: riscv: deprecate riscv,isa
On Tue, Jun 27, 2023 at 1:23 AM Palmer Dabbelt wrote: > > On Mon, 26 Jun 2023 10:38:43 PDT (-0700), apa...@ventanamicro.com wrote: > > On Mon, Jun 26, 2023 at 3:42 PM Conor Dooley > > wrote: > >> > >> intro > >> = > >> > >> When the RISC-V dt-bindings were accepted upstream in Linux, the base > >> ISA etc had yet to be ratified. By the ratification of the base ISA, > >> incompatible changes had snuck into the specifications - for example the > >> Zicsr and Zifencei extensions were spun out of the base ISA. > >> > >> Fast forward to today, and the reason for this patch. > >> Currently the riscv,isa dt property permits only a specific subset of > >> the ISA string - in particular it excludes version numbering. > >> With the current constraints, it is not possible to discern whether > >> "rv64i" means that the hart supports the fence.i instruction, for > >> example. > >> Future systems may choose to implement their own instruction fencing, > >> perhaps using a vendor extension, or they may not implement the optional > >> counter extensions. Software needs a way to determine this. > >> > >> versioning schemes > >> == > >> > >> "Use the extension versions that are described in the ISA manual" you > >> may say, and it's not like this has not been considered. > >> Firstly, software that parses the riscv,isa property at runtime will > >> need to contain a lookup table of some sort that maps arbitrary versions > >> to versions it understands. There is not a consistent application of > >> version number applied to extensions, with a higgledy-piggledy > >> collection of tags, "bare" and versioned documents awaiting the reader > >> on the "recently ratified extensions" page: > >> https://wiki.riscv.org/display/HOME/Recently+Ratified+Extensions > >> > >> As an aside, and this is reflected in the patch too, since many > >> extensions have yet to appear in a release of the ISA specs, > >> they are defined by commits in their respective "working draft" > >> repositories. > >> > >> Secondly, there is an issue of backwards compatibility, whereby allowing > >> numbers in the ISA string, some parsers may be broken. This would > >> require an additional property to be created to even use the versions in > >> this manner. > >> > >> ~boolean properties~ string array property > >> == > >> > >> If a new property is needed, the whole approach may as well be looked at > >> from the bottom up. A string with limited character choices etc is > >> hardly the best approach for communicating extension information to > >> software. > >> > >> Switching to using properties that are defined on a per extension basis, > >> allows us to define explicit meanings for the DT representation of each > >> extension - rather than the current situation where different operating > >> systems or other bits of software may impart different meanings to > >> characters in the string. > >> Clearly the best source of meanings is the specifications themselves, > >> this just provides us the ability to choose at what point in time the > >> meaning is set. If an extension changes incompatibility in the future, > >> a new property will be required. > >> > >> Off-list, some of the RVI folks have committed to shoring up the wording > >> in either the ISA specifications, the riscv-isa-manual or > >> so that in the future, modifications to and additions or removals of > >> features will require a new extension. Codifying that assertion > >> somewhere would make it quite unlikely that compatibility would be > >> broken, but we have the tools required to deal with it, if & when it > >> crops up. > >> It is in our collective interest, as consumers of extension meanings, to > >> define a scheme that enforces compatibility. > >> > >> The use of individual properties, rather than elements in a single > >> string, will also permit validation that the properties have a meaning, > >> as well as potentially reject mutually exclusive combinations, or > >> enforce dependencies between extensions. That would not have be possible > >> with the current dt-schema infrastructure for arbitrary strings, as we > >> would need to add a riscv,isa parser to dt-validate! > >> That's not implemented in this patch, but rather left as future work (for > >> the brave, or the foolish). > >> > >> acpi > >> > >> > >> The current ACPI ECR is based on having a single ISA string unfortunately, > >> but ideally ACPI will move to another method, perhaps GUIDs, that give > >> explicit meaning to extensions. > > > > Drop this paragraph on ACPI. > > > > We clearly mentioned previously that ACPI will follow specs defined by RVI. > > There are scalability issues in using GUIDs for each ISA extension. > > Which spec are we following for the ACPI ISA string? ACPI RHCT follows the "ISA Extension Naming Conventions" defined by the RISC-V unpriv spec. I understand that there are unresolved issues in the "ISA Exten
