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 6428FC3DA49 for ; Thu, 25 Jul 2024 05:54:28 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:CC:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=mXvRUEWdVcaZN+h7WS/2ZfLarQXHOiOFcVf6Y4xsMmk=; b=VaR8njBbGKqHc/e0okTO1BQiL0 2dnsgv7jUR7+g+BIc58qi9vzi5RtX5hoIoHVKjMMemOv/yPX2pwD5O29p2miuNEGUEcFxJewlsJZD HrfpodR4J2HnUR4WuIW1b1h6TskxT8NUvVTtUEsMtvWsy5bb295e3CLcrnHzryHKu8ubDPMYVWksf uQLol36TVsCW1Dz0TCivixX7oFGZg/qrzi+l0sT34MDFAFeLCS36JyFLMHRwZPrldCUKmJ3oKYJy4 Krl4qzsuuNmEKx7oR6WDbMFMEVK1F6rtfzmuQtEVjrC444O8xHCd1xVWYqT9x7Hh3YCcXS1BwZlqC L0yoTLmw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sWrQg-0000000HVkY-0240; Thu, 25 Jul 2024 05:54:18 +0000 Received: from fllv0015.ext.ti.com ([198.47.19.141]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sWrQH-0000000HVhJ-085G for linux-arm-kernel@lists.infradead.org; Thu, 25 Jul 2024 05:53:55 +0000 Received: from fllv0035.itg.ti.com ([10.64.41.0]) by fllv0015.ext.ti.com (8.15.2/8.15.2) with ESMTP id 46P5rjpS103866; Thu, 25 Jul 2024 00:53:45 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ti.com; s=ti-com-17Q1; t=1721886825; bh=mXvRUEWdVcaZN+h7WS/2ZfLarQXHOiOFcVf6Y4xsMmk=; h=Date:From:To:CC:Subject:References:In-Reply-To; b=uDUDqTEFebvRL+mGuS7XzdeAdDfZjK7FyzbbotNDJOjJSFJ/9UHI/GUUQFG3wEEzE oRf2Nk/luJL73FkeBw1SKjiI7CwR4B07idkRCJHhz8alcQo5eSI6pF1QlTRlurO4K0 DQXFiuVx1MXjC4EZENZl+d70b4ym52gTNTHEzIhk= Received: from DFLE112.ent.ti.com (dfle112.ent.ti.com [10.64.6.33]) by fllv0035.itg.ti.com (8.15.2/8.15.2) with ESMTPS id 46P5rjaO044137 (version=TLSv1.2 cipher=AES256-GCM-SHA384 bits=256 verify=FAIL); Thu, 25 Jul 2024 00:53:45 -0500 Received: from DFLE114.ent.ti.com (10.64.6.35) by DFLE112.ent.ti.com (10.64.6.33) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.23; Thu, 25 Jul 2024 00:53:45 -0500 Received: from lelvsmtp5.itg.ti.com (10.180.75.250) by DFLE114.ent.ti.com (10.64.6.35) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2507.23 via Frontend Transport; Thu, 25 Jul 2024 00:53:45 -0500 Received: from localhost (uda0492258.dhcp.ti.com [10.24.72.81]) by lelvsmtp5.itg.ti.com (8.15.2/8.15.2) with ESMTP id 46P5rihr080277; Thu, 25 Jul 2024 00:53:44 -0500 Date: Thu, 25 Jul 2024 11:23:43 +0530 From: Siddharth Vadapalli To: Bjorn Helgaas CC: Siddharth Vadapalli , , , , , , , , , , , , Subject: Re: [PATCH] PCI: j721e: Set .map_irq and .swizzle_irq to NULL Message-ID: References: <20240724065048.285838-1-s-vadapalli@ti.com> <20240724162304.GA802428@bhelgaas> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20240724162304.GA802428@bhelgaas> X-EXCLAIMER-MD-CONFIG: e1e8a2fd-e40a-4ac6-ac9b-f7e9cc9ee180 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240724_225353_202079_9951A2F6 X-CRM114-Status: GOOD ( 35.47 ) 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: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Jul 24, 2024 at 11:23:04AM -0500, Bjorn Helgaas wrote: > Subject should say something about why this change is needed, not just > translate the C code to English. My intent was to summarize the change to make it easy for anyone to find out what's being done. The commit message below explains in detail as to why they are set to NULL. As an alternative, I could change the $subject to: PCI: j721e: Disable INTx mapping and swizzling where the "Disable" is equivalent to NULL. Kindly let me know if this is acceptable or needs to be improved further. > > On Wed, Jul 24, 2024 at 12:20:48PM +0530, Siddharth Vadapalli wrote: > > Since the configuration of Legacy Interrupts (INTx) is not supported, set > > I assume you mean J721E doesn't support INTx? Yes, the driver support doesn't exist and the device-tree also doesn't have the required nodes. Kishon had posted a series to enable it for J721E, J7200 and AM64 on the 4th of August 2021: https://lore.kernel.org/r/20210804132912.30685-1-kishon@ti.com/ and based on the discussion on the series, an Errata was discovered for J721E: https://www.ti.com/lit/er/sprz455d/sprz455d.pdf i2094: PCIe: End of Interrupt (EOI) Not Enabled for PCIe Legacy Interrupts Thus, there is no support for Legacy Interrupts (INTx) in the pci-j721e.c driver and as a consequence, no device-tree nodes either. > > > the .map_irq and .swizzle_irq callbacks to NULL. This fixes the error: > > of_irq_parse_pci: failed with rc=-22 > > I guess this happens because devm_of_pci_bridge_init() initializes > .map_irq and .swizzle_irq unconditionally? Yes. The PCIe Controller on J721E and other SoCs is the Cadence PCIe Controller. In the commit which added driver support for the Cadence PCIe Controller's Host functionality: https://github.com/torvalds/linux/commit/1b79c5284439 'map_irq' and 'swizzle_irq' were set to the same functions: 'of_irq_parse_and_map_pci()' and 'pci_common_swizzle()' respectively. Commit: https://github.com/torvalds/linux/commit/b64aa11eb2dd extracted out the shared defaults from multiple Host Controller drivers and moved them into the common 'devm_of_pci_bridge_init()' function. While 'map_irq' and 'swizzle_irq' are unconditionally set to the defaults in 'devm_of_pci_bridge_init()', those Host Controller driver which haven't been using the shared defaults do have the choice of overriding them in the driver (which is already being done). Similarly, for pci-j721e.c, since the shared defaults inherited from the Cadence PCIe Controller are not applicable (due to the Errata), the pci-j721e.c driver should override them to NULL as done in this patch. > > I'm not sure the default assumption should be that a host bridge > supports INTx. > > Maybe .map_irq and .swizzle_irq should only be set when we discover > some information about INTx routing, e.g., an ACPI _PRT or the > corresponding DT properties. I believe that the defaults were set since most of the Host drivers were using them in their respective drivers as indicated in the commit: https://github.com/torvalds/linux/commit/b64aa11eb2dd [...] > > > > drivers/pci/controller/cadence/pci-j721e.c | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/drivers/pci/controller/cadence/pci-j721e.c b/drivers/pci/controller/cadence/pci-j721e.c > > index 85718246016b..5372218849a8 100644 > > --- a/drivers/pci/controller/cadence/pci-j721e.c > > +++ b/drivers/pci/controller/cadence/pci-j721e.c > > @@ -417,6 +417,10 @@ static int j721e_pcie_probe(struct platform_device *pdev) > > if (!bridge) > > return -ENOMEM; > > > > + /* Legacy interrupts are not supported */ > > Say "INTx" explicitly here instead of assuming "legacy" == "INTx". Sure. I will update it in the v2 patch. Thank you for reviewing this patch and sharing your feedback. Regards, Siddharth.