[PATCH] PCI/ASPM: Enable only L0s and L1 for devicetree platforms

[PATCH] PCI/ASPM: Enable only L0s and L1 for devicetree platforms

[Bjorn Helgaas]

From: Bjorn Helgaas <bhelgaas@google.com>

f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree
platforms") enabled Clock Power Management and L1 PM Substates, but those
features depend on CLKREQ# and possibly other device-specific
configuration.  We don't know whether CLKREQ# is supported, so we shouldn't
blindly enable Clock PM and L1 PM Substates.

Enable only ASPM L0s and L1, and only when both ends of the link advertise
support for them.

Fixes: f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree platforms")
Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
Link: https://lore.kernel.org/r/db5c95a1-cf3e-46f9-8045-a1b04908051a@xenosoft.de/
Reported-by: FUKAUMI Naoki <naoki@radxa.com>
Closes: https://lore.kernel.org/r/22594781424C5C98+22cb5d61-19b1-4353-9818-3bb2b311da0b@radxa.com/
Reported-by: Herve Codina <herve.codina@bootlin.com>
Link: https://lore.kernel.org/r/20251015101304.3ec03e6b@bootlin.com/
Reported-by: Diederik de Haas <diederik@cknow-tech.com>
Link: https://lore.kernel.org/r/DDJXHRIRGTW9.GYC2ULZ5WQAL@cknow-tech.com/
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Tested-by: FUKAUMI Naoki <naoki@radxa.com>
---
I intend this for v6.18-rc3.

I think it will fix the issues reported by Diederik and FUKAUMI Naoki (both
on Rockchip).  I hope it will fix Christian's report on powerpc, but don't
have confirmation.  I think the performance regression Herve reported is
related, but this patch doesn't seem to fix it.

FUKAUMI Naoki's successful testing report:
https://lore.kernel.org/r/4B275FBD7B747BE6+a3e5b367-9710-4b67-9d66-3ea34fc30866@radxa.com/
---
 drivers/pci/pcie/aspm.c | 34 +++++++++-------------------------
 1 file changed, 9 insertions(+), 25 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index 7cc8281e7011..79b965158473 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -243,8 +243,7 @@ struct pcie_link_state {
 	/* Clock PM state */
 	u32 clkpm_capable:1;		/* Clock PM capable? */
 	u32 clkpm_enabled:1;		/* Current Clock PM state */
-	u32 clkpm_default:1;		/* Default Clock PM state by BIOS or
-					   override */
+	u32 clkpm_default:1;		/* Default Clock PM state by BIOS */
 	u32 clkpm_disable:1;		/* Clock PM disabled */
 };
 
@@ -376,18 +375,6 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
 	pcie_set_clkpm_nocheck(link, enable);
 }
 
-static void pcie_clkpm_override_default_link_state(struct pcie_link_state *link,
-						   int enabled)
-{
-	struct pci_dev *pdev = link->downstream;
-
-	/* For devicetree platforms, enable ClockPM by default */
-	if (of_have_populated_dt() && !enabled) {
-		link->clkpm_default = 1;
-		pci_info(pdev, "ASPM: DT platform, enabling ClockPM\n");
-	}
-}
-
 static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
 {
 	int capable = 1, enabled = 1;
@@ -410,7 +397,6 @@ static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
 	}
 	link->clkpm_enabled = enabled;
 	link->clkpm_default = enabled;
-	pcie_clkpm_override_default_link_state(link, enabled);
 	link->clkpm_capable = capable;
 	link->clkpm_disable = blacklist ? 1 : 0;
 }
@@ -811,19 +797,17 @@ static void pcie_aspm_override_default_link_state(struct pcie_link_state *link)
 	struct pci_dev *pdev = link->downstream;
 	u32 override;
 
-	/* For devicetree platforms, enable all ASPM states by default */
+	/* For devicetree platforms, enable L0s and L1 by default */
 	if (of_have_populated_dt()) {
-		link->aspm_default = PCIE_LINK_STATE_ASPM_ALL;
+		if (link->aspm_support & PCIE_LINK_STATE_L0S)
+			link->aspm_default |= PCIE_LINK_STATE_L0S;
+		if (link->aspm_support & PCIE_LINK_STATE_L1)
+			link->aspm_default |= PCIE_LINK_STATE_L1;
 		override = link->aspm_default & ~link->aspm_enabled;
 		if (override)
-			pci_info(pdev, "ASPM: DT platform, enabling%s%s%s%s%s%s%s\n",
-				 FLAG(override, L0S_UP, " L0s-up"),
-				 FLAG(override, L0S_DW, " L0s-dw"),
-				 FLAG(override, L1, " L1"),
-				 FLAG(override, L1_1, " ASPM-L1.1"),
-				 FLAG(override, L1_2, " ASPM-L1.2"),
-				 FLAG(override, L1_1_PCIPM, " PCI-PM-L1.1"),
-				 FLAG(override, L1_2_PCIPM, " PCI-PM-L1.2"));
+			pci_info(pdev, "ASPM: default states%s%s\n",
+				 FLAG(override, L0S, " L0s"),
+				 FLAG(override, L1, " L1"));
 	}
 }
 
-- 
2.43.0

Re: [PATCH] PCI/ASPM: Enable only L0s and L1 for devicetree platforms

[Bjorn Helgaas]

On Thu, Oct 23, 2025 at 01:06:26PM -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree
> platforms") enabled Clock Power Management and L1 PM Substates, but those
> features depend on CLKREQ# and possibly other device-specific
> configuration.  We don't know whether CLKREQ# is supported, so we shouldn't
> blindly enable Clock PM and L1 PM Substates.
> 
> Enable only ASPM L0s and L1, and only when both ends of the link advertise
> support for them.
> 
> Fixes: f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree platforms")
> Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> Link: https://lore.kernel.org/r/db5c95a1-cf3e-46f9-8045-a1b04908051a@xenosoft.de/
> Reported-by: FUKAUMI Naoki <naoki@radxa.com>
> Closes: https://lore.kernel.org/r/22594781424C5C98+22cb5d61-19b1-4353-9818-3bb2b311da0b@radxa.com/
> Reported-by: Herve Codina <herve.codina@bootlin.com>
> Link: https://lore.kernel.org/r/20251015101304.3ec03e6b@bootlin.com/
> Reported-by: Diederik de Haas <diederik@cknow-tech.com>
> Link: https://lore.kernel.org/r/DDJXHRIRGTW9.GYC2ULZ5WQAL@cknow-tech.com/
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Tested-by: FUKAUMI Naoki <naoki@radxa.com>

Provisionally applied to pci/for-linus, hoping to make v6.18-rc3.

Happy to add any testing reports or amend as needed.

> ---
> I intend this for v6.18-rc3.
> 
> I think it will fix the issues reported by Diederik and FUKAUMI Naoki (both
> on Rockchip).  I hope it will fix Christian's report on powerpc, but don't
> have confirmation.  I think the performance regression Herve reported is
> related, but this patch doesn't seem to fix it.
> 
> FUKAUMI Naoki's successful testing report:
> https://lore.kernel.org/r/4B275FBD7B747BE6+a3e5b367-9710-4b67-9d66-3ea34fc30866@radxa.com/
> ---
>  drivers/pci/pcie/aspm.c | 34 +++++++++-------------------------
>  1 file changed, 9 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 7cc8281e7011..79b965158473 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -243,8 +243,7 @@ struct pcie_link_state {
>  	/* Clock PM state */
>  	u32 clkpm_capable:1;		/* Clock PM capable? */
>  	u32 clkpm_enabled:1;		/* Current Clock PM state */
> -	u32 clkpm_default:1;		/* Default Clock PM state by BIOS or
> -					   override */
> +	u32 clkpm_default:1;		/* Default Clock PM state by BIOS */
>  	u32 clkpm_disable:1;		/* Clock PM disabled */
>  };
>  
> @@ -376,18 +375,6 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
>  	pcie_set_clkpm_nocheck(link, enable);
>  }
>  
> -static void pcie_clkpm_override_default_link_state(struct pcie_link_state *link,
> -						   int enabled)
> -{
> -	struct pci_dev *pdev = link->downstream;
> -
> -	/* For devicetree platforms, enable ClockPM by default */
> -	if (of_have_populated_dt() && !enabled) {
> -		link->clkpm_default = 1;
> -		pci_info(pdev, "ASPM: DT platform, enabling ClockPM\n");
> -	}
> -}
> -
>  static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
>  {
>  	int capable = 1, enabled = 1;
> @@ -410,7 +397,6 @@ static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
>  	}
>  	link->clkpm_enabled = enabled;
>  	link->clkpm_default = enabled;
> -	pcie_clkpm_override_default_link_state(link, enabled);
>  	link->clkpm_capable = capable;
>  	link->clkpm_disable = blacklist ? 1 : 0;
>  }
> @@ -811,19 +797,17 @@ static void pcie_aspm_override_default_link_state(struct pcie_link_state *link)
>  	struct pci_dev *pdev = link->downstream;
>  	u32 override;
>  
> -	/* For devicetree platforms, enable all ASPM states by default */
> +	/* For devicetree platforms, enable L0s and L1 by default */
>  	if (of_have_populated_dt()) {
> -		link->aspm_default = PCIE_LINK_STATE_ASPM_ALL;
> +		if (link->aspm_support & PCIE_LINK_STATE_L0S)
> +			link->aspm_default |= PCIE_LINK_STATE_L0S;
> +		if (link->aspm_support & PCIE_LINK_STATE_L1)
> +			link->aspm_default |= PCIE_LINK_STATE_L1;
>  		override = link->aspm_default & ~link->aspm_enabled;
>  		if (override)
> -			pci_info(pdev, "ASPM: DT platform, enabling%s%s%s%s%s%s%s\n",
> -				 FLAG(override, L0S_UP, " L0s-up"),
> -				 FLAG(override, L0S_DW, " L0s-dw"),
> -				 FLAG(override, L1, " L1"),
> -				 FLAG(override, L1_1, " ASPM-L1.1"),
> -				 FLAG(override, L1_2, " ASPM-L1.2"),
> -				 FLAG(override, L1_1_PCIPM, " PCI-PM-L1.1"),
> -				 FLAG(override, L1_2_PCIPM, " PCI-PM-L1.2"));
> +			pci_info(pdev, "ASPM: default states%s%s\n",
> +				 FLAG(override, L0S, " L0s"),
> +				 FLAG(override, L1, " L1"));
>  	}
>  }
>  
> -- 
> 2.43.0
> 

Re: [PATCH] PCI/ASPM: Enable only L0s and L1 for devicetree platforms

[Dragan Simic]

Hello Bjorn,

On Thursday, October 23, 2025 20:06 CEST, Bjorn Helgaas <helgaas@kernel.org> wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree
> platforms") enabled Clock Power Management and L1 PM Substates, but those
> features depend on CLKREQ# and possibly other device-specific
> configuration.  We don't know whether CLKREQ# is supported, so we shouldn't
> blindly enable Clock PM and L1 PM Substates.
> 
> Enable only ASPM L0s and L1, and only when both ends of the link advertise
> support for them.
> 
> Fixes: f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree platforms")
> Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> Link: https://lore.kernel.org/r/db5c95a1-cf3e-46f9-8045-a1b04908051a@xenosoft.de/
> Reported-by: FUKAUMI Naoki <naoki@radxa.com>
> Closes: https://lore.kernel.org/r/22594781424C5C98+22cb5d61-19b1-4353-9818-3bb2b311da0b@radxa.com/
> Reported-by: Herve Codina <herve.codina@bootlin.com>
> Link: https://lore.kernel.org/r/20251015101304.3ec03e6b@bootlin.com/
> Reported-by: Diederik de Haas <diederik@cknow-tech.com>
> Link: https://lore.kernel.org/r/DDJXHRIRGTW9.GYC2ULZ5WQAL@cknow-tech.com/
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Tested-by: FUKAUMI Naoki <naoki@radxa.com>
> ---
> I intend this for v6.18-rc3.
> 
> I think it will fix the issues reported by Diederik and FUKAUMI Naoki (both
> on Rockchip).  I hope it will fix Christian's report on powerpc, but don't
> have confirmation.  I think the performance regression Herve reported is
> related, but this patch doesn't seem to fix it.
> 
> FUKAUMI Naoki's successful testing report:
> https://lore.kernel.org/r/4B275FBD7B747BE6+a3e5b367-9710-4b67-9d66-3ea34fc30866@radxa.com/

I'm more than happy with the way ASPM patches for DT platforms and
Rockchip SoCs in particular are unfolding!  Admittedly, we've had
a rough start with the blanket enabling of ASPM, which followed the
theory, but the theory often differs from practice, so the combined
state of this and associated patches from Shawn should be fine.

Thank you very much for all the effort that included quite a lot
of back and forth, and please feel free to include

Acked-by: Dragan Simic <dsimic@manjaro.org>

> ---
>  drivers/pci/pcie/aspm.c | 34 +++++++++-------------------------
>  1 file changed, 9 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index 7cc8281e7011..79b965158473 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c
> @@ -243,8 +243,7 @@ struct pcie_link_state {
>  	/* Clock PM state */
>  	u32 clkpm_capable:1;		/* Clock PM capable? */
>  	u32 clkpm_enabled:1;		/* Current Clock PM state */
> -	u32 clkpm_default:1;		/* Default Clock PM state by BIOS or
> -					   override */
> +	u32 clkpm_default:1;		/* Default Clock PM state by BIOS */
>  	u32 clkpm_disable:1;		/* Clock PM disabled */
>  };
>  
> @@ -376,18 +375,6 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
>  	pcie_set_clkpm_nocheck(link, enable);
>  }
>  
> -static void pcie_clkpm_override_default_link_state(struct pcie_link_state *link,
> -						   int enabled)
> -{
> -	struct pci_dev *pdev = link->downstream;
> -
> -	/* For devicetree platforms, enable ClockPM by default */
> -	if (of_have_populated_dt() && !enabled) {
> -		link->clkpm_default = 1;
> -		pci_info(pdev, "ASPM: DT platform, enabling ClockPM\n");
> -	}
> -}
> -
>  static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
>  {
>  	int capable = 1, enabled = 1;
> @@ -410,7 +397,6 @@ static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
>  	}
>  	link->clkpm_enabled = enabled;
>  	link->clkpm_default = enabled;
> -	pcie_clkpm_override_default_link_state(link, enabled);
>  	link->clkpm_capable = capable;
>  	link->clkpm_disable = blacklist ? 1 : 0;
>  }
> @@ -811,19 +797,17 @@ static void pcie_aspm_override_default_link_state(struct pcie_link_state *link)
>  	struct pci_dev *pdev = link->downstream;
>  	u32 override;
>  
> -	/* For devicetree platforms, enable all ASPM states by default */
> +	/* For devicetree platforms, enable L0s and L1 by default */
>  	if (of_have_populated_dt()) {
> -		link->aspm_default = PCIE_LINK_STATE_ASPM_ALL;
> +		if (link->aspm_support & PCIE_LINK_STATE_L0S)
> +			link->aspm_default |= PCIE_LINK_STATE_L0S;
> +		if (link->aspm_support & PCIE_LINK_STATE_L1)
> +			link->aspm_default |= PCIE_LINK_STATE_L1;
>  		override = link->aspm_default & ~link->aspm_enabled;
>  		if (override)
> -			pci_info(pdev, "ASPM: DT platform, enabling%s%s%s%s%s%s%s\n",
> -				 FLAG(override, L0S_UP, " L0s-up"),
> -				 FLAG(override, L0S_DW, " L0s-dw"),
> -				 FLAG(override, L1, " L1"),
> -				 FLAG(override, L1_1, " ASPM-L1.1"),
> -				 FLAG(override, L1_2, " ASPM-L1.2"),
> -				 FLAG(override, L1_1_PCIPM, " PCI-PM-L1.1"),
> -				 FLAG(override, L1_2_PCIPM, " PCI-PM-L1.2"));
> +			pci_info(pdev, "ASPM: default states%s%s\n",
> +				 FLAG(override, L0S, " L0s"),
> +				 FLAG(override, L1, " L1"));
>  	}
>  }

Re: [PATCH] PCI/ASPM: Enable only L0s and L1 for devicetree platforms

[Diederik de Haas]

Hi Bjorn,

Thanks for the patch :-)

On Thu Oct 23, 2025 at 8:25 PM CEST, Bjorn Helgaas wrote:
> On Thu, Oct 23, 2025 at 01:06:26PM -0500, Bjorn Helgaas wrote:
>> From: Bjorn Helgaas <bhelgaas@google.com>
>> 
>> f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree
>> platforms") enabled Clock Power Management and L1 PM Substates, but those
>> features depend on CLKREQ# and possibly other device-specific
>> configuration.  We don't know whether CLKREQ# is supported, so we shouldn't
>> blindly enable Clock PM and L1 PM Substates.
>> 
>> Enable only ASPM L0s and L1, and only when both ends of the link advertise
>> support for them.
>> 
>> Fixes: f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree platforms")
>> Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
>> Link: https://lore.kernel.org/r/db5c95a1-cf3e-46f9-8045-a1b04908051a@xenosoft.de/
>> Reported-by: FUKAUMI Naoki <naoki@radxa.com>
>> Closes: https://lore.kernel.org/r/22594781424C5C98+22cb5d61-19b1-4353-9818-3bb2b311da0b@radxa.com/
>> Reported-by: Herve Codina <herve.codina@bootlin.com>
>> Link: https://lore.kernel.org/r/20251015101304.3ec03e6b@bootlin.com/
>> Reported-by: Diederik de Haas <diederik@cknow-tech.com>
>> Link: https://lore.kernel.org/r/DDJXHRIRGTW9.GYC2ULZ5WQAL@cknow-tech.com/
>> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
>> Tested-by: FUKAUMI Naoki <naoki@radxa.com>
>
> Provisionally applied to pci/for-linus, hoping to make v6.18-rc3.
>
> Happy to add any testing reports or amend as needed.

My build with your patch (on top of 6.18-rc2) just finished, so I
installed it and rebooted into it.
Happy to report that everything seems to be working fine and I can't
find any errors, warnings or other messages with 'nvme' in dmesg that
indicate sth could be wrong. IOW: it does indeed fix the issue I
reported earlier. So feel free to add my:

Tested-by: Diederik de Haas <diederik@cknow-tech.com>

Cheers,
  Diederik

>> ---
>> I intend this for v6.18-rc3.
>> 
>> I think it will fix the issues reported by Diederik and FUKAUMI Naoki (both
>> on Rockchip).  I hope it will fix Christian's report on powerpc, but don't
>> have confirmation.  I think the performance regression Herve reported is
>> related, but this patch doesn't seem to fix it.
>> 
>> FUKAUMI Naoki's successful testing report:
>> https://lore.kernel.org/r/4B275FBD7B747BE6+a3e5b367-9710-4b67-9d66-3ea34fc30866@radxa.com/
>> ---
>>  drivers/pci/pcie/aspm.c | 34 +++++++++-------------------------
>>  1 file changed, 9 insertions(+), 25 deletions(-)
>> 
>> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
>> index 7cc8281e7011..79b965158473 100644
>> --- a/drivers/pci/pcie/aspm.c
>> +++ b/drivers/pci/pcie/aspm.c
>> @@ -243,8 +243,7 @@ struct pcie_link_state {
>>  	/* Clock PM state */
>>  	u32 clkpm_capable:1;		/* Clock PM capable? */
>>  	u32 clkpm_enabled:1;		/* Current Clock PM state */
>> -	u32 clkpm_default:1;		/* Default Clock PM state by BIOS or
>> -					   override */
>> +	u32 clkpm_default:1;		/* Default Clock PM state by BIOS */
>>  	u32 clkpm_disable:1;		/* Clock PM disabled */
>>  };
>>  
>> @@ -376,18 +375,6 @@ static void pcie_set_clkpm(struct pcie_link_state *link, int enable)
>>  	pcie_set_clkpm_nocheck(link, enable);
>>  }
>>  
>> -static void pcie_clkpm_override_default_link_state(struct pcie_link_state *link,
>> -						   int enabled)
>> -{
>> -	struct pci_dev *pdev = link->downstream;
>> -
>> -	/* For devicetree platforms, enable ClockPM by default */
>> -	if (of_have_populated_dt() && !enabled) {
>> -		link->clkpm_default = 1;
>> -		pci_info(pdev, "ASPM: DT platform, enabling ClockPM\n");
>> -	}
>> -}
>> -
>>  static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
>>  {
>>  	int capable = 1, enabled = 1;
>> @@ -410,7 +397,6 @@ static void pcie_clkpm_cap_init(struct pcie_link_state *link, int blacklist)
>>  	}
>>  	link->clkpm_enabled = enabled;
>>  	link->clkpm_default = enabled;
>> -	pcie_clkpm_override_default_link_state(link, enabled);
>>  	link->clkpm_capable = capable;
>>  	link->clkpm_disable = blacklist ? 1 : 0;
>>  }
>> @@ -811,19 +797,17 @@ static void pcie_aspm_override_default_link_state(struct pcie_link_state *link)
>>  	struct pci_dev *pdev = link->downstream;
>>  	u32 override;
>>  
>> -	/* For devicetree platforms, enable all ASPM states by default */
>> +	/* For devicetree platforms, enable L0s and L1 by default */
>>  	if (of_have_populated_dt()) {
>> -		link->aspm_default = PCIE_LINK_STATE_ASPM_ALL;
>> +		if (link->aspm_support & PCIE_LINK_STATE_L0S)
>> +			link->aspm_default |= PCIE_LINK_STATE_L0S;
>> +		if (link->aspm_support & PCIE_LINK_STATE_L1)
>> +			link->aspm_default |= PCIE_LINK_STATE_L1;
>>  		override = link->aspm_default & ~link->aspm_enabled;
>>  		if (override)
>> -			pci_info(pdev, "ASPM: DT platform, enabling%s%s%s%s%s%s%s\n",
>> -				 FLAG(override, L0S_UP, " L0s-up"),
>> -				 FLAG(override, L0S_DW, " L0s-dw"),
>> -				 FLAG(override, L1, " L1"),
>> -				 FLAG(override, L1_1, " ASPM-L1.1"),
>> -				 FLAG(override, L1_2, " ASPM-L1.2"),
>> -				 FLAG(override, L1_1_PCIPM, " PCI-PM-L1.1"),
>> -				 FLAG(override, L1_2_PCIPM, " PCI-PM-L1.2"));
>> +			pci_info(pdev, "ASPM: default states%s%s\n",
>> +				 FLAG(override, L0S, " L0s"),
>> +				 FLAG(override, L1, " L1"));
>>  	}
>>  }
>>  
>> -- 
>> 2.43.0
>> 

Re: [PATCH] PCI/ASPM: Enable only L0s and L1 for devicetree platforms

[Bjorn Helgaas]

On Thu, Oct 23, 2025 at 08:27:25PM +0200, Dragan Simic wrote:
> On Thursday, October 23, 2025 20:06 CEST, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > From: Bjorn Helgaas <bhelgaas@google.com>
> > 
> > f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree
> > platforms") enabled Clock Power Management and L1 PM Substates, but those
> > features depend on CLKREQ# and possibly other device-specific
> > configuration.  We don't know whether CLKREQ# is supported, so we shouldn't
> > blindly enable Clock PM and L1 PM Substates.
> > 
> > Enable only ASPM L0s and L1, and only when both ends of the link advertise
> > support for them.
> > 
> > Fixes: f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree platforms")
> > Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> > Link: https://lore.kernel.org/r/db5c95a1-cf3e-46f9-8045-a1b04908051a@xenosoft.de/
> > Reported-by: FUKAUMI Naoki <naoki@radxa.com>
> > Closes: https://lore.kernel.org/r/22594781424C5C98+22cb5d61-19b1-4353-9818-3bb2b311da0b@radxa.com/
> > Reported-by: Herve Codina <herve.codina@bootlin.com>
> > Link: https://lore.kernel.org/r/20251015101304.3ec03e6b@bootlin.com/
> > Reported-by: Diederik de Haas <diederik@cknow-tech.com>
> > Link: https://lore.kernel.org/r/DDJXHRIRGTW9.GYC2ULZ5WQAL@cknow-tech.com/
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > Tested-by: FUKAUMI Naoki <naoki@radxa.com>
> > ---
> > I intend this for v6.18-rc3.
> > 
> > I think it will fix the issues reported by Diederik and FUKAUMI Naoki (both
> > on Rockchip).  I hope it will fix Christian's report on powerpc, but don't
> > have confirmation.  I think the performance regression Herve reported is
> > related, but this patch doesn't seem to fix it.
> > 
> > FUKAUMI Naoki's successful testing report:
> > https://lore.kernel.org/r/4B275FBD7B747BE6+a3e5b367-9710-4b67-9d66-3ea34fc30866@radxa.com/
> 
> I'm more than happy with the way ASPM patches for DT platforms and
> Rockchip SoCs in particular are unfolding!  Admittedly, we've had
> a rough start with the blanket enabling of ASPM, which followed the
> theory, but the theory often differs from practice, so the combined
> state of this and associated patches from Shawn should be fine.
> 
> Thank you very much for all the effort that included quite a lot
> of back and forth, and please feel free to include
> 
> Acked-by: Dragan Simic <dsimic@manjaro.org>

Added your ack; thanks for all your help!

Re: [PATCH] PCI/ASPM: Enable only L0s and L1 for devicetree platforms

[Bjorn Helgaas]

