* [RFC PATCH] mmc: tmio: properly balance RPM on remove
@ 2020-05-13 17:12 Wolfram Sang
2020-05-14 10:23 ` Ulf Hansson
2020-05-15 14:08 ` Geert Uytterhoeven
0 siblings, 2 replies; 4+ messages in thread
From: Wolfram Sang @ 2020-05-13 17:12 UTC (permalink / raw)
To: linux-mmc
Cc: linux-renesas-soc, Ulf Hansson, Wolfram Sang, Geert Uytterhoeven
From: Wolfram Sang <wsa+renesas@sang-engineering.com>
Because we enabled the device with _noresume, we should disable it with
_noidle to match the ref counting of the clocks during remove().
Reported-by: Geert Uytterhoeven <geert+renesas@glider.be>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
I think this is the proper fix to the problem Geert reported [1]. I am
not sure about a proper Fixes-tag, though. The corresponding _noidle
call in the probe()-error-path was added with:
aa86f1a38875 ("mmc: tmio: Fixup runtime PM management during probe")
However, from my understanding this is more fitting?
1b32999e205b ("mmc: tmio: Avoid boilerplate code in ->runtime_suspend()")
But maybe my understanding of the situation is still not perfect and
even the commit message is bogus? Ulf, since both mentioned commits are
from you, could you have a look? Thanks in advance!
Wolfram
[1] Message-ID: <alpine.DEB.2.21.2004291630090.4052@ramsan.of.borg>
drivers/mmc/host/tmio_mmc_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
index 9520bd94cf43..9a4ae954553b 100644
--- a/drivers/mmc/host/tmio_mmc_core.c
+++ b/drivers/mmc/host/tmio_mmc_core.c
@@ -1235,7 +1235,7 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host)
pm_runtime_dont_use_autosuspend(&pdev->dev);
if (host->native_hotplug)
pm_runtime_put_noidle(&pdev->dev);
- pm_runtime_put_sync(&pdev->dev);
+ pm_runtime_put_noidle(&pdev->dev);
pm_runtime_disable(&pdev->dev);
}
EXPORT_SYMBOL_GPL(tmio_mmc_host_remove);
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] mmc: tmio: properly balance RPM on remove
2020-05-13 17:12 [RFC PATCH] mmc: tmio: properly balance RPM on remove Wolfram Sang
@ 2020-05-14 10:23 ` Ulf Hansson
2020-05-14 10:31 ` Wolfram Sang
2020-05-15 14:08 ` Geert Uytterhoeven
1 sibling, 1 reply; 4+ messages in thread
From: Ulf Hansson @ 2020-05-14 10:23 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-mmc, Linux-Renesas, Wolfram Sang, Geert Uytterhoeven
On Wed, 13 May 2020 at 19:12, Wolfram Sang <wsa@kernel.org> wrote:
>
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> Because we enabled the device with _noresume, we should disable it with
> _noidle to match the ref counting of the clocks during remove().
>
> Reported-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>
> I think this is the proper fix to the problem Geert reported [1]. I am
> not sure about a proper Fixes-tag, though. The corresponding _noidle
> call in the probe()-error-path was added with:
>
> aa86f1a38875 ("mmc: tmio: Fixup runtime PM management during probe")
>
> However, from my understanding this is more fitting?
>
> 1b32999e205b ("mmc: tmio: Avoid boilerplate code in ->runtime_suspend()")
>
> But maybe my understanding of the situation is still not perfect and
> even the commit message is bogus? Ulf, since both mentioned commits are
> from you, could you have a look? Thanks in advance!
I did start to look at it, but got sidetracked with other things.
Anyway, I will check in the next couple of days.
Kind regards
Uffe
>
> Wolfram
>
> [1] Message-ID: <alpine.DEB.2.21.2004291630090.4052@ramsan.of.borg>
>
> drivers/mmc/host/tmio_mmc_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/tmio_mmc_core.c b/drivers/mmc/host/tmio_mmc_core.c
> index 9520bd94cf43..9a4ae954553b 100644
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -1235,7 +1235,7 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host)
> pm_runtime_dont_use_autosuspend(&pdev->dev);
> if (host->native_hotplug)
> pm_runtime_put_noidle(&pdev->dev);
> - pm_runtime_put_sync(&pdev->dev);
> + pm_runtime_put_noidle(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
> }
> EXPORT_SYMBOL_GPL(tmio_mmc_host_remove);
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] mmc: tmio: properly balance RPM on remove
2020-05-14 10:23 ` Ulf Hansson
@ 2020-05-14 10:31 ` Wolfram Sang
0 siblings, 0 replies; 4+ messages in thread
From: Wolfram Sang @ 2020-05-14 10:31 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Linux-Renesas, Geert Uytterhoeven
[-- Attachment #1: Type: text/plain, Size: 148 bytes --]
> I did start to look at it, but got sidetracked with other things.
> Anyway, I will check in the next couple of days.
Much appreciated, thanks!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [RFC PATCH] mmc: tmio: properly balance RPM on remove
2020-05-13 17:12 [RFC PATCH] mmc: tmio: properly balance RPM on remove Wolfram Sang
2020-05-14 10:23 ` Ulf Hansson
@ 2020-05-15 14:08 ` Geert Uytterhoeven
1 sibling, 0 replies; 4+ messages in thread
From: Geert Uytterhoeven @ 2020-05-15 14:08 UTC (permalink / raw)
To: Wolfram Sang; +Cc: Linux MMC List, Linux-Renesas, Ulf Hansson, Wolfram Sang
Hi Wolfram,
On Wed, May 13, 2020 at 7:12 PM Wolfram Sang <wsa@kernel.org> wrote:
> From: Wolfram Sang <wsa+renesas@sang-engineering.com>
>
> Because we enabled the device with _noresume, we should disable it with
> _noidle to match the ref counting of the clocks during remove().
>
> Reported-by: Geert Uytterhoeven <geert+renesas@glider.be>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> I think this is the proper fix to the problem Geert reported [1]. I am
Thanks, I can confirm this fixes the issue on unbind, so
Tested-by: Geert Uytterhoeven <geert+renesas@glider.be>
> not sure about a proper Fixes-tag, though. The corresponding _noidle
> call in the probe()-error-path was added with:
>
> aa86f1a38875 ("mmc: tmio: Fixup runtime PM management during probe")
That one also added a pm_runtime_put() call to the normal probe path,
but a pm_runtime_put_noidle() call to the error path, causing another
possible imbalance, which is still there.
So I guess the former has to be replaced by a _noidle call, too?
Also, the conditional
if (_host->native_hotplug)
pm_runtime_get_noresume(&pdev->dev);
in probe() is not undone in the error path.
> However, from my understanding this is more fitting?
>
> 1b32999e205b ("mmc: tmio: Avoid boilerplate code in ->runtime_suspend()")
>
> But maybe my understanding of the situation is still not perfect and
> even the commit message is bogus? Ulf, since both mentioned commits are
> from you, could you have a look? Thanks in advance!
>
> Wolfram
>
> [1] Message-ID: <alpine.DEB.2.21.2004291630090.4052@ramsan.of.borg>
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] 4+ messages in thread
end of thread, other threads:[~2020-05-15 14:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-13 17:12 [RFC PATCH] mmc: tmio: properly balance RPM on remove Wolfram Sang
2020-05-14 10:23 ` Ulf Hansson
2020-05-14 10:31 ` Wolfram Sang
2020-05-15 14:08 ` Geert Uytterhoeven
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.