
On 12 December 2016 at 06:27, Simon Glass sjg@chromium.org wrote:
Hi Nathan,
On 11 December 2016 at 08:58, Nathan Rossi nathan@nathanrossi.com wrote:
Add two functions for use by board implementations to decode the memory banks of the /memory node so as to populate the global data with ram_size and board info for memory banks.
The fdtdec_setup_memory_size() function decodes the first memory bank and sets up the gd->ram_size with the size of the memory bank. This function should be called from the boards dram_init().
The fdtdec_setup_memory_banksize() function decode the memory banks (up to the CONFIG_NR_DRAM_BANKS) and populates the base address and size into the gd->bd->bi_dram array of banks. This function should be called from the boards dram_init_banksize().
Signed-off-by: Nathan Rossi nathan@nathanrossi.com Cc: Simon Glass sjg@chromium.org Cc: Michal Simek monstr@monstr.eu
This implementation of decoding has been tested on zynq and zynqmp boards with address/size cells of (1, 1), (1, 2), (2, 1), (2, 2) and up to 2 memory banks.
include/fdtdec.h | 25 +++++++++++++++++++++++++ lib/fdtdec.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
Please see nit below.
diff --git a/include/fdtdec.h b/include/fdtdec.h index 27887c8c21..59a204b571 100644 --- a/include/fdtdec.h +++ b/include/fdtdec.h @@ -976,6 +976,31 @@ struct display_timing { */ int fdtdec_decode_display_timing(const void *blob, int node, int index, struct display_timing *config);
+/**
- fdtdec_setup_memory_size() - decode and setup gd->ram_size
- Decode the /memory 'reg' property to determine the size of the first memory
- bank, populate the global data with the size of the first bank of memory.
- This function should be called from the boards dram_init().
- @return 0 if OK, -EINVAL if the /memory node or reg property is missing or
- invalid
- */
+int fdtdec_setup_memory_size(void);
+/**
- fdtdec_setup_memory_banksize() - decode and populate gd->bd->bi_dram
- Decode the /memory 'reg' property to determine the address and size of the
- memory banks. Use this data to populate the global data board info with the
- phys address and size of memory banks. This function should be called from
- the boards dram_init_banksize().
- @return 0 if OK, negative on error
Good to be specific, if e.g. it can only return -EINVAL.
I will update this based on the change below.
- */
+int fdtdec_setup_memory_banksize(void);
/**
- Set up the device tree ready for use
*/ diff --git a/lib/fdtdec.c b/lib/fdtdec.c index 4e619c49a2..bc3be017b6 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1174,6 +1174,60 @@ int fdtdec_decode_display_timing(const void *blob, int parent, int index, return ret; }
+int fdtdec_setup_memory_size(void) +{
int ret, mem;
struct fdt_resource res;
mem = fdt_path_offset(gd->fdt_blob, "/memory");
if (mem < 0) {
debug("%s: Missing /memory node\n", __func__);
return -EINVAL;
}
ret = fdt_get_resource(gd->fdt_blob, mem, "reg", 0, &res);
if (ret != 0) {
debug("%s: Unable to decode first memory bank\n", __func__);
return -EINVAL;
}
gd->ram_size = (phys_size_t)(res.end - res.start + 1);
debug("%s: Initial DRAM size %llx\n", __func__, (u64)gd->ram_size);
return 0;
+}
+int fdtdec_setup_memory_banksize(void) +{
int bank, ret, mem;
struct fdt_resource res;
mem = fdt_path_offset(gd->fdt_blob, "/memory");
if (mem < 0) {
debug("%s: Missing /memory node\n", __func__);
return -EINVAL;
}
for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
ret = fdt_get_resource(gd->fdt_blob, mem, "reg", bank, &res);
if (ret == -FDT_ERR_NOTFOUND)
break;
if (ret != 0)
return ret;
The return above return -EINVAL, but this one returns a -FDT_ERR_... which is different. So my suggestion here would be to return -EINVAL here, unless you want to change the function to always return an FDT error (although fdtdec_decode_memory_region() returns an errno error so perhaps it is better to be consistent with that).
Agreed, returning only -EINVAL in both error conditions makes more sense than returning an -FDT_ERR_* error. This also makes it consistent with the function above this function. Will send out v2.
Regards, Nathan