On 07/27/17 00:23, David Gibson wrote:

On Wed, Jul 26, 2017 at 06:59:35AM -0700, Frank Rowand wrote:

quoted

On 07/25/17 21:32, David Gibson wrote:

quoted

On Fri, Jul 14, 2017 at 10:21:01AM +0300, Pantelis Antoniou wrote:

quoted

Hi Frank,

On Thu, 2017-07-13 at 14:40 -0700, Frank Rowand wrote:

quoted

On 07/13/17 14:22, Phil Elwell wrote:

quoted

On 13/07/2017 21:07, Frank Rowand wrote:

quoted

On 07/13/17 12:38, Phil Elwell wrote:

[snip]

quoted

quoted

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.

quoted

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.

quoted

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.

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.

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.

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?

quoted

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

quoted

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.

quoted

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.

quoted

-Frank

quoted

quoted

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.