| Message ID | 20170117143528.11404-6-abailon@baylibre.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
On 01/17/2017 05:35 PM, Alexandre Bailon wrote: > Currently, DA8xx doesn't support PM runtime. > In addition, the glue driver is managing the clock itself. > But the CPPI DMA needs to manage this clock too. > Add support to PM runtime and use the callback to enable / disable > the clock. And because the CPPI 4.1 is a child of Da8xx USB, > it will be able to enable / disable the clock by using PM runtime. > > Signed-off-by: Alexandre Bailon <abailon@baylibre.com> > --- > drivers/usb/musb/da8xx.c | 41 ++++++++++++++++++++++++++++++++++------- > 1 file changed, 34 insertions(+), 7 deletions(-) > > diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c > index 046356f..e67c41d 100644 > --- a/drivers/usb/musb/da8xx.c > +++ b/drivers/usb/musb/da8xx.c > @@ -379,11 +379,7 @@ static int da8xx_musb_init(struct musb *musb) > > musb->mregs += DA8XX_MENTOR_CORE_OFFSET; > > - ret = clk_prepare_enable(glue->clk); > - if (ret) { > - dev_err(glue->dev, "failed to enable clock\n"); > - return ret; > - } > + pm_runtime_get(musb->controller->parent); Not get_sync()? You're accessing a register next thing... > /* Returns zero if e.g. not clocked */ > rev = musb_readl(reg_base, DA8XX_USB_REVISION_REG); [...] > @@ -614,12 +612,41 @@ static const struct of_device_id da8xx_id_table[] = { > MODULE_DEVICE_TABLE(of, da8xx_id_table); > #endif > > +static int da8xx_runtime_suspend(struct device *dev) > +{ > + struct da8xx_glue *glue = dev_get_drvdata(dev); > + > + clk_disable_unprepare(glue->clk); I thought RPM would do that for you... > + > + return 0; > +} > + > +static int da8xx_runtime_resume(struct device *dev) > +{ > + int ret; > + struct da8xx_glue *glue = dev_get_drvdata(dev); > + > + ret = clk_prepare_enable(glue->clk); And this too... > + if (ret) { > + dev_err(glue->dev, "failed to enable clock\n"); > + return ret; > + } > + > + return 0; > +} [...] MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/17/2017 06:37 PM, Sergei Shtylyov wrote: > On 01/17/2017 05:35 PM, Alexandre Bailon wrote: > >> Currently, DA8xx doesn't support PM runtime. >> In addition, the glue driver is managing the clock itself. >> But the CPPI DMA needs to manage this clock too. >> Add support to PM runtime and use the callback to enable / disable >> the clock. And because the CPPI 4.1 is a child of Da8xx USB, >> it will be able to enable / disable the clock by using PM runtime. >> >> Signed-off-by: Alexandre Bailon <abailon@baylibre.com> >> --- >> drivers/usb/musb/da8xx.c | 41 ++++++++++++++++++++++++++++++++++------- >> 1 file changed, 34 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c >> index 046356f..e67c41d 100644 >> --- a/drivers/usb/musb/da8xx.c >> +++ b/drivers/usb/musb/da8xx.c >> @@ -379,11 +379,7 @@ static int da8xx_musb_init(struct musb *musb) >> >> musb->mregs += DA8XX_MENTOR_CORE_OFFSET; >> >> - ret = clk_prepare_enable(glue->clk); >> - if (ret) { >> - dev_err(glue->dev, "failed to enable clock\n"); >> - return ret; >> - } >> + pm_runtime_get(musb->controller->parent); > > Not get_sync()? You're accessing a register next thing... > >> /* Returns zero if e.g. not clocked */ >> rev = musb_readl(reg_base, DA8XX_USB_REVISION_REG); > [...] >> @@ -614,12 +612,41 @@ static const struct of_device_id >> da8xx_id_table[] = { >> MODULE_DEVICE_TABLE(of, da8xx_id_table); >> #endif >> >> +static int da8xx_runtime_suspend(struct device *dev) >> +{ >> + struct da8xx_glue *glue = dev_get_drvdata(dev); >> + >> + clk_disable_unprepare(glue->clk); > > I thought RPM would do that for you... DA8xx doesn't support yet the common clock framework. So for now, we have to manage clock in the driver. > >> + >> + return 0; >> +} >> + >> +static int da8xx_runtime_resume(struct device *dev) >> +{ >> + int ret; >> + struct da8xx_glue *glue = dev_get_drvdata(dev); >> + >> + ret = clk_prepare_enable(glue->clk); > > And this too... > >> + if (ret) { >> + dev_err(glue->dev, "failed to enable clock\n"); >> + return ret; >> + } >> + >> + return 0; >> +} > [...] > > MBR, Sergei > Best Regards, Alexandre -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wednesday 18 January 2017 07:21 PM, Alexandre Bailon wrote: >>> +static int da8xx_runtime_suspend(struct device *dev) >>> +{ >>> + struct da8xx_glue *glue = dev_get_drvdata(dev); >>> + >>> + clk_disable_unprepare(glue->clk); >> I thought RPM would do that for you... > DA8xx doesn't support yet the common clock framework. > So for now, we have to manage clock in the driver. We do have arch/arm/mach-davinci/pm_domain.c which should help with pm_runtime. Did you already try without the explicit clock enables and it did not work? Please do use _sync() version of get() and put() calls also. Thanks, Sekhar -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/18/2017 03:02 PM, Sekhar Nori wrote: > On Wednesday 18 January 2017 07:21 PM, Alexandre Bailon wrote: >>>> +static int da8xx_runtime_suspend(struct device *dev) >>>> +{ >>>> + struct da8xx_glue *glue = dev_get_drvdata(dev); >>>> + >>>> + clk_disable_unprepare(glue->clk); > >>> I thought RPM would do that for you... > >> DA8xx doesn't support yet the common clock framework. >> So for now, we have to manage clock in the driver. > > We do have arch/arm/mach-davinci/pm_domain.c which should help with > pm_runtime. OK. I will take a look. > > Did you already try without the explicit clock enables and it did not > work? Please do use _sync() version of get() and put() calls also. Yes, without the clock enables, it doesn't work. > > Thanks, > Sekhar > Thanks, Alexandre -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/usb/musb/da8xx.c b/drivers/usb/musb/da8xx.c index 046356f..e67c41d 100644 --- a/drivers/usb/musb/da8xx.c +++ b/drivers/usb/musb/da8xx.c @@ -379,11 +379,7 @@ static int da8xx_musb_init(struct musb *musb) musb->mregs += DA8XX_MENTOR_CORE_OFFSET; - ret = clk_prepare_enable(glue->clk); - if (ret) { - dev_err(glue->dev, "failed to enable clock\n"); - return ret; - } + pm_runtime_get(musb->controller->parent); /* Returns zero if e.g. not clocked */ rev = musb_readl(reg_base, DA8XX_USB_REVISION_REG); @@ -426,7 +422,7 @@ static int da8xx_musb_init(struct musb *musb) err_phy_power_on: phy_exit(glue->phy); fail: - clk_disable_unprepare(glue->clk); + pm_runtime_put(musb->controller->parent); return ret; } @@ -438,7 +434,7 @@ static int da8xx_musb_exit(struct musb *musb) phy_power_off(glue->phy); phy_exit(glue->phy); - clk_disable_unprepare(glue->clk); + pm_runtime_put(musb->controller->parent); usb_put_phy(musb->xceiv); @@ -584,6 +580,8 @@ static int da8xx_probe(struct platform_device *pdev) pinfo.data = pdata; pinfo.size_data = sizeof(*pdata); + pm_runtime_enable(&pdev->dev); + glue->musb = platform_device_register_full(&pinfo); ret = PTR_ERR_OR_ZERO(glue->musb); if (ret) { @@ -614,12 +612,41 @@ static const struct of_device_id da8xx_id_table[] = { MODULE_DEVICE_TABLE(of, da8xx_id_table); #endif +static int da8xx_runtime_suspend(struct device *dev) +{ + struct da8xx_glue *glue = dev_get_drvdata(dev); + + clk_disable_unprepare(glue->clk); + + return 0; +} + +static int da8xx_runtime_resume(struct device *dev) +{ + int ret; + struct da8xx_glue *glue = dev_get_drvdata(dev); + + ret = clk_prepare_enable(glue->clk); + if (ret) { + dev_err(glue->dev, "failed to enable clock\n"); + return ret; + } + + return 0; +} + +static const struct dev_pm_ops da8xx_pm_ops = { + .runtime_suspend = da8xx_runtime_suspend, + .runtime_resume = da8xx_runtime_resume, +}; + static struct platform_driver da8xx_driver = { .probe = da8xx_probe, .remove = da8xx_remove, .driver = { .name = "musb-da8xx", .of_match_table = of_match_ptr(da8xx_id_table), + .pm = &da8xx_pm_ops, }, };
Currently, DA8xx doesn't support PM runtime. In addition, the glue driver is managing the clock itself. But the CPPI DMA needs to manage this clock too. Add support to PM runtime and use the callback to enable / disable the clock. And because the CPPI 4.1 is a child of Da8xx USB, it will be able to enable / disable the clock by using PM runtime. Signed-off-by: Alexandre Bailon <abailon@baylibre.com> --- drivers/usb/musb/da8xx.c | 41 ++++++++++++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 7 deletions(-)