From: Geert Uytterhoeven <geert@linux-m68k.org>
To: "Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>
Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>,
Masahiro Yamada <yamada.masahiro@socionext.com>,
Ulf Hansson <ulf.hansson@linaro.org>,
linux-mmc@vger.kernel.org, linux-pm@vger.kernel.org,
linux-renesas-soc@vger.kernel.org
Subject: Re: [PATCH 1/2] mmc: tmio: undo PM autosuspend when removing the host
Date: Wed, 29 Apr 2020 19:15:57 +0200 (CEST) [thread overview]
Message-ID: <alpine.DEB.2.21.2004291630090.4052@ramsan.of.borg> (raw)
In-Reply-To: <20190109223452.11184-2-niklas.soderlund+renesas@ragnatech.se>
[-- Attachment #1: Type: text/plain, Size: 6302 bytes --]
Hi Niklas,
On Wed, 9 Jan 2019, Niklas Söderlund wrote:
> When removing the driver make sure to undo the PM autosuspend configured
> when probing the host.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> Reviewed-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
This is commit bb60023c6387842b ("mmc: tmio: undo PM autosuspend when
removing the host"), and I've bisected a regression to it.
Sorry for taking that long to discover this issue (by loading a DT
overlay to enable SCIFA5 on Koelsch, which requires unbinding SHDI0
first).
> --- a/drivers/mmc/host/tmio_mmc_core.c
> +++ b/drivers/mmc/host/tmio_mmc_core.c
> @@ -1287,6 +1287,7 @@ void tmio_mmc_host_remove(struct tmio_mmc_host *host)
> cancel_delayed_work_sync(&host->delayed_reset_work);
> tmio_mmc_release_dma(host);
>
> + pm_runtime_dont_use_autosuspend(&pdev->dev);
> pm_runtime_put_sync(&pdev->dev);
> pm_runtime_disable(&pdev->dev);
> }
When unbinding the sdhi driver on e.g. Koelsch or Salvator-XS:
echo ee100000.sd > /sys/bus/platform/drivers/sh_mobile_sdhi/unbind
echo ee100000.sd > /sys/bus/platform/drivers/renesas_sdhi_internal_dmac/unbind
Two warnings are triggered:
WARNING: CPU: 0 PID: 586 at drivers/clk/clk.c:954 clk_core_disable+0x40/0x214
sdhi0 already disabled
CPU: 0 PID: 586 Comm: bash Not tainted 5.7.0-rc3-shmobile-00508-g3c2c87c467f1c34e-dirty #547
Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
[<c010de5c>] (unwind_backtrace) from [<c0109cbc>] (show_stack+0x10/0x14)
[<c0109cbc>] (show_stack) from [<c072d5ac>] (dump_stack+0x88/0xa8)
[<c072d5ac>] (dump_stack) from [<c011c190>] (__warn+0xd0/0xec)
[<c011c190>] (__warn) from [<c011c21c>] (warn_slowpath_fmt+0x70/0x9c)
[<c011c21c>] (warn_slowpath_fmt) from [<c03a09e4>] (clk_core_disable+0x40/0x214)
[<c03a09e4>] (clk_core_disable) from [<c03a0bd0>] (clk_core_disable_lock+0x18/0x24)
[<c03a0bd0>] (clk_core_disable_lock) from [<c059e910>] (renesas_sdhi_clk_disable+0x14/0x34)
[<c059e910>] (renesas_sdhi_clk_disable) from [<c059f2fc>] (renesas_sdhi_remove+0x18/0x20)
[<c059f2fc>] (renesas_sdhi_remove) from [<c043c054>] (platform_drv_remove+0x24/0x3c)
[<c043c054>] (platform_drv_remove) from [<c043afb4>] (device_release_driver_internal+0xf0/0x198)
[<c043afb4>] (device_release_driver_internal) from [<c0439140>] (unbind_store+0x44/0x68)
[<c0439140>] (unbind_store) from [<c02941fc>] (kernfs_fop_write+0x128/0x1e4)
[<c02941fc>] (kernfs_fop_write) from [<c0225598>] (__vfs_write+0x28/0xe0)
[<c0225598>] (__vfs_write) from [<c0226664>] (vfs_write+0x98/0xbc)
[<c0226664>] (vfs_write) from [<c02267ec>] (ksys_write+0x68/0xb4)
[<c02267ec>] (ksys_write) from [<c0100060>] (ret_fast_syscall+0x0/0x54)
...
WARNING: CPU: 0 PID: 586 at drivers/clk/clk.c:813 clk_core_unprepare+0x40/0x108
sdhi0 already unprepared
CPU: 0 PID: 586 Comm: bash Tainted: G W 5.7.0-rc3-shmobile-00508-g3c2c87c467f1c34e-dirty #547
Hardware name: Generic R-Car Gen2 (Flattened Device Tree)
[<c010de5c>] (unwind_backtrace) from [<c0109cbc>] (show_stack+0x10/0x14)
[<c0109cbc>] (show_stack) from [<c072d5ac>] (dump_stack+0x88/0xa8)
[<c072d5ac>] (dump_stack) from [<c011c190>] (__warn+0xd0/0xec)
[<c011c190>] (__warn) from [<c011c21c>] (warn_slowpath_fmt+0x70/0x9c)
[<c011c21c>] (warn_slowpath_fmt) from [<c03a1150>] (clk_core_unprepare+0x40/0x108)
[<c03a1150>] (clk_core_unprepare) from [<c03a2d34>] (clk_unprepare+0x24/0x2c)
[<c03a2d34>] (clk_unprepare) from [<c044e244>] (__pm_clk_remove+0x38/0x54)
[<c044e244>] (__pm_clk_remove) from [<c044e6cc>] (pm_clk_destroy+0xc4/0xdc)
[<c044e6cc>] (pm_clk_destroy) from [<c044c4ec>] (genpd_remove_device+0x10c/0x128)
[<c044c4ec>] (genpd_remove_device) from [<c044c54c>] (genpd_dev_pm_detach+0x44/0xe4)
[<c044c54c>] (genpd_dev_pm_detach) from [<c043c064>] (platform_drv_remove+0x34/0x3c)
[<c043c064>] (platform_drv_remove) from [<c043afb4>] (device_release_driver_internal+0xf0/0x198)
[<c043afb4>] (device_release_driver_internal) from [<c0439140>] (unbind_store+0x44/0x68)
[<c0439140>] (unbind_store) from [<c02941fc>] (kernfs_fop_write+0x128/0x1e4)
[<c02941fc>] (kernfs_fop_write) from [<c0225598>] (__vfs_write+0x28/0xe0)
[<c0225598>] (__vfs_write) from [<c0226664>] (vfs_write+0x98/0xbc)
[<c0226664>] (vfs_write) from [<c02267ec>] (ksys_write+0x68/0xb4)
[<c02267ec>] (ksys_write) from [<c0100060>] (ret_fast_syscall+0x0/0x54)
Removing the call to pm_runtime_dont_use_autosuspend() fixes that
symptom. But that is definitely not the right fix, as doing so causes
genpd_stop_dev() to no longer being called on unbind.
What seems to happen is:
- During driver probe, the clocks are enabled twice, by
1. renesas_sdhi_clk_enable(), called from renesas_sdhi_probe(),
2. pm_clk_resume(), called from tmio_mmc_host_probe().
- During driver unbind, the clocks are disabled thrice, by:
1. renesas_sdhi_clk_disable(), called from
tmio_mmc_host_runtime_suspend(),
2. pm_clk_suspend(), called from tmio_mmc_host_remove() through
pm_runtime_put_sync(),
3. renesas_sdhi_clk_disable(), called from renesas_sdhi_remove().
In addition, an extra clock unprepare happens in __pm_clk_remove(),
called from genpd_remove_device() through pm_clk_destroy().
Note that tmio_mmc_host_runtime_suspend() is never called, so there is
definitely an imbalance in the Runtime PM handling..
Without the call to pm_runtime_dont_use_autosuspend(), the clocks are
disabled twice during driver unbind, by:
1. renesas_sdhi_clk_disable(), called from renesas_sdhi_remove(),
2. __pm_clk_remove(), called from genpd_remove_device() through
pm_clk_destroy().
Hence the warning is gone, by accident, as e.g. the pm_clk_resume() call is
never balanced by its counterpart pm_clk_suspend().
Thanks for your comments!
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
next parent reply other threads:[~2020-04-29 17:16 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <20190109223452.11184-1-niklas.soderlund+renesas@ragnatech.se>
[not found] ` <20190109223452.11184-2-niklas.soderlund+renesas@ragnatech.se>
2020-04-29 17:15 ` Geert Uytterhoeven [this message]
2020-05-07 11:16 ` [PATCH 1/2] mmc: tmio: undo PM autosuspend when removing the host Wolfram Sang
2020-05-07 11:49 ` Geert Uytterhoeven
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.21.2004291630090.4052@ramsan.of.borg \
--to=geert@linux-m68k.org \
--cc=linux-mmc@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=linux-renesas-soc@vger.kernel.org \
--cc=niklas.soderlund+renesas@ragnatech.se \
--cc=ulf.hansson@linaro.org \
--cc=wsa+renesas@sang-engineering.com \
--cc=yamada.masahiro@socionext.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).