public inbox for linux-acpi@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 6/6] ACPICA: Misc comments to minimize code divergence
@ 2010-12-06 15:59 Lin Ming
  2010-12-09 22:32 ` Rafael J. Wysocki
  0 siblings, 1 reply; 2+ messages in thread
From: Lin Ming @ 2010-12-06 15:59 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown, Bob Moore; +Cc: linux-acpi

Modify/add some comments to minimize ACPICA/linux GPE code divergence.

Signed-off-by: Lin Ming <ming.m.lin@intel.com>
--- 
drivers/acpi/acpica/achware.h   |    2 +-
 drivers/acpi/acpica/evgpe.c     |   23 ++++++++++++++---------
 drivers/acpi/acpica/evgpeblk.c  |   18 ++++++++----------
 drivers/acpi/acpica/evgpeinit.c |   24 ++++++++++++++++++++----
 drivers/acpi/acpica/evxfgpe.c   |   32 +++++++++++++++++++++-----------
 drivers/acpi/acpica/hwgpe.c     |   30 ++++++++++++++++++------------
 6 files changed, 82 insertions(+), 47 deletions(-)

diff --git a/drivers/acpi/acpica/achware.h b/drivers/acpi/acpica/achware.h
index 167470a..258d628 100644
--- a/drivers/acpi/acpica/achware.h
+++ b/drivers/acpi/acpica/achware.h
@@ -94,7 +94,7 @@ u32 acpi_hw_get_gpe_register_bit(struct acpi_gpe_event_info *gpe_event_info,
 			     struct acpi_gpe_register_info *gpe_register_info);
 
 acpi_status
-acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u8 action);
+acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action);
 
 acpi_status
 acpi_hw_disable_gpe_block(struct acpi_gpe_xrupt_info *gpe_xrupt_info,
diff --git a/drivers/acpi/acpica/evgpe.c b/drivers/acpi/acpica/evgpe.c
index c9d9ecf..6cb679f 100644
--- a/drivers/acpi/acpica/evgpe.c
+++ b/drivers/acpi/acpica/evgpe.c
@@ -104,7 +104,7 @@ acpi_ev_update_gpe_enable_mask(struct acpi_gpe_event_info *gpe_event_info)
  *
  * RETURN:      Status
  *
- * DESCRIPTION: Clear the given GPE from stale events and enable it.
+ * DESCRIPTION: Clear a GPE of stale events and enable it.
  *
  ******************************************************************************/
 acpi_status
@@ -142,7 +142,7 @@ acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info)
  *
  * FUNCTION:    acpi_ev_add_gpe_reference
  *
- * PARAMETERS:  gpe_event_info  - GPE to enable
+ * PARAMETERS:  gpe_event_info          - Add a reference to this GPE
  *
  * RETURN:      Status
  *
@@ -163,6 +163,9 @@ acpi_status acpi_ev_add_gpe_reference(struct acpi_gpe_event_info *gpe_event_info
 
 	gpe_event_info->runtime_count++;
 	if (gpe_event_info->runtime_count == 1) {
+
+		/* Enable on first reference */
+
 		status = acpi_ev_update_gpe_enable_mask(gpe_event_info);
 		if (ACPI_SUCCESS(status)) {
 			status = acpi_ev_enable_gpe(gpe_event_info);
@@ -180,7 +183,7 @@ acpi_status acpi_ev_add_gpe_reference(struct acpi_gpe_event_info *gpe_event_info
  *
  * FUNCTION:    acpi_ev_remove_gpe_reference
  *
- * PARAMETERS:  gpe_event_info  - GPE to disable
+ * PARAMETERS:  gpe_event_info          - Remove a reference to this GPE
  *
  * RETURN:      Status
  *
@@ -201,6 +204,9 @@ acpi_status acpi_ev_remove_gpe_reference(struct acpi_gpe_event_info *gpe_event_i
 
 	gpe_event_info->runtime_count--;
 	if (!gpe_event_info->runtime_count) {
+
+		/* Disable on last reference */
+
 		status = acpi_ev_update_gpe_enable_mask(gpe_event_info);
 		if (ACPI_SUCCESS(status)) {
 			status = acpi_hw_low_set_gpe(gpe_event_info,
@@ -386,7 +392,7 @@ u32 acpi_ev_gpe_detect(struct acpi_gpe_xrupt_info * gpe_xrupt_list)
 			}
 
 			ACPI_DEBUG_PRINT((ACPI_DB_INTERRUPTS,
-					  "Read GPE Register at GPE%X: Status=%02X, Enable=%02X\n",
+					  "Read GPE Register at GPE%02X: Status=%02X, Enable=%02X\n",
 					  gpe_register_info->base_gpe_number,
 					  status_reg, enable_reg));
 
@@ -567,7 +573,7 @@ static void ACPI_SYSTEM_XFACE acpi_ev_asynch_execute_gpe_method(void *context)
  ******************************************************************************/
 
 static void ACPI_SYSTEM_XFACE acpi_ev_asynch_enable_gpe(void *context)
- {
+{
  	struct acpi_gpe_event_info *gpe_event_info = context;
 
 	(void)acpi_ev_finish_gpe(gpe_event_info);
@@ -659,8 +665,7 @@ acpi_ev_gpe_dispatch(struct acpi_namespace_node *gpe_device,
 		status = acpi_hw_clear_gpe(gpe_event_info);
 		if (ACPI_FAILURE(status)) {
 			ACPI_EXCEPTION((AE_INFO, status,
-					"Unable to clear GPE[0x%2X]",
-					gpe_number));
+					"Unable to clear GPE%02X", gpe_number));
 			return_UINT32(ACPI_INTERRUPT_NOT_HANDLED);
 		}
 	}
@@ -719,7 +724,7 @@ acpi_ev_gpe_dispatch(struct acpi_namespace_node *gpe_device,
 					 gpe_event_info);
 		if (ACPI_FAILURE(status)) {
 			ACPI_EXCEPTION((AE_INFO, status,
-					"Unable to queue handler for GPE[0x%2X] - event disabled",
+					"Unable to queue handler for GPE%2X - event disabled",
 					gpe_number));
 		}
 		break;
@@ -732,7 +737,7 @@ acpi_ev_gpe_dispatch(struct acpi_namespace_node *gpe_device,
 		 * a GPE to be enabled if it has no handler or method.
 		 */
 		ACPI_ERROR((AE_INFO,
-			    "No handler or method for GPE[0x%2X], disabling event",
+			    "No handler or method for GPE%02X, disabling event",
 			    gpe_number));
 
 		break;
diff --git a/drivers/acpi/acpica/evgpeblk.c b/drivers/acpi/acpica/evgpeblk.c
index e2e8164..9acb869 100644
--- a/drivers/acpi/acpica/evgpeblk.c
+++ b/drivers/acpi/acpica/evgpeblk.c
@@ -361,9 +361,9 @@ acpi_ev_create_gpe_block(struct acpi_namespace_node *gpe_device,
 
 	gpe_block->node = gpe_device;
 	gpe_block->gpe_count = (u16)(register_count * ACPI_GPE_REGISTER_WIDTH);
+	gpe_block->initialized = FALSE;
 	gpe_block->register_count = register_count;
 	gpe_block->block_base_number = gpe_block_base_number;
-	gpe_block->initialized = FALSE;
 
 	ACPI_MEMCPY(&gpe_block->block_address, gpe_block_address,
 		    sizeof(struct acpi_generic_address));
@@ -423,14 +423,12 @@ acpi_ev_create_gpe_block(struct acpi_namespace_node *gpe_device,
  *
  * FUNCTION:    acpi_ev_initialize_gpe_block
  *
- * PARAMETERS:  gpe_device          - Handle to the parent GPE block
- *              gpe_block           - Gpe Block info
+ * PARAMETERS:  acpi_gpe_callback
  *
  * RETURN:      Status
  *
- * DESCRIPTION: Initialize and enable a GPE block. First find and run any
- *              _PRT methods associated with the block, then enable the
- *              appropriate GPEs.
+ * DESCRIPTION: Initialize and enable a GPE block. Enable GPEs that have
+ *              associated methods.
  *              Note: Assumes namespace is locked.
  *
  ******************************************************************************/
@@ -450,8 +448,8 @@ acpi_ev_initialize_gpe_block(struct acpi_gpe_xrupt_info *gpe_xrupt_info,
 	ACPI_FUNCTION_TRACE(ev_initialize_gpe_block);
 
 	/*
-	 * Ignore a null GPE block (e.g., if no GPE block 1 exists) and
-	 * GPE blocks that have been initialized already.
+	 * Ignore a null GPE block (e.g., if no GPE block 1 exists), and
+	 * any GPE blocks that have been initialized already.
 	 */
 	if (!gpe_block || gpe_block->initialized) {
 		return_ACPI_STATUS(AE_OK);
@@ -459,8 +457,8 @@ acpi_ev_initialize_gpe_block(struct acpi_gpe_xrupt_info *gpe_xrupt_info,
 
 	/*
 	 * Enable all GPEs that have a corresponding method and have the
-	 * ACPI_GPE_CAN_WAKE flag unset.  Any other GPEs within this block must
-	 * be enabled via the acpi_enable_gpe() interface.
+	 * ACPI_GPE_CAN_WAKE flag unset. Any other GPEs within this block
+	 * must be enabled via the acpi_enable_gpe() interface.
 	 */
 	gpe_enabled_count = 0;
 
diff --git a/drivers/acpi/acpica/evgpeinit.c b/drivers/acpi/acpica/evgpeinit.c
index 223dba2..beb3675 100644
--- a/drivers/acpi/acpica/evgpeinit.c
+++ b/drivers/acpi/acpica/evgpeinit.c
@@ -45,11 +45,27 @@
 #include "accommon.h"
 #include "acevents.h"
 #include "acnamesp.h"
-#include "acinterp.h"
 
 #define _COMPONENT          ACPI_EVENTS
 ACPI_MODULE_NAME("evgpeinit")
 
+/*
+ * Note: History of _PRW support in ACPICA
+ *
+ * Originally (2000 - 2010), the GPE initialization code performed a walk of
+ * the entire namespace to execute the _PRW methods and detect all GPEs
+ * capable of waking the system.
+ *
+ * As of 10/2010, the _PRW method execution has been removed since it is
+ * actually unnecessary. The host OS must in fact execute all _PRW methods
+ * in order to identify the device/power-resource dependencies. We now put
+ * the onus on the host OS to identify the wake GPEs as part of this process
+ * and to inform ACPICA of these GPEs via the acpi_setup_gpe_for_wake interface. This
+ * not only reduces the complexity of the ACPICA initialization code, but in
+ * some cases (on systems with very large namespaces) it should reduce the
+ * kernel boot time as well.
+ */
+
 /*******************************************************************************
  *
  * FUNCTION:    acpi_ev_gpe_initialize
@@ -222,7 +238,7 @@ void acpi_ev_update_gpes(acpi_owner_id table_owner_id)
 	acpi_status status = AE_OK;
 
 	/*
-	 * 2) Find any _Lxx/_Exx GPE methods that have just been loaded.
+	 * Find any _Lxx/_Exx GPE methods that have just been loaded.
 	 *
 	 * Any GPEs that correspond to new _Lxx/_Exx methods are immediately
 	 * enabled.
@@ -235,9 +251,9 @@ void acpi_ev_update_gpes(acpi_owner_id table_owner_id)
 		return;
 	}
 
+	walk_info.count = 0;
 	walk_info.owner_id = table_owner_id;
 	walk_info.execute_by_owner_id = TRUE;
-	walk_info.count = 0;
 
 	/* Walk the interrupt level descriptor list */
 
@@ -298,7 +314,7 @@ void acpi_ev_update_gpes(acpi_owner_id table_owner_id)
  *                  xx     - is the GPE number [in HEX]
  *
  * If walk_info->execute_by_owner_id is TRUE, we only execute examine GPE methods
- *    with that owner.
+ * with that owner.
  *
  ******************************************************************************/
 
diff --git a/drivers/acpi/acpica/evxfgpe.c b/drivers/acpi/acpica/evxfgpe.c
index fcf4a5c..416845b 100644
--- a/drivers/acpi/acpica/evxfgpe.c
+++ b/drivers/acpi/acpica/evxfgpe.c
@@ -55,13 +55,19 @@ ACPI_MODULE_NAME("evxfgpe")
  *
  * PARAMETERS:  None
  *
- * RETURN:      None
+ * RETURN:      Status
+ *
+ * DESCRIPTION: Complete GPE initialization and enable all GPEs that have
+ *              associated _Lxx or _Exx methods and are not pointed to by any
+ *              device _PRW methods (this indicates that these GPEs are
+ *              generally intended for system or device wakeup. Such GPEs
+ *              have to be enabled directly when the devices whose _PRW
+ *              methods point to them are set up for wakeup signaling.)
  *
- * DESCRIPTION: Enable all GPEs that have associated _Lxx or _Exx methods and
- *              are not pointed to by any device _PRW methods indicating that
- *              these GPEs are generally intended for system or device wakeup
- *              (such GPEs have to be enabled directly when the devices whose
- *              _PRW methods point to them are set up for wakeup signaling).
+ * NOTE: Should be called after any GPEs are added to the system. Primarily,
+ * after the system _PRW methods have been run, but also after a GPE Block
+ * Device has been added or if any new GPE methods have been added via a
+ * dynamic table load.
  *
  ******************************************************************************/
 
@@ -252,7 +258,8 @@ ACPI_EXPORT_SYMBOL(acpi_setup_gpe_for_wake)
  *
  * RETURN:      Status
  *
- * DESCRIPTION: Set or clear the GPE's wakeup enable mask bit.
+ * DESCRIPTION: Set or clear the GPE's wakeup enable mask bit. The GPE must
+ *              already be marked as a WAKE GPE.
  *
  ******************************************************************************/
 
@@ -268,8 +275,10 @@ acpi_status acpi_set_gpe_wake_mask(acpi_handle gpe_device, u32 gpe_number, u8 ac
 
 	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
 
-	/* Ensure that we have a valid GPE number */
-
+	/*
+	 * Ensure that we have a valid GPE number and that this GPE is in
+	 * fact a wake GPE
+	 */
 	gpe_event_info = acpi_ev_get_gpe_event_info(gpe_device, gpe_number);
 	if (!gpe_event_info) {
 		status = AE_BAD_PARAMETER;
@@ -366,7 +375,7 @@ ACPI_EXPORT_SYMBOL(acpi_clear_gpe)
  *
  * RETURN:      Status
  *
- * DESCRIPTION: Get status of an event (general purpose)
+ * DESCRIPTION: Get the current status of a GPE (signalled/not_signalled)
  *
  ******************************************************************************/
 acpi_status
@@ -476,7 +485,8 @@ ACPI_EXPORT_SYMBOL(acpi_enable_all_runtime_gpes)
  *
  * RETURN:      Status
  *
- * DESCRIPTION: Create and Install a block of GPE registers
+ * DESCRIPTION: Create and Install a block of GPE registers. The GPEs are not
+ *              enabled here.
  *
  ******************************************************************************/
 acpi_status
diff --git a/drivers/acpi/acpica/hwgpe.c b/drivers/acpi/acpica/hwgpe.c
index 7c6d485..85c3cbd 100644
--- a/drivers/acpi/acpica/hwgpe.c
+++ b/drivers/acpi/acpica/hwgpe.c
@@ -62,10 +62,10 @@ acpi_hw_enable_wakeup_gpe_block(struct acpi_gpe_xrupt_info *gpe_xrupt_info,
  * PARAMETERS:	gpe_event_info	    - Info block for the GPE
  *		gpe_register_info   - Info block for the GPE register
  *
- * RETURN:	Status
+ * RETURN:	Register mask with a one in the GPE bit position
  *
- * DESCRIPTION:	Compute GPE enable mask with one bit corresponding to the given
- *		GPE set.
+ * DESCRIPTION: Compute the register mask for this GPE. One bit is set in the
+ *              correct position for the input GPE.
  *
  ******************************************************************************/
 
@@ -85,12 +85,12 @@ u32 acpi_hw_get_gpe_register_bit(struct acpi_gpe_event_info *gpe_event_info,
  *
  * RETURN:	Status
  *
- * DESCRIPTION: Enable or disable a single GPE in its enable register.
+ * DESCRIPTION: Enable or disable a single GPE in the parent enable register.
  *
  ******************************************************************************/
 
 acpi_status
-acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u8 action)
+acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action)
 {
 	struct acpi_gpe_register_info *gpe_register_info;
 	acpi_status status;
@@ -113,14 +113,20 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u8 action)
 		return (status);
 	}
 
-	/* Set ot clear just the bit that corresponds to this GPE */
+	/* Set or clear just the bit that corresponds to this GPE */
 
 	register_bit = acpi_hw_get_gpe_register_bit(gpe_event_info,
 						gpe_register_info);
 	switch (action) {
 	case ACPI_GPE_CONDITIONAL_ENABLE:
-		if (!(register_bit & gpe_register_info->enable_for_run))
+
+		/* Only enable if the enable_for_run bit is set */
+
+		if (!(register_bit & gpe_register_info->enable_for_run)) {
 			return (AE_BAD_PARAMETER);
+		}
+
+		/*lint -fallthrough */
 
 	case ACPI_GPE_ENABLE:
 		ACPI_SET_BIT(enable_mask, register_bit);
@@ -131,7 +137,7 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u8 action)
 		break;
 
 	default:
-		ACPI_ERROR((AE_INFO, "Invalid action\n"));
+		ACPI_ERROR((AE_INFO, "Invalid GPE Action, %u\n", action));
 		return (AE_BAD_PARAMETER);
 	}
 
@@ -168,13 +174,13 @@ acpi_status acpi_hw_clear_gpe(struct acpi_gpe_event_info * gpe_event_info)
 		return (AE_NOT_EXIST);
 	}
 
-	register_bit = acpi_hw_get_gpe_register_bit(gpe_event_info,
-						gpe_register_info);
-
 	/*
 	 * Write a one to the appropriate bit in the status register to
 	 * clear this GPE.
 	 */
+	register_bit =
+	    acpi_hw_get_gpe_register_bit(gpe_event_info, gpe_register_info);
+
 	status = acpi_hw_write(register_bit,
 			       &gpe_register_info->status_address);
 
@@ -201,8 +207,8 @@ acpi_hw_get_gpe_status(struct acpi_gpe_event_info * gpe_event_info,
 	u32 in_byte;
 	u32 register_bit;
 	struct acpi_gpe_register_info *gpe_register_info;
-	acpi_status status;
 	acpi_event_status local_event_status = 0;
+	acpi_status status;
 
 	ACPI_FUNCTION_ENTRY();
 
-- 
1.5.3





^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH 6/6] ACPICA: Misc comments to minimize code divergence
  2010-12-06 15:59 [PATCH 6/6] ACPICA: Misc comments to minimize code divergence Lin Ming
@ 2010-12-09 22:32 ` Rafael J. Wysocki
  0 siblings, 0 replies; 2+ messages in thread
From: Rafael J. Wysocki @ 2010-12-09 22:32 UTC (permalink / raw)
  To: Lin Ming; +Cc: Len Brown, Bob Moore, linux-acpi

On Monday, December 06, 2010, Lin Ming wrote:
> Modify/add some comments to minimize ACPICA/linux GPE code divergence.
> 
> Signed-off-by: Lin Ming <ming.m.lin@intel.com>

Acked-by: Rafael J. Wysocki <rjw@sisk.pl>

> --- 
> drivers/acpi/acpica/achware.h   |    2 +-
>  drivers/acpi/acpica/evgpe.c     |   23 ++++++++++++++---------
>  drivers/acpi/acpica/evgpeblk.c  |   18 ++++++++----------
>  drivers/acpi/acpica/evgpeinit.c |   24 ++++++++++++++++++++----
>  drivers/acpi/acpica/evxfgpe.c   |   32 +++++++++++++++++++++-----------
>  drivers/acpi/acpica/hwgpe.c     |   30 ++++++++++++++++++------------
>  6 files changed, 82 insertions(+), 47 deletions(-)
> 
> diff --git a/drivers/acpi/acpica/achware.h b/drivers/acpi/acpica/achware.h
> index 167470a..258d628 100644
> --- a/drivers/acpi/acpica/achware.h
> +++ b/drivers/acpi/acpica/achware.h
> @@ -94,7 +94,7 @@ u32 acpi_hw_get_gpe_register_bit(struct acpi_gpe_event_info *gpe_event_info,
>  			     struct acpi_gpe_register_info *gpe_register_info);
>  
>  acpi_status
> -acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u8 action);
> +acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action);
>  
>  acpi_status
>  acpi_hw_disable_gpe_block(struct acpi_gpe_xrupt_info *gpe_xrupt_info,
> diff --git a/drivers/acpi/acpica/evgpe.c b/drivers/acpi/acpica/evgpe.c
> index c9d9ecf..6cb679f 100644
> --- a/drivers/acpi/acpica/evgpe.c
> +++ b/drivers/acpi/acpica/evgpe.c
> @@ -104,7 +104,7 @@ acpi_ev_update_gpe_enable_mask(struct acpi_gpe_event_info *gpe_event_info)
>   *
>   * RETURN:      Status
>   *
> - * DESCRIPTION: Clear the given GPE from stale events and enable it.
> + * DESCRIPTION: Clear a GPE of stale events and enable it.
>   *
>   ******************************************************************************/
>  acpi_status
> @@ -142,7 +142,7 @@ acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info)
>   *
>   * FUNCTION:    acpi_ev_add_gpe_reference
>   *
> - * PARAMETERS:  gpe_event_info  - GPE to enable
> + * PARAMETERS:  gpe_event_info          - Add a reference to this GPE
>   *
>   * RETURN:      Status
>   *
> @@ -163,6 +163,9 @@ acpi_status acpi_ev_add_gpe_reference(struct acpi_gpe_event_info *gpe_event_info
>  
>  	gpe_event_info->runtime_count++;
>  	if (gpe_event_info->runtime_count == 1) {
> +
> +		/* Enable on first reference */
> +
>  		status = acpi_ev_update_gpe_enable_mask(gpe_event_info);
>  		if (ACPI_SUCCESS(status)) {
>  			status = acpi_ev_enable_gpe(gpe_event_info);
> @@ -180,7 +183,7 @@ acpi_status acpi_ev_add_gpe_reference(struct acpi_gpe_event_info *gpe_event_info
>   *
>   * FUNCTION:    acpi_ev_remove_gpe_reference
>   *
> - * PARAMETERS:  gpe_event_info  - GPE to disable
> + * PARAMETERS:  gpe_event_info          - Remove a reference to this GPE
>   *
>   * RETURN:      Status
>   *
> @@ -201,6 +204,9 @@ acpi_status acpi_ev_remove_gpe_reference(struct acpi_gpe_event_info *gpe_event_i
>  
>  	gpe_event_info->runtime_count--;
>  	if (!gpe_event_info->runtime_count) {
> +
> +		/* Disable on last reference */
> +
>  		status = acpi_ev_update_gpe_enable_mask(gpe_event_info);
>  		if (ACPI_SUCCESS(status)) {
>  			status = acpi_hw_low_set_gpe(gpe_event_info,
> @@ -386,7 +392,7 @@ u32 acpi_ev_gpe_detect(struct acpi_gpe_xrupt_info * gpe_xrupt_list)
>  			}
>  
>  			ACPI_DEBUG_PRINT((ACPI_DB_INTERRUPTS,
> -					  "Read GPE Register at GPE%X: Status=%02X, Enable=%02X\n",
> +					  "Read GPE Register at GPE%02X: Status=%02X, Enable=%02X\n",
>  					  gpe_register_info->base_gpe_number,
>  					  status_reg, enable_reg));
>  
> @@ -567,7 +573,7 @@ static void ACPI_SYSTEM_XFACE acpi_ev_asynch_execute_gpe_method(void *context)
>   ******************************************************************************/
>  
>  static void ACPI_SYSTEM_XFACE acpi_ev_asynch_enable_gpe(void *context)
> - {
> +{
>   	struct acpi_gpe_event_info *gpe_event_info = context;
>  
>  	(void)acpi_ev_finish_gpe(gpe_event_info);
> @@ -659,8 +665,7 @@ acpi_ev_gpe_dispatch(struct acpi_namespace_node *gpe_device,
>  		status = acpi_hw_clear_gpe(gpe_event_info);
>  		if (ACPI_FAILURE(status)) {
>  			ACPI_EXCEPTION((AE_INFO, status,
> -					"Unable to clear GPE[0x%2X]",
> -					gpe_number));
> +					"Unable to clear GPE%02X", gpe_number));
>  			return_UINT32(ACPI_INTERRUPT_NOT_HANDLED);
>  		}
>  	}
> @@ -719,7 +724,7 @@ acpi_ev_gpe_dispatch(struct acpi_namespace_node *gpe_device,
>  					 gpe_event_info);
>  		if (ACPI_FAILURE(status)) {
>  			ACPI_EXCEPTION((AE_INFO, status,
> -					"Unable to queue handler for GPE[0x%2X] - event disabled",
> +					"Unable to queue handler for GPE%2X - event disabled",
>  					gpe_number));
>  		}
>  		break;
> @@ -732,7 +737,7 @@ acpi_ev_gpe_dispatch(struct acpi_namespace_node *gpe_device,
>  		 * a GPE to be enabled if it has no handler or method.
>  		 */
>  		ACPI_ERROR((AE_INFO,
> -			    "No handler or method for GPE[0x%2X], disabling event",
> +			    "No handler or method for GPE%02X, disabling event",
>  			    gpe_number));
>  
>  		break;
> diff --git a/drivers/acpi/acpica/evgpeblk.c b/drivers/acpi/acpica/evgpeblk.c
> index e2e8164..9acb869 100644
> --- a/drivers/acpi/acpica/evgpeblk.c
> +++ b/drivers/acpi/acpica/evgpeblk.c
> @@ -361,9 +361,9 @@ acpi_ev_create_gpe_block(struct acpi_namespace_node *gpe_device,
>  
>  	gpe_block->node = gpe_device;
>  	gpe_block->gpe_count = (u16)(register_count * ACPI_GPE_REGISTER_WIDTH);
> +	gpe_block->initialized = FALSE;
>  	gpe_block->register_count = register_count;
>  	gpe_block->block_base_number = gpe_block_base_number;
> -	gpe_block->initialized = FALSE;
>  
>  	ACPI_MEMCPY(&gpe_block->block_address, gpe_block_address,
>  		    sizeof(struct acpi_generic_address));
> @@ -423,14 +423,12 @@ acpi_ev_create_gpe_block(struct acpi_namespace_node *gpe_device,
>   *
>   * FUNCTION:    acpi_ev_initialize_gpe_block
>   *
> - * PARAMETERS:  gpe_device          - Handle to the parent GPE block
> - *              gpe_block           - Gpe Block info
> + * PARAMETERS:  acpi_gpe_callback
>   *
>   * RETURN:      Status
>   *
> - * DESCRIPTION: Initialize and enable a GPE block. First find and run any
> - *              _PRT methods associated with the block, then enable the
> - *              appropriate GPEs.
> + * DESCRIPTION: Initialize and enable a GPE block. Enable GPEs that have
> + *              associated methods.
>   *              Note: Assumes namespace is locked.
>   *
>   ******************************************************************************/
> @@ -450,8 +448,8 @@ acpi_ev_initialize_gpe_block(struct acpi_gpe_xrupt_info *gpe_xrupt_info,
>  	ACPI_FUNCTION_TRACE(ev_initialize_gpe_block);
>  
>  	/*
> -	 * Ignore a null GPE block (e.g., if no GPE block 1 exists) and
> -	 * GPE blocks that have been initialized already.
> +	 * Ignore a null GPE block (e.g., if no GPE block 1 exists), and
> +	 * any GPE blocks that have been initialized already.
>  	 */
>  	if (!gpe_block || gpe_block->initialized) {
>  		return_ACPI_STATUS(AE_OK);
> @@ -459,8 +457,8 @@ acpi_ev_initialize_gpe_block(struct acpi_gpe_xrupt_info *gpe_xrupt_info,
>  
>  	/*
>  	 * Enable all GPEs that have a corresponding method and have the
> -	 * ACPI_GPE_CAN_WAKE flag unset.  Any other GPEs within this block must
> -	 * be enabled via the acpi_enable_gpe() interface.
> +	 * ACPI_GPE_CAN_WAKE flag unset. Any other GPEs within this block
> +	 * must be enabled via the acpi_enable_gpe() interface.
>  	 */
>  	gpe_enabled_count = 0;
>  
> diff --git a/drivers/acpi/acpica/evgpeinit.c b/drivers/acpi/acpica/evgpeinit.c
> index 223dba2..beb3675 100644
> --- a/drivers/acpi/acpica/evgpeinit.c
> +++ b/drivers/acpi/acpica/evgpeinit.c
> @@ -45,11 +45,27 @@
>  #include "accommon.h"
>  #include "acevents.h"
>  #include "acnamesp.h"
> -#include "acinterp.h"
>  
>  #define _COMPONENT          ACPI_EVENTS
>  ACPI_MODULE_NAME("evgpeinit")
>  
> +/*
> + * Note: History of _PRW support in ACPICA
> + *
> + * Originally (2000 - 2010), the GPE initialization code performed a walk of
> + * the entire namespace to execute the _PRW methods and detect all GPEs
> + * capable of waking the system.
> + *
> + * As of 10/2010, the _PRW method execution has been removed since it is
> + * actually unnecessary. The host OS must in fact execute all _PRW methods
> + * in order to identify the device/power-resource dependencies. We now put
> + * the onus on the host OS to identify the wake GPEs as part of this process
> + * and to inform ACPICA of these GPEs via the acpi_setup_gpe_for_wake interface. This
> + * not only reduces the complexity of the ACPICA initialization code, but in
> + * some cases (on systems with very large namespaces) it should reduce the
> + * kernel boot time as well.
> + */
> +
>  /*******************************************************************************
>   *
>   * FUNCTION:    acpi_ev_gpe_initialize
> @@ -222,7 +238,7 @@ void acpi_ev_update_gpes(acpi_owner_id table_owner_id)
>  	acpi_status status = AE_OK;
>  
>  	/*
> -	 * 2) Find any _Lxx/_Exx GPE methods that have just been loaded.
> +	 * Find any _Lxx/_Exx GPE methods that have just been loaded.
>  	 *
>  	 * Any GPEs that correspond to new _Lxx/_Exx methods are immediately
>  	 * enabled.
> @@ -235,9 +251,9 @@ void acpi_ev_update_gpes(acpi_owner_id table_owner_id)
>  		return;
>  	}
>  
> +	walk_info.count = 0;
>  	walk_info.owner_id = table_owner_id;
>  	walk_info.execute_by_owner_id = TRUE;
> -	walk_info.count = 0;
>  
>  	/* Walk the interrupt level descriptor list */
>  
> @@ -298,7 +314,7 @@ void acpi_ev_update_gpes(acpi_owner_id table_owner_id)
>   *                  xx     - is the GPE number [in HEX]
>   *
>   * If walk_info->execute_by_owner_id is TRUE, we only execute examine GPE methods
> - *    with that owner.
> + * with that owner.
>   *
>   ******************************************************************************/
>  
> diff --git a/drivers/acpi/acpica/evxfgpe.c b/drivers/acpi/acpica/evxfgpe.c
> index fcf4a5c..416845b 100644
> --- a/drivers/acpi/acpica/evxfgpe.c
> +++ b/drivers/acpi/acpica/evxfgpe.c
> @@ -55,13 +55,19 @@ ACPI_MODULE_NAME("evxfgpe")
>   *
>   * PARAMETERS:  None
>   *
> - * RETURN:      None
> + * RETURN:      Status
> + *
> + * DESCRIPTION: Complete GPE initialization and enable all GPEs that have
> + *              associated _Lxx or _Exx methods and are not pointed to by any
> + *              device _PRW methods (this indicates that these GPEs are
> + *              generally intended for system or device wakeup. Such GPEs
> + *              have to be enabled directly when the devices whose _PRW
> + *              methods point to them are set up for wakeup signaling.)
>   *
> - * DESCRIPTION: Enable all GPEs that have associated _Lxx or _Exx methods and
> - *              are not pointed to by any device _PRW methods indicating that
> - *              these GPEs are generally intended for system or device wakeup
> - *              (such GPEs have to be enabled directly when the devices whose
> - *              _PRW methods point to them are set up for wakeup signaling).
> + * NOTE: Should be called after any GPEs are added to the system. Primarily,
> + * after the system _PRW methods have been run, but also after a GPE Block
> + * Device has been added or if any new GPE methods have been added via a
> + * dynamic table load.
>   *
>   ******************************************************************************/
>  
> @@ -252,7 +258,8 @@ ACPI_EXPORT_SYMBOL(acpi_setup_gpe_for_wake)
>   *
>   * RETURN:      Status
>   *
> - * DESCRIPTION: Set or clear the GPE's wakeup enable mask bit.
> + * DESCRIPTION: Set or clear the GPE's wakeup enable mask bit. The GPE must
> + *              already be marked as a WAKE GPE.
>   *
>   ******************************************************************************/
>  
> @@ -268,8 +275,10 @@ acpi_status acpi_set_gpe_wake_mask(acpi_handle gpe_device, u32 gpe_number, u8 ac
>  
>  	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
>  
> -	/* Ensure that we have a valid GPE number */
> -
> +	/*
> +	 * Ensure that we have a valid GPE number and that this GPE is in
> +	 * fact a wake GPE
> +	 */
>  	gpe_event_info = acpi_ev_get_gpe_event_info(gpe_device, gpe_number);
>  	if (!gpe_event_info) {
>  		status = AE_BAD_PARAMETER;
> @@ -366,7 +375,7 @@ ACPI_EXPORT_SYMBOL(acpi_clear_gpe)
>   *
>   * RETURN:      Status
>   *
> - * DESCRIPTION: Get status of an event (general purpose)
> + * DESCRIPTION: Get the current status of a GPE (signalled/not_signalled)
>   *
>   ******************************************************************************/
>  acpi_status
> @@ -476,7 +485,8 @@ ACPI_EXPORT_SYMBOL(acpi_enable_all_runtime_gpes)
>   *
>   * RETURN:      Status
>   *
> - * DESCRIPTION: Create and Install a block of GPE registers
> + * DESCRIPTION: Create and Install a block of GPE registers. The GPEs are not
> + *              enabled here.
>   *
>   ******************************************************************************/
>  acpi_status
> diff --git a/drivers/acpi/acpica/hwgpe.c b/drivers/acpi/acpica/hwgpe.c
> index 7c6d485..85c3cbd 100644
> --- a/drivers/acpi/acpica/hwgpe.c
> +++ b/drivers/acpi/acpica/hwgpe.c
> @@ -62,10 +62,10 @@ acpi_hw_enable_wakeup_gpe_block(struct acpi_gpe_xrupt_info *gpe_xrupt_info,
>   * PARAMETERS:	gpe_event_info	    - Info block for the GPE
>   *		gpe_register_info   - Info block for the GPE register
>   *
> - * RETURN:	Status
> + * RETURN:	Register mask with a one in the GPE bit position
>   *
> - * DESCRIPTION:	Compute GPE enable mask with one bit corresponding to the given
> - *		GPE set.
> + * DESCRIPTION: Compute the register mask for this GPE. One bit is set in the
> + *              correct position for the input GPE.
>   *
>   ******************************************************************************/
>  
> @@ -85,12 +85,12 @@ u32 acpi_hw_get_gpe_register_bit(struct acpi_gpe_event_info *gpe_event_info,
>   *
>   * RETURN:	Status
>   *
> - * DESCRIPTION: Enable or disable a single GPE in its enable register.
> + * DESCRIPTION: Enable or disable a single GPE in the parent enable register.
>   *
>   ******************************************************************************/
>  
>  acpi_status
> -acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u8 action)
> +acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u32 action)
>  {
>  	struct acpi_gpe_register_info *gpe_register_info;
>  	acpi_status status;
> @@ -113,14 +113,20 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u8 action)
>  		return (status);
>  	}
>  
> -	/* Set ot clear just the bit that corresponds to this GPE */
> +	/* Set or clear just the bit that corresponds to this GPE */
>  
>  	register_bit = acpi_hw_get_gpe_register_bit(gpe_event_info,
>  						gpe_register_info);
>  	switch (action) {
>  	case ACPI_GPE_CONDITIONAL_ENABLE:
> -		if (!(register_bit & gpe_register_info->enable_for_run))
> +
> +		/* Only enable if the enable_for_run bit is set */
> +
> +		if (!(register_bit & gpe_register_info->enable_for_run)) {
>  			return (AE_BAD_PARAMETER);
> +		}
> +
> +		/*lint -fallthrough */
>  
>  	case ACPI_GPE_ENABLE:
>  		ACPI_SET_BIT(enable_mask, register_bit);
> @@ -131,7 +137,7 @@ acpi_hw_low_set_gpe(struct acpi_gpe_event_info *gpe_event_info, u8 action)
>  		break;
>  
>  	default:
> -		ACPI_ERROR((AE_INFO, "Invalid action\n"));
> +		ACPI_ERROR((AE_INFO, "Invalid GPE Action, %u\n", action));
>  		return (AE_BAD_PARAMETER);
>  	}
>  
> @@ -168,13 +174,13 @@ acpi_status acpi_hw_clear_gpe(struct acpi_gpe_event_info * gpe_event_info)
>  		return (AE_NOT_EXIST);
>  	}
>  
> -	register_bit = acpi_hw_get_gpe_register_bit(gpe_event_info,
> -						gpe_register_info);
> -
>  	/*
>  	 * Write a one to the appropriate bit in the status register to
>  	 * clear this GPE.
>  	 */
> +	register_bit =
> +	    acpi_hw_get_gpe_register_bit(gpe_event_info, gpe_register_info);
> +
>  	status = acpi_hw_write(register_bit,
>  			       &gpe_register_info->status_address);
>  
> @@ -201,8 +207,8 @@ acpi_hw_get_gpe_status(struct acpi_gpe_event_info * gpe_event_info,
>  	u32 in_byte;
>  	u32 register_bit;
>  	struct acpi_gpe_register_info *gpe_register_info;
> -	acpi_status status;
>  	acpi_event_status local_event_status = 0;
> +	acpi_status status;
>  
>  	ACPI_FUNCTION_ENTRY();
>  
> 


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2010-12-09 22:33 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-12-06 15:59 [PATCH 6/6] ACPICA: Misc comments to minimize code divergence Lin Ming
2010-12-09 22:32 ` 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