New input driver for PIUIO input/output board
Subsystems (2)
Hi all,
I've written an input driver for a custom I/O board often found in arcade dance machines. I have maintained it out-of-tree on my GitHub
(https://github.com/djpohly/piuio) for a number of years, and it has been running on a handful of "production" systems for just as long.
This is my first major submission to the kernel, so I would welcome any comments or criticisms that people may have.
Thanks!
*dp
PATCH 1/1 Input: add support for PIUIO input/output board
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.
--- 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
MAINTAINERS +6 -0
@@ -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 <[email protected]> +W: https://github.com/djpohly/piuio +S: Maintained +F: drivers/input/joystick/piuio.c + PKTCDVD DRIVER S: Orphan M: [email protected]
drivers/input/joystick/Kconfig +11 -0
@@ -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
drivers/input/joystick/Makefile +1 -0
@@ -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
drivers/input/joystick/piuio.c new file +672 -0
672 lines not on this page · raw message
Signature · 1 line
2.16.0
Dmitry Torokhov
maintainerRe: [PATCH] Input: add support for PIUIO input/output board
Quoted 366 lines · Devin J. Pohly
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 <[email protected]>--- 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.cMAINTAINERS +6
-0@@ -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 <[email protected]> +W: https://github.com/djpohly/piuio +S: Maintained +F: drivers/input/joystick/piuio.c + PKTCDVD DRIVER S: Orphan M: [email protected]drivers/input/joystick/Kconfig +11
-0@@ -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. + endifdrivers/input/joystick/Makefile +1
-0@@ -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.odrivers/input/joystick/piuio.c new file +291
-0@@ -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 ([email protected]) + +#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; + } +
drivers/input/joystick/piuio.c line 301
Quoted 10 lines
drivers/input/joystick/piuio.c new file +10
-0+/* 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().
drivers/input/joystick/piuio.c line 303
Quoted 2 lines
drivers/input/joystick/piuio.c new file +2
-0+ piu->outputs[0 ] |= piu->set; + piu->outputs[2 ] &= ~((1 << piu->type->mplex_bits) -1 );
And here.
Quoted 51 lines
drivers/input/joystick/piuio.c new file +51
-0+ 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,
drivers/input/joystick/piuio.c line 364
Quoted 10 lines
drivers/input/joystick/piuio.c new file +10
-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?
Quoted 13 lines
drivers/input/joystick/piuio.c new file +13
-0+ +/* 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; +
drivers/input/joystick/piuio.c line 387
Quoted 10 lines
drivers/input/joystick/piuio.c new file +10
-0+/* 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?
Quoted 25 lines
drivers/input/joystick/piuio.c new file +25
-0+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 +/*
drivers/input/joystick/piuio.c line 422
Quoted 10 lines
drivers/input/joystick/piuio.c new file +10
-0+* 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?
drivers/input/joystick/piuio.c line 427
Quoted 7 lines
drivers/input/joystick/piuio.c new file +5
-0+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.
drivers/input/joystick/piuio.c line 431
Quoted 4 lines
drivers/input/joystick/piuio.c new file +4
-0+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?
drivers/input/joystick/piuio.c line 435
Quoted 4 lines
drivers/input/joystick/piuio.c new file +4
-0+} + +int +piu_led_register (struct piuio_led *led)
static
drivers/input/joystick/piuio.c line 442
Quoted 7 lines
drivers/input/joystick/piuio.c new file +7
-0+{ +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". Alsodevm_led_classdev_register().
drivers/input/joystick/piuio.c line 450
Quoted 8 lines
drivers/input/joystick/piuio.c new file +8
-0+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.
Quoted 5 lines
drivers/input/joystick/piuio.c new file +5
-0+if (ret) { +led_classdev_unregister (&led->dev); +return ret; + } + }
drivers/input/joystick/piuio.c line 465
Quoted 10 lines
drivers/input/joystick/piuio.c new file +10
-0+ } +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().
Quoted 20 lines
drivers/input/joystick/piuio.c new file +20
-0+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);
drivers/input/joystick/piuio.c line 495
Quoted 10 lines
drivers/input/joystick/piuio.c new file +10
-0+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.
Quoted 12 lines
drivers/input/joystick/piuio.c new file +12
-0+} +#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 +*/
drivers/input/joystick/piuio.c line 517
Quoted 10 lines
drivers/input/joystick/piuio.c new file +10
-0+ +/* 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()
Quoted 46 lines
drivers/input/joystick/piuio.c new file +46
-0+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); +} + +
drivers/input/joystick/piuio.c line 573
Quoted 10 lines
drivers/input/joystick/piuio.c new file +10
-0+/* +* 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.
drivers/input/joystick/piuio.c line 576
Quoted 3 lines
drivers/input/joystick/piuio.c new file +3
-0+ +/* Allocate PIUIO state and determine device type */ + piu =kzalloc (sizeof (struct piuio ),GFP_KERNEL );
devm_kzalloc()sizeof(*piu)
drivers/input/joystick/piuio.c line 587
Quoted 11 lines
drivers/input/joystick/piuio.c new file +11
-0+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.
drivers/input/joystick/piuio.c line 590
Quoted 3 lines
drivers/input/joystick/piuio.c new file +3
-0+ +/* Allocate input device for generating buttonpresses */ + idev =input_allocate_device ();
devm_input_allocate_device().
Quoted 33 lines
drivers/input/joystick/piuio.c new file +33
-0+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);
drivers/input/joystick/piuio.c line 633
Quoted 10 lines
drivers/input/joystick/piuio.c new file +10
-0+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?
drivers/input/joystick/piuio.c line 639
Quoted 6 lines
drivers/input/joystick/piuio.c new file +6
-0+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().
Quoted 36 lines
drivers/input/joystick/piuio.c new file +33
-0+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);Signature · 1 line
2.16.0
Thanks.
Signature · 1 line
Dmitry