Re: [PATCH 4/4] cdc-acm: clear halt condition

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <ladis@xxxxxxxxxxxxxx>
---
 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux