Re: [PATCH v3 4/5] eficonfig: use efi_get_next_variable_name_int()
On Tue, 6 Dec 2022 at 23:12, Ilias Apalodimas
wrote:
>
> On Sat, Dec 03, 2022 at 09:56:20AM +0900, Masahisa Kojima wrote:
> > On Fri, 2 Dec 2022 at 16:36, Ilias Apalodimas
> > wrote:
> > >
> > > On Fri, Dec 02, 2022 at 01:59:36PM +0900, Masahisa Kojima wrote:
> > > > eficonfig command reads all possible UEFI load options
> > > > from 0x to 0x to construct the menu. This takes too much
> > > > time in some environment.
> > > > This commit uses efi_get_next_variable_name_int() to read all
> > > > existing UEFI load options to significantlly reduce the count of
> > > > efi_get_var() call.
> > > >
> > > > Signed-off-by: Masahisa Kojima
> > > > ---
> > > > No update since v2
> > > >
> > > > v1->v2:
> > > > - totaly change the implemention, remove new Kconfig introduced in v1.
> > > > - use efi_get_next_variable_name_int() to read the all existing
> > > > UEFI variables, then enumerate the "Boot" variables
> > > > - this commit does not provide the common function to enumerate
> > > > all "Boot" variables. I think common function does not
> > > > simplify the implementation, because caller of
> > > > efi_get_next_variable_name_int()
> > > > needs to remember original buffer size, new buffer size and buffer
> > > > address
> > > > and it is a bit complicated when we consider to create common
> > > > function.
> > > >
> > > > cmd/eficonfig.c | 141 +++-
> > > > 1 file changed, 117 insertions(+), 24 deletions(-)
> > > >
> > > > diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> > > > index 88d507d04c..394ae67cce 100644
> > > > --- a/cmd/eficonfig.c
> > > > +++ b/cmd/eficonfig.c
> > > > @@ -1683,7 +1683,8 @@ static efi_status_t
> > > > eficonfig_show_boot_selection(unsigned int *selected)
> > > > u32 i;
> > > > u16 *bootorder;
> > > > efi_status_t ret;
> > > > - efi_uintn_t num, size;
> > > > + u16 *var_name16 = NULL, *p;
> > > > + efi_uintn_t num, size, buf_size;
> > > > struct efimenu *efi_menu;
> > > > struct list_head *pos, *n;
> > > > struct eficonfig_entry *entry;
> > > > @@ -1707,14 +1708,43 @@ static efi_status_t
> > > > eficonfig_show_boot_selection(unsigned int *selected)
> > > > }
> > > >
> > > > /* list the remaining load option not included in the BootOrder */
> > > > - for (i = 0; i <= 0x; i++) {
> > > > - /* If the index is included in the BootOrder, skip it */
> > > > - if (search_bootorder(bootorder, num, i, NULL))
> > > > - continue;
> > > > + buf_size = 128;
> > > > + var_name16 = malloc(buf_size);
> > > > + if (!var_name16)
> > > > + return EFI_OUT_OF_RESOURCES;
> > > >
> > > > - ret = eficonfig_add_boot_selection_entry(efi_menu, i,
> > > > selected);
> > > > - if (ret != EFI_SUCCESS)
> > > > - goto out;
> > > > + var_name16[0] = 0;
> > >
> > > Is it worth using calloc instead of malloc and get rid of this?
> > >
> > > > + for (;;) {
> > > > + int index;
> > > > + efi_guid_t guid;
> > > > +
> > > > + size = buf_size;
> > > > + ret = efi_get_next_variable_name_int(, var_name16,
> > > > );
> > > > + if (ret == EFI_NOT_FOUND)
> > > > + break;
> > > > + if (ret == EFI_BUFFER_TOO_SMALL) {
> > > > + buf_size = size;
> > > > + p = realloc(var_name16, buf_size);
> > > > + if (!p) {
> > > > + free(var_name16);
> > > > + return EFI_OUT_OF_RESOURCES;
> > > > + }
> > > > + var_name16 = p;
> > > > + ret = efi_get_next_variable_name_int(,
> > > > var_name16, );
> > > > + }
> > > > + if (ret != EFI_SUCCESS) {
> > > > + free(var_name16);
> > > > + return ret;
> > > > + }
> > > > + if (efi_varname_is_load_option(var_name16, )) {
> > > > + /* If the index is included in the BootOrder,
> > > > skip it */
> > > > + if (search_bootorder(bootorder, num, index, NULL))
> > > > + continue;
> > > > +
> > > > + ret =
> > > > eficonfig_add_boot_selection_entry(efi_menu, index, selected);
> > > > + if (ret != EFI_SUCCESS)
> > > > + goto out;
> > > > + }
> > > >
> > > > if (efi_menu->count >= EFICONFIG_ENTRY_NUM_MAX - 1)
> > > > break;
> > > > @@ -1732,6 +1762,8 @@ out:
> > > > }
> > > > eficonfig_destroy(efi_menu);
> > > >
> > > > + free(var_name16);
> > > > +
> > > > return ret;
> > > > }
> > > >
> > > > @@ -1994,6 +2026,8 @@ static efi_status_t
> > > > eficonfig_create_change_boot_order_entry(struct
Re: [PATCH 1/1] mvebu: fix end-of-array check
Hi Pali,
Hi Derek,
On 12/6/22 20:56, Pali Rohár wrote:
On Tuesday 06 December 2022 07:10:15 Stefan Roese wrote:
Hi Pali,
On 12/5/22 19:18, Pali Rohár wrote:
On Monday 05 December 2022 12:42:44 Stefan Roese wrote:
Hi!
On 12/4/22 11:39, Pali Rohár wrote:
Hello!
I would suggest to change description of the patch from
"mvebu: fix end-of-array check"
to something like:
"arm: mvebu: Espressobin: fix end-of-array check in env"
as it affects only Espressobin boards (not other mvebu).
Yes, please update the commit subject here.
Stefan, please look below as this issue/fix is important.
Yes.
On Wednesday 30 November 2022 13:33:40 Derek LaHousse wrote:
Properly seek the end of default_environment variables.
The current algorithm overwrites from the second variable. This
replacement finds the end of the array of strings.
Stomped variables include "board", "soc", "loadaddr". These can be
seen on a "env default -a" after patch, but they are not seen with a
version before the patch.
This is a real issue which I introduced in the past. So it some fix for
this issue should be pulled into the v2023.01 release.
Understood.
Signed-off-by: Derek LaHousse
---
board/Marvell/mvebu_armada-37xx/board.c | 7 +--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/board/Marvell/mvebu_armada-37xx/board.c
b/board/Marvell/mvebu_armada-37xx/board.c
index c6ecc323bb..ac29ac5b95 100644
--- a/board/Marvell/mvebu_armada-37xx/board.c
+++ b/board/Marvell/mvebu_armada-37xx/board.c
@@ -100,8 +100,11 @@ int board_late_init(void)
return 0;
/* Find free buffer in default_environment[] for new variables
*/
- while (*ptr != '\0' && *(ptr+1) != '\0') ptr++;
- ptr += 2;
+ if (*ptr != '\0') { // Defending against empty default env
+ while ((i = strlen(ptr)) != 0) {
+ ptr += i + 1;
+ }
+ }
If I'm looking at the outer-if condition correctly then it is not
needed. strlen("") returns zero and so inner-while loop stops
immediately.
My proposed fix for this issue was just changing correct while loop
check to ensure that ptr is set after the _last_ variable.
- while (*ptr != '\0' && *(ptr+1) != '\0') ptr++;
- ptr += 2;
+ while (*ptr != '\0' || *(ptr+1) != '\0') ptr++;
+ ptr++;
Both changes should be equivalent, but I'm not sure which one is more
readable. The original issue was introduced really by non-readable
code...
Stefan, do you have a preference which one fix is better / more
readable?
I would prefer to get Pali's corrected version included. Could you
please prepare a v2 patch with this update and also with the added
or changed patch subject.
Originally this issue was reported half year ago on the armbian forum:
https://forum.armbian.com/topic/19564-making-espressobin-v7-work-in-2022/?do=findComment=138136
U-Boot "changed" variable name scriptaddr to criptaddr (without leading
s) and broke booting.
Details are later in comments:
https://forum.armbian.com/topic/19564-making-espressobin-v7-work-in-2022/?do=findComment=154062
https://forum.armbian.com/topic/19564-making-espressobin-v7-work-in-2022/?do=findComment=154235
If you prefer my variant, I can prepare a v2 patch.
Yes, please do. That's easiest for me. I'll push this quickly then.
Thanks,
Stefan
Hello Stefan! During discussion on the forum, Derek pointed that my
"fix" does not work for another edge case when *ptr is '\0' before
entering into while-loop. That is a valid point and code needs to be
adjusted.
With Derek we do not have an agreement how to write this peace of the
code in readable way, which should move ptr pointer to the end of env
list - find two nul bytes which indicates end of the env list and then
set ptr to the second nul byte, so at this position can be put another
new variable. Plus handle special case nul byte is at the beginning.
My idea was to use single while loop to find this location to have code
minimal in its size. Derek thinks that code is better readable if
iteration is done via strlen() calls in while-loop, like it is in this
version.
Stefan, do you have an idea how to write this code in a way that is fast
and also readable and maintainable?
Sorry, I don't have an "idea" quickly that might be "better" than your
suggested version(s) without diving into this deeper. You are both
experienced developers I assume. I have no real problem using Derek's
version if this solves the problem and is a bit slower. The code
patch we are talking about is not accessed frequently I assume.
Thanks,
Stefan
Re: [PATCH v5 3/3] board: qemu-riscv: enable semihosting
Hi Kautuk, On Tue, Dec 06, 2022 at 05:02:49PM +0530, Kautuk Consul wrote: > Hi Leo, > > On Tue, Dec 6, 2022 at 4:29 PM Leo Liang wrote: > > > > Hi Kautuk, > > > > We have tested your patchset with QEMU 7.1.0. > > It generally looks fine, but CI error seems to persist. > > https://source.denx.de/u-boot/custodians/u-boot-riscv/-/pipelines/14314 > > > > The error comes from CI testcase timed-out. > > The reason for the time-out is not yet confirmed, > > but we suspect it's because when executing under semihosting, > > QEMU could not exit normally. (thru ctrl+x a) > > > > There is a seemingly relevent patchset that sits on QEMU mailing list for > > some time. > > https://lore.kernel.org/all/20220620190834.ga16...@ws2101.lin.mbt.kalray.eu/T/#m1bc32cc32511b6ac8adfaf67983dc2bccd4b9ec9 > > > > On the u-boot side, what do you think if we disable semihosting by default? > > (i.e., not adding CONFIG_SEMIHOSTING_XXX in qemu's defconfig) > > I think it is okay to disable semihosting by default. Then the user > will configure it when needed. > So then can you ACK the first 2 patches ? I think we can leave out the > 3rd qemu config patch for now. > No problem! Additionally, could you rebase the patchset to current master, add what Sean suggested, and then send again? I think I could merge your patch as soon as you re-send it. Best regards, Leo > > > > Best regards, > > Leo > > > > On Tue, Dec 06, 2022 at 11:12:41AM +0530, Kautuk Consul wrote: > > > Hi, > > > > > > On Mon, Dec 5, 2022 at 8:46 PM Sean Anderson > > > wrote: > > > > > > > > On 12/5/22 00:51, Kautuk Consul wrote: > > > > > Hi, > > > > > > > > > > On Sat, Dec 3, 2022 at 9:44 AM Bin Meng wrote: > > > > >> > > > > >> On Fri, Sep 23, 2022 at 3:03 PM Kautuk Consul > > > > >> wrote: > > > > >> > > > > > >> > To enable semihosting we also need to enable the following > > > > >> > configs in defconfigs: > > > > >> > CONFIG_SEMIHOSTING > > > > >> > CONFIG_SPL_SEMIHOSTING > > > > >> > CONFIG_SEMIHOSTING_SERIAL > > > > >> > CONFIG_SERIAL_PROBE_ALL > > > > >> > CONFIG_SPL_FS_EXT4 > > > > >> > CONFIG_SPL_FS_FAT > > > > >> > > > > >> Why should these _SPL_FS_xxx be required? If it's required by > > > > >> SEMIHOSTING, could the dependency be fixed there? > > > > > > > > > > The build dependencies require that these options be there. > > > > > > > > What error do you get? > > > > > > If I disable both the _SPL_FS_* config options then I get the > > > following compilation error: > > > common/spl/spl_semihosting.c: In function 'spl_smh_load_image': > > > common/spl/spl_semihosting.c:27:32: error: > > > 'CONFIG_SPL_FS_LOAD_PAYLOAD_NAME' undeclared (first use in this > > > function) > > >27 | const char *filename = CONFIG_SPL_FS_LOAD_PAYLOAD_NAME; > > > |^~~ > > > common/spl/spl_semihosting.c:27:32: note: each undeclared identifier > > > is reported only once for each function it appears in > > > > > > Bin/Sean: This error is not really related to the semihosting feature > > > but is related to COFIG_SPL in general. > > > Can you please accept this patch-set and then I'll try and find time > > > in the future maybe to rectify this build dependency > > > problem ? > > > > > > > > > > > --Sean > > > > > > > > >> > > > > >> > > > > > >> > Signed-off-by: Kautuk Consul > > > > >> > --- > > > > >> > configs/qemu-riscv32_defconfig | 4 > > > > >> > configs/qemu-riscv32_smode_defconfig | 4 > > > > >> > configs/qemu-riscv32_spl_defconfig | 7 +++ > > > > >> > configs/qemu-riscv64_defconfig | 4 > > > > >> > configs/qemu-riscv64_smode_defconfig | 4 > > > > >> > configs/qemu-riscv64_spl_defconfig | 7 +++ > > > > >> > 6 files changed, 30 insertions(+) > > > > >> > > > > > >> > > > > >> Regards, > > > > >> Bin > > > >
Re: [PATCH 0/4] Synquacer DT schema fixes
Hi Rob On Tue, 6 Dec 2022 at 18:16, Rob Herring wrote: > > This is a series of DT fixes for the Synquacer. These issues were found > running the dtschema tools. > > I don't have a board, but Ilias has tested the changes for me. Thanks! I did and fwiw for the series Tested-by: Ilias Apalodimas Reviewed-by: Ilias Apalodimas but I'd prefer having someone from Socionext config that. Kojima-san does it look ok to you as well? Regards /Ilias > > Signed-off-by: Rob Herring > > --- > Rob Herring (4): > dts: synquacer: Drop CPU 'arm,armv8' compatibles > dts: synquacer: Use generic node names > dts: synquacer: Fix "arm,armv7-timer-mem" node address sizes > dts: synquacer: Fix idle-states 'entry-method' value > > .../dts/synquacer-sc2a11-developerbox-u-boot.dtsi | 2 +- > arch/arm/dts/synquacer-sc2a11-developerbox.dts | 2 +- > arch/arm/dts/synquacer-sc2a11.dtsi | 70 > +++--- > 3 files changed, 37 insertions(+), 37 deletions(-) > --- > base-commit: bebb393b340295edb9ba50a996fc0510cd1b6ac0 > change-id: 20221206-synquacer-dts-521500f88a1d > > Best regards, > -- > Rob Herring
[PATCH 4/4] doc: ae350: Add Fast Boot description
Descript how to boot Kernel with Fast Boot and record booting messages here. Signed-off-by: Rick Chen --- doc/board/AndesTech/ax25-ae350.rst | 140 + 1 file changed, 140 insertions(+) diff --git a/doc/board/AndesTech/ax25-ae350.rst b/doc/board/AndesTech/ax25-ae350.rst index b46f427f4b..01b7159117 100644 --- a/doc/board/AndesTech/ax25-ae350.rst +++ b/doc/board/AndesTech/ax25-ae350.rst @@ -522,3 +522,143 @@ Messages of U-Boot SPL boots Kernel on AE350 board nfs4flexfilelayout_init: NFSv4 Flexfile Layout Driver Registering... ~ # + + +Fast-Boot on AE350 +-- +In hope of reducing the boot time, Andes U-Boot is implemented with a feature similar to +U-Boot FALCON mode. The boot flow using this feature, called Fast Boot, initializes memory +with the U-Boot SPL at the first stage, just like what a regular booting process +(i.e. Normal Boot) does in the beginning. Instead of jumping to the U-Boot proper (normal mode) +from OpenSBI before booting kernel, the Fast Boot process jumps directly to kernel to enabl +shorter boot time. + + +Fast-Boot flow +-- +U-Boot SPL --> openSBI --> Linux Kernel + +How to run Fast-Boot on AE350 +--- +1. Copy Kernel Image (arch/riscv/boot/Image) into U-Boot root directory. +2. make ae350_rv[32|64]_spl_fastboot_defconfig and build. +3. linux.itb will be generated here. + + +Messages of Fast-Boot Kernel on AE350 board +--- +U-Boot SPL 2023.01-rc1-00100-gf854773d8a-dirty (Dec 05 2022 - 13:54:20 +0800) +Trying to boot from RAM +[0.00] OF: fdt: Ignoring memory range 0x0 - 0x180 +[0.00] Linux version 5.4.192-18651-gf2d0487a5fb8-dirty (rick@atcsqa06) (gcc version 10.3.0 (2022-05-13_nds64le-linux-glibc-v5d-_experimental)) #2 SMP PREEMPT Thu Oct 13 10:39:07 CST 2022 +[0.00] earlycon: sbi0 at I/O port 0x0 (options '') +[0.00] printk: bootconsole [sbi0] enabled +[0.00] initrd not found or empty - disabling initrd +[0.00] Zone ranges: +[0.00] DMA32[mem 0x0180-0x3fff] +[0.00] Normal empty +[0.00] Movable zone start for each node +[0.00] Early memory node ranges +[0.00] node 0: [mem 0x0180-0x3fff] +[0.00] Initmem setup node 0 [mem 0x0180-0x3fff] +[0.00] SBI specification v0.3 detected +[0.00] SBI implementation ID=0x1 Version=0x1 +[0.00] SBI v0.2 TIME extension detected +[0.00] SBI v0.2 IPI extension detected +[0.00] SBI v0.2 RFENCE extension detected +[0.00] SBI SRST extension detected +[0.00] SBI v0.2 HSM extension detected +[0.00] percpu: Embedded 16 pages/cpu s28120 r8192 d29224 u65536 +[0.00] Built 1 zonelists, mobility grouping on. Total pages: 252500 +[0.00] Kernel command line: console=ttyS0,38400n8 debug loglevel=7 earlycon=sbi +[0.00] Dentry cache hash table entries: 131072 (order: 8, 1048576 bytes, linear) +[0.00] Inode-cache hash table entries: 65536 (order: 7, 524288 bytes, linear) +[0.00] Sorting __ex_table... +[0.00] mem auto-init: stack:off, heap alloc:off, heap free:off +[0.00] Memory: 994544K/1024000K available (4565K kernel code, 290K rwdata, 1655K rodata, 7083K init, 189K bss, 29456K reserved, 0K cma-reserved) +[0.00] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=1, Nodes=1 +[0.00] rcu: Preemptible hierarchical RCU implementation. +[0.00] rcu: RCU restricting CPUs from NR_CPUS=8 to nr_cpu_ids=1. +[0.00] Tasks RCU enabled. +[0.00] rcu: RCU calculated value of scheduler-enlistment delay is 10 jiffies. +[0.00] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=1 +[0.00] NR_IRQS: 72, nr_irqs: 72, preallocated irqs: 0 +[0.00] plic: mapped 71 interrupts with 1 handlers for 2 contexts. +[0.00] riscv_timer_init_dt: Registering clocksource cpuid [0] hartid [0] +[0.00] clocksource: riscv_clocksource: mask: 0x max_cycles: 0x1bacf917bf, max_idle_ns: 881590412290 ns +[0.91] sched_clock: 64 bits at 60MHz, resolution 16ns, wraps every 4398046511098ns +[0.025459] Console: colour dummy device 40x30 +[0.039116] Calibrating delay loop (skipped), value calculated using timer frequency.. 120.00 BogoMIPS (lpj=60) +[0.070377] pid_max: default: 32768 minimum: 301 +[0.086700] Mount-cache hash table entries: 2048 (order: 2, 16384 bytes, linear) +[0.109186] Mountpoint-cache hash table entries: 2048 (order: 2, 16384 bytes, linear) +[0.160697] rcu: Hierarchical SRCU implementation. +[0.181706] smp: Bringing up secondary CPUs ... +[0.195674] smp: Brought up 1 node, 1 CPU +[0.213734] devtmpfs: initialized +[0.247406] random: get_random_u32 called from bucket_table_alloc.isra.0+0x74/0xb8
[PATCH 3/4] riscv: ae350: Support Fast Boot
Add defconfig for Fast Boot
Signed-off-by: Rick Chen
---
board/AndesTech/ax25-ae350/ax25-ae350.c | 7 ++-
configs/ae350_rv32_spl_fastboot_defconfig | 53 +++
configs/ae350_rv64_spl_fastboot_defconfig | 53 +++
3 files changed, 111 insertions(+), 2 deletions(-)
create mode 100644 configs/ae350_rv32_spl_fastboot_defconfig
create mode 100644 configs/ae350_rv64_spl_fastboot_defconfig
diff --git a/board/AndesTech/ax25-ae350/ax25-ae350.c
b/board/AndesTech/ax25-ae350/ax25-ae350.c
index 63a966e092..b5b2b93f76 100644
--- a/board/AndesTech/ax25-ae350/ax25-ae350.c
+++ b/board/AndesTech/ax25-ae350/ax25-ae350.c
@@ -134,7 +134,10 @@ void board_boot_order(u32 *spl_boot_list)
#ifdef CONFIG_SPL_LOAD_FIT
int board_fit_config_name_match(const char *name)
{
- /* boot using first FIT config */
- return 0;
+#if IS_ENABLED(CONFIG_SPL_OPENSBI_OS_BOOT)
+ return strcmp("linux", name);
+#endif
+
+ return -EINVAL;
}
#endif
diff --git a/configs/ae350_rv32_spl_fastboot_defconfig
b/configs/ae350_rv32_spl_fastboot_defconfig
new file mode 100644
index 00..fe9959b75e
--- /dev/null
+++ b/configs/ae350_rv32_spl_fastboot_defconfig
@@ -0,0 +1,53 @@
+CONFIG_RISCV=y
+CONFIG_TEXT_BASE=0x0180
+CONFIG_SYS_MALLOC_LEN=0x8
+CONFIG_NR_DRAM_BANKS=2
+CONFIG_ENV_SECT_SIZE=0x1000
+CONFIG_DEFAULT_DEVICE_TREE="ae350_32"
+CONFIG_SYS_PROMPT="RISC-V # "
+CONFIG_SPL_SYS_MALLOC_F_LEN=0x200
+CONFIG_SPL=y
+CONFIG_SPL_OPENSBI_OS_BOOT=y
+CONFIG_SYS_LOAD_ADDR=0x10
+CONFIG_TARGET_AX25_AE350=y
+CONFIG_RISCV_SMODE=y
+# CONFIG_AVAILABLE_HARTS is not set
+CONFIG_DISTRO_DEFAULTS=y
+CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xf00
+CONFIG_SYS_MONITOR_LEN=786432
+CONFIG_FIT=y
+CONFIG_SPL_LOAD_FIT_ADDRESS=0x1000
+CONFIG_SYS_MONITOR_BASE=0x8800
+CONFIG_BOOTDELAY=3
+CONFIG_BOARD_EARLY_INIT_F=y
+CONFIG_SPL_MAX_SIZE=0x10
+CONFIG_SPL_BSS_START_ADDR=0x40
+CONFIG_SYS_PBSIZE=1050
+CONFIG_SYS_BOOTM_LEN=0x400
+CONFIG_CMD_IMLS=y
+CONFIG_CMD_MMC=y
+CONFIG_CMD_SF_TEST=y
+# CONFIG_CMD_SETEXPR is not set
+CONFIG_BOOTP_PREFER_SERVERIP=y
+CONFIG_CMD_CACHE=y
+CONFIG_ENV_OVERWRITE=y
+CONFIG_ENV_IS_IN_SPI_FLASH=y
+CONFIG_BOOTP_SEND_HOSTNAME=y
+CONFIG_NET_RANDOM_ETHADDR=y
+CONFIG_MMC=y
+CONFIG_FTSDC010=y
+CONFIG_FTSDC010_SDIO=y
+CONFIG_MTD_NOR_FLASH=y
+CONFIG_FLASH_CFI_DRIVER=y
+CONFIG_SYS_FLASH_CFI_WIDTH_16BIT=y
+CONFIG_SYS_CFI_FLASH_STATUS_POLL=y
+CONFIG_SYS_FLASH_USE_BUFFER_WRITE=y
+CONFIG_SYS_FLASH_CFI=y
+CONFIG_SPI_FLASH_MACRONIX=y
+CONFIG_FTMAC100=y
+CONFIG_BAUDRATE=38400
+CONFIG_SYS_NS16550=y
+CONFIG_SPI=y
+CONFIG_ATCSPI200_SPI=y
+# CONFIG_BINMAN_FDT is not set
diff --git a/configs/ae350_rv64_spl_fastboot_defconfig
b/configs/ae350_rv64_spl_fastboot_defconfig
new file mode 100644
index 00..118d13bcda
--- /dev/null
+++ b/configs/ae350_rv64_spl_fastboot_defconfig
@@ -0,0 +1,53 @@
+CONFIG_RISCV=y
+CONFIG_TEXT_BASE=0x0180
+CONFIG_SYS_MALLOC_LEN=0x8
+CONFIG_NR_DRAM_BANKS=2
+CONFIG_ENV_SECT_SIZE=0x1000
+CONFIG_DEFAULT_DEVICE_TREE="ae350_64"
+CONFIG_SYS_PROMPT="RISC-V # "
+CONFIG_SPL_SYS_MALLOC_F_LEN=0x200
+CONFIG_SPL=y
+CONFIG_SPL_OPENSBI_OS_BOOT=y
+CONFIG_SYS_LOAD_ADDR=0x10
+CONFIG_TARGET_AX25_AE350=y
+CONFIG_ARCH_RV64I=y
+CONFIG_RISCV_SMODE=y
+# CONFIG_AVAILABLE_HARTS is not set
+CONFIG_DISTRO_DEFAULTS=y
+CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xe70
+CONFIG_FIT=y
+CONFIG_SPL_LOAD_FIT_ADDRESS=0x1000
+CONFIG_SYS_MONITOR_BASE=0x8800
+CONFIG_BOOTDELAY=3
+CONFIG_BOARD_EARLY_INIT_F=y
+CONFIG_SPL_MAX_SIZE=0x10
+CONFIG_SPL_BSS_START_ADDR=0x40
+CONFIG_SYS_PBSIZE=1050
+CONFIG_SYS_BOOTM_LEN=0x400
+CONFIG_CMD_IMLS=y
+CONFIG_CMD_MMC=y
+CONFIG_CMD_SF_TEST=y
+# CONFIG_CMD_SETEXPR is not set
+CONFIG_BOOTP_PREFER_SERVERIP=y
+CONFIG_CMD_CACHE=y
+CONFIG_ENV_OVERWRITE=y
+CONFIG_ENV_IS_IN_SPI_FLASH=y
+CONFIG_BOOTP_SEND_HOSTNAME=y
+CONFIG_NET_RANDOM_ETHADDR=y
+CONFIG_MMC=y
+CONFIG_FTSDC010=y
+CONFIG_FTSDC010_SDIO=y
+CONFIG_MTD_NOR_FLASH=y
+CONFIG_FLASH_CFI_DRIVER=y
+CONFIG_SYS_FLASH_CFI_WIDTH_16BIT=y
+CONFIG_SYS_CFI_FLASH_STATUS_POLL=y
+CONFIG_SYS_FLASH_USE_BUFFER_WRITE=y
+CONFIG_SYS_FLASH_CFI=y
+CONFIG_SPI_FLASH_MACRONIX=y
+CONFIG_FTMAC100=y
+CONFIG_BAUDRATE=38400
+CONFIG_SYS_NS16550=y
+CONFIG_SPI=y
+CONFIG_ATCSPI200_SPI=y
+# CONFIG_BINMAN_FDT is not set
--
2.17.1
[PATCH 2/4] riscv: dts: Support Fast-Boot
By enabling SPL_OPENSBI_OS_BOOT, it will generate linux.itb instead
of default u-boot.itb after compiling. And Lnux Kernel Image will be
appended in linux.itb. Then it can jump to Linux Kernel from openSBI
directly.
Signed-off-by: Rick Chen
---
arch/riscv/dts/binman.dtsi | 24 ++--
1 file changed, 22 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/dts/binman.dtsi b/arch/riscv/dts/binman.dtsi
index b8fc8f7f35..ca72719d19 100644
--- a/arch/riscv/dts/binman.dtsi
+++ b/arch/riscv/dts/binman.dtsi
@@ -13,14 +13,27 @@
{
itb {
- filename = "u-boot.itb";
+ filename = CONFIG_SPL_OPENSBI_FIT_NAME;
fit {
- description = "Configuration to load OpenSBI before
U-Boot";
+ description = "Configuration to load OpenSBI before OS";
#address-cells = <1>;
fit,fdt-list = "of-list";
images {
+#ifdef CONFIG_SPL_OPENSBI_OS_BOOT
+ linux {
+ description = "Linux";
+ type = "standalone";
+ os = "Linux";
+ arch = "riscv";
+ compression = "none";
+ load = ;
+ linux_blob: blob-ext {
+ filename = "Image";
+ };
+ };
+#else
uboot {
description = "U-Boot";
type = "standalone";
@@ -33,6 +46,7 @@
filename = "u-boot-nodtb.bin";
};
};
+#endif
opensbi {
description = "OpenSBI fw_dynamic
Firmware";
@@ -72,6 +86,12 @@
fdt = "fdt-SEQ";
#endif
};
+
+ conf-2 {
+ description = "linux";
+ firmware = "opensbi";
+ loadables = "linux";
+ };
};
};
};
--
2.17.1
[PATCH 1/4] riscv: spl: Introduce SPL_OPENSBI_OS_BOOT
In RISC-V, it only provide normal mode booting currently.
To speed up the booting process, here provide SPL_OPENSBI_OS_BOOT
to achieve this feature which will be call Fast-Boot mode. By
enabling SPL_OPENSBI_OS_BOOT, it will generate linux.itb instead
of default u-boot.itb after compiling. It initializes memory with
the U-Boot SPL at the first stage, just like what a regular booting
process (i.e. Normal Boot) does in the beginning. Instead of jumping
to the U-Boot proper from OpenSBI before booting Linux Kernel, the
Fast Boot process jumps directly to Linux Kernel to gain shorter
booting time.
Signed-off-by: Rick Chen
---
common/spl/Kconfig | 14 ++
common/spl/spl_fit.c | 3 ++-
common/spl/spl_opensbi.c | 25 -
3 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/common/spl/Kconfig b/common/spl/Kconfig
index 05181bdba3..8805aba1b7 100644
--- a/common/spl/Kconfig
+++ b/common/spl/Kconfig
@@ -1509,6 +1509,20 @@ config SPL_OPENSBI_SCRATCH_OPTIONS
Options passed to fw_dynamic, for example SBI_SCRATCH_NO_BOOT_PRINTS
or
SBI_SCRATCH_DEBUG_PRINTS.
+config SPL_OPENSBI_OS_BOOT
+ bool "openSBI Fast Boot"
+ depends on SPL_OPENSBI
+ help
+ Enable this openSBI can jump to Linux Kernel directly.
+
+config SPL_OPENSBI_FIT_NAME
+ string "SPL openSBI fit image name"
+ depends on SPL_OPENSBI
+ default "linux.itb" if SPL_OPENSBI_OS_BOOT
+ default "u-boot.itb"
+ help
+ This will help to generate different fit name accordingly.
+
config SPL_TARGET
string "Addtional build targets for 'make'"
default "spl/u-boot-spl.srec" if RCAR_GEN2
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index c1ed31e367..c5b1dfb3ba 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -363,7 +363,8 @@ static bool os_takes_devicetree(uint8_t os)
case IH_OS_U_BOOT:
return true;
case IH_OS_LINUX:
- return IS_ENABLED(CONFIG_SPL_OS_BOOT);
+ return IS_ENABLED(CONFIG_SPL_OS_BOOT) ||
+ IS_ENABLED(CONFIG_SPL_OPENSBI_OS_BOOT);
default:
return false;
}
diff --git a/common/spl/spl_opensbi.c b/common/spl/spl_opensbi.c
index b0f40076c3..83869c6b18 100644
--- a/common/spl/spl_opensbi.c
+++ b/common/spl/spl_opensbi.c
@@ -20,7 +20,7 @@ DECLARE_GLOBAL_DATA_PTR;
struct fw_dynamic_info opensbi_info;
-static int spl_opensbi_find_uboot_node(void *blob, int *uboot_node)
+static int spl_opensbi_find_os_node(void *blob, int *os_node)
{
int fit_images_node, node;
const char *fit_os;
@@ -34,10 +34,9 @@ static int spl_opensbi_find_uboot_node(void *blob, int
*uboot_node)
if (!fit_os)
continue;
- if (genimg_get_os_id(fit_os) == IH_OS_U_BOOT) {
- *uboot_node = node;
- return 0;
- }
+ *os_node = node;
+
+ return 0;
}
return -ENODEV;
@@ -45,8 +44,8 @@ static int spl_opensbi_find_uboot_node(void *blob, int
*uboot_node)
void spl_invoke_opensbi(struct spl_image_info *spl_image)
{
- int ret, uboot_node;
- ulong uboot_entry;
+ int ret, os_node;
+ ulong os_entry;
void (*opensbi_entry)(ulong hartid, ulong dtb, ulong info);
if (!spl_image->fdt_addr) {
@@ -54,22 +53,22 @@ void spl_invoke_opensbi(struct spl_image_info *spl_image)
hang();
}
- /* Find U-Boot image in /fit-images */
- ret = spl_opensbi_find_uboot_node(spl_image->fdt_addr, _node);
+ /* Find U-Boot or Linux image in /fit-images */
+ ret = spl_opensbi_find_os_node(spl_image->fdt_addr, _node);
if (ret) {
pr_err("Can't find U-Boot node, %d\n", ret);
hang();
}
- /* Get U-Boot entry point */
- ret = fit_image_get_entry(spl_image->fdt_addr, uboot_node,
_entry);
+ /* Get os entry point */
+ ret = fit_image_get_entry(spl_image->fdt_addr, os_node, _entry);
if (ret)
- ret = fit_image_get_load(spl_image->fdt_addr, uboot_node,
_entry);
+ ret = fit_image_get_load(spl_image->fdt_addr, os_node,
_entry);
/* Prepare opensbi_info object */
opensbi_info.magic = FW_DYNAMIC_INFO_MAGIC_VALUE;
opensbi_info.version = FW_DYNAMIC_INFO_VERSION;
- opensbi_info.next_addr = uboot_entry;
+ opensbi_info.next_addr = os_entry;
opensbi_info.next_mode = FW_DYNAMIC_INFO_NEXT_MODE_S;
opensbi_info.options = CONFIG_SPL_OPENSBI_SCRATCH_OPTIONS;
opensbi_info.boot_hart = gd->arch.boot_hart;
--
2.17.1
Re: [TF-A] Re: [RFC] Proposed location to host the firmware handoff specification.
> OK, this seems to be the crux of the problem. Is it possible for us say that > users are free to register new TLs, while at the same time recommending the > use of existing formats for complex structures (e.g. FDT, HOB)? I'm not really sure what qualifies as a "complex structure" in this discussion? I think each individual TE should deal with a single concern, and separate concerns should be implemented via separate TEs (IIRC I tried to propose language in that direction in my PR)... so I agree that TEs shouldn't be "complex" (if this is what you mean by that), but I think the solution to that is just to divide the complexity down into a larger number of more simple TEs. I also agree that usually each TE should be representable by a simple C structure like I think Simon mentioned somewhere (although I think that should be a recommendation instead of an absolute requirement in case projects have long-established data structures that they want to continue using... the FDT and HOB list TEs aren't representable by a C structure either, after all). > > On the other hand, if this is just supposed to be an early boot flow > > extension to FDTs, then that's very different from what I understood the > > goal > > to be and then I think the text of the spec should be honest about that > > front > > and center. In that case, I wouldn't expect much adoption beyond the > > ecosystem that's already deeply tied to FDT to begin with, though, and that > > would be far from "universal". > > That is another valid usage model. Some ecosystems are deeply wedded to FDT > or HOB/ACPI (there may be others) and this spec is not going to change that. > I don't think we're going to get something universal in the way you're > hoping. But we might be able to at least enable better > interoperability/reusability between fw components across different > ecosystems. Sure, I didn't mean to say this should be disallowed, but the question is whether that is the only allowed use and the FDT is required to pass certain data, or whether adopters are free to choose how they represent their data and the FDT tag is just one of many. Like you said there really needs to be maintainer consensus about this that must be written down somewhere. I don't believe there's going to be a lot of sharing between projects either to be honest, but ultimately that's what this whole thing was started for, right? I think in practice the most likely opportunities for sharing are going to be from small pieces that all projects need, like a TE defining which serial console to print debug output on. (That's another good reason to keep TEs simple, single-concern, and directly encoding data instead of wrapping another structure. In practice I assume that the projects that mostly rely on the wrapped HOB list or FDT will duplicate a few pieces of data into separate TEs to be able to share those with components that can't parse the larger structure.) > 1. A tag that is intended to be used for information passed between > different firmware projects. Such as from TF-A to EDK2 and u-boot. > > 2. A tag for _internal_ use within a firmware project. Here a > firmware project should be free to do whatever they want, but they > still will likely want to use tag IDs that will not conflict with > other uses. I don't see the value of cluttering the firmware > handoff spec with layouts internal to specific firmware projects. This solves the accidental overlap concern but not the organically emerging standard concern. I guess we could say that a tag from the "vendor" range can be promoted to "standard" at a later point by adding it to the global spec (with its existing ID in the vendor range). This does hurt discoverability though, i.e. it's harder for someone who is trying to implement a new TE to realize that the problem has already been solved by another project in a way that would also work out well for them. I still think it's important that tag layouts must be immutable and cannot change once allocated if they come out of that range, that's the basis for any interoperability and backwards-compatibility. And the best way to ensure that is to have them all listed in a global location. Otherwise, even if you write down somewhere that layouts mustn't change, tags shouldn't be reused after they get deprecated, etc., the fact that nobody would notice if you do will encourage people to silently do it anyway. I agree that we don't want to have all of them clutter a single .rst file once there are hundreds or thousands, but there are ways to organize that better which we can decide on once we get to that point.
Re: Re: [PATCH] lib: rsa: cosmetic: fix building warning
Hi,Simon The toolchain source code we use comes from here(https://github.com/riscv-collab/riscv-gnu-toolchain), but we have made some changes. > On Wed, 7 Dec 2022 at 03:50, Haijun Qin wrote: > > > > add initialization of variable 'node',this can aviod the building > > warning: > > > > 'node' may be used uninitialized [-Wmaybe-uninitialized] > > > > Signed-off-by: Haijun Qin > > --- > > lib/rsa/rsa-sign.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > Reviewed-by: Simon Glass > > What toolchain is this? > > > > > > diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c > > index b2a21199e4..d20bdb58a5 100644 > > --- a/lib/rsa/rsa-sign.c > > +++ b/lib/rsa/rsa-sign.c > > @@ -608,7 +608,7 @@ int rsa_add_verify_data(struct image_sign_info *info, > > void *keydest) > > BIGNUM *modulus, *r_squared; > > uint64_t exponent; > > uint32_t n0_inv; > > - int parent, node; > > + int parent, node = -FDT_ERR_NOTFOUND; > > char name[100]; > > int ret; > > int bits; > > > > base-commit: d2c5607edde2544e059fa871927877213f6bd532 > > -- > > 2.17.1 > >
Re: [PATCH v4 0/7] binman: rockchip: Migrate from rockchip SPL_FIT_GENERATOR script
Hi Kevar, On Mon, 7 Nov 2022 at 11:40, Simon Glass wrote: > > At present rockchip 64-bit boards make use of a FIT-generator script > written in Python. The script supports splitting an ELF file into several > 'loadable' nodes in the FIT. Binman does not current support this feature. > > This series adds binman support for ELF splitting. This works by adding a > new 'fit,operation' property to the FIT subnodes, allowing this new way of > generating nodes. > > Some other fixes and improvements are needed along the way. > > A new, common binman description is added for 64-bit boards which includes > the required u-boot.itb file. > > The existing script is removed, so that only a few zynq boards are now > using a SPL_FIT_GENERATOR script: > >avnet_ultrazedev_cc_v1_0_ultrazedev_som_v1_0 >xilinx_zynqmp_virt > > Migration of those is hopefully in progress. > > Note however that tools/k3_fit_atf.sh remains, used by a few boards that > enable CONFIG_TI_SECURE_DEVICE so this series is copied there too: > > am335x_hs_evm > am335x_hs_evm_uart > am43xx_hs_evm > am57xx_hs_evm > am57xx_hs_evm_usb > am65x_hs_evm_a53 > am65x_hs_evm_r5 > dra7xx_hs_evm > dra7xx_hs_evm_usb > j721e_hs_evm_a72 > j721e_hs_evm_r5 > k2e_hs_evm > k2g_hs_evm > k2hk_hs_evm > k2l_hs_evm > > Ivan Mikhaylov has sent a patch to help with these, but I need to take a > look at the testing side. In any case they should really be using binman > for the image generation. > > Changes in v4: > - Add new patch to disable USE_SPL_FIT_GENERATOR by default > > Changes in v3: > - Add an offset to the FIT description > - Add support for writing sections in binman > - Rebase to master > > Changes in v2: > - Rename op-tee to tee-os > - Drop use of .itb2 > - Drop patches previously applied > - Add various suggestions from Alper Nebi Yasak > - Add patches to refactor binman's FIT support > > Simon Glass (7): > binman: Allow writing section contents to a file > rockchip: evb-rk3288: Drop raw-image support > rockchip: Include binman script in 64-bit boards > rockchip: Support building the all output files in binman > rockchip: Convert all boards to use binman > rockchip: Drop the FIT generator script > treewide: Disable USE_SPL_FIT_GENERATOR by default > Can this one please be applied in time for the release? Regards, Simon
Re: [TF-A] Re: [RFC] Proposed location to host the firmware handoff specification.
On 12/5/22 10:18 PM, Julius Werner via TF-A wrote: It seems like some of us still have very different opinions about how this handoff structure should and shouldn't be used, which I really think need to be worked out and then codified in the spec before we can call the document final, because otherwise no firmware project can trust that it is safe to adopt this. My understanding was that this is supposed to be a universal handoff structure that's free to be used by any open source firmware project for any of its internal purposes at any stage of the boot flow as a standalone solution that doesn't force them to pull in dependencies to any other format. If that is the case then I think the spec should reflect this very clearly and should particularly not contain any language about deferring certain types of data to other handoff structures wrapped in the TL. It needs to be clear that projects will be allowed to freely register tags for their needs without the risk of suddenly getting told by the maintainers that they need to use an FDT for this or that instead. I can see 3 types of use cases: 1. A tag that is intended to be used for information passed between different firmware projects. Such as from TF-A to EDK2 and u-boot. 2. A tag for _internal_ use within a firmware project. Here a firmware project should be free to do whatever they want, but they still will likely want to use tag IDs that will not conflict with other uses. I don't see the value of cluttering the firmware handoff spec with layouts internal to specific firmware projects. 3. A tag for experimental or other uses where standardization is not wanted. We have plenty of bits, so why not define 3 ranges: 0x0 -- 0xf_ Standard tag id range. Any tag id in this range must first be allocated in this specification before being used. The allocation of the tag id requires the entry layout to be defined as well. 0x10_ -- 0x10_ Vendor tag id range. Any tag id in this range must first be allocated in this specification before being used. A vendor or project requests a range of tag IDs. The layout of entries in this range is not standardized. 0x11_ -- 0x11_ Non-standard range. A platform firmware integrator can create entries in this range without coordination with the firmware handoff specification. Different platforms are allowed to have tag ids in this range with distinct data formats. Entries in this range are not standardized. 0x12_ -- 0x_Reserved Thanks, Stuart
Re: [PATCH] rtc: add ht1380 driver
Hi Sergei,
On Tue, 6 Dec 2022 at 23:07, Sergei Antonov wrote:
>
> Support Holtek HT1380/HT1381 Serial Timekeeper Chip. It provides seconds
> , minutes, hours, day of the week, date, month and year information.
>
> Datasheet:
> https://www.holtek.com.tw/documents/10179/11842/ht1380_1v130.pdf
>
> Signed-off-by: Sergei Antonov
> ---
>
> v2:
> * The RESET pin is now to be described as ACTIVE_LOW in dts.
>
> Changes suggested by Simon Glass:
> * a more detailed driver description in Kconfig
> * multi-line comments' style
> * enum for 0x80 and the 0x20 at top of file
> * lower-case hex constants
> * function comments for ht1380_reset_on/off
> * blank line before returns
>
> PROTECT remains in a function scope for the sake of locality of definitions.
>
> drivers/rtc/Kconfig | 8 +
> drivers/rtc/Makefile | 1 +
> drivers/rtc/ht1380.c | 337 +++
> 3 files changed, 346 insertions(+)
> create mode 100644 drivers/rtc/ht1380.c
>
> diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
> index 23963271928a..eed48e35a578 100644
> --- a/drivers/rtc/Kconfig
> +++ b/drivers/rtc/Kconfig
> @@ -220,4 +220,12 @@ config RTC_ZYNQMP
> Say "yes" here to support the on chip real time clock
> present on Xilinx ZynqMP SoC.
>
> +config RTC_HT1380
> + bool "Enable Holtek HT1380/HT1381 RTC driver"
> + depends on DM_RTC && DM_GPIO
> + help
> + Say "yes" here to get support for Holtek HT1380/HT1381
> + Serial Timekeeper IC which provides seconds, minutes, hours,
> + day of the week, date, month and year information.
Perhaps mention how it is connected, i.e. three GPIOs.
> +
> endmenu
> diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
> index 009dd9d28c95..f3164782b605 100644
> --- a/drivers/rtc/Makefile
> +++ b/drivers/rtc/Makefile
> @@ -24,6 +24,7 @@ obj-$(CONFIG_RTC_DS3231) += ds3231.o
> obj-$(CONFIG_RTC_DS3232) += ds3232.o
> obj-$(CONFIG_RTC_EMULATION) += emul_rtc.o
> obj-$(CONFIG_RTC_FTRTC010) += ftrtc010.o
> +obj-$(CONFIG_RTC_HT1380) += ht1380.o
> obj-$(CONFIG_SANDBOX) += i2c_rtc_emul.o
> obj-$(CONFIG_RTC_IMXDI) += imxdi.o
> obj-$(CONFIG_RTC_ISL1208) += isl1208.o
> diff --git a/drivers/rtc/ht1380.c b/drivers/rtc/ht1380.c
> new file mode 100644
> index ..25335227d893
> --- /dev/null
> +++ b/drivers/rtc/ht1380.c
> @@ -0,0 +1,337 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Holtek HT1380/HT1381 Serial Timekeeper Chip
> + *
> + * Communication with the chip is vendor-specific.
> + * It is done via 3 GPIO pins: reset, clock, and data.
> + * Describe in .dts this way:
> + *
> + * rtc {
> + * compatible = "holtek,ht1380";
> + * rst-gpio = < 19 GPIO_ACTIVE_LOW>;
> + * clk-gpio = < 20 GPIO_ACTIVE_HIGH>;
> + * dat-gpio = < 21 GPIO_ACTIVE_HIGH>;
> + * };
Is there a binding file for this?
I believe the standard name should be rst-gpios (i.e. plural)?
> + *
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +struct ht1380_priv {
> + struct gpio_desc rst_desc;
> + struct gpio_desc clk_desc;
> + struct gpio_desc dat_desc;
> +};
> +
> +enum registers {
> + SEC,
> + MIN,
> + HOUR,
> + MDAY,
> + MONTH,
> + WDAY,
> + YEAR,
> + WP,
> + N_REGS
> +};
> +
> +enum hour_mode {
> + AMPM_MODE = 0x80, /* RTC is in AM/PM mode */
> + PM_NOW = 0x20,/* set if PM, clear if AM */
> +};
> +
> +static const int BURST = 0xbe;
> +static const int READ = 1;
> +
> +static void ht1380_half_period_delay(void)
> +{
> + /*
> +* Delay for half a period. 1 us complies with the 500 KHz maximum
> +* input serial clock limit given by the datasheet.
> +*/
> + udelay(1);
> +}
> +
> +static int ht1380_send_byte(struct ht1380_priv *priv, int byte)
> +{
> + int ret;
> +
> + for (int bit = 0; bit < 8; bit++) {
> + ret = dm_gpio_set_value(>dat_desc, byte >> bit & 1);
> + if (ret)
> + break;
> + ht1380_half_period_delay();
> +
> + ret = dm_gpio_set_value(>clk_desc, 1);
> + if (ret)
> + break;
> + ht1380_half_period_delay();
> +
> + ret = dm_gpio_set_value(>clk_desc, 0);
> + if (ret)
> + break;
> + }
> +
> + return ret;
> +}
> +
> +/*
> + * Leave reset state. The transfer operation can then be started.
> + */
> +static int ht1380_reset_off(struct ht1380_priv *priv)
> +{
> + const unsigned int T_CC = 4; /* us, Reset to Clock Setup */
> + int ret;
> +
> + /*
> +* Leave RESET state.
> +* Make sure we make the minimal delay required by the datasheet.
> +*/
> + ret = dm_gpio_set_value(>rst_desc, 0);
> + udelay(T_CC);
> +
> + return ret;
> +}
> +
> +/*
> + * Enter reset state. Completes the
Re: [PATCH] cmd: spi: Judge the number of added parameters
On Wed, 7 Dec 2022 at 03:50, chenzhipeng wrote:
>
> When only sspi is entered, help information can be printed.
>
> Signed-off-by: chenzhipeng
> ---
> cmd/spi.c | 3 +++
> 1 file changed, 3 insertions(+)
>
Reviewed-by: Simon Glass
> diff --git a/cmd/spi.c b/cmd/spi.c
> index 454ebe37d7..f30018f33b 100644
> --- a/cmd/spi.c
> +++ b/cmd/spi.c
> @@ -112,6 +112,9 @@ int do_spi(struct cmd_tbl *cmdtp, int flag, int argc,
> char *const argv[])
>
> if ((flag & CMD_FLAG_REPEAT) == 0)
> {
> + if (argc < 2)
> + return CMD_RET_USAGE;
> +
> if (argc >= 2) {
> mode = CONFIG_DEFAULT_SPI_MODE;
> bus = dectoul(argv[1], );
> --
> 2.25.1
>
Re: [PATCH] lib: rsa: cosmetic: fix building warning
On Wed, 7 Dec 2022 at 03:50, Haijun Qin wrote: > > add initialization of variable 'node',this can aviod the building > warning: > > 'node' may be used uninitialized [-Wmaybe-uninitialized] > > Signed-off-by: Haijun Qin > --- > lib/rsa/rsa-sign.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Simon Glass What toolchain is this? > > diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c > index b2a21199e4..d20bdb58a5 100644 > --- a/lib/rsa/rsa-sign.c > +++ b/lib/rsa/rsa-sign.c > @@ -608,7 +608,7 @@ int rsa_add_verify_data(struct image_sign_info *info, > void *keydest) > BIGNUM *modulus, *r_squared; > uint64_t exponent; > uint32_t n0_inv; > - int parent, node; > + int parent, node = -FDT_ERR_NOTFOUND; > char name[100]; > int ret; > int bits; > > base-commit: d2c5607edde2544e059fa871927877213f6bd532 > -- > 2.17.1 >
Re: [PATCHv3 010/149] rsa-verify: Rework host check for CONFIG_RSA_VERIFY_WITH_PKEY
On Wed, 7 Dec 2022 at 07:51, Tom Rini wrote: > > While we do not want to use CONFIG_RSA_VERIFY_WITH_PKEY on the host, we > cannot undef the symbol in this manner. As this ends up being a test > within another function we can use !tools_build() as a test here. > > Cc: AKASHI Takahiro > Cc: Simon Glass > Signed-off-by: Tom Rini > --- > Changes in v3: > - Move comment, per Akashi-san > Changes in v2: > - Switch to !tools_build() per Simon > --- > lib/rsa/rsa-verify.c | 20 +++- > 1 file changed, 7 insertions(+), 13 deletions(-) Reviewed-by: Simon Glass
Re: [PATCH] rockchip: jerry: Enable RESET driver
Hi Kever,
On Wed, 28 Sept 2022 at 20:40, Kever Yang wrote:
>
> Hi Simon,
>
> On 2022/9/28 10:40, Simon Glass wrote:
> > At present the display does not work since it needs the reset driver to
> > operate. Fix this by enabling it.
> >
> > Signed-off-by: Simon Glass
> > Fixes: cd529f7ad62 ("rockchip: video: edp: Add missing reset support")
> > Fixes: 9749d2ea29e ("rockchip: video: vop: Add reset support")
> > ---
> Reviewed-by: Kever Yang
Can this fix please be applied, along with the FIT series? We have hit rc3 now.
https://patchwork.ozlabs.org/project/uboot/list/?series=326766
>
> Thanks,
> - Kever
> >
> > configs/chromebook_jerry_defconfig | 1 +
> > 1 file changed, 1 insertion(+)
Regards,
Simon
[PATCH v2 4/4] ARM: stm32: Make ECDSA authentication available to U-Boot
With U-Boot having access to ROM API call table, it is possible to use the ROM API call it authenticate e.g. signed kernel fitImages using the BootROM ECDSA support. Make this available by pulling the ECDSA BootROM call support from SPL-only guard. Reviewed-by: Patrice Chotard Reviewed-by: Patrick Delaunay Signed-off-by: Marek Vasut --- Cc: Alexandru Gagniuc Cc: Patrice Chotard Cc: Patrick Delaunay --- V2: Add RB from Patrice and Patrick --- arch/arm/mach-stm32mp/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mach-stm32mp/Makefile b/arch/arm/mach-stm32mp/Makefile index 1db9057e049..a19b2797c8b 100644 --- a/arch/arm/mach-stm32mp/Makefile +++ b/arch/arm/mach-stm32mp/Makefile @@ -11,10 +11,10 @@ obj-y += bsec.o obj-$(CONFIG_STM32MP13x) += stm32mp13x.o obj-$(CONFIG_STM32MP15x) += stm32mp15x.o +obj-$(CONFIG_STM32_ECDSA_VERIFY) += ecdsa_romapi.o ifdef CONFIG_SPL_BUILD obj-y += spl.o obj-y += tzc400.o -obj-$(CONFIG_STM32_ECDSA_VERIFY) += ecdsa_romapi.o else obj-y += cmd_stm32prog/ obj-$(CONFIG_CMD_STM32KEY) += cmd_stm32key.o -- 2.35.1
[PATCH v2 3/4] ARM: stm32: Pass ROM API table pointer to U-Boot proper
The ROM API table pointer is no longer accessible from U-Boot, fix
this by passing the ROM API pointer through. This makes it possible
for U-Boot to call ROM API functions to authenticate payload like
signed fitImages.
Signed-off-by: Marek Vasut
---
Cc: Alexandru Gagniuc
Cc: Patrice Chotard
Cc: Patrick Delaunay
---
V2: - Rename image_entry_noargs_t to image_entry_stm32_t
- Add missing __noreturn
---
arch/arm/mach-stm32mp/cpu.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c
index ee59866bb73..dc4112d5e6c 100644
--- a/arch/arm/mach-stm32mp/cpu.c
+++ b/arch/arm/mach-stm32mp/cpu.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
/*
* early TLB into the .data section so that it not get cleared
@@ -413,3 +414,17 @@ uintptr_t get_stm32mp_bl2_dtb(void)
{
return nt_fw_dtb;
}
+
+#ifdef CONFIG_SPL_BUILD
+void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
+{
+ typedef void __noreturn (*image_entry_stm32_t)(u32 romapi);
+ uintptr_t romapi = get_stm32mp_rom_api_table();
+
+ image_entry_stm32_t image_entry =
+ (image_entry_stm32_t)spl_image->entry_point;
+
+ printf("image entry point: 0x%lx\n", spl_image->entry_point);
+ image_entry(romapi);
+}
+#endif
--
2.35.1
[PATCH v2 2/4] ARM: stm32: Factor out save_boot_params
The STM32MP15xx platform currently comes with two incompatible
implementations of save_boot_params() weak function override.
Factor the save_boot_params() implementation into common cpu.c
code and provide accessors to read out both ROM API table address
and DT address from any place in the code instead.
Signed-off-by: Marek Vasut
---
Cc: Alexandru Gagniuc
Cc: Patrice Chotard
Cc: Patrick Delaunay
---
V2: Avoid #if CONFIG... , use if (CONFIG... instead
---
arch/arm/mach-stm32mp/boot_params.c | 20 ++-
arch/arm/mach-stm32mp/cpu.c | 35 +++
arch/arm/mach-stm32mp/ecdsa_romapi.c | 20 ++-
.../arm/mach-stm32mp/include/mach/sys_proto.h | 3 ++
4 files changed, 42 insertions(+), 36 deletions(-)
diff --git a/arch/arm/mach-stm32mp/boot_params.c
b/arch/arm/mach-stm32mp/boot_params.c
index e91ef1b2fc7..e40cca938ef 100644
--- a/arch/arm/mach-stm32mp/boot_params.c
+++ b/arch/arm/mach-stm32mp/boot_params.c
@@ -11,30 +11,14 @@
#include
#include
-/*
- * Force data-section, as .bss will not be valid
- * when save_boot_params is invoked.
- */
-static unsigned long nt_fw_dtb __section(".data");
-
-/*
- * Save the FDT address provided by TF-A in r2 at boot time
- * This function is called from start.S
- */
-void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2,
- unsigned long r3)
-{
- nt_fw_dtb = r2;
-
- save_boot_params_ret();
-}
-
/*
* Use the saved FDT address provided by TF-A at boot time (NT_FW_CONFIG =
* Non Trusted Firmware configuration file) when the pointer is valid
*/
void *board_fdt_blob_setup(int *err)
{
+ unsigned long nt_fw_dtb = get_stm32mp_bl2_dtb();
+
log_debug("%s: nt_fw_dtb=%lx\n", __func__, nt_fw_dtb);
*err = 0;
diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c
index 855fc755fe0..ee59866bb73 100644
--- a/arch/arm/mach-stm32mp/cpu.c
+++ b/arch/arm/mach-stm32mp/cpu.c
@@ -378,3 +378,38 @@ int arch_misc_init(void)
return 0;
}
+
+/*
+ * Without forcing the ".data" section, this would get saved in ".bss". BSS
+ * will be cleared soon after, so it's not suitable.
+ */
+static uintptr_t rom_api_table __section(".data");
+static uintptr_t nt_fw_dtb __section(".data");
+
+/*
+ * The ROM gives us the API location in r0 when starting. This is only
available
+ * during SPL, as there isn't (yet) a mechanism to pass this on to u-boot. Save
+ * the FDT address provided by TF-A in r2 at boot time. This function is called
+ * from start.S
+ */
+void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2,
+ unsigned long r3)
+{
+ if (IS_ENABLED(CONFIG_STM32_ECDSA_VERIFY))
+ rom_api_table = r0;
+
+ if (IS_ENABLED(CONFIG_TFABOOT))
+ nt_fw_dtb = r2;
+
+ save_boot_params_ret();
+}
+
+uintptr_t get_stm32mp_rom_api_table(void)
+{
+ return rom_api_table;
+}
+
+uintptr_t get_stm32mp_bl2_dtb(void)
+{
+ return nt_fw_dtb;
+}
diff --git a/arch/arm/mach-stm32mp/ecdsa_romapi.c
b/arch/arm/mach-stm32mp/ecdsa_romapi.c
index 082178ce83f..b1ab49165e7 100644
--- a/arch/arm/mach-stm32mp/ecdsa_romapi.c
+++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c
@@ -24,26 +24,10 @@ struct ecdsa_rom_api {
uint32_t ecc_algo);
};
-/*
- * Without forcing the ".data" section, this would get saved in ".bss". BSS
- * will be cleared soon after, so it's not suitable.
- */
-static uintptr_t rom_api_loc __section(".data");
-
-/*
- * The ROM gives us the API location in r0 when starting. This is only
available
- * during SPL, as there isn't (yet) a mechanism to pass this on to u-boot.
- */
-void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2,
- unsigned long r3)
-{
- rom_api_loc = r0;
- save_boot_params_ret();
-}
-
static void stm32mp_rom_get_ecdsa_functions(struct ecdsa_rom_api *rom)
{
- uintptr_t verify_ptr = rom_api_loc + ROM_API_OFFSET_ECDSA_VERIFY;
+ uintptr_t verify_ptr = get_stm32mp_rom_api_table() +
+ ROM_API_OFFSET_ECDSA_VERIFY;
rom->ecdsa_verify_signature = *(void **)verify_ptr;
}
diff --git a/arch/arm/mach-stm32mp/include/mach/sys_proto.h
b/arch/arm/mach-stm32mp/include/mach/sys_proto.h
index f19a70e53e0..0d39b67178e 100644
--- a/arch/arm/mach-stm32mp/include/mach/sys_proto.h
+++ b/arch/arm/mach-stm32mp/include/mach/sys_proto.h
@@ -77,3 +77,6 @@ void stm32mp_misc_init(void);
/* helper function: read data from OTP */
u32 get_otp(int index, int shift, int mask);
+
+uintptr_t get_stm32mp_rom_api_table(void);
+uintptr_t get_stm32mp_bl2_dtb(void);
--
2.35.1
[PATCH v2 1/4] ARM: stm32: Fix ECDSA authentication with Dcache enabled
In case Dcache is enabled while the ECDSA authentication function is
called via BootROM ROM API, the CRYP DMA might pick stale version of
data from DRAM. Disable Dcache around the BootROM call to avoid this
issue.
Signed-off-by: Marek Vasut
---
Cc: Alexandru Gagniuc
Cc: Patrice Chotard
Cc: Patrick Delaunay
---
V2: - Initialize reenable_dcache variable
---
arch/arm/mach-stm32mp/ecdsa_romapi.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/arm/mach-stm32mp/ecdsa_romapi.c
b/arch/arm/mach-stm32mp/ecdsa_romapi.c
index a2f63ff879f..082178ce83f 100644
--- a/arch/arm/mach-stm32mp/ecdsa_romapi.c
+++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c
@@ -63,6 +63,7 @@ static int romapi_ecdsa_verify(struct udevice *dev,
const void *hash, size_t hash_len,
const void *signature, size_t sig_len)
{
+ bool reenable_dcache = false;
struct ecdsa_rom_api rom;
uint8_t raw_key[64];
uint32_t rom_ret;
@@ -81,8 +82,21 @@ static int romapi_ecdsa_verify(struct udevice *dev,
memcpy(raw_key + 32, pubkey->y, 32);
stm32mp_rom_get_ecdsa_functions();
+
+ /*
+* Disable D-cache before calling into BootROM, else CRYP DMA
+* may fail to pick up the correct data.
+*/
+ if (dcache_status()) {
+ dcache_disable();
+ reenable_dcache = true;
+ }
+
rom_ret = rom.ecdsa_verify_signature(hash, raw_key, signature, algo);
+ if (reenable_dcache)
+ dcache_enable();
+
return rom_ret == ROM_API_SUCCESS ? 0 : -EPERM;
}
--
2.35.1
Re: [PATCH 4/6] arm: Use the WEAK assembly entry point consistently
On Thu, Nov 24, 2022 at 12:40:44AM +0100, Pali Rohár wrote:
> On Wednesday 23 November 2022 18:15:17 Tom Rini wrote:
> > On Wed, Nov 23, 2022 at 11:50:59PM +0100, Pali Rohár wrote:
> > > On Tuesday 22 November 2022 12:31:56 Tom Rini wrote:
> > > > It is a bad idea, and more modern toolchains will fail, if you declare
> > > > an assembly function to be global and then weak, instead of declaring it
> > > > weak to start with. Update assorted assembly files to use the WEAK macro
> > > > directly.
> > > >
> > > > Signed-off-by: Tom Rini
> > >
> > > During debugging of Nokia N900 code I was looking at this and I
> > > originally thought that this redefinition is the issue why N900 u-boot
> > > did not work... (but as we now know, the n900 issue was somewhere else).
> > >
> > > So I agree with this change, feel free to add my:
> > >
> > > Reviewed-by: Pali Rohár
> > >
> > > ... but even after this change, linked u-boot.bin binary is
> > > not-so-correct. It works but has an issue: In final u-boot.bin binary
> > > there is both weak and non-weak version of every weak function. You can
> > > verify it for example by looking at "save_boot_params" code (really
> > > code, not just symbol) in u-boot ELF binary.
> > >
> > > The reason for this is that linker (even LTO enabled) cannot eliminate
> > > code for weak version of function because it does not know how to
> > > "drop" it from binary/assembly code. So linker just set that non-weak
> > > version of function is active and let non-weak version present in binary
> > > as probably dead code.
> > >
> > > This is affected only by assembly files, not by C files, because gcc is
> > > called with -ffunction-sections -fdata-sections flags which cause that
> > > every (weak) function is in its separate section (so C function
> > > "int abc() { ... }" is put into the section ".text.abc" instead of
> > > ".text") and linker's flag --gc-sections (or LTO optimization) then drop
> > > all unreferenced sections.
> > >
> > > I do not know how fix this issue in assembly files. But cannot be WEAK
> > > macro modified to change section to ".text." to mimic C
> > > compiler behavior? Would this cause any issues?
> >
> > Yes, you're right about the cause, and potential solution. If you can
> > come up with a way to get each assembly function put in to a separate
> > .text.funcname section, that would be great and much appreciated. I
> > think I tried this at one point a long long time ago and it did work,
> > but I didn't have something clean, either. I think I was hoping that the
> > linux kernel folks would solve it in time, but they decided the
> > time/effort for --gc-sections wasn't worth it, in the end.
>
> I quickly looked at this. If "as" is invoked with --sectname-subst flag
> then it is possible to use '.section %S.' and '.previous'
> directives. See documentation where is example of that:
> https://sourceware.org/binutils/docs/as/Section.html#ELF-Version
>
> I experimented with adding into include/linux/linkage.h:
>
> #define WEAKSECT(name) \
> .section .text.name ASM_NL \
> WEAK(name)
>
> #define ENDPROCSECT(name) \
> ENDPROC(name) ASM_NL \
> .previous
>
> And then defining in arch/arm/cpu/armv7/start.S:
>
> WEAKSECT(save_boot_params)
> b save_boot_params_ret
> ENDPROCSECT(save_boot_params)
>
> (Note that n900 has custom non-weak save_boot_params)
>
> Then I run:
>
> make CROSS_COMPILE=arm-linux-gnueabi- nokia_rx51_defconfig u-boot.bin
> KBUILD_LDFLAGS="--print-gc-sections"
>
> And it showed me:
>
> ld: removing unused section '.text.save_boot_params' in file
> 'arch/arm/cpu/armv7/start.o'
>
> So seems that it is working.
>
> For proper integration it would be needed to integrate --sectname-subst
> flag support and then replace all usage by new macros.
That's all good to know, thanks for digging more. It would be good to
know if a quick and dirty experimental patch showed enough size savings
to be worth a more full pursuit or if we really don't have many / any
unused assembly functions or what we do have unused could be more easily
handled with an ifdef or refactoring into multiple files.
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH 0/7] updates for km board series
On Fri, Dec 02, 2022 at 06:22:36PM +0100, Holger Brunck wrote: > - migrate all boards to environment.txt files > - migrate PPC boards to DM_I2C > - some cleanup > > Holger Brunck (7): > board/km: move ls102xa boards to environment text files > km/powerpc: migrate to env.txt file > board/km/cent2: migrate to environment text file > board/km/secu: migrate to use environment text files > board/km: remove obsolete ARCH_KIRKWOOD > km/ppc: migrate all mpc83xx to DM_I2C > km/mpc8360: remove unused CONFIG_SYS_PAXE defines Thanks for doing this. As this is the first in my mind big environment conversion, do you have any further feedback on the mechanism, things that would make this easier / more useful, etc? -- Tom signature.asc Description: PGP signature
Re: [PATCH 1/1] mvebu: fix end-of-array check
On Tuesday 06 December 2022 07:10:15 Stefan Roese wrote:
> Hi Pali,
>
> On 12/5/22 19:18, Pali Rohár wrote:
> > On Monday 05 December 2022 12:42:44 Stefan Roese wrote:
> > > Hi!
> > >
> > > On 12/4/22 11:39, Pali Rohár wrote:
> > > > Hello!
> > > >
> > > > I would suggest to change description of the patch from
> > > >
> > > > "mvebu: fix end-of-array check"
> > > >
> > > > to something like:
> > > >
> > > > "arm: mvebu: Espressobin: fix end-of-array check in env"
> > > >
> > > > as it affects only Espressobin boards (not other mvebu).
> > >
> > > Yes, please update the commit subject here.
> > >
> > > > Stefan, please look below as this issue/fix is important.
> > >
> > > Yes.
> > >
> > > > On Wednesday 30 November 2022 13:33:40 Derek LaHousse wrote:
> > > > > Properly seek the end of default_environment variables.
> > > > >
> > > > > The current algorithm overwrites from the second variable. This
> > > > > replacement finds the end of the array of strings.
> > > > >
> > > > > Stomped variables include "board", "soc", "loadaddr". These can be
> > > > > seen on a "env default -a" after patch, but they are not seen with a
> > > > > version before the patch.
> > > >
> > > > This is a real issue which I introduced in the past. So it some fix for
> > > > this issue should be pulled into the v2023.01 release.
> > >
> > > Understood.
> > >
> > > > > Signed-off-by: Derek LaHousse
> > > > > ---
> > > > >board/Marvell/mvebu_armada-37xx/board.c | 7 +--
> > > > >1 file changed, 5 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/board/Marvell/mvebu_armada-37xx/board.c
> > > > > b/board/Marvell/mvebu_armada-37xx/board.c
> > > > > index c6ecc323bb..ac29ac5b95 100644
> > > > > --- a/board/Marvell/mvebu_armada-37xx/board.c
> > > > > +++ b/board/Marvell/mvebu_armada-37xx/board.c
> > > > > @@ -100,8 +100,11 @@ int board_late_init(void)
> > > > > return 0;
> > > > > /* Find free buffer in default_environment[] for new variables
> > > > > */
> > > > > - while (*ptr != '\0' && *(ptr+1) != '\0') ptr++;
> > > > > - ptr += 2;
> > > > > + if (*ptr != '\0') { // Defending against empty default env
> > > > > + while ((i = strlen(ptr)) != 0) {
> > > > > + ptr += i + 1;
> > > > > + }
> > > > > + }
> > > >
> > > > If I'm looking at the outer-if condition correctly then it is not
> > > > needed. strlen("") returns zero and so inner-while loop stops
> > > > immediately.
> > > >
> > > > My proposed fix for this issue was just changing correct while loop
> > > > check to ensure that ptr is set after the _last_ variable.
> > > >
> > > > - while (*ptr != '\0' && *(ptr+1) != '\0') ptr++;
> > > > - ptr += 2;
> > > > + while (*ptr != '\0' || *(ptr+1) != '\0') ptr++;
> > > > + ptr++;
> > > >
> > > > Both changes should be equivalent, but I'm not sure which one is more
> > > > readable. The original issue was introduced really by non-readable
> > > > code...
> > > >
> > > > Stefan, do you have a preference which one fix is better / more
> > > > readable?
> > >
> > > I would prefer to get Pali's corrected version included. Could you
> > > please prepare a v2 patch with this update and also with the added
> > > or changed patch subject.
> >
> > Originally this issue was reported half year ago on the armbian forum:
> > https://forum.armbian.com/topic/19564-making-espressobin-v7-work-in-2022/?do=findComment=138136
> >
> > U-Boot "changed" variable name scriptaddr to criptaddr (without leading
> > s) and broke booting.
> >
> > Details are later in comments:
> > https://forum.armbian.com/topic/19564-making-espressobin-v7-work-in-2022/?do=findComment=154062
> > https://forum.armbian.com/topic/19564-making-espressobin-v7-work-in-2022/?do=findComment=154235
> >
> > If you prefer my variant, I can prepare a v2 patch.
>
> Yes, please do. That's easiest for me. I'll push this quickly then.
>
> Thanks,
> Stefan
Hello Stefan! During discussion on the forum, Derek pointed that my
"fix" does not work for another edge case when *ptr is '\0' before
entering into while-loop. That is a valid point and code needs to be
adjusted.
With Derek we do not have an agreement how to write this peace of the
code in readable way, which should move ptr pointer to the end of env
list - find two nul bytes which indicates end of the env list and then
set ptr to the second nul byte, so at this position can be put another
new variable. Plus handle special case nul byte is at the beginning.
My idea was to use single while loop to find this location to have code
minimal in its size. Derek thinks that code is better readable if
iteration is done via strlen() calls in while-loop, like it is in this
version.
Stefan, do you have an idea how to write this code in a way that is fast
and also readable and maintainable?
[PATCHv3 010/149] rsa-verify: Rework host check for CONFIG_RSA_VERIFY_WITH_PKEY
While we do not want to use CONFIG_RSA_VERIFY_WITH_PKEY on the host, we
cannot undef the symbol in this manner. As this ends up being a test
within another function we can use !tools_build() as a test here.
Cc: AKASHI Takahiro
Cc: Simon Glass
Signed-off-by: Tom Rini
---
Changes in v3:
- Move comment, per Akashi-san
Changes in v2:
- Switch to !tools_build() per Simon
---
lib/rsa/rsa-verify.c | 20 +++-
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/lib/rsa/rsa-verify.c b/lib/rsa/rsa-verify.c
index 9605c376390a..2f3b34403913 100644
--- a/lib/rsa/rsa-verify.c
+++ b/lib/rsa/rsa-verify.c
@@ -23,18 +23,6 @@
#include
#include
-#ifndef __UBOOT__
-/*
- * NOTE:
- * Since host tools, like mkimage, make use of openssl library for
- * RSA encryption, rsa_verify_with_pkey()/rsa_gen_key_prop() are
- * of no use and should not be compiled in.
- * So just turn off CONFIG_RSA_VERIFY_WITH_PKEY.
- */
-
-#undef CONFIG_RSA_VERIFY_WITH_PKEY
-#endif
-
/* Default public exponent for backward compatibility */
#define RSA_DEFAULT_PUBEXP 65537
@@ -506,7 +494,13 @@ int rsa_verify_hash(struct image_sign_info *info,
{
int ret = -EACCES;
- if (CONFIG_IS_ENABLED(RSA_VERIFY_WITH_PKEY) && !info->fdt_blob) {
+ /*
+* Since host tools, like mkimage, make use of openssl library for
+* RSA encryption, rsa_verify_with_pkey()/rsa_gen_key_prop() are
+* of no use and should not be compiled in.
+*/
+ if (!tools_build() && CONFIG_IS_ENABLED(RSA_VERIFY_WITH_PKEY) &&
+ !info->fdt_blob) {
/* don't rely on fdt properties */
ret = rsa_verify_with_pkey(info, hash, sig, sig_len);
if (ret)
--
2.25.1
Re: [PATCH v2] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2.
El Mon, Dec 05, 2022 at 08:08:40PM +0100, Marek Vasut deia: > On 12/5/22 19:54, Xavier Drudis Ferran wrote: > > 5- Trying to replicate linux and have usb2phy somehow provide a clk, > > or have a separate clock device for usb2phy in addition to the phy > > device. I just can't seem to imagine how to achieve this with the > > U-Boot driver model, maybe because of my limited familiarity with > > it. > > Yes please > > Have a look at the end of drivers/led/led_gpio.c and how gpio_led_wrap binds > a gpio_led driver to each LED. You can bind an UCLASS_PHY and UCLASS_CLK > driver to the u2phy0 the same way, the u2phy0 would behave the same way as > gpio_led_wrap wrapper . You would likely be using device_bind_driver() > instead of device_bind_driver_to_node() in the bind callback. > Ok, I will take a look and send a v3 if I understand it. Thank you.
Re: PSU initialization code on zynqmp platforms
Hi Michal, On 2022-12-06 02:50, Michal Simek wrote: Hi, On 12/6/22 02:01, Graeme Smecher wrote: Hi Michal, (Well, that's embarassing. Sending again with a useful subject.) I'm bringing up u-boot on a custom zynqmp platform. For the PSU initialization, I can start with the psu_init_gpl.[ch] files embedded in the .xsa generated by Vivado. However, these are pretty flawed [1, 2]. What is your workflow for generating and maintaining the in-tree psu_init_gpl files you maintain? Are these files cleaned up and maintained by hand? If your workflow is functional, I will happily emulate it instead - even if it means straying from Vivado's configuration GUI. I am just copy psu init files from xsa and copy it somewhere and then run ./tools/zynqmp_psu_init_minimize.sh to minimize it. And then commit and small manual cleanups to resolved issues reported by checkpatch. Thanks - that script is exactly what I needed to plug the gap between Vivado and what's in-tree. best, Graeme
Re: Pull request for sound-2023-01-rc4
On Tue, Dec 06, 2022 at 09:42:39AM +0100, Heinrich Schuchardt wrote: > Dear Tom, > > The following changes since commit a50622d78c5c6babd1853ae913f339df54fe532c: > > Merge tag 'xilinx-for-v2023.01-rc3-v2' of > https://source.denx.de/u-boot/custodians/u-boot-microblaze (2022-12-05 > 08:33:19 -0500) > > are available in the Git repository at: > > https://source.denx.de/u-boot/custodians/u-boot-efi.git > tags/sound-2023-01-rc4 > > for you to fetch changes up to 304bc9f437df51b4d982fe25fd0988350c8f5fc9: > > sandbox: fix sound driver (2022-12-05 17:43:23 +0100) > > Gitlab CI showed no issues: > https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/14337 > Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
[PATCH 3/4] dts: synquacer: Fix "arm, armv7-timer-mem" node address sizes
The "arm,armv7-timer-mem" schema defines the address sizes for child
nodes to be 32-bit as there's no need for 64-bit offsets and sizes of
the child 'frame' nodes.
Signed-off-by: Rob Herring
---
arch/arm/dts/synquacer-sc2a11.dtsi | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/arch/arm/dts/synquacer-sc2a11.dtsi
b/arch/arm/dts/synquacer-sc2a11.dtsi
index 0e1bc164549f..049afcb0af8a 100644
--- a/arch/arm/dts/synquacer-sc2a11.dtsi
+++ b/arch/arm/dts/synquacer-sc2a11.dtsi
@@ -364,13 +364,13 @@
timer@2a81 {
compatible = "arm,armv7-timer-mem";
reg = <0x0 0x2a81 0x0 0x1>;
-#address-cells = <2>;
-#size-cells = <2>;
-ranges;
-frame@2a83 {
+#address-cells = <1>;
+#size-cells = <1>;
+ranges = <0x0 0x0 0x2a81 0x3>;
+frame@2 {
frame-number = <0>;
interrupts = ;
-reg = <0x0 0x2a83 0x0 0x1>;
+reg = <0x2 0x1>;
};
};
--
b4 0.11.0-dev
[PATCH 4/4] dts: synquacer: Fix idle-states 'entry-method' value
The correct value for 'entry-method' in the idle-states binding is 'psci',
not 'arm,psci'. It hasn't mattered because it isn't used by the OS.
Signed-off-by: Rob Herring
---
arch/arm/dts/synquacer-sc2a11.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/dts/synquacer-sc2a11.dtsi
b/arch/arm/dts/synquacer-sc2a11.dtsi
index 049afcb0af8a..0dd2969b5e3c 100644
--- a/arch/arm/dts/synquacer-sc2a11.dtsi
+++ b/arch/arm/dts/synquacer-sc2a11.dtsi
@@ -309,7 +309,7 @@
};
idle-states {
-entry-method = "arm,psci";
+entry-method = "psci";
CPU_SLEEP_0: cpu-sleep-0 {
compatible = "arm,idle-state";
--
b4 0.11.0-dev
[PATCH 1/4] dts: synquacer: Drop CPU 'arm,armv8' compatibles
'arm,armv8' compatible is for software models only. so drop it from cpu
nodes.
Signed-off-by: Rob Herring
---
arch/arm/dts/synquacer-sc2a11.dtsi | 48 +++---
1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/arch/arm/dts/synquacer-sc2a11.dtsi
b/arch/arm/dts/synquacer-sc2a11.dtsi
index 1fe7d214b9c9..1f8cd9d25780 100644
--- a/arch/arm/dts/synquacer-sc2a11.dtsi
+++ b/arch/arm/dts/synquacer-sc2a11.dtsi
@@ -41,168 +41,168 @@
CPU0: cpu@0 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x0>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU1: cpu@1 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x1>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU2: cpu@100 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x100>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU3: cpu@101 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x101>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU4: cpu@200 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x200>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU5: cpu@201 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x201>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU6: cpu@300 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x300>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU7: cpu@301 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x301>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU8: cpu@400 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x400>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU9: cpu@401 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x401>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU10: cpu@500 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x500>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU11: cpu@501 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x501>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU12: cpu@600 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x600>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU13: cpu@601 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x601>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU14: cpu@700 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x700>;
enable-method = "psci";
cpu-idle-states = <_SLEEP_0 _SLEEP_0>;
};
CPU15: cpu@701 {
device_type = "cpu";
-compatible = "arm,cortex-a53","arm,armv8";
+compatible = "arm,cortex-a53";
reg = <0x701>;
[PATCH 2/4] dts: synquacer: Use generic node names
DT node names should follow generic names defined in the DT spec. These
are also now checked by dtschema tools.
Signed-off-by: Rob Herring
---
arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi | 2 +-
arch/arm/dts/synquacer-sc2a11-developerbox.dts | 2 +-
arch/arm/dts/synquacer-sc2a11.dtsi | 10 +-
3 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi
b/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi
index 7a56116d6f12..6b85c05458d4 100644
--- a/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi
+++ b/arch/arm/dts/synquacer-sc2a11-developerbox-u-boot.dtsi
@@ -23,7 +23,7 @@
active_clk_edges;
chipselect_num = <1>;
- spi-flash@0 {
+ flash@0 {
#address-cells = <1>;
#size-cells = <1>;
compatible = "jedec,spi-nor";
diff --git a/arch/arm/dts/synquacer-sc2a11-developerbox.dts
b/arch/arm/dts/synquacer-sc2a11-developerbox.dts
index 42b6cbbb8251..c8087b99a781 100644
--- a/arch/arm/dts/synquacer-sc2a11-developerbox.dts
+++ b/arch/arm/dts/synquacer-sc2a11-developerbox.dts
@@ -18,7 +18,7 @@
compatible = "gpio-keys";
interrupt-parent = <>;
-power {
+power-button {
label = "Power Button";
linux,code = ;
interrupts = ;
diff --git a/arch/arm/dts/synquacer-sc2a11.dtsi
b/arch/arm/dts/synquacer-sc2a11.dtsi
index 1f8cd9d25780..0e1bc164549f 100644
--- a/arch/arm/dts/synquacer-sc2a11.dtsi
+++ b/arch/arm/dts/synquacer-sc2a11.dtsi
@@ -344,7 +344,7 @@
interrupt-controller;
interrupts = ;
-its: gic-its@3002 {
+its: msi-controller@3002 {
compatible = "arm,gic-v3-its";
reg = <0x0 0x3002 0x0 0x2>;
#msi-cells = <1>;
@@ -361,7 +361,7 @@
; // HYP
};
-mmio-timer@2a81 {
+timer@2a81 {
compatible = "arm,armv7-timer-mem";
reg = <0x0 0x2a81 0x0 0x1>;
#address-cells = <2>;
@@ -398,7 +398,7 @@
clock-output-names = "apb_pclk";
};
-soc_uart0: uart@2a40 {
+soc_uart0: serial@2a40 {
compatible = "arm,pl011", "arm,primecell";
reg = <0x0 0x2a40 0x0 0x1000>;
interrupts = ;
@@ -406,7 +406,7 @@
clock-names = "uartclk", "apb_pclk";
};
-fuart: uart@5104 {
+fuart: serial@5104 {
compatible = "snps,dw-apb-uart";
reg = <0x0 0x5104 0x0 0x1000>;
interrupts = ;
@@ -523,7 +523,7 @@
clock-output-names = "sd_sd4clk";
};
-sdhci: sdhci@5230 {
+sdhci: mmc@5230 {
compatible = "socionext,synquacer-sdhci", "fujitsu,mb86s70-sdhci-3.0";
reg = <0 0x5230 0x0 0x1000>;
interrupts = ,
--
b4 0.11.0-dev
[PATCH 0/4] Synquacer DT schema fixes
This is a series of DT fixes for the Synquacer. These issues were found running the dtschema tools. I don't have a board, but Ilias has tested the changes for me. Thanks! Signed-off-by: Rob Herring --- Rob Herring (4): dts: synquacer: Drop CPU 'arm,armv8' compatibles dts: synquacer: Use generic node names dts: synquacer: Fix "arm,armv7-timer-mem" node address sizes dts: synquacer: Fix idle-states 'entry-method' value .../dts/synquacer-sc2a11-developerbox-u-boot.dtsi | 2 +- arch/arm/dts/synquacer-sc2a11-developerbox.dts | 2 +- arch/arm/dts/synquacer-sc2a11.dtsi | 70 +++--- 3 files changed, 37 insertions(+), 37 deletions(-) --- base-commit: bebb393b340295edb9ba50a996fc0510cd1b6ac0 change-id: 20221206-synquacer-dts-521500f88a1d Best regards, -- Rob Herring
RE: [TF-A] [RFC] Proposed location to host the firmware handoff specification.
Hi Julius > -Original Message- > From: Julius Werner > Sent: 06 December 2022 04:18 > > It seems like some of us still have very different opinions about how this > handoff structure should and shouldn't be used, which I really think need to > be worked out and then codified in the spec before we can call the document > final, because otherwise no firmware project can trust that it is safe to > adopt this. Yes, there are some very differing opinions on usage, but I think it's possible to accommodate multiple usage models at the same time. I agree we need to have maintainer consensus on how the spec will evolve and have this written down (e.g. the tag allocation policy). > My understanding was that this is supposed to be a universal > handoff structure that's free to be used by any open source firmware project > for any of its internal purposes at any stage of the boot flow as a > standalone solution that doesn't force them to pull in dependencies to any > other format. That is a valid usage model, though not the only (universal) one. > If that is the case then I think the spec should reflect this > very clearly and should particularly not contain any language about deferring > certain types of data to other handoff structures wrapped in the TL. It needs > to be clear that projects will be allowed to freely register tags for their > needs without the risk of suddenly getting told by the maintainers that they > need to use an FDT for this or that instead. > OK, this seems to be the crux of the problem. Is it possible for us say that users are free to register new TLs, while at the same time recommending the use of existing formats for complex structures (e.g. FDT, HOB)? > On the other hand, if this is just supposed to be an early boot flow > extension to FDTs, then that's very different from what I understood the goal > to be and then I think the text of the spec should be honest about that front > and center. In that case, I wouldn't expect much adoption beyond the > ecosystem that's already deeply tied to FDT to begin with, though, and that > would be far from "universal". That is another valid usage model. Some ecosystems are deeply wedded to FDT or HOB/ACPI (there may be others) and this spec is not going to change that. I don't think we're going to get something universal in the way you're hoping. But we might be able to at least enable better interoperability/reusability between fw components across different ecosystems. Regards Dan.
[PATCH v2] renesas: rcar: Apply ATF overlay for reserved-memory
The function fdtdec_board_setup() is only called by fdtdec_setup() which
needs to be called by the board file.
This is not the case for the renesas boards so rename the
fdtdec_board_setup() function to a local name and call it directly from
ft_board_setup(), before cleaning up the memory nodes.
Signed-off-by: Detlev Casanova
---
board/renesas/rcar-common/common.c | 19 +--
1 file changed, 9 insertions(+), 10 deletions(-)
diff --git a/board/renesas/rcar-common/common.c
b/board/renesas/rcar-common/common.c
index daa1beb14f..c50d09ef8b 100644
--- a/board/renesas/rcar-common/common.c
+++ b/board/renesas/rcar-common/common.c
@@ -25,16 +25,6 @@ extern u64 rcar_atf_boot_args[];
#define FDT_RPC_PATH "/soc/spi@ee20"
-int fdtdec_board_setup(const void *fdt_blob)
-{
- void *atf_fdt_blob = (void *)(rcar_atf_boot_args[1]);
-
- if (fdt_magic(atf_fdt_blob) == FDT_MAGIC)
- fdt_overlay_apply_node((void *)fdt_blob, 0, atf_fdt_blob, 0);
-
- return 0;
-}
-
int dram_init(void)
{
return fdtdec_setup_mem_size_base();
@@ -48,6 +38,14 @@ int dram_init_banksize(void)
}
#if defined(CONFIG_OF_BOARD_SETUP)
+static void apply_atf_overlay(void *fdt_blob)
+{
+ void *atf_fdt_blob = (void *)(rcar_atf_boot_args[1]);
+
+ if (fdt_magic(atf_fdt_blob) == FDT_MAGIC)
+ fdt_overlay_apply_node(fdt_blob, 0, atf_fdt_blob, 0);
+}
+
static int is_mem_overlap(void *blob, int first_mem_node, int curr_mem_node)
{
struct fdt_resource first_mem_res, curr_mem_res;
@@ -159,6 +157,7 @@ static void update_rpc_status(void *blob)
int ft_board_setup(void *blob, struct bd_info *bd)
{
+ apply_atf_overlay(blob);
scrub_duplicate_memory(blob);
update_rpc_status(blob);
--
2.38.1
Re: [PATCH] renesas: rcar: Apply ATF overlay for reserved-memory
Hi Detlev,
On Tue, Dec 6, 2022 at 4:35 PM Detlev Casanova
wrote:
> The function fdtdec_board_setup() is only called by fdtdec_setup() which
> needs to be called by the board file.
>
> This is not the case for the renesas boards so rename the
> fdtdec_board_setup() function to a local name and call it directly from
> ft_board_setup(), before cleaning up the memory nodes.
>
> Signed-off-by: Detlev Casanova
Thanks for your patch!
> --- a/board/renesas/rcar-common/common.c
> +++ b/board/renesas/rcar-common/common.c
> @@ -25,14 +25,12 @@ extern u64 rcar_atf_boot_args[];
>
> #define FDT_RPC_PATH "/soc/spi@ee20"
>
> -int fdtdec_board_setup(const void *fdt_blob)
> +void apply_atf_overlay(void *fdt_blob)
static?
> {
> void *atf_fdt_blob = (void *)(rcar_atf_boot_args[1]);
>
> if (fdt_magic(atf_fdt_blob) == FDT_MAGIC)
> - fdt_overlay_apply_node((void *)fdt_blob, 0, atf_fdt_blob, 0);
> -
> - return 0;
> + fdt_overlay_apply_node(fdt_blob, 0, atf_fdt_blob, 0);
> }
>
> int dram_init(void)
> @@ -159,6 +157,7 @@ static void update_rpc_status(void *blob)
>
> int ft_board_setup(void *blob, struct bd_info *bd)
> {
> + apply_atf_overlay(blob);
> scrub_duplicate_memory(blob);
> update_rpc_status(blob);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
[PATCH] renesas: rcar: Apply ATF overlay for reserved-memory
The function fdtdec_board_setup() is only called by fdtdec_setup() which
needs to be called by the board file.
This is not the case for the renesas boards so rename the
fdtdec_board_setup() function to a local name and call it directly from
ft_board_setup(), before cleaning up the memory nodes.
Signed-off-by: Detlev Casanova
---
board/renesas/rcar-common/common.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/board/renesas/rcar-common/common.c
b/board/renesas/rcar-common/common.c
index daa1beb14f..6eaa0d1a65 100644
--- a/board/renesas/rcar-common/common.c
+++ b/board/renesas/rcar-common/common.c
@@ -25,14 +25,12 @@ extern u64 rcar_atf_boot_args[];
#define FDT_RPC_PATH "/soc/spi@ee20"
-int fdtdec_board_setup(const void *fdt_blob)
+void apply_atf_overlay(void *fdt_blob)
{
void *atf_fdt_blob = (void *)(rcar_atf_boot_args[1]);
if (fdt_magic(atf_fdt_blob) == FDT_MAGIC)
- fdt_overlay_apply_node((void *)fdt_blob, 0, atf_fdt_blob, 0);
-
- return 0;
+ fdt_overlay_apply_node(fdt_blob, 0, atf_fdt_blob, 0);
}
int dram_init(void)
@@ -159,6 +157,7 @@ static void update_rpc_status(void *blob)
int ft_board_setup(void *blob, struct bd_info *bd)
{
+ apply_atf_overlay(blob);
scrub_duplicate_memory(blob);
update_rpc_status(blob);
--
2.38.1
Re: [PATCH v5 3/3] board: qemu-riscv: enable semihosting
On 12/6/22 00:42, Kautuk Consul wrote: > Hi, > > On Mon, Dec 5, 2022 at 8:46 PM Sean Anderson wrote: >> >> On 12/5/22 00:51, Kautuk Consul wrote: >> > Hi, >> > >> > On Sat, Dec 3, 2022 at 9:44 AM Bin Meng wrote: >> >> >> >> On Fri, Sep 23, 2022 at 3:03 PM Kautuk Consul >> >> wrote: >> >> > >> >> > To enable semihosting we also need to enable the following >> >> > configs in defconfigs: >> >> > CONFIG_SEMIHOSTING >> >> > CONFIG_SPL_SEMIHOSTING >> >> > CONFIG_SEMIHOSTING_SERIAL >> >> > CONFIG_SERIAL_PROBE_ALL >> >> > CONFIG_SPL_FS_EXT4 >> >> > CONFIG_SPL_FS_FAT >> >> >> >> Why should these _SPL_FS_xxx be required? If it's required by >> >> SEMIHOSTING, could the dependency be fixed there? >> > >> > The build dependencies require that these options be there. >> >> What error do you get? > > If I disable both the _SPL_FS_* config options then I get the > following compilation error: > common/spl/spl_semihosting.c: In function 'spl_smh_load_image': > common/spl/spl_semihosting.c:27:32: error: > 'CONFIG_SPL_FS_LOAD_PAYLOAD_NAME' undeclared (first use in this > function) >27 | const char *filename = CONFIG_SPL_FS_LOAD_PAYLOAD_NAME; > |^~~ > common/spl/spl_semihosting.c:27:32: note: each undeclared identifier > is reported only once for each function it appears in > > Bin/Sean: This error is not really related to the semihosting feature > but is related to COFIG_SPL in general. > Can you please accept this patch-set and then I'll try and find time > in the future maybe to rectify this build dependency > problem ? config SPL_FS_LOAD_PAYLOAD_NAME string "File to load for U-Boot from the filesystem" depends on SPL_FS_EXT4 || SPL_FS_FAT || SPL_FS_SQUASHFS default "tispl.bin" if SYS_K3_SPL_ATF default "u-boot.itb" if SPL_LOAD_FIT default "u-boot.img" help Filename to read to load U-Boot when reading from filesystem. Add CONFIG_SPL_SEMIHOSTING to the depends. --Sean >> >> --Sean >> >> >> >> >> > >> >> > Signed-off-by: Kautuk Consul >> >> > --- >> >> > configs/qemu-riscv32_defconfig | 4 >> >> > configs/qemu-riscv32_smode_defconfig | 4 >> >> > configs/qemu-riscv32_spl_defconfig | 7 +++ >> >> > configs/qemu-riscv64_defconfig | 4 >> >> > configs/qemu-riscv64_smode_defconfig | 4 >> >> > configs/qemu-riscv64_spl_defconfig | 7 +++ >> >> > 6 files changed, 30 insertions(+) >> >> > >> >> >> >> Regards, >> >> Bin >>
Re: [PATCH] Makefile: With BINMAN_ALLOW_MISSING=1 don't error on missing
On Tue, Dec 06, 2022 at 03:36:49PM +1300, Simon Glass wrote:
> Hi Tom,
>
> On Tue, 6 Dec 2022 at 15:03, Tom Rini wrote:
> >
> > When the user builds with BINMAN_ALLOW_MISSING=1 they're explicitly
> > setting the flag to allow for additional binaries to be missing and so
> > have acknowledged the output might not work. In this case we want to
> > default to not passing a non-zero exit code.
> >
> > Cc: Simon Glass
> > Reported-by: Peter Robinson
> > Signed-off-by: Tom Rini
> > ---
> > This passes CI as-is:
> > https://source.denx.de/u-boot/u-boot/-/pipelines/14340
> > ---
> > Makefile | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Makefile b/Makefile
> > index de5746399a63..03de1da1bfd0 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -1334,7 +1334,7 @@ cmd_binman = $(srctree)/tools/binman/binman $(if
> > $(BINMAN_DEBUG),-D) \
> > --toolpath $(objtree)/tools \
> > $(if $(BINMAN_VERBOSE),-v$(BINMAN_VERBOSE)) \
> > build -u -d u-boot.dtb -O . -m \
> > - $(if $(BINMAN_ALLOW_MISSING),--allow-missing
> > --fake-ext-blobs) \
> > + $(if $(BINMAN_ALLOW_MISSING),--allow-missing
> > --ignore-missing) \
> > -I . -I $(srctree) -I $(srctree)/board/$(BOARDDIR) \
> > -I arch/$(ARCH)/dts -a of-list=$(CONFIG_OF_LIST) \
> > $(foreach f,$(BINMAN_INDIRS),-I $(f)) \
> > --
> > 2.25.1
> >
>
> I believe we need the --fake-ext-blobs flag as well, since otherwise
> boards which use tools (like mkimage) on things that don't exist will
> not work.
So, we need to list out all the use cases perhaps, as we had missed one
before. In my mind we have:
- Board requires 1 or more blobs, developer will be running this on
hardware, expects output from U-Boot 'make' (or buildman) to boot.
Blobs must be present or non-zero exist status by default.
We didn't have this before, and we do now.
- Board requires 1 or more blobs AND has optional blobs, will be running
this on hardware, expects output from U-Boot 'make' (or buildman) to
boot. This is the case we had missed before as allwinner requires bl31
and has optional PMIC firmware. This is the case Peter reported and we
had missed before, which this patch allows for, with the caveat that
if you forget BL31 you're not going to boot and make won't complain
exit-code wise. Another way to resolve this would be a property in the
binman node to mark a blob as optional and warn if missing, rather
than error if missing.
- Board requires 1 or more blobs, output will NOT be run. This is the CI
case and the compile testing lots of board cases. This is what CI
passes still, with the above.
- Board requires 1 or more blobs, developer will be running this on
hardware, BUT will be injecting the blobs later on. I think this is
the use case you're talking about?
> Yes I know this passes on CI, but it will cause breakages when people
> use mkimage or other things which have missing inputs. It will be
> quite confusing too!
>
> As to the logic, I thought you had wanted the build to fail if there
> are missing blobs, regardless of whether they were allowed or not.
> There is currently code in buildman to detect this failure and either
> report it or ignore it:
>
> if (result.return_code == 2 and
> ('Some images are invalid' in result.stderr)):
> # This is handled later by the check for output in
> # stderr
> result.return_code = 0
>
> Since buildman sets BINMAN_ALLOW_MISSING=1 if -M is given, we will not
> be able to detect missing binaries at all when built from buildman. I
> think a suitable fix would be to update the code above to detect the
> 'Some images are invalid' regardless of the return code.
Well, what we designed here first missed how the allwinner case is used
by Fedora. That needs both a zero exit code and allowing what turns out
to be an optional blob to be missing.
--
Tom
signature.asc
Description: PGP signature
[tom.r...@gmail.com: Fwd: New Defects reported by Coverity Scan for Das U-Boot]
Here's the latest report
-- Forwarded message -
From:
Date: Mon, Dec 5, 2022, 3:35 PM
Subject: New Defects reported by Coverity Scan for Das U-Boot
To:
Hi,
Please find the latest report on new defect(s) introduced to Das U-Boot
found with Coverity Scan.
4 new defect(s) introduced to Das U-Boot found with Coverity Scan.
1 defect(s), reported by Coverity Scan earlier, were marked fixed in the
recent build analyzed by Coverity Scan.
New defect(s) Reported-by: Coverity Scan
Showing 4 of 4 defect(s)
** CID 430977: Null pointer dereferences (FORWARD_NULL)
/net/ndisc.c: 268 in ndisc_receive()
*** CID 430977: Null pointer dereferences (FORWARD_NULL)
/net/ndisc.c: 268 in ndisc_receive()
262 sizeof(struct in6_addr)) == 0) &&
263 ndisc_has_option(ip6, ND_OPT_TARGET_LL_ADDR)) {
264 ndisc_extract_enetaddr(ndisc,
neigh_eth_addr);
265
266 /* save address for later use */
267 if (!net_nd_packet_mac)
>>> CID 430977: Null pointer dereferences (FORWARD_NULL)
>>> Passing null pointer "net_nd_packet_mac" to "memcpy", which
dereferences it. [Note: The source code implementation of the function has
been overridden by a builtin model.]
268 memcpy(net_nd_packet_mac,
neigh_eth_addr, 7);
269
270 /* modify header, and transmit it */
271 memcpy(((struct ethernet_hdr
*)net_nd_tx_packet)->et_dest,
272neigh_eth_addr, 6);
273
** CID 430976: Control flow issues (DEADCODE)
/net/tftp.c: 744 in sanitize_tftp_block_size_option()
*** CID 430976: Control flow issues (DEADCODE)
/net/tftp.c: 744 in sanitize_tftp_block_size_option()
738 }
739 /*
740 * If not CONFIG_IP_DEFRAG, cap at the same value as
741 * for tftp put, namely normal MTU minus protocol
742 * overhead.
743 */
>>> CID 430976: Control flow issues (DEADCODE)
>>> Execution cannot reach this statement: "[[fallthrough]];".
744 fallthrough;
745 case TFTPPUT:
746 default:
747 /*
748 * U-Boot does not support IP fragmentation on TX,
so
749 * this must be small enough that it fits normal MTU
** CID 430975: Control flow issues (MISSING_BREAK)
/net/net.c: 1270 in net_process_received_packet()
*** CID 430975: Control flow issues (MISSING_BREAK)
/net/net.c: 1270 in net_process_received_packet()
1264 #ifdef CONFIG_CMD_RARP
1265case PROT_RARP:
1266rarp_receive(ip, len);
1267break;
1268 #endif
1269 #if IS_ENABLED(CONFIG_IPV6)
>>> CID 430975: Control flow issues (MISSING_BREAK)
>>> The case for value "34525" is not terminated by a "break" statement.
1270case PROT_IP6:
1271net_ip6_handler(et, (struct ip6_hdr *)ip, len);
1272 #endif
1273case PROT_IP:
1274debug_cond(DEBUG_NET_PKT, "Got IP\n");
1275/* Before we start poking the header, make sure it
is there */
** CID 430974: Memory - corruptions (OVERRUN)
/net/ndisc.c: 268 in ndisc_receive()
*** CID 430974: Memory - corruptions (OVERRUN)
/net/ndisc.c: 268 in ndisc_receive()
262 sizeof(struct in6_addr)) == 0) &&
263 ndisc_has_option(ip6, ND_OPT_TARGET_LL_ADDR)) {
264 ndisc_extract_enetaddr(ndisc,
neigh_eth_addr);
265
266 /* save address for later use */
267 if (!net_nd_packet_mac)
>>> CID 430974: Memory - corruptions (OVERRUN)
>>> Overrunning array "neigh_eth_addr" of 6 bytes by passing it to a
function which accesses it at byte offset 6 using argument "7UL". [Note:
The source code implementation of the function has been overridden by a
builtin model.]
268 memcpy(net_nd_packet_mac,
neigh_eth_addr, 7);
269
270 /* modify header, and transmit it */
271 memcpy(((struct ethernet_hdr
*)net_nd_tx_packet)->et_dest,
272neigh_eth_addr, 6);
273
--
Tom
signature.asc
Description: PGP signature
[PATCH] cmd: spi: Judge the number of added parameters
When only sspi is entered, help information can be printed.
Signed-off-by: chenzhipeng
---
cmd/spi.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/cmd/spi.c b/cmd/spi.c
index 454ebe37d7..f30018f33b 100644
--- a/cmd/spi.c
+++ b/cmd/spi.c
@@ -112,6 +112,9 @@ int do_spi(struct cmd_tbl *cmdtp, int flag, int argc, char
*const argv[])
if ((flag & CMD_FLAG_REPEAT) == 0)
{
+ if (argc < 2)
+ return CMD_RET_USAGE;
+
if (argc >= 2) {
mode = CONFIG_DEFAULT_SPI_MODE;
bus = dectoul(argv[1], );
--
2.25.1
[PATCH] lib: rsa: cosmetic: fix building warning
add initialization of variable 'node',this can aviod the building warning: 'node' may be used uninitialized [-Wmaybe-uninitialized] Signed-off-by: Haijun Qin --- lib/rsa/rsa-sign.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/rsa/rsa-sign.c b/lib/rsa/rsa-sign.c index b2a21199e4..d20bdb58a5 100644 --- a/lib/rsa/rsa-sign.c +++ b/lib/rsa/rsa-sign.c @@ -608,7 +608,7 @@ int rsa_add_verify_data(struct image_sign_info *info, void *keydest) BIGNUM *modulus, *r_squared; uint64_t exponent; uint32_t n0_inv; - int parent, node; + int parent, node = -FDT_ERR_NOTFOUND; char name[100]; int ret; int bits; base-commit: d2c5607edde2544e059fa871927877213f6bd532 -- 2.17.1
[PATCH] timer: Kconfig: add clint timer support
add the clint timer device configuration item to Kconfig Signed-off-by: chenzhipeng --- drivers/timer/Kconfig | 7 +++ 1 file changed, 7 insertions(+) diff --git a/drivers/timer/Kconfig b/drivers/timer/Kconfig index 6d6665005c..3dd1b940f1 100644 --- a/drivers/timer/Kconfig +++ b/drivers/timer/Kconfig @@ -216,6 +216,13 @@ config RISCV_TIMER Select this to enable support for a generic RISC-V S-Mode timer driver. +config CLINT_TIMER + bool "RISC-V clint timer support" + depends on TIMER && RISCV + help + Select this to enables the clint timer for RISC-V systems. The clint + driver is usually used for NoMMU RISC-V systems. + config ROCKCHIP_TIMER bool "Rockchip timer support" depends on TIMER -- 2.25.1
Re: [PATCH v3 4/5] eficonfig: use efi_get_next_variable_name_int()
On Sat, Dec 03, 2022 at 09:56:20AM +0900, Masahisa Kojima wrote:
> On Fri, 2 Dec 2022 at 16:36, Ilias Apalodimas
> wrote:
> >
> > On Fri, Dec 02, 2022 at 01:59:36PM +0900, Masahisa Kojima wrote:
> > > eficonfig command reads all possible UEFI load options
> > > from 0x to 0x to construct the menu. This takes too much
> > > time in some environment.
> > > This commit uses efi_get_next_variable_name_int() to read all
> > > existing UEFI load options to significantlly reduce the count of
> > > efi_get_var() call.
> > >
> > > Signed-off-by: Masahisa Kojima
> > > ---
> > > No update since v2
> > >
> > > v1->v2:
> > > - totaly change the implemention, remove new Kconfig introduced in v1.
> > > - use efi_get_next_variable_name_int() to read the all existing
> > > UEFI variables, then enumerate the "Boot" variables
> > > - this commit does not provide the common function to enumerate
> > > all "Boot" variables. I think common function does not
> > > simplify the implementation, because caller of
> > > efi_get_next_variable_name_int()
> > > needs to remember original buffer size, new buffer size and buffer
> > > address
> > > and it is a bit complicated when we consider to create common function.
> > >
> > > cmd/eficonfig.c | 141 +++-
> > > 1 file changed, 117 insertions(+), 24 deletions(-)
> > >
> > > diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
> > > index 88d507d04c..394ae67cce 100644
> > > --- a/cmd/eficonfig.c
> > > +++ b/cmd/eficonfig.c
> > > @@ -1683,7 +1683,8 @@ static efi_status_t
> > > eficonfig_show_boot_selection(unsigned int *selected)
> > > u32 i;
> > > u16 *bootorder;
> > > efi_status_t ret;
> > > - efi_uintn_t num, size;
> > > + u16 *var_name16 = NULL, *p;
> > > + efi_uintn_t num, size, buf_size;
> > > struct efimenu *efi_menu;
> > > struct list_head *pos, *n;
> > > struct eficonfig_entry *entry;
> > > @@ -1707,14 +1708,43 @@ static efi_status_t
> > > eficonfig_show_boot_selection(unsigned int *selected)
> > > }
> > >
> > > /* list the remaining load option not included in the BootOrder */
> > > - for (i = 0; i <= 0x; i++) {
> > > - /* If the index is included in the BootOrder, skip it */
> > > - if (search_bootorder(bootorder, num, i, NULL))
> > > - continue;
> > > + buf_size = 128;
> > > + var_name16 = malloc(buf_size);
> > > + if (!var_name16)
> > > + return EFI_OUT_OF_RESOURCES;
> > >
> > > - ret = eficonfig_add_boot_selection_entry(efi_menu, i,
> > > selected);
> > > - if (ret != EFI_SUCCESS)
> > > - goto out;
> > > + var_name16[0] = 0;
> >
> > Is it worth using calloc instead of malloc and get rid of this?
> >
> > > + for (;;) {
> > > + int index;
> > > + efi_guid_t guid;
> > > +
> > > + size = buf_size;
> > > + ret = efi_get_next_variable_name_int(, var_name16,
> > > );
> > > + if (ret == EFI_NOT_FOUND)
> > > + break;
> > > + if (ret == EFI_BUFFER_TOO_SMALL) {
> > > + buf_size = size;
> > > + p = realloc(var_name16, buf_size);
> > > + if (!p) {
> > > + free(var_name16);
> > > + return EFI_OUT_OF_RESOURCES;
> > > + }
> > > + var_name16 = p;
> > > + ret = efi_get_next_variable_name_int(,
> > > var_name16, );
> > > + }
> > > + if (ret != EFI_SUCCESS) {
> > > + free(var_name16);
> > > + return ret;
> > > + }
> > > + if (efi_varname_is_load_option(var_name16, )) {
> > > + /* If the index is included in the BootOrder, skip
> > > it */
> > > + if (search_bootorder(bootorder, num, index, NULL))
> > > + continue;
> > > +
> > > + ret = eficonfig_add_boot_selection_entry(efi_menu,
> > > index, selected);
> > > + if (ret != EFI_SUCCESS)
> > > + goto out;
> > > + }
> > >
> > > if (efi_menu->count >= EFICONFIG_ENTRY_NUM_MAX - 1)
> > > break;
> > > @@ -1732,6 +1762,8 @@ out:
> > > }
> > > eficonfig_destroy(efi_menu);
> > >
> > > + free(var_name16);
> > > +
> > > return ret;
> > > }
> > >
> > > @@ -1994,6 +2026,8 @@ static efi_status_t
> > > eficonfig_create_change_boot_order_entry(struct efimenu *efi
> > > u32 i;
> > > char *title;
> > > efi_status_t ret;
> > > + u16 *var_name16 = NULL, *p;
> > > + efi_uintn_t size, buf_size;
> > >
> > > /* list the load option in the order of BootOrder variable */
> > > for (i = 0; i < num; i++) {
> > > @@
[PATCH] phy: rockchip: handle clock without enable function
If a clock doesn't supply the enable hook, clk_enable() will return
-ENOSYS. In this case the clock is always enabled so there is no error
and the phy initialisation should continue.
Signed-off-by: John Keeping
---
drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
index 62b8ba3a4a..b32a498ea7 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
@@ -119,7 +119,7 @@ static int rockchip_usb2phy_init(struct phy *phy)
int ret;
ret = clk_enable(>phyclk);
- if (ret) {
+ if (ret && ret != -ENOSYS) {
dev_err(phy->dev, "failed to enable phyclk (ret=%d)\n", ret);
return ret;
}
--
2.38.1
Re: [PATCH] net: ipv6: Fix link-partner MAC address assignment
On 12/6/22 08:08, Viacheslav Mitrofanov wrote: MAC address of a link-partner is not saved for future use because of bad condition of if statement. Moreover it can potentially cause to NULL-pointer dereference. Signed-off-by: Viacheslav Mitrofanov --- net/ndisc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Daniel Schwierzeck diff --git a/net/ndisc.c b/net/ndisc.c index 3c0eeeaea3..56fc6390bc 100644 --- a/net/ndisc.c +++ b/net/ndisc.c @@ -264,7 +264,7 @@ int ndisc_receive(struct ethernet_hdr *et, struct ip6_hdr *ip6, int len) ndisc_extract_enetaddr(ndisc, neigh_eth_addr); /* save address for later use */ - if (!net_nd_packet_mac) + if (net_nd_packet_mac) memcpy(net_nd_packet_mac, neigh_eth_addr, 7); /* modify header, and transmit it */ -- - Daniel
Re: [PATCH] net: ipv6: Add missing break into IPv6 protocol handler
On 12/6/22 08:08, Viacheslav Mitrofanov wrote: IPv6 protocol handler is not terminated with a break statment. It can lead to running unexpected code. Signed-off-by: Viacheslav Mitrofanov --- net/net.c | 1 + 1 file changed, 1 insertion(+) thanks for the quick fix Reviewed-by: Daniel Schwierzeck diff --git a/net/net.c b/net/net.c index 1c39acc493..57da9bda85 100644 --- a/net/net.c +++ b/net/net.c @@ -1269,6 +1269,7 @@ void net_process_received_packet(uchar *in_packet, int len) #if IS_ENABLED(CONFIG_IPV6) case PROT_IP6: net_ip6_handler(et, (struct ip6_hdr *)ip, len); + break; #endif case PROT_IP: debug_cond(DEBUG_NET_PKT, "Got IP\n"); -- - Daniel
Re: [PATCH 1/4] ARM: stm32: Fix ECDSA authentication with Dcache enabled
On 12/6/22 10:13, Patrick DELAUNAY wrote:
Hi Marek,
Hi,
[...]
@@ -81,8 +82,21 @@ static int romapi_ecdsa_verify(struct udevice *dev,
memcpy(raw_key + 32, pubkey->y, 32);
stm32mp_rom_get_ecdsa_functions();
+
+ /*
+ * Disable D-cache before calling into BootROM, else CRYP DMA
+ * may fail to pick up the correct data.
+ */
+ if (dcache_status()) {
+ dcache_disable();
+ reenable_dcache = true;
+ }
+
rom_ret = rom.ecdsa_verify_signature(hash, raw_key, signature,
algo);
so the signature verification (the code execution) is done with dcache
OFF
flush the input data should be enought for DMA operation ?
=> call flush_dcache_all() or flush_dcache_range()
for example:
if (dcache_status())
flush_dcache_all();
Wouldn't you then also need to invalidate the dcache after the BootROM
call, so that the CPU with dcache enabled could read what the CRYP wrote
to DRAM instead of pulling stale data from Dcache ?
That's very much what the enable/disable trick does for you.
Re: [PATCH v5 3/3] board: qemu-riscv: enable semihosting
Hi Leo, On Tue, Dec 6, 2022 at 4:29 PM Leo Liang wrote: > > Hi Kautuk, > > We have tested your patchset with QEMU 7.1.0. > It generally looks fine, but CI error seems to persist. > https://source.denx.de/u-boot/custodians/u-boot-riscv/-/pipelines/14314 > > The error comes from CI testcase timed-out. > The reason for the time-out is not yet confirmed, > but we suspect it's because when executing under semihosting, > QEMU could not exit normally. (thru ctrl+x a) > > There is a seemingly relevent patchset that sits on QEMU mailing list for > some time. > https://lore.kernel.org/all/20220620190834.ga16...@ws2101.lin.mbt.kalray.eu/T/#m1bc32cc32511b6ac8adfaf67983dc2bccd4b9ec9 > > On the u-boot side, what do you think if we disable semihosting by default? > (i.e., not adding CONFIG_SEMIHOSTING_XXX in qemu's defconfig) I think it is okay to disable semihosting by default. Then the user will configure it when needed. So then can you ACK the first 2 patches ? I think we can leave out the 3rd qemu config patch for now. > > Best regards, > Leo > > On Tue, Dec 06, 2022 at 11:12:41AM +0530, Kautuk Consul wrote: > > Hi, > > > > On Mon, Dec 5, 2022 at 8:46 PM Sean Anderson wrote: > > > > > > On 12/5/22 00:51, Kautuk Consul wrote: > > > > Hi, > > > > > > > > On Sat, Dec 3, 2022 at 9:44 AM Bin Meng wrote: > > > >> > > > >> On Fri, Sep 23, 2022 at 3:03 PM Kautuk Consul > > > >> wrote: > > > >> > > > > >> > To enable semihosting we also need to enable the following > > > >> > configs in defconfigs: > > > >> > CONFIG_SEMIHOSTING > > > >> > CONFIG_SPL_SEMIHOSTING > > > >> > CONFIG_SEMIHOSTING_SERIAL > > > >> > CONFIG_SERIAL_PROBE_ALL > > > >> > CONFIG_SPL_FS_EXT4 > > > >> > CONFIG_SPL_FS_FAT > > > >> > > > >> Why should these _SPL_FS_xxx be required? If it's required by > > > >> SEMIHOSTING, could the dependency be fixed there? > > > > > > > > The build dependencies require that these options be there. > > > > > > What error do you get? > > > > If I disable both the _SPL_FS_* config options then I get the > > following compilation error: > > common/spl/spl_semihosting.c: In function 'spl_smh_load_image': > > common/spl/spl_semihosting.c:27:32: error: > > 'CONFIG_SPL_FS_LOAD_PAYLOAD_NAME' undeclared (first use in this > > function) > >27 | const char *filename = CONFIG_SPL_FS_LOAD_PAYLOAD_NAME; > > |^~~ > > common/spl/spl_semihosting.c:27:32: note: each undeclared identifier > > is reported only once for each function it appears in > > > > Bin/Sean: This error is not really related to the semihosting feature > > but is related to COFIG_SPL in general. > > Can you please accept this patch-set and then I'll try and find time > > in the future maybe to rectify this build dependency > > problem ? > > > > > > > > --Sean > > > > > > >> > > > >> > > > > >> > Signed-off-by: Kautuk Consul > > > >> > --- > > > >> > configs/qemu-riscv32_defconfig | 4 > > > >> > configs/qemu-riscv32_smode_defconfig | 4 > > > >> > configs/qemu-riscv32_spl_defconfig | 7 +++ > > > >> > configs/qemu-riscv64_defconfig | 4 > > > >> > configs/qemu-riscv64_smode_defconfig | 4 > > > >> > configs/qemu-riscv64_spl_defconfig | 7 +++ > > > >> > 6 files changed, 30 insertions(+) > > > >> > > > > >> > > > >> Regards, > > > >> Bin > > >
Re: [PATCH v5 3/3] board: qemu-riscv: enable semihosting
Hi Kautuk, We have tested your patchset with QEMU 7.1.0. It generally looks fine, but CI error seems to persist. https://source.denx.de/u-boot/custodians/u-boot-riscv/-/pipelines/14314 The error comes from CI testcase timed-out. The reason for the time-out is not yet confirmed, but we suspect it's because when executing under semihosting, QEMU could not exit normally. (thru ctrl+x a) There is a seemingly relevent patchset that sits on QEMU mailing list for some time. https://lore.kernel.org/all/20220620190834.ga16...@ws2101.lin.mbt.kalray.eu/T/#m1bc32cc32511b6ac8adfaf67983dc2bccd4b9ec9 On the u-boot side, what do you think if we disable semihosting by default? (i.e., not adding CONFIG_SEMIHOSTING_XXX in qemu's defconfig) Best regards, Leo On Tue, Dec 06, 2022 at 11:12:41AM +0530, Kautuk Consul wrote: > Hi, > > On Mon, Dec 5, 2022 at 8:46 PM Sean Anderson wrote: > > > > On 12/5/22 00:51, Kautuk Consul wrote: > > > Hi, > > > > > > On Sat, Dec 3, 2022 at 9:44 AM Bin Meng wrote: > > >> > > >> On Fri, Sep 23, 2022 at 3:03 PM Kautuk Consul > > >> wrote: > > >> > > > >> > To enable semihosting we also need to enable the following > > >> > configs in defconfigs: > > >> > CONFIG_SEMIHOSTING > > >> > CONFIG_SPL_SEMIHOSTING > > >> > CONFIG_SEMIHOSTING_SERIAL > > >> > CONFIG_SERIAL_PROBE_ALL > > >> > CONFIG_SPL_FS_EXT4 > > >> > CONFIG_SPL_FS_FAT > > >> > > >> Why should these _SPL_FS_xxx be required? If it's required by > > >> SEMIHOSTING, could the dependency be fixed there? > > > > > > The build dependencies require that these options be there. > > > > What error do you get? > > If I disable both the _SPL_FS_* config options then I get the > following compilation error: > common/spl/spl_semihosting.c: In function 'spl_smh_load_image': > common/spl/spl_semihosting.c:27:32: error: > 'CONFIG_SPL_FS_LOAD_PAYLOAD_NAME' undeclared (first use in this > function) >27 | const char *filename = CONFIG_SPL_FS_LOAD_PAYLOAD_NAME; > |^~~ > common/spl/spl_semihosting.c:27:32: note: each undeclared identifier > is reported only once for each function it appears in > > Bin/Sean: This error is not really related to the semihosting feature > but is related to COFIG_SPL in general. > Can you please accept this patch-set and then I'll try and find time > in the future maybe to rectify this build dependency > problem ? > > > > > --Sean > > > > >> > > >> > > > >> > Signed-off-by: Kautuk Consul > > >> > --- > > >> > configs/qemu-riscv32_defconfig | 4 > > >> > configs/qemu-riscv32_smode_defconfig | 4 > > >> > configs/qemu-riscv32_spl_defconfig | 7 +++ > > >> > configs/qemu-riscv64_defconfig | 4 > > >> > configs/qemu-riscv64_smode_defconfig | 4 > > >> > configs/qemu-riscv64_spl_defconfig | 7 +++ > > >> > 6 files changed, 30 insertions(+) > > >> > > > >> > > >> Regards, > > >> Bin > >
Re: PSU initialization code on zynqmp platforms
Hi, On 12/6/22 02:01, Graeme Smecher wrote: Hi Michal, (Well, that's embarassing. Sending again with a useful subject.) I'm bringing up u-boot on a custom zynqmp platform. For the PSU initialization, I can start with the psu_init_gpl.[ch] files embedded in the .xsa generated by Vivado. However, these are pretty flawed [1, 2]. What is your workflow for generating and maintaining the in-tree psu_init_gpl files you maintain? Are these files cleaned up and maintained by hand? If your workflow is functional, I will happily emulate it instead - even if it means straying from Vivado's configuration GUI. I am just copy psu init files from xsa and copy it somewhere and then run ./tools/zynqmp_psu_init_minimize.sh to minimize it. And then commit and small manual cleanups to resolved issues reported by checkpatch. Thanks, Michal
Re: [PATCH v2] fastboot: Add OEM run command
Hi, On 12/5/22 20:15, Sean Anderson wrote: On 12/5/22 14:04, Patrick DELAUNAY wrote: Hi, On 12/2/22 22:03, Sean Anderson wrote: This adds the UUU UCmd functionality as an OEM command. While the fastboot tool allows sending arbitrary commands as long as they are prefixed with "oem". This allows running generic U-Boot commands over fastboot without UUU, which is especially useful when not using USB. This is really the route we should have gone in the first place when adding these commands. While we're here, clean up the Kconfig a bit. Signed-off-by: Sean Anderson --- Changes in v2: - Document usage - Keep enum in order doc/android/fastboot.rst | 15 +++ drivers/fastboot/Kconfig | 10 +- drivers/fastboot/fb_command.c | 4 include/fastboot.h | 1 + 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/doc/android/fastboot.rst b/doc/android/fastboot.rst index 7611f07038..7f343d5dfc 100644 --- a/doc/android/fastboot.rst +++ b/doc/android/fastboot.rst @@ -28,6 +28,7 @@ The following OEM commands are supported (if enabled): - ``oem partconf`` - this executes ``mmc partconf %x 0`` to configure eMMC with = boot_ack boot_partition - ``oem bootbus`` - this executes ``mmc bootbus %x %s`` to configure eMMC +- ``oem run`` - this executes an arbitrary U-Boot command Support for both eMMC and NAND devices is included. @@ -127,6 +128,19 @@ Boot command When executing the fastboot ``boot`` command, if ``fastboot_bootcmd`` is set then that will be executed in place of ``bootm ``. +Running Shell Commands +-- + +Normally, arbitrary U-Boot command execution is not enabled. This is so +fastboot can be used to update systems using verified boot. However, such +functionality can be useful for production or when verified boot is not in use. +Enable ``CONFIG_FASTBOOT_UUU_SUPPORT`` to use this functionality. This will +enable the ``UCmd`` and ``ACmd`` commands for use with UUU [3]_. It also +enables the ``oem run`` command, which can be used with the fastboot client. +For example, to print "Hello world", run:: + + $ fastboot oem run:echo Hello world + Partition Names --- @@ -232,3 +246,4 @@ References .. [1] :doc:`fastboot-protocol` .. [2] https://developer.android.com/studio/releases/platform-tools +.. [3] https://github.com/NXPmicro/mfgtools diff --git a/drivers/fastboot/Kconfig b/drivers/fastboot/Kconfig index b97c67bf60..8f2d52cb8a 100644 --- a/drivers/fastboot/Kconfig +++ b/drivers/fastboot/Kconfig @@ -80,12 +80,12 @@ config FASTBOOT_FLASH this to enable the "fastboot flash" command. config FASTBOOT_UUU_SUPPORT - bool "Enable FASTBOOT i.MX UUU special command" + bool "Enable running arbitrary commands from FASTBOOT" help - The fastboot protocol includes "UCmd" and "ACmd" command. - Be aware that you provide full access to any U-Boot command, - including working with memory and may open a huge backdoor, - when enabling this option. + This extends the fastboot protocol with the "UCmd" and "ACmd" + commands, as well as the "oem run" command. These commands provide + full access to any U-Boot command, including working with memory. + This may open a huge backdoor if you are using verified boot. why the support of "oem run" generic support is include in the IMX config CONFIG_FASTBOOT_UUU_SUPPORT Because they are effectively the same command, but named differently. oem run is just an alias for UCmd which can be used by the fastboot Linux command without modification. "UCmd" and "Acmd" are imx additions in fastboot standard for UUU support https://android.googlesource.com/platform/system/core/+/2ec418a4c98f6e8f95395456e1ad4c2956cac007/fastboot/fastboot_protocol.txt These commands are absent from that standard (and thus are non-standard). Yes, they are 'non-standard', but the command prefixed by 'oem' are allowed by the fastboot specification. So all the "oem" commands can be used with the official Android tool... $> fastboot --help oem ... Executes oem specific command. => they are not reserved to IMX platforms the added command UCmd and Acmd can be only used with UUU imx tools. but "oem run" can be see as generic U-Boot 'oem' command to execute U-boot command. All of these commands are effectively non-standard, although "oem" is a common prefix. for me I see 2 separate configs to handle other platform than IMX ones (with UUU support) config FASTBOOT_CMD_OEM_RUN bool "Enable fastboot oem run command" config FASTBOOT_UUU_SUPPORT bool "Enable FASTBOOT i.MX UUU special command" select FASTBOOT_CMD_OEM_RUN PS: FASTBOOT_UUU_SUPPORT could depends on CONFIG_ARCH_IMX* Maybe we can have something like config FASTBOOT_ARBITRARY_EXECUTION which UUU_SUPPORT could depend on. whatever the Kconfig logic, the "fastboot oem run" should be
Re: [PATCH 04/41] topic_miami*: Disable networking support more fully
Looks fine to me, Acked-by: Mike Looijmans (PS: Sorry for top-posting, but otherwise our M$ mailserver will mangle it) Met vriendelijke groet / kind regards, Mike Looijmans System Expert TOPIC Embedded Products B.V. Materiaalweg 4, 5681 RJ Best The Netherlands T: +31 (0) 499 33 69 69 E: mike.looijm...@topicproducts.com W: www.topic.nl Please consider the environment before printing this e-mail On 27-11-2022 16:24, Tom Rini wrote: This platform had largely disabled networking support before. More completely disable it by turning off CONFIG_NET. Cc: Mike Looijmans Signed-off-by: Tom Rini --- configs/topic_miami_defconfig | 2 +- configs/topic_miamilite_defconfig | 2 +- configs/topic_miamiplus_defconfig | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/configs/topic_miami_defconfig b/configs/topic_miami_defconfig index 96aa62a7ec12..ece625f62924 100644 --- a/configs/topic_miami_defconfig +++ b/configs/topic_miami_defconfig @@ -46,11 +46,11 @@ CONFIG_CMD_I2C=y CONFIG_CMD_MMC=y CONFIG_CMD_USB=y # CONFIG_CMD_SETEXPR is not set -# CONFIG_CMD_NET is not set CONFIG_CMD_CACHE=y CONFIG_OF_EMBED=y CONFIG_ENV_OVERWRITE=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y +# CONFIG_NET is not set CONFIG_SPL_DM_SEQ_ALIAS=y CONFIG_DFU_RAM=y CONFIG_SYS_DFU_DATA_BUF_SIZE=0x60 diff --git a/configs/topic_miamilite_defconfig b/configs/topic_miamilite_defconfig index 41ba8a7487d1..693a602ea395 100644 --- a/configs/topic_miamilite_defconfig +++ b/configs/topic_miamilite_defconfig @@ -46,11 +46,11 @@ CONFIG_CMD_I2C=y CONFIG_CMD_MMC=y CONFIG_CMD_USB=y # CONFIG_CMD_SETEXPR is not set -# CONFIG_CMD_NET is not set CONFIG_CMD_CACHE=y CONFIG_OF_EMBED=y CONFIG_ENV_OVERWRITE=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y +# CONFIG_NET is not set CONFIG_SPL_DM_SEQ_ALIAS=y CONFIG_DFU_RAM=y CONFIG_SYS_DFU_DATA_BUF_SIZE=0x60 diff --git a/configs/topic_miamiplus_defconfig b/configs/topic_miamiplus_defconfig index 763bd8cccdd3..97624e69e722 100644 --- a/configs/topic_miamiplus_defconfig +++ b/configs/topic_miamiplus_defconfig @@ -50,6 +50,7 @@ CONFIG_CMD_CACHE=y CONFIG_OF_EMBED=y CONFIG_ENV_OVERWRITE=y CONFIG_SYS_RELOC_GD_ENV_ADDR=y +# CONFIG_NET is not set CONFIG_SPL_DM_SEQ_ALIAS=y CONFIG_DFU_RAM=y CONFIG_SYS_DFU_DATA_BUF_SIZE=0x60 @@ -62,7 +63,6 @@ CONFIG_MMC_SDHCI_ZYNQ=y CONFIG_SF_DEFAULT_SPEED=10800 CONFIG_SPI_FLASH_STMICRO=y # CONFIG_SPI_FLASH_USE_4K_SECTORS is not set -# CONFIG_NETDEVICES is not set CONFIG_DEBUG_UART_ZYNQ=y CONFIG_ARM_DCC=y CONFIG_ZYNQ_SERIAL=y -- Mike Looijmans
[PATCH] rtc: add ht1380 driver
Support Holtek HT1380/HT1381 Serial Timekeeper Chip. It provides seconds
, minutes, hours, day of the week, date, month and year information.
Datasheet:
https://www.holtek.com.tw/documents/10179/11842/ht1380_1v130.pdf
Signed-off-by: Sergei Antonov
---
v2:
* The RESET pin is now to be described as ACTIVE_LOW in dts.
Changes suggested by Simon Glass:
* a more detailed driver description in Kconfig
* multi-line comments' style
* enum for 0x80 and the 0x20 at top of file
* lower-case hex constants
* function comments for ht1380_reset_on/off
* blank line before returns
PROTECT remains in a function scope for the sake of locality of definitions.
drivers/rtc/Kconfig | 8 +
drivers/rtc/Makefile | 1 +
drivers/rtc/ht1380.c | 337 +++
3 files changed, 346 insertions(+)
create mode 100644 drivers/rtc/ht1380.c
diff --git a/drivers/rtc/Kconfig b/drivers/rtc/Kconfig
index 23963271928a..eed48e35a578 100644
--- a/drivers/rtc/Kconfig
+++ b/drivers/rtc/Kconfig
@@ -220,4 +220,12 @@ config RTC_ZYNQMP
Say "yes" here to support the on chip real time clock
present on Xilinx ZynqMP SoC.
+config RTC_HT1380
+ bool "Enable Holtek HT1380/HT1381 RTC driver"
+ depends on DM_RTC && DM_GPIO
+ help
+ Say "yes" here to get support for Holtek HT1380/HT1381
+ Serial Timekeeper IC which provides seconds, minutes, hours,
+ day of the week, date, month and year information.
+
endmenu
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile
index 009dd9d28c95..f3164782b605 100644
--- a/drivers/rtc/Makefile
+++ b/drivers/rtc/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_RTC_DS3231) += ds3231.o
obj-$(CONFIG_RTC_DS3232) += ds3232.o
obj-$(CONFIG_RTC_EMULATION) += emul_rtc.o
obj-$(CONFIG_RTC_FTRTC010) += ftrtc010.o
+obj-$(CONFIG_RTC_HT1380) += ht1380.o
obj-$(CONFIG_SANDBOX) += i2c_rtc_emul.o
obj-$(CONFIG_RTC_IMXDI) += imxdi.o
obj-$(CONFIG_RTC_ISL1208) += isl1208.o
diff --git a/drivers/rtc/ht1380.c b/drivers/rtc/ht1380.c
new file mode 100644
index ..25335227d893
--- /dev/null
+++ b/drivers/rtc/ht1380.c
@@ -0,0 +1,337 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Holtek HT1380/HT1381 Serial Timekeeper Chip
+ *
+ * Communication with the chip is vendor-specific.
+ * It is done via 3 GPIO pins: reset, clock, and data.
+ * Describe in .dts this way:
+ *
+ * rtc {
+ * compatible = "holtek,ht1380";
+ * rst-gpio = < 19 GPIO_ACTIVE_LOW>;
+ * clk-gpio = < 20 GPIO_ACTIVE_HIGH>;
+ * dat-gpio = < 21 GPIO_ACTIVE_HIGH>;
+ * };
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct ht1380_priv {
+ struct gpio_desc rst_desc;
+ struct gpio_desc clk_desc;
+ struct gpio_desc dat_desc;
+};
+
+enum registers {
+ SEC,
+ MIN,
+ HOUR,
+ MDAY,
+ MONTH,
+ WDAY,
+ YEAR,
+ WP,
+ N_REGS
+};
+
+enum hour_mode {
+ AMPM_MODE = 0x80, /* RTC is in AM/PM mode */
+ PM_NOW = 0x20,/* set if PM, clear if AM */
+};
+
+static const int BURST = 0xbe;
+static const int READ = 1;
+
+static void ht1380_half_period_delay(void)
+{
+ /*
+* Delay for half a period. 1 us complies with the 500 KHz maximum
+* input serial clock limit given by the datasheet.
+*/
+ udelay(1);
+}
+
+static int ht1380_send_byte(struct ht1380_priv *priv, int byte)
+{
+ int ret;
+
+ for (int bit = 0; bit < 8; bit++) {
+ ret = dm_gpio_set_value(>dat_desc, byte >> bit & 1);
+ if (ret)
+ break;
+ ht1380_half_period_delay();
+
+ ret = dm_gpio_set_value(>clk_desc, 1);
+ if (ret)
+ break;
+ ht1380_half_period_delay();
+
+ ret = dm_gpio_set_value(>clk_desc, 0);
+ if (ret)
+ break;
+ }
+
+ return ret;
+}
+
+/*
+ * Leave reset state. The transfer operation can then be started.
+ */
+static int ht1380_reset_off(struct ht1380_priv *priv)
+{
+ const unsigned int T_CC = 4; /* us, Reset to Clock Setup */
+ int ret;
+
+ /*
+* Leave RESET state.
+* Make sure we make the minimal delay required by the datasheet.
+*/
+ ret = dm_gpio_set_value(>rst_desc, 0);
+ udelay(T_CC);
+
+ return ret;
+}
+
+/*
+ * Enter reset state. Completes the transfer operation.
+ */
+static int ht1380_reset_on(struct ht1380_priv *priv)
+{
+ const unsigned int T_CWH = 4; /* us, Reset Inactive Time */
+ int ret;
+
+ /*
+* Enter RESET state.
+* Make sure we make the minimal delay required by the datasheet.
+*/
+ ret = dm_gpio_set_value(>rst_desc, 1);
+ udelay(T_CWH);
+
+ return ret;
+}
+
+static int ht1380_rtc_get(struct udevice *dev, struct rtc_time *tm)
+{
+ struct ht1380_priv *priv = dev_get_priv(dev);
+ int ret, i, bit,
Re: [PATCH 3/3] ARM: stm32: Increment WDT by default on DHSOM
Hi, On 12/6/22 03:35, Marek Vasut wrote: Enable watchdog timer on the DHSOM by default, both in U-Boot proper and in SPL. This can be used in combination with boot counter by either SPL or U-Boot proper to boot either copy of system software, e.g. in case of full A/B update strategy. Signed-off-by: Marek Vasut --- Cc: Patrice Chotard Cc: Patrick Delaunay --- configs/stm32mp15_dhcom_basic_defconfig | 2 ++ configs/stm32mp15_dhcor_basic_defconfig | 2 ++ 2 files changed, 4 insertions(+) Reviewed-by: Patrick Delaunay Thanks Patrick
Re: [PATCH 2/3] ARM: stm32: Increment boot counter in SPL on DHSOM
Hi, On 12/6/22 03:35, Marek Vasut wrote: Increment the boot counter already in U-Boot SPL instead of incrementing it only later in U-Boot proper. This can be used by SPL to boot either of two U-Boot copies and improve redundancy of software on the platform, e.g. in case of full A/B update strategy. Signed-off-by: Marek Vasut --- Cc: Patrice Chotard Cc: Patrick Delaunay --- configs/stm32mp15_dhcom_basic_defconfig | 1 + configs/stm32mp15_dhcor_basic_defconfig | 1 + 2 files changed, 2 insertions(+) Reviewed-by: Patrick Delaunay Thanks Patrick
Re: [PATCH 1/3] ARM: stm32: Enable assorted ST specific commands on DHSOM
Hi, On 12/6/22 03:35, Marek Vasut wrote: Enable the stm32prog, stm32key, stboard commands on DHSOM. Those can be used e.g. to implement verified boot. Signed-off-by: Marek Vasut --- Cc: Patrice Chotard Cc: Patrick Delaunay --- configs/stm32mp15_dhcom_basic_defconfig | 4 configs/stm32mp15_dhcor_basic_defconfig | 4 2 files changed, 8 insertions(+) Reviewed-by: Patrick Delaunay Thanks Patrick
Re: [PATCH 1/4] ARM: stm32: Fix ECDSA authentication with Dcache enabled
Hi Marek,
On 12/6/22 03:33, Marek Vasut wrote:
In case Dcache is enabled while the ECDSA authentication function is
called via BootROM ROM API, the CRYP DMA might pick stale version of
data from DRAM. Disable Dcache around the BootROM call to avoid this
issue.
Signed-off-by: Marek Vasut
---
Cc: Alexandru Gagniuc
Cc: Patrice Chotard
Cc: Patrick Delaunay
---
arch/arm/mach-stm32mp/ecdsa_romapi.c | 14 ++
1 file changed, 14 insertions(+)
diff --git a/arch/arm/mach-stm32mp/ecdsa_romapi.c
b/arch/arm/mach-stm32mp/ecdsa_romapi.c
index a2f63ff879f..72b87bf2c64 100644
--- a/arch/arm/mach-stm32mp/ecdsa_romapi.c
+++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c
@@ -64,6 +64,7 @@ static int romapi_ecdsa_verify(struct udevice *dev,
const void *signature, size_t sig_len)
{
struct ecdsa_rom_api rom;
+ bool reenable_dcache;
uint8_t raw_key[64];
uint32_t rom_ret;
int algo;
@@ -81,8 +82,21 @@ static int romapi_ecdsa_verify(struct udevice *dev,
memcpy(raw_key + 32, pubkey->y, 32);
stm32mp_rom_get_ecdsa_functions();
+
+ /*
+* Disable D-cache before calling into BootROM, else CRYP DMA
+* may fail to pick up the correct data.
+*/
+ if (dcache_status()) {
+ dcache_disable();
+ reenable_dcache = true;
+ }
+
rom_ret = rom.ecdsa_verify_signature(hash, raw_key, signature, algo);
so the signature verification (the code execution) is done with dcache
OFF
flush the input data should be enought for DMA operation ?
=> call flush_dcache_all() or flush_dcache_range()
for example:
if (dcache_status())
flush_dcache_all();
+ if (reenable_dcache)
+ dcache_enable();
+
return rom_ret == ROM_API_SUCCESS ? 0 : -EPERM;
}
Regards
Patrick
Re: [PATCH 4/4] ARM: stm32: Make ECDSA authentication available to U-Boot
Hi, On 12/6/22 03:33, Marek Vasut wrote: With U-Boot having access to ROM API call table, it is possible to use the ROM API call it authenticate e.g. signed kernel fitImages using the BootROM ECDSA support. Make this available by pulling the ECDSA BootROM call support from SPL-only guard. Signed-off-by: Marek Vasut --- Cc: Alexandru Gagniuc Cc: Patrice Chotard Cc: Patrick Delaunay --- arch/arm/mach-stm32mp/Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Patrick Delaunay Thanks Patrick
Re: [PATCH 1/4] ARM: stm32: Fix ECDSA authentication with Dcache enabled
just one remark
On 12/6/22 03:33, Marek Vasut wrote:
> In case Dcache is enabled while the ECDSA authentication function is
> called via BootROM ROM API, the CRYP DMA might pick stale version of
> data from DRAM. Disable Dcache around the BootROM call to avoid this
> issue.
>
> Signed-off-by: Marek Vasut
> ---
> Cc: Alexandru Gagniuc
> Cc: Patrice Chotard
> Cc: Patrick Delaunay
> ---
> arch/arm/mach-stm32mp/ecdsa_romapi.c | 14 ++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm/mach-stm32mp/ecdsa_romapi.c
> b/arch/arm/mach-stm32mp/ecdsa_romapi.c
> index a2f63ff879f..72b87bf2c64 100644
> --- a/arch/arm/mach-stm32mp/ecdsa_romapi.c
> +++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c
> @@ -64,6 +64,7 @@ static int romapi_ecdsa_verify(struct udevice *dev,
> const void *signature, size_t sig_len)
> {
> struct ecdsa_rom_api rom;
> + bool reenable_dcache;
reenable_dcache is used without being initialized
> uint8_t raw_key[64];
> uint32_t rom_ret;
> int algo;
> @@ -81,8 +82,21 @@ static int romapi_ecdsa_verify(struct udevice *dev,
> memcpy(raw_key + 32, pubkey->y, 32);
>
> stm32mp_rom_get_ecdsa_functions();
> +
> + /*
> + * Disable D-cache before calling into BootROM, else CRYP DMA
> + * may fail to pick up the correct data.
> + */
> + if (dcache_status()) {
> + dcache_disable();
> + reenable_dcache = true;
> + }
> +
> rom_ret = rom.ecdsa_verify_signature(hash, raw_key, signature, algo);
>
> + if (reenable_dcache)
> + dcache_enable();
> +
> return rom_ret == ROM_API_SUCCESS ? 0 : -EPERM;
> }
>
Re: [PATCH 3/4] ARM: stm32: Pass ROM API table pointer to U-Boot proper
Hi,
minor comment
On 12/6/22 03:33, Marek Vasut wrote:
The ROM API table pointer is no longer accessible from U-Boot, fix
this by passing the ROM API pointer through. This makes it possible
for U-Boot to call ROM API functions to authenticate payload like
signed fitImages.
Signed-off-by: Marek Vasut
---
Cc: Alexandru Gagniuc
Cc: Patrice Chotard
Cc: Patrick Delaunay
---
arch/arm/mach-stm32mp/cpu.c | 15 +++
1 file changed, 15 insertions(+)
diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c
index fa02a11d867..9553ecd243c 100644
--- a/arch/arm/mach-stm32mp/cpu.c
+++ b/arch/arm/mach-stm32mp/cpu.c
@@ -22,6 +22,7 @@
#include
#include
#include
+#include
/*
* early TLB into the .data section so that it not get cleared
@@ -413,3 +414,17 @@ uintptr_t get_stm32mp_bl2_dtb(void)
{
return nt_fw_dtb;
}
+
+#ifdef CONFIG_SPL_BUILD
+void jump_to_image_no_args(struct spl_image_info *spl_image)
missing "__noreturn" ?
void __noreturn jump_to_image_no_args(struct spl_image_info *spl_image)
+{
+ typedef void __noreturn (*image_entry_noargs_t)(u32 romapi);
really 'noargs' ?
I propose to replace to 'arg' => image_entry_arg_t
arch/powerpc/lib/spl.c:20
base on arch/arm/lib/spl.c:71
arch/microblaze/cpu/spl.c:37
typedef void __noreturn (*image_entry_noargs_t)(u32 romapi);
or with stm32:
typedef void __noreturn (*image_entry_stm32_t)(u32 romapi);
based on arch/riscv/lib/spl.c:40
+ uintptr_t romapi = get_stm32mp_rom_api_table();
+
+ image_entry_noargs_t image_entry =
+ (image_entry_noargs_t)spl_image->entry_point;
+
+ printf("image entry point: 0x%lx\n", spl_image->entry_point);
+ image_entry(romapi);
+}
+#endif
regards
Patrick
Re: [RFC 1/1] sound: allow waveform selection
On 12/6/22 00:55, Simon Glass wrote: Hi Heinrich, On Mon, 5 Dec 2022 at 13:38, Heinrich Schuchardt wrote: * Allow the sound command to select the sine or the square waveform. * Allow to play multiple tones with one command. * Adjust documentation. * Adjust unit test. Signed-off-by: Heinrich Schuchardt --- This would be the alternative to [v2,6/7] sound: add CONFIG_SOUND_SINE symbol For testing with the sandbox remove this line arch/sandbox/dts/test.dts:969 sandbox,silent; /* Don't emit sounds while testing */ run the sand box with './u-boot -T' and issue the following commands sound play sound play -s sound play -s 600 500 -q sound play -s 500 1047 500 880 500 0 500 1047 500 880 500 0 500 784 500 698 500 784 1000 698 Listening to the output demonstrates why patch 7/7 is needed. --- arch/sandbox/include/asm/test.h | 7 cmd/sound.c | 60 ++--- doc/usage/cmd/sound.rst | 28 ++- drivers/sound/sandbox.c | 7 drivers/sound/sound-uclass.c| 19 +-- include/sound.h | 21 +--- test/dm/sound.c | 45 - 7 files changed, 151 insertions(+), 36 deletions(-) This seems OK to me. Perhaps add a few run_command() tests as well? Hello Simon, thanks for reviewing. I have created a pull request for the bug fixes to go into 2023.01. As it is late in the cycle I don't want to introduce bigger changes. On a system with multiple sound devices playback on the sandbox hangs in sandbox_sdl_sound_stop() waiting for sdl.stopping. This needs further investigation. Best regards Heinrich Regards, Simon
Re: [PATCH 2/4] ARM: stm32: Factor out save_boot_params
Hi Marek,
On 12/6/22 03:33, Marek Vasut wrote:
The STM32MP15xx platform currently comes with two incompatible
implementations of save_boot_params() weak function override.
Factor the save_boot_params() implementation into common cpu.c
code and provide accessors to read out both ROM API table address
and DT address from any place in the code instead.
good idea.
Signed-off-by: Marek Vasut
---
Cc: Alexandru Gagniuc
Cc: Patrice Chotard
Cc: Patrick Delaunay
---
arch/arm/mach-stm32mp/boot_params.c | 20 ++-
arch/arm/mach-stm32mp/cpu.c | 35 +++
arch/arm/mach-stm32mp/ecdsa_romapi.c | 20 ++-
.../arm/mach-stm32mp/include/mach/sys_proto.h | 3 ++
4 files changed, 42 insertions(+), 36 deletions(-)
diff --git a/arch/arm/mach-stm32mp/boot_params.c
b/arch/arm/mach-stm32mp/boot_params.c
index e91ef1b2fc7..e40cca938ef 100644
--- a/arch/arm/mach-stm32mp/boot_params.c
+++ b/arch/arm/mach-stm32mp/boot_params.c
@@ -11,30 +11,14 @@
#include
#include
-/*
- * Force data-section, as .bss will not be valid
- * when save_boot_params is invoked.
- */
-static unsigned long nt_fw_dtb __section(".data");
-
-/*
- * Save the FDT address provided by TF-A in r2 at boot time
- * This function is called from start.S
- */
-void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2,
- unsigned long r3)
-{
- nt_fw_dtb = r2;
-
- save_boot_params_ret();
-}
-
/*
* Use the saved FDT address provided by TF-A at boot time (NT_FW_CONFIG =
* Non Trusted Firmware configuration file) when the pointer is valid
*/
void *board_fdt_blob_setup(int *err)
{
+ unsigned long nt_fw_dtb = get_stm32mp_bl2_dtb();
+
log_debug("%s: nt_fw_dtb=%lx\n", __func__, nt_fw_dtb);
*err = 0;
diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c
index 855fc755fe0..fa02a11d867 100644
--- a/arch/arm/mach-stm32mp/cpu.c
+++ b/arch/arm/mach-stm32mp/cpu.c
@@ -378,3 +378,38 @@ int arch_misc_init(void)
return 0;
}
+
+/*
+ * Without forcing the ".data" section, this would get saved in ".bss". BSS
+ * will be cleared soon after, so it's not suitable.
+ */
+static uintptr_t rom_api_table __section(".data");
+static uintptr_t nt_fw_dtb __section(".data");
+
+/*
+ * The ROM gives us the API location in r0 when starting. This is only
available
+ * during SPL, as there isn't (yet) a mechanism to pass this on to u-boot. Save
+ * the FDT address provided by TF-A in r2 at boot time. This function is called
+ * from start.S
+ */
+void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2,
+ unsigned long r3)
+{
+#if IS_ENABLED(CONFIG_STM32_ECDSA_VERIFY)
+ rom_api_table = r0;
+#endif
+#if IS_ENABLED(CONFIG_TFABOOT)
+ nt_fw_dtb = r2;
+#endif
jut avoid #if when it is possible.
if (IS_ENABLED(CONFIG_STM32_ECDSA_VERIFY))
rom_api_table = r0;
if (IS_ENABLED(CONFIG_TFABOOT))
nt_fw_dtb = r2;
+ save_boot_params_ret();
+}
+
+uintptr_t get_stm32mp_rom_api_table(void)
+{
+ return rom_api_table;
+}
+
+uintptr_t get_stm32mp_bl2_dtb(void)
+{
+ return nt_fw_dtb;
+}
diff --git a/arch/arm/mach-stm32mp/ecdsa_romapi.c
b/arch/arm/mach-stm32mp/ecdsa_romapi.c
index 72b87bf2c64..32a7357ad56 100644
--- a/arch/arm/mach-stm32mp/ecdsa_romapi.c
+++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c
@@ -24,26 +24,10 @@ struct ecdsa_rom_api {
uint32_t ecc_algo);
};
-/*
- * Without forcing the ".data" section, this would get saved in ".bss". BSS
- * will be cleared soon after, so it's not suitable.
- */
-static uintptr_t rom_api_loc __section(".data");
-
-/*
- * The ROM gives us the API location in r0 when starting. This is only
available
- * during SPL, as there isn't (yet) a mechanism to pass this on to u-boot.
- */
-void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2,
- unsigned long r3)
-{
- rom_api_loc = r0;
- save_boot_params_ret();
-}
-
static void stm32mp_rom_get_ecdsa_functions(struct ecdsa_rom_api *rom)
{
- uintptr_t verify_ptr = rom_api_loc + ROM_API_OFFSET_ECDSA_VERIFY;
+ uintptr_t verify_ptr = get_stm32mp_rom_api_table() +
+ ROM_API_OFFSET_ECDSA_VERIFY;
rom->ecdsa_verify_signature = *(void **)verify_ptr;
}
diff --git a/arch/arm/mach-stm32mp/include/mach/sys_proto.h
b/arch/arm/mach-stm32mp/include/mach/sys_proto.h
index f19a70e53e0..0d39b67178e 100644
--- a/arch/arm/mach-stm32mp/include/mach/sys_proto.h
+++ b/arch/arm/mach-stm32mp/include/mach/sys_proto.h
@@ -77,3 +77,6 @@ void stm32mp_misc_init(void);
/* helper function: read data from OTP */
u32 get_otp(int index, int shift, int mask);
+
+uintptr_t get_stm32mp_rom_api_table(void);
+uintptr_t get_stm32mp_bl2_dtb(void);
regards
Patrick
Pull request for sound-2023-01-rc4
Dear Tom, The following changes since commit a50622d78c5c6babd1853ae913f339df54fe532c: Merge tag 'xilinx-for-v2023.01-rc3-v2' of https://source.denx.de/u-boot/custodians/u-boot-microblaze (2022-12-05 08:33:19 -0500) are available in the Git repository at: https://source.denx.de/u-boot/custodians/u-boot-efi.git tags/sound-2023-01-rc4 for you to fetch changes up to 304bc9f437df51b4d982fe25fd0988350c8f5fc9: sandbox: fix sound driver (2022-12-05 17:43:23 +0100) Gitlab CI showed no issues: https://source.denx.de/u-boot/custodians/u-boot-efi/-/pipelines/14337 Pull request for sound-2023-01-rc4 * Avoid endless loop and amend unit test * Add man-page for the sound command * Fix sandbox sound driver Heinrich Schuchardt (5): sound: avoid endless loop test: test sandbox sound driver more rigorously cmd: fix long text for sound command doc: man-page for the sound command sandbox: fix sound driver arch/sandbox/cpu/sdl.c | 11 +-- arch/sandbox/include/asm/test.h | 10 ++ cmd/sound.c | 2 +- doc/usage/cmd/sound.rst | 41 + doc/usage/index.rst | 1 + drivers/sound/sandbox.c | 9 + drivers/sound/sound.c | 5 - test/dm/sound.c | 11 +++ 8 files changed, 82 insertions(+), 8 deletions(-) create mode 100644 doc/usage/cmd/sound.rst
Re: [PATCH 5/8] configs: stm32mp: activate CONFIG_ENV_MMC_USE_DT
On 11/10/22 11:49, Patrick Delaunay wrote: > Activate by default CONFIG_ENV_MMC_USE_DT as "u-boot,mmc-env-partition" > should be always use in STMicroelectronics boards device tree to locate > the environment for mmc backend. The 2 defines: > CONFIG_ENV_OFFSET=0x28 > CONFIG_ENV_OFFSET_REDUND=0x2C > are only valid for spi-nor and not for SD-Card or eMMC. > > Signed-off-by: Patrick Delaunay > --- > > configs/stm32mp13_defconfig | 1 + > configs/stm32mp15_basic_defconfig | 1 + > configs/stm32mp15_defconfig | 1 + > configs/stm32mp15_trusted_defconfig | 1 + > 4 files changed, 4 insertions(+) > > diff --git a/configs/stm32mp13_defconfig b/configs/stm32mp13_defconfig > index af6b1839d039..4cab07647349 100644 > --- a/configs/stm32mp13_defconfig > +++ b/configs/stm32mp13_defconfig > @@ -46,6 +46,7 @@ CONFIG_ENV_IS_IN_MMC=y > CONFIG_SYS_REDUNDAND_ENVIRONMENT=y > CONFIG_SYS_RELOC_GD_ENV_ADDR=y > CONFIG_SYS_MMC_ENV_DEV=-1 > +CONFIG_ENV_MMC_USE_DT=y > CONFIG_CLK_SCMI=y > CONFIG_GPIO_HOG=y > CONFIG_DM_I2C=y > diff --git a/configs/stm32mp15_basic_defconfig > b/configs/stm32mp15_basic_defconfig > index 86ebbef0a6c8..4a96ad22bcc8 100644 > --- a/configs/stm32mp15_basic_defconfig > +++ b/configs/stm32mp15_basic_defconfig > @@ -91,6 +91,7 @@ CONFIG_ENV_UBI_VOLUME="uboot_config" > CONFIG_ENV_UBI_VOLUME_REDUND="uboot_config_r" > CONFIG_SYS_RELOC_GD_ENV_ADDR=y > CONFIG_SYS_MMC_ENV_DEV=-1 > +CONFIG_ENV_MMC_USE_DT=y > # CONFIG_SPL_ENV_IS_NOWHERE is not set > # CONFIG_SPL_ENV_IS_IN_SPI_FLASH is not set > CONFIG_TFTP_TSIZE=y > diff --git a/configs/stm32mp15_defconfig b/configs/stm32mp15_defconfig > index caa79e68834f..151981849de9 100644 > --- a/configs/stm32mp15_defconfig > +++ b/configs/stm32mp15_defconfig > @@ -65,6 +65,7 @@ CONFIG_ENV_UBI_VOLUME="uboot_config" > CONFIG_ENV_UBI_VOLUME_REDUND="uboot_config_r" > CONFIG_SYS_RELOC_GD_ENV_ADDR=y > CONFIG_SYS_MMC_ENV_DEV=-1 > +CONFIG_ENV_MMC_USE_DT=y > CONFIG_TFTP_TSIZE=y > CONFIG_STM32_ADC=y > CONFIG_CLK_SCMI=y > diff --git a/configs/stm32mp15_trusted_defconfig > b/configs/stm32mp15_trusted_defconfig > index 3309c2e79246..098eedc9b727 100644 > --- a/configs/stm32mp15_trusted_defconfig > +++ b/configs/stm32mp15_trusted_defconfig > @@ -66,6 +66,7 @@ CONFIG_ENV_UBI_VOLUME="uboot_config" > CONFIG_ENV_UBI_VOLUME_REDUND="uboot_config_r" > CONFIG_SYS_RELOC_GD_ENV_ADDR=y > CONFIG_SYS_MMC_ENV_DEV=-1 > +CONFIG_ENV_MMC_USE_DT=y > CONFIG_TFTP_TSIZE=y > CONFIG_STM32_ADC=y > CONFIG_CLK_SCMI=y Reviewed-by: Patrice Chotard Thanks Patrice
Re: [Uboot-stm32] [PATCH 8/8] env: mmc: cosmetic: remove unused macro STR(X)
On 11/10/22 11:49, Patrick Delaunay wrote:
> Remove the unused macro STR(X) since the commit 2b2f727500dc ("env: mmc:
> allow support of mmc_get_env_dev with OF_CONTROL")
>
> Signed-off-by: Patrick Delaunay
> ---
>
> env/mmc.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/env/mmc.c b/env/mmc.c
> index 8941e0f5ff39..85761417f283 100644
> --- a/env/mmc.c
> +++ b/env/mmc.c
> @@ -21,9 +21,6 @@
> #include
> #include
>
> -#define __STR(X) #X
> -#define STR(X) __STR(X)
> -
> #define ENV_MMC_INVALID_OFFSET ((s64)-1)
>
> #if defined(CONFIG_ENV_MMC_USE_DT)
Reviewed-by: Patrice Chotard
Thanks
Patrice
Re: [Uboot-stm32] [PATCH 7/8] env: mmc: add debug message when mmc-env-partition is not found
On 11/10/22 11:49, Patrick Delaunay wrote:
> Add a debug message to indicate a potential issue when
> "u-boot,mmc-env-partition" is present in config node of device tree
> but this partition name is not found in the mmc device.
>
> Signed-off-by: Patrick Delaunay
> ---
>
> env/mmc.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/env/mmc.c b/env/mmc.c
> index bd7d51e6b633..8941e0f5ff39 100644
> --- a/env/mmc.c
> +++ b/env/mmc.c
> @@ -120,6 +120,7 @@ static inline s64 mmc_offset(int copy)
> err = mmc_offset_try_partition(str, copy, );
> if (!err)
> return val;
> + debug("env partition '%s' not found (%d)", str, err);
> }
>
> /* try the GPT partition with "U-Boot ENV" TYPE GUID */
Reviewed-by: Patrice Chotard
Thanks
Patrice
Re: [Uboot-stm32] [PATCH 6/8] env: mmc: select GPT env partition by type guid
On 11/10/22 11:49, Patrick Delaunay wrote:
> Since commit c0364ce1c695 ("doc/README.gpt: define partition type GUID for
> U-Boot environment"), a specific type GUID can be used to indicate
> the U-Boot environment partition on the device with GPT partition table.
>
> This patch uses this type GUID to found the env partition as fallback
> when the partition name property "u-boot,mmc-env-partition" is not present
> in config node or if the indicated partition name is not found.
>
> The mmc_offset_try_partition() function is reused, it selects the first
> partition with the correct type GUID when the parameter 'str' is NULL.
>
> Signed-off-by: Patrick Delaunay
> ---
>
> env/mmc.c | 19 ++-
> 1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/env/mmc.c b/env/mmc.c
> index 1894b6483220..bd7d51e6b633 100644
> --- a/env/mmc.c
> +++ b/env/mmc.c
> @@ -74,8 +74,18 @@ static inline int mmc_offset_try_partition(const char
> *str, int copy, s64 *val)
> if (ret < 0)
> return ret;
>
> - if (!strncmp((const char *)info.name, str, sizeof(info.name)))
> + if (str && !strncmp((const char *)info.name, str,
> sizeof(info.name)))
> break;
> +#ifdef CONFIG_PARTITION_TYPE_GUID
> + if (!str) {
> + const efi_guid_t env_guid =
> PARTITION_U_BOOT_ENVIRONMENT;
> + efi_guid_t type_guid;
> +
> + uuid_str_to_bin(info.type_guid, type_guid.b,
> UUID_STR_FORMAT_GUID);
> + if (!memcmp(_guid, _guid, sizeof(efi_guid_t)))
> + break;
> + }
> +#endif
> }
>
> /* round up to info.blksz */
> @@ -112,6 +122,13 @@ static inline s64 mmc_offset(int copy)
> return val;
> }
>
> + /* try the GPT partition with "U-Boot ENV" TYPE GUID */
> + if (IS_ENABLED(CONFIG_PARTITION_TYPE_GUID)) {
> + err = mmc_offset_try_partition(NULL, copy, );
> + if (!err)
> + return val;
> + }
> +
> defvalue = ENV_MMC_OFFSET;
> propname = dt_prop.offset;
>
Reviewed-by: Patrice Chotard
Thanks
Patrice
Re: [Uboot-stm32] [PATCH 4/8] env: mmc: add CONFIG_ENV_MMC_USE_DT
On 11/10/22 11:49, Patrick Delaunay wrote: > Add a new config CONFIG_ENV_MMC_USE_DT to force configuration of the > U-Boot environment offset with device tree config node. > > This patch avoids issues when several CONFIG_ENV_IS_IN_XXX are activated, > the defconfig file uses the same value for CONFIG_ENV_OFFSET or > CONFIG_ENV_OFFSET_REDUND for the several ENV backends (SPI_FLASH, EEPROM > NAND, SATA, MMC). > > After this patch a bad offset value is not possible when the selected > partition in device tree is not found. > > Signed-off-by: Patrick Delaunay > --- > > env/Kconfig | 16 > env/mmc.c | 7 +++ > 2 files changed, 23 insertions(+) > > diff --git a/env/Kconfig b/env/Kconfig > index 24111dfaf47b..f8ee99052b97 100644 > --- a/env/Kconfig > +++ b/env/Kconfig > @@ -242,6 +242,13 @@ config ENV_IS_IN_MMC > This value is also in units of bytes, but must also be aligned to > an MMC sector boundary. > > + CONFIG_ENV_MMC_USE_DT (optional): > + > + These define forces the configuration by the config node in device > + tree with partition name: "u-boot,mmc-env-partition" or with > + offset: "u-boot,mmc-env-offset", "u-boot,mmc-env-offset-redundant". > + CONFIG_ENV_OFFSET and CONFIG_ENV_OFFSET_REDUND are not used. > + > config ENV_IS_IN_NAND > bool "Environment in a NAND device" > depends on !CHAIN_OF_TRUST > @@ -650,6 +657,15 @@ config SYS_MMC_ENV_PART > partition 0 or the first boot partition, which is 1 or some other > defined > partition. > > +config ENV_MMC_USE_DT > + bool "Read partition name and offset in DT" > + depends on ENV_IS_IN_MMC && OF_CONTROL > + help > + Only use the device tree to get the environment location in MMC > + device, with partition name or with offset. > + The 2 defines CONFIG_ENV_OFFSET, CONFIG_ENV_OFFSET_REDUND > + are not used as fallback. > + > config USE_DEFAULT_ENV_FILE > bool "Create default environment from file" > help > diff --git a/env/mmc.c b/env/mmc.c > index 661a268ea07d..1894b6483220 100644 > --- a/env/mmc.c > +++ b/env/mmc.c > @@ -26,6 +26,12 @@ > > #define ENV_MMC_INVALID_OFFSET ((s64)-1) > > +#if defined(CONFIG_ENV_MMC_USE_DT) > +/* ENV offset is invalid when not defined in Device Tree */ > +#define ENV_MMC_OFFSET ENV_MMC_INVALID_OFFSET > +#define ENV_MMC_OFFSET_REDUNDENV_MMC_INVALID_OFFSET > + > +#else > /* Default ENV offset when not defined in Device Tree */ > #define ENV_MMC_OFFSET CONFIG_ENV_OFFSET > > @@ -34,6 +40,7 @@ > #else > #define ENV_MMC_OFFSET_REDUNDENV_MMC_INVALID_OFFSET > #endif > +#endif > > DECLARE_GLOBAL_DATA_PTR; > Reviewed-by: Patrice Chotard Thanks Patrice
Re: [Uboot-stm32] [PATCH 3/8] env: mcc: fix compilation error with ENV_IS_EMBEDDED
On 11/10/22 11:49, Patrick Delaunay wrote:
> When ENV_IS_EMBEDDED is enabled, ret is not defined but is used as a
> return value in env_mmc_load().
> This patch correct this issue and simplify the existing code, test only
> one time #if defined(ENV_IS_EMBEDDED) and not in the function.
>
> Signed-off-by: Patrick Delaunay
> ---
>
> env/mmc.c | 12 +++-
> 1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/env/mmc.c b/env/mmc.c
> index b36bd9ad77ee..661a268ea07d 100644
> --- a/env/mmc.c
> +++ b/env/mmc.c
> @@ -353,10 +353,14 @@ static inline int read_env(struct mmc *mmc, unsigned
> long size,
> return (n == blk_cnt) ? 0 : -1;
> }
>
> -#if defined(CONFIG_SYS_REDUNDAND_ENVIRONMENT)
> +#if defined(ENV_IS_EMBEDDED)
> +static int env_mmc_load(void)
> +{
> + return 0;
> +}
> +#elif defined(CONFIG_SYS_REDUNDAND_ENVIRONMENT)
> static int env_mmc_load(void)
> {
> -#if !defined(ENV_IS_EMBEDDED)
> struct mmc *mmc;
> u32 offset1, offset2;
> int read1_fail = 0, read2_fail = 0;
> @@ -408,13 +412,11 @@ err:
> if (ret)
> env_set_default(errmsg, 0);
>
> -#endif
> return ret;
> }
> #else /* ! CONFIG_SYS_REDUNDAND_ENVIRONMENT */
> static int env_mmc_load(void)
> {
> -#if !defined(ENV_IS_EMBEDDED)
> ALLOC_CACHE_ALIGN_BUFFER(char, buf, CONFIG_ENV_SIZE);
> struct mmc *mmc;
> u32 offset;
> @@ -453,7 +455,7 @@ fini:
> err:
> if (ret)
> env_set_default(errmsg, 0);
> -#endif
> +
> return ret;
> }
> #endif /* CONFIG_SYS_REDUNDAND_ENVIRONMENT */
Reviewed-by: Patrice Chotard
Thanks
Patrice
Re: [Uboot-stm32] [PATCH 2/8] env: mcc: Drop unnecessary #ifdefs
On 11/10/22 11:48, Patrick Delaunay wrote:
> This file has a lot of conditional code and much of it is unnecessary.
> Clean this up to reduce the number of build combinations.
>
> This patch replaces the test on CONFIG_ENV_OFFSET_REDUND for the
> more coherent CONFIG_SYS_REDUNDAND_ENVIRONMENT.
>
> This patch also corrects a compilation issue in init_mmc_for_env()
> when CONFIG_SYS_MMC_ENV_PART is not activated, env_mmc_orig_hwpart is
> not defined.
>
> Signed-off-by: Patrick Delaunay
> ---
>
> env/mmc.c | 120 +-
> 1 file changed, 64 insertions(+), 56 deletions(-)
>
> diff --git a/env/mmc.c b/env/mmc.c
> index 42bcf7e775cc..b36bd9ad77ee 100644
> --- a/env/mmc.c
> +++ b/env/mmc.c
> @@ -46,7 +46,7 @@ DECLARE_GLOBAL_DATA_PTR;
> #if (defined(CONFIG_SYS_REDUNDAND_ENVIRONMENT) && \
> (CONFIG_SYS_MMC_ENV_PART == 1) && \
> (CONFIG_ENV_OFFSET == CONFIG_ENV_OFFSET_REDUND))
> -#define ENV_MMC_HWPART_REDUND
> +#define ENV_MMC_HWPART_REDUND1
> #endif
>
> #if CONFIG_IS_ENABLED(OF_CONTROL)
> @@ -108,12 +108,11 @@ static inline s64 mmc_offset(int copy)
> defvalue = ENV_MMC_OFFSET;
> propname = dt_prop.offset;
>
> -#if defined(CONFIG_ENV_OFFSET_REDUND)
> - if (copy) {
> + if (IS_ENABLED(CONFIG_SYS_REDUNDAND_ENVIRONMENT) && copy) {
> defvalue = ENV_MMC_OFFSET_REDUND;
> propname = dt_prop.offset_redund;
> }
> -#endif
> +
> return ofnode_conf_read_int(propname, defvalue);
> }
> #else
> @@ -121,10 +120,9 @@ static inline s64 mmc_offset(int copy)
> {
> s64 offset = ENV_MMC_OFFSET;
>
> -#if defined(CONFIG_ENV_OFFSET_REDUND)
> - if (copy)
> + if (IS_ENABLED(CONFIG_SYS_REDUNDAND_ENVIRONMENT) && copy)
> offset = ENV_MMC_OFFSET_REDUND;
> -#endif
> +
> return offset;
> }
> #endif
> @@ -165,8 +163,24 @@ static int mmc_set_env_part(struct mmc *mmc, uint part)
>
> return ret;
> }
> +
> +static bool mmc_set_env_part_init(struct mmc *mmc)
> +{
> + env_mmc_orig_hwpart = mmc_get_blk_desc(mmc)->hwpart;
> + if (mmc_set_env_part(mmc, mmc_get_env_part(mmc)))
> + return false;
> +
> + return true;
> +}
> +
> +static int mmc_set_env_part_restore(struct mmc *mmc)
> +{
> + return mmc_set_env_part(mmc, env_mmc_orig_hwpart);
> +}
> #else
> static inline int mmc_set_env_part(struct mmc *mmc, uint part) {return 0; };
> +static bool mmc_set_env_part_init(struct mmc *mmc) {return true; }
> +static inline int mmc_set_env_part_restore(struct mmc *mmc) {return 0; };
> #endif
>
> static const char *init_mmc_for_env(struct mmc *mmc)
> @@ -183,8 +197,7 @@ static const char *init_mmc_for_env(struct mmc *mmc)
> if (mmc_init(mmc))
> return "MMC init failed";
> #endif
> - env_mmc_orig_hwpart = mmc_get_blk_desc(mmc)->hwpart;
> - if (mmc_set_env_part(mmc, mmc_get_env_part(mmc)))
> + if (!mmc_set_env_part_init(mmc))
> return "MMC partition switch failed";
>
> return NULL;
> @@ -192,11 +205,7 @@ static const char *init_mmc_for_env(struct mmc *mmc)
>
> static void fini_mmc_for_env(struct mmc *mmc)
> {
> -#ifdef CONFIG_SYS_MMC_ENV_PART
> - int dev = mmc_get_env_dev();
> -
> - blk_select_hwpart_devnum(UCLASS_MMC, dev, env_mmc_orig_hwpart);
> -#endif
> + mmc_set_env_part_restore(mmc);
> }
>
> #if defined(CONFIG_CMD_SAVEENV) && !defined(CONFIG_SPL_BUILD)
> @@ -233,21 +242,20 @@ static int env_mmc_save(void)
> if (ret)
> goto fini;
>
> -#ifdef CONFIG_ENV_OFFSET_REDUND
> - if (gd->env_valid == ENV_VALID)
> - copy = 1;
> -
> -#ifdef ENV_MMC_HWPART_REDUND
> - ret = mmc_set_env_part(mmc, copy + 1);
> - if (ret)
> - goto fini;
> -#endif
> + if (IS_ENABLED(CONFIG_SYS_REDUNDAND_ENVIRONMENT)) {
> + if (gd->env_valid == ENV_VALID)
> + copy = 1;
>
> -#endif
> + if (IS_ENABLED(ENV_MMC_HWPART_REDUND)) {
> + ret = mmc_set_env_part(mmc, copy + 1);
> + if (ret)
> + goto fini;
> + }
>
> - if (mmc_get_env_addr(mmc, copy, )) {
> - ret = 1;
> - goto fini;
> + if (mmc_get_env_addr(mmc, copy, )) {
> + ret = 1;
> + goto fini;
> + }
> }
>
> printf("Writing to %sMMC(%d)... ", copy ? "redundant " : "", dev);
> @@ -259,12 +267,12 @@ static int env_mmc_save(void)
>
> ret = 0;
>
> -#ifdef CONFIG_ENV_OFFSET_REDUND
> - gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID : ENV_REDUND;
> -#endif
> + if (IS_ENABLED(CONFIG_SYS_REDUNDAND_ENVIRONMENT))
> + gd->env_valid = gd->env_valid == ENV_REDUND ? ENV_VALID :
> ENV_REDUND;
>
> fini:
> fini_mmc_for_env(mmc);
> +
> return ret;
> }
>
> @@ -308,22 +316,22 @@ static int env_mmc_erase(void)
> printf("\n");
>
Re: [Uboot-stm32] [PATCH 1/8] env: mmc: introduced ENV_MMC_OFFSET
On 11/10/22 11:48, Patrick Delaunay wrote:
> Introduce ENV_MMC_OFFSET defines.
> It is a preliminary step to the next patches to simplify the code.
>
> Signed-off-by: Patrick Delaunay
> ---
>
> env/mmc.c | 24
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/env/mmc.c b/env/mmc.c
> index c28f4c6c6dc0..42bcf7e775cc 100644
> --- a/env/mmc.c
> +++ b/env/mmc.c
> @@ -24,6 +24,17 @@
> #define __STR(X) #X
> #define STR(X) __STR(X)
>
> +#define ENV_MMC_INVALID_OFFSET ((s64)-1)
> +
> +/* Default ENV offset when not defined in Device Tree */
> +#define ENV_MMC_OFFSET CONFIG_ENV_OFFSET
> +
> +#if defined(CONFIG_ENV_OFFSET_REDUND)
> +#define ENV_MMC_OFFSET_REDUNDCONFIG_ENV_OFFSET_REDUND
> +#else
> +#define ENV_MMC_OFFSET_REDUNDENV_MMC_INVALID_OFFSET
> +#endif
> +
> DECLARE_GLOBAL_DATA_PTR;
>
> /*
> @@ -94,12 +105,12 @@ static inline s64 mmc_offset(int copy)
> return val;
> }
>
> - defvalue = CONFIG_ENV_OFFSET;
> + defvalue = ENV_MMC_OFFSET;
> propname = dt_prop.offset;
>
> #if defined(CONFIG_ENV_OFFSET_REDUND)
> if (copy) {
> - defvalue = CONFIG_ENV_OFFSET_REDUND;
> + defvalue = ENV_MMC_OFFSET_REDUND;
> propname = dt_prop.offset_redund;
> }
> #endif
> @@ -108,11 +119,11 @@ static inline s64 mmc_offset(int copy)
> #else
> static inline s64 mmc_offset(int copy)
> {
> - s64 offset = CONFIG_ENV_OFFSET;
> + s64 offset = ENV_MMC_OFFSET;
>
> #if defined(CONFIG_ENV_OFFSET_REDUND)
> if (copy)
> - offset = CONFIG_ENV_OFFSET_REDUND;
> + offset = ENV_MMC_OFFSET_REDUND;
> #endif
> return offset;
> }
> @@ -122,6 +133,11 @@ __weak int mmc_get_env_addr(struct mmc *mmc, int copy,
> u32 *env_addr)
> {
> s64 offset = mmc_offset(copy);
>
> + if (offset == ENV_MMC_INVALID_OFFSET) {
> + printf("Invalid ENV offset in MMC, copy=%d\n", copy);
> + return -ENOENT;
> + }
> +
> if (offset < 0)
> offset += mmc->capacity;
>
Reviewed-by: Patrice Chotard
Thanks
Patrice
Re: [PATCH 4/4] ARM: stm32: Make ECDSA authentication available to U-Boot
On 12/6/22 03:33, Marek Vasut wrote: > With U-Boot having access to ROM API call table, it is possible to use > the ROM API call it authenticate e.g. signed kernel fitImages using the > BootROM ECDSA support. Make this available by pulling the ECDSA BootROM > call support from SPL-only guard. > > Signed-off-by: Marek Vasut > --- > Cc: Alexandru Gagniuc > Cc: Patrice Chotard > Cc: Patrick Delaunay > --- > arch/arm/mach-stm32mp/Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/mach-stm32mp/Makefile b/arch/arm/mach-stm32mp/Makefile > index 1db9057e049..a19b2797c8b 100644 > --- a/arch/arm/mach-stm32mp/Makefile > +++ b/arch/arm/mach-stm32mp/Makefile > @@ -11,10 +11,10 @@ obj-y += bsec.o > obj-$(CONFIG_STM32MP13x) += stm32mp13x.o > obj-$(CONFIG_STM32MP15x) += stm32mp15x.o > > +obj-$(CONFIG_STM32_ECDSA_VERIFY) += ecdsa_romapi.o > ifdef CONFIG_SPL_BUILD > obj-y += spl.o > obj-y += tzc400.o > -obj-$(CONFIG_STM32_ECDSA_VERIFY) += ecdsa_romapi.o > else > obj-y += cmd_stm32prog/ > obj-$(CONFIG_CMD_STM32KEY) += cmd_stm32key.o Reviewed-by: Patrice Chotard Thanks Patrice
Re: [PATCH 3/4] ARM: stm32: Pass ROM API table pointer to U-Boot proper
On 12/6/22 03:33, Marek Vasut wrote:
> The ROM API table pointer is no longer accessible from U-Boot, fix
> this by passing the ROM API pointer through. This makes it possible
> for U-Boot to call ROM API functions to authenticate payload like
> signed fitImages.
>
> Signed-off-by: Marek Vasut
> ---
> Cc: Alexandru Gagniuc
> Cc: Patrice Chotard
> Cc: Patrick Delaunay
> ---
> arch/arm/mach-stm32mp/cpu.c | 15 +++
> 1 file changed, 15 insertions(+)
>
> diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c
> index fa02a11d867..9553ecd243c 100644
> --- a/arch/arm/mach-stm32mp/cpu.c
> +++ b/arch/arm/mach-stm32mp/cpu.c
> @@ -22,6 +22,7 @@
> #include
> #include
> #include
> +#include
>
> /*
> * early TLB into the .data section so that it not get cleared
> @@ -413,3 +414,17 @@ uintptr_t get_stm32mp_bl2_dtb(void)
> {
> return nt_fw_dtb;
> }
> +
> +#ifdef CONFIG_SPL_BUILD
> +void jump_to_image_no_args(struct spl_image_info *spl_image)
> +{
> + typedef void __noreturn (*image_entry_noargs_t)(u32 romapi);
> + uintptr_t romapi = get_stm32mp_rom_api_table();
> +
> + image_entry_noargs_t image_entry =
> + (image_entry_noargs_t)spl_image->entry_point;
> +
> + printf("image entry point: 0x%lx\n", spl_image->entry_point);
> + image_entry(romapi);
> +}
> +#endif
Reviewed-by: Patrice Chotard
Thanks
Patrice
Re: [PATCH 2/4] ARM: stm32: Factor out save_boot_params
On 12/6/22 03:33, Marek Vasut wrote:
> The STM32MP15xx platform currently comes with two incompatible
> implementations of save_boot_params() weak function override.
> Factor the save_boot_params() implementation into common cpu.c
> code and provide accessors to read out both ROM API table address
> and DT address from any place in the code instead.
>
> Signed-off-by: Marek Vasut
> ---
> Cc: Alexandru Gagniuc
> Cc: Patrice Chotard
> Cc: Patrick Delaunay
> ---
> arch/arm/mach-stm32mp/boot_params.c | 20 ++-
> arch/arm/mach-stm32mp/cpu.c | 35 +++
> arch/arm/mach-stm32mp/ecdsa_romapi.c | 20 ++-
> .../arm/mach-stm32mp/include/mach/sys_proto.h | 3 ++
> 4 files changed, 42 insertions(+), 36 deletions(-)
>
> diff --git a/arch/arm/mach-stm32mp/boot_params.c
> b/arch/arm/mach-stm32mp/boot_params.c
> index e91ef1b2fc7..e40cca938ef 100644
> --- a/arch/arm/mach-stm32mp/boot_params.c
> +++ b/arch/arm/mach-stm32mp/boot_params.c
> @@ -11,30 +11,14 @@
> #include
> #include
>
> -/*
> - * Force data-section, as .bss will not be valid
> - * when save_boot_params is invoked.
> - */
> -static unsigned long nt_fw_dtb __section(".data");
> -
> -/*
> - * Save the FDT address provided by TF-A in r2 at boot time
> - * This function is called from start.S
> - */
> -void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2,
> - unsigned long r3)
> -{
> - nt_fw_dtb = r2;
> -
> - save_boot_params_ret();
> -}
> -
> /*
> * Use the saved FDT address provided by TF-A at boot time (NT_FW_CONFIG =
> * Non Trusted Firmware configuration file) when the pointer is valid
> */
> void *board_fdt_blob_setup(int *err)
> {
> + unsigned long nt_fw_dtb = get_stm32mp_bl2_dtb();
> +
> log_debug("%s: nt_fw_dtb=%lx\n", __func__, nt_fw_dtb);
>
> *err = 0;
> diff --git a/arch/arm/mach-stm32mp/cpu.c b/arch/arm/mach-stm32mp/cpu.c
> index 855fc755fe0..fa02a11d867 100644
> --- a/arch/arm/mach-stm32mp/cpu.c
> +++ b/arch/arm/mach-stm32mp/cpu.c
> @@ -378,3 +378,38 @@ int arch_misc_init(void)
>
> return 0;
> }
> +
> +/*
> + * Without forcing the ".data" section, this would get saved in ".bss". BSS
> + * will be cleared soon after, so it's not suitable.
> + */
> +static uintptr_t rom_api_table __section(".data");
> +static uintptr_t nt_fw_dtb __section(".data");
> +
> +/*
> + * The ROM gives us the API location in r0 when starting. This is only
> available
> + * during SPL, as there isn't (yet) a mechanism to pass this on to u-boot.
> Save
> + * the FDT address provided by TF-A in r2 at boot time. This function is
> called
> + * from start.S
> + */
> +void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2,
> + unsigned long r3)
> +{
> +#if IS_ENABLED(CONFIG_STM32_ECDSA_VERIFY)
> + rom_api_table = r0;
> +#endif
> +#if IS_ENABLED(CONFIG_TFABOOT)
> + nt_fw_dtb = r2;
> +#endif
> + save_boot_params_ret();
> +}
> +
> +uintptr_t get_stm32mp_rom_api_table(void)
> +{
> + return rom_api_table;
> +}
> +
> +uintptr_t get_stm32mp_bl2_dtb(void)
> +{
> + return nt_fw_dtb;
> +}
> diff --git a/arch/arm/mach-stm32mp/ecdsa_romapi.c
> b/arch/arm/mach-stm32mp/ecdsa_romapi.c
> index 72b87bf2c64..32a7357ad56 100644
> --- a/arch/arm/mach-stm32mp/ecdsa_romapi.c
> +++ b/arch/arm/mach-stm32mp/ecdsa_romapi.c
> @@ -24,26 +24,10 @@ struct ecdsa_rom_api {
> uint32_t ecc_algo);
> };
>
> -/*
> - * Without forcing the ".data" section, this would get saved in ".bss". BSS
> - * will be cleared soon after, so it's not suitable.
> - */
> -static uintptr_t rom_api_loc __section(".data");
> -
> -/*
> - * The ROM gives us the API location in r0 when starting. This is only
> available
> - * during SPL, as there isn't (yet) a mechanism to pass this on to u-boot.
> - */
> -void save_boot_params(unsigned long r0, unsigned long r1, unsigned long r2,
> - unsigned long r3)
> -{
> - rom_api_loc = r0;
> - save_boot_params_ret();
> -}
> -
> static void stm32mp_rom_get_ecdsa_functions(struct ecdsa_rom_api *rom)
> {
> - uintptr_t verify_ptr = rom_api_loc + ROM_API_OFFSET_ECDSA_VERIFY;
> + uintptr_t verify_ptr = get_stm32mp_rom_api_table() +
> +ROM_API_OFFSET_ECDSA_VERIFY;
>
> rom->ecdsa_verify_signature = *(void **)verify_ptr;
> }
> diff --git a/arch/arm/mach-stm32mp/include/mach/sys_proto.h
> b/arch/arm/mach-stm32mp/include/mach/sys_proto.h
> index f19a70e53e0..0d39b67178e 100644
> --- a/arch/arm/mach-stm32mp/include/mach/sys_proto.h
> +++ b/arch/arm/mach-stm32mp/include/mach/sys_proto.h
> @@ -77,3 +77,6 @@ void stm32mp_misc_init(void);
>
> /* helper function: read data from OTP */
> u32 get_otp(int index, int shift, int mask);
> +
> +uintptr_t get_stm32mp_rom_api_table(void);
> +uintptr_t get_stm32mp_bl2_dtb(void);
Reviewed-by: Patrice Chotard
Thanks
Patrice