On Thu, Oct 23, 2025 at 09:59:41PM +0200, Diederik de Haas wrote:
> Hi Bjorn,
> 
> Thanks for the patch :-)
> 
> On Thu Oct 23, 2025 at 8:25 PM CEST, Bjorn Helgaas wrote:
> > On Thu, Oct 23, 2025 at 01:06:26PM -0500, Bjorn Helgaas wrote:
> >> From: Bjorn Helgaas <bhelgaas@google.com>
> >> 
> >> f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree
> >> platforms") enabled Clock Power Management and L1 PM Substates, but those
> >> features depend on CLKREQ# and possibly other device-specific
> >> configuration.  We don't know whether CLKREQ# is supported, so we shouldn't
> >> blindly enable Clock PM and L1 PM Substates.
> >> 
> >> Enable only ASPM L0s and L1, and only when both ends of the link advertise
> >> support for them.
> >> 
> >> Fixes: f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree platforms")
> >> Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> >> Link: https://lore.kernel.org/r/db5c95a1-cf3e-46f9-8045-a1b04908051a@xenosoft.de/
> >> Reported-by: FUKAUMI Naoki <naoki@radxa.com>
> >> Closes: https://lore.kernel.org/r/22594781424C5C98+22cb5d61-19b1-4353-9818-3bb2b311da0b@radxa.com/
> >> Reported-by: Herve Codina <herve.codina@bootlin.com>
> >> Link: https://lore.kernel.org/r/20251015101304.3ec03e6b@bootlin.com/
> >> Reported-by: Diederik de Haas <diederik@cknow-tech.com>
> >> Link: https://lore.kernel.org/r/DDJXHRIRGTW9.GYC2ULZ5WQAL@cknow-tech.com/
> >> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> >> Tested-by: FUKAUMI Naoki <naoki@radxa.com>
> >
> > Provisionally applied to pci/for-linus, hoping to make v6.18-rc3.
> >
> > Happy to add any testing reports or amend as needed.
> 
> My build with your patch (on top of 6.18-rc2) just finished, so I
> installed it and rebooted into it.
> Happy to report that everything seems to be working fine and I can't
> find any errors, warnings or other messages with 'nvme' in dmesg that
> indicate sth could be wrong. IOW: it does indeed fix the issue I
> reported earlier. So feel free to add my:
> 
> Tested-by: Diederik de Haas <diederik@cknow-tech.com>

That's tremendous, thank you for all your help and testing!  I know 
it's frustrating and time consuming when things break, and I really
appreciate your reports and help.

Bjorn

Re: [PATCH] PCI/ASPM: Enable only L0s and L1 for devicetree platforms

[Christian Zigotzky]

On 23/10/25 20:25, Bjorn Helgaas wrote:
 >> ---
 >> I intend this for v6.18-rc3.
 >>
 >> I think it will fix the issues reported by Diederik and FUKAUMI 
Naoki (both
 >> on Rockchip).  I hope it will fix Christian's report on powerpc, but 
don't
 >> have confirmation.
 >>

Hello Bjorn,

Thanks a lot for fixing the issue. :-)

I will test the RC3 of kernel 6.18 with activated CONFIG_PCIEASPM and 
CONFIG_PCIEASPM_DEFAULT next week.

Have a nice weekend,

Christian

-- 
Sent with BrassMonkey 33.8.2 
(https://github.com/chzigotzky/Web-Browsers-and-Suites-for-Linux-PPC/releases/tag/BrassMonkey_33.8.2)

Re: [PATCH] PCI/ASPM: Enable only L0s and L1 for devicetree platforms

[Johan Hovold]

On Thu, Oct 23, 2025 at 01:06:26PM -0500, Bjorn Helgaas wrote:
> From: Bjorn Helgaas <bhelgaas@google.com>
> 
> f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree
> platforms") enabled Clock Power Management and L1 PM Substates, but those
> features depend on CLKREQ# and possibly other device-specific
> configuration.  We don't know whether CLKREQ# is supported, so we shouldn't
> blindly enable Clock PM and L1 PM Substates.
> 
> Enable only ASPM L0s and L1, and only when both ends of the link advertise
> support for them.
> 
> Fixes: f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree platforms")
> Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> Link: https://lore.kernel.org/r/db5c95a1-cf3e-46f9-8045-a1b04908051a@xenosoft.de/
> Reported-by: FUKAUMI Naoki <naoki@radxa.com>
> Closes: https://lore.kernel.org/r/22594781424C5C98+22cb5d61-19b1-4353-9818-3bb2b311da0b@radxa.com/
> Reported-by: Herve Codina <herve.codina@bootlin.com>
> Link: https://lore.kernel.org/r/20251015101304.3ec03e6b@bootlin.com/
> Reported-by: Diederik de Haas <diederik@cknow-tech.com>
> Link: https://lore.kernel.org/r/DDJXHRIRGTW9.GYC2ULZ5WQAL@cknow-tech.com/
> Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> Tested-by: FUKAUMI Naoki <naoki@radxa.com>
> ---
> I intend this for v6.18-rc3.

Note that this will regress ASPM on Qualcomm platforms further by
disabling L1SS for devices that do not use pwrctrl (e.g. NVMe). ASPM
with pwrctrl is already broken since 6.15. [1]

Reverting also a729c1664619 ("PCI: qcom: Remove custom ASPM enablement
code") should avoid the new regression until a proper fix for the 6.15
regression is in place.

Johan


[1] https://lore.kernel.org/lkml/aH4JPBIk_GEoAezy@hovoldconsulting.com/

Re: [PATCH] PCI/ASPM: Enable only L0s and L1 for devicetree platforms

[Johan Hovold]

On Fri, Oct 24, 2025 at 05:12:38PM +0200, Johan Hovold wrote:
> On Thu, Oct 23, 2025 at 01:06:26PM -0500, Bjorn Helgaas wrote:
> > From: Bjorn Helgaas <bhelgaas@google.com>
> > 
> > f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree
> > platforms") enabled Clock Power Management and L1 PM Substates, but those
> > features depend on CLKREQ# and possibly other device-specific
> > configuration.  We don't know whether CLKREQ# is supported, so we shouldn't
> > blindly enable Clock PM and L1 PM Substates.
> > 
> > Enable only ASPM L0s and L1, and only when both ends of the link advertise
> > support for them.
> > 
> > Fixes: f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree platforms")
> > Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> > Link: https://lore.kernel.org/r/db5c95a1-cf3e-46f9-8045-a1b04908051a@xenosoft.de/
> > Reported-by: FUKAUMI Naoki <naoki@radxa.com>
> > Closes: https://lore.kernel.org/r/22594781424C5C98+22cb5d61-19b1-4353-9818-3bb2b311da0b@radxa.com/
> > Reported-by: Herve Codina <herve.codina@bootlin.com>
> > Link: https://lore.kernel.org/r/20251015101304.3ec03e6b@bootlin.com/
> > Reported-by: Diederik de Haas <diederik@cknow-tech.com>
> > Link: https://lore.kernel.org/r/DDJXHRIRGTW9.GYC2ULZ5WQAL@cknow-tech.com/
> > Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
> > Tested-by: FUKAUMI Naoki <naoki@radxa.com>
> > ---
> > I intend this for v6.18-rc3.
> 
> Note that this will regress ASPM on Qualcomm platforms further by
> disabling L1SS for devices that do not use pwrctrl (e.g. NVMe). ASPM
> with pwrctrl is already broken since 6.15. [1]

Actually, the 6.15 regression was fixed in 6.18-rc1 by the offending
commit, but pwrctrl devices will now also regress again.

> Reverting also a729c1664619 ("PCI: qcom: Remove custom ASPM enablement
> code") should avoid the new regression until a proper fix for the 6.15
> regression is in place.
 
Johan

> [1] https://lore.kernel.org/lkml/aH4JPBIk_GEoAezy@hovoldconsulting.com/

Re: [PATCH] PCI/ASPM: Enable only L0s and L1 for devicetree platforms

[Bjorn Helgaas]

[+cc Shawn, Frank]

On Fri, Oct 24, 2025 at 05:20:33PM +0200, Johan Hovold wrote:
> On Fri, Oct 24, 2025 at 05:12:38PM +0200, Johan Hovold wrote:
> > On Thu, Oct 23, 2025 at 01:06:26PM -0500, Bjorn Helgaas wrote:
> > > From: Bjorn Helgaas <bhelgaas@google.com>
> > > 
> > > f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for
> > > devicetree platforms") enabled Clock Power Management and L1 PM
> > > Substates, but those features depend on CLKREQ# and possibly
> > > other device-specific configuration.  We don't know whether
> > > CLKREQ# is supported, so we shouldn't blindly enable Clock PM
> > > and L1 PM Substates.
> > > 
> > > Enable only ASPM L0s and L1, and only when both ends of the link
> > > advertise support for them.
> > > 
> > > Fixes: f3ac2ff14834 ("PCI/ASPM: Enable all ClockPM and ASPM states for devicetree platforms")
> ...

> > > ---
> > > I intend this for v6.18-rc3.
> > 
> > Note that this will regress ASPM on Qualcomm platforms further by
> > disabling L1SS for devices that do not use pwrctrl (e.g. NVMe). ASPM
> > with pwrctrl is already broken since 6.15. [1]
> 
> Actually, the 6.15 regression was fixed in 6.18-rc1 by the offending
> commit, but pwrctrl devices will now also regress again.
> 
> > Reverting also a729c1664619 ("PCI: qcom: Remove custom ASPM enablement
> > code") should avoid the new regression until a proper fix for the 6.15
> > regression is in place.

Help me think through this.  I just sent a pull request [2] that
includes df5192d9bb0e ("PCI/ASPM: Enable only L0s and L1 for
devicetree platforms").  If all goes well, v6.18-rc3 will enable L0s
and L1 (but not L1SS) on Qualcomm platforms.

IIUC, if we then revert a729c1664619 ("PCI: qcom: Remove custom ASPM
enablement code"), it will enable L1SS again, but since this is done
in a dw_pcie_host_ops .post_init() hook, L1SS will only be enabled for
devices powered on at qcom-pcie probe time.  It will *not* be enabled
for pwrctrl devices because .post_init() was run when those devices
were powered off.

I think this is the same as in v6.17.  v6.18-rc1 enabled L1SS for
everything, including pwrctrl devices, because it was done in the PCI
enumeration path, not the host controller probe path.  I think that
enumeration is the right place to do this, but we need to figure out
how to do it in a generic way.  At a minimum, we need to know that
CLKREQ# is supported, and some platforms like dw-rockchip also need
device-specific configuration [3].

Bottom line, I think we need to revert a729c1664619 for v6.18 to get
all ASPM states including L1SS enabled on Qualcomm platforms for
non-pwrctrl devices.  I'll post a patch for this.

Then try to figure out how to make this work for pwrctrl devices for
v6.19.  Does this sound right?

Bjorn

> > [1] https://lore.kernel.org/lkml/aH4JPBIk_GEoAezy@hovoldconsulting.com/

[2] https://lore.kernel.org/r/20251024192903.GA1360890@bhelgaas
[3] https://lore.kernel.org/r/1761187883-150120-1-git-send-email-shawn.lin@rock-chips.com

Re: [PATCH] PCI/ASPM: Enable only L0s and L1 for devicetree platforms

[Johan Hovold]

On Fri, Oct 24, 2025 at 03:39:24PM -0500, Bjorn Helgaas wrote:
> On Fri, Oct 24, 2025 at 05:20:33PM +0200, Johan Hovold wrote:
> > On Fri, Oct 24, 2025 at 05:12:38PM +0200, Johan Hovold wrote:

> > > Note that this will regress ASPM on Qualcomm platforms further by
> > > disabling L1SS for devices that do not use pwrctrl (e.g. NVMe). ASPM
> > > with pwrctrl is already broken since 6.15. [1]
> > 
> > Actually, the 6.15 regression was fixed in 6.18-rc1 by the offending
> > commit, but pwrctrl devices will now also regress again.
> > 
> > > Reverting also a729c1664619 ("PCI: qcom: Remove custom ASPM enablement
> > > code") should avoid the new regression until a proper fix for the 6.15
> > > regression is in place.
> 
> Help me think through this.  I just sent a pull request [2] that
> includes df5192d9bb0e ("PCI/ASPM: Enable only L0s and L1 for
> devicetree platforms").  If all goes well, v6.18-rc3 will enable L0s
> and L1 (but not L1SS) on Qualcomm platforms.
> 
> IIUC, if we then revert a729c1664619 ("PCI: qcom: Remove custom ASPM
> enablement code"), it will enable L1SS again, but since this is done
> in a dw_pcie_host_ops .post_init() hook, L1SS will only be enabled for
> devices powered on at qcom-pcie probe time.  It will *not* be enabled
> for pwrctrl devices because .post_init() was run when those devices
> were powered off.

Correct.

> I think this is the same as in v6.17.  v6.18-rc1 enabled L1SS for
> everything, including pwrctrl devices, because it was done in the PCI
> enumeration path, not the host controller probe path.  I think that
> enumeration is the right place to do this, but we need to figure out
> how to do it in a generic way.  At a minimum, we need to know that
> CLKREQ# is supported, and some platforms like dw-rockchip also need
> device-specific configuration [3].
> 
> Bottom line, I think we need to revert a729c1664619 for v6.18 to get
> all ASPM states including L1SS enabled on Qualcomm platforms for
> non-pwrctrl devices.  I'll post a patch for this.

Right, that would at least restore the 6.17 state of things with respect
to Qualcomm machines.
 
> Then try to figure out how to make this work for pwrctrl devices for
> v6.19.  Does this sound right?

Yes, unless you can come up with a simple way to enable L1SS during
enumeration on Qualcomm platforms, for example, using a driver callback
that returns true for platforms using the 1.9.0 ops to indicate that
L1SS is supported. That should address also the 6.15 pwrctrl regression.

Johan

[PATCH] PCI/ASPM: Enable only L0s and L1 for devicetree platforms

[Christian Zigotzky]

Hi All,

I activated CONFIG_PCIEASPM and CONFIG_PCIEASPM_DEFAULT again for the 
RC3 of kernel 6.18. Unfortunately my AMD Radeon HD6870 doesn't work with 
the latest patches.

But that doesn't matter because we disable the above kernel options by 
default. We don't need power management for PCI Express because of boot 
issues and performance issues.

Cheers,
Christian

Re: [PATCH] PCI/ASPM: Enable only L0s and L1 for devicetree platforms

[Bjorn Helgaas]

On Mon, Oct 27, 2025 at 06:12:24PM +0100, Christian Zigotzky wrote:
> Hi All,
> 
> I activated CONFIG_PCIEASPM and CONFIG_PCIEASPM_DEFAULT again for the RC3 of
> kernel 6.18. Unfortunately my AMD Radeon HD6870 doesn't work with the latest
> patches.
> 
> But that doesn't matter because we disable the above kernel options by
> default. We don't need power management for PCI Express because of boot
> issues and performance issues.

It matters to me!  The kernel should work correctly with or without
CONFIG_PCIEASPM and any of the CONFIG_PCIEASPM_* settings.  We can't
expect users to know a magic combination of config settings to make
things work.

I assume AMD Radeon HD6870 is used on a variety of platforms, and I
don't see other reports of ASPM problems with it, so I suspect the
problem is something else.

  - v6.17 CONFIG_PCIEASPM=y, works OK

  - v6.18-rc3 CONFIG_PCIEASPM unset, works OK, as expected since we
    don't do anything with ASPM

  - v6.18-rc3 CONFIG_PCIEASPM=y, boot fails (this report, no logs)

I looked at Hypexed's logs from
https://github.com/chzigotzky/kernels/issues/17#issuecomment-3400419966,
all of which worked fine:

  - 6.18.0-a8-dmesg.log, looks like CONFIG_PCIEASPM unset, so we would
    expect this to be fine.

  - 6.18.0-a7-dmesg.log, CONFIG_PCIEASPM=y, ASPM fully enabled,
    reported to work fine.

    Hardware name: pasemi,nemo PA6T 0x900102 A-EON Amigaone X1000
    Found PA-PXP PCI host bridge.
    pci 0000:00:10.0: [1959:a002] type 01 class 0x060400 PCIe Root Port

    All the Root Ports are [1959:a002], and ASPM for 01:00.0 and
    05:12.0 (apparently the only endpoints that advertise ASPM) seemed
    to work fine.

  - 6.18.0-a7-2-dmesg.log, looks like CONFIG_PCIEASPM unset, so we would
    expect this to be fine.

So the only data point I see is that [1959:a002] seems to work.

Christian, can you collect the output of "sudo lspci -vv" from your
machine where CONFIG_PCIEASPM=y doesn't work?  Doesn't matter what
kernel you're running when you collect it.

I assume your machine is this:

  Hardware name: varisys,CYRUS5040 e5500 0x80240012 CoreNet Generic

  Found FSL PCI host bridge at 0x0000000ffe200000. Firmware bus number: 0->1
  Found FSL PCI host bridge at 0x0000000ffe201000. Firmware bus number: 0->8
  fsl-pci ffe200000.pcie: PCI host bridge to bus 0000:00
  pci 0000:00:00.0: [1957:0451] type 01 class 0x060400 PCIe Root Port
  pci 0000:01:00.0: [1002:6738] type 00 class 0x030000 PCIe Legacy Endpoint
  pci 0000:01:00.1: [1002:aa88] type 00 class 0x040300 PCIe Legacy Endpoint

  [1957:0451] Freescale (some kind of Root Port)
  [1002:6738] AMD Barts XT [Radeon HD 6870]
  [1002:aa88] AMD Barts HDMI Audio [Radeon HD 6790/6850/6870 / 7720 OEM]

I don't see any real info about that Freescale Root Port.

If you have a chance, could you try the patch below on top of
v6.18-rc3 with CONFIG_PCIEASPM=y?

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 214ed060ca1b..2b6d4e0958aa 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -2524,6 +2524,7 @@ static void quirk_disable_aspm_l0s_l1(struct pci_dev *dev)
  * disable both L0s and L1 for now to be safe.
  */
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ASMEDIA, 0x1080, quirk_disable_aspm_l0s_l1);
+DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_FREESCALE, 0x0451, quirk_disable_aspm_l0s_l1);
 
 /*
  * Some Pericom PCIe-to-PCI bridges in reverse mode need the PCIe Retrain

Re: [PATCH] PCI/ASPM: Enable only L0s and L1 for devicetree platforms

[Christian Zigotzky]

> On 29 October 2025 at 00:33 am, Bjorn Helgaas <helgaas@kernel.org> wrote:
> 
> On Mon, Oct 27, 2025 at 06:12:24PM +0100, Christian Zigotzky wrote:
>> Hi All,
>> 
>> I activated CONFIG_PCIEASPM and CONFIG_PCIEASPM_DEFAULT again for the RC3 of
>> kernel 6.18. Unfortunately my AMD Radeon HD6870 doesn't work with the latest
>> patches.
>> 
>> But that doesn't matter because we disable the above kernel options by
>> default. We don't need power management for PCI Express because of boot
>> issues and performance issues.
> 
> If you have a chance, could you try the patch below on top of
> v6.18-rc3 with CONFIG_PCIEASPM=y?
> 
> diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> index 214ed060ca1b..2b6d4e0958aa 100644
> --- a/drivers/pci/quirks.c
> +++ b/drivers/pci/quirks.c
> @@ -2524,6 +2524,7 @@ static void quirk_disable_aspm_l0s_l1(struct pci_dev *dev)
>  * disable both L0s and L1 for now to be safe.
>  */
> DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ASMEDIA, 0x1080, quirk_disable_aspm_l0s_l1);
> +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_FREESCALE, 0x0451, quirk_disable_aspm_l0s_l1);
> 
> /*
>  * Some Pericom PCIe-to-PCI bridges in reverse mode need the PCIe Retrain

Thanks for the patch. 

I will test it on my FSL Cyrus+ board over the weekend.
BTW, I also tested my PASemi Nemo board with the RC3 of kernel 6.18 and with power management for PCI Express enabled. Unfortunately, the installed AMD Radeon HD5870 does not work with power management for PCI Express enabled either.

Power management for PCI Express is not interesting for our machines because it is somewhat slower and we do not want power management to impair performance.
But it is a good thing for 24/7 servers.

- Christian

Re: [PATCH] PCI/ASPM: Enable only L0s and L1 for devicetree platforms

[Bjorn Helgaas]

On Wed, Oct 29, 2025 at 06:47:19AM +0100, Christian Zigotzky wrote:
> > On 29 October 2025 at 00:33 am, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Mon, Oct 27, 2025 at 06:12:24PM +0100, Christian Zigotzky wrote:
> >> Hi All,
> >> 
> >> I activated CONFIG_PCIEASPM and CONFIG_PCIEASPM_DEFAULT again for the RC3 of
> >> kernel 6.18. Unfortunately my AMD Radeon HD6870 doesn't work with the latest
> >> patches.
> >> 
> >> But that doesn't matter because we disable the above kernel options by
> >> default. We don't need power management for PCI Express because of boot
> >> issues and performance issues.
>  
> > If you have a chance, could you try the patch below on top of
> > v6.18-rc3 with CONFIG_PCIEASPM=y?
> > 
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index 214ed060ca1b..2b6d4e0958aa 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -2524,6 +2524,7 @@ static void quirk_disable_aspm_l0s_l1(struct pci_dev *dev)
> >  * disable both L0s and L1 for now to be safe.
> >  */
> > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ASMEDIA, 0x1080, quirk_disable_aspm_l0s_l1);
> > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_FREESCALE, 0x0451, quirk_disable_aspm_l0s_l1);
> > 
> > /*
> >  * Some Pericom PCIe-to-PCI bridges in reverse mode need the PCIe Retrain
> 
> Thanks for the patch. 
> 
> I will test it on my FSL Cyrus+ board over the weekend.
> BTW, I also tested my PASemi Nemo board with the RC3 of kernel 6.18
> and with power management for PCI Express enabled. Unfortunately,
> the installed AMD Radeon HD5870 does not work with power management
> for PCI Express enabled either.

Can you share the output of "sudo lspci -vv" and the dmesg log for
this Nemo board that doesn't work with v6.18-rc3?  I would still guess
that Nemo root port has an issue rather than the HD5870.

Hypexed reported that "pasemi,nemo PA6T 0x900102 A-EON Amigaone X1000"
worked fine on 0739473694c4, which was just before v6.18-rc1.  That
system has:

  pci 0000:00:10.0: [1959:a002] type 01 class 0x060400 PCIe Root Port
  pci 0000:00:10.0: PCI bridge to [bus 01]
  pci 0000:01:00.0: [1002:6610] type 00 class 0x030000 PCIe Legacy Endpoint
  pci 0000:01:00.0: ASPM: DT platform, enabling L0s-up L0s-dw L1 ASPM-L1.1 ASPM-L1.2 PCI-PM-L1.1 PCI-PM-L1.2

which is a [1959:a002] PA Semi PWRficient PCI-Express Root Port
leading to [1002:6610] AMD Oland XT [Radeon HD 8670 / R5 340X OEM / R7
250/350/350X OEM], and we tried to enable all ASPM states.

Bjorn

Re: [PATCH] PCI/ASPM: Enable only L0s and L1 for devicetree platforms

[Bjorn Helgaas]

On Wed, Oct 29, 2025 at 06:47:19AM +0100, Christian Zigotzky wrote:
> > On 29 October 2025 at 00:33 am, Bjorn Helgaas <helgaas@kernel.org> wrote:
> > On Mon, Oct 27, 2025 at 06:12:24PM +0100, Christian Zigotzky wrote:
> >> Hi All,
> >> 
> >> I activated CONFIG_PCIEASPM and CONFIG_PCIEASPM_DEFAULT again for the RC3 of
> >> kernel 6.18. Unfortunately my AMD Radeon HD6870 doesn't work with the latest
> >> patches.
> >> 
> >> But that doesn't matter because we disable the above kernel options by
> >> default. We don't need power management for PCI Express because of boot
> >> issues and performance issues.
> > 
> > If you have a chance, could you try the patch below on top of
> > v6.18-rc3 with CONFIG_PCIEASPM=y?
> > 
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index 214ed060ca1b..2b6d4e0958aa 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -2524,6 +2524,7 @@ static void quirk_disable_aspm_l0s_l1(struct pci_dev *dev)
> >  * disable both L0s and L1 for now to be safe.
> >  */
> > DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ASMEDIA, 0x1080, quirk_disable_aspm_l0s_l1);
> > +DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_FREESCALE, 0x0451, quirk_disable_aspm_l0s_l1);
> > 
> > /*
> >  * Some Pericom PCIe-to-PCI bridges in reverse mode need the PCIe Retrain
> 
> Thanks for the patch. 
> 
> I will test it on my FSL Cyrus+ board over the weekend.
> BTW, I also tested my PASemi Nemo board with the RC3 of kernel 6.18
> and with power management for PCI Express enabled. Unfortunately,
> the installed AMD Radeon HD5870 does not work with power management
> for PCI Express enabled either.

Oops, I made that fixup run too late.  Instead of the patch above, can
you test the one below?

You'll likely see something like this, which is a little misleading
because even though we claim "default L1" for 01:00.0 (or whatever
your Radeon is), the fact that L0s and L1 are disabled at the other
end of the link (00:00.0) should prevent us from actually enabling it:

  pci 0000:00:00.0: Disabling ASPM L0s/L1
  pci 0000:01:00.0: ASPM: default states L1

diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 214ed060ca1b..27777ded9a2c 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -2524,6 +2524,7 @@ static void quirk_disable_aspm_l0s_l1(struct pci_dev *dev)
  * disable both L0s and L1 for now to be safe.
  */
 DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_ASMEDIA, 0x1080, quirk_disable_aspm_l0s_l1);
+DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, 0x0451, quirk_disable_aspm_l0s_l1);
 
 /*
  * Some Pericom PCIe-to-PCI bridges in reverse mode need the PCIe Retrain