| Message ID | 20161211135824.6191-1-nathan@nathanrossi.com |
|---|---|
| State | Superseded |
| Delegated to: | Michal Simek |
| Headers | show |
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. > + */ > +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). > + > + gd->bd->bi_dram[bank].start = (phys_addr_t)res.start; > + gd->bd->bi_dram[bank].size = > + (phys_size_t)(res.end - res.start + 1); > + > + debug("%s: DRAM Bank #%d: start = 0x%llx, size = 0x%llx\n", > + __func__, bank, > + (unsigned long long)gd->bd->bi_dram[bank].start, > + (unsigned long long)gd->bd->bi_dram[bank].size); > + } > + > + return 0; > +} > + > int fdtdec_setup(void) > { > #if CONFIG_IS_ENABLED(OF_CONTROL) > -- > 2.10.2 > Regards, Simon
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
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 + */ +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; + + gd->bd->bi_dram[bank].start = (phys_addr_t)res.start; + gd->bd->bi_dram[bank].size = + (phys_size_t)(res.end - res.start + 1); + + debug("%s: DRAM Bank #%d: start = 0x%llx, size = 0x%llx\n", + __func__, bank, + (unsigned long long)gd->bd->bi_dram[bank].start, + (unsigned long long)gd->bd->bi_dram[bank].size); + } + + return 0; +} + int fdtdec_setup(void) { #if CONFIG_IS_ENABLED(OF_CONTROL)
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(+)