diff mbox

[v4,2/3] hwmon: (adt7475) temperature smoothing

Message ID 20170515013029.31397-3-chris.packham@alliedtelesis.co.nz (mailing list archive)
State Accepted
Headers show

Commit Message

Chris Packham May 15, 2017, 1:30 a.m. UTC 
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
Changes in v3:
- change enh_acou to enh_acoustics
- simplify show_temp_st()
Changes in v4:
- removed dead code.
- Make the order of the smoothing attributes match the other temperature
  attributes.

Guenter,

We'd previously discussed making the smoothing values set CONFIG6[SLOW] to
expose the other set of potential values. I wasn't sure where you wanted to go
on that one.

Personally I was on the fence since the difference would only be noticeable for
the higher values. If we do want to add support for the other values it could
be done as a subsequent patch (or a v5 if you want it).

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

Comments

Guenter Roeck May 15, 2017, 1:18 p.m. UTC | #1 
On 05/14/2017 06:30 PM, 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>

Applied to -next.

> ---
> Changes in v2:
> - use a single tempN_smoothing attribute
> Changes in v3:
> - change enh_acou to enh_acoustics
> - simplify show_temp_st()
> Changes in v4:
> - removed dead code.
> - Make the order of the smoothing attributes match the other temperature
>   attributes.
>
> Guenter,
>
> We'd previously discussed making the smoothing values set CONFIG6[SLOW] to
> expose the other set of potential values. I wasn't sure where you wanted to go
> on that one.
>
> Personally I was on the fence since the difference would only be noticeable for
> the higher values. If we do want to add support for the other values it could
> be done as a subsequent patch (or a v5 if you want it).
>
It can be added later if anyone cares.

Thanks,
Guenter

>  Documentation/hwmon/adt7475 |  4 ++
>  drivers/hwmon/adt7475.c     | 91 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 95 insertions(+)
>
> diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
> index dc0b55794c47..09d73a10644c 100644
> --- a/Documentation/hwmon/adt7475
> +++ b/Documentation/hwmon/adt7475
> @@ -114,6 +114,10 @@ minimum (i.e. auto_point1_pwm). This behaviour can be configured using the
>  pwm[1-*]_stall_disable 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 3eb8c5c2f8af..3056076fae27 100644
> --- a/drivers/hwmon/adt7475.c
> +++ b/drivers/hwmon/adt7475.c
> @@ -526,6 +526,88 @@ 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);
> +	long val;
> +
> +	switch (sattr->index) {
> +	case 0:
> +		val = data->enh_acoustics[0] & 0xf;
> +		break;
> +	case 1:
> +		val = (data->enh_acoustics[1] >> 4) & 0xf;
> +		break;
> +	case 2:
> +	default:
> +		val = data->enh_acoustics[1] & 0xf;
> +		break;
> +	}
> +
> +	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 = 0;
> +		idx = 1;
> +		break;
> +	case 2:
> +	default:
> +		reg = REG_ENHANCE_ACOUSTICS2;
> +		shift = 4;
> +		idx = 1;
> +		break;
> +	}
> +
> +	if (val > 0) {
> +		val = find_closest_descending(val, ad7475_st_map,
> +					      ARRAY_SIZE(ad7475_st_map));
> +		val |= 0x8;
> +	}
> +
> +	mutex_lock(&data->lock);
> +
> +	data->enh_acoustics[idx] &= ~(0xf << shift);
> +	data->enh_acoustics[idx] |= (val << shift);
> +
> +	i2c_smbus_write_byte_data(client, reg, data->enh_acoustics[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 +1090,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 +1108,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 +1127,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 +1213,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 +1223,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 +1234,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,
>

--
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
kernel test robot May 16, 2017, 8:17 a.m. UTC | #2 
Hi Chris,

[auto build test ERROR on hwmon/hwmon-next]
[also build test ERROR on v4.12-rc1 next-20170516]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Chris-Packham/hwmon-adt7475-fan-stall-prevention/20170515-093530
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
config: x86_64-rhel (attached as .config)
compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All errors (new ones prefixed by >>):

   drivers/hwmon/adt7475.c: In function 'set_temp_st':
>> drivers/hwmon/adt7475.c:622:9: error: implicit declaration of function 'find_closest_descending' [-Werror=implicit-function-declaration]
      val = find_closest_descending(val, ad7475_st_map,
            ^~~~~~~~~~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/find_closest_descending +622 drivers/hwmon/adt7475.c

   616			shift = 4;
   617			idx = 1;
   618			break;
   619		}
   620	
   621		if (val > 0) {
 > 622			val = find_closest_descending(val, ad7475_st_map,
   623						      ARRAY_SIZE(ad7475_st_map));
   624			val |= 0x8;
   625		}

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Chris Packham May 16, 2017, 8:30 p.m. UTC | #3 
On 16/05/17 20:23, kbuild test robot wrote:
> Hi Chris,
>
> [auto build test ERROR on hwmon/hwmon-next]
> [also build test ERROR on v4.12-rc1 next-20170516]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>
> url:    https://github.com/0day-ci/linux/commits/Chris-Packham/hwmon-adt7475-fan-stall-prevention/20170515-093530
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
> config: x86_64-rhel (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=x86_64
>
> All errors (new ones prefixed by >>):
>
>    drivers/hwmon/adt7475.c: In function 'set_temp_st':
>>> drivers/hwmon/adt7475.c:622:9: error: implicit declaration of function 'find_closest_descending' [-Werror=implicit-function-declaration]
>       val = find_closest_descending(val, ad7475_st_map,
>             ^~~~~~~~~~~~~~~~~~~~~~~
>    cc1: some warnings being treated as errors
>
> vim +/find_closest_descending +622 drivers/hwmon/adt7475.c
>
>    616			shift = 4;
>    617			idx = 1;
>    618			break;
>    619		}
>    620	
>    621		if (val > 0) {
>  > 622			val = find_closest_descending(val, ad7475_st_map,
>    623						      ARRAY_SIZE(ad7475_st_map));
>    624			val |= 0x8;
>    625		}
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>

I'm not sure how this is failing. find_closest_descending() is a macro 
defined in linux/util_macros.h which is explicitly included in 
drivers/hwmon/adt7475.c. Aside from the include guards there's nothing 
conditional about it.
--
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
Guenter Roeck May 16, 2017, 10 p.m. UTC | #4 
On Tue, May 16, 2017 at 08:30:52PM +0000, Chris Packham wrote:
> On 16/05/17 20:23, kbuild test robot wrote:
> > Hi Chris,
> >
> > [auto build test ERROR on hwmon/hwmon-next]
> > [also build test ERROR on v4.12-rc1 next-20170516]
> > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> >
> > url:    https://github.com/0day-ci/linux/commits/Chris-Packham/hwmon-adt7475-fan-stall-prevention/20170515-093530
> > base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
> > config: x86_64-rhel (attached as .config)
> > compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> > reproduce:
> >         # save the attached .config to linux build tree
> >         make ARCH=x86_64
> >
> > All errors (new ones prefixed by >>):
> >
> >    drivers/hwmon/adt7475.c: In function 'set_temp_st':
> >>> drivers/hwmon/adt7475.c:622:9: error: implicit declaration of function 'find_closest_descending' [-Werror=implicit-function-declaration]
> >       val = find_closest_descending(val, ad7475_st_map,
> >             ^~~~~~~~~~~~~~~~~~~~~~~
> >    cc1: some warnings being treated as errors
> >
> > vim +/find_closest_descending +622 drivers/hwmon/adt7475.c
> >
> >    616			shift = 4;
> >    617			idx = 1;
> >    618			break;
> >    619		}
> >    620	
> >    621		if (val > 0) {
> >  > 622			val = find_closest_descending(val, ad7475_st_map,
> >    623						      ARRAY_SIZE(ad7475_st_map));
> >    624			val |= 0x8;
> >    625		}
> >
> > ---
> > 0-DAY kernel test infrastructure                Open Source Technology Center
> > https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
> >
> 
> I'm not sure how this is failing. find_closest_descending() is a macro 
> defined in linux/util_macros.h which is explicitly included in 
> drivers/hwmon/adt7475.c. Aside from the include guards there's nothing 
> conditional about it.

I suspect it was tested out of sequence, without the preceding patches,
and the header file include was missing.

Guenter
--
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
Ye Xiaolong May 17, 2017, 3:06 a.m. UTC | #5 
On 05/16, Chris Packham wrote:
>On 16/05/17 20:23, kbuild test robot wrote:
>> Hi Chris,
>>
>> [auto build test ERROR on hwmon/hwmon-next]
>> [also build test ERROR on v4.12-rc1 next-20170516]
>> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>>
>> url:    https://github.com/0day-ci/linux/commits/Chris-Packham/hwmon-adt7475-fan-stall-prevention/20170515-093530
>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
>> config: x86_64-rhel (attached as .config)
>> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
>> reproduce:
>>         # save the attached .config to linux build tree
>>         make ARCH=x86_64
>>
>> All errors (new ones prefixed by >>):
>>
>>    drivers/hwmon/adt7475.c: In function 'set_temp_st':
>>>> drivers/hwmon/adt7475.c:622:9: error: implicit declaration of function 'find_closest_descending' [-Werror=implicit-function-declaration]
>>       val = find_closest_descending(val, ad7475_st_map,
>>             ^~~~~~~~~~~~~~~~~~~~~~~
>>    cc1: some warnings being treated as errors
>>
>> vim +/find_closest_descending +622 drivers/hwmon/adt7475.c
>>
>>    616			shift = 4;
>>    617			idx = 1;
>>    618			break;
>>    619		}
>>    620	
>>    621		if (val > 0) {
>>  > 622			val = find_closest_descending(val, ad7475_st_map,
>>    623						      ARRAY_SIZE(ad7475_st_map));
>>    624			val |= 0x8;
>>    625		}
>>
>> ---
>> 0-DAY kernel test infrastructure                Open Source Technology Center
>> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>>
>
>I'm not sure how this is failing. find_closest_descending() is a macro 
>defined in linux/util_macros.h which is explicitly included in 
>drivers/hwmon/adt7475.c. Aside from the include guards there's nothing 
>conditional about it.

Hi,

0day bot applied your patchset on top of commit 6eaaea1 ("hwmon: (pmbus) Add client driver for IR35221"),
is it wrong or you have some prerequisite patches?

Thanks,
Xiaolong
>_______________________________________________
>kbuild-all mailing list
>kbuild-all@lists.01.org
>https://lists.01.org/mailman/listinfo/kbuild-all
--
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
Chris Packham May 17, 2017, 3:26 a.m. UTC | #6 
On 17/05/17 15:09, Ye Xiaolong wrote:
> On 05/16, Chris Packham wrote:
>> On 16/05/17 20:23, kbuild test robot wrote:
>>> Hi Chris,
>>>
>>> [auto build test ERROR on hwmon/hwmon-next]
>>> [also build test ERROR on v4.12-rc1 next-20170516]
>>> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>>>
>>> url:    https://github.com/0day-ci/linux/commits/Chris-Packham/hwmon-adt7475-fan-stall-prevention/20170515-093530
>>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
>>> config: x86_64-rhel (attached as .config)
>>> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
>>> reproduce:
>>>         # save the attached .config to linux build tree
>>>         make ARCH=x86_64
>>>
>>> All errors (new ones prefixed by >>):
>>>
>>>    drivers/hwmon/adt7475.c: In function 'set_temp_st':
>>>>> drivers/hwmon/adt7475.c:622:9: error: implicit declaration of function 'find_closest_descending' [-Werror=implicit-function-declaration]
>>>       val = find_closest_descending(val, ad7475_st_map,
>>>             ^~~~~~~~~~~~~~~~~~~~~~~
>>>    cc1: some warnings being treated as errors
>>>
>>> vim +/find_closest_descending +622 drivers/hwmon/adt7475.c
>>>
>>>    616			shift = 4;
>>>    617			idx = 1;
>>>    618			break;
>>>    619		}
>>>    620	
>>>    621		if (val > 0) {
>>>  > 622			val = find_closest_descending(val, ad7475_st_map,
>>>    623						      ARRAY_SIZE(ad7475_st_map));
>>>    624			val |= 0x8;
>>>    625		}
>>>
>>> ---
>>> 0-DAY kernel test infrastructure                Open Source Technology Center
>>> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>>>
>>
>> I'm not sure how this is failing. find_closest_descending() is a macro
>> defined in linux/util_macros.h which is explicitly included in
>> drivers/hwmon/adt7475.c. Aside from the include guards there's nothing
>> conditional about it.
>
> Hi,
>
> 0day bot applied your patchset on top of commit 6eaaea1 ("hwmon: (pmbus) Add client driver for IR35221"),
> is it wrong or you have some prerequisite patches?
>

Looks like it's missing 
https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git/commit/?h=hwmon-next&id=bbb4dd0ff 
which was part of the series but was applied after v3 so I didn't send 
it out with v4.
--
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
Ye Xiaolong May 17, 2017, 4:39 a.m. UTC | #7 
On 05/17, Chris Packham wrote:
>On 17/05/17 15:09, Ye Xiaolong wrote:
>> On 05/16, Chris Packham wrote:
>>> On 16/05/17 20:23, kbuild test robot wrote:
>>>> Hi Chris,
>>>>
>>>> [auto build test ERROR on hwmon/hwmon-next]
>>>> [also build test ERROR on v4.12-rc1 next-20170516]
>>>> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>>>>
>>>> url:    https://github.com/0day-ci/linux/commits/Chris-Packham/hwmon-adt7475-fan-stall-prevention/20170515-093530
>>>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
>>>> config: x86_64-rhel (attached as .config)
>>>> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
>>>> reproduce:
>>>>         # save the attached .config to linux build tree
>>>>         make ARCH=x86_64
>>>>
>>>> All errors (new ones prefixed by >>):
>>>>
>>>>    drivers/hwmon/adt7475.c: In function 'set_temp_st':
>>>>>> drivers/hwmon/adt7475.c:622:9: error: implicit declaration of function 'find_closest_descending' [-Werror=implicit-function-declaration]
>>>>       val = find_closest_descending(val, ad7475_st_map,
>>>>             ^~~~~~~~~~~~~~~~~~~~~~~
>>>>    cc1: some warnings being treated as errors
>>>>
>>>> vim +/find_closest_descending +622 drivers/hwmon/adt7475.c
>>>>
>>>>    616			shift = 4;
>>>>    617			idx = 1;
>>>>    618			break;
>>>>    619		}
>>>>    620	
>>>>    621		if (val > 0) {
>>>>  > 622			val = find_closest_descending(val, ad7475_st_map,
>>>>    623						      ARRAY_SIZE(ad7475_st_map));
>>>>    624			val |= 0x8;
>>>>    625		}
>>>>
>>>> ---
>>>> 0-DAY kernel test infrastructure                Open Source Technology Center
>>>> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>>>>
>>>
>>> I'm not sure how this is failing. find_closest_descending() is a macro
>>> defined in linux/util_macros.h which is explicitly included in
>>> drivers/hwmon/adt7475.c. Aside from the include guards there's nothing
>>> conditional about it.
>>
>> Hi,
>>
>> 0day bot applied your patchset on top of commit 6eaaea1 ("hwmon: (pmbus) Add client driver for IR35221"),
>> is it wrong or you have some prerequisite patches?

Thanks for the info, seems we need to improve the kbuild bot by pulling the
latest tree before applying new patches.

Thanks,
Xiaolong
>>
>
>Looks like it's missing 
>https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git/commit/?h=hwmon-next&id=bbb4dd0ff 
>which was part of the series but was applied after v3 so I didn't send 
>it out with v4.
--
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
Guenter Roeck May 17, 2017, 5:24 a.m. UTC | #8 
On 05/16/2017 09:39 PM, Ye Xiaolong wrote:
> On 05/17, Chris Packham wrote:
>> On 17/05/17 15:09, Ye Xiaolong wrote:
>>> On 05/16, Chris Packham wrote:
>>>> On 16/05/17 20:23, kbuild test robot wrote:
>>>>> Hi Chris,
>>>>>
>>>>> [auto build test ERROR on hwmon/hwmon-next]
>>>>> [also build test ERROR on v4.12-rc1 next-20170516]
>>>>> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
>>>>>
>>>>> url:    https://github.com/0day-ci/linux/commits/Chris-Packham/hwmon-adt7475-fan-stall-prevention/20170515-093530
>>>>> base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
>>>>> config: x86_64-rhel (attached as .config)
>>>>> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
>>>>> reproduce:
>>>>>         # save the attached .config to linux build tree
>>>>>         make ARCH=x86_64
>>>>>
>>>>> All errors (new ones prefixed by >>):
>>>>>
>>>>>    drivers/hwmon/adt7475.c: In function 'set_temp_st':
>>>>>>> drivers/hwmon/adt7475.c:622:9: error: implicit declaration of function 'find_closest_descending' [-Werror=implicit-function-declaration]
>>>>>       val = find_closest_descending(val, ad7475_st_map,
>>>>>             ^~~~~~~~~~~~~~~~~~~~~~~
>>>>>    cc1: some warnings being treated as errors
>>>>>
>>>>> vim +/find_closest_descending +622 drivers/hwmon/adt7475.c
>>>>>
>>>>>    616			shift = 4;
>>>>>    617			idx = 1;
>>>>>    618			break;
>>>>>    619		}
>>>>>    620	
>>>>>    621		if (val > 0) {
>>>>>  > 622			val = find_closest_descending(val, ad7475_st_map,
>>>>>    623						      ARRAY_SIZE(ad7475_st_map));
>>>>>    624			val |= 0x8;
>>>>>    625		}
>>>>>
>>>>> ---
>>>>> 0-DAY kernel test infrastructure                Open Source Technology Center
>>>>> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
>>>>>
>>>>
>>>> I'm not sure how this is failing. find_closest_descending() is a macro
>>>> defined in linux/util_macros.h which is explicitly included in
>>>> drivers/hwmon/adt7475.c. Aside from the include guards there's nothing
>>>> conditional about it.
>>>
>>> Hi,
>>>
>>> 0day bot applied your patchset on top of commit 6eaaea1 ("hwmon: (pmbus) Add client driver for IR35221"),
>>> is it wrong or you have some prerequisite patches?
>
> Thanks for the info, seems we need to improve the kbuild bot by pulling the
> latest tree before applying new patches.
>

No worries. I don't mind the occasional false positive. Better a false positive than a missed bug.

Guenter

> Thanks,
> Xiaolong
>>>
>>
>> Looks like it's missing
>> https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git/commit/?h=hwmon-next&id=bbb4dd0ff
>> which was part of the series but was applied after v3 so I didn't send
>> it out with v4.
>

--
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
Ye Xiaolong May 18, 2017, 3:05 a.m. UTC | #9 
On 05/16, Guenter Roeck wrote:
>>>>0day bot applied your patchset on top of commit 6eaaea1 ("hwmon: (pmbus) Add client driver for IR35221"),
>>>>is it wrong or you have some prerequisite patches?
>>
>>Thanks for the info, seems we need to improve the kbuild bot by pulling the
>>latest tree before applying new patches.
>>
>
>No worries. I don't mind the occasional false positive. Better a false positive than a missed bug.
>

Just curiosity, I noticed you committed the missing patch bbb4dd0fff ("hwmon:
(adt7475) replace find_nearest() with find_closest()") at May 14, did you push
it to git.kernel.org immediately or sometime after that? Since 0day makes
mirror based on repos in git.kernel.org in this case.

Thanks,
Xiaolong

>Guenter
>
>>Thanks,
>>Xiaolong
>>>>
>>>
>>>Looks like it's missing
>>>https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git/commit/?h=hwmon-next&id=bbb4dd0ff
>>>which was part of the series but was applied after v3 so I didn't send
>>>it out with v4.
>>
>
--
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
Guenter Roeck May 18, 2017, 4:09 a.m. UTC | #10 
On 05/17/2017 08:05 PM, Ye Xiaolong wrote:
> On 05/16, Guenter Roeck wrote:
>>>>> 0day bot applied your patchset on top of commit 6eaaea1 ("hwmon: (pmbus) Add client driver for IR35221"),
>>>>> is it wrong or you have some prerequisite patches?
>>>
>>> Thanks for the info, seems we need to improve the kbuild bot by pulling the
>>> latest tree before applying new patches.
>>>
>>
>> No worries. I don't mind the occasional false positive. Better a false positive than a missed bug.
>>
>
> Just curiosity, I noticed you committed the missing patch bbb4dd0fff ("hwmon:
> (adt7475) replace find_nearest() with find_closest()") at May 14, did you push
> it to git.kernel.org immediately or sometime after that? Since 0day makes
> mirror based on repos in git.kernel.org in this case.
>

I don't recall, sorry. Most likely I didn't push it immediately.

Guenter

--
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
diff mbox

Patch

diff --git a/Documentation/hwmon/adt7475 b/Documentation/hwmon/adt7475
index dc0b55794c47..09d73a10644c 100644
--- a/Documentation/hwmon/adt7475
+++ b/Documentation/hwmon/adt7475
@@ -114,6 +114,10 @@  minimum (i.e. auto_point1_pwm). This behaviour can be configured using the
 pwm[1-*]_stall_disable 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 3eb8c5c2f8af..3056076fae27 100644
--- a/drivers/hwmon/adt7475.c
+++ b/drivers/hwmon/adt7475.c
@@ -526,6 +526,88 @@  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);
+	long val;
+
+	switch (sattr->index) {
+	case 0:
+		val = data->enh_acoustics[0] & 0xf;
+		break;
+	case 1:
+		val = (data->enh_acoustics[1] >> 4) & 0xf;
+		break;
+	case 2:
+	default:
+		val = data->enh_acoustics[1] & 0xf;
+		break;
+	}
+
+	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 = 0;
+		idx = 1;
+		break;
+	case 2:
+	default:
+		reg = REG_ENHANCE_ACOUSTICS2;
+		shift = 4;
+		idx = 1;
+		break;
+	}
+
+	if (val > 0) {
+		val = find_closest_descending(val, ad7475_st_map,
+					      ARRAY_SIZE(ad7475_st_map));
+		val |= 0x8;
+	}
+
+	mutex_lock(&data->lock);
+
+	data->enh_acoustics[idx] &= ~(0xf << shift);
+	data->enh_acoustics[idx] |= (val << shift);
+
+	i2c_smbus_write_byte_data(client, reg, data->enh_acoustics[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 +1090,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 +1108,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 +1127,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 +1213,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 +1223,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 +1234,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,