| Message ID | 1251915173-10144-1-git-send-email-mjg@redhat.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Wednesday 02 September 2009, Matthew Garrett wrote: > ACPI GPEs may map to multiple devices. The current GPE interface only > provides a mechanism for enabling and disabling GPEs, making it difficult > to change the state of GPEs at runtime without extensive cooperation > between devices. Add an API to allow devices to indicate whether or not > they want their device's GPE to be enabled for both runtime and wakeup > events. Looks fine in general, but are we going to do anything about unbalanced puts? Rafael > Signed-off-by: Matthew Garrett <mjg@redhat.com> > --- > drivers/acpi/acpica/aclocal.h | 2 + > drivers/acpi/acpica/evxfevnt.c | 161 ++++++++++++++++++++++++++++++++++++++++ > include/acpi/acpixf.h | 8 ++ > 3 files changed, 171 insertions(+), 0 deletions(-) > > diff --git a/drivers/acpi/acpica/aclocal.h b/drivers/acpi/acpica/aclocal.h > index ee986ed..935e351 100644 > --- a/drivers/acpi/acpica/aclocal.h > +++ b/drivers/acpi/acpica/aclocal.h > @@ -413,6 +413,8 @@ struct acpi_gpe_event_info { > struct acpi_gpe_register_info *register_info; /* Backpointer to register info */ > u8 flags; /* Misc info about this GPE */ > u8 gpe_number; /* This GPE */ > + u8 runtime_count; > + u8 wakeup_count; > }; > > /* Information about a GPE register pair, one per each status/enable pair in an array */ > diff --git a/drivers/acpi/acpica/evxfevnt.c b/drivers/acpi/acpica/evxfevnt.c > index 4721f58..2a98818 100644 > --- a/drivers/acpi/acpica/evxfevnt.c > +++ b/drivers/acpi/acpica/evxfevnt.c > @@ -201,6 +201,167 @@ ACPI_EXPORT_SYMBOL(acpi_enable_event) > > /******************************************************************************* > * > + * FUNCTION: acpi_get_runtime_gpe > + * > + * PARAMETERS: gpe_device - Parent GPE Device > + * gpe_number - GPE level within the GPE block > + * > + * RETURN: Status > + * > + * DESCRIPTION: Take a reference to a runtime GPE > + * > + ******************************************************************************/ > +acpi_status acpi_get_runtime_gpe(acpi_handle gpe_device, u32 gpe_number) > +{ > + acpi_status status = AE_OK; > + acpi_cpu_flags flags; > + struct acpi_gpe_event_info *gpe_event_info; > + > + ACPI_FUNCTION_TRACE(acpi_get_runtime_gpe); > + > + 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; > + } > + > + if (++gpe_event_info->runtime_count == 1) > + status = acpi_ev_enable_gpe(gpe_event_info, TRUE); > + > + if (ACPI_FAILURE(status)) > + gpe_event_info->runtime_count--; > + > +unlock_and_exit: > + acpi_os_release_lock(acpi_gbl_gpe_lock, flags); > + return_ACPI_STATUS(status); > +} > +ACPI_EXPORT_SYMBOL(acpi_get_runtime_gpe) > + > +/******************************************************************************* > + * > + * FUNCTION: acpi_put_runtime_gpe > + * > + * PARAMETERS: gpe_device - Parent GPE Device > + * gpe_number - GPE level within the GPE block > + * > + * RETURN: Status > + * > + * DESCRIPTION: Release a reference to a runtime GPE > + * > + ******************************************************************************/ > +acpi_status acpi_put_runtime_gpe(acpi_handle gpe_device, u32 gpe_number) > +{ > + acpi_status status = AE_OK; > + acpi_cpu_flags flags; > + struct acpi_gpe_event_info *gpe_event_info; > + > + ACPI_FUNCTION_TRACE(acpi_put_runtime_gpe); > + > + 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; > + } > + > + if (--gpe_event_info->runtime_count == 0) > + acpi_ev_disable_gpe(gpe_event_info); > + > +unlock_and_exit: > + acpi_os_release_lock(acpi_gbl_gpe_lock, flags); > + return_ACPI_STATUS(status); > +} > +ACPI_EXPORT_SYMBOL(acpi_put_runtime_gpe) > + > +/******************************************************************************* > + * > + * FUNCTION: acpi_get_wakeup_gpe > + * > + * PARAMETERS: gpe_device - Parent GPE Device > + * gpe_number - GPE level within the GPE block > + * > + * RETURN: Status > + * > + * DESCRIPTION: Take a reference to a wakeup GPE > + * > + ******************************************************************************/ > +acpi_status acpi_get_wakeup_gpe(acpi_handle gpe_device, u32 gpe_number) > +{ > + acpi_status status = AE_OK; > + acpi_cpu_flags flags; > + struct acpi_gpe_event_info *gpe_event_info; > + > + ACPI_FUNCTION_TRACE(acpi_get_wakeup_gpe); > + > + 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; > + } > + > + if (++gpe_event_info->wakeup_count == 1) > + acpi_ev_update_gpe_enable_masks(gpe_event_info, > + ACPI_GPE_ENABLE); > + > +unlock_and_exit: > + acpi_os_release_lock(acpi_gbl_gpe_lock, flags); > + return_ACPI_STATUS(status); > +} > +ACPI_EXPORT_SYMBOL(acpi_get_wakeup_gpe) > + > +/******************************************************************************* > + * > + * FUNCTION: acpi_put_wakeup_gpe > + * > + * PARAMETERS: gpe_device - Parent GPE Device > + * gpe_number - GPE level within the GPE block > + * > + * RETURN: Status > + * > + * DESCRIPTION: Release a reference to a wakeup GPE > + * > + ******************************************************************************/ > +acpi_status acpi_put_wakeup_gpe(acpi_handle gpe_device, u32 gpe_number) > +{ > + acpi_status status = AE_OK; > + acpi_cpu_flags flags; > + struct acpi_gpe_event_info *gpe_event_info; > + > + ACPI_FUNCTION_TRACE(acpi_put_wakeup_gpe); > + > + 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; > + } > + > + if (--gpe_event_info->wakeup_count == 0) > + acpi_ev_update_gpe_enable_masks(gpe_event_info, > + ACPI_GPE_DISABLE); > + > +unlock_and_exit: > + acpi_os_release_lock(acpi_gbl_gpe_lock, flags); > + return_ACPI_STATUS(status); > +} > +ACPI_EXPORT_SYMBOL(acpi_put_wakeup_gpe) > + > +/******************************************************************************* > + * > * FUNCTION: acpi_set_gpe_type > * > * PARAMETERS: gpe_device - Parent GPE Device > diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h > index 82ec6a3..671e699 100644 > --- a/include/acpi/acpixf.h > +++ b/include/acpi/acpixf.h > @@ -280,6 +280,14 @@ acpi_status acpi_get_event_status(u32 event, acpi_event_status * event_status); > */ > acpi_status acpi_set_gpe_type(acpi_handle gpe_device, u32 gpe_number, u8 type); > > +acpi_status acpi_get_runtime_gpe(acpi_handle gpe_device, u32 gpe_number); > + > +acpi_status acpi_put_runtime_gpe(acpi_handle gpe_device, u32 gpe_number); > + > +acpi_status acpi_get_wakeup_gpe(acpi_handle gpe_device, u32 gpe_number); > + > +acpi_status acpi_put_wakeup_gpe(acpi_handle gpe_device, u32 gpe_number); > + > acpi_status acpi_enable_gpe(acpi_handle gpe_device, u32 gpe_number); > > acpi_status acpi_disable_gpe(acpi_handle gpe_device, u32 gpe_number); > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
I have a couple of comments and questions: 1) The core issue that needs to be solved concerns enabling/disabling the wake and wake/run GPEs, is this correct? As far as I can tell, the normal runtime GPEs do not need any change. The runtime GPEs are mostly defined by _Lxx/_Exx methods. The main exception to this is the EC, it has a GPE defined by the _GPE method under the EC device. Are there any issues here? 2) A large change that goes unmentioned is that the ACPICA core would no longer execute the _PRW control methods in order to discover the wake GPEs. This makes sense to me because it is clear that the bus scan code must execute these _PRW methods anyway, in order to obtain the wake GPE and other info. So it makes sense to just remove the _PRW and wake GPE management code from ACPICA and let the host OS and drivers handle this task, since it does it already anyway. Do I understand this correctly? Thanks, Bob >-----Original Message----- >From: Rafael J. Wysocki [mailto:rjw@sisk.pl] >Sent: Tuesday, September 15, 2009 2:31 PM >To: Matthew Garrett >Cc: Moore, Robert; linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org >Subject: Re: [PATCH 1/3] ACPI: Add infrastructure for refcounting GPE >consumers > >On Wednesday 02 September 2009, Matthew Garrett wrote: >> ACPI GPEs may map to multiple devices. The current GPE interface only >> provides a mechanism for enabling and disabling GPEs, making it difficult >> to change the state of GPEs at runtime without extensive cooperation >> between devices. Add an API to allow devices to indicate whether or not >> they want their device's GPE to be enabled for both runtime and wakeup >> events. > >Looks fine in general, but are we going to do anything about unbalanced >puts? > >Rafael > -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/acpi/acpica/aclocal.h b/drivers/acpi/acpica/aclocal.h index ee986ed..935e351 100644 --- a/drivers/acpi/acpica/aclocal.h +++ b/drivers/acpi/acpica/aclocal.h @@ -413,6 +413,8 @@ struct acpi_gpe_event_info { struct acpi_gpe_register_info *register_info; /* Backpointer to register info */ u8 flags; /* Misc info about this GPE */ u8 gpe_number; /* This GPE */ + u8 runtime_count; + u8 wakeup_count; }; /* Information about a GPE register pair, one per each status/enable pair in an array */ diff --git a/drivers/acpi/acpica/evxfevnt.c b/drivers/acpi/acpica/evxfevnt.c index 4721f58..2a98818 100644 --- a/drivers/acpi/acpica/evxfevnt.c +++ b/drivers/acpi/acpica/evxfevnt.c @@ -201,6 +201,167 @@ ACPI_EXPORT_SYMBOL(acpi_enable_event) /******************************************************************************* * + * FUNCTION: acpi_get_runtime_gpe + * + * PARAMETERS: gpe_device - Parent GPE Device + * gpe_number - GPE level within the GPE block + * + * RETURN: Status + * + * DESCRIPTION: Take a reference to a runtime GPE + * + ******************************************************************************/ +acpi_status acpi_get_runtime_gpe(acpi_handle gpe_device, u32 gpe_number) +{ + acpi_status status = AE_OK; + acpi_cpu_flags flags; + struct acpi_gpe_event_info *gpe_event_info; + + ACPI_FUNCTION_TRACE(acpi_get_runtime_gpe); + + 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; + } + + if (++gpe_event_info->runtime_count == 1) + status = acpi_ev_enable_gpe(gpe_event_info, TRUE); + + if (ACPI_FAILURE(status)) + gpe_event_info->runtime_count--; + +unlock_and_exit: + acpi_os_release_lock(acpi_gbl_gpe_lock, flags); + return_ACPI_STATUS(status); +} +ACPI_EXPORT_SYMBOL(acpi_get_runtime_gpe) + +/******************************************************************************* + * + * FUNCTION: acpi_put_runtime_gpe + * + * PARAMETERS: gpe_device - Parent GPE Device + * gpe_number - GPE level within the GPE block + * + * RETURN: Status + * + * DESCRIPTION: Release a reference to a runtime GPE + * + ******************************************************************************/ +acpi_status acpi_put_runtime_gpe(acpi_handle gpe_device, u32 gpe_number) +{ + acpi_status status = AE_OK; + acpi_cpu_flags flags; + struct acpi_gpe_event_info *gpe_event_info; + + ACPI_FUNCTION_TRACE(acpi_put_runtime_gpe); + + 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; + } + + if (--gpe_event_info->runtime_count == 0) + acpi_ev_disable_gpe(gpe_event_info); + +unlock_and_exit: + acpi_os_release_lock(acpi_gbl_gpe_lock, flags); + return_ACPI_STATUS(status); +} +ACPI_EXPORT_SYMBOL(acpi_put_runtime_gpe) + +/******************************************************************************* + * + * FUNCTION: acpi_get_wakeup_gpe + * + * PARAMETERS: gpe_device - Parent GPE Device + * gpe_number - GPE level within the GPE block + * + * RETURN: Status + * + * DESCRIPTION: Take a reference to a wakeup GPE + * + ******************************************************************************/ +acpi_status acpi_get_wakeup_gpe(acpi_handle gpe_device, u32 gpe_number) +{ + acpi_status status = AE_OK; + acpi_cpu_flags flags; + struct acpi_gpe_event_info *gpe_event_info; + + ACPI_FUNCTION_TRACE(acpi_get_wakeup_gpe); + + 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; + } + + if (++gpe_event_info->wakeup_count == 1) + acpi_ev_update_gpe_enable_masks(gpe_event_info, + ACPI_GPE_ENABLE); + +unlock_and_exit: + acpi_os_release_lock(acpi_gbl_gpe_lock, flags); + return_ACPI_STATUS(status); +} +ACPI_EXPORT_SYMBOL(acpi_get_wakeup_gpe) + +/******************************************************************************* + * + * FUNCTION: acpi_put_wakeup_gpe + * + * PARAMETERS: gpe_device - Parent GPE Device + * gpe_number - GPE level within the GPE block + * + * RETURN: Status + * + * DESCRIPTION: Release a reference to a wakeup GPE + * + ******************************************************************************/ +acpi_status acpi_put_wakeup_gpe(acpi_handle gpe_device, u32 gpe_number) +{ + acpi_status status = AE_OK; + acpi_cpu_flags flags; + struct acpi_gpe_event_info *gpe_event_info; + + ACPI_FUNCTION_TRACE(acpi_put_wakeup_gpe); + + 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; + } + + if (--gpe_event_info->wakeup_count == 0) + acpi_ev_update_gpe_enable_masks(gpe_event_info, + ACPI_GPE_DISABLE); + +unlock_and_exit: + acpi_os_release_lock(acpi_gbl_gpe_lock, flags); + return_ACPI_STATUS(status); +} +ACPI_EXPORT_SYMBOL(acpi_put_wakeup_gpe) + +/******************************************************************************* + * * FUNCTION: acpi_set_gpe_type * * PARAMETERS: gpe_device - Parent GPE Device diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h index 82ec6a3..671e699 100644 --- a/include/acpi/acpixf.h +++ b/include/acpi/acpixf.h @@ -280,6 +280,14 @@ acpi_status acpi_get_event_status(u32 event, acpi_event_status * event_status); */ acpi_status acpi_set_gpe_type(acpi_handle gpe_device, u32 gpe_number, u8 type); +acpi_status acpi_get_runtime_gpe(acpi_handle gpe_device, u32 gpe_number); + +acpi_status acpi_put_runtime_gpe(acpi_handle gpe_device, u32 gpe_number); + +acpi_status acpi_get_wakeup_gpe(acpi_handle gpe_device, u32 gpe_number); + +acpi_status acpi_put_wakeup_gpe(acpi_handle gpe_device, u32 gpe_number); + acpi_status acpi_enable_gpe(acpi_handle gpe_device, u32 gpe_number); acpi_status acpi_disable_gpe(acpi_handle gpe_device, u32 gpe_number);
ACPI GPEs may map to multiple devices. The current GPE interface only provides a mechanism for enabling and disabling GPEs, making it difficult to change the state of GPEs at runtime without extensive cooperation between devices. Add an API to allow devices to indicate whether or not they want their device's GPE to be enabled for both runtime and wakeup events. Signed-off-by: Matthew Garrett <mjg@redhat.com> --- drivers/acpi/acpica/aclocal.h | 2 + drivers/acpi/acpica/evxfevnt.c | 161 ++++++++++++++++++++++++++++++++++++++++ include/acpi/acpixf.h | 8 ++ 3 files changed, 171 insertions(+), 0 deletions(-)