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: <20240521-unskilled-spindle-c4161092ae83@spud>
Date: Tue, 21 May 2024 20:25:10 +0100
From: Conor Dooley <conor@...nel.org>
To: Elliot Berman <quic_eberman@...cinc.com>
Cc: Rob Herring <robh+dt@...nel.org>, 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 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 :(

Thanks,
Conor.


Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)