drivers/pci/pci.h | 2 ++ drivers/pci/pcie/aspm.c | 25 +++++++++++++++++-------- drivers/pci/probe.c | 7 +++++++ drivers/pci/quirks.c | 38 +++++++++++++++++++------------------- include/linux/pci.h | 2 ++ 5 files changed, 47 insertions(+), 27 deletions(-)
From: Bjorn Helgaas <bhelgaas@google.com>
We enabled ASPM too aggressively in v6.18-rc1. f3ac2ff14834 ("PCI/ASPM:
Enable all ClockPM and ASPM states for devicetree platforms") enabled ASPM
L0s, L1, and (if advertised) L1 PM Substates.
df5192d9bb0e ("PCI/ASPM: Enable only L0s and L1 for devicetree platforms")
(v6.18-rc3) backed off and omitted Clock PM and L1 Substates because we
don't have good infrastructure to discover CLKREQ# support, and L1
Substates may require device-specific configuration.
L0s and L1 are generically discoverable and should not require
device-specific support, but some devices advertise them even though they
don't work correctly. This series is a way to add quirks avoid L0s and L1
in this case.
Bjorn Helgaas (4):
PCI/ASPM: Cache L0s/L1 Supported so advertised link states can be
overridden
PCI/ASPM: Add pcie_aspm_remove_cap() to override advertised link
states
PCI/ASPM: Convert quirks to override advertised link states
PCI/ASPM: Avoid L0s and L1 on Freescale Root Ports
drivers/pci/pci.h | 2 ++
drivers/pci/pcie/aspm.c | 25 +++++++++++++++++--------
drivers/pci/probe.c | 7 +++++++
drivers/pci/quirks.c | 38 +++++++++++++++++++-------------------
include/linux/pci.h | 2 ++
5 files changed, 47 insertions(+), 27 deletions(-)
--
v1: https://lore.kernel.org/r/20251106183643.1963801-1-helgaas@kernel.org
Changes between v1 and v2:
- Cache just the two bits for L0s and L1 support, not the entire Link
Capabilities (Lukas)
- Add pcie_aspm_remove_cap() to override the ASPM Support bits in Link
Capabilities (Lukas)
- Convert existing quirks to use pcie_aspm_remove_cap() instead of
pci_disable_link_state(), and from FINAL to HEADER (Mani)
On Mon, Nov 10, 2025 at 04:22:24PM -0600, Bjorn Helgaas wrote:
> We enabled ASPM too aggressively in v6.18-rc1. f3ac2ff14834 ("PCI/ASPM:
> Enable all ClockPM and ASPM states for devicetree platforms") enabled ASPM
> L0s, L1, and (if advertised) L1 PM Substates.
>
> df5192d9bb0e ("PCI/ASPM: Enable only L0s and L1 for devicetree platforms")
> (v6.18-rc3) backed off and omitted Clock PM and L1 Substates because we
> don't have good infrastructure to discover CLKREQ# support, and L1
> Substates may require device-specific configuration.
>
> L0s and L1 are generically discoverable and should not require
> device-specific support, but some devices advertise them even though they
> don't work correctly. This series is a way to add quirks avoid L0s and L1
> in this case.
Reviewed-by: Lukas Wunner <lukas@wunner.de>
I note that a number of drivers call pci_disable_link_state() or
pci_disable_link_state_locked() to disable ASPM on probe.
Can we convert (all of) these to quirks which use the new helper
introduced here?
I think that would be useful because it would disable ASPM even if
the driver isn't available and thus avoid e.g. AER messages caused
by ASPM issues.
pcie_aspm_init_link_state() also contains the following code comment:
/*
* At this stage drivers haven't had an opportunity to change the
* link policy setting. Enabling ASPM on broken hardware can cripple
* it even before the driver has had a chance to disable ASPM, so
* default to a safe level right now. If we're enabling ASPM beyond
* the BIOS's expectation, we'll do so once pci_enable_device() is
* called.
*/
If we'd mask out incorrect or non-working L0s/L1 capabilities for all
devices early during enumeration via quirks, we wouldn't have to go
through these contortions of setting up deeper ASPM states only at
device enable time.
Thanks,
Lukas
On Tue, Nov 11, 2025 at 10:33:43AM +0100, Lukas Wunner wrote:
> On Mon, Nov 10, 2025 at 04:22:24PM -0600, Bjorn Helgaas wrote:
> > We enabled ASPM too aggressively in v6.18-rc1. f3ac2ff14834 ("PCI/ASPM:
> > Enable all ClockPM and ASPM states for devicetree platforms") enabled ASPM
> > L0s, L1, and (if advertised) L1 PM Substates.
> >
> > df5192d9bb0e ("PCI/ASPM: Enable only L0s and L1 for devicetree platforms")
> > (v6.18-rc3) backed off and omitted Clock PM and L1 Substates because we
> > don't have good infrastructure to discover CLKREQ# support, and L1
> > Substates may require device-specific configuration.
> >
> > L0s and L1 are generically discoverable and should not require
> > device-specific support, but some devices advertise them even though they
> > don't work correctly. This series is a way to add quirks avoid L0s and L1
> > in this case.
>
> Reviewed-by: Lukas Wunner <lukas@wunner.de>
Thanks!
> I note that a number of drivers call pci_disable_link_state() or
> pci_disable_link_state_locked() to disable ASPM on probe.
> Can we convert (all of) these to quirks which use the new helper
> introduced here?
>
> I think that would be useful because it would disable ASPM even if
> the driver isn't available and thus avoid e.g. AER messages caused
> by ASPM issues.
>
> pcie_aspm_init_link_state() also contains the following code comment:
>
> /*
> * At this stage drivers haven't had an opportunity to change the
> * link policy setting. Enabling ASPM on broken hardware can cripple
> * it even before the driver has had a chance to disable ASPM, so
> * default to a safe level right now. If we're enabling ASPM beyond
> * the BIOS's expectation, we'll do so once pci_enable_device() is
> * called.
> */
>
> If we'd mask out incorrect or non-working L0s/L1 capabilities for all
> devices early during enumeration via quirks, we wouldn't have to go
> through these contortions of setting up deeper ASPM states only at
> device enable time.
I definitely agree. I forgot to follow up on all of those cases.
There aren't that many of them, but it looks like probably too many to
address for v6.18, and I *think* it's safe to wait and deal with them
for v6.19.
Bjorn
On Tue, Nov 11, 2025 at 09:44:45AM -0600, Bjorn Helgaas wrote: > On Tue, Nov 11, 2025 at 10:33:43AM +0100, Lukas Wunner wrote: > > I note that a number of drivers call pci_disable_link_state() or > > pci_disable_link_state_locked() to disable ASPM on probe. > > Can we convert (all of) these to quirks which use the new helper > > introduced here? [...] > I definitely agree. I forgot to follow up on all of those cases. > There aren't that many of them, but it looks like probably too many to > address for v6.18, and I *think* it's safe to wait and deal with them > for v6.19. Yes I agree this isn't necessary for v6.18. It may even be too late for v6.19 given the amount of time to come up with patches, get them reviewed and allow sufficient time to soak in linux-next. And this would be a cleanup, so not really urgent. I note that a lot of material has queued up in patchwork and only few new features have been applied to pci.git this cycle, which I guess has been caused by the unusual number of regressions introduced during the merge window. Given that, I'd down-prioritize conversion of pci_disable_link_state() calls. One feature I'd be keen to get in for v6.19 (if time permits) is this one: https://lore.kernel.org/all/cover.1760274044.git.lukas@wunner.de/ I'm worried that the "pci_save_state() after pci_restore_state()" anti-pattern gets cargo-culted to more drivers if it's not removed soon. I can split patch [2/2] in that series into smaller patches and funnel them through the individual subsystem trees if you'd rather not apply them wholesale in one large treewide patch. Thanks, Lukas
在 2025/11/11 星期二 6:22, Bjorn Helgaas 写道:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> We enabled ASPM too aggressively in v6.18-rc1. f3ac2ff14834 ("PCI/ASPM:
> Enable all ClockPM and ASPM states for devicetree platforms") enabled ASPM
> L0s, L1, and (if advertised) L1 PM Substates.
>
> df5192d9bb0e ("PCI/ASPM: Enable only L0s and L1 for devicetree platforms")
> (v6.18-rc3) backed off and omitted Clock PM and L1 Substates because we
> don't have good infrastructure to discover CLKREQ# support, and L1
> Substates may require device-specific configuration.
>
> L0s and L1 are generically discoverable and should not require
> device-specific support, but some devices advertise them even though they
> don't work correctly. This series is a way to add quirks avoid L0s and L1
> in this case.
>
Tested-by: Shawn Lin <shawn.lin@rock-chips.com>
>
> Bjorn Helgaas (4):
> PCI/ASPM: Cache L0s/L1 Supported so advertised link states can be
> overridden
> PCI/ASPM: Add pcie_aspm_remove_cap() to override advertised link
> states
> PCI/ASPM: Convert quirks to override advertised link states
> PCI/ASPM: Avoid L0s and L1 on Freescale Root Ports
>
> drivers/pci/pci.h | 2 ++
> drivers/pci/pcie/aspm.c | 25 +++++++++++++++++--------
> drivers/pci/probe.c | 7 +++++++
> drivers/pci/quirks.c | 38 +++++++++++++++++++-------------------
> include/linux/pci.h | 2 ++
> 5 files changed, 47 insertions(+), 27 deletions(-)
>
On Mon, Nov 10, 2025 at 04:22:24PM -0600, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
>
> We enabled ASPM too aggressively in v6.18-rc1. f3ac2ff14834 ("PCI/ASPM:
> Enable all ClockPM and ASPM states for devicetree platforms") enabled ASPM
> L0s, L1, and (if advertised) L1 PM Substates.
>
> df5192d9bb0e ("PCI/ASPM: Enable only L0s and L1 for devicetree platforms")
> (v6.18-rc3) backed off and omitted Clock PM and L1 Substates because we
> don't have good infrastructure to discover CLKREQ# support, and L1
> Substates may require device-specific configuration.
>
> L0s and L1 are generically discoverable and should not require
> device-specific support, but some devices advertise them even though they
> don't work correctly. This series is a way to add quirks avoid L0s and L1
> in this case.
>
>
> Bjorn Helgaas (4):
> PCI/ASPM: Cache L0s/L1 Supported so advertised link states can be
> overridden
> PCI/ASPM: Add pcie_aspm_remove_cap() to override advertised link
> states
> PCI/ASPM: Convert quirks to override advertised link states
> PCI/ASPM: Avoid L0s and L1 on Freescale Root Ports
>
> drivers/pci/pci.h | 2 ++
> drivers/pci/pcie/aspm.c | 25 +++++++++++++++++--------
> drivers/pci/probe.c | 7 +++++++
> drivers/pci/quirks.c | 38 +++++++++++++++++++-------------------
> include/linux/pci.h | 2 ++
> 5 files changed, 47 insertions(+), 27 deletions(-)
Applied to pci/for-linus, hoping for v6.18. Thanks Shawn and Lukas
for testing and reviewing. Any other comments and testing would be
very welcome.
I think we'll need to add a similar quirk for Christian's X1000
(https://lore.kernel.org/r/a41d2ca1-fcd9-c416-b111-a958e92e94bf@xenosoft.de),
but I don't know the device ID for it yet.
> --
>
> v1: https://lore.kernel.org/r/20251106183643.1963801-1-helgaas@kernel.org
>
> Changes between v1 and v2:
> - Cache just the two bits for L0s and L1 support, not the entire Link
> Capabilities (Lukas)
> - Add pcie_aspm_remove_cap() to override the ASPM Support bits in Link
> Capabilities (Lukas)
> - Convert existing quirks to use pcie_aspm_remove_cap() instead of
> pci_disable_link_state(), and from FINAL to HEADER (Mani)
Hello all!
I started to get to my email these discussions about Linux a few days ago. I have not subscribed any mailing list, or given anyone permission to include my name on such list!
So, if there is a moderator on this list, could you PLEASE remove immediately my email (rol7and@gmx.com) from the recipients/cc list? Thank you!
If there is no moderator, could the participants then please take care to remove my email from their messages? Thank you!
Regards,
Roland
> On Mon, Nov 10, 2025 at 04:22:24PM -0600, Bjorn Helgaas wrote:
>> From: Bjorn Helgaas <bhelgaas@google.com>
>>
>> We enabled ASPM too aggressively in v6.18-rc1. f3ac2ff14834 ("PCI/ASPM:
>> Enable all ClockPM and ASPM states for devicetree platforms") enabled
>> ASPM L0s, L1, and (if advertised) L1 PM Substates.
>>
>> df5192d9bb0e ("PCI/ASPM: Enable only L0s and L1 for devicetree
>> platforms") (v6.18-rc3) backed off and omitted Clock PM and L1 Substates
>> because we don't have good infrastructure to discover CLKREQ# support,
>> and L1 Substates may require device-specific configuration.
>>
>> L0s and L1 are generically discoverable and should not require
>> device-specific support, but some devices advertise them even though
>> they don't work correctly. This series is a way to add quirks avoid L0s
>> and L1 in this case.
>>
>>
>> Bjorn Helgaas (4):
>> PCI/ASPM: Cache L0s/L1 Supported so advertised link states can be
>> overridden
>> PCI/ASPM: Add pcie_aspm_remove_cap() to override advertised link
>> states
>> PCI/ASPM: Convert quirks to override advertised link states
>> PCI/ASPM: Avoid L0s and L1 on Freescale Root Ports
>>
>> drivers/pci/pci.h | 2 ++
>> drivers/pci/pcie/aspm.c | 25 +++++++++++++++++--------
>> drivers/pci/probe.c | 7 +++++++
>> drivers/pci/quirks.c | 38 +++++++++++++++++++-------------------
>> include/linux/pci.h | 2 ++
>> 5 files changed, 47 insertions(+), 27 deletions(-)
>
> Applied to pci/for-linus, hoping for v6.18. Thanks Shawn and Lukas
> for testing and reviewing. Any other comments and testing would be
> very welcome.
>
> I think we'll need to add a similar quirk for Christian's X1000
> (https://lore.kernel.org/r/a41d2ca1-fcd9-c416-b111-a958e92e94bf@xenosoft.de),
> but I don't know the device ID for it yet.
>
>> --
>>
>> v1:
>> https://lore.kernel.org/r/20251106183643.1963801-1-helgaas@kernel.org
>
>> Changes between v1 and v2:
>> - Cache just the two bits for L0s and L1 support, not the entire Link
>> Capabilities (Lukas)
>> - Add pcie_aspm_remove_cap() to override the ASPM Support bits in Link
>> Capabilities (Lukas)
>> - Convert existing quirks to use pcie_aspm_remove_cap() instead of
>> pci_disable_link_state(), and from FINAL to HEADER (Mani)
Regards
© 2016 - 2025 Red Hat, Inc.