From patchwork Thu May 26 13:31:31 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Benjamin Tissoires X-Patchwork-Id: 9136981 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 E3AA0607D5 for ; Thu, 26 May 2016 13:31:38 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D5305282F6 for ; Thu, 26 May 2016 13:31:38 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C9701282F8; Thu, 26 May 2016 13:31:38 +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.8 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, T_DKIM_INVALID 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 1EBE0282F6 for ; Thu, 26 May 2016 13:31:37 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753643AbcEZNbg (ORCPT ); Thu, 26 May 2016 09:31:36 -0400 Received: from mail-wm0-f65.google.com ([74.125.82.65]:35748 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753499AbcEZNbf (ORCPT ); Thu, 26 May 2016 09:31:35 -0400 Received: by mail-wm0-f65.google.com with SMTP id e3so5482861wme.2; Thu, 26 May 2016 06:31:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=from:subject:to:references:cc:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=Ryz4rlxwxzeDRq/0BRphPTIlg605TWZOdvTESNlHKcI=; b=e/RssqHHSb/zL+Rmy4asCtQE7NXT9yJKyeriWCVw7zh7nDGVOvJEs3X5mc/kWJUojl Flosmr2yihXVsKUA33XRElg/G07EikBKsM8g+Kukv8o1Yp4WViAZ3204fSm9FgeSl9AT TfOMLWT5p/IXE0lIIBUNym5XE7llu3tmqcIu3Yui1FvBa5cW2Z+2cOLg74zt5CRh0bYc suaF9oi0e3jz0z8oo5+EfQy8kGnJ+LMqp2xdYtqIdZE4Viv5wRqWGQXPTuwGvnV5LZQq YA8sAvgfEmf9rUL2HWI3slGPNz7PYLjbo2CbI95HocnZ3QxGvVEcZF2THTJtoCPCeqk/ PGhg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:subject:to:references:cc:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=Ryz4rlxwxzeDRq/0BRphPTIlg605TWZOdvTESNlHKcI=; b=E6Bp3Bw4RthL6mGAaYpLlVMcv9xkrIwH4bBGW4d4jL3LWnpkWAFjyBqOls7727keqz NN0JE1EqByhSezzwPERwKRulrWTIAsj13Vgu3VRx5Vqokze15sVkGfCkEQ28aFYlVdNd TO9tObgMgorCVaQLnYvYjVf33yOnCHzcadF0/MUW0QVtQE+uPuiISXjJFUz2clz92Xtr ddC86VIJTI1s2HwjPGX69Ag/ZAJ8R546CDWG2GIkRH78S6lr/VKBLHIUMc+NFPvAWIo2 KchzLiRkR8AV1qsPU/lIfgq6MaEbkdiudwP04IztXEeMRQ2LQSzrWMcLW0sGskbhw9c8 EqIQ== X-Gm-Message-State: ALyK8tIsqe7MfwuT5NYSNM9kERDqXgCuBjM9ROg3fkCo8up8a4CWqBywAQitBVJ/4lcHxA== X-Received: by 10.28.1.151 with SMTP id 145mr3599196wmb.25.1464269493086; Thu, 26 May 2016 06:31:33 -0700 (PDT) Received: from plouf.banquise.eu (gra33-3-88-180-253-3.fbx.proxad.net. [88.180.253.3]) by smtp.googlemail.com with ESMTPSA id q125sm3570294wmd.19.2016.05.26.06.31.31 (version=TLSv1/SSLv3 cipher=OTHER); Thu, 26 May 2016 06:31:32 -0700 (PDT) From: Benjamin Tissoires Subject: Re: [RFC PATCH 1/2] ACPI / button: Send "open" state after boot/resume To: "Rafael J. Wysocki" References: <1AE640813FDE7649BE1B193DEA596E883BBA2DED@SHSMSX101.ccr.corp.intel.com> <1AE640813FDE7649BE1B193DEA596E883BBA3088@SHSMSX101.ccr.corp.intel.com> Cc: "Zheng, Lv" , "Wysocki, Rafael J" , "Rafael J. Wysocki" , "Brown, Len" , Lv Zheng , Linux Kernel Mailing List , ACPI Devel Maling List , "Bastien Nocera:" Message-ID: <5746FAB3.7090604@gmail.com> Date: Thu, 26 May 2016 15:31:31 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.1 MIME-Version: 1.0 In-Reply-To: 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 [Jumping in the discussion at Bastien's request] On Thu, May 19, 2016 at 3:21 PM, Rafael J. Wysocki wrote: > On Thu, May 19, 2016 at 3:50 AM, Zheng, Lv wrote: >> Hi, >> >>> From: rjwysocki@gmail.com [mailto:rjwysocki@gmail.com] On Behalf Of >>> Rafael J. Wysocki >>> Subject: Re: [RFC PATCH 1/2] ACPI / button: Send "open" state after >>> boot/resume > > [cut] > >>> > That's because of systemd implementation. >>> > It contains code logic that: >>> > When the lid state is closed, a re-checking mechanism is installed. >>> > So if we do not send any notification after boot/resume and the old lid state >>> is "closed". >>> > systemd determines to suspend in the re-checking mechanism. >>> >>> If that really is the case, it is plain silly and I don't think we can >>> do anything in the kernel to help here. >> >> [Lv Zheng] >> The problem is: >> If we just removed the 2 lines sending wrong lid state after boot/resume. >> Problem couldn't be solved. >> It could only be solved by changing both the systemd and the kernel (deleting the 2 lines). > > There are two things here, there's a kernel issue (sending the fake > input events) and there's a user-visible problem. Yes, it may not be > possible to fix the user-visible problem by fixing the kernel issue > alone, but pretty much by definition we can only fix the kernel issue > in the kernel. > > However, it looks like it may not be possible to fix the user-visible > problem without fixing the kernel issue in the first place, so maybe > we should do that and attach the additional user space patch to the > bug entries in question? > > [cut] > >>> > I intentionally kept the _LID evaluation right after boot/resume. >>> > Because I validated Windows behavior. >>> > It seems Windows evaluates _LID right after boot. >>> > So I kept _LID evaluated right after boot to prevent compliance issues. >>> >>> I don't quite see what compliance issues could result from skipping >>> the _LID evaluation after boot. >> >> [Lv Zheng] >> I'm not sure if there is a platform putting named object initialization code in _LID. >> If you don't like it, we can stop evaluating _LID in the next version. > > Well, unless there is a well-documented reason for doing this, I'd at > least try to see what happens if we don't. > > Doing things for unspecified reasons is not a very good idea overall IMO. I found an issue on the surface 3 which explains why the initial state of the _LID switch is wrong. In gpiolib-acpi, we initialize an operation region for the LID switch to be controlled by a GPIO. This GPIO triggers an _E4C method when changed (see https://bugzilla.kernel.org/attachment.cgi?id=187171 in GPO0). This GPIO event actually sets the correct initial state of LIDB, which is forwarded by _LID. Now, on the surface 3, there is an other gpio event (_E10) which, when triggered at boot seems to put the sensor hub (over i2c-hid) in a better shape: I used to receive a5 a5 a5 a5 a5.. (garbage) after enabling S0 on the sensor and when requesting data from it. Now I get a nice [ +0.000137] i2c_hid i2c-MSHW0102:00: report (len=17): 11 00 01 02 05 00 00 00 00 00 00 00 00 00 18 fc 00 which seems more sensible from a HID point of view. The patch is the following: --- From 2c76d14a5ad089d0321a029edde3f91f3bc93ae3 Mon Sep 17 00:00:00 2001 From: Benjamin Tissoires Date: Thu, 26 May 2016 15:29:10 +0200 Subject: [PATCH] gpiolib-acpi: make sure we trigger the events at least once on boot The Surface 3 has its _LID state controlled by an ACPI operation region triggered by a GPIO event: OperationRegion (GPOR, GeneralPurposeIo, Zero, One) Field (GPOR, ByteAcc, NoLock, Preserve) { Connection ( GpioIo (Shared, PullNone, 0x0000, 0x0000, IoRestrictionNone, "\\_SB.GPO0", 0x00, ResourceConsumer, , ) { // Pin list 0x004C } ), HELD, 1 } Method (_E4C, 0, Serialized) // _Exx: Edge-Triggered GPE { If ((HELD == One)) { ^^LID.LIDB = One } Else { ^^LID.LIDB = Zero Notify (LID, 0x80) // Status Change } Notify (^^PCI0.SPI1.NTRG, One) // Device Check } Currently, the state of LIDB is wrong until the user actually closes or open the cover. We need to trigger the GPIO event once to update the internal ACPI state. Coincidentally, this also enables the integrated HID sensor hub which also requires an ACPI gpio operation region to start initialization. Signed-off-by: Benjamin Tissoires --- drivers/gpio/gpiolib-acpi.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c index 2dc5258..71775a0 100644 --- a/drivers/gpio/gpiolib-acpi.c +++ b/drivers/gpio/gpiolib-acpi.c @@ -175,7 +175,7 @@ static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares, irq_handler_t handler = NULL; struct gpio_desc *desc; unsigned long irqflags; - int ret, pin, irq; + int ret, pin, irq, value; if (ares->type != ACPI_RESOURCE_TYPE_GPIO) return AE_OK; @@ -214,6 +214,8 @@ static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares, gpiod_direction_input(desc); + value = gpiod_get_value(desc); + ret = gpiochip_lock_as_irq(chip, pin); if (ret) { dev_err(chip->parent, "Failed to lock GPIO as interrupt\n"); @@ -266,6 +268,15 @@ static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares, } list_add_tail(&event->node, &acpi_gpio->events); + + /* + * Make sure we trigger the initial state of the IRQ when + * using RISING or FALLING. + */ + if (((irqflags & IRQF_TRIGGER_RISING) && value == 1) || + ((irqflags & IRQF_TRIGGER_FALLING) && value == 0)) + handler(-1, event); + return AE_OK; fail_free_event: