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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/22/2012 06:12 PM, Felipe Balbi wrote:
> Hi,
> 
> On Thu, Nov 22, 2012 at 05:00:45PM +0200, Roger Quadros wrote:
>> On 11/22/2012 03:56 PM, Felipe Balbi wrote:
>>> Hi,
>>>
>>> On Thu, Nov 22, 2012 at 09:49:05PM +0800, Andy Green wrote:
>>>>> Again it sounds like something that should be done at the hub driver
>>>>> level. I mean using the GPIO (for reset) and Power Regulator.
>>>>>
>>>>> not implementing the regulator part itself, but using it.
>>>>
>>>> If you mean change the regulator managing code from living in
>>>> omap-hsusb to ehci-hcd, it sounds like a good idea.
>>>
>>> I mean that drivers/usb/core/hub.c should manage it, since that's what
>>> actually manages the HUB entity.
>>
>> Yes. I agree that powering up the on-board hubs should be done
>> generically and not in ehci-omap.c. I'm not sure how it can be done in
>> hub.c.
>>
>> I'm assuming that such problem is limited to root hub ports where system
> 
> an external LAN95xx HUB is not the roothub. LAN95xx is connected to the
> roothub.
> 

I didn't say LAN95xx is the root hub. It is connected to the root hub.
So it is in theory, the root hub port's responsibility to power the LAN95xx.

>> designers have the flexibility to provide power to the peripherals
>> outside the USB Hub specification.
>>
>> I can think of two solutions
>>
>> 1) Associate the regulators with the root hub _ports_ and enable them as
>> part of port's power-up routine.
> 
> doesn't make sense. We need to figure out a way to attach the regulator
> to the ports which actually have them. In this case it's the external
> LAN95xx, right ?

I think you are confused here. The LAN95xx's ports are compatible with
USB hub specification and they work using the in-band set_port_feature
mechanism already. We have a problem powering the LAN95xx itself which
ideally should have been powered with set_port_feature on the root hub.
(or ehci_port_power() in this case).

> 
>> 2) Have a global list of regulators that are registered by platform code
>> and enabled them all at once on hcd init.
> 
> also wrong as it might cause increased power consumption even when only
> a single USB port is currently in use.

Yes that is true. I'm not for (2) certainly.
> 
> The patch below is *CLEARLY WRONG* it's just to illustrate how this
> could be started:
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 1af04bd..662d4cf 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -26,6 +26,7 @@
>  #include <linux/mutex.h>
>  #include <linux/freezer.h>
>  #include <linux/random.h>
> +#include <linux/regulator/consumer.h>
>  
>  #include <asm/uaccess.h>
>  #include <asm/byteorder.h>
> @@ -44,6 +45,7 @@ struct usb_port {
>  	struct device dev;
>  	struct dev_state *port_owner;
>  	enum usb_port_connect_type connect_type;
> +	struct regulator *port_regulator;
>  };
>  
>  struct usb_hub {
> @@ -847,8 +849,41 @@ static unsigned hub_power_on(struct usb_hub *hub, bool do_delay)
>  	else
>  		dev_dbg(hub->intfdev, "trying to enable port power on "
>  				"non-switchable hub\n");
> -	for (port1 = 1; port1 <= hub->descriptor->bNbrPorts; port1++)
> +	for (port1 = 1; port1 <= hub->descriptor->bNbrPorts; port1++) {
> +		regulator_enable(hub->ports[port1]->port_regulator);
>  		set_port_feature(hub->hdev, port1, USB_PORT_FEAT_POWER);
> +	}
> +
> +	/* Wait at least 100 msec for power to become stable */
> +	delay = max(pgood_delay, (unsigned) 100);
> +	if (do_delay)
> +		msleep(delay);
> +	return delay;
> +}
> +
> +static unsigned hub_power_off(struct usb_hub *hub, bool do_delay)
> +{
> +	int port1;
> +	unsigned pgood_delay = hub->descriptor->bPwrOn2PwrGood * 2;
> +	unsigned delay;
> +	u16 wHubCharacteristics =
> +			le16_to_cpu(hub->descriptor->wHubCharacteristics);
> +
> +	/* Disable power on each port.  Some hubs have reserved values
> +	 * of LPSM (> 2) in their descriptors, even though they are
> +	 * USB 2.0 hubs.  Some hubs do not implement port-power switching
> +	 * but only emulate it.  In all cases, the ports won't work
> +	 * unless we send these messages to the hub.
> +	 */
> +	if ((wHubCharacteristics & HUB_CHAR_LPSM) < 2)
> +		dev_dbg(hub->intfdev, "disabling power on all ports\n");
> +	else
> +		dev_dbg(hub->intfdev, "trying to disable port power on "
> +				"non-switchable hub\n");
> +	for (port1 = 1; port1 <= hub->descriptor->bNbrPorts; port1++) {
> +		regulator_disable(hub->ports[port1]->port_regulator);
> +		clear_port_feature(hub->hdev, port1, USB_PORT_FEAT_POWER);
> +	}
>  
>  	/* Wait at least 100 msec for power to become stable */
>  	delay = max(pgood_delay, (unsigned) 100);
> @@ -1336,6 +1371,9 @@ static int hub_configure(struct usb_hub *hub,
>  		goto fail;
>  	}
>  
> +	if (hub->has_external_port_regulator) /* maybe implement with a quirk flag ??? */
> +		regulator_get(hub_dev, "hub_port_supply\n");
> +
>  	wHubCharacteristics = le16_to_cpu(hub->descriptor->wHubCharacteristics);
>  
>  	/* FIXME for USB 3.0, skip for now */
> @@ -4205,8 +4243,10 @@ static void hub_port_connect_change(struct usb_hub *hub, int port1,
>  
>  		/* maybe switch power back on (e.g. root hub was reset) */
>  		if ((wHubCharacteristics & HUB_CHAR_LPSM) < 2
> -				&& !port_is_power_on(hub, portstatus))
> +				&& !port_is_power_on(hub, portstatus)) {
> +			regulator_enable(hub->ports[port1]->port_regulator);
>  			set_port_feature(hdev, port1, USB_PORT_FEAT_POWER);
> +		}
>  
>  		if (portstatus & USB_PORT_STAT_ENABLE)
>    			goto done;
> 
> Note that if we have a single regulator, than all ports' regulators
> should point to the same struct regulator so that regulator_get() and
> regulator_put() can do proper reference counting.
> 
> This is just an idea though.
> 

Thanks for the example. The only problem is, how do we associate a
regulator to a specific port in the system.

cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux