On Tue, May 21, 2024 at 2:25 PM Conor Dooley <conor@xxxxxxxxxx> 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@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 :( 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@xxxxxxxxxxxxxxxx/thread/DZCZSOCRH5BN7YOXEL2OQKSDIY7DCW2M/
