Re: [PATCH v4 2/2] input: add ADC resistor ladder driver
On Fri, Jul 29, 2016 at 12:49:21AM +0200, Alexandre Belloni wrote: > On 28/07/2016 at 15:09:18 -0700, Dmitry Torokhov wrote : > > On Tue, Jul 12, 2016 at 05:41:50PM -0700, Dmitry Torokhov wrote: > > > Hi Alexandre, > > > > > > On Tue, Jul 12, 2016 at 09:36:26PM +0200, Alexandre Belloni wrote: > > > > A common way of multiplexing buttons on a single input in cheap devices > > > > is > > > > to use a resistor ladder on an ADC. This driver supports that > > > > configuration > > > > by polling an ADC channel provided by IIO. > > > > > > This looks quite reasonable, just a few small comments. > > > > Ping. > > > > Did the version below work for you? I am trying sweep in the brand new > > drivers before we close merge window. > > > > I was thinking it was too late for 4.8. I've juste tested and it works Nah, it is OK - the dirver is brand-new so we can't possibly regress anyone ;) > fine. However, I just sent v5 including the exact same changes but > changing mvolt to millivolt as that seemed preferred by Rob. Both > versions haver been tested and are fine for me. Let's see if he acks the binding then... Thanks. -- Dmitry
Re: [PATCH v4 2/2] input: add ADC resistor ladder driver
On 28/07/2016 at 15:09:18 -0700, Dmitry Torokhov wrote : > On Tue, Jul 12, 2016 at 05:41:50PM -0700, Dmitry Torokhov wrote: > > Hi Alexandre, > > > > On Tue, Jul 12, 2016 at 09:36:26PM +0200, Alexandre Belloni wrote: > > > A common way of multiplexing buttons on a single input in cheap devices is > > > to use a resistor ladder on an ADC. This driver supports that > > > configuration > > > by polling an ADC channel provided by IIO. > > > > This looks quite reasonable, just a few small comments. > > Ping. > > Did the version below work for you? I am trying sweep in the brand new > drivers before we close merge window. > I was thinking it was too late for 4.8. I've juste tested and it works fine. However, I just sent v5 including the exact same changes but changing mvolt to millivolt as that seemed preferred by Rob. Both versions haver been tested and are fine for me. -- Alexandre Belloni, Free Electrons Embedded Linux and Kernel engineering http://free-electrons.com
Re: [PATCH v4 2/2] input: add ADC resistor ladder driver
On Tue, Jul 12, 2016 at 05:41:50PM -0700, Dmitry Torokhov wrote:
> Hi Alexandre,
>
> On Tue, Jul 12, 2016 at 09:36:26PM +0200, Alexandre Belloni wrote:
> > A common way of multiplexing buttons on a single input in cheap devices is
> > to use a resistor ladder on an ADC. This driver supports that configuration
> > by polling an ADC channel provided by IIO.
>
> This looks quite reasonable, just a few small comments.
Ping.
Did the version below work for you? I am trying sweep in the brand new
drivers before we close merge window.
Thanks!
>
> >
> > Acked-by: Jonathan Cameron
> > Signed-off-by: Alexandre Belloni
> > ---
> > drivers/input/keyboard/Kconfig| 15 +++
> > drivers/input/keyboard/Makefile | 1 +
> > drivers/input/keyboard/adc-keys.c | 210
> > ++
> > 3 files changed, 226 insertions(+)
> > create mode 100644 drivers/input/keyboard/adc-keys.c
> >
> > diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> > index 509608c95994..4cf042cc5e63 100644
> > --- a/drivers/input/keyboard/Kconfig
> > +++ b/drivers/input/keyboard/Kconfig
> > @@ -12,6 +12,21 @@ menuconfig INPUT_KEYBOARD
> >
> > if INPUT_KEYBOARD
> >
> > +config KEYBOARD_ADC
> > + tristate "ADC ladder Buttons"
> > + depends on IIO
> > + select INPUT_POLLDEV
> > + help
> > + This driver implements support for buttons connected
> > + to an ADC using a resistor ladder.
> > +
> > + Say Y here if your device has such buttons connected to an ADC. Your
> > + board-specific setup logic must also provide a configuration data
> > + saying mapping voltages to buttons.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called adc_keys.
> > +
> > config KEYBOARD_ADP5520
> > tristate "Keypad Support for ADP5520 PMIC"
> > depends on PMIC_ADP5520
> > diff --git a/drivers/input/keyboard/Makefile
> > b/drivers/input/keyboard/Makefile
> > index 1d416ddf84e4..d9f4cfcf3410 100644
> > --- a/drivers/input/keyboard/Makefile
> > +++ b/drivers/input/keyboard/Makefile
> > @@ -4,6 +4,7 @@
> >
> > # Each configuration option enables a list of files.
> >
> > +obj-$(CONFIG_KEYBOARD_ADC) += adc-keys.o
> > obj-$(CONFIG_KEYBOARD_ADP5520) += adp5520-keys.o
> > obj-$(CONFIG_KEYBOARD_ADP5588) += adp5588-keys.o
> > obj-$(CONFIG_KEYBOARD_ADP5589) += adp5589-keys.o
> > diff --git a/drivers/input/keyboard/adc-keys.c
> > b/drivers/input/keyboard/adc-keys.c
> > new file mode 100644
> > index ..cf299ff517a0
> > --- /dev/null
> > +++ b/drivers/input/keyboard/adc-keys.c
> > @@ -0,0 +1,210 @@
> > +/* Input driver for resistor ladder connected on ADC
> > + *
> > + * Copyright (c) 2016 Alexandre Belloni
> > + *
> > + * 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
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +struct adc_keys_button {
> > + u32 voltage;
> > + u32 keycode;
> > +};
> > +
> > +struct adc_keys_state {
> > + struct iio_channel *channel;
> > + u32 num_keys;
> > + u32 last_key;
> > + u32 keyup_voltage;
> > + struct adc_keys_button *map;
>
> const
>
> > +};
> > +
> > +static void adc_keys_poll(struct input_polled_dev *dev)
> > +{
> > + struct adc_keys_state *st = dev->private;
> > + int i, value, ret;
> > + u32 diff, closest = 0x;
> > + int keycode = 0;
> > +
> > + ret = iio_read_channel_processed(st->channel, &value);
> > + if (ret < 0) {
>
> > + if (st->last_key) {
> > + input_report_key(dev->input, st->last_key, 0);
> > + input_sync(dev->input);
> > + st->last_key = 0;
> > + }
> > + return;
> > + }
> > +
> > + for (i = 0; i < st->num_keys; i++) {
> > + diff = abs(st->map[i].voltage - value);
> > + if (diff < closest) {
> > + closest = diff;
> > + keycode = st->map[i].keycode;
> > + }
> > + }
> > +
> > + if (abs(st->keyup_voltage - value) < closest) {
> > + input_report_key(dev->input, st->last_key, 0);
> > + st->last_key = 0;
> > + } else {
> > + if (st->last_key && st->last_key != keycode)
> > + input_report_key(dev->input, st->last_key, 0);
> > + input_report_key(dev->input, keycode, 1);
> > + st->last_key = keycode;
> > + }
>
> I think this can be simplified a bit, see version below.
>
> > +
> > + input_sync(dev->input);
> > +}
> > +
> > +static int adc_keys_load_dt_keymap(struct device *dev,
> > + struct adc_keys_state *st)
> > +{
> > + struct device_node *pp, *np = dev->of_nod
Re: [PATCH v4 2/2] input: add ADC resistor ladder driver
Hi Alexandre,
On Tue, Jul 12, 2016 at 09:36:26PM +0200, Alexandre Belloni wrote:
> A common way of multiplexing buttons on a single input in cheap devices is
> to use a resistor ladder on an ADC. This driver supports that configuration
> by polling an ADC channel provided by IIO.
This looks quite reasonable, just a few small comments.
>
> Acked-by: Jonathan Cameron
> Signed-off-by: Alexandre Belloni
> ---
> drivers/input/keyboard/Kconfig| 15 +++
> drivers/input/keyboard/Makefile | 1 +
> drivers/input/keyboard/adc-keys.c | 210
> ++
> 3 files changed, 226 insertions(+)
> create mode 100644 drivers/input/keyboard/adc-keys.c
>
> diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
> index 509608c95994..4cf042cc5e63 100644
> --- a/drivers/input/keyboard/Kconfig
> +++ b/drivers/input/keyboard/Kconfig
> @@ -12,6 +12,21 @@ menuconfig INPUT_KEYBOARD
>
> if INPUT_KEYBOARD
>
> +config KEYBOARD_ADC
> + tristate "ADC ladder Buttons"
> + depends on IIO
> + select INPUT_POLLDEV
> + help
> + This driver implements support for buttons connected
> + to an ADC using a resistor ladder.
> +
> + Say Y here if your device has such buttons connected to an ADC. Your
> + board-specific setup logic must also provide a configuration data
> + saying mapping voltages to buttons.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called adc_keys.
> +
> config KEYBOARD_ADP5520
> tristate "Keypad Support for ADP5520 PMIC"
> depends on PMIC_ADP5520
> diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
> index 1d416ddf84e4..d9f4cfcf3410 100644
> --- a/drivers/input/keyboard/Makefile
> +++ b/drivers/input/keyboard/Makefile
> @@ -4,6 +4,7 @@
>
> # Each configuration option enables a list of files.
>
> +obj-$(CONFIG_KEYBOARD_ADC) += adc-keys.o
> obj-$(CONFIG_KEYBOARD_ADP5520) += adp5520-keys.o
> obj-$(CONFIG_KEYBOARD_ADP5588) += adp5588-keys.o
> obj-$(CONFIG_KEYBOARD_ADP5589) += adp5589-keys.o
> diff --git a/drivers/input/keyboard/adc-keys.c
> b/drivers/input/keyboard/adc-keys.c
> new file mode 100644
> index ..cf299ff517a0
> --- /dev/null
> +++ b/drivers/input/keyboard/adc-keys.c
> @@ -0,0 +1,210 @@
> +/* Input driver for resistor ladder connected on ADC
> + *
> + * Copyright (c) 2016 Alexandre Belloni
> + *
> + * 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
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +struct adc_keys_button {
> + u32 voltage;
> + u32 keycode;
> +};
> +
> +struct adc_keys_state {
> + struct iio_channel *channel;
> + u32 num_keys;
> + u32 last_key;
> + u32 keyup_voltage;
> + struct adc_keys_button *map;
const
> +};
> +
> +static void adc_keys_poll(struct input_polled_dev *dev)
> +{
> + struct adc_keys_state *st = dev->private;
> + int i, value, ret;
> + u32 diff, closest = 0x;
> + int keycode = 0;
> +
> + ret = iio_read_channel_processed(st->channel, &value);
> + if (ret < 0) {
> + if (st->last_key) {
> + input_report_key(dev->input, st->last_key, 0);
> + input_sync(dev->input);
> + st->last_key = 0;
> + }
> + return;
> + }
> +
> + for (i = 0; i < st->num_keys; i++) {
> + diff = abs(st->map[i].voltage - value);
> + if (diff < closest) {
> + closest = diff;
> + keycode = st->map[i].keycode;
> + }
> + }
> +
> + if (abs(st->keyup_voltage - value) < closest) {
> + input_report_key(dev->input, st->last_key, 0);
> + st->last_key = 0;
> + } else {
> + if (st->last_key && st->last_key != keycode)
> + input_report_key(dev->input, st->last_key, 0);
> + input_report_key(dev->input, keycode, 1);
> + st->last_key = keycode;
> + }
I think this can be simplified a bit, see version below.
> +
> + input_sync(dev->input);
> +}
> +
> +static int adc_keys_load_dt_keymap(struct device *dev,
> +struct adc_keys_state *st)
> +{
> + struct device_node *pp, *np = dev->of_node;
> + int i;
> +
> + st->num_keys = of_get_child_count(np);
> + if (st->num_keys == 0) {
> + dev_err(dev, "keymap is missing\n");
> + return -EINVAL;
> + }
There is no need to limit this driver to OF, generic device properties
will allow us to use it on DT, ACPI and legacy boards.
> +
> + st->map = devm_kmalloc_array(dev, st->num_ke
[PATCH v4 2/2] input: add ADC resistor ladder driver
A common way of multiplexing buttons on a single input in cheap devices is
to use a resistor ladder on an ADC. This driver supports that configuration
by polling an ADC channel provided by IIO.
Acked-by: Jonathan Cameron
Signed-off-by: Alexandre Belloni
---
drivers/input/keyboard/Kconfig| 15 +++
drivers/input/keyboard/Makefile | 1 +
drivers/input/keyboard/adc-keys.c | 210 ++
3 files changed, 226 insertions(+)
create mode 100644 drivers/input/keyboard/adc-keys.c
diff --git a/drivers/input/keyboard/Kconfig b/drivers/input/keyboard/Kconfig
index 509608c95994..4cf042cc5e63 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -12,6 +12,21 @@ menuconfig INPUT_KEYBOARD
if INPUT_KEYBOARD
+config KEYBOARD_ADC
+ tristate "ADC ladder Buttons"
+ depends on IIO
+ select INPUT_POLLDEV
+ help
+ This driver implements support for buttons connected
+ to an ADC using a resistor ladder.
+
+ Say Y here if your device has such buttons connected to an ADC. Your
+ board-specific setup logic must also provide a configuration data
+ saying mapping voltages to buttons.
+
+ To compile this driver as a module, choose M here: the
+ module will be called adc_keys.
+
config KEYBOARD_ADP5520
tristate "Keypad Support for ADP5520 PMIC"
depends on PMIC_ADP5520
diff --git a/drivers/input/keyboard/Makefile b/drivers/input/keyboard/Makefile
index 1d416ddf84e4..d9f4cfcf3410 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -4,6 +4,7 @@
# Each configuration option enables a list of files.
+obj-$(CONFIG_KEYBOARD_ADC) += adc-keys.o
obj-$(CONFIG_KEYBOARD_ADP5520) += adp5520-keys.o
obj-$(CONFIG_KEYBOARD_ADP5588) += adp5588-keys.o
obj-$(CONFIG_KEYBOARD_ADP5589) += adp5589-keys.o
diff --git a/drivers/input/keyboard/adc-keys.c
b/drivers/input/keyboard/adc-keys.c
new file mode 100644
index ..cf299ff517a0
--- /dev/null
+++ b/drivers/input/keyboard/adc-keys.c
@@ -0,0 +1,210 @@
+/* Input driver for resistor ladder connected on ADC
+ *
+ * Copyright (c) 2016 Alexandre Belloni
+ *
+ * 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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+struct adc_keys_button {
+ u32 voltage;
+ u32 keycode;
+};
+
+struct adc_keys_state {
+ struct iio_channel *channel;
+ u32 num_keys;
+ u32 last_key;
+ u32 keyup_voltage;
+ struct adc_keys_button *map;
+};
+
+static void adc_keys_poll(struct input_polled_dev *dev)
+{
+ struct adc_keys_state *st = dev->private;
+ int i, value, ret;
+ u32 diff, closest = 0x;
+ int keycode = 0;
+
+ ret = iio_read_channel_processed(st->channel, &value);
+ if (ret < 0) {
+ if (st->last_key) {
+ input_report_key(dev->input, st->last_key, 0);
+ input_sync(dev->input);
+ st->last_key = 0;
+ }
+ return;
+ }
+
+ for (i = 0; i < st->num_keys; i++) {
+ diff = abs(st->map[i].voltage - value);
+ if (diff < closest) {
+ closest = diff;
+ keycode = st->map[i].keycode;
+ }
+ }
+
+ if (abs(st->keyup_voltage - value) < closest) {
+ input_report_key(dev->input, st->last_key, 0);
+ st->last_key = 0;
+ } else {
+ if (st->last_key && st->last_key != keycode)
+ input_report_key(dev->input, st->last_key, 0);
+ input_report_key(dev->input, keycode, 1);
+ st->last_key = keycode;
+ }
+
+ input_sync(dev->input);
+}
+
+static int adc_keys_load_dt_keymap(struct device *dev,
+ struct adc_keys_state *st)
+{
+ struct device_node *pp, *np = dev->of_node;
+ int i;
+
+ st->num_keys = of_get_child_count(np);
+ if (st->num_keys == 0) {
+ dev_err(dev, "keymap is missing\n");
+ return -EINVAL;
+ }
+
+ st->map = devm_kmalloc_array(dev, st->num_keys, sizeof(*st->map),
+GFP_KERNEL);
+ if (!st->map)
+ return -ENOMEM;
+
+ i = 0;
+ for_each_child_of_node(np, pp) {
+ struct adc_keys_button *map = &st->map[i];
+
+ if (of_property_read_u32(pp, "press-threshold-mvolt",
+&map->voltage)) {
+ dev_err(dev, "%s: Invalid or missing voltage\n",
+ pp->name);
+ return -EI
