Re: [PATCH 19/29] hwmon: (adt7x10) Rely on subsystem locking

["Nuno Sá" <noname.nuno@gmail.com>]

On Fri, 2025-10-17 at 06:02 -0700, Guenter Roeck wrote:
> Attribute access is now serialized in the hardware monitoring core,
> so locking in the driver code is no longer necessary. Drop it.
> 
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---

Reviewed-by: Nuno Sá <nuno.sa@analog.com>

>  drivers/hwmon/adt7x10.c | 27 +++++----------------------
>  1 file changed, 5 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/hwmon/adt7x10.c b/drivers/hwmon/adt7x10.c
> index 2d329391ed3f..d003ee3ebf06 100644
> --- a/drivers/hwmon/adt7x10.c
> +++ b/drivers/hwmon/adt7x10.c
> @@ -15,7 +15,6 @@
>  #include <linux/jiffies.h>
>  #include <linux/hwmon.h>
>  #include <linux/err.h>
> -#include <linux/mutex.h>
>  #include <linux/delay.h>
>  #include <linux/interrupt.h>
>  #include <linux/regmap.h>
> @@ -55,7 +54,6 @@
>  /* Each client has this additional data */
>  struct adt7x10_data {
>  	struct regmap		*regmap;
> -	struct mutex		update_lock;
>  	u8			config;
>  	u8			oldconfig;
>  	bool			valid;		/* true if temperature valid
> */
> @@ -137,17 +135,13 @@ static int adt7x10_temp_read(struct adt7x10_data *data,
> int index, long *val)
>  	unsigned int regval;
>  	int ret;
>  
> -	mutex_lock(&data->update_lock);
>  	if (index == adt7x10_temperature && !data->valid) {
>  		/* wait for valid temperature */
>  		ret = adt7x10_temp_ready(data->regmap);
> -		if (ret) {
> -			mutex_unlock(&data->update_lock);
> +		if (ret)
>  			return ret;
> -		}
>  		data->valid = true;
>  	}
> -	mutex_unlock(&data->update_lock);
>  
>  	ret = regmap_read(data->regmap, ADT7X10_REG_TEMP[index], &regval);
>  	if (ret)
> @@ -159,13 +153,8 @@ static int adt7x10_temp_read(struct adt7x10_data *data,
> int index, long *val)
>  
>  static int adt7x10_temp_write(struct adt7x10_data *data, int index, long
> temp)
>  {
> -	int ret;
> -
> -	mutex_lock(&data->update_lock);
> -	ret = regmap_write(data->regmap, ADT7X10_REG_TEMP[index],
> -			   ADT7X10_TEMP_TO_REG(temp));
> -	mutex_unlock(&data->update_lock);
> -	return ret;
> +	return regmap_write(data->regmap, ADT7X10_REG_TEMP[index],
> +			    ADT7X10_TEMP_TO_REG(temp));
>  }
>  
>  static int adt7x10_hyst_read(struct adt7x10_data *data, int index, long *val)
> @@ -197,22 +186,17 @@ static int adt7x10_hyst_write(struct adt7x10_data *data,
> long hyst)
>  	unsigned int regval;
>  	int limit, ret;
>  
> -	mutex_lock(&data->update_lock);
> -
>  	/* convert absolute hysteresis value to a 4 bit delta value */
>  	ret = regmap_read(data->regmap, ADT7X10_T_ALARM_HIGH, &regval);
>  	if (ret < 0)
> -		goto abort;
> +		return ret;
>  
>  	limit = ADT7X10_REG_TO_TEMP(data, regval);
>  
>  	hyst = clamp_val(hyst, ADT7X10_TEMP_MIN, ADT7X10_TEMP_MAX);
>  	regval = clamp_val(DIV_ROUND_CLOSEST(limit - hyst, 1000), 0,
>  			   ADT7X10_T_HYST_MASK);
> -	ret = regmap_write(data->regmap, ADT7X10_T_HYST, regval);
> -abort:
> -	mutex_unlock(&data->update_lock);
> -	return ret;
> +	return regmap_write(data->regmap, ADT7X10_T_HYST, regval);
>  }
>  
>  static int adt7x10_alarm_read(struct adt7x10_data *data, int index, long
> *val)
> @@ -344,7 +328,6 @@ int adt7x10_probe(struct device *dev, const char *name,
> int irq,
>  	data->regmap = regmap;
>  
>  	dev_set_drvdata(dev, data);
> -	mutex_init(&data->update_lock);
>  
>  	/* configure as specified */
>  	ret = regmap_read(regmap, ADT7X10_CONFIG, &config);