On Thu, Jul 27, 2017 at 10:24:17AM -0700, Frank Rowand wrote:
> On 07/27/17 00:23, David Gibson wrote:
> > On Wed, Jul 26, 2017 at 06:59:35AM -0700, Frank Rowand wrote:
> >> On 07/25/17 21:32, David Gibson wrote:
> >>> On Fri, Jul 14, 2017 at 10:21:01AM +0300, Pantelis Antoniou wrote:
> >>>> Hi Frank,
> >>>>
> >>>> On Thu, 2017-07-13 at 14:40 -0700, Frank Rowand wrote:
> >>>>> On 07/13/17 14:22, Phil Elwell wrote:
> >>>>>> On 13/07/2017 21:07, Frank Rowand wrote:
> >>>>>>> On 07/13/17 12:38, Phil Elwell wrote:
> >>>>>>>
> >>>>
> >>>> [snip]
> >>>>
> >>>>>> hope an inability to solve the problem posed by this advanced usage won't
> >>>>>> prevent a solution to a simpler problem from being accepted.
> >>>>
> >>>> I have waited until people started commenting on this patchset before
> >>>> replying.
> >>>>
> >>>> I think we agree on a few things to keep the discussion moving forward.
> >>>>
> >>>> 1. Stacked overlays are useful and make overlays easier to use.
> >>>
> >>> Agreed.
> >>>
> >>>> 2. Changing the overlay symbols format now would be unwise.
> >>>
> >>> Agreed. At least, I don't think updating the symbols alone would be
> >>> silly without revisiting everything in the overlay format and making
> >>> something completely new.
> >>>
> >>>> 3. A number of extensions have been put forward/requested.
> >>>>
> >>>> 3.1. There should be a method to place a symbol on a node that didn't
> >>>> have one originally (due to vendor supplying broken DTB or being
> >>>> generated by firmware at runtime).
> >>>
> >>> There already is. An overlay can update *anything* in the base tree,
> >>> including the /__symbols__ node. Of course you need the exact path of
> >>> the node to tag in the base tree, but you were always going to need
> >>> that.
> >>
> >> I haven't tested that, but it should work with the existing dtc and
> >> Linux kernel.
> >>
> >> But it will stop working in the future _if_ some changes are made
> >> that I would like:
> >>
> >> - dtc no longer accept node names beginning with underscore as
> >> valid.
> >
> > I don't like that idea. Warning, sure, but I don't wish to outright
> > ban constructs which are allowed by the syntax and IEEE1275. Allowing
> > special effects like the above is one reason for that.
>
> Oops, my bad, sorry. I left out half of what I have been advocating
> in the past, so basically I agree with you. My past statements also
> included some escape mechanism, such as a command line option that
> would allow node names beginning with an underscore.
>
> You accepted the patch to add -Wnode_name_chars_strict that warns of
> use of underscore anywhere in the node name, so Linux does have the
> option of enabling that warning, which would be another partial
> solution to what I was suggesting.
Ok.
> I wrote the previous two paragraphs, and then realized I missed part
> of what I was responding to. The ePAPR and the Device Tree Specification
> both say that a node name "shall start with a lower or uppercase character",
> so the construct is not allowed by the syntax.
Ah, ok, I hadn't remembered that.
> Sigh. You explicitly said "IEEE125". So off I go to read that a bit,
> and as far as a quick reading goes, there is no restriction of what
> the first character may be.
Yeah, it doesn't put a lot of restrictions on the namespace. It does
have a list of acceptable characters but.. some of those are rarely if
ever used in practice, and there are some others which are used in the
wild even though they weren't allowed by 1275 :/.
> I have previously ignored IEEE1275, because the ePAPR was what I
> thought the Linux kernel and dtc used as a reference (and now
> Linux has moved on to the "Device Tree Specification"). What
> is the policy of the dtc project when the ePAPR (and "Device
> Tree Reference") differ from IEEE1275?
Well, it hasn't come enough to have a policy as such, but I guess the
principles to decide a specific case would be:
* dtc should be able to deal with any tree that's valid under
either 1275 or "modern" (epapr or dt spec) rules.
* "modern" rules should be the default when it's not possible to
automatically deal with both options
Note that IEEE1275 still has direct relevance, POWER servers still
have real 1275 firmware. For that reason I don't think it's a good
idea to diverge from that for more modern specs unless there's a
really compelling reason to do so.
That said, I think the main relevance of IEEE1275 isn't as a direct
specification, but as inspiration. When we come across some new
problem to solve, it's worth looking at IEEE1275 and it's ecosystem of
bindings solved similar problems in the past.
> >> - dtc supports Pantelis' sugar syntax
> >
> > Uh.. I don't see how that's relevant.
>
> The sugar syntax is the only way that I am aware of to code an
> overlay dts such that dtc would create the __fixups__, __local_fixups__,
> and fragment nodes, without explicitly coding node names beginning
> with an underscore in the source dts.
Right.. but you _can_ explicitly code underscore names, even if it
does require suppressing a warning.
> >> My intent behind these changes is to hide the implementation details
> >> of the overlay extensions (__symbols__, __fixups__, __local_fixups__,
> >> fragements nodes, etc) from the normal dts developer.
> >
> > A good goal, but that doesn't preclude them being accessible to
> > the.. uh.. abnormal dts developer?
>
>
> (I wrote the following three paragraphs before looking at IEEE125.)
>
> The ePAPR and the Device Tree Specification both state that a node
> name "shall start with a lower or uppercase character". That should
> probably be 'letter' instead of 'character', but my understanding
> is that 'letter' is the intent.
>
> It seems useful for dtc to disallow what would seem to me to be an
> error in dts source, when a node name started with some other
> character.
>
> But again, there should probably be a command line option or some
> other method to allow an underscore as the first character of a
> node name, at least in a transition period.
Right, I believe we can currently turn off individual warnings, so
that's straightforward enough. And/or we can use the -f option.
>
>
> >> This should
> >> make it easier to write and understand overlays, and reduce errors
> >> in the dtb.
> >>
> >> With those changes, it would not be possible to write an overlay
> >> that applied against node __symbols__ because it would not be
> >> possible to create a label on __symbols__, which would be needed
> >> to reference __symbols__ with the sugar syntax.
> >
> > I haven't looked at Pantelis' latest patches. But AFAIK it's based on
> > the existing compile time overlay syntax, which allows addressing by
> > path as well label. So you could do
> >
> > &{/__symbols__} {
> > gadget = "/path/to/forgotten/gadget";
> > };
>
> Hmmmm.
>
> (_If_ a leading underscore is not allowed as the first character of
> a node name:)
>
> I would fall back on my suggestion that the leading underscore should
> be detected as an illegal node name here also (override allowed...).
> I don't know if that would be the case in this specific location if
> it was detected as illegal when specifying a node in the tree. In
> other words, I don't know how the dtc code checks for a valid node
> name, so I don't know if the same code would check for valid node
> name in the two different locations of the syntax.
In this case.. probably not. Checks (other than the most basic
fundamentals, such as no nuls) aren't part of the main compile path,
but rather in the "checks" subsystem. Once assembled into overlay
form, that target node name wouldn't actually appear in a node name -
only indirectly in the target-path property.
That said, the checks mechanism needs some reworking to deal with
overlay more sensibly.
> >> -Frank
> >>
> >>>> 3.2. Scoping symbol visibility in case of clashes. This can the ability
> >>>> to put multiple path references to a single label/symbol. i.e.
> >>>> foo = "/path/bar", "/path/bar/baz";
> >>>> Resolving the ambiguity would require the caller to provide it's
> >>>> 'location' on the tree. I.e. a device under /path/bar/baz would resolve
> >>>> to the latter. It is a big change semantically.
> >>>
> >>> I think this would be a nice idea, but trying to do it as a update to
> >>> the existing overlay format will be really difficult verging on
> >>> impossible.
> >>>
> >>> Better to keep this in mind as a design goal for a new format to
> >>> replace overlays.
> >>>
> >>
> >
>
--
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
