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

On Thu, Jan 04, 2018 at 03:08:08PM -0700, Ian Lepore wrote:
> On Thu, 2018-01-04 at 15:47 -0600, 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  wrote:
> > > > > 
> > > > > [... snip ...]
> > > > > 
> > > > > Does this remove the need for the proposed patch, or am I still
> > > > > missing something?
> > > > ... nope. Apparently I never tested this with this particular dtc(1)
> > > > and instead just assumed it did the same as ours- allocate phandle
> > > > sparsely, even with -@. That certainly removes the need for this
> > > > patch, and I'm somewhat upset that I hadn't previously considered
> > > > this.
> > > > 
> > > > @David, Jon: Please disregard all of the patches along these lines...
> > > > I'll fix this in our dtc, where it should be fixed.
> > > > 
> > > > Thanks, Frank!
> > > Actually, I'm kind of torn on whether this is useful or not. With
> > > being able to have EFI-provided FDT, it's hard to guarantee whether
> > > the FDT we're provided has been compiled with GPL dtc(1) and -@. The
> > > above solves this problem for most of my personal use-cases , though,
> > > since I can guarantee that our FDT and U-Boot provided FDT is compiled
> > > properly.
> > Apologies for the triple post; I realized that this argument is
> > inherently wrong, since we can't reference the node if there's no
> > symbol anyways.
> > 
> > The only way this might still be a good idea is to support more
> > minimal cases where an implementation might prefer to not create a
> > phandle for nodes that haven't been referenced.
> > 
> > In our case, we have a function [1] that walks the tree and generates
> > metadata on nodes that have phandles, under the assumption that these
> > have been referenced somewhere and provides a way to more quickly
> > reference these specifically through a separate linked link.
> > Allocating phandles for everything as GPL dtc does adds quite a bit
> > more overhead to this.
> > 
> > [1] http://src.illumos.org/source/xref/freebsd-head/sys/dev/ofw/openfirm.c#119
> 
> In particular, it makes lookups more expensive as it now must traverse
> a list that includes every node in the dtb, rather than just nodes that
> are actually referenced.  (It also increases the amount of storage, but
> at 20-ish bytes per node, that's not a big deal.)

Lookups of what exactly?  Aren't you unflattening the tree after you
read it in?

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