On 2 June 2017 at 10:47, Geert Uytterhoeven [off-list ref] wrote:
Hi Wolfram, Ulf, Simon,
While investigating suspend/resume for the R-Car Gen3 clock driver, I
noticed a clock imbalance for SDHI on Salvator-X.
After boot:
# head -2 /sys/kernel/debug/clk/clk_summary
clock enable_cnt prepare_cnt rate accuracy phase
-------------------------------------------------------------
# grep sd /sys/kernel/debug/clk/clk_summary
.sdsrc 3 3 399999984 0 0
sd3 1 1 6250000 0 0
sdif3 1 2 6250000 0 0
sd2 1 1 199999992 0 0
sdif2 1 2 199999992 0 0
sd1 0 0 199999992 0 0
sdif1 0 0 199999992 0 0
sd0 1 1 6250000 0 0
sdif0 1 2 6250000 0 0
After s2idle suspend/resume:
# grep sd /sys/kernel/debug/clk/clk_summary
.sdsrc 3 3 399999984 0 0
sd3 1 1 6250000 0 0
sdif3 2 2 6250000 0 0
sd2 1 1 199999992 0 0
sdif2 2 2 199999992 0 0
sd1 0 0 199999992 0 0
sdif1 0 0 199999992 0 0
sd0 1 1 6250000 0 0
sdif0 2 2 6250000 0 0
Enable counts are 1 before suspend, and 2 after resume.
Boot: Enabled once (also at hardware level):
platform_drv_probe
renesas_sdhi_sys_dmac_probe
renesas_sdhi_probe
tmio_mmc_host_probe
renesas_sdhi_clk_enable
The driver calls pm_runtime_set_active() during ->probe(), which means
genpd's ->runtime_resume() callback isn't invoked during that point.
In other words, the clocks managed by the clock domain isn't enabled
during ->probe() as genpd's doesn't get to run pm_clk_resume() from
its ->runtime_resume() callback. Unless I am missing something. :-)
I think this is the reason to the following problems. How to fix it?
The driver needs to call pm_runtime_get_sync() instead of
pm_runtime_set_active(), however that may requires some careful
changes if one wants the driver to be able to enable clocks also when
CONFIG_PM is unset. If not, it's pretty easy, else I would do
something like below.
Add a "init" flag to host struct, and set that flag before calling
pm_runtime_get_sync() in ->probe(). When the driver's
->runtime_resume() callbacks get called when the "init" flag is set,
just do an early return 0, as t means ->probe() is running and has
already taken care of the enabling the clock. When probe is done, and
before dropping runtime usage count with pm_runtime_put(), reset the
"init" flag.
Suspend: Disabled once (also at hardware level):
suspend_devices_and_enter
dpm_suspend_start
dpm_suspend
__device_suspend
dpm_run_callback
pm_runtime_force_suspend
genpd_runtime_suspend
pm_generic_runtime_suspend
tmio_mmc_host_runtime_suspend
renesas_sdhi_clk_disable
Resume: Enabled twice (first one enables at hardware level):
dpm_resume_noirq
device_resume_noirq
dpm_run_callback
pm_genpd_resume_noirq
pm_runtime_force_resume
genpd_runtime_resume
genpd_start_dev
pm_clk_resume (1)
__genpd_runtime_resume
pm_generic_runtime_resume
tmio_mmc_host_runtime_resume
renesas_sdhi_clk_enable (2)
During subsequent suspends, the clock is disabled twice (last one disables
at hardware level), as expected:
suspend_devices_and_enter
dpm_suspend_start
dpm_suspend
__device_suspend
dpm_run_callback
pm_runtime_force_suspend
genpd_runtime_suspend
pm_generic_runtime_suspend
tmio_mmc_host_runtime_suspend
renesas_sdhi_clk_disable (1)
genpd_stop_dev
pm_clk_suspend (2)
From now on, the imbalance is gone.
Note that at boot and initial suspend, genpd does not seem to call into the
clock domain operations at all.
Presumable some call to pm_runtime_get_sync() is missing?
Thanks.
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
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Kind regards
Uffe