Make it work again with USB.

PATCH Superseded Andre Schwarz 3 participants 10 messages Nov 16, 2010 – Apr 5, 2011 PowerPC

lore

Share & tools

lore

Andre Schwarz

author

2010-11-16 12:37 UTC #0 lore raw

PATCH v2 1/1 Make it work again with USB.

USB register range IMMR+0x00-0xff is reserved and hangs the CPU.

Signed-off-by Andre Schwarz <[email protected]>

---
 arch/powerpc/cpu/mpc83xx/cpu_init.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

arch/powerpc/cpu/mpc83xx/cpu_init.c +1 -1

index 7a1cae7..cfead18 100644
@@ -327,7 +327,7 @@ void cpu_init_f (volatile immap_t * im)
 	im->gpio[1].dir = CONFIG_SYS_GPIO2_DIR;
 #endif
 #ifdef CONFIG_USB_EHCI_FSL
-#ifndef CONFIG_MPC834x
+#if !defined(CONFIG_MPC834x) && !defined(CONFIG_MPC837x)
 	uint32_t temp;
 	struct usb_ehci *ehci = (struct usb_ehci *)CONFIG_SYS_FSL_USB_ADDR;

Signature · 1 line

1.7.0.4

Kim Phillips

NAK

2010-11-28 15:10 UTC #1 lore raw

PATCH v2 1/1 Make it work again with USB.

Quoted 4 lines · On Tue, 16 Nov 2010 13:37:50 +0100 Andre Schwarz…

USB register range IMMR+0x00-0xff is reserved and hangs the CPU.
Signed-off-by Andre Schwarz <[email protected]>
---

nack, 837x has a usb controller at IMMR+0x23000. Check to see whether there is an invalid USB clock setting in the SCCR?

Thanks,

Kim

Schwarz

2010-11-29 07:59 UTC #2 lore raw

PATCH v2 1/1 Make it work again with USB.

Kim,

Quoted 9 lines

Quoted 4 lines · On Tue, 16 Nov 2010 13:37:50 +0100 Andre Schwarz…
USB register range IMMR+0x00-0xff is reserved and hangs the CPU.
Signed-off-by Andre Schwarz <[email protected]>
---
nack, 837x has a usb controller at IMMR+0x23000.?

yes - but offset 0x00-0xff is explicitly reserved regarding to the manual. Don't know whether it is a "should not" or "must not be touched". ?
All I can see is a CPU hang with arbiter event register reporting a timeout on 0xe0023000.
?

? Check to see whether there is an invalid USB clock setting in the SCCR?? All clocks are turned on except SEC and 2nd TSEC.
?
After all USB is running fine with this patch, i.e. there can hardly be a missing clock.

Please re-think you NAK.
?
--
Gru?,
Andr? Schwarz

MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner

Kim Phillips

2010-12-13 22:52 UTC #3 lore raw

PATCH v2 1/1 Make it work again with USB.

Quoted 19 lines · On Mon, 29 Nov 2010 08:59:21 +0100 "Schwarz,Andre"…

Kim,
Quoted 9 lines
Quoted 4 lines · On Tue, 16 Nov 2010 13:37:50 +0100 Andre Schwarz…
USB register range IMMR+0x00-0xff is reserved and hangs the CPU.
Signed-off-by Andre Schwarz <[email protected]>
---
nack, 837x has a usb controller at IMMR+0x23000.?
yes - but offset 0x00-0xff is explicitly reserved regarding to the manual. Don't know whether it is a "should not" or "must not be touched". ?
All I can see is a CPU hang with arbiter event register reporting a timeout on 0xe0023000.
?

Quoted 8 lines

? Check to see whether there is an invalid USB clock setting in the SCCR?? All clocks are turned on except SEC and 2nd TSEC.
?
After all USB is running fine with this patch, i.e. there can hardly be a missing clock.
Please re-think you NAK.

afaik, 834x and 837x don't have any special USB settings in common, so, this patch, at least in it's current form, is not on.

0xe0023500 should be the address of the config register being accessed here; please check the code isn't accessing 0xe0023000, as you mention above.

If that's correct, try something like the following so we can determine what setting the USB controller didn't agree with:

arch/powerpc/cpu/mpc83xx/cpu_init.c +1 -1

index 7a1cae7..cbc4157 100644
@@ -332,7 +332,7 @@ void cpu_init_f (volatile immap_t * im)
 	struct usb_ehci *ehci = (struct usb_ehci *)CONFIG_SYS_FSL_USB_ADDR;
 
 	/* Configure interface. */
-	setbits_be32(&ehci->control, REFSEL_16MHZ | UTMI_PHY_EN);
+	setbits_be32(&ehci->control, 0);
 
 	/* Wait for clock to stabilize */
 	do {

Thanks,

Kim

Andre Schwarz

author

2011-02-28 16:18 UTC #4 lore raw

PATCH v2 1/1 Make it work again with USB.

Kim,

finally found some time to re-work my 8377 board code for submission.

sorry to bother you again, but I again stumbled over the discussed USB init issue :

Quoted 15 lines

Quoted 15 lines
Quoted 1 line
nack, 837x has a usb controller at IMMR+0x23000.
yes - but offset 0x00-0xff is explicitly reserved regarding to the manual. Don't know whether it is a "should not" or "must not be touched".
All I can see is a CPU hang with arbiter event register reporting a timeout on 0xe0023000.
Check to see whether there is an invalid USB clock setting in the SCCR?
All clocks are turned on except SEC and 2nd TSEC.
After all USB is running fine with this patch, i.e. there can hardly be a missing clock.

Quoted 7 lines

Quoted 1 line
Please re-think you NAK.
afaik, 834x and 837x don't have any special USB settings in common, so, this patch, at least in it's current form, is not on.
0xe0023500 should be the address of the config register being accessed here; please check the code isn't accessing 0xe0023000, as you mention above.

ok - this was some kind of misunderstanding.
ehci regs are based at immr + 0x23000 with the "config" pointing to offset 0x500 inside ehci.
This looks sane to me.

arch/powerpc/cpu/mpc83xx/cpu_init.c void cpu_init_f (volatile immap_t * im) · line 335

Quoted 12 lines

If that's correct, try something like the following so we can determine what setting the USB controller didn't agree with:
arch/powerpc/cpu/mpc83xx/cpu_init.c +0 -1
index 7a1cae7..cbc4157 100644
@@ -332,7 +332,7 @@ void cpu_init_f (volatile immap_t * im)
  	struct usb_ehci *ehci = (struct usb_ehci *)CONFIG_SYS_FSL_USB_ADDR;
 
  	/* Configure interface. */
-	setbits_be32(&ehci->control, REFSEL_16MHZ | UTMI_PHY_EN);

MPC837x has only 2 working bits inside the control register :

Bit29: USB_EN -> should be set to 1 before USB can be used.
Bit31: ULPI_INT_EN -> enables an ULPI wake-up irq.

Both "REFSEL_16MHZ" and "UTMI_PHY_EN" are completely out of scope for MPC837x.

arch/powerpc/cpu/mpc83xx/cpu_init.c void cpu_init_f (volatile immap_t * im) · line 337

Quoted 3 lines

arch/powerpc/cpu/mpc83xx/cpu_init.c +1 -0

+	setbits_be32(&ehci->control, 0);
 
  	/* Wait for clock to stabilize */

This loop never returns on MPC837x because "PHY_CLK_VALID" isn't valid.

Quoted 4 lines

arch/powerpc/cpu/mpc83xx/cpu_init.c +0 -0

  	do {    temp = __raw_readl(&ehci->control);

                 udelay(1000);
         } while (!(temp&  PHY_CLK_VALID));

I still wonder how there can be a single working MPC837x board with CONFIG_USB_EHCI_FSL set.

Some pending patches on your side ?
What kind of patch might get an ACK from your side ?

Signature · 8 lines

Regards,
Andre



MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner

Kim Phillips

2011-03-31 01:14 UTC #5 lore raw

PATCH v2 1/1 Make it work again with USB.

Quoted 21 lines · On Mon, 28 Feb 2011 17:18:38 +0100 Andre Schwarz…

sorry to bother you again, but I again stumbled over the discussed USB init issue :
Quoted 19 lines
Quoted 8 lines
Quoted 1 line
nack, 837x has a usb controller at IMMR+0x23000.
yes - but offset 0x00-0xff is explicitly reserved regarding to the manual. Don't know whether it is a "should not" or "must not be touched".
All I can see is a CPU hang with arbiter event register reporting a timeout on 0xe0023000.
Quoted 8 lines
Check to see whether there is an invalid USB clock setting in the SCCR?
All clocks are turned on except SEC and 2nd TSEC.
After all USB is running fine with this patch, i.e. there can hardly be a missing clock.
Please re-think you NAK.
afaik, 834x and 837x don't have any special USB settings in common, so, this patch, at least in it's current form, is not on.

Quoted 8 lines

Quoted 3 lines
0xe0023500 should be the address of the config register being accessed here; please check the code isn't accessing 0xe0023000, as you mention above.
ok - this was some kind of misunderstanding.
ehci regs are based at immr + 0x23000 with the "config" pointing to offset 0x500 inside ehci.
This looks sane to me.

ok, as long as it's confirmed.

Quoted 14 lines

arch/powerpc/cpu/mpc83xx/cpu_init.c void cpu_init_f (volatile immap_t * im) · line 335
Quoted 12 lines
If that's correct, try something like the following so we can determine what setting the USB controller didn't agree with:
arch/powerpc/cpu/mpc83xx/cpu_init.c +0 -1
index 7a1cae7..cbc4157 100644
@@ -332,7 +332,7 @@ void cpu_init_f (volatile immap_t * im)
  	struct usb_ehci *ehci = (struct usb_ehci *)CONFIG_SYS_FSL_USB_ADDR;
 
  	/* Configure interface. */
-	setbits_be32(&ehci->control, REFSEL_16MHZ | UTMI_PHY_EN);
MPC837x has only 2 working bits inside the control register :

Quoted 5 lines

Bit29: USB_EN -> should be set to 1 before USB can be used.
Bit31: ULPI_INT_EN -> enables an ULPI wake-up irq.
Both "REFSEL_16MHZ" and "UTMI_PHY_EN" are completely out of scope for MPC837x.

that's why I'm suggesting we confirm that touching the REFSEL_16MHZ and UTMI_PHY_EN bits aren't sending the 837x controller into oblivion - did you test the patch?

Quoted 4 lines

arch/powerpc/cpu/mpc83xx/cpu_init.c void cpu_init_f (volatile immap_t * im) · line 337
Quoted 3 lines
arch/powerpc/cpu/mpc83xx/cpu_init.c +1 -0
+	setbits_be32(&ehci->control, 0);
 
  	/* Wait for clock to stabilize */
This loop never returns on MPC837x because "PHY_CLK_VALID" isn't valid.

right, we need to narrow down the reason for this.

Quoted 9 lines

Quoted 4 lines
arch/powerpc/cpu/mpc83xx/cpu_init.c +0 -0
  	do {    temp = __raw_readl(&ehci->control);
                 udelay(1000);
         } while (!(temp&  PHY_CLK_VALID));
I still wonder how there can be a single working MPC837x board with CONFIG_USB_EHCI_FSL set.
Some pending patches on your side ?
What kind of patch might get an ACK from your side ?

nothing that suggests 834x and 837x have any special USB settings in common - because it's not true and therefore misleading.

Kim

Andre Schwarz

author

2011-03-31 07:52 UTC #6 lore raw

PATCH v2 1/1 Make it work again with USB.

Kim,

Quoted 47 lines

Quoted 21 lines · On Mon, 28 Feb 2011 17:18:38 +0100 Andre…
sorry to bother you again, but I again stumbled over the discussed USB init issue :
Quoted 19 lines
Quoted 8 lines
Quoted 1 line
nack, 837x has a usb controller at IMMR+0x23000.
yes - but offset 0x00-0xff is explicitly reserved regarding to the manual. Don't know whether it is a "should not" or "must not be touched".
All I can see is a CPU hang with arbiter event register reporting a timeout on 0xe0023000.
Quoted 8 lines
Check to see whether there is an invalid USB clock setting in the SCCR?
All clocks are turned on except SEC and 2nd TSEC.
After all USB is running fine with this patch, i.e. there can hardly be a missing clock.
Please re-think you NAK.
afaik, 834x and 837x don't have any special USB settings in common, so, this patch, at least in it's current form, is not on.
Quoted 7 lines
Quoted 3 lines
0xe0023500 should be the address of the config register being accessed here; please check the code isn't accessing 0xe0023000, as you mention above.
ok - this was some kind of misunderstanding.
ehci regs are based at immr + 0x23000 with the "config" pointing to offset 0x500 inside ehci.
This looks sane to me.
ok, as long as it's confirmed.
Quoted 14 lines
arch/powerpc/cpu/mpc83xx/cpu_init.c void cpu_init_f (volatile immap_t * im) · line 335
Quoted 12 lines
If that's correct, try something like the following so we can determine what setting the USB controller didn't agree with:
arch/powerpc/cpu/mpc83xx/cpu_init.c +0 -1
index 7a1cae7..cbc4157 100644
@@ -332,7 +332,7 @@ void cpu_init_f (volatile immap_t * im)
   	struct usb_ehci *ehci = (struct usb_ehci *)CONFIG_SYS_FSL_USB_ADDR;
 
   	/* Configure interface. */
-	setbits_be32(&ehci->control, REFSEL_16MHZ | UTMI_PHY_EN);
MPC837x has only 2 working bits inside the control register :

Quoted 8 lines

Quoted 5 lines
Bit29: USB_EN -> should be set to 1 before USB can be used.
Bit31: ULPI_INT_EN -> enables an ULPI wake-up irq.
Both "REFSEL_16MHZ" and "UTMI_PHY_EN" are completely out of scope for MPC837x.
that's why I'm suggesting we confirm that touching the REFSEL_16MHZ and UTMI_PHY_EN bits aren't sending the 837x controller into oblivion - did you test the patch?

yes - writing those bits or not makes no difference ...

Quoted 6 lines

Quoted 4 lines
arch/powerpc/cpu/mpc83xx/cpu_init.c void cpu_init_f (volatile immap_t * im) · line 337
Quoted 3 lines
arch/powerpc/cpu/mpc83xx/cpu_init.c +1 -0
+	setbits_be32(&ehci->control, 0);
 
   	/* Wait for clock to stabilize */
This loop never returns on MPC837x because "PHY_CLK_VALID" isn't valid.
right, we need to narrow down the reason for this.

... it is this loop that *can not* return since ehci->control[PHY_CLK_VALID] is always 0 on 837x.
This can be seen by having a look at the reference manual (Rev. 1 page 20-46 / Chapter 20.3.2.28).

Quoted 11 lines

Quoted 9 lines
Quoted 4 lines
arch/powerpc/cpu/mpc83xx/cpu_init.c +0 -0
   	do {    temp = __raw_readl(&ehci->control);
                  udelay(1000);
          } while (!(temp&   PHY_CLK_VALID));
I still wonder how there can be a single working MPC837x board with CONFIG_USB_EHCI_FSL set.
Some pending patches on your side ?
What kind of patch might get an ACK from your side ?
nothing that suggests 834x and 837x have any special USB settings in common - because it's not true and therefore misleading.

I never ever said that 834x and 837x have anything in common regarding USB. All I say is that they both must not run into this loop.

If you see any problems or'ing 837x into the #ifndef I suggest you come up with a positive
#ifdef being valid for only those chips that need it. Honestly I don't know which SoC's will need it.

All I want is to skip this loop on 837x.

Regards,
Andr?

MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner

Kim Phillips

2011-04-01 21:53 UTC #7 lore raw

PATCH 1/1 mpc83xx: restrict UTMI PHY configuration to 831x parts

i.e, to those parts that have PHY_CLK_VALID bits in their USB CONTROL registers:

mpc8308 WU_INT, PHY_CLK_SEL, USB_EN, WU_INT_EN, ULPI_INT_EN
mpc831x PHY_CLK_VALID, WU_INT, CLKIN_SEL, PHY_CLK_SEL, UTMI_PHY_EN,
PLL_RESET, REFSEL, OTG_PORT, KEEP_OTG_ON, LSF_EN, USB_EN, ULPI_INT_EN
mpc834x USB_EN, ULPI_INT1_EN (MPH only), ULPI_INT0_EN
mpc837x USB_EN, ULPI_INT_EN

(mpc832x, mpc8360 don't have a USB_EHCI_FSL compatible controller)

this prevents non-831x parts from never completing cpu_init_f(), because the (non-existent) PHY_CLK_VALID bit never gets set.

Reported-by Andre Schwarz <[email protected]>

Signed-off-by Kim Phillips <[email protected]>

---
 arch/powerpc/cpu/mpc83xx/cpu_init.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

arch/powerpc/cpu/mpc83xx/cpu_init.c +1 -3

index 7a1cae7..76afba5 100644
@@ -326,8 +326,7 @@ void cpu_init_f (volatile immap_t * im)
 	im->gpio[1].dat = CONFIG_SYS_GPIO2_DAT;
 	im->gpio[1].dir = CONFIG_SYS_GPIO2_DIR;
 #endif
-#ifdef CONFIG_USB_EHCI_FSL
-#ifndef CONFIG_MPC834x
+#if defined(CONFIG_USB_EHCI_FSL) && defined(CONFIG_MPC831x)
 	uint32_t temp;
 	struct usb_ehci *ehci = (struct usb_ehci *)CONFIG_SYS_FSL_USB_ADDR;

@@ -340,7 +339,6 @@ void cpu_init_f (volatile immap_t * im)
 		udelay(1000);
 	} while (!(temp & PHY_CLK_VALID));
 #endif
-#endif
 }
 
 int cpu_init_r (void)

Signature · 1 line

1.7.4

Andre Schwarz

author

2011-04-04 10:52 UTC #8 lore raw

PATCH 1/1 mpc83xx: restrict UTMI PHY configuration to 831x parts

Kim,

excellent - thanks.

Quoted 10 lines

i.e, to those parts that have PHY_CLK_VALID bits in their USB CONTROL registers:
mpc8308 WU_INT, PHY_CLK_SEL, USB_EN, WU_INT_EN, ULPI_INT_EN
mpc831x PHY_CLK_VALID, WU_INT, CLKIN_SEL, PHY_CLK_SEL, UTMI_PHY_EN,
PLL_RESET, REFSEL, OTG_PORT, KEEP_OTG_ON, LSF_EN, USB_EN, ULPI_INT_EN
mpc834x USB_EN, ULPI_INT1_EN (MPH only), ULPI_INT0_EN
mpc837x USB_EN, ULPI_INT_EN

Quoted 7 lines

(mpc832x, mpc8360 don't have a USB_EHCI_FSL compatible controller)
this prevents non-831x parts from never completing cpu_init_f(), because the (non-existent) PHY_CLK_VALID bit never gets set.
Reported-by Andre Schwarz<[email protected]>
Signed-off-by Kim Phillips<[email protected]>

Tested-by Andre Schwarz <[email protected]>

Works on MPC8343 and MPC8377.

Quoted 19 lines

---
  arch/powerpc/cpu/mpc83xx/cpu_init.c |    4 +---
  1 files changed, 1 insertions(+), 3 deletions(-)

arch/powerpc/cpu/mpc83xx/cpu_init.c +1 -2

index 7a1cae7..76afba5 100644
@@ -326,8 +326,7 @@ void cpu_init_f (volatile immap_t * im)
  	im->gpio[1].dat = CONFIG_SYS_GPIO2_DAT;
  	im->gpio[1].dir = CONFIG_SYS_GPIO2_DIR;
  #endif
-#ifdef CONFIG_USB_EHCI_FSL
-#ifndef CONFIG_MPC834x
+#if defined(CONFIG_USB_EHCI_FSL)&&  defined(CONFIG_MPC831x)
  	uint32_t temp;
  	struct usb_ehci *ehci = (struct usb_ehci *)CONFIG_SYS_FSL_USB_ADDR;

arch/powerpc/cpu/mpc83xx/cpu_init.c void cpu_init_f (volatile immap_t * im) · line 344

Quoted 7 lines

arch/powerpc/cpu/mpc83xx/cpu_init.c +0 -1

@@ -340,7 +339,6 @@ void cpu_init_f (volatile immap_t * im)
  		udelay(1000);
  	} while (!(temp&  PHY_CLK_VALID));
  #endif
-#endif
  }
 
  int cpu_init_r (void)

Cheers,
Andr?

MATRIX VISION GmbH, Talstrasse 16, DE-71570 Oppenweiler
Registergericht: Amtsgericht Stuttgart, HRB 271090
Geschaeftsfuehrer: Gerhard Thullner, Werner Armingeon, Uwe Furtner

Kim Phillips

Applied

2011-04-05 01:26 UTC #9 lore raw

PATCH 1/1 mpc83xx: restrict UTMI PHY configuration to 831x parts

Quoted 12 lines · On Mon, 4 Apr 2011 12:52:25 +0200 Andre Schwarz…

Quoted 12 lines
i.e, to those parts that have PHY_CLK_VALID bits in their USB CONTROL registers:
mpc8308 WU_INT, PHY_CLK_SEL, USB_EN, WU_INT_EN, ULPI_INT_EN
mpc831x PHY_CLK_VALID, WU_INT, CLKIN_SEL, PHY_CLK_SEL, UTMI_PHY_EN,
PLL_RESET, REFSEL, OTG_PORT, KEEP_OTG_ON, LSF_EN, USB_EN, ULPI_INT_EN
mpc834x USB_EN, ULPI_INT1_EN (MPH only), ULPI_INT0_EN
mpc837x USB_EN, ULPI_INT_EN
(mpc832x, mpc8360 don't have a USB_EHCI_FSL compatible controller)

Quoted 8 lines

Quoted 5 lines
this prevents non-831x parts from never completing cpu_init_f(), because the (non-existent) PHY_CLK_VALID bit never gets set.
Reported-by Andre Schwarz<[email protected]>
Signed-off-by Kim Phillips<[email protected]>
Tested-by Andre Schwarz <[email protected]>
Works on MPC8343 and MPC8377.

applied to mpc83xx/master.

thanks for testing!

Kim