* [PATCH] Input: add support for PIUIO input/output board
From: Devin J. Pohly @ 2018-01-23 19:20 UTC (permalink / raw)
To: linux-input; +Cc: Dmitry Torokhov, Devin J. Pohly
In-Reply-To: <20180123192029.9468-1-djpohly@gmail.com>
The PIUIO is a digital input/output board most often found in arcade
dance cabinets. It uses a polling-based USB protocol to get/set the
state of inputs and outputs, with the potential for up to 48 of each
(though actual boards have fewer physical connections).
This commit provides a driver which exposes the inputs as joystick
buttons and the outputs as LEDs.
The driver also supports a smaller form of the board with 8 inputs and
outputs which uses the same protocol.
Signed-off-by: Devin J. Pohly <djpohly@gmail.com>
---
MAINTAINERS | 6 +
drivers/input/joystick/Kconfig | 11 +
drivers/input/joystick/Makefile | 1 +
drivers/input/joystick/piuio.c | 672 ++++++++++++++++++++++++++++++++++++++++
4 files changed, 690 insertions(+)
create mode 100644 drivers/input/joystick/piuio.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 2f4e462aa4a2..a39675bff62a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10691,6 +10691,12 @@ F: arch/mips/include/asm/mach-pistachio/
F: arch/mips/boot/dts/img/pistachio*
F: arch/mips/configs/pistachio*_defconfig
+PIUIO DRIVER
+M: Devin J. Pohly <djpohly@gmail.com>
+W: https://github.com/djpohly/piuio
+S: Maintained
+F: drivers/input/joystick/piuio.c
+
PKTCDVD DRIVER
S: Orphan
M: linux-block@vger.kernel.org
diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index f3c2f6ea8b44..5d156e760db8 100644
--- a/drivers/input/joystick/Kconfig
+++ b/drivers/input/joystick/Kconfig
@@ -351,4 +351,15 @@ config JOYSTICK_PSXPAD_SPI_FF
To drive rumble motor a dedicated power supply is required.
+config JOYSTICK_PIUIO
+ tristate "PIUIO input/output interface"
+ depends on USB_ARCH_HAS_HCD
+ select USB
+ help
+ Say Y here if you want to use the PIUIO interface board for
+ digital inputs/outputs.
+
+ To compile this driver as a module, choose M here: the
+ module will be called piuio.
+
endif
diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
index 67651efda2e1..33c4e54fb516 100644
--- a/drivers/input/joystick/Makefile
+++ b/drivers/input/joystick/Makefile
@@ -22,6 +22,7 @@ obj-$(CONFIG_JOYSTICK_INTERACT) += interact.o
obj-$(CONFIG_JOYSTICK_JOYDUMP) += joydump.o
obj-$(CONFIG_JOYSTICK_MAGELLAN) += magellan.o
obj-$(CONFIG_JOYSTICK_MAPLE) += maplecontrol.o
+obj-$(CONFIG_JOYSTICK_PIUIO) += piuio.o
obj-$(CONFIG_JOYSTICK_PSXPAD_SPI) += psxpad-spi.o
obj-$(CONFIG_JOYSTICK_SIDEWINDER) += sidewinder.o
obj-$(CONFIG_JOYSTICK_SPACEBALL) += spaceball.o
diff --git a/drivers/input/joystick/piuio.c b/drivers/input/joystick/piuio.c
new file mode 100644
index 000000000000..e1d9b34692e2
--- /dev/null
+++ b/drivers/input/joystick/piuio.c
@@ -0,0 +1,672 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+// Andamiro PIU IO input module, for use with the PIUIO input/output boards
+// commonly found in arcade dance cabinets.
+//
+// Copyright (C) 2012-2018 Devin J. Pohly (djpohly@gmail.com)
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/stat.h>
+#include <linux/sysfs.h>
+#include <linux/errno.h>
+#include <linux/bitops.h>
+#include <linux/leds.h>
+#include <linux/wait.h>
+#include <linux/jiffies.h>
+#include <linux/input.h>
+#include <linux/usb.h>
+#include <linux/usb/input.h>
+
+
+/*
+ * Device and protocol definitions
+ */
+#define USB_VENDOR_ID_ANCHOR 0x0547
+#define USB_PRODUCT_ID_PYTHON2 0x1002
+#define USB_VENDOR_ID_BTNBOARD 0x0d2f
+#define USB_PRODUCT_ID_BTNBOARD 0x1010
+
+/* USB message used to communicate with the device */
+#define PIUIO_MSG_REQ 0xae
+#define PIUIO_MSG_VAL 0
+#define PIUIO_MSG_IDX 0
+
+#define PIUIO_MSG_SZ 8
+#define PIUIO_MSG_LONGS (PIUIO_MSG_SZ / sizeof(unsigned long))
+
+/* Input keycode ranges */
+#define PIUIO_BTN_REG BTN_JOYSTICK
+#define PIUIO_NUM_REG (BTN_GAMEPAD - BTN_JOYSTICK)
+#define PIUIO_BTN_EXTRA BTN_TRIGGER_HAPPY
+#define PIUIO_NUM_EXTRA (KEY_MAX - BTN_TRIGGER_HAPPY)
+#define PIUIO_NUM_BTNS (PIUIO_NUM_REG + PIUIO_NUM_EXTRA)
+
+
+#ifdef CONFIG_LEDS_CLASS
+/**
+ * struct piuio_led - auxiliary struct for led devices
+ * @piu: Pointer back to the enclosing structure
+ * @dev: Actual led device
+ */
+struct piuio_led {
+ struct piuio *piu;
+ struct led_classdev dev;
+};
+
+static const char *const led_names[] = {
+ "piuio::output0",
+ "piuio::output1",
+ "piuio::output2",
+ "piuio::output3",
+ "piuio::output4",
+ "piuio::output5",
+ "piuio::output6",
+ "piuio::output7",
+ "piuio::output8",
+ "piuio::output9",
+ "piuio::output10",
+ "piuio::output11",
+ "piuio::output12",
+ "piuio::output13",
+ "piuio::output14",
+ "piuio::output15",
+ "piuio::output16",
+ "piuio::output17",
+ "piuio::output18",
+ "piuio::output19",
+ "piuio::output20",
+ "piuio::output21",
+ "piuio::output22",
+ "piuio::output23",
+ "piuio::output24",
+ "piuio::output25",
+ "piuio::output26",
+ "piuio::output27",
+ "piuio::output28",
+ "piuio::output29",
+ "piuio::output30",
+ "piuio::output31",
+ "piuio::output32",
+ "piuio::output33",
+ "piuio::output34",
+ "piuio::output35",
+ "piuio::output36",
+ "piuio::output37",
+ "piuio::output38",
+ "piuio::output39",
+ "piuio::output40",
+ "piuio::output41",
+ "piuio::output42",
+ "piuio::output43",
+ "piuio::output44",
+ "piuio::output45",
+ "piuio::output46",
+ "piuio::output47",
+};
+
+static const char *const bbled_names[] = {
+ "piuio::bboutput0",
+ "piuio::bboutput1",
+ "piuio::bboutput2",
+ "piuio::bboutput3",
+ "piuio::bboutput4",
+ "piuio::bboutput5",
+ "piuio::bboutput6",
+ "piuio::bboutput7",
+};
+#endif
+
+/**
+ * struct piuio_devtype - parameters for different types of PIUIO devices
+ * @led_names: Array of LED names, of length @outputs, to use in sysfs
+ * @inputs: Number of input pins
+ * @outputs: Number of output pins
+ * @mplex: Number of sets of inputs
+ * @mplex_bits: Number of output bits reserved for multiplexing
+ */
+struct piuio_devtype {
+#ifdef CONFIG_LEDS_CLASS
+ const char *const *led_names;
+#endif
+ int inputs;
+ int outputs;
+ int mplex;
+ int mplex_bits;
+};
+
+/**
+ * struct piuio - state of each attached PIUIO
+ * @type: Type of PIUIO device (currently either full or buttonboard)
+ * @idev: Input device associated with this PIUIO
+ * @phys: Physical path of the device. @idev's phys field points to this
+ * buffer
+ * @udev: USB device associated with this PIUIO
+ * @in: URB for requesting the current state of one set of inputs
+ * @out: URB for sending data to outputs and multiplexer
+ * @cr_in: Setup packet for @in URB
+ * @cr_out: Setup packet for @out URB
+ * @old_inputs: Previous state of input pins from the @in URB for each of the
+ * input sets. These are used to determine when a press or release
+ * has happened for a group of correlated inputs.
+ * @inputs: Buffer for the @in URB
+ * @outputs: Buffer for the @out URB
+ * @new_outputs:
+ * Staging for the @outputs buffer
+ * @set: Current set of inputs to read (0 .. @type->mplex - 1)
+ */
+struct piuio {
+ struct piuio_devtype *type;
+
+ struct input_dev *idev;
+ char phys[64];
+
+ struct usb_device *udev;
+ struct urb *in, *out;
+ struct usb_ctrlrequest cr_in, cr_out;
+ wait_queue_head_t shutdown_wait;
+
+ unsigned long (*old_inputs)[PIUIO_MSG_LONGS];
+ unsigned long inputs[PIUIO_MSG_LONGS];
+ unsigned char outputs[PIUIO_MSG_SZ];
+ unsigned char new_outputs[PIUIO_MSG_SZ];
+
+#ifdef CONFIG_LEDS_CLASS
+ struct piuio_led *led;
+#endif
+
+ int set;
+};
+
+/* Full device parameters */
+static struct piuio_devtype piuio_dev_full = {
+#ifdef CONFIG_LEDS_CLASS
+ .led_names = led_names,
+#endif
+ .inputs = (PIUIO_NUM_BTNS < 48) ? PIUIO_NUM_BTNS : 48,
+ .outputs = 48,
+ .mplex = 4,
+ .mplex_bits = 2,
+};
+
+/* Button board device parameters */
+static struct piuio_devtype piuio_dev_bb = {
+#ifdef CONFIG_LEDS_CLASS
+ .led_names = bbled_names,
+#endif
+ .inputs = (PIUIO_NUM_BTNS < 8) ? PIUIO_NUM_BTNS : 8,
+ .outputs = 8,
+ .mplex = 1,
+ .mplex_bits = 0,
+};
+
+
+/*
+ * Auxiliary functions for reporting input events
+ */
+static int keycode(unsigned int pin)
+{
+ /* Use joystick buttons first, then the extra "trigger happy" range. */
+ if (pin < PIUIO_NUM_REG)
+ return PIUIO_BTN_REG + pin;
+ pin -= PIUIO_NUM_REG;
+ return PIUIO_BTN_EXTRA + pin;
+}
+
+
+/*
+ * URB completion handlers
+ */
+static void piuio_in_completed(struct urb *urb)
+{
+ struct piuio *piu = urb->context;
+ unsigned long changed[PIUIO_MSG_LONGS];
+ unsigned long b;
+ int i, s;
+ int cur_set;
+ int ret = urb->status;
+
+ if (ret) {
+ dev_warn(&piu->udev->dev,
+ "piuio callback(in): error %d\n", ret);
+ goto resubmit;
+ }
+
+ /* Get index of the previous input set (always 0 if no multiplexer) */
+ cur_set = (piu->set + piu->type->mplex - 1) % piu->type->mplex;
+
+ /* Note what has changed in this input set, then store the inputs for
+ * next time
+ */
+ for (i = 0; i < PIUIO_MSG_LONGS; i++) {
+ changed[i] = piu->inputs[i] ^ piu->old_inputs[cur_set][i];
+ piu->old_inputs[cur_set][i] = piu->inputs[i];
+ }
+
+ /* If we are using a multiplexer, changes only count when none of the
+ * corresponding inputs in other sets are pressed. Since "pressed"
+ * reads as 0, we can use & to knock those bits out of the changes.
+ */
+ for (s = 0; s < piu->type->mplex; s++) {
+ if (s == cur_set)
+ continue;
+ for (i = 0; i < PIUIO_MSG_LONGS; i++)
+ changed[i] &= piu->old_inputs[s][i];
+ }
+
+ /* For each input which has changed state, report whether it was pressed
+ * or released based on the current value.
+ */
+ for_each_set_bit(b, changed, piu->type->inputs) {
+ input_event(piu->idev, EV_MSC, MSC_SCAN, b + 1);
+ input_report_key(piu->idev, keycode(b),
+ !test_bit(b, piu->inputs));
+ }
+
+ /* Done reporting input events */
+ input_sync(piu->idev);
+
+resubmit:
+ ret = usb_submit_urb(urb, GFP_ATOMIC);
+ if (ret == -EPERM)
+ dev_info(&piu->udev->dev, "piuio resubmit(in): shutdown\n");
+ else if (ret)
+ dev_err(&piu->udev->dev, "piuio resubmit(in): error %d\n", ret);
+
+ /* Let any waiting threads know we're done here */
+ wake_up(&piu->shutdown_wait);
+}
+
+static void piuio_out_completed(struct urb *urb)
+{
+ struct piuio *piu = urb->context;
+ int ret = urb->status;
+
+ if (ret) {
+ dev_warn(&piu->udev->dev,
+ "piuio callback(out): error %d\n", ret);
+ goto resubmit;
+ }
+
+ /* Copy in the new outputs */
+ memcpy(piu->outputs, piu->new_outputs, PIUIO_MSG_SZ);
+
+ /* If we have a multiplexer, switch to the next input set in rotation
+ * and set the appropriate output bits
+ */
+ piu->set = (piu->set + 1) % piu->type->mplex;
+
+ /* Set multiplexer bits */
+ piu->outputs[0] &= ~((1 << piu->type->mplex_bits) - 1);
+ piu->outputs[0] |= piu->set;
+ piu->outputs[2] &= ~((1 << piu->type->mplex_bits) - 1);
+ piu->outputs[2] |= piu->set;
+
+resubmit:
+ ret = usb_submit_urb(piu->out, GFP_ATOMIC);
+ if (ret == -EPERM)
+ dev_info(&piu->udev->dev, "piuio resubmit(out): shutdown\n");
+ else if (ret)
+ dev_err(&piu->udev->dev,
+ "piuio resubmit(out): error %d\n", ret);
+
+ /* Let any waiting threads know we're done here */
+ wake_up(&piu->shutdown_wait);
+}
+
+
+/*
+ * Input device events
+ */
+static int piuio_open(struct input_dev *idev)
+{
+ struct piuio *piu = input_get_drvdata(idev);
+ int ret;
+
+ /* Kick off the polling */
+ ret = usb_submit_urb(piu->out, GFP_KERNEL);
+ if (ret) {
+ dev_err(&piu->udev->dev, "piuio submit(out): error %d\n", ret);
+ return -EIO;
+ }
+
+ ret = usb_submit_urb(piu->in, GFP_KERNEL);
+ if (ret) {
+ dev_err(&piu->udev->dev, "piuio submit(in): error %d\n", ret);
+ usb_kill_urb(piu->out);
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static void piuio_close(struct input_dev *idev)
+{
+ struct piuio *piu = input_get_drvdata(idev);
+ long remaining;
+
+ /* Stop polling, but wait for the last requests to complete */
+ usb_block_urb(piu->in);
+ usb_block_urb(piu->out);
+ remaining = wait_event_timeout(piu->shutdown_wait,
+ atomic_read(&piu->in->use_count) == 0 &&
+ atomic_read(&piu->out->use_count) == 0,
+ msecs_to_jiffies(5));
+ usb_unblock_urb(piu->in);
+ usb_unblock_urb(piu->out);
+
+ if (!remaining) {
+ // Timed out
+ dev_warn(&piu->udev->dev, "piuio close: urb timeout\n");
+ usb_kill_urb(piu->in);
+ usb_kill_urb(piu->out);
+ }
+
+ /* XXX Reset the outputs? */
+}
+
+
+/*
+ * Structure initialization and destruction
+ */
+static void piuio_input_init(struct piuio *piu, struct device *parent)
+{
+ struct input_dev *idev = piu->idev;
+ int i;
+
+ /* Fill in basic fields */
+ idev->name = "PIUIO input";
+ idev->phys = piu->phys;
+ usb_to_input_id(piu->udev, &idev->id);
+ idev->dev.parent = parent;
+
+ /* HACK: Buttons are sufficient to trigger a /dev/input/js* device, but
+ * for systemd (and consequently udev and Xorg) to consider us a
+ * joystick, we have to have a set of XY absolute axes.
+ */
+ set_bit(EV_KEY, idev->evbit);
+ set_bit(EV_ABS, idev->evbit);
+
+ /* Configure buttons */
+ for (i = 0; i < piu->type->inputs; i++)
+ set_bit(keycode(i), idev->keybit);
+ clear_bit(0, idev->keybit);
+
+ /* Configure fake axes */
+ set_bit(ABS_X, idev->absbit);
+ set_bit(ABS_Y, idev->absbit);
+ input_set_abs_params(idev, ABS_X, 0, 0, 0, 0);
+ input_set_abs_params(idev, ABS_Y, 0, 0, 0, 0);
+
+ /* Set device callbacks */
+ idev->open = piuio_open;
+ idev->close = piuio_close;
+
+ /* Link input device back to PIUIO */
+ input_set_drvdata(idev, piu);
+}
+
+
+#ifdef CONFIG_LEDS_CLASS
+/*
+ * Led device event
+ */
+static void piuio_led_set(struct led_classdev *dev, enum led_brightness b)
+{
+ struct piuio_led *led = container_of(dev, struct piuio_led, dev);
+ struct piuio *piu = led->piu;
+ int n;
+
+ n = led - piu->led;
+ if (n > piu->type->outputs) {
+ dev_err(&piu->udev->dev, "piuio led: bad number %d\n", n);
+ return;
+ }
+
+ /* Meh, forget atomicity, these aren't super-important */
+ if (b)
+ __set_bit(n, (unsigned long *) piu->new_outputs);
+ else
+ __clear_bit(n, (unsigned long *) piu->new_outputs);
+}
+
+int
+piu_led_register(struct piuio_led *led)
+{
+ const struct attribute_group **ag;
+ struct attribute **attr;
+ int ret;
+
+ /* Register led device */
+ ret = led_classdev_register(&led->piu->udev->dev, &led->dev);
+ if (ret)
+ return ret;
+
+ /* Relax permissions on led attributes */
+ for (ag = led->dev.dev->class->dev_groups; *ag; ag++) {
+ for (attr = (*ag)->attrs; *attr; attr++) {
+ ret = sysfs_chmod_file(&led->dev.dev->kobj, *attr,
+ 0666);
+ if (ret) {
+ led_classdev_unregister(&led->dev);
+ return ret;
+ }
+ }
+ }
+ return 0;
+}
+
+static int piuio_leds_init(struct piuio *piu)
+{
+ int i;
+ int ret;
+
+ piu->led = kcalloc(piu->type->outputs, sizeof(*piu->led), GFP_KERNEL);
+ if (!piu->led)
+ return -ENOMEM;
+
+ for (i = 0; i < piu->type->outputs; i++) {
+ /* Initialize led device and point back to piuio struct */
+ piu->led[i].dev.name = piu->type->led_names[i];
+ piu->led[i].dev.brightness_set = piuio_led_set;
+ piu->led[i].piu = piu;
+
+ ret = piu_led_register(&piu->led[i]);
+ if (ret)
+ goto out_unregister;
+ }
+
+ return 0;
+
+out_unregister:
+ for (--i; i >= 0; i--)
+ led_classdev_unregister(&piu->led[i].dev);
+ kfree(piu->led);
+ return ret;
+}
+
+static void piuio_leds_destroy(struct piuio *piu)
+{
+ int i;
+
+ for (i = 0; i < piu->type->outputs; i++)
+ led_classdev_unregister(&piu->led[i].dev);
+ kfree(piu->led);
+}
+#else
+static int piuio_leds_init(struct piuio *piu) { return 0; }
+static void piuio_leds_destroy(struct piuio *piu) {}
+#endif
+
+static int piuio_init(struct piuio *piu, struct input_dev *idev,
+ struct usb_device *udev)
+{
+ /* Note: if this function returns an error, piuio_destroy will still be
+ * called, so we don't need to clean up here
+ */
+
+ /* Allocate USB request blocks */
+ piu->in = usb_alloc_urb(0, GFP_KERNEL);
+ piu->out = usb_alloc_urb(0, GFP_KERNEL);
+ if (!piu->in || !piu->out)
+ return -ENOMEM;
+
+ /* Create dynamically allocated arrays */
+ piu->old_inputs = kcalloc(piu->type->mplex, sizeof(*piu->old_inputs),
+ GFP_KERNEL);
+ if (!piu->old_inputs)
+ return -ENOMEM;
+
+ init_waitqueue_head(&piu->shutdown_wait);
+
+ piu->idev = idev;
+ piu->udev = udev;
+ usb_make_path(udev, piu->phys, sizeof(piu->phys));
+ strlcat(piu->phys, "/input0", sizeof(piu->phys));
+
+ /* Prepare URB for multiplexer and outputs */
+ piu->cr_out.bRequestType = USB_DIR_OUT | USB_TYPE_VENDOR |
+ USB_RECIP_DEVICE;
+ piu->cr_out.bRequest = PIUIO_MSG_REQ;
+ piu->cr_out.wValue = cpu_to_le16(PIUIO_MSG_VAL);
+ piu->cr_out.wIndex = cpu_to_le16(PIUIO_MSG_IDX);
+ piu->cr_out.wLength = cpu_to_le16(PIUIO_MSG_SZ);
+ usb_fill_control_urb(piu->out, udev, usb_sndctrlpipe(udev, 0),
+ (void *) &piu->cr_out, piu->outputs, PIUIO_MSG_SZ,
+ piuio_out_completed, piu);
+
+ /* Prepare URB for inputs */
+ piu->cr_in.bRequestType = USB_DIR_IN | USB_TYPE_VENDOR |
+ USB_RECIP_DEVICE;
+ piu->cr_in.bRequest = PIUIO_MSG_REQ;
+ piu->cr_in.wValue = cpu_to_le16(PIUIO_MSG_VAL);
+ piu->cr_in.wIndex = cpu_to_le16(PIUIO_MSG_IDX);
+ piu->cr_in.wLength = cpu_to_le16(PIUIO_MSG_SZ);
+ usb_fill_control_urb(piu->in, udev, usb_rcvctrlpipe(udev, 0),
+ (void *) &piu->cr_in, piu->inputs, PIUIO_MSG_SZ,
+ piuio_in_completed, piu);
+
+ return 0;
+}
+
+static void piuio_destroy(struct piuio *piu)
+{
+ /* These handle NULL gracefully, so we can call this to clean up if init
+ * fails
+ */
+ kfree(piu->old_inputs);
+ usb_free_urb(piu->out);
+ usb_free_urb(piu->in);
+}
+
+
+/*
+ * USB connect and disconnect events
+ */
+static int piuio_probe(struct usb_interface *intf,
+ const struct usb_device_id *id)
+{
+ struct piuio *piu;
+ struct usb_device *udev = interface_to_usbdev(intf);
+ struct input_dev *idev;
+ int ret = -ENOMEM;
+
+ /* Allocate PIUIO state and determine device type */
+ piu = kzalloc(sizeof(struct piuio), GFP_KERNEL);
+ if (!piu)
+ return ret;
+
+ if (id->idVendor == USB_VENDOR_ID_BTNBOARD &&
+ id->idProduct == USB_PRODUCT_ID_BTNBOARD) {
+ /* Button board card */
+ piu->type = &piuio_dev_bb;
+ } else {
+ /* Full card */
+ piu->type = &piuio_dev_full;
+ }
+
+ /* Allocate input device for generating buttonpresses */
+ idev = input_allocate_device();
+ if (!idev) {
+ kfree(piu->old_inputs);
+ kfree(piu);
+ return ret;
+ }
+
+ /* Initialize PIUIO state and input device */
+ ret = piuio_init(piu, idev, udev);
+ if (ret)
+ goto err;
+
+ piuio_input_init(piu, &intf->dev);
+
+ /* Initialize and register led devices */
+ ret = piuio_leds_init(piu);
+ if (ret)
+ goto err;
+
+ /* Register input device */
+ ret = input_register_device(piu->idev);
+ if (ret) {
+ dev_err(&intf->dev, "piuio probe: failed to register input dev\n");
+ piuio_leds_destroy(piu);
+ goto err;
+ }
+
+ /* Final USB setup */
+ usb_set_intfdata(intf, piu);
+ return 0;
+
+err:
+ piuio_destroy(piu);
+ input_free_device(idev);
+ kfree(piu);
+ return ret;
+}
+
+static void piuio_disconnect(struct usb_interface *intf)
+{
+ struct piuio *piu = usb_get_intfdata(intf);
+
+ usb_set_intfdata(intf, NULL);
+ if (!piu) {
+ dev_err(&intf->dev, "piuio disconnect: uninitialized device?\n");
+ return;
+ }
+
+ usb_kill_urb(piu->in);
+ usb_kill_urb(piu->out);
+ piuio_leds_destroy(piu);
+ input_unregister_device(piu->idev);
+ piuio_destroy(piu);
+ kfree(piu);
+}
+
+
+/*
+ * USB driver and module definitions
+ */
+static struct usb_device_id piuio_id_table[] = {
+ /* Python WDM2 Encoder used for PIUIO boards */
+ { USB_DEVICE(USB_VENDOR_ID_ANCHOR, USB_PRODUCT_ID_PYTHON2) },
+ /* Special USB ID for button board devices */
+ { USB_DEVICE(USB_VENDOR_ID_BTNBOARD, USB_PRODUCT_ID_BTNBOARD) },
+ {},
+};
+
+MODULE_DEVICE_TABLE(usb, piuio_id_table);
+
+static struct usb_driver piuio_driver = {
+ .name = "piuio",
+ .probe = piuio_probe,
+ .disconnect = piuio_disconnect,
+ .id_table = piuio_id_table,
+};
+
+MODULE_AUTHOR("Devin J. Pohly");
+MODULE_DESCRIPTION("PIUIO input/output driver");
+MODULE_VERSION("1.0");
+MODULE_LICENSE("GPL v2");
+
+module_usb_driver(piuio_driver);
--
2.16.0
^ permalink raw reply related
* Re: [PATCH] Input: add support for PIUIO input/output board
From: Dmitry Torokhov @ 2018-01-23 19:39 UTC (permalink / raw)
To: Devin J. Pohly; +Cc: linux-input
In-Reply-To: <20180123192029.9468-2-djpohly@gmail.com>
On Tue, Jan 23, 2018 at 01:20:29PM -0600, Devin J. Pohly wrote:
> The PIUIO is a digital input/output board most often found in arcade
> dance cabinets. It uses a polling-based USB protocol to get/set the
> state of inputs and outputs, with the potential for up to 48 of each
> (though actual boards have fewer physical connections).
>
> This commit provides a driver which exposes the inputs as joystick
> buttons and the outputs as LEDs.
>
> The driver also supports a smaller form of the board with 8 inputs and
> outputs which uses the same protocol.
>
> Signed-off-by: Devin J. Pohly <djpohly@gmail.com>
> ---
> MAINTAINERS | 6 +
> drivers/input/joystick/Kconfig | 11 +
> drivers/input/joystick/Makefile | 1 +
> drivers/input/joystick/piuio.c | 672 ++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 690 insertions(+)
> create mode 100644 drivers/input/joystick/piuio.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2f4e462aa4a2..a39675bff62a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10691,6 +10691,12 @@ F: arch/mips/include/asm/mach-pistachio/
> F: arch/mips/boot/dts/img/pistachio*
> F: arch/mips/configs/pistachio*_defconfig
>
> +PIUIO DRIVER
> +M: Devin J. Pohly <djpohly@gmail.com>
> +W: https://github.com/djpohly/piuio
> +S: Maintained
> +F: drivers/input/joystick/piuio.c
> +
> PKTCDVD DRIVER
> S: Orphan
> M: linux-block@vger.kernel.org
> diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
> index f3c2f6ea8b44..5d156e760db8 100644
> --- a/drivers/input/joystick/Kconfig
> +++ b/drivers/input/joystick/Kconfig
> @@ -351,4 +351,15 @@ config JOYSTICK_PSXPAD_SPI_FF
>
> To drive rumble motor a dedicated power supply is required.
>
> +config JOYSTICK_PIUIO
> + tristate "PIUIO input/output interface"
> + depends on USB_ARCH_HAS_HCD
> + select USB
> + help
> + Say Y here if you want to use the PIUIO interface board for
> + digital inputs/outputs.
> +
> + To compile this driver as a module, choose M here: the
> + module will be called piuio.
> +
> endif
> diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
> index 67651efda2e1..33c4e54fb516 100644
> --- a/drivers/input/joystick/Makefile
> +++ b/drivers/input/joystick/Makefile
> @@ -22,6 +22,7 @@ obj-$(CONFIG_JOYSTICK_INTERACT) += interact.o
> obj-$(CONFIG_JOYSTICK_JOYDUMP) += joydump.o
> obj-$(CONFIG_JOYSTICK_MAGELLAN) += magellan.o
> obj-$(CONFIG_JOYSTICK_MAPLE) += maplecontrol.o
> +obj-$(CONFIG_JOYSTICK_PIUIO) += piuio.o
> obj-$(CONFIG_JOYSTICK_PSXPAD_SPI) += psxpad-spi.o
> obj-$(CONFIG_JOYSTICK_SIDEWINDER) += sidewinder.o
> obj-$(CONFIG_JOYSTICK_SPACEBALL) += spaceball.o
> diff --git a/drivers/input/joystick/piuio.c b/drivers/input/joystick/piuio.c
> new file mode 100644
> index 000000000000..e1d9b34692e2
> --- /dev/null
> +++ b/drivers/input/joystick/piuio.c
> @@ -0,0 +1,672 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Andamiro PIU IO input module, for use with the PIUIO input/output boards
> +// commonly found in arcade dance cabinets.
> +//
> +// Copyright (C) 2012-2018 Devin J. Pohly (djpohly@gmail.com)
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/stat.h>
> +#include <linux/sysfs.h>
> +#include <linux/errno.h>
> +#include <linux/bitops.h>
> +#include <linux/leds.h>
> +#include <linux/wait.h>
> +#include <linux/jiffies.h>
> +#include <linux/input.h>
> +#include <linux/usb.h>
> +#include <linux/usb/input.h>
> +
> +
> +/*
> + * Device and protocol definitions
> + */
> +#define USB_VENDOR_ID_ANCHOR 0x0547
> +#define USB_PRODUCT_ID_PYTHON2 0x1002
> +#define USB_VENDOR_ID_BTNBOARD 0x0d2f
> +#define USB_PRODUCT_ID_BTNBOARD 0x1010
> +
> +/* USB message used to communicate with the device */
> +#define PIUIO_MSG_REQ 0xae
> +#define PIUIO_MSG_VAL 0
> +#define PIUIO_MSG_IDX 0
> +
> +#define PIUIO_MSG_SZ 8
> +#define PIUIO_MSG_LONGS (PIUIO_MSG_SZ / sizeof(unsigned long))
> +
> +/* Input keycode ranges */
> +#define PIUIO_BTN_REG BTN_JOYSTICK
> +#define PIUIO_NUM_REG (BTN_GAMEPAD - BTN_JOYSTICK)
> +#define PIUIO_BTN_EXTRA BTN_TRIGGER_HAPPY
> +#define PIUIO_NUM_EXTRA (KEY_MAX - BTN_TRIGGER_HAPPY)
> +#define PIUIO_NUM_BTNS (PIUIO_NUM_REG + PIUIO_NUM_EXTRA)
> +
> +
> +#ifdef CONFIG_LEDS_CLASS
> +/**
> + * struct piuio_led - auxiliary struct for led devices
> + * @piu: Pointer back to the enclosing structure
> + * @dev: Actual led device
> + */
> +struct piuio_led {
> + struct piuio *piu;
> + struct led_classdev dev;
> +};
> +
> +static const char *const led_names[] = {
> + "piuio::output0",
> + "piuio::output1",
> + "piuio::output2",
> + "piuio::output3",
> + "piuio::output4",
> + "piuio::output5",
> + "piuio::output6",
> + "piuio::output7",
> + "piuio::output8",
> + "piuio::output9",
> + "piuio::output10",
> + "piuio::output11",
> + "piuio::output12",
> + "piuio::output13",
> + "piuio::output14",
> + "piuio::output15",
> + "piuio::output16",
> + "piuio::output17",
> + "piuio::output18",
> + "piuio::output19",
> + "piuio::output20",
> + "piuio::output21",
> + "piuio::output22",
> + "piuio::output23",
> + "piuio::output24",
> + "piuio::output25",
> + "piuio::output26",
> + "piuio::output27",
> + "piuio::output28",
> + "piuio::output29",
> + "piuio::output30",
> + "piuio::output31",
> + "piuio::output32",
> + "piuio::output33",
> + "piuio::output34",
> + "piuio::output35",
> + "piuio::output36",
> + "piuio::output37",
> + "piuio::output38",
> + "piuio::output39",
> + "piuio::output40",
> + "piuio::output41",
> + "piuio::output42",
> + "piuio::output43",
> + "piuio::output44",
> + "piuio::output45",
> + "piuio::output46",
> + "piuio::output47",
> +};
> +
> +static const char *const bbled_names[] = {
> + "piuio::bboutput0",
> + "piuio::bboutput1",
> + "piuio::bboutput2",
> + "piuio::bboutput3",
> + "piuio::bboutput4",
> + "piuio::bboutput5",
> + "piuio::bboutput6",
> + "piuio::bboutput7",
> +};
> +#endif
> +
> +/**
> + * struct piuio_devtype - parameters for different types of PIUIO devices
> + * @led_names: Array of LED names, of length @outputs, to use in sysfs
> + * @inputs: Number of input pins
> + * @outputs: Number of output pins
> + * @mplex: Number of sets of inputs
> + * @mplex_bits: Number of output bits reserved for multiplexing
> + */
> +struct piuio_devtype {
> +#ifdef CONFIG_LEDS_CLASS
> + const char *const *led_names;
> +#endif
> + int inputs;
> + int outputs;
> + int mplex;
> + int mplex_bits;
> +};
> +
> +/**
> + * struct piuio - state of each attached PIUIO
> + * @type: Type of PIUIO device (currently either full or buttonboard)
> + * @idev: Input device associated with this PIUIO
> + * @phys: Physical path of the device. @idev's phys field points to this
> + * buffer
> + * @udev: USB device associated with this PIUIO
> + * @in: URB for requesting the current state of one set of inputs
> + * @out: URB for sending data to outputs and multiplexer
> + * @cr_in: Setup packet for @in URB
> + * @cr_out: Setup packet for @out URB
> + * @old_inputs: Previous state of input pins from the @in URB for each of the
> + * input sets. These are used to determine when a press or release
> + * has happened for a group of correlated inputs.
> + * @inputs: Buffer for the @in URB
> + * @outputs: Buffer for the @out URB
> + * @new_outputs:
> + * Staging for the @outputs buffer
> + * @set: Current set of inputs to read (0 .. @type->mplex - 1)
> + */
> +struct piuio {
> + struct piuio_devtype *type;
> +
> + struct input_dev *idev;
> + char phys[64];
> +
> + struct usb_device *udev;
> + struct urb *in, *out;
> + struct usb_ctrlrequest cr_in, cr_out;
> + wait_queue_head_t shutdown_wait;
> +
> + unsigned long (*old_inputs)[PIUIO_MSG_LONGS];
> + unsigned long inputs[PIUIO_MSG_LONGS];
> + unsigned char outputs[PIUIO_MSG_SZ];
> + unsigned char new_outputs[PIUIO_MSG_SZ];
> +
> +#ifdef CONFIG_LEDS_CLASS
> + struct piuio_led *led;
> +#endif
> +
> + int set;
> +};
> +
> +/* Full device parameters */
> +static struct piuio_devtype piuio_dev_full = {
> +#ifdef CONFIG_LEDS_CLASS
> + .led_names = led_names,
> +#endif
> + .inputs = (PIUIO_NUM_BTNS < 48) ? PIUIO_NUM_BTNS : 48,
> + .outputs = 48,
> + .mplex = 4,
> + .mplex_bits = 2,
> +};
> +
> +/* Button board device parameters */
> +static struct piuio_devtype piuio_dev_bb = {
> +#ifdef CONFIG_LEDS_CLASS
> + .led_names = bbled_names,
> +#endif
> + .inputs = (PIUIO_NUM_BTNS < 8) ? PIUIO_NUM_BTNS : 8,
> + .outputs = 8,
> + .mplex = 1,
> + .mplex_bits = 0,
> +};
> +
> +
> +/*
> + * Auxiliary functions for reporting input events
> + */
> +static int keycode(unsigned int pin)
> +{
> + /* Use joystick buttons first, then the extra "trigger happy" range. */
> + if (pin < PIUIO_NUM_REG)
> + return PIUIO_BTN_REG + pin;
> + pin -= PIUIO_NUM_REG;
> + return PIUIO_BTN_EXTRA + pin;
> +}
> +
> +
> +/*
> + * URB completion handlers
> + */
> +static void piuio_in_completed(struct urb *urb)
> +{
> + struct piuio *piu = urb->context;
> + unsigned long changed[PIUIO_MSG_LONGS];
> + unsigned long b;
> + int i, s;
> + int cur_set;
> + int ret = urb->status;
> +
> + if (ret) {
> + dev_warn(&piu->udev->dev,
> + "piuio callback(in): error %d\n", ret);
> + goto resubmit;
> + }
> +
> + /* Get index of the previous input set (always 0 if no multiplexer) */
> + cur_set = (piu->set + piu->type->mplex - 1) % piu->type->mplex;
> +
> + /* Note what has changed in this input set, then store the inputs for
> + * next time
> + */
> + for (i = 0; i < PIUIO_MSG_LONGS; i++) {
> + changed[i] = piu->inputs[i] ^ piu->old_inputs[cur_set][i];
> + piu->old_inputs[cur_set][i] = piu->inputs[i];
> + }
> +
> + /* If we are using a multiplexer, changes only count when none of the
> + * corresponding inputs in other sets are pressed. Since "pressed"
> + * reads as 0, we can use & to knock those bits out of the changes.
> + */
> + for (s = 0; s < piu->type->mplex; s++) {
> + if (s == cur_set)
> + continue;
> + for (i = 0; i < PIUIO_MSG_LONGS; i++)
> + changed[i] &= piu->old_inputs[s][i];
> + }
> +
> + /* For each input which has changed state, report whether it was pressed
> + * or released based on the current value.
> + */
> + for_each_set_bit(b, changed, piu->type->inputs) {
> + input_event(piu->idev, EV_MSC, MSC_SCAN, b + 1);
> + input_report_key(piu->idev, keycode(b),
> + !test_bit(b, piu->inputs));
> + }
> +
> + /* Done reporting input events */
> + input_sync(piu->idev);
> +
> +resubmit:
> + ret = usb_submit_urb(urb, GFP_ATOMIC);
> + if (ret == -EPERM)
> + dev_info(&piu->udev->dev, "piuio resubmit(in): shutdown\n");
> + else if (ret)
> + dev_err(&piu->udev->dev, "piuio resubmit(in): error %d\n", ret);
> +
> + /* Let any waiting threads know we're done here */
> + wake_up(&piu->shutdown_wait);
> +}
> +
> +static void piuio_out_completed(struct urb *urb)
> +{
> + struct piuio *piu = urb->context;
> + int ret = urb->status;
> +
> + if (ret) {
> + dev_warn(&piu->udev->dev,
> + "piuio callback(out): error %d\n", ret);
> + goto resubmit;
> + }
> +
> + /* Copy in the new outputs */
> + memcpy(piu->outputs, piu->new_outputs, PIUIO_MSG_SZ);
> +
> + /* If we have a multiplexer, switch to the next input set in rotation
> + * and set the appropriate output bits
> + */
> + piu->set = (piu->set + 1) % piu->type->mplex;
> +
> + /* Set multiplexer bits */
> + piu->outputs[0] &= ~((1 << piu->type->mplex_bits) - 1);
GENMASK().
> + piu->outputs[0] |= piu->set;
> + piu->outputs[2] &= ~((1 << piu->type->mplex_bits) - 1);
And here.
> + piu->outputs[2] |= piu->set;
> +
> +resubmit:
> + ret = usb_submit_urb(piu->out, GFP_ATOMIC);
> + if (ret == -EPERM)
> + dev_info(&piu->udev->dev, "piuio resubmit(out): shutdown\n");
> + else if (ret)
> + dev_err(&piu->udev->dev,
> + "piuio resubmit(out): error %d\n", ret);
> +
> + /* Let any waiting threads know we're done here */
> + wake_up(&piu->shutdown_wait);
> +}
> +
> +
> +/*
> + * Input device events
> + */
> +static int piuio_open(struct input_dev *idev)
> +{
> + struct piuio *piu = input_get_drvdata(idev);
> + int ret;
> +
> + /* Kick off the polling */
> + ret = usb_submit_urb(piu->out, GFP_KERNEL);
> + if (ret) {
> + dev_err(&piu->udev->dev, "piuio submit(out): error %d\n", ret);
> + return -EIO;
> + }
> +
> + ret = usb_submit_urb(piu->in, GFP_KERNEL);
> + if (ret) {
> + dev_err(&piu->udev->dev, "piuio submit(in): error %d\n", ret);
> + usb_kill_urb(piu->out);
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> +static void piuio_close(struct input_dev *idev)
> +{
> + struct piuio *piu = input_get_drvdata(idev);
> + long remaining;
> +
> + /* Stop polling, but wait for the last requests to complete */
> + usb_block_urb(piu->in);
> + usb_block_urb(piu->out);
> + remaining = wait_event_timeout(piu->shutdown_wait,
> + atomic_read(&piu->in->use_count) == 0 &&
> + atomic_read(&piu->out->use_count) == 0,
> + msecs_to_jiffies(5));
> + usb_unblock_urb(piu->in);
> + usb_unblock_urb(piu->out);
> +
> + if (!remaining) {
> + // Timed out
> + dev_warn(&piu->udev->dev, "piuio close: urb timeout\n");
> + usb_kill_urb(piu->in);
> + usb_kill_urb(piu->out);
> + }
Why not simply usb_kill_urb()? Especially for IN?
> +
> + /* XXX Reset the outputs? */
> +}
> +
> +
> +/*
> + * Structure initialization and destruction
> + */
> +static void piuio_input_init(struct piuio *piu, struct device *parent)
> +{
> + struct input_dev *idev = piu->idev;
> + int i;
> +
> + /* Fill in basic fields */
> + idev->name = "PIUIO input";
> + idev->phys = piu->phys;
> + usb_to_input_id(piu->udev, &idev->id);
> + idev->dev.parent = parent;
> +
> + /* HACK: Buttons are sufficient to trigger a /dev/input/js* device, but
> + * for systemd (and consequently udev and Xorg) to consider us a
> + * joystick, we have to have a set of XY absolute axes.
> + */
Why don't you fix it in systemd?
> + set_bit(EV_KEY, idev->evbit);
> + set_bit(EV_ABS, idev->evbit);
> +
> + /* Configure buttons */
> + for (i = 0; i < piu->type->inputs; i++)
> + set_bit(keycode(i), idev->keybit);
> + clear_bit(0, idev->keybit);
> +
> + /* Configure fake axes */
> + set_bit(ABS_X, idev->absbit);
> + set_bit(ABS_Y, idev->absbit);
> + input_set_abs_params(idev, ABS_X, 0, 0, 0, 0);
> + input_set_abs_params(idev, ABS_Y, 0, 0, 0, 0);
> +
> + /* Set device callbacks */
> + idev->open = piuio_open;
> + idev->close = piuio_close;
> +
> + /* Link input device back to PIUIO */
> + input_set_drvdata(idev, piu);
> +}
> +
> +
> +#ifdef CONFIG_LEDS_CLASS
> +/*
> + * Led device event
> + */
> +static void piuio_led_set(struct led_classdev *dev, enum led_brightness b)
> +{
> + struct piuio_led *led = container_of(dev, struct piuio_led, dev);
> + struct piuio *piu = led->piu;
> + int n;
> +
> + n = led - piu->led;
> + if (n > piu->type->outputs) {
How can this happen?
> + dev_err(&piu->udev->dev, "piuio led: bad number %d\n", n);
> + return;
> + }
> +
> + /* Meh, forget atomicity, these aren't super-important */
I'd rather we not.
> + if (b)
> + __set_bit(n, (unsigned long *) piu->new_outputs);
> + else
> + __clear_bit(n, (unsigned long *) piu->new_outputs);
What causes LED to actually update?
> +}
> +
> +int
> +piu_led_register(struct piuio_led *led)
static
> +{
> + const struct attribute_group **ag;
> + struct attribute **attr;
> + int ret;
> +
> + /* Register led device */
> + ret = led_classdev_register(&led->piu->udev->dev, &led->dev);
Let's call all variables like this "error". Also
devm_led_classdev_register().
> + if (ret)
> + return ret;
> +
> + /* Relax permissions on led attributes */
> + for (ag = led->dev.dev->class->dev_groups; *ag; ag++) {
> + for (attr = (*ag)->attrs; *attr; attr++) {
> + ret = sysfs_chmod_file(&led->dev.dev->kobj, *attr,
> + 0666);
No. If you want this then do it from userspace.
> + if (ret) {
> + led_classdev_unregister(&led->dev);
> + return ret;
> + }
> + }
> + }
> + return 0;
> +}
> +
> +static int piuio_leds_init(struct piuio *piu)
> +{
> + int i;
> + int ret;
> +
> + piu->led = kcalloc(piu->type->outputs, sizeof(*piu->led), GFP_KERNEL);
devm_kcalloc().
> + if (!piu->led)
> + return -ENOMEM;
> +
> + for (i = 0; i < piu->type->outputs; i++) {
> + /* Initialize led device and point back to piuio struct */
> + piu->led[i].dev.name = piu->type->led_names[i];
> + piu->led[i].dev.brightness_set = piuio_led_set;
> + piu->led[i].piu = piu;
> +
> + ret = piu_led_register(&piu->led[i]);
> + if (ret)
> + goto out_unregister;
> + }
> +
> + return 0;
> +
> +out_unregister:
> + for (--i; i >= 0; i--)
> + led_classdev_unregister(&piu->led[i].dev);
> + kfree(piu->led);
> + return ret;
> +}
> +
> +static void piuio_leds_destroy(struct piuio *piu)
> +{
> + int i;
> +
> + for (i = 0; i < piu->type->outputs; i++)
> + led_classdev_unregister(&piu->led[i].dev);
> + kfree(piu->led);
Likely not needed with devm.
> +}
> +#else
> +static int piuio_leds_init(struct piuio *piu) { return 0; }
> +static void piuio_leds_destroy(struct piuio *piu) {}
> +#endif
> +
> +static int piuio_init(struct piuio *piu, struct input_dev *idev,
> + struct usb_device *udev)
> +{
> + /* Note: if this function returns an error, piuio_destroy will still be
> + * called, so we don't need to clean up here
> + */
> +
> + /* Allocate USB request blocks */
> + piu->in = usb_alloc_urb(0, GFP_KERNEL);
> + piu->out = usb_alloc_urb(0, GFP_KERNEL);
> + if (!piu->in || !piu->out)
> + return -ENOMEM;
> +
> + /* Create dynamically allocated arrays */
> + piu->old_inputs = kcalloc(piu->type->mplex, sizeof(*piu->old_inputs),
> + GFP_KERNEL);
devm_kcalloc()
> + if (!piu->old_inputs)
> + return -ENOMEM;
> +
> + init_waitqueue_head(&piu->shutdown_wait);
> +
> + piu->idev = idev;
> + piu->udev = udev;
> + usb_make_path(udev, piu->phys, sizeof(piu->phys));
> + strlcat(piu->phys, "/input0", sizeof(piu->phys));
> +
> + /* Prepare URB for multiplexer and outputs */
> + piu->cr_out.bRequestType = USB_DIR_OUT | USB_TYPE_VENDOR |
> + USB_RECIP_DEVICE;
> + piu->cr_out.bRequest = PIUIO_MSG_REQ;
> + piu->cr_out.wValue = cpu_to_le16(PIUIO_MSG_VAL);
> + piu->cr_out.wIndex = cpu_to_le16(PIUIO_MSG_IDX);
> + piu->cr_out.wLength = cpu_to_le16(PIUIO_MSG_SZ);
> + usb_fill_control_urb(piu->out, udev, usb_sndctrlpipe(udev, 0),
> + (void *) &piu->cr_out, piu->outputs, PIUIO_MSG_SZ,
> + piuio_out_completed, piu);
> +
> + /* Prepare URB for inputs */
> + piu->cr_in.bRequestType = USB_DIR_IN | USB_TYPE_VENDOR |
> + USB_RECIP_DEVICE;
> + piu->cr_in.bRequest = PIUIO_MSG_REQ;
> + piu->cr_in.wValue = cpu_to_le16(PIUIO_MSG_VAL);
> + piu->cr_in.wIndex = cpu_to_le16(PIUIO_MSG_IDX);
> + piu->cr_in.wLength = cpu_to_le16(PIUIO_MSG_SZ);
> + usb_fill_control_urb(piu->in, udev, usb_rcvctrlpipe(udev, 0),
> + (void *) &piu->cr_in, piu->inputs, PIUIO_MSG_SZ,
> + piuio_in_completed, piu);
> +
> + return 0;
> +}
> +
> +static void piuio_destroy(struct piuio *piu)
> +{
> + /* These handle NULL gracefully, so we can call this to clean up if init
> + * fails
> + */
> + kfree(piu->old_inputs);
> + usb_free_urb(piu->out);
> + usb_free_urb(piu->in);
> +}
> +
> +
> +/*
> + * USB connect and disconnect events
> + */
> +static int piuio_probe(struct usb_interface *intf,
> + const struct usb_device_id *id)
> +{
> + struct piuio *piu;
> + struct usb_device *udev = interface_to_usbdev(intf);
> + struct input_dev *idev;
> + int ret = -ENOMEM;
I prefer you do not pre-seed error code.
> +
> + /* Allocate PIUIO state and determine device type */
> + piu = kzalloc(sizeof(struct piuio), GFP_KERNEL);
devm_kzalloc()
sizeof(*piu)
> + if (!piu)
> + return ret;
> +
> + if (id->idVendor == USB_VENDOR_ID_BTNBOARD &&
> + id->idProduct == USB_PRODUCT_ID_BTNBOARD) {
> + /* Button board card */
> + piu->type = &piuio_dev_bb;
> + } else {
> + /* Full card */
> + piu->type = &piuio_dev_full;
> + }
I think you want to make sure that you have right number of
interfaces/endpoints so maliciously crafted devices is not able to crash
the kernel.
> +
> + /* Allocate input device for generating buttonpresses */
> + idev = input_allocate_device();
devm_input_allocate_device().
> + if (!idev) {
> + kfree(piu->old_inputs);
> + kfree(piu);
> + return ret;
> + }
> +
> + /* Initialize PIUIO state and input device */
> + ret = piuio_init(piu, idev, udev);
> + if (ret)
> + goto err;
> +
> + piuio_input_init(piu, &intf->dev);
> +
> + /* Initialize and register led devices */
> + ret = piuio_leds_init(piu);
> + if (ret)
> + goto err;
> +
> + /* Register input device */
> + ret = input_register_device(piu->idev);
> + if (ret) {
> + dev_err(&intf->dev, "piuio probe: failed to register input dev\n");
> + piuio_leds_destroy(piu);
> + goto err;
> + }
> +
> + /* Final USB setup */
> + usb_set_intfdata(intf, piu);
> + return 0;
> +
> +err:
> + piuio_destroy(piu);
> + input_free_device(idev);
> + kfree(piu);
> + return ret;
> +}
> +
> +static void piuio_disconnect(struct usb_interface *intf)
> +{
> + struct piuio *piu = usb_get_intfdata(intf);
> +
> + usb_set_intfdata(intf, NULL);
> + if (!piu) {
How can this happen?
> + dev_err(&intf->dev, "piuio disconnect: uninitialized device?\n");
> + return;
> + }
> +
> + usb_kill_urb(piu->in);
> + usb_kill_urb(piu->out);
I do not think this is needed as you handle this is close().
> + piuio_leds_destroy(piu);
> + input_unregister_device(piu->idev);
> + piuio_destroy(piu);
> + kfree(piu);
> +}
> +
> +
> +/*
> + * USB driver and module definitions
> + */
> +static struct usb_device_id piuio_id_table[] = {
> + /* Python WDM2 Encoder used for PIUIO boards */
> + { USB_DEVICE(USB_VENDOR_ID_ANCHOR, USB_PRODUCT_ID_PYTHON2) },
> + /* Special USB ID for button board devices */
> + { USB_DEVICE(USB_VENDOR_ID_BTNBOARD, USB_PRODUCT_ID_BTNBOARD) },
> + {},
> +};
> +
> +MODULE_DEVICE_TABLE(usb, piuio_id_table);
> +
> +static struct usb_driver piuio_driver = {
> + .name = "piuio",
> + .probe = piuio_probe,
> + .disconnect = piuio_disconnect,
> + .id_table = piuio_id_table,
> +};
> +
> +MODULE_AUTHOR("Devin J. Pohly");
> +MODULE_DESCRIPTION("PIUIO input/output driver");
> +MODULE_VERSION("1.0");
> +MODULE_LICENSE("GPL v2");
> +
> +module_usb_driver(piuio_driver);
> --
> 2.16.0
>
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] platform/x86: silead_dmi: Add touchscreen platform data for the Teclast X3 Plus tablet
From: Darren Hart @ 2018-01-23 23:27 UTC (permalink / raw)
To: Alberto Ponces
Cc: linux-input, Hans de Goede, Andy Shevchenko, platform-driver-x86,
linux-kernel
In-Reply-To: <1516732419-125-1-git-send-email-ponces26@gmail.com>
On Tue, Jan 23, 2018 at 06:33:38PM +0000, Alberto Ponces wrote:
> Add touchscreen platform data for the Teclast X3 Plus tablet.
>
> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>
> Signed-off-by: Alberto Ponces <ponces26@gmail.com>
Queued, thanks.
Note for the future: Author signoff goes first, then reviewers, then committer.
--
Darren Hart
VMware Open Source Technology Center
^ permalink raw reply
* Re: [PATCH v2] platform/x86: silead_dmi: Add touchscreen platform data for the Teclast X3 Plus tablet
From: Dmitry Torokhov @ 2018-01-24 0:37 UTC (permalink / raw)
To: Darren Hart
Cc: Alberto Ponces, linux-input@vger.kernel.org, Hans de Goede,
Andy Shevchenko, Platform Driver, lkml
In-Reply-To: <20180123232754.GC16464@fury>
On Tue, Jan 23, 2018 at 3:27 PM, Darren Hart <dvhart@infradead.org> wrote:
> On Tue, Jan 23, 2018 at 06:33:38PM +0000, Alberto Ponces wrote:
>> Add touchscreen platform data for the Teclast X3 Plus tablet.
>>
>> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
>>
>> Signed-off-by: Alberto Ponces <ponces26@gmail.com>
>
> Queued, thanks.
>
> Note for the future: Author signoff goes first, then reviewers, then committer.
In this case Alberto was the committer, so his sign-off is last, as it
should be. Any reported-by, suggested-by, acked-by or reviewed-by he
collected should go above his sign-off. Once you picked up his patch
you 'll become committer, so any markings you add should go between
his sign-off and yours.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v2] platform/x86: silead_dmi: Add touchscreen platform data for the Teclast X3 Plus tablet
From: Darren Hart @ 2018-01-24 1:02 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Alberto Ponces, linux-input@vger.kernel.org, Hans de Goede,
Andy Shevchenko, Platform Driver, lkml
In-Reply-To: <CAKdAkRQWhnNAA72QKmcz6hDw_RUzSzmrRVM_PG_y0dta=VEqhA@mail.gmail.com>
On Tue, Jan 23, 2018 at 04:37:43PM -0800, Dmitry Torokhov wrote:
> On Tue, Jan 23, 2018 at 3:27 PM, Darren Hart <dvhart@infradead.org> wrote:
> > On Tue, Jan 23, 2018 at 06:33:38PM +0000, Alberto Ponces wrote:
> >> Add touchscreen platform data for the Teclast X3 Plus tablet.
> >>
> >> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> >>
> >> Signed-off-by: Alberto Ponces <ponces26@gmail.com>
> >
> > Queued, thanks.
> >
> > Note for the future: Author signoff goes first, then reviewers, then committer.
>
> In this case Alberto was the committer, so his sign-off is last, as it
> should be. Any reported-by, suggested-by, acked-by or reviewed-by he
> collected should go above his sign-off. Once you picked up his patch
> you 'll become committer, so any markings you add should go between
> his sign-off and yours.
Of course, you're correct. Thank you for the correction - and Alberto, apologies
for the noise/confusion.
Dmitry, I reviewed documentation/process/submitting-patches.rst and
5.Posting.rst and didn't find this clearly defined. Have I missed where this is
documented, or is an update in order?
--
Darren Hart
VMware Open Source Technology Center
^ permalink raw reply
* Re: [PATCH v2] platform/x86: silead_dmi: Add touchscreen platform data for the Teclast X3 Plus tablet
From: Dmitry Torokhov @ 2018-01-24 1:18 UTC (permalink / raw)
To: Darren Hart
Cc: Alberto Ponces, linux-input@vger.kernel.org, Hans de Goede,
Andy Shevchenko, Platform Driver, lkml
In-Reply-To: <20180124010259.GJ16464@fury>
On Tue, Jan 23, 2018 at 05:02:59PM -0800, Darren Hart wrote:
> On Tue, Jan 23, 2018 at 04:37:43PM -0800, Dmitry Torokhov wrote:
> > On Tue, Jan 23, 2018 at 3:27 PM, Darren Hart <dvhart@infradead.org> wrote:
> > > On Tue, Jan 23, 2018 at 06:33:38PM +0000, Alberto Ponces wrote:
> > >> Add touchscreen platform data for the Teclast X3 Plus tablet.
> > >>
> > >> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> > >>
> > >> Signed-off-by: Alberto Ponces <ponces26@gmail.com>
> > >
> > > Queued, thanks.
> > >
> > > Note for the future: Author signoff goes first, then reviewers, then committer.
> >
> > In this case Alberto was the committer, so his sign-off is last, as it
> > should be. Any reported-by, suggested-by, acked-by or reviewed-by he
> > collected should go above his sign-off. Once you picked up his patch
> > you 'll become committer, so any markings you add should go between
> > his sign-off and yours.
>
> Of course, you're correct. Thank you for the correction - and Alberto, apologies
> for the noise/confusion.
>
> Dmitry, I reviewed documentation/process/submitting-patches.rst and
> 5.Posting.rst and didn't find this clearly defined. Have I missed where this is
> documented, or is an update in order?
I do not think is was ever stated explicitly, the closest comes "14) The
canonical patch format" which states that sign off goes before the ---
divider.
I think it comes naturally if you consider patch vs pull request: if you
decided to pull from Alberto's tree (or anyone else's tree) instead of
taking the patch via email, then if they'd put non-sign-off tags after
the sign-off, they'd end up in your and then Linus' tree like that, as
you would not add your sign-off when doing git merge.
But if you believe this should be called explicitly then adding a few
more words to section 14 should work.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH] Input: edt-ft5x06 - fix error handling for factory mode on non-M06
From: Andi Shyti @ 2018-01-24 2:45 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, Simon Budig, Andi Shyti, Lothar Waßmann,
Mylene Josserand, linux-kernel
In-Reply-To: <20180123185236.fxjqpnscgx42s5lf@dtor-ws>
Hi Dmitry,
On Tue, Jan 23, 2018 at 10:52:36AM -0800, Dmitry Torokhov wrote:
> When attempting enter factory mode on firmware that does not support it,
> we'd error out, but leave the device with interrupts disabled, and thus
> touch not working. Fix it by moving the check before we disable
> interrupts/allocate memory for debug buffers.
>
> Fixes: fd335ab04b3f ("Input: edt-ft5x06 - add support for M09 firmware version")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
makes sense!
Reviewed-by: Andi Shyti <andi@etezian.org>
Andi
> ---
> drivers/input/touchscreen/edt-ft5x06.c | 14 ++++++--------
> 1 file changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c
> index c53a3d7239e72..1e18ca0d1b4e1 100644
> --- a/drivers/input/touchscreen/edt-ft5x06.c
> +++ b/drivers/input/touchscreen/edt-ft5x06.c
> @@ -511,6 +511,12 @@ static int edt_ft5x06_factory_mode(struct edt_ft5x06_ts_data *tsdata)
> int ret;
> int error;
>
> + if (tsdata->version != EDT_M06) {
> + dev_err(&client->dev,
> + "No factory mode support for non-M06 devices\n");
> + return -EINVAL;
> + }
> +
> disable_irq(client->irq);
>
> if (!tsdata->raw_buffer) {
> @@ -524,9 +530,6 @@ static int edt_ft5x06_factory_mode(struct edt_ft5x06_ts_data *tsdata)
> }
>
> /* mode register is 0x3c when in the work mode */
> - if (tsdata->version != EDT_M06)
> - goto m09_out;
> -
> error = edt_ft5x06_register_write(tsdata, WORK_REGISTER_OPMODE, 0x03);
> if (error) {
> dev_err(&client->dev,
> @@ -559,11 +562,6 @@ static int edt_ft5x06_factory_mode(struct edt_ft5x06_ts_data *tsdata)
> enable_irq(client->irq);
>
> return error;
> -
> -m09_out:
> - dev_err(&client->dev, "No factory mode support for M09/M12/GENERIC_FT\n");
> - return -EINVAL;
> -
> }
>
> static int edt_ft5x06_work_mode(struct edt_ft5x06_ts_data *tsdata)
> --
> 2.16.0.rc1.238.g530d649a79-goog
>
>
> --
> Dmitry
^ permalink raw reply
* Re: [PATCH v2] platform/x86: silead_dmi: Add touchscreen platform data for the Teclast X3 Plus tablet
From: Alberto Ponces @ 2018-01-24 9:51 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Darren Hart, linux-input@vger.kernel.org, Hans de Goede,
Andy Shevchenko, Platform Driver, lkml
In-Reply-To: <20180124011844.k4hfwujev2jkjetk@dtor-ws>
There is no problem whatsoever!
I'm beginner on this so every correction or warning is welcolme.
Thank you both for your help!
--
Alberto Ponces
On Tue, Jan 23, 2018 at 05:18:44PM -0800, Dmitry Torokhov wrote:
> On Tue, Jan 23, 2018 at 05:02:59PM -0800, Darren Hart wrote:
> > On Tue, Jan 23, 2018 at 04:37:43PM -0800, Dmitry Torokhov wrote:
> > > On Tue, Jan 23, 2018 at 3:27 PM, Darren Hart <dvhart@infradead.org> wrote:
> > > > On Tue, Jan 23, 2018 at 06:33:38PM +0000, Alberto Ponces wrote:
> > > >> Add touchscreen platform data for the Teclast X3 Plus tablet.
> > > >>
> > > >> Reviewed-by: Hans de Goede <hdegoede@redhat.com>
> > > >>
> > > >> Signed-off-by: Alberto Ponces <ponces26@gmail.com>
> > > >
> > > > Queued, thanks.
> > > >
> > > > Note for the future: Author signoff goes first, then reviewers, then committer.
> > >
> > > In this case Alberto was the committer, so his sign-off is last, as it
> > > should be. Any reported-by, suggested-by, acked-by or reviewed-by he
> > > collected should go above his sign-off. Once you picked up his patch
> > > you 'll become committer, so any markings you add should go between
> > > his sign-off and yours.
> >
> > Of course, you're correct. Thank you for the correction - and Alberto, apologies
> > for the noise/confusion.
> >
> > Dmitry, I reviewed documentation/process/submitting-patches.rst and
> > 5.Posting.rst and didn't find this clearly defined. Have I missed where this is
> > documented, or is an update in order?
>
> I do not think is was ever stated explicitly, the closest comes "14) The
> canonical patch format" which states that sign off goes before the ---
> divider.
>
> I think it comes naturally if you consider patch vs pull request: if you
> decided to pull from Alberto's tree (or anyone else's tree) instead of
> taking the patch via email, then if they'd put non-sign-off tags after
> the sign-off, they'd end up in your and then Linus' tree like that, as
> you would not add your sign-off when doing git merge.
>
> But if you believe this should be called explicitly then adding a few
> more words to section 14 should work.
>
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* Dell docking station & Dell Embedded Controller & PS/2 devices
From: Pali Rohár @ 2018-01-24 10:41 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Mario.Limonciello, linux-input, linux-kernel
Hi Dmitry!
I'm observing a problem with internal touchpad (handled by psmouse.ko)
on Dell laptops connected to Dell E docking station. When I connect
external PS/2 keyboard to docking station then internal laptop touchpad
switch from multitouch absolute mode to relative bare PS/2 mode.
And because ALPS driver in psmouse.ko is capable to process interleaved
bare 3-byte PS/2 packets with 6-byte ALPS packets (which handles
trackstick data on some ALPS models), ALPS driver does not show any
message about this "downgrade" from multitouch to bare mode. And
continue working in bare mode.
When I rmmod psmouse and modprobe it again, then touchpad switch back to
multitouch mode.
Mario told me that Dell Embedded Controller, which handle internal
keyboard, internal touchpad and external PS/2 keyboard, automatically
send RESET command to *all* those devices when external PS/2 keyboard is
connected. Therefore this is reason why touchpad downgrade to to bare
mode. And according to Mario, host system should issue vendor specific
PS/2 commands to re-initialize all PS/2 devices when this situation
happen. Mario also told me that Windows is doing this action.
Every time when I connect external PS/2 keyboard to dock I see this
message in dmesg:
Spurious ACK... Some program might be trying to access hardware directly.
I see it also every time when I dock laptop into docking station (to
which is keyboard already connected). And it happens also when I connect
external PS/2 mouse to dock.
Dmitry, how to handle this situation to re-initialize psmouse.ko when
external PS/2 device is connected to Dell E docking station? According
to Mario, this is how Dell Embedded Controller is designed and suppose
how OS should work with it.
Manually rmmoding and modprobing for every docking/undocking laptop is
not ideal solution.
Could it be possible to use that Spurious ATKBD_RET_ACK from atkbd.c be
handled on Dell systems (probably via DMI) as an event to reset and
reinitialize all PS/2 devices?
--
Pali Rohár
pali.rohar@gmail.com
^ permalink raw reply
* [PATCH 0/2] Input-gtco: Adjustments for gtco_probe()
From: SF Markus Elfring @ 2018-01-24 15:13 UTC (permalink / raw)
To: linux-input, Andrey Konovalov, Dmitry Torokhov; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 24 Jan 2018 15:26:56 +0100
Two update suggestions were taken into account
from static source code analysis.
Markus Elfring (2):
Delete an error message for a failed memory allocation
Improve a size determination
drivers/input/tablet/gtco.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
--
2.16.1
^ permalink raw reply
* [PATCH 1/2] Input: gtco: Delete an error message for a failed memory allocation in gtco_probe()
From: SF Markus Elfring @ 2018-01-24 15:14 UTC (permalink / raw)
To: linux-input, Andrey Konovalov, Dmitry Torokhov; +Cc: LKML, kernel-janitors
In-Reply-To: <5c4410ab-5174-ce8d-cab6-7979aa61f996@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 24 Jan 2018 15:03:33 +0100
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/input/tablet/gtco.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/input/tablet/gtco.c b/drivers/input/tablet/gtco.c
index 4b8b9d7aa75e..f2670872735f 100644
--- a/drivers/input/tablet/gtco.c
+++ b/drivers/input/tablet/gtco.c
@@ -902,7 +902,6 @@ static int gtco_probe(struct usb_interface *usbinterface,
report = kzalloc(le16_to_cpu(hid_desc->wDescriptorLength), GFP_KERNEL);
if (!report) {
- dev_err(&usbinterface->dev, "No more memory for report\n");
error = -ENOMEM;
goto err_free_urb;
}
--
2.16.1
^ permalink raw reply related
* [PATCH 2/2] Input: gtco: Improve a size determination in gtco_probe()
From: SF Markus Elfring @ 2018-01-24 15:15 UTC (permalink / raw)
To: linux-input, Andrey Konovalov, Dmitry Torokhov; +Cc: LKML, kernel-janitors
In-Reply-To: <5c4410ab-5174-ce8d-cab6-7979aa61f996@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 24 Jan 2018 15:08:42 +0100
Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/input/tablet/gtco.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/tablet/gtco.c b/drivers/input/tablet/gtco.c
index f2670872735f..6094241d4aa1 100644
--- a/drivers/input/tablet/gtco.c
+++ b/drivers/input/tablet/gtco.c
@@ -829,7 +829,7 @@ static int gtco_probe(struct usb_interface *usbinterface,
struct usb_device *udev = interface_to_usbdev(usbinterface);
/* Allocate memory for device structure */
- gtco = kzalloc(sizeof(struct gtco), GFP_KERNEL);
+ gtco = kzalloc(sizeof(*gtco), GFP_KERNEL);
input_dev = input_allocate_device();
if (!gtco || !input_dev) {
dev_err(&usbinterface->dev, "No more memory\n");
--
2.16.1
^ permalink raw reply related
* [PATCH 1/2] input: pagasus_notetaker: fix deadlock in autosuspend
From: Marcus Folkesson @ 2018-01-24 15:46 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Marcus Folkesson, linux-input, linux-kernel
usb_autopm_get_interface() that is called in pegasus_open() does an
autoresume if the device is suspended.
input_dev->mutex used in pegasus_resume() is in this case already
taken by the input subsystem and will cause a deadlock.
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
drivers/input/tablet/pegasus_notetaker.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
diff --git a/drivers/input/tablet/pegasus_notetaker.c b/drivers/input/tablet/pegasus_notetaker.c
index 47de5a81172f..9ab1ed5e20e7 100644
--- a/drivers/input/tablet/pegasus_notetaker.c
+++ b/drivers/input/tablet/pegasus_notetaker.c
@@ -41,6 +41,7 @@
#include <linux/usb/input.h>
#include <linux/slab.h>
#include <linux/workqueue.h>
+#include <linux/mutex.h>
/* USB HID defines */
#define USB_REQ_GET_REPORT 0x01
@@ -76,6 +77,10 @@ struct pegasus {
struct usb_device *usbdev;
struct usb_interface *intf;
struct urb *irq;
+
+ /* serialize access to open/suspend */
+ struct mutex pm_mutex;
+
char name[128];
char phys[64];
struct work_struct init;
@@ -216,6 +221,7 @@ static int pegasus_open(struct input_dev *dev)
if (error)
return error;
+ mutex_lock(&pegasus->pm_mutex);
pegasus->irq->dev = pegasus->usbdev;
if (usb_submit_urb(pegasus->irq, GFP_KERNEL)) {
error = -EIO;
@@ -226,12 +232,14 @@ static int pegasus_open(struct input_dev *dev)
if (error)
goto err_kill_urb;
+ mutex_unlock(&pegasus->pm_mutex);
return 0;
err_kill_urb:
usb_kill_urb(pegasus->irq);
cancel_work_sync(&pegasus->init);
err_autopm_put:
+ mutex_unlock(&pegasus->pm_mutex);
usb_autopm_put_interface(pegasus->intf);
return error;
}
@@ -240,8 +248,11 @@ static void pegasus_close(struct input_dev *dev)
{
struct pegasus *pegasus = input_get_drvdata(dev);
+ mutex_lock(&pegasus->pm_mutex);
usb_kill_urb(pegasus->irq);
cancel_work_sync(&pegasus->init);
+ mutex_unlock(&pegasus->pm_mutex);
+
usb_autopm_put_interface(pegasus->intf);
}
@@ -274,6 +285,8 @@ static int pegasus_probe(struct usb_interface *intf,
goto err_free_mem;
}
+ mutex_init(&pegasus->pm_mutex);
+
pegasus->usbdev = dev;
pegasus->dev = input_dev;
pegasus->intf = intf;
@@ -388,10 +401,10 @@ static int pegasus_suspend(struct usb_interface *intf, pm_message_t message)
{
struct pegasus *pegasus = usb_get_intfdata(intf);
- mutex_lock(&pegasus->dev->mutex);
+ mutex_lock(&pegasus->pm_mutex);
usb_kill_urb(pegasus->irq);
cancel_work_sync(&pegasus->init);
- mutex_unlock(&pegasus->dev->mutex);
+ mutex_unlock(&pegasus->pm_mutex);
return 0;
}
@@ -401,10 +414,10 @@ static int pegasus_resume(struct usb_interface *intf)
struct pegasus *pegasus = usb_get_intfdata(intf);
int retval = 0;
- mutex_lock(&pegasus->dev->mutex);
+ mutex_lock(&pegasus->pm_mutex);
if (pegasus->dev->users && usb_submit_urb(pegasus->irq, GFP_NOIO) < 0)
retval = -EIO;
- mutex_unlock(&pegasus->dev->mutex);
+ mutex_unlock(&pegasus->pm_mutex);
return retval;
}
@@ -414,14 +427,14 @@ static int pegasus_reset_resume(struct usb_interface *intf)
struct pegasus *pegasus = usb_get_intfdata(intf);
int retval = 0;
- mutex_lock(&pegasus->dev->mutex);
+ mutex_lock(&pegasus->pm_mutex);
if (pegasus->dev->users) {
retval = pegasus_set_mode(pegasus, PEN_MODE_XY,
NOTETAKER_LED_MOUSE);
if (!retval && usb_submit_urb(pegasus->irq, GFP_NOIO) < 0)
retval = -EIO;
}
- mutex_unlock(&pegasus->dev->mutex);
+ mutex_unlock(&pegasus->pm_mutex);
return retval;
}
--
2.15.1
^ permalink raw reply related
* [PATCH 2/2] input: pegasus_notetaker: do not rely on input_dev->users
From: Marcus Folkesson @ 2018-01-24 15:46 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Marcus Folkesson, linux-input, linux-kernel
In-Reply-To: <20180124154642.28824-1-marcus.folkesson@gmail.com>
If the device is unused and suspended, a call to open will cause the
device to autoresume through the call to usb_autopm_get_interface().
input_dev->users is already incremented by the input subsystem,
therefore this expression will always be evaluated to true:
if (pegasus->dev->users && usb_submit_urb(pegasus->irq, GFP_NOIO) < 0)
retval = -EIO;
The same URB will then be fail when resubmitted in pegasus_open().
Introduce pegasus->is_open to keep track of the state instead.
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
drivers/input/tablet/pegasus_notetaker.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/input/tablet/pegasus_notetaker.c b/drivers/input/tablet/pegasus_notetaker.c
index 9ab1ed5e20e7..74c93e09c337 100644
--- a/drivers/input/tablet/pegasus_notetaker.c
+++ b/drivers/input/tablet/pegasus_notetaker.c
@@ -80,6 +80,7 @@ struct pegasus {
/* serialize access to open/suspend */
struct mutex pm_mutex;
+ bool is_open;
char name[128];
char phys[64];
@@ -232,6 +233,7 @@ static int pegasus_open(struct input_dev *dev)
if (error)
goto err_kill_urb;
+ pegasus->is_open = 1;
mutex_unlock(&pegasus->pm_mutex);
return 0;
@@ -251,6 +253,7 @@ static void pegasus_close(struct input_dev *dev)
mutex_lock(&pegasus->pm_mutex);
usb_kill_urb(pegasus->irq);
cancel_work_sync(&pegasus->init);
+ pegasus->is_open = 0;
mutex_unlock(&pegasus->pm_mutex);
usb_autopm_put_interface(pegasus->intf);
@@ -415,7 +418,7 @@ static int pegasus_resume(struct usb_interface *intf)
int retval = 0;
mutex_lock(&pegasus->pm_mutex);
- if (pegasus->dev->users && usb_submit_urb(pegasus->irq, GFP_NOIO) < 0)
+ if (pegasus->is_open && usb_submit_urb(pegasus->irq, GFP_NOIO) < 0)
retval = -EIO;
mutex_unlock(&pegasus->pm_mutex);
@@ -428,7 +431,7 @@ static int pegasus_reset_resume(struct usb_interface *intf)
int retval = 0;
mutex_lock(&pegasus->pm_mutex);
- if (pegasus->dev->users) {
+ if (pegasus->is_open) {
retval = pegasus_set_mode(pegasus, PEN_MODE_XY,
NOTETAKER_LED_MOUSE);
if (!retval && usb_submit_urb(pegasus->irq, GFP_NOIO) < 0)
--
2.15.1
^ permalink raw reply related
* [PATCH 1/2] input: usbtouchscreen: fix deadlock in autosuspend
From: Marcus Folkesson @ 2018-01-24 15:47 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Marcus Folkesson, linux-input, linux-kernel
usb_autopm_get_interface() that is called in usbtouch_open() does an
autoresume if the device is suspended.
input_dev->mutex used in usbtouch_resume() is in this case already
taken by the input subsystem and will cause a deadlock.
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
drivers/input/touchscreen/usbtouchscreen.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c
index 2c41107240de..e964658203d8 100644
--- a/drivers/input/touchscreen/usbtouchscreen.c
+++ b/drivers/input/touchscreen/usbtouchscreen.c
@@ -54,6 +54,7 @@
#include <linux/usb.h>
#include <linux/usb/input.h>
#include <linux/hid.h>
+#include <linux/mutex.h>
#define DRIVER_VERSION "v0.6"
@@ -112,6 +113,7 @@ struct usbtouch_usb {
struct usb_interface *interface;
struct input_dev *input;
struct usbtouch_device_info *type;
+ struct mutex pm_mutex; /* serialize access to open/suspend */
char name[128];
char phys[64];
void *priv;
@@ -1455,6 +1457,7 @@ static int usbtouch_open(struct input_dev *input)
if (r < 0)
goto out;
+ mutex_lock(&usbtouch->pm_mutex);
if (!usbtouch->type->irq_always) {
if (usb_submit_urb(usbtouch->irq, GFP_KERNEL)) {
r = -EIO;
@@ -1464,6 +1467,7 @@ static int usbtouch_open(struct input_dev *input)
usbtouch->interface->needs_remote_wakeup = 1;
out_put:
+ mutex_unlock(&usbtouch->pm_mutex);
usb_autopm_put_interface(usbtouch->interface);
out:
return r;
@@ -1474,8 +1478,11 @@ static void usbtouch_close(struct input_dev *input)
struct usbtouch_usb *usbtouch = input_get_drvdata(input);
int r;
+ mutex_lock(&usbtouch->pm_mutex);
if (!usbtouch->type->irq_always)
usb_kill_urb(usbtouch->irq);
+ mutex_lock(&usbtouch->pm_mutex);
+
r = usb_autopm_get_interface(usbtouch->interface);
usbtouch->interface->needs_remote_wakeup = 0;
if (!r)
@@ -1498,10 +1505,10 @@ static int usbtouch_resume(struct usb_interface *intf)
struct input_dev *input = usbtouch->input;
int result = 0;
- mutex_lock(&input->mutex);
+ mutex_lock(&usbtouch->pm_mutex);
if (input->users || usbtouch->type->irq_always)
result = usb_submit_urb(usbtouch->irq, GFP_NOIO);
- mutex_unlock(&input->mutex);
+ mutex_unlock(&usbtouch->pm_mutex);
return result;
}
@@ -1524,10 +1531,10 @@ static int usbtouch_reset_resume(struct usb_interface *intf)
}
/* restart IO if needed */
- mutex_lock(&input->mutex);
+ mutex_lock(&usbtouch->pm_mutex);
if (input->users)
err = usb_submit_urb(usbtouch->irq, GFP_NOIO);
- mutex_unlock(&input->mutex);
+ mutex_unlock(&usbtouch->pm_mutex);
return err;
}
--
2.15.1
^ permalink raw reply related
* [PATCH 2/2] input: usbtouchscreen: do not rely on input_dev->users
From: Marcus Folkesson @ 2018-01-24 15:47 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Marcus Folkesson, linux-input, linux-kernel
In-Reply-To: <20180124154713.28962-1-marcus.folkesson@gmail.com>
If the device is unused and suspended, a call to open will cause the
device to autoresume through the call to usb_autopm_get_interface().
input_dev->users is already incremented by the input subsystem,
therefore this expression will always be evaluated to true:
if (input->users || usbtouch->type->irq_always)
result = usb_submit_urb(usbtouch->irq, GFP_NOIO);
The same URB will then be fail when resubmitted in usbtouch_open().
Introduce usbtouch->is_open to keep track of the state instead.
Signed-off-by: Marcus Folkesson <marcus.folkesson@gmail.com>
---
drivers/input/touchscreen/usbtouchscreen.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c
index e964658203d8..0356ad792e40 100644
--- a/drivers/input/touchscreen/usbtouchscreen.c
+++ b/drivers/input/touchscreen/usbtouchscreen.c
@@ -114,6 +114,7 @@ struct usbtouch_usb {
struct input_dev *input;
struct usbtouch_device_info *type;
struct mutex pm_mutex; /* serialize access to open/suspend */
+ bool is_open;
char name[128];
char phys[64];
void *priv;
@@ -1466,6 +1467,7 @@ static int usbtouch_open(struct input_dev *input)
}
usbtouch->interface->needs_remote_wakeup = 1;
+ usbtouch->is_open = 1;
out_put:
mutex_unlock(&usbtouch->pm_mutex);
usb_autopm_put_interface(usbtouch->interface);
@@ -1481,6 +1483,7 @@ static void usbtouch_close(struct input_dev *input)
mutex_lock(&usbtouch->pm_mutex);
if (!usbtouch->type->irq_always)
usb_kill_urb(usbtouch->irq);
+ usbtouch->is_open = 0;
mutex_lock(&usbtouch->pm_mutex);
r = usb_autopm_get_interface(usbtouch->interface);
@@ -1502,11 +1505,10 @@ static int usbtouch_suspend
static int usbtouch_resume(struct usb_interface *intf)
{
struct usbtouch_usb *usbtouch = usb_get_intfdata(intf);
- struct input_dev *input = usbtouch->input;
int result = 0;
mutex_lock(&usbtouch->pm_mutex);
- if (input->users || usbtouch->type->irq_always)
+ if (usbtouch->is_open || usbtouch->type->irq_always)
result = usb_submit_urb(usbtouch->irq, GFP_NOIO);
mutex_unlock(&usbtouch->pm_mutex);
@@ -1516,7 +1518,6 @@ static int usbtouch_resume(struct usb_interface *intf)
static int usbtouch_reset_resume(struct usb_interface *intf)
{
struct usbtouch_usb *usbtouch = usb_get_intfdata(intf);
- struct input_dev *input = usbtouch->input;
int err = 0;
/* reinit the device */
@@ -1532,7 +1533,7 @@ static int usbtouch_reset_resume(struct usb_interface *intf)
/* restart IO if needed */
mutex_lock(&usbtouch->pm_mutex);
- if (input->users)
+ if (usbtouch->is_open)
err = usb_submit_urb(usbtouch->irq, GFP_NOIO);
mutex_unlock(&usbtouch->pm_mutex);
--
2.15.1
^ permalink raw reply related
* [PATCH 0/3] Input-apbps2: Adjustments for two function implementations
From: SF Markus Elfring @ 2018-01-24 16:33 UTC (permalink / raw)
To: linux-input, Dmitry Torokhov; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 24 Jan 2018 17:30:12 +0100
Three update suggestions were taken into account
from static source code analysis.
Markus Elfring (3):
Delete an error message for a failed memory allocation
Improve a size determination
Fix typos in a comment
drivers/input/serio/apbps2.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
--
2.16.1
^ permalink raw reply
* [PATCH 1/3] Input: apbps2: Delete an error message for a failed memory allocation in apbps2_of_probe()
From: SF Markus Elfring @ 2018-01-24 16:34 UTC (permalink / raw)
To: linux-input, Dmitry Torokhov; +Cc: LKML, kernel-janitors
In-Reply-To: <50013a33-b7ca-fce1-9769-bfe3db850a8a@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 24 Jan 2018 16:43:04 +0100
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/input/serio/apbps2.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/input/serio/apbps2.c b/drivers/input/serio/apbps2.c
index 45d4e08ca4f8..9b75c4e1bc13 100644
--- a/drivers/input/serio/apbps2.c
+++ b/drivers/input/serio/apbps2.c
@@ -140,10 +140,8 @@ static int apbps2_of_probe(struct platform_device *ofdev)
struct resource *res;
priv = devm_kzalloc(&ofdev->dev, sizeof(*priv), GFP_KERNEL);
- if (!priv) {
- dev_err(&ofdev->dev, "memory allocation failed\n");
+ if (!priv)
return -ENOMEM;
- }
/* Find Device Address */
res = platform_get_resource(ofdev, IORESOURCE_MEM, 0);
--
2.16.1
^ permalink raw reply related
* [PATCH 2/3] Input: apbps2: Improve a size determination in apbps2_of_probe()
From: SF Markus Elfring @ 2018-01-24 16:35 UTC (permalink / raw)
To: linux-input, Dmitry Torokhov; +Cc: LKML, kernel-janitors
In-Reply-To: <50013a33-b7ca-fce1-9769-bfe3db850a8a@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 24 Jan 2018 17:15:39 +0100
Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/input/serio/apbps2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/serio/apbps2.c b/drivers/input/serio/apbps2.c
index 9b75c4e1bc13..a1c64df3b6cb 100644
--- a/drivers/input/serio/apbps2.c
+++ b/drivers/input/serio/apbps2.c
@@ -170,7 +170,7 @@ static int apbps2_of_probe(struct platform_device *ofdev)
/* Set reload register to core freq in kHz/10 */
iowrite32be(freq_hz / 10000, &priv->regs->reload);
- priv->io = kzalloc(sizeof(struct serio), GFP_KERNEL);
+ priv->io = kzalloc(sizeof(*priv->io), GFP_KERNEL);
if (!priv->io)
return -ENOMEM;
--
2.16.1
^ permalink raw reply related
* [PATCH 3/3] Input: apbps2: Fix typos in a comment
From: SF Markus Elfring @ 2018-01-24 16:36 UTC (permalink / raw)
To: linux-input, Dmitry Torokhov; +Cc: LKML, kernel-janitors, trivial
In-Reply-To: <50013a33-b7ca-fce1-9769-bfe3db850a8a@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 24 Jan 2018 17:21:18 +0100
Adjust two words in this description.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/input/serio/apbps2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/input/serio/apbps2.c b/drivers/input/serio/apbps2.c
index a1c64df3b6cb..baa93bad993c 100644
--- a/drivers/input/serio/apbps2.c
+++ b/drivers/input/serio/apbps2.c
@@ -117,7 +117,7 @@ static int apbps2_open(struct serio *io)
while ((ioread32be(&priv->regs->status) & APBPS2_STATUS_DR) && --limit)
tmp = ioread32be(&priv->regs->data);
- /* Enable reciever and it's interrupt */
+ /* Enable receiver and its interrupt */
iowrite32be(APBPS2_CTRL_RE | APBPS2_CTRL_RI, &priv->regs->ctrl);
return 0;
--
2.16.1
^ permalink raw reply related
* [PATCH 0/2] Input-arc_ps2: Adjustments for two function implementations
From: SF Markus Elfring @ 2018-01-24 17:38 UTC (permalink / raw)
To: linux-input, Dmitry Torokhov; +Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 24 Jan 2018 18:34:56 +0100
Two update suggestions were taken into account
from static source code analysis.
Markus Elfring (2):
Delete an error message for a failed memory allocation in arc_ps2_probe()
Improve a size determination in two functions
drivers/input/serio/arc_ps2.c | 10 +++-------
1 file changed, 3 insertions(+), 7 deletions(-)
--
2.16.1
^ permalink raw reply
* [PATCH 1/2] Input: arc_ps2: Delete an error message for a failed memory allocation in arc_ps2_probe()
From: SF Markus Elfring @ 2018-01-24 17:39 UTC (permalink / raw)
To: linux-input, Dmitry Torokhov; +Cc: LKML, kernel-janitors
In-Reply-To: <f3c837b2-7be1-11fe-ba2c-1792590125d2@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 24 Jan 2018 18:17:32 +0100
Omit an extra message for a memory allocation failure in this function.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/input/serio/arc_ps2.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/input/serio/arc_ps2.c b/drivers/input/serio/arc_ps2.c
index 99e57a418753..9860b1c1e67a 100644
--- a/drivers/input/serio/arc_ps2.c
+++ b/drivers/input/serio/arc_ps2.c
@@ -197,10 +197,8 @@ static int arc_ps2_probe(struct platform_device *pdev)
arc_ps2 = devm_kzalloc(&pdev->dev, sizeof(struct arc_ps2_data),
GFP_KERNEL);
- if (!arc_ps2) {
- dev_err(&pdev->dev, "out of memory\n");
+ if (!arc_ps2)
return -ENOMEM;
- }
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
arc_ps2->addr = devm_ioremap_resource(&pdev->dev, res);
--
2.16.1
^ permalink raw reply related
* [PATCH 2/2] Input: arc_ps2: Improve a size determination in two functions
From: SF Markus Elfring @ 2018-01-24 17:40 UTC (permalink / raw)
To: linux-input, Dmitry Torokhov; +Cc: LKML, kernel-janitors
In-Reply-To: <f3c837b2-7be1-11fe-ba2c-1792590125d2@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 24 Jan 2018 18:30:45 +0100
Replace the specification of data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/input/serio/arc_ps2.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/input/serio/arc_ps2.c b/drivers/input/serio/arc_ps2.c
index 9860b1c1e67a..6a3c845e12ea 100644
--- a/drivers/input/serio/arc_ps2.c
+++ b/drivers/input/serio/arc_ps2.c
@@ -156,9 +156,8 @@ static int arc_ps2_create_port(struct platform_device *pdev,
int index)
{
struct arc_ps2_port *port = &arc_ps2->port[index];
- struct serio *io;
+ struct serio *io = kzalloc(sizeof(*io), GFP_KERNEL);
- io = kzalloc(sizeof(struct serio), GFP_KERNEL);
if (!io)
return -ENOMEM;
@@ -195,8 +194,7 @@ static int arc_ps2_probe(struct platform_device *pdev)
return -EINVAL;
}
- arc_ps2 = devm_kzalloc(&pdev->dev, sizeof(struct arc_ps2_data),
- GFP_KERNEL);
+ arc_ps2 = devm_kzalloc(&pdev->dev, sizeof(*arc_ps2), GFP_KERNEL);
if (!arc_ps2)
return -ENOMEM;
--
2.16.1
^ permalink raw reply related
* [PATCH] input/rmi4: Delete an error message for a failed memory allocation in two functions
From: SF Markus Elfring @ 2018-01-24 18:28 UTC (permalink / raw)
To: linux-input, Andrew Duggan, Benjamin Tissoires,
Christophe Jaillet, Dmitry Torokhov, Lyude Paul, Nick Dyer,
Wei Yongjun
Cc: LKML, kernel-janitors
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Wed, 24 Jan 2018 19:19:14 +0100
Omit an extra message for a memory allocation failure in these functions.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/input/rmi4/rmi_driver.c | 4 +---
drivers/input/rmi4/rmi_f30.c | 4 +---
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/drivers/input/rmi4/rmi_driver.c b/drivers/input/rmi4/rmi_driver.c
index 4f2bb5947a4e..6c47e4f3ca7e 100644
--- a/drivers/input/rmi4/rmi_driver.c
+++ b/drivers/input/rmi4/rmi_driver.c
@@ -1060,10 +1060,8 @@ int rmi_probe_interrupts(struct rmi_driver_data *data)
size = BITS_TO_LONGS(data->irq_count) * sizeof(unsigned long);
data->irq_memory = devm_kzalloc(dev, size * 4, GFP_KERNEL);
- if (!data->irq_memory) {
- dev_err(dev, "Failed to allocate memory for irq masks.\n");
+ if (!data->irq_memory)
return -ENOMEM;
- }
data->irq_status = data->irq_memory + size * 0;
data->fn_irq_bits = data->irq_memory + size * 1;
diff --git a/drivers/input/rmi4/rmi_f30.c b/drivers/input/rmi4/rmi_f30.c
index 82e0f0d43d55..a111ea370c30 100644
--- a/drivers/input/rmi4/rmi_f30.c
+++ b/drivers/input/rmi4/rmi_f30.c
@@ -238,10 +238,8 @@ static int rmi_f30_map_gpios(struct rmi_function *fn,
button_count,
sizeof(f30->gpioled_key_map[0]),
GFP_KERNEL);
- if (!f30->gpioled_key_map) {
- dev_err(&fn->dev, "Failed to allocate gpioled map memory.\n");
+ if (!f30->gpioled_key_map)
return -ENOMEM;
- }
for (i = 0; i < button_count; i++) {
if (!rmi_f30_is_valid_button(i, f30->ctrl))
--
2.16.1
^ permalink raw reply related
* Re: [PATCH v2] Input: mms114 - drop platform data and use generic APIs
From: Dmitry Torokhov @ 2018-01-24 19:37 UTC (permalink / raw)
To: Simon Shields
Cc: Andi Shyti, linux-input-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring
In-Reply-To: <20180113020456.12391-1-simon-WP75azK+jQYgsBAKwltoeQ@public.gmane.org>
On Sat, Jan 13, 2018 at 01:04:56PM +1100, Simon Shields wrote:
> The MMS114 platform data has no in-tree users, so drop it,
> and make the driver depend on CONFIG_OF.
>
> Switch to using the standard touchscreen properties via
> touchscreen_parse_properties(), and move the old DT parsing code
> to use device_property_*() APIs.
>
> Finally, use touchscreen_report_pos to report x/y coordinates
> and drop the custom x/y inversion code.
>
> Signed-off-by: Simon Shields <simon-WP75azK+jQYgsBAKwltoeQ@public.gmane.org>
> ---
> .../bindings/input/touchscreen/mms114.txt | 29 ++--
> drivers/input/touchscreen/Kconfig | 1 +
> drivers/input/touchscreen/mms114.c | 152 +++++++++------------
> include/linux/platform_data/mms114.h | 24 ----
> 4 files changed, 83 insertions(+), 123 deletions(-)
> delete mode 100644 include/linux/platform_data/mms114.h
>
> diff --git a/Documentation/devicetree/bindings/input/touchscreen/mms114.txt b/Documentation/devicetree/bindings/input/touchscreen/mms114.txt
> index 89d4c56c5671..8f9f9f38eff4 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/mms114.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/mms114.txt
> @@ -4,14 +4,18 @@ Required properties:
> - compatible: must be "melfas,mms114"
> - reg: I2C address of the chip
> - interrupts: interrupt to which the chip is connected
> -- x-size: horizontal resolution of touchscreen
> -- y-size: vertical resolution of touchscreen
> +- touchscreen-size-x: See [1]
> +- touchscreen-size-y: See [1]
>
> Optional properties:
> -- contact-threshold:
> -- moving-threshold:
> -- x-invert: invert X axis
> -- y-invert: invert Y axis
> +- touchscreen-fuzz-x: See [1]
> +- touchscreen-fuzz-y: See [1]
> +- touchscreen-fuzz-pressure: See [1]
> +- touchscreen-inverted-x: See [1]
> +- touchscreen-inverted-y: See [1]
> +- touchscreen-swapped-x-y: See [1]
> +
> +[1]: Documentation/devicetree/bindings/input/touchscreen/touchscreen.txt
>
> Example:
>
> @@ -22,12 +26,13 @@ Example:
> compatible = "melfas,mms114";
> reg = <0x48>;
> interrupts = <39 0>;
> - x-size = <720>;
> - y-size = <1280>;
> - contact-threshold = <10>;
> - moving-threshold = <10>;
> - x-invert;
> - y-invert;
> + touchscreen-size-x = <720>;
> + touchscreen-size-y = <1280>;
> + touchscreen-fuzz-x = <10>;
> + touchscreen-fuzz-y = <10>;
> + touchscreen-fuzz-pressure = <10>;
> + touchscreen-inverted-x;
> + touchscreen-inverted-y;
> };
>
> /* ... */
> diff --git a/drivers/input/touchscreen/Kconfig b/drivers/input/touchscreen/Kconfig
> index 38a226f9fcbd..f7ea5522ef91 100644
> --- a/drivers/input/touchscreen/Kconfig
> +++ b/drivers/input/touchscreen/Kconfig
> @@ -524,6 +524,7 @@ config TOUCHSCREEN_MCS5000
> config TOUCHSCREEN_MMS114
> tristate "MELFAS MMS114 touchscreen"
> depends on I2C
> + depends on OF
There is no need for that, with generic device properties we can use it
with ACPI or static board files.
> help
> Say Y here if you have the MELFAS MMS114 touchscreen controller
> chip in your system.
> diff --git a/drivers/input/touchscreen/mms114.c b/drivers/input/touchscreen/mms114.c
> index e5eeb6311f7d..6276a3387cd4 100644
> --- a/drivers/input/touchscreen/mms114.c
> +++ b/drivers/input/touchscreen/mms114.c
> @@ -12,8 +12,8 @@
> #include <linux/of.h>
> #include <linux/i2c.h>
> #include <linux/input/mt.h>
> +#include <linux/input/touchscreen.h>
> #include <linux/interrupt.h>
> -#include <linux/platform_data/mms114.h>
> #include <linux/regulator/consumer.h>
> #include <linux/slab.h>
>
> @@ -55,7 +55,9 @@ struct mms114_data {
> struct input_dev *input_dev;
> struct regulator *core_reg;
> struct regulator *io_reg;
> - const struct mms114_platform_data *pdata;
> + struct touchscreen_properties props;
> + unsigned int contact_threshold;
> + unsigned int moving_threshold;
>
> /* Use cache data for mode control register(write only) */
> u8 cache_mode_control;
> @@ -143,7 +145,6 @@ static int mms114_write_reg(struct mms114_data *data, unsigned int reg,
>
> static void mms114_process_mt(struct mms114_data *data, struct mms114_touch *touch)
> {
> - const struct mms114_platform_data *pdata = data->pdata;
> struct i2c_client *client = data->client;
> struct input_dev *input_dev = data->input_dev;
> unsigned int id;
> @@ -163,16 +164,6 @@ static void mms114_process_mt(struct mms114_data *data, struct mms114_touch *tou
> id = touch->id - 1;
> x = touch->x_lo | touch->x_hi << 8;
> y = touch->y_lo | touch->y_hi << 8;
> - if (x > pdata->x_size || y > pdata->y_size) {
> - dev_dbg(&client->dev,
> - "Wrong touch coordinates (%d, %d)\n", x, y);
> - return;
> - }
> -
> - if (pdata->x_invert)
> - x = pdata->x_size - x;
> - if (pdata->y_invert)
> - y = pdata->y_size - y;
>
> dev_dbg(&client->dev,
> "id: %d, type: %d, pressed: %d, x: %d, y: %d, width: %d, strength: %d\n",
> @@ -183,9 +174,8 @@ static void mms114_process_mt(struct mms114_data *data, struct mms114_touch *tou
> input_mt_report_slot_state(input_dev, MT_TOOL_FINGER, touch->pressed);
>
> if (touch->pressed) {
> + touchscreen_report_pos(input_dev, &data->props, x, y, true);
> input_report_abs(input_dev, ABS_MT_TOUCH_MAJOR, touch->width);
> - input_report_abs(input_dev, ABS_MT_POSITION_X, x);
> - input_report_abs(input_dev, ABS_MT_POSITION_Y, y);
> input_report_abs(input_dev, ABS_MT_PRESSURE, touch->strength);
> }
> }
> @@ -263,7 +253,7 @@ static int mms114_get_version(struct mms114_data *data)
>
> static int mms114_setup_regs(struct mms114_data *data)
> {
> - const struct mms114_platform_data *pdata = data->pdata;
> + const struct touchscreen_properties *props = &data->props;
> int val;
> int error;
>
> @@ -275,32 +265,32 @@ static int mms114_setup_regs(struct mms114_data *data)
> if (error < 0)
> return error;
>
> - val = (pdata->x_size >> 8) & 0xf;
> - val |= ((pdata->y_size >> 8) & 0xf) << 4;
> + val = (props->max_x >> 8) & 0xf;
> + val |= ((props->max_y >> 8) & 0xf) << 4;
> error = mms114_write_reg(data, MMS114_XY_RESOLUTION_H, val);
> if (error < 0)
> return error;
>
> - val = pdata->x_size & 0xff;
> + val = props->max_x & 0xff;
> error = mms114_write_reg(data, MMS114_X_RESOLUTION, val);
> if (error < 0)
> return error;
>
> - val = pdata->y_size & 0xff;
> + val = props->max_x & 0xff;
> error = mms114_write_reg(data, MMS114_Y_RESOLUTION, val);
> if (error < 0)
> return error;
>
> - if (pdata->contact_threshold) {
> + if (data->contact_threshold) {
> error = mms114_write_reg(data, MMS114_CONTACT_THRESHOLD,
> - pdata->contact_threshold);
> + data->contact_threshold);
> if (error < 0)
> return error;
> }
>
> - if (pdata->moving_threshold) {
> + if (data->moving_threshold) {
> error = mms114_write_reg(data, MMS114_MOVING_THRESHOLD,
> - pdata->moving_threshold);
> + data->moving_threshold);
> if (error < 0)
> return error;
> }
> @@ -335,9 +325,6 @@ static int mms114_start(struct mms114_data *data)
> return error;
> }
>
> - if (data->pdata->cfg_pin)
> - data->pdata->cfg_pin(true);
> -
> enable_irq(client->irq);
>
> return 0;
> @@ -350,9 +337,6 @@ static void mms114_stop(struct mms114_data *data)
>
> disable_irq(client->irq);
>
> - if (data->pdata->cfg_pin)
> - data->pdata->cfg_pin(false);
> -
> error = regulator_disable(data->io_reg);
> if (error)
> dev_warn(&client->dev, "Failed to disable vdd: %d\n", error);
> @@ -376,67 +360,43 @@ static void mms114_input_close(struct input_dev *dev)
> mms114_stop(data);
> }
>
> -#ifdef CONFIG_OF
> -static struct mms114_platform_data *mms114_parse_dt(struct device *dev)
> +static int mms114_parse_dt(struct mms114_data *data)
> {
> - struct mms114_platform_data *pdata;
> - struct device_node *np = dev->of_node;
> -
> - if (!np)
> - return NULL;
> + struct device *dev = &data->client->dev;
> + struct touchscreen_properties *props = &data->props;
>
> - pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
> - if (!pdata) {
> - dev_err(dev, "failed to allocate platform data\n");
> - return NULL;
> + if (device_property_read_u32(dev, "x-size", &props->max_x)) {
> + dev_dbg(dev, "failed to get legacy x-size property\n");
> + return -EINVAL;
> }
>
> - if (of_property_read_u32(np, "x-size", &pdata->x_size)) {
> - dev_err(dev, "failed to get x-size property\n");
> - return NULL;
> + if (device_property_read_u32(dev, "y-size", &props->max_y)) {
> + dev_dbg(dev, "failed to get legacy y-size property\n");
> + return -EINVAL;
> }
>
> - if (of_property_read_u32(np, "y-size", &pdata->y_size)) {
> - dev_err(dev, "failed to get y-size property\n");
> - return NULL;
> - }
> + device_property_read_u32(dev, "contact-threshold",
> + &data->contact_threshold);
> + device_property_read_u32(dev, "moving-threshold",
> + &data->moving_threshold);
>
> - of_property_read_u32(np, "contact-threshold",
> - &pdata->contact_threshold);
> - of_property_read_u32(np, "moving-threshold",
> - &pdata->moving_threshold);
> + if (device_property_read_bool(dev, "x-invert"))
> + props->invert_x = true;
> + if (device_property_read_bool(dev, "y-invert"))
> + props->invert_y = true;
>
> - if (of_find_property(np, "x-invert", NULL))
> - pdata->x_invert = true;
> - if (of_find_property(np, "y-invert", NULL))
> - pdata->y_invert = true;
> + props->swap_x_y = false;
>
> - return pdata;
> -}
> -#else
> -static inline struct mms114_platform_data *mms114_parse_dt(struct device *dev)
> -{
> - return NULL;
> + return 0;
> }
> -#endif
>
> static int mms114_probe(struct i2c_client *client,
> const struct i2c_device_id *id)
> {
> - const struct mms114_platform_data *pdata;
> struct mms114_data *data;
> struct input_dev *input_dev;
> int error;
>
> - pdata = dev_get_platdata(&client->dev);
> - if (!pdata)
> - pdata = mms114_parse_dt(&client->dev);
> -
> - if (!pdata) {
> - dev_err(&client->dev, "Need platform data\n");
> - return -EINVAL;
> - }
> -
> if (!i2c_check_functionality(client->adapter,
> I2C_FUNC_PROTOCOL_MANGLING)) {
> dev_err(&client->dev,
> @@ -453,8 +413,39 @@ static int mms114_probe(struct i2c_client *client,
> }
>
> data->client = client;
> +
> + input_set_capability(input_dev, EV_KEY, BTN_TOUCH);
> + input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_X);
> + input_set_capability(input_dev, EV_ABS, ABS_MT_POSITION_Y);
> + input_set_capability(input_dev, EV_ABS, ABS_MT_PRESSURE);
> + input_set_capability(input_dev, EV_ABS, ABS_MT_TOUCH_MAJOR);
> +
> + input_set_abs_params(input_dev, ABS_MT_PRESSURE, 0, 255, 0, 0);
> +
> + if (mms114_parse_dt(data) < 0) {
> + /* No valid legacy binding found, try the common one */
> + touchscreen_parse_properties(input_dev, true, &data->props);
My preference would be going in the other direction: try the newer
binding first, fall back to legacy.
I have a couple more changes ot the driver, I'll post them and the
updated version of your patch as well.
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
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox