ACPICA modifications
Subsystems (2)
cover ACPICA modifications
Hi,
[1/3] is at the Bob's request, [2/3] is a concurrency fix and [3/3] reduces code duplication slightly.
Please tell me what you think.
Rafael
RFC PATCH 1/3 ACPI / ACPICA: Multiple system notify handlers for the root object
Following commit 3f0be67188c60ebf1b5d00354b44b4b24f5af313 (ACPI / ACPICA: Multiple system notify handlers per device) make it possible to install multiple system notify handlers for the root object.
--- drivers/acpi/acpica/evmisc.c | 31 +++++------- drivers/acpi/acpica/evxface.c | 103 ++++++++++++++++++++++++++++++++++-------- 2 files changed, 98 insertions(+), 36 deletions(-)
drivers/acpi/acpica/evxface.c +85 -18
@@ -225,6 +225,7 @@ ACPI_EXPORT_SYMBOL(acpi_remove_fixed_eve* ACPI_SYSTEM_NOTIFY: system_handler (00-7f) * ACPI_DEVICE_NOTIFY: driver_handler (80-ff) * ACPI_ALL_NOTIFY: both system and device +* node - Device the notifies will be handled for * handler - Address of the handler * context - Value passed to the handler on each GPE * next - Address of a handler object to link to
@@ -237,10 +238,12 @@ ACPI_EXPORT_SYMBOL(acpi_remove_fixed_evestatic void acpi_populate_handler_object (struct acpi_object_notify_handler *handler_obj,u32 handler_type, +struct acpi_namespace_node *node,acpi_notify_handler handler,void *context,struct acpi_object_notify_handler *next) { handler_obj->handler_type = handler_type; + handler_obj->node = node; handler_obj->handler = handler; handler_obj->context = context; handler_obj->next = next;
@@ -251,6 +254,7 @@ acpi_populate_handler_object(struct acpi* FUNCTION: acpi_add_handler_object * * PARAMETERS: parent_obj - Parent of the new object +* node - Device the notifies will be handled for * handler - Address of the handler * context - Value passed to the handler on each GPE *
@@ -261,6 +265,7 @@ acpi_populate_handler_object(struct acpi******************************************************************************/ static acpi_status acpi_add_handler_object (struct acpi_object_notify_handler *parent_obj, +struct acpi_namespace_node *node,acpi_notify_handler handler,void *context) {struct acpi_object_notify_handler *handler_obj;
@@ -275,6 +280,7 @@ acpi_add_handler_object(struct acpi_objeacpi_populate_handler_object (handler_obj,ACPI_SYSTEM_NOTIFY , + node, handler, context, parent_obj->next); parent_obj->next = handler_obj;
@@ -339,28 +345,43 @@ acpi_install_notify_handler(acpi_handle*/ if (device ==ACPI_ROOT_OBJECT ) { -/* Make sure the handler is not already installed */ - -if (((handler_type &ACPI_SYSTEM_NOTIFY ) && - acpi_gbl_system_notify.handler) || - ((handler_type &ACPI_DEVICE_NOTIFY ) && - acpi_gbl_device_notify.handler)) { +/* For a device notify, make sure there's no handler. */ +if ((handler_type &ACPI_DEVICE_NOTIFY ) && + acpi_gbl_device_notify.handler) { status =AE_ALREADY_EXISTS ; goto unlock_and_exit; } -if (handler_type &ACPI_SYSTEM_NOTIFY ) { - acpi_gbl_system_notify.node = node; - acpi_gbl_system_notify.handler = handler; - acpi_gbl_system_notify.context = context; - } +/* System notifies may have more handlers installed. */ +if ((handler_type &ACPI_SYSTEM_NOTIFY ) && + acpi_gbl_device_notify.handler) { +if (handler_type &ACPI_DEVICE_NOTIFY ) { + status =AE_ALREADY_EXISTS ; + goto unlock_and_exit; + } -if (handler_type &ACPI_DEVICE_NOTIFY ) { - acpi_gbl_device_notify.node = node; - acpi_gbl_device_notify.handler = handler; - acpi_gbl_device_notify.context = context; + status =acpi_add_handler_object ( + &acpi_gbl_device_notify, + node, + handler, + context); + goto unlock_and_exit; } +if (handler_type &ACPI_SYSTEM_NOTIFY ) +acpi_populate_handler_object (&acpi_gbl_system_notify, + handler_type, + node, + handler, context, +NULL ); + +if (handler_type &ACPI_DEVICE_NOTIFY ) +acpi_populate_handler_object (&acpi_gbl_device_notify, + handler_type, + node, + handler, context, +NULL ); +/* Global notify handler installed */ }
@@ -404,6 +425,7 @@ acpi_install_notify_handler(acpi_handle parent_obj = ¬ify_obj->notify; status =acpi_add_handler_object (parent_obj, + node, handler, context); goto unlock_and_exit;
@@ -441,6 +463,7 @@ acpi_install_notify_handler(acpi_handleacpi_populate_handler_object (¬ify_obj->notify, handler_type, + node, handler, context,NULL );
@@ -534,9 +557,53 @@ acpi_remove_notify_handler(acpi_handle d }if (handler_type &ACPI_SYSTEM_NOTIFY ) { - acpi_gbl_system_notify.node =NULL ; - acpi_gbl_system_notify.handler =NULL ; - acpi_gbl_system_notify.context =NULL ; +struct acpi_object_notify_handler *handler_obj; +struct acpi_object_notify_handler *parent_obj; + + handler_obj = &acpi_gbl_system_notify; + parent_obj =NULL ; +while (handler_obj->handler != handler) { +if (handler_obj->next) { + parent_obj = handler_obj; + handler_obj = handler_obj->next; + }else { +break ; + } + } + +if (handler_obj->handler != handler) { + status =AE_BAD_PARAMETER ; + goto unlock_and_exit; + } + +/* +* Remove the handler. There are three possible cases. +* First, we may need to remove a nonstatic object. +* Second, we may need to remove the static object's +* handler data, while nonstatic objects exist. +* Finally, we may need to clear the static object's +* fields. +*/ +if (parent_obj) { +/* Nonstatic object is being removed. */ + parent_obj->next = handler_obj->next; +ACPI_FREE (handler_obj); + }else if (acpi_gbl_system_notify.next) { +/* +* The handler matches the static object, but +* there are more handler objects in the list. +* Replace the static object's data with the +* first next object's data and remove that +* object. +*/ + handler_obj = acpi_gbl_system_notify.next; + acpi_gbl_system_notify = *handler_obj; +ACPI_FREE (handler_obj); + }else { + acpi_gbl_system_notify.node =NULL ; + acpi_gbl_system_notify.handler =NULL ; + acpi_gbl_system_notify.context =NULL ; + } }if (handler_type &ACPI_DEVICE_NOTIFY ) {
drivers/acpi/acpica/evmisc.c +13 -18
@@ -221,9 +221,8 @@ static void ACPI_SYSTEM_XFACE acpi_ev_no {union acpi_generic_state *notify_info = (union acpi_generic_state *)context; -acpi_notify_handler global_handler =NULL ; -void *global_context =NULL ;union acpi_operand_object *handler_obj; +struct acpi_object_notify_handler *notifier =NULL ;ACPI_FUNCTION_ENTRY ();
@@ -233,34 +232,30 @@ static void ACPI_SYSTEM_XFACE acpi_ev_no*/ if (notify_info->notify.value <=ACPI_MAX_SYS_NOTIFY ) { -/* Global system notification handler */ +/* Global system notification */ -if (acpi_gbl_system_notify.handler) { - global_handler = acpi_gbl_system_notify.handler; - global_context = acpi_gbl_system_notify.context; - } +if (acpi_gbl_system_notify.handler) + notifier = &acpi_gbl_system_notify; }else { -/* Global driver notification handler */ +/* Global driver notification */ -if (acpi_gbl_device_notify.handler) { - global_handler = acpi_gbl_device_notify.handler; - global_context = acpi_gbl_device_notify.context; - } +if (acpi_gbl_device_notify.handler) + notifier = &acpi_gbl_device_notify; } -/* Invoke the system handler first, if present */ +/* Invoke the system handlers first, if present */ -if (global_handler) { -global_handler (notify_info->notify.node, - notify_info->notify.value, global_context); +while (notifier) { + notifier->handler (notify_info->notify.node, + notify_info->notify.value, + notifier->context); + notifier = notifier->next; }/* Now invoke the per-device handler, if present */ handler_obj = notify_info->notify.handler_obj;if (handler_obj) { -struct acpi_object_notify_handler *notifier; - notifier = &handler_obj->notify;while (notifier) { notifier->handler(notify_info->notify.node,
RFC PATCH 2/3 ACPI / ACPICA: Fix locking in acpi_remove_gpe_handler()
In reply to Rafael J. WysockiThe releasing of ACPI_MTX_EVENTS in the middle of acpi_remove_gpe_handler() in order to call acpi_os_wait_events_complete() is a mistake, because it creates a window for undesirable concurrent changes to be made while acpi_os_wait_events_complete() is being executed. At the same time it doesn't make any difference to call acpi_os_wait_events_complete() before acquiring ACPI_MTX_EVENTS for the first time, which allows us to avoid the race condition.
--- drivers/acpi/acpica/evxface.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-)
drivers/acpi/acpica/evxface.c +4 -9
@@ -842,6 +842,10 @@ acpi_remove_gpe_handler(acpi_handle gpe_return_ACPI_STATUS (AE_BAD_PARAMETER ); } +/* Make sure all deferred tasks are completed */ + +acpi_os_wait_events_complete (NULL ); + status =acpi_ut_acquire_mutex (ACPI_MTX_EVENTS );if (ACPI_FAILURE (status)) {return_ACPI_STATUS (status);
@@ -870,15 +874,6 @@ acpi_remove_gpe_handler(acpi_handle gpe_ goto unlock_and_exit; } -/* Make sure all deferred tasks are completed */ - - (void )acpi_ut_release_mutex (ACPI_MTX_EVENTS ); -acpi_os_wait_events_complete (NULL ); - status =acpi_ut_acquire_mutex (ACPI_MTX_EVENTS ); -if (ACPI_FAILURE (status)) { -return_ACPI_STATUS (status); - } -/* Remove the handler */ flags =acpi_os_acquire_lock (acpi_gbl_gpe_lock);
RFC PATCH 3/3 ACPI / ACPICA: Reduce code duplication in acpi_ev_get_gpe_event_info()
In reply to Rafael J. WysockiMove duplicated code from acpi_ev_get_gpe_event_info(), which is also quite difficult to read, to a separate static function.
--- drivers/acpi/acpica/evgpe.c | 56 +++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 23 deletions(-)
drivers/acpi/acpica/evgpe.c +33 -23
@@ -168,6 +168,33 @@ acpi_status acpi_ev_disable_gpe(struct areturn_ACPI_STATUS (status); } + +/******************************************************************************* +* +* FUNCTION: acpi_ev_get_info +* +* PARAMETERS: gpe_block - GPE block to search +* gpe_number - Raw GPE number +* +* RETURN: A GPE event_info struct if found. NULL if not found +* +******************************************************************************/ + +static struct acpi_gpe_event_info *acpi_ev_get_info ( +struct acpi_gpe_block_info *gpe_block, +u32 gpe_number) +{ +u32 gpe_index, gpe_count; + +if (!gpe_block || gpe_number < gpe_block->block_base_number) +return (NULL ); + + gpe_index = gpe_number - gpe_block->block_base_number; + gpe_count = gpe_block->register_count *ACPI_GPE_REGISTER_WIDTH ; +return gpe_index < gpe_count ? + (&gpe_block->event_info[gpe_index]) : (NULL ); +} +/******************************************************************************* * * FUNCTION: acpi_ev_get_gpe_event_info
@@ -189,7 +216,7 @@ struct acpi_gpe_event_info *acpi_ev_get_ u32 gpe_number) {union acpi_operand_object *obj_desc; -struct acpi_gpe_block_info *gpe_block; +struct acpi_gpe_event_info *gpe_info;u32 i;ACPI_FUNCTION_ENTRY ();
@@ -201,18 +228,10 @@ struct acpi_gpe_event_info *acpi_ev_get_/* Examine GPE Block 0 and 1 (These blocks are permanent) */ for (i =0 ; i <ACPI_MAX_GPE_BLOCKS ; i++) { - gpe_block = acpi_gbl_gpe_fadt_blocks[i]; -if (gpe_block) { -if ((gpe_number >= gpe_block->block_base_number) - && (gpe_number < - gpe_block->block_base_number + - (gpe_block->register_count *8 ))) { -return (&gpe_block-> - event_info[gpe_number - - gpe_block-> - block_base_number]); - } - } + gpe_info =acpi_ev_get_info (acpi_gbl_gpe_fadt_blocks[i], + gpe_number); +if (gpe_info) +return (gpe_info); }/* The gpe_number was not in the range of either FADT GPE block */
@@ -228,16 +247,7 @@ struct acpi_gpe_event_info *acpi_ev_get_return (NULL ); } - gpe_block = obj_desc->device.gpe_block; - -if ((gpe_number >= gpe_block->block_base_number) && - (gpe_number < - gpe_block->block_base_number + (gpe_block->register_count *8 ))) { -return (&gpe_block-> - event_info[gpe_number - gpe_block->block_base_number]); - } - -return (NULL ); +return acpi_ev_get_info (obj_desc->device.gpe_block, gpe_number); }/*******************************************************************************
RFC PATCH 3/3 ACPI / ACPICA: Reduce code duplication related to GPE lookup
From: Rafael J. Wysocki <[email protected]>
There is some code duplication related to the looking up a GPE in a GPE block. Namely, the same code construct, which is quite difficult to read and inefficient, is used in three different places (two times in acpi_ev_get_gpe_event_info() and once in acpi_ev_match_prw_and_gpe()). To reduce the code duplication move it to a separate function and clean it up.
---
As you noticed, the same piece of code is actually used in one more place, so it's possible to reduce the code duplication even more.
Appended is updated patch doing that.
Thanks,
Rafael
--- drivers/acpi/acpica/acevents.h | 4 ++ drivers/acpi/acpica/evgpe.c | 58 ++++++++++++++++++++++++----------------- drivers/acpi/acpica/evgpeblk.c | 14 +++------ 3 files changed, 44 insertions(+), 32 deletions(-)
drivers/acpi/acpica/evgpe.c +35 -23
@@ -168,6 +168,33 @@ acpi_status acpi_ev_disable_gpe(struct areturn_ACPI_STATUS (status); } + +/******************************************************************************* +* +* FUNCTION: acpi_ev_gpeblk_event_info +* +* PARAMETERS: gpe_block - GPE block to search +* gpe_number - Raw GPE number +* +* RETURN: A GPE event_info struct if found. NULL if not found +* +******************************************************************************/ + +struct acpi_gpe_event_info *acpi_ev_gpeblk_event_info ( +struct acpi_gpe_block_info *gpe_block, +u32 gpe_number) +{ +u32 gpe_index, gpe_count; + +if (!gpe_block || gpe_number < gpe_block->block_base_number) +return (NULL ); + + gpe_index = gpe_number - gpe_block->block_base_number; + gpe_count = gpe_block->register_count *ACPI_GPE_REGISTER_WIDTH ; +return gpe_index < gpe_count ? + (&gpe_block->event_info[gpe_index]) : (NULL ); +} +/******************************************************************************* * * FUNCTION: acpi_ev_get_gpe_event_info
@@ -189,7 +216,7 @@ struct acpi_gpe_event_info *acpi_ev_get_ u32 gpe_number) {union acpi_operand_object *obj_desc; -struct acpi_gpe_block_info *gpe_block; +struct acpi_gpe_event_info *gpe_info;u32 i;ACPI_FUNCTION_ENTRY ();
@@ -201,18 +228,11 @@ struct acpi_gpe_event_info *acpi_ev_get_/* Examine GPE Block 0 and 1 (These blocks are permanent) */ for (i =0 ; i <ACPI_MAX_GPE_BLOCKS ; i++) { - gpe_block = acpi_gbl_gpe_fadt_blocks[i]; -if (gpe_block) { -if ((gpe_number >= gpe_block->block_base_number) - && (gpe_number < - gpe_block->block_base_number + - (gpe_block->register_count *8 ))) { -return (&gpe_block-> - event_info[gpe_number - - gpe_block-> - block_base_number]); - } - } + gpe_info =acpi_ev_gpeblk_event_info ( + acpi_gbl_gpe_fadt_blocks[i], + gpe_number); +if (gpe_info) +return (gpe_info); }/* The gpe_number was not in the range of either FADT GPE block */
@@ -228,16 +248,8 @@ struct acpi_gpe_event_info *acpi_ev_get_return (NULL ); } - gpe_block = obj_desc->device.gpe_block; - -if ((gpe_number >= gpe_block->block_base_number) && - (gpe_number < - gpe_block->block_base_number + (gpe_block->register_count *8 ))) { -return (&gpe_block-> - event_info[gpe_number - gpe_block->block_base_number]); - } - -return (NULL ); +return acpi_ev_gpeblk_event_info (obj_desc->device.gpe_block, + gpe_number); }/*******************************************************************************
drivers/acpi/acpica/acevents.h +4 -0
@@ -82,6 +82,10 @@ acpi_status acpi_ev_enable_gpe(struct acacpi_status acpi_ev_disable_gpe (struct acpi_gpe_event_info *gpe_event_info); +struct acpi_gpe_event_info *acpi_ev_gpeblk_event_info ( +struct acpi_gpe_block_info *gpe_block, +u32 gpe_number); +struct acpi_gpe_event_info *acpi_ev_get_gpe_event_info (acpi_handle gpe_device,u32 gpe_number);
drivers/acpi/acpica/evgpeblk.c +5 -9
@@ -439,15 +439,11 @@ acpi_ev_match_prw_and_gpe(acpi_handle ob* 2) The GPE index(number) is within the range of the Gpe Block * associated with the GPE device. */ -if ((gpe_device == target_gpe_device) && - (gpe_number >= gpe_block->block_base_number) && - (gpe_number < gpe_block->block_base_number + - (gpe_block->register_count *8 ))) { - gpe_event_info = &gpe_block->event_info[gpe_number - - gpe_block-> - block_base_number]; - - gpe_event_info->flags |=ACPI_GPE_CAN_WAKE ; +if (gpe_device == target_gpe_device) { + gpe_event_info =acpi_ev_gpeblk_event_info (gpe_block, + gpe_number); +if (gpe_event_info) + gpe_event_info->flags |=ACPI_GPE_CAN_WAKE ; } cleanup:
RFC PATCH 3/3 ACPI / ACPICA: Reduce code duplication related to GPE lookup
From: Rafael J. Wysocki <[email protected]>
Subject: ACPI / ACPICA: Reduce code duplication related to GPE lookup
There is some code duplication related to the looking up a GPE in a GPE block. Namely, the same code construct, which is quite difficult to read and inefficient, is used in four different places (two times in acpi_ev_get_gpe_event_info(), in acpi_ev_match_prw_and_gpe() and in acpi_ev_save_method_info()). To reduce the code duplication move it to a separate function and clean it up.
--- drivers/acpi/acpica/acevents.h | 4 ++ drivers/acpi/acpica/evgpe.c | 58 ++++++++++++++++++++++++----------------- drivers/acpi/acpica/evgpeblk.c | 21 ++++---------- 3 files changed, 46 insertions(+), 37 deletions(-)
drivers/acpi/acpica/evgpe.c +35 -23
@@ -168,6 +168,33 @@ acpi_status acpi_ev_disable_gpe(struct areturn_ACPI_STATUS (status); } + +/******************************************************************************* +* +* FUNCTION: acpi_ev_gpeblk_event_info +* +* PARAMETERS: gpe_block - GPE block to search +* gpe_number - Raw GPE number +* +* RETURN: A GPE event_info struct if found. NULL if not found +* +******************************************************************************/ + +struct acpi_gpe_event_info *acpi_ev_gpeblk_event_info ( +struct acpi_gpe_block_info *gpe_block, +u32 gpe_number) +{ +u32 gpe_index, gpe_count; + +if (!gpe_block || gpe_number < gpe_block->block_base_number) +return (NULL ); + + gpe_index = gpe_number - gpe_block->block_base_number; + gpe_count = gpe_block->register_count *ACPI_GPE_REGISTER_WIDTH ; +return gpe_index < gpe_count ? + (&gpe_block->event_info[gpe_index]) : (NULL ); +} +/******************************************************************************* * * FUNCTION: acpi_ev_get_gpe_event_info
@@ -189,7 +216,7 @@ struct acpi_gpe_event_info *acpi_ev_get_ u32 gpe_number) {union acpi_operand_object *obj_desc; -struct acpi_gpe_block_info *gpe_block; +struct acpi_gpe_event_info *gpe_info;u32 i;ACPI_FUNCTION_ENTRY ();
@@ -201,18 +228,11 @@ struct acpi_gpe_event_info *acpi_ev_get_/* Examine GPE Block 0 and 1 (These blocks are permanent) */ for (i =0 ; i <ACPI_MAX_GPE_BLOCKS ; i++) { - gpe_block = acpi_gbl_gpe_fadt_blocks[i]; -if (gpe_block) { -if ((gpe_number >= gpe_block->block_base_number) - && (gpe_number < - gpe_block->block_base_number + - (gpe_block->register_count *8 ))) { -return (&gpe_block-> - event_info[gpe_number - - gpe_block-> - block_base_number]); - } - } + gpe_info =acpi_ev_gpeblk_event_info ( + acpi_gbl_gpe_fadt_blocks[i], + gpe_number); +if (gpe_info) +return (gpe_info); }/* The gpe_number was not in the range of either FADT GPE block */
@@ -228,16 +248,8 @@ struct acpi_gpe_event_info *acpi_ev_get_return (NULL ); } - gpe_block = obj_desc->device.gpe_block; - -if ((gpe_number >= gpe_block->block_base_number) && - (gpe_number < - gpe_block->block_base_number + (gpe_block->register_count *8 ))) { -return (&gpe_block-> - event_info[gpe_number - gpe_block->block_base_number]); - } - -return (NULL ); +return acpi_ev_gpeblk_event_info (obj_desc->device.gpe_block, + gpe_number); }/*******************************************************************************
drivers/acpi/acpica/acevents.h +4 -0
@@ -82,6 +82,10 @@ acpi_status acpi_ev_enable_gpe(struct acacpi_status acpi_ev_disable_gpe (struct acpi_gpe_event_info *gpe_event_info); +struct acpi_gpe_event_info *acpi_ev_gpeblk_event_info ( +struct acpi_gpe_block_info *gpe_block, +u32 gpe_number); +struct acpi_gpe_event_info *acpi_ev_get_gpe_event_info (acpi_handle gpe_device,u32 gpe_number);
drivers/acpi/acpica/evgpeblk.c +7 -14
@@ -311,9 +311,8 @@ acpi_ev_save_method_info(acpi_handle obj/* Ensure that we have a valid GPE number for this GPE block */ -if ((gpe_number < gpe_block->block_base_number) || - (gpe_number >= (gpe_block->block_base_number + - (gpe_block->register_count *8 )))) { + gpe_event_info =acpi_ev_gpeblk_event_info (gpe_block, gpe_number); +if (!gpe_event_info) {/* * Not valid for this GPE block, just ignore it. However, it may be * valid for a different GPE block, since GPE0 and GPE1 methods both
@@ -326,8 +325,6 @@ acpi_ev_save_method_info(acpi_handle obj* Now we can add this information to the gpe_event_info block for use * during dispatch of this GPE. */ - gpe_event_info = - &gpe_block->event_info[gpe_number - gpe_block->block_base_number]; gpe_event_info->flags = (u8) (type |ACPI_GPE_DISPATCH_METHOD );
@@ -439,15 +436,11 @@ acpi_ev_match_prw_and_gpe(acpi_handle ob* 2) The GPE index(number) is within the range of the Gpe Block * associated with the GPE device. */ -if ((gpe_device == target_gpe_device) && - (gpe_number >= gpe_block->block_base_number) && - (gpe_number < gpe_block->block_base_number + - (gpe_block->register_count *8 ))) { - gpe_event_info = &gpe_block->event_info[gpe_number - - gpe_block-> - block_base_number]; - - gpe_event_info->flags |=ACPI_GPE_CAN_WAKE ; +if (gpe_device == target_gpe_device) { + gpe_event_info =acpi_ev_gpeblk_event_info (gpe_block, + gpe_number); +if (gpe_event_info) + gpe_event_info->flags |=ACPI_GPE_CAN_WAKE ; } cleanup: