[PATCH 0/2] PCI: Keystone: __init and IRQ Fixes

[PATCH 0/2] PCI: Keystone: __init and IRQ Fixes

[Siddharth Vadapalli]

Hello,

This series is based on commit
320475fbd590 Merge tag 'mtd/fixes-for-6.17-rc6' of git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux
of Mainline Linux.

The first patch in the series has been posted as a Fix in contrast to
its predecessor at:
https://lore.kernel.org/r/20250903124505.365913-10-s-vadapalli@ti.com/
based on the feedback provided by Jiri Slaby <jirislaby@kernel.org> at:
https://lore.kernel.org/r/3d3a4b52-e343-42f3-9d69-94c259812143@kernel.org/
Since the Fix is independent of enabling loadable module support for the
pci-keystone.c driver, it is being posted as a new patch.

Checking out at the commit of Mainline Linux which this series is based
on, I noticed an exception triggered by the pci-keystone.c driver during
its probe. Although this is not a fatal exception and Linux continues to
boot, the driver is non-functional. I root-caused the exception to
free_initmem() freeing the memory associated with the ks_pcie_host_init()
function in the driver before the driver's probe was invoked. This
appears to be a race condition but it is easily reproducible with the
Linux .config that I have used. The fix therefore is to remove the
__init macro which is implemented by the second patch in the series.

For reference, the logs for the case where Linux is built by checking
out at the base commit of Mainline Linux are:
https://gist.github.com/Siddharth-Vadapalli-at-TI/f4891b707921c53dfb464ad2f3a968bf
and the logs clearly prove that the print associated with free_initmem()
which is:
[    2.446834] Freeing unused kernel memory: 4864K
is displayed prior to the prints associated with the pci-keystone.c
driver being probed which is:
[    7.707103] keystone-pcie 5500000.pcie: host bridge /bus@100000/pcie@5500000 ranges:

Building Linux by applying both patches in the series on the base commit of
Mainline Linux, the driver probes successfully without any exceptions or
errors. This was tested on AM654-EVM with an NVMe SSD connected to the
PCIe Connector on the board. The NVMe SSD enumerates successfully.
Additionally, the 'hdparm' utility was used to read from the SSD
confirming that the SSD is functional. The logs corresponding to this are:
https://gist.github.com/Siddharth-Vadapalli-at-TI/1b09a12a53db4233e82c5bcfc0e89214

Regards,
Siddharth.

Siddharth Vadapalli (2):
  PCI: keystone: Use devm_request_irq() to free "ks-pcie-error-irq" on
    exit
  PCI: keystone: Remove the __init macro for the ks_pcie_host_init()
    callback

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

-- 
2.43.0

[PATCH 1/2] PCI: keystone: Use devm_request_irq() to free "ks-pcie-error-irq" on exit

[Siddharth Vadapalli]

Commit under Fixes introduced the IRQ handler for "ks-pcie-error-irq".
The interrupt is acquired using "request_irq()" but is never freed if
the driver exits due to an error. Although the section in the driver that
invokes "request_irq()" has moved around over time, the issue hasn't been
addressed until now.

Fix this by using "devm_request_irq()" which shall automatically release
the interrupt if the driver exits. Also, since the interrupt handler for
the "ks-pcie-error-irq" namely "ks_pcie_handle_error_irq() is only printing
the error and clearing the interrupt, there is no necessity to prefer
devm_request_threaded_irq() over devm_request_irq().

Fixes: 025dd3daeda7 ("PCI: keystone: Add error IRQ handler")
Reported-by: Jiri Slaby <jirislaby@kernel.org>
Closes: https://lore.kernel.org/r/3d3a4b52-e343-42f3-9d69-94c259812143@kernel.org
Cc: <stable@vger.kernel.org>
Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/pci/controller/dwc/pci-keystone.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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

[PATCH 2/2] PCI: keystone: Remove the __init macro for the ks_pcie_host_init() callback

[Siddharth Vadapalli]

The ks_pcie_host_init() callback registered by the driver is invoked by
dw_pcie_host_init(). Since the driver probe is not guaranteed to finish
before the kernel initialization phase, the memory associated with
ks_pcie_host_init() may already be freed by free_initmem().

It is observed in practice that the print associated with free_initmem()
which is:
	"Freeing unused kernel memory: ..."
is displayed before the driver is probed, following which an exception is
triggered when ks_pcie_host_init() is invoked which looks like:

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

Fix this by removing the "__init" macro associated with the
ks_pcie_host_init() callback and the ks_pcie_init_id() function that it
internally invokes.

Fixes: 0c4ffcfe1fbc ("PCI: keystone: Add TI Keystone PCIe driver")
Cc: <stable@vger.kernel.org>
Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
---
 drivers/pci/controller/dwc/pci-keystone.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
index 21808a9e5158..c6e082dcb3bc 100644
--- a/drivers/pci/controller/dwc/pci-keystone.c
+++ b/drivers/pci/controller/dwc/pci-keystone.c
@@ -799,7 +799,7 @@ static int ks_pcie_fault(unsigned long addr, unsigned int fsr,
 }
 #endif
 
-static int __init ks_pcie_init_id(struct keystone_pcie *ks_pcie)
+static int ks_pcie_init_id(struct keystone_pcie *ks_pcie)
 {
 	int ret;
 	unsigned int id;
@@ -831,7 +831,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);
-- 
2.43.0

Re: [PATCH 0/2] PCI: Keystone: __init and IRQ Fixes

[Manivannan Sadhasivam]

