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

Thank you for your contribution! Sashiko AI review found 1 potential issue(=
s) to consider:
- [Medium] ASPM L1 is conditionally disabled at boot but unconditionally di=
sabled upon resume, causing state inconsistency across power cycles.
--

--- Patch [1]: [PATCH v2] PCI: imx6: Enable ASPM L1 only iff CLKREQ# is sup=
ported ---
commit 7a899eb7e9f8116a0334832c7490e52f71ae751c
Author: Michal Gorlas <[email protected]>

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

This commit conditionally disables ASPM L1 capabilities in the LNKCAP
register if supports-clkreq is not set in the device tree. It also disables
L1 during resume for platforms advertising IMX_PCIE_FLAG_BROKEN_SUSPEND.

Link: https://patch.msgid.link/20260719-l1-conditional-imx-v1-1-a55a57915c6=
[email protected]

> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controll=
er/dwc/pci-imx6.c
> index 98e1db751132a..1fdcbcd2422e5 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)
> =20
>  	imx_setup_phy_mpll(imx_pcie);
> =20
> +	if (!imx_pcie->supports_clkreq)
> +		imx_pcie_disable_l1(pci);
> +
>  	return 0;
> =20
>  err_phy_off:
[ ... ]
> @@ -1748,6 +1767,7 @@ static int imx_pcie_resume_noirq(struct device *dev)
>  			return ret;
>  		imx_pcie_deassert_core_reset(imx_pcie);
>  		imx_pcie_assert_perst(imx_pcie, false);
> +		imx_pcie_disable_l1(imx_pcie->pci);

[Severity: Medium]
Will this cause an inconsistent ASPM L1 state across power cycles?

During boot in imx_pcie_host_init(), imx_pcie_disable_l1() is only called if
imx_pcie->supports_clkreq is false. However, during resume here in
imx_pcie_resume_noirq(), the L1 capability is disabled unconditionally for
platforms with IMX_PCIE_FLAG_BROKEN_SUSPEND.

If a platform has supports_clkreq set to true, it would retain the L1
capability at boot, but lose it unconditionally after a suspend/resume cycl=
e.

Modifying the read-only Link Capabilities asymmetrically across power cycles
can confuse the OS ASPM subsystem, which caches LNKCAP during enumeration.
Should this also check for !imx_pcie->supports_clkreq before disabling L1
during resume?

> =20
>  		/*
>  		 * Using PCIE_TEST_PD seems to disable MSI and powers down the
[ ... ]

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260731-l1-conditi=
[email protected]?part=3D1