[U-Boot] [PATCH] usb: musb-new: omap2430: Reset the MUSB controller early

[Marek Vasut]

On Monday, January 19, 2015 at 02:29:29 PM, Paul Kocialkowski wrote:
> Le dimanche 18 janvier 2015 à 23:51 +0100, Marek Vasut a écrit :
> > On Sunday, January 18, 2015 at 05:44:45 PM, Paul Kocialkowski wrote:
> > > When booting from USB peripheral boot, the bootrom will not properly
> > > deinit the MUSB controller, which doesn't clearly indicate an USB
> > > disconnection to the host and leaves U-Boot to deal with the state of
> > > the previous USB session.
> > > 
> > > On some host controller drivers (e.g. xhci_hcd), this ends up in a
> > > failure during set address, caused by the lack of proper disconnection
> > > notification.
> > > 
> > > Resetting the controller early in U-Boot notifies the host of the
> > > disconnection and doesn't hurt other use cases.
> > > 
> > > Signed-off-by: Paul Kocialkowski <contact at paulk.fr>
> > > ---
> > > 
> > >  drivers/usb/musb-new/omap2430.c | 17 +++++++++++++++++
> > >  1 file changed, 17 insertions(+)
> > > 
> > > diff --git a/drivers/usb/musb-new/omap2430.c
> > > b/drivers/usb/musb-new/omap2430.c index 98f4830..6b256bb 100644
> > > --- a/drivers/usb/musb-new/omap2430.c
> > > +++ b/drivers/usb/musb-new/omap2430.c
> > > @@ -321,6 +321,7 @@ static int omap2430_musb_init(struct musb *musb)
> > > 
> > >  {
> > >  
> > >  	u32 l;
> > >  	int status = 0;
> > > 
> > > +	int timeout = 10;
> > > 
> > >  #ifndef __UBOOT__
> > >  
> > >  	struct device *dev = musb->controller;
> > >  	struct omap2430_glue *glue = dev_get_drvdata(dev->parent);
> > > 
> > > @@ -331,6 +332,22 @@ static int omap2430_musb_init(struct musb *musb)
> > > 
> > >  		(struct omap_musb_board_data *)musb->controller;
> > >  
> > >  #endif
> > > 
> > > +	/* Reset the controller */
> > > +	musb_writel(musb->mregs, OTG_SYSCONFIG, SOFTRST);
> > > +
> > > +	while (timeout--) {
> > 
> > Hi!
> > 
> > Just a minor nitpick, you might want to replace the timeout with
> > get_timer(0) and friends, see for example drivers/spi/altera_spi.c:161
> > (the inner loop in spi_xfer() ).
> 
> Well to be honest here, I added that timeout code "just because" and
> every time I tried, the bit was cleared on the first read right after
> instructing the reset, so I doubt there is a real need for "doing it
> right". Let me know if you really think this would be better with
> timers.

By using this get_timer() approach, you implement a much more consistent
and deterministic timeout, which expires around the number of mSec it was
supposed to take, while in this current approach, the real duration of the
loop might vary greatly.

The get_timer() approach is also the favored way of implementing timeouts
in U-Boot, so I would prefer to use this approach if/where possible.

> > +CC Hans, can you test on sunxi please ?
> 
> I could test on sunxi as well if that helps, but isn't that part of the
> code specific to the OMAP?

Oh, oops, sorry. You're right, thanks for pointing it out.

In that case, please fix the loop and unless anyone has any objections,
I'd pick V2 for the current merge window. Tom, do you agree please ?

> Thanks for the review!

Best regards,
Marek Vasut