[PATCH 00/10] make every acpi_device have a HID

[PATCH 00/10] make every acpi_device have a HID

[Bjorn Helgaas]

This series was prompted by an issue we saw recently in the mm tree:
http://lkml.org/lkml/2009/7/20/422

Synopsis:

    Linux/ACPI discovered a video device.  The device had no _HID or _CID
    methods, but based on acpi_is_video_device(), we added a sythetic CID
    ("LNXVIDEO"), so the device ended up with a CID but no HID.

    We bound the acpi_video_bus (drivers/acpi/video.c) driver to the device
    based on the CID.  This driver has a .notify() method, so we tried to
    install a notify handler on the device.  This uses strcmp() on the
    device HID to determine whether it's a fixed hardware device, and this
    oopsed because this device had no HID.

    This used to work because the HID was stored in an array that was
    initialized to zeroes, but a recent change replaced this with a pointer
    that is NULL until a HID is set.

This thread: http://marc.info/?l=linux-acpi&m=124959955813409&w=2
has patches that work around this by basically returning a pointer to an
empty string rather than a NULL pointer in this case.

However, the HID is a pretty central part of an acpi_device, and I think it
complicates the driver model too much to deal with HIDs that might be empty
strings.  I think it's better for Linux/ACPI to just make sure that *every*
acpi_device has at least one ID (either a HID, a CID, or a synthetic ID).

The main place this default "device" ID appears is with ACPI devices that
just have _ADR methods, e.g., PCI slots.

This series:
    - Applies on top of this previous series:
	http://marc.info/?l=linux-acpi&m=124907619701194&w=2
      (Len said these were applied to acpi-test, but I haven't actually
      seen that yet.)
    - Fixes a memory leak introduced by the previous series.  Could be
      folded into:
	http://marc.info/?l=linux-acpi&m=124907622001229&w=2
      if git tree management allows.
    - Fixes the synthetic HID for \_SB_.  This has been broken for a long
      time, so we've been seeing "device:00" instead of "LNXSYBUS" in
      sysfs.
    - Makes sure every acpi_device has a default "device" HID if nothing
      else.  (We already used "device" in sysfs if we didn't have a HID or
      CID.)
    - Adds a single list containing the HID and any CIDs.  No users make a
      distinction between them, so maintaining them separately just made
      things complicated.
    - Removes the _UID stuff, which nobody uses.

---

Bjorn Helgaas (10):
      ACPI: fix leak in debug code
      ACPI: add ACPI_IS_ROOT_DEVICE()
      ACPI: fix synthetic HID for \_SB_
      ACPI: use acpi_device_hid() when possible
      ACPI: make sure every acpi_device has an ID
      ACPI: maintain a single list of _HID and _CID IDs
      ACPI: remove acpi_device.flags.compatible_ids
      ACPI: remove acpi_device.flags.hardware_id
      ACPI: remove acpi_device_uid() and related stuff
      ACPI: simplify building device HID/CID list


 drivers/acpi/scan.c        |  260 +++++++++++++++++---------------------------
 drivers/pnp/pnpacpi/core.c |   21 ++--
 include/acpi/acpi_bus.h    |   19 +--
 3 files changed, 117 insertions(+), 183 deletions(-)

-- 
Bjorn

[PATCH 01/10] ACPI: fix leak in debug code

[Bjorn Helgaas]

Free the buffer after getting the namespace pathname.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
 drivers/acpi/scan.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 87d3df4..cbeb4c0 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1299,6 +1299,7 @@ end:
 			 (char *) buffer.pointer,
 			 device->parent ? dev_name(&device->parent->dev) :
 					  "(null)"));
+		kfree(buffer.pointer);
 		*child = device;
 	} else {
 		kfree(device->pnp.cid_list);

[PATCH 02/10] ACPI: add ACPI_IS_ROOT_DEVICE()

[Bjorn Helgaas]

This adds a macro to identify the root of the acpi_device tree.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
 drivers/acpi/scan.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index cbeb4c0..a253373 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -22,6 +22,8 @@ extern struct acpi_device *acpi_root;
 #define ACPI_BUS_HID			"LNXSYBUS"
 #define ACPI_BUS_DEVICE_NAME		"System Bus"
 
+#define ACPI_IS_ROOT_DEVICE(device)	(!(device)->parent)
+
 static LIST_HEAD(acpi_device_list);
 static LIST_HEAD(acpi_bus_id_list);
 DEFINE_MUTEX(acpi_device_lock);
@@ -950,7 +952,7 @@ static void acpi_device_get_busid(struct acpi_device *device)
 	 * The device's Bus ID is simply the object name.
 	 * TBD: Shouldn't this value be unique (within the ACPI namespace)?
 	 */
-	if (!device->parent) {
+	if (ACPI_IS_ROOT_DEVICE(device)) {
 		strcpy(device->pnp.bus_id, "ACPI");
 		return;
 	}
@@ -1033,7 +1035,7 @@ static void acpi_device_set_id(struct acpi_device *device)
 
 	switch (device->device_type) {
 	case ACPI_BUS_TYPE_DEVICE:
-		if (!device->parent) {
+		if (ACPI_IS_ROOT_DEVICE(device)) {
 			hid = ACPI_SYSTEM_HID;
 			break;
 		}

[PATCH 03/10] ACPI: fix synthetic HID for \_SB_

[Bjorn Helgaas]

This makes \_SB_ show up as /sys/devices/LNXSYSTM:00/LNXSYBUS:00
rather than "device:00".  This has been broken for a loooong time
(at least since 2.6.13) because device->parent is an acpi_device
pointer, not a handle.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
 drivers/acpi/scan.c |   18 ++++++------------
 1 files changed, 6 insertions(+), 12 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index a253373..6de6b11 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1038,6 +1038,12 @@ static void acpi_device_set_id(struct acpi_device *device)
 		if (ACPI_IS_ROOT_DEVICE(device)) {
 			hid = ACPI_SYSTEM_HID;
 			break;
+		} else if (ACPI_IS_ROOT_DEVICE(device->parent)) {
+			/* \_SB_, the only root-level namespace device */
+			hid = ACPI_BUS_HID;
+			strcpy(device->pnp.device_name, ACPI_BUS_DEVICE_NAME);
+			strcpy(device->pnp.device_class, ACPI_BUS_CLASS);
+			break;
 		}
 
 		status = acpi_get_object_info(device->handle, &buffer);
@@ -1088,18 +1094,6 @@ static void acpi_device_set_id(struct acpi_device *device)
 		break;
 	}
 
-	/*
-	 * \_SB
-	 * ----
-	 * Fix for the system root bus device -- the only root-level device.
-	 */
-	if (((acpi_handle)device->parent == ACPI_ROOT_OBJECT) &&
-	     (device->device_type == ACPI_BUS_TYPE_DEVICE)) {
-		hid = ACPI_BUS_HID;
-		strcpy(device->pnp.device_name, ACPI_BUS_DEVICE_NAME);
-		strcpy(device->pnp.device_class, ACPI_BUS_CLASS);
-	}
-
 	if (hid) {
 		strcpy(device->pnp.hardware_id, hid);
 		device->flags.hardware_id = 1;

[PATCH 04/10] ACPI: use acpi_device_hid() when possible

[Bjorn Helgaas]

Use acpi_device_hid() rather than accessing acpi_device.pnp.hardware_id
directly.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
 drivers/acpi/scan.c        |    6 +++---
 drivers/pnp/pnpacpi/core.c |    2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 6de6b11..8a7616f 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -185,7 +185,7 @@ static ssize_t
 acpi_device_hid_show(struct device *dev, struct device_attribute *attr, char *buf) {
 	struct acpi_device *acpi_dev = to_acpi_device(dev);
 
-	return sprintf(buf, "%s\n", acpi_dev->pnp.hardware_id);
+	return sprintf(buf, "%s\n", acpi_device_hid(acpi_dev));
 }
 static DEVICE_ATTR(hid, 0444, acpi_device_hid_show, NULL);
 
@@ -497,7 +497,7 @@ static int acpi_device_register(struct acpi_device *device)
 	 * If failed, create one and link it into acpi_bus_id_list
 	 */
 	list_for_each_entry(acpi_device_bus_id, &acpi_bus_id_list, node) {
-		if(!strcmp(acpi_device_bus_id->bus_id, device->flags.hardware_id? device->pnp.hardware_id : "device")) {
+		if (!strcmp(acpi_device_bus_id->bus_id, device->flags.hardware_id ? acpi_device_hid(device) : "device")) {
 			acpi_device_bus_id->instance_no ++;
 			found = 1;
 			kfree(new_bus_id);
@@ -506,7 +506,7 @@ static int acpi_device_register(struct acpi_device *device)
 	}
 	if (!found) {
 		acpi_device_bus_id = new_bus_id;
-		strcpy(acpi_device_bus_id->bus_id, device->flags.hardware_id ? device->pnp.hardware_id : "device");
+		strcpy(acpi_device_bus_id->bus_id, device->flags.hardware_id ? acpi_device_hid(device) : "device");
 		acpi_device_bus_id->instance_no = 0;
 		list_add_tail(&acpi_device_bus_id->node, &acpi_bus_id_list);
 	}
diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
index 9496494..1f5e2c1 100644
--- a/drivers/pnp/pnpacpi/core.c
+++ b/drivers/pnp/pnpacpi/core.c
@@ -234,7 +234,7 @@ static int __init acpi_pnp_match(struct device *dev, void *_pnp)
 	/* true means it matched */
 	return acpi->flags.hardware_id
 	    && !acpi_get_physical_device(acpi->handle)
-	    && compare_pnp_id(pnp->id, acpi->pnp.hardware_id);
+	    && compare_pnp_id(pnp->id, acpi_device_hid(acpi));
 }
 
 static int __init acpi_pnp_find_device(struct device *dev, acpi_handle * handle)

[PATCH 05/10] ACPI: make sure every acpi_device has an ID

[Bjorn Helgaas]

This makes sure every acpi_device has at least one ID.  If we build an
acpi_device for a namespace node with no _HID or _CID, we sometimes
synthesize an ID like "LNXCPU" or "LNXVIDEO".  If we don't even have
that, give it a default "device" ID.

Note that this means things like:
    /sys/devices/LNXSYSTM:00/LNXSYBUS:00/HWP0001:00/HWP0002:04/device:00
(a PCI slot SxFy device) will have "hid" and "modprobe" entries, where
they didn't before.  These aren't very useful (a HID of "device" doesn't
tell you what *kind* of device it is, so it doesn't help find a driver),
but I don't think they're harmful.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
 drivers/acpi/scan.c |   10 ++++++++++
 1 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 8a7616f..88f39e1 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1094,6 +1094,16 @@ static void acpi_device_set_id(struct acpi_device *device)
 		break;
 	}
 
+	/*
+	 * We build acpi_devices for some objects that don't have _HID or _CID,
+	 * e.g., PCI bridges and slots.  Drivers can't bind to these objects,
+	 * but we do use them indirectly by traversing the acpi_device tree.
+	 * This generic ID isn't useful for driver binding, but it provides
+	 * the useful property that "every acpi_device has an ID."
+	 */
+	if (!hid && !cid_list && !cid_add)
+		hid = "device";
+
 	if (hid) {
 		strcpy(device->pnp.hardware_id, hid);
 		device->flags.hardware_id = 1;

[PATCH 06/10] ACPI: maintain a single list of _HID and _CID IDs

[Bjorn Helgaas]

There's no need to treat _HID and _CID differently.  Keeping them in
a single list makes code that uses the IDs a little simpler because it
can just traverse the list rather than checking "do we have a HID?",
"do we have any CIDs?"

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
Reviewed-by: Alex Chiang <achiang@hp.com>
---
 drivers/acpi/scan.c        |  136 +++++++++++++++++---------------------------
 drivers/pnp/pnpacpi/core.c |   16 ++---
 include/acpi/acpi_bus.h    |   11 ++--
 3 files changed, 68 insertions(+), 95 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 88f39e1..76ba200 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -45,6 +45,7 @@ static int create_modalias(struct acpi_device *acpi_dev, char *modalias,
 {
 	int len;
 	int count;
+	struct acpi_hardware_id *id;
 
 	if (!acpi_dev->flags.hardware_id && !acpi_dev->flags.compatible_ids)
 		return -ENODEV;
@@ -52,33 +53,14 @@ static int create_modalias(struct acpi_device *acpi_dev, char *modalias,
 	len = snprintf(modalias, size, "acpi:");
 	size -= len;
 
-	if (acpi_dev->flags.hardware_id) {
-		count = snprintf(&modalias[len], size, "%s:",
-				 acpi_dev->pnp.hardware_id);
+	list_for_each_entry(id, &acpi_dev->pnp.ids, list) {
+		count = snprintf(&modalias[len], size, "%s:", id->id);
 		if (count < 0 || count >= size)
 			return -EINVAL;
 		len += count;
 		size -= count;
 	}
 
-	if (acpi_dev->flags.compatible_ids) {
-		struct acpi_compatible_id_list *cid_list;
-		int i;
-
-		cid_list = acpi_dev->pnp.cid_list;
-		for (i = 0; i < cid_list->count; i++) {
-			count = snprintf(&modalias[len], size, "%s:",
-					 cid_list->id[i].value);
-			if (count < 0 || count >= size) {
-				printk(KERN_ERR PREFIX "%s cid[%i] exceeds event buffer size",
-				       acpi_dev->pnp.device_name, i);
-				break;
-			}
-			len += count;
-			size -= count;
-		}
-	}
-
 	modalias[len] = '\0';
 	return len;
 }
@@ -273,6 +255,7 @@ int acpi_match_device_ids(struct acpi_device *device,
 			  const struct acpi_device_id *ids)
 {
 	const struct acpi_device_id *id;
+	struct acpi_hardware_id *hwid;
 
 	/*
 	 * If the device is not present, it is unnecessary to load device
@@ -281,36 +264,30 @@ int acpi_match_device_ids(struct acpi_device *device,
 	if (!device->status.present)
 		return -ENODEV;
 
-	if (device->flags.hardware_id) {
-		for (id = ids; id->id[0]; id++) {
-			if (!strcmp((char*)id->id, device->pnp.hardware_id))
+	for (id = ids; id->id[0]; id++)
+		list_for_each_entry(hwid, &device->pnp.ids, list)
+			if (!strcmp((char *) id->id, hwid->id))
 				return 0;
-		}
-	}
-
-	if (device->flags.compatible_ids) {
-		struct acpi_compatible_id_list *cid_list = device->pnp.cid_list;
-		int i;
-
-		for (id = ids; id->id[0]; id++) {
-			/* compare multiple _CID entries against driver ids */
-			for (i = 0; i < cid_list->count; i++) {
-				if (!strcmp((char*)id->id,
-					    cid_list->id[i].value))
-					return 0;
-			}
-		}
-	}
 
 	return -ENOENT;
 }
 EXPORT_SYMBOL(acpi_match_device_ids);
 
+static void acpi_free_ids(struct acpi_device *device)
+{
+	struct acpi_hardware_id *id, *tmp;
+
+	list_for_each_entry_safe(id, tmp, &device->pnp.ids, list) {
+		kfree(id->id);
+		kfree(id);
+	}
+}
+
 static void acpi_device_release(struct device *dev)
 {
 	struct acpi_device *acpi_dev = to_acpi_device(dev);
 
-	kfree(acpi_dev->pnp.cid_list);
+	acpi_free_ids(acpi_dev);
 	kfree(acpi_dev);
 }
 
@@ -1023,6 +1000,32 @@ static int acpi_dock_match(struct acpi_device *device)
 	return acpi_get_handle(device->handle, "_DCK", &tmp);
 }
 
+char *acpi_device_hid(struct acpi_device *device)
+{
+	struct acpi_hardware_id *hid;
+
+	hid = list_first_entry(&device->pnp.ids, struct acpi_hardware_id, list);
+	return hid->id;
+}
+
+static void acpi_add_id(struct acpi_device *device, const char *dev_id)
+{
+	struct acpi_hardware_id *id;
+
+	id = kmalloc(sizeof(*id), GFP_KERNEL);
+	if (!id)
+		return;
+
+	id->id = kmalloc(strlen(dev_id) + 1, GFP_KERNEL);
+	if (!id->id) {
+		kfree(id);
+		return;
+	}
+
+	strcpy(id->id, dev_id);
+	list_add_tail(&id->list, &device->pnp.ids);
+}
+
 static void acpi_device_set_id(struct acpi_device *device)
 {
 	struct acpi_device_info *info;
@@ -1032,6 +1035,7 @@ static void acpi_device_set_id(struct acpi_device *device)
 	struct acpi_compatible_id_list *cid_list = NULL;
 	const char *cid_add = NULL;
 	acpi_status status;
+	int i;
 
 	switch (device->device_type) {
 	case ACPI_BUS_TYPE_DEVICE:
@@ -1105,52 +1109,19 @@ static void acpi_device_set_id(struct acpi_device *device)
 		hid = "device";
 
 	if (hid) {
-		strcpy(device->pnp.hardware_id, hid);
+		acpi_add_id(device, hid);
 		device->flags.hardware_id = 1;
 	}
 	if (uid) {
 		strcpy(device->pnp.unique_id, uid);
 		device->flags.unique_id = 1;
 	}
-	if (cid_list || cid_add) {
-		struct  acpi_compatible_id_list *list;
-		int size = 0;
-		int count = 0;
-
-		if (cid_list) {
-			size = cid_list->size;
-		} else if (cid_add) {
-			size = sizeof(struct acpi_compatible_id_list);
-			cid_list = ACPI_ALLOCATE_ZEROED((acpi_size) size);
-			if (!cid_list) {
-				printk(KERN_ERR "Memory allocation error\n");
-				kfree(buffer.pointer);
-				return;
-			} else {
-				cid_list->count = 0;
-				cid_list->size = size;
-			}
-		}
-		if (cid_add)
-			size += sizeof(struct acpi_compatible_id);
-		list = kmalloc(size, GFP_KERNEL);
-
-		if (list) {
-			if (cid_list) {
-				memcpy(list, cid_list, cid_list->size);
-				count = cid_list->count;
-			}
-			if (cid_add) {
-				strncpy(list->id[count].value, cid_add,
-					ACPI_MAX_CID_LENGTH);
-				count++;
-				device->flags.compatible_ids = 1;
-			}
-			list->size = size;
-			list->count = count;
-			device->pnp.cid_list = list;
-		} else
-			printk(KERN_ERR PREFIX "Memory allocation error\n");
+	if (cid_list)
+		for (i = 0; i < cid_list->count; i++)
+			acpi_add_id(device, cid_list->id[i].value);
+	if (cid_add) {
+		acpi_add_id(device, cid_add);
+		device->flags.compatible_ids = 1;
 	}
 
 	kfree(buffer.pointer);
@@ -1217,6 +1188,7 @@ static int acpi_add_single_object(struct acpi_device **child,
 		return -ENOMEM;
 	}
 
+	INIT_LIST_HEAD(&device->pnp.ids);
 	device->device_type = type;
 	device->handle = handle;
 	device->parent = acpi_bus_get_parent(handle);
@@ -1308,7 +1280,7 @@ end:
 		kfree(buffer.pointer);
 		*child = device;
 	} else {
-		kfree(device->pnp.cid_list);
+		acpi_free_ids(device);
 		kfree(device);
 	}
 
diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
index 1f5e2c1..3a4478f 100644
--- a/drivers/pnp/pnpacpi/core.c
+++ b/drivers/pnp/pnpacpi/core.c
@@ -153,6 +153,7 @@ static int __init pnpacpi_add_device(struct acpi_device *device)
 	acpi_handle temp = NULL;
 	acpi_status status;
 	struct pnp_dev *dev;
+	struct acpi_hardware_id *id;
 
 	/*
 	 * If a PnPacpi device is not present , the device
@@ -193,15 +194,12 @@ static int __init pnpacpi_add_device(struct acpi_device *device)
 	if (dev->capabilities & PNP_CONFIGURABLE)
 		pnpacpi_parse_resource_option_data(dev);
 
-	if (device->flags.compatible_ids) {
-		struct acpi_compatible_id_list *cid_list = device->pnp.cid_list;
-		int i;
-
-		for (i = 0; i < cid_list->count; i++) {
-			if (!ispnpidacpi(cid_list->id[i].value))
-				continue;
-			pnp_add_id(dev, cid_list->id[i].value);
-		}
+	list_for_each_entry(id, &device->pnp.ids, list) {
+		if (!strcmp(id->id, acpi_device_hid(device)))
+			continue;
+		if (!ispnpidacpi(id->id))
+			continue;
+		pnp_add_id(dev, id->id);
 	}
 
 	/* clear out the damaged flags */
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 26be825..3b3fef5 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -170,16 +170,19 @@ struct acpi_device_dir {
 
 typedef char acpi_bus_id[8];
 typedef unsigned long acpi_bus_address;
-typedef char acpi_hardware_id[15];
 typedef char acpi_unique_id[9];
 typedef char acpi_device_name[40];
 typedef char acpi_device_class[20];
 
+struct acpi_hardware_id {
+	struct list_head list;
+	char *id;
+};
+
 struct acpi_device_pnp {
 	acpi_bus_id bus_id;	/* Object name */
 	acpi_bus_address bus_address;	/* _ADR */
-	acpi_hardware_id hardware_id;	/* _HID */
-	struct acpi_compatible_id_list *cid_list;	/* _CIDs */
+	struct list_head ids;		/* _HID and _CIDs */
 	acpi_unique_id unique_id;	/* _UID */
 	acpi_device_name device_name;	/* Driver-determined */
 	acpi_device_class device_class;	/*        "          */
@@ -187,7 +190,7 @@ struct acpi_device_pnp {
 
 #define acpi_device_bid(d)	((d)->pnp.bus_id)
 #define acpi_device_adr(d)	((d)->pnp.bus_address)
-#define acpi_device_hid(d)	((d)->pnp.hardware_id)
+char *acpi_device_hid(struct acpi_device *device);
 #define acpi_device_uid(d)	((d)->pnp.unique_id)
 #define acpi_device_name(d)	((d)->pnp.device_name)
 #define acpi_device_class(d)	((d)->pnp.device_class)

[PATCH 07/10] ACPI: remove acpi_device.flags.compatible_ids

[Bjorn Helgaas]

We now keep a single list of IDs that includes both the _HID and any
_CIDs.  We no longer need to keep track of whether the device has a _CID.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
 drivers/acpi/scan.c     |   15 ++++-----------
 include/acpi/acpi_bus.h |    3 +--
 2 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 76ba200..f389510 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -47,7 +47,7 @@ static int create_modalias(struct acpi_device *acpi_dev, char *modalias,
 	int count;
 	struct acpi_hardware_id *id;
 
-	if (!acpi_dev->flags.hardware_id && !acpi_dev->flags.compatible_ids)
+	if (!acpi_dev->flags.hardware_id)
 		return -ENODEV;
 
 	len = snprintf(modalias, size, "acpi:");
@@ -209,7 +209,7 @@ static int acpi_device_setup_files(struct acpi_device *dev)
 			goto end;
 	}
 
-	if (dev->flags.hardware_id || dev->flags.compatible_ids) {
+	if (dev->flags.hardware_id) {
 		result = device_create_file(&dev->dev, &dev_attr_modalias);
 		if (result)
 			goto end;
@@ -239,7 +239,7 @@ static void acpi_device_remove_files(struct acpi_device *dev)
 	if (ACPI_SUCCESS(status))
 		device_remove_file(&dev->dev, &dev_attr_eject);
 
-	if (dev->flags.hardware_id || dev->flags.compatible_ids)
+	if (dev->flags.hardware_id)
 		device_remove_file(&dev->dev, &dev_attr_modalias);
 
 	if (dev->flags.hardware_id)
@@ -875,11 +875,6 @@ static int acpi_bus_get_flags(struct acpi_device *device)
 	if (ACPI_SUCCESS(status))
 		device->flags.dynamic_status = 1;
 
-	/* Presence of _CID indicates 'compatible_ids' */
-	status = acpi_get_handle(device->handle, "_CID", &temp);
-	if (ACPI_SUCCESS(status))
-		device->flags.compatible_ids = 1;
-
 	/* Presence of _RMV indicates 'removable' */
 	status = acpi_get_handle(device->handle, "_RMV", &temp);
 	if (ACPI_SUCCESS(status))
@@ -1119,10 +1114,8 @@ static void acpi_device_set_id(struct acpi_device *device)
 	if (cid_list)
 		for (i = 0; i < cid_list->count; i++)
 			acpi_add_id(device, cid_list->id[i].value);
-	if (cid_add) {
+	if (cid_add)
 		acpi_add_id(device, cid_add);
-		device->flags.compatible_ids = 1;
-	}
 
 	kfree(buffer.pointer);
 }
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 3b3fef5..c7702a5 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -144,7 +144,6 @@ struct acpi_device_status {
 struct acpi_device_flags {
 	u32 dynamic_status:1;
 	u32 hardware_id:1;
-	u32 compatible_ids:1;
 	u32 bus_address:1;
 	u32 unique_id:1;
 	u32 removable:1;
@@ -155,7 +154,7 @@ struct acpi_device_flags {
 	u32 performance_manageable:1;
 	u32 wake_capable:1;	/* Wakeup(_PRW) supported? */
 	u32 force_power_state:1;
-	u32 reserved:19;
+	u32 reserved:20;
 };
 
 /* File System */

[PATCH 08/10] ACPI: remove acpi_device.flags.hardware_id

[Bjorn Helgaas]

Every acpi_device has at least one ID (if there's no _HID or _CID, we
give it a synthetic or default ID).  So there's no longer a need to
check whether an ID exists; we can just use it.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
 drivers/acpi/scan.c        |   37 +++++++++++++------------------------
 drivers/pnp/pnpacpi/core.c |    3 +--
 include/acpi/acpi_bus.h    |    3 +--
 3 files changed, 15 insertions(+), 28 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index f389510..c181728 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -47,9 +47,6 @@ static int create_modalias(struct acpi_device *acpi_dev, char *modalias,
 	int count;
 	struct acpi_hardware_id *id;
 
-	if (!acpi_dev->flags.hardware_id)
-		return -ENODEV;
-
 	len = snprintf(modalias, size, "acpi:");
 	size -= len;
 
@@ -203,17 +200,13 @@ static int acpi_device_setup_files(struct acpi_device *dev)
 			goto end;
 	}
 
-	if (dev->flags.hardware_id) {
-		result = device_create_file(&dev->dev, &dev_attr_hid);
-		if (result)
-			goto end;
-	}
+	result = device_create_file(&dev->dev, &dev_attr_hid);
+	if (result)
+		goto end;
 
-	if (dev->flags.hardware_id) {
-		result = device_create_file(&dev->dev, &dev_attr_modalias);
-		if (result)
-			goto end;
-	}
+	result = device_create_file(&dev->dev, &dev_attr_modalias);
+	if (result)
+		goto end;
 
         /*
          * If device has _EJ0, 'eject' file is created that is used to trigger
@@ -239,11 +232,8 @@ static void acpi_device_remove_files(struct acpi_device *dev)
 	if (ACPI_SUCCESS(status))
 		device_remove_file(&dev->dev, &dev_attr_eject);
 
-	if (dev->flags.hardware_id)
-		device_remove_file(&dev->dev, &dev_attr_modalias);
-
-	if (dev->flags.hardware_id)
-		device_remove_file(&dev->dev, &dev_attr_hid);
+	device_remove_file(&dev->dev, &dev_attr_modalias);
+	device_remove_file(&dev->dev, &dev_attr_hid);
 	if (dev->handle)
 		device_remove_file(&dev->dev, &dev_attr_path);
 }
@@ -474,8 +464,9 @@ static int acpi_device_register(struct acpi_device *device)
 	 * If failed, create one and link it into acpi_bus_id_list
 	 */
 	list_for_each_entry(acpi_device_bus_id, &acpi_bus_id_list, node) {
-		if (!strcmp(acpi_device_bus_id->bus_id, device->flags.hardware_id ? acpi_device_hid(device) : "device")) {
-			acpi_device_bus_id->instance_no ++;
+		if (!strcmp(acpi_device_bus_id->bus_id,
+			    acpi_device_hid(device))) {
+			acpi_device_bus_id->instance_no++;
 			found = 1;
 			kfree(new_bus_id);
 			break;
@@ -483,7 +474,7 @@ static int acpi_device_register(struct acpi_device *device)
 	}
 	if (!found) {
 		acpi_device_bus_id = new_bus_id;
-		strcpy(acpi_device_bus_id->bus_id, device->flags.hardware_id ? acpi_device_hid(device) : "device");
+		strcpy(acpi_device_bus_id->bus_id, acpi_device_hid(device));
 		acpi_device_bus_id->instance_no = 0;
 		list_add_tail(&acpi_device_bus_id->node, &acpi_bus_id_list);
 	}
@@ -1103,10 +1094,8 @@ static void acpi_device_set_id(struct acpi_device *device)
 	if (!hid && !cid_list && !cid_add)
 		hid = "device";
 
-	if (hid) {
+	if (hid)
 		acpi_add_id(device, hid);
-		device->flags.hardware_id = 1;
-	}
 	if (uid) {
 		strcpy(device->pnp.unique_id, uid);
 		device->flags.unique_id = 1;
diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
index 3a4478f..83b8b5a 100644
--- a/drivers/pnp/pnpacpi/core.c
+++ b/drivers/pnp/pnpacpi/core.c
@@ -230,8 +230,7 @@ static int __init acpi_pnp_match(struct device *dev, void *_pnp)
 	struct pnp_dev *pnp = _pnp;
 
 	/* true means it matched */
-	return acpi->flags.hardware_id
-	    && !acpi_get_physical_device(acpi->handle)
+	return !acpi_get_physical_device(acpi->handle)
 	    && compare_pnp_id(pnp->id, acpi_device_hid(acpi));
 }
 
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index c7702a5..7b353b7 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -143,7 +143,6 @@ struct acpi_device_status {
 
 struct acpi_device_flags {
 	u32 dynamic_status:1;
-	u32 hardware_id:1;
 	u32 bus_address:1;
 	u32 unique_id:1;
 	u32 removable:1;
@@ -154,7 +153,7 @@ struct acpi_device_flags {
 	u32 performance_manageable:1;
 	u32 wake_capable:1;	/* Wakeup(_PRW) supported? */
 	u32 force_power_state:1;
-	u32 reserved:20;
+	u32 reserved:21;
 };
 
 /* File System */

[PATCH 09/10] ACPI: remove acpi_device_uid() and related stuff

[Bjorn Helgaas]

Nobody uses acpi_device_uid(), so this patch removes it.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
 drivers/acpi/scan.c     |   12 ------------
 include/acpi/acpi_bus.h |    6 +-----
 2 files changed, 1 insertions(+), 17 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index c181728..a776018 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1017,7 +1017,6 @@ static void acpi_device_set_id(struct acpi_device *device)
 	struct acpi_device_info *info;
 	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
 	char *hid = NULL;
-	char *uid = NULL;
 	struct acpi_compatible_id_list *cid_list = NULL;
 	const char *cid_add = NULL;
 	acpi_status status;
@@ -1045,8 +1044,6 @@ static void acpi_device_set_id(struct acpi_device *device)
 		info = buffer.pointer;
 		if (info->valid & ACPI_VALID_HID)
 			hid = info->hardware_id.value;
-		if (info->valid & ACPI_VALID_UID)
-			uid = info->unique_id.value;
 		if (info->valid & ACPI_VALID_CID)
 			cid_list = &info->compatibility_id;
 		if (info->valid & ACPI_VALID_ADR) {
@@ -1096,10 +1093,6 @@ static void acpi_device_set_id(struct acpi_device *device)
 
 	if (hid)
 		acpi_add_id(device, hid);
-	if (uid) {
-		strcpy(device->pnp.unique_id, uid);
-		device->flags.unique_id = 1;
-	}
 	if (cid_list)
 		for (i = 0; i < cid_list->count; i++)
 			acpi_add_id(device, cid_list->id[i].value);
@@ -1194,11 +1187,6 @@ static int acpi_add_single_object(struct acpi_device **child,
 	 * -----------------
 	 * TBD: Synch with Core's enumeration/initialization process.
 	 */
-
-	/*
-	 * Hardware ID, Unique ID, & Bus Address
-	 * -------------------------------------
-	 */
 	acpi_device_set_id(device);
 
 	/*
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 7b353b7..d8b93a2 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -144,7 +144,6 @@ struct acpi_device_status {
 struct acpi_device_flags {
 	u32 dynamic_status:1;
 	u32 bus_address:1;
-	u32 unique_id:1;
 	u32 removable:1;
 	u32 ejectable:1;
 	u32 lockable:1;
@@ -153,7 +152,7 @@ struct acpi_device_flags {
 	u32 performance_manageable:1;
 	u32 wake_capable:1;	/* Wakeup(_PRW) supported? */
 	u32 force_power_state:1;
-	u32 reserved:21;
+	u32 reserved:22;
 };
 
 /* File System */
@@ -168,7 +167,6 @@ struct acpi_device_dir {
 
 typedef char acpi_bus_id[8];
 typedef unsigned long acpi_bus_address;
-typedef char acpi_unique_id[9];
 typedef char acpi_device_name[40];
 typedef char acpi_device_class[20];
 
@@ -181,7 +179,6 @@ struct acpi_device_pnp {
 	acpi_bus_id bus_id;	/* Object name */
 	acpi_bus_address bus_address;	/* _ADR */
 	struct list_head ids;		/* _HID and _CIDs */
-	acpi_unique_id unique_id;	/* _UID */
 	acpi_device_name device_name;	/* Driver-determined */
 	acpi_device_class device_class;	/*        "          */
 };
@@ -189,7 +186,6 @@ struct acpi_device_pnp {
 #define acpi_device_bid(d)	((d)->pnp.bus_id)
 #define acpi_device_adr(d)	((d)->pnp.bus_address)
 char *acpi_device_hid(struct acpi_device *device);
-#define acpi_device_uid(d)	((d)->pnp.unique_id)
 #define acpi_device_name(d)	((d)->pnp.device_name)
 #define acpi_device_class(d)	((d)->pnp.device_class)
 

[PATCH 10/10] ACPI: simplify building device HID/CID list

[Bjorn Helgaas]

Minor code cleanup, no functional change.  Instead of remembering
what HIDs & CIDs to add later, just add them immediately.

Signed-off-by: Bjorn Helgaas <bjorn.helgaas@hp.com>
---
 drivers/acpi/scan.c |   59 ++++++++++++++++++++++-----------------------------
 1 files changed, 25 insertions(+), 34 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index a776018..44c333f 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1014,22 +1014,20 @@ static void acpi_add_id(struct acpi_device *device, const char *dev_id)
 
 static void acpi_device_set_id(struct acpi_device *device)
 {
-	struct acpi_device_info *info;
-	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
-	char *hid = NULL;
-	struct acpi_compatible_id_list *cid_list = NULL;
-	const char *cid_add = NULL;
 	acpi_status status;
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	struct acpi_device_info *info;
+	struct acpi_compatible_id_list *cid_list;
 	int i;
 
 	switch (device->device_type) {
 	case ACPI_BUS_TYPE_DEVICE:
 		if (ACPI_IS_ROOT_DEVICE(device)) {
-			hid = ACPI_SYSTEM_HID;
+			acpi_add_id(device, ACPI_SYSTEM_HID);
 			break;
 		} else if (ACPI_IS_ROOT_DEVICE(device->parent)) {
 			/* \_SB_, the only root-level namespace device */
-			hid = ACPI_BUS_HID;
+			acpi_add_id(device, ACPI_BUS_HID);
 			strcpy(device->pnp.device_name, ACPI_BUS_DEVICE_NAME);
 			strcpy(device->pnp.device_class, ACPI_BUS_CLASS);
 			break;
@@ -1043,41 +1041,44 @@ static void acpi_device_set_id(struct acpi_device *device)
 
 		info = buffer.pointer;
 		if (info->valid & ACPI_VALID_HID)
-			hid = info->hardware_id.value;
-		if (info->valid & ACPI_VALID_CID)
+			acpi_add_id(device, info->hardware_id.value);
+		if (info->valid & ACPI_VALID_CID) {
 			cid_list = &info->compatibility_id;
+			for (i = 0; i < cid_list->count; i++)
+				acpi_add_id(device, cid_list->id[i].value);
+		}
 		if (info->valid & ACPI_VALID_ADR) {
 			device->pnp.bus_address = info->address;
 			device->flags.bus_address = 1;
 		}
+		kfree(buffer.pointer);
 
-		/* If we have a video/bay/dock device, add our selfdefined
-		   HID to the CID list. Like that the video/bay/dock drivers
-		   will get autoloaded and the device might still match
-		   against another driver.
-		*/
+		/*
+		 * Some devices don't reliably have _HIDs & _CIDs, so add
+		 * synthetic HIDs to make sure drivers can find them.
+		 */
 		if (acpi_is_video_device(device))
-			cid_add = ACPI_VIDEO_HID;
+			acpi_add_id(device, ACPI_VIDEO_HID);
 		else if (ACPI_SUCCESS(acpi_bay_match(device)))
-			cid_add = ACPI_BAY_HID;
+			acpi_add_id(device, ACPI_BAY_HID);
 		else if (ACPI_SUCCESS(acpi_dock_match(device)))
-			cid_add = ACPI_DOCK_HID;
+			acpi_add_id(device, ACPI_DOCK_HID);
 
 		break;
 	case ACPI_BUS_TYPE_POWER:
-		hid = ACPI_POWER_HID;
+		acpi_add_id(device, ACPI_POWER_HID);
 		break;
 	case ACPI_BUS_TYPE_PROCESSOR:
-		hid = ACPI_PROCESSOR_OBJECT_HID;
+		acpi_add_id(device, ACPI_PROCESSOR_OBJECT_HID);
 		break;
 	case ACPI_BUS_TYPE_THERMAL:
-		hid = ACPI_THERMAL_HID;
+		acpi_add_id(device, ACPI_THERMAL_HID);
 		break;
 	case ACPI_BUS_TYPE_POWER_BUTTON:
-		hid = ACPI_BUTTON_HID_POWERF;
+		acpi_add_id(device, ACPI_BUTTON_HID_POWERF);
 		break;
 	case ACPI_BUS_TYPE_SLEEP_BUTTON:
-		hid = ACPI_BUTTON_HID_SLEEPF;
+		acpi_add_id(device, ACPI_BUTTON_HID_SLEEPF);
 		break;
 	}
 
@@ -1088,18 +1089,8 @@ static void acpi_device_set_id(struct acpi_device *device)
 	 * This generic ID isn't useful for driver binding, but it provides
 	 * the useful property that "every acpi_device has an ID."
 	 */
-	if (!hid && !cid_list && !cid_add)
-		hid = "device";
-
-	if (hid)
-		acpi_add_id(device, hid);
-	if (cid_list)
-		for (i = 0; i < cid_list->count; i++)
-			acpi_add_id(device, cid_list->id[i].value);
-	if (cid_add)
-		acpi_add_id(device, cid_add);
-
-	kfree(buffer.pointer);
+	if (list_empty(&device->pnp.ids))
+		acpi_add_id(device, "device");
 }
 
 static int acpi_device_set_context(struct acpi_device *device)