From patchwork Tue Aug 3 21:55:14 2010 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Rafael Wysocki X-Patchwork-Id: 116884 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by demeter.kernel.org (8.14.4/8.14.3) with ESMTP id o73Lv0aD021531 for ; Tue, 3 Aug 2010 21:57:31 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757823Ab0HCV5a (ORCPT ); Tue, 3 Aug 2010 17:57:30 -0400 Received: from ogre.sisk.pl ([217.79.144.158]:33948 "EHLO ogre.sisk.pl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757820Ab0HCV5a (ORCPT ); Tue, 3 Aug 2010 17:57:30 -0400 Received: from localhost (localhost.localdomain [127.0.0.1]) by ogre.sisk.pl (Postfix) with ESMTP id 5A15B18F3EB; Tue, 3 Aug 2010 23:50:34 +0200 (CEST) Received: from ogre.sisk.pl ([127.0.0.1]) by localhost (ogre.sisk.pl [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 10169-08; Tue, 3 Aug 2010 23:50:27 +0200 (CEST) Received: from ferrari.localnet (unknown [82.210.184.220]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by ogre.sisk.pl (Postfix) with ESMTP id D62B51874C6; Tue, 3 Aug 2010 23:50:26 +0200 (CEST) From: "Rafael J. Wysocki" To: Len Brown Subject: [Resend][PATCH] ACPI / ACPICA: Fix reference counting problems with GPE handlers Date: Tue, 3 Aug 2010 23:55:14 +0200 User-Agent: KMail/1.13.5 (Linux/2.6.35-rjw+; KDE/4.4.4; x86_64; ; ) Cc: ACPI Devel Maling List , "Moore, Robert" , Lin Ming MIME-Version: 1.0 Message-Id: <201008032355.14414.rjw@sisk.pl> X-Virus-Scanned: amavisd-new at ogre.sisk.pl using MkS_Vir for Linux Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org X-Greylist: IP, sender and recipient auto-whitelisted, not delayed by milter-greylist-4.2.3 (demeter.kernel.org [140.211.167.41]); Tue, 03 Aug 2010 21:57:31 +0000 (UTC) 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 @@ -691,12 +691,22 @@ acpi_install_gpe_handler(acpi_handle gpe return_ACPI_STATUS(status); } + /* Allocate memory for the handler object */ + + handler = ACPI_ALLOCATE_ZEROED(sizeof(struct acpi_handler_info)); + if (!handler) { + status = AE_NO_MEMORY; + goto unlock_and_exit; + } + + 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; + goto free_and_exit; } /* Make sure that there isn't a handler there already */ @@ -704,24 +714,30 @@ acpi_install_gpe_handler(acpi_handle gpe if ((gpe_event_info->flags & ACPI_GPE_DISPATCH_MASK) == ACPI_GPE_DISPATCH_HANDLER) { status = AE_ALREADY_EXISTS; - goto unlock_and_exit; + goto free_and_exit; } /* Allocate and init handler object */ - handler = ACPI_ALLOCATE_ZEROED(sizeof(struct acpi_handler_info)); - if (!handler) { - status = AE_NO_MEMORY; - goto unlock_and_exit; - } - handler->address = address; handler->context = context; handler->method_node = gpe_event_info->dispatch.method_node; + handler->orig_flags = gpe_event_info->flags & + (ACPI_GPE_XRUPT_TYPE_MASK | ACPI_GPE_DISPATCH_MASK); + + /* + * If the GPE is associated with a method and it cannot wake up the + * system from sleep states, it was enabled automatically during + * initialization, so it has to be disabled now to avoid spurious + * execution of the handler. + */ + + if ((handler->orig_flags & ACPI_GPE_DISPATCH_METHOD) + && !(gpe_event_info->flags & ACPI_GPE_CAN_WAKE)) + (void)acpi_raw_disable_gpe(gpe_event_info); /* Install the handler */ - flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock); gpe_event_info->dispatch.handler = handler; /* Setup up dispatch flags to indicate handler (vs. method) */ @@ -735,6 +751,11 @@ acpi_install_gpe_handler(acpi_handle gpe unlock_and_exit: (void)acpi_ut_release_mutex(ACPI_MTX_EVENTS); return_ACPI_STATUS(status); + +free_and_exit: + acpi_os_release_lock(acpi_gbl_gpe_lock, flags); + ACPI_FREE(handler); + goto unlock_and_exit; } ACPI_EXPORT_SYMBOL(acpi_install_gpe_handler) @@ -770,11 +791,17 @@ acpi_remove_gpe_handler(acpi_handle gpe_ return_ACPI_STATUS(AE_BAD_PARAMETER); } + /* Make sure all deferred tasks are completed */ + + acpi_os_wait_events_complete(NULL); + status = acpi_ut_acquire_mutex(ACPI_MTX_EVENTS); if (ACPI_FAILURE(status)) { return_ACPI_STATUS(status); } + 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); @@ -798,34 +825,34 @@ acpi_remove_gpe_handler(acpi_handle gpe_ goto unlock_and_exit; } - /* Make sure all deferred tasks are completed */ - - (void)acpi_ut_release_mutex(ACPI_MTX_EVENTS); - acpi_os_wait_events_complete(NULL); - status = acpi_ut_acquire_mutex(ACPI_MTX_EVENTS); - if (ACPI_FAILURE(status)) { - return_ACPI_STATUS(status); - } - /* Remove the handler */ - flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock); handler = gpe_event_info->dispatch.handler; /* Restore Method node (if any), set dispatch flags */ gpe_event_info->dispatch.method_node = handler->method_node; - gpe_event_info->flags &= ~ACPI_GPE_DISPATCH_MASK; /* Clear bits */ - if (handler->method_node) { - gpe_event_info->flags |= ACPI_GPE_DISPATCH_METHOD; - } - acpi_os_release_lock(acpi_gbl_gpe_lock, flags); + gpe_event_info->flags &= + ~(ACPI_GPE_XRUPT_TYPE_MASK | ACPI_GPE_DISPATCH_MASK); + gpe_event_info->flags |= handler->orig_flags; + + /* + * If the GPE was previously associated with a method and it cannot wake + * up the system from sleep states, it should be enabled at this point + * to restore the post-initialization configuration. + */ + + if ((handler->orig_flags & ACPI_GPE_DISPATCH_METHOD) + && !(gpe_event_info->flags & ACPI_GPE_CAN_WAKE)) + (void)acpi_raw_enable_gpe(gpe_event_info); /* Now we can free the handler object */ ACPI_FREE(handler); - unlock_and_exit: +unlock_and_exit: + acpi_os_release_lock(acpi_gbl_gpe_lock, flags); + (void)acpi_ut_release_mutex(ACPI_MTX_EVENTS); return_ACPI_STATUS(status); } 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 @@ -412,6 +412,7 @@ struct acpi_handler_info { acpi_event_handler address; /* Address of handler, if any */ void *context; /* Context to be passed to handler */ struct acpi_namespace_node *method_node; /* Method node for this GPE level (saved) */ + u8 orig_flags; /* Original misc info about this GPE */ }; union acpi_gpe_dispatch_info { 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 @@ -137,6 +137,79 @@ acpi_ev_enable_gpe(struct acpi_gpe_event /******************************************************************************* * + * FUNCTION: acpi_raw_enable_gpe + * + * PARAMETERS: gpe_event_info - GPE to enable + * + * RETURN: Status + * + * DESCRIPTION: Add a reference to a GPE. On the first reference, the GPE is + * hardware-enabled. + * + ******************************************************************************/ + +acpi_status acpi_raw_enable_gpe(struct acpi_gpe_event_info *gpe_event_info) +{ + acpi_status status = AE_OK; + + if (gpe_event_info->runtime_count == ACPI_UINT8_MAX) { + return_ACPI_STATUS(AE_LIMIT); + } + + gpe_event_info->runtime_count++; + if (gpe_event_info->runtime_count == 1) { + status = acpi_ev_update_gpe_enable_mask(gpe_event_info); + if (ACPI_SUCCESS(status)) { + status = acpi_ev_enable_gpe(gpe_event_info); + } + + if (ACPI_FAILURE(status)) { + gpe_event_info->runtime_count--; + } + } + + return_ACPI_STATUS(status); +} + +/******************************************************************************* + * + * FUNCTION: acpi_raw_disable_gpe + * + * PARAMETERS: gpe_event_info - GPE to disable + * + * RETURN: Status + * + * DESCRIPTION: Remove a reference to a GPE. When the last reference is + * removed, the GPE is hardware-disabled. + * + ******************************************************************************/ + +acpi_status acpi_raw_disable_gpe(struct acpi_gpe_event_info *gpe_event_info) +{ + acpi_status status = AE_OK; + + if (!gpe_event_info->runtime_count) { + return_ACPI_STATUS(AE_LIMIT); + } + + gpe_event_info->runtime_count--; + if (!gpe_event_info->runtime_count) { + status = acpi_ev_update_gpe_enable_mask(gpe_event_info); + if (ACPI_SUCCESS(status)) { + status = acpi_hw_low_set_gpe(gpe_event_info, + ACPI_GPE_DISABLE); + } + + if (ACPI_FAILURE(status)) { + gpe_event_info->runtime_count++; + } + } + + return_ACPI_STATUS(status); +} + +/******************************************************************************* + * * FUNCTION: acpi_ev_low_get_gpe_info * * PARAMETERS: gpe_number - Raw GPE number 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 @@ -82,6 +82,10 @@ acpi_ev_update_gpe_enable_mask(struct ac acpi_status acpi_ev_enable_gpe(struct acpi_gpe_event_info *gpe_event_info); +acpi_status acpi_raw_enable_gpe(struct acpi_gpe_event_info *gpe_event_info); + +acpi_status acpi_raw_disable_gpe(struct acpi_gpe_event_info *gpe_event_info); + struct acpi_gpe_event_info *acpi_ev_get_gpe_event_info(acpi_handle gpe_device, u32 gpe_number); 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 @@ -294,7 +294,7 @@ ACPI_EXPORT_SYMBOL(acpi_gpe_wakeup) ******************************************************************************/ acpi_status acpi_enable_gpe(acpi_handle gpe_device, u32 gpe_number) { - acpi_status status = AE_OK; + acpi_status status = AE_BAD_PARAMETER; struct acpi_gpe_event_info *gpe_event_info; acpi_cpu_flags flags; @@ -305,28 +305,10 @@ acpi_status acpi_enable_gpe(acpi_handle /* 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) { + status = acpi_raw_enable_gpe(gpe_event_info); } - if (gpe_event_info->runtime_count == ACPI_UINT8_MAX) { - status = AE_LIMIT; /* Too many references */ - goto unlock_and_exit; - } - - gpe_event_info->runtime_count++; - if (gpe_event_info->runtime_count == 1) { - status = acpi_ev_update_gpe_enable_mask(gpe_event_info); - if (ACPI_SUCCESS(status)) { - status = acpi_ev_enable_gpe(gpe_event_info); - } - if (ACPI_FAILURE(status)) { - gpe_event_info->runtime_count--; - } - } - -unlock_and_exit: acpi_os_release_lock(acpi_gbl_gpe_lock, flags); return_ACPI_STATUS(status); } @@ -348,7 +330,7 @@ ACPI_EXPORT_SYMBOL(acpi_enable_gpe) ******************************************************************************/ acpi_status acpi_disable_gpe(acpi_handle gpe_device, u32 gpe_number) { - acpi_status status = AE_OK; + acpi_status status = AE_BAD_PARAMETER; struct acpi_gpe_event_info *gpe_event_info; acpi_cpu_flags flags; @@ -359,32 +341,10 @@ acpi_status acpi_disable_gpe(acpi_handle /* 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; - } - - /* Hardware-disable a runtime GPE on removal of the last reference */ - - if (!gpe_event_info->runtime_count) { - status = AE_LIMIT; /* There are no references to remove */ - goto unlock_and_exit; + if (gpe_event_info) { + status = acpi_raw_disable_gpe(gpe_event_info) ; } - gpe_event_info->runtime_count--; - if (!gpe_event_info->runtime_count) { - status = acpi_ev_update_gpe_enable_mask(gpe_event_info); - if (ACPI_SUCCESS(status)) { - status = - acpi_hw_low_set_gpe(gpe_event_info, - ACPI_GPE_DISABLE); - } - if (ACPI_FAILURE(status)) { - gpe_event_info->runtime_count++; - } - } - -unlock_and_exit: acpi_os_release_lock(acpi_gbl_gpe_lock, flags); return_ACPI_STATUS(status); } @@ -411,7 +371,6 @@ acpi_status acpi_gpe_can_wake(acpi_handl acpi_status status = AE_OK; struct acpi_gpe_event_info *gpe_event_info; acpi_cpu_flags flags; - u8 disable = 0; ACPI_FUNCTION_TRACE(acpi_gpe_can_wake); @@ -430,15 +389,12 @@ acpi_status acpi_gpe_can_wake(acpi_handl } gpe_event_info->flags |= ACPI_GPE_CAN_WAKE; - disable = (gpe_event_info->flags & ACPI_GPE_DISPATCH_METHOD) - && gpe_event_info->runtime_count; + if (gpe_event_info->flags & ACPI_GPE_DISPATCH_METHOD) { + (void)acpi_raw_disable_gpe(gpe_event_info); + } unlock_and_exit: acpi_os_release_lock(acpi_gbl_gpe_lock, flags); - - if (disable) - status = acpi_disable_gpe(gpe_device, gpe_number); - return_ACPI_STATUS(status); } ACPI_EXPORT_SYMBOL(acpi_gpe_can_wake)