Re: [PATCH v2 1/2] i8k: Autodetect maximal fan speed and fan RPM multiplier

From: Pali RohÃr
Date: Sat Dec 20 2014 - 07:18:17 EST


On Saturday 20 December 2014 13:04:03 Guenter Roeck wrote:
> On 12/20/2014 12:57 AM, Pali RohÃr wrote:
> > On Friday 19 December 2014 20:28:08 Guenter Roeck wrote:
> >> On Fri, Dec 19, 2014 at 07:51:25PM +0100, Pali RohÃr wrote:
> >>> On Friday 19 December 2014 19:32:37 Guenter Roeck wrote:
> >>>>> -static int i8k_fan_mult;
> >>>>> -static int i8k_pwm_mult;
> >>>>> -static int i8k_fan_max = I8K_FAN_HIGH;
> >>>>> +static int i8k_fan_mult[2];
> >>>>> +static int i8k_pwm_mult[2];
> >>>>> +static int i8k_fan_max[2];
> >>>>
> >>>> The rationale for this change is not explained in the
> >>>> commit log.
> >>>>
> >>>> Do you have any indication that those values would ever
> >>>> be different for the two fans, ie that you actually need
> >>>> arrays here ?
> >>>
> >>> I do not know... But if we decide to use only single value
> >>> for multiplier and max value which fan to use for
> >>> autodetection?
> >>
> >> That does not answer my question. That you can not decide
> >> which fan to use for auto-detection does not mean that the
> >> result of that auto-detection would be different for
> >> different fans.
> >
> > Really I do not know if some dell products which have more
> > fans (some Precision models have 2) and each fan is using
> > different multiplier or has different max speed value.
>
> "I do not know" is not a reason for introducing such code.
>
> Guenter

And having broken fan rpm output in userspace is not good.

My code introduce fan rpm detection for each fan. I do not see
any problem for doing this detection per fan. You suggest to do
some global detection and use it for each fan. And for your
suggestion I have 2 objections:

1) I do not know if global multiplier will work for all fans.
Detection per fan does not have this problem "I do not know".

2) How to do that global multiplier detection? I have no idea how
you want to do that.

So I would say, "I do not know" refers yo your suggestion, not to
my current code.

--
Pali RohÃr
pali.rohar@xxxxxxxxx

Attachment: signature.asc
Description: This is a digitally signed message part.