public inbox for devicetree-compiler@vger.kernel.org
 help / color / mirror / Atom feed
From: David Gibson <david@gibson.dropbear.id.au>
To: Andreas Gnau <andreas.gnau@iopsys.eu>
Cc: Ayush Singh <ayush@beagleboard.org>,
	d-gole@ti.com, lorforlinux@beagleboard.org,
	jkridner@beagleboard.org, robertcnelson@beagleboard.org,
	nenad.marinkovic@mikroe.com, Andrew Davis <afd@ti.com>,
	Geert Uytterhoeven <geert@linux-m68k.org>,
	Robert Nelson <robertcnelson@gmail.com>,
	devicetree-compiler@vger.kernel.org,
	Simon Glass <sjg@chromium.org>
Subject: Re: [PATCH v3 0/2] Add capability to append to property
Date: Mon, 16 Dec 2024 17:09:55 +1100	[thread overview]
Message-ID: <Z1_EM1dEmN4KHhBr@zatzit> (raw)
In-Reply-To: <58c44f5d-7b45-4489-bf26-2be36c44e39a@iopsys.eu>

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

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.

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

  parent reply	other threads:[~2024-12-16  6:10 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-11  9:54 [PATCH v3 0/2] Add capability to append to property Ayush Singh
2024-11-11  9:54 ` [PATCH v3 1/2] dtc: Add /append-property/ Ayush Singh
2024-11-15  4:07   ` Dhruva Gole
2024-11-11  9:54 ` [PATCH v3 2/2] tests: Add test for append-property Ayush Singh
2024-12-04 13:29 ` [PATCH v3 0/2] Add capability to append to property Ayush Singh
2024-12-10 15:34 ` Andreas Gnau
2024-12-11  4:51   ` Ayush Singh
2024-12-16  6:09   ` David Gibson [this message]
2024-12-20 15:30     ` Ayush Singh
2024-12-26  6:54       ` David Gibson
2024-12-26 11:10         ` Geert Uytterhoeven
2024-12-27  4:05           ` David Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Z1_EM1dEmN4KHhBr@zatzit \
    --to=david@gibson.dropbear.id.au \
    --cc=afd@ti.com \
    --cc=andreas.gnau@iopsys.eu \
    --cc=ayush@beagleboard.org \
    --cc=d-gole@ti.com \
    --cc=devicetree-compiler@vger.kernel.org \
    --cc=geert@linux-m68k.org \
    --cc=jkridner@beagleboard.org \
    --cc=lorforlinux@beagleboard.org \
    --cc=nenad.marinkovic@mikroe.com \
    --cc=robertcnelson@beagleboard.org \
    --cc=robertcnelson@gmail.com \
    --cc=sjg@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox