Linux on ARM based TI OMAP SoCs
 help / color / mirror / Atom feed
* [PATCH RFC 1/2] usb: ehci-omap: factor out clock handling
@ 2010-05-13 15:00 Anand Gadiyar
       [not found] ` <1273762822-23073-1-git-send-email-gadiyar-l0cyMroinI0@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Anand Gadiyar @ 2010-05-13 15:00 UTC (permalink / raw)
  To: linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA
  Cc: Anand Gadiyar

Factor out the clock enable/disable calls in the driver for
reuse in the suspend/resume paths.

Signed-off-by: Anand Gadiyar <gadiyar-l0cyMroinI0@public.gmane.org>
---
Based off v2.6.34-rc7 + gregkh-07-usb-2.6.34-rc7.patch
and one recent patch I posted [1]

[1] http://marc.info/?l=linux-usb&m=127315680127935&w=2

 drivers/usb/host/ehci-omap.c |   46 +++++++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 17 deletions(-)

Index: linux-2.6/drivers/usb/host/ehci-omap.c
===================================================================
--- linux-2.6.orig/drivers/usb/host/ehci-omap.c
+++ linux-2.6/drivers/usb/host/ehci-omap.c
@@ -190,6 +190,23 @@ struct ehci_hcd_omap {
 
 /*-------------------------------------------------------------------------*/
 
+static void ehci_omap_clock_power(struct ehci_hcd_omap *omap, int on)
+{
+	if (on) {
+		clk_enable(omap->usbtll_ick);
+		clk_enable(omap->usbtll_fck);
+		clk_enable(omap->usbhost_ick);
+		clk_enable(omap->usbhost1_48m_fck);
+		clk_enable(omap->usbhost2_120m_fck);
+	} else {
+		clk_disable(omap->usbhost2_120m_fck);
+		clk_disable(omap->usbhost1_48m_fck);
+		clk_disable(omap->usbhost_ick);
+		clk_disable(omap->usbtll_fck);
+		clk_disable(omap->usbtll_ick);
+	}
+}
+
 static void omap_usb_utmi_init(struct ehci_hcd_omap *omap, u8 tll_channel_mask)
 {
 	unsigned reg;
@@ -248,27 +265,27 @@ static int omap_start_ehc(struct ehci_hc
 
 	dev_dbg(omap->dev, "starting TI EHCI USB Controller\n");
 
-	/* Enable Clocks for USBHOST */
+	/* Get all the clock handles we need */
 	omap->usbhost_ick = clk_get(omap->dev, "usbhost_ick");
 	if (IS_ERR(omap->usbhost_ick)) {
+		dev_err(omap->dev, "could not get usbhost_ick\n");
 		ret =  PTR_ERR(omap->usbhost_ick);
 		goto err_host_ick;
 	}
-	clk_enable(omap->usbhost_ick);
 
 	omap->usbhost2_120m_fck = clk_get(omap->dev, "usbhost_120m_fck");
 	if (IS_ERR(omap->usbhost2_120m_fck)) {
+		dev_err(omap->dev, "could not get usbhost_120m_fck\n");
 		ret = PTR_ERR(omap->usbhost2_120m_fck);
 		goto err_host_120m_fck;
 	}
-	clk_enable(omap->usbhost2_120m_fck);
 
 	omap->usbhost1_48m_fck = clk_get(omap->dev, "usbhost_48m_fck");
 	if (IS_ERR(omap->usbhost1_48m_fck)) {
+		dev_err(omap->dev, "could not get usbhost_48m_fck\n");
 		ret = PTR_ERR(omap->usbhost1_48m_fck);
 		goto err_host_48m_fck;
 	}
-	clk_enable(omap->usbhost1_48m_fck);
 
 	if (omap->phy_reset) {
 		/* Refer: ISSUE1 */
@@ -288,20 +305,22 @@ static int omap_start_ehc(struct ehci_hc
 		udelay(10);
 	}
 
-	/* Configure TLL for 60Mhz clk for ULPI */
 	omap->usbtll_fck = clk_get(omap->dev, "usbtll_fck");
 	if (IS_ERR(omap->usbtll_fck)) {
+		dev_err(omap->dev, "could not get usbtll_fck\n");
 		ret = PTR_ERR(omap->usbtll_fck);
 		goto err_tll_fck;
 	}
-	clk_enable(omap->usbtll_fck);
 
 	omap->usbtll_ick = clk_get(omap->dev, "usbtll_ick");
 	if (IS_ERR(omap->usbtll_ick)) {
+		dev_err(omap->dev, "could not get usbtll_ick\n");
 		ret = PTR_ERR(omap->usbtll_ick);
 		goto err_tll_ick;
 	}
-	clk_enable(omap->usbtll_ick);
+
+	/* Now enable all the clocks in the correct order */
+	ehci_omap_clock_power(omap, 1);
 
 	/* perform TLL soft reset, and wait until reset is complete */
 	ehci_omap_writel(omap->tll_base, OMAP_USBTLL_SYSCONFIG,
@@ -428,15 +447,13 @@ static int omap_start_ehc(struct ehci_hc
 	return 0;
 
 err_sys_status:
-	clk_disable(omap->usbtll_ick);
+	ehci_omap_clock_power(omap, 0);
 	clk_put(omap->usbtll_ick);
 
 err_tll_ick:
-	clk_disable(omap->usbtll_fck);
 	clk_put(omap->usbtll_fck);
 
 err_tll_fck:
-	clk_disable(omap->usbhost1_48m_fck);
 	clk_put(omap->usbhost1_48m_fck);
 
 	if (omap->phy_reset) {
@@ -448,11 +465,9 @@ err_tll_fck:
 	}
 
 err_host_48m_fck:
-	clk_disable(omap->usbhost2_120m_fck);
 	clk_put(omap->usbhost2_120m_fck);
 
 err_host_120m_fck:
-	clk_disable(omap->usbhost_ick);
 	clk_put(omap->usbhost_ick);
 
 err_host_ick:
@@ -502,32 +517,29 @@ static void omap_stop_ehc(struct ehci_hc
 			dev_dbg(omap->dev, "operation timed out\n");
 	}
 
+	ehci_omap_clock_power(omap, 0);
+
 	if (omap->usbtll_fck != NULL) {
-		clk_disable(omap->usbtll_fck);
 		clk_put(omap->usbtll_fck);
 		omap->usbtll_fck = NULL;
 	}
 
 	if (omap->usbhost_ick != NULL) {
-		clk_disable(omap->usbhost_ick);
 		clk_put(omap->usbhost_ick);
 		omap->usbhost_ick = NULL;
 	}
 
 	if (omap->usbhost1_48m_fck != NULL) {
-		clk_disable(omap->usbhost1_48m_fck);
 		clk_put(omap->usbhost1_48m_fck);
 		omap->usbhost1_48m_fck = NULL;
 	}
 
 	if (omap->usbhost2_120m_fck != NULL) {
-		clk_disable(omap->usbhost2_120m_fck);
 		clk_put(omap->usbhost2_120m_fck);
 		omap->usbhost2_120m_fck = NULL;
 	}
 
 	if (omap->usbtll_ick != NULL) {
-		clk_disable(omap->usbtll_ick);
 		clk_put(omap->usbtll_ick);
 		omap->usbtll_ick = NULL;
 	}
--
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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH RFC 2/2] usb: ehci-omap: add suspend/resume support
       [not found] ` <1273762822-23073-1-git-send-email-gadiyar-l0cyMroinI0@public.gmane.org>
