All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jean Delvare <khali@linux-fr.org>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: lm-sensors@lm-sensors.org,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [lm-sensors] Simple fan question
Date: Thu, 06 May 2010 16:40:11 +0000	[thread overview]
Message-ID: <20100506184011.2a2cfec9@hyperion.delvare> (raw)
In-Reply-To: <1272581806.24542.185.camel@pasglop>

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

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

WARNING: multiple messages have this Message-ID (diff)
From: Jean Delvare <khali@linux-fr.org>
To: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: lm-sensors@lm-sensors.org,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [lm-sensors] Simple fan question
Date: Thu, 6 May 2010 18:40:11 +0200	[thread overview]
Message-ID: <20100506184011.2a2cfec9@hyperion.delvare> (raw)
In-Reply-To: <1272581806.24542.185.camel@pasglop>

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

  reply	other threads:[~2010-05-06 16:40 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-04-29  5:21 [lm-sensors] Simple fan question Benjamin Herrenschmidt
2010-04-29  5:21 ` Benjamin Herrenschmidt
2010-04-29  5:34 ` [lm-sensors] " Dmitry Gromov
2010-04-29  8:57 ` Jean Delvare
2010-04-29  8:57   ` Jean Delvare
2010-04-29 22:56   ` Benjamin Herrenschmidt
2010-04-29 22:56     ` Benjamin Herrenschmidt
2010-05-06 16:40     ` Jean Delvare [this message]
2010-05-06 16:40       ` Jean Delvare
2010-05-17  7:46     ` Pavel Machek
2010-05-17  7:46       ` Pavel Machek
2010-05-17  8:14       ` Jean Delvare
2010-05-17  8:14         ` Jean Delvare
2010-05-17  8:30         ` Benjamin Herrenschmidt
2010-05-17  8:30           ` Benjamin Herrenschmidt
2010-05-17 15:59 ` Ray Lee
2010-05-20 11:57   ` Jean Delvare
2010-05-20 11:57     ` Jean Delvare

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20100506184011.2a2cfec9@hyperion.delvare \
    --to=khali@linux-fr.org \
    --cc=benh@kernel.crashing.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lm-sensors@lm-sensors.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.