| Message ID | 20220623105044.152832-2-clement.leger@bootlin.com |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | of: populate of_root node if not set | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | success |
Hi Clement, I said that I would send a patch to do this, but have failed to follow through. Sorry about that. On 6/23/22 06:50, 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. Create this root node > by unflattening an empty builtin dtb with of_fdt_unflatten(). > > 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/Makefile | 2 +- > drivers/of/base.c | 18 ++++++++++++++++-- > drivers/of/empty_root.dts | 6 ++++++ > 3 files changed, 23 insertions(+), 3 deletions(-) > create mode 100644 drivers/of/empty_root.dts > > diff --git a/drivers/of/Makefile b/drivers/of/Makefile > index e0360a44306e..ce56c8b95c83 100644 > --- a/drivers/of/Makefile > +++ b/drivers/of/Makefile > @@ -1,5 +1,5 @@ > # SPDX-License-Identifier: GPL-2.0 > -obj-y = base.o device.o platform.o property.o > +obj-y = base.o empty_root.dtb.o device.o platform.o property.o > obj-$(CONFIG_OF_KOBJ) += kobj.o > obj-$(CONFIG_OF_DYNAMIC) += dynamic.o > obj-$(CONFIG_OF_FLATTREE) += fdt.o > diff --git a/drivers/of/base.c b/drivers/of/base.c > index d4f98c8469ed..43e0f027a49c 100644 > --- a/drivers/of/base.c > +++ b/drivers/of/base.c > @@ -22,6 +22,7 @@ > #include <linux/module.h> > #include <linux/of.h> > #include <linux/of_device.h> > +#include <linux/of_fdt.h> > #include <linux/of_graph.h> > #include <linux/spinlock.h> > #include <linux/slab.h> > @@ -163,6 +164,8 @@ void __of_phandle_cache_inv_entry(phandle handle) > phandle_cache[handle_hash] = NULL; > } > > +extern const char __dtb_empty_root_begin[]; > + > void __init of_core_init(void) > { > struct device_node *np; > @@ -176,6 +179,18 @@ void __init of_core_init(void) > pr_err("failed to register existing nodes\n"); > return; > } > + > + if (!of_root) { > + void *dt; > + const unsigned long *fdt = (const unsigned long *) > + __dtb_empty_root_begin; > + dt = of_fdt_unflatten_tree(fdt, NULL, &of_root); > + if (!dt) { > + pr_err("Failed to setup empty root dt\n"); > + return; > + } > + } > + > for_each_of_allnodes(np) { > __of_attach_node_sysfs(np); > if (np->phandle && !phandle_cache[of_phandle_cache_hash(np->phandle)]) > @@ -184,8 +199,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"); > } This approach is adding an additional method of unflattening the tree. I would prefer to consolidate in a single location. I have leveraged this patch series into a different patch series to accomplish that. I have boot tested with one configuration, but want to test two more configurations before sending the new series. It should only take "a few minutes". -Frank > > static struct property *__of_find_property(const struct device_node *np, > diff --git a/drivers/of/empty_root.dts b/drivers/of/empty_root.dts > new file mode 100644 > index 000000000000..cf9e97a60f48 > --- /dev/null > +++ b/drivers/of/empty_root.dts > @@ -0,0 +1,6 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +/dts-v1/; > + > +/ { > + > +};
On 6/23/22 19:15, Frank Rowand wrote: > Hi Clement, > > I said that I would send a patch to do this, but have failed to follow > through. Sorry about that. > > > On 6/23/22 06:50, 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. Create this root node >> by unflattening an empty builtin dtb with of_fdt_unflatten(). >> >> 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/Makefile | 2 +- >> drivers/of/base.c | 18 ++++++++++++++++-- >> drivers/of/empty_root.dts | 6 ++++++ >> 3 files changed, 23 insertions(+), 3 deletions(-) >> create mode 100644 drivers/of/empty_root.dts >> >> diff --git a/drivers/of/Makefile b/drivers/of/Makefile >> index e0360a44306e..ce56c8b95c83 100644 >> --- a/drivers/of/Makefile >> +++ b/drivers/of/Makefile >> @@ -1,5 +1,5 @@ >> # SPDX-License-Identifier: GPL-2.0 >> -obj-y = base.o device.o platform.o property.o >> +obj-y = base.o empty_root.dtb.o device.o platform.o property.o >> obj-$(CONFIG_OF_KOBJ) += kobj.o >> obj-$(CONFIG_OF_DYNAMIC) += dynamic.o >> obj-$(CONFIG_OF_FLATTREE) += fdt.o >> diff --git a/drivers/of/base.c b/drivers/of/base.c >> index d4f98c8469ed..43e0f027a49c 100644 >> --- a/drivers/of/base.c >> +++ b/drivers/of/base.c >> @@ -22,6 +22,7 @@ >> #include <linux/module.h> >> #include <linux/of.h> >> #include <linux/of_device.h> >> +#include <linux/of_fdt.h> >> #include <linux/of_graph.h> >> #include <linux/spinlock.h> >> #include <linux/slab.h> >> @@ -163,6 +164,8 @@ void __of_phandle_cache_inv_entry(phandle handle) >> phandle_cache[handle_hash] = NULL; >> } >> >> +extern const char __dtb_empty_root_begin[]; >> + >> void __init of_core_init(void) >> { >> struct device_node *np; >> @@ -176,6 +179,18 @@ void __init of_core_init(void) >> pr_err("failed to register existing nodes\n"); >> return; >> } >> + >> + if (!of_root) { >> + void *dt; >> + const unsigned long *fdt = (const unsigned long *) >> + __dtb_empty_root_begin; >> + dt = of_fdt_unflatten_tree(fdt, NULL, &of_root); >> + if (!dt) { >> + pr_err("Failed to setup empty root dt\n"); >> + return; >> + } >> + } >> + >> for_each_of_allnodes(np) { >> __of_attach_node_sysfs(np); >> if (np->phandle && !phandle_cache[of_phandle_cache_hash(np->phandle)]) >> @@ -184,8 +199,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"); >> } > > This approach is adding an additional method of unflattening the tree. > I would prefer to consolidate in a single location. > > I have leveraged this patch series into a different patch series to > accomplish that. I have boot tested with one configuration, but want > to test two more configurations before sending the new series. It > should only take "a few minutes". "a few minutes" is taking longer than expected. One of the devicetree interrupt unittests is not giving the expected result for one of the configurations I am checking. -Frank > > -Frank > >> >> static struct property *__of_find_property(const struct device_node *np, >> diff --git a/drivers/of/empty_root.dts b/drivers/of/empty_root.dts >> new file mode 100644 >> index 000000000000..cf9e97a60f48 >> --- /dev/null >> +++ b/drivers/of/empty_root.dts >> @@ -0,0 +1,6 @@ >> +// SPDX-License-Identifier: GPL-2.0-only >> +/dts-v1/; >> + >> +/ { >> + >> +}; >
On 6/23/22 21:01, Frank Rowand wrote: > On 6/23/22 19:15, Frank Rowand wrote: >> Hi Clement, >> >> I said that I would send a patch to do this, but have failed to follow >> through. Sorry about that. >> >> >> On 6/23/22 06:50, 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. Create this root node >>> by unflattening an empty builtin dtb with of_fdt_unflatten(). >>> >>> 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/Makefile | 2 +- >>> drivers/of/base.c | 18 ++++++++++++++++-- >>> drivers/of/empty_root.dts | 6 ++++++ >>> 3 files changed, 23 insertions(+), 3 deletions(-) >>> create mode 100644 drivers/of/empty_root.dts >>> >>> diff --git a/drivers/of/Makefile b/drivers/of/Makefile >>> index e0360a44306e..ce56c8b95c83 100644 >>> --- a/drivers/of/Makefile >>> +++ b/drivers/of/Makefile >>> @@ -1,5 +1,5 @@ >>> # SPDX-License-Identifier: GPL-2.0 >>> -obj-y = base.o device.o platform.o property.o >>> +obj-y = base.o empty_root.dtb.o device.o platform.o property.o >>> obj-$(CONFIG_OF_KOBJ) += kobj.o >>> obj-$(CONFIG_OF_DYNAMIC) += dynamic.o >>> obj-$(CONFIG_OF_FLATTREE) += fdt.o >>> diff --git a/drivers/of/base.c b/drivers/of/base.c >>> index d4f98c8469ed..43e0f027a49c 100644 >>> --- a/drivers/of/base.c >>> +++ b/drivers/of/base.c >>> @@ -22,6 +22,7 @@ >>> #include <linux/module.h> >>> #include <linux/of.h> >>> #include <linux/of_device.h> >>> +#include <linux/of_fdt.h> >>> #include <linux/of_graph.h> >>> #include <linux/spinlock.h> >>> #include <linux/slab.h> >>> @@ -163,6 +164,8 @@ void __of_phandle_cache_inv_entry(phandle handle) >>> phandle_cache[handle_hash] = NULL; >>> } >>> >>> +extern const char __dtb_empty_root_begin[]; >>> + >>> void __init of_core_init(void) >>> { >>> struct device_node *np; >>> @@ -176,6 +179,18 @@ void __init of_core_init(void) >>> pr_err("failed to register existing nodes\n"); >>> return; >>> } >>> + >>> + if (!of_root) { >>> + void *dt; >>> + const unsigned long *fdt = (const unsigned long *) >>> + __dtb_empty_root_begin; >>> + dt = of_fdt_unflatten_tree(fdt, NULL, &of_root); >>> + if (!dt) { >>> + pr_err("Failed to setup empty root dt\n"); >>> + return; >>> + } >>> + } >>> + >>> for_each_of_allnodes(np) { >>> __of_attach_node_sysfs(np); >>> if (np->phandle && !phandle_cache[of_phandle_cache_hash(np->phandle)]) >>> @@ -184,8 +199,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"); >>> } >> >> This approach is adding an additional method of unflattening the tree. >> I would prefer to consolidate in a single location. >> >> I have leveraged this patch series into a different patch series to >> accomplish that. I have boot tested with one configuration, but want >> to test two more configurations before sending the new series. It >> should only take "a few minutes". > > "a few minutes" is taking longer than expected. One of the devicetree > interrupt unittests is not giving the expected result for one of the > configurations I am checking. The test is ok. The problem was unrelated asynchronous output to the console intermingled with test output. My suggested alternative patch series is at: https://lore.kernel.org/r/20220624034327.2542112-1-frowand.list@gmail.com -Frank > > -Frank > >> >> -Frank >> >>> >>> static struct property *__of_find_property(const struct device_node *np, >>> diff --git a/drivers/of/empty_root.dts b/drivers/of/empty_root.dts >>> new file mode 100644 >>> index 000000000000..cf9e97a60f48 >>> --- /dev/null >>> +++ b/drivers/of/empty_root.dts >>> @@ -0,0 +1,6 @@ >>> +// SPDX-License-Identifier: GPL-2.0-only >>> +/dts-v1/; >>> + >>> +/ { >>> + >>> +}; >> >
Le Thu, 23 Jun 2022 18:15:31 -0500, Frank Rowand <frowand.list@gmail.com> a écrit : > Hi Clement, > > I said that I would send a patch to do this, but have failed to follow > through. Sorry about that. Hi Frank, no worries, I just needed this topic to move forward, that's why I decided to sent this . Thanks ! Clément > > > On 6/23/22 06:50, 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. Create this root node > > by unflattening an empty builtin dtb with of_fdt_unflatten(). > > > > 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/Makefile | 2 +- > > drivers/of/base.c | 18 ++++++++++++++++-- > > drivers/of/empty_root.dts | 6 ++++++ > > 3 files changed, 23 insertions(+), 3 deletions(-) > > create mode 100644 drivers/of/empty_root.dts > > > > diff --git a/drivers/of/Makefile b/drivers/of/Makefile > > index e0360a44306e..ce56c8b95c83 100644 > > --- a/drivers/of/Makefile > > +++ b/drivers/of/Makefile > > @@ -1,5 +1,5 @@ > > # SPDX-License-Identifier: GPL-2.0 > > -obj-y = base.o device.o platform.o property.o > > +obj-y = base.o empty_root.dtb.o device.o platform.o property.o > > obj-$(CONFIG_OF_KOBJ) += kobj.o > > obj-$(CONFIG_OF_DYNAMIC) += dynamic.o > > obj-$(CONFIG_OF_FLATTREE) += fdt.o > > diff --git a/drivers/of/base.c b/drivers/of/base.c > > index d4f98c8469ed..43e0f027a49c 100644 > > --- a/drivers/of/base.c > > +++ b/drivers/of/base.c > > @@ -22,6 +22,7 @@ > > #include <linux/module.h> > > #include <linux/of.h> > > #include <linux/of_device.h> > > +#include <linux/of_fdt.h> > > #include <linux/of_graph.h> > > #include <linux/spinlock.h> > > #include <linux/slab.h> > > @@ -163,6 +164,8 @@ void __of_phandle_cache_inv_entry(phandle handle) > > phandle_cache[handle_hash] = NULL; > > } > > > > +extern const char __dtb_empty_root_begin[]; > > + > > void __init of_core_init(void) > > { > > struct device_node *np; > > @@ -176,6 +179,18 @@ void __init of_core_init(void) > > pr_err("failed to register existing nodes\n"); > > return; > > } > > + > > + if (!of_root) { > > + void *dt; > > + const unsigned long *fdt = (const unsigned long *) > > + __dtb_empty_root_begin; > > + dt = of_fdt_unflatten_tree(fdt, NULL, &of_root); > > + if (!dt) { > > + pr_err("Failed to setup empty root dt\n"); > > + return; > > + } > > + } > > + > > for_each_of_allnodes(np) { > > __of_attach_node_sysfs(np); > > if (np->phandle && !phandle_cache[of_phandle_cache_hash(np->phandle)]) > > @@ -184,8 +199,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"); > > } > > This approach is adding an additional method of unflattening the tree. > I would prefer to consolidate in a single location. > > I have leveraged this patch series into a different patch series to > accomplish that. I have boot tested with one configuration, but want > to test two more configurations before sending the new series. It > should only take "a few minutes". > > -Frank > > > > > static struct property *__of_find_property(const struct device_node *np, > > diff --git a/drivers/of/empty_root.dts b/drivers/of/empty_root.dts > > new file mode 100644 > > index 000000000000..cf9e97a60f48 > > --- /dev/null > > +++ b/drivers/of/empty_root.dts > > @@ -0,0 +1,6 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/dts-v1/; > > + > > +/ { > > + > > +}; >
diff --git a/drivers/of/Makefile b/drivers/of/Makefile index e0360a44306e..ce56c8b95c83 100644 --- a/drivers/of/Makefile +++ b/drivers/of/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0 -obj-y = base.o device.o platform.o property.o +obj-y = base.o empty_root.dtb.o device.o platform.o property.o obj-$(CONFIG_OF_KOBJ) += kobj.o obj-$(CONFIG_OF_DYNAMIC) += dynamic.o obj-$(CONFIG_OF_FLATTREE) += fdt.o diff --git a/drivers/of/base.c b/drivers/of/base.c index d4f98c8469ed..43e0f027a49c 100644 --- a/drivers/of/base.c +++ b/drivers/of/base.c @@ -22,6 +22,7 @@ #include <linux/module.h> #include <linux/of.h> #include <linux/of_device.h> +#include <linux/of_fdt.h> #include <linux/of_graph.h> #include <linux/spinlock.h> #include <linux/slab.h> @@ -163,6 +164,8 @@ void __of_phandle_cache_inv_entry(phandle handle) phandle_cache[handle_hash] = NULL; } +extern const char __dtb_empty_root_begin[]; + void __init of_core_init(void) { struct device_node *np; @@ -176,6 +179,18 @@ void __init of_core_init(void) pr_err("failed to register existing nodes\n"); return; } + + if (!of_root) { + void *dt; + const unsigned long *fdt = (const unsigned long *) + __dtb_empty_root_begin; + dt = of_fdt_unflatten_tree(fdt, NULL, &of_root); + if (!dt) { + pr_err("Failed to setup empty root dt\n"); + return; + } + } + for_each_of_allnodes(np) { __of_attach_node_sysfs(np); if (np->phandle && !phandle_cache[of_phandle_cache_hash(np->phandle)]) @@ -184,8 +199,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, diff --git a/drivers/of/empty_root.dts b/drivers/of/empty_root.dts new file mode 100644 index 000000000000..cf9e97a60f48 --- /dev/null +++ b/drivers/of/empty_root.dts @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: GPL-2.0-only +/dts-v1/; + +/ { + +};