Simple fan question

Simple fan question

[Benjamin Herrenschmidt]

Hi folks !

I'm writing some support for d-link dns323 rev C1 NAS. It has a fan that
is controlled by some non-programmable PWM circuit. Basically, all I can
do is tweak to GPIOs that controls the feed into the circuitry for the
fans to be off, slow or fast.

I don't know what the actual PWM values are for "slow" or "fast". I
-might- be able to do some measurements but I can't promise it.

Now I'm trying to do a simple hwmon driver for that in order to easy
userspace support for these guys, and I don't really see a 'nice' way to
expose that which would fit the interfaces documented
Documentation/hwmon/sysfs-interface.

So before I do something horrible, I felt I might poke you guys see if
you have a good idea here :-)

Before I read the above document I was thinking about a sysfs file that
contains "off", "slow" or "fast" but it looks like this won't fit at all
the typical hwmon APIs.

Another comment while at it is when implementing the thermal control for
PowerMacs a while back (windfarm etc...) I had to deal with two
different type of interfaces to fans. RPM controlled and PWM controlled.

The later basically let me program a percentile value (a percent of the
duty cycle).

I looks like the described sysfs interface only does RPM, or at least
doesn't provide a way to expose the units used...

Cheers,
Ben.

Re: [lm-sensors] Simple fan question

[Jean Delvare]

Hi Ben,

On Thu, 29 Apr 2010 15:21:46 +1000, Benjamin Herrenschmidt wrote:
> I'm writing some support for d-link dns323 rev C1 NAS. It has a fan that
> is controlled by some non-programmable PWM circuit. Basically, all I can
> do is tweak to GPIOs that controls the feed into the circuitry for the
> fans to be off, slow or fast.
> 
> I don't know what the actual PWM values are for "slow" or "fast". I
> -might- be able to do some measurements but I can't promise it.
> 
> Now I'm trying to do a simple hwmon driver for that in order to easy
> userspace support for these guys, and I don't really see a 'nice' way to
> expose that which would fit the interfaces documented
> Documentation/hwmon/sysfs-interface.
> 
> So before I do something horrible, I felt I might poke you guys see if
> you have a good idea here :-)
> 
> Before I read the above document I was thinking about a sysfs file that
> contains "off", "slow" or "fast" but it looks like this won't fit at all
> the typical hwmon APIs.

Indeed, that wouldn't fit. The nearest entry in the document is:

pwm[1-*]	Pulse width modulation fan control.
		Integer value in the range 0 to 255
		RW
		255 is max or 100%.

In your case, the file would have only 3 possible values, with "off"
mapping to 0, and "slow" and "fast" mapping to arbitrary positive
values, like 64 and 192 or whatever you think is suitable. I understand
that in your case, you don't really control the PWM output directly,
but we do not have any interface for this, and I don't think there
would be much value in adding one.

That being said, I am also only mildly convinced that fitting your chip
in the standard pwm1 interface will be very helpful. I don't really
expect tools such as the fancontrol script to behave properly when the
pwm1 file only support a small number of discrete values. So the
benefit of using the standard file name and semantics seems thin.

> Another comment while at it is when implementing the thermal control for
> PowerMacs a while back (windfarm etc...) I had to deal with two
> different type of interfaces to fans. RPM controlled and PWM controlled.
> 
> The later basically let me program a percentile value (a percent of the
> duty cycle).

This is exactly what pwm[1-*] files are about, except that we used
range 0-255 instead of 0-100 for historical and practical reasons.

> I looks like the described sysfs interface only does RPM, or at least
> doesn't provide a way to expose the units used...

For RPM-controlled, look at the following entry instead:

fan[1-*]_target
		Desired fan speed
		Unit: revolution/min (RPM)
		RW
		Only makes sense if the chip supports closed-loop fan speed
		control based on the measured fan speed.

One significant difference is that, in this case, you always know which
fan you control, while in the pwm[1-*] case you don't.

-- 
Jean Delvare

Re: [lm-sensors] Simple fan question

[Benjamin Herrenschmidt]

Hi Jean !

> In your case, the file would have only 3 possible values, with "off"
> mapping to 0, and "slow" and "fast" mapping to arbitrary positive
> values, like 64 and 192 or whatever you think is suitable. I understand
> that in your case, you don't really control the PWM output directly,
> but we do not have any interface for this, and I don't think there
> would be much value in adding one.
> 
> That being said, I am also only mildly convinced that fitting your chip
> in the standard pwm1 interface will be very helpful. I don't really
> expect tools such as the fancontrol script to behave properly when the
> pwm1 file only support a small number of discrete values. So the
> benefit of using the standard file name and semantics seems thin.

Yes, I'm not too sure either.

> > Another comment while at it is when implementing the thermal control for
> > PowerMacs a while back (windfarm etc...) I had to deal with two
> > different type of interfaces to fans. RPM controlled and PWM controlled.
> > 
> > The later basically let me program a percentile value (a percent of the
> > duty cycle).
> 
> This is exactly what pwm[1-*] files are about, except that we used
> range 0-255 instead of 0-100 for historical and practical reasons.

Ok, I missed those in the doco.

> > I looks like the described sysfs interface only does RPM, or at least
> > doesn't provide a way to expose the units used...
> 
> For RPM-controlled, look at the following entry instead:
> 
> fan[1-*]_target
> 		Desired fan speed
> 		Unit: revolution/min (RPM)
> 		RW
> 		Only makes sense if the chip supports closed-loop fan speed
> 		control based on the measured fan speed.
> 
> One significant difference is that, in this case, you always know which
> fan you control, while in the pwm[1-*] case you don't.

Right.

Now, maybe the best option is to have instead:

	fan[1-*]_discrete_value
		Discrete value
                RW

	fan[1-*]_supported values
                List of supported discrete values
                RO

IE. I like the interface to be self-explanatory rather than relying on
the user to know in advance what to write there. In which case I could
either use 0,1,2 as values or even "off, slow, fast".

I can then make a custom fancontrol script (or add a wart to the
existing one) to deal with this HW.

What do you think ?

Another option of course is to do the whole thermal control in a kernel
thread :-) That wouldn't be very hard nor take a lot of code, but I'm
sure I'll encounter resistance trying to merge that :-)

Cheers,
Ben.

Re: [lm-sensors] Simple fan question

[Jean Delvare]

Hi Ben,

Sorry for the delay.

On Fri, 30 Apr 2010 08:56:46 +1000, Benjamin Herrenschmidt wrote:
> > For RPM-controlled, look at the following entry instead:
> > 
> > fan[1-*]_target
> > 		Desired fan speed
> > 		Unit: revolution/min (RPM)
> > 		RW
> > 		Only makes sense if the chip supports closed-loop fan speed
> > 		control based on the measured fan speed.
> > 
> > One significant difference is that, in this case, you always know which
> > fan you control, while in the pwm[1-*] case you don't.
> 
> Right.
> 
> Now, maybe the best option is to have instead:
> 
> 	fan[1-*]_discrete_value
> 		Discrete value
>                 RW
> 
> 	fan[1-*]_supported values
>                 List of supported discrete values
>                 RO
> 
> IE. I like the interface to be self-explanatory rather than relying on
> the user to know in advance what to write there. In which case I could
> either use 0,1,2 as values or even "off, slow, fast".

I have no objection.

> I can then make a custom fancontrol script (or add a wart to the
> existing one) to deal with this HW.
> 
> What do you think ?

Please don't try to add this to the fancontrol script. It's messy
enough as is ;) You probably want to implement the kernel part and the
user-space part together before you propose a standard interface,
otherwise it might be difficult to make the best decisions with regards
to attribute names and values.

> Another option of course is to do the whole thermal control in a kernel
> thread :-) That wouldn't be very hard nor take a lot of code, but I'm
> sure I'll encounter resistance trying to merge that :-)

Me, I wouldn't object. That's what you did for other systems as far as I
can see? As long as things work in the end, I have no problem with
fan speed control being in the kernel. Having it in user-space has its
share of issues (e.g. risk of overheating is the script dies for any
reason.)

But yes, others may complain.

-- 
Jean Delvare

Re: [lm-sensors] Simple fan question

[Pavel Machek]

Hi!

> Right.
> 
> Now, maybe the best option is to have instead:
> 
> 	fan[1-*]_discrete_value
> 		Discrete value
>                 RW
> 
> 	fan[1-*]_supported values
>                 List of supported discrete values
>                 RO

Hmm, for 100 different values, that will get ugly. What about simple
fan_max file with values 0..fan_max being valid?

IIRC thinkpads have 8-or-so possible discrete cases...

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: [lm-sensors] Simple fan question

[Jean Delvare]

Hi Pavel,

On Mon, 17 May 2010 09:46:21 +0200, Pavel Machek wrote:
> Hi!
> 
> > Right.
> > 
> > Now, maybe the best option is to have instead:
> > 
> > 	fan[1-*]_discrete_value
> > 		Discrete value
> >                 RW
> > 
> > 	fan[1-*]_supported values
> >                 List of supported discrete values
> >                 RO
> 
> Hmm, for 100 different values, that will get ugly. What about simple
> fan_max file with values 0..fan_max being valid?

If you have 100 different values, you can map them to the standard
0..255 range of pwm[1-*] files.

> IIRC thinkpads have 8-or-so possible discrete cases...

Correct. And even these would map nicely to the 0..255 range IMHO.

-- 
Jean Delvare

Re: [lm-sensors] Simple fan question

[Benjamin Herrenschmidt]

On Mon, 2010-05-17 at 10:14 +0200, Jean Delvare wrote:
> If you have 100 different values, you can map them to the standard
> 0..255 range of pwm[1-*] files.
> 
> > IIRC thinkpads have 8-or-so possible discrete cases...
> 
> Correct. And even these would map nicely to the 0..255 range IMHO.

well, I eventually settled for doing that with my 3 values :-) We'll see
how fan-control copes.

Cheers,
Ben.

Re: [lm-sensors] Simple fan question

[Jean Delvare]

On Mon, 17 May 2010 08:59:50 -0700, Ray Lee wrote:
> On Thu, Apr 29, 2010 at 3:56 PM, Benjamin Herrenschmidt <
> benh@kernel.crashing.org> wrote:
> 
> > Now, maybe the best option is to have instead:
> >
> >        fan[1-*]_discrete_value
> >                Discrete value
> >                RW
> >
> >        fan[1-*]_supported values
> >                List of supported discrete values
> >                RO
> >
> 
> Perhaps a granularity (ie, step-size) instead of a full list of values?

A possible addition to the current API, yes. But it may not cover all
cases: the step size isn't necessarily constant.

-- 
Jean Delvare