linux-kernel - Re: [PATCH RFC 0/3] watchdog: rzg2l_wdt: Enable properly the watchdog clocks and power domain

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <201cbbe6-99eb-4622-bc5f-3d298f9e30b4@tuxon.dev>
Date: Wed, 7 Aug 2024 14:21:25 +0300
From: claudiu beznea <claudiu.beznea@...on.dev>
To: ulf.hansson@...aro.org, wim@...ux-watchdog.org, linux@...ck-us.net,
 rafael@...nel.org
Cc: linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-watchdog@...r.kernel.org, geert+renesas@...der.be,
 linux-renesas-soc@...r.kernel.org,
 Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
Subject: Re: [PATCH RFC 0/3] watchdog: rzg2l_wdt: Enable properly the watchdog
 clocks and power domain

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@...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(-)
>