[U-Boot] [PATCH] mvebu: usb: Add missing controller reset after initialization

[Marek Vasut <marex@denx.de>]

On Monday, January 04, 2016 at 12:38:07 AM, Phil Sutter wrote:
> 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.

OK, so the controller is OTG capable and upon reset, it's in Gadget mode?

> 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?

It's the USBMODE register, see for example [1] . It's part of EHCI cores
which are OTG capable.

[1] http://lxr.free-electrons.com/source/drivers/usb/host/ehci-hcd.c#L179

> 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.

Probably add a fixup into ehci-orion.c in Linux or something along those
lines. It should configure the code according to the "dr_mode" DT prop.