At Wed, 3 Mar 2010 18:01:33 -0800 (PST), Paul Goyette
<paul%whooppee.com@localhost> wrote:
Subject: Re: unable to build i386 ALL kernel, errors in smbus_acpi.c
(ACPI_SMBUS_COMPONENT undeclared)
>
> Please try the attached patch...
Your last checked in changes to smbus_acpi.c don't seem to have ever
been run through a compiler, at least not with the options I reported. :-)
As a side note I think that's the first time I've ever seen one file in
the NetBSD kernel blow up so bad that it not only blew away my 2000-line
scroll-back buffer, but would have done so even if it had been 5000
lines! This ACPI stuff must be pretty big and complex! Another 5061
lines of compiler output followed these few when I tried to compile
again today:
compile ALL/smbus_acpi.o
/rest/work/woods/m-NetBSD-current/sys/dev/acpi/smbus_acpi.c:42: error: expected
declaration specifiers or '...' before string consta
nt
cc1: warnings being treated as errors
In file included from /rest/work/woods/m-NetBSD-current/sys/sys/stdint.h:36,
from /rest/work/woods/m-NetBSD-current/sys/sys/inttypes.h:39,
from /rest/work/woods/m-NetBSD-current/sys/sys/param.h:100,
from
/rest/work/woods/m-NetBSD-current/sys/dev/acpi/smbus_acpi.c:44:
./machine/int_types.h:45: warning: return type defaults to 'int'
./machine/int_types.h:45: warning: function declaration isn't a prototype
./machine/int_types.h: In function 'ACPI_MODULE_NAME':
I think it really would help a lot to make sure to have run a successful
compile of the ALL kernel before committing any changes. That way it is
ensured that the most possible lines of code have been at least parsed
and compiled and that the results will link into a coherent binary
without any conflicts or missing bits. I realize developers might not
want to run two compiles every time they do anything (one to build
something to test, and one to test their changes in the context of the
largest workable environment), but not doing so can waste a whole lot of
other peoples time too.
The fix seems simple enough this time, based on observation of other
similar modules in the same directory:
--- smbus_acpi.c 04 Mar 2010 13:48:05 -0500 1.6
+++ smbus_acpi.c 04 Mar 2010 14:10:30 -0500
@@ -38,9 +38,6 @@
#include <sys/cdefs.h>
__KERNEL_RCSID(0, "$NetBSD: smbus_acpi.c,v 1.6 2010/03/04 13:11:14 pgoyette
Exp $");
-#define _COMPONENT ACPI_BUS_COMPONENT
-ACPI_MODULE_NAME ("smbus_acpi")
-
#include <sys/param.h>
#include <sys/systm.h>
#include <sys/kernel.h>
@@ -56,6 +53,9 @@
#include <dev/acpi/acpireg.h>
#include <dev/acpi/acpivar.h>
+#define _COMPONENT ACPI_BUS_COMPONENT
+ACPI_MODULE_NAME ("smbus_acpi")
+
#include <dev/i2c/i2cvar.h>
/*
At this point it seems as if your suggested patch is not necessary to
achieve a successful compile, though perhaps it would still be desirable
for other reasons -- I haven't tried it though.
Now I'm on to the next similar errors:
/rest/work/woods/m-NetBSD-current/sys/dev/acpi/atk0110.c: In function
'aibs_attach_sif':
/rest/work/woods/m-NetBSD-current/sys/dev/acpi/atk0110.c:174: error:
'ACPI_RESOURCE_COMPONENT' undeclared (first use in this function)
/rest/work/woods/m-NetBSD-current/sys/dev/acpi/atk0110.c:174: error: (Each
undeclared identifier is reported only once
/rest/work/woods/m-NetBSD-current/sys/dev/acpi/atk0110.c:174: error: for each
function it appears in.)
/rest/work/woods/m-NetBSD-current/sys/dev/acpi/atk0110.c: In function
'aibs_refresh':
/rest/work/woods/m-NetBSD-current/sys/dev/acpi/atk0110.c:333: error:
'ACPI_RESOURCE_COMPONENT' undeclared (first use in this function)
The following changes complete the ACPI-related fixes necessary to
compile the ALL kernel:
Index: sys/dev/acpi/atk0110.c
==================================================================RCS file:
/cvs/master/m-NetBSD/main/src/sys/dev/acpi/atk0110.c,v
retrieving revision 1.7
diff -u -r1.7 atk0110.c
--- sys/dev/acpi/atk0110.c 4 Mar 2010 08:44:55 -0000 1.7
+++ sys/dev/acpi/atk0110.c 4 Mar 2010 19:25:38 -0000
@@ -27,9 +27,6 @@
#include <dev/sysmon/sysmonvar.h>
-#include "acpi.h"
-#include "acpivar.h"
-
/*
* ASUSTeK AI Booster (ACPI ASOC ATK0110).
*
@@ -42,6 +39,10 @@
* -- Constantine A. Murenin <http://cnst.su/>
*/
+#include <dev/acpi/acpica.h>
+#include <dev/acpi/acpireg.h>
+#include <dev/acpi/acpivar.h>
+
#define _COMPONENT ACPI_RESOURCE_COMPONENT
ACPI_MODULE_NAME ("acpi_aibs")
Index: sys/dev/acpi/acpi.c
==================================================================RCS file:
/cvs/master/m-NetBSD/main/src/sys/dev/acpi/acpi.c,v
retrieving revision 1.152
diff -u -r1.152 acpi.c
--- sys/dev/acpi/acpi.c 3 Mar 2010 06:57:05 -0000 1.152
+++ sys/dev/acpi/acpi.c 4 Mar 2010 01:11:59 -0000
@@ -823,7 +823,7 @@
#ifdef ACPI_DEBUG
aprint_normal("acpi_activate_device: %s, old status=%x\n",
- (*di)->HardwareId.Value, (*di)->CurrentStatus);
+ (*di)->HardwareId.String, (*di)->CurrentStatus);
#endif
rv = acpi_allocate_resources(handle);
@@ -841,7 +841,7 @@
#ifdef ACPI_DEBUG
aprint_normal("acpi_activate_device: %s, new status=%x\n",
- (*di)->HardwareId.Value, (*di)->CurrentStatus);
+ (*di)->HardwareId.String, (*di)->CurrentStatus);
#endif
}
#endif /* ACPI_ACTIVATE_DEV */
The need for this last change kinda stuns me since someone went
carefully through the same file and changed every other instance of
HardwareId.Value -- but it looks as though they did it based only on one
set of compiler errors and not by truly reading and searching through
the code.
I made one more non-essential change to try to debug problems on some HP
hardware:
Index: sys/dev/acpi/acpi_tz.c
==================================================================RCS file:
/cvs/master/m-NetBSD/main/src/sys/dev/acpi/acpi_tz.c,v
retrieving revision 1.59
diff -u -r1.59 acpi_tz.c
--- sys/dev/acpi/acpi_tz.c 18 Feb 2010 14:10:15 -0000 1.59
+++ sys/dev/acpi/acpi_tz.c 25 Feb 2010 01:45:51 -0000
@@ -179,7 +179,7 @@
ACPI_STATUS rv;
ACPI_INTEGER v;
-#if 0
+#if ACPIVERBOSE
sc->sc_flags = ATZ_F_VERBOSE;
#endif
sc->sc_devnode = aa->aa_node;
@@ -255,7 +255,7 @@
}
if (acpitz_get_integer(dv, "_TMP", &tmp)) {
- aprint_error_dev(dv, "failed to evaluate _TMP\n");
+ aprint_error_dev(dv, "failed to get ACPI _TMP value, no
acpitz(4) support?\n");
return;
}
@@ -586,6 +586,11 @@
*tmp = ATZ_TMP_INVALID;
}
+/*
+ * XXX OpenBSD handles some values differently, "debouncing" invalid readings
+ * for some number of retries...
+ */
+
static int
acpitz_get_integer(device_t dv, const char *cm, UINT32 *val)
{
--
Greg A. Woods
Planix, Inc.
<woods%planix.com@localhost> +1 416 218 0099 http://www.planix.com/
Attachment:
pgpyeOPuSDwT4.pgp
Description: PGP signature