Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1406750

Re: [RFC PATCH 1/2] Input: rotary-encoder- Add support for absolute encoder

Show all headers | View raw

On 05/24/2016 01:50 PM, Uwe Kleine-König wrote:
> 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:
[...]
>>>
>>> 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.

But my hardware should not be affected by this problem. The whole point
of absolute encoder is to overcome the difficulty of software keeping
track of steps, one can read the gpios state anytime and find out what
value is the encoder pointing to.

> 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.
> 

And both cannot guarantee that an event is not missed (on a loaded
system) and all of the state logic goes for a toss. For binary encoding
multiple IRQs(thats why incremental encoders are usually gray coded)
will fire at same time and handling all of the them is a considerable
overhead, there is good chance that some events are missed.

>> 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. 

I doubt there exists any incremental encoder with more than 2
gpios(except for the extra reference GPIO or Z signal). So modifying
full/half period handlers maybe unnecessary.

> This way all types of devices benefit and more code is shared.

Sorry.. but IMHO, there is little code sharing and more complexity. So,
I will leave it to the maintainer to decide whats the best approach here.

-- 
Regards
Vignesh

Back to linux.kernel | Previous | Next — Previous in thread | Find similar | Unroll thread

Thread

[RFC PATCH 0/2] AM335x-ICE: Add support for rotary-encoder Vignesh R <vigneshr@ti.com> - 2016-05-19 11:10 +0200
  [RFC PATCH 2/2] ARM: dts: am335x-icev2: Add rotary-encoder node Vignesh R <vigneshr@ti.com> - 2016-05-19 11:10 +0200
  [RFC PATCH 1/2] Input: rotary-encoder- Add support for absolute encoder Vignesh R <vigneshr@ti.com> - 2016-05-19 11:10 +0200
    Re: [RFC PATCH 1/2] Input: rotary-encoder- Add support for absolute  encoder Krzysztof Kozlowski <k.kozlowski@samsung.com> - 2016-05-19 13:30 +0200
      Re: [RFC PATCH 1/2] Input: rotary-encoder- Add support for absolute  encoder Vignesh R <vigneshr@ti.com> - 2016-05-19 13:50 +0200
    Re: [RFC PATCH 1/2] Input: rotary-encoder- Add support for absolute  encoder Dmitry Torokhov <dmitry.torokhov@gmail.com> - 2016-05-20 18:40 +0200
      Re: [RFC PATCH 1/2] Input: rotary-encoder- Add support for absolute  encoder "R, Vignesh" <vigneshr@ti.com> - 2016-05-23 11:30 +0200
        Re: [RFC PATCH 1/2] Input: rotary-encoder- Add support for absolute  encoder Vignesh R <vigneshr@ti.com> - 2016-05-25 10:50 +0200
    Re: [RFC PATCH 1/2] Input: rotary-encoder- Add support for absolute  encoder Rob Herring <robh@kernel.org> - 2016-05-20 23:50 +0200
    Re: [RFC PATCH 1/2] Input: rotary-encoder- Add support for absolute  encoder Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2016-05-22 12:30 +0200
      Re: [RFC PATCH 1/2] Input: rotary-encoder- Add support for absolute  encoder "R, Vignesh" <vigneshr@ti.com> - 2016-05-23 13:30 +0200
        Re: [RFC PATCH 1/2] Input: rotary-encoder- Add support for absolute  encoder Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2016-05-23 15:20 +0200
          Re: [RFC PATCH 1/2] Input: rotary-encoder- Add support for absolute  encoder Vignesh R <vigneshr@ti.com> - 2016-05-24 07:20 +0200
            Re: [RFC PATCH 1/2] Input: rotary-encoder- Add support for absolute  encoder Uwe Kleine-König   <u.kleine-koenig@pengutronix.de> - 2016-05-24 10:30 +0200
              Re: [RFC PATCH 1/2] Input: rotary-encoder- Add support for absolute  encoder Vignesh R <vigneshr@ti.com> - 2016-05-25 10:50 +0200

csiph-web