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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
