RE: [PATCH v1] PCI: imx6: Add force_suspend flag to override L1SS suspend skip

[Hongxing Zhu]

> -----Original Message-----
> From: Bjorn Helgaas <helgaas@xxxxxxxxxx>
> Sent: 2026年3月24日 6:09
> To: Hongxing Zhu <hongxing.zhu@xxxxxxx>
> Cc: Frank Li <frank.li@xxxxxxx>; jingoohan1@xxxxxxxxx;
> l.stach@xxxxxxxxxxxxxx; lpieralisi@xxxxxxxxxx; kwilczynski@xxxxxxxxxx;
> mani@xxxxxxxxxx; robh@xxxxxxxxxx; bhelgaas@xxxxxxxxxx;
> s.hauer@xxxxxxxxxxxxxx; kernel@xxxxxxxxxxxxxx; festevam@xxxxxxxxx;
> linux-pci@xxxxxxxxxxxxxxx; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx;
> imx@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; stable@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v1] PCI: imx6: Add force_suspend flag to override L1SS
> suspend skip
> 
> On Wed, Mar 18, 2026 at 02:55:45AM +0000, Hongxing Zhu wrote:
> > > -----Original Message-----
> > > From: Bjorn Helgaas <helgaas@xxxxxxxxxx>
> > ... [messed up quoting]
> 
> > > On Tue, Mar 17, 2026 at 02:12:56PM +0800, Richard Zhu wrote:
> > > > Add a force_suspend flag to allow platform drivers to force the
> > > > PCIe link into L2 state during suspend, even when L1SS (ASPM L1
> > > > Sub-States) is enabled.
> > > >
> > > > By default, the DesignWare PCIe host controller skips L2 suspend
> > > > when L1SS is supported to meet low resume latency requirements for
> > > > devices like NVMe. However, some platforms like i.MX PCIe need to
> > > > enter L2 state for proper power management regardless of L1SS
> support.
> > > >
> > > > Enable force_suspend for i.MX PCIe to ensure the link enters L2
> > > > during system suspend.
> > >
> > > I'm a little bit skeptical about this.
> > >
> > > What exactly does a "low resume latency requirement" mean?  Is this
> > > an actual functional requirement that's special to NVMe, or is it
> > > just the desire for low resume latency that everybody has for all
> > > devices?
> >
> > From my understanding, L1SS mode is characterized by lower latency
> > when compared to L2 or L3 modes.
> >
> > It can be used on all devices, avoiding frequent power on/off cycles.
> > NVMe can also extend the service life of the equipment.
> 
> All the above applies to all platforms, so it's not an argument for
> i.MX-specific code here.
>
Hi Bjorn:
Thanks for your kindly review. 
Yes, it is.
> > > Is there something special about i.MX here?  Why do we want i.MX to
> > > be different from other host controllers?
> >
> > i.MX PCIe loses power supply during Deep Sleep Mode (DSM), requiring
> > full reinitialization after system wake-up.
> 
> I don't know what DSM means in PCIe or how it would help justify this
> change.
> 
i.MX PCIe power is gated off during suspend, requiring full reinitialization
on resume

Best Regards
Richard Zhu
> > Removing the L1SS check allows the suspend process to complete
> > successfully and ensures the pci->suspended flag is set to true, which
> > triggers the proper resume sequence during system wake-up for i.MX
> > PCIes.
> 
> > > > Cc: stable@xxxxxxxxxxxxxxx
> > > > Fixes: 4774faf854f5 ("PCI: dwc: Implement generic suspend/resume
> > > > functionality")
> > > > Signed-off-by: Richard Zhu <hongxing.zhu@xxxxxxx>
> > > > ---
> > > >  drivers/pci/controller/dwc/pci-imx6.c             | 1 +
> > > >  drivers/pci/controller/dwc/pcie-designware-host.c | 4 +++-
> > > >  drivers/pci/controller/dwc/pcie-designware.h      | 1 +
> > > >  3 files changed, 5 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pci-imx6.c
> > > > b/drivers/pci/controller/dwc/pci-imx6.c
> > > > index 81a7093494c8..7902d39185a5 100644
> > > > --- a/drivers/pci/controller/dwc/pci-imx6.c
> > > > +++ b/drivers/pci/controller/dwc/pci-imx6.c
> > > > @@ -1831,6 +1831,7 @@ static int imx_pcie_probe(struct
> > > > platform_device
> > > *pdev)
> > > >  		if (imx_check_flag(imx_pcie, IMX_PCIE_FLAG_SKIP_L23_READY))
> > > >  			pci->pp.skip_l23_ready = true;
> > > >  		pci->pp.use_atu_msg = true;
> > > > +		pci->pp.force_l2_suspend = true;
> > > >  		ret = dw_pcie_host_init(&pci->pp);
> > > >  		if (ret < 0)
> > > >  			return ret;
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-host.c
> > > > b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > > index a74339982c24..720154fd4ff0 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware-host.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware-host.c
> > > > @@ -1229,7 +1229,9 @@ int dw_pcie_suspend_noirq(struct dw_pcie
> *pci)
> > > >  	 * If L1SS is supported, then do not put the link into L2 as some
> > > >  	 * devices such as NVMe expect low resume latency.
> > > >  	 */
> > > > -	if (dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKCTL) &
> > > PCI_EXP_LNKCTL_ASPM_L1)
> > > > +	if (!pci->pp.force_l2_suspend &&
> > > > +	    (dw_pcie_readw_dbi(pci, offset + PCI_EXP_LNKCTL) &
> > > > +	     PCI_EXP_LNKCTL_ASPM_L1))
> > > >  		return 0;
> > > >
> > > >  	if (pci->pp.ops->pme_turn_off) { diff --git
> > > > a/drivers/pci/controller/dwc/pcie-designware.h
> > > > b/drivers/pci/controller/dwc/pcie-designware.h
> > > > index ae6389dd9caa..5261036bbe6e 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware.h
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware.h
> > > > @@ -447,6 +447,7 @@ struct dw_pcie_rp {
> > > >  	bool			ecam_enabled;
> > > >  	bool			native_ecam;
> > > >  	bool                    skip_l23_ready;
> > > > +	bool                    force_l2_suspend;
> > > >  };
> > > >
> > > >  struct dw_pcie_ep_ops {
> > > > --
> > > > 2.37.1
> > > >