@ 2010-05-13 15:00   ` Anand Gadiyar
  2010-05-13 17:09     ` Alan Stern
  2010-05-13 20:01   ` [PATCH RFC 1/2] usb: ehci-omap: factor out clock handling Felipe Balbi
  1 sibling, 1 reply; 7+ messages in thread
From: Anand Gadiyar @ 2010-05-13 15:00 UTC (permalink / raw)
  To: linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA
  Cc: Anand Gadiyar, Ajay Kumar Gupta

Add support for suspend and resume to the ehci-omap driver.
Added routines for platform_driver suspend/resume and 
wrappers around ehci_bus_suspend/resume.

Disable the USB clocks after a bus_suspend and re-enable them
back before a bus_resume.

This allows the OMAP to enter low power modes when the driver
is loaded but no device is connected, or when all connected
devices are suspended and the root-hub autosuspends.

TODO:
- This patch breaks USB remote-wakeup after the root-hub
  autosuspends. This needs to be handled by enabling
  wakeup-detection on the IO pads, and is work-in-progress.

Signed-off-by: Anand Gadiyar <gadiyar-l0cyMroinI0@public.gmane.org>
Signed-off-by: Ajay Kumar Gupta <ajay.gupta-l0cyMroinI0@public.gmane.org>
---
Tested against the linux-omap-pm tree at [1] which allows the chip
to hit low power modes after all the clocks are disabled.

Tested on OMAP3 SDPs (which use an NXP ISP1504 PHY). Suspend-resume
works great with or without any connected devices.

Tested on OMAP3EVM EHCI port which has SMSC3320 PHY. Suspend/resume
works fine when no devices are connected to EHCI port but if any
device is connected then it gets detected as full speed device after
resume and EHCI port becomes unusable. This is likely due to known
errata i542 on SMSC PHY interoperability with OMAP.

[1] git://git.kernel.org/pub/scm/linux/kernel/git/khilman/linux-omap-pm.git

 drivers/usb/host/ehci-omap.c |  134 ++++++++++++++++++++++++++++++++++++-------
 1 file changed, 115 insertions(+), 19 deletions(-)

Index: linux-2.6/drivers/usb/host/ehci-omap.c
===================================================================
--- linux-2.6.orig/drivers/usb/host/ehci-omap.c
+++ linux-2.6/drivers/usb/host/ehci-omap.c
@@ -49,7 +49,10 @@
 #define	OMAP_USBTLL_REVISION				(0x00)
 #define	OMAP_USBTLL_SYSCONFIG				(0x10)
 #define	OMAP_USBTLL_SYSCONFIG_CACTIVITY			(1 << 8)
-#define	OMAP_USBTLL_SYSCONFIG_SIDLEMODE			(1 << 3)
+#define	OMAP_USBTLL_SYSCONFIG_SMARTIDLE		(2 << 3)
+#define	OMAP_USBTLL_SYSCONFIG_NOIDLE			(1 << 3)
+#define	OMAP_USBTLL_SYSCONFIG_FORCEIDLE		(0 << 3)
+#define	OMAP_USBTLL_SYSCONFIG_SIDLEMASK			(3 << 3)
 #define	OMAP_USBTLL_SYSCONFIG_ENAWAKEUP			(1 << 2)
 #define	OMAP_USBTLL_SYSCONFIG_SOFTRESET			(1 << 1)
 #define	OMAP_USBTLL_SYSCONFIG_AUTOIDLE			(1 << 0)
@@ -92,9 +95,15 @@
 /* UHH Register Set */
 #define	OMAP_UHH_REVISION				(0x00)
 #define	OMAP_UHH_SYSCONFIG				(0x10)
-#define	OMAP_UHH_SYSCONFIG_MIDLEMODE			(1 << 12)
+#define	OMAP_UHH_SYSCONFIG_SMARTSTDBY			(2 << 12)
+#define	OMAP_UHH_SYSCONFIG_NOSTDBY			(1 << 12)
+#define	OMAP_UHH_SYSCONFIG_FORCESTDBY			(0 << 12)
+#define	OMAP_UHH_SYSCONFIG_MIDLEMASK			(3 << 12)
 #define	OMAP_UHH_SYSCONFIG_CACTIVITY			(1 << 8)
-#define	OMAP_UHH_SYSCONFIG_SIDLEMODE			(1 << 3)
+#define	OMAP_UHH_SYSCONFIG_SMARTIDLE			(2 << 3)
+#define	OMAP_UHH_SYSCONFIG_NOIDLE			(1 << 3)
+#define	OMAP_UHH_SYSCONFIG_FORCEIDLE			(0 << 3)
+#define	OMAP_UHH_SYSCONFIG_SIDLEMASK			(3 << 3)
 #define	OMAP_UHH_SYSCONFIG_ENAWAKEUP			(1 << 2)
 #define	OMAP_UHH_SYSCONFIG_SOFTRESET			(1 << 1)
 #define	OMAP_UHH_SYSCONFIG_AUTOIDLE			(1 << 0)
@@ -159,6 +168,7 @@ struct ehci_hcd_omap {
 	struct clk		*usbhost1_48m_fck;
 	struct clk		*usbtll_fck;
 	struct clk		*usbtll_ick;
+	unsigned		suspended:1;
 
 	/* FIXME the following two workarounds are
 	 * board specific not silicon-specific so these
@@ -207,6 +217,35 @@ static void ehci_omap_clock_power(struct
 	}
 }
 
+static void ehci_omap_enable(struct ehci_hcd_omap *omap, int enable)
+{
+	u32 reg;
+
+	if (enable) {
+		ehci_omap_clock_power(omap, 1);
+
+		/* Enable NoIdle/NoStandby mode */
+		reg = ehci_omap_readl(omap->uhh_base, OMAP_UHH_SYSCONFIG);
+		reg &= ~(OMAP_UHH_SYSCONFIG_SIDLEMASK
+				| OMAP_UHH_SYSCONFIG_MIDLEMASK);
+		reg |= OMAP_UHH_SYSCONFIG_NOIDLE
+				| OMAP_UHH_SYSCONFIG_NOSTDBY;
+		ehci_omap_writel(omap->uhh_base, OMAP_UHH_SYSCONFIG, reg);
+		omap->suspended = 0;
+	} else {
+		/* Enable ForceIdle/ForceStandby mode */
+		reg = ehci_omap_readl(omap->uhh_base, OMAP_UHH_SYSCONFIG);
+		reg &= ~(OMAP_UHH_SYSCONFIG_SIDLEMASK
+				| OMAP_UHH_SYSCONFIG_MIDLEMASK);
+		reg |= OMAP_UHH_SYSCONFIG_FORCEIDLE
+				| OMAP_UHH_SYSCONFIG_FORCESTDBY;
+		ehci_omap_writel(omap->uhh_base, OMAP_UHH_SYSCONFIG, reg);
+
+		ehci_omap_clock_power(omap, 0);
+		omap->suspended = 1;
+	}
+}
+
 static void omap_usb_utmi_init(struct ehci_hcd_omap *omap, u8 tll_channel_mask)
 {
 	unsigned reg;
@@ -340,20 +379,21 @@ static int omap_start_ehc(struct ehci_hc
 
 	dev_dbg(omap->dev, "TLL RESET DONE\n");
 
-	/* (1<<3) = no idle mode only for initial debugging */
-	ehci_omap_writel(omap->tll_base, OMAP_USBTLL_SYSCONFIG,
-			OMAP_USBTLL_SYSCONFIG_ENAWAKEUP |
-			OMAP_USBTLL_SYSCONFIG_SIDLEMODE |
-			OMAP_USBTLL_SYSCONFIG_CACTIVITY);
-
+	/* Enable smart-idle, wakeup */
+	reg = OMAP_USBTLL_SYSCONFIG_CACTIVITY
+			| OMAP_USBTLL_SYSCONFIG_AUTOIDLE
+			| OMAP_USBTLL_SYSCONFIG_ENAWAKEUP
+			| OMAP_USBTLL_SYSCONFIG_SMARTIDLE;
+	ehci_omap_writel(omap->tll_base, OMAP_USBTLL_SYSCONFIG, reg);
 
 	/* Put UHH in NoIdle/NoStandby mode */
 	reg = ehci_omap_readl(omap->uhh_base, OMAP_UHH_SYSCONFIG);
-	reg |= (OMAP_UHH_SYSCONFIG_ENAWAKEUP
-			| OMAP_UHH_SYSCONFIG_SIDLEMODE
-			| OMAP_UHH_SYSCONFIG_CACTIVITY
-			| OMAP_UHH_SYSCONFIG_MIDLEMODE);
-	reg &= ~OMAP_UHH_SYSCONFIG_AUTOIDLE;
+	reg |= OMAP_UHH_SYSCONFIG_CACTIVITY
+			| OMAP_UHH_SYSCONFIG_AUTOIDLE
+			| OMAP_UHH_SYSCONFIG_ENAWAKEUP;
+	reg &= ~(OMAP_UHH_SYSCONFIG_SIDLEMASK | OMAP_UHH_SYSCONFIG_MIDLEMASK);
+	reg |= OMAP_UHH_SYSCONFIG_NOIDLE
+			| OMAP_UHH_SYSCONFIG_NOSTDBY;
 
 	ehci_omap_writel(omap->uhh_base, OMAP_UHH_SYSCONFIG, reg);
 
@@ -555,7 +595,56 @@ static void omap_stop_ehc(struct ehci_hc
 	dev_dbg(omap->dev, "Clock to USB host has been disabled\n");
 }
 
+#ifdef CONFIG_PM
 /*-------------------------------------------------------------------------*/
+static int ehci_omap_dev_suspend(struct device *dev)
+{
+	struct ehci_hcd_omap *omap = dev_get_drvdata(dev);
+
+	if (!omap->suspended)
+		ehci_omap_enable(omap, 0);
+	return 0;
+}
+
+static int ehci_omap_dev_resume(struct device *dev)
+{
+	struct ehci_hcd_omap *omap = dev_get_drvdata(dev);
+
+	if (omap->suspended)
+		ehci_omap_enable(omap, 1);
+	return 0;
+}
+
+static int ehci_omap_bus_suspend(struct usb_hcd *hcd)
+{
+	struct usb_bus *bus = hcd_to_bus(hcd);
+	int ret;
+
+	ret = ehci_bus_suspend(hcd);
+
+	ehci_omap_dev_suspend(bus->controller);
+
+	return ret;
+}
+static int ehci_omap_bus_resume(struct usb_hcd *hcd)
+{
+	struct usb_bus *bus = hcd_to_bus(hcd);
+	int ret;
+
+	ehci_omap_dev_resume(bus->controller);
+
+	ret = ehci_bus_resume(hcd);
+
+	return ret;
+}
+static const struct dev_pm_ops ehci_omap_dev_pm_ops = {
+	.suspend	= ehci_omap_dev_suspend,
+	.resume_noirq	= ehci_omap_dev_resume,
+};
+#define EHCI_OMAP_DEV_PM_OPS (&ehci_omap_dev_pm_ops)
+#else
+#define EHCI_OMAP_DEV_PM_OPS NULL
+#endif
 
 static const struct hc_driver ehci_omap_hc_driver;
 
@@ -614,6 +703,7 @@ static int ehci_hcd_omap_probe(struct pl
 	omap->port_mode[2]		= pdata->port_mode[2];
 	omap->ehci		= hcd_to_ehci(hcd);
 	omap->ehci->sbrn	= 0x20;
+	omap->suspended = 0;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 
@@ -735,6 +825,9 @@ static int ehci_hcd_omap_remove(struct p
 	struct usb_hcd *hcd = ehci_to_hcd(omap->ehci);
 	int i;
 
+	if (omap->suspended)
+		ehci_omap_enable(omap, 1);
+
 	usb_remove_hcd(hcd);
 	omap_stop_ehc(omap, hcd);
 	iounmap(hcd->regs);
@@ -757,6 +850,9 @@ static void ehci_hcd_omap_shutdown(struc
 	struct ehci_hcd_omap *omap = platform_get_drvdata(pdev);
 	struct usb_hcd *hcd = ehci_to_hcd(omap->ehci);
 
+	if (omap->suspended)
+		ehci_omap_enable(omap, 1);
+
 	if (hcd->driver->shutdown)
 		hcd->driver->shutdown(hcd);
 }
@@ -765,10 +861,9 @@ static struct platform_driver ehci_hcd_o
 	.probe			= ehci_hcd_omap_probe,
 	.remove			= ehci_hcd_omap_remove,
 	.shutdown		= ehci_hcd_omap_shutdown,
-	/*.suspend		= ehci_hcd_omap_suspend, */
-	/*.resume		= ehci_hcd_omap_resume, */
 	.driver = {
 		.name		= "ehci-omap",
+		.pm		= EHCI_OMAP_DEV_PM_OPS,
 	}
 };
 
@@ -811,9 +906,10 @@ static const struct hc_driver ehci_omap_
 	 */
 	.hub_status_data	= ehci_hub_status_data,
 	.hub_control		= ehci_hub_control,
-	.bus_suspend		= ehci_bus_suspend,
-	.bus_resume		= ehci_bus_resume,

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH RFC 2/2] usb: ehci-omap: add suspend/resume support
  2010-05-13 15:00   ` [PATCH RFC 2/2] usb: ehci-omap: add suspend/resume support Anand Gadiyar
@ 2010-05-13 17:09     ` Alan Stern
  2010-05-14 14:01       ` Gadiyar, Anand
  0 siblings, 1 reply; 7+ messages in thread
From: Alan Stern @ 2010-05-13 17:09 UTC (permalink / raw)
  To: Anand Gadiyar; +Cc: linux-usb, linux-omap, Ajay Kumar Gupta

On Thu, 13 May 2010, Anand Gadiyar wrote:

> Add support for suspend and resume to the ehci-omap driver.
> Added routines for platform_driver suspend/resume and 
> wrappers around ehci_bus_suspend/resume.

> +#ifdef CONFIG_PM
>  /*-------------------------------------------------------------------------*/
> +static int ehci_omap_dev_suspend(struct device *dev)
> +{
> +	struct ehci_hcd_omap *omap = dev_get_drvdata(dev);
> +
> +	if (!omap->suspended)
> +		ehci_omap_enable(omap, 0);
> +	return 0;
> +}
> +
> +static int ehci_omap_dev_resume(struct device *dev)
> +{
> +	struct ehci_hcd_omap *omap = dev_get_drvdata(dev);
> +
> +	if (omap->suspended)
> +		ehci_omap_enable(omap, 1);
> +	return 0;
> +}
> +
> +static int ehci_omap_bus_suspend(struct usb_hcd *hcd)
> +{
> +	struct usb_bus *bus = hcd_to_bus(hcd);
> +	int ret;
> +
> +	ret = ehci_bus_suspend(hcd);
> +
> +	ehci_omap_dev_suspend(bus->controller);
> +
> +	return ret;
> +}
> +static int ehci_omap_bus_resume(struct usb_hcd *hcd)
> +{
> +	struct usb_bus *bus = hcd_to_bus(hcd);
> +	int ret;
> +
> +	ehci_omap_dev_resume(bus->controller);
> +
> +	ret = ehci_bus_resume(hcd);
> +
> +	return ret;
> +}

You could use the runtime-PM interface instead of explicitly suspending 
and resuming the controller.  It is now standard.

Alan Stern


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH RFC 1/2] usb: ehci-omap: factor out clock handling
       [not found] ` <1273762822-23073-1-git-send-email-gadiyar-l0cyMroinI0@public.gmane.org>
  2010-05-13 15:00   ` [PATCH RFC 2/2] usb: ehci-omap: add suspend/resume support Anand Gadiyar
@ 2010-05-13 20:01   ` Felipe Balbi
  1 sibling, 0 replies; 7+ messages in thread
From: Felipe Balbi @ 2010-05-13 20:01 UTC (permalink / raw)
  To: Anand Gadiyar
  Cc: linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-omap-u79uwXL29TY76Z2rM5mHXA

On Thu, May 13, 2010 at 08:30:21PM +0530, Anand Gadiyar wrote:
> @@ -190,6 +190,23 @@ struct ehci_hcd_omap {
>  
>  /*-------------------------------------------------------------------------*/
>  
> +static void ehci_omap_clock_power(struct ehci_hcd_omap *omap, int on)
> +{
> +	if (on) {
> +		clk_enable(omap->usbtll_ick);
> +		clk_enable(omap->usbtll_fck);
> +		clk_enable(omap->usbhost_ick);
> +		clk_enable(omap->usbhost1_48m_fck);
> +		clk_enable(omap->usbhost2_120m_fck);
> +	} else {
> +		clk_disable(omap->usbhost2_120m_fck);
> +		clk_disable(omap->usbhost1_48m_fck);
> +		clk_disable(omap->usbhost_ick);
> +		clk_disable(omap->usbtll_fck);
> +		clk_disable(omap->usbtll_ick);
> +	}
> +}

this is sure useful, but it prevents from turning off interface clocks
while we don't need them. AFAICT, we only need interface clocks when we
need to read/write registers. And having a this functions makes things
more difficult to implement that sort of PM optmizations.

Functional clocks should be kept alive for as long as the driver is
probed and not suspended, true. But for the interface clocks we could
enable them dynamicaly when talking to specific register bases. I think
runtime pm can help with that, no ???

Ideally, when you reach suspend, all the activity on the device should
have already stopped, so you'd only need to disable the functional
clocks because interface clocks would have already been disabled due to
not needing them basically. Just needs some investigation on how long
does it take for the clock to actually stabilize and if enabling for
each register read/write is good or bad. IOW, you just need to check
which granularity we need for controlling interface clocks.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [PATCH RFC 2/2] usb: ehci-omap: add suspend/resume support
  2010-05-13 17:09     ` Alan Stern
@ 2010-05-14 14:01       ` Gadiyar, Anand
       [not found]         ` <5A47E75E594F054BAF48C5E4FC4B92AB0322D7D8AA-/tLxBxkBPtCIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
  0 siblings, 1 reply; 7+ messages in thread
From: Gadiyar, Anand @ 2010-05-14 14:01 UTC (permalink / raw)
  To: Alan Stern
  Cc: linux-usb@vger.kernel.org, linux-omap@vger.kernel.org,
	Gupta, Ajay Kumar

Alan Stern wrote:
> On Thu, 13 May 2010, Anand Gadiyar wrote:
> 
> > Add support for suspend and resume to the ehci-omap driver.
> > Added routines for platform_driver suspend/resume and 
> > wrappers around ehci_bus_suspend/resume.
> 
> > +#ifdef CONFIG_PM
> >  
> /*-------------------------------------------------------------------------*/
> > +static int ehci_omap_dev_suspend(struct device *dev)
> > +{
> > +	struct ehci_hcd_omap *omap = dev_get_drvdata(dev);
> > +
> > +	if (!omap->suspended)
> > +		ehci_omap_enable(omap, 0);
> > +	return 0;
> > +}
> > +
> > +static int ehci_omap_dev_resume(struct device *dev)
> > +{
> > +	struct ehci_hcd_omap *omap = dev_get_drvdata(dev);
> > +
> > +	if (omap->suspended)
> > +		ehci_omap_enable(omap, 1);
> > +	return 0;
> > +}
> > +
> > +static int ehci_omap_bus_suspend(struct usb_hcd *hcd)
> > +{
> > +	struct usb_bus *bus = hcd_to_bus(hcd);
> > +	int ret;
> > +
> > +	ret = ehci_bus_suspend(hcd);
> > +
> > +	ehci_omap_dev_suspend(bus->controller);
> > +
> > +	return ret;
> > +}
> > +static int ehci_omap_bus_resume(struct usb_hcd *hcd)
> > +{
> > +	struct usb_bus *bus = hcd_to_bus(hcd);
> > +	int ret;
> > +
> > +	ehci_omap_dev_resume(bus->controller);
> > +
> > +	ret = ehci_bus_resume(hcd);
> > +
> > +	return ret;
> > +}
> 
> You could use the runtime-PM interface instead of explicitly suspending 
> and resuming the controller.  It is now standard.
> 

