Re: [PATCH v4 00/17] IPv6 support
Hello Tom! I tested this problem and I think it is necessary to set CONFIG_IPV6. Without that option tests fail. Thanks! От: Tom Rini Отправлено: 28 ноября 2022 г. 18:34:58 Кому: Vyacheslav Mitrofanov V Копия: rfried@gmail.com; joe.hershber...@ni.com; w...@denx.de; u-boot@lists.denx.de; judge.pack...@gmail.com; li...@yadro.com; s...@chromium.org Тема: Re: [PATCH v4 00/17] IPv6 support On Thu, Sep 08, 2022 at 02:58:48PM +0300, Viacheslav Mitrofanov wrote: > This patch set adds basic IPv6 support to U-boot. > It is based on Chris's Packham patches > (https://lists.denx.de/pipermail/u-boot/2017-January/279366.html) > Chris's patches were taken as base. There were efforts to launch it on > HiFive SiFive Unmatched board but the board didn't work well. The code was > refactored, fixed some bugs as CRC for little-endian, some parts were > implemented in > our own way, something was taken from Linux. Finally we did manual tests and > the > board worked well. > > Testing was done on HiFive SiFive Unmatched board (RISC-V) > > Signed-off-by: Viacheslav Mitrofanov > --- > Changes in v2: > - Split big patches into smaller > - If an address in tftpboot is IPv6 than use IPv6 to boot > - Add tests > > Changes in v3: > - Added functions and structures description in whole patch-series > - Removed memory allocation in on_ip6addr() > - Some functions got return code from errno.h > - Add to string_to_ip6() length parameter to avoid obligatory null > termination > - Add a lot of small decorative cnages > > Changes in v4: > - Fixed funcs and structures style description > - Added omitted tags Testing this locally in sandbox I see: FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_csum_ipv6_magic] - AssertionError: as... FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_ip6_addr_in_subnet] - AssertionError:... FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_ip6_make_lladdr] - assert False FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_ip6_make_snma] - assert False FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_string_to_ip6] - AssertionError: asse... and this happens in CI as well: https://source.denx.de/u-boot/u-boot/-/pipelines/14245 https://dev.azure.com/u-boot/u-boot/_build/results?buildId=5432&view=results If you can't replicate the failures locally you should be able to trigger CI: https://u-boot.readthedocs.io/en/latest/develop/ci_testing.html Thanks for pushing this along! -- Tom
Re: [PATCH v4 00/17] IPv6 support
Tom, maybe it is better to change configs add ifdefs or do sth else to exclude them from the build if IPV6 is not configured? Thanks! От: Vyacheslav Mitrofanov V Отправлено: 29 ноября 2022 г. 11:35:13 Кому: Tom Rini Копия: rfried@gmail.com; joe.hershber...@ni.com; w...@denx.de; u-boot@lists.denx.de; judge.pack...@gmail.com; li...@yadro.com; s...@chromium.org Тема: Re: [PATCH v4 00/17] IPv6 support Hello Tom! I tested this problem and I think it is necessary to set CONFIG_IPV6. Without that option tests fail. Thanks! От: Tom Rini Отправлено: 28 ноября 2022 г. 18:34:58 Кому: Vyacheslav Mitrofanov V Копия: rfried@gmail.com; joe.hershber...@ni.com; w...@denx.de; u-boot@lists.denx.de; judge.pack...@gmail.com; li...@yadro.com; s...@chromium.org Тема: Re: [PATCH v4 00/17] IPv6 support On Thu, Sep 08, 2022 at 02:58:48PM +0300, Viacheslav Mitrofanov wrote: > This patch set adds basic IPv6 support to U-boot. > It is based on Chris's Packham patches > (https://lists.denx.de/pipermail/u-boot/2017-January/279366.html) > Chris's patches were taken as base. There were efforts to launch it on > HiFive SiFive Unmatched board but the board didn't work well. The code was > refactored, fixed some bugs as CRC for little-endian, some parts were > implemented in > our own way, something was taken from Linux. Finally we did manual tests and > the > board worked well. > > Testing was done on HiFive SiFive Unmatched board (RISC-V) > > Signed-off-by: Viacheslav Mitrofanov > --- > Changes in v2: > - Split big patches into smaller > - If an address in tftpboot is IPv6 than use IPv6 to boot > - Add tests > > Changes in v3: > - Added functions and structures description in whole patch-series > - Removed memory allocation in on_ip6addr() > - Some functions got return code from errno.h > - Add to string_to_ip6() length parameter to avoid obligatory null > termination > - Add a lot of small decorative cnages > > Changes in v4: > - Fixed funcs and structures style description > - Added omitted tags Testing this locally in sandbox I see: FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_csum_ipv6_magic] - AssertionError: as... FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_ip6_addr_in_subnet] - AssertionError:... FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_ip6_make_lladdr] - assert False FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_ip6_make_snma] - assert False FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_string_to_ip6] - AssertionError: asse... and this happens in CI as well: https://source.denx.de/u-boot/u-boot/-/pipelines/14245 https://dev.azure.com/u-boot/u-boot/_build/results?buildId=5432&view=results If you can't replicate the failures locally you should be able to trigger CI: https://u-boot.readthedocs.io/en/latest/develop/ci_testing.html Thanks for pushing this along! -- Tom
Re: [PATCH] serial: ns16550: Enable clocks during probe
On 11/28/22 06:48, Samuel Holland wrote: If the UART bus or baud clock has a gate, it must be enabled before the UART can be used. Signed-off-by: Samuel Holland --- drivers/serial/ns16550.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/serial/ns16550.c b/drivers/serial/ns16550.c index 7592979cab5..785fb520062 100644 --- a/drivers/serial/ns16550.c +++ b/drivers/serial/ns16550.c @@ -506,6 +506,7 @@ int ns16550_serial_probe(struct udevice *dev) struct ns16550_plat *plat = dev_get_plat(dev); struct ns16550 *const com_port = dev_get_priv(dev); struct reset_ctl_bulk reset_bulk; + struct clk_bulk clk_bulk; fdt_addr_t addr; int ret; @@ -524,6 +525,10 @@ int ns16550_serial_probe(struct udevice *dev) if (!ret) reset_deassert_bulk(&reset_bulk); + ret = clk_get_bulk(dev, &clk_bulk); + if (!ret) + clk_enable_bulk(&clk_bulk); + com_port->plat = dev_get_plat(dev); ns16550_init(com_port, -1); Reviewed-by: Stefan Roese Thanks, Stefan
Re: [v1] spl: nand: allow partial nand page reads during nand_spl_load_image
Hi Colinn
On Fri, Nov 18, 2022 at 1:08 PM Dario Binacchi
wrote:
>
> Hi Colin,
>
> On Tue, Nov 15, 2022 at 5:35 PM Colin Foster
> wrote:
> >
> > The nand_spl_load_image function was guaranteed to read an entire block
> > into RAM, regardless of how many bytes were to be read. This is
> > particularly problematic when spl_load_legacy_image is called, as this
> > function attempts to load a struct image_header but gets surprised with
> > a full flash sector.
> >
> > Allow partial reads to restore functionality to the code path
> > spl_nand_load_element() -> spl_load_legacy_img() ->
> > spl_nand_legacy_read(load, header, sizeof(hdr), header);
> >
> > Signed-off-by: Colin Foster
> > ---
> > drivers/mtd/nand/raw/nand_spl_loaders.c | 36 -
> > 1 file changed, 24 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/nand_spl_loaders.c
> > b/drivers/mtd/nand/raw/nand_spl_loaders.c
> > index 4befc75c04..84ac482c06 100644
> > --- a/drivers/mtd/nand/raw/nand_spl_loaders.c
> > +++ b/drivers/mtd/nand/raw/nand_spl_loaders.c
> > @@ -1,9 +1,18 @@
> > +/*
> > + * Temporary storage for non NAND page aligned and non NAND page sized
> > + * reads. Note: This does not support runtime detected FLASH yet, but
> > + * that should be reasonably easy to fix by making the buffer large
> > + * enough :)
> > + */
> > +static u8 scratch_buf[CONFIG_SYS_NAND_PAGE_SIZE];
> > +
> > int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
> > {
> > + unsigned int read_this_time = CONFIG_SYS_NAND_PAGE_SIZE;
> > + unsigned int size_remaining = size;
> > unsigned int block, lastblock;
> > unsigned int page, page_offset;
> >
> > - /* offs has to be aligned to a page address! */
> > block = offs / CONFIG_SYS_NAND_BLOCK_SIZE;
> > lastblock = (offs + size - 1) / CONFIG_SYS_NAND_BLOCK_SIZE;
> > page = (offs % CONFIG_SYS_NAND_BLOCK_SIZE) /
> > CONFIG_SYS_NAND_PAGE_SIZE;
> > @@ -12,8 +21,18 @@ int nand_spl_load_image(uint32_t offs, unsigned int
> > size, void *dst)
> > while (block <= lastblock) {
> > if (!nand_is_bad_block(block)) {
> > /* Skip bad blocks */
> > - while (page < CONFIG_SYS_NAND_PAGE_COUNT) {
> > - nand_read_page(block, page, dst);
> > + while (page < CONFIG_SYS_NAND_PAGE_COUNT &&
> > + size_remaining > 0) {
> > + if (size_remaining <
> > CONFIG_SYS_NAND_PAGE_SIZE)
> > + {
> > + nand_read_page(block, page,
> > + scratch_buf);
> > + memcpy(dst, scratch_buf,
> > + size_remaining);
> > + read_this_time = size_remaining;
> > + } else {
> > + nand_read_page(block, page, dst);
> > + }
> > /*
> > * When offs is not aligned to page address
> > the
> > * extra offset is copied to dst as well.
> > Copy
> > @@ -26,7 +45,8 @@ int nand_spl_load_image(uint32_t offs, unsigned int size,
> > void *dst)
> > dst = (void *)((int)dst -
> > page_offset);
> > page_offset = 0;
> > }
> > - dst += CONFIG_SYS_NAND_PAGE_SIZE;
> > + dst += read_this_time;
> > + size_remaining -= read_this_time;
> > page++;
> > }
> >
> > @@ -70,14 +90,6 @@ u32 nand_spl_adjust_offset(u32 sector, u32 offs)
> > }
> >
> > #ifdef CONFIG_SPL_UBI
> > -/*
> > - * Temporary storage for non NAND page aligned and non NAND page sized
> > - * reads. Note: This does not support runtime detected FLASH yet, but
> > - * that should be reasonably easy to fix by making the buffer large
> > - * enough :)
> > - */
> > -static u8 scratch_buf[CONFIG_SYS_NAND_PAGE_SIZE];
> > -
> > /**
> > * nand_spl_read_block - Read data from physical eraseblock into a buffer
> > * @block: Number of the physical eraseblock
> > --
> > 2.25.1
> >
>
> Reviewed-by: Dario Binacchi
>
> Thanks and regards,
> Dario
>
This is the error reported by the CI/CD pipeline:
arm: + corvus
241+arm-linux-gnueabi-ld.bfd: u-boot-spl section `.bss' will not fit
in region `.sdram'
242+arm-linux-gnueabi-ld.bfd: SPL image BSS too big
243+arm-linux-gnueabi-ld.bfd: region `.sdram' overflowed by 1388 bytes
244+make[2]: *** [scripts/Makefile.spl:527: spl/u-boot-spl] Error 1
245+make[1]: *** [Makefile:2074: spl/u-boot-spl] Error 2
246+make: *** [Makef
Re: [PATCH v2 3/5] efi_loader: utility function to check the variable name is "Boot####"
Hi Ilias,
On Tue, 29 Nov 2022 at 16:33, Ilias Apalodimas
wrote:
>
> On Mon, Nov 28, 2022 at 09:45:07PM +0900, Masahisa Kojima wrote:
> > Some commands need to enumerate the existing UEFI load
> > option variable("Boot"). This commit transfers some code
> > from cmd/efidebug.c to lib/efi_loder/, then exposes u16_tohex() and
> > efi_varname_is_load_option() function to check whether the
> > UEFI variable name is "Boot".
> >
> > Signed-off-by: Masahisa Kojima
> > ---
> > Newly created in v2
> >
> > cmd/efidebug.c | 23 +--
> > include/efi_loader.h| 2 ++
> > lib/efi_loader/efi_helper.c | 33 +
> > 3 files changed, 36 insertions(+), 22 deletions(-)
> >
> > diff --git a/cmd/efidebug.c b/cmd/efidebug.c
> > index ef239bb34b..ceb3aa5cee 100644
> > --- a/cmd/efidebug.c
> > +++ b/cmd/efidebug.c
> > @@ -1010,17 +1010,6 @@ static void show_efi_boot_opt(u16 *varname16)
> > }
> > }
> >
> > -static int u16_tohex(u16 c)
> > -{
> > - if (c >= '0' && c <= '9')
> > - return c - '0';
> > - if (c >= 'A' && c <= 'F')
> > - return c - 'A' + 10;
> > -
> > - /* not hexadecimal */
> > - return -1;
> > -}
> > -
> > /**
> > * show_efi_boot_dump() - dump all UEFI load options
> > *
> > @@ -1041,7 +1030,6 @@ static int do_efi_boot_dump(struct cmd_tbl *cmdtp,
> > int flag,
> > u16 *var_name16, *p;
> > efi_uintn_t buf_size, size;
> > efi_guid_t guid;
> > - int id, i, digit;
> > efi_status_t ret;
> >
> > if (argc > 1)
> > @@ -1074,16 +1062,7 @@ static int do_efi_boot_dump(struct cmd_tbl *cmdtp,
> > int flag,
> > return CMD_RET_FAILURE;
> > }
> >
> > - if (memcmp(var_name16, u"Boot", 8))
> > - continue;
> > -
> > - for (id = 0, i = 0; i < 4; i++) {
> > - digit = u16_tohex(var_name16[4 + i]);
> > - if (digit < 0)
> > - break;
> > - id = (id << 4) + digit;
> > - }
> > - if (i == 4 && !var_name16[8])
> > + if (efi_varname_is_load_option(var_name16, NULL))
> > show_efi_boot_opt(var_name16);
> > }
> >
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index 0c6c95ba46..b1ded811e7 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -707,6 +707,8 @@ int algo_to_len(const char *algo);
> >
> > int efi_link_dev(efi_handle_t handle, struct udevice *dev);
> > int efi_unlink_dev(efi_handle_t handle);
> > +int u16_tohex(u16 c);
> > +bool efi_varname_is_load_option(u16 *var_name16, int *index);
> >
> > /**
> > * efi_size_in_pages() - convert size in bytes to size in pages
> > diff --git a/lib/efi_loader/efi_helper.c b/lib/efi_loader/efi_helper.c
> > index c71e87d118..8a7afcc381 100644
> > --- a/lib/efi_loader/efi_helper.c
> > +++ b/lib/efi_loader/efi_helper.c
> > @@ -190,3 +190,36 @@ int efi_unlink_dev(efi_handle_t handle)
> >
> > return 0;
> > }
> > +
> > +int u16_tohex(u16 c)
>
> This needs static
OK, I will fix it.
>
> > +{
> > + if (c >= '0' && c <= '9')
> > + return c - '0';
> > + if (c >= 'A' && c <= 'F')
> > + return c - 'A' + 10;
> > +
> > + /* not hexadecimal */
> > + return -1;
> > +}
> > +
> > +bool efi_varname_is_load_option(u16 *var_name16, int *index)
> > +{
> > + int id, i, digit;
> > +
> > + if (memcmp(var_name16, u"Boot", 8))
> > + return false;
> > +
> > + for (id = 0, i = 0; i < 4; i++) {
> > + digit = u16_tohex(var_name16[4 + i]);
> > + if (digit < 0)
> > + break;
> > + id = (id << 4) + digit;
> > + }
> > + if (i == 4 && !var_name16[8]) {
> > + if (index)
> > + *index = id;
> > + return true;
> > + }
> > +
> > + return false;
> > +}
> > --
> > 2.17.1
> >
>
> With that
> Reviewed-by: Ilias Apalodimas
Thank you for the review.
Regards,
Masahisa Kojima
>
[PATCH] arm64: versal-net: Enable defconfig for Micron octal flashes
From: Ashok Reddy Soma Micron mt35 series octal flashes are under config option CONFIG_SPI_FLASH_MT35XU. Enable it in default defconfig for octal flashes to be detected. Signed-off-by: Ashok Reddy Soma Signed-off-by: Michal Simek --- configs/xilinx_versal_net_virt_defconfig | 1 + 1 file changed, 1 insertion(+) diff --git a/configs/xilinx_versal_net_virt_defconfig b/configs/xilinx_versal_net_virt_defconfig index 431a8de9fcf2..2fdf99f7cbbd 100644 --- a/configs/xilinx_versal_net_virt_defconfig +++ b/configs/xilinx_versal_net_virt_defconfig @@ -87,6 +87,7 @@ CONFIG_SPI_FLASH_ISSI=y CONFIG_SPI_FLASH_MACRONIX=y CONFIG_SPI_FLASH_SPANSION=y CONFIG_SPI_FLASH_STMICRO=y +CONFIG_SPI_FLASH_MT35XU=y CONFIG_SPI_FLASH_SST=y CONFIG_SPI_FLASH_WINBOND=y # CONFIG_SPI_FLASH_USE_4K_SECTORS is not set -- 2.36.1
Re: [PATCH 01/17] android: boot: rename andr_img_hdr -> andr_boot_img_hdr_v0_v1_v2
On Mon, 28 Nov 2022 11:08:20 -0500
Sean Anderson wrote:
> On 11/26/22 11:59, Safae Ouajih wrote:
> > [You don't often get email from soua...@baylibre.com. Learn why this is
> > important at https://aka.ms/LearnAboutSenderIdentification ]
> >
> > Android introduced boot header version 3 or 4.
> > The header structure change with version 3 and 4 to support
> > the new updates such as:
> > - Introducing Vendor boot image: with a vendor ramdisk
> > - Bootconfig feature (v4)
> >
> > To maintain support for version v0, v1 and v2 while introducing
> > version 3 and 4, this struct name must change to refer to the header
> > structure before v3.
> >
> > Signed-off-by: Safae Ouajih
> > ---
> > boot/image-android.c | 26 +-
> > boot/image-fdt.c | 2 +-
> > cmd/abootimg.c| 4 ++--
> > drivers/fastboot/fb_mmc.c | 8
> > include/android_image.h | 4 ++--
> > include/image.h | 18 +-
> > 6 files changed, 31 insertions(+), 31 deletions(-)
> >
> > diff --git a/boot/image-android.c b/boot/image-android.c
> > index 2628db374121..926d94ecbe8e 100644
> > --- a/boot/image-android.c
> > +++ b/boot/image-android.c
> > @@ -18,7 +18,7 @@
> >
> > static char andr_tmp_str[ANDR_BOOT_ARGS_SIZE + 1];
> >
> > -static ulong android_image_get_kernel_addr(const struct andr_img_hdr *hdr)
> > +static ulong android_image_get_kernel_addr(const struct
> > andr_boot_img_hdr_v0_v1_v2 *hdr)
>
> This line is too long. You will have to break it like
>
> static ulong
> android_image_get_kernel_addr(const struct andr_boot_img_hdr_v0_v1_v2 *hdr)
>
> But really, this is because your struct name is too long. Can we use
> something like andr_img_hdr_v0 instead? I would like to have only one
> version in the struct name because then there is no urge to update the
> name when e.g. v5 comes out with mostly the same format. And _boot adds
> nothing.
>
> --Sean
>
Hello Sean,
Thank you for your review.
Since vendor boot image is introduced, we need to differentiate it from the
boot image. Thus,
I suggest to use :
andr_boot_img_hdr_v0 and andr_boot_img_hdr_v3 for the boot image
andr_vendor_img_hdr for the vendor boot image
--Safae
> > {
> > /*
> > * All the Android tools that generate a boot.img use this
> > @@ -59,7 +59,7 @@ static ulong android_image_get_kernel_addr(const struct
> > andr_img_hdr *hdr)
> > * Return: Zero, os start address and length on success,
> > * otherwise on failure.
> > */
> > -int android_image_get_kernel(const struct andr_img_hdr *hdr, int verify,
> > +int android_image_get_kernel(const struct andr_boot_img_hdr_v0_v1_v2 *hdr,
> > int verify,
> > ulong *os_data, ulong *os_len)
> > {
> > u32 kernel_addr = android_image_get_kernel_addr(hdr);
> > @@ -122,12 +122,12 @@ int android_image_get_kernel(const struct
> > andr_img_hdr *hdr, int verify,
> > return 0;
> > }
> >
> > -int android_image_check_header(const struct andr_img_hdr *hdr)
> > +int android_image_check_header(const struct andr_boot_img_hdr_v0_v1_v2
> > *hdr)
> > {
> > return memcmp(ANDR_BOOT_MAGIC, hdr->magic, ANDR_BOOT_MAGIC_SIZE);
> > }
> >
> > -ulong android_image_get_end(const struct andr_img_hdr *hdr)
> > +ulong android_image_get_end(const struct andr_boot_img_hdr_v0_v1_v2 *hdr)
> > {
> > ulong end;
> >
> > @@ -150,12 +150,12 @@ ulong android_image_get_end(const struct andr_img_hdr
> > *hdr)
> > return end;
> > }
> >
> > -ulong android_image_get_kload(const struct andr_img_hdr *hdr)
> > +ulong android_image_get_kload(const struct andr_boot_img_hdr_v0_v1_v2 *hdr)
> > {
> > return android_image_get_kernel_addr(hdr);
> > }
> >
> > -ulong android_image_get_kcomp(const struct andr_img_hdr *hdr)
> > +ulong android_image_get_kcomp(const struct andr_boot_img_hdr_v0_v1_v2 *hdr)
> > {
> > const void *p = (void *)((uintptr_t)hdr + hdr->page_size);
> >
> > @@ -167,7 +167,7 @@ ulong android_image_get_kcomp(const struct andr_img_hdr
> > *hdr)
> > return image_decomp_type(p, sizeof(u32));
> > }
> >
> > -int android_image_get_ramdisk(const struct andr_img_hdr *hdr,
> > +int android_image_get_ramdisk(const struct andr_boot_img_hdr_v0_v1_v2 *hdr,
> > ulong *rd_data, ulong *rd_len)
> > {
> > if (!hdr->ramdisk_size) {
> > @@ -186,7 +186,7 @@ int android_image_get_ramdisk(const struct andr_img_hdr
> > *hdr,
> > return 0;
> > }
> >
> > -int android_image_get_second(const struct andr_img_hdr *hdr,
> > +int android_image_get_second(const struct andr_boot_img_hdr_v0_v1_v2 *hdr,
> > ulong *second_data, ulong *second_len)
> > {
> > if (!hdr->second_size) {
> > @@ -226,7 +226,7 @@ int android_image_get_second(const struct andr_img_hdr
> > *hdr,
> > */
> > bool android_image_get_dtbo(ulong hdr_addr, ulong *addr, u32 *size)
> >
[PATCH] spi: cadence-qspi: Remove condition for calling enable linear mode
cadence_qspi_apb_enable_linear_mode() has a weak function defined, so no
need to gaurd this under if (CONFIG_IS_ENABLED(ARCH_VERSAL)).
In cadence_qspi_apb_write_execute(), enable linear mode is called twice by
mistake, remove extra one.
Signed-off-by: Ashok Reddy Soma
---
drivers/spi/cadence_qspi_apb.c | 6 +-
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/spi/cadence_qspi_apb.c b/drivers/spi/cadence_qspi_apb.c
index cfae5dcbda..d1f89138ef 100644
--- a/drivers/spi/cadence_qspi_apb.c
+++ b/drivers/spi/cadence_qspi_apb.c
@@ -735,8 +735,7 @@ int cadence_qspi_apb_read_execute(struct cadence_spi_priv
*priv,
void *buf = op->data.buf.in;
size_t len = op->data.nbytes;
- if (CONFIG_IS_ENABLED(ARCH_VERSAL))
- cadence_qspi_apb_enable_linear_mode(true);
+ cadence_qspi_apb_enable_linear_mode(true);
if (priv->use_dac_mode && (from + len < priv->ahbsize)) {
if (len < 256 ||
@@ -905,9 +904,6 @@ int cadence_qspi_apb_write_execute(struct cadence_spi_priv
*priv,
const void *buf = op->data.buf.out;
size_t len = op->data.nbytes;
- if (CONFIG_IS_ENABLED(ARCH_VERSAL))
- cadence_qspi_apb_enable_linear_mode(true);
-
/*
* Some flashes like the Cypress Semper flash expect a dummy 4-byte
* address (all 0s) with the read status register command in DTR mode.
--
2.17.1
Re: [RFC PATCH] board: ti: common: board_detect: Fix EEPROM read quirk for 2-byte
вт, 29 нояб. 2022 г. в 09:50, Neha Malcom Francis :
>
> EEPROM detection logic in ti_i2c_eeprom_get() involves figuring out
> whether addressing is 1-byte or 2-byte. There are currently different
> behaviours seen across boards as documented in commit bf6376642fe8
> ("board: ti: common: board_detect: Fix EEPROM read quirk"). Adding to
> the list, we see that there are 2-byte EEPROMs that read properly
> with 1-byte addressing with no offset.
>
> For ti_i2c_eeprom_am6_get where eeprom parse operation is dynamic, the
> earlier commit d2ab2a2bafd5 ("board: ti: common: board_detect: Fix
> EEPROM read quirk for AM6 style data") tried to resolve this by running
> ti_i2c_eeprom_get() twice. However this commit along with its former
> commit fails on J7 platforms where EEPROM successfully return back the
> header on 1-byte addressing and continues to do so until an offset is
> introduced. So the second read incorrectly determines the EEPROM as
> 1-byte addressing.
>
> A more generic solution is introduced here to solve
> this issue: 1-byte read without offset and 1-byte read with offset. If
> both passes, it follows 1-byte addressing else we proceed with 2-byte
> addressing check.
>
> Tested on J721E, J7200, DRA7xx, AM64x
I'll try to test this on the AM335x boards I have as soon as possible.
>
> Signed-off-by: Neha Malcom Francis
> Fixes: d2ab2a2bafd5 (board: ti: common: board_detect: Fix EEPROM read
> quirk for AM6 style data) and bf6376642fe8 (board: ti: common: board_detect:
> Fix EEPROM read quirk)
> ---
> board/ti/common/board_detect.c | 29 ++---
> 1 file changed, 18 insertions(+), 11 deletions(-)
>
> diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c
> index c37629fe8a..b9f2ebf2a0 100644
> --- a/board/ti/common/board_detect.c
> +++ b/board/ti/common/board_detect.c
> @@ -91,6 +91,8 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr,
> int dev_addr,
> #if CONFIG_IS_ENABLED(DM_I2C)
Should #else branch also be modified according to the new algo?
> struct udevice *dev;
> struct udevice *bus;
> + uint8_t offset_test;
> + bool one_byte_addressing = true;
>
> rc = uclass_get_device_by_seq(UCLASS_I2C, bus_addr, &bus);
> if (rc)
> @@ -114,8 +116,23 @@ static int __maybe_unused ti_i2c_eeprom_get(int
> bus_addr, int dev_addr,
> */
> (void)dm_i2c_read(dev, 0, ep, size);
>
> + if (*((u32 *)ep) != header)
> + one_byte_addressing = false;
> +
> + /*
> +* Handle case of bad 2 byte eeproms that responds to 1 byte
> addressing
> +* but gets stuck in const addressing when read requests are performed
> +* on offsets. We perform an offset test to make sure it is not a 2
> byte
> +* eeprom that works with 1 byte addressing but just without an offset
> +*/
> +
> + rc = dm_i2c_read(dev, 0x1, &offset_test, sizeof(offset_test));
> +
> + if (*((u32 *)ep) != (header & 0xFF))
> + one_byte_addressing = false;
> +
> /* Corrupted data??? */
> - if (*((u32 *)ep) != header) {
> + if (!one_byte_addressing) {
> /*
> * read the eeprom header using i2c again, but use only a
> * 2 byte address (some newer boards need this..)
> @@ -444,16 +461,6 @@ int __maybe_unused ti_i2c_eeprom_am6_get(int bus_addr,
> int dev_addr,
> if (rc)
> return rc;
>
> - /*
> -* Handle case of bad 2 byte eeproms that responds to 1 byte
> addressing
> -* but gets stuck in const addressing when read requests are performed
> -* on offsets. We re-read the board ID to ensure we have sane data
> back
> -*/
> - rc = ti_i2c_eeprom_get(bus_addr, dev_addr, TI_EEPROM_HEADER_MAGIC,
> - sizeof(board_id), (uint8_t *)&board_id);
> - if (rc)
> - return rc;
> -
> if (board_id.header.id != TI_AM6_EEPROM_RECORD_BOARD_ID) {
> pr_err("%s: Invalid board ID record!\n", __func__);
> return -EINVAL;
> --
> 2.34.1
>
--
With best regards,
Matwey V. Kornilov
[PATCH v4 0/5] add support for Theobroma Systems PX30-µQ7 (Ringneck) with Haikou devkit
From: Quentin Schulz The PX30-uQ7 (Ringneck) SoM is a µQseven-compatible (40mmx70mm, MXM-230 connector) system-on-module from Theobroma Systems[1], featuring the Rockchip PX30. It provides the following feature set: * up to 4GB DDR4 * up to 128GB on-module eMMC (with 8-bit 1.8V interface) * SD card (on a baseboard) via edge connector * Fast Ethernet with on-module TI DP83825I PHY * MIPI-DSI/LVDS * MIPI-CSI * USB - 1x USB 2.0 dual-role - 3x USB 2.0 host * on-module companion controller (STM32 Cortex-M0 or ATtiny), implementing: - low-power RTC functionality (ISL1208 emulation) - fan controller (AMC6821 emulation) - USB<->CAN bridge controller (STM32 only) * on-module Espressif ESP32 for Bluetooth + 2.4GHz WiFi * on-module NXP SE05x Secure Element [1] https://www.theobroma-systems.com/som-product/px30-uq7/ Cheers, Quentin To: Heiko Stuebner To: Simon Glass To: Philipp Tomsich To: Klaus Goger Cc: Quentin Schulz Cc: u-boot@lists.denx.de Cc: Kever Yang Signed-off-by: Quentin Schulz --- Changes in v4: - added CONFIG_ENV_OVERWRITE to defconfig to match Puma behavior and allow commands specified in our user manual, - added comment in commit log for the DTS origin (next-20221114), - Link to v3: https://lore.kernel.org/r/20221017-upstream-ringneck-v3-0-d6ae387f3...@theobroma-systems.com Changes in v3: - added Rb, - rebased on top of master, - fixed http links to use u instead of encoded µ, - updated u-boot,mmc-env-offset from 16KB to 20KB to avoid GPT corruption when saving env + reduced env size to 0x3000 to adapt to the offset change, - removed video support from defconfig as there's no display support ATM, - Link to v2: https://lore.kernel.org/r/20221017-upstream-ringneck-v2-0-0f03912eb...@theobroma-systems.com Changes in v2: - updated DTS from Linux with v2 of the Linux kernel patch series, - updated node nade for bios-disable-override-hog to match v2 of Linux kernel, - removed uapi input patch since it is not needed anymore, --- Quentin Schulz (5): rockchip: px30: fix CONFIG_IRAM_BASE rockchip: px30: list possible SPL boot devices rockchip: px30: insert u-boot,spl-boot-device into U-Boot device tree arm64: dts: rockchip: sync px30 DTSI with Linux kernel next-20221114 rockchip: add support for PX30 Ringneck SoM on Haikou Devkit arch/arm/dts/px30-ringneck-haikou-u-boot.dtsi | 91 + arch/arm/dts/px30-ringneck-haikou.dts | 232 + arch/arm/dts/px30-ringneck.dtsi| 382 + arch/arm/dts/px30.dtsi | 28 +- arch/arm/mach-rockchip/px30/Kconfig| 25 ++ arch/arm/mach-rockchip/px30/px30.c | 56 +++ board/theobroma-systems/ringneck_px30/Kconfig | 18 + board/theobroma-systems/ringneck_px30/MAINTAINERS | 9 + board/theobroma-systems/ringneck_px30/Makefile | 7 + board/theobroma-systems/ringneck_px30/README | 69 .../ringneck_px30/ringneck-px30.c | 175 ++ configs/ringneck-px30_defconfig| 128 +++ doc/board/rockchip/rockchip.rst| 1 + include/configs/px30_common.h | 3 +- include/configs/ringneck_px30.h| 15 + 15 files changed, 1234 insertions(+), 5 deletions(-) --- base-commit: 39b81955d38c11254b455322b9d98e07010049d6 change-id: 20221017-upstream-ringneck-57abd09a7aaa Best regards, -- Quentin Schulz
[PATCH v4 1/5] rockchip: px30: fix CONFIG_IRAM_BASE
From: Quentin Schulz
The IRAM on PX30 (or Int_MEM in datasheet) starts at 0xff0e and not
0xff02 as rightfully stated in the FIXME comment.
Let's fix it so that BROM_BOOTSOURCE_ID_ADDR points to the correct
address for PX30.
Fixes: 46281a76bee3 ("rockchip: add core px30 headers")
Cc: Quentin Schulz
Reviewed-by: Kever Yang
Signed-off-by: Quentin Schulz
---
include/configs/px30_common.h | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/include/configs/px30_common.h b/include/configs/px30_common.h
index 49d1878ebd..c93bb053a5 100644
--- a/include/configs/px30_common.h
+++ b/include/configs/px30_common.h
@@ -10,8 +10,7 @@
#define CONFIG_SYS_NS16550_MEM32
-/* FIXME: ff02 is pmu_mem (10k), while ff0e is regular int_mem */
-#define CONFIG_IRAM_BASE 0xff02
+#define CONFIG_IRAM_BASE 0xff0e
#define GICD_BASE 0xff131000
#define GICC_BASE 0xff132000
--
b4 0.10.1
[PATCH v4 2/5] rockchip: px30: list possible SPL boot devices
From: Quentin Schulz
BOOTROM sets a bit in a CPU register so that the software can know from
where the first stage bootloader was booted. One use case for this is to
specify the default loading medium for U-Boot proper to match the one
used by the BOOTROM to load the SPL (same-as-spl in
u-boot,spl-boot-order).
Let's create the mapping between BOOTROM value and Device Tree node
names for MMC devices.
Cc: Quentin Schulz
Reviewed-by: Kever Yang
Signed-off-by: Quentin Schulz
---
arch/arm/mach-rockchip/px30/px30.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/arch/arm/mach-rockchip/px30/px30.c
b/arch/arm/mach-rockchip/px30/px30.c
index 0641e6af0f..481b50235e 100644
--- a/arch/arm/mach-rockchip/px30/px30.c
+++ b/arch/arm/mach-rockchip/px30/px30.c
@@ -8,6 +8,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -15,6 +16,11 @@
#include
#include
+const char * const boot_devices[BROM_LAST_BOOTSOURCE + 1] = {
+ [BROM_BOOTSOURCE_EMMC] = "/mmc@ff39",
+ [BROM_BOOTSOURCE_SD] = "/mmc@ff37",
+};
+
static struct mm_region px30_mem_map[] = {
{
.virt = 0x0UL,
--
b4 0.10.1
[PATCH v4 3/5] rockchip: px30: insert u-boot, spl-boot-device into U-Boot device tree
From: Quentin Schulz
It is possible to boot U-Boot proper from a different storage medium
than the one used by the BOOTROM to load the SPL. This information is
stored in the u-boot,spl-boot-device Device Tree property and is
accessible from U-Boot proper so that it has knowledge at runtime where
it was loaded from.
Let's add support for this feature for px30.
Cc: Quentin Schulz
Signed-off-by: Quentin Schulz
---
arch/arm/mach-rockchip/px30/px30.c | 50 ++
1 file changed, 50 insertions(+)
diff --git a/arch/arm/mach-rockchip/px30/px30.c
b/arch/arm/mach-rockchip/px30/px30.c
index 481b50235e..5f26128d01 100644
--- a/arch/arm/mach-rockchip/px30/px30.c
+++ b/arch/arm/mach-rockchip/px30/px30.c
@@ -6,6 +6,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -427,3 +428,52 @@ void board_debug_uart_init(void)
#endif /* CONFIG_DEBUG_UART_BASE && CONFIG_DEBUG_UART_BASE == ... */
}
#endif /* CONFIG_DEBUG_UART_BOARD_INIT */
+
+#if defined(CONFIG_SPL_BUILD) && !defined(CONFIG_TPL_BUILD)
+const char *spl_decode_boot_device(u32 boot_device)
+{
+ int i;
+ static const struct {
+ u32 boot_device;
+ const char *ofpath;
+ } spl_boot_devices_tbl[] = {
+ { BOOT_DEVICE_MMC2, "/mmc@ff37" },
+ { BOOT_DEVICE_MMC1, "/mmc@ff39" },
+ };
+
+ for (i = 0; i < ARRAY_SIZE(spl_boot_devices_tbl); ++i)
+ if (spl_boot_devices_tbl[i].boot_device == boot_device)
+ return spl_boot_devices_tbl[i].ofpath;
+
+ return NULL;
+}
+
+void spl_perform_fixups(struct spl_image_info *spl_image)
+{
+ void *blob = spl_image->fdt_addr;
+ const char *boot_ofpath;
+ int chosen;
+
+ /*
+* Inject the ofpath of the device the full U-Boot (or Linux in
+* Falcon-mode) was booted from into the FDT, if a FDT has been
+* loaded at the same time.
+*/
+ if (!blob)
+ return;
+
+ boot_ofpath = spl_decode_boot_device(spl_image->boot_device);
+ if (!boot_ofpath) {
+ pr_err("%s: could not map boot_device to ofpath\n", __func__);
+ return;
+ }
+
+ chosen = fdt_find_or_add_subnode(blob, 0, "chosen");
+ if (chosen < 0) {
+ pr_err("%s: could not find/create '/chosen'\n", __func__);
+ return;
+ }
+ fdt_setprop_string(blob, chosen,
+ "u-boot,spl-boot-device", boot_ofpath);
+}
+#endif
--
b4 0.10.1
[PATCH v4 4/5] arm64: dts: rockchip: sync px30 DTSI with Linux kernel next-20221114
From: Quentin Schulz
Sync the px30 dtsi from Linux kernel next-20221114.
Cc: Quentin Schulz
Reviewed-by: Kever Yang
Signed-off-by: Quentin Schulz
---
arch/arm/dts/px30.dtsi | 28 +---
1 file changed, 25 insertions(+), 3 deletions(-)
diff --git a/arch/arm/dts/px30.dtsi b/arch/arm/dts/px30.dtsi
index 00f50b05d5..bfa3580429 100644
--- a/arch/arm/dts/px30.dtsi
+++ b/arch/arm/dts/px30.dtsi
@@ -365,6 +365,28 @@
status = "disabled";
};
+ i2s0_8ch: i2s@ff06 {
+ compatible = "rockchip,px30-i2s-tdm";
+ reg = <0x0 0xff06 0x0 0x1000>;
+ interrupts = ;
+ clocks = <&cru SCLK_I2S0_TX>, <&cru SCLK_I2S0_RX>, <&cru
HCLK_I2S0>;
+ clock-names = "mclk_tx", "mclk_rx", "hclk";
+ dmas = <&dmac 16>, <&dmac 17>;
+ dma-names = "tx", "rx";
+ rockchip,grf = <&grf>;
+ resets = <&cru SRST_I2S0_TX>, <&cru SRST_I2S0_RX>;
+ reset-names = "tx-m", "rx-m";
+ pinctrl-names = "default";
+ pinctrl-0 = <&i2s0_8ch_sclktx &i2s0_8ch_sclkrx
+&i2s0_8ch_lrcktx &i2s0_8ch_lrckrx
+&i2s0_8ch_sdo0 &i2s0_8ch_sdi0
+&i2s0_8ch_sdo1 &i2s0_8ch_sdi1
+&i2s0_8ch_sdo2 &i2s0_8ch_sdi2
+&i2s0_8ch_sdo3 &i2s0_8ch_sdi3>;
+ #sound-dai-cells = <0>;
+ status = "disabled";
+ };
+
i2s1_2ch: i2s@ff07 {
compatible = "rockchip,px30-i2s", "rockchip,rk3066-i2s";
reg = <0x0 0xff07 0x0 0x1000>;
@@ -528,7 +550,7 @@
i2c0: i2c@ff18 {
compatible = "rockchip,px30-i2c", "rockchip,rk3399-i2c";
reg = <0x0 0xff18 0x0 0x1000>;
- clocks = <&cru SCLK_I2C0>, <&cru PCLK_I2C0>;
+ clocks = <&cru SCLK_I2C0>, <&cru PCLK_I2C0>;
clock-names = "i2c", "pclk";
interrupts = ;
pinctrl-names = "default";
@@ -711,7 +733,7 @@
clock-names = "pclk", "timer";
};
- dmac: dmac@ff24 {
+ dmac: dma-controller@ff24 {
compatible = "arm,pl330", "arm,primecell";
reg = <0x0 0xff24 0x0 0x4000>;
interrupts = ,
@@ -1072,7 +1094,7 @@
};
dsi: dsi@ff45 {
- compatible = "rockchip,px30-mipi-dsi";
+ compatible = "rockchip,px30-mipi-dsi", "snps,dw-mipi-dsi";
reg = <0x0 0xff45 0x0 0x1>;
interrupts = ;
clocks = <&cru PCLK_MIPI_DSI>;
--
b4 0.10.1
[PATCH v4 5/5] rockchip: add support for PX30 Ringneck SoM on Haikou Devkit
From: Quentin Schulz
The PX30-µQ7 (Ringneck) is a system-on-module featuring the Rockchip
PX30 in a micro Qseven-compatible form-factor.
PX30-µQ7 features:
* CPU: quad-core Cortex-A35
* DRAM: 2GB dual-channel
* eMMC: onboard eMMC
* SD/MMC
* TI DP83825I 10/100Mbps PHY
* USB:
* USB2.0 dual role port
* 3x USB2.0 host via onboard USB2.0 hub
* Display: MIPI-DSI
* Camera: MIPI-CSI
* onboard 2.4GHz WiFi + Bluetooth module
* Companion Controller: on-board additional microcontroller
(STM32 Cortex-M0 or ATtiny):
* RTC
* fan controller
* CAN (only STM32)
The non-U-Boot DTS files are imported from Linux next-20221114.
Cc: Quentin Schulz
Signed-off-by: Quentin Schulz
---
arch/arm/dts/px30-ringneck-haikou-u-boot.dtsi | 91 +
arch/arm/dts/px30-ringneck-haikou.dts | 232 +
arch/arm/dts/px30-ringneck.dtsi| 382 +
arch/arm/mach-rockchip/px30/Kconfig| 25 ++
board/theobroma-systems/ringneck_px30/Kconfig | 18 +
board/theobroma-systems/ringneck_px30/MAINTAINERS | 9 +
board/theobroma-systems/ringneck_px30/Makefile | 7 +
board/theobroma-systems/ringneck_px30/README | 69
.../ringneck_px30/ringneck-px30.c | 175 ++
configs/ringneck-px30_defconfig| 128 +++
doc/board/rockchip/rockchip.rst| 1 +
include/configs/ringneck_px30.h| 15 +
12 files changed, 1152 insertions(+)
diff --git a/arch/arm/dts/px30-ringneck-haikou-u-boot.dtsi
b/arch/arm/dts/px30-ringneck-haikou-u-boot.dtsi
new file mode 100644
index 00..e8a34c7c1e
--- /dev/null
+++ b/arch/arm/dts/px30-ringneck-haikou-u-boot.dtsi
@@ -0,0 +1,91 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include "px30-u-boot.dtsi"
+
+/ {
+ config {
+ u-boot,mmc-env-offset = <0x5000>; /* @ 20KB */
+ u-boot,efi-partition-entries-offset = <0x20>; /* 2MB */
+ u-boot,boot-led = "module_led";
+ };
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ u-boot,spl-boot-order = "same-as-spl", &emmc, &sdmmc;
+ };
+};
+
+&binman {
+ simple-bin {
+ blob {
+ offset = <((CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR -
64) * 512)>;
+ };
+ };
+};
+
+&emmc_clk {
+ u-boot,dm-pre-reloc;
+};
+
+&emmc_cmd {
+ u-boot,dm-pre-reloc;
+};
+
+&emmc_bus8 {
+ u-boot,dm-pre-reloc;
+};
+
+&gpio0 {
+ u-boot,dm-pre-reloc;
+};
+
+&gpio1 {
+ u-boot,dm-pre-reloc;
+};
+
+&gpio2 {
+ u-boot,dm-pre-reloc;
+
+ /*
+* The Qseven BIOS_DISABLE signal on the PX30-µQ7 keeps the on-module
+* eMMC powered-down initially (in fact it keeps the reset signal
+* asserted). BIOS_DISABLE_OVERRIDE pin allows to re-enable eMMC after
+* the SPL has been booted from SD Card.
+*/
+ bios-disable-override-hog {
+ u-boot,dm-pre-reloc;
+ };
+};
+
+&pinctrl {
+ u-boot,dm-pre-reloc;
+};
+
+&pcfg_pull_none_8ma {
+ u-boot,dm-pre-reloc;
+};
+
+&pcfg_pull_up_8ma {
+ u-boot,dm-pre-reloc;
+};
+
+&sdmmc_bus4 {
+ u-boot,dm-pre-reloc;
+};
+
+&sdmmc_clk {
+ u-boot,dm-pre-reloc;
+};
+
+&sdmmc_cmd {
+ u-boot,dm-pre-reloc;
+};
+
+&sdmmc_det {
+ u-boot,dm-pre-reloc;
+};
+
+&uart0 {
+ clock-frequency = <2400>;
+ u-boot,dm-pre-reloc;
+};
diff --git a/arch/arm/dts/px30-ringneck-haikou.dts
b/arch/arm/dts/px30-ringneck-haikou.dts
new file mode 100644
index 00..08a3ad3e7a
--- /dev/null
+++ b/arch/arm/dts/px30-ringneck-haikou.dts
@@ -0,0 +1,232 @@
+// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
+/*
+ * Copyright (c) 2022 Theobroma Systems Design und Consulting GmbH
+ */
+
+/dts-v1/;
+#include "px30-ringneck.dtsi"
+#include
+#include
+
+/ {
+ model = "Theobroma Systems PX30-uQ7 SoM on Haikou devkit";
+ compatible = "tsd,px30-ringneck-haikou", "rockchip,px30";
+
+ aliases {
+ mmc2 = &sdmmc;
+ };
+
+ chosen {
+ stdout-path = "serial0:115200n8";
+ };
+
+ gpio-keys {
+ compatible = "gpio-keys";
+ pinctrl-0 = <&haikou_keys_pin>;
+ pinctrl-names = "default";
+
+ button-batlow-n {
+ label = "BATLOW#";
+ linux,code = ;
+ gpios = <&gpio3 RK_PA7 GPIO_ACTIVE_LOW>;
+ };
+
+ button-slp-btn-n {
+ label = "SLP_BTN#";
+ linux,code = ;
+ gpios = <&gpio1 RK_PB7 GPIO_ACTIVE_LOW>;
+ };
+
+ button-wake-n {
+ label = "WAKE#";
+ linux,
[PATCH] ARM: zynq: Add missing twd timer for mini configurations
The commit b7e0750d8872 ("zynq: Convert arm twd timer to DM driver")
switched timer to DM but missing to add nodes to all mini configurations.
Based on it missing timer end up in non functional system where any delay
doesn't work.
Signed-off-by: Michal Simek
---
arch/arm/dts/zynq-cse-nand.dts | 7 +++
arch/arm/dts/zynq-cse-nor.dts | 7 +++
arch/arm/dts/zynq-cse-qspi.dtsi | 7 +++
3 files changed, 21 insertions(+)
diff --git a/arch/arm/dts/zynq-cse-nand.dts b/arch/arm/dts/zynq-cse-nand.dts
index 32cb3bffcb94..27adfb921622 100644
--- a/arch/arm/dts/zynq-cse-nand.dts
+++ b/arch/arm/dts/zynq-cse-nand.dts
@@ -86,6 +86,13 @@
reg = <0x100 0x100>;
};
};
+
+ scutimer: timer@f8f00600 {
+ u-boot,dm-pre-reloc;
+ compatible = "arm,cortex-a9-twd-timer";
+ reg = <0xf8f00600 0x20>;
+ clock-frequency = <3>;
+ };
};
};
diff --git a/arch/arm/dts/zynq-cse-nor.dts b/arch/arm/dts/zynq-cse-nor.dts
index 197fbd717aae..f22a149f7924 100644
--- a/arch/arm/dts/zynq-cse-nor.dts
+++ b/arch/arm/dts/zynq-cse-nor.dts
@@ -85,6 +85,13 @@
#address-cells = <1>;
#size-cells = <1>;
};
+
+ scutimer: timer@f8f00600 {
+ u-boot,dm-pre-reloc;
+ compatible = "arm,cortex-a9-twd-timer";
+ reg = <0xf8f00600 0x20>;
+ clock-frequency = <3>;
+ };
};
};
diff --git a/arch/arm/dts/zynq-cse-qspi.dtsi b/arch/arm/dts/zynq-cse-qspi.dtsi
index 38410eeca886..f7ac92b8026d 100644
--- a/arch/arm/dts/zynq-cse-qspi.dtsi
+++ b/arch/arm/dts/zynq-cse-qspi.dtsi
@@ -116,6 +116,13 @@
reg = <0x100 0x100>;
};
};
+
+ scutimer: timer@f8f00600 {
+ u-boot,dm-pre-reloc;
+ compatible = "arm,cortex-a9-twd-timer";
+ reg = <0xf8f00600 0x20>;
+ clock-frequency = <3>;
+ };
};
};
--
2.36.1
Re: [u-boot][PATCH 06/14] mtd: rawnand: nand_base: Allow base driver to be used in SPL without nand_bbt
Hi Michael,
On 28/11/2022 16:27, Michael Nazzareno Trimarchi wrote:
> Hi
>
> On Tue, Oct 11, 2022 at 1:50 PM Roger Quadros wrote:
>>
>> nand_bbt.c is not being built with the nand_base driver during SPL
>> build. This results in build failures if we try to access any nand_bbt
>> related functions.
>>
>> Don't use any nand_bbt functions for SPL build.
>>
>> Signed-off-by: Roger Quadros
>> ---
>> drivers/mtd/nand/raw/nand_base.c | 18 +-
>> 1 file changed, 17 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/nand/raw/nand_base.c
>> b/drivers/mtd/nand/raw/nand_base.c
>> index 4b09a11288..826ae633ce 100644
>> --- a/drivers/mtd/nand/raw/nand_base.c
>> +++ b/drivers/mtd/nand/raw/nand_base.c
>> @@ -447,7 +447,10 @@ static int nand_default_block_markbad(struct mtd_info
>> *mtd, loff_t ofs)
>> static int nand_block_markbad_lowlevel(struct mtd_info *mtd, loff_t ofs)
>> {
>> struct nand_chip *chip = mtd_to_nand(mtd);
>> - int res, ret = 0;
>> + int ret = 0;
>> +#ifndef CONFIG_SPL_BUILD
>> + int res;
>> +#endif
>>
>> if (!(chip->bbt_options & NAND_BBT_NO_OOB_BBM)) {
>> struct erase_info einfo;
>> @@ -465,12 +468,14 @@ static int nand_block_markbad_lowlevel(struct mtd_info
>> *mtd, loff_t ofs)
>> nand_release_device(mtd);
>> }
>>
>> +#ifndef CONFIG_SPL_BUILD
>> /* Mark block bad in BBT */
>> if (chip->bbt) {
>> res = nand_markbad_bbt(mtd, ofs);
>> if (!ret)
>> ret = res;
>> }
>> +#endif
>>
>> if (!ret)
>> mtd->ecc_stats.badblocks++;
>> @@ -517,7 +522,11 @@ static int nand_block_isreserved(struct mtd_info *mtd,
>> loff_t ofs)
>> if (!chip->bbt)
>> return 0;
>> /* Return info from the table */
>> +#ifndef CONFIG_SPL_BUILD
>> return nand_isreserved_bbt(mtd, ofs);
>> +#else
>> + return 0;
>> +#endif
>> }
>>
>> /**
>> @@ -543,7 +552,11 @@ static int nand_block_checkbad(struct mtd_info *mtd,
>> loff_t ofs, int allowbbt)
>> return chip->block_bad(mtd, ofs);
>>
>> /* Return info from the table */
>> +#ifndef CONFIG_SPL_BUILD
>> return nand_isbad_bbt(mtd, ofs, allowbbt);
>> +#else
>> + return 0;
>> +#endif
>> }
>
> Can you please send me the config that let this fail?
I've pushed a test branch here where relevant changes are done to
am64x_evm_a53_defconfig
https://github.com/rogerq/u-boot/commits/for-v2023.01/am64-nand-base-1.0-test
Attaching the resulting spl/u-boot.cfg that causes the failure
>
> Michael
>>
>> /**
>> @@ -3752,8 +3765,11 @@ static void nand_set_defaults(struct nand_chip *chip,
>> int busw)
>> chip->write_byte = busw ? nand_write_byte16 :
>> nand_write_byte;
>> if (!chip->read_buf || chip->read_buf == nand_read_buf)
>> chip->read_buf = busw ? nand_read_buf16 : nand_read_buf;
>> +
>> +#ifndef CONFIG_SPL_BUILD
>> if (!chip->scan_bbt)
>> chip->scan_bbt = nand_default_bbt;
>> +#endif
>>
>> if (!chip->controller) {
>> chip->controller = &chip->hwcontrol;
>> --
>> 2.17.1
>>
>
>
--
cheers,
-roger#define CONFIG_CMD_MTD 1
#define CONFIG_ETH 1
#define CONFIG_SYS_MMCSD_FS_BOOT_PARTITION 1
#define CONFIG_SPL_FIT_SOURCE ""
#define CONFIG_SYS_SPI_U_BOOT_OFFS 0x28
#define CONFIG_CMD_FAT 1
#define CONFIG_TI_K3_PSIL 1
#define CONFIG_SPL_DM_SERIAL 1
#define CONFIG_TOOLS_SHA1 1
#define CONFIG_EFI_DEVICE_PATH_UTIL 1
#define CONFIG_BOOTM_NETBSD 1
#define CONFIG_OF_SPL_REMOVE_PROPS "interrupt-parent interrupts"
#define CONFIG_CMD_FDT 1
#define CONFIG_SPL_DFU 1
#define CONFIG_OMAP_SERIAL 1
#define CONFIG_NAND_OMAP_GPMC 1
#define CONFIG_USB_GADGET_DOWNLOAD 1
#define CONFIG_SYS_CLK_FREQ 0
#define CONFIG_BOOTMETH_VBE_SIMPLE 1
#define CONFIG_CMD_ITEST 1
#define CONFIG_SPL_POWER_DOMAIN 1
#define CONFIG_REMOTEPROC_TI_K3_ARM64 1
#define CONFIG_BOOTM_VXWORKS 1
#define CONFIG_ERR_PTR_OFFSET 0x0
#define CONFIG_CMD_EDITENV 1
#define CONFIG_OF_OVERLAY_LIST ""
#define CONFIG_SPL_SPRINTF 1
#define CONFIG_SPL_DMA 1
#define CONFIG_CMD_MTDPARTS 1
#define CONFIG_EFI_PLATFORM_LANG_CODES "en-US"
#define CONFIG_SPL_NAND_SUPPORT 1
#define CONFIG_ARM_PSCI_FW 1
#define CONFIG_CMD_SETEXPR 1
#define CONFIG_TOOLS_SHA384 1
#define CONFIG_SYS_MAX_NAND_DEVICE 1
#define CONFIG_TOOLS_OF_LIBFDT 1
#define CONFIG_CMD_BOOTP 1
#define CONFIG_SPL_MULTI_DTB_FIT_NO_COMPRESSION 1
#define CONFIG_SYS_MEM_TOP_HIDE 0x0
#define CONFIG_CMD_PART 1
#define CONFIG_MISC 1
#define CONFIG_DFU_OVER_USB 1
#define CONFIG_ENV_SUPPORT 1
#define CONFIG_SPL_LOGLEVEL 4
#define CONFIG_CMD_ENV_EXISTS 1
#define CONFIG_SF_DEFAULT_MODE 0x0
#define CONFIG_SYS_LONGHELP 1
#define CONFIG_SYS_NAND_5_ADDR_CYCLE
#define CONFIG_GCC_VERSION 90201
#define CONFIG_DM_MAILBOX 1
#define CONFIG_SYS_LOAD_ADDR 0x8200
#define CONFIG_HASH 1
#define CONFIG_DISPLAY_BOARDINFO 1
#define CONFIG_CMD_XIMG 1
#define CONF
Re: [RFC PATCH] board: ti: common: board_detect: Fix EEPROM read quirk for 2-byte
On Tue, Nov 29, 2022 at 03:10:12PM +0300, Matwey V. Kornilov wrote:
> вт, 29 нояб. 2022 г. в 09:50, Neha Malcom Francis :
> >
> > EEPROM detection logic in ti_i2c_eeprom_get() involves figuring out
> > whether addressing is 1-byte or 2-byte. There are currently different
> > behaviours seen across boards as documented in commit bf6376642fe8
> > ("board: ti: common: board_detect: Fix EEPROM read quirk"). Adding to
> > the list, we see that there are 2-byte EEPROMs that read properly
> > with 1-byte addressing with no offset.
> >
> > For ti_i2c_eeprom_am6_get where eeprom parse operation is dynamic, the
> > earlier commit d2ab2a2bafd5 ("board: ti: common: board_detect: Fix
> > EEPROM read quirk for AM6 style data") tried to resolve this by running
> > ti_i2c_eeprom_get() twice. However this commit along with its former
> > commit fails on J7 platforms where EEPROM successfully return back the
> > header on 1-byte addressing and continues to do so until an offset is
> > introduced. So the second read incorrectly determines the EEPROM as
> > 1-byte addressing.
> >
> > A more generic solution is introduced here to solve
> > this issue: 1-byte read without offset and 1-byte read with offset. If
> > both passes, it follows 1-byte addressing else we proceed with 2-byte
> > addressing check.
> >
> > Tested on J721E, J7200, DRA7xx, AM64x
>
> I'll try to test this on the AM335x boards I have as soon as possible.
I wonder if we should re-factor this code a bit and not have a single
ti_i2c_eeprom_get but instead build for whichever sets of quirks a given
family of boards has with their EEPROMs. I really worry that we're going
to find that this change here breaks yet another different EEPROM than
before.
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v4 00/17] IPv6 support
On Tue, Nov 29, 2022 at 08:39:36AM +, Vyacheslav Mitrofanov V wrote: > Tom, maybe it is better to change configs add ifdefs or do sth else to > exclude them from the build if IPV6 is not configured? There's two parts to this, yes. Sandbox needs to enable ipv6 so that the tests are run, and the tests need to be be appropriately ifdef'd so that they don't try and be run on platforms without ipv6. -- Tom signature.asc Description: PGP signature
Re: [PATCH v4 00/17] IPv6 support
I'll fix it soon and send new patchset! Thanks! От: Tom Rini Отправлено: 29 ноября 2022 г. 16:49:48 Кому: Vyacheslav Mitrofanov V Копия: rfried@gmail.com; joe.hershber...@ni.com; w...@denx.de; u-boot@lists.denx.de; judge.pack...@gmail.com; li...@yadro.com; s...@chromium.org Тема: Re: [PATCH v4 00/17] IPv6 support On Tue, Nov 29, 2022 at 08:39:36AM +, Vyacheslav Mitrofanov V wrote: > Tom, maybe it is better to change configs add ifdefs or do sth else to > exclude them from the build if IPV6 is not configured? There's two parts to this, yes. Sandbox needs to enable ipv6 so that the tests are run, and the tests need to be be appropriately ifdef'd so that they don't try and be run on platforms without ipv6. -- Tom
[PATCH 1/1] efi_loader: don't use EFI_LOADER_DATA internally
Memory allocated by U-Boot for internal usage should be
EFI_BOOT_SERVICES_DATA or _CODE or EFI_RUNTIME_SERVICES_DATA or _CODE.
Reported-by: François-Frédéric Ozog
Signed-off-by: Heinrich Schuchardt
---
cmd/efidebug.c | 2 +-
lib/efi_loader/efi_memory.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index ef239bb34b..64104da130 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -600,7 +600,7 @@ static int do_efi_show_memmap(struct cmd_tbl *cmdtp, int
flag,
ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL);
if (ret == EFI_BUFFER_TOO_SMALL) {
map_size += sizeof(struct efi_mem_desc); /* for my own */
- ret = efi_allocate_pool(EFI_LOADER_DATA, map_size,
+ ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, map_size,
(void *)&memmap);
if (ret != EFI_SUCCESS)
return CMD_RET_FAILURE;
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index a17b426d11..0c336f98d2 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -823,7 +823,7 @@ static void add_u_boot_and_runtime(void)
uboot_stack_size) & ~EFI_PAGE_MASK;
uboot_pages = ((uintptr_t)map_sysmem(gd->ram_top - 1, 0) -
uboot_start + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT;
- efi_add_memory_map_pg(uboot_start, uboot_pages, EFI_LOADER_DATA,
+ efi_add_memory_map_pg(uboot_start, uboot_pages, EFI_BOOT_SERVICES_DATA,
false);
#if defined(__aarch64__)
@@ -857,7 +857,7 @@ int efi_memory_init(void)
/* Request a 32bit 64MB bounce buffer region */
uint64_t efi_bounce_buffer_addr = 0x;
- if (efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS, EFI_LOADER_DATA,
+ if (efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS, EFI_BOOT_SERVICES_DATA,
(64 * 1024 * 1024) >> EFI_PAGE_SHIFT,
&efi_bounce_buffer_addr) != EFI_SUCCESS)
return -1;
--
2.37.2
[PATCH] arm64: zynqmp: Do not enable IPI by default
ZynqMP mini configurations are not using IPI driver and enabling this is adding additional ~1200 Bytes (depends on configuration). This ends up in situation that there is no enough space in OCM for relocation that's why disable this driver for all mini configurations. Signed-off-by: Michal Simek --- arch/arm/Kconfig | 4 ++-- configs/xilinx_zynqmp_mini_defconfig | 1 + configs/xilinx_zynqmp_mini_emmc0_defconfig | 1 + configs/xilinx_zynqmp_mini_emmc1_defconfig | 1 + configs/xilinx_zynqmp_mini_nand_defconfig| 1 + configs/xilinx_zynqmp_mini_nand_single_defconfig | 1 + configs/xilinx_zynqmp_mini_qspi_defconfig| 1 + 7 files changed, 8 insertions(+), 2 deletions(-) diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index f95ed71b2466..3f68d0988b7f 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1302,7 +1302,7 @@ config ARCH_ZYNQMP select DM select DEBUG_UART_BOARD_INIT if SPL && DEBUG_UART select DM_ETH if NET - select DM_MAILBOX + imply DM_MAILBOX select DM_MMC if MMC select DM_SERIAL select DM_SPI if SPI @@ -1319,7 +1319,7 @@ config ARCH_ZYNQMP imply SPL_FIRMWARE if SPL select SPL_SEPARATE_BSS if SPL select SUPPORT_SPL - select ZYNQMP_IPI + imply ZYNQMP_IPI if DM_MAILBOX select SOC_DEVICE imply BOARD_LATE_INIT imply CMD_DM diff --git a/configs/xilinx_zynqmp_mini_defconfig b/configs/xilinx_zynqmp_mini_defconfig index 245b6a42b935..f29128bf55d9 100644 --- a/configs/xilinx_zynqmp_mini_defconfig +++ b/configs/xilinx_zynqmp_mini_defconfig @@ -59,6 +59,7 @@ CONFIG_SYS_RELOC_GD_ENV_ADDR=y # CONFIG_NET is not set # CONFIG_DM_WARN is not set # CONFIG_DM_DEVICE_REMOVE is not set +# CONFIG_DM_MAILBOX is not set # CONFIG_MMC is not set CONFIG_ARM_DCC=y CONFIG_PANIC_HANG=y diff --git a/configs/xilinx_zynqmp_mini_emmc0_defconfig b/configs/xilinx_zynqmp_mini_emmc0_defconfig index adf1dae66ede..611da78239db 100644 --- a/configs/xilinx_zynqmp_mini_emmc0_defconfig +++ b/configs/xilinx_zynqmp_mini_emmc0_defconfig @@ -71,6 +71,7 @@ CONFIG_SYS_RELOC_GD_ENV_ADDR=y # CONFIG_DM_WARN is not set # CONFIG_DM_DEVICE_REMOVE is not set CONFIG_SPL_DM_SEQ_ALIAS=y +# CONFIG_DM_MAILBOX is not set CONFIG_SUPPORT_EMMC_BOOT=y CONFIG_MMC_SDHCI=y CONFIG_MMC_SDHCI_ZYNQ=y diff --git a/configs/xilinx_zynqmp_mini_emmc1_defconfig b/configs/xilinx_zynqmp_mini_emmc1_defconfig index 9d799ad0e3d5..c8084e6ae050 100644 --- a/configs/xilinx_zynqmp_mini_emmc1_defconfig +++ b/configs/xilinx_zynqmp_mini_emmc1_defconfig @@ -71,6 +71,7 @@ CONFIG_SYS_RELOC_GD_ENV_ADDR=y # CONFIG_DM_WARN is not set # CONFIG_DM_DEVICE_REMOVE is not set CONFIG_SPL_DM_SEQ_ALIAS=y +# CONFIG_DM_MAILBOX is not set CONFIG_SUPPORT_EMMC_BOOT=y CONFIG_MMC_SDHCI=y CONFIG_MMC_SDHCI_ZYNQ=y diff --git a/configs/xilinx_zynqmp_mini_nand_defconfig b/configs/xilinx_zynqmp_mini_nand_defconfig index 29040a39e970..f5a467940266 100644 --- a/configs/xilinx_zynqmp_mini_nand_defconfig +++ b/configs/xilinx_zynqmp_mini_nand_defconfig @@ -55,6 +55,7 @@ CONFIG_SYS_RELOC_GD_ENV_ADDR=y # CONFIG_NET is not set # CONFIG_DM_WARN is not set # CONFIG_DM_DEVICE_REMOVE is not set +# CONFIG_DM_MAILBOX is not set # CONFIG_MMC is not set CONFIG_MTD=y CONFIG_DM_MTD=y diff --git a/configs/xilinx_zynqmp_mini_nand_single_defconfig b/configs/xilinx_zynqmp_mini_nand_single_defconfig index 7c17c061d424..61c44e2a6a44 100644 --- a/configs/xilinx_zynqmp_mini_nand_single_defconfig +++ b/configs/xilinx_zynqmp_mini_nand_single_defconfig @@ -55,6 +55,7 @@ CONFIG_SYS_RELOC_GD_ENV_ADDR=y # CONFIG_NET is not set # CONFIG_DM_WARN is not set # CONFIG_DM_DEVICE_REMOVE is not set +# CONFIG_DM_MAILBOX is not set # CONFIG_MMC is not set CONFIG_MTD=y CONFIG_DM_MTD=y diff --git a/configs/xilinx_zynqmp_mini_qspi_defconfig b/configs/xilinx_zynqmp_mini_qspi_defconfig index 513b51998d25..a4b754e09ebb 100644 --- a/configs/xilinx_zynqmp_mini_qspi_defconfig +++ b/configs/xilinx_zynqmp_mini_qspi_defconfig @@ -76,6 +76,7 @@ CONFIG_SPL_DM_SEQ_ALIAS=y # CONFIG_GPIO is not set # CONFIG_I2C is not set # CONFIG_INPUT is not set +# CONFIG_DM_MAILBOX is not set # CONFIG_MMC is not set # CONFIG_SPI_FLASH_SMART_HWCAPS is not set # CONFIG_SPI_FLASH_UNLOCK_ALL is not set -- 2.36.1
Re: [u-boot][PATCH 05/14] mtd: rawnand: omap_gpmc: Fix BCH6/16 HW based correction
Hi Roger,
On Tue, Oct 11, 2022 at 1:50 PM Roger Quadros wrote:
>
> The BCH detection hardware can generate ECC bytes for multiple
> sectors in one go. Use that feature.
>
> correct() only corrects one sector at a time so we need to call it
> repeatedly for each sector.
>
> Signed-off-by: Roger Quadros
> ---
> drivers/mtd/nand/raw/omap_gpmc.c | 325 +--
> 1 file changed, 223 insertions(+), 102 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/omap_gpmc.c
> b/drivers/mtd/nand/raw/omap_gpmc.c
> index b36fe762b3..b5ad66ad49 100644
> --- a/drivers/mtd/nand/raw/omap_gpmc.c
> +++ b/drivers/mtd/nand/raw/omap_gpmc.c
> @@ -27,6 +27,9 @@
>
> #define BADBLOCK_MARKER_LENGTH 2
> #define SECTOR_BYTES 512
> +#define ECCSIZE0_SHIFT 12
> +#define ECCSIZE1_SHIFT 22
> +#define ECC1RESULTSIZE 0x1
> #define ECCCLEAR (0x1 << 8)
> #define ECCRESULTREG1 (0x1 << 0)
> /* 4 bit padding to make byte aligned, 56 = 52 + 4 */
> @@ -187,72 +190,35 @@ static int __maybe_unused omap_correct_data(struct
> mtd_info *mtd, uint8_t *dat,
> __maybe_unused
> static void omap_enable_hwecc(struct mtd_info *mtd, int32_t mode)
> {
> - struct nand_chip*nand = mtd_to_nand(mtd);
> - struct omap_nand_info *info = nand_get_controller_data(nand);
> + struct nand_chip *nand = mtd_to_nand(mtd);
> + struct omap_nand_info *info = nand_get_controller_data(nand);
> unsigned int dev_width = (nand->options & NAND_BUSWIDTH_16) ? 1 : 0;
> - unsigned int ecc_algo = 0;
> - unsigned int bch_type = 0;
> - unsigned int eccsize1 = 0x00, eccsize0 = 0x00, bch_wrapmode = 0x00;
> - u32 ecc_size_config_val = 0;
> - u32 ecc_config_val = 0;
> - int cs = info->cs;
> + u32 val;
>
> - /* configure GPMC for specific ecc-scheme */
> - switch (info->ecc_scheme) {
> - case OMAP_ECC_HAM1_CODE_SW:
> - return;
> - case OMAP_ECC_HAM1_CODE_HW:
> - ecc_algo = 0x0;
> - bch_type = 0x0;
> - bch_wrapmode = 0x00;
> - eccsize0 = 0xFF;
> - eccsize1 = 0xFF;
> + /* Clear ecc and enable bits */
> + writel(ECCCLEAR | ECCRESULTREG1, &gpmc_cfg->ecc_control);
> +
> + /* program ecc and result sizes */
> + val = nand->ecc.size >> 1) - 1) << ECCSIZE1_SHIFT) |
> + ECC1RESULTSIZE);
> + writel(val, &gpmc_cfg->ecc_size_config);
> +
> + switch (mode) {
> + case NAND_ECC_READ:
> + case NAND_ECC_WRITE:
> + writel(ECCCLEAR | ECCRESULTREG1, &gpmc_cfg->ecc_control);
> break;
> - case OMAP_ECC_BCH8_CODE_HW_DETECTION_SW:
> - case OMAP_ECC_BCH8_CODE_HW:
> - ecc_algo = 0x1;
> - bch_type = 0x1;
> - if (mode == NAND_ECC_WRITE) {
> - bch_wrapmode = 0x01;
> - eccsize0 = 0; /* extra bits in nibbles per sector */
> - eccsize1 = 28; /* OOB bits in nibbles per sector */
> - } else {
> - bch_wrapmode = 0x01;
> - eccsize0 = 26; /* ECC bits in nibbles per sector */
> - eccsize1 = 2; /* non-ECC bits in nibbles per sector
> */
> - }
> - break;
> - case OMAP_ECC_BCH16_CODE_HW:
> - ecc_algo = 0x1;
> - bch_type = 0x2;
> - if (mode == NAND_ECC_WRITE) {
> - bch_wrapmode = 0x01;
> - eccsize0 = 0; /* extra bits in nibbles per sector */
> - eccsize1 = 52; /* OOB bits in nibbles per sector */
> - } else {
> - bch_wrapmode = 0x01;
> - eccsize0 = 52; /* ECC bits in nibbles per sector */
> - eccsize1 = 0; /* non-ECC bits in nibbles per sector
> */
> - }
> + case NAND_ECC_READSYN:
> + writel(ECCCLEAR, &gpmc_cfg->ecc_control);
> break;
> default:
> - return;
> + printf("%s: error: unrecognized Mode[%d]!\n", __func__, mode);
> + break;
> }
> - /* Clear ecc and enable bits */
> - writel(ECCCLEAR | ECCRESULTREG1, &gpmc_cfg->ecc_control);
> - /* Configure ecc size for BCH */
> - ecc_size_config_val = (eccsize1 << 22) | (eccsize0 << 12);
> - writel(ecc_size_config_val, &gpmc_cfg->ecc_size_config);
> -
> - /* Configure device details for BCH engine */
> - ecc_config_val = ((ecc_algo << 16) | /* HAM1 | BCHx */
> - (bch_type << 12)| /* BCH4/BCH8/BCH16 */
> - (bch_wrapmode << 8) | /* wrap mode */
> - (dev_width << 7)| /* bus width */
> - (0x0 << 4) | /* number of sectors */
Re: [PATCH 1/1] efi_loader: don't use EFI_LOADER_DATA internally
Hi Heinrich, On Tue, 29 Nov 2022 at 17:04, Heinrich Schuchardt wrote: > > Memory allocated by U-Boot for internal usage should be > EFI_BOOT_SERVICES_DATA or _CODE or EFI_RUNTIME_SERVICES_DATA or _CODE. Agreed, EFI_LOADER_DATA should be for EFI apps. > > Reported-by: François-Frédéric Ozog > Signed-off-by: Heinrich Schuchardt [...] > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c > index a17b426d11..0c336f98d2 100644 > --- a/lib/efi_loader/efi_memory.c > +++ b/lib/efi_loader/efi_memory.c > @@ -823,7 +823,7 @@ static void add_u_boot_and_runtime(void) >uboot_stack_size) & ~EFI_PAGE_MASK; > uboot_pages = ((uintptr_t)map_sysmem(gd->ram_top - 1, 0) - >uboot_start + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT; > - efi_add_memory_map_pg(uboot_start, uboot_pages, EFI_LOADER_DATA, > + efi_add_memory_map_pg(uboot_start, uboot_pages, > EFI_BOOT_SERVICES_DATA, > false); I am not sure if we should have this as _DATA or _CODE. None of these is an exact match of what we allocate here and both of these are backed by EFI_MEMORY_WB. So your reasoning here is prefer _DATA since it's not memory that holds boottime service drivers? [...] Thanks /Ilias
Re: [PATCH 1/1] efi_loader: don't use EFI_LOADER_DATA internally
Reviewed-by: François-Frédéric Ozog
I confirm the patch addresses the issue
Le 29/11/2022 16:04, « Heinrich Schuchardt »
a écrit :
Memory allocated by U-Boot for internal usage should be
EFI_BOOT_SERVICES_DATA or _CODE or EFI_RUNTIME_SERVICES_DATA or _CODE.
Reported-by: François-Frédéric Ozog
Signed-off-by: Heinrich Schuchardt
---
cmd/efidebug.c | 2 +-
lib/efi_loader/efi_memory.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/cmd/efidebug.c b/cmd/efidebug.c
index ef239bb34b..64104da130 100644
--- a/cmd/efidebug.c
+++ b/cmd/efidebug.c
@@ -600,7 +600,7 @@ static int do_efi_show_memmap(struct cmd_tbl *cmdtp,
int flag,
ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL);
if (ret == EFI_BUFFER_TOO_SMALL) {
map_size += sizeof(struct efi_mem_desc); /* for my own */
- ret = efi_allocate_pool(EFI_LOADER_DATA, map_size,
+ ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, map_size,
(void *)&memmap);
if (ret != EFI_SUCCESS)
return CMD_RET_FAILURE;
diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
index a17b426d11..0c336f98d2 100644
--- a/lib/efi_loader/efi_memory.c
+++ b/lib/efi_loader/efi_memory.c
@@ -823,7 +823,7 @@ static void add_u_boot_and_runtime(void)
uboot_stack_size) & ~EFI_PAGE_MASK;
uboot_pages = ((uintptr_t)map_sysmem(gd->ram_top - 1, 0) -
uboot_start + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT;
- efi_add_memory_map_pg(uboot_start, uboot_pages, EFI_LOADER_DATA,
+ efi_add_memory_map_pg(uboot_start, uboot_pages, EFI_BOOT_SERVICES_DATA,
false);
#if defined(__aarch64__)
@@ -857,7 +857,7 @@ int efi_memory_init(void)
/* Request a 32bit 64MB bounce buffer region */
uint64_t efi_bounce_buffer_addr = 0x;
- if (efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS, EFI_LOADER_DATA,
+ if (efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS, EFI_BOOT_SERVICES_DATA,
(64 * 1024 * 1024) >> EFI_PAGE_SHIFT,
&efi_bounce_buffer_addr) != EFI_SUCCESS)
return -1;
--
2.37.2
Re: [u-boot][PATCH 05/14] mtd: rawnand: omap_gpmc: Fix BCH6/16 HW based correction
On Tue, Nov 29, 2022 at 04:25:13PM +0100, Dario Binacchi wrote: > Hi Roger, > > On Tue, Oct 11, 2022 at 1:50 PM Roger Quadros wrote: > > > > The BCH detection hardware can generate ECC bytes for multiple > > sectors in one go. Use that feature. > > > > correct() only corrects one sector at a time so we need to call it > > repeatedly for each sector. > > > > Signed-off-by: Roger Quadros [snip] > > +/** > > + * omap_calculate_ecc_bch - ECC generator for 1 sector > > + * @mtd:MTD device structure > > + * @dat: The pointer to data on which ecc is computed > > + * @ecc_code: The ecc_code buffer > > + * > > + * Support calculating of BCH4/8/16 ECC vectors for one sector. This is > > used > > + * when SW based correction is required as ECC is required for one sector > > + * at a time. > > + */ > > +static int omap_calculate_ecc_bch(struct mtd_info *mtd, > > + const u_char *dat, u_char *ecc_calc) > > Please add __maybe_unused. Without it the CI/CD pipeline fails: > >arm: + devkit8000 > +drivers/mtd/nand/raw/omap_gpmc.c:442:12: error: > 'omap_calculate_ecc_bch' defined but not used > [-Werror=unused-function] > + 442 | static int omap_calculate_ecc_bch(struct mtd_info *mtd, > + |^~ > +cc1: all warnings being treated as errors While not a firm rule, a general suggestion is if it's easy to fix a CI error like this, do so (and add Signed-off-by tag) during testing a PR rather than ask for a resubmit. Unless there's other more complex changes needed as well. -- Tom signature.asc Description: PGP signature
Re: [PATCH 1/1] efi_loader: don't use EFI_LOADER_DATA internally
On 11/29/22 16:38, Ilias Apalodimas wrote: Hi Heinrich, On Tue, 29 Nov 2022 at 17:04, Heinrich Schuchardt wrote: Memory allocated by U-Boot for internal usage should be EFI_BOOT_SERVICES_DATA or _CODE or EFI_RUNTIME_SERVICES_DATA or _CODE. Agreed, EFI_LOADER_DATA should be for EFI apps. Reported-by: François-Frédéric Ozog Signed-off-by: Heinrich Schuchardt [...] diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index a17b426d11..0c336f98d2 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -823,7 +823,7 @@ static void add_u_boot_and_runtime(void) uboot_stack_size) & ~EFI_PAGE_MASK; uboot_pages = ((uintptr_t)map_sysmem(gd->ram_top - 1, 0) - uboot_start + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT; - efi_add_memory_map_pg(uboot_start, uboot_pages, EFI_LOADER_DATA, + efi_add_memory_map_pg(uboot_start, uboot_pages, EFI_BOOT_SERVICES_DATA, false); I am not sure if we should have this as _DATA or _CODE. None of these is an exact match of what we allocate here and both of these are backed by EFI_MEMORY_WB. So your reasoning here is prefer _DATA since it's not memory that holds boottime service drivers? We are lacking a clear separation of data and code here. We would have to add another pointer global data and enforce that data is in separate pages if we wanted to do so. The same problem exists when loading applications as some sections are data and others are code but we put all into EFI_LOADER_CODE. Please, tell if you would prefer EFI_BOOT_SERVICES_CODE here. Best regards Heinrich
[PATCH] configs: set CONFIG_LMB_MAX_REGIONS=64 for all mt798[16] boards
With recently added wireless offloading features in Linux [1] the number of reserved memory regions with MediaTek SoCs supporting offloading wireless-to-Ethernet traffic grew beyond the default (8) which breaks booting Linux: ERROR: Failed to allocate 0xa6ac bytes below 0xc000. device tree - allocation error FDT creation failed! resetting ... Raise CONFIG_LMB_MAX_REGIONS to 64 like it is already done for other SoCs which require a larger number of reserved memory regions, eg. exynos78x0 based a3y17lte, a5y17lte and a7y17lte or dragonboard845c. [1]: https://lore.kernel.org/netdev/e3489a697b404bd47447190cd2e5adf090ae61c2.1667687249.git.lore...@kernel.org/ https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net-next.git/commit/?id=eed4f1ddad8c5ad7596b229caec8bd7b477b81ee Signed-off-by: Daniel Golle --- configs/mt7981_emmc_rfb_defconfig| 1 + configs/mt7981_rfb_defconfig | 1 + configs/mt7981_sd_rfb_defconfig | 1 + configs/mt7986_rfb_defconfig | 1 + configs/mt7986a_bpir3_emmc_defconfig | 1 + configs/mt7986a_bpir3_sd_defconfig | 1 + 6 files changed, 6 insertions(+) diff --git a/configs/mt7981_emmc_rfb_defconfig b/configs/mt7981_emmc_rfb_defconfig index 4832a22643..b3b37b6e5e 100644 --- a/configs/mt7981_emmc_rfb_defconfig +++ b/configs/mt7981_emmc_rfb_defconfig @@ -62,3 +62,4 @@ CONFIG_MTK_SERIAL=y CONFIG_FAT_WRITE=y CONFIG_HEXDUMP=y # CONFIG_EFI_LOADER is not set +CONFIG_LMB_MAX_REGIONS=64 diff --git a/configs/mt7981_rfb_defconfig b/configs/mt7981_rfb_defconfig index c397527887..b7ffb4dfa7 100644 --- a/configs/mt7981_rfb_defconfig +++ b/configs/mt7981_rfb_defconfig @@ -64,3 +64,4 @@ CONFIG_SPI=y CONFIG_DM_SPI=y CONFIG_MTK_SPIM=y CONFIG_HEXDUMP=y +CONFIG_LMB_MAX_REGIONS=64 diff --git a/configs/mt7981_sd_rfb_defconfig b/configs/mt7981_sd_rfb_defconfig index 17592dc22b..85be9bbc50 100644 --- a/configs/mt7981_sd_rfb_defconfig +++ b/configs/mt7981_sd_rfb_defconfig @@ -62,3 +62,4 @@ CONFIG_MTK_SERIAL=y CONFIG_FAT_WRITE=y CONFIG_HEXDUMP=y # CONFIG_EFI_LOADER is not set +CONFIG_LMB_MAX_REGIONS=64 diff --git a/configs/mt7986_rfb_defconfig b/configs/mt7986_rfb_defconfig index 1363f9dc6d..ac91c93efb 100644 --- a/configs/mt7986_rfb_defconfig +++ b/configs/mt7986_rfb_defconfig @@ -64,3 +64,4 @@ CONFIG_SPI=y CONFIG_DM_SPI=y CONFIG_MTK_SPIM=y CONFIG_HEXDUMP=y +CONFIG_LMB_MAX_REGIONS=64 diff --git a/configs/mt7986a_bpir3_emmc_defconfig b/configs/mt7986a_bpir3_emmc_defconfig index 354159df9b..2d4876f299 100644 --- a/configs/mt7986a_bpir3_emmc_defconfig +++ b/configs/mt7986a_bpir3_emmc_defconfig @@ -62,3 +62,4 @@ CONFIG_MTK_SERIAL=y CONFIG_FAT_WRITE=y CONFIG_HEXDUMP=y # CONFIG_EFI_LOADER is not set +CONFIG_LMB_MAX_REGIONS=64 diff --git a/configs/mt7986a_bpir3_sd_defconfig b/configs/mt7986a_bpir3_sd_defconfig index db7ef98d80..08edfe7ac4 100644 --- a/configs/mt7986a_bpir3_sd_defconfig +++ b/configs/mt7986a_bpir3_sd_defconfig @@ -62,3 +62,4 @@ CONFIG_MTK_SERIAL=y CONFIG_FAT_WRITE=y CONFIG_HEXDUMP=y # CONFIG_EFI_LOADER is not set +CONFIG_LMB_MAX_REGIONS=64 -- 2.38.1
Re: [PATCH] usb: gadget: dfu: Fix the unchecked length field
While I haven't yet gotten around to trying DFU with this patch applied, my guess as to the issue would be the checks of the form "if (ctrl-> bRequestType == USB_DIR_OUT)" or "if (ctrl->bRequestType == USB_DIR_IN)". The bRequestType field contains many flag bits other than the direction bit. The checks should just check that the USB_DIR_IN bit (0x80) is set or not set, rather than checking if the entire ctrl->bRequestType field equals some value. Sultan On Mon, Nov 28, 2022 at 7:48 AM Marek Vasut wrote: > On 11/21/22 18:34, Tom Rini wrote: > > On Thu, Nov 03, 2022 at 09:37:48AM +0530, Venkatesh Yadav Abbarapu wrote: > > > >> DFU implementation does not bound the length field in USB > >> DFU download setup packets, and it does not verify that > >> the transfer direction. Fixing the length and transfer > >> direction. > >> > >> CVE-2022-2347 > >> > >> Signed-off-by: Venkatesh Yadav Abbarapu > >> Reviewed-by: Marek Vasut > > > > Applied to u-boot/master, thanks! > > So this breaks DFU support in SPL as I just found out. > Any idea why ? >
Re: [PATCH] usb: gadget: dfu: Fix the unchecked length field
On 11/29/22 20:49, Sultan Khan wrote: While I haven't yet gotten around to trying DFU with this patch applied, my guess as to the issue would be the checks of the form "if (ctrl-> bRequestType == USB_DIR_OUT)" or "if (ctrl->bRequestType == USB_DIR_IN)". The bRequestType field contains many flag bits other than the direction bit. The checks should just check that the USB_DIR_IN bit (0x80) is set or not set, rather than checking if the entire ctrl->bRequestType field equals some value. Sultan I'm looking forward to a proper fix, thanks !
Re: [PATCH v8 4/8] net: dsa: allow rcv() and xmit() to be optional
On Mon, Nov 28, 2022 at 7:58 AM Tom Rini wrote:
>
> On Thu, Oct 27, 2022 at 05:49:33PM -0700, Tim Harvey wrote:
>
> > Allow rcv() and xmit() dsa driver ops to be optional in case a driver
> > does not care to mangle a packet as in U-Boot only one network port is
> > enabled at a time and thus no packet mangling is necessary.
> >
> > Suggested-by: Vladimir Oltean
> > Signed-off-by: Tim Harvey
> > Reviewed-by: Vladimir Oltean
> > Reviewed-by: Fabio Estevam
>
> This causes:
> FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_dsa] - AssertionError:
> assert False
>
> In sandbox, and I don't know if the test or the code is wrong.
>
Tom,
I'm not familiar at all with U-Boot's sandbox or unit test
infrastructure and am trying to learn.
I've figured out how to build sandbox and run the dm_test_dsa
(./u-boot -T -c "ut dm dsa") and see the same failure as you but I
don't understand how to debug this as it seems prints I add in
dsa_port_send and dsa_port_recv (which is what this patch modifies)
don't get print when run from the test infrastructure.
When I boot u-boot sandbox (./u-boot) I don't see any network devices
at all - perhaps I'm not booting sandbox with dm or something? I need
to understand what devices/drivers sandbox is using and how to
recreate the network environment that the dm_test_dsa function is
using which calls 'net_loop':
net_ping_ip = string_to_ip("1.2.3.5");
env_set("ethact", "eth2");
ut_assertok(net_loop(PING));
env_set("ethact", "lan0");
ut_assertok(net_loop(PING));
env_set("ethact", "lan1");
ut_assertok(net_loop(PING));
env_set("ethact", "");
Best Regards,
Tim
Re: [PATCH] usb: gadget: dfu: Fix the unchecked length field
Hi Sultan,
On Tue, Nov 29, 2022 at 4:49 PM Sultan Khan wrote:
>
> While I haven't yet gotten around to trying DFU with this patch applied, my
> guess as to the issue would be the checks of the form "if (ctrl->
> bRequestType == USB_DIR_OUT)" or "if (ctrl->bRequestType == USB_DIR_IN)".
> The bRequestType field contains many flag bits other than the direction
> bit. The checks should just check that the USB_DIR_IN bit (0x80) is set or
> not set, rather than checking if the entire ctrl->bRequestType field equals
> some value.
Is your suggestion as below?
--- a/drivers/usb/gadget/f_dfu.c
+++ b/drivers/usb/gadget/f_dfu.c
@@ -325,7 +325,7 @@ static int state_dfu_idle(struct f_dfu *f_dfu,
switch (ctrl->bRequest) {
case USB_REQ_DFU_DNLOAD:
- if (ctrl->bRequestType == USB_DIR_OUT) {
+ if (ctrl->bRequestType & USB_DIR_OUT) {
if (len == 0) {
f_dfu->dfu_state = DFU_STATE_dfuERROR;
value = RET_STALL;
@@ -337,7 +337,7 @@ static int state_dfu_idle(struct f_dfu *f_dfu,
}
break;
case USB_REQ_DFU_UPLOAD:
- if (ctrl->bRequestType == USB_DIR_IN) {
+ if (ctrl->bRequestType & USB_DIR_IN) {
f_dfu->dfu_state = DFU_STATE_dfuUPLOAD_IDLE;
f_dfu->blk_seq_num = 0;
value = handle_upload(req, len);
@@ -436,7 +436,7 @@ static int state_dfu_dnload_idle(struct f_dfu *f_dfu,
switch (ctrl->bRequest) {
case USB_REQ_DFU_DNLOAD:
- if (ctrl->bRequestType == USB_DIR_OUT) {
+ if (ctrl->bRequestType & USB_DIR_OUT) {
f_dfu->dfu_state = DFU_STATE_dfuDNLOAD_SYNC;
f_dfu->blk_seq_num = w_value;
value = handle_dnload(gadget, len);
@@ -527,7 +527,7 @@ static int state_dfu_upload_idle(struct f_dfu *f_dfu,
switch (ctrl->bRequest) {
case USB_REQ_DFU_UPLOAD:
- if (ctrl->bRequestType == USB_DIR_IN) {
+ if (ctrl->bRequestType & USB_DIR_IN) {
/* state transition if less data then requested */
f_dfu->blk_seq_num = w_value;
value = handle_upload(req, len);
Re: [PATCH v8 4/8] net: dsa: allow rcv() and xmit() to be optional
On Tue, Nov 29, 2022 at 02:53:15PM -0800, Tim Harvey wrote:
> On Mon, Nov 28, 2022 at 7:58 AM Tom Rini wrote:
> >
> > On Thu, Oct 27, 2022 at 05:49:33PM -0700, Tim Harvey wrote:
> >
> > > Allow rcv() and xmit() dsa driver ops to be optional in case a driver
> > > does not care to mangle a packet as in U-Boot only one network port is
> > > enabled at a time and thus no packet mangling is necessary.
> > >
> > > Suggested-by: Vladimir Oltean
> > > Signed-off-by: Tim Harvey
> > > Reviewed-by: Vladimir Oltean
> > > Reviewed-by: Fabio Estevam
> >
> > This causes:
> > FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_dsa] -
> > AssertionError: assert False
> >
> > In sandbox, and I don't know if the test or the code is wrong.
> >
>
> Tom,
>
> I'm not familiar at all with U-Boot's sandbox or unit test
> infrastructure and am trying to learn.
>
> I've figured out how to build sandbox and run the dm_test_dsa
> (./u-boot -T -c "ut dm dsa") and see the same failure as you but I
> don't understand how to debug this as it seems prints I add in
> dsa_port_send and dsa_port_recv (which is what this patch modifies)
> don't get print when run from the test infrastructure.
>
> When I boot u-boot sandbox (./u-boot) I don't see any network devices
> at all - perhaps I'm not booting sandbox with dm or something? I need
> to understand what devices/drivers sandbox is using and how to
> recreate the network environment that the dm_test_dsa function is
> using which calls 'net_loop':
>
> net_ping_ip = string_to_ip("1.2.3.5");
>
> env_set("ethact", "eth2");
> ut_assertok(net_loop(PING));
>
> env_set("ethact", "lan0");
> ut_assertok(net_loop(PING));
> env_set("ethact", "lan1");
> ut_assertok(net_loop(PING));
>
> env_set("ethact", "");
This is, I think, covered by:
https://u-boot.readthedocs.io/en/latest/develop/tests_sandbox.html
and that you want ./u-boot -T (or -d path/to/test.dtb)
--
Tom
signature.asc
Description: PGP signature
Re: [PATCH v4] schemas: Add schema for U-Boot driver model 'phase tags'
Hi Rob,
ping for any comments?
- Simon
On Wed, 23 Nov 2022 at 12:59, Simon Glass wrote:
>
> Hi Rob,
>
> On Tue, 15 Nov 2022 at 14:56, Simon Glass wrote:
> >
> > Hi Rob,
> >
> > On Mon, 14 Nov 2022 at 10:44, Rob Herring wrote:
> > >
> > > +Ilias, Bill and Joakim
> > >
> > > On Sat, Nov 12, 2022 at 9:21 AM Simon Glass wrote:
> > > >
> > > > Hi Rob,
> > > >
> > > > (unfortunately I have a filter on this list due to the insane traffic
> > > > and am not sure how to let these emails through, so I just saw this)
> > >
> > > https://lore.kernel.org/linux-devicetree/?q=a%3Asjg%40chromium.org
> >
> > Thanks, I stumbled upon that a week ago and the search works well.
> > Will bookmark. But as to filters I am using gmail for now...
> >
> > >
> > > And 'lei' can make that search a persistent mailbox.
> >
> > What is lei?
> >
> > >
> > > >
> > > > On Thu, 10 Nov 2022 at 11:30, Rob Herring wrote:
> > > > >
> > > > > On Thu, Nov 10, 2022 at 10:59 AM Simon Glass
> > > > > wrote:
> > > > > >
> > > > > > Hi Rob,
> > > > > >
> > > > > > On Tue, 8 Nov 2022 at 10:19, Rob Herring wrote:
> > > > > > >
> > > > > > > On Tue, Nov 1, 2022 at 10:13 PM Simon Glass
> > > > > > > wrote:
> > > > > > > >
> > > > > > > > U-Boot has some particular challenges with device tree and
> > > > > > > > devices:
> > > > > > > >
> > > > > > > > - U-Boot has multiple build phases, such as a Secondary Program
> > > > > > > > Loader
> > > > > > > > (SPL) phase which typically runs in a pre-SDRAM environment
> > > > > > > > where code
> > > > > > > > and data space are limited. In particular, there may not be
> > > > > > > > enough
> > > > > > > > space for the full device tree blob. U-Boot uses various
> > > > > > > > automated
> > > > > > > > techniques to reduce the size from perhaps 40KB to 3KB. It is
> > > > > > > > not
> > > > > > > > always possible to handle these tags entirely at build time,
> > > > > > > > since
> > > > > > > > U-Boot proper must have the full device tree, even though we
> > > > > > > > do not
> > > > > > > > want it to process all nodes until after relocation.
> > > > > > > > - Some U-Boot phases needs to run before the clocks are
> > > > > > > > properly set up,
> > > > > > > > where the CPU may be running very slowly. Therefore it is
> > > > > > > > important to
> > > > > > > > bind only those devices which are actually needed in that
> > > > > > > > phase
> > > > > > > > - U-Boot uses lazy initialisation for its devices, with 'bind'
> > > > > > > > and
> > > > > > > > 'probe' being separate steps. Even if a device is bound, it
> > > > > > > > is not
> > > > > > > > actually probed until it is used. This is necessary to keep
> > > > > > > > the boot
> > > > > > > > time reasonable, e.g. to under a second
> > > > > > > >
> > > > > > > > The phases of U-Boot in order are: TPL, VPL, SPL, U-Boot (first
> > > > > > > > pre-relocation, then post-relocation). ALl but the last two are
> > > > > > > > optional.
> > > > > > > >
> > > > > > > > For the above reasons, U-Boot only includes the full device
> > > > > > > > tree in the
> > > > > > > > final 'U-Boot proper' build. Even then, before relocation
> > > > > > > > U-Boot only
> > > > > > > > processes nodes which are marked as being needed.
> > > > > > > >
> > > > > > > > For this to work, U-Boot's driver model[1] provides a way to
> > > > > > > > mark device
> > > > > > > > tree nodes as applicable for a particular phase. This works by
> > > > > > > > adding a
> > > > > > > > tag to the node, e.g.:
> > > > > > > >
> > > > > > > >cru: clock-controller@ff76 {
> > > > > > > > phase,all;
> > > > > > > > compatible = "rockchip,rk3399-cru";
> > > > > > > > reg = <0x0 0xff76 0x0 0x1000>;
> > > > > > > > rockchip,grf = <&grf>;
> > > > > > > > #clock-cells = <1>;
> > > > > > > > #reset-cells = <1>;
> > > > > > > > ...
> > > > > > > >};
> > > > > > > >
> > > > > > > > Here the "phase,all" tag indicates that the node must be
> > > > > > > > present in all
> > > > > > > > phases, since the clock driver is required.
> > > > > > > >
> > > > > > > > There has been discussion over the years about whether this
> > > > > > > > could be done
> > > > > > > > in a property instead, e.g.
> > > > > > > >
> > > > > > > >options {
> > > > > > > > phase,all = <&cru> <&gpio_a> ...;
> > > > > > > > ...
> > > > > > > >};
> > > > > > > >
> > > > > > > > Some problems with this:
> > > > > > > >
> > > > > > > > - we need to be able to merge several such tags from different
> > > > > > > > .dtsi files
> > > > > > > > since many boards have their own specific requirements
> > > > > > > > - it is hard to find and cross-reference the affected nodes
> > > > > > > > - it is more error-prone
> > > > > > > > - it requires significant tool rework in U-Boot, including
> > > > > > > > fdtgrep and
> > > > > > > > the build system
> > > > > > > > - is harder (slower, more code) to pr
Re: [PATCH v8 4/8] net: dsa: allow rcv() and xmit() to be optional
On Tue, Nov 29, 2022 at 02:53:15PM -0800, Tim Harvey wrote:
> On Mon, Nov 28, 2022 at 7:58 AM Tom Rini wrote:
> >
> > On Thu, Oct 27, 2022 at 05:49:33PM -0700, Tim Harvey wrote:
> >
> > > Allow rcv() and xmit() dsa driver ops to be optional in case a driver
> > > does not care to mangle a packet as in U-Boot only one network port is
> > > enabled at a time and thus no packet mangling is necessary.
> > >
> > > Suggested-by: Vladimir Oltean
> > > Signed-off-by: Tim Harvey
> > > Reviewed-by: Vladimir Oltean
> > > Reviewed-by: Fabio Estevam
> >
> > This causes:
> > FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_dsa] -
> > AssertionError: assert False
> >
> > In sandbox, and I don't know if the test or the code is wrong.
> >
>
> Tom,
>
> I'm not familiar at all with U-Boot's sandbox or unit test
> infrastructure and am trying to learn.
>
> I've figured out how to build sandbox and run the dm_test_dsa
> (./u-boot -T -c "ut dm dsa") and see the same failure as you but I
> don't understand how to debug this as it seems prints I add in
> dsa_port_send and dsa_port_recv (which is what this patch modifies)
> don't get print when run from the test infrastructure.
>
> When I boot u-boot sandbox (./u-boot) I don't see any network devices
> at all - perhaps I'm not booting sandbox with dm or something? I need
> to understand what devices/drivers sandbox is using and how to
> recreate the network environment that the dm_test_dsa function is
> using which calls 'net_loop':
>
> net_ping_ip = string_to_ip("1.2.3.5");
>
> env_set("ethact", "eth2");
> ut_assertok(net_loop(PING));
>
> env_set("ethact", "lan0");
> ut_assertok(net_loop(PING));
> env_set("ethact", "lan1");
> ut_assertok(net_loop(PING));
>
> env_set("ethact", "");
>
> Best Regards,
>
> Tim
I use the following steps:
export KBUILD_OUTPUT=output-sandbox
make sandbox_defconfig NO_SDL=1
make -j 8 NO_SDL=1
$KBUILD_OUTPUT/u-boot -d $KBUILD_OUTPUT/arch/sandbox/dts/test.dtb
setenv ethact lan1
ping 1.2.3.5
ut dm dsa
In this case the problem with the patch is simple, sorry for not
noticing during review.
dsa_port_mangle_packet() changes the "length" variable. But if ops->xmit
exists, eth_get_ops(master)->send() is called with a bad (old) length,
the one pre mangling.
Re: [PATCH] riscv: use imply instead of select for SPL_SEPARATE_BSS
> From: Zong Li > Sent: Tuesday, November 29, 2022 10:02 AM > To: Sean Anderson > Cc: s...@chromium.org; michal.si...@amd.com; sean.ander...@seco.com; Leo > Yu-Chi Liang(梁育齊) ; Rick Jian-Zhi Chen(陳建志) > ; u-boot@lists.denx.de > Subject: Re: [PATCH] riscv: use imply instead of select for SPL_SEPARATE_BSS > > On Mon, Nov 21, 2022 at 8:17 PM Zong Li wrote: > > > > On Mon, Nov 21, 2022 at 12:00 PM Sean Anderson wrote: > > > > > > On 11/16/22 02:08, Zong Li wrote: > > > > Use imply instead of select, then it can still be disabled by > > > > board-specific defconfig, or be set to n manually. > > > > > > > > Signed-off-by: Zong Li > > > > --- > > > > arch/Kconfig | 2 +- > > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > > > diff --git a/arch/Kconfig b/arch/Kconfig index > > > > ae39716697..102956d24c 100644 > > > > --- a/arch/Kconfig > > > > +++ b/arch/Kconfig > > > > @@ -111,7 +111,7 @@ config RISCV > > > > select SUPPORT_OF_CONTROL > > > > select OF_CONTROL > > > > select DM > > > > - select SPL_SEPARATE_BSS if SPL > > > > + imply SPL_SEPARATE_BSS if SPL > > > > imply DM_SERIAL > > > > imply DM_ETH > > > > imply DM_EVENT > > > > > > Do you have an example of a board which does this? > > > > > > > Hi Sean, > > We'd like to disable 'SPL_SEPARATE_BSS' on our internal platforms that > > don't exist in the mainline. It seems to me that using 'imply' might > > be not only working on the mainline's board, but also making it > > flexible to disable 'SPL_SEPARATE_BSS' by board-specific configuration > > or disable it manually for debug purposes. Hope the idea is good to > > you all. Thanks > > > > Hi all, > Thanks for your reviewing, could I know whether this patch makes sense to you? It's reasonable for me. Thanks, Rick > > > > --Sean
Re: [PATCH v8 4/8] net: dsa: allow rcv() and xmit() to be optional
On Tue, Nov 29, 2022 at 4:11 PM Vladimir Oltean wrote:
>
> On Tue, Nov 29, 2022 at 02:53:15PM -0800, Tim Harvey wrote:
> > On Mon, Nov 28, 2022 at 7:58 AM Tom Rini wrote:
> > >
> > > On Thu, Oct 27, 2022 at 05:49:33PM -0700, Tim Harvey wrote:
> > >
> > > > Allow rcv() and xmit() dsa driver ops to be optional in case a driver
> > > > does not care to mangle a packet as in U-Boot only one network port is
> > > > enabled at a time and thus no packet mangling is necessary.
> > > >
> > > > Suggested-by: Vladimir Oltean
> > > > Signed-off-by: Tim Harvey
> > > > Reviewed-by: Vladimir Oltean
> > > > Reviewed-by: Fabio Estevam
> > >
> > > This causes:
> > > FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_dsa] -
> > > AssertionError: assert False
> > >
> > > In sandbox, and I don't know if the test or the code is wrong.
> > >
> >
> > Tom,
> >
> > I'm not familiar at all with U-Boot's sandbox or unit test
> > infrastructure and am trying to learn.
> >
> > I've figured out how to build sandbox and run the dm_test_dsa
> > (./u-boot -T -c "ut dm dsa") and see the same failure as you but I
> > don't understand how to debug this as it seems prints I add in
> > dsa_port_send and dsa_port_recv (which is what this patch modifies)
> > don't get print when run from the test infrastructure.
> >
> > When I boot u-boot sandbox (./u-boot) I don't see any network devices
> > at all - perhaps I'm not booting sandbox with dm or something? I need
> > to understand what devices/drivers sandbox is using and how to
> > recreate the network environment that the dm_test_dsa function is
> > using which calls 'net_loop':
> >
> > net_ping_ip = string_to_ip("1.2.3.5");
> >
> > env_set("ethact", "eth2");
> > ut_assertok(net_loop(PING));
> >
> > env_set("ethact", "lan0");
> > ut_assertok(net_loop(PING));
> > env_set("ethact", "lan1");
> > ut_assertok(net_loop(PING));
> >
> > env_set("ethact", "");
> >
> > Best Regards,
> >
> > Tim
>
> I use the following steps:
>
> export KBUILD_OUTPUT=output-sandbox
> make sandbox_defconfig NO_SDL=1
> make -j 8 NO_SDL=1
> $KBUILD_OUTPUT/u-boot -d $KBUILD_OUTPUT/arch/sandbox/dts/test.dtb
> setenv ethact lan1
> ping 1.2.3.5
> ut dm dsa
Thanks - now I understand how to feed sandbox a dtb!
>
> In this case the problem with the patch is simple, sorry for not
> noticing during review.
>
> dsa_port_mangle_packet() changes the "length" variable. But if ops->xmit
> exists, eth_get_ops(master)->send() is called with a bad (old) length,
> the one pre mangling.
Yes, it makes sense. How about the following patch instead:
diff --git a/net/dsa-uclass.c b/net/dsa-uclass.c
index 211a991cdd0d..1ae9adc66eda 100644
--- a/net/dsa-uclass.c
+++ b/net/dsa-uclass.c
@@ -142,6 +142,9 @@ static int dsa_port_send(struct udevice *pdev,
void *packet, int length)
struct dsa_port_pdata *port_pdata;
int err;
+ if (!ops->xmit)
+ return eth_get_ops(master)->send(master, packet, length);
+
if (length + head + tail > PKTSIZE_ALIGN)
return -EINVAL;
@@ -172,7 +175,7 @@ static int dsa_port_recv(struct udevice *pdev, int
flags, uchar **packetp)
int length, port_index, err;
length = eth_get_ops(master)->recv(master, flags, packetp);
- if (length <= 0)
+ if (length <= 0 || !ops->rcv)
return length;
/*
Perhaps it's more elegant to wrap the bulk of dsa_port_send with an
'if (ops->xmit)' but changing the indentation makes the patch more
difficult to follow.
Best Regards,
Tim
Re: [TF-A] [RFC] Proposed location to host the firmware handoff specification.
Hi Jose, Apologies for the late response, I had to find some time to dig back into this topic first. > The proposal is that the tag assignments are handled via a PR to [1]. > A PR should provide reasoning for the proposed entry layout as well as a > description of the use-case being serviced. > The community is invited to comment on the pull requests. We should aim to > have community consensus before merging any PR. I think this is a good approach if we understand "community consensus" to be a quick, couple-of-days review cycle like we have for TF-A patches that doesn't create a huge threshold for additions, and it's fine for entries to be very specific to the needs of a single platform when necessary. But I think the document should still have more guidance about when standard vs. non-standard tags should be used, and in particular just strongly discourage the use of non-standard tags in general (basically I think they should only be used for experiments that won't reach production code, or for cases where all code reading or writing that tag is closed source). I think the spec should work really hard in its language and guidance to discourage a situation where firmware components just assign a non-standard tag to something because it's easier and avoids the hassle of submitting something you don't think you'd want to share at the time, and then later it becomes entrenched and reused and eventually everyone has to pay attention to not reuse the non-standard tags that accidentally acquired a fixed meaning in enough implementations to become a problem. (It may also make sense to just make the standard tag range much bigger than the non-standard range, to make it clearer that the majority of things are expected to go in there.) > It seems sensible that data contained in an entry should not exceed 32MB. > Could we still accommodate the notion of a hdr_size in the TL entry header? > We'd have the following fields on an 8-byte TE header: > tag_id -- 4 bytes > hdr_size -- 1 byte > Data_size -- 3 bytes Yeah, that seems like a reasonable compromise, if you really think that variable header length is useful. I still think the table header could be smaller as well (down to 16 bytes from 32 by just avoiding so much reserved space). Do you want me to send those suggestions as pull requests to make them easier to discuss? > The document states that any tag in the standard range must be allocated in > the spec before being used in code [4] -- what this is really saying is: > until a tag id is allocated anyone can request it. So code written assuming a > particular standard tag id (absent from the spec) may have to change later if > someone in the meantime allocates that tag id for a different entry type. > I believe it’s a per-community policy to prevent code submissions containing > standard tag ids until these have been reserved in this spec. > This is the advised policy, but this spec cannot mandate it, rather we must > rely on the communities to enforce this. I would say the spec should mandate that, honestly. If there is a standardized range, no firmware implementing transfer lists according to this spec should use tags from that range until they have been defined in the global repository. On the flip side, we need to make the process of defining new tags fast and painless so that this requirement doesn't become a burden. > For the current set of entries, the data is included within the TL. Note that > this does not prevent newer entries from pointing to off-TL data. > Could this be handled on a case-by-case basis, via pull request, after an > initial full release of the spec? Yes, we can leave it like this and later add tags for the off-list equivalent of the tags defined there if necessary.
Re: [TF-A] [RFC] Proposed location to host the firmware handoff specification.
Hi, On Wed, 30 Nov 2022 at 14:52, Julius Werner wrote: > > Hi Jose, > > Apologies for the late response, I had to find some time to dig back > into this topic first. > > > The proposal is that the tag assignments are handled via a PR to [1]. > > A PR should provide reasoning for the proposed entry layout as well as a > > description of the use-case being serviced. > > The community is invited to comment on the pull requests. We should aim to > > have community consensus before merging any PR. > > I think this is a good approach if we understand "community consensus" > to be a quick, couple-of-days review cycle like we have for TF-A > patches that doesn't create a huge threshold for additions, and it's > fine for entries to be very specific to the needs of a single platform > when necessary. But I think the document should still have more > guidance about when standard vs. non-standard tags should be used, and > in particular just strongly discourage the use of non-standard tags in > general (basically I think they should only be used for experiments > that won't reach production code, or for cases where all code reading > or writing that tag is closed source). I think the spec should work That's right > really hard in its language and guidance to discourage a situation > where firmware components just assign a non-standard tag to something > because it's easier and avoids the hassle of submitting something you > don't think you'd want to share at the time, and then later it becomes > entrenched and reused and eventually everyone has to pay attention to > not reuse the non-standard tags that accidentally acquired a fixed > meaning in enough implementations to become a problem. (It may also > make sense to just make the standard tag range much bigger than the > non-standard range, to make it clearer that the majority of things are > expected to go in there.) One way to enforce that is for firmware components to refuse to accept a transfer list with unknown tags, assuming that all firmware is built from source at the same time. It might be a pain, but I suspect it could help avoid security problems, where bad actors use the structure to pass code to a firmware blob, etc. > > > It seems sensible that data contained in an entry should not exceed 32MB. > > Could we still accommodate the notion of a hdr_size in the TL entry header? > > We'd have the following fields on an 8-byte TE header: > > tag_id -- 4 bytes > > hdr_size -- 1 byte > > Data_size -- 3 bytes > > Yeah, that seems like a reasonable compromise, if you really think > that variable header length is useful. > > I still think the table header could be smaller as well (down to 16 > bytes from 32 by just avoiding so much reserved space). > > Do you want me to send those suggestions as pull requests to make them > easier to discuss? I have to disagree here. The context is gone in this email, so I cannot reply to the comments there. But I think it was talking about using the transfer list for very tiny structures, just a few words. That is not the intent. Firstly we should really have TF-A use the device tree (embedded in the transfer list) instead of replicating bl_params in this structure. Secondly, we should group things into a C structure that is at least 16-32 bytes long. So I don't see a need for a smaller header. The 16-byte alignment works well with ACPI tables and we should not be using this structure for tiny things where the 8-byte overhead is significant. Believe me I have spent plenty of time dealing with memory space in U-Boot TPL, but As to the maximum size, I can see how things might grow larger, for example to pass large amounts of data between firmware pieces, such as images, executables, even a ramdisk. It seems short-sighted to limit the size to 32MB. For the header size, this provides for backwards compatibility in the case where we need to store more there. I don't think this is a big deal and certainly one byte should be enough... But overall I think having a 16-byte header makes sense. > > > The document states that any tag in the standard range must be allocated in > > the spec before being used in code [4] -- what this is really saying is: > > until a tag id is allocated anyone can request it. So code written assuming > > a particular standard tag id (absent from the spec) may have to change > > later if someone in the meantime allocates that tag id for a different > > entry type. > > I believe it’s a per-community policy to prevent code submissions > > containing standard tag ids until these have been reserved in this spec. > > This is the advised policy, but this spec cannot mandate it, rather we must > > rely on the communities to enforce this. > > I would say the spec should mandate that, honestly. If there is a > standardized range, no firmware implementing transfer lists according > to this spec should use tags from that range until they have been > defined in the global repository. On the flip side
Re: [PATCH] usb: gadget: dfu: Fix the unchecked length field
Almost, but not quite. USB_DIR_IN is 0x80, USB_DIR_OUT is just 0, so testing
(ctrl->bRequestType & USB_DIR_OUT) is meaningless. Instead, the test for an OUT
transfer should be !(ctrl->bRequestType & USB_DIR_IN).
Sultan
> On Nov 29, 2022, at 6:05 PM, Fabio Estevam wrote:
>
> Hi Sultan,
>
> On Tue, Nov 29, 2022 at 4:49 PM Sultan Khan wrote:
>>
>> While I haven't yet gotten around to trying DFU with this patch applied, my
>> guess as to the issue would be the checks of the form "if (ctrl->
>> bRequestType == USB_DIR_OUT)" or "if (ctrl->bRequestType == USB_DIR_IN)".
>> The bRequestType field contains many flag bits other than the direction
>> bit. The checks should just check that the USB_DIR_IN bit (0x80) is set or
>> not set, rather than checking if the entire ctrl->bRequestType field equals
>> some value.
>
> Is your suggestion as below?
>
> --- a/drivers/usb/gadget/f_dfu.c
> +++ b/drivers/usb/gadget/f_dfu.c
> @@ -325,7 +325,7 @@ static int state_dfu_idle(struct f_dfu *f_dfu,
>
>switch (ctrl->bRequest) {
>case USB_REQ_DFU_DNLOAD:
> - if (ctrl->bRequestType == USB_DIR_OUT) {
> + if (ctrl->bRequestType & USB_DIR_OUT) {
>if (len == 0) {
>f_dfu->dfu_state = DFU_STATE_dfuERROR;
>value = RET_STALL;
> @@ -337,7 +337,7 @@ static int state_dfu_idle(struct f_dfu *f_dfu,
>}
>break;
>case USB_REQ_DFU_UPLOAD:
> - if (ctrl->bRequestType == USB_DIR_IN) {
> + if (ctrl->bRequestType & USB_DIR_IN) {
>f_dfu->dfu_state = DFU_STATE_dfuUPLOAD_IDLE;
>f_dfu->blk_seq_num = 0;
>value = handle_upload(req, len);
> @@ -436,7 +436,7 @@ static int state_dfu_dnload_idle(struct f_dfu *f_dfu,
>
>switch (ctrl->bRequest) {
>case USB_REQ_DFU_DNLOAD:
> - if (ctrl->bRequestType == USB_DIR_OUT) {
> + if (ctrl->bRequestType & USB_DIR_OUT) {
>f_dfu->dfu_state = DFU_STATE_dfuDNLOAD_SYNC;
>f_dfu->blk_seq_num = w_value;
>value = handle_dnload(gadget, len);
> @@ -527,7 +527,7 @@ static int state_dfu_upload_idle(struct f_dfu *f_dfu,
>
>switch (ctrl->bRequest) {
>case USB_REQ_DFU_UPLOAD:
> - if (ctrl->bRequestType == USB_DIR_IN) {
> + if (ctrl->bRequestType & USB_DIR_IN) {
>/* state transition if less data then requested */
>f_dfu->blk_seq_num = w_value;
>value = handle_upload(req, len);
Re: [TF-A] [RFC] Proposed location to host the firmware handoff specification.
Okay, FWIW I created a pull request with my suggestions here: https://github.com/FirmwareHandoff/firmware_handoff/pull/4 That should make it easier to discuss specific details, hopefully. As I was looking at the header size and format more closely I noticed that the checksum calculation and some details about what the size fields mean seems underspecified right now, so I suggested a change for that as well. Let me know what you think. > One way to enforce that is for firmware components to refuse to accept > a transfer list with unknown tags, assuming that all firmware is built > from source at the same time. It might be a pain, but I suspect it > could help avoid security problems, where bad actors use the structure > to pass code to a firmware blob, etc. I don't think that's feasible, particularly if tag IDs are not allocated in order (which I think is useful so that we can try to group similar tags by vendor, e.g. if say MediaTek wanted to add some tags they could just find some nice round number in the tag space, say 0x200, and then start allocating 0x201, 0x202, 0x203, etc. while the next vendor can do their own thing at 0x300 instead). It also creates automatic incompatibilities if you put newer and older components in the same image, which I think is something we explicitly don't want. I think the right policy to encourage compatibility is that unknown tags just get ignored. > I have to disagree here. The context is gone in this email, so I > cannot reply to the comments there. But I think it was talking about > using the transfer list for very tiny structures, just a few words. > That is not the intent. Firstly we should really have TF-A use the > device tree (embedded in the transfer list) instead of replicating > bl_params in this structure. Secondly, we should group things into a C > structure that is at least 16-32 bytes long. Uhh... okay, sorry, but then you're going against everything that was discussed in this thread and in the previous discussions last year. The whole idea of making this a tagged list came from the argument that other structures (like FDT) are unnecessarily bulky and complex for some of the intended use cases (like replacing TF-A bl_aux_params). I mean, what's the point of even having a transfer list if all it does is wrap an FDT? You could just use the FDT directly! I think all the 4 tags that are currently specified in that document are pathological edge cases that may be necessary for legacy applications but represent the opposite of how this structure *should* be used in the ideal case. We don't need just more wrappers for the stuff that we already have, that doesn't promote interoperability in any way. If project A only wants to use HOBs but not FDT, and project B only wants to use FDT but not HOBs, then creating a transfer list structure that can wrap both an FDT and a HOB in a TE is pointless and not going to fix anything (project A will still not be able to parse the FDT-wrapping TE, and project B will still not be able to parse the HOB-wrapping TE). The only way this whole exercise can actually be useful and improve anything is if we can use it to eventually (slowly) get away from both HOBs and FDTs towards a point where all data is encoded in TEs directly, without any extra layers. > So I don't see a need for a smaller header. The 16-byte alignment > works well with ACPI tables and we should not be using this structure > for tiny things where the 8-byte overhead is significant. FWIW, the compromise proposal now preserves the 16-byte TE alignment. (8 byte header, then small entries can use the next 8 bytes for data and the whole TE is only 16 bytes with only up to 7 bytes of forced padding. Once you need more than 8 bytes of data it gets a bit more ugly but at that point I guess you're not really a super small entry anymore.)
Re: [RFC PATCH] board: ti: common: board_detect: Fix EEPROM read quirk for 2-byte
Hi Matwey
On 29/11/22 17:40, Matwey V. Kornilov wrote:
вт, 29 нояб. 2022 г. в 09:50, Neha Malcom Francis :
EEPROM detection logic in ti_i2c_eeprom_get() involves figuring out
whether addressing is 1-byte or 2-byte. There are currently different
behaviours seen across boards as documented in commit bf6376642fe8
("board: ti: common: board_detect: Fix EEPROM read quirk"). Adding to
the list, we see that there are 2-byte EEPROMs that read properly
with 1-byte addressing with no offset.
For ti_i2c_eeprom_am6_get where eeprom parse operation is dynamic, the
earlier commit d2ab2a2bafd5 ("board: ti: common: board_detect: Fix
EEPROM read quirk for AM6 style data") tried to resolve this by running
ti_i2c_eeprom_get() twice. However this commit along with its former
commit fails on J7 platforms where EEPROM successfully return back the
header on 1-byte addressing and continues to do so until an offset is
introduced. So the second read incorrectly determines the EEPROM as
1-byte addressing.
A more generic solution is introduced here to solve
this issue: 1-byte read without offset and 1-byte read with offset. If
both passes, it follows 1-byte addressing else we proceed with 2-byte
addressing check.
Tested on J721E, J7200, DRA7xx, AM64x
I'll try to test this on the AM335x boards I have as soon as possible.
Thanks!
Signed-off-by: Neha Malcom Francis
Fixes: d2ab2a2bafd5 (board: ti: common: board_detect: Fix EEPROM read
quirk for AM6 style data) and bf6376642fe8 (board: ti: common: board_detect:
Fix EEPROM read quirk)
---
board/ti/common/board_detect.c | 29 ++---
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c
index c37629fe8a..b9f2ebf2a0 100644
--- a/board/ti/common/board_detect.c
+++ b/board/ti/common/board_detect.c
@@ -91,6 +91,8 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr, int
dev_addr,
#if CONFIG_IS_ENABLED(DM_I2C)
Should #else branch also be modified according to the new algo?
Yeah you're right; let me try getting into that as well.
struct udevice *dev;
struct udevice *bus;
+ uint8_t offset_test;
+ bool one_byte_addressing = true;
rc = uclass_get_device_by_seq(UCLASS_I2C, bus_addr, &bus);
if (rc)
@@ -114,8 +116,23 @@ static int __maybe_unused ti_i2c_eeprom_get(int bus_addr,
int dev_addr,
*/
(void)dm_i2c_read(dev, 0, ep, size);
+ if (*((u32 *)ep) != header)
+ one_byte_addressing = false;
+
+ /*
+* Handle case of bad 2 byte eeproms that responds to 1 byte addressing
+* but gets stuck in const addressing when read requests are performed
+* on offsets. We perform an offset test to make sure it is not a 2 byte
+* eeprom that works with 1 byte addressing but just without an offset
+*/
+
+ rc = dm_i2c_read(dev, 0x1, &offset_test, sizeof(offset_test));
+
+ if (*((u32 *)ep) != (header & 0xFF))
+ one_byte_addressing = false;
+
/* Corrupted data??? */
- if (*((u32 *)ep) != header) {
+ if (!one_byte_addressing) {
/*
* read the eeprom header using i2c again, but use only a
* 2 byte address (some newer boards need this..)
@@ -444,16 +461,6 @@ int __maybe_unused ti_i2c_eeprom_am6_get(int bus_addr, int
dev_addr,
if (rc)
return rc;
- /*
-* Handle case of bad 2 byte eeproms that responds to 1 byte addressing
-* but gets stuck in const addressing when read requests are performed
-* on offsets. We re-read the board ID to ensure we have sane data back
-*/
- rc = ti_i2c_eeprom_get(bus_addr, dev_addr, TI_EEPROM_HEADER_MAGIC,
- sizeof(board_id), (uint8_t *)&board_id);
- if (rc)
- return rc;
-
if (board_id.header.id != TI_AM6_EEPROM_RECORD_BOARD_ID) {
pr_err("%s: Invalid board ID record!\n", __func__);
return -EINVAL;
--
2.34.1
--
Thanking You
Neha Malcom Francis
Re: [RFC PATCH] board: ti: common: board_detect: Fix EEPROM read quirk for 2-byte
Hi Tom
On 29/11/22 19:01, Tom Rini wrote:
On Tue, Nov 29, 2022 at 03:10:12PM +0300, Matwey V. Kornilov wrote:
вт, 29 нояб. 2022 г. в 09:50, Neha Malcom Francis :
EEPROM detection logic in ti_i2c_eeprom_get() involves figuring out
whether addressing is 1-byte or 2-byte. There are currently different
behaviours seen across boards as documented in commit bf6376642fe8
("board: ti: common: board_detect: Fix EEPROM read quirk"). Adding to
the list, we see that there are 2-byte EEPROMs that read properly
with 1-byte addressing with no offset.
For ti_i2c_eeprom_am6_get where eeprom parse operation is dynamic, the
earlier commit d2ab2a2bafd5 ("board: ti: common: board_detect: Fix
EEPROM read quirk for AM6 style data") tried to resolve this by running
ti_i2c_eeprom_get() twice. However this commit along with its former
commit fails on J7 platforms where EEPROM successfully return back the
header on 1-byte addressing and continues to do so until an offset is
introduced. So the second read incorrectly determines the EEPROM as
1-byte addressing.
A more generic solution is introduced here to solve
this issue: 1-byte read without offset and 1-byte read with offset. If
both passes, it follows 1-byte addressing else we proceed with 2-byte
addressing check.
Tested on J721E, J7200, DRA7xx, AM64x
I'll try to test this on the AM335x boards I have as soon as possible.
I wonder if we should re-factor this code a bit and not have a single
ti_i2c_eeprom_get but instead build for whichever sets of quirks a given
family of boards has with their EEPROMs. I really worry that we're going
to find that this change here breaks yet another different EEPROM than
before.
Yes that does make sense... considering a new behavior of EEPROM keeps
showing up. I will try refactoring the logic that way.
--
Thanking You
Neha Malcom Francis
Re: [PATCH 1/1] efi_loader: don't use EFI_LOADER_DATA internally
On Tue, Nov 29, 2022 at 06:35:40PM +0100, Heinrich Schuchardt wrote: > On 11/29/22 16:38, Ilias Apalodimas wrote: > > Hi Heinrich, > > > > On Tue, 29 Nov 2022 at 17:04, Heinrich Schuchardt > > wrote: > > > > > > Memory allocated by U-Boot for internal usage should be > > > EFI_BOOT_SERVICES_DATA or _CODE or EFI_RUNTIME_SERVICES_DATA or _CODE. > > > > Agreed, EFI_LOADER_DATA should be for EFI apps. > > > > > > > > Reported-by: François-Frédéric Ozog > > > Signed-off-by: Heinrich Schuchardt > > > > [...] > > > > > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c > > > index a17b426d11..0c336f98d2 100644 > > > --- a/lib/efi_loader/efi_memory.c > > > +++ b/lib/efi_loader/efi_memory.c > > > @@ -823,7 +823,7 @@ static void add_u_boot_and_runtime(void) > > > uboot_stack_size) & ~EFI_PAGE_MASK; > > > uboot_pages = ((uintptr_t)map_sysmem(gd->ram_top - 1, 0) - > > > uboot_start + EFI_PAGE_MASK) >> EFI_PAGE_SHIFT; > > > - efi_add_memory_map_pg(uboot_start, uboot_pages, EFI_LOADER_DATA, > > > + efi_add_memory_map_pg(uboot_start, uboot_pages, > > > EFI_BOOT_SERVICES_DATA, > > >false); > > > > I am not sure if we should have this as _DATA or _CODE. None of these > > is an exact match of what we allocate here and both of these are > > backed by EFI_MEMORY_WB. So your reasoning here is prefer _DATA since > > it's not memory that holds boottime service drivers? > > We are lacking a clear separation of data and code here. We would have to > add another pointer global data and enforce that data is in separate pages > if we wanted to do so. > > The same problem exists when loading applications as some sections are data > and others are code but we put all into EFI_LOADER_CODE. > > Please, tell if you would prefer EFI_BOOT_SERVICES_CODE here. I think I prefer _CODE, but I don't really mind tbh With or without the changes. In case you update the patch can you add a sentence along the lines of "EFI_LOADER_DATA/CODE is reserved for EFI applications" Reviewed-by: Ilias Apalodimas > > Best regards > > Heinrich
Re: [PATCH 1/1] efi_loader: fix handling of DHCP acknowledge
Hi Heinrich,
On Sat, Nov 26, 2022 at 05:10:56PM +0100, Heinrich Schuchardt wrote:
> The dhcp command may be executed after the first UEFI command.
> We should still update the EFI_PXE_BASE_CODE_PROTOCOL.
>
> Don't leak content of prior acknowledge packages.
>
> Handle out of memory.
>
The patch looks correct, but the description is a bit confusing. Apart
from what you mention this patch also fixes
- An unchecked allocation
- shadowing of the global netobj
Can we please update the commit message to something that mentions all of
these?
Thanks
/Ilias
> Signed-off-by: Heinrich Schuchardt
> ---
> lib/efi_loader/efi_net.c | 13 ++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
> index 69276b275d..96a5bcca27 100644
> --- a/lib/efi_loader/efi_net.c
> +++ b/lib/efi_loader/efi_net.c
> @@ -30,6 +30,7 @@ static uchar **receive_buffer;
> static size_t *receive_lengths;
> static int rx_packet_idx;
> static int rx_packet_num;
> +static struct efi_net_obj *netobj;
>
> /*
> * The notification function of this event is called in every timer cycle
> @@ -660,10 +661,16 @@ void efi_net_set_dhcp_ack(void *pkt, int len)
> {
> int maxsize = sizeof(*dhcp_ack);
>
> - if (!dhcp_ack)
> + if (!dhcp_ack) {
> dhcp_ack = malloc(maxsize);
> -
> + if (!dhcp_ack)
> + return;
> + }
> + memset(dhcp_ack, 0, maxsize);
> memcpy(dhcp_ack, pkt, min(len, maxsize));
> +
> + if (netobj)
> + netobj->pxe_mode.dhcp_ack = *dhcp_ack;
> }
>
> /**
> @@ -853,7 +860,6 @@ static efi_status_t EFIAPI efi_pxe_base_code_set_packets(
> */
> efi_status_t efi_net_register(void)
> {
> - struct efi_net_obj *netobj = NULL;
> efi_status_t r;
> int i;
>
> @@ -982,6 +988,7 @@ failure_to_add_protocol:
> return r;
> out_of_resources:
> free(netobj);
> + netobj = NULL;
> free(transmit_buffer);
> if (receive_buffer)
> for (i = 0; i < ETH_PACKETS_BATCH_RECV; i++)
> --
> 2.37.2
>
Re: [PATCH 1/1] efi_loader: fix handling of DHCP acknowledge
On 11/30/22 08:37, Ilias Apalodimas wrote:
Hi Heinrich,
On Sat, Nov 26, 2022 at 05:10:56PM +0100, Heinrich Schuchardt wrote:
The dhcp command may be executed after the first UEFI command.
We should still update the EFI_PXE_BASE_CODE_PROTOCOL.
Don't leak content of prior acknowledge packages.
Handle out of memory.
The patch looks correct, but the description is a bit confusing. Apart
from what you mention this patch also fixes
- An unchecked allocation
I can add this.
- shadowing of the global netobj
netobj was a local variable before this patch.
Thanks for reviewing.
Best regards
Heinrich
Can we please update the commit message to something that mentions all of
these?
Thanks
/Ilias
Signed-off-by: Heinrich Schuchardt
---
lib/efi_loader/efi_net.c | 13 ++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
index 69276b275d..96a5bcca27 100644
--- a/lib/efi_loader/efi_net.c
+++ b/lib/efi_loader/efi_net.c
@@ -30,6 +30,7 @@ static uchar **receive_buffer;
static size_t *receive_lengths;
static int rx_packet_idx;
static int rx_packet_num;
+static struct efi_net_obj *netobj;
/*
* The notification function of this event is called in every timer cycle
@@ -660,10 +661,16 @@ void efi_net_set_dhcp_ack(void *pkt, int len)
{
int maxsize = sizeof(*dhcp_ack);
- if (!dhcp_ack)
+ if (!dhcp_ack) {
dhcp_ack = malloc(maxsize);
-
+ if (!dhcp_ack)
+ return;
+ }
+ memset(dhcp_ack, 0, maxsize);
memcpy(dhcp_ack, pkt, min(len, maxsize));
+
+ if (netobj)
+ netobj->pxe_mode.dhcp_ack = *dhcp_ack;
}
/**
@@ -853,7 +860,6 @@ static efi_status_t EFIAPI efi_pxe_base_code_set_packets(
*/
efi_status_t efi_net_register(void)
{
- struct efi_net_obj *netobj = NULL;
efi_status_t r;
int i;
@@ -982,6 +988,7 @@ failure_to_add_protocol:
return r;
out_of_resources:
free(netobj);
+ netobj = NULL;
free(transmit_buffer);
if (receive_buffer)
for (i = 0; i < ETH_PACKETS_BATCH_RECV; i++)
--
2.37.2
