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

[Frank Li <Frank.li@oss.nxp.com>]

On Thu, Aug 27, 2026 at 08:29:09PM +0200, Michal Gorlas wrote:
> On Thu Aug 27, 2026 at 7:10 PM CEST, Frank Li wrote:
> > On Thu, Aug 27, 2026 at 06:51:00PM +0200, Michal Gorlas wrote:
> >> 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 i.MX SoCs
> >> (for e.g. i.MX 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 LNKCAP register if supports-clkreq
> >> is not set in the DT explicitly. For devices that set
> >> IMX_PCIE_FLAG_BROKEN_SUSPEND, disable L1 on resume as well, as these
> >> skip imx_pcie_host_init().
> >>
> >> 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>
> >> Tested-by: Richard Zhu <hongxing.zhu@nxp.com>
> >> Reviewed-by: Richard Zhu <hongxing.zhu@nxp.com>
> >> ---
> >> Changes in v2:
> >> - disable L1 unconditionally on resume for platforms advertising
> >> IMX_PCIE_FLAG_BROKEN_SUSPEND (Sashiko)
> >> - typos in the commit message
> >> - Link to v1: https://patch.msgid.link/20260719-l1-conditional-imx-v1-1-a55a57915c68@9elements.com
> >> ---
> >>  drivers/pci/controller/dwc/pci-imx6.c | 20 ++++++++++++++++++++
> >>  1 file changed, 20 insertions(+)
> >>
> >> diff --git a/drivers/pci/controller/dwc/pci-imx6.c b/drivers/pci/controller/dwc/pci-imx6.c
> >> index 98e1db751132..1fdcbcd2422e 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:
> >> @@ -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);
> >
> > Does l1 disable permanent after resume? should it be
> >
> > 	if (!imx_pcie->supports_clkreq)
> > 		mx_pcie_disable_l1(imx_pcie->pci);
>
> It would be unnecessary, the imx_pcie_disable_l1() there is already
> guarded by IMX_PCIE_FLAG_BROKEN_SUSPEND, so we know that only iMX6Q
> ends up there on resume. And none of iMX6Q DT's sets supports-clkreq.

Okay,

Reviewed-by: Frank Li <Frank.Li@nxp.com>

>
> Best,
> Michal
> >
> > Frank
> >
> >>
> >>  		/*
> >>  		 * Using PCIE_TEST_PD seems to disable MSI and powers down the
> >>
> >> ---
> >> base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
> >> change-id: 20260713-l1-conditional-imx-c2bfd20ae085
> >>
> >> Best regards,
> >> --
> >> Michal Gorlas <michal.gorlas@9elements.com>
> >>
>