On Tue, Feb 27, 2018 at 07:47:04PM -0800, Frank Rowand wrote:
> On 02/27/18 17:48, David Gibson wrote:
> > On Tue, Feb 27, 2018 at 09:58:00AM -0800, Frank Rowand wrote:
> >> On 02/27/18 07:54, Rob Herring wrote:
> >>> On Sun, Feb 25, 2018 at 9:22 PM, <frowand.list@xxxxxxxxx> wrote:
> >>>> From: Frank Rowand <frowand.list@xxxxxxxxx>
> >>>>
> >>>> Hi All,
> >>>>
> >>>> I am once again out of my depth with bison and flex.
> >>>
> >>> Can't help on that one...
> >>>
> >>>> The Devicetree Specification and ePAPR both tell me that I can
> >>>> put a label on a node, as in:
> >>>>
> >>>> [label:] node-name[@unit-address] {
> >>>> [properties definitions]
> >>>> [child nodes]
> >>>> }
> >>>
> >>> Applying this to the root node would also imply that a unit-address is
> >>> also allowed which it is not. I'd contend that the root is special.
> >>
> >> Good point. Looks like I may have some additions to propose for the
> >> spec to clarify things.
> >>
> >> Also not mentioned in the spec:
> >> - "[label:]" should be something like "[label:} ..." (not sure of
> >> the syntax we use in the spec for multiples>
> >
> > Right. In general you can have multiple labels on anything.
> >
> >> - that labels can be put on /memreserve/
> >
> > So, like labels within property values, labels on memreserve will only
> > do something measurable with -Oasm output, and isn't really useful
> > with modern device tree workflows.
>
> I don't have enough visibility into the various bootloaders (and
> operating systems other than Linux), so I did not have any guesses
> about how useful labels on /memreserve/ are.
Not very. To understand both these and within-property labels you
need to understand a use case that sounded good in the early days, but
I don't think was ever really used: that's of a very static board,
which a single fixed device tree image *almost* works. The only
exception being a few very small in place changes between different
models: e.g. size of RAM, size of flash. The idea was that you could
make a really tiny bootloader for such a platform by simply compiling
in the dtb (using -Oasm mode). You put labels on the handful of
numbers that need updating, and the bootloader can set them to the
right values by simply assigning the symbols, no parsing of the dtb
structure necessary.
In practice it's turned out that even for pretty simple boards you
usually either want to a) parse and read parts of the dtb for internal
use in the bootloader and/or b) make at least a few changes that
involve adding, removing or resizing nodes or properties (note that
even allowing the kernel command line to be updated will require the
last). Once you have that, the model above doesn't really work. Plus
the fact that libfdt is fairly straightforward to use, even for more
complex read/write cases means that that model was never widely used.
> If no one is using
> them and nobody cares if they exist,
That's probably true..
> then getting rid of them is
> one possible fallback if nothing else works.
..but I'm still very hesitant about making a backwards incompatible
change to the source format. Especially since the rationale for
labels on the root node is also looking pretty weak at this point.
Oh, I also realised there is a way to put a label on the root node
using existing dts syntax.. although it's a bit weird and won't work
in a /plugin/ file. It uses the fact that "static" overlays (as in
resolved by dtc, rather than later) can also add labels:
/ {
/* stuff */
}
rootlabel: &{/} { };
I haven't actually tried that, but I think it should work.
> >>>> and do not make any mention of the root node as being different
> >>>> that any other node in this respect.
> >>>>
> >>>> However, the dtc implementation does not allow a label on the
> >>>> root node. I tried modifying the dtc parser to allow a label
> >>>> on the root node. The result is patch 1/2 in this series.
> >>>
> >>> Why would you want a label there? Because using "/" is too short or
> >>> the syntax for phandles using the path is too hard to remember (I
> >>> never can)?
> >>
> >> I'm not sure I _really_ want to, but I want to at least see if
> >> I can implement it. My quest started when I was trying to convert
> >> Laurent's R-Car overlays to sugar format. The first obstacle
> >> was that the target of most of the overlays is the root node.
> >> One way to do that is to put a label on the root node.
> >
> > You should also be able to use &{/} to reference the root node with
> > sugar syntax. I believe that should generate a target-path format
> > fragment. And if it doesn't, we should fix that.
>
> I had not thought of that possibility.
>
> I took one of Laurent's overlay .dts files to see if it would work.
>
> The first attachment is one of Laurent's overlay files with explicit
> fragments and using target-path instead of target to locate each
> fragment.
>
> The next attachment, ..._v1.dts, is the overlay modified to use sugar
> syntax, assuming that a label can be placed on the base devicetree
> root node. The result of compiling this overlay, with -O dts generating
> a dts is attachment ov_v1.dts. This looks like the result I would expect
> from sugar syntax.
>
> The third attachment, ..._v2.dts is ..._v1.dts modified to use &{/}
> in place of referring to a root node label. The result of compiling
> this overlay, with -O dts generating a dts is attachment ov_v2.dts.
> This is not quite the result I would expect from sugar syntax.
Right, it's generating 'target' instead of 'target-path' even though
the reference is given in path form. That's a bug: even without the
special case of trying to reference the root node, this can't work.
> First, the resulting overlay has a phandle on the root node. Properties
> outside the fragment nodes are not expected, and will not be applied to
> the target system. Even if we modified the kernel's overlay code to
> add this root phandle property, we would have a conflicting root
> phandle property as soon as a second overlay was applied that had
> a target of &{/}.
>
> The other complication is that fragment@0/target becomes a __local_fixups__
> instead of a __fixups__.
Right this is all consequences of trying to use 'target' instead of
'target-path'. Because the __fixups__ syntax only fixes things up to
point to symbols it can't work for something specified by path.
> So using &{some-path-or-other} as sugar syntax seems problematic.
No, the _syntax_ is fine, it just hasn't been implemented. We need to
change add_ophan_node() so that it tells the diference between a
symbol reference and a path reference and produces proper output for
the two cases.
--
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:
signature.asc
Description: PGP signature
