ACPI: Use reference counting only for GPEs enabled at runtime
Subsystems (3)
cover ACPI: Use reference counting only for GPEs enabled at runtime
Hi,
The following two patches rework the GPE code so that reference counting is only used for GPEs enabled at run time.
As a result, the third argument of acpi_{enable|disable}_gpe() is removed, which simplifies the runtime GPE handling quite a bit. The "wakeup" GPEs are handled with the help of a new function that only manipulates their enable bits in their registers' enable_for_wake masks and is only called in two places in drivers/acpi/wakeup.c.
Please apply.
Rafael
PATCH 1/2 ACPI: Introduce acpi_gpe_wakeup()
From: Rafael J. Wysocki <[email protected]>
ACPICA uses reference counters to avoid disabling GPEs too early in case they have been enabled for many times. This is done separately for run time and for wakeup, but the wakeup GPE reference counter is not really necessary, because GPEs are only enabled to wake up the system at the hardware level by acpi_enter_sleep_state(). Thus it only is necessary to set the corresponding bits in the wakeup enable masks of these GPEs' registers right before the system enters a sleep state. Moreover, the GPE wakeup enable bits can only be set when the target sleep state of the system is known and they need to be cleared immediately after wakeup regardless of how many wakeup devices are associated with a given GPE.
On the basis of the above observations, introduce function acpi_gpe_wakeup() to be used for setting or clearing the enable bit corresponding to a given GPE in its enable register's enable_for_wake mask. Modify the ACPI suspend and wakeup code the use acpi_gpe_wakeup() instead of acpi_{enable|disable}_gpe() to set and clear GPE enable bits in their registers' enable_for_wake masks during system transitions to a sleep state and back to the working state, respectively. [This will allow us to drop the third argument of acpi_{enable|disable}_gpe() and simplify the GPE handling code.]
--- drivers/acpi/acpica/evxfevnt.c | 61 +++++++++++++++++++++++++++++++++++++++++ drivers/acpi/sleep.c | 15 ++-------- drivers/acpi/wakeup.c | 18 +++++++----- include/acpi/acpixf.h | 2 + include/acpi/actypes.h | 2 - 5 files changed, 78 insertions(+), 20 deletions(-)
drivers/acpi/acpica/evxfevnt.c +61 -0
@@ -208,6 +208,67 @@ acpi_status acpi_enable_event(u32 event,ACPI_EXPORT_SYMBOL (acpi_enable_event) +/****************************************************************************** +* +* FUNCTION: acpi_gpe_wakeup +* +* PARAMETERS: gpe_device - Parent GPE Device. NULL for GPE0/GPE1 +* gpe_number - GPE level within the GPE block +* action - Disable or enable +* +* RETURN: Status +* +* DESCRIPTION: Set or clear the GPE's wakeup enable mask bit. +* +******************************************************************************/ +acpi_status acpi_gpe_wakeup(acpi_handle gpe_device, u32 gpe_number, u8 action) +{ +acpi_status status =AE_OK ; +struct acpi_gpe_event_info *gpe_event_info; +struct acpi_gpe_register_info *gpe_register_info; +u32 register_bit; +acpi_cpu_flags flags; + +ACPI_FUNCTION_TRACE (acpi_gpe_wakeup); + + flags =acpi_os_acquire_lock (acpi_gbl_gpe_lock); + +/* Ensure that we have a valid GPE number */ + + gpe_event_info =acpi_ev_get_gpe_event_info (gpe_device, gpe_number); +if (!gpe_event_info) { + status =AE_BAD_PARAMETER ; + goto unlock_and_exit; + } + + gpe_register_info = gpe_event_info->register_info; +if (!gpe_register_info) { + status =AE_NOT_EXIST ; + goto unlock_and_exit; + } + + register_bit =acpi_hw_gpe_register_bit (gpe_event_info, + gpe_register_info); +switch (action) { +case ACPI_GPE_ENABLE : +ACPI_SET_BIT (gpe_register_info->enable_for_wake, register_bit); +break ; + +case ACPI_GPE_DISABLE : +ACPI_CLEAR_BIT (gpe_register_info->enable_for_wake, register_bit); +break ; + +default : +ACPI_ERROR ((AE_INFO ,"Invalid action\n" )); + status =AE_BAD_PARAMETER ; + } + +unlock_and_exit: +acpi_os_release_lock (acpi_gbl_gpe_lock, flags); +return_ACPI_STATUS (status); +} +ACPI_EXPORT_SYMBOL (acpi_gpe_wakeup) +/******************************************************************************* * * FUNCTION: acpi_clear_and_enable_gpe
include/acpi/acpixf.h +2 -0
@@ -281,6 +281,8 @@ acpi_status acpi_get_event_status(u32 ev/* * GPE Interfaces */ +acpi_status acpi_gpe_wakeup (acpi_handle gpe_device,u32 gpe_number,u8 action); +acpi_status acpi_set_gpe (acpi_handle gpe_device,u32 gpe_number,u8 action); acpi_status
drivers/acpi/sleep.c +3 -12
@@ -664,18 +664,9 @@ int acpi_pm_device_sleep_wake(struct devreturn -ENODEV ; } -if (enable) { - error =acpi_enable_wakeup_device_power (adev, - acpi_target_sleep_state); -if (!error) -acpi_enable_gpe (adev->wakeup.gpe_device, - adev->wakeup.gpe_number, -ACPI_GPE_TYPE_WAKE ); - }else { -acpi_disable_gpe (adev->wakeup.gpe_device, adev->wakeup.gpe_number, -ACPI_GPE_TYPE_WAKE ); - error =acpi_disable_wakeup_device_power (adev); - } + error = enable ? +acpi_enable_wakeup_device_power (adev, acpi_target_sleep_state) : +acpi_disable_wakeup_device_power (adev);if (!error)dev_info (dev,"wake-up capability %s by ACPI\n" , enable ?"enabled" :"disabled" );
drivers/acpi/wakeup.c +11 -7
@@ -64,13 +64,14 @@ void acpi_enable_wakeup_device(u8 sleep_struct acpi_device *dev = container_of(node, struct acpi_device, wakeup_list); -if (!dev->wakeup.flags.valid || !dev->wakeup.state.enabled +if (!dev->wakeup.flags.valid + || !(dev->wakeup.state.enabled || dev->wakeup.prepare_count) || sleep_state > (u32 ) dev->wakeup.sleep_state)continue ;/* The wake-up power should have been enabled already. */ -acpi_enable_gpe (dev->wakeup.gpe_device, dev->wakeup.gpe_number, -ACPI_GPE_TYPE_WAKE ); +acpi_gpe_wakeup (dev->wakeup.gpe_device, dev->wakeup.gpe_number, +ACPI_GPE_ENABLE ); } }
@@ -89,13 +90,16 @@ void acpi_disable_wakeup_device(u8 sleepstruct acpi_device *dev = container_of(node, struct acpi_device, wakeup_list); -if (!dev->wakeup.flags.valid || !dev->wakeup.state.enabled +if (!dev->wakeup.flags.valid + || !(dev->wakeup.state.enabled || dev->wakeup.prepare_count) || (sleep_state > (u32 ) dev->wakeup.sleep_state))continue ; -acpi_disable_gpe (dev->wakeup.gpe_device, dev->wakeup.gpe_number, -ACPI_GPE_TYPE_WAKE ); -acpi_disable_wakeup_device_power (dev); +acpi_gpe_wakeup (dev->wakeup.gpe_device, dev->wakeup.gpe_number, +ACPI_GPE_DISABLE ); + +if (dev->wakeup.state.enabled) +acpi_disable_wakeup_device_power (dev); } }
include/acpi/actypes.h +1 -1
@@ -663,7 +663,7 @@ typedef u32 acpi_event_status;#define ACPI_GPE_MAX 0xFF#define ACPI_NUM_GPE 256 -/* Actions for acpi_set_gpe and acpi_hw_low_set_gpe */ +/* Actions for acpi_set_gpe, acpi_hw_low_set_gpe and acpi_gpe_wakeup */ #define ACPI_GPE_ENABLE 0#define ACPI_GPE_DISABLE 1
PATCH 2/2 ACPI: Remove wakeup GPE reference counting which is not used
In reply to Rafael J. WysockiFrom: Rafael J. Wysocki <[email protected]>
After the previous patch that introduced acpi_gpe_wakeup() and modified the ACPI suspend and wakeup code to use it, the third argument of acpi_{enable|disable}_gpe() and the GPE wakeup reference counter are not necessary any more. Remove them and modify all of the users of acpi_{enable|disable}_gpe() accordingly. Also drop GPE type constants that aren't used any more.
--- drivers/acpi/acpica/acevents.h | 2 drivers/acpi/acpica/aclocal.h | 1 drivers/acpi/acpica/evgpe.c | 13 +--- drivers/acpi/acpica/evgpeblk.c | 3 - drivers/acpi/acpica/evgpeinit.c | 3 - drivers/acpi/acpica/evxfevnt.c | 114 ++++++++-------------------------------- drivers/acpi/button.c | 6 -- drivers/acpi/ec.c | 6 +- drivers/acpi/system.c | 6 -- drivers/pci/pci-acpi.c | 6 -- include/acpi/acpixf.h | 6 -- include/acpi/actypes.h | 6 -- 12 files changed, 43 insertions(+), 129 deletions(-)
drivers/acpi/acpica/acevents.h +1 -1
@@ -78,7 +78,7 @@ acpi_ev_queue_notify_request(struct acpiu32 acpi_ev_gpe_detect (struct acpi_gpe_xrupt_info *gpe_xrupt_list);acpi_status -acpi_ev_update_gpe_enable_masks (struct acpi_gpe_event_info *gpe_event_info); +acpi_ev_update_gpe_enable_mask (struct acpi_gpe_event_info *gpe_event_info);struct acpi_gpe_event_info *acpi_ev_get_gpe_event_info (acpi_handle gpe_device,u32 gpe_number);
drivers/acpi/acpica/evgpe.c +4 -9
@@ -54,7 +54,7 @@ static void ACPI_SYSTEM_XFACE acpi_ev_as/******************************************************************************* * -* FUNCTION: acpi_ev_update_gpe_enable_masks +* FUNCTION: acpi_ev_update_gpe_enable_mask * * PARAMETERS: gpe_event_info - GPE to update *
@@ -66,7 +66,7 @@ static void ACPI_SYSTEM_XFACE acpi_ev_as******************************************************************************/ acpi_status -acpi_ev_update_gpe_enable_masks (struct acpi_gpe_event_info *gpe_event_info) +acpi_ev_update_gpe_enable_mask (struct acpi_gpe_event_info *gpe_event_info) {struct acpi_gpe_register_info *gpe_register_info;u32 register_bit;
@@ -81,21 +81,16 @@ acpi_ev_update_gpe_enable_masks(struct a register_bit =acpi_hw_gpe_register_bit (gpe_event_info, gpe_register_info); -/* Clear the wake/run bits up front */ +/* Clear the run bit up front */ -ACPI_CLEAR_BIT (gpe_register_info->enable_for_wake, register_bit);ACPI_CLEAR_BIT (gpe_register_info->enable_for_run, register_bit); -/* Set the mask bits only if there are references to this GPE */ +/* Set the mask bit only if there are references to this GPE */ if (gpe_event_info->runtime_count) {ACPI_SET_BIT (gpe_register_info->enable_for_run, register_bit); } -if (gpe_event_info->wakeup_count) { -ACPI_SET_BIT (gpe_register_info->enable_for_wake, register_bit); - } -return_ACPI_STATUS (AE_OK ); }
drivers/acpi/acpica/evxfevnt.c +25 -89
170 lines not on this page · raw message
include/acpi/actypes.h +0 -6
@@ -669,12 +669,6 @@ typedef u32 acpi_event_status;#define ACPI_GPE_DISABLE 1#define ACPI_GPE_COND_ENABLE 2 -/* gpe_types for acpi_enable_gpe and acpi_disable_gpe */ - -#define ACPI_GPE_TYPE_WAKE (u8) 0x01 -#define ACPI_GPE_TYPE_RUNTIME (u8) 0x02 -#define ACPI_GPE_TYPE_WAKE_RUN (u8) 0x03 -/* * GPE info flags - Per GPE * +-------+---+-+-+
drivers/acpi/ec.c +3 -3
@@ -822,7 +822,7 @@ static int ec_install_handlers(struct acif (ACPI_FAILURE (status))return -ENODEV ; -acpi_enable_gpe (NULL , ec->gpe,ACPI_GPE_TYPE_RUNTIME ); +acpi_enable_gpe (NULL , ec->gpe); status = acpi_install_address_space_handler(ec->handle, ACPI_ADR_SPACE_EC, &acpi_ec_space_handler,
@@ -839,7 +839,7 @@ static int ec_install_handlers(struct ac }else {acpi_remove_gpe_handler (NULL , ec->gpe, &acpi_ec_gpe_handler); -acpi_disable_gpe (NULL , ec->gpe,ACPI_GPE_TYPE_RUNTIME ); +acpi_disable_gpe (NULL , ec->gpe);return -ENODEV ; } }
@@ -850,7 +850,7 @@ static int ec_install_handlers(struct acstatic void ec_remove_handlers (struct acpi_ec *ec) { -acpi_disable_gpe (NULL , ec->gpe,ACPI_GPE_TYPE_RUNTIME ); +acpi_disable_gpe (NULL , ec->gpe);if (ACPI_FAILURE (acpi_remove_address_space_handler (ec->handle,ACPI_ADR_SPACE_EC , &acpi_ec_space_handler)))pr_err (PREFIX "failed to remove space handler\n" );
drivers/acpi/system.c +2 -4
@@ -388,12 +388,10 @@ static ssize_t counter_set(struct kobjecif (index < num_gpes) {if (!strcmp (buf,"disable\n" ) && (status &ACPI_EVENT_FLAG_ENABLED )) - result =acpi_disable_gpe (handle, index, -ACPI_GPE_TYPE_RUNTIME ); + result =acpi_disable_gpe (handle, index);else if (!strcmp (buf,"enable\n" ) && !(status &ACPI_EVENT_FLAG_ENABLED )) - result =acpi_enable_gpe (handle, index, -ACPI_GPE_TYPE_RUNTIME ); + result =acpi_enable_gpe (handle, index);else if (!strcmp (buf,"clear\n" ) && (status &ACPI_EVENT_FLAG_SET )) result =acpi_clear_gpe (handle, index);
drivers/acpi/acpica/evgpeinit.c +1 -2
@@ -482,8 +482,7 @@ acpi_ev_match_gpe_method(acpi_handle obj gpe_device =NULL ; } - status =acpi_enable_gpe (gpe_device, gpe_number, -ACPI_GPE_TYPE_RUNTIME ); + status =acpi_enable_gpe (gpe_device, gpe_number);if (ACPI_FAILURE (status)) { ACPI_EXCEPTION((AE_INFO, status, "Could not enable GPE 0x%02X",
drivers/acpi/acpica/evgpeblk.c +1 -2
@@ -529,8 +529,7 @@ acpi_ev_initialize_gpe_block(struct acpi/* Enable this GPE */ - status =acpi_enable_gpe (gpe_device, gpe_number, -ACPI_GPE_TYPE_RUNTIME ); + status =acpi_enable_gpe (gpe_device, gpe_number);if (ACPI_FAILURE (status)) { ACPI_EXCEPTION((AE_INFO, status, "Could not enable GPE 0x%02X",
drivers/acpi/button.c +2 -4
@@ -424,8 +424,7 @@ static int acpi_button_add(struct acpi_dif (device->wakeup.flags.valid) {/* Button's GPE is run-wake GPE */ acpi_enable_gpe (device->wakeup.gpe_device, - device->wakeup.gpe_number, -ACPI_GPE_TYPE_RUNTIME ); + device->wakeup.gpe_number); device->wakeup.run_wake_count++; device->wakeup.state.enabled =1 ; }
@@ -448,8 +447,7 @@ static int acpi_button_remove(struct acpif (device->wakeup.flags.valid) {acpi_disable_gpe (device->wakeup.gpe_device, - device->wakeup.gpe_number, -ACPI_GPE_TYPE_RUNTIME ); + device->wakeup.gpe_number); device->wakeup.run_wake_count--; device->wakeup.state.enabled =0 ; }
drivers/pci/pci-acpi.c +2 -4
@@ -297,14 +297,12 @@ static int acpi_dev_run_wake(struct deviif (!dev->wakeup.run_wake_count++) {acpi_enable_wakeup_device_power (dev,ACPI_STATE_S0 );acpi_enable_gpe (dev->wakeup.gpe_device, - dev->wakeup.gpe_number, -ACPI_GPE_TYPE_RUNTIME ); + dev->wakeup.gpe_number); } }else if (dev->wakeup.run_wake_count >0 ) {if (!--dev->wakeup.run_wake_count) {acpi_disable_gpe (dev->wakeup.gpe_device, - dev->wakeup.gpe_number, -ACPI_GPE_TYPE_RUNTIME ); + dev->wakeup.gpe_number);acpi_disable_wakeup_device_power (dev); } }else {
drivers/acpi/acpica/aclocal.h +0 -1
@@ -428,7 +428,6 @@ struct acpi_gpe_event_info {u8 flags;/* Misc info about this GPE */ u8 gpe_number;/* This GPE */ u8 runtime_count;/* References to a run GPE */ -u8 wakeup_count;/* References to a wake GPE */ };/* Information about a GPE register pair, one per each status/enable pair in an array */
include/acpi/acpixf.h +2 -4
@@ -285,11 +285,9 @@ acpi_status acpi_gpe_wakeup(acpi_handleacpi_status acpi_set_gpe (acpi_handle gpe_device,u32 gpe_number,u8 action); -acpi_status -acpi_enable_gpe (acpi_handle gpe_device,u32 gpe_number,u8 gpe_type); +acpi_status acpi_enable_gpe (acpi_handle gpe_device,u32 gpe_number); -acpi_status -acpi_disable_gpe (acpi_handle gpe_device,u32 gpe_number,u8 gpe_type); +acpi_status acpi_disable_gpe (acpi_handle gpe_device,u32 gpe_number);acpi_status acpi_clear_gpe (acpi_handle gpe_device,u32 gpe_number);