Re: [PATCH RFC 0/3] watchdog: rzg2l_wdt: Enable properly the watchdog clocks and power domain
From: claudiu beznea
Date: Wed Aug 07 2024 - 07:22:12 EST
Hi, Ulf,
Please, do you have any input/suggestions on this?
Thank you,
Claudiu Beznea
On 19.06.2024 15:09, Claudiu wrote:
> From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx>
>
> Hi,
>
> Watchdog device available on RZ/G3S SoC is part of a software-controlled
> power domain. The watchdog driver implements struct
> watchdog_ops::restart() handler which is called in atomic context via
> this call chain:
>
> kernel_restart() ->
> machine_restart() ->
> do_kernel_restart() ->
> atomic_notifier_call_chain() ->
> watchdog_restart_notifier()
> rzg2l_wdt_restart()
>
> When the rzg2l_wdt_restart() is called it may happen that the watchdog
> clocks to be disabled and the associated power domain to be off.
> Accessing watchdog registers in this state leads to aborts and system
> blocks.
>
> To solve this issue the series proposes a new API called
> dev_pm_genpd_resume_restart_dev() that is intended to be called in
> scenarios like this. In this RFC series the
> dev_pm_genpd_resume_restart_dev() checks if the system is in
> SYSTEM_RESTART context and call dev_pm_genpd_resume(). I also wanted to
> mark the device as a restart device with a new member in struct dev_pm_info
> (similar to struct dev_pm_info::syscore) and check it in the newly
> introduced API but then I told myself maybe it would be better to keep it
> simpler for the moment.
>
> Please let me know how do you consider this.
>
> Along with it, series addresses the usage of clk_prepare_enable() in
> rzg2l_wdt_restart() reported by Ulf Hansson at [1] and use the
> dev_pm_genpd_resume_restart_dev() in rzg2l_wdt driver.
>
> Please note that series is built on top of [1].
>
> A similar approach (using directly the dev_pm_genpd_resume() function in
> rzg2l_wdt was proposed at [2]). This series was posted separatelly to
> avoid blocking the initial support for the RZ/G3S SoC.
>
> Thank you,
> Claudiu Beznea
>
> [1] https://lore.kernel.org/all/20240531065723.1085423-1-claudiu.beznea.uj@xxxxxxxxxxxxxx/
> [2] https://lore.kernel.org/all/20240410134044.2138310-10-claudiu.beznea.uj@xxxxxxxxxxxxxx/
>
> Claudiu Beznea (3):
> pmdomain: core: Add a helper to power on the restart devices
> watchdog: rzg2l_wdt: Keep the clocks prepared
> watchdog: rzg2l_wdt: Power on the PM domain in rzg2l_wdt_restart()
>
> drivers/pmdomain/core.c | 18 +++++++++++++++
> drivers/watchdog/rzg2l_wdt.c | 43 +++++++++++++++++++++++++++++++-----
> include/linux/pm_domain.h | 2 ++
> 3 files changed, 58 insertions(+), 5 deletions(-)
>