1. Patchew
  2. linux
  3. [PATCH v5 0/4] PCI: Keystone: Enable loadable module support
  4. View patch

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

Siddharth Vadapalli posted 4 patches 1 month, 2 weeks ago
[PATCH v5 4/4] PCI: keystone: Add support to build as a loadable module
Posted by Siddharth Vadapalli 1 month, 2 weeks ago 
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
Re: [PATCH v5 4/4] PCI: keystone: Add support to build as a loadable module
Posted by Bjorn Helgaas 1 month ago 
[+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
Re: [PATCH v5 4/4] PCI: keystone: Add support to build as a loadable module
Posted by Russell King (Oracle) 1 month ago 
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!
Re: [PATCH v5 4/4] PCI: keystone: Add support to build as a loadable module
Posted by Bjorn Helgaas 1 month ago 
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.
Re: [PATCH v5 4/4] PCI: keystone: Add support to build as a loadable module
Posted by Manivannan Sadhasivam 1 month, 1 week ago 
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

-- 
மணிவண்ணன் சதாசிவம்
Re: [PATCH v5 4/4] PCI: keystone: Add support to build as a loadable module
Posted by Siddharth Vadapalli 1 month, 1 week ago 
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.
Re: [PATCH v5 4/4] PCI: keystone: Add support to build as a loadable module
Posted by Manivannan Sadhasivam 1 month, 1 week ago 
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

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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.