Re: [PATCH 1/5] usb: dwc3: fix DEPSTARTCFG for non-EP0 EPs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Wed, Sep 28, 2011 at 11:40:08AM -0700, Paul Zimmerman wrote:
> > From: Felipe Balbi [mailto:balbi@xxxxxx]
> > Sent: Wednesday, September 28, 2011 11:18 AM
> > 
> > On Wed, Sep 28, 2011 at 11:12:55AM -0700, Paul Zimmerman wrote:
> > > > > @@ -1161,6 +1165,8 @@ static int dwc3_gadget_start(struct usb_gadget *g,
> > > > >  	reg |= DWC3_DCFG_SUPERSPEED;
> > > > >  	dwc3_writel(dwc->regs, DWC3_DCFG, reg);
> > > > >
> > > > > +	dwc->config_start = false;
> > > >
> > > > not needed.
> > > >
> > > > > @@ -1592,6 +1598,7 @@ static void dwc3_gadget_disconnect_interrupt(struct dwc3 *dwc)
> > > > >
> > > > >  	dwc3_stop_active_transfers(dwc);
> > > > >  	dwc3_disconnect_gadget(dwc);
> > > > > +	dwc->config_start = false;
> > > >
> > > > not needed.
> > > >
> > > > > @@ -1644,6 +1651,7 @@ static void dwc3_gadget_reset_interrupt(struct dwc3 *dwc)
> > > > >
> > > > >  	dwc3_stop_active_transfers(dwc);
> > > > >  	dwc3_clear_stall_all_ep(dwc);
> > > > > +	dwc->config_start = false;
> > > >
> > > > not needed.
> > >
> > > With the possible exception of this last one, I'm pretty sure these are all
> > > needed. Can you explain why you think they are not?
> > 
> > you must go through a set_config() in order to get that
> > dwc3_gadget_start_config() called. Then you always set it to false in
> > the beginning of our set_config() handler. Sounds a bit unnecessary to
> > me to set it to false in so many places.
> 
> If the host does a SetConfig, and then we get interrupted for some reason
> before it completes (the USB cable gets yanked, say), then we need to
> clear the flag to be ready when the host re-enumerates the device. That
> is the reason for clearing the flag in the disconnect and reset interupt
> handlers.
> 
> I wasn't sure about dwc3_gadget_start(), but I saw other things were also
> getting initialized there, so I thought maybe it needed to be done there
> too.

Good point. I had missed that. If you resend just changing the name of
the flag and adding the kernel doc to the structure, I'll queue it.

-- 
balbi

Attachment: signature.asc
Description: Digital signature


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux