Probe CPUs after the PCI hierarchy on i386, amd64, and ia64. This allows

the cpufreq drivers to reliably use properties of PCI devices for quirks,
etc.
- For the legacy drivers, add CPU devices via an identify routine in the
  CPU driver itself rather than in the legacy driver's attach routine.
- Add CPU devices after Host-PCI bridges in the acpi bus driver.
- Change the ichss(4) driver to use pci_find_bsf() to locate the ICH and
  check its device ID rather than having a bogus PCI attachment that only
  checked for the ID in probe and always failed.  As a side effect, you
  can now kldload ichss after boot.
- Fix the ichss(4) driver to use the correct device_t for the ICH (and not
  for ichss0) when doing PCI config space operations to enable SpeedStep.

MFC after:	2 weeks
Reviewed by:	njl, Andriy Gapon  avg of icyb.net.ua

2 files changed, 72 insertions, 67 deletions

diff --git a/sys/dev/acpica/acpi.c b/sys/dev/acpica/acpi.c
index dba4edd..44a9012 100644
--- a/sys/dev/acpica/acpi.c
+++ b/sys/dev/acpica/acpi.c
@@ -1533,18 +1533,31 @@ acpi_probe_children(device_t bus)
 static int
 acpi_probe_order(ACPI_HANDLE handle, int *order)
 {
+    ACPI_OBJECT_TYPE type;
+    u_int addr;
 
     /*
      * 1. I/O port and memory system resource holders
      * 2. Embedded controllers (to handle early accesses)
      * 3. PCI Link Devices
+     * 11 - 266. Host-PCI bridges sorted by _ADR
+     * 280. CPUs
      */
+    AcpiGetType(handle, &type);
     if (acpi_MatchHid(handle, "PNP0C01") || acpi_MatchHid(handle, "PNP0C02"))
 	*order = 1;
     else if (acpi_MatchHid(handle, "PNP0C09"))
 	*order = 2;
     else if (acpi_MatchHid(handle, "PNP0C0F"))
 	*order = 3;
+    else if (acpi_MatchHid(handle, "PNP0A03")) {
+	if (ACPI_SUCCESS(acpi_GetInteger(handle, "_ADR", &addr)))
+	    *order = 11 + ACPI_ADR_PCI_SLOT(addr) * (PCI_FUNCMAX + 1) +
+	       ACPI_ADR_PCI_FUNC(addr);
+	else
+	    *order = 11;
+    } else if (type == ACPI_TYPE_PROCESSOR)
+	*order = 280;
     return (0);
 }
 
@@ -1591,14 +1604,17 @@ acpi_probe_child(ACPI_HANDLE handle, UINT32 level, void *context, void **status)
 		break;
 
 	    /* 
-	     * Create a placeholder device for this node.  Sort the placeholder
-	     * so that the probe/attach passes will run breadth-first.  Orders
-	     * less than ACPI_DEV_BASE_ORDER are reserved for special objects
-	     * (i.e., system resources).  Larger values are used for all other
-	     * devices.
+	     * Create a placeholder device for this node.  Sort the
+	     * placeholder so that the probe/attach passes will run
+	     * breadth-first.  Orders less than ACPI_DEV_BASE_ORDER
+	     * are reserved for special objects (i.e., system
+	     * resources).  Orders between ACPI_DEV_BASE_ORDER and 300
+	     * are used for Host-PCI bridges (and effectively all
+	     * their children) and CPUs.  Larger values are used for
+	     * all other devices.
 	     */
 	    ACPI_DEBUG_PRINT((ACPI_DB_OBJECTS, "scanning '%s'\n", handle_str));
-	    order = (level + 1) * ACPI_DEV_BASE_ORDER;
+	    order = level * 10 + 300;
 	    acpi_probe_order(handle, &order);
 	    child = BUS_ADD_CHILD(bus, order, NULL, -1);
 	    if (child == NULL)
diff --git a/sys/dev/cpufreq/ichss.c b/sys/dev/cpufreq/ichss.c
index 13d004d..179886e 100644
--- a/sys/dev/cpufreq/ichss.c
+++ b/sys/dev/cpufreq/ichss.c
@@ -91,7 +91,7 @@ struct ichss_softc {
 	(bus_space_write_1(rman_get_bustag((reg)), 	\
 	    rman_get_bushandle((reg)), 0, (val)))
 
-static int	ichss_pci_probe(device_t dev);
+static void	ichss_identify(driver_t *driver, device_t parent);
 static int	ichss_probe(device_t dev);
 static int	ichss_attach(device_t dev);
 static int	ichss_detach(device_t dev);
@@ -103,6 +103,7 @@ static int	ichss_type(device_t dev, int *type);
 
 static device_method_t ichss_methods[] = {
 	/* Device interface */
+	DEVMETHOD(device_identify,	ichss_identify),
 	DEVMETHOD(device_probe,		ichss_probe),
 	DEVMETHOD(device_attach,	ichss_attach),
 	DEVMETHOD(device_detach,	ichss_detach),
@@ -120,15 +121,7 @@ static driver_t ichss_driver = {
 static devclass_t ichss_devclass;
 DRIVER_MODULE(ichss, cpu, ichss_driver, ichss_devclass, 0, 0);
 
-static device_method_t ichss_pci_methods[] = {
-	DEVMETHOD(device_probe,		ichss_pci_probe),
-	{0, 0}
-};
-static driver_t ichss_pci_driver = {
-	"ichss_pci", ichss_pci_methods, 0
-};
-static devclass_t ichss_pci_devclass;
-DRIVER_MODULE(ichss_pci, pci, ichss_pci_driver, ichss_pci_devclass, 0, 0);
+static device_t ich_device;
 
 #if 0
 #define DPRINT(x...)	printf(x)
@@ -136,70 +129,69 @@ DRIVER_MODULE(ichss_pci, pci, ichss_pci_driver, ichss_pci_devclass, 0, 0);
 #define DPRINT(x...)
 #endif
 
-/*
- * We detect the chipset by looking for its LPC bus ID during the PCI
- * scan and reading its config registers during the probe.  However,
- * we add the ichss child under the cpu device since even though the
- * chipset provides the control, it really affects the cpu only.
- *
- * XXX This approach does not work if the module is loaded after boot.
- */
-static int
-ichss_pci_probe(device_t dev)
+static void
+ichss_identify(driver_t *driver, device_t parent)
 {
-	device_t child, parent;
+	device_t child;
 	uint32_t pmbase;
 
+	if (resource_disabled("ichss", 0))
+		return;
+
 	/*
-	 * TODO: add a quirk to disable if we see the 82815_MC along
-	 * with the 82801BA and revision < 5.
+	 * It appears that ICH SpeedStep only requires a single CPU to
+	 * set the value (since the chipset is shared by all CPUs.)
+	 * Thus, we only add a child to cpu 0.
 	 */
-	if (pci_get_vendor(dev) != PCI_VENDOR_INTEL ||
-	    (pci_get_device(dev) != PCI_DEV_82801BA &&
-	    pci_get_device(dev) != PCI_DEV_82801CA &&
-	    pci_get_device(dev) != PCI_DEV_82801DB))
-		return (ENXIO);
+	if (device_get_unit(parent) != 0)
+		return;
 
-	/* Only one CPU is supported for this hardware. */
-	if (devclass_get_device(ichss_devclass, 0))
-		return (ENXIO);
+	/* Avoid duplicates. */
+	if (device_find_child(parent, "ichss", -1))
+		return;
 
 	/*
-	 * Add a child under the CPU parent.  It appears that ICH SpeedStep
-	 * only requires a single CPU to set the value (since the chipset
-	 * is shared by all CPUs.)  Thus, we only add a child to cpu 0.
+	 * ICH2/3/4-M I/O Controller Hub is at bus 0, slot 1F, function 0.
+	 * E.g. see Section 6.1 "PCI Devices and Functions" and table 6.1 of
+	 * Intel(r) 82801BA I/O Controller Hub 2 (ICH2) and Intel(r) 82801BAM
+	 * I/O Controller Hub 2 Mobile (ICH2-M).
+	 *
+	 * TODO: add a quirk to disable if we see the 82815_MC along
+	 * with the 82801BA and revision < 5.
 	 */
-	parent = devclass_get_device(devclass_find("cpu"), 0);
-	KASSERT(parent != NULL, ("cpu parent is NULL"));
-	child = BUS_ADD_CHILD(parent, 0, "ichss", 0);
-	if (child == NULL) {
-		device_printf(parent, "add SpeedStep child failed\n");
-		return (ENXIO);
-	}
+	ich_device = pci_find_bsf(0, 0x1f, 0);
+	if (ich_device == NULL ||
+	    pci_get_vendor(ich_device) != PCI_VENDOR_INTEL ||
+	    (pci_get_device(ich_device) != PCI_DEV_82801BA &&
+	    pci_get_device(ich_device) != PCI_DEV_82801CA &&
+	    pci_get_device(ich_device) != PCI_DEV_82801DB))
+		return;
 
 	/* Find the PMBASE register from our PCI config header. */
-	pmbase = pci_read_config(dev, ICHSS_PMBASE_OFFSET, sizeof(pmbase));
+	pmbase = pci_read_config(ich_device, ICHSS_PMBASE_OFFSET,
+	    sizeof(pmbase));
 	if ((pmbase & ICHSS_IO_REG) == 0) {
 		printf("ichss: invalid PMBASE memory type\n");
-		return (ENXIO);
+		return;
 	}
 	pmbase &= ICHSS_PMBASE_MASK;
 	if (pmbase == 0) {
 		printf("ichss: invalid zero PMBASE address\n");
-		return (ENXIO);
+		return;
 	}
 	DPRINT("ichss: PMBASE is %#x\n", pmbase);
 
+	child = BUS_ADD_CHILD(parent, 0, "ichss", 0);
+	if (child == NULL) {
+		device_printf(parent, "add SpeedStep child failed\n");
+		return;
+	}
+
 	/* Add the bus master arbitration and control registers. */
 	bus_set_resource(child, SYS_RES_IOPORT, 0, pmbase + ICHSS_BM_OFFSET,
 	    1);
 	bus_set_resource(child, SYS_RES_IOPORT, 1, pmbase + ICHSS_CTRL_OFFSET,
 	    1);
-
-	/* Attach the new CPU child now. */
-	device_probe_and_attach(child);
-
-	return (ENXIO);
 }
 
 static int
@@ -207,10 +199,6 @@ ichss_probe(device_t dev)
 {
 	device_t est_dev, perf_dev;
 	int error, type;
-	uint16_t ss_en;
-
-	if (resource_disabled("ichss", 0))
-		return (ENXIO);
 
 	/*
 	 * If the ACPI perf driver has attached and is not just offering
@@ -227,14 +215,6 @@ ichss_probe(device_t dev)
 	if (est_dev && device_is_attached(est_dev))
 		return (ENXIO);
 
-	/* Activate SpeedStep control if not already enabled. */
-	ss_en = pci_read_config(dev, ICHSS_PMCFG_OFFSET, sizeof(ss_en));
-	if ((ss_en & ICHSS_ENABLE) == 0) {
-		printf("ichss: enabling SpeedStep support\n");
-		pci_write_config(dev, ICHSS_PMCFG_OFFSET,
-		    ss_en | ICHSS_ENABLE, sizeof(ss_en));
-	}
-
 	device_set_desc(dev, "SpeedStep ICH");
 	return (-1000);
 }
@@ -243,6 +223,7 @@ static int
 ichss_attach(device_t dev)
 {
 	struct ichss_softc *sc;
+	uint16_t ss_en;
 
 	sc = device_get_softc(dev);
 	sc->dev = dev;
@@ -264,6 +245,14 @@ ichss_attach(device_t dev)
 		return (ENXIO);
 	}
 
+	/* Activate SpeedStep control if not already enabled. */
+	ss_en = pci_read_config(ich_device, ICHSS_PMCFG_OFFSET, sizeof(ss_en));
+	if ((ss_en & ICHSS_ENABLE) == 0) {
+		device_printf(dev, "enabling SpeedStep support\n");
+		pci_write_config(ich_device, ICHSS_PMCFG_OFFSET,
+		    ss_en | ICHSS_ENABLE, sizeof(ss_en));
+	}
+
 	/* Setup some defaults for our exported settings. */
 	sc->sets[0].freq = CPUFREQ_VAL_UNKNOWN;
 	sc->sets[0].volts = CPUFREQ_VAL_UNKNOWN;