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

[Siddharth Vadapalli <s-vadapalli@ti.com>]

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.