Re: [PATCH 3/3] hwmon: lenovo-ec-sensors: Use devm_request_region for automatic cleanup

[Guenter Roeck <linux@roeck-us.net>]

On 5/13/26 19:39, Mark Pearson wrote:
> Hi Guenter,
> 
> On Wed, May 13, 2026, at 9:36 PM, Guenter Roeck wrote:
>> On 5/13/26 18:14, Kean wrote:
>>> Replace manual request_region()/release_region() with
>>> devm_request_region(). This lets the device-managed framework
>>> handle I/O region lifetime automatically and fixes:
>>>
>>> - A double release_region() when probe fails after acquiring the
>>>     I/O region: the probe error path releases it, and then
>>>     lenovo_ec_init() releases it again on the same error path.
>>>
>>> - A release-after-free in lenovo_ec_exit() where release_region()
>>>     was called after platform_device_unregister(), which has already
>>>     released the I/O region via the platform device removal path.
>>>
>>> - Missing release_region() in lenovo_ec_probe() on the DMI match
>>>     failure path, which leaked the I/O region.
>>>
>>> Remove all manual release_region() calls that are now handled
>>> automatically by the devm framework.
>>>
>>> Signed-off-by: Kean <rh_king@163.com>
>>>
>> Why this empty line ?
>>
>>> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
>>
>> How do I know that this review really happened ?
> 
> The reviewed by's are correct - I can confirm I did review these patches before Kean pushed them upstream. We did some internal review first to discuss the issues he identified.
> 
> We sometimes take this approach with the platform/x86 patches. We can do it separately next time if preferred here.
> 

Kean <rh_king@163.com> is not a full name, has no patches in the upstream
kernel or in linux-next, and otherwise seems to be a complete unknown.
How do you expect me to know that this isn't all made up by some AI ?

Anyway, when you do that, please at least ask people to run checkpatch.
I really don't want to have to deal with trivial issues such as

ERROR: trailing whitespace
#135: FILE: drivers/hwmon/lenovo-ec-sensors.c:611:
+^Iif (IS_ERR(lenovo_ec_sensors_platform_device)) $

in this patch and the first patch of the series.

Guenter

> Mark
> 
>>
>>> ---
>>>    drivers/hwmon/lenovo-ec-sensors.c | 13 +++++--------
>>>    1 file changed, 5 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/hwmon/lenovo-ec-sensors.c b/drivers/hwmon/lenovo-ec-sensors.c
>>> index b0f2a04ce679..ea74bddbad5a 100644
>>> --- a/drivers/hwmon/lenovo-ec-sensors.c
>>> +++ b/drivers/hwmon/lenovo-ec-sensors.c
>>> @@ -519,8 +519,8 @@ static int lenovo_ec_probe(struct platform_device *pdev)
>>>    	if (!ec_data)
>>>    		return -ENOMEM;
>>>    
>>> -	if (!request_region(IO_REGION_START, IO_REGION_LENGTH, "LNV-WKS")) {
>>> -		pr_err(":request fail\n");
>>> +	if (!devm_request_region(dev, IO_REGION_START, IO_REGION_LENGTH, "LNV-WKS")) {
>>> +		dev_err(dev, "Failed to request I/O region.\n");
>>>    		return -EIO;
>>>    	}
>>>    
>>> @@ -541,7 +541,6 @@ static int lenovo_ec_probe(struct platform_device *pdev)
>>>    	    (inb_p(MCHP_EMI0_EC_DATA_BYTE1) != 'C') ||
>>>    	    (inb_p(MCHP_EMI0_EC_DATA_BYTE2) != 'H') ||
>>>    	    (inb_p(MCHP_EMI0_EC_DATA_BYTE3) != 'P')) {
>>> -		release_region(IO_REGION_START, IO_REGION_LENGTH);
>>>    		return -ENODEV;
>>>    	}
>>>    
>>> @@ -579,7 +578,8 @@ static int lenovo_ec_probe(struct platform_device *pdev)
>>>    		lenovo_ec_chip_info.info = lenovo_ec_hwmon_info_p8;
>>>    		break;
>>>    	default:
>>> -		release_region(IO_REGION_START, IO_REGION_LENGTH);
>>> +		dev_err(dev, "Unsupported platform type %ld\n",
>>> +			(long)dmi_id->driver_data);
>>
>> This is not documented in the commit message and, on top of that, pointless.
>> It isn't even noise, it is just pointless (the default case can not be reached).
>>
>>>    		return -ENODEV;
>>>    	}
>>>    
>>> @@ -608,10 +608,8 @@ static int __init lenovo_ec_init(void)
>>>    		platform_create_bundle(&lenovo_ec_sensors_platform_driver,
>>>    				       lenovo_ec_probe, NULL, 0, NULL, 0);
>>>    
>>> -	if (IS_ERR(lenovo_ec_sensors_platform_device)) {
>>> -		release_region(IO_REGION_START, IO_REGION_LENGTH);
>>> +	if (IS_ERR(lenovo_ec_sensors_platform_device))
>>>    		return PTR_ERR(lenovo_ec_sensors_platform_device);
>>> -	}
>>>    
>>>    	return 0;
>>>    }
>>> @@ -619,7 +617,6 @@ module_init(lenovo_ec_init);
>>>    
>>>    static void __exit lenovo_ec_exit(void)
>>>    {
>>> -	release_region(IO_REGION_START, IO_REGION_LENGTH);
>>>    	platform_device_unregister(lenovo_ec_sensors_platform_device);
>>>    	platform_driver_unregister(&lenovo_ec_sensors_platform_driver);
>>>    }
>