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

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

[Michal Gorlas]

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
(for e.g. iMX 8M Plus Kontron SMARC eval carrier) to enter L1 even
though CLKREQ# is not supported (and consequently supports-clkreq is
not set in pcie controller node of their DT), which causes the device to
never wake up from L1.

Hence, disable L1 capability in the LINKCAP register if supports-clkreq
is not set in the DT explicitly.

Note, the logic here is very similar to 613f3255a35a, only difference
is that leaving L0s enabled does not cause issues (at least
I never encountered them myself when testing on the device) as its
not dependent on CLKREQ# being physically there.
Since there is already a way of specifying its presence in the DT,
the driver should be aware and disable L1.

Fixes: f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree platforms")
Signed-off-by: Michal Gorlas <michal.gorlas@9elements.com>
---
 drivers/pci/controller/dwc/pci-imx6.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
index 98e1db751132..7471501fbf04 100644
--- a/drivers/pci/controller/dwc/pci-imx6.c
+++ b/drivers/pci/controller/dwc/pci-imx6.c
@@ -1318,6 +1318,22 @@ static void imx_pcie_assert_perst(struct imx_pcie *imx_pcie, bool assert)
 	}
 }
 
+static void imx_pcie_disable_l1(struct dw_pcie *pci)
+{
+	u32 val;
+	u8 offset;
+
+	offset = dw_pcie_find_capability(pci, PCI_CAP_ID_EXP);
+
+	dw_pcie_dbi_ro_wr_en(pci);
+
+	val = dw_pcie_readl_dbi(pci, offset + PCI_EXP_LNKCAP);
+	val &= ~PCI_EXP_LNKCAP_ASPM_L1;
+	dw_pcie_writel_dbi(pci, offset + PCI_EXP_LNKCAP, val);
+
+	dw_pcie_dbi_ro_wr_dis(pci);
+}
+
 static int imx_pcie_host_init(struct dw_pcie_rp *pp)
 {
 	struct dw_pcie *pci = to_dw_pcie_from_pp(pp);
@@ -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);
+
 	return 0;
 
 err_phy_off:

---
base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
change-id: 20260713-l1-conditional-imx-c2bfd20ae085

Best regards,
--  
Michal Gorlas <michal.gorlas@9elements.com>

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

[sashiko-bot]

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