Re: [PATCH v2 1/2] overlays: auto allocate phandles for nodes in base fdt

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

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

On Wed, Jan 03, 2018 at 08:04:52AM -0600, Kyle Evans wrote:
> [Resending with a proper mail client, because it didn't go to the lists]
> 
> On Tue, Jan 2, 2018 at 11:42 PM, David Gibson
> <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org> wrote:
> > Regardless of anything else, these two patches need different one-line
> > summaries.
> >
> > On Mon, Jan 01, 2018 at 12:59:44AM -0600, kevans-HZy0K5TPuP5AfugRpC6u6w@public.gmane.org wrote:
> >> Currently, references cannot be made to nodes in the base that do not already
> >> have phandles, limiting us to nodes that have been referenced in the base fdt.
> >> Lift that restriction by allocating them on an as-needed basis.
> >
> > Hmm.  I'm a bit dubious about this.
> >
> > - My feeling is that one of the problems with the overlay format is
> >   that it's already too free, allowing the overlay to change
> >   essentially anything in the base tree.  So I'm not that keen on
> >   making it even more free.
> >
> > - An overlay can already add a 'phandle' property to a node in the
> >   base tree.  Can you use that directly instead of adding a new
> >   mechanism?
> >
> 
> That feels like it might be a bit difficult to work with, for a couple
> of reasons that might be logically wrong, so forgive me if I'm
> thinking wrong:
> 
>  - A phandle is just a number, so it won't get adjusted as overlays
> get applied. All of the overlays that one of my boards needs to apply
> would need to choose sufficiently high phandles that they hopefully
> won't conflict with other overlays, especially as new nodes get
> introduced with their own phandles.

So, I think that could be handled by applying a local fixup to it in
the overlay which would offset it correctly.  I'd have to work through
the details, but I won't bother because..

>  - If more than one overlay needs to reference a specific node, we
> have other conflicts. These overlays aren't necessarily related, but
> they would need to agree with each other on the phandle of the node OR
> make sure they apply in a specific order so one can add the phandle
> and subsequent overlays can reference them symbolically only as
> intended.

..that's a good point.  Adding the phandle in the overlay effectively
*requires* that the phandle be missing in the base, which gets really
messy if you have a bunch of overlays to juggle.

You could maybe handle it by handling "base tree fixes" as a separate
overlay which must be applied first before the "real" overlays, but at
that point your solution is probably a better one.

> The problem is that these overlays aren't necessarily curated by a
> single authority, so conflicts with multiple overlays trying to
> reference a node is scary. Ideally, we'd like these things to be as
> usable as possible for average Joe Blow.

Yeah, fair enough.  I'll re-evaluate your patches with that in mind.

-- 
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 --]