| Message ID | 20240619120920.2703605-1-claudiu.beznea.uj@bp.renesas.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | watchdog: rzg2l_wdt: Enable properly the watchdog clocks and power domain | expand |
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@bp.renesas.com> > > 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@bp.renesas.com/ > [2] https://lore.kernel.org/all/20240410134044.2138310-10-claudiu.beznea.uj@bp.renesas.com/ > > 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(-) >
From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> 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@bp.renesas.com/ [2] https://lore.kernel.org/all/20240410134044.2138310-10-claudiu.beznea.uj@bp.renesas.com/ 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(-)