* Re: [PATCH input-next 2/4] dt-bindings: input: mpr121: Add poll-interval property
[not found] ` <c8c909e5-1e1f-23a7-dc22-f112fcaeab4c@ysoft.com>
@ 2019-09-27 6:47 ` Michal Vokáč
2019-09-27 12:46 ` Rob Herring
1 sibling, 0 replies; 3+ messages in thread
From: Michal Vokáč @ 2019-09-27 6:47 UTC (permalink / raw)
To: Rob Herring
Cc: Dmitry Torokhov, Shawn Guo, Fabio Estevam, linux-input,
devicetree
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
>>>
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH input-next 2/4] dt-bindings: input: mpr121: Add poll-interval property
[not found] ` <c8c909e5-1e1f-23a7-dc22-f112fcaeab4c@ysoft.com>
2019-09-27 6:47 ` [PATCH input-next 2/4] dt-bindings: input: mpr121: Add poll-interval property Michal Vokáč
@ 2019-09-27 12:46 ` Rob Herring
1 sibling, 0 replies; 3+ messages in thread
From: Rob Herring @ 2019-09-27 12:46 UTC (permalink / raw)
To: Michal Vokáč
Cc: Dmitry Torokhov, Shawn Guo, Fabio Estevam, Linux Input,
devicetree
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
^ permalink raw reply [flat|nested] 3+ messages in thread