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

"Michal Gorlas" <[email protected]> Fri, 31 Jul 2026 17:27:27 +0200
Newsgroups dev.linux.lists.imx,dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-pci
Message-ID <[email protected]>
On Fri Jul 31, 2026 at 3:54 PM CEST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issu=
e(s) to consider:
> - [Medium] ASPM L1 is conditionally disabled at boot but unconditionally =
disabled upon resume, causing state inconsistency across power cycles.
> --
>
> --- Patch [1]: [PATCH v2] PCI: imx6: Enable ASPM L1 only iff CLKREQ# is s=
upported ---
> 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 disabl=
es
> L1 during resume for platforms advertising IMX_PCIE_FLAG_BROKEN_SUSPEND.
>
> Link: https://patch.msgid.link/20260719-l1-conditional-imx-v1-1-a55a57915=
[email protected]
>
>> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/control=
ler/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 *p=
p)
>> =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 *de=
v)
>>  			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 fo=
r
> 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 cy=
cle.

Platforms that set IMX_PCIE_FLAG_BROKEN_SUSPEND (i.MX6Q) do not
set supports-clkreq in their DTs. So no, such scenario won't occur.