* Re: [PATCH v2] mmc: tmio: move runtime PM enablement to the driver implementations
2019-04-10 22:22 [PATCH v2] mmc: tmio: move runtime PM enablement to the driver implementations Niklas Söderlund
@ 2019-04-15 11:10 ` Wolfram Sang
2019-05-08 9:03 ` Geert Uytterhoeven
2019-05-27 13:14 ` Ulf Hansson
2019-05-28 8:52 ` Ulf Hansson
2 siblings, 1 reply; 8+ messages in thread
From: Wolfram Sang @ 2019-04-15 11:10 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Wolfram Sang, Masahiro Yamada, Geert Uytterhoeven, linux-mmc,
linux-renesas-soc, Geert Uytterhoeven
[-- Attachment #1: Type: text/plain, Size: 5511 bytes --]
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
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] mmc: tmio: move runtime PM enablement to the driver implementations
2019-04-15 11:10 ` Wolfram Sang
@ 2019-05-08 9:03 ` Geert Uytterhoeven
2019-05-16 23:02 ` Niklas Söderlund
0 siblings, 1 reply; 8+ messages in thread
From: Geert Uytterhoeven @ 2019-05-08 9:03 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Wolfram Sang, Masahiro Yamada, Linux MMC List, Linux-Renesas,
Geert Uytterhoeven
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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] mmc: tmio: move runtime PM enablement to the driver implementations
2019-05-08 9:03 ` Geert Uytterhoeven
@ 2019-05-16 23:02 ` Niklas Söderlund
2019-05-17 7:29 ` Geert Uytterhoeven
0 siblings, 1 reply; 8+ messages in thread
From: Niklas Söderlund @ 2019-05-16 23:02 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Wolfram Sang, Masahiro Yamada, Linux MMC List, Linux-Renesas,
Geert Uytterhoeven
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 ?
--
Regards,
Niklas Söderlund
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] mmc: tmio: move runtime PM enablement to the driver implementations
2019-05-16 23:02 ` Niklas Söderlund
@ 2019-05-17 7:29 ` Geert Uytterhoeven
0 siblings, 0 replies; 8+ messages in thread
From: Geert Uytterhoeven @ 2019-05-17 7:29 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Wolfram Sang, Masahiro Yamada, Linux MMC List, Linux-Renesas,
Geert Uytterhoeven
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
--
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
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] mmc: tmio: move runtime PM enablement to the driver implementations
2019-04-10 22:22 [PATCH v2] mmc: tmio: move runtime PM enablement to the driver implementations Niklas Söderlund
2019-04-15 11:10 ` Wolfram Sang
@ 2019-05-27 13:14 ` Ulf Hansson
2019-05-27 13:21 ` Wolfram Sang
2019-05-28 8:52 ` Ulf Hansson
2 siblings, 1 reply; 8+ messages in thread
From: Ulf Hansson @ 2019-05-27 13:14 UTC (permalink / raw)
To: Niklas Söderlund, Wolfram Sang
Cc: Masahiro Yamada, Geert Uytterhoeven, linux-mmc, Linux-Renesas,
Geert Uytterhoeven
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH v2] mmc: tmio: move runtime PM enablement to the driver implementations
2019-05-27 13:14 ` Ulf Hansson
@ 2019-05-27 13:21 ` Wolfram Sang
0 siblings, 0 replies; 8+ messages in thread
From: Wolfram Sang @ 2019-05-27 13:21 UTC (permalink / raw)
To: Ulf Hansson
Cc: Niklas Söderlund, Wolfram Sang, Masahiro Yamada,
Geert Uytterhoeven, linux-mmc, Linux-Renesas, Geert Uytterhoeven
[-- Attachment #1: Type: text/plain, Size: 875 bytes --]
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>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] mmc: tmio: move runtime PM enablement to the driver implementations
2019-04-10 22:22 [PATCH v2] mmc: tmio: move runtime PM enablement to the driver implementations Niklas Söderlund
2019-04-15 11:10 ` Wolfram Sang
2019-05-27 13:14 ` Ulf Hansson
@ 2019-05-28 8:52 ` Ulf Hansson
2 siblings, 0 replies; 8+ messages in thread
From: Ulf Hansson @ 2019-05-28 8:52 UTC (permalink / raw)
To: Niklas Söderlund
Cc: Wolfram Sang, Masahiro Yamada, Geert Uytterhoeven, linux-mmc,
Linux-Renesas, Geert Uytterhoeven
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
>
^ permalink raw reply [flat|nested] 8+ messages in thread