diff mbox series

[v2,1/2] dt-bindings: input: Add Nuvoton MA35D1 keypad

Message ID 20241112053059.3361-2-mjchen0829@gmail.com
State Changes Requested
Headers show
Series Add support for nuvoton ma35d1 keypad controller | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Ming-Jen Chen Nov. 12, 2024, 5:30 a.m. UTC
Add YAML bindings for MA35D1 SoC keypad.

Signed-off-by: mjchen <mjchen0829@gmail.com>
---
 .../bindings/input/nuvoton,ma35d1-keypad.yaml | 89 +++++++++++++++++++
 1 file changed, 89 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml

Comments

Conor Dooley Nov. 12, 2024, 5:59 p.m. UTC | #1
On Tue, Nov 12, 2024 at 05:30:58AM +0000, mjchen wrote:
> Add YAML bindings for MA35D1 SoC keypad.
> 
> Signed-off-by: mjchen <mjchen0829@gmail.com>
> ---
>  .../bindings/input/nuvoton,ma35d1-keypad.yaml | 89 +++++++++++++++++++
>  1 file changed, 89 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml b/Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml
> new file mode 100644
> index 000000000000..71debafc3890
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml
> @@ -0,0 +1,89 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/nuvoton,ma35d1-keypad.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Nuvoton MA35D1 Keypad
> +
> +maintainers:
> +  - Ming-jen Chen <mjchen0829@gmail.com>
> +
> +allOf:
> +  - $ref: /schemas/input/matrix-keymap.yaml#
> +
> +properties:
> +  compatible:
> +    const: nuvoton,ma35d1-kpi
> +
> +  debounce-period:

Not a common property, so it needs a vendor prefix.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192]
> +    description: |
> +      Key debounce period select, specified in terms of keypad IP clock cycles.
> +      Valid values include 0 (no debounce) and specific clock cycle values:
> +      8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, and 8192.
> +

> +  nuvoton,key-scan-time:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      Set the time it takes to scan each key in the keypad, in clock cycles of the IP.
> +      This parameter controls how frequently the keypad is scanned, adjusting the response time.
> +      The valid range is from 1 to 256 clock cycles.
> +    minimum: 1
> +    maximum: 256
> +
> +  nuvoton,key-scan-time-div:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      Set a divider that adjusts the scan time for each key.
> +      This value scales the time it takes to scan each key
> +      by multiplying the key-scan-time by the specified factor.
> +      For example, if you set key-scan-time to 64 cycles and configure key-scan-time-div to 2,
> +      the scan time for each key will be increased to 128 cycles (64 cycles * 2). time.
> +    minimum: 1
> +    maximum: 256

Why are both of these properties required? Why not just provide a single
value and let the driver figure it out (and why not use the standard
input property poll-interval)?

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - linux,keymap
> +  - debounce-period
> +  - nuvoton,key-scan-time
> +  - nuvoton,key-scan-time-div
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/input/input.h>
> +    keypad@404A0000 {
> +      compatible = "nuvoton,ma35d1-kpi";
> +      reg = <0x404A0000 0x10000>;
> +      interrupts = <79>;
> +      clocks = <&clk>;
> +      keypad,num-rows = <2>;
> +      keypad,num-columns = <2>;
> +
> +      linux,keymap = <
> +         MATRIX_KEY(0, 0, KEY_ENTER)
> +         MATRIX_KEY(0, 1, KEY_ENTER)
> +         MATRIX_KEY(1, 0, KEY_SPACE)
> +         MATRIX_KEY(1, 1, KEY_Z)
> +      >;
> +
> +      debounce-period = <8>;
> +      nuvoton,key-scan-time = <1>;
> +      nuvoton,key-scan-time-div = <24>;
> +    };
> -- 
> 2.25.1
>
Rob Herring (Arm) Nov. 12, 2024, 6:25 p.m. UTC | #2
On Tue, Nov 12, 2024 at 05:30:58AM +0000, mjchen wrote:
> Add YAML bindings for MA35D1 SoC keypad.
> 
> Signed-off-by: mjchen <mjchen0829@gmail.com>
> ---
>  .../bindings/input/nuvoton,ma35d1-keypad.yaml | 89 +++++++++++++++++++
>  1 file changed, 89 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml b/Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml
> new file mode 100644
> index 000000000000..71debafc3890
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml
> @@ -0,0 +1,89 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/nuvoton,ma35d1-keypad.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Nuvoton MA35D1 Keypad
> +
> +maintainers:
> +  - Ming-jen Chen <mjchen0829@gmail.com>
> +
> +allOf:
> +  - $ref: /schemas/input/matrix-keymap.yaml#
> +
> +properties:
> +  compatible:
> +    const: nuvoton,ma35d1-kpi
> +
> +  debounce-period:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    enum: [0, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192]
> +    description: |
> +      Key debounce period select, specified in terms of keypad IP clock cycles.
> +      Valid values include 0 (no debounce) and specific clock cycle values:
> +      8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, and 8192.

No need to list the values twice.

We already have a bunch of debounce time properties. Don't add more. If 
you have the clock frequency, then you can use the existing 
"debounce-delay-ms" and convert to register values.

> +
> +  nuvoton,key-scan-time:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      Set the time it takes to scan each key in the keypad, in clock cycles of the IP.
> +      This parameter controls how frequently the keypad is scanned, adjusting the response time.
> +      The valid range is from 1 to 256 clock cycles.
> +    minimum: 1
> +    maximum: 256
> +
> +  nuvoton,key-scan-time-div:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description: |
> +      Set a divider that adjusts the scan time for each key.
> +      This value scales the time it takes to scan each key
> +      by multiplying the key-scan-time by the specified factor.
> +      For example, if you set key-scan-time to 64 cycles and configure key-scan-time-div to 2,
> +      the scan time for each key will be increased to 128 cycles (64 cycles * 2). time.
> +    minimum: 1
> +    maximum: 256

Again, we have existing properties such as scan-interval, 
scan-interval-ms, and scan-delay. How is this different? 

With a single property in time units, you can solve for how many clock 
cycles.


> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  clocks:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - linux,keymap
> +  - debounce-period
> +  - nuvoton,key-scan-time
> +  - nuvoton,key-scan-time-div
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/input/input.h>
> +    keypad@404A0000 {
> +      compatible = "nuvoton,ma35d1-kpi";
> +      reg = <0x404A0000 0x10000>;
> +      interrupts = <79>;
> +      clocks = <&clk>;

> +      keypad,num-rows = <2>;
> +      keypad,num-columns = <2>;

Surely these should be required?

> +
> +      linux,keymap = <
> +         MATRIX_KEY(0, 0, KEY_ENTER)
> +         MATRIX_KEY(0, 1, KEY_ENTER)
> +         MATRIX_KEY(1, 0, KEY_SPACE)
> +         MATRIX_KEY(1, 1, KEY_Z)
> +      >;
> +
> +      debounce-period = <8>;
> +      nuvoton,key-scan-time = <1>;
> +      nuvoton,key-scan-time-div = <24>;
> +    };
> -- 
> 2.25.1
>
Ming-Jen Chen Nov. 13, 2024, 11:58 p.m. UTC | #3
On 2024/11/13 上午 02:25, Rob Herring wrote:
> On Tue, Nov 12, 2024 at 05:30:58AM +0000, mjchen wrote:
>> Add YAML bindings for MA35D1 SoC keypad.
>>
>> Signed-off-by: mjchen <mjchen0829@gmail.com>
>> ---
>>   .../bindings/input/nuvoton,ma35d1-keypad.yaml | 89 +++++++++++++++++++
>>   1 file changed, 89 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml b/Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml
>> new file mode 100644
>> index 000000000000..71debafc3890
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml
>> @@ -0,0 +1,89 @@
>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/input/nuvoton,ma35d1-keypad.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Nuvoton MA35D1 Keypad
>> +
>> +maintainers:
>> +  - Ming-jen Chen <mjchen0829@gmail.com>
>> +
>> +allOf:
>> +  - $ref: /schemas/input/matrix-keymap.yaml#
>> +
>> +properties:
>> +  compatible:
>> +    const: nuvoton,ma35d1-kpi
>> +
>> +  debounce-period:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    enum: [0, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192]
>> +    description: |
>> +      Key debounce period select, specified in terms of keypad IP clock cycles.
>> +      Valid values include 0 (no debounce) and specific clock cycle values:
>> +      8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, and 8192.
> No need to list the values twice.
>
> We already have a bunch of debounce time properties. Don't add more. If
> you have the clock frequency, then you can use the existing
> "debounce-delay-ms" and convert to register values.
>
>> +
>> +  nuvoton,key-scan-time:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description: |
>> +      Set the time it takes to scan each key in the keypad, in clock cycles of the IP.
>> +      This parameter controls how frequently the keypad is scanned, adjusting the response time.
>> +      The valid range is from 1 to 256 clock cycles.
>> +    minimum: 1
>> +    maximum: 256
>> +
>> +  nuvoton,key-scan-time-div:
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +    description: |
>> +      Set a divider that adjusts the scan time for each key.
>> +      This value scales the time it takes to scan each key
>> +      by multiplying the key-scan-time by the specified factor.
>> +      For example, if you set key-scan-time to 64 cycles and configure key-scan-time-div to 2,
>> +      the scan time for each key will be increased to 128 cycles (64 cycles * 2). time.
>> +    minimum: 1
>> +    maximum: 256
> Again, we have existing properties such as scan-interval,
> scan-interval-ms, and scan-delay. How is this different?
>
> With a single property in time units, you can solve for how many clock
> cycles.
>
I will remove the custom properties.

