All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] fdt_support: call mtdparts_init() after finding MTD node to fix up
@ 2020-07-15 10:56 Masahiro Yamada
  2020-07-15 10:56 ` [PATCH 2/2] fdt_support: skip MTD node with "disabled" in fdt_fixup_mtdparts() Masahiro Yamada
  2020-07-16 15:43 ` [PATCH 1/2] fdt_support: call mtdparts_init() after finding MTD node to fix up Simon Glass
  0 siblings, 2 replies; 6+ messages in thread
From: Masahiro Yamada @ 2020-07-15 10:56 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] fdt_support: skip MTD node with "disabled" in fdt_fixup_mtdparts()
  2020-07-15 10:56 [PATCH 1/2] fdt_support: call mtdparts_init() after finding MTD node to fix up Masahiro Yamada
@ 2020-07-15 10:56 ` Masahiro Yamada
  2020-07-16 15:43   ` Simon Glass
  2020-07-16 15:43 ` [PATCH 1/2] fdt_support: call mtdparts_init() after finding MTD node to fix up Simon Glass
  1 sibling, 1 reply; 6+ messages in thread
From: Masahiro Yamada @ 2020-07-15 10:56 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 1/2] fdt_support: call mtdparts_init() after finding MTD node to fix up
  2020-07-15 10:56 [PATCH 1/2] fdt_support: call mtdparts_init() after finding MTD node to fix up Masahiro Yamada
  2020-07-15 10:56 ` [PATCH 2/2] fdt_support: skip MTD node with "disabled" in fdt_fixup_mtdparts() Masahiro Yamada
@ 2020-07-16 15:43 ` Simon Glass
  1 sibling, 0 replies; 6+ messages in thread
From: Simon Glass @ 2020-07-16 15:43 UTC (permalink / raw)
  To: u-boot

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
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] fdt_support: skip MTD node with "disabled" in fdt_fixup_mtdparts()
  2020-07-15 10:56 ` [PATCH 2/2] fdt_support: skip MTD node with "disabled" in fdt_fixup_mtdparts() Masahiro Yamada
@ 2020-07-16 15:43   ` Simon Glass
  2020-07-17  1:35     ` Masahiro Yamada
  0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2020-07-16 15:43 UTC (permalink / raw)
  To: u-boot

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...

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] fdt_support: skip MTD node with "disabled" in fdt_fixup_mtdparts()
  2020-07-16 15:43   ` Simon Glass
@ 2020-07-17  1:35     ` Masahiro Yamada
  2020-07-26 14:54       ` Simon Glass
  0 siblings, 1 reply; 6+ messages in thread
From: Masahiro Yamada @ 2020-07-17  1:35 UTC (permalink / raw)
  To: u-boot

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.

-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] fdt_support: skip MTD node with "disabled" in fdt_fixup_mtdparts()
  2020-07-17  1:35     ` Masahiro Yamada
@ 2020-07-26 14:54       ` Simon Glass
  0 siblings, 0 replies; 6+ messages in thread
From: Simon Glass @ 2020-07-26 14:54 UTC (permalink / raw)
  To: u-boot

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-07-26 14:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-15 10:56 [PATCH 1/2] fdt_support: call mtdparts_init() after finding MTD node to fix up Masahiro Yamada
2020-07-15 10:56 ` [PATCH 2/2] fdt_support: skip MTD node with "disabled" in fdt_fixup_mtdparts() Masahiro Yamada
2020-07-16 15:43   ` Simon Glass
2020-07-17  1:35     ` Masahiro Yamada
2020-07-26 14:54       ` Simon Glass
2020-07-16 15:43 ` [PATCH 1/2] fdt_support: call mtdparts_init() after finding MTD node to fix up Simon Glass

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.