| Message ID | 20161218140334.5260-3-nathan@nathanrossi.com |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
On 18.12.2016 15:03, Nathan Rossi wrote: > The dram_init and dram_init_banksize functions were using a board > specific implementation for decoding the memory banks from the fdt. This > board specific implementation uses a static variable 'tmp' which makes > these functions unsafe for execution from within the board_init_f > context. > > This change makes the dram_init* functions use a generic implementation > of decoding and populating memory bank and size data. > > Signed-off-by: Nathan Rossi <nathan@nathanrossi.com> > Fixes: 8d59d7f63b ("ARM64: zynqmp: Read RAM information from DT") > Cc: Michal Simek <michal.simek@xilinx.com> > --- > board/xilinx/zynqmp/zynqmp.c | 112 ++----------------------------------------- > 1 file changed, 3 insertions(+), 109 deletions(-) > > diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c > index cef1f6a13a..8a3d0043b9 100644 > --- a/board/xilinx/zynqmp/zynqmp.c > +++ b/board/xilinx/zynqmp/zynqmp.c > @@ -180,121 +180,15 @@ 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) > -{ > - 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)); > - } > - > - /* Return the first address size */ > - return *(phys_size_t *)((char *)buf + sizeof(phys_addr_t)); > -} > - > -#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]; > - > void dram_init_banksize(void) > { > - int bank; > - > - memcpy(&gd->bd->bi_dram[0], &tmp, sizeof(tmp)); > - > - 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); > - } > + fdtdec_setup_memory_banksize(); > } > > 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; > - } > - > - /* 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; > - } > - > - gd->ram_size = fdt_get_reg(blob, node, &tmp, cell, len / FDT_REG_SIZE); > - > - debug("%s: Initial DRAM size %llx\n", __func__, (u64)gd->ram_size); > + if (fdtdec_setup_memory_size() != 0) > + return -EINVAL; > > return 0; > } > I have tested it on zynq and zynqmp platforms and all looks good. https://travis-ci.org/michalsimek-test/u-boot/builds/185160761 Build for others look good too. Applied all to my tree Thanks, Michal
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c index cef1f6a13a..8a3d0043b9 100644 --- a/board/xilinx/zynqmp/zynqmp.c +++ b/board/xilinx/zynqmp/zynqmp.c @@ -180,121 +180,15 @@ 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) -{ - 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)); - } - - /* Return the first address size */ - return *(phys_size_t *)((char *)buf + sizeof(phys_addr_t)); -} - -#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]; - void dram_init_banksize(void) { - int bank; - - memcpy(&gd->bd->bi_dram[0], &tmp, sizeof(tmp)); - - 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); - } + fdtdec_setup_memory_banksize(); } 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; - } - - /* 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; - } - - gd->ram_size = fdt_get_reg(blob, node, &tmp, cell, len / FDT_REG_SIZE); - - debug("%s: Initial DRAM size %llx\n", __func__, (u64)gd->ram_size); + if (fdtdec_setup_memory_size() != 0) + return -EINVAL; return 0; }
The dram_init and dram_init_banksize functions were using a board specific implementation for decoding the memory banks from the fdt. This board specific implementation uses a static variable 'tmp' which makes these functions unsafe for execution from within the board_init_f context. This change makes the dram_init* functions use a generic implementation of decoding and populating memory bank and size data. Signed-off-by: Nathan Rossi <nathan@nathanrossi.com> Fixes: 8d59d7f63b ("ARM64: zynqmp: Read RAM information from DT") Cc: Michal Simek <michal.simek@xilinx.com> --- board/xilinx/zynqmp/zynqmp.c | 112 ++----------------------------------------- 1 file changed, 3 insertions(+), 109 deletions(-)