public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH 0/3] ACPICA modifications
@ 2010-03-07 23:34 Rafael J. Wysocki
  2010-03-07 23:35 ` [RFC][PATCH 1/3] ACPI / ACPICA: Multiple system notify handlers for the root object Rafael J. Wysocki
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2010-03-07 23:34 UTC (permalink / raw)
  To: Moore, Robert, Len Brown; +Cc: ACPI Devel Maling List, Matthew Garrett

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


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [RFC][PATCH 1/3] ACPI / ACPICA: Multiple system notify handlers for the root object
  2010-03-07 23:34 [RFC][PATCH 0/3] ACPICA modifications Rafael J. Wysocki
@ 2010-03-07 23:35 ` Rafael J. Wysocki
  2010-03-07 23:36 ` [RFC][PATCH 2/3] ACPI / ACPICA: Fix locking in acpi_remove_gpe_handler() Rafael J. Wysocki
  2010-03-07 23:37 ` [RFC][PATCH 3/3] ACPI / ACPICA: Reduce code duplication in acpi_ev_get_gpe_event_info() Rafael J. Wysocki
  2 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2010-03-07 23:35 UTC (permalink / raw)
  To: Moore, Robert; +Cc: Len Brown, ACPI Devel Maling List, Matthew Garrett

Following commit 3f0be67188c60ebf1b5d00354b44b4b24f5af313
(ACPI / ACPICA: Multiple system notify handlers per device) make it
possible to install multiple system notify handlers for the root
object.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/acpi/acpica/evmisc.c  |   31 +++++-------
 drivers/acpi/acpica/evxface.c |  103 ++++++++++++++++++++++++++++++++++--------
 2 files changed, 98 insertions(+), 36 deletions(-)

Index: linux-2.6/drivers/acpi/acpica/evxface.c
===================================================================
--- linux-2.6.orig/drivers/acpi/acpica/evxface.c
+++ linux-2.6/drivers/acpi/acpica/evxface.c
@@ -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_eve
 static 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_obje
 
 	acpi_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 = &notify_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_handle 
 
 		acpi_populate_handler_object(&notify_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) {
Index: linux-2.6/drivers/acpi/acpica/evmisc.c
===================================================================
--- linux-2.6.orig/drivers/acpi/acpica/evmisc.c
+++ linux-2.6/drivers/acpi/acpica/evmisc.c
@@ -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,


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [RFC][PATCH 2/3] ACPI / ACPICA: Fix locking in acpi_remove_gpe_handler()
  2010-03-07 23:34 [RFC][PATCH 0/3] ACPICA modifications Rafael J. Wysocki
  2010-03-07 23:35 ` [RFC][PATCH 1/3] ACPI / ACPICA: Multiple system notify handlers for the root object Rafael J. Wysocki
@ 2010-03-07 23:36 ` Rafael J. Wysocki
  2010-03-07 23:37 ` [RFC][PATCH 3/3] ACPI / ACPICA: Reduce code duplication in acpi_ev_get_gpe_event_info() Rafael J. Wysocki
  2 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2010-03-07 23:36 UTC (permalink / raw)
  To: Moore, Robert; +Cc: Len Brown, ACPI Devel Maling List, Matthew Garrett

The 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.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/acpi/acpica/evxface.c |   13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

Index: linux-2.6/drivers/acpi/acpica/evxface.c
===================================================================
--- linux-2.6.orig/drivers/acpi/acpica/evxface.c
+++ linux-2.6/drivers/acpi/acpica/evxface.c
@@ -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);


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [RFC][PATCH 3/3] ACPI / ACPICA: Reduce code duplication in acpi_ev_get_gpe_event_info()
  2010-03-07 23:34 [RFC][PATCH 0/3] ACPICA modifications Rafael J. Wysocki
  2010-03-07 23:35 ` [RFC][PATCH 1/3] ACPI / ACPICA: Multiple system notify handlers for the root object Rafael J. Wysocki
  2010-03-07 23:36 ` [RFC][PATCH 2/3] ACPI / ACPICA: Fix locking in acpi_remove_gpe_handler() Rafael J. Wysocki
@ 2010-03-07 23:37 ` Rafael J. Wysocki
  2010-03-12 23:14   ` [Update][RFC][PATCH 3/3] ACPI / ACPICA: Reduce code duplication related to GPE lookup Rafael J. Wysocki
  2 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2010-03-07 23:37 UTC (permalink / raw)
  To: Moore, Robert; +Cc: Len Brown, ACPI Devel Maling List, Matthew Garrett

Move duplicated code from acpi_ev_get_gpe_event_info(), which is also
quite difficult to read, to a separate static function.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/acpi/acpica/evgpe.c |   56 +++++++++++++++++++++++++-------------------
 1 file changed, 33 insertions(+), 23 deletions(-)

Index: linux-2.6/drivers/acpi/acpica/evgpe.c
===================================================================
--- linux-2.6.orig/drivers/acpi/acpica/evgpe.c
+++ linux-2.6/drivers/acpi/acpica/evgpe.c
@@ -168,6 +168,33 @@ acpi_status acpi_ev_disable_gpe(struct a
 	return_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);
 }
 
 /*******************************************************************************


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [Update][RFC][PATCH 3/3] ACPI / ACPICA: Reduce code duplication related to GPE lookup
  2010-03-07 23:37 ` [RFC][PATCH 3/3] ACPI / ACPICA: Reduce code duplication in acpi_ev_get_gpe_event_info() Rafael J. Wysocki
@ 2010-03-12 23:14   ` Rafael J. Wysocki
  2010-03-16 22:19     ` [Update 2x][RFC][PATCH " Rafael J. Wysocki
  0 siblings, 1 reply; 6+ messages in thread
From: Rafael J. Wysocki @ 2010-03-12 23:14 UTC (permalink / raw)
  To: Moore, Robert; +Cc: Len Brown, ACPI Devel Maling List, Matthew Garrett

From: Rafael J. Wysocki <rjw@sisk.pl>

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.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
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(-)

Index: linux-2.6/drivers/acpi/acpica/evgpe.c
===================================================================
--- linux-2.6.orig/drivers/acpi/acpica/evgpe.c
+++ linux-2.6/drivers/acpi/acpica/evgpe.c
@@ -168,6 +168,33 @@ acpi_status acpi_ev_disable_gpe(struct a
 	return_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);
 }
 
 /*******************************************************************************
Index: linux-2.6/drivers/acpi/acpica/acevents.h
===================================================================
--- linux-2.6.orig/drivers/acpi/acpica/acevents.h
+++ linux-2.6/drivers/acpi/acpica/acevents.h
@@ -82,6 +82,10 @@ acpi_status acpi_ev_enable_gpe(struct ac
 
 acpi_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);
 
Index: linux-2.6/drivers/acpi/acpica/evgpeblk.c
===================================================================
--- linux-2.6.orig/drivers/acpi/acpica/evgpeblk.c
+++ linux-2.6/drivers/acpi/acpica/evgpeblk.c
@@ -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:

^ permalink raw reply	[flat|nested] 6+ messages in thread
* [Update 2x][RFC][PATCH 3/3] ACPI / ACPICA: Reduce code duplication related to GPE lookup
  2010-03-12 23:14   ` [Update][RFC][PATCH 3/3] ACPI / ACPICA: Reduce code duplication related to GPE lookup Rafael J. Wysocki
@ 2010-03-16 22:19     ` Rafael J. Wysocki
  0 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2010-03-16 22:19 UTC (permalink / raw)
  To: Moore, Robert; +Cc: Len Brown, Matthew Garrett, ACPI Devel Maling List

From: Rafael J. Wysocki <rjw@sisk.pl>
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.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 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(-)

Index: linux-2.6/drivers/acpi/acpica/evgpe.c
===================================================================
--- linux-2.6.orig/drivers/acpi/acpica/evgpe.c
+++ linux-2.6/drivers/acpi/acpica/evgpe.c
@@ -168,6 +168,33 @@ acpi_status acpi_ev_disable_gpe(struct a
 	return_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);
 }
 
 /*******************************************************************************
Index: linux-2.6/drivers/acpi/acpica/acevents.h
===================================================================
--- linux-2.6.orig/drivers/acpi/acpica/acevents.h
+++ linux-2.6/drivers/acpi/acpica/acevents.h
@@ -82,6 +82,10 @@ acpi_status acpi_ev_enable_gpe(struct ac
 
 acpi_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);
 
Index: linux-2.6/drivers/acpi/acpica/evgpeblk.c
===================================================================
--- linux-2.6.orig/drivers/acpi/acpica/evgpeblk.c
+++ linux-2.6/drivers/acpi/acpica/evgpeblk.c
@@ -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:

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-03-16 22:16 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-07 23:34 [RFC][PATCH 0/3] ACPICA modifications Rafael J. Wysocki
2010-03-07 23:35 ` [RFC][PATCH 1/3] ACPI / ACPICA: Multiple system notify handlers for the root object Rafael J. Wysocki
2010-03-07 23:36 ` [RFC][PATCH 2/3] ACPI / ACPICA: Fix locking in acpi_remove_gpe_handler() Rafael J. Wysocki
2010-03-07 23:37 ` [RFC][PATCH 3/3] ACPI / ACPICA: Reduce code duplication in acpi_ev_get_gpe_event_info() Rafael J. Wysocki
2010-03-12 23:14   ` [Update][RFC][PATCH 3/3] ACPI / ACPICA: Reduce code duplication related to GPE lookup Rafael J. Wysocki
2010-03-16 22:19     ` [Update 2x][RFC][PATCH " Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox