> From: David Gibson
> Sent: Saturday, August 3, 2024 12:37 AM
> ...
> dtb overlays are kind of a hack layered on top of dtb, and don't have a way to represent things at a granularity less than a whole property (except for special handling of phandles).
I'm going to say that they seem to be a _very_ useful hack.
> Introducing new syntax to dts always requires care to not break compatibility, but that's really the least of the problems here. To implement this you'd need to invent a way of encoding partial property updates into dtb, which would be an incompatible extension. It would need to cover ways of describing the part of the property to be replaced more complex than fixed byte offsets to handle the string list case here.
Maybe I'm being naive, but I think that this syntax could be very simple. It looks to me like the current implementation disallows properties with brackets in the name, so I would propose taking advantage of that to create an invalid property name:
PROPERTY[S*INDEX] = /bits/ BITS VALUE;
where S (single byte element size) is one of 1,2,4,8 or s (string array). I could see "S*" being filled in automatically from the RHS data type (but override-able if you need to do something special). So, in our string array case that means:
PROPERTY[INDEX] = "string";
becomes a property named "PROPERTY[s*INDEX]" with the value "string" when stored in the dtbo.
> It would need to cover ways of describing the part of the property to be replaced more complex than fixed byte offsets to handle the string list case here.
It seems like string arrays are the only complicated case, and by knowing that it's a string array (like with the above "s" size byte) the implementation can count NULL bytes to find the correct element to replace.
> Finally you'd need to update libfdt (and any other overlay implementation out there) to process this new fixup information.
>
> So, yeah, it's not going to happen.
Maybe I'm missing something, but if we're adding the information to the property name in a way that's currently invalid then does that solve this problem? For older utilities, or ones without the handling for this extra information, the utility generates an error or just adds a strangely named property that gets ignored (when I gave this a quick try by hex editing a DTBO, the kernel happily gave me a "gpio-line-names[s*148]").
> However. If you're able to have an actual program on the target processing things, rather than just applying an overlay, then there are more options. You can then do whatever logic you need in your program, using libfdt to read update the dtb. I'm open to adding additional helper functions if they're useful for things here.
For my case I am already using dtmerge on the target to produce the final dtb, so I can easily add a hack to dtmerge to manage this process. That said, I'd like to avoid making something "completely" non-standard. My preference would be to implement the feature in the official tools (in a way that's acceptable to the broader community) and then hack our tools to work with the same thing.
> Aside: you could argue that using a huge list of strings like this is a poor design choice in the gpio binding, instead of, say, having each gpio line as a different subnode using 'reg' to index them explicitly. But, spec design tradeoffs are always tricky, and we're pretty much stuck with it now, anyway.
> ...
Yeah, I understand why they did it the way they did - but it definitely makes this particular task harder. Some of our systems are incredibly storage-constrained, otherwise I would just throw the device tree headers and a compiler on the system and use a #define for each pin.
Best,
Erich E. Hoover, Ph.D.
erich.hoover@xxxxxxxxxxxx
