Hi Guenter,
On Mon, 29 Oct 2012 17:45:44 -0700, Guenter Roeck wrote:
> Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
> ---
> v3: s/S_IRUGOWU/S_IRUGO | S_IWUSR/
> Limit support for new attributes to IT8716F and higher
> Clarify code to select target register in set_temp().
> Only clear valid flag only when writing into offset registers,
Duplicate word "only".
> not for any other temperature limit set operations.
> v2: The macro to calculate IT87_REG_TEMP_OFFSET was broken. Use array instead.
> When writing the temperature offset attribute, set the flag to enable it.
> Add documentation describing the new attributes.
>
> Documentation/hwmon/it87 | 9 +++++++
> drivers/hwmon/it87.c | 67 ++++++++++++++++++++++++++++++++++++++++++----
> 2 files changed, 71 insertions(+), 5 deletions(-)
> (...)
> --- a/drivers/hwmon/it87.c
> +++ b/drivers/hwmon/it87.c
> (...)
> @@ -312,6 +314,15 @@ static inline int has_newer_autopwm(const struct it87_data *data)
> || data->type == it8728;
> }
>
> +static inline int has_temp_offset(const struct it87_data *data)
> +{
> + return data->type == it8716
> + || data->type == it8721
> + || data->type == it8728
> + || data->type == it8782
> + || data->type == it8783;
What about it8718 and it8720?
> +}
> +
> static int adc_lsb(const struct it87_data *data, int nr)
> {
> int lsb = has_12mv_adc(data) ? 12 : 16;
> @@ -546,16 +557,37 @@ static ssize_t set_temp(struct device *dev, struct device_attribute *attr,
> int index = sattr->index;
> struct it87_data *data = dev_get_drvdata(dev);
> long val;
> + u8 reg;
>
> if (kstrtol(buf, 10, &val) < 0)
> return -EINVAL;
>
> + switch (index) {
> + case 1:
> + reg = IT87_REG_TEMP_LOW(nr);
> + break;
> + case 2:
> + reg = IT87_REG_TEMP_HIGH(nr);
> + break;
> + case 3:
> + reg = IT87_REG_TEMP_OFFSET[nr];
> + break;
> + default:
> + WARN_ONCE(true, "Driver implementation error\n");
> + return -EINVAL;
> + }
Or just use default for either case and be done with it. It's not
like the warning will ever be printed, and that's what the original
code was doing anyway.
> +
> mutex_lock(&data->update_lock);
> data->temp[nr][index] = TEMP_TO_REG(val);
> - it87_write_value(data,
> - index == 1 ? IT87_REG_TEMP_LOW(nr)
> - : IT87_REG_TEMP_HIGH(nr),
> - data->temp[nr][index]);
> + if (index == 3) {
> + u8 regval = it87_read_value(data, IT87_REG_BEEP_ENABLE);
> + if (!(regval & 0x80)) {
> + regval |= 0x80;
> + it87_write_value(data, IT87_REG_BEEP_ENABLE, regval);
> + }
> + data->valid = 0;
> + }
This could be moved to the switch above, assuming earlier locking,
saving a test.
> + it87_write_value(data, reg, data->temp[nr][index]);
> mutex_unlock(&data->update_lock);
> return count;
> }
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
