Re: [RESEND PATCH] watchdog: sbsa_gwdt: add force_enable module parameter

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

Pre-existing issues:
- [High] The driver incorrectly modifies the global module parameter `actio=
n` during probe, which corrupts the state for other watchdog instances.
--

--- Patch [1]: [RESEND PATCH] watchdog: sbsa_gwdt: add force_enable module =
parameter ---
commit 623541ce58e645c9b614e94550edb9e275d4dbbe
Author: Zexin Wang <[email protected]>

watchdog: sbsa_gwdt: add force_enable module parameter

The sbsa_gwdt driver only manages and registers the watchdog timer if it
was already enabled by a previous boot stage. This patch introduces a new
module parameter 'force_enable' to allow overriding this behavior and
actively enable it during the probe phase.

> diff --git a/drivers/watchdog/sbsa_gwdt.c b/drivers/watchdog/sbsa_gwdt.c
> index e04d42cc7774d..4b49e1aeef84f 100644
> --- a/drivers/watchdog/sbsa_gwdt.c
> +++ b/drivers/watchdog/sbsa_gwdt.c
[ ... ]
> @@ -386,14 +392,27 @@ static int sbsa_gwdt_probe(struct platform_device *=
pdev)
[ ... ]
>  	dev_info(dev, "Initialized with %ds timeout @ %u Hz, action=3D%d.%s\n",
>  		 wdd->timeout, gwdt->clk, action,

[Severity: High]
This is a pre-existing issue, but does modifying the global module parameter
action during probe cause issues for systems with multiple watchdog instanc=
es?

Earlier in sbsa_gwdt_probe(), if action=3D1 but the interrupt setup fails, =
the
driver overwrites the global module parameter:

	if (action) {
		irq =3D platform_get_irq(pdev, 0);
		if (irq < 0) {
			action =3D 0;
			dev_warn(dev, "unable to get ws0 interrupt.\n");
		} else {
			...
			if (devm_request_irq(...)) {
				action =3D 0;
				dev_warn(dev, "unable to request IRQ %d.\n", irq);
			}
		}

If a second watchdog instance fails its IRQ setup and sets action =3D 0, a =
first
successfully initialized watchdog will now read action =3D=3D 0 when pinged.

This causes sbsa_gwdt_set_timeout() to halve its programmed hardware timeou=
t:

	if (action)
		sbsa_gwdt_reg_write((u64)gwdt->clk * timeout, gwdt);
	else
		sbsa_gwdt_reg_write(((u64)gwdt->clk / 2) * timeout, gwdt);

Could this lead to premature hardware resets on systems with multiple
sbsa_gwdt instances?

> -		 status & SBSA_GWDT_WCS_EN ? " [enabled]" : "");
> +		 watchdog_hw_running(wdd) ? " [enabled]" : "");
> =20
>  	return 0;
>  }

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260803071823.2775=
[email protected]?part=3D1