Message ID | 20240902132402.2628900-5-claudiu.beznea.uj@bp.renesas.com |
---|---|
State | Superseded |
Headers | show |
Series | watchdog: rzg2l_wdt: Enable properly the watchdog clocks and power domain | expand |
On Mon, Sep 2, 2024 at 3:24 PM Claudiu <claudiu.beznea@tuxon.dev> wrote: > From: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > > On RZ/G3S the watchdog can be part of a software-controlled PM domain. In > this case, the watchdog device need to be powered on in > struct watchdog_ops::restart API. This can be done though > pm_runtime_resume_and_get() API if the watchdog PM domain and watchdog > device are marked as IRQ safe. We mark the watchdog PM domain as IRQ safe > with GENPD_FLAG_IRQ_SAFE when the watchdog PM domain is registered and the > watchdog device though pm_runtime_irq_safe(). > > Before commit e4cf89596c1f ("watchdog: rzg2l_wdt: Fix 'BUG: Invalid wait > context'") pm_runtime_get_sync() was used in watchdog restart handler > (which is similar to pm_runtime_resume_and_get() except the later one > handles the runtime resume errors). > > Commit e4cf89596c1f ("watchdog: rzg2l_wdt: Fix 'BUG: Invalid wait > context'") dropped the pm_runtime_get_sync() and replaced it with > clk_prepare_enable() to avoid invalid wait context due to genpd_lock() > in genpd_runtime_resume() being called from atomic context. But > clk_prepare_enable() doesn't fit for this either (as reported by > Ulf Hansson) as clk_prepare() can also sleep (it just not throw invalid > wait context warning as it is not written for this). > > Because the watchdog device is marked now as IRQ safe (though this patch) > the irq_safe_dev_in_sleep_domain() call from genpd_runtime_resume() returns > 1 for devices not registering an IRQ safe PM domain for watchdog (as the > watchdog device is IRQ safe, PM domain is not and watchdog PM domain is > always-on), this being the case for RZ/G3S with old device trees and > the rest of the SoCs that use this driver, we can now drop also the > clk_prepare_enable() calls in restart handler and rely on > pm_runtime_resume_and_get(). > > Thus, drop clk_prepare_enable() and use pm_runtime_resume_and_get() in > watchdog restart handler. > > Acked-by: Guenter Roeck <linux@roeck-us.net> > Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> > Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com> > --- > > Changes in v4: > - collected Ulf's tag LGTM, so Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Gr{oetje,eeting}s, Geert
diff --git a/drivers/watchdog/rzg2l_wdt.c b/drivers/watchdog/rzg2l_wdt.c index 2a35f890a288..11bbe48160ec 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> @@ -166,8 +167,22 @@ 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); + /* + * In case of RZ/G3S the watchdog device may be part of an IRQ safe power + * domain that is currently powered off. In this case we need to power + * it on before accessing registers. Along with this the clocks will be + * enabled. We don't undo the pm_runtime_resume_and_get() as the device + * need to be on for the reboot to happen. + * + * For the rest of SoCs not registering a watchdog IRQ safe power + * domain it is safe to call pm_runtime_resume_and_get() as the + * irq_safe_dev_in_sleep_domain() call in genpd_runtime_resume() + * returns non zero value and the genpd_lock() is avoided, thus, there + * will be no invalid wait context reported by lockdep. + */ + ret = pm_runtime_resume_and_get(wdev->parent); + if (ret) + return ret; if (priv->devtype == WDT_RZG2L) { ret = reset_control_deassert(priv->rstc); @@ -275,6 +290,7 @@ static int rzg2l_wdt_probe(struct platform_device *pdev) priv->devtype = (uintptr_t)of_device_get_match_data(dev); + pm_runtime_irq_safe(&pdev->dev); pm_runtime_enable(&pdev->dev); priv->wdev.info = &rzg2l_wdt_ident;