[PATCH v2 2/2] input: rotary-encoder: Add 'on-each-step' to binding documentation

[PATCH v2 2/2] input: rotary-encoder: Add 'on-each-step' to binding documentation

[Ezequiel Garcia]

The driver now supports a new mode to handle the interruptions generated
by the device: on this new mode an input event is generated on each step
(i.e. on each IRQ). Therefore, add a new DT property, to select the
mode: 'rotary-encoder,on-each-step'.

Cc: Daniel Mack <zonque-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Dmitry Torokhov <dmitry.torokhov-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Rob Herring <rob.herring-bsGFqQB8/DxBDgjK7y7TUQ@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Ezequiel Garcia <ezequiel-30ULvvUtt6G51wMPkGsGjgyUoB5FGQPZ@public.gmane.org>
---
I'm not at all happy with this DT binding as it's way to customized
for the current driver. For instance, if we want to support mapping
key events (or better arbitrary linux-input event types) it seems
there's no easy way to fix the binding.

Maybe a better way of handling the different 'modes' is through
compatible strings?

I'm not really sure, so I hope the DT guys have some comment on this.

 Documentation/devicetree/bindings/input/rotary-encoder.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.txt b/Documentation/devicetree/bindings/input/rotary-encoder.txt
index 3315495..b89e38d 100644
--- a/Documentation/devicetree/bindings/input/rotary-encoder.txt
+++ b/Documentation/devicetree/bindings/input/rotary-encoder.txt
@@ -15,6 +15,7 @@ Optional properties:
 - rotary-encoder,rollover: Automatic rollove when the rotary value becomes
   greater than the specified steps or smaller than 0. For absolute axis only.
 - rotary-encoder,half-period: Makes the driver work on half-period mode.
+- rotary-encoder,on-each-step: Makes the driver send an event on each step.
 
 See Documentation/input/rotary-encoder.txt for more information.
 
-- 
1.8.1.5

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] input: rotary-encoder: Add 'on-each-step' to binding documentation

[Mark Rutland]

On Fri, Oct 04, 2013 at 01:53:23PM +0100, Ezequiel Garcia wrote:
> The driver now supports a new mode to handle the interruptions generated
> by the device: on this new mode an input event is generated on each step
> (i.e. on each IRQ). Therefore, add a new DT property, to select the
> mode: 'rotary-encoder,on-each-step'.
> 
> Cc: Daniel Mack <zonque@gmail.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> ---
> I'm not at all happy with this DT binding as it's way to customized
> for the current driver. For instance, if we want to support mapping
> key events (or better arbitrary linux-input event types) it seems
> there's no easy way to fix the binding.
> 
> Maybe a better way of handling the different 'modes' is through
> compatible strings?

I'd prefer not to have more pseudo-devices in DT, and would prefer not
to have compatible strings that boil down to driver options. We end up
just embedding a tonne of Linux-specific driver configuration in the DT
rather than describing hardware.

That said, I'm not sure what the best solution is here.

> 
> I'm not really sure, so I hope the DT guys have some comment on this.
> 
>  Documentation/devicetree/bindings/input/rotary-encoder.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.txt b/Documentation/devicetree/bindings/input/rotary-encoder.txt
> index 3315495..b89e38d 100644
> --- a/Documentation/devicetree/bindings/input/rotary-encoder.txt
> +++ b/Documentation/devicetree/bindings/input/rotary-encoder.txt
> @@ -15,6 +15,7 @@ Optional properties:
>  - rotary-encoder,rollover: Automatic rollove when the rotary value becomes
>    greater than the specified steps or smaller than 0. For absolute axis only.
>  - rotary-encoder,half-period: Makes the driver work on half-period mode.
> +- rotary-encoder,on-each-step: Makes the driver send an event on each step.

Could this not be something requested at runtime?

Could you explain what you want to achieve with this? -- what events do
you want to occur when, to be handled in what way?

Cheers,
Mark.

Re: [PATCH v2 2/2] input: rotary-encoder: Add 'on-each-step' to binding documentation

[Ezequiel Garcia]

On Fri, Oct 04, 2013 at 02:19:56PM +0100, Mark Rutland wrote:
> On Fri, Oct 04, 2013 at 01:53:23PM +0100, Ezequiel Garcia wrote:
> > The driver now supports a new mode to handle the interruptions generated
> > by the device: on this new mode an input event is generated on each step
> > (i.e. on each IRQ). Therefore, add a new DT property, to select the
> > mode: 'rotary-encoder,on-each-step'.
> > 
> > Cc: Daniel Mack <zonque@gmail.com>
> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Cc: Rob Herring <rob.herring@calxeda.com>
> > Cc: devicetree@vger.kernel.org
> > Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> > ---
> > I'm not at all happy with this DT binding as it's way to customized
> > for the current driver. For instance, if we want to support mapping
> > key events (or better arbitrary linux-input event types) it seems
> > there's no easy way to fix the binding.
> > 
> > Maybe a better way of handling the different 'modes' is through
> > compatible strings?
> 
> I'd prefer not to have more pseudo-devices in DT, and would prefer not
> to have compatible strings that boil down to driver options. We end up
> just embedding a tonne of Linux-specific driver configuration in the DT
> rather than describing hardware.
> 
> That said, I'm not sure what the best solution is here.
> 
> > 
> > I'm not really sure, so I hope the DT guys have some comment on this.
> > 
> >  Documentation/devicetree/bindings/input/rotary-encoder.txt | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.txt b/Documentation/devicetree/bindings/input/rotary-encoder.txt
> > index 3315495..b89e38d 100644
> > --- a/Documentation/devicetree/bindings/input/rotary-encoder.txt
> > +++ b/Documentation/devicetree/bindings/input/rotary-encoder.txt
> > @@ -15,6 +15,7 @@ Optional properties:
> >  - rotary-encoder,rollover: Automatic rollove when the rotary value becomes
> >    greater than the specified steps or smaller than 0. For absolute axis only.
> >  - rotary-encoder,half-period: Makes the driver work on half-period mode.
> > +- rotary-encoder,on-each-step: Makes the driver send an event on each step.
> 
> Could this not be something requested at runtime?
> 

Sure. The different modes:

* default (no option)
* rotary-encoder,half-period
* rotary-encoder,on-each-step

Just map to different interruption handlers. I don't have any other
rotary-encoder device, so I'm not at all sure what's the use of the
other two cases.
My particular device is detented, and produces a 'stable' event on each
step (i.e on each IRQ).

Regarding the runtime specification: you mean as a module parameter?
That should be trivial to add, no?

> Could you explain what you want to achieve with this? -- what events do
> you want to occur when, to be handled in what way?
> 

Hm.. maybe I should have added the binding to the 1/2 patch and CCed
everybody involved for better context.

Anyway, I hope the above is clearer, I'm not really sure how to specify
the details in the DT binding, since it's a specific interruption handler
for this class of encoder devices (stable on each step).

That said, I really hope I'm crafting a generic solution and not some
tailor-made implementation that just happens to match my use case.

The input maintainer's opinion on this would be valuable.
-- 
Ezequiel García, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2 2/2] input: rotary-encoder: Add 'on-each-step' to binding documentation

[Ezequiel García]

On 4 October 2013 11:09, Ezequiel Garcia
<ezequiel.garcia@free-electrons.com> wrote:
> On Fri, Oct 04, 2013 at 02:19:56PM +0100, Mark Rutland wrote:
>> On Fri, Oct 04, 2013 at 01:53:23PM +0100, Ezequiel Garcia wrote:
>> > The driver now supports a new mode to handle the interruptions generated
>> > by the device: on this new mode an input event is generated on each step
>> > (i.e. on each IRQ). Therefore, add a new DT property, to select the
>> > mode: 'rotary-encoder,on-each-step'.
>> >
>> > Cc: Daniel Mack <zonque@gmail.com>
>> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> > Cc: Rob Herring <rob.herring@calxeda.com>
>> > Cc: devicetree@vger.kernel.org
>> > Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
>> > ---
>> > I'm not at all happy with this DT binding as it's way to customized
>> > for the current driver. For instance, if we want to support mapping
>> > key events (or better arbitrary linux-input event types) it seems
>> > there's no easy way to fix the binding.
>> >
>> > Maybe a better way of handling the different 'modes' is through
>> > compatible strings?
>>
>> I'd prefer not to have more pseudo-devices in DT, and would prefer not
>> to have compatible strings that boil down to driver options. We end up
>> just embedding a tonne of Linux-specific driver configuration in the DT
>> rather than describing hardware.
>>
>> That said, I'm not sure what the best solution is here.
>>
>> >
>> > I'm not really sure, so I hope the DT guys have some comment on this.
>> >
>> >  Documentation/devicetree/bindings/input/rotary-encoder.txt | 1 +
>> >  1 file changed, 1 insertion(+)
>> >
>> > diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.txt b/Documentation/devicetree/bindings/input/rotary-encoder.txt
>> > index 3315495..b89e38d 100644
>> > --- a/Documentation/devicetree/bindings/input/rotary-encoder.txt
>> > +++ b/Documentation/devicetree/bindings/input/rotary-encoder.txt
>> > @@ -15,6 +15,7 @@ Optional properties:
>> >  - rotary-encoder,rollover: Automatic rollove when the rotary value becomes
>> >    greater than the specified steps or smaller than 0. For absolute axis only.
>> >  - rotary-encoder,half-period: Makes the driver work on half-period mode.
>> > +- rotary-encoder,on-each-step: Makes the driver send an event on each step.
>>
>> Could this not be something requested at runtime?
>>
>
> Sure. The different modes:
>
> * default (no option)
> * rotary-encoder,half-period
> * rotary-encoder,on-each-step
>
> Just map to different interruption handlers. I don't have any other
> rotary-encoder device, so I'm not at all sure what's the use of the
> other two cases.
> My particular device is detented, and produces a 'stable' event on each
> step (i.e on each IRQ).
>
> Regarding the runtime specification: you mean as a module parameter?
> That should be trivial to add, no?
>
>> Could you explain what you want to achieve with this? -- what events do
>> you want to occur when, to be handled in what way?
>>
>
> Hm.. maybe I should have added the binding to the 1/2 patch and CCed
> everybody involved for better context.
>
> Anyway, I hope the above is clearer, I'm not really sure how to specify
> the details in the DT binding, since it's a specific interruption handler
> for this class of encoder devices (stable on each step).
>
> That said, I really hope I'm crafting a generic solution and not some
> tailor-made implementation that just happens to match my use case.
>
> The input maintainer's opinion on this would be valuable.

Any insights on this binding?

If nobody objects, then I'd like to get this change accepted,
together with the driver change.

Thanks!
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html