Re: [PATCH v2] PCI: imx6: Move clock enable after core reset assertion

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

Pre-existing issues:
- [High] The return value of `imx_pcie_host_init()` is ignored in `imx_add_pcie_ep()`, allowing initialization to proceed even if critical hardware resources failed to enable.
--

--- Patch [1]: [PATCH v2] PCI: imx6: Move clock enable after core reset assertion ---
commit e96724a712688ba6f907245675d12f392129c5de
Author: Richard Zhu <[email protected]>

PCI: imx6: Move clock enable after core reset assertion

Restores the sequence where clocks are enabled after asserting core reset
and configuring the controller type. Fixes a regression on i.MX6Q Apalis
platforms with ASM1061/ASM1062 SATA controllers connected.

Link: https://lore.kernel.org/all/bl7i3obu2clzsgeoct2a4mtfhv6typcjdqmgneropf3hpgwve6@n2m5uhlduw57/T/#u
Link: https://lore.kernel.org/all/t65y5d54axtksbfs7r4olcefqhwm6m4dz3njgnrnf7fcotj74i@o7avoznlafbj/

> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 39790e66b98dc..f7a2eb257c169 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
[ ... ]
> @@ -1415,6 +1409,12 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
>  
>  	imx_pcie_configure_type(imx_pcie);
>  
> +	ret = imx_pcie_clk_enable(imx_pcie);
> +	if (ret) {
> +		dev_err(dev, "unable to enable pcie clocks: %d\n", ret);
> +		goto err_pwrctrl_power_off;
> +	}

[Severity: High]
This isn't a bug introduced by this patch, but is it safe to ignore the
return value of imx_pcie_host_init() in imx_add_pcie_ep()?

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

If imx_pcie_host_init() fails (for example, if imx_pcie_clk_enable() fails
here), the error is discarded and the endpoint initialization proceeds.

Could this lead to a kernel panic or synchronous external abort when
dw_pcie_ep_init() subsequently attempts to access unclocked or unpowered
PCIe registers?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1