rotary_encoder

rotary_encoder

[Christian Gmeiner]

Hi

I have a device with a rotary encoder and I got the device working with
the generic driver for rotary encoders connected to GPIO lines. Now my
goal is to send two different key codes - depending on the rotation direction.

left ->ALT + tab
right -> tab

Is there a way to extend the current encoder, which is accepted for mainline
inclusion or should I do this step in user space?

thanks
--
Christian Gmeiner, MSc

Re: rotary_encoder

[Dmitry Torokhov]

On Tuesday, May 21, 2013 10:38:44 AM Christian Gmeiner wrote:
> Hi
> 
> I have a device with a rotary encoder and I got the device working with
> the generic driver for rotary encoders connected to GPIO lines. Now my
> goal is to send two different key codes - depending on the rotation
> direction.
> 
> left ->ALT + tab
> right -> tab
> 
> Is there a way to extend the current encoder, which is accepted for mainline
> inclusion or should I do this step in user space?

Userspace please.

-- 
Dmitry

Re: rotary encoder

[Daniel Mack]

(Cc'ing the linux-input mailing list)

On Wed, Jun 02, 2010 at 05:12:49PM +0400, Dmitriy Vasil'ev wrote:
> I have founded your rotary encoder driver at the Linux kernel.
> I am sorry, but I do not understand how I can use it.
> 
> How I can read data from encoder in a user space programm?
> Do I can have input like keyboard?
> For example, I would like have key-left and key-left-alt emulation keyboard-keys while rotate counterclockwise,
> key-right and key-right-alt keyboard-keys while rotate clockwise,
> shift and enter keys emulation while pressing.
> How I can do it?

You have to register this driver as platform device in your board
support code. See arch/arm/mach-pxa/raumfeld.c for an example of how to
do that. What hardware are you working on?

Daniel

Re: rotary encoder

[Daniel Mack]

(please don't drop any Cc'ed addresses)

On Wed, Jun 02, 2010 at 05:28:35PM +0400, Dmitriy Vasil'ev wrote:
> thank you for reply,
> 
> I am using custom variant of the Atmel's board at91sam9260-ek.
> Yes, I understand how I can attach your driver to my board.
> 
> But how I can use events in user space?
> 
> Do I must using /dev/input/evenX for processing encoder's events?

Yes, once the driver is successfully probed, you should see a message in
dmesg. Can you post your dmesg output? Depending on your platform_data,
you should then get relative or absolute axis information in the
/dev/input handle the driver allocated.

> How I can translate it to keyboard codes?

Up to you, or your userspace application :)

> Sorry for my stupid questions.

No problem. Just remember to keep the mailing list in the Cc, so others
can follow and respond as well.

Thanks,
Daniel

Re: rotary encoder

[Daniel Mack]

On Thu, Jun 17, 2010 at 01:47:16PM +0400, Dmitriy Vasil'ev wrote:
> I have written advanced version of your rotary encoder driver.
> I added support for press and release encoder button.
> I think, this is will more usable.
> 
> See attach for details.
> p.s.
> I tested this code only with parameters described in rotary-encoder.txt

Thanks for sharing this. But please send such contributions as patch
against the current mainline source code. The best - however not
necessarily only possible - way is to use git for that.

Thanks,
Daniel

Re: rotary encoder

[Daniel Mack]

On Thu, Jun 17, 2010 at 02:48:08PM +0400, Dmitriy Vasil'ev wrote:
> I added .diff file.
> I am sorry, but I can do it only for kernel version 2.6.34
> See attach.

Please, just put the patch inline, not as attachment and not zipped.
That way, the patch can be commented line-by-line.

Thanks,
Daniel

Re: rotary encoder

[Dmitriy Vasil'ev]

diff -ur linux-2.6.34/Documentation/input/rotary-encoder.txt 
linux/Documentation/input/rotary-encoder.txt
--- linux-2.6.34/Documentation/input/rotary-encoder.txt Mon May 17 01:17:36 
2010
+++ linux/Documentation/input/rotary-encoder.txt Thu Jun 17 13:24:14 2010
@@ -86,16 +86,26 @@

 #define GPIO_ROTARY_A 1
 #define GPIO_ROTARY_B 2
+#define GPIO_ROTARY_S 3

 static struct rotary_encoder_platform_data my_rotary_encoder_info = {
- .steps  = 24,
- .axis  = ABS_X,
- .relative_axis = false,
- .rollover = false,
+ .steps  = 1,
+ .type  = EV_KEY, //(EV_KEY, EV_REL, EV_ABS)
  .gpio_a  = GPIO_ROTARY_A,
  .gpio_b  = GPIO_ROTARY_B,
  .inverted_a = 0,
  .inverted_b = 0,
+ .codeleft = KEY_LEFT, //(REL_X, REL_Y, ABS_X, ABS_Y, KEY_...)
+ .coderight = KEY_RIGHT, //(REL_X, REL_Y, ABS_X, ABS_Y, KEY_...)
+ .rollover = 0,
+ .button  = {
+  .code = KEY_ENTER,
+  .gpio = GPIO_ROTARY_S,
+  .active_low = 1,
+  .type = EV_KEY,
+  .debounce_interval = 10,
+  .rep = false
+ }
 };

 static struct platform_device rotary_encoder_device = {
diff -ur linux-2.6.34/drivers/input/misc/rotary_encoder.c 
linux/drivers/input/misc/rotary_encoder.c
--- linux-2.6.34/drivers/input/misc/rotary_encoder.c Mon May 17 01:17:36 
2010
+++ linux/drivers/input/misc/rotary_encoder.c Thu Jun 17 13:40:16 2010
@@ -1,257 +1,356 @@
-/*
- * rotary_encoder.c
- *
- * (c) 2009 Daniel Mack <daniel@caiaq.de>
- *
- * state machine code inspired by code from Tim Ruetz
- *
- * A generic driver for rotary encoders connected to GPIO lines.
- * See file:Documentation/input/rotary_encoder.txt for more information
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License version 2 as
- * published by the Free Software Foundation.
- */
-
-#include <linux/kernel.h>
-#include <linux/module.h>
-#include <linux/init.h>
-#include <linux/interrupt.h>
-#include <linux/input.h>
-#include <linux/device.h>
-#include <linux/platform_device.h>
-#include <linux/gpio.h>
-#include <linux/rotary_encoder.h>
-#include <linux/slab.h>
-
-#define DRV_NAME "rotary-encoder"
-
-struct rotary_encoder {
- struct input_dev *input;
- struct rotary_encoder_platform_data *pdata;
-
- unsigned int axis;
- unsigned int pos;
-
- unsigned int irq_a;
- unsigned int irq_b;
-
- bool armed;
- unsigned char dir; /* 0 - clockwise, 1 - CCW */
-};
-
-static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
-{
- struct rotary_encoder *encoder = dev_id;
- struct rotary_encoder_platform_data *pdata = encoder->pdata;
- int a = !!gpio_get_value(pdata->gpio_a);
- int b = !!gpio_get_value(pdata->gpio_b);
- int state;
-
- a ^= pdata->inverted_a;
- b ^= pdata->inverted_b;
- state = (a << 1) | b;
-
- switch (state) {
-
- case 0x0:
-  if (!encoder->armed)
-   break;
-
-  if (pdata->relative_axis) {
-   input_report_rel(encoder->input, pdata->axis,
-      encoder->dir ? -1 : 1);
-  } else {
-   unsigned int pos = encoder->pos;
-
-   if (encoder->dir) {
-    /* turning counter-clockwise */
-    if (pdata->rollover)
-     pos += pdata->steps;
-    if (pos)
-     pos--;
-   } else {
-    /* turning clockwise */
-    if (pdata->rollover || pos < pdata->steps)
-     pos++;
-   }
-   if (pdata->rollover)
-    pos %= pdata->steps;
-   encoder->pos = pos;
-   input_report_abs(encoder->input, pdata->axis,
-      encoder->pos);
-  }
-  input_sync(encoder->input);
-
-  encoder->armed = false;
-  break;
-
- case 0x1:
- case 0x2:
-  if (encoder->armed)
-   encoder->dir = state - 1;
-  break;
-
- case 0x3:
-  encoder->armed = true;
-  break;
- }
-
- return IRQ_HANDLED;
-}
-
-static int __devinit rotary_encoder_probe(struct platform_device *pdev)
-{
- struct rotary_encoder_platform_data *pdata = pdev->dev.platform_data;
- struct rotary_encoder *encoder;
- struct input_dev *input;
- int err;
-
- if (!pdata) {
-  dev_err(&pdev->dev, "missing platform data\n");
-  return -ENOENT;
- }
-
- encoder = kzalloc(sizeof(struct rotary_encoder), GFP_KERNEL);
- input = input_allocate_device();
- if (!encoder || !input) {
-  dev_err(&pdev->dev, "failed to allocate memory for device\n");
-  err = -ENOMEM;
-  goto exit_free_mem;
- }
-
- encoder->input = input;
- encoder->pdata = pdata;
- encoder->irq_a = gpio_to_irq(pdata->gpio_a);
- encoder->irq_b = gpio_to_irq(pdata->gpio_b);
-
- /* create and register the input driver */
- input->name = pdev->name;
- input->id.bustype = BUS_HOST;
- input->dev.parent = &pdev->dev;
-
- if (pdata->relative_axis) {
-  input->evbit[0] = BIT_MASK(EV_REL);
-  input->relbit[0] = BIT_MASK(pdata->axis);
- } else {
-  input->evbit[0] = BIT_MASK(EV_ABS);
-  input_set_abs_params(encoder->input,
-         pdata->axis, 0, pdata->steps, 0, 1);
- }
-
- err = input_register_device(input);
- if (err) {
-  dev_err(&pdev->dev, "failed to register input device\n");
-  goto exit_free_mem;
- }
-
- /* request the GPIOs */
- err = gpio_request(pdata->gpio_a, DRV_NAME);
- if (err) {
-  dev_err(&pdev->dev, "unable to request GPIO %d\n",
-   pdata->gpio_a);
-  goto exit_unregister_input;
- }
-
- err = gpio_direction_input(pdata->gpio_a);
- if (err) {
-  dev_err(&pdev->dev, "unable to set GPIO %d for input\n",
-   pdata->gpio_a);
-  goto exit_unregister_input;
- }
-
- err = gpio_request(pdata->gpio_b, DRV_NAME);
- if (err) {
-  dev_err(&pdev->dev, "unable to request GPIO %d\n",
-   pdata->gpio_b);
-  goto exit_free_gpio_a;
- }
-
- err = gpio_direction_input(pdata->gpio_b);
- if (err) {
-  dev_err(&pdev->dev, "unable to set GPIO %d for input\n",
-   pdata->gpio_b);
-  goto exit_free_gpio_a;
- }
-
- /* request the IRQs */
- err = request_irq(encoder->irq_a, &rotary_encoder_irq,
-     IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
-     DRV_NAME, encoder);
- if (err) {
-  dev_err(&pdev->dev, "unable to request IRQ %d\n",
-   encoder->irq_a);
-  goto exit_free_gpio_b;
- }
-
- err = request_irq(encoder->irq_b, &rotary_encoder_irq,
-     IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
-     DRV_NAME, encoder);
- if (err) {
-  dev_err(&pdev->dev, "unable to request IRQ %d\n",
-   encoder->irq_b);
-  goto exit_free_irq_a;
- }
-
- platform_set_drvdata(pdev, encoder);
-
- return 0;
-
-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_unregister_input:
- input_unregister_device(input);
- input = NULL; /* so we don't try to free it */
-exit_free_mem:
- input_free_device(input);
- kfree(encoder);
- return err;
-}
-
-static int __devexit rotary_encoder_remove(struct platform_device *pdev)
-{
- struct rotary_encoder *encoder = platform_get_drvdata(pdev);
- struct rotary_encoder_platform_data *pdata = pdev->dev.platform_data;
-
- 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);
- platform_set_drvdata(pdev, NULL);
- kfree(encoder);
-
- return 0;
-}
-
-static struct platform_driver rotary_encoder_driver = {
- .probe  = rotary_encoder_probe,
- .remove  = __devexit_p(rotary_encoder_remove),
- .driver  = {
-  .name = DRV_NAME,
-  .owner = THIS_MODULE,
- }
-};
-
-static int __init rotary_encoder_init(void)
-{
- return platform_driver_register(&rotary_encoder_driver);
-}
-
-static void __exit rotary_encoder_exit(void)
-{
- platform_driver_unregister(&rotary_encoder_driver);
-}
-
-module_init(rotary_encoder_init);
-module_exit(rotary_encoder_exit);
-
-MODULE_ALIAS("platform:" DRV_NAME);
-MODULE_DESCRIPTION("GPIO rotary encoder driver");
-MODULE_AUTHOR("Daniel Mack <daniel@caiaq.de>");
-MODULE_LICENSE("GPL v2");
-
+/*
+ * rotary_encoder.c
+ *
+ * (c) 2009 Daniel Mack <daniel@caiaq.de>
+ *
+ * state machine code inspired by code from Tim Ruetz
+ *
+ * A generic driver for rotary encoders connected to GPIO lines.
+ * See file:Documentation/input/rotary_encoder.txt for more information
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/input.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/gpio.h>
+#include <linux/rotary_encoder.h>
+#include <linux/slab.h>
+#include <linux/workqueue.h>
+#include <linux/sched.h>
+
+#define DRV_NAME "rotary-encoder"
+
+struct rotary_encoder_button_data {
+ struct rotary_encoder_button button;
+ unsigned int irq;
+ struct timer_list timer;
+ struct work_struct work;
+};
+
+struct rotary_encoder {
+ struct input_dev *input;
+ struct rotary_encoder_platform_data *pdata;
+
+ unsigned int pos;
+
+ unsigned int irq_a;
+ unsigned int irq_b;
+
+ bool armed;
+ unsigned char dir; /* 0 - clockwise, 1 - CCW */
+
+ struct rotary_encoder_button_data bdata;
+};
+
+static struct rotary_encoder* encoder;
+
+static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
+{
+ struct rotary_encoder *encoder = dev_id;
+ struct rotary_encoder_platform_data *pdata = encoder->pdata;
+ int a = !!gpio_get_value(pdata->gpio_a);
+ int b = !!gpio_get_value(pdata->gpio_b);
+ int state;
+
+ a ^= pdata->inverted_a;
+ b ^= pdata->inverted_b;
+ state = (a << 1) | b;
+
+ switch (state) {
+
+ case 0x0:
+  if (!encoder->armed)
+   break;
+
+  if (pdata->type == EV_REL) {
+   input_report_rel(encoder->input, pdata->codeleft,
+      encoder->dir ? -1 : 1);
+  } else
+  if (pdata->type == EV_ABS) {
+   unsigned int pos = encoder->pos;
+
+   if (encoder->dir) {
+    /* turning counter-clockwise */
+    if (pdata->rollover)
+     pos += pdata->steps;
+    if (pos)
+     pos--;
+   } else {
+    /* turning clockwise */
+    if (pdata->rollover || pos < pdata->steps)
+     pos++;
+   }
+   if (pdata->rollover)
+    pos %= pdata->steps;
+   encoder->pos = pos;
+   input_report_abs(encoder->input, pdata->codeleft,
+      encoder->pos);
+  } else
+  if (pdata->type == EV_KEY)
+  {
+   input_report_key(encoder->input, encoder->dir ? pdata->codeleft : 
pdata->coderight, 1);
+   input_sync(encoder->input);
+   input_report_key(encoder->input, encoder->dir ? pdata->codeleft : 
pdata->coderight, 0);
+   input_sync(encoder->input);
+  }
+
+  encoder->armed = false;
+  break;
+
+ case 0x1:
+ case 0x2:
+  if (encoder->armed)
+   encoder->dir = state - 1;
+  break;
+
+ case 0x3:
+  encoder->armed = true;
+  break;
+ }
+
+ return IRQ_HANDLED;
+}
+
+static void rotary_encoder_report_event(struct rotary_encoder *encoder)
+{
+ struct rotary_encoder_button *button = &(encoder->bdata.button);
+ struct input_dev *input = encoder->input;
+ int state = (gpio_get_value(button->gpio) ? 1 : 0) ^ button->active_low;
+
+ input_event(input, EV_KEY, button->code, !!state);
+ input_sync(input);
+}
+
+static void rotary_encoder_work_func(struct work_struct *work)
+{
+ struct rotary_encoder *encoder =
+  container_of(work, struct rotary_encoder, bdata.work);
+
+ rotary_encoder_report_event(encoder);
+}
+
+static void rotary_encoder_timer(unsigned long _data)
+{
+ struct rotary_encoder *encoder = (struct rotary_encoder *)_data;
+
+ schedule_work(&encoder->bdata.work);
+}
+
+static irqreturn_t rotary_encoder_key_irq(int irq, void *dev_id)
+{
+ struct rotary_encoder *encoder = dev_id;
+ struct rotary_encoder_button_data *bdata = &encoder->bdata;
+ struct rotary_encoder_button *button = &bdata->button;
+
+ if (button->debounce_interval)
+  mod_timer(&bdata->timer,
+   jiffies + msecs_to_jiffies(button->debounce_interval));
+ else
+  schedule_work(&bdata->work);
+
+ return IRQ_HANDLED;
+}
+
+static int __devinit rotary_encoder_probe(struct platform_device *pdev)
+{
+ struct rotary_encoder_platform_data *pdata = pdev->dev.platform_data;
+ struct rotary_encoder *encoder;
+ struct input_dev *input;
+ int err;
+
+ if (!pdata) {
+  dev_err(&pdev->dev, "missing platform data\n");
+  return -ENOENT;
+ }
+
+ encoder = kzalloc(sizeof(struct rotary_encoder), GFP_KERNEL);
+ input = input_allocate_device();
+ if (!encoder || !input) {
+  dev_err(&pdev->dev, "failed to allocate memory for device\n");
+  err = -ENOMEM;
+  goto exit_free_mem;
+ }
+
+ encoder->input = input;
+ encoder->pdata = pdata;
+ encoder->irq_a = gpio_to_irq(pdata->gpio_a);
+ encoder->irq_b = gpio_to_irq(pdata->gpio_b);
+ encoder->bdata.irq = gpio_to_irq(pdata->button.gpio);
+
+ /* create and register the input driver */
+ input->name = pdev->name;
+ input->id.bustype = BUS_HOST;
+ input->dev.parent = &pdev->dev;
+
+ /* Enable auto repeat feature of Linux input subsystem */
+ if (pdata->button.rep)
+  set_bit(EV_REP, input->evbit);
+
+ encoder->bdata.button = pdata->button;
+ //input_set_capability(input, EV_KEY, bdata->button.code);
+
+ set_bit(pdata->type, input->evbit);
+ set_bit(pdata->codeleft, input->keybit);
+ set_bit(pdata->coderight, input->keybit);
+ //
+ set_bit(encoder->bdata.button.type, input->evbit);
+ set_bit(encoder->bdata.button.code, input->keybit);
+
+ setup_timer(&encoder->bdata.timer, rotary_encoder_timer, (unsigned 
long)encoder);
+ INIT_WORK(&encoder->bdata.work, rotary_encoder_work_func);
+
+ err = input_register_device(input);
+ if (err) {
+  dev_err(&pdev->dev, "failed to register input device\n");
+  goto exit_free_mem;
+ }
+
+ /* request the GPIOs */
+ err = gpio_request(pdata->gpio_a, DRV_NAME);
+ if (err) {
+  dev_err(&pdev->dev, "unable to request GPIO %d\n",
+   pdata->gpio_a);
+  goto exit_unregister_input;
+ }
+
+ err = gpio_direction_input(pdata->gpio_a);
+ if (err) {
+  dev_err(&pdev->dev, "unable to set GPIO %d for input\n",
+   pdata->gpio_a);
+  goto exit_unregister_input;
+ }
+
+ err = gpio_request(pdata->gpio_b, DRV_NAME);
+ if (err) {
+  dev_err(&pdev->dev, "unable to request GPIO %d\n",
+   pdata->gpio_b);
+  goto exit_free_gpio_a;
+ }
+
+ err = gpio_direction_input(pdata->gpio_b);
+ if (err) {
+  dev_err(&pdev->dev, "unable to set GPIO %d for input\n",
+   pdata->gpio_b);
+  goto exit_free_gpio_a;
+ }
+
+ err = gpio_request(pdata->button.gpio, DRV_NAME);
+ if (err) {
+  dev_err(&pdev->dev, "unable to request GPIO %d\n",
+   pdata->button.gpio);
+  goto exit_free_gpio_b;
+ }
+
+ err = gpio_direction_input(pdata->button.gpio);
+ if (err) {
+  dev_err(&pdev->dev, "unable to set GPIO %d for input\n",
+   pdata->button.gpio);
+  goto exit_free_gpio_b;
+ }
+
+ /* request the IRQs */
+ err = request_irq(encoder->irq_a, &rotary_encoder_irq,
+     IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
+     DRV_NAME, encoder);
+ if (err) {
+  dev_err(&pdev->dev, "unable to request IRQ %d\n",
+   encoder->irq_a);
+  goto exit_free_gpio_s;
+ }
+
+ err = request_irq(encoder->irq_b, &rotary_encoder_irq,
+     IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
+     DRV_NAME, encoder);
+ if (err) {
+  dev_err(&pdev->dev, "unable to request IRQ %d\n",
+   encoder->irq_b);
+  goto exit_free_irq_a;
+ }
+
+ err = request_irq(encoder->bdata.irq, &rotary_encoder_key_irq,
+     IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
+     DRV_NAME, encoder);
+ if (err) {
+  dev_err(&pdev->dev, "unable to request IRQ %d\n",
+   encoder->bdata.irq);
+  goto exit_free_irq_b;
+ }
+
+ 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_s:
+ gpio_free(pdata->button.gpio);
+exit_free_gpio_b:
+ gpio_free(pdata->gpio_b);
+exit_free_gpio_a:
+ gpio_free(pdata->gpio_a);
+exit_unregister_input:
+ input_unregister_device(input);
+ input = NULL; /* so we don't try to free it */
+exit_free_mem:
+ input_free_device(input);
+ kfree(encoder);
+ return err;
+}
+
+static int __devexit rotary_encoder_remove(struct platform_device *pdev)
+{
+ struct rotary_encoder *encoder = platform_get_drvdata(pdev);
+ struct rotary_encoder_platform_data *pdata = pdev->dev.platform_data;
+
+ free_irq(encoder->irq_a, encoder);
+ free_irq(encoder->irq_b, encoder);
+ free_irq(encoder->bdata.irq, encoder);
+ if (pdata->button.debounce_interval)
+  del_timer_sync(&encoder->bdata.timer);
+ cancel_work_sync(&encoder->bdata.work);
+ gpio_free(pdata->gpio_a);
+ gpio_free(pdata->gpio_b);
+ gpio_free(pdata->button.gpio);
+ input_unregister_device(encoder->input);
+ platform_set_drvdata(pdev, NULL);
+ kfree(encoder);
+
+ return 0;
+}
+
+static struct platform_driver rotary_encoder_driver = {
+ .probe  = rotary_encoder_probe,
+ .remove  = __devexit_p(rotary_encoder_remove),
+ .driver  = {
+  .name = DRV_NAME,
+  .owner = THIS_MODULE,
+ }
+};
+
+static int __init rotary_encoder_init(void)
+{
+ return platform_driver_register(&rotary_encoder_driver);
+}
+
+static void __exit rotary_encoder_exit(void)
+{
+ platform_driver_unregister(&rotary_encoder_driver);
+}
+
+module_init(rotary_encoder_init);
+module_exit(rotary_encoder_exit);
+
+MODULE_ALIAS("platform:" DRV_NAME);
+MODULE_DESCRIPTION("GPIO rotary encoder driver");
+MODULE_AUTHOR("Daniel Mack <daniel@caiaq.de>");
+MODULE_LICENSE("GPL v2");
+
diff -ur linux-2.6.34/include/linux/rotary_encoder.h 
linux/include/linux/rotary_encoder.h
--- linux-2.6.34/include/linux/rotary_encoder.h Mon May 17 01:17:36 2010
+++ linux/include/linux/rotary_encoder.h Thu Jun 17 14:23:26 2010
@@ -1,15 +1,28 @@
 #ifndef __ROTARY_ENCODER_H__
 #define __ROTARY_ENCODER_H__

