On Sun, Nov 06, 2016 at 07:25:19AM +0100, Oliver Neukum wrote:
> On Sun, 2016-11-06 at 01:36 +0100, Ladislav Michl wrote:
> >
> > @@ -475,7 +490,30 @@ static void acm_softint(struct work_struct *work)
> > {
> > struct acm *acm = container_of(work, struct acm, work);
> >
> > - tty_port_tty_wakeup(&acm->port);
> > + dev_vdbg(&acm->data->dev, "scheduled work\n");
> > +
> > + if (test_bit(EVENT_RX_STALL, &acm->flags)) {
> > + int i, status;
> > +
> > + for (i = 0; i < acm->rx_buflimit; i++) {
> > + usb_kill_urb(acm->read_urbs[i]);
> > + set_bit(i, &acm->read_urbs_free);
> > + }
> > +
> > + status = usb_autopm_get_interface(acm->data);
>
> No. If you really resume the device here, you reanimate the URBs
> you just killed. The order must be inverted.
>
> > + if (!status) {
> > + status = usb_clear_halt(acm->dev, acm->in);
> > + usb_autopm_put_interface(acm->data);
> > + }
> > + if (!status)
> > + acm_submit_read_urbs(acm, GFP_KERNEL);
>
> No, you would kill the device. Either do it all conditionally
> or nothing.
I do not follow. Did you mean something like this?
-- >8 --
Subject: [PATCHv2 4/4] cdc-acm: clear halt condition
Signed-off-by: Ladislav Michl <[email protected]>
---
drivers/usb/class/cdc-acm.c | 53 ++++++++++++++++++++++++++++++++++++++-------
drivers/usb/class/cdc-acm.h | 3 +++
2 files changed, 48 insertions(+), 8 deletions(-)
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index b76c95c..c122fdd 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,37 @@ 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, status;
struct acm *acm = container_of(work, struct acm, work);
- tty_port_tty_wakeup(&acm->port);
+ dev_vdbg(&acm->data->dev, "scheduled work\n");
+
+ if (test_bit(EVENT_RX_STALL, &acm->flags)) {
+ status = usb_autopm_get_interface(acm->data);
+ if (!status) {
+ for (i = 0; i < acm->rx_buflimit; i++) {
+ usb_kill_urb(acm->read_urbs[i]);
+ set_bit(i, &acm->read_urbs_free);
+ }
+ status = usb_clear_halt(acm->dev, acm->in);
+ usb_autopm_put_interface(acm->data);
+ if (!status)
+ acm_submit_read_urbs(acm, GFP_KERNEL);
+
+ }
+ 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);
+ }
}
/*
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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
