Re: [PATCH 07/11] watchdog: Add ARM TWD (MPCore private) watchdog driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] watchdog: arm_twd_wdt: premature reset due to ignored prescaler update
--

commit f10545beccf6bb20bb2cd3eed188125eef8d1afc
Author: Fil Dunsky <filipp.dunsky@gmail.com>

watchdog: Add ARM TWD (MPCore private) watchdog driver

This commit introduces a watchdog driver for ARM Cortex-A5/A9 and ARM11 MPCore
systems. It programs, pings, and stops the watchdogs of all online CPUs, and
takes over the hardware if it is found already running from the bootloader.

> diff --git a/drivers/watchdog/arm_twd_wdt.c b/drivers/watchdog/arm_twd_wdt.c
> new file mode 100644
> index 0000000000000..1931e9a39655c
> --- /dev/null
> +++ b/drivers/watchdog/arm_twd_wdt.c

[ ... ]

> +static u32 twd_wdt_ticks(struct twd_wdt *wdt, unsigned int secs)
> +{
> +	u64 ticks = div_u64((u64)wdt->rate * secs, TWD_WDT_PRESCALE + 1);

[Severity: High]
Does this calculation always match the actual hardware prescaler?

The timeout calculation strictly assumes the prescaler was successfully
updated to TWD_WDT_PRESCALE. If the prescaler update fails, this causes
a huge mismatch in the expected timeout.

> +
> +	return min_t(u64, ticks, U32_MAX);
> +}

[ ... ]

> +/* The following run on the CPU whose watchdog they access */
> +static void twd_wdt_cpu_start(void *data)
> +{
> +	struct twd_wdt *wdt = data;
> +
> +	/*
> +	 * Load the counter before enabling, it may still be zero. A watchdog
> +	 * that is already running is reloaded and keeps running, the mode bit
> +	 * cannot be cleared by writing the control register.
> +	 */
> +	writel(twd_wdt_ticks(wdt, wdt->wdd.timeout), wdt->base + TWD_WDT_LOAD);
> +	writel(FIELD_PREP(TWD_WDT_CTRL_PRESCALER, TWD_WDT_PRESCALE) |
> +	       TWD_WDT_CTRL_WD_MODE | TWD_WDT_CTRL_ENABLE,
> +	       wdt->base + TWD_WDT_CONTROL);

[Severity: High]
Can this write fail to update the prescaler if the watchdog is already
running?

According to the ARM Cortex-A9 MPCore TRM, writes to the prescaler field in
the Control Register are ignored if the timer is already in watchdog mode.
Since the driver takes over a running watchdog without stopping it first via
the disable keys, the hardware retains the bootloader's prescaler.

If the bootloader set a smaller prescaler (e.g., 0), the timer counts down
much faster than calculated above, leading to a premature system reset
shortly after the driver probes.

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-b4-cs75xx-s2-v1-0-8a7641b7b79c@gmail.com?part=7