Commit 38203b83 authored by Jerome Brunet's avatar Jerome Brunet Committed by Greg Kroah-Hartman
Browse files

usb: cdc-acm: fix cooldown mechanism 



Commit a4e7279c ("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: a4e7279c ("cdc-acm: introduce a cool down")
Cc: Oliver Neukum <oneukum@suse.com>
Reported-by: default avatarPascal Vizeli <pascal.vizeli@nabucasa.com>
Acked-by: default avatarOliver Neukum <oneukum@suse.com>
Signed-off-by: default avatarJerome 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: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 3cd54a61

drivers/usb/class/cdc-acm.c

+5 −7
Original line number Diff line number Diff line
@@ -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 @@ made_compressed_probe: 
	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
Original line number Diff line number Diff line
@@ -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 */

CRA Git | Maintained and supported by SUSTech CRA and CCSE