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

On 6/25/26 10:52, [email protected] wrote:
> 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?
> 
I don't understand this code either. The parallel platform_device_put()
on error in se_30g2_60_create_device() seems risky. Each of the Lenovo
drivers uses a different mechanism to register the driver and the device.
Why ? That just makes it more difficult to review the code.

Guenter