All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH v2 0/3] hwmon: (adt7475) small enhancements
@ 2017-05-03  0:40 Chris Packham
  2017-05-03  0:40 ` [RFC PATCH v2 1/3] hwmon: (adt7475) replace find_nearest() with find_closest() Chris Packham
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Chris Packham @ 2017-05-03  0:40 UTC (permalink / raw)
  To: linux, linux-hwmon, jdelvare

This is a series of small enhancements to the adt7475 driver. We're using an
adt7476 chip in several network switches and generally speaking the
requirement for this type of equipment is to keep the fan moving so that we
know it is working when we need it.

Chris Packham (3):
      hwmon: (adt7475) replace find_nearest() with find_closest()
      hwmon: (adt7475) fan stall prevention
      hwmon: (adt7475) temperature smoothing

 Documentation/hwmon/adt7475 |   9 +++
 drivers/hwmon/adt7475.c     | 149 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 158 insertions(+)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [RFC PATCH v2 1/3] hwmon: (adt7475) replace find_nearest() with find_closest()
  2017-05-03  0:40 [RFC PATCH v2 0/3] hwmon: (adt7475) small enhancements Chris Packham
@ 2017-05-03  0:40 ` Chris Packham
  2017-05-03  0:40 ` [RFC PATCH v2 2/3] hwmon: (adt7475) fan stall prevention Chris Packham
  2017-05-03  0:40 ` [RFC PATCH v2 3/3] hwmon: (adt7475) temperature smoothing Chris Packham
  2 siblings, 0 replies; 9+ messages in thread
From: Chris Packham @ 2017-05-03  0:40 UTC (permalink / raw)
  To: linux, linux-hwmon, jdelvare; +Cc: Chris Packham, linux-kernel

The adt7475 has had find_nearest() since it's creation in 2009. Since
then find_closest() has been introduced and several drivers have been
updated to use it. Update the adt7475 to use find_closest() and remove
the now unused find_nearest().

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
 drivers/hwmon/adt7475.c | 34 +++-------------------------------
 1 file changed, 3 insertions(+), 31 deletions(-)

diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index c803e3c5fcd4..ec0c43fbcdce 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -22,6 +22,7 @@
 #include <linux/hwmon-vid.h>
 #include <linux/err.h>
 #include <linux/jiffies.h>
+#include <linux/util_macros.h>
 
 /* Indexes for the sysfs hooks */
 
@@ -314,35 +315,6 @@ static void adt7475_write_word(struct i2c_client *client, int reg, u16 val)
 	i2c_smbus_write_byte_data(client, reg, val & 0xFF);
 }
 
-/*
- * Find the nearest value in a table - used for pwm frequency and
- * auto temp range
- */
-static int find_nearest(long val, const int *array, int size)
-{
-	int i;
-
-	if (val < array[0])
-		return 0;
-
-	if (val > array[size - 1])
-		return size - 1;
-
-	for (i = 0; i < size - 1; i++) {
-		int a, b;
-
-		if (val > array[i + 1])
-			continue;
-
-		a = val - array[i];
-		b = array[i + 1] - val;
-
-		return (a <= b) ? i : i + 1;
-	}
-
-	return 0;
-}
-
 static ssize_t show_voltage(struct device *dev, struct device_attribute *attr,
 			    char *buf)
 {
@@ -606,7 +578,7 @@ static ssize_t set_point2(struct device *dev, struct device_attribute *attr,
 	val -= temp;
 
 	/* Find the nearest table entry to what the user wrote */
-	val = find_nearest(val, autorange_table, ARRAY_SIZE(autorange_table));
+	val = find_closest(val, autorange_table, ARRAY_SIZE(autorange_table));
 
 	data->range[sattr->index] &= ~0xF0;
 	data->range[sattr->index] |= val << 4;
@@ -864,7 +836,7 @@ static ssize_t set_pwmfreq(struct device *dev, struct device_attribute *attr,
 	if (kstrtol(buf, 10, &val))
 		return -EINVAL;
 
-	out = find_nearest(val, pwmfreq_table, ARRAY_SIZE(pwmfreq_table));
+	out = find_closest(val, pwmfreq_table, ARRAY_SIZE(pwmfreq_table));
 
 	mutex_lock(&data->lock);
 
-- 
2.11.0.24.ge6920cf


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC PATCH v2 2/3] hwmon: (adt7475) fan stall prevention
  2017-05-03  0:40 [RFC PATCH v2 0/3] hwmon: (adt7475) small enhancements Chris Packham
  2017-05-03  0:40 ` [RFC PATCH v2 1/3] hwmon: (adt7475) replace find_nearest() with find_closest() Chris Packham
@ 2017-05-03  0:40 ` Chris Packham
  2017-05-03 16:10   ` Guenter Roeck
  2017-05-03  0:40 ` [RFC PATCH v2 3/3] hwmon: (adt7475) temperature smoothing Chris Packham
  2 siblings, 1 reply; 9+ messages in thread
From: Chris Packham @ 2017-05-03  0:40 UTC (permalink / raw)
  To: linux, linux-hwmon, jdelvare
  Cc: Chris Packham, Jonathan Corbet, linux-doc, linux-kernel

By default adt7475 will stop the fans (pwm duty cycle 0%) when the
temperature drops past Tmin - hysteresis. Some systems want to keep the
fans moving even when the temperature drops so add new sysfs attributes
that configure the enhanced acoustics min 1-3 which allows the fans to
run at the minimum configure pwm duty cycle.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
Changes in v2:
- use pwmN_stall_dis as the attribute name. I think this describes the purpose
  pretty well. I went with a new attribute instead of overloading
  pwmN_auto_point1_pwm so this doesn't affect existing users.

 Documentation/hwmon/adt7475 |  5 +++++
 drivers/hwmon/adt7475.c     | 50 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
index 0502f2b464e1..63507402cd4f 100644
--- a/Documentation/hwmon/adt7475
+++ b/Documentation/hwmon/adt7475
@@ -109,6 +109,11 @@ fan speed) is applied. PWM values range from 0 (off) to 255 (full speed).
 Fan speed may be set to maximum when the temperature sensor associated with
 the PWM control exceeds temp#_max.
 
+At Tmin - hysteresis the PWM output can either be off (0% duty cycle) or at the
+minimum (i.e. auto_point1_pwm). This behaviour be configured using the
+pwm[1-*]_stall_dis sysfs attribute. A value of 0 means the fans will shut off.
+A value of 1 means the fans will run at auto_point1_pwm.
+
 Notes
 -----
 
diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index ec0c43fbcdce..85957324cd85 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -79,6 +79,9 @@
 
 #define REG_TEMP_TRANGE_BASE	0x5F
 
+#define REG_ENHANCE_ACOUSTICS1	0x62
+#define REG_ENHANCE_ACOUSTICS2	0x63
+
 #define REG_PWM_MIN_BASE	0x64
 
 #define REG_TEMP_TMIN_BASE	0x67
@@ -209,6 +212,7 @@ struct adt7475_data {
 	u8 range[3];
 	u8 pwmctl[3];
 	u8 pwmchan[3];
+	u8 enh_acou[2];
 
 	u8 vid;
 	u8 vrm;
@@ -700,6 +704,43 @@ static ssize_t set_pwm(struct device *dev, struct device_attribute *attr,
 	data->pwm[sattr->nr][sattr->index] = clamp_val(val, 0, 0xFF);
 	i2c_smbus_write_byte_data(client, reg,
 				  data->pwm[sattr->nr][sattr->index]);
+	mutex_unlock(&data->lock);
+
+	return count;
+}
+
+
+static ssize_t show_stall_dis(struct device *dev, struct device_attribute *attr,
+			      char *buf)
+{
+	struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+	struct i2c_client *client = to_i2c_client(dev);
+	struct adt7475_data *data = i2c_get_clientdata(client);
+	u8 mask = BIT(5 + sattr->index);
+
+	return sprintf(buf, "%d\n", !!(data->enh_acou[0] & mask));
+}
+
+static ssize_t set_stall_dis(struct device *dev, struct device_attribute *attr,
+			     const char *buf, size_t count)
+{
+	struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+	struct i2c_client *client = to_i2c_client(dev);
+	struct adt7475_data *data = i2c_get_clientdata(client);
+	long val;
+	u8 mask = BIT(5 + sattr->index);
+
+	if (kstrtol(buf, 10, &val))
+		return -EINVAL;
+
+	mutex_lock(&data->lock);
+
+	data->enh_acou[0] &= ~mask;
+	if (val)
+		data->enh_acou[0] |= mask;
+
+	i2c_smbus_write_byte_data(client, REG_ENHANCE_ACOUSTICS1,
+				  data->enh_acou[0]);
 
 	mutex_unlock(&data->lock);
 
@@ -1028,6 +1069,8 @@ static SENSOR_DEVICE_ATTR_2(pwm1_auto_point1_pwm, S_IRUGO | S_IWUSR, show_pwm,
 			    set_pwm, MIN, 0);
 static SENSOR_DEVICE_ATTR_2(pwm1_auto_point2_pwm, S_IRUGO | S_IWUSR, show_pwm,
 			    set_pwm, MAX, 0);
+static SENSOR_DEVICE_ATTR_2(pwm1_stall_dis, S_IRUGO | S_IWUSR, show_stall_dis,
+			    set_stall_dis, 0, 0);
 static SENSOR_DEVICE_ATTR_2(pwm2, S_IRUGO | S_IWUSR, show_pwm, set_pwm, INPUT,
 			    1);
 static SENSOR_DEVICE_ATTR_2(pwm2_freq, S_IRUGO | S_IWUSR, show_pwmfreq,
@@ -1040,6 +1083,8 @@ static SENSOR_DEVICE_ATTR_2(pwm2_auto_point1_pwm, S_IRUGO | S_IWUSR, show_pwm,
 			    set_pwm, MIN, 1);
 static SENSOR_DEVICE_ATTR_2(pwm2_auto_point2_pwm, S_IRUGO | S_IWUSR, show_pwm,
 			    set_pwm, MAX, 1);
+static SENSOR_DEVICE_ATTR_2(pwm2_stall_dis, S_IRUGO | S_IWUSR, show_stall_dis,
+			    set_stall_dis, 0, 1);
 static SENSOR_DEVICE_ATTR_2(pwm3, S_IRUGO | S_IWUSR, show_pwm, set_pwm, INPUT,
 			    2);
 static SENSOR_DEVICE_ATTR_2(pwm3_freq, S_IRUGO | S_IWUSR, show_pwmfreq,
@@ -1052,6 +1097,8 @@ static SENSOR_DEVICE_ATTR_2(pwm3_auto_point1_pwm, S_IRUGO | S_IWUSR, show_pwm,
 			    set_pwm, MIN, 2);
 static SENSOR_DEVICE_ATTR_2(pwm3_auto_point2_pwm, S_IRUGO | S_IWUSR, show_pwm,
 			    set_pwm, MAX, 2);
+static SENSOR_DEVICE_ATTR_2(pwm3_stall_dis, S_IRUGO | S_IWUSR, show_stall_dis,
+			    set_stall_dis, 0, 2);
 
 /* Non-standard name, might need revisiting */
 static DEVICE_ATTR_RW(pwm_use_point2_pwm_at_crit);
@@ -1112,12 +1159,14 @@ static struct attribute *adt7475_attrs[] = {
 	&sensor_dev_attr_pwm1_auto_channels_temp.dev_attr.attr,
 	&sensor_dev_attr_pwm1_auto_point1_pwm.dev_attr.attr,
 	&sensor_dev_attr_pwm1_auto_point2_pwm.dev_attr.attr,
+	&sensor_dev_attr_pwm1_stall_dis.dev_attr.attr,
 	&sensor_dev_attr_pwm3.dev_attr.attr,
 	&sensor_dev_attr_pwm3_freq.dev_attr.attr,
 	&sensor_dev_attr_pwm3_enable.dev_attr.attr,
 	&sensor_dev_attr_pwm3_auto_channels_temp.dev_attr.attr,
 	&sensor_dev_attr_pwm3_auto_point1_pwm.dev_attr.attr,
 	&sensor_dev_attr_pwm3_auto_point2_pwm.dev_attr.attr,
+	&sensor_dev_attr_pwm3_stall_dis.dev_attr.attr,
 	&dev_attr_pwm_use_point2_pwm_at_crit.attr,
 	NULL,
 };
@@ -1136,6 +1185,7 @@ static struct attribute *pwm2_attrs[] = {
 	&sensor_dev_attr_pwm2_auto_channels_temp.dev_attr.attr,
 	&sensor_dev_attr_pwm2_auto_point1_pwm.dev_attr.attr,
 	&sensor_dev_attr_pwm2_auto_point2_pwm.dev_attr.attr,
+	&sensor_dev_attr_pwm2_stall_dis.dev_attr.attr,
 	NULL
 };
 
-- 
2.11.0.24.ge6920cf


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [RFC PATCH v2 3/3] hwmon: (adt7475) temperature smoothing
  2017-05-03  0:40 [RFC PATCH v2 0/3] hwmon: (adt7475) small enhancements Chris Packham
  2017-05-03  0:40 ` [RFC PATCH v2 1/3] hwmon: (adt7475) replace find_nearest() with find_closest() Chris Packham
  2017-05-03  0:40 ` [RFC PATCH v2 2/3] hwmon: (adt7475) fan stall prevention Chris Packham
@ 2017-05-03  0:40 ` Chris Packham
  2017-05-03  8:14   ` Chris Packham
  2017-05-03 16:30   ` Guenter Roeck
  2 siblings, 2 replies; 9+ messages in thread
From: Chris Packham @ 2017-05-03  0:40 UTC (permalink / raw)
  To: linux, linux-hwmon, jdelvare
  Cc: Chris Packham, Jonathan Corbet, linux-doc, linux-kernel

When enabled temperature smoothing allows ramping the fan speed over a
configurable period of time instead of jumping to the new speed
instantaneously.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---
Changes in v2:
- use a single tempN_smoothing attribute

 Documentation/hwmon/adt7475 |  4 ++
 drivers/hwmon/adt7475.c     | 99 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+)

diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
index 63507402cd4f..dd6433d819d0 100644
--- a/Documentation/hwmon/adt7475
+++ b/Documentation/hwmon/adt7475
@@ -114,6 +114,10 @@ minimum (i.e. auto_point1_pwm). This behaviour be configured using the
 pwm[1-*]_stall_dis sysfs attribute. A value of 0 means the fans will shut off.
 A value of 1 means the fans will run at auto_point1_pwm.
 
+The responsiveness of the ADT747x to temperature changes can be configured.
+This allows smoothing of the fan speed transition. To set the transition time
+set the value in ms in the temp[1-*]_smoothing sysfs attribute.
+
 Notes
 -----
 
diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
index 85957324cd85..41342de6e20c 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -526,6 +526,96 @@ static ssize_t set_temp(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
+/* Assuming CONFIG6[SLOW] is 0 */
+static const int ad7475_st_map[] = {
+	37500, 18800, 12500, 7500, 4700, 3100, 1600, 800,
+};
+
+static ssize_t show_temp_st(struct device *dev, struct device_attribute *attr,
+				  char *buf)
+{
+	struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+	struct i2c_client *client = to_i2c_client(dev);
+	struct adt7475_data *data = i2c_get_clientdata(client);
+	int shift, idx;
+	long val;
+
+	switch (sattr->index) {
+	case 0:
+		shift = 0;
+		idx = 0;
+		break;
+	case 1:
+		shift = 4;
+		idx = 1;
+		break;
+	case 2:
+		shift = 0;
+		idx = 1;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	val = data->enh_acou[idx] >> shift;
+	if (val & 0x8) {
+		return sprintf(buf, "%d\n", ad7475_st_map[val & 0x7]);
+	} else {
+		return sprintf(buf, "0\n");
+	}
+}
+
+static ssize_t set_temp_st(struct device *dev, struct device_attribute *attr,
+				 const char *buf, size_t count)
+{
+	struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
+	struct i2c_client *client = to_i2c_client(dev);
+	struct adt7475_data *data = i2c_get_clientdata(client);
+	unsigned char reg;
+	int shift, idx;
+	ulong val;
+
+	if (kstrtoul(buf, 10, &val))
+		return -EINVAL;
+
+	switch (sattr->index) {
+	case 0:
+		reg = REG_ENHANCE_ACOUSTICS1;
+		shift = 0;
+		idx = 0;
+		break;
+	case 1:
+		reg = REG_ENHANCE_ACOUSTICS2;
+		shift = 4;
+		idx = 1;
+		break;
+	case 2:
+		reg = REG_ENHANCE_ACOUSTICS2;
+		shift = 0;
+		idx = 1;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	if (val > 0) {
+		val = find_closest_descending(val, ad7475_st_map,
+					      ARRAY_SIZE(ad7475_st_map));
+		val |= 0x8;
+	}
+
+	mutex_lock(&data->lock);
+
+	data->enh_acou[idx] &= ~(0xf << shift);
+	data->enh_acou[idx] |= (val << shift);
+
+	i2c_smbus_write_byte_data(client, reg, data->enh_acou[idx]);
+
+	mutex_unlock(&data->lock);
+
+	return count;
+}
+
 /*
  * Table of autorange values - the user will write the value in millidegrees,
  * and we'll convert it
@@ -1008,6 +1098,8 @@ static SENSOR_DEVICE_ATTR_2(temp1_crit, S_IRUGO | S_IWUSR, show_temp, set_temp,
 			    THERM, 0);
 static SENSOR_DEVICE_ATTR_2(temp1_crit_hyst, S_IRUGO | S_IWUSR, show_temp,
 			    set_temp, HYSTERSIS, 0);
+static SENSOR_DEVICE_ATTR_2(temp1_smoothing, S_IRUGO | S_IWUSR, show_temp_st,
+			    set_temp_st, 0, 0);
 static SENSOR_DEVICE_ATTR_2(temp2_input, S_IRUGO, show_temp, NULL, INPUT, 1);
 static SENSOR_DEVICE_ATTR_2(temp2_alarm, S_IRUGO, show_temp, NULL, ALARM, 1);
 static SENSOR_DEVICE_ATTR_2(temp2_max, S_IRUGO | S_IWUSR, show_temp, set_temp,
@@ -1024,6 +1116,8 @@ static SENSOR_DEVICE_ATTR_2(temp2_crit, S_IRUGO | S_IWUSR, show_temp, set_temp,
 			    THERM, 1);
 static SENSOR_DEVICE_ATTR_2(temp2_crit_hyst, S_IRUGO | S_IWUSR, show_temp,
 			    set_temp, HYSTERSIS, 1);
+static SENSOR_DEVICE_ATTR_2(temp2_smoothing, S_IRUGO | S_IWUSR, show_temp_st,
+			    set_temp_st, 0, 1);
 static SENSOR_DEVICE_ATTR_2(temp3_input, S_IRUGO, show_temp, NULL, INPUT, 2);
 static SENSOR_DEVICE_ATTR_2(temp3_alarm, S_IRUGO, show_temp, NULL, ALARM, 2);
 static SENSOR_DEVICE_ATTR_2(temp3_fault, S_IRUGO, show_temp, NULL, FAULT, 2);
@@ -1041,6 +1135,8 @@ static SENSOR_DEVICE_ATTR_2(temp3_crit, S_IRUGO | S_IWUSR, show_temp, set_temp,
 			    THERM, 2);
 static SENSOR_DEVICE_ATTR_2(temp3_crit_hyst, S_IRUGO | S_IWUSR, show_temp,
 			    set_temp, HYSTERSIS, 2);
+static SENSOR_DEVICE_ATTR_2(temp3_smoothing, S_IRUGO | S_IWUSR, show_temp_st,
+			    set_temp_st, 0, 2);
 static SENSOR_DEVICE_ATTR_2(fan1_input, S_IRUGO, show_tach, NULL, INPUT, 0);
 static SENSOR_DEVICE_ATTR_2(fan1_min, S_IRUGO | S_IWUSR, show_tach, set_tach,
 			    MIN, 0);
@@ -1125,6 +1221,7 @@ static struct attribute *adt7475_attrs[] = {
 	&sensor_dev_attr_temp1_auto_point2_temp.dev_attr.attr,
 	&sensor_dev_attr_temp1_crit.dev_attr.attr,
 	&sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
+	&sensor_dev_attr_temp1_smoothing.dev_attr.attr,
 	&sensor_dev_attr_temp2_input.dev_attr.attr,
 	&sensor_dev_attr_temp2_alarm.dev_attr.attr,
 	&sensor_dev_attr_temp2_max.dev_attr.attr,
@@ -1134,6 +1231,7 @@ static struct attribute *adt7475_attrs[] = {
 	&sensor_dev_attr_temp2_auto_point2_temp.dev_attr.attr,
 	&sensor_dev_attr_temp2_crit.dev_attr.attr,
 	&sensor_dev_attr_temp2_crit_hyst.dev_attr.attr,
+	&sensor_dev_attr_temp2_smoothing.dev_attr.attr,
 	&sensor_dev_attr_temp3_input.dev_attr.attr,
 	&sensor_dev_attr_temp3_fault.dev_attr.attr,
 	&sensor_dev_attr_temp3_alarm.dev_attr.attr,
@@ -1144,6 +1242,7 @@ static struct attribute *adt7475_attrs[] = {
 	&sensor_dev_attr_temp3_auto_point2_temp.dev_attr.attr,
 	&sensor_dev_attr_temp3_crit.dev_attr.attr,
 	&sensor_dev_attr_temp3_crit_hyst.dev_attr.attr,
+	&sensor_dev_attr_temp3_smoothing.dev_attr.attr,
 	&sensor_dev_attr_fan1_input.dev_attr.attr,
 	&sensor_dev_attr_fan1_min.dev_attr.attr,
 	&sensor_dev_attr_fan1_alarm.dev_attr.attr,
-- 
2.11.0.24.ge6920cf

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH v2 3/3] hwmon: (adt7475) temperature smoothing
  2017-05-03  0:40 ` [RFC PATCH v2 3/3] hwmon: (adt7475) temperature smoothing Chris Packham
@ 2017-05-03  8:14   ` Chris Packham
  2017-05-03 16:30   ` Guenter Roeck
  1 sibling, 0 replies; 9+ messages in thread
From: Chris Packham @ 2017-05-03  8:14 UTC (permalink / raw)
  To: linux, linux-hwmon, jdelvare; +Cc: Jonathan Corbet, linux-doc, linux-kernel

On 03/05/17 12:40, Chris Packham wrote:
> When enabled temperature smoothing allows ramping the fan speed over a
> configurable period of time instead of jumping to the new speed
> instantaneously.
>
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> Changes in v2:
> - use a single tempN_smoothing attribute
>
>  Documentation/hwmon/adt7475 |  4 ++
>  drivers/hwmon/adt7475.c     | 99 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 103 insertions(+)

I've had some feedback internally on this patch around making 
show_temp_st() smaller and using "enh_acoustics" instead of "enh_acou". 
I'll wait for a while to see if there's any more feedback from the list 
before sending out v3.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH v2 2/3] hwmon: (adt7475) fan stall prevention
  2017-05-03  0:40 ` [RFC PATCH v2 2/3] hwmon: (adt7475) fan stall prevention Chris Packham
@ 2017-05-03 16:10   ` Guenter Roeck
  2017-05-03 20:44     ` Chris Packham
  0 siblings, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2017-05-03 16:10 UTC (permalink / raw)
  To: Chris Packham
  Cc: linux-hwmon, jdelvare, Jonathan Corbet, linux-doc, linux-kernel

On Wed, May 03, 2017 at 12:40:08PM +1200, Chris Packham wrote:
> By default adt7475 will stop the fans (pwm duty cycle 0%) when the
> temperature drops past Tmin - hysteresis. Some systems want to keep the
> fans moving even when the temperature drops so add new sysfs attributes
> that configure the enhanced acoustics min 1-3 which allows the fans to
> run at the minimum configure pwm duty cycle.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> Changes in v2:
> - use pwmN_stall_dis as the attribute name. I think this describes the purpose
>   pretty well. I went with a new attribute instead of overloading
>   pwmN_auto_point1_pwm so this doesn't affect existing users.
> 
>  Documentation/hwmon/adt7475 |  5 +++++
>  drivers/hwmon/adt7475.c     | 50 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 55 insertions(+)
> 
> diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
> index 0502f2b464e1..63507402cd4f 100644
> --- a/Documentation/hwmon/adt7475
> +++ b/Documentation/hwmon/adt7475
> @@ -109,6 +109,11 @@ fan speed) is applied. PWM values range from 0 (off) to 255 (full speed).
>  Fan speed may be set to maximum when the temperature sensor associated with
>  the PWM control exceeds temp#_max.
>  
> +At Tmin - hysteresis the PWM output can either be off (0% duty cycle) or at the
> +minimum (i.e. auto_point1_pwm). This behaviour be configured using the
> +pwm[1-*]_stall_dis sysfs attribute. A value of 0 means the fans will shut off.

That is really an awkward attribute name. I'll have to think about this some
more.

Guenter

> +A value of 1 means the fans will run at auto_point1_pwm.
> +
>  Notes
>  -----
>  
> diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
> index ec0c43fbcdce..85957324cd85 100644
> --- a/drivers/hwmon/adt7475.c
> +++ b/drivers/hwmon/adt7475.c
> @@ -79,6 +79,9 @@
>  
>  #define REG_TEMP_TRANGE_BASE	0x5F
>  
> +#define REG_ENHANCE_ACOUSTICS1	0x62
> +#define REG_ENHANCE_ACOUSTICS2	0x63
> +
>  #define REG_PWM_MIN_BASE	0x64
>  
>  #define REG_TEMP_TMIN_BASE	0x67
> @@ -209,6 +212,7 @@ struct adt7475_data {
>  	u8 range[3];
>  	u8 pwmctl[3];
>  	u8 pwmchan[3];
> +	u8 enh_acou[2];
>  
>  	u8 vid;
>  	u8 vrm;
> @@ -700,6 +704,43 @@ static ssize_t set_pwm(struct device *dev, struct device_attribute *attr,
>  	data->pwm[sattr->nr][sattr->index] = clamp_val(val, 0, 0xFF);
>  	i2c_smbus_write_byte_data(client, reg,
>  				  data->pwm[sattr->nr][sattr->index]);
> +	mutex_unlock(&data->lock);
> +
> +	return count;
> +}
> +
> +
> +static ssize_t show_stall_dis(struct device *dev, struct device_attribute *attr,
> +			      char *buf)
> +{
> +	struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct adt7475_data *data = i2c_get_clientdata(client);
> +	u8 mask = BIT(5 + sattr->index);
> +
> +	return sprintf(buf, "%d\n", !!(data->enh_acou[0] & mask));
> +}
> +
> +static ssize_t set_stall_dis(struct device *dev, struct device_attribute *attr,
> +			     const char *buf, size_t count)
> +{
> +	struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct adt7475_data *data = i2c_get_clientdata(client);
> +	long val;
> +	u8 mask = BIT(5 + sattr->index);
> +
> +	if (kstrtol(buf, 10, &val))
> +		return -EINVAL;
> +
> +	mutex_lock(&data->lock);
> +
> +	data->enh_acou[0] &= ~mask;
> +	if (val)
> +		data->enh_acou[0] |= mask;
> +
> +	i2c_smbus_write_byte_data(client, REG_ENHANCE_ACOUSTICS1,
> +				  data->enh_acou[0]);
>  
>  	mutex_unlock(&data->lock);
>  
> @@ -1028,6 +1069,8 @@ static SENSOR_DEVICE_ATTR_2(pwm1_auto_point1_pwm, S_IRUGO | S_IWUSR, show_pwm,
>  			    set_pwm, MIN, 0);
>  static SENSOR_DEVICE_ATTR_2(pwm1_auto_point2_pwm, S_IRUGO | S_IWUSR, show_pwm,
>  			    set_pwm, MAX, 0);
> +static SENSOR_DEVICE_ATTR_2(pwm1_stall_dis, S_IRUGO | S_IWUSR, show_stall_dis,
> +			    set_stall_dis, 0, 0);
>  static SENSOR_DEVICE_ATTR_2(pwm2, S_IRUGO | S_IWUSR, show_pwm, set_pwm, INPUT,
>  			    1);
>  static SENSOR_DEVICE_ATTR_2(pwm2_freq, S_IRUGO | S_IWUSR, show_pwmfreq,
> @@ -1040,6 +1083,8 @@ static SENSOR_DEVICE_ATTR_2(pwm2_auto_point1_pwm, S_IRUGO | S_IWUSR, show_pwm,
>  			    set_pwm, MIN, 1);
>  static SENSOR_DEVICE_ATTR_2(pwm2_auto_point2_pwm, S_IRUGO | S_IWUSR, show_pwm,
>  			    set_pwm, MAX, 1);
> +static SENSOR_DEVICE_ATTR_2(pwm2_stall_dis, S_IRUGO | S_IWUSR, show_stall_dis,
> +			    set_stall_dis, 0, 1);
>  static SENSOR_DEVICE_ATTR_2(pwm3, S_IRUGO | S_IWUSR, show_pwm, set_pwm, INPUT,
>  			    2);
>  static SENSOR_DEVICE_ATTR_2(pwm3_freq, S_IRUGO | S_IWUSR, show_pwmfreq,
> @@ -1052,6 +1097,8 @@ static SENSOR_DEVICE_ATTR_2(pwm3_auto_point1_pwm, S_IRUGO | S_IWUSR, show_pwm,
>  			    set_pwm, MIN, 2);
>  static SENSOR_DEVICE_ATTR_2(pwm3_auto_point2_pwm, S_IRUGO | S_IWUSR, show_pwm,
>  			    set_pwm, MAX, 2);
> +static SENSOR_DEVICE_ATTR_2(pwm3_stall_dis, S_IRUGO | S_IWUSR, show_stall_dis,
> +			    set_stall_dis, 0, 2);
>  
>  /* Non-standard name, might need revisiting */
>  static DEVICE_ATTR_RW(pwm_use_point2_pwm_at_crit);
> @@ -1112,12 +1159,14 @@ static struct attribute *adt7475_attrs[] = {
>  	&sensor_dev_attr_pwm1_auto_channels_temp.dev_attr.attr,
>  	&sensor_dev_attr_pwm1_auto_point1_pwm.dev_attr.attr,
>  	&sensor_dev_attr_pwm1_auto_point2_pwm.dev_attr.attr,
> +	&sensor_dev_attr_pwm1_stall_dis.dev_attr.attr,
>  	&sensor_dev_attr_pwm3.dev_attr.attr,
>  	&sensor_dev_attr_pwm3_freq.dev_attr.attr,
>  	&sensor_dev_attr_pwm3_enable.dev_attr.attr,
>  	&sensor_dev_attr_pwm3_auto_channels_temp.dev_attr.attr,
>  	&sensor_dev_attr_pwm3_auto_point1_pwm.dev_attr.attr,
>  	&sensor_dev_attr_pwm3_auto_point2_pwm.dev_attr.attr,
> +	&sensor_dev_attr_pwm3_stall_dis.dev_attr.attr,
>  	&dev_attr_pwm_use_point2_pwm_at_crit.attr,
>  	NULL,
>  };
> @@ -1136,6 +1185,7 @@ static struct attribute *pwm2_attrs[] = {
>  	&sensor_dev_attr_pwm2_auto_channels_temp.dev_attr.attr,
>  	&sensor_dev_attr_pwm2_auto_point1_pwm.dev_attr.attr,
>  	&sensor_dev_attr_pwm2_auto_point2_pwm.dev_attr.attr,
> +	&sensor_dev_attr_pwm2_stall_dis.dev_attr.attr,
>  	NULL
>  };
>  
> -- 
> 2.11.0.24.ge6920cf
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH v2 3/3] hwmon: (adt7475) temperature smoothing
  2017-05-03  0:40 ` [RFC PATCH v2 3/3] hwmon: (adt7475) temperature smoothing Chris Packham
  2017-05-03  8:14   ` Chris Packham
@ 2017-05-03 16:30   ` Guenter Roeck
  2017-05-04  1:28     ` Chris Packham
  1 sibling, 1 reply; 9+ messages in thread
From: Guenter Roeck @ 2017-05-03 16:30 UTC (permalink / raw)
  To: Chris Packham
  Cc: linux-hwmon, jdelvare, Jonathan Corbet, linux-doc, linux-kernel

On Wed, May 03, 2017 at 12:40:09PM +1200, Chris Packham wrote:
> When enabled temperature smoothing allows ramping the fan speed over a
> configurable period of time instead of jumping to the new speed
> instantaneously.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> Changes in v2:
> - use a single tempN_smoothing attribute
> 
>  Documentation/hwmon/adt7475 |  4 ++
>  drivers/hwmon/adt7475.c     | 99 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 103 insertions(+)
> 
> diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
> index 63507402cd4f..dd6433d819d0 100644
> --- a/Documentation/hwmon/adt7475
> +++ b/Documentation/hwmon/adt7475
> @@ -114,6 +114,10 @@ minimum (i.e. auto_point1_pwm). This behaviour be configured using the
>  pwm[1-*]_stall_dis sysfs attribute. A value of 0 means the fans will shut off.
>  A value of 1 means the fans will run at auto_point1_pwm.
>  
> +The responsiveness of the ADT747x to temperature changes can be configured.
> +This allows smoothing of the fan speed transition. To set the transition time
> +set the value in ms in the temp[1-*]_smoothing sysfs attribute.
> +
>  Notes
>  -----
>  
> diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
> index 85957324cd85..41342de6e20c 100644
> --- a/drivers/hwmon/adt7475.c
> +++ b/drivers/hwmon/adt7475.c
> @@ -526,6 +526,96 @@ static ssize_t set_temp(struct device *dev, struct device_attribute *attr,
>  	return count;
>  }
>  
> +/* Assuming CONFIG6[SLOW] is 0 */

Can you take that into account and calculate a "best fit" based on the
available options ?

> +static const int ad7475_st_map[] = {
> +	37500, 18800, 12500, 7500, 4700, 3100, 1600, 800,
> +};
> +
> +static ssize_t show_temp_st(struct device *dev, struct device_attribute *attr,
> +				  char *buf)
> +{
> +	struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct adt7475_data *data = i2c_get_clientdata(client);
> +	int shift, idx;
> +	long val;
> +
> +	switch (sattr->index) {
> +	case 0:
> +		shift = 0;
> +		idx = 0;
> +		break;
> +	case 1:
> +		shift = 4;
> +		idx = 1;
> +		break;
> +	case 2:
> +		shift = 0;
> +		idx = 1;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	val = data->enh_acou[idx] >> shift;
> +	if (val & 0x8) {
> +		return sprintf(buf, "%d\n", ad7475_st_map[val & 0x7]);
> +	} else {
> +		return sprintf(buf, "0\n");
> +	}
> +}
> +
> +static ssize_t set_temp_st(struct device *dev, struct device_attribute *attr,
> +				 const char *buf, size_t count)
> +{
> +	struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
> +	struct i2c_client *client = to_i2c_client(dev);
> +	struct adt7475_data *data = i2c_get_clientdata(client);
> +	unsigned char reg;
> +	int shift, idx;
> +	ulong val;
> +
> +	if (kstrtoul(buf, 10, &val))
> +		return -EINVAL;
> +
> +	switch (sattr->index) {
> +	case 0:
> +		reg = REG_ENHANCE_ACOUSTICS1;
> +		shift = 0;
> +		idx = 0;
> +		break;
> +	case 1:
> +		reg = REG_ENHANCE_ACOUSTICS2;
> +		shift = 4;
> +		idx = 1;
> +		break;
> +	case 2:
> +		reg = REG_ENHANCE_ACOUSTICS2;
> +		shift = 0;
> +		idx = 1;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	if (val > 0) {
> +		val = find_closest_descending(val, ad7475_st_map,
> +					      ARRAY_SIZE(ad7475_st_map));
> +		val |= 0x8;
> +	}
> +
> +	mutex_lock(&data->lock);
> +
> +	data->enh_acou[idx] &= ~(0xf << shift);
> +	data->enh_acou[idx] |= (val << shift);
> +
> +	i2c_smbus_write_byte_data(client, reg, data->enh_acou[idx]);
> +
> +	mutex_unlock(&data->lock);
> +
> +	return count;
> +}
> +
>  /*
>   * Table of autorange values - the user will write the value in millidegrees,
>   * and we'll convert it
> @@ -1008,6 +1098,8 @@ static SENSOR_DEVICE_ATTR_2(temp1_crit, S_IRUGO | S_IWUSR, show_temp, set_temp,
>  			    THERM, 0);
>  static SENSOR_DEVICE_ATTR_2(temp1_crit_hyst, S_IRUGO | S_IWUSR, show_temp,
>  			    set_temp, HYSTERSIS, 0);
> +static SENSOR_DEVICE_ATTR_2(temp1_smoothing, S_IRUGO | S_IWUSR, show_temp_st,
> +			    set_temp_st, 0, 0);
>  static SENSOR_DEVICE_ATTR_2(temp2_input, S_IRUGO, show_temp, NULL, INPUT, 1);
>  static SENSOR_DEVICE_ATTR_2(temp2_alarm, S_IRUGO, show_temp, NULL, ALARM, 1);
>  static SENSOR_DEVICE_ATTR_2(temp2_max, S_IRUGO | S_IWUSR, show_temp, set_temp,
> @@ -1024,6 +1116,8 @@ static SENSOR_DEVICE_ATTR_2(temp2_crit, S_IRUGO | S_IWUSR, show_temp, set_temp,
>  			    THERM, 1);
>  static SENSOR_DEVICE_ATTR_2(temp2_crit_hyst, S_IRUGO | S_IWUSR, show_temp,
>  			    set_temp, HYSTERSIS, 1);
> +static SENSOR_DEVICE_ATTR_2(temp2_smoothing, S_IRUGO | S_IWUSR, show_temp_st,
> +			    set_temp_st, 0, 1);
>  static SENSOR_DEVICE_ATTR_2(temp3_input, S_IRUGO, show_temp, NULL, INPUT, 2);
>  static SENSOR_DEVICE_ATTR_2(temp3_alarm, S_IRUGO, show_temp, NULL, ALARM, 2);
>  static SENSOR_DEVICE_ATTR_2(temp3_fault, S_IRUGO, show_temp, NULL, FAULT, 2);
> @@ -1041,6 +1135,8 @@ static SENSOR_DEVICE_ATTR_2(temp3_crit, S_IRUGO | S_IWUSR, show_temp, set_temp,
>  			    THERM, 2);
>  static SENSOR_DEVICE_ATTR_2(temp3_crit_hyst, S_IRUGO | S_IWUSR, show_temp,
>  			    set_temp, HYSTERSIS, 2);
> +static SENSOR_DEVICE_ATTR_2(temp3_smoothing, S_IRUGO | S_IWUSR, show_temp_st,
> +			    set_temp_st, 0, 2);
>  static SENSOR_DEVICE_ATTR_2(fan1_input, S_IRUGO, show_tach, NULL, INPUT, 0);
>  static SENSOR_DEVICE_ATTR_2(fan1_min, S_IRUGO | S_IWUSR, show_tach, set_tach,
>  			    MIN, 0);
> @@ -1125,6 +1221,7 @@ static struct attribute *adt7475_attrs[] = {
>  	&sensor_dev_attr_temp1_auto_point2_temp.dev_attr.attr,
>  	&sensor_dev_attr_temp1_crit.dev_attr.attr,
>  	&sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
> +	&sensor_dev_attr_temp1_smoothing.dev_attr.attr,
>  	&sensor_dev_attr_temp2_input.dev_attr.attr,
>  	&sensor_dev_attr_temp2_alarm.dev_attr.attr,
>  	&sensor_dev_attr_temp2_max.dev_attr.attr,
> @@ -1134,6 +1231,7 @@ static struct attribute *adt7475_attrs[] = {
>  	&sensor_dev_attr_temp2_auto_point2_temp.dev_attr.attr,
>  	&sensor_dev_attr_temp2_crit.dev_attr.attr,
>  	&sensor_dev_attr_temp2_crit_hyst.dev_attr.attr,
> +	&sensor_dev_attr_temp2_smoothing.dev_attr.attr,
>  	&sensor_dev_attr_temp3_input.dev_attr.attr,
>  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
>  	&sensor_dev_attr_temp3_alarm.dev_attr.attr,
> @@ -1144,6 +1242,7 @@ static struct attribute *adt7475_attrs[] = {
>  	&sensor_dev_attr_temp3_auto_point2_temp.dev_attr.attr,
>  	&sensor_dev_attr_temp3_crit.dev_attr.attr,
>  	&sensor_dev_attr_temp3_crit_hyst.dev_attr.attr,
> +	&sensor_dev_attr_temp3_smoothing.dev_attr.attr,
>  	&sensor_dev_attr_fan1_input.dev_attr.attr,
>  	&sensor_dev_attr_fan1_min.dev_attr.attr,
>  	&sensor_dev_attr_fan1_alarm.dev_attr.attr,
> -- 
> 2.11.0.24.ge6920cf
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH v2 2/3] hwmon: (adt7475) fan stall prevention
  2017-05-03 16:10   ` Guenter Roeck
@ 2017-05-03 20:44     ` Chris Packham
  0 siblings, 0 replies; 9+ messages in thread
From: Chris Packham @ 2017-05-03 20:44 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-hwmon, jdelvare, Jonathan Corbet, linux-doc, linux-kernel

On 04/05/17 04:10, Guenter Roeck wrote:
> On Wed, May 03, 2017 at 12:40:08PM +1200, Chris Packham wrote:
>> By default adt7475 will stop the fans (pwm duty cycle 0%) when the
>> temperature drops past Tmin - hysteresis. Some systems want to keep the
>> fans moving even when the temperature drops so add new sysfs attributes
>> that configure the enhanced acoustics min 1-3 which allows the fans to
>> run at the minimum configure pwm duty cycle.
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>> Changes in v2:
>> - use pwmN_stall_dis as the attribute name. I think this describes the purpose
>>   pretty well. I went with a new attribute instead of overloading
>>   pwmN_auto_point1_pwm so this doesn't affect existing users.
>>
>>  Documentation/hwmon/adt7475 |  5 +++++
>>  drivers/hwmon/adt7475.c     | 50 +++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 55 insertions(+)
>>
>> diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
>> index 0502f2b464e1..63507402cd4f 100644
>> --- a/Documentation/hwmon/adt7475
>> +++ b/Documentation/hwmon/adt7475
>> @@ -109,6 +109,11 @@ fan speed) is applied. PWM values range from 0 (off) to 255 (full speed).
>>  Fan speed may be set to maximum when the temperature sensor associated with
>>  the PWM control exceeds temp#_max.
>>
>> +At Tmin - hysteresis the PWM output can either be off (0% duty cycle) or at the
>> +minimum (i.e. auto_point1_pwm). This behaviour be configured using the
>> +pwm[1-*]_stall_dis sysfs attribute. A value of 0 means the fans will shut off.
>
> That is really an awkward attribute name. I'll have to think about this some
> more.

I agree. The other thing I considered was "halt" and using inverted 
logic so halt == 1 reflected the HW default of allowing fans to stop.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [RFC PATCH v2 3/3] hwmon: (adt7475) temperature smoothing
  2017-05-03 16:30   ` Guenter Roeck
@ 2017-05-04  1:28     ` Chris Packham
  0 siblings, 0 replies; 9+ messages in thread
From: Chris Packham @ 2017-05-04  1:28 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: linux-hwmon, jdelvare, Jonathan Corbet, linux-doc, linux-kernel

On 04/05/17 04:30, Guenter Roeck wrote:
> On Wed, May 03, 2017 at 12:40:09PM +1200, Chris Packham wrote:
>> When enabled temperature smoothing allows ramping the fan speed over a
>> configurable period of time instead of jumping to the new speed
>> instantaneously.
>>
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>> Changes in v2:
>> - use a single tempN_smoothing attribute
>>
>>  Documentation/hwmon/adt7475 |  4 ++
>>  drivers/hwmon/adt7475.c     | 99 +++++++++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 103 insertions(+)
>>
>> diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
>> index 63507402cd4f..dd6433d819d0 100644
>> --- a/Documentation/hwmon/adt7475
>> +++ b/Documentation/hwmon/adt7475
>> @@ -114,6 +114,10 @@ minimum (i.e. auto_point1_pwm). This behaviour be configured using the
>>  pwm[1-*]_stall_dis sysfs attribute. A value of 0 means the fans will shut off.
>>  A value of 1 means the fans will run at auto_point1_pwm.
>>
>> +The responsiveness of the ADT747x to temperature changes can be configured.
>> +This allows smoothing of the fan speed transition. To set the transition time
>> +set the value in ms in the temp[1-*]_smoothing sysfs attribute.
>> +
>>  Notes
>>  -----
>>
>> diff --git a/drivers/hwmon/adt7475.c b/drivers/hwmon/adt7475.c
>> index 85957324cd85..41342de6e20c 100644
>> --- a/drivers/hwmon/adt7475.c
>> +++ b/drivers/hwmon/adt7475.c
>> @@ -526,6 +526,96 @@ static ssize_t set_temp(struct device *dev, struct device_attribute *attr,
>>  	return count;
>>  }
>>
>> +/* Assuming CONFIG6[SLOW] is 0 */
>
> Can you take that into account and calculate a "best fit" based on the
> available options ?

Do you mean check CONFIG6[SLOW] and choose between 1 of 2 possible maps? 
Or have a unified map and choose both the SLOW and ACOU values?

I briefly considered the latter but things soon started to get 
complicated. It would look something like this (please excuse using a 
MUA as a code editor).

static const int ad7475_st_map[] = {
	52200, 37500, 26100, 18800, 17400, 12500, 10400, 7500,
	6500, 4700, 4400, 3100, 2200, 1600, 1100, 800,
};

i = find_closest_descending(val, ad7475_st_map,
			    ARRAY_SIZE(ad7475_st_map));
acou = i / 2;
slow = i % 2;

Going in reverse then gets really fun

slow = !!(i2c_smbus_read_byte_data(client, REG_CONFIG6) & BIT(PWMx))
acou = data->enh_acou[idx];

i = (acou * 2) + slow;

return sprintf(buf, "%d\n", ad7475_st_map[i]);

I could probably make the above work I just wasn't sure it was worth the 
hassle. Only the higher ranges would really be noticeable to anyone.

>
>> +static const int ad7475_st_map[] = {
>> +	37500, 18800, 12500, 7500, 4700, 3100, 1600, 800,
>> +};
>> +
>> +static ssize_t show_temp_st(struct device *dev, struct device_attribute *attr,
>> +				  char *buf)
>> +{
>> +	struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
>> +	struct i2c_client *client = to_i2c_client(dev);
>> +	struct adt7475_data *data = i2c_get_clientdata(client);
>> +	int shift, idx;
>> +	long val;
>> +
>> +	switch (sattr->index) {
>> +	case 0:
>> +		shift = 0;
>> +		idx = 0;
>> +		break;
>> +	case 1:
>> +		shift = 4;
>> +		idx = 1;
>> +		break;
>> +	case 2:
>> +		shift = 0;
>> +		idx = 1;
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	val = data->enh_acou[idx] >> shift;
>> +	if (val & 0x8) {
>> +		return sprintf(buf, "%d\n", ad7475_st_map[val & 0x7]);
>> +	} else {
>> +		return sprintf(buf, "0\n");
>> +	}
>> +}
>> +
>> +static ssize_t set_temp_st(struct device *dev, struct device_attribute *attr,
>> +				 const char *buf, size_t count)
>> +{
>> +	struct sensor_device_attribute_2 *sattr = to_sensor_dev_attr_2(attr);
>> +	struct i2c_client *client = to_i2c_client(dev);
>> +	struct adt7475_data *data = i2c_get_clientdata(client);
>> +	unsigned char reg;
>> +	int shift, idx;
>> +	ulong val;
>> +
>> +	if (kstrtoul(buf, 10, &val))
>> +		return -EINVAL;
>> +
>> +	switch (sattr->index) {
>> +	case 0:
>> +		reg = REG_ENHANCE_ACOUSTICS1;
>> +		shift = 0;
>> +		idx = 0;
>> +		break;
>> +	case 1:
>> +		reg = REG_ENHANCE_ACOUSTICS2;
>> +		shift = 4;
>> +		idx = 1;
>> +		break;
>> +	case 2:
>> +		reg = REG_ENHANCE_ACOUSTICS2;
>> +		shift = 0;
>> +		idx = 1;
>> +		break;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +
>> +	if (val > 0) {
>> +		val = find_closest_descending(val, ad7475_st_map,
>> +					      ARRAY_SIZE(ad7475_st_map));
>> +		val |= 0x8;
>> +	}
>> +
>> +	mutex_lock(&data->lock);
>> +
>> +	data->enh_acou[idx] &= ~(0xf << shift);
>> +	data->enh_acou[idx] |= (val << shift);
>> +
>> +	i2c_smbus_write_byte_data(client, reg, data->enh_acou[idx]);
>> +
>> +	mutex_unlock(&data->lock);
>> +
>> +	return count;
>> +}
>> +
>>  /*
>>   * Table of autorange values - the user will write the value in millidegrees,
>>   * and we'll convert it
>> @@ -1008,6 +1098,8 @@ static SENSOR_DEVICE_ATTR_2(temp1_crit, S_IRUGO | S_IWUSR, show_temp, set_temp,
>>  			    THERM, 0);
>>  static SENSOR_DEVICE_ATTR_2(temp1_crit_hyst, S_IRUGO | S_IWUSR, show_temp,
>>  			    set_temp, HYSTERSIS, 0);
>> +static SENSOR_DEVICE_ATTR_2(temp1_smoothing, S_IRUGO | S_IWUSR, show_temp_st,
>> +			    set_temp_st, 0, 0);
>>  static SENSOR_DEVICE_ATTR_2(temp2_input, S_IRUGO, show_temp, NULL, INPUT, 1);
>>  static SENSOR_DEVICE_ATTR_2(temp2_alarm, S_IRUGO, show_temp, NULL, ALARM, 1);
>>  static SENSOR_DEVICE_ATTR_2(temp2_max, S_IRUGO | S_IWUSR, show_temp, set_temp,
>> @@ -1024,6 +1116,8 @@ static SENSOR_DEVICE_ATTR_2(temp2_crit, S_IRUGO | S_IWUSR, show_temp, set_temp,
>>  			    THERM, 1);
>>  static SENSOR_DEVICE_ATTR_2(temp2_crit_hyst, S_IRUGO | S_IWUSR, show_temp,
>>  			    set_temp, HYSTERSIS, 1);
>> +static SENSOR_DEVICE_ATTR_2(temp2_smoothing, S_IRUGO | S_IWUSR, show_temp_st,
>> +			    set_temp_st, 0, 1);
>>  static SENSOR_DEVICE_ATTR_2(temp3_input, S_IRUGO, show_temp, NULL, INPUT, 2);
>>  static SENSOR_DEVICE_ATTR_2(temp3_alarm, S_IRUGO, show_temp, NULL, ALARM, 2);
>>  static SENSOR_DEVICE_ATTR_2(temp3_fault, S_IRUGO, show_temp, NULL, FAULT, 2);
>> @@ -1041,6 +1135,8 @@ static SENSOR_DEVICE_ATTR_2(temp3_crit, S_IRUGO | S_IWUSR, show_temp, set_temp,
>>  			    THERM, 2);
>>  static SENSOR_DEVICE_ATTR_2(temp3_crit_hyst, S_IRUGO | S_IWUSR, show_temp,
>>  			    set_temp, HYSTERSIS, 2);
>> +static SENSOR_DEVICE_ATTR_2(temp3_smoothing, S_IRUGO | S_IWUSR, show_temp_st,
>> +			    set_temp_st, 0, 2);
>>  static SENSOR_DEVICE_ATTR_2(fan1_input, S_IRUGO, show_tach, NULL, INPUT, 0);
>>  static SENSOR_DEVICE_ATTR_2(fan1_min, S_IRUGO | S_IWUSR, show_tach, set_tach,
>>  			    MIN, 0);
>> @@ -1125,6 +1221,7 @@ static struct attribute *adt7475_attrs[] = {
>>  	&sensor_dev_attr_temp1_auto_point2_temp.dev_attr.attr,
>>  	&sensor_dev_attr_temp1_crit.dev_attr.attr,
>>  	&sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
>> +	&sensor_dev_attr_temp1_smoothing.dev_attr.attr,
>>  	&sensor_dev_attr_temp2_input.dev_attr.attr,
>>  	&sensor_dev_attr_temp2_alarm.dev_attr.attr,
>>  	&sensor_dev_attr_temp2_max.dev_attr.attr,
>> @@ -1134,6 +1231,7 @@ static struct attribute *adt7475_attrs[] = {
>>  	&sensor_dev_attr_temp2_auto_point2_temp.dev_attr.attr,
>>  	&sensor_dev_attr_temp2_crit.dev_attr.attr,
>>  	&sensor_dev_attr_temp2_crit_hyst.dev_attr.attr,
>> +	&sensor_dev_attr_temp2_smoothing.dev_attr.attr,
>>  	&sensor_dev_attr_temp3_input.dev_attr.attr,
>>  	&sensor_dev_attr_temp3_fault.dev_attr.attr,
>>  	&sensor_dev_attr_temp3_alarm.dev_attr.attr,
>> @@ -1144,6 +1242,7 @@ static struct attribute *adt7475_attrs[] = {
>>  	&sensor_dev_attr_temp3_auto_point2_temp.dev_attr.attr,
>>  	&sensor_dev_attr_temp3_crit.dev_attr.attr,
>>  	&sensor_dev_attr_temp3_crit_hyst.dev_attr.attr,
>> +	&sensor_dev_attr_temp3_smoothing.dev_attr.attr,
>>  	&sensor_dev_attr_fan1_input.dev_attr.attr,
>>  	&sensor_dev_attr_fan1_min.dev_attr.attr,
>>  	&sensor_dev_attr_fan1_alarm.dev_attr.attr,
>> --
>> 2.11.0.24.ge6920cf
>>
>


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-05-04  1:28 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-03  0:40 [RFC PATCH v2 0/3] hwmon: (adt7475) small enhancements Chris Packham
2017-05-03  0:40 ` [RFC PATCH v2 1/3] hwmon: (adt7475) replace find_nearest() with find_closest() Chris Packham
2017-05-03  0:40 ` [RFC PATCH v2 2/3] hwmon: (adt7475) fan stall prevention Chris Packham
2017-05-03 16:10   ` Guenter Roeck
2017-05-03 20:44     ` Chris Packham
2017-05-03  0:40 ` [RFC PATCH v2 3/3] hwmon: (adt7475) temperature smoothing Chris Packham
2017-05-03  8:14   ` Chris Packham
2017-05-03 16:30   ` Guenter Roeck
2017-05-04  1:28     ` Chris Packham

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.