->disconnect() is usually called in two places:
- on device / gadget removal
- on reset interrupt aka device disconnect.
The udc usually calls this an then cleans up the hardware. On the gadget
side, the disconnect callbacks disables every function which leads to
endpoint disabling and killing all requests.
If we perform this async (and later) then there is not much harm:
- the requests are most likely gone, the gadget isn't canceling them. If
not, we still do it.
- the endpoint is disabled, disabling it again shouldn't cause any harm.
- freeing the memory "later". The memory is allocated again in "SET
CONFIG" which is performed by the same work queue so it is
synchronized.
In theory we could get into the following scenario:
- disconnect, -> schedule wq for config cleanup
- connect again
- SET ADDRESS
- GET CONFIG || STATUS || INTERFACE before the wq finished running.
To avoid this unlikely event, disc_pend has been introduced. In that
case we behave act like we don't have any config set and return with
an error like the spec wants us to.
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
---
drivers/usb/gadget/composite.c | 43 +++++++++++++++++++++++++++++----------
include/linux/usb/composite.h | 2 +
2 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 051db6f..f5c5bd8 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -1176,7 +1176,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
case USB_REQ_GET_CONFIGURATION:
if (ctrl->bRequestType != USB_DIR_IN)
goto unknown;
- if (cdev->config)
+ if (cdev->config && !cdev->disc_pend)
*(u8 *)req->buf = cdev->config->bConfigurationValue;
else
*(u8 *)req->buf = 0;
@@ -1189,7 +1189,9 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
case USB_REQ_SET_INTERFACE:
if (ctrl->bRequestType != USB_RECIP_INTERFACE)
goto unknown;
- if (!cdev->config || intf >= MAX_CONFIG_INTERFACES)
+ if (!cdev->config || cdev->disc_pend)
+ break;
+ if (intf >= MAX_CONFIG_INTERFACES)
break;
f = cdev->config->interface[intf];
if (!f)
@@ -1202,7 +1204,9 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
case USB_REQ_GET_INTERFACE:
if (ctrl->bRequestType != (USB_DIR_IN|USB_RECIP_INTERFACE))
goto unknown;
- if (!cdev->config || intf >= MAX_CONFIG_INTERFACES)
+ if (!cdev->config || cdev->disc_pend)
+ break;
+ if (intf >= MAX_CONFIG_INTERFACES)
break;
f = cdev->config->interface[intf];
if (!f)
@@ -1229,7 +1233,9 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
goto unknown;
value = 2; /* This is the length of the get_status reply */
put_unaligned_le16(0, req->buf);
- if (!cdev->config || intf >= MAX_CONFIG_INTERFACES)
+ if (!cdev->config || cdev->disc_pend)
+ break;
+ if (intf >= MAX_CONFIG_INTERFACES)
break;
f = cdev->config->interface[intf];
if (!f)
@@ -1252,7 +1258,9 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
goto unknown;
switch (w_value) {
case USB_INTRF_FUNC_SUSPEND:
- if (!cdev->config || intf >= MAX_CONFIG_INTERFACES)
+ if (!cdev->config || cdev->disc_pend)
+ break;
+ if (intf >= MAX_CONFIG_INTERFACES)
break;
f = cdev->config->interface[intf];
if (!f)
@@ -1286,7 +1294,9 @@ unknown:
*/
switch (ctrl->bRequestType & USB_RECIP_MASK) {
case USB_RECIP_INTERFACE:
- if (!cdev->config || intf >= MAX_CONFIG_INTERFACES)
+ if (!cdev->config || cdev->disc_pend)
+ break;
+ if (intf >= MAX_CONFIG_INTERFACES)
break;
f = cdev->config->interface[intf];
break;
@@ -1336,18 +1346,27 @@ done:
return value;
}
-static void composite_disconnect(struct usb_gadget *gadget)
+static void composite_disconnect_wq(struct work_struct *wq)
{
- struct usb_composite_dev *cdev = get_gadget_data(gadget);
- unsigned long flags;
+ struct usb_composite_dev *cdev = container_of(wq,
+ struct usb_composite_dev, disc_work);
/* REVISIT: should we have config and device level
* disconnect callbacks?
*/
- spin_lock_irqsave(&cdev->lock, flags);
+ spin_lock_irq(&cdev->lock);
if (cdev->config)
reset_config(cdev);
- spin_unlock_irqrestore(&cdev->lock, flags);
+ spin_unlock_irq(&cdev->lock);
+ cdev->disc_pend = 0;
+}
+
+static void composite_disconnect(struct usb_gadget *gadget)
+{
+ struct usb_composite_dev *cdev = get_gadget_data(gadget);
+
+ cdev->disc_pend = 1;
+ queue_work(system_nrt_wq, &cdev->disc_work);
}
/*-------------------------------------------------------------------------*/
@@ -1374,6 +1393,7 @@ composite_unbind(struct usb_gadget *gadget)
* so there's no i/o concurrency that could affect the
* state protected by cdev->lock.
*/
+ flush_work(&cdev->disc_work);
flush_work(&cdev->wq);
WARN_ON(cdev->config);
@@ -1440,6 +1460,7 @@ static int composite_bind(struct usb_gadget *gadget)
spin_lock_init(&cdev->lock);
spin_lock_init(&cdev->deact_lock);
INIT_WORK(&cdev->wq, composite_delayed_setup);
+ INIT_WORK(&cdev->disc_work, composite_disconnect_wq);
cdev->gadget = gadget;
set_gadget_data(gadget, cdev);
INIT_LIST_HEAD(&cdev->configs);
diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
index a332cc2..576432d 100644
--- a/include/linux/usb/composite.h
+++ b/include/linux/usb/composite.h
@@ -339,6 +339,7 @@ struct usb_composite_dev {
/* private: */
/* internals */
unsigned int suspended:1;
+ unsigned int disc_pend:1;
struct usb_device_descriptor desc;
struct list_head configs;
struct usb_composite_driver *driver;
@@ -347,6 +348,7 @@ struct usb_composite_dev {
u8 product_override;
u8 serial_override;
+ struct work_struct disc_work;
/* the gadget driver won't enable the data pullup
* while the deactivation count is nonzero.
*/
--
1.7.8.3
--
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
