From patchwork Tue Apr 29 14:51:20 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kieran Clancy X-Patchwork-Id: 4088011 Return-Path: X-Original-To: patchwork-linux-acpi@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 1BC309F39D for ; Tue, 29 Apr 2014 14:52:42 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 2F2AC20138 for ; Tue, 29 Apr 2014 14:52:40 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 2C6DF2018B for ; Tue, 29 Apr 2014 14:52:39 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757875AbaD2OwQ (ORCPT ); Tue, 29 Apr 2014 10:52:16 -0400 Received: from mail-pd0-f169.google.com ([209.85.192.169]:49510 "EHLO mail-pd0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751330AbaD2OwO (ORCPT ); Tue, 29 Apr 2014 10:52:14 -0400 Received: by mail-pd0-f169.google.com with SMTP id y13so277789pdi.28 for ; Tue, 29 Apr 2014 07:52:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:to:cc:subject:date:message-id; bh=9Z8kQnjKBFeYofOZy3RZPdZ7SWetxpgk5ealawMRt7M=; b=XIkmUc9/ekdvxqEbuvAtCNj9WiKWU+U1iH7r8xOqJ4znfy0Y98w0iK5KowoLU3R+q+ B972/c9cNcnzrr2oeVyHvrgIguBMy8OluCrVAnUjeo/VP8p70f8nuiUcc8Xqm/Kd8u4o Wy6wkuLhNVFXP1bbU6z9HNxVbO+box0a1ZPBYQYiWEFiuKeWetJlLDUT90NuFOcZbe/Y hdd2bXhDlx/XoHjnqBdUrAvHlqCL9YDJilmnRGMHi6Fk7cdwuYHb5Nrth3lPwJYCm7by KkyUNtLOz4GtV7AK0iSB0wD9sVxlWRzojU02bn5i6EUocE6MPWwkyKg091hTnE/ZkD08 Ku4g== X-Received: by 10.66.141.165 with SMTP id rp5mr34417275pab.90.1398783128343; Tue, 29 Apr 2014 07:52:08 -0700 (PDT) Received: from kbook.kieranclancy.com. ([60.240.75.5]) by mx.google.com with ESMTPSA id f5sm112335274pat.11.2014.04.29.07.52.04 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 29 Apr 2014 07:52:07 -0700 (PDT) From: Kieran Clancy To: Len Brown , "Rafael J. Wysocki" Cc: linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org, Kieran Clancy , Lan Tianyu , Stefan Biereigel Subject: [PATCH] ACPI / EC: Process rather than discard events in acpi_ec_clear Date: Wed, 30 Apr 2014 00:21:20 +0930 Message-Id: <1398783080-24554-1-git-send-email-clancy.kieran@gmail.com> X-Mailer: git-send-email 1.9.0 Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, RP_MATCHES_RCVD, T_DKIM_INVALID, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Address a regression caused by commit ad332c8a4533: (ACPI / EC: Clear stale EC events on Samsung systems) After the earlier patch, there was found to be a race condition on some earlier Samsung systems (N150/N210/N220). The function acpi_ec_clear was sometimes discarding a new EC event before its GPE was triggered by the system. In the case of these systems, this meant that the "lid open" event was not registered on resume if that was the cause of the wake, leading to problems when attempting to close the lid to suspend again. After testing on a number of Samsung systems, both those affected by the previous EC bug and those affected by the race condition, it seemed that the best course of action was to process rather than discard the events. On Samsung systems which accumulate stale EC events, there does not seem to be any adverse side-effects of running the associated _Q methods. This patch adds an argument to the static function acpi_ec_sync_query so that it may be used within the acpi_ec_clear loop in place of acpi_ec_query_unlocked which was used previously. With thanks to Stefan Biereigel for reporting the issue, and for all the people who helped test the new patch on affected systems. References: https://lkml.kernel.org/r/532FE3B2.9060808@biereigel-wb.de References: https://bugzilla.kernel.org/show_bug.cgi?id=44161#c173 Reported-by: Stefan Biereigel Signed-off-by: Kieran Clancy Tested-by: Stefan Biereigel Tested-by: Dennis Jansen Tested-by: Nicolas Porcel Tested-by: Maurizio D'Addona Tested-by: Juan Manuel Cabo Tested-by: Giannis Koutsou Tested-by: Kieran Clancy Cc: Lan Tianyu --- To maintainers: Assuming this patch is accepted, please mark this for inclusion in all -stable trees. It should be noted that the previous patch (ad332c8a4533) was excluded from a number of stable trees after the regression was found, but should now be included again along with this patch. I am not sure of the correct way to annotate this above. drivers/acpi/ec.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index d7d32c2..ad11ba4 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -206,13 +206,13 @@ unlock: spin_unlock_irqrestore(&ec->lock, flags); } -static int acpi_ec_sync_query(struct acpi_ec *ec); +static int acpi_ec_sync_query(struct acpi_ec *ec, u8 *data); static int ec_check_sci_sync(struct acpi_ec *ec, u8 state) { if (state & ACPI_EC_FLAG_SCI) { if (!test_and_set_bit(EC_FLAGS_QUERY_PENDING, &ec->flags)) - return acpi_ec_sync_query(ec); + return acpi_ec_sync_query(ec, NULL); } return 0; } @@ -443,10 +443,8 @@ acpi_handle ec_get_handle(void) EXPORT_SYMBOL(ec_get_handle); -static int acpi_ec_query_unlocked(struct acpi_ec *ec, u8 *data); - /* - * Clears stale _Q events that might have accumulated in the EC. + * Process _Q events that might have accumulated in the EC. * Run with locked ec mutex. */ static void acpi_ec_clear(struct acpi_ec *ec) @@ -455,7 +453,7 @@ static void acpi_ec_clear(struct acpi_ec *ec) u8 value = 0; for (i = 0; i < ACPI_EC_CLEAR_MAX; i++) { - status = acpi_ec_query_unlocked(ec, &value); + status = acpi_ec_sync_query(ec, &value); if (status || !value) break; } @@ -582,13 +580,18 @@ static void acpi_ec_run(void *cxt) kfree(handler); } -static int acpi_ec_sync_query(struct acpi_ec *ec) +static int acpi_ec_sync_query(struct acpi_ec *ec, u8 *data) { u8 value = 0; int status; struct acpi_ec_query_handler *handler, *copy; - if ((status = acpi_ec_query_unlocked(ec, &value))) + + status = acpi_ec_query_unlocked(ec, &value); + if (data) + *data = value; + if (status) return status; + list_for_each_entry(handler, &ec->list, node) { if (value == handler->query_bit) { /* have custom handler for this bit */ @@ -612,7 +615,7 @@ static void acpi_ec_gpe_query(void *ec_cxt) if (!ec) return; mutex_lock(&ec->mutex); - acpi_ec_sync_query(ec); + acpi_ec_sync_query(ec, NULL); mutex_unlock(&ec->mutex); }