public inbox for linux-arm-kernel@lists.infradead.org
 help / color / mirror / Atom feed
* [PATCH v5 0/4] PCI: Keystone: Enable loadable module support
@ 2025-10-29  8:04 Siddharth Vadapalli
  2025-10-29  8:04 ` [PATCH v5 1/4] PCI: Export pci_get_host_bridge_device() for use by pci-keystone Siddharth Vadapalli
                   ` (4 more replies)
  0 siblings, 5 replies; 13+ messages in thread
From: Siddharth Vadapalli @ 2025-10-29  8:04 UTC (permalink / raw)
  To: lpieralisi, kwilczynski, mani, robh, bhelgaas, jingoohan1,
	christian.bruel, krishna.chundru, qiang.yu, shradha.t,
	thippeswamy.havalige, inochiama, fan.ni, cassel, kishon,
	18255117159, rongqianfeng, jirislaby
  Cc: linux-pci, linux-kernel, linux-arm-kernel, srk, s-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 6.18-rc1 tag of Mainline Linux.

This series has __NO__ dependencies.

v4:
https://lore.kernel.org/r/20251022095724.997218-1-s-vadapalli@ti.com/
Changes since v4:
- To fix the build error on ARM32 platforms as reported at:
  https://lore.kernel.org/r/202510281008.jw19XuyP-lkp@intel.com/
  patch 4 in the series has been updated by introducing a new config
  named "PCI_KEYSTONE_TRISTATE" which allows building the driver as
  a loadable module. Additionally, this newly introduced config can
  be enabled only for non-ARM32 platforms. As a result, ARM32 platforms
  continue using the existing PCI_KEYSTONE config which is a bool, while
  ARM64 platforms can use PCI_KEYSTONE_TRISTATE which is a tristate, and
  can optionally enabled loadable module support being enabled by this
  series.

Series has been compile tested with W=1 for ARM32 using
arm-none-linux-gnueabihf-gcc

Series has been tested for functionality on an ARM64 platform (AM65 SoC):
AM654-EVM with an NVMe SSD connected to the PCIe Connector of the EVM.
Test Logs:
https://gist.github.com/Siddharth-Vadapalli-at-TI/474a16037d990206e1d22399a93dfee7

Regards,
Siddharth.

Siddharth Vadapalli (4):
  PCI: Export pci_get_host_bridge_device() for use by pci-keystone
  PCI: dwc: Export dw_pcie_allocate_domains() and
    dw_pcie_ep_raise_msix_irq()
  PCI: keystone: Exit ks_pcie_probe() for invalid mode
  PCI: keystone: Add support to build as a loadable module

 drivers/pci/controller/dwc/Kconfig            | 15 +++-
 drivers/pci/controller/dwc/Makefile           |  3 +
 drivers/pci/controller/dwc/pci-keystone.c     | 80 +++++++++++--------
 .../pci/controller/dwc/pcie-designware-ep.c   |  1 +
 .../pci/controller/dwc/pcie-designware-host.c |  1 +
 drivers/pci/host-bridge.c                     |  1 +
 include/linux/pci.h                           |  1 +
 7 files changed, 65 insertions(+), 37 deletions(-)

-- 
2.51.0



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v5 1/4] PCI: Export pci_get_host_bridge_device() for use by pci-keystone
  2025-10-29  8:04 [PATCH v5 0/4] PCI: Keystone: Enable loadable module support Siddharth Vadapalli
@ 2025-10-29  8:04 ` Siddharth Vadapalli
  2025-10-29  8:04 ` [PATCH v5 2/4] PCI: dwc: Export dw_pcie_allocate_domains() and dw_pcie_ep_raise_msix_irq() Siddharth Vadapalli
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Siddharth Vadapalli @ 2025-10-29  8:04 UTC (permalink / raw)
  To: lpieralisi, kwilczynski, mani, robh, bhelgaas, jingoohan1,
	christian.bruel, krishna.chundru, qiang.yu, shradha.t,
	thippeswamy.havalige, inochiama, fan.ni, cassel, kishon,
	18255117159, rongqianfeng, jirislaby
  Cc: linux-pci, linux-kernel, linux-arm-kernel, srk, s-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>
---

v4:
https://lore.kernel.org/r/20251022095724.997218-2-s-vadapalli@ti.com/
No Changes since v4.

Regards,
Siddharth.

 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.51.0



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v5 2/4] PCI: dwc: Export dw_pcie_allocate_domains() and dw_pcie_ep_raise_msix_irq()
  2025-10-29  8:04 [PATCH v5 0/4] PCI: Keystone: Enable loadable module support Siddharth Vadapalli
  2025-10-29  8:04 ` [PATCH v5 1/4] PCI: Export pci_get_host_bridge_device() for use by pci-keystone Siddharth Vadapalli
@ 2025-10-29  8:04 ` Siddharth Vadapalli
  2025-10-29  8:04 ` [PATCH v5 3/4] PCI: keystone: Exit ks_pcie_probe() for invalid mode Siddharth Vadapalli
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 13+ messages in thread
From: Siddharth Vadapalli @ 2025-10-29  8:04 UTC (permalink / raw)
  To: lpieralisi, kwilczynski, mani, robh, bhelgaas, jingoohan1,
	christian.bruel, krishna.chundru, qiang.yu, shradha.t,
	thippeswamy.havalige, inochiama, fan.ni, cassel, kishon,
	18255117159, rongqianfeng, jirislaby
  Cc: linux-pci, linux-kernel, linux-arm-kernel, srk, s-vadapalli

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

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

v4:
https://lore.kernel.org/r/20251022095724.997218-3-s-vadapalli@ti.com/
No Changes since v4.

Regards,
Siddharth.

 drivers/pci/controller/dwc/pcie-designware-ep.c   | 1 +
 drivers/pci/controller/dwc/pcie-designware-host.c | 1 +
 2 files changed, 2 insertions(+)

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
diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c b/drivers/pci/controller/dwc/pcie-designware-host.c
index e92513c5bda5..39fda91db27b 100644
--- a/drivers/pci/controller/dwc/pcie-designware-host.c
+++ b/drivers/pci/controller/dwc/pcie-designware-host.c
@@ -233,6 +233,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.51.0



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v5 3/4] PCI: keystone: Exit ks_pcie_probe() for invalid mode
  2025-10-29  8:04 [PATCH v5 0/4] PCI: Keystone: Enable loadable module support Siddharth Vadapalli
  2025-10-29  8:04 ` [PATCH v5 1/4] PCI: Export pci_get_host_bridge_device() for use by pci-keystone Siddharth Vadapalli
  2025-10-29  8:04 ` [PATCH v5 2/4] PCI: dwc: Export dw_pcie_allocate_domains() and dw_pcie_ep_raise_msix_irq() Siddharth Vadapalli
@ 2025-10-29  8:04 ` Siddharth Vadapalli
  2025-10-29  8:04 ` [PATCH v5 4/4] PCI: keystone: Add support to build as a loadable module Siddharth Vadapalli
  2025-11-06  8:10 ` [PATCH v5 0/4] PCI: Keystone: Enable loadable module support Manivannan Sadhasivam
  4 siblings, 0 replies; 13+ messages in thread
From: Siddharth Vadapalli @ 2025-10-29  8:04 UTC (permalink / raw)
  To: lpieralisi, kwilczynski, mani, robh, bhelgaas, jingoohan1,
	christian.bruel, krishna.chundru, qiang.yu, shradha.t,
	thippeswamy.havalige, inochiama, fan.ni, cassel, kishon,
	18255117159, rongqianfeng, jirislaby
  Cc: linux-pci, linux-kernel, linux-arm-kernel, srk, s-vadapalli

Commit under Fixes introduced support for PCIe EP mode on AM654x platforms.
When the mode happens to be either "DW_PCIE_RC_TYPE" or "DW_PCIE_EP_TYPE",
the PCIe Controller is configured accordingly. However, when the mode is
neither of them, an error message is displayed but the driver probe
succeeds. Since this "invalid" mode is not associated with a functional
PCIe Controller, the probe should fail.

Fix the behavior by exiting "ks_pcie_probe()" with the return value of
"-EINVAL" in addition to displaying the existing error message when the
mode is invalid.

Fixes: 23284ad677a9 ("PCI: keystone: Add support for PCIe EP in AM654x Platforms")
Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---

NOTE: As stated in the v4 patch, although a Fixes tag has been added,
the patch doesn't have to be backported. Hence, 'stable' hasn't been
CCed on purpose.

v4:
https://lore.kernel.org/r/20251022095724.997218-4-s-vadapalli@ti.com/
No Changes since v4.

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 eb00aa380722..25b8193ffbcf 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -1337,6 +1337,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.51.0



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v5 4/4] PCI: keystone: Add support to build as a loadable module
  2025-10-29  8:04 [PATCH v5 0/4] PCI: Keystone: Enable loadable module support Siddharth Vadapalli
                   ` (2 preceding siblings ...)
  2025-10-29  8:04 ` [PATCH v5 3/4] PCI: keystone: Exit ks_pcie_probe() for invalid mode Siddharth Vadapalli
@ 2025-10-29  8:04 ` Siddharth Vadapalli
  2025-11-05 17:23   ` Manivannan Sadhasivam
  2025-11-13 18:13   ` Bjorn Helgaas
  2025-11-06  8:10 ` [PATCH v5 0/4] PCI: Keystone: Enable loadable module support Manivannan Sadhasivam
  4 siblings, 2 replies; 13+ messages in thread
From: Siddharth Vadapalli @ 2025-10-29  8:04 UTC (permalink / raw)
  To: lpieralisi, kwilczynski, mani, robh, bhelgaas, jingoohan1,
	christian.bruel, krishna.chundru, qiang.yu, shradha.t,
	thippeswamy.havalige, inochiama, fan.ni, cassel, kishon,
	18255117159, rongqianfeng, jirislaby
  Cc: linux-pci, linux-kernel, linux-arm-kernel, srk, s-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.

Additionally, the functions marked by the '__init' keyword may be invoked:
a) After a probe deferral
OR
b) During a delayed probe - Delay attributed to driver being built as a
   loadable module

In both of the cases mentioned above, the '__init' memory will be freed
before the functions are invoked. This results in an exception of the form:

	Unable to handle kernel paging request at virtual address ...
	Mem abort info:
	...
	pc : ks_pcie_host_init+0x0/0x540
	lr : dw_pcie_host_init+0x170/0x498
	...
	ks_pcie_host_init+0x0/0x540 (P)
	ks_pcie_probe+0x728/0x84c
	platform_probe+0x5c/0x98
	really_probe+0xbc/0x29c
	__driver_probe_device+0x78/0x12c
	driver_probe_device+0xd8/0x15c
	...

To address this, introduce a new function namely 'ks_pcie_init()' to
register the 'fault handler' while removing the '__init' keyword from
existing functions.

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

v4:
https://lore.kernel.org/r/20251022095724.997218-5-s-vadapalli@ti.com/
Changes since v4:
- To fix the build error on ARM32 platforms as reported at:
  https://lore.kernel.org/r/202510281008.jw19XuyP-lkp@intel.com/
  patch 4 in the series has been updated by introducing a new config
  named "PCI_KEYSTONE_TRISTATE" which allows building the driver as
  a loadable module. Additionally, this newly introduced config can
  be enabled only for non-ARM32 platforms. As a result, ARM32 platforms
  continue using the existing PCI_KEYSTONE config which is a bool, while
  ARM64 platforms can use PCI_KEYSTONE_TRISTATE which is a tristate, and
  can optionally enabled loadable module support being enabled by this
  series.

Regards,
Siddharth.

 drivers/pci/controller/dwc/Kconfig        | 15 +++--
 drivers/pci/controller/dwc/Makefile       |  3 +
 drivers/pci/controller/dwc/pci-keystone.c | 78 +++++++++++++----------
 3 files changed, 59 insertions(+), 37 deletions(-)

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 349d4657393c..c5bc2f0b1f39 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -482,15 +482,21 @@ config PCI_DRA7XX_EP
 	  to enable device-specific features PCI_DRA7XX_EP must be selected.
 	  This uses the DesignWare core.
 
+# ARM32 platforms use hook_fault_code() and cannot support loadable module.
 config PCI_KEYSTONE
 	bool
 
+# On non-ARM32 platforms, loadable module can be supported.
+config PCI_KEYSTONE_TRISTATE
+	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
-	select PCI_KEYSTONE
+	select PCI_KEYSTONE if ARM
+	select PCI_KEYSTONE_TRISTATE if !ARM
 	help
 	  Enables support for the PCIe controller in the Keystone SoC to
 	  work in host mode. The PCI controller on Keystone is based on
@@ -498,11 +504,12 @@ 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
-	select PCI_KEYSTONE
+	select PCI_KEYSTONE if ARM
+	select PCI_KEYSTONE_TRISTATE if !ARM
 	help
 	  Enables support for the PCIe controller in the Keystone SoC to
 	  work in endpoint mode. The PCI controller on Keystone is based
diff --git a/drivers/pci/controller/dwc/Makefile b/drivers/pci/controller/dwc/Makefile
index 7ae28f3b0fb3..7c8de0067612 100644
--- a/drivers/pci/controller/dwc/Makefile
+++ b/drivers/pci/controller/dwc/Makefile
@@ -11,7 +11,10 @@ obj-$(CONFIG_PCI_EXYNOS) += pci-exynos.o
 obj-$(CONFIG_PCIE_FU740) += pcie-fu740.o
 obj-$(CONFIG_PCI_IMX6) += pci-imx6.o
 obj-$(CONFIG_PCIE_SPEAR13XX) += pcie-spear13xx.o
+# ARM32 platforms use hook_fault_code() and cannot support loadable module.
 obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone.o
+# On non-ARM32 platforms, loadable module can be supported.
+obj-$(CONFIG_PCI_KEYSTONE_TRISTATE) += pci-keystone.o
 obj-$(CONFIG_PCI_LAYERSCAPE) += pci-layerscape.o
 obj-$(CONFIG_PCI_LAYERSCAPE_EP) += pci-layerscape-ep.o
 obj-$(CONFIG_PCIE_QCOM_COMMON) += pcie-qcom-common.o
diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 25b8193ffbcf..53f88b31ad43 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>
@@ -777,29 +778,7 @@ static int ks_pcie_config_intx_irq(struct keystone_pcie *ks_pcie)
 	return ret;
 }
 
-#ifdef CONFIG_ARM
-/*
- * When a PCI device does not exist during config cycles, keystone host
- * gets a bus error instead of returning 0xffffffff (PCI_ERROR_RESPONSE).
- * This handler always returns 0 for this kind of fault.
- */
-static int ks_pcie_fault(unsigned long addr, unsigned int fsr,
-			 struct pt_regs *regs)
-{
-	unsigned long instr = *(unsigned long *) instruction_pointer(regs);
-
-	if ((instr & 0x0e100090) == 0x00100090) {
-		int reg = (instr >> 12) & 15;
-
-		regs->uregs[reg] = -1;
-		regs->ARM_pc += 4;
-	}
-
-	return 0;
-}
-#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;
@@ -831,7 +810,7 @@ static int __init ks_pcie_init_id(struct keystone_pcie *ks_pcie)
 	return 0;
 }
 
-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);
@@ -861,15 +840,6 @@ static int __init ks_pcie_host_init(struct dw_pcie_rp *pp)
 	if (ret < 0)
 		return ret;
 
-#ifdef CONFIG_ARM
-	/*
-	 * PCIe access errors that result into OCP errors are caught by ARM as
-	 * "External aborts"
-	 */
-	hook_fault_code(17, ks_pcie_fault, SIGBUS, 0,
-			"Asynchronous external abort");
-#endif
-
 	return 0;
 }
 
@@ -1134,6 +1104,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)
 {
@@ -1381,4 +1352,45 @@ static struct platform_driver ks_pcie_driver = {
 		.of_match_table = ks_pcie_of_match,
 	},
 };
+
+#ifdef CONFIG_ARM
+/*
+ * When a PCI device does not exist during config cycles, keystone host
+ * gets a bus error instead of returning 0xffffffff (PCI_ERROR_RESPONSE).
+ * This handler always returns 0 for this kind of fault.
+ */
+static int ks_pcie_fault(unsigned long addr, unsigned int fsr,
+			 struct pt_regs *regs)
+{
+	unsigned long instr = *(unsigned long *)instruction_pointer(regs);
+
+	if ((instr & 0x0e100090) == 0x00100090) {
+		int reg = (instr >> 12) & 15;
+
+		regs->uregs[reg] = -1;
+		regs->ARM_pc += 4;
+	}
+
+	return 0;
+}
+
+static int __init ks_pcie_init(void)
+{
+	/*
+	 * PCIe access errors that result into OCP errors are caught by ARM as
+	 * "External aborts"
+	 */
+	if (of_find_matching_node(NULL, ks_pcie_of_match))
+		hook_fault_code(17, ks_pcie_fault, SIGBUS, 0,
+				"Asynchronous external abort");
+
+	return platform_driver_register(&ks_pcie_driver);
+}
+device_initcall(ks_pcie_init);
+#else
 builtin_platform_driver(ks_pcie_driver);
+#endif
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("PCIe controller driver for Texas Instruments Keystone SoCs");
+MODULE_AUTHOR("Murali Karicheri <m-karicheri2@ti.com>");
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 4/4] PCI: keystone: Add support to build as a loadable module
  2025-10-29  8:04 ` [PATCH v5 4/4] PCI: keystone: Add support to build as a loadable module Siddharth Vadapalli
@ 2025-11-05 17:23   ` Manivannan Sadhasivam
  2025-11-06  7:01     ` Siddharth Vadapalli
  2025-11-13 18:13   ` Bjorn Helgaas
  1 sibling, 1 reply; 13+ messages in thread
From: Manivannan Sadhasivam @ 2025-11-05 17:23 UTC (permalink / raw)
  To: Siddharth Vadapalli
  Cc: lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
	christian.bruel, krishna.chundru, qiang.yu, shradha.t,
	thippeswamy.havalige, inochiama, fan.ni, cassel, kishon,
	18255117159, rongqianfeng, jirislaby, linux-pci, linux-kernel,
	linux-arm-kernel, srk

On Wed, Oct 29, 2025 at 01:34:52PM +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.
> 
> Additionally, the functions marked by the '__init' keyword may be invoked:
> a) After a probe deferral
> OR
> b) During a delayed probe - Delay attributed to driver being built as a
>    loadable module
> 
> In both of the cases mentioned above, the '__init' memory will be freed
> before the functions are invoked. This results in an exception of the form:
> 
> 	Unable to handle kernel paging request at virtual address ...
> 	Mem abort info:
> 	...
> 	pc : ks_pcie_host_init+0x0/0x540
> 	lr : dw_pcie_host_init+0x170/0x498
> 	...
> 	ks_pcie_host_init+0x0/0x540 (P)
> 	ks_pcie_probe+0x728/0x84c
> 	platform_probe+0x5c/0x98
> 	really_probe+0xbc/0x29c
> 	__driver_probe_device+0x78/0x12c
> 	driver_probe_device+0xd8/0x15c
> 	...
> 
> To address this, introduce a new function namely 'ks_pcie_init()' to
> register the 'fault handler' while removing the '__init' keyword from
> existing functions.
> 
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> ---
> 
> v4:
> https://lore.kernel.org/r/20251022095724.997218-5-s-vadapalli@ti.com/
> Changes since v4:
> - To fix the build error on ARM32 platforms as reported at:
>   https://lore.kernel.org/r/202510281008.jw19XuyP-lkp@intel.com/
>   patch 4 in the series has been updated by introducing a new config
>   named "PCI_KEYSTONE_TRISTATE" which allows building the driver as
>   a loadable module. Additionally, this newly introduced config can
>   be enabled only for non-ARM32 platforms. As a result, ARM32 platforms
>   continue using the existing PCI_KEYSTONE config which is a bool, while
>   ARM64 platforms can use PCI_KEYSTONE_TRISTATE which is a tristate, and
>   can optionally enabled loadable module support being enabled by this
>   series.
> 
> Regards,
> Siddharth.
> 
>  drivers/pci/controller/dwc/Kconfig        | 15 +++--
>  drivers/pci/controller/dwc/Makefile       |  3 +
>  drivers/pci/controller/dwc/pci-keystone.c | 78 +++++++++++++----------
>  3 files changed, 59 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 349d4657393c..c5bc2f0b1f39 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -482,15 +482,21 @@ config PCI_DRA7XX_EP
>  	  to enable device-specific features PCI_DRA7XX_EP must be selected.
>  	  This uses the DesignWare core.
>  
> +# ARM32 platforms use hook_fault_code() and cannot support loadable module.
>  config PCI_KEYSTONE
>  	bool
>  
> +# On non-ARM32 platforms, loadable module can be supported.
> +config PCI_KEYSTONE_TRISTATE
> +	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
> -	select PCI_KEYSTONE
> +	select PCI_KEYSTONE if ARM
> +	select PCI_KEYSTONE_TRISTATE if !ARM

Wouldn't below change work for you?

diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
index 349d4657393c..b1219e7136c9 100644
--- a/drivers/pci/controller/dwc/Kconfig
+++ b/drivers/pci/controller/dwc/Kconfig
@@ -486,8 +486,9 @@ config PCI_KEYSTONE
        bool
 
 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
+       default y if ARCH_KEYSTONE
        depends on PCI_MSI
        select PCIE_DW_HOST
        select PCI_KEYSTONE

- Mani

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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 4/4] PCI: keystone: Add support to build as a loadable module
  2025-11-05 17:23   ` Manivannan Sadhasivam
@ 2025-11-06  7:01     ` Siddharth Vadapalli
  2025-11-06  8:00       ` Manivannan Sadhasivam
  0 siblings, 1 reply; 13+ messages in thread
From: Siddharth Vadapalli @ 2025-11-06  7:01 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
	christian.bruel, krishna.chundru, qiang.yu, shradha.t,
	thippeswamy.havalige, inochiama, fan.ni, cassel, kishon,
	18255117159, rongqianfeng, jirislaby, linux-pci, linux-kernel,
	linux-arm-kernel, srk, s-vadapalli

On Wed, 2025-11-05 at 22:53 +0530, Manivannan Sadhasivam wrote:
> On Wed, Oct 29, 2025 at 01:34:52PM +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.
> > 
> > Additionally, the functions marked by the '__init' keyword may be invoked:
> > a) After a probe deferral
> > OR
> > b) During a delayed probe - Delay attributed to driver being built as a
> >    loadable module
> > 
> > In both of the cases mentioned above, the '__init' memory will be freed
> > before the functions are invoked. This results in an exception of the form:
> > 
> > 	Unable to handle kernel paging request at virtual address ...
> > 	Mem abort info:
> > 	...
> > 	pc : ks_pcie_host_init+0x0/0x540
> > 	lr : dw_pcie_host_init+0x170/0x498
> > 	...
> > 	ks_pcie_host_init+0x0/0x540 (P)
> > 	ks_pcie_probe+0x728/0x84c
> > 	platform_probe+0x5c/0x98
> > 	really_probe+0xbc/0x29c
> > 	__driver_probe_device+0x78/0x12c
> > 	driver_probe_device+0xd8/0x15c
> > 	...
> > 
> > To address this, introduce a new function namely 'ks_pcie_init()' to
> > register the 'fault handler' while removing the '__init' keyword from
> > existing functions.
> > 
> > Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> > ---
> > 
> > v4:
> > https://lore.kernel.org/r/20251022095724.997218-5-s-vadapalli@ti.com/
> > Changes since v4:
> > - To fix the build error on ARM32 platforms as reported at:
> >   https://lore.kernel.org/r/202510281008.jw19XuyP-lkp@intel.com/
> >   patch 4 in the series has been updated by introducing a new config
> >   named "PCI_KEYSTONE_TRISTATE" which allows building the driver as
> >   a loadable module. Additionally, this newly introduced config can
> >   be enabled only for non-ARM32 platforms. As a result, ARM32 platforms
> >   continue using the existing PCI_KEYSTONE config which is a bool, while
> >   ARM64 platforms can use PCI_KEYSTONE_TRISTATE which is a tristate, and
> >   can optionally enabled loadable module support being enabled by this
> >   series.
> > 
> > Regards,
> > Siddharth.
> > 
> >  drivers/pci/controller/dwc/Kconfig        | 15 +++--
> >  drivers/pci/controller/dwc/Makefile       |  3 +
> >  drivers/pci/controller/dwc/pci-keystone.c | 78 +++++++++++++----------
> >  3 files changed, 59 insertions(+), 37 deletions(-)
> > 
> > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > index 349d4657393c..c5bc2f0b1f39 100644
> > --- a/drivers/pci/controller/dwc/Kconfig
> > +++ b/drivers/pci/controller/dwc/Kconfig
> > @@ -482,15 +482,21 @@ config PCI_DRA7XX_EP
> >  	  to enable device-specific features PCI_DRA7XX_EP must be selected.
> >  	  This uses the DesignWare core.
> >  
> > +# ARM32 platforms use hook_fault_code() and cannot support loadable module.
> >  config PCI_KEYSTONE
> >  	bool
> >  
> > +# On non-ARM32 platforms, loadable module can be supported.
> > +config PCI_KEYSTONE_TRISTATE
> > +	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
> > -	select PCI_KEYSTONE
> > +	select PCI_KEYSTONE if ARM
> > +	select PCI_KEYSTONE_TRISTATE if !ARM
> 
> Wouldn't below change work for you?
> 
> diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> index 349d4657393c..b1219e7136c9 100644
> --- a/drivers/pci/controller/dwc/Kconfig
> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -486,8 +486,9 @@ config PCI_KEYSTONE
>         bool
>  
>  config PCI_KEYSTONE_HOST
> -       bool "TI Keystone PCIe controller (host mode)"
> +       tristate "TI Keystone PCIe controller (host mode)"

This doesn't alter the build of the pci-keystone.c driver. From the
existing Makefile, we have:
 obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone.o
implying that it is only CONFIG_PCI_KEYSTONE that determines whether the
pci-keystone.c driver is built as a loadable module or a built-in module.

>         depends on ARCH_KEYSTONE || ARCH_K3 || COMPILE_TEST
> +       default y if ARCH_KEYSTONE

The default flag only specifies what should be selected by default, but it
doesn't prevent the user from attempting to built it as a loadable module
using menuconfig. Building the pci-keystone.c driver as a loadable module
(CONFIG_PCI_KEYSTONE set to 'm') will cause build errors for ARM32
platforms due to the presence of hook_fault_code() which is __init code.

The Kconfig and Makefile changes made by the patch do the following:
1. Allow building the pci-keystone.c driver as a loadable module for non-
ARM32 platforms by introducing the PCI_KEYSTONE_TRISTATE config which is a
tristate unlike the existing PCI_KEYSTONE config which is a bool.
2. Associate PCI_KEYSTONE with ARM32 platforms and associate
PCI_KEYSTONE_TRISTATE with non-ARM32 platforms to prevent users from
building the pci-keystone.c driver as a loadable module for ARM32
platforms.

Regards,
Siddharth.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 4/4] PCI: keystone: Add support to build as a loadable module
  2025-11-06  7:01     ` Siddharth Vadapalli
