Re: [PATCH v2 1/4] PCI: dwc: Advertise L1 PM Substates only if driver requests it

Manivannan Sadhasivam
2025-11-20T07:52:59Z

On Tue, Nov 18, 2025 at 05:34:18PM -0500, Frank Li wrote:
> On Tue, Nov 18, 2025 at 03:42:15PM -0600, Bjorn Helgaas wrote:
> > From: Bjorn Helgaas <bhelgaas@google.com>
> >
> > L1 PM Substates require the CLKREQ# signal and may also require
> > device-specific support. If CLKREQ# is not supported or driver support is
> > lacking, enabling L1.1 or L1.2 may cause errors when accessing devices,
> > e.g.,
> >
> > nvme nvme0: controller is down; will reset: CSTS=0xffffffff, PCI_STATUS=0x10
> >
> > If the kernel is built with CONFIG_PCIEASPM_POWER_SUPERSAVE=y or users
> > enable L1.x via sysfs, users may trip over these errors even if L1
> > Substates haven't been enabled by firmware or the driver.
> >
> > To prevent such errors, disable advertising the L1 PM Substates unless the
> > driver sets "dw_pcie.l1ss_support" to indicate that it knows CLKREQ# is
> > present and any device-specific configuration has been done.
> >
> > Set "dw_pcie.l1ss_support" in tegra194 (if DT includes the
> > "supports-clkreq' property) and qcom (for cfg_2_7_0, cfg_1_9_0, cfg_1_34_0,
> > and cfg_sc8280xp controllers) so they can continue to use L1 Substates.
> >
> > Based on Niklas's patch:
> > https://patch.msgid.link/20251017163252.598812-2-cassel@kernel.org
> >
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > ---
> > .../pci/controller/dwc/pcie-designware-ep.c | 2 ++
> > .../pci/controller/dwc/pcie-designware-host.c | 2 ++
> > drivers/pci/controller/dwc/pcie-designware.c | 24 +++++++++++++++++++
> > drivers/pci/controller/dwc/pcie-designware.h | 2 ++
> > drivers/pci/controller/dwc/pcie-qcom.c | 2 ++
> > drivers/pci/controller/dwc/pcie-tegra194.c | 3 +++
> > 6 files changed, 35 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > index 7f2112c2fb21..ad6c0fd67a65 100644
> > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > @@ -966,6 +966,8 @@ int dw_pcie_ep_init_registers(struct dw_pcie_ep *ep)
> > if (ep->ops->init)
> > ep->ops->init(ep);
> >
> > + dw_pcie_hide_unsupported_l1ss(pci);
> > +
>
> And, I don't think EP need clean L1SS CAP flags. If EP don't support L1SS,
> it should be force pull down #clkreq.
>
> EP don't know if #clkreq connected in host boards, assume EP with same
> software, which can run at two difference host system, one host system
> connect #clkreq, the other system have not connect #clkreq.
>
> otherwords, support-clkreq property should be only for RC side. not EP
> side. EP side l1ss support should depend on specific epf function and
> controller's capablity.
>

'support-clkreq' DT property is only applicable to the RC as per dtschema. So
using it in the EP driver is wrong in the first place. We could add it to the EP
schema, but that should be done first before relying on it in the driver.

Also, I do think that the device capability should accurately reflect whether
L1ss is supported or not and not assume that the EP will pull the CLKREQ# low
when it is not supported. If the L1ss CAP is available on both RC and EP, host
software may enable L1ss and expect it to work.

But enabling/disabling the CAP for EP should be done later based on either the
DT property or controller specific flag.

- Mani

--
மணிவண்ணன் சதாசிவம்