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=-9.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_PATCH,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 2F1D2C433DB for ; Fri, 12 Mar 2021 09:16:44 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (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 74CCA64FD0 for ; Fri, 12 Mar 2021 09:16:43 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 74CCA64FD0 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+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=desiato.20200630; 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:Subject:Cc:To: From:Message-ID:Date:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=rhzrcwA4iG5GuXpRBxIAOKGTzkNmFV3LFTiy6IFboXg=; b=kgNRt7i5xXhFOrXgdR6RGEDAn 9XCOPWVWoaV6E68rk9197KnXlUDpNorDAoJfyTy1GW8x4KEoQFIWlytEDhMiG11009nWCWAxq14KL jeXGIRIJG+qKffyJHuXpAkZRn6EujhzhHIAKFtZWNOQFqeC7lrfQ5LYWl+43rHKad8n7CzYYEVuuE oX4UbAS6SYe7BGzTfJxYLEDNVdpX1XLE8hoj+lmF2idMAOKvDtAbG+FIUNOmP60ayf85JO7dTJ65O A3gRSiGK0LoDcXxXem1srAvxeJPT/qAGh2U1FaXbDLiWcFezwCWbFsYO9TtSytQ5LZNL60iVDI7sZ Bhc1zfTOQ==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lKdst-00Ayqm-3X; Fri, 12 Mar 2021 09:15:03 +0000 Received: from mail.kernel.org ([198.145.29.99]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lKdsk-00AypV-F2; Fri, 12 Mar 2021 09:14:57 +0000 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 9B67B64FD6; Fri, 12 Mar 2021 09:14:52 +0000 (UTC) Received: from 78.163-31-62.static.virginmediabusiness.co.uk ([62.31.163.78] helo=why.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94) (envelope-from ) id 1lKdsg-001Ajr-NA; Fri, 12 Mar 2021 09:14:50 +0000 Date: Fri, 12 Mar 2021 09:14:49 +0000 Message-ID: <87im5wg2d2.wl-maz@kernel.org> From: Marc Zyngier To: Jianjun Wang Cc: Bjorn Helgaas , Rob Herring , Lorenzo Pieralisi , Ryder Lee , Philipp Zabel , "Matthias\ Brugger" , , , , , , "Sj\ Huang" , , , , , , , Subject: Re: [v8,5/7] PCI: mediatek-gen3: Add MSI support In-Reply-To: <1615456065.25662.60.camel@mhfsdcap03> References: <20210224061132.26526-1-jianjun.wang@mediatek.com> <20210224061132.26526-6-jianjun.wang@mediatek.com> <87pn08y3ja.wl-maz@kernel.org> <1615358929.25662.47.camel@mhfsdcap03> <87a6rbxs4w.wl-maz@kernel.org> <1615456065.25662.60.camel@mhfsdcap03> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/27.1 (x86_64-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") X-SA-Exim-Connect-IP: 62.31.163.78 X-SA-Exim-Rcpt-To: jianjun.wang@mediatek.com, bhelgaas@google.com, robh+dt@kernel.org, lorenzo.pieralisi@arm.com, ryder.lee@mediatek.com, p.zabel@pengutronix.de, matthias.bgg@gmail.com, linux-pci@vger.kernel.org, linux-mediatek@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, sj.huang@mediatek.com, youlin.pei@mediatek.com, chuanjia.liu@mediatek.com, qizhong.cheng@mediatek.com, sin_jieyang@mediatek.com, drinkcat@chromium.org, Rex-BC.Chen@mediatek.com, anson.chuang@mediatek.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210312_091455_210792_EC2DB01E X-CRM114-Status: GOOD ( 34.04 ) 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 Thu, 11 Mar 2021 09:47:45 +0000, Jianjun Wang wrote: > > On Wed, 2021-03-10 at 09:41 +0000, Marc Zyngier wrote: > > On Wed, 10 Mar 2021 06:48:49 +0000, > > Jianjun Wang wrote: > > > > > @@ -408,6 +677,14 @@ static void mtk_pcie_irq_handler(struct irq_desc *desc) > > > > > generic_handle_irq(virq); > > > > > } > > > > > > > > > > + irq_bit = PCIE_MSI_SHIFT; > > > > > + for_each_set_bit_from(irq_bit, &status, PCIE_MSI_SET_NUM + > > > > > + PCIE_MSI_SHIFT) { > > > > > + mtk_pcie_msi_handler(port, irq_bit - PCIE_MSI_SHIFT); > > > > > + > > > > > + writel_relaxed(BIT(irq_bit), port->base + PCIE_INT_STATUS_REG); > > > > > > > > Isn't this write the same thing you have for EOI in the INTx case? > > > > While I could understand your description in that case (this is a > > > > resampling operation), I don't get what this does here. Either this is > > > > also an EOI, but your initial description doesn't make sense, or it is > > > > an Ack, and it should be moved to the right place. > > > > > > > > Which one is it? > > > > > > I think it should be an EOI which used to clear the interrupt status of > > > a single set in the PCIe intc field, maybe I should move it to the end > > > of the mtk_pcie_msi_handler() function. > > > > I doubt this is an EOI. If, as I suspect, it instructs the HW to clear > > the bit so that new pending bits can be recorded, it must take place > > *before* the interrupt is handled, or you may lose MSIs in the > > interval between the handling of the interrupt and the clearing of the > > pending bit. To satisfy this requirement, this should be an ACK, which > > is consistent with the way most MSI controllers such as this one work. > > These bits are similar with the interrupt status of INTx, and the > interrupt status will remain until all the status of the corresponding > set are cleared. There is a while loop in mtk_pcie_msi_handler() which > is used to continuously polling and ACK the status of the MSI set, I > think the MSI may not be lose in this case. Ah, is that the write to PCIE_MSI_SET_STATUS_OFFSET that you are referring to? In that case, yes, I agree. However, this write to PCIE_INT_STATUS_REG is more a property of the mux interrupt and not one of the MSI interrupt. Given that you do not represent that level as another level of chained controller, you might as well leave it where it is... Thanks, M. -- Without deviation from the norm, progress is not possible. _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel