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]

 



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.

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

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

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.

-- 
balbi

Attachment: signature.asc
Description: Digital signature


[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