| Message ID | 20220427094502.456111-2-clement.leger@bootlin.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | add dynamic PCI device of_node creation for overlay | expand |
On Wed, Apr 27, 2022 at 11:45:00AM +0200, Clément Léger wrote: > When enabling CONFIG_OF on a platform where of_root is not populated by > firmware, we end up without a root node. In order to apply overlays and > create subnodes of the root node, we need one. This commit creates an > empty root node if not present. The existing unittest essentially does the same thing for running the tests on non-DT systems. It should be modified to use this support instead. Maybe that's just removing the unittest code that set of_root. I expect Frank will have some comments. > Co-developed-by: Rob Herring <robh@kernel.org> > Signed-off-by: Rob Herring <robh@kernel.org> > Signed-off-by: Clément Léger <clement.leger@bootlin.com> > --- > drivers/of/base.c | 16 ++++++++++++++-- > 1 file changed, 14 insertions(+), 2 deletions(-) > > diff --git a/drivers/of/base.c b/drivers/of/base.c > index e7d92b67cb8a..6b8584c39f73 100644 > --- a/drivers/of/base.c > +++ b/drivers/of/base.c > @@ -177,6 +177,19 @@ void __init of_core_init(void) > pr_err("failed to register existing nodes\n"); > return; > } > + > + if (!of_root) { > + of_root = kzalloc(sizeof(*of_root), GFP_KERNEL); > + if (!of_root) { > + mutex_unlock(&of_mutex); > + pr_err("failed to create root node\n"); > + return; > + } > + > + of_root->full_name = "/"; > + of_node_init(of_root); > + } > + > for_each_of_allnodes(np) { > __of_attach_node_sysfs(np); > if (np->phandle && !phandle_cache[of_phandle_cache_hash(np->phandle)]) > @@ -185,8 +198,7 @@ void __init of_core_init(void) > mutex_unlock(&of_mutex); > > /* Symlink in /proc as required by userspace ABI */ > - if (of_root) > - proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base"); > + proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base"); > } > > static struct property *__of_find_property(const struct device_node *np, > -- > 2.34.1 > >
Le Tue, 3 May 2022 08:45:11 -0500, Rob Herring <robh@kernel.org> a écrit : > On Wed, Apr 27, 2022 at 11:45:00AM +0200, Clément Léger wrote: > > When enabling CONFIG_OF on a platform where of_root is not populated by > > firmware, we end up without a root node. In order to apply overlays and > > create subnodes of the root node, we need one. This commit creates an > > empty root node if not present. > > The existing unittest essentially does the same thing for running the > tests on non-DT systems. It should be modified to use this support > instead. Maybe that's just removing the unittest code that set of_root. Acked, I'll try the unit test on my system. > > I expect Frank will have some comments. > > > Co-developed-by: Rob Herring <robh@kernel.org> > > Signed-off-by: Rob Herring <robh@kernel.org> > > Signed-off-by: Clément Léger <clement.leger@bootlin.com> > > --- > > drivers/of/base.c | 16 ++++++++++++++-- > > 1 file changed, 14 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/of/base.c b/drivers/of/base.c > > index e7d92b67cb8a..6b8584c39f73 100644 > > --- a/drivers/of/base.c > > +++ b/drivers/of/base.c > > @@ -177,6 +177,19 @@ void __init of_core_init(void) > > pr_err("failed to register existing nodes\n"); > > return; > > } > > + > > + if (!of_root) { > > + of_root = kzalloc(sizeof(*of_root), GFP_KERNEL); > > + if (!of_root) { > > + mutex_unlock(&of_mutex); > > + pr_err("failed to create root node\n"); > > + return; > > + } > > + > > + of_root->full_name = "/"; > > + of_node_init(of_root); > > + } > > + > > for_each_of_allnodes(np) { > > __of_attach_node_sysfs(np); > > if (np->phandle && !phandle_cache[of_phandle_cache_hash(np->phandle)]) > > @@ -185,8 +198,7 @@ void __init of_core_init(void) > > mutex_unlock(&of_mutex); > > > > /* Symlink in /proc as required by userspace ABI */ > > - if (of_root) > > - proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base"); > > + proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base"); > > } > > > > static struct property *__of_find_property(const struct device_node *np, > > -- > > 2.34.1 > > > >
On 5/3/22 08:45, Rob Herring wrote: > On Wed, Apr 27, 2022 at 11:45:00AM +0200, Clément Léger wrote: >> When enabling CONFIG_OF on a platform where of_root is not populated by >> firmware, we end up without a root node. In order to apply overlays and >> create subnodes of the root node, we need one. This commit creates an >> empty root node if not present. > > The existing unittest essentially does the same thing for running the > tests on non-DT systems. It should be modified to use this support > instead. Maybe that's just removing the unittest code that set of_root. > > I expect Frank will have some comments. > < snip > This patch series is next on my list, after what I am currently working on (updating the .dts -> .dtso patch). I may get to this today, but more likely it will be tomorrow. -Frank
On 5/3/22 08:45, Rob Herring wrote: > On Wed, Apr 27, 2022 at 11:45:00AM +0200, Clément Léger wrote: >> When enabling CONFIG_OF on a platform where of_root is not populated by >> firmware, we end up without a root node. In order to apply overlays and >> create subnodes of the root node, we need one. This commit creates an >> empty root node if not present. > > The existing unittest essentially does the same thing for running the > tests on non-DT systems. It should be modified to use this support > instead. Maybe that's just removing the unittest code that set of_root. > > I expect Frank will have some comments. My preference would be for unflatten_and_copy_device_tree() to use a compiled in FDT that only contains a root node, in the case that no valid device tree is found (in other words, "if (!initial_boot_params)". unflatten_and_copy_device_tree() calls unittest_unflatten_overlay_base() after unflattening the device tree passed into the booting kernel. This step is needed for a specific portion of the unittests. I'm still looking at the bigger picture of using overlays for the PCIe card, so more comments will be coming about that bigger picture. -Frank > >> Co-developed-by: Rob Herring <robh@kernel.org> >> Signed-off-by: Rob Herring <robh@kernel.org> >> Signed-off-by: Clément Léger <clement.leger@bootlin.com> >> --- >> drivers/of/base.c | 16 ++++++++++++++-- >> 1 file changed, 14 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/of/base.c b/drivers/of/base.c >> index e7d92b67cb8a..6b8584c39f73 100644 >> --- a/drivers/of/base.c >> +++ b/drivers/of/base.c >> @@ -177,6 +177,19 @@ void __init of_core_init(void) >> pr_err("failed to register existing nodes\n"); >> return; >> } >> + >> + if (!of_root) { >> + of_root = kzalloc(sizeof(*of_root), GFP_KERNEL); >> + if (!of_root) { >> + mutex_unlock(&of_mutex); >> + pr_err("failed to create root node\n"); >> + return; >> + } >> + >> + of_root->full_name = "/"; >> + of_node_init(of_root); >> + } >> + >> for_each_of_allnodes(np) { >> __of_attach_node_sysfs(np); >> if (np->phandle && !phandle_cache[of_phandle_cache_hash(np->phandle)]) >> @@ -185,8 +198,7 @@ void __init of_core_init(void) >> mutex_unlock(&of_mutex); >> >> /* Symlink in /proc as required by userspace ABI */ >> - if (of_root) >> - proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base"); >> + proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base"); >> } >> >> static struct property *__of_find_property(const struct device_node *np, >> -- >> 2.34.1 >> >>
Le Mon, 16 May 2022 23:11:03 -0400, Frank Rowand <frowand.list@gmail.com> a écrit : > On 5/3/22 08:45, Rob Herring wrote: > > On Wed, Apr 27, 2022 at 11:45:00AM +0200, Clément Léger wrote: > >> When enabling CONFIG_OF on a platform where of_root is not populated by > >> firmware, we end up without a root node. In order to apply overlays and > >> create subnodes of the root node, we need one. This commit creates an > >> empty root node if not present. > > > > The existing unittest essentially does the same thing for running the > > tests on non-DT systems. It should be modified to use this support > > instead. Maybe that's just removing the unittest code that set of_root. > > > > I expect Frank will have some comments. > > My preference would be for unflatten_and_copy_device_tree() to > use a compiled in FDT that only contains a root node, in the > case that no valid device tree is found (in other words, > "if (!initial_boot_params)". Ok, so basically, instead of creating the root node manually, you expect a device-tree which contains the following to be builtin the kernel and unflattened if needed: / { }; Maybe "chosen" and "aliases" nodes should also be provided as empty nodes since the unittest are creating them anyway and the core DT code also uses them. Thanks, Clément > > unflatten_and_copy_device_tree() calls unittest_unflatten_overlay_base() > after unflattening the device tree passed into the booting kernel. This > step is needed for a specific portion of the unittests. > > I'm still looking at the bigger picture of using overlays for the PCIe > card, so more comments will be coming about that bigger picture. > > -Frank >
On 5/17/22 02:37, Clément Léger wrote: > Le Mon, 16 May 2022 23:11:03 -0400, > Frank Rowand <frowand.list@gmail.com> a écrit : > >> On 5/3/22 08:45, Rob Herring wrote: >>> On Wed, Apr 27, 2022 at 11:45:00AM +0200, Clément Léger wrote: >>>> When enabling CONFIG_OF on a platform where of_root is not populated by >>>> firmware, we end up without a root node. In order to apply overlays and >>>> create subnodes of the root node, we need one. This commit creates an >>>> empty root node if not present. >>> >>> The existing unittest essentially does the same thing for running the >>> tests on non-DT systems. It should be modified to use this support >>> instead. Maybe that's just removing the unittest code that set of_root. >>> >>> I expect Frank will have some comments. >> >> My preference would be for unflatten_and_copy_device_tree() to >> use a compiled in FDT that only contains a root node, in the >> case that no valid device tree is found (in other words, >> "if (!initial_boot_params)". > > Ok, so basically, instead of creating the root node manually, you > expect a device-tree which contains the following to be builtin the > kernel and unflattened if needed: > > / { > > }; Yes. If you agree with this I can create a patch to implement it. I think it is useful even stand alone from the rest of the series. > > Maybe "chosen" and "aliases" nodes should also be provided as empty > nodes since the unittest are creating them anyway and the core DT code > also uses them. No. Unittest does not create both of them (I'm pretty sure, but I'm not going to double check). If I recall correctly, unittest adds a property in one of those two nodes, and thus implicitly creates the node if not already present. Unittest does populate internal pointers to those two nodes if the nodes are present (otherwise the pointers will have the value of null). There is no need for the nodes to be present if empty. -Frank > > Thanks, > > Clément > >> >> unflatten_and_copy_device_tree() calls unittest_unflatten_overlay_base() >> after unflattening the device tree passed into the booting kernel. This >> step is needed for a specific portion of the unittests. >> >> I'm still looking at the bigger picture of using overlays for the PCIe >> card, so more comments will be coming about that bigger picture. >> >> -Frank >> > >
Le Tue, 17 May 2022 11:03:41 -0400, Frank Rowand <frowand.list@gmail.com> a écrit : > On 5/17/22 02:37, Clément Léger wrote: > > Le Mon, 16 May 2022 23:11:03 -0400, > > Frank Rowand <frowand.list@gmail.com> a écrit : > > > >> On 5/3/22 08:45, Rob Herring wrote: > >>> On Wed, Apr 27, 2022 at 11:45:00AM +0200, Clément Léger wrote: > >>>> When enabling CONFIG_OF on a platform where of_root is not populated by > >>>> firmware, we end up without a root node. In order to apply overlays and > >>>> create subnodes of the root node, we need one. This commit creates an > >>>> empty root node if not present. > >>> > >>> The existing unittest essentially does the same thing for running the > >>> tests on non-DT systems. It should be modified to use this support > >>> instead. Maybe that's just removing the unittest code that set of_root. > >>> > >>> I expect Frank will have some comments. > >> > >> My preference would be for unflatten_and_copy_device_tree() to > >> use a compiled in FDT that only contains a root node, in the > >> case that no valid device tree is found (in other words, > >> "if (!initial_boot_params)". > > > > Ok, so basically, instead of creating the root node manually, you > > expect a device-tree which contains the following to be builtin the > > kernel and unflattened if needed: > > > > / { > > > > }; > > Yes. If you agree with this I can create a patch to implement it. I think > it is useful even stand alone from the rest of the series. If you want to implement this, feel free to do so, I'll (at least) be able to test it. > > > > > Maybe "chosen" and "aliases" nodes should also be provided as empty > > nodes since the unittest are creating them anyway and the core DT code > > also uses them. > > No. Unittest does not create both of them (I'm pretty sure, but I'm not > going to double check). If I recall correctly, unittest adds a property > in one of those two nodes, and thus implicitly creates the node if not > already present. Unittest does populate internal pointers to those two > nodes if the nodes are present (otherwise the pointers will have the > value of null). There is no need for the nodes to be present if empty. Acked, makes sense. Clément > > -Frank > > > > > Thanks, > > > > Clément > > > >> > >> unflatten_and_copy_device_tree() calls unittest_unflatten_overlay_base() > >> after unflattening the device tree passed into the booting kernel. This > >> step is needed for a specific portion of the unittests. > >> > >> I'm still looking at the bigger picture of using overlays for the PCIe > >> card, so more comments will be coming about that bigger picture. > >> > >> -Frank > >> > > > > >
diff --git a/drivers/of/base.c b/drivers/of/base.c index e7d92b67cb8a..6b8584c39f73 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -177,6 +177,19 @@ void __init of_core_init(void) pr_err("failed to register existing nodes\n"); return; } + + if (!of_root) { + of_root = kzalloc(sizeof(*of_root), GFP_KERNEL); + if (!of_root) { + mutex_unlock(&of_mutex); + pr_err("failed to create root node\n"); + return; + } + + of_root->full_name = "/"; + of_node_init(of_root); + } + for_each_of_allnodes(np) { __of_attach_node_sysfs(np); if (np->phandle && !phandle_cache[of_phandle_cache_hash(np->phandle)]) @@ -185,8 +198,7 @@ void __init of_core_init(void) mutex_unlock(&of_mutex); /* Symlink in /proc as required by userspace ABI */ - if (of_root) - proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base"); + proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base"); } static struct property *__of_find_property(const struct device_node *np,