On Fri, 12 Sep 2025 15:37:57 +0530, Siddharth Vadapalli wrote:
> This series is based on commit
> 320475fbd590 Merge tag 'mtd/fixes-for-6.17-rc6' of git://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux
> of Mainline Linux.
> 
> The first patch in the series has been posted as a Fix in contrast to
> its predecessor at:
> https://lore.kernel.org/r/20250903124505.365913-10-s-vadapalli@ti.com/
> based on the feedback provided by Jiri Slaby <jirislaby@kernel.org> at:
> https://lore.kernel.org/r/3d3a4b52-e343-42f3-9d69-94c259812143@kernel.org/
> Since the Fix is independent of enabling loadable module support for the
> pci-keystone.c driver, it is being posted as a new patch.
> 
> [...]

Applied, thanks!

[1/2] PCI: keystone: Use devm_request_irq() to free "ks-pcie-error-irq" on exit
      commit: e51d05f523e43ce5d2bad957943a2b14f68078cd
[2/2] PCI: keystone: Remove the __init macro for the ks_pcie_host_init() callback
      commit: 860daf4ba3c034995bafa4c3756942262a9cd32d

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

Re: [PATCH 2/2] PCI: keystone: Remove the __init macro for the ks_pcie_host_init() callback

[Bjorn Helgaas]

On Fri, Sep 12, 2025 at 03:37:59PM +0530, Siddharth Vadapalli wrote:
> The ks_pcie_host_init() callback registered by the driver is invoked by
> dw_pcie_host_init(). Since the driver probe is not guaranteed to finish
> before the kernel initialization phase, the memory associated with
> ks_pcie_host_init() may already be freed by free_initmem().
> 
> It is observed in practice that the print associated with free_initmem()
> which is:
> 	"Freeing unused kernel memory: ..."
> is displayed before the driver is probed, following which an exception is
> triggered when ks_pcie_host_init() is invoked which looks like:
> 
> 	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
> 	...
> 
> Fix this by removing the "__init" macro associated with the
> ks_pcie_host_init() callback and the ks_pcie_init_id() function that it
> internally invokes.
> 
> Fixes: 0c4ffcfe1fbc ("PCI: keystone: Add TI Keystone PCIe driver")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>

I dropped this from pci/controller/keystone because of the resulting
section mismatch:

  https://lore.kernel.org/r/202510010726.GPljD7FR-lkp@intel.com

ks_pcie_host_init() calls hook_fault_code(), which is __init, so we
can't make ks_pcie_host_init() non-__init.

Both are bad problems, but there's no point in just swapping one
problem for a different one.

> ---
>  drivers/pci/controller/dwc/pci-keystone.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pci/controller/dwc/pci-keystone.c b/drivers/pci/controller/dwc/pci-keystone.c
> index 21808a9e5158..c6e082dcb3bc 100644
> --- a/drivers/pci/controller/dwc/pci-keystone.c
> +++ b/drivers/pci/controller/dwc/pci-keystone.c
> @@ -799,7 +799,7 @@ static int ks_pcie_fault(unsigned long addr, unsigned int fsr,
>  }
>  #endif
>  
> -static int __init ks_pcie_init_id(struct keystone_pcie *ks_pcie)
> +static int ks_pcie_init_id(struct keystone_pcie *ks_pcie)
>  {
>  	int ret;
>  	unsigned int id;
> @@ -831,7 +831,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);
> -- 
> 2.43.0
> 

Re: [PATCH 2/2] PCI: keystone: Remove the __init macro for the ks_pcie_host_init() callback

[Siddharth Vadapalli]

On Thu, 2025-10-02 at 09:36 -0500, Bjorn Helgaas wrote:

Hello Bjorn,

> On Fri, Sep 12, 2025 at 03:37:59PM +0530, Siddharth Vadapalli wrote:
> > The ks_pcie_host_init() callback registered by the driver is invoked by
> > dw_pcie_host_init(). Since the driver probe is not guaranteed to finish
> > before the kernel initialization phase, the memory associated with
> > ks_pcie_host_init() may already be freed by free_initmem().
> > 
> > It is observed in practice that the print associated with free_initmem()
> > which is:
> > 	"Freeing unused kernel memory: ..."
> > is displayed before the driver is probed, following which an exception is
> > triggered when ks_pcie_host_init() is invoked which looks like:
> > 
> > 	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
> > 	...
> > 
> > Fix this by removing the "__init" macro associated with the
> > ks_pcie_host_init() callback and the ks_pcie_init_id() function that it
> > internally invokes.
> > 
> > Fixes: 0c4ffcfe1fbc ("PCI: keystone: Add TI Keystone PCIe driver")
> > Cc: <stable@vger.kernel.org>
> > Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
> 
> I dropped this from pci/controller/keystone because of the resulting
> section mismatch:
> 
>   https://lore.kernel.org/r/202510010726.GPljD7FR-lkp@intel.com
> 
> ks_pcie_host_init() calls hook_fault_code(), which is __init, so we
> can't make ks_pcie_host_init() non-__init.
> 
> Both are bad problems, but there's no point in just swapping one
> problem for a different one.

Since this patch is required only for the case where the driver supports
being built as a loadable module, I have reworked on the patch and have
squashed it into patch 4 of the following series:
https://lore.kernel.org/r/20251022095724.997218-5-s-vadapalli@ti.com/
The implementation above ensures that 'hook_fault_code()' is placed within
an '__init' function while the '__init' keywords can safely be removed from
the remaining functions. Please review and let me know.

Regards,
Siddharth.