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
| ||
|
Message-ID: <20160524082040.GR23704@pengutronix.de> Date: Tue, 24 May 2016 10:20:40 +0200 From: Uwe Kleine-König <u.kleine-koenig@...gutronix.de> To: Vignesh R <vigneshr@...com> Cc: Dmitry Torokhov <dmitry.torokhov@...il.com>, Rob Herring <robh+dt@...nel.org>, Tony Lindgren <tony@...mide.com>, Jonathan Corbet <corbet@....net>, Johan Hovold <johan@...nel.org>, Sylvain Rochet <sylvain.rochet@...secur.com>, Masanari Iida <standby24x7@...il.com>, Ezequiel Garcia <ezequiel@...guardiasur.com.ar>, S Twiss <stwiss.opensource@...semi.com>, Krzysztof Kozlowski <k.kozlowski@...sung.com>, Moritz Fischer <moritz.fischer@...us.com>, Arnd Bergmann <arnd@...db.de>, Geert Uytterhoeven <geert@...ux-m68k.org>, Timo Teras <timo.teras@....fi>, Clifton Barnes <clifton.a.barnes@...il.com>, "linux-input@...r.kernel.org" <linux-input@...r.kernel.org>, "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>, "linux-omap@...r.kernel.org" <linux-omap@...r.kernel.org>, "linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org> Subject: Re: [RFC PATCH 1/2] Input: rotary-encoder- Add support for absolute encoder Hello, On Tue, May 24, 2016 at 10:39:26AM +0530, Vignesh R wrote: > On 05/23/2016 06:48 PM, Uwe Kleine-König wrote: > > On Mon, May 23, 2016 at 04:48:40PM +0530, R, Vignesh wrote: > >> On 5/22/2016 3:56 PM, Uwe Kleine-König wrote: > >>> On Thu, May 19, 2016 at 02:34:00PM +0530, Vignesh R wrote: > >>>> +- rotary-encoder,absolute-encoder: support encoders where GPIO lines > >>>> + reflect the actual position of the rotary encoder dial. For example, > >>>> + if dial points to 9, then four GPIO lines read HLLH(1001b = 9). > >>>> + In this case, rotary-encoder,steps-per-period needed not be defined. > >>> > >>> IMHO this is wrong, I'd formalize this device as: > >>> > >>> { > >>> compatible = "rotary-encoder"; > >>> gpios = <&gpio 19 1>, <&gpio 20 0>, <...>, <...>; > >>> rotary-encoder,encoding = "binary"; > >>> rotary-encoder,steps = <16>; > >>> rotary-encoder,steps-per-period = <16>; > >> > >> The above bindings essential means quarter_period device. I would not > >> like to bother with all the logic in rotary_encoder_quarter_period_irq() > >> when we can know encoder->pos by directly reading state of gpio lines. > > > > OK, we have code that is more complex than it needs to be for your > > device. But your device is a special case of the supported devices, so > > I'd say don't bother that there is more logic in the driver than you > > need and be lucky. > > More complexity is just a overhead. Since, encoder can be turned at a > rate faster than handling of IRQs (rotary_encoder_quarter_period_irq() > is threaded IRQ hence, priority is not close to real time), some states This problem isn't unique to your hardware. An "ordinary" encoder with just two GPIOs and more than one period can be rotated faster than 1/irq_latency, too. There are two things that can be done: - undo the conversion to threaded irqs; or - read out the gpios in the fast handler and only delay decoding and reporting of the event Both approaches have their disadvantages. > can be missed. rotary_encoder_quarter_period_irq() is not robust in this > case, reading gpios directly is more suitable option. I see similar > views expressed in previously[1] > > [1]http://lists.infradead.org/pipermail/linux-arm-kernel/2015-December/391196.html IMHO the right thing to do is to improve rotary_encoder_quarter_period_irq (and also the other handlers for full and half period mode) to make use of additional GPIOs. This way all types of devices benefit and more code is shared. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König | Industrial Linux Solutions | http://www.pengutronix.de/ |
Powered by blists - more mailing lists