phandles using absolute paths in DT overlays

phandles using absolute paths in DT overlays

[Geert Uytterhoeven]

Hi,

Some early revisions of SoCs may have hardware bugs that need to be
fixed up in DT.  Currently we are handling this by including DTS files
and fixing up nodes and properties, to create different DTB files for
different SoC revisons (see arch/arm64/boot/dts/renesas/*es1*).

As an alternative, I'm envisioning the use of DT overlays and the
fdtoverlay tool, in the hope of simplifying the generation of DTBs for
the various SoC/board combinations.

Ideally, such DTBs would not contain symbols, to avoid inflating DTB
size.  Hence if fixup overlays would not contain symbolic references,
there would be no need for symbols.

For anchors, the "&{/path/to/node@address}" syntax is working fine.
For phandles, while documented on
https://elinux.org/Device_Tree_Mysteries, and while working fine for the
non-overlay case, dtc seems to have issues interpreting the DTB:

    $ scripts/dtc/dtc -I dtb -O dts my.dtb  | less
    <stdout>: ERROR (property_name_chars):
/__fixups__:/path/to/node@deadbeef: Bad     character '/' in property
name
    ERROR: Input tree has errors, aborting (use -f to force output)

With -f, the fixup generated seems to contain the expected value, though:

    __fixups__ {
        /path/to/node@deadbeef = "/fragment@0/__overlay__:power-domains:0";
    };

When using ftdoverlay, the situation is worse:

    Failed to apply my.dtb (-1)

Are these known issues?
Thanks for your comments!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: phandles using absolute paths in DT overlays

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2514 bytes --]

On Thu, Dec 06, 2018 at 01:56:45PM +0100, Geert Uytterhoeven wrote:
> Hi,
> 
> Some early revisions of SoCs may have hardware bugs that need to be
> fixed up in DT.  Currently we are handling this by including DTS files
> and fixing up nodes and properties, to create different DTB files for
> different SoC revisons (see arch/arm64/boot/dts/renesas/*es1*).
> 
> As an alternative, I'm envisioning the use of DT overlays and the
> fdtoverlay tool, in the hope of simplifying the generation of DTBs for
> the various SoC/board combinations.
> 
> Ideally, such DTBs would not contain symbols, to avoid inflating DTB
> size.  Hence if fixup overlays would not contain symbolic references,
> there would be no need for symbols.
> 
> For anchors, the "&{/path/to/node@address}" syntax is working fine.
> For phandles, while documented on
> https://elinux.org/Device_Tree_Mysteries, and while working fine for the
> non-overlay case, dtc seems to have issues interpreting the DTB:
> 
>     $ scripts/dtc/dtc -I dtb -O dts my.dtb  | less
>     <stdout>: ERROR (property_name_chars):
> /__fixups__:/path/to/node@deadbeef: Bad     character '/' in property
> name
>     ERROR: Input tree has errors, aborting (use -f to force output)
> 
> With -f, the fixup generated seems to contain the expected value, though:
> 
>     __fixups__ {
>         /path/to/node@deadbeef = "/fragment@0/__overlay__:power-domains:0";
>     };
> 
> When using ftdoverlay, the situation is worse:
> 
>     Failed to apply my.dtb (-1)
> 
> Are these known issues?

Unfortunately, this can't work with the current overlay format.  We
have a specific syntax to allow the target of an overlay fragment to
specified as a path, but phandle references by path won't work.

The problem is that the encoding of the fixups node has the fixup
target as a property name, and as the error says '/' is an illegal
character in property names - for a bunch of good reasons, so we can't
just go and remove that restriction.

So to allow this we'd need to come up with a different encoding for
path-targeted fixups and add support for it in the various pieces of
the chain.

One way to do that would be to entirely rework the overlay format to
something more sensible, which would have a number of other benefits.

-- 
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: phandles using absolute paths in DT overlays

[Geert Uytterhoeven]

Hi David,

Thanks for your answer!

On Fri, Dec 7, 2018 at 2:44 AM David Gibson <david@gibson.dropbear.id.au> wrote:
> On Thu, Dec 06, 2018 at 01:56:45PM +0100, Geert Uytterhoeven wrote:
> > Some early revisions of SoCs may have hardware bugs that need to be
> > fixed up in DT.  Currently we are handling this by including DTS files
> > and fixing up nodes and properties, to create different DTB files for
> > different SoC revisons (see arch/arm64/boot/dts/renesas/*es1*).
> >
> > As an alternative, I'm envisioning the use of DT overlays and the
> > fdtoverlay tool, in the hope of simplifying the generation of DTBs for
> > the various SoC/board combinations.
> >
> > Ideally, such DTBs would not contain symbols, to avoid inflating DTB
> > size.  Hence if fixup overlays would not contain symbolic references,
> > there would be no need for symbols.
> >
> > For anchors, the "&{/path/to/node@address}" syntax is working fine.
> > For phandles, while documented on
> > https://elinux.org/Device_Tree_Mysteries, and while working fine for the
> > non-overlay case, dtc seems to have issues interpreting the DTB:
> >
> >     $ scripts/dtc/dtc -I dtb -O dts my.dtb  | less
> >     <stdout>: ERROR (property_name_chars):
> > /__fixups__:/path/to/node@deadbeef: Bad     character '/' in property
> > name
> >     ERROR: Input tree has errors, aborting (use -f to force output)
> >
> > With -f, the fixup generated seems to contain the expected value, though:
> >
> >     __fixups__ {
> >         /path/to/node@deadbeef = "/fragment@0/__overlay__:power-domains:0";
> >     };
> >
> > When using ftdoverlay, the situation is worse:
> >
> >     Failed to apply my.dtb (-1)
> >
> > Are these known issues?
>
> Unfortunately, this can't work with the current overlay format.  We
> have a specific syntax to allow the target of an overlay fragment to
> specified as a path, but phandle references by path won't work.
>
> The problem is that the encoding of the fixups node has the fixup
> target as a property name, and as the error says '/' is an illegal
> character in property names - for a bunch of good reasons, so we can't
> just go and remove that restriction.

For the uneducated, can you please explain why '/' is an illegal character,
and why it can't be special cased for phandle references by path in overlays?

> So to allow this we'd need to come up with a different encoding for
> path-targeted fixups and add support for it in the various pieces of
> the chain.
>
> One way to do that would be to entirely rework the overlay format to
> something more sensible, which would have a number of other benefits.

IC.

Thanks!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: phandles using absolute paths in DT overlays

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 5111 bytes --]

On Fri, Dec 07, 2018 at 09:19:24AM +0100, Geert Uytterhoeven wrote:
> Hi David,
> 
> Thanks for your answer!
> 
> On Fri, Dec 7, 2018 at 2:44 AM David Gibson <david@gibson.dropbear.id.au> wrote:
> > On Thu, Dec 06, 2018 at 01:56:45PM +0100, Geert Uytterhoeven wrote:
> > > Some early revisions of SoCs may have hardware bugs that need to be
> > > fixed up in DT.  Currently we are handling this by including DTS files
> > > and fixing up nodes and properties, to create different DTB files for
> > > different SoC revisons (see arch/arm64/boot/dts/renesas/*es1*).
> > >
> > > As an alternative, I'm envisioning the use of DT overlays and the
> > > fdtoverlay tool, in the hope of simplifying the generation of DTBs for
> > > the various SoC/board combinations.
> > >
> > > Ideally, such DTBs would not contain symbols, to avoid inflating DTB
> > > size.  Hence if fixup overlays would not contain symbolic references,
> > > there would be no need for symbols.
> > >
> > > For anchors, the "&{/path/to/node@address}" syntax is working fine.
> > > For phandles, while documented on
> > > https://elinux.org/Device_Tree_Mysteries, and while working fine for the
> > > non-overlay case, dtc seems to have issues interpreting the DTB:
> > >
> > >     $ scripts/dtc/dtc -I dtb -O dts my.dtb  | less
> > >     <stdout>: ERROR (property_name_chars):
> > > /__fixups__:/path/to/node@deadbeef: Bad     character '/' in property
> > > name
> > >     ERROR: Input tree has errors, aborting (use -f to force output)
> > >
> > > With -f, the fixup generated seems to contain the expected value, though:
> > >
> > >     __fixups__ {
> > >         /path/to/node@deadbeef = "/fragment@0/__overlay__:power-domains:0";
> > >     };
> > >
> > > When using ftdoverlay, the situation is worse:
> > >
> > >     Failed to apply my.dtb (-1)
> > >
> > > Are these known issues?
> >
> > Unfortunately, this can't work with the current overlay format.  We
> > have a specific syntax to allow the target of an overlay fragment to
> > specified as a path, but phandle references by path won't work.
> >
> > The problem is that the encoding of the fixups node has the fixup
> > target as a property name, and as the error says '/' is an illegal
> > character in property names - for a bunch of good reasons, so we can't
> > just go and remove that restriction.
> 
> For the uneducated, can you please explain why '/' is an illegal character,

So, for *node* names, '/' clearly has to be banned, because that's the
path separator, so allowing it in components would make paths
ambiguous.

I realized after I wrote that that it's not quite so clear cut for
property names.  There are some reasons not to allow it still, but
they're not quite as irrefutable as for node names:

 * Keeping the same allowed character set for each, for consistency

 * Avoiding the same ambiguity if people use the notation
   /path/to/node/property.  That's not really correct anyway, but you
   see it sometimes

 * Avoiding problems for clients which translate property into names
   in the filesystem (Linux does this in order to present
   /proc/device-tree and similar for debugging).

Note that what we've historically allowed for fdts is a bit different
from what IEE1275 officially allowed - part of that is because a bunch
of OF implementations in the wild long used characters that weren't
permitted by the spec (we're looking at you, Apple).  '/' has always
been prohibited in every variant, though.

For properties, 1275 allowed any printable character except uppercase
letters, '/', '\', ':', '[', ']' and '@'.

For nodes, 1275 allowed only characters in [a-zA-Z0-9,._+-].  That's
not counting the unit-address part (from the @ sign onwards), which is
bus dependent.  I'm not exactly sure if there were general limits on
what was bus definitions could allow in that part, but in practice
things outside [0-9a-f] are rare, and I know of no bus which allowed
things outside [0-9a-z,].

In dtc, we allow [a-zA-Z0-9,._+*#?@-] for both nodes and properties,
and it doesn't look like that's changed since 2007.

> and why it can't be special cased for phandle references by path in
> overlays?

We possibly could, although I don't much like the idea.  Note that
merely allowing it wouldn't make it magically work.  Tools which apply
overlays (libfdt & the kernel at minimum) would still need code
changes to explicitly understand this type of fixup.

> > So to allow this we'd need to come up with a different encoding for
> > path-targeted fixups and add support for it in the various pieces of
> > the chain.
> >
> > One way to do that would be to entirely rework the overlay format to
> > something more sensible, which would have a number of other benefits.
> 
> IC.
> 
> Thanks!
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 

-- 
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: phandles using absolute paths in DT overlays

[Frank Rowand]

Hi Geert,

On 12/9/18 9:49 PM, David Gibson wrote:
> On Fri, Dec 07, 2018 at 09:19:24AM +0100, Geert Uytterhoeven wrote:
>> Hi David,
>>
>> Thanks for your answer!
>>
>> On Fri, Dec 7, 2018 at 2:44 AM David Gibson <david@gibson.dropbear.id.au> wrote:
>>> On Thu, Dec 06, 2018 at 01:56:45PM +0100, Geert Uytterhoeven wrote:
>>>> Some early revisions of SoCs may have hardware bugs that need to be
>>>> fixed up in DT.  Currently we are handling this by including DTS files
>>>> and fixing up nodes and properties, to create different DTB files for
>>>> different SoC revisons (see arch/arm64/boot/dts/renesas/*es1*).
>>>>
>>>> As an alternative, I'm envisioning the use of DT overlays and the
>>>> fdtoverlay tool, in the hope of simplifying the generation of DTBs for
>>>> the various SoC/board combinations.
>>>>
>>>> Ideally, such DTBs would not contain symbols, to avoid inflating DTB
>>>> size.  Hence if fixup overlays would not contain symbolic references,
>>>> there would be no need for symbols.
>>>>
>>>> For anchors, the "&{/path/to/node@address}" syntax is working fine.
>>>> For phandles, while documented on
>>>> https://elinux.org/Device_Tree_Mysteries, and while working fine for the
>>>> non-overlay case, dtc seems to have issues interpreting the DTB:

< snip >

I have updated elinux.org with the limitation for overlays that you report
and that David explained.

-Frank

Re: phandles using absolute paths in DT overlays

[Geert Uytterhoeven]

Hi Frank,

On Mon, Dec 10, 2018 at 11:10 PM Frank Rowand <frowand.list@gmail.com> wrote:
> On 12/9/18 9:49 PM, David Gibson wrote:
> > On Fri, Dec 07, 2018 at 09:19:24AM +0100, Geert Uytterhoeven wrote:
> >> Hi David,
> >>
> >> Thanks for your answer!
> >>
> >> On Fri, Dec 7, 2018 at 2:44 AM David Gibson <david@gibson.dropbear.id.au> wrote:
> >>> On Thu, Dec 06, 2018 at 01:56:45PM +0100, Geert Uytterhoeven wrote:
> >>>> Some early revisions of SoCs may have hardware bugs that need to be
> >>>> fixed up in DT.  Currently we are handling this by including DTS files
> >>>> and fixing up nodes and properties, to create different DTB files for
> >>>> different SoC revisons (see arch/arm64/boot/dts/renesas/*es1*).
> >>>>
> >>>> As an alternative, I'm envisioning the use of DT overlays and the
> >>>> fdtoverlay tool, in the hope of simplifying the generation of DTBs for
> >>>> the various SoC/board combinations.
> >>>>
> >>>> Ideally, such DTBs would not contain symbols, to avoid inflating DTB
> >>>> size.  Hence if fixup overlays would not contain symbolic references,
> >>>> there would be no need for symbols.
> >>>>
> >>>> For anchors, the "&{/path/to/node@address}" syntax is working fine.
> >>>> For phandles, while documented on
> >>>> https://elinux.org/Device_Tree_Mysteries, and while working fine for the
> >>>> non-overlay case, dtc seems to have issues interpreting the DTB:
>
> < snip >
>
> I have updated elinux.org with the limitation for overlays that you report
> and that David explained.

Thanks, LGTM.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds