From patchwork Fri Jan 8 17:35:13 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 12007103 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=-19.0 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 14B95C433E0 for ; Fri, 8 Jan 2021 17:36:43 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id D12FB23A56 for ; Fri, 8 Jan 2021 17:36:42 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728306AbhAHRg1 (ORCPT ); Fri, 8 Jan 2021 12:36:27 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55686 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728470AbhAHRg0 (ORCPT ); Fri, 8 Jan 2021 12:36:26 -0500 Received: from mail-pg1-x52f.google.com (mail-pg1-x52f.google.com [IPv6:2607:f8b0:4864:20::52f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A665BC061381 for ; Fri, 8 Jan 2021 09:35:46 -0800 (PST) Received: by mail-pg1-x52f.google.com with SMTP id n10so8029121pgl.10 for ; Fri, 08 Jan 2021 09:35:46 -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=fIz/eHR9LskEHw4WLLVJ7WCTAff9/EEzxkuWGjNwGgU=; b=WlwaqwFsYhMsqUKnT/AaL7XcehUzRYirQrcBXLWPIBWhlDag9bjYtAVro7EPZ9PGtV RZNU3W3lobcv+R+adPpTbkKfDnllsiSMvx1JRCSe6qbJ7FBddv4+UNY0tkPZ8OiLZxVw ZnELNoxN4OylGrK8KvSs5q2vNXSZLDWfTYan8= 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=fIz/eHR9LskEHw4WLLVJ7WCTAff9/EEzxkuWGjNwGgU=; b=WIQBoDE8tnf6JOp4sVdWzktBcr+N5kd3pKQwtbgnNBYsc8aEY4dSSLehd5bSrqY4QF tTUMh5Wbhim8nb4il4LDYzEYl+p9pv9skfNNHZ2DApf5a+ZWZYaWRTzNWE/vz4ga5W0c PSLp15PYCt5w3bhE1oTNbn1zRtpeIQeJdxpIJbQeeeY6Iwka7pihOSg3k/TqPJcY6RI6 7XZDu6rVytG38OGO+tsuc3fL2vIPQ2Zv1KAKuZ7NkG74rO9qoCYsLELo3/0EONnDlXol VH4bzG2L42Rxwkl0L0DBNAEgGzuWZM1C+MYp7Ycm5LC1bzAf1piX7t65zOq2HwNFB3s5 mMxA== X-Gm-Message-State: AOAM53197fMSgP2UZ0G/Xol9P0FxBfkzifauTSwFqHCm92PYCGzLgjfJ tIWgQNo+XLUxeXeUVXdqECWN9A== X-Google-Smtp-Source: ABdhPJwgCuoXzQ8WbtRCg6+DM7Uo1SMHWPIFQGnMvfK1xRW9OJclLvCSV+xdvBG2f8r5GKEvIhmTzA== X-Received: by 2002:a62:9208:0:b029:19e:a15f:169e with SMTP id o8-20020a6292080000b029019ea15f169emr4585621pfd.71.1610127346134; Fri, 08 Jan 2021 09:35: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 z23sm10245619pfj.143.2021.01.08.09.35.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 08 Jan 2021 09:35:45 -0800 (PST) From: Douglas Anderson To: Marc Zyngier , Thomas Gleixner , Jason Cooper , Linus Walleij Cc: Bjorn Andersson , Neeraj Upadhyay , Rajendra Nayak , Stephen Boyd , Maulik Shah , linux-gpio@vger.kernel.org, Srinivas Ramana , linux-arm-msm@vger.kernel.org, Douglas Anderson , Andy Gross , linux-kernel@vger.kernel.org Subject: [PATCH v5 1/4] pinctrl: qcom: Allow SoCs to specify a GPIO function that's not 0 Date: Fri, 8 Jan 2021 09:35:13 -0800 Message-Id: <20210108093339.v5.1.I3ad184e3423d8e479bc3e86f5b393abb1704a1d1@changeid> X-Mailer: git-send-email 2.29.2.729.g45daf8777d-goog 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 Reviewed-by: Maulik Shah Tested-by: Maulik Shah Reviewed-by: Bjorn Andersson --- (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 e051aecf95c4..1d2a78452c2d 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 Jan 8 17:35:14 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 12007105 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=-19.0 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 D16E0C433E0 for ; Fri, 8 Jan 2021 17:37:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id A1B9D23A58 for ; Fri, 8 Jan 2021 17:37:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728524AbhAHRhH (ORCPT ); Fri, 8 Jan 2021 12:37:07 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55794 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727884AbhAHRhH (ORCPT ); Fri, 8 Jan 2021 12:37:07 -0500 Received: from mail-pf1-x432.google.com (mail-pf1-x432.google.com [IPv6:2607:f8b0:4864:20::432]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 858BEC0612FF for ; Fri, 8 Jan 2021 09:35:48 -0800 (PST) Received: by mail-pf1-x432.google.com with SMTP id c12so6666214pfo.10 for ; Fri, 08 Jan 2021 09:35:48 -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=ZYkzYWMleX3qMt/pYBfE+NT2hyGkrkjahRGphftC+6U=; b=NHlEzY+bSJyUuMChX1xu1cawgmxyUcHAbrm3nVHqPnG/bRp7qYCTOeZ0oLneiu4A63 1CVsPYp6LNwFyLxuS33DztP8G3p9CGGYRUGdAEvlAaFNh4QqlWQ9f1hxFLolsOsCMfyW TRLloydHP96EsjiC1hiUcwdjIvVDnTfGMaV/M= 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=ZYkzYWMleX3qMt/pYBfE+NT2hyGkrkjahRGphftC+6U=; b=TGFPe7x3dVytqLhdxW+WwqtGoVFE7EcdvwsU5aZANqzQu77LgyeAkhAkgzXBlmEmpw jHmo9yWHOdxEhETTP7AE9IWG9spapZZqktf3WbSwYd/YxJaZb4GLX9RSQ7SKTtNxkD8f 1jlUL98vZK875QKGYxTHKfsR5/1KiAjBsLO5yHQz3vocvETnAJ3FIWsi+a67lvyS/qGl ickrcVZJRL6Nx8MSVfoLyHP5I01tr9AeQheNqVsfB7UmJ6p4Zj3HWfMMT5KmMdJuR07c jiDiZkX6o6XM0AAuxtB9XKxsR9wWe6X2eW/c8per9nMTzHeaXdluUonJPXY1mIdsdWGe kU5g== X-Gm-Message-State: AOAM532goy6FTN3yOpdDiwaDKrDe8509QZMWfivjg1ejqlA1+eFTOZpH orWdEYIovPYx4BrybidILzwHwg== X-Google-Smtp-Source: ABdhPJyK0aabqVagQDNA17Xpi+5kBKYUQBiRd6UN2GtPqlDhIWRZKS9di/e0FCPbLoRKAxKH7axQhA== X-Received: by 2002:a63:1142:: with SMTP id 2mr8114323pgr.263.1610127348077; Fri, 08 Jan 2021 09:35: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 z23sm10245619pfj.143.2021.01.08.09.35.46 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 08 Jan 2021 09:35:47 -0800 (PST) From: Douglas Anderson To: Marc Zyngier , Thomas Gleixner , Jason Cooper , Linus Walleij Cc: Bjorn Andersson , Neeraj Upadhyay , Rajendra Nayak , Stephen Boyd , Maulik Shah , linux-gpio@vger.kernel.org, Srinivas Ramana , linux-arm-msm@vger.kernel.org, Douglas Anderson , Andy Gross , linux-kernel@vger.kernel.org Subject: [PATCH v5 2/4] pinctrl: qcom: No need to read-modify-write the interrupt status Date: Fri, 8 Jan 2021 09:35:14 -0800 Message-Id: <20210108093339.v5.2.I3635de080604e1feda770591c5563bd6e63dd39d@changeid> X-Mailer: git-send-email 2.29.2.729.g45daf8777d-goog In-Reply-To: <20210108093339.v5.1.I3ad184e3423d8e479bc3e86f5b393abb1704a1d1@changeid> References: <20210108093339.v5.1.I3ad184e3423d8e479bc3e86f5b393abb1704a1d1@changeid> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org When the Qualcomm pinctrl driver wants to Ack an interrupt, it does a read-modify-write on the interrupt status register. On some SoCs it makes sure that the status bit is 1 to "Ack" and on others it makes sure that the bit is 0 to "Ack". Presumably the first type of interrupt controller is a "write 1 to clear" type register and the second just let you directly set the interrupt status register. As far as I can tell from scanning structure definitions, the interrupt status bit is always in a register by itself. Thus with both types of interrupt controllers it is safe to "Ack" interrupts without doing a read-modify-write. We can do a simple write. It should be noted that if the interrupt status bit _was_ ever in a register with other things (like maybe status bits for other GPIOs): a) For "write 1 clear" type controllers then read-modify-write would be totally wrong because we'd accidentally end up clearing interrupts we weren't looking at. b) For "direct set" type controllers then read-modify-write would also be wrong because someone setting one of the other bits in the register might accidentally clear (or set) our interrupt. I say this simply to show that the current read-modify-write doesn't provide any sort of "future proofing" of the code. In fact (for "write 1 clear" controllers) the new code is slightly more "future proof" since it would allow more than one interrupt status bits to share a register. NOTE: this code fixes no bugs--it simply avoids an extra register read. Signed-off-by: Douglas Anderson Reviewed-by: Maulik Shah Tested-by: Maulik Shah Reviewed-by: Stephen Boyd Reviewed-by: Bjorn Andersson --- Changes in v5: - ("pinctrl: qcom: No need to read-modify-write the ...") new for v5. drivers/pinctrl/qcom/pinctrl-msm.c | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c index 1d2a78452c2d..1787ada6bfab 100644 --- a/drivers/pinctrl/qcom/pinctrl-msm.c +++ b/drivers/pinctrl/qcom/pinctrl-msm.c @@ -792,16 +792,13 @@ 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); - } + /* + * clear the interrupt status bit before unmask to avoid + * any erroneous interrupts that would have got latched + * when the interrupt is not in use. + */ + if (status_clear) + msm_writel_intr_status(0, pctrl, g); val = msm_readl_intr_cfg(pctrl, g); val |= BIT(g->intr_raw_status_bit); @@ -906,11 +903,7 @@ static void msm_gpio_irq_ack(struct irq_data *d) raw_spin_lock_irqsave(&pctrl->lock, flags); - val = msm_readl_intr_status(pctrl, g); - if (g->intr_ack_high) - val |= BIT(g->intr_status_bit); - else - val &= ~BIT(g->intr_status_bit); + val = (g->intr_ack_high) ? BIT(g->intr_status_bit) : 0; msm_writel_intr_status(val, pctrl, g); if (test_bit(d->hwirq, pctrl->dual_edge_irqs)) From patchwork Fri Jan 8 17:35:15 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 12007107 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=-19.0 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 548C2C43381 for ; Fri, 8 Jan 2021 17:37:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 1629123A56 for ; Fri, 8 Jan 2021 17:37:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728530AbhAHRhI (ORCPT ); Fri, 8 Jan 2021 12:37:08 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55800 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728395AbhAHRhH (ORCPT ); Fri, 8 Jan 2021 12:37:07 -0500 Received: from mail-pf1-x42d.google.com (mail-pf1-x42d.google.com [IPv6:2607:f8b0:4864:20::42d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 57AFEC06129E for ; Fri, 8 Jan 2021 09:35:50 -0800 (PST) Received: by mail-pf1-x42d.google.com with SMTP id x126so6676038pfc.7 for ; Fri, 08 Jan 2021 09:35:50 -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=NWeNVhHBQriYHElYut4sIDkeHCxpOsy0zeO3R0EcrZs=; b=gitdbsRXUUNlxvhhaDHU9H74LPVrCzER8lNo58HHLgmXTbtA4Zz0DS47MFLORIDenw AuIjVRUIYBxTr+zdgUMdLGrT49UyLpVE7nYPnEwgudirgVEUcQ2kKSj8GvkzivuYfo9R oEWhmBwgYquKQr+3HCKaquX6HOmmgZ6fQnV4U= 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=NWeNVhHBQriYHElYut4sIDkeHCxpOsy0zeO3R0EcrZs=; b=FWP6zD9kWSjtUDu1OdC2TsDTT16jTkJquvIwFpTEYa8tDFxAmdAaNvY0I/NTlS3Alb Q4kO6SNqIGfM76XJ5ShUO46rMIIjO41k/4rC+jj7mrGjlf9gyQD5Kk7X+XUmMlyzuRpU OheRBpiinWo+aqrUggntmcKZ4LghDLrgOfIYTsDsLaHW2VwogrrWzExj+qVu6X9CQw/e LNckam7NXW4DN94eH2ExXwNBY//Ttlx97LmmC65J615mRdabmycNkCFXvgFVbVDkWSGx S73pOu1ZExR3ztmDAUffSWMxyqmDqN9slkUjM0pbjnENh80J5UNzFRM+5UZh6uddeWyK qVLQ== X-Gm-Message-State: AOAM532zCknD1HVQvc8F/ovwiRFs+PrRYTpXmBZvU8+sYYzwejt5Qanb +dv5NkHOI0AsklEDQYslXoGc8g== X-Google-Smtp-Source: ABdhPJwGKvhGrqpD2mUiuU9++lk/mDnNGEcCmGVCYte+Fgza04DZRHEsAH8Sgef/6OucwTC5fZ1xgA== X-Received: by 2002:a63:dc4c:: with SMTP id f12mr8023342pgj.167.1610127349809; Fri, 08 Jan 2021 09:35:49 -0800 (PST) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:42b0:34ff:fe3d:58e6]) by smtp.gmail.com with ESMTPSA id z23sm10245619pfj.143.2021.01.08.09.35.48 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 08 Jan 2021 09:35:49 -0800 (PST) From: Douglas Anderson To: Marc Zyngier , Thomas Gleixner , Jason Cooper , Linus Walleij Cc: Bjorn Andersson , Neeraj Upadhyay , Rajendra Nayak , Stephen Boyd , Maulik Shah , linux-gpio@vger.kernel.org, Srinivas Ramana , linux-arm-msm@vger.kernel.org, Douglas Anderson , Andy Gross , linux-kernel@vger.kernel.org Subject: [PATCH v5 3/4] pinctrl: qcom: Properly clear "intr_ack_high" interrupts when unmasking Date: Fri, 8 Jan 2021 09:35:15 -0800 Message-Id: <20210108093339.v5.3.I32d0f4e174d45363b49ab611a13c3da8f1e87d0f@changeid> X-Mailer: git-send-email 2.29.2.729.g45daf8777d-goog In-Reply-To: <20210108093339.v5.1.I3ad184e3423d8e479bc3e86f5b393abb1704a1d1@changeid> References: <20210108093339.v5.1.I3ad184e3423d8e479bc3e86f5b393abb1704a1d1@changeid> MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org In commit 4b7618fdc7e6 ("pinctrl: qcom: Add irq_enable callback for msm gpio") we tried to Ack interrupts during unmask. However, that patch forgot to check "intr_ack_high" so, presumably, it only worked for a certain subset of SoCs. Let's add a small accessor so we don't need to open-code the logic in both places. This was found by code inspection. I don't have any access to the hardware in question nor software that needs the Ack during unmask. Fixes: 4b7618fdc7e6 ("pinctrl: qcom: Add irq_enable callback for msm gpio") Signed-off-by: Douglas Anderson Reviewed-by: Maulik Shah Tested-by: Maulik Shah Reviewed-by: Stephen Boyd Reviewed-by: Bjorn Andersson --- It should be noted that this code will be moved in the next patch. In theory this could be squashed into the next patch but it seems more documenting to have this as a separate patch. Changes in v5: - ("pinctrl: qcom: Properly clear "intr_ack_high" interrupts...") new for v5. drivers/pinctrl/qcom/pinctrl-msm.c | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c index 1787ada6bfab..a6b0c17e2f78 100644 --- a/drivers/pinctrl/qcom/pinctrl-msm.c +++ b/drivers/pinctrl/qcom/pinctrl-msm.c @@ -96,6 +96,14 @@ MSM_ACCESSOR(intr_cfg) MSM_ACCESSOR(intr_status) MSM_ACCESSOR(intr_target) +static void msm_ack_intr_status(struct msm_pinctrl *pctrl, + const struct msm_pingroup *g) +{ + u32 val = (g->intr_ack_high) ? BIT(g->intr_status_bit) : 0; + + msm_writel_intr_status(val, pctrl, g); +} + static int msm_get_groups_count(struct pinctrl_dev *pctldev) { struct msm_pinctrl *pctrl = pinctrl_dev_get_drvdata(pctldev); @@ -798,7 +806,7 @@ static void msm_gpio_irq_clear_unmask(struct irq_data *d, bool status_clear) * when the interrupt is not in use. */ if (status_clear) - msm_writel_intr_status(0, pctrl, g); + msm_ack_intr_status(pctrl, g); val = msm_readl_intr_cfg(pctrl, g); val |= BIT(g->intr_raw_status_bit); @@ -891,7 +899,6 @@ static void msm_gpio_irq_ack(struct irq_data *d) struct msm_pinctrl *pctrl = gpiochip_get_data(gc); const struct msm_pingroup *g; unsigned long flags; - u32 val; if (test_bit(d->hwirq, pctrl->skip_wake_irqs)) { if (test_bit(d->hwirq, pctrl->dual_edge_irqs)) @@ -903,8 +910,7 @@ static void msm_gpio_irq_ack(struct irq_data *d) raw_spin_lock_irqsave(&pctrl->lock, flags); - val = (g->intr_ack_high) ? BIT(g->intr_status_bit) : 0; - msm_writel_intr_status(val, pctrl, g); + msm_ack_intr_status(pctrl, g); if (test_bit(d->hwirq, pctrl->dual_edge_irqs)) msm_gpio_update_dual_edge_pos(pctrl, g, d); From patchwork Fri Jan 8 17:35:16 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Doug Anderson X-Patchwork-Id: 12007109 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=-19.0 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 5A5BCC4332B for ; Fri, 8 Jan 2021 17:37:24 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 392F723A00 for ; Fri, 8 Jan 2021 17:37:24 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728520AbhAHRhI (ORCPT ); Fri, 8 Jan 2021 12:37:08 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55802 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728519AbhAHRhH (ORCPT ); Fri, 8 Jan 2021 12:37:07 -0500 Received: from mail-pg1-x534.google.com (mail-pg1-x534.google.com [IPv6:2607:f8b0:4864:20::534]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 7E085C06129F for ; Fri, 8 Jan 2021 09:35:52 -0800 (PST) Received: by mail-pg1-x534.google.com with SMTP id n10so8029275pgl.10 for ; Fri, 08 Jan 2021 09:35:52 -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=qJxj/FpYvZvPaV8YwpHtvsP0jgiqVzfBNAujDbm4i4o=; b=a7B4zVipAI5Ae8bsdl35Y+6rsvVJiVp8OEo1X7LA2PbnvrWHB1oWgpwBbnl0ylAUox vQTiqBlt8MvVlFZdYJzztwmcg6Fd5ICtmdXja1gE5eOy7vuGu0kHd/tKt74fV+pnf6TD Spn6YKy0vxtZN/0sF5CSu+Z+HqHAP8yCZ/HEM= 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=qJxj/FpYvZvPaV8YwpHtvsP0jgiqVzfBNAujDbm4i4o=; b=jVmUjRRG4902AfVJqSp91gg8m3NkiJmlZO0UR44Vlws8yAkovJwXKulSzu/6W3qR4N Kd1GLGTHizR2hKNaQdcByw93GvhubFSHWxlBJ0l3qObFZo7KNfgdQP+JZTMziCJB5Ze5 SU3x09ko1D04M6j8YUIFiXSjEghS4qGQKz4hQzagW9QhFdErfpo+gdM/UYbaYkopiRsW LCiRJpnNUGdSscvUQlomvPzzByaDmJ9m3iMpyo6ZBCDN4n0XyCLeFQ5K84ahBpvXyKZe 5SncblU9uILAYuQ7I5DE/IKlMX65iq9isLHkuGQwFjiQ88dFWxJt63N2Y5ysI2sn7ueP jm+w== X-Gm-Message-State: AOAM530t1Wh3wi6T68XACuCUjALmFqYIdUqy+CFVRfqXelPUul9AmfaO Gv1iJSBIMYeVGanNeVlktMvNug== X-Google-Smtp-Source: ABdhPJyQbZ60izOyrJ4ubXVQ1sf18D8u+mdyz7KI+Q08P0rftjV+C2OBDimYuM2M5vChzxZDu4e0zg== X-Received: by 2002:a63:ce42:: with SMTP id r2mr8161235pgi.8.1610127351916; Fri, 08 Jan 2021 09:35:51 -0800 (PST) Received: from tictac2.mtv.corp.google.com ([2620:15c:202:1:42b0:34ff:fe3d:58e6]) by smtp.gmail.com with ESMTPSA id z23sm10245619pfj.143.2021.01.08.09.35.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 08 Jan 2021 09:35:51 -0800 (PST) From: Douglas Anderson To: Marc Zyngier , Thomas Gleixner , Jason Cooper , Linus Walleij Cc: Bjorn Andersson , Neeraj Upadhyay , Rajendra Nayak , Stephen Boyd , Maulik Shah , linux-gpio@vger.kernel.org, Srinivas Ramana , linux-arm-msm@vger.kernel.org, Douglas Anderson , Andy Gross , linux-kernel@vger.kernel.org Subject: [PATCH v5 4/4] pinctrl: qcom: Don't clear pending interrupts when enabling Date: Fri, 8 Jan 2021 09:35:16 -0800 Message-Id: <20210108093339.v5.4.I7cf3019783720feb57b958c95c2b684940264cd1@changeid> X-Mailer: git-send-email 2.29.2.729.g45daf8777d-goog In-Reply-To: <20210108093339.v5.1.I3ad184e3423d8e479bc3e86f5b393abb1704a1d1@changeid> References: <20210108093339.v5.1.I3ad184e3423d8e479bc3e86f5b393abb1704a1d1@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 and it had problems (it caused sc7180-trogdor devices to fail to suspend). Let's fix. From my understanding the source of the phantom interrupt in the were these two things: 1. One that could have been introduced in msm_gpio_irq_set_type() (only for the non-PDC case). 2. Edges could have been detected when a GPIO was muxed away. Fixing case #1 is easy. We can just add a clear in msm_gpio_irq_set_type(). Fixing case #2 is harder. Let's use a concrete example. 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 (or the non-PDC interrupt detection logic) 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 interrupt. This isn't necessary in the above case since the client already masked us, but it's a good idea in general. * When we mux back will clear any interrupts and unmask. Fixes: 4b7618fdc7e6 ("pinctrl: qcom: Add irq_enable callback for msm gpio") Fixes: 71266d9d3936 ("pinctrl: qcom: Move clearing pending IRQ to .irq_request_resources callback") Signed-off-by: Douglas Anderson Reviewed-by: Maulik Shah Tested-by: Maulik Shah --- Note that patch #1 of v4 has now landed so it's dropped from the v5 post. Also note that there is no dependency of this series on the v4 patch #1 so no special coordination need happen for that. I didn't add Rajendra's and Stephen's tags from v4 since there were enough changes from v4 to v5 to warrant a re-review. My tests here for the non-PDC case are mostly synthetic and I don't have any good way to test the case that the original patch was added for. Hopefully it's all good? Changes in v5: - Re-combined PDC and non-PDC since non-PDC their issues are similar. - "it" => "the interrupt" in comment. - Handle 2nd case where edges came when muxed away. - Handle controllers where you write 1 to Ack. Changes in v4: - Split non-PDC fix and PDC fix in two. - Totally rewrote again with my new understanding of the world. 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 | 74 +++++++++++++++++++----------- 1 file changed, 46 insertions(+), 28 deletions(-) diff --git a/drivers/pinctrl/qcom/pinctrl-msm.c b/drivers/pinctrl/qcom/pinctrl-msm.c index a6b0c17e2f78..d5d1f3430c6c 100644 --- a/drivers/pinctrl/qcom/pinctrl-msm.c +++ b/drivers/pinctrl/qcom/pinctrl-msm.c @@ -51,6 +51,7 @@ * @dual_edge_irqs: Bitmap of irqs that need sw emulated dual edge * detection. * @skip_wake_irqs: Skip IRQs that are handled by wakeup interrupt controller + * @disabled_for_mux: These IRQs were disabled because we muxed away. * @soc: Reference to soc_data of platform specific data. * @regs: Base addresses for the TLMM tiles. * @phys_base: Physical base address @@ -72,6 +73,7 @@ struct msm_pinctrl { DECLARE_BITMAP(dual_edge_irqs, MAX_NR_GPIO); DECLARE_BITMAP(enabled_irqs, MAX_NR_GPIO); DECLARE_BITMAP(skip_wake_irqs, MAX_NR_GPIO); + DECLARE_BITMAP(disabled_for_mux, MAX_NR_GPIO); const struct msm_pinctrl_soc_data *soc; void __iomem *regs[MAX_NR_TILES]; @@ -179,6 +181,10 @@ 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; unsigned long flags; u32 val, mask; @@ -195,6 +201,20 @@ 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 then we need special + * handling. Specifically interrupt detection logic will still see + * the pin twiddle even when we're muxed away. + * + * When we see a pin with an interrupt setup on it then we'll disable + * (mask) interrupts on it when we mux away until we mux back. Note + * that disable_irq() refcounts and interrupts are disabled as long as + * at least one disable_irq() has been called. + */ + if (d && i != gpio_func && + !test_and_set_bit(d->hwirq, pctrl->disabled_for_mux)) + disable_irq(irq); + raw_spin_lock_irqsave(&pctrl->lock, flags); val = msm_readl_ctl(pctrl, g); @@ -204,6 +224,20 @@ static int msm_pinmux_set_mux(struct pinctrl_dev *pctldev, raw_spin_unlock_irqrestore(&pctrl->lock, flags); + if (d && i == gpio_func && + test_and_clear_bit(d->hwirq, pctrl->disabled_for_mux)) { + /* + * Clear interrupts detected while not GPIO since we only + * masked things. + */ + if (d->parent_data && test_bit(d->hwirq, pctrl->skip_wake_irqs)) + irq_chip_set_parent_state(d, IRQCHIP_STATE_PENDING, false); + else + msm_ack_intr_status(pctrl, g); + + enable_irq(irq); + } + return 0; } @@ -782,7 +816,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); @@ -800,14 +834,6 @@ static void msm_gpio_irq_clear_unmask(struct irq_data *d, bool status_clear) raw_spin_lock_irqsave(&pctrl->lock, flags); - /* - * clear the interrupt status bit before unmask to avoid - * any erroneous interrupts that would have got latched - * when the interrupt is not in use. - */ - if (status_clear) - msm_ack_intr_status(pctrl, g); - val = msm_readl_intr_cfg(pctrl, g); val |= BIT(g->intr_raw_status_bit); val |= BIT(g->intr_enable_bit); @@ -827,7 +853,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) @@ -842,11 +868,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. @@ -935,6 +956,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)) { @@ -996,6 +1018,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); @@ -1045,6 +1068,14 @@ 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 the interrupt. This is safe because we have + * IRQCHIP_SET_TYPE_MASKED. + */ + if (!was_enabled) + msm_ack_intr_status(pctrl, g); + if (test_bit(d->hwirq, pctrl->dual_edge_irqs)) msm_gpio_update_dual_edge_pos(pctrl, g, d); @@ -1096,19 +1127,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);