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
- To: Paul Goyette <paul%whooppee.com@localhost>
- Subject: Re: aibs(4): ASUSTeK AI Booster (ACPI ATK0110) hardware monitor with limit support
- From: Constantine Aleksandrovich Murenin <C++%Cns.SU@localhost>
- Date: Thu, 31 Dec 2009 15:51:54 -0500
On Thu, Dec 31, 2009 at 09:58:14AM -0800, Paul Goyette wrote:
On Thu, 31 Dec 2009, Constantine Aleksandrovich Murenin wrote:
>An updated patch is attached.
+#ifdef AIBS_MORE_SENSORS
+ n = bp->Package.Count - 1;
+#endif
+ 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 whooppee.com |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer | | pgoyette at netbsd.org |
-------------------------------------------------------------------------
Yes, that all makes sense in general, however,
it doesn't appear to work in practice:
* before applying the attached patch (that no longer overwrites states in
refresh) only the user-set limits were ignored and never reported;
* after applying the attached patch, neither user-set limits nor driver-set
limits are reported.
I'm attaching envstat -x -d aibs0 output, too, from which you can see
the following:
* there's a fan sensor with a 0 speed which is below warning-min;
* there's a temperature sensor which is under the user-set warning-min, too.
Note that for both of these the states are not set correctly.
Perhaps there's some bug in the framework? I'll try to go further
into the code sometime later, probably after the New Year.
Happy New Year!
Best regards,
Constantine. http://cnst.su/
--
Constantine A. Murenin
David R. Cheriton School of Computer Science
University of Waterloo
200 University Avenue West
Waterloo, Ontario N2L 3G1
Canada
diff --git a/sys/dev/acpi/atk0110.c b/sys/dev/acpi/atk0110.c
index 4598c94..b6e85ef 100644
--- a/sys/dev/acpi/atk0110.c
+++ b/sys/dev/acpi/atk0110.c
@@ -188,7 +188,7 @@ aibs_attach_sif(device_t self, enum envsys_units st)
#ifdef AIBS_MORE_SENSORS
n = bp->Package.Count - 1;
#endif
- aprint_error_dev(self, "%s: misformed package: %i/%i"
+ aprint_error_dev(self, "%s: malformed package: %i/%i"
", assume %i\n", name, on, bp->Package.Count - 1, n);
}
if (n < 1) {
@@ -296,7 +296,6 @@ aibs_refresh(struct sysmon_envsys *sme, envsys_data_t
*edata)
const char *name;
struct aibs_sensor *as;
ACPI_INTEGER v;
- ACPI_INTEGER l, h;
switch (st) {
case ENVSYS_STEMP:
@@ -318,9 +317,6 @@ aibs_refresh(struct sysmon_envsys *sme, envsys_data_t
*edata)
return;
for (i = 0; as[i].s.sensor != s->sensor; i++)
;
- l = as[i].l;
- h = as[i].h;
-
p.Type = ACPI_TYPE_INTEGER;
p.Integer.Value = as[i].i;
mp.Count = 1;
@@ -346,48 +342,22 @@ aibs_refresh(struct sysmon_envsys *sme, envsys_data_t
*edata)
if (v == 0) {
s->state = ENVSYS_SINVALID;
s->flags |= ENVSYS_FMONNOTSUPP;
- } else {
- if (v > h)
- s->state = ENVSYS_SCRITOVER;
- else if (v > l)
- s->state = ENVSYS_SWARNOVER;
- else
- s->state = ENVSYS_SVALID;
- s->flags &= ~ENVSYS_FMONNOTSUPP;
+ return;
}
break;
case ENVSYS_SFANRPM:
s->value_cur = v;
- /* some boards have strange limits for fans */
- if (l == 0) {
- if (v < h)
- s->state = ENVSYS_SWARNUNDER;
- else
- s->state = ENVSYS_SVALID;
- } else {
- if (l > v)
- s->state = ENVSYS_SWARNUNDER;
- else if (v > h)
- s->state = ENVSYS_SWARNOVER;
- else
- s->state = ENVSYS_SVALID;
- }
- s->flags &= ~ENVSYS_FMONNOTSUPP;
break;
case ENVSYS_SVOLTS_DC:
s->value_cur = v * 1000;
- if (l > v)
- s->state = ENVSYS_SCRITUNDER;
- else if (v > h)
- s->state = ENVSYS_SCRITOVER;
- else
- s->state = ENVSYS_SVALID;
- s->flags &= ~ENVSYS_FMONNOTSUPP;
break;
default:
/* NOTREACHED */
break;
}
+ if (s->state == 0 || s->state == ENVSYS_SINVALID)
+ s->state = ENVSYS_SVALID;
+ s->flags &= ~ENVSYS_FMONNOTSUPP;
}
#ifdef AIBS_MONLIMITS
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE plist PUBLIC "-//Apple Computer//DTD PLIST 1.0//EN"
"http://www.apple.com/DTDs/PropertyList-1.0.dtd";>
<plist version="1.0">
<dict>
<key>aibs0</key>
<array>
<dict>
<key>allow-rfact</key>
<false/>
<key>critical-max</key>
<integer>1600000</integer>
<key>critical-min</key>
<integer>850000</integer>
<key>cur-value</key>
<integer>1152000</integer>
<key>description</key>
<string>Vcore Voltage</string>
<key>index</key>
<string>sensor0</string>
<key>monitoring-state-hw-range-limit</key>
<true/>
<key>monitoring-supported</key>
<true/>
<key>rfact</key>
<integer>0</integer>
<key>state</key>
<string>valid</string>
<key>type</key>
<string>Voltage DC</string>
</dict>
<dict>
<key>allow-rfact</key>
<false/>
<key>critical-max</key>
<integer>3630000</integer>
<key>critical-min</key>
<integer>2970000</integer>
<key>cur-value</key>
<integer>3312000</integer>
<key>description</key>
<string> +3.3 Voltage</string>
<key>index</key>
<string>sensor1</string>
<key>monitoring-state-hw-range-limit</key>
<true/>
<key>monitoring-supported</key>
<true/>
<key>rfact</key>
<integer>0</integer>
<key>state</key>
<string>valid</string>
<key>type</key>
<string>Voltage DC</string>
</dict>
<dict>
<key>allow-rfact</key>
<false/>
<key>critical-max</key>
<integer>5500000</integer>
<key>critical-min</key>
<integer>4500000</integer>
<key>cur-value</key>
<integer>5017000</integer>
<key>description</key>
<string> +5 Voltage</string>
<key>index</key>
<string>sensor2</string>
<key>monitoring-state-hw-range-limit</key>
<true/>
<key>monitoring-supported</key>
<true/>
<key>rfact</key>
<integer>0</integer>
<key>state</key>
<string>valid</string>
<key>type</key>
<string>Voltage DC</string>
</dict>
<dict>
<key>allow-rfact</key>
<false/>
<key>critical-max</key>
<integer>13800000</integer>
<key>critical-min</key>
<integer>10200000</integer>
<key>cur-value</key>
<integer>12302000</integer>
<key>description</key>
<string> +12 Voltage</string>
<key>index</key>
<string>sensor3</string>
<key>monitoring-state-hw-range-limit</key>
<true/>
<key>monitoring-supported</key>
<true/>
<key>rfact</key>
<integer>0</integer>
<key>state</key>
<string>valid</string>
<key>type</key>
<string>Voltage DC</string>
</dict>
<dict>
<key>critical-max</key>
<integer>368150000</integer>
<key>critical-min</key>
<integer>305150000</integer>
<key>cur-value</key>
<integer>298150000</integer>
<key>description</key>
<string>CPU Temperature</string>
<key>index</key>
<string>sensor4</string>
<key>monitoring-state-hw-range-limit</key>
<true/>
<key>monitoring-supported</key>
<true/>
<key>state</key>
<string>valid</string>
<key>type</key>
<string>Temperature</string>
<key>warning-max</key>
<integer>353150000</integer>
</dict>
<dict>
<key>critical-max</key>
<integer>368150000</integer>
<key>cur-value</key>
<integer>330150000</integer>
<key>description</key>
<string>MB Temperature</string>
<key>index</key>
<string>sensor5</string>
<key>monitoring-state-hw-range-limit</key>
<true/>
<key>monitoring-supported</key>
<true/>
<key>state</key>
<string>valid</string>
<key>type</key>
<string>Temperature</string>
<key>warning-max</key>
<integer>333150000</integer>
</dict>
<dict>
<key>cur-value</key>
<integer>869</integer>
<key>description</key>
<string>CPU FAN Speed</string>
<key>index</key>
<string>sensor6</string>
<key>monitoring-state-hw-range-limit</key>
<true/>
<key>monitoring-supported</key>
<true/>
<key>rpms</key>
<integer>0x0</integer>
<key>state</key>
<string>valid</string>
<key>type</key>
<string>Fan</string>
<key>warning-max</key>
<integer>7200</integer>
<key>warning-min</key>
<integer>600</integer>
</dict>
<dict>
<key>cur-value</key>
<integer>0</integer>
<key>description</key>
<string>CHASSIS FAN Speed</string>
<key>index</key>
<string>sensor7</string>
<key>monitoring-state-hw-range-limit</key>
<true/>
<key>monitoring-supported</key>
<true/>
<key>rpms</key>
<integer>0x0</integer>
<key>state</key>
<string>valid</string>
<key>type</key>
<string>Fan</string>
<key>warning-max</key>
<integer>7200</integer>
<key>warning-min</key>
<integer>700</integer>
</dict>
<dict>
<key>device-properties</key>
<dict>
<key>refresh-timeout</key>
<integer>0x1e</integer>
</dict>
</dict>
</array>
<key>coretemp0</key>
<array>
<dict>
<key>cur-value</key>
<integer>301150000</integer>
<key>description</key>
<string>cpu0 temperature</string>
<key>index</key>
<string>sensor0</string>
<key>monitoring-state-critical</key>
<true/>
<key>monitoring-supported</key>
<true/>
<key>state</key>
<string>valid</string>
<key>type</key>
<string>Temperature</string>
</dict>
<dict>
<key>device-properties</key>
<dict>
<key>refresh-timeout</key>
<integer>0x1e</integer>
</dict>
</dict>
</array>
<key>coretemp1</key>
<array>
<dict>
<key>cur-value</key>
<integer>303150000</integer>
<key>description</key>
<string>cpu1 temperature</string>
<key>index</key>
<string>sensor0</string>
<key>monitoring-state-critical</key>
<true/>
<key>monitoring-supported</key>
<true/>
<key>state</key>
<string>valid</string>
<key>type</key>
<string>Temperature</string>
</dict>
<dict>
<key>device-properties</key>
<dict>
<key>refresh-timeout</key>
<integer>0x1e</integer>
</dict>
</dict>
</array>
</dict>
</plist>
Home |
Main Index |
Thread Index |
Old Index