public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Phil Sutter <phil@nwl.cc>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH] mvebu: usb: Add missing controller reset after initialization
Date: Mon, 4 Jan 2016 00:38:07 +0100	[thread overview]
Message-ID: <20160103233643.3CAC061C7D@mail.nwl.cc> (raw)
In-Reply-To: <20160103085923.B6C3038226E@gemini.denx.de>

Hi,

On Sun, Jan 03, 2016 at 09:59:23AM +0100, Wolfgang Denk wrote:
> [added USB custodian to Cc:]
> 
> In message <20160102211834.E49A161C74@mail.nwl.cc> you wrote:
> > In order to allow for Linux properly register the integrated EHCI host,
> > we have to reset it after initialization. Without this, enumeration
> > fails if 'usb start' wasn't issued prior to booting Linux.
> 
> I think this reset should actually be done in the Linux driver.
> 
> From what you write I suspect that after a "usb stop" in U-Boot the
> Linux driver maight fail, too.  Leaving USB enabled in U-Boot is
> extremely dangerous - see

Ah, thanks for pointing this out! I had no idea the HCD does things like
this. Though I'm a bit confused now how to solve the issue at hand:

> commit 3d71c81a9bb03f866a1e98da96363ef3f46c76b3
> Author: Markus Klotzb?cher <mk@denx.de>
> Date:   Thu Jul 10 14:47:09 2008 +0200
> 
>     USB: shutdown USB before booting
>     
>     This patch fixes a potentially serious issue related to USB which was
>     discouvered by Martin Krause <martin.krause@tqs.de> and fixed for
>     ARM920T. Martin wrote: 
>     
>       Turn off USB to prevent the host controller from writing to the 
>       SDRAM while Linux is booting. This could happen, because the HCCA
>       (Host Controller Communication Area) lies within the SDRAM and the
>       host controller writes continously to this area (as busmaster!), for
>       example to increase the HccaFrameNumber variable, which happens
>       every 1 ms.
>     
>     This is a slightly modified version of the patch in order to shutdown
>     USB when booting on all architectures.
>     
>     Signed-off-by: Markus Klotzbuecher <mk@denx.de>

This commit exists in my tree, therefore when I call 'bootm' it should
stop USB. To be sure, I tested calling 'usb stop' manually before doing
'tftpboot; bootm' and the enumeration still succeeds in Linux.

Since this all does not seem to make much sense, I reviewed my changes
again to see if the controller really is enabled after the reset - it is
not, but I found something more important: The reset is actually not
necessary at all!

The bit which really was missing is the USB mode setting I smuggled in
along with my patch - setting the devices to host mode is sufficient for
Linux to successfully enumerate the HCD.

Checking the logs of the vendor's U-Boot fork, it appears that devices 1
and 2 are configured to host mode, while the third is set to device
mode. I changed my code to copy that, but am not sure it's necessary at
all: The DS414 exports only a single port of the SoC's EHCI, and Linux
detects that:

| orion-ehci f1050000.usb: EHCI Host Controller
| orion-ehci f1050000.usb: new USB bus registered, assigned bus number 3
| orion-ehci f1050000.usb: irq 27, io mem 0xf1050000
| orion-ehci f1050000.usb: USB 2.0 started, EHCI 1.00
| hub 3-0:1.0: USB hub found
| hub 3-0:1.0: 1 port detected

OTOH I'm not sure how far this configuration is device specific in the
first place. What puzzles me is that I couldn't find a reference to this
USB mode register at 0x501A8 in Marvell's MV78230 specs, still it seems
to be crucial. Does anyone of you have this register referenced in some
Marvell datasheet somewhere?

Maybe there's also a more generic way to do this, 'usb start' (which
solves the problem without any changes to the SoC init code) seems to
not address this register.

Thanks, Phil

  parent reply	other threads:[~2016-01-03 23:38 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-02 21:19 [U-Boot] [PATCH] mvebu: usb: Add missing controller reset after initialization Phil Sutter
2016-01-03  8:59 ` Wolfgang Denk
2016-01-03  9:35   ` Marek Vasut
2016-01-03 23:38   ` Phil Sutter [this message]
2016-01-03 23:47     ` Marek Vasut
2016-01-04  3:02       ` Phil Sutter
2016-01-04 11:25         ` Marek Vasut
2016-03-24  9:13           ` Stefan Roese
2016-03-24  9:47             ` Phil Sutter

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=20160103233643.3CAC061C7D@mail.nwl.cc \
    --to=phil@nwl.cc \
    --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