+struct rotary_encoder_button {
+ /* Configuration parameters */
+ u16 code;  /* input event code (KEY_*, SW_*) */
+ int gpio;
+ int active_low;
+ //char *desc;
+ u16 type;  /* input event type (EV_KEY, EV_SW) */
+ int debounce_interval; /* debounce ticks interval in msecs */
+ bool rep;/* enable input subsystem auto repeat */
+};
+
 struct rotary_encoder_platform_data {
  unsigned int steps;
- unsigned int axis;
+ u16 type; /*(EV_KEY, EV_REL, EV_ABS)*/
  unsigned int gpio_a;
  unsigned int gpio_b;
  unsigned int inverted_a;
  unsigned int inverted_b;
- bool relative_axis;
+ u16 codeleft; /*(REL_X, REL_Y, ABS_X, ABS_Y, KEY_...)*/
+ u16 coderight; /*(REL_X, REL_Y, ABS_X, ABS_Y, KEY_...)*/
  bool rollover;
+ struct rotary_encoder_button button;
 };

 #endif /* __ROTARY_ENCODER_H__ */

----- Original Message ----- 
From: "Daniel Mack" <daniel@caiaq.de>
To: "Dmitriy Vasil'ev" <tech@digiton.ru>
Cc: <linux-input@vger.kernel.org>
Sent: Thursday, June 17, 2010 3:21 PM
Subject: Re: rotary encoder


> On Thu, Jun 17, 2010 at 02:48:08PM +0400, Dmitriy Vasil'ev wrote:
>> I added .diff file.
>> I am sorry, but I can do it only for kernel version 2.6.34
>> See attach.
>
> Please, just put the patch inline, not as attachment and not zipped.
> That way, the patch can be commented line-by-line.
>
> Thanks,
> Daniel 

Re: rotary encoder

[Daniel Mack]

Hi Dmitriy,

Sorry, but your patch can't be reviewed that way. It removes the whole
driver and adds a complete new version, which is not what a patch should
normally do.

Could you please check out the latest sources from git, copy your
version of the driver to the tree and then run 'git diff'?

This howto can give you a good start:

  http://linux.yyz.us/git-howto.html


Also, please have a look at the file Documentation/CodingStyle,
especially because your indentation and comments are not as they should
be.

I'd be happy to give feedback, but there are some rules to follow to
make that process feasible.

Thanks,
Daniel


On Thu, Jun 17, 2010 at 03:29:51PM +0400, Dmitriy Vasil'ev wrote:
> diff -ur linux-2.6.34/Documentation/input/rotary-encoder.txt
> linux/Documentation/input/rotary-encoder.txt
> --- linux-2.6.34/Documentation/input/rotary-encoder.txt Mon May 17
> 01:17:36 2010
> +++ linux/Documentation/input/rotary-encoder.txt Thu Jun 17 13:24:14 2010
> @@ -86,16 +86,26 @@
> 
> #define GPIO_ROTARY_A 1
> #define GPIO_ROTARY_B 2
> +#define GPIO_ROTARY_S 3
> 
> static struct rotary_encoder_platform_data my_rotary_encoder_info = {
> - .steps  = 24,
> - .axis  = ABS_X,
> - .relative_axis = false,
> - .rollover = false,
> + .steps  = 1,
> + .type  = EV_KEY, //(EV_KEY, EV_REL, EV_ABS)
>  .gpio_a  = GPIO_ROTARY_A,
>  .gpio_b  = GPIO_ROTARY_B,
>  .inverted_a = 0,
>  .inverted_b = 0,
> + .codeleft = KEY_LEFT, //(REL_X, REL_Y, ABS_X, ABS_Y, KEY_...)
> + .coderight = KEY_RIGHT, //(REL_X, REL_Y, ABS_X, ABS_Y, KEY_...)
> + .rollover = 0,
> + .button  = {
> +  .code = KEY_ENTER,
> +  .gpio = GPIO_ROTARY_S,
> +  .active_low = 1,
> +  .type = EV_KEY,
> +  .debounce_interval = 10,
> +  .rep = false
> + }
> };
> 
> static struct platform_device rotary_encoder_device = {
> diff -ur linux-2.6.34/drivers/input/misc/rotary_encoder.c
> linux/drivers/input/misc/rotary_encoder.c
> --- linux-2.6.34/drivers/input/misc/rotary_encoder.c Mon May 17
> 01:17:36 2010
> +++ linux/drivers/input/misc/rotary_encoder.c Thu Jun 17 13:40:16 2010
> @@ -1,257 +1,356 @@
> -/*
> - * rotary_encoder.c
> - *
> - * (c) 2009 Daniel Mack <daniel@caiaq.de>
> - *
> - * state machine code inspired by code from Tim Ruetz
> - *
> - * A generic driver for rotary encoders connected to GPIO lines.
> - * See file:Documentation/input/rotary_encoder.txt for more information
> - *
> - * This program is free software; you can redistribute it and/or modify
> - * it under the terms of the GNU General Public License version 2 as
> - * published by the Free Software Foundation.
> - */
> -
> -#include <linux/kernel.h>
> -#include <linux/module.h>
> -#include <linux/init.h>
> -#include <linux/interrupt.h>
> -#include <linux/input.h>
> -#include <linux/device.h>
> -#include <linux/platform_device.h>
> -#include <linux/gpio.h>
> -#include <linux/rotary_encoder.h>
> -#include <linux/slab.h>
> -
> -#define DRV_NAME "rotary-encoder"
> -
> -struct rotary_encoder {
> - struct input_dev *input;
> - struct rotary_encoder_platform_data *pdata;
> -
> - unsigned int axis;
> - unsigned int pos;
> -
> - unsigned int irq_a;
> - unsigned int irq_b;
> -
> - bool armed;
> - unsigned char dir; /* 0 - clockwise, 1 - CCW */
> -};
> -
> -static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
> -{
> - struct rotary_encoder *encoder = dev_id;
> - struct rotary_encoder_platform_data *pdata = encoder->pdata;
> - int a = !!gpio_get_value(pdata->gpio_a);
> - int b = !!gpio_get_value(pdata->gpio_b);
> - int state;
> -
> - a ^= pdata->inverted_a;
> - b ^= pdata->inverted_b;
> - state = (a << 1) | b;
> -
> - switch (state) {
> -
> - case 0x0:
> -  if (!encoder->armed)
> -   break;
> -
> -  if (pdata->relative_axis) {
> -   input_report_rel(encoder->input, pdata->axis,
> -      encoder->dir ? -1 : 1);
> -  } else {
> -   unsigned int pos = encoder->pos;
> -
> -   if (encoder->dir) {
> -    /* turning counter-clockwise */
> -    if (pdata->rollover)
> -     pos += pdata->steps;
> -    if (pos)
> -     pos--;
> -   } else {
> -    /* turning clockwise */
> -    if (pdata->rollover || pos < pdata->steps)
> -     pos++;
> -   }
> -   if (pdata->rollover)
> -    pos %= pdata->steps;
> -   encoder->pos = pos;
> -   input_report_abs(encoder->input, pdata->axis,
> -      encoder->pos);
> -  }
> -  input_sync(encoder->input);
> -
> -  encoder->armed = false;
> -  break;
> -
> - case 0x1:
> - case 0x2:
> -  if (encoder->armed)
> -   encoder->dir = state - 1;
> -  break;
> -
> - case 0x3:
> -  encoder->armed = true;
> -  break;
> - }
> -
> - return IRQ_HANDLED;
> -}
> -
> -static int __devinit rotary_encoder_probe(struct platform_device *pdev)
> -{
> - struct rotary_encoder_platform_data *pdata = pdev->dev.platform_data;
> - struct rotary_encoder *encoder;
> - struct input_dev *input;
> - int err;
> -
> - if (!pdata) {
> -  dev_err(&pdev->dev, "missing platform data\n");
> -  return -ENOENT;
> - }
> -
> - encoder = kzalloc(sizeof(struct rotary_encoder), GFP_KERNEL);
> - input = input_allocate_device();
> - if (!encoder || !input) {
> -  dev_err(&pdev->dev, "failed to allocate memory for device\n");
> -  err = -ENOMEM;
> -  goto exit_free_mem;
> - }
> -
> - encoder->input = input;
> - encoder->pdata = pdata;
> - encoder->irq_a = gpio_to_irq(pdata->gpio_a);
> - encoder->irq_b = gpio_to_irq(pdata->gpio_b);
> -
> - /* create and register the input driver */
> - input->name = pdev->name;
> - input->id.bustype = BUS_HOST;
> - input->dev.parent = &pdev->dev;
> -
> - if (pdata->relative_axis) {
> -  input->evbit[0] = BIT_MASK(EV_REL);
> -  input->relbit[0] = BIT_MASK(pdata->axis);
> - } else {
> -  input->evbit[0] = BIT_MASK(EV_ABS);
> -  input_set_abs_params(encoder->input,
> -         pdata->axis, 0, pdata->steps, 0, 1);
> - }
> -
> - err = input_register_device(input);
> - if (err) {
> -  dev_err(&pdev->dev, "failed to register input device\n");
> -  goto exit_free_mem;
> - }
> -
> - /* request the GPIOs */
> - err = gpio_request(pdata->gpio_a, DRV_NAME);
> - if (err) {
> -  dev_err(&pdev->dev, "unable to request GPIO %d\n",
> -   pdata->gpio_a);
> -  goto exit_unregister_input;
> - }
> -
> - err = gpio_direction_input(pdata->gpio_a);
> - if (err) {
> -  dev_err(&pdev->dev, "unable to set GPIO %d for input\n",
> -   pdata->gpio_a);
> -  goto exit_unregister_input;
> - }
> -
> - err = gpio_request(pdata->gpio_b, DRV_NAME);
> - if (err) {
> -  dev_err(&pdev->dev, "unable to request GPIO %d\n",
> -   pdata->gpio_b);
> -  goto exit_free_gpio_a;
> - }
> -
> - err = gpio_direction_input(pdata->gpio_b);
> - if (err) {
> -  dev_err(&pdev->dev, "unable to set GPIO %d for input\n",
> -   pdata->gpio_b);
> -  goto exit_free_gpio_a;
> - }
> -
> - /* request the IRQs */
> - err = request_irq(encoder->irq_a, &rotary_encoder_irq,
> -     IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
> -     DRV_NAME, encoder);
> - if (err) {
> -  dev_err(&pdev->dev, "unable to request IRQ %d\n",
> -   encoder->irq_a);
> -  goto exit_free_gpio_b;
> - }
> -
> - err = request_irq(encoder->irq_b, &rotary_encoder_irq,
> -     IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
> -     DRV_NAME, encoder);
> - if (err) {
> -  dev_err(&pdev->dev, "unable to request IRQ %d\n",
> -   encoder->irq_b);
> -  goto exit_free_irq_a;
> - }
> -
> - platform_set_drvdata(pdev, encoder);
> -
> - return 0;
> -
> -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_unregister_input:
> - input_unregister_device(input);
> - input = NULL; /* so we don't try to free it */
> -exit_free_mem:
> - input_free_device(input);
> - kfree(encoder);
> - return err;
> -}
> -
> -static int __devexit rotary_encoder_remove(struct platform_device *pdev)
> -{
> - struct rotary_encoder *encoder = platform_get_drvdata(pdev);
> - struct rotary_encoder_platform_data *pdata = pdev->dev.platform_data;
> -
> - 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);
> - platform_set_drvdata(pdev, NULL);
> - kfree(encoder);
> -
> - return 0;
> -}
> -
> -static struct platform_driver rotary_encoder_driver = {
> - .probe  = rotary_encoder_probe,
> - .remove  = __devexit_p(rotary_encoder_remove),
> - .driver  = {
> -  .name = DRV_NAME,
> -  .owner = THIS_MODULE,
> - }
> -};
> -
> -static int __init rotary_encoder_init(void)
> -{
> - return platform_driver_register(&rotary_encoder_driver);
> -}
> -
> -static void __exit rotary_encoder_exit(void)
> -{
> - platform_driver_unregister(&rotary_encoder_driver);
> -}
> -
> -module_init(rotary_encoder_init);
> -module_exit(rotary_encoder_exit);
> -
> -MODULE_ALIAS("platform:" DRV_NAME);
> -MODULE_DESCRIPTION("GPIO rotary encoder driver");
> -MODULE_AUTHOR("Daniel Mack <daniel@caiaq.de>");
> -MODULE_LICENSE("GPL v2");
> -
> +/*
> + * rotary_encoder.c
> + *
> + * (c) 2009 Daniel Mack <daniel@caiaq.de>
> + *
> + * state machine code inspired by code from Tim Ruetz
> + *
> + * A generic driver for rotary encoders connected to GPIO lines.
> + * See file:Documentation/input/rotary_encoder.txt for more information
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/interrupt.h>
> +#include <linux/input.h>
> +#include <linux/device.h>
> +#include <linux/platform_device.h>
> +#include <linux/gpio.h>
> +#include <linux/rotary_encoder.h>
> +#include <linux/slab.h>
> +#include <linux/workqueue.h>
> +#include <linux/sched.h>
> +
> +#define DRV_NAME "rotary-encoder"
> +
> +struct rotary_encoder_button_data {
> + struct rotary_encoder_button button;
> + unsigned int irq;
> + struct timer_list timer;
> + struct work_struct work;
> +};
> +
> +struct rotary_encoder {
> + struct input_dev *input;
> + struct rotary_encoder_platform_data *pdata;
> +
> + unsigned int pos;
> +
> + unsigned int irq_a;
> + unsigned int irq_b;
> +
> + bool armed;
> + unsigned char dir; /* 0 - clockwise, 1 - CCW */
> +
> + struct rotary_encoder_button_data bdata;
> +};
> +
> +static struct rotary_encoder* encoder;
> +
> +static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
> +{
> + struct rotary_encoder *encoder = dev_id;
> + struct rotary_encoder_platform_data *pdata = encoder->pdata;
> + int a = !!gpio_get_value(pdata->gpio_a);
> + int b = !!gpio_get_value(pdata->gpio_b);
> + int state;
> +
> + a ^= pdata->inverted_a;
> + b ^= pdata->inverted_b;
> + state = (a << 1) | b;
> +
> + switch (state) {
> +
> + case 0x0:
> +  if (!encoder->armed)
> +   break;
> +
> +  if (pdata->type == EV_REL) {
> +   input_report_rel(encoder->input, pdata->codeleft,
> +      encoder->dir ? -1 : 1);
> +  } else
> +  if (pdata->type == EV_ABS) {
> +   unsigned int pos = encoder->pos;
> +
> +   if (encoder->dir) {
> +    /* turning counter-clockwise */
> +    if (pdata->rollover)
> +     pos += pdata->steps;
> +    if (pos)
> +     pos--;
> +   } else {
> +    /* turning clockwise */
> +    if (pdata->rollover || pos < pdata->steps)
> +     pos++;
> +   }
> +   if (pdata->rollover)
> +    pos %= pdata->steps;
> +   encoder->pos = pos;
> +   input_report_abs(encoder->input, pdata->codeleft,
> +      encoder->pos);
> +  } else
> +  if (pdata->type == EV_KEY)
> +  {
> +   input_report_key(encoder->input, encoder->dir ? pdata->codeleft
> : pdata->coderight, 1);
> +   input_sync(encoder->input);
> +   input_report_key(encoder->input, encoder->dir ? pdata->codeleft
> : pdata->coderight, 0);
> +   input_sync(encoder->input);
> +  }
> +
> +  encoder->armed = false;
> +  break;
> +
> + case 0x1:
> + case 0x2:
> +  if (encoder->armed)
> +   encoder->dir = state - 1;
> +  break;
> +
> + case 0x3:
> +  encoder->armed = true;
> +  break;
> + }
> +
> + return IRQ_HANDLED;
> +}
> +
> +static void rotary_encoder_report_event(struct rotary_encoder *encoder)
> +{
> + struct rotary_encoder_button *button = &(encoder->bdata.button);
> + struct input_dev *input = encoder->input;
> + int state = (gpio_get_value(button->gpio) ? 1 : 0) ^ button->active_low;
> +
> + input_event(input, EV_KEY, button->code, !!state);
> + input_sync(input);
> +}
> +
> +static void rotary_encoder_work_func(struct work_struct *work)
> +{
> + struct rotary_encoder *encoder =
> +  container_of(work, struct rotary_encoder, bdata.work);
> +
> + rotary_encoder_report_event(encoder);
> +}
> +
> +static void rotary_encoder_timer(unsigned long _data)
> +{
> + struct rotary_encoder *encoder = (struct rotary_encoder *)_data;
> +
> + schedule_work(&encoder->bdata.work);
> +}
> +
> +static irqreturn_t rotary_encoder_key_irq(int irq, void *dev_id)
> +{
> + struct rotary_encoder *encoder = dev_id;
> + struct rotary_encoder_button_data *bdata = &encoder->bdata;
> + struct rotary_encoder_button *button = &bdata->button;
> +
> + if (button->debounce_interval)
> +  mod_timer(&bdata->timer,
> +   jiffies + msecs_to_jiffies(button->debounce_interval));
> + else
> +  schedule_work(&bdata->work);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int __devinit rotary_encoder_probe(struct platform_device *pdev)
> +{
> + struct rotary_encoder_platform_data *pdata = pdev->dev.platform_data;
> + struct rotary_encoder *encoder;
> + struct input_dev *input;
> + int err;
> +
> + if (!pdata) {
> +  dev_err(&pdev->dev, "missing platform data\n");
> +  return -ENOENT;
> + }
> +
> + encoder = kzalloc(sizeof(struct rotary_encoder), GFP_KERNEL);
> + input = input_allocate_device();
> + if (!encoder || !input) {
> +  dev_err(&pdev->dev, "failed to allocate memory for device\n");
> +  err = -ENOMEM;
> +  goto exit_free_mem;
> + }
> +
> + encoder->input = input;
> + encoder->pdata = pdata;
> + encoder->irq_a = gpio_to_irq(pdata->gpio_a);
> + encoder->irq_b = gpio_to_irq(pdata->gpio_b);
> + encoder->bdata.irq = gpio_to_irq(pdata->button.gpio);
> +
> + /* create and register the input driver */
> + input->name = pdev->name;
> + input->id.bustype = BUS_HOST;
> + input->dev.parent = &pdev->dev;
> +
> + /* Enable auto repeat feature of Linux input subsystem */
> + if (pdata->button.rep)
> +  set_bit(EV_REP, input->evbit);
> +
> + encoder->bdata.button = pdata->button;
> + //input_set_capability(input, EV_KEY, bdata->button.code);
> +
> + set_bit(pdata->type, input->evbit);
> + set_bit(pdata->codeleft, input->keybit);
> + set_bit(pdata->coderight, input->keybit);
> + //
> + set_bit(encoder->bdata.button.type, input->evbit);
> + set_bit(encoder->bdata.button.code, input->keybit);
> +
> + setup_timer(&encoder->bdata.timer, rotary_encoder_timer, (unsigned
> long)encoder);
> + INIT_WORK(&encoder->bdata.work, rotary_encoder_work_func);
> +
> + err = input_register_device(input);
> + if (err) {
> +  dev_err(&pdev->dev, "failed to register input device\n");
> +  goto exit_free_mem;
> + }
> +
> + /* request the GPIOs */
> + err = gpio_request(pdata->gpio_a, DRV_NAME);
> + if (err) {
> +  dev_err(&pdev->dev, "unable to request GPIO %d\n",
> +   pdata->gpio_a);
> +  goto exit_unregister_input;
> + }
> +
> + err = gpio_direction_input(pdata->gpio_a);
> + if (err) {
> +  dev_err(&pdev->dev, "unable to set GPIO %d for input\n",
> +   pdata->gpio_a);
> +  goto exit_unregister_input;
> + }
> +
> + err = gpio_request(pdata->gpio_b, DRV_NAME);
> + if (err) {
> +  dev_err(&pdev->dev, "unable to request GPIO %d\n",
> +   pdata->gpio_b);
> +  goto exit_free_gpio_a;
> + }
> +
> + err = gpio_direction_input(pdata->gpio_b);
> + if (err) {
> +  dev_err(&pdev->dev, "unable to set GPIO %d for input\n",
> +   pdata->gpio_b);
> +  goto exit_free_gpio_a;
> + }
> +
> + err = gpio_request(pdata->button.gpio, DRV_NAME);
> + if (err) {
> +  dev_err(&pdev->dev, "unable to request GPIO %d\n",
> +   pdata->button.gpio);
> +  goto exit_free_gpio_b;
> + }
> +
> + err = gpio_direction_input(pdata->button.gpio);
> + if (err) {
> +  dev_err(&pdev->dev, "unable to set GPIO %d for input\n",
> +   pdata->button.gpio);
> +  goto exit_free_gpio_b;
> + }
> +
> + /* request the IRQs */
> + err = request_irq(encoder->irq_a, &rotary_encoder_irq,
> +     IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
> +     DRV_NAME, encoder);
> + if (err) {
> +  dev_err(&pdev->dev, "unable to request IRQ %d\n",
> +   encoder->irq_a);
> +  goto exit_free_gpio_s;
> + }
> +
> + err = request_irq(encoder->irq_b, &rotary_encoder_irq,
> +     IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
> +     DRV_NAME, encoder);
> + if (err) {
> +  dev_err(&pdev->dev, "unable to request IRQ %d\n",
> +   encoder->irq_b);
> +  goto exit_free_irq_a;
> + }
> +
> + err = request_irq(encoder->bdata.irq, &rotary_encoder_key_irq,
> +     IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
> +     DRV_NAME, encoder);
> + if (err) {
> +  dev_err(&pdev->dev, "unable to request IRQ %d\n",
> +   encoder->bdata.irq);
> +  goto exit_free_irq_b;
> + }
> +
> + 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_s:
> + gpio_free(pdata->button.gpio);
> +exit_free_gpio_b:
> + gpio_free(pdata->gpio_b);
> +exit_free_gpio_a:
> + gpio_free(pdata->gpio_a);
> +exit_unregister_input:
> + input_unregister_device(input);
> + input = NULL; /* so we don't try to free it */
> +exit_free_mem:
> + input_free_device(input);
> + kfree(encoder);
> + return err;
> +}
> +
> +static int __devexit rotary_encoder_remove(struct platform_device *pdev)
> +{
> + struct rotary_encoder *encoder = platform_get_drvdata(pdev);
> + struct rotary_encoder_platform_data *pdata = pdev->dev.platform_data;
> +
> + free_irq(encoder->irq_a, encoder);
> + free_irq(encoder->irq_b, encoder);
> + free_irq(encoder->bdata.irq, encoder);
> + if (pdata->button.debounce_interval)
> +  del_timer_sync(&encoder->bdata.timer);
> + cancel_work_sync(&encoder->bdata.work);
> + gpio_free(pdata->gpio_a);
> + gpio_free(pdata->gpio_b);
> + gpio_free(pdata->button.gpio);
> + input_unregister_device(encoder->input);
> + platform_set_drvdata(pdev, NULL);
> + kfree(encoder);
> +
> + return 0;
> +}
> +
> +static struct platform_driver rotary_encoder_driver = {
> + .probe  = rotary_encoder_probe,
> + .remove  = __devexit_p(rotary_encoder_remove),
> + .driver  = {
> +  .name = DRV_NAME,
> +  .owner = THIS_MODULE,
> + }
> +};
> +
> +static int __init rotary_encoder_init(void)
> +{
> + return platform_driver_register(&rotary_encoder_driver);
> +}
> +
> +static void __exit rotary_encoder_exit(void)
> +{
> + platform_driver_unregister(&rotary_encoder_driver);
> +}
> +
> +module_init(rotary_encoder_init);
> +module_exit(rotary_encoder_exit);
> +
> +MODULE_ALIAS("platform:" DRV_NAME);
> +MODULE_DESCRIPTION("GPIO rotary encoder driver");
> +MODULE_AUTHOR("Daniel Mack <daniel@caiaq.de>");
> +MODULE_LICENSE("GPL v2");
> +
> diff -ur linux-2.6.34/include/linux/rotary_encoder.h
> linux/include/linux/rotary_encoder.h
> --- linux-2.6.34/include/linux/rotary_encoder.h Mon May 17 01:17:36 2010
> +++ linux/include/linux/rotary_encoder.h Thu Jun 17 14:23:26 2010
> @@ -1,15 +1,28 @@
> #ifndef __ROTARY_ENCODER_H__
> #define __ROTARY_ENCODER_H__
> 
> +struct rotary_encoder_button {
> + /* Configuration parameters */
> + u16 code;  /* input event code (KEY_*, SW_*) */
> + int gpio;
> + int active_low;
> + //char *desc;
> + u16 type;  /* input event type (EV_KEY, EV_SW) */
> + int debounce_interval; /* debounce ticks interval in msecs */
> + bool rep;/* enable input subsystem auto repeat */
> +};
> +
> struct rotary_encoder_platform_data {
>  unsigned int steps;
> - unsigned int axis;
> + u16 type; /*(EV_KEY, EV_REL, EV_ABS)*/
>  unsigned int gpio_a;
>  unsigned int gpio_b;
>  unsigned int inverted_a;
>  unsigned int inverted_b;
> - bool relative_axis;
> + u16 codeleft; /*(REL_X, REL_Y, ABS_X, ABS_Y, KEY_...)*/
> + u16 coderight; /*(REL_X, REL_Y, ABS_X, ABS_Y, KEY_...)*/
>  bool rollover;
> + struct rotary_encoder_button button;
> };
> 
> #endif /* __ROTARY_ENCODER_H__ */
> 
> ----- Original Message ----- From: "Daniel Mack" <daniel@caiaq.de>
> To: "Dmitriy Vasil'ev" <tech@digiton.ru>
> Cc: <linux-input@vger.kernel.org>
> Sent: Thursday, June 17, 2010 3:21 PM
> Subject: Re: rotary encoder
> 
> 
> >On Thu, Jun 17, 2010 at 02:48:08PM +0400, Dmitriy Vasil'ev wrote:
> >>I added .diff file.
> >>I am sorry, but I can do it only for kernel version 2.6.34
> >>See attach.
> >
> >Please, just put the patch inline, not as attachment and not zipped.
> >That way, the patch can be commented line-by-line.
> >
> >Thanks,
> >Daniel
> 

Re: rotary encoder

[Dmitriy Vasil'ev]

diff utility make this strange result.

I added only one structure, debouncing timer and handle EV_KEY type.

You can see difference with "total commander" and it command "Files->Compare 
by content..."

Thank you,
Dmitriy


----- Original Message ----- 
From: "Daniel Mack" <daniel@caiaq.de>
To: "Dmitriy Vasil'ev" <tech@digiton.ru>
Cc: <linux-input@vger.kernel.org>
Sent: Thursday, June 17, 2010 3:37 PM
Subject: Re: rotary encoder


> Hi Dmitriy,
>
> Sorry, but your patch can't be reviewed that way. It removes the whole
> driver and adds a complete new version, which is not what a patch should
> normally do.
>
> Could you please check out the latest sources from git, copy your
> version of the driver to the tree and then run 'git diff'?
>
> This howto can give you a good start:
>
>  http://linux.yyz.us/git-howto.html
>
>
> Also, please have a look at the file Documentation/CodingStyle,
> especially because your indentation and comments are not as they should
> be.
>
> I'd be happy to give feedback, but there are some rules to follow to
> make that process feasible.
>
> Thanks,
> Daniel
>
>
> On Thu, Jun 17, 2010 at 03:29:51PM +0400, Dmitriy Vasil'ev wrote:
>> diff -ur linux-2.6.34/Documentation/input/rotary-encoder.txt
>> linux/Documentation/input/rotary-encoder.txt
>> --- linux-2.6.34/Documentation/input/rotary-encoder.txt Mon May 17
>> 01:17:36 2010
>> +++ linux/Documentation/input/rotary-encoder.txt Thu Jun 17 13:24:14 2010
>> @@ -86,16 +86,26 @@
>>
>> #define GPIO_ROTARY_A 1
>> #define GPIO_ROTARY_B 2
>> +#define GPIO_ROTARY_S 3
>>
>> static struct rotary_encoder_platform_data my_rotary_encoder_info = {
>> - .steps  = 24,
>> - .axis  = ABS_X,
>> - .relative_axis = false,
>> - .rollover = false,
>> + .steps  = 1,
>> + .type  = EV_KEY, //(EV_KEY, EV_REL, EV_ABS)
>>  .gpio_a  = GPIO_ROTARY_A,
>>  .gpio_b  = GPIO_ROTARY_B,
>>  .inverted_a = 0,
>>  .inverted_b = 0,
>> + .codeleft = KEY_LEFT, //(REL_X, REL_Y, ABS_X, ABS_Y, KEY_...)
>> + .coderight = KEY_RIGHT, //(REL_X, REL_Y, ABS_X, ABS_Y, KEY_...)
>> + .rollover = 0,
>> + .button  = {
>> +  .code = KEY_ENTER,
>> +  .gpio = GPIO_ROTARY_S,
>> +  .active_low = 1,
>> +  .type = EV_KEY,
>> +  .debounce_interval = 10,
>> +  .rep = false
>> + }
>> };
>>
>> static struct platform_device rotary_encoder_device = {
>> diff -ur linux-2.6.34/drivers/input/misc/rotary_encoder.c
>> linux/drivers/input/misc/rotary_encoder.c
>> --- linux-2.6.34/drivers/input/misc/rotary_encoder.c Mon May 17
>> 01:17:36 2010
>> +++ linux/drivers/input/misc/rotary_encoder.c Thu Jun 17 13:40:16 2010
>> @@ -1,257 +1,356 @@
>> -/*
>> - * rotary_encoder.c
>> - *
>> - * (c) 2009 Daniel Mack <daniel@caiaq.de>
>> - *
>> - * state machine code inspired by code from Tim Ruetz
>> - *
>> - * A generic driver for rotary encoders connected to GPIO lines.
>> - * See file:Documentation/input/rotary_encoder.txt for more information
>> - *
>> - * This program is free software; you can redistribute it and/or modify
>> - * it under the terms of the GNU General Public License version 2 as
>> - * published by the Free Software Foundation.
>> - */
>> -
>> -#include <linux/kernel.h>
>> -#include <linux/module.h>
>> -#include <linux/init.h>
>> -#include <linux/interrupt.h>
>> -#include <linux/input.h>
>> -#include <linux/device.h>
>> -#include <linux/platform_device.h>
>> -#include <linux/gpio.h>
>> -#include <linux/rotary_encoder.h>
>> -#include <linux/slab.h>
>> -
>> -#define DRV_NAME "rotary-encoder"
>> -
>> -struct rotary_encoder {
>> - struct input_dev *input;
>> - struct rotary_encoder_platform_data *pdata;
>> -
>> - unsigned int axis;
>> - unsigned int pos;
>> -
>> - unsigned int irq_a;
>> - unsigned int irq_b;
>> -
>> - bool armed;
>> - unsigned char dir; /* 0 - clockwise, 1 - CCW */
>> -};
>> -
>> -static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
>> -{
>> - struct rotary_encoder *encoder = dev_id;
>> - struct rotary_encoder_platform_data *pdata = encoder->pdata;
>> - int a = !!gpio_get_value(pdata->gpio_a);
>> - int b = !!gpio_get_value(pdata->gpio_b);
>> - int state;
>> -
>> - a ^= pdata->inverted_a;
>> - b ^= pdata->inverted_b;
>> - state = (a << 1) | b;
>> -
>> - switch (state) {
>> -
>> - case 0x0:
>> -  if (!encoder->armed)
>> -   break;
>> -
>> -  if (pdata->relative_axis) {
>> -   input_report_rel(encoder->input, pdata->axis,
>> -      encoder->dir ? -1 : 1);
>> -  } else {
>> -   unsigned int pos = encoder->pos;
>> -
>> -   if (encoder->dir) {
>> -    /* turning counter-clockwise */
>> -    if (pdata->rollover)
>> -     pos += pdata->steps;
>> -    if (pos)
>> -     pos--;
>> -   } else {
>> -    /* turning clockwise */
>> -    if (pdata->rollover || pos < pdata->steps)
>> -     pos++;
>> -   }
>> -   if (pdata->rollover)
>> -    pos %= pdata->steps;
>> -   encoder->pos = pos;
>> -   input_report_abs(encoder->input, pdata->axis,
>> -      encoder->pos);
>> -  }
>> -  input_sync(encoder->input);
>> -
>> -  encoder->armed = false;
>> -  break;
>> -
>> - case 0x1:
>> - case 0x2:
>> -  if (encoder->armed)
>> -   encoder->dir = state - 1;
>> -  break;
>> -
>> - case 0x3:
>> -  encoder->armed = true;
>> -  break;
>> - }
>> -
>> - return IRQ_HANDLED;
>> -}
>> -
>> -static int __devinit rotary_encoder_probe(struct platform_device *pdev)
>> -{
>> - struct rotary_encoder_platform_data *pdata = pdev->dev.platform_data;
>> - struct rotary_encoder *encoder;
>> - struct input_dev *input;
>> - int err;
>> -
>> - if (!pdata) {
>> -  dev_err(&pdev->dev, "missing platform data\n");
>> -  return -ENOENT;
>> - }
>> -
>> - encoder = kzalloc(sizeof(struct rotary_encoder), GFP_KERNEL);
>> - input = input_allocate_device();
>> - if (!encoder || !input) {
>> -  dev_err(&pdev->dev, "failed to allocate memory for device\n");
>> -  err = -ENOMEM;
>> -  goto exit_free_mem;
>> - }
>> -
>> - encoder->input = input;
>> - encoder->pdata = pdata;
>> - encoder->irq_a = gpio_to_irq(pdata->gpio_a);
>> - encoder->irq_b = gpio_to_irq(pdata->gpio_b);
>> -
>> - /* create and register the input driver */
>> - input->name = pdev->name;
>> - input->id.bustype = BUS_HOST;
>> - input->dev.parent = &pdev->dev;
>> -
>> - if (pdata->relative_axis) {
>> -  input->evbit[0] = BIT_MASK(EV_REL);
>> -  input->relbit[0] = BIT_MASK(pdata->axis);
>> - } else {
>> -  input->evbit[0] = BIT_MASK(EV_ABS);
>> -  input_set_abs_params(encoder->input,
>> -         pdata->axis, 0, pdata->steps, 0, 1);
>> - }
>> -
>> - err = input_register_device(input);
>> - if (err) {
>> -  dev_err(&pdev->dev, "failed to register input device\n");
>> -  goto exit_free_mem;
>> - }
>> -
>> - /* request the GPIOs */
>> - err = gpio_request(pdata->gpio_a, DRV_NAME);
>> - if (err) {
>> -  dev_err(&pdev->dev, "unable to request GPIO %d\n",
>> -   pdata->gpio_a);
>> -  goto exit_unregister_input;
>> - }
>> -
>> - err = gpio_direction_input(pdata->gpio_a);
>> - if (err) {
>> -  dev_err(&pdev->dev, "unable to set GPIO %d for input\n",
>> -   pdata->gpio_a);
>> -  goto exit_unregister_input;
>> - }
>> -
>> - err = gpio_request(pdata->gpio_b, DRV_NAME);
>> - if (err) {
>> -  dev_err(&pdev->dev, "unable to request GPIO %d\n",
>> -   pdata->gpio_b);
>> -  goto exit_free_gpio_a;
>> - }
>> -
>> - err = gpio_direction_input(pdata->gpio_b);
>> - if (err) {
>> -  dev_err(&pdev->dev, "unable to set GPIO %d for input\n",
>> -   pdata->gpio_b);
>> -  goto exit_free_gpio_a;
>> - }
>> -
>> - /* request the IRQs */
>> - err = request_irq(encoder->irq_a, &rotary_encoder_irq,
>> -     IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
>> -     DRV_NAME, encoder);
>> - if (err) {
>> -  dev_err(&pdev->dev, "unable to request IRQ %d\n",
>> -   encoder->irq_a);
>> -  goto exit_free_gpio_b;
>> - }
>> -
>> - err = request_irq(encoder->irq_b, &rotary_encoder_irq,
>> -     IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
>> -     DRV_NAME, encoder);
>> - if (err) {
>> -  dev_err(&pdev->dev, "unable to request IRQ %d\n",
>> -   encoder->irq_b);
>> -  goto exit_free_irq_a;
>> - }
>> -
>> - platform_set_drvdata(pdev, encoder);
>> -
>> - return 0;
>> -
>> -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_unregister_input:
>> - input_unregister_device(input);
>> - input = NULL; /* so we don't try to free it */
>> -exit_free_mem:
>> - input_free_device(input);
>> - kfree(encoder);
>> - return err;
>> -}
>> -
>> -static int __devexit rotary_encoder_remove(struct platform_device *pdev)
>> -{
>> - struct rotary_encoder *encoder = platform_get_drvdata(pdev);
>> - struct rotary_encoder_platform_data *pdata = pdev->dev.platform_data;
>> -
>> - 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);
>> - platform_set_drvdata(pdev, NULL);
>> - kfree(encoder);
>> -
>> - return 0;
>> -}
>> -
>> -static struct platform_driver rotary_encoder_driver = {
>> - .probe  = rotary_encoder_probe,
>> - .remove  = __devexit_p(rotary_encoder_remove),
>> - .driver  = {
>> -  .name = DRV_NAME,
>> -  .owner = THIS_MODULE,
>> - }
>> -};
>> -
>> -static int __init rotary_encoder_init(void)
>> -{
>> - return platform_driver_register(&rotary_encoder_driver);
>> -}
>> -
>> -static void __exit rotary_encoder_exit(void)
>> -{
>> - platform_driver_unregister(&rotary_encoder_driver);
>> -}
>> -
>> -module_init(rotary_encoder_init);
>> -module_exit(rotary_encoder_exit);
>> -
>> -MODULE_ALIAS("platform:" DRV_NAME);
>> -MODULE_DESCRIPTION("GPIO rotary encoder driver");
>> -MODULE_AUTHOR("Daniel Mack <daniel@caiaq.de>");
>> -MODULE_LICENSE("GPL v2");
>> -
>> +/*
>> + * rotary_encoder.c
>> + *
>> + * (c) 2009 Daniel Mack <daniel@caiaq.de>
>> + *
>> + * state machine code inspired by code from Tim Ruetz
>> + *
>> + * A generic driver for rotary encoders connected to GPIO lines.
>> + * See file:Documentation/input/rotary_encoder.txt for more information
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/module.h>
>> +#include <linux/init.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/input.h>
>> +#include <linux/device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/gpio.h>
>> +#include <linux/rotary_encoder.h>
>> +#include <linux/slab.h>
>> +#include <linux/workqueue.h>
>> +#include <linux/sched.h>
>> +
>> +#define DRV_NAME "rotary-encoder"
>> +
>> +struct rotary_encoder_button_data {
>> + struct rotary_encoder_button button;
>> + unsigned int irq;
>> + struct timer_list timer;
>> + struct work_struct work;
>> +};
>> +
>> +struct rotary_encoder {
>> + struct input_dev *input;
>> + struct rotary_encoder_platform_data *pdata;
>> +
>> + unsigned int pos;
>> +
>> + unsigned int irq_a;
>> + unsigned int irq_b;
>> +
>> + bool armed;
>> + unsigned char dir; /* 0 - clockwise, 1 - CCW */
>> +
>> + struct rotary_encoder_button_data bdata;
>> +};
>> +
>> +static struct rotary_encoder* encoder;
>> +
>> +static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
>> +{
>> + struct rotary_encoder *encoder = dev_id;
>> + struct rotary_encoder_platform_data *pdata = encoder->pdata;
>> + int a = !!gpio_get_value(pdata->gpio_a);
>> + int b = !!gpio_get_value(pdata->gpio_b);
>> + int state;
>> +
>> + a ^= pdata->inverted_a;
>> + b ^= pdata->inverted_b;
>> + state = (a << 1) | b;
>> +
>> + switch (state) {
>> +
>> + case 0x0:
>> +  if (!encoder->armed)
>> +   break;
>> +
>> +  if (pdata->type == EV_REL) {
>> +   input_report_rel(encoder->input, pdata->codeleft,
>> +      encoder->dir ? -1 : 1);
>> +  } else
>> +  if (pdata->type == EV_ABS) {
>> +   unsigned int pos = encoder->pos;
>> +
>> +   if (encoder->dir) {
>> +    /* turning counter-clockwise */
>> +    if (pdata->rollover)
>> +     pos += pdata->steps;
>> +    if (pos)
>> +     pos--;
>> +   } else {
>> +    /* turning clockwise */
>> +    if (pdata->rollover || pos < pdata->steps)
>> +     pos++;
>> +   }
>> +   if (pdata->rollover)
>> +    pos %= pdata->steps;
>> +   encoder->pos = pos;
>> +   input_report_abs(encoder->input, pdata->codeleft,
>> +      encoder->pos);
>> +  } else
>> +  if (pdata->type == EV_KEY)
>> +  {
>> +   input_report_key(encoder->input, encoder->dir ? pdata->codeleft
>> : pdata->coderight, 1);
>> +   input_sync(encoder->input);
>> +   input_report_key(encoder->input, encoder->dir ? pdata->codeleft
>> : pdata->coderight, 0);
>> +   input_sync(encoder->input);
>> +  }
>> +
>> +  encoder->armed = false;
>> +  break;
>> +
>> + case 0x1:
>> + case 0x2:
>> +  if (encoder->armed)
>> +   encoder->dir = state - 1;
>> +  break;
>> +
>> + case 0x3:
>> +  encoder->armed = true;
>> +  break;
>> + }
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static void rotary_encoder_report_event(struct rotary_encoder *encoder)
>> +{
>> + struct rotary_encoder_button *button = &(encoder->bdata.button);
>> + struct input_dev *input = encoder->input;
>> + int state = (gpio_get_value(button->gpio) ? 1 : 0) ^ 
>> button->active_low;
>> +
>> + input_event(input, EV_KEY, button->code, !!state);
>> + input_sync(input);
>> +}
>> +
>> +static void rotary_encoder_work_func(struct work_struct *work)
>> +{
>> + struct rotary_encoder *encoder =
>> +  container_of(work, struct rotary_encoder, bdata.work);
>> +
>> + rotary_encoder_report_event(encoder);
>> +}
>> +
>> +static void rotary_encoder_timer(unsigned long _data)
>> +{
>> + struct rotary_encoder *encoder = (struct rotary_encoder *)_data;
>> +
>> + schedule_work(&encoder->bdata.work);
>> +}
>> +
>> +static irqreturn_t rotary_encoder_key_irq(int irq, void *dev_id)
>> +{
>> + struct rotary_encoder *encoder = dev_id;
>> + struct rotary_encoder_button_data *bdata = &encoder->bdata;
>> + struct rotary_encoder_button *button = &bdata->button;
>> +
>> + if (button->debounce_interval)
>> +  mod_timer(&bdata->timer,
>> +   jiffies + msecs_to_jiffies(button->debounce_interval));
>> + else
>> +  schedule_work(&bdata->work);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int __devinit rotary_encoder_probe(struct platform_device *pdev)
>> +{
>> + struct rotary_encoder_platform_data *pdata = pdev->dev.platform_data;
>> + struct rotary_encoder *encoder;
>> + struct input_dev *input;
>> + int err;
>> +
>> + if (!pdata) {
>> +  dev_err(&pdev->dev, "missing platform data\n");
>> +  return -ENOENT;
>> + }
>> +
>> + encoder = kzalloc(sizeof(struct rotary_encoder), GFP_KERNEL);
>> + input = input_allocate_device();
>> + if (!encoder || !input) {
>> +  dev_err(&pdev->dev, "failed to allocate memory for device\n");
>> +  err = -ENOMEM;
>> +  goto exit_free_mem;
>> + }
>> +
>> + encoder->input = input;
>> + encoder->pdata = pdata;
>> + encoder->irq_a = gpio_to_irq(pdata->gpio_a);
>> + encoder->irq_b = gpio_to_irq(pdata->gpio_b);
>> + encoder->bdata.irq = gpio_to_irq(pdata->button.gpio);
>> +
>> + /* create and register the input driver */
>> + input->name = pdev->name;
>> + input->id.bustype = BUS_HOST;
>> + input->dev.parent = &pdev->dev;
>> +
>> + /* Enable auto repeat feature of Linux input subsystem */
>> + if (pdata->button.rep)
>> +  set_bit(EV_REP, input->evbit);
>> +
>> + encoder->bdata.button = pdata->button;
>> + //input_set_capability(input, EV_KEY, bdata->button.code);
>> +
>> + set_bit(pdata->type, input->evbit);
>> + set_bit(pdata->codeleft, input->keybit);
>> + set_bit(pdata->coderight, input->keybit);
>> + //
>> + set_bit(encoder->bdata.button.type, input->evbit);
>> + set_bit(encoder->bdata.button.code, input->keybit);
>> +
>> + setup_timer(&encoder->bdata.timer, rotary_encoder_timer, (unsigned
>> long)encoder);
>> + INIT_WORK(&encoder->bdata.work, rotary_encoder_work_func);
>> +
>> + err = input_register_device(input);
>> + if (err) {
>> +  dev_err(&pdev->dev, "failed to register input device\n");
>> +  goto exit_free_mem;
>> + }
>> +
>> + /* request the GPIOs */
>> + err = gpio_request(pdata->gpio_a, DRV_NAME);
>> + if (err) {
>> +  dev_err(&pdev->dev, "unable to request GPIO %d\n",
>> +   pdata->gpio_a);
>> +  goto exit_unregister_input;
>> + }
>> +
>> + err = gpio_direction_input(pdata->gpio_a);
>> + if (err) {
>> +  dev_err(&pdev->dev, "unable to set GPIO %d for input\n",
>> +   pdata->gpio_a);
>> +  goto exit_unregister_input;
>> + }
>> +
>> + err = gpio_request(pdata->gpio_b, DRV_NAME);
>> + if (err) {
>> +  dev_err(&pdev->dev, "unable to request GPIO %d\n",
>> +   pdata->gpio_b);
>> +  goto exit_free_gpio_a;
>> + }
>> +
>> + err = gpio_direction_input(pdata->gpio_b);
>> + if (err) {
>> +  dev_err(&pdev->dev, "unable to set GPIO %d for input\n",
>> +   pdata->gpio_b);
>> +  goto exit_free_gpio_a;
>> + }
>> +
>> + err = gpio_request(pdata->button.gpio, DRV_NAME);
>> + if (err) {
>> +  dev_err(&pdev->dev, "unable to request GPIO %d\n",
>> +   pdata->button.gpio);
>> +  goto exit_free_gpio_b;
>> + }
>> +
>> + err = gpio_direction_input(pdata->button.gpio);
>> + if (err) {
>> +  dev_err(&pdev->dev, "unable to set GPIO %d for input\n",
>> +   pdata->button.gpio);
>> +  goto exit_free_gpio_b;
>> + }
>> +
>> + /* request the IRQs */
>> + err = request_irq(encoder->irq_a, &rotary_encoder_irq,
>> +     IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
>> +     DRV_NAME, encoder);
>> + if (err) {
>> +  dev_err(&pdev->dev, "unable to request IRQ %d\n",
>> +   encoder->irq_a);
>> +  goto exit_free_gpio_s;
>> + }
>> +
>> + err = request_irq(encoder->irq_b, &rotary_encoder_irq,
>> +     IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
>> +     DRV_NAME, encoder);
>> + if (err) {
>> +  dev_err(&pdev->dev, "unable to request IRQ %d\n",
>> +   encoder->irq_b);
>> +  goto exit_free_irq_a;
>> + }
>> +
>> + err = request_irq(encoder->bdata.irq, &rotary_encoder_key_irq,
>> +     IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
>> +     DRV_NAME, encoder);
>> + if (err) {
>> +  dev_err(&pdev->dev, "unable to request IRQ %d\n",
>> +   encoder->bdata.irq);
>> +  goto exit_free_irq_b;
>> + }
>> +
>> + 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_s:
>> + gpio_free(pdata->button.gpio);
>> +exit_free_gpio_b:
>> + gpio_free(pdata->gpio_b);
>> +exit_free_gpio_a:
>> + gpio_free(pdata->gpio_a);
>> +exit_unregister_input:
>> + input_unregister_device(input);
>> + input = NULL; /* so we don't try to free it */
>> +exit_free_mem:
>> + input_free_device(input);
>> + kfree(encoder);
>> + return err;
>> +}
>> +
>> +static int __devexit rotary_encoder_remove(struct platform_device *pdev)
>> +{
>> + struct rotary_encoder *encoder = platform_get_drvdata(pdev);
>> + struct rotary_encoder_platform_data *pdata = pdev->dev.platform_data;
>> +
>> + free_irq(encoder->irq_a, encoder);
>> + free_irq(encoder->irq_b, encoder);
>> + free_irq(encoder->bdata.irq, encoder);
>> + if (pdata->button.debounce_interval)
>> +  del_timer_sync(&encoder->bdata.timer);
>> + cancel_work_sync(&encoder->bdata.work);
>> + gpio_free(pdata->gpio_a);
>> + gpio_free(pdata->gpio_b);
>> + gpio_free(pdata->button.gpio);
>> + input_unregister_device(encoder->input);
>> + platform_set_drvdata(pdev, NULL);
>> + kfree(encoder);
>> +
>> + return 0;
>> +}
>> +
>> +static struct platform_driver rotary_encoder_driver = {
>> + .probe  = rotary_encoder_probe,
>> + .remove  = __devexit_p(rotary_encoder_remove),
>> + .driver  = {
>> +  .name = DRV_NAME,
>> +  .owner = THIS_MODULE,
>> + }
>> +};
>> +
>> +static int __init rotary_encoder_init(void)
>> +{
>> + return platform_driver_register(&rotary_encoder_driver);
>> +}
>> +
>> +static void __exit rotary_encoder_exit(void)
>> +{
>> + platform_driver_unregister(&rotary_encoder_driver);
>> +}
>> +
>> +module_init(rotary_encoder_init);
>> +module_exit(rotary_encoder_exit);
>> +
>> +MODULE_ALIAS("platform:" DRV_NAME);
>> +MODULE_DESCRIPTION("GPIO rotary encoder driver");
>> +MODULE_AUTHOR("Daniel Mack <daniel@caiaq.de>");
>> +MODULE_LICENSE("GPL v2");
>> +
>> diff -ur linux-2.6.34/include/linux/rotary_encoder.h
>> linux/include/linux/rotary_encoder.h
>> --- linux-2.6.34/include/linux/rotary_encoder.h Mon May 17 01:17:36 2010
>> +++ linux/include/linux/rotary_encoder.h Thu Jun 17 14:23:26 2010
>> @@ -1,15 +1,28 @@
>> #ifndef __ROTARY_ENCODER_H__
>> #define __ROTARY_ENCODER_H__
>>
>> +struct rotary_encoder_button {
>> + /* Configuration parameters */
>> + u16 code;  /* input event code (KEY_*, SW_*) */
>> + int gpio;
>> + int active_low;
>> + //char *desc;
>> + u16 type;  /* input event type (EV_KEY, EV_SW) */
>> + int debounce_interval; /* debounce ticks interval in msecs */
>> + bool rep;/* enable input subsystem auto repeat */
>> +};
>> +
>> struct rotary_encoder_platform_data {
>>  unsigned int steps;
>> - unsigned int axis;
>> + u16 type; /*(EV_KEY, EV_REL, EV_ABS)*/
>>  unsigned int gpio_a;
>>  unsigned int gpio_b;
>>  unsigned int inverted_a;
>>  unsigned int inverted_b;
>> - bool relative_axis;
>> + u16 codeleft; /*(REL_X, REL_Y, ABS_X, ABS_Y, KEY_...)*/
>> + u16 coderight; /*(REL_X, REL_Y, ABS_X, ABS_Y, KEY_...)*/
>>  bool rollover;
>> + struct rotary_encoder_button button;
>> };
>>
>> #endif /* __ROTARY_ENCODER_H__ */
>>
>> ----- Original Message ----- From: "Daniel Mack" <daniel@caiaq.de>
>> To: "Dmitriy Vasil'ev" <tech@digiton.ru>
>> Cc: <linux-input@vger.kernel.org>
>> Sent: Thursday, June 17, 2010 3:21 PM
>> Subject: Re: rotary encoder
>>
>>
>> >On Thu, Jun 17, 2010 at 02:48:08PM +0400, Dmitriy Vasil'ev wrote:
>> >>I added .diff file.
>> >>I am sorry, but I can do it only for kernel version 2.6.34
>> >>See attach.
>> >
>> >Please, just put the patch inline, not as attachment and not zipped.
>> >That way, the patch can be commented line-by-line.
>> >
>> >Thanks,
>> >Daniel
>> 

Re: rotary encoder

[Daniel Mack]

On Thu, Jun 17, 2010 at 03:55:05PM +0400, Dmitriy Vasil'ev wrote:
> diff utility make this strange result.
> 
> I added only one structure, debouncing timer and handle EV_KEY type.

Yo, and a proper patch generated by a well working tool would show
exactly this.

> You can see difference with "total commander" and it command
> "Files->Compare by content..."

I don't have this tool. I don't even have Windows. Developers in the
kernel community have agreed to share patches as unified diffs, and your
contribution really shouldn't make any difference.

As a matter of fact, the tool that just gets it right all the way is
git, and I can only encourage you to get your head around it and use it.

Also, please read Documentation/SubmittingPatches, which also describes
the process of collaboration very well.

Thanks,
Daniel



> ----- Original Message ----- From: "Daniel Mack" <daniel@caiaq.de>
> To: "Dmitriy Vasil'ev" <tech@digiton.ru>
> Cc: <linux-input@vger.kernel.org>
> Sent: Thursday, June 17, 2010 3:37 PM
> Subject: Re: rotary encoder
> 
> 
> >Hi Dmitriy,
> >
> >Sorry, but your patch can't be reviewed that way. It removes the whole
> >driver and adds a complete new version, which is not what a patch should
> >normally do.
> >
> >Could you please check out the latest sources from git, copy your
> >version of the driver to the tree and then run 'git diff'?
> >
> >This howto can give you a good start:
> >
> > http://linux.yyz.us/git-howto.html
> >
> >
> >Also, please have a look at the file Documentation/CodingStyle,
> >especially because your indentation and comments are not as they should
> >be.
> >
> >I'd be happy to give feedback, but there are some rules to follow to
> >make that process feasible.
> >
> >Thanks,
> >Daniel
> >
> >
> >On Thu, Jun 17, 2010 at 03:29:51PM +0400, Dmitriy Vasil'ev wrote:
> >>diff -ur linux-2.6.34/Documentation/input/rotary-encoder.txt
> >>linux/Documentation/input/rotary-encoder.txt
> >>--- linux-2.6.34/Documentation/input/rotary-encoder.txt Mon May 17
> >>01:17:36 2010
> >>+++ linux/Documentation/input/rotary-encoder.txt Thu Jun 17 13:24:14 2010
> >>@@ -86,16 +86,26 @@
> >>
> >>#define GPIO_ROTARY_A 1
> >>#define GPIO_ROTARY_B 2
> >>+#define GPIO_ROTARY_S 3
> >>
> >>static struct rotary_encoder_platform_data my_rotary_encoder_info = {
> >>- .steps  = 24,
> >>- .axis  = ABS_X,
> >>- .relative_axis = false,
> >>- .rollover = false,
> >>+ .steps  = 1,
> >>+ .type  = EV_KEY, //(EV_KEY, EV_REL, EV_ABS)
> >> .gpio_a  = GPIO_ROTARY_A,
> >> .gpio_b  = GPIO_ROTARY_B,
> >> .inverted_a = 0,
> >> .inverted_b = 0,
> >>+ .codeleft = KEY_LEFT, //(REL_X, REL_Y, ABS_X, ABS_Y, KEY_...)
> >>+ .coderight = KEY_RIGHT, //(REL_X, REL_Y, ABS_X, ABS_Y, KEY_...)
> >>+ .rollover = 0,
> >>+ .button  = {
> >>+  .code = KEY_ENTER,
> >>+  .gpio = GPIO_ROTARY_S,
> >>+  .active_low = 1,
> >>+  .type = EV_KEY,
> >>+  .debounce_interval = 10,
> >>+  .rep = false
> >>+ }
> >>};
> >>
> >>static struct platform_device rotary_encoder_device = {
> >>diff -ur linux-2.6.34/drivers/input/misc/rotary_encoder.c
> >>linux/drivers/input/misc/rotary_encoder.c
> >>--- linux-2.6.34/drivers/input/misc/rotary_encoder.c Mon May 17
> >>01:17:36 2010
> >>+++ linux/drivers/input/misc/rotary_encoder.c Thu Jun 17 13:40:16 2010
> >>@@ -1,257 +1,356 @@
> >>-/*
> >>- * rotary_encoder.c
> >>- *
> >>- * (c) 2009 Daniel Mack <daniel@caiaq.de>
> >>- *
> >>- * state machine code inspired by code from Tim Ruetz
> >>- *
> >>- * A generic driver for rotary encoders connected to GPIO lines.
> >>- * See file:Documentation/input/rotary_encoder.txt for more information
> >>- *
> >>- * This program is free software; you can redistribute it and/or modify
> >>- * it under the terms of the GNU General Public License version 2 as
> >>- * published by the Free Software Foundation.
> >>- */
> >>-
> >>-#include <linux/kernel.h>
> >>-#include <linux/module.h>
> >>-#include <linux/init.h>
> >>-#include <linux/interrupt.h>
> >>-#include <linux/input.h>
> >>-#include <linux/device.h>
> >>-#include <linux/platform_device.h>
> >>-#include <linux/gpio.h>
> >>-#include <linux/rotary_encoder.h>
> >>-#include <linux/slab.h>
> >>-
> >>-#define DRV_NAME "rotary-encoder"
> >>-
> >>-struct rotary_encoder {
> >>- struct input_dev *input;
> >>- struct rotary_encoder_platform_data *pdata;
> >>-
> >>- unsigned int axis;
> >>- unsigned int pos;
> >>-
> >>- unsigned int irq_a;
> >>- unsigned int irq_b;
> >>-
> >>- bool armed;
> >>- unsigned char dir; /* 0 - clockwise, 1 - CCW */
> >>-};
> >>-
> >>-static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
> >>-{
> >>- struct rotary_encoder *encoder = dev_id;
> >>- struct rotary_encoder_platform_data *pdata = encoder->pdata;
> >>- int a = !!gpio_get_value(pdata->gpio_a);
> >>- int b = !!gpio_get_value(pdata->gpio_b);
> >>- int state;
> >>-
> >>- a ^= pdata->inverted_a;
> >>- b ^= pdata->inverted_b;
> >>- state = (a << 1) | b;
> >>-
> >>- switch (state) {
> >>-
> >>- case 0x0:
> >>-  if (!encoder->armed)
> >>-   break;
> >>-
> >>-  if (pdata->relative_axis) {
> >>-   input_report_rel(encoder->input, pdata->axis,
> >>-      encoder->dir ? -1 : 1);
> >>-  } else {
> >>-   unsigned int pos = encoder->pos;
> >>-
> >>-   if (encoder->dir) {
> >>-    /* turning counter-clockwise */
> >>-    if (pdata->rollover)
> >>-     pos += pdata->steps;
> >>-    if (pos)
> >>-     pos--;
> >>-   } else {
> >>-    /* turning clockwise */
> >>-    if (pdata->rollover || pos < pdata->steps)
> >>-     pos++;
> >>-   }
> >>-   if (pdata->rollover)
> >>-    pos %= pdata->steps;
> >>-   encoder->pos = pos;
> >>-   input_report_abs(encoder->input, pdata->axis,
> >>-      encoder->pos);
> >>-  }
> >>-  input_sync(encoder->input);
> >>-
> >>-  encoder->armed = false;
> >>-  break;
> >>-
> >>- case 0x1:
> >>- case 0x2:
> >>-  if (encoder->armed)
> >>-   encoder->dir = state - 1;
> >>-  break;
> >>-
> >>- case 0x3:
> >>-  encoder->armed = true;
> >>-  break;
> >>- }
> >>-
> >>- return IRQ_HANDLED;
> >>-}
> >>-
> >>-static int __devinit rotary_encoder_probe(struct platform_device *pdev)
> >>-{
> >>- struct rotary_encoder_platform_data *pdata = pdev->dev.platform_data;
> >>- struct rotary_encoder *encoder;
> >>- struct input_dev *input;
> >>- int err;
> >>-
> >>- if (!pdata) {
> >>-  dev_err(&pdev->dev, "missing platform data\n");
> >>-  return -ENOENT;
> >>- }
> >>-
> >>- encoder = kzalloc(sizeof(struct rotary_encoder), GFP_KERNEL);
> >>- input = input_allocate_device();
> >>- if (!encoder || !input) {
> >>-  dev_err(&pdev->dev, "failed to allocate memory for device\n");
> >>-  err = -ENOMEM;
> >>-  goto exit_free_mem;
> >>- }
> >>-
> >>- encoder->input = input;
> >>- encoder->pdata = pdata;
> >>- encoder->irq_a = gpio_to_irq(pdata->gpio_a);
> >>- encoder->irq_b = gpio_to_irq(pdata->gpio_b);
> >>-
> >>- /* create and register the input driver */
> >>- input->name = pdev->name;
> >>- input->id.bustype = BUS_HOST;
> >>- input->dev.parent = &pdev->dev;
> >>-
> >>- if (pdata->relative_axis) {
> >>-  input->evbit[0] = BIT_MASK(EV_REL);
> >>-  input->relbit[0] = BIT_MASK(pdata->axis);
> >>- } else {
> >>-  input->evbit[0] = BIT_MASK(EV_ABS);
> >>-  input_set_abs_params(encoder->input,
> >>-         pdata->axis, 0, pdata->steps, 0, 1);
> >>- }
> >>-
> >>- err = input_register_device(input);
> >>- if (err) {
> >>-  dev_err(&pdev->dev, "failed to register input device\n");
> >>-  goto exit_free_mem;
> >>- }
> >>-
> >>- /* request the GPIOs */
> >>- err = gpio_request(pdata->gpio_a, DRV_NAME);
> >>- if (err) {
> >>-  dev_err(&pdev->dev, "unable to request GPIO %d\n",
> >>-   pdata->gpio_a);
> >>-  goto exit_unregister_input;
> >>- }
> >>-
> >>- err = gpio_direction_input(pdata->gpio_a);
> >>- if (err) {
> >>-  dev_err(&pdev->dev, "unable to set GPIO %d for input\n",
> >>-   pdata->gpio_a);
> >>-  goto exit_unregister_input;
> >>- }
> >>-
> >>- err = gpio_request(pdata->gpio_b, DRV_NAME);
> >>- if (err) {
> >>-  dev_err(&pdev->dev, "unable to request GPIO %d\n",
> >>-   pdata->gpio_b);
> >>-  goto exit_free_gpio_a;
> >>- }
> >>-
> >>- err = gpio_direction_input(pdata->gpio_b);
> >>- if (err) {
> >>-  dev_err(&pdev->dev, "unable to set GPIO %d for input\n",
> >>-   pdata->gpio_b);
> >>-  goto exit_free_gpio_a;
> >>- }
> >>-
> >>- /* request the IRQs */
> >>- err = request_irq(encoder->irq_a, &rotary_encoder_irq,
> >>-     IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
> >>-     DRV_NAME, encoder);
> >>- if (err) {
> >>-  dev_err(&pdev->dev, "unable to request IRQ %d\n",
> >>-   encoder->irq_a);
> >>-  goto exit_free_gpio_b;
> >>- }
> >>-
> >>- err = request_irq(encoder->irq_b, &rotary_encoder_irq,
> >>-     IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
> >>-     DRV_NAME, encoder);
> >>- if (err) {
> >>-  dev_err(&pdev->dev, "unable to request IRQ %d\n",
> >>-   encoder->irq_b);
> >>-  goto exit_free_irq_a;
> >>- }
> >>-
> >>- platform_set_drvdata(pdev, encoder);
> >>-
> >>- return 0;
> >>-
> >>-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_unregister_input:
> >>- input_unregister_device(input);
> >>- input = NULL; /* so we don't try to free it */
> >>-exit_free_mem:
> >>- input_free_device(input);
> >>- kfree(encoder);
> >>- return err;
> >>-}
> >>-
> >>-static int __devexit rotary_encoder_remove(struct platform_device *pdev)
> >>-{
> >>- struct rotary_encoder *encoder = platform_get_drvdata(pdev);
> >>- struct rotary_encoder_platform_data *pdata = pdev->dev.platform_data;
> >>-
> >>- 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);
> >>- platform_set_drvdata(pdev, NULL);
> >>- kfree(encoder);
> >>-
> >>- return 0;
> >>-}
> >>-
> >>-static struct platform_driver rotary_encoder_driver = {
> >>- .probe  = rotary_encoder_probe,
> >>- .remove  = __devexit_p(rotary_encoder_remove),
> >>- .driver  = {
> >>-  .name = DRV_NAME,
> >>-  .owner = THIS_MODULE,
> >>- }
> >>-};
> >>-
> >>-static int __init rotary_encoder_init(void)
> >>-{
> >>- return platform_driver_register(&rotary_encoder_driver);
> >>-}
> >>-
> >>-static void __exit rotary_encoder_exit(void)
> >>-{
> >>- platform_driver_unregister(&rotary_encoder_driver);
> >>-}
> >>-
> >>-module_init(rotary_encoder_init);
> >>-module_exit(rotary_encoder_exit);
> >>-
> >>-MODULE_ALIAS("platform:" DRV_NAME);
> >>-MODULE_DESCRIPTION("GPIO rotary encoder driver");
> >>-MODULE_AUTHOR("Daniel Mack <daniel@caiaq.de>");
> >>-MODULE_LICENSE("GPL v2");
> >>-
> >>+/*
> >>+ * rotary_encoder.c
> >>+ *
> >>+ * (c) 2009 Daniel Mack <daniel@caiaq.de>
> >>+ *
> >>+ * state machine code inspired by code from Tim Ruetz
> >>+ *
> >>+ * A generic driver for rotary encoders connected to GPIO lines.
> >>+ * See file:Documentation/input/rotary_encoder.txt for more information
> >>+ *
> >>+ * This program is free software; you can redistribute it and/or modify
> >>+ * it under the terms of the GNU General Public License version 2 as
> >>+ * published by the Free Software Foundation.
> >>+ */
> >>+
> >>+#include <linux/kernel.h>
> >>+#include <linux/module.h>
> >>+#include <linux/init.h>
> >>+#include <linux/interrupt.h>
> >>+#include <linux/input.h>
> >>+#include <linux/device.h>
> >>+#include <linux/platform_device.h>
> >>+#include <linux/gpio.h>
> >>+#include <linux/rotary_encoder.h>
> >>+#include <linux/slab.h>
> >>+#include <linux/workqueue.h>
> >>+#include <linux/sched.h>
> >>+
> >>+#define DRV_NAME "rotary-encoder"
> >>+
> >>+struct rotary_encoder_button_data {
> >>+ struct rotary_encoder_button button;
> >>+ unsigned int irq;
> >>+ struct timer_list timer;
> >>+ struct work_struct work;
> >>+};
> >>+
> >>+struct rotary_encoder {
> >>+ struct input_dev *input;
> >>+ struct rotary_encoder_platform_data *pdata;
> >>+
> >>+ unsigned int pos;
> >>+
> >>+ unsigned int irq_a;
> >>+ unsigned int irq_b;
> >>+
> >>+ bool armed;
> >>+ unsigned char dir; /* 0 - clockwise, 1 - CCW */
> >>+
> >>+ struct rotary_encoder_button_data bdata;
> >>+};
> >>+
> >>+static struct rotary_encoder* encoder;
> >>+
> >>+static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
> >>+{
> >>+ struct rotary_encoder *encoder = dev_id;
> >>+ struct rotary_encoder_platform_data *pdata = encoder->pdata;
> >>+ int a = !!gpio_get_value(pdata->gpio_a);
> >>+ int b = !!gpio_get_value(pdata->gpio_b);
> >>+ int state;
> >>+
> >>+ a ^= pdata->inverted_a;
> >>+ b ^= pdata->inverted_b;
> >>+ state = (a << 1) | b;
> >>+
> >>+ switch (state) {
> >>+
> >>+ case 0x0:
> >>+  if (!encoder->armed)
> >>+   break;
> >>+
> >>+  if (pdata->type == EV_REL) {
> >>+   input_report_rel(encoder->input, pdata->codeleft,
> >>+      encoder->dir ? -1 : 1);
> >>+  } else
> >>+  if (pdata->type == EV_ABS) {
> >>+   unsigned int pos = encoder->pos;
> >>+
> >>+   if (encoder->dir) {
> >>+    /* turning counter-clockwise */
> >>+    if (pdata->rollover)
> >>+     pos += pdata->steps;
> >>+    if (pos)
> >>+     pos--;
> >>+   } else {
> >>+    /* turning clockwise */
> >>+    if (pdata->rollover || pos < pdata->steps)
> >>+     pos++;
> >>+   }
> >>+   if (pdata->rollover)
> >>+    pos %= pdata->steps;
> >>+   encoder->pos = pos;
> >>+   input_report_abs(encoder->input, pdata->codeleft,
> >>+      encoder->pos);
> >>+  } else
> >>+  if (pdata->type == EV_KEY)
> >>+  {
> >>+   input_report_key(encoder->input, encoder->dir ? pdata->codeleft
> >>: pdata->coderight, 1);
> >>+   input_sync(encoder->input);
> >>+   input_report_key(encoder->input, encoder->dir ? pdata->codeleft
> >>: pdata->coderight, 0);
> >>+   input_sync(encoder->input);
> >>+  }
> >>+
> >>+  encoder->armed = false;
> >>+  break;
> >>+
> >>+ case 0x1:
> >>+ case 0x2:
> >>+  if (encoder->armed)
> >>+   encoder->dir = state - 1;
> >>+  break;
> >>+
> >>+ case 0x3:
> >>+  encoder->armed = true;
> >>+  break;
> >>+ }
> >>+
> >>+ return IRQ_HANDLED;
> >>+}
> >>+
> >>+static void rotary_encoder_report_event(struct rotary_encoder *encoder)
> >>+{
> >>+ struct rotary_encoder_button *button = &(encoder->bdata.button);
> >>+ struct input_dev *input = encoder->input;
> >>+ int state = (gpio_get_value(button->gpio) ? 1 : 0) ^
> >>button->active_low;
> >>+
> >>+ input_event(input, EV_KEY, button->code, !!state);
> >>+ input_sync(input);
> >>+}
> >>+
> >>+static void rotary_encoder_work_func(struct work_struct *work)
> >>+{
> >>+ struct rotary_encoder *encoder =
> >>+  container_of(work, struct rotary_encoder, bdata.work);
> >>+
> >>+ rotary_encoder_report_event(encoder);
> >>+}
> >>+
> >>+static void rotary_encoder_timer(unsigned long _data)
> >>+{
> >>+ struct rotary_encoder *encoder = (struct rotary_encoder *)_data;
> >>+
> >>+ schedule_work(&encoder->bdata.work);
> >>+}
> >>+
> >>+static irqreturn_t rotary_encoder_key_irq(int irq, void *dev_id)
> >>+{
> >>+ struct rotary_encoder *encoder = dev_id;
> >>+ struct rotary_encoder_button_data *bdata = &encoder->bdata;
> >>+ struct rotary_encoder_button *button = &bdata->button;
> >>+
> >>+ if (button->debounce_interval)
> >>+  mod_timer(&bdata->timer,
> >>+   jiffies + msecs_to_jiffies(button->debounce_interval));
> >>+ else
> >>+  schedule_work(&bdata->work);
> >>+
> >>+ return IRQ_HANDLED;
> >>+}
> >>+
> >>+static int __devinit rotary_encoder_probe(struct platform_device *pdev)
> >>+{
> >>+ struct rotary_encoder_platform_data *pdata = pdev->dev.platform_data;
> >>+ struct rotary_encoder *encoder;
> >>+ struct input_dev *input;
> >>+ int err;
> >>+
> >>+ if (!pdata) {
> >>+  dev_err(&pdev->dev, "missing platform data\n");
> >>+  return -ENOENT;
> >>+ }
> >>+
> >>+ encoder = kzalloc(sizeof(struct rotary_encoder), GFP_KERNEL);
> >>+ input = input_allocate_device();
> >>+ if (!encoder || !input) {
> >>+  dev_err(&pdev->dev, "failed to allocate memory for device\n");
> >>+  err = -ENOMEM;
> >>+  goto exit_free_mem;
> >>+ }
> >>+
> >>+ encoder->input = input;
> >>+ encoder->pdata = pdata;
> >>+ encoder->irq_a = gpio_to_irq(pdata->gpio_a);
> >>+ encoder->irq_b = gpio_to_irq(pdata->gpio_b);
> >>+ encoder->bdata.irq = gpio_to_irq(pdata->button.gpio);
> >>+
> >>+ /* create and register the input driver */
> >>+ input->name = pdev->name;
> >>+ input->id.bustype = BUS_HOST;
> >>+ input->dev.parent = &pdev->dev;
> >>+
> >>+ /* Enable auto repeat feature of Linux input subsystem */
> >>+ if (pdata->button.rep)
> >>+  set_bit(EV_REP, input->evbit);
> >>+
> >>+ encoder->bdata.button = pdata->button;
> >>+ //input_set_capability(input, EV_KEY, bdata->button.code);
> >>+
> >>+ set_bit(pdata->type, input->evbit);
> >>+ set_bit(pdata->codeleft, input->keybit);
> >>+ set_bit(pdata->coderight, input->keybit);
> >>+ //
> >>+ set_bit(encoder->bdata.button.type, input->evbit);
> >>+ set_bit(encoder->bdata.button.code, input->keybit);
> >>+
> >>+ setup_timer(&encoder->bdata.timer, rotary_encoder_timer, (unsigned
> >>long)encoder);
> >>+ INIT_WORK(&encoder->bdata.work, rotary_encoder_work_func);
> >>+
> >>+ err = input_register_device(input);
> >>+ if (err) {
> >>+  dev_err(&pdev->dev, "failed to register input device\n");
> >>+  goto exit_free_mem;
> >>+ }
> >>+
> >>+ /* request the GPIOs */
> >>+ err = gpio_request(pdata->gpio_a, DRV_NAME);
> >>+ if (err) {
> >>+  dev_err(&pdev->dev, "unable to request GPIO %d\n",
> >>+   pdata->gpio_a);
> >>+  goto exit_unregister_input;
> >>+ }
> >>+
> >>+ err = gpio_direction_input(pdata->gpio_a);
> >>+ if (err) {
> >>+  dev_err(&pdev->dev, "unable to set GPIO %d for input\n",
> >>+   pdata->gpio_a);
> >>+  goto exit_unregister_input;
> >>+ }
> >>+
> >>+ err = gpio_request(pdata->gpio_b, DRV_NAME);
> >>+ if (err) {
> >>+  dev_err(&pdev->dev, "unable to request GPIO %d\n",
> >>+   pdata->gpio_b);
> >>+  goto exit_free_gpio_a;
> >>+ }
> >>+
> >>+ err = gpio_direction_input(pdata->gpio_b);
> >>+ if (err) {
> >>+  dev_err(&pdev->dev, "unable to set GPIO %d for input\n",
> >>+   pdata->gpio_b);
> >>+  goto exit_free_gpio_a;
> >>+ }
> >>+
> >>+ err = gpio_request(pdata->button.gpio, DRV_NAME);
> >>+ if (err) {
> >>+  dev_err(&pdev->dev, "unable to request GPIO %d\n",
> >>+   pdata->button.gpio);
> >>+  goto exit_free_gpio_b;
> >>+ }
> >>+
> >>+ err = gpio_direction_input(pdata->button.gpio);
> >>+ if (err) {
> >>+  dev_err(&pdev->dev, "unable to set GPIO %d for input\n",
> >>+   pdata->button.gpio);
> >>+  goto exit_free_gpio_b;
> >>+ }
> >>+
> >>+ /* request the IRQs */
> >>+ err = request_irq(encoder->irq_a, &rotary_encoder_irq,
> >>+     IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
> >>+     DRV_NAME, encoder);
> >>+ if (err) {
> >>+  dev_err(&pdev->dev, "unable to request IRQ %d\n",
> >>+   encoder->irq_a);
> >>+  goto exit_free_gpio_s;
> >>+ }
> >>+
> >>+ err = request_irq(encoder->irq_b, &rotary_encoder_irq,
> >>+     IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
> >>+     DRV_NAME, encoder);
> >>+ if (err) {
> >>+  dev_err(&pdev->dev, "unable to request IRQ %d\n",
> >>+   encoder->irq_b);
> >>+  goto exit_free_irq_a;
> >>+ }
> >>+
> >>+ err = request_irq(encoder->bdata.irq, &rotary_encoder_key_irq,
> >>+     IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
> >>+     DRV_NAME, encoder);
> >>+ if (err) {
> >>+  dev_err(&pdev->dev, "unable to request IRQ %d\n",
> >>+   encoder->bdata.irq);
> >>+  goto exit_free_irq_b;
> >>+ }
> >>+
> >>+ 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_s:
> >>+ gpio_free(pdata->button.gpio);
> >>+exit_free_gpio_b:
> >>+ gpio_free(pdata->gpio_b);
> >>+exit_free_gpio_a:
> >>+ gpio_free(pdata->gpio_a);
> >>+exit_unregister_input:
> >>+ input_unregister_device(input);
> >>+ input = NULL; /* so we don't try to free it */
> >>+exit_free_mem:
> >>+ input_free_device(input);
> >>+ kfree(encoder);
> >>+ return err;
> >>+}
> >>+
> >>+static int __devexit rotary_encoder_remove(struct platform_device *pdev)
> >>+{
> >>+ struct rotary_encoder *encoder = platform_get_drvdata(pdev);
> >>+ struct rotary_encoder_platform_data *pdata = pdev->dev.platform_data;
> >>+
> >>+ free_irq(encoder->irq_a, encoder);
> >>+ free_irq(encoder->irq_b, encoder);
> >>+ free_irq(encoder->bdata.irq, encoder);
> >>+ if (pdata->button.debounce_interval)
> >>+  del_timer_sync(&encoder->bdata.timer);
> >>+ cancel_work_sync(&encoder->bdata.work);
> >>+ gpio_free(pdata->gpio_a);
> >>+ gpio_free(pdata->gpio_b);
> >>+ gpio_free(pdata->button.gpio);
> >>+ input_unregister_device(encoder->input);
> >>+ platform_set_drvdata(pdev, NULL);
> >>+ kfree(encoder);
> >>+
> >>+ return 0;
> >>+}
> >>+
> >>+static struct platform_driver rotary_encoder_driver = {
> >>+ .probe  = rotary_encoder_probe,
> >>+ .remove  = __devexit_p(rotary_encoder_remove),
> >>+ .driver  = {
> >>+  .name = DRV_NAME,
> >>+  .owner = THIS_MODULE,
> >>+ }
> >>+};
> >>+
> >>+static int __init rotary_encoder_init(void)
> >>+{
> >>+ return platform_driver_register(&rotary_encoder_driver);
> >>+}
> >>+
> >>+static void __exit rotary_encoder_exit(void)
> >>+{
> >>+ platform_driver_unregister(&rotary_encoder_driver);
> >>+}
> >>+
> >>+module_init(rotary_encoder_init);
> >>+module_exit(rotary_encoder_exit);
> >>+
> >>+MODULE_ALIAS("platform:" DRV_NAME);
> >>+MODULE_DESCRIPTION("GPIO rotary encoder driver");
> >>+MODULE_AUTHOR("Daniel Mack <daniel@caiaq.de>");
> >>+MODULE_LICENSE("GPL v2");
> >>+
> >>diff -ur linux-2.6.34/include/linux/rotary_encoder.h
> >>linux/include/linux/rotary_encoder.h
> >>--- linux-2.6.34/include/linux/rotary_encoder.h Mon May 17 01:17:36 2010
> >>+++ linux/include/linux/rotary_encoder.h Thu Jun 17 14:23:26 2010
> >>@@ -1,15 +1,28 @@
> >>#ifndef __ROTARY_ENCODER_H__
> >>#define __ROTARY_ENCODER_H__
> >>
> >>+struct rotary_encoder_button {
> >>+ /* Configuration parameters */
> >>+ u16 code;  /* input event code (KEY_*, SW_*) */
> >>+ int gpio;
> >>+ int active_low;
> >>+ //char *desc;
> >>+ u16 type;  /* input event type (EV_KEY, EV_SW) */
> >>+ int debounce_interval; /* debounce ticks interval in msecs */
> >>+ bool rep;/* enable input subsystem auto repeat */
> >>+};
> >>+
> >>struct rotary_encoder_platform_data {
> >> unsigned int steps;
> >>- unsigned int axis;
> >>+ u16 type; /*(EV_KEY, EV_REL, EV_ABS)*/
> >> unsigned int gpio_a;
> >> unsigned int gpio_b;
> >> unsigned int inverted_a;
> >> unsigned int inverted_b;
> >>- bool relative_axis;
> >>+ u16 codeleft; /*(REL_X, REL_Y, ABS_X, ABS_Y, KEY_...)*/
> >>+ u16 coderight; /*(REL_X, REL_Y, ABS_X, ABS_Y, KEY_...)*/
> >> bool rollover;
> >>+ struct rotary_encoder_button button;
> >>};
> >>
> >>#endif /* __ROTARY_ENCODER_H__ */
> >>
> >>----- Original Message ----- From: "Daniel Mack" <daniel@caiaq.de>
> >>To: "Dmitriy Vasil'ev" <tech@digiton.ru>
> >>Cc: <linux-input@vger.kernel.org>
> >>Sent: Thursday, June 17, 2010 3:21 PM
> >>Subject: Re: rotary encoder
> >>
> >>
> >>>On Thu, Jun 17, 2010 at 02:48:08PM +0400, Dmitriy Vasil'ev wrote:
> >>>>I added .diff file.
> >>>>I am sorry, but I can do it only for kernel version 2.6.34
> >>>>See attach.
> >>>
> >>>Please, just put the patch inline, not as attachment and not zipped.
> >>>That way, the patch can be commented line-by-line.
> >>>
> >>>Thanks,
> >>>Daniel
> >>
> 

Re: rotary encoder

[Dmitry Torokhov]

On Thu, Jun 17, 2010 at 02:00:05PM +0200, Daniel Mack wrote:
> On Thu, Jun 17, 2010 at 03:55:05PM +0400, Dmitriy Vasil'ev wrote:
> > diff utility make this strange result.
> > 
> > I added only one structure, debouncing timer and handle EV_KEY type.
> 
> Yo, and a proper patch generated by a well working tool would show
> exactly this.
> 
> > You can see difference with "total commander" and it command
> > "Files->Compare by content..."
> 
> I don't have this tool. I don't even have Windows. Developers in the
> kernel community have agreed to share patches as unified diffs, and your
> contribution really shouldn't make any difference.
> 
> As a matter of fact, the tool that just gets it right all the way is
> git, and I can only encourage you to get your head around it and use it.
> 
> Also, please read Documentation/SubmittingPatches, which also describes
> the process of collaboration very well.
> 

It looks like Dmitriy's editor of choice happily messed up all
whitespace in the original version converting tabs to spaces.

Pity as it makes the result completely unusable.

-- 
Dmitry

Re: rotary encoder

[Daniel Mack]

On Mon, Jun 28, 2010 at 11:22:26AM -0700, Dmitry Torokhov wrote:
> It looks like Dmitriy's editor of choice happily messed up all
> whitespace in the original version converting tabs to spaces.

Yeah, something like that. Dmitriy, don't get me wrong - any kind of
help is certainly appreciated and it's good you try to push things back
mainline. But please understand that there are some rules to follow :)

Daniel

Re: rotary encoder

[Dmitriy Vasil'ev]

I am sorry, I am not a kernel programmer, I just share my code.
I think, kernel maintainers must heck any code before applying it to kernel.

I have found of the problem:
my editor uses \r\n as end of line symbol.
original code uses \n as end of line

diff utlility could be more flexible....

1. I do not know how many people use encoder without button.
Probably, need correct this code to disable using the button and free a irq, 
if button do not uses.

2. I do not know what code inside the kernel use the rotary encoder.
If this code there is, probably it need correct these code.

My patch, check it please before applying to kernel:

diff -ur linux-2.6.34/Documentation/input/rotary-encoder.txt 
linux/Documentation/input/rotary-encoder.txt
--- linux-2.6.34/Documentation/input/rotary-encoder.txt Mon May 17 01:17:36 
2010
+++ linux/Documentation/input/rotary-encoder.txt Thu Jun 17 13:24:14 2010
@@ -86,16 +86,26 @@

 #define GPIO_ROTARY_A 1
 #define GPIO_ROTARY_B 2
+#define GPIO_ROTARY_S 3

 static struct rotary_encoder_platform_data my_rotary_encoder_info = {
- .steps  = 24,
- .axis  = ABS_X,
- .relative_axis = false,
- .rollover = false,
+ .steps  = 1,
+ .type  = EV_KEY, //(EV_KEY, EV_REL, EV_ABS)
  .gpio_a  = GPIO_ROTARY_A,
  .gpio_b  = GPIO_ROTARY_B,
  .inverted_a = 0,
  .inverted_b = 0,
+ .codeleft = KEY_LEFT, //(REL_X, REL_Y, ABS_X, ABS_Y, KEY_...)
+ .coderight = KEY_RIGHT, //(REL_X, REL_Y, ABS_X, ABS_Y, KEY_...)
+ .rollover = 0,
+ .button  = {
+  .code = KEY_ENTER,
+  .gpio = GPIO_ROTARY_S,
+  .active_low = 1,
+  .type = EV_KEY,
+  .debounce_interval = 10,
+  .rep = false
+ }
 };

 static struct platform_device rotary_encoder_device = {
diff -ur linux-2.6.34/drivers/input/misc/rotary_encoder.c 
linux/drivers/input/misc/rotary_encoder.c
--- linux-2.6.34/drivers/input/misc/rotary_encoder.c Mon May 17 01:17:36 
2010
+++ linux/drivers/input/misc/rotary_encoder.c Tue Jun 29 18:49:16 2010
@@ -1,7 +1,8 @@
 /*
- * rotary_encoder.c
- *
- * (c) 2009 Daniel Mack <daniel@caiaq.de>
+ * based on the:
+ * rotary_encoder.c, (c) 2009 Daniel Mack <daniel@caiaq.de>
+ * and
+ * gpio_keys.c, Copyright 2005 Phil Blundell
  *
  * state machine code inspired by code from Tim Ruetz
  *
@@ -23,14 +24,22 @@
 #include <linux/gpio.h>
 #include <linux/rotary_encoder.h>
 #include <linux/slab.h>
+#include <linux/workqueue.h>
+#include <linux/sched.h>

 #define DRV_NAME "rotary-encoder"

+struct rotary_encoder_button_data {
+ struct rotary_encoder_button button;
+ unsigned int irq;
+ struct timer_list timer;
+ struct work_struct work;
+};
+
 struct rotary_encoder {
  struct input_dev *input;
  struct rotary_encoder_platform_data *pdata;

- unsigned int axis;
  unsigned int pos;

  unsigned int irq_a;
@@ -38,8 +47,12 @@

  bool armed;
  unsigned char dir; /* 0 - clockwise, 1 - CCW */
+
+ struct rotary_encoder_button_data bdata;
 };

+static struct rotary_encoder* encoder;
+
 static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
 {
  struct rotary_encoder *encoder = dev_id;
@@ -58,10 +71,11 @@
   if (!encoder->armed)
    break;

-  if (pdata->relative_axis) {
-   input_report_rel(encoder->input, pdata->axis,
+  if (pdata->type == EV_REL) {
+   input_report_rel(encoder->input, pdata->codeleft,
       encoder->dir ? -1 : 1);
-  } else {
+  } else
+  if (pdata->type == EV_ABS) {
    unsigned int pos = encoder->pos;

    if (encoder->dir) {
@@ -78,8 +92,14 @@
    if (pdata->rollover)
     pos %= pdata->steps;
    encoder->pos = pos;
-   input_report_abs(encoder->input, pdata->axis,
+   input_report_abs(encoder->input, pdata->codeleft,
       encoder->pos);
+  } else
+  //if (pdata->type == EV_KEY)// this if not needed, because we are 
supporting only EV_REL, EV_ABS, EV_KEY
+  {
+   input_report_key(encoder->input, encoder->dir ? pdata->codeleft : 
pdata->coderight, 1);
+   input_sync(encoder->input);
+   input_report_key(encoder->input, encoder->dir ? pdata->codeleft : 
pdata->coderight, 0);
   }
   input_sync(encoder->input);

@@ -100,6 +120,46 @@
  return IRQ_HANDLED;
 }

+static void rotary_encoder_report_event(struct rotary_encoder *encoder)
+{
+ struct rotary_encoder_button *button = &(encoder->bdata.button);
+ struct input_dev *input = encoder->input;
+ int state = (gpio_get_value(button->gpio) ? 1 : 0) ^ button->active_low;
+
+ input_event(input, EV_KEY, button->code, !!state);
+ input_sync(input);
+}
+
+static void rotary_encoder_work_func(struct work_struct *work)
+{
+ struct rotary_encoder *encoder =
+  container_of(work, struct rotary_encoder, bdata.work);
+
+ rotary_encoder_report_event(encoder);
+}
+
+static void rotary_encoder_timer(unsigned long _data)
+{
+ struct rotary_encoder *encoder = (struct rotary_encoder *)_data;
+
+ schedule_work(&encoder->bdata.work);
+}
+
+static irqreturn_t rotary_encoder_key_irq(int irq, void *dev_id)
+{
+ struct rotary_encoder *encoder = dev_id;
+ struct rotary_encoder_button_data *bdata = &encoder->bdata;
+ struct rotary_encoder_button *button = &bdata->button;
+
+ if (button->debounce_interval)
+  mod_timer(&bdata->timer,
+   jiffies + msecs_to_jiffies(button->debounce_interval));
+ else
+  schedule_work(&bdata->work);
+
+ return IRQ_HANDLED;
+}
+
 static int __devinit rotary_encoder_probe(struct platform_device *pdev)
 {
  struct rotary_encoder_platform_data *pdata = pdev->dev.platform_data;
@@ -124,20 +184,27 @@
  encoder->pdata = pdata;
  encoder->irq_a = gpio_to_irq(pdata->gpio_a);
  encoder->irq_b = gpio_to_irq(pdata->gpio_b);
+ encoder->bdata.irq = gpio_to_irq(pdata->button.gpio);

  /* create and register the input driver */
  input->name = pdev->name;
  input->id.bustype = BUS_HOST;
  input->dev.parent = &pdev->dev;

- if (pdata->relative_axis) {
-  input->evbit[0] = BIT_MASK(EV_REL);
-  input->relbit[0] = BIT_MASK(pdata->axis);
- } else {
-  input->evbit[0] = BIT_MASK(EV_ABS);
-  input_set_abs_params(encoder->input,
-         pdata->axis, 0, pdata->steps, 0, 1);
- }
+ /* Enable auto repeat feature of Linux input subsystem */
+ if (pdata->button.rep)
+  set_bit(EV_REP, input->evbit);
+
+ encoder->bdata.button = pdata->button;
+
+ set_bit(pdata->type, input->evbit);
+ set_bit(pdata->codeleft, input->keybit);
+ set_bit(pdata->coderight, input->keybit);
+ set_bit(encoder->bdata.button.type, input->evbit);
+ set_bit(encoder->bdata.button.code, input->keybit);
+
+ setup_timer(&encoder->bdata.timer, rotary_encoder_timer, (unsigned 
long)encoder);
+ INIT_WORK(&encoder->bdata.work, rotary_encoder_work_func);

  err = input_register_device(input);
  if (err) {
@@ -174,6 +241,20 @@
   goto exit_free_gpio_a;
  }

+ err = gpio_request(pdata->button.gpio, DRV_NAME);
+ if (err) {
+  dev_err(&pdev->dev, "unable to request GPIO %d\n",
+   pdata->button.gpio);
+  goto exit_free_gpio_b;
+ }
+
+ err = gpio_direction_input(pdata->button.gpio);
+ if (err) {
+  dev_err(&pdev->dev, "unable to set GPIO %d for input\n",
+   pdata->button.gpio);
+  goto exit_free_gpio_b;
+ }
+
  /* request the IRQs */
  err = request_irq(encoder->irq_a, &rotary_encoder_irq,
      IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
@@ -181,7 +262,7 @@
  if (err) {
   dev_err(&pdev->dev, "unable to request IRQ %d\n",
    encoder->irq_a);
-  goto exit_free_gpio_b;
+  goto exit_free_gpio_s;
  }

  err = request_irq(encoder->irq_b, &rotary_encoder_irq,
@@ -193,12 +274,24 @@
   goto exit_free_irq_a;
  }

+ err = request_irq(encoder->bdata.irq, &rotary_encoder_key_irq,
+     IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
+     DRV_NAME, encoder);
+ if (err) {
+  dev_err(&pdev->dev, "unable to request IRQ %d\n",
+   encoder->bdata.irq);
+  goto exit_free_irq_b;
+ }
+
  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_s:
+ gpio_free(pdata->button.gpio);
 exit_free_gpio_b:
  gpio_free(pdata->gpio_b);
 exit_free_gpio_a:
@@ -219,8 +312,13 @@

  free_irq(encoder->irq_a, encoder);
  free_irq(encoder->irq_b, encoder);
+ free_irq(encoder->bdata.irq, encoder);
+ if (pdata->button.debounce_interval)
+  del_timer_sync(&encoder->bdata.timer);
+ cancel_work_sync(&encoder->bdata.work);
  gpio_free(pdata->gpio_a);
  gpio_free(pdata->gpio_b);
+ gpio_free(pdata->button.gpio);
  input_unregister_device(encoder->input);
  platform_set_drvdata(pdev, NULL);
  kfree(encoder);
diff -ur linux-2.6.34/include/linux/rotary_encoder.h 
linux/include/linux/rotary_encoder.h
--- linux-2.6.34/include/linux/rotary_encoder.h Mon May 17 01:17:36 2010
+++ linux/include/linux/rotary_encoder.h Thu Jun 17 14:23:26 2010
@@ -1,15 +1,28 @@
 #ifndef __ROTARY_ENCODER_H__
 #define __ROTARY_ENCODER_H__

+struct rotary_encoder_button {
+ /* Configuration parameters */
+ u16 code;  /* input event code (KEY_*, SW_*) */
+ int gpio;
+ int active_low;
+ //char *desc;
+ u16 type;  /* input event type (EV_KEY, EV_SW) */
+ int debounce_interval; /* debounce ticks interval in msecs */
+ bool rep;/* enable input subsystem auto repeat */
+};
+
 struct rotary_encoder_platform_data {
  unsigned int steps;
- unsigned int axis;
+ u16 type; /*(EV_KEY, EV_REL, EV_ABS)*/
  unsigned int gpio_a;
  unsigned int gpio_b;
  unsigned int inverted_a;
  unsigned int inverted_b;
- bool relative_axis;
+ u16 codeleft; /*(REL_X, REL_Y, ABS_X, ABS_Y, KEY_...)*/
+ u16 coderight; /*(REL_X, REL_Y, ABS_X, ABS_Y, KEY_...)*/
  bool rollover;
+ struct rotary_encoder_button button;
 };

 #endif /* __ROTARY_ENCODER_H__ */


----- Original Message ----- 
From: "Daniel Mack" <daniel@caiaq.de>
To: "Dmitry Torokhov" <dmitry.torokhov@gmail.com>
Cc: "Dmitriy Vasil'ev" <tech@digiton.ru>; <linux-input@vger.kernel.org>
Sent: Monday, June 28, 2010 10:32 PM
Subject: Re: rotary encoder


> On Mon, Jun 28, 2010 at 11:22:26AM -0700, Dmitry Torokhov wrote:
>> It looks like Dmitriy's editor of choice happily messed up all
>> whitespace in the original version converting tabs to spaces.
>
> Yeah, something like that. Dmitriy, don't get me wrong - any kind of
> help is certainly appreciated and it's good you try to push things back
> mainline. But please understand that there are some rules to follow :)
>
> Daniel 

Re: rotary encoder

[Dmitry Torokhov]

On Tue, Jun 29, 2010 at 07:06:24PM +0400, Dmitriy Vasil'ev wrote:
> I am sorry, I am not a kernel programmer, I just share my code.
> I think, kernel maintainers must heck any code before applying it to kernel.
> 
> I have found of the problem:
> my editor uses \r\n as end of line symbol.
> original code uses \n as end of line
> 
> diff utlility could be more flexible....
> 

The diff utility does not know on which OS the resulting file will be
used so, by default, it reports everything that was changed. And it was
a good thing it showd all these differences as we do not really need DOS
format in kernel code.

> 1. I do not know how many people use encoder without button.
> Probably, need correct this code to disable using the button and
> free a irq, if button do not uses.
> 
> 2. I do not know what code inside the kernel use the rotary encoder.
> If this code there is, probably it need correct these code.

Just grep for it.

> 
> My patch, check it please before applying to kernel:
> 

I will assume that it was Outlook that butchered all indentation, but
no, it does not apply... It also does not seem to follow our kernel
style (and please do not consider us too picky, but we do request
everyone follow the coding style so that review process is easier and
we can concentrate on more important details).

Anyway, I think that mapping rotary actions on EV_KEY does make sense
and would be nice to have; the separate gpio/irq for press button is
better handled as a separate input device by gpio_keys driver.

Thanks.

-- 
Dmitry

Re: rotary encoder

[Dmitriy Vasil'ev]

> the separate gpio/irq for press button is
> better handled as a separate input device by gpio_keys driver.
> 
> Thanks.
> 
> -- 
> Dmitry

Small note:

if you use two separate drivers for rotary encoder and it's button,
you must work with two device nodes in user space.
If user space driver working with one device node, this more usable.

Re: rotary encoder

[Dmitry Torokhov]

On Wed, Jun 30, 2010 at 01:33:25PM +0400, Dmitriy Vasil'ev wrote:
> >the separate gpio/irq for press button is
> >better handled as a separate input device by gpio_keys driver.
> >
> >Thanks.
> >
> >-- 
> >Dmitry
> 
> Small note:
> 
> if you use two separate drivers for rotary encoder and it's button,
> you must work with two device nodes in user space.

Yes.

> If user space driver working with one device node, this more usable.

Not really. Having to deal with multiple input devices is something
that you have to handle. Hotplug, USB (taht sometimes splits its
interfaces into several logical devices), etc - devices are coming
and going and you simply have to handle this.

Thanks.

-- 
Dmitry

Re: Rotary encoder

[Daniel Mack]

Hi Jelle,

(please Cc: linux-input ML for such questions, as I'm not the only one
 decide about that topic ... )

On Tue, May 11, 2010 at 04:10:24PM +0200, Jelle Martijn Kok wrote:
> I saw you had contributed code for the rotary encoder.
> We made a similar device driver, but with some differences.

Good to see more users of the driver, and merging back your changes is
definitely appreciated.

> The differences:
> - We increase and decrease on every "00" and "11"
> - We send out input events "KEY_VOLUMEDOWN" and "KEY_VOLUMEUP".
> 
> So I made some changes to the rotary encoder files, that adds this
> functionality.
> Changes to the header file:
> - "steps" is replaced by "initial_value" "min_value" and "max_value"
> - "event_up" and "event_down" is added
> 
> "rotary_encoder.c" is modified such way that it matches these changes.
> 
> My questions:
> - Is the removal of the "steps" field an issue for compatibility ?

Yes. This driver has active users, so all changes should be
backwards-compatible. We could introduce an "initial_value", but I'm
against deprecating the "steps" parameter.

> - "relative mode" and the "key events" are quite similar, is this "ugly ?

No, they're not. One is an axis information, the other one is a button.
I have no problem adding the feature for button event generation,
though.

> - Are you willing to make these updates to the kernel ?

Could you send your changes as patch against the current kernel's git
HEAD please? That's much easier to review.

Thanks,
Daniel


> #ifndef __ROTARY_ENCODER_H__
> #define __ROTARY_ENCODER_H__
> 
> struct rotary_encoder_platform_data {
> 	unsigned int gpio_a;
> 	unsigned int gpio_b;
> 	unsigned int inverted_a;
> 	unsigned int inverted_b;
> 	unsigned int axis;
> 	unsigned int initial_value;
> 	unsigned int min_value;
> 	unsigned int max_value;
> 	unsigned int event_up;
> 	unsigned int event_down;
> 	bool relative_axis;
> 	bool rollover;
> };
> 
> #endif /* __ROTARY_ENCODER_H__ */

> /*
>  * rotary_encoder.c
>  *
>  * (c) 2009 Daniel Mack <daniel@caiaq.de>
>  *
>  * state machine code inspired by code from Tim Ruetz
>  *
>  * A generic driver for rotary encoders connected to GPIO lines.
>  * See file:Documentation/input/rotary_encoder.txt for more information
>  *
>  * This program is free software; you can redistribute it and/or modify
>  * it under the terms of the GNU General Public License version 2 as
>  * published by the Free Software Foundation.
>  */
> 
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/init.h>
> #include <linux/interrupt.h>
> #include <linux/input.h>
> #include <linux/device.h>
> #include <linux/platform_device.h>
> #include <linux/gpio.h>
> #include <linux/rotary_encoder.h>
> 
> #define DRV_NAME "rotary-encoder"
> 
> struct rotary_encoder {
> 	struct input_dev *input;
> 	struct rotary_encoder_platform_data *pdata;
> 
> 	unsigned int axis;
> 	unsigned int pos;
> 
> 	unsigned int irq_a;
> 	unsigned int irq_b;
> 
> 	int last_state;
> 	//~ bool armed;
> 	//~ unsigned char dir;	/* 0 - clockwise, 1 - CCW */
> };
> 
> static irqreturn_t rotary_encoder_irq(int irq, void *dev_id)
> {
> 	struct rotary_encoder *encoder = dev_id;
> 	struct rotary_encoder_platform_data *pdata = encoder->pdata;
> 	int a = !!gpio_get_value(pdata->gpio_a);
> 	int b = !!gpio_get_value(pdata->gpio_b);
> 	int state;
> 
> 	a ^= pdata->inverted_a;
> 	b ^= pdata->inverted_b;
> 	state = (a << 1) | b;
> 
> 	// Only process in the rest states
> 	if (((state == 0x0) || (state == 0x3)) && ((encoder->last_state == 0x01) || (encoder->last_state == 0x02)))  {
> 		int state_exor = state ^ encoder->last_state;
> 		int dir = 0;
> 
> 		if (state_exor == 0x01) {
> 			dir = +1;
> 			if (pdata->event_up) {
> 				input_report_key(encoder->input, pdata->event_up, 1);
> 				input_report_key(encoder->input, pdata->event_up, 0);
> 			}
> 		}
> 		else {
> 			dir = -1;
> 			if (pdata->event_down) {
> 				input_report_key(encoder->input, pdata->event_down, 1);
> 				input_report_key(encoder->input, pdata->event_down, 0);
> 			}
> 		}
> 
> 		if (dir) {
> 			if (pdata->relative_axis) {
> 				input_report_rel(encoder->input, pdata->axis, dir);
> 			}
> 			else {
> 				unsigned int pos = encoder->pos;
> 
> 				if (dir == -1) {
> 					/* turning counter-clockwise */
> 					if (pos > pdata->min_value)
> 						pos--;
> 					else if (pdata->rollover)
> 						pos = pdata->max_value;
> 				} 
> 				else {
> 					/* turning clockwise */
> 					if (pos < pdata->max_value)
> 						pos++;
> 					else if (pdata->rollover)
> 						pos = pdata->min_value;
> 				}
> 				encoder->pos = pos;
> 				input_report_abs(encoder->input, pdata->axis, encoder->pos);
> 			}
> 			input_sync(encoder->input);
> 		}
> 	}
> 
> 	/* always store the state - even on a 00 or 11 */
> 	encoder->last_state = state;
> 
> 	return IRQ_HANDLED;
> }
> 
> static int __devinit rotary_encoder_probe(struct platform_device *pdev)
> {
> 	struct rotary_encoder_platform_data *pdata = pdev->dev.platform_data;
> 	struct rotary_encoder *encoder;
> 	struct input_dev *input;
> 	int err;
> 
> 	if (!pdata) {
> 		dev_err(&pdev->dev, "missing platform data\n");
> 		return -ENOENT;
> 	}
> 
> 	encoder = kzalloc(sizeof(struct rotary_encoder), GFP_KERNEL);
> 	input = input_allocate_device();
> 	if (!encoder || !input) {
> 		dev_err(&pdev->dev, "failed to allocate memory for device\n");
> 		err = -ENOMEM;
> 		goto exit_free_mem;
> 	}
> 
> 	encoder->input = input;
> 	encoder->pdata = pdata;
> 	encoder->irq_a = gpio_to_irq(pdata->gpio_a);
> 	encoder->irq_b = gpio_to_irq(pdata->gpio_b);
> 	encoder->pos = pdata->initial_value;
> 
> 	/* create and register the input driver */
> 	input->name = pdev->name;
> 	input->id.bustype = BUS_HOST;
> 	input->dev.parent = &pdev->dev;
> 
> 	if (pdata->event_up || pdata->event_down) {
> 		input->evbit[0] |= BIT(EV_KEY);
> 		set_bit(pdata->event_up, input->keybit);
> 		set_bit(pdata->event_down, input->keybit);
> 		clear_bit(KEY_RESERVED, input->keybit);
> 		input_set_capability(input, EV_MSC, MSC_SCAN);
> 	}
> 	if (pdata->relative_axis) {
> 		input->evbit[0] |= BIT_MASK(EV_REL);
> 		input->relbit[0] = BIT(pdata->axis);
> 	}
> 	else {
> 		input->evbit[0] |= BIT_MASK(EV_ABS);
> 		input_set_abs_params(encoder->input, pdata->axis, 
> 			pdata->min_value, pdata->max_value, 
> 			0, 1);
> 	}
> 
> 	err = input_register_device(input);
> 	if (err) {
> 		dev_err(&pdev->dev, "failed to register input device\n");
> 		goto exit_free_mem;
> 	}
> 
> 	/* request the GPIOs */
> 	err = gpio_request(pdata->gpio_a, DRV_NAME);
> 	if (err) {
> 		dev_err(&pdev->dev, "unable to request GPIO %d\n",
> 			pdata->gpio_a);
> 		goto exit_unregister_input;
> 	}
> 
> 	err = gpio_request(pdata->gpio_b, DRV_NAME);
> 	if (err) {
> 		dev_err(&pdev->dev, "unable to request GPIO %d\n",
> 			pdata->gpio_b);
> 		goto exit_free_gpio_a;
> 	}
> 
> 	/* request the IRQs */
> 	err = request_irq(encoder->irq_a, &rotary_encoder_irq,
> 			  IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
> 			  DRV_NAME, encoder);
> 	if (err) {
> 		dev_err(&pdev->dev, "unable to request IRQ %d\n",
> 			encoder->irq_a);
> 		goto exit_free_gpio_b;
> 	}
> 
> 	err = request_irq(encoder->irq_b, &rotary_encoder_irq,
> 			  IORESOURCE_IRQ_HIGHEDGE | IORESOURCE_IRQ_LOWEDGE,
> 			  DRV_NAME, encoder);
> 	if (err) {
> 		dev_err(&pdev->dev, "unable to request IRQ %d\n",
> 			encoder->irq_b);
> 		goto exit_free_irq_a;
> 	}
> 
> 	platform_set_drvdata(pdev, encoder);
> 
> 	return 0;
> 
> 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_unregister_input:
> 	input_unregister_device(input);
> 	input = NULL; /* so we don't try to free it */
> exit_free_mem:
> 	input_free_device(input);
> 	kfree(encoder);
> 	return err;
> }
> 
> static int __devexit rotary_encoder_remove(struct platform_device *pdev)
> {
> 	struct rotary_encoder *encoder = platform_get_drvdata(pdev);
> 	struct rotary_encoder_platform_data *pdata = pdev->dev.platform_data;
> 
> 	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);
> 	platform_set_drvdata(pdev, NULL);
> 	kfree(encoder);
> 
> 	return 0;
> }
> 
> static struct platform_driver rotary_encoder_driver = {
> 	.probe		= rotary_encoder_probe,
> 	.remove		= __devexit_p(rotary_encoder_remove),
> 	.driver		= {
> 		.name	= DRV_NAME,
> 		.owner	= THIS_MODULE,
> 	}
> };
> 
> static int __init rotary_encoder_init(void)
> {
> 	return platform_driver_register(&rotary_encoder_driver);
> }
> 
> static void __exit rotary_encoder_exit(void)
> {
> 	platform_driver_unregister(&rotary_encoder_driver);
> }
> 
> module_init(rotary_encoder_init);
> module_exit(rotary_encoder_exit);
> 
> MODULE_ALIAS("platform:" DRV_NAME);
> MODULE_DESCRIPTION("GPIO rotary encoder driver");
> MODULE_AUTHOR("Daniel Mack <daniel@caiaq.de>");
> MODULE_LICENSE("GPL v2");
>