On Thu, 2015-02-26 at 10:48 +0100, Thomas Haschka wrote:
> Hello,
>
> I hope I get it correct this time, thanks again.
Thanks. Sorry for the delay, I've been a bit swamped. I looked at your
code a bit more in depth and I would appreciate a couple more changes
if you don't mind:
.../...
> +static void update_fans_speed_singlefan (struct thermostat *th) {
> +
> + /* Single Fan Laptops i.e. 12 inch Powerbook, Ibook etc.
> + * Necessites the readout of all three thermal sensors
> + * in order to avoid the harddisk and other components to overheat
> + * in the case of cold CPU. */
> +
> + int lastvar = 0; /* last variation, for iBook */
> + int i = 0;
> + int var = 0;
> + int var_sensor[3];
> + int started = 0;
> + int fan_number = 0;
So here you hard code fan_number instead of looping accross all fans,
you could just use a constant...
> + for (i = 0; i < 3; i++) {
> + var_sensor[i] = th->temps[i] - th->limits[i];
> + }
> + var = var_sensor[0];
> + for (i = 1; i < 3; i++) {
> + if (var_sensor[i] > var) {
> + var = var_sensor[i];
> + }
> + }
Why two loops ?
> + if (var > -1) {
> + int step = (255 - fan_speed) / 7;
> + int new_speed = 0;
> + /* hysteresis : change fan speed only if variation is
> + * more than two degrees */
> + if (abs(var - th->last_var[fan_number]) > 2) {
> +
> + started = 1;
> + new_speed = fan_speed + ((var-1)*step);
> +
> + if (new_speed < fan_speed)
> + new_speed = fan_speed;
> + if (new_speed > 255)
> + new_speed = 255;
> +
> + if (verbose)
> + printk(KERN_DEBUG "adt746x:"
> + " Setting fans speed to %d "
> + "(limit exceeded by %d on %s)\n",
> + new_speed, var,
> + sensor_location[fan_number+1]);
> + write_both_fan_speed(th, new_speed);
> + th->last_var[fan_number] = var;
> + }
> + } else if (var < -2) {
> + /* don't stop fan if sensor2 is cold and sensor1 is not
> + * so cold (lastvar >= -1) */
> + if (i == 2 && lastvar < -1) {
> + if (th->last_speed[fan_number] != 0)
> + if (verbose)
> + printk(KERN_DEBUG "adt746x:"
> + " Stopping "
> + "fans.\n");
> + write_both_fan_speed(th, 0);
> + }
> + }
This looks identical between the two functions (single/multi fan), can
you factor it out into a single static helper that they both call ?
Cheers,
Ben.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
