| Message ID | 20200213184800.13968-1-patrick.delaunay@st.com |
|---|---|
| State | Superseded |
| Delegated to: | Simon Glass |
| Headers | show |
| Series | [v2] dm: core: Move "/chosen" and "/firmware" node scan | expand |
čt 13. 2. 2020 v 19:48 odesílatel Patrick Delaunay <patrick.delaunay@st.com> napsal: > > Use the new function dm_scan_fdt_ofnode_path() to scan all the nodes > which aren't devices themselves but may contain some: > - "/chosen" > - "/clocks" > - "/firmware" > > The patch removes the strcmp call in recursive function dm_scan_fdt_live() > and also corrects a conflict with the 2 applied patches in > the commit 1712ca21924b ("dm: core: Scan /firmware node by default") > and in the commit 747558d01457 ("dm: fdt: scan for devices under > /firmware too"): the subnodes of "/firmware" (optee for example) > are bound 2 times. > > For example the dm tree command result on STM32MP1 is: > > STM32MP> dm tree > Class Index Probed Driver Name > ----------------------------------------------------------- > root 0 [ + ] root_driver root_driver > firmware 0 [ ] psci |-- psci > sysreset 0 [ ] psci-sysreset | `-- psci-sysreset > simple_bus 0 [ + ] generic_simple_bus |-- soc > ... > tee 0 [ + ] optee |-- optee > ... > tee 1 [ ] optee `-- optee > > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> > --- > > Changes in v2: > - update commit message (Serie-cc => Series-cc) > > drivers/core/root.c | 52 +++++++++++++++------------------------------ > 1 file changed, 17 insertions(+), 35 deletions(-) > > diff --git a/drivers/core/root.c b/drivers/core/root.c > index e85643819e..14df16c280 100644 > --- a/drivers/core/root.c > +++ b/drivers/core/root.c > @@ -203,15 +203,6 @@ static int dm_scan_fdt_live(struct udevice *parent, > int ret = 0, err; > > for (np = node_parent->child; np; np = np->sibling) { > - /* "chosen" node isn't a device itself but may contain some: */ > - if (!strcmp(np->name, "chosen")) { > - pr_debug("parsing subnodes of \"chosen\"\n"); > - > - err = dm_scan_fdt_live(parent, np, pre_reloc_only); > - if (err && !ret) > - ret = err; > - continue; > - } > > if (!of_device_is_available(np)) { > pr_debug(" - ignoring disabled device\n"); > @@ -256,21 +247,6 @@ static int dm_scan_fdt_node(struct udevice *parent, const void *blob, > offset = fdt_next_subnode(blob, offset)) { > const char *node_name = fdt_get_name(blob, offset, NULL); > > - /* > - * The "chosen" and "firmware" nodes aren't devices > - * themselves but may contain some: > - */ > - if (!strcmp(node_name, "chosen") || > - !strcmp(node_name, "firmware")) { > - pr_debug("parsing subnodes of \"%s\"\n", node_name); > - > - err = dm_scan_fdt_node(parent, blob, offset, > - pre_reloc_only); > - if (err && !ret) > - ret = err; > - continue; > - } > - > if (!fdtdec_get_is_enabled(blob, offset)) { > pr_debug(" - ignoring disabled device\n"); > continue; > @@ -315,7 +291,8 @@ int dm_scan_fdt(const void *blob, bool pre_reloc_only) > return dm_scan_fdt_node(gd->dm_root, blob, 0, pre_reloc_only); > } > > -static int dm_scan_fdt_ofnode_path(const char *path, bool pre_reloc_only) > +static int dm_scan_fdt_ofnode_path(const void *blob, const char *path, > + bool pre_reloc_only) > { > ofnode node; > > @@ -327,13 +304,18 @@ static int dm_scan_fdt_ofnode_path(const char *path, bool pre_reloc_only) > if (of_live_active()) > return dm_scan_fdt_live(gd->dm_root, node.np, pre_reloc_only); > #endif > - return dm_scan_fdt_node(gd->dm_root, gd->fdt_blob, node.of_offset, > + return dm_scan_fdt_node(gd->dm_root, blob, node.of_offset, > pre_reloc_only); > } > > int dm_extended_scan_fdt(const void *blob, bool pre_reloc_only) > { > - int ret; > + int ret, i; > + const char * const nodes[] = { > + "/chosen", > + "/clocks", > + "/firmware" > + }; > > ret = dm_scan_fdt(blob, pre_reloc_only); > if (ret) { > @@ -341,16 +323,16 @@ int dm_extended_scan_fdt(const void *blob, bool pre_reloc_only) > return ret; > } > > - ret = dm_scan_fdt_ofnode_path("/clocks", pre_reloc_only); > - if (ret) { > - debug("scan for /clocks failed: %d\n", ret); > - return ret; > + /* Some nodes aren't devices themselves but may contain some */ > + for (i = 0; i < ARRAY_SIZE(nodes); i++) { > + ret = dm_scan_fdt_ofnode_path(blob, nodes[i], pre_reloc_only); > + if (ret) { > + debug("dm_scan_fdt() scan for %s failed: %d\n", > + nodes[i], ret); > + return ret; > + } > } > > - ret = dm_scan_fdt_ofnode_path("/firmware", pre_reloc_only); > - if (ret) > - debug("scan for /firmware failed: %d\n", ret); > - > return ret; > } > #endif > -- > 2.17.1 > It looks good to me. I have reported here. https://lists.denx.de/pipermail/u-boot/2020-January/395427.html Tested-by: Michal Simek <michal.simek@xilinx.com> Thanks, Michal
Hi Patrick, On Thu, 13 Feb 2020 at 11:48, Patrick Delaunay <patrick.delaunay@st.com> wrote: > > Use the new function dm_scan_fdt_ofnode_path() to scan all the nodes > which aren't devices themselves but may contain some: > - "/chosen" > - "/clocks" > - "/firmware" > > The patch removes the strcmp call in recursive function dm_scan_fdt_live() > and also corrects a conflict with the 2 applied patches in > the commit 1712ca21924b ("dm: core: Scan /firmware node by default") > and in the commit 747558d01457 ("dm: fdt: scan for devices under > /firmware too"): the subnodes of "/firmware" (optee for example) > are bound 2 times. > > For example the dm tree command result on STM32MP1 is: > > STM32MP> dm tree > Class Index Probed Driver Name > ----------------------------------------------------------- > root 0 [ + ] root_driver root_driver > firmware 0 [ ] psci |-- psci > sysreset 0 [ ] psci-sysreset | `-- psci-sysreset > simple_bus 0 [ + ] generic_simple_bus |-- soc > ... > tee 0 [ + ] optee |-- optee > ... > tee 1 [ ] optee `-- optee > > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> > --- > > Changes in v2: > - update commit message (Serie-cc => Series-cc) > > drivers/core/root.c | 52 +++++++++++++++------------------------------ > 1 file changed, 17 insertions(+), 35 deletions(-) This looks good to me, but please can you address the test failure (make qcheck)? Regards, Simon
Hi, > From: Simon Glass <sjg@chromium.org> > Sent: lundi 17 février 2020 04:56 > > Hi Patrick, > > On Thu, 13 Feb 2020 at 11:48, Patrick Delaunay <patrick.delaunay@st.com> > wrote: > > > > Use the new function dm_scan_fdt_ofnode_path() to scan all the nodes > > which aren't devices themselves but may contain some: > > - "/chosen" > > - "/clocks" > > - "/firmware" > > > > The patch removes the strcmp call in recursive function > > dm_scan_fdt_live() and also corrects a conflict with the 2 applied > > patches in the commit 1712ca21924b ("dm: core: Scan /firmware node by > > default") and in the commit 747558d01457 ("dm: fdt: scan for devices > > under /firmware too"): the subnodes of "/firmware" (optee for example) > > are bound 2 times. > > > > For example the dm tree command result on STM32MP1 is: > > > > STM32MP> dm tree > > Class Index Probed Driver Name > > ----------------------------------------------------------- > > root 0 [ + ] root_driver root_driver > > firmware 0 [ ] psci |-- psci > > sysreset 0 [ ] psci-sysreset | `-- psci-sysreset > > simple_bus 0 [ + ] generic_simple_bus |-- soc > > ... > > tee 0 [ + ] optee |-- optee > > ... > > tee 1 [ ] optee `-- optee > > > > Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> > > --- > > > > Changes in v2: > > - update commit message (Serie-cc => Series-cc) > > > > drivers/core/root.c | 52 > > +++++++++++++++------------------------------ > > 1 file changed, 17 insertions(+), 35 deletions(-) > > > This looks good to me, but please can you address the test failure (make > qcheck)? I forget to execute it, sorry. Work in progress.... > Regards, > Simon Patrick
diff --git a/drivers/core/root.c b/drivers/core/root.c index e85643819e..14df16c280 100644 --- a/drivers/core/root.c +++ b/drivers/core/root.c @@ -203,15 +203,6 @@ static int dm_scan_fdt_live(struct udevice *parent, int ret = 0, err; for (np = node_parent->child; np; np = np->sibling) { - /* "chosen" node isn't a device itself but may contain some: */ - if (!strcmp(np->name, "chosen")) { - pr_debug("parsing subnodes of \"chosen\"\n"); - - err = dm_scan_fdt_live(parent, np, pre_reloc_only); - if (err && !ret) - ret = err; - continue; - } if (!of_device_is_available(np)) { pr_debug(" - ignoring disabled device\n"); @@ -256,21 +247,6 @@ static int dm_scan_fdt_node(struct udevice *parent, const void *blob, offset = fdt_next_subnode(blob, offset)) { const char *node_name = fdt_get_name(blob, offset, NULL); - /* - * The "chosen" and "firmware" nodes aren't devices - * themselves but may contain some: - */ - if (!strcmp(node_name, "chosen") || - !strcmp(node_name, "firmware")) { - pr_debug("parsing subnodes of \"%s\"\n", node_name); - - err = dm_scan_fdt_node(parent, blob, offset, - pre_reloc_only); - if (err && !ret) - ret = err; - continue; - } - if (!fdtdec_get_is_enabled(blob, offset)) { pr_debug(" - ignoring disabled device\n"); continue; @@ -315,7 +291,8 @@ int dm_scan_fdt(const void *blob, bool pre_reloc_only) return dm_scan_fdt_node(gd->dm_root, blob, 0, pre_reloc_only); } -static int dm_scan_fdt_ofnode_path(const char *path, bool pre_reloc_only) +static int dm_scan_fdt_ofnode_path(const void *blob, const char *path, + bool pre_reloc_only) { ofnode node; @@ -327,13 +304,18 @@ static int dm_scan_fdt_ofnode_path(const char *path, bool pre_reloc_only) if (of_live_active()) return dm_scan_fdt_live(gd->dm_root, node.np, pre_reloc_only); #endif - return dm_scan_fdt_node(gd->dm_root, gd->fdt_blob, node.of_offset, + return dm_scan_fdt_node(gd->dm_root, blob, node.of_offset, pre_reloc_only); } int dm_extended_scan_fdt(const void *blob, bool pre_reloc_only) { - int ret; + int ret, i; + const char * const nodes[] = { + "/chosen", + "/clocks", + "/firmware" + }; ret = dm_scan_fdt(blob, pre_reloc_only); if (ret) { @@ -341,16 +323,16 @@ int dm_extended_scan_fdt(const void *blob, bool pre_reloc_only) return ret; } - ret = dm_scan_fdt_ofnode_path("/clocks", pre_reloc_only); - if (ret) { - debug("scan for /clocks failed: %d\n", ret); - return ret; + /* Some nodes aren't devices themselves but may contain some */ + for (i = 0; i < ARRAY_SIZE(nodes); i++) { + ret = dm_scan_fdt_ofnode_path(blob, nodes[i], pre_reloc_only); + if (ret) { + debug("dm_scan_fdt() scan for %s failed: %d\n", + nodes[i], ret); + return ret; + } } - ret = dm_scan_fdt_ofnode_path("/firmware", pre_reloc_only); - if (ret) - debug("scan for /firmware failed: %d\n", ret); - return ret; } #endif
Use the new function dm_scan_fdt_ofnode_path() to scan all the nodes which aren't devices themselves but may contain some: - "/chosen" - "/clocks" - "/firmware" The patch removes the strcmp call in recursive function dm_scan_fdt_live() and also corrects a conflict with the 2 applied patches in the commit 1712ca21924b ("dm: core: Scan /firmware node by default") and in the commit 747558d01457 ("dm: fdt: scan for devices under /firmware too"): the subnodes of "/firmware" (optee for example) are bound 2 times. For example the dm tree command result on STM32MP1 is: STM32MP> dm tree Class Index Probed Driver Name ----------------------------------------------------------- root 0 [ + ] root_driver root_driver firmware 0 [ ] psci |-- psci sysreset 0 [ ] psci-sysreset | `-- psci-sysreset simple_bus 0 [ + ] generic_simple_bus |-- soc ... tee 0 [ + ] optee |-- optee ... tee 1 [ ] optee `-- optee Signed-off-by: Patrick Delaunay <patrick.delaunay@st.com> --- Changes in v2: - update commit message (Serie-cc => Series-cc) drivers/core/root.c | 52 +++++++++++++++------------------------------ 1 file changed, 17 insertions(+), 35 deletions(-)