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.