From mboxrd@z Thu Jan 1 00:00:00 1970 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=-11.0 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_CR_TRAILER,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED 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 02F0DC433E6 for ; Fri, 15 Jan 2021 03:20:07 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id AD04723AFE for ; Fri, 15 Jan 2021 03:20:06 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1732459AbhAODUG (ORCPT ); Thu, 14 Jan 2021 22:20:06 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33300 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727134AbhAODUF (ORCPT ); Thu, 14 Jan 2021 22:20:05 -0500 Received: from mail-pj1-x1029.google.com (mail-pj1-x1029.google.com [IPv6:2607:f8b0:4864:20::1029]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0CAB5C061575 for ; Thu, 14 Jan 2021 19:19:25 -0800 (PST) Received: by mail-pj1-x1029.google.com with SMTP id l23so4400028pjg.1 for ; Thu, 14 Jan 2021 19:19:25 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:content-transfer-encoding:in-reply-to:references :subject:from:cc:to:date:message-id:user-agent; bh=x4FBc9O82mLwuBIYHsjWOjbvAlD6yiol0Z+T88PC/zQ=; b=Qyp8gRZvW/3Drd2bA1Ib8XVZ398D3Q7q2jBLpx01Adj3VlJ1xbADdf/RwhTa528vag Rm482dObSn+/simqn8sm+BQRcLViFlbSn4FRPdIEzg/WTFaHjlKJRtDvJaz9VXs/wDRT 5xU7cWOpnbXwxs9ArDuvkbV7n7oUcJLEpgsbA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:content-transfer-encoding :in-reply-to:references:subject:from:cc:to:date:message-id :user-agent; bh=x4FBc9O82mLwuBIYHsjWOjbvAlD6yiol0Z+T88PC/zQ=; b=q6xHWOPLgibFnjX27xaOJIiQNDJZQf2j/900fVc8e9HzKq9JPv/PO6KsEQ2IB8xvgS EgsUbkRDj0taqXqMeIax6gCEI28pUwP0suloaOdBVvKYEEr++soVfv2Y5Soc42s794h9 n5tEmMX/taMl6ZYa+XixCKH1QREc9LrdWMQ9Jo2OCK8FBqJRKT5wmn+8lYmXnNIUd5ge esxFtDf76R+85bMal4it8LVIKyuq6bjr58VRM6F00eI4dBNhiU/B5h1bEpBtuXdRA/FE NtTUkQy6BKZO4QPfkm0tHHFnGji6yAZ6fTYQhLZsqKvZXPXsQeFp/MR7ShlklFhXZwja VBBg== X-Gm-Message-State: AOAM532fP78Ki677o4wEy6l2XS1phD0lYmFRpzONktEZyiMJb2dkNRbZ hbhLW08pUmVSXaiwjBTpXwVqyA== X-Google-Smtp-Source: ABdhPJxJ/FjQHhgUkexO4OnMOFbf52AYYclIrUodYUPvwiumxigQiNUSzUyqXpXhHavHTlVUgsayTg== X-Received: by 2002:a17:90a:77c1:: with SMTP id e1mr8457773pjs.141.1610680764632; Thu, 14 Jan 2021 19:19:24 -0800 (PST) Received: from chromium.org ([2620:15c:202:201:3e52:82ff:fe6c:83ab]) by smtp.gmail.com with ESMTPSA id w19sm6432582pgf.23.2021.01.14.19.19.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 14 Jan 2021 19:19:24 -0800 (PST) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable In-Reply-To: <20210114191601.v7.4.I7cf3019783720feb57b958c95c2b684940264cd1@changeid> References: <20210114191601.v7.1.I3ad184e3423d8e479bc3e86f5b393abb1704a1d1@changeid> <20210114191601.v7.4.I7cf3019783720feb57b958c95c2b684940264cd1@changeid> Subject: Re: [PATCH v7 4/4] pinctrl: qcom: Don't clear pending interrupts when enabling From: Stephen Boyd Cc: Neeraj Upadhyay , linux-arm-msm@vger.kernel.org, linux-gpio@vger.kernel.org, Bjorn Andersson , Rajendra Nayak , Srinivas Ramana , Maulik Shah , Douglas Anderson , Andy Gross , linux-kernel@vger.kernel.org To: Douglas Anderson , Jason Cooper , Linus Walleij , Marc Zyngier , Thomas Gleixner Date: Thu, 14 Jan 2021 19:19:22 -0800 Message-ID: <161068076244.3661239.337771722271707457@swboyd.mtv.corp.google.com> User-Agent: alot/0.9.1 Precedence: bulk List-ID: X-Mailing-List: linux-arm-msm@vger.kernel.org Quoting Douglas Anderson (2021-01-14 19:16:24) > 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. >=20 > 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. >=20 > 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(). >=20 > 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. >=20 > 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. >=20 > 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. >=20 > Fixing case #1 is easy. We can just add a clear in > msm_gpio_irq_set_type(). >=20 > 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. >=20 > 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. >=20 > 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. >=20 > Fixes: 4b7618fdc7e6 ("pinctrl: qcom: Add irq_enable callback for msm gpio= ") > Fixes: 71266d9d3936 ("pinctrl: qcom: Move clearing pending IRQ to .irq_re= quest_resources callback") > Signed-off-by: Douglas Anderson > Reviewed-by: Maulik Shah > Tested-by: Maulik Shah > --- Reviewed-by: Stephen Boyd