Re: [PATCH v3 0/2] Add capability to append to property

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

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

On Fri, Dec 20, 2024 at 09:00:14PM +0530, Ayush Singh wrote:
> 
> On 16/12/24 11:39, David Gibson wrote:
> > On Tue, Dec 10, 2024 at 04:34:17PM +0100, Andreas Gnau wrote:
> > > On 2024-11-11 10:54, Ayush Singh wrote:
> > > > Allow appending values to a property instead of overriding the previous
> > > > values of property.
> > > > 
> > > > Currently, we have /delete-node/ and /delete-property/, but lack
> > > > /append-property/. Hence we end up having to repeat all existing values
> > > > when appending to a property (e.g. see [1] appending to clocks from
> > > > [2]).
> > > > 
> > > > This functionality is also important for creating a device tree based
> > > > implementation to support different types of addon-boards such as
> > > > mikroBUS, Grove [3], etc.
> > > > 
> > > > In practice, it looks as follows:
> > > > 
> > > > ```
> > > > dts-v1/;
> > > > 
> > > > / {
> > > > 	str-prop = "0";
> > > > };
> > > > 
> > > > / {
> > > > 	/append-property/ str-prop = "1";
> > > > };
> > > > ```
> > > If we add /append-property/, why not add /prepend-property/ as well? This is
> > > not only "nice for consistency", but it would also enable solving a problem
> > > where SoC compatible strings from dtsi [1] need to be repeated in board dts
> > > [2], because one cannot prepend to an existing property.
> > > 
> > > ```
> > > dts-v1/;
> > > / {
> > > 	compatible = "soc-vendor,soc1234";
> > > };
> > > 
> > > / {
> > > 	/prepend-property/ compatible = "board-vendor,board-xyz";
> > > };
> > > ```
> > > 
> > > What do you think?
> > So, this kind of demonstrates why I don't love /append-property/ as a
> > proposed syntax.  The way I'd prefer to do this, ideally, is to allow
> > properties to be described as expressions.  We already have integer
> > expressions that can be used in < > context, but I had intended to
> > extent to string and bytestring expressions - but I've never had the
> > time to implement that.
> > 
> > Under that proposal, I'd expect appending to look something like:
> > 	str-prop = /previous-value/, "1";
> > 
> > Prepending,
> > 	str-prop = "1", /previous-value/;
> > 
> > .. and you could do both at once in the obvious way.
> > 
> > The downside, of course, is that this is a much more complicated
> > proposal to implement.  Parsing that syntax isn't too hard, but I
> > think doing it sensibly will need some structural changes in order to
> > evaluate property values as expressions, rather than simply
> > constructing the properties directly left to right.  In particular the
> > interactions between expression syntax and within-property labels (and
> > other markers) could be fiddly to get right.
> > 
> 
> Do you wish to allow `/previous-value/` to be repeated in the same property?
> Something like this:
> 
>     str-prop = "1", /previous-value/, "2", /previous-value/, "3";

Yes, I'd expect that to work.  Note that I don't particularly like the
name "/previous-value/" - that was just an example to demonstrate what
I had in mind.  If anyone can think of a better or more succinct name
for this, please suggest it.

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