All of lore.kernel.org
 help / color / mirror / Atom feed
* [lm-sensors] [PATCH] Add fan control support to W83627EHF
@ 2006-03-04 20:59 Rudolf Marek
  2006-03-04 21:17 ` David Hubbard
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Rudolf Marek @ 2006-03-04 20:59 UTC (permalink / raw)
  To: lm-sensors

Hi all,

This is just first shot patch. I finished Yuans work and now we have the ability to control
the fans and two automatic modes. It took me literaly whole day :/

David,
Please can you test the patch as much as possible?
You may need to apply the voltages patch first to be able to patch this.

Second file contains some very simple documentation that should help to test the stuff.

Yuan,

I removed the speed cruise option and reworked the interface. I would like to ask
why you did not allow the FAN4 PWM interface? I checked new datasheet from your website
and it seems that it can be there when checked if correctly programmed.

Thanks,

Regards
Rudolf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: w83627ehf_add_fancrt.patch
Type: text/x-patch
Size: 15571 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20060304/debc98ee/w83627ehf_add_fancrt-0001.bin
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: docs
Url: http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20060304/debc98ee/docs-0001.pl

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

* [lm-sensors] [PATCH] Add fan control support to W83627EHF
  2006-03-04 20:59 [lm-sensors] [PATCH] Add fan control support to W83627EHF Rudolf Marek
@ 2006-03-04 21:17 ` David Hubbard
  2006-03-04 21:47 ` Rudolf Marek
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: David Hubbard @ 2006-03-04 21:17 UTC (permalink / raw)
  To: lm-sensors

Hi Rudolf,

Thanks for posting your work! I'm testing it right now. I have a question:

> pwm[1-4]_enable - this file controls mode of fan/temperature control:
>        * 0 Disabled. In fact the pwm is set to 255
>        * 1 Manual Mode, write to pwm file any value 0-255 (fullspeed)
>        * 2 Thermal Cruise
>        * 3 Smart FAN III

There is another mode, RPM Cruise, in the datasheet. Smart Fan 3 is
only supported by CPUFANOUT0 and CPUFANOUT1. Tell me what you think,
maybe mode 3 should be RPM cruise and mode 4 is either not allowed
(-EINVAL) or for the two fans that allow Smart Fan 3, it turns it on.

That's all until I get through testing,
David


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

* [lm-sensors] [PATCH] Add fan control support to W83627EHF
  2006-03-04 20:59 [lm-sensors] [PATCH] Add fan control support to W83627EHF Rudolf Marek
  2006-03-04 21:17 ` David Hubbard
@ 2006-03-04 21:47 ` Rudolf Marek
  2006-03-05  9:52 ` Rudolf Marek
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Rudolf Marek @ 2006-03-04 21:47 UTC (permalink / raw)
  To: lm-sensors

Hi,

> Thanks for posting your work! I'm testing it right now. I have a question:
> 
> 
>>pwm[1-4]_enable - this file controls mode of fan/temperature control:
>>       * 0 Disabled. In fact the pwm is set to 255
>>       * 1 Manual Mode, write to pwm file any value 0-255 (fullspeed)
>>       * 2 Thermal Cruise
>>       * 3 Smart FAN III
> 
> 
> There is another mode, RPM Cruise, in the datasheet. Smart Fan 3 is
> only supported by CPUFANOUT0 and CPUFANOUT1. Tell me what you think,
> maybe mode 3 should be RPM cruise and mode 4 is either not allowed
> (-EINVAL) or for the two fans that allow Smart Fan 3, it turns it on.

I removed the support for RPM cruise because I cannot see the use of such mode.
The mode can be supported in the driver and in fact it was there.
see the

+static const u8 W83627EHF_PWM_MODE_USER_MAP[] = { 0, 0, 1, 3 };
+static const u8 W83627EHF_PWM_MODE_CHIP_MAP[] = { 1, 2, 4, 3 };

This translates between the former modes:
            * 0 Manual mode
            * 1 Thermal Cruise
            * 2 Fan Speed Cruise
            * 3 Smart FAN III

And yes the we must not allow the smartfan III on the fan1 and 3...

TODO:
not allow the SF3 mode on fans without support
check if fan4 is really enabled

You can meet me on IRC freenode.net #linux-sensors

Regards
Rudolf

> That's all until I get through testing,
> David


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

* [lm-sensors] [PATCH] Add fan control support to W83627EHF
  2006-03-04 20:59 [lm-sensors] [PATCH] Add fan control support to W83627EHF Rudolf Marek
  2006-03-04 21:17 ` David Hubbard
  2006-03-04 21:47 ` Rudolf Marek
@ 2006-03-05  9:52 ` Rudolf Marek
  2006-03-05 16:42 ` David Hubbard
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Rudolf Marek @ 2006-03-05  9:52 UTC (permalink / raw)
  To: lm-sensors

David Hubbard wrote:
> Hi Rudolf,
> I disagree. If users want to ignore the RPM cruise or speed cruise
> then it's easy to modify the sensors.conf to suit their taste. I agree
> that there isn't a standardized sysfs file for the target RPM, but I
> am planning on using the RPM cruise feature. Yuan has already
> implemented it. I don't think it'd be a problem to support it. Since
> it is present on all four PWM outputs, and Smart Fan 3 isn't, I
> thought it would make sense to have PWM cruise be mode 3.
> 

Well it could be implemented when you will create some files like fanX_target
(Because this file has to be in RPM and you need to convert the RPM into ticks and
write it back to the "target temp" register. On the other hand it will mean that
user receive nonsence when reading from tempX_target

I think I'm missing the whole point of RPM cruise. To me it seems I can
manually setup the requested RPM through the pwm file in manual mode.

Can you explain please how the RPM cruise mode is different from that?

Please always CC to the list so others know too ;)

Thanks,
Regards
Rudolf


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

* [lm-sensors] [PATCH] Add fan control support to W83627EHF
  2006-03-04 20:59 [lm-sensors] [PATCH] Add fan control support to W83627EHF Rudolf Marek
                   ` (2 preceding siblings ...)
  2006-03-05  9:52 ` Rudolf Marek
@ 2006-03-05 16:42 ` David Hubbard
  2006-03-05 20:05 ` Rudolf Marek
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: David Hubbard @ 2006-03-05 16:42 UTC (permalink / raw)
  To: lm-sensors

> > I disagree. If users want to ignore the RPM cruise or speed cruise
> > then it's easy to modify the sensors.conf to suit their taste. I agree
> > that there isn't a standardized sysfs file for the target RPM, but I
> > am planning on using the RPM cruise feature. Yuan has already
> > implemented it. I don't think it'd be a problem to support it. Since
> > it is present on all four PWM outputs, and Smart Fan 3 isn't, I
> > thought it would make sense to have PWM cruise be mode 3.
>
> Well it could be implemented when you will create some files like fanX_target
> (Because this file has to be in RPM and you need to convert the RPM into ticks and
> write it back to the "target temp" register. On the other hand it will mean that
> user receive nonsence when reading from tempX_target
>
> I think I'm missing the whole point of RPM cruise. To me it seems I can
> manually setup the requested RPM through the pwm file in manual mode.
>
> Can you explain please how the RPM cruise mode is different from that?
>
> Please always CC to the list so others know too ;)

Hi Rudolf,

The tempX_target register can still contain a target temperature,
either the default value loaded by the BIOS or the last temp target
when the chip was in temp cruise. fanX_target is a good way to add the
sysfs interface, I like that.

RPM cruise allows me to set an RPM directly without knowing the PWM
value. Or, more specifically, it's good for low RPMs, which are hard
to get perfectly right, since the PWM value is typically 100 or so. At
PWM values of 70 or so, the fan just stops spinning. Yes, if I wanted
to read out the RPM value and tinker with the PWM until I got the RPM
I wanted, I could find what the low threshold is where the fan
overcomes friction and starts spinning. But that's why RPM cruise is
so nice.

Anyway, as long as the chip implements it, it doesn't hurt to export
the interface, right?

David


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

* [lm-sensors] [PATCH] Add fan control support to W83627EHF
  2006-03-04 20:59 [lm-sensors] [PATCH] Add fan control support to W83627EHF Rudolf Marek
                   ` (3 preceding siblings ...)
  2006-03-05 16:42 ` David Hubbard
@ 2006-03-05 20:05 ` Rudolf Marek
  2006-03-05 21:10 ` David Hubbard
  2006-03-05 21:14 ` Rudolf Marek
  6 siblings, 0 replies; 8+ messages in thread
From: Rudolf Marek @ 2006-03-05 20:05 UTC (permalink / raw)
  To: lm-sensors

Hello again,

> The tempX_target register can still contain a target temperature,
> either the default value loaded by the BIOS or the last temp target
> when the chip was in temp cruise. fanX_target is a good way to add the
> sysfs interface, I like that.

Yep. The target temp register is shared with this speed register so some logic in the driver
must be programmed.

> 
> RPM cruise allows me to set an RPM directly without knowing the PWM
> value. Or, more specifically, it's good for low RPMs, which are hard
> to get perfectly right, since the PWM value is typically 100 or so. At
> PWM values of 70 or so, the fan just stops spinning. Yes, if I wanted
> to read out the RPM value and tinker with the PWM until I got the RPM
> I wanted, I could find what the low threshold is where the fan
> overcomes friction and starts spinning. But that's why RPM cruise is
> so nice.

Ok now I get it.

> Anyway, as long as the chip implements it, it doesn't hurt to export
> the interface, right?

I will certainly have no time for this in next 5 days. So feel free to implement it.
(As the patch on the top of mine) If you have time please check my implementation or
change it so it is even better :) (also provide a patch on top of my original patch)

So to restore the RPM cruise you need to delete both mapping arrays (USER_MAP and CHIP_MAP iirc)
then the pwm_enable will be like:
            * 0 Manual mode
            * 1 Thermal Cruise
            * 2 Fan Speed Cruise
            * 3 Smart FAN III
This has to be fixed so 0 is disabled (to comply with the interface def)
So best to emulate disabled (please check datasheet if it cannot be disabled) is to write 255 to corresponding
pwm ...

Thanks,

Regards
Rudolf



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

* [lm-sensors] [PATCH] Add fan control support to W83627EHF
  2006-03-04 20:59 [lm-sensors] [PATCH] Add fan control support to W83627EHF Rudolf Marek
                   ` (4 preceding siblings ...)
  2006-03-05 20:05 ` Rudolf Marek
@ 2006-03-05 21:10 ` David Hubbard
  2006-03-05 21:14 ` Rudolf Marek
  6 siblings, 0 replies; 8+ messages in thread
From: David Hubbard @ 2006-03-05 21:10 UTC (permalink / raw)
  To: lm-sensors

Hi Rudolf!

> > The tempX_target register can still contain a target temperature,
> > either the default value loaded by the BIOS or the last temp target
> > when the chip was in temp cruise. fanX_target is a good way to add the
> > sysfs interface, I like that.
>
> Yep. The target temp register is shared with this speed register so some logic in the
> driver must be programmed.
>
> > RPM cruise allows me to set an RPM directly without knowing the PWM
> > value. Or, more specifically, it's good for low RPMs, which are hard
> > to get perfectly right, since the PWM value is typically 100 or so. At
> > PWM values of 70 or so, the fan just stops spinning. Yes, if I wanted
> > to read out the RPM value and tinker with the PWM until I got the RPM
> > I wanted, I could find what the low threshold is where the fan
> > overcomes friction and starts spinning. But that's why RPM cruise is
> > so nice.
>
> Ok now I get it.
>
> > Anyway, as long as the chip implements it, it doesn't hurt to export
> > the interface, right?
>
> I will certainly have no time for this in next 5 days. So feel free to implement it.
> (As the patch on the top of mine) If you have time please check my implementation or
> change it so it is even better :) (also provide a patch on top of my original patch)
>
> So to restore the RPM cruise you need to delete both mapping arrays (USER_MAP
> and CHIP_MAP iirc) then the pwm_enable will be like:
>             * 0 Manual mode
>             * 1 Thermal Cruise
>             * 2 Fan Speed Cruise
>             * 3 Smart FAN III
> This has to be fixed so 0 is disabled (to comply with the interface def)
> So best to emulate disabled (please check datasheet if it cannot be disabled) is to
> write 255 to corresponding pwm ...

So add 1?
             * 0 Disabled (set PWM to 255)
             * 1 Manual mode
             * 2 Thermal Cruise
             * 3 Fan Speed Cruise
             * 4 Smart FAN III

I'll put together a quick patch and send that back to you tomorrow.

David


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

* [lm-sensors] [PATCH] Add fan control support to W83627EHF
  2006-03-04 20:59 [lm-sensors] [PATCH] Add fan control support to W83627EHF Rudolf Marek
                   ` (5 preceding siblings ...)
  2006-03-05 21:10 ` David Hubbard
@ 2006-03-05 21:14 ` Rudolf Marek
  6 siblings, 0 replies; 8+ messages in thread
From: Rudolf Marek @ 2006-03-05 21:14 UTC (permalink / raw)
  To: lm-sensors

> 
> So add 1?
>              * 0 Disabled (set PWM to 255)
>              * 1 Manual mode
>              * 2 Thermal Cruise
>              * 3 Fan Speed Cruise
>              * 4 Smart FAN III

Yes like this plus extra fanX_target and fanX_tolerance + the logic to preserve the tempX_target and tempX_tolerance.

> I'll put together a quick patch and send that back to you tomorrow.

No need to hurry, I must fix the voltages for Jean tomorrow ;)

Regards
Rudolf


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

end of thread, other threads:[~2006-03-05 21:14 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-03-04 20:59 [lm-sensors] [PATCH] Add fan control support to W83627EHF Rudolf Marek
2006-03-04 21:17 ` David Hubbard
2006-03-04 21:47 ` Rudolf Marek
2006-03-05  9:52 ` Rudolf Marek
2006-03-05 16:42 ` David Hubbard
2006-03-05 20:05 ` Rudolf Marek
2006-03-05 21:10 ` David Hubbard
2006-03-05 21:14 ` Rudolf Marek

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.