@ 2025-11-06  8:00       ` Manivannan Sadhasivam
  0 siblings, 0 replies; 13+ messages in thread
From: Manivannan Sadhasivam @ 2025-11-06  8:00 UTC (permalink / raw)
  To: Siddharth Vadapalli
  Cc: lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
	christian.bruel, krishna.chundru, qiang.yu, shradha.t,
	thippeswamy.havalige, inochiama, fan.ni, cassel, kishon,
	18255117159, rongqianfeng, jirislaby, linux-pci, linux-kernel,
	linux-arm-kernel, srk

On Thu, Nov 06, 2025 at 12:31:08PM +0530, Siddharth Vadapalli wrote:
> On Wed, 2025-11-05 at 22:53 +0530, Manivannan Sadhasivam wrote:
> > On Wed, Oct 29, 2025 at 01:34:52PM +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.
> > > 
> > > Additionally, the functions marked by the '__init' keyword may be invoked:
> > > a) After a probe deferral
> > > OR
> > > b) During a delayed probe - Delay attributed to driver being built as a
> > >    loadable module
> > > 
> > > In both of the cases mentioned above, the '__init' memory will be freed
> > > before the functions are invoked. This results in an exception of the form:
> > > 
> > > 	Unable to handle kernel paging request at virtual address ...
> > > 	Mem abort info:
> > > 	...
> > > 	pc : ks_pcie_host_init+0x0/0x540
> > > 	lr : dw_pcie_host_init+0x170/0x498
> > > 	...
> > > 	ks_pcie_host_init+0x0/0x540 (P)
> > > 	ks_pcie_probe+0x728/0x84c
> > > 	platform_probe+0x5c/0x98
> > > 	really_probe+0xbc/0x29c
> > > 	__driver_probe_device+0x78/0x12c
> > > 	driver_probe_device+0xd8/0x15c
> > > 	...
> > > 
> > > To address this, introduce a new function namely 'ks_pcie_init()' to
> > > register the 'fault handler' while removing the '__init' keyword from
> > > existing functions.
> > > 
> > > Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> > > ---
> > > 
> > > v4:
> > > https://lore.kernel.org/r/20251022095724.997218-5-s-vadapalli@ti.com/
> > > Changes since v4:
> > > - To fix the build error on ARM32 platforms as reported at:
> > >   https://lore.kernel.org/r/202510281008.jw19XuyP-lkp@intel.com/
> > >   patch 4 in the series has been updated by introducing a new config
> > >   named "PCI_KEYSTONE_TRISTATE" which allows building the driver as
> > >   a loadable module. Additionally, this newly introduced config can
> > >   be enabled only for non-ARM32 platforms. As a result, ARM32 platforms
> > >   continue using the existing PCI_KEYSTONE config which is a bool, while
> > >   ARM64 platforms can use PCI_KEYSTONE_TRISTATE which is a tristate, and
> > >   can optionally enabled loadable module support being enabled by this
> > >   series.
> > > 
> > > Regards,
> > > Siddharth.
> > > 
> > >  drivers/pci/controller/dwc/Kconfig        | 15 +++--
> > >  drivers/pci/controller/dwc/Makefile       |  3 +
> > >  drivers/pci/controller/dwc/pci-keystone.c | 78 +++++++++++++----------
> > >  3 files changed, 59 insertions(+), 37 deletions(-)
> > > 
> > > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > > index 349d4657393c..c5bc2f0b1f39 100644
> > > --- a/drivers/pci/controller/dwc/Kconfig
> > > +++ b/drivers/pci/controller/dwc/Kconfig
> > > @@ -482,15 +482,21 @@ config PCI_DRA7XX_EP
> > >  	  to enable device-specific features PCI_DRA7XX_EP must be selected.
> > >  	  This uses the DesignWare core.
> > >  
> > > +# ARM32 platforms use hook_fault_code() and cannot support loadable module.
> > >  config PCI_KEYSTONE
> > >  	bool
> > >  
> > > +# On non-ARM32 platforms, loadable module can be supported.
> > > +config PCI_KEYSTONE_TRISTATE
> > > +	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
> > > -	select PCI_KEYSTONE
> > > +	select PCI_KEYSTONE if ARM
> > > +	select PCI_KEYSTONE_TRISTATE if !ARM
> > 
> > Wouldn't below change work for you?
> > 
> > diff --git a/drivers/pci/controller/dwc/Kconfig b/drivers/pci/controller/dwc/Kconfig
> > index 349d4657393c..b1219e7136c9 100644
> > --- a/drivers/pci/controller/dwc/Kconfig
> > +++ b/drivers/pci/controller/dwc/Kconfig
> > @@ -486,8 +486,9 @@ config PCI_KEYSTONE
> >         bool
> >  
> >  config PCI_KEYSTONE_HOST
> > -       bool "TI Keystone PCIe controller (host mode)"
> > +       tristate "TI Keystone PCIe controller (host mode)"
> 
> This doesn't alter the build of the pci-keystone.c driver. From the
> existing Makefile, we have:
>  obj-$(CONFIG_PCI_KEYSTONE) += pci-keystone.o
> implying that it is only CONFIG_PCI_KEYSTONE that determines whether the
> pci-keystone.c driver is built as a loadable module or a built-in module.
> 

Ah, I missed the fact that PCI_KEYSTONE_HOST is just used inside the driver and
not in the Makefile.

> >         depends on ARCH_KEYSTONE || ARCH_K3 || COMPILE_TEST
> > +       default y if ARCH_KEYSTONE
> 
> The default flag only specifies what should be selected by default, but it
> doesn't prevent the user from attempting to built it as a loadable module
> using menuconfig. Building the pci-keystone.c driver as a loadable module
> (CONFIG_PCI_KEYSTONE set to 'm') will cause build errors for ARM32
> platforms due to the presence of hook_fault_code() which is __init code.
> 
> The Kconfig and Makefile changes made by the patch do the following:
> 1. Allow building the pci-keystone.c driver as a loadable module for non-
> ARM32 platforms by introducing the PCI_KEYSTONE_TRISTATE config which is a
> tristate unlike the existing PCI_KEYSTONE config which is a bool.
> 2. Associate PCI_KEYSTONE with ARM32 platforms and associate
> PCI_KEYSTONE_TRISTATE with non-ARM32 platforms to prevent users from
> building the pci-keystone.c driver as a loadable module for ARM32
> platforms.
> 

Ok. I don't have a better solution to this problem. So fine with me.

- Mani

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


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 0/4] PCI: Keystone: Enable loadable module support
  2025-10-29  8:04 [PATCH v5 0/4] PCI: Keystone: Enable loadable module support Siddharth Vadapalli
                   ` (3 preceding siblings ...)
  2025-10-29  8:04 ` [PATCH v5 4/4] PCI: keystone: Add support to build as a loadable module Siddharth Vadapalli
@ 2025-11-06  8:10 ` Manivannan Sadhasivam
  2025-11-13 18:25   ` Bjorn Helgaas
  4 siblings, 1 reply; 13+ messages in thread
From: Manivannan Sadhasivam @ 2025-11-06  8:10 UTC (permalink / raw)
  To: lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
	christian.bruel, krishna.chundru, qiang.yu, shradha.t,
	thippeswamy.havalige, inochiama, fan.ni, cassel, kishon,
	18255117159, rongqianfeng, jirislaby, Siddharth Vadapalli
  Cc: linux-pci, linux-kernel, linux-arm-kernel, srk


On Wed, 29 Oct 2025 13:34:48 +0530, Siddharth Vadapalli wrote:
> 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 6.18-rc1 tag of Mainline Linux.
> 
> [...]

Applied, thanks!

[1/4] PCI: Export pci_get_host_bridge_device() for use by pci-keystone
      commit: 88254d46823be8563f8f81d78390a7313ae6fad7
[2/4] PCI: dwc: Export dw_pcie_allocate_domains() and dw_pcie_ep_raise_msix_irq()
      commit: 9acc60a5bca02351f852651c0123d9994663ec0a
[3/4] PCI: keystone: Exit ks_pcie_probe() for invalid mode
      commit: 7b5a5b7715c2dea2541e7fd3da15c2881fdfc553
[4/4] PCI: keystone: Add support to build as a loadable module
      commit: 041c2f0e34ba4823101bf307d6a6d41d98f5dac3

Best regards,
-- 
Manivannan Sadhasivam <mani@kernel.org>



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 4/4] PCI: keystone: Add support to build as a loadable module
  2025-10-29  8:04 ` [PATCH v5 4/4] PCI: keystone: Add support to build as a loadable module Siddharth Vadapalli
  2025-11-05 17:23   ` Manivannan Sadhasivam
@ 2025-11-13 18:13   ` Bjorn Helgaas
  2025-11-13 18:35     ` Russell King (Oracle)
  1 sibling, 1 reply; 13+ messages in thread
From: Bjorn Helgaas @ 2025-11-13 18:13 UTC (permalink / raw)
  To: Siddharth Vadapalli, Russell King
  Cc: lpieralisi, kwilczynski, mani, robh, bhelgaas, jingoohan1,
	christian.bruel, krishna.chundru, qiang.yu, shradha.t,
	thippeswamy.havalige, inochiama, fan.ni, cassel, kishon,
	18255117159, rongqianfeng, jirislaby, linux-pci, linux-kernel,
	linux-arm-kernel, srk

[+to Russell, hook_fault_code() __init-ness]

On Wed, Oct 29, 2025 at 01:34:52PM +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.
> 
> Additionally, the functions marked by the '__init' keyword may be invoked:
> a) After a probe deferral
> OR
> b) During a delayed probe - Delay attributed to driver being built as a
>    loadable module
> 
> In both of the cases mentioned above, the '__init' memory will be freed
> before the functions are invoked. This results in an exception of the form:
> 
> 	Unable to handle kernel paging request at virtual address ...
> 	Mem abort info:
> 	...
> 	pc : ks_pcie_host_init+0x0/0x540
> 	lr : dw_pcie_host_init+0x170/0x498
> 	...
> 	ks_pcie_host_init+0x0/0x540 (P)
> 	ks_pcie_probe+0x728/0x84c
> 	platform_probe+0x5c/0x98
> 	really_probe+0xbc/0x29c
> 	__driver_probe_device+0x78/0x12c
> 	driver_probe_device+0xd8/0x15c
> 	...
> 
> To address this, introduce a new function namely 'ks_pcie_init()' to
> register the 'fault handler' while removing the '__init' keyword from
> existing functions.
> ...

> +++ b/drivers/pci/controller/dwc/Kconfig
> @@ -482,15 +482,21 @@ config PCI_DRA7XX_EP
>  	  to enable device-specific features PCI_DRA7XX_EP must be selected.
>  	  This uses the DesignWare core.
>  
> +# ARM32 platforms use hook_fault_code() and cannot support loadable module.
>  config PCI_KEYSTONE
>  	bool
>  
> +# On non-ARM32 platforms, loadable module can be supported.
> +config PCI_KEYSTONE_TRISTATE
> +	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
> -	select PCI_KEYSTONE
> +	select PCI_KEYSTONE if ARM
> +	select PCI_KEYSTONE_TRISTATE if !ARM

This is kind of a lot of dancing to make keystone built-in on ARM32
because hook_fault_code() is __init, while making it modular
everywhere else.

Is hook_fault_code() __init for some intrinsic reason?  All the
existing callers are __init, so that's one reason.  But could it be
made non-__init?

There are several drivers that use hook_fault_code() and could
potentially be modular (imx6, keystone, ixp4xx, rcar).

Bjorn


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 0/4] PCI: Keystone: Enable loadable module support
  2025-11-06  8:10 ` [PATCH v5 0/4] PCI: Keystone: Enable loadable module support Manivannan Sadhasivam
@ 2025-11-13 18:25   ` Bjorn Helgaas
  0 siblings, 0 replies; 13+ messages in thread
From: Bjorn Helgaas @ 2025-11-13 18:25 UTC (permalink / raw)
  To: Manivannan Sadhasivam
  Cc: lpieralisi, kwilczynski, robh, bhelgaas, jingoohan1,
	christian.bruel, krishna.chundru, qiang.yu, shradha.t,
	thippeswamy.havalige, inochiama, fan.ni, cassel, kishon,
	18255117159, rongqianfeng, jirislaby, Siddharth Vadapalli,
	linux-pci, linux-kernel, linux-arm-kernel, srk

On Thu, Nov 06, 2025 at 01:40:10PM +0530, Manivannan Sadhasivam wrote:
> 
> On Wed, 29 Oct 2025 13:34:48 +0530, Siddharth Vadapalli wrote:
> > 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 6.18-rc1 tag of Mainline Linux.
> > 
> > [...]
> 
> Applied, thanks!
> 
> [1/4] PCI: Export pci_get_host_bridge_device() for use by pci-keystone
>       commit: 88254d46823be8563f8f81d78390a7313ae6fad7
> [2/4] PCI: dwc: Export dw_pcie_allocate_domains() and dw_pcie_ep_raise_msix_irq()
>       commit: 9acc60a5bca02351f852651c0123d9994663ec0a
> [3/4] PCI: keystone: Exit ks_pcie_probe() for invalid mode
>       commit: 7b5a5b7715c2dea2541e7fd3da15c2881fdfc553
> [4/4] PCI: keystone: Add support to build as a loadable module
>       commit: 041c2f0e34ba4823101bf307d6a6d41d98f5dac3

Just FYI, I moved these to pci/controller/keystone to separate them
from the j721e changes on pci/controller/ti.  It's a little easier for
me when each driver is on a separate topic branch.

I also reordered them so all the module-related changes are together
instead of having the invalid mode bug fix in the middle of them.


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 4/4] PCI: keystone: Add support to build as a loadable module
  2025-11-13 18:13   ` Bjorn Helgaas
@ 2025-11-13 18:35     ` Russell King (Oracle)
  2025-11-13 18:48       ` Bjorn Helgaas
  0 siblings, 1 reply; 13+ messages in thread
From: Russell King (Oracle) @ 2025-11-13 18:35 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Siddharth Vadapalli, lpieralisi, kwilczynski, mani, robh,
	bhelgaas, jingoohan1, christian.bruel, krishna.chundru, qiang.yu,
	shradha.t, thippeswamy.havalige, inochiama, fan.ni, cassel,
	kishon, 18255117159, rongqianfeng, jirislaby, linux-pci,
	linux-kernel, linux-arm-kernel, srk

On Thu, Nov 13, 2025 at 12:13:55PM -0600, Bjorn Helgaas wrote:
> >  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
> > -	select PCI_KEYSTONE
> > +	select PCI_KEYSTONE if ARM
> > +	select PCI_KEYSTONE_TRISTATE if !ARM
> 
> This is kind of a lot of dancing to make keystone built-in on ARM32
> because hook_fault_code() is __init, while making it modular
> everywhere else.
> 
> Is hook_fault_code() __init for some intrinsic reason?  All the
> existing callers are __init, so that's one reason.  But could it be
> made non-__init?

Yes. To discourage use in modules, because there is *no* way to safely
remove a hook.

While one can call hook_fault_code() with a NULL handler, that doesn't
mean that another CPU isn't executing in that function. If that code
gets unmapped while another CPU is executing it (because of a module
being unmapped) then we'll get another fault.

Trying to throw locks at this doesn't help - not without holding locks
over the execution of the called function, which *will* be extremely
detrimental on all fault handling, and probably introduce deadlocks.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v5 4/4] PCI: keystone: Add support to build as a loadable module
  2025-11-13 18:35     ` Russell King (Oracle)
@ 2025-11-13 18:48       ` Bjorn Helgaas
  0 siblings, 0 replies; 13+ messages in thread
From: Bjorn Helgaas @ 2025-11-13 18:48 UTC (permalink / raw)
  To: Russell King (Oracle)
  Cc: Siddharth Vadapalli, lpieralisi, kwilczynski, mani, robh,
	bhelgaas, jingoohan1, christian.bruel, krishna.chundru, qiang.yu,
	shradha.t, thippeswamy.havalige, inochiama, fan.ni, cassel,
	kishon, 18255117159, rongqianfeng, jirislaby, linux-pci,
	linux-kernel, linux-arm-kernel, srk

On Thu, Nov 13, 2025 at 06:35:13PM +0000, Russell King (Oracle) wrote:
> On Thu, Nov 13, 2025 at 12:13:55PM -0600, Bjorn Helgaas wrote:
> > >  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
> > > -	select PCI_KEYSTONE
> > > +	select PCI_KEYSTONE if ARM
> > > +	select PCI_KEYSTONE_TRISTATE if !ARM
> > 
> > This is kind of a lot of dancing to make keystone built-in on ARM32
> > because hook_fault_code() is __init, while making it modular
> > everywhere else.
> > 
> > Is hook_fault_code() __init for some intrinsic reason?  All the
> > existing callers are __init, so that's one reason.  But could it be
> > made non-__init?
> 
> Yes. To discourage use in modules, because there is *no* way to safely
> remove a hook.
> 
> While one can call hook_fault_code() with a NULL handler, that doesn't
> mean that another CPU isn't executing in that function. If that code
> gets unmapped while another CPU is executing it (because of a module
> being unmapped) then we'll get another fault.
> 
> Trying to throw locks at this doesn't help - not without holding locks
> over the execution of the called function, which *will* be extremely
> detrimental on all fault handling, and probably introduce deadlocks.

Ah, thanks, I hadn't thought about the removal problem.


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2025-11-13 18:48 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-29  8:04 [PATCH v5 0/4] PCI: Keystone: Enable loadable module support Siddharth Vadapalli
2025-10-29  8:04 ` [PATCH v5 1/4] PCI: Export pci_get_host_bridge_device() for use by pci-keystone Siddharth Vadapalli
2025-10-29  8:04 ` [PATCH v5 2/4] PCI: dwc: Export dw_pcie_allocate_domains() and dw_pcie_ep_raise_msix_irq() Siddharth Vadapalli
2025-10-29  8:04 ` [PATCH v5 3/4] PCI: keystone: Exit ks_pcie_probe() for invalid mode Siddharth Vadapalli
2025-10-29  8:04 ` [PATCH v5 4/4] PCI: keystone: Add support to build as a loadable module Siddharth Vadapalli
2025-11-05 17:23   ` Manivannan Sadhasivam
2025-11-06  7:01     ` Siddharth Vadapalli
2025-11-06  8:00       ` Manivannan Sadhasivam
2025-11-13 18:13   ` Bjorn Helgaas
2025-11-13 18:35     ` Russell King (Oracle)
2025-11-13 18:48       ` Bjorn Helgaas
2025-11-06  8:10 ` [PATCH v5 0/4] PCI: Keystone: Enable loadable module support Manivannan Sadhasivam
2025-11-13 18:25   ` Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox