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, ®val); > @@ -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, ®val); > 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