* Re: [lm-sensors] [PATCH] hwmon: (w83627ehf) Disable setting DC mode for pwm2, pwm3 on NCT6776F
2012-01-28 1:57 [lm-sensors] [PATCH] hwmon: (w83627ehf) Disable setting DC mode for pwm2, pwm3 on NCT6776F Guenter Roeck
@ 2012-01-28 8:49 ` Jean Delvare
2012-01-28 12:25 ` CC
` (4 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: Jean Delvare @ 2012-01-28 8:49 UTC (permalink / raw)
To: lm-sensors
On Fri, 27 Jan 2012 17:57:56 -0800, Guenter Roeck wrote:
> NCT6776F only supports pwm mode for pwm2 and pwm3. Return error if an attempt
> is made to set those pwm channels to DC mode.
>
> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
> ---
> drivers/hwmon/w83627ehf.c | 6 ++++++
> 1 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/hwmon/w83627ehf.c b/drivers/hwmon/w83627ehf.c
> index c0ef1a3..4f58202 100644
> --- a/drivers/hwmon/w83627ehf.c
> +++ b/drivers/hwmon/w83627ehf.c
> @@ -1318,6 +1318,7 @@ store_pwm_mode(struct device *dev, struct device_attribute *attr,
> const char *buf, size_t count)
> {
> struct w83627ehf_data *data = dev_get_drvdata(dev);
> + struct w83627ehf_sio_data *sio_data = dev->platform_data;
> struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
> int nr = sensor_attr->index;
> unsigned long val;
> @@ -1330,6 +1331,11 @@ store_pwm_mode(struct device *dev, struct device_attribute *attr,
>
> if (val > 1)
> return -EINVAL;
> +
> + /* On NCT67766F, DC mode is only supported for pwm1 */
> + if (sio_data->kind = nct6776 && nr && val != 1)
> + return -EINVAL;
> +
> mutex_lock(&data->update_lock);
> reg = w83627ehf_read_value(data, W83627EHF_REG_PWM_ENABLE[nr]);
> data->pwm_mode[nr] = val;
Good catch. Might even be a candidate for -stable.
Acked-by: Jean Delvare <khali@linux-fr.org>
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [lm-sensors] [PATCH] hwmon: (w83627ehf) Disable setting DC mode for pwm2, pwm3 on NCT6776F
2012-01-28 1:57 [lm-sensors] [PATCH] hwmon: (w83627ehf) Disable setting DC mode for pwm2, pwm3 on NCT6776F Guenter Roeck
2012-01-28 8:49 ` Jean Delvare
@ 2012-01-28 12:25 ` CC
2012-01-28 12:28 ` CC
` (3 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: CC @ 2012-01-28 12:25 UTC (permalink / raw)
To: lm-sensors
On Sat, Jan 28, 2012 at 9:49 AM, Jean Delvare <khali@linux-fr.org> wrote:
> On Fri, 27 Jan 2012 17:57:56 -0800, Guenter Roeck wrote:
>> NCT6776F only supports pwm mode for pwm2 and pwm3. Return error if an attempt
>> is made to set those pwm channels to DC mode.
>>
>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>> ---
>> drivers/hwmon/w83627ehf.c | 6 ++++++
>> 1 files changed, 6 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/hwmon/w83627ehf.c b/drivers/hwmon/w83627ehf.c
>> index c0ef1a3..4f58202 100644
>> --- a/drivers/hwmon/w83627ehf.c
>> +++ b/drivers/hwmon/w83627ehf.c
>> @@ -1318,6 +1318,7 @@ store_pwm_mode(struct device *dev, struct device_attribute *attr,
>> const char *buf, size_t count)
>> {
>> struct w83627ehf_data *data = dev_get_drvdata(dev);
>> + struct w83627ehf_sio_data *sio_data = dev->platform_data;
>> struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
>> int nr = sensor_attr->index;
>> unsigned long val;
>> @@ -1330,6 +1331,11 @@ store_pwm_mode(struct device *dev, struct device_attribute *attr,
>>
>> if (val > 1)
>> return -EINVAL;
>> +
>> + /* On NCT67766F, DC mode is only supported for pwm1 */
>> + if (sio_data->kind == nct6776 && nr && val != 1)
>> + return -EINVAL;
>> +
>> mutex_lock(&data->update_lock);
>> reg = w83627ehf_read_value(data, W83627EHF_REG_PWM_ENABLE[nr]);
>> data->pwm_mode[nr] = val;
>
> Good catch. Might even be a candidate for -stable.
>
> Acked-by: Jean Delvare <khali@linux-fr.org>
>
> --
> Jean Delvare
It's great to disable writing to the attribute, but as per my report,
I am still confident that it cannot be PWM the fans are controlled
with. So it must be "DC".
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [lm-sensors] [PATCH] hwmon: (w83627ehf) Disable setting DC mode for pwm2, pwm3 on NCT6776F
2012-01-28 1:57 [lm-sensors] [PATCH] hwmon: (w83627ehf) Disable setting DC mode for pwm2, pwm3 on NCT6776F Guenter Roeck
2012-01-28 8:49 ` Jean Delvare
2012-01-28 12:25 ` CC
@ 2012-01-28 12:28 ` CC
2012-01-28 12:56 ` Jean Delvare
` (2 subsequent siblings)
5 siblings, 0 replies; 7+ messages in thread
From: CC @ 2012-01-28 12:28 UTC (permalink / raw)
To: lm-sensors
On Sat, Jan 28, 2012 at 1:25 PM, CC <ccomren@gmail.com> wrote:
> On Sat, Jan 28, 2012 at 9:49 AM, Jean Delvare <khali@linux-fr.org> wrote:
>> On Fri, 27 Jan 2012 17:57:56 -0800, Guenter Roeck wrote:
>>> NCT6776F only supports pwm mode for pwm2 and pwm3. Return error if an attempt
>>> is made to set those pwm channels to DC mode.
>>>
>>> Signed-off-by: Guenter Roeck <linux@roeck-us.net>
>>> ---
>>> drivers/hwmon/w83627ehf.c | 6 ++++++
>>> 1 files changed, 6 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/drivers/hwmon/w83627ehf.c b/drivers/hwmon/w83627ehf.c
>>> index c0ef1a3..4f58202 100644
>>> --- a/drivers/hwmon/w83627ehf.c
>>> +++ b/drivers/hwmon/w83627ehf.c
>>> @@ -1318,6 +1318,7 @@ store_pwm_mode(struct device *dev, struct device_attribute *attr,
>>> const char *buf, size_t count)
>>> {
>>> struct w83627ehf_data *data = dev_get_drvdata(dev);
>>> + struct w83627ehf_sio_data *sio_data = dev->platform_data;
>>> struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
>>> int nr = sensor_attr->index;
>>> unsigned long val;
>>> @@ -1330,6 +1331,11 @@ store_pwm_mode(struct device *dev, struct device_attribute *attr,
>>>
>>> if (val > 1)
>>> return -EINVAL;
>>> +
>>> + /* On NCT67766F, DC mode is only supported for pwm1 */
>>> + if (sio_data->kind == nct6776 && nr && val != 1)
>>> + return -EINVAL;
>>> +
>>> mutex_lock(&data->update_lock);
>>> reg = w83627ehf_read_value(data, W83627EHF_REG_PWM_ENABLE[nr]);
>>> data->pwm_mode[nr] = val;
>>
>> Good catch. Might even be a candidate for -stable.
>>
>> Acked-by: Jean Delvare <khali@linux-fr.org>
>>
>> --
>> Jean Delvare
>
> It's great to disable writing to the attribute, but as per my report,
> I am still confident that it cannot be PWM the fans are controlled
> with. So it must be "DC".
What's more important than terminology is the fact that setting pwm2
to other modes works (as is to be expected since this one has a 4-pin
connector with a PWM signal, and the others don't). It's pwm1 and pwm3
that should be disabled. Maybe the code got the enumeration somewhere
else wrong?
Best, CC
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [lm-sensors] [PATCH] hwmon: (w83627ehf) Disable setting DC mode for pwm2, pwm3 on NCT6776F
2012-01-28 1:57 [lm-sensors] [PATCH] hwmon: (w83627ehf) Disable setting DC mode for pwm2, pwm3 on NCT6776F Guenter Roeck
` (2 preceding siblings ...)
2012-01-28 12:28 ` CC
@ 2012-01-28 12:56 ` Jean Delvare
2012-01-28 15:34 ` CC
2012-01-28 17:23 ` Guenter Roeck
5 siblings, 0 replies; 7+ messages in thread
From: Jean Delvare @ 2012-01-28 12:56 UTC (permalink / raw)
To: lm-sensors
On Sat, 28 Jan 2012 13:28:04 +0100, CC wrote:
> What's more important than terminology is the fact that setting pwm2
> to other modes works (as is to be expected since this one has a 4-pin
> connector with a PWM signal, and the others don't). It's pwm1 and pwm3
> that should be disabled. Maybe the code got the enumeration somewhere
> else wrong?
Maybe you could let the guys with the experience and datasheets write
driver code? We apperciate your testing and feedback, don't get me
wrong, but your conclusions aren't exactly scientifically correct.
--
Jean Delvare
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [lm-sensors] [PATCH] hwmon: (w83627ehf) Disable setting DC mode for pwm2, pwm3 on NCT6776F
2012-01-28 1:57 [lm-sensors] [PATCH] hwmon: (w83627ehf) Disable setting DC mode for pwm2, pwm3 on NCT6776F Guenter Roeck
` (3 preceding siblings ...)
2012-01-28 12:56 ` Jean Delvare
@ 2012-01-28 15:34 ` CC
2012-01-28 17:23 ` Guenter Roeck
5 siblings, 0 replies; 7+ messages in thread
From: CC @ 2012-01-28 15:34 UTC (permalink / raw)
To: lm-sensors
On Sat, Jan 28, 2012 at 1:56 PM, Jean Delvare <khali@linux-fr.org> wrote:
> Maybe you could let the guys with the experience and datasheets write
> driver code? We apperciate your testing and feedback, don't get me
> wrong, but your conclusions aren't exactly scientifically correct.
I am sorry for getting in the way (and having had a terrible confusion).
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [lm-sensors] [PATCH] hwmon: (w83627ehf) Disable setting DC mode for pwm2, pwm3 on NCT6776F
2012-01-28 1:57 [lm-sensors] [PATCH] hwmon: (w83627ehf) Disable setting DC mode for pwm2, pwm3 on NCT6776F Guenter Roeck
` (4 preceding siblings ...)
2012-01-28 15:34 ` CC
@ 2012-01-28 17:23 ` Guenter Roeck
5 siblings, 0 replies; 7+ messages in thread
From: Guenter Roeck @ 2012-01-28 17:23 UTC (permalink / raw)
To: lm-sensors
On Sat, Jan 28, 2012 at 10:34:24AM -0500, CC wrote:
> On Sat, Jan 28, 2012 at 1:56 PM, Jean Delvare <khali@linux-fr.org> wrote:
> > Maybe you could let the guys with the experience and datasheets write
> > driver code? We apperciate your testing and feedback, don't get me
> > wrong, but your conclusions aren't exactly scientifically correct.
>
> I am sorry for getting in the way (and having had a terrible confusion).
>
No worries, I am confused all the time ;). Thanks for bringing this to our attention.
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 7+ messages in thread