[PATCH] PCI/ASPM: Mask ASPM states based on Devicetree properties

[PATCH] PCI/ASPM: Mask ASPM states based on Devicetree properties

[Krishna Chaitanya Chundru]

Some platforms require selectively disabling specific ASPM states on a
given PCIe link to avoid link instability or functional failures caused
by board-level connectivity constraints such as PCB routing, connectors,
slots, or external cabling.

Devicetree initially supported disabling ASPM L0s via the 'aspm-no-l0s'
property, and has since been extended to also allow disabling ASPM L1 and
L1 PM Substates using the 'aspm-no-l1' [1] and 'aspm-no-l1ss' [2]
properties. However, the ASPM driver does not currently account for these
properties when determining the default ASPM link state.

Update ASPM link initialization to check for these Devicetree properties
on either end of the link and mask the corresponding ASPM states from
link->aspm_support before applying the default ASPM policy.

Link [1]: https://github.com/devicetree-org/dt-schema/pull/188
Link [2]: https://github.com/devicetree-org/dt-schema/pull/190

Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
---
 drivers/pci/pcie/aspm.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 925373b98dff..3111ef7fd226 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -799,6 +799,13 @@ static void aspm_l1ss_init(struct pcie_link_state *link)
 
 #define FLAG(x, y, d)	(((x) & (PCIE_LINK_STATE_##y)) ? d : "")
 
