[PATCH 0/3] input: Simplifying the rotary encoder devicetree binding

[PATCH 0/3] input: Simplifying the rotary encoder devicetree binding

[Ezequiel Garcia]

Hi Dmitry,

This patchset is another attempt at supporting rotary encoders
with stable state on each output states. Previous efforts to
support this has been done by Sascha Hauer [1] and myself [2].

After some discussion we agreed to remove the current ad-hoc
bindings for each of the rotary-encoders types, and instead
introduce "one binding to rule them all".

This patchset introduces a new 'steps-per-period' property.
Such a property can be used to model the three types
of rotary-encoders.

>From the GPIO output diagram, we can see where these types
come from, and how the steps-per-period describes each
of them.

                  _____       _____       _____
                 |     |     |     |     |     |
  Channel A  ____|     |_____|     |_____|     |____

                 :  :  :  :  :  :  :  :  :  :  :  :
            __       _____       _____       _____
              |     |     |     |     |     |     |
  Channel B   |_____|     |_____|     |_____|     |__

                 :  :  :  :  :  :  :  :  :  :  :  :
  Event          a  b  c  d  a  b  c  d  a  b  c  d

                |<-------->|
                  one step, steps-per-period = 1

                |<-->|
                  one step, steps-per-period = 2

                |<>|
                  one step, steps-per-period = 4

The 'half-period' property is marked as deprecated and a
warning message is issued if it's used, although the driver
retains its old behavior.

I'm also submitting Ben Gamari's cleanup, with the commit log
slightly ammended.

Feedback welcome!

[1] http://www.spinics.net/lists/linux-input/msg28701.html
[2] http://www.spinics.net/lists/linux-input/msg27644.html

Ben Gamari (1):
  input: rotary-encoder: Use of_property_read_bool

Ezequiel Garcia (2):
  input: rotary-encoder: Introduce new 'steps-per-period' property
  input: rotary-encoder: Support 'steps-per-period' DT property

 .../devicetree/bindings/input/rotary-encoder.txt   |  9 +++
 Documentation/input/rotary-encoder.txt             |  8 +-
 drivers/input/misc/rotary_encoder.c                | 94 +++++++++++++++++++---
 include/linux/rotary_encoder.h                     |  2 +-
 4 files changed, 101 insertions(+), 12 deletions(-)

-- 
2.5.2

[PATCH 1/3] input: rotary-encoder: Use of_property_read_bool

[Ezequiel Garcia]

From: Ben Gamari <bgamari.foss-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

This commit makes uses of_property_read_bool() to read
boolean properties. This is just cosmetic cleanup.

Signed-off-by: Ben Gamari <bgamari.foss-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/input/misc/rotary_encoder.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
index f27f81ee84ed..15a4cc670a3f 100644
--- a/drivers/input/misc/rotary_encoder.c
+++ b/drivers/input/misc/rotary_encoder.c
@@ -174,12 +174,9 @@ static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct devic
 	pdata->gpio_b = of_get_gpio_flags(np, 1, &flags);
 	pdata->inverted_b = flags & OF_GPIO_ACTIVE_LOW;
 
-	pdata->relative_axis = !!of_get_property(np,
-					"rotary-encoder,relative-axis", NULL);
-	pdata->rollover = !!of_get_property(np,
-					"rotary-encoder,rollover", NULL);
-	pdata->half_period = !!of_get_property(np,
-					"rotary-encoder,half-period", NULL);
+	pdata->relative_axis = of_property_read_bool(np, "rotary-encoder,relative-axis");
+	pdata->rollover = of_property_read_bool(np, "rotary-encoder,rollover");
+	pdata->half_period = of_property_read_bool(np, "rotary-encoder,half-period");
 
 	return pdata;
 }
-- 
2.5.2

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/3] input: rotary-encoder: Introduce new 'steps-per-period' property

[Ezequiel Garcia]

This commit deprecates the 'half-period' property and introduces
a new property 'steps-per-period'. This property specifies the
number of steps (stable states) produced by the rotary encoder
for each GPIO period.

Such a property is required to accurately describe all kinds of
rotary-encoders devices.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 Documentation/devicetree/bindings/input/rotary-encoder.txt | 9 +++++++++
 Documentation/input/rotary-encoder.txt                     | 8 ++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.txt b/Documentation/devicetree/bindings/input/rotary-encoder.txt
index 331549593ed5..5715ad70d28e 100644
--- a/Documentation/devicetree/bindings/input/rotary-encoder.txt
+++ b/Documentation/devicetree/bindings/input/rotary-encoder.txt
@@ -14,7 +14,16 @@ Optional properties:
   device, hence no steps need to be passed.
 - 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,steps-per-period: Number of steps (stable states) per period.
+  The values have the following meaning:
+  1: Full-period mode (default)
+  2: Half-period mode
+  4: Quarter-period mode
+
+Deprecated properties:
 - rotary-encoder,half-period: Makes the driver work on half-period mode.
+  This property is deprecated. Instead, a 'steps-per-period ' value should
+  be used, such as "rotary-encoder,steps-per-period = <2>".
 
 See Documentation/input/rotary-encoder.txt for more information.
 
diff --git a/Documentation/input/rotary-encoder.txt b/Documentation/input/rotary-encoder.txt
index 5737e3590adb..00acf88e4064 100644
--- a/Documentation/input/rotary-encoder.txt
+++ b/Documentation/input/rotary-encoder.txt
@@ -9,8 +9,9 @@ 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 +33,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
 	https://en.wikipedia.org/wiki/Rotary_encoder
 
-- 
2.5.2

[PATCH 3/3] input: rotary-encoder: Support 'steps-per-period' DT property

[Ezequiel Garcia]

As per the recent devicetree binding changes, this commit adds the
support for the new 'steps-per-period' property.

Legacy properties to specify the rotary behavior, are now deprecated
and instead the new 'steps-per-period' is supported. The default behavior
is retained.

This allows to support rotary-encoder devices with detents wich are capable
of producing a stable event on each step.

Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/input/misc/rotary_encoder.c | 87 +++++++++++++++++++++++++++++++++++--
 include/linux/rotary_encoder.h      |  2 +-
 2 files changed, 84 insertions(+), 5 deletions(-)

diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
index 15a4cc670a3f..e021615dd3c0 100644
--- a/drivers/input/misc/rotary_encoder.c
+++ b/drivers/input/misc/rotary_encoder.c
@@ -142,6 +142,55 @@ static irqreturn_t rotary_encoder_half_period_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;
+	unsigned char sum;
+	int state;
+
+	state = rotary_encoder_get_state(encoder->pdata);
+
+	/*
+	 * We encode the previous and the current state using a byte.
+	 * The previous state in the MSB nibble, the current state in the LSB
+	 * nibble. Then use a table to decide the direction of the turn.
+	 */
+	sum = (encoder->last_stable << 4) + state;
+	switch (sum) {
+	case 0x31:
+	case 0x10:
+	case 0x02:
+	case 0x23:
+		encoder->dir = 0; /* clockwise */
+		break;
+
+	case 0x13:
+	case 0x01:
+	case 0x20:
+	case 0x32:
+		encoder->dir = 1; /* counter-clockwise */
+		break;
+
+	default:
+		/*
+		 * Ignore all other values. This covers the case when the
+		 * state didn't change (a spurious interrupt) and the
+		 * cases where the state changed by two steps, making it
+		 * impossible to tell the direction.
+		 *
+		 * In either case, don't report any event and save the
+		 * state for later.
+		 */
+		goto out;
+	}
+
+	rotary_encoder_report_event(encoder);
+
+out:
+	encoder->last_stable = state;
+	return IRQ_HANDLED;
+}
+
 #ifdef CONFIG_OF
 static const struct of_device_id rotary_encoder_of_match[] = {
 	{ .compatible = "rotary-encoder", },
@@ -176,7 +225,26 @@ static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct devic
 
 	pdata->relative_axis = of_property_read_bool(np, "rotary-encoder,relative-axis");
 	pdata->rollover = of_property_read_bool(np, "rotary-encoder,rollover");
-	pdata->half_period = of_property_read_bool(np, "rotary-encoder,half-period");
+
+	if (!of_get_property(np, "rotary-encoder,steps-per-period", NULL)) {
+		/*
+		 * Fallback to a one step per period behavior if the
+		 * 'steps-per-period' is not set.
+		 */
+		pdata->steps_per_period = 1;
+	} else {
+		of_property_read_u32(np, "rotary-encoder,steps-per-period",
+				     &pdata->steps_per_period);
+	}
+
+	/*
+	 * The 'half-period' property has been deprecated, you must use
+	 * 'steps-per-period' and set an appropriate value.
+	 */
+	if (of_get_property(np, "rotary-encoder,half-period", NULL)) {
+		pr_warn(FW_WARN "\"rotary-encoder,half-period\" is deprecated\n");
+		pdata->steps_per_period = 2;
+	}
 
 	return pdata;
 }
@@ -247,12 +315,23 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 	encoder->irq_a = gpio_to_irq(pdata->gpio_a);
 	encoder->irq_b = gpio_to_irq(pdata->gpio_b);
 
-	/* request the IRQs */
-	if (pdata->half_period) {
+	switch (pdata->steps_per_period) {
+	case 4:
+		handler = &rotary_encoder_quarter_period_irq;
+		encoder->last_stable = rotary_encoder_get_state(pdata);
+		break;
+	case 2:
 		handler = &rotary_encoder_half_period_irq;
 		encoder->last_stable = rotary_encoder_get_state(pdata);
-	} else {
+		break;
+	case 1:
 		handler = &rotary_encoder_irq;
+		break;
+	default:
+		dev_err(dev, "'%d' is not a valid steps-per-period value\n",
+			pdata->steps_per_period);
+		err = -EINVAL;
+		goto exit_free_gpio_b;
 	}
 
 	err = request_irq(encoder->irq_a, handler,
diff --git a/include/linux/rotary_encoder.h b/include/linux/rotary_encoder.h
index 3f594dce5716..0bebba31908c 100644
--- a/include/linux/rotary_encoder.h
+++ b/include/linux/rotary_encoder.h
@@ -10,7 +10,7 @@ struct rotary_encoder_platform_data {
 	unsigned int inverted_b;
 	bool relative_axis;
 	bool rollover;
-	bool half_period;
+	unsigned int steps_per_period;
 };
 
 #endif /* __ROTARY_ENCODER_H__ */
-- 
2.5.2

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

Re: [PATCH 2/3] input: rotary-encoder: Introduce new 'steps-per-period' property

[Rob Herring]

On Fri, Oct 9, 2015 at 8:46 AM, Ezequiel Garcia
<ezequiel@vanguardiasur.com.ar> wrote:
> This commit deprecates the 'half-period' property and introduces
> a new property 'steps-per-period'. This property specifies the
> number of steps (stable states) produced by the rotary encoder
> for each GPIO period.
>
> Such a property is required to accurately describe all kinds of
> rotary-encoders devices.
>
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>

Acked-by: Rob Herring <robh@kernel.org>

Are there any in tree dts's using the deprecated property? If so,
please update them.

Rob

> ---
>  Documentation/devicetree/bindings/input/rotary-encoder.txt | 9 +++++++++
>  Documentation/input/rotary-encoder.txt                     | 8 ++++++--
>  2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.txt b/Documentation/devicetree/bindings/input/rotary-encoder.txt
> index 331549593ed5..5715ad70d28e 100644
> --- a/Documentation/devicetree/bindings/input/rotary-encoder.txt
> +++ b/Documentation/devicetree/bindings/input/rotary-encoder.txt
> @@ -14,7 +14,16 @@ Optional properties:
>    device, hence no steps need to be passed.
>  - 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,steps-per-period: Number of steps (stable states) per period.
> +  The values have the following meaning:
> +  1: Full-period mode (default)
> +  2: Half-period mode
> +  4: Quarter-period mode
> +
> +Deprecated properties:
>  - rotary-encoder,half-period: Makes the driver work on half-period mode.
> +  This property is deprecated. Instead, a 'steps-per-period ' value should
> +  be used, such as "rotary-encoder,steps-per-period = <2>".
>
>  See Documentation/input/rotary-encoder.txt for more information.
>
> diff --git a/Documentation/input/rotary-encoder.txt b/Documentation/input/rotary-encoder.txt
> index 5737e3590adb..00acf88e4064 100644
> --- a/Documentation/input/rotary-encoder.txt
> +++ b/Documentation/input/rotary-encoder.txt
> @@ -9,8 +9,9 @@ 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 +33,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
>         https://en.wikipedia.org/wiki/Rotary_encoder
>
> --
> 2.5.2
>

Re: [PATCH 2/3] input: rotary-encoder: Introduce new 'steps-per-period' property

[Ezequiel Garcia]

On 9 October 2015 at 10:57, Rob Herring <robh@kernel.org> wrote:
> On Fri, Oct 9, 2015 at 8:46 AM, Ezequiel Garcia
> <ezequiel@vanguardiasur.com.ar> wrote:
>> This commit deprecates the 'half-period' property and introduces
>> a new property 'steps-per-period'. This property specifies the
>> number of steps (stable states) produced by the rotary encoder
>> for each GPIO period.
>>
>> Such a property is required to accurately describe all kinds of
>> rotary-encoders devices.
>>
>> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
>
> Acked-by: Rob Herring <robh@kernel.org>
>
> Are there any in tree dts's using the deprecated property? If so,
> please update them.
>

Nope, no dts's using it.

Thanks,
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar
--
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

Re: [PATCH 3/3] input: rotary-encoder: Support 'steps-per-period' DT property

[Dmitry Torokhov]

On Fri, Oct 09, 2015 at 10:46:56AM -0300, Ezequiel Garcia wrote:
> As per the recent devicetree binding changes, this commit adds the
> support for the new 'steps-per-period' property.
> 
> Legacy properties to specify the rotary behavior, are now deprecated
> and instead the new 'steps-per-period' is supported. The default behavior
> is retained.
> 
> This allows to support rotary-encoder devices with detents wich are capable
> of producing a stable event on each step.
> 
> Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> ---
>  drivers/input/misc/rotary_encoder.c | 87 +++++++++++++++++++++++++++++++++++--
>  include/linux/rotary_encoder.h      |  2 +-
>  2 files changed, 84 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
> index 15a4cc670a3f..e021615dd3c0 100644
> --- a/drivers/input/misc/rotary_encoder.c
> +++ b/drivers/input/misc/rotary_encoder.c
> @@ -142,6 +142,55 @@ static irqreturn_t rotary_encoder_half_period_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;
> +	unsigned char sum;
> +	int state;
> +
> +	state = rotary_encoder_get_state(encoder->pdata);
> +
> +	/*
> +	 * We encode the previous and the current state using a byte.
> +	 * The previous state in the MSB nibble, the current state in the LSB
> +	 * nibble. Then use a table to decide the direction of the turn.
> +	 */
> +	sum = (encoder->last_stable << 4) + state;
> +	switch (sum) {
> +	case 0x31:
> +	case 0x10:
> +	case 0x02:
> +	case 0x23:
> +		encoder->dir = 0; /* clockwise */
> +		break;
> +
> +	case 0x13:
> +	case 0x01:
> +	case 0x20:
> +	case 0x32:
> +		encoder->dir = 1; /* counter-clockwise */
> +		break;
> +
> +	default:
> +		/*
> +		 * Ignore all other values. This covers the case when the
> +		 * state didn't change (a spurious interrupt) and the
> +		 * cases where the state changed by two steps, making it
> +		 * impossible to tell the direction.
> +		 *
> +		 * In either case, don't report any event and save the
> +		 * state for later.
> +		 */
> +		goto out;
> +	}
> +
> +	rotary_encoder_report_event(encoder);
> +
> +out:
> +	encoder->last_stable = state;
> +	return IRQ_HANDLED;
> +}
> +
>  #ifdef CONFIG_OF
>  static const struct of_device_id rotary_encoder_of_match[] = {
>  	{ .compatible = "rotary-encoder", },
> @@ -176,7 +225,26 @@ static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct devic
>  
>  	pdata->relative_axis = of_property_read_bool(np, "rotary-encoder,relative-axis");
>  	pdata->rollover = of_property_read_bool(np, "rotary-encoder,rollover");
> -	pdata->half_period = of_property_read_bool(np, "rotary-encoder,half-period");
> +
> +	if (!of_get_property(np, "rotary-encoder,steps-per-period", NULL)) {
> +		/*
> +		 * Fallback to a one step per period behavior if the
> +		 * 'steps-per-period' is not set.
> +		 */
> +		pdata->steps_per_period = 1;
> +	} else {
> +		of_property_read_u32(np, "rotary-encoder,steps-per-period",
> +				     &pdata->steps_per_period);
> +	}
> +
> +	/*
> +	 * The 'half-period' property has been deprecated, you must use
> +	 * 'steps-per-period' and set an appropriate value.
> +	 */
> +	if (of_get_property(np, "rotary-encoder,half-period", NULL)) {
> +		pr_warn(FW_WARN "\"rotary-encoder,half-period\" is deprecated\n");
> +		pdata->steps_per_period = 2;
> +	}

Hmm, I do not think we need to warn about old DTSes if we consider them
ABI. How about if we parse like this:

	error = of_property_read_u32(np, "rotary-encoder,steps-per-period",
				     &pdata->steps_per_period);
	if (error) {
		/*
		 * The 'half-period' property has been deprecated, you must use
		 * 'steps-per-period' and set an appropriate value, but we still
		 * need to parse it to maintain compatibility.
		 */
		if (of_property_read_bool(np, "rotary-encoder,half-period")) {
			pdata->steps_per_period = 2;
		} else {
			/* Fallback to a one step per period behavior */
			pdata->steps_per_period = 1;
		}
	}


(no need to resubmit if you are OK with this).

Thanks.

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

Re: [PATCH 3/3] input: rotary-encoder: Support 'steps-per-period' DT property

[Ezequiel Garcia]

On 14 October 2015 at 03:55, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> On Fri, Oct 09, 2015 at 10:46:56AM -0300, Ezequiel Garcia wrote:
>> As per the recent devicetree binding changes, this commit adds the
>> support for the new 'steps-per-period' property.
>>
>> Legacy properties to specify the rotary behavior, are now deprecated
>> and instead the new 'steps-per-period' is supported. The default behavior
>> is retained.
>>
>> This allows to support rotary-encoder devices with detents wich are capable
>> of producing a stable event on each step.
>>
>> Signed-off-by: Guido Martínez <guido@vanguardiasur.com.ar>
>> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
>> ---
>>  drivers/input/misc/rotary_encoder.c | 87 +++++++++++++++++++++++++++++++++++--
>>  include/linux/rotary_encoder.h      |  2 +-
>>  2 files changed, 84 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
>> index 15a4cc670a3f..e021615dd3c0 100644
>> --- a/drivers/input/misc/rotary_encoder.c
>> +++ b/drivers/input/misc/rotary_encoder.c
>> @@ -142,6 +142,55 @@ static irqreturn_t rotary_encoder_half_period_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;
>> +     unsigned char sum;
>> +     int state;
>> +
>> +     state = rotary_encoder_get_state(encoder->pdata);
>> +
>> +     /*
>> +      * We encode the previous and the current state using a byte.
>> +      * The previous state in the MSB nibble, the current state in the LSB
>> +      * nibble. Then use a table to decide the direction of the turn.
>> +      */
>> +     sum = (encoder->last_stable << 4) + state;
>> +     switch (sum) {
>> +     case 0x31:
>> +     case 0x10:
>> +     case 0x02:
>> +     case 0x23:
>> +             encoder->dir = 0; /* clockwise */
>> +             break;
>> +
>> +     case 0x13:
>> +     case 0x01:
>> +     case 0x20:
>> +     case 0x32:
>> +             encoder->dir = 1; /* counter-clockwise */
>> +             break;
>> +
>> +     default:
>> +             /*
>> +              * Ignore all other values. This covers the case when the
>> +              * state didn't change (a spurious interrupt) and the
>> +              * cases where the state changed by two steps, making it
>> +              * impossible to tell the direction.
>> +              *
>> +              * In either case, don't report any event and save the
>> +              * state for later.
>> +              */
>> +             goto out;
>> +     }
>> +
>> +     rotary_encoder_report_event(encoder);
>> +
>> +out:
>> +     encoder->last_stable = state;
>> +     return IRQ_HANDLED;
>> +}
>> +
>>  #ifdef CONFIG_OF
>>  static const struct of_device_id rotary_encoder_of_match[] = {
>>       { .compatible = "rotary-encoder", },
>> @@ -176,7 +225,26 @@ static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct devic
>>
>>       pdata->relative_axis = of_property_read_bool(np, "rotary-encoder,relative-axis");
>>       pdata->rollover = of_property_read_bool(np, "rotary-encoder,rollover");
>> -     pdata->half_period = of_property_read_bool(np, "rotary-encoder,half-period");
>> +
>> +     if (!of_get_property(np, "rotary-encoder,steps-per-period", NULL)) {
>> +             /*
>> +              * Fallback to a one step per period behavior if the
>> +              * 'steps-per-period' is not set.
>> +              */
>> +             pdata->steps_per_period = 1;
>> +     } else {
>> +             of_property_read_u32(np, "rotary-encoder,steps-per-period",
>> +                                  &pdata->steps_per_period);
>> +     }
>> +
>> +     /*
>> +      * The 'half-period' property has been deprecated, you must use
>> +      * 'steps-per-period' and set an appropriate value.
>> +      */
>> +     if (of_get_property(np, "rotary-encoder,half-period", NULL)) {
>> +             pr_warn(FW_WARN "\"rotary-encoder,half-period\" is deprecated\n");
>> +             pdata->steps_per_period = 2;
>> +     }
>
> Hmm, I do not think we need to warn about old DTSes if we consider them
> ABI. How about if we parse like this:
>
>         error = of_property_read_u32(np, "rotary-encoder,steps-per-period",
>                                      &pdata->steps_per_period);
>         if (error) {
>                 /*
>                  * The 'half-period' property has been deprecated, you must use
>                  * 'steps-per-period' and set an appropriate value, but we still
>                  * need to parse it to maintain compatibility.
>                  */
>                 if (of_property_read_bool(np, "rotary-encoder,half-period")) {
>                         pdata->steps_per_period = 2;
>                 } else {
>                         /* Fallback to a one step per period behavior */
>                         pdata->steps_per_period = 1;
>                 }
>         }
>
>
> (no need to resubmit if you are OK with this).
>

Looks good to me. Feel free to pick the patches and amend them
as above.

Thanks a lot!
-- 
Ezequiel García, VanguardiaSur
www.vanguardiasur.com.ar
--
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