Skip to content

Commit cbbc0de

Browse files
rjwysockijbarnes993
rjwysocki
authored and
jbarnes993
committed
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: Rafael J. Wysocki <rjw@sisk.pl> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
1 parent 7bc5e3f commit cbbc0de

File tree

4 files changed

+30
-22
lines changed

4 files changed

+30
-22
lines changed

‎drivers/acpi/acpica/acevents.h

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,7 @@ acpi_ev_queue_notify_request(struct acpi_namespace_node *node,
7878
acpi_status
7979
acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info);
8080

81-
acpi_status
82-
acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info,
83-
u8 write_to_hardware);
81+
acpi_status acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info);
8482

8583
acpi_status acpi_ev_disable_gpe(struct acpi_gpe_event_info *gpe_event_info);
8684

‎drivers/acpi/acpica/evgpe.c

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -98,18 +98,14 @@ acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info)
9898
* FUNCTION: acpi_ev_enable_gpe
9999
*
100100
* PARAMETERS: gpe_event_info - GPE to enable
101-
* write_to_hardware - Enable now, or just mark data structs
102-
* (WAKE GPEs should be deferred)
103101
*
104102
* RETURN: Status
105103
*
106104
* DESCRIPTION: Enable a GPE based on the GPE type
107105
*
108106
******************************************************************************/
109107

110-
acpi_status
111-
acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info,
112-
u8 write_to_hardware)
108+
acpi_status acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info)
113109
{
114110
acpi_status status;
115111

@@ -123,7 +119,7 @@ acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info,
123119

124120
/* Mark wake-enabled or HW enable, or both */
125121

126-
if (gpe_event_info->runtime_count && write_to_hardware) {
122+
if (gpe_event_info->runtime_count) {
127123
/* Clear the GPE (of stale events), then enable it */
128124
status = acpi_hw_clear_gpe(gpe_event_info);
129125
if (ACPI_FAILURE(status))
@@ -400,7 +396,7 @@ static void ACPI_SYSTEM_XFACE acpi_ev_asynch_execute_gpe_method(void *context)
400396

401397
/* Set the GPE flags for return to enabled state */
402398

403-
(void)acpi_ev_enable_gpe(gpe_event_info, FALSE);
399+
(void)acpi_ev_update_gpe_enable_masks(gpe_event_info);
404400

405401
/*
406402
* Take a snapshot of the GPE info for this level - we copy the info to

‎drivers/acpi/acpica/evxfevnt.c

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ acpi_status acpi_set_gpe(acpi_handle gpe_device, u32 gpe_number, u8 action)
235235

236236
switch (action) {
237237
case ACPI_GPE_ENABLE:
238-
status = acpi_ev_enable_gpe(gpe_event_info, TRUE);
238+
status = acpi_ev_enable_gpe(gpe_event_info);
239239
break;
240240

241241
case ACPI_GPE_DISABLE:
@@ -276,6 +276,9 @@ acpi_status acpi_enable_gpe(acpi_handle gpe_device, u32 gpe_number, u8 type)
276276

277277
ACPI_FUNCTION_TRACE(acpi_enable_gpe);
278278

279+
if (type & ~ACPI_GPE_TYPE_WAKE_RUN)
280+
return_ACPI_STATUS(AE_BAD_PARAMETER);
281+
279282
flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
280283

281284
/* 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)
287290
}
288291

289292
if (type & ACPI_GPE_TYPE_RUNTIME) {
290-
if (++gpe_event_info->runtime_count == 1)
291-
status = acpi_ev_enable_gpe(gpe_event_info, TRUE);
292-
293-
if (ACPI_FAILURE(status))
294-
gpe_event_info->runtime_count--;
293+
if (++gpe_event_info->runtime_count == 1) {
294+
status = acpi_ev_enable_gpe(gpe_event_info);
295+
if (ACPI_FAILURE(status))
296+
gpe_event_info->runtime_count--;
297+
}
295298
}
296299

297300
if (type & ACPI_GPE_TYPE_WAKE) {
@@ -335,6 +338,9 @@ acpi_status acpi_disable_gpe(acpi_handle gpe_device, u32 gpe_number, u8 type)
335338

336339
ACPI_FUNCTION_TRACE(acpi_disable_gpe);
337340

341+
if (type & ~ACPI_GPE_TYPE_WAKE_RUN)
342+
return_ACPI_STATUS(AE_BAD_PARAMETER);
343+
338344
flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
339345
/* Ensure that we have a valid GPE number */
340346

@@ -344,12 +350,12 @@ acpi_status acpi_disable_gpe(acpi_handle gpe_device, u32 gpe_number, u8 type)
344350
goto unlock_and_exit;
345351
}
346352

347-
if ((type & ACPI_GPE_TYPE_WAKE) && gpe_event_info->runtime_count) {
353+
if ((type & ACPI_GPE_TYPE_RUNTIME) && gpe_event_info->runtime_count) {
348354
if (--gpe_event_info->runtime_count == 0)
349-
acpi_ev_disable_gpe(gpe_event_info);
355+
status = acpi_ev_disable_gpe(gpe_event_info);
350356
}
351357

352-
if ((type & ACPI_GPE_TYPE_RUNTIME) && gpe_event_info->wakeup_count) {
358+
if ((type & ACPI_GPE_TYPE_WAKE) && gpe_event_info->wakeup_count) {
353359
/*
354360
* Wake-up GPEs are not enabled after leaving system sleep
355361
* states, so we don't need to disable them here.

‎drivers/acpi/ec.c

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,10 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t)
307307
pr_debug(PREFIX "transaction start\n");
308308
/* disable GPE during transaction if storm is detected */
309309
if (test_bit(EC_FLAGS_GPE_STORM, &ec->flags)) {
310+
/*
311+
* It has to be disabled at the hardware level regardless of the
312+
* GPE reference counting, so that it doesn't trigger.
313+
*/
310314
acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_DISABLE);
311315
}
312316

@@ -316,7 +320,11 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t)
316320
ec_check_sci_sync(ec, acpi_ec_read_status(ec));
317321
if (test_bit(EC_FLAGS_GPE_STORM, &ec->flags)) {
318322
msleep(1);
319-
/* it is safe to enable GPE outside of transaction */
323+
/*
324+
* It is safe to enable the GPE outside of the transaction. Use
325+
* acpi_set_gpe() for that, since we used it to disable the GPE
326+
* above.
327+
*/
320328
acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_ENABLE);
321329
} else if (t->irq_count > ACPI_EC_STORM_THRESHOLD) {
322330
pr_info(PREFIX "GPE storm detected, "
@@ -1059,15 +1067,15 @@ int __init acpi_ec_ecdt_probe(void)
10591067
static int acpi_ec_suspend(struct acpi_device *device, pm_message_t state)
10601068
{
10611069
struct acpi_ec *ec = acpi_driver_data(device);
1062-
/* Stop using GPE */
1070+
/* Stop using the GPE, but keep it reference counted. */
10631071
acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_DISABLE);
10641072
return 0;
10651073
}
10661074

10671075
static int acpi_ec_resume(struct acpi_device *device)
10681076
{
10691077
struct acpi_ec *ec = acpi_driver_data(device);
1070-
/* Enable use of GPE back */
1078+
/* Enable the GPE again, but don't reference count it once more. */
10711079
acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_ENABLE);
10721080
return 0;
10731081
}

0 commit comments

Comments
 (0)