+static bool pcie_link_has_aspm_override(const struct pcie_link_state *link,
+					const char *aspm)
+{
+	return (device_property_present(&link->pdev->dev, aspm) ||
+		device_property_present(&link->downstream->dev, aspm));
+}
+
 static void pcie_aspm_override_default_link_state(struct pcie_link_state *link)
 {
 	struct pci_dev *pdev = link->downstream;
@@ -806,6 +813,15 @@ static void pcie_aspm_override_default_link_state(struct pcie_link_state *link)
 
 	/* For devicetree platforms, enable L0s and L1 by default */
 	if (of_have_populated_dt()) {
+		if (pcie_link_has_aspm_override(link, "aspm-no-l0s"))
+			link->aspm_support &= ~PCIE_LINK_STATE_L0S;
+
+		if (pcie_link_has_aspm_override(link, "aspm-no-l1"))
+			link->aspm_support &= ~PCIE_LINK_STATE_L1;
+
+		if (pcie_link_has_aspm_override(link, "aspm-no-l1ss"))
+			link->aspm_support &= ~PCIE_LINK_STATE_L1SS;
+
 		if (link->aspm_support & PCIE_LINK_STATE_L0S)
 			link->aspm_default |= PCIE_LINK_STATE_L0S;
 		if (link->aspm_support & PCIE_LINK_STATE_L1)

---
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
change-id: 20260429-aspm-da55a54279e4

Best regards,
--  
Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

Re: [PATCH] PCI/ASPM: Mask ASPM states based on Devicetree properties

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Modifying `link->aspm_support` bypasses `pcie_config_aspm_l1ss()`, failing to disable ASPM L1 Substates in hardware if the bootloader previously enabled them.
- [Medium] The `aspm-no-l1` override only masks `PCIE_LINK_STATE_L1` from `link->aspm_support`, failing to mask the functionally dependent L1 PM Substates (`PCIE_LINK_STATE_L1SS`), leading to sysfs inconsistency.
--

commit 3739505256d4ced724de1faf6f3f111a9994fa59
Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>

PCI/ASPM: Mask ASPM states based on Devicetree properties

This commit updates ASPM link initialization to check for aspm-no-l0s,
aspm-no-l1, and aspm-no-l1ss Devicetree properties. It masks the
corresponding ASPM states from link->aspm_support before applying the
default ASPM policy.

Link: https://github.com/devicetree-org/dt-schema/pull/188
Link: https://github.com/devicetree-org/dt-schema/pull/190

> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 925373b98dff0..3111ef7fd226e 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c

[ ... ]

> @@ -806,6 +813,15 @@ static void pcie_aspm_override_default_link_state(struct pcie_link_state *link)
>  
>  	/* For devicetree platforms, enable L0s and L1 by default */
>  	if (of_have_populated_dt()) {
> +		if (pcie_link_has_aspm_override(link, "aspm-no-l0s"))
> +			link->aspm_support &= ~PCIE_LINK_STATE_L0S;
> +
> +		if (pcie_link_has_aspm_override(link, "aspm-no-l1"))
> +			link->aspm_support &= ~PCIE_LINK_STATE_L1;

Should this also clear PCIE_LINK_STATE_L1SS?

If we clear only PCIE_LINK_STATE_L1, link->aspm_capable will retain the
L1SS bits while dropping the L1 bit. The sysfs visibility check in
aspm_ctrl_attrs_are_visible() relies on aspm_capable, which means the main
L1 control attribute will be hidden, but the L1.1 and L1.2 substate
attributes will be incorrectly exposed.

Any userspace attempts to enable these exposed L1SS attributes via sysfs
will silently fail, as pcie_config_aspm_link() enforces:

if (!(state & PCIE_LINK_STATE_L1))
        state &= ~PCIE_LINK_STATE_L1SS;

> +
> +		if (pcie_link_has_aspm_override(link, "aspm-no-l1ss"))
> +			link->aspm_support &= ~PCIE_LINK_STATE_L1SS;

Will modifying link->aspm_support here bypass disabling ASPM L1 Substates
in hardware if the bootloader previously enabled them?

By clearing bits from link->aspm_support, the changes propagate to
link->aspm_capable. Later, when the kernel applies the ASPM policy in
pcie_config_aspm_link(), the hardware update for L1SS is guarded by:

if (link->aspm_capable & PCIE_LINK_STATE_L1SS)

Because the capability bit was cleared by the DT override, the kernel skips
calling pcie_config_aspm_l1ss() entirely, leaving the hardware actively
using L1SS if the firmware or bootloader had enabled it.

Would setting bits in link->aspm_disable instead (similar to
pci_disable_link_state()) correctly disable these states in hardware
while keeping aspm_capable intact?

> +
>  		if (link->aspm_support & PCIE_LINK_STATE_L0S)
>  			link->aspm_default |= PCIE_LINK_STATE_L0S;
>  		if (link->aspm_support & PCIE_LINK_STATE_L1)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260511-aspm-v1-1-b4a9fe955cf9@oss.qualcomm.com?part=1

Re: [PATCH] PCI/ASPM: Mask ASPM states based on Devicetree properties

[Krishna Chaitanya Chundru]

On 5/12/2026 6:16 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Modifying `link->aspm_support` bypasses `pcie_config_aspm_l1ss()`, failing to disable ASPM L1 Substates in hardware if the bootloader previously enabled them.
> - [Medium] The `aspm-no-l1` override only masks `PCIE_LINK_STATE_L1` from `link->aspm_support`, failing to mask the functionally dependent L1 PM Substates (`PCIE_LINK_STATE_L1SS`), leading to sysfs inconsistency.
> --
>
> commit 3739505256d4ced724de1faf6f3f111a9994fa59
> Author: Krishna Chaitanya Chundru <krishna.chundru@oss.qualcomm.com>
>
> PCI/ASPM: Mask ASPM states based on Devicetree properties
>
> This commit updates ASPM link initialization to check for aspm-no-l0s,
> aspm-no-l1, and aspm-no-l1ss Devicetree properties. It masks the
> corresponding ASPM states from link->aspm_support before applying the
> default ASPM policy.
>
> Link: https://github.com/devicetree-org/dt-schema/pull/188
> Link: https://github.com/devicetree-org/dt-schema/pull/190
>
>> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
>> index 925373b98dff0..3111ef7fd226e 100644
>> --- a/drivers/pci/pcie/aspm.c
>> +++ b/drivers/pci/pcie/aspm.c
> [ ... ]
>
>> @@ -806,6 +813,15 @@ static void pcie_aspm_override_default_link_state(struct pcie_link_state *link)
>>  
>>  	/* For devicetree platforms, enable L0s and L1 by default */
>>  	if (of_have_populated_dt()) {
>> +		if (pcie_link_has_aspm_override(link, "aspm-no-l0s"))
>> +			link->aspm_support &= ~PCIE_LINK_STATE_L0S;
>> +
>> +		if (pcie_link_has_aspm_override(link, "aspm-no-l1"))
>> +			link->aspm_support &= ~PCIE_LINK_STATE_L1;
> Should this also clear PCIE_LINK_STATE_L1SS?
>
> If we clear only PCIE_LINK_STATE_L1, link->aspm_capable will retain the
> L1SS bits while dropping the L1 bit. The sysfs visibility check in
> aspm_ctrl_attrs_are_visible() relies on aspm_capable, which means the main
> L1 control attribute will be hidden, but the L1.1 and L1.2 substate
> attributes will be incorrectly exposed.
>
> Any userspace attempts to enable these exposed L1SS attributes via sysfs
> will silently fail, as pcie_config_aspm_link() enforces:
>
> if (!(state & PCIE_LINK_STATE_L1))
>         state &= ~PCIE_LINK_STATE_L1SS;
ack
>> +
>> +		if (pcie_link_has_aspm_override(link, "aspm-no-l1ss"))
>> +			link->aspm_support &= ~PCIE_LINK_STATE_L1SS;
> Will modifying link->aspm_support here bypass disabling ASPM L1 Substates
> in hardware if the bootloader previously enabled them?
>
> By clearing bits from link->aspm_support, the changes propagate to
> link->aspm_capable. Later, when the kernel applies the ASPM policy in
> pcie_config_aspm_link(), the hardware update for L1SS is guarded by:
>
> if (link->aspm_capable & PCIE_LINK_STATE_L1SS)
>
> Because the capability bit was cleared by the DT override, the kernel skips
> calling pcie_config_aspm_l1ss() entirely, leaving the hardware actively
> using L1SS if the firmware or bootloader had enabled it.
>
> Would setting bits in link->aspm_disable instead (similar to
> pci_disable_link_state()) correctly disable these states in hardware
> while keeping aspm_capable intact?
we can't keep aspm_capable intact as this can expose l1ss capability to sysfs
as pointed
by you in the above, I will do like below, 
 if (pcie_link_has_aspm_override(link, "aspm-no-l1ss")) {
      /*
       * Clear L1SS in hardware before updating aspm_support. Once
       * aspm_capable is derived from aspm_support, pcie_config_aspm_link()
       * skips pcie_config_aspm_l1ss() entirely via the aspm_capable guard,
       * leaving firmware-enabled L1SS substates active in hardware.
       */
      if (link->aspm_support & PCIE_LINK_STATE_L1SS)
          pcie_config_aspm_l1ss(link, 0);
      link->aspm_support &= ~PCIE_LINK_STATE_L1SS;
  }

Bjorn,
can you share your taught's on this.

- Krishna Chaitanya.
>> +
>>  		if (link->aspm_support & PCIE_LINK_STATE_L0S)
>>  			link->aspm_default |= PCIE_LINK_STATE_L0S;
>>  		if (link->aspm_support & PCIE_LINK_STATE_L1)