Re: [PATCH v2 2/2] input: rotary-encoder: Add 'on-each-step' tobinding 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@xxxxxxxxx>
> > Cc: Dmitry Torokhov <dmitry.torokhov@xxxxxxxxx>
> > Cc: Rob Herring <rob.herring@xxxxxxxxxxx>
> > Cc: devicetree@xxxxxxxxxxxxxxx
> > Signed-off-by: Ezequiel Garcia <ezequiel@xxxxxxxxxxxxxxxxxxxx>
> > ---
> > 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-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/