Re: Array manipulation revisit

[David Gibson <david@gibson.dropbear.id.au>]

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

On Mon, Aug 05, 2024 at 04:35:14PM +0000, Hoover, Erich (Orion) wrote:
> > 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.

I won't disagree.

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

Eh, maybe, but as I say that's the least of the problems.

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

Not really.  Mixed properties that have both integers and strings in
them get even worse.  Those aren't common, but they exist.  Or
properties that have a mixture of integer sizes - that's much more
common, since it can include 'reg', 'ranges' and 'interrupts'.

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

Ah, so you're suggesting putting those annotations in the property
name itself inside the dtbo.  That's not a bad idea as encodings go.

> 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]").

Right, but then you silently get output that wasn't what you expected
which can be nasty to debug.

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

Well, that raises another issue: this would add a substantial amount
of code to libfdt, which is supposed to be runnable in very
constrained environments.

> 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@arcfield.com

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