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 A6107CD11DD for ; Tue, 26 Mar 2024 13:56:54 +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=lnVop1hUcMzSfKUX9mO8gHRKnsxFmmc2a5VZCiy5Zu0=; b=KMJiyKAThWI0io oNWzrkF4c8c1C1jYDghGVM8y8YlLKD/qhsp5IUNoNi9VUQsBlT86vVXcW3S6GJfUErbPAFi7udhaN my5OfBwJ1VFF8asQuYoJCywoaW4kTSE4ctb2ArVU6zbIg+ezupZan2ycxHELh/jpHYZeB6KfXtl8S RE68Kd/VdCGKwFi7YD2TmVMVSdaXFmtHqZVpwYaTvqrfLIsCtlxgHxNhdAEW20gzxXqx1ZAkadA/u 5gmhfSm03iSyIMlEjQpbeyyz3VxAoCvO2RwqEl9+Varxck+22r6JnUZ8W9PUQcPDSu3RxG4RwIVs3 8C4HPOOa9eWnp9zQ3nFQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rp7I7-00000004mdB-1JgO; Tue, 26 Mar 2024 13:56:39 +0000 Received: from sin.source.kernel.org ([145.40.73.55]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rp7I4-00000004mcC-3ZLY for linux-arm-kernel@lists.infradead.org; Tue, 26 Mar 2024 13:56:38 +0000 Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by sin.source.kernel.org (Postfix) with ESMTP id D6E6ECE1FE3; Tue, 26 Mar 2024 13:56:33 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 4B779C433F1; Tue, 26 Mar 2024 13:56:29 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1711461393; bh=kFHIDWQI67kN/OX0dUZ5DC/EkIeuT7MrsCc1k2qtV4I=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=Euiac2GBjlWSQQNq9+bL4zbkpwlDWDdJ/1P4L6K6p2Xk3aFL6a0WtzgH/AE9nGz38 FYntVfFjsnupMI4L3C8I7syWHizMxvpzZI06EkMVlP1aZ8tjLZ615tvKBtxkqUpV7L VcL2I7/TVtMYiCSRG+mzqAZ8h19BP0GP7kFGAsmK6RazlG1krGgLjNbtNbJTjgH2fL 0yjTDFDOrUPh3gMIHhBUqwlM4JVwP3RHOzCJl5/0guoM5xQcvEjNFdCHyCYQBdyWQa HBc7iFm6aWzz+TffNusXJ8QXcaXUuKs7xln4NJYJiSQY9+nwvZrzy2xnjfXFREu4Hp os5xZTAUFxigQ== Date: Tue, 26 Mar 2024 14:56:26 +0100 From: Niklas Cassel To: Siddharth Vadapalli Cc: lpieralisi@kernel.org, kw@linux.com, robh@kernel.org, bhelgaas@google.com, manivannan.sadhasivam@linaro.org, fancer.lancer@gmail.com, u.kleine-koenig@pengutronix.de, dlemoal@kernel.org, yoshihiro.shimoda.uh@renesas.com, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, srk@ti.com Subject: Re: [PATCH v5] PCI: keystone: Fix pci_ops for AM654x SoC Message-ID: References: <20240326111905.2369778-1-s-vadapalli@ti.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240326111905.2369778-1-s-vadapalli@ti.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240326_065637_307352_4B00BEF8 X-CRM114-Status: GOOD ( 27.67 ) 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, Mar 26, 2024 at 04:49:05PM +0530, Siddharth Vadapalli wrote: > In the process of converting .scan_bus() callbacks to .add_bus(), the > ks_pcie_v3_65_scan_bus() function was changed to ks_pcie_v3_65_add_bus(). > The .scan_bus() method belonged to ks_pcie_host_ops which was specific > to controller version 3.65a, while the .add_bus() method had been added > to ks_pcie_ops which is shared between the controller versions 3.65a and > 4.90a. Neither the older ks_pcie_v3_65_scan_bus() method, nor the newer > ks_pcie_v3_65_add_bus() method are applicable to the controller version > 4.90a which is present in AM654x SoCs. > > Thus, as a fix, move the contents of "ks_pcie_v3_65_add_bus()" to the > .msi_init callback "ks_pcie_msi_host_init()" which is specific to the > 3.65a controller. Also, move the definitions of ks_pcie_set_dbi_mode() > and ks_pcie_clear_dbi_mode() above ks_pcie_msi_host_init() in order to > avoid forward declaration. > > Fixes: 6ab15b5e7057 ("PCI: dwc: keystone: Convert .scan_bus() callback to use add_bus") > Suggested-by: Serge Semin > Suggested-by: Bjorn Helgaas > Suggested-by: Niklas Cassel > Signed-off-by: Siddharth Vadapalli > --- > > Hello, > > This patch is based on linux-next tagged next-20240326. > > v4: > https://lore.kernel.org/r/20240325053722.1955433-1-s-vadapalli@ti.com/ > Changes since v4: > - As suggested by Niklas Cassel at: > https://lore.kernel.org/r/ZgF_5fYsI5lOFjOv@ryzen/ > the contents of "ks_pcie_v3_65_add_bus()" have been moved to > "ks_pcie_msi_host_init()" instead of "ks_pcie_host_init()". This > avoids unnecessary checks for "!ks_pcie->is_am6" since > "ks_pcie_msi_host_init()" is specific to the v3.65a controller version > which corresponds to "!ks_pcie->is_am6". > - Updated commit message to match the change in implementation. > - Added "Suggested-by" tag of Niklas Cassel based on: > https://lore.kernel.org/r/ZgKaNrhoReJ0A525@x1-carbon/ > - Moved the definitions for ks_pcie_set_dbi_mode() and > ks_pcie_clear_dbi_mode() above ks_pcie_msi_host_init(). > > Regards, > Siddharth. > > drivers/pci/controller/dwc/pci-keystone.c | 136 ++++++++++------------ > 1 file changed, 60 insertions(+), 76 deletions(-) > > diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c > index 844de4418724..c2252448d9e8 100644 > --- a/drivers/pci/controller/dwc/pci-keystone.c > +++ b/drivers/pci/controller/dwc/pci-keystone.c > @@ -245,8 +245,68 @@ static struct irq_chip ks_pcie_msi_irq_chip = { > .irq_unmask = ks_pcie_msi_unmask, > }; > > +/** > + * ks_pcie_set_dbi_mode() - Set DBI mode to access overlaid BAR mask registers > + * @ks_pcie: A pointer to the keystone_pcie structure which holds the KeyStone > + * PCIe host controller driver information. > + * > + * Since modification of dbi_cs2 involves different clock domain, read the > + * status back to ensure the transition is complete. > + */ > +static void ks_pcie_set_dbi_mode(struct keystone_pcie *ks_pcie) > +{ > + u32 val; > + > + val = ks_pcie_app_readl(ks_pcie, CMD_STATUS); > + val |= DBI_CS2; > + ks_pcie_app_writel(ks_pcie, CMD_STATUS, val); > + > + do { > + val = ks_pcie_app_readl(ks_pcie, CMD_STATUS); > + } while (!(val & DBI_CS2)); > +} > + > +/** > + * ks_pcie_clear_dbi_mode() - Disable DBI mode > + * @ks_pcie: A pointer to the keystone_pcie structure which holds the KeyStone > + * PCIe host controller driver information. > + * > + * Since modification of dbi_cs2 involves different clock domain, read the > + * status back to ensure the transition is complete. > + */ > +static void ks_pcie_clear_dbi_mode(struct keystone_pcie *ks_pcie) > +{ > + u32 val; > + > + val = ks_pcie_app_readl(ks_pcie, CMD_STATUS); > + val &= ~DBI_CS2; > + ks_pcie_app_writel(ks_pcie, CMD_STATUS, val); > + > + do { > + val = ks_pcie_app_readl(ks_pcie, CMD_STATUS); > + } while (val & DBI_CS2); > +} > + > static int ks_pcie_msi_host_init(struct dw_pcie_rp *pp) > { > + struct dw_pcie *pci = to_dw_pcie_from_pp(pp); > + struct keystone_pcie *ks_pcie = to_keystone_pcie(pci); > + > + /* Configure and set up BAR0 */ > + ks_pcie_set_dbi_mode(ks_pcie); > + > + /* Enable BAR0 */ > + dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_0, 1); > + dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_0, SZ_4K - 1); > + > + ks_pcie_clear_dbi_mode(ks_pcie); > + > + /* > + * For BAR0, just setting bus address for inbound writes (MSI) should > + * be sufficient. Use physical address to avoid any conflicts. > + */ This comment seems to have wrong indentation. With that fixed: Reviewed-by: Niklas Cassel > + dw_pcie_writel_dbi(pci, PCI_BASE_ADDRESS_0, ks_pcie->app.start); > + > pp->msi_irq_chip = &ks_pcie_msi_irq_chip; > return dw_pcie_allocate_domains(pp); > } _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel