From patchwork Wed Jun 7 09:43:39 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Lv Zheng X-Patchwork-Id: 9771121 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 09F6960350 for ; Wed, 7 Jun 2017 09:44:07 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E5C402624B for ; Wed, 7 Jun 2017 09:44:06 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id DA62328471; Wed, 7 Jun 2017 09:44:06 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 58EFB2624B for ; Wed, 7 Jun 2017 09:44:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751723AbdFGJoF (ORCPT ); Wed, 7 Jun 2017 05:44:05 -0400 Received: from mga05.intel.com ([192.55.52.43]:24956 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751718AbdFGJoE (ORCPT ); Wed, 7 Jun 2017 05:44:04 -0400 Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga105.fm.intel.com with ESMTP; 07 Jun 2017 02:43:43 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.39,310,1493708400"; d="scan'208";a="1138831930" Received: from lvzheng-moblsp3.sh.intel.com ([10.239.159.38]) by orsmga001.jf.intel.com with ESMTP; 07 Jun 2017 02:43:39 -0700 From: Lv Zheng To: "Rafael J . Wysocki" , "Rafael J . Wysocki" , Len Brown Cc: Lv Zheng , Lv Zheng , linux-kernel@vger.kernel.org, linux-acpi@vger.kernel.org, systemd-devel@lists.freedesktop.org, Benjamin Tissoires , Peter Hutterer Subject: [RFC PATCH v5 1/2] ACPI: button: Fix issue that button notify cannot report stateful SW_LID state Date: Wed, 7 Jun 2017 17:43:39 +0800 Message-Id: <58f7af6e2945603ca4ff6647eeb5ff6fee753a57.1496828498.git.lv.zheng@intel.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: References: <2a779ae8c280c968b3237ac4a3d9580df7262a46.1493951798.git.lv.zheng@intel.com> Sender: linux-acpi-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-acpi@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP There are platform variations implementing ACPI lid device in different ways: 1. Some platforms send "open" events to OS and the events arrive before button driver is resumed; 2. Some platforms send "open" events to OS, but the events arrive after button driver is resumed, ex., Samsung N210+; 3. Some platforms never send "open" events to OS, but send "open" events to update the cached _LID return value, and the update events arrive before button driver is resumed; 4. Some platforms never send "open" events to OS, but send "open" events to update the cached _LID return value, but the update events arrive after button driver is resumed, ex., Surface Pro 3; 5. Some platforms never send "open" events, _LID returns value sticks to "close", ex., Surface Pro 1. Currently, only case 1,3 works fine with systemd 229. Case 2,4 can be treated as an order issue. This patch first fixes this issue by defer sending initial lid state 10 seconds later after resume, which ensures acpi_ec_resume() is always invoked before acpi_button_resume(). However we can see different problems due to systemd bugs: systemd won't suspend right after seeing "close" event, it has a timeout, within the timeout, user may opens lid again. But even lid firmware/driver properly deliver this "open" to user space, when the timeout tickes, systemd still suspends the platform. Then user has to close/open again to wake the system up. Noticing that the first close event will remain in firmware, after resume, user space can still see a "close" followed by "open", and nothing can stop systemd from suspending again. This problem can only be fixed by continously updating lid state. Thus this patch doesn't kill the timer after seeing the BIOS notification, but continously sending _LID return value to the input layer for button.lid_init_state=method mode. The users can configure update interval via button.lid_update_interval. Cc: Cc: Benjamin Tissoires Cc: Peter Hutterer Signed-off-by: Lv Zheng --- drivers/acpi/button.c | 36 +++++++++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 7 deletions(-) diff --git a/drivers/acpi/button.c b/drivers/acpi/button.c index e19f530..fd8eff6 100644 --- a/drivers/acpi/button.c +++ b/drivers/acpi/button.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include #include @@ -79,6 +80,7 @@ MODULE_DEVICE_TABLE(acpi, button_device_ids); static int acpi_button_add(struct acpi_device *device); static int acpi_button_remove(struct acpi_device *device); static void acpi_button_notify(struct acpi_device *device, u32 event); +static void acpi_lid_timeout(ulong arg); #ifdef CONFIG_PM_SLEEP static int acpi_button_suspend(struct device *dev); @@ -104,6 +106,7 @@ static struct acpi_driver acpi_button_driver = { struct acpi_button { unsigned int type; struct input_dev *input; + struct timer_list lid_timer; char phys[32]; /* for input device */ unsigned long pushed; int last_state; @@ -119,6 +122,10 @@ static unsigned long lid_report_interval __read_mostly = 500; module_param(lid_report_interval, ulong, 0644); MODULE_PARM_DESC(lid_report_interval, "Interval (ms) between lid key events"); +static unsigned long lid_update_interval __read_mostly = 10 * MSEC_PER_SEC; +module_param(lid_update_interval, ulong, 0644); +MODULE_PARM_DESC(lid_update_interval, "Interval (ms) between lid state updates"); + /* -------------------------------------------------------------------------- FS Interface (/proc) -------------------------------------------------------------------------- */ @@ -371,17 +378,25 @@ static int acpi_lid_update_state(struct acpi_device *device) return acpi_lid_notify_state(device, state); } -static void acpi_lid_initialize_state(struct acpi_device *device) +static void acpi_lid_tick(struct acpi_device *device) +{ + struct acpi_button *button = acpi_driver_data(device); + + mod_timer(&button->lid_timer, + jiffies + msecs_to_jiffies(lid_update_interval)); +} + +static void acpi_lid_timeout(ulong arg) { + struct acpi_device *device = (struct acpi_device *)arg; + switch (lid_init_state) { case ACPI_BUTTON_LID_INIT_OPEN: (void)acpi_lid_notify_state(device, 1); break; case ACPI_BUTTON_LID_INIT_METHOD: - (void)acpi_lid_update_state(device); - break; - case ACPI_BUTTON_LID_INIT_IGNORE: - default: + acpi_lid_update_state(device); + acpi_lid_tick(device); break; } } @@ -432,6 +447,8 @@ static int acpi_button_suspend(struct device *dev) struct acpi_device *device = to_acpi_device(dev); struct acpi_button *button = acpi_driver_data(device); + if (button->type == ACPI_BUTTON_TYPE_LID) + del_timer(&button->lid_timer); button->suspended = true; return 0; } @@ -443,7 +460,7 @@ static int acpi_button_resume(struct device *dev) button->suspended = false; if (button->type == ACPI_BUTTON_TYPE_LID) - acpi_lid_initialize_state(device); + acpi_lid_tick(device); return 0; } #endif @@ -467,6 +484,7 @@ static int acpi_button_add(struct acpi_device *device) error = -ENOMEM; goto err_free_button; } + init_timer(&button->lid_timer); name = acpi_device_name(device); class = acpi_device_class(device); @@ -490,6 +508,8 @@ static int acpi_button_add(struct acpi_device *device) ACPI_BUTTON_CLASS, ACPI_BUTTON_SUBCLASS_LID); button->last_state = !!acpi_lid_evaluate_state(device); button->last_time = ktime_get(); + setup_timer(&button->lid_timer, + acpi_lid_timeout, (ulong)device); } else { printk(KERN_ERR PREFIX "Unsupported hid [%s]\n", hid); error = -ENODEV; @@ -526,7 +546,7 @@ static int acpi_button_add(struct acpi_device *device) if (error) goto err_remove_fs; if (button->type == ACPI_BUTTON_TYPE_LID) { - acpi_lid_initialize_state(device); + acpi_lid_tick(device); /* * This assumes there's only one lid device, or if there are * more we only care about the last one... @@ -550,6 +570,8 @@ static int acpi_button_remove(struct acpi_device *device) { struct acpi_button *button = acpi_driver_data(device); + if (button->type == ACPI_BUTTON_TYPE_LID) + del_timer(&button->lid_timer); acpi_button_remove_fs(device); input_unregister_device(button->input); kfree(button);