Re: [PATCH] iio: temperature: mlx90614: use guard(mutex) for EEPROM access locking

[Jonathan Cameron <jic23@kernel.org>]

On Sat, 18 Apr 2026 17:51:36 -0300
Luiz Mugnaini <luizmugnaini@usp.br> wrote:

> From: Luiz Mugnaini <luizmugnaini@usp.br>
> 
> Replace mutex_lock()/mutex_unlock() pairs with guard(mutex) from
> cleanup.h for cleaner and safer mutex handling. The lock protects
> EEPROM access across five call sites in mlx90614_read_raw(),
> mlx90614_write_raw(), and mlx90614_sleep().
> 
> In all cases, the code between mutex_unlock() and the end of scope
> is either trivial computation, a return statement, or a call to
> mlx90614_power_put() (which only schedules a deferred autosuspend
> via pm_runtime_put_autosuspend()). The slightly extended lock scope
> is negligible compared to the existing hold times, which include
> EEPROM write cycles with 40ms sleeps.
> 
> Signed-off-by: Luiz Mugnaini <luizmugnaini@usp.br>
Hi Luiz,

When making this sort of modernization change I'd suggest taking
a look at the feedback and final form of other similar patches.
Pretty much everyone gets the same feedback on these and you'd have
saved everyone time if you'd realized it applies here as well.

I'd also consider if having a locked reader helper might be
worth doing here as well

Something like

static xxx mlx90614_read_word_data_locked(xxxx)
{
	guard(mutex)(&data->lock);
	return i2c_smbus_read_word_data();
}
> ---
>  drivers/iio/temperature/mlx90614.c | 16 ++++++----------
>  1 file changed, 6 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/iio/temperature/mlx90614.c b/drivers/iio/temperature/mlx90614.c
> index 1ad21b73e..adf100756 100644
> --- a/drivers/iio/temperature/mlx90614.c
> +++ b/drivers/iio/temperature/mlx90614.c
> @@ -23,6 +23,7 @@
>   */
>  
>  #include <linux/bitfield.h>
> +#include <linux/cleanup.h>
>  #include <linux/delay.h>
>  #include <linux/err.h>
>  #include <linux/gpio/consumer.h>
> @@ -296,10 +297,9 @@ static int mlx90614_read_raw(struct iio_dev *indio_dev,
>  		if (ret < 0)
>  			return ret;
>  
> -		mutex_lock(&data->lock);
> +		guard(mutex)(&data->lock);

What is the scope of this guard?  Hint, that gets 'interesting' when in a switch
case statement and there is a standard solution for that...

>  		ret = i2c_smbus_read_word_data(data->client,
>  					       chip_info->op_eeprom_emissivity);
> -		mutex_unlock(&data->lock);
>  		mlx90614_power_put(data);
>  
>  		if (ret < 0)
> @@ -319,10 +319,9 @@ static int mlx90614_read_raw(struct iio_dev *indio_dev,
>  		if (ret < 0)
>  			return ret;
>  
> -		mutex_lock(&data->lock);
> +		guard(mutex)(&data->lock);
Same applies here and probably a load more cases below.

>  		ret = i2c_smbus_read_word_data(data->client,
>  					       chip_info->op_eeprom_config1);
> -		mutex_unlock(&data->lock);
>  		mlx90614_power_put(data);
It may make sense to take a closer look at whether we can use cleanup magic
to handle the power management as well though that would need more substantial
refactors so wouldn't belong in this patch.


>  
>  		if (ret < 0)
> @@ -358,10 +357,9 @@ static int mlx90614_write_raw(struct iio_dev *indio_dev,
>  		if (ret < 0)
>  			return ret;
>  
> -		mutex_lock(&data->lock);
> +		guard(mutex)(&data->lock);
>  		ret = mlx90614_write_word(data->client,
>  					  chip_info->op_eeprom_emissivity, val);
> -		mutex_unlock(&data->lock);
>  		mlx90614_power_put(data);
>  
>  		return ret;
> @@ -373,10 +371,9 @@ static int mlx90614_write_raw(struct iio_dev *indio_dev,
>  		if (ret < 0)
>  			return ret;
>  
> -		mutex_lock(&data->lock);
> +		guard(mutex)(&data->lock);
>  		ret = mlx90614_iir_search(data->client,
>  					  val * 100 + val2 / 10000);
> -		mutex_unlock(&data->lock);
>  		mlx90614_power_put(data);
>  
>  		return ret;
> @@ -476,12 +473,11 @@ static int mlx90614_sleep(struct mlx90614_data *data)
>  
>  	dev_dbg(&data->client->dev, "Requesting sleep");
>  
> -	mutex_lock(&data->lock);
> +	guard(mutex)(&data->lock);
>  	ret = i2c_smbus_xfer(data->client->adapter, data->client->addr,
>  			     data->client->flags | I2C_CLIENT_PEC,
>  			     I2C_SMBUS_WRITE, chip_info->op_sleep,
>  			     I2C_SMBUS_BYTE, NULL);
> -	mutex_unlock(&data->lock);
>  
>  	return ret;
	reutrn i2c_smbus_xfer();

>  }