From: Lin Ming <ming.m.lin@intel.com>
To: "Rafael J. Wysocki" <rjw@sisk.pl>, Len Brown <lenb@kernel.org>,
Bob Moore <robert.moore@intel.com>
Cc: linux-acpi@vger.kernel.org
Subject: [PATCH 6/6] ACPICA: Misc comments to minimize code divergence
Date: Mon, 06 Dec 2010 23:59:17 +0800 [thread overview]
Message-ID: <1291651157.2197.88.camel@localhost> (raw)
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
next reply other threads:[~2010-12-06 15:58 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-12-06 15:59 Lin Ming [this message]
2010-12-09 22:32 ` [PATCH 6/6] ACPICA: Misc comments to minimize code divergence Rafael J. Wysocki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1291651157.2197.88.camel@localhost \
--to=ming.m.lin@intel.com \
--cc=lenb@kernel.org \
--cc=linux-acpi@vger.kernel.org \
--cc=rjw@sisk.pl \
--cc=robert.moore@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.