Possible incorrectness in w83627hf.c

[Mark Studebaker]

Jean Delvare wrote:
> MDS,
> 
> Here is an excerpt from w83627hf.c:
> 
> if (type != w83697hf) {
> 	/* enable PWM2 control (can't hurt since PWM reg
>            should have been reset to 0xff) */
> 	w83627hf_write_value(client, W83781D_REG_PWMCLK12, 0x19);
> }
> 
> According to the data sheets, this register (0x5C) does only have a
> meaning for the W83627HF. So I'd suggest changing the condition to "type
> == w83627hf" so that we don't access that register on the W83627THF (and
> possibly others such as the W83637HF, which I plan to add support for).
> 
> And, as we come to speak about this, I don't frankly see the point in
> writing anything to this register. You seem to try and write the default
> value anyway (and possibly change the value of reserved bits BTW, which
> is no good). Can't we simply get rid of that write?

What should really happen is adding code to w83627hf_pwm()
that turns the PWM on and off when the user writes the second value in the pwm file.

I note that there's still way too much initialization code in the driver.
At the very least init should be set to 0, no?
Either I didn't rip out enough code or some of it snuck
back in later.

> 
> BTW, if someone could tell me what these "PWM clock frequencies" are
> meant for... The data sheet don't seem to explain it at all.
> 

The PWM pulses have a certain duty cycle (that's what you set with
the pwm entry in /proc to control the fan speed)
and a certain frequency.
The frequency generally doesn't matter but some fans may
work better, or be less noisy, with a higher or lower frequency.
We don't support frequency changing in any driver.

> Thanks.
> 
> PS: Looks like this driver would benefit a massive refresh. The
> references to w83781d everywhere make things hard to understand
> sometimes.
> 

feel free :)