Re: [PATCH AUTOSEL 6.18-6.12] watchdog: imx7ulp_wdt: Keep WDOG running until A55 enters WFI on i.MX94

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] Hardware manipulation functions unconditionally re-enable local interrupts and sleep, causing kernel panics when invoked from atomic contexts such as the restart handler and noirq suspend.
- [High] Watchdog is not stopped during suspend if it was started by the bootloader but not yet opened by userspace, leading to a system reset.
--

commit 3360fb6ec8b553a77597875579f51efb6523f1f5
Author: Ranjani Vaidyanathan <ranjani.vaidyanathan@nxp.com>
watchdog: imx7ulp_wdt: Keep WDOG running until A55 enters WFI on i.MX94

This commit introduces a hardware feature flag to indicate CPU low-power-mode
auto clock gating support on i.MX94. It uses this flag to prevent the watchdog
from being stopped during suspend, allowing the LPCG hardware to safely manage
it.

Link: https://lore.kernel.org/r/20260206-imx94-wdog-v2-1-4dd725faec1f@nxp.com

> diff --git a/drivers/watchdog/imx7ulp_wdt.c b/drivers/watchdog/imx7ulp_wdt.c
> index 0f13a30533574..33e5eb87e05e6 100644
> --- a/drivers/watchdog/imx7ulp_wdt.c
> +++ b/drivers/watchdog/imx7ulp_wdt.c

[ ... ]

> @@ -359,7 +360,7 @@ static int __maybe_unused imx7ulp_wdt_suspend_noirq(struct device *dev)
>  {
>  	struct imx7ulp_wdt_device *imx7ulp_wdt = dev_get_drvdata(dev);
>  
> -	if (watchdog_active(&imx7ulp_wdt->wdd))
> +	if (watchdog_active(&imx7ulp_wdt->wdd) && !imx7ulp_wdt->hw->cpu_lpm_auto_cg)

[Severity: High]
This is a pre-existing issue, but does this conditional overlook watchdogs left
running by the bootloader?

If the bootloader starts the watchdog (so watchdog_hw_running() is true) but
userspace has not yet opened it (so watchdog_active() is false), the kernel's
ping worker handles the pinging. During system suspend, the PM core cancels
this ping worker.

Because this code only checks watchdog_active(), it will fail to stop the
hardware watchdog in this scenario. Consequently, the watchdog hardware keeps
running while the system is suspended, and because the ping worker is frozen,
the watchdog will time out and cause an unexpected system reset.

Should this code also check watchdog_hw_running(&imx7ulp_wdt->wdd)?

>  		imx7ulp_wdt_stop(&imx7ulp_wdt->wdd);

[Severity: High]
This is a pre-existing issue, but does this call chain trigger a "scheduling
while atomic" panic or break atomic context constraints?

imx7ulp_wdt_suspend_noirq() is called during the noirq phase of system suspend
with local interrupts disabled. It calls imx7ulp_wdt_stop(), which eventually
reaches _imx7ulp_wdt_enable().

_imx7ulp_wdt_enable() unconditionally calls local_irq_enable(), which breaks
the noirq interrupt constraints.

Furthermore, _imx7ulp_wdt_enable() then calls imx7ulp_wdt_wait_rcs(), which can
execute usleep_range() or readl_poll_timeout(). Sleeping with interrupts
disabled or inside a noirq callback will trigger a panic.

Since this same path can be reached during reboot via imx7ulp_wdt_restart()
(which is also called with interrupts disabled), is it safe to allow
_imx7ulp_wdt_enable() to re-enable interrupts and sleep?

>  	clk_disable_unprepare(imx7ulp_wdt->clk);
>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831133314.4125787-398-sashal@kernel.org?part=1