* [PATCH] external references for device tree overlays @ 2017-06-05 12:59 ` Stefani Seibold 0 siblings, 0 replies; 27+ messages in thread From: Stefani Seibold @ 2017-06-05 12:59 UTC (permalink / raw) To: Stefani Seibold, Pantelis Antoniou, Rob Herring, Frank Rowand, devicetree, linux-kernel Cc: Holm Rauchfuss, Stefani Seibold, Stefani Seibold From: Stefani Seibold <stefani@seibold.net> This patch enables external references for symbols which are not exported by the current device tree. For example // RASPI example (only for testing) /dts-v1/; /plugin/; / { compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; fragment@0 { target-path = "/soc/i2s@7e203000"; __overlay__ { #address-cells = <0x00000001>; #size-cells = <0x00000001>; test = "test"; timer = <&timer>; }; }; __external_symbols__ { timer = "/soc/timer@7e003000"; }; }; The "timer" symbol is not exported by the RASPI device tree, because it is missing in the __symbols__ section of the device tree. In case of the RASPI device tree this could be simple fixed by modifing the device tree source, but when the device tree is provided by a closed source BIOS this kind of missing symbol could not be fixed. An additional benefit is to override a (possible broken) symbol exported by the currect live device tree. The patch is based and tested on linux 4.12-rc3. Signed-off-by: Stefani Seibold <stefani.seibold.ext@huawei.com> Signed-off-by: Stefani Seibold <stefani@seibold.net> --- drivers/of/overlay.c | 19 +++++++++++++++++++ drivers/of/resolver.c | 27 ++++++++++++++++++++++----- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 7827786718d8..de6516ea0fcd 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -50,6 +50,7 @@ struct of_overlay { int id; struct list_head node; int count; + struct device_node *tree; struct of_overlay_info *ovinfo_tab; struct of_changeset cset; }; @@ -422,6 +423,8 @@ int of_overlay_create(struct device_node *tree) /* add to the tail of the overlay list */ list_add_tail(&ov->node, &ov_list); + ov->tree = tree; + of_overlay_notify(ov, OF_OVERLAY_POST_APPLY); mutex_unlock(&of_mutex); @@ -524,6 +527,7 @@ int of_overlay_destroy(int id) { struct of_overlay *ov; int err; + phandle phandle; mutex_lock(&of_mutex); @@ -540,6 +544,8 @@ int of_overlay_destroy(int id) goto out; } + phandle = ov->tree->phandle; + of_overlay_notify(ov, OF_OVERLAY_PRE_REMOVE); list_del(&ov->node); __of_changeset_revert(&ov->cset); @@ -549,6 +555,19 @@ int of_overlay_destroy(int id) of_changeset_destroy(&ov->cset); kfree(ov); + if (phandle) { + struct device_node *node; + unsigned long flags; + + raw_spin_lock_irqsave(&devtree_lock, flags); + for_each_of_allnodes(node) { + if (node->phandle >= phandle) + node->phandle = 0; + } + raw_spin_unlock_irqrestore(&devtree_lock, flags); + } + + err = 0; out: diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c index 771f4844c781..31b5f32c9b27 100644 --- a/drivers/of/resolver.c +++ b/drivers/of/resolver.c @@ -286,13 +286,14 @@ static int adjust_local_phandle_references(struct device_node *local_fixups, int of_resolve_phandles(struct device_node *overlay) { struct device_node *child, *local_fixups, *refnode; - struct device_node *tree_symbols, *overlay_fixups; + struct device_node *tree_symbols, *ext_symbols, *overlay_fixups; struct property *prop; const char *refpath; phandle phandle, phandle_delta; int err; tree_symbols = NULL; + ext_symbols = NULL; if (!overlay) { pr_err("null overlay\n"); @@ -321,6 +322,9 @@ int of_resolve_phandles(struct device_node *overlay) for_each_child_of_node(overlay, child) { if (!of_node_cmp(child->name, "__fixups__")) overlay_fixups = child; + else + if (!of_node_cmp(child->name, "__external_symbols__")) + ext_symbols = child; } if (!overlay_fixups) { @@ -329,20 +333,30 @@ int of_resolve_phandles(struct device_node *overlay) } tree_symbols = of_find_node_by_path("/__symbols__"); - if (!tree_symbols) { - pr_err("no symbols in root of device tree.\n"); + if (!tree_symbols && !ext_symbols) { + pr_err("no symbols for resolve in device tree.\n"); err = -EINVAL; goto out; } + phandle_delta = live_tree_max_phandle() + 1; + for_each_property_of_node(overlay_fixups, prop) { /* skip properties added automatically */ if (!of_prop_cmp(prop->name, "name")) continue; - err = of_property_read_string(tree_symbols, - prop->name, &refpath); + err = -1; + + if (ext_symbols) + err = of_property_read_string(ext_symbols, + prop->name, &refpath); + + if (err && tree_symbols) + err = of_property_read_string(tree_symbols, + prop->name, &refpath); + if (err) goto out; @@ -352,6 +366,9 @@ int of_resolve_phandles(struct device_node *overlay) goto out; } + if (!refnode->phandle) + refnode->phandle = ++phandle_delta; + phandle = refnode->phandle; of_node_put(refnode); -- 2.13.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* [PATCH] external references for device tree overlays @ 2017-06-05 12:59 ` Stefani Seibold 0 siblings, 0 replies; 27+ messages in thread From: Stefani Seibold @ 2017-06-05 12:59 UTC (permalink / raw) To: Pantelis Antoniou, Rob Herring, Frank Rowand, devicetree, linux-kernel Cc: Holm Rauchfuss, Stefani Seibold, Stefani Seibold From: Stefani Seibold <stefani@seibold.net> This patch enables external references for symbols which are not exported by the current device tree. For example // RASPI example (only for testing) /dts-v1/; /plugin/; / { compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; fragment@0 { target-path = "/soc/i2s@7e203000"; __overlay__ { #address-cells = <0x00000001>; #size-cells = <0x00000001>; test = "test"; timer = <&timer>; }; }; __external_symbols__ { timer = "/soc/timer@7e003000"; }; }; The "timer" symbol is not exported by the RASPI device tree, because it is missing in the __symbols__ section of the device tree. In case of the RASPI device tree this could be simple fixed by modifing the device tree source, but when the device tree is provided by a closed source BIOS this kind of missing symbol could not be fixed. An additional benefit is to override a (possible broken) symbol exported by the currect live device tree. The patch is based and tested on linux 4.12-rc3. Signed-off-by: Stefani Seibold <stefani.seibold.ext@huawei.com> Signed-off-by: Stefani Seibold <stefani@seibold.net> --- drivers/of/overlay.c | 19 +++++++++++++++++++ drivers/of/resolver.c | 27 ++++++++++++++++++++++----- 2 files changed, 41 insertions(+), 5 deletions(-) diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index 7827786718d8..de6516ea0fcd 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -50,6 +50,7 @@ struct of_overlay { int id; struct list_head node; int count; + struct device_node *tree; struct of_overlay_info *ovinfo_tab; struct of_changeset cset; }; @@ -422,6 +423,8 @@ int of_overlay_create(struct device_node *tree) /* add to the tail of the overlay list */ list_add_tail(&ov->node, &ov_list); + ov->tree = tree; + of_overlay_notify(ov, OF_OVERLAY_POST_APPLY); mutex_unlock(&of_mutex); @@ -524,6 +527,7 @@ int of_overlay_destroy(int id) { struct of_overlay *ov; int err; + phandle phandle; mutex_lock(&of_mutex); @@ -540,6 +544,8 @@ int of_overlay_destroy(int id) goto out; } + phandle = ov->tree->phandle; + of_overlay_notify(ov, OF_OVERLAY_PRE_REMOVE); list_del(&ov->node); __of_changeset_revert(&ov->cset); @@ -549,6 +555,19 @@ int of_overlay_destroy(int id) of_changeset_destroy(&ov->cset); kfree(ov); + if (phandle) { + struct device_node *node; + unsigned long flags; + + raw_spin_lock_irqsave(&devtree_lock, flags); + for_each_of_allnodes(node) { + if (node->phandle >= phandle) + node->phandle = 0; + } + raw_spin_unlock_irqrestore(&devtree_lock, flags); + } + + err = 0; out: diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c index 771f4844c781..31b5f32c9b27 100644 --- a/drivers/of/resolver.c +++ b/drivers/of/resolver.c @@ -286,13 +286,14 @@ static int adjust_local_phandle_references(struct device_node *local_fixups, int of_resolve_phandles(struct device_node *overlay) { struct device_node *child, *local_fixups, *refnode; - struct device_node *tree_symbols, *overlay_fixups; + struct device_node *tree_symbols, *ext_symbols, *overlay_fixups; struct property *prop; const char *refpath; phandle phandle, phandle_delta; int err; tree_symbols = NULL; + ext_symbols = NULL; if (!overlay) { pr_err("null overlay\n"); @@ -321,6 +322,9 @@ int of_resolve_phandles(struct device_node *overlay) for_each_child_of_node(overlay, child) { if (!of_node_cmp(child->name, "__fixups__")) overlay_fixups = child; + else + if (!of_node_cmp(child->name, "__external_symbols__")) + ext_symbols = child; } if (!overlay_fixups) { @@ -329,20 +333,30 @@ int of_resolve_phandles(struct device_node *overlay) } tree_symbols = of_find_node_by_path("/__symbols__"); - if (!tree_symbols) { - pr_err("no symbols in root of device tree.\n"); + if (!tree_symbols && !ext_symbols) { + pr_err("no symbols for resolve in device tree.\n"); err = -EINVAL; goto out; } + phandle_delta = live_tree_max_phandle() + 1; + for_each_property_of_node(overlay_fixups, prop) { /* skip properties added automatically */ if (!of_prop_cmp(prop->name, "name")) continue; - err = of_property_read_string(tree_symbols, - prop->name, &refpath); + err = -1; + + if (ext_symbols) + err = of_property_read_string(ext_symbols, + prop->name, &refpath); + + if (err && tree_symbols) + err = of_property_read_string(tree_symbols, + prop->name, &refpath); + if (err) goto out; @@ -352,6 +366,9 @@ int of_resolve_phandles(struct device_node *overlay) goto out; } + if (!refnode->phandle) + refnode->phandle = ++phandle_delta; + phandle = refnode->phandle; of_node_put(refnode); -- 2.13.0 ^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH] external references for device tree overlays 2017-06-05 12:59 ` Stefani Seibold (?) @ 2017-06-05 18:43 ` Pantelis Antoniou 2017-06-06 19:17 ` Stefani Seibold -1 siblings, 1 reply; 27+ messages in thread From: Pantelis Antoniou @ 2017-06-05 18:43 UTC (permalink / raw) To: Stefani Seibold Cc: Stefani Seibold, Rob Herring, Frank Rowand, devicetree, linux-kernel, Holm Rauchfuss Hi Stefani, On Mon, 2017-06-05 at 14:59 +0200, Stefani Seibold wrote: > From: Stefani Seibold <stefani@seibold.net> > > This patch enables external references for symbols which are not > exported by the current device tree. For example > > // RASPI example (only for testing) > /dts-v1/; > /plugin/; > > / { > compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; > > fragment@0 { > target-path = "/soc/i2s@7e203000"; > __overlay__ { > #address-cells = <0x00000001>; > #size-cells = <0x00000001>; > test = "test"; > timer = <&timer>; > }; > }; > > __external_symbols__ { > timer = "/soc/timer@7e003000"; > }; > }; > I understand the problem. I am just not fond of the __external_symbols__ solution. There's a facility in the DT source language that allows to declare pathspec labels. The 'timer = <&timer>;' statement could be rewritten as 'timer = <&{/soc/timer@7e0030000}>;' Internally you can 'catch' that this refers to a symbol in the base tree and then do the same symbol insertion as the patch you've submitted. The benefit to the above is that you don't introduce manually edited special nodes. Regards -- Pantelis > The "timer" symbol is not exported by the RASPI device tree, because it is > missing in the __symbols__ section of the device tree. > > In case of the RASPI device tree this could be simple fixed by modifing > the device tree source, but when the device tree is provided by a closed > source BIOS this kind of missing symbol could not be fixed. > > An additional benefit is to override a (possible broken) symbol exported > by the currect live device tree. > > The patch is based and tested on linux 4.12-rc3. > > Signed-off-by: Stefani Seibold <stefani.seibold.ext@huawei.com> > Signed-off-by: Stefani Seibold <stefani@seibold.net> > --- > drivers/of/overlay.c | 19 +++++++++++++++++++ > drivers/of/resolver.c | 27 ++++++++++++++++++++++----- > 2 files changed, 41 insertions(+), 5 deletions(-) > > diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c > index 7827786718d8..de6516ea0fcd 100644 > --- a/drivers/of/overlay.c > +++ b/drivers/of/overlay.c > @@ -50,6 +50,7 @@ struct of_overlay { > int id; > struct list_head node; > int count; > + struct device_node *tree; > struct of_overlay_info *ovinfo_tab; > struct of_changeset cset; > }; > @@ -422,6 +423,8 @@ int of_overlay_create(struct device_node *tree) > /* add to the tail of the overlay list */ > list_add_tail(&ov->node, &ov_list); > > + ov->tree = tree; > + > of_overlay_notify(ov, OF_OVERLAY_POST_APPLY); > > mutex_unlock(&of_mutex); > @@ -524,6 +527,7 @@ int of_overlay_destroy(int id) > { > struct of_overlay *ov; > int err; > + phandle phandle; > > mutex_lock(&of_mutex); > > @@ -540,6 +544,8 @@ int of_overlay_destroy(int id) > goto out; > } > > + phandle = ov->tree->phandle; > + > of_overlay_notify(ov, OF_OVERLAY_PRE_REMOVE); > list_del(&ov->node); > __of_changeset_revert(&ov->cset); > @@ -549,6 +555,19 @@ int of_overlay_destroy(int id) > of_changeset_destroy(&ov->cset); > kfree(ov); > > + if (phandle) { > + struct device_node *node; > + unsigned long flags; > + > + raw_spin_lock_irqsave(&devtree_lock, flags); > + for_each_of_allnodes(node) { > + if (node->phandle >= phandle) > + node->phandle = 0; > + } > + raw_spin_unlock_irqrestore(&devtree_lock, flags); > + } > + > + > err = 0; > > out: > diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c > index 771f4844c781..31b5f32c9b27 100644 > --- a/drivers/of/resolver.c > +++ b/drivers/of/resolver.c > @@ -286,13 +286,14 @@ static int adjust_local_phandle_references(struct device_node *local_fixups, > int of_resolve_phandles(struct device_node *overlay) > { > struct device_node *child, *local_fixups, *refnode; > - struct device_node *tree_symbols, *overlay_fixups; > + struct device_node *tree_symbols, *ext_symbols, *overlay_fixups; > struct property *prop; > const char *refpath; > phandle phandle, phandle_delta; > int err; > > tree_symbols = NULL; > + ext_symbols = NULL; > > if (!overlay) { > pr_err("null overlay\n"); > @@ -321,6 +322,9 @@ int of_resolve_phandles(struct device_node *overlay) > for_each_child_of_node(overlay, child) { > if (!of_node_cmp(child->name, "__fixups__")) > overlay_fixups = child; > + else > + if (!of_node_cmp(child->name, "__external_symbols__")) > + ext_symbols = child; > } > > if (!overlay_fixups) { > @@ -329,20 +333,30 @@ int of_resolve_phandles(struct device_node *overlay) > } > > tree_symbols = of_find_node_by_path("/__symbols__"); > - if (!tree_symbols) { > - pr_err("no symbols in root of device tree.\n"); > + if (!tree_symbols && !ext_symbols) { > + pr_err("no symbols for resolve in device tree.\n"); > err = -EINVAL; > goto out; > } > > + phandle_delta = live_tree_max_phandle() + 1; > + > for_each_property_of_node(overlay_fixups, prop) { > > /* skip properties added automatically */ > if (!of_prop_cmp(prop->name, "name")) > continue; > > - err = of_property_read_string(tree_symbols, > - prop->name, &refpath); > + err = -1; > + > + if (ext_symbols) > + err = of_property_read_string(ext_symbols, > + prop->name, &refpath); > + > + if (err && tree_symbols) > + err = of_property_read_string(tree_symbols, > + prop->name, &refpath); > + > if (err) > goto out; > > @@ -352,6 +366,9 @@ int of_resolve_phandles(struct device_node *overlay) > goto out; > } > > + if (!refnode->phandle) > + refnode->phandle = ++phandle_delta; > + > phandle = refnode->phandle; > of_node_put(refnode); > ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] external references for device tree overlays 2017-06-05 18:43 ` Pantelis Antoniou @ 2017-06-06 19:17 ` Stefani Seibold 2017-06-06 22:05 ` Rob Herring 2017-06-07 8:11 ` Pantelis Antoniou 0 siblings, 2 replies; 27+ messages in thread From: Stefani Seibold @ 2017-06-06 19:17 UTC (permalink / raw) To: Pantelis Antoniou, Stefani Seibold Cc: Rob Herring, Frank Rowand, devicetree, linux-kernel, Holm Rauchfuss Hi Pantelis, thanks for the suggestion. This feature is not very well documented. I tried this on my rasp1 running 4.12.0-rc3 and it doesn't work. My source is: // rapsi example /dts-v1/; /plugin/; / { compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; fragment@0 { target-path = "/soc/i2s@7e203000"; __overlay__ { #address-cells = <0x00000001>; #size-cells = <0x00000001>; test = "test"; timer = <&{/soc/timer@7e0030000}>; }; }; }; The resulting overlay is (decompiled with fdtdump): /dts-v1/; // magic: 0xd00dfeed // totalsize: 0x19a (410) // off_dt_struct: 0x38 // off_dt_strings: 0x148 // off_mem_rsvmap: 0x28 // version: 17 // last_comp_version: 16 // boot_cpuid_phys: 0x0 // size_dt_strings: 0x52 // size_dt_struct: 0x110 / { compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; fragment@0 { target-path = "/soc/i2s@7e203000"; __overlay__ { #address-cells = <0x00000001>; #size-cells = <0x00000001>; test = "test"; timer = <0xdeadbeef>; }; }; __fixups__ { /soc/timer@7e0030000 = "/fragment@0/__overlay__:timer:0"; }; }; But this will not apply: OF: resolver: overlay phandle fixup failed: -22 create_overlay: Failed to resolve tree Anyway, the reason for my patch is that i can reference to nodes which lacks a phandle. The phandle will be created on the fly and also destroyed when the overlay is unloaded. I have a real use case for this patch: I have a BIOS on some ARM64 servers which provides broken device tree. It also lacks some devices in this tree which needs references to other devices which lacks a phandle. Since the BIOSes are closed source i need a way to work arround this problem without patching all the drivers involved to this devices. Hope this helps to understand the reason for this patch. - Stefani Am Montag, den 05.06.2017, 21:43 +0300 schrieb Pantelis Antoniou: > Hi Stefani, > > On Mon, 2017-06-05 at 14:59 +0200, Stefani Seibold wrote: > > From: Stefani Seibold <stefani@seibold.net> > > > > This patch enables external references for symbols which are not > > exported by the current device tree. For example > > > > // RASPI example (only for testing) > > /dts-v1/; > > /plugin/; > > > > / { > > compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; > > > > fragment@0 { > > target-path = "/soc/i2s@7e203000"; > > __overlay__ { > > #address-cells = <0x00000001>; > > #size-cells = <0x00000001>; > > test = "test"; > > timer = <&timer>; > > }; > > }; > > > > __external_symbols__ { > > timer = "/soc/timer@7e003000"; > > }; > > }; > > > > I understand the problem. I am just not fond of the > __external_symbols__ > solution. > > There's a facility in the DT source language that allows to declare > pathspec labels. > > The 'timer = <&timer>;' statement could be rewritten as > 'timer = <&{/soc/timer@7e0030000}>;' > > Internally you can 'catch' that this refers to a symbol in the base > tree > and then do the same symbol insertion as the patch you've submitted. > > The benefit to the above is that you don't introduce manually edited > special nodes. > > Regards > > -- Pantelis > > > The "timer" symbol is not exported by the RASPI device tree, > > because it is > > missing in the __symbols__ section of the device tree. > > > > In case of the RASPI device tree this could be simple fixed by > > modifing > > the device tree source, but when the device tree is provided by a > > closed > > source BIOS this kind of missing symbol could not be fixed. > > > > An additional benefit is to override a (possible broken) symbol > > exported > > by the currect live device tree. > > > > The patch is based and tested on linux 4.12-rc3. > > > > Signed-off-by: Stefani Seibold <stefani.seibold.ext@huawei.com> > > Signed-off-by: Stefani Seibold <stefani@seibold.net> > > --- > > drivers/of/overlay.c | 19 +++++++++++++++++++ > > drivers/of/resolver.c | 27 ++++++++++++++++++++++----- > > 2 files changed, 41 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c > > index 7827786718d8..de6516ea0fcd 100644 > > --- a/drivers/of/overlay.c > > +++ b/drivers/of/overlay.c > > @@ -50,6 +50,7 @@ struct of_overlay { > > int id; > > struct list_head node; > > int count; > > + struct device_node *tree; > > struct of_overlay_info *ovinfo_tab; > > struct of_changeset cset; > > }; > > @@ -422,6 +423,8 @@ int of_overlay_create(struct device_node *tree) > > /* add to the tail of the overlay list */ > > list_add_tail(&ov->node, &ov_list); > > > > + ov->tree = tree; > > + > > of_overlay_notify(ov, OF_OVERLAY_POST_APPLY); > > > > mutex_unlock(&of_mutex); > > @@ -524,6 +527,7 @@ int of_overlay_destroy(int id) > > { > > struct of_overlay *ov; > > int err; > > + phandle phandle; > > > > mutex_lock(&of_mutex); > > > > @@ -540,6 +544,8 @@ int of_overlay_destroy(int id) > > goto out; > > } > > > > + phandle = ov->tree->phandle; > > + > > of_overlay_notify(ov, OF_OVERLAY_PRE_REMOVE); > > list_del(&ov->node); > > __of_changeset_revert(&ov->cset); > > @@ -549,6 +555,19 @@ int of_overlay_destroy(int id) > > of_changeset_destroy(&ov->cset); > > kfree(ov); > > > > + if (phandle) { > > + struct device_node *node; > > + unsigned long flags; > > + > > + raw_spin_lock_irqsave(&devtree_lock, flags); > > + for_each_of_allnodes(node) { > > + if (node->phandle >= phandle) > > + node->phandle = 0; > > + } > > + raw_spin_unlock_irqrestore(&devtree_lock, flags); > > + } > > + > > + > > err = 0; > > > > out: > > diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c > > index 771f4844c781..31b5f32c9b27 100644 > > --- a/drivers/of/resolver.c > > +++ b/drivers/of/resolver.c > > @@ -286,13 +286,14 @@ static int > > adjust_local_phandle_references(struct device_node *local_fixups, > > int of_resolve_phandles(struct device_node *overlay) > > { > > struct device_node *child, *local_fixups, *refnode; > > - struct device_node *tree_symbols, *overlay_fixups; > > + struct device_node *tree_symbols, *ext_symbols, > > *overlay_fixups; > > struct property *prop; > > const char *refpath; > > phandle phandle, phandle_delta; > > int err; > > > > tree_symbols = NULL; > > + ext_symbols = NULL; > > > > if (!overlay) { > > pr_err("null overlay\n"); > > @@ -321,6 +322,9 @@ int of_resolve_phandles(struct device_node > > *overlay) > > for_each_child_of_node(overlay, child) { > > if (!of_node_cmp(child->name, "__fixups__")) > > overlay_fixups = child; > > + else > > + if (!of_node_cmp(child->name, > > "__external_symbols__")) > > + ext_symbols = child; > > } > > > > if (!overlay_fixups) { > > @@ -329,20 +333,30 @@ int of_resolve_phandles(struct device_node > > *overlay) > > } > > > > tree_symbols = of_find_node_by_path("/__symbols__"); > > - if (!tree_symbols) { > > - pr_err("no symbols in root of device tree.\n"); > > + if (!tree_symbols && !ext_symbols) { > > + pr_err("no symbols for resolve in device > > tree.\n"); > > err = -EINVAL; > > goto out; > > } > > > > + phandle_delta = live_tree_max_phandle() + 1; > > + > > for_each_property_of_node(overlay_fixups, prop) { > > > > /* skip properties added automatically */ > > if (!of_prop_cmp(prop->name, "name")) > > continue; > > > > - err = of_property_read_string(tree_symbols, > > - prop->name, &refpath); > > + err = -1; > > + > > + if (ext_symbols) > > + err = of_property_read_string(ext_symbols, > > + prop->name, &refpath); > > + > > + if (err && tree_symbols) > > + err = > > of_property_read_string(tree_symbols, > > + prop->name, &refpath); > > + > > if (err) > > goto out; > > > > @@ -352,6 +366,9 @@ int of_resolve_phandles(struct device_node > > *overlay) > > goto out; > > } > > > > + if (!refnode->phandle) > > + refnode->phandle = ++phandle_delta; > > + > > phandle = refnode->phandle; > > of_node_put(refnode); > > > > ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] external references for device tree overlays @ 2017-06-06 22:05 ` Rob Herring 0 siblings, 0 replies; 27+ messages in thread From: Rob Herring @ 2017-06-06 22:05 UTC (permalink / raw) To: Stefani Seibold Cc: Pantelis Antoniou, Stefani Seibold, Frank Rowand, devicetree, linux-kernel, Holm Rauchfuss On Tue, Jun 6, 2017 at 2:17 PM, Stefani Seibold <stefani@seibold.net> wrote: > Hi Pantelis, > > thanks for the suggestion. This feature is not very well documented. I > tried this on my rasp1 running 4.12.0-rc3 and it doesn't work. My > source is: > > // rapsi example > /dts-v1/; > /plugin/; > > / { > compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; > > fragment@0 { > target-path = "/soc/i2s@7e203000"; > __overlay__ { > #address-cells = <0x00000001>; > #size-cells = <0x00000001>; > test = "test"; > timer = <&{/soc/timer@7e0030000}>; > }; > }; > }; > > > The resulting overlay is (decompiled with fdtdump): > > /dts-v1/; > // magic: 0xd00dfeed > // totalsize: 0x19a (410) > // off_dt_struct: 0x38 > // off_dt_strings: 0x148 > // off_mem_rsvmap: 0x28 > // version: 17 > // last_comp_version: 16 > // boot_cpuid_phys: 0x0 > // size_dt_strings: 0x52 > // size_dt_struct: 0x110 > > / { > compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; > fragment@0 { > target-path = "/soc/i2s@7e203000"; > __overlay__ { > #address-cells = <0x00000001>; > #size-cells = <0x00000001>; > test = "test"; > timer = <0xdeadbeef>; > }; > }; > __fixups__ { > /soc/timer@7e0030000 = "/fragment@0/__overlay__:timer:0"; Looks like you (Pantelis had the typo) have an extra 0 in 7e0030000 compared to your original example. Rob ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] external references for device tree overlays @ 2017-06-06 22:05 ` Rob Herring 0 siblings, 0 replies; 27+ messages in thread From: Rob Herring @ 2017-06-06 22:05 UTC (permalink / raw) To: Stefani Seibold Cc: Pantelis Antoniou, Stefani Seibold, Frank Rowand, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Holm Rauchfuss On Tue, Jun 6, 2017 at 2:17 PM, Stefani Seibold <stefani-mkwtCZVSLSnR7s880joybQ@public.gmane.org> wrote: > Hi Pantelis, > > thanks for the suggestion. This feature is not very well documented. I > tried this on my rasp1 running 4.12.0-rc3 and it doesn't work. My > source is: > > // rapsi example > /dts-v1/; > /plugin/; > > / { > compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; > > fragment@0 { > target-path = "/soc/i2s@7e203000"; > __overlay__ { > #address-cells = <0x00000001>; > #size-cells = <0x00000001>; > test = "test"; > timer = <&{/soc/timer@7e0030000}>; > }; > }; > }; > > > The resulting overlay is (decompiled with fdtdump): > > /dts-v1/; > // magic: 0xd00dfeed > // totalsize: 0x19a (410) > // off_dt_struct: 0x38 > // off_dt_strings: 0x148 > // off_mem_rsvmap: 0x28 > // version: 17 > // last_comp_version: 16 > // boot_cpuid_phys: 0x0 > // size_dt_strings: 0x52 > // size_dt_struct: 0x110 > > / { > compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; > fragment@0 { > target-path = "/soc/i2s@7e203000"; > __overlay__ { > #address-cells = <0x00000001>; > #size-cells = <0x00000001>; > test = "test"; > timer = <0xdeadbeef>; > }; > }; > __fixups__ { > /soc/timer@7e0030000 = "/fragment@0/__overlay__:timer:0"; Looks like you (Pantelis had the typo) have an extra 0 in 7e0030000 compared to your original example. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] external references for device tree overlays 2017-06-06 19:17 ` Stefani Seibold 2017-06-06 22:05 ` Rob Herring @ 2017-06-07 8:11 ` Pantelis Antoniou 2017-06-07 22:19 ` Rob Herring 2017-06-08 6:48 ` Stefani Seibold 1 sibling, 2 replies; 27+ messages in thread From: Pantelis Antoniou @ 2017-06-07 8:11 UTC (permalink / raw) To: Stefani Seibold Cc: Stefani Seibold, Rob Herring, Frank Rowand, devicetree, linux-kernel, Holm Rauchfuss Hi Stefani, On Tue, 2017-06-06 at 21:17 +0200, Stefani Seibold wrote: > Hi Pantelis, > > thanks for the suggestion. This feature is not very well documented. I > tried this on my rasp1 running 4.12.0-rc3 and it doesn't work. My > source is: > > // rapsi example > /dts-v1/; > /plugin/; > > / { > compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; > > fragment@0 { > target-path = "/soc/i2s@7e203000"; > __overlay__ { > #address-cells = <0x00000001>; > #size-cells = <0x00000001>; > test = "test"; > timer = <&{/soc/timer@7e0030000}>; > }; > }; > }; > > > The resulting overlay is (decompiled with fdtdump): > > /dts-v1/; > // magic: 0xd00dfeed > // totalsize: 0x19a (410) > // off_dt_struct: 0x38 > // off_dt_strings: 0x148 > // off_mem_rsvmap: 0x28 > // version: 17 > // last_comp_version: 16 > // boot_cpuid_phys: 0x0 > // size_dt_strings: 0x52 > // size_dt_struct: 0x110 > > / { > compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; > fragment@0 { > target-path = "/soc/i2s@7e203000"; > __overlay__ { > #address-cells = <0x00000001>; > #size-cells = <0x00000001>; > test = "test"; > timer = <0xdeadbeef>; > }; > }; > __fixups__ { > /soc/timer@7e0030000 = "/fragment@0/__overlay__:timer:0"; > }; > }; > > But this will not apply: > > OF: resolver: overlay phandle fixup failed: -22 > create_overlay: Failed to resolve tree > > Yes, it will not work as it is; my point is that you don't need the magic __*__ node. You will need to modify the overlay application code to live insert a phandle (if it doesn't exist) when it encounters a /path fixup. > Anyway, the reason for my patch is that i can reference to nodes which > lacks a phandle. The phandle will be created on the fly and also > destroyed when the overlay is unloaded. > > I have a real use case for this patch: > > I have a BIOS on some ARM64 servers which provides broken device tree. > It also lacks some devices in this tree which needs references to other > devices which lacks a phandle. > > Since the BIOSes are closed source i need a way to work arround this > problem without patching all the drivers involved to this devices. > > Hope this helps to understand the reason for this patch. > FWIW your problem seems like something that would happen on the field. We can berate the vendor of not providing the correct device tree, but in the end workarounds for broken vendor things are common in the kernel. Regards -- Pantelis > - Stefani > > Am Montag, den 05.06.2017, 21:43 +0300 schrieb Pantelis Antoniou: > > Hi Stefani, > > > > On Mon, 2017-06-05 at 14:59 +0200, Stefani Seibold wrote: > > > From: Stefani Seibold <stefani@seibold.net> > > > > > > This patch enables external references for symbols which are not > > > exported by the current device tree. For example > > > > > > // RASPI example (only for testing) > > > /dts-v1/; > > > /plugin/; > > > > > > / { > > > compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; > > > > > > fragment@0 { > > > target-path = "/soc/i2s@7e203000"; > > > __overlay__ { > > > #address-cells = <0x00000001>; > > > #size-cells = <0x00000001>; > > > test = "test"; > > > timer = <&timer>; > > > }; > > > }; > > > > > > __external_symbols__ { > > > timer = "/soc/timer@7e003000"; > > > }; > > > }; > > > > > > > I understand the problem. I am just not fond of the > > __external_symbols__ > > solution. > > > > There's a facility in the DT source language that allows to declare > > pathspec labels. > > > > The 'timer = <&timer>;' statement could be rewritten as > > 'timer = <&{/soc/timer@7e0030000}>;' > > > > Internally you can 'catch' that this refers to a symbol in the base > > tree > > and then do the same symbol insertion as the patch you've submitted. > > > > The benefit to the above is that you don't introduce manually edited > > special nodes. > > > > Regards > > > > -- Pantelis > > > > > The "timer" symbol is not exported by the RASPI device tree, > > > because it is > > > missing in the __symbols__ section of the device tree. > > > > > > In case of the RASPI device tree this could be simple fixed by > > > modifing > > > the device tree source, but when the device tree is provided by a > > > closed > > > source BIOS this kind of missing symbol could not be fixed. > > > > > > An additional benefit is to override a (possible broken) symbol > > > exported > > > by the currect live device tree. > > > > > > The patch is based and tested on linux 4.12-rc3. > > > > > > Signed-off-by: Stefani Seibold <stefani.seibold.ext@huawei.com> > > > Signed-off-by: Stefani Seibold <stefani@seibold.net> > > > --- > > > drivers/of/overlay.c | 19 +++++++++++++++++++ > > > drivers/of/resolver.c | 27 ++++++++++++++++++++++----- > > > 2 files changed, 41 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c > > > index 7827786718d8..de6516ea0fcd 100644 > > > --- a/drivers/of/overlay.c > > > +++ b/drivers/of/overlay.c > > > @@ -50,6 +50,7 @@ struct of_overlay { > > > int id; > > > struct list_head node; > > > int count; > > > + struct device_node *tree; > > > struct of_overlay_info *ovinfo_tab; > > > struct of_changeset cset; > > > }; > > > @@ -422,6 +423,8 @@ int of_overlay_create(struct device_node *tree) > > > /* add to the tail of the overlay list */ > > > list_add_tail(&ov->node, &ov_list); > > > > > > + ov->tree = tree; > > > + > > > of_overlay_notify(ov, OF_OVERLAY_POST_APPLY); > > > > > > mutex_unlock(&of_mutex); > > > @@ -524,6 +527,7 @@ int of_overlay_destroy(int id) > > > { > > > struct of_overlay *ov; > > > int err; > > > + phandle phandle; > > > > > > mutex_lock(&of_mutex); > > > > > > @@ -540,6 +544,8 @@ int of_overlay_destroy(int id) > > > goto out; > > > } > > > > > > + phandle = ov->tree->phandle; > > > + > > > of_overlay_notify(ov, OF_OVERLAY_PRE_REMOVE); > > > list_del(&ov->node); > > > __of_changeset_revert(&ov->cset); > > > @@ -549,6 +555,19 @@ int of_overlay_destroy(int id) > > > of_changeset_destroy(&ov->cset); > > > kfree(ov); > > > > > > + if (phandle) { > > > + struct device_node *node; > > > + unsigned long flags; > > > + > > > + raw_spin_lock_irqsave(&devtree_lock, flags); > > > + for_each_of_allnodes(node) { > > > + if (node->phandle >= phandle) > > > + node->phandle = 0; > > > + } > > > + raw_spin_unlock_irqrestore(&devtree_lock, flags); > > > + } > > > + > > > + > > > err = 0; > > > > > > out: > > > diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c > > > index 771f4844c781..31b5f32c9b27 100644 > > > --- a/drivers/of/resolver.c > > > +++ b/drivers/of/resolver.c > > > @@ -286,13 +286,14 @@ static int > > > adjust_local_phandle_references(struct device_node *local_fixups, > > > int of_resolve_phandles(struct device_node *overlay) > > > { > > > struct device_node *child, *local_fixups, *refnode; > > > - struct device_node *tree_symbols, *overlay_fixups; > > > + struct device_node *tree_symbols, *ext_symbols, > > > *overlay_fixups; > > > struct property *prop; > > > const char *refpath; > > > phandle phandle, phandle_delta; > > > int err; > > > > > > tree_symbols = NULL; > > > + ext_symbols = NULL; > > > > > > if (!overlay) { > > > pr_err("null overlay\n"); > > > @@ -321,6 +322,9 @@ int of_resolve_phandles(struct device_node > > > *overlay) > > > for_each_child_of_node(overlay, child) { > > > if (!of_node_cmp(child->name, "__fixups__")) > > > overlay_fixups = child; > > > + else > > > + if (!of_node_cmp(child->name, > > > "__external_symbols__")) > > > + ext_symbols = child; > > > } > > > > > > if (!overlay_fixups) { > > > @@ -329,20 +333,30 @@ int of_resolve_phandles(struct device_node > > > *overlay) > > > } > > > > > > tree_symbols = of_find_node_by_path("/__symbols__"); > > > - if (!tree_symbols) { > > > - pr_err("no symbols in root of device tree.\n"); > > > + if (!tree_symbols && !ext_symbols) { > > > + pr_err("no symbols for resolve in device > > > tree.\n"); > > > err = -EINVAL; > > > goto out; > > > } > > > > > > + phandle_delta = live_tree_max_phandle() + 1; > > > + > > > for_each_property_of_node(overlay_fixups, prop) { > > > > > > /* skip properties added automatically */ > > > if (!of_prop_cmp(prop->name, "name")) > > > continue; > > > > > > - err = of_property_read_string(tree_symbols, > > > - prop->name, &refpath); > > > + err = -1; > > > + > > > + if (ext_symbols) > > > + err = of_property_read_string(ext_symbols, > > > + prop->name, &refpath); > > > + > > > + if (err && tree_symbols) > > > + err = > > > of_property_read_string(tree_symbols, > > > + prop->name, &refpath); > > > + > > > if (err) > > > goto out; > > > > > > @@ -352,6 +366,9 @@ int of_resolve_phandles(struct device_node > > > *overlay) > > > goto out; > > > } > > > > > > + if (!refnode->phandle) > > > + refnode->phandle = ++phandle_delta; > > > + > > > phandle = refnode->phandle; > > > of_node_put(refnode); > > > > > > > ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] external references for device tree overlays 2017-06-07 8:11 ` Pantelis Antoniou @ 2017-06-07 22:19 ` Rob Herring 2017-06-08 6:48 ` Stefani Seibold 1 sibling, 0 replies; 27+ messages in thread From: Rob Herring @ 2017-06-07 22:19 UTC (permalink / raw) To: Pantelis Antoniou Cc: Stefani Seibold, Stefani Seibold, Frank Rowand, devicetree, linux-kernel, Holm Rauchfuss On Wed, Jun 7, 2017 at 3:11 AM, Pantelis Antoniou <pantelis.antoniou@konsulko.com> wrote: > Hi Stefani, > > On Tue, 2017-06-06 at 21:17 +0200, Stefani Seibold wrote: >> Hi Pantelis, >> >> thanks for the suggestion. This feature is not very well documented. I >> tried this on my rasp1 running 4.12.0-rc3 and it doesn't work. My >> source is: >> >> // rapsi example >> /dts-v1/; >> /plugin/; >> >> / { >> compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; >> >> fragment@0 { >> target-path = "/soc/i2s@7e203000"; >> __overlay__ { >> #address-cells = <0x00000001>; >> #size-cells = <0x00000001>; >> test = "test"; >> timer = <&{/soc/timer@7e0030000}>; >> }; >> }; >> }; >> >> >> The resulting overlay is (decompiled with fdtdump): >> >> /dts-v1/; >> // magic: 0xd00dfeed >> // totalsize: 0x19a (410) >> // off_dt_struct: 0x38 >> // off_dt_strings: 0x148 >> // off_mem_rsvmap: 0x28 >> // version: 17 >> // last_comp_version: 16 >> // boot_cpuid_phys: 0x0 >> // size_dt_strings: 0x52 >> // size_dt_struct: 0x110 >> >> / { >> compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; >> fragment@0 { >> target-path = "/soc/i2s@7e203000"; >> __overlay__ { >> #address-cells = <0x00000001>; >> #size-cells = <0x00000001>; >> test = "test"; >> timer = <0xdeadbeef>; >> }; >> }; >> __fixups__ { >> /soc/timer@7e0030000 = "/fragment@0/__overlay__:timer:0"; >> }; >> }; >> >> But this will not apply: >> >> OF: resolver: overlay phandle fixup failed: -22 >> create_overlay: Failed to resolve tree >> >> > > Yes, it will not work as it is; my point is that you don't need the > magic __*__ node. > > You will need to modify the overlay application code to live insert a > phandle (if it doesn't exist) when it encounters a /path fixup. phandles only exist if something in the base tree refers to that node. Adding them when they don't exist should definitely be something we support for overlays. But don't call that a broken DT. That would be a separate issue. Rob ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] external references for device tree overlays @ 2017-06-07 22:19 ` Rob Herring 0 siblings, 0 replies; 27+ messages in thread From: Rob Herring @ 2017-06-07 22:19 UTC (permalink / raw) To: Pantelis Antoniou Cc: Stefani Seibold, Stefani Seibold, Frank Rowand, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Holm Rauchfuss On Wed, Jun 7, 2017 at 3:11 AM, Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote: > Hi Stefani, > > On Tue, 2017-06-06 at 21:17 +0200, Stefani Seibold wrote: >> Hi Pantelis, >> >> thanks for the suggestion. This feature is not very well documented. I >> tried this on my rasp1 running 4.12.0-rc3 and it doesn't work. My >> source is: >> >> // rapsi example >> /dts-v1/; >> /plugin/; >> >> / { >> compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; >> >> fragment@0 { >> target-path = "/soc/i2s@7e203000"; >> __overlay__ { >> #address-cells = <0x00000001>; >> #size-cells = <0x00000001>; >> test = "test"; >> timer = <&{/soc/timer@7e0030000}>; >> }; >> }; >> }; >> >> >> The resulting overlay is (decompiled with fdtdump): >> >> /dts-v1/; >> // magic: 0xd00dfeed >> // totalsize: 0x19a (410) >> // off_dt_struct: 0x38 >> // off_dt_strings: 0x148 >> // off_mem_rsvmap: 0x28 >> // version: 17 >> // last_comp_version: 16 >> // boot_cpuid_phys: 0x0 >> // size_dt_strings: 0x52 >> // size_dt_struct: 0x110 >> >> / { >> compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; >> fragment@0 { >> target-path = "/soc/i2s@7e203000"; >> __overlay__ { >> #address-cells = <0x00000001>; >> #size-cells = <0x00000001>; >> test = "test"; >> timer = <0xdeadbeef>; >> }; >> }; >> __fixups__ { >> /soc/timer@7e0030000 = "/fragment@0/__overlay__:timer:0"; >> }; >> }; >> >> But this will not apply: >> >> OF: resolver: overlay phandle fixup failed: -22 >> create_overlay: Failed to resolve tree >> >> > > Yes, it will not work as it is; my point is that you don't need the > magic __*__ node. > > You will need to modify the overlay application code to live insert a > phandle (if it doesn't exist) when it encounters a /path fixup. phandles only exist if something in the base tree refers to that node. Adding them when they don't exist should definitely be something we support for overlays. But don't call that a broken DT. That would be a separate issue. Rob -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] external references for device tree overlays @ 2017-06-08 6:51 ` Stefani Seibold 0 siblings, 0 replies; 27+ messages in thread From: Stefani Seibold @ 2017-06-08 6:51 UTC (permalink / raw) To: Rob Herring, Pantelis Antoniou Cc: Stefani Seibold, Frank Rowand, devicetree, linux-kernel, Holm Rauchfuss On Wed, 2017-06-07 at 17:19 -0500, Rob Herring wrote: > On Wed, Jun 7, 2017 at 3:11 AM, Pantelis Antoniou > <pantelis.antoniou@konsulko.com> wrote: > > Hi Stefani, > > > > On Tue, 2017-06-06 at 21:17 +0200, Stefani Seibold wrote: > > > Hi Pantelis, > > > > > > thanks for the suggestion. This feature is not very well > > > documented. I > > > tried this on my rasp1 running 4.12.0-rc3 and it doesn't work. My > > > source is: > > > > > > // rapsi example > > > /dts-v1/; > > > /plugin/; > > > > > > / { > > > compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; > > > > > > fragment@0 { > > > target-path = "/soc/i2s@7e203000"; > > > __overlay__ { > > > #address-cells = <0x00000001>; > > > #size-cells = <0x00000001>; > > > test = "test"; > > > timer = <&{/soc/timer@7e0030000}>; > > > }; > > > }; > > > }; > > > > > > > > > The resulting overlay is (decompiled with fdtdump): > > > > > > /dts-v1/; > > > // magic: 0xd00dfeed > > > // totalsize: 0x19a (410) > > > // off_dt_struct: 0x38 > > > // off_dt_strings: 0x148 > > > // off_mem_rsvmap: 0x28 > > > // version: 17 > > > // last_comp_version: 16 > > > // boot_cpuid_phys: 0x0 > > > // size_dt_strings: 0x52 > > > // size_dt_struct: 0x110 > > > > > > / { > > > compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; > > > fragment@0 { > > > target-path = "/soc/i2s@7e203000"; > > > __overlay__ { > > > #address-cells = <0x00000001>; > > > #size-cells = <0x00000001>; > > > test = "test"; > > > timer = <0xdeadbeef>; > > > }; > > > }; > > > __fixups__ { > > > /soc/timer@7e0030000 = "/fragment@0/__overlay__:timer:0"; > > > }; > > > }; > > > > > > But this will not apply: > > > > > > OF: resolver: overlay phandle fixup failed: -22 > > > create_overlay: Failed to resolve tree > > > > > > > > > > Yes, it will not work as it is; my point is that you don't need the > > magic __*__ node. > > > > You will need to modify the overlay application code to live insert > > a > > phandle (if it doesn't exist) when it encounters a /path fixup. > > phandles only exist if something in the base tree refers to that > node. > Adding them when they don't exist should definitely be something we > support for overlays. But don't call that a broken DT. That would be > a > separate issue. > Believe me it is broken. Due a NDA i am not able to give you more details about the vendor. But there forgot do provide an device node which must refer to the attached network and interrupt controller. - Stefani ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] external references for device tree overlays @ 2017-06-08 6:51 ` Stefani Seibold 0 siblings, 0 replies; 27+ messages in thread From: Stefani Seibold @ 2017-06-08 6:51 UTC (permalink / raw) To: Rob Herring, Pantelis Antoniou Cc: Stefani Seibold, Frank Rowand, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Holm Rauchfuss On Wed, 2017-06-07 at 17:19 -0500, Rob Herring wrote: > On Wed, Jun 7, 2017 at 3:11 AM, Pantelis Antoniou > <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote: > > Hi Stefani, > > > > On Tue, 2017-06-06 at 21:17 +0200, Stefani Seibold wrote: > > > Hi Pantelis, > > > > > > thanks for the suggestion. This feature is not very well > > > documented. I > > > tried this on my rasp1 running 4.12.0-rc3 and it doesn't work. My > > > source is: > > > > > > // rapsi example > > > /dts-v1/; > > > /plugin/; > > > > > > / { > > > compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; > > > > > > fragment@0 { > > > target-path = "/soc/i2s@7e203000"; > > > __overlay__ { > > > #address-cells = <0x00000001>; > > > #size-cells = <0x00000001>; > > > test = "test"; > > > timer = <&{/soc/timer@7e0030000}>; > > > }; > > > }; > > > }; > > > > > > > > > The resulting overlay is (decompiled with fdtdump): > > > > > > /dts-v1/; > > > // magic: 0xd00dfeed > > > // totalsize: 0x19a (410) > > > // off_dt_struct: 0x38 > > > // off_dt_strings: 0x148 > > > // off_mem_rsvmap: 0x28 > > > // version: 17 > > > // last_comp_version: 16 > > > // boot_cpuid_phys: 0x0 > > > // size_dt_strings: 0x52 > > > // size_dt_struct: 0x110 > > > > > > / { > > > compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; > > > fragment@0 { > > > target-path = "/soc/i2s@7e203000"; > > > __overlay__ { > > > #address-cells = <0x00000001>; > > > #size-cells = <0x00000001>; > > > test = "test"; > > > timer = <0xdeadbeef>; > > > }; > > > }; > > > __fixups__ { > > > /soc/timer@7e0030000 = "/fragment@0/__overlay__:timer:0"; > > > }; > > > }; > > > > > > But this will not apply: > > > > > > OF: resolver: overlay phandle fixup failed: -22 > > > create_overlay: Failed to resolve tree > > > > > > > > > > Yes, it will not work as it is; my point is that you don't need the > > magic __*__ node. > > > > You will need to modify the overlay application code to live insert > > a > > phandle (if it doesn't exist) when it encounters a /path fixup. > > phandles only exist if something in the base tree refers to that > node. > Adding them when they don't exist should definitely be something we > support for overlays. But don't call that a broken DT. That would be > a > separate issue. > Believe me it is broken. Due a NDA i am not able to give you more details about the vendor. But there forgot do provide an device node which must refer to the attached network and interrupt controller. - Stefani -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] external references for device tree overlays 2017-06-07 8:11 ` Pantelis Antoniou @ 2017-06-08 6:48 ` Stefani Seibold 2017-06-08 6:48 ` Stefani Seibold 1 sibling, 0 replies; 27+ messages in thread From: Stefani Seibold @ 2017-06-08 6:48 UTC (permalink / raw) To: Pantelis Antoniou Cc: Stefani Seibold, Rob Herring, Frank Rowand, devicetree, linux-kernel, Holm Rauchfuss Hi Pantelis, On Wed, 2017-06-07 at 11:11 +0300, Pantelis Antoniou wrote: > Hi Stefani, > > On Tue, 2017-06-06 at 21:17 +0200, Stefani Seibold wrote: > > Hi Pantelis, > > > > thanks for the suggestion. This feature is not very well > > documented. I > > tried this on my rasp1 running 4.12.0-rc3 and it doesn't work. My > > source is: > > > > // rapsi example > > /dts-v1/; > > /plugin/; > > > > / { > > compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; > > > > fragment@0 { > > target-path = "/soc/i2s@7e203000"; > > __overlay__ { > > #address-cells = <0x00000001>; > > #size-cells = <0x00000001>; > > test = "test"; > > timer = <&{/soc/timer@7e0030000}>; > > }; > > }; > > }; > > > > > > The resulting overlay is (decompiled with fdtdump): > > > > /dts-v1/; > > // magic: 0xd00dfeed > > // totalsize: 0x19a (410) > > // off_dt_struct: 0x38 > > // off_dt_strings: 0x148 > > // off_mem_rsvmap: 0x28 > > // version: 17 > > // last_comp_version: 16 > > // boot_cpuid_phys: 0x0 > > // size_dt_strings: 0x52 > > // size_dt_struct: 0x110 > > > > / { > > compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; > > fragment@0 { > > target-path = "/soc/i2s@7e203000"; > > __overlay__ { > > #address-cells = <0x00000001>; > > #size-cells = <0x00000001>; > > test = "test"; > > timer = <0xdeadbeef>; > > }; > > }; > > __fixups__ { > > /soc/timer@7e0030000 = "/fragment@0/__overlay__:timer:0"; > > }; > > }; > > > > But this will not apply: > > > > OF: resolver: overlay phandle fixup failed: -22 > > create_overlay: Failed to resolve tree > > > > > > Yes, it will not work as it is; my point is that you don't need the > magic __*__ node. > The magic __fixups__ node was inserted by the device tree compiler. I use the dtc from https://github.com/pantoniou/dtc at commit d990b8013889b816ec054c7e07a77db59c56c400. > You will need to modify the overlay application code to live insert a > phandle (if it doesn't exist) when it encounters a /path fixup. > That is part of my patch! > > Anyway, the reason for my patch is that i can reference to nodes > > which > > lacks a phandle. The phandle will be created on the fly and also > > destroyed when the overlay is unloaded. > > > > I have a real use case for this patch: > > > > I have a BIOS on some ARM64 servers which provides broken device > > tree. > > It also lacks some devices in this tree which needs references to > > other > > devices which lacks a phandle. > > > > Since the BIOSes are closed source i need a way to work arround > > this > > problem without patching all the drivers involved to this devices. > > > > Hope this helps to understand the reason for this patch. > > > > FWIW your problem seems like something that would happen on the > field. > We can berate the vendor of not providing the correct device tree, > but > in the end workarounds for broken vendor things are common in the > kernel. > Yes, that is the way how linux do the things. Linux has a long history to bypassing bugs of BIOSes, ACPI or broken devices. Greetings, Stefani ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] external references for device tree overlays @ 2017-06-08 6:48 ` Stefani Seibold 0 siblings, 0 replies; 27+ messages in thread From: Stefani Seibold @ 2017-06-08 6:48 UTC (permalink / raw) To: Pantelis Antoniou Cc: Stefani Seibold, Rob Herring, Frank Rowand, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Holm Rauchfuss Hi Pantelis, On Wed, 2017-06-07 at 11:11 +0300, Pantelis Antoniou wrote: > Hi Stefani, > > On Tue, 2017-06-06 at 21:17 +0200, Stefani Seibold wrote: > > Hi Pantelis, > > > > thanks for the suggestion. This feature is not very well > > documented. I > > tried this on my rasp1 running 4.12.0-rc3 and it doesn't work. My > > source is: > > > > // rapsi example > > /dts-v1/; > > /plugin/; > > > > / { > > compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; > > > > fragment@0 { > > target-path = "/soc/i2s@7e203000"; > > __overlay__ { > > #address-cells = <0x00000001>; > > #size-cells = <0x00000001>; > > test = "test"; > > timer = <&{/soc/timer@7e0030000}>; > > }; > > }; > > }; > > > > > > The resulting overlay is (decompiled with fdtdump): > > > > /dts-v1/; > > // magic: 0xd00dfeed > > // totalsize: 0x19a (410) > > // off_dt_struct: 0x38 > > // off_dt_strings: 0x148 > > // off_mem_rsvmap: 0x28 > > // version: 17 > > // last_comp_version: 16 > > // boot_cpuid_phys: 0x0 > > // size_dt_strings: 0x52 > > // size_dt_struct: 0x110 > > > > / { > > compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; > > fragment@0 { > > target-path = "/soc/i2s@7e203000"; > > __overlay__ { > > #address-cells = <0x00000001>; > > #size-cells = <0x00000001>; > > test = "test"; > > timer = <0xdeadbeef>; > > }; > > }; > > __fixups__ { > > /soc/timer@7e0030000 = "/fragment@0/__overlay__:timer:0"; > > }; > > }; > > > > But this will not apply: > > > > OF: resolver: overlay phandle fixup failed: -22 > > create_overlay: Failed to resolve tree > > > > > > Yes, it will not work as it is; my point is that you don't need the > magic __*__ node. > The magic __fixups__ node was inserted by the device tree compiler. I use the dtc from https://github.com/pantoniou/dtc at commit d990b8013889b816ec054c7e07a77db59c56c400. > You will need to modify the overlay application code to live insert a > phandle (if it doesn't exist) when it encounters a /path fixup. > That is part of my patch! > > Anyway, the reason for my patch is that i can reference to nodes > > which > > lacks a phandle. The phandle will be created on the fly and also > > destroyed when the overlay is unloaded. > > > > I have a real use case for this patch: > > > > I have a BIOS on some ARM64 servers which provides broken device > > tree. > > It also lacks some devices in this tree which needs references to > > other > > devices which lacks a phandle. > > > > Since the BIOSes are closed source i need a way to work arround > > this > > problem without patching all the drivers involved to this devices. > > > > Hope this helps to understand the reason for this patch. > > > > FWIW your problem seems like something that would happen on the > field. > We can berate the vendor of not providing the correct device tree, > but > in the end workarounds for broken vendor things are common in the > kernel. > Yes, that is the way how linux do the things. Linux has a long history to bypassing bugs of BIOSes, ACPI or broken devices. Greetings, Stefani -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] external references for device tree overlays @ 2017-06-11 23:04 ` Frank Rowand 0 siblings, 0 replies; 27+ messages in thread From: Frank Rowand @ 2017-06-11 23:04 UTC (permalink / raw) To: Stefani Seibold, Pantelis Antoniou Cc: Stefani Seibold, Rob Herring, devicetree, linux-kernel, Holm Rauchfuss On 06/07/17 23:48, Stefani Seibold wrote: > Hi Pantelis, > > On Wed, 2017-06-07 at 11:11 +0300, Pantelis Antoniou wrote: >> Hi Stefani, >> >> On Tue, 2017-06-06 at 21:17 +0200, Stefani Seibold wrote: >>> Hi Pantelis, >>> >>> thanks for the suggestion. This feature is not very well >>> documented. I >>> tried this on my rasp1 running 4.12.0-rc3 and it doesn't work. My >>> source is: >>> >>> // rapsi example >>> /dts-v1/; >>> /plugin/; >>> >>> / { >>> compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; >>> >>> fragment@0 { >>> target-path = "/soc/i2s@7e203000"; >>> __overlay__ { >>> #address-cells = <0x00000001>; >>> #size-cells = <0x00000001>; >>> test = "test"; >>> timer = <&{/soc/timer@7e0030000}>; >>> }; >>> }; >>> }; >>> >>> >>> The resulting overlay is (decompiled with fdtdump): >>> >>> /dts-v1/; >>> // magic: 0xd00dfeed >>> // totalsize: 0x19a (410) >>> // off_dt_struct: 0x38 >>> // off_dt_strings: 0x148 >>> // off_mem_rsvmap: 0x28 >>> // version: 17 >>> // last_comp_version: 16 >>> // boot_cpuid_phys: 0x0 >>> // size_dt_strings: 0x52 >>> // size_dt_struct: 0x110 >>> >>> / { >>> compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; >>> fragment@0 { >>> target-path = "/soc/i2s@7e203000"; >>> __overlay__ { >>> #address-cells = <0x00000001>; >>> #size-cells = <0x00000001>; >>> test = "test"; >>> timer = <0xdeadbeef>; >>> }; >>> }; >>> __fixups__ { >>> /soc/timer@7e0030000 = "/fragment@0/__overlay__:timer:0"; >>> }; >>> }; >>> >>> But this will not apply: >>> >>> OF: resolver: overlay phandle fixup failed: -22 >>> create_overlay: Failed to resolve tree >>> >>> >> >> Yes, it will not work as it is; my point is that you don't need the >> magic __*__ node. >> > > The magic __fixups__ node was inserted by the device tree compiler. I > use the dtc from https://github.com/pantoniou/dtc at commit > d990b8013889b816ec054c7e07a77db59c56c400. > >> You will need to modify the overlay application code to live insert a >> phandle (if it doesn't exist) when it encounters a /path fixup. >> > > That is part of my patch! > >>> Anyway, the reason for my patch is that i can reference to nodes >>> which >>> lacks a phandle. The phandle will be created on the fly and also >>> destroyed when the overlay is unloaded. >>> >>> I have a real use case for this patch: >>> >>> I have a BIOS on some ARM64 servers which provides broken device >>> tree. >>> It also lacks some devices in this tree which needs references to >>> other >>> devices which lacks a phandle. >>> >>> Since the BIOSes are closed source i need a way to work arround >>> this >>> problem without patching all the drivers involved to this devices. >>> >>> Hope this helps to understand the reason for this patch. >>> >> >> FWIW your problem seems like something that would happen on the >> field. >> We can berate the vendor of not providing the correct device tree, >> but >> in the end workarounds for broken vendor things are common in the >> kernel. >> > > Yes, that is the way how linux do the things. Linux has a long history > to bypassing bugs of BIOSes, ACPI or broken devices. ARM device tree in Linux is not like BIOSes or ACPI. ARM device tree in Linux is GPL v2 licensed (yes, it may also be dual licensed for other uses) and thus "free software". One of the key points of "free software" is that you have access to the source and the ability to modify it. Instead of bypassing device tree bugs, you have the ability to fix device tree bugs. This is a fundamental difference. -Frank > > Greetings, > Stefani > > ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] external references for device tree overlays @ 2017-06-11 23:04 ` Frank Rowand 0 siblings, 0 replies; 27+ messages in thread From: Frank Rowand @ 2017-06-11 23:04 UTC (permalink / raw) To: Stefani Seibold, Pantelis Antoniou Cc: Stefani Seibold, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Holm Rauchfuss On 06/07/17 23:48, Stefani Seibold wrote: > Hi Pantelis, > > On Wed, 2017-06-07 at 11:11 +0300, Pantelis Antoniou wrote: >> Hi Stefani, >> >> On Tue, 2017-06-06 at 21:17 +0200, Stefani Seibold wrote: >>> Hi Pantelis, >>> >>> thanks for the suggestion. This feature is not very well >>> documented. I >>> tried this on my rasp1 running 4.12.0-rc3 and it doesn't work. My >>> source is: >>> >>> // rapsi example >>> /dts-v1/; >>> /plugin/; >>> >>> / { >>> compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; >>> >>> fragment@0 { >>> target-path = "/soc/i2s@7e203000"; >>> __overlay__ { >>> #address-cells = <0x00000001>; >>> #size-cells = <0x00000001>; >>> test = "test"; >>> timer = <&{/soc/timer@7e0030000}>; >>> }; >>> }; >>> }; >>> >>> >>> The resulting overlay is (decompiled with fdtdump): >>> >>> /dts-v1/; >>> // magic: 0xd00dfeed >>> // totalsize: 0x19a (410) >>> // off_dt_struct: 0x38 >>> // off_dt_strings: 0x148 >>> // off_mem_rsvmap: 0x28 >>> // version: 17 >>> // last_comp_version: 16 >>> // boot_cpuid_phys: 0x0 >>> // size_dt_strings: 0x52 >>> // size_dt_struct: 0x110 >>> >>> / { >>> compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; >>> fragment@0 { >>> target-path = "/soc/i2s@7e203000"; >>> __overlay__ { >>> #address-cells = <0x00000001>; >>> #size-cells = <0x00000001>; >>> test = "test"; >>> timer = <0xdeadbeef>; >>> }; >>> }; >>> __fixups__ { >>> /soc/timer@7e0030000 = "/fragment@0/__overlay__:timer:0"; >>> }; >>> }; >>> >>> But this will not apply: >>> >>> OF: resolver: overlay phandle fixup failed: -22 >>> create_overlay: Failed to resolve tree >>> >>> >> >> Yes, it will not work as it is; my point is that you don't need the >> magic __*__ node. >> > > The magic __fixups__ node was inserted by the device tree compiler. I > use the dtc from https://github.com/pantoniou/dtc at commit > d990b8013889b816ec054c7e07a77db59c56c400. > >> You will need to modify the overlay application code to live insert a >> phandle (if it doesn't exist) when it encounters a /path fixup. >> > > That is part of my patch! > >>> Anyway, the reason for my patch is that i can reference to nodes >>> which >>> lacks a phandle. The phandle will be created on the fly and also >>> destroyed when the overlay is unloaded. >>> >>> I have a real use case for this patch: >>> >>> I have a BIOS on some ARM64 servers which provides broken device >>> tree. >>> It also lacks some devices in this tree which needs references to >>> other >>> devices which lacks a phandle. >>> >>> Since the BIOSes are closed source i need a way to work arround >>> this >>> problem without patching all the drivers involved to this devices. >>> >>> Hope this helps to understand the reason for this patch. >>> >> >> FWIW your problem seems like something that would happen on the >> field. >> We can berate the vendor of not providing the correct device tree, >> but >> in the end workarounds for broken vendor things are common in the >> kernel. >> > > Yes, that is the way how linux do the things. Linux has a long history > to bypassing bugs of BIOSes, ACPI or broken devices. ARM device tree in Linux is not like BIOSes or ACPI. ARM device tree in Linux is GPL v2 licensed (yes, it may also be dual licensed for other uses) and thus "free software". One of the key points of "free software" is that you have access to the source and the ability to modify it. Instead of bypassing device tree bugs, you have the ability to fix device tree bugs. This is a fundamental difference. -Frank > > Greetings, > Stefani > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] external references for device tree overlays 2017-06-05 12:59 ` Stefani Seibold (?) (?) @ 2017-06-06 7:20 ` Frank Rowand 2017-06-06 16:12 ` Frank Rowand 2017-06-06 19:22 ` Stefani Seibold -1 siblings, 2 replies; 27+ messages in thread From: Frank Rowand @ 2017-06-06 7:20 UTC (permalink / raw) To: Stefani Seibold, Stefani Seibold, Pantelis Antoniou, Rob Herring, devicetree, linux-kernel Cc: Holm Rauchfuss On 06/05/17 05:59, Stefani Seibold wrote: > From: Stefani Seibold <stefani@seibold.net> > > This patch enables external references for symbols which are not > exported by the current device tree. For example > > // RASPI example (only for testing) > /dts-v1/; > /plugin/; > > / { > compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; > > fragment@0 { > target-path = "/soc/i2s@7e203000"; > __overlay__ { > #address-cells = <0x00000001>; > #size-cells = <0x00000001>; > test = "test"; > timer = <&timer>; > }; > }; > > __external_symbols__ { > timer = "/soc/timer@7e003000"; > }; > }; My hope is that the dtc compiler will stop supporting specification of the __symbols__ node in dts source, and only generate it automatically in the dtb. That change to dtc would not allow any node name specified in a dts to begin with an underscore. Thus node __external_symbols__ would not be allowed. > > The "timer" symbol is not exported by the RASPI device tree, because it is > missing in the __symbols__ section of the device tree. > > In case of the RASPI device tree this could be simple fixed by modifing > the device tree source, but when the device tree is provided by a closed > source BIOS this kind of missing symbol could not be fixed. Is there a real example of this issue, or is this a theoretical concern? If this is a real example, we should be discouraging such behavior. The suggestion by Pantelis should work, but that is just a hack to get you out of a bad situation, not a good practice. > > An additional benefit is to override a (possible broken) symbol exported > by the currect live device tree. > > The patch is based and tested on linux 4.12-rc3. > > Signed-off-by: Stefani Seibold <stefani.seibold.ext@huawei.com> > Signed-off-by: Stefani Seibold <stefani@seibold.net> > --- > drivers/of/overlay.c | 19 +++++++++++++++++++ > drivers/of/resolver.c | 27 ++++++++++++++++++++++----- > 2 files changed, 41 insertions(+), 5 deletions(-) > > diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c > index 7827786718d8..de6516ea0fcd 100644 > --- a/drivers/of/overlay.c > +++ b/drivers/of/overlay.c < snip > -Frank ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] external references for device tree overlays @ 2017-06-06 16:12 ` Frank Rowand 0 siblings, 0 replies; 27+ messages in thread From: Frank Rowand @ 2017-06-06 16:12 UTC (permalink / raw) To: Stefani Seibold, Stefani Seibold, Pantelis Antoniou, Rob Herring, devicetree, linux-kernel Cc: Holm Rauchfuss On 06/06/17 00:20, Frank Rowand wrote: > On 06/05/17 05:59, Stefani Seibold wrote: >> From: Stefani Seibold <stefani@seibold.net> >> >> This patch enables external references for symbols which are not >> exported by the current device tree. For example >> >> // RASPI example (only for testing) >> /dts-v1/; >> /plugin/; >> >> / { >> compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; >> >> fragment@0 { >> target-path = "/soc/i2s@7e203000"; >> __overlay__ { >> #address-cells = <0x00000001>; >> #size-cells = <0x00000001>; >> test = "test"; >> timer = <&timer>; >> }; >> }; >> >> __external_symbols__ { >> timer = "/soc/timer@7e003000"; >> }; >> }; > > My hope is that the dtc compiler will stop supporting specification of the > __symbols__ node in dts source, and only generate it automatically in the dtb. > That change to dtc would not allow any node name specified in a dts to begin > with an underscore. Thus node __external_symbols__ would not be allowed. > > >> >> The "timer" symbol is not exported by the RASPI device tree, because it is >> missing in the __symbols__ section of the device tree. >> >> In case of the RASPI device tree this could be simple fixed by modifing >> the device tree source, but when the device tree is provided by a closed >> source BIOS this kind of missing symbol could not be fixed. > > Is there a real example of this issue, or is this a theoretical concern? > If this is a real example, we should be discouraging such behavior. > > The suggestion by Pantelis should work, but that is just a hack to get > you out of a bad situation, not a good practice. Digging a little bit deeper into the suggestion by Pantelis, I don't like that approach either. It is still rather hacky. > >> >> An additional benefit is to override a (possible broken) symbol exported >> by the currect live device tree. >> >> The patch is based and tested on linux 4.12-rc3. >> >> Signed-off-by: Stefani Seibold <stefani.seibold.ext@huawei.com> >> Signed-off-by: Stefani Seibold <stefani@seibold.net> >> --- >> drivers/of/overlay.c | 19 +++++++++++++++++++ >> drivers/of/resolver.c | 27 ++++++++++++++++++++++----- >> 2 files changed, 41 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c >> index 7827786718d8..de6516ea0fcd 100644 >> --- a/drivers/of/overlay.c >> +++ b/drivers/of/overlay.c > > < snip > > > -Frank > ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] external references for device tree overlays @ 2017-06-06 16:12 ` Frank Rowand 0 siblings, 0 replies; 27+ messages in thread From: Frank Rowand @ 2017-06-06 16:12 UTC (permalink / raw) To: Stefani Seibold, Stefani Seibold, Pantelis Antoniou, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA Cc: Holm Rauchfuss On 06/06/17 00:20, Frank Rowand wrote: > On 06/05/17 05:59, Stefani Seibold wrote: >> From: Stefani Seibold <stefani-mkwtCZVSLSnR7s880joybQ@public.gmane.org> >> >> This patch enables external references for symbols which are not >> exported by the current device tree. For example >> >> // RASPI example (only for testing) >> /dts-v1/; >> /plugin/; >> >> / { >> compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; >> >> fragment@0 { >> target-path = "/soc/i2s@7e203000"; >> __overlay__ { >> #address-cells = <0x00000001>; >> #size-cells = <0x00000001>; >> test = "test"; >> timer = <&timer>; >> }; >> }; >> >> __external_symbols__ { >> timer = "/soc/timer@7e003000"; >> }; >> }; > > My hope is that the dtc compiler will stop supporting specification of the > __symbols__ node in dts source, and only generate it automatically in the dtb. > That change to dtc would not allow any node name specified in a dts to begin > with an underscore. Thus node __external_symbols__ would not be allowed. > > >> >> The "timer" symbol is not exported by the RASPI device tree, because it is >> missing in the __symbols__ section of the device tree. >> >> In case of the RASPI device tree this could be simple fixed by modifing >> the device tree source, but when the device tree is provided by a closed >> source BIOS this kind of missing symbol could not be fixed. > > Is there a real example of this issue, or is this a theoretical concern? > If this is a real example, we should be discouraging such behavior. > > The suggestion by Pantelis should work, but that is just a hack to get > you out of a bad situation, not a good practice. Digging a little bit deeper into the suggestion by Pantelis, I don't like that approach either. It is still rather hacky. > >> >> An additional benefit is to override a (possible broken) symbol exported >> by the currect live device tree. >> >> The patch is based and tested on linux 4.12-rc3. >> >> Signed-off-by: Stefani Seibold <stefani.seibold.ext-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> >> Signed-off-by: Stefani Seibold <stefani-mkwtCZVSLSnR7s880joybQ@public.gmane.org> >> --- >> drivers/of/overlay.c | 19 +++++++++++++++++++ >> drivers/of/resolver.c | 27 ++++++++++++++++++++++----- >> 2 files changed, 41 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c >> index 7827786718d8..de6516ea0fcd 100644 >> --- a/drivers/of/overlay.c >> +++ b/drivers/of/overlay.c > > < snip > > > -Frank > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] external references for device tree overlays 2017-06-06 7:20 ` Frank Rowand 2017-06-06 16:12 ` Frank Rowand @ 2017-06-06 19:22 ` Stefani Seibold 2017-06-07 0:46 ` Frank Rowand 1 sibling, 1 reply; 27+ messages in thread From: Stefani Seibold @ 2017-06-06 19:22 UTC (permalink / raw) To: Frank Rowand, Stefani Seibold, Pantelis Antoniou, Rob Herring, devicetree, linux-kernel Cc: Holm Rauchfuss Hi Frank, On 06.06.2017, 00:20 -0700 Frank Rowand wrote:: > On 06/05/17 05:59, Stefani Seibold wrote: > > From: Stefani Seibold <stefani@seibold.net> > > > > This patch enables external references for symbols which are not > > exported by the current device tree. For example > > > > // RASPI example (only for testing) > > /dts-v1/; > > /plugin/; > > > > / { > > compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; > > > > fragment@0 { > > target-path = "/soc/i2s@7e203000"; > > __overlay__ { > > #address-cells = <0x00000001>; > > #size-cells = <0x00000001>; > > test = "test"; > > timer = <&timer>; > > }; > > }; > > > > __external_symbols__ { > > timer = "/soc/timer@7e003000"; > > }; > > }; > > My hope is that the dtc compiler will stop supporting specification > of the > __symbols__ node in dts source, and only generate it automatically in > the dtb. > That change to dtc would not allow any node name specified in a dts > to begin > with an underscore. Thus node __external_symbols__ would not be > allowed. > The name is not so important to me, only the solution. > > In case of the RASPI device tree this could be simple fixed by > > modifing > > the device tree source, but when the device tree is provided by a > > closed > > source BIOS this kind of missing symbol could not be fixed. > > Is there a real example of this issue, or is this a theoretical > concern? > If this is a real example, we should be discouraging such behavior. > Yes, I have a BIOS on some ARM64 servers which provides broken device tree. It also lacks some devices in this tree which needs references to other devices which lacks a phandle. > The suggestion by Pantelis should work, but that is just a hack to > get > you out of a bad situation, not a good practice. > I tried it, but it doesn't work. Look at my post to Pantelis. - Stefani ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] external references for device tree overlays @ 2017-06-07 0:46 ` Frank Rowand 0 siblings, 0 replies; 27+ messages in thread From: Frank Rowand @ 2017-06-07 0:46 UTC (permalink / raw) To: Stefani Seibold, Stefani Seibold, Pantelis Antoniou, Rob Herring, devicetree, linux-kernel Cc: Holm Rauchfuss On 06/06/17 12:22, Stefani Seibold wrote: > Hi Frank, > > On 06.06.2017, 00:20 -0700 Frank Rowand wrote:: >> On 06/05/17 05:59, Stefani Seibold wrote: >>> From: Stefani Seibold <stefani@seibold.net> >>> >>> This patch enables external references for symbols which are not >>> exported by the current device tree. For example >>> >>> // RASPI example (only for testing) >>> /dts-v1/; >>> /plugin/; >>> >>> / { >>> compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; >>> >>> fragment@0 { >>> target-path = "/soc/i2s@7e203000"; >>> __overlay__ { >>> #address-cells = <0x00000001>; >>> #size-cells = <0x00000001>; >>> test = "test"; >>> timer = <&timer>; >>> }; >>> }; >>> >>> __external_symbols__ { >>> timer = "/soc/timer@7e003000"; >>> }; >>> }; >> >> My hope is that the dtc compiler will stop supporting specification >> of the >> __symbols__ node in dts source, and only generate it automatically in >> the dtb. >> That change to dtc would not allow any node name specified in a dts >> to begin >> with an underscore. Thus node __external_symbols__ would not be >> allowed. >> > > The name is not so important to me, only the solution. > >>> In case of the RASPI device tree this could be simple fixed by >>> modifing >>> the device tree source, but when the device tree is provided by a >>> closed >>> source BIOS this kind of missing symbol could not be fixed. >> >> Is there a real example of this issue, or is this a theoretical >> concern? >> If this is a real example, we should be discouraging such behavior. >> > > Yes, I have a BIOS on some ARM64 servers which provides broken device > tree. It also lacks some devices in this tree which needs references > to other devices which lacks a phandle. Jon Masters is pushing a message that if the firmware on your arm64 server is broken, then insist that the vendor fix it. I think he was talking about ACPI, but the same message should also apply to device tree. If you are having trouble getting your vendor to fix it, ask Jon if he is willing to help apply pressure. >> The suggestion by Pantelis should work, but that is just a hack to >> get >> you out of a bad situation, not a good practice. >> > > I tried it, but it doesn't work. Look at my post to Pantelis. Yes, I realized that the method Pantelis gave would also require a code change. I don't like that code change either. > > - Stefani > . > ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] external references for device tree overlays @ 2017-06-07 0:46 ` Frank Rowand 0 siblings, 0 replies; 27+ messages in thread From: Frank Rowand @ 2017-06-07 0:46 UTC (permalink / raw) To: Stefani Seibold, Stefani Seibold, Pantelis Antoniou, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA Cc: Holm Rauchfuss On 06/06/17 12:22, Stefani Seibold wrote: > Hi Frank, > > On 06.06.2017, 00:20 -0700 Frank Rowand wrote:: >> On 06/05/17 05:59, Stefani Seibold wrote: >>> From: Stefani Seibold <stefani-mkwtCZVSLSnR7s880joybQ@public.gmane.org> >>> >>> This patch enables external references for symbols which are not >>> exported by the current device tree. For example >>> >>> // RASPI example (only for testing) >>> /dts-v1/; >>> /plugin/; >>> >>> / { >>> compatible = "brcm,bcm2835", "brcm,bcm2708", "brcm,bcm2709"; >>> >>> fragment@0 { >>> target-path = "/soc/i2s@7e203000"; >>> __overlay__ { >>> #address-cells = <0x00000001>; >>> #size-cells = <0x00000001>; >>> test = "test"; >>> timer = <&timer>; >>> }; >>> }; >>> >>> __external_symbols__ { >>> timer = "/soc/timer@7e003000"; >>> }; >>> }; >> >> My hope is that the dtc compiler will stop supporting specification >> of the >> __symbols__ node in dts source, and only generate it automatically in >> the dtb. >> That change to dtc would not allow any node name specified in a dts >> to begin >> with an underscore. Thus node __external_symbols__ would not be >> allowed. >> > > The name is not so important to me, only the solution. > >>> In case of the RASPI device tree this could be simple fixed by >>> modifing >>> the device tree source, but when the device tree is provided by a >>> closed >>> source BIOS this kind of missing symbol could not be fixed. >> >> Is there a real example of this issue, or is this a theoretical >> concern? >> If this is a real example, we should be discouraging such behavior. >> > > Yes, I have a BIOS on some ARM64 servers which provides broken device > tree. It also lacks some devices in this tree which needs references > to other devices which lacks a phandle. Jon Masters is pushing a message that if the firmware on your arm64 server is broken, then insist that the vendor fix it. I think he was talking about ACPI, but the same message should also apply to device tree. If you are having trouble getting your vendor to fix it, ask Jon if he is willing to help apply pressure. >> The suggestion by Pantelis should work, but that is just a hack to >> get >> you out of a bad situation, not a good practice. >> > > I tried it, but it doesn't work. Look at my post to Pantelis. Yes, I realized that the method Pantelis gave would also require a code change. I don't like that code change either. > > - Stefani > . > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <1496815399.6999.1.camel@seibold.net>]
[parent not found: <1496815399.6999.1.camel-mkwtCZVSLSnR7s880joybQ@public.gmane.org>]
* Re: [PATCH] external references for device tree overlays [not found] ` <1496815399.6999.1.camel-mkwtCZVSLSnR7s880joybQ@public.gmane.org> @ 2017-06-08 3:07 ` Frank Rowand [not found] ` <5938BF8F.2010001-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 27+ messages in thread From: Frank Rowand @ 2017-06-08 3:07 UTC (permalink / raw) To: Stefani Seibold, jcm-Zp4isUonpHBD60Wz+7aTrA Cc: Stefani Seibold, Pantelis Antoniou, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA Adding Jon. Adding back the original distribution. Hi Stefani, On 06/06/17 23:03, Stefani Seibold wrote: > Am Dienstag, den 06.06.2017, 17:46 -0700 schrieb Frank Rowand: >> On 06/06/17 12:22, Stefani Seibold wrote: >>> Hi Frank, >>> >>> On 06.06.2017, 00:20 -0700 Frank Rowand wrote:: >>>> On 06/05/17 05:59, Stefani Seibold wrote: >>>>> From: Stefani Seibold <stefani-mkwtCZVSLSnR7s880joybQ@public.gmane.org> >>>>> >>>>> This patch enables external references for symbols which are >>>>> not >>>>> exported by the current device tree. For example >>>>> >>>>> // RASPI example (only for testing) >>>>> /dts-v1/; >>>>> /plugin/; >>>>> >>>>> / { >>>>> compatible = "brcm,bcm2835", "brcm,bcm2708", >>>>> "brcm,bcm2709"; >>>>> >>>>> fragment@0 { >>>>> target-path = "/soc/i2s@7e203000"; >>>>> __overlay__ { >>>>> #address-cells = <0x00000001>; >>>>> #size-cells = <0x00000001>; >>>>> test = "test"; >>>>> timer = <&timer>; >>>>> }; >>>>> }; >>>>> >>>>> __external_symbols__ { >>>>> timer = "/soc/timer@7e003000"; >>>>> }; >>>>> }; >>>> >>>> My hope is that the dtc compiler will stop supporting >>>> specification of the __symbols__ node in dts source, and only >>>> generate it automatically in the dtb. That change to dtc would >>>> not allow any node name specified in a dts to begin with an >>>> underscore. Thus node __external_symbols__ would not be >>>> allowed. >>>> >>> >>> The name is not so important to me, only the solution. >>> >>>>> In case of the RASPI device tree this could be simple fixed >>>>> by modifing the device tree source, but when the device tree >>>>> is provided by a closed source BIOS this kind of missing >>>>> symbol could not be fixed. >>>> >>>> Is there a real example of this issue, or is this a >>>> theoretical concern? If this is a real example, we should be >>>> discouraging such behavior. >>>> >>> >>> Yes, I have a BIOS on some ARM64 servers which provides broken >>> device tree. It also lacks some devices in this tree which needs >>> references to other devices which lacks a phandle. I want to make sure I understand the use case, or cases. My interpretation of the patch email is that there is an ARM64 server with a device tree that is missing a single symbol in the __symbols__ node. I could also read that email as saying that there is no __symbols__ node in the device tree. Which is the actual case? >> Jon Masters is pushing a message that if the firmware on your >> arm64 server is broken, then insist that the vendor fix it. > > Well with this kind of arguments you can also sit and wait until a > vendor decided to provide a kernel driver for an device. As we know > this would in 99% percent of the cases never happened. > > So that is not a solution if the vendor does not do it. Fu.... > closed source is as it is. So i need a solution and i think other > developer to. > >> I think he was talking about ACPI, but the same message should also >> apply to device tree. Jon, are you also interested in broken device trees? > > I talk about a device tree, not ACPI! > >> If you are having trouble getting your vendor to fix it, ask Jon >> if he is willing to help apply pressure. >> > > My vendor will not fix it. Believe me. If you are not going to try to get your vendor to fix it (or have already tried and failed), can you tell us the name and model of the computer so that other people can be aware of the broken device tree? And can you describe the specific way the device tree is broken? > And again: We need a solution when a closed source BIOS or > bootloader is not fixed by the vendor, which is also a case for > devices like single board computer, settop boxes, tv, smartphones and > tablets. The patch does not fix the general case of a broken device tree. It only potentially addresses a missing symbol from the base device tree that is needed for an overlay. The general case is an entirely different discussion. > And my solution is a very tiny one, which works exactly in the same > way as the __symbols__ node. > >> >>>> The suggestion by Pantelis should work, but that is just a >>>> hack to get you out of a bad situation, not a good practice. >>>> >>> >>> I tried it, but it doesn't work. Look at my post to Pantelis. >> >> Yes, I realized that the method Pantelis gave would also require a >> code change. I don't like that code change either. >> > > If you have an other solution than provide it. Otherwise please > apply the patch. Lack of me providing another solution is not a reason to apply a patch that I disagree with. But here is another solution: decompile the broken device tree and apply the fix to the decompiled tree. This can be a bit challenging since the original labels will be missing in the decompiled tree (though you will be able to get them from the __overlays__ node if that exists). -Frank > > - Stefani -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <5938BF8F.2010001-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH] external references for device tree overlays [not found] ` <5938BF8F.2010001-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2017-06-08 7:08 ` Pantelis Antoniou 2017-06-08 7:08 ` Stefani Seibold 1 sibling, 0 replies; 27+ messages in thread From: Pantelis Antoniou @ 2017-06-08 7:08 UTC (permalink / raw) To: Frank Rowand Cc: Stefani Seibold, jcm-Zp4isUonpHBD60Wz+7aTrA, Stefani Seibold, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA Hi Frank, On Wed, 2017-06-07 at 20:07 -0700, Frank Rowand wrote: > Adding Jon. > [snip] > __overlays__ node if that exists). > For what is worth, I will shortly post a method for fixing up busted or incomplete device trees. It is unrelated to this specific problem but I think addresses that too (with a couple minor fixes). The general problem has to do with phandles. We do have a method to apply overlays to modify the content of the tree, but we lack a method to add missing phandles to nodes that should have them. The first missing part of the puzzle is to pick up symbols from applied overlays and then work from there. > -Frank > > > > > - Stefani Regards -- Pantelis -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] external references for device tree overlays [not found] ` <5938BF8F.2010001-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2017-06-08 7:08 ` Pantelis Antoniou @ 2017-06-08 7:08 ` Stefani Seibold [not found] ` <1496905725.7999.5.camel-mkwtCZVSLSnR7s880joybQ@public.gmane.org> 1 sibling, 1 reply; 27+ messages in thread From: Stefani Seibold @ 2017-06-08 7:08 UTC (permalink / raw) To: Frank Rowand, jcm-Zp4isUonpHBD60Wz+7aTrA Cc: Stefani Seibold, Pantelis Antoniou, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA Hi Frank, On Wed, 2017-06-07 at 20:07 -0700, Frank Rowand wrote: > Adding Jon. > > Adding back the original distribution. > > Hi Stefani, > > On 06/06/17 23:03, Stefani Seibold wrote: > > Am Dienstag, den 06.06.2017, 17:46 -0700 schrieb Frank Rowand: > > > On 06/06/17 12:22, Stefani Seibold wrote: > > > > Hi Frank, > > > > > > > > On 06.06.2017, 00:20 -0700 Frank Rowand wrote:: > > > > > On 06/05/17 05:59, Stefani Seibold wrote: > > > > > > From: Stefani Seibold <stefani-mkwtCZVSLSnR7s880joybQ@public.gmane.org> > > > > > > > > > > > > This patch enables external references for symbols which > > > > > > are > > > > > > not > > > > > > exported by the current device tree. For example > > > > > > > > > > > > // RASPI example (only for testing) > > > > > > /dts-v1/; > > > > > > /plugin/; > > > > > > > > > > > > / { > > > > > > compatible = "brcm,bcm2835", "brcm,bcm2708", > > > > > > "brcm,bcm2709"; > > > > > > > > > > > > fragment@0 { > > > > > > target-path = "/soc/i2s@7e203000"; > > > > > > __overlay__ { > > > > > > #address-cells = <0x00000001>; > > > > > > #size-cells = <0x00000001>; > > > > > > test = "test"; > > > > > > timer = <&timer>; > > > > > > }; > > > > > > }; > > > > > > > > > > > > __external_symbols__ { > > > > > > timer = "/soc/timer@7e003000"; > > > > > > }; > > > > > > }; > > > > > > > > > > My hope is that the dtc compiler will stop supporting > > > > > specification of the __symbols__ node in dts source, and only > > > > > generate it automatically in the dtb. That change to dtc > > > > > would > > > > > not allow any node name specified in a dts to begin with an > > > > > underscore. Thus node __external_symbols__ would not be > > > > > allowed. > > > > > > > > > > > > > The name is not so important to me, only the solution. > > > > > > > > > > In case of the RASPI device tree this could be simple fixed > > > > > > by modifing the device tree source, but when the device > > > > > > tree > > > > > > is provided by a closed source BIOS this kind of missing > > > > > > symbol could not be fixed. > > > > > > > > > > Is there a real example of this issue, or is this a > > > > > theoretical concern? If this is a real example, we should be > > > > > discouraging such behavior. > > > > > > > > > > > > > Yes, I have a BIOS on some ARM64 servers which provides broken > > > > device tree. It also lacks some devices in this tree which > > > > needs > > > > references to other devices which lacks a phandle. > > I want to make sure I understand the use case, or cases. > > My interpretation of the patch email is that there is an ARM64 server > with a device tree that is missing a single symbol in the __symbols__ > node. I could also read that email as saying that there is no > __symbols__ node in the device tree. Which is the actual case? > > > > > Jon Masters is pushing a message that if the firmware on your > > > arm64 server is broken, then insist that the vendor fix it. > > > > Well with this kind of arguments you can also sit and wait until a > > vendor decided to provide a kernel driver for an device. As we > > know > > this would in 99% percent of the cases never happened. > > > > So that is not a solution if the vendor does not do it. Fu.... > > closed source is as it is. So i need a solution and i think other > > developer to. > > > > > I think he was talking about ACPI, but the same message should > > > also > > > apply to device tree. > > Jon, are you also interested in broken device trees? > > > > > > I talk about a device tree, not ACPI! > > > > > If you are having trouble getting your vendor to fix it, ask Jon > > > if he is willing to help apply pressure. > > > > > > > My vendor will not fix it. Believe me. > > If you are not going to try to get your vendor to fix it (or have > already tried and failed), can you tell us the name and model of the > computer so that other people can be aware of the broken device tree? Due an NDA i am not allowed to give you this kind of information. > And can you describe the specific way the device tree is broken? > > In this case the device tree is broken due a missing device node which must refer to the network interface(s) and interrupt controller. Since there is no symbols for the network interfaces and there are also missing a phandle, this information must be generated on the fly. And that is what my patch do: provide an path to the node (symbol) and generate the phandle on the fly. > > And again: We need a solution when a closed source BIOS or > > bootloader is not fixed by the vendor, which is also a case for > > devices like single board computer, settop boxes, tv, smartphones > > and > > tablets. > > The patch does not fix the general case of a broken device tree. It > only potentially addresses a missing symbol from the base device tree > that is needed for an overlay. The general case is an entirely > different discussion. > IMHO i think this is the last puzzle piece for an general solution. All other things are there: Overlays and Symbols. With this patch i can overwrite and modify any part of a device tree. But here is another solution: decompile the broken device tree > and apply the fix to the decompiled tree. This can be a bit > challenging since the original labels will be missing in the > decompiled tree (though you will be able to get them from the > __overlays__ node if that exists). > You are wrong. This will not work as a general solution due some important information are provided by the BIOS, like the MAC addresses and the memory layout and the NUMA nodes and the IDs for RoCE and so one. In this case i need to decompile a lot of machines and patch them. And every time the memory is changed i must do this again. Thats is not feasible solution and very error prone. > Lack of me providing another solution is not a reason to apply a > patch that I disagree with. The patch works exact the same way as the current __symbols__ node. It is very tiny and easy to review and has absolut no side effects. If you have no technical objections you should apply it until you have a better solution. - Stefani -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <1496905725.7999.5.camel-mkwtCZVSLSnR7s880joybQ@public.gmane.org>]
* Re: [PATCH] external references for device tree overlays [not found] ` <1496905725.7999.5.camel-mkwtCZVSLSnR7s880joybQ@public.gmane.org> @ 2017-06-11 23:14 ` Frank Rowand [not found] ` <1497248029.9234.1.camel@seibold.net> 0 siblings, 1 reply; 27+ messages in thread From: Frank Rowand @ 2017-06-11 23:14 UTC (permalink / raw) To: Stefani Seibold, jcm-Zp4isUonpHBD60Wz+7aTrA Cc: Stefani Seibold, Pantelis Antoniou, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA On 06/08/17 00:08, Stefani Seibold wrote: > Hi Frank, > > > On Wed, 2017-06-07 at 20:07 -0700, Frank Rowand wrote: >> Adding Jon. >> >> Adding back the original distribution. >> >> Hi Stefani, >> >> On 06/06/17 23:03, Stefani Seibold wrote: >>> Am Dienstag, den 06.06.2017, 17:46 -0700 schrieb Frank Rowand: >>>> On 06/06/17 12:22, Stefani Seibold wrote: >>>>> Hi Frank, >>>>> >>>>> On 06.06.2017, 00:20 -0700 Frank Rowand wrote:: >>>>>> On 06/05/17 05:59, Stefani Seibold wrote: >>>>>>> From: Stefani Seibold <stefani-mkwtCZVSLSnR7s880joybQ@public.gmane.org> >>>>>>> >>>>>>> This patch enables external references for symbols which >>>>>>> are >>>>>>> not >>>>>>> exported by the current device tree. For example >>>>>>> >>>>>>> // RASPI example (only for testing) >>>>>>> /dts-v1/; >>>>>>> /plugin/; >>>>>>> >>>>>>> / { >>>>>>> compatible = "brcm,bcm2835", "brcm,bcm2708", >>>>>>> "brcm,bcm2709"; >>>>>>> >>>>>>> fragment@0 { >>>>>>> target-path = "/soc/i2s@7e203000"; >>>>>>> __overlay__ { >>>>>>> #address-cells = <0x00000001>; >>>>>>> #size-cells = <0x00000001>; >>>>>>> test = "test"; >>>>>>> timer = <&timer>; >>>>>>> }; >>>>>>> }; >>>>>>> >>>>>>> __external_symbols__ { >>>>>>> timer = "/soc/timer@7e003000"; >>>>>>> }; >>>>>>> }; >>>>>> >>>>>> My hope is that the dtc compiler will stop supporting >>>>>> specification of the __symbols__ node in dts source, and only >>>>>> generate it automatically in the dtb. That change to dtc >>>>>> would >>>>>> not allow any node name specified in a dts to begin with an >>>>>> underscore. Thus node __external_symbols__ would not be >>>>>> allowed. >>>>>> >>>>> >>>>> The name is not so important to me, only the solution. >>>>> >>>>>>> In case of the RASPI device tree this could be simple fixed >>>>>>> by modifing the device tree source, but when the device >>>>>>> tree >>>>>>> is provided by a closed source BIOS this kind of missing >>>>>>> symbol could not be fixed. >>>>>> >>>>>> Is there a real example of this issue, or is this a >>>>>> theoretical concern? If this is a real example, we should be >>>>>> discouraging such behavior. >>>>>> >>>>> >>>>> Yes, I have a BIOS on some ARM64 servers which provides broken >>>>> device tree. It also lacks some devices in this tree which >>>>> needs >>>>> references to other devices which lacks a phandle. >> >> I want to make sure I understand the use case, or cases. >> >> My interpretation of the patch email is that there is an ARM64 server >> with a device tree that is missing a single symbol in the __symbols__ >> node. I could also read that email as saying that there is no >> __symbols__ node in the device tree. Which is the actual case? >> >> >>>> Jon Masters is pushing a message that if the firmware on your >>>> arm64 server is broken, then insist that the vendor fix it. >>> >>> Well with this kind of arguments you can also sit and wait until a >>> vendor decided to provide a kernel driver for an device. As we >>> know >>> this would in 99% percent of the cases never happened. >>> >>> So that is not a solution if the vendor does not do it. Fu.... >>> closed source is as it is. So i need a solution and i think other >>> developer to. >>> >>>> I think he was talking about ACPI, but the same message should >>>> also >>>> apply to device tree. >> >> Jon, are you also interested in broken device trees? >> >> >>> >>> I talk about a device tree, not ACPI! >>> >>>> If you are having trouble getting your vendor to fix it, ask Jon >>>> if he is willing to help apply pressure. >>>> >>> >>> My vendor will not fix it. Believe me. >> >> If you are not going to try to get your vendor to fix it (or have >> already tried and failed), can you tell us the name and model of the >> computer so that other people can be aware of the broken device tree? > > Due an NDA i am not allowed to give you this kind of information. > >> And can you describe the specific way the device tree is broken? >> >> > > In this case the device tree is broken due a missing device node which > must refer to the network interface(s) and interrupt controller. > > Since there is no symbols for the network interfaces and there are also > missing a phandle, this information must be generated on the fly. > > And that is what my patch do: provide an path to the node (symbol) and > generate the phandle on the fly. > >>> And again: We need a solution when a closed source BIOS or >>> bootloader is not fixed by the vendor, which is also a case for >>> devices like single board computer, settop boxes, tv, smartphones >>> and >>> tablets. >> >> The patch does not fix the general case of a broken device tree. It >> only potentially addresses a missing symbol from the base device tree >> that is needed for an overlay. The general case is an entirely >> different discussion. >> > > IMHO i think this is the last puzzle piece for an general solution. All > other things are there: Overlays and Symbols. > > With this patch i can overwrite and modify any part of a device tree. > > But here is another solution: decompile the broken device tree >> and apply the fix to the decompiled tree. This can be a bit >> challenging since the original labels will be missing in the >> decompiled tree (though you will be able to get them from the >> __overlays__ node if that exists). >> > > You are wrong. This will not work as a general solution due some > important information are provided by the BIOS, like the MAC addresses > and the memory layout and the NUMA nodes and the IDs for RoCE and so > one. > > In this case i need to decompile a lot of machines and patch them. And > every time the memory is changed i must do this again. Thats is not > feasible solution and very error prone. I am guessing that every one of you machines uses the same device tree and that the bootloader modifies the per machine information, such as MAC addresses. Why can't you just replace the bad device tree with a good device tree? >> Lack of me providing another solution is not a reason to apply a >> patch that I disagree with. > > The patch works exact the same way as the current __symbols__ node. It > is very tiny and easy to review and has absolut no side effects. > > If you have no technical objections you should apply it until you have > a better solution. My first reply provided a technical reason. A leading underscore in a node name is not valid device tree syntax. It is currently allowed to handle hand coded overlays, but patches have been submitted to the device tree compiler to remove this temporary hack. Hand coded overlay nodes are overly complex and more likely to contain errors than an auto-generated node. If hand coded overlay nodes were allowed then additional error checking would need to be added to the kernel. Assuming the compiler patches are accepted (which I believe is the correct direction) the proposed special node name of __external_symbols__ will result in a compile error. -Frank > > - Stefani > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 27+ messages in thread
[parent not found: <1497248029.9234.1.camel@seibold.net>]
* Re: [PATCH] external references for device tree overlays @ 2017-06-12 18:46 ` Frank Rowand 0 siblings, 0 replies; 27+ messages in thread From: Frank Rowand @ 2017-06-12 18:46 UTC (permalink / raw) To: Stefani Seibold Cc: Holm.Rauchfuss, Stefani Seibold, Pantelis Antoniou, Rob Herring, devicetree, linux-kernel Adding back the Cc: list On 06/11/17 23:13, Stefani Seibold wrote: > Hi Frank,i > > i am in vaction for the next two weeks. I will rewrite the patch when i > am back. > > Regards, > Stefani Hi Stefani, Please let us know how you intend to solve the problems you are facing before you invest a lot of time developing the patch. < snip > -Frank ^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] external references for device tree overlays @ 2017-06-12 18:46 ` Frank Rowand 0 siblings, 0 replies; 27+ messages in thread From: Frank Rowand @ 2017-06-12 18:46 UTC (permalink / raw) To: Stefani Seibold Cc: Holm.Rauchfuss-hv44wF8Li93QT0dZR+AlfA, Stefani Seibold, Pantelis Antoniou, Rob Herring, devicetree-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA Adding back the Cc: list On 06/11/17 23:13, Stefani Seibold wrote: > Hi Frank,i > > i am in vaction for the next two weeks. I will rewrite the patch when i > am back. > > Regards, > Stefani Hi Stefani, Please let us know how you intend to solve the problems you are facing before you invest a lot of time developing the patch. < snip > -Frank -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2017-06-12 18:47 UTC | newest] Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-06-05 12:59 [PATCH] external references for device tree overlays Stefani Seibold 2017-06-05 12:59 ` Stefani Seibold 2017-06-05 18:43 ` Pantelis Antoniou 2017-06-06 19:17 ` Stefani Seibold 2017-06-06 22:05 ` Rob Herring 2017-06-06 22:05 ` Rob Herring 2017-06-07 8:11 ` Pantelis Antoniou 2017-06-07 22:19 ` Rob Herring 2017-06-07 22:19 ` Rob Herring 2017-06-08 6:51 ` Stefani Seibold 2017-06-08 6:51 ` Stefani Seibold 2017-06-08 6:48 ` Stefani Seibold 2017-06-08 6:48 ` Stefani Seibold 2017-06-11 23:04 ` Frank Rowand 2017-06-11 23:04 ` Frank Rowand 2017-06-06 7:20 ` Frank Rowand 2017-06-06 16:12 ` Frank Rowand 2017-06-06 16:12 ` Frank Rowand 2017-06-06 19:22 ` Stefani Seibold 2017-06-07 0:46 ` Frank Rowand 2017-06-07 0:46 ` Frank Rowand [not found] ` <1496815399.6999.1.camel@seibold.net> [not found] ` <1496815399.6999.1.camel-mkwtCZVSLSnR7s880joybQ@public.gmane.org> 2017-06-08 3:07 ` Frank Rowand [not found] ` <5938BF8F.2010001-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2017-06-08 7:08 ` Pantelis Antoniou 2017-06-08 7:08 ` Stefani Seibold [not found] ` <1496905725.7999.5.camel-mkwtCZVSLSnR7s880joybQ@public.gmane.org> 2017-06-11 23:14 ` Frank Rowand [not found] ` <1497248029.9234.1.camel@seibold.net> 2017-06-12 18:46 ` Frank Rowand 2017-06-12 18:46 ` Frank Rowand
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.