On Mon, Aug 24, 2015 at 10:53:50AM -0500, Felipe Balbi wrote:
> On Mon, Aug 24, 2015 at 10:51:04AM -0400, Alan Stern wrote:
> > On Mon, 24 Aug 2015, Peter Chen wrote:
> > 
> > > Thanks, that's much clear.
> > > 
> > > At udc driver:
> > > 
> > > __set_halt(struct usb_ep *ep, int value, bool may_fail)
> > > {
> > >   if (may_fail && ep queue is not empty) {
> > >           return false
> > >   } else {
> > >           do stall;
> > >           return true;
> > >   }
> > > }
> > > 
> > > gadget_ops:
> > > .set_halt      = ep_set_halt,
> > > 
> > > ep_set_halt(struct usb_ep *ep, int value)
> > > {
> > >   __set_halt(ep, value, true);
> > > }
> > > 
> > > And call __set_halt(ep, value, false) at below conditions:
> > > - SET(CLEAR)_FEATURE for Set(Clear)-Halt
> > > - If ep0 request has failed
> > 
> > Yes, that should work.  In fact, when a control request fails, you 
> > could even call ep_set_halt instead of __set_halt, because the ep0 
> > queue will certainly be empty.
> 
> but you'll already hold controller's lock. ->set_halt() expects lock to
> be held. Since you only know of a failing request from within EP0 IRQ
> handler (well, you could defer to a tasklet or workqueue, or whatever),
> you'll already hold controller's lock and you'd end up with recursive
> locking errors.
> 

Other EPs may call ->set_halt() at its completion, it is at interrupt
handler too, we may unlock first for both cases.

-- 

Best Regards,
Peter Chen
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to