On Wed, 10 Apr 2024 at 16:19, Claudiu <claudiu.beznea@xxxxxxxxx> wrote: > > From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> > > The rzg2l_wdt_restart() is called from atomic context. Calling > pm_runtime_{get_sync, resume_and_get}() or any other runtime PM resume > APIs is not an option as it may lead to issues as described in commit > e4cf89596c1f ("watchdog: rzg2l_wdt: Fix 'BUG: Invalid wait context'") > that removed the pm_runtime_get_sync() and used directly the > clk_prepare_enable() APIs. Calling clk_prepare_enable() doesn't work from an atomic context either as it may sleep on the clk prepare mutex. As I said in the other reply too, it looks like we need a different solution. I am not sure what, but I am happy to help discuss it. > > Starting with RZ/G3S the watchdog could be part of its own software > controlled power domain (see the initial implementation in Link section). > In case the watchdog is not used the power domain is off and accessing > watchdog registers leads to aborts. > > To solve this the patch powers on the power domain using > dev_pm_genpd_resume() API before enabling its clock. This is not > sleeping or taking any other locks as the power domain will not be > registered with GENPD_FLAG_IRQ_SAFE flags. > > Link: https://lore.kernel.org/all/20240208124300.2740313-1-claudiu.beznea.uj@xxxxxxxxxxxxxx > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> Kind regards Uffe > --- > > Changes in v8: > - none, this patch is new > > drivers/watchdog/rzg2l_wdt.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c > index c8c20cfb97a3..98e5e9914a5d 100644 > --- a/drivers/watchdog/rzg2l_wdt.c > +++ b/drivers/watchdog/rzg2l_wdt.c > @@ -12,6 +12,7 @@ > #include <linux/module.h> > #include <linux/of.h> > #include <linux/platform_device.h> > +#include <linux/pm_domain.h> > #include <linux/pm_runtime.h> > #include <linux/reset.h> > #include <linux/units.h> > @@ -164,6 +165,17 @@ static int rzg2l_wdt_restart(struct watchdog_device *wdev, > struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev); > int ret; > > + /* > + * The device may be part of a power domain that is currently > + * powered off. We need to power it up before accessing registers. > + * We don't undo the dev_pm_genpd_resume() as the device need to > + * be up for the reboot to happen. Also, as we are in atomic context > + * here there is no need to increment PM runtime usage counter > + * (to make sure pm_runtime_active() doesn't return wrong code). > + */ > + if (!pm_runtime_active(wdev->parent)) > + dev_pm_genpd_resume(wdev->parent); > + > clk_prepare_enable(priv->pclk); > clk_prepare_enable(priv->osc_clk); > > -- > 2.39.2 > >