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.
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?
>
>> - 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.
>> 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.
>> 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.
>> -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.
>>>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
