Re: [PATCH] usb: host: tegra: Reset Tegra USB controller before init

[Stephen Warren <swarren@wwwdotorg.org>]

On 02/28/2013 11:52 AM, Alan Stern wrote:
> On Thu, 28 Feb 2013, Stephen Warren wrote:
> 
>> I think this patch might cause unintended consequences.
>>
>> When the Tegra PHY code is converted to a driver (i.e. has its own
>> probe), the initial order of execution of the PHY and EHCI driver probes
>> will not be guaranteed.
>>
>> In particular, since the EHCI probe will attempt to "find" the PHY
>> device, and defer the EHCI probe until it can do so, this guarantees
>> that the PHY's probe() will have completed before EHCI's probe()
>> completes (although EHCI's probe may start running first some number of
>> times, and be retried with -EPROBE_DEFERRED for a variety of reasons).
>>
>> Now, if the PHY driver's probe() actually touches HW and sets up some
>> registers, isn't this reset call going to trash any of that register
>> setup? Or, will PHY probe() not touch registers, but only do so during
>> the standard PHY open/init "op"/API calls?
>>
>> I think the way to solve this is to put the reset call into the PHY
>> driver. I assume it has access to the appropriate clock object. This may
>> also address Alan's query re: when the unexpected interrupt occurs; it's
>> triggered by (or correlated with at least) the PHY (or USB port in
>> general) being in device mode due to the boot ROM setting it up this
>> way, then switching to host mode via the Linux driver. I /think/ that
>> device/host mode switching is more related to the PHY than EHCI driver,
>> although I could well be wrong here.
> 
> With the PCI platform driver, the handoff from the firmware (we can
> categorize U-Boot as firmware for this discussion) is handled as soon
> as the controller is discovered by the platform-specific code.  
> There's a special pci-quirks.c file to take care of it.  It is not
> handled by the driver or the glue layer.
> 
> In general I think that's what needs to be done.  Errant interrupt 
> sources should be disabled as quickly as possible.
> 
> In this case I don't know exactly when the earliest opportunity is.  I
> assume that the EHCI driver and/or the PHY driver gets probed because
> some platform-layer code has registered the device.  If this is so then
> that platform-layer code is the right place to do the reset.

The first platform-specific code that is executed in this case is the
(struct platform_driver) probe() functions for the PHY and/or EHCI device.

On Tegra, all devices are listed in device tree, and core kernel code
instantiates platform devices based on the information in device tree.
The first non-core code that runs on a platform device is the driver's
probe() method. That probe() method is for a very specific piece of HW,
and hence seems to be the best place to put any quirks for it. Putting
the quirks outside the driver would mean some piece of the core DT code
would need to have a quirk list, and be able to map the registers for
the device, acquire clocks, etc. etc., implement the quirk, and tear it
all down again. All of which would just be duplicated with the driver's
own probe() function.