Re: [PATCH v3 03/13] watchdog: stm32_iwdg: add pretimeout support for iwdg v4.0

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] watchdog: stm32_iwdg: IRQ enabled before state init in
  stm32_iwdg_probe()
--

commit f781715ad1087a5b9a86d2dd54ce3b998243a348
Author: Antonio Borneo <antonio.borneo@foss.st.com>

watchdog: stm32_iwdg: add pretimeout support for iwdg v4.0

This commit updates stm32_iwdg_isr() to clear pretimeout interrupts via
IWDG_ICR on IWDG hardware version 4.0 and later, and adds a reg_set()
helper.

> diff --git a/drivers/watchdog/stm32_iwdg.c b/drivers/watchdog/stm32_iwdg.c
> index ce3e1f96ef50a..6121c23889655 100644
> --- a/drivers/watchdog/stm32_iwdg.c
> +++ b/drivers/watchdog/stm32_iwdg.c
[ ... ]
> @@ -194,11 +207,11 @@ static irqreturn_t stm32_iwdg_isr(int irq, void *wdog_arg)
>  {
>  	struct watchdog_device *wdd = wdog_arg;
>  	struct stm32_iwdg *wdt = watchdog_get_drvdata(wdd);
> -	u32 reg;
>  
> -	reg = reg_read(wdt->regs, IWDG_EWCR);
> -	reg |= EWCR_EWIC;
> -	reg_write(wdt->regs, IWDG_EWCR, reg);
> +	if (wdt->hw_version >= ICR_MIN_VER)
> +		reg_write(wdt->regs, IWDG_ICR, ICR_EWIC);
> +	else
> +		reg_set(wdt->regs, IWDG_EWCR, EWCR_EWIC);
>  
>  	watchdog_notify_pretimeout(wdd);

[Severity: Medium]
Can stm32_iwdg_isr() run before wdt->hw_version and wdd's driver data are
initialized in stm32_iwdg_probe()?

drivers/watchdog/stm32_iwdg.c:stm32_iwdg_probe() {
    ...
	ret = stm32_iwdg_irq_init(pdev, wdt);
	if (ret)
		return ret;

	wdt->hw_version = reg_read(wdt->regs, IWDG_VERR) & VERR_MASK;

	watchdog_set_drvdata(wdd, wdt);
    ...
}

stm32_iwdg_irq_init() calls devm_request_irq() before wdt->hw_version is
read from IWDG_VERR and before watchdog_set_drvdata() sets wdd's driver
data.  If the watchdog was already started by the bootloader and a
pretimeout interrupt is pending when the IRQ is requested, stm32_iwdg_isr()
gets NULL from watchdog_get_drvdata(wdd) and dereferences a NULL pointer,
and even with driver data set, wdt->hw_version would still be 0 so v4.0+
hardware would write to IWDG_EWCR instead of IWDG_ICR and fail to clear
the interrupt.

Initializing wdt->hw_version and calling watchdog_set_drvdata(wdd, wdt)
before stm32_iwdg_irq_init() would avoid this window.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106163429.618392-1-antonio.borneo@foss.st.com?part=3