From: "Uwe Kleine-König" <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
To: David Gibson <david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org>
Cc: "Uwe Kleine-König"
<uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org>,
devicetree-compiler-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
entwicklung-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org
Subject: Re: [PATCH v2] write_propval_string: Use a list of strings instead of "\0" in a string
Date: Sun, 30 Apr 2023 15:23:08 +0200 [thread overview]
Message-ID: <20230430132308.et66uaghvpenacj2@pengutronix.de> (raw)
In-Reply-To: <ZEy5Xnh/DM/wzUWa@yekko>
[-- Attachment #1: Type: text/plain, Size: 3221 bytes --]
On Sat, Apr 29, 2023 at 04:29:50PM +1000, David Gibson wrote:
> On Fri, Apr 28, 2023 at 01:32:17PM +0200, Uwe Kleine-König wrote:
> > From: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> >
> > A string that contains '\0' can be written as a list of strings e.g.
> >
> > clock-names = "di0_pll\0di1_pll\0di0_sel\0di1_sel\0di2_sel\0di3_sel\0di0\0di1";
> >
> > is equivalent to
> >
> > clock-names = "di0_pll", "di1_pll", "di0_sel", "di1_sel", "di2_sel", "di3_sel", "di0", "di1";
> >
> > The latter is easier to read, to use this format instead.
> >
> > Two test files are adapted accordingly to keep the test suite happy.
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> > ---
> > Changes since (implicit) v1, sent with Message-Id:
> > 20230426182405.572729-1-u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org:
> >
> > - Adapt the test suite
> >
> > tests/type-preservation.dt.yaml | 2 +-
> > tests/type-preservation.dts | 2 +-
> > treesource.c | 2 +-
> > 3 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/tests/type-preservation.dt.yaml b/tests/type-preservation.dt.yaml
> > index a0cc64cc4b69..e238d395aa02 100644
> > --- a/tests/type-preservation.dt.yaml
> > +++ b/tests/type-preservation.dt.yaml
> > @@ -12,7 +12,7 @@
> > int16-matrix: [!u16 [0x1234, 0x5678], [0x90ab, 0xcdef]]
> > int64: [!u64 [0x200000000]]
> > int64-array: [!u64 [0x100000000, 0x0]]
> > - a-string-with-nulls: ["foo\0bar", "baz"]
> > + a-string-array: ["foo", "bar", "baz"]
>
>
> Ah. I was afraid of this. So "fixing" the test highlights another
> problem. It's pretty clear to me that the whole point of this test is
> to test the preservation of the internal \0, as distinct from the
> separate strings later in there. So, this is exercising an edge case
> of the typing markers we now add.
>
> Now.. personally, I've never been particularly convinced that the type
> preservation stuff was a particularly good idea. It will never be
> perfect, and it can give a misleading impression that information is
> preserved into the dtb which isn't. But, it's pretty well established
> now, and I assume people had reasons for wanting it.
>
> So, your patch makes things nicer when going from dtb -> dts, but
> breaks an established feature when going dts -> dts, or dts -> yaml.
> So, I think we need to rethink.
>
> Rather than changing how "strings" are emitted, I think you want to
> change how we guess typing information when coming from dtb: instead
> of just putting a single string marker on the property, we should put
> one after every \0 and we should get the output you're after.
Yeah, sounds reasonable. I looked into that, and I don't see an easy way
to do that. I'll put that aside and concentrate on the other two bigger
changes (i.e. not overwriting phandles by an overlay and label/reference
restoring) first.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2023-04-30 13:23 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-28 11:32 [PATCH v2] write_propval_string: Use a list of strings instead of "\0" in a string Uwe Kleine-König
[not found] ` <20230428113217.744447-1-uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.org>
2023-04-29 6:29 ` David Gibson
2023-04-30 13:23 ` Uwe Kleine-König [this message]
2023-05-01 18:43 ` Rob Herring
[not found] ` <CAL_JsqJ9QManUTkOpgaE85b-uVB_JQZGsLXdJCz7_5J_3Aqq2g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2023-05-03 13:43 ` 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=20230430132308.et66uaghvpenacj2@pengutronix.de \
--to=u.kleine-koenig-bicnvbalz9megne8c9+irq@public.gmane.org \
--cc=david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+@public.gmane.org \
--cc=devicetree-compiler-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
--cc=entwicklung-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org \
--cc=uwe-rXY34ruvC2xidJT2blvkqNi2O/JbrIOy@public.gmane.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;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).