[PATCH 1/2] fdt_support: call mtdparts_init() after finding MTD node to fix up

Platform code can call fdt_fixup_mtdparts() in order to hand U-Boot's MTD partitions over to the Linux device tree.
Currently, fdt_fixup_mtdparts() calls mtdparts_init() in its entry. If no target MTD device is found, an error message like follows is displayed:
Device nand0 not found!
This occurs when the same code (e.g. arch/arm/mach-uniphier/fdt-fixup.c) is shared among several boards, but not all boards support an MTD device.
Parse the DT first, then call mtdparts_init() only when the target MTD node is found.
Yet, you still need to call mtdparts_init() before device_find().
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
common/fdt_support.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/common/fdt_support.c b/common/fdt_support.c index b010d0b552ad..717b2b6354c0 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -951,9 +951,7 @@ void fdt_fixup_mtdparts(void *blob, const struct node_info *node_info, struct mtd_device *dev; int i, idx; int noff; - - if (mtdparts_init() != 0) - return; + bool initialized = false;
for (i = 0; i < node_info_size; i++) { idx = 0; @@ -963,6 +961,13 @@ void fdt_fixup_mtdparts(void *blob, const struct node_info *node_info, debug("%s: %s, mtd dev type %d\n", fdt_get_name(blob, noff, 0), node_info[i].compat, node_info[i].type); + + if (!initialized) { + if (mtdparts_init() != 0) + return; + initialized = true; + } + dev = device_find(node_info[i].type, idx++); if (dev) { if (fdt_node_set_part_info(blob, noff, dev))

Currently, fdt_fixup_mtdparts() only checks the compatible property. It is pointless to fix up the disabled node.
Skip the node if it has the property:
status = "disabled"
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com ---
common/fdt_support.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/common/fdt_support.c b/common/fdt_support.c index 717b2b6354c0..9e7191e22f70 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -955,9 +955,16 @@ void fdt_fixup_mtdparts(void *blob, const struct node_info *node_info,
for (i = 0; i < node_info_size; i++) { idx = 0; - noff = fdt_node_offset_by_compatible(blob, -1, - node_info[i].compat); - while (noff != -FDT_ERR_NOTFOUND) { + noff = -1; + + while ((noff = fdt_node_offset_by_compatible(blob, noff, + node_info[i].compat)) >= 0) { + const char *prop; + + prop = fdt_getprop(blob, noff, "status", NULL); + if (prop && !strcmp(prop, "disabled")) + continue; + debug("%s: %s, mtd dev type %d\n", fdt_get_name(blob, noff, 0), node_info[i].compat, node_info[i].type); @@ -973,10 +980,6 @@ void fdt_fixup_mtdparts(void *blob, const struct node_info *node_info, if (fdt_node_set_part_info(blob, noff, dev)) return; /* return on error */ } - - /* Jump to next flash node */ - noff = fdt_node_offset_by_compatible(blob, noff, - node_info[i].compat); } } }

On Wed, 15 Jul 2020 at 04:57, Masahiro Yamada yamada.masahiro@socionext.com wrote:
Currently, fdt_fixup_mtdparts() only checks the compatible property. It is pointless to fix up the disabled node.
Skip the node if it has the property:
status = "disabled"
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
common/fdt_support.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Are there any tests for this code?
I am thinking we should migrate fdt_support to use livetree...

On Fri, Jul 17, 2020 at 12:44 AM Simon Glass sjg@chromium.org wrote:
On Wed, 15 Jul 2020 at 04:57, Masahiro Yamada yamada.masahiro@socionext.com wrote:
Currently, fdt_fixup_mtdparts() only checks the compatible property. It is pointless to fix up the disabled node.
Skip the node if it has the property:
status = "disabled"
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
common/fdt_support.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Are there any tests for this code?
No test code.
It makes more effort since testing this would need to probe an MTD device as well as parsing DT.
I am thinking we should migrate fdt_support to use livetree...
One important thing we should notice is we have two different DT instances:
[1] DT blob for U-Boot - used for U-Boot driver model [2] DT blob for Linux - passed when booting Linux
In my understanding, the livetree is supposed to unflatten [1].
fdt_fixup_mtdparts() is obviously fixing [2].
I do not know how livetree would work for this function.

Hi Masahiro,
On Thu, 16 Jul 2020 at 19:36, Masahiro Yamada masahiroy@kernel.org wrote:
On Fri, Jul 17, 2020 at 12:44 AM Simon Glass sjg@chromium.org wrote:
On Wed, 15 Jul 2020 at 04:57, Masahiro Yamada yamada.masahiro@socionext.com wrote:
Currently, fdt_fixup_mtdparts() only checks the compatible property. It is pointless to fix up the disabled node.
Skip the node if it has the property:
status = "disabled"
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
common/fdt_support.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
Are there any tests for this code?
No test code.
It makes more effort since testing this would need to probe an MTD device as well as parsing DT.
I am thinking we should migrate fdt_support to use livetree...
One important thing we should notice is we have two different DT instances:
[1] DT blob for U-Boot - used for U-Boot driver model [2] DT blob for Linux - passed when booting Linux
In my understanding, the livetree is supposed to unflatten [1].
fdt_fixup_mtdparts() is obviously fixing [2].
I do not know how livetree would work for this function.
We need an implementation of livetree for [2].
Regards, Simon

On Wed, 15 Jul 2020 at 04:57, Masahiro Yamada yamada.masahiro@socionext.com wrote:
Platform code can call fdt_fixup_mtdparts() in order to hand U-Boot's MTD partitions over to the Linux device tree.
Currently, fdt_fixup_mtdparts() calls mtdparts_init() in its entry. If no target MTD device is found, an error message like follows is displayed:
Device nand0 not found!
This occurs when the same code (e.g. arch/arm/mach-uniphier/fdt-fixup.c) is shared among several boards, but not all boards support an MTD device.
Parse the DT first, then call mtdparts_init() only when the target MTD node is found.
Yet, you still need to call mtdparts_init() before device_find().
Signed-off-by: Masahiro Yamada yamada.masahiro@socionext.com
common/fdt_support.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)
Reviewed-by: Simon Glass sjg@chromium.org
diff --git a/common/fdt_support.c b/common/fdt_support.c index b010d0b552ad..717b2b6354c0 100644 --- a/common/fdt_support.c +++ b/common/fdt_support.c @@ -951,9 +951,7 @@ void fdt_fixup_mtdparts(void *blob, const struct node_info *node_info, struct mtd_device *dev; int i, idx; int noff;
if (mtdparts_init() != 0)
return;
bool initialized = false;
How about inited as it is shorter?
for (i = 0; i < node_info_size; i++) { idx = 0;
@@ -963,6 +961,13 @@ void fdt_fixup_mtdparts(void *blob, const struct node_info *node_info, debug("%s: %s, mtd dev type %d\n", fdt_get_name(blob, noff, 0), node_info[i].compat, node_info[i].type);
if (!initialized) {
if (mtdparts_init() != 0)
return;
initialized = true;
}
dev = device_find(node_info[i].type, idx++); if (dev) { if (fdt_node_set_part_info(blob, noff, dev))
-- 2.17.1
participants (3)
-
Masahiro Yamada
-
Masahiro Yamada
-
Simon Glass