On Sun, 29 Jul 2007 06:00:09 +0200
Quentin Garnier <cube@cubidou.net> wrote:

> Hi,
> 
> I've spent most of the last 24 hours tracking a bug that made envstat
> (8) show bogus values for the status of my battery.
> 
> (Very) long story short, the reason for that behaviour is that the
> sensors from acpi_bat(4) and acpi_acad(4) (and maybe acpi_tz(4), too)
> are queried before interrupts are enabled.
> 
> I haven't finished tracking exactly why they need interrupts, but I
> guess the ACPI embedded controller kind of reports back to the driver
> at this point, and when that reporting fails, I end up with bogus
> values.
> 
> Initially I only config_interrupts()'d the envsys attachment for
> acpi_bat(4) and it wasn't enough, so I did the same for acpi_acad(4)
> and acpi_tz(4), and now it just works.
> 
> I'm wondering if other people are in a similar situation, and whether
> or not we should accept that kind of requirement from an ACPI
> implementation, and of course, if we should comply.
> 
> While deferring the attachment to envsys(9) is very easy to do and not
> really meaningless, it obviously break apm@acpi, and I don't see an
> easy way of working around that.  Not that I care a lot about that
> hack, though.  Besides, I lost a lot of time because of it.
> 
> Thoughts ?

Understood... at least you found what was the problem. The following
patch avoids calling the sme_gtredata function callback at register time
(sysmon_envsys_register()). This should fix at least the problem with
envsys2:

Index: sysmon_envsys.c
===================================================================
RCS file: /cvsroot/src/sys/dev/sysmon/sysmon_envsys.c,v
retrieving revision 1.44
diff -b -u -r1.44 sysmon_envsys.c
--- sysmon_envsys.c	27 Jul 2007 11:59:09 -0000	1.44
+++ sysmon_envsys.c	29 Jul 2007 10:01:08 -0000
@@ -636,7 +636,6 @@
 	 * values specified by the sysmon envsys driver.
 	 */
 	for (i = 0; i < sme->sme_nsensors; i++) {
-		mutex_enter(&sme_mtx);
 		/*
 		 * XXX:
 		 *
@@ -651,19 +650,6 @@
 				sme->sme_sensor_data[0].sensor = 0;
 
 		edata = &sme->sme_sensor_data[i];
-		/* 
-		 * refresh sensor data via sme_gtredata only if the
-		 * flag is not set.
-		 */
-		if ((sme->sme_flags & SME_DISABLE_GTREDATA) == 0) {
-			if ((*sme->sme_gtredata)(sme, edata)) {
-				DPRINTF(("%s: sme->sme_gtredata[%d]\n",
-				    __func__, i));
-				mutex_exit(&sme_mtx);
-				continue;
-			}
-		}
-		mutex_exit(&sme_mtx);
 		sme_make_dictionary(sme, array, edata);
 	}
 

Not sure, what to do about acpi_apm.c...

-- 
Juan Romero Pardines 	- The NetBSD Project
http://plog.xtrarom.org 	- NetBSD/pkgsrc news in Spanish