Hi, On Wed, Sep 28, 2011 at 02:41:24PM -0400, Alan Stern wrote: > > On Wed, Sep 28, 2011 at 10:53:44AM -0700, Paul Zimmerman wrote: > > > > From: Felipe Balbi [mailto:balbi@xxxxxx] > > > > Sent: Wednesday, September 28, 2011 3:28 AM > > > > > > > > On Tue, Sep 27, 2011 at 10:52:39PM -0700, Paul Zimmerman wrote: > > > > > This makes DWC3_EP_WEDGE do the right thing (I think) > > > > > > > > > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > > > > > index 29d9e4a..5283956 100644 > > > > > --- a/drivers/usb/dwc3/gadget.c > > > > > +++ b/drivers/usb/dwc3/gadget.c > > > > > @@ -873,6 +873,9 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value) > > > > > struct dwc3 *dwc = dep->dwc; > > > > > int number, ret; > > > > > > > > > > + if (!value && (dep->flags & DWC3_EP_WEDGE)) > > > > > + return 0; > > > > > + > > > > > memset(¶ms, 0x00, sizeof(params)); > > > > > > > > > > if (dep->number == 1) > > > > > @@ -928,6 +931,9 @@ static int dwc3_gadget_ep_set_halt(struct usb_ep *ep, int value) > > > > > goto out; > > > > > } > > > > > > > > > > + if (!value && (dep->flags & DWC3_EP_WEDGE)) > > > > > + dep->flags &= ~DWC3_EP_WEDGE; > > > > > > > > wouldn't this be enough ?? > > > > > > > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > > > > index b2820aa..79aa093 100644 > > > > --- a/drivers/usb/dwc3/gadget.c > > > > +++ b/drivers/usb/dwc3/gadget.c > > > > @@ -897,7 +897,7 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value) > > > > value ? "set" : "clear", > > > > dep->name); > > > > else > > > > - dep->flags &= ~DWC3_EP_STALL; > > > > + dep->flags &= ~(DWC3_EP_STALL | DWC3_EP_WEDGE); > > > > } > > > > return ret; > > > > } > > > > > > Yes, that would be enough for clearing the flag. You still need the first > > > hunk, though, so the flag actually does something. > > > > first hunk will prevent that flag from ever being cleared, no ? > > According to documentation > > include/linux/usb/gadget.h::usb_gadget_set_wedge(), it should prevent > > ClearFeature(HALT) from succeding, but gadget driver still should be > > able to clear that flag. > > > > Unless I'm missing something, I think your first hunk is also wrong, as > > it would prevent WEDGE and STALL flags from ever being cleared after > > they are set. > > No, that is correct. Once the WEDGE flag is set, the only way to clear > it is by doing something drastic, like resetting the gadget or > disabling and re-enabling the endpoint (as in Set-Interface or > Set-Config). > > This is the required behavior for the Mass Storage bulk-only transport > class, as described in section 6.6.1 of that spec. Sorry but what's correct ? My rationale or original patch ? -- balbi
Attachment:
signature.asc
Description: Digital signature
