Re: [PATCH 1/2] libfdt: overlay: Allow resolving phandle symbols

[David Gibson <david@gibson.dropbear.id.au>]

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

On Mon, Sep 09, 2024 at 12:54:34PM +0530, Ayush Singh wrote:
> On 9/9/24 10:33, David Gibson wrote:
> 
> > On Mon, Sep 02, 2024 at 05:47:55PM +0530, Ayush Singh wrote:
> > > Add ability to resolve symbols pointing to phandles instead of strings.
> > > 
> > > Combining this with existing fixups infrastructure allows creating
> > > symbols in overlays that refer to undefined phandles. This is planned to
> > > be used for addon board chaining [1].
> > I don't think this "autodetection" of whether the value is a phandle
> > or path is a good idea.  Yes, it's probably unlikely to get it wrong
> > in practice, but sloppy cases like this have a habit of coming back to
> > bite you later on.  If you want this, I think you need to design a new
> > way of encoding the new options.
> 
> Would something like `__symbols_phandle__` work?

Preferable to the overloaded values in the original version, certainly.

> It should be fairly
> straightforward to do. I am not sure how old devicetree compilers will react
> to it though?

Well, the devicetree compiler itself never actually looks at the
overlay encoding stuff.  The relevant thing is libfdt's overlay
application logic... and any other implementations of overlay handling
that are out there.

At which I should probably emphasize that changes to the overlay
format aren't really mine to approve or not.  It's more or less an
open standard, although not one with a particularly formal definition.
Of course, libfdt's implementation - and therefore I - do have a fair
bit of influence on what's considered the standard.

> I really do not think having a different syntax for phandle symbols would be
> good since that would mean we will have 2 ways to represent phandles:
> 
> 1. For __symbols__
> 
> 2. For every other node.

I'm really not sure what you're getting at here.

> I also am not in the favor of doing something bespoke that does not play
> nice with the existing __fixups__ infra since that has already been
> thoroughly tested, and already creates __fixups__ for symbols.

Hmm.  Honestly, the (runtime) overlay format was pretty a hack that's
already trying to do rather more than it was really designed for.  I'm
a bit sceptical of any attempt to extend it further without
redesigning the whole thing with a bit more care and forethought.  I
believe Rob Herring has some thoughts along these lines too.

-- 
David Gibson (he or they)	| 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 --]