Re: [PATCH v2] hid: usbhid: hid-core: fix recursive deadlock
On Fri, 20 Nov 2015 22:19:02 +0200
Ioan-Adrian Ratiu wrote:
> The critical section protected by usbhid->lock in hid_ctrl() is too
> big and because of this it causes a recursive deadlock. "Too big" means
> the case statement and the call to hid_input_report() do not need to be
> protected by the spinlock (no URB operations are done inside them).
>
> The deadlock happens because in certain rare cases drivers try to grab
> the lock while handling the ctrl irq which grabs the lock before them
> as described above. For example newer wacom tablets like 056a:033c try
> to reschedule proximity reads from wacom_intuos_schedule_prox_event()
> calling hid_hw_request() -> usbhid_request() -> usbhid_submit_report()
> which tries to grab the usbhid lock already held by hid_ctrl().
>
> There are two ways to get out of this deadlock:
> 1. Make the drivers work "around" the ctrl critical region, in the
> wacom case for ex. by delaying the scheduling of the proximity read
> request itself to a workqueue.
> 2. Shrink the critical region so the usbhid lock protects only the
> instructions which modify usbhid state, calling hid_input_report()
> with the spinlock unlocked, allowing the device driver to grab the
> lock first, finish and then grab the lock afterwards in hid_ctrl().
>
> This patch implements the 2nd solution.
>
> Signed-off-by: Ioan-Adrian Ratiu
> ---
> drivers/hid/usbhid/hid-core.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index 36712e9..5dd426f 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -477,8 +477,6 @@ static void hid_ctrl(struct urb *urb)
> struct usbhid_device *usbhid = hid->driver_data;
> int unplug = 0, status = urb->status;
>
> - spin_lock(&usbhid->lock);
> -
> switch (status) {
> case 0: /* success */
> if (usbhid->ctrl[usbhid->ctrltail].dir == USB_DIR_IN)
> @@ -498,6 +496,8 @@ static void hid_ctrl(struct urb *urb)
> hid_warn(urb->dev, "ctrl urb status %d received\n", status);
> }
>
> + spin_lock(&usbhid->lock);
> +
> if (unplug) {
> usbhid->ctrltail = usbhid->ctrlhead;
> } else {
Hello again
Can this please be merged in the 4.4? There are quite a few people who have
their tablets deadlock and don't know how to patch their kernels so are stuck
waiting for a new release.
The severity of this issue is much bigger than I initially thought. Since I've
posted on the wacom mailing list that I have a fix for this deadlock I've
recived lots of email from people complaining of the same problem on a wide
range of tablets.
Some of those people know how to patch a kernel, some found this patch on the
mailing list and tested it and confirmed that it works on the wacom mailing
list (you can verify the deadlock + fix on that mailing list).
Thank you,
Adrian
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 0/5] usb: usbtmc: Add support for missing functions in USBTMC-USB488 spec
Implement support for the USB488 defined READ_STATUS_BYTE ioctl (1/5) and SRQ notifications with fasync (2/5) and poll/select (3/5) in order to be able to synchronize with variable duration instrument operations. Add ioctls for other USB488 requests: REN_CONTROL, GOTO_LOCAL and LOCAL_LOCKOUT. (4/5) Add convenience ioctl to return all device capabilities (5/5) PATCH Changelog: v6 - Remove more excess newlines Rearrange declaration blocks aesthetically Remove __func__ parameter from dev_xxx calls Simplify tests for interrupt-in notifications Propagate return code from usb_submit_urb() v5 - Remove excess newlines and parens - Move dev variable initialisations to declaration - Add comment on interrupt btag wrap - simplify test in usbtmc_free_int() v4 - Remove excess newlines and parens - simplify some expressions v3 - Split into multiple patches as per gregkh request V2 - Fix V1 bug: not waking sleepers on disconnect. - Correct sparse warnings. V1 - Original patch Testing: All functions tested ok on an USBTMC-USB488 compliant oscilloscope Dave Penkler (5): Implement an ioctl to support the USMTMC-USB488 READ_STATUS_BYTE operation. Add support for USBTMC USB488 SRQ notification with fasync Add support for receiving USBTMC USB488 SRQ notifications via poll/select Add ioctl to retrieve USBTMC-USB488 capabilities Add ioctls to enable and disable local controls on an instrument drivers/usb/class/usbtmc.c | 330 +++ include/uapi/linux/usb/tmc.h | 29 +++- 2 files changed, 356 insertions(+), 3 deletions(-) -- 2.6.3 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 1/5] Implement an ioctl to support the USMTMC-USB488 READ_STATUS_BYTE operation.
Background:
When performing a read on an instrument that is executing a function
that runs longer than the USB timeout the instrument may hang and
require a device reset to recover. The READ_STATUS_BYTE operation
always returns even when the instrument is busy permitting to poll
for the appropriate condition. This capability is referred to in
instrument application notes on synchronizing acquisitions for other
platforms.
Signed-off-by: Dave Penkler
---
drivers/usb/class/usbtmc.c | 200 +++
include/uapi/linux/usb/tmc.h | 2 +
2 files changed, 202 insertions(+)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 7a11a82..89456df9 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -87,6 +87,19 @@ struct usbtmc_device_data {
u8 bTag_last_write; /* needed for abort */
u8 bTag_last_read; /* needed for abort */
+ /* data for interrupt in endpoint handling */
+ u8 bNotify1;
+ u8 bNotify2;
+ u16ifnum;
+ u8 iin_bTag;
+ u8*iin_buffer;
+ atomic_t iin_data_valid;
+ unsigned int iin_ep;
+ intiin_ep_present;
+ intiin_interval;
+ struct urb*iin_urb;
+ u16iin_wMaxPacketSize;
+
u8 rigol_quirk;
/* attributes from the USB TMC spec for this device */
@@ -99,6 +112,7 @@ struct usbtmc_device_data {
struct usbtmc_dev_capabilities capabilities;
struct kref kref;
struct mutex io_mutex; /* only one i/o function running at a time */
+ wait_queue_head_t waitq;
};
#define to_usbtmc_data(d) container_of(d, struct usbtmc_device_data, kref)
@@ -373,6 +387,83 @@ exit:
return rv;
}
+static int usbtmc488_ioctl_read_stb(struct usbtmc_device_data *data,
+ unsigned long arg)
+{
+ struct device *dev = &data->intf->dev;
+ u8 *buffer;
+ u8 tag, stb;
+ int rv;
+
+ dev_dbg(dev, "Enter ioctl_read_stb iin_ep_present: %d\n",
+ data->iin_ep_present);
+
+ buffer = kmalloc(8, GFP_KERNEL);
+ if (!buffer)
+ return -ENOMEM;
+
+ atomic_set(&data->iin_data_valid, 0);
+
+ rv = usb_control_msg(data->usb_dev,
+ usb_rcvctrlpipe(data->usb_dev, 0),
+ USBTMC488_REQUEST_READ_STATUS_BYTE,
+ USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+ data->iin_bTag,
+ data->ifnum,
+ buffer, 0x03, USBTMC_TIMEOUT);
+ if (rv < 0) {
+ dev_err(dev, "stb usb_control_msg returned %d\n", rv);
+ goto exit;
+ }
+
+ if (buffer[0] != USBTMC_STATUS_SUCCESS) {
+ dev_err(dev, "control status returned %x\n", buffer[0]);
+ rv = -EIO;
+ goto exit;
+ }
+
+ if (data->iin_ep_present) {
+ rv = wait_event_interruptible_timeout(
+ data->waitq,
+ atomic_read(&data->iin_data_valid) != 0,
+ USBTMC_TIMEOUT);
+ if (rv < 0) {
+ dev_dbg(dev, "wait interrupted %d\n", rv);
+ goto exit;
+ }
+
+ if (rv == 0) {
+ dev_dbg(dev, "wait timed out\n");
+ rv = -ETIME;
+ goto exit;
+ }
+
+ tag = data->bNotify1 & 0x7f;
+ if (tag != data->iin_bTag) {
+ dev_err(dev, "expected bTag %x got %x\n",
+ data->iin_bTag, tag);
+ }
+
+ stb = data->bNotify2;
+ } else {
+ stb = buffer[2];
+ }
+
+ rv = copy_to_user((void __user *)arg, &stb, sizeof(stb));
+ if (rv)
+ rv = -EFAULT;
+
+ exit:
+ /* bump interrupt bTag */
+ data->iin_bTag += 1;
+ if (data->iin_bTag > 127)
+ /* 1 is for SRQ see USBTMC-USB488 subclass spec section 4.3.1 */
+ data->iin_bTag = 2;
+
+ kfree(buffer);
+ return rv;
+}
+
/*
* Sends a REQUEST_DEV_DEP_MSG_IN message on the Bulk-IN endpoint.
* @transfer_size: number of bytes to request from the device.
@@ -1069,6 +1160,10 @@ static long usbtmc_ioctl(struct file *file, unsigned int
cmd, unsigned long arg)
case USBTMC_IOCTL_ABORT_BULK_IN:
retval = usbtmc_ioctl_abort_bulk_in(data);
break;
+
+ case USBTMC488_IOCTL_READ_STB:
+ retval = usbtmc488_ioctl_read_stb(data, arg);
+ break;
}
skip_io_on_zombie:
@@ -1092,6 +1187,57 @@ static struct usb_class_driver usbtmc_class = {
.minor_base = USBTMC_MINOR_BASE,
};
+static void usbtmc_interrupt(struct urb *urb)
+{
+ struc
[PATCH v6 2/5] Add support for USBTMC USB488 SRQ notification with fasync
Background:
By configuring an instrument's event status register various
conditions can be reported via an SRQ notification. This complements
the synchronous polling approach using the READ_STATUS_BYTE ioctl
with an asynchronous notification.
Signed-off-by: Dave Penkler
---
drivers/usb/class/usbtmc.c | 24
1 file changed, 24 insertions(+)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 89456df9..7bfd6ec 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -99,6 +99,7 @@ struct usbtmc_device_data {
intiin_interval;
struct urb*iin_urb;
u16iin_wMaxPacketSize;
+ atomic_t srq_asserted;
u8 rigol_quirk;
@@ -113,6 +114,7 @@ struct usbtmc_device_data {
struct kref kref;
struct mutex io_mutex; /* only one i/o function running at a time */
wait_queue_head_t waitq;
+ struct fasync_struct *fasync;
};
#define to_usbtmc_data(d) container_of(d, struct usbtmc_device_data, kref)
@@ -404,6 +406,9 @@ static int usbtmc488_ioctl_read_stb(struct
usbtmc_device_data *data,
atomic_set(&data->iin_data_valid, 0);
+ /* must issue read_stb before using poll or select */
+ atomic_set(&data->srq_asserted, 0);
+
rv = usb_control_msg(data->usb_dev,
usb_rcvctrlpipe(data->usb_dev, 0),
USBTMC488_REQUEST_READ_STATUS_BYTE,
@@ -1171,6 +1176,13 @@ skip_io_on_zombie:
return retval;
}
+static int usbtmc_fasync(int fd, struct file *file, int on)
+{
+ struct usbtmc_device_data *data = file->private_data;
+
+ return fasync_helper(fd, file, on, &data->fasync);
+}
+
static const struct file_operations fops = {
.owner = THIS_MODULE,
.read = usbtmc_read,
@@ -1178,6 +1190,7 @@ static const struct file_operations fops = {
.open = usbtmc_open,
.release= usbtmc_release,
.unlocked_ioctl = usbtmc_ioctl,
+ .fasync = usbtmc_fasync,
.llseek = default_llseek,
};
@@ -1207,6 +1220,16 @@ static void usbtmc_interrupt(struct urb *urb)
wake_up_interruptible(&data->waitq);
goto exit;
}
+ /* check for SRQ notification */
+ if (data->iin_buffer[0] == 0x81) {
+ if (data->fasync)
+ kill_fasync(&data->fasync,
+ SIGIO, POLL_IN);
+
+ atomic_set(&data->srq_asserted, 1);
+ wake_up_interruptible(&data->waitq);
+ goto exit;
+ }
dev_warn(dev, "invalid notification: %x\n",
data->iin_buffer[0]);
break;
case -EOVERFLOW:
@@ -1262,6 +1285,7 @@ static int usbtmc_probe(struct usb_interface *intf,
mutex_init(&data->io_mutex);
init_waitqueue_head(&data->waitq);
atomic_set(&data->iin_data_valid, 0);
+ atomic_set(&data->srq_asserted, 0);
data->zombie = 0;
/* Determine if it is a Rigol or not */
--
2.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 3/5] Add support for receiving USBTMC USB488 SRQ notifications via poll/select
Background:
In many situations operations on multiple instruments need to be
synchronized. poll/select provide a convenient way of waiting on a
number of different instruments and other peripherals
simultaneously.
Signed-off-by: Dave Penkler
---
drivers/usb/class/usbtmc.c | 23 +++
1 file changed, 23 insertions(+)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 7bfd6ec..3b85ef5 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1183,6 +1184,27 @@ static int usbtmc_fasync(int fd, struct file *file, int
on)
return fasync_helper(fd, file, on, &data->fasync);
}
+static unsigned int usbtmc_poll(struct file *file, poll_table *wait)
+{
+ struct usbtmc_device_data *data = file->private_data;
+ unsigned int mask;
+
+ mutex_lock(&data->io_mutex);
+
+ if (data->zombie) {
+ mask = POLLHUP | POLLERR;
+ goto no_poll;
+ }
+
+ poll_wait(file, &data->waitq, wait);
+
+ mask = (atomic_read(&data->srq_asserted)) ? POLLIN | POLLRDNORM : 0;
+
+no_poll:
+ mutex_unlock(&data->io_mutex);
+ return mask;
+}
+
static const struct file_operations fops = {
.owner = THIS_MODULE,
.read = usbtmc_read,
@@ -1191,6 +1213,7 @@ static const struct file_operations fops = {
.release= usbtmc_release,
.unlocked_ioctl = usbtmc_ioctl,
.fasync = usbtmc_fasync,
+ .poll = usbtmc_poll,
.llseek = default_llseek,
};
--
2.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 4/5] Add ioctl to retrieve USBTMC-USB488 capabilities
This is a convenience function to obtain an instrument's
capabilities from its file descriptor without having to access sysfs
from the user program.
Signed-off-by: Dave Penkler
---
drivers/usb/class/usbtmc.c | 12
include/uapi/linux/usb/tmc.h | 21 ++---
2 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 3b85ef5..3a3264c 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -102,6 +102,9 @@ struct usbtmc_device_data {
u16iin_wMaxPacketSize;
atomic_t srq_asserted;
+ /* coalesced usb488_caps from usbtmc_dev_capabilities */
+ u8 usb488_caps;
+
u8 rigol_quirk;
/* attributes from the USB TMC spec for this device */
@@ -992,6 +995,7 @@ static int get_capabilities(struct usbtmc_device_data *data)
data->capabilities.device_capabilities = buffer[5];
data->capabilities.usb488_interface_capabilities = buffer[14];
data->capabilities.usb488_device_capabilities = buffer[15];
+ data->usb488_caps = (buffer[14] & 0x07) | ((buffer[15] & 0x0f) << 4);
rv = 0;
err_out:
@@ -1167,6 +1171,14 @@ static long usbtmc_ioctl(struct file *file, unsigned int
cmd, unsigned long arg)
retval = usbtmc_ioctl_abort_bulk_in(data);
break;
+ case USBTMC488_IOCTL_GET_CAPS:
+ retval = copy_to_user((void __user *)arg,
+ &data->usb488_caps,
+ sizeof(data->usb488_caps));
+ if (retval)
+ retval = -EFAULT;
+ break;
+
case USBTMC488_IOCTL_READ_STB:
retval = usbtmc488_ioctl_read_stb(data, arg);
break;
diff --git a/include/uapi/linux/usb/tmc.h b/include/uapi/linux/usb/tmc.h
index 7e5ced8..1dc3af1 100644
--- a/include/uapi/linux/usb/tmc.h
+++ b/include/uapi/linux/usb/tmc.h
@@ -2,12 +2,14 @@
* Copyright (C) 2007 Stefan Kopp, Gechingen, Germany
* Copyright (C) 2008 Novell, Inc.
* Copyright (C) 2008 Greg Kroah-Hartman
+ * Copyright (C) 2015 Dave Penkler
*
* This file holds USB constants defined by the USB Device Class
- * Definition for Test and Measurement devices published by the USB-IF.
+ * and USB488 Subclass Definitions for Test and Measurement devices
+ * published by the USB-IF.
*
- * It also has the ioctl definitions for the usbtmc kernel driver that
- * userspace needs to know about.
+ * It also has the ioctl and capability definitions for the
+ * usbtmc kernel driver that userspace needs to know about.
*/
#ifndef __LINUX_USB_TMC_H
@@ -40,6 +42,19 @@
#define USBTMC_IOCTL_ABORT_BULK_IN _IO(USBTMC_IOC_NR, 4)
#define USBTMC_IOCTL_CLEAR_OUT_HALT_IO(USBTMC_IOC_NR, 6)
#define USBTMC_IOCTL_CLEAR_IN_HALT _IO(USBTMC_IOC_NR, 7)
+#define USBTMC488_IOCTL_GET_CAPS _IO(USBTMC_IOC_NR, 17)
#define USBTMC488_IOCTL_READ_STB _IOR(USBTMC_IOC_NR, 18, unsigned char)
+/* Driver encoded usb488 capabilities */
+#define USBTMC488_CAPABILITY_TRIGGER 1
+#define USBTMC488_CAPABILITY_SIMPLE 2
+#define USBTMC488_CAPABILITY_REN_CONTROL 2
+#define USBTMC488_CAPABILITY_GOTO_LOCAL 2
+#define USBTMC488_CAPABILITY_LOCAL_LOCKOUT 2
+#define USBTMC488_CAPABILITY_488_DOT_2 4
+#define USBTMC488_CAPABILITY_DT1 16
+#define USBTMC488_CAPABILITY_RL1 32
+#define USBTMC488_CAPABILITY_SR1 64
+#define USBTMC488_CAPABILITY_FULL_SCPI 128
+
#endif
--
2.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 5/5] Add ioctls to enable and disable local controls on an instrument
These ioctls provide support for the USBTMC-USB488 control requests
for REN_CONTROL, GO_TO_LOCAL and LOCAL_LOCKOUT
Signed-off-by: Dave Penkler
---
drivers/usb/class/usbtmc.c | 71
include/uapi/linux/usb/tmc.h | 6
2 files changed, 77 insertions(+)
diff --git a/drivers/usb/class/usbtmc.c b/drivers/usb/class/usbtmc.c
index 3a3264c..f04a086 100644
--- a/drivers/usb/class/usbtmc.c
+++ b/drivers/usb/class/usbtmc.c
@@ -473,6 +473,62 @@ static int usbtmc488_ioctl_read_stb(struct
usbtmc_device_data *data,
return rv;
}
+static int usbtmc488_ioctl_simple(struct usbtmc_device_data *data,
+ unsigned long arg,
+ unsigned int cmd)
+{
+ struct device *dev = &data->intf->dev;
+ unsigned int val;
+ u8 *buffer;
+ u16 wValue;
+ int rv;
+
+ if (!(data->usb488_caps & USBTMC488_CAPABILITY_SIMPLE))
+ return -EINVAL;
+
+ buffer = kmalloc(8, GFP_KERNEL);
+ if (!buffer)
+ return -ENOMEM;
+
+ if (cmd == USBTMC488_REQUEST_REN_CONTROL) {
+ rv = copy_from_user(&val, (void __user *)arg, sizeof(val));
+ if (rv) {
+ rv = -EFAULT;
+ goto exit;
+ }
+ wValue = val ? 1 : 0;
+ } else {
+ wValue = 0;
+ }
+
+ rv = usb_control_msg(data->usb_dev,
+ usb_rcvctrlpipe(data->usb_dev, 0),
+ cmd,
+ USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE,
+ wValue,
+ data->ifnum,
+ buffer, 0x01, USBTMC_TIMEOUT);
+ if (rv < 0) {
+ dev_err(dev, "simple usb_control_msg failed %d\n", rv);
+ goto exit;
+ } else if (rv != 1) {
+ dev_warn(dev, "simple usb_control_msg returned %d\n", rv);
+ rv = -EIO;
+ goto exit;
+ }
+
+ if (buffer[0] != USBTMC_STATUS_SUCCESS) {
+ dev_err(dev, "simple control status returned %x\n", buffer[0]);
+ rv = -EIO;
+ goto exit;
+ }
+ rv = 0;
+
+ exit:
+ kfree(buffer);
+ return rv;
+}
+
/*
* Sends a REQUEST_DEV_DEP_MSG_IN message on the Bulk-IN endpoint.
* @transfer_size: number of bytes to request from the device.
@@ -1182,6 +1238,21 @@ static long usbtmc_ioctl(struct file *file, unsigned int
cmd, unsigned long arg)
case USBTMC488_IOCTL_READ_STB:
retval = usbtmc488_ioctl_read_stb(data, arg);
break;
+
+ case USBTMC488_IOCTL_REN_CONTROL:
+ retval = usbtmc488_ioctl_simple(data, arg,
+ USBTMC488_REQUEST_REN_CONTROL);
+ break;
+
+ case USBTMC488_IOCTL_GOTO_LOCAL:
+ retval = usbtmc488_ioctl_simple(data, arg,
+ USBTMC488_REQUEST_GOTO_LOCAL);
+ break;
+
+ case USBTMC488_IOCTL_LOCAL_LOCKOUT:
+ retval = usbtmc488_ioctl_simple(data, arg,
+
USBTMC488_REQUEST_LOCAL_LOCKOUT);
+ break;
}
skip_io_on_zombie:
diff --git a/include/uapi/linux/usb/tmc.h b/include/uapi/linux/usb/tmc.h
index 1dc3af1..0d852c9 100644
--- a/include/uapi/linux/usb/tmc.h
+++ b/include/uapi/linux/usb/tmc.h
@@ -33,6 +33,9 @@
#define USBTMC_REQUEST_GET_CAPABILITIES7
#define USBTMC_REQUEST_INDICATOR_PULSE 64
#define USBTMC488_REQUEST_READ_STATUS_BYTE 128
+#define USBTMC488_REQUEST_REN_CONTROL 160
+#define USBTMC488_REQUEST_GOTO_LOCAL 161
+#define USBTMC488_REQUEST_LOCAL_LOCKOUT162
/* Request values for USBTMC driver's ioctl entry point */
#define USBTMC_IOC_NR 91
@@ -44,6 +47,9 @@
#define USBTMC_IOCTL_CLEAR_IN_HALT _IO(USBTMC_IOC_NR, 7)
#define USBTMC488_IOCTL_GET_CAPS _IO(USBTMC_IOC_NR, 17)
#define USBTMC488_IOCTL_READ_STB _IOR(USBTMC_IOC_NR, 18, unsigned char)
+#define USBTMC488_IOCTL_REN_CONTROL_IOW(USBTMC_IOC_NR, 19, unsigned char)
+#define USBTMC488_IOCTL_GOTO_LOCAL _IO(USBTMC_IOC_NR, 20)
+#define USBTMC488_IOCTL_LOCAL_LOCKOUT _IO(USBTMC_IOC_NR, 21)
/* Driver encoded usb488 capabilities */
#define USBTMC488_CAPABILITY_TRIGGER 1
--
2.6.3
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] net: usb: ch9200: use kmemdup instead of kmalloc + memcpy
From: Anup Limbu Date: Wed, 25 Nov 2015 15:37:21 +0530 > replace kmalloc + memset with kmemdup > > Signed-off-by: Anup Limbu I agree with Bjorn that this microturfing cleanup misses the real higher level opportunity to use the usbnet_read_cmd() and usbnet_write_cmd() helpers that already exist. Over time I am getting more and more weary of laser focused microscopic cleanups. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/2] Minor improvement for smsc95xx netusb driver performance.
From: Ameen Date: Wed, 25 Nov 2015 23:55:26 +0200 > if (csum) > - tx_cmd_b |= TX_CMD_B_CSUM_ENABLE; > - cpu_to_le32s(&tx_cmd_b); > - memcpy(skb->data, &tx_cmd_b, 4); > + tx_cmds.cmd_b |= TX_CMD_B_CSUM_ENABLE; You've corrupted the indentation here. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 0/2] Two fix for dwc2 gadget driver
From: "Du, Changbin" With the first patch, enable a enabled ep will return -EBUSY. The second patch forbid queuing on disabled ep to avoid panic. Du, Changbin (2): usb: dwc2: add ep enabled flag to avoid double enable/disable usb: dwc2: forbid queuing request to a disabled ep drivers/usb/dwc2/core.h | 1 + drivers/usb/dwc2/gadget.c | 26 +- 2 files changed, 26 insertions(+), 1 deletion(-) -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 1/2] usb: dwc2: add ep enabled flag to avoid double enable/disable
From: "Du, Changbin"
Enabling a already enabled ep is illegal, because the ep may has trbs
running. Reprogram the ep may break running transfer. So udc driver
must avoid this happening by return an error -EBUSY. Gadget function
driver also should avoid such things, but that is out of udc driver.
Similarly, disable a disabled ep makes no sense, but no need return
an error here.
Signed-off-by: Du, Changbin
---
drivers/usb/dwc2/core.h | 1 +
drivers/usb/dwc2/gadget.c | 20 +++-
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index a66d3cb..cf7eccd 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -162,6 +162,7 @@ struct dwc2_hsotg_ep {
unsigned char mc;
unsigned char interval;
+ unsigned intenabled:1;
unsigned inthalted:1;
unsigned intperiodic:1;
unsigned intisochronous:1;
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 0abf73c..586bbcd 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -2423,6 +2423,7 @@ void dwc2_hsotg_core_init_disconnected(struct dwc2_hsotg
*hsotg,
/* enable, but don't activate EP0in */
dwc2_writel(dwc2_hsotg_ep0_mps(hsotg->eps_out[0]->ep.maxpacket) |
DXEPCTL_USBACTEP, hsotg->regs + DIEPCTL0);
+ hsotg->eps_out[0]->enabled = 1;
dwc2_hsotg_enqueue_setup(hsotg);
@@ -2680,6 +2681,14 @@ static int dwc2_hsotg_ep_enable(struct usb_ep *ep,
return -EINVAL;
}
+ spin_lock_irqsave(&hsotg->lock, flags);
+ if (hs_ep->enabled) {
+ dev_warn(hsotg->dev, "%s: ep %s already enabled\n",
+ __func__, hs_ep->name);
+ ret = -EBUSY;
+ goto error;
+ }
+
mps = usb_endpoint_maxp(desc);
/* note, we handle this here instead of dwc2_hsotg_set_ep_maxpacket */
@@ -2690,7 +2699,6 @@ static int dwc2_hsotg_ep_enable(struct usb_ep *ep,
dev_dbg(hsotg->dev, "%s: read DxEPCTL=0x%08x from 0x%08x\n",
__func__, epctrl, epctrl_reg);
- spin_lock_irqsave(&hsotg->lock, flags);
epctrl &= ~(DXEPCTL_EPTYPE_MASK | DXEPCTL_MPS_MASK);
epctrl |= DXEPCTL_MPS(mps);
@@ -2806,6 +2814,8 @@ static int dwc2_hsotg_ep_enable(struct usb_ep *ep,
/* enable the endpoint interrupt */
dwc2_hsotg_ctrl_epint(hsotg, index, dir_in, 1);
+ hs_ep->enabled = 1;
+
error:
spin_unlock_irqrestore(&hsotg->lock, flags);
return ret;
@@ -2835,6 +2845,11 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep)
epctrl_reg = dir_in ? DIEPCTL(index) : DOEPCTL(index);
spin_lock_irqsave(&hsotg->lock, flags);
+ if (!hs_ep->enabled) {
+ dev_warn(hsotg->dev, "%s: ep %s already disabled\n",
+ __func__, hs_ep->name);
+ goto out;
+ }
hsotg->fifo_map &= ~(1fifo_index = 0;
@@ -2854,6 +2869,9 @@ static int dwc2_hsotg_ep_disable(struct usb_ep *ep)
/* terminate all requests with shutdown */
kill_all_requests(hsotg, hs_ep, -ESHUTDOWN);
+ hs_ep->enabled = 0;
+
+out:
spin_unlock_irqrestore(&hsotg->lock, flags);
return 0;
}
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 2/2] usb: dwc2: forbid queuing request to a disabled ep
From: "Du, Changbin"
Queue a request to disabled ep doesn't make sense, and induce caller
make mistakes.
Here is a example for the android mtp gadget function driver. A mem
corruption can happen on below senario.
1) On disconnect, mtp driver disable its EPs,
2) During send_file_work and receive_file_work, mtp queues a request
to ep. (The mtp driver need improve its synchronization logic!)
3) mtp_function_unbind is invoked and all mtp requests are freed.
4) when dwc2 process the request queued on step 2, will cause kernel
NULL pointer dereference exception.
Signed-off-by: Du, Changbin
---
drivers/usb/dwc2/gadget.c | 6 ++
1 file changed, 6 insertions(+)
diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 586bbcd..4d637ab 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -786,6 +786,12 @@ static int dwc2_hsotg_ep_queue(struct usb_ep *ep, struct
usb_request *req,
ep->name, req, req->length, req->buf, req->no_interrupt,
req->zero, req->short_not_ok);
+ if (!hs_ep->enabled) {
+ dev_warn(hs->dev, "%s: cannot queue to disabled ep\n",
+ __func__);
+ return -ESHUTDOWN;
+ }
+
/* Prevent new request submission when controller is suspended */
if (hs->lx_state == DWC2_L2) {
dev_dbg(hs->dev, "%s: don't submit request while suspended\n",
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usb: host: pci_quirks: fix memory leak, by adding iounmap
pinging in case this patch is lost. On 6 November 2015 at 17:46, Saurabh Sengar wrote: > added iounmap inorder to free memory mapped to base before returning > > Signed-off-by: Saurabh Sengar > --- > drivers/usb/host/pci-quirks.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/usb/host/pci-quirks.c b/drivers/usb/host/pci-quirks.c > index f940056..332f687 100644 > --- a/drivers/usb/host/pci-quirks.c > +++ b/drivers/usb/host/pci-quirks.c > @@ -990,7 +990,7 @@ static void quirk_usb_handoff_xhci(struct pci_dev *pdev) > /* We're reading garbage from the controller */ > dev_warn(&pdev->dev, > "xHCI controller failing to respond"); > - return; > + goto iounmap; > } > > if (!ext_cap_offset) > @@ -1061,7 +1061,7 @@ hc_init: > "xHCI HW did not halt within %d usec status = > 0x%x\n", > XHCI_MAX_HALT_USEC, val); > } > - > +iounmap: > iounmap(base); > } > > -- > 1.9.1 > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
