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

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

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