[PATCH v3 0/8] ACPI: make every acpi_device have a HID

[PATCH v3 0/8] ACPI: 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.

Original posting: http://marc.info/?l=linux-acpi&m=125113191017124&w=2

Changes from v2 to v3:
    - Refreshed to apply on current acpi-test (193a6dec1c0246a).

Changes from initial post to v2:
    - Applies on acpi-test (on top of "ACPI: cleanups for hotplug",
      http://marc.info/?l=linux-acpi&m=125175792329288&w=2).
    - Removed memory leak fix (folded into v2 of series that introduced it).
    - 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 (8):
      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        |  283 +++++++++++++-------------------------------
 drivers/pnp/pnpacpi/core.c |   21 +--
 include/acpi/acpi_bus.h    |   16 +-
 3 files changed, 100 insertions(+), 220 deletions(-)

-- 
Bjorn

[PATCH v3 1/8] 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 2c4cac5..e9227ea 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1100,6 +1100,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, &info);
@@ -1149,18 +1155,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) {
 		device->pnp.hardware_id = ACPI_ALLOCATE_ZEROED(strlen (hid) + 1);
 		if (device->pnp.hardware_id) {

[PATCH v3 2/8] 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 e9227ea..269c0aa 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);
 
@@ -501,7 +501,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);
@@ -510,7 +510,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 c07fdb9..ff963d4 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 v3 3/8] 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 269c0aa..53b96e7 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1155,6 +1155,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) {
 		device->pnp.hardware_id = ACPI_ALLOCATE_ZEROED(strlen (hid) + 1);
 		if (device->pnp.hardware_id) {

[PATCH v3 4/8] 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        |  165 ++++++++++++--------------------------------
 drivers/pnp/pnpacpi/core.c |   16 ++--
 include/acpi/acpi_bus.h    |   10 ++-
 3 files changed, 59 insertions(+), 132 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 53b96e7..9c65244 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 acpica_device_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->ids[i].string);
-			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,40 +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 acpica_device_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->ids[i].string))
-					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);
-	if (acpi_dev->flags.hardware_id)
-		kfree(acpi_dev->pnp.hardware_id);
-	if (acpi_dev->flags.unique_id)
-		kfree(acpi_dev->pnp.unique_id);
+	acpi_free_ids(acpi_dev);
 	kfree(acpi_dev);
 }
 
@@ -1028,62 +1001,30 @@ static int acpi_dock_match(struct acpi_device *device)
 	return acpi_get_handle(device->handle, "_DCK", &tmp);
 }
 
-static struct acpica_device_id_list*
-acpi_add_cid(
-	struct acpi_device_info         *info,
-	struct acpica_device_id         *new_cid)
+char *acpi_device_hid(struct acpi_device *device)
 {
-	struct acpica_device_id_list    *cid;
-	char                            *next_id_string;
-	acpi_size                       cid_length;
-	acpi_size                       new_cid_length;
-	u32                             i;
-
-
-	/* Allocate new CID list with room for the new CID */
-
-	if (!new_cid)
-		new_cid_length = info->compatible_id_list.list_size;
-	else if (info->compatible_id_list.list_size)
-		new_cid_length = info->compatible_id_list.list_size +
-			new_cid->length + sizeof(struct acpica_device_id);
-	else
-		new_cid_length = sizeof(struct acpica_device_id_list) + new_cid->length;
-
-	cid = ACPI_ALLOCATE_ZEROED(new_cid_length);
-	if (!cid) {
-		return NULL;
-	}
-
-	cid->list_size = new_cid_length;
-	cid->count = info->compatible_id_list.count;
-	if (new_cid)
-		cid->count++;
-	next_id_string = (char *) cid->ids + (cid->count * sizeof(struct acpica_device_id));
-
-	/* Copy all existing CIDs */
+	struct acpi_hardware_id *hid;
 
-	for (i = 0; i < info->compatible_id_list.count; i++) {
-		cid_length = info->compatible_id_list.ids[i].length;
-		cid->ids[i].string = next_id_string;
-		cid->ids[i].length = cid_length;
-
-		ACPI_MEMCPY(next_id_string, info->compatible_id_list.ids[i].string,
-			cid_length);
-
-		next_id_string += cid_length;
-	}
+	hid = list_first_entry(&device->pnp.ids, struct acpi_hardware_id, list);
+	return hid->id;
+}
 
-	/* Append the new CID */
+static void acpi_add_id(struct acpi_device *device, const char *dev_id)
+{
+	struct acpi_hardware_id *id;
 
-	if (new_cid) {
-		cid->ids[i].string = next_id_string;
-		cid->ids[i].length = new_cid->length;
+	id = kmalloc(sizeof(*id), GFP_KERNEL);
+	if (!id)
+		return;
 
-		ACPI_MEMCPY(next_id_string, new_cid->string, new_cid->length);
+	id->id = kmalloc(strlen(dev_id) + 1, GFP_KERNEL);
+	if (!id->id) {
+		kfree(id);
+		return;
 	}
 
-	return cid;
+	strcpy(id->id, dev_id);
+	list_add_tail(&id->list, &device->pnp.ids);
 }
 
 static void acpi_device_set_id(struct acpi_device *device)
@@ -1094,6 +1035,7 @@ static void acpi_device_set_id(struct acpi_device *device)
 	struct acpica_device_id_list *cid_list = NULL;
 	char *cid_add = NULL;
 	acpi_status status;
+	int i;
 
 	switch (device->device_type) {
 	case ACPI_BUS_TYPE_DEVICE:
@@ -1166,15 +1108,9 @@ static void acpi_device_set_id(struct acpi_device *device)
 		hid = "device";
 
 	if (hid) {
-		device->pnp.hardware_id = ACPI_ALLOCATE_ZEROED(strlen (hid) + 1);
-		if (device->pnp.hardware_id) {
-			strcpy(device->pnp.hardware_id, hid);
-			device->flags.hardware_id = 1;
-		}
+		acpi_add_id(device, hid);
+		device->flags.hardware_id = 1;
 	}
-	if (!device->flags.hardware_id)
-		device->pnp.hardware_id = "";
-
 	if (uid) {
 		device->pnp.unique_id = ACPI_ALLOCATE_ZEROED(strlen (uid) + 1);
 		if (device->pnp.unique_id) {
@@ -1185,24 +1121,12 @@ static void acpi_device_set_id(struct acpi_device *device)
 	if (!device->flags.unique_id)
 		device->pnp.unique_id = "";
 
-	if (cid_list || cid_add) {
-		struct acpica_device_id_list *list;
-
-		if (cid_add) {
-			struct acpica_device_id cid;
-			cid.length = strlen (cid_add) + 1;
-			cid.string = cid_add;
-
-			list = acpi_add_cid(info, &cid);
-		} else {
-			list = acpi_add_cid(info, NULL);
-		}
-
-		if (list) {
-			device->pnp.cid_list = list;
-			if (cid_add)
-				device->flags.compatible_ids = 1;
-		}
+	if (cid_list)
+		for (i = 0; i < cid_list->count; i++)
+			acpi_add_id(device, cid_list->ids[i].string);
+	if (cid_add) {
+		acpi_add_id(device, cid_add);
+		device->flags.compatible_ids = 1;
 	}
 
 	kfree(info);
@@ -1269,6 +1193,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);
diff --git a/drivers/pnp/pnpacpi/core.c b/drivers/pnp/pnpacpi/core.c
index ff963d4..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 acpica_device_id_list *cid_list = device->pnp.cid_list;
-		int i;
-
-		for (i = 0; i < cid_list->count; i++) {
-			if (!ispnpidacpi(cid_list->ids[i].string))
-				continue;
-			pnp_add_id(dev, cid_list->ids[i].string);
-		}
+	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 13a63a6..635e305 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -171,19 +171,23 @@ typedef unsigned long acpi_bus_address;
 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 */
-	char *hardware_id;	/* _HID */
-	struct acpica_device_id_list *cid_list;	/* _CIDs */
 	char *unique_id;	/* _UID */
+	struct list_head ids;		/* _HID and _CIDs */
 	acpi_device_name device_name;	/* Driver-determined */
 	acpi_device_class device_class;	/*        "          */
 };
 
 #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 v3 5/8] 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 9c65244..954cb1d 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)
@@ -876,11 +876,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))
@@ -1124,10 +1119,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->ids[i].string);
-	if (cid_add) {
+	if (cid_add)
 		acpi_add_id(device, cid_add);
-		device->flags.compatible_ids = 1;
-	}
 
 	kfree(info);
 }
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 635e305..e422547 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -142,7 +142,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;
@@ -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:19;
+	u32 reserved:20;
 };
 
 /* File System */

[PATCH v3 6/8] 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 954cb1d..3f9b8d0 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);
 	}
@@ -1102,10 +1093,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) {
 		device->pnp.unique_id = ACPI_ALLOCATE_ZEROED(strlen (uid) + 1);
 		if (device->pnp.unique_id) {
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 e422547..1d205f5 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -141,7 +141,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;
@@ -152,7 +151,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 v3 7/8] 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     |   18 ------------------
 include/acpi/acpi_bus.h |    4 +---
 2 files changed, 1 insertions(+), 21 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 3f9b8d0..6880852 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 = NULL;
 	char *hid = NULL;
-	char *uid = NULL;
 	struct acpica_device_id_list *cid_list = NULL;
 	char *cid_add = NULL;
 	acpi_status status;
@@ -1044,8 +1043,6 @@ static void acpi_device_set_id(struct acpi_device *device)
 
 		if (info->valid & ACPI_VALID_HID)
 			hid = info->hardware_id.string;
-		if (info->valid & ACPI_VALID_UID)
-			uid = info->unique_id.string;
 		if (info->valid & ACPI_VALID_CID)
 			cid_list = &info->compatible_id_list;
 		if (info->valid & ACPI_VALID_ADR) {
@@ -1095,16 +1092,6 @@ static void acpi_device_set_id(struct acpi_device *device)
 
 	if (hid)
 		acpi_add_id(device, hid);
-	if (uid) {
-		device->pnp.unique_id = ACPI_ALLOCATE_ZEROED(strlen (uid) + 1);
-		if (device->pnp.unique_id) {
-			strcpy(device->pnp.unique_id, uid);
-			device->flags.unique_id = 1;
-		}
-	}
-	if (!device->flags.unique_id)
-		device->pnp.unique_id = "";
-
 	if (cid_list)
 		for (i = 0; i < cid_list->count; i++)
 			acpi_add_id(device, cid_list->ids[i].string);
@@ -1199,11 +1186,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 1d205f5..b8f195d 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -142,7 +142,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;
@@ -151,7 +150,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 */
@@ -186,7 +185,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 v3 8/8] 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 |   56 +++++++++++++++++++++------------------------------
 1 files changed, 23 insertions(+), 33 deletions(-)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 6880852..1ef5b7e 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -1015,21 +1015,19 @@ 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 = NULL;
-	char *hid = NULL;
-	struct acpica_device_id_list *cid_list = NULL;
-	char *cid_add = NULL;
 	acpi_status status;
+	struct acpi_device_info *info;
+	struct acpica_device_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;
@@ -1042,41 +1040,43 @@ static void acpi_device_set_id(struct acpi_device *device)
 		}
 
 		if (info->valid & ACPI_VALID_HID)
-			hid = info->hardware_id.string;
-		if (info->valid & ACPI_VALID_CID)
+			acpi_add_id(device, info->hardware_id.string);
+		if (info->valid & ACPI_VALID_CID) {
 			cid_list = &info->compatible_id_list;
+			for (i = 0; i < cid_list->count; i++)
+				acpi_add_id(device, cid_list->ids[i].string);
+		}
 		if (info->valid & ACPI_VALID_ADR) {
 			device->pnp.bus_address = info->address;
 			device->flags.bus_address = 1;
 		}
 
-		/* 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;
 	}
 
@@ -1087,18 +1087,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->ids[i].string);
-	if (cid_add)
-		acpi_add_id(device, cid_add);
-
-	kfree(info);
+	if (list_empty(&device->pnp.ids))
+		acpi_add_id(device, "device");
 }
 
 static int acpi_device_set_context(struct acpi_device *device)

Re: [PATCH v3 1/8] ACPI: fix synthetic HID for \_SB_

[ykzhao]

On Tue, 2009-09-22 at 03:35 +0800, Bjorn Helgaas wrote:
> 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 2c4cac5..e9227ea 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1100,6 +1100,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)) {
Can we still add the judgement about the device type?
   device->type == ACPI_BUS_TYPE_DEVICE

Where can I find the macro definition of ACPI_IS_ROOT_DEVICE?
Thanks.
> +			/* \_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, &info);
> @@ -1149,18 +1155,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) {
>  		device->pnp.hardware_id = ACPI_ALLOCATE_ZEROED(strlen (hid) + 1);
>  		if (device->pnp.hardware_id) {
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/8] ACPI: fix synthetic HID for \_SB_

[Bjorn Helgaas]

On Tuesday 22 September 2009 08:21:46 pm ykzhao wrote:
> On Tue, 2009-09-22 at 03:35 +0800, Bjorn Helgaas wrote:
> > 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 2c4cac5..e9227ea 100644
> > --- a/drivers/acpi/scan.c
> > +++ b/drivers/acpi/scan.c
> > @@ -1100,6 +1100,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)) {

> Can we still add the judgement about the device type?
>    device->type == ACPI_BUS_TYPE_DEVICE

We are already checking this because this test is inside the
ACPI_BUS_TYPE_DEVICE case of a switch statement.

> Where can I find the macro definition of ACPI_IS_ROOT_DEVICE?

It's patch 13/17 in the previous "ACPI: cleanups for hotplug" series.
I should have mentioned that this series depends on that one.

Bjorn

> > +			/* \_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, &info);
> > @@ -1149,18 +1155,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) {
> >  		device->pnp.hardware_id = ACPI_ALLOCATE_ZEROED(strlen (hid) + 1);
> >  		if (device->pnp.hardware_id) {
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

Re: [PATCH v3 1/8] ACPI: fix synthetic HID for \_SB_

[ykzhao]

On Thu, 2009-09-24 at 00:19 +0800, Bjorn Helgaas wrote:
> On Tuesday 22 September 2009 08:21:46 pm ykzhao wrote:
> > On Tue, 2009-09-22 at 03:35 +0800, Bjorn Helgaas wrote:
> > > 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 2c4cac5..e9227ea 100644
> > > --- a/drivers/acpi/scan.c
> > > +++ b/drivers/acpi/scan.c
> > > @@ -1100,6 +1100,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)) {
> 
> > Can we still add the judgement about the device type?
> >    device->type == ACPI_BUS_TYPE_DEVICE
> 
> We are already checking this because this test is inside the
> ACPI_BUS_TYPE_DEVICE case of a switch statement.
If this is checed inside the ACPI_BUS_TYPE_DEVICE case, can we delete
the following check as the type of root device is ACPI_BUS_TYPE_SYSTEM?
   If (ACPI_IS_ROOT_DEVICE(device)) {
		hid = ACPI_SYSTEM_HID;
		break;
	}
  
> 
> > Where can I find the macro definition of ACPI_IS_ROOT_DEVICE?
> 
> It's patch 13/17 in the previous "ACPI: cleanups for hotplug" series.
> I should have mentioned that this series depends on that one.
Yes. 
I also find that it is defined in patch 13/17.
It had better be defined as early as possible. Otherwise we may fail in
git-bisect.
thanks.
> 
> Bjorn
> 
> > > +			/* \_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, &info);
> > > @@ -1149,18 +1155,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) {
> > >  		device->pnp.hardware_id = ACPI_ALLOCATE_ZEROED(strlen (hid) + 1);
> > >  		if (device->pnp.hardware_id) {
> > > 
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > 
> > 
> 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/8] ACPI: fix synthetic HID for \_SB_

[Bjorn Helgaas]

On Thu, 2009-09-24 at 09:44 +0800, ykzhao wrote:
> On Thu, 2009-09-24 at 00:19 +0800, Bjorn Helgaas wrote:
> > On Tuesday 22 September 2009 08:21:46 pm ykzhao wrote:
> > > On Tue, 2009-09-22 at 03:35 +0800, Bjorn Helgaas wrote:
> > > > 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 2c4cac5..e9227ea 100644
> > > > --- a/drivers/acpi/scan.c
> > > > +++ b/drivers/acpi/scan.c
> > > > @@ -1100,6 +1100,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)) {
> > 
> > > Can we still add the judgement about the device type?
> > >    device->type == ACPI_BUS_TYPE_DEVICE
> > 
> > We are already checking this because this test is inside the
> > ACPI_BUS_TYPE_DEVICE case of a switch statement.
> If this is checed inside the ACPI_BUS_TYPE_DEVICE case, can we delete
> the following check as the type of root device is ACPI_BUS_TYPE_SYSTEM?
>    If (ACPI_IS_ROOT_DEVICE(device)) {
> 		hid = ACPI_SYSTEM_HID;
> 		break;
> 	}

What?  Are you saying that we shouldn't add a HID of "LNXSYSTM" to the
root object?  I'm really not sure what you're proposing.

> > 
> > > Where can I find the macro definition of ACPI_IS_ROOT_DEVICE?
> > 
> > It's patch 13/17 in the previous "ACPI: cleanups for hotplug" series.
> > I should have mentioned that this series depends on that one.
> Yes. 
> I also find that it is defined in patch 13/17.
> It had better be defined as early as possible. Otherwise we may fail in
> git-bisect.

Absolutely.  I'm pretty sure I defined ACPI_IS_ROOT_DEVICE before any
uses of it, but please point it out if I'm mistaken.

Bjorn

> > > > +			/* \_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, &info);
> > > > @@ -1149,18 +1155,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) {
> > > >  		device->pnp.hardware_id = ACPI_ALLOCATE_ZEROED(strlen (hid) + 1);
> > > >  		if (device->pnp.hardware_id) {
> > > > 
> > > > --
> > > > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > > > the body of a message to majordomo@vger.kernel.org
> > > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > 
> > > 
> > 
> > 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/8] ACPI: fix synthetic HID for \_SB_

[ykzhao]

On Thu, 2009-09-24 at 11:36 +0800, Bjorn Helgaas wrote:
> On Thu, 2009-09-24 at 09:44 +0800, ykzhao wrote:
> > On Thu, 2009-09-24 at 00:19 +0800, Bjorn Helgaas wrote:
> > > On Tuesday 22 September 2009 08:21:46 pm ykzhao wrote:
> > > > On Tue, 2009-09-22 at 03:35 +0800, Bjorn Helgaas wrote:
> > > > > 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 2c4cac5..e9227ea 100644
> > > > > --- a/drivers/acpi/scan.c
> > > > > +++ b/drivers/acpi/scan.c
> > > > > @@ -1100,6 +1100,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)) {
> > > 
> > > > Can we still add the judgement about the device type?
> > > >    device->type == ACPI_BUS_TYPE_DEVICE
> > > 
> > > We are already checking this because this test is inside the
> > > ACPI_BUS_TYPE_DEVICE case of a switch statement.
> > If this is checed inside the ACPI_BUS_TYPE_DEVICE case, can we delete
> > the following check as the type of root device is ACPI_BUS_TYPE_SYSTEM?
> >    If (ACPI_IS_ROOT_DEVICE(device)) {
> > 		hid = ACPI_SYSTEM_HID;
> > 		break;
> > 	}
> 
> What?  Are you saying that we shouldn't add a HID of "LNXSYSTM" to the
> root object?  I'm really not sure what you're proposing.
Maybe I mix the two patch sets. One has 8 patches. And another has 17
patches. In fact there exists the dependency between the two patch set.

And you delete the ACPI_BUS_TYPE_SYSTEM type in another patch set,
right?

What is the benefit if we change the type from ACPI_BUS_TYPE_SYSTEM to
ACPI_BUS_TYPE_DEVICE for the root device?

Thanks.
> 
> > > 
> > > > Where can I find the macro definition of ACPI_IS_ROOT_DEVICE?
> > > 
> > > It's patch 13/17 in the previous "ACPI: cleanups for hotplug" series.
> > > I should have mentioned that this series depends on that one.
> > Yes. 
> > I also find that it is defined in patch 13/17.
> > It had better be defined as early as possible. Otherwise we may fail in
> > git-bisect.
> 
> Absolutely.  I'm pretty sure I defined ACPI_IS_ROOT_DEVICE before any
> uses of it, but please point it out if I'm mistaken.
> 
> Bjorn
> 
> > > > > +			/* \_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, &info);
> > > > > @@ -1149,18 +1155,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) {
> > > > >  		device->pnp.hardware_id = ACPI_ALLOCATE_ZEROED(strlen (hid) + 1);
> > > > >  		if (device->pnp.hardware_id) {
> > > > > 
> > > > > --
> > > > > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > > > > the body of a message to majordomo@vger.kernel.org
> > > > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > > 
> > > > 
> > > 
> > > 
> > 
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v3 1/8] ACPI: fix synthetic HID for \_SB_

[Bjorn Helgaas]

On Wednesday 23 September 2009 11:22:28 pm ykzhao wrote:
> On Thu, 2009-09-24 at 11:36 +0800, Bjorn Helgaas wrote:
> > On Thu, 2009-09-24 at 09:44 +0800, ykzhao wrote:
> > > On Thu, 2009-09-24 at 00:19 +0800, Bjorn Helgaas wrote:
> > > > On Tuesday 22 September 2009 08:21:46 pm ykzhao wrote:
> > > > > On Tue, 2009-09-22 at 03:35 +0800, Bjorn Helgaas wrote:
> > > > > > 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 2c4cac5..e9227ea 100644
> > > > > > --- a/drivers/acpi/scan.c
> > > > > > +++ b/drivers/acpi/scan.c
> > > > > > @@ -1100,6 +1100,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)) {
> > > > 
> > > > > Can we still add the judgement about the device type?
> > > > >    device->type == ACPI_BUS_TYPE_DEVICE
> > > > 
> > > > We are already checking this because this test is inside the
> > > > ACPI_BUS_TYPE_DEVICE case of a switch statement.
> > > If this is checed inside the ACPI_BUS_TYPE_DEVICE case, can we delete
> > > the following check as the type of root device is ACPI_BUS_TYPE_SYSTEM?
> > >    If (ACPI_IS_ROOT_DEVICE(device)) {
> > > 		hid = ACPI_SYSTEM_HID;
> > > 		break;
> > > 	}
> > 
> > What?  Are you saying that we shouldn't add a HID of "LNXSYSTM" to the
> > root object?  I'm really not sure what you're proposing.
> Maybe I mix the two patch sets. One has 8 patches. And another has 17
> patches. In fact there exists the dependency between the two patch set.

Yes, the second series ("make every acpi_device have a HID") does
depend on the first series ("cleanups for hotplug").  I forgot to
mention that in the initial posting, sorry.

> And you delete the ACPI_BUS_TYPE_SYSTEM type in another patch set,
> right?

Yes, I removed ACPI_BUS_TYPE_SYSTEM in patch [13/17] of the "ACPI:
cleanups for hotplug" series.

> What is the benefit if we change the type from ACPI_BUS_TYPE_SYSTEM to
> ACPI_BUS_TYPE_DEVICE for the root device?

The benefit is removing special cases from the code.  If we don't
need to keep track of the difference between ACPI_BUS_TYPE_SYSTEM
and ACPI_BUS_TYPE_DEVICE, we shouldn't do it.

Bjorn

> Thanks.
> > 
> > > > 
> > > > > Where can I find the macro definition of ACPI_IS_ROOT_DEVICE?
> > > > 
> > > > It's patch 13/17 in the previous "ACPI: cleanups for hotplug" series.
> > > > I should have mentioned that this series depends on that one.
> > > Yes. 
> > > I also find that it is defined in patch 13/17.
> > > It had better be defined as early as possible. Otherwise we may fail in
> > > git-bisect.
> > 
> > Absolutely.  I'm pretty sure I defined ACPI_IS_ROOT_DEVICE before any
> > uses of it, but please point it out if I'm mistaken.
> > 
> > Bjorn
> > 
> > > > > > +			/* \_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, &info);
> > > > > > @@ -1149,18 +1155,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) {
> > > > > >  		device->pnp.hardware_id = ACPI_ALLOCATE_ZEROED(strlen (hid) + 1);
> > > > > >  		if (device->pnp.hardware_id) {
> > > > > > 
> > > > > > --
> > > > > > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > > > > > the body of a message to majordomo@vger.kernel.org
> > > > > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> > > > > 
> > > > > 
> > > > 
> > > > 
> > > 
> > 
> 
> 


--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html