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@xxxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > Signed-off-by: Stefani Seibold <stefani@xxxxxxxxxxx>
> > ---
> > 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);
> >
>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
