[U-Boot] [PATCH v3 3/3] ARM64: zynqmp: Replace board specific with generic memory bank decoding

[Michal Simek]

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 at nathanrossi.com>
> Fixes: 8d59d7f63b ("ARM64: zynqmp: Read RAM information from DT")
> Cc: Michal Simek <michal.simek at 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

-- 
Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91
w: www.monstr.eu p: +42-0-721842854
Maintainer of Linux kernel - Xilinx Microblaze
Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP ARM64 SoCs
U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP SoCs


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20161220/86b63a1a/attachment.sig>