Skip to contentSkip to conversation

usb: gadget: udc: core: update usb_ep_queue() documentation

PATCH New Felipe Balbi 1 participant 1 message Mar 27, 2018 USB

lore
Share & tools
lore
#0 lore raw

PATCH 1/2 usb: gadget: udc: core: update usb_ep_queue() documentation

Hi,

Quoted 27 lines · Alan Stern
Quoted 13 lines · Felipe Balbi

Mention that ->complete() should never be called from within usb_ep_queue().

---
 drivers/usb/gadget/udc/core.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/usb/gadget/udc/core.c +0 -0
index 50988b21a21b..842814bc0e4f 100644

drivers/usb/gadget/udc/core.c EXPORT_SYMBOL_GPL(usb_ep_free_request); · line 243

Quoted 6 lines
drivers/usb/gadget/udc/core.c +3 -0
@@ -238,6 +238,9 @@ EXPORT_SYMBOL_GPL(usb_ep_free_request);
  * arranges to poll once per interval, and the gadget driver usually will
  * have queued some data to transfer at that time.
  *
+ * Note that @req's ->complete() callback must never be called from
+ * within usb_ep_queue() as that can create deadlock situations.
+ *

I think this is highly questionable. Certainly it was not David Brownell's original intention; his dummy-hcd driver will sometimes give back a request from within usb_ep_queue() -- and I believe he wrote it that way in order to emulate a feature of his net2280 driver.

Quoted 5 lines

In this particular case, the problem is that a driver acquires a spinlock in its complete() routine, but then it holds that same spinlock while submitting a request. This is a bug; it should be fixed in the driver. The spinlock should be dropped while the request is submitted. I'm sure there are examples whether other drivers do this.

usb_ep_queue() can be called from atomic, there's no explicit requirement that locks should be released. Either one case or the other should be made explicit.