[try#1 PATCH 0/7] Introduce device_asset and use to control Panda HUB+ETH power and clock

[try#1 PATCH 0/7] Introduce device_asset and use to control Panda HUB+ETH power and clock

[Andy Green]

The following series implements the device_asset addition to
struct device that has been discussed on the usb and omap lists
with Alan Stern and GKH.  Several of the ideas here are from Alan
Stern.

First we add the new struct to linux/device.h and cause it to be
used just before device probe with one device_asset-specific
callback to "enable" the assets and just after device removal
with another device_asset-specific callback to "disable" the
associated assets.

Typical assets are things like regulators and clocks which are
not known to the generic driver but which are bound to the
specific device instance by hardware design.

Second we provide stock, default device_asset callbacks if your
asset is a struct regulator.  You can just use those and the
regulator will have a get and enable until the device instance
is removed, when it will be disabled and put.

Third we provide the same for struct clk.  These stock callbacks
mean there won't be any code duplication caused in the common case.

Fourth we remove all the struct regulator code from ehci-omap.

Fifth we implement Panda HUB+ETH (smsc95xx chip) regulator control
using the device_asset scheme.  Since the right device, ehci-omap.0
is created under mfd, the device_asset array is passed in
platform_data to the right place where it's added to the
platform_device's device before registration.

Sixth we add the Panda's external ULPI PHY clock to the
device_asset array we established for the regulator case above.

The end result is we are able to remove the code about regulator
and don't need any for clock control in the generic drivers.
Instead we are able to associate the board-specific prerequisites
for the drivers to work on a board externally with the enable and
disable hidden in the plumbing.

The end result is power and clock to the HUB+PHY (smsc95xx chip)
remains off until ehci-hcd module is inserted, and both are
disabled again when the module is removed.

---

Andy Green (7):
      drivers: base: introduce device assets
      regulator: core: add default device asset handlers
      clk: add default device asset handlers
      usb: omap ehci: remove all regulator control from ehci omap
      omap4: panda: add smsc95xx regulator and reset dependent on root hub
      omap4 panda add smsc95xx clock dependent on root hub
      config omap2plus add ehci bits


 arch/arm/configs/omap2plus_defconfig   |   42 +++++-------
 arch/arm/mach-omap2/board-omap4panda.c |  113 +++++++++++++++++++++++---------
 arch/arm/mach-omap2/usb-host.c         |    2 -
 arch/arm/plat-omap/include/plat/usb.h  |    9 +--
 drivers/base/dd.c                      |   36 ++++++++++
 drivers/clk/clkdev.c                   |   39 +++++++++++
 drivers/mfd/omap-usb-host.c            |    9 ++-
 drivers/regulator/core.c               |   34 ++++++++++
 drivers/usb/host/ehci-omap.c           |   37 ----------
 include/linux/clk.h                    |   33 +++++++++
 include/linux/device.h                 |   25 +++++++
 include/linux/regulator/machine.h      |   30 ++++++++
 12 files changed, 303 insertions(+), 106 deletions(-)

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

[try#1 PATCH 1/7] drivers: base: introduce device assets

[Andy Green]

This patch adds support for a new struct device member "assets"
which may point to an array of struct assets.

The array is terminated by one with a NULL pre_probe callback.

These assets consist of named (in .name) or anonymous object
pointers (.data) operated on by specified callbacks.  A void *
is provided to give configuration data or pointer if needed.

Before device probe, any assets associated with the device have
their pre_probe() callback called, which will typically "enable"
them, and after device removal the post_remove() callback is
called which will typically disable them.

Signed-off-by: Andy Green <andy.green@linaro.org>
---
 drivers/base/dd.c      |   36 ++++++++++++++++++++++++++++++++++++
 include/linux/device.h |   25 +++++++++++++++++++++++++
 2 files changed, 61 insertions(+)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index e3bbed8..d37210a 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -261,6 +261,7 @@ static DECLARE_WAIT_QUEUE_HEAD(probe_waitqueue);
 
 static int really_probe(struct device *dev, struct device_driver *drv)
 {
+	struct device_asset *asset;
 	int ret = 0;
 
 	atomic_inc(&probe_count);
@@ -275,6 +276,23 @@ static int really_probe(struct device *dev, struct device_driver *drv)
 		goto probe_failed;
 	}
 
+	asset = dev->assets;
+	while (asset && asset->pre_probe) {
+		dev_info(dev, "Enabling pre-probe asset %s\n", asset->name);
+		ret = asset->pre_probe(dev, asset);
+		if (ret) {
+			dev_err(dev, "Error Enabling pre-probe asset %s\n",
+								  asset->name);
+			if (asset != dev->assets)
+				do {
+					asset--;
+					asset->post_remove(dev, asset);
+				} while (asset != dev->assets);
+			goto probe_failed;
+		}
+		asset++;
+	}
+
 	if (dev->bus->probe) {
 		ret = dev->bus->probe(dev);
 		if (ret)
@@ -478,6 +496,7 @@ EXPORT_SYMBOL_GPL(driver_attach);
 static void __device_release_driver(struct device *dev)
 {
 	struct device_driver *drv;
+	struct device_asset *asset;
 
 	drv = dev->driver;
 	if (drv) {
@@ -496,6 +515,23 @@ static void __device_release_driver(struct device *dev)
 			dev->bus->remove(dev);
 		else if (drv->remove)
 			drv->remove(dev);
+
+		asset = dev->assets;
+		if (asset) {
+			/* remove in reverse order */
+			while (asset->pre_probe)
+				asset++;
+
+			if (asset != dev->assets)
+				do {
+					asset--;
+					dev_info(dev,
+					   "Disabling post-remove asset %s\n",
+								  asset->name);
+					asset->post_remove(dev, asset);
+				} while (asset != dev->assets);
+		}
+
 		devres_release_all(dev);
 		dev->driver = NULL;
 		dev_set_drvdata(dev, NULL);
diff --git a/include/linux/device.h b/include/linux/device.h
index 86ef6ab..6eabe1d 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -577,6 +577,26 @@ struct device_dma_parameters {
 };
 
 /**
+ * struct device_asset - a prerequisite for this device's probing
+ * @name:	Name of the regulator, clock, etc.  Optional.
+ * @asset:	Pointer to the regulator, clock, etc.  If no name is given,
+ *		this can be set before device probe, otherwise the pre_probe
+ *		handler will dereference the name and store a pointer here
+ * @data:	Optional configuration data the asset may need
+ * @pre_probe:	Called before this device this is associated with gets
+ *		probed.
+ * @post_remove: Called after this device instance gets removed.
+ */
+
+struct device_asset {
+	const char *name;
+	void *asset;
+	void *data;
+	int (*pre_probe)(struct device *device, struct device_asset *asset);
+	void (*post_remove)(struct device *device, struct device_asset *asset);
+};
+
+/**
  * struct device - The basic device structure
  * @parent:	The device's "parent" device, the device to which it is attached.
  * 		In most cases, a parent device is some sort of bus or host
@@ -600,6 +620,9 @@ struct device_dma_parameters {
  * 		variants, which GPIO pins act in what additional roles, and so
  * 		on.  This shrinks the "Board Support Packages" (BSPs) and
  * 		minimizes board-specific #ifdefs in drivers.
+ * @assets:	Pointer to a NULL-pre_probe terminated array of named or
+ *		pointed-to objects that should be enabled for this device
+ *		just before probe and disabled after removal
  * @power:	For device power management.
  * 		See Documentation/power/devices.txt for details.
  * @pm_domain:	Provide callbacks that are executed during system suspend,
@@ -653,6 +676,8 @@ struct device {
 					   device */
 	void		*platform_data;	/* Platform specific data, device
 					   core doesn't touch it */
+	struct device_asset *assets; /* optional assets enabled before probe
+				      * and disabled after removal */
 	struct dev_pm_info	power;
 	struct dev_pm_domain	*pm_domain;
 

[try#1 PATCH 2/7] regulator: core: add default device asset handlers

[Andy Green]

This adds default device_asset handlers for struct regulator.  If you
want an associated regulator asset of a device to be powered before
probe, and depowered after removal, these callbacks will take care of
everything including get and put.

By defining them here in regulator core, code duplication at the usages
is nipped in the bud.

Signed-off-by: Andy Green <andy.green-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/regulator/core.c          |   34 ++++++++++++++++++++++++++++++++++
 include/linux/regulator/machine.h |   30 ++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index e872c8b..1834e41 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -3495,6 +3495,40 @@ void regulator_unregister(struct regulator_dev *rdev)
 }
 EXPORT_SYMBOL_GPL(regulator_unregister);
 
+/*
+ * Default handlers for regulator asset preprobe and postremoval
+ */
+int regulator_asset_default_preprobe(struct device *device,
+						struct device_asset *asset)
+{
+	struct regulator **reg = (struct regulator **)&asset->asset;
+	int n;
+
+	*reg = regulator_get(device, asset->name);
+	if (IS_ERR(*reg))
+		return PTR_ERR(*reg);
+	n = regulator_enable(*reg);
+	if (n < 0)
+		regulator_put(*reg);
+
+	dev_info(device, "Enabled regulator asset %s\n", asset->name);
+
+	return n;
+}
+EXPORT_SYMBOL_GPL(regulator_asset_default_preprobe);
+
+void regulator_asset_default_postremove(struct device *device,
+						struct device_asset *asset)
+{
+	struct regulator *reg = asset->asset;
+
+	dev_info(device, "Disabling post-remove asset %s\n", asset->name);
+
+	regulator_disable(reg);
+	regulator_put(reg);
+}
+EXPORT_SYMBOL_GPL(regulator_asset_default_postremove);
+
 /**
  * regulator_suspend_prepare - prepare regulators for system wide suspend
  * @state: system suspend state
diff --git a/include/linux/regulator/machine.h b/include/linux/regulator/machine.h
index 36adbc8..151a330 100644
--- a/include/linux/regulator/machine.h
+++ b/include/linux/regulator/machine.h
@@ -191,9 +191,39 @@ int regulator_suspend_prepare(suspend_state_t state);
 int regulator_suspend_finish(void);
 
 #ifdef CONFIG_REGULATOR
+/**
+ * regulator_asset_default_preprobe: default probe handler for regulator assets
+ * @device:	the device whose assets we are enabling just before probing it
+ * @asset: the named regulator asset we are going to get and enable
+ *
+ * You can give this as the handler for .pre_probe callback in device_asset to
+ * deal with pre-enabling/get of a device's named regulator assets
+ */
+int regulator_asset_default_preprobe(struct device *device,
+						struct device_asset *asset);
+/**
+ * regulator_asset_default_postremove: default remove handler for reg assets
+ * @device:	the device whose assets we are disabling just after removing it
+ * @asset: the regulator asset we are going to disable and put
+ *
+ * You can give this as the handler for .post_remove callback in device_asset
+ * to deal with post-disabling/put of a device's regulator assets
+ */
+void regulator_asset_default_postremove(struct device *device,
+						struct device_asset *asset);
+
 void regulator_has_full_constraints(void);
 void regulator_use_dummy_regulator(void);
 #else
+static inline int regulator_asset_default_preprobe(struct device *device,
+						    struct device_asset *asset)
+{
+	return 0;
+}
+static inline void regulator_asset_default_postremove(struct device *device,
+						    struct device_asset *asset)
+{
+}
 static inline void regulator_has_full_constraints(void)
 {
 }

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

[try#1 PATCH 4/7] usb: omap ehci: remove all regulator control from ehci omap

[Andy Green]

This series migrates it to be assets of the logical ehci-omap.0
device object.

Signed-off-by: Andy Green <andy.green-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm/mach-omap2/usb-host.c        |    1 -
 arch/arm/plat-omap/include/plat/usb.h |    7 ------
 drivers/usb/host/ehci-omap.c          |   37 ---------------------------------
 3 files changed, 45 deletions(-)

diff --git a/arch/arm/mach-omap2/usb-host.c b/arch/arm/mach-omap2/usb-host.c
index 3c43449..98f3287 100644
--- a/arch/arm/mach-omap2/usb-host.c
+++ b/arch/arm/mach-omap2/usb-host.c
@@ -498,7 +498,6 @@ void __init usbhs_init(const struct usbhs_omap_board_data *pdata)
 		ohci_data.port_mode[i] = pdata->port_mode[i];
 		ehci_data.port_mode[i] = pdata->port_mode[i];
 		ehci_data.reset_gpio_port[i] = pdata->reset_gpio_port[i];
-		ehci_data.regulator[i] = pdata->regulator[i];
 	}
 	ehci_data.phy_reset = pdata->phy_reset;
 	ohci_data.es2_compatibility = pdata->es2_compatibility;
diff --git a/arch/arm/plat-omap/include/plat/usb.h b/arch/arm/plat-omap/include/plat/usb.h
index 87ee140..70acdee 100644
--- a/arch/arm/plat-omap/include/plat/usb.h
+++ b/arch/arm/plat-omap/include/plat/usb.h
@@ -36,12 +36,6 @@ struct usbhs_omap_board_data {
 	unsigned			es2_compatibility:1;
 
 	unsigned			phy_reset:1;
-
-	/*
-	 * Regulators for USB PHYs.
-	 * Each PHY can have a separate regulator.
-	 */
-	struct regulator		*regulator[OMAP3_HS_USB_PORTS];
 };
 
 #ifdef CONFIG_ARCH_OMAP2PLUS
@@ -49,7 +43,6 @@ struct usbhs_omap_board_data {
 struct ehci_hcd_omap_platform_data {
 	enum usbhs_omap_port_mode	port_mode[OMAP3_HS_USB_PORTS];
 	int				reset_gpio_port[OMAP3_HS_USB_PORTS];
-	struct regulator		*regulator[OMAP3_HS_USB_PORTS];
 	unsigned			phy_reset:1;
 };
 
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index 44e7d0f..7ab7c54 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -40,7 +40,6 @@
 #include <linux/slab.h>
 #include <linux/usb/ulpi.h>
 #include <plat/usb.h>
-#include <linux/regulator/consumer.h>
 #include <linux/pm_runtime.h>
 #include <linux/gpio.h>
 #include <linux/clk.h>
@@ -149,19 +148,6 @@ static int omap_ehci_init(struct usb_hcd *hcd)
 	return rc;
 }
 
-static void disable_put_regulator(
-		struct ehci_hcd_omap_platform_data *pdata)
-{
-	int i;
-
-	for (i = 0 ; i < OMAP3_HS_USB_PORTS ; i++) {
-		if (pdata->regulator[i]) {
-			regulator_disable(pdata->regulator[i]);
-			regulator_put(pdata->regulator[i]);
-		}
-	}
-}
-
 /* configure so an HC device and id are always provided */
 /* always called with process context; sleeping is OK */
 
@@ -175,14 +161,11 @@ static void disable_put_regulator(
 static int ehci_hcd_omap_probe(struct platform_device *pdev)
 {
 	struct device				*dev = &pdev->dev;
-	struct ehci_hcd_omap_platform_data	*pdata = dev->platform_data;
 	struct resource				*res;
 	struct usb_hcd				*hcd;
 	void __iomem				*regs;
 	int					ret = -ENODEV;
 	int					irq;
-	int					i;
-	char					supply[7];
 
 	if (usb_disabled())
 		return -ENODEV;
@@ -223,23 +206,6 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 	hcd->rsrc_len = resource_size(res);
 	hcd->regs = regs;
 
-	/* get ehci regulator and enable */
-	for (i = 0 ; i < OMAP3_HS_USB_PORTS ; i++) {
-		if (pdata->port_mode[i] != OMAP_EHCI_PORT_MODE_PHY) {
-			pdata->regulator[i] = NULL;
-			continue;
-		}
-		snprintf(supply, sizeof(supply), "hsusb%d", i);
-		pdata->regulator[i] = regulator_get(dev, supply);
-		if (IS_ERR(pdata->regulator[i])) {
-			pdata->regulator[i] = NULL;
-			dev_dbg(dev,
-			"failed to get ehci port%d regulator\n", i);
-		} else {
-			regulator_enable(pdata->regulator[i]);
-		}
-	}
-
 	pm_runtime_enable(dev);
 	pm_runtime_get_sync(dev);
 
@@ -261,11 +227,9 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 		goto err_pm_runtime;
 	}
 

[try#1 PATCH 3/7] clk: add default device asset handlers

[Andy Green]

This adds default device_asset handlers for struct clk.  If you
want an associated clk asset of a device to be optionally set, and
enabled  before probe, and disabled after removal, these callbacks
will take care of everything including get and put.

By defining them here in regulator core, code duplication at the usages
is nipped in the bud.

Signed-off-by: Andy Green <andy.green@linaro.org>
---
 drivers/clk/clkdev.c |   39 +++++++++++++++++++++++++++++++++++++++
 include/linux/clk.h  |   33 ++++++++++++++++++++++++++++++++-
 2 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 442a313..01d3fcd 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -327,3 +327,42 @@ int clk_register_clkdevs(struct clk *clk, struct clk_lookup *cl, size_t num)
 	return 0;
 }
 EXPORT_SYMBOL(clk_register_clkdevs);
+
+/*
+ * Default handlers for clock asset preprobe and postremoval
+ */
+int clk_asset_default_preprobe(struct device *device,
+						struct device_asset *asset)
+{
+	struct clk **clk = (struct clk **)&asset->asset;
+	int n;
+
+	*clk = clk_get(device, asset->name);
+	if (IS_ERR(*clk))
+		return PTR_ERR(*clk);
+	if (asset->data) {
+		n = clk_set_rate(*clk, (unsigned long)asset->data);
+		if (n < 0)
+			goto bail;
+	}
+	n = clk_prepare_enable(*clk);
+	if (n < 0)
+		goto bail;
+
+	return 0;
+bail:
+	clk_put(*clk);
+
+	return n;
+}
+EXPORT_SYMBOL_GPL(clk_asset_default_preprobe);
+
+void clk_asset_default_postremove(struct device *device,
+						struct device_asset *asset)
+{
+	struct clk **clk = (struct clk **)&asset->asset;
+
+	clk_disable_unprepare(*clk);
+	clk_put(*clk);
+}
+EXPORT_SYMBOL_GPL(clk_asset_default_postremove);
diff --git a/include/linux/clk.h b/include/linux/clk.h
index b3ac22d..3956675 100644
--- a/include/linux/clk.h
+++ b/include/linux/clk.h
@@ -17,6 +17,7 @@
 #include <linux/notifier.h>
 
 struct device;
+struct device_asset;
 
 struct clk;
 
@@ -276,6 +277,27 @@ struct clk *clk_get_parent(struct clk *clk);
  */
 struct clk *clk_get_sys(const char *dev_id, const char *con_id);
 
+/**
+ * clk_asset_default_preprobe: default probe handler for clock device assets
+ * @device:	the device whose assets we are enabling just before probing it
+ * @asset: the clock asset we are going to get and enable
+ *
+ * You can give this as the handler for .pre_probe callback in device_asset to
+ * deal with pre-enabling/get of a device's clock assets
+ */
+int clk_asset_default_preprobe(struct device *device,
+						struct device_asset *asset);
+/**
+ * clk_asset_default_postremove: default remove handler for clock device assets
+ * @device:	the device whose assets we are disabling just after removing it
+ * @asset: the clock asset we are going to disable and put
+ *
+ * You can give this as the handler for .post_remove callback in device_asset
+ * to deal with post-disabling/put of a device's clock assets
+ */
+void clk_asset_default_postremove(struct device *device,
+						struct device_asset *asset);
+
 #else /* !CONFIG_HAVE_CLK */
 
 static inline struct clk *clk_get(struct device *dev, const char *id)
@@ -323,7 +345,16 @@ static inline struct clk *clk_get_parent(struct clk *clk)
 {
 	return NULL;
 }
-
+static inline int clk_asset_default_preprobe(struct device *device,
+						    struct device_asset *asset)
+{
+	return 0;
+}
+static inline void clk_asset_default_postremove(struct device *device,
+						    struct device_asset *asset)
+{
+}
+s
 #endif
 
 /* clk_prepare_enable helps cases using clk_enable in non-atomic context. */

[try#1 PATCH 5/7] omap4: panda: add smsc95xx regulator and reset dependent on root hub

[Andy Green]

This adds regulators which are controlled by the OMAP4 PandaBoard (ES)'s
EHCI logical root hub existing.

The regulators are made a device_asset of the EHCI logical root hub by
passing them through the hsusb -> mfd path.  Although the ehci-related
platform_device is created at boot time, it is not instantiated until the
ehci-hcd module is inserted if it is modular.

Since the regulator is an asset of the ehci-omap.0 device, it's turned on
during successful probe and off when the device is removed.

Without power control, the ULPI PHY + SMSC9614 on the Panda eats 700-900mW
all the time, which is around the same as the idle power of the SoC and
rest of the board.

This allows us to start off with it depowered, and only power it if the
ehci-hcd module is inserted.  When the module is removed, it's depowered
again.

Signed-off-by: Andy Green <andy.green@linaro.org>
---
 arch/arm/mach-omap2/board-omap4panda.c |  100 ++++++++++++++++++++++++++------
 arch/arm/mach-omap2/usb-host.c         |    1 
 arch/arm/plat-omap/include/plat/usb.h  |    2 +
 drivers/mfd/omap-usb-host.c            |    9 ++-
 4 files changed, 89 insertions(+), 23 deletions(-)

diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c
index bfcd397..52add03 100644
--- a/arch/arm/mach-omap2/board-omap4panda.c
+++ b/arch/arm/mach-omap2/board-omap4panda.c
@@ -144,6 +144,15 @@ static struct platform_device *panda_devices[] __initdata = {
 	&btwilink_device,
 };
 
+static struct device_asset assets_ehci_omap0[] = {
+	{
+		.name = "reg-panda-smsc95xx",
+		.pre_probe = regulator_asset_default_preprobe,
+		.post_remove = regulator_asset_default_postremove,
+	},
+	{ }
+};
+
 static const struct usbhs_omap_board_data usbhs_bdata __initconst = {
 	.port_mode[0] = OMAP_EHCI_PORT_MODE_PHY,
 	.port_mode[1] = OMAP_USBHS_PORT_MODE_UNUSED,
@@ -151,12 +160,8 @@ static const struct usbhs_omap_board_data usbhs_bdata __initconst = {
 	.phy_reset  = false,
 	.reset_gpio_port[0]  = -EINVAL,
 	.reset_gpio_port[1]  = -EINVAL,
-	.reset_gpio_port[2]  = -EINVAL
-};
-
-static struct gpio panda_ehci_gpios[] __initdata = {
-	{ GPIO_HUB_POWER,	GPIOF_OUT_INIT_LOW,  "hub_power"  },
-	{ GPIO_HUB_NRESET,	GPIOF_OUT_INIT_LOW,  "hub_nreset" },
+	.reset_gpio_port[2]  = -EINVAL,
+	.assets = assets_ehci_omap0,
 };
 
 static void __init omap4_ehci_init(void)
@@ -173,23 +178,76 @@ static void __init omap4_ehci_init(void)
 	clk_set_rate(phy_ref_clk, 19200000);
 	clk_prepare_enable(phy_ref_clk);
 
-	/* disable the power to the usb hub prior to init and reset phy+hub */
-	ret = gpio_request_array(panda_ehci_gpios,
-				 ARRAY_SIZE(panda_ehci_gpios));
-	if (ret) {
-		pr_err("Unable to initialize EHCI power/reset\n");
-		return;
-	}
+	usbhs_init(&usbhs_bdata);
+}
 
-	gpio_export(GPIO_HUB_POWER, 0);
-	gpio_export(GPIO_HUB_NRESET, 0);
-	gpio_set_value(GPIO_HUB_NRESET, 1);
+/*
+ * hub_nreset also resets the ULPI PHY and is required after powering SMSC chip
+ *	ULPI PHY is always powered... need to do reset once for both once
+ * hub_power enables a 3.3V regulator for (hub + eth) chip
+ *	however there's no point having ULPI PHY in use alone
+ *	since it's only connected to the (hub + eth) chip
+ */
 
-	usbhs_init(&usbhs_bdata);
+static struct regulator_init_data panda_hub = {
+	.constraints = {
+		.name = "vhub",
+		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
+	},
+};
 
-	/* enable power to hub */
-	gpio_set_value(GPIO_HUB_POWER, 1);
-}
+static struct fixed_voltage_config panda_vhub = {
+	.supply_name = "vhub",
+	.microvolts = 3300000,
+	.gpio = GPIO_HUB_POWER,
+	.startup_delay = 70000, /* 70msec */
+	.enable_high = 1,
+	.enabled_at_boot = 0,
+	.init_data = &panda_hub,
+};
+
+static struct platform_device omap_vhub_device = {
+	.name		= "reg-fixed-voltage",
+	.id		= 2,
+	.dev = {
+		.platform_data = &panda_vhub,
+	},
+};
+
+static struct regulator_init_data panda_ulpireset = {
+	/*
+	 * idea is that when operating ulpireset, regulator api will make
+	 * sure that the hub+eth chip is powered, since it's the "parent"
+	 */
+	.supply_regulator = "vhub", /* we are a child of vhub */
+	.constraints = {
+		/*
+		 * this magic string associates us with ehci-omap.0 root hub
+		 * when the root hub logical device is up, we will power
+		 * and reset [ ULPI PHY + [ HUB + ETH ] ]
+		 */
+		.name = "reg-panda-smsc95xx",
+		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
+	},
+};
+
+static struct fixed_voltage_config panda_vulpireset = {
+	.supply_name = "reg-panda-smsc95xx",
+	.microvolts = 3300000,
+	.gpio = GPIO_HUB_NRESET,
+	.startup_delay = 70000, /* 70msec */
+	.enable_high = 1,
+	.enabled_at_boot = 0,
+	.init_data = &panda_ulpireset,
+};
+
+static struct platform_device omap_vulpireset_device = {
+	.name		= "reg-fixed-voltage",
+	.id		= 3,
+	.dev = {
+		.platform_data = &panda_vulpireset,
+	},
+};
 
 static struct omap_musb_board_data musb_board_data = {
 	.interface_type		= MUSB_INTERFACE_UTMI,
@@ -504,6 +562,8 @@ static void __init omap4_panda_init(void)
 	omap4_panda_i2c_init();
 	platform_add_devices(panda_devices, ARRAY_SIZE(panda_devices));
 	platform_device_register(&omap_vwlan_device);
+	platform_device_register(&omap_vhub_device);
+	platform_device_register(&omap_vulpireset_device);
 	omap_serial_init();
 	omap_sdrc_init(NULL, NULL);
 	omap4_twl6030_hsmmc_init(mmc);
diff --git a/arch/arm/mach-omap2/usb-host.c b/arch/arm/mach-omap2/usb-host.c
index 98f3287..2a0fdf9 100644
--- a/arch/arm/mach-omap2/usb-host.c
+++ b/arch/arm/mach-omap2/usb-host.c
@@ -501,6 +501,7 @@ void __init usbhs_init(const struct usbhs_omap_board_data *pdata)
 	}
 	ehci_data.phy_reset = pdata->phy_reset;
 	ohci_data.es2_compatibility = pdata->es2_compatibility;
+	ehci_data.assets = pdata->assets;
 	usbhs_data.ehci_data = &ehci_data;
 	usbhs_data.ohci_data = &ohci_data;
 
diff --git a/arch/arm/plat-omap/include/plat/usb.h b/arch/arm/plat-omap/include/plat/usb.h
index 70acdee..6f6235e 100644
--- a/arch/arm/plat-omap/include/plat/usb.h
+++ b/arch/arm/plat-omap/include/plat/usb.h
@@ -36,6 +36,7 @@ struct usbhs_omap_board_data {
 	unsigned			es2_compatibility:1;
 
 	unsigned			phy_reset:1;
+	struct device_asset		*assets;
 };
 
 #ifdef CONFIG_ARCH_OMAP2PLUS
@@ -44,6 +45,7 @@ struct ehci_hcd_omap_platform_data {
 	enum usbhs_omap_port_mode	port_mode[OMAP3_HS_USB_PORTS];
 	int				reset_gpio_port[OMAP3_HS_USB_PORTS];
 	unsigned			phy_reset:1;
+	struct device_asset		*assets;
 };
 
 struct ohci_hcd_omap_platform_data {
diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 23cec57..6d57c9d 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -25,6 +25,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/spinlock.h>
 #include <linux/gpio.h>
+#include <linux/device.h>
 #include <plat/cpu.h>
 #include <plat/usb.h>
 #include <linux/pm_runtime.h>
@@ -136,7 +137,8 @@ static inline u8 usbhs_readb(void __iomem *base, u8 reg)
 
 static struct platform_device *omap_usbhs_alloc_child(const char *name,
 			struct resource	*res, int num_resources, void *pdata,
-			size_t pdata_size, struct device *dev)
+			size_t pdata_size, struct device *dev,
+			struct device_asset *assets)
 {
 	struct platform_device	*child;
 	int			ret;
@@ -160,6 +162,7 @@ static struct platform_device *omap_usbhs_alloc_child(const char *name,
 		goto err_alloc;
 	}
 
+	child->dev.assets = assets;
 	child->dev.dma_mask		= &usbhs_dmamask;
 	dma_set_coherent_mask(&child->dev, DMA_BIT_MASK(32));
 	child->dev.parent		= dev;
@@ -212,7 +215,7 @@ static int omap_usbhs_alloc_children(struct platform_device *pdev)
 	resources[1] = *res;
 
 	ehci = omap_usbhs_alloc_child(OMAP_EHCI_DEVICE, resources, 2, ehci_data,
-		sizeof(*ehci_data), dev);
+		sizeof(*ehci_data), dev, ehci_data->assets);
 
 	if (!ehci) {
 		dev_err(dev, "omap_usbhs_alloc_child failed\n");
@@ -237,7 +240,7 @@ static int omap_usbhs_alloc_children(struct platform_device *pdev)
 	resources[1] = *res;
 
 	ohci = omap_usbhs_alloc_child(OMAP_OHCI_DEVICE, resources, 2, ohci_data,
-		sizeof(*ohci_data), dev);
+		sizeof(*ohci_data), dev, NULL);
 	if (!ohci) {
 		dev_err(dev, "omap_usbhs_alloc_child failed\n");
 		ret = -ENOMEM;

Re: [try#1 PATCH 5/7] omap4: panda: add smsc95xx regulator and reset dependent on root hub

[Roger Quadros]

On 11/28/2012 02:59 PM, Andy Green wrote:
> This adds regulators which are controlled by the OMAP4 PandaBoard (ES)'s
> EHCI logical root hub existing.
> 
> The regulators are made a device_asset of the EHCI logical root hub by
> passing them through the hsusb -> mfd path.  Although the ehci-related
> platform_device is created at boot time, it is not instantiated until the
> ehci-hcd module is inserted if it is modular.
> 
> Since the regulator is an asset of the ehci-omap.0 device, it's turned on
> during successful probe and off when the device is removed.
> 
> Without power control, the ULPI PHY + SMSC9614 on the Panda eats 700-900mW
> all the time, which is around the same as the idle power of the SoC and
> rest of the board.
> 
> This allows us to start off with it depowered, and only power it if the
> ehci-hcd module is inserted.  When the module is removed, it's depowered
> again.
> 
> Signed-off-by: Andy Green <andy.green-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  arch/arm/mach-omap2/board-omap4panda.c |  100 ++++++++++++++++++++++++++------
>  arch/arm/mach-omap2/usb-host.c         |    1 
>  arch/arm/plat-omap/include/plat/usb.h  |    2 +
>  drivers/mfd/omap-usb-host.c            |    9 ++-
>  4 files changed, 89 insertions(+), 23 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c
> index bfcd397..52add03 100644
> --- a/arch/arm/mach-omap2/board-omap4panda.c
> +++ b/arch/arm/mach-omap2/board-omap4panda.c
> @@ -144,6 +144,15 @@ static struct platform_device *panda_devices[] __initdata = {
>  	&btwilink_device,
>  };
>  
> +static struct device_asset assets_ehci_omap0[] = {
> +	{
> +		.name = "reg-panda-smsc95xx",
> +		.pre_probe = regulator_asset_default_preprobe,
> +		.post_remove = regulator_asset_default_postremove,
> +	},
> +	{ }
> +};
> +
>  static const struct usbhs_omap_board_data usbhs_bdata __initconst = {
>  	.port_mode[0] = OMAP_EHCI_PORT_MODE_PHY,
>  	.port_mode[1] = OMAP_USBHS_PORT_MODE_UNUSED,
> @@ -151,12 +160,8 @@ static const struct usbhs_omap_board_data usbhs_bdata __initconst = {
>  	.phy_reset  = false,
>  	.reset_gpio_port[0]  = -EINVAL,
>  	.reset_gpio_port[1]  = -EINVAL,
> -	.reset_gpio_port[2]  = -EINVAL
> -};
> -
> -static struct gpio panda_ehci_gpios[] __initdata = {
> -	{ GPIO_HUB_POWER,	GPIOF_OUT_INIT_LOW,  "hub_power"  },
> -	{ GPIO_HUB_NRESET,	GPIOF_OUT_INIT_LOW,  "hub_nreset" },
> +	.reset_gpio_port[2]  = -EINVAL,
> +	.assets = assets_ehci_omap0,
>  };
>  
>  static void __init omap4_ehci_init(void)
> @@ -173,23 +178,76 @@ static void __init omap4_ehci_init(void)
>  	clk_set_rate(phy_ref_clk, 19200000);
>  	clk_prepare_enable(phy_ref_clk);
>  
> -	/* disable the power to the usb hub prior to init and reset phy+hub */
> -	ret = gpio_request_array(panda_ehci_gpios,
> -				 ARRAY_SIZE(panda_ehci_gpios));
> -	if (ret) {
> -		pr_err("Unable to initialize EHCI power/reset\n");
> -		return;
> -	}
> +	usbhs_init(&usbhs_bdata);
> +}
>  
> -	gpio_export(GPIO_HUB_POWER, 0);
> -	gpio_export(GPIO_HUB_NRESET, 0);
> -	gpio_set_value(GPIO_HUB_NRESET, 1);
> +/*
> + * hub_nreset also resets the ULPI PHY and is required after powering SMSC chip
> + *	ULPI PHY is always powered... need to do reset once for both once
> + * hub_power enables a 3.3V regulator for (hub + eth) chip
> + *	however there's no point having ULPI PHY in use alone
> + *	since it's only connected to the (hub + eth) chip
> + */
>  
> -	usbhs_init(&usbhs_bdata);
> +static struct regulator_init_data panda_hub = {
> +	.constraints = {
> +		.name = "vhub",
> +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
> +	},
> +};
>  
> -	/* enable power to hub */
> -	gpio_set_value(GPIO_HUB_POWER, 1);
> -}
> +static struct fixed_voltage_config panda_vhub = {
> +	.supply_name = "vhub",
> +	.microvolts = 3300000,
> +	.gpio = GPIO_HUB_POWER,
> +	.startup_delay = 70000, /* 70msec */
> +	.enable_high = 1,
> +	.enabled_at_boot = 0,
> +	.init_data = &panda_hub,
> +};
> +
> +static struct platform_device omap_vhub_device = {
> +	.name		= "reg-fixed-voltage",
> +	.id		= 2,
> +	.dev = {
> +		.platform_data = &panda_vhub,
> +	},
> +};
> +
> +static struct regulator_init_data panda_ulpireset = {
> +	/*
> +	 * idea is that when operating ulpireset, regulator api will make
> +	 * sure that the hub+eth chip is powered, since it's the "parent"
> +	 */
> +	.supply_regulator = "vhub", /* we are a child of vhub */
> +	.constraints = {
> +		/*
> +		 * this magic string associates us with ehci-omap.0 root hub
> +		 * when the root hub logical device is up, we will power
> +		 * and reset [ ULPI PHY + [ HUB + ETH ] ]
> +		 */
> +		.name = "reg-panda-smsc95xx",
> +		.valid_ops_mask = REGULATOR_CHANGE_STATUS,
> +	},
> +};
> +
> +static struct fixed_voltage_config panda_vulpireset = {
> +	.supply_name = "reg-panda-smsc95xx",
> +	.microvolts = 3300000,
> +	.gpio = GPIO_HUB_NRESET,
> +	.startup_delay = 70000, /* 70msec */
> +	.enable_high = 1,
> +	.enabled_at_boot = 0,
> +	.init_data = &panda_ulpireset,
> +};
> +
> +static struct platform_device omap_vulpireset_device = {
> +	.name		= "reg-fixed-voltage",
> +	.id		= 3,
> +	.dev = {
> +		.platform_data = &panda_vulpireset,
> +	},
> +};
>  
>  static struct omap_musb_board_data musb_board_data = {
>  	.interface_type		= MUSB_INTERFACE_UTMI,
> @@ -504,6 +562,8 @@ static void __init omap4_panda_init(void)
>  	omap4_panda_i2c_init();
>  	platform_add_devices(panda_devices, ARRAY_SIZE(panda_devices));
>  	platform_device_register(&omap_vwlan_device);
> +	platform_device_register(&omap_vhub_device);
> +	platform_device_register(&omap_vulpireset_device);
>  	omap_serial_init();
>  	omap_sdrc_init(NULL, NULL);
>  	omap4_twl6030_hsmmc_init(mmc);
> diff --git a/arch/arm/mach-omap2/usb-host.c b/arch/arm/mach-omap2/usb-host.c
> index 98f3287..2a0fdf9 100644
> --- a/arch/arm/mach-omap2/usb-host.c
> +++ b/arch/arm/mach-omap2/usb-host.c
> @@ -501,6 +501,7 @@ void __init usbhs_init(const struct usbhs_omap_board_data *pdata)
>  	}
>  	ehci_data.phy_reset = pdata->phy_reset;
>  	ohci_data.es2_compatibility = pdata->es2_compatibility;
> +	ehci_data.assets = pdata->assets;

Just wondering if it makes more sense to tie the regulator and clock
assets on the Panda to LAN95xx platform device instead of ehci_omap's
platform device.

The only thing we need to do is add a dummy platform device for the
LAN9xx chip and probe it in the smsc9xx driver.

The benefit of this is we can choose to power up/down the LAN9xx device
by insmod/rmmod smsc0xx driver and still have other OMAP USB ports
functional.

what do you think?

regards,
-roger

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

Re: [try#1 PATCH 5/7] omap4: panda: add smsc95xx regulator and reset dependent on root hub

[Andy Green]

On 11/28/2012 11:06 PM, the mail apparently from Roger Quadros included:

Hi Roger -

> On 11/28/2012 02:59 PM, Andy Green wrote:
>> This adds regulators which are controlled by the OMAP4 PandaBoard (ES)'s
>> EHCI logical root hub existing.
>>
>> The regulators are made a device_asset of the EHCI logical root hub by
>> passing them through the hsusb -> mfd path.  Although the ehci-related
>> platform_device is created at boot time, it is not instantiated until the
>> ehci-hcd module is inserted if it is modular.
>>
>> Since the regulator is an asset of the ehci-omap.0 device, it's turned on
>> during successful probe and off when the device is removed.
>>
>> Without power control, the ULPI PHY + SMSC9614 on the Panda eats 700-900mW
>> all the time, which is around the same as the idle power of the SoC and
>> rest of the board.
>>
>> This allows us to start off with it depowered, and only power it if the
>> ehci-hcd module is inserted.  When the module is removed, it's depowered
>> again.

...

>> diff --git a/arch/arm/mach-omap2/usb-host.c b/arch/arm/mach-omap2/usb-host.c
>> index 98f3287..2a0fdf9 100644
>> --- a/arch/arm/mach-omap2/usb-host.c
>> +++ b/arch/arm/mach-omap2/usb-host.c
>> @@ -501,6 +501,7 @@ void __init usbhs_init(const struct usbhs_omap_board_data *pdata)
>>   	}
>>   	ehci_data.phy_reset = pdata->phy_reset;
>>   	ohci_data.es2_compatibility = pdata->es2_compatibility;
>> +	ehci_data.assets = pdata->assets;
>
> Just wondering if it makes more sense to tie the regulator and clock
> assets on the Panda to LAN95xx platform device instead of ehci_omap's
> platform device.
>
> The only thing we need to do is add a dummy platform device for the
> LAN9xx chip and probe it in the smsc9xx driver.
>
> The benefit of this is we can choose to power up/down the LAN9xx device
> by insmod/rmmod smsc0xx driver and still have other OMAP USB ports
> functional.
>
> what do you think?

I think it's cool but I am not sure it hangs together.  Just to make 
sure we're on the same page, the "LAN95XX platform device" doesn't exist 
at the moment, right?

With hsusb mfd -> ehci the platform device can be made from boot because 
the memory-mapped hardware is always there.  As soon as the driver 
appears, it can be probed and made into a real live device and 
everything is straight.

But when the platform_device would represent a usb device, that's not 
quite the same.  AIUI the usb stack only creates the device when it has 
probed a vid:pid and identified a driver that claims to serve it.

If the power for the HUB+ETH was controlled by an asset on the probe for 
the HUB+ETH device, isn't that never going to happen?  The usb stack 
can't see the vid+pid for smsc95xx device until we give it power (it's 
connected but off), in this scheme we don't give it power until 
something ran probe for an smsc95xx device?

There's another quirk on Panda that makes trouble too, after enabling 
power on the HUB+ETH chip, we must reset it.  But the same reset signal 
resets the ULPI PHY too.  So if the powering deadlock was solved we 
would still run into a problem where we caused ULPI errors bringing up 
the smsc95xx, if ehci+ULPI was already going.  It's a violation of the 
independence of the ULPI and [usb device on other side of ulpi] caused 
by the Panda design using the same reset signal. That is why the current 
approach gets power + reset over with once at the time we would anyway 
want to reset the ULPI PHY.

However I think what you're saying about binding to hub power is good. 
The hub ports are not devices, but it would be possible to bind an asset 
array to them and make the pre- and post- code functions.

But AFAIK neither that, nor the platform_device idea for smsc95xx even 
get off the ground without a device_path type addressing scheme, because 
you are targeting from the board file specific logical devices that only 
exist later after other probes.

I think it will be possible to address objections around the "pathiness" 
by being able to seed the path match with a platform_device pointer 
(there exists in the board file time a platform_device for ehci-omap.0 
...) and just matching the remainder on a single usb device's name, like 
"*-1.1-1".

-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-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [try#1 PATCH 5/7] omap4: panda: add smsc95xx regulator and reset dependent on root hub

[Alan Stern]

On Thu, 29 Nov 2012, Andy Green wrote:

> However I think what you're saying about binding to hub power is good. 
> The hub ports are not devices, but it would be possible to bind an asset 
> array to them and make the pre- and post- code functions.

In the 3.6 kernel, hub ports are not devices.  In 3.7 they are -- that 
is, each hub port has its own struct device.

> I think it will be possible to address objections around the "pathiness" 
> by being able to seed the path match with a platform_device pointer 
> (there exists in the board file time a platform_device for ehci-omap.0 
> ...) and just matching the remainder on a single usb device's name, like 
> "*-1.1-1".

Can you think of a way to do this without checking for a match every 
time a new device is registered?  For instance, in this case it would 
be preferable to do this match only for descendants of ehci-omap.0.  To 
match the port device, the string would have to be something like 
"*-0:1.0/port2".

In fact, if the match were anchored at the end of the string, we 
wouldn't need the wildcard at all -- at least, not in this case.  The 
string could simply be "-0:1.0/port2".  But that's only if the match is 
restricted to devices below ehci-omap.0.

Alan Stern

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

Re: [try#1 PATCH 5/7] omap4: panda: add smsc95xx regulator and reset dependent on root hub

[Andy Green]

On 11/30/2012 01:57 AM, the mail apparently from Alan Stern included:
> On Thu, 29 Nov 2012, Andy Green wrote:
>
>> However I think what you're saying about binding to hub power is good.
>> The hub ports are not devices, but it would be possible to bind an asset
>> array to them and make the pre- and post- code functions.
>
> In the 3.6 kernel, hub ports are not devices.  In 3.7 they are -- that
> is, each hub port has its own struct device.

Right, I should have seen this in hub.c before.  It's much better like that.

>> I think it will be possible to address objections around the "pathiness"
>> by being able to seed the path match with a platform_device pointer
>> (there exists in the board file time a platform_device for ehci-omap.0
>> ...) and just matching the remainder on a single usb device's name, like
>> "*-1.1-1".
>
> Can you think of a way to do this without checking for a match every
> time a new device is registered?  For instance, in this case it would
> be preferable to do this match only for descendants of ehci-omap.0.  To
> match the port device, the string would have to be something like
> "*-0:1.0/port2".

Yes.  How about adding a third callback to struct device_asset, along 
the lines of

	int (*pre_child_register)(struct device *child);

then, in register_device() we add code that before we get down to it, we 
walk up the new device's parents calling ->pre_child_register() on any 
assets the parents may have.  In the typical case that's a rapid NOP 
once per device registration.

However... if we had arranged back at boot time that the ehci-omap.0 
struct device had an asset with only pre_device_register callback set, 
we can use that asset's .name for the right-justified child device name 
we are looking for like "-0:1.0/port2", and its .asset member to point 
to another asset table the pre_child_register callback will attach to 
the child device if the name matches.  So in the board file:

	struct device_asset ehci_omap0_smsc_hub_port_assets[] = {
		/* the smsc regulator and clock assets destined for the hub port */
		{ }
	};

	/* below is attached to ehci-omap.0 like in try#1 */

	struct device_asset ehci_omap0_assets[] = {
		{
			.name = "-0:1.0/port2",
			.asset = ehci_omap0_smsc_hub_port_assets,
			.pre_child_register = device_asset_attach_to_child,
		},
		{ }
	};

In that way we can project as many stashed asset tables on to 
dynamically probed devices as we like from the platform_devices at boot 
time.  Only children of the right platform devices do any checking or 
processing.

> In fact, if the match were anchored at the end of the string, we
> wouldn't need the wildcard at all -- at least, not in this case.  The
> string could simply be "-0:1.0/port2".  But that's only if the match is
> restricted to devices below ehci-omap.0.

It's a good idea, it won't get fooled by a hub getting plugged there 
which has its own port2 either, the -0:1.0 bit will have been elaborated 
for the subsequent hub "path" and won't match.


It may be neater to split out the device_asset callbacks to an ..._ops 
struct.

	struct device_asset_ops {
		int (*pre_probe)(struct device *device, struct device_asset *asset);
		void (*post_remove)(struct device *device, struct device_asset *asset);
		int (*pre_child_register)(struct device *child);
	};

	struct device_asset {
		...
		struct device_asset_ops *ops;
	};

that also lets us export and set one thing to select say regulator 
"handler", instead of n callbacks that must match.


Something else I think mux would be a great target for device_asset 
support.  That way it could become normal for mux function to get set as 
part of the specific device instantiation, so if you know you have an 
8-bit ULPI PHY that will be logically created by the platform_device, 
you can attach ULPI-mode mux config appropriate for your exact SoC as an 
asset to the platform_device.

When the device is destroyed, balls can go back to safe mode and save 
power, and if the balls are muxed with other things again the right mux 
asset can be associated with that so it switches automagically according 
to what you are doing.  That's a lot better than forcing them once at 
boot which is the current method.  Are there any gotchas with that?

-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-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [try#1 PATCH 5/7] omap4: panda: add smsc95xx regulator and reset dependent on root hub

["Andy Green (林安廸)"]

On 11/30/2012 04:58 AM, the mail apparently from Andy Green included:
> On 11/30/2012 01:57 AM, the mail apparently from Alan Stern included:
>> On Thu, 29 Nov 2012, Andy Green wrote:
>>
>>> However I think what you're saying about binding to hub power is good.
>>> The hub ports are not devices, but it would be possible to bind an asset
>>> array to them and make the pre- and post- code functions.
>>
>> In the 3.6 kernel, hub ports are not devices.  In 3.7 they are -- that
>> is, each hub port has its own struct device.
>
> Right, I should have seen this in hub.c before.  It's much better like
> that.
>
>>> I think it will be possible to address objections around the "pathiness"
>>> by being able to seed the path match with a platform_device pointer
>>> (there exists in the board file time a platform_device for ehci-omap.0
>>> ...) and just matching the remainder on a single usb device's name, like
>>> "*-1.1-1".
>>
>> Can you think of a way to do this without checking for a match every
>> time a new device is registered?  For instance, in this case it would
>> be preferable to do this match only for descendants of ehci-omap.0.  To
>> match the port device, the string would have to be something like
>> "*-0:1.0/port2".
>
> Yes.  How about adding a third callback to struct device_asset, along
> the lines of
>
>      int (*pre_child_register)(struct device *child);
>
> then, in register_device() we add code that before we get down to it, we
> walk up the new device's parents calling ->pre_child_register() on any
> assets the parents may have.  In the typical case that's a rapid NOP
> once per device registration.
>
> However... if we had arranged back at boot time that the ehci-omap.0
> struct device had an asset with only pre_device_register callback set,
> we can use that asset's .name for the right-justified child device name
> we are looking for like "-0:1.0/port2", and its .asset member to point
> to another asset table the pre_child_register callback will attach to
> the child device if the name matches.  So in the board file:
>
>      struct device_asset ehci_omap0_smsc_hub_port_assets[] = {
>          /* the smsc regulator and clock assets destined for the hub
> port */
>          { }
>      };
>
>      /* below is attached to ehci-omap.0 like in try#1 */
>
>      struct device_asset ehci_omap0_assets[] = {
>          {
>              .name = "-0:1.0/port2",
>              .asset = ehci_omap0_smsc_hub_port_assets,
>              .pre_child_register = device_asset_attach_to_child,
>          },
>          { }
>      };
>
> In that way we can project as many stashed asset tables on to
> dynamically probed devices as we like from the platform_devices at boot
> time.  Only children of the right platform devices do any checking or
> processing.
>
>> In fact, if the match were anchored at the end of the string, we
>> wouldn't need the wildcard at all -- at least, not in this case.  The
>> string could simply be "-0:1.0/port2".  But that's only if the match is
>> restricted to devices below ehci-omap.0.
>
> It's a good idea, it won't get fooled by a hub getting plugged there
> which has its own port2 either, the -0:1.0 bit will have been elaborated
> for the subsequent hub "path" and won't match.
>
>
> It may be neater to split out the device_asset callbacks to an ..._ops
> struct.
>
>      struct device_asset_ops {
>          int (*pre_probe)(struct device *device, struct device_asset
> *asset);
>          void (*post_remove)(struct device *device, struct device_asset
> *asset);
>          int (*pre_child_register)(struct device *child);
>      };
>
>      struct device_asset {
>          ...
>          struct device_asset_ops *ops;
>      };
>
> that also lets us export and set one thing to select say regulator
> "handler", instead of n callbacks that must match.

I have everything discussed above ready for a try#2, including the 
descendant matching stuff in separate patches.  The code got a lot 
smaller and better with the _ops struct.

The new code can attach the assets to the targeted hub port as discussed 
(using only "-0:1.0/port1" and only checking ehci-omap.0 descendants at 
device_add() time), but the hub port device never probes, unless I 
missed the point it's because it actually never binds to a driver, it's 
a very unusual standalone logical device.

If that's the case I could work around that by doing the probe() asset 
stuff at device_add() time if there's no driver name on the device, but 
actually I am not sure that's where we wanted to end up.

Now we got so far as to succeed to associate regulator + clock assets to 
the logical hub port device, isn't it that we want the assets to be 
enabled and disabled according to hub port power state?  In that case it 
needs to go in the direction of calling device_asset helpers in the 
hub.c code that handles enable and disable hub port power.  Is this 
sounding like the right way, or something else?

-Andy

> Something else I think mux would be a great target for device_asset
> support.  That way it could become normal for mux function to get set as
> part of the specific device instantiation, so if you know you have an
> 8-bit ULPI PHY that will be logically created by the platform_device,
> you can attach ULPI-mode mux config appropriate for your exact SoC as an
> asset to the platform_device.
>
> When the device is destroyed, balls can go back to safe mode and save
> power, and if the balls are muxed with other things again the right mux
> asset can be associated with that so it switches automagically according
> to what you are doing.  That's a lot better than forcing them once at
> boot which is the current method.  Are there any gotchas with that?
>
> -Andy
>

Re: [try#1 PATCH 5/7] omap4: panda: add smsc95xx regulator and reset dependent on root hub

[Alan Stern]

On Fri, 30 Nov 2012, "Andy Green (林安廸)" wrote:

> I have everything discussed above ready for a try#2, including the 
> descendant matching stuff in separate patches.  The code got a lot 
> smaller and better with the _ops struct.
> 
> The new code can attach the assets to the targeted hub port as discussed 
> (using only "-0:1.0/port1" and only checking ehci-omap.0 descendants at 
> device_add() time), but the hub port device never probes, unless I 
> missed the point it's because it actually never binds to a driver, it's 
> a very unusual standalone logical device.

That's right; it gets registered but it never binds.  It doesn't need a 
driver because all the port-related activity is done by the hub driver.  

> If that's the case I could work around that by doing the probe() asset 
> stuff at device_add() time if there's no driver name on the device, but 
> actually I am not sure that's where we wanted to end up.
> 
> Now we got so far as to succeed to associate regulator + clock assets to 
> the logical hub port device, isn't it that we want the assets to be 
> enabled and disabled according to hub port power state?  In that case it 
> needs to go in the direction of calling device_asset helpers in the 
> hub.c code that handles enable and disable hub port power.  Is this 
> sounding like the right way, or something else?

I'm not so sure about this.  Maybe we're trying to solve too many
different kinds of problem at once.

The original device asset scheme is fine if all you want to do is turn
on the regulator when ehci-omap binds and turn it off when the driver
unbinds.  But if we want to go farther, maybe the device asset approach
isn't the right way.

In particular, we would like the regulator to get turned off and on by 
the port driver as part of its suspend/resume handling.  The most 
straightforward way to do this is to modify the PM code in port.c.

This would be specific to USB ports.  I toyed around with the idea of a
more general-purpose approach using "PM assets" that could hook into
various PM callbacks, but it seemed to get too complicated.  And
besides, it only ended up looking like a way to put something into a PM
domain without the driver's knowledge.

So maybe something like Ming Lei's suggestion really would be best.  
We could add code specifically for regulators and clocks to port.c.

Trying to write something that can work for all types of devices, not 
just USB ports, may be over-reaching.

Alan Stern

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

[try#1 PATCH 6/7] omap4 panda add smsc95xx clock dependent on root hub

[Andy Green]

This patch makes the ULPI PHY clock control also be a device_asset
of the ehci-omap.0 device, along with the [HUB + ETH] smsc95xx chip
power regulator.

Without clock control, the PHY clock is running all the time from
boot whether USB is in use or not, and during suspend distorting
power measurements there.

Signed-off-by: Andy Green <andy.green@linaro.org>
---
 arch/arm/mach-omap2/board-omap4panda.c |   25 +++++++------------------
 1 file changed, 7 insertions(+), 18 deletions(-)

diff --git a/arch/arm/mach-omap2/board-omap4panda.c b/arch/arm/mach-omap2/board-omap4panda.c
index 52add03..97489c7 100644
--- a/arch/arm/mach-omap2/board-omap4panda.c
+++ b/arch/arm/mach-omap2/board-omap4panda.c
@@ -150,6 +150,12 @@ static struct device_asset assets_ehci_omap0[] = {
 		.pre_probe = regulator_asset_default_preprobe,
 		.post_remove = regulator_asset_default_postremove,
 	},
+	{
+		.name = "auxclk3_ck",
+		.data = (void *)19200000,
+		.pre_probe = clk_asset_default_preprobe,
+		.post_remove = clk_asset_default_postremove,
+	},
 	{ }
 };
 
@@ -164,23 +170,6 @@ static const struct usbhs_omap_board_data usbhs_bdata __initconst = {
 	.assets = assets_ehci_omap0,
 };
 
-static void __init omap4_ehci_init(void)
-{
-	int ret;
-	struct clk *phy_ref_clk;
-
-	/* FREF_CLK3 provides the 19.2 MHz reference clock to the PHY */
-	phy_ref_clk = clk_get(NULL, "auxclk3_ck");
-	if (IS_ERR(phy_ref_clk)) {
-		pr_err("Cannot request auxclk3\n");
-		return;
-	}
-	clk_set_rate(phy_ref_clk, 19200000);
-	clk_prepare_enable(phy_ref_clk);
-
-	usbhs_init(&usbhs_bdata);
-}
-
 /*
  * hub_nreset also resets the ULPI PHY and is required after powering SMSC chip
  *	ULPI PHY is always powered... need to do reset once for both once
@@ -567,7 +556,7 @@ static void __init omap4_panda_init(void)
 	omap_serial_init();
 	omap_sdrc_init(NULL, NULL);
 	omap4_twl6030_hsmmc_init(mmc);
-	omap4_ehci_init();
+	usbhs_init(&usbhs_bdata);
 	usb_musb_init(&musb_board_data);
 	omap4_panda_display_init();
 }

[try#1 PATCH 7/7] config omap2plus add ehci bits

[Andy Green]

omap2plus seems to have rotted a bit, this is the delta that appears
if we enable modular build for ehci-hcd and smsc95xx.

Signed-off-by: Andy Green <andy.green@linaro.org>
---
 arch/arm/configs/omap2plus_defconfig |   42 ++++++++++++++--------------------
 1 file changed, 17 insertions(+), 25 deletions(-)

diff --git a/arch/arm/configs/omap2plus_defconfig b/arch/arm/configs/omap2plus_defconfig
index 6230304..2f858a3 100644
--- a/arch/arm/configs/omap2plus_defconfig
+++ b/arch/arm/configs/omap2plus_defconfig
@@ -1,14 +1,14 @@
 CONFIG_EXPERIMENTAL=y
 CONFIG_SYSVIPC=y
 CONFIG_POSIX_MQUEUE=y
+CONFIG_NO_HZ=y
+CONFIG_HIGH_RES_TIMERS=y
 CONFIG_BSD_PROCESS_ACCT=y
 CONFIG_IKCONFIG=y
 CONFIG_IKCONFIG_PROC=y
 CONFIG_LOG_BUF_SHIFT=16
 CONFIG_BLK_DEV_INITRD=y
 CONFIG_EXPERT=y
-# CONFIG_SYSCTL_SYSCALL is not set
-CONFIG_KALLSYMS_EXTRA_PASS=y
 CONFIG_SLAB=y
 CONFIG_PROFILING=y
 CONFIG_OPROFILE=y
@@ -20,16 +20,15 @@ CONFIG_MODULE_FORCE_UNLOAD=y
 CONFIG_MODVERSIONS=y
 CONFIG_MODULE_SRCVERSION_ALL=y
 # CONFIG_BLK_DEV_BSG is not set
+CONFIG_PARTITION_ADVANCED=y
 CONFIG_ARCH_OMAP=y
 CONFIG_OMAP_RESET_CLOCKS=y
 CONFIG_OMAP_MUX_DEBUG=y
+CONFIG_SOC_OMAP5=y
 CONFIG_ARM_THUMBEE=y
 CONFIG_ARM_ERRATA_411920=y
-CONFIG_NO_HZ=y
-CONFIG_HIGH_RES_TIMERS=y
 CONFIG_SMP=y
 CONFIG_NR_CPUS=2
-CONFIG_LEDS=y
 CONFIG_ZBOOT_ROM_TEXT=0x0
 CONFIG_ZBOOT_ROM_BSS=0x0
 CONFIG_CMDLINE="root=/dev/mmcblk0p2 rootwait console=ttyO2,115200"
@@ -87,22 +86,21 @@ CONFIG_SCSI_MULTI_LUN=y
 CONFIG_SCSI_SCAN_ASYNC=y
 CONFIG_MD=y
 CONFIG_NETDEVICES=y
-CONFIG_SMSC_PHY=y
-CONFIG_NET_ETHERNET=y
-CONFIG_SMC91X=y
-CONFIG_SMSC911X=y
 CONFIG_KS8851=y
 CONFIG_KS8851_MLL=y
-CONFIG_LIBERTAS=m
-CONFIG_LIBERTAS_USB=m
-CONFIG_LIBERTAS_SDIO=m
-CONFIG_LIBERTAS_DEBUG=y
+CONFIG_SMC91X=y
+CONFIG_SMSC911X=y
+CONFIG_SMSC_PHY=y
 CONFIG_USB_USBNET=y
-CONFIG_USB_NET_SMSC95XX=y
+CONFIG_USB_NET_SMSC95XX=m
 CONFIG_USB_ALI_M5632=y
 CONFIG_USB_AN2720=y
 CONFIG_USB_EPSON2888=y
 CONFIG_USB_KC2190=y
+CONFIG_LIBERTAS=m
+CONFIG_LIBERTAS_USB=m
+CONFIG_LIBERTAS_SDIO=m
+CONFIG_LIBERTAS_DEBUG=y
 CONFIG_INPUT_JOYDEV=y
 CONFIG_INPUT_EVDEV=y
 CONFIG_KEYBOARD_GPIO=y
@@ -132,14 +130,13 @@ CONFIG_POWER_SUPPLY=y
 CONFIG_WATCHDOG=y
 CONFIG_OMAP_WATCHDOG=y
 CONFIG_TWL4030_WATCHDOG=y
-CONFIG_REGULATOR_TWL4030=y
 CONFIG_REGULATOR_TPS65023=y
 CONFIG_REGULATOR_TPS6507X=y
+CONFIG_REGULATOR_TWL4030=y
 CONFIG_FB=y
 CONFIG_FIRMWARE_EDID=y
 CONFIG_FB_MODE_HELPERS=y
 CONFIG_FB_TILEBLITTING=y
-CONFIG_FB_OMAP_LCD_VGA=y
 CONFIG_OMAP2_DSS=m
 CONFIG_OMAP2_DSS_RFBI=y
 CONFIG_OMAP2_DSS_SDI=y
@@ -154,7 +151,6 @@ CONFIG_PANEL_ACX565AKM=m
 CONFIG_BACKLIGHT_LCD_SUPPORT=y
 CONFIG_LCD_CLASS_DEVICE=y
 CONFIG_LCD_PLATFORM=y
-CONFIG_DISPLAY_SUPPORT=y
 CONFIG_FRAMEBUFFER_CONSOLE=y
 CONFIG_FRAMEBUFFER_CONSOLE_ROTATION=y
 CONFIG_FONTS=y
@@ -174,13 +170,15 @@ CONFIG_SND_OMAP_SOC_OMAP3_PANDORA=m
 CONFIG_USB=y
 CONFIG_USB_DEBUG=y
 CONFIG_USB_ANNOUNCE_NEW_DEVICES=y
-CONFIG_USB_DEVICEFS=y
 CONFIG_USB_SUSPEND=y
 CONFIG_USB_MON=y
+CONFIG_USB_EHCI_HCD=m
+CONFIG_USB_EHCI_ROOT_HUB_TT=y
+CONFIG_USB_EHCI_HCD_PLATFORM=m
 CONFIG_USB_WDM=y
 CONFIG_USB_STORAGE=y
-CONFIG_USB_LIBUSUAL=y
 CONFIG_USB_TEST=y
+CONFIG_OMAP_USB2=m
 CONFIG_USB_GADGET=y
 CONFIG_USB_GADGET_DEBUG=y
 CONFIG_USB_GADGET_DEBUG_FILES=y
@@ -214,23 +212,18 @@ CONFIG_JFFS2_RUBIN=y
 CONFIG_UBIFS_FS=y
 CONFIG_CRAMFS=y
 CONFIG_NFS_FS=y
-CONFIG_NFS_V3=y
 CONFIG_NFS_V3_ACL=y
 CONFIG_NFS_V4=y
 CONFIG_ROOT_NFS=y
-CONFIG_PARTITION_ADVANCED=y
 CONFIG_NLS_CODEPAGE_437=y
 CONFIG_NLS_ISO8859_1=y
 CONFIG_PRINTK_TIME=y
 CONFIG_MAGIC_SYSRQ=y
-CONFIG_DEBUG_KERNEL=y
 CONFIG_SCHEDSTATS=y
 CONFIG_TIMER_STATS=y
 CONFIG_PROVE_LOCKING=y
-CONFIG_DEBUG_SPINLOCK_SLEEP=y
 # CONFIG_DEBUG_BUGVERBOSE is not set
 CONFIG_DEBUG_INFO=y
-# CONFIG_RCU_CPU_STALL_DETECTOR is not set
 CONFIG_SECURITY=y
 CONFIG_CRYPTO_MICHAEL_MIC=y
 # CONFIG_CRYPTO_ANSI_CPRNG is not set
@@ -239,4 +232,3 @@ CONFIG_CRC_T10DIF=y
 CONFIG_CRC_ITU_T=y
 CONFIG_CRC7=y
 CONFIG_LIBCRC32C=y
-CONFIG_SOC_OMAP5=y