On Fri, Sep 20, 2024 at 10:04:56PM +0530, Ayush Singh wrote:
> On 9/18/24 08:06, David Gibson wrote:
>
> > On Mon, Sep 16, 2024 at 03:10:52PM +0530, Ayush Singh wrote:
> > > On 9/12/24 09:08, David Gibson wrote:
> > >
> > > > On Mon, Sep 09, 2024 at 12:54:34PM +0530, Ayush Singh wrote:
> > > > > On 9/9/24 10:33, David Gibson wrote:
> > > > >
> > > > > > On Mon, Sep 02, 2024 at 05:47:55PM +0530, Ayush Singh wrote:
> > > > > > > Add ability to resolve symbols pointing to phandles instead of strings.
> > > > > > >
> > > > > > > Combining this with existing fixups infrastructure allows creating
> > > > > > > symbols in overlays that refer to undefined phandles. This is planned to
> > > > > > > be used for addon board chaining [1].
> > > > > > I don't think this "autodetection" of whether the value is a phandle
> > > > > > or path is a good idea. Yes, it's probably unlikely to get it wrong
> > > > > > in practice, but sloppy cases like this have a habit of coming back to
> > > > > > bite you later on. If you want this, I think you need to design a new
> > > > > > way of encoding the new options.
> > > > > Would something like `__symbols_phandle__` work?
> > > > Preferable to the overloaded values in the original version, certainly.
> > > I can whip it up if that would be sufficient. But if we are talking about
> > > any big rewrite, well, I would like to get the details for that sorted out
> > > first.
> > Fair enough.
> >
> > > > > It should be fairly
> > > > > straightforward to do. I am not sure how old devicetree compilers will react
> > > > > to it though?
> > > > Well, the devicetree compiler itself never actually looks at the
> > > > overlay encoding stuff. The relevant thing is libfdt's overlay
> > > > application logic... and any other implementations of overlay handling
> > > > that are out there.
> > > >
> > > > At which I should probably emphasize that changes to the overlay
> > > > format aren't really mine to approve or not. It's more or less an
> > > > open standard, although not one with a particularly formal definition.
> > > > Of course, libfdt's implementation - and therefore I - do have a fair
> > > > bit of influence on what's considered the standard.
> > > So do I need to start a discussion for this somewhere other than the
> > > devicetree-compiler mailing list? Since ZephyrRTOS is also using devicetree,
> > > maybe things need to be discussed with them as well?
> > <devicetree-spec@xxxxxxxxxxxxxxx> and <devicetree@xxxxxxxxxxxxxxx> are
> > the obvious candidate places. There will be substantial overlap with
> > devicetree-compiler of course, but not total probably.
> >
> > > > > I really do not think having a different syntax for phandle symbols would be
> > > > > good since that would mean we will have 2 ways to represent phandles:
> > > > >
> > > > > 1. For __symbols__
> > > > >
> > > > > 2. For every other node.
> > > > I'm really not sure what you're getting at here.
> > > I just meant that I would like to keep the syntax the same:
> > >
> > > sym = <&abc>;
> > Ok, the syntax for creating them in dts, rather than the encoding
> > within the dtb. Why are you manually creating symbols?
> >
> > So.. aside from all the rest, I don't really understand why you want
> > to encode the symbols as phandles rather than paths.
>
> It's for connector stacking using the approach described here [0].
Thanks for the link.
> To go into more detail, let us assume that we have a mikrobus connector on
> the base board. We connect an addon board that exposes a grove connector.
> Now to describe the parent i2c adapter of the grove connector, we cannot
> really specify the full node path. However, having support for phandles
> would make writing the overlay for such an addon board possible.
>
> In practice it might look as follows:
>
>
> mikrobus-connector.dtso
>
>
> &{/} {
>
> __symbols__ {
>
> MIKROBUS_SCL_I2C = "path";
>
> ...
>
> };
>
> }
>
>
> grove-connector-addon.dtso
>
>
> &{/} {
>
> __symbols__ {
>
> GROVE_PIN1_I2C = <&MIKROBUS_SCL_I2C>;
>
> };
>
> };
So, essentially you're just adding new labels as aliases to existing
labels?
Ok, I can see at least two ways of doing that which I think are a more
natural fit than allowing symbols to be phandles.
# Method 1: Allow path references in overlays
dts allows references both in integer context:
foo = <&bar>;
in which case it resolves to a phandle, but also in string/bytestring context:
foo = &bar;
In which case it resolves to a path.
Runtime overlays, only support the former, but could be extended to
support the latter form. It would be a trickier than phandle
references, because updating a path reference would require expanding
/ shrinking the property including it, but I don't think it's super
difficult.
It might be somewhat harder to imlpement than your original proposal,
but it's conceptually simpler and more versatile. In fact it removes
a currently existing asymmetry between what dts and overlays can do.
# Method 2: /aliases
/__symbols__ is very similar to the much older IEEE1275 concept of
/aliases. In fact they're encoded identically except for appearing in
/aliases instead of /__symbols__. The original use for these was in
interactive Open Firmware, so you could type, say, "dev hd" instead of
"dev /pci@XXXXXXXX/scsi@X,Y/lun@XX/..." or whatever path the primary
hard disk had. Arguably, __symbols__ should never have been invented,
and we should have just used /aliases. This is the kind of thing I
mean when I say they overlay encoding wasn't very well thought out.
But, here's the thing:
a) aliases can be defined in terms of other aliases:
aliases {
scsi0 = "/pci@XXXXX/pci-bridge@X,Y/scsi@X,Y";
hd = "scsi0/lun@YY";
}
b) fdt_path_offset() already resolves aliases
If given a path without a leading '/', it will look up the first
component as an alias, then look up the rest of the path relative to
that.
So, in your example, if the base layer defined MIKROBUS_SCL_I2C as
an alias rather than a symbol, then in the next layer you could have:
&{/} {
aliases {
GROVE_PIN1_I2C = "MIKROBUS_SCL_I2C";
}
}
libfdt should already resolve this when applying overlays, because it
just calls fdt_path_offset().
So your only remainingly difficulty is /aliases versus /__symbols__.
It would be fairly simple to make overlay application expand
__symbols__ in much the same way as aliases. Or, you could just have
a copy of everything in __symbols__ in aliases as well (which might be
a path to eventually deprecating /__symbols__). dtc already has the
-A flag which will but all labels into /aliases, very much like -@
will put them all in /__symbols__.
[snip]
> Well, a lot of work is still going in this direction [1]. I myself
> am trying to use it for mikroBUS connectors.
Sure, and I can see why: it seems tantalizingly close to working
simply. But that doesn't mean it won't have limitations that will
bite you down the track.
> The reason for using devicetree overlays for such connectors is
> because of their loose nature (mikroBUS is also open
> connector). This means that as long as the sensor/ device can make
> do with the pins provided by mikroBUS connector, it can be an addon
> board. And there is no limitation of staking the boards. So one can
> do rpi hat -> mikrbus connectors -> grove connector -> grove some
> addon board.
For example, the very fact that these are loose exposes you to one
pretty obvious limitation here. Suppose some future board has a
connector with enough pins that you can hang two independent grove
connectors off it, and someone makes a hat/shield/whatever that does
exactly that. But now the grove addon dtbs need to be different
depending on whether they attach to grove connector 1 or grove
connector 2.
The old "connector binding" proposals I was describing aimed to
decouple the type of the connector from the instance of the connector
for exactly this sort of case.
--
David Gibson (he or they) | 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:
signature.asc
Description: PGP signature
