Re: [PATCH] PCI: imx6: enable ASPM L1 only iff CLKREQ# is supported

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing state restoration on resume: The patch disables ASPM L1 capability during probe for platforms lacking CLKREQ#, but fails to re-apply this hardware configuration during resume for platforms with the `IMX_PCIE_FLAG_BROKEN_SUSPEND` flag (e.g., IMX6Q).
--

--- Patch [1]: [PATCH] PCI: imx6: enable ASPM L1 only iff CLKREQ# is supported ---
commit 35b1dac6afa595d56fd03bd1c31c2ca679682abe
Author: Michal Gorlas <michal.gorlas@9elements.com>

PCI: imx6: enable ASPM L1 only iff CLKREQ# is supported

Since commit f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM
states for devicetree platforms"), ASPM states are force enabled
on all devicetree platforms. This causes PCI devices on some iMX SoCs
to enter L1 even though CLKREQ# is not supported, causing devices to
never wake up. This patch disables the L1 capability in the LINKCAP
register if supports-clkreq is not set in the DT explicitly.

> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> index 98e1db751132a..7471501fbf043 100644
> --- a/drivers/pci/controller/dwc/pci-imx6.c
> +++ b/drivers/pci/controller/dwc/pci-imx6.c
[ ... ]
> @@ -1442,6 +1458,9 @@ static int imx_pcie_host_init(struct dw_pcie_rp *pp)
>  
>  	imx_setup_phy_mpll(imx_pcie);
>  
> +	if (!imx_pcie->supports_clkreq)
> +		imx_pcie_disable_l1(pci);
> +

[Severity: High]
For platforms with the IMX_PCIE_FLAG_BROKEN_SUSPEND flag (such as IMX6Q),
imx_pcie_resume_noirq() bypasses dw_pcie_resume_noirq() (which normally calls
imx_pcie_host_init()). Instead, it directly calls dw_pcie_setup_rc():

drivers/pci/controller/dwc/pci-imx6.c:imx_pcie_resume_noirq() {
    ...
	if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_BROKEN_SUSPEND)) {
        ...
		/*
		 * Using PCIE_TEST_PD seems to disable MSI and powers down the
		 * root complex. This is why we have to setup the rc again and
		 * why we have to restore the MSI register.
		 */
		ret = dw_pcie_setup_rc(&imx_pcie->pci->pp);
		if (ret)
			return ret;
    ...
}

Because the root complex is powered down during suspend, its configuration
registers revert to hardware defaults.

Will this allow ASPM L1 to be re-enabled on resume, potentially causing a
device hang?

>  	return 0;
>  
>  err_phy_off:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260719-l1-conditional-imx-v1-1-a55a57915c68@9elements.com?part=1