Re: [PATCH v3 16/16] HID: ft260: use lock guards

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] HID: ft260: mixing guard(mutex) with goto cleanup in xfer
  functions
--

commit 27addf69822143f9d183be5b9b500ff0df6b356b
Author: Michael Zaidman <michael.zaidman@gmail.com>

HID: ft260: use lock guards

This commit converts mutex_lock() and mutex_unlock() pairs in hid-ft260 to
guard(mutex) and scoped_guard(mutex, ...).

> diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> index 53321c8d70fd0..716b37cead7dc 100644
> --- a/drivers/hid/hid-ft260.c
> +++ b/drivers/hid/hid-ft260.c

[ ... ]

> @@ -1034,12 +1031,11 @@ static int ft260_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs,
>  	struct ft260_device *dev = i2c_get_adapdata(adapter);
>  	struct hid_device *hdev = dev->hdev;
>  
> -	mutex_lock(&dev->lock);
> +	guard(mutex)(&dev->lock);
>  
>  	ret = hid_hw_power(hdev, PM_HINT_FULLON);
>  	if (ret < 0) {
>  		hid_err(hdev, "failed to enter FULLON power mode: %d\n", ret);
> -		mutex_unlock(&dev->lock);
>  		return ret;
>  	}
>  
> @@ -1063,7 +1059,6 @@ static int ft260_i2c_xfer(struct i2c_adapter *adapter, struct i2c_msg *msgs,
>  	ret = num;
>  i2c_exit:
>  	hid_hw_power(hdev, PM_HINT_NORMAL);
> -	mutex_unlock(&dev->lock);
>  	return ret;
>  }
>  
> @@ -1077,12 +1072,11 @@ static int ft260_smbus_xfer(struct i2c_adapter *adapter, u16 addr, u16 flags,
>  
>  	ft260_dbg("smbus size %d\n", size);
>  
> -	mutex_lock(&dev->lock);
> +	guard(mutex)(&dev->lock);
>  
>  	ret = hid_hw_power(hdev, PM_HINT_FULLON);
>  	if (ret < 0) {
>  		hid_err(hdev, "power management error: %d\n", ret);
> -		mutex_unlock(&dev->lock);
>  		return ret;
>  	}
>  
> @@ -1190,7 +1184,6 @@ static int ft260_smbus_xfer(struct i2c_adapter *adapter, u16 addr, u16 flags,
>  
>  smbus_exit:
>  	hid_hw_power(hdev, PM_HINT_NORMAL);
> -	mutex_unlock(&dev->lock);
>  	return ret;
>  }

[Severity: Low]
This isn't a bug, but ft260_i2c_xfer() and ft260_smbus_xfer() now mix
guard(mutex) with goto-based cleanup at the i2c_exit and smbus_exit labels,
contrary to the cleanup.h convention of not mixing scope-based cleanup with
goto unwinding in the same function.

In ft260_i2c_xfer(), the i2c_exit label can be dropped by calling
hid_hw_power(hdev, PM_HINT_NORMAL) after the if/else block and returning
ret < 0 ? ret : num, while ft260_smbus_xfer() could either factor the
transfer switch into a helper or keep explicit mutex_lock() and
mutex_unlock() alongside smbus_exit.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261114230800.1344401-1-michael.zaidman@gmail.com?part=16