Re: [PATCH 1/2] fdt: Allow stacked overlays phandle references

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 3981 bytes --]

On Wed, Jul 26, 2017 at 06:59:35AM -0700, Frank Rowand wrote:
> On 07/25/17 21:32, David Gibson wrote:
> > On Fri, Jul 14, 2017 at 10:21:01AM +0300, Pantelis Antoniou wrote:
> >> Hi Frank,
> >>
> >> On Thu, 2017-07-13 at 14:40 -0700, Frank Rowand wrote:
> >>> On 07/13/17 14:22, Phil Elwell wrote:
> >>>> On 13/07/2017 21:07, Frank Rowand wrote:
> >>>>> On 07/13/17 12:38, Phil Elwell wrote:
> >>>>>
> >>
> >> [snip]
> >>
> >>>> hope an inability to solve the problem posed by this advanced usage won't
> >>>> prevent a solution to a simpler problem from being accepted.
> >>
> >> I have waited until people started commenting on this patchset before
> >> replying.
> >>
> >> I think we agree on a few things to keep the discussion moving forward.
> >>
> >> 1. Stacked overlays are useful and make overlays easier to use.
> > 
> > Agreed.
> > 
> >> 2. Changing the overlay symbols format now would be unwise.
> > 
> > Agreed.  At least, I don't think updating the symbols alone would be
> > silly without revisiting everything in the overlay format and making
> > something completely new.
> > 
> >> 3. A number of extensions have been put forward/requested.
> >>
> >> 3.1. There should be a method to place a symbol on a node that didn't
> >> have one originally (due to vendor supplying broken DTB or being
> >> generated by firmware at runtime).
> > 
> > There already is.  An overlay can update *anything* in the base tree,
> > including the /__symbols__ node.  Of course you need the exact path of
> > the node to tag in the base tree, but you were always going to need
> > that.
> 
> I haven't tested that, but it should work with the existing dtc and
> Linux kernel.
> 
> But it will stop working in the future _if_ some changes are made
> that I would like:
> 
>   - dtc no longer accept node names beginning with underscore as
>     valid.

I don't like that idea.  Warning, sure, but I don't wish to outright
ban constructs which are allowed by the syntax and IEEE1275.  Allowing
special effects like the above is one reason for that.

>   - dtc supports Pantelis' sugar syntax

Uh.. I don't see how that's relevant.

> My intent behind these changes is to hide the implementation details
> of the overlay extensions (__symbols__, __fixups__, __local_fixups__,
> fragements nodes, etc) from the normal dts developer.

A good goal, but that doesn't preclude them being accessible to
the.. uh.. abnormal dts developer?

> This should
> make it easier to write and understand overlays, and reduce errors
> in the dtb.
> 
> With those changes, it would not be possible to write an overlay
> that applied against node __symbols__ because it would not be
> possible to create a label on __symbols__, which would be needed
> to reference __symbols__ with the sugar syntax.

I haven't looked at Pantelis' latest patches.  But AFAIK it's based on
the existing compile time overlay syntax, which allows addressing by
path as well label.  So you could do

&{/__symbols__} {
	gadget = "/path/to/forgotten/gadget";
};

> 
> -Frank
> 
> >> 3.2. Scoping symbol visibility in case of clashes. This can the ability
> >> to put multiple path references to a single label/symbol. i.e.
> >> foo = "/path/bar", "/path/bar/baz";
> >> Resolving the ambiguity would require the caller to provide it's
> >> 'location' on the tree. I.e. a device under /path/bar/baz would resolve
> >> to the latter. It is a big change semantically.
> > 
> > I think this would be a nice idea, but trying to do it as a update to
> > the existing overlay format will be really difficult verging on
> > impossible.
> > 
> > Better to keep this in mind as a design goal for a new format to
> > replace overlays.
> > 
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]