[PATCH RFC 0/3] input: rotary_encoder: use more than two gpios as input

[PATCH RFC 0/3] input: rotary_encoder: use more than two gpios as input

[Uwe Kleine-König]

Hello,

some time ago I worked on the rotary encoder driver to make it support
more than two input lines. This is the (only slightly tested[1]) rebase of
this series on top of 4.4-rc2 (from 4.1).

It would be great to get some feedback, especially (but not only) for my
change to raumfeld.c.

Before Ezequiel's patch 3a341a4c30d4 ("Input: rotary-encoder - add
support for quarter-period mode") we had a dt property
"rotary-encoder,half-period" defined. It's presence meant that we had 2
indents per period; it's absence that there is only 1. Ezequiel
introduced rotary-encoder,steps-per-period instead when adding support
for quarter-period mode (which has 4 indents per period).

Up to now (i.e. with two inputs) a period has length 4. Now with (say)
four inputs a period has length 16 instead. Now how should this be
modeled in dt? This series implements that I have to pass

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

now for "quarter-period mode" (i.e. 4 indents per 4 state changes[2]),
but that feels unnatural. I'd prefer to set a property to <1> instead,
meaning the device has an indent for each state change[2]. half-period
mode would be <2> and full-period mode would be <4>. But I don't have a
nice name for such a property and maybe it's easier to live with
steps-per-period = <16>? What do you think?

Also note that these patches are not as technically versatile as
possible. With 4 (n) input lines we could detect a quick rotation where the
machine's latency only allows to sample after 7 (2^(n-1)-1) state
changes. Currently this is not implemented, but can be done later.

Best regards
Uwe

[1] the machine that has the device is ~600 km away from me
[2] if you have a better word for that, please tell me.

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                | 283 ++++++++++-----------
 include/linux/rotary_encoder.h                     |   4 -
 4 files changed, 153 insertions(+), 161 deletions(-)

-- 
2.6.2

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

[PATCH RFC 0/3] input: rotary_encoder: use more than two gpios as input

[Uwe Kleine-König]

Hello,

some time ago I worked on the rotary encoder driver to make it support
more than two input lines. This is the (only slightly tested[1]) rebase of
this series on top of 4.4-rc2 (from 4.1).

It would be great to get some feedback, especially (but not only) for my
change to raumfeld.c.

Before Ezequiel's patch 3a341a4c30d4 ("Input: rotary-encoder - add
support for quarter-period mode") we had a dt property
"rotary-encoder,half-period" defined. It's presence meant that we had 2
indents per period; it's absence that there is only 1. Ezequiel
introduced rotary-encoder,steps-per-period instead when adding support
for quarter-period mode (which has 4 indents per period).

Up to now (i.e. with two inputs) a period has length 4. Now with (say)
four inputs a period has length 16 instead. Now how should this be
modeled in dt? This series implements that I have to pass

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

now for "quarter-period mode" (i.e. 4 indents per 4 state changes[2]),
but that feels unnatural. I'd prefer to set a property to <1> instead,
meaning the device has an indent for each state change[2]. half-period
mode would be <2> and full-period mode would be <4>. But I don't have a
nice name for such a property and maybe it's easier to live with
steps-per-period = <16>? What do you think?

Also note that these patches are not as technically versatile as
possible. With 4 (n) input lines we could detect a quick rotation where the
machine's latency only allows to sample after 7 (2^(n-1)-1) state
changes. Currently this is not implemented, but can be done later.

Best regards
Uwe

[1] the machine that has the device is ~600 km away from me
[2] if you have a better word for that, please tell me.

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                | 283 ++++++++++-----------
 include/linux/rotary_encoder.h                     |   4 -
 4 files changed, 153 insertions(+), 161 deletions(-)

-- 
2.6.2

[PATCH RFC 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.6.2

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

[PATCH RFC 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.6.2

[PATCH RFC 2/3] input: rotary_encoder: move configuration data to driver data

[Uwe Kleine-König]

This is a preparation for the next few patches. 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.6.2

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

[PATCH RFC 2/3] input: rotary_encoder: move configuration data to driver data

[Uwe Kleine-König]

This is a preparation for the next few patches. 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.6.2

[PATCH RFC 3/3] input: rotary_encoder: support more than 2 gpios as input

[Uwe Kleine-König]

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                | 127 +++++++++------------
 include/linux/rotary_encoder.h                     |   4 -
 4 files changed, 79 insertions(+), 79 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 36571a9a44fe..f47e59b0efa8 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 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..99f19e037370 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;
 	}
@@ -134,7 +135,7 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id)
 
 	switch (state) {
 	case 0x00:
-	case 0x03:
+	case 0x02:
 		if (state != encoder->last_stable) {
 			rotary_encoder_report_event(encoder);
 			encoder->last_stable = state;
@@ -142,8 +143,8 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id)
 		break;
 
 	case 0x01:
-	case 0x02:
-		encoder->dir = (encoder->last_stable + state) & 0x01;
+	case 0x03:
+		encoder->dir = ((encoder->last_stable - state + 1) % 4) - 1;
 		break;
 	}
 
@@ -212,7 +213,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 +221,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 +267,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 +280,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 +297,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 +322,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 +340,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 +384,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 +397,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.6.2

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

[PATCH RFC 3/3] input: rotary_encoder: support more than 2 gpios as input

[Uwe Kleine-König]

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                | 127 +++++++++------------
 include/linux/rotary_encoder.h                     |   4 -
 4 files changed, 79 insertions(+), 79 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 36571a9a44fe..f47e59b0efa8 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 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..99f19e037370 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;
 	}
@@ -134,7 +135,7 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id)
 
 	switch (state) {
 	case 0x00:
-	case 0x03:
+	case 0x02:
 		if (state != encoder->last_stable) {
 			rotary_encoder_report_event(encoder);
 			encoder->last_stable = state;
@@ -142,8 +143,8 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id)
 		break;
 
 	case 0x01:
-	case 0x02:
-		encoder->dir = (encoder->last_stable + state) & 0x01;
+	case 0x03:
+		encoder->dir = ((encoder->last_stable - state + 1) % 4) - 1;
 		break;
 	}
 
@@ -212,7 +213,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 +221,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 +267,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 +280,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 +297,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 +322,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 +340,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 +384,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 +397,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.6.2

Re: [PATCH RFC 0/3] input: rotary_encoder: use more than two gpios as input

[Rojhalat Ibrahim]

On Wednesday 02 December 2015 11:07:11 Uwe Kleine-König wrote:
> Hello,
> 
> some time ago I worked on the rotary encoder driver to make it support
> more than two input lines. This is the (only slightly tested[1]) rebase of
> this series on top of 4.4-rc2 (from 4.1).
> 
> It would be great to get some feedback, especially (but not only) for my
> change to raumfeld.c.
> 
> Before Ezequiel's patch 3a341a4c30d4 ("Input: rotary-encoder - add
> support for quarter-period mode") we had a dt property
> "rotary-encoder,half-period" defined. It's presence meant that we had 2
> indents per period; it's absence that there is only 1. Ezequiel
> introduced rotary-encoder,steps-per-period instead when adding support
> for quarter-period mode (which has 4 indents per period).
> 
> Up to now (i.e. with two inputs) a period has length 4. Now with (say)
> four inputs a period has length 16 instead. Now how should this be
> modeled in dt? This series implements that I have to pass
> 
> 	rotary-encoder,steps-per-period = <16>;
> 
> now for "quarter-period mode" (i.e. 4 indents per 4 state changes[2]),
> but that feels unnatural. I'd prefer to set a property to <1> instead,
> meaning the device has an indent for each state change[2]. half-period
> mode would be <2> and full-period mode would be <4>. But I don't have a
> nice name for such a property and maybe it's easier to live with
> steps-per-period = <16>? What do you think?
> 
> Also note that these patches are not as technically versatile as
> possible. With 4 (n) input lines we could detect a quick rotation where the
> machine's latency only allows to sample after 7 (2^(n-1)-1) state
> changes. Currently this is not implemented, but can be done later.
> 
> Best regards
> Uwe
> 

AFAIUI the rotary encoder driver only handles incremental encoders not
absolute encoders. (So in fact the assumed rotary encoder could also be a
linear encoder with an incremental interface.) Those encoders almost always
have an interface with two outputs (A, B) with a phase shift of 90 degrees
between them. So in this case we have 4 steps per period. Sometimes there
is only one line for 1 or 2 steps per period. But I have never seen or
heard of a device with more than 2 lines (except for the third output which
serves as a reference position index marker).

Do devices wirh more than two outputs actually exist?
Or is the purpose of supporting more than 2 lines something other than
connecting an actual encoder to them?

   Rojhalat

--
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 RFC 0/3] input: rotary_encoder: use more than two gpios as input

[Rojhalat Ibrahim]

On Wednesday 02 December 2015 11:07:11 Uwe Kleine-K?nig wrote:
> Hello,
> 
> some time ago I worked on the rotary encoder driver to make it support
> more than two input lines. This is the (only slightly tested[1]) rebase of
> this series on top of 4.4-rc2 (from 4.1).
> 
> It would be great to get some feedback, especially (but not only) for my
> change to raumfeld.c.
> 
> Before Ezequiel's patch 3a341a4c30d4 ("Input: rotary-encoder - add
> support for quarter-period mode") we had a dt property
> "rotary-encoder,half-period" defined. It's presence meant that we had 2
> indents per period; it's absence that there is only 1. Ezequiel
> introduced rotary-encoder,steps-per-period instead when adding support
> for quarter-period mode (which has 4 indents per period).
> 
> Up to now (i.e. with two inputs) a period has length 4. Now with (say)
> four inputs a period has length 16 instead. Now how should this be
> modeled in dt? This series implements that I have to pass
> 
> 	rotary-encoder,steps-per-period = <16>;
> 
> now for "quarter-period mode" (i.e. 4 indents per 4 state changes[2]),
> but that feels unnatural. I'd prefer to set a property to <1> instead,
> meaning the device has an indent for each state change[2]. half-period
> mode would be <2> and full-period mode would be <4>. But I don't have a
> nice name for such a property and maybe it's easier to live with
> steps-per-period = <16>? What do you think?
> 
> Also note that these patches are not as technically versatile as
> possible. With 4 (n) input lines we could detect a quick rotation where the
> machine's latency only allows to sample after 7 (2^(n-1)-1) state
> changes. Currently this is not implemented, but can be done later.
> 
> Best regards
> Uwe
> 

AFAIUI the rotary encoder driver only handles incremental encoders not
absolute encoders. (So in fact the assumed rotary encoder could also be a
linear encoder with an incremental interface.) Those encoders almost always
have an interface with two outputs (A, B) with a phase shift of 90 degrees
between them. So in this case we have 4 steps per period. Sometimes there
is only one line for 1 or 2 steps per period. But I have never seen or
heard of a device with more than 2 lines (except for the third output which
serves as a reference position index marker).

Do devices wirh more than two outputs actually exist?
Or is the purpose of supporting more than 2 lines something other than
connecting an actual encoder to them?

   Rojhalat

Re: [PATCH RFC 3/3] input: rotary_encoder: support more than 2 gpios as input

[Rob Herring]

On Wed, Dec 02, 2015 at 11:07:14AM +0100, Uwe Kleine-König wrote:
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> ---
>  .../devicetree/bindings/input/rotary-encoder.txt   |   2 +-

For the binding:

Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

>  arch/arm/mach-pxa/raumfeld.c                       |  25 +++-
>  drivers/input/misc/rotary_encoder.c                | 127 +++++++++------------
>  include/linux/rotary_encoder.h                     |   4 -
>  4 files changed, 79 insertions(+), 79 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 36571a9a44fe..f47e59b0efa8 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 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..99f19e037370 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;
>  	}
> @@ -134,7 +135,7 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id)
>  
>  	switch (state) {
>  	case 0x00:
> -	case 0x03:
> +	case 0x02:
>  		if (state != encoder->last_stable) {
>  			rotary_encoder_report_event(encoder);
>  			encoder->last_stable = state;
> @@ -142,8 +143,8 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id)
>  		break;
>  
>  	case 0x01:
> -	case 0x02:
> -		encoder->dir = (encoder->last_stable + state) & 0x01;
> +	case 0x03:
> +		encoder->dir = ((encoder->last_stable - state + 1) % 4) - 1;
>  		break;
>  	}
>  
> @@ -212,7 +213,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 +221,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 +267,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 +280,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 +297,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 +322,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 +340,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 +384,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 +397,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.6.2
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH RFC 3/3] input: rotary_encoder: support more than 2 gpios as input

[Rob Herring]

On Wed, Dec 02, 2015 at 11:07:14AM +0100, Uwe Kleine-K?nig wrote:
> Signed-off-by: Uwe Kleine-K?nig <u.kleine-koenig@pengutronix.de>
> ---
>  .../devicetree/bindings/input/rotary-encoder.txt   |   2 +-

For the binding:

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

>  arch/arm/mach-pxa/raumfeld.c                       |  25 +++-
>  drivers/input/misc/rotary_encoder.c                | 127 +++++++++------------
>  include/linux/rotary_encoder.h                     |   4 -
>  4 files changed, 79 insertions(+), 79 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 36571a9a44fe..f47e59b0efa8 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 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..99f19e037370 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;
>  	}
> @@ -134,7 +135,7 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id)
>  
>  	switch (state) {
>  	case 0x00:
> -	case 0x03:
> +	case 0x02:
>  		if (state != encoder->last_stable) {
>  			rotary_encoder_report_event(encoder);
>  			encoder->last_stable = state;
> @@ -142,8 +143,8 @@ static irqreturn_t rotary_encoder_half_period_irq(int irq, void *dev_id)
>  		break;
>  
>  	case 0x01:
> -	case 0x02:
> -		encoder->dir = (encoder->last_stable + state) & 0x01;
> +	case 0x03:
> +		encoder->dir = ((encoder->last_stable - state + 1) % 4) - 1;
>  		break;
>  	}
>  
> @@ -212,7 +213,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 +221,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 +267,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 +280,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 +297,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 +322,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 +340,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 +384,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 +397,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.6.2
> 

Re: [PATCH RFC 0/3] input: rotary_encoder: use more than two gpios as input

[Uwe Kleine-König]

Hello,

On Wed, Dec 02, 2015 at 01:52:58PM +0100, Rojhalat Ibrahim wrote:
> On Wednesday 02 December 2015 11:07:11 Uwe Kleine-König wrote:
> > some time ago I worked on the rotary encoder driver to make it support
> > more than two input lines. This is the (only slightly tested[1]) rebase of
> > this series on top of 4.4-rc2 (from 4.1).
> > 
> > It would be great to get some feedback, especially (but not only) for my
> > change to raumfeld.c.
> > 
> > Before Ezequiel's patch 3a341a4c30d4 ("Input: rotary-encoder - add
> > support for quarter-period mode") we had a dt property
> > "rotary-encoder,half-period" defined. It's presence meant that we had 2
> > indents per period; it's absence that there is only 1. Ezequiel
> > introduced rotary-encoder,steps-per-period instead when adding support
> > for quarter-period mode (which has 4 indents per period).
> > 
> > Up to now (i.e. with two inputs) a period has length 4. Now with (say)
> > four inputs a period has length 16 instead. Now how should this be
> > modeled in dt? This series implements that I have to pass
> > 
> > 	rotary-encoder,steps-per-period = <16>;
> > 
> > now for "quarter-period mode" (i.e. 4 indents per 4 state changes[2]),
> > but that feels unnatural. I'd prefer to set a property to <1> instead,
> > meaning the device has an indent for each state change[2]. half-period
> > mode would be <2> and full-period mode would be <4>. But I don't have a
> > nice name for such a property and maybe it's easier to live with
> > steps-per-period = <16>? What do you think?
> > 
> > Also note that these patches are not as technically versatile as
> > possible. With 4 (n) input lines we could detect a quick rotation where the
> > machine's latency only allows to sample after 7 (2^(n-1)-1) state
> > changes. Currently this is not implemented, but can be done later.
> > 
> > Best regards
> > Uwe
> > 
> 
> AFAIUI the rotary encoder driver only handles incremental encoders not
> absolute encoders. (So in fact the assumed rotary encoder could also be a

There is a device tree property "rotary-encoder,relative-axis" which
switches between absolute and relative reporting. This is maybe badly
named, but IIUC is there to differenciate between incremental and
absolute encoders.

> linear encoder with an incremental interface.) Those encoders almost always
> have an interface with two outputs (A, B) with a phase shift of 90 degrees
> between them. So in this case we have 4 steps per period. Sometimes there
> is only one line for 1 or 2 steps per period. But I have never seen or

I don't see how using only a single gpio would work for a rotary
encoder. I suspect we use different vocabulary to describe our devices
and so don't understand each other?! At least you cannot detect the
direction of the movement with a single input line, do you?

> heard of a device with more than 2 lines (except for the third output which
> serves as a reference position index marker).
> 
> Do devices wirh more than two outputs actually exist?
> Or is the purpose of supporting more than 2 lines something other than
> connecting an actual encoder to them?

I have a real rotary encoder with 4 input lines and so 16
distinguishable positions. It would be described as:

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

with the binding implemented in my patches.

The wikipedia article about rotary encoders[1] uses a device with 3
input lines to explain gray encoding. So I didn't consider "my" device
as exotic up to now.

Best regards
Uwe

[1] https://en.wikipedia.org/wiki/Rotary_encoder

-- 
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 RFC 0/3] input: rotary_encoder: use more than two gpios as input

[Uwe Kleine-König]

Hello,

On Wed, Dec 02, 2015 at 01:52:58PM +0100, Rojhalat Ibrahim wrote:
> On Wednesday 02 December 2015 11:07:11 Uwe Kleine-K?nig wrote:
> > some time ago I worked on the rotary encoder driver to make it support
> > more than two input lines. This is the (only slightly tested[1]) rebase of
> > this series on top of 4.4-rc2 (from 4.1).
> > 
> > It would be great to get some feedback, especially (but not only) for my
> > change to raumfeld.c.
> > 
> > Before Ezequiel's patch 3a341a4c30d4 ("Input: rotary-encoder - add
> > support for quarter-period mode") we had a dt property
> > "rotary-encoder,half-period" defined. It's presence meant that we had 2
> > indents per period; it's absence that there is only 1. Ezequiel
> > introduced rotary-encoder,steps-per-period instead when adding support
> > for quarter-period mode (which has 4 indents per period).
> > 
> > Up to now (i.e. with two inputs) a period has length 4. Now with (say)
> > four inputs a period has length 16 instead. Now how should this be
> > modeled in dt? This series implements that I have to pass
> > 
> > 	rotary-encoder,steps-per-period = <16>;
> > 
> > now for "quarter-period mode" (i.e. 4 indents per 4 state changes[2]),
> > but that feels unnatural. I'd prefer to set a property to <1> instead,
> > meaning the device has an indent for each state change[2]. half-period
> > mode would be <2> and full-period mode would be <4>. But I don't have a
> > nice name for such a property and maybe it's easier to live with
> > steps-per-period = <16>? What do you think?
> > 
> > Also note that these patches are not as technically versatile as
> > possible. With 4 (n) input lines we could detect a quick rotation where the
> > machine's latency only allows to sample after 7 (2^(n-1)-1) state
> > changes. Currently this is not implemented, but can be done later.
> > 
> > Best regards
> > Uwe
> > 
> 
> AFAIUI the rotary encoder driver only handles incremental encoders not
> absolute encoders. (So in fact the assumed rotary encoder could also be a

There is a device tree property "rotary-encoder,relative-axis" which
switches between absolute and relative reporting. This is maybe badly
named, but IIUC is there to differenciate between incremental and
absolute encoders.

> linear encoder with an incremental interface.) Those encoders almost always
> have an interface with two outputs (A, B) with a phase shift of 90 degrees
> between them. So in this case we have 4 steps per period. Sometimes there
> is only one line for 1 or 2 steps per period. But I have never seen or

I don't see how using only a single gpio would work for a rotary
encoder. I suspect we use different vocabulary to describe our devices
and so don't understand each other?! At least you cannot detect the
direction of the movement with a single input line, do you?

> heard of a device with more than 2 lines (except for the third output which
> serves as a reference position index marker).
> 
> Do devices wirh more than two outputs actually exist?
> Or is the purpose of supporting more than 2 lines something other than
> connecting an actual encoder to them?

I have a real rotary encoder with 4 input lines and so 16
distinguishable positions. It would be described as:

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

with the binding implemented in my patches.

The wikipedia article about rotary encoders[1] uses a device with 3
input lines to explain gray encoding. So I didn't consider "my" device
as exotic up to now.

Best regards
Uwe

[1] https://en.wikipedia.org/wiki/Rotary_encoder

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

Re: [PATCH RFC 0/3] input: rotary_encoder: use more than two gpios as input

[Rojhalat Ibrahim]

On Wednesday 02 December 2015 19:59:20 Uwe Kleine-König wrote:
> Hello,
> 
> On Wed, Dec 02, 2015 at 01:52:58PM +0100, Rojhalat Ibrahim wrote:
> > On Wednesday 02 December 2015 11:07:11 Uwe Kleine-König wrote:
> > > some time ago I worked on the rotary encoder driver to make it support
> > > more than two input lines. This is the (only slightly tested[1]) rebase of
> > > this series on top of 4.4-rc2 (from 4.1).
> > > 
> > > It would be great to get some feedback, especially (but not only) for my
> > > change to raumfeld.c.
> > > 
> > > Before Ezequiel's patch 3a341a4c30d4 ("Input: rotary-encoder - add
> > > support for quarter-period mode") we had a dt property
> > > "rotary-encoder,half-period" defined. It's presence meant that we had 2
> > > indents per period; it's absence that there is only 1. Ezequiel
> > > introduced rotary-encoder,steps-per-period instead when adding support
> > > for quarter-period mode (which has 4 indents per period).
> > > 
> > > Up to now (i.e. with two inputs) a period has length 4. Now with (say)
> > > four inputs a period has length 16 instead. Now how should this be
> > > modeled in dt? This series implements that I have to pass
> > > 
> > > 	rotary-encoder,steps-per-period = <16>;
> > > 
> > > now for "quarter-period mode" (i.e. 4 indents per 4 state changes[2]),
> > > but that feels unnatural. I'd prefer to set a property to <1> instead,
> > > meaning the device has an indent for each state change[2]. half-period
> > > mode would be <2> and full-period mode would be <4>. But I don't have a
> > > nice name for such a property and maybe it's easier to live with
> > > steps-per-period = <16>? What do you think?
> > > 
> > > Also note that these patches are not as technically versatile as
> > > possible. With 4 (n) input lines we could detect a quick rotation where the
> > > machine's latency only allows to sample after 7 (2^(n-1)-1) state
> > > changes. Currently this is not implemented, but can be done later.
> > > 
> > > Best regards
> > > Uwe
> > > 
> > 
> > AFAIUI the rotary encoder driver only handles incremental encoders not
> > absolute encoders. (So in fact the assumed rotary encoder could also be a
> 
> There is a device tree property "rotary-encoder,relative-axis" which
> switches between absolute and relative reporting. This is maybe badly
> named, but IIUC is there to differenciate between incremental and
> absolute encoders.
> 

IMHO the property switches between absolute and relative reporting but the
driver still handles only incremental encoders. Incremental encoder means that
the input lines tell you when the encoder moves and in which direction. So
by counting the steps you can generate an absolute value (which is what the
driver does). But this absolute value is always _relative_ to the position
at the time of start-up.

An absolute encoder allows you to determine its position by just looking at
the state of the input lines. There is no need to count steps.

> > linear encoder with an incremental interface.) Those encoders almost always
> > have an interface with two outputs (A, B) with a phase shift of 90 degrees
> > between them. So in this case we have 4 steps per period. Sometimes there
> > is only one line for 1 or 2 steps per period. But I have never seen or
> 
> I don't see how using only a single gpio would work for a rotary
> encoder. I suspect we use different vocabulary to describe our devices
> and so don't understand each other?! At least you cannot detect the
> direction of the movement with a single input line, do you?
> 

Correct. With a single input line you can only count steps upwards and have
no direction information. Sometimes that is sufficient.

> > heard of a device with more than 2 lines (except for the third output which
> > serves as a reference position index marker).
> > 
> > Do devices wirh more than two outputs actually exist?
> > Or is the purpose of supporting more than 2 lines something other than
> > connecting an actual encoder to them?
> 
> I have a real rotary encoder with 4 input lines and so 16
> distinguishable positions. It would be described as:
> 
> 	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>;
> 
> with the binding implemented in my patches.
> 
> The wikipedia article about rotary encoders[1] uses a device with 3
> input lines to explain gray encoding. So I didn't consider "my" device
> as exotic up to now.
> 

So you have an absolute encoder. From the state of the four input lines you
can determine the absolute position value. There is no need to count steps.
At least that's the original purpose of your device.

Of course it might be conceivable to use that kind of absolute encoder in an
incremental way and count the steps throughout multiple revolutions.
But why would you do that? The resolution of 16 steps per revolution is not
very good (incremental encoders often have hundreds or thousands of steps per
revolution). And the whole point of an absolute encoder is that you are able
to determine the current position at any time just by looking at the inputs
without having to count steps.

   Rojhalat


> Best regards
> Uwe
> 
> [1] https://en.wikipedia.org/wiki/Rotary_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 RFC 0/3] input: rotary_encoder: use more than two gpios as input

[Rojhalat Ibrahim]

On Wednesday 02 December 2015 19:59:20 Uwe Kleine-K?nig wrote:
> Hello,
> 
> On Wed, Dec 02, 2015 at 01:52:58PM +0100, Rojhalat Ibrahim wrote:
> > On Wednesday 02 December 2015 11:07:11 Uwe Kleine-K?nig wrote:
> > > some time ago I worked on the rotary encoder driver to make it support
> > > more than two input lines. This is the (only slightly tested[1]) rebase of
> > > this series on top of 4.4-rc2 (from 4.1).
> > > 
> > > It would be great to get some feedback, especially (but not only) for my
> > > change to raumfeld.c.
> > > 
> > > Before Ezequiel's patch 3a341a4c30d4 ("Input: rotary-encoder - add
> > > support for quarter-period mode") we had a dt property
> > > "rotary-encoder,half-period" defined. It's presence meant that we had 2
> > > indents per period; it's absence that there is only 1. Ezequiel
> > > introduced rotary-encoder,steps-per-period instead when adding support
> > > for quarter-period mode (which has 4 indents per period).
> > > 
> > > Up to now (i.e. with two inputs) a period has length 4. Now with (say)
> > > four inputs a period has length 16 instead. Now how should this be
> > > modeled in dt? This series implements that I have to pass
> > > 
> > > 	rotary-encoder,steps-per-period = <16>;
> > > 
> > > now for "quarter-period mode" (i.e. 4 indents per 4 state changes[2]),
> > > but that feels unnatural. I'd prefer to set a property to <1> instead,
> > > meaning the device has an indent for each state change[2]. half-period
> > > mode would be <2> and full-period mode would be <4>. But I don't have a
> > > nice name for such a property and maybe it's easier to live with
> > > steps-per-period = <16>? What do you think?
> > > 
> > > Also note that these patches are not as technically versatile as
> > > possible. With 4 (n) input lines we could detect a quick rotation where the
> > > machine's latency only allows to sample after 7 (2^(n-1)-1) state
> > > changes. Currently this is not implemented, but can be done later.
> > > 
> > > Best regards
> > > Uwe
> > > 
> > 
> > AFAIUI the rotary encoder driver only handles incremental encoders not
> > absolute encoders. (So in fact the assumed rotary encoder could also be a
> 
> There is a device tree property "rotary-encoder,relative-axis" which
> switches between absolute and relative reporting. This is maybe badly
> named, but IIUC is there to differenciate between incremental and
> absolute encoders.
> 

IMHO the property switches between absolute and relative reporting but the
driver still handles only incremental encoders. Incremental encoder means that
the input lines tell you when the encoder moves and in which direction. So
by counting the steps you can generate an absolute value (which is what the
driver does). But this absolute value is always _relative_ to the position
at the time of start-up.

An absolute encoder allows you to determine its position by just looking at
the state of the input lines. There is no need to count steps.

> > linear encoder with an incremental interface.) Those encoders almost always
> > have an interface with two outputs (A, B) with a phase shift of 90 degrees
> > between them. So in this case we have 4 steps per period. Sometimes there
> > is only one line for 1 or 2 steps per period. But I have never seen or
> 
> I don't see how using only a single gpio would work for a rotary
> encoder. I suspect we use different vocabulary to describe our devices
> and so don't understand each other?! At least you cannot detect the
> direction of the movement with a single input line, do you?
> 