Will work on this. I'll be out for a while, so it could be some time
before I repost.

Thanks,
Anand

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH RFC 2/2] usb: ehci-omap: add suspend/resume support
       [not found]         ` <5A47E75E594F054BAF48C5E4FC4B92AB0322D7D8AA-/tLxBxkBPtCIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
@ 2010-10-07 19:47           ` Laine Walker-Avina
  2010-10-07 20:04             ` Gadiyar, Anand
  0 siblings, 1 reply; 7+ messages in thread
From: Laine Walker-Avina @ 2010-10-07 19:47 UTC (permalink / raw)
  To: Gadiyar, Anand
  Cc: Alan Stern, linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Gupta, Ajay Kumar

On Fri, May 14, 2010 at 7:01 AM, Gadiyar, Anand <gadiyar-l0cyMroinI0@public.gmane.org> wrote:
> Alan Stern wrote:
>> On Thu, 13 May 2010, Anand Gadiyar wrote:
>>
>> > Add support for suspend and resume to the ehci-omap driver.
>> > Added routines for platform_driver suspend/resume and
>> > wrappers around ehci_bus_suspend/resume.
>>
>> > +#ifdef CONFIG_PM
>> >
>> /*-------------------------------------------------------------------------*/
>> > +static int ehci_omap_dev_suspend(struct device *dev)
>> > +{
>> > +   struct ehci_hcd_omap *omap = dev_get_drvdata(dev);
>> > +
>> > +   if (!omap->suspended)
>> > +           ehci_omap_enable(omap, 0);
>> > +   return 0;
>> > +}
>> > +
>> > +static int ehci_omap_dev_resume(struct device *dev)
>> > +{
>> > +   struct ehci_hcd_omap *omap = dev_get_drvdata(dev);
>> > +
>> > +   if (omap->suspended)
>> > +           ehci_omap_enable(omap, 1);
>> > +   return 0;
>> > +}
>> > +
>> > +static int ehci_omap_bus_suspend(struct usb_hcd *hcd)
>> > +{
>> > +   struct usb_bus *bus = hcd_to_bus(hcd);
>> > +   int ret;
>> > +
>> > +   ret = ehci_bus_suspend(hcd);
>> > +
>> > +   ehci_omap_dev_suspend(bus->controller);
>> > +
>> > +   return ret;
>> > +}
>> > +static int ehci_omap_bus_resume(struct usb_hcd *hcd)
>> > +{
>> > +   struct usb_bus *bus = hcd_to_bus(hcd);
>> > +   int ret;
>> > +
>> > +   ehci_omap_dev_resume(bus->controller);
>> > +
>> > +   ret = ehci_bus_resume(hcd);
>> > +
>> > +   return ret;
>> > +}
>>
>> You could use the runtime-PM interface instead of explicitly suspending
>> and resuming the controller.  It is now standard.
>>
>
> Will work on this. I'll be out for a while, so it could be some time
> before I repost.
>
> Thanks,
> Anand

Any more developments on this?

-- 
Laine Walker-Avina
--
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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH RFC 2/2] usb: ehci-omap: add suspend/resume support
  2010-10-07 19:47           ` Laine Walker-Avina
@ 2010-10-07 20:04             ` Gadiyar, Anand
  0 siblings, 0 replies; 7+ messages in thread
From: Gadiyar, Anand @ 2010-10-07 20:04 UTC (permalink / raw)
  To: Laine Walker-Avina
  Cc: Alan Stern, linux-usb@vger.kernel.org, linux-omap@vger.kernel.org,
	Gupta, Ajay Kumar

On Fri, Oct 8, 2010 at 1:17 AM, Laine Walker-Avina <lwalkera@ieee.org> wrote:
> On Fri, May 14, 2010 at 7:01 AM, Gadiyar, Anand <gadiyar@ti.com> wrote:
>> Alan Stern wrote:
>>> On Thu, 13 May 2010, Anand Gadiyar wrote:
>>>
>>> > Add support for suspend and resume to the ehci-omap driver.
>>> > Added routines for platform_driver suspend/resume and
>>> > wrappers around ehci_bus_suspend/resume.
>>>

...

>>> You could use the runtime-PM interface instead of explicitly suspending
>>> and resuming the controller.  It is now standard.
>>>
>>
>> Will work on this. I'll be out for a while, so it could be some time
>> before I repost.
>>
>> Thanks,
>> Anand
>
> Any more developments on this?
>

Yup. We've rewritten this driver to factor out the common
TLL and UHH programming, so that it plays nicely
when EHCI and OHCI are both loaded. This was needed
to be able to use the omap_device layer to use runtime PM

We didn't want to do the work twice, that's why we
abandoned this initial patch to do it cleanly once.
The current version is being tested now and should
be posted in a while.

It's available on internal trees on dev.omapzoom.org,
if you want to take a look. I can send a link if you like.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2010-10-07 20:04 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-13 15:00 [PATCH RFC 1/2] usb: ehci-omap: factor out clock handling Anand Gadiyar
     [not found] ` <1273762822-23073-1-git-send-email-gadiyar-l0cyMroinI0@public.gmane.org>
2010-05-13 15:00   ` [PATCH RFC 2/2] usb: ehci-omap: add suspend/resume support Anand Gadiyar
2010-05-13 17:09     ` Alan Stern
2010-05-14 14:01       ` Gadiyar, Anand
     [not found]         ` <5A47E75E594F054BAF48C5E4FC4B92AB0322D7D8AA-/tLxBxkBPtCIQmiDNMet8wC/G2K4zDHf@public.gmane.org>
2010-10-07 19:47           ` Laine Walker-Avina
2010-10-07 20:04             ` Gadiyar, Anand
2010-05-13 20:01   ` [PATCH RFC 1/2] usb: ehci-omap: factor out clock handling Felipe Balbi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox