Re: [PATCH] external references for device tree overlays

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux