public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Andre Schwarz <andre.schwarz@matrix-vision.de>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] [MPC837x v2] Make it work again with USB.
Date: Thu, 31 Mar 2011 09:52:06 +0200	[thread overview]
Message-ID: <4D9432A6.4030701@matrix-vision.de> (raw)
In-Reply-To: <20110330201414.a930d14a.kim.phillips@freescale.com>

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

  reply	other threads:[~2011-03-31  7:52 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-11-16 12:37 [U-Boot] [PATCH] [MPC837x v2] Make it work again with USB Andre Schwarz
2010-11-28 15:10 ` Kim Phillips
2010-11-29  7:59   ` Schwarz, Andre
2010-12-13 22:52     ` Kim Phillips
2011-02-28 16:18       ` Andre Schwarz
2011-03-31  1:14         ` Kim Phillips
2011-03-31  7:52           ` Andre Schwarz [this message]
2011-04-01 21:53             ` [U-Boot] [PATCH] mpc83xx: restrict UTMI PHY configuration to 831x parts Kim Phillips
2011-04-04 10:52               ` Andre Schwarz
2011-04-05  1:26                 ` Kim Phillips

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4D9432A6.4030701@matrix-vision.de \
    --to=andre.schwarz@matrix-vision.de \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox