This reverts commit f22e9b67f19ccc73de1ae04375d4b30684e261f8.
The regression reported in
https://lore.kernel.org/all/[email protected]/ is being
fixed in
commit 7d7863db7cc0 ("usb: gadget: udc: core: Offload usb_udc_vbus_handler processing").
Hence reverting the revert.
Signed-off-by: Badhri Jagan Sridharan <[email protected]>
---
drivers/usb/gadget/udc/core.c | 148 ++++++++++++++++++++++++----------
1 file changed, 104 insertions(+), 44 deletions(-)
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 52e6d2e84e35..583c339876ab 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -37,6 +37,10 @@ static const struct bus_type gadget_bus_type;
* @vbus: for udcs who care about vbus status, this value is real vbus status;
* for udcs who do not care about vbus status, this value is always true
* @started: the UDC's started state. True if the UDC had started.
+ * @connect_lock: protects udc->vbus, udc->started, gadget->connect, gadget->deactivate related
+ * functions. usb_gadget_connect_locked, usb_gadget_disconnect_locked,
+ * usb_udc_connect_control_locked, usb_gadget_udc_start_locked, usb_gadget_udc_stop_locked are
+ * called with this lock held.
*
* This represents the internal data structure which is used by the UDC-class
* to hold information about udc driver and gadget together.
@@ -48,6 +52,7 @@ struct usb_udc {
struct list_head list;
bool vbus;
bool started;
+ struct mutex connect_lock;
};
static struct class *udc_class;
@@ -687,17 +692,9 @@ int usb_gadget_vbus_disconnect(struct usb_gadget *gadget)
}
EXPORT_SYMBOL_GPL(usb_gadget_vbus_disconnect);
-/**
- * usb_gadget_connect - software-controlled connect to USB host
- * @gadget:the peripheral being connected
- *
- * Enables the D+ (or potentially D-) pullup. The host will start
- * enumerating this gadget when the pullup is active and a VBUS session
- * is active (the link is powered).
- *
- * Returns zero on success, else negative errno.
- */
-int usb_gadget_connect(struct usb_gadget *gadget)
+/* Internal version of usb_gadget_connect needs to be called with connect_lock held. */
+static int usb_gadget_connect_locked(struct usb_gadget *gadget)
+ __must_hold(&gadget->udc->connect_lock)
{
int ret = 0;
@@ -706,10 +703,12 @@ int usb_gadget_connect(struct usb_gadget *gadget)
goto out;
}
- if (gadget->deactivated) {
+ if (gadget->deactivated || !gadget->udc->started) {
/*
* If gadget is deactivated we only save new state.
* Gadget will be connected automatically after activation.
+ *
+ * udc first needs to be started before gadget can be pulled up.
*/
gadget->connected = true;
goto out;
@@ -724,22 +723,32 @@ int usb_gadget_connect(struct usb_gadget *gadget)
return ret;
}
-EXPORT_SYMBOL_GPL(usb_gadget_connect);
/**
- * usb_gadget_disconnect - software-controlled disconnect from USB host
- * @gadget:the peripheral being disconnected
- *
- * Disables the D+ (or potentially D-) pullup, which the host may see
- * as a disconnect (when a VBUS session is active). Not all systems
- * support software pullup controls.
+ * usb_gadget_connect - software-controlled connect to USB host
+ * @gadget:the peripheral being connected
*
- * Following a successful disconnect, invoke the ->disconnect() callback
- * for the current gadget driver so that UDC drivers don't need to.
+ * Enables the D+ (or potentially D-) pullup. The host will start
+ * enumerating this gadget when the pullup is active and a VBUS session
+ * is active (the link is powered).
*
* Returns zero on success, else negative errno.
*/
-int usb_gadget_disconnect(struct usb_gadget *gadget)
+int usb_gadget_connect(struct usb_gadget *gadget)
+{
+ int ret;
+
+ mutex_lock(&gadget->udc->connect_lock);
+ ret = usb_gadget_connect_locked(gadget);
+ mutex_unlock(&gadget->udc->connect_lock);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(usb_gadget_connect);
+
+/* Internal version of usb_gadget_disconnect needs to be called with connect_lock held. */
+static int usb_gadget_disconnect_locked(struct usb_gadget *gadget)
+ __must_hold(&gadget->udc->connect_lock)
{
int ret = 0;
@@ -751,10 +760,12 @@ int usb_gadget_disconnect(struct usb_gadget *gadget)
if (!gadget->connected)
goto out;
- if (gadget->deactivated) {
+ if (gadget->deactivated || !gadget->udc->started) {
/*
* If gadget is deactivated we only save new state.
* Gadget will stay disconnected after activation.
+ *
+ * udc should have been started before gadget being pulled down.
*/
gadget->connected = false;
goto out;
@@ -774,6 +785,30 @@ int usb_gadget_disconnect(struct usb_gadget *gadget)
return ret;
}
+
+/**
+ * usb_gadget_disconnect - software-controlled disconnect from USB host
+ * @gadget:the peripheral being disconnected
+ *
+ * Disables the D+ (or potentially D-) pullup, which the host may see
+ * as a disconnect (when a VBUS session is active). Not all systems
+ * support software pullup controls.
+ *
+ * Following a successful disconnect, invoke the ->disconnect() callback
+ * for the current gadget driver so that UDC drivers don't need to.
+ *
+ * Returns zero on success, else negative errno.
+ */
+int usb_gadget_disconnect(struct usb_gadget *gadget)
+{
+ int ret;
+
+ mutex_lock(&gadget->udc->connect_lock);
+ ret = usb_gadget_disconnect_locked(gadget);
+ mutex_unlock(&gadget->udc->connect_lock);
+
+ return ret;
+}
EXPORT_SYMBOL_GPL(usb_gadget_disconnect);
/**
@@ -794,10 +829,11 @@ int usb_gadget_deactivate(struct usb_gadget *gadget)
if (gadget->deactivated)
goto out;
+ mutex_lock(&gadget->udc->connect_lock);
if (gadget->connected) {
- ret = usb_gadget_disconnect(gadget);
+ ret = usb_gadget_disconnect_locked(gadget);
if (ret)
- goto out;
+ goto unlock;
/*
* If gadget was being connected before deactivation, we want
@@ -807,6 +843,8 @@ int usb_gadget_deactivate(struct usb_gadget *gadget)
}
gadget->deactivated = true;
+unlock:
+ mutex_unlock(&gadget->udc->connect_lock);
out:
trace_usb_gadget_deactivate(gadget, ret);
@@ -830,6 +868,7 @@ int usb_gadget_activate(struct usb_gadget *gadget)
if (!gadget->deactivated)
goto out;
+ mutex_lock(&gadget->udc->connect_lock);
gadget->deactivated = false;
/*
@@ -837,7 +876,8 @@ int usb_gadget_activate(struct usb_gadget *gadget)
* while it was being deactivated, we call usb_gadget_connect().
*/
if (gadget->connected)
- ret = usb_gadget_connect(gadget);
+ ret = usb_gadget_connect_locked(gadget);
+ mutex_unlock(&gadget->udc->connect_lock);
out:
trace_usb_gadget_activate(gadget, ret);
@@ -1078,12 +1118,13 @@ EXPORT_SYMBOL_GPL(usb_gadget_set_state);
/* ------------------------------------------------------------------------- */
-static void usb_udc_connect_control(struct usb_udc *udc)
+/* Acquire connect_lock before calling this function. */
+static void usb_udc_connect_control_locked(struct usb_udc *udc) __must_hold(&udc->connect_lock)
{
- if (udc->vbus)
- usb_gadget_connect(udc->gadget);
+ if (udc->vbus && udc->started)
+ usb_gadget_connect_locked(udc->gadget);
else
- usb_gadget_disconnect(udc->gadget);
+ usb_gadget_disconnect_locked(udc->gadget);
}
/**
@@ -1099,10 +1140,12 @@ void usb_udc_vbus_handler(struct usb_gadget *gadget, bool status)
{
struct usb_udc *udc = gadget->udc;
+ mutex_lock(&udc->connect_lock);
if (udc) {
udc->vbus = status;
- usb_udc_connect_control(udc);
+ usb_udc_connect_control_locked(udc);
}
+ mutex_unlock(&udc->connect_lock);
}
EXPORT_SYMBOL_GPL(usb_udc_vbus_handler);
@@ -1124,7 +1167,7 @@ void usb_gadget_udc_reset(struct usb_gadget *gadget,
EXPORT_SYMBOL_GPL(usb_gadget_udc_reset);
/**
- * usb_gadget_udc_start - tells usb device controller to start up
+ * usb_gadget_udc_start_locked - tells usb device controller to start up
* @udc: The UDC to be started
*
* This call is issued by the UDC Class driver when it's about
@@ -1135,8 +1178,11 @@ EXPORT_SYMBOL_GPL(usb_gadget_udc_reset);
* necessary to have it powered on.
*
* Returns zero on success, else negative errno.
+ *
+ * Caller should acquire connect_lock before invoking this function.
*/
-static inline int usb_gadget_udc_start(struct usb_udc *udc)
+static inline int usb_gadget_udc_start_locked(struct usb_udc *udc)
+ __must_hold(&udc->connect_lock)
{
int ret;
@@ -1153,7 +1199,7 @@ static inline int usb_gadget_udc_start(struct usb_udc *udc)
}
/**
- * usb_gadget_udc_stop - tells usb device controller we don't need it anymore
+ * usb_gadget_udc_stop_locked - tells usb device controller we don't need it anymore
* @udc: The UDC to be stopped
*
* This call is issued by the UDC Class driver after calling
@@ -1162,8 +1208,11 @@ static inline int usb_gadget_udc_start(struct usb_udc *udc)
* The details are implementation specific, but it can go as
* far as powering off UDC completely and disable its data
* line pullups.
+ *
+ * Caller should acquire connect lock before invoking this function.
*/
-static inline void usb_gadget_udc_stop(struct usb_udc *udc)
+static inline void usb_gadget_udc_stop_locked(struct usb_udc *udc)
+ __must_hold(&udc->connect_lock)
{
if (!udc->started) {
dev_err(&udc->dev, "UDC had already stopped\n");
@@ -1322,6 +1371,7 @@ int usb_add_gadget(struct usb_gadget *gadget)
udc->gadget = gadget;
gadget->udc = udc;
+ mutex_init(&udc->connect_lock);
udc->started = false;
@@ -1523,11 +1573,15 @@ static int gadget_bind_driver(struct device *dev)
if (ret)
goto err_bind;
- ret = usb_gadget_udc_start(udc);
- if (ret)
+ mutex_lock(&udc->connect_lock);
+ ret = usb_gadget_udc_start_locked(udc);
+ if (ret) {
+ mutex_unlock(&udc->connect_lock);
goto err_start;
+ }
usb_gadget_enable_async_callbacks(udc);
- usb_udc_connect_control(udc);
+ usb_udc_connect_control_locked(udc);
+ mutex_unlock(&udc->connect_lock);
kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE);
return 0;
@@ -1558,12 +1612,14 @@ static void gadget_unbind_driver(struct device *dev)
kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE);
- usb_gadget_disconnect(gadget);
+ mutex_lock(&udc->connect_lock);
+ usb_gadget_disconnect_locked(gadget);
usb_gadget_disable_async_callbacks(udc);
if (gadget->irq)
synchronize_irq(gadget->irq);
udc->driver->unbind(gadget);
- usb_gadget_udc_stop(udc);
+ usb_gadget_udc_stop_locked(udc);
+ mutex_unlock(&udc->connect_lock);
mutex_lock(&udc_lock);
driver->is_bound = false;
@@ -1649,11 +1705,15 @@ static ssize_t soft_connect_store(struct device *dev,
}
if (sysfs_streq(buf, "connect")) {
- usb_gadget_udc_start(udc);
- usb_gadget_connect(udc->gadget);
+ mutex_lock(&udc->connect_lock);
+ usb_gadget_udc_start_locked(udc);
+ usb_gadget_connect_locked(udc->gadget);
+ mutex_unlock(&udc->connect_lock);
} else if (sysfs_streq(buf, "disconnect")) {
- usb_gadget_disconnect(udc->gadget);
- usb_gadget_udc_stop(udc);
+ mutex_lock(&udc->connect_lock);
+ usb_gadget_disconnect_locked(udc->gadget);
+ usb_gadget_udc_stop_locked(udc);
+ mutex_unlock(&udc->connect_lock);
} else {
dev_err(dev, "unsupported command '%s'\n", buf);
ret = -EINVAL;
base-commit: b4a4be8471846d96b0ac52a0e9e7d48005cc97e2
--
2.41.0.rc0.172.g3f132b7071-goog
chipidea udc calls usb_udc_vbus_handler from udc_start gadget
ops causing a deadlock. Avoid this by offloading usb_udc_vbus_handler
processing.
============================================
WARNING: possible recursive locking detected
640-rc1-000-devel-00005-gcda3c69ebc14 #1 Not tainted
-------------------------------------------
CPU0
----
lock(&udc->connect_lock);
lock(&udc->connect_lock);
DEADLOCK
stack backtrace:
CPU: 1 PID: 566 Comm: echo Not tainted 640-rc1-000-devel-00005-gcda3c69ebc14 #1
Hardware name: Freescale iMX7 Dual (Device Tree)
unwind_backtrace from show_stack+0x10/0x14
show_stack from dump_stack_lvl+0x70/0xb0
dump_stack_lvl from __lock_acquire+0x924/0x22c4
__lock_acquire from lock_acquire+0x100/0x370
lock_acquire from __mutex_lock+0xa8/0xfb4
__mutex_lock from mutex_lock_nested+0x1c/0x24
mutex_lock_nested from usb_udc_vbus_handler+0x1c/0x60
usb_udc_vbus_handler from ci_udc_start+0x74/0x9c
ci_udc_start from gadget_bind_driver+0x130/0x230
gadget_bind_driver from really_probe+0xd8/0x3fc
really_probe from __driver_probe_device+0x94/0x1f0
__driver_probe_device from driver_probe_device+0x2c/0xc4
driver_probe_device from __driver_attach+0x114/0x1cc
__driver_attach from bus_for_each_dev+0x7c/0xcc
bus_for_each_dev from bus_add_driver+0xd4/0x200
bus_add_driver from driver_register+0x7c/0x114
driver_register from usb_gadget_register_driver_owner+0x40/0xe0
usb_gadget_register_driver_owner from gadget_dev_desc_UDC_store+0xd4/0x110
gadget_dev_desc_UDC_store from configfs_write_iter+0xac/0x118
configfs_write_iter from vfs_write+0x1b4/0x40c
vfs_write from ksys_write+0x70/0xf8
ksys_write from ret_fast_syscall+0x0/0x1c
Fixes: 0db213ea8eed ("Revert "Revert "usb: gadget: udc: core: Prevent redundant calls to pullup""")
Cc: [email protected]
Reported-by: Stephan Gerhold <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]/
Reported-by: Francesco Dolcini <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]/
Reported-by: Alistair <[email protected]>
Closes: https://lore.kernel.org/lkml/[email protected]/
Signed-off-by: Badhri Jagan Sridharan <[email protected]>
---
Changes since v1:
- Address Alan Stern's comment on usb_udc_vbus_handler invocation from
atomic context:
* vbus_events_lock is now a spinlock and allocations in
* usb_udc_vbus_handler are atomic now.
Changes since v2:
- Addressing Alan Stern's comments:
** connect_lock is now held by callers of
* usb_gadget_pullup_update_locked() and gadget_(un)bind_driver() does
* notdirectly hold the lock.
** Both usb_gadget_(dis)connect() and usb_udc_vbus_handler() would
* set/clear udc->vbus and invoke usb_gadget_pullup_update_locked.
** Add "unbinding" to prevent new connections after the gadget is being
* unbound.
Changes since v3:
** Made a minor cleanup which I missed to do in v3 in
* usb_udc_vbus_handler().
---
drivers/usb/gadget/udc/core.c | 269 ++++++++++++++++------------------
1 file changed, 123 insertions(+), 146 deletions(-)
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 4641153e9706..26380e621e9f 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -38,9 +38,10 @@ static const struct bus_type gadget_bus_type;
* for udcs who do not care about vbus status, this value is always true
* @started: the UDC's started state. True if the UDC had started.
* @connect_lock: protects udc->vbus, udc->started, gadget->connect, gadget->deactivate related
- * functions. usb_gadget_connect_locked, usb_gadget_disconnect_locked,
- * usb_udc_connect_control_locked, usb_gadget_udc_start_locked, usb_gadget_udc_stop_locked are
- * called with this lock held.
+ * functions. usb_gadget_pullup_update_locked called with this lock held.
+ * @vbus_events: list head for processing vbus updates on usb_udc_vbus_handler.
+ * @vbus_events_lock: protects vbus_events list
+ * @vbus_work: work item that invokes usb_gadget_pullup_update_locked.
*
* This represents the internal data structure which is used by the UDC-class
* to hold information about udc driver and gadget together.
@@ -53,6 +54,20 @@ struct usb_udc {
bool vbus;
bool started;
struct mutex connect_lock;
+ struct list_head vbus_events;
+ spinlock_t vbus_events_lock;
+ struct work_struct vbus_work;
+ bool unbinding;
+};
+
+/**
+ * struct vbus_event - used to notify vbus updates posted through usb_udc_vbus_handler.
+ * @vbus_on: true when vbus is on. false other wise.
+ * @node: list node for maintaining a list of pending updates to be processed.
+ */
+struct vbus_event {
+ bool vbus_on;
+ struct list_head node;
};
static struct class *udc_class;
@@ -693,40 +708,46 @@ int usb_gadget_vbus_disconnect(struct usb_gadget *gadget)
EXPORT_SYMBOL_GPL(usb_gadget_vbus_disconnect);
/* Internal version of usb_gadget_connect needs to be called with connect_lock held. */
-static int usb_gadget_connect_locked(struct usb_gadget *gadget)
+static int usb_gadget_pullup_update_locked(struct usb_gadget *gadget)
__must_hold(&gadget->udc->connect_lock)
{
int ret = 0;
+ bool connect = !gadget->deactivated && gadget->udc->started && gadget->udc->vbus &&
+ !gadget->udc->unbinding;
if (!gadget->ops->pullup) {
ret = -EOPNOTSUPP;
goto out;
}
- if (gadget->connected)
- goto out;
-
- if (gadget->deactivated || !gadget->udc->started) {
- /*
- * If gadget is deactivated we only save new state.
- * Gadget will be connected automatically after activation.
- *
- * udc first needs to be started before gadget can be pulled up.
- */
- gadget->connected = true;
- goto out;
+ if (connect != gadget->connected) {
+ ret = gadget->ops->pullup(gadget, connect);
+ if (!ret)
+ gadget->connected = connect;
+ mutex_lock(&udc_lock);
+ if (!connect)
+ gadget->udc->driver->disconnect(gadget);
+ mutex_unlock(&udc_lock);
}
- ret = gadget->ops->pullup(gadget, 1);
- if (!ret)
- gadget->connected = 1;
-
out:
trace_usb_gadget_connect(gadget, ret);
return ret;
}
+static int usb_gadget_set_vbus(struct usb_gadget *gadget, bool vbus)
+{
+ int ret;
+
+ mutex_lock(&gadget->udc->connect_lock);
+ gadget->udc->vbus = vbus;
+ ret = usb_gadget_pullup_update_locked(gadget);
+ mutex_unlock(&gadget->udc->connect_lock);
+
+ return ret;
+}
+
/**
* usb_gadget_connect - software-controlled connect to USB host
* @gadget:the peripheral being connected
@@ -739,56 +760,10 @@ static int usb_gadget_connect_locked(struct usb_gadget *gadget)
*/
int usb_gadget_connect(struct usb_gadget *gadget)
{
- int ret;
-
- mutex_lock(&gadget->udc->connect_lock);
- ret = usb_gadget_connect_locked(gadget);
- mutex_unlock(&gadget->udc->connect_lock);
-
- return ret;
+ return usb_gadget_set_vbus(gadget, true);
}
EXPORT_SYMBOL_GPL(usb_gadget_connect);
-/* Internal version of usb_gadget_disconnect needs to be called with connect_lock held. */
-static int usb_gadget_disconnect_locked(struct usb_gadget *gadget)
- __must_hold(&gadget->udc->connect_lock)
-{
- int ret = 0;
-
- if (!gadget->ops->pullup) {
- ret = -EOPNOTSUPP;
- goto out;
- }
-
- if (!gadget->connected)
- goto out;
-
- if (gadget->deactivated || !gadget->udc->started) {
- /*
- * If gadget is deactivated we only save new state.
- * Gadget will stay disconnected after activation.
- *
- * udc should have been started before gadget being pulled down.
- */
- gadget->connected = false;
- goto out;
- }
-
- ret = gadget->ops->pullup(gadget, 0);
- if (!ret)
- gadget->connected = 0;
-
- mutex_lock(&udc_lock);
- if (gadget->udc->driver)
- gadget->udc->driver->disconnect(gadget);
- mutex_unlock(&udc_lock);
-
-out:
- trace_usb_gadget_disconnect(gadget, ret);
-
- return ret;
-}
-
/**
* usb_gadget_disconnect - software-controlled disconnect from USB host
* @gadget:the peripheral being disconnected
@@ -803,16 +778,22 @@ static int usb_gadget_disconnect_locked(struct usb_gadget *gadget)
* Returns zero on success, else negative errno.
*/
int usb_gadget_disconnect(struct usb_gadget *gadget)
+{
+ return usb_gadget_set_vbus(gadget, false);
+}
+EXPORT_SYMBOL_GPL(usb_gadget_disconnect);
+
+static int usb_gadget_set_deactivate(struct usb_gadget *gadget, bool deactivate)
{
int ret;
mutex_lock(&gadget->udc->connect_lock);
- ret = usb_gadget_disconnect_locked(gadget);
+ gadget->deactivated = deactivate;
+ ret = usb_gadget_pullup_update_locked(gadget);
mutex_unlock(&gadget->udc->connect_lock);
return ret;
}
-EXPORT_SYMBOL_GPL(usb_gadget_disconnect);
/**
* usb_gadget_deactivate - deactivate function which is not ready to work
@@ -829,26 +810,7 @@ int usb_gadget_deactivate(struct usb_gadget *gadget)
{
int ret = 0;
- if (gadget->deactivated)
- goto out;
-
- mutex_lock(&gadget->udc->connect_lock);
- if (gadget->connected) {
- ret = usb_gadget_disconnect_locked(gadget);
- if (ret)
- goto unlock;
-
- /*
- * If gadget was being connected before deactivation, we want
- * to reconnect it in usb_gadget_activate().
- */
- gadget->connected = true;
- }
- gadget->deactivated = true;
-
-unlock:
- mutex_unlock(&gadget->udc->connect_lock);
-out:
+ ret = usb_gadget_set_deactivate(gadget, true);
trace_usb_gadget_deactivate(gadget, ret);
return ret;
@@ -868,21 +830,7 @@ int usb_gadget_activate(struct usb_gadget *gadget)
{
int ret = 0;
- if (!gadget->deactivated)
- goto out;
-
- mutex_lock(&gadget->udc->connect_lock);
- gadget->deactivated = false;
-
- /*
- * If gadget has been connected before deactivation, or became connected
- * while it was being deactivated, we call usb_gadget_connect().
- */
- if (gadget->connected)
- ret = usb_gadget_connect_locked(gadget);
- mutex_unlock(&gadget->udc->connect_lock);
-
-out:
+ ret = usb_gadget_set_deactivate(gadget, false);
trace_usb_gadget_activate(gadget, ret);
return ret;
@@ -1121,13 +1069,28 @@ EXPORT_SYMBOL_GPL(usb_gadget_set_state);
/* ------------------------------------------------------------------------- */
-/* Acquire connect_lock before calling this function. */
-static void usb_udc_connect_control_locked(struct usb_udc *udc) __must_hold(&udc->connect_lock)
+static void vbus_event_work(struct work_struct *work)
{
- if (udc->vbus && udc->started)
- usb_gadget_connect_locked(udc->gadget);
- else
- usb_gadget_disconnect_locked(udc->gadget);
+ struct vbus_event *event, *n;
+ struct usb_udc *udc = container_of(work, struct usb_udc, vbus_work);
+ unsigned long flags;
+
+ spin_lock_irqsave(&udc->vbus_events_lock, flags);
+ list_for_each_entry_safe(event, n, &udc->vbus_events, node) {
+ list_del(&event->node);
+ /* OK to drop the lock here as it suffice to syncrhronize udc->vbus_events node
+ * retrieval and deletion against usb_udc_vbus_handler. usb_udc_vbus_handler does
+ * list_add_tail so n would be the same even if the lock is dropped.
+ */
+ spin_unlock_irqrestore(&udc->vbus_events_lock, flags);
+ mutex_lock(&udc->connect_lock);
+ udc->vbus = event->vbus_on;
+ usb_gadget_pullup_update_locked(udc->gadget);
+ kfree(event);
+ mutex_unlock(&udc->connect_lock);
+ spin_lock_irqsave(&udc->vbus_events_lock, flags);
+ }
+ spin_unlock_irqrestore(&udc->vbus_events_lock, flags);
}
/**
@@ -1141,14 +1104,24 @@ static void usb_udc_connect_control_locked(struct usb_udc *udc) __must_hold(&udc
*/
void usb_udc_vbus_handler(struct usb_gadget *gadget, bool status)
{
- struct usb_udc *udc = gadget->udc;
+ struct usb_udc *udc;
+ struct vbus_event *vbus_event;
+ unsigned long flags;
- mutex_lock(&udc->connect_lock);
- if (udc) {
- udc->vbus = status;
- usb_udc_connect_control_locked(udc);
- }
- mutex_unlock(&udc->connect_lock);
+ if (!gadget || !gadget->udc)
+ return;
+
+ udc = gadget->udc;
+
+ vbus_event = kzalloc(sizeof(*vbus_event), GFP_ATOMIC);
+ if (!vbus_event)
+ return;
+
+ spin_lock_irqsave(&udc->vbus_events_lock, flags);
+ vbus_event->vbus_on = status;
+ list_add_tail(&vbus_event->node, &udc->vbus_events);
+ spin_unlock_irqrestore(&udc->vbus_events_lock, flags);
+ schedule_work(&udc->vbus_work);
}
EXPORT_SYMBOL_GPL(usb_udc_vbus_handler);
@@ -1170,7 +1143,7 @@ void usb_gadget_udc_reset(struct usb_gadget *gadget,
EXPORT_SYMBOL_GPL(usb_gadget_udc_reset);
/**
- * usb_gadget_udc_start_locked - tells usb device controller to start up
+ * usb_gadget_udc_start - tells usb device controller to start up
* @udc: The UDC to be started
*
* This call is issued by the UDC Class driver when it's about
@@ -1181,11 +1154,8 @@ EXPORT_SYMBOL_GPL(usb_gadget_udc_reset);
* necessary to have it powered on.
*
* Returns zero on success, else negative errno.
- *
- * Caller should acquire connect_lock before invoking this function.
*/
-static inline int usb_gadget_udc_start_locked(struct usb_udc *udc)
- __must_hold(&udc->connect_lock)
+static inline int usb_gadget_udc_start(struct usb_udc *udc)
{
int ret;
@@ -1194,15 +1164,17 @@ static inline int usb_gadget_udc_start_locked(struct usb_udc *udc)
return -EBUSY;
}
+ mutex_lock(&udc->connect_lock);
ret = udc->gadget->ops->udc_start(udc->gadget, udc->driver);
if (!ret)
udc->started = true;
+ mutex_unlock(&udc->connect_lock);
return ret;
}
/**
- * usb_gadget_udc_stop_locked - tells usb device controller we don't need it anymore
+ * usb_gadget_udc_stop - tells usb device controller we don't need it anymore
* @udc: The UDC to be stopped
*
* This call is issued by the UDC Class driver after calling
@@ -1211,19 +1183,18 @@ static inline int usb_gadget_udc_start_locked(struct usb_udc *udc)
* The details are implementation specific, but it can go as
* far as powering off UDC completely and disable its data
* line pullups.
- *
- * Caller should acquire connect lock before invoking this function.
*/
-static inline void usb_gadget_udc_stop_locked(struct usb_udc *udc)
- __must_hold(&udc->connect_lock)
+static inline void usb_gadget_udc_stop(struct usb_udc *udc)
{
if (!udc->started) {
dev_err(&udc->dev, "UDC had already stopped\n");
return;
}
+ mutex_lock(&udc->connect_lock);
udc->gadget->ops->udc_stop(udc->gadget);
udc->started = false;
+ mutex_unlock(&udc->connect_lock);
}
/**
@@ -1362,6 +1333,7 @@ int usb_add_gadget(struct usb_gadget *gadget)
if (!udc)
goto error;
+ udc->unbinding = true;
device_initialize(&udc->dev);
udc->dev.release = usb_udc_release;
udc->dev.class = udc_class;
@@ -1375,6 +1347,9 @@ int usb_add_gadget(struct usb_gadget *gadget)
udc->gadget = gadget;
gadget->udc = udc;
mutex_init(&udc->connect_lock);
+ INIT_LIST_HEAD(&udc->vbus_events);
+ spin_lock_init(&udc->vbus_events_lock);
+ INIT_WORK(&udc->vbus_work, vbus_event_work);
udc->started = false;
@@ -1474,6 +1449,17 @@ char *usb_get_gadget_udc_name(void)
}
EXPORT_SYMBOL_GPL(usb_get_gadget_udc_name);
+static int usb_gadget_set_unbinding(struct usb_udc *udc, bool status)
+{
+ int ret;
+
+ mutex_lock(&udc->connect_lock);
+ udc->unbinding = status;
+ ret = usb_gadget_pullup_update_locked(udc->gadget);
+ mutex_unlock(&udc->connect_lock);
+
+ return ret;
+}
/**
* usb_add_gadget_udc - adds a new gadget to the udc class driver list
* @parent: the parent device to this udc. Usually the controller
@@ -1576,15 +1562,11 @@ static int gadget_bind_driver(struct device *dev)
if (ret)
goto err_bind;
- mutex_lock(&udc->connect_lock);
- ret = usb_gadget_udc_start_locked(udc);
- if (ret) {
- mutex_unlock(&udc->connect_lock);
+ ret = usb_gadget_udc_start(udc);
+ if (ret)
goto err_start;
- }
usb_gadget_enable_async_callbacks(udc);
- usb_udc_connect_control_locked(udc);
- mutex_unlock(&udc->connect_lock);
+ usb_gadget_set_unbinding(udc, false);
kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE);
return 0;
@@ -1615,14 +1597,13 @@ static void gadget_unbind_driver(struct device *dev)
kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE);
- mutex_lock(&udc->connect_lock);
- usb_gadget_disconnect_locked(gadget);
+ usb_gadget_set_unbinding(udc, true);
+ cancel_work_sync(&udc->vbus_work);
usb_gadget_disable_async_callbacks(udc);
if (gadget->irq)
synchronize_irq(gadget->irq);
udc->driver->unbind(gadget);
- usb_gadget_udc_stop_locked(udc);
- mutex_unlock(&udc->connect_lock);
+ usb_gadget_udc_stop(udc);
mutex_lock(&udc_lock);
driver->is_bound = false;
@@ -1708,15 +1689,11 @@ static ssize_t soft_connect_store(struct device *dev,
}
if (sysfs_streq(buf, "connect")) {
- mutex_lock(&udc->connect_lock);
- usb_gadget_udc_start_locked(udc);
- usb_gadget_connect_locked(udc->gadget);
- mutex_unlock(&udc->connect_lock);
+ usb_gadget_udc_start(udc);
+ usb_udc_vbus_handler(udc->gadget, true);
} else if (sysfs_streq(buf, "disconnect")) {
- mutex_lock(&udc->connect_lock);
- usb_gadget_disconnect_locked(udc->gadget);
- usb_gadget_udc_stop_locked(udc);
- mutex_unlock(&udc->connect_lock);
+ usb_udc_vbus_handler(udc->gadget, false);
+ usb_gadget_udc_stop(udc);
} else {
dev_err(dev, "unsupported command '%s'\n", buf);
ret = -EINVAL;
--
2.41.0.rc0.172.g3f132b7071-goog
This reverts commit 5e1617210aede9f1b91bb9819c93097b6da481f9.
The regression reported in
https://lore.kernel.org/all/[email protected]/ is being
fixed in
commit 7d7863db7cc0 ("usb: gadget: udc: core: Offload usb_udc_vbus_handler processing").
Hence reverting the revert.
Signed-off-by: Badhri Jagan Sridharan <[email protected]>
---
drivers/usb/gadget/udc/core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 583c339876ab..4641153e9706 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -703,6 +703,9 @@ static int usb_gadget_connect_locked(struct usb_gadget *gadget)
goto out;
}
+ if (gadget->connected)
+ goto out;
+
if (gadget->deactivated || !gadget->udc->started) {
/*
* If gadget is deactivated we only save new state.
--
2.41.0.rc0.172.g3f132b7071-goog
On Mon, May 29, 2023 at 11:48:14PM +0000, Badhri Jagan Sridharan wrote:
> This reverts commit f22e9b67f19ccc73de1ae04375d4b30684e261f8.
>
> The regression reported in
> https://lore.kernel.org/all/[email protected]/ is being
> fixed in
> commit 7d7863db7cc0 ("usb: gadget: udc: core: Offload usb_udc_vbus_handler processing").
What commit is that? It doesn't exist yet, at least, not in the
mainline kernel.
> Hence reverting the revert.
>
> Signed-off-by: Badhri Jagan Sridharan <[email protected]>
No! Do not do this. If you do, there will again be a version of the
kernel that has the bug that caused the revert in the first place. Even
if it's only temporary, it could still affect people who are (for
example) trying to run bisections.
Instead, reorder the patches. First fix the underlying problem that
led to the deadlocks. Once that's in good shape then you can safely
make this change.
Alan Stern
On Mon, May 29, 2023 at 11:48:15PM +0000, Badhri Jagan Sridharan wrote:
> This reverts commit 5e1617210aede9f1b91bb9819c93097b6da481f9.
>
> The regression reported in
> https://lore.kernel.org/all/[email protected]/ is being
> fixed in
> commit 7d7863db7cc0 ("usb: gadget: udc: core: Offload usb_udc_vbus_handler processing").
> Hence reverting the revert.
>
> Signed-off-by: Badhri Jagan Sridharan <[email protected]>
> ---
> drivers/usb/gadget/udc/core.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
> index 583c339876ab..4641153e9706 100644
> --- a/drivers/usb/gadget/udc/core.c
> +++ b/drivers/usb/gadget/udc/core.c
> @@ -703,6 +703,9 @@ static int usb_gadget_connect_locked(struct usb_gadget *gadget)
> goto out;
> }
>
> + if (gadget->connected)
> + goto out;
> +
> if (gadget->deactivated || !gadget->udc->started) {
> /*
> * If gadget is deactivated we only save new state.
This is silly. There's no need to make this a separate commit; it
should be merged in with the preceding patch. There's no good reason
for creating a commit that contains a known error.
Alan Stern
On Mon, May 29, 2023 at 08:42:18PM -0400, Alan Stern wrote:
> On Mon, May 29, 2023 at 11:48:14PM +0000, Badhri Jagan Sridharan wrote:
> > This reverts commit f22e9b67f19ccc73de1ae04375d4b30684e261f8.
This is not the format we use for referring to commits.
> >
> > The regression reported in
> > https://lore.kernel.org/all/[email protected]/ is being
> > fixed in
> > commit 7d7863db7cc0 ("usb: gadget: udc: core: Offload usb_udc_vbus_handler processing").
That is the correct format.
> What commit is that? It doesn't exist yet, at least, not in the
> mainline kernel.
>
> > Hence reverting the revert.
> >
> > Signed-off-by: Badhri Jagan Sridharan <[email protected]>
>
> No! Do not do this. If you do, there will again be a version of the
> kernel that has the bug that caused the revert in the first place. Even
> if it's only temporary, it could still affect people who are (for
> example) trying to run bisections.
>
> Instead, reorder the patches. First fix the underlying problem that
> led to the deadlocks. Once that's in good shape then you can safely
> make this change.
I forgot to mention... When you do eventually resubmit this, do NOT use
the commit message above. It says absolutely nothing about what the
patch actually does or why it is needed.
It's okay to mention that this reinstates something that had to be
reverted. But you also need to include the information that was in the
original commit.
Alan Stern
On Mon, May 29, 2023 at 11:48:16PM +0000, Badhri Jagan Sridharan wrote:
> chipidea udc calls usb_udc_vbus_handler from udc_start gadget
> ops causing a deadlock. Avoid this by offloading usb_udc_vbus_handler
> processing.
This is not a good explanation. In particular, it doesn't explain why
moving the processing to a workqueue is the proper solution.
You should describe the issue I raised earlier, namely, that
usb_udc_vbus_handler() has to run in interrupt context but it calls
usb_udc_connect_control(), which has to run in process context. And
explain _why_ these routines have to run in those contexts.
> ---
> drivers/usb/gadget/udc/core.c | 269 ++++++++++++++++------------------
> 1 file changed, 123 insertions(+), 146 deletions(-)
>
> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
> index 4641153e9706..26380e621e9f 100644
> --- a/drivers/usb/gadget/udc/core.c
> +++ b/drivers/usb/gadget/udc/core.c
> @@ -38,9 +38,10 @@ static const struct bus_type gadget_bus_type;
> * for udcs who do not care about vbus status, this value is always true
> * @started: the UDC's started state. True if the UDC had started.
> * @connect_lock: protects udc->vbus, udc->started, gadget->connect, gadget->deactivate related
> - * functions. usb_gadget_connect_locked, usb_gadget_disconnect_locked,
> - * usb_udc_connect_control_locked, usb_gadget_udc_start_locked, usb_gadget_udc_stop_locked are
> - * called with this lock held.
> + * functions. usb_gadget_pullup_update_locked called with this lock held.
> + * @vbus_events: list head for processing vbus updates on usb_udc_vbus_handler.
> + * @vbus_events_lock: protects vbus_events list
> + * @vbus_work: work item that invokes usb_gadget_pullup_update_locked.
> *
> * This represents the internal data structure which is used by the UDC-class
> * to hold information about udc driver and gadget together.
> @@ -53,6 +54,20 @@ struct usb_udc {
> bool vbus;
> bool started;
> struct mutex connect_lock;
> + struct list_head vbus_events;
> + spinlock_t vbus_events_lock;
> + struct work_struct vbus_work;
Do you really need three new fields here? Isn't vbus_work sufficient?
> + bool unbinding;
Do not include this in the same patch. The unbinding flag does
something different from the vbus_work structure, so it belongs in a
different patch.
> +};
> +
> +/**
> + * struct vbus_event - used to notify vbus updates posted through usb_udc_vbus_handler.
> + * @vbus_on: true when vbus is on. false other wise.
> + * @node: list node for maintaining a list of pending updates to be processed.
> + */
> +struct vbus_event {
> + bool vbus_on;
> + struct list_head node;
> };
Why do we need this? Why can't the work routine simply set the pullup
according to the current setting of vbus and the other flags? That's
what usb_udc_vbus_handler() does now.
>
> static struct class *udc_class;
> @@ -693,40 +708,46 @@ int usb_gadget_vbus_disconnect(struct usb_gadget *gadget)
> EXPORT_SYMBOL_GPL(usb_gadget_vbus_disconnect);
>
> /* Internal version of usb_gadget_connect needs to be called with connect_lock held. */
> -static int usb_gadget_connect_locked(struct usb_gadget *gadget)
> +static int usb_gadget_pullup_update_locked(struct usb_gadget *gadget)
> __must_hold(&gadget->udc->connect_lock)
> {
> int ret = 0;
> + bool connect = !gadget->deactivated && gadget->udc->started && gadget->udc->vbus &&
> + !gadget->udc->unbinding;
Since you are wrapping this line anyway, you might as well wrap it
before column 76.
>
> if (!gadget->ops->pullup) {
> ret = -EOPNOTSUPP;
> goto out;
> }
>
> - if (gadget->connected)
> - goto out;
> -
> - if (gadget->deactivated || !gadget->udc->started) {
> - /*
> - * If gadget is deactivated we only save new state.
> - * Gadget will be connected automatically after activation.
> - *
> - * udc first needs to be started before gadget can be pulled up.
> - */
> - gadget->connected = true;
> - goto out;
> + if (connect != gadget->connected) {
> + ret = gadget->ops->pullup(gadget, connect);
> + if (!ret)
> + gadget->connected = connect;
> + mutex_lock(&udc_lock);
> + if (!connect)
> + gadget->udc->driver->disconnect(gadget);
> + mutex_unlock(&udc_lock);
> }
What will happen if the gadget isn't deactivated, but it is started and
VBUS power is prevent and the driver isn't unbinding, but the gadget
driver decides to call usb_gadget_disconnect()?
>
> - ret = gadget->ops->pullup(gadget, 1);
> - if (!ret)
> - gadget->connected = 1;
> -
> out:
> trace_usb_gadget_connect(gadget, ret);
>
> return ret;
> }
>
> +static int usb_gadget_set_vbus(struct usb_gadget *gadget, bool vbus)
> +{
> + int ret;
> +
> + mutex_lock(&gadget->udc->connect_lock);
> + gadget->udc->vbus = vbus;
Why does this have to be here? What's wrong with setting vbus in
interrupt context?
> + ret = usb_gadget_pullup_update_locked(gadget);
> + mutex_unlock(&gadget->udc->connect_lock);
Sorry, but at this point I'm getting tired of pointing out all the
problems in this patch, so I'm going to stop here.
How about instead doing something really simple, like just make
usb_udc_vbus_handler() queue up a work routine that calls
usb_udc_connect_control()? Just a minimal change that will be really
easy to review.
Alan Stern
On Mon, May 29, 2023 at 11:48:14PM +0000, Badhri Jagan Sridharan wrote:
> This reverts commit f22e9b67f19ccc73de1ae04375d4b30684e261f8.
reverts of reverts aren't good. Just submit the real fix please.
thanks,
greg k-h
On Mon, May 29, 2023 at 6:08 PM Alan Stern <[email protected]> wrote:
>
> On Mon, May 29, 2023 at 11:48:16PM +0000, Badhri Jagan Sridharan wrote:
> > chipidea udc calls usb_udc_vbus_handler from udc_start gadget
> > ops causing a deadlock. Avoid this by offloading usb_udc_vbus_handler
> > processing.
>
> This is not a good explanation. In particular, it doesn't explain why
> moving the processing to a workqueue is the proper solution.
>
> You should describe the issue I raised earlier, namely, that
> usb_udc_vbus_handler() has to run in interrupt context but it calls
> usb_udc_connect_control(), which has to run in process context. And
> explain _why_ these routines have to run in those contexts.
>
> > ---
> > drivers/usb/gadget/udc/core.c | 269 ++++++++++++++++------------------
> > 1 file changed, 123 insertions(+), 146 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
> > index 4641153e9706..26380e621e9f 100644
> > --- a/drivers/usb/gadget/udc/core.c
> > +++ b/drivers/usb/gadget/udc/core.c
> > @@ -38,9 +38,10 @@ static const struct bus_type gadget_bus_type;
> > * for udcs who do not care about vbus status, this value is always true
> > * @started: the UDC's started state. True if the UDC had started.
> > * @connect_lock: protects udc->vbus, udc->started, gadget->connect, gadget->deactivate related
> > - * functions. usb_gadget_connect_locked, usb_gadget_disconnect_locked,
> > - * usb_udc_connect_control_locked, usb_gadget_udc_start_locked, usb_gadget_udc_stop_locked are
> > - * called with this lock held.
> > + * functions. usb_gadget_pullup_update_locked called with this lock held.
> > + * @vbus_events: list head for processing vbus updates on usb_udc_vbus_handler.
> > + * @vbus_events_lock: protects vbus_events list
> > + * @vbus_work: work item that invokes usb_gadget_pullup_update_locked.
> > *
> > * This represents the internal data structure which is used by the UDC-class
> > * to hold information about udc driver and gadget together.
> > @@ -53,6 +54,20 @@ struct usb_udc {
> > bool vbus;
> > bool started;
> > struct mutex connect_lock;
> > + struct list_head vbus_events;
> > + spinlock_t vbus_events_lock;
> > + struct work_struct vbus_work;
>
> Do you really need three new fields here? Isn't vbus_work sufficient?
Ack. Just the vbus_work is sufficient as vbus can be updated to the
latest value.
Addressing in v5.
>
> > + bool unbinding;
>
> Do not include this in the same patch. The unbinding flag does
> something different from the vbus_work structure, so it belongs in a
> different patch.
Sure, uploaded as a separate patch in v5.
However, I named it allow_start instead as I believe that UDC should
neither be started nor pulled up when unbound.
Let me know your thoughts in v5 !
>
> > +};
> > +
> > +/**
> > + * struct vbus_event - used to notify vbus updates posted through usb_udc_vbus_handler.
> > + * @vbus_on: true when vbus is on. false other wise.
> > + * @node: list node for maintaining a list of pending updates to be processed.
> > + */
> > +struct vbus_event {
> > + bool vbus_on;
> > + struct list_head node;
> > };
>
> Why do we need this? Why can't the work routine simply set the pullup
> according to the current setting of vbus and the other flags? That's
> what usb_udc_vbus_handler() does now.
Ack. Dropping vbus_event and related fields.
>
> >
> > static struct class *udc_class;
> > @@ -693,40 +708,46 @@ int usb_gadget_vbus_disconnect(struct usb_gadget *gadget)
> > EXPORT_SYMBOL_GPL(usb_gadget_vbus_disconnect);
> >
> > /* Internal version of usb_gadget_connect needs to be called with connect_lock held. */
> > -static int usb_gadget_connect_locked(struct usb_gadget *gadget)
> > +static int usb_gadget_pullup_update_locked(struct usb_gadget *gadget)
> > __must_hold(&gadget->udc->connect_lock)
> > {
> > int ret = 0;
> > + bool connect = !gadget->deactivated && gadget->udc->started && gadget->udc->vbus &&
> > + !gadget->udc->unbinding;
>
> Since you are wrapping this line anyway, you might as well wrap it
> before column 76.
>
> >
> > if (!gadget->ops->pullup) {
> > ret = -EOPNOTSUPP;
> > goto out;
> > }
> >
> > - if (gadget->connected)
> > - goto out;
> > -
> > - if (gadget->deactivated || !gadget->udc->started) {
> > - /*
> > - * If gadget is deactivated we only save new state.
> > - * Gadget will be connected automatically after activation.
> > - *
> > - * udc first needs to be started before gadget can be pulled up.
> > - */
> > - gadget->connected = true;
> > - goto out;
> > + if (connect != gadget->connected) {
> > + ret = gadget->ops->pullup(gadget, connect);
> > + if (!ret)
> > + gadget->connected = connect;
> > + mutex_lock(&udc_lock);
> > + if (!connect)
> > + gadget->udc->driver->disconnect(gadget);
> > + mutex_unlock(&udc_lock);
> > }
>
> What will happen if the gadget isn't deactivated, but it is started and
> VBUS power is prevent and the driver isn't unbinding, but the gadget
> driver decides to call usb_gadget_disconnect()?
Simplified as you recommended to directly call
usb_udc_connect_control() from the work item. So, this is no longer an
issue.
>
> >
> > - ret = gadget->ops->pullup(gadget, 1);
> > - if (!ret)
> > - gadget->connected = 1;
> > -
> > out:
> > trace_usb_gadget_connect(gadget, ret);
> >
> > return ret;
> > }
> >
> > +static int usb_gadget_set_vbus(struct usb_gadget *gadget, bool vbus)
> > +{
> > + int ret;
> > +
> > + mutex_lock(&gadget->udc->connect_lock);
> > + gadget->udc->vbus = vbus;
>
> Why does this have to be here? What's wrong with setting vbus in
> interrupt context?
>
> > + ret = usb_gadget_pullup_update_locked(gadget);
> > + mutex_unlock(&gadget->udc->connect_lock);
>
> Sorry, but at this point I'm getting tired of pointing out all the
> problems in this patch, so I'm going to stop here.
>
> How about instead doing something really simple, like just make
> usb_udc_vbus_handler() queue up a work routine that calls
> usb_udc_connect_control()? Just a minimal change that will be really
> easy to review.
Ack. v5 now does this.
Thanks for all the feedback,
Badhri
>
> Alan Stern