Re: dtc 1.6.0 overlay full-path reference in array produces broken property names

[David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>]

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

On Mon, Nov 01, 2021 at 05:19:20PM +1100, David Gibson wrote:
> On Mon, Sep 27, 2021 at 02:04:56PM +0000, Vincent Pelletier wrote:
> > (please keep me cc'ed in replies)
> > 
> > Hello,
> > 
> > I suspect this may be a dtc bug, but I am not too familiar with
> > devicetree specs to tell.
> > 
> > Here is a simplified example reproducing this issue:
> > 
> > (SNIP)
> > /dts-v1/;
> > /plugin/;
> > 
> > / {
> >   compatible = "foo";
> > };
> > 
> > &{/soc/gpio@7e200000} {
> >     status = "okay";
> > };
> > 
> > &{/soc/spi@7e204000} {
> >     cs-gpios = <&{/soc/gpio@7e200000} 8 0x01>;
> > };
> > (SNIP)
> > 
> > This code builds fine:
> >   $ dtc -I dts -O dtb -o test.dtbo test.dts
> >   $
> > But then when disassembling the output, I get:
> >   $ dtc -I dtb -O dts test.dtbo
> >   <stdout>: ERROR (property_name_chars): /__fixups__:/soc/gpio@7e200000: Bad character '/' in property name
> >   ERROR: Input tree has errors, aborting (use -f to force output)
> > And forcing output:
> >   $ dtc -I dtb -O dts -f test.dtbo
> >   <stdout>: ERROR (property_name_chars): /__fixups__:/soc/gpio@7e200000: Bad character '/' in property name
> >   Warning: Input tree has errors, output forced
> >   /dts-v1/;
> > 
> >   / {
> >           compatible = "foo";
> > 
> >           fragment@0 {
> >                   target-path = "/soc/gpio@7e200000";
> > 
> >                   __overlay__ {
> >                           status = "okay";
> >                   };
> >           };
> > 
> >           fragment@1 {
> >                   target-path = "/soc/spi@7e204000";
> > 
> >                   __overlay__ {
> >                           cs-gpios = <0xffffffff 0x08 0x01>;
> >                   };
> >           };
> > 
> >           __fixups__ {
> >                   /soc/gpio@7e200000 = "/fragment@1/__overlay__:cs-gpios:0";
> >           };
> >   };
> > 
> > Indeed, the node in __fixups__ has an invalid name.
> > 
> > I suspect this is uncommon because:
> > - it probably only occurs in overlays and not in self-contained trees,
> >   as a self-contained tree would likely produce an internal phandle and
> >   never store the full path anywhere.
> > - ...which does not attach to labels but full paths
> >   Labels being (probably ?) valid node names, they would avoid the issue.
> > - ...in arrays
> >   In a node overlay, the full path ends up as a quoted string, not as a
> >   name, so it is safe.
> > 
> > Is this a dtc bug ?
> 
> I've looked at this again and... not really.
> 
> Unfortunately, such a reference can't work given the specification of
> the overlay format.  The property name in fixups is the target of the
> reference, which works fine for labels, but will necessarily result in
> an invalid property name for path references.
> 
> dtc should, of course, have a less ugly failure mode.

I finally got a chance to tackle this, and just sent out a patch which
should make this case fail on attempting to build the dtbo in the
first place, rather than later when processing the bad output.

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