[PATCH v5 1/3] usb: gadget: udc: core: Offload usb_udc_vbus_handler processing

From: Badhri Jagan Sridharan
Date: Wed May 31 2023 - 00:02:20 EST


usb_udc_vbus_handler() can be invoked from interrupt context by irq
handlers of the gadget drivers, however, usb_udc_connect_control() has
to run in non-atomic context due to the following:
a. Some of the gadget driver implementations expect the ->pullup
callback to be invoked in non-atomic context.
b. usb_gadget_disconnect() acquires udc_lock which is a mutex.

Hence offload invocation of usb_udc_connect_control()
to workqueue.

Cc: stable@xxxxxxxxxxxxxxx
Fixes: 1016fc0c096c ("USB: gadget: Fix obscure lockdep violation for udc_mutex")
Signed-off-by: Badhri Jagan Sridharan <badhri@xxxxxxxxxx>
---
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().

Changes since v4:
- Addressing Alan Stern's comments:
** usb_udc_vbus_handler() now offloads invocation of usb_udc_connect_control()
* from workqueue.

** Dropped vbus_events list as this was redundant. Updating to the
* latest value is suffice
---
drivers/usb/gadget/udc/core.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
index 52e6d2e84e35..44a9f32679b5 100644
--- a/drivers/usb/gadget/udc/core.c
+++ b/drivers/usb/gadget/udc/core.c
@@ -48,6 +48,7 @@ struct usb_udc {
struct list_head list;
bool vbus;
bool started;
+ struct work_struct vbus_work;
};

static struct class *udc_class;
@@ -1086,6 +1087,13 @@ static void usb_udc_connect_control(struct usb_udc *udc)
usb_gadget_disconnect(udc->gadget);
}

+static void vbus_event_work(struct work_struct *work)
+{
+ struct usb_udc *udc = container_of(work, struct usb_udc, vbus_work);
+
+ usb_udc_connect_control(udc);
+}
+
/**
* usb_udc_vbus_handler - updates the udc core vbus status, and try to
* connect or disconnect gadget
@@ -1094,6 +1102,13 @@ static void usb_udc_connect_control(struct usb_udc *udc)
*
* The udc driver calls it when it wants to connect or disconnect gadget
* according to vbus status.
+ *
+ * This function can be invoked from interrupt context by irq handlers of the gadget drivers,
+ * however, usb_udc_connect_control() has to run in non-atomic context due to the following:
+ * a. Some of the gadget driver implementations expect the ->pullup callback to be invoked in
+ * non-atomic context.
+ * b. usb_gadget_disconnect() acquires udc_lock which is a mutex.
+ * Hence offload invocation of usb_udc_connect_control() to workqueue.
*/
void usb_udc_vbus_handler(struct usb_gadget *gadget, bool status)
{
@@ -1101,7 +1116,7 @@ void usb_udc_vbus_handler(struct usb_gadget *gadget, bool status)

if (udc) {
udc->vbus = status;
- usb_udc_connect_control(udc);
+ schedule_work(&udc->vbus_work);
}
}
EXPORT_SYMBOL_GPL(usb_udc_vbus_handler);
@@ -1328,6 +1343,7 @@ int usb_add_gadget(struct usb_gadget *gadget)
mutex_lock(&udc_lock);
list_add_tail(&udc->list, &udc_list);
mutex_unlock(&udc_lock);
+ INIT_WORK(&udc->vbus_work, vbus_event_work);

ret = device_add(&udc->dev);
if (ret)
@@ -1558,6 +1574,7 @@ static void gadget_unbind_driver(struct device *dev)

kobject_uevent(&udc->dev.kobj, KOBJ_CHANGE);

+ cancel_work_sync(&udc->vbus_work);
usb_gadget_disconnect(gadget);
usb_gadget_disable_async_callbacks(udc);
if (gadget->irq)

base-commit: 046895105d9666ab56e86ce8dd9786f8003125c6
--
2.41.0.rc0.172.g3f132b7071-goog