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@xxxxxxxxxxx> > > --- > > .../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@xxxxxxxxxxx> > > + > > +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.
Attachment:
signature.asc
Description: PGP signature
