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

[PATCH v2] 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/
Signed-off-by: Zhijian Han <hanzhijian1991@gmail.com>
---
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;
 
-- 
2.43.0

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

[Markus Elfring]

…
> Check the return value of imx_pcie_host_init() and …

How do you think about to add any tags (like “Fixes” and “Cc”) accordingly?

See also:
* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2#n145
* https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/stable-kernel-rules.rst?h=v7.2#n34


Regards,
Markus

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

[Frank Li]

On Tue, Aug 25, 2026 at 02:03:40PM +0800, Zhijian Han wrote:
> 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/
> Signed-off-by: Zhijian Han <hanzhijian1991@gmail.com>
> ---

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> 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;
>
> --
> 2.43.0
>
>