[bug] timer: orion-timer: static variable patch breaks armada-388-clearfog boot
Hi, Commit 5387b093 was a fix for early_init_done causing issues on Pogo v4 [1]. The fix breaks armada-388-clearfog because the boot hangs indefinitely. It appears that the cause is an infinite loop in __udelay due to get_ticks() always returning 0. After adding some printf statements it became apparent that the timer was never getting initialised. git-blame identified the "timer: orion-timer: Fix problem with early static variable" patch as a likely issue. Reverting the commit restores boot for armada-388-clearfog. Patching the issue is a bit beyond me without understanding the Pogo v4 issue, but I'm happy to test any patches that are developed. Regards Martin [1] https://lore.kernel.org/u-boot/20221221091849.1018783-1...@denx.de/
Re: [PATCH v2] vbe: Allow probing the VBE bootmeth to fail in OS fixup
On 2023-01-12, Simon Glass wrote:
> This device is created when there are no bootmeths defined in the device
> tree. But it cannot be probed without a device tree node.
>
> For now, ignore a probe failure.
>
> Signed-off-by: Simon Glass
> Reported-by: Karsten Merker
> Suggested-by: Heinrich Schuchardt
> Fixes: a56f663f0707 ("vbe: Add info about the VBE device to the fwupd node")
I was able to reproduce the issue using the qemu-riscv64 instructions
Karsten provided, and applying the patch fixes it, thanks!
Tested-by: Vagrant Cascadian
live well,
vagrant
> ---
>
> Changes in v2:
> - With 'with' typo
> - Change to a debug message and add a comment
>
> boot/vbe_simple_os.c | 16
> 1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/boot/vbe_simple_os.c b/boot/vbe_simple_os.c
> index b2041a95a30..8c641ec07e2 100644
> --- a/boot/vbe_simple_os.c
> +++ b/boot/vbe_simple_os.c
> @@ -72,6 +72,18 @@ static int bootmeth_vbe_simple_ft_fixup(void *ctx, struct
> event *event)
> chosen = oftree_path(tree, "/chosen");
> if (!ofnode_valid(chosen))
> continue;
> +
> + ret = device_probe(dev);
> + if (ret) {
> + /*
> + * This should become an error when VBE is updated to
> + * only bind this device when a node exists
> + */
> + log_debug("VBE device '%s' failed to probe (err=%d)",
> + dev->name, ret);
> + return 0;
> + }
> +
> ret = ofnode_add_subnode(chosen, "fwupd", );
> if (ret && ret != -EEXIST)
> return log_msg_ret("fwu", ret);
> @@ -80,10 +92,6 @@ static int bootmeth_vbe_simple_ft_fixup(void *ctx, struct
> event *event)
> if (ret && ret != -EEXIST)
> return log_msg_ret("dev", ret);
>
> - ret = device_probe(dev);
> - if (ret)
> - return log_msg_ret("probe", ret);
> -
> /* Copy over the vbe properties for fwupd */
> log_debug("Fixing up: %s\n", dev->name);
> ret = ofnode_copy_props(dev_ofnode(dev), subnode);
signature.asc
Description: PGP signature
Re: [PATCH 1/1] configs: enable CONFIG_LEGACY_IMAGE_FORMAT on sandbox
On Sat, 14 Jan 2023 at 04:43, Heinrich Schuchardt wrote: > > The sandbox should be able to execute scripts in legacy U-Boot image format > for testing. > > Signed-off-by: Heinrich Schuchardt > --- > configs/sandbox_defconfig | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Simon Glass
Re: [PATCH 1/2] x86: Fix saying arch_cpu_init_dm in debug/docs
On Sat, 14 Jan 2023 at 13:49, Tom Rini wrote:
>
> The function arch_cpu_init_dm was renamed to fsp_setup_pinctrl in these
> cases, so rename debug / docs to match.
>
> Cc: Simon Glass
> Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
> Signed-off-by: Tom Rini
> ---
> arch/x86/lib/spl.c| 2 +-
> doc/board/google/chromebook_coral.rst | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
Reviewed-by: Simon Glass
Re: [PATCH 2/2] event: Correct dependencies on the EVENT framework
Hi Tom,
On Sat, 14 Jan 2023 at 13:49, Tom Rini wrote:
>
> The event framework is just that, a framework. Enabling it by itself
> does nothing, so we shouldn't ask the user about it. Reword (and correct
> typos) around this the option and help text. This also applies to
> DM_EVENT, so reword as well.
>
> With this, it's time to address the larger problems. When functionality
> uses events, typically via EVENT_SPY, the appropriate framework then
> must be select'd and NOT imply'd. As the functionality will cease to
> work (and so, platforms will fail to boot) this is non-optional and
> where select is appropriate. Audit the current users of EVENT_SPY to
> have a more fine-grained approach to select'ing the framework where
> used.
>
> Cc: Simon Glass
> Reported-by: Oliver Graute
> Reported-by: Francesco Dolcini
> Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
> Fixes: 42fdcebf859f ("event: Convert misc_init_f() to use events")
> Signed-off-by: Tom Rini
> ---
> arch/Kconfig | 6 +++---
> arch/arm/Kconfig | 9 -
> arch/arm/mach-omap2/Kconfig | 3 +++
> arch/mips/Kconfig| 2 +-
> arch/powerpc/cpu/mpc85xx/Kconfig | 1 +
> arch/x86/Kconfig | 1 +
> arch/x86/cpu/baytrail/Kconfig| 1 +
> arch/x86/cpu/broadwell/Kconfig | 1 +
> arch/x86/cpu/ivybridge/Kconfig | 1 +
> arch/x86/cpu/quark/Kconfig | 1 +
> board/google/Kconfig | 1 +
> board/keymile/Kconfig| 1 +
> boot/Kconfig | 1 +
> cmd/Kconfig | 1 +
> common/Kconfig | 17 -
> drivers/core/Kconfig | 9 +
> drivers/cpu/Kconfig | 1 -
Reviewed-by: Simon Glass
Tested on chromebook-coral:
Tested-by: Simon Glass
I am not quite sure what the effective difference is between select
and imply. Doesn't this suggest that there are some conflicting config
options? The only way imply would be disabled ( I thought) is if a
board does it explicitly?
Regards,
Simon
RE: [PATCH 1/1] i2c:aspeed:support ast2600 i2c new register mode driver
Hello, I resend the v2 version here https://www.mail-archive.com/u-boot@lists.denx.de/msg460560.html Ryan Chen > -Original Message- > From: Simon Glass > Sent: Thursday, January 12, 2023 5:08 AM > To: Ryan Chen > Cc: Heiko Schocher ; BMC-SW ; > u-boot@lists.denx.de > Subject: Re: [PATCH 1/1] i2c:aspeed:support ast2600 i2c new register mode > driver > > Hi Ryan, > > On Tue, 10 Jan 2023 at 23:48, ryan_chen > wrote: > > > > Add i2c new register mode driver to support AST2600 i2c new register > > mode. AST2600 i2c controller have legacy and new register mode. The > > new register mode have global register support 4 base clock for scl > > clock selection, and new clock divider mode. > > > > Signed-off-by: ryan_chen > > --- > > MAINTAINERS | 6 + > > drivers/i2c/Kconfig | 10 + > > drivers/i2c/Makefile | 1 + > > drivers/i2c/ast2600_i2c.c | 375 > > ++ > > drivers/i2c/ast2600_i2c.h | 120 > > 5 files changed, 512 insertions(+) > > create mode 100644 drivers/i2c/ast2600_i2c.c create mode 100644 > > drivers/i2c/ast2600_i2c.h > > Is this a new version? > > Regards, > Simon
Re: [PATCH 3/8] sunxi: SPL SPI: allow multiple boot attempt
于 2023年1月15日 GMT+08:00 上午3:56:08, Samuel Holland 写到:
>On 10/13/22 22:05, Icenowy Zheng wrote:
>> As we're going to add support for SPI NAND to this code, add code that
>> allows multiple boot attempts with different load offsets and functions.
>>
>> To keep compatibility with loading raw binary on SPI NOR, a bool
>> parameter is used to allow booting without valid magic number when
>> booting with SPI NOR.
>
>So the issue is that when CONFIG_SPL_RAW_IMAGE_SUPPORT=y, then
>spl_parse_image_header() will return 0 even when using the wrong NAND
>parameters? I don't see a better solution, so:
Good point, maybe the next patch needs to add some dependency
of raw image support to SPI NAND support option.
>
>Reviewed-by: Samuel Holland
>Tested-by: Samuel Holland # Orange Pi Zero Plus
>
>> Signed-off-by: Icenowy Zheng
>> ---
>> arch/arm/mach-sunxi/spl_spi_sunxi.c | 58 +++--
>> 1 file changed, 38 insertions(+), 20 deletions(-)
>>
>> diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c
>> b/arch/arm/mach-sunxi/spl_spi_sunxi.c
>> index 88c15a3ee9..21be33a23f 100644
>> --- a/arch/arm/mach-sunxi/spl_spi_sunxi.c
>> +++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c
>> @@ -340,8 +340,8 @@ static void spi0_read_data(void *buf, u32 addr, u32 len,
>> u32 addr_len)
>> }
>> }
>>
>> -static ulong spi_load_read(struct spl_load_info *load, ulong sector,
>> - ulong count, void *buf)
>> +static ulong spi_load_read_nor(struct spl_load_info *load, ulong sector,
>> + ulong count, void *buf)
>> {
>> spi0_read_data(buf, sector, count, 3);
>>
>> @@ -350,41 +350,59 @@ static ulong spi_load_read(struct spl_load_info *load,
>> ulong sector,
>>
>>
>> /*/
>>
>> -static int spl_spi_load_image(struct spl_image_info *spl_image,
>> - struct spl_boot_device *bootdev)
>> +static int spl_spi_try_load(struct spl_image_info *spl_image,
>> +struct spl_boot_device *bootdev,
>> +struct spl_load_info *load, u32 offset,
>> +bool allow_raw)
>> {
>> int ret = 0;
>> struct legacy_img_hdr *header;
>> -uint32_t load_offset = sunxi_get_spl_size();
>> -
>
>nit: keep this blank line
>
>> header = (struct legacy_img_hdr *)CONFIG_SYS_TEXT_BASE;
>> -load_offset = max_t(uint32_t, load_offset, CONFIG_SYS_SPI_U_BOOT_OFFS);
>> -
>> -spi0_init();
>>
>> -spi0_read_data((void *)header, load_offset, 0x40, 3);
>> +if (load->read(load, offset, 0x40, (void *)header) == 0)
>> +return -EINVAL;
>>
>> if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
>> image_get_magic(header) == FDT_MAGIC) {
>> -struct spl_load_info load;
>>
>> debug("Found FIT image\n");
>> -load.dev = NULL;
>> -load.priv = NULL;
>> -load.filename = NULL;
>> -load.bl_len = 1;
>> -load.read = spi_load_read;
>> -ret = spl_load_simple_fit(spl_image, ,
>> - load_offset, header);
>> +ret = spl_load_simple_fit(spl_image, load,
>> + offset, header);
>> } else {
>> +if (!allow_raw && image_get_magic(header) != IH_MAGIC)
>> +return -EINVAL;
>> +
>> ret = spl_parse_image_header(spl_image, bootdev, header);
>> if (ret)
>> return ret;
>>
>> -spi0_read_data((void *)spl_image->load_addr,
>> - load_offset, spl_image->size, 3);
>> +if (load->read(load, offset, spl_image->size,
>> + (void *)spl_image->load_addr) == 0)
>> +ret = -EINVAL;
>> }
>>
>> +return ret;
>> +}
>> +
>> +static int spl_spi_load_image(struct spl_image_info *spl_image,
>> + struct spl_boot_device *bootdev)
>> +{
>> +int ret = 0;
>> +uint32_t load_offset = sunxi_get_spl_size();
>> +struct spl_load_info load;
>> +
>> +load_offset = max_t(uint32_t, load_offset, CONFIG_SYS_SPI_U_BOOT_OFFS);
>> +
>> +load.dev = NULL;
>> +load.priv = NULL;
>> +load.filename = NULL;
>> +load.bl_len = 1;
>> +
>> +spi0_init();
>> +
>> +load.read = spi_load_read_nor;
>> +ret = spl_spi_try_load(spl_image, bootdev, , load_offset, true);
>> +
>> spi0_deinit();
>>
>> return ret;
>
[PATCH] odroid: limit boot memory to lowmem
Commit 4963f63fe61f15329d77472a762b1d8bf754d24b changed the default value of the `bootm_size` environment variable. For the odroid board this caused the initrd and fdt to be placed in highmem. This patch restricts the boot data conservatively to 256MB. Signed-off-by: Joost van Zwieten --- include/configs/odroid.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/configs/odroid.h b/include/configs/odroid.h index 560a23c23e..7255efc2df 100644 --- a/include/configs/odroid.h +++ b/include/configs/odroid.h @@ -17,6 +17,7 @@ #define CFG_SYS_PL310_BASE 0x10502000 #endif +#define CFG_SYS_BOOTMAPSZ 0x1000 #define CFG_SYS_SDRAM_BASE 0x4000 #define SDRAM_BANK_SIZE (256 << 20) /* 256 MB */ #define PHYS_SDRAM_1CFG_SYS_SDRAM_BASE -- 2.30.2
Re: kwboot: UART booting with bin_hdr u-boot (Marvell Armada 385)
On Sat, Jan 14, 2023 at 2:53 PM Pali Rohár wrote:
>
> On Saturday 14 January 2023 14:48:28 Tony Dinh wrote:
> > Hi Pali,
> >
> > On Sat, Jan 14, 2023 at 2:12 PM Tony Dinh wrote:
> > >
> > > Hi Pali,
> > >
> > > On Sat, Jan 14, 2023 at 1:44 PM Pali Rohár wrote:
> > > >
> > > > On Saturday 14 January 2023 13:35:08 Tony Dinh wrote:
> > > > > Hi Pali,
> > > > >
> > > > > On Fri, Jan 13, 2023 at 5:32 PM Pali Rohár wrote:
> > > > > >
> > > > > > On Wednesday 11 January 2023 21:56:41 Pali Rohár wrote:
> > > > > > > On Wednesday 11 January 2023 12:44:45 Tony Dinh wrote:
> > > > > > > > Hi Pali,
> > > > > > > >
> > > > > > > > On Wed, Jan 11, 2023 at 12:04 AM Pali Rohár
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > On Tuesday 10 January 2023 21:07:45 Tony Dinh wrote:
> > > > > > > > > > Hi Pali,
> > > > > > > > > >
> > > > > > > > > > I got burned for being lazy :) it turned out that the mix
> > > > > > > > > > of #ifdef
> > > > > > > > > > and #if defined was the problem. Just stepped away and came
> > > > > > > > > > back for a
> > > > > > > > > > few minutes and I can see that I just need to define the
> > > > > > > > > > CONFIG_DDR4
> > > > > > > > > > properly in arch/arm/mach-mvebu/Kconfig and build it to
> > > > > > > > > > pass the
> > > > > > > > > > CONFIG_DDR4 stuff.
> > > > > > > > > >
> > > > > > > > > > One more small hurdle after that was
> > > > > > > > > > CONFIG_USE_PRIVATE_LIBGCC must be
> > > > > > > > > > undefined for it to build (so I am using
> > > > > > > > > > /usr/lib/gcc/arm-linux-gnueabi/10/libgcc.a, and not using
> > > > > > > > > > arch/arm/lib/lib.a)
> > > > > > > > >
> > > > > > > > > Hello! It is normally a good idea to unset
> > > > > > > > > CONFIG_USE_PRIVATE_LIBGCC
> > > > > > > > > unless you have compiled gcc for target CPU.
> > > > > > > >
> > > > > > > > CONFIG_USE_PRIVATE_LIBGCC was set as a default for ARM target,
> > > > > > > > since
> > > > > > > > u-boot has arch/arm/lib/lib.a. But using arch/arm/lib/lib.a I
> > > > > > > > got
> > > > > > > > several build errors like these:
> > > > > > > >
> > > > > > > > ld.bfd:
> > > > > > > > drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.o: in
> > > > > > > > function `mv_ddr4_copt_get':
> > > > > > > > /usr/src/u-boot/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:809:
> > > > > > > > undefined reference to `__aeabi_i2d'
> > > > > > > > ld.bfd:
> > > > > > > > /usr/src/u-boot/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:809:
> > > > > > > > undefined reference to `__aeabi_dmul'
> > > > > > > > ld.bfd:
> > > > > > > > /usr/src/u-boot/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:809:
> > > > > > > > undefined reference to `__aeabi_d2uiz'
> > > > > > >
> > > > > > > This looks like a bug in that training code. I would need to see
> > > > > > > source
> > > > > > > code, so I can figure out how to fix it.
> > > > > >
> > > > > > Have you solved this issue? Or if not, can you show what you had on
> > > > > > that
> > > > > > problematic line 809?
> > > > >
> > > > > No, I have not and actually have no idea what that code does! And I
> > > > > thought you recognized the error, so I submitted the DDR4 patch so you
> > > > > can compile and see the error. Here is the code snipet, the error is
> > > > > now at 717.
> > > > >
> > > > > # grep -n10 PBS_VALUE_FACTOR
> > > > > /usr/src/u-boot-master/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c
> > > > > 704- u8 copt_val;
> > > > > 705- u8 dq_idx;
> > > > > 706- u8 center_zone_max_low = 0;
> > > > > 707- u8 center_zone_min_high = 128;
> > > > > 708- u8 vw_zone_max_low = 0;
> > > > > 709- u8 vw_zone_min_high = 128;
> > > > > 710- u8 min_vw = 63; /* minimum valid window between all bits */
> > > > > 711- u8 center_low_el;
> > > > > 712- u8 center_high_el;
> > > > > 713-
> > > > > 714: /* lambda calculated as D * PBS_VALUE_FACTOR / d */
> > > > > 715- //printf("Copt::Debug::\t");
> > > > > 716- for (dq_idx = 0; dq_idx < 8; dq_idx++) {
> > > > > 717- center_per_dq[dq_idx] = 0.5 * (vw_h[dq_idx] + vw_l[dq_idx]);
> > > > > 718- vw_per_dq[dq_idx] = 1 + (vw_h[dq_idx] - vw_l[dq_idx]);
> > > > > 719- if (min_vw > vw_per_dq[dq_idx])
> > > > > 720- min_vw = vw_per_dq[dq_idx];
> > > > > 721- }
> > > > > 722-
> > > > > 723- /* calculate center zone */
> > > > > 724- for (dq_idx = 0; dq_idx < 8; dq_idx++) {
> > > >
> > > > Haha! That is pretty simple. On line 717 you have fractional number 0.5
> > > > which is represented by floating point and all numeric operation on that
> > > > line are done as floating point.
> > > >
> > > > U-Boot and kernel does not floating point HW and instruct compiler to
> > > > replace all floating point operations by function calls (which implement
> > > > software floating point support).
> > > >
> > > > U-Boot (for very good reasons) do not implement any floating point
> > > > operations.
> > > >
> > > > Fix should be very simple, use only integer operations. So replace
> > > >
> > > > 0.5 *
Re: kwboot: UART booting with bin_hdr u-boot (Marvell Armada 385)
On Saturday 14 January 2023 14:48:28 Tony Dinh wrote:
> Hi Pali,
>
> On Sat, Jan 14, 2023 at 2:12 PM Tony Dinh wrote:
> >
> > Hi Pali,
> >
> > On Sat, Jan 14, 2023 at 1:44 PM Pali Rohár wrote:
> > >
> > > On Saturday 14 January 2023 13:35:08 Tony Dinh wrote:
> > > > Hi Pali,
> > > >
> > > > On Fri, Jan 13, 2023 at 5:32 PM Pali Rohár wrote:
> > > > >
> > > > > On Wednesday 11 January 2023 21:56:41 Pali Rohár wrote:
> > > > > > On Wednesday 11 January 2023 12:44:45 Tony Dinh wrote:
> > > > > > > Hi Pali,
> > > > > > >
> > > > > > > On Wed, Jan 11, 2023 at 12:04 AM Pali Rohár
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > On Tuesday 10 January 2023 21:07:45 Tony Dinh wrote:
> > > > > > > > > Hi Pali,
> > > > > > > > >
> > > > > > > > > I got burned for being lazy :) it turned out that the mix of
> > > > > > > > > #ifdef
> > > > > > > > > and #if defined was the problem. Just stepped away and came
> > > > > > > > > back for a
> > > > > > > > > few minutes and I can see that I just need to define the
> > > > > > > > > CONFIG_DDR4
> > > > > > > > > properly in arch/arm/mach-mvebu/Kconfig and build it to pass
> > > > > > > > > the
> > > > > > > > > CONFIG_DDR4 stuff.
> > > > > > > > >
> > > > > > > > > One more small hurdle after that was
> > > > > > > > > CONFIG_USE_PRIVATE_LIBGCC must be
> > > > > > > > > undefined for it to build (so I am using
> > > > > > > > > /usr/lib/gcc/arm-linux-gnueabi/10/libgcc.a, and not using
> > > > > > > > > arch/arm/lib/lib.a)
> > > > > > > >
> > > > > > > > Hello! It is normally a good idea to unset
> > > > > > > > CONFIG_USE_PRIVATE_LIBGCC
> > > > > > > > unless you have compiled gcc for target CPU.
> > > > > > >
> > > > > > > CONFIG_USE_PRIVATE_LIBGCC was set as a default for ARM target,
> > > > > > > since
> > > > > > > u-boot has arch/arm/lib/lib.a. But using arch/arm/lib/lib.a I got
> > > > > > > several build errors like these:
> > > > > > >
> > > > > > > ld.bfd: drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.o:
> > > > > > > in
> > > > > > > function `mv_ddr4_copt_get':
> > > > > > > /usr/src/u-boot/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:809:
> > > > > > > undefined reference to `__aeabi_i2d'
> > > > > > > ld.bfd:
> > > > > > > /usr/src/u-boot/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:809:
> > > > > > > undefined reference to `__aeabi_dmul'
> > > > > > > ld.bfd:
> > > > > > > /usr/src/u-boot/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:809:
> > > > > > > undefined reference to `__aeabi_d2uiz'
> > > > > >
> > > > > > This looks like a bug in that training code. I would need to see
> > > > > > source
> > > > > > code, so I can figure out how to fix it.
> > > > >
> > > > > Have you solved this issue? Or if not, can you show what you had on
> > > > > that
> > > > > problematic line 809?
> > > >
> > > > No, I have not and actually have no idea what that code does! And I
> > > > thought you recognized the error, so I submitted the DDR4 patch so you
> > > > can compile and see the error. Here is the code snipet, the error is
> > > > now at 717.
> > > >
> > > > # grep -n10 PBS_VALUE_FACTOR
> > > > /usr/src/u-boot-master/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c
> > > > 704- u8 copt_val;
> > > > 705- u8 dq_idx;
> > > > 706- u8 center_zone_max_low = 0;
> > > > 707- u8 center_zone_min_high = 128;
> > > > 708- u8 vw_zone_max_low = 0;
> > > > 709- u8 vw_zone_min_high = 128;
> > > > 710- u8 min_vw = 63; /* minimum valid window between all bits */
> > > > 711- u8 center_low_el;
> > > > 712- u8 center_high_el;
> > > > 713-
> > > > 714: /* lambda calculated as D * PBS_VALUE_FACTOR / d */
> > > > 715- //printf("Copt::Debug::\t");
> > > > 716- for (dq_idx = 0; dq_idx < 8; dq_idx++) {
> > > > 717- center_per_dq[dq_idx] = 0.5 * (vw_h[dq_idx] + vw_l[dq_idx]);
> > > > 718- vw_per_dq[dq_idx] = 1 + (vw_h[dq_idx] - vw_l[dq_idx]);
> > > > 719- if (min_vw > vw_per_dq[dq_idx])
> > > > 720- min_vw = vw_per_dq[dq_idx];
> > > > 721- }
> > > > 722-
> > > > 723- /* calculate center zone */
> > > > 724- for (dq_idx = 0; dq_idx < 8; dq_idx++) {
> > >
> > > Haha! That is pretty simple. On line 717 you have fractional number 0.5
> > > which is represented by floating point and all numeric operation on that
> > > line are done as floating point.
> > >
> > > U-Boot and kernel does not floating point HW and instruct compiler to
> > > replace all floating point operations by function calls (which implement
> > > software floating point support).
> > >
> > > U-Boot (for very good reasons) do not implement any floating point
> > > operations.
> > >
> > > Fix should be very simple, use only integer operations. So replace
> > >
> > > 0.5 * something
> > >
> > > by:
> > >
> > > something / 2
> > >
> > > And check if it compiles and if it works.
> > >
> > > I'm surprised that Marvell was trying to do floating point...
> > >
> >
> > Yes!!! that was impressive how quick you saw the root cause. I've
> > replaced those floating point
Re: kwboot: UART booting with bin_hdr u-boot (Marvell Armada 385)
Hi Pali,
On Sat, Jan 14, 2023 at 2:12 PM Tony Dinh wrote:
>
> Hi Pali,
>
> On Sat, Jan 14, 2023 at 1:44 PM Pali Rohár wrote:
> >
> > On Saturday 14 January 2023 13:35:08 Tony Dinh wrote:
> > > Hi Pali,
> > >
> > > On Fri, Jan 13, 2023 at 5:32 PM Pali Rohár wrote:
> > > >
> > > > On Wednesday 11 January 2023 21:56:41 Pali Rohár wrote:
> > > > > On Wednesday 11 January 2023 12:44:45 Tony Dinh wrote:
> > > > > > Hi Pali,
> > > > > >
> > > > > > On Wed, Jan 11, 2023 at 12:04 AM Pali Rohár wrote:
> > > > > > >
> > > > > > > On Tuesday 10 January 2023 21:07:45 Tony Dinh wrote:
> > > > > > > > Hi Pali,
> > > > > > > >
> > > > > > > > I got burned for being lazy :) it turned out that the mix of
> > > > > > > > #ifdef
> > > > > > > > and #if defined was the problem. Just stepped away and came
> > > > > > > > back for a
> > > > > > > > few minutes and I can see that I just need to define the
> > > > > > > > CONFIG_DDR4
> > > > > > > > properly in arch/arm/mach-mvebu/Kconfig and build it to pass the
> > > > > > > > CONFIG_DDR4 stuff.
> > > > > > > >
> > > > > > > > One more small hurdle after that was CONFIG_USE_PRIVATE_LIBGCC
> > > > > > > > must be
> > > > > > > > undefined for it to build (so I am using
> > > > > > > > /usr/lib/gcc/arm-linux-gnueabi/10/libgcc.a, and not using
> > > > > > > > arch/arm/lib/lib.a)
> > > > > > >
> > > > > > > Hello! It is normally a good idea to unset
> > > > > > > CONFIG_USE_PRIVATE_LIBGCC
> > > > > > > unless you have compiled gcc for target CPU.
> > > > > >
> > > > > > CONFIG_USE_PRIVATE_LIBGCC was set as a default for ARM target, since
> > > > > > u-boot has arch/arm/lib/lib.a. But using arch/arm/lib/lib.a I got
> > > > > > several build errors like these:
> > > > > >
> > > > > > ld.bfd: drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.o: in
> > > > > > function `mv_ddr4_copt_get':
> > > > > > /usr/src/u-boot/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:809:
> > > > > > undefined reference to `__aeabi_i2d'
> > > > > > ld.bfd:
> > > > > > /usr/src/u-boot/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:809:
> > > > > > undefined reference to `__aeabi_dmul'
> > > > > > ld.bfd:
> > > > > > /usr/src/u-boot/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:809:
> > > > > > undefined reference to `__aeabi_d2uiz'
> > > > >
> > > > > This looks like a bug in that training code. I would need to see
> > > > > source
> > > > > code, so I can figure out how to fix it.
> > > >
> > > > Have you solved this issue? Or if not, can you show what you had on that
> > > > problematic line 809?
> > >
> > > No, I have not and actually have no idea what that code does! And I
> > > thought you recognized the error, so I submitted the DDR4 patch so you
> > > can compile and see the error. Here is the code snipet, the error is
> > > now at 717.
> > >
> > > # grep -n10 PBS_VALUE_FACTOR
> > > /usr/src/u-boot-master/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c
> > > 704- u8 copt_val;
> > > 705- u8 dq_idx;
> > > 706- u8 center_zone_max_low = 0;
> > > 707- u8 center_zone_min_high = 128;
> > > 708- u8 vw_zone_max_low = 0;
> > > 709- u8 vw_zone_min_high = 128;
> > > 710- u8 min_vw = 63; /* minimum valid window between all bits */
> > > 711- u8 center_low_el;
> > > 712- u8 center_high_el;
> > > 713-
> > > 714: /* lambda calculated as D * PBS_VALUE_FACTOR / d */
> > > 715- //printf("Copt::Debug::\t");
> > > 716- for (dq_idx = 0; dq_idx < 8; dq_idx++) {
> > > 717- center_per_dq[dq_idx] = 0.5 * (vw_h[dq_idx] + vw_l[dq_idx]);
> > > 718- vw_per_dq[dq_idx] = 1 + (vw_h[dq_idx] - vw_l[dq_idx]);
> > > 719- if (min_vw > vw_per_dq[dq_idx])
> > > 720- min_vw = vw_per_dq[dq_idx];
> > > 721- }
> > > 722-
> > > 723- /* calculate center zone */
> > > 724- for (dq_idx = 0; dq_idx < 8; dq_idx++) {
> >
> > Haha! That is pretty simple. On line 717 you have fractional number 0.5
> > which is represented by floating point and all numeric operation on that
> > line are done as floating point.
> >
> > U-Boot and kernel does not floating point HW and instruct compiler to
> > replace all floating point operations by function calls (which implement
> > software floating point support).
> >
> > U-Boot (for very good reasons) do not implement any floating point
> > operations.
> >
> > Fix should be very simple, use only integer operations. So replace
> >
> > 0.5 * something
> >
> > by:
> >
> > something / 2
> >
> > And check if it compiles and if it works.
> >
> > I'm surprised that Marvell was trying to do floating point...
> >
>
> Yes!!! that was impressive how quick you saw the root cause. I've
> replaced those floating point operations with integer operations and
> it built fine. Let see it will run.
Indeed! It runs without problem with the changes below.
diff --git a/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c
b/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c
index e4e86c6f2e..7e596ce78a 100644
--- a/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c
+++
Re: kwboot: UART booting with bin_hdr u-boot (Marvell Armada 385)
Hi Pali,
On Sat, Jan 14, 2023 at 1:44 PM Pali Rohár wrote:
>
> On Saturday 14 January 2023 13:35:08 Tony Dinh wrote:
> > Hi Pali,
> >
> > On Fri, Jan 13, 2023 at 5:32 PM Pali Rohár wrote:
> > >
> > > On Wednesday 11 January 2023 21:56:41 Pali Rohár wrote:
> > > > On Wednesday 11 January 2023 12:44:45 Tony Dinh wrote:
> > > > > Hi Pali,
> > > > >
> > > > > On Wed, Jan 11, 2023 at 12:04 AM Pali Rohár wrote:
> > > > > >
> > > > > > On Tuesday 10 January 2023 21:07:45 Tony Dinh wrote:
> > > > > > > Hi Pali,
> > > > > > >
> > > > > > > I got burned for being lazy :) it turned out that the mix of
> > > > > > > #ifdef
> > > > > > > and #if defined was the problem. Just stepped away and came back
> > > > > > > for a
> > > > > > > few minutes and I can see that I just need to define the
> > > > > > > CONFIG_DDR4
> > > > > > > properly in arch/arm/mach-mvebu/Kconfig and build it to pass the
> > > > > > > CONFIG_DDR4 stuff.
> > > > > > >
> > > > > > > One more small hurdle after that was CONFIG_USE_PRIVATE_LIBGCC
> > > > > > > must be
> > > > > > > undefined for it to build (so I am using
> > > > > > > /usr/lib/gcc/arm-linux-gnueabi/10/libgcc.a, and not using
> > > > > > > arch/arm/lib/lib.a)
> > > > > >
> > > > > > Hello! It is normally a good idea to unset CONFIG_USE_PRIVATE_LIBGCC
> > > > > > unless you have compiled gcc for target CPU.
> > > > >
> > > > > CONFIG_USE_PRIVATE_LIBGCC was set as a default for ARM target, since
> > > > > u-boot has arch/arm/lib/lib.a. But using arch/arm/lib/lib.a I got
> > > > > several build errors like these:
> > > > >
> > > > > ld.bfd: drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.o: in
> > > > > function `mv_ddr4_copt_get':
> > > > > /usr/src/u-boot/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:809:
> > > > > undefined reference to `__aeabi_i2d'
> > > > > ld.bfd:
> > > > > /usr/src/u-boot/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:809:
> > > > > undefined reference to `__aeabi_dmul'
> > > > > ld.bfd:
> > > > > /usr/src/u-boot/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:809:
> > > > > undefined reference to `__aeabi_d2uiz'
> > > >
> > > > This looks like a bug in that training code. I would need to see source
> > > > code, so I can figure out how to fix it.
> > >
> > > Have you solved this issue? Or if not, can you show what you had on that
> > > problematic line 809?
> >
> > No, I have not and actually have no idea what that code does! And I
> > thought you recognized the error, so I submitted the DDR4 patch so you
> > can compile and see the error. Here is the code snipet, the error is
> > now at 717.
> >
> > # grep -n10 PBS_VALUE_FACTOR
> > /usr/src/u-boot-master/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c
> > 704- u8 copt_val;
> > 705- u8 dq_idx;
> > 706- u8 center_zone_max_low = 0;
> > 707- u8 center_zone_min_high = 128;
> > 708- u8 vw_zone_max_low = 0;
> > 709- u8 vw_zone_min_high = 128;
> > 710- u8 min_vw = 63; /* minimum valid window between all bits */
> > 711- u8 center_low_el;
> > 712- u8 center_high_el;
> > 713-
> > 714: /* lambda calculated as D * PBS_VALUE_FACTOR / d */
> > 715- //printf("Copt::Debug::\t");
> > 716- for (dq_idx = 0; dq_idx < 8; dq_idx++) {
> > 717- center_per_dq[dq_idx] = 0.5 * (vw_h[dq_idx] + vw_l[dq_idx]);
> > 718- vw_per_dq[dq_idx] = 1 + (vw_h[dq_idx] - vw_l[dq_idx]);
> > 719- if (min_vw > vw_per_dq[dq_idx])
> > 720- min_vw = vw_per_dq[dq_idx];
> > 721- }
> > 722-
> > 723- /* calculate center zone */
> > 724- for (dq_idx = 0; dq_idx < 8; dq_idx++) {
>
> Haha! That is pretty simple. On line 717 you have fractional number 0.5
> which is represented by floating point and all numeric operation on that
> line are done as floating point.
>
> U-Boot and kernel does not floating point HW and instruct compiler to
> replace all floating point operations by function calls (which implement
> software floating point support).
>
> U-Boot (for very good reasons) do not implement any floating point
> operations.
>
> Fix should be very simple, use only integer operations. So replace
>
> 0.5 * something
>
> by:
>
> something / 2
>
> And check if it compiles and if it works.
>
> I'm surprised that Marvell was trying to do floating point...
>
Yes!!! that was impressive how quick you saw the root cause. I've
replaced those floating point operations with integer operations and
it built fine. Let see it will run.
Thanks,
Tony
> > Here are the build errors with the current code.
> >
> >
> > rm -f spl/common/spl/built-in.o; ar cDPrsT spl/common/spl/built-in.o
> > spl/common/spl/spl.o spl/common/spl/spl_bootrom.o
> > spl/common/spl/spl_spi.o
> > ( cd spl && ld.bfd -T u-boot-spl.lds --gc-sections -Bstatic
> > --gc-sections --no-dynamic-linker --build-id=none -Ttext 0x4030
> > arch/arm/cpu/armv7/start.o --whole-archive
> > arch/arm/mach-mvebu/built-in.o arch/arm/cpu/armv7/built-in.o
> > arch/arm/cpu/built-in.o arch/arm/lib/built-in.o
> > board/thecus/n2350/built-in.o common/spl/built-in.o
> >
Re: [PATCH] ddr: marvell: a38x: Add support for DDR4 from Marvell mv-ddr-marvell repository
Hi Pali & Tom, On Sat, Jan 14, 2023 at 12:06 PM Pali Rohár wrote: > > On Saturday 14 January 2023 15:03:41 Tom Rini wrote: > > On Sat, Jan 14, 2023 at 07:51:00PM +0100, Pali Rohár wrote: > > > On Friday 13 January 2023 21:00:21 Tom Rini wrote: > > > > On Sat, Jan 14, 2023 at 02:41:32AM +0100, Pali Rohár wrote: > > > > > On Friday 13 January 2023 16:38:55 Tony Dinh wrote: > > > > > > @@ -16,4 +19,9 @@ obj-$(CONFIG_SPL_BUILD) += mv_ddr_build_message.o > > > > > > obj-$(CONFIG_SPL_BUILD) += mv_ddr_common.o > > > > > > obj-$(CONFIG_SPL_BUILD) += mv_ddr_spd.o > > > > > > obj-$(CONFIG_SPL_BUILD) += mv_ddr_topology.o > > > > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_mpr_pda_if.o > > > > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_training.o > > > > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_training_calibration.o > > > > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_training_db.o > > > > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_training_leveling.o > > > > > > obj-$(CONFIG_SPL_BUILD) += xor.o > > > > > > > > > > And all these new files are ddr4 specific, so should be wrapped in > > > > > makefile section: > > > > > ifdef CONFIG_DDR4 > > > > > > > > Looking at the Makefile in question, I think we might want to make the > > > > whole thing ifdef CONFIG_SPL_BUILD ... endif and then more finely > > > > control building of what objects are built. Perhaps: > > > > drivers/Makefile:obj-$(CONFIG_ARMADA_38X) += ddr/marvell/a38x/ > > > > should only be for SPL instead, even? > > > > > > Some cleanup like this can be done. But it is related to DDR4 support > > > and is mostly independent of it. So lets do it after having DDR4 there. > > > > We're going to also want to not build the DDR3 code on the DDR4 > > platforms, right? A little clean up would make adding the DDR4 code a > > bit cleaner for both cases. It's not a hard no, if someone really wants > > to do the clean-up after. > > I can look at it _after_ all other stuff is done and merged. Thanks for the review and comments! and thanks Pali in advance for the Makefile improvement after. I will submit the V2 patches to fix the dead code and other editorial issues per Pali's review. All the best, Tony
Re: kwboot: UART booting with bin_hdr u-boot (Marvell Armada 385)
On Saturday 14 January 2023 13:35:08 Tony Dinh wrote:
> Hi Pali,
>
> On Fri, Jan 13, 2023 at 5:32 PM Pali Rohár wrote:
> >
> > On Wednesday 11 January 2023 21:56:41 Pali Rohár wrote:
> > > On Wednesday 11 January 2023 12:44:45 Tony Dinh wrote:
> > > > Hi Pali,
> > > >
> > > > On Wed, Jan 11, 2023 at 12:04 AM Pali Rohár wrote:
> > > > >
> > > > > On Tuesday 10 January 2023 21:07:45 Tony Dinh wrote:
> > > > > > Hi Pali,
> > > > > >
> > > > > > I got burned for being lazy :) it turned out that the mix of #ifdef
> > > > > > and #if defined was the problem. Just stepped away and came back
> > > > > > for a
> > > > > > few minutes and I can see that I just need to define the CONFIG_DDR4
> > > > > > properly in arch/arm/mach-mvebu/Kconfig and build it to pass the
> > > > > > CONFIG_DDR4 stuff.
> > > > > >
> > > > > > One more small hurdle after that was CONFIG_USE_PRIVATE_LIBGCC must
> > > > > > be
> > > > > > undefined for it to build (so I am using
> > > > > > /usr/lib/gcc/arm-linux-gnueabi/10/libgcc.a, and not using
> > > > > > arch/arm/lib/lib.a)
> > > > >
> > > > > Hello! It is normally a good idea to unset CONFIG_USE_PRIVATE_LIBGCC
> > > > > unless you have compiled gcc for target CPU.
> > > >
> > > > CONFIG_USE_PRIVATE_LIBGCC was set as a default for ARM target, since
> > > > u-boot has arch/arm/lib/lib.a. But using arch/arm/lib/lib.a I got
> > > > several build errors like these:
> > > >
> > > > ld.bfd: drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.o: in
> > > > function `mv_ddr4_copt_get':
> > > > /usr/src/u-boot/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:809:
> > > > undefined reference to `__aeabi_i2d'
> > > > ld.bfd:
> > > > /usr/src/u-boot/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:809:
> > > > undefined reference to `__aeabi_dmul'
> > > > ld.bfd:
> > > > /usr/src/u-boot/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:809:
> > > > undefined reference to `__aeabi_d2uiz'
> > >
> > > This looks like a bug in that training code. I would need to see source
> > > code, so I can figure out how to fix it.
> >
> > Have you solved this issue? Or if not, can you show what you had on that
> > problematic line 809?
>
> No, I have not and actually have no idea what that code does! And I
> thought you recognized the error, so I submitted the DDR4 patch so you
> can compile and see the error. Here is the code snipet, the error is
> now at 717.
>
> # grep -n10 PBS_VALUE_FACTOR
> /usr/src/u-boot-master/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c
> 704- u8 copt_val;
> 705- u8 dq_idx;
> 706- u8 center_zone_max_low = 0;
> 707- u8 center_zone_min_high = 128;
> 708- u8 vw_zone_max_low = 0;
> 709- u8 vw_zone_min_high = 128;
> 710- u8 min_vw = 63; /* minimum valid window between all bits */
> 711- u8 center_low_el;
> 712- u8 center_high_el;
> 713-
> 714: /* lambda calculated as D * PBS_VALUE_FACTOR / d */
> 715- //printf("Copt::Debug::\t");
> 716- for (dq_idx = 0; dq_idx < 8; dq_idx++) {
> 717- center_per_dq[dq_idx] = 0.5 * (vw_h[dq_idx] + vw_l[dq_idx]);
> 718- vw_per_dq[dq_idx] = 1 + (vw_h[dq_idx] - vw_l[dq_idx]);
> 719- if (min_vw > vw_per_dq[dq_idx])
> 720- min_vw = vw_per_dq[dq_idx];
> 721- }
> 722-
> 723- /* calculate center zone */
> 724- for (dq_idx = 0; dq_idx < 8; dq_idx++) {
Haha! That is pretty simple. On line 717 you have fractional number 0.5
which is represented by floating point and all numeric operation on that
line are done as floating point.
U-Boot and kernel does not floating point HW and instruct compiler to
replace all floating point operations by function calls (which implement
software floating point support).
U-Boot (for very good reasons) do not implement any floating point
operations.
Fix should be very simple, use only integer operations. So replace
0.5 * something
by:
something / 2
And check if it compiles and if it works.
I'm surprised that Marvell was trying to do floating point...
> Here are the build errors with the current code.
>
>
> rm -f spl/common/spl/built-in.o; ar cDPrsT spl/common/spl/built-in.o
> spl/common/spl/spl.o spl/common/spl/spl_bootrom.o
> spl/common/spl/spl_spi.o
> ( cd spl && ld.bfd -T u-boot-spl.lds --gc-sections -Bstatic
> --gc-sections --no-dynamic-linker --build-id=none -Ttext 0x4030
> arch/arm/cpu/armv7/start.o --whole-archive
> arch/arm/mach-mvebu/built-in.o arch/arm/cpu/armv7/built-in.o
> arch/arm/cpu/built-in.o arch/arm/lib/built-in.o
> board/thecus/n2350/built-in.o common/spl/built-in.o
> common/init/built-in.o boot/built-in.o common/built-in.o
> cmd/built-in.o env/built-in.o lib/built-in.o disk/built-in.o
> drivers/built-in.o dts/built-in.o fs/built-in.o --no-whole-archive
> arch/arm/lib/eabi_compat.o arch/arm/lib/lib.a -Map u-boot-spl.map -o
> u-boot-spl )
> ld.bfd: drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.o: in
> function `mv_ddr4_copt_get':
> /usr/src/u-boot-master/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:717:
> undefined reference to
Re: kwboot: UART booting with bin_hdr u-boot (Marvell Armada 385)
Hi Pali,
On Fri, Jan 13, 2023 at 5:32 PM Pali Rohár wrote:
>
> On Wednesday 11 January 2023 21:56:41 Pali Rohár wrote:
> > On Wednesday 11 January 2023 12:44:45 Tony Dinh wrote:
> > > Hi Pali,
> > >
> > > On Wed, Jan 11, 2023 at 12:04 AM Pali Rohár wrote:
> > > >
> > > > On Tuesday 10 January 2023 21:07:45 Tony Dinh wrote:
> > > > > Hi Pali,
> > > > >
> > > > > I got burned for being lazy :) it turned out that the mix of #ifdef
> > > > > and #if defined was the problem. Just stepped away and came back for a
> > > > > few minutes and I can see that I just need to define the CONFIG_DDR4
> > > > > properly in arch/arm/mach-mvebu/Kconfig and build it to pass the
> > > > > CONFIG_DDR4 stuff.
> > > > >
> > > > > One more small hurdle after that was CONFIG_USE_PRIVATE_LIBGCC must be
> > > > > undefined for it to build (so I am using
> > > > > /usr/lib/gcc/arm-linux-gnueabi/10/libgcc.a, and not using
> > > > > arch/arm/lib/lib.a)
> > > >
> > > > Hello! It is normally a good idea to unset CONFIG_USE_PRIVATE_LIBGCC
> > > > unless you have compiled gcc for target CPU.
> > >
> > > CONFIG_USE_PRIVATE_LIBGCC was set as a default for ARM target, since
> > > u-boot has arch/arm/lib/lib.a. But using arch/arm/lib/lib.a I got
> > > several build errors like these:
> > >
> > > ld.bfd: drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.o: in
> > > function `mv_ddr4_copt_get':
> > > /usr/src/u-boot/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:809:
> > > undefined reference to `__aeabi_i2d'
> > > ld.bfd:
> > > /usr/src/u-boot/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:809:
> > > undefined reference to `__aeabi_dmul'
> > > ld.bfd:
> > > /usr/src/u-boot/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:809:
> > > undefined reference to `__aeabi_d2uiz'
> >
> > This looks like a bug in that training code. I would need to see source
> > code, so I can figure out how to fix it.
>
> Have you solved this issue? Or if not, can you show what you had on that
> problematic line 809?
No, I have not and actually have no idea what that code does! And I
thought you recognized the error, so I submitted the DDR4 patch so you
can compile and see the error. Here is the code snipet, the error is
now at 717.
# grep -n10 PBS_VALUE_FACTOR
/usr/src/u-boot-master/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c
704- u8 copt_val;
705- u8 dq_idx;
706- u8 center_zone_max_low = 0;
707- u8 center_zone_min_high = 128;
708- u8 vw_zone_max_low = 0;
709- u8 vw_zone_min_high = 128;
710- u8 min_vw = 63; /* minimum valid window between all bits */
711- u8 center_low_el;
712- u8 center_high_el;
713-
714: /* lambda calculated as D * PBS_VALUE_FACTOR / d */
715- //printf("Copt::Debug::\t");
716- for (dq_idx = 0; dq_idx < 8; dq_idx++) {
717- center_per_dq[dq_idx] = 0.5 * (vw_h[dq_idx] + vw_l[dq_idx]);
718- vw_per_dq[dq_idx] = 1 + (vw_h[dq_idx] - vw_l[dq_idx]);
719- if (min_vw > vw_per_dq[dq_idx])
720- min_vw = vw_per_dq[dq_idx];
721- }
722-
723- /* calculate center zone */
724- for (dq_idx = 0; dq_idx < 8; dq_idx++) {
Here are the build errors with the current code.
rm -f spl/common/spl/built-in.o; ar cDPrsT spl/common/spl/built-in.o
spl/common/spl/spl.o spl/common/spl/spl_bootrom.o
spl/common/spl/spl_spi.o
( cd spl && ld.bfd -T u-boot-spl.lds --gc-sections -Bstatic
--gc-sections --no-dynamic-linker --build-id=none -Ttext 0x4030
arch/arm/cpu/armv7/start.o --whole-archive
arch/arm/mach-mvebu/built-in.o arch/arm/cpu/armv7/built-in.o
arch/arm/cpu/built-in.o arch/arm/lib/built-in.o
board/thecus/n2350/built-in.o common/spl/built-in.o
common/init/built-in.o boot/built-in.o common/built-in.o
cmd/built-in.o env/built-in.o lib/built-in.o disk/built-in.o
drivers/built-in.o dts/built-in.o fs/built-in.o --no-whole-archive
arch/arm/lib/eabi_compat.o arch/arm/lib/lib.a -Map u-boot-spl.map -o
u-boot-spl )
ld.bfd: drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.o: in
function `mv_ddr4_copt_get':
/usr/src/u-boot-master/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:717:
undefined reference to `__aeabi_i2d'
ld.bfd:
/usr/src/u-boot-master/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:717:
undefined reference to `__aeabi_dmul'
ld.bfd:
/usr/src/u-boot-master/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:717:
undefined reference to `__aeabi_d2uiz'
ld.bfd:
/usr/src/u-boot-master/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:757:
undefined reference to `__aeabi_i2d'
ld.bfd:
/usr/src/u-boot-master/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:757:
undefined reference to `__aeabi_i2d'
ld.bfd:
/usr/src/u-boot-master/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:757:
undefined reference to `__aeabi_dmul'
ld.bfd:
/usr/src/u-boot-master/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:757:
undefined reference to `__aeabi_i2d'
ld.bfd:
/usr/src/u-boot-master/drivers/ddr/marvell/a38x/mv_ddr4_training_calibration.c:757:
undefined reference to `__aeabi_dadd'
ld.bfd:
Re: [PATCH v2 u-boot 2/3] powerpc/mpc85xx: socrates: Rename u-boot-socrates.bin to u-boot.bin
On Sat, Jan 14, 2023 at 10:12:06PM +0100, Pali Rohár wrote: > On Friday 13 January 2023 18:16:03 Tom Rini wrote: > > On Sat, Jan 14, 2023 at 12:11:22AM +0100, Pali Rohár wrote: > > > U-Boot build process for socrates board produces final U-Boot binary in > > > file u-boot-socrates.bin (by binman) And as a bonus it produces two > > > unusable broken binaries u-boot-dtb.bin and u-boot.bin (by Makefile). > > > > > > So do not build broken u-boot-dtb.bin and u-boot.bin binaries and rename > > > board specific u-boot-socrates.bin binary to u-boot.bin. > > > > > > Renaming requires to define a new socrates specific Makefile target for > > > u-boot.bin (via binman) and also changing output name in socrates binman > > > config file. > > > > > > With this change U-Boot build process for socrates board also produces > > > final U-Boot binary in file u-boot.bin. > > > > > > Signed-off-by: Pali Rohár > > > --- > > > Added make dependency on u-boot.dtb > > > --- > > > Makefile | 11 +++ > > > arch/powerpc/dts/socrates-u-boot.dtsi | 2 +- > > > 2 files changed, 12 insertions(+), 1 deletion(-) > > > > > > diff --git a/Makefile b/Makefile > > > index a4a14d5d35a8..5473bea25332 100644 > > > --- a/Makefile > > > +++ b/Makefile > > > @@ -1195,22 +1195,30 @@ endif > > > u-boot.bin: u-boot-fit-dtb.bin FORCE > > > $(call if_changed,copy) > > > > > > +ifneq ($(CONFIG_TARGET_SOCRATES),y) > > > u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE > > > $(call if_changed,cat) > > > +endif > > > > > > else ifeq ($(CONFIG_OF_SEPARATE).$(CONFIG_OF_OMIT_DTB),y.) > > > +ifneq ($(CONFIG_TARGET_SOCRATES),y) > > > u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE > > > $(call if_changed,cat) > > > +endif > > > > > > ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy) > > > +ifneq ($(CONFIG_TARGET_SOCRATES),y) > > > u-boot.bin: u-boot-dtb.bin FORCE > > > $(call if_changed,copy) > > > endif > > > +endif > > > > > > else ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy) > > > +ifneq ($(CONFIG_TARGET_SOCRATES),y) > > > u-boot.bin: u-boot-nodtb.bin FORCE > > > $(call if_changed,copy) > > > endif > > > +endif > > > > Simon's point from before still stands. This is the opposite of what we > > want. There must not be CONFIG_TARGET_ logic introduced to the > > top-level Makefile. socrate is "just" another mpc85xx platform, it > > doesn't have a special ROM, we need to adjust it back to acting like > > other platforms. > > socrates has its own flash layout, own build procedure and purpose of > this patch is just to prevent another breakage (like it was done in the > past) by throwing make errors. > > Trying to adjust board code and changing its layout is really not up to > me. I do not have this board. > > One there is generic binman build rules from make then it can be > switches to that generic binman rule. Until it happen there is not > better option... Yes, it should be Heiko, as the board maintainer, dealing with fixing this part. I don't understand the flash layout, and the partition table laid out in arch/powerpc/dts/socrates.dts confuses things even more to me. But, the board maintainer should be able to sort this all out. Because we do not want to add CONFIG_TARGET_ logic to the top-level Makefile. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 u-boot 2/3] powerpc/mpc85xx: socrates: Rename u-boot-socrates.bin to u-boot.bin
On Friday 13 January 2023 18:16:03 Tom Rini wrote: > On Sat, Jan 14, 2023 at 12:11:22AM +0100, Pali Rohár wrote: > > U-Boot build process for socrates board produces final U-Boot binary in > > file u-boot-socrates.bin (by binman) And as a bonus it produces two > > unusable broken binaries u-boot-dtb.bin and u-boot.bin (by Makefile). > > > > So do not build broken u-boot-dtb.bin and u-boot.bin binaries and rename > > board specific u-boot-socrates.bin binary to u-boot.bin. > > > > Renaming requires to define a new socrates specific Makefile target for > > u-boot.bin (via binman) and also changing output name in socrates binman > > config file. > > > > With this change U-Boot build process for socrates board also produces > > final U-Boot binary in file u-boot.bin. > > > > Signed-off-by: Pali Rohár > > --- > > Added make dependency on u-boot.dtb > > --- > > Makefile | 11 +++ > > arch/powerpc/dts/socrates-u-boot.dtsi | 2 +- > > 2 files changed, 12 insertions(+), 1 deletion(-) > > > > diff --git a/Makefile b/Makefile > > index a4a14d5d35a8..5473bea25332 100644 > > --- a/Makefile > > +++ b/Makefile > > @@ -1195,22 +1195,30 @@ endif > > u-boot.bin: u-boot-fit-dtb.bin FORCE > > $(call if_changed,copy) > > > > +ifneq ($(CONFIG_TARGET_SOCRATES),y) > > u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE > > $(call if_changed,cat) > > +endif > > > > else ifeq ($(CONFIG_OF_SEPARATE).$(CONFIG_OF_OMIT_DTB),y.) > > +ifneq ($(CONFIG_TARGET_SOCRATES),y) > > u-boot-dtb.bin: u-boot-nodtb.bin dts/dt.dtb FORCE > > $(call if_changed,cat) > > +endif > > > > ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy) > > +ifneq ($(CONFIG_TARGET_SOCRATES),y) > > u-boot.bin: u-boot-dtb.bin FORCE > > $(call if_changed,copy) > > endif > > +endif > > > > else ifneq ($(CONFIG_MPC85XX_HAVE_RESET_VECTOR)$(CONFIG_OF_SEPARATE),yy) > > +ifneq ($(CONFIG_TARGET_SOCRATES),y) > > u-boot.bin: u-boot-nodtb.bin FORCE > > $(call if_changed,copy) > > endif > > +endif > > Simon's point from before still stands. This is the opposite of what we > want. There must not be CONFIG_TARGET_ logic introduced to the > top-level Makefile. socrate is "just" another mpc85xx platform, it > doesn't have a special ROM, we need to adjust it back to acting like > other platforms. socrates has its own flash layout, own build procedure and purpose of this patch is just to prevent another breakage (like it was done in the past) by throwing make errors. Trying to adjust board code and changing its layout is really not up to me. I do not have this board. One there is generic binman build rules from make then it can be switches to that generic binman rule. Until it happen there is not better option...
Re: [PATCH v2 1/1] doc: man-page for source command
On 1/14/23 14:15, Heinrich Schuchardt wrote:
Provide a man-page for the source command.
Signed-off-by: Heinrich Schuchardt
---
v2:
describe defaults for arguments
mention signing and verifying FIT images
link to uImage.FIT/signature.txt
add an example with incbin
describe escaping hash sign
provide examples without script address
---
doc/usage/cmd/source.rst | 193 +++
doc/usage/index.rst | 1 +
2 files changed, 194 insertions(+)
create mode 100644 doc/usage/cmd/source.rst
diff --git a/doc/usage/cmd/source.rst b/doc/usage/cmd/source.rst
new file mode 100644
index 00..61a4505909
--- /dev/null
+++ b/doc/usage/cmd/source.rst
@@ -0,0 +1,193 @@
+.. SPDX-License-Identifier: GPL-2.0+
+.. Copyright 2022, Heinrich Schuchardt
+
+source command
+==
+
+Synopsis
+
+
+::
+
+source [][:[]|#[]]
+
+Description
+---
+
+The *source* command is used to execute a script file from memory.
+
+Two formats for script files exist:
+
+* legacy U-Boot image format
+* Flat Image Tree (FIT)
+
+The benefit of the FIT images is that they can be signed and verifed as
+decribed in :download:`signature.txt <../../uImage.FIT/signature.txt>`.
+
+Both formats can be created with the mkimage tool.
+
+addr
+location of the script file in memory, defaults to CONFIG_SYS_LOAD_ADDR.
+
+image
+name of an image in a FIT file
+
+config
+name of a configuration in a FIT file. A hash sign following white space
+starts a comment. Hence, if no *addr* is specified, the hash sign has to be
+escaped with a backslash or the argument must be quoted.
+
+If both *image* and *config* are omitted, the default configuration is used, or
+if no configuration is defined, the default image.
Although I would like to see mentioned that # and : can be used without image
and
config (which is quite useful when you e.g. only have one config).
+Examples
+
+
+FIT image
+'
+
+For creating a FIT image an image tree source file (\*.its) is needed. Here is
+an example (source.its).
+
+.. code-block::
+
+/dts-v1/;
+
+/ {
+description = "FIT image to test the source command";
+#address-cells = <1>;
+
+images {
+default = "script-1";
+
+script-1 {
+data = "echo 1";
+type = "script";
+compression = "none";
+};
+
+script-2 {
+data = "echo 2";
+type = "script";
+compression = "none";
+};
+};
+
+configurations {
+default = "conf-2";
+
+conf-1 {
+script = "script-1";
+};
+
+conf-2 {
+script = "script-2";
+};
+};
+};
+
+The FIT image file (boot.itb) is created with:
+
+.. code-block:: bash
+
+mkimage -f source.its boot.itb
+
+In U-Boot the script can be loaded and execute like this
+
+.. code-block::
+
+=> load host 0:1 $loadaddr boot.itb
+1552 bytes read in 0 ms
+=> source $loadaddr#conf-1
+## Executing script at
+1
+=> source $loadaddr#conf-2
+## Executing script at
+2
+=> source $loadaddr:script-1
+## Executing script at
+1
+=> source $loadaddr:script-2
+## Executing script at
+2
+=> source $loadaddr
+## Executing script at
+2
+=> source \#conf-1
+## Executing script at
+1
+=> source '#conf-1'
+## Executing script at
+1
+=> source ':script-1'
+## Executing script at
+1
+=> source
+## Executing script at
+2
+=>
+
+Instead of specifying command line instructions directly in the *data* property
+of the image tree source file another file can be included. Here is a minimal
+example which encapsulates the file boot.txt:
+
+.. code-block::
+
+/dts-v1/;
+/ {
+description = "";
+images {
+script {
+data = /incbin/("./boot.txt");
+type = "script";
+};
+};
+};
+
+Legacy U-Boot image
+'''
+
+A script file using the legacy U-Boot image file format can be created based on
+a text file. Let's use this example text file (boot.txt):
+
+.. code-block:: bash
+
+echo Hello from a script
+echo ---
+
+The boot scripts (boot.scr) is created with:
+
+.. code-block:: bash
+
+mkimage -T script -n 'Test script' -d boot.txt boot.scr
+
+The script can be execute in U-boot like this:
+
+.. code-block::
+
+=> load host 0:1 $loadaddr boot.scr
+122 bytes read in 0 ms
+=> source $loadaddr
+## Executing script at
+Hello from a script
+---
+=> source
+## Executing script at
+Hello from a script
+---
+=>
+
[PATCH 2/2] event: Correct dependencies on the EVENT framework
The event framework is just that, a framework. Enabling it by itself
does nothing, so we shouldn't ask the user about it. Reword (and correct
typos) around this the option and help text. This also applies to
DM_EVENT, so reword as well.
With this, it's time to address the larger problems. When functionality
uses events, typically via EVENT_SPY, the appropriate framework then
must be select'd and NOT imply'd. As the functionality will cease to
work (and so, platforms will fail to boot) this is non-optional and
where select is appropriate. Audit the current users of EVENT_SPY to
have a more fine-grained approach to select'ing the framework where
used.
Cc: Simon Glass
Reported-by: Oliver Graute
Reported-by: Francesco Dolcini
Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
Fixes: 42fdcebf859f ("event: Convert misc_init_f() to use events")
Signed-off-by: Tom Rini
---
arch/Kconfig | 6 +++---
arch/arm/Kconfig | 9 -
arch/arm/mach-omap2/Kconfig | 3 +++
arch/mips/Kconfig| 2 +-
arch/powerpc/cpu/mpc85xx/Kconfig | 1 +
arch/x86/Kconfig | 1 +
arch/x86/cpu/baytrail/Kconfig| 1 +
arch/x86/cpu/broadwell/Kconfig | 1 +
arch/x86/cpu/ivybridge/Kconfig | 1 +
arch/x86/cpu/quark/Kconfig | 1 +
board/google/Kconfig | 1 +
board/keymile/Kconfig| 1 +
boot/Kconfig | 1 +
cmd/Kconfig | 1 +
common/Kconfig | 17 -
drivers/core/Kconfig | 9 +
drivers/cpu/Kconfig | 1 -
lib/efi_loader/Kconfig | 5 ++---
18 files changed, 36 insertions(+), 26 deletions(-)
diff --git a/arch/Kconfig b/arch/Kconfig
index 8fb87b7d857c..d30676ae817b 100644
--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -93,7 +93,7 @@ config NIOS2
bool "Nios II architecture"
select CPU
select DM
- imply DM_EVENT
+ select DM_EVENT
select OF_CONTROL
select SUPPORT_OF_CONTROL
imply CMD_DM
@@ -111,9 +111,9 @@ config RISCV
select SUPPORT_OF_CONTROL
select OF_CONTROL
select DM
+ select DM_EVENT
imply SPL_SEPARATE_BSS if SPL
imply DM_SERIAL
- imply DM_EVENT
imply DM_MMC
imply DM_SPI
imply DM_SPI_FLASH
@@ -136,6 +136,7 @@ config SANDBOX
select BZIP2
select CMD_POWEROFF
select DM
+ select DM_EVENT
select DM_FUZZING_ENGINE
select DM_GPIO
select DM_I2C
@@ -240,7 +241,6 @@ config X86
imply CMD_SF
imply CMD_SF_TEST
imply CMD_ZBOOT
- imply DM_EVENT
imply DM_GPIO
imply DM_KEYBOARD
imply DM_MMC
diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index bbf1d5227b3f..c9a44ebc2213 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -778,7 +778,6 @@ config ARCH_OMAP2PLUS
select SUPPORT_SPL
imply TI_SYSC if DM && OF_CONTROL
imply FIT
- imply DM_EVENT
imply SPL_SEPARATE_BSS
config ARCH_MESON
@@ -823,11 +822,11 @@ config ARCH_IMX8
select SYS_FSL_SEC_COMPAT_4
select SYS_FSL_SEC_LE
select DM
+ select DM_EVENT
select GPIO_EXTRA_HEADER
select MACH_IMX
select OF_CONTROL
select ENABLE_ARM_SOC_BOOT0_HOOK
- imply DM_EVENT
config ARCH_IMX8M
bool "NXP i.MX8M platform"
@@ -839,14 +838,15 @@ config ARCH_IMX8M
select SYS_FSL_SEC_LE
select SYS_I2C_MXC
select DM
+ select DM_EVENT if CLK
select SUPPORT_SPL
imply CMD_DM
- imply DM_EVENT
config ARCH_IMX8ULP
bool "NXP i.MX8ULP platform"
select ARM64
select DM
+ select DM_EVENT
select MACH_IMX
select OF_CONTROL
select SUPPORT_SPL
@@ -854,19 +854,18 @@ config ARCH_IMX8ULP
select MISC
select IMX_SENTINEL
imply CMD_DM
- imply DM_EVENT
config ARCH_IMX9
bool "NXP i.MX9 platform"
select ARM64
select DM
+ select DM_EVENT
select MACH_IMX
select SUPPORT_SPL
select GPIO_EXTRA_HEADER
select MISC
select IMX_SENTINEL
imply CMD_DM
- imply DM_EVENT
config ARCH_IMXRT
bool "NXP i.MXRT platform"
diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index 1db71df27212..309b967b0dd5 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -31,6 +31,7 @@ config OMAP34XX
config OMAP44XX
bool "OMAP44XX SoC"
+ select DM_EVENT
select SPL_USE_TINY_PRINTF
select SPL_SYS_NO_VECTOR_TABLE if SPL
imply NAND_OMAP_ELM
@@ -55,6 +56,7 @@ config OMAP54XX
bool "OMAP54XX SoC"
select ARM_CORTEX_A15_CVE_2017_5715
select ARM_ERRATA_798870
+ select DM_EVENT
select SYS_THUMB_BUILD
imply NAND_OMAP_ELM
imply
[PATCH 1/2] x86: Fix saying arch_cpu_init_dm in debug/docs
The function arch_cpu_init_dm was renamed to fsp_setup_pinctrl in these
cases, so rename debug / docs to match.
Cc: Simon Glass
Fixes: 7fe32b3442f0 ("event: Convert arch_cpu_init_dm() to use events")
Signed-off-by: Tom Rini
---
arch/x86/lib/spl.c| 2 +-
doc/board/google/chromebook_coral.rst | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/lib/spl.c b/arch/x86/lib/spl.c
index 34ef68f2bb7d..bdf57ef7b5bd 100644
--- a/arch/x86/lib/spl.c
+++ b/arch/x86/lib/spl.c
@@ -92,7 +92,7 @@ static int x86_spl_init(void)
#ifndef CONFIG_TPL
ret = fsp_setup_pinctrl(NULL, NULL);
if (ret) {
- debug("%s: arch_cpu_init_dm() failed\n", __func__);
+ debug("%s: fsp_setup_pinctrl() failed\n", __func__);
return ret;
}
#endif
diff --git a/doc/board/google/chromebook_coral.rst
b/doc/board/google/chromebook_coral.rst
index 8edbf0429cd0..23e2db455641 100644
--- a/doc/board/google/chromebook_coral.rst
+++ b/doc/board/google/chromebook_coral.rst
@@ -259,7 +259,7 @@ Boot flow - U-Boot pre-relocation
U-Boot (running from start_from_spl.S) starts running in RAM and uses the same
stack as SPL. It does various init activities before relocation. Notably
-arch_cpu_init_dm() sets up the pin muxing for the chip using a very large table
+fsp_setup_pinctrl() sets up the pin muxing for the chip using a very large
table
in the device tree.
PCI auto-config is not used before relocation, but CONFIG_PCI of course is
--
2.25.1
Re: [PATCH] sunxi: eMMC: support TOC0 on boot partitions
Hi Andre, On 1/4/23 19:58, Andre Przywara wrote: > To determine whether we have been booted from an eMMC boot partition, we > replay some of the checks that the BROM must have done to successfully > load the SPL. This involves a checksum check, which currently relies on > the SPL being wrapped in an "eGON" header. > > If a board has secure boot enabled, the BROM will only accept the "TOC0" > format, which is internally very different, but uses the same > checksumming algorithm. Actually the only difference for calculating the > checksum is that the size of the SPL is stored at a different offset. > > Do a header check to determine whether we deal with an eGON or TOC0 > format, then set the SPL size accordingly. The rest of the code is > unchanged. > > This fixes booting from an eMMC boot partition on devices with secure > boot enabled, like the Remix Mini PC. > > Signed-off-by: Andre Przywara > --- > arch/arm/mach-sunxi/board.c | 11 +-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c > index 0c4b6dd1ca3..a90c88210cd 100644 > --- a/arch/arm/mach-sunxi/board.c > +++ b/arch/arm/mach-sunxi/board.c > @@ -365,6 +365,7 @@ static bool sunxi_valid_emmc_boot(struct mmc *mmc) > struct blk_desc *bd = mmc_get_blk_desc(mmc); > u32 *buffer = (void *)(uintptr_t)CONFIG_TEXT_BASE; > struct boot_file_head *egon_head = (void *)buffer; > + struct toc0_main_info *toc0_info = (void *)buffer; > int bootpart = EXT_CSD_EXTRACT_BOOT_PART(mmc->part_config); > uint32_t spl_size, emmc_checksum, chksum = 0; > ulong count; > @@ -391,11 +392,17 @@ static bool sunxi_valid_emmc_boot(struct mmc *mmc) > > /* Read the first block to do some sanity checks on the eGON header. */ > count = blk_dread(bd, 0, 1, buffer); > - if (count != 1 || !sunxi_egon_valid(egon_head)) > + if (count != 1) > + return false; > + > + if (sunxi_egon_valid(egon_head)) > + spl_size = buffer[4]; > + else if (sunxi_toc0_valid(toc0_info)) > + spl_size = buffer[7]; Please dereference the appropriate structure member here instead of indexing the buffer. Regards, Samuel > + else > return false; > > /* Read the rest of the SPL now we know it's halfway sane. */ > - spl_size = buffer[4]; > count = blk_dread(bd, 1, DIV_ROUND_UP(spl_size, bd->blksz) - 1, > buffer + bd->blksz / 4); >
Re: [PATCH] sunxi: f1c100s: re-enable SYSRESET
On 1/4/23 20:04, Andre Przywara wrote:
> The SoC .dtsi originally submitted for the Allwinner F1C100s had the
> wrong compatible string for the watchdog, which broke U-Boot's reset
> functionality. To quickly fix this, we disable CONFIG_SYSRESET in
> cfcf1952c11e6ffcbbf88 ("sunxi: f1c100s: Drop SYSRESET to enable reset
> functionality"), so that U-Boot's hardcoded reset driver could take over.
>
> After this was properly fixed in the devicetree, we reverted that patch
> in 92373de041ea ("Revert "sunxi: f1c100s: Drop SYSRESET to enable reset
> functionality"), however this line sneaked back in with d0ee7f295d74
> ("Convert CONFIG_SYS_PBSIZE to Kconfig"), so during a Kconfig update.
>
> Remove this line (again), to use the proper reset driver.
>
> Fixes: d0ee7f295d74 ("Convert CONFIG_SYS_PBSIZE to Kconfig")
> Signed-off-by: Andre Przywara
> ---
> Hi,
>
> this is not critical (so nothing for 2023.01), reset works either way.
> But we should fix it anyway.
>
> Cheers,
> Andre
>
> configs/licheepi_nano_defconfig | 1 -
> 1 file changed, 1 deletion(-)
Reviewed-by: Samuel Holland
Re: [PATCH] sunxi: f1c100s: Drop no-MMC hack
On 1/4/23 20:09, Andre Przywara wrote: > When support for the Allwinner F1C100s SoC was originally introduced, > its DT lacked any MMC nodes, which upset our sunxi-u-boot.dtsi overlay, > when it tried to add an alias to the SD card. To quickly fix this back > then, we guarded that alias with a preprocessor macro. > > Now the F1C100s family has gained MMC nodes, so we don't need the > special treatment anymore. Just remove this guard. > > Signed-off-by: Andre Przywara > --- > arch/arm/dts/sunxi-u-boot.dtsi | 2 -- > 1 file changed, 2 deletions(-) Reviewed-by: Samuel Holland
Re: [PATCH 5/8] sunxi: enable support for SPI NAND booting on SUNIV
On 10/13/22 22:05, Icenowy Zheng wrote:
> As we added support for SPI NAND to the existing SPL SPI codepath, route
> the boot code to it when it detects the BROM loads SPL from SPI NAND, as
> for SoCs with both SPI NAND and boot media indicator support, the boot
> media indicator is the same for SPI NOR and NAND.
>
> Signed-off-by: Icenowy Zheng
> ---
> arch/arm/mach-sunxi/board.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
Reviewed-by: Samuel Holland
> diff --git a/arch/arm/mach-sunxi/board.c b/arch/arm/mach-sunxi/board.c
> index 220ed80ba7..3a81743e8f 100644
> --- a/arch/arm/mach-sunxi/board.c
> +++ b/arch/arm/mach-sunxi/board.c
> @@ -210,12 +210,10 @@ static int suniv_get_boot_source(void)
> case SUNIV_BOOTED_FROM_MMC0:
> return SUNXI_BOOTED_FROM_MMC0;
> case SUNIV_BOOTED_FROM_SPI:
> + case SUNIV_BOOTED_FROM_NAND:
> return SUNXI_BOOTED_FROM_SPI;
> case SUNIV_BOOTED_FROM_MMC1:
> return SUNXI_BOOTED_FROM_MMC2;
> - /* SPI NAND is not supported yet. */
> - case SUNIV_BOOTED_FROM_NAND:
> - return SUNXI_INVALID_BOOT_SOURCE;
> }
> /* If we get here something went wrong try to boot from FEL.*/
> printf("Unknown boot source from BROM: 0x%x\n", brom_call);
Re: [PATCH 4/8] sunxi: SPL SPI: add initial support for booting from SPI NAND
On 10/13/22 22:05, Icenowy Zheng wrote:
> This commit adds support for booting from SPI NAND to SPL SPI code by
> mimicing the behavior of boot ROM (use fixed page size and sequentially
> try SPI NOR and NAND).
One warning generated when SPL_SPI_SUNXI_NAND is disabled. Otherwise, it
looks fine to me. I verified that NOR booting still works when
SPL_SPI_SUNXI_NAND is enabled, so:
Tested-by: Samuel Holland # Orange Pi Zero Plus
> Signed-off-by: Icenowy Zheng
> ---
> arch/arm/mach-sunxi/Kconfig | 16 +++
> arch/arm/mach-sunxi/spl_spi_sunxi.c | 74 +
> 2 files changed, 90 insertions(+)
>
> diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig
> index 840bbc19b3..6afbb4acb5 100644
> --- a/arch/arm/mach-sunxi/Kconfig
> +++ b/arch/arm/mach-sunxi/Kconfig
> @@ -1018,6 +1018,22 @@ config SPL_SPI_SUNXI
> sunxi SPI Flash. It uses the same method as the boot ROM, so does
> not need any extra configuration.
>
> +config SPL_SPI_SUNXI_NAND
> + bool "Support for SPI NAND Flash on Allwinner SoCs in SPL"
> + depends on SPL_SPI_SUNXI
> + help
> + Enable support for SPI NAND Flash. This option allows SPL to mimic
> + Allwinner boot ROM's behavior to gain support for SPI NAND Flash;
> + a fixed page size needs to be assumed when building the SPL image.
> +
> +config SPL_SPI_SUNXI_NAND_ASSUMED_PAGESIZE
> + hex "Assumed pagesize for SPI NAND Flash in SPL"
> + depends on SPL_SPI_SUNXI_NAND
> + default 0x400 if MACH_SUNIV
> + help
> + Set the page size assumed by the SPL SPI NAND code, the default
> + value is the same with the boot ROM.
> +
> config PINE64_DT_SELECTION
> bool "Enable Pine64 device tree selection code"
> depends on MACH_SUN50I
> diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c
> b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> index 21be33a23f..5178908327 100644
> --- a/arch/arm/mach-sunxi/spl_spi_sunxi.c
> +++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> @@ -305,6 +305,49 @@ static void spi0_xfer(const u8 *txbuf, u32 txlen, u8
> *rxbuf, u32 rxlen)
> }
> }
>
> +#if defined(CONFIG_SPL_SPI_SUNXI_NAND)
> +static int spi0_nand_switch_page(u32 page)
> +{
> + unsigned count;
> + u8 buf[4];
> +
> + /* Configure the Page Data Read (13h) command header */
> + buf[0] = 0x13;
> + buf[1] = (u8)(page >> 16);
> + buf[2] = (u8)(page >> 8);
> + buf[3] = (u8)(page);
> +
> + spi0_xfer(buf, 4, NULL, 0);
> +
> + /* Wait for NAND chip to exit busy state */
> + buf[0] = 0x0f;
> + buf[1] = 0xc0;
> +
> + /* Load a NAND page can take up to 2-decimal-digit microseconds */
> + for (count = 0; count < 100; count ++) {
> + udelay(1);
> + spi0_xfer(buf, 2, buf+2, 1);
> + if (!(buf[2] & 0x1))
> + return 0;
> + }
> +
> + return -ETIMEDOUT;
> +}
> +
> +static void spi0_nand_reset(void)
> +{
> + u8 buf[1];
> +
> + /* Configure the Device RESET (ffh) command */
> + buf[0] = 0xff;
> +
> + spi0_xfer(buf, 1, NULL, 0);
> +
> + /* Wait for the NAND to finish resetting */
> + udelay(10);
> +}
> +#endif
> +
> static void spi0_read_data(void *buf, u32 addr, u32 len, u32 addr_len)
> {
> u8 *buf8 = buf;
> @@ -348,6 +391,28 @@ static ulong spi_load_read_nor(struct spl_load_info
> *load, ulong sector,
> return count;
> }
>
> +#if defined(CONFIG_SPL_SPI_SUNXI_NAND)
> +static ulong spi_load_read_nand(struct spl_load_info *load, ulong sector,
> +ulong count, void *buf)
> +{
> + const ulong pagesize = CONFIG_SPL_SPI_SUNXI_NAND_ASSUMED_PAGESIZE;
> + ulong remain = count;
> +
> + while (remain) {
> + ulong count_in_page = min(remain, pagesize - (sector %
> pagesize));
> + ulong current_page = sector / pagesize;
> + if (spi0_nand_switch_page(current_page) != 0)
> + return 0;
> + spi0_read_data(buf, sector % pagesize, count_in_page, 2);
> + remain -= count_in_page;
> + sector += count_in_page;
> + buf += count_in_page;
> + }
> +
> + return count;
> +}
> +#endif
> +
>
> /*/
>
> static int spl_spi_try_load(struct spl_image_info *spl_image,
> @@ -400,9 +465,18 @@ static int spl_spi_load_image(struct spl_image_info
> *spl_image,
>
> spi0_init();
>
> +#if defined(CONFIG_SPL_SPI_SUNXI_NAND)
> + spi0_nand_reset();
> + load.read = spi_load_read_nand;
> + ret = spl_spi_try_load(spl_image, bootdev, , load_offset, false);
> + if (!ret)
> + goto out;
> +#endif
> +
> load.read = spi_load_read_nor;
> ret = spl_spi_try_load(spl_image, bootdev, , load_offset, true);
>
> +out:
arch/arm/mach-sunxi/spl_spi_sunxi.c: In function 'spl_spi_load_image':
arch/arm/mach-sunxi/spl_spi_sunxi.c:482:1: warning:
Re: [PATCH] ddr: marvell: a38x: Add support for DDR4 from Marvell mv-ddr-marvell repository
On Saturday 14 January 2023 15:03:41 Tom Rini wrote: > On Sat, Jan 14, 2023 at 07:51:00PM +0100, Pali Rohár wrote: > > On Friday 13 January 2023 21:00:21 Tom Rini wrote: > > > On Sat, Jan 14, 2023 at 02:41:32AM +0100, Pali Rohár wrote: > > > > On Friday 13 January 2023 16:38:55 Tony Dinh wrote: > > > > > @@ -16,4 +19,9 @@ obj-$(CONFIG_SPL_BUILD) += mv_ddr_build_message.o > > > > > obj-$(CONFIG_SPL_BUILD) += mv_ddr_common.o > > > > > obj-$(CONFIG_SPL_BUILD) += mv_ddr_spd.o > > > > > obj-$(CONFIG_SPL_BUILD) += mv_ddr_topology.o > > > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_mpr_pda_if.o > > > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_training.o > > > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_training_calibration.o > > > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_training_db.o > > > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_training_leveling.o > > > > > obj-$(CONFIG_SPL_BUILD) += xor.o > > > > > > > > And all these new files are ddr4 specific, so should be wrapped in > > > > makefile section: > > > > ifdef CONFIG_DDR4 > > > > > > Looking at the Makefile in question, I think we might want to make the > > > whole thing ifdef CONFIG_SPL_BUILD ... endif and then more finely > > > control building of what objects are built. Perhaps: > > > drivers/Makefile:obj-$(CONFIG_ARMADA_38X) += ddr/marvell/a38x/ > > > should only be for SPL instead, even? > > > > Some cleanup like this can be done. But it is related to DDR4 support > > and is mostly independent of it. So lets do it after having DDR4 there. > > We're going to also want to not build the DDR3 code on the DDR4 > platforms, right? A little clean up would make adding the DDR4 code a > bit cleaner for both cases. It's not a hard no, if someone really wants > to do the clean-up after. I can look at it _after_ all other stuff is done and merged.
Re: [PATCH] ddr: marvell: a38x: Add support for DDR4 from Marvell mv-ddr-marvell repository
On Sat, Jan 14, 2023 at 07:51:00PM +0100, Pali Rohár wrote: > On Friday 13 January 2023 21:00:21 Tom Rini wrote: > > On Sat, Jan 14, 2023 at 02:41:32AM +0100, Pali Rohár wrote: > > > On Friday 13 January 2023 16:38:55 Tony Dinh wrote: > > > > @@ -16,4 +19,9 @@ obj-$(CONFIG_SPL_BUILD) += mv_ddr_build_message.o > > > > obj-$(CONFIG_SPL_BUILD) += mv_ddr_common.o > > > > obj-$(CONFIG_SPL_BUILD) += mv_ddr_spd.o > > > > obj-$(CONFIG_SPL_BUILD) += mv_ddr_topology.o > > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_mpr_pda_if.o > > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_training.o > > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_training_calibration.o > > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_training_db.o > > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_training_leveling.o > > > > obj-$(CONFIG_SPL_BUILD) += xor.o > > > > > > And all these new files are ddr4 specific, so should be wrapped in > > > makefile section: > > > ifdef CONFIG_DDR4 > > > > Looking at the Makefile in question, I think we might want to make the > > whole thing ifdef CONFIG_SPL_BUILD ... endif and then more finely > > control building of what objects are built. Perhaps: > > drivers/Makefile:obj-$(CONFIG_ARMADA_38X) += ddr/marvell/a38x/ > > should only be for SPL instead, even? > > Some cleanup like this can be done. But it is related to DDR4 support > and is mostly independent of it. So lets do it after having DDR4 there. We're going to also want to not build the DDR3 code on the DDR4 platforms, right? A little clean up would make adding the DDR4 code a bit cleaner for both cases. It's not a hard no, if someone really wants to do the clean-up after. -- Tom signature.asc Description: PGP signature
Re: [PATCH 3/8] sunxi: SPL SPI: allow multiple boot attempt
On 10/13/22 22:05, Icenowy Zheng wrote:
> As we're going to add support for SPI NAND to this code, add code that
> allows multiple boot attempts with different load offsets and functions.
>
> To keep compatibility with loading raw binary on SPI NOR, a bool
> parameter is used to allow booting without valid magic number when
> booting with SPI NOR.
So the issue is that when CONFIG_SPL_RAW_IMAGE_SUPPORT=y, then
spl_parse_image_header() will return 0 even when using the wrong NAND
parameters? I don't see a better solution, so:
Reviewed-by: Samuel Holland
Tested-by: Samuel Holland # Orange Pi Zero Plus
> Signed-off-by: Icenowy Zheng
> ---
> arch/arm/mach-sunxi/spl_spi_sunxi.c | 58 +++--
> 1 file changed, 38 insertions(+), 20 deletions(-)
>
> diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c
> b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> index 88c15a3ee9..21be33a23f 100644
> --- a/arch/arm/mach-sunxi/spl_spi_sunxi.c
> +++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> @@ -340,8 +340,8 @@ static void spi0_read_data(void *buf, u32 addr, u32 len,
> u32 addr_len)
> }
> }
>
> -static ulong spi_load_read(struct spl_load_info *load, ulong sector,
> -ulong count, void *buf)
> +static ulong spi_load_read_nor(struct spl_load_info *load, ulong sector,
> +ulong count, void *buf)
> {
> spi0_read_data(buf, sector, count, 3);
>
> @@ -350,41 +350,59 @@ static ulong spi_load_read(struct spl_load_info *load,
> ulong sector,
>
>
> /*/
>
> -static int spl_spi_load_image(struct spl_image_info *spl_image,
> - struct spl_boot_device *bootdev)
> +static int spl_spi_try_load(struct spl_image_info *spl_image,
> + struct spl_boot_device *bootdev,
> + struct spl_load_info *load, u32 offset,
> + bool allow_raw)
> {
> int ret = 0;
> struct legacy_img_hdr *header;
> - uint32_t load_offset = sunxi_get_spl_size();
> -
nit: keep this blank line
> header = (struct legacy_img_hdr *)CONFIG_SYS_TEXT_BASE;
> - load_offset = max_t(uint32_t, load_offset, CONFIG_SYS_SPI_U_BOOT_OFFS);
> -
> - spi0_init();
>
> - spi0_read_data((void *)header, load_offset, 0x40, 3);
> + if (load->read(load, offset, 0x40, (void *)header) == 0)
> + return -EINVAL;
>
> if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
> image_get_magic(header) == FDT_MAGIC) {
> - struct spl_load_info load;
>
> debug("Found FIT image\n");
> - load.dev = NULL;
> - load.priv = NULL;
> - load.filename = NULL;
> - load.bl_len = 1;
> - load.read = spi_load_read;
> - ret = spl_load_simple_fit(spl_image, ,
> - load_offset, header);
> + ret = spl_load_simple_fit(spl_image, load,
> + offset, header);
> } else {
> + if (!allow_raw && image_get_magic(header) != IH_MAGIC)
> + return -EINVAL;
> +
> ret = spl_parse_image_header(spl_image, bootdev, header);
> if (ret)
> return ret;
>
> - spi0_read_data((void *)spl_image->load_addr,
> -load_offset, spl_image->size, 3);
> + if (load->read(load, offset, spl_image->size,
> +(void *)spl_image->load_addr) == 0)
> + ret = -EINVAL;
> }
>
> + return ret;
> +}
> +
> +static int spl_spi_load_image(struct spl_image_info *spl_image,
> + struct spl_boot_device *bootdev)
> +{
> + int ret = 0;
> + uint32_t load_offset = sunxi_get_spl_size();
> + struct spl_load_info load;
> +
> + load_offset = max_t(uint32_t, load_offset, CONFIG_SYS_SPI_U_BOOT_OFFS);
> +
> + load.dev = NULL;
> + load.priv = NULL;
> + load.filename = NULL;
> + load.bl_len = 1;
> +
> + spi0_init();
> +
> + load.read = spi_load_read_nor;
> + ret = spl_spi_try_load(spl_image, bootdev, , load_offset, true);
> +
> spi0_deinit();
>
> return ret;
Re: [PATCH 2/3] dm: button: add support for linux_code in button-gpio.c driver
dev_read_u32 will fail, if linux,code is not found.
We shouldn't fail here, as linux,code is optional, so maybe dev_read_u32_default
with 0 default value, instead of negative error code?
ср, 11 янв. 2023 г. в 18:48, Quentin Schulz
:
>
> Hi Dzmitry,
>
> On 1/11/23 11:19, Dzmitry Sankouski wrote:
> > Linux event code may be used in input devices, using buttons.
> >
> > Signed-off-by: Dzmitry Sankouski
> > ---
> > drivers/button/button-gpio.c | 20
> > drivers/button/button-uclass.c | 10 ++
> > include/button.h | 16
> > 3 files changed, 46 insertions(+)
> >
> > diff --git a/drivers/button/button-gpio.c b/drivers/button/button-gpio.c
> > index dbb000622c..e6eff5c1da 100644
> > --- a/drivers/button/button-gpio.c
> > +++ b/drivers/button/button-gpio.c
> > @@ -13,6 +13,7 @@
> >
> > struct button_gpio_priv {
> > struct gpio_desc gpio;
> > + u32 linux_code;
> > };
> >
> > static enum button_state_t button_gpio_get_state(struct udevice *dev)
> > @@ -29,10 +30,22 @@ static enum button_state_t button_gpio_get_state(struct
> > udevice *dev)
> > return ret ? BUTTON_ON : BUTTON_OFF;
> > }
> >
> > +static u32 button_gpio_get_code(struct udevice *dev)
> > +{
> > + struct button_gpio_priv *priv = dev_get_priv(dev);
> > + u32 code = priv->linux_code;
> > +
> > + if (!code)
> > + return 0;
> > +
> > + return code;
> > +}
> > +
> > static int button_gpio_probe(struct udevice *dev)
> > {
> > struct button_uc_plat *uc_plat = dev_get_uclass_plat(dev);
> > struct button_gpio_priv *priv = dev_get_priv(dev);
> > + u32 linux_code;
> > int ret;
> >
> > /* Ignore the top-level button node */
> > @@ -43,6 +56,12 @@ static int button_gpio_probe(struct udevice *dev)
> > if (ret)
> > return ret;
> >
> > + linux_code = dev_read_u32_default(dev, "linux,code", -ENODATA);
> > + debug("linux code value: %d, ret: %d", linux_code, ret);
> > + if (ret)
> > + return ret;
>
> ret is not modified here so it'll always pass even if it fails to parse
> dev_read_u32_default.
>
> I believe dev_read_u32_default incorrectly requests an int as last
> argument while it's going to fill it with u32 data on success.
> dev_read_u8/u16 get this correctly so that might just be an oversight.
>
> Please use dev_read_u32 instead:
>
> ret = dev_read_u32(dev, "linux,code", >linux_code);
> debug("linux code value: %d, ret: %d", linux_code, ret);
> return ret;
>
> (no need to check the value of ret since it's the same as
> if (ret)
> return ret;
>
> return 0;
> )
>
> Cheers,
> Quentin
Re: [PATCH 2/8] sunxi: SPL SPI: add support for read command with 2 byte address
On 10/13/22 22:05, Icenowy Zheng wrote:
> This kind of read command is utilized in SPI NANDs for reading data
> inside a selected page, which is obviously smaller than how much 2
> byte address can address. So 2 bytes are used for the address and one
> dummy byte is needed after the real address. As the address is sent out
> in bit endian, this makes it not compatible with usual 3 byte address.
typo: big
> Signed-off-by: Icenowy Zheng
> ---
> arch/arm/mach-sunxi/spl_spi_sunxi.c | 20 +---
> 1 file changed, 13 insertions(+), 7 deletions(-)
Reviewed-by: Samuel Holland
Tested-by: Samuel Holland # Orange Pi Zero Plus
> diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c
> b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> index 7975457758..88c15a3ee9 100644
> --- a/arch/arm/mach-sunxi/spl_spi_sunxi.c
> +++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> @@ -305,7 +305,7 @@ static void spi0_xfer(const u8 *txbuf, u32 txlen, u8
> *rxbuf, u32 rxlen)
> }
> }
>
> -static void spi0_read_data(void *buf, u32 addr, u32 len)
> +static void spi0_read_data(void *buf, u32 addr, u32 len, u32 addr_len)
> {
> u8 *buf8 = buf;
> u32 chunk_len;
> @@ -316,9 +316,15 @@ static void spi0_read_data(void *buf, u32 addr, u32 len)
>
> /* Configure the Read Data Bytes (03h) command header */
> txbuf[0] = 0x03;
> - txbuf[1] = (u8)(addr >> 16);
> - txbuf[2] = (u8)(addr >> 8);
> - txbuf[3] = (u8)(addr);
> + if (addr_len == 3) {
> + txbuf[1] = (u8)(addr >> 16);
> + txbuf[2] = (u8)(addr >> 8);
> + txbuf[3] = (u8)(addr);
> + } else if (addr_len == 2) {
> + txbuf[1] = (u8)(addr >> 8);
> + txbuf[2] = (u8)(addr);
> + txbuf[3] = 0; /* dummy */
> + }
>
> if (chunk_len > SPI_READ_MAX_SIZE)
> chunk_len = SPI_READ_MAX_SIZE;
> @@ -337,7 +343,7 @@ static void spi0_read_data(void *buf, u32 addr, u32 len)
> static ulong spi_load_read(struct spl_load_info *load, ulong sector,
> ulong count, void *buf)
> {
> - spi0_read_data(buf, sector, count);
> + spi0_read_data(buf, sector, count, 3);
>
> return count;
> }
> @@ -356,7 +362,7 @@ static int spl_spi_load_image(struct spl_image_info
> *spl_image,
>
> spi0_init();
>
> - spi0_read_data((void *)header, load_offset, 0x40);
> + spi0_read_data((void *)header, load_offset, 0x40, 3);
>
> if (IS_ENABLED(CONFIG_SPL_LOAD_FIT) &&
> image_get_magic(header) == FDT_MAGIC) {
> @@ -376,7 +382,7 @@ static int spl_spi_load_image(struct spl_image_info
> *spl_image,
> return ret;
>
> spi0_read_data((void *)spl_image->load_addr,
> -load_offset, spl_image->size);
> +load_offset, spl_image->size, 3);
> }
>
> spi0_deinit();
Re: [PATCH 1/8] sunxi: SPL SPI: extract code for doing SPI transfer
On 10/13/22 22:05, Icenowy Zheng wrote:
> To support SPI NAND flashes, more commands than Read (03h) are needed.
>
> Extract the code for doing SPI transfer from the reading code for code
> reuse.
>
> Signed-off-by: Icenowy Zheng
One comment below.
Reviewed-by: Samuel Holland
Tested-by: Samuel Holland # Orange Pi Zero Plus
> ---
> arch/arm/mach-sunxi/spl_spi_sunxi.c | 105
> 1 file changed, 59 insertions(+), 46 deletions(-)
>
> diff --git a/arch/arm/mach-sunxi/spl_spi_sunxi.c
> b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> index 925bf85f2d..7975457758 100644
> --- a/arch/arm/mach-sunxi/spl_spi_sunxi.c
> +++ b/arch/arm/mach-sunxi/spl_spi_sunxi.c
> @@ -243,77 +243,90 @@ static void spi0_deinit(void)
>
> #define SPI_READ_MAX_SIZE 60 /* FIFO size, minus 4 bytes of the header */
>
> -static void sunxi_spi0_read_data(u8 *buf, u32 addr, u32 bufsize,
> - ulong spi_ctl_reg,
> - ulong spi_ctl_xch_bitmask,
> - ulong spi_fifo_reg,
> - ulong spi_tx_reg,
> - ulong spi_rx_reg,
> - ulong spi_bc_reg,
> - ulong spi_tc_reg,
> - ulong spi_bcc_reg)
> +static void sunxi_spi0_xfer(const u8 *txbuf, u32 txlen,
> + u8 *rxbuf, u32 rxlen,
> + ulong spi_ctl_reg,
> + ulong spi_ctl_xch_bitmask,
> + ulong spi_fifo_reg,
> + ulong spi_tx_reg,
> + ulong spi_rx_reg,
> + ulong spi_bc_reg,
> + ulong spi_tc_reg,
> + ulong spi_bcc_reg)
> {
> - writel(4 + bufsize, spi_bc_reg); /* Burst counter (total bytes) */
> - writel(4, spi_tc_reg); /* Transfer counter (bytes to send) */
> + writel(txlen + rxlen, spi_bc_reg); /* Burst counter (total bytes) */
> + writel(txlen, spi_tc_reg); /* Transfer counter (bytes to send)
> */
> if (spi_bcc_reg)
> - writel(4, spi_bcc_reg); /* SUN6I also needs this */
> + writel(txlen, spi_bcc_reg); /* SUN6I also needs this */
>
> - /* Send the Read Data Bytes (03h) command header */
> - writeb(0x03, spi_tx_reg);
> - writeb((u8)(addr >> 16), spi_tx_reg);
> - writeb((u8)(addr >> 8), spi_tx_reg);
> - writeb((u8)(addr), spi_tx_reg);
> + for (u32 i = 0; i < txlen; i++)
> + writeb(*(txbuf++), spi_tx_reg);
I think txbuf[i] would be a bit clearer here.
Regards,
Samuel
>
> /* Start the data transfer */
> setbits_le32(spi_ctl_reg, spi_ctl_xch_bitmask);
>
> /* Wait until everything is received in the RX FIFO */
> - while ((readl(spi_fifo_reg) & 0x7F) < 4 + bufsize)
> + while ((readl(spi_fifo_reg) & 0x7F) < txlen + rxlen)
> ;
>
> - /* Skip 4 bytes */
> - readl(spi_rx_reg);
> + /* Skip txlen bytes */
> + for (u32 i = 0; i < txlen; i++)
> + readb(spi_rx_reg);
>
> /* Read the data */
> - while (bufsize-- > 0)
> - *buf++ = readb(spi_rx_reg);
> + while (rxlen-- > 0)
> + *rxbuf++ = readb(spi_rx_reg);
> +}
> +
> +static void spi0_xfer(const u8 *txbuf, u32 txlen, u8 *rxbuf, u32 rxlen)
> +{
> + uintptr_t base = spi0_base_address();
>
> - /* tSHSL time is up to 100 ns in various SPI flash datasheets */
> - udelay(1);
> + if (is_sun6i_gen_spi()) {
> + sunxi_spi0_xfer(txbuf, txlen, rxbuf, rxlen,
> + base + SUN6I_SPI0_TCR,
> + SUN6I_TCR_XCH,
> + base + SUN6I_SPI0_FIFO_STA,
> + base + SUN6I_SPI0_TXD,
> + base + SUN6I_SPI0_RXD,
> + base + SUN6I_SPI0_MBC,
> + base + SUN6I_SPI0_MTC,
> + base + SUN6I_SPI0_BCC);
> + } else {
> + sunxi_spi0_xfer(txbuf, txlen, rxbuf, rxlen,
> + base + SUN4I_SPI0_CTL,
> + SUN4I_CTL_XCH,
> + base + SUN4I_SPI0_FIFO_STA,
> + base + SUN4I_SPI0_TX,
> + base + SUN4I_SPI0_RX,
> + base + SUN4I_SPI0_BC,
> + base + SUN4I_SPI0_TC,
> + 0);
> + }
> }
>
> static void spi0_read_data(void *buf, u32 addr, u32 len)
> {
> u8 *buf8 = buf;
> u32 chunk_len;
> - uintptr_t base = spi0_base_address();
> + u8 txbuf[4];
>
> while (len > 0) {
> chunk_len = len;
> +
> + /* Configure the Read Data Bytes (03h) command header */
> + txbuf[0] = 0x03;
> + txbuf[1] = (u8)(addr >> 16);
> +
[PATCH v2 1/1] doc: man-page for source command
Provide a man-page for the source command.
Signed-off-by: Heinrich Schuchardt
---
v2:
describe defaults for arguments
mention signing and verifying FIT images
link to uImage.FIT/signature.txt
add an example with incbin
describe escaping hash sign
provide examples without script address
---
doc/usage/cmd/source.rst | 193 +++
doc/usage/index.rst | 1 +
2 files changed, 194 insertions(+)
create mode 100644 doc/usage/cmd/source.rst
diff --git a/doc/usage/cmd/source.rst b/doc/usage/cmd/source.rst
new file mode 100644
index 00..61a4505909
--- /dev/null
+++ b/doc/usage/cmd/source.rst
@@ -0,0 +1,193 @@
+.. SPDX-License-Identifier: GPL-2.0+
+.. Copyright 2022, Heinrich Schuchardt
+
+source command
+==
+
+Synopsis
+
+
+::
+
+source [][:[]|#[]]
+
+Description
+---
+
+The *source* command is used to execute a script file from memory.
+
+Two formats for script files exist:
+
+* legacy U-Boot image format
+* Flat Image Tree (FIT)
+
+The benefit of the FIT images is that they can be signed and verifed as
+decribed in :download:`signature.txt <../../uImage.FIT/signature.txt>`.
+
+Both formats can be created with the mkimage tool.
+
+addr
+location of the script file in memory, defaults to CONFIG_SYS_LOAD_ADDR.
+
+image
+name of an image in a FIT file
+
+config
+name of a configuration in a FIT file. A hash sign following white space
+starts a comment. Hence, if no *addr* is specified, the hash sign has to be
+escaped with a backslash or the argument must be quoted.
+
+If both *image* and *config* are omitted, the default configuration is used, or
+if no configuration is defined, the default image.
+
+Examples
+
+
+FIT image
+'
+
+For creating a FIT image an image tree source file (\*.its) is needed. Here is
+an example (source.its).
+
+.. code-block::
+
+/dts-v1/;
+
+/ {
+description = "FIT image to test the source command";
+#address-cells = <1>;
+
+images {
+default = "script-1";
+
+script-1 {
+data = "echo 1";
+type = "script";
+compression = "none";
+};
+
+script-2 {
+data = "echo 2";
+type = "script";
+compression = "none";
+};
+};
+
+configurations {
+default = "conf-2";
+
+conf-1 {
+script = "script-1";
+};
+
+conf-2 {
+script = "script-2";
+};
+};
+};
+
+The FIT image file (boot.itb) is created with:
+
+.. code-block:: bash
+
+mkimage -f source.its boot.itb
+
+In U-Boot the script can be loaded and execute like this
+
+.. code-block::
+
+=> load host 0:1 $loadaddr boot.itb
+1552 bytes read in 0 ms
+=> source $loadaddr#conf-1
+## Executing script at
+1
+=> source $loadaddr#conf-2
+## Executing script at
+2
+=> source $loadaddr:script-1
+## Executing script at
+1
+=> source $loadaddr:script-2
+## Executing script at
+2
+=> source $loadaddr
+## Executing script at
+2
+=> source \#conf-1
+## Executing script at
+1
+=> source '#conf-1'
+## Executing script at
+1
+=> source ':script-1'
+## Executing script at
+1
+=> source
+## Executing script at
+2
+=>
+
+Instead of specifying command line instructions directly in the *data* property
+of the image tree source file another file can be included. Here is a minimal
+example which encapsulates the file boot.txt:
+
+.. code-block::
+
+/dts-v1/;
+/ {
+description = "";
+images {
+script {
+data = /incbin/("./boot.txt");
+type = "script";
+};
+};
+};
+
+Legacy U-Boot image
+'''
+
+A script file using the legacy U-Boot image file format can be created based on
+a text file. Let's use this example text file (boot.txt):
+
+.. code-block:: bash
+
+echo Hello from a script
+echo ---
+
+The boot scripts (boot.scr) is created with:
+
+.. code-block:: bash
+
+mkimage -T script -n 'Test script' -d boot.txt boot.scr
+
+The script can be execute in U-boot like this:
+
+.. code-block::
+
+=> load host 0:1 $loadaddr boot.scr
+122 bytes read in 0 ms
+=> source $loadaddr
+## Executing script at
+Hello from a script
+---
+=> source
+## Executing script at
+Hello from a script
+---
+=>
+
+Configuration
+-
+
+The source command is only available if CONFIG_CMD_SOURCE=y.
+The FIT image file format requires CONFIG_FIT=y.#
+The legacy U-Boot image file format requires
Re: [PATCH] ddr: marvell: a38x: Add support for DDR4 from Marvell mv-ddr-marvell repository
On Friday 13 January 2023 21:00:21 Tom Rini wrote: > On Sat, Jan 14, 2023 at 02:41:32AM +0100, Pali Rohár wrote: > > On Friday 13 January 2023 16:38:55 Tony Dinh wrote: > > > @@ -16,4 +19,9 @@ obj-$(CONFIG_SPL_BUILD) += mv_ddr_build_message.o > > > obj-$(CONFIG_SPL_BUILD) += mv_ddr_common.o > > > obj-$(CONFIG_SPL_BUILD) += mv_ddr_spd.o > > > obj-$(CONFIG_SPL_BUILD) += mv_ddr_topology.o > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_mpr_pda_if.o > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_training.o > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_training_calibration.o > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_training_db.o > > > +obj-$(CONFIG_SPL_BUILD) += mv_ddr4_training_leveling.o > > > obj-$(CONFIG_SPL_BUILD) += xor.o > > > > And all these new files are ddr4 specific, so should be wrapped in makefile > > section: > > ifdef CONFIG_DDR4 > > Looking at the Makefile in question, I think we might want to make the > whole thing ifdef CONFIG_SPL_BUILD ... endif and then more finely > control building of what objects are built. Perhaps: > drivers/Makefile:obj-$(CONFIG_ARMADA_38X) += ddr/marvell/a38x/ > should only be for SPL instead, even? Some cleanup like this can be done. But it is related to DDR4 support and is mostly independent of it. So lets do it after having DDR4 there.
Re: [PATCH 1/1] doc: man-page for source command
On 1/14/23 18:50, Sean Anderson wrote:
On 1/14/23 12:41, Heinrich Schuchardt wrote:
On 1/14/23 18:07, Sean Anderson wrote:
On 1/14/23 06:51, Heinrich Schuchardt wrote:
Provide a man-page for the source command.
Signed-off-by: Heinrich Schuchardt
---
doc/usage/cmd/source.rst | 154
+++
doc/usage/index.rst | 1 +
2 files changed, 155 insertions(+)
create mode 100644 doc/usage/cmd/source.rst
diff --git a/doc/usage/cmd/source.rst b/doc/usage/cmd/source.rst
new file mode 100644
index 00..9622f1d5a8
--- /dev/null
+++ b/doc/usage/cmd/source.rst
@@ -0,0 +1,154 @@
+.. SPDX-License-Identifier: GPL-2.0+
+.. Copyright 2022, Heinrich Schuchardt
+
+source command
+==
+
+Synopsis
+
+
+::
+
+ source [][:[]|#[]]
+
+Description
+---
+
+The *source* command is used to execute a script file from memory.
+
+Two formats for script files exist:
+
+* legacy U-Boot image format
+* Flat Image Tree (FIT)
+
+Both formats can be created with the mkimage tool.
+
+addr
+ location of the script file in memory, defaults to
CONFIG_SYS_LOAD_ADDR.
+
+image
+ name of an image in a FIT file
+
+config
+ name of a configuration in a FIT file
We need a note here about how these are optional:
Thank you for reviewing.
image
name of an image in a FIT file. May be omitted to pick the
default image.
config
name of a configuration in a FIT file. May be omitted to pick
the default
config. If addr is not specified, the # must be escaped or
quoted to prevent
it from being interpreted as a comment.
And probably a general note about priorities:
- If : is specified, then *source* will choose an image.
- If # is specified, then *source* will choose an image based on a
configuration.
- If neither : nor # is specified, then *source* will try to choose
the image in the
default configuration. If no configurations are present, then it
will pick the default
image.
And a note about secure boot:
If you are using verified boot, signing keys are required based on the
We nowhere describe what we mean by "verified boot". EFI secure boot
is also a type of verified boot but it does not rely on anything in
U-Boot's device-tree.
"U-Boot verified boot"? This is what the uImage stuff calls it.
It think it would be enough to add the reference:
doc/uImage.FIT/signature.txt describes how FIT images including
scripts can be signed and verified.
All text documents in doc/uImage should be converted to restructured
text and added to the HTML documentation.
I agree.
value of the "required"
property in the key's node in U-Boot's device tree. If the value of
this property is "image",
then scripts will always be verified. However, if the value of this
node is "conf", then scripts
will only be verified when a # is specified, as this forces the image
to be determined based on
a configuration. For more information, refer to
doc/uImage.FIT/signature.txt. Additionally, as
is typical, legacy images must be disabled for verified boot, as they
do not support signing.
This is easy to misread.
CONFIG_LEGACY_IMAGE_FORMAT=y does not stop the verification of anything.
Probably you mean:
CONFIG_LEGACY_IMAGE_FORMAT should be disabled a hardened systems as
legacy images cannot be signed.
That works too.
+Examples
+
+
+For creating a FIT image an image tree source file (\*.its) is
needed. Here is
+an example (source.its).
+
+.. code-block::
+
+ /dts-v1/;
+
+ / {
+ description = "FIT image to test the source command";
+ #address-cells = <1>;
+
+ images {
+ default = "script-1";
+
+ script-1 {
+ data = "echo 1";
We should use /incbin/ here, since that is more typical.
We should mention /incbin/ below. Here I want an example that is
trivial to understand.
+ type = "script";
+ compression = "none";
+ };
+
+ script-2 {
+ data = "echo 2";
+ type = "script";
+ compression = "none";
+ };
+ };
+
+ configurations {
+ default = "conf-2";
+
+ conf-1 {
+ script = "script-1";
+ };
+
+ conf-2 {
+ script = "script-2";
+ };
And omit the second script/config.
I want to be able to demonstrate what #config is used for.
This is not possible without a second config.
We should mention that the configurations node is optional.
+ };
+ };
+
+The FIT image file (boot.itb) is created with:
+
+.. code-block:: bash
+
+ mkimage -f source.its boot.itb
+
+In U-Boot the script can be loaded and execute like this
+
+.. code-block::
+
+ => load host 0:1 $loadaddr boot.itb
+ 1552 bytes read in 0 ms
+ => source $loadaddr#conf-1
+ ## Executing script at
+ 1
+ => source $loadaddr#conf-2
+ ## Executing script at
Re: [PATCH v2 01/22] apalis-imx8: fix booting caused by missing dm_event
On Sat, Jan 14, 2023 at 07:00:08PM +0100, Heinrich Schuchardt wrote: > On 1/14/23 18:22, Tom Rini wrote: > > On Sat, Jan 14, 2023 at 11:57:20AM -0500, Tom Rini wrote: > > > On Sat, Jan 14, 2023 at 08:27:25AM -0500, Tom Rini wrote: > > > > On Sat, Jan 14, 2023 at 08:25:19AM -0300, Fabio Estevam wrote: > > > > > Hi Francesco, > > > > > > > > > > On Fri, Jan 13, 2023 at 2:18 PM Francesco Dolcini > > > > > wrote: > > > > > > > > > > > > From: Marcel Ziswiler > > > > > > > > > > > > Without the DM_EVENT absolutely no output whatsoever and the system > > > > > > does > > > > > > not boot at all. > > > > > > > > > > Why do we need to select CONFIG_EVENT=y in the first place? > > > > > > > > > > Oliver on CC reported the same issue for his board, and we have other > > > > > broken boards too > > > > > that do not select CONFIG_EVENT=y. > > > > > > > > > > Can we have a generic fix for this? > > > > > > > > > > Tom, Heinrich? > > > > > > > > Yes, if someone can explain what the problem is, we can correct the > > > > dependency. I gather most platforms have whatever this problem is > > > > ignored due to enabling EFI_LOADER. > > > > > > The more I look at this, the more confused I get. With DEBUG_UART > > > enabled, where do thing fail without CONFIG_EVENT (and CONFIG_DM_EVENT, > > > which is then enabled due to "imply DM_EVENT" in arch/arm/Kconfig, under > > > ARCH_IMX8) ? How about if you turn DM_EVENT back off? > > > > The only other data point I can add right now is that it's not entirely > > generic, I think. I can disable EFI_LOADER and EVENT and DM_EVENT on > > am65x_evm (both r5 and a53 portions) and still boot up fine. I don't > > have something imx8 in my lab, however. > > > > I cannot find event_register() used outside of > lib/efi_driver/efi_block_device.c and test/common/event.c) > > Why should any board with CONFIG_EFI_LOADER=n depend on events? There's other events, using EVENT_SPY(...). I'm working on a patch to fix things up now. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 01/22] apalis-imx8: fix booting caused by missing dm_event
On 1/14/23 18:22, Tom Rini wrote: On Sat, Jan 14, 2023 at 11:57:20AM -0500, Tom Rini wrote: On Sat, Jan 14, 2023 at 08:27:25AM -0500, Tom Rini wrote: On Sat, Jan 14, 2023 at 08:25:19AM -0300, Fabio Estevam wrote: Hi Francesco, On Fri, Jan 13, 2023 at 2:18 PM Francesco Dolcini wrote: From: Marcel Ziswiler Without the DM_EVENT absolutely no output whatsoever and the system does not boot at all. Why do we need to select CONFIG_EVENT=y in the first place? Oliver on CC reported the same issue for his board, and we have other broken boards too that do not select CONFIG_EVENT=y. Can we have a generic fix for this? Tom, Heinrich? Yes, if someone can explain what the problem is, we can correct the dependency. I gather most platforms have whatever this problem is ignored due to enabling EFI_LOADER. The more I look at this, the more confused I get. With DEBUG_UART enabled, where do thing fail without CONFIG_EVENT (and CONFIG_DM_EVENT, which is then enabled due to "imply DM_EVENT" in arch/arm/Kconfig, under ARCH_IMX8) ? How about if you turn DM_EVENT back off? The only other data point I can add right now is that it's not entirely generic, I think. I can disable EFI_LOADER and EVENT and DM_EVENT on am65x_evm (both r5 and a53 portions) and still boot up fine. I don't have something imx8 in my lab, however. I cannot find event_register() used outside of lib/efi_driver/efi_block_device.c and test/common/event.c) Why should any board with CONFIG_EFI_LOADER=n depend on events? Could it be that we have some other problem which by chance is hidden when addresses are moved by enabling events? Best regards Heinrich
Re: [PATCH 1/1] doc: man-page for source command
On 1/14/23 12:41, Heinrich Schuchardt wrote:
On 1/14/23 18:07, Sean Anderson wrote:
On 1/14/23 06:51, Heinrich Schuchardt wrote:
Provide a man-page for the source command.
Signed-off-by: Heinrich Schuchardt
---
doc/usage/cmd/source.rst | 154 +++
doc/usage/index.rst | 1 +
2 files changed, 155 insertions(+)
create mode 100644 doc/usage/cmd/source.rst
diff --git a/doc/usage/cmd/source.rst b/doc/usage/cmd/source.rst
new file mode 100644
index 00..9622f1d5a8
--- /dev/null
+++ b/doc/usage/cmd/source.rst
@@ -0,0 +1,154 @@
+.. SPDX-License-Identifier: GPL-2.0+
+.. Copyright 2022, Heinrich Schuchardt
+
+source command
+==
+
+Synopsis
+
+
+::
+
+ source [][:[]|#[]]
+
+Description
+---
+
+The *source* command is used to execute a script file from memory.
+
+Two formats for script files exist:
+
+* legacy U-Boot image format
+* Flat Image Tree (FIT)
+
+Both formats can be created with the mkimage tool.
+
+addr
+ location of the script file in memory, defaults to CONFIG_SYS_LOAD_ADDR.
+
+image
+ name of an image in a FIT file
+
+config
+ name of a configuration in a FIT file
We need a note here about how these are optional:
Thank you for reviewing.
image
name of an image in a FIT file. May be omitted to pick the default image.
config
name of a configuration in a FIT file. May be omitted to pick the default
config. If addr is not specified, the # must be escaped or quoted to
prevent
it from being interpreted as a comment.
And probably a general note about priorities:
- If : is specified, then *source* will choose an image.
- If # is specified, then *source* will choose an image based on a
configuration.
- If neither : nor # is specified, then *source* will try to choose the image
in the
default configuration. If no configurations are present, then it will pick
the default
image.
And a note about secure boot:
If you are using verified boot, signing keys are required based on the
We nowhere describe what we mean by "verified boot". EFI secure boot is also a
type of verified boot but it does not rely on anything in U-Boot's device-tree.
"U-Boot verified boot"? This is what the uImage stuff calls it.
It think it would be enough to add the reference:
doc/uImage.FIT/signature.txt describes how FIT images including scripts can be
signed and verified.
All text documents in doc/uImage should be converted to restructured text and
added to the HTML documentation.
I agree.
value of the "required"
property in the key's node in U-Boot's device tree. If the value of this property is
"image",
then scripts will always be verified. However, if the value of this node is
"conf", then scripts
will only be verified when a # is specified, as this forces the image to be
determined based on
a configuration. For more information, refer to doc/uImage.FIT/signature.txt.
Additionally, as
is typical, legacy images must be disabled for verified boot, as they do not
support signing.
This is easy to misread.
CONFIG_LEGACY_IMAGE_FORMAT=y does not stop the verification of anything.
Probably you mean:
CONFIG_LEGACY_IMAGE_FORMAT should be disabled a hardened systems as legacy
images cannot be signed.
That works too.
+Examples
+
+
+For creating a FIT image an image tree source file (\*.its) is needed. Here is
+an example (source.its).
+
+.. code-block::
+
+ /dts-v1/;
+
+ / {
+ description = "FIT image to test the source command";
+ #address-cells = <1>;
+
+ images {
+ default = "script-1";
+
+ script-1 {
+ data = "echo 1";
We should use /incbin/ here, since that is more typical.
We should mention /incbin/ below. Here I want an example that is trivial to
understand.
+ type = "script";
+ compression = "none";
+ };
+
+ script-2 {
+ data = "echo 2";
+ type = "script";
+ compression = "none";
+ };
+ };
+
+ configurations {
+ default = "conf-2";
+
+ conf-1 {
+ script = "script-1";
+ };
+
+ conf-2 {
+ script = "script-2";
+ };
And omit the second script/config.
I want to be able to demonstrate what #config is used for.
This is not possible without a second config.
We should mention that the configurations node is optional.
+ };
+ };
+
+The FIT image file (boot.itb) is created with:
+
+.. code-block:: bash
+
+ mkimage -f source.its boot.itb
+
+In U-Boot the script can be loaded and execute like this
+
+.. code-block::
+
+ => load host 0:1 $loadaddr boot.itb
+ 1552 bytes read in 0 ms
+ => source $loadaddr#conf-1
+ ## Executing script at
+ 1
+ => source $loadaddr#conf-2
+ ## Executing script at
+ 2
+ => source $loadaddr:script1
+ ##
Re: [PATCH v2 01/22] apalis-imx8: fix booting caused by missing dm_event
On Sat, Jan 14, 2023 at 12:22:24PM -0500, Tom Rini wrote: > On Sat, Jan 14, 2023 at 11:57:20AM -0500, Tom Rini wrote: > > On Sat, Jan 14, 2023 at 08:27:25AM -0500, Tom Rini wrote: > > > On Sat, Jan 14, 2023 at 08:25:19AM -0300, Fabio Estevam wrote: > > > > Hi Francesco, > > > > > > > > On Fri, Jan 13, 2023 at 2:18 PM Francesco Dolcini > > > > wrote: > > > > > > > > > > From: Marcel Ziswiler > > > > > > > > > > Without the DM_EVENT absolutely no output whatsoever and the system > > > > > does > > > > > not boot at all. > > > > > > > > Why do we need to select CONFIG_EVENT=y in the first place? > > > > > > > > Oliver on CC reported the same issue for his board, and we have other > > > > broken boards too > > > > that do not select CONFIG_EVENT=y. > > > > > > > > Can we have a generic fix for this? > > > > > > > > Tom, Heinrich? > > > > > > Yes, if someone can explain what the problem is, we can correct the > > > dependency. I gather most platforms have whatever this problem is > > > ignored due to enabling EFI_LOADER. > > > > The more I look at this, the more confused I get. With DEBUG_UART > > enabled, where do thing fail without CONFIG_EVENT (and CONFIG_DM_EVENT, > > which is then enabled due to "imply DM_EVENT" in arch/arm/Kconfig, under > > ARCH_IMX8) ? How about if you turn DM_EVENT back off? > > The only other data point I can add right now is that it's not entirely > generic, I think. I can disable EFI_LOADER and EVENT and DM_EVENT on > am65x_evm (both r5 and a53 portions) and still boot up fine. I don't > have something imx8 in my lab, however. Wait, no, I finally found it I think: commit 7fe32b3442f0d0e77a0768dcc1ee65fb352a080a Author: Simon Glass Date: Fri Mar 4 08:43:05 2022 -0700 event: Convert arch_cpu_init_dm() to use events Instead of a special function, send an event after driver model is inited and adjust the boards which use this function. Signed-off-by: Simon Glass That uses "imply DM_EVENT" when it needs to be using select. I'll post a patch shortly. -- Tom signature.asc Description: PGP signature
Re: [PATCH 1/1] doc: man-page for source command
On 1/14/23 18:07, Sean Anderson wrote:
On 1/14/23 06:51, Heinrich Schuchardt wrote:
Provide a man-page for the source command.
Signed-off-by: Heinrich Schuchardt
---
doc/usage/cmd/source.rst | 154 +++
doc/usage/index.rst | 1 +
2 files changed, 155 insertions(+)
create mode 100644 doc/usage/cmd/source.rst
diff --git a/doc/usage/cmd/source.rst b/doc/usage/cmd/source.rst
new file mode 100644
index 00..9622f1d5a8
--- /dev/null
+++ b/doc/usage/cmd/source.rst
@@ -0,0 +1,154 @@
+.. SPDX-License-Identifier: GPL-2.0+
+.. Copyright 2022, Heinrich Schuchardt
+
+source command
+==
+
+Synopsis
+
+
+::
+
+ source [][:[]|#[]]
+
+Description
+---
+
+The *source* command is used to execute a script file from memory.
+
+Two formats for script files exist:
+
+* legacy U-Boot image format
+* Flat Image Tree (FIT)
+
+Both formats can be created with the mkimage tool.
+
+addr
+ location of the script file in memory, defaults to
CONFIG_SYS_LOAD_ADDR.
+
+image
+ name of an image in a FIT file
+
+config
+ name of a configuration in a FIT file
We need a note here about how these are optional:
Thank you for reviewing.
image
name of an image in a FIT file. May be omitted to pick the default
image.
config
name of a configuration in a FIT file. May be omitted to pick the
default
config. If addr is not specified, the # must be escaped or quoted
to prevent
it from being interpreted as a comment.
And probably a general note about priorities:
- If : is specified, then *source* will choose an image.
- If # is specified, then *source* will choose an image based on a
configuration.
- If neither : nor # is specified, then *source* will try to choose the
image in the
default configuration. If no configurations are present, then it will
pick the default
image.
And a note about secure boot:
If you are using verified boot, signing keys are required based on the
We nowhere describe what we mean by "verified boot". EFI secure boot is
also a type of verified boot but it does not rely on anything in
U-Boot's device-tree.
It think it would be enough to add the reference:
doc/uImage.FIT/signature.txt describes how FIT images including scripts
can be signed and verified.
All text documents in doc/uImage should be converted to restructured
text and added to the HTML documentation.
value of the "required"
property in the key's node in U-Boot's device tree. If the value of this
property is "image",
then scripts will always be verified. However, if the value of this node
is "conf", then scripts
will only be verified when a # is specified, as this forces the image to
be determined based on
a configuration. For more information, refer to
doc/uImage.FIT/signature.txt. Additionally, as
is typical, legacy images must be disabled for verified boot, as they do
not support signing.
This is easy to misread.
CONFIG_LEGACY_IMAGE_FORMAT=y does not stop the verification of anything.
Probably you mean:
CONFIG_LEGACY_IMAGE_FORMAT should be disabled a hardened systems as
legacy images cannot be signed.
+Examples
+
+
+For creating a FIT image an image tree source file (\*.its) is
needed. Here is
+an example (source.its).
+
+.. code-block::
+
+ /dts-v1/;
+
+ / {
+ description = "FIT image to test the source command";
+ #address-cells = <1>;
+
+ images {
+ default = "script-1";
+
+ script-1 {
+ data = "echo 1";
We should use /incbin/ here, since that is more typical.
We should mention /incbin/ below. Here I want an example that is trivial
to understand.
+ type = "script";
+ compression = "none";
+ };
+
+ script-2 {
+ data = "echo 2";
+ type = "script";
+ compression = "none";
+ };
+ };
+
+ configurations {
+ default = "conf-2";
+
+ conf-1 {
+ script = "script-1";
+ };
+
+ conf-2 {
+ script = "script-2";
+ };
And omit the second script/config.
I want to be able to demonstrate what #config is used for.
This is not possible without a second config.
We should mention that the configurations node is optional.
+ };
+ };
+
+The FIT image file (boot.itb) is created with:
+
+.. code-block:: bash
+
+ mkimage -f source.its boot.itb
+
+In U-Boot the script can be loaded and execute like this
+
+.. code-block::
+
+ => load host 0:1 $loadaddr boot.itb
+ 1552 bytes read in 0 ms
+ => source $loadaddr#conf-1
+ ## Executing script at
+ 1
+ => source $loadaddr#conf-2
+ ## Executing script at
+ 2
+ => source $loadaddr:script1
+ ## Executing script at
+ Can't find 'script1' FIT subimage
+ => source $loadaddr:script-1
+ ## Executing script
Re: [PATCH v2 10/22] apalis-imx8: update spdx license identifier string
On Fri, Jan 13, 2023 at 06:17:39PM +0100, Francesco Dolcini wrote: > From: Marcel Ziswiler > > Update SPDX license identifier string. > > While at it also update copyright period. > > Signed-off-by: Marcel Ziswiler > Signed-off-by: Francesco Dolcini > --- > v2: no changes Please don't do this. We have far more "GPL-2.0+" (almost 1) than we do "GPL-2.0-or-later" (225). And updating copyright years without any other change isn't meaningful. If your legal department is insisting it must be done, fine, please have them Signed-off-by the change. But for the dts file, please get that by re-syncing with the kernel instead. -- Tom signature.asc Description: PGP signature
Re: [PATCH v2 01/22] apalis-imx8: fix booting caused by missing dm_event
On Sat, Jan 14, 2023 at 11:57:20AM -0500, Tom Rini wrote: > On Sat, Jan 14, 2023 at 08:27:25AM -0500, Tom Rini wrote: > > On Sat, Jan 14, 2023 at 08:25:19AM -0300, Fabio Estevam wrote: > > > Hi Francesco, > > > > > > On Fri, Jan 13, 2023 at 2:18 PM Francesco Dolcini > > > wrote: > > > > > > > > From: Marcel Ziswiler > > > > > > > > Without the DM_EVENT absolutely no output whatsoever and the system does > > > > not boot at all. > > > > > > Why do we need to select CONFIG_EVENT=y in the first place? > > > > > > Oliver on CC reported the same issue for his board, and we have other > > > broken boards too > > > that do not select CONFIG_EVENT=y. > > > > > > Can we have a generic fix for this? > > > > > > Tom, Heinrich? > > > > Yes, if someone can explain what the problem is, we can correct the > > dependency. I gather most platforms have whatever this problem is > > ignored due to enabling EFI_LOADER. > > The more I look at this, the more confused I get. With DEBUG_UART > enabled, where do thing fail without CONFIG_EVENT (and CONFIG_DM_EVENT, > which is then enabled due to "imply DM_EVENT" in arch/arm/Kconfig, under > ARCH_IMX8) ? How about if you turn DM_EVENT back off? The only other data point I can add right now is that it's not entirely generic, I think. I can disable EFI_LOADER and EVENT and DM_EVENT on am65x_evm (both r5 and a53 portions) and still boot up fine. I don't have something imx8 in my lab, however. -- Tom signature.asc Description: PGP signature
Re: [PATCH 1/1] doc: man-page for source command
On 1/14/23 06:51, Heinrich Schuchardt wrote:
Provide a man-page for the source command.
Signed-off-by: Heinrich Schuchardt
---
doc/usage/cmd/source.rst | 154 +++
doc/usage/index.rst | 1 +
2 files changed, 155 insertions(+)
create mode 100644 doc/usage/cmd/source.rst
diff --git a/doc/usage/cmd/source.rst b/doc/usage/cmd/source.rst
new file mode 100644
index 00..9622f1d5a8
--- /dev/null
+++ b/doc/usage/cmd/source.rst
@@ -0,0 +1,154 @@
+.. SPDX-License-Identifier: GPL-2.0+
+.. Copyright 2022, Heinrich Schuchardt
+
+source command
+==
+
+Synopsis
+
+
+::
+
+source [][:[]|#[]]
+
+Description
+---
+
+The *source* command is used to execute a script file from memory.
+
+Two formats for script files exist:
+
+* legacy U-Boot image format
+* Flat Image Tree (FIT)
+
+Both formats can be created with the mkimage tool.
+
+addr
+location of the script file in memory, defaults to CONFIG_SYS_LOAD_ADDR.
+
+image
+name of an image in a FIT file
+
+config
+name of a configuration in a FIT file
We need a note here about how these are optional:
image
name of an image in a FIT file. May be omitted to pick the default image.
config
name of a configuration in a FIT file. May be omitted to pick the default
config. If addr is not specified, the # must be escaped or quoted to prevent
it from being interpreted as a comment.
And probably a general note about priorities:
- If : is specified, then *source* will choose an image.
- If # is specified, then *source* will choose an image based on a
configuration.
- If neither : nor # is specified, then *source* will try to choose the image
in the
default configuration. If no configurations are present, then it will pick
the default
image.
And a note about secure boot:
If you are using verified boot, signing keys are required based on the value of the
"required"
property in the key's node in U-Boot's device tree. If the value of this property is
"image",
then scripts will always be verified. However, if the value of this node is
"conf", then scripts
will only be verified when a # is specified, as this forces the image to be
determined based on
a configuration. For more information, refer to doc/uImage.FIT/signature.txt.
Additionally, as
is typical, legacy images must be disabled for verified boot, as they do not
support signing.
+Examples
+
+
+For creating a FIT image an image tree source file (\*.its) is needed. Here is
+an example (source.its).
+
+.. code-block::
+
+/dts-v1/;
+
+/ {
+description = "FIT image to test the source command";
+#address-cells = <1>;
+
+images {
+default = "script-1";
+
+script-1 {
+data = "echo 1";
We should use /incbin/ here, since that is more typical.
+type = "script";
+compression = "none";
+};
+
+script-2 {
+data = "echo 2";
+type = "script";
+compression = "none";
+};
+};
+
+configurations {
+default = "conf-2";
+
+conf-1 {
+script = "script-1";
+};
+
+conf-2 {
+script = "script-2";
+};
And omit the second script/config.
+};
+};
+
+The FIT image file (boot.itb) is created with:
+
+.. code-block:: bash
+
+mkimage -f source.its boot.itb
+
+In U-Boot the script can be loaded and execute like this
+
+.. code-block::
+
+=> load host 0:1 $loadaddr boot.itb
+1552 bytes read in 0 ms
+=> source $loadaddr#conf-1
+## Executing script at
+1
+=> source $loadaddr#conf-2
+## Executing script at
+2
+=> source $loadaddr:script1
+## Executing script at
+Can't find 'script1' FIT subimage
+=> source $loadaddr:script-1
+## Executing script at
+1
+=> source $loadaddr:script-2
+## Executing script at
+2
+=> source $loadaddr
+## Executing script at
$loadaddr can be omitted, as it is the default
--Sean
+2
+=> source
+## Executing script at
+2
+=>
+
+A legacy boot script can be created starting with a text file.
+Here is an example file (boot.txt):
+
+.. code-block:: bash
+
+ echo Hello from a script
+ echo ---
+
+The boot scripts (boot.scr) is created with:
+
+.. code-block:: bash
+
+mkimage -T script -n 'Test script' -d boot.txt boot.scr
+
+The script can be execute in U-boot like this:
+
+.. code-block::
+
+=> load host 0:1 $loadaddr boot.scr
+122 bytes read in 0 ms
+=> source $loadaddr
+## Executing script at
+Hello from a script
+---
+=> source
+## Executing script at
+Hello from a script
+---
+=>
+
+Configuration
Re: [PATCH v2 01/22] apalis-imx8: fix booting caused by missing dm_event
On Sat, Jan 14, 2023 at 08:27:25AM -0500, Tom Rini wrote: > On Sat, Jan 14, 2023 at 08:25:19AM -0300, Fabio Estevam wrote: > > Hi Francesco, > > > > On Fri, Jan 13, 2023 at 2:18 PM Francesco Dolcini > > wrote: > > > > > > From: Marcel Ziswiler > > > > > > Without the DM_EVENT absolutely no output whatsoever and the system does > > > not boot at all. > > > > Why do we need to select CONFIG_EVENT=y in the first place? > > > > Oliver on CC reported the same issue for his board, and we have other > > broken boards too > > that do not select CONFIG_EVENT=y. > > > > Can we have a generic fix for this? > > > > Tom, Heinrich? > > Yes, if someone can explain what the problem is, we can correct the > dependency. I gather most platforms have whatever this problem is > ignored due to enabling EFI_LOADER. The more I look at this, the more confused I get. With DEBUG_UART enabled, where do thing fail without CONFIG_EVENT (and CONFIG_DM_EVENT, which is then enabled due to "imply DM_EVENT" in arch/arm/Kconfig, under ARCH_IMX8) ? How about if you turn DM_EVENT back off? -- Tom signature.asc Description: PGP signature
Re: [U-Boot][bug report] clearfog: EMMC boot broken on clearfog pro
Hello again > So in order to move forward, the current plan is to find out/bisect first > commit after v2021.01 which has introduced this BootROM SD/eMMC/SATA boot > regression and inform involved folks (Author and Commiter) accordingly with a > proper bug report. I did try to bisect the eMMC issue, but there are a few related breakages for different reasons over the last 2 years, so it's not one commit that has done it. For example, just the 4 changes I listed for kwbimage.c are introduced by the following 5 commits: 5c61710c9880290d54db72878c4435cdaee07d78 501a54a29cc20ce7df70f290fa274b8e2ea9d6f4 e0c243c398a771df22fd3caf309b04eef3ba2683 700ea98b2e364a8107a9af962ba39f2eeadfc678 aa6943ca3122db7ea0063684f94b5dbdf9c5cf51 > Since you're able to boot from SATA, it would help to know if it's broken as > well. Having BootROM output with failing header offsets would be nice as well. SATA boot works for me up until around 2022.10-rc4. The following configs were required on top of the clearfog_defconfig: CONFIG_MVEBU_SPL_BOOT_DEVICE_SATA=y CONFIG_SPL_SATA=y CONFIG_SPL_FS_EXT4=y UART boot and SPI boot work, too. After the commit I tested (1977d72a69f3c8d97bd25a86a6be4da27cde3724) some mvebu timer changes got merged and __udelay gets stuck in an infinite loop while probing hardware. I'm still trying to pin that one down, but it seems entirely unrelated to BootROM finding invalid headers. In case you still need it, here's the BootROM output for SATA with no u-boot installed: BootROM - 1.73 Booting from AHCI Probing HBA- 0 port 0 SATA device found BootROM: Bad header at offset 0001 BootROM: Bad header at offset 0022 BootROM: Bad header at offset 1000 BootROM: Bad header at offset 2000 BootROM: Bad header at offset 3000 BootROM: Bad header at offset 4000 BootROM: Bad header at offset 5000 BootROM: Bad header at offset 6000 BootROM: Bad header at offset 7000 Also, I found a non-eMMC clearfog base to test SD card boot. I had assumed that SD card boot was working, therefore my patches would have broken it. It appears the inverse is true. I can boot from a SD card with the same changes I posted that fix eMMC. That means my changes address two different issues: 1. eMMC not being detected, which could be fixed with a CONFIG_CLEARFOG_EMMC kconfig and ifdef for the dts file, similar to the existing clearfog kconfigs 2. MMC boot (both eMMC and SD card) not working, which is fixed by removing the special handling that converts to/from sectors in the SDIO case and allowing a 1 sector offset for CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR How 2 gets implemented depends on what subset of devices MMC boot is broken for. If all devices are broken, my changes would fix it cleanly, otherwise the changes would need to be wrapped in some kind of ifdef which would mean the kwboot tool is no longer universal but must be compiled for specific boards. Let me know if there is any other data that would be useful. Regards Martin
Re: [PATCH v2 01/22] apalis-imx8: fix booting caused by missing dm_event
On Sat, Jan 14, 2023 at 08:25:19AM -0300, Fabio Estevam wrote: > Hi Francesco, > > On Fri, Jan 13, 2023 at 2:18 PM Francesco Dolcini > wrote: > > > > From: Marcel Ziswiler > > > > Without the DM_EVENT absolutely no output whatsoever and the system does > > not boot at all. > > Why do we need to select CONFIG_EVENT=y in the first place? > > Oliver on CC reported the same issue for his board, and we have other > broken boards too > that do not select CONFIG_EVENT=y. > > Can we have a generic fix for this? > > Tom, Heinrich? Yes, if someone can explain what the problem is, we can correct the dependency. I gather most platforms have whatever this problem is ignored due to enabling EFI_LOADER. -- Tom signature.asc Description: PGP signature
[PATCH 1/1] doc: man-page for source command
Provide a man-page for the source command.
Signed-off-by: Heinrich Schuchardt
---
doc/usage/cmd/source.rst | 154 +++
doc/usage/index.rst | 1 +
2 files changed, 155 insertions(+)
create mode 100644 doc/usage/cmd/source.rst
diff --git a/doc/usage/cmd/source.rst b/doc/usage/cmd/source.rst
new file mode 100644
index 00..9622f1d5a8
--- /dev/null
+++ b/doc/usage/cmd/source.rst
@@ -0,0 +1,154 @@
+.. SPDX-License-Identifier: GPL-2.0+
+.. Copyright 2022, Heinrich Schuchardt
+
+source command
+==
+
+Synopsis
+
+
+::
+
+source [][:[]|#[]]
+
+Description
+---
+
+The *source* command is used to execute a script file from memory.
+
+Two formats for script files exist:
+
+* legacy U-Boot image format
+* Flat Image Tree (FIT)
+
+Both formats can be created with the mkimage tool.
+
+addr
+location of the script file in memory, defaults to CONFIG_SYS_LOAD_ADDR.
+
+image
+name of an image in a FIT file
+
+config
+name of a configuration in a FIT file
+
+Examples
+
+
+For creating a FIT image an image tree source file (\*.its) is needed. Here is
+an example (source.its).
+
+.. code-block::
+
+/dts-v1/;
+
+/ {
+description = "FIT image to test the source command";
+#address-cells = <1>;
+
+images {
+default = "script-1";
+
+script-1 {
+data = "echo 1";
+type = "script";
+compression = "none";
+};
+
+script-2 {
+data = "echo 2";
+type = "script";
+compression = "none";
+};
+};
+
+configurations {
+default = "conf-2";
+
+conf-1 {
+script = "script-1";
+};
+
+conf-2 {
+script = "script-2";
+};
+};
+};
+
+The FIT image file (boot.itb) is created with:
+
+.. code-block:: bash
+
+mkimage -f source.its boot.itb
+
+In U-Boot the script can be loaded and execute like this
+
+.. code-block::
+
+=> load host 0:1 $loadaddr boot.itb
+1552 bytes read in 0 ms
+=> source $loadaddr#conf-1
+## Executing script at
+1
+=> source $loadaddr#conf-2
+## Executing script at
+2
+=> source $loadaddr:script1
+## Executing script at
+Can't find 'script1' FIT subimage
+=> source $loadaddr:script-1
+## Executing script at
+1
+=> source $loadaddr:script-2
+## Executing script at
+2
+=> source $loadaddr
+## Executing script at
+2
+=> source
+## Executing script at
+2
+=>
+
+A legacy boot script can be created starting with a text file.
+Here is an example file (boot.txt):
+
+.. code-block:: bash
+
+ echo Hello from a script
+ echo ---
+
+The boot scripts (boot.scr) is created with:
+
+.. code-block:: bash
+
+mkimage -T script -n 'Test script' -d boot.txt boot.scr
+
+The script can be execute in U-boot like this:
+
+.. code-block::
+
+=> load host 0:1 $loadaddr boot.scr
+122 bytes read in 0 ms
+=> source $loadaddr
+## Executing script at
+Hello from a script
+---
+=> source
+## Executing script at
+Hello from a script
+---
+=>
+
+Configuration
+-
+
+The source command is only available if CONFIG_CMD_SOURCE=y.
+The FIT image file format requires CONFIG_FIT=y.#
+The legacy U-Boot image file format requires CONFIG_LEGACY_IMAGE_FORMAT=y.
+
+Return value
+
+
+If the scripts is executed successfully, the return value $? is 0 (true).
+Otherwise it is 1 (false).
diff --git a/doc/usage/index.rst b/doc/usage/index.rst
index bbd40a6e18..14457aba69 100644
--- a/doc/usage/index.rst
+++ b/doc/usage/index.rst
@@ -74,6 +74,7 @@ Shell commands
cmd/setexpr
cmd/size
cmd/sound
+ cmd/source
cmd/temperature
cmd/tftpput
cmd/true
--
2.37.2
[PATCH 1/1] configs: enable CONFIG_LEGACY_IMAGE_FORMAT on sandbox
The sandbox should be able to execute scripts in legacy U-Boot image format for testing. Signed-off-by: Heinrich Schuchardt --- configs/sandbox_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index de799b5cea..6239d495db 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -13,6 +13,7 @@ CONFIG_FIT=y CONFIG_FIT_RSASSA_PSS=y CONFIG_FIT_CIPHER=y CONFIG_FIT_VERBOSE=y +CONFIG_LEGACY_IMAGE_FORMAT=y CONFIG_BOOTSTAGE=y CONFIG_BOOTSTAGE_REPORT=y CONFIG_BOOTSTAGE_FDT=y -- 2.37.2
Re: [PATCH v2 00/22] apalis-imx8: boot issue fix and support refresh
Hi Francesco, On Fri, Jan 13, 2023 at 2:18 PM Francesco Dolcini wrote: > > From: Francesco Dolcini > > This series fixes a boot issue and refreshes Apalis iMX8 support as > follows: The whole series looks good. > - fix booting caused by missing DM_EVENT My only comment is that this one should be fixed in a global way, as it does not affect apalis-imx8 only. With this fixed, for all the series: Reviewed-by: Fabio Estevam
Re: [PATCH v2 01/22] apalis-imx8: fix booting caused by missing dm_event
Hi Francesco, On Fri, Jan 13, 2023 at 2:18 PM Francesco Dolcini wrote: > > From: Marcel Ziswiler > > Without the DM_EVENT absolutely no output whatsoever and the system does > not boot at all. Why do we need to select CONFIG_EVENT=y in the first place? Oliver on CC reported the same issue for his board, and we have other broken boards too that do not select CONFIG_EVENT=y. Can we have a generic fix for this? Tom, Heinrich?
Re: [PATCH v3 3/3] eficonfig: add vertical scroll support
On 1/14/23 11:06, Heinrich Schuchardt wrote:
On 1/5/23 03:58, Masahisa Kojima wrote:
The current eficonfig menu does not support vertical scroll,
so it can not display the menu entries greater than
the console row size.
This commit add the vertial scroll support.
The console size is retrieved by
SIMPLE_TEXT_OUTPUT_PROTOCOL.QueryMode() service, then
calculates the row size for menu entry by subtracting
menu header and description row size from the console row size.
"start" and "end" are added in the efimenu structure.
"start" keeps the menu entry index at the top, "end" keeps
the bottom menu entry index. item_data_print() menu function
only draws the menu entry between "start" and "end".
Signed-off-by: Masahisa Kojima
Hello Masahisa,
unfortunately this does not work.
I create a boot.scr script to create 100 boot options:
#!/bin/bash
a=0
while [ $a -lt 100 ]; do
b="$a"
c=${b: -4}
echo "efidebug boot add -b $c label$c host 0:1 dtbdump.efi" >> boot.txt
a=$(( $a + 1 ))
done
mkimage -T script -n foo -d boot.txt boot.scr
rm boot.txt
In sandbox_defconfig:
host bind 0 ../sandbox.img
host load 0:1 $kernel_addr_r boot.scr
source $kernel_addr_r
efidebug boot dump
Now I have 100 boot options.
I start 'eficonfig'. Whenever I push the up or down key this immediately
leads to exiting the command and I am back at the console prompt.
Best regards
Heinrich
The problem seems to be that the space for variables is exhausted.
The user should see an error message in this case.
With CONFIG_EFI_VAR_BUF_SIZE=65536 the navigation works but I only see
entries label - label0095. Where are the other entries?
I have been adding CONFIG_LEGACY_IMAGE_FORMAT=y to read the boot.scr file.
Best regards
Heinrich
Re: [PATCH v3 3/3] eficonfig: add vertical scroll support
On 1/5/23 03:58, Masahisa Kojima wrote:
The current eficonfig menu does not support vertical scroll,
so it can not display the menu entries greater than
the console row size.
This commit add the vertial scroll support.
The console size is retrieved by
SIMPLE_TEXT_OUTPUT_PROTOCOL.QueryMode() service, then
calculates the row size for menu entry by subtracting
menu header and description row size from the console row size.
"start" and "end" are added in the efimenu structure.
"start" keeps the menu entry index at the top, "end" keeps
the bottom menu entry index. item_data_print() menu function
only draws the menu entry between "start" and "end".
Signed-off-by: Masahisa Kojima
Hello Masahisa,
unfortunately this does not work.
I create a boot.scr script to create 100 boot options:
#!/bin/bash
a=0
while [ $a -lt 100 ]; do
b="$a"
c=${b: -4}
echo "efidebug boot add -b $c label$c host 0:1 dtbdump.efi" >> boot.txt
a=$(( $a + 1 ))
done
mkimage -T script -n foo -d boot.txt boot.scr
rm boot.txt
In sandbox_defconfig:
host bind 0 ../sandbox.img
host load 0:1 $kernel_addr_r boot.scr
source $kernel_addr_r
efidebug boot dump
Now I have 100 boot options.
I start 'eficonfig'. Whenever I push the up or down key this immediately
leads to exiting the command and I am back at the console prompt.
Best regards
Heinrich
---
No update since v1
cmd/eficonfig.c | 79
include/efi_config.h | 4 +++
include/efi_loader.h | 1 +
3 files changed, 70 insertions(+), 14 deletions(-)
diff --git a/cmd/eficonfig.c b/cmd/eficonfig.c
index 6d5ebc4055..bf765a239b 100644
--- a/cmd/eficonfig.c
+++ b/cmd/eficonfig.c
@@ -29,8 +29,13 @@ static const char *eficonfig_change_boot_order_desc =
" Press SPACE to activate or deactivate the entry\n"
" Select [Save] to complete, ESC/CTRL+C to quit";
+static struct efi_simple_text_output_protocol *cout;
+static int avail_row;
+
#define EFICONFIG_DESCRIPTION_MAX 32
#define EFICONFIG_OPTIONAL_DATA_MAX 64
+#define EFICONFIG_MENU_HEADER_ROW_NUM 3
+#define EFICONFIG_MENU_DESC_ROW_NUM 5
/**
* struct eficonfig_filepath_info - structure to be used to store file path
@@ -156,18 +161,16 @@ void eficonfig_print_entry(void *data)
struct eficonfig_entry *entry = data;
bool reverse = (entry->efi_menu->active == entry->num);
- /* TODO: support scroll or page for many entries */
+ if (entry->efi_menu->start > entry->num || entry->efi_menu->end <
entry->num)
+ return;
- /*
-* Move cursor to line where the entry will be drawn (entry->num)
-* First 3 lines(menu header) + 1 empty line
-*/
- printf(ANSI_CURSOR_POSITION, entry->num + 4, 7);
+ printf(ANSI_CURSOR_POSITION, (entry->num - entry->efi_menu->start) +
+ EFICONFIG_MENU_HEADER_ROW_NUM + 1, 7);
if (reverse)
puts(ANSI_COLOR_REVERSE);
- printf("%s", entry->title);
+ printf(ANSI_CLEAR_LINE "%s", entry->title);
if (reverse)
puts(ANSI_COLOR_RESET);
@@ -190,8 +193,8 @@ void eficonfig_display_statusline(struct menu *m)
ANSI_CURSOR_POSITION ANSI_CLEAR_LINE ANSI_CURSOR_POSITION
"%s"
ANSI_CLEAR_LINE_TO_END,
- 1, 1, entry->efi_menu->menu_header, entry->efi_menu->count + 5,
1,
- entry->efi_menu->count + 6, 1, entry->efi_menu->menu_desc);
+ 1, 1, entry->efi_menu->menu_header, avail_row + 4, 1,
+ avail_row + 5, 1, entry->efi_menu->menu_desc);
}
/**
@@ -213,13 +216,23 @@ char *eficonfig_choice_entry(void *data)
switch (key) {
case KEY_UP:
- if (efi_menu->active > 0)
+ if (efi_menu->active > 0) {
--efi_menu->active;
+ if (efi_menu->start > efi_menu->active) {
+ efi_menu->start--;
+ efi_menu->end--;
+ }
+ }
/* no menu key selected, regenerate menu */
return NULL;
case KEY_DOWN:
- if (efi_menu->active < efi_menu->count - 1)
+ if (efi_menu->active < efi_menu->count - 1) {
++efi_menu->active;
+ if (efi_menu->end < efi_menu->active) {
+ efi_menu->start++;
+ efi_menu->end++;
+ }
+ }
/* no menu key selected, regenerate menu */
return NULL;
case KEY_SELECT:
@@ -399,6 +412,8 @@ efi_status_t eficonfig_process_common(struct efimenu
*efi_menu,
efi_menu->delay = -1;
efi_menu->active = 0;
+
tools/ifwitool.c:573:26: warning: array subscript [28, 51539607568] is outside array bounds of ‘struct bpdt[0]’ [-Warray-bounds
Hello Mikhail, compiling sandbox_defconfig with gives me a lot of these warnings with gcc 12.2 (as available in Ubuntu 23.04 Lunar). As you have been contributing to tools/ifwitool.c lately you might want to have a look at it. tools/ifwitool.c:1435:22: note: at offset [32, 51539607572] into object ‘s’ of size 8 1435 | struct bpdt *s = buffer_get(buf); | ^ In function ‘write_ble8’, inlined from ‘write_le8’ at tools/ifwitool.c:591:2, inlined from ‘write_le16’ at tools/ifwitool.c:616:2, inlined from ‘write_le32’ at tools/ifwitool.c:646:2, inlined from ‘write_at_le32’ at tools/ifwitool.c:655:2, inlined from ‘fix_member’ at tools/ifwitool.c:734:3, inlined from ‘bpdt_fixup_write_buffer’ at tools/ifwitool.c:1457:12, inlined from ‘bpdt_write’ at tools/ifwitool.c:1464:2: tools/ifwitool.c:573:26: warning: array subscript [32, 51539607572] is outside array bounds of ‘struct bpdt[0]’ [-Warray-bounds] 573 | *(uint8_t *)dest = val; | ~^ tools/ifwitool.c: In function ‘bpdt_write’: tools/ifwitool.c:1435:22: note: at offset [32, 51539607572] into object ‘s’ of size 8 1435 | struct bpdt *s = buffer_get(buf); | ^ Best regards Heinrich
