Re: [PATCH V6 2/4] PCI: imx6: Add skip_pwrctrl_off flag support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Hardware initialization failure is ignored in `imx_add_pcie_ep()`, likely leading to a system crash.
--

--- Patch [2]: [PATCH V6 2/4] PCI: imx6: Add skip_pwrctrl_off flag support ---
commit 6bc8a94c274f464b53bad3ac0ac1a464c280f05e
Author: Sherry Sun <sherry.sun@nxp.com>

PCI: imx6: Add skip_pwrctrl_off flag support

Use dw_pcie_rp::skip_pwrctrl_off to avoid powering off devices during
suspend to preserve wakeup capability of the devices and also not to
power on the devices in the init path.

> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 92f8e4a299e88..afcf3b6bf3cd7 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
> @@ -1382,10 +1382,12 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
>  		}
>  	}
>  
> -	ret = pci_pwrctrl_power_on_devices(dev);
> -	if (ret) {
> -		dev_err(dev, "failed to power on pwrctrl devices\n");
> -		goto err_reg_disable;
> +	if (!pp->skip_pwrctrl_off) {
> +		ret = pci_pwrctrl_power_on_devices(dev);
> +		if (ret) {
> +			dev_err(dev, "failed to power on pwrctrl devices\n");
> +			goto err_reg_disable;
> +		}
>  	}
>  
>  	ret = imx_pcie_clk_enable(imx_pcie);

[Severity: High]
This is a pre-existing issue, but since this patch modifies
imx_pcie_host_init(), I noticed an issue with how its return value is handled.

Looking at imx_add_pcie_ep() which calls this function:

drivers/pci/controller/dwc/pci-imx6.c:imx_add_pcie_ep() {
	...
	imx_pcie_host_init(pp);
	ep = &pci->ep;
	ep->ops = &pcie_ep_ops;
	...
}

The return value of imx_pcie_host_init() is entirely ignored here. If hardware
initialization fails (e.g., clocks fail to enable or PHY fails to power on),
imx_pcie_host_init() returns a negative error code but leaves the hardware
unclocked or unpowered.

The code then proceeds to call dw_pcie_ep_init_registers(), which accesses
unclocked DBI memory-mapped registers (e.g., PCI_HEADER_TYPE).

Can this lead to a synchronous external abort and kernel panic when accessing
these unclocked peripheral registers?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713023435.235765-1-sherry.sun@oss.nxp.com?part=2