public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers
@ 2013-06-12 23:23 Rafael J. Wysocki
  2013-06-12 23:23 ` [PATCH 1/5] ACPI / scan: Do not bind ACPI drivers to objects with " Rafael J. Wysocki
                   ` (5 more replies)
  0 siblings, 6 replies; 19+ messages in thread
From: Rafael J. Wysocki @ 2013-06-12 23:23 UTC (permalink / raw)
  To: ACPI Devel Maling List; +Cc: LKML, Luck, Tony, Toshi Kani, Aaron Lu

Hi All,

It turns out that some BIOSes add container device IDs as _CIDs to device
object that in principle may be matched against the other scan handlers (or
ACPI drivers, but that's not a problem, because the container scan handler
can co-exist with an ACPI driver).  That's why our recent fix for an issue
related to the ACPI video driver had to be reverted right before -rc5.

Although I submitted an alternative fix for that bug, I think the problem
with the container scan handler possibly matching devices already having
some other scan handlers attached needs addressing, because we may need to
use the container hotplug profile for those devices.  The following patch
series is supposed to address it.

[1/5] ACPI / scan: Do not bind ACPI drivers to objects with scan handlers
   (this version shouldn't break the Tony's IA64 HP box the previous one broke)
[2/5] ACPI / scan: Separate hotplug profiles from scan handlers
[3/5] ACPI / scan: Add hotplug profile pointer to struct acpi_device
[4/5] ACPI / scan: Use container hotplug profile for matching device objects
[5/5] ACPI / ia64 / sba_iommu: Use ACPI scan handler for discovery

Patches [1-4/5] were run on my Toshiba test box and didn't break it, but it
really doesn't do any ACPI hotplug notifications.

Patch [5/5] is kind of additional, but it wouldn't work correctly without the
previous ones (to be honest, I haven't tried to compile it yet, but here it
goes for completness).

The patches are against the linux-next branch of the linux-pm.git tree.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/5] ACPI / scan: Do not bind ACPI drivers to objects with scan handlers
  2013-06-12 23:23 [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers Rafael J. Wysocki
@ 2013-06-12 23:23 ` Rafael J. Wysocki
  2013-06-12 23:24 ` [PATCH 2/5] ACPI / scan: Separate hotplug profiles from " Rafael J. Wysocki
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Rafael J. Wysocki @ 2013-06-12 23:23 UTC (permalink / raw)
  To: ACPI Devel Maling List; +Cc: LKML, Luck, Tony, Toshi Kani, Aaron Lu

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

ACPI drivers generally should not be bound to device objects having
scan handlers attatched to them.

However, there is one exception which is the container scan handler.
Namely, it turns out that some BIOSes (on ia64 HP rx2600 in
particular) put container device IDs into the same objects along with
some other more specific ones which makes those device objects match
the container scan handler as well as some other scan handlers or
ACPI drivers.  Fortunately, though, the container scan handler is
only needed for hotplug to work and it doesn't actually do anything
to the device objects in question, so it is really OK to have both
the container scan handler and something else attached to the same
device object at the same time.

Following that, make acpi_device_probe() fail with -EINVAL if the
device object being probed has an ACPI scan handler that is not the
container scan handler attached to it.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/container.c |    5 +++--
 drivers/acpi/internal.h  |   10 ++++++++++
 drivers/acpi/scan.c      |    4 ++++
 drivers/acpi/video.c     |    3 ---
 4 files changed, 17 insertions(+), 5 deletions(-)

Index: linux-pm/drivers/acpi/container.c
===================================================================
--- linux-pm.orig/drivers/acpi/container.c
+++ linux-pm/drivers/acpi/container.c
@@ -51,7 +51,7 @@ static int container_device_attach(struc
 	return 1;
 }
 
-static struct acpi_scan_handler container_handler = {
+struct acpi_scan_handler container_scan_handler = {
 	.ids = container_device_ids,
 	.attach = container_device_attach,
 	.hotplug = {
@@ -62,5 +62,6 @@ static struct acpi_scan_handler containe
 
 void __init acpi_container_init(void)
 {
-	acpi_scan_add_handler_with_hotplug(&container_handler, "container");
+	acpi_scan_add_handler_with_hotplug(&container_scan_handler,
+					   "container");
 }
Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -37,8 +37,18 @@ void acpi_processor_init(void);
 void acpi_platform_init(void);
 int acpi_sysfs_init(void);
 #ifdef CONFIG_ACPI_CONTAINER
+extern struct acpi_scan_handler container_scan_handler;
+
+static inline struct acpi_scan_handler *acpi_container_scan_handler(void)
+{
+	return &container_scan_handler;
+}
 void acpi_container_init(void);
 #else
+static inline struct acpi_scan_handler *acpi_container_scan_handler(void)
+{
+	return NULL;
+}
 static inline void acpi_container_init(void) {}
 #endif
 #ifdef CONFIG_ACPI_HOTPLUG_MEMORY
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -939,6 +939,10 @@ static int acpi_device_probe(struct devi
 	struct acpi_driver *acpi_drv = to_acpi_driver(dev->driver);
 	int ret;
 
+	if (acpi_dev->handler
+	    && acpi_dev->handler != acpi_container_scan_handler())
+		return -EINVAL;
+
 	if (!acpi_drv->ops.add)
 		return -ENOSYS;
 
Index: linux-pm/drivers/acpi/video.c
===================================================================
--- linux-pm.orig/drivers/acpi/video.c
+++ linux-pm/drivers/acpi/video.c
@@ -1722,9 +1722,6 @@ static int acpi_video_bus_add(struct acp
 	int error;
 	acpi_status status;
 
-	if (device->handler)
-		return -EINVAL;
-
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE,
 				device->parent->handle, 1,
 				acpi_video_bus_match, NULL,

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 2/5] ACPI / scan: Separate hotplug profiles from scan handlers
  2013-06-12 23:23 [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers Rafael J. Wysocki
  2013-06-12 23:23 ` [PATCH 1/5] ACPI / scan: Do not bind ACPI drivers to objects with " Rafael J. Wysocki
@ 2013-06-12 23:24 ` Rafael J. Wysocki
  2013-06-12 23:25 ` [PATCH 3/5] ACPI / scan: Add hotplug profile pointer to struct acpi_device Rafael J. Wysocki
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Rafael J. Wysocki @ 2013-06-12 23:24 UTC (permalink / raw)
  To: ACPI Devel Maling List; +Cc: LKML, Luck, Tony, Toshi Kani, Aaron Lu

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Commit a33ec39 (ACPI / scan: Introduce common code for ACPI-based
device hotplug) decided to embed struct acpi_hotplug_profile into
struct acpi_scan_handler, because it looked like having a hotplug
profile without a scan handler wouldn't make sense.

While that still appears to be the case, it actually does make sense
to combine the container hotplug profile (that kind of plays the role
of a generic hotplug profile) with multiple different scan handlers.

To allow that to be done, replace the hotplug field of struct
acpi_scan_handler with a pointer and modify the code using that
field accordingly.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpi_memhotplug.c |   11 +++++++----
 drivers/acpi/acpi_processor.c  |   13 +++++++++----
 drivers/acpi/container.c       |   14 ++++++++------
 drivers/acpi/internal.h        |    5 +----
 drivers/acpi/scan.c            |   29 ++++++++++-------------------
 drivers/acpi/sysfs.c           |   10 ++++++----
 include/acpi/acpi_bus.h        |    3 ++-
 7 files changed, 43 insertions(+), 42 deletions(-)

Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -64,10 +64,7 @@ static inline void acpi_cmos_rtc_init(vo
 
 extern bool acpi_force_hot_remove;
 
-void acpi_sysfs_add_hotplug_profile(struct acpi_hotplug_profile *hotplug,
-				    const char *name);
-int acpi_scan_add_handler_with_hotplug(struct acpi_scan_handler *handler,
-				       const char *hotplug_profile_name);
+void acpi_sysfs_add_hotplug_profile(struct acpi_hotplug_profile *hotplug);
 void acpi_scan_hotplug_enabled(struct acpi_hotplug_profile *hotplug, bool val);
 
 #ifdef CONFIG_DEBUG_FS
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -66,19 +66,9 @@ int acpi_scan_add_handler(struct acpi_sc
 		return -EINVAL;
 
 	list_add_tail(&handler->list_node, &acpi_scan_handlers_list);
-	return 0;
-}
-
-int acpi_scan_add_handler_with_hotplug(struct acpi_scan_handler *handler,
-				       const char *hotplug_profile_name)
-{
-	int error;
-
-	error = acpi_scan_add_handler(handler);
-	if (error)
-		return error;
+	if (handler->hotplug)
+		acpi_sysfs_add_hotplug_profile(handler->hotplug);
 
-	acpi_sysfs_add_hotplug_profile(&handler->hotplug, hotplug_profile_name);
 	return 0;
 }
 
@@ -316,13 +306,13 @@ static void acpi_bus_device_eject(void *
 		goto err_out;
 
 	handler = device->handler;
-	if (!handler || !handler->hotplug.enabled) {
+	if (!handler || !handler->hotplug || !handler->hotplug->enabled) {
 		ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
 		goto err_out;
 	}
 	acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST,
 				  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
-	if (handler->hotplug.mode == AHM_CONTAINER) {
+	if (handler->hotplug->mode == AHM_CONTAINER) {
 		device->flags.eject_pending = true;
 		kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);
 	} else {
@@ -371,7 +361,8 @@ static void acpi_scan_bus_device_check(a
 		goto out;
 	}
 	ost_code = ACPI_OST_SC_SUCCESS;
-	if (device->handler && device->handler->hotplug.mode == AHM_CONTAINER)
+	if (device->handler && device->handler->hotplug
+	    && device->handler->hotplug->mode == AHM_CONTAINER)
 		kobject_uevent(&device->dev.kobj, KOBJ_ONLINE);
 
  out:
@@ -426,7 +417,7 @@ static void acpi_hotplug_notify_cb(acpi_
 	struct acpi_scan_handler *handler = data;
 	acpi_status status;
 
-	if (!handler->hotplug.enabled)
+	if (!handler->hotplug || !handler->hotplug->enabled)
 		return acpi_hotplug_unsupported(handle, type);
 
 	switch (type) {
@@ -521,8 +512,8 @@ acpi_eject_store(struct device *d, struc
 	if (!count || buf[0] != '1')
 		return -EINVAL;
 
-	if ((!acpi_device->handler || !acpi_device->handler->hotplug.enabled)
-	    && !acpi_device->driver)
+	if ((!acpi_device->handler || !acpi_device->handler->hotplug
+	    || !acpi_device->handler->hotplug->enabled) && !acpi_device->driver)
 		return -ENODEV;
 
 	status = acpi_get_type(acpi_device->handle, &not_used);
@@ -1883,7 +1874,7 @@ static void acpi_scan_init_hotplug(acpi_
 	 */
 	list_for_each_entry(hwid, &pnp.ids, list) {
 		handler = acpi_scan_match_handler(hwid->id, NULL);
-		if (handler) {
+		if (handler && handler->hotplug) {
 			acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
 					acpi_hotplug_notify_cb, handler);
 			break;
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -88,6 +88,7 @@ enum acpi_hotplug_mode {
 };
 
 struct acpi_hotplug_profile {
+	const char *name;
 	struct kobject kobj;
 	bool enabled:1;
 	enum acpi_hotplug_mode mode;
@@ -104,7 +105,7 @@ struct acpi_scan_handler {
 	struct list_head list_node;
 	int (*attach)(struct acpi_device *dev, const struct acpi_device_id *id);
 	void (*detach)(struct acpi_device *dev);
-	struct acpi_hotplug_profile hotplug;
+	struct acpi_hotplug_profile *hotplug;
 };
 
 /*
Index: linux-pm/drivers/acpi/container.c
===================================================================
--- linux-pm.orig/drivers/acpi/container.c
+++ linux-pm/drivers/acpi/container.c
@@ -51,17 +51,19 @@ static int container_device_attach(struc
 	return 1;
 }
 
+static struct acpi_hotplug_profile container_hotplug_profile = {
+	.name = "container",
+	.enabled = true,
+	.mode = AHM_CONTAINER,
+};
+
 struct acpi_scan_handler container_scan_handler = {
 	.ids = container_device_ids,
 	.attach = container_device_attach,
-	.hotplug = {
-		.enabled = true,
-		.mode = AHM_CONTAINER,
-	},
+	.hotplug = &container_hotplug_profile,
 };
 
 void __init acpi_container_init(void)
 {
-	acpi_scan_add_handler_with_hotplug(&container_scan_handler,
-					   "container");
+	acpi_scan_add_handler(&container_scan_handler);
 }
Index: linux-pm/drivers/acpi/sysfs.c
===================================================================
--- linux-pm.orig/drivers/acpi/sysfs.c
+++ linux-pm/drivers/acpi/sysfs.c
@@ -755,16 +755,18 @@ static struct kobj_type acpi_hotplug_pro
 	.default_attrs = hotplug_profile_attrs,
 };
 
-void acpi_sysfs_add_hotplug_profile(struct acpi_hotplug_profile *hotplug,
-				    const char *name)
+void acpi_sysfs_add_hotplug_profile(struct acpi_hotplug_profile *hotplug)
 {
 	int error;
 
+	if (!hotplug || !hotplug->name)
+		return;
+
 	if (!hotplug_kobj)
 		goto err_out;
 
 	kobject_init(&hotplug->kobj, &acpi_hotplug_profile_ktype);
-	error = kobject_set_name(&hotplug->kobj, "%s", name);
+	error = kobject_set_name(&hotplug->kobj, "%s", hotplug->name);
 	if (error)
 		goto err_out;
 
@@ -777,7 +779,7 @@ void acpi_sysfs_add_hotplug_profile(stru
 	return;
 
  err_out:
-	pr_err(PREFIX "Unable to add hotplug profile '%s'\n", name);
+	pr_err(PREFIX "Unable to add hotplug profile '%s'\n", hotplug->name);
 }
 
 static ssize_t force_remove_show(struct kobject *kobj,
Index: linux-pm/drivers/acpi/acpi_memhotplug.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_memhotplug.c
+++ linux-pm/drivers/acpi/acpi_memhotplug.c
@@ -58,13 +58,16 @@ static const struct acpi_device_id memor
 	{"", 0},
 };
 
+static struct acpi_hotplug_profile memory_hotplug_profile = {
+	.name = "memory",
+	.enabled = true,
+};
+
 static struct acpi_scan_handler memory_device_handler = {
 	.ids = memory_device_ids,
 	.attach = acpi_memory_device_add,
 	.detach = acpi_memory_device_remove,
-	.hotplug = {
-		.enabled = true,
-	},
+	.hotplug = &memory_hotplug_profile,
 };
 
 struct acpi_memory_info {
@@ -361,5 +364,5 @@ static void acpi_memory_device_remove(st
 
 void __init acpi_memory_hotplug_init(void)
 {
-	acpi_scan_add_handler_with_hotplug(&memory_device_handler, "memory");
+	acpi_scan_add_handler(&memory_device_handler);
 }
Index: linux-pm/drivers/acpi/acpi_processor.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_processor.c
+++ linux-pm/drivers/acpi/acpi_processor.c
@@ -477,18 +477,23 @@ static const struct acpi_device_id proce
 	{ }
 };
 
+#ifdef CONFIG_ACPI_HOTPLUG_CPU
+static struct acpi_hotplug_profile processor_hotplug_profile = {
+	.name = "processor",
+	.enabled = true,
+};
+#endif
+
 static struct acpi_scan_handler __refdata processor_handler = {
 	.ids = processor_device_ids,
 	.attach = acpi_processor_add,
 #ifdef CONFIG_ACPI_HOTPLUG_CPU
 	.detach = acpi_processor_remove,
+	.hotplug = &processor_hotplug_profile,
 #endif
-	.hotplug = {
-		.enabled = true,
-	},
 };
 
 void __init acpi_processor_init(void)
 {
-	acpi_scan_add_handler_with_hotplug(&processor_handler, "processor");
+	acpi_scan_add_handler(&processor_handler);
 }

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 3/5] ACPI / scan: Add hotplug profile pointer to struct acpi_device
  2013-06-12 23:23 [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers Rafael J. Wysocki
  2013-06-12 23:23 ` [PATCH 1/5] ACPI / scan: Do not bind ACPI drivers to objects with " Rafael J. Wysocki
  2013-06-12 23:24 ` [PATCH 2/5] ACPI / scan: Separate hotplug profiles from " Rafael J. Wysocki
@ 2013-06-12 23:25 ` Rafael J. Wysocki
  2013-06-12 23:26 ` [PATCH 4/5] ACPI / scan: Use container hotplug profile for matching device objects Rafael J. Wysocki
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 19+ messages in thread
From: Rafael J. Wysocki @ 2013-06-12 23:25 UTC (permalink / raw)
  To: ACPI Devel Maling List; +Cc: LKML, Luck, Tony, Toshi Kani, Aaron Lu

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Since it generally make sense to combine different ACPI scan handlers
(that don't have their own hotplug profiles) with the container
hotplug profile, make it possible by adding a hotplug profile pointer
to struct acpi_device (so that the hotplug profile and scan handler
may be set for device objects independently of each other) and make
the generic hotplug code use the device object's hotplug profile
rather than the hotplug profile from its scan handler.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/scan.c     |   24 +++++++++++++-----------
 include/acpi/acpi_bus.h |    1 +
 2 files changed, 14 insertions(+), 11 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -296,7 +296,7 @@ static void acpi_bus_device_eject(void *
 {
 	acpi_handle handle = context;
 	struct acpi_device *device = NULL;
-	struct acpi_scan_handler *handler;
+	struct acpi_hotplug_profile *hotplug;
 	u32 ost_code = ACPI_OST_SC_NON_SPECIFIC_FAILURE;
 
 	mutex_lock(&acpi_scan_lock);
@@ -305,14 +305,14 @@ static void acpi_bus_device_eject(void *
 	if (!device)
 		goto err_out;
 
-	handler = device->handler;
-	if (!handler || !handler->hotplug || !handler->hotplug->enabled) {
+	hotplug = device->hotplug;
+	if (!hotplug || !hotplug->enabled) {
 		ost_code = ACPI_OST_SC_EJECT_NOT_SUPPORTED;
 		goto err_out;
 	}
 	acpi_evaluate_hotplug_ost(handle, ACPI_NOTIFY_EJECT_REQUEST,
 				  ACPI_OST_SC_EJECT_IN_PROGRESS, NULL);
-	if (handler->hotplug->mode == AHM_CONTAINER) {
+	if (hotplug->mode == AHM_CONTAINER) {
 		device->flags.eject_pending = true;
 		kobject_uevent(&device->dev.kobj, KOBJ_OFFLINE);
 	} else {
@@ -361,8 +361,7 @@ static void acpi_scan_bus_device_check(a
 		goto out;
 	}
 	ost_code = ACPI_OST_SC_SUCCESS;
-	if (device->handler && device->handler->hotplug
-	    && device->handler->hotplug->mode == AHM_CONTAINER)
+	if (device->hotplug && device->hotplug->mode == AHM_CONTAINER)
 		kobject_uevent(&device->dev.kobj, KOBJ_ONLINE);
 
  out:
@@ -414,10 +413,10 @@ static void acpi_hotplug_unsupported(acp
 static void acpi_hotplug_notify_cb(acpi_handle handle, u32 type, void *data)
 {
 	acpi_osd_exec_callback callback;
-	struct acpi_scan_handler *handler = data;
+	struct acpi_hotplug_profile *hotplug = data;
 	acpi_status status;
 
-	if (!handler->hotplug || !handler->hotplug->enabled)
+	if (!hotplug || !hotplug->enabled)
 		return acpi_hotplug_unsupported(handle, type);
 
 	switch (type) {
@@ -512,8 +511,8 @@ acpi_eject_store(struct device *d, struc
 	if (!count || buf[0] != '1')
 		return -EINVAL;
 
-	if ((!acpi_device->handler || !acpi_device->handler->hotplug
-	    || !acpi_device->handler->hotplug->enabled) && !acpi_device->driver)
+	if ((!acpi_device->hotplug || !acpi_device->hotplug->enabled)
+	    && !acpi_device->driver)
 		return -ENODEV;
 
 	status = acpi_get_type(acpi_device->handle, &not_used);
@@ -1876,7 +1875,8 @@ static void acpi_scan_init_hotplug(acpi_
 		handler = acpi_scan_match_handler(hwid->id, NULL);
 		if (handler && handler->hotplug) {
 			acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
-					acpi_hotplug_notify_cb, handler);
+						    acpi_hotplug_notify_cb,
+						    handler->hotplug);
 			break;
 		}
 	}
@@ -1948,6 +1948,7 @@ static int acpi_scan_attach_handler(stru
 			ret = handler->attach(device, devid);
 			if (ret > 0) {
 				device->handler = handler;
+				device->hotplug = handler->hotplug;
 				break;
 			} else if (ret < 0) {
 				break;
@@ -2028,6 +2029,7 @@ static acpi_status acpi_bus_device_detac
 				dev_handler->detach(device);
 
 			device->handler = NULL;
+			device->hotplug = NULL;
 		} else {
 			device_release_driver(&device->dev);
 		}
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -300,6 +300,7 @@ struct acpi_device {
 	struct acpi_device_perf performance;
 	struct acpi_device_dir dir;
 	struct acpi_scan_handler *handler;
+	struct acpi_hotplug_profile *hotplug;
 	struct acpi_driver *driver;
 	void *driver_data;
 	struct device dev;

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 4/5] ACPI / scan: Use container hotplug profile for matching device objects
  2013-06-12 23:23 [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers Rafael J. Wysocki
                   ` (2 preceding siblings ...)
  2013-06-12 23:25 ` [PATCH 3/5] ACPI / scan: Add hotplug profile pointer to struct acpi_device Rafael J. Wysocki
@ 2013-06-12 23:26 ` Rafael J. Wysocki
  2013-06-12 23:26 ` [PATCH 5/5] ACPI / ia64 / sba_iommu: Use ACPI scan handler for discovery Rafael J. Wysocki
  2013-06-13 21:28 ` [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers Toshi Kani
  5 siblings, 0 replies; 19+ messages in thread
From: Rafael J. Wysocki @ 2013-06-12 23:26 UTC (permalink / raw)
  To: ACPI Devel Maling List; +Cc: LKML, Luck, Tony, Toshi Kani, Aaron Lu

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

On some systems (ia64 HP rx2600 in particular) the ACPI namespace
contains device objects with container compatibility IDs in addition
to some other hardware IDs.  This means that those devices are
containers and if an ACPI scan handler without a hotplug profile is
attached to one of them, the container hotplug profile should be used
for that device.

Moreover, the container hotplug profile should be used for handling
bus check and device check notifications targeted at device handles
with matching scan handlers that don't provide hotplug profiles.

Modify the ACPI namespace scan code to ensure that the above will
happen.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/scan.c |   33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -1855,11 +1855,26 @@ void acpi_scan_hotplug_enabled(struct ac
 	mutex_unlock(&acpi_scan_lock);
 }
 
+static struct acpi_hotplug_profile *acpi_container_hotplug(
+						struct acpi_device_pnp *pnp)
+{
+	if (acpi_container_scan_handler()) {
+		struct acpi_scan_handler *handler;
+		struct acpi_hardware_id *hwid;
+
+		handler = acpi_container_scan_handler();
+		list_for_each_entry(hwid, &pnp->ids, list)
+			if (acpi_scan_handler_matching(handler, hwid->id, NULL))
+				return handler->hotplug;
+	}
+	return NULL;
+}
+
 static void acpi_scan_init_hotplug(acpi_handle handle, int type)
 {
 	struct acpi_device_pnp pnp = {};
 	struct acpi_hardware_id *hwid;
-	struct acpi_scan_handler *handler;
+	struct acpi_hotplug_profile *hotplug = NULL;
 
 	INIT_LIST_HEAD(&pnp.ids);
 	acpi_set_pnp_ids(handle, &pnp, type);
@@ -1872,14 +1887,20 @@ static void acpi_scan_init_hotplug(acpi_
 	 * install the same notify handler routine twice for the same handle.
 	 */
 	list_for_each_entry(hwid, &pnp.ids, list) {
+		struct acpi_scan_handler *handler;
+
 		handler = acpi_scan_match_handler(hwid->id, NULL);
 		if (handler && handler->hotplug) {
-			acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
-						    acpi_hotplug_notify_cb,
-						    handler->hotplug);
+			hotplug = handler->hotplug;
 			break;
 		}
 	}
+	if (!hotplug)
+		hotplug = acpi_container_hotplug(&pnp);
+
+	if (hotplug)
+		acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
+					    acpi_hotplug_notify_cb, hotplug);
 
 out:
 	acpi_free_pnp_ids(&pnp);
@@ -1948,7 +1969,9 @@ static int acpi_scan_attach_handler(stru
 			ret = handler->attach(device, devid);
 			if (ret > 0) {
 				device->handler = handler;
-				device->hotplug = handler->hotplug;
+				device->hotplug = handler->hotplug ?
+					handler->hotplug :
+					acpi_container_hotplug(&device->pnp);
 				break;
 			} else if (ret < 0) {
 				break;

^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 5/5] ACPI / ia64 / sba_iommu: Use ACPI scan handler for discovery
  2013-06-12 23:23 [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers Rafael J. Wysocki
                   ` (3 preceding siblings ...)
  2013-06-12 23:26 ` [PATCH 4/5] ACPI / scan: Use container hotplug profile for matching device objects Rafael J. Wysocki
@ 2013-06-12 23:26 ` Rafael J. Wysocki
  2013-06-13 21:28 ` [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers Toshi Kani
  5 siblings, 0 replies; 19+ messages in thread
From: Rafael J. Wysocki @ 2013-06-12 23:26 UTC (permalink / raw)
  To: ACPI Devel Maling List; +Cc: LKML, Luck, Tony, Toshi Kani, Aaron Lu

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

The IA64 System Bus Adapter (SBA) I/O MMU driver uses an ACPI driver
object to look for device objects it needs in the ACPI namespace, but
the way that driver object is used by it is kind of abusive.

First of all, the ACPI driver is registered with the assumption that
its .add() routine (an equivalent of .probe()) will run immediately
during the registration, which in principle may or may not happen (it
happens in practice due to the way the kernel is built, but that's
not actually guaranteed).  Second, .add() is the only functionality
provided by that driver object (which means that it probes devices
without really doing anything with them afterward) and that .add() is
only supposed to run during system initialization, so having the
driver object registered (and present in sysfs) throughout the
system's life time isn't particularly useful.

A cleaner way to achieve the same goal that driver object is for
is to use an ACPI scan handler instead of it, which at least
prevents sysfs from being littered with useless data and causes
an improved algorithm of matching ACPI device IDs to be used.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 arch/ia64/hp/common/sba_iommu.c |   24 ++++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

Index: linux-pm/arch/ia64/hp/common/sba_iommu.c
===================================================================
--- linux-pm.orig/arch/ia64/hp/common/sba_iommu.c
+++ linux-pm/arch/ia64/hp/common/sba_iommu.c
@@ -2042,7 +2042,8 @@ sba_map_ioc_to_node(struct ioc *ioc, acp
 #endif
 
 static int __init
-acpi_sba_ioc_add(struct acpi_device *device)
+acpi_sba_ioc_add(struct acpi_device *device,
+		 const struct acpi_device_id *not_used)
 {
 	struct ioc *ioc;
 	acpi_status status;
@@ -2090,14 +2091,18 @@ static const struct acpi_device_id hp_io
 	{"HWP0004", 0},
 	{"", 0},
 };
-static struct acpi_driver acpi_sba_ioc_driver = {
-	.name		= "IOC IOMMU Driver",
-	.ids		= hp_ioc_iommu_device_ids,
-	.ops		= {
-		.add	= acpi_sba_ioc_add,
-	},
+static struct acpi_scan_handler acpi_sba_ioc_handler = {
+	.ids	= hp_ioc_iommu_device_ids,
+	.attach	= acpi_sba_ioc_add,
 };
 
+static int __init acpi_sba_ioc_init_acpi(void)
+{
+	return acpi_scan_add_handler(&acpi_sba_ioc_handler);
+}
+/* This has to run before acpi_scan_init(). */
+arch_initcall(acpi_sba_ioc_init_acpi);
+
 extern struct dma_map_ops swiotlb_dma_ops;
 
 static int __init
@@ -2122,7 +2127,10 @@ sba_init(void)
 	}
 #endif
 
-	acpi_bus_register_driver(&acpi_sba_ioc_driver);
+	/*
+	 * ioc_list should be populated by the acpi_sba_ioc_handler's .attach()
+	 * routine, but that only happens if acpi_scan_init() has already run.
+	 */
 	if (!ioc_list) {
 #ifdef CONFIG_IA64_GENERIC
 		/*


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers
  2013-06-12 23:23 [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers Rafael J. Wysocki
                   ` (4 preceding siblings ...)
  2013-06-12 23:26 ` [PATCH 5/5] ACPI / ia64 / sba_iommu: Use ACPI scan handler for discovery Rafael J. Wysocki
@ 2013-06-13 21:28 ` Toshi Kani
  2013-06-13 22:13   ` Rafael J. Wysocki
  5 siblings, 1 reply; 19+ messages in thread
From: Toshi Kani @ 2013-06-13 21:28 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: ACPI Devel Maling List, LKML, Luck, Tony, Aaron Lu

On Thu, 2013-06-13 at 01:23 +0200, Rafael J. Wysocki wrote:
> Hi All,
> 
> It turns out that some BIOSes add container device IDs as _CIDs to device
> object that in principle may be matched against the other scan handlers (or
> ACPI drivers, but that's not a problem, because the container scan handler
> can co-exist with an ACPI driver).  That's why our recent fix for an issue
> related to the ACPI video driver had to be reverted right before -rc5.

I am familiar with this firmware, although I no longer have access to
the systems.  An SBA device object has _HID with an HP-specific PNPID
and _CID with a generic container PNPID.  The _HID allows an OS with the
HP SBA driver (which recognizes the _HID) to enable the SBA's I/O TLB
functionality, while the _CID allows an OS without the HP SBA driver to
boot-up by treating this SBA as a container.  The _CID is needed because
some OS skips scanning underneath when it finds an unrecognized object.

> Although I submitted an alternative fix for that bug, I think the problem
> with the container scan handler possibly matching devices already having
> some other scan handlers attached needs addressing, because we may need to
> use the container hotplug profile for those devices.  The following patch
> series is supposed to address it.

When the HP SBA driver is bound to the SBA object, this driver needs to
handle a hotplug request when it is supported.  This is because the I/O
TLB functionality requires its hot-delete operation as well.  The
container scan handler can be used only when this driver is bound to the
SBA object as a container and therefore its I/O TLB functionality is not
used. 

> [1/5] ACPI / scan: Do not bind ACPI drivers to objects with scan handlers
>    (this version shouldn't break the Tony's IA64 HP box the previous one broke)
> [2/5] ACPI / scan: Separate hotplug profiles from scan handlers
> [3/5] ACPI / scan: Add hotplug profile pointer to struct acpi_device
> [4/5] ACPI / scan: Use container hotplug profile for matching device objects
> [5/5] ACPI / ia64 / sba_iommu: Use ACPI scan handler for discovery
> 
> Patches [1-4/5] were run on my Toshiba test box and didn't break it, but it
> really doesn't do any ACPI hotplug notifications.
> 
> Patch [5/5] is kind of additional, but it wouldn't work correctly without the
> previous ones (to be honest, I haven't tried to compile it yet, but here it
> goes for completness).

I think we only need patch [5/5] to address the problem.  We have
enhanced the match function of scan handlers to match a proper driver
with respect to their priority order, i.e. matching with _HID first and
then with _CIDs.  Patch [5/5] should assure that the HP SBA driver is
bound to an SBA object when this driver is configured to the kernel.

Thanks,
-Toshi


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers
  2013-06-13 21:28 ` [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers Toshi Kani
@ 2013-06-13 22:13   ` Rafael J. Wysocki
  2013-06-13 22:23     ` Toshi Kani
  0 siblings, 1 reply; 19+ messages in thread
From: Rafael J. Wysocki @ 2013-06-13 22:13 UTC (permalink / raw)
  To: Toshi Kani; +Cc: ACPI Devel Maling List, LKML, Luck, Tony, Aaron Lu

On Thursday, June 13, 2013 03:28:59 PM Toshi Kani wrote:
> On Thu, 2013-06-13 at 01:23 +0200, Rafael J. Wysocki wrote:
> > Hi All,
> > 
> > It turns out that some BIOSes add container device IDs as _CIDs to device
> > object that in principle may be matched against the other scan handlers (or
> > ACPI drivers, but that's not a problem, because the container scan handler
> > can co-exist with an ACPI driver).  That's why our recent fix for an issue
> > related to the ACPI video driver had to be reverted right before -rc5.
> 
> I am familiar with this firmware, although I no longer have access to
> the systems.  An SBA device object has _HID with an HP-specific PNPID
> and _CID with a generic container PNPID.  The _HID allows an OS with the
> HP SBA driver (which recognizes the _HID) to enable the SBA's I/O TLB
> functionality, while the _CID allows an OS without the HP SBA driver to
> boot-up by treating this SBA as a container.  The _CID is needed because
> some OS skips scanning underneath when it finds an unrecognized object.

How cute.

> > Although I submitted an alternative fix for that bug, I think the problem
> > with the container scan handler possibly matching devices already having
> > some other scan handlers attached needs addressing, because we may need to
> > use the container hotplug profile for those devices.  The following patch
> > series is supposed to address it.
> 
> When the HP SBA driver is bound to the SBA object, this driver needs to
> handle a hotplug request when it is supported.  This is because the I/O
> TLB functionality requires its hot-delete operation as well.  The
> container scan handler can be used only when this driver is bound to the
> SBA object as a container and therefore its I/O TLB functionality is not
> used. 

Ah, so in fact those device IDs are kind of mutually exclusive?  That is,
we only should use the _CID if we don't use the _HID, right?

We have a bug there, then, but it probably is bening enough for 3.10 to be left
as is.

> > [1/5] ACPI / scan: Do not bind ACPI drivers to objects with scan handlers
> >    (this version shouldn't break the Tony's IA64 HP box the previous one broke)
> > [2/5] ACPI / scan: Separate hotplug profiles from scan handlers
> > [3/5] ACPI / scan: Add hotplug profile pointer to struct acpi_device
> > [4/5] ACPI / scan: Use container hotplug profile for matching device objects
> > [5/5] ACPI / ia64 / sba_iommu: Use ACPI scan handler for discovery
> > 
> > Patches [1-4/5] were run on my Toshiba test box and didn't break it, but it
> > really doesn't do any ACPI hotplug notifications.
> > 
> > Patch [5/5] is kind of additional, but it wouldn't work correctly without the
> > previous ones (to be honest, I haven't tried to compile it yet, but here it
> > goes for completness).
> 
> I think we only need patch [5/5] to address the problem.  We have
> enhanced the match function of scan handlers to match a proper driver
> with respect to their priority order, i.e. matching with _HID first and
> then with _CIDs.  Patch [5/5] should assure that the HP SBA driver is
> bound to an SBA object when this driver is configured to the kernel.

OK, but then I'd like to apply a modified version of [1/5] that won't
check if the scan handler is the container handler, but will just return
-EINVAL if any scan handler has been set already.  And the changelog of
[5/5] needs to be modified slightly.

Tony promised me to test those patches on his box, so we'll know for sure
in a while.

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers
  2013-06-13 22:13   ` Rafael J. Wysocki
@ 2013-06-13 22:23     ` Toshi Kani
  2013-06-14 18:01       ` Luck, Tony
  0 siblings, 1 reply; 19+ messages in thread
From: Toshi Kani @ 2013-06-13 22:23 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: ACPI Devel Maling List, LKML, Luck, Tony, Aaron Lu

On Fri, 2013-06-14 at 00:13 +0200, Rafael J. Wysocki wrote:
> On Thursday, June 13, 2013 03:28:59 PM Toshi Kani wrote:
> > On Thu, 2013-06-13 at 01:23 +0200, Rafael J. Wysocki wrote:
> > > Hi All,
> > > 
> > > It turns out that some BIOSes add container device IDs as _CIDs to device
> > > object that in principle may be matched against the other scan handlers (or
> > > ACPI drivers, but that's not a problem, because the container scan handler
> > > can co-exist with an ACPI driver).  That's why our recent fix for an issue
> > > related to the ACPI video driver had to be reverted right before -rc5.
> > 
> > I am familiar with this firmware, although I no longer have access to
> > the systems.  An SBA device object has _HID with an HP-specific PNPID
> > and _CID with a generic container PNPID.  The _HID allows an OS with the
> > HP SBA driver (which recognizes the _HID) to enable the SBA's I/O TLB
> > functionality, while the _CID allows an OS without the HP SBA driver to
> > boot-up by treating this SBA as a container.  The _CID is needed because
> > some OS skips scanning underneath when it finds an unrecognized object.
> 
> How cute.
> 
> > > Although I submitted an alternative fix for that bug, I think the problem
> > > with the container scan handler possibly matching devices already having
> > > some other scan handlers attached needs addressing, because we may need to
> > > use the container hotplug profile for those devices.  The following patch
> > > series is supposed to address it.
> > 
> > When the HP SBA driver is bound to the SBA object, this driver needs to
> > handle a hotplug request when it is supported.  This is because the I/O
> > TLB functionality requires its hot-delete operation as well.  The
> > container scan handler can be used only when this driver is bound to the
> > SBA object as a container and therefore its I/O TLB functionality is not
> > used. 
> 
> Ah, so in fact those device IDs are kind of mutually exclusive?  That is,
> we only should use the _CID if we don't use the _HID, right?

Yes and yes.

> We have a bug there, then, but it probably is bening enough for 3.10 to be left
> as is.
> 
> > > [1/5] ACPI / scan: Do not bind ACPI drivers to objects with scan handlers
> > >    (this version shouldn't break the Tony's IA64 HP box the previous one broke)
> > > [2/5] ACPI / scan: Separate hotplug profiles from scan handlers
> > > [3/5] ACPI / scan: Add hotplug profile pointer to struct acpi_device
> > > [4/5] ACPI / scan: Use container hotplug profile for matching device objects
> > > [5/5] ACPI / ia64 / sba_iommu: Use ACPI scan handler for discovery
> > > 
> > > Patches [1-4/5] were run on my Toshiba test box and didn't break it, but it
> > > really doesn't do any ACPI hotplug notifications.
> > > 
> > > Patch [5/5] is kind of additional, but it wouldn't work correctly without the
> > > previous ones (to be honest, I haven't tried to compile it yet, but here it
> > > goes for completness).
> > 
> > I think we only need patch [5/5] to address the problem.  We have
> > enhanced the match function of scan handlers to match a proper driver
> > with respect to their priority order, i.e. matching with _HID first and
> > then with _CIDs.  Patch [5/5] should assure that the HP SBA driver is
> > bound to an SBA object when this driver is configured to the kernel.
> 
> OK, but then I'd like to apply a modified version of [1/5] that won't
> check if the scan handler is the container handler, but will just return
> -EINVAL if any scan handler has been set already.  And the changelog of
> [5/5] needs to be modified slightly.

Agreed.

> Tony promised me to test those patches on his box, so we'll know for sure
> in a while.

Cool.

Thanks,
-Toshi



^ permalink raw reply	[flat|nested] 19+ messages in thread

* RE: [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers
  2013-06-13 22:23     ` Toshi Kani
@ 2013-06-14 18:01       ` Luck, Tony
  2013-06-14 22:23         ` Rafael J. Wysocki
  0 siblings, 1 reply; 19+ messages in thread
From: Luck, Tony @ 2013-06-14 18:01 UTC (permalink / raw)
  To: Toshi Kani, Rafael J. Wysocki; +Cc: ACPI Devel Maling List, LKML, Lu, Aaron

>> Tony promised me to test those patches on his box, so we'll know for sure
>> in a while.

Tested this series - and the box boots just fine with no unexpected messages.

But I should note that this box doesn't have anything that is hot pluggable, so I
couldn't test hotplug (which seems to be deeply involved with things that this
patch is touching).

Of course that means that I haven't been testing hotplug - so it might have been
broken for years and I'd never have noticed.

-Tony

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers
  2013-06-14 18:01       ` Luck, Tony
@ 2013-06-14 22:23         ` Rafael J. Wysocki
  2013-06-14 22:32           ` Tony Luck
  0 siblings, 1 reply; 19+ messages in thread
From: Rafael J. Wysocki @ 2013-06-14 22:23 UTC (permalink / raw)
  To: Luck, Tony; +Cc: Toshi Kani, ACPI Devel Maling List, LKML, Lu, Aaron

On Friday, June 14, 2013 06:01:41 PM Luck, Tony wrote:
> >> Tony promised me to test those patches on his box, so we'll know for sure
> >> in a while.
> 
> Tested this series - and the box boots just fine with no unexpected messages.

Thanks!

> But I should note that this box doesn't have anything that is hot pluggable, so I
> couldn't test hotplug (which seems to be deeply involved with things that this
> patch is touching).
> 
> Of course that means that I haven't been testing hotplug - so it might have been
> broken for years and I'd never have noticed.

Can you please just test patch [5/5] alone without patches [1-4/5]?  We believe
that this should work too and if that's the case, we'll only need that patch
and a reworked [1/5].

Thanks,
Rafael


-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers
  2013-06-14 22:23         ` Rafael J. Wysocki
@ 2013-06-14 22:32           ` Tony Luck
  2013-06-14 23:20             ` Rafael J. Wysocki
  0 siblings, 1 reply; 19+ messages in thread
From: Tony Luck @ 2013-06-14 22:32 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Toshi Kani, ACPI Devel Maling List, LKML, Lu, Aaron

On Fri, Jun 14, 2013 at 3:23 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> Can you please just test patch [5/5] alone without patches [1-4/5]?  We believe
> that this should work too and if that's the case, we'll only need that patch
> and a reworked [1/5].

Your belief is sound - I popped all five patches and then applied just
5/5 ... and
the system still works.

-Tony

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers
  2013-06-14 22:32           ` Tony Luck
@ 2013-06-14 23:20             ` Rafael J. Wysocki
  2013-06-19 17:37               ` Tony Luck
  0 siblings, 1 reply; 19+ messages in thread
From: Rafael J. Wysocki @ 2013-06-14 23:20 UTC (permalink / raw)
  To: Tony Luck; +Cc: Toshi Kani, ACPI Devel Maling List, LKML, Lu, Aaron

On Friday, June 14, 2013 03:32:42 PM Tony Luck wrote:
> On Fri, Jun 14, 2013 at 3:23 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > Can you please just test patch [5/5] alone without patches [1-4/5]?  We believe
> > that this should work too and if that's the case, we'll only need that patch
> > and a reworked [1/5].
> 
> Your belief is sound - I popped all five patches and then applied just
> 5/5 ... and
> the system still works.

Great, thanks!

Can you please apply the appended patch on top of it and see if the system
still works then?

Rafael


---
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: ACPI / scan: Do not bind ACPI drivers to objects with scan handlers

ACPI drivers must not be bound to device objects having scan handlers
attatched to them, so make acpi_device_probe() fail with -EINVAL if the
device object being probed has an ACPI scan handler.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/scan.c  |    3 +++
 drivers/acpi/video.c |    3 ---
 2 files changed, 3 insertions(+), 3 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -939,6 +939,9 @@ static int acpi_device_probe(struct devi
 	struct acpi_driver *acpi_drv = to_acpi_driver(dev->driver);
 	int ret;
 
+	if (acpi_dev->handler)
+		return -EINVAL;
+
 	if (!acpi_drv->ops.add)
 		return -ENOSYS;
 
Index: linux-pm/drivers/acpi/video.c
===================================================================
--- linux-pm.orig/drivers/acpi/video.c
+++ linux-pm/drivers/acpi/video.c
@@ -1722,9 +1722,6 @@ static int acpi_video_bus_add(struct acp
 	int error;
 	acpi_status status;
 
-	if (device->handler)
-		return -EINVAL;
-
 	status = acpi_walk_namespace(ACPI_TYPE_DEVICE,
 				device->parent->handle, 1,
 				acpi_video_bus_match, NULL,


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers
  2013-06-14 23:20             ` Rafael J. Wysocki
@ 2013-06-19 17:37               ` Tony Luck
  2013-06-19 22:24                 ` Rafael J. Wysocki
  0 siblings, 1 reply; 19+ messages in thread
From: Tony Luck @ 2013-06-19 17:37 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Toshi Kani, ACPI Devel Maling List, LKML, Lu, Aaron

> Can you please apply the appended patch on top of it and see if the system
> still works then?

Still works with this patch.

-Tony

> ---
>  drivers/acpi/scan.c  |    3 +++
>  drivers/acpi/video.c |    3 ---
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> Index: linux-pm/drivers/acpi/scan.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/scan.c
> +++ linux-pm/drivers/acpi/scan.c
> @@ -939,6 +939,9 @@ static int acpi_device_probe(struct devi
>         struct acpi_driver *acpi_drv = to_acpi_driver(dev->driver);
>         int ret;
>
> +       if (acpi_dev->handler)
> +               return -EINVAL;
> +
>         if (!acpi_drv->ops.add)
>                 return -ENOSYS;
>
> Index: linux-pm/drivers/acpi/video.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/video.c
> +++ linux-pm/drivers/acpi/video.c
> @@ -1722,9 +1722,6 @@ static int acpi_video_bus_add(struct acp
>         int error;
>         acpi_status status;
>
> -       if (device->handler)
> -               return -EINVAL;
> -
>         status = acpi_walk_namespace(ACPI_TYPE_DEVICE,
>                                 device->parent->handle, 1,
>                                 acpi_video_bus_match, NULL,
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers
  2013-06-19 17:37               ` Tony Luck
@ 2013-06-19 22:24                 ` Rafael J. Wysocki
  2013-06-19 22:40                   ` Tony Luck
  2013-06-19 23:22                   ` Toshi Kani
  0 siblings, 2 replies; 19+ messages in thread
From: Rafael J. Wysocki @ 2013-06-19 22:24 UTC (permalink / raw)
  To: Tony Luck; +Cc: Toshi Kani, ACPI Devel Maling List, LKML, Lu, Aaron

On Wednesday, June 19, 2013 10:37:27 AM Tony Luck wrote:
> > Can you please apply the appended patch on top of it and see if the system
> > still works then?
> 
> Still works with this patch.

Cool, thanks! :-)

If you don't mind, I'll queue up https://patchwork.kernel.org/patch/2712741/ and
this for 3.11.

Thanks,
Rafael


> > ---
> >  drivers/acpi/scan.c  |    3 +++
> >  drivers/acpi/video.c |    3 ---
> >  2 files changed, 3 insertions(+), 3 deletions(-)
> >
> > Index: linux-pm/drivers/acpi/scan.c
> > ===================================================================
> > --- linux-pm.orig/drivers/acpi/scan.c
> > +++ linux-pm/drivers/acpi/scan.c
> > @@ -939,6 +939,9 @@ static int acpi_device_probe(struct devi
> >         struct acpi_driver *acpi_drv = to_acpi_driver(dev->driver);
> >         int ret;
> >
> > +       if (acpi_dev->handler)
> > +               return -EINVAL;
> > +
> >         if (!acpi_drv->ops.add)
> >                 return -ENOSYS;
> >
> > Index: linux-pm/drivers/acpi/video.c
> > ===================================================================
> > --- linux-pm.orig/drivers/acpi/video.c
> > +++ linux-pm/drivers/acpi/video.c
> > @@ -1722,9 +1722,6 @@ static int acpi_video_bus_add(struct acp
> >         int error;
> >         acpi_status status;
> >
> > -       if (device->handler)
> > -               return -EINVAL;
> > -
> >         status = acpi_walk_namespace(ACPI_TYPE_DEVICE,
> >                                 device->parent->handle, 1,
> >                                 acpi_video_bus_match, NULL,
> >
-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers
  2013-06-19 22:24                 ` Rafael J. Wysocki
@ 2013-06-19 22:40                   ` Tony Luck
  2013-06-19 22:51                     ` Rafael J. Wysocki
  2013-06-19 23:22                   ` Toshi Kani
  1 sibling, 1 reply; 19+ messages in thread
From: Tony Luck @ 2013-06-19 22:40 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Toshi Kani, ACPI Devel Maling List, LKML, Lu, Aaron

> If you don't mind, I'll queue up https://patchwork.kernel.org/patch/2712741/ and
> this for 3.11.

Mark them

Tested-by: Tony Luck <tony.luck@intel.com>

if you like.

-Tony

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers
  2013-06-19 22:40                   ` Tony Luck
@ 2013-06-19 22:51                     ` Rafael J. Wysocki
  0 siblings, 0 replies; 19+ messages in thread
From: Rafael J. Wysocki @ 2013-06-19 22:51 UTC (permalink / raw)
  To: Tony Luck; +Cc: Toshi Kani, ACPI Devel Maling List, LKML, Lu, Aaron

On Wednesday, June 19, 2013 03:40:21 PM Tony Luck wrote:
> > If you don't mind, I'll queue up https://patchwork.kernel.org/patch/2712741/ and
> > this for 3.11.
> 
> Mark them
> 
> Tested-by: Tony Luck <tony.luck@intel.com>
> 
> if you like.

I will, thank you!

Rafael


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers
  2013-06-19 22:24                 ` Rafael J. Wysocki
  2013-06-19 22:40                   ` Tony Luck
@ 2013-06-19 23:22                   ` Toshi Kani
  2013-06-19 23:35                     ` Rafael J. Wysocki
  1 sibling, 1 reply; 19+ messages in thread
From: Toshi Kani @ 2013-06-19 23:22 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Tony Luck, ACPI Devel Maling List, LKML, Lu, Aaron

On Thu, 2013-06-20 at 00:24 +0200, Rafael J. Wysocki wrote:
> On Wednesday, June 19, 2013 10:37:27 AM Tony Luck wrote:
> > > Can you please apply the appended patch on top of it and see if the system
> > > still works then?
> > 
> > Still works with this patch.
> 
> Cool, thanks! :-)
> 
> If you don't mind, I'll queue up https://patchwork.kernel.org/patch/2712741/ and
> this for 3.11.

For both patches:

Acked-by: Toshi Kani <toshi.kani@hp.com>

Thanks,
-Toshi


> 
> Thanks,
> Rafael
> 
> 
> > > ---
> > >  drivers/acpi/scan.c  |    3 +++
> > >  drivers/acpi/video.c |    3 ---
> > >  2 files changed, 3 insertions(+), 3 deletions(-)
> > >
> > > Index: linux-pm/drivers/acpi/scan.c
> > > ===================================================================
> > > --- linux-pm.orig/drivers/acpi/scan.c
> > > +++ linux-pm/drivers/acpi/scan.c
> > > @@ -939,6 +939,9 @@ static int acpi_device_probe(struct devi
> > >         struct acpi_driver *acpi_drv = to_acpi_driver(dev->driver);
> > >         int ret;
> > >
> > > +       if (acpi_dev->handler)
> > > +               return -EINVAL;
> > > +
> > >         if (!acpi_drv->ops.add)
> > >                 return -ENOSYS;
> > >
> > > Index: linux-pm/drivers/acpi/video.c
> > > ===================================================================
> > > --- linux-pm.orig/drivers/acpi/video.c
> > > +++ linux-pm/drivers/acpi/video.c
> > > @@ -1722,9 +1722,6 @@ static int acpi_video_bus_add(struct acp
> > >         int error;
> > >         acpi_status status;
> > >
> > > -       if (device->handler)
> > > -               return -EINVAL;
> > > -
> > >         status = acpi_walk_namespace(ACPI_TYPE_DEVICE,
> > >                                 device->parent->handle, 1,
> > >                                 acpi_video_bus_match, NULL,
> > >



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers
  2013-06-19 23:22                   ` Toshi Kani
@ 2013-06-19 23:35                     ` Rafael J. Wysocki
  0 siblings, 0 replies; 19+ messages in thread
From: Rafael J. Wysocki @ 2013-06-19 23:35 UTC (permalink / raw)
  To: Toshi Kani; +Cc: Tony Luck, ACPI Devel Maling List, LKML, Lu, Aaron

On Wednesday, June 19, 2013 05:22:04 PM Toshi Kani wrote:
> On Thu, 2013-06-20 at 00:24 +0200, Rafael J. Wysocki wrote:
> > On Wednesday, June 19, 2013 10:37:27 AM Tony Luck wrote:
> > > > Can you please apply the appended patch on top of it and see if the system
> > > > still works then?
> > > 
> > > Still works with this patch.
> > 
> > Cool, thanks! :-)
> > 
> > If you don't mind, I'll queue up https://patchwork.kernel.org/patch/2712741/ and
> > this for 3.11.
> 
> For both patches:
> 
> Acked-by: Toshi Kani <toshi.kani@hp.com>

Thanks!


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2013-06-19 23:26 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-06-12 23:23 [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers Rafael J. Wysocki
2013-06-12 23:23 ` [PATCH 1/5] ACPI / scan: Do not bind ACPI drivers to objects with " Rafael J. Wysocki
2013-06-12 23:24 ` [PATCH 2/5] ACPI / scan: Separate hotplug profiles from " Rafael J. Wysocki
2013-06-12 23:25 ` [PATCH 3/5] ACPI / scan: Add hotplug profile pointer to struct acpi_device Rafael J. Wysocki
2013-06-12 23:26 ` [PATCH 4/5] ACPI / scan: Use container hotplug profile for matching device objects Rafael J. Wysocki
2013-06-12 23:26 ` [PATCH 5/5] ACPI / ia64 / sba_iommu: Use ACPI scan handler for discovery Rafael J. Wysocki
2013-06-13 21:28 ` [PATCH 0/5] ACPI / scan: Make it possible to use the container hotplug with other scan handlers Toshi Kani
2013-06-13 22:13   ` Rafael J. Wysocki
2013-06-13 22:23     ` Toshi Kani
2013-06-14 18:01       ` Luck, Tony
2013-06-14 22:23         ` Rafael J. Wysocki
2013-06-14 22:32           ` Tony Luck
2013-06-14 23:20             ` Rafael J. Wysocki
2013-06-19 17:37               ` Tony Luck
2013-06-19 22:24                 ` Rafael J. Wysocki
2013-06-19 22:40                   ` Tony Luck
2013-06-19 22:51                     ` Rafael J. Wysocki
2013-06-19 23:22                   ` Toshi Kani
2013-06-19 23:35                     ` Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox