Re: [PATCH 09/14] dt-bindings: input: samsung,s3c6410-keypad: introduce compact binding

Dmitry Torokhov
2024-08-19T15:49:14Z

On Mon, Aug 19, 2024 at 03:02:07PM +0200, Krzysztof Kozlowski wrote:
> On Sun, Aug 18, 2024 at 09:58:06PM -0700, Dmitry Torokhov wrote:
> > The binding with a sub-node per each key is very verbose and is hard to
> > use with static device properties. Allow standard matrix keymap binding
> > in addition to the verbose one.
> >
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> > .../input/samsung,s3c6410-keypad.yaml | 57 ++++++++++++++++++-
> > 1 file changed, 54 insertions(+), 3 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml b/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml
> > index a53569aa0ee7..28a318a0ff7e 100644
> > --- a/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml
> > +++ b/Documentation/devicetree/bindings/input/samsung,s3c6410-keypad.yaml
> > @@ -16,6 +16,10 @@ description:
> > maintainers:
> > - Krzysztof Kozlowski <krzk@kernel.org>
> >
> > +allOf:
> > + - $ref: input.yaml#
> > + - $ref: matrix-keymap.yaml#
> > +
> > properties:
> > compatible:
> > enum:
> > @@ -37,6 +41,10 @@ properties:
> >
> > wakeup-source: true
> >
> > + keypad,num-columns: true
> > + keypad,num-rows: true
> > + linux,keymap: true
> > +
> > linux,input-no-autorepeat:
> > type: boolean
> > description:
> > @@ -81,12 +89,33 @@ patternProperties:
> > - keypad,row
> > - linux,code
> >
> > +dependencies:
> > + linux,keymap:
> > + required:
>
> Why "required" keyword? The dependencies should have just list of
> properties. See example-schema.

OK, changed this to

dependencies:
linux,keymap: [ "keypad,num-columns", "keypad,num-rows" ]

>
> > + - keypad,num-columns
> > + - keypad,num-rows
> > +
> > required:
> > - compatible
> > - reg
> > - interrupts
> > - - samsung,keypad-num-columns
> > - - samsung,keypad-num-rows
> > +
> > +if:
>
> put allOf: here and this within allOf, so you the "if" could grow in the
> future.

Hmm, there is already "allOf" at the beginning of the file, so adding
another one results in complaints about duplicate "allOf". I can move it
all to the top, like this:

allOf:
- $ref: input.yaml#
- $ref: matrix-keymap.yaml#
- if:
required:
- linux,keymap
then:
properties:
samsung,keypad-num-columns: false
samsung,keypad-num-rows: false
patternProperties:
'^key-[0-9a-z]+$': false
else:
properties:
keypad,num-columns: false
keypad,num-rows: false
required:
- samsung,keypad-num-columns
- samsung,keypad-num-rows

Is this OK? I don't quite like that "tweaks" are listed before main
body of properties.

Thanks.

--
Dmitry