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. If no one is using
them and nobody cares if they exist, then getting rid of them is
one possible fallback if nothing else works.
>>>> 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.
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__.
So using &{some-path-or-other} as sugar syntax seems problematic.
>> Another approach would be for Linux to automagically add a phandle
>> to the root node and create a corresponding symbol that the target
>> could use for a phandle reference. Name of this magic symbol is
>> an exercise left to the reader or bikeshedding.
>
> I really dislike that option since it adds more complexity that any
> overlay applied would need to know.
I somewhat dislike that option, but I am trying to think out of the box.
Just in case it comes up with a good solution.
>> I'm exploring alternatives. Any other thoughts would be appreciated.
>>
>> Geert started another thread based on the rcar-du patch, where
>> he says that not being able to have a label on the root node
>> is one of two issues that is causing him issues with converting
>> to sugar syntax. He says this "is the real blocker for me".
>> So there is some indication that this may be a widespread issue.
>>
>> I have a long standing concern about arbitrary overlays being
>> applied at arbitrary locations. Being able to target an
>> overlay to the root of the tree can only make this issue
>> worse. :-) So there may also be a need to create an
>> administrative mechanism to provide some control. I don't
>> really want to start on that yet, but it may be a gating
>> factor before allowing an overlay loader in the Linux tree.
>>
>> -Frank
>
// SPDX-License-Identifier: GPL-2.0
/*
* rcar_du_of_lvds_r8a7790.dts - Legacy LVDS DT bindings conversion for R8A7790
*
* Copyright (C) 2018 Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
*
* Based on work from Jyri Sarha <jsarha@xxxxxx>
* Copyright (C) 2015 Texas Instruments
*/
/dts-v1/;
/plugin/;
/ {
fragment@0 {
target-path = "/";
__overlay__ {
#address-cells = <2>;
#size-cells = <2>;
lvds@feb90000 {
compatible = "renesas,r8a7790-lvds";
reg = <0 0xfeb90000 0 0x1c>;
ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
lvds0_input: endpoint {
};
};
port@1 {
reg = <1>;
lvds0_out: endpoint {
};
};
};
};
lvds@feb94000 {
compatible = "renesas,r8a7790-lvds";
reg = <0 0xfeb94000 0 0x1c>;
ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
lvds1_input: endpoint {
};
};
port@1 {
reg = <1>;
lvds1_out: endpoint {
};
};
};
};
};
};
fragment@1 {
target-path = "/display@feb00000/ports";
__overlay__ {
port@1 {
endpoint {
remote-endpoint = <&lvds0_input>;
};
};
port@2 {
endpoint {
remote-endpoint = <&lvds1_input>;
};
};
};
};
};
// SPDX-License-Identifier: GPL-2.0
/*
* rcar_du_of_lvds_r8a7790.dts - Legacy LVDS DT bindings conversion for R8A7790
*
* Copyright (C) 2018 Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
*
* Based on work from Jyri Sarha <jsarha@xxxxxx>
* Copyright (C) 2015 Texas Instruments
*/
/dts-v1/;
/plugin/;
&my_root {
#address-cells = <2>;
#size-cells = <2>;
lvds@feb90000 {
compatible = "renesas,r8a7790-lvds";
reg = <0 0xfeb90000 0 0x1c>;
ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
lvds0_input: endpoint {
};
};
port@1 {
reg = <1>;
lvds0_out: endpoint {
};
};
};
};
lvds@feb94000 {
compatible = "renesas,r8a7790-lvds";
reg = <0 0xfeb94000 0 0x1c>;
ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
lvds1_input: endpoint {
};
};
port@1 {
reg = <1>;
lvds1_out: endpoint {
};
};
};
};
};
&display_at_feb00000__ports {
port@1 {
endpoint {
remote-endpoint = <&lvds0_input>;
};
};
port@2 {
endpoint {
remote-endpoint = <&lvds1_input>;
};
};
};
// SPDX-License-Identifier: GPL-2.0
/*
* rcar_du_of_lvds_r8a7790.dts - Legacy LVDS DT bindings conversion for R8A7790
*
* Copyright (C) 2018 Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
*
* Based on work from Jyri Sarha <jsarha@xxxxxx>
* Copyright (C) 2015 Texas Instruments
*/
/dts-v1/;
/plugin/;
&{/} {
#address-cells = <2>;
#size-cells = <2>;
lvds@feb90000 {
compatible = "renesas,r8a7790-lvds";
reg = <0 0xfeb90000 0 0x1c>;
ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
lvds0_input: endpoint {
};
};
port@1 {
reg = <1>;
lvds0_out: endpoint {
};
};
};
};
lvds@feb94000 {
compatible = "renesas,r8a7790-lvds";
reg = <0 0xfeb94000 0 0x1c>;
ports {
#address-cells = <1>;
#size-cells = <0>;
port@0 {
reg = <0>;
lvds1_input: endpoint {
};
};
port@1 {
reg = <1>;
lvds1_out: endpoint {
};
};
};
};
};
&display_at_feb00000__ports {
port@1 {
endpoint {
remote-endpoint = <&lvds0_input>;
};
};
port@2 {
endpoint {
remote-endpoint = <&lvds1_input>;
};
};
};
/dts-v1/;
/ {
fragment@0 {
target = <0xffffffff>;
__overlay__ {
#address-cells = <0x2>;
#size-cells = <0x2>;
lvds@feb90000 {
compatible = "renesas,r8a7790-lvds";
reg = <0x0 0xfeb90000 0x0 0x1c>;
ports {
#address-cells = <0x1>;
#size-cells = <0x0>;
port@0 {
reg = <0x0>;
lvds0_input: endpoint {
phandle = <0x1>;
};
};
port@1 {
reg = <0x1>;
lvds0_out: endpoint {
};
};
};
};
lvds@feb94000 {
compatible = "renesas,r8a7790-lvds";
reg = <0x0 0xfeb94000 0x0 0x1c>;
ports {
#address-cells = <0x1>;
#size-cells = <0x0>;
port@0 {
reg = <0x0>;
lvds1_input: endpoint {
phandle = <0x2>;
};
};
port@1 {
reg = <0x1>;
lvds1_out: endpoint {
};
};
};
};
};
};
fragment@1 {
target = <0xffffffff>;
__overlay__ {
port@1 {
endpoint {
remote-endpoint = <0x1>;
};
};
port@2 {
endpoint {
remote-endpoint = <0x2>;
};
};
};
};
__fixups__ {
my_root = "/fragment@0:target:0";
display_at_feb00000__ports = "/fragment@1:target:0";
};
__local_fixups__ {
fragment@1 {
__overlay__ {
port@1 {
endpoint {
remote-endpoint = <0x0>;
};
};
port@2 {
endpoint {
remote-endpoint = <0x0>;
};
};
};
};
};
};
/dts-v1/;
/ {
phandle = <0x1>;
fragment@0 {
target = <0x1>;
__overlay__ {
#address-cells = <0x2>;
#size-cells = <0x2>;
lvds@feb90000 {
compatible = "renesas,r8a7790-lvds";
reg = <0x0 0xfeb90000 0x0 0x1c>;
ports {
#address-cells = <0x1>;
#size-cells = <0x0>;
port@0 {
reg = <0x0>;
lvds0_input: endpoint {
phandle = <0x2>;
};
};
port@1 {
reg = <0x1>;
lvds0_out: endpoint {
};
};
};
};
lvds@feb94000 {
compatible = "renesas,r8a7790-lvds";
reg = <0x0 0xfeb94000 0x0 0x1c>;
ports {
#address-cells = <0x1>;
#size-cells = <0x0>;
port@0 {
reg = <0x0>;
lvds1_input: endpoint {
phandle = <0x3>;
};
};
port@1 {
reg = <0x1>;
lvds1_out: endpoint {
};
};
};
};
};
};
fragment@1 {
target = <0xffffffff>;
__overlay__ {
port@1 {
endpoint {
remote-endpoint = <0x2>;
};
};
port@2 {
endpoint {
remote-endpoint = <0x3>;
};
};
};
};
__fixups__ {
display_at_feb00000__ports = "/fragment@1:target:0";
};
__local_fixups__ {
fragment@0 {
target = <0x0>;
};
fragment@1 {
__overlay__ {
port@1 {
endpoint {
remote-endpoint = <0x0>;
};
};
port@2 {
endpoint {
remote-endpoint = <0x0>;
};
};
};
};
};
};