Correct. With a single input line you can only count steps upwards and have
no direction information. Sometimes that is sufficient.

> > heard of a device with more than 2 lines (except for the third output which
> > serves as a reference position index marker).
> > 
> > Do devices wirh more than two outputs actually exist?
> > Or is the purpose of supporting more than 2 lines something other than
> > connecting an actual encoder to them?
> 
> I have a real rotary encoder with 4 input lines and so 16
> distinguishable positions. It would be described as:
> 
> 	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>;
> 
> with the binding implemented in my patches.
> 
> The wikipedia article about rotary encoders[1] uses a device with 3
> input lines to explain gray encoding. So I didn't consider "my" device
> as exotic up to now.
> 

So you have an absolute encoder. From the state of the four input lines you
can determine the absolute position value. There is no need to count steps.
At least that's the original purpose of your device.

Of course it might be conceivable to use that kind of absolute encoder in an
incremental way and count the steps throughout multiple revolutions.
But why would you do that? The resolution of 16 steps per revolution is not
very good (incremental encoders often have hundreds or thousands of steps per
revolution). And the whole point of an absolute encoder is that you are able
to determine the current position at any time just by looking at the inputs
without having to count steps.

   Rojhalat


> Best regards
> Uwe
> 
> [1] https://en.wikipedia.org/wiki/Rotary_encoder
> 
> 

Re: [PATCH RFC 0/3] input: rotary_encoder: use more than two gpios as input

[Daniel Mack]

On 12/03/2015 02:01 PM, Rojhalat Ibrahim wrote:
> On Wednesday 02 December 2015 19:59:20 Uwe Kleine-König wrote:

>> I have a real rotary encoder with 4 input lines and so 16
>> distinguishable positions. It would be described as:
>>
>> 	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>;
>>
>> with the binding implemented in my patches.
>>
>> The wikipedia article about rotary encoders[1] uses a device with 3
>> input lines to explain gray encoding. So I didn't consider "my" device
>> as exotic up to now.
>>
> 
> So you have an absolute encoder. From the state of the four input lines you
> can determine the absolute position value. There is no need to count steps.
> At least that's the original purpose of your device.
> 
> Of course it might be conceivable to use that kind of absolute encoder in an
> incremental way and count the steps throughout multiple revolutions.
> But why would you do that? The resolution of 16 steps per revolution is not
> very good (incremental encoders often have hundreds or thousands of steps per
> revolution). And the whole point of an absolute encoder is that you are able
> to determine the current position at any time just by looking at the inputs
> without having to count steps.

Yes, you're right, that's actually a very different kind of device which
should be supported with a different mode or something, so that the
output of the driver reflects the absolute state of the encoder. This
even simplifies things in the driver: The entire state logic would be
bypassed in this mode, and the current state of the GPIO lines would be
used directly to get the absolute value. Makes sense?


Thanks,
Daniel

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

[PATCH RFC 0/3] input: rotary_encoder: use more than two gpios as input

[Daniel Mack]

On 12/03/2015 02:01 PM, Rojhalat Ibrahim wrote:
> On Wednesday 02 December 2015 19:59:20 Uwe Kleine-K?nig wrote:

