From 38203b8385bf6283537162bde7d499f830964711 Mon Sep 17 00:00:00 2001

From: Jerome Brunet <jbrunet@baylibre.com>

Date: Mon, 19 Oct 2020 19:07:02 +0200

Subject: [PATCH] usb: cdc-acm: fix cooldown mechanism

Git-commit: 38203b8385bf6283537162bde7d499f830964711

References: git-fixes

Patch-mainline: v5.10-rc2

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.

> 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: a4e7279cd1d1 ("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: Oliver Neukum <oneukum@suse.com>

---

 drivers/usb/class/cdc-acm.c |   12 +++++-------

 drivers/usb/class/cdc-acm.h |    3 +--

 2 files changed, 6 insertions(+), 9 deletions(-)

--- a/drivers/usb/class/cdc-acm.c

+++ b/drivers/usb/class/cdc-acm.c

@@ -520,6 +520,7 @@ static void acm_read_bulk_callback(struc

 			"%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:

@@ -545,7 +546,7 @@ static void acm_read_bulk_callback(struc

 	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;

@@ -580,13 +581,13 @@ static void acm_write_bulk(struct urb *u

 	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() */

@@ -602,7 +603,7 @@ static void acm_softint(struct work_stru

 	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))

@@ -1404,7 +1405,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);

@@ -1618,7 +1618,6 @@ static void acm_disconnect(struct usb_in

 	}

 	acm_kill_urbs(acm);

-	cancel_work_sync(&acm->work);

 	cancel_delayed_work_sync(&acm->dwork);

 	tty_unregister_device(acm_tty_driver, acm->minor);

@@ -1661,7 +1660,6 @@ static int acm_suspend(struct usb_interf

 		return 0;

 	acm_kill_urbs(acm);

-	cancel_work_sync(&acm->work);

 	cancel_delayed_work_sync(&acm->dwork);

 	acm->urbs_in_error_delay = 0;

--- a/drivers/usb/class/cdc-acm.h

+++ b/drivers/usb/class/cdc-acm.h

@@ -109,8 +109,7 @@ struct acm {

 #		define EVENT_RX_STALL	1

 #		define ACM_ERROR_DELAY	3

 	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 long urbs_in_error_delay;		/* these need to be restarted after a delay */

 	unsigned int ctrlin;				/* input control lines (DCD, DSR, RI, break, overruns) */

 	unsigned int ctrlout;				/* output control lines (DTR, RTS) */