On 06/29/2018 08:32 AM, Felipe Balbi wrote: > > Hi, > > Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> writes: >> On Thu, 21 Jun 2018, Roger Quadros wrote: >> >>>>> Can we avoid the spin_lock() and the work-queue and call usb_ep_dequeue() directly from here? >>>>>> What is the purpose of the spin_lock()? >>>> >>>> I agree that the lock doesn't seem to be necessary. But I believe the whole >>>> thing is already running in non-sleeping context, even before the spinlock >>>> is taken. So this wouldn't help much. >>>> >>>> Even the io_cancel() syscall takes a spinlock before invoking the cancel >>>> function. So this issue is not exclusive to program termination. >>>> >>>> Are there any documented guidelines on which context usb_ep_dequeue() should >>>> be able to be called in? The sleep in the dwc3 driver seems to be a recent >>>> addition. >>> >>> drivers/usb/udc/gadget/core.c has the only documentation, but context is not mentioned there. >>> Felipe, what do you suggest? >> >> As far as I remember, usb_ep_dequeue() is supposed to be more or less >> analogous to usb_ep_queue(); drivers should be allowed to call either >> routine in an atomic context. > > Hmm, that's what I remember, but we don't have that documented and dwc3 > has a sleep in its dequeue, which I need to remove for other reasons > anyway. > > Can we get a patch updating documentation to make it clear that both > queue and dequeue should be callable from any context? > Felipe, for the DWC3 do you see an issue with moving the giveback and the cleanup of the TRBs simply into END_TRANSFER interrupt? That seems to work for me for fixing the issue, but I don't have a full understanding of all the dependencies of the DWC3 gadget driver and don't know if it could introduce other issues.
