diff mbox

input: rotary encoder: implement quarter period mode

Message ID 1387377798-22344-1-git-send-email-s.hauer@pengutronix.de (mailing list archive)
State Superseded
Headers show

Commit Message

Sascha Hauer Dec. 18, 2013, 2:43 p.m. UTC
Some rotary encoders have a stable state in all output state
combinations. Add support for this type of encoder.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Cc: Daniel Mack <daniel@zonque.org>
Cc: linux-input@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
 .../devicetree/bindings/input/rotary-encoder.txt   |  1 +
 Documentation/input/rotary-encoder.txt             |  9 +++++--
 drivers/input/misc/rotary_encoder.c                | 30 ++++++++++++++++++++--
 include/linux/rotary_encoder.h                     |  1 +
 4 files changed, 37 insertions(+), 4 deletions(-)

Comments

Mark Rutland May 7, 2014, 2:45 p.m. UTC | #1
Hi,

On Wed, Dec 18, 2013 at 02:43:18PM +0000, Sascha Hauer wrote:
> Some rotary encoders have a stable state in all output state
> combinations. Add support for this type of encoder.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> Cc: Daniel Mack <daniel@zonque.org>
> Cc: linux-input@vger.kernel.org
> Cc: devicetree@vger.kernel.org
> ---
>  .../devicetree/bindings/input/rotary-encoder.txt   |  1 +
>  Documentation/input/rotary-encoder.txt             |  9 +++++--
>  drivers/input/misc/rotary_encoder.c                | 30 ++++++++++++++++++++--
>  include/linux/rotary_encoder.h                     |  1 +
>  4 files changed, 37 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.txt b/Documentation/devicetree/bindings/input/rotary-encoder.txt
> index 3315495..cbdb29b 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,quarter-period: Makes the driver work on quarter-period mode.

The new property looks as sane to me as the half-period property, so for
the binding addition:

Acked-by: Mark Rutland <mark.rutland@arm.com>

As a general nitpick it would be nicer if the binding didn't refer to
the driver and had a description of {half,quarter} period modes inline,
but that might be a bit tricky and shouldn't block this.

Cheers,
Mark.
--
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
Ezequiel Garcia May 7, 2014, 2:51 p.m. UTC | #2
On 7 May 2014 11:45, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Dec 18, 2013 at 02:43:18PM +0000, Sascha Hauer wrote:
>> Some rotary encoders have a stable state in all output state
>> combinations. Add support for this type of encoder.
>>
>> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
>> Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> Cc: Daniel Mack <daniel@zonque.org>
>> Cc: linux-input@vger.kernel.org
>> Cc: devicetree@vger.kernel.org
>> ---
>>  .../devicetree/bindings/input/rotary-encoder.txt   |  1 +
>>  Documentation/input/rotary-encoder.txt             |  9 +++++--
>>  drivers/input/misc/rotary_encoder.c                | 30 ++++++++++++++++++++--
>>  include/linux/rotary_encoder.h                     |  1 +
>>  4 files changed, 37 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.txt b/Documentation/devicetree/bindings/input/rotary-encoder.txt
>> index 3315495..cbdb29b 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,quarter-period: Makes the driver work on quarter-period mode.
>
> The new property looks as sane to me as the half-period property, so for
> the binding addition:
>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
>

Thanks!

> As a general nitpick it would be nicer if the binding didn't refer to
> the driver and had a description of {half,quarter} period modes inline,
> but that might be a bit tricky and shouldn't block this.
>

I can submit a follow-up patch to clean the binding. It shouldn't be
too hard to re-phrase
the properties so it's clear it's a hardware property.

Dmitry, Do you think you can merge this for v3.16? I've posted a
completely equivalent
patch a while ago [1], which was acked by Daniel, but it was never
merged. Sascha's patch
looks fine as well:

Acked-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>

[1] https://lkml.org/lkml/2013/10/4/167
Dmitry Torokhov May 7, 2014, 6:59 p.m. UTC | #3
On Wed, May 07, 2014 at 03:45:43PM +0100, Mark Rutland wrote:
> Hi,
> 
> On Wed, Dec 18, 2013 at 02:43:18PM +0000, Sascha Hauer wrote:
> > Some rotary encoders have a stable state in all output state
> > combinations. Add support for this type of encoder.
> > 
> > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > Cc: Daniel Mack <daniel@zonque.org>
> > Cc: linux-input@vger.kernel.org
> > Cc: devicetree@vger.kernel.org
> > ---
> >  .../devicetree/bindings/input/rotary-encoder.txt   |  1 +
> >  Documentation/input/rotary-encoder.txt             |  9 +++++--
> >  drivers/input/misc/rotary_encoder.c                | 30 ++++++++++++++++++++--
> >  include/linux/rotary_encoder.h                     |  1 +
> >  4 files changed, 37 insertions(+), 4 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.txt b/Documentation/devicetree/bindings/input/rotary-encoder.txt
> > index 3315495..cbdb29b 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,quarter-period: Makes the driver work on quarter-period mode.
> 
> The new property looks as sane to me as the half-period property, so for
> the binding addition:

Actually, maybe we should deprecate rotary-encoder,half-period and
instead add rotary-encoder,type property?

Thanks.
Ezequiel Garcia May 8, 2014, 5:01 p.m. UTC | #4
On 07 May 11:59 AM, Dmitry Torokhov wrote:
> On Wed, May 07, 2014 at 03:45:43PM +0100, Mark Rutland wrote:
> > Hi,
> > 
> > On Wed, Dec 18, 2013 at 02:43:18PM +0000, Sascha Hauer wrote:
> > > Some rotary encoders have a stable state in all output state
> > > combinations. Add support for this type of encoder.
> > > 
> > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > Cc: Daniel Mack <daniel@zonque.org>
> > > Cc: linux-input@vger.kernel.org
> > > Cc: devicetree@vger.kernel.org
> > > ---
> > >  .../devicetree/bindings/input/rotary-encoder.txt   |  1 +
> > >  Documentation/input/rotary-encoder.txt             |  9 +++++--
> > >  drivers/input/misc/rotary_encoder.c                | 30 ++++++++++++++++++++--
> > >  include/linux/rotary_encoder.h                     |  1 +
> > >  4 files changed, 37 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.txt b/Documentation/devicetree/bindings/input/rotary-encoder.txt
> > > index 3315495..cbdb29b 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,quarter-period: Makes the driver work on quarter-period mode.
> > 
> > The new property looks as sane to me as the half-period property, so for
> > the binding addition:
> 
> Actually, maybe we should deprecate rotary-encoder,half-period and
> instead add rotary-encoder,type property?
> 

Mark: what do you say?

I can fix a few patches if everyone agrees...
Ezequiel Garcia May 19, 2014, 3:26 a.m. UTC | #5
On 08 May 02:01 PM, Ezequiel García wrote:
> On 07 May 11:59 AM, Dmitry Torokhov wrote:
> > On Wed, May 07, 2014 at 03:45:43PM +0100, Mark Rutland wrote:
> > > On Wed, Dec 18, 2013 at 02:43:18PM +0000, Sascha Hauer wrote:
> > > > Some rotary encoders have a stable state in all output state
> > > > combinations. Add support for this type of encoder.
> > > > 
> > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > > Cc: Daniel Mack <daniel@zonque.org>
> > > > Cc: linux-input@vger.kernel.org
> > > > Cc: devicetree@vger.kernel.org
> > > > ---
> > > >  .../devicetree/bindings/input/rotary-encoder.txt   |  1 +
> > > >  Documentation/input/rotary-encoder.txt             |  9 +++++--
> > > >  drivers/input/misc/rotary_encoder.c                | 30 ++++++++++++++++++++--
> > > >  include/linux/rotary_encoder.h                     |  1 +
> > > >  4 files changed, 37 insertions(+), 4 deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.txt b/Documentation/devicetree/bindings/input/rotary-encoder.txt
> > > > index 3315495..cbdb29b 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,quarter-period: Makes the driver work on quarter-period mode.
> > > 
> > > The new property looks as sane to me as the half-period property, so for
> > > the binding addition:
> > 
> > Actually, maybe we should deprecate rotary-encoder,half-period and
> > instead add rotary-encoder,type property?
> > 
> 
> Mark: what do you say?
> 
> I can fix a few patches if everyone agrees...

After some thought, it seemed to me we can define a more specific property
to describe this, instead of a generic "type".

The difference among these three "modes" is the number of turns needed to make
a step.

In the current driver, the default is 4 turns per step, where for the half-period
mode it's 2 turns per step. Now, we need to support 1 turn per step and hence
Sascha proposes a new quarter-period mode.

Given we only need to describe the number of turns per step, I'd say it's
more accurate and less confusing to deprecate the bool half-period property
and instead introduce a "rotary-encoder,turns-per-step" integer property.

How does this sound?
Dmitry Torokhov May 19, 2014, 3:45 a.m. UTC | #6
On Mon, May 19, 2014 at 12:26:59AM -0300, Ezequiel García wrote:
> On 08 May 02:01 PM, Ezequiel García wrote:
> > On 07 May 11:59 AM, Dmitry Torokhov wrote:
> > > On Wed, May 07, 2014 at 03:45:43PM +0100, Mark Rutland wrote:
> > > > On Wed, Dec 18, 2013 at 02:43:18PM +0000, Sascha Hauer wrote:
> > > > > Some rotary encoders have a stable state in all output state
> > > > > combinations. Add support for this type of encoder.
> > > > > 
> > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> > > > > Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > > > > Cc: Daniel Mack <daniel@zonque.org>
> > > > > Cc: linux-input@vger.kernel.org
> > > > > Cc: devicetree@vger.kernel.org
> > > > > ---
> > > > >  .../devicetree/bindings/input/rotary-encoder.txt   |  1 +
> > > > >  Documentation/input/rotary-encoder.txt             |  9 +++++--
> > > > >  drivers/input/misc/rotary_encoder.c                | 30 ++++++++++++++++++++--
> > > > >  include/linux/rotary_encoder.h                     |  1 +
> > > > >  4 files changed, 37 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.txt b/Documentation/devicetree/bindings/input/rotary-encoder.txt
> > > > > index 3315495..cbdb29b 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,quarter-period: Makes the driver work on quarter-period mode.
> > > > 
> > > > The new property looks as sane to me as the half-period property, so for
> > > > the binding addition:
> > > 
> > > Actually, maybe we should deprecate rotary-encoder,half-period and
> > > instead add rotary-encoder,type property?
> > > 
> > 
> > Mark: what do you say?
> > 
> > I can fix a few patches if everyone agrees...
> 
> After some thought, it seemed to me we can define a more specific property
> to describe this, instead of a generic "type".
> 
> The difference among these three "modes" is the number of turns needed to make
> a step.
> 
> In the current driver, the default is 4 turns per step, where for the half-period
> mode it's 2 turns per step. Now, we need to support 1 turn per step and hence
> Sascha proposes a new quarter-period mode.
> 
> Given we only need to describe the number of turns per step, I'd say it's
> more accurate and less confusing to deprecate the bool half-period property
> and instead introduce a "rotary-encoder,turns-per-step" integer property.
> 
> How does this sound?

We'd have to "filter out" invalid step settings, but that sounds fine by
me.
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.txt b/Documentation/devicetree/bindings/input/rotary-encoder.txt
index 3315495..cbdb29b 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,quarter-period: Makes the driver work on quarter-period mode.
 
 See Documentation/input/rotary-encoder.txt for more information.
 
diff --git a/Documentation/input/rotary-encoder.txt b/Documentation/input/rotary-encoder.txt
index 92e68bc..0bbff7e 100644
--- a/Documentation/input/rotary-encoder.txt
+++ b/Documentation/input/rotary-encoder.txt
@@ -9,8 +9,10 @@  peripherals with two wires. The outputs are phase-shifted by 90 degrees
 and by triggering on falling and rising edges, the turn direction can
 be determined.
 
-Some encoders have both outputs low in stable states, whereas others also have
-a stable state with both outputs high (half-period mode).
+
+Some encoders have both outputs low in stable states, others also have
+a stable state with both outputs high (half-period mode) and some have
+a stable state in all steps (quarter-period mode).
 
 The phase diagram of these two outputs look like this:
 
@@ -32,6 +34,9 @@  The phase diagram of these two outputs look like this:
                 |<-->|
 	          one step (half-period mode)
 
+                |<>|
+	          one step (quarter-period mode)
+
 For more information, please see
 	http://en.wikipedia.org/wiki/Rotary_encoder
 
diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
index 5b1aff8..264501c 100644
--- a/drivers/input/misc/rotary_encoder.c
+++ b/drivers/input/misc/rotary_encoder.c
@@ -42,7 +42,7 @@  struct rotary_encoder {
 	bool armed;
 	unsigned char dir;	/* 0 - clockwise, 1 - CCW */
 
-	char last_stable;
+	int last_stable;
 };
 
 static int rotary_encoder_get_state(const struct rotary_encoder_platform_data *pdata)
@@ -117,6 +117,28 @@  static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
 	return IRQ_HANDLED;
 }
 
+static irqreturn_t rotary_encoder_quarter_period_irq(int irq, void *dev_id)
+{
+	struct rotary_encoder *encoder = dev_id;
+	int state;
+	static const u8 states[4][4] = {
+		{ -1,  1,  0, -1 },
+		{  0, -1, -1,  1 },
+		{  1, -1, -1,  0 },
+		{ -1,  0,  1, -1 },
+	};
+
+	state = rotary_encoder_get_state(encoder->pdata);
+
+	encoder->dir = states[encoder->last_stable][state];
+	encoder->last_stable = state;
+
+	if (encoder->dir >= 0)
+		rotary_encoder_report_event(encoder);
+
+	return IRQ_HANDLED;
+}
+
 static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id)
 {
 	struct rotary_encoder *encoder = dev_id;
@@ -180,7 +202,8 @@  static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct devic
 					"rotary-encoder,rollover", NULL);
 	pdata->half_period = !!of_get_property(np,
 					"rotary-encoder,half-period", NULL);
-
+	pdata->quarter_period = !!of_get_property(np,
+					"rotary-encoder,quarter-period", NULL);
 	return pdata;
 }
 #else
@@ -254,6 +277,9 @@  static int rotary_encoder_probe(struct platform_device *pdev)
 	if (pdata->half_period) {
 		handler = &rotary_encoder_half_period_irq;
 		encoder->last_stable = rotary_encoder_get_state(pdata);
+	} else if (pdata->quarter_period) {
+		handler = &rotary_encoder_quarter_period_irq;
+		encoder->last_stable = rotary_encoder_get_state(pdata);
 	} else {
 		handler = &rotary_encoder_irq;
 	}
diff --git a/include/linux/rotary_encoder.h b/include/linux/rotary_encoder.h
index 3f594dc..fd0a70f 100644
--- a/include/linux/rotary_encoder.h
+++ b/include/linux/rotary_encoder.h
@@ -11,6 +11,7 @@  struct rotary_encoder_platform_data {
 	bool relative_axis;
 	bool rollover;
 	bool half_period;
+	bool quarter_period;
 };
 
 #endif /* __ROTARY_ENCODER_H__ */