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: 3747 bytes --]

On Fri, Jan 05, 2018 at 12:40:13PM -0800, Frank Rowand wrote:
> On 01/04/18 21:47, Kyle Evans wrote:
> > On Thu, Jan 4, 2018 at 11:02 PM, Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >> On 01/04/18 19:50, Kyle Evans wrote:
> >>> On Thu, Jan 4, 2018 at 9:14 PM, Frank Rowand <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> >>>> On 01/04/18 18:39, Kyle Evans wrote:
> >>>>> On Thu, Jan 4, 2018 at 7:55 PM, Frank Rowand <frowand.list-Re5JQEeQqe8@public.gmane.orgm> wrote:
> >>>>>> On 01/04/18 13:47, Kyle Evans wrote:
> >>>>>>> On Thu, Jan 4, 2018 at 3:34 PM, Kyle Evans <kevans-h+KGxgPPiopAfugRpC6u6w@public.gmane.org> wrote:
> >>>>>>>> On Thu, Jan 4, 2018 at 2:41 PM, Kyle Evans <kevans-h+KGxgPPiopAfugRpC6u6w@public.gmane.org> wrote:
> >>>>>>>>> On Thu, Jan 4, 2018 at 2:33 PM, Frank Rowand <frowand.list@gmail.com> wrote:
> >>>>>>>>>> [... snip ...]
[snip]
> > Your implementation knows what my overlay means otherwise because I
> > reference a labelled node using &foo, dtc generated a /__fixup__ for
> > it, your implementation takes that /__fixup__ and does the lookup in
> > the symbol table. The symbol exists and points to a node, what's the
> > point of rejecting it there?> 
> > It seems unreasonable to me, because you cannot always control the
> > source of your live tree. In many cases, sure, it's generated in-tree
> > or in U-Boot and passed to you, and you can reasonably expect you
> > won't encounter this. What if you have vendor-provided tree?
> > 
> > I think the point I'm getting at is that it seems like this will have
> > to change at some point anyways simply because you can't control all
> > sources of your devicetree, and this isn't strictly wrong. Telling
> > someone "No, we can't apply that overlay because your vendor's
> > internal tool for generating dts and $other_format_used_internally
> > simultaneously didn't generate a phandle for that" is kind of hard,> especially when your reasoning ISN'T "their blob is malformed" or
> > "your overlay's reference is ambiguous" but rather, "we know what
> > that's pointing at, but we just don't generate handles."
> 
> No, the blob _is_ malformed.  I know there is no official binding
> document for overlays (we do need such things once we figure out
> what we are doing), so this comment is purely my opinion.

In this case it's not a spec for overlays that's the issue, it's a
spec for what base trees require in order to accept overlays.

> The blob is malformed because it was not compiled with the "-@"
> flag.  Mostly not because of anything in the source, although
> again the __symbols__ node should not be hand coded.

I don't think it's reasonable to claim a blob is malformed based
purely on how it was generated, it needs to be something about it's
actualy contents.

So the question is: is "includes a phandle for every node with a
symbol" a requirement for an overlay accepting base tree.  It's never
been explicitly stated, since overlays were just kind of hacked
together rather than fully specced out.  dtc has been written assuming
that is a requirement, BSDdtc has not.

I'm inclined to say "yes, it should be a requirement" on the grounds
that:
    a) that's the interpretation that's more widely deployed already
and
    b) that simplifies the overlay application logic, which generally
       takes place in a more restricted environment than the initial
       compilation.

I'm entirely open to arguments against that position though.

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