usb: musb-new: omap2430: Reset the MUSB controller early

PATCH New Paul Kocialkowski 3 participants 7 messages Jan 18, 2015 – Jan 22, 2015

Subsystems (2)

USB OMAP USB support

lore

Share & tools

lore

Paul Kocialkowski

author

2015-01-18 16:44 UTC #0 lore raw

PATCH 1/1 usb: musb-new: omap2430: Reset the MUSB controller early

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 <[email protected]>

---
 drivers/usb/musb-new/omap2430.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

drivers/usb/musb-new/omap2430.c +17 -0

index 98f4830..6b256bb 100644
@@ -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

Signature · 1 line

1.9.1

Marek Vasut

2015-01-18 22:51 UTC #1 lore raw

PATCH 1/1 usb: musb-new: omap2430: Reset the MUSB controller early

Quoted 30 lines · Paul Kocialkowski

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 <[email protected]>
---
 drivers/usb/musb-new/omap2430.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
(quoted diff) +0 -0
b/drivers/usb/musb-new/omap2430.c index 98f4830..6b256bb 100644
drivers/usb/musb-new/omap2430.c +1 -0
@@ -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);

drivers/usb/musb-new/omap2430.c static int omap2430_musb_init(struct musb *musb) · line 338

Quoted 7 lines

drivers/usb/musb-new/omap2430.c +4 -0

@@ -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

Paul Kocialkowski

author

2015-01-19 13:29 UTC #2 lore raw

PATCH 1/1 usb: musb-new: omap2430: Reset the MUSB controller early

Le dimanche 18 janvier 2015 ? 23:51 +0100, Marek Vasut a ?crit :

Quoted 41 lines · Paul Kocialkowski

Quoted 30 lines
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 <[email protected]>
---
 drivers/usb/musb-new/omap2430.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
(quoted diff) +0 -0
b/drivers/usb/musb-new/omap2430.c index 98f4830..6b256bb 100644
drivers/usb/musb-new/omap2430.c +1 -0
@@ -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);
drivers/usb/musb-new/omap2430.c static int omap2430_musb_init(struct musb *musb) · line 338
Quoted 7 lines
drivers/usb/musb-new/omap2430.c +4 -0
@@ -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!

Quoted 3 lines

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.

Quoted 1 line

+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!

Signature · 17 lines

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>

Marek Vasut

2015-01-19 15:12 UTC #3 lore raw

PATCH 1/1 usb: musb-new: omap2430: Reset the MUSB controller early

Quoted 55 lines · Paul Kocialkowski

Le dimanche 18 janvier 2015 ? 23:51 +0100, Marek Vasut a ?crit :
Quoted 50 lines · Paul Kocialkowski
Quoted 37 lines
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 <[email protected]>
---
 drivers/usb/musb-new/omap2430.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
(quoted diff) +0 -0
b/drivers/usb/musb-new/omap2430.c index 98f4830..6b256bb 100644
drivers/usb/musb-new/omap2430.c +1 -0
@@ -321,6 +321,7 @@ static int omap2430_musb_init(struct musb *musb)
 
 {
 
 	u32 l;
 	int status = 0;
 
+	int timeout = 10;
#ifndef __UBOOT__
drivers/usb/musb-new/omap2430.c +0 -0
 	struct device *dev = musb->controller;
 	struct omap2430_glue *glue = dev_get_drvdata(dev->parent);
drivers/usb/musb-new/omap2430.c static int omap2430_musb_init(struct musb *musb) · line 340
Quoted 9 lines
drivers/usb/musb-new/omap2430.c +4 -0
@@ -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!
Quoted 3 lines
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() ).

Quoted 5 lines

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.

Quoted 4 lines

Quoted 1 line
+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 ?

Quoted 1 line

Thanks for the review!

Best regards,
Marek Vasut

Paul Kocialkowski

author

2015-01-19 17:33 UTC #4 lore raw

PATCH v2 1/1 usb: musb-new: omap2430: Reset the MUSB controller early

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 <[email protected]>

---
 drivers/usb/musb-new/omap2430.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

drivers/usb/musb-new/omap2430.c +16 -0

index 98f4830..31a280e 100644
@@ -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

Signature · 1 line

1.9.1

Tom Rini

2015-01-20 15:23 UTC #5 lore raw

PATCH v2 1/1 usb: musb-new: omap2430: Reset the MUSB controller early

Quoted 11 lines · Paul Kocialkowski

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 <[email protected]>

Reviewed-by Tom Rini <[email protected]>

Signature · 8 lines

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>

Marek Vasut

Applied

2015-01-22 07:19 UTC #6 lore raw

PATCH v2 1/1 usb: musb-new: omap2430: Reset the MUSB controller early

Quoted 10 lines · Tom Rini

Quoted 9 lines · Paul Kocialkowski
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.

Quoted 6 lines

Quoted 4 lines
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 <[email protected]>
Reviewed-by Tom Rini <[email protected]>

Applied, thanks!

Best regards,
Marek Vasut