On Sun, Nov 06, 2016 at 10:30:02PM +0100, Oliver Neukum wrote:
> Hi,
>
> almost. Two issues left with this section.
>
> 1. It makes no sense to check the results of usb_clear_halt()
> If it fails, we cannot do anything sensible. We have to restart
> IO and hope for the best. Not doing it that way risks introducing
> a regression.
>
> 2. usb_autopm_put_interface() must be after acm_submit_read_urbs()
> because the URBs are to be killed when the device is suspended.
>
> And on a related note:
>
> 3. The device may be reset externally before the work queue
> is executed. pre_reset() needs to clear the flag EVENT_RX_STALL.
Hi,
thanks for feedback. Here's an update:
-- >8 --
Subject: [PATCHv3 4/4] cdc-acm: clear halt condition
Signed-off-by: Ladislav Michl <ladis@xxxxxxxxxxxxxx>
---
drivers/usb/class/cdc-acm.c | 59 +++++++++++++++++++++++++++++++++++++++------
drivers/usb/class/cdc-acm.h | 3 +++
2 files changed, 54 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index b76c95c..711d680 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -424,15 +424,29 @@ static void acm_read_bulk_callback(struct urb *urb)
return;
}
- if (status) {
- set_bit(rb->index, &acm->read_urbs_free);
- if ((status != -ENOENT) || (urb->actual_length == 0))
- return;
+ switch (status) {
+ case 0:
+ usb_mark_last_busy(acm->dev);
+ acm_process_read_urb(acm, urb);
+ break;
+ case -EPIPE:
+ set_bit(EVENT_RX_STALL, &acm->flags);
+ schedule_work(&acm->work);
+ return;
+ case -ECONNRESET:
+ case -ENOENT:
+ case -ESHUTDOWN:
+ dev_dbg(&acm->data->dev,
+ "%s - urb shutting down with status: %d\n",
+ __func__, status);
+ return;
+ default:
+ dev_dbg(&acm->data->dev,
+ "%s - nonzero urb status received: %d\n",
+ __func__, status);
+ break;
}
- usb_mark_last_busy(acm->dev);
-
- acm_process_read_urb(acm, urb);
/*
* Unthrottle may run on another CPU which needs to see events
* in the same order. Submission has an implict barrier
@@ -468,14 +482,33 @@ static void acm_write_bulk(struct urb *urb)
spin_lock_irqsave(&acm->write_lock, flags);
acm_write_done(acm, wb);
spin_unlock_irqrestore(&acm->write_lock, flags);
+ set_bit(EVENT_TTY_WAKEUP, &acm->flags);
schedule_work(&acm->work);
}
static void acm_softint(struct work_struct *work)
{
+ int i;
struct acm *acm = container_of(work, struct acm, work);
- tty_port_tty_wakeup(&acm->port);
+ if (test_bit(EVENT_RX_STALL, &acm->flags)) {
+ if (!(usb_autopm_get_interface(acm->data))) {
+ for (i = 0; i < acm->rx_buflimit; i++) {
+ usb_kill_urb(acm->read_urbs[i]);
+ set_bit(i, &acm->read_urbs_free);
+ }
+ usb_clear_halt(acm->dev, acm->in);
+ acm_submit_read_urbs(acm, GFP_KERNEL);
+ usb_autopm_put_interface(acm->data);
+
+ }
+ clear_bit(EVENT_RX_STALL, &acm->flags);
+ }
+
+ if (test_bit(EVENT_TTY_WAKEUP, &acm->flags)) {
+ tty_port_tty_wakeup(&acm->port);
+ clear_bit(EVENT_TTY_WAKEUP, &acm->flags);
+ }
}
/*
@@ -1654,6 +1687,15 @@ static int acm_reset_resume(struct usb_interface *intf)
#endif /* CONFIG_PM */
+static int acm_pre_reset(struct usb_interface *intf)
+{
+ struct acm *acm = usb_get_intfdata(intf);
+
+ clear_bit(EVENT_RX_STALL, &acm->flags);
+
+ return 0;
+}
+
#define NOKIA_PCSUITE_ACM_INFO(x) \
USB_DEVICE_AND_INTERFACE_INFO(0x0421, x, \
USB_CLASS_COMM, USB_CDC_SUBCLASS_ACM, \
@@ -1895,6 +1937,7 @@ static struct usb_driver acm_driver = {
.resume = acm_resume,
.reset_resume = acm_reset_resume,
#endif
+ .pre_reset = acm_pre_reset,
.id_table = acm_ids,
#ifdef CONFIG_PM
.supports_autosuspend = 1,
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index 58ddd25..1db974d 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -103,6 +103,9 @@ struct acm {
spinlock_t write_lock;
struct mutex mutex;
bool disconnected;
+ unsigned long flags;
+# define EVENT_TTY_WAKEUP 0
+# define EVENT_RX_STALL 1
struct usb_cdc_line_coding line; /* bits, stop, parity */
struct work_struct work; /* work queue entry for line discipline waking up */
unsigned int ctrlin; /* input control lines (DCD, DSR, RI, break, overruns) */
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
