From 5a345c20c17d87099224a4be12e69e5bd7023dca Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 26 May 2014 19:23:36 +0200 Subject: USB: cdc-acm: fix write and suspend race Fix race between write() and suspend() which could lead to writes being dropped (or I/O while suspended) if the device is runtime suspended while a write request is being processed. Specifically, suspend() releases the write_lock after determining the device is idle but before incrementing the susp_count, thus leaving a window where a concurrent write() can submit an urb. Fixes: 11ea859d64b6 ("USB: additional power savings for cdc-acm devices that support remote wakeup") Cc: # v2.6.27 Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-acm.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) (limited to 'drivers/usb/class/cdc-acm.c') diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 904efb6035b..3bd4226c13d 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -1514,18 +1514,15 @@ static int acm_suspend(struct usb_interface *intf, pm_message_t message) struct acm *acm = usb_get_intfdata(intf); int cnt; + spin_lock_irq(&acm->read_lock); + spin_lock(&acm->write_lock); if (PMSG_IS_AUTO(message)) { - int b; - - spin_lock_irq(&acm->write_lock); - b = acm->transmitting; - spin_unlock_irq(&acm->write_lock); - if (b) + if (acm->transmitting) { + spin_unlock(&acm->write_lock); + spin_unlock_irq(&acm->read_lock); return -EBUSY; + } } - - spin_lock_irq(&acm->read_lock); - spin_lock(&acm->write_lock); cnt = acm->susp_count++; spin_unlock(&acm->write_lock); spin_unlock_irq(&acm->read_lock); -- cgit v1.2.3-70-g09d2 From e144ed28bed10684f9aaec6325ed974d53f76110 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 26 May 2014 19:23:37 +0200 Subject: USB: cdc-acm: fix write and resume race Fix race between write() and resume() due to improper locking that could lead to writes being reordered. Resume must be done atomically and susp_count be protected by the write_lock in order to prevent racing with write(). This could otherwise lead to writes being reordered if write() grabs the write_lock after susp_count is decremented, but before the delayed urb is submitted. Fixes: 11ea859d64b6 ("USB: additional power savings for cdc-acm devices that support remote wakeup") Cc: # v2.6.27 Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-acm.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) (limited to 'drivers/usb/class/cdc-acm.c') diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 3bd4226c13d..e72a657a617 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -1541,27 +1541,20 @@ static int acm_resume(struct usb_interface *intf) struct acm *acm = usb_get_intfdata(intf); struct acm_wb *wb; int rv = 0; - int cnt; spin_lock_irq(&acm->read_lock); - acm->susp_count -= 1; - cnt = acm->susp_count; - spin_unlock_irq(&acm->read_lock); + spin_lock(&acm->write_lock); - if (cnt) - return 0; + if (--acm->susp_count) + goto out; if (test_bit(ASYNCB_INITIALIZED, &acm->port.flags)) { - rv = usb_submit_urb(acm->ctrlurb, GFP_NOIO); + rv = usb_submit_urb(acm->ctrlurb, GFP_ATOMIC); - spin_lock_irq(&acm->write_lock); if (acm->delayed_wb) { wb = acm->delayed_wb; acm->delayed_wb = NULL; - spin_unlock_irq(&acm->write_lock); acm_start_wb(acm, wb); - } else { - spin_unlock_irq(&acm->write_lock); } /* @@ -1569,12 +1562,14 @@ static int acm_resume(struct usb_interface *intf) * do the write path at all cost */ if (rv < 0) - goto err_out; + goto out; - rv = acm_submit_read_urbs(acm, GFP_NOIO); + rv = acm_submit_read_urbs(acm, GFP_ATOMIC); } +out: + spin_unlock(&acm->write_lock); + spin_unlock_irq(&acm->read_lock); -err_out: return rv; } -- cgit v1.2.3-70-g09d2 From 140cb81ac8c625942a1d695875932c615767a526 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 26 May 2014 19:23:38 +0200 Subject: USB: cdc-acm: fix broken runtime suspend The current ACM runtime-suspend implementation is broken in several ways: Firstly, it buffers only the first write request being made while suspended -- any further writes are silently dropped. Secondly, writes being dropped also leak write urbs, which are never reclaimed (until the device is unbound). Thirdly, even the single buffered write is not cleared at shutdown (which may happen before the device is resumed), something which can lead to another urb leak as well as a PM usage-counter leak. Fix this by implementing a delayed-write queue using urb anchors and making sure to discard the queue properly at shutdown. Fixes: 11ea859d64b6 ("USB: additional power savings for cdc-acm devices that support remote wakeup") Reported-by: Xiao Jin Cc: # v2.6.27 Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-acm.c | 32 ++++++++++++++++++++++---------- drivers/usb/class/cdc-acm.h | 2 +- 2 files changed, 23 insertions(+), 11 deletions(-) (limited to 'drivers/usb/class/cdc-acm.c') diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index e72a657a617..56419254ef7 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -573,6 +573,8 @@ static void acm_port_destruct(struct tty_port *port) static void acm_port_shutdown(struct tty_port *port) { struct acm *acm = container_of(port, struct acm, port); + struct urb *urb; + struct acm_wb *wb; int i; dev_dbg(&acm->control->dev, "%s\n", __func__); @@ -581,6 +583,16 @@ static void acm_port_shutdown(struct tty_port *port) if (!acm->disconnected) { usb_autopm_get_interface(acm->control); acm_set_control(acm, acm->ctrlout = 0); + + for (;;) { + urb = usb_get_from_anchor(&acm->delayed); + if (!urb) + break; + wb = urb->context; + wb->use = 0; + usb_autopm_put_interface_async(acm->control); + } + usb_kill_urb(acm->ctrlurb); for (i = 0; i < ACM_NW; i++) usb_kill_urb(acm->wb[i].urb); @@ -648,12 +660,9 @@ static int acm_tty_write(struct tty_struct *tty, usb_autopm_get_interface_async(acm->control); if (acm->susp_count) { - if (!acm->delayed_wb) - acm->delayed_wb = wb; - else - usb_autopm_put_interface_async(acm->control); + usb_anchor_urb(wb->urb, &acm->delayed); spin_unlock_irqrestore(&acm->write_lock, flags); - return count; /* A white lie */ + return count; } usb_mark_last_busy(acm->dev); @@ -1269,6 +1278,7 @@ made_compressed_probe: acm->bInterval = epread->bInterval; tty_port_init(&acm->port); acm->port.ops = &acm_port_ops; + init_usb_anchor(&acm->delayed); buf = usb_alloc_coherent(usb_dev, ctrlsize, GFP_KERNEL, &acm->ctrl_dma); if (!buf) { @@ -1539,7 +1549,7 @@ static int acm_suspend(struct usb_interface *intf, pm_message_t message) static int acm_resume(struct usb_interface *intf) { struct acm *acm = usb_get_intfdata(intf); - struct acm_wb *wb; + struct urb *urb; int rv = 0; spin_lock_irq(&acm->read_lock); @@ -1551,10 +1561,12 @@ static int acm_resume(struct usb_interface *intf) if (test_bit(ASYNCB_INITIALIZED, &acm->port.flags)) { rv = usb_submit_urb(acm->ctrlurb, GFP_ATOMIC); - if (acm->delayed_wb) { - wb = acm->delayed_wb; - acm->delayed_wb = NULL; - acm_start_wb(acm, wb); + for (;;) { + urb = usb_get_from_anchor(&acm->delayed); + if (!urb) + break; + + acm_start_wb(acm, urb->context); } /* diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h index e38dc785808..80826f843e0 100644 --- a/drivers/usb/class/cdc-acm.h +++ b/drivers/usb/class/cdc-acm.h @@ -120,7 +120,7 @@ struct acm { unsigned int throttled:1; /* actually throttled */ unsigned int throttle_req:1; /* throttle requested */ u8 bInterval; - struct acm_wb *delayed_wb; /* write queued for a device about to be woken */ + struct usb_anchor delayed; /* writes queued for a device about to be woken */ }; #define CDC_DATA_INTERFACE_TYPE 0x0a -- cgit v1.2.3-70-g09d2 From bae3f4c53585e9a170da9436e0f06919874bda9a Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 26 May 2014 19:23:39 +0200 Subject: USB: cdc-acm: fix runtime PM for control messages Fix runtime PM handling of control messages by adding the required PM counter operations. Fixes: 11ea859d64b6 ("USB: additional power savings for cdc-acm devices that support remote wakeup") Cc: # v2.6.27 Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-acm.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'drivers/usb/class/cdc-acm.c') diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 56419254ef7..2258827df08 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -122,13 +122,23 @@ static void acm_release_minor(struct acm *acm) static int acm_ctrl_msg(struct acm *acm, int request, int value, void *buf, int len) { - int retval = usb_control_msg(acm->dev, usb_sndctrlpipe(acm->dev, 0), + int retval; + + retval = usb_autopm_get_interface(acm->control); + if (retval) + return retval; + + retval = usb_control_msg(acm->dev, usb_sndctrlpipe(acm->dev, 0), request, USB_RT_ACM, value, acm->control->altsetting[0].desc.bInterfaceNumber, buf, len, 5000); + dev_dbg(&acm->control->dev, "%s - rq 0x%02x, val %#x, len %#x, result %d\n", __func__, request, value, len, retval); + + usb_autopm_put_interface(acm->control); + return retval < 0 ? retval : 0; } -- cgit v1.2.3-70-g09d2 From ed797074031a37bb9bf4a70952fffc606b77274d Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 26 May 2014 19:23:40 +0200 Subject: USB: cdc-acm: fix shutdown and suspend race We should stop I/O unconditionally at suspend rather than rely on the tty-port initialised flag (which is set prior to stopping I/O during shutdown) in order to prevent suspend returning with URBs still active. Fixes: 11ea859d64b6 ("USB: additional power savings for cdc-acm devices that support remote wakeup") Cc: # v2.6.27 Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-acm.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/usb/class/cdc-acm.c') diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 2258827df08..1ac6c5dda9f 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -1550,8 +1550,7 @@ static int acm_suspend(struct usb_interface *intf, pm_message_t message) if (cnt) return 0; - if (test_bit(ASYNCB_INITIALIZED, &acm->port.flags)) - stop_data_traffic(acm); + stop_data_traffic(acm); return 0; } -- cgit v1.2.3-70-g09d2 From 183a45087d126d126e8dd1d9b2602fc129dff9ad Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 26 May 2014 19:23:41 +0200 Subject: USB: cdc-acm: fix potential urb leak and PM imbalance in write Make sure to check return value of autopm get in write() in order to avoid urb leak and PM counter imbalance on errors. Fixes: 11ea859d64b6 ("USB: additional power savings for cdc-acm devices that support remote wakeup") Cc: # v2.6.27 Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-acm.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'drivers/usb/class/cdc-acm.c') diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 1ac6c5dda9f..c255e77282a 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -668,7 +668,13 @@ static int acm_tty_write(struct tty_struct *tty, memcpy(wb->buf, buf, count); wb->len = count; - usb_autopm_get_interface_async(acm->control); + stat = usb_autopm_get_interface_async(acm->control); + if (stat) { + wb->use = 0; + spin_unlock_irqrestore(&acm->write_lock, flags); + return stat; + } + if (acm->susp_count) { usb_anchor_urb(wb->urb, &acm->delayed); spin_unlock_irqrestore(&acm->write_lock, flags); -- cgit v1.2.3-70-g09d2 From 703df3297fb1950b0aa53e656108eb936d3f21d9 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 26 May 2014 19:23:42 +0200 Subject: USB: cdc-acm: fix open and suspend race We must not do the usb_autopm_put_interface() before submitting the read urbs or we might end up doing I/O to a suspended device. Fixes: 088c64f81284 ("USB: cdc-acm: re-write read processing") Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-acm.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'drivers/usb/class/cdc-acm.c') diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index c255e77282a..74df311c850 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -528,19 +528,15 @@ static int acm_port_activate(struct tty_port *port, struct tty_struct *tty) if (usb_submit_urb(acm->ctrlurb, GFP_KERNEL)) { dev_err(&acm->control->dev, "%s - usb_submit_urb(ctrl irq) failed\n", __func__); - usb_autopm_put_interface(acm->control); goto error_submit_urb; } acm->ctrlout = ACM_CTRL_DTR | ACM_CTRL_RTS; if (acm_set_control(acm, acm->ctrlout) < 0 && (acm->ctrl_caps & USB_CDC_CAP_LINE)) { - usb_autopm_put_interface(acm->control); goto error_set_control; } - usb_autopm_put_interface(acm->control); - /* * Unthrottle device in case the TTY was closed while throttled. */ @@ -552,6 +548,8 @@ static int acm_port_activate(struct tty_port *port, struct tty_struct *tty) if (acm_submit_read_urbs(acm, GFP_KERNEL)) goto error_submit_read_urbs; + usb_autopm_put_interface(acm->control); + mutex_unlock(&acm->mutex); return 0; @@ -562,6 +560,7 @@ error_submit_read_urbs: error_set_control: usb_kill_urb(acm->ctrlurb); error_submit_urb: + usb_autopm_put_interface(acm->control); error_get_interface: disconnected: mutex_unlock(&acm->mutex); -- cgit v1.2.3-70-g09d2 From 8727bf689a77a79816065e23a7a58a474ad544f9 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 26 May 2014 19:23:43 +0200 Subject: USB: cdc-acm: fix failed open not being detected Fix errors during open not being returned to userspace. Specifically, failed control-line manipulations or control or read urb submissions would not be detected. Fixes: 7fb57a019f94 ("USB: cdc-acm: Fix potential deadlock (lockdep warning)") Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-acm.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'drivers/usb/class/cdc-acm.c') diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 74df311c850..6c6928a154b 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -525,17 +525,17 @@ static int acm_port_activate(struct tty_port *port, struct tty_struct *tty) acm->control->needs_remote_wakeup = 1; acm->ctrlurb->dev = acm->dev; - if (usb_submit_urb(acm->ctrlurb, GFP_KERNEL)) { + retval = usb_submit_urb(acm->ctrlurb, GFP_KERNEL); + if (retval) { dev_err(&acm->control->dev, "%s - usb_submit_urb(ctrl irq) failed\n", __func__); goto error_submit_urb; } acm->ctrlout = ACM_CTRL_DTR | ACM_CTRL_RTS; - if (acm_set_control(acm, acm->ctrlout) < 0 && - (acm->ctrl_caps & USB_CDC_CAP_LINE)) { + retval = acm_set_control(acm, acm->ctrlout); + if (retval < 0 && (acm->ctrl_caps & USB_CDC_CAP_LINE)) goto error_set_control; - } /* * Unthrottle device in case the TTY was closed while throttled. @@ -545,7 +545,8 @@ static int acm_port_activate(struct tty_port *port, struct tty_struct *tty) acm->throttle_req = 0; spin_unlock_irq(&acm->read_lock); - if (acm_submit_read_urbs(acm, GFP_KERNEL)) + retval = acm_submit_read_urbs(acm, GFP_KERNEL); + if (retval) goto error_submit_read_urbs; usb_autopm_put_interface(acm->control); @@ -564,7 +565,8 @@ error_submit_urb: error_get_interface: disconnected: mutex_unlock(&acm->mutex); - return retval; + + return usb_translate_errors(retval); } static void acm_port_destruct(struct tty_port *port) -- cgit v1.2.3-70-g09d2 From e4c36076c2a6195ec62c35b03c3fde84d0087dc8 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 26 May 2014 19:23:44 +0200 Subject: USB: cdc-acm: fix I/O after failed open Make sure to kill any already submitted read urbs on read-urb submission failures in open in order to prevent doing I/O for a closed port. Fixes: 088c64f81284 ("USB: cdc-acm: re-write read processing") Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-acm.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'drivers/usb/class/cdc-acm.c') diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 6c6928a154b..eddeba61a88 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -506,6 +506,7 @@ static int acm_port_activate(struct tty_port *port, struct tty_struct *tty) { struct acm *acm = container_of(port, struct acm, port); int retval = -ENODEV; + int i; dev_dbg(&acm->control->dev, "%s\n", __func__); @@ -556,6 +557,8 @@ static int acm_port_activate(struct tty_port *port, struct tty_struct *tty) return 0; error_submit_read_urbs: + for (i = 0; i < acm->rx_buflimit; i++) + usb_kill_urb(acm->read_urbs[i]); acm->ctrlout = 0; acm_set_control(acm, acm->ctrlout); error_set_control: -- cgit v1.2.3-70-g09d2 From 5292afa657d0e790b7479ad8eef9450c1e040b3d Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 26 May 2014 19:23:45 +0200 Subject: USB: cdc-acm: fix runtime PM imbalance at shutdown Make sure only to decrement the PM counters if they were actually incremented. Note that the USB PM counter, but not necessarily the driver core PM counter, is reset when the interface is unbound. Fixes: 11ea859d64b6 ("USB: additional power savings for cdc-acm devices that support remote wakeup") Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-acm.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'drivers/usb/class/cdc-acm.c') diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index eddeba61a88..6bbd203f186 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -590,12 +590,13 @@ static void acm_port_shutdown(struct tty_port *port) struct urb *urb; struct acm_wb *wb; int i; + int pm_err; dev_dbg(&acm->control->dev, "%s\n", __func__); mutex_lock(&acm->mutex); if (!acm->disconnected) { - usb_autopm_get_interface(acm->control); + pm_err = usb_autopm_get_interface(acm->control); acm_set_control(acm, acm->ctrlout = 0); for (;;) { @@ -613,7 +614,8 @@ static void acm_port_shutdown(struct tty_port *port) for (i = 0; i < acm->rx_buflimit; i++) usb_kill_urb(acm->read_urbs[i]); acm->control->needs_remote_wakeup = 0; - usb_autopm_put_interface(acm->control); + if (!pm_err) + usb_autopm_put_interface(acm->control); } mutex_unlock(&acm->mutex); } -- cgit v1.2.3-70-g09d2 From bbf0cb3e93a1b6ef8bf22a67f35d7c98ef378f2b Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 26 May 2014 19:23:46 +0200 Subject: USB: cdc-acm: simplify runtime PM locking We can simply the runtime PM locking as there's no need to check the susp_count in the read path (at least not since killing the rx tasklet). Specifically, the read urbs will never be resubmitted by the completion handler when killed during suspend. Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-acm.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) (limited to 'drivers/usb/class/cdc-acm.c') diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 6bbd203f186..bc7a2a6fc4a 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -428,7 +428,7 @@ static void acm_read_bulk_callback(struct urb *urb) /* throttle device if requested by tty */ spin_lock_irqsave(&acm->read_lock, flags); acm->throttled = acm->throttle_req; - if (!acm->throttled && !acm->susp_count) { + if (!acm->throttled) { spin_unlock_irqrestore(&acm->read_lock, flags); acm_submit_read_urb(acm, rb->index, GFP_ATOMIC); } else { @@ -1546,18 +1546,15 @@ static int acm_suspend(struct usb_interface *intf, pm_message_t message) struct acm *acm = usb_get_intfdata(intf); int cnt; - spin_lock_irq(&acm->read_lock); - spin_lock(&acm->write_lock); + spin_lock_irq(&acm->write_lock); if (PMSG_IS_AUTO(message)) { if (acm->transmitting) { - spin_unlock(&acm->write_lock); - spin_unlock_irq(&acm->read_lock); + spin_unlock_irq(&acm->write_lock); return -EBUSY; } } cnt = acm->susp_count++; - spin_unlock(&acm->write_lock); - spin_unlock_irq(&acm->read_lock); + spin_unlock_irq(&acm->write_lock); if (cnt) return 0; @@ -1573,8 +1570,7 @@ static int acm_resume(struct usb_interface *intf) struct urb *urb; int rv = 0; - spin_lock_irq(&acm->read_lock); - spin_lock(&acm->write_lock); + spin_lock_irq(&acm->write_lock); if (--acm->susp_count) goto out; @@ -1600,8 +1596,7 @@ static int acm_resume(struct usb_interface *intf) rv = acm_submit_read_urbs(acm, GFP_ATOMIC); } out: - spin_unlock(&acm->write_lock); - spin_unlock_irq(&acm->read_lock); + spin_unlock_irq(&acm->write_lock); return rv; } -- cgit v1.2.3-70-g09d2 From 89e54e4468338df5a4ab7627c5b8b10786ee43e8 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 26 May 2014 19:23:47 +0200 Subject: USB: cdc-acm: remove redundant disconnected test from shutdown Remove redundant disconnect test from shutdown(), which is never called post disconnect() where we do synchronous hangup. Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-acm.c | 41 +++++++++++++++++++---------------------- 1 file changed, 19 insertions(+), 22 deletions(-) (limited to 'drivers/usb/class/cdc-acm.c') diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index bc7a2a6fc4a..91fdc293196 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -594,30 +594,27 @@ static void acm_port_shutdown(struct tty_port *port) dev_dbg(&acm->control->dev, "%s\n", __func__); - mutex_lock(&acm->mutex); - if (!acm->disconnected) { - pm_err = usb_autopm_get_interface(acm->control); - acm_set_control(acm, acm->ctrlout = 0); - - for (;;) { - urb = usb_get_from_anchor(&acm->delayed); - if (!urb) - break; - wb = urb->context; - wb->use = 0; - usb_autopm_put_interface_async(acm->control); - } + pm_err = usb_autopm_get_interface(acm->control); + acm_set_control(acm, acm->ctrlout = 0); - usb_kill_urb(acm->ctrlurb); - for (i = 0; i < ACM_NW; i++) - usb_kill_urb(acm->wb[i].urb); - for (i = 0; i < acm->rx_buflimit; i++) - usb_kill_urb(acm->read_urbs[i]); - acm->control->needs_remote_wakeup = 0; - if (!pm_err) - usb_autopm_put_interface(acm->control); + for (;;) { + urb = usb_get_from_anchor(&acm->delayed); + if (!urb) + break; + wb = urb->context; + wb->use = 0; + usb_autopm_put_interface_async(acm->control); } - mutex_unlock(&acm->mutex); + + usb_kill_urb(acm->ctrlurb); + for (i = 0; i < ACM_NW; i++) + usb_kill_urb(acm->wb[i].urb); + for (i = 0; i < acm->rx_buflimit; i++) + usb_kill_urb(acm->read_urbs[i]); + + acm->control->needs_remote_wakeup = 0; + if (!pm_err) + usb_autopm_put_interface(acm->control); } static void acm_tty_cleanup(struct tty_struct *tty) -- cgit v1.2.3-70-g09d2 From b1d42efc217fdc1a6a704b344fd902ae52a012c8 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 26 May 2014 19:23:48 +0200 Subject: USB: cdc-acm: minimise no-suspend window during shutdown Now that acm_set_control() handles runtime PM properly, the only remaining reason for the PM operations in shutdown is to clear the needs_remote_wakeup flag before the final put. Note that this also means that we now need to grab the write_lock to prevent racing with resume. Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-acm.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) (limited to 'drivers/usb/class/cdc-acm.c') diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 91fdc293196..f038f390db9 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -590,13 +590,22 @@ static void acm_port_shutdown(struct tty_port *port) struct urb *urb; struct acm_wb *wb; int i; - int pm_err; dev_dbg(&acm->control->dev, "%s\n", __func__); - pm_err = usb_autopm_get_interface(acm->control); acm_set_control(acm, acm->ctrlout = 0); + /* + * Need to grab write_lock to prevent race with resume, but no need to + * hold it due to the tty-port initialised flag. + */ + spin_lock_irq(&acm->write_lock); + spin_unlock_irq(&acm->write_lock); + + usb_autopm_get_interface_no_resume(acm->control); + acm->control->needs_remote_wakeup = 0; + usb_autopm_put_interface(acm->control); + for (;;) { urb = usb_get_from_anchor(&acm->delayed); if (!urb) @@ -611,10 +620,6 @@ static void acm_port_shutdown(struct tty_port *port) usb_kill_urb(acm->wb[i].urb); for (i = 0; i < acm->rx_buflimit; i++) usb_kill_urb(acm->read_urbs[i]); - - acm->control->needs_remote_wakeup = 0; - if (!pm_err) - usb_autopm_put_interface(acm->control); } static void acm_tty_cleanup(struct tty_struct *tty) -- cgit v1.2.3-70-g09d2 From 4a8ee5059a241114c644350b6cb564c729a340fa Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 26 May 2014 19:23:49 +0200 Subject: USB: cdc-acm: do not update PM busy on read errors There's no need to update the runtime PM last_busy field on read urb errors (e.g. when the urb is being killed on shutdown). Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-acm.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/usb/class/cdc-acm.c') diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index f038f390db9..3c7cfac48e3 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -416,13 +416,15 @@ static void acm_read_bulk_callback(struct urb *urb) dev_dbg(&acm->data->dev, "%s - disconnected\n", __func__); return; } - usb_mark_last_busy(acm->dev); if (urb->status) { dev_dbg(&acm->data->dev, "%s - non-zero urb status: %d\n", __func__, urb->status); return; } + + usb_mark_last_busy(acm->dev); + acm_process_read_urb(acm, urb); /* throttle device if requested by tty */ -- cgit v1.2.3-70-g09d2 From 308fee18e0f06215b47b54a2b254bfaf55527bdd Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 26 May 2014 19:23:50 +0200 Subject: USB: cdc-acm: remove redundant usb_mark_last_busy There's no need to call usb_mark_last_busy after having increased the PM counter in write(). The device will be marked busy by USB core when the PM counter is balanced in the completion handler. Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-acm.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers/usb/class/cdc-acm.c') diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 3c7cfac48e3..8f654ce5257 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -690,7 +690,6 @@ static int acm_tty_write(struct tty_struct *tty, spin_unlock_irqrestore(&acm->write_lock, flags); return count; } - usb_mark_last_busy(acm->dev); stat = acm_start_wb(acm, wb); spin_unlock_irqrestore(&acm->write_lock, flags); -- cgit v1.2.3-70-g09d2 From 0943d8ead30e9474034cc5e92225ab0fd29fd0d4 Mon Sep 17 00:00:00 2001 From: Johan Hovold Date: Mon, 26 May 2014 19:23:51 +0200 Subject: USB: cdc-acm: use tty-port dtr_rts Add dtr_rts tty-port operation which implements proper DTR/RTS handling (e.g. only lower DTR/RTS during shutdown if HUPCL is set). Note that modem-control locking still needs to be added throughout the driver. Signed-off-by: Johan Hovold Signed-off-by: Greg Kroah-Hartman --- drivers/usb/class/cdc-acm.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) (limited to 'drivers/usb/class/cdc-acm.c') diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c index 8f654ce5257..e934e19f49f 100644 --- a/drivers/usb/class/cdc-acm.c +++ b/drivers/usb/class/cdc-acm.c @@ -504,6 +504,25 @@ static int acm_tty_open(struct tty_struct *tty, struct file *filp) return tty_port_open(&acm->port, tty, filp); } +static void acm_port_dtr_rts(struct tty_port *port, int raise) +{ + struct acm *acm = container_of(port, struct acm, port); + int val; + int res; + + if (raise) + val = ACM_CTRL_DTR | ACM_CTRL_RTS; + else + val = 0; + + /* FIXME: add missing ctrlout locking throughout driver */ + acm->ctrlout = val; + + res = acm_set_control(acm, val); + if (res && (acm->ctrl_caps & USB_CDC_CAP_LINE)) + dev_err(&acm->control->dev, "failed to set dtr/rts\n"); +} + static int acm_port_activate(struct tty_port *port, struct tty_struct *tty) { struct acm *acm = container_of(port, struct acm, port); @@ -535,11 +554,6 @@ static int acm_port_activate(struct tty_port *port, struct tty_struct *tty) goto error_submit_urb; } - acm->ctrlout = ACM_CTRL_DTR | ACM_CTRL_RTS; - retval = acm_set_control(acm, acm->ctrlout); - if (retval < 0 && (acm->ctrl_caps & USB_CDC_CAP_LINE)) - goto error_set_control; - /* * Unthrottle device in case the TTY was closed while throttled. */ @@ -561,9 +575,6 @@ static int acm_port_activate(struct tty_port *port, struct tty_struct *tty) error_submit_read_urbs: for (i = 0; i < acm->rx_buflimit; i++) usb_kill_urb(acm->read_urbs[i]); - acm->ctrlout = 0; - acm_set_control(acm, acm->ctrlout); -error_set_control: usb_kill_urb(acm->ctrlurb); error_submit_urb: usb_autopm_put_interface(acm->control); @@ -595,8 +606,6 @@ static void acm_port_shutdown(struct tty_port *port) dev_dbg(&acm->control->dev, "%s\n", __func__); - acm_set_control(acm, acm->ctrlout = 0); - /* * Need to grab write_lock to prevent race with resume, but no need to * hold it due to the tty-port initialised flag. @@ -992,6 +1001,7 @@ static void acm_tty_set_termios(struct tty_struct *tty, } static const struct tty_port_operations acm_port_ops = { + .dtr_rts = acm_port_dtr_rts, .shutdown = acm_port_shutdown, .activate = acm_port_activate, .destruct = acm_port_destruct, @@ -1429,8 +1439,6 @@ skip_countries: dev_info(&intf->dev, "ttyACM%d: USB ACM device\n", minor); - acm_set_control(acm, acm->ctrlout); - acm->line.dwDTERate = cpu_to_le32(9600); acm->line.bDataBits = 8; acm_set_line(acm, &acm->line); -- cgit v1.2.3-70-g09d2