* [PATCH 0/5] ACPI: Rework acpi_bus_trim()
@ 2013-01-14 21:33 Rafael J. Wysocki
2013-01-14 21:37 ` [PATCH 1/5] ACPI: Remove the ops field from struct acpi_device Rafael J. Wysocki
` (6 more replies)
0 siblings, 7 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2013-01-14 21:33 UTC (permalink / raw)
To: ACPI Devel Maling List
Cc: Bjorn Helgaas, LKML, Toshi Kani, Yinghai Lu, Jiang Liu,
Taku Izumi
Hi All,
This series of patches changes the way acpi_bus_trim() works so that,
eventually, it walks the namespace twice detaching (ACPI) drivers from
device nodes being removed in the first pass and removing the device
nodes in the second pass.
The first two patches are just cleanups removing unused stuff:
[1/5] Remove the ops field from struct acpi_device (unrelated to the rest).
[2/5] Drop the second argument of acpi_device_unregister().
The next three patches actually rework acpi_bus_trim() in three steps:
[3/5] Drop the second argument of acpi_bus_trim() (all callers pass 1 in there
anyway.
[4/5] Reimplement acpi_bus_trim() using acpi_walk_namespace().
[3/5] Make acpi_bus_trim() carry out two passes as described above.
I'm aware of the fact that this will conflict with the patches that Yinghai
posted a few days ago, but in my opinion the changes here are prerequisite for
the Yinghai's patchset.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/5] ACPI: Remove the ops field from struct acpi_device
2013-01-14 21:33 [PATCH 0/5] ACPI: Rework acpi_bus_trim() Rafael J. Wysocki
@ 2013-01-14 21:37 ` Rafael J. Wysocki
2013-01-14 21:37 ` [PATCH 2/5] ACPI / scan: Drop the second argument of acpi_device_unregister() Rafael J. Wysocki
` (5 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2013-01-14 21:37 UTC (permalink / raw)
To: ACPI Devel Maling List
Cc: Bjorn Helgaas, LKML, Toshi Kani, Yinghai Lu, Jiang Liu,
Taku Izumi
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
The ops field in struct acpi_device is not used anywhere, so remove
it.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
include/acpi/acpi_bus.h | 1 -
1 file changed, 1 deletion(-)
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -271,7 +271,6 @@ struct acpi_device {
struct acpi_device_wakeup wakeup;
struct acpi_device_perf performance;
struct acpi_device_dir dir;
- struct acpi_device_ops ops;
struct acpi_driver *driver;
void *driver_data;
struct device dev;
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/5] ACPI / scan: Drop the second argument of acpi_device_unregister()
2013-01-14 21:33 [PATCH 0/5] ACPI: Rework acpi_bus_trim() Rafael J. Wysocki
2013-01-14 21:37 ` [PATCH 1/5] ACPI: Remove the ops field from struct acpi_device Rafael J. Wysocki
@ 2013-01-14 21:37 ` Rafael J. Wysocki
2013-01-14 21:37 ` [PATCH 3/5] ACPI / scan: Drop the second argument of acpi_bus_trim() Rafael J. Wysocki
` (4 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2013-01-14 21:37 UTC (permalink / raw)
To: ACPI Devel Maling List
Cc: Bjorn Helgaas, LKML, Toshi Kani, Yinghai Lu, Jiang Liu,
Taku Izumi
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Drop the second argument of acpi_device_unregister(), type, which is
not used by that function.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/scan.c | 6 +++---
1 file 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
@@ -776,7 +776,7 @@ int acpi_device_register(struct acpi_dev
return result;
}
-static void acpi_device_unregister(struct acpi_device *device, int type)
+static void acpi_device_unregister(struct acpi_device *device)
{
mutex_lock(&acpi_device_lock);
if (device->parent)
@@ -1436,7 +1436,7 @@ static int acpi_bus_remove(struct acpi_d
if (!rmdevice)
return 0;
- acpi_device_unregister(dev, ACPI_BUS_REMOVAL_EJECT);
+ acpi_device_unregister(dev);
return 0;
}
@@ -1750,7 +1750,7 @@ int __init acpi_scan_init(void)
result = acpi_bus_scan_fixed();
if (result)
- acpi_device_unregister(acpi_root, ACPI_BUS_REMOVAL_NORMAL);
+ acpi_device_unregister(acpi_root);
else
acpi_update_all_gpes();
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/5] ACPI / scan: Drop the second argument of acpi_bus_trim()
2013-01-14 21:33 [PATCH 0/5] ACPI: Rework acpi_bus_trim() Rafael J. Wysocki
2013-01-14 21:37 ` [PATCH 1/5] ACPI: Remove the ops field from struct acpi_device Rafael J. Wysocki
2013-01-14 21:37 ` [PATCH 2/5] ACPI / scan: Drop the second argument of acpi_device_unregister() Rafael J. Wysocki
@ 2013-01-14 21:37 ` Rafael J. Wysocki
2013-01-14 21:38 ` [PATCH 4/5] ACPI / scan: Change the implementation " Rafael J. Wysocki
` (3 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2013-01-14 21:37 UTC (permalink / raw)
To: ACPI Devel Maling List
Cc: Bjorn Helgaas, LKML, Toshi Kani, Yinghai Lu, Jiang Liu,
Taku Izumi
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
All callers of acpi_bus_trim() pass 1 (true) as the second argument
of it, so remove that argument entirely and change acpi_bus_trim()
to always behave as though it were 1.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/dock.c | 2 +-
drivers/acpi/scan.c | 16 ++++------------
drivers/pci/hotplug/acpiphp_glue.c | 4 ++--
drivers/pci/hotplug/sgi_hotplug.c | 2 +-
include/acpi/acpi_bus.h | 2 +-
5 files changed, 9 insertions(+), 17 deletions(-)
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -133,7 +133,7 @@ void acpi_bus_hot_remove_device(void *co
ACPI_DEBUG_PRINT((ACPI_DB_INFO,
"Hot-removing device %s...\n", dev_name(&device->dev)));
- if (acpi_bus_trim(device, 1)) {
+ if (acpi_bus_trim(device)) {
printk(KERN_ERR PREFIX
"Removing device failed\n");
goto err_out;
@@ -1425,7 +1425,7 @@ static void acpi_device_set_id(struct ac
}
}
-static int acpi_bus_remove(struct acpi_device *dev, int rmdevice)
+static int acpi_bus_remove(struct acpi_device *dev)
{
if (!dev)
return -EINVAL;
@@ -1433,9 +1433,6 @@ static int acpi_bus_remove(struct acpi_d
dev->removal_type = ACPI_BUS_REMOVAL_EJECT;
device_release_driver(&dev->dev);
- if (!rmdevice)
- return 0;
-
acpi_device_unregister(dev);
return 0;
@@ -1647,7 +1644,7 @@ int acpi_bus_add(acpi_handle handle)
}
EXPORT_SYMBOL(acpi_bus_add);
-int acpi_bus_trim(struct acpi_device *start, int rmdevice)
+int acpi_bus_trim(struct acpi_device *start)
{
acpi_status status;
struct acpi_device *parent, *child;
@@ -1673,12 +1670,7 @@ int acpi_bus_trim(struct acpi_device *st
acpi_get_parent(phandle, &phandle);
child = parent;
parent = parent->parent;
-
- if (level == 0)
- err = acpi_bus_remove(child, rmdevice);
- else
- err = acpi_bus_remove(child, 1);
-
+ err = acpi_bus_remove(child);
continue;
}
Index: linux-pm/include/acpi/acpi_bus.h
===================================================================
--- linux-pm.orig/include/acpi/acpi_bus.h
+++ linux-pm/include/acpi/acpi_bus.h
@@ -387,7 +387,7 @@ int acpi_bus_register_driver(struct acpi
void acpi_bus_unregister_driver(struct acpi_driver *driver);
int acpi_bus_add(acpi_handle handle);
void acpi_bus_hot_remove_device(void *context);
-int acpi_bus_trim(struct acpi_device *start, int rmdevice);
+int acpi_bus_trim(struct acpi_device *start);
acpi_status acpi_bus_get_ejd(acpi_handle handle, acpi_handle * ejd);
int acpi_match_device_ids(struct acpi_device *device,
const struct acpi_device_id *ids);
Index: linux-pm/drivers/acpi/dock.c
===================================================================
--- linux-pm.orig/drivers/acpi/dock.c
+++ linux-pm/drivers/acpi/dock.c
@@ -339,7 +339,7 @@ static void dock_remove_acpi_device(acpi
int ret;
if (!acpi_bus_get_device(handle, &device)) {
- ret = acpi_bus_trim(device, 1);
+ ret = acpi_bus_trim(device);
if (ret)
pr_debug("error removing bus, %x\n", -ret);
}
Index: linux-pm/drivers/pci/hotplug/acpiphp_glue.c
===================================================================
--- linux-pm.orig/drivers/pci/hotplug/acpiphp_glue.c
+++ linux-pm/drivers/pci/hotplug/acpiphp_glue.c
@@ -742,7 +742,7 @@ static int acpiphp_bus_add(struct acpiph
/* this shouldn't be in here, so remove
* the bus then re-add it...
*/
- ret_val = acpi_bus_trim(device, 1);
+ ret_val = acpi_bus_trim(device);
dbg("acpi_bus_trim return %x\n", ret_val);
}
@@ -772,7 +772,7 @@ static int acpiphp_bus_trim(acpi_handle
return retval;
}
- retval = acpi_bus_trim(device, 1);
+ retval = acpi_bus_trim(device);
if (retval)
err("cannot remove from acpi list\n");
Index: linux-pm/drivers/pci/hotplug/sgi_hotplug.c
===================================================================
--- linux-pm.orig/drivers/pci/hotplug/sgi_hotplug.c
+++ linux-pm/drivers/pci/hotplug/sgi_hotplug.c
@@ -535,7 +535,7 @@ static int disable_slot(struct hotplug_s
ret = acpi_bus_get_device(chandle,
&device);
if (ACPI_SUCCESS(ret))
- acpi_bus_trim(device, 1);
+ acpi_bus_trim(device);
}
}
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/5] ACPI / scan: Change the implementation of acpi_bus_trim()
2013-01-14 21:33 [PATCH 0/5] ACPI: Rework acpi_bus_trim() Rafael J. Wysocki
` (2 preceding siblings ...)
2013-01-14 21:37 ` [PATCH 3/5] ACPI / scan: Drop the second argument of acpi_bus_trim() Rafael J. Wysocki
@ 2013-01-14 21:38 ` Rafael J. Wysocki
2013-01-14 21:39 ` [PATCH 5/5] ACPI / scan: Add second pass to acpi_bus_trim() Rafael J. Wysocki
` (2 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2013-01-14 21:38 UTC (permalink / raw)
To: ACPI Devel Maling List
Cc: Bjorn Helgaas, LKML, Toshi Kani, Yinghai Lu, Jiang Liu,
Taku Izumi
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
The current acpi_bus_trim() implementation is not really
straightforward and may be simplified significantly by using
acpi_walk_namespace() with acpi_bus_remove() as a post-order
callback.
Observe that acpi_bus_remove(), as called by acpi_bus_trim(), cannot
actually fail, because its first argument is guaranteed not to be
NULL thanks to the acpi_bus_get_device() check in acpi_bus_trim(),
so simply move the acpi_bus_get_device() check to acpi_bus_remove()
and use acpi_walk_namespace() to execute it for every device under
start->handle as a post-order callback. The, run it directly for
start->handle itself.
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/scan.c | 60 +++++++++-------------------------------------------
1 file changed, 11 insertions(+), 49 deletions(-)
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -1425,17 +1425,20 @@ static void acpi_device_set_id(struct ac
}
}
-static int acpi_bus_remove(struct acpi_device *dev)
+static acpi_status acpi_bus_remove(acpi_handle handle, u32 lvl_not_used,
+ void *not_used, void **ret_not_used)
{
- if (!dev)
- return -EINVAL;
+ struct acpi_device *dev = NULL;
+
+ if (acpi_bus_get_device(handle, &dev))
+ return AE_OK;
dev->removal_type = ACPI_BUS_REMOVAL_EJECT;
device_release_driver(&dev->dev);
acpi_device_unregister(dev);
- return 0;
+ return AE_OK;
}
void acpi_init_device_object(struct acpi_device *device, acpi_handle handle,
@@ -1646,51 +1649,10 @@ EXPORT_SYMBOL(acpi_bus_add);
int acpi_bus_trim(struct acpi_device *start)
{
- acpi_status status;
- struct acpi_device *parent, *child;
- acpi_handle phandle, chandle;
- acpi_object_type type;
- u32 level = 1;
- int err = 0;
-
- parent = start;
- phandle = start->handle;
- child = chandle = NULL;
-
- while ((level > 0) && parent && (!err)) {
- status = acpi_get_next_object(ACPI_TYPE_ANY, phandle,
- chandle, &chandle);
-
- /*
- * If this scope is exhausted then move our way back up.
- */
- if (ACPI_FAILURE(status)) {
- level--;
- chandle = phandle;
- acpi_get_parent(phandle, &phandle);
- child = parent;
- parent = parent->parent;
- err = acpi_bus_remove(child);
- continue;
- }
-
- status = acpi_get_type(chandle, &type);
- if (ACPI_FAILURE(status)) {
- continue;
- }
- /*
- * If there is a device corresponding to chandle then
- * parse it (depth-first).
- */
- if (acpi_bus_get_device(chandle, &child) == 0) {
- level++;
- phandle = chandle;
- chandle = NULL;
- parent = child;
- }
- continue;
- }
- return err;
+ acpi_walk_namespace(ACPI_TYPE_ANY, start->handle, ACPI_UINT32_MAX, NULL,
+ acpi_bus_remove, NULL, NULL);
+ acpi_bus_remove(start->handle, 0, NULL, NULL);
+ return 0;
}
EXPORT_SYMBOL_GPL(acpi_bus_trim);
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 5/5] ACPI / scan: Add second pass to acpi_bus_trim()
2013-01-14 21:33 [PATCH 0/5] ACPI: Rework acpi_bus_trim() Rafael J. Wysocki
` (3 preceding siblings ...)
2013-01-14 21:38 ` [PATCH 4/5] ACPI / scan: Change the implementation " Rafael J. Wysocki
@ 2013-01-14 21:39 ` Rafael J. Wysocki
2013-01-14 23:13 ` Toshi Kani
2013-01-14 22:21 ` [PATCH 0/5] ACPI: Rework acpi_bus_trim() Yinghai Lu
2013-01-15 0:38 ` Yasuaki Ishimatsu
6 siblings, 1 reply; 11+ messages in thread
From: Rafael J. Wysocki @ 2013-01-14 21:39 UTC (permalink / raw)
To: ACPI Devel Maling List
Cc: Bjorn Helgaas, LKML, Toshi Kani, Yinghai Lu, Jiang Liu,
Taku Izumi
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Make acpi_bus_trim() work in analogy with acpi_bus_scan() and carry
out two passes such that ACPI drivers will be detached from device
nodes being removed in the first pass and the device nodes themselves
will be removed in the second pass.
For this purpose split the driver unregistration out of
acpi_bus_remove() into a new routine, acpi_bus_device_detach(), that
will be executed by acpi_bus_trim() in the additional first pass as
a post-order callback.
This is necessary, because some ACPI drivers' .remove() routines
unregister struct device objects associated with the ACPI device
nodes being removed and that needs to happen while the ACPI
device nodes are still around (for example, in case they need to be
used for power management or similar things at that time).
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/scan.c | 44 ++++++++++++++++++++++++++++----------------
1 file changed, 28 insertions(+), 16 deletions(-)
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -1425,22 +1425,6 @@ static void acpi_device_set_id(struct ac
}
}
-static acpi_status acpi_bus_remove(acpi_handle handle, u32 lvl_not_used,
- void *not_used, void **ret_not_used)
-{
- struct acpi_device *dev = NULL;
-
- if (acpi_bus_get_device(handle, &dev))
- return AE_OK;
-
- dev->removal_type = ACPI_BUS_REMOVAL_EJECT;
- device_release_driver(&dev->dev);
-
- acpi_device_unregister(dev);
-
- return AE_OK;
-}
-
void acpi_init_device_object(struct acpi_device *device, acpi_handle handle,
int type, unsigned long long sta)
{
@@ -1647,8 +1631,36 @@ int acpi_bus_add(acpi_handle handle)
}
EXPORT_SYMBOL(acpi_bus_add);
+static acpi_status acpi_bus_device_detach(acpi_handle handle, u32 lvl_not_used,
+ void *not_used, void **ret_not_used)
+{
+ struct acpi_device *device = NULL;
+
+ if (!acpi_bus_get_device(handle, &device)) {
+ device->removal_type = ACPI_BUS_REMOVAL_EJECT;
+ device_release_driver(&device->dev);
+ }
+ return AE_OK;
+}
+
+static acpi_status acpi_bus_remove(acpi_handle handle, u32 lvl_not_used,
+ void *not_used, void **ret_not_used)
+{
+ struct acpi_device *device = NULL;
+
+ if (!acpi_bus_get_device(handle, &device))
+ acpi_device_unregister(device);
+
+ return AE_OK;
+}
+
int acpi_bus_trim(struct acpi_device *start)
{
+ /* Detach all ACPI drivers from the device nodes being removed. */
+ acpi_walk_namespace(ACPI_TYPE_ANY, start->handle, ACPI_UINT32_MAX, NULL,
+ acpi_bus_device_detach, NULL, NULL);
+ acpi_bus_device_detach(start->handle, 0, NULL, NULL);
+ /* Remove the device nodes. */
acpi_walk_namespace(ACPI_TYPE_ANY, start->handle, ACPI_UINT32_MAX, NULL,
acpi_bus_remove, NULL, NULL);
acpi_bus_remove(start->handle, 0, NULL, NULL);
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/5] ACPI: Rework acpi_bus_trim()
2013-01-14 21:33 [PATCH 0/5] ACPI: Rework acpi_bus_trim() Rafael J. Wysocki
` (4 preceding siblings ...)
2013-01-14 21:39 ` [PATCH 5/5] ACPI / scan: Add second pass to acpi_bus_trim() Rafael J. Wysocki
@ 2013-01-14 22:21 ` Yinghai Lu
2013-01-14 23:51 ` Rafael J. Wysocki
2013-01-15 0:38 ` Yasuaki Ishimatsu
6 siblings, 1 reply; 11+ messages in thread
From: Yinghai Lu @ 2013-01-14 22:21 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: ACPI Devel Maling List, Bjorn Helgaas, LKML, Toshi Kani,
Jiang Liu, Taku Izumi
On Mon, Jan 14, 2013 at 1:33 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> The next three patches actually rework acpi_bus_trim() in three steps:
>
> [3/5] Drop the second argument of acpi_bus_trim() (all callers pass 1 in there
> anyway.
> [4/5] Reimplement acpi_bus_trim() using acpi_walk_namespace().
> [3/5] Make acpi_bus_trim() carry out two passes as described above.
>
> I'm aware of the fact that this will conflict with the patches that Yinghai
> posted a few days ago, but in my opinion the changes here are prerequisite for
> the Yinghai's patchset.
Sure, I will drop my change to acpi_bus_trim() and update one
reference accordingly.
Acked-by: for those 5 patches.
So you will put those patches in acpi-scan, and Bjorn will pull that
again to pci tree?
Thanks
Yinghai
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 5/5] ACPI / scan: Add second pass to acpi_bus_trim()
2013-01-14 21:39 ` [PATCH 5/5] ACPI / scan: Add second pass to acpi_bus_trim() Rafael J. Wysocki
@ 2013-01-14 23:13 ` Toshi Kani
2013-01-14 23:44 ` Rafael J. Wysocki
0 siblings, 1 reply; 11+ messages in thread
From: Toshi Kani @ 2013-01-14 23:13 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: ACPI Devel Maling List, Bjorn Helgaas, LKML, Yinghai Lu,
Jiang Liu, Taku Izumi
On Mon, 2013-01-14 at 22:39 +0100, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> Make acpi_bus_trim() work in analogy with acpi_bus_scan() and carry
> out two passes such that ACPI drivers will be detached from device
> nodes being removed in the first pass and the device nodes themselves
> will be removed in the second pass.
>
> For this purpose split the driver unregistration out of
> acpi_bus_remove() into a new routine, acpi_bus_device_detach(), that
> will be executed by acpi_bus_trim() in the additional first pass as
> a post-order callback.
>
> This is necessary, because some ACPI drivers' .remove() routines
> unregister struct device objects associated with the ACPI device
> nodes being removed and that needs to happen while the ACPI
> device nodes are still around (for example, in case they need to be
> used for power management or similar things at that time).
>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
I'd suggest to add some comment stating that acpi_bus_device_detach()
acpi_bus_remove() are post-order callbacks. I do not think many people
recognize it by just looking the calls to acpi_walk_namespace().
Otherwise, the patchset looks good. For the patch series:
Acked-by: Toshi Kani <toshi.kani@hp.com>
Thanks,
-Toshi
> ---
> drivers/acpi/scan.c | 44 ++++++++++++++++++++++++++++----------------
> 1 file changed, 28 insertions(+), 16 deletions(-)
>
> Index: linux-pm/drivers/acpi/scan.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/scan.c
> +++ linux-pm/drivers/acpi/scan.c
> @@ -1425,22 +1425,6 @@ static void acpi_device_set_id(struct ac
> }
> }
>
> -static acpi_status acpi_bus_remove(acpi_handle handle, u32 lvl_not_used,
> - void *not_used, void **ret_not_used)
> -{
> - struct acpi_device *dev = NULL;
> -
> - if (acpi_bus_get_device(handle, &dev))
> - return AE_OK;
> -
> - dev->removal_type = ACPI_BUS_REMOVAL_EJECT;
> - device_release_driver(&dev->dev);
> -
> - acpi_device_unregister(dev);
> -
> - return AE_OK;
> -}
> -
> void acpi_init_device_object(struct acpi_device *device, acpi_handle handle,
> int type, unsigned long long sta)
> {
> @@ -1647,8 +1631,36 @@ int acpi_bus_add(acpi_handle handle)
> }
> EXPORT_SYMBOL(acpi_bus_add);
>
> +static acpi_status acpi_bus_device_detach(acpi_handle handle, u32 lvl_not_used,
> + void *not_used, void **ret_not_used)
> +{
> + struct acpi_device *device = NULL;
> +
> + if (!acpi_bus_get_device(handle, &device)) {
> + device->removal_type = ACPI_BUS_REMOVAL_EJECT;
> + device_release_driver(&device->dev);
> + }
> + return AE_OK;
> +}
> +
> +static acpi_status acpi_bus_remove(acpi_handle handle, u32 lvl_not_used,
> + void *not_used, void **ret_not_used)
> +{
> + struct acpi_device *device = NULL;
> +
> + if (!acpi_bus_get_device(handle, &device))
> + acpi_device_unregister(device);
> +
> + return AE_OK;
> +}
> +
> int acpi_bus_trim(struct acpi_device *start)
> {
> + /* Detach all ACPI drivers from the device nodes being removed. */
> + acpi_walk_namespace(ACPI_TYPE_ANY, start->handle, ACPI_UINT32_MAX, NULL,
> + acpi_bus_device_detach, NULL, NULL);
> + acpi_bus_device_detach(start->handle, 0, NULL, NULL);
> + /* Remove the device nodes. */
> acpi_walk_namespace(ACPI_TYPE_ANY, start->handle, ACPI_UINT32_MAX, NULL,
> acpi_bus_remove, NULL, NULL);
> acpi_bus_remove(start->handle, 0, NULL, NULL);
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 5/5] ACPI / scan: Add second pass to acpi_bus_trim()
2013-01-14 23:13 ` Toshi Kani
@ 2013-01-14 23:44 ` Rafael J. Wysocki
0 siblings, 0 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2013-01-14 23:44 UTC (permalink / raw)
To: Toshi Kani
Cc: ACPI Devel Maling List, Bjorn Helgaas, LKML, Yinghai Lu,
Jiang Liu, Taku Izumi
On Monday, January 14, 2013 04:13:43 PM Toshi Kani wrote:
> On Mon, 2013-01-14 at 22:39 +0100, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > Make acpi_bus_trim() work in analogy with acpi_bus_scan() and carry
> > out two passes such that ACPI drivers will be detached from device
> > nodes being removed in the first pass and the device nodes themselves
> > will be removed in the second pass.
> >
> > For this purpose split the driver unregistration out of
> > acpi_bus_remove() into a new routine, acpi_bus_device_detach(), that
> > will be executed by acpi_bus_trim() in the additional first pass as
> > a post-order callback.
> >
> > This is necessary, because some ACPI drivers' .remove() routines
> > unregister struct device objects associated with the ACPI device
> > nodes being removed and that needs to happen while the ACPI
> > device nodes are still around (for example, in case they need to be
> > used for power management or similar things at that time).
> >
> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> I'd suggest to add some comment stating that acpi_bus_device_detach()
> acpi_bus_remove() are post-order callbacks. I do not think many people
> recognize it by just looking the calls to acpi_walk_namespace().
Yeah, good idea.
> Otherwise, the patchset looks good. For the patch series:
> Acked-by: Toshi Kani <toshi.kani@hp.com>
Thanks!
Rafael
> > ---
> > drivers/acpi/scan.c | 44 ++++++++++++++++++++++++++++----------------
> > 1 file changed, 28 insertions(+), 16 deletions(-)
> >
> > Index: linux-pm/drivers/acpi/scan.c
> > ===================================================================
> > --- linux-pm.orig/drivers/acpi/scan.c
> > +++ linux-pm/drivers/acpi/scan.c
> > @@ -1425,22 +1425,6 @@ static void acpi_device_set_id(struct ac
> > }
> > }
> >
> > -static acpi_status acpi_bus_remove(acpi_handle handle, u32 lvl_not_used,
> > - void *not_used, void **ret_not_used)
> > -{
> > - struct acpi_device *dev = NULL;
> > -
> > - if (acpi_bus_get_device(handle, &dev))
> > - return AE_OK;
> > -
> > - dev->removal_type = ACPI_BUS_REMOVAL_EJECT;
> > - device_release_driver(&dev->dev);
> > -
> > - acpi_device_unregister(dev);
> > -
> > - return AE_OK;
> > -}
> > -
> > void acpi_init_device_object(struct acpi_device *device, acpi_handle handle,
> > int type, unsigned long long sta)
> > {
> > @@ -1647,8 +1631,36 @@ int acpi_bus_add(acpi_handle handle)
> > }
> > EXPORT_SYMBOL(acpi_bus_add);
> >
> > +static acpi_status acpi_bus_device_detach(acpi_handle handle, u32 lvl_not_used,
> > + void *not_used, void **ret_not_used)
> > +{
> > + struct acpi_device *device = NULL;
> > +
> > + if (!acpi_bus_get_device(handle, &device)) {
> > + device->removal_type = ACPI_BUS_REMOVAL_EJECT;
> > + device_release_driver(&device->dev);
> > + }
> > + return AE_OK;
> > +}
> > +
> > +static acpi_status acpi_bus_remove(acpi_handle handle, u32 lvl_not_used,
> > + void *not_used, void **ret_not_used)
> > +{
> > + struct acpi_device *device = NULL;
> > +
> > + if (!acpi_bus_get_device(handle, &device))
> > + acpi_device_unregister(device);
> > +
> > + return AE_OK;
> > +}
> > +
> > int acpi_bus_trim(struct acpi_device *start)
> > {
> > + /* Detach all ACPI drivers from the device nodes being removed. */
> > + acpi_walk_namespace(ACPI_TYPE_ANY, start->handle, ACPI_UINT32_MAX, NULL,
> > + acpi_bus_device_detach, NULL, NULL);
> > + acpi_bus_device_detach(start->handle, 0, NULL, NULL);
> > + /* Remove the device nodes. */
> > acpi_walk_namespace(ACPI_TYPE_ANY, start->handle, ACPI_UINT32_MAX, NULL,
> > acpi_bus_remove, NULL, NULL);
> > acpi_bus_remove(start->handle, 0, NULL, NULL);
> >
>
>
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/5] ACPI: Rework acpi_bus_trim()
2013-01-14 22:21 ` [PATCH 0/5] ACPI: Rework acpi_bus_trim() Yinghai Lu
@ 2013-01-14 23:51 ` Rafael J. Wysocki
0 siblings, 0 replies; 11+ messages in thread
From: Rafael J. Wysocki @ 2013-01-14 23:51 UTC (permalink / raw)
To: Yinghai Lu
Cc: ACPI Devel Maling List, Bjorn Helgaas, LKML, Toshi Kani,
Jiang Liu, Taku Izumi
On Monday, January 14, 2013 02:21:38 PM Yinghai Lu wrote:
> On Mon, Jan 14, 2013 at 1:33 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote:
> > The next three patches actually rework acpi_bus_trim() in three steps:
> >
> > [3/5] Drop the second argument of acpi_bus_trim() (all callers pass 1 in there
> > anyway.
> > [4/5] Reimplement acpi_bus_trim() using acpi_walk_namespace().
> > [3/5] Make acpi_bus_trim() carry out two passes as described above.
> >
> > I'm aware of the fact that this will conflict with the patches that Yinghai
> > posted a few days ago, but in my opinion the changes here are prerequisite for
> > the Yinghai's patchset.
>
> Sure, I will drop my change to acpi_bus_trim() and update one
> reference accordingly.
>
> Acked-by: for those 5 patches.
Thanks!
> So you will put those patches in acpi-scan, and Bjorn will pull that
> again to pci tree?
I'm going to put them into acpi-scan some time later this week, but I'd like
them to go through linux-next first to catch teething problems, if any.
I can't speak for Bjorn, though. I suppose he will pull them if you ask him to.
Thanks,
Rafael
--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/5] ACPI: Rework acpi_bus_trim()
2013-01-14 21:33 [PATCH 0/5] ACPI: Rework acpi_bus_trim() Rafael J. Wysocki
` (5 preceding siblings ...)
2013-01-14 22:21 ` [PATCH 0/5] ACPI: Rework acpi_bus_trim() Yinghai Lu
@ 2013-01-15 0:38 ` Yasuaki Ishimatsu
6 siblings, 0 replies; 11+ messages in thread
From: Yasuaki Ishimatsu @ 2013-01-15 0:38 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: ACPI Devel Maling List, Bjorn Helgaas, LKML, Toshi Kani,
Yinghai Lu, Jiang Liu, Taku Izumi
2013/01/15 6:33, Rafael J. Wysocki wrote:
> Hi All,
>
> This series of patches changes the way acpi_bus_trim() works so that,
> eventually, it walks the namespace twice detaching (ACPI) drivers from
> device nodes being removed in the first pass and removing the device
> nodes in the second pass.
>
> The first two patches are just cleanups removing unused stuff:
>
> [1/5] Remove the ops field from struct acpi_device (unrelated to the rest).
> [2/5] Drop the second argument of acpi_device_unregister().
>
> The next three patches actually rework acpi_bus_trim() in three steps:
>
> [3/5] Drop the second argument of acpi_bus_trim() (all callers pass 1 in there
> anyway.
> [4/5] Reimplement acpi_bus_trim() using acpi_walk_namespace().
> [3/5] Make acpi_bus_trim() carry out two passes as described above.
Acked-by: Yasuaki Ishimatsu <isimatu.yasuaki@jp.fujitsu.com>
>
> I'm aware of the fact that this will conflict with the patches that Yinghai
> posted a few days ago, but in my opinion the changes here are prerequisite for
> the Yinghai's patchset.
>
> Thanks,
> Rafael
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2013-01-15 0:38 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-14 21:33 [PATCH 0/5] ACPI: Rework acpi_bus_trim() Rafael J. Wysocki
2013-01-14 21:37 ` [PATCH 1/5] ACPI: Remove the ops field from struct acpi_device Rafael J. Wysocki
2013-01-14 21:37 ` [PATCH 2/5] ACPI / scan: Drop the second argument of acpi_device_unregister() Rafael J. Wysocki
2013-01-14 21:37 ` [PATCH 3/5] ACPI / scan: Drop the second argument of acpi_bus_trim() Rafael J. Wysocki
2013-01-14 21:38 ` [PATCH 4/5] ACPI / scan: Change the implementation " Rafael J. Wysocki
2013-01-14 21:39 ` [PATCH 5/5] ACPI / scan: Add second pass to acpi_bus_trim() Rafael J. Wysocki
2013-01-14 23:13 ` Toshi Kani
2013-01-14 23:44 ` Rafael J. Wysocki
2013-01-14 22:21 ` [PATCH 0/5] ACPI: Rework acpi_bus_trim() Yinghai Lu
2013-01-14 23:51 ` Rafael J. Wysocki
2013-01-15 0:38 ` Yasuaki Ishimatsu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).