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

[PATCH v2 00/10] 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 commit
bfd2c81b061c Merge branch 'pci/misc'
of pci/next.

NOTE for MAINTAINERS: This series has the following dependencies:
1. The following commit in Linux-Next is a build-dependency for the
series:
https://github.com/ColinIanKing/linux-next/commit/8de1de5a3a8d42975953382068fb5195e9d6e6c6
Since the v1 series was based on linux-next, there were no build errors.
However, since this series is based on pci/next based on the feedback from
Manivannan Sadhasivam <mani@kernel.org> at:
https://lore.kernel.org/r/2gzqupa7i7qhiscwm4uin2jmdb6qowp55mzk7w4o3f73ob64e7@taf5vjd7lhc5/
without the aforementioned commit, build will fail.
2. The following patch series include fixes for the driver which are
required to verify the driver functionality:
https://lore.kernel.org/r/20250912100802.3136121-1-s-vadapalli@ti.com/

v1 of this series is at:
https://lore.kernel.org/r/20250903124505.365913-1-s-vadapalli@ti.com/
Changes since v1:
- Based on the feedback provided by Jiri Slaby <jirislaby@kernel.org> at:
  https://lore.kernel.org/r/3d3a4b52-e343-42f3-9d69-94c259812143@kernel.org/
  patch 09/11 of the v1 series has been posted as a FIX at:
  https://lore.kernel.org/r/20250912100802.3136121-2-s-vadapalli@ti.com/
  and is therefore no longer a part of the current series.
- Based on the feedback provided by Manivannan Sadhasivam <mani@kernel.org> at:
  https://lore.kernel.org/r/2gzqupa7i7qhiscwm4uin2jmdb6qowp55mzk7w4o3f73ob64e7@taf5vjd7lhc5/
  patch 11/11 of the v1 series which is patch 10/10 of this series has
  been modified by retaining 'builtin_platform_driver()' instead of
  changing it to 'module_platform_driver()'.
- Series has been based on pci/next instead of linux-next.

For testing the series, Linux has been built in the following manner:
1. Check out at commit bfd2c81b061c Merge branch 'pci/misc' of pci/next
2. Apply commit and patch series mentioned as dependencies above.
3. Apply current series.
4. Build Linux with CONFIG_PCI_KEYSTONE, CONFIG_PCI_KEYSTONE_HOST and
   CONFIG_PCI_KEYSTONE_EP set to 'm'.

Series has been tested on AM654-EVM with an NVMe SSD connected to the
PCIe connector on the board and verifying that the NVMe SSD enumerates
successfully. Additionally, the 'hdparm' utility has been used to read
the NVMe SSD for verifying functionality. Test Logs:
https://gist.github.com/Siddharth-Vadapalli-at-TI/54848d3414dc965bec3c29253a1b2764

Regards,
Siddharth.

Siddharth Vadapalli (10):
  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: 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     | 101 ++++++++++++++++++
 .../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, 122 insertions(+), 3 deletions(-)

-- 
2.43.0

[PATCH v2 01/10] 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>
---

v1: https://lore.kernel.org/r/20250903124505.365913-2-s-vadapalli@ti.com/
No changes since v1.

 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 v2 02/10] 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>
---

v1: https://lore.kernel.org/r/20250903124505.365913-3-s-vadapalli@ti.com/
No changes since v1.

 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 v2 03/10] 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>
---

v1: https://lore.kernel.org/r/20250903124505.365913-4-s-vadapalli@ti.com/
No changes since v1.

 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 a333ab0b0bbd..89659d05ab29 100644
--- a/drivers/pci/controller/dwc/pcie-designware.h
+++ b/drivers/pci/controller/dwc/pcie-designware.h
@@ -801,6 +801,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);
@@ -845,6 +846,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 v2 04/10] 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>
---

v1: https://lore.kernel.org/r/20250903124505.365913-5-s-vadapalli@ti.com/
No changes since v1.

 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 v2 05/10] 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>
---

v1: https://lore.kernel.org/r/20250903124505.365913-6-s-vadapalli@ti.com/
No changes since v1.

 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 d03e95bf7d54..81c3686688c0 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 v2 06/10] 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>
