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=-3.8 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,T_DKIMWL_WL_HIGH 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 5C050C4321A for ; Mon, 10 Jun 2019 22:26:01 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 2F64F20859 for ; Mon, 10 Jun 2019 22:26:01 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="OZfewmL1"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=chromium.org header.i=@chromium.org header.b="anjqADdv" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 2F64F20859 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Date:To:Subject:From:References: In-Reply-To:MIME-Version:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=KYJktwr2smeWYUZhftA6mTV0X8Nypdx2Vx3XCS003t0=; b=OZfewmL1Hwpul4 SWSzTJ0G3m3p2fDMshHzG2YRL/HcFSZHVTYbwAeJjeAOYo4bsyFquRWoEw0VaTUxMrv3SeoO00k8C i2Vqy6vKg2cyeE76gpz5hSmTv8kqUOT65nRFuOenp5Lt0C9OFyoGwFQH5BXHYr0g6b3+5jEVIkteM bPHq7lrZwwbyByPQ+aph77nhS+Q3Ol6oIWHYCur0aNynOqbWMSsgyI9/yW/MNOPybvoOf63zwh5Fn oaJMLEpJ2hlO9F6ssHwFSYO8KE+Lwd/helFpwElg/DvaNA5sJQufT4Kg0lJkNeIloj+NLRnirzF0b cqdd2PPIiThgUFnt8hdA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92 #3 (Red Hat Linux)) id 1haSjo-0007bn-BJ; Mon, 10 Jun 2019 22:26:00 +0000 Received: from mail-pg1-x542.google.com ([2607:f8b0:4864:20::542]) by bombadil.infradead.org with esmtps (Exim 4.92 #3 (Red Hat Linux)) id 1haSjm-0007aN-0j for linux-arm-kernel@lists.infradead.org; Mon, 10 Jun 2019 22:25:59 +0000 Received: by mail-pg1-x542.google.com with SMTP id s27so5769367pgl.2 for ; Mon, 10 Jun 2019 15:25:55 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=message-id:mime-version:content-transfer-encoding:in-reply-to :references:from:cc:subject:to:user-agent:date; bh=9hBqZJD2Tz09wfOJDUxIvJB/XBCbBflZm3IPa1V/vxE=; b=anjqADdvlUHgxpXTA/xLq/4vsfjEG1pLhPFxuBOoSnhDfQK/DLla5XumwWWyElC6Pc PS/vAaTAZ0X4ENtlyOgXM52+IXMmYlK/mrW38hordPGkJpua0G7qIjeRrGDafXtUX4JO TTshciiU8SjktCILESKUC21nDTZENol6TjYeY= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:mime-version :content-transfer-encoding:in-reply-to:references:from:cc:subject:to :user-agent:date; bh=9hBqZJD2Tz09wfOJDUxIvJB/XBCbBflZm3IPa1V/vxE=; b=MG96N19pjwFTLzqstnUhevEv47t66LpKG2K9OxstisjzQwq7QrTp2Lf9KkPYPRcNTp 1nB5yWPn3C8hFJw1q4x0u1e4pKRVnrreYgZP1joHbxB5JXUMN5hmilb7M9rrd8TvzZsX jDW+rlfxtE0kXHzjMFThGVT5liPsxatVJ+cqLcRqo+LXEhqf9t9yJ+ehGPcFILT+3vMu H63QthJtFjKR3NCPuwbaXQU8DaStHPABy0yEE5L96A/HOUaPHldwGmh1f9LoEbhlGmOP VAiKSU/H+zDa/N3yLjrYd+fV/ITJmD4x7EwtkXPFn39e9lVH/C7sv8RaNnzuDsyPNN/g ok1Q== X-Gm-Message-State: APjAAAXBVhu2lTR+QL0TecrhfARdENQhbOJcKRPyNG8baZ+J69XzhjoI vS3+UKBS/77T/QKKZrCYSgAEGA== X-Google-Smtp-Source: APXvYqywzldrMc5v86IYfA+eXCNafyrhC15q1lAcvIj3lXXvZ4LL1QGrW0wDFfeIOenalH46fBvZdg== X-Received: by 2002:aa7:8219:: with SMTP id k25mr79051964pfi.38.1560205555034; Mon, 10 Jun 2019 15:25:55 -0700 (PDT) Received: from chromium.org ([2620:15c:202:1:fa53:7765:582b:82b9]) by smtp.gmail.com with ESMTPSA id v138sm8863906pfc.15.2019.06.10.15.25.54 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Mon, 10 Jun 2019 15:25:54 -0700 (PDT) Message-ID: <5cfed8f2.1c69fb81.1dcd3.27c1@mx.google.com> MIME-Version: 1.0 In-Reply-To: References: <20190429035515.73611-1-drinkcat@chromium.org> <20190429035515.73611-3-drinkcat@chromium.org> <155778659317.14659.136626364818483852@swboyd.mtv.corp.google.com> <155786487644.14659.17142525593824613967@swboyd.mtv.corp.google.com> From: Stephen Boyd Subject: Re: [PATCH 2/2] pinctrl: mediatek: Update cur_mask in mask/mask ops To: Evan Green , Nicolas Boichat User-Agent: alot/0.8.1 Date: Mon, 10 Jun 2019 15:25:53 -0700 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20190610_152558_078585_3C0B3CFD X-CRM114-Status: GOOD ( 32.39 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Chuanjia Liu , Linus Walleij , Sean Wang , lkml , linux-gpio@vger.kernel.org, "moderated list:ARM/Mediatek SoC support" , Matthias Brugger , Thomas Gleixner , linux-arm Mailing List Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org Quoting Evan Green (2019-05-30 10:12:03) > On Wed, May 15, 2019 at 1:05 AM Nicolas Boichat wrote: > > > > On Wed, May 15, 2019 at 4:14 AM Stephen Boyd wrote: > > > > > We could immediately unmask those lines in the hardware when the > > > set_wake() callback is called. That way the genirq layer can use the > > > driver to do what it wants with the hardware and the driver can make > > > sure that set_wake() will always cause the wakeup interrupt to be > > > delivered to genirq even when software has disabled it. > > > > > > But I think that there might be a problem with how genirq understands > > > the masked state of a line when the wakeup implementation conflates > > > masked state with wakeup armed state. Consider this call-flow: > > > > > > irq masked in hardware, IRQD_IRQ_MASKED is set > > > enable_irq_wake() > > > unmask_irq in hardware > > > IRQD_WAKEUP_ARMED is set > > > > > > handle_level_irq() > > > mask_ack_irq() > > > mask_irq() > > > if (irqd_irq_masked()) -> returns true and skips masking! > > > if (desc->irq_data.chip->irq_ack) > > > ... > > > irq_may_run() > > > irq_pm_check_wakeup() > > > irq_disable() > > > mask_irq() -> does nothing again > > > > > > In the above flow, we never mask the irq because we thought it was > > > already masked when it was disabled, but the irqchip implementation > > > unmasked it to make wakeup work. Maybe we should always mask the irq if > > > wakeup is armed and we're trying to call mask_irq()? Looks hacky. > > > > > > diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c > > > index 51128bea3846..20257d528880 100644 > > > --- a/kernel/irq/chip.c > > > +++ b/kernel/irq/chip.c > > > @@ -411,7 +411,7 @@ static inline void mask_ack_irq(struct irq_desc *desc) > > > > > > void mask_irq(struct irq_desc *desc) > > > { > > > - if (irqd_irq_masked(&desc->irq_data)) > > > + if (!irqd_is_wakeup_armed(&desc->irq_data) && irqd_irq_masked(&desc->irq_data)) > > > return; > > > > > > if (desc->irq_data.chip->irq_mask) { > > > > I'm... really not sure what's the best approach here. But basically, > > yes, if we can find a way to properly handle wake and interrupt > > behaviour for drivers with a single mask, that'd be good. > > IRQCHIP_MASK_ON_SUSPEND only seems to be doing half of the work, since > > it does not cover the disable+wake source case. > > > > Thanks, > > I finally got around to studying this patch. This series seems okay to > me. The underlying problem is really that the hardware IRQ enabled > state is out of sync with what Linux thinks. This happens during > suspend because Linux thinks the irq is disabled, but due to the > hardware constraints on this platform, the interrupt has to be enabled > for it to be a wake source. So the mtk driver re-enables the > interrupt, and then has to find a way to get back in sync with Linux's > IRQ mask state. > > One possible approach is mentioned above by Stephen: stop calling > disable_irq in the cros EC driver. Then both linux and mtk agree the > interrupt is enabled at suspend time. I think this ran into other > problems though, where the EC gets its interrupt but is unable to > silence it because the underlying SPI bus is still suspended. Does this happen? I thought the interrupt would only be delivered once all drivers have resumed from the noirq resume phase. Maybe the SPI controller needs to resume there instead of in the normal resume path and then this isn't a problem. > > The other approach, taken here, is to mask the interrupt when it first > comes in, getting Linux and mtk back in agreement that yes, the > interrupt is masked. Outside of enlightening the generic IRQ core > about these types of interrupts that need to get re-enabled to be wake > sources, this seems like a reasonable approach. I prefer we teach the genirq layer about these types of irqchips. That way the solution is common and not a one-off fix for Mediatek chips. I'm sure that this same problem will come up again for another SoC vendor out there so having a chip flag that does the other half of IRQCHIP_MASK_ON_SUSPEND would be appropriate. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel