All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: lm-sensors@vger.kernel.org
Subject: Re: [lm-sensors] PATCH: fix various sysfs callback function issues
Date: Sun, 14 Dec 2008 18:40:37 +0000	[thread overview]
Message-ID: <49455325.5030707@redhat.com> (raw)
In-Reply-To: <4941926A.600@redhat.com>

[-- Attachment #1: Type: text/plain, Size: 811 bytes --]



Jean Delvare wrote:
> Hi Hans,
> 
> On Sun, 14 Dec 2008 15:32:04 +0100, Hans de Goede wrote:
>> Attached is a new version with all mentioned issues fixed.
> 
> Doesn't build:
> 
> drivers/hwmon/f71882fg.c: In function ‘store_pwm_auto_point_temp_hyst’:
> drivers/hwmon/f71882fg.c:1361: error: too many arguments to function ‘f71882fg_read8’
> drivers/hwmon/f71882fg.c:1364: error: too many arguments to function ‘f71882fg_read8’
> 

oops. I guess I forgot to test compile that one particular in between patches
state, the next patch in the series fixes this compile error (by completely 
nuking the affected lines.

Anyways attached is a fixed version, as well as an updated version of the 09 
patch, so that it applies on top of the fixed version.

Regards,

Hans





[-- Attachment #2: hwmon-f71882fg-08-fix-store-functions-v3.patch --]
[-- Type: text/plain, Size: 11698 bytes --]

While working on adding f8000 support I noticed that various of the
store sysfs functions (and a few of the show also) had issues.

This patch fixes the following issues in these functions:
* store: storing the result of strto[u]l in an int, resulting in a possible
  overflow before boundary checking
* store: use of f71882fg_update_device(), we don't want to read the whole
  device in store functions, just the registers we need
* store: use of cached register values instead of reading the needed regs
  in the store function, including cases where f71882fg_update_device() was
  not used, this could cause real isues
* show: shown value is a calculation of 2 or more cached register reads,
  without locking the data struct.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
--- linux/drivers/hwmon/f71882fg.c.07-applied	2008-12-14 14:06:02.000000000 +0100
+++ linux/drivers/hwmon/f71882fg.c	2008-12-14 14:19:47.000000000 +0100
@@ -835,6 +835,7 @@
 	val = fan_to_reg(val);
 
 	mutex_lock(&data->update_lock);
+	data->pwm_enable = f71882fg_read8(data, F71882FG_REG_PWM_ENABLE);
 	if (data->pwm_enable & (1 << (2 * nr)))
 		/* PWM mode */
 		count = -EINVAL;
@@ -865,9 +866,10 @@
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
-	int val = simple_strtoul(buf, NULL, 10);
+	unsigned long val = simple_strtoul(buf, NULL, 10);
 
 	mutex_lock(&data->update_lock);
+	data->fan_beep = f71882fg_read8(data, F71882FG_REG_FAN_BEEP);
 	if (val)
 		data->fan_beep |= 1 << nr;
 	else
@@ -912,10 +914,8 @@
 	*devattr, const char *buf, size_t count)
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
-	int val = simple_strtoul(buf, NULL, 10) / 8;
-
-	if (val > 255)
-		val = 255;
+	long val = simple_strtol(buf, NULL, 10) / 8;
+	val = SENSORS_LIMIT(val, 0, 255);
 
 	mutex_lock(&data->update_lock);
 	f71882fg_write8(data, F71882FG_REG_IN1_HIGH, val);
@@ -942,9 +942,10 @@
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
-	int val = simple_strtoul(buf, NULL, 10);
+	unsigned long val = simple_strtoul(buf, NULL, 10);
 
 	mutex_lock(&data->update_lock);
+	data->in_beep = f71882fg_read8(data, F71882FG_REG_IN_BEEP);
 	if (val)
 		data->in_beep |= 1 << nr;
 	else
@@ -991,10 +992,8 @@
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
-	int val = simple_strtoul(buf, NULL, 10) / 1000;
-
-	if (val > 255)
-		val = 255;
+	long val = simple_strtol(buf, NULL, 10) / 1000;
+	val = SENSORS_LIMIT(val, 0, 255);
 
 	mutex_lock(&data->update_lock);
 	f71882fg_write8(data, F71882FG_REG_TEMP_HIGH(nr), val);
@@ -1009,9 +1008,13 @@
 {
 	struct f71882fg_data *data = f71882fg_update_device(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
+	int temp_max_hyst;
 
-	return sprintf(buf, "%d\n",
-		(data->temp_high[nr] - data->temp_hyst[nr]) * 1000);
+	mutex_lock(&data->update_lock);
+	temp_max_hyst = (data->temp_high[nr] - data->temp_hyst[nr]) * 1000;
+	mutex_unlock(&data->update_lock);
+
+	return sprintf(buf, "%d\n", temp_max_hyst);
 }
 
 static ssize_t store_temp_max_hyst(struct device *dev, struct device_attribute
@@ -1019,37 +1022,38 @@
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
-	int val = simple_strtoul(buf, NULL, 10) / 1000;
+	long val = simple_strtol(buf, NULL, 10) / 1000;
 	ssize_t ret = count;
+	u8 reg;
 
 	mutex_lock(&data->update_lock);
 
 	/* convert abs to relative and check */
+	data->temp_high[nr] = f71882fg_read8(data, F71882FG_REG_TEMP_HIGH(nr));
+	val = SENSORS_LIMIT(val, data->temp_high[nr] - 15,
+			    data->temp_high[nr]);
 	val = data->temp_high[nr] - val;
-	if (val < 0 || val > 15) {
-		ret = -EINVAL;
-		goto store_temp_max_hyst_exit;
-	}
-
 	data->temp_hyst[nr] = val;
 
 	/* convert value to register contents */
 	switch (nr) {
 		case 1:
-			val = val << 4;
+			reg = f71882fg_read8(data, F71882FG_REG_TEMP_HYST1);
+			reg = (reg & 0x0f) | (val << 4);
 			break;
 		case 2:
-			val = val | (data->temp_hyst[3] << 4);
+			reg = f71882fg_read8(data, F71882FG_REG_TEMP_HYST23);
+			reg = (reg & 0xf0) | val;
 			break;
 		case 3:
-			val = data->temp_hyst[2] | (val << 4);
+			reg = f71882fg_read8(data, F71882FG_REG_TEMP_HYST23);
+			reg = (reg & 0x0f) | (val << 4);
 			break;
 	}
 
 	f71882fg_write8(data, (nr <= 1) ? F71882FG_REG_TEMP_HYST1 :
-		F71882FG_REG_TEMP_HYST23, val);
+		F71882FG_REG_TEMP_HYST23, reg);
 
-store_temp_max_hyst_exit:
 	mutex_unlock(&data->update_lock);
 	return ret;
 }
@@ -1068,10 +1072,8 @@
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
-	int val = simple_strtoul(buf, NULL, 10) / 1000;
-
-	if (val > 255)
-		val = 255;
+	long val = simple_strtol(buf, NULL, 10) / 1000;
+	val = SENSORS_LIMIT(val, 0, 255);
 
 	mutex_lock(&data->update_lock);
 	f71882fg_write8(data, F71882FG_REG_TEMP_OVT(nr), val);
@@ -1086,9 +1088,13 @@
 {
 	struct f71882fg_data *data = f71882fg_update_device(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
+	int temp_crit_hyst;
 
-	return sprintf(buf, "%d\n",
-		(data->temp_ovt[nr] - data->temp_hyst[nr]) * 1000);
+	mutex_lock(&data->update_lock);
+	temp_crit_hyst = (data->temp_ovt[nr] - data->temp_hyst[nr]) * 1000;
+	mutex_unlock(&data->update_lock);
+
+	return sprintf(buf, "%d\n", temp_crit_hyst);
 }
 
 static ssize_t show_temp_type(struct device *dev, struct device_attribute
@@ -1117,9 +1123,10 @@
 {
 	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
-	int val = simple_strtoul(buf, NULL, 10);
+	unsigned long val = simple_strtoul(buf, NULL, 10);
 
 	mutex_lock(&data->update_lock);
+	data->temp_beep = f71882fg_read8(data, F71882FG_REG_TEMP_BEEP);
 	if (val)
 		data->temp_beep |= 1 << nr;
 	else
@@ -1160,16 +1167,16 @@
 {
 	struct f71882fg_data *data = f71882fg_update_device(dev);
 	int val, nr = to_sensor_dev_attr_2(devattr)->index;
+	mutex_lock(&data->update_lock);
 	if (data->pwm_enable & (1 << (2 * nr)))
 		/* PWM mode */
 		val = data->pwm[nr];
 	else {
 		/* RPM mode */
-		mutex_lock(&data->update_lock);
 		val = 255 * fan_from_reg(data->fan_target[nr])
 			/ fan_from_reg(data->fan_full_speed[nr]);
-		mutex_unlock(&data->update_lock);
 	}
+	mutex_unlock(&data->update_lock);
 	return sprintf(buf, "%d\n", val);
 }
 
@@ -1177,23 +1184,26 @@
 			 struct device_attribute *devattr, const char *buf,
 			 size_t count)
 {
-	/* struct f71882fg_data *data = dev_get_drvdata(dev); */
-	struct f71882fg_data *data = f71882fg_update_device(dev);
+	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
 	long val = simple_strtol(buf, NULL, 10);
 	val = SENSORS_LIMIT(val, 0, 255);
 
 	mutex_lock(&data->update_lock);
+	data->pwm_enable = f71882fg_read8(data, F71882FG_REG_PWM_ENABLE);
 	if (data->pwm_enable & (1 << (2 * nr))) {
 		/* PWM mode */
 		f71882fg_write8(data, F71882FG_REG_PWM(nr), val);
 		data->pwm[nr] = val;
 	} else {
 		/* RPM mode */
-		int target = val * fan_from_reg(data->fan_full_speed[nr]) / 255;
-		f71882fg_write16(data, F71882FG_REG_FAN_TARGET(nr),
-				 fan_to_reg(target));
-		data->fan_target[nr] = fan_to_reg(target);
+		int target, full_speed;
+		full_speed = f71882fg_read16(data,
+					     F71882FG_REG_FAN_FULL_SPEED(nr));
+		target = fan_to_reg(val * fan_from_reg(full_speed) / 255);
+		f71882fg_write16(data, F71882FG_REG_FAN_TARGET(nr), target);
+		data->fan_target[nr] = target;
+		data->fan_full_speed[nr] = full_speed;
 	}
 	mutex_unlock(&data->update_lock);
 
@@ -1225,6 +1235,7 @@
 		return -EINVAL;
 
 	mutex_lock(&data->update_lock);
+	data->pwm_enable = f71882fg_read8(data, F71882FG_REG_PWM_ENABLE);
 	switch (val) {
 	case 1:
 		data->pwm_enable |= 2 << (2 * nr);
@@ -1258,6 +1269,7 @@
 	int pwm = to_sensor_dev_attr_2(devattr)->index;
 	int point = to_sensor_dev_attr_2(devattr)->nr;
 
+	mutex_lock(&data->update_lock);
 	if (data->pwm_enable & (1 << (2 * pwm))) {
 		/* PWM mode */
 		result = data->pwm_auto_point_pwm[pwm][point];
@@ -1265,6 +1277,7 @@
 		/* RPM mode */
 		result = 32 * 255 / (32 + data->pwm_auto_point_pwm[pwm][point]);
 	}
+	mutex_unlock(&data->update_lock);
 
 	return sprintf(buf, "%d\n", result);
 }
@@ -1273,14 +1286,14 @@
 					struct device_attribute *devattr,
 					const char *buf, size_t count)
 {
-	/* struct f71882fg_data *data = dev_get_drvdata(dev); */
-	struct f71882fg_data *data = f71882fg_update_device(dev);
+	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int pwm = to_sensor_dev_attr_2(devattr)->index;
 	int point = to_sensor_dev_attr_2(devattr)->nr;
-	int val = simple_strtoul(buf, NULL, 10);
+	long val = simple_strtol(buf, NULL, 10);
 	val = SENSORS_LIMIT(val, 0, 255);
 
 	mutex_lock(&data->update_lock);
+	data->pwm_enable = f71882fg_read8(data, F71882FG_REG_PWM_ENABLE);
 	if (data->pwm_enable & (1 << (2 * pwm))) {
 		/* PWM mode */
 	} else {
@@ -1331,16 +1344,25 @@
 					      struct device_attribute *devattr,
 					      const char *buf, size_t count)
 {
-	struct f71882fg_data *data = f71882fg_update_device(dev);
+	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
 	int point = to_sensor_dev_attr_2(devattr)->nr;
 	long val = simple_strtol(buf, NULL, 10) / 1000;
 
 	mutex_lock(&data->update_lock);
+	data->pwm_auto_point_temp[nr][point] =
+		f71882fg_read8(data, F71882FG_REG_POINT_TEMP(nr, point));
 	val = SENSORS_LIMIT(val, data->pwm_auto_point_temp[nr][point] - 15,
 				data->pwm_auto_point_temp[nr][point]);
 	val = data->pwm_auto_point_temp[nr][point] - val;
 
+	if (nr == 0 || nr == 1) {
+		data->pwm_auto_point_hyst[0] =
+			f71882fg_read8(data, F71882FG_REG_FAN_HYST0);
+	} else {
+		data->pwm_auto_point_hyst[1] =
+			f71882fg_read8(data, F71882FG_REG_FAN_HYST1);
+	}
 	switch (nr) {
 	case 0:
 		val = (data->pwm_auto_point_hyst[0] & 0xf0) | val;
@@ -1383,11 +1405,13 @@
 				     struct device_attribute *devattr,
 				     const char *buf, size_t count)
 {
-	/* struct f71882fg_data *data = dev_get_drvdata(dev); */
-	struct f71882fg_data *data = f71882fg_update_device(dev);
+	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
-	int val = simple_strtoul(buf, NULL, 10);
+	unsigned long val = simple_strtoul(buf, NULL, 10);
+
 	mutex_lock(&data->update_lock);
+	data->pwm_auto_point_mapping[nr] =
+		f71882fg_read8(data, F71882FG_REG_POINT_MAPPING(nr));
 	if (val)
 		val = data->pwm_auto_point_mapping[nr] | (1 << 4);
 	else
@@ -1416,8 +1440,7 @@
 					    struct device_attribute *devattr,
 					    const char *buf, size_t count)
 {
-	/* struct f71882fg_data *data = dev_get_drvdata(dev); */
-	struct f71882fg_data *data = f71882fg_update_device(dev);
+	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int nr = to_sensor_dev_attr_2(devattr)->index;
 	long val = simple_strtol(buf, NULL, 10);
 	switch (val) {
@@ -1434,6 +1457,8 @@
 		return -EINVAL;
 	}
 	mutex_lock(&data->update_lock);
+	data->pwm_auto_point_mapping[nr] =
+		f71882fg_read8(data, F71882FG_REG_POINT_MAPPING(nr));
 	val = (data->pwm_auto_point_mapping[nr] & 0xfc) | val;
 	f71882fg_write8(data, F71882FG_REG_POINT_MAPPING(nr), val);
 	data->pwm_auto_point_mapping[nr] = val;
@@ -1459,8 +1484,7 @@
 					 struct device_attribute *devattr,
 					 const char *buf, size_t count)
 {
-	/* struct f71882fg_data *data = dev_get_drvdata(dev); */
-	struct f71882fg_data *data = f71882fg_update_device(dev);
+	struct f71882fg_data *data = dev_get_drvdata(dev);
 	int pwm = to_sensor_dev_attr_2(devattr)->index;
 	int point = to_sensor_dev_attr_2(devattr)->nr;
 	long val = simple_strtol(buf, NULL, 10) / 1000;

[-- Attachment #3: hwmon-f71882fg-09-simplify-hyst-functions-v2.patch --]
[-- Type: text/plain, Size: 6549 bytes --]

Simplify fan and temp hyst. handling by treating the registers as an array of
nibbles instead of using switch cases. Also unify the way hysts are handled
between temp and fans, the temp code was storing the actual per temp hyst
values in 4 u8's, where as the fan code was stroing actual register values.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
--- linux/drivers/hwmon/f71882fg.c.08-applied	2008-12-14 14:22:40.000000000 +0100
+++ linux/drivers/hwmon/f71882fg.c	2008-12-14 15:16:30.000000000 +0100
@@ -68,8 +68,7 @@
 #define F71882FG_REG_TEMP_HIGH(nr)	(0x81 + 2 * (nr))
 #define F71882FG_REG_TEMP_STATUS	0x62
 #define F71882FG_REG_TEMP_BEEP		0x63
-#define F71882FG_REG_TEMP_HYST1		0x6C
-#define F71882FG_REG_TEMP_HYST23	0x6D
+#define F71882FG_REG_TEMP_HYST(nr)	(0x6C + (nr))
 #define F71882FG_REG_TEMP_TYPE		0x6B
 #define F71882FG_REG_TEMP_DIODE_OPEN	0x6F
 
@@ -77,8 +76,7 @@
 #define F71882FG_REG_PWM_TYPE		0x94
 #define F71882FG_REG_PWM_ENABLE		0x96
 
-#define F71882FG_REG_FAN_HYST0		0x98
-#define F71882FG_REG_FAN_HYST1		0x99
+#define F71882FG_REG_FAN_HYST(nr)	(0x98 + (nr))
 
 #define F71882FG_REG_POINT_PWM(pwm, point)	(0xAA + (point) + (16 * (pwm)))
 #define F71882FG_REG_POINT_TEMP(pwm, point)	(0xA6 + (point) + (16 * (pwm)))
@@ -144,7 +142,7 @@
 	u8	temp[4];
 	u8	temp_ovt[4];
 	u8	temp_high[4];
-	u8	temp_hyst[4];
+	u8	temp_hyst[2]; /* 2 hysts stored per reg */
 	u8	temp_type[4];
 	u8	temp_status;
 	u8	temp_beep;
@@ -688,13 +686,11 @@
 						F71882FG_REG_TEMP_HIGH(nr));
 		}
 
-		/* Have to hardcode hyst*/
-		data->temp_hyst[1] = f71882fg_read8(data,
-						F71882FG_REG_TEMP_HYST1) >> 4;
-		/* Hyst temps 2 & 3 stored in same register */
-		reg = f71882fg_read8(data, F71882FG_REG_TEMP_HYST23);
-		data->temp_hyst[2] = reg & 0x0F;
-		data->temp_hyst[3] = reg >> 4;
+		/* hyst */
+		data->temp_hyst[0] =
+			f71882fg_read8(data, F71882FG_REG_TEMP_HYST(0));
+		data->temp_hyst[1] =
+			f71882fg_read8(data, F71882FG_REG_TEMP_HYST(1));
 
 		/* Have to hardcode type, because temp1 is special */
 		reg  = f71882fg_read8(data, F71882FG_REG_TEMP_TYPE);
@@ -715,10 +711,11 @@
 
 		data->pwm_enable = f71882fg_read8(data,
 						  F71882FG_REG_PWM_ENABLE);
-		data->pwm_auto_point_hyst[0] = f71882fg_read8(data,
-						      F71882FG_REG_FAN_HYST0);
-		data->pwm_auto_point_hyst[1] = f71882fg_read8(data,
-						      F71882FG_REG_FAN_HYST1);
+		data->pwm_auto_point_hyst[0] =
+			f71882fg_read8(data, F71882FG_REG_FAN_HYST(0));
+		data->pwm_auto_point_hyst[1] =
+			f71882fg_read8(data, F71882FG_REG_FAN_HYST(1));
+
 		for (nr = 0; nr < nr_fans; nr++) {
 			data->pwm_auto_point_mapping[nr] =
 			    f71882fg_read8(data,
@@ -1011,7 +1008,11 @@
 	int temp_max_hyst;
 
 	mutex_lock(&data->update_lock);
-	temp_max_hyst = (data->temp_high[nr] - data->temp_hyst[nr]) * 1000;
+	if (nr & 1)
+		temp_max_hyst = data->temp_hyst[nr / 2] >> 4;
+	else
+		temp_max_hyst = data->temp_hyst[nr / 2] & 0x0f;
+	temp_max_hyst = (data->temp_high[nr] - temp_max_hyst) * 1000;
 	mutex_unlock(&data->update_lock);
 
 	return sprintf(buf, "%d\n", temp_max_hyst);
@@ -1033,26 +1034,15 @@
 	val = SENSORS_LIMIT(val, data->temp_high[nr] - 15,
 			    data->temp_high[nr]);
 	val = data->temp_high[nr] - val;
-	data->temp_hyst[nr] = val;
 
 	/* convert value to register contents */
-	switch (nr) {
-		case 1:
-			reg = f71882fg_read8(data, F71882FG_REG_TEMP_HYST1);
-			reg = (reg & 0x0f) | (val << 4);
-			break;
-		case 2:
-			reg = f71882fg_read8(data, F71882FG_REG_TEMP_HYST23);
-			reg = (reg & 0xf0) | val;
-			break;
-		case 3:
-			reg = f71882fg_read8(data, F71882FG_REG_TEMP_HYST23);
-			reg = (reg & 0x0f) | (val << 4);
-			break;
-	}
-
-	f71882fg_write8(data, (nr <= 1) ? F71882FG_REG_TEMP_HYST1 :
-		F71882FG_REG_TEMP_HYST23, reg);
+	reg = f71882fg_read8(data, F71882FG_REG_TEMP_HYST(nr / 2));
+	if (nr & 1)
+		reg = (reg & 0x0f) | (val << 4);
+	else
+		reg = (reg & 0xf0) | val;
+	f71882fg_write8(data, F71882FG_REG_TEMP_HYST(nr / 2), reg);
+	data->temp_hyst[nr / 2] = reg;
 
 	mutex_unlock(&data->update_lock);
 	return ret;
@@ -1091,7 +1081,11 @@
 	int temp_crit_hyst;
 
 	mutex_lock(&data->update_lock);
-	temp_crit_hyst = (data->temp_ovt[nr] - data->temp_hyst[nr]) * 1000;
+	if (nr & 1)
+		temp_crit_hyst = data->temp_hyst[nr / 2] >> 4;
+	else
+		temp_crit_hyst = data->temp_hyst[nr / 2] & 0x0f;
+	temp_crit_hyst = (data->temp_ovt[nr] - temp_crit_hyst) * 1000;
 	mutex_unlock(&data->update_lock);
 
 	return sprintf(buf, "%d\n", temp_crit_hyst);
@@ -1320,20 +1314,10 @@
 	int point = to_sensor_dev_attr_2(devattr)->nr;
 
 	mutex_lock(&data->update_lock);
-	switch (nr) {
-	case 0:
-		result = data->pwm_auto_point_hyst[0] & 0x0f;
-		break;
-	case 1:
-		result = data->pwm_auto_point_hyst[0] >> 4;
-		break;
-	case 2:
-		result = data->pwm_auto_point_hyst[1] & 0x0f;
-		break;
-	case 3:
-		result = data->pwm_auto_point_hyst[1] >> 4;
-		break;
-	}
+	if (nr & 1)
+		result = data->pwm_auto_point_hyst[nr / 2] >> 4;
+	else
+		result = data->pwm_auto_point_hyst[nr / 2] & 0x0f;
 	result = 1000 * (data->pwm_auto_point_temp[nr][point] - result);
 	mutex_unlock(&data->update_lock);
 
@@ -1348,6 +1332,7 @@
 	int nr = to_sensor_dev_attr_2(devattr)->index;
 	int point = to_sensor_dev_attr_2(devattr)->nr;
 	long val = simple_strtol(buf, NULL, 10) / 1000;
+	u8 reg;
 
 	mutex_lock(&data->update_lock);
 	data->pwm_auto_point_temp[nr][point] =
@@ -1356,34 +1341,14 @@
 				data->pwm_auto_point_temp[nr][point]);
 	val = data->pwm_auto_point_temp[nr][point] - val;
 
-	if (nr == 0 || nr == 1) {
-		data->pwm_auto_point_hyst[0] =
-			f71882fg_read8(data, F71882FG_REG_FAN_HYST0);
-	} else {
-		data->pwm_auto_point_hyst[1] =
-			f71882fg_read8(data, F71882FG_REG_FAN_HYST1);
-	}
-	switch (nr) {
-	case 0:
-		val = (data->pwm_auto_point_hyst[0] & 0xf0) | val;
-		break;
-	case 1:
-		val = (data->pwm_auto_point_hyst[0] & 0x0f) | (val << 4);
-		break;
-	case 2:
-		val = (data->pwm_auto_point_hyst[1] & 0xf0) | val;
-		break;
-	case 3:
-		val = (data->pwm_auto_point_hyst[1] & 0x0f) | (val << 4);
-		break;
-	}
-	if (nr == 0 || nr == 1) {
-		f71882fg_write8(data, F71882FG_REG_FAN_HYST0, val);
-		data->pwm_auto_point_hyst[0] = val;
-	} else {
-		f71882fg_write8(data, F71882FG_REG_FAN_HYST1, val);
-		data->pwm_auto_point_hyst[1] = val;
-	}
+	reg = f71882fg_read8(data, F71882FG_REG_FAN_HYST(nr / 2));
+	if (nr & 1)
+		reg = (reg & 0x0f) | (val << 4);
+	else
+		reg = (reg & 0xf0) | val;
+
+	f71882fg_write8(data, F71882FG_REG_FAN_HYST(nr / 2), reg);
+	data->pwm_auto_point_hyst[nr / 2] = reg;
 	mutex_unlock(&data->update_lock);
 
 	return count;

[-- Attachment #4: Type: text/plain, Size: 153 bytes --]

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

  parent reply	other threads:[~2008-12-14 18:40 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-12-11 22:21 [lm-sensors] PATCH: fix various sysfs callback function issues in Hans de Goede
2008-12-13 22:18 ` [lm-sensors] PATCH: fix various sysfs callback function issues Jean Delvare
2008-12-14 14:32 ` Hans de Goede
2008-12-14 18:04 ` Jean Delvare
2008-12-14 18:40 ` Hans de Goede [this message]
2008-12-14 21:30 ` Jean Delvare

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=49455325.5030707@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=lm-sensors@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.