USB: mdm_bridge: Fix control bridge suspend
The current code does not abort suspend when there are pending URBs.
They are simply killed during suspend. This may lead to QMI stall.
Add proper checks to ensure that there is no pending Tx/Rx data
during suspend. Clear the "SUSPENDED" flag in resume only after
taking out all the URBs from "tx_deferred" anchor.
CRs-Fixed: 451414
Change-Id: Icc48afc5eeedcc126d0ab1bec951bde4a53ac165
Signed-off-by: Pavankumar Kondeti <pkondeti@codeaurora.org>
diff --git a/drivers/usb/misc/mdm_ctrl_bridge.c b/drivers/usb/misc/mdm_ctrl_bridge.c
index 69e28e4..dab9022 100644
--- a/drivers/usb/misc/mdm_ctrl_bridge.c
+++ b/drivers/usb/misc/mdm_ctrl_bridge.c
@@ -35,6 +35,12 @@
#define SUSPENDED BIT(0)
+enum ctrl_bridge_rx_state {
+ RX_IDLE, /* inturb is not queued */
+ RX_WAIT, /* inturb is queued and waiting for data */
+ RX_BUSY, /* inturb is completed. processing RX */
+};
+
struct ctrl_bridge {
struct usb_device *udev;
struct usb_interface *intf;
@@ -63,6 +69,9 @@
/* output control lines (DTR, RTS) */
unsigned int cbits_tomdm;
+ spinlock_t lock;
+ enum ctrl_bridge_rx_state rx_state;
+
/* counters */
unsigned int snd_encap_cmd;
unsigned int get_encap_res;
@@ -133,12 +142,39 @@
}
EXPORT_SYMBOL(ctrl_bridge_set_cbits);
+static int ctrl_bridge_start_read(struct ctrl_bridge *dev, gfp_t gfp_flags)
+{
+ int retval = 0;
+ unsigned long flags;
+
+ if (!dev->inturb) {
+ dev_err(&dev->intf->dev, "%s: inturb is NULL\n", __func__);
+ return -ENODEV;
+ }
+
+ retval = usb_submit_urb(dev->inturb, gfp_flags);
+ if (retval < 0 && retval != -EPERM) {
+ dev_err(&dev->intf->dev,
+ "%s error submitting int urb %d\n",
+ __func__, retval);
+ }
+
+ spin_lock_irqsave(&dev->lock, flags);
+ if (retval)
+ dev->rx_state = RX_IDLE;
+ else
+ dev->rx_state = RX_WAIT;
+ spin_unlock_irqrestore(&dev->lock, flags);
+
+ return retval;
+}
+
static void resp_avail_cb(struct urb *urb)
{
struct ctrl_bridge *dev = urb->context;
- int status = 0;
int resubmit_urb = 1;
struct bridge *brdg = dev->brdg;
+ unsigned long flags;
/*usb device disconnect*/
if (urb->dev->state == USB_STATE_NOTATTACHED)
@@ -170,12 +206,14 @@
if (resubmit_urb) {
/*re- submit int urb to check response available*/
- status = usb_submit_urb(dev->inturb, GFP_ATOMIC);
- if (status)
- dev_err(&dev->intf->dev,
- "%s: Error re-submitting Int URB %d\n",
- __func__, status);
+ ctrl_bridge_start_read(dev, GFP_ATOMIC);
+ } else {
+ spin_lock_irqsave(&dev->lock, flags);
+ dev->rx_state = RX_IDLE;
+ spin_unlock_irqrestore(&dev->lock, flags);
}
+
+ usb_autopm_put_interface_async(dev->intf);
}
static void notification_available_cb(struct urb *urb)
@@ -186,11 +224,16 @@
struct bridge *brdg = dev->brdg;
unsigned int ctrl_bits;
unsigned char *data;
+ unsigned long flags;
/*usb device disconnect*/
if (urb->dev->state == USB_STATE_NOTATTACHED)
return;
+ spin_lock_irqsave(&dev->lock, flags);
+ dev->rx_state = RX_IDLE;
+ spin_unlock_irqrestore(&dev->lock, flags);
+
switch (urb->status) {
case 0:
/*success*/
@@ -217,7 +260,11 @@
switch (ctrl->bNotificationType) {
case USB_CDC_NOTIFY_RESPONSE_AVAILABLE:
+ spin_lock_irqsave(&dev->lock, flags);
+ dev->rx_state = RX_BUSY;
+ spin_unlock_irqrestore(&dev->lock, flags);
dev->resp_avail++;
+ usb_autopm_get_interface_no_resume(dev->intf);
usb_fill_control_urb(dev->readurb, dev->udev,
usb_rcvctrlpipe(dev->udev, 0),
(unsigned char *)dev->in_ctlreq,
@@ -230,6 +277,7 @@
dev_err(&dev->intf->dev,
"%s: Error submitting Read URB %d\n",
__func__, status);
+ usb_autopm_put_interface_async(dev->intf);
goto resubmit_int_urb;
}
return;
@@ -253,28 +301,7 @@
}
resubmit_int_urb:
- status = usb_submit_urb(urb, GFP_ATOMIC);
- if (status)
- dev_err(&dev->intf->dev, "%s: Error re-submitting Int URB %d\n",
- __func__, status);
-}
-
-static int ctrl_bridge_start_read(struct ctrl_bridge *dev)
-{
- int retval = 0;
-
- if (!dev->inturb) {
- dev_err(&dev->intf->dev, "%s: inturb is NULL\n", __func__);
- return -ENODEV;
- }
-
- retval = usb_submit_urb(dev->inturb, GFP_KERNEL);
- if (retval < 0)
- dev_err(&dev->intf->dev,
- "%s error submitting int urb %d\n",
- __func__, retval);
-
- return retval;
+ ctrl_bridge_start_read(dev, GFP_ATOMIC);
}
int ctrl_bridge_open(struct bridge *brdg)
@@ -356,6 +383,7 @@
struct urb *writeurb;
struct usb_ctrlrequest *out_ctlreq;
struct ctrl_bridge *dev;
+ unsigned long flags;
if (id >= MAX_BRIDGE_DEVICES) {
result = -EINVAL;
@@ -424,12 +452,15 @@
goto free_ctrlreq;
}
+ spin_lock_irqsave(&dev->lock, flags);
if (test_bit(SUSPENDED, &dev->flags)) {
usb_anchor_urb(writeurb, &dev->tx_deferred);
+ spin_unlock_irqrestore(&dev->lock, flags);
goto deferred;
}
usb_anchor_urb(writeurb, &dev->tx_submitted);
+ spin_unlock_irqrestore(&dev->lock, flags);
result = usb_submit_urb(writeurb, GFP_ATOMIC);
if (result < 0) {
dev_err(&dev->intf->dev, "%s: submit URB error %d\n",
@@ -456,6 +487,7 @@
int ctrl_bridge_suspend(unsigned int id)
{
struct ctrl_bridge *dev;
+ unsigned long flags;
if (id >= MAX_BRIDGE_DEVICES)
return -EINVAL;
@@ -464,10 +496,27 @@
if (!dev)
return -ENODEV;
- set_bit(SUSPENDED, &dev->flags);
+ spin_lock_irqsave(&dev->lock, flags);
+ if (!usb_anchor_empty(&dev->tx_submitted) || dev->rx_state == RX_BUSY) {
+ spin_unlock_irqrestore(&dev->lock, flags);
+ return -EBUSY;
+ }
+ spin_unlock_irqrestore(&dev->lock, flags);
+
usb_kill_urb(dev->inturb);
- usb_kill_urb(dev->readurb);
- usb_kill_anchored_urbs(&dev->tx_submitted);
+
+ spin_lock_irqsave(&dev->lock, flags);
+ if (dev->rx_state != RX_IDLE) {
+ spin_unlock_irqrestore(&dev->lock, flags);
+ return -EBUSY;
+ }
+ if (!usb_anchor_empty(&dev->tx_submitted)) {
+ spin_unlock_irqrestore(&dev->lock, flags);
+ ctrl_bridge_start_read(dev, GFP_KERNEL);
+ return -EBUSY;
+ }
+ set_bit(SUSPENDED, &dev->flags);
+ spin_unlock_irqrestore(&dev->lock, flags);
return 0;
}
@@ -476,6 +525,8 @@
{
struct ctrl_bridge *dev;
struct urb *urb;
+ unsigned long flags;
+ int ret;
if (id >= MAX_BRIDGE_DEVICES)
return -EINVAL;
@@ -484,12 +535,19 @@
if (!dev)
return -ENODEV;
- if (!test_and_clear_bit(SUSPENDED, &dev->flags))
+ if (!test_bit(SUSPENDED, &dev->flags))
return 0;
+ spin_lock_irqsave(&dev->lock, flags);
/* submit pending write requests */
while ((urb = usb_get_from_anchor(&dev->tx_deferred))) {
- int ret;
+ spin_unlock_irqrestore(&dev->lock, flags);
+ /*
+ * usb_get_from_anchor() does not drop the
+ * ref count incremented by the usb_anchro_urb()
+ * called in Tx submission path. Let us do it.
+ */
+ usb_put_urb(urb);
usb_anchor_urb(urb, &dev->tx_submitted);
ret = usb_submit_urb(urb, GFP_ATOMIC);
if (ret < 0) {
@@ -499,9 +557,12 @@
usb_free_urb(urb);
usb_autopm_put_interface_async(dev->intf);
}
+ spin_lock_irqsave(&dev->lock, flags);
}
+ clear_bit(SUSPENDED, &dev->flags);
+ spin_unlock_irqrestore(&dev->lock, flags);
- return ctrl_bridge_start_read(dev);
+ return ctrl_bridge_start_read(dev, GFP_KERNEL);
}
#if defined(CONFIG_DEBUG_FS)
@@ -698,7 +759,7 @@
platform_device_add(dev->pdev);
- return ctrl_bridge_start_read(dev);
+ return ctrl_bridge_start_read(dev, GFP_KERNEL);
free_rbuf:
kfree(dev->readbuf);
@@ -727,6 +788,7 @@
platform_device_unregister(dev->pdev);
+ usb_scuttle_anchored_urbs(&dev->tx_deferred);
usb_kill_anchored_urbs(&dev->tx_submitted);
usb_kill_urb(dev->inturb);
@@ -758,6 +820,7 @@
/*transport name will be set during probe*/
dev->name = "none";
+ spin_lock_init(&dev->lock);
init_usb_anchor(&dev->tx_submitted);
init_usb_anchor(&dev->tx_deferred);