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@xxxxxxxxxxxxxxxxxxx> >> --- >> 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 >> > -- To unsubscribe from this list: send the line "unsubscribe linux-doc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html