[PATCH 0/2] GPIO joystick driver

[PATCH 0/2] GPIO joystick driver

[Hans Holmberg]

This series adds a driver for digital joysticks connected
via GPIOs.

Tested on Rasperry Pi and Minnowboard MAX using evtest.

The DSDT AML-code snippet below illustrates how to configure a joystick
on the Minnowboard MAX, using the new _DSD support in the kernel.


Scope (_SB)
{
        Device (JOY)
        {
            Name (_HID, "PRP0001")
            Name (_CRS, ResourceTemplate () {
               GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
                  "\\_SB.GPO2", 0, ResourceConsumer) {1}
               GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
                  "\\_SB.GPO2", 0, ResourceConsumer) {0}
               GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
                  "\\_SB.GPO0", 0, ResourceConsumer) {95}
               GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
                  "\\_SB.GPO0", 0, ResourceConsumer) {94}
               GpioIo (Exclusive, PullUp, 0, 0, IoRestrictionInputOnly,
                  "\\_SB.GPO2", 0, ResourceConsumer) {2}
            })

            Name (_DSD, Package () {
                ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
                Package () {
                    Package () {"compatible", "gpio-joy"},
                    Package () {"debounce-interval-ms", 15},
                    Package () {"left-gpio",    Package () {^JOY, 0, 0, 1}},
                    Package () {"right-gpio",   Package () {^JOY, 1, 0, 1}},
                    Package () {"up-gpio",      Package () {^JOY, 2, 0, 1}},
                    Package () {"down-gpio",    Package () {^JOY, 3, 0, 1}},
                    Package () {"button-gpio",  Package () {^JOY, 4, 0, 1}},
               }
            })
        }
}

Best regards,
Hans

Hans Holmberg (2):
  Documentation: Device Tree binding information for gpio-joy driver
  input: gpio-joy - GPIO joystick driver

 .../devicetree/bindings/input/gpio-joy.txt         |  29 +++
 drivers/input/joystick/Kconfig                     |  10 +
 drivers/input/joystick/Makefile                    |   1 +
 drivers/input/joystick/gpio_joy.c                  | 208 +++++++++++++++++++++
 4 files changed, 248 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/gpio-joy.txt
 create mode 100644 drivers/input/joystick/gpio_joy.c

-- 
1.9.1

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

[PATCH 2/2] input: gpio-joy - GPIO joystick driver

[Hans Holmberg]

This is an interrupt-driven driver for digital joysticks
connected to GPIOs. Supports any digital joystick with
signals for up, down, left, right and one signal for
trigger button press, i.e. C64/Atari joysticks.

Signed-off-by: Hans Holmberg <hans.holmberg@intel.com>
---
 drivers/input/joystick/Kconfig    |  10 ++
 drivers/input/joystick/Makefile   |   1 +
 drivers/input/joystick/gpio_joy.c | 208 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 219 insertions(+)
 create mode 100644 drivers/input/joystick/gpio_joy.c

diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index 56eb471..de0220e 100644
--- a/drivers/input/joystick/Kconfig
+++ b/drivers/input/joystick/Kconfig
@@ -70,6 +70,16 @@ config JOYSTICK_GF2K
 	  To compile this driver as a module, choose M here: the
 	  module will be called gf2k.
 
+config JOYSTICK_GPIO
+	tristate "Joysticks connected to GPIOs"
+	depends on GPIOLIB
+	help
+	 Say Y here if you have one or more joysticks connected to your
+	 device directly via GPIOs.
+
+	 To compile this driver as a module, choose M here: the
+	 module will be called gpio_joy.
+
 config JOYSTICK_GRIP
 	tristate "Gravis GrIP joysticks and gamepads"
 	select GAMEPORT
diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
index 92dc0de..c20a51a 100644
--- a/drivers/input/joystick/Makefile
+++ b/drivers/input/joystick/Makefile
@@ -13,6 +13,7 @@ obj-$(CONFIG_JOYSTICK_COBRA)		+= cobra.o
 obj-$(CONFIG_JOYSTICK_DB9)		+= db9.o
 obj-$(CONFIG_JOYSTICK_GAMECON)		+= gamecon.o
 obj-$(CONFIG_JOYSTICK_GF2K)		+= gf2k.o
+obj-$(CONFIG_JOYSTICK_GPIO)		+= gpio_joy.o
 obj-$(CONFIG_JOYSTICK_GRIP)		+= grip.o
 obj-$(CONFIG_JOYSTICK_GRIP_MP)		+= grip_mp.o
 obj-$(CONFIG_JOYSTICK_GUILLEMOT)	+= guillemot.o
diff --git a/drivers/input/joystick/gpio_joy.c b/drivers/input/joystick/gpio_joy.c
new file mode 100644
index 0000000..d0e22b3
--- /dev/null
+++ b/drivers/input/joystick/gpio_joy.c
@@ -0,0 +1,208 @@
+/*
+ * Driver for digital joysticks connected using GPIOs
+ *
+ * Copyright (C) 2015, Intel Corporation
+ * Author: Hans Holmberg  <hans.holmberg@intel.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/gpio/consumer.h>
+#include <linux/workqueue.h>
+#include <linux/kernel.h>
+#include <linux/irq.h>
+#include <linux/interrupt.h>
+#include <linux/platform_device.h>
+#include <linux/timer.h>
+#include <linux/jiffies.h>
+#include <linux/input.h>
+#include <linux/property.h>
+
+#define DRV_NAME "gpio-joy"
+#define DEFAULT_DEBOUNCE_MS 10
+
+enum control_pin_indices {
+	LEFT,
+	RIGHT,
+	UP,
+	DOWN,
+	BUTTON,
+	NUM_CONTROL_PINS,
+};
+
+static const char *gpio_ids[NUM_CONTROL_PINS] = {
+	"left", "right", "up", "down", "button"};
+
+struct control_pin {
+	struct gpio_desc *gpio;
+	int irq;
+};
+
+struct gpio_joy_drvdata {
+	struct input_dev *input;
+	struct delayed_work work;
+	unsigned long debounce_jiffies;
+	struct control_pin control_pins[NUM_CONTROL_PINS];
+};
+
+static void gpio_joy_report_state(struct gpio_joy_drvdata *ddata)
+{
+	struct input_dev *input = ddata->input;
+
+	input_report_abs(input, ABS_X,
+		gpiod_get_value_cansleep(ddata->control_pins[RIGHT].gpio) -
+		gpiod_get_value_cansleep(ddata->control_pins[LEFT].gpio));
+
+	input_report_abs(input, ABS_Y,
+		gpiod_get_value_cansleep(ddata->control_pins[DOWN].gpio) -
+		gpiod_get_value_cansleep(ddata->control_pins[UP].gpio));
+
+	input_report_key(input, BTN_TRIGGER,
+		gpiod_get_value_cansleep(ddata->control_pins[BUTTON].gpio));
+
+	input_sync(input);
+}
+
+static void gpio_joy_work(struct work_struct *work)
+{
+	struct delayed_work *delayed_work =
+		container_of(work, struct delayed_work, work);
+	struct gpio_joy_drvdata *ddata =
+		container_of(delayed_work, struct gpio_joy_drvdata, work);
+
+	gpio_joy_report_state(ddata);
+}
+
+static irqreturn_t gpio_joy_isr(int irq, void *dev_id)
+{
+	struct gpio_joy_drvdata *ddata = dev_id;
+
+	mod_delayed_work(system_wq, &ddata->work, ddata->debounce_jiffies);
+
+	return IRQ_HANDLED;
+}
+
+static void gpio_joy_quiesce(void *data)
+{
+	struct gpio_joy_drvdata *ddata = data;
+
+	cancel_delayed_work_sync(&ddata->work);
+}
+
+static int gpio_joy_probe(struct platform_device *pdev)
+{
+	struct device *dev = &pdev->dev;
+	struct input_dev *input;
+	struct gpio_joy_drvdata *ddata;
+	int i, err;
+	unsigned int debounce_ms;
+
+	ddata = devm_kzalloc(dev, sizeof(struct gpio_joy_drvdata), GFP_KERNEL);
+	if (!ddata)
+		return -ENOMEM;
+
+	input = devm_input_allocate_device(dev);
+	if (!input)
+		return -ENOMEM;
+
+	ddata->input = input;
+	platform_set_drvdata(pdev, ddata);
+
+	input->name = pdev->name;
+	input->phys = DRV_NAME"/input0";
+	input->dev.parent = &pdev->dev;
+
+	input->id.bustype = BUS_HOST;
+	input->id.vendor = 0x0001;
+	input->id.product = 0x0001;
+	input->id.version = 0x0100;
+
+	set_bit(EV_KEY, input->evbit);
+	set_bit(EV_ABS, input->evbit);
+	set_bit(BTN_TRIGGER, input->keybit);
+
+	input_set_abs_params(input, ABS_X, -1, 1, 0, 0);
+	input_set_abs_params(input, ABS_Y, -1, 1, 0, 0);
+
+	/* debounce interval is optional */
+	if (device_property_read_u32(dev, "debounce-interval-ms", &debounce_ms))
+		debounce_ms = DEFAULT_DEBOUNCE_MS;
+	ddata->debounce_jiffies = msecs_to_jiffies(debounce_ms);
+
+	for (i = 0; i < ARRAY_SIZE(ddata->control_pins) ; i++) {
+		struct gpio_desc *gpio;
+		int irq;
+
+		gpio = devm_gpiod_get(&pdev->dev,
+				gpio_ids[i]);
+		if (IS_ERR(gpio)) {
+			dev_err(dev, "unable to allocate gpio: %s\n",
+					gpio_ids[i]);
+			err = PTR_ERR(gpio);
+			goto fail;
+		}
+		ddata->control_pins[i].gpio = gpio;
+
+		irq = gpiod_to_irq(gpio);
+		if (irq < 0) {
+			dev_err(dev, "unable to get irq for gpio: %s\n",
+					gpio_ids[i]);
+			err = irq;
+			goto fail;
+		}
+		ddata->control_pins[i].irq = irq;
+	}
+
+	INIT_DELAYED_WORK(&ddata->work, gpio_joy_work);
+	err = devm_add_action(dev, gpio_joy_quiesce, ddata);
+
+	for (i = 0; i < ARRAY_SIZE(ddata->control_pins) ; i++) {
+		err = devm_request_any_context_irq(dev,
+				ddata->control_pins[i].irq,
+				gpio_joy_isr,
+				IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING |
+				IRQF_ONESHOT,
+				input->name, ddata);
+
+		if (err < 0) {
+			dev_err(dev, "unable to claim irq %d\n",
+				ddata->control_pins[i].irq);
+			goto fail;
+		}
+	}
+
+	err = input_register_device(input);
+	if (err) {
+		dev_err(dev, "unable to register input device\n");
+		goto fail;
+	}
+
+	return 0;
+fail:
+	return err;
+}
+
+static const struct of_device_id gpio_joy_of_match[] = {
+	{ .compatible = DRV_NAME, },
+	{ },
+};
+
+static struct platform_driver gpio_joy_driver = {
+	.probe		= gpio_joy_probe,
+	.driver		= {
+		.name	= DRV_NAME,
+		.of_match_table = gpio_joy_of_match,
+	}
+};
+
+module_platform_driver(gpio_joy_driver);
+
+MODULE_ALIAS("platform:" DRV_NAME);
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Hans Holmberg <hans.holmberg@intel.com>");
+MODULE_DESCRIPTION("Driver for digital joysticks connected via GPIOs");
-- 
1.9.1

Re: [PATCH 2/2] input: gpio-joy - GPIO joystick driver

[Mark Rutland]

On Fri, Feb 06, 2015 at 09:32:02AM +0000, Hans Holmberg wrote:
> This is an interrupt-driven driver for digital joysticks
> connected to GPIOs. Supports any digital joystick with
> signals for up, down, left, right and one signal for
> trigger button press, i.e. C64/Atari joysticks.
> 
> Signed-off-by: Hans Holmberg <hans.holmberg-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/input/joystick/Kconfig    |  10 ++
>  drivers/input/joystick/Makefile   |   1 +
>  drivers/input/joystick/gpio_joy.c | 208 ++++++++++++++++++++++++++++++++++++++
>  3 files changed, 219 insertions(+)
>  create mode 100644 drivers/input/joystick/gpio_joy.c

[...]

> +static int gpio_joy_probe(struct platform_device *pdev)
> +{
> +	struct device *dev = &pdev->dev;
> +	struct input_dev *input;
> +	struct gpio_joy_drvdata *ddata;
> +	int i, err;
> +	unsigned int debounce_ms;

This should be a u32 given you pass it to a function expecting a u32
pointer.

> +
> +	ddata = devm_kzalloc(dev, sizeof(struct gpio_joy_drvdata), GFP_KERNEL);

Use sizeof(*ddata)

[...]

> +	return 0;
> +fail:
> +	return err;
> +}

Given there's no cleanup, the fail path seems redundant.

[...]

> +static const struct of_device_id gpio_joy_of_match[] = {
> +	{ .compatible = DRV_NAME, },
> +	{ },
> +};

The compatible string should be independent of DRV_NAME, because it must
match the binding. The driver can be arbitrarily and independently renamed.

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

RE: [PATCH 2/2] input: gpio-joy - GPIO joystick driver

[Holmberg, Hans]

> > +static int gpio_joy_probe(struct platform_device *pdev) {
> > +	struct device *dev = &pdev->dev;
> > +	struct input_dev *input;
> > +	struct gpio_joy_drvdata *ddata;
> > +	int i, err;
> > +	unsigned int debounce_ms;
> 
> This should be a u32 given you pass it to a function expecting a u32 pointer.
> 
> > +
> > +	ddata = devm_kzalloc(dev, sizeof(struct gpio_joy_drvdata),
> > +GFP_KERNEL);
> 
> Use sizeof(*ddata)
> 
> [...]
> 
> > +	return 0;
> > +fail:
> > +	return err;
> > +}
> 
> Given there's no cleanup, the fail path seems redundant.
> 
> [...]
> 
> > +static const struct of_device_id gpio_joy_of_match[] = {
> > +	{ .compatible = DRV_NAME, },
> > +	{ },
> > +};
> 
> The compatible string should be independent of DRV_NAME, because it must
> match the binding. The driver can be arbitrarily and independently renamed.
> 

I'll address all of these comments, thanks for reviewing!
Hans

[PATCH 1/2] Documentation: Device Tree binding information for gpio-joy driver

[Hans Holmberg]

This document describes the Device Tree bindings for the gpio-joy
driver.

Signed-off-by: Hans Holmberg <hans.holmberg-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 .../devicetree/bindings/input/gpio-joy.txt         | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/input/gpio-joy.txt

diff --git a/Documentation/devicetree/bindings/input/gpio-joy.txt b/Documentation/devicetree/bindings/input/gpio-joy.txt
new file mode 100644
index 0000000..ef8ad09
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/gpio-joy.txt
@@ -0,0 +1,29 @@
+Device-Tree bindings for input/joystick/gpio_joy.c joystick driver
+
+This driver can be used for connecting i.e. C64 and Atari joysticks.
+
+Required properties:
+	- compatible    = "gpio-joy";
+
+	- left-gpio     : gpio connected to the "left" joystick signal
+	- right-gpio    : gpio connected to the "right" joystick signal
+	- up-gpio       : gpio connected to the "up" joystick signal
+	- down-gpio     : gpio connected to the "down" joystick signal
+	- button-gpio   : gpio connected to the "button" joystick signal
+		See OF device-tree gpio specification.
+
+Optional properties:
+	- debounce-interval-ms: debounce interval in milliseconds for connected pins
+		Default is 10 milliseconds
+
+Example node:
+
+joystick {
+	compatible = "gpio-joy";
+	left-gpio = <&gpio 24 GPIO_ACTIVE_LOW>;
+	right-gpio = <&gpio 10 GPIO_ACTIVE_LOW>;
+	up-gpio	= <&gpio 3 GPIO_ACTIVE_LOW>;
+	down-gpio = <&gpio 27 GPIO_ACTIVE_LOW>;
+	button-gpio = <&gpio 2 GPIO_ACTIVE_LOW>;
+};
+
-- 
1.9.1

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

Re: [PATCH 1/2] Documentation: Device Tree binding information for gpio-joy driver

[Mark Rutland]

On Fri, Feb 06, 2015 at 09:32:01AM +0000, Hans Holmberg wrote:
> This document describes the Device Tree bindings for the gpio-joy
> driver.
> 
> Signed-off-by: Hans Holmberg <hans.holmberg-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  .../devicetree/bindings/input/gpio-joy.txt         | 29 ++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/gpio-joy.txt
> 
> diff --git a/Documentation/devicetree/bindings/input/gpio-joy.txt b/Documentation/devicetree/bindings/input/gpio-joy.txt
> new file mode 100644
> index 0000000..ef8ad09
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/gpio-joy.txt
> @@ -0,0 +1,29 @@
> +Device-Tree bindings for input/joystick/gpio_joy.c joystick driver
> +
> +This driver can be used for connecting i.e. C64 and Atari joysticks.

The binding should be for a class of hardware, not for a driver. Please
describe the class of hardware the binding represents, and remove
references to the Linux driver.

> +Required properties:
> +	- compatible    = "gpio-joy";

That does not strike me as a very informative compatible string. At the
least, s/joy/joystick/.

> +
> +	- left-gpio     : gpio connected to the "left" joystick signal
> +	- right-gpio    : gpio connected to the "right" joystick signal
> +	- up-gpio       : gpio connected to the "up" joystick signal
> +	- down-gpio     : gpio connected to the "down" joystick signal
> +	- button-gpio   : gpio connected to the "button" joystick signal

Only one button?

Thanks,
Mark.

> +		See OF device-tree gpio specification.
> +
> +Optional properties:
> +	- debounce-interval-ms: debounce interval in milliseconds for connected pins
> +		Default is 10 milliseconds
> +
> +Example node:
> +
> +joystick {
> +	compatible = "gpio-joy";
> +	left-gpio = <&gpio 24 GPIO_ACTIVE_LOW>;
> +	right-gpio = <&gpio 10 GPIO_ACTIVE_LOW>;
> +	up-gpio	= <&gpio 3 GPIO_ACTIVE_LOW>;
> +	down-gpio = <&gpio 27 GPIO_ACTIVE_LOW>;
> +	button-gpio = <&gpio 2 GPIO_ACTIVE_LOW>;
> +};
> +
> -- 
> 1.9.1
> 
> --
> To unsubscribe from this list: send the line "unsubscribe devicetree" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] Documentation: Device Tree binding information for gpio-joy driver

[Holmberg, Hans]

> The binding should be for a class of hardware, not for a driver. Please
> describe the class of hardware the binding represents, and remove
> references to the Linux driver.

Will do.

> 
> > +Required properties:
> > +	- compatible    = "gpio-joy";
> 
> That does not strike me as a very informative compatible string. At the least,
> s/joy/joystick/.

You're very right, gpio-joystick is better and as about as specific as it can be I believe.

> > +
> > +	- left-gpio     : gpio connected to the "left" joystick signal
> > +	- right-gpio    : gpio connected to the "right" joystick signal
> > +	- up-gpio       : gpio connected to the "up" joystick signal
> > +	- down-gpio     : gpio connected to the "down" joystick signal
> > +	- button-gpio   : gpio connected to the "button" joystick signal
> 
> Only one button?

Hmm, yes, but it would make more sense to have sub-nodes defining an arbitrary 
number of buttons instead, right?

Thanks for reviewing!
Hans

Re: [PATCH 0/2] GPIO joystick driver

[Dmitry Torokhov]

Hi Hans,

On February 6, 2015 1:32:00 AM PST, Hans Holmberg <hans.holmberg-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>This series adds a driver for digital joysticks connected
>via GPIOs.

Why do we need this new driver when we have gpio-keys driver that can be configured to emit ABS_* events?

Thanks.

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

RE: [PATCH 0/2] GPIO joystick driver

[Holmberg, Hans]

Hi Dmitry,

