[U-Boot] [PATCH] fdt: fix fdtdec_setup_memory_banksize()

[U-Boot] [PATCH] fdt: fix fdtdec_setup_memory_banksize()

[Jens Wiklander]

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(-)

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;
 
-- 
2.17.1

[U-Boot] [PATCH] fdt: fix fdtdec_setup_memory_banksize()

[Simon Glass]

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

[U-Boot] [PATCH] fdt: fix fdtdec_setup_memory_banksize()

[Jens Wiklander]

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

[U-Boot] [PATCH] fdt: fix fdtdec_setup_memory_banksize()

[Simon Glass]

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

[U-Boot] [PATCH] fdt: fix fdtdec_setup_memory_banksize()

[Jens Wiklander]

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

[U-Boot] [PATCH] fdt: fix fdtdec_setup_memory_banksize()

[Simon Glass]

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

[U-Boot] [PATCH] fdt: fix fdtdec_setup_memory_banksize()

[Simon Glass]

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.