[PATCH v3] PCI: imx6: fix endpoint init error handling

[PATCH v3] PCI: imx6: fix endpoint init error handling

[Zhijian Han]

imx_add_pcie_ep() ignores the return value of imx_pcie_host_init(), so
a failure to enable the clocks, regulators, or PHY during endpoint
initialization goes unnoticed and the driver proceeds to initialize the
endpoint against unpowered hardware.

It also returns directly without releasing the host resources when
dw_pcie_ep_init() or dw_pcie_ep_init_registers() fails, leaking the
clocks, regulators, and PHY that imx_pcie_host_init() acquired.

Check the return value of imx_pcie_host_init() and register
imx_pcie_host_exit() with devm_add_action_or_reset() so the host
resources are released through the devres framework, which unregisters
the EPC device before powering the hardware off.  This mirrors the root
port path, where dw_pcie_host_init() releases these resources through
the same framework.

Reported-by: sashiko-bot@kernel.org
Link: https://lore.kernel.org/all/20260824152857.DB9771F00A3D@smtp.kernel.org/
Fixes: 75c2f26da03f ("PCI: imx6: Add i.MX PCIe EP mode support")
Cc: stable@vger.kernel.org
Signed-off-by: Zhijian Han <hanzhijian1991@gmail.com>
---
v3:
- Add Fixes: and Cc: stable tags

v2:
- Use devm_add_action_or_reset() for imx_pcie_host_exit() instead of
  calling it directly on the error paths, so the EPC device is
  unregistered before the hardware is powered off

 drivers/pci/controller/dwc/pci-imx6.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 39790e66b..f0fb4e1b0 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1488,6 +1488,11 @@ static void imx_pcie_host_exit(struct dw_pcie_rp *pp)
 		regulator_disable(imx_pcie->vpcie);
 }
 
+static void imx_pcie_host_exit_action(void *data)
+{
+	imx_pcie_host_exit(data);
+}
+
 static void imx_pcie_host_post_init(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -1634,7 +1639,20 @@ static int imx_add_pcie_ep(struct imx_pcie *imx_pcie,
 	struct dw_pcie_rp *pp = &pci->pp;
 	struct device *dev = pci->dev;
 
-	imx_pcie_host_init(pp);
+	ret = imx_pcie_host_init(pp);
+	if (ret)
+		return ret;
+
+	/*
+	 * Tear the host resources down via a devm action so that, on probe
+	 * failure, the EPC device created by dw_pcie_ep_init() is
+	 * unregistered (also via devres) before the clocks, regulators and
+	 * PHY are switched off.
+	 */
+	ret = devm_add_action_or_reset(dev, imx_pcie_host_exit_action, pp);
+	if (ret)
+		return ret;
+
 	ep = &pci->ep;
 	ep->ops = &pcie_ep_ops;
 

Re: [PATCH v3] PCI: imx6: fix endpoint init error handling

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Deferring imx_pcie_host_exit() via devm_add_action_or_reset() creates a teardown ordering conflict with manual cleanup in probe, causing pwrctrl devices to leak on endpoint initialization failure.
--

--- Patch [1]: [PATCH v3] PCI: imx6: fix endpoint init error handling ---
commit 8e59197090261ab72469bfcfa7c4d49abbeafbae
Author: Zhijian Han <hanzhijian1991@gmail.com>

PCI: imx6: fix endpoint init error handling

This commit updates imx_add_pcie_ep() to check the return value of
imx_pcie_host_init(). It also registers a devres action to release host
resources on error, aiming to unregister the EPC device before powering
off hardware.

Link: https://lore.kernel.org/all/20260824152857.DB9771F00A3D@smtp.kernel.org/

> This mirrors the root
> port path, where dw_pcie_host_init() releases these resources through
> the same framework.

[Severity: Low]
Is this description accurate? The root port path handles deinitialization
synchronously without devres, rather than through the same framework.

> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 39790e66b98dc..f0fb4e1b0ac6b 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c

[ ... ]

> @@ -1634,7 +1639,20 @@ static int imx_add_pcie_ep(struct imx_pcie *imx_pcie,
>  	struct dw_pcie_rp *pp = &pci->pp;
>  	struct device *dev = pci->dev;
>  
> -	imx_pcie_host_init(pp);
> +	ret = imx_pcie_host_init(pp);
> +	if (ret)
> +		return ret;
> +
> +	/*
> +	 * Tear the host resources down via a devm action so that, on probe
> +	 * failure, the EPC device created by dw_pcie_ep_init() is
> +	 * unregistered (also via devres) before the clocks, regulators and
> +	 * PHY are switched off.
> +	 */
> +	ret = devm_add_action_or_reset(dev, imx_pcie_host_exit_action, pp);

[Severity: High]
Does deferring imx_pcie_host_exit() via devres create a teardown ordering
conflict with the manual cleanup in imx_pcie_probe()?

If imx_add_pcie_ep() returns an error during endpoint initialization,
imx_pcie_probe() will execute its manual cleanup before any devres actions
run:

drivers/pci/controller/dwc/pci-imx6.c:imx_pcie_probe() {
    ...
    if (imx_pcie->drvdata->mode == DW_PCIE_EP_TYPE) {
        ret = imx_add_pcie_ep(imx_pcie, pdev);
        if (ret < 0)
            goto err_pwrctrl_destroy;
    ...
err_pwrctrl_destroy:
    pci_pwrctrl_destroy_devices(dev);
    ...
}

Since pci_pwrctrl_destroy_devices() unregisters the child pwrctrl devices,
when the deferred imx_pcie_host_exit_action() finally executes and calls
pci_pwrctrl_power_off_devices(), the devices are already unregistered.

Could this cause the power-off to be skipped, leaving the hardware resources
permanently powered on?

> +	if (ret)
> +		return ret;
> +
>  	ep = &pci->ep;
>  	ep->ops = &pcie_ep_ops;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825141526.3911472-1-hanzhijian1991@gmail.com?part=1

Re: [PATCH v3] PCI: imx6: fix endpoint init error handling

[Zhijian Han]

On Wed, Aug 26, 2026 at 12:18:26PM +0200, Markus Elfring wrote:
> I would find it nicer when involved reviewers (including me) would
> also be specified in a message recipient list directly.

Sure, I'll add you (and any other reviewers) to the recipient list of
future versions.  Thanks for the review.

Regards,
Zhijian