Re: [PATCH] external references for device tree overlays

From: Stefani Seibold
Date: Tue Jun 06 2017 - 15:37:36 EST

Next message: David Miller: "Re: [PATCH 14/16] sparc: vio: use dev_groups and not dev_attrs for bus_type"
Previous message: Li, Yi: "Re: [PATCHv2 0/3] Enable no_cache flag to driver_data"
In reply to: Pantelis Antoniou: "Re: [PATCH] external references for device tree overlays"
Next in thread: Rob Herring: "Re: [PATCH] external references for device tree overlays"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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);
> > Â
>
>

Next message: David Miller: "Re: [PATCH 14/16] sparc: vio: use dev_groups and not dev_attrs for bus_type"
Previous message: Li, Yi: "Re: [PATCHv2 0/3] Enable no_cache flag to driver_data"
In reply to: Pantelis Antoniou: "Re: [PATCH] external references for device tree overlays"
Next in thread: Rob Herring: "Re: [PATCH] external references for device tree overlays"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]