| Message ID | 9393976.KUs5xrSxYx@vostro.rjw.lan (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
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); > -- 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
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); > > > >
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);