On Thu, Nov 03, 2016 at 02:12:58PM +0100, Oliver Neukum wrote:
[snip]
> I think the way usbnet handles -EPIPE is the best. We are a bit on thin
> ice because the CDC spec does not list under which conditions we should
> see a stall, thus by implication: never.
> But in general you cannot ignore a stall. You need to clear the halt.
This still cannot recover from "usb 3-1.4: clear tt 1 (9061) error -71",
but does recover from stall. If I got it wrong, please, let me know.
Thank you,
ladis
diff --git a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c
index 4c931d9..60e148d 100644
--- a/drivers/usb/class/cdc-acm.c
+++ b/drivers/usb/class/cdc-acm.c
@@ -28,8 +28,8 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
-#undef DEBUG
-#undef VERBOSE_DEBUG
+#define DEBUG
+#define VERBOSE_DEBUG
#include <linux/kernel.h>
#include <linux/errno.h>
@@ -416,8 +416,7 @@ static void acm_read_bulk_callback(struct urb *urb)
int status = urb->status;
dev_vdbg(&acm->data->dev, "got urb %d, len %d, status %d\n",
- rb->index, urb->actual_length,
- status);
+ rb->index, urb->actual_length, status);
if (!acm->dev) {
set_bit(rb->index, &acm->read_urbs_free);
@@ -430,18 +429,22 @@ static void acm_read_bulk_callback(struct urb *urb)
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->control->dev,
- "%s - urb shutting down with status: %d\n",
- __func__, status);
+ dev_dbg(&acm->data->dev,
+ "%s - urb shutting down with status: %d\n",
+ __func__, status);
return;
default:
- dev_dbg(&acm->control->dev,
- "%s - nonzero urb status received: %d\n",
- __func__, status);
- break;
+ dev_dbg(&acm->data->dev,
+ "%s - nonzero urb status received: %d\n",
+ __func__, status);
+ return;
}
/*
@@ -479,6 +482,7 @@ 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);
}
@@ -486,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);
+ 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);
+ 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);
+ }
}
/*
@@ -1098,19 +1125,17 @@ static void acm_write_buffers_free(struct acm *acm)
{
int i;
struct acm_wb *wb;
- struct usb_device *usb_dev = interface_to_usbdev(acm->control);
for (wb = &acm->wb[0], i = 0; i < ACM_NW; i++, wb++)
- usb_free_coherent(usb_dev, acm->writesize, wb->buf, wb->dmah);
+ usb_free_coherent(acm->dev, acm->writesize, wb->buf, wb->dmah);
}
static void acm_read_buffers_free(struct acm *acm)
{
- struct usb_device *usb_dev = interface_to_usbdev(acm->control);
int i;
for (i = 0; i < acm->rx_buflimit; i++)
- usb_free_coherent(usb_dev, acm->readsize,
+ usb_free_coherent(acm->dev, acm->readsize,
acm->read_buffers[i].base, acm->read_buffers[i].dma);
}
@@ -1354,8 +1379,15 @@ static int acm_probe(struct usb_interface *intf,
spin_lock_init(&acm->read_lock);
mutex_init(&acm->mutex);
acm->is_int_ep = usb_endpoint_xfer_int(epread);
- if (acm->is_int_ep)
+ if (acm->is_int_ep) {
acm->bInterval = epread->bInterval;
+ acm->in = usb_rcvintpipe(usb_dev, epread->bEndpointAddress);
+ } else
+ acm->in = usb_rcvbulkpipe(usb_dev, epread->bEndpointAddress);
+ if (usb_endpoint_xfer_int(epwrite))
+ acm->out = usb_sndintpipe(usb_dev, epwrite->bEndpointAddress);
+ else
+ acm->out = usb_sndbulkpipe(usb_dev, epwrite->bEndpointAddress);
tty_port_init(&acm->port);
acm->port.ops = &acm_port_ops;
init_usb_anchor(&acm->delayed);
@@ -1391,16 +1423,12 @@ static int acm_probe(struct usb_interface *intf,
urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
urb->transfer_dma = rb->dma;
if (acm->is_int_ep) {
- usb_fill_int_urb(urb, acm->dev,
- usb_rcvintpipe(usb_dev, epread->bEndpointAddress),
- rb->base,
+ usb_fill_int_urb(urb, acm->dev, acm->in, rb->base,
acm->readsize,
acm_read_bulk_callback, rb,
acm->bInterval);
} else {
- usb_fill_bulk_urb(urb, acm->dev,
- usb_rcvbulkpipe(usb_dev, epread->bEndpointAddress),
- rb->base,
+ usb_fill_bulk_urb(urb, acm->dev, acm->in, rb->base,
acm->readsize,
acm_read_bulk_callback, rb);
}
@@ -1416,12 +1444,10 @@ static int acm_probe(struct usb_interface *intf,
goto alloc_fail7;
if (usb_endpoint_xfer_int(epwrite))
- usb_fill_int_urb(snd->urb, usb_dev,
- usb_sndintpipe(usb_dev, epwrite->bEndpointAddress),
+ usb_fill_int_urb(snd->urb, usb_dev, acm->out,
NULL, acm->writesize, acm_write_bulk, snd, epwrite->bInterval);
else
- usb_fill_bulk_urb(snd->urb, usb_dev,
- usb_sndbulkpipe(usb_dev, epwrite->bEndpointAddress),
+ usb_fill_bulk_urb(snd->urb, usb_dev, acm->out,
NULL, acm->writesize, acm_write_bulk, snd);
snd->urb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
if (quirks & SEND_ZERO_PACKET)
@@ -1493,8 +1519,8 @@ static int acm_probe(struct usb_interface *intf,
}
if (quirks & CLEAR_HALT_CONDITIONS) {
- usb_clear_halt(usb_dev, usb_rcvbulkpipe(usb_dev, epread->bEndpointAddress));
- usb_clear_halt(usb_dev, usb_sndbulkpipe(usb_dev, epwrite->bEndpointAddress));
+ usb_clear_halt(usb_dev, acm->in);
+ usb_clear_halt(usb_dev, acm->out);
}
return 0;
@@ -1544,7 +1570,6 @@ static void stop_data_traffic(struct acm *acm)
static void acm_disconnect(struct usb_interface *intf)
{
struct acm *acm = usb_get_intfdata(intf);
- struct usb_device *usb_dev = interface_to_usbdev(intf);
struct tty_struct *tty;
int i;
@@ -1582,7 +1607,7 @@ static void acm_disconnect(struct usb_interface *intf)
for (i = 0; i < acm->rx_buflimit; i++)
usb_free_urb(acm->read_urbs[i]);
acm_write_buffers_free(acm);
- usb_free_coherent(usb_dev, acm->ctrlsize, acm->ctrl_buffer, acm->ctrl_dma);
+ usb_free_coherent(acm->dev, acm->ctrlsize, acm->ctrl_buffer, acm->ctrl_dma);
acm_read_buffers_free(acm);
if (!acm->combined_interfaces)
diff --git a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h
index 1f1eabf..1db974d 100644
--- a/drivers/usb/class/cdc-acm.h
+++ b/drivers/usb/class/cdc-acm.h
@@ -83,6 +83,7 @@ struct acm {
struct usb_device *dev; /* the corresponding usb device */
struct usb_interface *control; /* control interface */
struct usb_interface *data; /* data interface */
+ unsigned in, out; /* i/o pipes */
struct tty_port port; /* our tty port data */
struct urb *ctrlurb; /* urbs */
u8 *ctrl_buffer; /* buffers of urbs */
@@ -102,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) */
> We cannot do full error handling for modems because they would drop
> the connection if we reset the device.
>
> Regards
> Oliver
>
--
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
