| Message ID | 20190410222240.5800-1-niklas.soderlund+renesas@ragnatech.se (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] mmc: tmio: move runtime PM enablement to the driver implementations | expand |
On Thu, Apr 11, 2019 at 12:22:40AM +0200, Niklas Söderlund wrote: > Both the Renesas and Uniphier implementations perform actions which > affect runtime PM before calling into the core tmio_mmc_host_probe() > which enabled runtime PM. Move pm_runtime_enable() from the core and > tmio_mmc_host_probe() into each drivers probe() so it can be called > before any clocks or other resources are switched on. > > Reported-by: Geert Uytterhoeven <geert+renesas@glider.be> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > --- Thanks for keeping at this. Setting up the APE6 board for further tests was painful, I understood that. Since you lost the cover-letter from the last series, I think it should be mentioned that this fixes a clock imbalance problem (at least on Gen3). For the APE6 tests, we need to wait until Geert comes back. I surely would like his input. And Yamada-san's, too, to make sure his platform also benefits. > drivers/mmc/host/renesas_sdhi_core.c | 6 ++++++ > drivers/mmc/host/tmio_mmc.c | 5 +++++ > drivers/mmc/host/tmio_mmc_core.c | 11 +++++++++-- > drivers/mmc/host/uniphier-sd.c | 3 +++ > 4 files changed, 23 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c > index 5e9e36ed2107a01c..db73f9f1b186f0ff 100644 > --- a/drivers/mmc/host/renesas_sdhi_core.c > +++ b/drivers/mmc/host/renesas_sdhi_core.c > @@ -770,6 +770,8 @@ int renesas_sdhi_probe(struct platform_device *pdev, > /* All SDHI have SDIO status bits which must be 1 */ > mmc_data->flags |= TMIO_MMC_SDIO_STATUS_SETBITS; > > + pm_runtime_enable(&pdev->dev); > + > ret = renesas_sdhi_clk_enable(host); > if (ret) > goto efree; > @@ -850,6 +852,8 @@ int renesas_sdhi_probe(struct platform_device *pdev, > efree: > tmio_mmc_host_free(host); > > + pm_runtime_disable(&pdev->dev); > + > return ret; > } > EXPORT_SYMBOL_GPL(renesas_sdhi_probe); > @@ -861,6 +865,8 @@ int renesas_sdhi_remove(struct platform_device *pdev) > tmio_mmc_host_remove(host); > renesas_sdhi_clk_disable(host); > > + pm_runtime_disable(&pdev->dev); > + > return 0; > } > EXPORT_SYMBOL_GPL(renesas_sdhi_remove); > diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c > index 93e83ad25976e756..8539e10784b40961 100644 > --- a/drivers/mmc/host/tmio_mmc.c > +++ b/drivers/mmc/host/tmio_mmc.c > @@ -172,6 +172,8 @@ static int tmio_mmc_probe(struct platform_device *pdev) > host->mmc->f_max = pdata->hclk; > host->mmc->f_min = pdata->hclk / 512; > > + pm_runtime_enable(&pdev->dev); > + > ret = tmio_mmc_host_probe(host); > if (ret) > goto host_free; > @@ -191,6 +193,7 @@ static int tmio_mmc_probe(struct platform_device *pdev) > tmio_mmc_host_remove(host); > host_free: > tmio_mmc_host_free(host); > + pm_runtime_disable(&pdev->dev); > cell_disable: > if (cell->disable) > cell->disable(pdev); > @@ -207,6 +210,8 @@ static int tmio_mmc_remove(struct platform_device *pdev) > if (cell->disable) > cell->disable(pdev); > > + pm_runtime_disable(&pdev->dev); > + > return 0; > } > > diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c > index 130b91cb0f8a3fd1..26c148d3c8a2e655 100644 > --- a/drivers/mmc/host/tmio_mmc_core.c > +++ b/drivers/mmc/host/tmio_mmc_core.c > @@ -1152,6 +1152,15 @@ void tmio_mmc_host_free(struct tmio_mmc_host *host) > } > EXPORT_SYMBOL_GPL(tmio_mmc_host_free); > > +/** > + * tmio_mmc_host_probe() - Common probe for all implementations > + * @_host: Host to probe > + * > + * Perform tasks common to all implementations probe functions. > + * > + * The caller should have called pm_runtime_enable() prior to calling > + * the common probe function. > + */ > int tmio_mmc_host_probe(struct tmio_mmc_host *_host) > { > struct platform_device *pdev = _host->pdev; > @@ -1260,7 +1269,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host) > pm_runtime_set_active(&pdev->dev); > pm_runtime_set_autosuspend_delay(&pdev->dev, 50); > pm_runtime_use_autosuspend(&pdev->dev); > - pm_runtime_enable(&pdev->dev); > > ret = mmc_add_host(mmc); > if (ret) > @@ -1296,7 +1304,6 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host) > > pm_runtime_dont_use_autosuspend(&pdev->dev); > pm_runtime_put_sync(&pdev->dev); > - pm_runtime_disable(&pdev->dev); > } > EXPORT_SYMBOL_GPL(tmio_mmc_host_remove); > > diff --git a/drivers/mmc/host/uniphier-sd.c b/drivers/mmc/host/uniphier-sd.c > index 91a2be41edf6196b..49aad9a79c18d24a 100644 > --- a/drivers/mmc/host/uniphier-sd.c > +++ b/drivers/mmc/host/uniphier-sd.c > @@ -631,6 +631,7 @@ static int uniphier_sd_probe(struct platform_device *pdev) > host->clk_disable = uniphier_sd_clk_disable; > host->set_clock = uniphier_sd_set_clock; > > + pm_runtime_enable(&pdev->dev); > ret = uniphier_sd_clk_enable(host); > if (ret) > goto free_host; > @@ -652,6 +653,7 @@ static int uniphier_sd_probe(struct platform_device *pdev) > > free_host: > tmio_mmc_host_free(host); > + pm_runtime_disable(&pdev->dev); > > return ret; > } > @@ -662,6 +664,7 @@ static int uniphier_sd_remove(struct platform_device *pdev) > > tmio_mmc_host_remove(host); > uniphier_sd_clk_disable(host); > + pm_runtime_disable(&pdev->dev); > > return 0; > } > -- > 2.21.0 >
Hi Niklas, On Mon, Apr 15, 2019 at 1:10 PM Wolfram Sang <wsa@the-dreams.de> wrote: > On Thu, Apr 11, 2019 at 12:22:40AM +0200, Niklas Söderlund wrote: > > Both the Renesas and Uniphier implementations perform actions which > > affect runtime PM before calling into the core tmio_mmc_host_probe() Do you know which pm_runtime_*() calls were done too early? I guess they returned an error, which is not checked? I checked the various pm_runtime_get*() calls, but none of them failed, while they typically return -EACCES when called too early. > > which enabled runtime PM. Move pm_runtime_enable() from the core and > > tmio_mmc_host_probe() into each drivers probe() so it can be called > > before any clocks or other resources are switched on. > > > > Reported-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > --- > > Thanks for keeping at this. Setting up the APE6 board for further tests > was painful, I understood that. > > Since you lost the cover-letter from the last series, I think it should > be mentioned that this fixes a clock imbalance problem (at least on > Gen3). > > For the APE6 tests, we need to wait until Geert comes back. I surely > would like his input. And Yamada-san's, too, to make sure his platform > also benefits. Thanks, but I still see a clock imbalances in /sys/kernel/debug/clk/clk_summary when comparing before/after s2ram. On ape6evm: - mmcif0 2 2 0 100000000 0 0 50000 + mmcif0 1 1 0 100000000 0 0 50000 On r8a77965/salvator-xs: - s0d3 1 2 0 266240000 0 0 50000 + s0d3 2 2 0 266240000 0 0 50000 - sys-dmac0 0 1 0 266240000 0 0 50000 + sys-dmac0 1 1 0 266240000 0 0 50000 Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds
Hi Geert, Thanks for your feedback. On 2019-05-08 11:03:05 +0200, Geert Uytterhoeven wrote: > Hi Niklas, > > On Mon, Apr 15, 2019 at 1:10 PM Wolfram Sang <wsa@the-dreams.de> wrote: > > On Thu, Apr 11, 2019 at 12:22:40AM +0200, Niklas Söderlund wrote: > > > Both the Renesas and Uniphier implementations perform actions which > > > affect runtime PM before calling into the core tmio_mmc_host_probe() > > Do you know which pm_runtime_*() calls were done too early? > I guess they returned an error, which is not checked? The ones in tmio_mmc_host_probe() which is removed in this patch where called before PM was enabled. > > I checked the various pm_runtime_get*() calls, but none of them failed, > while they typically return -EACCES when called too early. > > > > which enabled runtime PM. Move pm_runtime_enable() from the core and > > > tmio_mmc_host_probe() into each drivers probe() so it can be called > > > before any clocks or other resources are switched on. > > > > > > Reported-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > > --- > > > > Thanks for keeping at this. Setting up the APE6 board for further tests > > was painful, I understood that. > > > > Since you lost the cover-letter from the last series, I think it should > > be mentioned that this fixes a clock imbalance problem (at least on > > Gen3). > > > > For the APE6 tests, we need to wait until Geert comes back. I surely > > would like his input. And Yamada-san's, too, to make sure his platform > > also benefits. > > Thanks, but I still see a clock imbalances in /sys/kernel/debug/clk/clk_summary > when comparing before/after s2ram. > > On ape6evm: > > - mmcif0 2 2 0 100000000 0 0 50000 > + mmcif0 1 1 0 100000000 0 0 50000 This is unrelated to this patch, this clock is handled by the sh_mmcif driver. I get the same diff of the mmcif0 clock with a suspend cycle even if i do not include the renesas_sdhi_* drivers in the system. I had a quick look at the issue and it's related to that the MCC core do not call MMC_POWER_UP after suspend while it do call it during boot. Why it does so I'm not sure. Also if I mock convert sh_mmcif to require PM the imbalance is gone which perplexes me a bit and wonder if I converted it wrong somehow. > > On r8a77965/salvator-xs: > > - s0d3 1 2 0 266240000 0 0 50000 > + s0d3 2 2 0 266240000 0 0 50000 > > - sys-dmac0 0 1 0 266240000 0 0 50000 > + sys-dmac0 1 1 0 266240000 0 0 50000 Even these are unrelated to this patch. If I test without renesas_sdhi_* driver in the system I get the same clock differences, in fact I get one more for sys-dmac1 (both with and without the shdi drivers). - s0d3 2 6 0 266240000 0 0 50000 + s0d3 4 6 0 266240000 0 0 50000 - sys-dmac0 0 1 0 266240000 0 0 50000 + sys-dmac0 1 1 0 266240000 0 0 50000 - sys-dmac1 0 1 0 266240000 0 0 50000 + sys-dmac1 1 1 0 266240000 0 0 50000 I have not investigate this further as I wish to make sens of this patch first ;-) Would you agree that with this information we should move forward with this patch as it solves the issue for the sdhi clocks on all effected SoCs ?
Hi Niklas, On Fri, May 17, 2019 at 1:03 AM Niklas Söderlund <niklas.soderlund@ragnatech.se> wrote: > On 2019-05-08 11:03:05 +0200, Geert Uytterhoeven wrote: > > On Mon, Apr 15, 2019 at 1:10 PM Wolfram Sang <wsa@the-dreams.de> wrote: > > > On Thu, Apr 11, 2019 at 12:22:40AM +0200, Niklas Söderlund wrote: > > > > Both the Renesas and Uniphier implementations perform actions which > > > > affect runtime PM before calling into the core tmio_mmc_host_probe() > > > > which enabled runtime PM. Move pm_runtime_enable() from the core and > > > > tmio_mmc_host_probe() into each drivers probe() so it can be called > > > > before any clocks or other resources are switched on. > > > > > > > > Reported-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > > > --- > > > > > > Thanks for keeping at this. Setting up the APE6 board for further tests > > > was painful, I understood that. > > > > > > Since you lost the cover-letter from the last series, I think it should > > > be mentioned that this fixes a clock imbalance problem (at least on > > > Gen3). > > > > > > For the APE6 tests, we need to wait until Geert comes back. I surely > > > would like his input. And Yamada-san's, too, to make sure his platform > > > also benefits. > > > > Thanks, but I still see a clock imbalances in /sys/kernel/debug/clk/clk_summary > > when comparing before/after s2ram. > > > > On ape6evm: > > > > - mmcif0 2 2 0 100000000 0 0 50000 > > + mmcif0 1 1 0 100000000 0 0 50000 > > This is unrelated to this patch, this clock is handled by the sh_mmcif > driver. I get the same diff of the mmcif0 clock with a suspend cycle > even if i do not include the renesas_sdhi_* drivers in the system. OK. > I had a quick look at the issue and it's related to that the MCC core do > not call MMC_POWER_UP after suspend while it do call it during boot. Why > it does so I'm not sure. Also if I mock convert sh_mmcif to require PM > the imbalance is gone which perplexes me a bit and wonder if I converted > it wrong somehow. Weird... To be investigated further? > > On r8a77965/salvator-xs: > > > > - s0d3 1 2 0 266240000 0 0 50000 > > + s0d3 2 2 0 266240000 0 0 50000 > > > > - sys-dmac0 0 1 0 266240000 0 0 50000 > > + sys-dmac0 1 1 0 266240000 0 0 50000 > > Even these are unrelated to this patch. If I test without renesas_sdhi_* > driver in the system I get the same clock differences, in fact I get one > more for sys-dmac1 (both with and without the shdi drivers). > > - s0d3 2 6 0 266240000 0 0 50000 > + s0d3 4 6 0 266240000 0 0 50000 > > - sys-dmac0 0 1 0 266240000 0 0 50000 > + sys-dmac0 1 1 0 266240000 0 0 50000 > > - sys-dmac1 0 1 0 266240000 0 0 50000 > + sys-dmac1 1 1 0 266240000 0 0 50000 Please ignore. I must have misread "s0d3" as "sd3" ;-) > I have not investigate this further as I wish to make sens of this patch > first ;-) Would you agree that with this information we should move > forward with this patch as it solves the issue for the sdhi clocks on > all effected SoCs ? Yes please, and thanks again! Gr{oetje,eeting}s, Geert
On Thu, 11 Apr 2019 at 00:29, Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> wrote: > > Both the Renesas and Uniphier implementations perform actions which > affect runtime PM before calling into the core tmio_mmc_host_probe() > which enabled runtime PM. Move pm_runtime_enable() from the core and > tmio_mmc_host_probe() into each drivers probe() so it can be called > before any clocks or other resources are switched on. > > Reported-by: Geert Uytterhoeven <geert+renesas@glider.be> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> Niklas, Wolfram, Can I apply this for next? Kind regards Uffe > --- > drivers/mmc/host/renesas_sdhi_core.c | 6 ++++++ > drivers/mmc/host/tmio_mmc.c | 5 +++++ > drivers/mmc/host/tmio_mmc_core.c | 11 +++++++++-- > drivers/mmc/host/uniphier-sd.c | 3 +++ > 4 files changed, 23 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c > index 5e9e36ed2107a01c..db73f9f1b186f0ff 100644 > --- a/drivers/mmc/host/renesas_sdhi_core.c > +++ b/drivers/mmc/host/renesas_sdhi_core.c > @@ -770,6 +770,8 @@ int renesas_sdhi_probe(struct platform_device *pdev, > /* All SDHI have SDIO status bits which must be 1 */ > mmc_data->flags |= TMIO_MMC_SDIO_STATUS_SETBITS; > > + pm_runtime_enable(&pdev->dev); > + > ret = renesas_sdhi_clk_enable(host); > if (ret) > goto efree; > @@ -850,6 +852,8 @@ int renesas_sdhi_probe(struct platform_device *pdev, > efree: > tmio_mmc_host_free(host); > > + pm_runtime_disable(&pdev->dev); > + > return ret; > } > EXPORT_SYMBOL_GPL(renesas_sdhi_probe); > @@ -861,6 +865,8 @@ int renesas_sdhi_remove(struct platform_device *pdev) > tmio_mmc_host_remove(host); > renesas_sdhi_clk_disable(host); > > + pm_runtime_disable(&pdev->dev); > + > return 0; > } > EXPORT_SYMBOL_GPL(renesas_sdhi_remove); > diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c > index 93e83ad25976e756..8539e10784b40961 100644 > --- a/drivers/mmc/host/tmio_mmc.c > +++ b/drivers/mmc/host/tmio_mmc.c > @@ -172,6 +172,8 @@ static int tmio_mmc_probe(struct platform_device *pdev) > host->mmc->f_max = pdata->hclk; > host->mmc->f_min = pdata->hclk / 512; > > + pm_runtime_enable(&pdev->dev); > + > ret = tmio_mmc_host_probe(host); > if (ret) > goto host_free; > @@ -191,6 +193,7 @@ static int tmio_mmc_probe(struct platform_device *pdev) > tmio_mmc_host_remove(host); > host_free: > tmio_mmc_host_free(host); > + pm_runtime_disable(&pdev->dev); > cell_disable: > if (cell->disable) > cell->disable(pdev); > @@ -207,6 +210,8 @@ static int tmio_mmc_remove(struct platform_device *pdev) > if (cell->disable) > cell->disable(pdev); > > + pm_runtime_disable(&pdev->dev); > + > return 0; > } > > diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c > index 130b91cb0f8a3fd1..26c148d3c8a2e655 100644 > --- a/drivers/mmc/host/tmio_mmc_core.c > +++ b/drivers/mmc/host/tmio_mmc_core.c > @@ -1152,6 +1152,15 @@ void tmio_mmc_host_free(struct tmio_mmc_host *host) > } > EXPORT_SYMBOL_GPL(tmio_mmc_host_free); > > +/** > + * tmio_mmc_host_probe() - Common probe for all implementations > + * @_host: Host to probe > + * > + * Perform tasks common to all implementations probe functions. > + * > + * The caller should have called pm_runtime_enable() prior to calling > + * the common probe function. > + */ > int tmio_mmc_host_probe(struct tmio_mmc_host *_host) > { > struct platform_device *pdev = _host->pdev; > @@ -1260,7 +1269,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host) > pm_runtime_set_active(&pdev->dev); > pm_runtime_set_autosuspend_delay(&pdev->dev, 50); > pm_runtime_use_autosuspend(&pdev->dev); > - pm_runtime_enable(&pdev->dev); > > ret = mmc_add_host(mmc); > if (ret) > @@ -1296,7 +1304,6 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host) > > pm_runtime_dont_use_autosuspend(&pdev->dev); > pm_runtime_put_sync(&pdev->dev); > - pm_runtime_disable(&pdev->dev); > } > EXPORT_SYMBOL_GPL(tmio_mmc_host_remove); > > diff --git a/drivers/mmc/host/uniphier-sd.c b/drivers/mmc/host/uniphier-sd.c > index 91a2be41edf6196b..49aad9a79c18d24a 100644 > --- a/drivers/mmc/host/uniphier-sd.c > +++ b/drivers/mmc/host/uniphier-sd.c > @@ -631,6 +631,7 @@ static int uniphier_sd_probe(struct platform_device *pdev) > host->clk_disable = uniphier_sd_clk_disable; > host->set_clock = uniphier_sd_set_clock; > > + pm_runtime_enable(&pdev->dev); > ret = uniphier_sd_clk_enable(host); > if (ret) > goto free_host; > @@ -652,6 +653,7 @@ static int uniphier_sd_probe(struct platform_device *pdev) > > free_host: > tmio_mmc_host_free(host); > + pm_runtime_disable(&pdev->dev); > > return ret; > } > @@ -662,6 +664,7 @@ static int uniphier_sd_remove(struct platform_device *pdev) > > tmio_mmc_host_remove(host); > uniphier_sd_clk_disable(host); > + pm_runtime_disable(&pdev->dev); > > return 0; > } > -- > 2.21.0 >
On Mon, May 27, 2019 at 03:14:36PM +0200, Ulf Hansson wrote: > On Thu, 11 Apr 2019 at 00:29, Niklas Söderlund > <niklas.soderlund+renesas@ragnatech.se> wrote: > > > > Both the Renesas and Uniphier implementations perform actions which > > affect runtime PM before calling into the core tmio_mmc_host_probe() > > which enabled runtime PM. Move pm_runtime_enable() from the core and > > tmio_mmc_host_probe() into each drivers probe() so it can be called > > before any clocks or other resources are switched on. > > > > Reported-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> > > Niklas, Wolfram, > > Can I apply this for next? Yes, Geert and Niklas sorted out that the regression with APE6 was something else. Acked-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
On Thu, 11 Apr 2019 at 00:29, Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> wrote: > > Both the Renesas and Uniphier implementations perform actions which > affect runtime PM before calling into the core tmio_mmc_host_probe() > which enabled runtime PM. Move pm_runtime_enable() from the core and > tmio_mmc_host_probe() into each drivers probe() so it can be called > before any clocks or other resources are switched on. > > Reported-by: Geert Uytterhoeven <geert+renesas@glider.be> > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> Applied for next, thanks! Kind regards Uffe > --- > drivers/mmc/host/renesas_sdhi_core.c | 6 ++++++ > drivers/mmc/host/tmio_mmc.c | 5 +++++ > drivers/mmc/host/tmio_mmc_core.c | 11 +++++++++-- > drivers/mmc/host/uniphier-sd.c | 3 +++ > 4 files changed, 23 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c > index 5e9e36ed2107a01c..db73f9f1b186f0ff 100644 > --- a/drivers/mmc/host/renesas_sdhi_core.c > +++ b/drivers/mmc/host/renesas_sdhi_core.c > @@ -770,6 +770,8 @@ int renesas_sdhi_probe(struct platform_device *pdev, > /* All SDHI have SDIO status bits which must be 1 */ > mmc_data->flags |= TMIO_MMC_SDIO_STATUS_SETBITS; > > + pm_runtime_enable(&pdev->dev); > + > ret = renesas_sdhi_clk_enable(host); > if (ret) > goto efree; > @@ -850,6 +852,8 @@ int renesas_sdhi_probe(struct platform_device *pdev, > efree: > tmio_mmc_host_free(host); > > + pm_runtime_disable(&pdev->dev); > + > return ret; > } > EXPORT_SYMBOL_GPL(renesas_sdhi_probe); > @@ -861,6 +865,8 @@ int renesas_sdhi_remove(struct platform_device *pdev) > tmio_mmc_host_remove(host); > renesas_sdhi_clk_disable(host); > > + pm_runtime_disable(&pdev->dev); > + > return 0; > } > EXPORT_SYMBOL_GPL(renesas_sdhi_remove); > diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c > index 93e83ad25976e756..8539e10784b40961 100644 > --- a/drivers/mmc/host/tmio_mmc.c > +++ b/drivers/mmc/host/tmio_mmc.c > @@ -172,6 +172,8 @@ static int tmio_mmc_probe(struct platform_device *pdev) > host->mmc->f_max = pdata->hclk; > host->mmc->f_min = pdata->hclk / 512; > > + pm_runtime_enable(&pdev->dev); > + > ret = tmio_mmc_host_probe(host); > if (ret) > goto host_free; > @@ -191,6 +193,7 @@ static int tmio_mmc_probe(struct platform_device *pdev) > tmio_mmc_host_remove(host); > host_free: > tmio_mmc_host_free(host); > + pm_runtime_disable(&pdev->dev); > cell_disable: > if (cell->disable) > cell->disable(pdev); > @@ -207,6 +210,8 @@ static int tmio_mmc_remove(struct platform_device *pdev) > if (cell->disable) > cell->disable(pdev); > > + pm_runtime_disable(&pdev->dev); > + > return 0; > } > > diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c > index 130b91cb0f8a3fd1..26c148d3c8a2e655 100644 > --- a/drivers/mmc/host/tmio_mmc_core.c > +++ b/drivers/mmc/host/tmio_mmc_core.c > @@ -1152,6 +1152,15 @@ void tmio_mmc_host_free(struct tmio_mmc_host *host) > } > EXPORT_SYMBOL_GPL(tmio_mmc_host_free); > > +/** > + * tmio_mmc_host_probe() - Common probe for all implementations > + * @_host: Host to probe > + * > + * Perform tasks common to all implementations probe functions. > + * > + * The caller should have called pm_runtime_enable() prior to calling > + * the common probe function. > + */ > int tmio_mmc_host_probe(struct tmio_mmc_host *_host) > { > struct platform_device *pdev = _host->pdev; > @@ -1260,7 +1269,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host) > pm_runtime_set_active(&pdev->dev); > pm_runtime_set_autosuspend_delay(&pdev->dev, 50); > pm_runtime_use_autosuspend(&pdev->dev); > - pm_runtime_enable(&pdev->dev); > > ret = mmc_add_host(mmc); > if (ret) > @@ -1296,7 +1304,6 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host) > > pm_runtime_dont_use_autosuspend(&pdev->dev); > pm_runtime_put_sync(&pdev->dev); > - pm_runtime_disable(&pdev->dev); > } > EXPORT_SYMBOL_GPL(tmio_mmc_host_remove); > > diff --git a/drivers/mmc/host/uniphier-sd.c b/drivers/mmc/host/uniphier-sd.c > index 91a2be41edf6196b..49aad9a79c18d24a 100644 > --- a/drivers/mmc/host/uniphier-sd.c > +++ b/drivers/mmc/host/uniphier-sd.c > @@ -631,6 +631,7 @@ static int uniphier_sd_probe(struct platform_device *pdev) > host->clk_disable = uniphier_sd_clk_disable; > host->set_clock = uniphier_sd_set_clock; > > + pm_runtime_enable(&pdev->dev); > ret = uniphier_sd_clk_enable(host); > if (ret) > goto free_host; > @@ -652,6 +653,7 @@ static int uniphier_sd_probe(struct platform_device *pdev) > > free_host: > tmio_mmc_host_free(host); > + pm_runtime_disable(&pdev->dev); > > return ret; > } > @@ -662,6 +664,7 @@ static int uniphier_sd_remove(struct platform_device *pdev) > > tmio_mmc_host_remove(host); > uniphier_sd_clk_disable(host); > + pm_runtime_disable(&pdev->dev); > > return 0; > } > -- > 2.21.0 >
diff --git a/drivers/mmc/host/renesas_sdhi_core.c b/drivers/mmc/host/renesas_sdhi_core.c index 5e9e36ed2107a01c..db73f9f1b186f0ff 100644 --- a/drivers/mmc/host/renesas_sdhi_core.c +++ b/drivers/mmc/host/renesas_sdhi_core.c @@ -770,6 +770,8 @@ int renesas_sdhi_probe(struct platform_device *pdev, /* All SDHI have SDIO status bits which must be 1 */ mmc_data->flags |= TMIO_MMC_SDIO_STATUS_SETBITS; + pm_runtime_enable(&pdev->dev); + ret = renesas_sdhi_clk_enable(host); if (ret) goto efree; @@ -850,6 +852,8 @@ int renesas_sdhi_probe(struct platform_device *pdev, efree: tmio_mmc_host_free(host); + pm_runtime_disable(&pdev->dev); + return ret; } EXPORT_SYMBOL_GPL(renesas_sdhi_probe); @@ -861,6 +865,8 @@ int renesas_sdhi_remove(struct platform_device *pdev) tmio_mmc_host_remove(host); renesas_sdhi_clk_disable(host); + pm_runtime_disable(&pdev->dev); + return 0; } EXPORT_SYMBOL_GPL(renesas_sdhi_remove); diff --git a/drivers/mmc/host/tmio_mmc.c b/drivers/mmc/host/tmio_mmc.c index 93e83ad25976e756..8539e10784b40961 100644 --- a/drivers/mmc/host/tmio_mmc.c +++ b/drivers/mmc/host/tmio_mmc.c @@ -172,6 +172,8 @@ static int tmio_mmc_probe(struct platform_device *pdev) host->mmc->f_max = pdata->hclk; host->mmc->f_min = pdata->hclk / 512; + pm_runtime_enable(&pdev->dev); + ret = tmio_mmc_host_probe(host); if (ret) goto host_free; @@ -191,6 +193,7 @@ static int tmio_mmc_probe(struct platform_device *pdev) tmio_mmc_host_remove(host); host_free: tmio_mmc_host_free(host); + pm_runtime_disable(&pdev->dev); cell_disable: if (cell->disable) cell->disable(pdev); @@ -207,6 +210,8 @@ static int tmio_mmc_remove(struct platform_device *pdev) if (cell->disable) cell->disable(pdev); + pm_runtime_disable(&pdev->dev); + return 0; } diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c index 130b91cb0f8a3fd1..26c148d3c8a2e655 100644 --- a/drivers/mmc/host/tmio_mmc_core.c +++ b/drivers/mmc/host/tmio_mmc_core.c @@ -1152,6 +1152,15 @@ void tmio_mmc_host_free(struct tmio_mmc_host *host) } EXPORT_SYMBOL_GPL(tmio_mmc_host_free); +/** + * tmio_mmc_host_probe() - Common probe for all implementations + * @_host: Host to probe + * + * Perform tasks common to all implementations probe functions. + * + * The caller should have called pm_runtime_enable() prior to calling + * the common probe function. + */ int tmio_mmc_host_probe(struct tmio_mmc_host *_host) { struct platform_device *pdev = _host->pdev; @@ -1260,7 +1269,6 @@ int tmio_mmc_host_probe(struct tmio_mmc_host *_host) pm_runtime_set_active(&pdev->dev); pm_runtime_set_autosuspend_delay(&pdev->dev, 50); pm_runtime_use_autosuspend(&pdev->dev); - pm_runtime_enable(&pdev->dev); ret = mmc_add_host(mmc); if (ret) @@ -1296,7 +1304,6 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host) pm_runtime_dont_use_autosuspend(&pdev->dev); pm_runtime_put_sync(&pdev->dev); - pm_runtime_disable(&pdev->dev); } EXPORT_SYMBOL_GPL(tmio_mmc_host_remove); diff --git a/drivers/mmc/host/uniphier-sd.c b/drivers/mmc/host/uniphier-sd.c index 91a2be41edf6196b..49aad9a79c18d24a 100644 --- a/drivers/mmc/host/uniphier-sd.c +++ b/drivers/mmc/host/uniphier-sd.c @@ -631,6 +631,7 @@ static int uniphier_sd_probe(struct platform_device *pdev) host->clk_disable = uniphier_sd_clk_disable; host->set_clock = uniphier_sd_set_clock; + pm_runtime_enable(&pdev->dev); ret = uniphier_sd_clk_enable(host); if (ret) goto free_host; @@ -652,6 +653,7 @@ static int uniphier_sd_probe(struct platform_device *pdev) free_host: tmio_mmc_host_free(host); + pm_runtime_disable(&pdev->dev); return ret; } @@ -662,6 +664,7 @@ static int uniphier_sd_remove(struct platform_device *pdev) tmio_mmc_host_remove(host); uniphier_sd_clk_disable(host); + pm_runtime_disable(&pdev->dev); return 0; }
Both the Renesas and Uniphier implementations perform actions which affect runtime PM before calling into the core tmio_mmc_host_probe() which enabled runtime PM. Move pm_runtime_enable() from the core and tmio_mmc_host_probe() into each drivers probe() so it can be called before any clocks or other resources are switched on. Reported-by: Geert Uytterhoeven <geert+renesas@glider.be> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se> --- drivers/mmc/host/renesas_sdhi_core.c | 6 ++++++ drivers/mmc/host/tmio_mmc.c | 5 +++++ drivers/mmc/host/tmio_mmc_core.c | 11 +++++++++-- drivers/mmc/host/uniphier-sd.c | 3 +++ 4 files changed, 23 insertions(+), 2 deletions(-)