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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 4 Oct 2013 11:09:26 -0300
From:	Ezequiel Garcia <ezequiel.garcia@...e-electrons.com>
To:	Mark Rutland <mark.rutland@....com>
Cc:	Ezequiel Garcia <ezequiel@...guardiasur.com.ar>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-input@...r.kernel.org" <linux-input@...r.kernel.org>,
	Daniel Mack <zonque@...il.com>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	"rob.herring@...xeda.com" <rob.herring@...xeda.com>,
	devicetree@...r.kernel.org
Subject: Re: [PATCH v2 2/2] input: rotary-encoder: Add 'on-each-step' to
 binding documentation

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@...il.com>
> > Cc: Dmitry Torokhov <dmitry.torokhov@...il.com>
> > Cc: Rob Herring <rob.herring@...xeda.com>
> > Cc: devicetree@...r.kernel.org
> > Signed-off-by: Ezequiel Garcia <ezequiel@...guardiasur.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-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/