* [U-Boot] [PATCH] usb: musb-new: omap2430: Reset the MUSB controller early @ 2015-01-18 16:44 Paul Kocialkowski 2015-01-18 22:51 ` Marek Vasut 0 siblings, 1 reply; 7+ messages in thread From: Paul Kocialkowski @ 2015-01-18 16:44 UTC (permalink / raw) To: u-boot 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@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--) { + l = musb_readl(musb->mregs, OTG_SYSCONFIG); + + if (l & SOFTRST) + udelay(1); + else + break; + } + + if (timeout == 0) { + dev_err(musb->controller, "MUSB reset is taking too long\n"); + return -ENODEV; + } #ifndef __UBOOT__ /* We require some kind of external transceiver, hooked -- 1.9.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] usb: musb-new: omap2430: Reset the MUSB controller early 2015-01-18 16:44 [U-Boot] [PATCH] usb: musb-new: omap2430: Reset the MUSB controller early Paul Kocialkowski @ 2015-01-18 22:51 ` Marek Vasut 2015-01-19 13:29 ` Paul Kocialkowski 0 siblings, 1 reply; 7+ messages in thread From: Marek Vasut @ 2015-01-18 22:51 UTC (permalink / raw) To: u-boot 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@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() ). +CC Hans, can you test on sunxi please ? Thank you ! Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] usb: musb-new: omap2430: Reset the MUSB controller early 2015-01-18 22:51 ` Marek Vasut @ 2015-01-19 13:29 ` Paul Kocialkowski 2015-01-19 15:12 ` Marek Vasut 0 siblings, 1 reply; 7+ messages in thread From: Paul Kocialkowski @ 2015-01-19 13:29 UTC (permalink / raw) To: u-boot 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@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. > +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? Thanks for the review! -- Paul Kocialkowski, Replicant developer Replicant is a fully free Android distribution running on several devices, a free software mobile operating system putting the emphasis on freedom and privacy/security. Website: http://www.replicant.us/ Blog: http://blog.replicant.us/ Wiki/tracker/forums: http://redmine.replicant.us/ -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 819 bytes Desc: This is a digitally signed message part URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150119/3fec1bf4/attachment.pgp> ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH] usb: musb-new: omap2430: Reset the MUSB controller early 2015-01-19 13:29 ` Paul Kocialkowski @ 2015-01-19 15:12 ` Marek Vasut 2015-01-19 17:33 ` [U-Boot] [PATCH v2] " Paul Kocialkowski 0 siblings, 1 reply; 7+ messages in thread From: Marek Vasut @ 2015-01-19 15:12 UTC (permalink / raw) To: u-boot 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@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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH v2] usb: musb-new: omap2430: Reset the MUSB controller early 2015-01-19 15:12 ` Marek Vasut @ 2015-01-19 17:33 ` Paul Kocialkowski 2015-01-20 15:23 ` Tom Rini 0 siblings, 1 reply; 7+ messages in thread From: Paul Kocialkowski @ 2015-01-19 17:33 UTC (permalink / raw) To: u-boot 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@paulk.fr> --- drivers/usb/musb-new/omap2430.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/drivers/usb/musb-new/omap2430.c b/drivers/usb/musb-new/omap2430.c index 98f4830..31a280e 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; + unsigned long int start; #ifndef __UBOOT__ struct device *dev = musb->controller; struct omap2430_glue *glue = dev_get_drvdata(dev->parent); @@ -331,6 +332,21 @@ 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); + + start = get_timer(0); + + while (1) { + l = musb_readl(musb->mregs, OTG_SYSCONFIG); + if ((l & SOFTRST) == 0) + break; + + if (get_timer(start) > (CONFIG_SYS_HZ / 1000)) { + dev_err(musb->controller, "MUSB reset is taking too long\n"); + return -ENODEV; + } + } #ifndef __UBOOT__ /* We require some kind of external transceiver, hooked -- 1.9.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH v2] usb: musb-new: omap2430: Reset the MUSB controller early 2015-01-19 17:33 ` [U-Boot] [PATCH v2] " Paul Kocialkowski @ 2015-01-20 15:23 ` Tom Rini 2015-01-22 7:19 ` Marek Vasut 0 siblings, 1 reply; 7+ messages in thread From: Tom Rini @ 2015-01-20 15:23 UTC (permalink / raw) To: u-boot On Mon, Jan 19, 2015 at 06:33:43PM +0100, 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@paulk.fr> Reviewed-by: Tom Rini <trini@ti.com> -- Tom -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: Digital signature URL: <http://lists.denx.de/pipermail/u-boot/attachments/20150120/d93c5d5d/attachment.pgp> ^ permalink raw reply [flat|nested] 7+ messages in thread
* [U-Boot] [PATCH v2] usb: musb-new: omap2430: Reset the MUSB controller early 2015-01-20 15:23 ` Tom Rini @ 2015-01-22 7:19 ` Marek Vasut 0 siblings, 0 replies; 7+ messages in thread From: Marek Vasut @ 2015-01-22 7:19 UTC (permalink / raw) To: u-boot On Tuesday, January 20, 2015 at 04:23:10 PM, Tom Rini wrote: > On Mon, Jan 19, 2015 at 06:33:43PM +0100, 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@paulk.fr> > > Reviewed-by: Tom Rini <trini@ti.com> Applied, thanks! Best regards, Marek Vasut ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-01-22 7:19 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-01-18 16:44 [U-Boot] [PATCH] usb: musb-new: omap2430: Reset the MUSB controller early Paul Kocialkowski 2015-01-18 22:51 ` Marek Vasut 2015-01-19 13:29 ` Paul Kocialkowski 2015-01-19 15:12 ` Marek Vasut 2015-01-19 17:33 ` [U-Boot] [PATCH v2] " Paul Kocialkowski 2015-01-20 15:23 ` Tom Rini 2015-01-22 7:19 ` Marek Vasut
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox