| Message ID | 20240819045813.2154642-10-dmitry.torokhov@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | Remove support for platform data from samsung keypad | expand |
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.
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.
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; + };
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(-)