>> I have a real rotary encoder with 4 input lines and so 16
>> distinguishable positions. It would be described as:
>>
>> 	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>;
>>
>> with the binding implemented in my patches.
>>
>> The wikipedia article about rotary encoders[1] uses a device with 3
>> input lines to explain gray encoding. So I didn't consider "my" device
>> as exotic up to now.
>>
> 
> So you have an absolute encoder. From the state of the four input lines you
> can determine the absolute position value. There is no need to count steps.
> At least that's the original purpose of your device.
> 
> Of course it might be conceivable to use that kind of absolute encoder in an
> incremental way and count the steps throughout multiple revolutions.
> But why would you do that? The resolution of 16 steps per revolution is not
> very good (incremental encoders often have hundreds or thousands of steps per
> revolution). And the whole point of an absolute encoder is that you are able
> to determine the current position at any time just by looking at the inputs
> without having to count steps.

Yes, you're right, that's actually a very different kind of device which
should be supported with a different mode or something, so that the
output of the driver reflects the absolute state of the encoder. This
even simplifies things in the driver: The entire state logic would be
bypassed in this mode, and the current state of the GPIO lines would be
used directly to get the absolute value. Makes sense?


Thanks,
Daniel

Re: [PATCH RFC 0/3] input: rotary_encoder: use more than two gpios as input

[Rojhalat Ibrahim]

On Saturday 05 December 2015 20:50:18 Daniel Mack wrote:
> On 12/03/2015 02:01 PM, Rojhalat Ibrahim wrote:
> > On Wednesday 02 December 2015 19:59:20 Uwe Kleine-König wrote:
> 
> >> I have a real rotary encoder with 4 input lines and so 16
> >> distinguishable positions. It would be described as:
> >>
> >> 	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>;
> >>
> >> with the binding implemented in my patches.
> >>
> >> The wikipedia article about rotary encoders[1] uses a device with 3
> >> input lines to explain gray encoding. So I didn't consider "my" device
> >> as exotic up to now.
> >>
> > 
> > So you have an absolute encoder. From the state of the four input lines you
> > can determine the absolute position value. There is no need to count steps.
> > At least that's the original purpose of your device.
> > 
> > Of course it might be conceivable to use that kind of absolute encoder in an
> > incremental way and count the steps throughout multiple revolutions.
> > But why would you do that? The resolution of 16 steps per revolution is not
> > very good (incremental encoders often have hundreds or thousands of steps per
> > revolution). And the whole point of an absolute encoder is that you are able
> > to determine the current position at any time just by looking at the inputs
> > without having to count steps.
> 
> Yes, you're right, that's actually a very different kind of device which
> should be supported with a different mode or something, so that the
> output of the driver reflects the absolute state of the encoder. This
> even simplifies things in the driver: The entire state logic would be
> bypassed in this mode, and the current state of the GPIO lines would be
> used directly to get the absolute value. Makes sense?
> 

Definitely makes sense to me.

Thanks.
   Rojhalat


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

--
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 RFC 0/3] input: rotary_encoder: use more than two gpios as input

[Rojhalat Ibrahim]

On Saturday 05 December 2015 20:50:18 Daniel Mack wrote:
> On 12/03/2015 02:01 PM, Rojhalat Ibrahim wrote:
> > On Wednesday 02 December 2015 19:59:20 Uwe Kleine-K?nig wrote:
> 
> >> I have a real rotary encoder with 4 input lines and so 16
> >> distinguishable positions. It would be described as:
> >>
> >> 	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>;
> >>
> >> with the binding implemented in my patches.
> >>
> >> The wikipedia article about rotary encoders[1] uses a device with 3
> >> input lines to explain gray encoding. So I didn't consider "my" device
> >> as exotic up to now.
> >>
> > 
> > So you have an absolute encoder. From the state of the four input lines you
> > can determine the absolute position value. There is no need to count steps.
> > At least that's the original purpose of your device.
> > 
> > Of course it might be conceivable to use that kind of absolute encoder in an
> > incremental way and count the steps throughout multiple revolutions.
> > But why would you do that? The resolution of 16 steps per revolution is not
> > very good (incremental encoders often have hundreds or thousands of steps per
> > revolution). And the whole point of an absolute encoder is that you are able
> > to determine the current position at any time just by looking at the inputs
> > without having to count steps.
> 
> Yes, you're right, that's actually a very different kind of device which
> should be supported with a different mode or something, so that the
> output of the driver reflects the absolute state of the encoder. This
> even simplifies things in the driver: The entire state logic would be
> bypassed in this mode, and the current state of the GPIO lines would be
> used directly to get the absolute value. Makes sense?
> 

Definitely makes sense to me.

Thanks.
   Rojhalat


> 
> Thanks,
> Daniel
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel