patch "usb: cdc-acm: fix cooldown mechanism" added to usb-linus
Subsystems (3)
This is a note to let you know that I've just added the patch titled
usb: cdc-acm: fix cooldown mechanism
to my usb git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
in the usb-linus branch.
The patch will show up in the next release of the linux-next tree (usually sometime within the next 24 hours during the week.)
The patch will hopefully also be merged in Linus's tree for the next -rc kernel release.
If you have any questions about this process, please let me know.
From 38203b8385bf6283537162bde7d499f830964711 Mon Sep 17 00:00:00 2001
From: Jerome Brunet <[email protected]>
Date: Mon, 19 Oct 2020 19:07:02 +0200
Subject: usb: cdc-acm: fix cooldown mechanism
Commit a4e7279cd1d1 ("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.
Quoted 1 line
struct acm *acm =
container_of(work, struct acm, work)
is incorrect in case of a delayed work and causes warnings, usually from the workqueue:
Quoted 1 line
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.
--- drivers/usb/class/cdc-acm.c | 12 +++++------- drivers/usb/class/cdc-acm.h | 3 +-- 2 files changed, 6 insertions(+), 9 deletions(-)
drivers/usb/class/cdc-acm.c +5 -7
@@ -508,6 +508,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 :
@@ -533,7 +534,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 ;
@@ -563,13 +564,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() */
@@ -585,7 +586,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))
@@ -1351,7 +1352,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);
@@ -1561,7 +1561,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);
@@ -1604,7 +1603,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 ;
drivers/usb/class/cdc-acm.h +1 -2
@@ -112,8 +112,7 @@ struct acm {# define ACM_ERROR_DELAY 3unsigned 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 */
Signature · 1 line
2.29.1