* [PATCH v2] hwmon: (nct7802) Fix overflows seen when writing into limit attributes
@ 2016-12-09 20:41 Guenter Roeck
2016-12-12 10:03 ` Jean Delvare
0 siblings, 1 reply; 4+ messages in thread
From: Guenter Roeck @ 2016-12-09 20:41 UTC (permalink / raw)
To: Hardware Monitoring; +Cc: Jean Delvare, Guenter Roeck
Fix overflows seen when writing voltage and temperature limit attributes.
The value passed to DIV_ROUND_CLOSEST() needs to be clamped, and the
value parameter passed to nct7802_write_fan_min() is an unsigned long.
Also, writing values larger than 2700000 into a limit attribute results
in writing 0 into the chip's limit registers. The exact behavior when
writing this value is unspecified. For consistency, report a limit of
1350000 if the chip register reads 0. This may be wrong, and the chip
behavior should be verified with the actual chip, but it is better than
reporting a value of 0 (which, if written, results in writing a value
of 0x1fff into the chip register).
Fixes: 3434f3783580 ("hwmon: Driver for Nuvoton NCT7802Y")
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: When reading a register value of 0 from a limit register,
report it as speed of 1350000.
Avoid overflow due to different variable types passed to
nct7802_write_fan_min().
Fix low limit in store_temp (-128, not -127 as in v1).
drivers/hwmon/nct7802.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/hwmon/nct7802.c b/drivers/hwmon/nct7802.c
index 3ce33d244cc0..12b94b094c0d 100644
--- a/drivers/hwmon/nct7802.c
+++ b/drivers/hwmon/nct7802.c
@@ -259,13 +259,15 @@ static int nct7802_read_fan_min(struct nct7802_data *data, u8 reg_fan_low,
ret = 0;
else if (ret)
ret = DIV_ROUND_CLOSEST(1350000U, ret);
+ else
+ ret = 1350000U;
abort:
mutex_unlock(&data->access_lock);
return ret;
}
static int nct7802_write_fan_min(struct nct7802_data *data, u8 reg_fan_low,
- u8 reg_fan_high, unsigned int limit)
+ u8 reg_fan_high, unsigned long limit)
{
int err;
@@ -326,8 +328,8 @@ static int nct7802_write_voltage(struct nct7802_data *data, int nr, int index,
int shift = 8 - REG_VOLTAGE_LIMIT_MSB_SHIFT[index - 1][nr];
int err;
+ voltage = clamp_val(voltage, 0, 0x3ff * nct7802_vmul[nr]);
voltage = DIV_ROUND_CLOSEST(voltage, nct7802_vmul[nr]);
- voltage = clamp_val(voltage, 0, 0x3ff);
mutex_lock(&data->access_lock);
err = regmap_write(data->regmap,
@@ -402,7 +404,7 @@ static ssize_t store_temp(struct device *dev, struct device_attribute *attr,
if (err < 0)
return err;
- val = clamp_val(DIV_ROUND_CLOSEST(val, 1000), -128, 127);
+ val = DIV_ROUND_CLOSEST(clamp_val(val, -128000, 127000), 1000);
err = regmap_write(data->regmap, nr, val & 0xff);
return err ? : count;
--
2.5.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] hwmon: (nct7802) Fix overflows seen when writing into limit attributes
2016-12-09 20:41 [PATCH v2] hwmon: (nct7802) Fix overflows seen when writing into limit attributes Guenter Roeck
@ 2016-12-12 10:03 ` Jean Delvare
2016-12-12 14:14 ` Guenter Roeck
0 siblings, 1 reply; 4+ messages in thread
From: Jean Delvare @ 2016-12-12 10:03 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Hardware Monitoring
On Fri, 9 Dec 2016 12:41:02 -0800, Guenter Roeck wrote:
> Fix overflows seen when writing voltage and temperature limit attributes.
>
> The value passed to DIV_ROUND_CLOSEST() needs to be clamped, and the
> value parameter passed to nct7802_write_fan_min() is an unsigned long.
>
> Also, writing values larger than 2700000 into a limit attribute results
> in writing 0 into the chip's limit registers.
You are only talking about _fan_ limits, right?
> The exact behavior when
> writing this value is unspecified. For consistency, report a limit of
> 1350000 if the chip register reads 0. This may be wrong, and the chip
> behavior should be verified with the actual chip, but it is better than
> reporting a value of 0 (which, if written, results in writing a value
> of 0x1fff into the chip register).
This fix is good by doesn't seem to be related with the overflows?
>
> Fixes: 3434f3783580 ("hwmon: Driver for Nuvoton NCT7802Y")
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> v2: When reading a register value of 0 from a limit register,
> report it as speed of 1350000.
> Avoid overflow due to different variable types passed to
> nct7802_write_fan_min().
> Fix low limit in store_temp (-128, not -127 as in v1).
>
> drivers/hwmon/nct7802.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hwmon/nct7802.c b/drivers/hwmon/nct7802.c
> index 3ce33d244cc0..12b94b094c0d 100644
> --- a/drivers/hwmon/nct7802.c
> +++ b/drivers/hwmon/nct7802.c
> @@ -259,13 +259,15 @@ static int nct7802_read_fan_min(struct nct7802_data *data, u8 reg_fan_low,
> ret = 0;
> else if (ret)
> ret = DIV_ROUND_CLOSEST(1350000U, ret);
> + else
> + ret = 1350000U;
> abort:
> mutex_unlock(&data->access_lock);
> return ret;
> }
>
> static int nct7802_write_fan_min(struct nct7802_data *data, u8 reg_fan_low,
> - u8 reg_fan_high, unsigned int limit)
> + u8 reg_fan_high, unsigned long limit)
> {
> int err;
>
> @@ -326,8 +328,8 @@ static int nct7802_write_voltage(struct nct7802_data *data, int nr, int index,
> int shift = 8 - REG_VOLTAGE_LIMIT_MSB_SHIFT[index - 1][nr];
> int err;
>
> + voltage = clamp_val(voltage, 0, 0x3ff * nct7802_vmul[nr]);
> voltage = DIV_ROUND_CLOSEST(voltage, nct7802_vmul[nr]);
> - voltage = clamp_val(voltage, 0, 0x3ff);
>
> mutex_lock(&data->access_lock);
> err = regmap_write(data->regmap,
> @@ -402,7 +404,7 @@ static ssize_t store_temp(struct device *dev, struct device_attribute *attr,
> if (err < 0)
> return err;
>
> - val = clamp_val(DIV_ROUND_CLOSEST(val, 1000), -128, 127);
> + val = DIV_ROUND_CLOSEST(clamp_val(val, -128000, 127000), 1000);
>
> err = regmap_write(data->regmap, nr, val & 0xff);
> return err ? : count;
Reviewed-by: Jean Delvare <jdelvare@suse.de>
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] hwmon: (nct7802) Fix overflows seen when writing into limit attributes
2016-12-12 10:03 ` Jean Delvare
@ 2016-12-12 14:14 ` Guenter Roeck
2016-12-12 15:43 ` Jean Delvare
0 siblings, 1 reply; 4+ messages in thread
From: Guenter Roeck @ 2016-12-12 14:14 UTC (permalink / raw)
To: Jean Delvare; +Cc: Hardware Monitoring
Hi Jean,
On 12/12/2016 02:03 AM, Jean Delvare wrote:
> On Fri, 9 Dec 2016 12:41:02 -0800, Guenter Roeck wrote:
>> Fix overflows seen when writing voltage and temperature limit attributes.
>>
>> The value passed to DIV_ROUND_CLOSEST() needs to be clamped, and the
>> value parameter passed to nct7802_write_fan_min() is an unsigned long.
>>
>> Also, writing values larger than 2700000 into a limit attribute results
>> in writing 0 into the chip's limit registers.
>
> You are only talking about _fan_ limits, right?
>
Yes. I'll clarify.
>> The exact behavior when
>> writing this value is unspecified. For consistency, report a limit of
>> 1350000 if the chip register reads 0. This may be wrong, and the chip
>> behavior should be verified with the actual chip, but it is better than
>> reporting a value of 0 (which, if written, results in writing a value
>> of 0x1fff into the chip register).
>
> This fix is good by doesn't seem to be related with the overflows?
>
Writing a limit larger than 2700000 results in writing 0 into the register,
which is reported back as 0. 2699999 -> 1 -> 1350000, 2700000 -> 0 -> 0.
I consider that an overflow situation.
Thanks,
Guenter
>>
>> Fixes: 3434f3783580 ("hwmon: Driver for Nuvoton NCT7802Y")
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> v2: When reading a register value of 0 from a limit register,
>> report it as speed of 1350000.
>> Avoid overflow due to different variable types passed to
>> nct7802_write_fan_min().
>> Fix low limit in store_temp (-128, not -127 as in v1).
>>
>> drivers/hwmon/nct7802.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hwmon/nct7802.c b/drivers/hwmon/nct7802.c
>> index 3ce33d244cc0..12b94b094c0d 100644
>> --- a/drivers/hwmon/nct7802.c
>> +++ b/drivers/hwmon/nct7802.c
>> @@ -259,13 +259,15 @@ static int nct7802_read_fan_min(struct nct7802_data *data, u8 reg_fan_low,
>> ret = 0;
>> else if (ret)
>> ret = DIV_ROUND_CLOSEST(1350000U, ret);
>> + else
>> + ret = 1350000U;
>> abort:
>> mutex_unlock(&data->access_lock);
>> return ret;
>> }
>>
>> static int nct7802_write_fan_min(struct nct7802_data *data, u8 reg_fan_low,
>> - u8 reg_fan_high, unsigned int limit)
>> + u8 reg_fan_high, unsigned long limit)
>> {
>> int err;
>>
>> @@ -326,8 +328,8 @@ static int nct7802_write_voltage(struct nct7802_data *data, int nr, int index,
>> int shift = 8 - REG_VOLTAGE_LIMIT_MSB_SHIFT[index - 1][nr];
>> int err;
>>
>> + voltage = clamp_val(voltage, 0, 0x3ff * nct7802_vmul[nr]);
>> voltage = DIV_ROUND_CLOSEST(voltage, nct7802_vmul[nr]);
>> - voltage = clamp_val(voltage, 0, 0x3ff);
>>
>> mutex_lock(&data->access_lock);
>> err = regmap_write(data->regmap,
>> @@ -402,7 +404,7 @@ static ssize_t store_temp(struct device *dev, struct device_attribute *attr,
>> if (err < 0)
>> return err;
>>
>> - val = clamp_val(DIV_ROUND_CLOSEST(val, 1000), -128, 127);
>> + val = DIV_ROUND_CLOSEST(clamp_val(val, -128000, 127000), 1000);
>>
>> err = regmap_write(data->regmap, nr, val & 0xff);
>> return err ? : count;
>
> Reviewed-by: Jean Delvare <jdelvare@suse.de>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] hwmon: (nct7802) Fix overflows seen when writing into limit attributes
2016-12-12 14:14 ` Guenter Roeck
@ 2016-12-12 15:43 ` Jean Delvare
0 siblings, 0 replies; 4+ messages in thread
From: Jean Delvare @ 2016-12-12 15:43 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Hardware Monitoring
On Mon, 12 Dec 2016 06:14:14 -0800, Guenter Roeck wrote:
> Hi Jean,
>
> On 12/12/2016 02:03 AM, Jean Delvare wrote:
> > On Fri, 9 Dec 2016 12:41:02 -0800, Guenter Roeck wrote:
> >> Fix overflows seen when writing voltage and temperature limit attributes.
> >>
> >> The value passed to DIV_ROUND_CLOSEST() needs to be clamped, and the
> >> value parameter passed to nct7802_write_fan_min() is an unsigned long.
> >>
> >> Also, writing values larger than 2700000 into a limit attribute results
> >> in writing 0 into the chip's limit registers.
> >
> > You are only talking about _fan_ limits, right?
> >
> Yes. I'll clarify.
>
> >> The exact behavior when
> >> writing this value is unspecified. For consistency, report a limit of
> >> 1350000 if the chip register reads 0. This may be wrong, and the chip
> >> behavior should be verified with the actual chip, but it is better than
> >> reporting a value of 0 (which, if written, results in writing a value
> >> of 0x1fff into the chip register).
> >
> > This fix is good by doesn't seem to be related with the overflows?
> >
> Writing a limit larger than 2700000 results in writing 0 into the register,
> which is reported back as 0. 2699999 -> 1 -> 1350000, 2700000 -> 0 -> 0.
> I consider that an overflow situation.
OK, I understand, you are right.
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-12-12 15:43 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-09 20:41 [PATCH v2] hwmon: (nct7802) Fix overflows seen when writing into limit attributes Guenter Roeck
2016-12-12 10:03 ` Jean Delvare
2016-12-12 14:14 ` Guenter Roeck
2016-12-12 15:43 ` Jean Delvare
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.