Re: [RFC PATCH 4/5] arm: omap2: support port power on lan95xx devices

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

 



On 02/12/12 23:01, the mail apparently from Ming Lei included:

Hi -

This patch defines power controller for powering on/off LAN95xx
USB hub and USB ethernet devices, and implements one match function
to associate the power controller with related USB port device.
The big problem of this approach is that it depends on the global
device ADD/DEL notifier.

Another idea of associating power controller with port device
is by introducing usb port driver, and move all this port power
control stuff from platform code to the port driver, which is just
what I think of and looks doable. The problem of the idea is that
port driver is per board, so maybe cause lots of platform sort of
code to be put under drivers/usb/port/, but this approach can avoid
global device ADD/DEL notifier.

I'd like to get some feedback about which one is better or other choice,
then I may do it in next cycle.

Cc: Andy Green <andy.green@xxxxxxxxxx>
Cc: Roger Quadros <rogerq@xxxxxx>
Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
Cc: Felipe Balbi <balbi@xxxxxx>
Signed-off-by: Ming Lei <tom.leiming@xxxxxxxxx>
---
  arch/arm/mach-omap2/board-omap4panda.c |   99 +++++++++++++++++++++++++++++++-
  1 file changed, 96 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c
index 5c8e9ce..3183832 100644
--- a/arch/arm/mach-omap2/board-omap4panda.c
+++ b/arch/arm/mach-omap2/board-omap4panda.c
@@ -32,6 +32,8 @@
  #include <linux/usb/musb.h>
  #include <linux/wl12xx.h>
  #include <linux/platform_data/omap-abe-twl6040.h>
+#include <linux/power_controller.h>
+#include <linux/usb/port.h>

  #include <asm/hardware/gic.h>
  #include <asm/mach-types.h>
@@ -154,6 +156,99 @@ static struct gpio panda_ehci_gpios[] __initdata = {
  	{ GPIO_HUB_NRESET,	GPIOF_OUT_INIT_LOW,  "hub_nreset" },
  };

+static void ehci_hub_power_on(struct power_controller *pc, struct device *dev)
+{
+	gpio_set_value(GPIO_HUB_NRESET, 1);
+	gpio_set_value(GPIO_HUB_POWER, 1);
+}

You should wait a bit after applying power to the smsc chip before letting go of nReset too. In the regulator-based implementation I sent it's handled by delays encoded in the regulator structs.

+static void ehci_hub_power_off(struct power_controller *pc, struct device *dev)
+{
+	gpio_set_value(GPIO_HUB_NRESET, 0);
+	gpio_set_value(GPIO_HUB_POWER, 0);
+}
+
+static struct usb_port_power_switch_data root_hub_port_data = {
+	.hub_tier	= 0,
+	.port_number = 1,
+	.type = USB_PORT_CONNECT_TYPE_HARD_WIRED,
+};
+
+static struct usb_port_power_switch_data smsc_hub_port_data = {
+	.hub_tier	= 1,
+	.port_number = 1,
+	.type = USB_PORT_CONNECT_TYPE_HARD_WIRED,
+};
+
+static struct power_controller pc = {
+	.name = "omap_hub_eth_pc",
+	.count = ATOMIC_INIT(0),
+	.power_on = ehci_hub_power_on,
+	.power_off = ehci_hub_power_off,
+};
+
+static inline int omap_ehci_hub_port(struct device *dev)
+{
+	/* we expect dev->parent points to ehcd controller */
+	if (dev->parent && !strcmp(dev_name(dev->parent), "ehci-omap.0"))
+		return 1;
+	return 0;
+}
+
+static inline int dev_pc_match(struct device *dev)
+{
+	struct device *anc;
+	int ret = 0;
+
+	if (likely(strcmp(dev_name(dev), "port1")))
+		goto exit;
+
+	if (dev->parent && (anc = dev->parent->parent)) {
+		if (omap_ehci_hub_port(anc)) {
+			ret = 1;
+			goto exit;
+		}
+
+		/* is it port of lan95xx hub? */
+		if ((anc = anc->parent) && omap_ehci_hub_port(anc)) {
+			ret = 2;
+			goto exit;
+		}
+	}
+exit:
+	return ret;
+}
+
+/*
+ * Notifications of device registration
+ */
+static int device_notify(struct notifier_block *nb, unsigned long action, void *data)
+{
+	struct device *dev = data;
+	int ret;
+
+	switch (action) {
+	case DEV_NOTIFY_ADD_DEVICE:
+		ret = dev_pc_match(dev);
+		if (likely(!ret))
+			goto exit;
+		if (ret == 1)
+			dev_pc_bind(&pc, dev, &root_hub_port_data, sizeof(root_hub_port_data));
+		else
+			dev_pc_bind(&pc, dev, &smsc_hub_port_data, sizeof(smsc_hub_port_data));
+		break;
+
+	case DEV_NOTIFY_DEL_DEVICE:
+		break;
+	}
+exit:
+	return 0;
+}
+
+static struct notifier_block usb_port_nb = {
+	.notifier_call = device_notify,
+};
+

Some thoughts on trying to make this functionality specific to power only and ehci hub port only:

- Quite a few boards have smsc95xx... they're all going to carry these additions in the board file? Surely you'll have to generalize the pieces that perform device_path business out of the omap4panda board file at least. At that point the path matching code becomes generic end-of-the-device-path matching code.

- How could these literals like "port1" etc be nicely provided by Device Tree? In ARM-land there's pressure to eventually eliminate board files completely and pass in everything from dtb. device_asset can neatly grow DT bindings in a generic way, since the footprint in the board file is some regulators that already have dt bindings and some device_asset structs. Similarly there's pressure for magic code to service a board (rather than SoC) to go elsewhere than the board file.

- Shouldn't this take care of enabling and disabling the ULPI PHY clock on Panda too? There's no purpose leaving it running if the one thing the ULPI PHY is connected to is depowered, and when you do power it, on Panda you will reset the ULPI PHY at the same time anyway (smsc reset and ULPI PHY reset are connected together on Panda). Then you can eliminate omap4_ehci_init() in the board file.

-Andy

  static void __init omap4_ehci_init(void)
  {
  	int ret;
@@ -178,12 +273,10 @@ static void __init omap4_ehci_init(void)

  	gpio_export(GPIO_HUB_POWER, 0);
  	gpio_export(GPIO_HUB_NRESET, 0);
-	gpio_set_value(GPIO_HUB_NRESET, 1);

  	usbhs_init(&usbhs_bdata);

-	/* enable power to hub */
-	gpio_set_value(GPIO_HUB_POWER, 1);
+	dev_register_notifier(&usb_port_nb);
  }

  static struct omap_musb_board_data musb_board_data = {



--
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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux