Re: [PATCH 16/16] ARM: OMAP: omap4panda: Power down the USB PHY and ETH when not in use

[Andy Green <andy.green-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]

On 11/22/12 03:54, the mail apparently from Felipe Balbi included:
> Hi,
>
> On Wed, Nov 21, 2012 at 06:07:57PM +0200, Roger Quadros wrote:
>> On 11/21/2012 05:32 PM, Alan Stern wrote:
>>> On Wed, 21 Nov 2012, Roger Quadros wrote:
>>>
>>>> On 11/21/2012 04:52 PM, Alan Stern wrote:
>>>>> On Wed, 21 Nov 2012, Felipe Balbi wrote:
>>>>>
>>>>>> On Thu, Nov 15, 2012 at 04:34:14PM +0200, Roger Quadros wrote:
>>>>>>> From: Andy Green <andy.green-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>>>>>>>
>>>>>>> This patch changes the management of the two GPIO for
>>>>>>> "hub reset" (actually controls enable of ULPI PHY and hub reset) and
>>>>>>> "hub power" (controls power to hub + eth).
>>>>>>
>>>>>> looks like this should be done by the hub driver. Alan, what would you
>>>>>> say ? Should the hub driver know how to power itself up ?
>>>>>
>>>>> Not knowing the context, I'm a little confused.  What is this hub
>>>>> you're talking about?  Is it a separate USB hub incorporated into the
>>>>> IP (like Intel's "rate-matching" hubs in their later chipsets)?  Or is
>>>>> it the root hub?
>>>>>
>>>>
>>>> This is actually a USB HUB + Ethernet combo chip (LAN9514) that is hard
>>>> wired on the panda board with its Power and Reset pins controlled by 2
>>>> GPIOs from the OMAP SoC.
>>>>
>>>> When powered, this chip can consume significant power (~0.7 W) because
>>>> of the (integrated Ethernet even when suspended. I suppose the ethernet
>>>> driver SMSC95XX) doesn't put it into a low enough power state on suspend.
>>>>
>>>> It doesn't make sense to power the chip when USB is not required on the
>>>> whole (e.g. ehci_hcd module is not loaded). This is what this patch is
>>>> trying to fix.
>>>
>>> Ah, okay.  Then yes, assuming you want to power this chip only when
>>> either ehci-hcd or the ethernet driver is loaded, the right places
>>> to manage the power GPIO are in the init and exit routines of the two
>>> drivers.
>>>
>>
>> The Ethernet function actually connects over USB within the chip. So
>> managing the power only in the ehci-hcd driver should suffice.
>
> the thing is that this could cause code duplication all over. LAN95xx is

I can see this point.  However DT will soak up these regulator 
definitions, at that point it's "for free".  On Linaro tilt-3.4 we 
already have this on the dts

	hubpower: fixedregulator@0 {
			compatible = "regulator-fixed";
			regulator-name = "vhub0";
			regulator-min-microvolt = <3300000>;
			regulator-max-microvolt = <3300000>;
			gpio = <&gpio1 1 0>;		/* gpio 1 : HUB Power */
			startup-delay-us = <70000>;
			enable-active-high;
	};

	hubreset: fixedregulator@1 {
			compatible = "regulator-fixed";
			regulator-name = "hsusb0";	/* tag to associate with PORT 1 */
			regulator-min-microvolt = <3300000>;
			regulator-max-microvolt = <3300000>;
			gpio = <&gpio2 30 0>;	/* gpio 62 : HUB & PHY Reset */
			startup-delay-us = <70000>;
			enable-active-high;
			vin-supply = "vhub0"; /* Makes regulator f/w enable power before reset */
	};

which is the equivalent to my patch: I don't think we need to sweat 
about code duplication.

> a generic USB Hub + Ethernet combo on one of ports from SMSC. *Any*
> platform could use it and could hook those power and reset pins to a
> GPIO. If we handle this at ehci-omap level, we risk having to duplicate
> the same piece of code for ehci-*.c

We need to consider power and reset separately.  Reset is a safe bet as 
a GPIO but power to the smsc chip is not.

On panda they happen to fit a gpio-controlled linear regulator to 
provide the smsc 3.3V supply.  On another device that can just as easily 
be a PMIC regulator channel: it boils down to controlling a power rail. 
  So using struct regulator as the abstraction for the power is the 
right way already.

> Since that's a hub driver anyway, I wonder if it would be better to play
> with those gpios somewhere else ?!?

The patch creates a regulator that binds to a magic regulator name 
defined by the hsusb driver, "hsusb0".

That regulator is taken up and down by hsusb driver with the lifecycle 
of the logical hsusb device.  So inserting ehci-hcd module powers the 
regulator until the module is removed.

Originally I bound the regulator to the smsc95xx driver, which also 
offers a struct regulator.  But there is a quirk on Panda that means 
that is not workable.

On Panda, they share one SoC gpio to reset both an external ULPI PHY 
chip and the smsc95xx that is downstream of it.

After you power up the smsc95xx, you must reset it in order for correct 
operation.  This actually resets the ULPI PHY too.

The ULPI PHY is permanently powered, only nRESET is provided to control it.

For that reason, as an attribute of being on a Panda board, we need to 
do the (shared) reset meddling once at hsusb init, and that is why the 
patch is as it is.

-Andy

-- 
Andy Green | TI Landing Team Leader
Linaro.org │ Open source software for ARM SoCs | Follow Linaro
http://facebook.com/pages/Linaro/155974581091106  - 
http://twitter.com/#!/linaroorg - http://linaro.org/linaro-blog
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html