Commit cbbc0de7 authored by Rafael J. Wysocki's avatar Rafael J. Wysocki Committed by Jesse Barnes

ACPI: Use GPE reference counting to support shared GPEs

To fix a bug and address the reviewers' comments regarding the ACPI
GPE refcounting patch, do the following additional changes:

o Remove the second argument of acpi_ev_enable_gpe(),
  'write_to_hardware', because it is not necessary any more.

o Add the "bad parameter" test against 'type' in
  acpi_enable_gpe() and acpi_disable_gpe().

o Make acpi_enable_gpe() only check 'status' for runtime GPEs if
  acpi_ev_enable_gpe() was actually called.

o Make acpi_disable_gpe() return 'status' returned by
  acpi_ev_disable_gpe() and fix a bug where ACPI_GPE_TYPE_WAKE
  and ACPI_GPE_TYPE_RUNTIME were exchanged by mistake.

o Add comments explaining why acpi_set_gpe() is used by the ACPI EC
  driver.
Signed-off-by: default avatarRafael J. Wysocki <rjw@sisk.pl>
Signed-off-by: default avatarJesse Barnes <jbarnes@virtuousgeek.org>
parent 7bc5e3f2
...@@ -78,9 +78,7 @@ acpi_ev_queue_notify_request(struct acpi_namespace_node *node, ...@@ -78,9 +78,7 @@ acpi_ev_queue_notify_request(struct acpi_namespace_node *node,
acpi_status acpi_status
acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info); acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info);
acpi_status acpi_status acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info);
acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info,
u8 write_to_hardware);
acpi_status acpi_ev_disable_gpe(struct acpi_gpe_event_info *gpe_event_info); acpi_status acpi_ev_disable_gpe(struct acpi_gpe_event_info *gpe_event_info);
......
...@@ -98,8 +98,6 @@ acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info) ...@@ -98,8 +98,6 @@ acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info)
* FUNCTION: acpi_ev_enable_gpe * FUNCTION: acpi_ev_enable_gpe
* *
* PARAMETERS: gpe_event_info - GPE to enable * PARAMETERS: gpe_event_info - GPE to enable
* write_to_hardware - Enable now, or just mark data structs
* (WAKE GPEs should be deferred)
* *
* RETURN: Status * RETURN: Status
* *
...@@ -107,9 +105,7 @@ acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info) ...@@ -107,9 +105,7 @@ acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info)
* *
******************************************************************************/ ******************************************************************************/
acpi_status acpi_status acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info)
acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info,
u8 write_to_hardware)
{ {
acpi_status status; acpi_status status;
...@@ -123,7 +119,7 @@ acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info, ...@@ -123,7 +119,7 @@ acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info,
/* Mark wake-enabled or HW enable, or both */ /* Mark wake-enabled or HW enable, or both */
if (gpe_event_info->runtime_count && write_to_hardware) { if (gpe_event_info->runtime_count) {
/* Clear the GPE (of stale events), then enable it */ /* Clear the GPE (of stale events), then enable it */
status = acpi_hw_clear_gpe(gpe_event_info); status = acpi_hw_clear_gpe(gpe_event_info);
if (ACPI_FAILURE(status)) if (ACPI_FAILURE(status))
...@@ -400,7 +396,7 @@ static void ACPI_SYSTEM_XFACE acpi_ev_asynch_execute_gpe_method(void *context) ...@@ -400,7 +396,7 @@ static void ACPI_SYSTEM_XFACE acpi_ev_asynch_execute_gpe_method(void *context)
/* Set the GPE flags for return to enabled state */ /* Set the GPE flags for return to enabled state */
(void)acpi_ev_enable_gpe(gpe_event_info, FALSE); (void)acpi_ev_update_gpe_enable_masks(gpe_event_info);
/* /*
* Take a snapshot of the GPE info for this level - we copy the info to * Take a snapshot of the GPE info for this level - we copy the info to
......
...@@ -235,7 +235,7 @@ acpi_status acpi_set_gpe(acpi_handle gpe_device, u32 gpe_number, u8 action) ...@@ -235,7 +235,7 @@ acpi_status acpi_set_gpe(acpi_handle gpe_device, u32 gpe_number, u8 action)
switch (action) { switch (action) {
case ACPI_GPE_ENABLE: case ACPI_GPE_ENABLE:
status = acpi_ev_enable_gpe(gpe_event_info, TRUE); status = acpi_ev_enable_gpe(gpe_event_info);
break; break;
case ACPI_GPE_DISABLE: case ACPI_GPE_DISABLE:
...@@ -276,6 +276,9 @@ acpi_status acpi_enable_gpe(acpi_handle gpe_device, u32 gpe_number, u8 type) ...@@ -276,6 +276,9 @@ acpi_status acpi_enable_gpe(acpi_handle gpe_device, u32 gpe_number, u8 type)
ACPI_FUNCTION_TRACE(acpi_enable_gpe); ACPI_FUNCTION_TRACE(acpi_enable_gpe);
if (type & ~ACPI_GPE_TYPE_WAKE_RUN)
return_ACPI_STATUS(AE_BAD_PARAMETER);
flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock); 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 */
...@@ -287,11 +290,11 @@ acpi_status acpi_enable_gpe(acpi_handle gpe_device, u32 gpe_number, u8 type) ...@@ -287,11 +290,11 @@ acpi_status acpi_enable_gpe(acpi_handle gpe_device, u32 gpe_number, u8 type)
} }
if (type & ACPI_GPE_TYPE_RUNTIME) { if (type & ACPI_GPE_TYPE_RUNTIME) {
if (++gpe_event_info->runtime_count == 1) if (++gpe_event_info->runtime_count == 1) {
status = acpi_ev_enable_gpe(gpe_event_info, TRUE); status = acpi_ev_enable_gpe(gpe_event_info);
if (ACPI_FAILURE(status))
if (ACPI_FAILURE(status)) gpe_event_info->runtime_count--;
gpe_event_info->runtime_count--; }
} }
if (type & ACPI_GPE_TYPE_WAKE) { if (type & ACPI_GPE_TYPE_WAKE) {
...@@ -335,6 +338,9 @@ acpi_status acpi_disable_gpe(acpi_handle gpe_device, u32 gpe_number, u8 type) ...@@ -335,6 +338,9 @@ acpi_status acpi_disable_gpe(acpi_handle gpe_device, u32 gpe_number, u8 type)
ACPI_FUNCTION_TRACE(acpi_disable_gpe); ACPI_FUNCTION_TRACE(acpi_disable_gpe);
if (type & ~ACPI_GPE_TYPE_WAKE_RUN)
return_ACPI_STATUS(AE_BAD_PARAMETER);
flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock); 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 */
...@@ -344,12 +350,12 @@ acpi_status acpi_disable_gpe(acpi_handle gpe_device, u32 gpe_number, u8 type) ...@@ -344,12 +350,12 @@ acpi_status acpi_disable_gpe(acpi_handle gpe_device, u32 gpe_number, u8 type)
goto unlock_and_exit; goto unlock_and_exit;
} }
if ((type & ACPI_GPE_TYPE_WAKE) && gpe_event_info->runtime_count) { if ((type & ACPI_GPE_TYPE_RUNTIME) && gpe_event_info->runtime_count) {
if (--gpe_event_info->runtime_count == 0) if (--gpe_event_info->runtime_count == 0)
acpi_ev_disable_gpe(gpe_event_info); status = acpi_ev_disable_gpe(gpe_event_info);
} }
if ((type & ACPI_GPE_TYPE_RUNTIME) && gpe_event_info->wakeup_count) { if ((type & ACPI_GPE_TYPE_WAKE) && gpe_event_info->wakeup_count) {
/* /*
* Wake-up GPEs are not enabled after leaving system sleep * Wake-up GPEs are not enabled after leaving system sleep
* states, so we don't need to disable them here. * states, so we don't need to disable them here.
......
...@@ -307,6 +307,10 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t) ...@@ -307,6 +307,10 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t)
pr_debug(PREFIX "transaction start\n"); pr_debug(PREFIX "transaction start\n");
/* disable GPE during transaction if storm is detected */ /* disable GPE during transaction if storm is detected */
if (test_bit(EC_FLAGS_GPE_STORM, &ec->flags)) { if (test_bit(EC_FLAGS_GPE_STORM, &ec->flags)) {
/*
* It has to be disabled at the hardware level regardless of the
* GPE reference counting, so that it doesn't trigger.
*/
acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_DISABLE); acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_DISABLE);
} }
...@@ -316,7 +320,11 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t) ...@@ -316,7 +320,11 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t)
ec_check_sci_sync(ec, acpi_ec_read_status(ec)); ec_check_sci_sync(ec, acpi_ec_read_status(ec));
if (test_bit(EC_FLAGS_GPE_STORM, &ec->flags)) { if (test_bit(EC_FLAGS_GPE_STORM, &ec->flags)) {
msleep(1); msleep(1);
/* it is safe to enable GPE outside of transaction */ /*
* It is safe to enable the GPE outside of the transaction. Use
* acpi_set_gpe() for that, since we used it to disable the GPE
* above.
*/
acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_ENABLE); acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_ENABLE);
} else if (t->irq_count > ACPI_EC_STORM_THRESHOLD) { } else if (t->irq_count > ACPI_EC_STORM_THRESHOLD) {
pr_info(PREFIX "GPE storm detected, " pr_info(PREFIX "GPE storm detected, "
...@@ -1059,7 +1067,7 @@ error: ...@@ -1059,7 +1067,7 @@ error:
static int acpi_ec_suspend(struct acpi_device *device, pm_message_t state) static int acpi_ec_suspend(struct acpi_device *device, pm_message_t state)
{ {
struct acpi_ec *ec = acpi_driver_data(device); struct acpi_ec *ec = acpi_driver_data(device);
/* Stop using GPE */ /* Stop using the GPE, but keep it reference counted. */
acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_DISABLE); acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_DISABLE);
return 0; return 0;
} }
...@@ -1067,7 +1075,7 @@ static int acpi_ec_suspend(struct acpi_device *device, pm_message_t state) ...@@ -1067,7 +1075,7 @@ static int acpi_ec_suspend(struct acpi_device *device, pm_message_t state)
static int acpi_ec_resume(struct acpi_device *device) static int acpi_ec_resume(struct acpi_device *device)
{ {
struct acpi_ec *ec = acpi_driver_data(device); struct acpi_ec *ec = acpi_driver_data(device);
/* Enable use of GPE back */ /* Enable the GPE again, but don't reference count it once more. */
acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_ENABLE); acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_ENABLE);
return 0; return 0;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment