On 01/04/18 19:50, Kyle Evans wrote: > On Thu, Jan 4, 2018 at 9:14 PM, Frank Rowand <frowand.list@xxxxxxxxx> wrote: >> 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. > > I think I might be misunderstanding something here- our dtc (BSDL > dtc), with -@, still writes a full /__symbols__ node of all labeled > nodes, given that this is what -@ is expected to do. The difference is I was talking about the .dts file in patch 2/2, where the __symbols__ node was hand coded, instead of being created by dtc. > that our dtc allocates phandles conservatively because they're a > concept for cross-referencing nodes. Conceptually, it doesn't seem > like either way is more or less wrong than the other. Yes, the phandles concept is for cross-referencing. In your specific context, creating phandle properties at the time that applying the overlay determines that the cross-reference actually occurs does allow dynamically creating the phandle properties. If overlays were only applied to flattened device trees, then that could be a useful optimization. >> 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. > > I'm probably being dense here, so please excuse me, but I don't see > this as necessarily a problem- likely because I have no familiarity of > how Linux works with overlays or FDT bits in general. My stupid > question of the hour: Linux actually supports applying overlays to > live devicetree? Not fully implemented yet. In fact there are some pretty important issues to be resolved or implemented. But there are people who have implemented enough of it out of mainline to be able to successfully apply specific overlays to the live devicetree for specific use cases. There is also the concept of removing an overlay from the live devicetree at some point after having applied it. > Our overlay support effectively ends at the bootloader. We load fdt > blob from the filesystem or pull it into bootloader memory from > EFI/U-Boot, apply any overlays, then pass it into the kernel. After > that, it's effectively immutable (IIRC; ian@ will have to clarify if > I'm wrong) and we don't support reloading FDT on a live system. > -- 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
