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 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 smtp.lore.kernel.org (Postfix) with ESMTPS id 5C6F1C28CF5 for ; Wed, 26 Jan 2022 03:39:43 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:MIME-Version:References:In-Reply-To: Date:CC:To:From:Subject:Message-ID:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=dT5mtxl3QcOImzQEXenXFdOnv5pQMKMOzAO0Vscw16o=; b=eHgHuY2Dgaa0j+ 6Hz/BR3clMJERbK3lkMj5z9xscywJq+VtkDhkZLHpASLVm5ppYaGDLUFxesQvcKnTAE2Cjns+VN3p M6IMa5cqiqpd0DtKbFqENCHCbZwcrhgLqgru5CfXK6wIUwZUFkeCMJrfanxzpR74X00BTZ7Q6yZRB ycSb4wAhDTgKTXWIY1lnRuIvMBtqwuU10+eWMHMsuyDMKSDHotkmX8qNunNeaqQjG02z+EeJiIdke GpwDnd2Nvh3TStZQgrkUTLZNILJd3+yFdjwiBFlWl4DX6Gk+cKtGThgMevrGRmLFFKkOm3N1qw/xQ SYDhajxMqDZ29sTIWAnQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nCZ8P-00A4EB-5e; Wed, 26 Jan 2022 03:38:13 +0000 Received: from mailgw02.mediatek.com ([216.200.240.185]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nCZ8J-00A4DL-1g; Wed, 26 Jan 2022 03:38:09 +0000 X-UUID: 49ba5485584e4e309c2c0f55437522b6-20220125 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=mediatek.com; s=dk; h=Content-Transfer-Encoding:MIME-Version:Content-Type:References:In-Reply-To:Date:CC:To:From:Subject:Message-ID; bh=a/gBlUU1hsQb2mCfMYxlytCU6fL3X0VehMaAcBYrxY4=; b=S0swC8UBGVArrCpgwY5mYYQ0Tmn9kn4wwVKd0FiobQ+hcHBAFl8GBiJ/9QdPrTZ/rNNeomBfLT7wnyUzkTyiQUEf9npbb0UP4UkQGtMWbGLdKbyApTGVBwpCuN/Rzn7mR/6KXYive/opGT0lsLHB8JFsfRwBENjf9POQawQQk9Q=; X-UUID: 49ba5485584e4e309c2c0f55437522b6-20220125 Received: from mtkcas67.mediatek.inc [(172.29.193.45)] by mailgw02.mediatek.com (envelope-from ) (musrelay.mediatek.com ESMTP with TLSv1.2 ECDHE-RSA-AES256-SHA384 256/256) with ESMTP id 541032328; Tue, 25 Jan 2022 20:38:02 -0700 Received: from mtkmbs10n2.mediatek.inc (172.21.101.183) by MTKMBS62DR.mediatek.inc (172.29.94.18) with Microsoft SMTP Server (TLS) id 15.0.1497.2; Tue, 25 Jan 2022 19:38:01 -0800 Received: from mtkcas10.mediatek.inc (172.21.101.39) by mtkmbs10n2.mediatek.inc (172.21.101.183) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA384) id 15.2.792.3; Wed, 26 Jan 2022 11:37:59 +0800 Received: from mhfsdcap04 (10.17.3.154) by mtkcas10.mediatek.inc (172.21.101.73) with Microsoft SMTP Server id 15.0.1497.2 via Frontend Transport; Wed, 26 Jan 2022 11:37:58 +0800 Message-ID: Subject: Re: [PATCH] PCI: mediatek: Change MSI interrupt processing sequence From: qizhong.cheng To: Marc Zyngier , Bjorn Helgaas CC: Ryder Lee , Jianjun Wang , Lorenzo Pieralisi , Krzysztof =?UTF-8?Q?Wilczy=C5=84ski?= , Bjorn Helgaas , , , , , Date: Wed, 26 Jan 2022 11:37:58 +0800 In-Reply-To: References: <20220125165748.GA1458116@bhelgaas> X-Mailer: Evolution 3.28.5-0ubuntu0.18.04.2 MIME-Version: 1.0 X-MTK: N X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220125_193808_511275_78CA7F7E X-CRM114-Status: GOOD ( 26.17 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Tue, 2022-01-25 at 17:21 +0000, Marc Zyngier wrote: > On 2022-01-25 16:57, Bjorn Helgaas wrote: > > All patches change *something*. Can you update the subject line so > > it > > says something specific about the change? > > > > Maybe something like "Clear MSI status before dispatching handler"? > > > > On Sun, Jan 23, 2022 at 11:33:06AM +0800, qizhong cheng wrote: > > > As an edge-triggered interrupts, its interrupt status should be > > > cleared > > > before dispatch to the handler of device. > > > > I'm not an IRQ expert, but the reasoning that "we should clear the > > MSI > > interrupt status before dispatching the handler because MSI is an > > edge-triggered interrupt" doesn't seem completely convincing > > because > > your code will now look like this: > > > > /* Clear the INTx */ > > writel(1 << bit, port->base + PCIE_INT_STATUS); > > generic_handle_domain_irq(port->irq_domain, bit - INTX_SHIFT); > > ... > > > > /* Clear MSI interrupt status */ > > writel(MSI_STATUS, port->base + PCIE_INT_STATUS); > > generic_handle_domain_irq(port->inner_domain, bit); > > > > You clear interrupt status before dispatching the handler for > > *both* > > level-triggered INTx interrupts and edge-triggered MSI interrupts. > > > > So it doesn't seem that simply being edge-triggered is the critical > > factor here. > > This is the usual problem with these half-baked implementations. > The signalling to the primary interrupt controller is level, as > they take a multitude of input and (crucially) latch the MSI > edges. Effectively, this is an edge-to-level converter, with > all the problems that this creates. > > By clearing the status *after* the handling, you lose edges that > have been received and coalesced after the read of the status > register. By clearing it *before*, you are acknowledging the > interrupts early, and allowing them to be coalesced independently > of the ones that have been received earlier. > > This is however mostly an educated guess. Someone with access > to the TRM should verify this. > Yes, as Maz said, we save the edge-interrupt status so that it becomes a level-interrupt. This is similar to an edge-to-level converter, so we need to clear it *before*. We found this problem through a lot of experiments and tested this patch. Thanks Helgaas and Maz for your comment. -- Jazz ain't dead, dreams haven't parted with you. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel