Re: [PATCH 22/29] hwmon: (ltc4282) 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/ltc4282.c | 32 ++++----------------------------
>  1 file changed, 4 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/hwmon/ltc4282.c b/drivers/hwmon/ltc4282.c
> index 44102879694a..b9cad89f2cd9 100644
> --- a/drivers/hwmon/ltc4282.c
> +++ b/drivers/hwmon/ltc4282.c
> @@ -17,7 +17,6 @@
>  #include <linux/minmax.h>
>  #include <linux/module.h>
>  #include <linux/mod_devicetable.h>
> -#include <linux/mutex.h>
>  #include <linux/regmap.h>
>  #include <linux/property.h>
>  #include <linux/string.h>
> @@ -131,8 +130,6 @@ struct ltc4282_cache {
>  
>  struct ltc4282_state {
>  	struct regmap *map;
> -	/* Protect against multiple accesses to the device registers */
> -	struct mutex lock;
>  	struct clk_hw clk_hw;
>  	/*
>  	 * Used to cache values for VDD/VSOURCE depending which will be used
> @@ -281,14 +278,12 @@ static int __ltc4282_read_alarm(struct ltc4282_state
> *st, u32 reg, u32 mask,
>  static int ltc4282_read_alarm(struct ltc4282_state *st, u32 reg, u32 mask,
>  			      long *val)
>  {
> -	guard(mutex)(&st->lock);
>  	return __ltc4282_read_alarm(st, reg, mask, val);
>  }
>  
>  static int ltc4282_vdd_source_read_in(struct ltc4282_state *st, u32 channel,
>  				      long *val)
>  {
> -	guard(mutex)(&st->lock);
>  	if (!st->in0_1_cache[channel].en)
>  		return -ENODATA;
>  
> @@ -300,7 +295,6 @@ static int ltc4282_vdd_source_read_hist(struct
> ltc4282_state *st, u32 reg,
>  {
>  	int ret;
>  
> -	guard(mutex)(&st->lock);
>  	if (!st->in0_1_cache[channel].en) {
>  		*val = *cached;
>  		return 0;
> @@ -317,7 +311,6 @@ static int ltc4282_vdd_source_read_hist(struct
> ltc4282_state *st, u32 reg,
>  static int ltc4282_vdd_source_read_lim(struct ltc4282_state *st, u32 reg,
>  				       u32 channel, u32 *cached, long *val)
>  {
> -	guard(mutex)(&st->lock);
>  	if (!st->in0_1_cache[channel].en)
>  		return ltc4282_read_voltage_byte_cached(st, reg, st->vfs_out,
>  							val, cached);
> @@ -328,7 +321,6 @@ static int ltc4282_vdd_source_read_lim(struct
> ltc4282_state *st, u32 reg,
>  static int ltc4282_vdd_source_read_alm(struct ltc4282_state *st, u32 mask,
>  				       u32 channel, long *val)
>  {
> -	guard(mutex)(&st->lock);
>  	if (!st->in0_1_cache[channel].en) {
>  		/*
>  		 * Do this otherwise alarms can get confused because we clear
> @@ -412,9 +404,7 @@ static int ltc4282_read_in(struct ltc4282_state *st, u32
> attr, long *val,
>  						   channel,
>  						   &st-
> >in0_1_cache[channel].in_min_raw, val);
>  	case hwmon_in_enable:
> -		scoped_guard(mutex, &st->lock) {
> -			*val = st->in0_1_cache[channel].en;
> -		}
> +		*val = st->in0_1_cache[channel].en;
>  		return 0;
>  	case hwmon_in_fault:
>  		/*
> @@ -612,15 +602,11 @@ static int ltc4282_read(struct device *dev, enum
> hwmon_sensor_types type,
>  	case hwmon_power:
>  		return ltc4282_read_power(st, attr, val);
>  	case hwmon_energy:
> -		scoped_guard(mutex, &st->lock) {
> -			*val = st->energy_en;
> -		}
> +		*val = st->energy_en;
>  		return 0;
>  	case hwmon_energy64:
> -		scoped_guard(mutex, &st->lock) {
> -			if (st->energy_en)
> -				return ltc4282_read_energy(st, (s64 *)val);
> -		}
> +		if (st->energy_en)
> +			return ltc4282_read_energy(st, (s64 *)val);
>  		return -ENODATA;
>  	default:
>  		return -EOPNOTSUPP;
> @@ -688,7 +674,6 @@ static int __ltc4282_in_write_history(const struct
> ltc4282_state *st, u32 reg,
>  static int ltc4282_in_write_history(struct ltc4282_state *st, u32 reg,
>  				    long lowest, long highest, u32 fs)
>  {
> -	guard(mutex)(&st->lock);
>  	return __ltc4282_in_write_history(st, reg, lowest, highest, fs);
>  }
>  
> @@ -696,8 +681,6 @@ static int ltc4282_power_reset_hist(struct ltc4282_state
> *st)
>  {
>  	int ret;
>  
> -	guard(mutex)(&st->lock);
> -
>  	ret = ltc4282_write_power_word(st, LTC4282_POWER_LOWEST,
>  				       st->power_max);
>  	if (ret)
> @@ -803,7 +786,6 @@ static int ltc4282_vdd_source_write_lim(struct
> ltc4282_state *st, u32 reg,
>  {
>  	int ret;
>  
> -	guard(mutex)(&st->lock);
>  	if (st->in0_1_cache[channel].en)
>  		ret = ltc4282_write_voltage_byte(st, reg, st->vfs_out, val);
>  	else
> @@ -821,7 +803,6 @@ static int ltc4282_vdd_source_reset_hist(struct
> ltc4282_state *st, int channel)
>  	if (channel == LTC4282_CHAN_VDD)
>  		lowest = st->vdd;
>  
> -	guard(mutex)(&st->lock);
>  	if (st->in0_1_cache[channel].en) {
>  		ret = __ltc4282_in_write_history(st, LTC4282_VSOURCE_LOWEST,
>  						 lowest, 0, st->vfs_out);
> @@ -861,7 +842,6 @@ static int ltc4282_vdd_source_enable(struct ltc4282_state
> *st, int channel,
>  	int ret, other_chan = ~channel & 0x1;
>  	u8 __val = val;
>  
> -	guard(mutex)(&st->lock);
>  	if (st->in0_1_cache[channel].en == !!val)
>  		return 0;
>  
> @@ -938,8 +918,6 @@ static int ltc4282_curr_reset_hist(struct ltc4282_state
> *st)
>  {
>  	int ret;
>  
> -	guard(mutex)(&st->lock);
> -
>  	ret = __ltc4282_in_write_history(st, LTC4282_VSENSE_LOWEST,
>  					 st->vsense_max, 0, 40 * MILLI);
>  	if (ret)
> @@ -974,7 +952,6 @@ static int ltc4282_energy_enable_set(struct ltc4282_state
> *st, long val)
>  {
>  	int ret;
>  
> -	guard(mutex)(&st->lock);
>  	/* setting the bit halts the meter */
>  	ret = regmap_update_bits(st->map, LTC4282_ADC_CTRL,
>  				 LTC4282_METER_HALT_MASK,
> @@ -1699,7 +1676,6 @@ static int ltc4282_probe(struct i2c_client *i2c)
>  	if (ret)
>  		return ret;
>  
> -	mutex_init(&st->lock);
>  	hwmon = devm_hwmon_device_register_with_info(dev, "ltc4282", st,
>  						     &ltc4282_chip_info,
> NULL);
>  	if (IS_ERR(hwmon))