| Message ID | 20101213165238.0b496e57.kim.phillips@freescale.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Kim Phillips |
| Headers | show |
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 : >>> 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. > 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. > If that's correct, try something like the following so we can determine > what setting the USB controller didn't agree with: > > diff --git a/arch/powerpc/cpu/mpc83xx/cpu_init.c b/arch/powerpc/cpu/mpc83xx/cpu_init.c > index 7a1cae7..cbc4157 100644 > --- a/arch/powerpc/cpu/mpc83xx/cpu_init.c > +++ b/arch/powerpc/cpu/mpc83xx/cpu_init.c > @@ -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. > + setbits_be32(&ehci->control, 0); > > /* Wait for clock to stabilize */ This loop never returns on MPC837x because "PHY_CLK_VALID" isn't valid. > 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 ?
On Mon, 28 Feb 2011 17:18:38 +0100 Andre Schwarz <andre.schwarz@matrix-vision.de> wrote: > sorry to bother you again, but I again stumbled over the discussed USB > init issue : > >>> 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. > > 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. ok, as long as it's confirmed. > > If that's correct, try something like the following so we can determine > > what setting the USB controller didn't agree with: > > > > diff --git a/arch/powerpc/cpu/mpc83xx/cpu_init.c b/arch/powerpc/cpu/mpc83xx/cpu_init.c > > index 7a1cae7..cbc4157 100644 > > --- a/arch/powerpc/cpu/mpc83xx/cpu_init.c > > +++ b/arch/powerpc/cpu/mpc83xx/cpu_init.c > > @@ -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. 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? > > + 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. > > 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
Kim, > On Mon, 28 Feb 2011 17:18:38 +0100 > Andre Schwarz<andre.schwarz@matrix-vision.de> wrote: > >> sorry to bother you again, but I again stumbled over the discussed USB >> init issue : >>>>> 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. >>> 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. > ok, as long as it's confirmed. > >>> If that's correct, try something like the following so we can determine >>> what setting the USB controller didn't agree with: >>> >>> diff --git a/arch/powerpc/cpu/mpc83xx/cpu_init.c b/arch/powerpc/cpu/mpc83xx/cpu_init.c >>> index 7a1cae7..cbc4157 100644 >>> --- a/arch/powerpc/cpu/mpc83xx/cpu_init.c >>> +++ b/arch/powerpc/cpu/mpc83xx/cpu_init.c >>> @@ -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. > 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 ... >>> + 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). >>> 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
diff --git a/arch/powerpc/cpu/mpc83xx/cpu_init.c b/arch/powerpc/cpu/mpc83xx/cpu_init.c index 7a1cae7..cbc4157 100644 --- a/arch/powerpc/cpu/mpc83xx/cpu_init.c +++ b/arch/powerpc/cpu/mpc83xx/cpu_init.c @@ -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 {