tech-kern archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: aibs(4): ASUSTeK AI Booster (ACPI ATK0110) hardware monitor with limit support

On Thu, 31 Dec 2009, Constantine Aleksandrovich Murenin wrote:

An updated patch is attached.

+               n = bp->Package.Count - 1;
+               aprint_error_dev(self, "%s: misformed package: %i/%i"
+                   ", assume %i\n", name, on, bp->Package.Count - 1, n);
+       }
+       if (n < 1) {

I think you should s/misformed/malformed/ in the error message.

In the aibs_refresh() routine, you really shouldn't be setting the sensor state to anything other than SVALID or SINVALID. (Yes, I know that I did the same thing in acpi_tz, but I need to fix that, too!) You've already exported the limit values, so let envsys's code do the comparisons.

By setting the SCRITOVER etc. values here, you prevent the user from overriding the limit values. In other words, the user might decide to set the CPU temperature warn-max value to 85 instead of the value 80 supplied by ACPI. As currently written, your code will generate the event at 80 degrees even though the user has attempted to override that setting.

(The driver should only be setting the CRITOVER etc. states when they are generated by the underlying hardware, _and_ the hardware thresholds can be updated to user-provided values. I think the sdtemp(4) driver might be a better example of this.)

|   Paul Goyette   | PGP DSS Key fingerprint: |  E-mail addresses:      |
| Customer Service | FA29 0E3B 35AF E8AE 6651 |  paul at   |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at |
| Kernel Developer |                          | pgoyette at  |

Home | Main Index | Thread Index | Old Index