From patchwork Fri Dec 11 22:15:35 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Anderson X-Patchwork-Id: 11969679 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 04018C433FE for ; Fri, 11 Dec 2020 23:13:25 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id C709022D06 for ; Fri, 11 Dec 2020 23:13:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2406772AbgLKWR2 (ORCPT ); Fri, 11 Dec 2020 17:17:28 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60150 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2406768AbgLKWQ3 (ORCPT ); Fri, 11 Dec 2020 17:16:29 -0500 Received: from mail-pj1-x1041.google.com (mail-pj1-x1041.google.com [IPv6:2607:f8b0:4864:20::1041]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2148FC06179C for ; Fri, 11 Dec 2020 14:15:44 -0800 (PST) Received: by mail-pj1-x1041.google.com with SMTP id hk16so3075383pjb.4 for ; Fri, 11 Dec 2020 14:15:44 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=eSXgvDLpVB7w74OUfv3cDJRgo0Ae+QBThzZ61gjo+zM=; b=RjZ/Yl0JAFwkfZgGfxUK6IM1WdZvS9NvWj2e7aH+rj+FX+LHVf5JZrhWKXJbGnGpeP tmUoJNEC3rB1oxmNbw52dkbdEvfZNA5zKbTD/+VmkdXldxrdyG4kRSbb2lPsxUWxGd61 zQ5OS9TS9SSwyvVOyG+79q7i4gO/e4fmrsrfM= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=eSXgvDLpVB7w74OUfv3cDJRgo0Ae+QBThzZ61gjo+zM=; b=ZyAmKLMMFtndQEmjmqy1A/74h70yWpWvcn+WDTu+g02v7QUc1J/NiLCVR+ONfmaxL9 dgBp+wmd28oOgPfX5UPKFZmVGzbcav8UnvEtfg1WtFbE37vK1mYOwzrOTKVfqyRPN6cp +O/cKHlhz/4pmmllB/x1Jwp2CWW2ON6fIXcNcy5FFEjx7cW1kf0bgMvycluWyaWsZMY+ xQTP3HAXBvPN/6VUWvS/dfhsapCY5+vVMaWK0b8xAuZJz4uyN1qebF+qJho59atMluCY SlgOkBn69a3ZX3P734JIa5zA/p+Q8KyCoTATzRKST/zpKB8kV3yAxU09CVcKt+Q/En7k IhSg== X-Gm-Message-State: AOAM531p35l5xa2ZKk7sNp07uULwbR5CndBANFIfaQ4031Pw0aAEr2ku UWJoqIyRxJIWBTUVC7fFq0EbpA== X-Google-Smtp-Source: ABdhPJyIv2Jehmt+dPfzYJ96qnWSdOKGFV7nzHGwAGIYg+MmYfmO6vSXyTlAjfuNOW3jvr2yWdLwgQ== X-Received: by 2002:a17:90a:f28f:: with SMTP id fs15mr15217771pjb.121.1607724943391; Fri, 11 Dec 2020 14:15:43 -0800 (PST) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:42b0:34ff:fe3d:58e6]) by smtp.gmail.com with ESMTPSA id s21sm11832981pgk.52.2020.12.11.14.15.41 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 11 Dec 2020 14:15:42 -0800 (PST) From: Douglas Anderson To: Marc Zyngier , Thomas Gleixner , Jason Cooper , Linus Walleij Cc: Bjorn Andersson , Rajendra Nayak , Maulik Shah , Stephen Boyd , linux-arm-msm@vger.kernel.org, Srinivas Ramana , Neeraj Upadhyay , linux-gpio@vger.kernel.org, Douglas Anderson , Andy Gross , Archana Sathyakumar , Lina Iyer , linux-kernel@vger.kernel.org Subject: [PATCH v4 1/4] irqchip: qcom-pdc: Fix phantom irq when changing between rising/falling Date: Fri, 11 Dec 2020 14:15:35 -0800 Message-Id: <20201211141514.v4.1.I2702919afc253e2a451bebc3b701b462b2d22344@changeid> X-Mailer: git-send-email 2.29.2.576.ga3fc446d84-goog MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org We have a problem if we use gpio-keys and configure wakeups such that we only want one edge to wake us up. AKA: wakeup-event-action = ; wakeup-source; Specifically we end up with a phantom interrupt that blocks suspend if the line was already high and we want wakeups on rising edges (AKA we want the GPIO to go low and then high again before we wake up). The opposite is also problematic. Specifically, here's what's happening today: 1. Normally, gpio-keys configures to look for both edges. Due to the current workaround introduced in commit c3c0c2e18d94 ("pinctrl: qcom: Handle broken/missing PDC dual edge IRQs on sc7180"), if the line was high we'd configure for falling edges. 2. At suspend time, we change to look for rising edges. 3. After qcom_pdc_gic_set_type() runs, we get a phantom interrupt. We can solve this by just clearing the phantom interrupt. NOTE: it is possible that this could cause problems for a client with very specific needs, but there's not much we can do with this hardware. As an example, let's say the interrupt signal is currently high and the client is looking for falling edges. The client now changes to look for rising edges. The client could possibly expect that if the line has a short pulse low (and back high) that it would always be detected. Specifically no matter when the pulse happened, it should either have tripped the (old) falling edge trigger or the (new) rising edge trigger. We will simply not trip it. We could narrow down the race a bit by polling our parent before changing types, but no matter what we do there will still be a period of time where we can't tell the difference between a real transition (or more than one transition) and the phantom. Fixes: f55c73aef890 ("irqchip/pdc: Add PDC interrupt controller for QCOM SoCs") Signed-off-by: Douglas Anderson Reviewed-by: Maulik Shah Tested-by: Maulik Shah Reviewed-by: Stephen Boyd --- There are no dependencies between this patch and patch #2/#3. It can go in by itself. Patches are only grouped together in one series because they address similar issues. Maulik has got confirmation from hardware guys and understands the problem. This patch is ready to land. Changes in v4: - No changes, this patch on its own ready to land. Changes in v3: - Adjusted the comment as per Maulik. Changes in v2: - 0 => false - If irq_chip_set_type_parent() fails don't bother clearing. - Add Fixes tag. drivers/irqchip/qcom-pdc.c | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/drivers/irqchip/qcom-pdc.c b/drivers/irqchip/qcom-pdc.c index bd39e9de6ecf..5dc63c20b67e 100644 --- a/drivers/irqchip/qcom-pdc.c +++ b/drivers/irqchip/qcom-pdc.c @@ -159,6 +159,8 @@ static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type) { int pin_out = d->hwirq; enum pdc_irq_config_bits pdc_type; + enum pdc_irq_config_bits old_pdc_type; + int ret; if (pin_out == GPIO_NO_WAKE_IRQ) return 0; @@ -187,9 +189,26 @@ static int qcom_pdc_gic_set_type(struct irq_data *d, unsigned int type) return -EINVAL; } + old_pdc_type = pdc_reg_read(IRQ_i_CFG, pin_out); pdc_reg_write(IRQ_i_CFG, pin_out, pdc_type); - return irq_chip_set_type_parent(d, type); + ret = irq_chip_set_type_parent(d, type); + if (ret) + return ret; + + /* + * When we change types the PDC can give a phantom interrupt. + * Clear it. Specifically the phantom shows up when reconfiguring + * polarity of interrupt without changing the state of the signal + * but let's be consistent and clear it always. + * + * Doing this works because we have IRQCHIP_SET_TYPE_MASKED so the + * interrupt will be cleared before the rest of the system sees it. + */ + if (old_pdc_type != pdc_type) + irq_chip_set_parent_state(d, IRQCHIP_STATE_PENDING, false); + + return 0; } static struct irq_chip qcom_pdc_gic_chip = { From patchwork Fri Dec 11 22:15:36 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Anderson X-Patchwork-Id: 11969667 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A1DABC4361B for ; Fri, 11 Dec 2020 23:10:52 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 762DC22D06 for ; Fri, 11 Dec 2020 23:10:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2406775AbgLKWQu (ORCPT ); Fri, 11 Dec 2020 17:16:50 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60152 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2406769AbgLKWQ3 (ORCPT ); Fri, 11 Dec 2020 17:16:29 -0500 Received: from mail-pl1-x641.google.com (mail-pl1-x641.google.com [IPv6:2607:f8b0:4864:20::641]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 86E10C0617A6 for ; Fri, 11 Dec 2020 14:15:45 -0800 (PST) Received: by mail-pl1-x641.google.com with SMTP id t6so5302521plq.1 for ; Fri, 11 Dec 2020 14:15:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=N3hybBp7EUYCA3vGL47ftPnar6+AkmdADxaaYRIyKZE=; b=IqdIWdK3AZ7RmSWUrbjPiTZk4GWmPtRYtLtdE0Jsap6eA0nC323cZ2zPDMc5rybJWm BWQHUnL3QHMdd6pRWBI7vqfgRxMNINHucXyUgMQd9GuK4QQ5etVIbLwsAQI5s6mrHnY1 3dTYROicSUrPOlBRkw2GYtIAARt4xkssKoB5M= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=N3hybBp7EUYCA3vGL47ftPnar6+AkmdADxaaYRIyKZE=; b=ZxYyBabX1yO7tsHMbOmb5m260+OrFZpdzzd4CZO34TlQT8TOe6KhKKIOFz9BUHg9XN DR60liir85eGYZm8C4ZmV7+lH/pLiIrlPjP9y+XfkKYpktLVcfPUmt8ZTmzH5J5zIv35 +6L8Wli1jU7wxbtk7wbP1HJUwJeTS7GBa8pdkNpfgGG8K7jsj7HVjl/Jxr8zmqS9MrkW v9ZXHCCOUq5DX7DVJWpvfoaEougRo14ZUt8tj4x6LT1HPkA6DBx3bNkUJkedrD127bgr 5fuQKeiHn4JOa3So97RJAlXM30EI7BT9qeHvryRe54iIYwEceO69gEZkhSJ1a1wH9o7P bKCw== X-Gm-Message-State: AOAM531BhXV/lNfcRa48/zyyz83t3wCp+p4vrCq7vFfXpOOl/BgRefLj qa3FOGKoligw1vgBCeyu4H7G5g== X-Google-Smtp-Source: ABdhPJwcduZHs4/bikaZX1IttJEerdlqSiW7Uwf3rjhmzSFcy5PFsOMxMelHaiFKZG0ZUXeAR3aDNw== X-Received: by 2002:a17:902:a504:b029:da:fbca:d49 with SMTP id s4-20020a170902a504b02900dafbca0d49mr6614064plq.72.1607724945117; Fri, 11 Dec 2020 14:15:45 -0800 (PST) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:42b0:34ff:fe3d:58e6]) by smtp.gmail.com with ESMTPSA id s21sm11832981pgk.52.2020.12.11.14.15.43 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 11 Dec 2020 14:15:44 -0800 (PST) From: Douglas Anderson To: Marc Zyngier , Thomas Gleixner , Jason Cooper , Linus Walleij Cc: Bjorn Andersson , Rajendra Nayak , Maulik Shah , Stephen Boyd , linux-arm-msm@vger.kernel.org, Srinivas Ramana , Neeraj Upadhyay , linux-gpio@vger.kernel.org, Douglas Anderson , Andy Gross , linux-kernel@vger.kernel.org Subject: [PATCH v4 2/4] pinctrl: qcom: Allow SoCs to specify a GPIO function that's not 0 Date: Fri, 11 Dec 2020 14:15:36 -0800 Message-Id: <20201211141514.v4.2.I3ad184e3423d8e479bc3e86f5b393abb1704a1d1@changeid> X-Mailer: git-send-email 2.29.2.576.ga3fc446d84-goog In-Reply-To: <20201211141514.v4.1.I2702919afc253e2a451bebc3b701b462b2d22344@changeid> References: <20201211141514.v4.1.I2702919afc253e2a451bebc3b701b462b2d22344@changeid> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org There's currently a comment in the code saying function 0 is GPIO. Instead of hardcoding it, let's add a member where an SoC can specify it. No known SoCs use a number other than 0, but this just makes the code clearer. NOTE: no SoC code needs to be updated since we can rely on zero-initialization. Signed-off-by: Douglas Anderson Reviewed-by: Stephen Boyd --- (no changes since v1) drivers/pinctrl/qcom/pinctrl-msm.c | 4 ++-- drivers/pinctrl/qcom/pinctrl-msm.h | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c index 77a25bdf0da7..588df91274e2 100644 --- a/drivers/pinctrl/qcom/pinctrl-msm.c +++ b/drivers/pinctrl/qcom/pinctrl-msm.c @@ -210,8 +210,8 @@ static int msm_pinmux_request_gpio(struct pinctrl_dev *pctldev, if (!g->nfuncs) return 0; - /* For now assume function 0 is GPIO because it always is */ - return msm_pinmux_set_mux(pctldev, g->funcs[0], offset); + return msm_pinmux_set_mux(pctldev, + g->funcs[pctrl->soc->gpio_func], offset); } static const struct pinmux_ops msm_pinmux_ops = { diff --git a/drivers/pinctrl/qcom/pinctrl-msm.h b/drivers/pinctrl/qcom/pinctrl-msm.h index 333f99243c43..e31a5167c91e 100644 --- a/drivers/pinctrl/qcom/pinctrl-msm.h +++ b/drivers/pinctrl/qcom/pinctrl-msm.h @@ -118,6 +118,7 @@ struct msm_gpio_wakeirq_map { * @wakeirq_dual_edge_errata: If true then GPIOs using the wakeirq_map need * to be aware that their parent can't handle dual * edge interrupts. + * @gpio_func: Which function number is GPIO (usually 0). */ struct msm_pinctrl_soc_data { const struct pinctrl_pin_desc *pins; @@ -134,6 +135,7 @@ struct msm_pinctrl_soc_data { const struct msm_gpio_wakeirq_map *wakeirq_map; unsigned int nwakeirq_map; bool wakeirq_dual_edge_errata; + unsigned int gpio_func; }; extern const struct dev_pm_ops msm_pinctrl_dev_pm_ops; From patchwork Fri Dec 11 22:15:37 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Anderson X-Patchwork-Id: 11969677 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_GIT autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C781DC433FE for ; Fri, 11 Dec 2020 23:13:09 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8E2E122D06 for ; Fri, 11 Dec 2020 23:13:09 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2406786AbgLKWRe (ORCPT ); Fri, 11 Dec 2020 17:17:34 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60244 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2406791AbgLKWRF (ORCPT ); Fri, 11 Dec 2020 17:17:05 -0500 Received: from mail-pl1-x644.google.com (mail-pl1-x644.google.com [IPv6:2607:f8b0:4864:20::644]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 4CA5DC061282 for ; Fri, 11 Dec 2020 14:15:47 -0800 (PST) Received: by mail-pl1-x644.google.com with SMTP id j1so5299109pld.3 for ; Fri, 11 Dec 2020 14:15:47 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=SCtCEdisGnwJ/m/Si2p+dicRUYBBDnixrXm0wKTUER8=; b=ZEW5iU4Fxh9N+lYYP0JBBAkU7IeYPA0hVyMVR63zKm8fhxD2mbwwvd1rQtQOXthW4L olxC/jeh/INp5y1veB8q6jCktNIYK54dpVYvno7fBxw7/g2jM3d/V1Z95t0aBqbkLnz6 IgUegy8umBQamsjVkhgOpEN055Z9uFuAUvjbE= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=SCtCEdisGnwJ/m/Si2p+dicRUYBBDnixrXm0wKTUER8=; b=eFWhg7qbU3Gvr5s7taBkzurH3xoKgBXXga4huWfeXWiKLIMzmxRm/a8K22y+LYaa4a cpr9w/w2/pI+kum456zNNA3rCw549aMiuk0vq5WGhmGk0XIt5LdoJXCke8ein8E4Xjpw vLI8BC70dWxtK2h6myPiczpkOY+kvpgi5xpju6i3IbrSIuSjAMzB+JtMAqt+LC5MXRDC ZLTCnPQ5jOm98oj2T73nOahLMjbhPF3CUvUB9Yv2mmElBjswMQPzPQ3j329Xza44ZurX IcbEi5RcbiHmBK44rXuL9wPka5WLXYkqGBvcgr9boGLvxNLy7f2CzWD4MVntLmqeWKZl m5wQ== X-Gm-Message-State: AOAM532TC1eqB6bLvVUbUl8eC2tpDUTvZkb14ErF4bbGSDF7jg4ypVc/ vny0vt7BmlIiHgx7uHqiVtO8Mw== X-Google-Smtp-Source: ABdhPJx0nEWDmBvQpziG9Q3KncbTdZ0FTJoDcPmTQQweqDdXF16kPufiackPupAAHqo28Bb3OH9Edw== X-Received: by 2002:a17:902:654f:b029:da:347d:7af3 with SMTP id d15-20020a170902654fb02900da347d7af3mr12764004pln.18.1607724946834; Fri, 11 Dec 2020 14:15:46 -0800 (PST) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:42b0:34ff:fe3d:58e6]) by smtp.gmail.com with ESMTPSA id s21sm11832981pgk.52.2020.12.11.14.15.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 11 Dec 2020 14:15:46 -0800 (PST) From: Douglas Anderson To: Marc Zyngier , Thomas Gleixner , Jason Cooper , Linus Walleij Cc: Bjorn Andersson , Rajendra Nayak , Maulik Shah , Stephen Boyd , linux-arm-msm@vger.kernel.org, Srinivas Ramana , Neeraj Upadhyay , linux-gpio@vger.kernel.org, Douglas Anderson , Andy Gross , linux-kernel@vger.kernel.org Subject: [PATCH v4 3/4] pinctrl: qcom: Don't clear pending interrupts when enabling Date: Fri, 11 Dec 2020 14:15:37 -0800 Message-Id: <20201211141514.v4.3.I7cf3019783720feb57b958c95c2b684940264cd1@changeid> X-Mailer: git-send-email 2.29.2.576.ga3fc446d84-goog In-Reply-To: <20201211141514.v4.1.I2702919afc253e2a451bebc3b701b462b2d22344@changeid> References: <20201211141514.v4.1.I2702919afc253e2a451bebc3b701b462b2d22344@changeid> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org In Linux, if a driver does disable_irq() and later does enable_irq() on its interrupt, I believe it's expecting these properties: * If an interrupt was pending when the driver disabled then it will still be pending after the driver re-enables. * If an edge-triggered interrupt comes in while an interrupt is disabled it should assert when the interrupt is re-enabled. If you think that the above sounds a lot like the disable_irq() and enable_irq() are supposed to be masking/unmasking the interrupt instead of disabling/enabling it then you've made an astute observation. Specifically when talking about interrupts, "mask" usually means to stop posting interrupts but keep tracking them and "disable" means to fully shut off interrupt detection. It's unfortunate that this is so confusing, but presumably this is all the way it is for historical reasons. Perhaps more confusing than the above is that, even though clients of IRQs themselves don't have a way to request mask/unmask vs. disable/enable calls, IRQ chips themselves can implement both. ...and yet more confusing is that if an IRQ chip implements disable/enable then they will be called when a client driver calls disable_irq() / enable_irq(). It does feel like some of the above could be cleared up. However, without any other core interrupt changes it should be clear that when an IRQ chip gets a request to "disable" an IRQ that it has to treat it like a mask of that IRQ. In any case, after that long interlude you can see that the "unmask and clear" can break things. Maulik tried to fix it so that we no longer did "unmask and clear" in commit 71266d9d3936 ("pinctrl: qcom: Move clearing pending IRQ to .irq_request_resources callback"), but it only handled the PDC case (it also had problems, but that's the subject of another patch). Let's fix this for the non-PDC case. From my understanding the source of the phantom interrupt in the non-PDC case was the one that could have been introduced in msm_gpio_irq_set_type(). Let's handle that one and then get rid of the clear. Fixes: 4b7618fdc7e6 ("pinctrl: qcom: Add irq_enable callback for msm gpio") Signed-off-by: Douglas Anderson Reviewed-by: Stephen Boyd Reviewed-by: Rajendra Nayak --- I don't have lots of good test cases here, so hopefully someone from Qualcomm can confirm that this works well for them and there isn't some other phantom interrupt source that I'm not aware of. Changes in v4: - ("pinctrl: qcom: Don't clear pending interrupts when enabling") split for v4. drivers/pinctrl/qcom/pinctrl-msm.c | 32 +++++++++++++----------------- 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c index 588df91274e2..f785646d1df7 100644 --- a/drivers/pinctrl/qcom/pinctrl-msm.c +++ b/drivers/pinctrl/qcom/pinctrl-msm.c @@ -774,7 +774,7 @@ static void msm_gpio_irq_mask(struct irq_data *d) raw_spin_unlock_irqrestore(&pctrl->lock, flags); } -static void msm_gpio_irq_clear_unmask(struct irq_data *d, bool status_clear) +static void msm_gpio_irq_unmask(struct irq_data *d) { struct gpio_chip *gc = irq_data_get_irq_chip_data(d); struct msm_pinctrl *pctrl = gpiochip_get_data(gc); @@ -792,17 +792,6 @@ static void msm_gpio_irq_clear_unmask(struct irq_data *d, bool status_clear) raw_spin_lock_irqsave(&pctrl->lock, flags); - if (status_clear) { - /* - * clear the interrupt status bit before unmask to avoid - * any erroneous interrupts that would have got latched - * when the interrupt is not in use. - */ - val = msm_readl_intr_status(pctrl, g); - val &= ~BIT(g->intr_status_bit); - msm_writel_intr_status(val, pctrl, g); - } - val = msm_readl_intr_cfg(pctrl, g); val |= BIT(g->intr_raw_status_bit); val |= BIT(g->intr_enable_bit); @@ -822,7 +811,7 @@ static void msm_gpio_irq_enable(struct irq_data *d) irq_chip_enable_parent(d); if (!test_bit(d->hwirq, pctrl->skip_wake_irqs)) - msm_gpio_irq_clear_unmask(d, true); + msm_gpio_irq_unmask(d); } static void msm_gpio_irq_disable(struct irq_data *d) @@ -837,11 +826,6 @@ static void msm_gpio_irq_disable(struct irq_data *d) msm_gpio_irq_mask(d); } -static void msm_gpio_irq_unmask(struct irq_data *d) -{ - msm_gpio_irq_clear_unmask(d, false); -} - /** * msm_gpio_update_dual_edge_parent() - Prime next edge for IRQs handled by parent. * @d: The irq dta. @@ -936,6 +920,7 @@ static int msm_gpio_irq_set_type(struct irq_data *d, unsigned int type) struct msm_pinctrl *pctrl = gpiochip_get_data(gc); const struct msm_pingroup *g; unsigned long flags; + bool was_enabled; u32 val; if (msm_gpio_needs_dual_edge_parent_workaround(d, type)) { @@ -997,6 +982,7 @@ static int msm_gpio_irq_set_type(struct irq_data *d, unsigned int type) * could cause the INTR_STATUS to be set for EDGE interrupts. */ val = msm_readl_intr_cfg(pctrl, g); + was_enabled = val & BIT(g->intr_raw_status_bit); val |= BIT(g->intr_raw_status_bit); if (g->intr_detection_width == 2) { val &= ~(3 << g->intr_detection_bit); @@ -1046,6 +1032,16 @@ static int msm_gpio_irq_set_type(struct irq_data *d, unsigned int type) } msm_writel_intr_cfg(val, pctrl, g); + /* + * The first time we set RAW_STATUS_EN it could trigger an interrupt. + * Clear it. This is safe because we have IRQCHIP_SET_TYPE_MASKED. + */ + if (!was_enabled) { + val = msm_readl_intr_status(pctrl, g); + val &= ~BIT(g->intr_status_bit); + msm_writel_intr_status(val, pctrl, g); + } + if (test_bit(d->hwirq, pctrl->dual_edge_irqs)) msm_gpio_update_dual_edge_pos(pctrl, g, d); From patchwork Fri Dec 11 22:15:38 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Douglas Anderson X-Patchwork-Id: 11969675 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-18.8 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED,USER_AGENT_GIT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 961E8C4361B for ; Fri, 11 Dec 2020 23:11:53 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 641A422D06 for ; Fri, 11 Dec 2020 23:11:53 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2406796AbgLKWRf (ORCPT ); Fri, 11 Dec 2020 17:17:35 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60264 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2406795AbgLKWRJ (ORCPT ); Fri, 11 Dec 2020 17:17:09 -0500 Received: from mail-pj1-x1041.google.com (mail-pj1-x1041.google.com [IPv6:2607:f8b0:4864:20::1041]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 269C8C0613CF for ; Fri, 11 Dec 2020 14:15:49 -0800 (PST) Received: by mail-pj1-x1041.google.com with SMTP id f14so2874790pju.4 for ; Fri, 11 Dec 2020 14:15:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=y7sUikV/F7gNe5Jy42I6mRR8y7okTjvOyaqlUQve9s8=; b=EYiob21Rk2nzqjeDbvTDWyfCsA5lwPGIN4WkDcg9WvyrIj+NdQqgSUBvcFUeMIaojD Qreh8bkGQrrrOE40m5XF9N4P8r0PR6Cvk+j5LTDKEfe7GA9uwRzUwZFKxnBIpsgEP7l9 6SPjZnja/FHUJJmDxX+CKRlXiqeTiJFh4QCq4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=y7sUikV/F7gNe5Jy42I6mRR8y7okTjvOyaqlUQve9s8=; b=QL1uFkxpfPnAwkywjZqC3jvl/Kq2VTD3UXlSvnfdNkgzQDgEdTUApeKhfL5MaXpcl5 cCCqot2+zPgxg28JowV7P1SjQdZOebvn/EETzS0+XSIfibSIKmldPFtRRdE6Y2MQY6i2 9P2mwouxr5Fk2bgySi+phSsRtGCO3YiuxzIjFs4bim6VlZGc/et0zk2cFYJ6eiCORS2W RRMbngwPI0VJl1PQDMy/54CtHAVOJrm3Z5doVuJHXinrVmuLK60WJY+z3QO+coS16mOf nsveWZ70IMq1DPLVRAJ6pcoGf3JnLP0N6jUGnOsYkBDSHTq33Q+xcqwctrgVnqGMKkq5 EMNw== X-Gm-Message-State: AOAM530QYyhQpeJsQ696RNnGbPH3t/E2frf69l4EqRqJamQ6V/98fBvC lW9DU6Zr2cYjCGkA4q93QAwepg== X-Google-Smtp-Source: ABdhPJwQaEII/ZFOuv5w3ZNFYFqtSx7WMh6WT8IlI/wAmhJhldG2bz6rBTVow8GDJR+cKB3eYQkmrw== X-Received: by 2002:a17:902:5581:b029:da:a817:1753 with SMTP id g1-20020a1709025581b02900daa8171753mr12637716pli.76.1607724948477; Fri, 11 Dec 2020 14:15:48 -0800 (PST) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:42b0:34ff:fe3d:58e6]) by smtp.gmail.com with ESMTPSA id s21sm11832981pgk.52.2020.12.11.14.15.47 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 11 Dec 2020 14:15:48 -0800 (PST) From: Douglas Anderson To: Marc Zyngier , Thomas Gleixner , Jason Cooper , Linus Walleij Cc: Bjorn Andersson , Rajendra Nayak , Maulik Shah , Stephen Boyd , linux-arm-msm@vger.kernel.org, Srinivas Ramana , Neeraj Upadhyay , linux-gpio@vger.kernel.org, Douglas Anderson , Andy Gross , linux-kernel@vger.kernel.org Subject: [PATCH v4 4/4] pinctrl: qcom: Clear possible pending parent irq when remuxing GPIOs Date: Fri, 11 Dec 2020 14:15:38 -0800 Message-Id: <20201211141514.v4.4.I771b6594b2a4d5b7fe7e12a991a6640f46386e8d@changeid> X-Mailer: git-send-email 2.29.2.576.ga3fc446d84-goog In-Reply-To: <20201211141514.v4.1.I2702919afc253e2a451bebc3b701b462b2d22344@changeid> References: <20201211141514.v4.1.I2702919afc253e2a451bebc3b701b462b2d22344@changeid> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org In commit 71266d9d3936 ("pinctrl: qcom: Move clearing pending IRQ to .irq_request_resources callback") we tried to make it so that the "enable" didn't clear pending interrupts for interrupts that were handled by our parent (the PDC). Unfortunately that regressed things. After that patch we found that sc7180-trogdor based devices could no longer enter suspend. Specifically in sc7180-trogdor.dtsi we configure the uart3 to have two pinctrl states, sleep and default, and mux between the two during runtime PM and system suspend (see geni_se_resources_{on,off}() for more details). The difference between the sleep and default state is that the RX pin is muxed to a GPIO during sleep and muxed to the UART otherwise. As per Qualcomm, when we mux the pin over to the UART function the PDC is still watching it / latching edges. These edges don't cause interrupts because the current code masks the interrupt unless we're entering suspend. However, as soon as we enter suspend we unmask the interrupt and it's counted as a wakeup. Let's deal with the problem like this: * When we mux away, we'll mask our parent. This isn't necessary in the above case since the parent already masked us, but it's a good idea in general. * When we mux back will clear any interrupts and unmask our parent if needed. Fixes: 71266d9d3936 ("pinctrl: qcom: Move clearing pending IRQ to .irq_request_resources callback") Signed-off-by: Douglas Anderson --- This patch depends on #2/#3 in the series, but not #1. #1 can land on its own and then #2/#3/#4 can land together even without #1. The only reason patch #1 and #2/#3/#4 are together in one series is because they address similar issues. I have done most of this patch testing on the Chrome OS 5.4 kernel tree (with many backports) but have sanity checked it on mainline. This patch definitely needs more testing / discussion, so please don't land without Qualcomm confirming that it looks OK in all the cases they are aware of. Changes in v4: - Totally rewrote again with my new understanding of the world. - Split non-PDC fix and PDC fix in two. Changes in v3: - Fixed bug in msm_gpio_direction_output() (s/oldval =/oldval = val =/) - Add back "if !skip_wake_irqs" test in msm_gpio_irq_enable() - For non-PDC, clear 1st interrupt in msm_gpio_irq_set_type() Changes in v2: - 0 => false - If skip_wake_irqs, don't need to clear normal intr. - Add comment about glitches in both output and input. drivers/pinctrl/qcom/pinctrl-msm.c | 42 +++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c index f785646d1df7..37fa95c5805c 100644 --- a/drivers/pinctrl/qcom/pinctrl-msm.c +++ b/drivers/pinctrl/qcom/pinctrl-msm.c @@ -171,7 +171,12 @@ static int msm_pinmux_set_mux(struct pinctrl_dev *pctldev, unsigned group) { struct msm_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); + struct gpio_chip *gc = &pctrl->chip; + unsigned int irq = irq_find_mapping(gc->irq.domain, group); + struct irq_data *d = irq_get_irq_data(irq); + unsigned int gpio_func = pctrl->soc->gpio_func; const struct msm_pingroup *g; + bool should_manage_parent; unsigned long flags; u32 val, mask; int i; @@ -187,6 +192,23 @@ static int msm_pinmux_set_mux(struct pinctrl_dev *pctldev, if (WARN_ON(i == g->nfuncs)) return -EINVAL; + /* + * If an GPIO interrupt is setup on this pin and those interrupts are + * handled by our parent we need special handling. Specifically the + * parent will still see the pin twiddle even when we're muxed away. + * + * If our GPIO was unmasked before muxing away from GPIO we need to + * mask our parent before switching so it doesn't see the twiddling. + * + * When we switch back we might need to clear any interrupts that were + * latched while were muxed away. + */ + should_manage_parent = d && d->parent_data && + test_bit(d->hwirq, pctrl->skip_wake_irqs); + + if (i != gpio_func && should_manage_parent && !irqd_irq_masked(d)) + irq_chip_mask_parent(d); + raw_spin_lock_irqsave(&pctrl->lock, flags); val = msm_readl_ctl(pctrl, g); @@ -196,6 +218,13 @@ static int msm_pinmux_set_mux(struct pinctrl_dev *pctldev, raw_spin_unlock_irqrestore(&pctrl->lock, flags); + if (i == gpio_func && should_manage_parent) { + irq_chip_set_parent_state(d, IRQCHIP_STATE_PENDING, false); + + if (!irqd_irq_masked(d)) + irq_chip_unmask_parent(d); + } + return 0; } @@ -1093,19 +1122,6 @@ static int msm_gpio_irq_reqres(struct irq_data *d) ret = -EINVAL; goto out; } - - /* - * Clear the interrupt that may be pending before we enable - * the line. - * This is especially a problem with the GPIOs routed to the - * PDC. These GPIOs are direct-connect interrupts to the GIC. - * Disabling the interrupt line at the PDC does not prevent - * the interrupt from being latched at the GIC. The state at - * GIC needs to be cleared before enabling. - */ - if (d->parent_data && test_bit(d->hwirq, pctrl->skip_wake_irqs)) - irq_chip_set_parent_state(d, IRQCHIP_STATE_PENDING, 0); - return 0; out: module_put(gc->owner);