Re: [lm-sensors] [PATCH 1/2] hwmon: add generic GPIO fan driver

[Simon Guinot <simon@sequanux.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2529 bytes --]

Hi Guenter,

On Tue, Oct 19, 2010 at 08:15:12AM -0700, Guenter Roeck wrote:
> Hi Simon,
> 
> On Tue, Oct 19, 2010 at 04:36:56AM -0400, Simon Guinot wrote:
> 
> [ ... ]
> 
> > /* Designed for fan 40x40x16: ADDA AD0412LB-D50 6000rpm@12v */
> > static struct gpio_fan_speed netspace_max_v2_fan_speed[] = {
> >         {    0,  0 },
> >         { 1500, 15 },
> >         { 1700, 14 },
> >         { 1800, 13 },
> >         { 2100, 12 },
> >         { 3100, 11 },
> >         { 3300, 10 },
> >         { 4300,  9 },
> >         { 5500,  8 },
> > };
> > 
> > The function rpm_speed(index) is really not linear. I am not sure about
> > the resulting behaviour when this will be used by a userland fan control
> > process...
> > 
> > If the pwm interface must be exported, I think we have to respect the
> > fan speed array. But as you said, maybe the work is rather on the
> > fancontrol script side.
> > 
> Not sure if there is an expectation that the mapping pwm->speed has
> to be linear. One might as well argue that steps should be equally
> far apart to ensure that results are better predictable.
> 
> > As a first step, we could keep the heavy pwm interface. Once the
> > fancontrol script (or something else) will be able to handle the rpm
> > attributes, we will drop the pwm compatibility.
> > Simply remove the gpio-fan pwm interface seems a good option too...
> > 
> I think we should keep the pwm interface after all. Changing fancontrol
> is a possibility, but the change would take a while to make it upstream.

Yes, I understand. I will probably look soon into fancontrol.
Some of the cheapest LaCie board have a fan and no temperature sensors.
The embedded system get a more or less accurate temperature from the
hard drive sensor via SMART commands :)
I'd like to have some userspace fan support for this boards too.

> 
> As for the actual code, I would prefer the simpler implementation, but I am willing 
> to go with the complex one if you feel wtrongly about it. Only thing I would ask you
> to do is to add an explanation of what is done and why.

Looking at the pwm_to_rpm() and rpm_to_pwm() functions, I can't feel
strongly a such implementation (but maybe wrongly).
So... I have changed my mind. The simpler implementation make sense:
the pwm value will simply map the speed index.
Moreover it appear to be very usable from a userspace point.

> 
> Reminds me - you'll also have to provide Documentation/hwmon/gpio-fan.

Ok.

Thanks,

Simon

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 198 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

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