On 01/04/18 18:39, Kyle Evans wrote: > On Thu, Jan 4, 2018 at 7:55 PM, Frank Rowand <frowand.list@xxxxxxxxx> wrote: >> On 01/04/18 13:47, Kyle Evans wrote: >>> On Thu, Jan 4, 2018 at 3:34 PM, Kyle Evans <kevans@xxxxxxxxxxx> wrote: >>>> On Thu, Jan 4, 2018 at 2:41 PM, Kyle Evans <kevans@xxxxxxxxxxx> wrote: >>>>> On Thu, Jan 4, 2018 at 2:33 PM, Frank Rowand <frowand.list@xxxxxxxxx> 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 >> >> The "-@" option does not add a phandle to _every_ node, just to nodes >> that have a label: >> > > In practice, this is still a not necessarily close superset of those > that are actually going to actually get referenced in a given .dts. I > note that a number of nodes will still get labelled that are unlikely > to be referenced for any number of reasons. > > For instance, as an example [1][2][3] of one of the boards I'm working > on currently, all of the ehci/ohci/mmc nodes, some set of the pio > nodes... almost every single node has a label, and most of them don't > need a phandle based on what is currently referenced and actually > used. I note that 27 of these nodes seem to be referenced, out of 39 > nodes with labels (numbers are approximate), leaving ~30% (12) of > labelled nodes unreferenced. > > For a slightly larger example, [4][5][6][7]; 74 referenced nodes out > of 113 labelled nodes, leaving ~35% (39) of labelled nodes > unreferenced. > > This is only bothersome because it starts adding up as these things > get bigger, and I don't think it really needs to. The alternative to > keep phandles down to the minimum set required doesn't add much in > maintenance cost or overhead from the overlay application side; > especially for blobs generated by this dtc(1) that make the active > decision to allocate liberally rather than conservatively. This concept relies on a hand coded __symbols__ node instead of having dtc generate it. This makes the overlay devicetree source more fragile. The example dts file in patch 2/2 is fairly safe, but it is legal syntax to specify: test = "/test-node"; instead of: test = &test; And the fragile syntax is exactly what results from a decompiled overlay dtb. On another note, in the case of Linux, I have to consider applying overlays to a live devicetree, as well as possibly the case of applying overlays to a flattened device tree. I do not expect that we will add the dynamic creation of missing phandles when applying an overlay to a live devicetree. This would result in an overlay that can be applied to a flattened device tree, but will fail to apply to a Linux live devicetree. BSD and Linux do not have to be fully compatible, but in my opinion, the more compatible the better. -Frank > [1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/sun8i-a83t-bananapi-m3.dts > [2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/sun8i-a83t.dtsi > [3] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/sunxi-common-regulators.dtsi > > [4] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/am335x-bonegreen.dts > [5] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/am33xx.dtsi > [6] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/am335x-bone-common.dtsi > [7] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/arm/boot/dts/am335x-bonegreen-common.dtsi > -- To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
