[U-Boot] [RFC PATCH] ARM: zynq: Replace dram_init* functions with board_init_f safe ones
Nathan Rossi
nathan at nathanrossi.com
Sun Dec 4 10:33:22 CET 2016
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.
Using the zynq_zybo configuration, U-Boot would generate the following
error during image load. This was caused due to dram_init overwriting
the relocations for the "image" variable within the do_bootm function.
Out of coincidence the un-initialized memory has a compression type
which is the same as the value for the relocation type R_ARM_RELATIVE.
Uncompressing Invalid Image ... Unimplemented compression type 23
It should be noted that this is just one way the issue could surface,
other cases my not be observed in normal boot flow.
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")
Cc: Michal Simek <monstr at monstr.eu>
---
I am sending this as an RFC first to query whether it would make sense
to have the setup of memory info based on FDT information as a common
function that could be re-used or maybe always used (if OF enabled and
SDRAM_SIZE not used) from board_init_f. And in that regard get some
feedback on the best way to implement such.
This change would also need to be applied to the zynqmp dram_init*
functions.
---
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 2c86940957..c5e7b99d97 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
More information about the U-Boot
mailing list