Re: [PATCH v2 2/8] watchdog: w83627hf_wdt: Convert to platform driver model

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

New issues:
- [High] Potential NULL pointer dereference in `wdt_probe()` when the devic=
e is bound via sysfs `driver_override`.
- [High] Unbinding and rebinding the platform driver leaks watchdog status =
state, permanently breaking the watchdog.
- [Medium] The watchdog device's parent pointer is not initialized before r=
egistration, breaking the sysfs device hierarchy.

Pre-existing issues:
- [High] Hardware watchdog is left running if `devm_watchdog_register_devic=
e()` fails, guaranteeing an unintended system reboot.
--

--- Patch [2]: [PATCH v2 2/8] watchdog: w83627hf_wdt: Convert to platform d=
river model ---
commit d9d8da674e5ddb501516445617ff593614dca863
Author: Paul Louvel <[email protected]>
watchdog: w83627hf_wdt: Convert to platform driver model

This commit converts the w83627hf_wdt driver to the Linux driver model by
using a platform driver and platform device instead of the legacy module
init registration.

> diff --git a/drivers/watchdog/w83627hf_wdt.c b/drivers/watchdog/w83627hf_=
wdt.c
> index 1529a4e168209..206ab91fdb2a9 100644
> --- a/drivers/watchdog/w83627hf_wdt.c
> +++ b/drivers/watchdog/w83627hf_wdt.c
[ ... ]
> @@ -464,6 +465,36 @@ static int wdt_find(int addr)
>  	return ret;
>  }
> =20
> +static int wdt_probe(struct platform_device *pdev)
> +{
> +	const struct platform_device_id *id =3D platform_get_device_id(pdev);
> +	enum chips chip =3D id->driver_data;

[Severity: High]
Can platform_get_device_id() return NULL here if the device is bound via
sysfs driver_override? If the driver core's platform_match() succeeds by
matching the driver name and bypasses the id_table, id would be NULL,
causing a NULL pointer dereference when accessing id->driver_data.

> +	struct device *dev =3D &pdev->dev;
> +	int ret;
> +
> +	dev_info(dev, "WDT driver initialising\n");
> +
> +	snprintf(wdt_info.identity, sizeof(wdt_info.identity), "%s Watchdog",
> +		 id->name);
> +
> +	watchdog_init_timeout(&wdt_dev, timeout, NULL);
> +	watchdog_set_nowayout(&wdt_dev, nowayout);
> +	watchdog_stop_on_reboot(&wdt_dev);
> +
> +	ret =3D w83627hf_init(&wdt_dev, chip);
> +	if (ret)
> +		return dev_err_probe(dev, ret, "failed to initialize watchdog\n");
> +
> +	ret =3D devm_watchdog_register_device(&pdev->dev, &wdt_dev);

[Severity: Medium]
Should wdt_dev.parent be set to dev before calling
devm_watchdog_register_device()? Without this assignment, the watchdog
device is parented to the virtual sysfs root rather than the physical
platform device, which breaks udev hardware association rules.

[Severity: High]
Does unbinding and rebinding this platform driver leak watchdog status
state? Since wdt_dev is statically allocated and not zeroed during probe,
any state set during a previous bind (like the WDOG_ACTIVE bit) is
preserved. If an administrator unbinds and rebinds the driver, the core
will see WDOG_ACTIVE is already set and return success without calling
the hardware start routine, silently breaking the watchdog.

> +	if (ret)
> +		return ret;

[Severity: High]
This is a pre-existing issue, but does a failure in
devm_watchdog_register_device() leave the hardware watchdog running? If
the BIOS started the hardware watchdog before boot, w83627hf_init() resets
its timeout and leaves it ticking. Because probe returns an error and the
device node is never created, user-space has no way to ping the watchdog,
guaranteeing an unintended system reboot.

> +
> +	dev_info(dev, "initialized. timeout=3D%d sec (nowayout=3D%d)\n",
> +		 wdt_dev.timeout, nowayout);
> +
> +	return ret;
> +}

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