linux-kernel - Re: [PATCH RFC v3 2/9] dt-bindings: board: Introduce board-id

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAL_JsqJjvNfr_Vo7JNsBn6Fcv5r--ZqQTiforSLwrZ-z4_mURQ@mail.gmail.com>
Date: Tue, 21 May 2024 16:32:16 -0500
From: Rob Herring <robh+dt@...nel.org>
To: Conor Dooley <conor@...nel.org>
Cc: Elliot Berman <quic_eberman@...cinc.com>, Frank Rowand <frowand.list@...il.com>, 
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>, Conor Dooley <conor+dt@...nel.org>, 
	Bjorn Andersson <andersson@...nel.org>, Konrad Dybcio <konrad.dybcio@...aro.org>, 
	Amrit Anand <quic_amrianan@...cinc.com>, Peter Griffin <peter.griffin@...aro.org>, 
	Caleb Connolly <caleb.connolly@...aro.org>, Andy Gross <agross@...nel.org>, 
	Doug Anderson <dianders@...omium.org>, Simon Glass <sjg@...omium.org>, 
	Chen-Yu Tsai <wenst@...omium.org>, Julius Werner <jwerner@...omium.org>, 
	"Humphreys, Jonathan" <j-humphreys@...com>, Sumit Garg <sumit.garg@...aro.org>, 
	Jon Hunter <jonathanh@...dia.org>, Michal Simek <michal.simek@....com>, 
	boot-architecture@...ts.linaro.org, devicetree@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, 
	linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH RFC v3 2/9] dt-bindings: board: Introduce board-id

On Tue, May 21, 2024 at 2:25 PM Conor Dooley <conor@...nel.org> wrote:
>
> On Tue, May 21, 2024 at 08:21:45PM +0100, Conor Dooley wrote:
> > On Tue, May 21, 2024 at 11:37:59AM -0700, Elliot Berman wrote:
> > > Device manufcturers frequently ship multiple boards or SKUs under a
> > > single softwre package. These software packages ship multiple devicetree
> > > blobs and require some mechanims to pick the correct DTB for the boards
> > > that use the software package.
> >
> > Okay, you've got the problem statement here, nice.
> >
> > > This patch introduces a common language
> > > for adding board identifiers to devicetrees.
> >
> > But then a completely useless remainder of the commit message.
> > I open this patch, see the regexes, say "wtf", look at the commit
> > message and there is absolutely no explanation of what these properties
> > are for. That's quite frankly just not good enough - even for an RFC.
> >
> > >
> > > Signed-off-by: Elliot Berman <quic_eberman@...cinc.com>
> > > ---
> > >  .../devicetree/bindings/board/board-id.yaml        | 24 ++++++++++++++++++++++
> > >  1 file changed, 24 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/board/board-id.yaml b/Documentation/devicetree/bindings/board/board-id.yaml
> > > new file mode 100644
> > > index 000000000000..99514aef9718
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/board/board-id.yaml
> > > @@ -0,0 +1,24 @@
> > > +# SPDX-License-Identifier: BSD-2-Clause
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/board/board-id.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: board identifiers
> > > +description: Common property for board-id subnode
> >
> > s/property/properties/
> >
> > > +
> > > +maintainers:
> > > +  - Elliot Berman <quic_eberman@...cinc.com>
> > > +
> > > +properties:
> > > +  $nodename:
> > > +    const: '/'
> > > +  board-id:
> > > +    type: object
> > > +    patternProperties:
> > > +      "^.*(?!_str)$":
> >
> > Does this regex even work? Take "foo_str" as an example - doesn't "^.*"
> > consume all of the string, leaving the negative lookahead with nothing
> > to object to? I didn't properly test this with an example and the dt
> > tooling, but I lazily threw it into regex101 and both the python and
> > emcascript versions agree with me. Did you test this?
> >
> > And while I am here, no underscores in property names please. And if
> > "str" means string, I suggest not saving 3 characters.
> >
> > > +        $ref: /schemas/types.yaml#/definitions/uint32-matrix
> > > +      "^.*_str$":
> > > +        $ref: /schemas/types.yaml#/definitions/string-array
> >
> > Why do we even need two methods? Commit message tells me nothing and
> > there's no description at all... Why do we need regexes here, rather
> > than explicitly defined properties? Your commit message should explain
> > the justification for that and the property descriptions (as comments if
> > needs be for patternProperties) should explain why this is intended to
> > be used.
> >
> > How is anyone supposed to look at this binding and understand how it
> > should be used?
>
> Also, please do not CC private mailing lists on your postings, I do not
> want to get spammed by linaro's mailman :(

boot-architecture is not private[0]. It is where EBBR gets discussed
amongst other things. This came up in a thread there[1].

Rob

[0] https://lists.linaro.org/mailman3/lists/boot-architecture.lists.linaro.org/
[1] https://lists.linaro.org/archives/list/boot-architecture@lists.linaro.org/thread/DZCZSOCRH5BN7YOXEL2OQKSDIY7DCW2M/