On Wed, 17 May 2017, Tony Lindgren wrote:
> This is needed in preparation of adding support for omap3 and
> later OHCI. The runtime PM will only do something on platforms
> that implement it.
This patch isn't correct. See below.
> Cc: devicetree@xxxxxxxxxxxxxxx
> Cc: Hans de Goede <hdegoede@xxxxxxxxxx>
> Cc: Rob Herring <robh@xxxxxxxxxx>
> Cc: Roger Quadros <rogerq@xxxxxx>
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>
> Signed-off-by: Tony Lindgren <tony@xxxxxxxxxxx>
> ---
> drivers/usb/host/ohci-platform.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/usb/host/ohci-platform.c b/drivers/usb/host/ohci-platform.c
> --- a/drivers/usb/host/ohci-platform.c
> +++ b/drivers/usb/host/ohci-platform.c
> @@ -24,6 +24,7 @@
> #include <linux/err.h>
> #include <linux/phy/phy.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
> #include <linux/reset.h>
> #include <linux/usb/ohci_pdriver.h>
> #include <linux/usb.h>
> @@ -51,6 +52,10 @@ static int ohci_platform_power_on(struct platform_device *dev)
> struct ohci_platform_priv *priv = hcd_to_ohci_priv(hcd);
> int clk, ret, phy_num;
>
> + ret = pm_runtime_get_sync(&dev->dev);
> + if (ret < 0)
> + return ret;
> +
> for (clk = 0; clk < OHCI_MAX_CLKS && priv->clks[clk]; clk++) {
> ret = clk_prepare_enable(priv->clks[clk]);
> if (ret)
> @@ -96,6 +101,8 @@ static void ohci_platform_power_off(struct platform_device *dev)
> for (clk = OHCI_MAX_CLKS - 1; clk >= 0; clk--)
> if (priv->clks[clk])
> clk_disable_unprepare(priv->clks[clk]);
> +
> + pm_runtime_put_sync(&dev->dev);
> }
ohci_platform_power_on() is invoked (by default) by the runtime-resume
callback routine ohci_platform_resume(), through the pdata->power_on
method pointer. Likewise, ohci_platform_power_off() is invoked by the
runtime-suspend callback routine.
This means you shouldn't do pm_runtime_get/put calls from within these
routines.
Is there any particular reason you wanted to add these calls? In
general, USB host controllers are expected to go into runtime suspend
whenever there aren't any children keeping them awake. Hence they
usually don't need to worry about initiating their own suspends and
resumes.
> static struct hc_driver __read_mostly ohci_platform_hc_driver;
> @@ -242,6 +249,7 @@ static int ohci_platform_probe(struct platform_device *dev)
> }
> #endif
>
> + pm_runtime_enable(&dev->dev);
There should be a pm_runtime_set_active() just before this.
> if (pdata->power_on) {
> err = pdata->power_on(dev);
> if (err < 0)
> @@ -271,6 +279,7 @@ static int ohci_platform_probe(struct platform_device *dev)
> if (pdata->power_off)
> pdata->power_off(dev);
> err_reset:
> + pm_runtime_disable(&dev->dev);
> while (--rst >= 0)
> reset_control_assert(priv->resets[rst]);
> err_put_clks:
> @@ -305,6 +314,8 @@ static int ohci_platform_remove(struct platform_device *dev)
>
> usb_put_hcd(hcd);
>
> + pm_runtime_disable(&dev->dev);
> +
> if (pdata == &ohci_platform_defaults)
> dev->dev.platform_data = NULL;
These changes make sense.
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
