From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Renninger Subject: [PATCH] acpi_memoryhotplug.c needs not to walk whole namespace twice to register notify handler Date: Thu, 23 Feb 2006 01:03:15 +0100 Message-ID: <200602230103.15673.trenn@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from ns.suse.de ([195.135.220.2]:28351 "EHLO mx1.suse.de") by vger.kernel.org with ESMTP id S1750899AbWBWADW (ORCPT ); Wed, 22 Feb 2006 19:03:22 -0500 Content-Disposition: inline Sender: linux-acpi-owner@vger.kernel.org List-Id: linux-acpi@vger.kernel.org To: linux-acpi@vger.kernel.org, kamezawa.hiroyu@jp.fujitsu.com, "Yu, Luming" , Kristen Accardi 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? 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 drivers/acpi/acpi_memhotplug.c | 103 +++-------------------------------------- 1 files changed, 8 insertions(+), 95 deletions(-) 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 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 */ + status = 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); + status = 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;