> Why do we need this new driver when we have gpio-keys driver that can be
> configured to emit ABS_* events?
> 

As far as I can tell, there is no way to specify values for ABS-"keys" in the
device tree binding. 

While both the device tree binding and the gpio-keys driver could possibly be adapted to 
support this, it makes sense to me to describe joysticks in a separate binding and keep
the driver implementations apart.

In addition to this,  auto-repeat and the sys fs disable interface for individual keys 
does not make sense for joysticks.

In my mind, keyboards and joysticks are different enough.

Thanks for looking at this,
Hans

Re: [PATCH 0/2] GPIO joystick driver

[Dmitry Torokhov]

Hi Hans,

On Sat, Feb 07, 2015 at 07:35:46AM +0000, Holmberg, Hans wrote:
> Hi Dmitry,
> 
> > Why do we need this new driver when we have gpio-keys driver that can be
> > configured to emit ABS_* events?
> > 
> 
> As far as I can tell, there is no way to specify values for ABS-"keys" in the
> device tree binding. 

It may mot be present in device tree binding, but the driver does
support it, so I would rather extend the binding than have a brand new
driver.

> 
> While both the device tree binding and the gpio-keys driver could possibly be adapted to 
> support this, it makes sense to me to describe joysticks in a separate binding and keep
> the driver implementations apart.

Why? From the input core perspective there is no difference. It is just
a device that emits set of events.

> 
> In addition to this,  auto-repeat and the sys fs disable interface for individual keys 
> does not make sense for joysticks.

You do not need to use it when you describe the device (BTW autorepeat
for trigger might be a good thing).

Thanks.

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

RE: [PATCH 0/2] GPIO joystick driver

[Holmberg, Hans]

> > As far as I can tell, there is no way to specify values for ABS-"keys"
> > in the device tree binding.
> 
> It may not be present in device tree binding, but the driver does support it,
> so I would rather extend the binding than have a brand new driver.

All right, I find it a bit like whacking a round peg down a square hole, but I'll head down that path and see where it leads.

> > While both the device tree binding and the gpio-keys driver could
> > possibly be adapted to support this, it makes sense to me to describe
> > joysticks in a separate binding and keep the driver implementations apart.
> 
> Why? From the input core perspective there is no difference. It is just a
> device that emits set of events.
> 

>From a hardware-description/devicee-tree binding perspective its confusing (at least to me) to describe joysticks as keyboards.

> > In addition to this,  auto-repeat and the sys fs disable interface for
> > individual keys does not make sense for joysticks.
> 
> You do not need to use it when you describe the device (BTW autorepeat for
> trigger might be a good thing).

Ah, yes. autofire. 

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

RE: [PATCH 0/2] GPIO joystick driver

[Holmberg, Hans]

> > > As far as I can tell, there is no way to specify values for ABS-"keys"
> > > in the device tree binding.
> >
> > It may not be present in device tree binding, but the driver does
> > support it, so I would rather extend the binding than have a brand new
> driver.
> 
> All right, I find it a bit like whacking a round peg down a square hole, but I'll
> head down that path and see where it leads.
> 

This is what I've found:

A digital joystick driver needs to report three states per axis: min, max and neutral. The hardware only have two signals per axis(i.e. up and down) however. See http://wiki.icomp.de/wiki/DB9-Joystick 

This makes gpio-keys pretty much impossible to use for joysticks - as the driver only reports abs-events when a button/key is pressed, how would the neutral state be reported?  (as there is no signal for "neutral")

Even if through some hackery the driver would be modified to report a special neutral event if all  "buttons/keys" for that axis are not pressed, what value should be reported?  An average of the values for that axis?   Also, reporting such an implicit neutral event would break the behavior of existing abs-reporting instances, right?

Do you still prefer this approach?

Thanks,
Hans

Re: [PATCH 0/2] GPIO joystick driver

['Dmitry Torokhov']

On Wed, Feb 11, 2015 at 08:49:16AM +0000, Holmberg, Hans wrote:
> > > > As far as I can tell, there is no way to specify values for ABS-"keys"
> > > > in the device tree binding.
> > >
> > > It may not be present in device tree binding, but the driver does
> > > support it, so I would rather extend the binding than have a brand new
> > driver.
> > 
> > All right, I find it a bit like whacking a round peg down a square hole, but I'll
> > head down that path and see where it leads.
> > 
> 
> This is what I've found:
> 
> A digital joystick driver needs to report three states per axis: min,
> max and neutral. The hardware only have two signals per axis(i.e. up
> and down) however. See http://wiki.icomp.de/wiki/DB9-Joystick 
> 
> This makes gpio-keys pretty much impossible to use for joysticks - as
> the driver only reports abs-events when a button/key is pressed, how
> would the neutral state be reported?  (as there is no signal for
> "neutral")
> 
> Even if through some hackery the driver would be modified to report a
> special neutral event if all  "buttons/keys" for that axis are not
> pressed, what value should be reported?  An average of the values for
> that axis?   Also, reporting such an implicit neutral event would
> break the behavior of existing abs-reporting instances, right?

You have 2 gpio per axis, and since you can't measure how "far" down the
axis you have moved the driver should only report values -1, 0, 1 per
axis. You start at neutral position, and move right. IRQ for right gpio
is fired and you report EV_ABS/ABS_X/1. Then you start moving to the
left, gpio IRQ fires (you want it to trigger on both edges) and you
report EV_ABS/ABS_X/0. You continue moving to the left and IRQ for left
gpio fires up and you report EV_ABS/ABS_X/-1.

This assumes that you assign EV_ABS/ABS_X and value 1 for right gpio and
EV_ABS/ABS_X and value -1 for left gpio.

Thanks.

-- 
Dmitry

RE: [PATCH 0/2] GPIO joystick driver

[Holmberg, Hans]

> > A digital joystick driver needs to report three states per axis: min,
> > max and neutral. The hardware only have two signals per axis(i.e. up
> > and down) however. See http://wiki.icomp.de/wiki/DB9-Joystick
> >
> > This makes gpio-keys pretty much impossible to use for joysticks - as
> > the driver only reports abs-events when a button/key is pressed, how
> > would the neutral state be reported?  (as there is no signal for
> > "neutral")
> >
> > Even if through some hackery the driver would be modified to report a
> > special neutral event if all  "buttons/keys" for that axis are not
> > pressed, what value should be reported?  An average of the values for
> > that axis?   Also, reporting such an implicit neutral event would
> > break the behavior of existing abs-reporting instances, right?
> 
> You have 2 gpio per axis, and since you can't measure how "far" down the
> axis you have moved the driver should only report values -1, 0, 1 per axis.
> You start at neutral position, and move right. IRQ for right gpio is fired and
> you report EV_ABS/ABS_X/1. Then you start moving to the left, gpio IRQ fires
> (you want it to trigger on both edges) and you report EV_ABS/ABS_X/0. You
> continue moving to the left and IRQ for left gpio fires up and you report
> EV_ABS/ABS_X/-1.
> This assumes that you assign EV_ABS/ABS_X and value 1 for right gpio and
> EV_ABS/ABS_X and value -1 for left gpio.

Right, but the problem I was referring to is that "up/release" events are not reported by gpio-keys.c today:

        if (type == EV_ABS) {
                if (state)
                        input_event(input, type, button->code, button->value);
        } else {
                input_event(input, type, button->code, !!state);
        }

I see three solutions:

1. Start reporting ABS=0 if state == 0 - this is what I would expect of the driver, but would break the current behavior.
2. Add an optional down-abs-value per key. If it is not specified, It won't be reported. 
3. Add a device-global property ("report-neutral-abs") that will report ABS=0 on all "up" events for abs-keys.

Comments?

In any case, I have a patch set based on (3) that works (but that I need to test a bit more), enabling gpio-keys to support digital joysticks using the new device property api.

I had  to define the abs-value device tree property as  a 2's complement signed 32 integer. Negative numbers are not commonly used but the device tree compiler supports it.  In the ACPI case, the iasl compiler only supports unsigned ints, but it's easy enough to work around (-1 <-> 0xFFFFFFFF) .

Thanks!
Hans