[U-Boot] [PATCH] ppc4xx: Canyonlands: Fix USB host PHY reset sequence

[U-Boot] [PATCH] ppc4xx: Canyonlands: Fix USB host PHY reset sequence

[Dave Mitchell]

Current de-assert reset is not sufficient for the USB PHY reset
on some Canyonlands platforms. The patch adds an assert/de-assert
sequence. This addresses a USB detection problem for devices
attached prior to power-up. The delay lengths are needed for
power to the PHY to stabilize.

Signed-off-by: Jeff Mann <MannJ@embeddedplanet.com>
Signed-off-by: Dave Mitchell <dmitchell@appliedmicro.com>
Acked-by: Tirumala Reddy Marri <tmarri@appliedmicro.com>
---
 board/amcc/canyonlands/canyonlands.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/board/amcc/canyonlands/canyonlands.c b/board/amcc/canyonlands/canyonlands.c
index 13a0dac..bef12ae 100644
--- a/board/amcc/canyonlands/canyonlands.c
+++ b/board/amcc/canyonlands/canyonlands.c
@@ -177,8 +177,11 @@ int board_early_init_f(void)
 	/* Remove NOR-FLASH, NAND-FLASH & EEPROM hardware write protection */
 	out_8((void *)CONFIG_SYS_BCSR_BASE + 5, 0);
 
-	/* Enable USB host & USB-OTG */
+	/* Enable USB host & USB-OTG;force assert,then de-assert PHY reset */
+	out_8((void *)CONFIG_SYS_BCSR_BASE + 7, 1);
+	mdelay(100);
 	out_8((void *)CONFIG_SYS_BCSR_BASE + 7, 0);
+	mdealy(100);
 
 	mtsdr(SDR0_SRST1, 0);	/* Pull AHB out of reset default=1 */
 
-- 
1.6.3.2

[U-Boot] [PATCH] ppc4xx: Canyonlands: Fix USB host PHY reset sequence

[Wolfgang Denk]

Dear Dave Mitchell,

In message <1260821359-8705-1-git-send-email-dmitchell@appliedmicro.com> you wrote:
> Current de-assert reset is not sufficient for the USB PHY reset
> on some Canyonlands platforms. The patch adds an assert/de-assert
> sequence. This addresses a USB detection problem for devices
> attached prior to power-up. The delay lengths are needed for
> power to the PHY to stabilize.

Hm...

> -	/* Enable USB host & USB-OTG */
> +	/* Enable USB host & USB-OTG;force assert,then de-assert PHY reset */
> +	out_8((void *)CONFIG_SYS_BCSR_BASE + 7, 1);
> +	mdelay(100);
>  	out_8((void *)CONFIG_SYS_BCSR_BASE + 7, 0);
> +	mdealy(100);

It would have been nice if you had at least tried to compile that.
"mdealy()" is a typo, right?

And do we really need 200 milliseconds delay?

Some customers who care about boot times will not be happy about such
an aditional delay.  You add this code do board_early_init_f() which
always gets executed. But actually this is only needed when we want to
use the USB port in U-Boot, right?  So this should be added to the USB
init code, and not to the common code.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"They that can give up essential liberty to obtain a little temporary
saftey deserve neither liberty not saftey." - Benjamin Franklin, 1759

[U-Boot] [PATCH] ppc4xx: Canyonlands: Fix USB host PHY reset sequence

[Wolfgang Denk]

Dear Stefan,

In message <1260821359-8705-1-git-send-email-dmitchell@appliedmicro.com> Dave Mitchell wrote:
...
> --- a/board/amcc/canyonlands/canyonlands.c
> +++ b/board/amcc/canyonlands/canyonlands.c
> @@ -177,8 +177,11 @@ int board_early_init_f(void)
>  	/* Remove NOR-FLASH, NAND-FLASH & EEPROM hardware write protection */
>  	out_8((void *)CONFIG_SYS_BCSR_BASE + 5, 0);
>  
> -	/* Enable USB host & USB-OTG */
> +	/* Enable USB host & USB-OTG;force assert,then de-assert PHY reset */
> +	out_8((void *)CONFIG_SYS_BCSR_BASE + 7, 1);
> +	mdelay(100);
>  	out_8((void *)CONFIG_SYS_BCSR_BASE + 7, 0);
> +	mdealy(100);

We should get rid of this "CONFIG_SYS_BCSR_BASE + offset" stuff and
use a proper C struct for this.  What do you think?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Testing can show the presense of bugs, but not their absence.
                                                   -- Edsger Dijkstra

[U-Boot] [PATCH] ppc4xx: Canyonlands: Fix USB host PHY reset sequence

[David Mitchell]

Hi Wolfgang,

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de]
> Sent: Monday, December 14, 2009 2:42 PM
> To: David Mitchell
> Cc: u-boot at lists.denx.de; Stefan Roese; Jeff Mann
> Subject: Re: [U-Boot] [PATCH] ppc4xx: Canyonlands: Fix USB host PHY
> reset sequence
> 
> > -	/* Enable USB host & USB-OTG */
> > +	/* Enable USB host & USB-OTG;force assert,then de-assert PHY
> reset */
> > +	out_8((void *)CONFIG_SYS_BCSR_BASE + 7, 1);
> > +	mdelay(100);
> >  	out_8((void *)CONFIG_SYS_BCSR_BASE + 7, 0);
> > +	mdealy(100);
> 
> It would have been nice if you had at least tried to compile that.
> "mdealy()" is a typo, right?

Ah, sorry about that. I had the patch hanging around and moved it up to
the ToT by hand.

I'll fix and re-submit.

> 
> And do we really need 200 milliseconds delay?

I generated several versions for a remote team to test (I don't have the
failing hardware).
Only the 100ms version worked every time.

I did request a hardware investigation to clarify the 100ms delays. They
reported the time was needed for the power to stabilize.

> 
> Some customers who care about boot times will not be happy about such
> an aditional delay.  You add this code do board_early_init_f() which
> always gets executed. But actually this is only needed when we want to
> use the USB port in U-Boot, right?  So this should be added to the USB
> init code, and not to the common code.

Actually we need the reset prior to Linux boot as well. Otherwise Linux
will not discover attached devices. Which is the whole reason for the
patch...


Best regards,
-DM
--------------------------------------------------------

CONFIDENTIALITY NOTICE: This e-mail message, including any attachments, is for the sole use of the intended recipient(s) and contains information that is confidential and proprietary to AppliedMicro Corporation or its subsidiaries. It is to be used solely for the purpose of furthering the parties' business relationship. All unauthorized review, use, disclosure or distribution is prohibited. If you are not the intended recipient, please contact the sender by reply e-mail and destroy all copies of the original message.

[U-Boot] [PATCH] ppc4xx: Canyonlands: Fix USB host PHY reset sequence

[Wolfgang Denk]

Dear David,

In message <AC5E1C3367E37D44970B81A6ADD1DA2C087E7893@SDCEXCHANGE01.ad.amcc.com> you wrote:
>
> I generated several versions for a remote team to test (I don't have the
> failing hardware).
> Only the 100ms version worked every time.

I see :-(

> Actually we need the reset prior to Linux boot as well. Otherwise Linux
> will not discover attached devices. Which is the whole reason for the
> patch...

No, we don't. Why should we add a delay for all systems, even when
neither U-Boot not Linux will ever use any USB? For example ion
boards where not even a USB connector is physically present on the
hardware?

Please (re-) read the U-Boot Design Principles, item 2 "Keep it fast":
http://www.denx.de/wiki/view/U-Boot/DesignPrinciples#2_Keep_it_Fast

If USB is not used in U-Boot itself, U-Boot shall not  initialize  it
at all. And we should never impose restrictions or penalties (delays)
on everybody for features that are used only by some.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
In order to succeed in any enterprise, one  must  be  persistent  and
patient.  Even  if  one  has to run some risks, one must be brave and
strong enough to meet and overcome vexing challenges  to  maintain  a
successful  business  in  the  long  run.  I  cannot help saying that
Americans lack this necessary challenging spirit today.
- Hajime Karatsu