Re: [PATCH v2] watchdog: lenovo_se30g2_se60_wdt: Watchdog for Lenovo SE30G2 and SE60

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incorrect error cleanup for a registered platform device leaves a zombified sysfs device.
--

commit 728400a02a11a5d2e004ae6d02e2731670483271
Author: Mark Pearson <[email protected]>

watchdog: lenovo_se30g2_se60_wdt: Watchdog for Lenovo SE30G2 and SE60

This commit adds the watchdog driver implementation for Lenovo SE30G2 and
SE60 platforms.

> diff --git a/drivers/watchdog/lenovo_se30g2_se60_wdt.c b/drivers/watchdog/lenovo_se30g2_se60_wdt.c
> --- /dev/null
> +++ b/drivers/watchdog/lenovo_se30g2_se60_wdt.c
[ ... ]
> +static int se_30g2_60_create_device(const struct dmi_system_id *id)
> +{
> +	int err;
> +
> +	se_30g2_60_pdev = platform_device_alloc("lenovo-se30g2-se60-wdt", -1);
> +	if (!se_30g2_60_pdev)
> +		return -ENOMEM;
> +
> +	err = platform_device_add(se_30g2_60_pdev);
> +	if (err) {
> +		platform_device_put(se_30g2_60_pdev);
> +		se_30g2_60_pdev = NULL;
> +	}
> +
> +	return err;
> +}
[ ... ]
> +static int __init se_30g2_60_wdt_init(void)
> +{
> +	int err;
> +
> +	if (!dmi_check_system(se_30g2_60))
> +		return -ENODEV;
> +
> +	err = platform_driver_register(&se_30g2_60_wdt_driver);
> +	if (err && se_30g2_60_pdev) {
> +		platform_device_put(se_30g2_60_pdev);

[Severity: Medium]
Since the device was successfully added to the system bus via
platform_device_add() in the DMI callback earlier, will calling
platform_device_put() here just drop the allocation reference without
removing the device from the sysfs hierarchy?

Should this error path use platform_device_unregister() instead to avoid
leaving a zombie device on the bus that could cause collisions on a later
module load?

> +		se_30g2_60_pdev = NULL;
> +	}
> +
> +	return err;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1