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

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: [email protected]
> Link: https://lore.kernel.org/all/[email protected]/
> Signed-off-by: Zhijian Han <[email protected]>
> ---

Reviewed-by: Frank Li <[email protected]>

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