On 07/07/2014 12:22 AM, Jean Delvare wrote:
Hi Guenter,
On Sun, 6 Jul 2014 11:43:44 -0700, Guenter Roeck wrote:
It is customary to clamp limits instead of bailing out with an error
if a configured limit is out of the range supported by the driver.
This simplifies limit configuration, since the user will not typically
know chip and/or driver specific limits.
Agreed.
Signed-off-by: Guenter Roeck <linux@xxxxxxxxxxxx>
---
drivers/hwmon/emc2103.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/hwmon/emc2103.c b/drivers/hwmon/emc2103.c
index fd892dd..0f38294 100644
--- a/drivers/hwmon/emc2103.c
+++ b/drivers/hwmon/emc2103.c
@@ -250,9 +250,7 @@ static ssize_t set_temp_min(struct device *dev, struct device_attribute *da,
if (result < 0)
return result;
- val = DIV_ROUND_CLOSEST(val, 1000);
- if ((val < -63) || (val > 127))
- return -EINVAL;
+ val = clamp_val(DIV_ROUND_CLOSEST(val, 1000), -63, 127);
mutex_lock(&data->update_lock);
data->temp_min[nr] = val;
@@ -274,9 +272,7 @@ static ssize_t set_temp_max(struct device *dev, struct device_attribute *da,
if (result < 0)
return result;
- val = DIV_ROUND_CLOSEST(val, 1000);
- if ((val < -63) || (val > 127))
- return -EINVAL;
+ val = clamp_val(DIV_ROUND_CLOSEST(val, 1000), -63, 127);
mutex_lock(&data->update_lock);
data->temp_max[nr] = val;
I fully agree with these two.
@@ -397,8 +393,7 @@ static ssize_t set_fan_target(struct device *dev, struct device_attribute *da,
return result;
/* Datasheet states 16384 as maximum RPM target (table 3.2) */
- if ((rpm_target < 0) || (rpm_target > 16384))
- return -EINVAL;
+ rpm_target = clamp_val(rpm_target, 0, 16384);
mutex_lock(&data->update_lock);
Here however, < 0 is really invalid. There is no excuse for the user to
ask for a negative fan speed (as opposed to speeds above 16384, where
the limit is arbitrary and impossible to guess without reading the
datasheet.)
IMHO the best way to handle that is to change rpm_target to unsigned
long and set its value using kstrtoul (instead of kstrtol.)
Makes sense, I'll do that.
Reviewed-by: Jean Delvare <jdelvare@xxxxxxx>
Thanks,
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@xxxxxxxxxxxxxx
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
