Re: [PATCH 4/6] hwmon: (tmp464) Use multi-byte regmap operations

[Tzung-Bi Shih <tzungbi@kernel.org>]

On Tue, Jul 16, 2024 at 04:00:48PM -0700, Guenter Roeck wrote:
> @@ -147,11 +147,11 @@ static int tmp464_temp_read(struct device *dev, u32 attr, int channel, long *val
>  {
[...]
> -	mutex_lock(&data->update_lock);
> -
>  	switch (attr) {
>  	case hwmon_temp_max_alarm:
>  		err = regmap_read(regmap, TMP464_THERM_STATUS_REG, &regval);
> @@ -172,26 +172,27 @@ static int tmp464_temp_read(struct device *dev, u32 attr, int channel, long *val
>  		 * complete. That means we have to cache the value internally
>  		 * for one measurement cycle and report the cached value.
>  		 */
> +		mutex_lock(&data->update_lock);
>  		if (!data->valid || time_after(jiffies, data->last_updated +
>  					       msecs_to_jiffies(data->update_interval))) {
>  			err = regmap_read(regmap, TMP464_REMOTE_OPEN_REG, &regval);
>  			if (err < 0)
> -				break;
> +				goto unlock;
>  			data->open_reg = regval;
>  			data->last_updated = jiffies;
>  			data->valid = true;
>  		}
>  		*val = !!(data->open_reg & BIT(channel + 7));
> +unlock:
> +		mutex_unlock(&data->update_lock);
>  		break;

I think the function can entirely drop the mutex.  Only [1] needs it.

[1]: https://elixir.bootlin.com/linux/v6.10/source/drivers/hwmon/tmp464.c#L313