Hi Claudiu, Thank you for the patch. On Thu, Jun 20, 2024 at 9:29 AM Claudiu <claudiu.beznea@xxxxxxxxx> wrote: > > From: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> > > The watchdog restart handler is called with interrupts disabled. In > rzg2l_wdt_restart() we call clk_prepare_enable() to enable the watchdog > clocks. The prepare part of clk_prepare_enable() may sleep. Sleep in > atomic context should not happen. The clock drivers for all the > micro-architectures where the RZ/G2L watchdog driver is used are not > implementing struct clk_ops::prepare(). Even so, to be sure we are > not hitted by this at some point, keep the watchdog clocks prepared > and only enable them in restart handler. It is guaranteed that > clk_enable() can be called in atomic context. > > Reported-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx> > Closes: https://lore.kernel.org/all/CAPDyKFq1+cL1M9qGY0P58ETHUZHGymxQL0w92emUJPMe7a_GxA@xxxxxxxxxxxxxx > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@xxxxxxxxxxxxxx> > --- > drivers/watchdog/rzg2l_wdt.c | 31 ++++++++++++++++++++++++++----- > 1 file changed, 26 insertions(+), 5 deletions(-) > > diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c > index 2a35f890a288..6e3d7512f38c 100644 > --- a/drivers/watchdog/rzg2l_wdt.c > +++ b/drivers/watchdog/rzg2l_wdt.c > @@ -166,8 +166,8 @@ static int rzg2l_wdt_restart(struct watchdog_device *wdev, > struct rzg2l_wdt_priv *priv = watchdog_get_drvdata(wdev); > int ret; > > - clk_prepare_enable(priv->pclk); > - clk_prepare_enable(priv->osc_clk); > + clk_enable(priv->pclk); > + clk_enable(priv->osc_clk); > I think we need to add a check before enabling the clocks: if (!watchdog_active(wdev)) { clk_enable(priv->pclk); clk_enable(priv->osc_clk); } > if (priv->devtype == WDT_RZG2L) { > ret = reset_control_deassert(priv->rstc); > @@ -226,11 +226,28 @@ static const struct watchdog_ops rzg2l_wdt_ops = { > .restart = rzg2l_wdt_restart, > }; > > +static int rzg2l_clks_prepare(struct rzg2l_wdt_priv *priv) > +{ > + int ret; > + > + ret = clk_prepare(priv->pclk); > + if (ret) > + return ret; > + > + ret = clk_prepare(priv->osc_clk); > + if (ret) > + clk_unprepare(priv->pclk); > + > + return ret; > +} > + > static void rzg2l_wdt_pm_disable(void *data) > { > - struct watchdog_device *wdev = data; > + struct rzg2l_wdt_priv *priv = data; > > - pm_runtime_disable(wdev->parent); > + pm_runtime_disable(priv->wdev.parent); > + clk_unprepare(priv->osc_clk); > + clk_unprepare(priv->pclk); > } > All the above chunk can go away if we use devm_clk_get_prepared() while requesting the clocks in the probe. Cheers, Prabhakar > static int rzg2l_wdt_probe(struct platform_device *pdev) > @@ -275,6 +292,10 @@ static int rzg2l_wdt_probe(struct platform_device *pdev) > > priv->devtype = (uintptr_t)of_device_get_match_data(dev); > > + ret = rzg2l_clks_prepare(priv); > + if (ret) > + return ret; > + > pm_runtime_enable(&pdev->dev); > > priv->wdev.info = &rzg2l_wdt_ident; > @@ -287,7 +308,7 @@ static int rzg2l_wdt_probe(struct platform_device *pdev) > > watchdog_set_drvdata(&priv->wdev, priv); > dev_set_drvdata(dev, priv); > - ret = devm_add_action_or_reset(&pdev->dev, rzg2l_wdt_pm_disable, &priv->wdev); > + ret = devm_add_action_or_reset(&pdev->dev, rzg2l_wdt_pm_disable, &priv); > if (ret) > return ret; > > -- > 2.39.2 > >
