diff mbox series

[input-next,2/4] dt-bindings: input: mpr121: Add poll-interval property

Message ID 1567424417-3914-3-git-send-email-michal.vokac@ysoft.com (mailing list archive)
State Superseded
Headers show
Series Add polling mode to the MPR121 touchkey | expand

Commit Message

Michal Vokáč Sept. 2, 2019, 11:40 a.m. UTC
Add an option to periodicaly poll the device to get the buttons states
as the interrupt line may not be used on some platforms.

Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
---
I am not sure how to propperly handle this.
Either interrupt or linux,poll-interval is required, but not both.

 .../bindings/input/fsl,mpr121-touchkey.yaml          | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

Comments

Rob Herring (Arm) Sept. 3, 2019, 7:01 a.m. UTC | #1
On Mon, Sep 02, 2019 at 01:40:15PM +0200, Michal Vokáč wrote:
> Add an option to periodicaly poll the device to get the buttons states
> as the interrupt line may not be used on some platforms.
> 
> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
> ---
> I am not sure how to propperly handle this.
> Either interrupt or linux,poll-interval is required, but not both.

Add this at the top level:

oneOf:
  - required: [ interrupts ]
  - required: [ linux,poll-interval ]

> 
>  .../bindings/input/fsl,mpr121-touchkey.yaml          | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/input/fsl,mpr121-touchkey.yaml b/Documentation/devicetree/bindings/input/fsl,mpr121-touchkey.yaml
> index c463c1c81755..2b3073a3c9f4 100644
> --- a/Documentation/devicetree/bindings/input/fsl,mpr121-touchkey.yaml
> +++ b/Documentation/devicetree/bindings/input/fsl,mpr121-touchkey.yaml
> @@ -34,6 +34,10 @@ properties:
>      minItems: 1
>      maxItems: 12
>  
> +  linux,poll-interval:
> +    description: Poll interval time in milliseconds.
> +    maxItems: 1

We already have 'poll-interval' in several bindings. Use that.

This should have a type definition and you don't need maxItems:

$ref: /schemas/types.yaml#/definitions/uint32

Really this should go in a common input schema doc.
> +
>    wakeup-source: Use any event on keypad as wakeup event.
>      type: boolean
>  
> @@ -44,12 +48,12 @@ properties:
>  required:
>    - compatible
>    - reg
> -  - interrupts
>    - vdd-supply
>    - linux,keycodes
>  
>  examples:
>    - |
> +    // Example with interrupts
>      #include "dt-bindings/input/input.h"
>      touchkey: mpr121@5a {
>          compatible = "fsl,mpr121-touchkey";
> @@ -62,3 +66,17 @@ examples:
>                           <KEY_4> <KEY_5>, <KEY_6>, <KEY_7>,
>                           <KEY_8>, <KEY_9>, <KEY_A>, <KEY_B>;
>      };
> +
> +  - |
> +    // Example with polling
> +    #include "dt-bindings/input/input.h"
> +    touchkey: mpr121@5a {
> +        compatible = "fsl,mpr121-touchkey";
> +        reg = <0x5a>;
> +        linux,poll-interval = <20>;
> +        autorepeat;
> +        vdd-supply = <&ldo4_reg>;
> +        linux,keycodes = <KEY_0>, <KEY_1>, <KEY_2>, <KEY_3>,
> +                         <KEY_4> <KEY_5>, <KEY_6>, <KEY_7>,
> +                         <KEY_8>, <KEY_9>, <KEY_A>, <KEY_B>;
> +    );
> -- 
> 2.1.4
>
Michal Vokáč Sept. 17, 2019, 2:17 p.m. UTC | #2
On 03. 09. 19 9:01, Rob Herring wrote:
> On Mon, Sep 02, 2019 at 01:40:15PM +0200, Michal Vokáč wrote:
>> Add an option to periodicaly poll the device to get the buttons states
>> as the interrupt line may not be used on some platforms.
>>
>> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
>> ---
>> I am not sure how to propperly handle this.
>> Either interrupt or linux,poll-interval is required, but not both.
> 
> Add this at the top level:
> 
> oneOf:
>    - required: [ interrupts ]
>    - required: [ linux,poll-interval ]
>

Nice, works as expected.
  
>>
>>   .../bindings/input/fsl,mpr121-touchkey.yaml          | 20 +++++++++++++++++++-
>>   1 file changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/input/fsl,mpr121-touchkey.yaml b/Documentation/devicetree/bindings/input/fsl,mpr121-touchkey.yaml
>> index c463c1c81755..2b3073a3c9f4 100644
>> --- a/Documentation/devicetree/bindings/input/fsl,mpr121-touchkey.yaml
>> +++ b/Documentation/devicetree/bindings/input/fsl,mpr121-touchkey.yaml
>> @@ -34,6 +34,10 @@ properties:
>>       minItems: 1
>>       maxItems: 12
>>   
>> +  linux,poll-interval:
>> +    description: Poll interval time in milliseconds.
>> +    maxItems: 1
> 
> We already have 'poll-interval' in several bindings. Use that.

OK, will do.
linux,poll-intervall was initially suggested by Dmitry but AFAICT he is OK
with this.

> This should have a type definition and you don't need maxItems:
> 
> $ref: /schemas/types.yaml#/definitions/uint32

OK.

> Really this should go in a common input schema doc.

I am not sure how to deal with this. Do you suggest to create

  Documentation/devicetree/bindings/input/input.yaml

and put all the common input properties there? Something like:

diff --git a/Documentation/devicetree/bindings/input/input.yaml b/Documentation/devicetree/bindings/input/input.yaml
new file mode 100644
index 000000000000..5dc10ebdfdf0
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/input.yaml
@@ -0,0 +1,24 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/input.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Common input schema binding
+
+maintainers:
+  - Dmitry Torokhov <dmitry.torokhov@gmail.com>
+
+properties:
+  linux,keycodes:
+    description:
+      Specifies an array of numeric keycode values to be used for reporting
+      button presses. The array can contain up to 12 entries.
+    minItems: 1
+    maxItems: 12
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
+  poll-interval:
+    description: Poll interval time in milliseconds.
+    $ref: /schemas/types.yaml#/definitions/uint32
+
---

Or something else?
And what is the impact on the fsl,mpr121-touchkey.yaml content/format?
I could not find any example in any subsystem of such approach.

Thank you,
Michal

>> +
>>     wakeup-source: Use any event on keypad as wakeup event.
>>       type: boolean
>>   
>> @@ -44,12 +48,12 @@ properties:
>>   required:
>>     - compatible
>>     - reg
>> -  - interrupts
>>     - vdd-supply
>>     - linux,keycodes
>>   
>>   examples:
>>     - |
>> +    // Example with interrupts
>>       #include "dt-bindings/input/input.h"
>>       touchkey: mpr121@5a {
>>           compatible = "fsl,mpr121-touchkey";
>> @@ -62,3 +66,17 @@ examples:
>>                            <KEY_4> <KEY_5>, <KEY_6>, <KEY_7>,
>>                            <KEY_8>, <KEY_9>, <KEY_A>, <KEY_B>;
>>       };
>> +
>> +  - |
>> +    // Example with polling
>> +    #include "dt-bindings/input/input.h"
>> +    touchkey: mpr121@5a {
>> +        compatible = "fsl,mpr121-touchkey";
>> +        reg = <0x5a>;
>> +        linux,poll-interval = <20>;
>> +        autorepeat;
>> +        vdd-supply = <&ldo4_reg>;
>> +        linux,keycodes = <KEY_0>, <KEY_1>, <KEY_2>, <KEY_3>,
>> +                         <KEY_4> <KEY_5>, <KEY_6>, <KEY_7>,
>> +                         <KEY_8>, <KEY_9>, <KEY_A>, <KEY_B>;
>> +    );
>> -- 
>> 2.1.4
>>
Michal Vokáč Sept. 27, 2019, 6:47 a.m. UTC | #3
On 17. 09. 19 16:17, Michal Vokáč wrote:
> On 03. 09. 19 9:01, Rob Herring wrote:
>> On Mon, Sep 02, 2019 at 01:40:15PM +0200, Michal Vokáč wrote:
>>> Add an option to periodicaly poll the device to get the buttons states
>>> as the interrupt line may not be used on some platforms.
>>>
>>> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
>>> ---
>>> I am not sure how to propperly handle this.
>>> Either interrupt or linux,poll-interval is required, but not both.
>>
>> Add this at the top level:
>>
>> oneOf:
>>    - required: [ interrupts ]
>>    - required: [ linux,poll-interval ]
>>
> 
> Nice, works as expected.
> 
>>>
>>>   .../bindings/input/fsl,mpr121-touchkey.yaml          | 20 +++++++++++++++++++-
>>>   1 file changed, 19 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/input/fsl,mpr121-touchkey.yaml b/Documentation/devicetree/bindings/input/fsl,mpr121-touchkey.yaml
>>> index c463c1c81755..2b3073a3c9f4 100644
>>> --- a/Documentation/devicetree/bindings/input/fsl,mpr121-touchkey.yaml
>>> +++ b/Documentation/devicetree/bindings/input/fsl,mpr121-touchkey.yaml
>>> @@ -34,6 +34,10 @@ properties:
>>>       minItems: 1
>>>       maxItems: 12
>>> +  linux,poll-interval:
>>> +    description: Poll interval time in milliseconds.
>>> +    maxItems: 1
>>
>> We already have 'poll-interval' in several bindings. Use that.
> 
> OK, will do.
> linux,poll-intervall was initially suggested by Dmitry but AFAICT he is OK
> with this.
> 
>> This should have a type definition and you don't need maxItems:
>>
>> $ref: /schemas/types.yaml#/definitions/uint32
> 
> OK.
> 
>> Really this should go in a common input schema doc.
> 
> I am not sure how to deal with this. Do you suggest to create
> 
>   Documentation/devicetree/bindings/input/input.yaml
> 
> and put all the common input properties there? Something like:
> 
> diff --git a/Documentation/devicetree/bindings/input/input.yaml b/Documentation/devicetree/bindings/input/input.yaml
> new file mode 100644
> index 000000000000..5dc10ebdfdf0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/input.yaml
> @@ -0,0 +1,24 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/input.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Common input schema binding
> +
> +maintainers:
> +  - Dmitry Torokhov <dmitry.torokhov@gmail.com>
> +
> +properties:
> +  linux,keycodes:
> +    description:
> +      Specifies an array of numeric keycode values to be used for reporting
> +      button presses. The array can contain up to 12 entries.
> +    minItems: 1
> +    maxItems: 12
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +
> +  poll-interval:
> +    description: Poll interval time in milliseconds.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> ---
> 
> Or something else?
> And what is the impact on the fsl,mpr121-touchkey.yaml content/format?
> I could not find any example in any subsystem of such approach.

Hi Rob, maybe this went through the cracks unnoticed?

> Thank you,
> Michal
> 
>>> +
>>>     wakeup-source: Use any event on keypad as wakeup event.
>>>       type: boolean
>>> @@ -44,12 +48,12 @@ properties:
>>>   required:
>>>     - compatible
>>>     - reg
>>> -  - interrupts
>>>     - vdd-supply
>>>     - linux,keycodes
>>>   examples:
>>>     - |
>>> +    // Example with interrupts
>>>       #include "dt-bindings/input/input.h"
>>>       touchkey: mpr121@5a {
>>>           compatible = "fsl,mpr121-touchkey";
>>> @@ -62,3 +66,17 @@ examples:
>>>                            <KEY_4> <KEY_5>, <KEY_6>, <KEY_7>,
>>>                            <KEY_8>, <KEY_9>, <KEY_A>, <KEY_B>;
>>>       };
>>> +
>>> +  - |
>>> +    // Example with polling
>>> +    #include "dt-bindings/input/input.h"
>>> +    touchkey: mpr121@5a {
>>> +        compatible = "fsl,mpr121-touchkey";
>>> +        reg = <0x5a>;
>>> +        linux,poll-interval = <20>;
>>> +        autorepeat;
>>> +        vdd-supply = <&ldo4_reg>;
>>> +        linux,keycodes = <KEY_0>, <KEY_1>, <KEY_2>, <KEY_3>,
>>> +                         <KEY_4> <KEY_5>, <KEY_6>, <KEY_7>,
>>> +                         <KEY_8>, <KEY_9>, <KEY_A>, <KEY_B>;
>>> +    );
>>> -- 
>>> 2.1.4
>>>
>
Rob Herring (Arm) Sept. 27, 2019, 12:46 p.m. UTC | #4
On Tue, Sep 17, 2019 at 9:17 AM Michal Vokáč <michal.vokac@ysoft.com> wrote:
>
> On 03. 09. 19 9:01, Rob Herring wrote:
> > On Mon, Sep 02, 2019 at 01:40:15PM +0200, Michal Vokáč wrote:
> >> Add an option to periodicaly poll the device to get the buttons states
> >> as the interrupt line may not be used on some platforms.
> >>
> >> Signed-off-by: Michal Vokáč <michal.vokac@ysoft.com>
> >> ---
> >> I am not sure how to propperly handle this.
> >> Either interrupt or linux,poll-interval is required, but not both.
> >
> > Add this at the top level:
> >
> > oneOf:
> >    - required: [ interrupts ]
> >    - required: [ linux,poll-interval ]
> >
>
> Nice, works as expected.
>
> >>
> >>   .../bindings/input/fsl,mpr121-touchkey.yaml          | 20 +++++++++++++++++++-
> >>   1 file changed, 19 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/input/fsl,mpr121-touchkey.yaml b/Documentation/devicetree/bindings/input/fsl,mpr121-touchkey.yaml
> >> index c463c1c81755..2b3073a3c9f4 100644
> >> --- a/Documentation/devicetree/bindings/input/fsl,mpr121-touchkey.yaml
> >> +++ b/Documentation/devicetree/bindings/input/fsl,mpr121-touchkey.yaml
> >> @@ -34,6 +34,10 @@ properties:
> >>       minItems: 1
> >>       maxItems: 12
> >>
> >> +  linux,poll-interval:
> >> +    description: Poll interval time in milliseconds.
> >> +    maxItems: 1
> >
> > We already have 'poll-interval' in several bindings. Use that.
>
> OK, will do.
> linux,poll-intervall was initially suggested by Dmitry but AFAICT he is OK
> with this.
>
> > This should have a type definition and you don't need maxItems:
> >
> > $ref: /schemas/types.yaml#/definitions/uint32
>
> OK.
>
> > Really this should go in a common input schema doc.
>
> I am not sure how to deal with this. Do you suggest to create
>
>   Documentation/devicetree/bindings/input/input.yaml
>
> and put all the common input properties there? Something like:

Yes.

>
> diff --git a/Documentation/devicetree/bindings/input/input.yaml b/Documentation/devicetree/bindings/input/input.yaml
> new file mode 100644
> index 000000000000..5dc10ebdfdf0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/input.yaml
> @@ -0,0 +1,24 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/input.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Common input schema binding
> +
> +maintainers:
> +  - Dmitry Torokhov <dmitry.torokhov@gmail.com>
> +
> +properties:
> +  linux,keycodes:
> +    description:
> +      Specifies an array of numeric keycode values to be used for reporting
> +      button presses. The array can contain up to 12 entries.
> +    minItems: 1
> +    maxItems: 12
> +    $ref: /schemas/types.yaml#/definitions/uint32-array

When you have additional constraints on a referenced schema, the
reference has to be under an 'allOf':

allOf:
  - $ref: /schemas/types.yaml#/definitions/uint32-array

I still really don't get why and I've researched it...

However, for the common schema, you probably want to drop minItems/maxItems.

But you could define the value ranges?

items:
  minimum: 0
  maximum: 0xff


> +
> +  poll-interval:
> +    description: Poll interval time in milliseconds.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> ---
>
> Or something else?
> And what is the impact on the fsl,mpr121-touchkey.yaml content/format?
> I could not find any example in any subsystem of such approach.

You only need to define any additional constraints on the common
properties (such as minItems/maxItems) and don't need to repeat things
like the type or description.

In order to get the common schema applied, you do at the top-level of
your binding:

allOf:
  - $ref: input.yaml#

Sometimes we don't have to do this if we have a reliable way of
matching in the common binding such as the node name.

Rob
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/input/fsl,mpr121-touchkey.yaml b/Documentation/devicetree/bindings/input/fsl,mpr121-touchkey.yaml
index c463c1c81755..2b3073a3c9f4 100644
--- a/Documentation/devicetree/bindings/input/fsl,mpr121-touchkey.yaml
+++ b/Documentation/devicetree/bindings/input/fsl,mpr121-touchkey.yaml
@@ -34,6 +34,10 @@  properties:
     minItems: 1
     maxItems: 12
 
+  linux,poll-interval:
+    description: Poll interval time in milliseconds.
+    maxItems: 1
+
   wakeup-source: Use any event on keypad as wakeup event.
     type: boolean
 
@@ -44,12 +48,12 @@  properties:
 required:
   - compatible
   - reg
-  - interrupts
   - vdd-supply
   - linux,keycodes
 
 examples:
   - |
+    // Example with interrupts
     #include "dt-bindings/input/input.h"
     touchkey: mpr121@5a {
         compatible = "fsl,mpr121-touchkey";
@@ -62,3 +66,17 @@  examples:
                          <KEY_4> <KEY_5>, <KEY_6>, <KEY_7>,
                          <KEY_8>, <KEY_9>, <KEY_A>, <KEY_B>;
     };
+
+  - |
+    // Example with polling
+    #include "dt-bindings/input/input.h"
+    touchkey: mpr121@5a {
+        compatible = "fsl,mpr121-touchkey";
+        reg = <0x5a>;
+        linux,poll-interval = <20>;
+        autorepeat;
+        vdd-supply = <&ldo4_reg>;
+        linux,keycodes = <KEY_0>, <KEY_1>, <KEY_2>, <KEY_3>,
+                         <KEY_4> <KEY_5>, <KEY_6>, <KEY_7>,
+                         <KEY_8>, <KEY_9>, <KEY_A>, <KEY_B>;
+    );