I introduced and replaced them with the existing properties, such as

sacn-interval-ms, and debounce-delay-ms as you suggested.


>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  interrupts:
>> +    maxItems: 1
>> +
>> +  clocks:
>> +    maxItems: 1
>> +
>> +required:
>> +  - compatible
>> +  - reg
>> +  - interrupts
>> +  - clocks
>> +  - linux,keymap
>> +  - debounce-period
>> +  - nuvoton,key-scan-time
>> +  - nuvoton,key-scan-time-div
>> +
>> +unevaluatedProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/input/input.h>
>> +    keypad@404A0000 {
>> +      compatible = "nuvoton,ma35d1-kpi";
>> +      reg = <0x404A0000 0x10000>;
>> +      interrupts = <79>;
>> +      clocks = <&clk>;
>> +      keypad,num-rows = <2>;
>> +      keypad,num-columns = <2>;
> Surely these should be required?

I will add "keypad,num-rows" and "keypad,num-columes" to the required 
properties in the next verision.


Thank you for the feedback.

As both Conor Dooly and Rob Herring point out, I'll make the changes as 
suggested.

>> +
>> +      linux,keymap = <
>> +         MATRIX_KEY(0, 0, KEY_ENTER)
>> +         MATRIX_KEY(0, 1, KEY_ENTER)
>> +         MATRIX_KEY(1, 0, KEY_SPACE)
>> +         MATRIX_KEY(1, 1, KEY_Z)
>> +      >;
>> +
>> +      debounce-period = <8>;
>> +      nuvoton,key-scan-time = <1>;
>> +      nuvoton,key-scan-time-div = <24>;
>> +    };
>> -- 
>> 2.25.1
>>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml b/Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml
new file mode 100644
index 000000000000..71debafc3890
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/nuvoton,ma35d1-keypad.yaml
@@ -0,0 +1,89 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/nuvoton,ma35d1-keypad.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton MA35D1 Keypad
+
+maintainers:
+  - Ming-jen Chen <mjchen0829@gmail.com>
+
+allOf:
+  - $ref: /schemas/input/matrix-keymap.yaml#
+
+properties:
+  compatible:
+    const: nuvoton,ma35d1-kpi
+
+  debounce-period:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    enum: [0, 8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192]
+    description: |
+      Key debounce period select, specified in terms of keypad IP clock cycles.
+      Valid values include 0 (no debounce) and specific clock cycle values:
+      8, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, and 8192.
+
+  nuvoton,key-scan-time:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Set the time it takes to scan each key in the keypad, in clock cycles of the IP.
+      This parameter controls how frequently the keypad is scanned, adjusting the response time.
+      The valid range is from 1 to 256 clock cycles.
+    minimum: 1
+    maximum: 256
+
+  nuvoton,key-scan-time-div:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Set a divider that adjusts the scan time for each key.
+      This value scales the time it takes to scan each key
+      by multiplying the key-scan-time by the specified factor.
+      For example, if you set key-scan-time to 64 cycles and configure key-scan-time-div to 2,
+      the scan time for each key will be increased to 128 cycles (64 cycles * 2). time.
+    minimum: 1
+    maximum: 256
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - linux,keymap
+  - debounce-period
+  - nuvoton,key-scan-time
+  - nuvoton,key-scan-time-div
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/input/input.h>
+    keypad@404A0000 {
+      compatible = "nuvoton,ma35d1-kpi";
+      reg = <0x404A0000 0x10000>;
+      interrupts = <79>;
+      clocks = <&clk>;
+      keypad,num-rows = <2>;
+      keypad,num-columns = <2>;
+
+      linux,keymap = <
+         MATRIX_KEY(0, 0, KEY_ENTER)
+         MATRIX_KEY(0, 1, KEY_ENTER)
+         MATRIX_KEY(1, 0, KEY_SPACE)
+         MATRIX_KEY(1, 1, KEY_Z)
+      >;
+
+      debounce-period = <8>;
+      nuvoton,key-scan-time = <1>;
+      nuvoton,key-scan-time-div = <24>;
+    };