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:
+ - keypad,num-columns
+ - keypad,num-rows
+
required:
- compatible
- reg
- interrupts
- - samsung,keypad-num-columns
- - samsung,keypad-num-rows
+
+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
additionalProperties: false
@@ -94,8 +123,9 @@ examples:
- |
#include <dt-bindings/clock/exynos4.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/input/input.h>
- keypad@100a0000 {
+ keypad1@100a0000 {
compatible = "samsung,s5pv210-keypad";
reg = <0x100a0000 0x100>;
interrupts = <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>;
@@ -119,3 +149,24 @@ examples:
linux,code = <3>;
};
};
+ - |
+ #include <dt-bindings/clock/exynos4.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/input/input.h>
+
+ keypad2@100a0000 {
+ compatible = "samsung,s5pv210-keypad";
+ reg = <0x100a0000 0x100>;
+ interrupts = <GIC_SPI 109 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clock CLK_KEYIF>;
+ clock-names = "keypad";
+
+ keypad,num-rows = <2>;
+ keypad,num-columns = <8>;
+ linux,keymap = <
+ MATRIX_KEY(0, 3, 2)
+ MATRIX_KEY(0, 4, 3)
+ >;
+ linux,input-no-autorepeat;
+ wakeup-source;
+ };
--
2.46.0.184.g6999bdac58-googOn 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. > + - 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. Best regards, Krzysztof
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
On Mon, Aug 19, 2024 at 08:49:10AM -0700, Dmitry Torokhov wrote: > 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: > > > > > + - 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. The normal thing to do is to put the allOf at the end, not the start, in cases like this, for the reason you mention.
On Mon, Aug 19, 2024 at 05:48:06PM +0100, Conor Dooley wrote: > On Mon, Aug 19, 2024 at 08:49:10AM -0700, Dmitry Torokhov wrote: > > 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: > > > > > > > > + - 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. > > The normal thing to do is to put the allOf at the end, not the start, in > cases like this, for the reason you mention. I see, thanks. It would be nice if it could combine several "allOf"s into one internally. Thanks. -- Dmitry
© 2016 - 2026 Red Hat, Inc.