hi, On Mon, Jun 25, 2012 at 10:00:13PM +0530, Pratyush Anand wrote: > On 6/25/2012 9:48 PM, Felipe Balbi wrote: > >Hi, > > > >On Mon, Jun 25, 2012 at 08:04:08PM +0530, Pratyush Anand wrote: > >>On 6/25/2012 7:41 PM, Felipe Balbi wrote: > >>>whenever we want to stall ep0, we always call > >>>dwc3_ep0_stall_and_restart() which makes sure > >> > >>Probably this is not correct. A control ep can also be stalled by > >>calling set_halt. > >> > >>So we can have > >>dwc3_gadget_ep0_ops.set_halt as dwc3_ep0_stall_and_restart or some > >>similar function. > > > >I don't think we should allow gadget driver to directly stall ep0. I > >don't see the point because ep0 stall should be done only in case of a > >bad ctrl request or something which we found to be bogus and that we > >capture through the return of ->setup(). > > Not necessarily I think, > > > I looked into include/linux/usb/gadget.h > > * > * Control endpoints ... after getting a setup() callback, the driver > queues > * one response (even if it would be zero length). That enables the > * status ack, after transferring data as specified in the response. Setup > * functions may return negative error codes to generate protocol stalls. > * (Note that some USB device controllers disallow protocol stall responses > * in some cases.) When control responses are deferred (the response is > * written after the setup callback returns), then usb_ep_set_halt() may be > * used on ep0 to trigger protocol stalls. Depending on the controller, > * it may not be possible to trigger a status-stage protocol stall when the > * data stage is over, that is, from within the response's completion > * routine. good point. Then making stall_and_restart() ep0's set_halt() method is the way to go. Thanks for debugging this one. -- balbi
Attachment:
signature.asc
Description: Digital signature
