[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <c1ae0f3e-32ec-45cb-96fb-ef45979c9e1b@ti.com>
Date: Sat, 20 Sep 2025 14:10:02 +0530
From: Siddharth Vadapalli <s-vadapalli@...com>
To: Manivannan Sadhasivam <mani@...nel.org>
CC: Siddharth Vadapalli <s-vadapalli@...com>, <lpieralisi@...nel.org>,
<kwilczynski@...nel.org>, <robh@...nel.org>, <bhelgaas@...gle.com>,
<jingoohan1@...il.com>, <christian.bruel@...s.st.com>,
<qiang.yu@....qualcomm.com>, <mayank.rana@....qualcomm.com>,
<thippeswamy.havalige@....com>, <shradha.t@...sung.com>,
<quic_schintav@...cinc.com>, <inochiama@...il.com>,
<cassel@...nel.org>, <kishon@...nel.org>,
<sergio.paracuellos@...il.com>, <18255117159@....com>,
<rongqianfeng@...o.com>, <jirislaby@...nel.org>,
<linux-pci@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>, <srk@...com>
Subject: Re: [PATCH v2 10/10] PCI: keystone: Add support to build as a
loadable module
On Sat, Sep 20, 2025 at 02:01:03PM +0530, Manivannan Sadhasivam wrote:
> On Sat, Sep 20, 2025 at 01:41:35PM +0530, Siddharth Vadapalli wrote:
> > On Sat, Sep 20, 2025 at 12:10:59AM +0530, Manivannan Sadhasivam wrote:
> > > On Fri, Sep 12, 2025 at 05:46:21PM +0530, Siddharth Vadapalli wrote:
> > > > The 'pci-keystone.c' driver is the application/glue/wrapper driver for the
> > > > Designware PCIe Controllers on TI SoCs. Now that all of the helper APIs
> > > > that the 'pci-keystone.c' driver depends upon have been exported for use,
> > > > enable support to build the driver as a loadable module.
> > > >
> > > > Signed-off-by: Siddharth Vadapalli <s-vadapalli@...com>
> > > > ---
> > > >
> > > > v1: https://lore.kernel.org/r/20250903124505.365913-12-s-vadapalli@ti.com/
> > > > Changes since v1:
> > > > - Based on the feedback from Manivannan Sadhasivam <mani@...nel.org> at:
> > > > https://lore.kernel.org/r/2gzqupa7i7qhiscwm4uin2jmdb6qowp55mzk7w4o3f73ob64e7@taf5vjd7lhc5/
> > > > builtin_platform_driver() is being retained in the driver due to which
> > > > the change made in the v1 patch of replacing builtin_platform_driver()
> > > > with module_platform_driver() has been discarded in this patch.
> > > >
> > > > drivers/pci/controller/dwc/Kconfig | 6 +++---
> > > > drivers/pci/controller/dwc/pci-keystone.c | 22 ++++++++++++++++++++++
> > > > 2 files changed, 25 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > > > index 34abc859c107..46012d6a607e 100644
> > > > --- a/drivers/pci/controller/dwc/Kconfig
> > > > +++ b/drivers/pci/controller/dwc/Kconfig
> > > > @@ -482,10 +482,10 @@ config PCI_DRA7XX_EP
> > > > This uses the DesignWare core.
> > > >
> > > > config PCI_KEYSTONE
> > > > - bool
> > > > + tristate
> > > >
> > > > config PCI_KEYSTONE_HOST
> > > > - bool "TI Keystone PCIe controller (host mode)"
> > > > + tristate "TI Keystone PCIe controller (host mode)"
> > > > depends on ARCH_KEYSTONE || ARCH_K3 || COMPILE_TEST
> > > > depends on PCI_MSI
> > > > select PCIE_DW_HOST
> > > > @@ -497,7 +497,7 @@ config PCI_KEYSTONE_HOST
> > > > DesignWare core functions to implement the driver.
> > > >
> > > > config PCI_KEYSTONE_EP
> > > > - bool "TI Keystone PCIe controller (endpoint mode)"
> > > > + tristate "TI Keystone PCIe controller (endpoint mode)"
> > > > depends on ARCH_KEYSTONE || ARCH_K3 || COMPILE_TEST
> > > > depends on PCI_ENDPOINT
> > > > select PCIE_DW_EP
> > > > diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> > > > index e85942b4f6be..661e31b60a48 100644
> > > > --- a/drivers/pci/controller/dwc/pci-keystone.c
> > > > +++ b/drivers/pci/controller/dwc/pci-keystone.c
> > > > @@ -17,6 +17,7 @@
> > > > #include <linux/irqchip/chained_irq.h>
> > > > #include <linux/irqdomain.h>
> > > > #include <linux/mfd/syscon.h>
> > > > +#include <linux/module.h>
> > > > #include <linux/msi.h>
> > > > #include <linux/of.h>
> > > > #include <linux/of_irq.h>
> > > > @@ -132,6 +133,7 @@ struct keystone_pcie {
> > > > struct device_node *msi_intc_np;
> > > > struct irq_domain *intx_irq_domain;
> > > > struct device_node *np;
> > > > + struct gpio_desc *reset_gpio;
> > > >
> > > > /* Application register space */
> > > > void __iomem *va_app_base; /* DT 1st resource */
> > > > @@ -1211,6 +1213,7 @@ static const struct of_device_id ks_pcie_of_match[] = {
> > > > },
> > > > { },
> > > > };
> > > > +MODULE_DEVICE_TABLE(of, ks_pcie_of_match);
> > > >
> > > > static int ks_pcie_probe(struct platform_device *pdev)
> > > > {
> > > > @@ -1329,6 +1332,7 @@ static int ks_pcie_probe(struct platform_device *pdev)
> > > > dev_err(dev, "Failed to get reset GPIO\n");
> > > > goto err_link;
> > > > }
> > > > + ks_pcie->reset_gpio = gpiod;
> > > >
> > > > /* Obtain references to the PHYs */
> > > > for (i = 0; i < num_lanes; i++)
> > > > @@ -1440,9 +1444,23 @@ static void ks_pcie_remove(struct platform_device *pdev)
> > > > {
> > > > struct keystone_pcie *ks_pcie = platform_get_drvdata(pdev);
> > > > struct device_link **link = ks_pcie->link;
> > > > + struct dw_pcie *pci = ks_pcie->pci;
> > > > int num_lanes = ks_pcie->num_lanes;
> > > > + const struct ks_pcie_of_data *data;
> > > > struct device *dev = &pdev->dev;
> > > > + enum dw_pcie_device_mode mode;
> > > > +
> > > > + ks_pcie_disable_error_irq(ks_pcie);
> > > > + data = of_device_get_match_data(dev);
> > > > + mode = data->mode;
> > > > + if (mode == DW_PCIE_RC_TYPE) {
> > > > + dw_pcie_host_deinit(&pci->pp);
> > > > + } else {
> > > > + pci_epc_deinit_notify(pci->ep.epc);
> > > > + dw_pcie_ep_deinit(&pci->ep);
> > > > + }
> > > >
> > > > + gpiod_set_value_cansleep(ks_pcie->reset_gpio, 0);
> > >
> > > Can ks_pcie_remove() be called for a builtin_platform_driver?
> >
> > It cannot be executed but I have added it for the sake of completeness - in
> > the same manner that the initial implementation didn't simply make the
> > 'ks_pcie_remove()' function a no-op and the code exists as if it could
> > be executed or if it were to be executed at some point in the future.
> >
>
> It is a dead code, that's it. Drop it in this patch or separately before and add
> it back later when irq subsystem fixes the irq disposal issues.
I will discard the changes made to 'ks_pcie_remove()' in the v3 patch.
Regards,
Siddharth.
Powered by blists - more mailing lists