From patchwork Wed Jul 11 20:23:39 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans de Goede X-Patchwork-Id: 942695 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=linux-gpio-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dmarc=fail (p=none dis=none) header.from=redhat.com Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 41QrBB5ZwXz9s01 for ; Thu, 12 Jul 2018 06:23:46 +1000 (AEST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387692AbeGKU3q (ORCPT ); Wed, 11 Jul 2018 16:29:46 -0400 Received: from mx3-rdu2.redhat.com ([66.187.233.73]:57560 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1732855AbeGKU3q (ORCPT ); Wed, 11 Jul 2018 16:29:46 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.rdu2.redhat.com [10.11.54.5]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C677A4074475; Wed, 11 Jul 2018 20:23:44 +0000 (UTC) Received: from shalem.localdomain.com (ovpn-117-225.ams2.redhat.com [10.36.117.225]) by smtp.corp.redhat.com (Postfix) with ESMTP id 97B221C65F; Wed, 11 Jul 2018 20:23:43 +0000 (UTC) From: Hans de Goede To: Mika Westerberg , Heikki Krogerus , Linus Walleij , Benjamin Tissoires Cc: Hans de Goede , Andy Shevchenko , linux-gpio@vger.kernel.org Subject: [PATCH] gpiolib-acpi: make sure we trigger edge events at least once on boot Date: Wed, 11 Jul 2018 22:23:39 +0200 Message-Id: <20180711202339.16950-2-hdegoede@redhat.com> In-Reply-To: <20180711202339.16950-1-hdegoede@redhat.com> References: <20180711202339.16950-1-hdegoede@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.11.54.5 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Wed, 11 Jul 2018 20:23:44 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.7]); Wed, 11 Jul 2018 20:23:44 +0000 (UTC) for IP:'10.11.54.5' DOMAIN:'int-mx05.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'hdegoede@redhat.com' RCPT:'' Sender: linux-gpio-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-gpio@vger.kernel.org From: Benjamin Tissoires On some systems using edge triggered ACPI Event Interrupts, the initial state at boot is not setup by the firmware, instead relying on the edge irq event handler running at least once to setup the initial state. 2 known examples of this are: 1) 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 Surface 2 integrated HID sensor hub which also requires an ACPI gpio operation region to start initialization. 2) Various Bay Trail based tablets come with an external USB mux and TI T1210B USB phy to enable USB gadget mode. The mux is controlled by a GPIO which is controlled by an edge triggered ACPI Event Interrupt which monitors the micro-USB ID pin. When the tablet is connected to a PC (or no cable is plugged in), the ID pin is high and the tablet should be in gadget mode. But the GPIO controlling the mux is initialized by the firmware so that the USB data lines are muxed to the host controller. This means that if the user wants to use gadget mode, the user needs to first plug in a host-cable to force the ID pin low and then unplug it and connect the tablet to a PC, to get the ACPI event handler to run and switch the mux to device mode, This commit fixes both by running the event-handler once on boot. Note that the running of the event-handler is done 3 seconds after the GPIO driver loads, this is done because the event-handler AML code may rely on OperationRegions registered by other drivers and the GPIO driver is initialized very early on, where as the total init of all drivers can take up to 2.5 seconds. This delay avoid errors like these: [ 0.133026] ACPI Error: No handler for Region [XSCG] ((____ptrval____)) [GenericSerialBus] (20180531/evregion-132) [ 0.133036] ACPI Error: Region GenericSerialBus (ID=9) has no handler (20180531/exfldio-265) [ 0.133046] ACPI Error: Method parse/execution failed \_SB.GPO2._E12, AE_NOT_EXIST (20180531/psparse-516) Signed-off-by: Benjamin Tissoires [hdegoede: Document BYT USB mux reliance on initial trigger] [hdegoede: Run event handler after a delay, rather then immediately] Signed-off-by: Hans de Goede --- drivers/gpio/gpiolib-acpi.c | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c index e2232cbcec8b..75556b20d98e 100644 --- a/drivers/gpio/gpiolib-acpi.c +++ b/drivers/gpio/gpiolib-acpi.c @@ -20,15 +20,20 @@ #include #include #include +#include #include "gpiolib.h" +/* Initialization of all builtin drivers may take up to 2.5 seconds */ +#define ACPI_IRQ_INITIAL_SYNC_DELAY msecs_to_jiffies(3000) + struct acpi_gpio_event { struct list_head node; acpi_handle handle; unsigned int pin; unsigned int irq; struct gpio_desc *desc; + struct delayed_work initial_sync_work; }; struct acpi_gpio_connection { @@ -89,6 +94,7 @@ static irqreturn_t acpi_gpio_irq_handler(int irq, void *data) { struct acpi_gpio_event *event = data; + cancel_delayed_work_sync(&event->initial_sync_work); acpi_evaluate_object(event->handle, NULL, NULL, NULL); return IRQ_HANDLED; @@ -125,6 +131,15 @@ bool acpi_gpio_get_irq_resource(struct acpi_resource *ares, } EXPORT_SYMBOL_GPL(acpi_gpio_get_irq_resource); +static void acpi_irq_initial_sync(struct work_struct *work) +{ + struct acpi_gpio_event *event = + container_of(work, struct acpi_gpio_event, + initial_sync_work.work); + + acpi_evaluate_object(event->handle, NULL, NULL, NULL); +} + static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares, void *context) { @@ -136,7 +151,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 (!acpi_gpio_get_irq_resource(ares, &agpio)) return AE_OK; @@ -167,6 +182,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"); @@ -208,6 +225,7 @@ static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares, event->irq = irq; event->pin = pin; event->desc = desc; + INIT_DELAYED_WORK(&event->initial_sync_work, acpi_irq_initial_sync); ret = request_threaded_irq(event->irq, NULL, handler, irqflags, "ACPI:Event", event); @@ -222,6 +240,19 @@ static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares, enable_irq_wake(irq); list_add_tail(&event->node, &acpi_gpio->events); + + /* + * Make sure we trigger the initial state of the IRQ when using RISING + * or FALLING. Note this uses a delayed work-queue as the AML code + * may refer to OperationRegions from other (builtin) drivers which + * may be probed after us. + */ + if (handler == acpi_gpio_irq_handler && + (((irqflags & IRQF_TRIGGER_RISING) && value == 1) || + ((irqflags & IRQF_TRIGGER_FALLING) && value == 0))) + schedule_delayed_work(&event->initial_sync_work, + ACPI_IRQ_INITIAL_SYNC_DELAY); + return AE_OK; fail_free_event: @@ -298,6 +329,7 @@ void acpi_gpiochip_free_interrupts(struct gpio_chip *chip) disable_irq_wake(event->irq); free_irq(event->irq, event); + cancel_delayed_work_sync(&event->initial_sync_work); desc = event->desc; if (WARN_ON(IS_ERR(desc))) continue;