[PATCH 00/11] PCI: Keystone: Enable loadable module support

[PATCH 00/11] PCI: Keystone: Enable loadable module support

[Siddharth Vadapalli]

Hello,

This series enables support for the 'pci-keystone.c' driver to be built
as a loadable module. The motivation for the series is that PCIe is not
a necessity for booting Linux due to which the 'pci-keystone.c' driver
does not need to be built-in.

Series is based on linux-next tagged next-20250903.

Series has been tested on the AM654-IDK-EVM by validating module removal
and reprobe while connected to an Intel PCIe-Ethernet Adapter. Test Logs:
https://gist.github.com/Siddharth-Vadapalli-at-TI/dfd7821c187241d63098117eb7431a1b

Regards,
Siddharth.

Siddharth Vadapalli (11):
  PCI: Export pci_get_host_bridge_device() for use by pci-keystone
  PCI: dwc: Export dw_pcie_allocate_domains() for pci-keystone
  PCI: dwc: Add dw_pcie_free_domains() helper for cleanup
  PCI: dwc: ep: Export dw_pcie_ep_raise_msix_irq() for pci-keystone
  PCI: keystone: Add ks_pcie_free_msi_irq() helper for cleanup
  PCI: keystone: Add ks_pcie_free_intx_irq() helper for cleanup
  PCI: keystone: Add ks_pcie_host_deinit() helper for cleanup
  PCI: keystone: Add ks_pcie_disable_error_irq() helper for cleanup
  PCI: keystone: Switch to devm_request_irq() for "ks-pcie-error-irq"
    IRQ
  PCI: keystone: Exit ks_pcie_probe() for the default switch-case of
    "mode"
  PCI: keystone: Add support to build as a loadable module

 drivers/pci/controller/dwc/Kconfig            |   6 +-
 drivers/pci/controller/dwc/pci-keystone.c     | 111 +++++++++++++++++-
 .../pci/controller/dwc/pcie-designware-ep.c   |   1 +
 .../pci/controller/dwc/pcie-designware-host.c |  10 ++
 drivers/pci/controller/dwc/pcie-designware.h  |   5 +
 drivers/pci/host-bridge.c                     |   1 +
 include/linux/pci.h                           |   1 +
 7 files changed, 127 insertions(+), 8 deletions(-)

-- 
2.43.0

[PATCH 01/11] PCI: Export pci_get_host_bridge_device() for use by pci-keystone

[Siddharth Vadapalli]

The pci-keystone.c driver uses the 'pci_get_host_bridge_device()' helper.
In preparation for enabling the pci-keystone.c driver to be built as a
loadable module, export 'pci_get_host_bridge_device()'.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/pci/host-bridge.c | 1 +
 include/linux/pci.h       | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/pci/host-bridge.c b/drivers/pci/host-bridge.c
index afa50b446567..be5ef6516cff 100644
--- a/drivers/pci/host-bridge.c
+++ b/drivers/pci/host-bridge.c
@@ -33,6 +33,7 @@ struct device *pci_get_host_bridge_device(struct pci_dev *dev)
 	kobject_get(&bridge->kobj);
 	return bridge;
 }
+EXPORT_SYMBOL_GPL(pci_get_host_bridge_device);
 
 void  pci_put_host_bridge_device(struct device *dev)
 {
diff --git a/include/linux/pci.h b/include/linux/pci.h
index d1fdf81fbe1e..b253cbc27d36 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -646,6 +646,7 @@ struct pci_host_bridge *pci_alloc_host_bridge(size_t priv);
 struct pci_host_bridge *devm_pci_alloc_host_bridge(struct device *dev,
 						   size_t priv);
 void pci_free_host_bridge(struct pci_host_bridge *bridge);
+struct device *pci_get_host_bridge_device(struct pci_dev *dev);
 struct pci_host_bridge *pci_find_host_bridge(struct pci_bus *bus);
 
 void pci_set_host_bridge_release(struct pci_host_bridge *bridge,
-- 
2.43.0

[PATCH 02/11] PCI: dwc: Export dw_pcie_allocate_domains() for pci-keystone

[Siddharth Vadapalli]

The pci-keystone.c driver uses the 'dw_pcie_allocate_domains()' helper.
In preparation for enabling the pci-keystone.c driver to be built as a
loadable module, export 'dw_pcie_allocate_domains()'.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 952f8594b501..3cc83d921376 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -229,6 +229,7 @@ int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(dw_pcie_allocate_domains);
 
 void dw_pcie_free_msi(struct dw_pcie_rp *pp)
 {
-- 
2.43.0

[PATCH 03/11] PCI: dwc: Add dw_pcie_free_domains() helper for cleanup

[Siddharth Vadapalli]

Introduce the helper function dw_pcie_free_domains() which will undo the
allocation performed by the dw_pcie_allocate_domains() function. Export
this helper for the users of dw_pcie_allocate_domains().

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/pci/controller/dwc/pcie-designware-host.c | 9 +++++++++
 drivers/pci/controller/dwc/pcie-designware.h      | 5 +++++
 2 files changed, 14 insertions(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index 3cc83d921376..df55c0ed75e4 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -211,6 +211,15 @@ static const struct irq_domain_ops dw_pcie_msi_domain_ops = {
 	.free	= dw_pcie_irq_domain_free,
 };
 
+void dw_pcie_free_domains(struct dw_pcie_rp *pp)
+{
+	if (pp->irq_domain) {
+		irq_domain_remove(pp->irq_domain);
+		pp->irq_domain = NULL;
+	}
+}
+EXPORT_SYMBOL_GPL(dw_pcie_free_domains);
+
 int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
index a44f2113925d..9f6f6f0ecd93 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -802,6 +802,7 @@ void dw_pcie_free_msi(struct dw_pcie_rp *pp);
 int dw_pcie_setup_rc(struct dw_pcie_rp *pp);
 int dw_pcie_host_init(struct dw_pcie_rp *pp);
 void dw_pcie_host_deinit(struct dw_pcie_rp *pp);
+void dw_pcie_free_domains(struct dw_pcie_rp *pp);
 int dw_pcie_allocate_domains(struct dw_pcie_rp *pp);
 void __iomem *dw_pcie_own_conf_map_bus(struct pci_bus *bus, unsigned int devfn,
 				       int where);
@@ -846,6 +847,10 @@ static inline void dw_pcie_host_deinit(struct dw_pcie_rp *pp)
 {
 }
 
+static inline void dw_pcie_free_domains(struct dw_pcie_rp *pp)
+{
+}
+
 static inline int dw_pcie_allocate_domains(struct dw_pcie_rp *pp)
 {
 	return 0;
-- 
2.43.0

[PATCH 04/11] PCI: dwc: ep: Export dw_pcie_ep_raise_msix_irq() for pci-keystone

[Siddharth Vadapalli]

The pci-keystone.c driver uses the 'dw_pcie_ep_raise_msix_irq()' helper.
In preparation for enabling the pci-keystone.c driver to be built as a
loadable module, export 'dw_pcie_ep_raise_msix_irq()'.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/pci/controller/dwc/pcie-designware-ep.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index 7f2112c2fb21..19571ac2b961 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -797,6 +797,7 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(dw_pcie_ep_raise_msix_irq);
 
 /**
  * dw_pcie_ep_cleanup - Cleanup DWC EP resources after fundamental reset
-- 
2.43.0

[PATCH 05/11] PCI: keystone: Add ks_pcie_free_msi_irq() helper for cleanup

[Siddharth Vadapalli]

Introduce the helper function ks_pcie_free_msi_irq() which will undo the
configuration performed by the ks_pcie_config_msi_irq() function. This will
be required for implementing a future helper function to undo the
configuration performed by the ks_pcie_host_init() function.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/pci/controller/dwc/pci-keystone.c | 25 +++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 3d10e1112131..6cedb6dc4650 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -666,6 +666,31 @@ static void ks_pcie_intx_irq_handler(struct irq_desc *desc)
 	chained_irq_exit(chip, desc);
 }
 
+static void ks_pcie_free_msi_irq(struct keystone_pcie *ks_pcie)
+{
+	struct device_node *np = ks_pcie->np;
+	struct device_node *intc_np;
+	int irq_count, irq, i;
+
+	if (!IS_ENABLED(CONFIG_PCI_MSI))
+		return;
+
+	/* Nothing to do if MSI Interrupt Controller does not exist */
+	intc_np = of_get_child_by_name(np, "msi-interrupt-controller");
+	if (!intc_np)
+		return;
+
+	/* irq_count should be non-zero. Else, ks_pcie_host_init would have failed. */
+	irq_count = of_irq_count(intc_np);
+
+	for (i = 0; i < irq_count; i++) {
+		/* We expect to get an irq since it succeeded during 'config'. */
+		irq = irq_of_parse_and_map(intc_np, i);
+		irq_set_chained_handler(irq, NULL);
+	}
+	of_node_put(intc_np);
+}
+
 static int ks_pcie_config_msi_irq(struct keystone_pcie *ks_pcie)
 {
 	struct device *dev = ks_pcie->pci->dev;
-- 
2.43.0

[PATCH 06/11] PCI: keystone: Add ks_pcie_free_intx_irq() helper for cleanup

[Siddharth Vadapalli]

Introduce the helper function ks_pcie_free_intx_irq() which will undo the
configuration performed by the ks_pcie_config_intx_irq() function. This
will be required for implementing a future helper function to undo the
configuration performed by the ks_pcie_host_init() function.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/pci/controller/dwc/pci-keystone.c | 29 +++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 6cedb6dc4650..3afa298e89d1 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -745,6 +745,35 @@ static int ks_pcie_config_msi_irq(struct keystone_pcie *ks_pcie)
 	return ret;
 }
 
+static void ks_pcie_free_intx_irq(struct keystone_pcie *ks_pcie)
+{
+	struct device_node *np = ks_pcie->np;
+	struct device_node *intc_np;
+	int irq_count, i;
+	u32 val;
+
+	/* Nothing to do if INTx Interrupt Controller does not exist */
+	intc_np = of_get_child_by_name(np, "legacy-interrupt-controller");
+	if (!intc_np)
+		return;
+
+	/* irq_count should be non-zero. Else, ks_pcie_host_init would have failed. */
+	irq_count = of_irq_count(intc_np);
+
+	/* Disable all legacy interrupts */
+	for (i = 0; i < PCI_NUM_INTX; i++) {
+		val = ks_pcie_app_readl(ks_pcie, IRQ_ENABLE_SET(i));
+		val &= ~INTx_EN;
+		ks_pcie_app_writel(ks_pcie, IRQ_ENABLE_SET(i), val);
+	}
+
+	irq_domain_remove(ks_pcie->intx_irq_domain);
+	for (i = 0; i < irq_count; i++)
+		irq_set_chained_handler(ks_pcie->intx_host_irqs[i], NULL);
+
+	of_node_put(intc_np);
+}
+
 static int ks_pcie_config_intx_irq(struct keystone_pcie *ks_pcie)
 {
 	struct device *dev = ks_pcie->pci->dev;
-- 
2.43.0

[PATCH 07/11] PCI: keystone: Add ks_pcie_host_deinit() helper for cleanup

[Siddharth Vadapalli]

Introduce the helper function ks_pcie_host_deinit() to undo the
configuration performed by the ks_pcie_host_init() function and also to
free the MSI Domains if the '.msi_init' callback was implemented which
would have made a call to dw_pcie_allocate_domains().

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/pci/controller/dwc/pci-keystone.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 3afa298e89d1..f432818f6802 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -885,6 +885,18 @@ static int __init ks_pcie_init_id(struct keystone_pcie *ks_pcie)
 	return 0;
 }
 
+static void ks_pcie_host_deinit(struct dw_pcie_rp *pp)
+{
+	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
+	struct keystone_pcie *ks_pcie = to_keystone_pcie(pci);
+
+	ks_pcie_stop_link(pci);
+	ks_pcie_free_msi_irq(ks_pcie);
+	ks_pcie_free_intx_irq(ks_pcie);
+	if (pci->pp.ops->msi_init)
+		dw_pcie_free_domains(pp);
+}
+
 static int __init ks_pcie_host_init(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -929,11 +941,13 @@ static int __init ks_pcie_host_init(struct dw_pcie_rp *pp)
 
 static const struct dw_pcie_host_ops ks_pcie_host_ops = {
 	.init = ks_pcie_host_init,
+	.deinit = ks_pcie_host_deinit,
 	.msi_init = ks_pcie_msi_host_init,
 };
 
 static const struct dw_pcie_host_ops ks_pcie_am654_host_ops = {
 	.init = ks_pcie_host_init,
+	.deinit = ks_pcie_host_deinit,
 };
 
 static irqreturn_t ks_pcie_err_irq_handler(int irq, void *priv)
-- 
2.43.0

[PATCH 08/11] PCI: keystone: Add ks_pcie_disable_error_irq() helper for cleanup

[Siddharth Vadapalli]

Introduce the helper function ks_pcie_disable_error_irq() to disable the
error interrupts that have been enabled by ks_pcie_enable_error_irq().

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/pci/controller/dwc/pci-keystone.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index f432818f6802..bb93559f6468 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -329,6 +329,15 @@ static void ks_pcie_handle_intx_irq(struct keystone_pcie *ks_pcie,
 	ks_pcie_app_writel(ks_pcie, IRQ_EOI, offset);
 }
 
+static void ks_pcie_disable_error_irq(struct keystone_pcie *ks_pcie)
+{
+	u32 val;
+
+	val = ks_pcie_app_readl(ks_pcie, ERR_IRQ_ENABLE_SET);
+	val &= ~ERR_IRQ_ALL;
+	ks_pcie_app_writel(ks_pcie, ERR_IRQ_ENABLE_SET, val);
+}
+
 static void ks_pcie_enable_error_irq(struct keystone_pcie *ks_pcie)
 {
 	ks_pcie_app_writel(ks_pcie, ERR_IRQ_ENABLE_SET, ERR_IRQ_ALL);
-- 
2.43.0

[PATCH 09/11] PCI: keystone: Switch to devm_request_irq() for "ks-pcie-error-irq" IRQ

[Siddharth Vadapalli]

In preparation for enabling loadable module support for the driver,
there is motivation to switch to devm_request_irq() to simplify the
cleanup on driver removal. Additionally, since the interrupt handler
associated with the "ks-pcie-error-irq" namely "ks_pcie_handle_error_irq()
is only printing the error and is clearing the interrupt, there is no
necessity to prefer devm_request_threaded_irq() over devm_request_irq().
Hence, switch from request_irq() to devm_request_irq().

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/pci/controller/dwc/pci-keystone.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index bb93559f6468..02f9a6d0e4a8 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -1277,8 +1277,8 @@ static int ks_pcie_probe(struct platform_device *pdev)
 	if (irq < 0)
 		return irq;
 
-	ret = request_irq(irq, ks_pcie_err_irq_handler, IRQF_SHARED,
-			  "ks-pcie-error-irq", ks_pcie);
+	ret = devm_request_irq(dev, irq, ks_pcie_err_irq_handler, IRQF_SHARED,
+			       "ks-pcie-error-irq", ks_pcie);
 	if (ret < 0) {
 		dev_err(dev, "failed to request error IRQ %d\n",
 			irq);
-- 
2.43.0

[PATCH 10/11] PCI: keystone: Exit ks_pcie_probe() for the default switch-case of "mode"

[Siddharth Vadapalli]

In ks_pcie_probe(), the switch-case for the "mode" is used to configure
the PCIe Controller for either Root-Complex or Endpoint mode of operation.
Prior to the switch-case statement for "mode" an invalid mode will result
in probe failure only if "dw_pcie_ver_is_ge(pci, 480A)" is true, which
is the case for the AM654 platform. On the other hand, when that is not
the case, "ks_pcie_set_mode()" will be invoked, which does not validate
the mode. As a result, it is possible for the switch-case statement for
"mode" to receive an invalid mode. Currently, an error message is displayed
in the "default" case where "mode" is neither "DW_PCIE_RC_TYPE" nor
"DW_PCIE_EP_TYPE", but the probe succeeds. However, since the configuration
required for Root-Complex and Endpoint mode have not been performed, the
Controller is not operational.

Fix this by exiting "ks_pcie_probe()" with the return value of "-EINVAL"
in addition to displaying the existing error message.

Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---

NOTE: A "Fixes" tag is ommitted on purpose since the fix is not crucial:
1. It doesn't fix a crash or any fatal error
2. It doesn't enable controller functionality by fixing the issue

Therefore, the patch may not be worth backporting.

Regards,
Siddharth.

 drivers/pci/controller/dwc/pci-keystone.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 02f9a6d0e4a8..4ed6eab0a2f0 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -1414,6 +1414,8 @@ static int ks_pcie_probe(struct platform_device *pdev)
 		break;
 	default:
 		dev_err(dev, "INVALID device type %d\n", mode);
+		ret = -EINVAL;
+		goto err_get_sync;
 	}
 
 	ks_pcie_enable_error_irq(ks_pcie);
-- 
2.43.0

[PATCH 11/11] PCI: keystone: Add support to build as a loadable module

[Siddharth Vadapalli]

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@ti.com>
---
 drivers/pci/controller/dwc/Kconfig        |  6 ++---
 drivers/pci/controller/dwc/pci-keystone.c | 28 ++++++++++++++++++++---
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index deafc512b079..33f3dab7b385 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -458,10 +458,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
@@ -473,7 +473,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 4ed6eab0a2f0..eabe7e9ed44b 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 */
@@ -862,7 +864,7 @@ static int ks_pcie_fault(unsigned long addr, unsigned int fsr,
 }
 #endif
 
-static int __init ks_pcie_init_id(struct keystone_pcie *ks_pcie)
+static int ks_pcie_init_id(struct keystone_pcie *ks_pcie)
 {
 	int ret;
 	unsigned int id;
@@ -906,7 +908,7 @@ static void ks_pcie_host_deinit(struct dw_pcie_rp *pp)
 		dw_pcie_free_domains(pp);
 }
 
-static int __init ks_pcie_host_init(struct dw_pcie_rp *pp)
+static int ks_pcie_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);
@@ -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);
 	pm_runtime_put(dev);
 	pm_runtime_disable(dev);
 	ks_pcie_disable_phy(ks_pcie);
@@ -1458,4 +1476,8 @@ static struct platform_driver ks_pcie_driver = {
 		.of_match_table = ks_pcie_of_match,
 	},
 };
-builtin_platform_driver(ks_pcie_driver);
+module_platform_driver(ks_pcie_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("PCIe host controller driver for Texas Instruments Keystone SoCs");
+MODULE_AUTHOR("Murali Karicheri <m-karicheri2@ti.com>");
-- 
2.43.0

Re: [PATCH 09/11] PCI: keystone: Switch to devm_request_irq() for "ks-pcie-error-irq" IRQ

[Jiri Slaby]

On 03. 09. 25, 14:44, Siddharth Vadapalli wrote:
> In preparation for enabling loadable module support for the driver,
> there is motivation to switch to devm_request_irq() to simplify the
> cleanup on driver removal. Additionally, since the interrupt handler
> associated with the "ks-pcie-error-irq" namely "ks_pcie_handle_error_irq()
> is only printing the error and is clearing the interrupt, there is no
> necessity to prefer devm_request_threaded_irq() over devm_request_irq().
> Hence, switch from request_irq() to devm_request_irq().
> 
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
>   drivers/pci/controller/dwc/pci-keystone.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index bb93559f6468..02f9a6d0e4a8 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -1277,8 +1277,8 @@ static int ks_pcie_probe(struct platform_device *pdev)
>   	if (irq < 0)
>   		return irq;
>   
> -	ret = request_irq(irq, ks_pcie_err_irq_handler, IRQF_SHARED,
> -			  "ks-pcie-error-irq", ks_pcie);
> +	ret = devm_request_irq(dev, irq, ks_pcie_err_irq_handler, IRQF_SHARED,
> +			       "ks-pcie-error-irq", ks_pcie);
>   	if (ret < 0) {
>   		dev_err(dev, "failed to request error IRQ %d\n",
>   			irq);

Ugh, so you are not removing any free_irq() from anywhere?

<me checking>

Because there is none...

So you are actually fixing an IRQ leak in case something later fails -- 
I guess this needs Fixes and Cc stable tags, right?

thanks,
-- 
js
suse labs

Re: [PATCH 09/11] PCI: keystone: Switch to devm_request_irq() for "ks-pcie-error-irq" IRQ

[Siddharth Vadapalli]

On Thu, Sep 04, 2025 at 09:24:01AM +0200, Jiri Slaby wrote:
> On 03. 09. 25, 14:44, Siddharth Vadapalli wrote:
> > In preparation for enabling loadable module support for the driver,
> > there is motivation to switch to devm_request_irq() to simplify the
> > cleanup on driver removal. Additionally, since the interrupt handler
> > associated with the "ks-pcie-error-irq" namely "ks_pcie_handle_error_irq()
> > is only printing the error and is clearing the interrupt, there is no
> > necessity to prefer devm_request_threaded_irq() over devm_request_irq().
> > Hence, switch from request_irq() to devm_request_irq().
> > 
> > Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> > ---
> >   drivers/pci/controller/dwc/pci-keystone.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> > index bb93559f6468..02f9a6d0e4a8 100644
> > --- a/drivers/pci/controller/dwc/pci-keystone.c
> > +++ b/drivers/pci/controller/dwc/pci-keystone.c
> > @@ -1277,8 +1277,8 @@ static int ks_pcie_probe(struct platform_device *pdev)
> >   	if (irq < 0)
> >   		return irq;
> > -	ret = request_irq(irq, ks_pcie_err_irq_handler, IRQF_SHARED,
> > -			  "ks-pcie-error-irq", ks_pcie);
> > +	ret = devm_request_irq(dev, irq, ks_pcie_err_irq_handler, IRQF_SHARED,
> > +			       "ks-pcie-error-irq", ks_pcie);
> >   	if (ret < 0) {
> >   		dev_err(dev, "failed to request error IRQ %d\n",
> >   			irq);
> 
> Ugh, so you are not removing any free_irq() from anywhere?
> 
> <me checking>
> 
> Because there is none...
> 
> So you are actually fixing an IRQ leak in case something later fails -- I
> guess this needs Fixes and Cc stable tags, right?

Yes! While working on this series and unloading the module, I had seen
the issue and narrowed it down to the absence of a free_irq() in the
driver's .remove callback. Since the driver cannot be unloaded prior to
this series (driver can only be built-in), I had felt that a separate
fix may not be required for adding the missing free_irq() in the .remove
callback. However, after you pointed it out now, I realize that there
are other places during the driver's probe where we may exit in the
event of an error, without freeing the interrupt. Thank you for noticing
this. The commit to be fixed is:
0790eb175ee0 PCI: keystone: Cleanup error_irq configuration
Link to the commit is:
https://github.com/torvalds/linux/commit/0790eb175ee0
I will include the Fixes tag for the above commit and also Cc stable when
I post the v2 series. I will wait for feedback on the remaining patches
in the series before posting the v2 series.

Regards,
Siddharth.