On Thursday 23 February 2006 01:03, Thomas Renninger wrote:
> That is how it should be:
> acpi_memoryhotplug.c invokes register_driver in it's init func.
> There the ACPI device matching HID is searched the device pointer passed
> to the driver and memoryhotplug's .add function invoked.
> There (at .add func of memhotplug) the register_notify_handler should be
> placed ...
> if I see things right?
>
> This is how it is currently done in memhotplug:
> Driver_register is invoked. There the HID is searched and device pointer
> passed
> if a memory hotplug device is found.
> Then a second search for the same HID (by walk_namespace) is done in
> memhotplug
> and the notify handler is registered for those.
>
> I couldn't find an author for acpi_memoryhotplug.c, shouldn't there someone be
> added who working most on this stuff, I have no idea whom to add into CC for
> that...
> Could someone please review that, the register_notify_handler stuff there
> looks
> somewhat insane and if I see things correctly a call like:
> acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, ...)
> could be costy as it runs through all the namespace (some hundreds/thousands
> of objects?)
> and searches for a matching _HID. But I might have overseen something and
> this is really needed...
>
> Kristen: Maybe you should make the docking driver an ACPI device driver
> with register_driver in it's init func and an .add func where the notify
> handler is registered.
>
> BTW: How is this memory hotplugging supposed to work. It only reacts on ACPI
> notifies, my machine
> only has "Check" notify events for mem. What kind of hardware do I need to
> test this and
> how will the eject request be initiated?
>
This one is better...
Subject: acpi_memoryhotplug.c needs not to walk whole namespace twice
Do not walk through whole namespace to find the memhotplug device. It
is already found by register_driver, .add is invoked if register_driver
finds a device, register the notify handler there.
signed-off-by: Thomas Renninger <[EMAIL PROTECTED]>
drivers/acpi/acpi_memhotplug.c | 103 +++--------------------------------------
1 files changed, 8 insertions(+), 95 deletions(-)
Index: linux-2.6.15/drivers/acpi/acpi_memhotplug.c
===================================================================
--- linux-2.6.15.orig/drivers/acpi/acpi_memhotplug.c
+++ linux-2.6.15/drivers/acpi/acpi_memhotplug.c
@@ -394,6 +394,10 @@ static int acpi_memory_device_add(struct
/* Set the device state */
mem_device->state = MEMORY_POWER_ON_STATE;
+ /* register notify handler, ignore error if there could be any */
+ acpi_install_notify_handler(device->handle, ACPI_SYSTEM_NOTIFY,
+ acpi_memory_device_notify, NULL);
+
printk(KERN_INFO "%s \n", acpi_device_name(device));
return_VALUE(result);
@@ -408,84 +412,16 @@ static int acpi_memory_device_remove(str
if (!device || !acpi_driver_data(device))
return_VALUE(-EINVAL);
+ acpi_remove_notify_handler(device->handle,
+ ACPI_SYSTEM_NOTIFY,
+ acpi_memory_device_notify);
+
mem_device = (struct acpi_memory_device *)acpi_driver_data(device);
kfree(mem_device);
return_VALUE(0);
}
-/*
- * Helper function to check for memory device
- */
-static acpi_status is_memory_device(acpi_handle handle)
-{
- char *hardware_id;
- acpi_status status;
- struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
- struct acpi_device_info *info;
-
- ACPI_FUNCTION_TRACE("is_memory_device");
-
- status = acpi_get_object_info(handle, &buffer);
- if (ACPI_FAILURE(status))
- return_ACPI_STATUS(status);
-
- info = buffer.pointer;
- if (!(info->valid & ACPI_VALID_HID)) {
- acpi_os_free(buffer.pointer);
- return_ACPI_STATUS(AE_ERROR);
- }
-
- hardware_id = info->hardware_id.value;
- if ((hardware_id == NULL) ||
- (strcmp(hardware_id, ACPI_MEMORY_DEVICE_HID)))
- status = AE_ERROR;
-
- acpi_os_free(buffer.pointer);
- return_ACPI_STATUS(status);
-}
-
-static acpi_status
-acpi_memory_register_notify_handler(acpi_handle handle,
- u32 level, void *ctxt, void **retv)
-{
- acpi_status status;
-
- ACPI_FUNCTION_TRACE("acpi_memory_register_notify_handler");
-
- status = is_memory_device(handle);
- if (ACPI_FAILURE(status)){
- ACPI_EXCEPTION((AE_INFO, status, "handle is no memory device"));
- return_ACPI_STATUS(AE_OK); /* continue */
- }
-
- status = acpi_install_notify_handler(handle, ACPI_SYSTEM_NOTIFY,
- acpi_memory_device_notify, NULL);
- /* continue */
- return_ACPI_STATUS(AE_OK);
-}
-
-static acpi_status
-acpi_memory_deregister_notify_handler(acpi_handle handle,
- u32 level, void *ctxt, void **retv)
-{
- acpi_status status;
-
- ACPI_FUNCTION_TRACE("acpi_memory_deregister_notify_handler");
-
- status = is_memory_device(handle);
- if (ACPI_FAILURE(status)){
- ACPI_EXCEPTION((AE_INFO, status, "handle is no memory device"));
- return_ACPI_STATUS(AE_OK); /* continue */
- }
-
- status = acpi_remove_notify_handler(handle,
- ACPI_SYSTEM_NOTIFY,
- acpi_memory_device_notify);
-
- return_ACPI_STATUS(AE_OK); /* continue */
-}
-
static int __init acpi_memory_device_init(void)
{
int result;
@@ -498,17 +434,6 @@ static int __init acpi_memory_device_ini
if (result < 0)
return_VALUE(-ENODEV);
- status = acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
- ACPI_UINT32_MAX,
- acpi_memory_register_notify_handler,
- NULL, NULL);
-
- if (ACPI_FAILURE(status)) {
- ACPI_EXCEPTION((AE_INFO, status, "walk_namespace failed"));
- acpi_bus_unregister_driver(&acpi_memory_device_driver);
- return_VALUE(-ENODEV);
- }
-
return_VALUE(0);
}
@@ -518,18 +443,6 @@ static void __exit acpi_memory_device_ex
ACPI_FUNCTION_TRACE("acpi_memory_device_exit");
- /*
- * Adding this to un-install notification handlers for all the device
- * handles.
- */
- status = acpi_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT,
- ACPI_UINT32_MAX,
- acpi_memory_deregister_notify_handler,
- NULL, NULL);
-
- if (ACPI_FAILURE(status))
- ACPI_EXCEPTION((AE_INFO, status, "walk_namespace failed"));
-
acpi_bus_unregister_driver(&acpi_memory_device_driver);
return_VOID;
-
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