---

v1: https://lore.kernel.org/r/20250903124505.365913-7-s-vadapalli@ti.com/
No changes since v1.

 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 81c3686688c0..074566fb1d74 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 v2 07/10] 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>
---

v1: https://lore.kernel.org/r/20250903124505.365913-8-s-vadapalli@ti.com/
No changes since v1.

 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 074566fb1d74..3a3188e89a2a 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -885,6 +885,18 @@ static int 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 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 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 v2 08/10] 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>
---

v1: https://lore.kernel.org/r/20250903124505.365913-9-s-vadapalli@ti.com/
No changes since v1.

 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 3a3188e89a2a..2da9feaaf9ee 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 v2 09/10] 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>
---

v1: https://lore.kernel.org/r/20250903124505.365913-11-s-vadapalli@ti.com/
No changes since v1.

 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 2da9feaaf9ee..e85942b4f6be 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 v2 10/10] 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>
---

v1: https://lore.kernel.org/r/20250903124505.365913-12-s-vadapalli@ti.com/
Changes since v1:
- Based on the feedback from Manivannan Sadhasivam <mani@kernel.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);
 	pm_runtime_put(dev);
 	pm_runtime_disable(dev);
 	ks_pcie_disable_phy(ks_pcie);
@@ -1459,3 +1477,7 @@ static struct platform_driver ks_pcie_driver = {
 	},
 };
 builtin_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 v2 04/10] PCI: dwc: ep: Export dw_pcie_ep_raise_msix_irq() for pci-keystone

[Manivannan Sadhasivam]

On Fri, Sep 12, 2025 at 05:46:15PM +0530, Siddharth Vadapalli wrote:
> 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>

This patch could be merged with patch 2.

- Mani

> ---
> 
> v1: https://lore.kernel.org/r/20250903124505.365913-5-s-vadapalli@ti.com/
> No changes since v1.
> 
>  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
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 05/10] PCI: keystone: Add ks_pcie_free_msi_irq() helper for cleanup

[Manivannan Sadhasivam]

On Fri, Sep 12, 2025 at 05:46:16PM +0530, Siddharth Vadapalli wrote:
> 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>
> ---
> 
> v1: https://lore.kernel.org/r/20250903124505.365913-6-s-vadapalli@ti.com/
> No changes since v1.
> 
>  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 d03e95bf7d54..81c3686688c0 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))

Isn't the CONFIG_PCI_KEYSTONE_HOST always depend on PCI_MSI?

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 06/10] PCI: keystone: Add ks_pcie_free_intx_irq() helper for cleanup

[Manivannan Sadhasivam]

On Fri, Sep 12, 2025 at 05:46:17PM +0530, Siddharth Vadapalli wrote:
> 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>

Please squash all patches introducing new helpers.

- Mani

> ---
> 
> v1: https://lore.kernel.org/r/20250903124505.365913-7-s-vadapalli@ti.com/
> No changes since v1.
> 
>  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 81c3686688c0..074566fb1d74 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
> 

-- 
மணிவண்ணன் சதாசிவம்

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

[Manivannan Sadhasivam]

On Fri, Sep 12, 2025 at 05:46:20PM +0530, Siddharth Vadapalli wrote:
> 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>

Fixes tag? And probably CC stable since the controller seems to be not
operations without this fix.

- Mani

> ---
> 
> v1: https://lore.kernel.org/r/20250903124505.365913-11-s-vadapalli@ti.com/
> No changes since v1.
> 
>  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 2da9feaaf9ee..e85942b4f6be 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
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 10/10] PCI: keystone: Add support to build as a loadable module

[Manivannan Sadhasivam]

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@ti.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@kernel.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?

- Mani

-- 
மணிவண்ணன் சதாசிவம்

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

[Siddharth Vadapalli]

On Sat, Sep 20, 2025 at 12:00:47AM +0530, Manivannan Sadhasivam wrote:
> On Fri, Sep 12, 2025 at 05:46:15PM +0530, Siddharth Vadapalli wrote:
> > 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>
> 
> This patch could be merged with patch 2.

I will squash this in patch 2 in the v3 series.

Regards,
Siddharth.

Re: [PATCH v2 05/10] PCI: keystone: Add ks_pcie_free_msi_irq() helper for cleanup

[Siddharth Vadapalli]

On Sat, Sep 20, 2025 at 12:02:34AM +0530, Manivannan Sadhasivam wrote:
> On Fri, Sep 12, 2025 at 05:46:16PM +0530, Siddharth Vadapalli wrote:
> > 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>
> > ---
> > 
> > v1: https://lore.kernel.org/r/20250903124505.365913-6-s-vadapalli@ti.com/
> > No changes since v1.
> > 
> >  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 d03e95bf7d54..81c3686688c0 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))
> 
> Isn't the CONFIG_PCI_KEYSTONE_HOST always depend on PCI_MSI?

The reason I added the check is that it exists in 'ks_pcie_config_msi_irq()'.
But I realize now that it should be removed from
'ks_pcie_config_msi_irq()' as well. Since I had written the above
function with the objective of undoing the changes done by
'ks_pcie_config_msi_irq()', the 'config check' was retained since the
changes should be undone only if they were executed by
'ks_pcie_config_msi_irq()'. I will drop the check in the v3 series and
will also post a separate patch to drop if from 'ks_pcie_config_msi_irq()'
if that is acceptable. Please let me know.

Regards,
Siddharth.

Re: [PATCH v2 06/10] PCI: keystone: Add ks_pcie_free_intx_irq() helper for cleanup

[Siddharth Vadapalli]

On Sat, Sep 20, 2025 at 12:05:38AM +0530, Manivannan Sadhasivam wrote:
> On Fri, Sep 12, 2025 at 05:46:17PM +0530, Siddharth Vadapalli wrote:
> > 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>
> 
> Please squash all patches introducing new helpers.

I will squash them in the v3 series.

Regards,
Siddharth.

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

[Siddharth Vadapalli]

On Sat, Sep 20, 2025 at 12:06:46AM +0530, Manivannan Sadhasivam wrote:
> On Fri, Sep 12, 2025 at 05:46:20PM +0530, Siddharth Vadapalli wrote:
> > 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>
> 
> Fixes tag? And probably CC stable since the controller seems to be not
> operations without this fix.

While I had mentioned the rationale for not including the 'Fixes tag' in
the v1 patch below the tearline, I forgot to add it in this patch. I will
quote the same below:

    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.

Prior to this patch, the probe succeeded and the controller was
unusable. Post this patch, the probe will fail and the controller is
still unusable. Behavior is identical from a usability perspective but
the user is aware of the issue since the probe fails.

> 
> - Mani
> 
> > ---
> > 
> > v1: https://lore.kernel.org/r/20250903124505.365913-11-s-vadapalli@ti.com/
> > No changes since v1.
> > 
> >  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 2da9feaaf9ee..e85942b4f6be 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);

Regards,
Siddharth.

Re: [PATCH v2 10/10] PCI: keystone: Add support to build as a loadable module

[Siddharth Vadapalli]

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@ti.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@kernel.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.

Regards,
Siddharth.

Re: [PATCH v2 05/10] PCI: keystone: Add ks_pcie_free_msi_irq() helper for cleanup

[Manivannan Sadhasivam]

On Sat, Sep 20, 2025 at 01:34:15PM +0530, Siddharth Vadapalli wrote:
> On Sat, Sep 20, 2025 at 12:02:34AM +0530, Manivannan Sadhasivam wrote:
> > On Fri, Sep 12, 2025 at 05:46:16PM +0530, Siddharth Vadapalli wrote:
> > > 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>
> > > ---
> > > 
> > > v1: https://lore.kernel.org/r/20250903124505.365913-6-s-vadapalli@ti.com/
> > > No changes since v1.
> > > 
> > >  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 d03e95bf7d54..81c3686688c0 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))
> > 
> > Isn't the CONFIG_PCI_KEYSTONE_HOST always depend on PCI_MSI?
> 
> The reason I added the check is that it exists in 'ks_pcie_config_msi_irq()'.
> But I realize now that it should be removed from
> 'ks_pcie_config_msi_irq()' as well. Since I had written the above
> function with the objective of undoing the changes done by
> 'ks_pcie_config_msi_irq()', the 'config check' was retained since the
> changes should be undone only if they were executed by
> 'ks_pcie_config_msi_irq()'. I will drop the check in the v3 series and
> will also post a separate patch to drop if from 'ks_pcie_config_msi_irq()'
> if that is acceptable. Please let me know.
> 

Sounds good to me.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

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

[Manivannan Sadhasivam]

On Sat, Sep 20, 2025 at 01:39:22PM +0530, Siddharth Vadapalli wrote:
> On Sat, Sep 20, 2025 at 12:06:46AM +0530, Manivannan Sadhasivam wrote:
> > On Fri, Sep 12, 2025 at 05:46:20PM +0530, Siddharth Vadapalli wrote:
> > > 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>
> > 
> > Fixes tag? And probably CC stable since the controller seems to be not
> > operations without this fix.
> 
> While I had mentioned the rationale for not including the 'Fixes tag' in
> the v1 patch below the tearline, I forgot to add it in this patch. I will
> quote the same below:
> 
>     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
> 

Fixes tag should be added irrespective of the cruciality of the bug.

>     Therefore, the patch may not be worth backporting.
> 

Agree with this though.

> Prior to this patch, the probe succeeded and the controller was
> unusable. Post this patch, the probe will fail and the controller is
> still unusable. Behavior is identical from a usability perspective but
> the user is aware of the issue since the probe fails.
> 

Ok. I think the description could reworded to make it more clear. The actual
issue is that the default case lacks setting the errno, leading to probe
success. But the addition of ks_pcie_set_mode() and others seem to cause little
bit confusion.

- Mani

> > 
> > - Mani
> > 
> > > ---
> > > 
> > > v1: https://lore.kernel.org/r/20250903124505.365913-11-s-vadapalli@ti.com/
> > > No changes since v1.
> > > 
> > >  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 2da9feaaf9ee..e85942b4f6be 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);
> 
> Regards,
> Siddharth.

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH v2 10/10] PCI: keystone: Add support to build as a loadable module

[Manivannan Sadhasivam]

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@ti.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@kernel.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.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

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

[Siddharth Vadapalli]

On Sat, Sep 20, 2025 at 01:57:34PM +0530, Manivannan Sadhasivam wrote:
> On Sat, Sep 20, 2025 at 01:39:22PM +0530, Siddharth Vadapalli wrote:
> > On Sat, Sep 20, 2025 at 12:06:46AM +0530, Manivannan Sadhasivam wrote:
> > > On Fri, Sep 12, 2025 at 05:46:20PM +0530, Siddharth Vadapalli wrote:
> > > > 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>
> > > 
> > > Fixes tag? And probably CC stable since the controller seems to be not
> > > operations without this fix.
> > 
> > While I had mentioned the rationale for not including the 'Fixes tag' in
> > the v1 patch below the tearline, I forgot to add it in this patch. I will
> > quote the same below:
> > 
> >     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
> > 
> 
> Fixes tag should be added irrespective of the cruciality of the bug.

Ok, I will include the tag in the v3 series.

> 
> >     Therefore, the patch may not be worth backporting.
> > 
> 
> Agree with this though.
> 
> > Prior to this patch, the probe succeeded and the controller was
> > unusable. Post this patch, the probe will fail and the controller is
> > still unusable. Behavior is identical from a usability perspective but
> > the user is aware of the issue since the probe fails.
> > 
> 
> Ok. I think the description could reworded to make it more clear. The actual
> issue is that the default case lacks setting the errno, leading to probe
> success. But the addition of ks_pcie_set_mode() and others seem to cause little
> bit confusion.

I will update the commit message and keep it concise to highlight the
issue. I understand that the additional information provided in order to
set the context might have been counterproductive.

Thank you for reviewing the patch and providing feedback.

Regards,
Siddharth.

Re: [PATCH v2 10/10] PCI: keystone: Add support to build as a loadable module

[Siddharth Vadapalli]

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@ti.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@kernel.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.