* Recent GPE patches - some questions. @ 2010-01-29 20:30 Moore, Robert 2010-02-01 22:36 ` Matthew Garrett 0 siblings, 1 reply; 20+ messages in thread From: Moore, Robert @ 2010-01-29 20:30 UTC (permalink / raw) To: Matthew Garrett, linux-acpi@vger.kernel.org, rjw@sisk.pl Matthew, I've looked at the new GPE code and I have some comments and questions. I'm still looking at Patch 3/3, "Remove old GPE API and transition code entirely to new one", but here are the questions I have so far. Thanks, Bob [PATCH 0/3] acpica: Rewrite GPE handling Overall, can we say that this change to the GPE code is primarily intended to add better support for shared GPEs? I see that the execution of _PRW methods is no longer removed. This was a big question I had about the original patch. [PATCH 1/3] ACPI: Add infrastructure for refcounting GPE consumers: >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. I'm not sure which interface you are referring to, please explain [PATCH 2/3] ACPI: Add support for new refcounted GPE API to drivers: Reference count mechanism is meant to: 1) Enable GPE only on the first added reference 2) Disable GPE only on the last removed reference Then I don't understand why the code below needs to call acpi_enable_gpe: acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number); + acpi_ref_wakeup_gpe(dev->wakeup.gpe_device, + dev->wakeup.gpe_number); Because the GPE will be "enabled" (mask bit set for wakeup GPE) by acpi_ref_wakeup_gpe. There are several examples of this type of code. If it is true that the call to acpi_enable_gpe is unnecessary, then is acpi_enable_gpe interface needed at all? (same with disable_gpe). [PATCH 2/3] acpica: Add support for unregistering ACPI GPE methods: acpi_remove_gpe_method: Ok, this makes sense. Sad, but makes sense. [PATCH 1/3] acpi: Provide default GPE handler if the firmware doesn't >Firmware may support using GPEs for system wakeup without >providing any runtime GPE handlers. Do you mean by "runtime GPE handlers" to mean the _Lxx/_Exx GPE methods? If so, this should be clarified in the comments. ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Recent GPE patches - some questions. 2010-01-29 20:30 Recent GPE patches - some questions Moore, Robert @ 2010-02-01 22:36 ` Matthew Garrett 2010-02-02 23:02 ` Moore, Robert 0 siblings, 1 reply; 20+ messages in thread From: Matthew Garrett @ 2010-02-01 22:36 UTC (permalink / raw) To: Moore, Robert; +Cc: linux-acpi@vger.kernel.org, rjw@sisk.pl On Fri, Jan 29, 2010 at 12:30:25PM -0800, Moore, Robert wrote: > [PATCH 0/3] acpica: Rewrite GPE handling > > Overall, can we say that this change to the GPE code is primarily > intended to add better support for shared GPEs? Yes, that's probably the easiest way to look at it. > [PATCH 1/3] ACPI: Add infrastructure for refcounting GPE consumers: > > >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. > > I'm not sure which interface you are referring to, please explain This is simply the ref/unref functions. > [PATCH 2/3] ACPI: Add support for new refcounted GPE API to drivers: > > Reference count mechanism is meant to: > 1) Enable GPE only on the first added reference > 2) Disable GPE only on the last removed reference > > Then I don't understand why the code below needs to call acpi_enable_gpe: > > acpi_enable_gpe(dev->wakeup.gpe_device, > dev->wakeup.gpe_number); > + acpi_ref_wakeup_gpe(dev->wakeup.gpe_device, > + dev->wakeup.gpe_number); > > Because the GPE will be "enabled" (mask bit set for wakeup GPE) by > acpi_ref_wakeup_gpe. There are several examples of this type of code. > If it is true that the call to acpi_enable_gpe is unnecessary, then is > acpi_enable_gpe interface needed at all? (same with disable_gpe). That's simply to make it easier to bisect through the changes - 3/3 removes the explicit enable_gpe call. > [PATCH 1/3] acpi: Provide default GPE handler if the firmware doesn't > > >Firmware may support using GPEs for system wakeup without > >providing any runtime GPE handlers. > > Do you mean by "runtime GPE handlers" to mean the _Lxx/_Exx GPE methods? If so, this should be clarified in the comments. Ok. I'm not sure that we'll be pushing this aspect of it yet, we'll have to wait and see what the hardware support situation ends up being. -- Matthew Garrett | mjg59@srcf.ucam.org ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: Recent GPE patches - some questions. 2010-02-01 22:36 ` Matthew Garrett @ 2010-02-02 23:02 ` Moore, Robert 2010-02-06 23:31 ` Rafael J. Wysocki 0 siblings, 1 reply; 20+ messages in thread From: Moore, Robert @ 2010-02-02 23:02 UTC (permalink / raw) To: Matthew Garrett; +Cc: linux-acpi@vger.kernel.org, rjw@sisk.pl Matthew, Thanks for your response to my questions. I've been thinking about these interfaces: acpi_ref_runtime_gpe acpi_ref_wakeup_gpe acpi_unref_runtime_gpe acpi_unref_wakeup_gpe While I understand the need for the reference counting mechanism, it is a good idea to support the shared GPEs, I think it may be simpler and cleaner to simply not directly expose this mechanism to GPE users. I'm suggesting that we add the reference counting mechanism to the existing AcpiEnableGpe and AcpiDisableGpe interfaces and update their descriptions. We already hide the differences between wake, run, and wake-run GPEs behind these interfaces. Adding the reference count semantic to these interfaces changes their behavior in a fairly simple way: Support for shared GPEs: AcpiEnableGpe: For a given GPE, it is actually enabled only on the first call. AcpiDisableGpe: For a given GPE, it is actually disabled only on the last call. Bob >-----Original Message----- >From: Matthew Garrett [mailto:mjg@redhat.com] >Sent: Monday, February 01, 2010 2:36 PM >To: Moore, Robert >Cc: linux-acpi@vger.kernel.org; rjw@sisk.pl >Subject: Re: Recent GPE patches - some questions. > >On Fri, Jan 29, 2010 at 12:30:25PM -0800, Moore, Robert wrote: > >> [PATCH 0/3] acpica: Rewrite GPE handling >> >> Overall, can we say that this change to the GPE code is primarily >> intended to add better support for shared GPEs? > >Yes, that's probably the easiest way to look at it. > >> [PATCH 1/3] ACPI: Add infrastructure for refcounting GPE consumers: >> >> >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. >> >> I'm not sure which interface you are referring to, please explain > >This is simply the ref/unref functions. > > >> [PATCH 2/3] ACPI: Add support for new refcounted GPE API to drivers: >> >> Reference count mechanism is meant to: >> 1) Enable GPE only on the first added reference >> 2) Disable GPE only on the last removed reference >> >> Then I don't understand why the code below needs to call acpi_enable_gpe: >> >> acpi_enable_gpe(dev->wakeup.gpe_device, >> dev->wakeup.gpe_number); >> + acpi_ref_wakeup_gpe(dev->wakeup.gpe_device, >> + dev->wakeup.gpe_number); >> >> Because the GPE will be "enabled" (mask bit set for wakeup GPE) by >> acpi_ref_wakeup_gpe. There are several examples of this type of code. >> If it is true that the call to acpi_enable_gpe is unnecessary, then is >> acpi_enable_gpe interface needed at all? (same with disable_gpe). > >That's simply to make it easier to bisect through the changes - 3/3 >removes the explicit enable_gpe call. > >> [PATCH 1/3] acpi: Provide default GPE handler if the firmware doesn't >> >> >Firmware may support using GPEs for system wakeup without >> >providing any runtime GPE handlers. >> >> Do you mean by "runtime GPE handlers" to mean the _Lxx/_Exx GPE methods? >If so, this should be clarified in the comments. > >Ok. I'm not sure that we'll be pushing this aspect of it yet, we'll have >to wait and see what the hardware support situation ends up being. > >-- >Matthew Garrett | mjg59@srcf.ucam.org ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: Recent GPE patches - some questions. 2010-02-02 23:02 ` Moore, Robert @ 2010-02-06 23:31 ` Rafael J. Wysocki 2010-02-07 2:17 ` [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) Rafael J. Wysocki 2010-02-10 21:36 ` Recent GPE patches - some questions Moore, Robert 0 siblings, 2 replies; 20+ messages in thread From: Rafael J. Wysocki @ 2010-02-06 23:31 UTC (permalink / raw) To: Moore, Robert; +Cc: Matthew Garrett, linux-acpi@vger.kernel.org, Len Brown Hi, On Wednesday 03 February 2010, Moore, Robert wrote: > Matthew, > > Thanks for your response to my questions. > > I've been thinking about these interfaces: > > acpi_ref_runtime_gpe > acpi_ref_wakeup_gpe > acpi_unref_runtime_gpe > acpi_unref_wakeup_gpe > > While I understand the need for the reference counting mechanism, it is a > good idea to support the shared GPEs, I think it may be simpler and cleaner > to simply not directly expose this mechanism to GPE users. Well, it already is exposed to the users and in a way that doesn't look very clean to me. Namely, to enable a wakeup GPE for run time, the caller needs to set its type to ACPI_GPE_TYPE_WAKE_RUN before calling acpi_enable_gpe(), while with the Matthew's interface the only thing the caller would need to do would be calling acpi_ref_runtime_gpe(). > I'm suggesting that we add the reference counting mechanism to the existing > AcpiEnableGpe and AcpiDisableGpe interfaces and update their descriptions. > We already hide the differences between wake, run, and wake-run GPEs behind > these interfaces. Not really, as I said above. Moreover, we need two reference counters, because there are cases when a GPE should be enabled for wakeup and not enabled for run time and vice versa. > Adding the reference count semantic to these interfaces changes their > behavior in a fairly simple way: > > Support for shared GPEs: > AcpiEnableGpe: For a given GPE, it is actually enabled only on the first call. > AcpiDisableGpe: For a given GPE, it is actually disabled only on the last call. I suppose you mean acpi_enable_gpe() and acpi_disable_gpe(). That wouldn't work, because sometimes we need to actually hardware-disable GPEs and hardware-enable them regardless of the refcount mechanism, like for example in the EC suspend and resume routines. That said, if you are afraid that the new interface may be cumbersome for the callers, I think we can introduce just two callbacks, acpi_get_gpe() acpi_put_gpe() taking 3 arguments each, where the two first arguments are like for the Matthew's callbacks and the third argument is a mask of two bits: ACPI_GPE_TYPE_WAKE ACPI_GPE_TYPE_RUNTIME that will tell the callback whether to use the wakeup or runtime counter for reference counting (if called with ACPI_GPE_TYPE_WAKE_RUN, both reference counters will be modified at the same time). Please tell me what you think. Rafael ^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) 2010-02-06 23:31 ` Rafael J. Wysocki @ 2010-02-07 2:17 ` Rafael J. Wysocki 2010-02-07 2:22 ` [RFC][PATCH 1/3] ACPI: Add infrastructure for refcounting GPE consumers Rafael J. Wysocki ` (4 more replies) 2010-02-10 21:36 ` Recent GPE patches - some questions Moore, Robert 1 sibling, 5 replies; 20+ messages in thread From: Rafael J. Wysocki @ 2010-02-07 2:17 UTC (permalink / raw) To: Moore, Robert Cc: Matthew Garrett, linux-acpi@vger.kernel.org, Len Brown, Jesse Barnes On Sunday 07 February 2010, Rafael J. Wysocki wrote: > Hi, > > On Wednesday 03 February 2010, Moore, Robert wrote: > > Matthew, > > > > Thanks for your response to my questions. > > > > I've been thinking about these interfaces: > > > > acpi_ref_runtime_gpe > > acpi_ref_wakeup_gpe > > acpi_unref_runtime_gpe > > acpi_unref_wakeup_gpe > > > > While I understand the need for the reference counting mechanism, it is a > > good idea to support the shared GPEs, I think it may be simpler and cleaner > > to simply not directly expose this mechanism to GPE users. > > Well, it already is exposed to the users and in a way that doesn't look very > clean to me. Namely, to enable a wakeup GPE for run time, the caller needs to > set its type to ACPI_GPE_TYPE_WAKE_RUN before calling acpi_enable_gpe(), > while with the Matthew's interface the only thing the caller would need to do > would be calling acpi_ref_runtime_gpe(). > > > I'm suggesting that we add the reference counting mechanism to the existing > > AcpiEnableGpe and AcpiDisableGpe interfaces and update their descriptions. > > We already hide the differences between wake, run, and wake-run GPEs behind > > these interfaces. > > Not really, as I said above. > > Moreover, we need two reference counters, because there are cases when a > GPE should be enabled for wakeup and not enabled for run time and vice versa. > > > Adding the reference count semantic to these interfaces changes their > > behavior in a fairly simple way: > > > > Support for shared GPEs: > > AcpiEnableGpe: For a given GPE, it is actually enabled only on the first call. > > AcpiDisableGpe: For a given GPE, it is actually disabled only on the last call. > > I suppose you mean acpi_enable_gpe() and acpi_disable_gpe(). > > That wouldn't work, because sometimes we need to actually hardware-disable > GPEs and hardware-enable them regardless of the refcount mechanism, like for > example in the EC suspend and resume routines. > > That said, if you are afraid that the new interface may be cumbersome for the > callers, I think we can introduce just two callbacks, > > acpi_get_gpe() > acpi_put_gpe() > > taking 3 arguments each, where the two first arguments are like for the > Matthew's callbacks and the third argument is a mask of two bits: > > ACPI_GPE_TYPE_WAKE > ACPI_GPE_TYPE_RUNTIME > > that will tell the callback whether to use the wakeup or runtime counter for > reference counting (if called with ACPI_GPE_TYPE_WAKE_RUN, both > reference counters will be modified at the same time). > > Please tell me what you think. For easier reference, I reworked the Matthew's patches to implement the above idea. The patches follow, comments welcome. Rafael ^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC][PATCH 1/3] ACPI: Add infrastructure for refcounting GPE consumers 2010-02-07 2:17 ` [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) Rafael J. Wysocki @ 2010-02-07 2:22 ` Rafael J. Wysocki 2010-02-07 2:23 ` [RFC][PATCH 2/3] ACPI: Modify GPE consumers to use GPE refcounting Rafael J. Wysocki ` (3 subsequent siblings) 4 siblings, 0 replies; 20+ messages in thread From: Rafael J. Wysocki @ 2010-02-07 2:22 UTC (permalink / raw) To: Moore, Robert Cc: Matthew Garrett, linux-acpi@vger.kernel.org, Len Brown, Jesse Barnes From: Rafael J. Wysocki <rjw@sisk.pl> 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. Based on a patch from Matthew Garrett <mjg@redhat.com>. Signed-off-by: Matthew Garrett <mjg@redhat.com> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> --- drivers/acpi/acpica/aclocal.h | 2 + drivers/acpi/acpica/aclocal.h | 2 drivers/acpi/acpica/evxfevnt.c | 97 +++++++++++++++++++++++++++++++++++++++++ include/acpi/acpixf.h | 4 + 3 files changed, 103 insertions(+) Index: linux-2.6/drivers/acpi/acpica/aclocal.h =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/aclocal.h +++ linux-2.6/drivers/acpi/acpica/aclocal.h @@ -426,6 +426,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 */ Index: linux-2.6/drivers/acpi/acpica/evxfevnt.c =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/evxfevnt.c +++ linux-2.6/drivers/acpi/acpica/evxfevnt.c @@ -201,6 +201,103 @@ ACPI_EXPORT_SYMBOL(acpi_enable_event) /******************************************************************************* * + * FUNCTION: acpi_get_gpe + * + * PARAMETERS: gpe_device - Parent GPE Device + * gpe_number - GPE level within the GPE block + * type - Purpose the GPE will be used for + * + * RETURN: Status + * + * DESCRIPTION: Take a reference to a GPE and enable it if necessary + * + ******************************************************************************/ +acpi_status acpi_get_gpe(acpi_handle gpe_device, u32 gpe_number, u8 type) +{ + acpi_status status = AE_OK; + acpi_cpu_flags flags; + struct acpi_gpe_event_info *gpe_event_info; + + ACPI_FUNCTION_TRACE(acpi_ref_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 (type & ACPI_GPE_TYPE_RUNTIME) { + 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--; + } + + if (type & ACPI_GPE_TYPE_WAKE) { + 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_gpe) + +/******************************************************************************* + * + * FUNCTION: acpi_put_gpe + * + * PARAMETERS: gpe_device - Parent GPE Device + * gpe_number - GPE level within the GPE block + * type - Purpose the GPE won't be used for any more + * + * RETURN: Status + * + * DESCRIPTION: Release a reference to a GPE and disable it if necessary + * + ******************************************************************************/ +acpi_status acpi_put_gpe(acpi_handle gpe_device, u32 gpe_number, u8 type) +{ + acpi_status status = AE_OK; + acpi_cpu_flags flags; + struct acpi_gpe_event_info *gpe_event_info; + + ACPI_FUNCTION_TRACE(acpi_unref_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 ((type & ACPI_GPE_TYPE_WAKE) && gpe_event_info->runtime_count) { + if (--gpe_event_info->runtime_count == 0) + acpi_ev_disable_gpe(gpe_event_info); + } + + if ((type & ACPI_GPE_TYPE_RUNTIME) && gpe_event_info->wakeup_count) { + 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_gpe) + +/******************************************************************************* + * * FUNCTION: acpi_set_gpe_type * * PARAMETERS: gpe_device - Parent GPE Device Index: linux-2.6/include/acpi/acpixf.h =================================================================== --- linux-2.6.orig/include/acpi/acpixf.h +++ linux-2.6/include/acpi/acpixf.h @@ -283,6 +283,10 @@ acpi_status acpi_get_event_status(u32 ev */ acpi_status acpi_set_gpe_type(acpi_handle gpe_device, u32 gpe_number, u8 type); +acpi_status acpi_get_gpe(acpi_handle gpe_device, u32 gpe_number, u8 type); + +acpi_status acpi_put_gpe(acpi_handle gpe_device, u32 gpe_number, u8 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); ^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC][PATCH 2/3] ACPI: Modify GPE consumers to use GPE refcounting 2010-02-07 2:17 ` [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) Rafael J. Wysocki 2010-02-07 2:22 ` [RFC][PATCH 1/3] ACPI: Add infrastructure for refcounting GPE consumers Rafael J. Wysocki @ 2010-02-07 2:23 ` Rafael J. Wysocki 2010-02-07 2:24 ` [RFC][PATCH 3/3] ACPI: Remove old GPE API and transition code entirely to new one Rafael J. Wysocki ` (2 subsequent siblings) 4 siblings, 0 replies; 20+ messages in thread From: Rafael J. Wysocki @ 2010-02-07 2:23 UTC (permalink / raw) To: Moore, Robert Cc: Matthew Garrett, linux-acpi@vger.kernel.org, Len Brown, Jesse Barnes From: Rafael J. Wysocki <rjw@sisk.pl> Add GPE refcounting support to the existing GPE users. This will currently do little until the core is changed over to use the new behaviour. Based on a patch from Matthew Garrett <mjg@redhat.com>. Signed-off-by: Matthew Garrett <mjg@redhat.com> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> --- drivers/acpi/button.c | 12 ++++++++++++ drivers/acpi/button.c | 10 ++++++++++ drivers/acpi/ec.c | 4 +++- drivers/acpi/wakeup.c | 10 ++++++++++ 3 files changed, 23 insertions(+), 1 deletion(-) Index: linux-2.6/drivers/acpi/button.c =================================================================== --- linux-2.6.orig/drivers/acpi/button.c +++ linux-2.6/drivers/acpi/button.c @@ -427,6 +427,9 @@ static int acpi_button_add(struct acpi_d ACPI_GPE_TYPE_WAKE_RUN); acpi_enable_gpe(device->wakeup.gpe_device, device->wakeup.gpe_number); + acpi_get_gpe(device->wakeup.gpe_device, + device->wakeup.gpe_number, + ACPI_GPE_TYPE_WAKE_RUN); device->wakeup.state.enabled = 1; } @@ -446,6 +449,13 @@ static int acpi_button_remove(struct acp { struct acpi_button *button = acpi_driver_data(device); + if (device->wakeup.flags.valid) { + acpi_put_gpe(device->wakeup.gpe_device, + device->wakeup.gpe_number, + ACPI_GPE_TYPE_WAKE_RUN); + device->wakeup.state.enabled = 0; + } + acpi_button_remove_fs(device); input_unregister_device(button->input); kfree(button); Index: linux-2.6/drivers/acpi/ec.c =================================================================== --- linux-2.6.orig/drivers/acpi/ec.c +++ linux-2.6/drivers/acpi/ec.c @@ -789,7 +789,7 @@ static int ec_install_handlers(struct ac if (ACPI_FAILURE(status)) return -ENODEV; acpi_set_gpe_type(NULL, ec->gpe, ACPI_GPE_TYPE_RUNTIME); - acpi_enable_gpe(NULL, ec->gpe); + acpi_get_gpe(NULL, ec->gpe, ACPI_GPE_TYPE_RUNTIME); status = acpi_install_address_space_handler(ec->handle, ACPI_ADR_SPACE_EC, &acpi_ec_space_handler, @@ -806,6 +806,7 @@ static int ec_install_handlers(struct ac } else { acpi_remove_gpe_handler(NULL, ec->gpe, &acpi_ec_gpe_handler); + acpi_put_gpe(NULL, ec->gpe, ACPI_GPE_TYPE_RUNTIME); return -ENODEV; } } @@ -816,6 +817,7 @@ static int ec_install_handlers(struct ac static void ec_remove_handlers(struct acpi_ec *ec) { + acpi_put_gpe(NULL, ec->gpe, ACPI_GPE_TYPE_RUNTIME); 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"); Index: linux-2.6/drivers/acpi/wakeup.c =================================================================== --- linux-2.6.orig/drivers/acpi/wakeup.c +++ linux-2.6/drivers/acpi/wakeup.c @@ -78,9 +78,15 @@ void acpi_enable_wakeup_device(u8 sleep_ } continue; } + /* + * If the device is run_wake, the GPE is supposed to be enabled, + * so we don't need to enable it once again. + */ if (!dev->wakeup.flags.run_wake) acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number); + acpi_get_gpe(dev->wakeup.gpe_device, + dev->wakeup.gpe_number, ACPI_GPE_TYPE_WAKE); } } @@ -121,6 +127,8 @@ void acpi_disable_wakeup_device(u8 sleep acpi_clear_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number, ACPI_NOT_ISR); } + acpi_put_gpe(dev->wakeup.gpe_device, + dev->wakeup.gpe_number, ACPI_GPE_TYPE_WAKE); } } @@ -141,6 +149,8 @@ int __init acpi_wakeup_device_init(void) ACPI_GPE_TYPE_WAKE_RUN); acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number); + acpi_get_gpe(dev->wakeup.gpe_device, + dev->wakeup.gpe_number, ACPI_GPE_TYPE_WAKE); dev->wakeup.state.enabled = 1; } mutex_unlock(&acpi_device_lock); ^ permalink raw reply [flat|nested] 20+ messages in thread
* [RFC][PATCH 3/3] ACPI: Remove old GPE API and transition code entirely to new one 2010-02-07 2:17 ` [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) Rafael J. Wysocki 2010-02-07 2:22 ` [RFC][PATCH 1/3] ACPI: Add infrastructure for refcounting GPE consumers Rafael J. Wysocki 2010-02-07 2:23 ` [RFC][PATCH 2/3] ACPI: Modify GPE consumers to use GPE refcounting Rafael J. Wysocki @ 2010-02-07 2:24 ` Rafael J. Wysocki 2010-02-07 11:56 ` [Update] Re: [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) Rafael J. Wysocki 2010-02-10 21:29 ` [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) Maxim Levitsky 4 siblings, 0 replies; 20+ messages in thread From: Rafael J. Wysocki @ 2010-02-07 2:24 UTC (permalink / raw) To: Moore, Robert Cc: Matthew Garrett, linux-acpi@vger.kernel.org, Len Brown, Jesse Barnes From: Matthew Garrett <mjg@redhat.com> Remove the old GPE type handling entirely, which gets rid of various quirks, like the implicit disabling with GPE type setting. This requires a small amount of rework in order to ensure that non-wake GPEs are enabled by default to preserve existing behaviour. [rjw: Rework the patch to use acpi_get_gpe() and acpi_put_gpe().] Signed-off-by: Matthew Garrett <mjg@redhat.com> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> --- drivers/acpi/acpica/acevents.h | 6 - drivers/acpi/acpica/evgpe.c | 153 ++++------------------------------------- drivers/acpi/acpica/evgpeblk.c | 69 +++++------------- drivers/acpi/acpica/evxface.c | 14 --- drivers/acpi/acpica/evxfevnt.c | 48 ------------ drivers/acpi/button.c | 5 - drivers/acpi/ec.c | 2 drivers/acpi/wakeup.c | 55 ++------------ include/acpi/actypes.h | 28 ++----- 9 files changed, 59 insertions(+), 321 deletions(-) Index: linux-2.6/drivers/acpi/acpica/acevents.h =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/acevents.h +++ linux-2.6/drivers/acpi/acpica/acevents.h @@ -76,8 +76,7 @@ acpi_ev_queue_notify_request(struct acpi * evgpe - GPE handling and dispatch */ acpi_status -acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info, - u8 type); +acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info); acpi_status acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info, @@ -122,9 +121,6 @@ acpi_ev_gpe_dispatch(struct acpi_gpe_eve u32 acpi_ev_gpe_detect(struct acpi_gpe_xrupt_info *gpe_xrupt_list); acpi_status -acpi_ev_set_gpe_type(struct acpi_gpe_event_info *gpe_event_info, u8 type); - -acpi_status acpi_ev_check_for_wake_only_gpe(struct acpi_gpe_event_info *gpe_event_info); acpi_status acpi_ev_gpe_initialize(void); Index: linux-2.6/drivers/acpi/acpica/evgpe.c =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/evgpe.c +++ linux-2.6/drivers/acpi/acpica/evgpe.c @@ -54,54 +54,9 @@ static void ACPI_SYSTEM_XFACE acpi_ev_as /******************************************************************************* * - * FUNCTION: acpi_ev_set_gpe_type - * - * PARAMETERS: gpe_event_info - GPE to set - * Type - New type - * - * RETURN: Status - * - * DESCRIPTION: Sets the new type for the GPE (wake, run, or wake/run) - * - ******************************************************************************/ - -acpi_status -acpi_ev_set_gpe_type(struct acpi_gpe_event_info *gpe_event_info, u8 type) -{ - acpi_status status; - - ACPI_FUNCTION_TRACE(ev_set_gpe_type); - - /* Validate type and update register enable masks */ - - switch (type) { - case ACPI_GPE_TYPE_WAKE: - case ACPI_GPE_TYPE_RUNTIME: - case ACPI_GPE_TYPE_WAKE_RUN: - break; - - default: - return_ACPI_STATUS(AE_BAD_PARAMETER); - } - - /* Disable the GPE if currently enabled */ - - status = acpi_ev_disable_gpe(gpe_event_info); - - /* Clear the type bits and insert the new Type */ - - gpe_event_info->flags &= ~ACPI_GPE_TYPE_MASK; - gpe_event_info->flags |= type; - return_ACPI_STATUS(status); -} - -/******************************************************************************* - * * FUNCTION: acpi_ev_update_gpe_enable_masks * * PARAMETERS: gpe_event_info - GPE to update - * Type - What to do: ACPI_GPE_DISABLE or - * ACPI_GPE_ENABLE * * RETURN: Status * @@ -110,8 +65,7 @@ acpi_ev_set_gpe_type(struct acpi_gpe_eve ******************************************************************************/ acpi_status -acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info, - u8 type) +acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info) { struct acpi_gpe_register_info *gpe_register_info; u8 register_bit; @@ -127,37 +81,14 @@ acpi_ev_update_gpe_enable_masks(struct a (1 << (gpe_event_info->gpe_number - gpe_register_info->base_gpe_number)); - /* 1) Disable case. Simply clear all enable bits */ - - if (type == ACPI_GPE_DISABLE) { - ACPI_CLEAR_BIT(gpe_register_info->enable_for_wake, - register_bit); - ACPI_CLEAR_BIT(gpe_register_info->enable_for_run, register_bit); - return_ACPI_STATUS(AE_OK); - } - - /* 2) Enable case. Set/Clear the appropriate enable bits */ + ACPI_CLEAR_BIT(gpe_register_info->enable_for_wake, register_bit); + ACPI_CLEAR_BIT(gpe_register_info->enable_for_run, register_bit); - switch (gpe_event_info->flags & ACPI_GPE_TYPE_MASK) { - case ACPI_GPE_TYPE_WAKE: - ACPI_SET_BIT(gpe_register_info->enable_for_wake, register_bit); - ACPI_CLEAR_BIT(gpe_register_info->enable_for_run, register_bit); - break; - - case ACPI_GPE_TYPE_RUNTIME: - ACPI_CLEAR_BIT(gpe_register_info->enable_for_wake, - register_bit); + if (gpe_event_info->runtime_count) ACPI_SET_BIT(gpe_register_info->enable_for_run, register_bit); - break; - case ACPI_GPE_TYPE_WAKE_RUN: + if (gpe_event_info->wakeup_count) ACPI_SET_BIT(gpe_register_info->enable_for_wake, register_bit); - ACPI_SET_BIT(gpe_register_info->enable_for_run, register_bit); - break; - - default: - return_ACPI_STATUS(AE_BAD_PARAMETER); - } return_ACPI_STATUS(AE_OK); } @@ -186,47 +117,20 @@ acpi_ev_enable_gpe(struct acpi_gpe_event /* Make sure HW enable masks are updated */ - status = - acpi_ev_update_gpe_enable_masks(gpe_event_info, ACPI_GPE_ENABLE); - if (ACPI_FAILURE(status)) { + status = acpi_ev_update_gpe_enable_masks(gpe_event_info); + if (ACPI_FAILURE(status)) return_ACPI_STATUS(status); - } /* Mark wake-enabled or HW enable, or both */ - switch (gpe_event_info->flags & ACPI_GPE_TYPE_MASK) { - case ACPI_GPE_TYPE_WAKE: - - ACPI_SET_BIT(gpe_event_info->flags, ACPI_GPE_WAKE_ENABLED); - break; - - case ACPI_GPE_TYPE_WAKE_RUN: - - ACPI_SET_BIT(gpe_event_info->flags, ACPI_GPE_WAKE_ENABLED); - - /*lint -fallthrough */ - - case ACPI_GPE_TYPE_RUNTIME: - - ACPI_SET_BIT(gpe_event_info->flags, ACPI_GPE_RUN_ENABLED); - - if (write_to_hardware) { - - /* Clear the GPE (of stale events), then enable it */ - - status = acpi_hw_clear_gpe(gpe_event_info); - if (ACPI_FAILURE(status)) { - return_ACPI_STATUS(status); - } - - /* Enable the requested runtime GPE */ - - status = acpi_hw_write_gpe_enable_reg(gpe_event_info); - } - break; + if (gpe_event_info->runtime_count && write_to_hardware) { + /* Clear the GPE (of stale events), then enable it */ + status = acpi_hw_clear_gpe(gpe_event_info); + if (ACPI_FAILURE(status)) + return_ACPI_STATUS(status); - default: - return_ACPI_STATUS(AE_BAD_PARAMETER); + /* Enable the requested runtime GPE */ + status = acpi_hw_write_gpe_enable_reg(gpe_event_info); } return_ACPI_STATUS(AE_OK); @@ -252,34 +156,9 @@ acpi_status acpi_ev_disable_gpe(struct a /* Make sure HW enable masks are updated */ - status = - acpi_ev_update_gpe_enable_masks(gpe_event_info, ACPI_GPE_DISABLE); - if (ACPI_FAILURE(status)) { + status = acpi_ev_update_gpe_enable_masks(gpe_event_info); + if (ACPI_FAILURE(status)) return_ACPI_STATUS(status); - } - - /* Clear the appropriate enabled flags for this GPE */ - - switch (gpe_event_info->flags & ACPI_GPE_TYPE_MASK) { - case ACPI_GPE_TYPE_WAKE: - ACPI_CLEAR_BIT(gpe_event_info->flags, ACPI_GPE_WAKE_ENABLED); - break; - - case ACPI_GPE_TYPE_WAKE_RUN: - ACPI_CLEAR_BIT(gpe_event_info->flags, ACPI_GPE_WAKE_ENABLED); - - /* fallthrough */ - - case ACPI_GPE_TYPE_RUNTIME: - - /* Disable the requested runtime GPE */ - - ACPI_CLEAR_BIT(gpe_event_info->flags, ACPI_GPE_RUN_ENABLED); - break; - - default: - break; - } /* * Even if we don't know the GPE type, make sure that we always Index: linux-2.6/drivers/acpi/acpica/evgpeblk.c =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/evgpeblk.c +++ linux-2.6/drivers/acpi/acpica/evgpeblk.c @@ -258,7 +258,6 @@ acpi_ev_save_method_info(acpi_handle obj u32 gpe_number; char name[ACPI_NAME_SIZE + 1]; u8 type; - acpi_status status; ACPI_FUNCTION_TRACE(ev_save_method_info); @@ -325,26 +324,20 @@ acpi_ev_save_method_info(acpi_handle obj /* * Now we can add this information to the gpe_event_info block for use - * during dispatch of this GPE. Default type is RUNTIME, although this may - * change when the _PRW methods are executed later. + * during dispatch of this GPE. */ gpe_event_info = &gpe_block->event_info[gpe_number - gpe_block->block_base_number]; - gpe_event_info->flags = (u8) - (type | ACPI_GPE_DISPATCH_METHOD | ACPI_GPE_TYPE_RUNTIME); + gpe_event_info->flags = (u8) (type | ACPI_GPE_DISPATCH_METHOD); gpe_event_info->dispatch.method_node = (struct acpi_namespace_node *)obj_handle; - /* Update enable mask, but don't enable the HW GPE as of yet */ - - status = acpi_ev_enable_gpe(gpe_event_info, FALSE); - ACPI_DEBUG_PRINT((ACPI_DB_LOAD, "Registered GPE method %s as GPE number 0x%.2X\n", name, gpe_number)); - return_ACPI_STATUS(status); + return_ACPI_STATUS(AE_OK); } /******************************************************************************* @@ -454,20 +447,7 @@ acpi_ev_match_prw_and_gpe(acpi_handle ob gpe_block-> block_base_number]; - /* Mark GPE for WAKE-ONLY but WAKE_DISABLED */ - - gpe_event_info->flags &= - ~(ACPI_GPE_WAKE_ENABLED | ACPI_GPE_RUN_ENABLED); - - status = - acpi_ev_set_gpe_type(gpe_event_info, ACPI_GPE_TYPE_WAKE); - if (ACPI_FAILURE(status)) { - goto cleanup; - } - - status = - acpi_ev_update_gpe_enable_masks(gpe_event_info, - ACPI_GPE_DISABLE); + gpe_event_info->flags |= ACPI_GPE_CAN_WAKE; } cleanup: @@ -1027,33 +1007,32 @@ acpi_ev_initialize_gpe_block(struct acpi } /* - * Enable all GPEs in this block that have these attributes: - * 1) are "runtime" or "run/wake" GPEs, and - * 2) have a corresponding _Lxx or _Exx method - * - * Any other GPEs within this block must be enabled via the - * acpi_enable_gpe() external interface. + * Enable all GPEs that have a corresponding method and aren't + * capable of generating wakeups. Any other GPEs within this block + * must be enabled via the acpi_get_gpe() interface. */ wake_gpe_count = 0; gpe_enabled_count = 0; + if (gpe_device == acpi_gbl_fadt_gpe_device) + gpe_device = NULL; for (i = 0; i < gpe_block->register_count; i++) { for (j = 0; j < 8; j++) { + int gpe_number = i * ACPI_GPE_REGISTER_WIDTH + j; /* Get the info block for this particular GPE */ + gpe_event_info = + &gpe_block->event_info[(acpi_size)gpe_number]; - gpe_event_info = &gpe_block->event_info[((acpi_size) i * - ACPI_GPE_REGISTER_WIDTH) - + j]; - - if (((gpe_event_info->flags & ACPI_GPE_DISPATCH_MASK) == - ACPI_GPE_DISPATCH_METHOD) && - (gpe_event_info->flags & ACPI_GPE_TYPE_RUNTIME)) { - gpe_enabled_count++; + if (gpe_event_info->flags & ACPI_GPE_CAN_WAKE) { + wake_gpe_count++; + continue; } - if (gpe_event_info->flags & ACPI_GPE_TYPE_WAKE) { - wake_gpe_count++; + if (gpe_event_info->flags & ACPI_GPE_DISPATCH_METHOD) { + acpi_get_gpe(gpe_device, gpe_number, + ACPI_GPE_TYPE_RUNTIME); + gpe_enabled_count++; } } } @@ -1062,15 +1041,7 @@ acpi_ev_initialize_gpe_block(struct acpi "Found %u Wake, Enabled %u Runtime GPEs in this block\n", wake_gpe_count, gpe_enabled_count)); - /* Enable all valid runtime GPEs found above */ - - status = acpi_hw_enable_runtime_gpe_block(NULL, gpe_block, NULL); - if (ACPI_FAILURE(status)) { - ACPI_ERROR((AE_INFO, "Could not enable GPEs in GpeBlock %p", - gpe_block)); - } - - return_ACPI_STATUS(status); + return_ACPI_STATUS(AE_OK); } /******************************************************************************* Index: linux-2.6/drivers/acpi/acpica/evxface.c =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/evxface.c +++ linux-2.6/drivers/acpi/acpica/evxface.c @@ -617,13 +617,6 @@ acpi_install_gpe_handler(acpi_handle gpe handler->context = context; handler->method_node = gpe_event_info->dispatch.method_node; - /* Disable the GPE before installing the handler */ - - status = acpi_ev_disable_gpe(gpe_event_info); - if (ACPI_FAILURE(status)) { - goto unlock_and_exit; - } - /* Install the handler */ flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock); @@ -707,13 +700,6 @@ acpi_remove_gpe_handler(acpi_handle gpe_ goto unlock_and_exit; } - /* Disable the GPE before removing the handler */ - - status = acpi_ev_disable_gpe(gpe_event_info); - if (ACPI_FAILURE(status)) { - goto unlock_and_exit; - } - /* Make sure all deferred tasks are completed */ (void)acpi_ut_release_mutex(ACPI_MTX_EVENTS); Index: linux-2.6/drivers/acpi/acpica/evxfevnt.c =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/evxfevnt.c +++ linux-2.6/drivers/acpi/acpica/evxfevnt.c @@ -239,8 +239,7 @@ acpi_status acpi_get_gpe(acpi_handle gpe if (type & ACPI_GPE_TYPE_WAKE) { if (++gpe_event_info->wakeup_count == 1) - acpi_ev_update_gpe_enable_masks(gpe_event_info, - ACPI_GPE_ENABLE); + acpi_ev_update_gpe_enable_masks(gpe_event_info); } unlock_and_exit: @@ -286,8 +285,7 @@ acpi_status acpi_put_gpe(acpi_handle gpe if ((type & ACPI_GPE_TYPE_RUNTIME) && gpe_event_info->wakeup_count) { if (--gpe_event_info->wakeup_count == 0) - acpi_ev_update_gpe_enable_masks(gpe_event_info, - ACPI_GPE_DISABLE); + acpi_ev_update_gpe_enable_masks(gpe_event_info); } unlock_and_exit: @@ -298,48 +296,6 @@ ACPI_EXPORT_SYMBOL(acpi_put_gpe) /******************************************************************************* * - * FUNCTION: acpi_set_gpe_type - * - * PARAMETERS: gpe_device - Parent GPE Device - * gpe_number - GPE level within the GPE block - * Type - New GPE type - * - * RETURN: Status - * - * DESCRIPTION: Set the type of an individual GPE - * - ******************************************************************************/ -acpi_status acpi_set_gpe_type(acpi_handle gpe_device, u32 gpe_number, u8 type) -{ - acpi_status status = AE_OK; - struct acpi_gpe_event_info *gpe_event_info; - - ACPI_FUNCTION_TRACE(acpi_set_gpe_type); - - /* 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->flags & ACPI_GPE_TYPE_MASK) == type) { - return_ACPI_STATUS(AE_OK); - } - - /* Set the new type (will disable GPE if currently enabled) */ - - status = acpi_ev_set_gpe_type(gpe_event_info, type); - - unlock_and_exit: - return_ACPI_STATUS(status); -} - -ACPI_EXPORT_SYMBOL(acpi_set_gpe_type) - -/******************************************************************************* - * * FUNCTION: acpi_enable_gpe * * PARAMETERS: gpe_device - Parent GPE Device Index: linux-2.6/drivers/acpi/button.c =================================================================== --- linux-2.6.orig/drivers/acpi/button.c +++ linux-2.6/drivers/acpi/button.c @@ -422,11 +422,6 @@ static int acpi_button_add(struct acpi_d if (device->wakeup.flags.valid) { /* Button's GPE is run-wake GPE */ - acpi_set_gpe_type(device->wakeup.gpe_device, - device->wakeup.gpe_number, - ACPI_GPE_TYPE_WAKE_RUN); - acpi_enable_gpe(device->wakeup.gpe_device, - device->wakeup.gpe_number); acpi_get_gpe(device->wakeup.gpe_device, device->wakeup.gpe_number, ACPI_GPE_TYPE_WAKE_RUN); Index: linux-2.6/drivers/acpi/ec.c =================================================================== --- linux-2.6.orig/drivers/acpi/ec.c +++ linux-2.6/drivers/acpi/ec.c @@ -788,7 +788,7 @@ static int ec_install_handlers(struct ac &acpi_ec_gpe_handler, ec); if (ACPI_FAILURE(status)) return -ENODEV; - acpi_set_gpe_type(NULL, ec->gpe, ACPI_GPE_TYPE_RUNTIME); + acpi_get_gpe(NULL, ec->gpe, ACPI_GPE_TYPE_RUNTIME); status = acpi_install_address_space_handler(ec->handle, ACPI_ADR_SPACE_EC, Index: linux-2.6/drivers/acpi/wakeup.c =================================================================== --- linux-2.6.orig/drivers/acpi/wakeup.c +++ linux-2.6/drivers/acpi/wakeup.c @@ -62,29 +62,16 @@ void acpi_enable_wakeup_device(u8 sleep_ struct acpi_device *dev = container_of(node, struct acpi_device, wakeup_list); - if (!dev->wakeup.flags.valid) + if (!dev->wakeup.flags.valid || + !dev->wakeup.prepare_count || + !dev->wakeup.state.enabled || + (sleep_state > (u32) dev->wakeup.sleep_state)) continue; - /* If users want to disable run-wake GPE, - * we only disable it for wake and leave it for runtime - */ - if ((!dev->wakeup.state.enabled && !dev->wakeup.prepare_count) - || sleep_state > (u32) dev->wakeup.sleep_state) { - if (dev->wakeup.flags.run_wake) { - /* set_gpe_type will disable GPE, leave it like that */ - acpi_set_gpe_type(dev->wakeup.gpe_device, - dev->wakeup.gpe_number, - ACPI_GPE_TYPE_RUNTIME); - } - continue; - } /* * If the device is run_wake, the GPE is supposed to be enabled, * so we don't need to enable it once again. */ - if (!dev->wakeup.flags.run_wake) - acpi_enable_gpe(dev->wakeup.gpe_device, - dev->wakeup.gpe_number); acpi_get_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number, ACPI_GPE_TYPE_WAKE); } @@ -103,32 +90,13 @@ void acpi_disable_wakeup_device(u8 sleep struct acpi_device *dev = container_of(node, struct acpi_device, wakeup_list); - if (!dev->wakeup.flags.valid) - continue; - - if ((!dev->wakeup.state.enabled && !dev->wakeup.prepare_count) - || sleep_state > (u32) dev->wakeup.sleep_state) { - if (dev->wakeup.flags.run_wake) { - acpi_set_gpe_type(dev->wakeup.gpe_device, - dev->wakeup.gpe_number, - ACPI_GPE_TYPE_WAKE_RUN); - /* Re-enable it, since set_gpe_type will disable it */ - acpi_enable_gpe(dev->wakeup.gpe_device, - dev->wakeup.gpe_number); - } - continue; - } + if (dev->wakeup.state.enabled && + dev->wakeup.prepare_count && + sleep_state <= (u32) dev->wakeup.sleep_state) + acpi_put_gpe(dev->wakeup.gpe_device, + dev->wakeup.gpe_number, ACPI_GPE_TYPE_WAKE); acpi_disable_wakeup_device_power(dev); - /* Never disable run-wake GPE */ - if (!dev->wakeup.flags.run_wake) { - acpi_disable_gpe(dev->wakeup.gpe_device, - dev->wakeup.gpe_number); - acpi_clear_gpe(dev->wakeup.gpe_device, - dev->wakeup.gpe_number, ACPI_NOT_ISR); - } - acpi_put_gpe(dev->wakeup.gpe_device, - dev->wakeup.gpe_number, ACPI_GPE_TYPE_WAKE); } } @@ -144,11 +112,6 @@ int __init acpi_wakeup_device_init(void) /* In case user doesn't load button driver */ if (!dev->wakeup.flags.run_wake || dev->wakeup.state.enabled) continue; - acpi_set_gpe_type(dev->wakeup.gpe_device, - dev->wakeup.gpe_number, - ACPI_GPE_TYPE_WAKE_RUN); - acpi_enable_gpe(dev->wakeup.gpe_device, - dev->wakeup.gpe_number); acpi_get_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number, ACPI_GPE_TYPE_WAKE); dev->wakeup.state.enabled = 1; Index: linux-2.6/include/acpi/actypes.h =================================================================== --- linux-2.6.orig/include/acpi/actypes.h +++ linux-2.6/include/acpi/actypes.h @@ -668,15 +668,16 @@ typedef u32 acpi_event_status; /* * GPE info flags - Per GPE - * +-+-+-+---+---+-+ - * |7|6|5|4:3|2:1|0| - * +-+-+-+---+---+-+ - * | | | | | | - * | | | | | +--- Interrupt type: Edge or Level Triggered - * | | | | +--- Type: Wake-only, Runtime-only, or wake/runtime + * +-+-+-+---+-+-+-+ + * |7|6|5|4:3|2|1|0| + * +-+-+-+---+-+-+-+ + * | | | | | | | + * | | | | | | +--- Interrupt type: Edge or Level Triggered + * | | | | | +--- GPE can wake the system + * | | | | +--- Unused * | | | +--- Type of dispatch -- to method, handler, or none - * | | +--- Enabled for runtime? - * | +--- Enabled for wake? + * | | +--- Unused + * | +--- Unused * +--- Unused */ #define ACPI_GPE_XRUPT_TYPE_MASK (u8) 0x01 @@ -687,22 +688,13 @@ typedef u32 acpi_event_status; #define ACPI_GPE_TYPE_WAKE_RUN (u8) 0x06 #define ACPI_GPE_TYPE_WAKE (u8) 0x02 #define ACPI_GPE_TYPE_RUNTIME (u8) 0x04 /* Default */ +#define ACPI_GPE_CAN_WAKE (u8) 0x02 #define ACPI_GPE_DISPATCH_MASK (u8) 0x18 #define ACPI_GPE_DISPATCH_HANDLER (u8) 0x08 #define ACPI_GPE_DISPATCH_METHOD (u8) 0x10 #define ACPI_GPE_DISPATCH_NOT_USED (u8) 0x00 /* Default */ -#define ACPI_GPE_RUN_ENABLE_MASK (u8) 0x20 -#define ACPI_GPE_RUN_ENABLED (u8) 0x20 -#define ACPI_GPE_RUN_DISABLED (u8) 0x00 /* Default */ - -#define ACPI_GPE_WAKE_ENABLE_MASK (u8) 0x40 -#define ACPI_GPE_WAKE_ENABLED (u8) 0x40 -#define ACPI_GPE_WAKE_DISABLED (u8) 0x00 /* Default */ - -#define ACPI_GPE_ENABLE_MASK (u8) 0x60 /* Both run/wake */ - /* * Flags for GPE and Lock interfaces */ ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Update] Re: [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) 2010-02-07 2:17 ` [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) Rafael J. Wysocki ` (2 preceding siblings ...) 2010-02-07 2:24 ` [RFC][PATCH 3/3] ACPI: Remove old GPE API and transition code entirely to new one Rafael J. Wysocki @ 2010-02-07 11:56 ` Rafael J. Wysocki 2010-02-07 11:58 ` [Update][RFC][PATCH 1/3] ACPI: Add infrastructure for refcounting GPE consumers Rafael J. Wysocki ` (2 more replies) 2010-02-10 21:29 ` [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) Maxim Levitsky 4 siblings, 3 replies; 20+ messages in thread From: Rafael J. Wysocki @ 2010-02-07 11:56 UTC (permalink / raw) To: Moore, Robert, Len Brown Cc: Matthew Garrett, Jesse Barnes, ACPI Devel Maling List, pm list On Sunday 07 February 2010, Rafael J. Wysocki wrote: > On Sunday 07 February 2010, Rafael J. Wysocki wrote: > > Hi, > > > > On Wednesday 03 February 2010, Moore, Robert wrote: > > > Matthew, > > > > > > Thanks for your response to my questions. > > > > > > I've been thinking about these interfaces: > > > > > > acpi_ref_runtime_gpe > > > acpi_ref_wakeup_gpe > > > acpi_unref_runtime_gpe > > > acpi_unref_wakeup_gpe > > > > > > While I understand the need for the reference counting mechanism, it is a > > > good idea to support the shared GPEs, I think it may be simpler and cleaner > > > to simply not directly expose this mechanism to GPE users. > > > > Well, it already is exposed to the users and in a way that doesn't look very > > clean to me. Namely, to enable a wakeup GPE for run time, the caller needs to > > set its type to ACPI_GPE_TYPE_WAKE_RUN before calling acpi_enable_gpe(), > > while with the Matthew's interface the only thing the caller would need to do > > would be calling acpi_ref_runtime_gpe(). > > > > > I'm suggesting that we add the reference counting mechanism to the existing > > > AcpiEnableGpe and AcpiDisableGpe interfaces and update their descriptions. > > > We already hide the differences between wake, run, and wake-run GPEs behind > > > these interfaces. > > > > Not really, as I said above. > > > > Moreover, we need two reference counters, because there are cases when a > > GPE should be enabled for wakeup and not enabled for run time and vice versa. > > > > > Adding the reference count semantic to these interfaces changes their > > > behavior in a fairly simple way: > > > > > > Support for shared GPEs: > > > AcpiEnableGpe: For a given GPE, it is actually enabled only on the first call. > > > AcpiDisableGpe: For a given GPE, it is actually disabled only on the last call. > > > > I suppose you mean acpi_enable_gpe() and acpi_disable_gpe(). > > > > That wouldn't work, because sometimes we need to actually hardware-disable > > GPEs and hardware-enable them regardless of the refcount mechanism, like for > > example in the EC suspend and resume routines. > > > > That said, if you are afraid that the new interface may be cumbersome for the > > callers, I think we can introduce just two callbacks, > > > > acpi_get_gpe() > > acpi_put_gpe() > > > > taking 3 arguments each, where the two first arguments are like for the > > Matthew's callbacks and the third argument is a mask of two bits: > > > > ACPI_GPE_TYPE_WAKE > > ACPI_GPE_TYPE_RUNTIME > > > > that will tell the callback whether to use the wakeup or runtime counter for > > reference counting (if called with ACPI_GPE_TYPE_WAKE_RUN, both > > reference counters will be modified at the same time). > > > > Please tell me what you think. > > For easier reference, I reworked the Matthew's patches to implement the above > idea. I noticed that patch [2/3] was actually wrong, because it added the GPE refcounting to drivers/acpi/wakeup.c:acpi_[enable|disable]_wakeup_device(), while in fact it should have added it to drivers/acpi/sleep.c:acpi_pm_device_sleep_wake(). Moreover, patch [3/3] did not really enable the wakeup GPEs after they had been disabled by acpi_disable_all_gpes(). I fixed the two patches and added explanatory comments to patch [1/3]. Updated patches follow, comments welcome. Rafael ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Update][RFC][PATCH 1/3] ACPI: Add infrastructure for refcounting GPE consumers 2010-02-07 11:56 ` [Update] Re: [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) Rafael J. Wysocki @ 2010-02-07 11:58 ` Rafael J. Wysocki 2010-02-07 11:58 ` [Update][RFC][PATCH 2/3] ACPI: Modify GPE consumers to use GPE refcounting Rafael J. Wysocki 2010-02-07 11:59 ` [Update][RFC][PATCH 3/3] ACPI: Remove old GPE API and transition code entirely to new one Rafael J. Wysocki 2 siblings, 0 replies; 20+ messages in thread From: Rafael J. Wysocki @ 2010-02-07 11:58 UTC (permalink / raw) To: Moore, Robert Cc: Len Brown, Matthew Garrett, Jesse Barnes, ACPI Devel Maling List, pm list From: Rafael J. Wysocki <rjw@sisk.pl> 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. Based on a patch from Matthew Garrett <mjg@redhat.com>. Signed-off-by: Matthew Garrett <mjg@redhat.com> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> --- drivers/acpi/acpica/aclocal.h | 2 + drivers/acpi/acpica/aclocal.h | 2 drivers/acpi/acpica/evxfevnt.c | 105 +++++++++++++++++++++++++++++++++++++++++ include/acpi/acpixf.h | 4 + 3 files changed, 111 insertions(+) Index: linux-2.6/drivers/acpi/acpica/aclocal.h =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/aclocal.h +++ linux-2.6/drivers/acpi/acpica/aclocal.h @@ -426,6 +426,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 */ Index: linux-2.6/drivers/acpi/acpica/evxfevnt.c =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/evxfevnt.c +++ linux-2.6/drivers/acpi/acpica/evxfevnt.c @@ -201,6 +201,111 @@ ACPI_EXPORT_SYMBOL(acpi_enable_event) /******************************************************************************* * + * FUNCTION: acpi_get_gpe + * + * PARAMETERS: gpe_device - Parent GPE Device + * gpe_number - GPE level within the GPE block + * type - Purpose the GPE will be used for + * + * RETURN: Status + * + * DESCRIPTION: Take a reference to a GPE and enable it if necessary + * + ******************************************************************************/ +acpi_status acpi_get_gpe(acpi_handle gpe_device, u32 gpe_number, u8 type) +{ + acpi_status status = AE_OK; + acpi_cpu_flags flags; + struct acpi_gpe_event_info *gpe_event_info; + + ACPI_FUNCTION_TRACE(acpi_ref_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 (type & ACPI_GPE_TYPE_RUNTIME) { + 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--; + } + + if (type & ACPI_GPE_TYPE_WAKE) { + /* + * Wake-up GPEs are only enabled right prior to putting the + * system into a sleep state. + */ + 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_gpe) + +/******************************************************************************* + * + * FUNCTION: acpi_put_gpe + * + * PARAMETERS: gpe_device - Parent GPE Device + * gpe_number - GPE level within the GPE block + * type - Purpose the GPE won't be used for any more + * + * RETURN: Status + * + * DESCRIPTION: Release a reference to a GPE and disable it if necessary + * + ******************************************************************************/ +acpi_status acpi_put_gpe(acpi_handle gpe_device, u32 gpe_number, u8 type) +{ + acpi_status status = AE_OK; + acpi_cpu_flags flags; + struct acpi_gpe_event_info *gpe_event_info; + + ACPI_FUNCTION_TRACE(acpi_unref_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 ((type & ACPI_GPE_TYPE_WAKE) && gpe_event_info->runtime_count) { + if (--gpe_event_info->runtime_count == 0) + acpi_ev_disable_gpe(gpe_event_info); + } + + if ((type & ACPI_GPE_TYPE_RUNTIME) && gpe_event_info->wakeup_count) { + /* + * Wake-up GPEs are not enabled after leaving system sleep + * states, so we don't need to disable them here. + */ + 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_gpe) + +/******************************************************************************* + * * FUNCTION: acpi_set_gpe_type * * PARAMETERS: gpe_device - Parent GPE Device Index: linux-2.6/include/acpi/acpixf.h =================================================================== --- linux-2.6.orig/include/acpi/acpixf.h +++ linux-2.6/include/acpi/acpixf.h @@ -283,6 +283,10 @@ acpi_status acpi_get_event_status(u32 ev */ acpi_status acpi_set_gpe_type(acpi_handle gpe_device, u32 gpe_number, u8 type); +acpi_status acpi_get_gpe(acpi_handle gpe_device, u32 gpe_number, u8 type); + +acpi_status acpi_put_gpe(acpi_handle gpe_device, u32 gpe_number, u8 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); ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Update][RFC][PATCH 2/3] ACPI: Modify GPE consumers to use GPE refcounting 2010-02-07 11:56 ` [Update] Re: [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) Rafael J. Wysocki 2010-02-07 11:58 ` [Update][RFC][PATCH 1/3] ACPI: Add infrastructure for refcounting GPE consumers Rafael J. Wysocki @ 2010-02-07 11:58 ` Rafael J. Wysocki 2010-02-07 11:59 ` [Update][RFC][PATCH 3/3] ACPI: Remove old GPE API and transition code entirely to new one Rafael J. Wysocki 2 siblings, 0 replies; 20+ messages in thread From: Rafael J. Wysocki @ 2010-02-07 11:58 UTC (permalink / raw) To: Moore, Robert Cc: Len Brown, Matthew Garrett, Jesse Barnes, ACPI Devel Maling List, pm list From: Rafael J. Wysocki <rjw@sisk.pl> Add GPE refcounting support to the existing GPE users. This will currently do little until the core is changed over to fully implement the new behaviour. Based on a patch from Matthew Garrett <mjg@redhat.com>. Signed-off-by: Matthew Garrett <mjg@redhat.com> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> --- drivers/acpi/button.c | 10 ++++++++++ drivers/acpi/ec.c | 4 +++- drivers/acpi/sleep.c | 15 ++++++++++++--- drivers/acpi/wakeup.c | 2 ++ 4 files changed, 27 insertions(+), 4 deletions(-) Index: linux-2.6/drivers/acpi/button.c =================================================================== --- linux-2.6.orig/drivers/acpi/button.c +++ linux-2.6/drivers/acpi/button.c @@ -427,6 +427,9 @@ static int acpi_button_add(struct acpi_d ACPI_GPE_TYPE_WAKE_RUN); acpi_enable_gpe(device->wakeup.gpe_device, device->wakeup.gpe_number); + acpi_get_gpe(device->wakeup.gpe_device, + device->wakeup.gpe_number, + ACPI_GPE_TYPE_WAKE_RUN); device->wakeup.state.enabled = 1; } @@ -446,6 +449,13 @@ static int acpi_button_remove(struct acp { struct acpi_button *button = acpi_driver_data(device); + if (device->wakeup.flags.valid) { + acpi_put_gpe(device->wakeup.gpe_device, + device->wakeup.gpe_number, + ACPI_GPE_TYPE_WAKE_RUN); + device->wakeup.state.enabled = 0; + } + acpi_button_remove_fs(device); input_unregister_device(button->input); kfree(button); Index: linux-2.6/drivers/acpi/ec.c =================================================================== --- linux-2.6.orig/drivers/acpi/ec.c +++ linux-2.6/drivers/acpi/ec.c @@ -789,7 +789,7 @@ static int ec_install_handlers(struct ac if (ACPI_FAILURE(status)) return -ENODEV; acpi_set_gpe_type(NULL, ec->gpe, ACPI_GPE_TYPE_RUNTIME); - acpi_enable_gpe(NULL, ec->gpe); + acpi_get_gpe(NULL, ec->gpe, ACPI_GPE_TYPE_RUNTIME); status = acpi_install_address_space_handler(ec->handle, ACPI_ADR_SPACE_EC, &acpi_ec_space_handler, @@ -806,6 +806,7 @@ static int ec_install_handlers(struct ac } else { acpi_remove_gpe_handler(NULL, ec->gpe, &acpi_ec_gpe_handler); + acpi_put_gpe(NULL, ec->gpe, ACPI_GPE_TYPE_RUNTIME); return -ENODEV; } } @@ -816,6 +817,7 @@ static int ec_install_handlers(struct ac static void ec_remove_handlers(struct acpi_ec *ec) { + acpi_put_gpe(NULL, ec->gpe, ACPI_GPE_TYPE_RUNTIME); 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"); Index: linux-2.6/drivers/acpi/sleep.c =================================================================== --- linux-2.6.orig/drivers/acpi/sleep.c +++ linux-2.6/drivers/acpi/sleep.c @@ -745,9 +745,18 @@ int acpi_pm_device_sleep_wake(struct dev return -ENODEV; } - error = enable ? - acpi_enable_wakeup_device_power(adev, acpi_target_sleep_state) : - acpi_disable_wakeup_device_power(adev); + if (enable) { + error = acpi_enable_wakeup_device_power(adev, + acpi_target_sleep_state); + if (!error) + acpi_get_gpe(adev->wakeup.gpe_device, + adev->wakeup.gpe_number, + ACPI_GPE_TYPE_WAKE); + } else { + acpi_put_gpe(adev->wakeup.gpe_device, adev->wakeup.gpe_number, + ACPI_GPE_TYPE_WAKE); + error = acpi_disable_wakeup_device_power(adev); + } if (!error) dev_info(dev, "wake-up capability %s by ACPI\n", enable ? "enabled" : "disabled"); Index: linux-2.6/drivers/acpi/wakeup.c =================================================================== --- linux-2.6.orig/drivers/acpi/wakeup.c +++ linux-2.6/drivers/acpi/wakeup.c @@ -141,6 +141,8 @@ int __init acpi_wakeup_device_init(void) ACPI_GPE_TYPE_WAKE_RUN); acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number); + acpi_get_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number, + ACPI_GPE_TYPE_WAKE); dev->wakeup.state.enabled = 1; } mutex_unlock(&acpi_device_lock); ^ permalink raw reply [flat|nested] 20+ messages in thread
* [Update][RFC][PATCH 3/3] ACPI: Remove old GPE API and transition code entirely to new one 2010-02-07 11:56 ` [Update] Re: [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) Rafael J. Wysocki 2010-02-07 11:58 ` [Update][RFC][PATCH 1/3] ACPI: Add infrastructure for refcounting GPE consumers Rafael J. Wysocki 2010-02-07 11:58 ` [Update][RFC][PATCH 2/3] ACPI: Modify GPE consumers to use GPE refcounting Rafael J. Wysocki @ 2010-02-07 11:59 ` Rafael J. Wysocki 2 siblings, 0 replies; 20+ messages in thread From: Rafael J. Wysocki @ 2010-02-07 11:59 UTC (permalink / raw) To: Moore, Robert Cc: Len Brown, Matthew Garrett, Jesse Barnes, ACPI Devel Maling List, pm list From: Matthew Garrett <mjg@redhat.com> Remove the old GPE type handling entirely, which gets rid of various quirks, like the implicit disabling with GPE type setting. This requires a small amount of rework in order to ensure that non-wake GPEs are enabled by default to preserve existing behaviour. [rjw: Reworked the patch to use acpi_get_gpe() and acpi_put_gpe() and fixed the changes in drivers/acpi/wakeup.c] Signed-off-by: Matthew Garrett <mjg@redhat.com> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> --- drivers/acpi/acpica/acevents.h | 6 - drivers/acpi/acpica/evgpe.c | 153 ++++------------------------------------- drivers/acpi/acpica/evgpeblk.c | 69 +++++------------- drivers/acpi/acpica/evxface.c | 14 --- drivers/acpi/acpica/evxfevnt.c | 48 ------------ drivers/acpi/button.c | 5 - drivers/acpi/ec.c | 2 drivers/acpi/wakeup.c | 78 ++++++-------------- include/acpi/actypes.h | 28 ++----- 9 files changed, 74 insertions(+), 329 deletions(-) Index: linux-2.6/drivers/acpi/acpica/acevents.h =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/acevents.h +++ linux-2.6/drivers/acpi/acpica/acevents.h @@ -76,8 +76,7 @@ acpi_ev_queue_notify_request(struct acpi * evgpe - GPE handling and dispatch */ acpi_status -acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info, - u8 type); +acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info); acpi_status acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info, @@ -122,9 +121,6 @@ acpi_ev_gpe_dispatch(struct acpi_gpe_eve u32 acpi_ev_gpe_detect(struct acpi_gpe_xrupt_info *gpe_xrupt_list); acpi_status -acpi_ev_set_gpe_type(struct acpi_gpe_event_info *gpe_event_info, u8 type); - -acpi_status acpi_ev_check_for_wake_only_gpe(struct acpi_gpe_event_info *gpe_event_info); acpi_status acpi_ev_gpe_initialize(void); Index: linux-2.6/drivers/acpi/acpica/evgpe.c =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/evgpe.c +++ linux-2.6/drivers/acpi/acpica/evgpe.c @@ -54,54 +54,9 @@ static void ACPI_SYSTEM_XFACE acpi_ev_as /******************************************************************************* * - * FUNCTION: acpi_ev_set_gpe_type - * - * PARAMETERS: gpe_event_info - GPE to set - * Type - New type - * - * RETURN: Status - * - * DESCRIPTION: Sets the new type for the GPE (wake, run, or wake/run) - * - ******************************************************************************/ - -acpi_status -acpi_ev_set_gpe_type(struct acpi_gpe_event_info *gpe_event_info, u8 type) -{ - acpi_status status; - - ACPI_FUNCTION_TRACE(ev_set_gpe_type); - - /* Validate type and update register enable masks */ - - switch (type) { - case ACPI_GPE_TYPE_WAKE: - case ACPI_GPE_TYPE_RUNTIME: - case ACPI_GPE_TYPE_WAKE_RUN: - break; - - default: - return_ACPI_STATUS(AE_BAD_PARAMETER); - } - - /* Disable the GPE if currently enabled */ - - status = acpi_ev_disable_gpe(gpe_event_info); - - /* Clear the type bits and insert the new Type */ - - gpe_event_info->flags &= ~ACPI_GPE_TYPE_MASK; - gpe_event_info->flags |= type; - return_ACPI_STATUS(status); -} - -/******************************************************************************* - * * FUNCTION: acpi_ev_update_gpe_enable_masks * * PARAMETERS: gpe_event_info - GPE to update - * Type - What to do: ACPI_GPE_DISABLE or - * ACPI_GPE_ENABLE * * RETURN: Status * @@ -110,8 +65,7 @@ acpi_ev_set_gpe_type(struct acpi_gpe_eve ******************************************************************************/ acpi_status -acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info, - u8 type) +acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info) { struct acpi_gpe_register_info *gpe_register_info; u8 register_bit; @@ -127,37 +81,14 @@ acpi_ev_update_gpe_enable_masks(struct a (1 << (gpe_event_info->gpe_number - gpe_register_info->base_gpe_number)); - /* 1) Disable case. Simply clear all enable bits */ - - if (type == ACPI_GPE_DISABLE) { - ACPI_CLEAR_BIT(gpe_register_info->enable_for_wake, - register_bit); - ACPI_CLEAR_BIT(gpe_register_info->enable_for_run, register_bit); - return_ACPI_STATUS(AE_OK); - } - - /* 2) Enable case. Set/Clear the appropriate enable bits */ + ACPI_CLEAR_BIT(gpe_register_info->enable_for_wake, register_bit); + ACPI_CLEAR_BIT(gpe_register_info->enable_for_run, register_bit); - switch (gpe_event_info->flags & ACPI_GPE_TYPE_MASK) { - case ACPI_GPE_TYPE_WAKE: - ACPI_SET_BIT(gpe_register_info->enable_for_wake, register_bit); - ACPI_CLEAR_BIT(gpe_register_info->enable_for_run, register_bit); - break; - - case ACPI_GPE_TYPE_RUNTIME: - ACPI_CLEAR_BIT(gpe_register_info->enable_for_wake, - register_bit); + if (gpe_event_info->runtime_count) ACPI_SET_BIT(gpe_register_info->enable_for_run, register_bit); - break; - case ACPI_GPE_TYPE_WAKE_RUN: + if (gpe_event_info->wakeup_count) ACPI_SET_BIT(gpe_register_info->enable_for_wake, register_bit); - ACPI_SET_BIT(gpe_register_info->enable_for_run, register_bit); - break; - - default: - return_ACPI_STATUS(AE_BAD_PARAMETER); - } return_ACPI_STATUS(AE_OK); } @@ -186,47 +117,20 @@ acpi_ev_enable_gpe(struct acpi_gpe_event /* Make sure HW enable masks are updated */ - status = - acpi_ev_update_gpe_enable_masks(gpe_event_info, ACPI_GPE_ENABLE); - if (ACPI_FAILURE(status)) { + status = acpi_ev_update_gpe_enable_masks(gpe_event_info); + if (ACPI_FAILURE(status)) return_ACPI_STATUS(status); - } /* Mark wake-enabled or HW enable, or both */ - switch (gpe_event_info->flags & ACPI_GPE_TYPE_MASK) { - case ACPI_GPE_TYPE_WAKE: - - ACPI_SET_BIT(gpe_event_info->flags, ACPI_GPE_WAKE_ENABLED); - break; - - case ACPI_GPE_TYPE_WAKE_RUN: - - ACPI_SET_BIT(gpe_event_info->flags, ACPI_GPE_WAKE_ENABLED); - - /*lint -fallthrough */ - - case ACPI_GPE_TYPE_RUNTIME: - - ACPI_SET_BIT(gpe_event_info->flags, ACPI_GPE_RUN_ENABLED); - - if (write_to_hardware) { - - /* Clear the GPE (of stale events), then enable it */ - - status = acpi_hw_clear_gpe(gpe_event_info); - if (ACPI_FAILURE(status)) { - return_ACPI_STATUS(status); - } - - /* Enable the requested runtime GPE */ - - status = acpi_hw_write_gpe_enable_reg(gpe_event_info); - } - break; + if (gpe_event_info->runtime_count && write_to_hardware) { + /* Clear the GPE (of stale events), then enable it */ + status = acpi_hw_clear_gpe(gpe_event_info); + if (ACPI_FAILURE(status)) + return_ACPI_STATUS(status); - default: - return_ACPI_STATUS(AE_BAD_PARAMETER); + /* Enable the requested runtime GPE */ + status = acpi_hw_write_gpe_enable_reg(gpe_event_info); } return_ACPI_STATUS(AE_OK); @@ -252,34 +156,9 @@ acpi_status acpi_ev_disable_gpe(struct a /* Make sure HW enable masks are updated */ - status = - acpi_ev_update_gpe_enable_masks(gpe_event_info, ACPI_GPE_DISABLE); - if (ACPI_FAILURE(status)) { + status = acpi_ev_update_gpe_enable_masks(gpe_event_info); + if (ACPI_FAILURE(status)) return_ACPI_STATUS(status); - } - - /* Clear the appropriate enabled flags for this GPE */ - - switch (gpe_event_info->flags & ACPI_GPE_TYPE_MASK) { - case ACPI_GPE_TYPE_WAKE: - ACPI_CLEAR_BIT(gpe_event_info->flags, ACPI_GPE_WAKE_ENABLED); - break; - - case ACPI_GPE_TYPE_WAKE_RUN: - ACPI_CLEAR_BIT(gpe_event_info->flags, ACPI_GPE_WAKE_ENABLED); - - /* fallthrough */ - - case ACPI_GPE_TYPE_RUNTIME: - - /* Disable the requested runtime GPE */ - - ACPI_CLEAR_BIT(gpe_event_info->flags, ACPI_GPE_RUN_ENABLED); - break; - - default: - break; - } /* * Even if we don't know the GPE type, make sure that we always Index: linux-2.6/drivers/acpi/acpica/evgpeblk.c =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/evgpeblk.c +++ linux-2.6/drivers/acpi/acpica/evgpeblk.c @@ -258,7 +258,6 @@ acpi_ev_save_method_info(acpi_handle obj u32 gpe_number; char name[ACPI_NAME_SIZE + 1]; u8 type; - acpi_status status; ACPI_FUNCTION_TRACE(ev_save_method_info); @@ -325,26 +324,20 @@ acpi_ev_save_method_info(acpi_handle obj /* * Now we can add this information to the gpe_event_info block for use - * during dispatch of this GPE. Default type is RUNTIME, although this may - * change when the _PRW methods are executed later. + * during dispatch of this GPE. */ gpe_event_info = &gpe_block->event_info[gpe_number - gpe_block->block_base_number]; - gpe_event_info->flags = (u8) - (type | ACPI_GPE_DISPATCH_METHOD | ACPI_GPE_TYPE_RUNTIME); + gpe_event_info->flags = (u8) (type | ACPI_GPE_DISPATCH_METHOD); gpe_event_info->dispatch.method_node = (struct acpi_namespace_node *)obj_handle; - /* Update enable mask, but don't enable the HW GPE as of yet */ - - status = acpi_ev_enable_gpe(gpe_event_info, FALSE); - ACPI_DEBUG_PRINT((ACPI_DB_LOAD, "Registered GPE method %s as GPE number 0x%.2X\n", name, gpe_number)); - return_ACPI_STATUS(status); + return_ACPI_STATUS(AE_OK); } /******************************************************************************* @@ -454,20 +447,7 @@ acpi_ev_match_prw_and_gpe(acpi_handle ob gpe_block-> block_base_number]; - /* Mark GPE for WAKE-ONLY but WAKE_DISABLED */ - - gpe_event_info->flags &= - ~(ACPI_GPE_WAKE_ENABLED | ACPI_GPE_RUN_ENABLED); - - status = - acpi_ev_set_gpe_type(gpe_event_info, ACPI_GPE_TYPE_WAKE); - if (ACPI_FAILURE(status)) { - goto cleanup; - } - - status = - acpi_ev_update_gpe_enable_masks(gpe_event_info, - ACPI_GPE_DISABLE); + gpe_event_info->flags |= ACPI_GPE_CAN_WAKE; } cleanup: @@ -1027,33 +1007,32 @@ acpi_ev_initialize_gpe_block(struct acpi } /* - * Enable all GPEs in this block that have these attributes: - * 1) are "runtime" or "run/wake" GPEs, and - * 2) have a corresponding _Lxx or _Exx method - * - * Any other GPEs within this block must be enabled via the - * acpi_enable_gpe() external interface. + * Enable all GPEs that have a corresponding method and aren't + * capable of generating wakeups. Any other GPEs within this block + * must be enabled via the acpi_get_gpe() interface. */ wake_gpe_count = 0; gpe_enabled_count = 0; + if (gpe_device == acpi_gbl_fadt_gpe_device) + gpe_device = NULL; for (i = 0; i < gpe_block->register_count; i++) { for (j = 0; j < 8; j++) { + int gpe_number = i * ACPI_GPE_REGISTER_WIDTH + j; /* Get the info block for this particular GPE */ + gpe_event_info = + &gpe_block->event_info[(acpi_size)gpe_number]; - gpe_event_info = &gpe_block->event_info[((acpi_size) i * - ACPI_GPE_REGISTER_WIDTH) - + j]; - - if (((gpe_event_info->flags & ACPI_GPE_DISPATCH_MASK) == - ACPI_GPE_DISPATCH_METHOD) && - (gpe_event_info->flags & ACPI_GPE_TYPE_RUNTIME)) { - gpe_enabled_count++; + if (gpe_event_info->flags & ACPI_GPE_CAN_WAKE) { + wake_gpe_count++; + continue; } - if (gpe_event_info->flags & ACPI_GPE_TYPE_WAKE) { - wake_gpe_count++; + if (gpe_event_info->flags & ACPI_GPE_DISPATCH_METHOD) { + acpi_get_gpe(gpe_device, gpe_number, + ACPI_GPE_TYPE_RUNTIME); + gpe_enabled_count++; } } } @@ -1062,15 +1041,7 @@ acpi_ev_initialize_gpe_block(struct acpi "Found %u Wake, Enabled %u Runtime GPEs in this block\n", wake_gpe_count, gpe_enabled_count)); - /* Enable all valid runtime GPEs found above */ - - status = acpi_hw_enable_runtime_gpe_block(NULL, gpe_block, NULL); - if (ACPI_FAILURE(status)) { - ACPI_ERROR((AE_INFO, "Could not enable GPEs in GpeBlock %p", - gpe_block)); - } - - return_ACPI_STATUS(status); + return_ACPI_STATUS(AE_OK); } /******************************************************************************* Index: linux-2.6/drivers/acpi/acpica/evxface.c =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/evxface.c +++ linux-2.6/drivers/acpi/acpica/evxface.c @@ -617,13 +617,6 @@ acpi_install_gpe_handler(acpi_handle gpe handler->context = context; handler->method_node = gpe_event_info->dispatch.method_node; - /* Disable the GPE before installing the handler */ - - status = acpi_ev_disable_gpe(gpe_event_info); - if (ACPI_FAILURE(status)) { - goto unlock_and_exit; - } - /* Install the handler */ flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock); @@ -707,13 +700,6 @@ acpi_remove_gpe_handler(acpi_handle gpe_ goto unlock_and_exit; } - /* Disable the GPE before removing the handler */ - - status = acpi_ev_disable_gpe(gpe_event_info); - if (ACPI_FAILURE(status)) { - goto unlock_and_exit; - } - /* Make sure all deferred tasks are completed */ (void)acpi_ut_release_mutex(ACPI_MTX_EVENTS); Index: linux-2.6/drivers/acpi/acpica/evxfevnt.c =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/evxfevnt.c +++ linux-2.6/drivers/acpi/acpica/evxfevnt.c @@ -243,8 +243,7 @@ acpi_status acpi_get_gpe(acpi_handle gpe * system into a sleep state. */ if (++gpe_event_info->wakeup_count == 1) - acpi_ev_update_gpe_enable_masks(gpe_event_info, - ACPI_GPE_ENABLE); + acpi_ev_update_gpe_enable_masks(gpe_event_info); } unlock_and_exit: @@ -294,8 +293,7 @@ acpi_status acpi_put_gpe(acpi_handle gpe * states, so we don't need to enable them here. */ if (--gpe_event_info->wakeup_count == 0) - acpi_ev_update_gpe_enable_masks(gpe_event_info, - ACPI_GPE_DISABLE); + acpi_ev_update_gpe_enable_masks(gpe_event_info); } unlock_and_exit: @@ -306,48 +304,6 @@ ACPI_EXPORT_SYMBOL(acpi_put_gpe) /******************************************************************************* * - * FUNCTION: acpi_set_gpe_type - * - * PARAMETERS: gpe_device - Parent GPE Device - * gpe_number - GPE level within the GPE block - * Type - New GPE type - * - * RETURN: Status - * - * DESCRIPTION: Set the type of an individual GPE - * - ******************************************************************************/ -acpi_status acpi_set_gpe_type(acpi_handle gpe_device, u32 gpe_number, u8 type) -{ - acpi_status status = AE_OK; - struct acpi_gpe_event_info *gpe_event_info; - - ACPI_FUNCTION_TRACE(acpi_set_gpe_type); - - /* 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->flags & ACPI_GPE_TYPE_MASK) == type) { - return_ACPI_STATUS(AE_OK); - } - - /* Set the new type (will disable GPE if currently enabled) */ - - status = acpi_ev_set_gpe_type(gpe_event_info, type); - - unlock_and_exit: - return_ACPI_STATUS(status); -} - -ACPI_EXPORT_SYMBOL(acpi_set_gpe_type) - -/******************************************************************************* - * * FUNCTION: acpi_enable_gpe * * PARAMETERS: gpe_device - Parent GPE Device Index: linux-2.6/drivers/acpi/button.c =================================================================== --- linux-2.6.orig/drivers/acpi/button.c +++ linux-2.6/drivers/acpi/button.c @@ -422,11 +422,6 @@ static int acpi_button_add(struct acpi_d if (device->wakeup.flags.valid) { /* Button's GPE is run-wake GPE */ - acpi_set_gpe_type(device->wakeup.gpe_device, - device->wakeup.gpe_number, - ACPI_GPE_TYPE_WAKE_RUN); - acpi_enable_gpe(device->wakeup.gpe_device, - device->wakeup.gpe_number); acpi_get_gpe(device->wakeup.gpe_device, device->wakeup.gpe_number, ACPI_GPE_TYPE_WAKE_RUN); Index: linux-2.6/drivers/acpi/ec.c =================================================================== --- linux-2.6.orig/drivers/acpi/ec.c +++ linux-2.6/drivers/acpi/ec.c @@ -788,7 +788,7 @@ static int ec_install_handlers(struct ac &acpi_ec_gpe_handler, ec); if (ACPI_FAILURE(status)) return -ENODEV; - acpi_set_gpe_type(NULL, ec->gpe, ACPI_GPE_TYPE_RUNTIME); + acpi_get_gpe(NULL, ec->gpe, ACPI_GPE_TYPE_RUNTIME); status = acpi_install_address_space_handler(ec->handle, ACPI_ADR_SPACE_EC, Index: linux-2.6/drivers/acpi/wakeup.c =================================================================== --- linux-2.6.orig/drivers/acpi/wakeup.c +++ linux-2.6/drivers/acpi/wakeup.c @@ -21,12 +21,12 @@ ACPI_MODULE_NAME("wakeup_devices") /** - * acpi_enable_wakeup_device_prep - prepare wakeup devices - * @sleep_state: ACPI state - * Enable all wakup devices power if the devices' wakeup level - * is higher than requested sleep level + * acpi_enable_wakeup_device_prep - Prepare wake-up devices. + * @sleep_state: ACPI system sleep state. + * + * Enable all wake-up devices' power, unless the requested system sleep state is + * too deep. */ - void acpi_enable_wakeup_device_prep(u8 sleep_state) { struct list_head *node, *next; @@ -36,9 +36,8 @@ void acpi_enable_wakeup_device_prep(u8 s struct acpi_device, wakeup_list); - if (!dev->wakeup.flags.valid || - !dev->wakeup.state.enabled || - (sleep_state > (u32) dev->wakeup.sleep_state)) + if (!dev->wakeup.flags.valid || !dev->wakeup.state.enabled + || (sleep_state > (u32) dev->wakeup.sleep_state)) continue; acpi_enable_wakeup_device_power(dev, sleep_state); @@ -46,9 +45,12 @@ void acpi_enable_wakeup_device_prep(u8 s } /** - * acpi_enable_wakeup_device - enable wakeup devices - * @sleep_state: ACPI state - * Enable all wakup devices's GPE + * acpi_enable_wakeup_device - Enable wake-up device GPEs. + * @sleep_state: ACPI system sleep state. + * + * Enable all wake-up devices' GPEs, with the assumption that + * acpi_disable_all_gpes() was executed before, so we don't need to disable any + * GPEs here. */ void acpi_enable_wakeup_device(u8 sleep_state) { @@ -65,29 +67,21 @@ void acpi_enable_wakeup_device(u8 sleep_ if (!dev->wakeup.flags.valid) continue; - /* If users want to disable run-wake GPE, - * we only disable it for wake and leave it for runtime - */ if ((!dev->wakeup.state.enabled && !dev->wakeup.prepare_count) - || sleep_state > (u32) dev->wakeup.sleep_state) { - if (dev->wakeup.flags.run_wake) { - /* set_gpe_type will disable GPE, leave it like that */ - acpi_set_gpe_type(dev->wakeup.gpe_device, - dev->wakeup.gpe_number, - ACPI_GPE_TYPE_RUNTIME); - } + || sleep_state > (u32) dev->wakeup.sleep_state) continue; - } - if (!dev->wakeup.flags.run_wake) - acpi_enable_gpe(dev->wakeup.gpe_device, - dev->wakeup.gpe_number); + + /* The wake-up power should have been enabled already. */ + acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number); } } /** - * acpi_disable_wakeup_device - disable devices' wakeup capability - * @sleep_state: ACPI state - * Disable all wakup devices's GPE and wakeup capability + * acpi_disable_wakeup_device - Disable devices' wakeup capability. + * @sleep_state: ACPI system sleep state. + * + * This function only affects devices with wakeup.state.enabled set, which means + * that it reverses the changes made by acpi_enable_wakeup_device_prep(). */ void acpi_disable_wakeup_device(u8 sleep_state) { @@ -97,30 +91,11 @@ void acpi_disable_wakeup_device(u8 sleep struct acpi_device *dev = container_of(node, struct acpi_device, wakeup_list); - if (!dev->wakeup.flags.valid) - continue; - - if ((!dev->wakeup.state.enabled && !dev->wakeup.prepare_count) - || sleep_state > (u32) dev->wakeup.sleep_state) { - if (dev->wakeup.flags.run_wake) { - acpi_set_gpe_type(dev->wakeup.gpe_device, - dev->wakeup.gpe_number, - ACPI_GPE_TYPE_WAKE_RUN); - /* Re-enable it, since set_gpe_type will disable it */ - acpi_enable_gpe(dev->wakeup.gpe_device, - dev->wakeup.gpe_number); - } + if (!dev->wakeup.flags.valid || !dev->wakeup.state.enabled + || (sleep_state > (u32) dev->wakeup.sleep_state)) continue; - } acpi_disable_wakeup_device_power(dev); - /* Never disable run-wake GPE */ - if (!dev->wakeup.flags.run_wake) { - acpi_disable_gpe(dev->wakeup.gpe_device, - dev->wakeup.gpe_number); - acpi_clear_gpe(dev->wakeup.gpe_device, - dev->wakeup.gpe_number, ACPI_NOT_ISR); - } } } @@ -136,11 +111,6 @@ int __init acpi_wakeup_device_init(void) /* In case user doesn't load button driver */ if (!dev->wakeup.flags.run_wake || dev->wakeup.state.enabled) continue; - acpi_set_gpe_type(dev->wakeup.gpe_device, - dev->wakeup.gpe_number, - ACPI_GPE_TYPE_WAKE_RUN); - acpi_enable_gpe(dev->wakeup.gpe_device, - dev->wakeup.gpe_number); acpi_get_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number, ACPI_GPE_TYPE_WAKE); dev->wakeup.state.enabled = 1; Index: linux-2.6/include/acpi/actypes.h =================================================================== --- linux-2.6.orig/include/acpi/actypes.h +++ linux-2.6/include/acpi/actypes.h @@ -668,15 +668,16 @@ typedef u32 acpi_event_status; /* * GPE info flags - Per GPE - * +-+-+-+---+---+-+ - * |7|6|5|4:3|2:1|0| - * +-+-+-+---+---+-+ - * | | | | | | - * | | | | | +--- Interrupt type: Edge or Level Triggered - * | | | | +--- Type: Wake-only, Runtime-only, or wake/runtime + * +-+-+-+---+-+-+-+ + * |7|6|5|4:3|2|1|0| + * +-+-+-+---+-+-+-+ + * | | | | | | | + * | | | | | | +--- Interrupt type: Edge or Level Triggered + * | | | | | +--- GPE can wake the system + * | | | | +--- Unused * | | | +--- Type of dispatch -- to method, handler, or none - * | | +--- Enabled for runtime? - * | +--- Enabled for wake? + * | | +--- Unused + * | +--- Unused * +--- Unused */ #define ACPI_GPE_XRUPT_TYPE_MASK (u8) 0x01 @@ -687,22 +688,13 @@ typedef u32 acpi_event_status; #define ACPI_GPE_TYPE_WAKE_RUN (u8) 0x06 #define ACPI_GPE_TYPE_WAKE (u8) 0x02 #define ACPI_GPE_TYPE_RUNTIME (u8) 0x04 /* Default */ +#define ACPI_GPE_CAN_WAKE (u8) 0x02 #define ACPI_GPE_DISPATCH_MASK (u8) 0x18 #define ACPI_GPE_DISPATCH_HANDLER (u8) 0x08 #define ACPI_GPE_DISPATCH_METHOD (u8) 0x10 #define ACPI_GPE_DISPATCH_NOT_USED (u8) 0x00 /* Default */ -#define ACPI_GPE_RUN_ENABLE_MASK (u8) 0x20 -#define ACPI_GPE_RUN_ENABLED (u8) 0x20 -#define ACPI_GPE_RUN_DISABLED (u8) 0x00 /* Default */ - -#define ACPI_GPE_WAKE_ENABLE_MASK (u8) 0x40 -#define ACPI_GPE_WAKE_ENABLED (u8) 0x40 -#define ACPI_GPE_WAKE_DISABLED (u8) 0x00 /* Default */ - -#define ACPI_GPE_ENABLE_MASK (u8) 0x60 /* Both run/wake */ - /* * Flags for GPE and Lock interfaces */ ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) 2010-02-07 2:17 ` [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) Rafael J. Wysocki ` (3 preceding siblings ...) 2010-02-07 11:56 ` [Update] Re: [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) Rafael J. Wysocki @ 2010-02-10 21:29 ` Maxim Levitsky 2010-02-10 21:36 ` Rafael J. Wysocki 4 siblings, 1 reply; 20+ messages in thread From: Maxim Levitsky @ 2010-02-10 21:29 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Moore, Robert, Matthew Garrett, linux-acpi@vger.kernel.org, Len Brown, Jesse Barnes On Sun, 2010-02-07 at 03:17 +0100, Rafael J. Wysocki wrote: > On Sunday 07 February 2010, Rafael J. Wysocki wrote: > > Hi, > > > > On Wednesday 03 February 2010, Moore, Robert wrote: > > > Matthew, > > > > > > Thanks for your response to my questions. > > > > > > I've been thinking about these interfaces: > > > > > > acpi_ref_runtime_gpe > > > acpi_ref_wakeup_gpe > > > acpi_unref_runtime_gpe > > > acpi_unref_wakeup_gpe > > > One minor, mostly off-topic question. Currently to enable static wakeup one has to write /proc/acpi/wakeup. Do you consider propagating device wakeup settings to acpi, so /proc/acpi/wakeup could finally be deprecated? Best regards, Maxim Levitsky ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) 2010-02-10 21:29 ` [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) Maxim Levitsky @ 2010-02-10 21:36 ` Rafael J. Wysocki 2010-02-11 17:36 ` Maxim Levitsky 0 siblings, 1 reply; 20+ messages in thread From: Rafael J. Wysocki @ 2010-02-10 21:36 UTC (permalink / raw) To: Maxim Levitsky Cc: Moore, Robert, Matthew Garrett, linux-acpi@vger.kernel.org, Len Brown, Jesse Barnes On Wednesday 10 February 2010, Maxim Levitsky wrote: > On Sun, 2010-02-07 at 03:17 +0100, Rafael J. Wysocki wrote: > > On Sunday 07 February 2010, Rafael J. Wysocki wrote: > > > Hi, > > > > > > On Wednesday 03 February 2010, Moore, Robert wrote: > > > > Matthew, > > > > > > > > Thanks for your response to my questions. > > > > > > > > I've been thinking about these interfaces: > > > > > > > > acpi_ref_runtime_gpe > > > > acpi_ref_wakeup_gpe > > > > acpi_unref_runtime_gpe > > > > acpi_unref_wakeup_gpe > > > > > > One minor, mostly off-topic question. > Currently to enable static wakeup one has to write /proc/acpi/wakeup. That depends on the device. For PCI devices there's another interface for that, which is /sys/devices/.../power/wakeup . > Do you consider propagating device wakeup settings to acpi, > so /proc/acpi/wakeup could finally be deprecated? We already do that for PCI devices. In fact, I'd only recommend using /proc/acpi/wakeup for reading the list of wakeup devices visible to ACPI. Rafael ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) 2010-02-10 21:36 ` Rafael J. Wysocki @ 2010-02-11 17:36 ` Maxim Levitsky 2010-02-11 20:34 ` Rafael J. Wysocki 0 siblings, 1 reply; 20+ messages in thread From: Maxim Levitsky @ 2010-02-11 17:36 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Moore, Robert, Matthew Garrett, linux-acpi@vger.kernel.org, Len Brown, Jesse Barnes On Wed, 2010-02-10 at 22:36 +0100, Rafael J. Wysocki wrote: > On Wednesday 10 February 2010, Maxim Levitsky wrote: > > On Sun, 2010-02-07 at 03:17 +0100, Rafael J. Wysocki wrote: > > > On Sunday 07 February 2010, Rafael J. Wysocki wrote: > > > > Hi, > > > > > > > > On Wednesday 03 February 2010, Moore, Robert wrote: > > > > > Matthew, > > > > > > > > > > Thanks for your response to my questions. > > > > > > > > > > I've been thinking about these interfaces: > > > > > > > > > > acpi_ref_runtime_gpe > > > > > acpi_ref_wakeup_gpe > > > > > acpi_unref_runtime_gpe > > > > > acpi_unref_wakeup_gpe > > > > > > > > > One minor, mostly off-topic question. > > Currently to enable static wakeup one has to write /proc/acpi/wakeup. > > That depends on the device. For PCI devices there's another interface for > that, which is /sys/devices/.../power/wakeup . > > > Do you consider propagating device wakeup settings to acpi, > > so /proc/acpi/wakeup could finally be deprecated? > > We already do that for PCI devices. Not in 2.6.33-rc6 maxim@maxim-laptop:~$ cat /proc/acpi/wakeup Device S-state Status Sysfs node UHC1 S3 disabled pci:0000:00:1d.0 UHC2 S3 disabled pci:0000:00:1d.1 UHC3 S3 disabled pci:0000:00:1d.2 UHC4 S3 disabled pci:0000:00:1a.0 UHC5 S3 disabled pci:0000:00:1a.1 EHC1 S3 disabled pci:0000:00:1d.7 EHC2 S3 disabled pci:0000:00:1a.7 EXP3 S4 disabled pci:0000:00:1c.2 EXP5 S4 disabled EXP6 S4 disabled AZAL S4 disabled pci:0000:00:1b.0 MODM S4 disabled maxim@maxim-laptop:~$ echo enabled | sudo tee /sys/devices/pci0000 \:00/0000\:00\:1d.0/power/wakeup enabled cat /sys/devices/pci0000\:00/0000\:00\:1d.0/power/wakeup enabled maxim@maxim-laptop:~$ cat /proc/acpi/wakeup Device S-state Status Sysfs node UHC1 S3 disabled pci:0000:00:1d.0 UHC2 S3 disabled pci:0000:00:1d.1 UHC3 S3 disabled pci:0000:00:1d.2 UHC4 S3 disabled pci:0000:00:1a.0 UHC5 S3 disabled pci:0000:00:1a.1 EHC1 S3 disabled pci:0000:00:1d.7 EHC2 S3 disabled pci:0000:00:1a.7 EXP3 S4 disabled pci:0000:00:1c.2 EXP5 S4 disabled EXP6 S4 disabled AZAL S4 disabled pci:0000:00:1b.0 MODM S4 disabled Or am I mistaken somehow? Best regards, Maxim Levitsky ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) 2010-02-11 17:36 ` Maxim Levitsky @ 2010-02-11 20:34 ` Rafael J. Wysocki 2010-02-13 16:08 ` Maxim Levitsky 0 siblings, 1 reply; 20+ messages in thread From: Rafael J. Wysocki @ 2010-02-11 20:34 UTC (permalink / raw) To: Maxim Levitsky Cc: Moore, Robert, Matthew Garrett, linux-acpi@vger.kernel.org, Len Brown, Jesse Barnes On Thursday 11 February 2010, Maxim Levitsky wrote: > On Wed, 2010-02-10 at 22:36 +0100, Rafael J. Wysocki wrote: > > On Wednesday 10 February 2010, Maxim Levitsky wrote: > > > On Sun, 2010-02-07 at 03:17 +0100, Rafael J. Wysocki wrote: > > > > On Sunday 07 February 2010, Rafael J. Wysocki wrote: > > > > > Hi, > > > > > > > > > > On Wednesday 03 February 2010, Moore, Robert wrote: > > > > > > Matthew, > > > > > > > > > > > > Thanks for your response to my questions. > > > > > > > > > > > > I've been thinking about these interfaces: > > > > > > > > > > > > acpi_ref_runtime_gpe > > > > > > acpi_ref_wakeup_gpe > > > > > > acpi_unref_runtime_gpe > > > > > > acpi_unref_wakeup_gpe > > > > > > > > > > > > One minor, mostly off-topic question. > > > Currently to enable static wakeup one has to write /proc/acpi/wakeup. > > > > That depends on the device. For PCI devices there's another interface for > > that, which is /sys/devices/.../power/wakeup . > > > > > Do you consider propagating device wakeup settings to acpi, > > > so /proc/acpi/wakeup could finally be deprecated? > > > > We already do that for PCI devices. > > Not in 2.6.33-rc6 > > maxim@maxim-laptop:~$ cat /proc/acpi/wakeup > Device S-state Status Sysfs node > UHC1 S3 disabled pci:0000:00:1d.0 > UHC2 S3 disabled pci:0000:00:1d.1 > UHC3 S3 disabled pci:0000:00:1d.2 > UHC4 S3 disabled pci:0000:00:1a.0 > UHC5 S3 disabled pci:0000:00:1a.1 > EHC1 S3 disabled pci:0000:00:1d.7 > EHC2 S3 disabled pci:0000:00:1a.7 > EXP3 S4 disabled pci:0000:00:1c.2 > EXP5 S4 disabled > EXP6 S4 disabled > AZAL S4 disabled pci:0000:00:1b.0 > MODM S4 disabled > > maxim@maxim-laptop:~$ echo enabled | sudo tee /sys/devices/pci0000 > \:00/0000\:00\:1d.0/power/wakeup > enabled > > cat /sys/devices/pci0000\:00/0000\:00\:1d.0/power/wakeup enabled > > > maxim@maxim-laptop:~$ cat /proc/acpi/wakeup > Device S-state Status Sysfs node > UHC1 S3 disabled pci:0000:00:1d.0 > UHC2 S3 disabled pci:0000:00:1d.1 > UHC3 S3 disabled pci:0000:00:1d.2 > UHC4 S3 disabled pci:0000:00:1a.0 > UHC5 S3 disabled pci:0000:00:1a.1 > EHC1 S3 disabled pci:0000:00:1d.7 > EHC2 S3 disabled pci:0000:00:1a.7 > EXP3 S4 disabled pci:0000:00:1c.2 > EXP5 S4 disabled > EXP6 S4 disabled > AZAL S4 disabled pci:0000:00:1b.0 > MODM S4 disabled > > > Or am I mistaken somehow? You are. Please ignore the "Status" column in /proc/acpi/wakeup, it shows the value of a flag that's not used for PCI devices (unless you set it via /proc/acpi/wakeup). Rafael ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) 2010-02-11 20:34 ` Rafael J. Wysocki @ 2010-02-13 16:08 ` Maxim Levitsky 2010-02-14 2:24 ` Rafael J. Wysocki 0 siblings, 1 reply; 20+ messages in thread From: Maxim Levitsky @ 2010-02-13 16:08 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Moore, Robert, Matthew Garrett, linux-acpi@vger.kernel.org, Len Brown, Jesse Barnes On Thu, 2010-02-11 at 21:34 +0100, Rafael J. Wysocki wrote: > On Thursday 11 February 2010, Maxim Levitsky wrote: > > On Wed, 2010-02-10 at 22:36 +0100, Rafael J. Wysocki wrote: > > > On Wednesday 10 February 2010, Maxim Levitsky wrote: > > > > On Sun, 2010-02-07 at 03:17 +0100, Rafael J. Wysocki wrote: > > > > > On Sunday 07 February 2010, Rafael J. Wysocki wrote: > > > > > > Hi, > > > > > > > > > > > > On Wednesday 03 February 2010, Moore, Robert wrote: > > > > > > > Matthew, > > > > > > > > > > > > > > Thanks for your response to my questions. > > > > > > > > > > > > > > I've been thinking about these interfaces: > > > > > > > > > > > > > > acpi_ref_runtime_gpe > > > > > > > acpi_ref_wakeup_gpe > > > > > > > acpi_unref_runtime_gpe > > > > > > > acpi_unref_wakeup_gpe > > > > > > > > > > > > > > > One minor, mostly off-topic question. > > > > Currently to enable static wakeup one has to write /proc/acpi/wakeup. > > > > > > That depends on the device. For PCI devices there's another interface for > > > that, which is /sys/devices/.../power/wakeup . > > > > > > > Do you consider propagating device wakeup settings to acpi, > > > > so /proc/acpi/wakeup could finally be deprecated? > > > > > > We already do that for PCI devices. > > > > Not in 2.6.33-rc6 > > > > maxim@maxim-laptop:~$ cat /proc/acpi/wakeup > > Device S-state Status Sysfs node > > UHC1 S3 disabled pci:0000:00:1d.0 > > UHC2 S3 disabled pci:0000:00:1d.1 > > UHC3 S3 disabled pci:0000:00:1d.2 > > UHC4 S3 disabled pci:0000:00:1a.0 > > UHC5 S3 disabled pci:0000:00:1a.1 > > EHC1 S3 disabled pci:0000:00:1d.7 > > EHC2 S3 disabled pci:0000:00:1a.7 > > EXP3 S4 disabled pci:0000:00:1c.2 > > EXP5 S4 disabled > > EXP6 S4 disabled > > AZAL S4 disabled pci:0000:00:1b.0 > > MODM S4 disabled > > > > maxim@maxim-laptop:~$ echo enabled | sudo tee /sys/devices/pci0000 > > \:00/0000\:00\:1d.0/power/wakeup > > enabled > > > > cat /sys/devices/pci0000\:00/0000\:00\:1d.0/power/wakeup enabled > > > > > > maxim@maxim-laptop:~$ cat /proc/acpi/wakeup > > Device S-state Status Sysfs node > > UHC1 S3 disabled pci:0000:00:1d.0 > > UHC2 S3 disabled pci:0000:00:1d.1 > > UHC3 S3 disabled pci:0000:00:1d.2 > > UHC4 S3 disabled pci:0000:00:1a.0 > > UHC5 S3 disabled pci:0000:00:1a.1 > > EHC1 S3 disabled pci:0000:00:1d.7 > > EHC2 S3 disabled pci:0000:00:1a.7 > > EXP3 S4 disabled pci:0000:00:1c.2 > > EXP5 S4 disabled > > EXP6 S4 disabled > > AZAL S4 disabled pci:0000:00:1b.0 > > MODM S4 disabled > > > > > > Or am I mistaken somehow? > > You are. Please ignore the "Status" column in /proc/acpi/wakeup, it shows > the value of a flag that's not used for PCI devices (unless you set it via > /proc/acpi/wakeup It is indeed true. PnP device too are supported in this way? Then I think it would be very nice to remove all pci and pnp devices from this list to avoid confusion. Best regards, Maxim Levitsky ^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) 2010-02-13 16:08 ` Maxim Levitsky @ 2010-02-14 2:24 ` Rafael J. Wysocki 0 siblings, 0 replies; 20+ messages in thread From: Rafael J. Wysocki @ 2010-02-14 2:24 UTC (permalink / raw) To: Maxim Levitsky Cc: Moore, Robert, Matthew Garrett, linux-acpi@vger.kernel.org, Len Brown, Jesse Barnes On Saturday 13 February 2010, Maxim Levitsky wrote: > On Thu, 2010-02-11 at 21:34 +0100, Rafael J. Wysocki wrote: > > On Thursday 11 February 2010, Maxim Levitsky wrote: > > > On Wed, 2010-02-10 at 22:36 +0100, Rafael J. Wysocki wrote: > > > > On Wednesday 10 February 2010, Maxim Levitsky wrote: > > > > > On Sun, 2010-02-07 at 03:17 +0100, Rafael J. Wysocki wrote: > > > > > > On Sunday 07 February 2010, Rafael J. Wysocki wrote: > > > > > > > Hi, > > > > > > > > > > > > > > On Wednesday 03 February 2010, Moore, Robert wrote: > > > > > > > > Matthew, > > > > > > > > > > > > > > > > Thanks for your response to my questions. > > > > > > > > > > > > > > > > I've been thinking about these interfaces: > > > > > > > > > > > > > > > > acpi_ref_runtime_gpe > > > > > > > > acpi_ref_wakeup_gpe > > > > > > > > acpi_unref_runtime_gpe > > > > > > > > acpi_unref_wakeup_gpe > > > > > > > > > > > > > > > > > > One minor, mostly off-topic question. > > > > > Currently to enable static wakeup one has to write /proc/acpi/wakeup. > > > > > > > > That depends on the device. For PCI devices there's another interface for > > > > that, which is /sys/devices/.../power/wakeup . > > > > > > > > > Do you consider propagating device wakeup settings to acpi, > > > > > so /proc/acpi/wakeup could finally be deprecated? > > > > > > > > We already do that for PCI devices. > > > > > > Not in 2.6.33-rc6 > > > > > > maxim@maxim-laptop:~$ cat /proc/acpi/wakeup > > > Device S-state Status Sysfs node > > > UHC1 S3 disabled pci:0000:00:1d.0 > > > UHC2 S3 disabled pci:0000:00:1d.1 > > > UHC3 S3 disabled pci:0000:00:1d.2 > > > UHC4 S3 disabled pci:0000:00:1a.0 > > > UHC5 S3 disabled pci:0000:00:1a.1 > > > EHC1 S3 disabled pci:0000:00:1d.7 > > > EHC2 S3 disabled pci:0000:00:1a.7 > > > EXP3 S4 disabled pci:0000:00:1c.2 > > > EXP5 S4 disabled > > > EXP6 S4 disabled > > > AZAL S4 disabled pci:0000:00:1b.0 > > > MODM S4 disabled > > > > > > maxim@maxim-laptop:~$ echo enabled | sudo tee /sys/devices/pci0000 > > > \:00/0000\:00\:1d.0/power/wakeup > > > enabled > > > > > > cat /sys/devices/pci0000\:00/0000\:00\:1d.0/power/wakeup enabled > > > > > > > > > maxim@maxim-laptop:~$ cat /proc/acpi/wakeup > > > Device S-state Status Sysfs node > > > UHC1 S3 disabled pci:0000:00:1d.0 > > > UHC2 S3 disabled pci:0000:00:1d.1 > > > UHC3 S3 disabled pci:0000:00:1d.2 > > > UHC4 S3 disabled pci:0000:00:1a.0 > > > UHC5 S3 disabled pci:0000:00:1a.1 > > > EHC1 S3 disabled pci:0000:00:1d.7 > > > EHC2 S3 disabled pci:0000:00:1a.7 > > > EXP3 S4 disabled pci:0000:00:1c.2 > > > EXP5 S4 disabled > > > EXP6 S4 disabled > > > AZAL S4 disabled pci:0000:00:1b.0 > > > MODM S4 disabled > > > > > > > > > Or am I mistaken somehow? > > > > You are. Please ignore the "Status" column in /proc/acpi/wakeup, it shows > > the value of a flag that's not used for PCI devices (unless you set it via > > /proc/acpi/wakeup > > It is indeed true. > > PnP device too are supported in this way? I don't know really. > Then I think it would be very nice to remove all pci and pnp devices > from this list to avoid confusion. I wouldn't do that, because it allows us to see easily which devices are seen by ACPI as wakeup-capable. Perhaps the format of the file may be changed, though. Rafael ^ permalink raw reply [flat|nested] 20+ messages in thread
* RE: Recent GPE patches - some questions. 2010-02-06 23:31 ` Rafael J. Wysocki 2010-02-07 2:17 ` [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) Rafael J. Wysocki @ 2010-02-10 21:36 ` Moore, Robert 2010-02-11 22:51 ` [PATCH] ACPI: Use GPE reference counting to support shared GPEs (was: Re: Recent GPE patches - some questions.) Rafael J. Wysocki 1 sibling, 1 reply; 20+ messages in thread From: Moore, Robert @ 2010-02-10 21:36 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Matthew Garrett, linux-acpi@vger.kernel.org, Len Brown >-----Original Message----- >From: Rafael J. Wysocki [mailto:rjw@sisk.pl] >Sent: Saturday, February 06, 2010 3:32 PM >To: Moore, Robert >Cc: Matthew Garrett; linux-acpi@vger.kernel.org; Len Brown >Subject: Re: Recent GPE patches - some questions. > >Hi, > >On Wednesday 03 February 2010, Moore, Robert wrote: >> Matthew, >> >> Thanks for your response to my questions. >> >> I've been thinking about these interfaces: >> >> acpi_ref_runtime_gpe >> acpi_ref_wakeup_gpe >> acpi_unref_runtime_gpe >> acpi_unref_wakeup_gpe >> >> While I understand the need for the reference counting mechanism, it is a >> good idea to support the shared GPEs, I think it may be simpler and >cleaner >> to simply not directly expose this mechanism to GPE users. > >Well, it already is exposed to the users and in a way that doesn't look >very >clean to me. Namely, to enable a wakeup GPE for run time, the caller needs >to >set its type to ACPI_GPE_TYPE_WAKE_RUN before calling acpi_enable_gpe(), >while with the Matthew's interface the only thing the caller would need to >do >would be calling acpi_ref_runtime_gpe(). > >> I'm suggesting that we add the reference counting mechanism to the >existing >> AcpiEnableGpe and AcpiDisableGpe interfaces and update their >descriptions. >> We already hide the differences between wake, run, and wake-run GPEs >behind >> these interfaces. > >Not really, as I said above. > >Moreover, we need two reference counters, because there are cases when a >GPE should be enabled for wakeup and not enabled for run time and vice >versa. > >> Adding the reference count semantic to these interfaces changes their >> behavior in a fairly simple way: >> >> Support for shared GPEs: >> AcpiEnableGpe: For a given GPE, it is actually enabled only on the first >call. >> AcpiDisableGpe: For a given GPE, it is actually disabled only on the last >call. > >I suppose you mean acpi_enable_gpe() and acpi_disable_gpe(). > >That wouldn't work, because sometimes we need to actually hardware-disable >GPEs and hardware-enable them regardless of the refcount mechanism, like >for example in the EC suspend and resume routines. Yes, this makes sense, even if the EC GPE is shared. > >That said, if you are afraid that the new interface may be cumbersome for >the >callers, I think we can introduce just two callbacks, > >acpi_get_gpe() >acpi_put_gpe() > >taking 3 arguments each, where the two first arguments are like for the >Matthew's callbacks and the third argument is a mask of two bits: > >ACPI_GPE_TYPE_WAKE >ACPI_GPE_TYPE_RUNTIME > >that will tell the callback whether to use the wakeup or runtime counter >for >reference counting (if called with ACPI_GPE_TYPE_WAKE_RUN, both >reference counters will be modified at the same time). > >Please tell me what you think. > >Rafael Good, I like having the two interfaces better than the original four. However, I'm having some trouble with the naming. I think I would like to keep acpi_enable_gpe and acpi_disable_gpe as the high-level enable/disable interfaces that may or may not actually touch the hardware depending on reference count, run vs. wake, etc. Thus, enable_gpe and disable_gpe remain similar to today (with addition of new reference count mechanisms) and would correspond to put_gpe and get_gpe above. I think this also has the advantage of reducing changes to existing drivers across all operating systems supported by ACPICA. Then, we have the special case where a driver must absolutely (H/W) enable/disable a GPE, regardless of sharing. Probably only the EC driver needs this. I would like to see new interfaces for these. Actually, one new interface would probably suffice. Something like: acpi_set_gpe (ENABLE | DISABLE) Bob ^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH] ACPI: Use GPE reference counting to support shared GPEs (was: Re: Recent GPE patches - some questions.) 2010-02-10 21:36 ` Recent GPE patches - some questions Moore, Robert @ 2010-02-11 22:51 ` Rafael J. Wysocki 0 siblings, 0 replies; 20+ messages in thread From: Rafael J. Wysocki @ 2010-02-11 22:51 UTC (permalink / raw) To: Moore, Robert Cc: Matthew Garrett, linux-acpi@vger.kernel.org, Len Brown, pm list, Jesse Barnes On Wednesday 10 February 2010, Moore, Robert wrote: > > >-----Original Message----- > >From: Rafael J. Wysocki [mailto:rjw@sisk.pl] > >Sent: Saturday, February 06, 2010 3:32 PM > >To: Moore, Robert > >Cc: Matthew Garrett; linux-acpi@vger.kernel.org; Len Brown > >Subject: Re: Recent GPE patches - some questions. ... > >Moreover, we need two reference counters, because there are cases when a > >GPE should be enabled for wakeup and not enabled for run time and vice > >versa. > > > >> Adding the reference count semantic to these interfaces changes their > >> behavior in a fairly simple way: > >> > >> Support for shared GPEs: > >> AcpiEnableGpe: For a given GPE, it is actually enabled only on the first > >call. > >> AcpiDisableGpe: For a given GPE, it is actually disabled only on the last > >call. > > > >I suppose you mean acpi_enable_gpe() and acpi_disable_gpe(). > > > >That wouldn't work, because sometimes we need to actually hardware-disable > >GPEs and hardware-enable them regardless of the refcount mechanism, like > >for example in the EC suspend and resume routines. > > Yes, this makes sense, even if the EC GPE is shared. > > > > >That said, if you are afraid that the new interface may be cumbersome for > >the > >callers, I think we can introduce just two callbacks, > > > >acpi_get_gpe() > >acpi_put_gpe() > > > >taking 3 arguments each, where the two first arguments are like for the > >Matthew's callbacks and the third argument is a mask of two bits: > > > >ACPI_GPE_TYPE_WAKE > >ACPI_GPE_TYPE_RUNTIME > > > >that will tell the callback whether to use the wakeup or runtime counter > >for > >reference counting (if called with ACPI_GPE_TYPE_WAKE_RUN, both > >reference counters will be modified at the same time). > > > >Please tell me what you think. > > Good, I like having the two interfaces better than the original four. > > However, I'm having some trouble with the naming. I think I would like to > keep acpi_enable_gpe and acpi_disable_gpe as the high-level enable/disable > interfaces that may or may not actually touch the hardware depending on > reference count, run vs. wake, etc. Thus, enable_gpe and disable_gpe remain > similar to today (with addition of new reference count mechanisms) and would > correspond to put_gpe and get_gpe above. I think this also has the advantage > of reducing changes to existing drivers across all operating systems > supported by ACPICA. > > Then, we have the special case where a driver must absolutely (H/W) > enable/disable a GPE, regardless of sharing. Probably only the EC driver > needs this. I would like to see new interfaces for these. Actually, one new > interface would probably suffice. Something like: > > acpi_set_gpe (ENABLE | DISABLE) OK I had to fold the 3 patches into the appended one. I used the names like you suggested. The patch has been initially tested on a few machines and appears to work fine. Rafael --- Subject: ACPI: Use GPE reference counting to support shared GPEs From: Rafael J. Wysocki <rjw@sisk.pl> 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. Remove the old GPE type handling entirely, which gets rid of various quirks, like the implicit disabling with GPE type setting. This requires a small amount of rework in order to ensure that non-wake GPEs are enabled by default to preserve existing behaviour. Based on patches from Matthew Garrett <mjg@redhat.com>. Signed-off-by: Matthew Garrett <mjg@redhat.com> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> --- drivers/acpi/acpica/acevents.h | 6 - drivers/acpi/acpica/aclocal.h | 2 drivers/acpi/acpica/evgpe.c | 153 ++++------------------------------------- drivers/acpi/acpica/evgpeblk.c | 87 ++++++++--------------- drivers/acpi/acpica/evxface.c | 14 --- drivers/acpi/acpica/evxfevnt.c | 90 +++++++++++++++++------- drivers/acpi/button.c | 13 ++- drivers/acpi/ec.c | 14 ++- drivers/acpi/sleep.c | 15 +++- drivers/acpi/system.c | 4 - drivers/acpi/wakeup.c | 81 +++++++-------------- include/acpi/acpixf.h | 6 - include/acpi/actypes.h | 28 ++----- 13 files changed, 188 insertions(+), 325 deletions(-) Index: linux-2.6/drivers/acpi/acpica/aclocal.h =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/aclocal.h +++ linux-2.6/drivers/acpi/acpica/aclocal.h @@ -426,6 +426,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 */ Index: linux-2.6/drivers/acpi/acpica/evxfevnt.c =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/evxfevnt.c +++ linux-2.6/drivers/acpi/acpica/evxfevnt.c @@ -201,23 +201,27 @@ ACPI_EXPORT_SYMBOL(acpi_enable_event) /******************************************************************************* * - * FUNCTION: acpi_set_gpe_type + * FUNCTION: acpi_set_gpe * * PARAMETERS: gpe_device - Parent GPE Device * gpe_number - GPE level within the GPE block - * Type - New GPE type + * action - Enable or disable + * Called from ISR or not * * RETURN: Status * - * DESCRIPTION: Set the type of an individual GPE + * DESCRIPTION: Enable or disable an ACPI event (general purpose) * ******************************************************************************/ -acpi_status acpi_set_gpe_type(acpi_handle gpe_device, u32 gpe_number, u8 type) +acpi_status acpi_set_gpe(acpi_handle gpe_device, u32 gpe_number, u8 action) { acpi_status status = AE_OK; + acpi_cpu_flags flags; struct acpi_gpe_event_info *gpe_event_info; - ACPI_FUNCTION_TRACE(acpi_set_gpe_type); + ACPI_FUNCTION_TRACE(acpi_set_gpe); + + flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock); /* Ensure that we have a valid GPE number */ @@ -227,19 +231,29 @@ acpi_status acpi_set_gpe_type(acpi_handl goto unlock_and_exit; } - if ((gpe_event_info->flags & ACPI_GPE_TYPE_MASK) == type) { - return_ACPI_STATUS(AE_OK); - } + /* Perform the action */ - /* Set the new type (will disable GPE if currently enabled) */ + switch (action) { + case ACPI_GPE_ENABLE: + status = acpi_ev_enable_gpe(gpe_event_info, TRUE); + break; + + case ACPI_GPE_DISABLE: + status = acpi_ev_disable_gpe(gpe_event_info); + break; - status = acpi_ev_set_gpe_type(gpe_event_info, type); + default: + ACPI_ERROR((AE_INFO, "Invalid action\n")); + status = AE_BAD_PARAMETER; + break; + } unlock_and_exit: + acpi_os_release_lock(acpi_gbl_gpe_lock, flags); return_ACPI_STATUS(status); } -ACPI_EXPORT_SYMBOL(acpi_set_gpe_type) +ACPI_EXPORT_SYMBOL(acpi_set_gpe) /******************************************************************************* * @@ -247,15 +261,14 @@ ACPI_EXPORT_SYMBOL(acpi_set_gpe_type) * * PARAMETERS: gpe_device - Parent GPE Device * gpe_number - GPE level within the GPE block - * Flags - Just enable, or also wake enable? - * Called from ISR or not + * type - Purpose the GPE will be used for * * RETURN: Status * - * DESCRIPTION: Enable an ACPI event (general purpose) + * DESCRIPTION: Take a reference to a GPE and enable it if necessary * ******************************************************************************/ -acpi_status acpi_enable_gpe(acpi_handle gpe_device, u32 gpe_number) +acpi_status acpi_enable_gpe(acpi_handle gpe_device, u32 gpe_number, u8 type) { acpi_status status = AE_OK; acpi_cpu_flags flags; @@ -273,15 +286,32 @@ acpi_status acpi_enable_gpe(acpi_handle goto unlock_and_exit; } - /* Perform the enable */ + if (type & ACPI_GPE_TYPE_RUNTIME) { + if (++gpe_event_info->runtime_count == 1) + status = acpi_ev_enable_gpe(gpe_event_info, TRUE); - status = acpi_ev_enable_gpe(gpe_event_info, TRUE); + if (ACPI_FAILURE(status)) + gpe_event_info->runtime_count--; + } - unlock_and_exit: + if (type & ACPI_GPE_TYPE_WAKE) { + if (!(gpe_event_info->flags & ACPI_GPE_CAN_WAKE)) { + status = AE_BAD_PARAMETER; + goto unlock_and_exit; + } + + /* + * Wake-up GPEs are only enabled right prior to putting the + * system into a sleep state. + */ + if (++gpe_event_info->wakeup_count == 1) + acpi_ev_update_gpe_enable_masks(gpe_event_info); + } + +unlock_and_exit: acpi_os_release_lock(acpi_gbl_gpe_lock, flags); return_ACPI_STATUS(status); } - ACPI_EXPORT_SYMBOL(acpi_enable_gpe) /******************************************************************************* @@ -290,15 +320,14 @@ ACPI_EXPORT_SYMBOL(acpi_enable_gpe) * * PARAMETERS: gpe_device - Parent GPE Device * gpe_number - GPE level within the GPE block - * Flags - Just disable, or also wake disable? - * Called from ISR or not + * type - Purpose the GPE won't be used for any more * * RETURN: Status * - * DESCRIPTION: Disable an ACPI event (general purpose) + * DESCRIPTION: Release a reference to a GPE and disable it if necessary * ******************************************************************************/ -acpi_status acpi_disable_gpe(acpi_handle gpe_device, u32 gpe_number) +acpi_status acpi_disable_gpe(acpi_handle gpe_device, u32 gpe_number, u8 type) { acpi_status status = AE_OK; acpi_cpu_flags flags; @@ -315,13 +344,24 @@ acpi_status acpi_disable_gpe(acpi_handle goto unlock_and_exit; } - status = acpi_ev_disable_gpe(gpe_event_info); + if ((type & ACPI_GPE_TYPE_WAKE) && gpe_event_info->runtime_count) { + if (--gpe_event_info->runtime_count == 0) + acpi_ev_disable_gpe(gpe_event_info); + } + + if ((type & ACPI_GPE_TYPE_RUNTIME) && gpe_event_info->wakeup_count) { + /* + * Wake-up GPEs are not enabled after leaving system sleep + * states, so we don't need to disable them here. + */ + if (--gpe_event_info->wakeup_count == 0) + acpi_ev_update_gpe_enable_masks(gpe_event_info); + } unlock_and_exit: acpi_os_release_lock(acpi_gbl_gpe_lock, flags); return_ACPI_STATUS(status); } - ACPI_EXPORT_SYMBOL(acpi_disable_gpe) /******************************************************************************* Index: linux-2.6/include/acpi/acpixf.h =================================================================== --- linux-2.6.orig/include/acpi/acpixf.h +++ linux-2.6/include/acpi/acpixf.h @@ -281,11 +281,11 @@ acpi_status acpi_get_event_status(u32 ev /* * GPE Interfaces */ -acpi_status acpi_set_gpe_type(acpi_handle gpe_device, u32 gpe_number, u8 type); +acpi_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); +acpi_status acpi_enable_gpe(acpi_handle gpe_device, u32 gpe_number, u8 type); -acpi_status acpi_disable_gpe(acpi_handle gpe_device, u32 gpe_number); +acpi_status acpi_disable_gpe(acpi_handle gpe_device, u32 gpe_number, u8 type); acpi_status acpi_clear_gpe(acpi_handle gpe_device, u32 gpe_number, u32 flags); Index: linux-2.6/drivers/acpi/button.c =================================================================== --- linux-2.6.orig/drivers/acpi/button.c +++ linux-2.6/drivers/acpi/button.c @@ -422,11 +422,9 @@ static int acpi_button_add(struct acpi_d if (device->wakeup.flags.valid) { /* Button's GPE is run-wake GPE */ - acpi_set_gpe_type(device->wakeup.gpe_device, - device->wakeup.gpe_number, - ACPI_GPE_TYPE_WAKE_RUN); acpi_enable_gpe(device->wakeup.gpe_device, - device->wakeup.gpe_number); + device->wakeup.gpe_number, + ACPI_GPE_TYPE_WAKE_RUN); device->wakeup.state.enabled = 1; } @@ -446,6 +444,13 @@ static int acpi_button_remove(struct acp { struct acpi_button *button = acpi_driver_data(device); + if (device->wakeup.flags.valid) { + acpi_disable_gpe(device->wakeup.gpe_device, + device->wakeup.gpe_number, + ACPI_GPE_TYPE_WAKE_RUN); + device->wakeup.state.enabled = 0; + } + acpi_button_remove_fs(device); input_unregister_device(button->input); kfree(button); Index: linux-2.6/drivers/acpi/ec.c =================================================================== --- linux-2.6.orig/drivers/acpi/ec.c +++ linux-2.6/drivers/acpi/ec.c @@ -307,7 +307,7 @@ static int acpi_ec_transaction(struct ac pr_debug(PREFIX "transaction start\n"); /* disable GPE during transaction if storm is detected */ if (test_bit(EC_FLAGS_GPE_STORM, &ec->flags)) { - acpi_disable_gpe(NULL, ec->gpe); + acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_DISABLE); } status = acpi_ec_transaction_unlocked(ec, t); @@ -317,7 +317,7 @@ static int acpi_ec_transaction(struct ac if (test_bit(EC_FLAGS_GPE_STORM, &ec->flags)) { msleep(1); /* it is safe to enable GPE outside of transaction */ - acpi_enable_gpe(NULL, ec->gpe); + acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_ENABLE); } else if (t->irq_count > ACPI_EC_STORM_THRESHOLD) { pr_info(PREFIX "GPE storm detected, " "transactions will use polling mode\n"); @@ -788,8 +788,8 @@ static int ec_install_handlers(struct ac &acpi_ec_gpe_handler, ec); if (ACPI_FAILURE(status)) return -ENODEV; - acpi_set_gpe_type(NULL, ec->gpe, ACPI_GPE_TYPE_RUNTIME); - acpi_enable_gpe(NULL, ec->gpe); + + acpi_enable_gpe(NULL, ec->gpe, ACPI_GPE_TYPE_RUNTIME); status = acpi_install_address_space_handler(ec->handle, ACPI_ADR_SPACE_EC, &acpi_ec_space_handler, @@ -806,6 +806,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); return -ENODEV; } } @@ -816,6 +817,7 @@ static int ec_install_handlers(struct ac static void ec_remove_handlers(struct acpi_ec *ec) { + acpi_disable_gpe(NULL, ec->gpe, ACPI_GPE_TYPE_RUNTIME); 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"); @@ -1058,7 +1060,7 @@ static int acpi_ec_suspend(struct acpi_d { struct acpi_ec *ec = acpi_driver_data(device); /* Stop using GPE */ - acpi_disable_gpe(NULL, ec->gpe); + acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_DISABLE); return 0; } @@ -1066,7 +1068,7 @@ static int acpi_ec_resume(struct acpi_de { struct acpi_ec *ec = acpi_driver_data(device); /* Enable use of GPE back */ - acpi_enable_gpe(NULL, ec->gpe); + acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_ENABLE); return 0; } Index: linux-2.6/drivers/acpi/sleep.c =================================================================== --- linux-2.6.orig/drivers/acpi/sleep.c +++ linux-2.6/drivers/acpi/sleep.c @@ -745,9 +745,18 @@ int acpi_pm_device_sleep_wake(struct dev return -ENODEV; } - error = enable ? - acpi_enable_wakeup_device_power(adev, acpi_target_sleep_state) : - acpi_disable_wakeup_device_power(adev); + 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); + } if (!error) dev_info(dev, "wake-up capability %s by ACPI\n", enable ? "enabled" : "disabled"); Index: linux-2.6/drivers/acpi/wakeup.c =================================================================== --- linux-2.6.orig/drivers/acpi/wakeup.c +++ linux-2.6/drivers/acpi/wakeup.c @@ -21,12 +21,12 @@ ACPI_MODULE_NAME("wakeup_devices") /** - * acpi_enable_wakeup_device_prep - prepare wakeup devices - * @sleep_state: ACPI state - * Enable all wakup devices power if the devices' wakeup level - * is higher than requested sleep level + * acpi_enable_wakeup_device_prep - Prepare wake-up devices. + * @sleep_state: ACPI system sleep state. + * + * Enable all wake-up devices' power, unless the requested system sleep state is + * too deep. */ - void acpi_enable_wakeup_device_prep(u8 sleep_state) { struct list_head *node, *next; @@ -36,9 +36,8 @@ void acpi_enable_wakeup_device_prep(u8 s struct acpi_device, wakeup_list); - if (!dev->wakeup.flags.valid || - !dev->wakeup.state.enabled || - (sleep_state > (u32) dev->wakeup.sleep_state)) + if (!dev->wakeup.flags.valid || !dev->wakeup.state.enabled + || (sleep_state > (u32) dev->wakeup.sleep_state)) continue; acpi_enable_wakeup_device_power(dev, sleep_state); @@ -46,9 +45,12 @@ void acpi_enable_wakeup_device_prep(u8 s } /** - * acpi_enable_wakeup_device - enable wakeup devices - * @sleep_state: ACPI state - * Enable all wakup devices's GPE + * acpi_enable_wakeup_device - Enable wake-up device GPEs. + * @sleep_state: ACPI system sleep state. + * + * Enable all wake-up devices' GPEs, with the assumption that + * acpi_disable_all_gpes() was executed before, so we don't need to disable any + * GPEs here. */ void acpi_enable_wakeup_device(u8 sleep_state) { @@ -65,29 +67,22 @@ void acpi_enable_wakeup_device(u8 sleep_ if (!dev->wakeup.flags.valid) continue; - /* If users want to disable run-wake GPE, - * we only disable it for wake and leave it for runtime - */ if ((!dev->wakeup.state.enabled && !dev->wakeup.prepare_count) - || sleep_state > (u32) dev->wakeup.sleep_state) { - if (dev->wakeup.flags.run_wake) { - /* set_gpe_type will disable GPE, leave it like that */ - acpi_set_gpe_type(dev->wakeup.gpe_device, - dev->wakeup.gpe_number, - ACPI_GPE_TYPE_RUNTIME); - } + || sleep_state > (u32) dev->wakeup.sleep_state) continue; - } - if (!dev->wakeup.flags.run_wake) - acpi_enable_gpe(dev->wakeup.gpe_device, - dev->wakeup.gpe_number); + + /* The wake-up power should have been enabled already. */ + acpi_set_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number, + ACPI_GPE_ENABLE); } } /** - * acpi_disable_wakeup_device - disable devices' wakeup capability - * @sleep_state: ACPI state - * Disable all wakup devices's GPE and wakeup capability + * acpi_disable_wakeup_device - Disable devices' wakeup capability. + * @sleep_state: ACPI system sleep state. + * + * This function only affects devices with wakeup.state.enabled set, which means + * that it reverses the changes made by acpi_enable_wakeup_device_prep(). */ void acpi_disable_wakeup_device(u8 sleep_state) { @@ -97,30 +92,11 @@ void acpi_disable_wakeup_device(u8 sleep struct acpi_device *dev = container_of(node, struct acpi_device, wakeup_list); - if (!dev->wakeup.flags.valid) - continue; - - if ((!dev->wakeup.state.enabled && !dev->wakeup.prepare_count) - || sleep_state > (u32) dev->wakeup.sleep_state) { - if (dev->wakeup.flags.run_wake) { - acpi_set_gpe_type(dev->wakeup.gpe_device, - dev->wakeup.gpe_number, - ACPI_GPE_TYPE_WAKE_RUN); - /* Re-enable it, since set_gpe_type will disable it */ - acpi_enable_gpe(dev->wakeup.gpe_device, - dev->wakeup.gpe_number); - } + if (!dev->wakeup.flags.valid || !dev->wakeup.state.enabled + || (sleep_state > (u32) dev->wakeup.sleep_state)) continue; - } acpi_disable_wakeup_device_power(dev); - /* Never disable run-wake GPE */ - if (!dev->wakeup.flags.run_wake) { - acpi_disable_gpe(dev->wakeup.gpe_device, - dev->wakeup.gpe_number); - acpi_clear_gpe(dev->wakeup.gpe_device, - dev->wakeup.gpe_number, ACPI_NOT_ISR); - } } } @@ -136,11 +112,8 @@ int __init acpi_wakeup_device_init(void) /* In case user doesn't load button driver */ if (!dev->wakeup.flags.run_wake || dev->wakeup.state.enabled) continue; - acpi_set_gpe_type(dev->wakeup.gpe_device, - dev->wakeup.gpe_number, - ACPI_GPE_TYPE_WAKE_RUN); - acpi_enable_gpe(dev->wakeup.gpe_device, - dev->wakeup.gpe_number); + acpi_enable_gpe(dev->wakeup.gpe_device, dev->wakeup.gpe_number, + ACPI_GPE_TYPE_WAKE); dev->wakeup.state.enabled = 1; } mutex_unlock(&acpi_device_lock); Index: linux-2.6/drivers/acpi/acpica/acevents.h =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/acevents.h +++ linux-2.6/drivers/acpi/acpica/acevents.h @@ -76,8 +76,7 @@ acpi_ev_queue_notify_request(struct acpi * evgpe - GPE handling and dispatch */ acpi_status -acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info, - u8 type); +acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info); acpi_status acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info, @@ -122,9 +121,6 @@ acpi_ev_gpe_dispatch(struct acpi_gpe_eve u32 acpi_ev_gpe_detect(struct acpi_gpe_xrupt_info *gpe_xrupt_list); acpi_status -acpi_ev_set_gpe_type(struct acpi_gpe_event_info *gpe_event_info, u8 type); - -acpi_status acpi_ev_check_for_wake_only_gpe(struct acpi_gpe_event_info *gpe_event_info); acpi_status acpi_ev_gpe_initialize(void); Index: linux-2.6/drivers/acpi/acpica/evgpe.c =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/evgpe.c +++ linux-2.6/drivers/acpi/acpica/evgpe.c @@ -54,54 +54,9 @@ static void ACPI_SYSTEM_XFACE acpi_ev_as /******************************************************************************* * - * FUNCTION: acpi_ev_set_gpe_type - * - * PARAMETERS: gpe_event_info - GPE to set - * Type - New type - * - * RETURN: Status - * - * DESCRIPTION: Sets the new type for the GPE (wake, run, or wake/run) - * - ******************************************************************************/ - -acpi_status -acpi_ev_set_gpe_type(struct acpi_gpe_event_info *gpe_event_info, u8 type) -{ - acpi_status status; - - ACPI_FUNCTION_TRACE(ev_set_gpe_type); - - /* Validate type and update register enable masks */ - - switch (type) { - case ACPI_GPE_TYPE_WAKE: - case ACPI_GPE_TYPE_RUNTIME: - case ACPI_GPE_TYPE_WAKE_RUN: - break; - - default: - return_ACPI_STATUS(AE_BAD_PARAMETER); - } - - /* Disable the GPE if currently enabled */ - - status = acpi_ev_disable_gpe(gpe_event_info); - - /* Clear the type bits and insert the new Type */ - - gpe_event_info->flags &= ~ACPI_GPE_TYPE_MASK; - gpe_event_info->flags |= type; - return_ACPI_STATUS(status); -} - -/******************************************************************************* - * * FUNCTION: acpi_ev_update_gpe_enable_masks * * PARAMETERS: gpe_event_info - GPE to update - * Type - What to do: ACPI_GPE_DISABLE or - * ACPI_GPE_ENABLE * * RETURN: Status * @@ -110,8 +65,7 @@ acpi_ev_set_gpe_type(struct acpi_gpe_eve ******************************************************************************/ acpi_status -acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info, - u8 type) +acpi_ev_update_gpe_enable_masks(struct acpi_gpe_event_info *gpe_event_info) { struct acpi_gpe_register_info *gpe_register_info; u8 register_bit; @@ -127,37 +81,14 @@ acpi_ev_update_gpe_enable_masks(struct a (1 << (gpe_event_info->gpe_number - gpe_register_info->base_gpe_number)); - /* 1) Disable case. Simply clear all enable bits */ - - if (type == ACPI_GPE_DISABLE) { - ACPI_CLEAR_BIT(gpe_register_info->enable_for_wake, - register_bit); - ACPI_CLEAR_BIT(gpe_register_info->enable_for_run, register_bit); - return_ACPI_STATUS(AE_OK); - } - - /* 2) Enable case. Set/Clear the appropriate enable bits */ + ACPI_CLEAR_BIT(gpe_register_info->enable_for_wake, register_bit); + ACPI_CLEAR_BIT(gpe_register_info->enable_for_run, register_bit); - switch (gpe_event_info->flags & ACPI_GPE_TYPE_MASK) { - case ACPI_GPE_TYPE_WAKE: - ACPI_SET_BIT(gpe_register_info->enable_for_wake, register_bit); - ACPI_CLEAR_BIT(gpe_register_info->enable_for_run, register_bit); - break; - - case ACPI_GPE_TYPE_RUNTIME: - ACPI_CLEAR_BIT(gpe_register_info->enable_for_wake, - register_bit); + if (gpe_event_info->runtime_count) ACPI_SET_BIT(gpe_register_info->enable_for_run, register_bit); - break; - case ACPI_GPE_TYPE_WAKE_RUN: + if (gpe_event_info->wakeup_count) ACPI_SET_BIT(gpe_register_info->enable_for_wake, register_bit); - ACPI_SET_BIT(gpe_register_info->enable_for_run, register_bit); - break; - - default: - return_ACPI_STATUS(AE_BAD_PARAMETER); - } return_ACPI_STATUS(AE_OK); } @@ -186,47 +117,20 @@ acpi_ev_enable_gpe(struct acpi_gpe_event /* Make sure HW enable masks are updated */ - status = - acpi_ev_update_gpe_enable_masks(gpe_event_info, ACPI_GPE_ENABLE); - if (ACPI_FAILURE(status)) { + status = acpi_ev_update_gpe_enable_masks(gpe_event_info); + if (ACPI_FAILURE(status)) return_ACPI_STATUS(status); - } /* Mark wake-enabled or HW enable, or both */ - switch (gpe_event_info->flags & ACPI_GPE_TYPE_MASK) { - case ACPI_GPE_TYPE_WAKE: - - ACPI_SET_BIT(gpe_event_info->flags, ACPI_GPE_WAKE_ENABLED); - break; - - case ACPI_GPE_TYPE_WAKE_RUN: - - ACPI_SET_BIT(gpe_event_info->flags, ACPI_GPE_WAKE_ENABLED); - - /*lint -fallthrough */ - - case ACPI_GPE_TYPE_RUNTIME: - - ACPI_SET_BIT(gpe_event_info->flags, ACPI_GPE_RUN_ENABLED); - - if (write_to_hardware) { - - /* Clear the GPE (of stale events), then enable it */ - - status = acpi_hw_clear_gpe(gpe_event_info); - if (ACPI_FAILURE(status)) { - return_ACPI_STATUS(status); - } - - /* Enable the requested runtime GPE */ - - status = acpi_hw_write_gpe_enable_reg(gpe_event_info); - } - break; + if (gpe_event_info->runtime_count && write_to_hardware) { + /* Clear the GPE (of stale events), then enable it */ + status = acpi_hw_clear_gpe(gpe_event_info); + if (ACPI_FAILURE(status)) + return_ACPI_STATUS(status); - default: - return_ACPI_STATUS(AE_BAD_PARAMETER); + /* Enable the requested runtime GPE */ + status = acpi_hw_write_gpe_enable_reg(gpe_event_info); } return_ACPI_STATUS(AE_OK); @@ -252,34 +156,9 @@ acpi_status acpi_ev_disable_gpe(struct a /* Make sure HW enable masks are updated */ - status = - acpi_ev_update_gpe_enable_masks(gpe_event_info, ACPI_GPE_DISABLE); - if (ACPI_FAILURE(status)) { + status = acpi_ev_update_gpe_enable_masks(gpe_event_info); + if (ACPI_FAILURE(status)) return_ACPI_STATUS(status); - } - - /* Clear the appropriate enabled flags for this GPE */ - - switch (gpe_event_info->flags & ACPI_GPE_TYPE_MASK) { - case ACPI_GPE_TYPE_WAKE: - ACPI_CLEAR_BIT(gpe_event_info->flags, ACPI_GPE_WAKE_ENABLED); - break; - - case ACPI_GPE_TYPE_WAKE_RUN: - ACPI_CLEAR_BIT(gpe_event_info->flags, ACPI_GPE_WAKE_ENABLED); - - /* fallthrough */ - - case ACPI_GPE_TYPE_RUNTIME: - - /* Disable the requested runtime GPE */ - - ACPI_CLEAR_BIT(gpe_event_info->flags, ACPI_GPE_RUN_ENABLED); - break; - - default: - break; - } /* * Even if we don't know the GPE type, make sure that we always Index: linux-2.6/drivers/acpi/acpica/evgpeblk.c =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/evgpeblk.c +++ linux-2.6/drivers/acpi/acpica/evgpeblk.c @@ -258,7 +258,6 @@ acpi_ev_save_method_info(acpi_handle obj u32 gpe_number; char name[ACPI_NAME_SIZE + 1]; u8 type; - acpi_status status; ACPI_FUNCTION_TRACE(ev_save_method_info); @@ -325,26 +324,20 @@ acpi_ev_save_method_info(acpi_handle obj /* * Now we can add this information to the gpe_event_info block for use - * during dispatch of this GPE. Default type is RUNTIME, although this may - * change when the _PRW methods are executed later. + * during dispatch of this GPE. */ gpe_event_info = &gpe_block->event_info[gpe_number - gpe_block->block_base_number]; - gpe_event_info->flags = (u8) - (type | ACPI_GPE_DISPATCH_METHOD | ACPI_GPE_TYPE_RUNTIME); + gpe_event_info->flags = (u8) (type | ACPI_GPE_DISPATCH_METHOD); gpe_event_info->dispatch.method_node = (struct acpi_namespace_node *)obj_handle; - /* Update enable mask, but don't enable the HW GPE as of yet */ - - status = acpi_ev_enable_gpe(gpe_event_info, FALSE); - ACPI_DEBUG_PRINT((ACPI_DB_LOAD, "Registered GPE method %s as GPE number 0x%.2X\n", name, gpe_number)); - return_ACPI_STATUS(status); + return_ACPI_STATUS(AE_OK); } /******************************************************************************* @@ -454,20 +447,7 @@ acpi_ev_match_prw_and_gpe(acpi_handle ob gpe_block-> block_base_number]; - /* Mark GPE for WAKE-ONLY but WAKE_DISABLED */ - - gpe_event_info->flags &= - ~(ACPI_GPE_WAKE_ENABLED | ACPI_GPE_RUN_ENABLED); - - status = - acpi_ev_set_gpe_type(gpe_event_info, ACPI_GPE_TYPE_WAKE); - if (ACPI_FAILURE(status)) { - goto cleanup; - } - - status = - acpi_ev_update_gpe_enable_masks(gpe_event_info, - ACPI_GPE_DISABLE); + gpe_event_info->flags |= ACPI_GPE_CAN_WAKE; } cleanup: @@ -989,7 +969,6 @@ acpi_status acpi_ev_initialize_gpe_block(struct acpi_namespace_node *gpe_device, struct acpi_gpe_block_info *gpe_block) { - acpi_status status; struct acpi_gpe_event_info *gpe_event_info; struct acpi_gpe_walk_info gpe_info; u32 wake_gpe_count; @@ -1019,42 +998,50 @@ acpi_ev_initialize_gpe_block(struct acpi gpe_info.gpe_block = gpe_block; gpe_info.gpe_device = gpe_device; - status = - acpi_ns_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, + acpi_ns_walk_namespace(ACPI_TYPE_DEVICE, ACPI_ROOT_OBJECT, ACPI_UINT32_MAX, ACPI_NS_WALK_UNLOCK, acpi_ev_match_prw_and_gpe, NULL, &gpe_info, NULL); } /* - * Enable all GPEs in this block that have these attributes: - * 1) are "runtime" or "run/wake" GPEs, and - * 2) have a corresponding _Lxx or _Exx method - * - * Any other GPEs within this block must be enabled via the - * acpi_enable_gpe() external interface. + * Enable all GPEs that have a corresponding method and aren't + * capable of generating wakeups. Any other GPEs within this block + * must be enabled via the acpi_enable_gpe() interface. */ wake_gpe_count = 0; gpe_enabled_count = 0; + if (gpe_device == acpi_gbl_fadt_gpe_device) + gpe_device = NULL; for (i = 0; i < gpe_block->register_count; i++) { - for (j = 0; j < 8; j++) { + for (j = 0; j < ACPI_GPE_REGISTER_WIDTH; j++) { + acpi_status status; + acpi_size gpe_index; + int gpe_number; /* Get the info block for this particular GPE */ + gpe_index = (acpi_size)i * ACPI_GPE_REGISTER_WIDTH + j; + gpe_event_info = &gpe_block->event_info[gpe_index]; - gpe_event_info = &gpe_block->event_info[((acpi_size) i * - ACPI_GPE_REGISTER_WIDTH) - + j]; - - if (((gpe_event_info->flags & ACPI_GPE_DISPATCH_MASK) == - ACPI_GPE_DISPATCH_METHOD) && - (gpe_event_info->flags & ACPI_GPE_TYPE_RUNTIME)) { - gpe_enabled_count++; - } - - if (gpe_event_info->flags & ACPI_GPE_TYPE_WAKE) { + if (gpe_event_info->flags & ACPI_GPE_CAN_WAKE) { wake_gpe_count++; + if (acpi_gbl_leave_wake_gpes_disabled) + continue; } + + if (!(gpe_event_info->flags & ACPI_GPE_DISPATCH_METHOD)) + continue; + + gpe_number = gpe_index + gpe_block->block_base_number; + status = acpi_enable_gpe(gpe_device, gpe_number, + ACPI_GPE_TYPE_RUNTIME); + if (ACPI_FAILURE(status)) + ACPI_ERROR((AE_INFO, + "Failed to enable GPE %02X\n", + gpe_number)); + else + gpe_enabled_count++; } } @@ -1062,15 +1049,7 @@ acpi_ev_initialize_gpe_block(struct acpi "Found %u Wake, Enabled %u Runtime GPEs in this block\n", wake_gpe_count, gpe_enabled_count)); - /* Enable all valid runtime GPEs found above */ - - status = acpi_hw_enable_runtime_gpe_block(NULL, gpe_block, NULL); - if (ACPI_FAILURE(status)) { - ACPI_ERROR((AE_INFO, "Could not enable GPEs in GpeBlock %p", - gpe_block)); - } - - return_ACPI_STATUS(status); + return_ACPI_STATUS(AE_OK); } /******************************************************************************* Index: linux-2.6/drivers/acpi/acpica/evxface.c =================================================================== --- linux-2.6.orig/drivers/acpi/acpica/evxface.c +++ linux-2.6/drivers/acpi/acpica/evxface.c @@ -617,13 +617,6 @@ acpi_install_gpe_handler(acpi_handle gpe handler->context = context; handler->method_node = gpe_event_info->dispatch.method_node; - /* Disable the GPE before installing the handler */ - - status = acpi_ev_disable_gpe(gpe_event_info); - if (ACPI_FAILURE(status)) { - goto unlock_and_exit; - } - /* Install the handler */ flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock); @@ -707,13 +700,6 @@ acpi_remove_gpe_handler(acpi_handle gpe_ goto unlock_and_exit; } - /* Disable the GPE before removing the handler */ - - status = acpi_ev_disable_gpe(gpe_event_info); - if (ACPI_FAILURE(status)) { - goto unlock_and_exit; - } - /* Make sure all deferred tasks are completed */ (void)acpi_ut_release_mutex(ACPI_MTX_EVENTS); Index: linux-2.6/include/acpi/actypes.h =================================================================== --- linux-2.6.orig/include/acpi/actypes.h +++ linux-2.6/include/acpi/actypes.h @@ -668,15 +668,16 @@ typedef u32 acpi_event_status; /* * GPE info flags - Per GPE - * +-+-+-+---+---+-+ - * |7|6|5|4:3|2:1|0| - * +-+-+-+---+---+-+ - * | | | | | | - * | | | | | +--- Interrupt type: Edge or Level Triggered - * | | | | +--- Type: Wake-only, Runtime-only, or wake/runtime + * +-+-+-+---+-+-+-+ + * |7|6|5|4:3|2|1|0| + * +-+-+-+---+-+-+-+ + * | | | | | | | + * | | | | | | +--- Interrupt type: Edge or Level Triggered + * | | | | | +--- GPE can wake the system + * | | | | +--- Unused * | | | +--- Type of dispatch -- to method, handler, or none - * | | +--- Enabled for runtime? - * | +--- Enabled for wake? + * | | +--- Unused + * | +--- Unused * +--- Unused */ #define ACPI_GPE_XRUPT_TYPE_MASK (u8) 0x01 @@ -687,22 +688,13 @@ typedef u32 acpi_event_status; #define ACPI_GPE_TYPE_WAKE_RUN (u8) 0x06 #define ACPI_GPE_TYPE_WAKE (u8) 0x02 #define ACPI_GPE_TYPE_RUNTIME (u8) 0x04 /* Default */ +#define ACPI_GPE_CAN_WAKE (u8) 0x02 #define ACPI_GPE_DISPATCH_MASK (u8) 0x18 #define ACPI_GPE_DISPATCH_HANDLER (u8) 0x08 #define ACPI_GPE_DISPATCH_METHOD (u8) 0x10 #define ACPI_GPE_DISPATCH_NOT_USED (u8) 0x00 /* Default */ -#define ACPI_GPE_RUN_ENABLE_MASK (u8) 0x20 -#define ACPI_GPE_RUN_ENABLED (u8) 0x20 -#define ACPI_GPE_RUN_DISABLED (u8) 0x00 /* Default */ - -#define ACPI_GPE_WAKE_ENABLE_MASK (u8) 0x40 -#define ACPI_GPE_WAKE_ENABLED (u8) 0x40 -#define ACPI_GPE_WAKE_DISABLED (u8) 0x00 /* Default */ - -#define ACPI_GPE_ENABLE_MASK (u8) 0x60 /* Both run/wake */ - /* * Flags for GPE and Lock interfaces */ Index: linux-2.6/drivers/acpi/system.c =================================================================== --- linux-2.6.orig/drivers/acpi/system.c +++ linux-2.6/drivers/acpi/system.c @@ -387,10 +387,10 @@ static ssize_t counter_set(struct kobjec if (index < num_gpes) { if (!strcmp(buf, "disable\n") && (status & ACPI_EVENT_FLAG_ENABLED)) - result = acpi_disable_gpe(handle, index); + result = acpi_set_gpe(handle, index, ACPI_GPE_DISABLE); else if (!strcmp(buf, "enable\n") && !(status & ACPI_EVENT_FLAG_ENABLED)) - result = acpi_enable_gpe(handle, index); + result = acpi_set_gpe(handle, index, ACPI_GPE_ENABLE); else if (!strcmp(buf, "clear\n") && (status & ACPI_EVENT_FLAG_SET)) result = acpi_clear_gpe(handle, index, ACPI_NOT_ISR); ^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2010-02-14 2:23 UTC | newest] Thread overview: 20+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-01-29 20:30 Recent GPE patches - some questions Moore, Robert 2010-02-01 22:36 ` Matthew Garrett 2010-02-02 23:02 ` Moore, Robert 2010-02-06 23:31 ` Rafael J. Wysocki 2010-02-07 2:17 ` [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) Rafael J. Wysocki 2010-02-07 2:22 ` [RFC][PATCH 1/3] ACPI: Add infrastructure for refcounting GPE consumers Rafael J. Wysocki 2010-02-07 2:23 ` [RFC][PATCH 2/3] ACPI: Modify GPE consumers to use GPE refcounting Rafael J. Wysocki 2010-02-07 2:24 ` [RFC][PATCH 3/3] ACPI: Remove old GPE API and transition code entirely to new one Rafael J. Wysocki 2010-02-07 11:56 ` [Update] Re: [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) Rafael J. Wysocki 2010-02-07 11:58 ` [Update][RFC][PATCH 1/3] ACPI: Add infrastructure for refcounting GPE consumers Rafael J. Wysocki 2010-02-07 11:58 ` [Update][RFC][PATCH 2/3] ACPI: Modify GPE consumers to use GPE refcounting Rafael J. Wysocki 2010-02-07 11:59 ` [Update][RFC][PATCH 3/3] ACPI: Remove old GPE API and transition code entirely to new one Rafael J. Wysocki 2010-02-10 21:29 ` [RFC][PATCH 0/3] Introduce GPE refcounting (was: Re: Recent GPE patches - some questions.) Maxim Levitsky 2010-02-10 21:36 ` Rafael J. Wysocki 2010-02-11 17:36 ` Maxim Levitsky 2010-02-11 20:34 ` Rafael J. Wysocki 2010-02-13 16:08 ` Maxim Levitsky 2010-02-14 2:24 ` Rafael J. Wysocki 2010-02-10 21:36 ` Recent GPE patches - some questions Moore, Robert 2010-02-11 22:51 ` [PATCH] ACPI: Use GPE reference counting to support shared GPEs (was: Re: Recent GPE patches - some questions.) Rafael J. Wysocki
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox