From patchwork Tue Dec 4 19:42:46 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: 1007825 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 438XMb4vDqz9s3Z for ; Wed, 5 Dec 2018 06:42:51 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725859AbeLDTmu (ORCPT ); Tue, 4 Dec 2018 14:42:50 -0500 Received: from mx1.redhat.com ([209.132.183.28]:35374 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725797AbeLDTmu (ORCPT ); Tue, 4 Dec 2018 14:42:50 -0500 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 244F130023CD; Tue, 4 Dec 2018 19:42:50 +0000 (UTC) Received: from shalem.localdomain.com (unknown [10.36.118.17]) by smtp.corp.redhat.com (Postfix) with ESMTP id B47A75C21C; Tue, 4 Dec 2018 19:42:48 +0000 (UTC) From: Hans de Goede To: Mika Westerberg , Andy Shevchenko , Heikki Krogerus , Linus Walleij Cc: Hans de Goede , linux-gpio@vger.kernel.org, linux-acpi@vger.kernel.org Subject: [PATCH 1/2] pinctrl: cherryview: Add chv_gpio_clear_triggering() helper function Date: Tue, 4 Dec 2018 20:42:46 +0100 Message-Id: <20181204194247.671-1-hdegoede@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.44]); Tue, 04 Dec 2018 19:42:50 +0000 (UTC) Sender: linux-gpio-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-gpio@vger.kernel.org This is a preparation patch for clearing the interrupt trigger from chv_gpio_disable_free(). Signed-off-by: Hans de Goede Acked-by: Mika Westerberg --- drivers/pinctrl/intel/pinctrl-cherryview.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c index 9b0f4b9ef482..4ff7d60a2bf1 100644 --- a/drivers/pinctrl/intel/pinctrl-cherryview.c +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c @@ -846,6 +846,19 @@ static int chv_pinmux_set_mux(struct pinctrl_dev *pctldev, return 0; } +static void chv_gpio_clear_triggering(struct chv_pinctrl *pctrl, + unsigned int offset) +{ + void __iomem *reg; + u32 value; + + reg = chv_padreg(pctrl, offset, CHV_PADCTRL1); + value = readl(reg); + value &= ~CHV_PADCTRL1_INTWAKECFG_MASK; + value &= ~CHV_PADCTRL1_INVRXTX_MASK; + chv_writel(value, reg); +} + static int chv_gpio_request_enable(struct pinctrl_dev *pctldev, struct pinctrl_gpio_range *range, unsigned int offset) @@ -876,11 +889,7 @@ static int chv_gpio_request_enable(struct pinctrl_dev *pctldev, } /* Disable interrupt generation */ - reg = chv_padreg(pctrl, offset, CHV_PADCTRL1); - value = readl(reg); - value &= ~CHV_PADCTRL1_INTWAKECFG_MASK; - value &= ~CHV_PADCTRL1_INVRXTX_MASK; - chv_writel(value, reg); + chv_gpio_clear_triggering(pctrl, offset); reg = chv_padreg(pctrl, offset, CHV_PADCTRL0); value = readl(reg); From patchwork Tue Dec 4 19:42:47 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: 1007826 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 438XMc5ZQ1z9sBh for ; Wed, 5 Dec 2018 06:42:52 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725887AbeLDTmw (ORCPT ); Tue, 4 Dec 2018 14:42:52 -0500 Received: from mx1.redhat.com ([209.132.183.28]:40653 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725797AbeLDTmw (ORCPT ); Tue, 4 Dec 2018 14:42:52 -0500 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id D4BF6BDD1; Tue, 4 Dec 2018 19:42:51 +0000 (UTC) Received: from shalem.localdomain.com (unknown [10.36.118.17]) by smtp.corp.redhat.com (Postfix) with ESMTP id 66D405C21C; Tue, 4 Dec 2018 19:42:50 +0000 (UTC) From: Hans de Goede To: Mika Westerberg , Andy Shevchenko , Heikki Krogerus , Linus Walleij Cc: Hans de Goede , linux-gpio@vger.kernel.org, linux-acpi@vger.kernel.org Subject: [PATCH 2/2] pinctrl: cherryview: Stop clearing the GPIO_EN bit from chv_gpio_disable_free Date: Tue, 4 Dec 2018 20:42:47 +0100 Message-Id: <20181204194247.671-2-hdegoede@redhat.com> In-Reply-To: <20181204194247.671-1-hdegoede@redhat.com> References: <20181204194247.671-1-hdegoede@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Tue, 04 Dec 2018 19:42:52 +0000 (UTC) Sender: linux-gpio-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-gpio@vger.kernel.org Clearing the GPIO_EN bit from chv_gpio_disable_free is a bad idea and pinctrl-cherryview.c is the only Intel pinctrl driver doing something like this. Clearing the GPIO_EN bit means that if the pin was an output it is now effectively floating. The datasheet is not clear what happens to pull ups / downs in this case, but from testing it looks like these are disabled too, also floating input pins. One example where this is causing issues is the soc_button_array input driver, this parses ACPI tables to create 2 platform devices for the gpio_keys input driver. The list of GPIOs is passed through struct gpio_keys_platform_data which uses gpio numbers rather then gpio_desc pointers. The buttons handled by this drivers short the pin to ground when pressed and the volume buttons rely on the SoC's internal pull-up to pull the pin high when the button is not pressed. To get the gpio number, the soc_button_array code calls gpiod_get_index followed by a desc_to_gpio call and then gpiod_put on the gpio_desc. This last call causes chv_gpio_disable_free to clear the GPIO_EN bit. When the gpio_keys driver then loads next it gets the gpio_desc again causing the GPIO_EN bit to be set again and immediately reads the GPIO value which for the volume buttons reads 0 at this time, causing a spurious press of the volume buttons to get reported. Putting a small delay between the gpio_desc request and the read fixes this, I assume that this is caused by the pull-up being temporarily disabled while the GPIO_EN bit is cleared as the powerbutton which also has its GPIO_EN bit cleared does not have this problem. The soc_button_array code is not the only code temporarily requesting GPIOs the DWC3 PCI code also does this, to set the enable and reset GPIOs for the external phy, so that the code instantiating the ULPI phy can read the vendor and product ID registers from the phy. These GPIOs are released after this so that the PHY driver can claim and use them when it loads. Another example of temporary GPIO usage would be a user-space set_gpio utility using the userspace ioctls to set a GPIO as output value 0 or 1, having the GPIO revert to floating as soon as this utility exits would certainly be unexpected behavior. One argument in favor of clearing the GPIO_EN bit is if the GPIO is going to be muxed to another function after being released, but in that case chv_pinmux_set_mux() already clears it. TL;DR: Clearing the GPIO_EN bit from is a bad idea, this commit therefor removes the clearing from chv_gpio_disable_free(), replacing it with code to clear the interrupt-trigger condition so that the GPIO stops generating interrupts when released, as pinctrl-baytrail.c does. Note this commit adds a !chv_pad_locked() condition to the trigger clearing call, which the original GPIO_EN clearing code was missing. Signed-off-by: Hans de Goede Acked-by: Mika Westerberg --- drivers/pinctrl/intel/pinctrl-cherryview.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c index 4ff7d60a2bf1..2042f5f6833d 100644 --- a/drivers/pinctrl/intel/pinctrl-cherryview.c +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c @@ -921,14 +921,11 @@ static void chv_gpio_disable_free(struct pinctrl_dev *pctldev, { struct chv_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); unsigned long flags; - void __iomem *reg; - u32 value; raw_spin_lock_irqsave(&chv_lock, flags); - reg = chv_padreg(pctrl, offset, CHV_PADCTRL0); - value = readl(reg) & ~CHV_PADCTRL0_GPIOEN; - chv_writel(value, reg); + if (!chv_pad_locked(pctrl, offset)) + chv_gpio_clear_triggering(pctrl, offset); raw_spin_unlock_irqrestore(&chv_lock, flags); }