Re: [PATCH v2 1/2] i8k: Autodetect maximal fan speed and fan RPM multiplier
From: Guenter Roeck
Date: Sat Dec 20 2014 - 12:21:45 EST
On 12/20/2014 04:54 AM, Pali RohÃr wrote:
On Saturday 20 December 2014 13:44:59 Guenter Roeck wrote:
On 12/20/2014 04:18 AM, Pali RohÃr wrote:
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.
Sure, but only if it is in fact broken.
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:
The problem is that there is no evidence that a per-fan
multiplier is needed. You may not see it that way, but
unnecessary code _is_ a problem since it increases code size
for no good reason. Furthermore, people not involved in this
discussion will be inclined to believe that the code is
necessary, in the wrong assumption that it would not have
been introduced unless it was necessary.
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".
It has been working all along, with no evidence that it
doesn't work.
2) How to do that global multiplier detection? I have no
idea how you want to do that.
A single multiplier worked all along. It doesn't matter which
fan is used to auto-detect the multiplier, it is still a
single multiplier.
Again, introducing code if you don't know that or if it may be
needed is not an argument for introducing such code. Introduce
it if and when there is evidence that it is needed.
Thanks,
Guenter
Ok, so what you suggest now? We have function which reports on
supported models if speed level X is supported for fan Y and
which RPM speed Z is expected for that level X. I have no problem
to (re)write for fan multiplier detection and maximal fan speed
level, but I need to know how to do that (which approach you
accept and which not). What I want to have is correct data in
userspace and better without hardcoded constants for each laptop
in driver code.
Take your patch, run 's/\[fan\]//', see where it takes you.
Essentially assume that the auto-detected parameters for one fan
apply to all fans. I would think that it should be relatively safe
to assume that the first fan always exists, so you can start with
using that to detect parameters and see where it takes you.
Guenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/