[PATCH 1/2] hwmon: add generic GPIO fan driver

Guenter Roeck guenter.roeck at ericsson.com
Tue Oct 19 11:15:12 EDT 2010


Hi Simon,

On Tue, Oct 19, 2010 at 04:36:56AM -0400, Simon Guinot wrote:

[ ... ]

> /* Designed for fan 40x40x16: ADDA AD0412LB-D50 6000rpm at 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.

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.

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

Thanks,
Guenter




More information about the linux-arm-kernel mailing list