| Message ID | 20240716230050.2049534-6-linux@roeck-us.net (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | hwmon: Use multi-byte regmap operations | expand |
On Tue, Jul 16, 2024 at 04:00:49PM -0700, Guenter Roeck wrote: > @@ -88,25 +88,16 @@ struct max6639_data { > > static int max6639_temp_read_input(struct device *dev, int channel, long *temp) > { > + u32 regs[2] = {MAX6639_REG_TEMP_EXT(channel), MAX6639_REG_TEMP(channel) }; ^ ^ To be consistent, either drop the space or insert an extra space otherwise. > @@ -290,8 +281,10 @@ static umode_t max6639_fan_is_visible(const void *_data, u32 attr, int channel) > static int max6639_read_pwm(struct device *dev, u32 attr, int channel, > long *pwm_val) > { > + u32 regs[2] = { MAX6639_REG_FAN_CONFIG3(channel), MAX6639_REG_GCONFIG }; ^ ^ Same here. > @@ -303,26 +296,13 @@ static int max6639_read_pwm(struct device *dev, u32 attr, int channel, > *pwm_val = val * 255 / 120; > return 0; > case hwmon_pwm_freq: > - mutex_lock(&data->update_lock); > - res = regmap_read(data->regmap, MAX6639_REG_FAN_CONFIG3(channel), &val); > - if (res < 0) { > - mutex_unlock(&data->update_lock); > + res = regmap_multi_reg_read(data->regmap, regs, regvals, 2); > + if (res < 0) > return res; > - } > - i = val & MAX6639_FAN_CONFIG3_FREQ_MASK; > - > - res = regmap_read(data->regmap, MAX6639_REG_GCONFIG, &val); > - if (res < 0) { > - mutex_unlock(&data->update_lock); > - return res; > - } > - > - if (val & MAX6639_GCONFIG_PWM_FREQ_HI) > + i = regvals[0] & MAX6639_FAN_CONFIG3_FREQ_MASK; > + if (regvals[1] & MAX6639_GCONFIG_PWM_FREQ_HI) > i |= 0x4; > - i &= 0x7; Just a note: this line can be safely dropped.
On 7/17/24 06:36, Tzung-Bi Shih wrote: > On Tue, Jul 16, 2024 at 04:00:49PM -0700, Guenter Roeck wrote: >> @@ -88,25 +88,16 @@ struct max6639_data { >> >> static int max6639_temp_read_input(struct device *dev, int channel, long *temp) >> { >> + u32 regs[2] = {MAX6639_REG_TEMP_EXT(channel), MAX6639_REG_TEMP(channel) }; > ^ ^ > To be consistent, either drop the space or insert an extra space otherwise. > Good point. I added the space. Thanks, Guenter
On Wed, Jul 17, 2024 at 10:14:49AM -0700, Guenter Roeck wrote: > On 7/17/24 06:36, Tzung-Bi Shih wrote: > > On Tue, Jul 16, 2024 at 04:00:49PM -0700, Guenter Roeck wrote: > > > @@ -88,25 +88,16 @@ struct max6639_data { > > > static int max6639_temp_read_input(struct device *dev, int channel, long *temp) > > > { > > > + u32 regs[2] = {MAX6639_REG_TEMP_EXT(channel), MAX6639_REG_TEMP(channel) }; > > ^ ^ > > To be consistent, either drop the space or insert an extra space otherwise. > > > Good point. I added the space. With that, Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org>
On 7/17/24 18:28, Tzung-Bi Shih wrote: > On Wed, Jul 17, 2024 at 10:14:49AM -0700, Guenter Roeck wrote: >> On 7/17/24 06:36, Tzung-Bi Shih wrote: >>> On Tue, Jul 16, 2024 at 04:00:49PM -0700, Guenter Roeck wrote: >>>> @@ -88,25 +88,16 @@ struct max6639_data { >>>> static int max6639_temp_read_input(struct device *dev, int channel, long *temp) >>>> { >>>> + u32 regs[2] = {MAX6639_REG_TEMP_EXT(channel), MAX6639_REG_TEMP(channel) }; >>> ^ ^ >>> To be consistent, either drop the space or insert an extra space otherwise. >>> >> Good point. I added the space. > > With that, > Reviewed-by: Tzung-Bi Shih <tzungbi@kernel.org> Thanks a lot for the reviews ! Guenter
diff --git a/drivers/hwmon/max6639.c b/drivers/hwmon/max6639.c index f54720d3d2ce..e877a5520416 100644 --- a/drivers/hwmon/max6639.c +++ b/drivers/hwmon/max6639.c @@ -88,25 +88,16 @@ struct max6639_data { static int max6639_temp_read_input(struct device *dev, int channel, long *temp) { + u32 regs[2] = {MAX6639_REG_TEMP_EXT(channel), MAX6639_REG_TEMP(channel) }; struct max6639_data *data = dev_get_drvdata(dev); - unsigned int val; + u8 regvals[2]; int res; - /* - * Lock isn't needed as MAX6639_REG_TEMP wpnt change for at least 250ms after reading - * MAX6639_REG_TEMP_EXT - */ - res = regmap_read(data->regmap, MAX6639_REG_TEMP_EXT(channel), &val); + res = regmap_multi_reg_read(data->regmap, regs, regvals, 2); if (res < 0) return res; - *temp = val >> 5; - res = regmap_read(data->regmap, MAX6639_REG_TEMP(channel), &val); - if (res < 0) - return res; - - *temp |= val << 3; - *temp *= 125; + *temp = ((regvals[0] >> 5) | (regvals[1] << 3)) * 125; return 0; } @@ -290,8 +281,10 @@ static umode_t max6639_fan_is_visible(const void *_data, u32 attr, int channel) static int max6639_read_pwm(struct device *dev, u32 attr, int channel, long *pwm_val) { + u32 regs[2] = { MAX6639_REG_FAN_CONFIG3(channel), MAX6639_REG_GCONFIG }; struct max6639_data *data = dev_get_drvdata(dev); unsigned int val; + u8 regvals[2]; int res; u8 i; @@ -303,26 +296,13 @@ static int max6639_read_pwm(struct device *dev, u32 attr, int channel, *pwm_val = val * 255 / 120; return 0; case hwmon_pwm_freq: - mutex_lock(&data->update_lock); - res = regmap_read(data->regmap, MAX6639_REG_FAN_CONFIG3(channel), &val); - if (res < 0) { - mutex_unlock(&data->update_lock); + res = regmap_multi_reg_read(data->regmap, regs, regvals, 2); + if (res < 0) return res; - } - i = val & MAX6639_FAN_CONFIG3_FREQ_MASK; - - res = regmap_read(data->regmap, MAX6639_REG_GCONFIG, &val); - if (res < 0) { - mutex_unlock(&data->update_lock); - return res; - } - - if (val & MAX6639_GCONFIG_PWM_FREQ_HI) + i = regvals[0] & MAX6639_FAN_CONFIG3_FREQ_MASK; + if (regvals[1] & MAX6639_GCONFIG_PWM_FREQ_HI) i |= 0x4; - i &= 0x7; *pwm_val = freq_table[i]; - - mutex_unlock(&data->update_lock); return 0; default: return -EOPNOTSUPP;
Use multi-byte regmap operations where possible to reduce code size and the need for mutex protection. No functional changes. Signed-off-by: Guenter Roeck <linux@roeck-us.net> --- drivers/hwmon/max6639.c | 40 ++++++++++------------------------------ 1 file changed, 10 insertions(+), 30 deletions(-)