| Message ID | 20180713101211.26028-1-jens.wiklander@linaro.org |
|---|---|
| State | Accepted |
| Commit | 452bc121027df6bc5ad59e25db8c6b0a4ecbe8a4 |
| Headers | show |
| Series | fdt: fix fdtdec_setup_memory_banksize() | expand |
On 13 July 2018 at 04:12, Jens Wiklander <jens.wiklander@linaro.org> wrote: > Prior to this patch is fdtdec_setup_memory_banksize() incorrectly > ignoring the "status" field. This patch fixes that by testing the status > with fdtdec_get_is_enabled() before using a memory node. > > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org> > --- > lib/fdtdec.c | 20 +++++++++++++++----- > 1 file changed, 15 insertions(+), 5 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org> But can you convert this to livetree at the same time? E.g. use ofnode functions. Regards, Simon
On Sun, Jul 15, 2018 at 11:20:39PM -0600, Simon Glass wrote: > On 13 July 2018 at 04:12, Jens Wiklander <jens.wiklander@linaro.org> wrote: > > Prior to this patch is fdtdec_setup_memory_banksize() incorrectly > > ignoring the "status" field. This patch fixes that by testing the status > > with fdtdec_get_is_enabled() before using a memory node. > > > > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org> > > --- > > lib/fdtdec.c | 20 +++++++++++++++----- > > 1 file changed, 15 insertions(+), 5 deletions(-) > > Reviewed-by: Simon Glass <sjg@chromium.org> > > But can you convert this to livetree at the same time? E.g. use ofnode > functions. I can try, but the ofnode concept is new to me. This patch is based on the fdt_node_offset_by_prop_value() function. I can't find any such ofnode function or any other suitable helper function. Perhaps I'm missing something, or should I add an ofnode_by_prop_value() function (similar to ofnode_by_compatible())? Please advice. Thanks, Jens
Hi Jens, On 17 July 2018 at 09:42, Jens Wiklander <jens.wiklander@linaro.org> wrote: > On Sun, Jul 15, 2018 at 11:20:39PM -0600, Simon Glass wrote: >> On 13 July 2018 at 04:12, Jens Wiklander <jens.wiklander@linaro.org> wrote: >> > Prior to this patch is fdtdec_setup_memory_banksize() incorrectly >> > ignoring the "status" field. This patch fixes that by testing the status >> > with fdtdec_get_is_enabled() before using a memory node. >> > >> > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org> >> > --- >> > lib/fdtdec.c | 20 +++++++++++++++----- >> > 1 file changed, 15 insertions(+), 5 deletions(-) >> >> Reviewed-by: Simon Glass <sjg@chromium.org> >> >> But can you convert this to livetree at the same time? E.g. use ofnode >> functions. > > I can try, but the ofnode concept is new to me. > > This patch is based on the fdt_node_offset_by_prop_value() function. I > can't find any such ofnode function or any other suitable helper > function. Perhaps I'm missing something, or should I add an > ofnode_by_prop_value() function (similar to ofnode_by_compatible())? > > Please advice. advise :-) Yes, you could add that function. Sorry it doesn't already exists. Regards, Simon
On Thu, Jul 19, 2018 at 3:32 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Jens, > > On 17 July 2018 at 09:42, Jens Wiklander <jens.wiklander@linaro.org> wrote: >> On Sun, Jul 15, 2018 at 11:20:39PM -0600, Simon Glass wrote: >>> On 13 July 2018 at 04:12, Jens Wiklander <jens.wiklander@linaro.org> wrote: >>> > Prior to this patch is fdtdec_setup_memory_banksize() incorrectly >>> > ignoring the "status" field. This patch fixes that by testing the status >>> > with fdtdec_get_is_enabled() before using a memory node. >>> > >>> > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org> >>> > --- >>> > lib/fdtdec.c | 20 +++++++++++++++----- >>> > 1 file changed, 15 insertions(+), 5 deletions(-) >>> >>> Reviewed-by: Simon Glass <sjg@chromium.org> >>> >>> But can you convert this to livetree at the same time? E.g. use ofnode >>> functions. >> >> I can try, but the ofnode concept is new to me. >> >> This patch is based on the fdt_node_offset_by_prop_value() function. I >> can't find any such ofnode function or any other suitable helper >> function. Perhaps I'm missing something, or should I add an >> ofnode_by_prop_value() function (similar to ofnode_by_compatible())? >> >> Please advice. > > advise :-) > > Yes, you could add that function. Sorry it doesn't already exists. I'll give it a shot after my vacation. In the meantime can we take this patch or would you rather wait for the livetree version? Thanks, Jens
Hi Jens, On 19 July 2018 at 09:49, Jens Wiklander <jens.wiklander@linaro.org> wrote: > On Thu, Jul 19, 2018 at 3:32 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Jens, >> >> On 17 July 2018 at 09:42, Jens Wiklander <jens.wiklander@linaro.org> wrote: >>> On Sun, Jul 15, 2018 at 11:20:39PM -0600, Simon Glass wrote: >>>> On 13 July 2018 at 04:12, Jens Wiklander <jens.wiklander@linaro.org> wrote: >>>> > Prior to this patch is fdtdec_setup_memory_banksize() incorrectly >>>> > ignoring the "status" field. This patch fixes that by testing the status >>>> > with fdtdec_get_is_enabled() before using a memory node. >>>> > >>>> > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org> >>>> > --- >>>> > lib/fdtdec.c | 20 +++++++++++++++----- >>>> > 1 file changed, 15 insertions(+), 5 deletions(-) >>>> >>>> Reviewed-by: Simon Glass <sjg@chromium.org> >>>> >>>> But can you convert this to livetree at the same time? E.g. use ofnode >>>> functions. >>> >>> I can try, but the ofnode concept is new to me. >>> >>> This patch is based on the fdt_node_offset_by_prop_value() function. I >>> can't find any such ofnode function or any other suitable helper >>> function. Perhaps I'm missing something, or should I add an >>> ofnode_by_prop_value() function (similar to ofnode_by_compatible())? >>> >>> Please advice. >> >> advise :-) >> >> Yes, you could add that function. Sorry it doesn't already exists. > > I'll give it a shot after my vacation. In the meantime can we take this patch or > would you rather wait for the livetree version? We can take it. Regards, Simon
On 19 July 2018 at 20:17, Simon Glass <sjg@chromium.org> wrote: > Hi Jens, > > On 19 July 2018 at 09:49, Jens Wiklander <jens.wiklander@linaro.org> wrote: >> On Thu, Jul 19, 2018 at 3:32 AM, Simon Glass <sjg@chromium.org> wrote: >>> Hi Jens, >>> >>> On 17 July 2018 at 09:42, Jens Wiklander <jens.wiklander@linaro.org> wrote: >>>> On Sun, Jul 15, 2018 at 11:20:39PM -0600, Simon Glass wrote: >>>>> On 13 July 2018 at 04:12, Jens Wiklander <jens.wiklander@linaro.org> wrote: >>>>> > Prior to this patch is fdtdec_setup_memory_banksize() incorrectly >>>>> > ignoring the "status" field. This patch fixes that by testing the status >>>>> > with fdtdec_get_is_enabled() before using a memory node. >>>>> > >>>>> > Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org> >>>>> > --- >>>>> > lib/fdtdec.c | 20 +++++++++++++++----- >>>>> > 1 file changed, 15 insertions(+), 5 deletions(-) >>>>> >>>>> Reviewed-by: Simon Glass <sjg@chromium.org> >>>>> >>>>> But can you convert this to livetree at the same time? E.g. use ofnode >>>>> functions. >>>> >>>> I can try, but the ofnode concept is new to me. >>>> >>>> This patch is based on the fdt_node_offset_by_prop_value() function. I >>>> can't find any such ofnode function or any other suitable helper >>>> function. Perhaps I'm missing something, or should I add an >>>> ofnode_by_prop_value() function (similar to ofnode_by_compatible())? >>>> >>>> Please advice. >>> >>> advise :-) >>> >>> Yes, you could add that function. Sorry it doesn't already exists. >> >> I'll give it a shot after my vacation. In the meantime can we take this patch or >> would you rather wait for the livetree version? > > We can take it. Applied to u-boot-dm, thanks! Please send the follow up when you can.
diff --git a/lib/fdtdec.c b/lib/fdtdec.c index f4e8dbf699a8..14005d71507c 100644 --- a/lib/fdtdec.c +++ b/lib/fdtdec.c @@ -1179,13 +1179,25 @@ int fdtdec_setup_memory_size(void) } #if defined(CONFIG_NR_DRAM_BANKS) + +static int get_next_memory_node(const void *blob, int startoffset) +{ + int mem = -1; + + do { + mem = fdt_node_offset_by_prop_value(gd->fdt_blob, mem, + "device_type", "memory", 7); + } while (!fdtdec_get_is_enabled(blob, mem)); + + return mem; +} + int fdtdec_setup_memory_banksize(void) { int bank, ret, mem, reg = 0; struct fdt_resource res; - mem = fdt_node_offset_by_prop_value(gd->fdt_blob, -1, "device_type", - "memory", 7); + mem = get_next_memory_node(gd->fdt_blob, -1); if (mem < 0) { debug("%s: Missing /memory node\n", __func__); return -EINVAL; @@ -1195,9 +1207,7 @@ int fdtdec_setup_memory_banksize(void) ret = fdt_get_resource(gd->fdt_blob, mem, "reg", reg++, &res); if (ret == -FDT_ERR_NOTFOUND) { reg = 0; - mem = fdt_node_offset_by_prop_value(gd->fdt_blob, mem, - "device_type", - "memory", 7); + mem = get_next_memory_node(gd->fdt_blob, mem); if (mem == -FDT_ERR_NOTFOUND) break;
Prior to this patch is fdtdec_setup_memory_banksize() incorrectly ignoring the "status" field. This patch fixes that by testing the status with fdtdec_get_is_enabled() before using a memory node. Signed-off-by: Jens Wiklander <jens.wiklander@linaro.org> --- lib/fdtdec.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) -- 2.17.1