[meta-xilinx] [PATCH] u-boot-xlnx_2016.07: Apply patch for Zynq to resolve obscure bug
Nathan Rossi
nathan at nathanrossi.com
Tue Dec 13 17:50:34 PST 2016
On 4 December 2016 at 19:58, Nathan Rossi <nathan at nathanrossi.com> wrote:
> This patch fixes a bug that was introduced in u-boot v2016.05 as well
> as xilinx-v2016.2. The bug can appear in variety of ways, but was
> observed as:
>
> Uncompressing Invalid Image ... Unimplemented compression type 23
>
> See patch for specific details.
>
> Signed-off-by: Nathan Rossi <nathan at nathanrossi.com>
I'm not going to apply this patch, as I have submitted a proper fix
upstream (which also fixes this issue for zynqmp too). Once accepted I
will then backport (for both master and morty) to resolve this issue.
Upstream patches for reference:
https://patchwork.ozlabs.org/patch/704936/
https://patchwork.ozlabs.org/patch/704937/
https://patchwork.ozlabs.org/patch/704938/
Regards,
Nathan
> ---
> ...place-dram_init-functions-with-board_init.patch | 213 +++++++++++++++++++++
> recipes-bsp/u-boot/u-boot-xlnx_2016.07.bb | 1 +
> 2 files changed, 214 insertions(+)
> create mode 100644 recipes-bsp/u-boot/u-boot-xlnx/ARM-zynq-Replace-dram_init-functions-with-board_init.patch
>
> diff --git a/recipes-bsp/u-boot/u-boot-xlnx/ARM-zynq-Replace-dram_init-functions-with-board_init.patch b/recipes-bsp/u-boot/u-boot-xlnx/ARM-zynq-Replace-dram_init-functions-with-board_init.patch
> new file mode 100644
> index 0000000000..b1885a9afc
> --- /dev/null
> +++ b/recipes-bsp/u-boot/u-boot-xlnx/ARM-zynq-Replace-dram_init-functions-with-board_init.patch
> @@ -0,0 +1,213 @@
> +From c5db12d072b43f1fc635edfdbece8256111faf42 Mon Sep 17 00:00:00 2001
> +From: Nathan Rossi <nathan at nathanrossi.com>
> +Date: Sun, 4 Dec 2016 19:33:22 +1000
> +Subject: [PATCH] ARM: zynq: Replace dram_init* functions with board_init_f
> + safe ones
> +
> +The dram_init* functions for the zynq board are not safe for use from
> +the board_init_f stage due to its use of the 'tmp' static variable.
> +
> +This incorrect use of a static variable was causing rare issues where
> +the dram_init function would overwrite some parts the __rel_dyn section
> +which caused obscure failures.
> +
> +This change removes the existing code and copies the implementation of
> +the dram_init and dram_init_banksize from the
> +arch/arm/mach-uniphier/dram_init.c source. This version of these
> +functions does not use static variables and behaves the same (reading
> +banks from fdt, and using the first bank as the ram_size).
> +
> +Signed-off-by: Nathan Rossi <nathan at nathanrossi.com>
> +Fixes: 758f29d0f8 ("ARM: zynq: Support systems with more memory banks")
> +Upstream-Status: Pending
> +---
> +This issue appears as the following error during image load for the Zybo
> +board, however this is just one potential way this bug can surface.
> +
> + Uncompressing Invalid Image ... Unimplemented compression type 23
> +
> +Changes in section sizes will vary the outcomes, and in some cases not
> +present any issues during normal use.
> +---
> + board/xilinx/zynq/board.c | 148 ++++++++++++++++------------------------------
> + 1 file changed, 50 insertions(+), 98 deletions(-)
> +
> +diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c
> +index 44ecb77b74..f6d4df6ad5 100644
> +--- a/board/xilinx/zynq/board.c
> ++++ b/board/xilinx/zynq/board.c
> +@@ -124,121 +124,73 @@ int zynq_board_read_rom_ethaddr(unsigned char *ethaddr)
> + }
> +
> + #if !defined(CONFIG_SYS_SDRAM_BASE) && !defined(CONFIG_SYS_SDRAM_SIZE)
> +-/*
> +- * fdt_get_reg - Fill buffer by information from DT
> +- */
> +-static phys_size_t fdt_get_reg(const void *fdt, int nodeoffset, void *buf,
> +- const u32 *cell, int n)
> ++static const void *get_memory_reg_prop(const void *fdt, int *lenp)
> + {
> +- int i = 0, b, banks;
> +- int parent_offset = fdt_parent_offset(fdt, nodeoffset);
> +- int address_cells = fdt_address_cells(fdt, parent_offset);
> +- int size_cells = fdt_size_cells(fdt, parent_offset);
> +- char *p = buf;
> +- u64 val;
> +- u64 vals;
> +-
> +- debug("%s: addr_cells=%x, size_cell=%x, buf=%p, cell=%p\n",
> +- __func__, address_cells, size_cells, buf, cell);
> +-
> +- /* Check memory bank setup */
> +- banks = n % (address_cells + size_cells);
> +- if (banks)
> +- panic("Incorrect memory setup cells=%d, ac=%d, sc=%d\n",
> +- n, address_cells, size_cells);
> +-
> +- banks = n / (address_cells + size_cells);
> +-
> +- for (b = 0; b < banks; b++) {
> +- debug("%s: Bank #%d:\n", __func__, b);
> +- if (address_cells == 2) {
> +- val = cell[i + 1];
> +- val <<= 32;
> +- val |= cell[i];
> +- val = fdt64_to_cpu(val);
> +- debug("%s: addr64=%llx, ptr=%p, cell=%p\n",
> +- __func__, val, p, &cell[i]);
> +- *(phys_addr_t *)p = val;
> +- } else {
> +- debug("%s: addr32=%x, ptr=%p\n",
> +- __func__, fdt32_to_cpu(cell[i]), p);
> +- *(phys_addr_t *)p = fdt32_to_cpu(cell[i]);
> +- }
> +- p += sizeof(phys_addr_t);
> +- i += address_cells;
> +-
> +- debug("%s: pa=%p, i=%x, size=%zu\n", __func__, p, i,
> +- sizeof(phys_addr_t));
> +-
> +- if (size_cells == 2) {
> +- vals = cell[i + 1];
> +- vals <<= 32;
> +- vals |= cell[i];
> +- vals = fdt64_to_cpu(vals);
> +-
> +- debug("%s: size64=%llx, ptr=%p, cell=%p\n",
> +- __func__, vals, p, &cell[i]);
> +- *(phys_size_t *)p = vals;
> +- } else {
> +- debug("%s: size32=%x, ptr=%p\n",
> +- __func__, fdt32_to_cpu(cell[i]), p);
> +- *(phys_size_t *)p = fdt32_to_cpu(cell[i]);
> +- }
> +- p += sizeof(phys_size_t);
> +- i += size_cells;
> +-
> +- debug("%s: ps=%p, i=%x, size=%zu\n",
> +- __func__, p, i, sizeof(phys_size_t));
> +- }
> ++ int offset;
> +
> +- /* Return the first address size */
> +- return *(phys_size_t *)((char *)buf + sizeof(phys_addr_t));
> +-}
> ++ offset = fdt_path_offset(fdt, "/memory");
> ++ if (offset < 0)
> ++ return NULL;
> +
> +-#define FDT_REG_SIZE sizeof(u32)
> +-/* Temp location for sharing data for storing */
> +-/* Up to 64-bit address + 64-bit size */
> +-static u8 tmp[CONFIG_NR_DRAM_BANKS * 16];
> ++ return fdt_getprop(fdt, offset, "reg", lenp);
> ++}
> +
> + void dram_init_banksize(void)
> + {
> +- int bank;
> ++ const void *fdt = gd->fdt_blob;
> ++ const fdt32_t *val;
> ++ int ac, sc, cells, len, i;
> ++
> ++ val = get_memory_reg_prop(fdt, &len);
> ++ if (len < 0)
> ++ return;
> ++
> ++ ac = fdt_address_cells(fdt, 0);
> ++ sc = fdt_size_cells(fdt, 0);
> ++ if (ac < 1 || sc > 2 || sc < 1 || sc > 2) {
> ++ printf("invalid address/size cells\n");
> ++ return;
> ++ }
> +
> +- memcpy(&gd->bd->bi_dram[0], &tmp, sizeof(tmp));
> ++ cells = ac + sc;
> +
> +- for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
> +- debug("Bank #%d: start %llx\n", bank,
> +- (unsigned long long)gd->bd->bi_dram[bank].start);
> +- debug("Bank #%d: size %llx\n", bank,
> +- (unsigned long long)gd->bd->bi_dram[bank].size);
> ++ len /= sizeof(*val);
> ++
> ++ for (i = 0; i < CONFIG_NR_DRAM_BANKS && len >= cells;
> ++ i++, len -= cells) {
> ++ gd->bd->bi_dram[i].start = fdtdec_get_number(val, ac);
> ++ val += ac;
> ++ gd->bd->bi_dram[i].size = fdtdec_get_number(val, sc);
> ++ val += sc;
> ++
> ++ debug("DRAM bank %d: start = %08lx, size = %08lx\n",
> ++ i, (unsigned long)gd->bd->bi_dram[i].start,
> ++ (unsigned long)gd->bd->bi_dram[i].size);
> + }
> + }
> +
> + int dram_init(void)
> + {
> +- int node, len;
> +- const void *blob = gd->fdt_blob;
> +- const u32 *cell;
> +-
> +- memset(&tmp, 0, sizeof(tmp));
> +-
> +- /* find or create "/memory" node. */
> +- node = fdt_subnode_offset(blob, 0, "memory");
> +- if (node < 0) {
> +- printf("%s: Can't get memory node\n", __func__);
> +- return node;
> ++ const void *fdt = gd->fdt_blob;
> ++ const fdt32_t *val;
> ++ int ac, sc, len;
> ++
> ++ ac = fdt_address_cells(fdt, 0);
> ++ sc = fdt_size_cells(fdt, 0);
> ++ if (ac < 0 || sc < 1 || sc > 2) {
> ++ printf("invalid address/size cells\n");
> ++ return -EINVAL;
> + }
> +
> +- /* Get pointer to cells and lenght of it */
> +- cell = fdt_getprop(blob, node, "reg", &len);
> +- if (!cell) {
> +- printf("%s: Can't get reg property\n", __func__);
> +- return -1;
> +- }
> ++ val = get_memory_reg_prop(fdt, &len);
> ++ if (len / sizeof(*val) < ac + sc)
> ++ return -EINVAL;
> ++
> ++ val += ac;
> +
> +- gd->ram_size = fdt_get_reg(blob, node, &tmp, cell, len / FDT_REG_SIZE);
> ++ gd->ram_size = fdtdec_get_number(val, sc);
> +
> +- debug("%s: Initial DRAM size %llx\n", __func__, (u64)gd->ram_size);
> ++ debug("DRAM size = %08lx\n", (unsigned long)gd->ram_size);
> +
> + zynq_ddrc_init();
> +
> +--
> +2.10.2
> +
> diff --git a/recipes-bsp/u-boot/u-boot-xlnx_2016.07.bb b/recipes-bsp/u-boot/u-boot-xlnx_2016.07.bb
> index 61f1cb793b..6e393f1b81 100644
> --- a/recipes-bsp/u-boot/u-boot-xlnx_2016.07.bb
> +++ b/recipes-bsp/u-boot/u-boot-xlnx_2016.07.bb
> @@ -8,6 +8,7 @@ PV = "v2016.07-xilinx-${XILINX_RELEASE_VERSION}+git${SRCPV}"
>
> FILESEXTRAPATHS_prepend := "${THISDIR}/u-boot-xlnx:"
>
> +SRC_URI_append = " file://ARM-zynq-Replace-dram_init-functions-with-board_init.patch"
> SRC_URI_append_kc705-microblazeel = " file://microblaze-kc705-Convert-microblaze-generic-to-k.patch"
>
> LICENSE = "GPLv2+"
> --
> 2.10.2
>
More information about the meta-xilinx
mailing list