Hello Dmitry, On Tue, Jan 16, 2018 at 03:16:25PM -0800, Dmitry Torokhov wrote: > Hi Marcus, > > On Sat, Jan 13, 2018 at 09:15:32PM +0100, Marcus Folkesson wrote: > > This driver let you plug in your RC controller to the adapter and > > use it as input device in various RC simulators. > > > > Signed-off-by: Marcus Folkesson <marcus.folkesson@xxxxxxxxx> > > --- > > v3: > > - Use RUDDER and MISC instead of TILT_X and TILT_Y > > - Drop kref and anchor > > - Rework URB handling > > - Add PM support > > How did you test the PM support? By default the autopm is disabled on > USB devices; you need to enable it by writing to sysfs (I believe you > need to 'echo "auto" > /sys/bus/usb/<device>/power/control) and see if > it gets autosuspended when not in use and resumed after you start > interacting with it. The test I've done is simply reading from the input device and then call `pm-suspend`. It works, suspend is called and reset_resume() will submit the URB again. Without the PM code, the application did not read any events upon resume. However, I found another tricky part. If I enable autosuspend (as you suggest) it will suspend when noone is using the device. Good. But when someone is opening the device, input_dev->users is counted up to 1 before resume() is called. Is this intended? This code (from resume()) will therefor allways submit the URB: if (input_dev->users && usb_submit_urb(pxrc->urb, GFP_NOIO) < 0) Then open() is called and fails because the urb is allready submitted. input_dev->users is only incremented in input.c:input_open_device() what I can tell? I will move the submitting code to reset_resume() instead. > > > v2: > > - Change module license to GPLv2 to match SPDX tag > > > > Documentation/input/devices/pxrc.rst | 57 +++++++ > > drivers/input/joystick/Kconfig | 9 + > > drivers/input/joystick/Makefile | 1 + > > drivers/input/joystick/pxrc.c | 320 +++++++++++++++++++++++++++++++++++ > > 4 files changed, 387 insertions(+) > > create mode 100644 Documentation/input/devices/pxrc.rst > > create mode 100644 drivers/input/joystick/pxrc.c > > > > diff --git a/Documentation/input/devices/pxrc.rst b/Documentation/input/devices/pxrc.rst > > new file mode 100644 > > index 000000000000..ca11f646bae8 > > --- /dev/null > > +++ b/Documentation/input/devices/pxrc.rst > > @@ -0,0 +1,57 @@ > > +======================================================= > > +pxrc - PhoenixRC Flight Controller Adapter > > +======================================================= > > + > > +:Author: Marcus Folkesson <marcus.folkesson@xxxxxxxxx> > > + > > +This driver let you use your own RC controller plugged into the > > +adapter that comes with PhoenixRC [1]_ or other compatible adapters. > > + > > +The adapter supports 7 analog channels and 1 digital input switch. > > + > > +Notes > > +===== > > + > > +Many RC controllers is able to configure which stick goes to which channel. > > +This is also configurable in most simulators, so a matching is not necessary. > > + > > +The driver is generating the following input event for analog channels: > > + > > ++---------+----------------+ > > +| Channel | Event | > > ++=========+================+ > > +| 1 | ABS_X | > > ++---------+----------------+ > > +| 2 | ABS_Y | > > ++---------+----------------+ > > +| 3 | ABS_RX | > > ++---------+----------------+ > > +| 4 | ABS_RY | > > ++---------+----------------+ > > +| 5 | ABS_RUDDER | > > ++---------+----------------+ > > +| 6 | ABS_THROTTLE | > > ++---------+----------------+ > > +| 7 | ABS_MISC | > > ++---------+----------------+ > > + > > +The digital input switch is generated as an `BTN_A` event. > > + > > +Manual Testing > > +============== > > + > > +To test this driver's functionality you may use `input-event` which is part of > > +the `input layer utilities` suite [2]_. > > + > > +For example:: > > + > > + > modprobe pxrc > > + > input-events <devnr> > > + > > +To print all input events from input `devnr`. > > + > > +References > > +========== > > + > > +.. [1] http://www.phoenix-sim.com/ > > +.. [2] https://www.kraxel.org/cgit/input/ > > diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig > > index f3c2f6ea8b44..18ab6dafff41 100644 > > --- a/drivers/input/joystick/Kconfig > > +++ b/drivers/input/joystick/Kconfig > > @@ -351,4 +351,13 @@ config JOYSTICK_PSXPAD_SPI_FF > > > > To drive rumble motor a dedicated power supply is required. > > > > +config JOYSTICK_PXRC > > + tristate "PhoenixRC Flight Controller Adapter" > > + depends on USB_ARCH_HAS_HCD > > + select USB > > + help > > + Say Y here if you want to use the PhoenixRC Flight Controller Adapter. > > + > > + To compile this driver as a module, choose M here: the > > + module will be called pxrc. > > endif > > diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile > > index 67651efda2e1..dd0492ebbed7 100644 > > --- a/drivers/input/joystick/Makefile > > +++ b/drivers/input/joystick/Makefile > > @@ -23,6 +23,7 @@ obj-$(CONFIG_JOYSTICK_JOYDUMP) += joydump.o > > obj-$(CONFIG_JOYSTICK_MAGELLAN) += magellan.o > > obj-$(CONFIG_JOYSTICK_MAPLE) += maplecontrol.o > > obj-$(CONFIG_JOYSTICK_PSXPAD_SPI) += psxpad-spi.o > > +obj-$(CONFIG_JOYSTICK_PXRC) += pxrc.o > > obj-$(CONFIG_JOYSTICK_SIDEWINDER) += sidewinder.o > > obj-$(CONFIG_JOYSTICK_SPACEBALL) += spaceball.o > > obj-$(CONFIG_JOYSTICK_SPACEORB) += spaceorb.o > > diff --git a/drivers/input/joystick/pxrc.c b/drivers/input/joystick/pxrc.c > > new file mode 100644 > > index 000000000000..98d9b8184c46 > > --- /dev/null > > +++ b/drivers/input/joystick/pxrc.c > > @@ -0,0 +1,320 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Driver for Phoenix RC Flight Controller Adapter > > + * > > + * Copyright (C) 2018 Marcus Folkesson <marcus.folkesson@xxxxxxxxx> > > + * > > + */ > > + > > +#include <linux/kernel.h> > > +#include <linux/errno.h> > > +#include <linux/slab.h> > > +#include <linux/module.h> > > +#include <linux/uaccess.h> > > +#include <linux/usb.h> > > +#include <linux/usb/input.h> > > +#include <linux/mutex.h> > > +#include <linux/input.h> > > + > > +#define PXRC_VENDOR_ID (0x1781) > > +#define PXRC_PRODUCT_ID (0x0898) > > + > > +static const struct usb_device_id pxrc_table[] = { > > + { USB_DEVICE(PXRC_VENDOR_ID, PXRC_PRODUCT_ID) }, > > + { } > > +}; > > +MODULE_DEVICE_TABLE(usb, pxrc_table); > > + > > +struct pxrc { > > + struct input_dev *input; > > + struct usb_device *udev; > > + struct usb_interface *intf; > > + struct urb *urb; > > + __u8 epaddr; > > + char phys[64]; > > + unsigned char *data; > > + size_t bsize; > > +}; > > + > > +static void pxrc_usb_irq(struct urb *urb) > > +{ > > + struct pxrc *pxrc = urb->context; > > + int error; > > + > > + switch (urb->status) { > > + case 0: > > + /* success */ > > + break; > > + case -ETIME: > > + /* this urb is timing out */ > > + dev_dbg(&pxrc->intf->dev, > > + "%s - urb timed out - was the device unplugged?\n", > > + __func__); > > + return; > > + case -ECONNRESET: > > + case -ENOENT: > > + case -ESHUTDOWN: > > + case -EPIPE: > > + /* this urb is terminated, clean up */ > > + dev_dbg(&pxrc->intf->dev, "%s - urb shutting down with status: %d\n", > > + __func__, urb->status); > > + return; > > + default: > > + dev_dbg(&pxrc->intf->dev, "%s - nonzero urb status received: %d\n", > > + __func__, urb->status); > > + goto exit; > > + } > > + > > + if (urb->actual_length == 8) { > > + input_report_abs(pxrc->input, ABS_X, pxrc->data[0]); > > + input_report_abs(pxrc->input, ABS_Y, pxrc->data[2]); > > + input_report_abs(pxrc->input, ABS_RX, pxrc->data[3]); > > + input_report_abs(pxrc->input, ABS_RY, pxrc->data[4]); > > + input_report_abs(pxrc->input, ABS_RUDDER, pxrc->data[5]); > > + input_report_abs(pxrc->input, ABS_THROTTLE, pxrc->data[6]); > > + input_report_abs(pxrc->input, ABS_MISC, pxrc->data[7]); > > + > > + input_report_key(pxrc->input, BTN_A, pxrc->data[1]); > > + } > > + > > +exit: > > I think you need > > usb_mark_last_busy(interface_to_usbdev(pxrc->intf)); > > here. > Yes. Thank you. > > + /* Resubmit to fetch new fresh URBs */ > > + error = usb_submit_urb(urb, GFP_ATOMIC); > > + if (error && error != -EPERM) > > + dev_err(&pxrc->intf->dev, > > + "%s - usb_submit_urb failed with result: %d", > > + __func__, error); > > +} > > + > > +static int pxrc_open(struct input_dev *input) > > +{ > > + struct pxrc *pxrc = input_get_drvdata(input); > > + int retval; > > + > > + retval = usb_autopm_get_interface(pxrc->intf); > > + if (retval) { > > + dev_err(&pxrc->intf->dev, > > + "%s - usb_autopm_get_interface failed, error: %d\n", > > + __func__, retval); > > + return retval; > > + } > > + > > + retval = usb_submit_urb(pxrc->urb, GFP_KERNEL); > > + if (retval) { > > + dev_err(&pxrc->intf->dev, > > + "%s - usb_submit_urb failed, error: %d\n", > > + __func__, retval); > > + retval = -EIO; > > + goto out; > > + } > > + > > + pxrc->intf->needs_remote_wakeup = 1; > > + > > +out: > > + usb_autopm_put_interface(pxrc->intf); > > + return retval; > > +} > > + > > +static void pxrc_close(struct input_dev *input) > > +{ > > + struct pxrc *pxrc = input_get_drvdata(input); > > + int autopm_error; > > + > > + autopm_error = usb_autopm_get_interface(pxrc->intf); > > + > > + usb_kill_urb(pxrc->urb); > > + pxrc->intf->needs_remote_wakeup = 0; > > + > > + if (!autopm_error) > > + usb_autopm_put_interface(pxrc->intf); > > +} > > + > > +static int pxrc_usb_init(struct pxrc *pxrc) > > +{ > > + struct usb_endpoint_descriptor *epirq; > > + unsigned int pipe; > > + int retval; > > + > > + /* Set up the endpoint information */ > > + /* This device only has an interrupt endpoint */ > > + retval = usb_find_common_endpoints(pxrc->intf->cur_altsetting, > > + NULL, NULL, &epirq, NULL); > > + if (retval) { > > + dev_err(&pxrc->intf->dev, > > + "Could not find endpoint\n"); > > + goto error; > > + } > > + > > + pxrc->bsize = usb_endpoint_maxp(epirq); > > + pxrc->epaddr = epirq->bEndpointAddress; > > + pxrc->data = devm_kmalloc(&pxrc->intf->dev, pxrc->bsize, GFP_KERNEL); > > + if (!pxrc->data) { > > + retval = -ENOMEM; > > + goto error; > > + } > > + > > + usb_set_intfdata(pxrc->intf, pxrc); > > + usb_make_path(pxrc->udev, pxrc->phys, sizeof(pxrc->phys)); > > + strlcat(pxrc->phys, "/input0", sizeof(pxrc->phys)); > > + > > + pxrc->urb = usb_alloc_urb(0, GFP_KERNEL); > > + if (!pxrc->urb) { > > + retval = -ENOMEM; > > + goto error; > > + } > > + > > + pipe = usb_rcvintpipe(pxrc->udev, pxrc->epaddr), > > + usb_fill_int_urb(pxrc->urb, pxrc->udev, pipe, pxrc->data, pxrc->bsize, > > + pxrc_usb_irq, pxrc, 1); > > + > > +error: > > + return retval; > > + > > + > > +} > > + > > + > > +static int pxrc_input_init(struct pxrc *pxrc) > > +{ > > + pxrc->input = devm_input_allocate_device(&pxrc->intf->dev); > > + if (pxrc->input == NULL) { > > + dev_err(&pxrc->intf->dev, "couldn't allocate input device\n"); > > + return -ENOMEM; > > + } > > + > > + pxrc->input->name = "PXRC Flight Controller Adapter"; > > + pxrc->input->phys = pxrc->phys; > > + usb_to_input_id(pxrc->udev, &pxrc->input->id); > > + > > + pxrc->input->open = pxrc_open; > > + pxrc->input->close = pxrc_close; > > + > > + input_set_capability(pxrc->input, EV_KEY, BTN_A); > > + input_set_abs_params(pxrc->input, ABS_X, 0, 255, 0, 0); > > + input_set_abs_params(pxrc->input, ABS_Y, 0, 255, 0, 0); > > + input_set_abs_params(pxrc->input, ABS_RX, 0, 255, 0, 0); > > + input_set_abs_params(pxrc->input, ABS_RY, 0, 255, 0, 0); > > + input_set_abs_params(pxrc->input, ABS_RUDDER, 0, 255, 0, 0); > > + input_set_abs_params(pxrc->input, ABS_THROTTLE, 0, 255, 0, 0); > > + input_set_abs_params(pxrc->input, ABS_MISC, 0, 255, 0, 0); > > + > > + input_set_drvdata(pxrc->input, pxrc); > > + > > + return input_register_device(pxrc->input); > > +} > > + > > +static int pxrc_probe(struct usb_interface *intf, > > + const struct usb_device_id *id) > > +{ > > + struct pxrc *pxrc; > > + int retval; > > + > > + pxrc = devm_kzalloc(&intf->dev, sizeof(*pxrc), GFP_KERNEL); > > + > > + if (!pxrc) > > + return -ENOMEM; > > + > > + pxrc->udev = usb_get_dev(interface_to_usbdev(intf)); > > + pxrc->intf = intf; > > + > > + retval = pxrc_usb_init(pxrc); > > + if (retval) > > + goto error; > > + > > + retval = pxrc_input_init(pxrc); > > + if (retval) > > + goto err_free_urb; > > + > > + return 0; > > + > > +err_free_urb: > > + usb_free_urb(pxrc->urb); > > + > > +error: > > + return retval; > > +} > > + > > +static void pxrc_disconnect(struct usb_interface *intf) > > +{ > > + struct pxrc *pxrc = usb_get_intfdata(intf); > > + > > + usb_free_urb(pxrc->urb); > > + usb_set_intfdata(intf, NULL); > > +} > > + > > +static int pxrc_suspend(struct usb_interface *intf, pm_message_t message) > > +{ > > + struct pxrc *pxrc = usb_get_intfdata(intf); > > + struct input_dev *input_dev = pxrc->input; > > + > > + mutex_lock(&input_dev->mutex); > > + usb_kill_urb(pxrc->urb); > > + mutex_unlock(&input_dev->mutex); > > + > > + return 0; > > +} > > + > > +static int pxrc_resume(struct usb_interface *intf) > > +{ > > + struct pxrc *pxrc = usb_get_intfdata(intf); > > + struct input_dev *input_dev = pxrc->input; > > + int retval = 0; > > + > > + mutex_lock(&input_dev->mutex); > > + > > + if (input_dev->users && usb_submit_urb(pxrc->urb, GFP_NOIO) < 0) > > + retval = -EIO; > > + > > + mutex_unlock(&input_dev->mutex); > > + > > + return retval; > > +} > > + > > +static int pxrc_pre_reset(struct usb_interface *intf) > > +{ > > + struct pxrc *pxrc = usb_get_intfdata(intf); > > + struct input_dev *input_dev = pxrc->input; > > + > > + mutex_lock(&input_dev->mutex); > > + usb_kill_urb(pxrc->urb); > > + > > + return 0; > > +} > > + > > +static int pxrc_post_reset(struct usb_interface *intf) > > +{ > > + struct pxrc *pxrc = usb_get_intfdata(intf); > > + struct input_dev *input_dev = pxrc->input; > > + int retval = 0; > > + > > + if (input_dev->users && usb_submit_urb(pxrc->urb, GFP_NOIO) < 0) > > + retval = -EIO; > > + > > + mutex_unlock(&input_dev->mutex); > > + > > + return retval; > > +} > > + > > +static int pxrc_reset_resume(struct usb_interface *intf) > > +{ > > + return pxrc_resume(intf); > > +} > > + > > +static struct usb_driver pxrc_driver = { > > + .name = "pxrc", > > + .probe = pxrc_probe, > > + .disconnect = pxrc_disconnect, > > + .id_table = pxrc_table, > > + .suspend = pxrc_suspend, > > + .resume = pxrc_resume, > > + .pre_reset = pxrc_pre_reset, > > + .post_reset = pxrc_post_reset, > > + .reset_resume = pxrc_reset_resume, > > + .supports_autosuspend = 1, > > +}; > > + > > +module_usb_driver(pxrc_driver); > > + > > +MODULE_AUTHOR("Marcus Folkesson <marcus.folkesson@xxxxxxxxx>"); > > +MODULE_DESCRIPTION("PhoenixRC Flight Controller Adapter"); > > +MODULE_LICENSE("GPL v2"); > > -- > > 2.15.1 > > > > Thank you. > > -- > Dmitry Best regards Marcus Folkesson -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
