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

From: Frank Li

Date: Tue Aug 25 2026 - 10:37:02 EST


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@xxxxxxxxxx
> Link: https://lore.kernel.org/all/20260824152857.DB9771F00A3D@xxxxxxxxxxxxxxx/
> Signed-off-by: Zhijian Han <hanzhijian1991@xxxxxxxxx>
> ---

Reviewed-by: Frank Li <Frank.Li@xxxxxxx>

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