Hi Felipe,

On Mon, Aug 6, 2012 at 2:19 PM, Felipe Balbi [off-list ref] wrote:

Hi,

On Fri, Aug 03, 2012 at 08:01:44PM +0530, ABRAHAM, KISHON VIJAY wrote:

quoted

quoted

quoted

+     return 0;
+}
+
+#ifdef CONFIG_PM_RUNTIME
+
+static int omap_usb2_runtime_suspend(struct device *dev)
+{
+     struct platform_device  *pdev = to_platform_device(dev);
+     struct omap_usb *phy = platform_get_drvdata(pdev);
+
+     clk_disable(phy->wkupclk);

weird. I would expect the wakeup clock to be enabled on suspend and
disabled on resume. Isn't this causing any unbalanced disable warnings ?

Even I was expecting the wakeup clock to be enabled on suspend but if
we have this enabled coreaon domain is never
gated and core does not hit low power state. btw Why do think this is
unbalanced?

because you never do a clk_enable() on probe(), so on your first
suspend, you will disable the clock without having enabled it before,
no? Unless pm_runtime forces a runtime_resume() when you call
pm_runtime_enable()...

quoted

quoted

quoted

+static int omap_usb2_runtime_resume(struct device *dev)
+{
+     u32 ret = 0;
+     struct platform_device  *pdev = to_platform_device(dev);
+     struct omap_usb *phy = platform_get_drvdata(pdev);
+
+     ret = clk_enable(phy->wkupclk);
+     if (ret < 0)
+             dev_err(phy->dev, "Failed to enable wkupclk %d\n", ret);
+
+     return ret;
+}
+
+static const struct dev_pm_ops omap_usb2_pm_ops = {
+     SET_RUNTIME_PM_OPS(omap_usb2_runtime_suspend, omap_usb2_runtime_resume,
+             NULL)

only runtime ? What about static suspend ? We need this to work also
after a traditional echo mem > /sys/power/state ;-)

The static suspend case is handled by users of this phy using
set_suspend hooks.

I'm not sure if that's too wise, what if your user enabled USB, but
for whatever reason loaded only the phy driver and not musb or dwc3. It
will fail, right ?

The enabling and disabling of phy is solely governed by usb controller
driver (musb/dwc3). So if you dont have musb/dwc3 loaded, the phy will
be for sure disabled.

Thanks
Kishon