[PATCH v2 0/3] Input: rotary-encoder - use more than two gpios

[PATCH v2 0/3] Input: rotary-encoder - use more than two gpios

[Uwe Kleine-König]

Hello,

Some time ago I sent a v1 of this, now after testing the changes more
deeply patch 3 changed a bit. The old series started with

	Date: Wed,  2 Dec 2015 11:07:11 +0100                                                                                               
	From: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>                                                                             
	Subject: [PATCH RFC 0/3] input: rotary_encoder: use more than two gpios as input                                                    
	Message-Id: <1449050834-31779-1-git-send-email-u.kleine-koenig@pengutronix.de>                                                      

The two first patches are just preparation for the third patch.

There is an obvious improvement that allows detection of quick changes
more reliably with >2 gpios, but I didn't implement this yet. (With 4
GPIOs you can distinguish a counter clockwise movement of three states
from a clock wise movement of a single state. Still the patch is useful
as it makes these devices work at all.

My test device looks as follows:

        rotary@0 {
                compatible = "rotary-encoder";
                gpios = <&gpio4 12 GPIO_ACTIVE_HIGH>, <&gpio4 11 GPIO_ACTIVE_HIGH>, <&gpio4 10 GPIO_ACTIVE_HIGH>, <&gpio4 9 GPIO_ACTIVE_HIGH>;

                rotary-encoder,steps = <16>;
		rotary-encoder,steps-per-period = <16>;
        };

While Daniel Mack and Rojhalat Ibrahim agreed that this device is an
absolute encoder and should be supported by a simpler logic, I still
consider it worthwhile to get these patches in as a first step. Also the
binding looks right, so IMHO the comments shouldn't stop this series
from going in.

Best regards
Uwe

Uwe Kleine-König (3):
  Input: rotary-encoder - make use of devm_* to simplify .probe and
    .remove
  Input: rotary-encoder - move configuration data to driver data
  Input: rotary-encoder - support more than 2 gpios as input

 .../devicetree/bindings/input/rotary-encoder.txt   |   2 +-
 arch/arm/mach-pxa/raumfeld.c                       |  25 +-
 drivers/input/misc/rotary_encoder.c                | 327 +++++++++------------
 include/linux/rotary_encoder.h                     |   4 -
 4 files changed, 158 insertions(+), 200 deletions(-)

-- 
2.7.0.rc3

--
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 v2 0/3] Input: rotary-encoder - use more than two gpios

[Uwe Kleine-König]

Hello,

Some time ago I sent a v1 of this, now after testing the changes more
deeply patch 3 changed a bit. The old series started with

	Date: Wed,  2 Dec 2015 11:07:11 +0100                                                                                               
	From: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>                                                                             
	Subject: [PATCH RFC 0/3] input: rotary_encoder: use more than two gpios as input                                                    
	Message-Id: <1449050834-31779-1-git-send-email-u.kleine-koenig@pengutronix.de>                                                      

The two first patches are just preparation for the third patch.

There is an obvious improvement that allows detection of quick changes
more reliably with >2 gpios, but I didn't implement this yet. (With 4
GPIOs you can distinguish a counter clockwise movement of three states
from a clock wise movement of a single state. Still the patch is useful
as it makes these devices work at all.

My test device looks as follows:

        rotary at 0 {
                compatible = "rotary-encoder";
                gpios = <&gpio4 12 GPIO_ACTIVE_HIGH>, <&gpio4 11 GPIO_ACTIVE_HIGH>, <&gpio4 10 GPIO_ACTIVE_HIGH>, <&gpio4 9 GPIO_ACTIVE_HIGH>;

                rotary-encoder,steps = <16>;
		rotary-encoder,steps-per-period = <16>;
        };

While Daniel Mack and Rojhalat Ibrahim agreed that this device is an
absolute encoder and should be supported by a simpler logic, I still
consider it worthwhile to get these patches in as a first step. Also the
binding looks right, so IMHO the comments shouldn't stop this series
from going in.

Best regards
Uwe

Uwe Kleine-K?nig (3):
  Input: rotary-encoder - make use of devm_* to simplify .probe and
    .remove
  Input: rotary-encoder - move configuration data to driver data
  Input: rotary-encoder - support more than 2 gpios as input

 .../devicetree/bindings/input/rotary-encoder.txt   |   2 +-
 arch/arm/mach-pxa/raumfeld.c                       |  25 +-
 drivers/input/misc/rotary_encoder.c                | 327 +++++++++------------
 include/linux/rotary_encoder.h                     |   4 -
 4 files changed, 158 insertions(+), 200 deletions(-)

-- 
2.7.0.rc3

[PATCH v2 1/3] Input: rotary-encoder - make use of devm_* to simplify .probe and .remove

[Uwe Kleine-König]

For all operations called in .probe there are devm_* variants.
(input_register_device is clever enough to be devm aware on its own.)
This allows to get rid of the error unwind code paths in .probe and the
complete .remove function.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/input/misc/rotary_encoder.c | 74 +++++++++++--------------------------
 1 file changed, 22 insertions(+), 52 deletions(-)

diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
index 8aee71986430..386bdb5314e6 100644
--- a/drivers/input/misc/rotary_encoder.c
+++ b/drivers/input/misc/rotary_encoder.c
@@ -211,8 +211,8 @@ static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct devic
 	if (!of_id || !np)
 		return NULL;
 
-	pdata = kzalloc(sizeof(struct rotary_encoder_platform_data),
-			GFP_KERNEL);
+	pdata = devm_kzalloc(dev, sizeof(struct rotary_encoder_platform_data),
+			     GFP_KERNEL);
 	if (!pdata)
 		return ERR_PTR(-ENOMEM);
 
@@ -277,11 +277,11 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 		}
 	}
 
-	encoder = kzalloc(sizeof(struct rotary_encoder), GFP_KERNEL);
-	input = input_allocate_device();
+	encoder = devm_kzalloc(dev, sizeof(struct rotary_encoder), GFP_KERNEL);
+	input = devm_input_allocate_device(&pdev->dev);
 	if (!encoder || !input) {
-		err = -ENOMEM;
-		goto exit_free_mem;
+		dev_err(dev, "unable to allocate memory\n");
+		return -ENOMEM;
 	}
 
 	encoder->input = input;
@@ -301,16 +301,18 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 	}
 
 	/* request the GPIOs */
-	err = gpio_request_one(pdata->gpio_a, GPIOF_IN, dev_name(dev));
+	err = devm_gpio_request_one(dev, pdata->gpio_a,
+				    GPIOF_IN, dev_name(dev));
 	if (err) {
 		dev_err(dev, "unable to request GPIO %d\n", pdata->gpio_a);
-		goto exit_free_mem;
+		return err;
 	}
 
-	err = gpio_request_one(pdata->gpio_b, GPIOF_IN, dev_name(dev));
+	err = devm_gpio_request_one(dev, pdata->gpio_b,
+				    GPIOF_IN, dev_name(dev));
 	if (err) {
 		dev_err(dev, "unable to request GPIO %d\n", pdata->gpio_b);
-		goto exit_free_gpio_a;
+		return err;
 	}
 
 	encoder->irq_a = gpio_to_irq(pdata->gpio_a);
@@ -331,30 +333,29 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 	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;
+		return -EINVAL;
 	}
 
-	err = request_irq(encoder->irq_a, handler,
-			  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
-			  DRV_NAME, encoder);
+	err = devm_request_irq(dev, encoder->irq_a, handler,
+			       IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+			       DRV_NAME, encoder);
 	if (err) {
 		dev_err(dev, "unable to request IRQ %d\n", encoder->irq_a);
-		goto exit_free_gpio_b;
+		return err;
 	}
 
-	err = request_irq(encoder->irq_b, handler,
-			  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
-			  DRV_NAME, encoder);
+	err = devm_request_irq(dev, encoder->irq_b, handler,
+			       IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+			       DRV_NAME, encoder);
 	if (err) {
 		dev_err(dev, "unable to request IRQ %d\n", encoder->irq_b);
-		goto exit_free_irq_a;
+		return err;
 	}
 
 	err = input_register_device(input);
 	if (err) {
 		dev_err(dev, "failed to register input device\n");
-		goto exit_free_irq_b;
+		return err;
 	}
 
 	device_init_wakeup(&pdev->dev, pdata->wakeup_source);
@@ -362,42 +363,11 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, encoder);
 
 	return 0;
-
-exit_free_irq_b:
-	free_irq(encoder->irq_b, encoder);
-exit_free_irq_a:
-	free_irq(encoder->irq_a, encoder);
-exit_free_gpio_b:
-	gpio_free(pdata->gpio_b);
-exit_free_gpio_a:
-	gpio_free(pdata->gpio_a);
-exit_free_mem:
-	input_free_device(input);
-	kfree(encoder);
-	if (!dev_get_platdata(&pdev->dev))
-		kfree(pdata);
-
-	return err;
 }
 
 static int rotary_encoder_remove(struct platform_device *pdev)
 {
-	struct rotary_encoder *encoder = platform_get_drvdata(pdev);
-	const struct rotary_encoder_platform_data *pdata = encoder->pdata;
-
 	device_init_wakeup(&pdev->dev, false);
-
-	free_irq(encoder->irq_a, encoder);
-	free_irq(encoder->irq_b, encoder);
-	gpio_free(pdata->gpio_a);
-	gpio_free(pdata->gpio_b);
-
-	input_unregister_device(encoder->input);
-	kfree(encoder);
-
-	if (!dev_get_platdata(&pdev->dev))
-		kfree(pdata);
-
 	return 0;
 }
 
-- 
2.7.0.rc3

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

[PATCH v2 1/3] Input: rotary-encoder - make use of devm_* to simplify .probe and .remove

[Uwe Kleine-König]

For all operations called in .probe there are devm_* variants.
(input_register_device is clever enough to be devm aware on its own.)
This allows to get rid of the error unwind code paths in .probe and the
complete .remove function.

Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
 drivers/input/misc/rotary_encoder.c | 74 +++++++++++--------------------------
 1 file changed, 22 insertions(+), 52 deletions(-)

diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
index 8aee71986430..386bdb5314e6 100644
--- a/drivers/input/misc/rotary_encoder.c
+++ b/drivers/input/misc/rotary_encoder.c
@@ -211,8 +211,8 @@ static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct devic
 	if (!of_id || !np)
 		return NULL;
 
-	pdata = kzalloc(sizeof(struct rotary_encoder_platform_data),
-			GFP_KERNEL);
+	pdata = devm_kzalloc(dev, sizeof(struct rotary_encoder_platform_data),
+			     GFP_KERNEL);
 	if (!pdata)
 		return ERR_PTR(-ENOMEM);
 
@@ -277,11 +277,11 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 		}
 	}
 
-	encoder = kzalloc(sizeof(struct rotary_encoder), GFP_KERNEL);
-	input = input_allocate_device();
+	encoder = devm_kzalloc(dev, sizeof(struct rotary_encoder), GFP_KERNEL);
+	input = devm_input_allocate_device(&pdev->dev);
 	if (!encoder || !input) {
-		err = -ENOMEM;
-		goto exit_free_mem;
+		dev_err(dev, "unable to allocate memory\n");
+		return -ENOMEM;
 	}
 
 	encoder->input = input;
@@ -301,16 +301,18 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 	}
 
 	/* request the GPIOs */
-	err = gpio_request_one(pdata->gpio_a, GPIOF_IN, dev_name(dev));
+	err = devm_gpio_request_one(dev, pdata->gpio_a,
+				    GPIOF_IN, dev_name(dev));
 	if (err) {
 		dev_err(dev, "unable to request GPIO %d\n", pdata->gpio_a);
-		goto exit_free_mem;
+		return err;
 	}
 
-	err = gpio_request_one(pdata->gpio_b, GPIOF_IN, dev_name(dev));
+	err = devm_gpio_request_one(dev, pdata->gpio_b,
+				    GPIOF_IN, dev_name(dev));
 	if (err) {
 		dev_err(dev, "unable to request GPIO %d\n", pdata->gpio_b);
-		goto exit_free_gpio_a;
+		return err;
 	}
 
 	encoder->irq_a = gpio_to_irq(pdata->gpio_a);
@@ -331,30 +333,29 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 	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;
+		return -EINVAL;
 	}
 
-	err = request_irq(encoder->irq_a, handler,
-			  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
-			  DRV_NAME, encoder);
+	err = devm_request_irq(dev, encoder->irq_a, handler,
+			       IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+			       DRV_NAME, encoder);
 	if (err) {
 		dev_err(dev, "unable to request IRQ %d\n", encoder->irq_a);
-		goto exit_free_gpio_b;
+		return err;
 	}
 
-	err = request_irq(encoder->irq_b, handler,
-			  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
-			  DRV_NAME, encoder);
+	err = devm_request_irq(dev, encoder->irq_b, handler,
+			       IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+			       DRV_NAME, encoder);
 	if (err) {
 		dev_err(dev, "unable to request IRQ %d\n", encoder->irq_b);
-		goto exit_free_irq_a;
+		return err;
 	}
 
 	err = input_register_device(input);
 	if (err) {
 		dev_err(dev, "failed to register input device\n");
-		goto exit_free_irq_b;
+		return err;
 	}
 
 	device_init_wakeup(&pdev->dev, pdata->wakeup_source);
@@ -362,42 +363,11 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, encoder);
 
 	return 0;
-
-exit_free_irq_b:
-	free_irq(encoder->irq_b, encoder);
-exit_free_irq_a:
-	free_irq(encoder->irq_a, encoder);
-exit_free_gpio_b:
-	gpio_free(pdata->gpio_b);
-exit_free_gpio_a:
-	gpio_free(pdata->gpio_a);
-exit_free_mem:
-	input_free_device(input);
-	kfree(encoder);
-	if (!dev_get_platdata(&pdev->dev))
-		kfree(pdata);
-
-	return err;
 }
 
 static int rotary_encoder_remove(struct platform_device *pdev)
 {
-	struct rotary_encoder *encoder = platform_get_drvdata(pdev);
-	const struct rotary_encoder_platform_data *pdata = encoder->pdata;
-
 	device_init_wakeup(&pdev->dev, false);
-
-	free_irq(encoder->irq_a, encoder);
-	free_irq(encoder->irq_b, encoder);
-	gpio_free(pdata->gpio_a);
-	gpio_free(pdata->gpio_b);
-
-	input_unregister_device(encoder->input);
-	kfree(encoder);
-
-	if (!dev_get_platdata(&pdev->dev))
-		kfree(pdata);
-
 	return 0;
 }
 
-- 
2.7.0.rc3

Re: [PATCH v2 1/3] Input: rotary-encoder - make use of devm_* to simplify .probe and .remove

[Daniel Mack]

On 02/02/2016 11:24 AM, Uwe Kleine-König wrote:
> For all operations called in .probe there are devm_* variants.
> (input_register_device is clever enough to be devm aware on its own.)
> This allows to get rid of the error unwind code paths in .probe and the
> complete .remove function.

While at it, could you also convert this over to the gpiod_* API?


Thanks,
Daniel



> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/input/misc/rotary_encoder.c | 74 +++++++++++--------------------------
>  1 file changed, 22 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
> index 8aee71986430..386bdb5314e6 100644
> --- a/drivers/input/misc/rotary_encoder.c
> +++ b/drivers/input/misc/rotary_encoder.c
> @@ -211,8 +211,8 @@ static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct devic
>  	if (!of_id || !np)
>  		return NULL;
>  
> -	pdata = kzalloc(sizeof(struct rotary_encoder_platform_data),
> -			GFP_KERNEL);
> +	pdata = devm_kzalloc(dev, sizeof(struct rotary_encoder_platform_data),
> +			     GFP_KERNEL);
>  	if (!pdata)
>  		return ERR_PTR(-ENOMEM);
>  
> @@ -277,11 +277,11 @@ static int rotary_encoder_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	encoder = kzalloc(sizeof(struct rotary_encoder), GFP_KERNEL);
> -	input = input_allocate_device();
> +	encoder = devm_kzalloc(dev, sizeof(struct rotary_encoder), GFP_KERNEL);
> +	input = devm_input_allocate_device(&pdev->dev);
>  	if (!encoder || !input) {
> -		err = -ENOMEM;
> -		goto exit_free_mem;
> +		dev_err(dev, "unable to allocate memory\n");
> +		return -ENOMEM;
>  	}
>  
>  	encoder->input = input;
> @@ -301,16 +301,18 @@ static int rotary_encoder_probe(struct platform_device *pdev)
>  	}
>  
>  	/* request the GPIOs */
> -	err = gpio_request_one(pdata->gpio_a, GPIOF_IN, dev_name(dev));
> +	err = devm_gpio_request_one(dev, pdata->gpio_a,
> +				    GPIOF_IN, dev_name(dev));
>  	if (err) {
>  		dev_err(dev, "unable to request GPIO %d\n", pdata->gpio_a);
> -		goto exit_free_mem;
> +		return err;
>  	}
>  
> -	err = gpio_request_one(pdata->gpio_b, GPIOF_IN, dev_name(dev));
> +	err = devm_gpio_request_one(dev, pdata->gpio_b,
> +				    GPIOF_IN, dev_name(dev));
>  	if (err) {
>  		dev_err(dev, "unable to request GPIO %d\n", pdata->gpio_b);
> -		goto exit_free_gpio_a;
> +		return err;
>  	}
>  
>  	encoder->irq_a = gpio_to_irq(pdata->gpio_a);
> @@ -331,30 +333,29 @@ static int rotary_encoder_probe(struct platform_device *pdev)
>  	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;
> +		return -EINVAL;
>  	}
>  
> -	err = request_irq(encoder->irq_a, handler,
> -			  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> -			  DRV_NAME, encoder);
> +	err = devm_request_irq(dev, encoder->irq_a, handler,
> +			       IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> +			       DRV_NAME, encoder);
>  	if (err) {
>  		dev_err(dev, "unable to request IRQ %d\n", encoder->irq_a);
> -		goto exit_free_gpio_b;
> +		return err;
>  	}
>  
> -	err = request_irq(encoder->irq_b, handler,
> -			  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> -			  DRV_NAME, encoder);
> +	err = devm_request_irq(dev, encoder->irq_b, handler,
> +			       IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> +			       DRV_NAME, encoder);
>  	if (err) {
>  		dev_err(dev, "unable to request IRQ %d\n", encoder->irq_b);
> -		goto exit_free_irq_a;
> +		return err;
>  	}
>  
>  	err = input_register_device(input);
>  	if (err) {
>  		dev_err(dev, "failed to register input device\n");
> -		goto exit_free_irq_b;
> +		return err;
>  	}
>  
>  	device_init_wakeup(&pdev->dev, pdata->wakeup_source);
> @@ -362,42 +363,11 @@ static int rotary_encoder_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, encoder);
>  
>  	return 0;
> -
> -exit_free_irq_b:
> -	free_irq(encoder->irq_b, encoder);
> -exit_free_irq_a:
> -	free_irq(encoder->irq_a, encoder);
> -exit_free_gpio_b:
> -	gpio_free(pdata->gpio_b);
> -exit_free_gpio_a:
> -	gpio_free(pdata->gpio_a);
> -exit_free_mem:
> -	input_free_device(input);
> -	kfree(encoder);
> -	if (!dev_get_platdata(&pdev->dev))
> -		kfree(pdata);
> -
> -	return err;
>  }
>  
>  static int rotary_encoder_remove(struct platform_device *pdev)
>  {
> -	struct rotary_encoder *encoder = platform_get_drvdata(pdev);
> -	const struct rotary_encoder_platform_data *pdata = encoder->pdata;
> -
>  	device_init_wakeup(&pdev->dev, false);
> -
> -	free_irq(encoder->irq_a, encoder);
> -	free_irq(encoder->irq_b, encoder);
> -	gpio_free(pdata->gpio_a);
> -	gpio_free(pdata->gpio_b);
> -
> -	input_unregister_device(encoder->input);
> -	kfree(encoder);
> -
> -	if (!dev_get_platdata(&pdev->dev))
> -		kfree(pdata);
> -
>  	return 0;
>  }
>  
> 


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/3] Input: rotary-encoder - make use of devm_* to simplify .probe and .remove

[Daniel Mack]

On 02/02/2016 11:24 AM, Uwe Kleine-K?nig wrote:
> For all operations called in .probe there are devm_* variants.
> (input_register_device is clever enough to be devm aware on its own.)
> This allows to get rid of the error unwind code paths in .probe and the
> complete .remove function.

While at it, could you also convert this over to the gpiod_* API?


Thanks,
Daniel



> 
> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/input/misc/rotary_encoder.c | 74 +++++++++++--------------------------
>  1 file changed, 22 insertions(+), 52 deletions(-)
> 
> diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
> index 8aee71986430..386bdb5314e6 100644
> --- a/drivers/input/misc/rotary_encoder.c
> +++ b/drivers/input/misc/rotary_encoder.c
> @@ -211,8 +211,8 @@ static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct devic
>  	if (!of_id || !np)
>  		return NULL;
>  
> -	pdata = kzalloc(sizeof(struct rotary_encoder_platform_data),
> -			GFP_KERNEL);
> +	pdata = devm_kzalloc(dev, sizeof(struct rotary_encoder_platform_data),
> +			     GFP_KERNEL);
>  	if (!pdata)
>  		return ERR_PTR(-ENOMEM);
>  
> @@ -277,11 +277,11 @@ static int rotary_encoder_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> -	encoder = kzalloc(sizeof(struct rotary_encoder), GFP_KERNEL);
> -	input = input_allocate_device();
> +	encoder = devm_kzalloc(dev, sizeof(struct rotary_encoder), GFP_KERNEL);
> +	input = devm_input_allocate_device(&pdev->dev);
>  	if (!encoder || !input) {
> -		err = -ENOMEM;
> -		goto exit_free_mem;
> +		dev_err(dev, "unable to allocate memory\n");
> +		return -ENOMEM;
>  	}
>  
>  	encoder->input = input;
> @@ -301,16 +301,18 @@ static int rotary_encoder_probe(struct platform_device *pdev)
>  	}
>  
>  	/* request the GPIOs */
> -	err = gpio_request_one(pdata->gpio_a, GPIOF_IN, dev_name(dev));
> +	err = devm_gpio_request_one(dev, pdata->gpio_a,
> +				    GPIOF_IN, dev_name(dev));
>  	if (err) {
>  		dev_err(dev, "unable to request GPIO %d\n", pdata->gpio_a);
> -		goto exit_free_mem;
> +		return err;
>  	}
>  
> -	err = gpio_request_one(pdata->gpio_b, GPIOF_IN, dev_name(dev));
> +	err = devm_gpio_request_one(dev, pdata->gpio_b,
> +				    GPIOF_IN, dev_name(dev));
>  	if (err) {
>  		dev_err(dev, "unable to request GPIO %d\n", pdata->gpio_b);
> -		goto exit_free_gpio_a;
> +		return err;
>  	}
>  
>  	encoder->irq_a = gpio_to_irq(pdata->gpio_a);
> @@ -331,30 +333,29 @@ static int rotary_encoder_probe(struct platform_device *pdev)
>  	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;
> +		return -EINVAL;
>  	}
>  
> -	err = request_irq(encoder->irq_a, handler,
> -			  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> -			  DRV_NAME, encoder);
> +	err = devm_request_irq(dev, encoder->irq_a, handler,
> +			       IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> +			       DRV_NAME, encoder);
>  	if (err) {
>  		dev_err(dev, "unable to request IRQ %d\n", encoder->irq_a);
> -		goto exit_free_gpio_b;
> +		return err;
>  	}
>  
> -	err = request_irq(encoder->irq_b, handler,
> -			  IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> -			  DRV_NAME, encoder);
> +	err = devm_request_irq(dev, encoder->irq_b, handler,
> +			       IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
> +			       DRV_NAME, encoder);
>  	if (err) {
>  		dev_err(dev, "unable to request IRQ %d\n", encoder->irq_b);
> -		goto exit_free_irq_a;
> +		return err;
>  	}
>  
>  	err = input_register_device(input);
>  	if (err) {
>  		dev_err(dev, "failed to register input device\n");
> -		goto exit_free_irq_b;
> +		return err;
>  	}
>  
>  	device_init_wakeup(&pdev->dev, pdata->wakeup_source);
> @@ -362,42 +363,11 @@ static int rotary_encoder_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, encoder);
>  
>  	return 0;
> -
> -exit_free_irq_b:
> -	free_irq(encoder->irq_b, encoder);
> -exit_free_irq_a:
> -	free_irq(encoder->irq_a, encoder);
> -exit_free_gpio_b:
> -	gpio_free(pdata->gpio_b);
> -exit_free_gpio_a:
> -	gpio_free(pdata->gpio_a);
> -exit_free_mem:
> -	input_free_device(input);
> -	kfree(encoder);
> -	if (!dev_get_platdata(&pdev->dev))
> -		kfree(pdata);
> -
> -	return err;
>  }
>  
>  static int rotary_encoder_remove(struct platform_device *pdev)
>  {
> -	struct rotary_encoder *encoder = platform_get_drvdata(pdev);
> -	const struct rotary_encoder_platform_data *pdata = encoder->pdata;
> -
>  	device_init_wakeup(&pdev->dev, false);
> -
> -	free_irq(encoder->irq_a, encoder);
> -	free_irq(encoder->irq_b, encoder);
> -	gpio_free(pdata->gpio_a);
> -	gpio_free(pdata->gpio_b);
> -
> -	input_unregister_device(encoder->input);
> -	kfree(encoder);
> -
> -	if (!dev_get_platdata(&pdev->dev))
> -		kfree(pdata);
> -
>  	return 0;
>  }
>  
> 

Re: [PATCH v2 1/3] Input: rotary-encoder - make use of devm_* to simplify .probe and .remove

[Uwe Kleine-König]

On Tue, Feb 02, 2016 at 12:56:24PM +0100, Daniel Mack wrote:
> On 02/02/2016 11:24 AM, Uwe Kleine-König wrote:
> > For all operations called in .probe there are devm_* variants.
> > (input_register_device is clever enough to be devm aware on its own.)
> > This allows to get rid of the error unwind code paths in .probe and the
> > complete .remove function.
> 
> While at it, could you also convert this over to the gpiod_* API?

This is done in patch 3.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
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

[PATCH v2 1/3] Input: rotary-encoder - make use of devm_* to simplify .probe and .remove

[Uwe Kleine-König]

On Tue, Feb 02, 2016 at 12:56:24PM +0100, Daniel Mack wrote:
> On 02/02/2016 11:24 AM, Uwe Kleine-K?nig wrote:
> > For all operations called in .probe there are devm_* variants.
> > (input_register_device is clever enough to be devm aware on its own.)
> > This allows to get rid of the error unwind code paths in .probe and the
> > complete .remove function.
> 
> While at it, could you also convert this over to the gpiod_* API?

This is done in patch 3.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH v2 1/3] Input: rotary-encoder - make use of devm_* to simplify .probe and .remove

[Daniel Mack]

On 02/02/2016 02:00 PM, Uwe Kleine-König wrote:
> On Tue, Feb 02, 2016 at 12:56:24PM +0100, Daniel Mack wrote:
>> On 02/02/2016 11:24 AM, Uwe Kleine-König wrote:
>>> For all operations called in .probe there are devm_* variants.
>>> (input_register_device is clever enough to be devm aware on its own.)
>>> This allows to get rid of the error unwind code paths in .probe and the
>>> complete .remove function.
>>
>> While at it, could you also convert this over to the gpiod_* API?
> 
> This is done in patch 3.

Ah, right. Missed that detail, sorry.


Daniel

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

[PATCH v2 1/3] Input: rotary-encoder - make use of devm_* to simplify .probe and .remove

[Daniel Mack]

On 02/02/2016 02:00 PM, Uwe Kleine-K?nig wrote:
> On Tue, Feb 02, 2016 at 12:56:24PM +0100, Daniel Mack wrote:
>> On 02/02/2016 11:24 AM, Uwe Kleine-K?nig wrote:
>>> For all operations called in .probe there are devm_* variants.
>>> (input_register_device is clever enough to be devm aware on its own.)
>>> This allows to get rid of the error unwind code paths in .probe and the
>>> complete .remove function.
>>
>> While at it, could you also convert this over to the gpiod_* API?
> 
> This is done in patch 3.

Ah, right. Missed that detail, sorry.


Daniel

[PATCH v2 2/3] Input: rotary-encoder - move configuration data to driver data

[Uwe Kleine-König]

This is a preparation for the next patche. There is no change in
behaviour intended.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/input/misc/rotary_encoder.c | 166 ++++++++++++++++++++----------------
 1 file changed, 94 insertions(+), 72 deletions(-)

diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
index 386bdb5314e6..0582e851993f 100644
--- a/drivers/input/misc/rotary_encoder.c
+++ b/drivers/input/misc/rotary_encoder.c
@@ -32,58 +32,65 @@
 
 struct rotary_encoder {
 	struct input_dev *input;
-	const struct rotary_encoder_platform_data *pdata;
 
+	/* configuration */
+	unsigned int steps;
 	unsigned int axis;
-	unsigned int pos;
+	unsigned int gpio_a;
+	unsigned int gpio_b;
+	unsigned int inverted_a;
+	unsigned int inverted_b;
+	unsigned int steps_per_period;
+	bool relative_axis;
+	bool rollover;
+	bool wakeup_source;
 
 	unsigned int irq_a;
 	unsigned int irq_b;
 
+	/* state */
+	unsigned int pos;
 	bool armed;
 	unsigned char dir;	/* 0 - clockwise, 1 - CCW */
-
 	char last_stable;
 };
 
-static int rotary_encoder_get_state(const struct rotary_encoder_platform_data *pdata)
+static int rotary_encoder_get_state(const struct rotary_encoder *encoder)
 {
-	int a = !!gpio_get_value(pdata->gpio_a);
-	int b = !!gpio_get_value(pdata->gpio_b);
+	int a = !!gpio_get_value(encoder->gpio_a);
+	int b = !!gpio_get_value(encoder->gpio_b);
 
-	a ^= pdata->inverted_a;
-	b ^= pdata->inverted_b;
+	a ^= encoder->inverted_a;
+	b ^= encoder->inverted_b;
 
 	return ((a << 1) | b);
 }
 
 static void rotary_encoder_report_event(struct rotary_encoder *encoder)
 {
-	const struct rotary_encoder_platform_data *pdata = encoder->pdata;
-
-	if (pdata->relative_axis) {
+	if (encoder->relative_axis) {
 		input_report_rel(encoder->input,
-				 pdata->axis, encoder->dir ? -1 : 1);
+				 encoder->axis, encoder->dir ? -1 : 1);
 	} else {
 		unsigned int pos = encoder->pos;
 
 		if (encoder->dir) {
 			/* turning counter-clockwise */
-			if (pdata->rollover)
-				pos += pdata->steps;
+			if (encoder->rollover)
+				pos += encoder->steps;
 			if (pos)
 				pos--;
 		} else {
 			/* turning clockwise */
-			if (pdata->rollover || pos < pdata->steps)
+			if (encoder->rollover || pos < encoder->steps)
 				pos++;
 		}
 
-		if (pdata->rollover)
-			pos %= pdata->steps;
+		if (encoder->rollover)
+			pos %= encoder->steps;
 
 		encoder->pos = pos;
-		input_report_abs(encoder->input, pdata->axis, encoder->pos);
+		input_report_abs(encoder->input, encoder->axis, encoder->pos);
 	}
 
 	input_sync(encoder->input);
@@ -94,7 +101,7 @@ static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
 	struct rotary_encoder *encoder = dev_id;
 	int state;
 
-	state = rotary_encoder_get_state(encoder->pdata);
+	state = rotary_encoder_get_state(encoder);
 
 	switch (state) {
 	case 0x0:
@@ -123,7 +130,7 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id)
 	struct rotary_encoder *encoder = dev_id;
 	int state;
 
-	state = rotary_encoder_get_state(encoder->pdata);
+	state = rotary_encoder_get_state(encoder);
 
 	switch (state) {
 	case 0x00:
@@ -149,7 +156,7 @@ static irqreturn_t rotary_encoder_quarter_period_irq(int irq, void *dev_id)
 	unsigned char sum;
 	int state;
 
-	state = rotary_encoder_get_state(encoder->pdata);
+	state = rotary_encoder_get_state(encoder);
 
 	/*
 	 * We encode the previous and the current state using a byte.
@@ -199,38 +206,34 @@ static const struct of_device_id rotary_encoder_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, rotary_encoder_of_match);
 
-static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct device *dev)
+static int rotary_encoder_parse_dt(struct device *dev,
+				   struct rotary_encoder *encoder)
 {
 	const struct of_device_id *of_id =
 				of_match_device(rotary_encoder_of_match, dev);
 	struct device_node *np = dev->of_node;
-	struct rotary_encoder_platform_data *pdata;
 	enum of_gpio_flags flags;
 	int error;
 
 	if (!of_id || !np)
-		return NULL;
-
-	pdata = devm_kzalloc(dev, sizeof(struct rotary_encoder_platform_data),
-			     GFP_KERNEL);
-	if (!pdata)
-		return ERR_PTR(-ENOMEM);
+		return 1;
 
-	of_property_read_u32(np, "rotary-encoder,steps", &pdata->steps);
-	of_property_read_u32(np, "linux,axis", &pdata->axis);
+	of_property_read_u32(np, "rotary-encoder,steps", &encoder->steps);
+	of_property_read_u32(np, "linux,axis", &encoder->axis);
 
-	pdata->gpio_a = of_get_gpio_flags(np, 0, &flags);
-	pdata->inverted_a = flags & OF_GPIO_ACTIVE_LOW;
+	encoder->gpio_a = of_get_gpio_flags(np, 0, &flags);
+	encoder->inverted_a = flags & OF_GPIO_ACTIVE_LOW;
 
-	pdata->gpio_b = of_get_gpio_flags(np, 1, &flags);
-	pdata->inverted_b = flags & OF_GPIO_ACTIVE_LOW;
+	encoder->gpio_b = of_get_gpio_flags(np, 1, &flags);
+	encoder->inverted_b = flags & OF_GPIO_ACTIVE_LOW;
 
-	pdata->relative_axis =
+	encoder->relative_axis =
 		of_property_read_bool(np, "rotary-encoder,relative-axis");
-	pdata->rollover = of_property_read_bool(np, "rotary-encoder,rollover");
+	encoder->rollover =
+		of_property_read_bool(np, "rotary-encoder,rollover");
 
 	error = of_property_read_u32(np, "rotary-encoder,steps-per-period",
-				     &pdata->steps_per_period);
+				     &encoder->steps_per_period);
 	if (error) {
 		/*
 		 * The 'half-period' property has been deprecated, you must use
@@ -238,45 +241,57 @@ static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct devic
 		 * need to parse it to maintain compatibility.
 		 */
 		if (of_property_read_bool(np, "rotary-encoder,half-period")) {
-			pdata->steps_per_period = 2;
+			encoder->steps_per_period = 2;
 		} else {
 			/* Fallback to one step per period behavior */
-			pdata->steps_per_period = 1;
+			encoder->steps_per_period = 1;
 		}
 	}
 
-	pdata->wakeup_source = of_property_read_bool(np, "wakeup-source");
+	encoder->wakeup_source = of_property_read_bool(np, "wakeup-source");
 
-	return pdata;
+	return 0;
 }
 #else
-static inline struct rotary_encoder_platform_data *
-rotary_encoder_parse_dt(struct device *dev)
+static inline int rotary_encoder_parse_dt(struct device *dev,
+					  struct rotary_encoder *encoder)
 {
-	return NULL;
+	return 1;
 }
 #endif
 
+static int rotary_encoder_parse_pdata(struct device *dev,
+				      struct rotary_encoder *encoder)
+{
+	const struct rotary_encoder_platform_data *pdata;
+
+	pdata = dev_get_platdata(dev);
+	if (!pdata) {
+		dev_err(dev, "missing platform data\n");
+		return -EINVAL;
+	}
+
+	encoder->steps = pdata->steps;
+	encoder->axis = pdata->axis;
+	encoder->gpio_a = pdata->gpio_a;
+	encoder->gpio_b = pdata->gpio_b;
+	encoder->inverted_a = pdata->inverted_a;
+	encoder->inverted_b = pdata->inverted_b;
+	encoder->steps_per_period = pdata->steps_per_period;
+	encoder->relative_axis = pdata->relative_axis;
+	encoder->rollover = pdata->rollover;
+
+	return 0;
+}
+
 static int rotary_encoder_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	const struct rotary_encoder_platform_data *pdata = dev_get_platdata(dev);
 	struct rotary_encoder *encoder;
 	struct input_dev *input;
 	irq_handler_t handler;
 	int err;
 
-	if (!pdata) {
-		pdata = rotary_encoder_parse_dt(dev);
-		if (IS_ERR(pdata))
-			return PTR_ERR(pdata);
-
-		if (!pdata) {
-			dev_err(dev, "missing platform data\n");
-			return -EINVAL;
-		}
-	}
-
 	encoder = devm_kzalloc(dev, sizeof(struct rotary_encoder), GFP_KERNEL);
 	input = devm_input_allocate_device(&pdev->dev);
 	if (!encoder || !input) {
@@ -284,55 +299,62 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	err = rotary_encoder_parse_dt(dev, encoder);
+	if (err > 0)
+		/* not instatiated by dt */
+		err = rotary_encoder_parse_pdata(dev, encoder);
+
+	if (err < 0)
+		return err;
+
 	encoder->input = input;
-	encoder->pdata = pdata;
 
 	input->name = pdev->name;
 	input->id.bustype = BUS_HOST;
 	input->dev.parent = dev;
 
-	if (pdata->relative_axis) {
+	if (encoder->relative_axis) {
 		input->evbit[0] = BIT_MASK(EV_REL);
-		input->relbit[0] = BIT_MASK(pdata->axis);
+		input->relbit[0] = BIT_MASK(encoder->axis);
 	} else {
 		input->evbit[0] = BIT_MASK(EV_ABS);
 		input_set_abs_params(encoder->input,
-				     pdata->axis, 0, pdata->steps, 0, 1);
+				     encoder->axis, 0, encoder->steps, 0, 1);
 	}
 
 	/* request the GPIOs */
-	err = devm_gpio_request_one(dev, pdata->gpio_a,
+	err = devm_gpio_request_one(dev, encoder->gpio_a,
 				    GPIOF_IN, dev_name(dev));
 	if (err) {
-		dev_err(dev, "unable to request GPIO %d\n", pdata->gpio_a);
+		dev_err(dev, "unable to request GPIO %d\n", encoder->gpio_a);
 		return err;
 	}
 
-	err = devm_gpio_request_one(dev, pdata->gpio_b,
+	err = devm_gpio_request_one(dev, encoder->gpio_b,
 				    GPIOF_IN, dev_name(dev));
 	if (err) {
-		dev_err(dev, "unable to request GPIO %d\n", pdata->gpio_b);
+		dev_err(dev, "unable to request GPIO %d\n", encoder->gpio_b);
 		return err;
 	}
 
-	encoder->irq_a = gpio_to_irq(pdata->gpio_a);
-	encoder->irq_b = gpio_to_irq(pdata->gpio_b);
+	encoder->irq_a = gpio_to_irq(encoder->gpio_a);
+	encoder->irq_b = gpio_to_irq(encoder->gpio_b);
 
-	switch (pdata->steps_per_period) {
+	switch (encoder->steps_per_period) {
 	case 4:
 		handler = &rotary_encoder_quarter_period_irq;
-		encoder->last_stable = rotary_encoder_get_state(pdata);
+		encoder->last_stable = rotary_encoder_get_state(encoder);
 		break;
 	case 2:
 		handler = &rotary_encoder_half_period_irq;
-		encoder->last_stable = rotary_encoder_get_state(pdata);
+		encoder->last_stable = rotary_encoder_get_state(encoder);
 		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);
+			encoder->steps_per_period);
 		return -EINVAL;
 	}
 
@@ -358,7 +380,7 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	device_init_wakeup(&pdev->dev, pdata->wakeup_source);
+	device_init_wakeup(&pdev->dev, encoder->wakeup_source);
 
 	platform_set_drvdata(pdev, encoder);
 
-- 
2.7.0.rc3

--
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 v2 2/3] Input: rotary-encoder - move configuration data to driver data

[Uwe Kleine-König]

This is a preparation for the next patche. There is no change in
behaviour intended.

Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
 drivers/input/misc/rotary_encoder.c | 166 ++++++++++++++++++++----------------
 1 file changed, 94 insertions(+), 72 deletions(-)

diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
index 386bdb5314e6..0582e851993f 100644
--- a/drivers/input/misc/rotary_encoder.c
+++ b/drivers/input/misc/rotary_encoder.c
@@ -32,58 +32,65 @@
 
 struct rotary_encoder {
 	struct input_dev *input;
-	const struct rotary_encoder_platform_data *pdata;
 
+	/* configuration */
+	unsigned int steps;
 	unsigned int axis;
-	unsigned int pos;
+	unsigned int gpio_a;
+	unsigned int gpio_b;
+	unsigned int inverted_a;
+	unsigned int inverted_b;
+	unsigned int steps_per_period;
+	bool relative_axis;
+	bool rollover;
+	bool wakeup_source;
 
 	unsigned int irq_a;
 	unsigned int irq_b;
 
+	/* state */
+	unsigned int pos;
 	bool armed;
 	unsigned char dir;	/* 0 - clockwise, 1 - CCW */
-
 	char last_stable;
 };
 
-static int rotary_encoder_get_state(const struct rotary_encoder_platform_data *pdata)
+static int rotary_encoder_get_state(const struct rotary_encoder *encoder)
 {
-	int a = !!gpio_get_value(pdata->gpio_a);
-	int b = !!gpio_get_value(pdata->gpio_b);
+	int a = !!gpio_get_value(encoder->gpio_a);
+	int b = !!gpio_get_value(encoder->gpio_b);
 
-	a ^= pdata->inverted_a;
-	b ^= pdata->inverted_b;
+	a ^= encoder->inverted_a;
+	b ^= encoder->inverted_b;
 
 	return ((a << 1) | b);
 }
 
 static void rotary_encoder_report_event(struct rotary_encoder *encoder)
 {
-	const struct rotary_encoder_platform_data *pdata = encoder->pdata;
-
-	if (pdata->relative_axis) {
+	if (encoder->relative_axis) {
 		input_report_rel(encoder->input,
-				 pdata->axis, encoder->dir ? -1 : 1);
+				 encoder->axis, encoder->dir ? -1 : 1);
 	} else {
 		unsigned int pos = encoder->pos;
 
 		if (encoder->dir) {
 			/* turning counter-clockwise */
-			if (pdata->rollover)
-				pos += pdata->steps;
+			if (encoder->rollover)
+				pos += encoder->steps;
 			if (pos)
 				pos--;
 		} else {
 			/* turning clockwise */
-			if (pdata->rollover || pos < pdata->steps)
+			if (encoder->rollover || pos < encoder->steps)
 				pos++;
 		}
 
-		if (pdata->rollover)
-			pos %= pdata->steps;
+		if (encoder->rollover)
+			pos %= encoder->steps;
 
 		encoder->pos = pos;
-		input_report_abs(encoder->input, pdata->axis, encoder->pos);
+		input_report_abs(encoder->input, encoder->axis, encoder->pos);
 	}
 
 	input_sync(encoder->input);
@@ -94,7 +101,7 @@ static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
 	struct rotary_encoder *encoder = dev_id;
 	int state;
 
-	state = rotary_encoder_get_state(encoder->pdata);
+	state = rotary_encoder_get_state(encoder);
 
 	switch (state) {
 	case 0x0:
@@ -123,7 +130,7 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id)
 	struct rotary_encoder *encoder = dev_id;
 	int state;
 
-	state = rotary_encoder_get_state(encoder->pdata);
+	state = rotary_encoder_get_state(encoder);
 
 	switch (state) {
 	case 0x00:
@@ -149,7 +156,7 @@ static irqreturn_t rotary_encoder_quarter_period_irq(int irq, void *dev_id)
 	unsigned char sum;
 	int state;
 
-	state = rotary_encoder_get_state(encoder->pdata);
+	state = rotary_encoder_get_state(encoder);
 
 	/*
 	 * We encode the previous and the current state using a byte.
@@ -199,38 +206,34 @@ static const struct of_device_id rotary_encoder_of_match[] = {
 };
 MODULE_DEVICE_TABLE(of, rotary_encoder_of_match);
 
-static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct device *dev)
+static int rotary_encoder_parse_dt(struct device *dev,
+				   struct rotary_encoder *encoder)
 {
 	const struct of_device_id *of_id =
 				of_match_device(rotary_encoder_of_match, dev);
 	struct device_node *np = dev->of_node;
-	struct rotary_encoder_platform_data *pdata;
 	enum of_gpio_flags flags;
 	int error;
 
 	if (!of_id || !np)
-		return NULL;
-
-	pdata = devm_kzalloc(dev, sizeof(struct rotary_encoder_platform_data),
-			     GFP_KERNEL);
-	if (!pdata)
-		return ERR_PTR(-ENOMEM);
+		return 1;
 
-	of_property_read_u32(np, "rotary-encoder,steps", &pdata->steps);
-	of_property_read_u32(np, "linux,axis", &pdata->axis);
+	of_property_read_u32(np, "rotary-encoder,steps", &encoder->steps);
+	of_property_read_u32(np, "linux,axis", &encoder->axis);
 
-	pdata->gpio_a = of_get_gpio_flags(np, 0, &flags);
-	pdata->inverted_a = flags & OF_GPIO_ACTIVE_LOW;
+	encoder->gpio_a = of_get_gpio_flags(np, 0, &flags);
+	encoder->inverted_a = flags & OF_GPIO_ACTIVE_LOW;
 
-	pdata->gpio_b = of_get_gpio_flags(np, 1, &flags);
-	pdata->inverted_b = flags & OF_GPIO_ACTIVE_LOW;
+	encoder->gpio_b = of_get_gpio_flags(np, 1, &flags);
+	encoder->inverted_b = flags & OF_GPIO_ACTIVE_LOW;
 
-	pdata->relative_axis =
+	encoder->relative_axis =
 		of_property_read_bool(np, "rotary-encoder,relative-axis");
-	pdata->rollover = of_property_read_bool(np, "rotary-encoder,rollover");
+	encoder->rollover =
+		of_property_read_bool(np, "rotary-encoder,rollover");
 
 	error = of_property_read_u32(np, "rotary-encoder,steps-per-period",
-				     &pdata->steps_per_period);
+				     &encoder->steps_per_period);
 	if (error) {
 		/*
 		 * The 'half-period' property has been deprecated, you must use
@@ -238,45 +241,57 @@ static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct devic
 		 * need to parse it to maintain compatibility.
 		 */
 		if (of_property_read_bool(np, "rotary-encoder,half-period")) {
-			pdata->steps_per_period = 2;
+			encoder->steps_per_period = 2;
 		} else {
 			/* Fallback to one step per period behavior */
-			pdata->steps_per_period = 1;
+			encoder->steps_per_period = 1;
 		}
 	}
 
-	pdata->wakeup_source = of_property_read_bool(np, "wakeup-source");
+	encoder->wakeup_source = of_property_read_bool(np, "wakeup-source");
 
-	return pdata;
+	return 0;
 }
 #else
-static inline struct rotary_encoder_platform_data *
-rotary_encoder_parse_dt(struct device *dev)
+static inline int rotary_encoder_parse_dt(struct device *dev,
+					  struct rotary_encoder *encoder)
 {
-	return NULL;
+	return 1;
 }
 #endif
 
+static int rotary_encoder_parse_pdata(struct device *dev,
+				      struct rotary_encoder *encoder)
+{
+	const struct rotary_encoder_platform_data *pdata;
+
+	pdata = dev_get_platdata(dev);
+	if (!pdata) {
+		dev_err(dev, "missing platform data\n");
+		return -EINVAL;
+	}
+
+	encoder->steps = pdata->steps;
+	encoder->axis = pdata->axis;
+	encoder->gpio_a = pdata->gpio_a;
+	encoder->gpio_b = pdata->gpio_b;
+	encoder->inverted_a = pdata->inverted_a;
+	encoder->inverted_b = pdata->inverted_b;
+	encoder->steps_per_period = pdata->steps_per_period;
+	encoder->relative_axis = pdata->relative_axis;
+	encoder->rollover = pdata->rollover;
+
+	return 0;
+}
+
 static int rotary_encoder_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
-	const struct rotary_encoder_platform_data *pdata = dev_get_platdata(dev);
 	struct rotary_encoder *encoder;
 	struct input_dev *input;
 	irq_handler_t handler;
 	int err;
 
-	if (!pdata) {
-		pdata = rotary_encoder_parse_dt(dev);
-		if (IS_ERR(pdata))
-			return PTR_ERR(pdata);
-
-		if (!pdata) {
-			dev_err(dev, "missing platform data\n");
-			return -EINVAL;
-		}
-	}
-
 	encoder = devm_kzalloc(dev, sizeof(struct rotary_encoder), GFP_KERNEL);
 	input = devm_input_allocate_device(&pdev->dev);
 	if (!encoder || !input) {
@@ -284,55 +299,62 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 		return -ENOMEM;
 	}
 
+	err = rotary_encoder_parse_dt(dev, encoder);
+	if (err > 0)
+		/* not instatiated by dt */
+		err = rotary_encoder_parse_pdata(dev, encoder);
+
+	if (err < 0)
+		return err;
+
 	encoder->input = input;
-	encoder->pdata = pdata;
 
 	input->name = pdev->name;
 	input->id.bustype = BUS_HOST;
 	input->dev.parent = dev;
 
-	if (pdata->relative_axis) {
+	if (encoder->relative_axis) {
 		input->evbit[0] = BIT_MASK(EV_REL);
-		input->relbit[0] = BIT_MASK(pdata->axis);
+		input->relbit[0] = BIT_MASK(encoder->axis);
 	} else {
 		input->evbit[0] = BIT_MASK(EV_ABS);
 		input_set_abs_params(encoder->input,
-				     pdata->axis, 0, pdata->steps, 0, 1);
+				     encoder->axis, 0, encoder->steps, 0, 1);
 	}
 
 	/* request the GPIOs */
-	err = devm_gpio_request_one(dev, pdata->gpio_a,
+	err = devm_gpio_request_one(dev, encoder->gpio_a,
 				    GPIOF_IN, dev_name(dev));
 	if (err) {
-		dev_err(dev, "unable to request GPIO %d\n", pdata->gpio_a);
+		dev_err(dev, "unable to request GPIO %d\n", encoder->gpio_a);
 		return err;
 	}
 
-	err = devm_gpio_request_one(dev, pdata->gpio_b,
+	err = devm_gpio_request_one(dev, encoder->gpio_b,
 				    GPIOF_IN, dev_name(dev));
 	if (err) {
-		dev_err(dev, "unable to request GPIO %d\n", pdata->gpio_b);
+		dev_err(dev, "unable to request GPIO %d\n", encoder->gpio_b);
 		return err;
 	}
 
-	encoder->irq_a = gpio_to_irq(pdata->gpio_a);
-	encoder->irq_b = gpio_to_irq(pdata->gpio_b);
+	encoder->irq_a = gpio_to_irq(encoder->gpio_a);
+	encoder->irq_b = gpio_to_irq(encoder->gpio_b);
 
-	switch (pdata->steps_per_period) {
+	switch (encoder->steps_per_period) {
 	case 4:
 		handler = &rotary_encoder_quarter_period_irq;
-		encoder->last_stable = rotary_encoder_get_state(pdata);
+		encoder->last_stable = rotary_encoder_get_state(encoder);
 		break;
 	case 2:
 		handler = &rotary_encoder_half_period_irq;
-		encoder->last_stable = rotary_encoder_get_state(pdata);
+		encoder->last_stable = rotary_encoder_get_state(encoder);
 		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);
+			encoder->steps_per_period);
 		return -EINVAL;
 	}
 
@@ -358,7 +380,7 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 		return err;
 	}
 
-	device_init_wakeup(&pdev->dev, pdata->wakeup_source);
+	device_init_wakeup(&pdev->dev, encoder->wakeup_source);
 
 	platform_set_drvdata(pdev, encoder);
 
-- 
2.7.0.rc3

Re: [PATCH v2 2/3] Input: rotary-encoder - move configuration data to driver data

[Daniel Mack]

On 02/02/2016 11:24 AM, Uwe Kleine-König wrote:
> This is a preparation for the next patche. There is no change in
> behaviour intended.

This one looks good to me, but it clashes with Dmitry's latest patch
sets (which I haven't found the time to test yet, sorry!).

> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

 Acked-by: Daniel Mack <daniel@zonque.org>




Thanks,
Daniel


> ---
>  drivers/input/misc/rotary_encoder.c | 166 ++++++++++++++++++++----------------
>  1 file changed, 94 insertions(+), 72 deletions(-)
> 
> diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
> index 386bdb5314e6..0582e851993f 100644
> --- a/drivers/input/misc/rotary_encoder.c
> +++ b/drivers/input/misc/rotary_encoder.c
> @@ -32,58 +32,65 @@
>  
>  struct rotary_encoder {
>  	struct input_dev *input;
> -	const struct rotary_encoder_platform_data *pdata;
>  
> +	/* configuration */
> +	unsigned int steps;
>  	unsigned int axis;
> -	unsigned int pos;
> +	unsigned int gpio_a;
> +	unsigned int gpio_b;
> +	unsigned int inverted_a;
> +	unsigned int inverted_b;
> +	unsigned int steps_per_period;
> +	bool relative_axis;
> +	bool rollover;
> +	bool wakeup_source;
>  
>  	unsigned int irq_a;
>  	unsigned int irq_b;
>  
> +	/* state */
> +	unsigned int pos;
>  	bool armed;
>  	unsigned char dir;	/* 0 - clockwise, 1 - CCW */
> -
>  	char last_stable;
>  };
>  
> -static int rotary_encoder_get_state(const struct rotary_encoder_platform_data *pdata)
> +static int rotary_encoder_get_state(const struct rotary_encoder *encoder)
>  {
> -	int a = !!gpio_get_value(pdata->gpio_a);
> -	int b = !!gpio_get_value(pdata->gpio_b);
> +	int a = !!gpio_get_value(encoder->gpio_a);
> +	int b = !!gpio_get_value(encoder->gpio_b);
>  
> -	a ^= pdata->inverted_a;
> -	b ^= pdata->inverted_b;
> +	a ^= encoder->inverted_a;
> +	b ^= encoder->inverted_b;
>  
>  	return ((a << 1) | b);
>  }
>  
>  static void rotary_encoder_report_event(struct rotary_encoder *encoder)
>  {
> -	const struct rotary_encoder_platform_data *pdata = encoder->pdata;
> -
> -	if (pdata->relative_axis) {
> +	if (encoder->relative_axis) {
>  		input_report_rel(encoder->input,
> -				 pdata->axis, encoder->dir ? -1 : 1);
> +				 encoder->axis, encoder->dir ? -1 : 1);
>  	} else {
>  		unsigned int pos = encoder->pos;
>  
>  		if (encoder->dir) {
>  			/* turning counter-clockwise */
> -			if (pdata->rollover)
> -				pos += pdata->steps;
> +			if (encoder->rollover)
> +				pos += encoder->steps;
>  			if (pos)
>  				pos--;
>  		} else {
>  			/* turning clockwise */
> -			if (pdata->rollover || pos < pdata->steps)
> +			if (encoder->rollover || pos < encoder->steps)
>  				pos++;
>  		}
>  
> -		if (pdata->rollover)
> -			pos %= pdata->steps;
> +		if (encoder->rollover)
> +			pos %= encoder->steps;
>  
>  		encoder->pos = pos;
> -		input_report_abs(encoder->input, pdata->axis, encoder->pos);
> +		input_report_abs(encoder->input, encoder->axis, encoder->pos);
>  	}
>  
>  	input_sync(encoder->input);
> @@ -94,7 +101,7 @@ static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
>  	struct rotary_encoder *encoder = dev_id;
>  	int state;
>  
> -	state = rotary_encoder_get_state(encoder->pdata);
> +	state = rotary_encoder_get_state(encoder);
>  
>  	switch (state) {
>  	case 0x0:
> @@ -123,7 +130,7 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id)
>  	struct rotary_encoder *encoder = dev_id;
>  	int state;
>  
> -	state = rotary_encoder_get_state(encoder->pdata);
> +	state = rotary_encoder_get_state(encoder);
>  
>  	switch (state) {
>  	case 0x00:
> @@ -149,7 +156,7 @@ static irqreturn_t rotary_encoder_quarter_period_irq(int irq, void *dev_id)
>  	unsigned char sum;
>  	int state;
>  
> -	state = rotary_encoder_get_state(encoder->pdata);
> +	state = rotary_encoder_get_state(encoder);
>  
>  	/*
>  	 * We encode the previous and the current state using a byte.
> @@ -199,38 +206,34 @@ static const struct of_device_id rotary_encoder_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, rotary_encoder_of_match);
>  
> -static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct device *dev)
> +static int rotary_encoder_parse_dt(struct device *dev,
> +				   struct rotary_encoder *encoder)
>  {
>  	const struct of_device_id *of_id =
>  				of_match_device(rotary_encoder_of_match, dev);
>  	struct device_node *np = dev->of_node;
> -	struct rotary_encoder_platform_data *pdata;
>  	enum of_gpio_flags flags;
>  	int error;
>  
>  	if (!of_id || !np)
> -		return NULL;
> -
> -	pdata = devm_kzalloc(dev, sizeof(struct rotary_encoder_platform_data),
> -			     GFP_KERNEL);
> -	if (!pdata)
> -		return ERR_PTR(-ENOMEM);
> +		return 1;
>  
> -	of_property_read_u32(np, "rotary-encoder,steps", &pdata->steps);
> -	of_property_read_u32(np, "linux,axis", &pdata->axis);
> +	of_property_read_u32(np, "rotary-encoder,steps", &encoder->steps);
> +	of_property_read_u32(np, "linux,axis", &encoder->axis);
>  
> -	pdata->gpio_a = of_get_gpio_flags(np, 0, &flags);
> -	pdata->inverted_a = flags & OF_GPIO_ACTIVE_LOW;
> +	encoder->gpio_a = of_get_gpio_flags(np, 0, &flags);
> +	encoder->inverted_a = flags & OF_GPIO_ACTIVE_LOW;
>  
> -	pdata->gpio_b = of_get_gpio_flags(np, 1, &flags);
> -	pdata->inverted_b = flags & OF_GPIO_ACTIVE_LOW;
> +	encoder->gpio_b = of_get_gpio_flags(np, 1, &flags);
> +	encoder->inverted_b = flags & OF_GPIO_ACTIVE_LOW;
>  
> -	pdata->relative_axis =
> +	encoder->relative_axis =
>  		of_property_read_bool(np, "rotary-encoder,relative-axis");
> -	pdata->rollover = of_property_read_bool(np, "rotary-encoder,rollover");
> +	encoder->rollover =
> +		of_property_read_bool(np, "rotary-encoder,rollover");
>  
>  	error = of_property_read_u32(np, "rotary-encoder,steps-per-period",
> -				     &pdata->steps_per_period);
> +				     &encoder->steps_per_period);
>  	if (error) {
>  		/*
>  		 * The 'half-period' property has been deprecated, you must use
> @@ -238,45 +241,57 @@ static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct devic
>  		 * need to parse it to maintain compatibility.
>  		 */
>  		if (of_property_read_bool(np, "rotary-encoder,half-period")) {
> -			pdata->steps_per_period = 2;
> +			encoder->steps_per_period = 2;
>  		} else {
>  			/* Fallback to one step per period behavior */
> -			pdata->steps_per_period = 1;
> +			encoder->steps_per_period = 1;
>  		}
>  	}
>  
> -	pdata->wakeup_source = of_property_read_bool(np, "wakeup-source");
> +	encoder->wakeup_source = of_property_read_bool(np, "wakeup-source");
>  
> -	return pdata;
> +	return 0;
>  }
>  #else
> -static inline struct rotary_encoder_platform_data *
> -rotary_encoder_parse_dt(struct device *dev)
> +static inline int rotary_encoder_parse_dt(struct device *dev,
> +					  struct rotary_encoder *encoder)
>  {
> -	return NULL;
> +	return 1;
>  }
>  #endif
>  
> +static int rotary_encoder_parse_pdata(struct device *dev,
> +				      struct rotary_encoder *encoder)
> +{
> +	const struct rotary_encoder_platform_data *pdata;
> +
> +	pdata = dev_get_platdata(dev);
> +	if (!pdata) {
> +		dev_err(dev, "missing platform data\n");
> +		return -EINVAL;
> +	}
> +
> +	encoder->steps = pdata->steps;
> +	encoder->axis = pdata->axis;
> +	encoder->gpio_a = pdata->gpio_a;
> +	encoder->gpio_b = pdata->gpio_b;
> +	encoder->inverted_a = pdata->inverted_a;
> +	encoder->inverted_b = pdata->inverted_b;
> +	encoder->steps_per_period = pdata->steps_per_period;
> +	encoder->relative_axis = pdata->relative_axis;
> +	encoder->rollover = pdata->rollover;
> +
> +	return 0;
> +}
> +
>  static int rotary_encoder_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> -	const struct rotary_encoder_platform_data *pdata = dev_get_platdata(dev);
>  	struct rotary_encoder *encoder;
>  	struct input_dev *input;
>  	irq_handler_t handler;
>  	int err;
>  
> -	if (!pdata) {
> -		pdata = rotary_encoder_parse_dt(dev);
> -		if (IS_ERR(pdata))
> -			return PTR_ERR(pdata);
> -
> -		if (!pdata) {
> -			dev_err(dev, "missing platform data\n");
> -			return -EINVAL;
> -		}
> -	}
> -
>  	encoder = devm_kzalloc(dev, sizeof(struct rotary_encoder), GFP_KERNEL);
>  	input = devm_input_allocate_device(&pdev->dev);
>  	if (!encoder || !input) {
> @@ -284,55 +299,62 @@ static int rotary_encoder_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	}
>  
> +	err = rotary_encoder_parse_dt(dev, encoder);
> +	if (err > 0)
> +		/* not instatiated by dt */
> +		err = rotary_encoder_parse_pdata(dev, encoder);
> +
> +	if (err < 0)
> +		return err;
> +
>  	encoder->input = input;
> -	encoder->pdata = pdata;
>  
>  	input->name = pdev->name;
>  	input->id.bustype = BUS_HOST;
>  	input->dev.parent = dev;
>  
> -	if (pdata->relative_axis) {
> +	if (encoder->relative_axis) {
>  		input->evbit[0] = BIT_MASK(EV_REL);
> -		input->relbit[0] = BIT_MASK(pdata->axis);
> +		input->relbit[0] = BIT_MASK(encoder->axis);
>  	} else {
>  		input->evbit[0] = BIT_MASK(EV_ABS);
>  		input_set_abs_params(encoder->input,
> -				     pdata->axis, 0, pdata->steps, 0, 1);
> +				     encoder->axis, 0, encoder->steps, 0, 1);
>  	}
>  
>  	/* request the GPIOs */
> -	err = devm_gpio_request_one(dev, pdata->gpio_a,
> +	err = devm_gpio_request_one(dev, encoder->gpio_a,
>  				    GPIOF_IN, dev_name(dev));
>  	if (err) {
> -		dev_err(dev, "unable to request GPIO %d\n", pdata->gpio_a);
> +		dev_err(dev, "unable to request GPIO %d\n", encoder->gpio_a);
>  		return err;
>  	}
>  
> -	err = devm_gpio_request_one(dev, pdata->gpio_b,
> +	err = devm_gpio_request_one(dev, encoder->gpio_b,
>  				    GPIOF_IN, dev_name(dev));
>  	if (err) {
> -		dev_err(dev, "unable to request GPIO %d\n", pdata->gpio_b);
> +		dev_err(dev, "unable to request GPIO %d\n", encoder->gpio_b);
>  		return err;
>  	}
>  
> -	encoder->irq_a = gpio_to_irq(pdata->gpio_a);
> -	encoder->irq_b = gpio_to_irq(pdata->gpio_b);
> +	encoder->irq_a = gpio_to_irq(encoder->gpio_a);
> +	encoder->irq_b = gpio_to_irq(encoder->gpio_b);
>  
> -	switch (pdata->steps_per_period) {
> +	switch (encoder->steps_per_period) {
>  	case 4:
>  		handler = &rotary_encoder_quarter_period_irq;
> -		encoder->last_stable = rotary_encoder_get_state(pdata);
> +		encoder->last_stable = rotary_encoder_get_state(encoder);
>  		break;
>  	case 2:
>  		handler = &rotary_encoder_half_period_irq;
> -		encoder->last_stable = rotary_encoder_get_state(pdata);
> +		encoder->last_stable = rotary_encoder_get_state(encoder);
>  		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);
> +			encoder->steps_per_period);
>  		return -EINVAL;
>  	}
>  
> @@ -358,7 +380,7 @@ static int rotary_encoder_probe(struct platform_device *pdev)
>  		return err;
>  	}
>  
> -	device_init_wakeup(&pdev->dev, pdata->wakeup_source);
> +	device_init_wakeup(&pdev->dev, encoder->wakeup_source);
>  
>  	platform_set_drvdata(pdev, encoder);
>  
> 

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

[PATCH v2 2/3] Input: rotary-encoder - move configuration data to driver data

[Daniel Mack]

On 02/02/2016 11:24 AM, Uwe Kleine-K?nig wrote:
> This is a preparation for the next patche. There is no change in
> behaviour intended.

This one looks good to me, but it clashes with Dmitry's latest patch
sets (which I haven't found the time to test yet, sorry!).

> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>

 Acked-by: Daniel Mack <daniel@zonque.org>




Thanks,
Daniel


> ---
>  drivers/input/misc/rotary_encoder.c | 166 ++++++++++++++++++++----------------
>  1 file changed, 94 insertions(+), 72 deletions(-)
> 
> diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
> index 386bdb5314e6..0582e851993f 100644
> --- a/drivers/input/misc/rotary_encoder.c
> +++ b/drivers/input/misc/rotary_encoder.c
> @@ -32,58 +32,65 @@
>  
>  struct rotary_encoder {
>  	struct input_dev *input;
> -	const struct rotary_encoder_platform_data *pdata;
>  
> +	/* configuration */
> +	unsigned int steps;
>  	unsigned int axis;
> -	unsigned int pos;
> +	unsigned int gpio_a;
> +	unsigned int gpio_b;
> +	unsigned int inverted_a;
> +	unsigned int inverted_b;
> +	unsigned int steps_per_period;
> +	bool relative_axis;
> +	bool rollover;
> +	bool wakeup_source;
>  
>  	unsigned int irq_a;
>  	unsigned int irq_b;
>  
> +	/* state */
> +	unsigned int pos;
>  	bool armed;
>  	unsigned char dir;	/* 0 - clockwise, 1 - CCW */
> -
>  	char last_stable;
>  };
>  
> -static int rotary_encoder_get_state(const struct rotary_encoder_platform_data *pdata)
> +static int rotary_encoder_get_state(const struct rotary_encoder *encoder)
>  {
> -	int a = !!gpio_get_value(pdata->gpio_a);
> -	int b = !!gpio_get_value(pdata->gpio_b);
> +	int a = !!gpio_get_value(encoder->gpio_a);
> +	int b = !!gpio_get_value(encoder->gpio_b);
>  
> -	a ^= pdata->inverted_a;
> -	b ^= pdata->inverted_b;
> +	a ^= encoder->inverted_a;
> +	b ^= encoder->inverted_b;
>  
>  	return ((a << 1) | b);
>  }
>  
>  static void rotary_encoder_report_event(struct rotary_encoder *encoder)
>  {
> -	const struct rotary_encoder_platform_data *pdata = encoder->pdata;
> -
> -	if (pdata->relative_axis) {
> +	if (encoder->relative_axis) {
>  		input_report_rel(encoder->input,
> -				 pdata->axis, encoder->dir ? -1 : 1);
> +				 encoder->axis, encoder->dir ? -1 : 1);
>  	} else {
>  		unsigned int pos = encoder->pos;
>  
>  		if (encoder->dir) {
>  			/* turning counter-clockwise */
> -			if (pdata->rollover)
> -				pos += pdata->steps;
> +			if (encoder->rollover)
> +				pos += encoder->steps;
>  			if (pos)
>  				pos--;
>  		} else {
>  			/* turning clockwise */
> -			if (pdata->rollover || pos < pdata->steps)
> +			if (encoder->rollover || pos < encoder->steps)
>  				pos++;
>  		}
>  
> -		if (pdata->rollover)
> -			pos %= pdata->steps;
> +		if (encoder->rollover)
> +			pos %= encoder->steps;
>  
>  		encoder->pos = pos;
> -		input_report_abs(encoder->input, pdata->axis, encoder->pos);
> +		input_report_abs(encoder->input, encoder->axis, encoder->pos);
>  	}
>  
>  	input_sync(encoder->input);
> @@ -94,7 +101,7 @@ static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
>  	struct rotary_encoder *encoder = dev_id;
>  	int state;
>  
> -	state = rotary_encoder_get_state(encoder->pdata);
> +	state = rotary_encoder_get_state(encoder);
>  
>  	switch (state) {
>  	case 0x0:
> @@ -123,7 +130,7 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id)
>  	struct rotary_encoder *encoder = dev_id;
>  	int state;
>  
> -	state = rotary_encoder_get_state(encoder->pdata);
> +	state = rotary_encoder_get_state(encoder);
>  
>  	switch (state) {
>  	case 0x00:
> @@ -149,7 +156,7 @@ static irqreturn_t rotary_encoder_quarter_period_irq(int irq, void *dev_id)
>  	unsigned char sum;
>  	int state;
>  
> -	state = rotary_encoder_get_state(encoder->pdata);
> +	state = rotary_encoder_get_state(encoder);
>  
>  	/*
>  	 * We encode the previous and the current state using a byte.
> @@ -199,38 +206,34 @@ static const struct of_device_id rotary_encoder_of_match[] = {
>  };
>  MODULE_DEVICE_TABLE(of, rotary_encoder_of_match);
>  
> -static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct device *dev)
> +static int rotary_encoder_parse_dt(struct device *dev,
> +				   struct rotary_encoder *encoder)
>  {
>  	const struct of_device_id *of_id =
>  				of_match_device(rotary_encoder_of_match, dev);
>  	struct device_node *np = dev->of_node;
> -	struct rotary_encoder_platform_data *pdata;
>  	enum of_gpio_flags flags;
>  	int error;
>  
>  	if (!of_id || !np)
> -		return NULL;
> -
> -	pdata = devm_kzalloc(dev, sizeof(struct rotary_encoder_platform_data),
> -			     GFP_KERNEL);
> -	if (!pdata)
> -		return ERR_PTR(-ENOMEM);
> +		return 1;
>  
> -	of_property_read_u32(np, "rotary-encoder,steps", &pdata->steps);
> -	of_property_read_u32(np, "linux,axis", &pdata->axis);
> +	of_property_read_u32(np, "rotary-encoder,steps", &encoder->steps);
> +	of_property_read_u32(np, "linux,axis", &encoder->axis);
>  
> -	pdata->gpio_a = of_get_gpio_flags(np, 0, &flags);
> -	pdata->inverted_a = flags & OF_GPIO_ACTIVE_LOW;
> +	encoder->gpio_a = of_get_gpio_flags(np, 0, &flags);
> +	encoder->inverted_a = flags & OF_GPIO_ACTIVE_LOW;
>  
> -	pdata->gpio_b = of_get_gpio_flags(np, 1, &flags);
> -	pdata->inverted_b = flags & OF_GPIO_ACTIVE_LOW;
> +	encoder->gpio_b = of_get_gpio_flags(np, 1, &flags);
> +	encoder->inverted_b = flags & OF_GPIO_ACTIVE_LOW;
>  
> -	pdata->relative_axis =
> +	encoder->relative_axis =
>  		of_property_read_bool(np, "rotary-encoder,relative-axis");
> -	pdata->rollover = of_property_read_bool(np, "rotary-encoder,rollover");
> +	encoder->rollover =
> +		of_property_read_bool(np, "rotary-encoder,rollover");
>  
>  	error = of_property_read_u32(np, "rotary-encoder,steps-per-period",
> -				     &pdata->steps_per_period);
> +				     &encoder->steps_per_period);
>  	if (error) {
>  		/*
>  		 * The 'half-period' property has been deprecated, you must use
> @@ -238,45 +241,57 @@ static struct rotary_encoder_platform_data *rotary_encoder_parse_dt(struct devic
>  		 * need to parse it to maintain compatibility.
>  		 */
>  		if (of_property_read_bool(np, "rotary-encoder,half-period")) {
> -			pdata->steps_per_period = 2;
> +			encoder->steps_per_period = 2;
>  		} else {
>  			/* Fallback to one step per period behavior */
> -			pdata->steps_per_period = 1;
> +			encoder->steps_per_period = 1;
>  		}
>  	}
>  
> -	pdata->wakeup_source = of_property_read_bool(np, "wakeup-source");
> +	encoder->wakeup_source = of_property_read_bool(np, "wakeup-source");
>  
> -	return pdata;
> +	return 0;
>  }
>  #else
> -static inline struct rotary_encoder_platform_data *
> -rotary_encoder_parse_dt(struct device *dev)
> +static inline int rotary_encoder_parse_dt(struct device *dev,
> +					  struct rotary_encoder *encoder)
>  {
> -	return NULL;
> +	return 1;
>  }
>  #endif
>  
> +static int rotary_encoder_parse_pdata(struct device *dev,
> +				      struct rotary_encoder *encoder)
> +{
> +	const struct rotary_encoder_platform_data *pdata;
> +
> +	pdata = dev_get_platdata(dev);
> +	if (!pdata) {
> +		dev_err(dev, "missing platform data\n");
> +		return -EINVAL;
> +	}
> +
> +	encoder->steps = pdata->steps;
> +	encoder->axis = pdata->axis;
> +	encoder->gpio_a = pdata->gpio_a;
> +	encoder->gpio_b = pdata->gpio_b;
> +	encoder->inverted_a = pdata->inverted_a;
> +	encoder->inverted_b = pdata->inverted_b;
> +	encoder->steps_per_period = pdata->steps_per_period;
> +	encoder->relative_axis = pdata->relative_axis;
> +	encoder->rollover = pdata->rollover;
> +
> +	return 0;
> +}
> +
>  static int rotary_encoder_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> -	const struct rotary_encoder_platform_data *pdata = dev_get_platdata(dev);
>  	struct rotary_encoder *encoder;
>  	struct input_dev *input;
>  	irq_handler_t handler;
>  	int err;
>  
> -	if (!pdata) {
> -		pdata = rotary_encoder_parse_dt(dev);
> -		if (IS_ERR(pdata))
> -			return PTR_ERR(pdata);
> -
> -		if (!pdata) {
> -			dev_err(dev, "missing platform data\n");
> -			return -EINVAL;
> -		}
> -	}
> -
>  	encoder = devm_kzalloc(dev, sizeof(struct rotary_encoder), GFP_KERNEL);
>  	input = devm_input_allocate_device(&pdev->dev);
>  	if (!encoder || !input) {
> @@ -284,55 +299,62 @@ static int rotary_encoder_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  	}
>  
> +	err = rotary_encoder_parse_dt(dev, encoder);
> +	if (err > 0)
> +		/* not instatiated by dt */
> +		err = rotary_encoder_parse_pdata(dev, encoder);
> +
> +	if (err < 0)
> +		return err;
> +
>  	encoder->input = input;
> -	encoder->pdata = pdata;
>  
>  	input->name = pdev->name;
>  	input->id.bustype = BUS_HOST;
>  	input->dev.parent = dev;
>  
> -	if (pdata->relative_axis) {
> +	if (encoder->relative_axis) {
>  		input->evbit[0] = BIT_MASK(EV_REL);
> -		input->relbit[0] = BIT_MASK(pdata->axis);
> +		input->relbit[0] = BIT_MASK(encoder->axis);
>  	} else {
>  		input->evbit[0] = BIT_MASK(EV_ABS);
>  		input_set_abs_params(encoder->input,
> -				     pdata->axis, 0, pdata->steps, 0, 1);
> +				     encoder->axis, 0, encoder->steps, 0, 1);
>  	}
>  
>  	/* request the GPIOs */
> -	err = devm_gpio_request_one(dev, pdata->gpio_a,
> +	err = devm_gpio_request_one(dev, encoder->gpio_a,
>  				    GPIOF_IN, dev_name(dev));
>  	if (err) {
> -		dev_err(dev, "unable to request GPIO %d\n", pdata->gpio_a);
> +		dev_err(dev, "unable to request GPIO %d\n", encoder->gpio_a);
>  		return err;
>  	}
>  
> -	err = devm_gpio_request_one(dev, pdata->gpio_b,
> +	err = devm_gpio_request_one(dev, encoder->gpio_b,
>  				    GPIOF_IN, dev_name(dev));
>  	if (err) {
> -		dev_err(dev, "unable to request GPIO %d\n", pdata->gpio_b);
> +		dev_err(dev, "unable to request GPIO %d\n", encoder->gpio_b);
>  		return err;
>  	}
>  
> -	encoder->irq_a = gpio_to_irq(pdata->gpio_a);
> -	encoder->irq_b = gpio_to_irq(pdata->gpio_b);
> +	encoder->irq_a = gpio_to_irq(encoder->gpio_a);
> +	encoder->irq_b = gpio_to_irq(encoder->gpio_b);
>  
> -	switch (pdata->steps_per_period) {
> +	switch (encoder->steps_per_period) {
>  	case 4:
>  		handler = &rotary_encoder_quarter_period_irq;
> -		encoder->last_stable = rotary_encoder_get_state(pdata);
> +		encoder->last_stable = rotary_encoder_get_state(encoder);
>  		break;
>  	case 2:
>  		handler = &rotary_encoder_half_period_irq;
> -		encoder->last_stable = rotary_encoder_get_state(pdata);
> +		encoder->last_stable = rotary_encoder_get_state(encoder);
>  		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);
> +			encoder->steps_per_period);
>  		return -EINVAL;
>  	}
>  
> @@ -358,7 +380,7 @@ static int rotary_encoder_probe(struct platform_device *pdev)
>  		return err;
>  	}
>  
> -	device_init_wakeup(&pdev->dev, pdata->wakeup_source);
> +	device_init_wakeup(&pdev->dev, encoder->wakeup_source);
>  
>  	platform_set_drvdata(pdev, encoder);
>  
> 

Re: [PATCH v2 2/3] Input: rotary-encoder - move configuration data to driver data

[Uwe Kleine-König]

Hello,

On Tue, Feb 02, 2016 at 01:10:19PM +0100, Daniel Mack wrote:
> On 02/02/2016 11:24 AM, Uwe Kleine-König wrote:
> > This is a preparation for the next patche. There is no change in
> > behaviour intended.
> 
> This one looks good to me, but it clashes with Dmitry's latest patch
> sets (which I haven't found the time to test yet, sorry!).

There is nothing in next-20160203, so I assume Dmitry either fixes this
up when applying my patches or tells me when I should rebase.

> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> 
>  Acked-by: Daniel Mack <daniel-cYrQPVfZoowdnm+yROfE0A@public.gmane.org>

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
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 v2 2/3] Input: rotary-encoder - move configuration data to driver data

[Uwe Kleine-König]

Hello,

On Tue, Feb 02, 2016 at 01:10:19PM +0100, Daniel Mack wrote:
> On 02/02/2016 11:24 AM, Uwe Kleine-K?nig wrote:
> > This is a preparation for the next patche. There is no change in
> > behaviour intended.
> 
> This one looks good to me, but it clashes with Dmitry's latest patch
> sets (which I haven't found the time to test yet, sorry!).

There is nothing in next-20160203, so I assume Dmitry either fixes this
up when applying my patches or tells me when I should rebase.

> > Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> 
>  Acked-by: Daniel Mack <daniel@zonque.org>

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH v2 2/3] Input: rotary-encoder - move configuration data to driver data

[Dmitry Torokhov]

On Wed, Feb 03, 2016 at 10:35:27AM +0100, Uwe Kleine-König wrote:
> Hello,
> 
> On Tue, Feb 02, 2016 at 01:10:19PM +0100, Daniel Mack wrote:
> > On 02/02/2016 11:24 AM, Uwe Kleine-König wrote:
> > > This is a preparation for the next patche. There is no change in
> > > behaviour intended.
> > 
> > This one looks good to me, but it clashes with Dmitry's latest patch
> > sets (which I haven't found the time to test yet, sorry!).
> 
> There is nothing in next-20160203, so I assume Dmitry either fixes this
> up when applying my patches or tells me when I should rebase.

Please take a look at my rotary-encoder branch. I am waiting to merge it
into next until I can get some testers. If you have raumfild device that
would be awesome.

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

[PATCH v2 2/3] Input: rotary-encoder - move configuration data to driver data

[Dmitry Torokhov]

On Wed, Feb 03, 2016 at 10:35:27AM +0100, Uwe Kleine-K?nig wrote:
> Hello,
> 
> On Tue, Feb 02, 2016 at 01:10:19PM +0100, Daniel Mack wrote:
> > On 02/02/2016 11:24 AM, Uwe Kleine-K?nig wrote:
> > > This is a preparation for the next patche. There is no change in
> > > behaviour intended.
> > 
> > This one looks good to me, but it clashes with Dmitry's latest patch
> > sets (which I haven't found the time to test yet, sorry!).
> 
> There is nothing in next-20160203, so I assume Dmitry either fixes this
> up when applying my patches or tells me when I should rebase.

Please take a look at my rotary-encoder branch. I am waiting to merge it
into next until I can get some testers. If you have raumfild device that
would be awesome.

Thanks.

-- 
Dmitry

Re: [PATCH v2 2/3] Input: rotary-encoder - move configuration data to driver data

[Uwe Kleine-König]

Hello Dmitry,

On Thu, Feb 04, 2016 at 01:18:42PM -0800, Dmitry Torokhov wrote:
> On Wed, Feb 03, 2016 at 10:35:27AM +0100, Uwe Kleine-König wrote:
> > Hello,
> > 
> > On Tue, Feb 02, 2016 at 01:10:19PM +0100, Daniel Mack wrote:
> > > On 02/02/2016 11:24 AM, Uwe Kleine-König wrote:
> > > > This is a preparation for the next patche. There is no change in
> > > > behaviour intended.
> > > 
> > > This one looks good to me, but it clashes with Dmitry's latest patch
> > > sets (which I haven't found the time to test yet, sorry!).
> > 
> > There is nothing in next-20160203, so I assume Dmitry either fixes this
> > up when applying my patches or tells me when I should rebase.
> 
> Please take a look at my rotary-encoder branch. I am waiting to merge it
> into next until I can get some testers. If you have raumfild device that
> would be awesome.

I don't have a raumfeld device. I did a similar conversion and the only
relevant difference is that I used

	.dev_id = "rotary_encoder.0",

while you used

	.dev_id = "rotary-encoder",

in raumfeld_rotary_gpios_table.
I think a mixture of these is the right thing, i.e.

	.dev_id = "rotary-encoder.0",

Your series obsoletes my first two patches, so I will port my third
patch on top of your branch and retest there.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
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

[PATCH v2 2/3] Input: rotary-encoder - move configuration data to driver data

[Uwe Kleine-König]

Hello Dmitry,

On Thu, Feb 04, 2016 at 01:18:42PM -0800, Dmitry Torokhov wrote:
> On Wed, Feb 03, 2016 at 10:35:27AM +0100, Uwe Kleine-K?nig wrote:
> > Hello,
> > 
> > On Tue, Feb 02, 2016 at 01:10:19PM +0100, Daniel Mack wrote:
> > > On 02/02/2016 11:24 AM, Uwe Kleine-K?nig wrote:
> > > > This is a preparation for the next patche. There is no change in
> > > > behaviour intended.
> > > 
> > > This one looks good to me, but it clashes with Dmitry's latest patch
> > > sets (which I haven't found the time to test yet, sorry!).
> > 
> > There is nothing in next-20160203, so I assume Dmitry either fixes this
> > up when applying my patches or tells me when I should rebase.
> 
> Please take a look at my rotary-encoder branch. I am waiting to merge it
> into next until I can get some testers. If you have raumfild device that
> would be awesome.

I don't have a raumfeld device. I did a similar conversion and the only
relevant difference is that I used

	.dev_id = "rotary_encoder.0",

while you used

	.dev_id = "rotary-encoder",

in raumfeld_rotary_gpios_table.
I think a mixture of these is the right thing, i.e.

	.dev_id = "rotary-encoder.0",

Your series obsoletes my first two patches, so I will port my third
patch on top of your branch and retest there.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

[PATCH v2 3/3] Input: rotary-encoder - support more than 2 gpios as input

[Uwe Kleine-König]

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 .../devicetree/bindings/input/rotary-encoder.txt   |   2 +-
 arch/arm/mach-pxa/raumfeld.c                       |  25 ++-
 drivers/input/misc/rotary_encoder.c                | 171 ++++++++-------------
 include/linux/rotary_encoder.h                     |   4 -
 4 files changed, 84 insertions(+), 118 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.txt b/Documentation/devicetree/bindings/input/rotary-encoder.txt
index de99cbbbf6da..6c9f0c8a846c 100644
--- a/Documentation/devicetree/bindings/input/rotary-encoder.txt
+++ b/Documentation/devicetree/bindings/input/rotary-encoder.txt
@@ -1,7 +1,7 @@
 Rotary encoder DT bindings
 
 Required properties:
-- gpios: a spec for two GPIOs to be used
+- gpios: a spec for at least two GPIOs to be used, most significant first
 
 Optional properties:
 - linux,axis: the input subsystem axis to map to this rotary encoder.
diff --git a/arch/arm/mach-pxa/raumfeld.c b/arch/arm/mach-pxa/raumfeld.c
index 8347d87a713d..87988d717465 100644
--- a/arch/arm/mach-pxa/raumfeld.c
+++ b/arch/arm/mach-pxa/raumfeld.c
@@ -21,6 +21,7 @@
 #include <linux/platform_device.h>
 #include <linux/interrupt.h>
 #include <linux/gpio.h>
+#include <linux/gpio/machine.h>
 #include <linux/smsc911x.h>
 #include <linux/input.h>
 #include <linux/rotary_encoder.h>
@@ -370,10 +371,25 @@ static struct rotary_encoder_platform_data raumfeld_rotary_encoder_info = {
 	.steps		= 24,
 	.axis		= REL_X,
 	.relative_axis	= 1,
-	.gpio_a		= GPIO_VOLENC_A,
-	.gpio_b		= GPIO_VOLENC_B,
-	.inverted_a	= 1,
-	.inverted_b	= 0,
+};
+
+static struct gpiod_lookup_table raumfeld_rotary_encoder_gpio_lookup = {
+	.dev_id = "rotary_encoder.0",
+	.table = {
+		{
+			.chip_label = "gpio-0",
+			.chip_hwnum = GPIO_VOLENC_A,
+			.idx = 0,
+			.flags = GPIO_ACTIVE_LOW,
+		},
+		{
+			.chip_label = "gpio-0",
+			.chip_hwnum = GPIO_VOLENC_B,
+			.idx = 1,
+			.flags = GPIO_ACTIVE_HIGH,
+		},
+		{ /* sentinel */ }
+	},
 };
 
 static struct platform_device rotary_encoder_device = {
@@ -1051,6 +1067,7 @@ static void __init __maybe_unused raumfeld_controller_init(void)
 	int ret;
 
 	pxa3xx_mfp_config(ARRAY_AND_SIZE(raumfeld_controller_pin_config));
+	gpiod_add_lookup_table(&raumfeld_rotary_encoder_gpio_lookup);
 	platform_device_register(&rotary_encoder_device);
 	spi_register_board_info(ARRAY_AND_SIZE(controller_spi_devices));
 	i2c_register_board_info(0, &raumfeld_controller_i2c_board_info, 1);
diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
index 0582e851993f..256fe232a26b 100644
--- a/drivers/input/misc/rotary_encoder.c
+++ b/drivers/input/misc/rotary_encoder.c
@@ -25,7 +25,6 @@
 #include <linux/slab.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
-#include <linux/of_gpio.h>
 #include <linux/pm.h>
 
 #define DRV_NAME "rotary-encoder"
@@ -36,45 +35,47 @@ struct rotary_encoder {
 	/* configuration */
 	unsigned int steps;
 	unsigned int axis;
-	unsigned int gpio_a;
-	unsigned int gpio_b;
-	unsigned int inverted_a;
-	unsigned int inverted_b;
 	unsigned int steps_per_period;
 	bool relative_axis;
 	bool rollover;
 	bool wakeup_source;
 
-	unsigned int irq_a;
-	unsigned int irq_b;
+	struct gpio_descs *gpios;
+	unsigned int *irq;
 
 	/* state */
 	unsigned int pos;
 	bool armed;
-	unsigned char dir;	/* 0 - clockwise, 1 - CCW */
+	signed char dir;	/* 1 - clockwise, -1 - CCW */
 	char last_stable;
 };
 
-static int rotary_encoder_get_state(const struct rotary_encoder *encoder)
+static unsigned rotary_encoder_get_state(const struct rotary_encoder *encoder)
 {
-	int a = !!gpio_get_value(encoder->gpio_a);
-	int b = !!gpio_get_value(encoder->gpio_b);
+	int i;
+	unsigned ret = 0;
 
-	a ^= encoder->inverted_a;
-	b ^= encoder->inverted_b;
+	for (i = 0; i < encoder->gpios->ndescs; ++i) {
+		int val = gpiod_get_value(encoder->gpios->desc[i]);
+		/* convert from gray encoding to normal */
+		if (ret & 1)
+			val = !val;
 
-	return ((a << 1) | b);
+		ret = ret << 1 | val;
+	}
+
+	return ret & 3;
 }
 
 static void rotary_encoder_report_event(struct rotary_encoder *encoder)
 {
 	if (encoder->relative_axis) {
 		input_report_rel(encoder->input,
-				 encoder->axis, encoder->dir ? -1 : 1);
+				 encoder->axis, encoder->dir);
 	} else {
 		unsigned int pos = encoder->pos;
 
-		if (encoder->dir) {
+		if (encoder->dir < 0) {
 			/* turning counter-clockwise */
 			if (encoder->rollover)
 				pos += encoder->steps;
@@ -112,12 +113,12 @@ static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
 		break;
 
 	case 0x1:
-	case 0x2:
+	case 0x3:
 		if (encoder->armed)
-			encoder->dir = state - 1;
+			encoder->dir = 2 - state;
 		break;
 
-	case 0x3:
+	case 0x2:
 		encoder->armed = true;
 		break;
 	}
@@ -132,19 +133,13 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id)
 
 	state = rotary_encoder_get_state(encoder);
 
-	switch (state) {
-	case 0x00:
-	case 0x03:
+	if (state & 1) {
+		encoder->dir = ((encoder->last_stable - state + 1) % 4) - 1;
+	} else {
 		if (state != encoder->last_stable) {
 			rotary_encoder_report_event(encoder);
 			encoder->last_stable = state;
 		}
-		break;
-
-	case 0x01:
-	case 0x02:
-		encoder->dir = (encoder->last_stable + state) & 0x01;
-		break;
 	}
 
 	return IRQ_HANDLED;
@@ -153,44 +148,16 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id)
 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);
 
-	/*
-	 * 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.
-		 */
+	if ((encoder->last_stable + 1) % 4 == state)
+		encoder->dir = 1;
+	else if (encoder->last_stable == (state + 1) % 4)
+		encoder->dir = -1;
+	else
 		goto out;
-	}
 
 	rotary_encoder_report_event(encoder);
 
@@ -212,7 +179,6 @@ static int rotary_encoder_parse_dt(struct device *dev,
 	const struct of_device_id *of_id =
 				of_match_device(rotary_encoder_of_match, dev);
 	struct device_node *np = dev->of_node;
-	enum of_gpio_flags flags;
 	int error;
 
 	if (!of_id || !np)
@@ -221,12 +187,6 @@ static int rotary_encoder_parse_dt(struct device *dev,
 	of_property_read_u32(np, "rotary-encoder,steps", &encoder->steps);
 	of_property_read_u32(np, "linux,axis", &encoder->axis);
 
-	encoder->gpio_a = of_get_gpio_flags(np, 0, &flags);
-	encoder->inverted_a = flags & OF_GPIO_ACTIVE_LOW;
-
-	encoder->gpio_b = of_get_gpio_flags(np, 1, &flags);
-	encoder->inverted_b = flags & OF_GPIO_ACTIVE_LOW;
-
 	encoder->relative_axis =
 		of_property_read_bool(np, "rotary-encoder,relative-axis");
 	encoder->rollover =
@@ -273,11 +233,6 @@ static int rotary_encoder_parse_pdata(struct device *dev,
 
 	encoder->steps = pdata->steps;
 	encoder->axis = pdata->axis;
-	encoder->gpio_a = pdata->gpio_a;
-	encoder->gpio_b = pdata->gpio_b;
-	encoder->inverted_a = pdata->inverted_a;
-	encoder->inverted_b = pdata->inverted_b;
-	encoder->steps_per_period = pdata->steps_per_period;
 	encoder->relative_axis = pdata->relative_axis;
 	encoder->rollover = pdata->rollover;
 
@@ -291,6 +246,7 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 	struct input_dev *input;
 	irq_handler_t handler;
 	int err;
+	unsigned int i;
 
 	encoder = devm_kzalloc(dev, sizeof(struct rotary_encoder), GFP_KERNEL);
 	input = devm_input_allocate_device(&pdev->dev);
@@ -307,6 +263,16 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 	if (err < 0)
 		return err;
 
+	encoder->gpios = devm_gpiod_get_array(dev, NULL, GPIOD_IN);
+	if (IS_ERR(encoder->gpios)) {
+		dev_err(dev, "unable to get gpios\n");
+		return PTR_ERR(encoder->gpios);
+	}
+	if (encoder->gpios->ndescs < 2) {
+		dev_err(dev, "not enough gpios found\n");
+		return -EINVAL;
+	}
+
 	encoder->input = input;
 
 	input->name = pdev->name;
@@ -322,25 +288,7 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 				     encoder->axis, 0, encoder->steps, 0, 1);
 	}
 
-	/* request the GPIOs */
-	err = devm_gpio_request_one(dev, encoder->gpio_a,
-				    GPIOF_IN, dev_name(dev));
-	if (err) {
-		dev_err(dev, "unable to request GPIO %d\n", encoder->gpio_a);
-		return err;
-	}
-
-	err = devm_gpio_request_one(dev, encoder->gpio_b,
-				    GPIOF_IN, dev_name(dev));
-	if (err) {
-		dev_err(dev, "unable to request GPIO %d\n", encoder->gpio_b);
-		return err;
-	}
-
-	encoder->irq_a = gpio_to_irq(encoder->gpio_a);
-	encoder->irq_b = gpio_to_irq(encoder->gpio_b);
-
-	switch (encoder->steps_per_period) {
+	switch (encoder->steps_per_period >> (encoder->gpios->ndescs - 2)) {
 	case 4:
 		handler = &rotary_encoder_quarter_period_irq;
 		encoder->last_stable = rotary_encoder_get_state(encoder);
@@ -358,20 +306,23 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	err = devm_request_irq(dev, encoder->irq_a, handler,
-			       IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
-			       DRV_NAME, encoder);
-	if (err) {
-		dev_err(dev, "unable to request IRQ %d\n", encoder->irq_a);
-		return err;
-	}
+	encoder->irq =
+		devm_kzalloc(dev,
+			     sizeof(*encoder->irq) * encoder->gpios->ndescs,
+			     GFP_KERNEL);
+	if (!encoder->irq)
+		return -ENOMEM;
 
-	err = devm_request_irq(dev, encoder->irq_b, handler,
-			       IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
-			       DRV_NAME, encoder);
-	if (err) {
-		dev_err(dev, "unable to request IRQ %d\n", encoder->irq_b);
-		return err;
+	for (i = 0; i < encoder->gpios->ndescs; ++i) {
+		encoder->irq[i] = gpiod_to_irq(encoder->gpios->desc[i]);
+		err = devm_request_irq(dev, encoder->irq[i], handler,
+				       IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+				       DRV_NAME, encoder);
+		if (err) {
+			dev_err(dev, "unable to request IRQ %d (gpio#%d)\n",
+				encoder->irq[i], i);
+			return err;
+		}
 	}
 
 	err = input_register_device(input);
@@ -399,8 +350,9 @@ static int rotary_encoder_suspend(struct device *dev)
 	struct rotary_encoder *encoder = dev_get_drvdata(dev);
 
 	if (device_may_wakeup(dev)) {
-		enable_irq_wake(encoder->irq_a);
-		enable_irq_wake(encoder->irq_b);
+		unsigned int i;
+		for (i = 0; i < encoder->gpios->ndescs; ++i)
+			enable_irq_wake(encoder->irq[i]);
 	}
 
 	return 0;
@@ -411,8 +363,9 @@ static int rotary_encoder_resume(struct device *dev)
 	struct rotary_encoder *encoder = dev_get_drvdata(dev);
 
 	if (device_may_wakeup(dev)) {
-		disable_irq_wake(encoder->irq_a);
-		disable_irq_wake(encoder->irq_b);
+		unsigned int i;
+		for (i = 0; i < encoder->gpios->ndescs; ++i)
+			disable_irq_wake(encoder->irq[i]);
 	}
 
 	return 0;
diff --git a/include/linux/rotary_encoder.h b/include/linux/rotary_encoder.h
index fe3dc64e5aeb..4536c813a1e9 100644
--- a/include/linux/rotary_encoder.h
+++ b/include/linux/rotary_encoder.h
@@ -4,10 +4,6 @@
 struct rotary_encoder_platform_data {
 	unsigned int steps;
 	unsigned int axis;
-	unsigned int gpio_a;
-	unsigned int gpio_b;
-	unsigned int inverted_a;
-	unsigned int inverted_b;
 	unsigned int steps_per_period;
 	bool relative_axis;
 	bool rollover;
-- 
2.7.0.rc3

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

[PATCH v2 3/3] Input: rotary-encoder - support more than 2 gpios as input

[Uwe Kleine-König]

Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
---
 .../devicetree/bindings/input/rotary-encoder.txt   |   2 +-
 arch/arm/mach-pxa/raumfeld.c                       |  25 ++-
 drivers/input/misc/rotary_encoder.c                | 171 ++++++++-------------
 include/linux/rotary_encoder.h                     |   4 -
 4 files changed, 84 insertions(+), 118 deletions(-)

diff --git a/Documentation/devicetree/bindings/input/rotary-encoder.txt b/Documentation/devicetree/bindings/input/rotary-encoder.txt
index de99cbbbf6da..6c9f0c8a846c 100644
--- a/Documentation/devicetree/bindings/input/rotary-encoder.txt
+++ b/Documentation/devicetree/bindings/input/rotary-encoder.txt
@@ -1,7 +1,7 @@
 Rotary encoder DT bindings
 
 Required properties:
-- gpios: a spec for two GPIOs to be used
+- gpios: a spec for at least two GPIOs to be used, most significant first
 
 Optional properties:
 - linux,axis: the input subsystem axis to map to this rotary encoder.
diff --git a/arch/arm/mach-pxa/raumfeld.c b/arch/arm/mach-pxa/raumfeld.c
index 8347d87a713d..87988d717465 100644
--- a/arch/arm/mach-pxa/raumfeld.c
+++ b/arch/arm/mach-pxa/raumfeld.c
@@ -21,6 +21,7 @@
 #include <linux/platform_device.h>
 #include <linux/interrupt.h>
 #include <linux/gpio.h>
+#include <linux/gpio/machine.h>
 #include <linux/smsc911x.h>
 #include <linux/input.h>
 #include <linux/rotary_encoder.h>
@@ -370,10 +371,25 @@ static struct rotary_encoder_platform_data raumfeld_rotary_encoder_info = {
 	.steps		= 24,
 	.axis		= REL_X,
 	.relative_axis	= 1,
-	.gpio_a		= GPIO_VOLENC_A,
-	.gpio_b		= GPIO_VOLENC_B,
-	.inverted_a	= 1,
-	.inverted_b	= 0,
+};
+
+static struct gpiod_lookup_table raumfeld_rotary_encoder_gpio_lookup = {
+	.dev_id = "rotary_encoder.0",
+	.table = {
+		{
+			.chip_label = "gpio-0",
+			.chip_hwnum = GPIO_VOLENC_A,
+			.idx = 0,
+			.flags = GPIO_ACTIVE_LOW,
+		},
+		{
+			.chip_label = "gpio-0",
+			.chip_hwnum = GPIO_VOLENC_B,
+			.idx = 1,
+			.flags = GPIO_ACTIVE_HIGH,
+		},
+		{ /* sentinel */ }
+	},
 };
 
 static struct platform_device rotary_encoder_device = {
@@ -1051,6 +1067,7 @@ static void __init __maybe_unused raumfeld_controller_init(void)
 	int ret;
 
 	pxa3xx_mfp_config(ARRAY_AND_SIZE(raumfeld_controller_pin_config));
+	gpiod_add_lookup_table(&raumfeld_rotary_encoder_gpio_lookup);
 	platform_device_register(&rotary_encoder_device);
 	spi_register_board_info(ARRAY_AND_SIZE(controller_spi_devices));
 	i2c_register_board_info(0, &raumfeld_controller_i2c_board_info, 1);
diff --git a/drivers/input/misc/rotary_encoder.c b/drivers/input/misc/rotary_encoder.c
index 0582e851993f..256fe232a26b 100644
--- a/drivers/input/misc/rotary_encoder.c
+++ b/drivers/input/misc/rotary_encoder.c
@@ -25,7 +25,6 @@
 #include <linux/slab.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
-#include <linux/of_gpio.h>
 #include <linux/pm.h>
 
 #define DRV_NAME "rotary-encoder"
@@ -36,45 +35,47 @@ struct rotary_encoder {
 	/* configuration */
 	unsigned int steps;
 	unsigned int axis;
-	unsigned int gpio_a;
-	unsigned int gpio_b;
-	unsigned int inverted_a;
-	unsigned int inverted_b;
 	unsigned int steps_per_period;
 	bool relative_axis;
 	bool rollover;
 	bool wakeup_source;
 
-	unsigned int irq_a;
-	unsigned int irq_b;
+	struct gpio_descs *gpios;
+	unsigned int *irq;
 
 	/* state */
 	unsigned int pos;
 	bool armed;
-	unsigned char dir;	/* 0 - clockwise, 1 - CCW */
+	signed char dir;	/* 1 - clockwise, -1 - CCW */
 	char last_stable;
 };
 
-static int rotary_encoder_get_state(const struct rotary_encoder *encoder)
+static unsigned rotary_encoder_get_state(const struct rotary_encoder *encoder)
 {
-	int a = !!gpio_get_value(encoder->gpio_a);
-	int b = !!gpio_get_value(encoder->gpio_b);
+	int i;
+	unsigned ret = 0;
 
-	a ^= encoder->inverted_a;
-	b ^= encoder->inverted_b;
+	for (i = 0; i < encoder->gpios->ndescs; ++i) {
+		int val = gpiod_get_value(encoder->gpios->desc[i]);
+		/* convert from gray encoding to normal */
+		if (ret & 1)
+			val = !val;
 
-	return ((a << 1) | b);
+		ret = ret << 1 | val;
+	}
+
+	return ret & 3;
 }
 
 static void rotary_encoder_report_event(struct rotary_encoder *encoder)
 {
 	if (encoder->relative_axis) {
 		input_report_rel(encoder->input,
-				 encoder->axis, encoder->dir ? -1 : 1);
+				 encoder->axis, encoder->dir);
 	} else {
 		unsigned int pos = encoder->pos;
 
-		if (encoder->dir) {
+		if (encoder->dir < 0) {
 			/* turning counter-clockwise */
 			if (encoder->rollover)
 				pos += encoder->steps;
@@ -112,12 +113,12 @@ static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
 		break;
 
 	case 0x1:
-	case 0x2:
+	case 0x3:
 		if (encoder->armed)
-			encoder->dir = state - 1;
+			encoder->dir = 2 - state;
 		break;
 
-	case 0x3:
+	case 0x2:
 		encoder->armed = true;
 		break;
 	}
@@ -132,19 +133,13 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id)
 
 	state = rotary_encoder_get_state(encoder);
 
-	switch (state) {
-	case 0x00:
-	case 0x03:
+	if (state & 1) {
+		encoder->dir = ((encoder->last_stable - state + 1) % 4) - 1;
+	} else {
 		if (state != encoder->last_stable) {
 			rotary_encoder_report_event(encoder);
 			encoder->last_stable = state;
 		}
-		break;
-
-	case 0x01:
-	case 0x02:
-		encoder->dir = (encoder->last_stable + state) & 0x01;
-		break;
 	}
 
 	return IRQ_HANDLED;
@@ -153,44 +148,16 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id)
 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);
 
-	/*
-	 * 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.
-		 */
+	if ((encoder->last_stable + 1) % 4 == state)
+		encoder->dir = 1;
+	else if (encoder->last_stable == (state + 1) % 4)
+		encoder->dir = -1;
+	else
 		goto out;
-	}
 
 	rotary_encoder_report_event(encoder);
 
@@ -212,7 +179,6 @@ static int rotary_encoder_parse_dt(struct device *dev,
 	const struct of_device_id *of_id =
 				of_match_device(rotary_encoder_of_match, dev);
 	struct device_node *np = dev->of_node;
-	enum of_gpio_flags flags;
 	int error;
 
 	if (!of_id || !np)
@@ -221,12 +187,6 @@ static int rotary_encoder_parse_dt(struct device *dev,
 	of_property_read_u32(np, "rotary-encoder,steps", &encoder->steps);
 	of_property_read_u32(np, "linux,axis", &encoder->axis);
 
-	encoder->gpio_a = of_get_gpio_flags(np, 0, &flags);
-	encoder->inverted_a = flags & OF_GPIO_ACTIVE_LOW;
-
-	encoder->gpio_b = of_get_gpio_flags(np, 1, &flags);
-	encoder->inverted_b = flags & OF_GPIO_ACTIVE_LOW;
-
 	encoder->relative_axis =
 		of_property_read_bool(np, "rotary-encoder,relative-axis");
 	encoder->rollover =
@@ -273,11 +233,6 @@ static int rotary_encoder_parse_pdata(struct device *dev,
 
 	encoder->steps = pdata->steps;
 	encoder->axis = pdata->axis;
-	encoder->gpio_a = pdata->gpio_a;
-	encoder->gpio_b = pdata->gpio_b;
-	encoder->inverted_a = pdata->inverted_a;
-	encoder->inverted_b = pdata->inverted_b;
-	encoder->steps_per_period = pdata->steps_per_period;
 	encoder->relative_axis = pdata->relative_axis;
 	encoder->rollover = pdata->rollover;
 
@@ -291,6 +246,7 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 	struct input_dev *input;
 	irq_handler_t handler;
 	int err;
+	unsigned int i;
 
 	encoder = devm_kzalloc(dev, sizeof(struct rotary_encoder), GFP_KERNEL);
 	input = devm_input_allocate_device(&pdev->dev);
@@ -307,6 +263,16 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 	if (err < 0)
 		return err;
 
+	encoder->gpios = devm_gpiod_get_array(dev, NULL, GPIOD_IN);
+	if (IS_ERR(encoder->gpios)) {
+		dev_err(dev, "unable to get gpios\n");
+		return PTR_ERR(encoder->gpios);
+	}
+	if (encoder->gpios->ndescs < 2) {
+		dev_err(dev, "not enough gpios found\n");
+		return -EINVAL;
+	}
+
 	encoder->input = input;
 
 	input->name = pdev->name;
@@ -322,25 +288,7 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 				     encoder->axis, 0, encoder->steps, 0, 1);
 	}
 
-	/* request the GPIOs */
-	err = devm_gpio_request_one(dev, encoder->gpio_a,
-				    GPIOF_IN, dev_name(dev));
-	if (err) {
-		dev_err(dev, "unable to request GPIO %d\n", encoder->gpio_a);
-		return err;
-	}
-
-	err = devm_gpio_request_one(dev, encoder->gpio_b,
-				    GPIOF_IN, dev_name(dev));
-	if (err) {
-		dev_err(dev, "unable to request GPIO %d\n", encoder->gpio_b);
-		return err;
-	}
-
-	encoder->irq_a = gpio_to_irq(encoder->gpio_a);
-	encoder->irq_b = gpio_to_irq(encoder->gpio_b);
-
-	switch (encoder->steps_per_period) {
+	switch (encoder->steps_per_period >> (encoder->gpios->ndescs - 2)) {
 	case 4:
 		handler = &rotary_encoder_quarter_period_irq;
 		encoder->last_stable = rotary_encoder_get_state(encoder);
@@ -358,20 +306,23 @@ static int rotary_encoder_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	err = devm_request_irq(dev, encoder->irq_a, handler,
-			       IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
-			       DRV_NAME, encoder);
-	if (err) {
-		dev_err(dev, "unable to request IRQ %d\n", encoder->irq_a);
-		return err;
-	}
+	encoder->irq =
+		devm_kzalloc(dev,
+			     sizeof(*encoder->irq) * encoder->gpios->ndescs,
+			     GFP_KERNEL);
+	if (!encoder->irq)
+		return -ENOMEM;
 
-	err = devm_request_irq(dev, encoder->irq_b, handler,
-			       IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
-			       DRV_NAME, encoder);
-	if (err) {
-		dev_err(dev, "unable to request IRQ %d\n", encoder->irq_b);
-		return err;
+	for (i = 0; i < encoder->gpios->ndescs; ++i) {
+		encoder->irq[i] = gpiod_to_irq(encoder->gpios->desc[i]);
+		err = devm_request_irq(dev, encoder->irq[i], handler,
+				       IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING,
+				       DRV_NAME, encoder);
+		if (err) {
+			dev_err(dev, "unable to request IRQ %d (gpio#%d)\n",
+				encoder->irq[i], i);
+			return err;
+		}
 	}
 
 	err = input_register_device(input);
@@ -399,8 +350,9 @@ static int rotary_encoder_suspend(struct device *dev)
 	struct rotary_encoder *encoder = dev_get_drvdata(dev);
 
 	if (device_may_wakeup(dev)) {
-		enable_irq_wake(encoder->irq_a);
-		enable_irq_wake(encoder->irq_b);
+		unsigned int i;
+		for (i = 0; i < encoder->gpios->ndescs; ++i)
+			enable_irq_wake(encoder->irq[i]);
 	}
 
 	return 0;
@@ -411,8 +363,9 @@ static int rotary_encoder_resume(struct device *dev)
 	struct rotary_encoder *encoder = dev_get_drvdata(dev);
 
 	if (device_may_wakeup(dev)) {
-		disable_irq_wake(encoder->irq_a);
-		disable_irq_wake(encoder->irq_b);
+		unsigned int i;
+		for (i = 0; i < encoder->gpios->ndescs; ++i)
+			disable_irq_wake(encoder->irq[i]);
 	}
 
 	return 0;
diff --git a/include/linux/rotary_encoder.h b/include/linux/rotary_encoder.h
index fe3dc64e5aeb..4536c813a1e9 100644
--- a/include/linux/rotary_encoder.h
+++ b/include/linux/rotary_encoder.h
@@ -4,10 +4,6 @@
 struct rotary_encoder_platform_data {
 	unsigned int steps;
 	unsigned int axis;
-	unsigned int gpio_a;
-	unsigned int gpio_b;
-	unsigned int inverted_a;
-	unsigned int inverted_b;
 	unsigned int steps_per_period;
 	bool relative_axis;
 	bool rollover;
-- 
2.7.0.rc3

Re: [PATCH v2 0/3] Input: rotary-encoder - use more than two gpios

[Daniel Mack]

Hi Uwe,

Thanks for the update.

On 02/02/2016 11:24 AM, Uwe Kleine-König wrote:
> Some time ago I sent a v1 of this, now after testing the changes more
> deeply patch 3 changed a bit. The old series started with
> 
> 	Date: Wed,  2 Dec 2015 11:07:11 +0100                                                                                               
> 	From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>                                                                             
> 	Subject: [PATCH RFC 0/3] input: rotary_encoder: use more than two gpios as input                                                    
> 	Message-Id: <1449050834-31779-1-git-send-email-u.kleine-koenig@pengutronix.de>                                                      
> 
> The two first patches are just preparation for the third patch.
> 
> There is an obvious improvement that allows detection of quick changes
> more reliably with >2 gpios, but I didn't implement this yet. (With 4
> GPIOs you can distinguish a counter clockwise movement of three states
> from a clock wise movement of a single state. Still the patch is useful
> as it makes these devices work at all.
> 
> My test device looks as follows:
> 
>         rotary@0 {
>                 compatible = "rotary-encoder";
>                 gpios = <&gpio4 12 GPIO_ACTIVE_HIGH>, <&gpio4 11 GPIO_ACTIVE_HIGH>, <&gpio4 10 GPIO_ACTIVE_HIGH>, <&gpio4 9 GPIO_ACTIVE_HIGH>;
> 
>                 rotary-encoder,steps = <16>;
> 		rotary-encoder,steps-per-period = <16>;
>         };
> 
> While Daniel Mack and Rojhalat Ibrahim agreed that this device is an
> absolute encoder and should be supported by a simpler logic, I still
> consider it worthwhile to get these patches in as a first step. Also the
> binding looks right, so IMHO the comments shouldn't stop this series
> from going in.

I still don't understand why this is implemented that way, rather than
going for a much simpler logic of interpretation that also allows users
to read out the absolute position.

The code to read the value would be really just as simple as reading all
GPIOs and or-ing their values into the result, and skip the state
machine completely. This code would be active if a new attribute
(something like 'rotary-encoder,hardware-absolute') is set, or even
implicitly, when more than 2 GPIOs are specified.

Is there any reason for not doing that?


Thanks,
Daniel

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

[PATCH v2 0/3] Input: rotary-encoder - use more than two gpios

[Daniel Mack]

Hi Uwe,

Thanks for the update.

On 02/02/2016 11:24 AM, Uwe Kleine-K?nig wrote:
> Some time ago I sent a v1 of this, now after testing the changes more
> deeply patch 3 changed a bit. The old series started with
> 
> 	Date: Wed,  2 Dec 2015 11:07:11 +0100                                                                                               
> 	From: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>                                                                             
> 	Subject: [PATCH RFC 0/3] input: rotary_encoder: use more than two gpios as input                                                    
> 	Message-Id: <1449050834-31779-1-git-send-email-u.kleine-koenig@pengutronix.de>                                                      
> 
> The two first patches are just preparation for the third patch.
> 
> There is an obvious improvement that allows detection of quick changes
> more reliably with >2 gpios, but I didn't implement this yet. (With 4
> GPIOs you can distinguish a counter clockwise movement of three states
> from a clock wise movement of a single state. Still the patch is useful
> as it makes these devices work at all.
> 
> My test device looks as follows:
> 
>         rotary at 0 {
>                 compatible = "rotary-encoder";
>                 gpios = <&gpio4 12 GPIO_ACTIVE_HIGH>, <&gpio4 11 GPIO_ACTIVE_HIGH>, <&gpio4 10 GPIO_ACTIVE_HIGH>, <&gpio4 9 GPIO_ACTIVE_HIGH>;
> 
>                 rotary-encoder,steps = <16>;
> 		rotary-encoder,steps-per-period = <16>;
>         };
> 
> While Daniel Mack and Rojhalat Ibrahim agreed that this device is an
> absolute encoder and should be supported by a simpler logic, I still
> consider it worthwhile to get these patches in as a first step. Also the
> binding looks right, so IMHO the comments shouldn't stop this series
> from going in.

I still don't understand why this is implemented that way, rather than
going for a much simpler logic of interpretation that also allows users
to read out the absolute position.

The code to read the value would be really just as simple as reading all
GPIOs and or-ing their values into the result, and skip the state
machine completely. This code would be active if a new attribute
(something like 'rotary-encoder,hardware-absolute') is set, or even
implicitly, when more than 2 GPIOs are specified.

Is there any reason for not doing that?


Thanks,
Daniel

Re: [PATCH v2 0/3] Input: rotary-encoder - use more than two gpios

[Uwe Kleine-König]

Hello Daniel,

On Tue, Feb 02, 2016 at 01:08:07PM +0100, Daniel Mack wrote:
> On 02/02/2016 11:24 AM, Uwe Kleine-König wrote:
> > Some time ago I sent a v1 of this, now after testing the changes more
> > deeply patch 3 changed a bit. The old series started with
> > 
> > 	Date: Wed,  2 Dec 2015 11:07:11 +0100
> > 	From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> > 	Subject: [PATCH RFC 0/3] input: rotary_encoder: use more than two gpios as input
> > 	Message-Id: <1449050834-31779-1-git-send-email-u.kleine-koenig@pengutronix.de>
> > 
> > The two first patches are just preparation for the third patch.
> > 
> > There is an obvious improvement that allows detection of quick changes
> > more reliably with >2 gpios, but I didn't implement this yet. (With 4
> > GPIOs you can distinguish a counter clockwise movement of three states
> > from a clock wise movement of a single state. Still the patch is useful
> > as it makes these devices work at all.
> > 
> > My test device looks as follows:
> > 
> >         rotary@0 {
> >                 compatible = "rotary-encoder";
> >                 gpios = <&gpio4 12 GPIO_ACTIVE_HIGH>, <&gpio4 11 GPIO_ACTIVE_HIGH>, <&gpio4 10 GPIO_ACTIVE_HIGH>, <&gpio4 9 GPIO_ACTIVE_HIGH>;
> > 
> >                 rotary-encoder,steps = <16>;
> > 		rotary-encoder,steps-per-period = <16>;
> >         };
> > 
> > While Daniel Mack and Rojhalat Ibrahim agreed that this device is an
> > absolute encoder and should be supported by a simpler logic, I still
> > consider it worthwhile to get these patches in as a first step. Also the
> > binding looks right, so IMHO the comments shouldn't stop this series
> > from going in.
> 
> I still don't understand why this is implemented that way, rather than
> going for a much simpler logic of interpretation that also allows users
> to read out the absolute position.
> 
> The code to read the value would be really just as simple as reading all
> GPIOs and or-ing their values into the result, and skip the state
> machine completely. This code would be active if a new attribute
> (something like 'rotary-encoder,hardware-absolute') is set, or even
> implicitly, when more than 2 GPIOs are specified.
> 
> Is there any reason for not doing that?

Currently the reason is lack of time. And when implementing
rotary-encoder,hardware-absolute something similar would be the result
for the relative reporting anyhow. So the problem is only that I don't
have absolute support yet, but the patches as is would be the base for
that anyhow.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
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

[PATCH v2 0/3] Input: rotary-encoder - use more than two gpios

[Uwe Kleine-König]

Hello Daniel,

On Tue, Feb 02, 2016 at 01:08:07PM +0100, Daniel Mack wrote:
> On 02/02/2016 11:24 AM, Uwe Kleine-K?nig wrote:
> > Some time ago I sent a v1 of this, now after testing the changes more
> > deeply patch 3 changed a bit. The old series started with
> > 
> > 	Date: Wed,  2 Dec 2015 11:07:11 +0100
> > 	From: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> > 	Subject: [PATCH RFC 0/3] input: rotary_encoder: use more than two gpios as input
> > 	Message-Id: <1449050834-31779-1-git-send-email-u.kleine-koenig@pengutronix.de>
> > 
> > The two first patches are just preparation for the third patch.
> > 
> > There is an obvious improvement that allows detection of quick changes
> > more reliably with >2 gpios, but I didn't implement this yet. (With 4
> > GPIOs you can distinguish a counter clockwise movement of three states
> > from a clock wise movement of a single state. Still the patch is useful
> > as it makes these devices work at all.
> > 
> > My test device looks as follows:
> > 
> >         rotary at 0 {
> >                 compatible = "rotary-encoder";
> >                 gpios = <&gpio4 12 GPIO_ACTIVE_HIGH>, <&gpio4 11 GPIO_ACTIVE_HIGH>, <&gpio4 10 GPIO_ACTIVE_HIGH>, <&gpio4 9 GPIO_ACTIVE_HIGH>;
> > 
> >                 rotary-encoder,steps = <16>;
> > 		rotary-encoder,steps-per-period = <16>;
> >         };
> > 
> > While Daniel Mack and Rojhalat Ibrahim agreed that this device is an
> > absolute encoder and should be supported by a simpler logic, I still
> > consider it worthwhile to get these patches in as a first step. Also the
> > binding looks right, so IMHO the comments shouldn't stop this series
> > from going in.
> 
> I still don't understand why this is implemented that way, rather than
> going for a much simpler logic of interpretation that also allows users
> to read out the absolute position.
> 
> The code to read the value would be really just as simple as reading all
> GPIOs and or-ing their values into the result, and skip the state
> machine completely. This code would be active if a new attribute
> (something like 'rotary-encoder,hardware-absolute') is set, or even
> implicitly, when more than 2 GPIOs are specified.
> 
> Is there any reason for not doing that?

Currently the reason is lack of time. And when implementing
rotary-encoder,hardware-absolute something similar would be the result
for the relative reporting anyhow. So the problem is only that I don't
have absolute support yet, but the patches as is would be the base for
that anyhow.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

Re: [PATCH v2 0/3] Input: rotary-encoder - use more than two gpios

[Daniel Mack]

On 02/02/2016 01:56 PM, Uwe Kleine-König wrote:
> Hello Daniel,
> 
> On Tue, Feb 02, 2016 at 01:08:07PM +0100, Daniel Mack wrote:
>> On 02/02/2016 11:24 AM, Uwe Kleine-König wrote:
>>> Some time ago I sent a v1 of this, now after testing the changes more
>>> deeply patch 3 changed a bit. The old series started with
>>>
>>> 	Date: Wed,  2 Dec 2015 11:07:11 +0100
>>> 	From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
>>> 	Subject: [PATCH RFC 0/3] input: rotary_encoder: use more than two gpios as input
>>> 	Message-Id: <1449050834-31779-1-git-send-email-u.kleine-koenig@pengutronix.de>
>>>
>>> The two first patches are just preparation for the third patch.
>>>
>>> There is an obvious improvement that allows detection of quick changes
>>> more reliably with >2 gpios, but I didn't implement this yet. (With 4
>>> GPIOs you can distinguish a counter clockwise movement of three states
>>> from a clock wise movement of a single state. Still the patch is useful
>>> as it makes these devices work at all.
>>>
>>> My test device looks as follows:
>>>
>>>         rotary@0 {
>>>                 compatible = "rotary-encoder";
>>>                 gpios = <&gpio4 12 GPIO_ACTIVE_HIGH>, <&gpio4 11 GPIO_ACTIVE_HIGH>, <&gpio4 10 GPIO_ACTIVE_HIGH>, <&gpio4 9 GPIO_ACTIVE_HIGH>;
>>>
>>>                 rotary-encoder,steps = <16>;
>>> 		rotary-encoder,steps-per-period = <16>;
>>>         };
>>>
>>> While Daniel Mack and Rojhalat Ibrahim agreed that this device is an
>>> absolute encoder and should be supported by a simpler logic, I still
>>> consider it worthwhile to get these patches in as a first step. Also the
>>> binding looks right, so IMHO the comments shouldn't stop this series
>>> from going in.
>>
>> I still don't understand why this is implemented that way, rather than
>> going for a much simpler logic of interpretation that also allows users
>> to read out the absolute position.
>>
>> The code to read the value would be really just as simple as reading all
>> GPIOs and or-ing their values into the result, and skip the state
>> machine completely. This code would be active if a new attribute
>> (something like 'rotary-encoder,hardware-absolute') is set, or even
>> implicitly, when more than 2 GPIOs are specified.
>>
>> Is there any reason for not doing that?
> 
> Currently the reason is lack of time. And when implementing
> rotary-encoder,hardware-absolute something similar would be the result
> for the relative reporting anyhow. So the problem is only that I don't
> have absolute support yet, but the patches as is would be the base for
> that anyhow.

Because you would support relative support for such 4-pin encoders as
well? I would have thought that absolute encoders would report absolute
values only, but I guess you have a point here. Just to make sure we're
on the same page: For more than 2 GPIOs, and an absent
"rotary-encoder,relative-axis", the driver would switch to a mode in
which it bypasses the state machine, right?

If you're planning to implement that eventually, I'm fine with the
patches for now :)

Dmitry, if you are apply them, feel free to add my

  Acked-by: Daniel Mack <daniel@zonque.org>

to all 3 of them.


Thanks,
Daniel

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

[PATCH v2 0/3] Input: rotary-encoder - use more than two gpios

[Daniel Mack]

On 02/02/2016 01:56 PM, Uwe Kleine-K?nig wrote:
> Hello Daniel,
> 
> On Tue, Feb 02, 2016 at 01:08:07PM +0100, Daniel Mack wrote:
>> On 02/02/2016 11:24 AM, Uwe Kleine-K?nig wrote:
>>> Some time ago I sent a v1 of this, now after testing the changes more
>>> deeply patch 3 changed a bit. The old series started with
>>>
>>> 	Date: Wed,  2 Dec 2015 11:07:11 +0100
>>> 	From: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
>>> 	Subject: [PATCH RFC 0/3] input: rotary_encoder: use more than two gpios as input
>>> 	Message-Id: <1449050834-31779-1-git-send-email-u.kleine-koenig@pengutronix.de>
>>>
>>> The two first patches are just preparation for the third patch.
>>>
>>> There is an obvious improvement that allows detection of quick changes
>>> more reliably with >2 gpios, but I didn't implement this yet. (With 4
>>> GPIOs you can distinguish a counter clockwise movement of three states
>>> from a clock wise movement of a single state. Still the patch is useful
>>> as it makes these devices work at all.
>>>
>>> My test device looks as follows:
>>>
>>>         rotary at 0 {
>>>                 compatible = "rotary-encoder";
>>>                 gpios = <&gpio4 12 GPIO_ACTIVE_HIGH>, <&gpio4 11 GPIO_ACTIVE_HIGH>, <&gpio4 10 GPIO_ACTIVE_HIGH>, <&gpio4 9 GPIO_ACTIVE_HIGH>;
>>>
>>>                 rotary-encoder,steps = <16>;
>>> 		rotary-encoder,steps-per-period = <16>;
>>>         };
>>>
>>> While Daniel Mack and Rojhalat Ibrahim agreed that this device is an
>>> absolute encoder and should be supported by a simpler logic, I still
>>> consider it worthwhile to get these patches in as a first step. Also the
>>> binding looks right, so IMHO the comments shouldn't stop this series
>>> from going in.
>>
>> I still don't understand why this is implemented that way, rather than
>> going for a much simpler logic of interpretation that also allows users
>> to read out the absolute position.
>>
>> The code to read the value would be really just as simple as reading all
>> GPIOs and or-ing their values into the result, and skip the state
>> machine completely. This code would be active if a new attribute
>> (something like 'rotary-encoder,hardware-absolute') is set, or even
>> implicitly, when more than 2 GPIOs are specified.
>>
>> Is there any reason for not doing that?
> 
> Currently the reason is lack of time. And when implementing
> rotary-encoder,hardware-absolute something similar would be the result
> for the relative reporting anyhow. So the problem is only that I don't
> have absolute support yet, but the patches as is would be the base for
> that anyhow.

Because you would support relative support for such 4-pin encoders as
well? I would have thought that absolute encoders would report absolute
values only, but I guess you have a point here. Just to make sure we're
on the same page: For more than 2 GPIOs, and an absent
"rotary-encoder,relative-axis", the driver would switch to a mode in
which it bypasses the state machine, right?

If you're planning to implement that eventually, I'm fine with the
patches for now :)

Dmitry, if you are apply them, feel free to add my

  Acked-by: Daniel Mack <daniel@zonque.org>

to all 3 of them.


Thanks,
Daniel

Re: [PATCH v2 0/3] Input: rotary-encoder - use more than two gpios

[Uwe Kleine-König]

Hello Daniel,

On Tue, Feb 02, 2016 at 02:14:49PM +0100, Daniel Mack wrote:
> On 02/02/2016 01:56 PM, Uwe Kleine-König wrote:
> > Hello Daniel,
> > 
> > On Tue, Feb 02, 2016 at 01:08:07PM +0100, Daniel Mack wrote:
> >> On 02/02/2016 11:24 AM, Uwe Kleine-König wrote:
> >>> Some time ago I sent a v1 of this, now after testing the changes more
> >>> deeply patch 3 changed a bit. The old series started with
> >>>
> >>> 	Date: Wed,  2 Dec 2015 11:07:11 +0100
> >>> 	From: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> >>> 	Subject: [PATCH RFC 0/3] input: rotary_encoder: use more than two gpios as input
> >>> 	Message-Id: <1449050834-31779-1-git-send-email-u.kleine-koenig@pengutronix.de>
> >>>
> >>> The two first patches are just preparation for the third patch.
> >>>
> >>> There is an obvious improvement that allows detection of quick changes
> >>> more reliably with >2 gpios, but I didn't implement this yet. (With 4
> >>> GPIOs you can distinguish a counter clockwise movement of three states
> >>> from a clock wise movement of a single state. Still the patch is useful
> >>> as it makes these devices work at all.
> >>>
> >>> My test device looks as follows:
> >>>
> >>>         rotary@0 {
> >>>                 compatible = "rotary-encoder";
> >>>                 gpios = <&gpio4 12 GPIO_ACTIVE_HIGH>, <&gpio4 11 GPIO_ACTIVE_HIGH>, <&gpio4 10 GPIO_ACTIVE_HIGH>, <&gpio4 9 GPIO_ACTIVE_HIGH>;
> >>>
> >>>                 rotary-encoder,steps = <16>;
> >>> 		rotary-encoder,steps-per-period = <16>;
> >>>         };
> >>>
> >>> While Daniel Mack and Rojhalat Ibrahim agreed that this device is an
> >>> absolute encoder and should be supported by a simpler logic, I still
> >>> consider it worthwhile to get these patches in as a first step. Also the
> >>> binding looks right, so IMHO the comments shouldn't stop this series
> >>> from going in.
> >>
> >> I still don't understand why this is implemented that way, rather than
> >> going for a much simpler logic of interpretation that also allows users
> >> to read out the absolute position.
> >>
> >> The code to read the value would be really just as simple as reading all
> >> GPIOs and or-ing their values into the result, and skip the state
> >> machine completely. This code would be active if a new attribute
> >> (something like 'rotary-encoder,hardware-absolute') is set, or even
> >> implicitly, when more than 2 GPIOs are specified.
> >>
> >> Is there any reason for not doing that?
> > 
> > Currently the reason is lack of time. And when implementing
> > rotary-encoder,hardware-absolute something similar would be the result
> > for the relative reporting anyhow. So the problem is only that I don't
> > have absolute support yet, but the patches as is would be the base for
> > that anyhow.
> 
> Because you would support relative support for such 4-pin encoders as
> well? I would have thought that absolute encoders would report absolute
> values only, but I guess you have a point here. Just to make sure we're
> on the same page: For more than 2 GPIOs, and an absent
> "rotary-encoder,relative-axis", the driver would switch to a mode in
> which it bypasses the state machine, right?

Not sure about how this will be represented in the device tree, but yes,
that's more or less how I imagine to implement this. (And in the end
having support for a 4-line relative device is just a side effect of
having support for relative devices and for 4-line devices.)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
--
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 v2 0/3] Input: rotary-encoder - use more than two gpios

[Uwe Kleine-König]

Hello Daniel,

On Tue, Feb 02, 2016 at 02:14:49PM +0100, Daniel Mack wrote:
> On 02/02/2016 01:56 PM, Uwe Kleine-K?nig wrote:
> > Hello Daniel,
> > 
> > On Tue, Feb 02, 2016 at 01:08:07PM +0100, Daniel Mack wrote:
> >> On 02/02/2016 11:24 AM, Uwe Kleine-K?nig wrote:
> >>> Some time ago I sent a v1 of this, now after testing the changes more
> >>> deeply patch 3 changed a bit. The old series started with
> >>>
> >>> 	Date: Wed,  2 Dec 2015 11:07:11 +0100
> >>> 	From: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> >>> 	Subject: [PATCH RFC 0/3] input: rotary_encoder: use more than two gpios as input
> >>> 	Message-Id: <1449050834-31779-1-git-send-email-u.kleine-koenig@pengutronix.de>
> >>>
> >>> The two first patches are just preparation for the third patch.
> >>>
> >>> There is an obvious improvement that allows detection of quick changes
> >>> more reliably with >2 gpios, but I didn't implement this yet. (With 4
> >>> GPIOs you can distinguish a counter clockwise movement of three states
> >>> from a clock wise movement of a single state. Still the patch is useful
> >>> as it makes these devices work at all.
> >>>
> >>> My test device looks as follows:
> >>>
> >>>         rotary at 0 {
> >>>                 compatible = "rotary-encoder";
> >>>                 gpios = <&gpio4 12 GPIO_ACTIVE_HIGH>, <&gpio4 11 GPIO_ACTIVE_HIGH>, <&gpio4 10 GPIO_ACTIVE_HIGH>, <&gpio4 9 GPIO_ACTIVE_HIGH>;
> >>>
> >>>                 rotary-encoder,steps = <16>;
> >>> 		rotary-encoder,steps-per-period = <16>;
> >>>         };
> >>>
> >>> While Daniel Mack and Rojhalat Ibrahim agreed that this device is an
> >>> absolute encoder and should be supported by a simpler logic, I still
> >>> consider it worthwhile to get these patches in as a first step. Also the
> >>> binding looks right, so IMHO the comments shouldn't stop this series
> >>> from going in.
> >>
> >> I still don't understand why this is implemented that way, rather than
> >> going for a much simpler logic of interpretation that also allows users
> >> to read out the absolute position.
> >>
> >> The code to read the value would be really just as simple as reading all
> >> GPIOs and or-ing their values into the result, and skip the state
> >> machine completely. This code would be active if a new attribute
> >> (something like 'rotary-encoder,hardware-absolute') is set, or even
> >> implicitly, when more than 2 GPIOs are specified.
> >>
> >> Is there any reason for not doing that?
> > 
> > Currently the reason is lack of time. And when implementing
> > rotary-encoder,hardware-absolute something similar would be the result
> > for the relative reporting anyhow. So the problem is only that I don't
> > have absolute support yet, but the patches as is would be the base for
> > that anyhow.
> 
> Because you would support relative support for such 4-pin encoders as
> well? I would have thought that absolute encoders would report absolute
> values only, but I guess you have a point here. Just to make sure we're
> on the same page: For more than 2 GPIOs, and an absent
> "rotary-encoder,relative-axis", the driver would switch to a mode in
> which it bypasses the state machine, right?

Not sure about how this will be represented in the device tree, but yes,
that's more or less how I imagine to implement this. (And in the end
having support for a 4-line relative device is just a side effect of
having support for relative devices and for 4-line devices.)

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-K?nig            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |