Subject: ACPI devices attachment tweak
To: None <tech-kern@netbsd.org>
From: Quentin Garnier <cube@cubidou.net>
List: tech-kern
Date: 04/19/2005 02:42:16
--xi8lRpXYeGgNnUjs
Content-Type: multipart/mixed; boundary="BlkQeOBdElZ1aiuH"
Content-Disposition: inline


--BlkQeOBdElZ1aiuH
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Hi,

Attached is a small patch against acpi.c and acpivar.h in sys/dev/acpi
that does the two following things:

 1.  Adds the name of the device in the probe line.  This is purely
     cosmetic, but helps a bit identifying devices when the DSDT
     contains explicit (well, as explicit as four letters can be)
     names, which however is not always the case.

     That leads to the following output:

EHCI (ACPI Object Type 'Device' [0x06]) at acpi0 not configured
RMEM (PNP0C01) [System Board] at acpi0 not configured
acpibut0 at acpi0 (SLPB, PNP0C0E): ACPI Sleep Button

     Of course, it will not help when the syumbols in the DSDT are
     named C001, C002 and so on.  I've seen only one like this so far
     over quite a few.

 2.  Completely rework the way the device tree is built, effectively
     turning it into a tree instead of a simple per-scope list.

     The point of this is to allow a real device tree inside the ACPI
     namespace, in order to get something like the following:

acpivga0 at acpi0 (VGA): ACPI VGA device
acpivga0: 2 managed output devices:
acpivga0:   1<BIOS>, ID 0x0100 (opaque)
acpivga0:   1<BIOS>, ID 0x0400 (opaque)
avdisplay0 at acpivga0: video output device
avdisplay0: ID 0x0100
avdisplay1 at acpivga0: video output device
avdisplay1: ID 0x0400

     Don't sweat, my acpivga code doesn't do anything but displaying
     the output above, but at least, that way of listing and attaching
     devices allows it to attach its own children (by walking the list
     inside its devnode struct).

     Note in the patch how the device, if one actually attaches, is
     responsible for attaching its own devices.  However, in case no
     device was attached, all the children are probed directly at the
     acpi level, just like we are used to (and that's how the acpivga
     device above attaches to acpi0).

     Note also that this patch is incomplete because it doesn't manage
     the ad_level member properly.  That's easy enough to fix later.

Comments?  I'll wait for the import of the new ACPI-CA code before
committing any of this, so I guess there is some time [hi kochi!].

--=20
Quentin Garnier - cube@cubidou.net - cube@NetBSD.org
"When I find the controls, I'll go where I like, I'll know where I want
to be, but maybe for now I'll stay right here on a silent sea."
KT Tunstall, Silent Sea, Eye to the Telescope, 2004.

--BlkQeOBdElZ1aiuH
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="acpi_attach.diff"
Content-Transfer-Encoding: quoted-printable

Index: acpi.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /pub/NetBSD-CVS/src/sys/dev/acpi/acpi.c,v
retrieving revision 1.68
diff -u -r1.68 acpi.c
--- acpi.c	27 Feb 2005 00:26:58 -0000	1.68
+++ acpi.c	19 Apr 2005 00:23:38 -0000
@@ -149,6 +149,7 @@
 static void		acpi_shutdown(void *);
 static ACPI_STATUS	acpi_disable(struct acpi_softc *sc);
 static void		acpi_build_tree(struct acpi_softc *);
+static void		acpi_config_device(struct acpi_softc *, struct acpi_devnode *=
);
 static ACPI_STATUS	acpi_make_devnode(ACPI_HANDLE, UINT32, void *, void **);
=20
 static void		acpi_enable_fixed_events(struct acpi_softc *);
@@ -388,6 +389,7 @@
 struct acpi_make_devnode_state {
 	struct acpi_softc *softc;
 	struct acpi_scope *scope;
+	struct acpi_devnode_list *parent;
 };
=20
 /*
@@ -406,7 +408,6 @@
 		"\\_TZ_",	/* ACPI 1.0 thermal zone namespace */
 		NULL,
 	};
-	struct acpi_attach_args aa;
 	struct acpi_make_devnode_state state;
 	struct acpi_scope *as;
 	struct acpi_devnode *ad;
@@ -429,55 +430,61 @@
 		TAILQ_INSERT_TAIL(&sc->sc_scopes, as, as_list);
=20
 		state.scope =3D as;
+		state.parent =3D &as->as_devnodes;
=20
 		rv =3D AcpiGetHandle(ACPI_ROOT_OBJECT, (char *) scopes[i],
 		    &parent);
-		if (ACPI_SUCCESS(rv)) {
-			AcpiWalkNamespace(ACPI_TYPE_ANY, parent, 100,
-			    acpi_make_devnode, &state, NULL);
-		}
+		if (ACPI_FAILURE(rv))
+			continue;
+		AcpiWalkNamespace(ACPI_TYPE_ANY, parent, 1,
+		    acpi_make_devnode, &state, NULL);
=20
 		/* Now, for this namespace, try and attach the devices. */
-		TAILQ_FOREACH(ad, &as->as_devnodes, ad_list) {
-			aa.aa_node =3D ad;
-			aa.aa_iot =3D sc->sc_iot;
-			aa.aa_memt =3D sc->sc_memt;
-			aa.aa_pc =3D sc->sc_pc;
-			aa.aa_pciflags =3D sc->sc_pciflags;
-			aa.aa_ic =3D sc->sc_ic;
-
-			if (ad->ad_devinfo->Type =3D=3D ACPI_TYPE_DEVICE) {
-				/*
-				 * XXX We only attach devices which are:
-				 *
-				 *	- present
-				 *	- enabled
-				 *	- functioning properly
-				 *
-				 * However, if enabled, it's decoding resources,
-				 * so we should claim them, if possible.
-				 * Requires changes to bus_space(9).
-				 */
-				if ((ad->ad_devinfo->Valid & ACPI_VALID_STA) =3D=3D
-				    ACPI_VALID_STA &&
-				    (ad->ad_devinfo->CurrentStatus &
-				     (ACPI_STA_DEV_PRESENT|ACPI_STA_DEV_ENABLED|
-				      ACPI_STA_DEV_OK)) !=3D
-				    (ACPI_STA_DEV_PRESENT|ACPI_STA_DEV_ENABLED|
-				     ACPI_STA_DEV_OK))
-					continue;
-
-				/*
-				 * XXX Same problem as above...
-				 */
-				if ((ad->ad_devinfo->Valid & ACPI_VALID_HID)
-				    =3D=3D 0)
-					continue;
-			}
+		TAILQ_FOREACH(ad, &as->as_devnodes, ad_list)
+			acpi_config_device(sc, ad);
+	}
+}
=20
-			ad->ad_device =3D config_found(&sc->sc_dev,
-			    &aa, acpi_print);
-		}
+static void
+acpi_config_device(struct acpi_softc *sc, struct acpi_devnode *ad)
+{
+	struct acpi_attach_args aa;
+	struct acpi_devnode *child;
+
+	aa.aa_node =3D ad;
+	aa.aa_iot =3D sc->sc_iot;
+	aa.aa_memt =3D sc->sc_memt;
+	aa.aa_pc =3D sc->sc_pc;
+	aa.aa_pciflags =3D sc->sc_pciflags;
+	aa.aa_ic =3D sc->sc_ic;
+
+	if (ad->ad_devinfo->Type =3D=3D ACPI_TYPE_DEVICE)
+		/*
+		 * XXX We only attach devices which are:
+		 *
+		 *	- present
+		 *	- enabled
+		 *	- functioning properly
+		 *
+		 * However, if enabled, it's decoding resources,
+		 * so we should claim them, if possible.
+		 * Requires changes to bus_space(9).
+		 */
+		if ((ad->ad_devinfo->Valid & ACPI_VALID_STA) =3D=3D
+		    ACPI_VALID_STA &&
+		    (ad->ad_devinfo->CurrentStatus &
+		     (ACPI_STA_DEV_PRESENT|ACPI_STA_DEV_ENABLED|
+		      ACPI_STA_DEV_OK)) !=3D
+		    (ACPI_STA_DEV_PRESENT|ACPI_STA_DEV_ENABLED|
+		     ACPI_STA_DEV_OK))
+			return;
+
+	if ((ad->ad_device =3D config_found(&sc->sc_dev,
+	    &aa, acpi_print)) =3D=3D NULL) {
+		/* Device wasn't configured.  Let's attach its
+		 * children ourselves */
+		TAILQ_FOREACH(child, &ad->ad_children, ad_list)
+			acpi_config_device(sc, child);
 	}
 }
=20
@@ -534,6 +541,7 @@
 	ACPI_BUFFER buf;
 	ACPI_DEVICE_INFO *devinfo;
 	ACPI_STATUS rv;
+	struct acpi_devnode_list *parent;
=20
 	rv =3D AcpiGetType(handle, &type);
 	if (ACPI_SUCCESS(rv)) {
@@ -575,8 +583,15 @@
 			ad->ad_level =3D level;
 			ad->ad_scope =3D as;
 			ad->ad_type =3D type;
+			TAILQ_INIT(&ad->ad_children);
+
+			TAILQ_INSERT_TAIL(state->parent, ad, ad_list);
=20
-			TAILQ_INSERT_TAIL(&as->as_devnodes, ad, ad_list);
+			parent =3D state->parent;
+			state->parent =3D &ad->ad_children;
+			AcpiWalkNamespace(ACPI_TYPE_ANY, handle, 1,
+			    acpi_make_devnode, state, NULL);
+			state->parent =3D parent;
=20
 			if ((ad->ad_devinfo->Valid & ACPI_VALID_HID) =3D=3D 0)
 				goto out;
@@ -612,6 +627,16 @@
 {
 	struct acpi_attach_args *aa =3D aux;
 	ACPI_STATUS rv;
+	ACPI_NAME_UNION *anu =3D (ACPI_NAME_UNION *)&aa->aa_node->ad_devinfo->Nam=
e;
+	char name[5] =3D { 0 };
+	int i, clear =3D 0;
+
+	for (i =3D 3; i >=3D 0; i--) {
+		if (!clear && anu->Ascii[i] =3D=3D '_')
+			clear =3D 1;
+		else
+			name[i] =3D anu->Ascii[i];
+	}
=20
 	if (pnp) {
 		if (aa->aa_node->ad_devinfo->Valid & ACPI_VALID_HID) {
@@ -619,7 +644,7 @@
 			    aa->aa_node->ad_devinfo->HardwareId.Value;
 			char *str;
=20
-			aprint_normal("%s ", pnpstr);
+			aprint_normal("%s (%s) ", name, pnpstr);
 			rv =3D acpi_eval_string(aa->aa_node->ad_handle,
 			    "_STR", &str);
 			if (ACPI_SUCCESS(rv)) {
@@ -642,14 +667,16 @@
=20
 #endif
 		} else {
-			aprint_normal("ACPI Object Type '%s' (0x%02x) ",
-			   AcpiUtGetTypeName(aa->aa_node->ad_devinfo->Type),
-			   aa->aa_node->ad_devinfo->Type);
+			aprint_normal("%s (ACPI Object Type '%s' "
+			    "[0x%02x]) ", name,
+			     AcpiUtGetTypeName(aa->aa_node->ad_devinfo->Type),
+			     aa->aa_node->ad_devinfo->Type);
 		}
 		aprint_normal("at %s", pnp);
 	} else {
+		aprint_normal(" (%s", name);
 		if (aa->aa_node->ad_devinfo->Valid & ACPI_VALID_HID) {
-			aprint_normal(" (%s", aa->aa_node->ad_devinfo->HardwareId.Value);
+			aprint_normal(", %s", aa->aa_node->ad_devinfo->HardwareId.Value);
 			if (aa->aa_node->ad_devinfo->Valid & ACPI_VALID_UID) {
 				char *uid;
=20
@@ -658,8 +685,8 @@
 					uid =3D "<null>";
 				aprint_normal("-%s", uid);
 			}
-			aprint_normal(")");
 		}
+		aprint_normal(")");
 	}
=20
 	return UNCONF;
Index: acpivar.h
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /pub/NetBSD-CVS/src/sys/dev/acpi/acpivar.h,v
retrieving revision 1.20
diff -u -r1.20 acpivar.h
--- acpivar.h	26 May 2004 17:15:17 -0000	1.20
+++ acpivar.h	19 Apr 2005 00:23:38 -0000
@@ -76,6 +76,10 @@
  *
  *	An ACPI device node.
  */
+
+struct acpi_devnode;
+TAILQ_HEAD(acpi_devnode_list, acpi_devnode);
+
 struct acpi_devnode {
 	TAILQ_ENTRY(acpi_devnode) ad_list;
 	ACPI_HANDLE	ad_handle;	/* our ACPI handle */
@@ -84,6 +88,7 @@
 	ACPI_DEVICE_INFO *ad_devinfo;	/* our ACPI device info */
 	struct acpi_scope *ad_scope;	/* backpointer to scope */
 	struct device	*ad_device;	/* pointer to configured device */
+	struct acpi_devnode_list ad_children;	/* children devices */
 };
=20
 /*
@@ -97,7 +102,7 @@
 	/*
 	 * Device nodes we manage.
 	 */
-	TAILQ_HEAD(, acpi_devnode) as_devnodes;
+	struct acpi_devnode_list as_devnodes;
 };
=20
 /*

--BlkQeOBdElZ1aiuH--

--xi8lRpXYeGgNnUjs
Content-Type: application/pgp-signature
Content-Disposition: inline

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.6 (NetBSD)

iQEVAwUBQmRT6NgoQloHrPnoAQKkOAgAtCAl20XqQwpShiO9yBhG4e4nZpkm7RMn
70mte4FpGsdxPtT5suTcsGcAhiKsB/CanhJT4g+5ps2GoyV1/zJPxH9feXh6hxXO
dBcU5+TsaNpqv1LGeO9l5z639wIZ04WGISJpzUVP2M8vCuotffjq0WymNrIt15XY
M1Mp3MjxAS3lhdG+k/DovVzjjGAwlMobPJSvhvkmN84Qk5Lf41ezMvNJxBjbr8sC
JRjdNy6GB07HuBIVboO3ztSz3Ip3Sib5AfihYikZhBsoo/JxExlpZ+3ao+nRmiGT
k6uIHGEchlMxT6hGoBURVQNt1fWabM8xNfwngtSr5Bm1pK1oDzVOUA==
=jy4P
-----END PGP SIGNATURE-----

--xi8lRpXYeGgNnUjs--