fdt: fix fdtdec_setup_memory_banksize()

PATCH New Jens Wiklander 2 participants 7 messages Jul 13, 2018 – Jul 27, 2018 Library code

Share & tools

Jens Wiklander

author

2018-07-13 10:12 UTC #0 lore raw

PATCH 1/1 fdt: fix fdtdec_setup_memory_banksize()

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 <[email protected]>

---
 lib/fdtdec.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

lib/fdtdec.c +15 -5

index f4e8dbf699a8..14005d71507c 100644
@@ -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;

Signature · 1 line

2.17.1

Simon Glass

2018-07-16 05:20 UTC #1 lore raw

PATCH 1/1 fdt: fix fdtdec_setup_memory_banksize()

Quoted 8 lines · 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 <[email protected]>
---
 lib/fdtdec.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

Reviewed-by Simon Glass <[email protected]>

But can you convert this to livetree at the same time? E.g. use ofnode functions.

Regards,
Simon

Jens Wiklander

author

2018-07-17 15:42 UTC #2 lore raw

PATCH 1/1 fdt: fix fdtdec_setup_memory_banksize()

Quoted 10 lines · Simon Glass

Quoted 8 lines · 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 <[email protected]>
---
 lib/fdtdec.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

Quoted 4 lines

Reviewed-by Simon Glass <[email protected]>
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

Simon Glass

2018-07-19 01:32 UTC #3 lore raw

PATCH 1/1 fdt: fix fdtdec_setup_memory_banksize()

Hi Jens,

Quoted 16 lines · Jens Wiklander

Quoted 10 lines · Simon Glass
Quoted 8 lines · 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 <[email protected]>
---
 lib/fdtdec.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)
Quoted 4 lines
Reviewed-by Simon Glass <[email protected]>
But can you convert this to livetree at the same time? E.g. use ofnode functions.

Quoted 8 lines

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

Jens Wiklander

author

2018-07-19 15:49 UTC #4 lore raw

PATCH 1/1 fdt: fix fdtdec_setup_memory_banksize()

Quoted 26 lines · Simon Glass

Hi Jens,
Quoted 23 lines · Jens Wiklander
Quoted 10 lines · Simon Glass
Quoted 8 lines · 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 <[email protected]>
---
 lib/fdtdec.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)
Quoted 4 lines
Reviewed-by Simon Glass <[email protected]>
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())?

Quoted 5 lines

Quoted 1 line
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

Simon Glass

2018-07-20 02:17 UTC #5 lore raw

PATCH 1/1 fdt: fix fdtdec_setup_memory_banksize()

Hi Jens,

Quoted 27 lines · Jens Wiklander

Quoted 26 lines · Simon Glass
Hi Jens,
Quoted 23 lines · Jens Wiklander
Quoted 10 lines · Simon Glass
Quoted 8 lines · 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 <[email protected]>
---
 lib/fdtdec.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)
Quoted 4 lines
Reviewed-by Simon Glass <[email protected]>
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())?

Quoted 8 lines

Quoted 5 lines
Quoted 1 line
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

Simon Glass

2018-07-27 00:36 UTC #6 lore raw

PATCH 1/1 fdt: fix fdtdec_setup_memory_banksize()

Quoted 32 lines · Simon Glass

Hi Jens,
Quoted 29 lines · Jens Wiklander
Quoted 28 lines · Simon Glass
Hi Jens,
Quoted 24 lines · Jens Wiklander
Quoted 10 lines · Simon Glass
Quoted 8 lines · 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 <[email protected]>
---
 lib/fdtdec.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)
Quoted 4 lines
Reviewed-by Simon Glass <[email protected]>
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.

Quoted 8 lines

Quoted 6 lines
Quoted 3 lines
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.