From 19b85b3b87fd1388df1f4a35969823521d35d243 Mon Sep 17 00:00:00 2001 From: Bjørn Mork Date: Mon, 16 Jan 2012 15:11:58 +0100 Subject: USB: cdc-wdm: no need to fill the in request URB every time it's submitted MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Filling the same URB with the exact same data is pointless. It can be filled once and resubmitted. And not doing buffer allocation and URB filling at the same place only serves to hide size mismatch bugs Signed-off-by: Bjørn Mork Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-wdm.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-) (limited to 'drivers/usb/class') diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index 1c50baff7725..9734863a3a49 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -159,11 +159,9 @@ static void wdm_int_callback(struct urb *urb) int rv = 0; int status = urb->status; struct wdm_device *desc; - struct usb_ctrlrequest *req; struct usb_cdc_notification *dr; desc = urb->context; - req = desc->irq; dr = (struct usb_cdc_notification *)desc->sbuf; if (status) { @@ -210,24 +208,6 @@ static void wdm_int_callback(struct urb *urb) goto exit; } - req->bRequestType = (USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE); - req->bRequest = USB_CDC_GET_ENCAPSULATED_RESPONSE; - req->wValue = 0; - req->wIndex = desc->inum; - req->wLength = cpu_to_le16(desc->wMaxCommand); - - usb_fill_control_urb( - desc->response, - interface_to_usbdev(desc->intf), - /* using common endpoint 0 */ - usb_rcvctrlpipe(interface_to_usbdev(desc->intf), 0), - (unsigned char *)req, - desc->inbuf, - desc->wMaxCommand, - wdm_in_callback, - desc - ); - desc->response->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; spin_lock(&desc->iuspin); clear_bit(WDM_READ, &desc->flags); set_bit(WDM_RESPONDING, &desc->flags); @@ -734,6 +714,25 @@ next_desc: ); desc->validity->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; + desc->irq->bRequestType = (USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE); + desc->irq->bRequest = USB_CDC_GET_ENCAPSULATED_RESPONSE; + desc->irq->wValue = 0; + desc->irq->wIndex = desc->inum; + desc->irq->wLength = cpu_to_le16(desc->wMaxCommand); + + usb_fill_control_urb( + desc->response, + interface_to_usbdev(desc->intf), + /* using common endpoint 0 */ + usb_rcvctrlpipe(interface_to_usbdev(desc->intf), 0), + (unsigned char *)desc->irq, + desc->inbuf, + desc->wMaxCommand, + wdm_in_callback, + desc + ); + desc->response->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; + usb_set_intfdata(intf, desc); rv = usb_register_dev(intf, &wdm_class); if (rv < 0) -- cgit v1.2.3-59-g8ed1b From cafbe85fb0d00d32988905c4978df433ca9b6512 Mon Sep 17 00:00:00 2001 From: Bjørn Mork Date: Mon, 16 Jan 2012 15:11:59 +0100 Subject: USB: cdc-wdm: better allocate a buffer that is at least as big as we tell the USB core MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As it turns out, there was a mismatch between the allocated inbuf size (desc->bMaxPacketSize0, typically something like 64) and the length we specified in the URB (desc->wMaxCommand, typically something like 2048) Signed-off-by: Bjørn Mork Cc: Oliver Neukum Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-wdm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/class') diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index 9734863a3a49..846dfa603447 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -696,7 +696,7 @@ next_desc: goto err; desc->inbuf = usb_alloc_coherent(interface_to_usbdev(intf), - desc->bMaxPacketSize0, + desc->wMaxCommand, GFP_KERNEL, &desc->response->transfer_dma); if (!desc->inbuf) -- cgit v1.2.3-59-g8ed1b From 8457d99cab81e91724b43363f7fccd851d766187 Mon Sep 17 00:00:00 2001 From: Bjørn Mork Date: Mon, 16 Jan 2012 15:12:00 +0100 Subject: USB: cdc-wdm: no need to use usb_alloc_coherent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As Documentation/usb/dma.txt states: Most drivers should *NOT* be using these primitives; they don't need to use this type of memory (dma-coherent), and memory returned from kmalloc() will work just fine. This driver handle only very low bandwith transfers. It is not an obvious candidate for usb_alloc_coherent(). Using these calls only serves to complicate the code for no gain, as has been shown by multiple bugs related to this allocation path. Signed-off-by: Bjørn Mork Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-wdm.c | 39 +++++++++------------------------------ 1 file changed, 9 insertions(+), 30 deletions(-) (limited to 'drivers/usb/class') diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index 846dfa603447..8909058b1bb1 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -256,14 +256,8 @@ static void free_urbs(struct wdm_device *desc) static void cleanup(struct wdm_device *desc) { - usb_free_coherent(interface_to_usbdev(desc->intf), - desc->wMaxPacketSize, - desc->sbuf, - desc->validity->transfer_dma); - usb_free_coherent(interface_to_usbdev(desc->intf), - desc->bMaxPacketSize0, - desc->inbuf, - desc->response->transfer_dma); + kfree(desc->sbuf); + kfree(desc->inbuf); kfree(desc->orq); kfree(desc->irq); kfree(desc->ubuf); @@ -688,19 +682,13 @@ next_desc: if (!desc->ubuf) goto err; - desc->sbuf = usb_alloc_coherent(interface_to_usbdev(intf), - desc->wMaxPacketSize, - GFP_KERNEL, - &desc->validity->transfer_dma); + desc->sbuf = kmalloc(desc->wMaxPacketSize, GFP_KERNEL); if (!desc->sbuf) goto err; - desc->inbuf = usb_alloc_coherent(interface_to_usbdev(intf), - desc->wMaxCommand, - GFP_KERNEL, - &desc->response->transfer_dma); + desc->inbuf = kmalloc(desc->wMaxCommand, GFP_KERNEL); if (!desc->inbuf) - goto err2; + goto err; usb_fill_int_urb( desc->validity, @@ -712,7 +700,6 @@ next_desc: desc, ep->bInterval ); - desc->validity->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; desc->irq->bRequestType = (USB_DIR_IN | USB_TYPE_CLASS | USB_RECIP_INTERFACE); desc->irq->bRequest = USB_CDC_GET_ENCAPSULATED_RESPONSE; @@ -731,30 +718,22 @@ next_desc: wdm_in_callback, desc ); - desc->response->transfer_flags |= URB_NO_TRANSFER_DMA_MAP; usb_set_intfdata(intf, desc); rv = usb_register_dev(intf, &wdm_class); if (rv < 0) - goto err3; + goto err2; else dev_info(&intf->dev, "cdc-wdm%d: USB WDM device\n", intf->minor - WDM_MINOR_BASE); out: return rv; -err3: - usb_set_intfdata(intf, NULL); - usb_free_coherent(interface_to_usbdev(desc->intf), - desc->bMaxPacketSize0, - desc->inbuf, - desc->response->transfer_dma); err2: - usb_free_coherent(interface_to_usbdev(desc->intf), - desc->wMaxPacketSize, - desc->sbuf, - desc->validity->transfer_dma); + usb_set_intfdata(intf, NULL); err: free_urbs(desc); + kfree(desc->inbuf); + kfree(desc->sbuf); kfree(desc->ubuf); kfree(desc->orq); kfree(desc->irq); -- cgit v1.2.3-59-g8ed1b From 8143a8963c374116f84aba15dcaeaf02370c8098 Mon Sep 17 00:00:00 2001 From: Bjørn Mork Date: Mon, 16 Jan 2012 15:12:01 +0100 Subject: USB: cdc-wdm: kill the now unnecessary bMaxPacketSize0 field and udev variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don't need bMaxPacketSize0, and keeping all these different size fields around will only cause us to use the wrong one. Seems the udev variable was only used for getting bMaxPacketSize0. We could have used it for the usb_fill_*_urb() calls, but as it wasn't before - why start now? Instead make the interface_to_usbdev() calls consistent. Signed-off-by: Bjørn Mork Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-wdm.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) (limited to 'drivers/usb/class') diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index 8909058b1bb1..bb8208a13a53 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -80,7 +80,6 @@ struct wdm_device { u16 bufsize; u16 wMaxCommand; u16 wMaxPacketSize; - u16 bMaxPacketSize0; __le16 inum; int reslength; int length; @@ -597,7 +596,6 @@ static void wdm_rxwork(struct work_struct *work) static int wdm_probe(struct usb_interface *intf, const struct usb_device_id *id) { int rv = -EINVAL; - struct usb_device *udev = interface_to_usbdev(intf); struct wdm_device *desc; struct usb_host_interface *iface; struct usb_endpoint_descriptor *ep; @@ -657,7 +655,6 @@ next_desc: goto err; desc->wMaxPacketSize = usb_endpoint_maxp(ep); - desc->bMaxPacketSize0 = udev->descriptor.bMaxPacketSize0; desc->orq = kmalloc(sizeof(struct usb_ctrlrequest), GFP_KERNEL); if (!desc->orq) @@ -709,7 +706,7 @@ next_desc: usb_fill_control_urb( desc->response, - interface_to_usbdev(desc->intf), + interface_to_usbdev(intf), /* using common endpoint 0 */ usb_rcvctrlpipe(interface_to_usbdev(desc->intf), 0), (unsigned char *)desc->irq, -- cgit v1.2.3-59-g8ed1b From 7e3054a005537f28544ab2870c375458362f7473 Mon Sep 17 00:00:00 2001 From: Bjørn Mork Date: Fri, 20 Jan 2012 01:49:57 +0100 Subject: USB: cdc-wdm: Avoid hanging on interface with no USB_CDC_DMM_TYPE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The probe does not strictly require the USB_CDC_DMM_TYPE descriptor, which is a good thing as it makes the driver usable on non-conforming interfaces. A user could e.g. bind to it to a CDC ECM interface by using the new_id and bind sysfs files. But this would fail with a 0 buffer length due to the missing descriptor. Fix by defining a reasonable fallback size: The minimum device receive buffer size required by the CDC WMC standard, revision 1.1 Signed-off-by: Bjørn Mork Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-wdm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/usb/class') diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index bb8208a13a53..c0197af22fd8 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -57,6 +57,8 @@ MODULE_DEVICE_TABLE (usb, wdm_ids); #define WDM_MAX 16 +/* CDC-WMC r1.1 requires wMaxCommand to be "at least 256 decimal (0x100)" */ +#define WDM_DEFAULT_BUFSIZE 256 static DEFINE_MUTEX(wdm_mutex); @@ -602,7 +604,7 @@ static int wdm_probe(struct usb_interface *intf, const struct usb_device_id *id) struct usb_cdc_dmm_desc *dmhd; u8 *buffer = intf->altsetting->extra; int buflen = intf->altsetting->extralen; - u16 maxcom = 0; + u16 maxcom = WDM_DEFAULT_BUFSIZE; if (!buffer) goto out; -- cgit v1.2.3-59-g8ed1b From 820c629a595ad8d8f2694641e494738b18d29e7b Mon Sep 17 00:00:00 2001 From: Bjørn Mork Date: Fri, 20 Jan 2012 04:17:25 +0100 Subject: USB: cdc-wdm: avoid printing odd-looking "cdc-wdm-176" names MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit usb_register_dev() will change our .minor_base to 0 if CONFIG_USB_DYNAMIC_MINORS is set. And it usually is, of course. Use dev_name() to print the proper interface name instead Signed-off-by: Bjørn Mork Acked-by: Oliver Neukum Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-wdm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/usb/class') diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index c0197af22fd8..c154d4f1d674 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -723,8 +723,7 @@ next_desc: if (rv < 0) goto err2; else - dev_info(&intf->dev, "cdc-wdm%d: USB WDM device\n", - intf->minor - WDM_MINOR_BASE); + dev_info(&intf->dev, "%s: USB WDM device\n", dev_name(intf->usb_dev)); out: return rv; err2: -- cgit v1.2.3-59-g8ed1b From fec67b45bf045582c3172101970090d640cd56d9 Mon Sep 17 00:00:00 2001 From: Bjørn Mork Date: Wed, 25 Jan 2012 13:03:29 +0100 Subject: usb: cdc-wdm: Add device-id for Huawei 3G/LTE modems MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit [v2: Editorial changes suggested by Sergei Shtylyov] These modems use the Qualcomm MSM Interface (QMI) protocol for management of their CDC ECM like wwan interface. This driver is perfect for exporting the protocol to userspace. The created character device will be indistinguishable from a common AT command based Device Management interface, so userspace applications must do some intelligent matching on the USB device. Cc: Sergei Shtylyov Signed-off-by: Bjørn Mork Acked-by: Oliver Neukum Acked-by: Marcel Holtmann Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-wdm.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 'drivers/usb/class') diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index c154d4f1d674..23cf9d38eb54 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -31,6 +31,8 @@ #define DRIVER_AUTHOR "Oliver Neukum" #define DRIVER_DESC "USB Abstract Control Model driver for USB WCM Device Management" +#define HUAWEI_VENDOR_ID 0x12D1 + static const struct usb_device_id wdm_ids[] = { { .match_flags = USB_DEVICE_ID_MATCH_INT_CLASS | @@ -38,6 +40,20 @@ static const struct usb_device_id wdm_ids[] = { .bInterfaceClass = USB_CLASS_COMM, .bInterfaceSubClass = USB_CDC_SUBCLASS_DMM }, + { + /* + * Huawei E392, E398 and possibly other Qualcomm based modems + * embed the Qualcomm QMI protocol inside CDC on CDC ECM like + * control interfaces. Userspace access to this is required + * to configure the accompanying data interface + */ + .match_flags = USB_DEVICE_ID_MATCH_VENDOR | + USB_DEVICE_ID_MATCH_INT_INFO, + .idVendor = HUAWEI_VENDOR_ID, + .bInterfaceClass = USB_CLASS_VENDOR_SPEC, + .bInterfaceSubClass = 1, + .bInterfaceProtocol = 9, /* NOTE: CDC ECM control interface! */ + }, { } }; -- cgit v1.2.3-59-g8ed1b From 88044202756925ad47c51c2f634a4f2c17afe068 Mon Sep 17 00:00:00 2001 From: Bjørn Mork Date: Fri, 10 Feb 2012 09:44:08 +0100 Subject: usb: cdc-wdm: make reset work with blocking IO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a flag to tell wdm_read/wdm_write that a reset is in progress, and wake any blocking read/write before taking the mutexes. This allows the device to reset without waiting for blocking IO to finish. Signed-off-by: Bjørn Mork Acked-by: Oliver Neukum Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-wdm.c | 21 +++++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) (limited to 'drivers/usb/class') diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index f63601a2054c..b27bbb957e16 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -70,6 +70,7 @@ MODULE_DEVICE_TABLE (usb, wdm_ids); #define WDM_POLL_RUNNING 6 #define WDM_RESPONDING 7 #define WDM_SUSPENDING 8 +#define WDM_RESETTING 9 #define WDM_MAX 16 @@ -340,6 +341,10 @@ static ssize_t wdm_write else if (test_bit(WDM_IN_USE, &desc->flags)) r = -EAGAIN; + + if (test_bit(WDM_RESETTING, &desc->flags)) + r = -EIO; + if (r < 0) { kfree(buf); goto out; @@ -419,6 +424,10 @@ retry: rv = -ENODEV; goto err; } + if (test_bit(WDM_RESETTING, &desc->flags)) { + rv = -EIO; + goto err; + } usb_mark_last_busy(interface_to_usbdev(desc->intf)); if (rv < 0) { rv = -ERESTARTSYS; @@ -859,10 +868,6 @@ static int wdm_pre_reset(struct usb_interface *intf) { struct wdm_device *desc = usb_get_intfdata(intf); - mutex_lock(&desc->rlock); - mutex_lock(&desc->wlock); - kill_urbs(desc); - /* * we notify everybody using poll of * an exceptional situation @@ -870,9 +875,16 @@ static int wdm_pre_reset(struct usb_interface *intf) * message from the device is lost */ spin_lock_irq(&desc->iuspin); + set_bit(WDM_RESETTING, &desc->flags); /* inform read/write */ + set_bit(WDM_READ, &desc->flags); /* unblock read */ + clear_bit(WDM_IN_USE, &desc->flags); /* unblock write */ desc->rerr = -EINTR; spin_unlock_irq(&desc->iuspin); wake_up_all(&desc->wait); + mutex_lock(&desc->rlock); + mutex_lock(&desc->wlock); + kill_urbs(desc); + cancel_work_sync(&desc->rxwork); return 0; } @@ -881,6 +893,7 @@ static int wdm_post_reset(struct usb_interface *intf) struct wdm_device *desc = usb_get_intfdata(intf); int rv; + clear_bit(WDM_RESETTING, &desc->flags); rv = recover_from_urb_loss(desc); mutex_unlock(&desc->wlock); mutex_unlock(&desc->rlock); -- cgit v1.2.3-59-g8ed1b From 711c68b3c0f7a924ffbee4aa962d8f62b85188ff Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Sun, 12 Feb 2012 06:00:41 +0000 Subject: cdc-wdm: Fix more races on the read path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We must not allow the input buffer length to change while we're shuffling the buffer contents. We also mustn't clear the WDM_READ flag after more data might have arrived. Therefore move both of these into the spinlocked region at the bottom of wdm_read(). When reading desc->length without holding the iuspin lock, use ACCESS_ONCE() to ensure the compiler doesn't re-read it with inconsistent results. Signed-off-by: Ben Hutchings Tested-by: Bjørn Mork Cc: Oliver Neukum Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-wdm.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'drivers/usb/class') diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index b27bbb957e16..7ca54d4dea92 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -391,7 +391,7 @@ outnl: static ssize_t wdm_read (struct file *file, char __user *buffer, size_t count, loff_t *ppos) { - int rv, cntr = 0; + int rv, cntr; int i = 0; struct wdm_device *desc = file->private_data; @@ -400,7 +400,8 @@ static ssize_t wdm_read if (rv < 0) return -ERESTARTSYS; - if (desc->length == 0) { + cntr = ACCESS_ONCE(desc->length); + if (cntr == 0) { desc->read = 0; retry: if (test_bit(WDM_DISCONNECTING, &desc->flags)) { @@ -455,25 +456,30 @@ retry: goto retry; } clear_bit(WDM_READ, &desc->flags); + cntr = desc->length; spin_unlock_irq(&desc->iuspin); } - cntr = count > desc->length ? desc->length : count; + if (cntr > count) + cntr = count; rv = copy_to_user(buffer, desc->ubuf, cntr); if (rv > 0) { rv = -EFAULT; goto err; } + spin_lock_irq(&desc->iuspin); + for (i = 0; i < desc->length - cntr; i++) desc->ubuf[i] = desc->ubuf[i + cntr]; - spin_lock_irq(&desc->iuspin); desc->length -= cntr; - spin_unlock_irq(&desc->iuspin); /* in case we had outstanding data */ if (!desc->length) clear_bit(WDM_READ, &desc->flags); + + spin_unlock_irq(&desc->iuspin); + rv = cntr; err: -- cgit v1.2.3-59-g8ed1b From b7a205545345578712611106b371538992e142ff Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Sun, 12 Feb 2012 06:02:43 +0000 Subject: cdc-wdm: Don't clear WDM_READ unless entire read buffer is emptied MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The WDM_READ flag is cleared later iff desc->length is reduced to 0. Signed-off-by: Ben Hutchings Tested-by: Bjørn Mork Cc: Oliver Neukum Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-wdm.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/usb/class') diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index 7ca54d4dea92..6037b503153f 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -455,7 +455,6 @@ retry: spin_unlock_irq(&desc->iuspin); goto retry; } - clear_bit(WDM_READ, &desc->flags); cntr = desc->length; spin_unlock_irq(&desc->iuspin); } -- cgit v1.2.3-59-g8ed1b From 18c75720e667719c923e0547abb60dfcd9c4ee90 Mon Sep 17 00:00:00 2001 From: Oliver Neukum Date: Fri, 17 Feb 2012 17:21:24 -0500 Subject: USB: allow users to run setserial with cdc-acm We had a user report that running setserial on /dev/ttyACM0 didn't work. He pointed at an old patch by Oliver Neukum from 2008 that never went anywhere.. http://permalink.gmane.org/gmane.linux.usb.general/9236 I made some minor changes to get it to apply again, and got the user to retest on 3.1, and he reported it worked for him. https://bugzilla.redhat.com/show_bug.cgi?id=787607 The diff below is against 3.3rc. The only difference between this and the version the user tested is the removal of the if (!ACM_READY) test Havard removed ACM_READY in 99823f457d5994b3bd3775515578c8bfacc64b04 I'm unclear if there's need for a different test in its place. From: Oliver Neukum Signed-off-by: Dave Jones Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-acm.c | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) (limited to 'drivers/usb/class') diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 9543b19d410c..6dcc3a3fe6d1 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -39,6 +39,7 @@ #include #include #include +#include #include #include #include @@ -773,10 +774,37 @@ static int acm_tty_tiocmset(struct tty_struct *tty, return acm_set_control(acm, acm->ctrlout = newctrl); } +static int get_serial_info(struct acm *acm, struct serial_struct __user *info) +{ + struct serial_struct tmp; + + if (!info) + return -EINVAL; + + memset(&tmp, 0, sizeof(tmp)); + tmp.flags = ASYNC_LOW_LATENCY; + tmp.xmit_fifo_size = acm->writesize; + tmp.baud_base = le32_to_cpu(acm->line.dwDTERate); + + if (copy_to_user(info, &tmp, sizeof(tmp))) + return -EFAULT; + else + return 0; +} + static int acm_tty_ioctl(struct tty_struct *tty, unsigned int cmd, unsigned long arg) { - return -ENOIOCTLCMD; + struct acm *acm = tty->driver_data; + int rv = -ENOIOCTLCMD; + + switch (cmd) { + case TIOCGSERIAL: /* gets serial port data */ + rv = get_serial_info(acm, (struct serial_struct __user *) arg); + break; + } + + return rv; } static const __u32 acm_tty_speed[] = { -- cgit v1.2.3-59-g8ed1b From 0dffb4862a5f109dc9b72e3a4e0ecc85a87ce397 Mon Sep 17 00:00:00 2001 From: Bjørn Mork Date: Tue, 6 Mar 2012 17:29:20 +0100 Subject: usb: cdc-wdm: split out reusable parts of probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Preparing for the addition of subdriver registering as an alternative to probe for interface-less usage. This should not change anything apart from minor code reordering. Signed-off-by: Bjørn Mork Acked-by: Oliver Neukum Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-wdm.c | 105 +++++++++++++++++++++++--------------------- 1 file changed, 54 insertions(+), 51 deletions(-) (limited to 'drivers/usb/class') diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index 6037b503153f..451793036d37 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -631,47 +631,11 @@ static void wdm_rxwork(struct work_struct *work) /* --- hotplug --- */ -static int wdm_probe(struct usb_interface *intf, const struct usb_device_id *id) +static int wdm_create(struct usb_interface *intf, struct usb_endpoint_descriptor *ep, u16 bufsize) { - int rv = -EINVAL; + int rv = -ENOMEM; struct wdm_device *desc; - struct usb_host_interface *iface; - struct usb_endpoint_descriptor *ep; - struct usb_cdc_dmm_desc *dmhd; - u8 *buffer = intf->altsetting->extra; - int buflen = intf->altsetting->extralen; - u16 maxcom = WDM_DEFAULT_BUFSIZE; - - if (!buffer) - goto out; - - while (buflen > 2) { - if (buffer [1] != USB_DT_CS_INTERFACE) { - dev_err(&intf->dev, "skipping garbage\n"); - goto next_desc; - } - - switch (buffer [2]) { - case USB_CDC_HEADER_TYPE: - break; - case USB_CDC_DMM_TYPE: - dmhd = (struct usb_cdc_dmm_desc *)buffer; - maxcom = le16_to_cpu(dmhd->wMaxCommand); - dev_dbg(&intf->dev, - "Finding maximum buffer length: %d", maxcom); - break; - default: - dev_err(&intf->dev, - "Ignoring extra header, type %d, length %d\n", - buffer[2], buffer[0]); - break; - } -next_desc: - buflen -= buffer[0]; - buffer += buffer[0]; - } - rv = -ENOMEM; desc = kzalloc(sizeof(struct wdm_device), GFP_KERNEL); if (!desc) goto out; @@ -679,18 +643,14 @@ next_desc: mutex_init(&desc->wlock); spin_lock_init(&desc->iuspin); init_waitqueue_head(&desc->wait); - desc->wMaxCommand = maxcom; + desc->wMaxCommand = bufsize; /* this will be expanded and needed in hardware endianness */ desc->inum = cpu_to_le16((u16)intf->cur_altsetting->desc.bInterfaceNumber); desc->intf = intf; INIT_WORK(&desc->rxwork, wdm_rxwork); rv = -EINVAL; - iface = intf->cur_altsetting; - if (iface->desc.bNumEndpoints != 1) - goto err; - ep = &iface->endpoint[0].desc; - if (!ep || !usb_endpoint_is_int_in(ep)) + if (!usb_endpoint_is_int_in(ep)) goto err; desc->wMaxPacketSize = usb_endpoint_maxp(ep); @@ -766,13 +726,56 @@ out: err2: usb_set_intfdata(intf, NULL); err: - free_urbs(desc); - kfree(desc->inbuf); - kfree(desc->sbuf); - kfree(desc->ubuf); - kfree(desc->orq); - kfree(desc->irq); - kfree(desc); + cleanup(desc); + return rv; +} + +static int wdm_probe(struct usb_interface *intf, const struct usb_device_id *id) +{ + int rv = -EINVAL; + struct usb_host_interface *iface; + struct usb_endpoint_descriptor *ep; + struct usb_cdc_dmm_desc *dmhd; + u8 *buffer = intf->altsetting->extra; + int buflen = intf->altsetting->extralen; + u16 maxcom = WDM_DEFAULT_BUFSIZE; + + if (!buffer) + goto err; + while (buflen > 2) { + if (buffer[1] != USB_DT_CS_INTERFACE) { + dev_err(&intf->dev, "skipping garbage\n"); + goto next_desc; + } + + switch (buffer[2]) { + case USB_CDC_HEADER_TYPE: + break; + case USB_CDC_DMM_TYPE: + dmhd = (struct usb_cdc_dmm_desc *)buffer; + maxcom = le16_to_cpu(dmhd->wMaxCommand); + dev_dbg(&intf->dev, + "Finding maximum buffer length: %d", maxcom); + break; + default: + dev_err(&intf->dev, + "Ignoring extra header, type %d, length %d\n", + buffer[2], buffer[0]); + break; + } +next_desc: + buflen -= buffer[0]; + buffer += buffer[0]; + } + + iface = intf->cur_altsetting; + if (iface->desc.bNumEndpoints != 1) + goto err; + ep = &iface->endpoint[0].desc; + + rv = wdm_create(intf, ep, maxcom); + +err: return rv; } -- cgit v1.2.3-59-g8ed1b From b0c13860808a528cd580fdca61aef9f73352a331 Mon Sep 17 00:00:00 2001 From: Bjørn Mork Date: Tue, 6 Mar 2012 17:29:21 +0100 Subject: usb: cdc-wdm: adding list lookup indirection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Register all interfaces handled by this driver in a list, getting rid of the dependency on usb_set_intfdata. This allows further generalization and simplification of the probe/create functions. This is needed to decouple wdm_open from the driver owning the interface, and it also allows us to share all the code in wdm_create with drivers unable to do usb_set_intfdata. Signed-off-by: Bjørn Mork Acked-by: Oliver Neukum Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-wdm.c | 60 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 48 insertions(+), 12 deletions(-) (limited to 'drivers/usb/class') diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index 451793036d37..46827373ecf9 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -78,6 +78,8 @@ MODULE_DEVICE_TABLE (usb, wdm_ids); #define WDM_DEFAULT_BUFSIZE 256 static DEFINE_MUTEX(wdm_mutex); +static DEFINE_SPINLOCK(wdm_device_list_lock); +static LIST_HEAD(wdm_device_list); /* --- method tables --- */ @@ -112,10 +114,39 @@ struct wdm_device { struct work_struct rxwork; int werr; int rerr; + + struct list_head device_list; }; static struct usb_driver wdm_driver; +/* return intfdata if we own the interface, else look up intf in the list */ +static struct wdm_device *wdm_find_device(struct usb_interface *intf) +{ + struct wdm_device *desc = NULL; + + spin_lock(&wdm_device_list_lock); + list_for_each_entry(desc, &wdm_device_list, device_list) + if (desc->intf == intf) + break; + spin_unlock(&wdm_device_list_lock); + + return desc; +} + +static struct wdm_device *wdm_find_device_by_minor(int minor) +{ + struct wdm_device *desc = NULL; + + spin_lock(&wdm_device_list_lock); + list_for_each_entry(desc, &wdm_device_list, device_list) + if (desc->intf->minor == minor) + break; + spin_unlock(&wdm_device_list_lock); + + return desc; +} + /* --- callbacks --- */ static void wdm_out_callback(struct urb *urb) { @@ -275,6 +306,9 @@ static void free_urbs(struct wdm_device *desc) static void cleanup(struct wdm_device *desc) { + spin_lock(&wdm_device_list_lock); + list_del(&desc->device_list); + spin_unlock(&wdm_device_list_lock); kfree(desc->sbuf); kfree(desc->inbuf); kfree(desc->orq); @@ -532,11 +566,11 @@ static int wdm_open(struct inode *inode, struct file *file) struct wdm_device *desc; mutex_lock(&wdm_mutex); - intf = usb_find_interface(&wdm_driver, minor); - if (!intf) + desc = wdm_find_device_by_minor(minor); + if (!desc) goto out; - desc = usb_get_intfdata(intf); + intf = desc->intf; if (test_bit(WDM_DISCONNECTING, &desc->flags)) goto out; file->private_data = desc; @@ -639,6 +673,7 @@ static int wdm_create(struct usb_interface *intf, struct usb_endpoint_descriptor desc = kzalloc(sizeof(struct wdm_device), GFP_KERNEL); if (!desc) goto out; + INIT_LIST_HEAD(&desc->device_list); mutex_init(&desc->rlock); mutex_init(&desc->wlock); spin_lock_init(&desc->iuspin); @@ -715,16 +750,17 @@ static int wdm_create(struct usb_interface *intf, struct usb_endpoint_descriptor desc ); - usb_set_intfdata(intf, desc); + spin_lock(&wdm_device_list_lock); + list_add(&desc->device_list, &wdm_device_list); + spin_unlock(&wdm_device_list_lock); + rv = usb_register_dev(intf, &wdm_class); if (rv < 0) - goto err2; + goto err; else dev_info(&intf->dev, "%s: USB WDM device\n", dev_name(intf->usb_dev)); out: return rv; -err2: - usb_set_intfdata(intf, NULL); err: cleanup(desc); return rv; @@ -785,8 +821,8 @@ static void wdm_disconnect(struct usb_interface *intf) unsigned long flags; usb_deregister_dev(intf, &wdm_class); + desc = wdm_find_device(intf); mutex_lock(&wdm_mutex); - desc = usb_get_intfdata(intf); /* the spinlock makes sure no new urbs are generated in the callbacks */ spin_lock_irqsave(&desc->iuspin, flags); @@ -810,7 +846,7 @@ static void wdm_disconnect(struct usb_interface *intf) #ifdef CONFIG_PM static int wdm_suspend(struct usb_interface *intf, pm_message_t message) { - struct wdm_device *desc = usb_get_intfdata(intf); + struct wdm_device *desc = wdm_find_device(intf); int rv = 0; dev_dbg(&desc->intf->dev, "wdm%d_suspend\n", intf->minor); @@ -860,7 +896,7 @@ static int recover_from_urb_loss(struct wdm_device *desc) #ifdef CONFIG_PM static int wdm_resume(struct usb_interface *intf) { - struct wdm_device *desc = usb_get_intfdata(intf); + struct wdm_device *desc = wdm_find_device(intf); int rv; dev_dbg(&desc->intf->dev, "wdm%d_resume\n", intf->minor); @@ -874,7 +910,7 @@ static int wdm_resume(struct usb_interface *intf) static int wdm_pre_reset(struct usb_interface *intf) { - struct wdm_device *desc = usb_get_intfdata(intf); + struct wdm_device *desc = wdm_find_device(intf); /* * we notify everybody using poll of @@ -898,7 +934,7 @@ static int wdm_pre_reset(struct usb_interface *intf) static int wdm_post_reset(struct usb_interface *intf) { - struct wdm_device *desc = usb_get_intfdata(intf); + struct wdm_device *desc = wdm_find_device(intf); int rv; clear_bit(WDM_RESETTING, &desc->flags); -- cgit v1.2.3-59-g8ed1b From 3cc3615749dbd1b891512d5c9a5bf4559cfa9741 Mon Sep 17 00:00:00 2001 From: Bjørn Mork Date: Tue, 6 Mar 2012 17:29:22 +0100 Subject: usb: cdc-wdm: adding usb_cdc_wdm_register subdriver support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This driver can be used as a subdriver of another USB driver, allowing it to export a Device Managment interface consisting of a single interrupt endpoint with no dedicated USB interface. Some devices provide a Device Management function combined with a wwan function in a single USB interface having three endpoints (bulk in/out + interrupt). If the interrupt endpoint is used exclusively for DM notifications, then this driver can support that as a subdriver provided that the wwan driver calls the appropriate entry points on probe, suspend, resume, pre_reset, post_reset and disconnect. The main driver must have full control over all interface related settings, including the needs_remote_wakeup flag. A manage_power function must be provided by the main driver. A manage_power stub doing direct flag manipulation is used in normal driver mode. Signed-off-by: Bjørn Mork Acked-by: Oliver Neukum Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-wdm.c | 63 ++++++++++++++++++++++++++++++++++++++++++--- include/linux/usb/cdc-wdm.h | 19 ++++++++++++++ 2 files changed, 78 insertions(+), 4 deletions(-) create mode 100644 include/linux/usb/cdc-wdm.h (limited to 'drivers/usb/class') diff --git a/drivers/usb/class/cdc-wdm.c b/drivers/usb/class/cdc-wdm.c index 46827373ecf9..c6f6560d436c 100644 --- a/drivers/usb/class/cdc-wdm.c +++ b/drivers/usb/class/cdc-wdm.c @@ -23,6 +23,7 @@ #include #include #include +#include /* * Version Information @@ -116,6 +117,7 @@ struct wdm_device { int rerr; struct list_head device_list; + int (*manage_power)(struct usb_interface *, int); }; static struct usb_driver wdm_driver; @@ -580,7 +582,6 @@ static int wdm_open(struct inode *inode, struct file *file) dev_err(&desc->intf->dev, "Error autopm - %d\n", rv); goto out; } - intf->needs_remote_wakeup = 1; /* using write lock to protect desc->count */ mutex_lock(&desc->wlock); @@ -597,6 +598,8 @@ static int wdm_open(struct inode *inode, struct file *file) rv = 0; } mutex_unlock(&desc->wlock); + if (desc->count == 1) + desc->manage_power(intf, 1); usb_autopm_put_interface(desc->intf); out: mutex_unlock(&wdm_mutex); @@ -618,7 +621,7 @@ static int wdm_release(struct inode *inode, struct file *file) dev_dbg(&desc->intf->dev, "wdm_release: cleanup"); kill_urbs(desc); if (!test_bit(WDM_DISCONNECTING, &desc->flags)) - desc->intf->needs_remote_wakeup = 0; + desc->manage_power(desc->intf, 0); } mutex_unlock(&wdm_mutex); return 0; @@ -665,7 +668,8 @@ static void wdm_rxwork(struct work_struct *work) /* --- hotplug --- */ -static int wdm_create(struct usb_interface *intf, struct usb_endpoint_descriptor *ep, u16 bufsize) +static int wdm_create(struct usb_interface *intf, struct usb_endpoint_descriptor *ep, + u16 bufsize, int (*manage_power)(struct usb_interface *, int)) { int rv = -ENOMEM; struct wdm_device *desc; @@ -750,6 +754,8 @@ static int wdm_create(struct usb_interface *intf, struct usb_endpoint_descriptor desc ); + desc->manage_power = manage_power; + spin_lock(&wdm_device_list_lock); list_add(&desc->device_list, &wdm_device_list); spin_unlock(&wdm_device_list_lock); @@ -766,6 +772,19 @@ err: return rv; } +static int wdm_manage_power(struct usb_interface *intf, int on) +{ + /* need autopm_get/put here to ensure the usbcore sees the new value */ + int rv = usb_autopm_get_interface(intf); + if (rv < 0) + goto err; + + intf->needs_remote_wakeup = on; + usb_autopm_put_interface(intf); +err: + return rv; +} + static int wdm_probe(struct usb_interface *intf, const struct usb_device_id *id) { int rv = -EINVAL; @@ -809,12 +828,48 @@ next_desc: goto err; ep = &iface->endpoint[0].desc; - rv = wdm_create(intf, ep, maxcom); + rv = wdm_create(intf, ep, maxcom, &wdm_manage_power); err: return rv; } +/** + * usb_cdc_wdm_register - register a WDM subdriver + * @intf: usb interface the subdriver will associate with + * @ep: interrupt endpoint to monitor for notifications + * @bufsize: maximum message size to support for read/write + * + * Create WDM usb class character device and associate it with intf + * without binding, allowing another driver to manage the interface. + * + * The subdriver will manage the given interrupt endpoint exclusively + * and will issue control requests referring to the given intf. It + * will otherwise avoid interferring, and in particular not do + * usb_set_intfdata/usb_get_intfdata on intf. + * + * The return value is a pointer to the subdriver's struct usb_driver. + * The registering driver is responsible for calling this subdriver's + * disconnect, suspend, resume, pre_reset and post_reset methods from + * its own. + */ +struct usb_driver *usb_cdc_wdm_register(struct usb_interface *intf, + struct usb_endpoint_descriptor *ep, + int bufsize, + int (*manage_power)(struct usb_interface *, int)) +{ + int rv = -EINVAL; + + rv = wdm_create(intf, ep, bufsize, manage_power); + if (rv < 0) + goto err; + + return &wdm_driver; +err: + return ERR_PTR(rv); +} +EXPORT_SYMBOL(usb_cdc_wdm_register); + static void wdm_disconnect(struct usb_interface *intf) { struct wdm_device *desc; diff --git a/include/linux/usb/cdc-wdm.h b/include/linux/usb/cdc-wdm.h new file mode 100644 index 000000000000..719c332620fa --- /dev/null +++ b/include/linux/usb/cdc-wdm.h @@ -0,0 +1,19 @@ +/* + * USB CDC Device Management subdriver + * + * Copyright (c) 2012 Bjørn Mork + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public License + * version 2 as published by the Free Software Foundation. + */ + +#ifndef __LINUX_USB_CDC_WDM_H +#define __LINUX_USB_CDC_WDM_H + +extern struct usb_driver *usb_cdc_wdm_register(struct usb_interface *intf, + struct usb_endpoint_descriptor *ep, + int bufsize, + int (*manage_power)(struct usb_interface *, int)); + +#endif /* __LINUX_USB_CDC_WDM_H */ -- cgit v1.2.3-59-g8ed1b