Skip to content

Commit

Permalink
usb: cdc-acm: fix cooldown mechanism
Browse files Browse the repository at this point in the history 
commit 38203b8 upstream.

Commit a4e7279 ("cdc-acm: introduce a cool down") is causing
regression if there is some USB error, such as -EPROTO.

This has been reported on some samples of the Odroid-N2 using the Combee II
Zibgee USB dongle.

> struct acm *acm = container_of(work, struct acm, work)

is incorrect in case of a delayed work and causes warnings, usually from
the workqueue:

> WARNING: CPU: 0 PID: 0 at kernel/workqueue.c:1474 __queue_work+0x480/0x528.

When this happens, USB eventually stops working completely after a while.
Also the ACM_ERROR_DELAY bit is never set, so the cooldown mechanism
previously introduced cannot be triggered and acm_submit_read_urb() is
never called.

This changes makes the cdc-acm driver use a single delayed work, fixing the
pointer arithmetic in acm_softint() and set the ACM_ERROR_DELAY when the
cooldown mechanism appear to be needed.

Fixes: a4e7279 ("cdc-acm: introduce a cool down")
Cc: Oliver Neukum <oneukum@suse.com>
Reported-by: Pascal Vizeli <pascal.vizeli@nabucasa.com>
Acked-by: Oliver Neukum <oneukum@suse.com>
Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
Link: https://lore.kernel.org/r/20201019170702.150534-1-jbrunet@baylibre.com
Cc: stable <stable@vger.kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
@jbrun3t @gregkh
jbrun3t authored and gregkh committed Nov 5, 2020
1 parent c4c67e1 commit 88a5594
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 9 deletions.
12 changes: 5 additions & 7 deletions drivers/usb/class/cdc-acm.c
View file
Expand Up @@ -507,6 +507,7 @@ static void acm_read_bulk_callback(struct urb *urb)
"%s - cooling babbling device\n", __func__);
usb_mark_last_busy(acm->dev);
set_bit(rb->index, &acm->urbs_in_error_delay);
set_bit(ACM_ERROR_DELAY, &acm->flags);
cooldown = true;
break;
default:
Expand All @@ -532,7 +533,7 @@ static void acm_read_bulk_callback(struct urb *urb)

if (stopped || stalled || cooldown) {
if (stalled)
schedule_work(&acm->work);
schedule_delayed_work(&acm->dwork, 0);
else if (cooldown)
schedule_delayed_work(&acm->dwork, HZ / 2);
return;
Expand Down Expand Up @@ -562,13 +563,13 @@ static void acm_write_bulk(struct urb *urb)
acm_write_done(acm, wb);
spin_unlock_irqrestore(&acm->write_lock, flags);
set_bit(EVENT_TTY_WAKEUP, &acm->flags);
schedule_work(&acm->work);
schedule_delayed_work(&acm->dwork, 0);
}

static void acm_softint(struct work_struct *work)
{
int i;
struct acm *acm = container_of(work, struct acm, work);
struct acm *acm = container_of(work, struct acm, dwork.work);

if (test_bit(EVENT_RX_STALL, &acm->flags)) {
smp_mb(); /* against acm_suspend() */
Expand All @@ -584,7 +585,7 @@ static void acm_softint(struct work_struct *work)
if (test_and_clear_bit(ACM_ERROR_DELAY, &acm->flags)) {
for (i = 0; i < acm->rx_buflimit; i++)
if (test_and_clear_bit(i, &acm->urbs_in_error_delay))
acm_submit_read_urb(acm, i, GFP_NOIO);
acm_submit_read_urb(acm, i, GFP_KERNEL);
}

if (test_and_clear_bit(EVENT_TTY_WAKEUP, &acm->flags))
Expand Down Expand Up @@ -1364,7 +1365,6 @@ static int acm_probe(struct usb_interface *intf,
acm->ctrlsize = ctrlsize;
acm->readsize = readsize;
acm->rx_buflimit = num_rx_buf;
INIT_WORK(&acm->work, acm_softint);
INIT_DELAYED_WORK(&acm->dwork, acm_softint);
init_waitqueue_head(&acm->wioctl);
spin_lock_init(&acm->write_lock);
Expand Down Expand Up @@ -1574,7 +1574,6 @@ static void acm_disconnect(struct usb_interface *intf)
}

acm_kill_urbs(acm);
cancel_work_sync(&acm->work);
cancel_delayed_work_sync(&acm->dwork);

tty_unregister_device(acm_tty_driver, acm->minor);
Expand Down Expand Up @@ -1617,7 +1616,6 @@ static int acm_suspend(struct usb_interface *intf, pm_message_t message)
return 0;

acm_kill_urbs(acm);
cancel_work_sync(&acm->work);
cancel_delayed_work_sync(&acm->dwork);
acm->urbs_in_error_delay = 0;

Expand Down
3 changes: 1 addition & 2 deletions drivers/usb/class/cdc-acm.h
View file
Expand Up @@ -112,8 +112,7 @@ struct acm {
# define ACM_ERROR_DELAY 3
unsigned long urbs_in_error_delay; /* these need to be restarted after a delay */
struct usb_cdc_line_coding line; /* bits, stop, parity */
struct work_struct work; /* work queue entry for various purposes*/
struct delayed_work dwork; /* for cool downs needed in error recovery */
struct delayed_work dwork; /* work queue entry for various purposes */
unsigned int ctrlin; /* input control lines (DCD, DSR, RI, break, overruns) */
unsigned int ctrlout; /* output control lines (DTR, RTS) */
struct async_icount iocount; /* counters for control line changes */
Expand Down

0 comments on commit 88a5594

Please sign in to comment.