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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6DC90C433FE for ; Fri, 11 Feb 2022 18:21:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1343636AbiBKSVr (ORCPT ); Fri, 11 Feb 2022 13:21:47 -0500 Received: from mxb-00190b01.gslb.pphosted.com ([23.128.96.19]:60804 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241157AbiBKSVq (ORCPT ); Fri, 11 Feb 2022 13:21:46 -0500 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 71E7113A; Fri, 11 Feb 2022 10:21:44 -0800 (PST) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 1B6911042; Fri, 11 Feb 2022 10:21:44 -0800 (PST) Received: from lpieralisi (e121166-lin.cambridge.arm.com [10.1.196.255]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B72F33F70D; Fri, 11 Feb 2022 10:21:42 -0800 (PST) Date: Fri, 11 Feb 2022 18:21:37 +0000 From: Lorenzo Pieralisi To: Pali =?iso-8859-1?Q?Roh=E1r?= Cc: robh+dt@kernel.org, Bjorn Helgaas , Thomas Petazzoni , Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , Marek =?iso-8859-1?Q?Beh=FAn?= , Russell King , Marc Zyngier , linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v2 10/11] PCI: mvebu: Implement support for legacy INTx interrupts Message-ID: <20220211182137.GA2492@lpieralisi> References: <20220105150239.9628-1-pali@kernel.org> <20220112151814.24361-1-pali@kernel.org> <20220112151814.24361-11-pali@kernel.org> <20220211171917.GA740@lpieralisi> <20220211175202.gku5pkwn5wmjo5al@pali> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20220211175202.gku5pkwn5wmjo5al@pali> User-Agent: Mutt/1.9.4 (2018-02-28) Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Feb 11, 2022 at 06:52:02PM +0100, Pali Rohár wrote: [...] > > > @@ -1121,6 +1247,21 @@ static int mvebu_pcie_parse_port(struct mvebu_pcie *pcie, > > > port->io_attr = -1; > > > } > > > > > > + /* > > > + * Old DT bindings do not contain "intx" interrupt > > > + * so do not fail probing driver when interrupt does not exist. > > > + */ > > > + port->intx_irq = of_irq_get_byname(child, "intx"); > > > + if (port->intx_irq == -EPROBE_DEFER) { > > > + ret = port->intx_irq; > > > + goto err; > > > + } > > > + if (port->intx_irq <= 0) { > > > + dev_warn(dev, "%s: legacy INTx interrupts cannot be masked individually, " > > > + "%pOF does not contain intx interrupt\n", > > > + port->name, child); > > > > Here you end up with a new warning on existing firmware. Is it > > legitimate ? I would remove the dev_warn(). > > I added this warning in v2 because Marc wanted it. > > Should I (again) remove it in v3? No, I asked a question and gave an opinion, I appreciate Marc's concern so leave it (ie not everyone running a new kernel with new warnings on existing firmware would be happy - maybe it is a good way of forcing a firmware upgrade, you will tell me). Lorenzo > > Rob certainly has more insightful advice on this. > > > > Thanks, > > Lorenzo > > > > > + } > > > + > > > reset_gpio = of_get_named_gpio_flags(child, "reset-gpios", 0, &flags); > > > if (reset_gpio == -EPROBE_DEFER) { > > > ret = reset_gpio; > > > @@ -1317,6 +1458,7 @@ static int mvebu_pcie_probe(struct platform_device *pdev) > > > > > > for (i = 0; i < pcie->nports; i++) { > > > struct mvebu_pcie_port *port = &pcie->ports[i]; > > > + int irq = port->intx_irq; > > > > > > child = port->dn; > > > if (!child) > > > @@ -1344,6 +1486,22 @@ static int mvebu_pcie_probe(struct platform_device *pdev) > > > continue; > > > } > > > > > > + if (irq > 0) { > > > + ret = mvebu_pcie_init_irq_domain(port); > > > + if (ret) { > > > + dev_err(dev, "%s: cannot init irq domain\n", > > > + port->name); > > > + pci_bridge_emul_cleanup(&port->bridge); > > > + devm_iounmap(dev, port->base); > > > + port->base = NULL; > > > + mvebu_pcie_powerdown(port); > > > + continue; > > > + } > > > + irq_set_chained_handler_and_data(irq, > > > + mvebu_pcie_irq_handler, > > > + port); > > > + } > > > + > > > /* > > > * PCIe topology exported by mvebu hw is quite complicated. In > > > * reality has something like N fully independent host bridges > > > @@ -1448,6 +1606,7 @@ static int mvebu_pcie_remove(struct platform_device *pdev) > > > > > > for (i = 0; i < pcie->nports; i++) { > > > struct mvebu_pcie_port *port = &pcie->ports[i]; > > > + int irq = port->intx_irq; > > > > > > if (!port->base) > > > continue; > > > @@ -1458,7 +1617,17 @@ static int mvebu_pcie_remove(struct platform_device *pdev) > > > mvebu_writel(port, cmd, PCIE_CMD_OFF); > > > > > > /* Mask all interrupt sources. */ > > > - mvebu_writel(port, 0, PCIE_MASK_OFF); > > > + mvebu_writel(port, ~PCIE_INT_ALL_MASK, PCIE_INT_UNMASK_OFF); > > > + > > > + /* Clear all interrupt causes. */ > > > + mvebu_writel(port, ~PCIE_INT_ALL_MASK, PCIE_INT_CAUSE_OFF); > > > + > > > + if (irq > 0) > > > + irq_set_chained_handler_and_data(irq, NULL, NULL); > > > + > > > + /* Remove IRQ domains. */ > > > + if (port->intx_irq_domain) > > > + irq_domain_remove(port->intx_irq_domain); > > > > > > /* Free config space for emulated root bridge. */ > > > pci_bridge_emul_cleanup(&port->bridge); > > > -- > > > 2.20.1 > > > 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 49214C433EF for ; Fri, 11 Feb 2022 18:23:25 +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:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=tIUbgqLHo4XY9cp8yT/M2bFJP+hqlNvTJ3Fx/CGU5EY=; b=rZCDIRNME5LLKk GeUe7Wx9cDwGvZYnyX+50Z1kY7+pAg9inY/OgulVsGLzsaFYylzV3EgYqegbYYppanJ5iRogJXSJE JKoH75OtQr5oVC6O8O/hz1ncdCg/7dLgYmrJigrmuorw9HPuVrqYBzv+C2aewQYmM4pTlQA09sDiU eIrT6ikROmqcCT554EewHnjzvR8+zO2BS7gfDALHxod0jT1qupLx8wj3aU8cplACEE0Nel26d6Ck1 4xUJsLgp787QS94XQprMy12KH65CkGQQXTTlXk8XfjZPYSjxHDbAUW2dv0p+Rd3pQTF+H5wBonoTq F4CZAUkfWJMW5BpI5zZw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nIaYN-008Q7c-Ge; Fri, 11 Feb 2022 18:21:55 +0000 Received: from foss.arm.com ([217.140.110.172]) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nIaYD-008Q4X-MS for linux-arm-kernel@lists.infradead.org; Fri, 11 Feb 2022 18:21:47 +0000 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 1B6911042; Fri, 11 Feb 2022 10:21:44 -0800 (PST) Received: from lpieralisi (e121166-lin.cambridge.arm.com [10.1.196.255]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id B72F33F70D; Fri, 11 Feb 2022 10:21:42 -0800 (PST) Date: Fri, 11 Feb 2022 18:21:37 +0000 From: Lorenzo Pieralisi To: Pali =?iso-8859-1?Q?Roh=E1r?= Cc: robh+dt@kernel.org, Bjorn Helgaas , Thomas Petazzoni , Krzysztof =?utf-8?Q?Wilczy=C5=84ski?= , Marek =?iso-8859-1?Q?Beh=FAn?= , Russell King , Marc Zyngier , linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v2 10/11] PCI: mvebu: Implement support for legacy INTx interrupts Message-ID: <20220211182137.GA2492@lpieralisi> References: <20220105150239.9628-1-pali@kernel.org> <20220112151814.24361-1-pali@kernel.org> <20220112151814.24361-11-pali@kernel.org> <20220211171917.GA740@lpieralisi> <20220211175202.gku5pkwn5wmjo5al@pali> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220211175202.gku5pkwn5wmjo5al@pali> User-Agent: Mutt/1.9.4 (2018-02-28) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220211_102145_855836_9C0FC92A X-CRM114-Status: GOOD ( 23.14 ) 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="iso-8859-1" Content-Transfer-Encoding: quoted-printable Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Feb 11, 2022 at 06:52:02PM +0100, Pali Roh=E1r wrote: [...] > > > @@ -1121,6 +1247,21 @@ static int mvebu_pcie_parse_port(struct mvebu_= pcie *pcie, > > > port->io_attr =3D -1; > > > } > > > = > > > + /* > > > + * Old DT bindings do not contain "intx" interrupt > > > + * so do not fail probing driver when interrupt does not exist. > > > + */ > > > + port->intx_irq =3D of_irq_get_byname(child, "intx"); > > > + if (port->intx_irq =3D=3D -EPROBE_DEFER) { > > > + ret =3D port->intx_irq; > > > + goto err; > > > + } > > > + if (port->intx_irq <=3D 0) { > > > + dev_warn(dev, "%s: legacy INTx interrupts cannot be masked individ= ually, " > > > + "%pOF does not contain intx interrupt\n", > > > + port->name, child); > > = > > Here you end up with a new warning on existing firmware. Is it > > legitimate ? I would remove the dev_warn(). > = > I added this warning in v2 because Marc wanted it. > = > Should I (again) remove it in v3? No, I asked a question and gave an opinion, I appreciate Marc's concern so leave it (ie not everyone running a new kernel with new warnings on existing firmware would be happy - maybe it is a good way of forcing a firmware upgrade, you will tell me). Lorenzo > > Rob certainly has more insightful advice on this. > > = > > Thanks, > > Lorenzo > > = > > > + } > > > + > > > reset_gpio =3D of_get_named_gpio_flags(child, "reset-gpios", 0, &fl= ags); > > > if (reset_gpio =3D=3D -EPROBE_DEFER) { > > > ret =3D reset_gpio; > > > @@ -1317,6 +1458,7 @@ static int mvebu_pcie_probe(struct platform_dev= ice *pdev) > > > = > > > for (i =3D 0; i < pcie->nports; i++) { > > > struct mvebu_pcie_port *port =3D &pcie->ports[i]; > > > + int irq =3D port->intx_irq; > > > = > > > child =3D port->dn; > > > if (!child) > > > @@ -1344,6 +1486,22 @@ static int mvebu_pcie_probe(struct platform_de= vice *pdev) > > > continue; > > > } > > > = > > > + if (irq > 0) { > > > + ret =3D mvebu_pcie_init_irq_domain(port); > > > + if (ret) { > > > + dev_err(dev, "%s: cannot init irq domain\n", > > > + port->name); > > > + pci_bridge_emul_cleanup(&port->bridge); > > > + devm_iounmap(dev, port->base); > > > + port->base =3D NULL; > > > + mvebu_pcie_powerdown(port); > > > + continue; > > > + } > > > + irq_set_chained_handler_and_data(irq, > > > + mvebu_pcie_irq_handler, > > > + port); > > > + } > > > + > > > /* > > > * PCIe topology exported by mvebu hw is quite complicated. In > > > * reality has something like N fully independent host bridges > > > @@ -1448,6 +1606,7 @@ static int mvebu_pcie_remove(struct platform_de= vice *pdev) > > > = > > > for (i =3D 0; i < pcie->nports; i++) { > > > struct mvebu_pcie_port *port =3D &pcie->ports[i]; > > > + int irq =3D port->intx_irq; > > > = > > > if (!port->base) > > > continue; > > > @@ -1458,7 +1617,17 @@ static int mvebu_pcie_remove(struct platform_d= evice *pdev) > > > mvebu_writel(port, cmd, PCIE_CMD_OFF); > > > = > > > /* Mask all interrupt sources. */ > > > - mvebu_writel(port, 0, PCIE_MASK_OFF); > > > + mvebu_writel(port, ~PCIE_INT_ALL_MASK, PCIE_INT_UNMASK_OFF); > > > + > > > + /* Clear all interrupt causes. */ > > > + mvebu_writel(port, ~PCIE_INT_ALL_MASK, PCIE_INT_CAUSE_OFF); > > > + > > > + if (irq > 0) > > > + irq_set_chained_handler_and_data(irq, NULL, NULL); > > > + > > > + /* Remove IRQ domains. */ > > > + if (port->intx_irq_domain) > > > + irq_domain_remove(port->intx_irq_domain); > > > = > > > /* Free config space for emulated root bridge. */ > > > pci_bridge_emul_cleanup(&port->bridge); > > > -- = > > > 2.20.1 > > > = _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel