[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 Substates, but that
caused regressions because these features depend on CLKREQ#, and not all
devices and form factors support it.

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>
Link: https://lore.kernel.org/r/22594781424C5C98+22cb5d61-19b1-4353-9818-3bb2b311da0b@radxa.com/
---

Mani, not sure what you think we should do here.  Here's a stab at it as a
strawman and in case anybody can test it.

Not sure about the message log message.  Maybe OK for testing, but might be
overly verbose ultimately.

---
 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..dbc74cc85bcb 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: DT platform, enabling%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

[Manivannan Sadhasivam]

On Mon, Oct 20, 2025 at 05:12:07PM -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 Substates, but that
> caused regressions because these features depend on CLKREQ#, and not all
> devices and form factors support it.

I believe we haven't concluded that CLKREQ# is the cluprit here. It is probably
the best bet, but there could be the device specific issues as well.

> 
> 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/

Closes?

> Reported-by: FUKAUMI Naoki <naoki@radxa.com>
> Link: https://lore.kernel.org/r/22594781424C5C98+22cb5d61-19b1-4353-9818-3bb2b311da0b@radxa.com/

Same here.

> ---
> 
> Mani, not sure what you think we should do here.  Here's a stab at it as a
> strawman and in case anybody can test it.
> 

Thanks Bjorn! I had a similar change slated to be sent post Diwali, but you beat
me to it.

> Not sure about the message log message.  Maybe OK for testing, but might be
> overly verbose ultimately.
> 

Let's have it for sometime, so we have some clue when someone reports issue with
ASPM.

> ---
>  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..dbc74cc85bcb 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;

Not sure if it is worth setting these states conditionally. Link state
enablement code should make use of the cached ASPM cap in 'link->aspm_capable'.

>  		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: DT platform, enabling%s%s\n",

I think you added the 'DT platform,' prefix while applying my patch earlier.
This is somewhat redundant since the print implies that the platform is based on
devicetree.

Rest LGTM!

- Mani

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

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

[Manivannan Sadhasivam]

On Tue, Oct 21, 2025 at 09:31:32AM +0530, Manivannan Sadhasivam wrote:
> On Mon, Oct 20, 2025 at 05:12:07PM -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 Substates, but that
> > caused regressions because these features depend on CLKREQ#, and not all
> > devices and form factors support it.
> 
> I believe we haven't concluded that CLKREQ# is the cluprit here. It is probably
> the best bet, but there could be the device specific issues as well.
> 
> > 
> > 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/
> 
> Closes?
> 
> > Reported-by: FUKAUMI Naoki <naoki@radxa.com>
> > Link: https://lore.kernel.org/r/22594781424C5C98+22cb5d61-19b1-4353-9818-3bb2b311da0b@radxa.com/
> 
> Same here.
> 
> > ---
> > 
> > Mani, not sure what you think we should do here.  Here's a stab at it as a
> > strawman and in case anybody can test it.
> > 
> 
> Thanks Bjorn! I had a similar change slated to be sent post Diwali, but you beat
> me to it.
> 
> > Not sure about the message log message.  Maybe OK for testing, but might be
> > overly verbose ultimately.
> > 
> 
> Let's have it for sometime, so we have some clue when someone reports issue with
> ASPM.
> 
> > ---
> >  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..dbc74cc85bcb 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;
> 
> Not sure if it is worth setting these states conditionally. Link state
> enablement code should make use of the cached ASPM cap in 'link->aspm_capable'.
> 

I see the point now. Without the check, we falsely claim that the ASPM states
are getting enabled. But we cannot be sure until the register write to LNKCTL
happens, which will depend on many factors (own device capability,
upstream/downstream port capability,...)

To avoid ambiguity, can we reword the log to something like,

	"ASPM: Overridding default states %s%s\n"

- Mani

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

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

[Bjorn Helgaas]

On Tue, Oct 21, 2025 at 09:31:22AM +0530, Manivannan Sadhasivam wrote:
> On Mon, Oct 20, 2025 at 05:12:07PM -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 Substates, but that
> > caused regressions because these features depend on CLKREQ#, and not all
> > devices and form factors support it.
> 
> I believe we haven't concluded that CLKREQ# is the cluprit here. It
> is probably the best bet, but there could be the device specific
> issues as well.

Yes.  There might be things in addition to CLKREQ#, but Clock PM and
L1SS definitely can't work without CLKREQ#.  I'll try to convey that
somehow.

> > Reported-by: Christian Zigotzky <chzigotzky@xenosoft.de>
> > Link: https://lore.kernel.org/r/db5c95a1-cf3e-46f9-8045-a1b04908051a@xenosoft.de/
> 
> Closes?

Yes, makes sense.  I didn't use Closes yet because I didn't want to
presume that it actually fixed the issue.  Will change if we get
testing results.

> > +	/* 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;
> 
> Not sure if it is worth setting these states conditionally. Link
> state enablement code should make use of the cached ASPM cap in
> 'link->aspm_capable'.

I wanted to avoid mentioning an ASPM state that is not advertised in
Link Capabilities, even if later code would ignore it.

> > +			pci_info(pdev, "ASPM: DT platform, enabling%s%s\n",
> 
> I think you added the 'DT platform,' prefix while applying my patch
> earlier.  This is somewhat redundant since the print implies that
> the platform is based on devicetree.

I think "DT platform" is probably unnecessary clutter.  I'll drop it
next time.

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

[Bjorn Helgaas]

On Tue, Oct 21, 2025 at 06:05:07PM +0530, Manivannan Sadhasivam wrote:
> On Tue, Oct 21, 2025 at 09:31:32AM +0530, Manivannan Sadhasivam wrote:
> > On Mon, Oct 20, 2025 at 05:12:07PM -0500, Bjorn Helgaas wrote:

> > > +		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;
> > 
> > Not sure if it is worth setting these states conditionally. Link
> > state enablement code should make use of the cached ASPM cap in
> > 'link->aspm_capable'.
> 
> I see the point now. Without the check, we falsely claim that the
> ASPM states are getting enabled. But we cannot be sure until the
> register write to LNKCTL happens, which will depend on many factors
> (own device capability, upstream/downstream port capability,...)
> 
> To avoid ambiguity, can we reword the log to something like,
> 
> 	"ASPM: Overridding default states %s%s\n"

I think aspm_support already includes the capabilities from both ends
of the link:

        if (parent_lnkcap & child_lnkcap & PCI_EXP_LNKCAP_ASPM_L0S)
                link->aspm_support |= PCIE_LINK_STATE_L0S;

        if (parent_lnkcap & child_lnkcap & PCI_EXP_LNKCAP_ASPM_L1)
                link->aspm_support |= PCIE_LINK_STATE_L1;

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

[Bjorn Helgaas]

Christian, Naoki, any chance you could test this patch on top of
v6.18-rc1 to see whether it resolves the problem you reported?

I'd like to verify that it works before merging it.

On Mon, Oct 20, 2025 at 05:12:07PM -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 Substates, but that
> caused regressions because these features depend on CLKREQ#, and not all
> devices and form factors support it.
> 
> 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>
> Link: https://lore.kernel.org/r/22594781424C5C98+22cb5d61-19b1-4353-9818-3bb2b311da0b@radxa.com/
> ---
> 
> Mani, not sure what you think we should do here.  Here's a stab at it as a
> strawman and in case anybody can test it.
> 
> Not sure about the message log message.  Maybe OK for testing, but might be
> overly verbose ultimately.
> 
> ---
>  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..dbc74cc85bcb 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: DT platform, enabling%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

[FUKAUMI Naoki]

Hi Bjorn,

On 10/23/25 04:13, Bjorn Helgaas wrote:
> Christian, Naoki, any chance you could test this patch on top of
> v6.18-rc1 to see whether it resolves the problem you reported?
> 
> I'd like to verify that it works before merging it.

I'll be testing now. May I test on v6.18-rc2 without the following patch?

  "PCI: dw-rockchip: Prevent advertising L1 Substates support"

Best regards,

--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.

> On Mon, Oct 20, 2025 at 05:12:07PM -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 Substates, but that
>> caused regressions because these features depend on CLKREQ#, and not all
>> devices and form factors support it.
>>
>> 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>
>> Link: https://lore.kernel.org/r/22594781424C5C98+22cb5d61-19b1-4353-9818-3bb2b311da0b@radxa.com/
>> ---
>>
>> Mani, not sure what you think we should do here.  Here's a stab at it as a
>> strawman and in case anybody can test it.
>>
>> Not sure about the message log message.  Maybe OK for testing, but might be
>> overly verbose ultimately.
>>
>> ---
>>   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..dbc74cc85bcb 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: DT platform, enabling%s%s\n",
>> +				 FLAG(override, L0S, " L0s"),
>> +				 FLAG(override, L1, " L1"));
>>   	}
>>   }
>>   
>> -- 
>> 2.43.0
>>
> 

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

[FUKAUMI Naoki]

# Fixes the ML address for linux-rockchip
# Please resend the original patch to linux-rockchip@lists.infradead.org

Hi Bjorn,

On 10/23/25 04:13, Bjorn Helgaas wrote:
> Christian, Naoki, any chance you could test this patch on top of
> v6.18-rc1 to see whether it resolves the problem you reported?
> 
> I'd like to verify that it works before merging it.

I'll be testing now. May I test on v6.18-rc2 without the following patch?

  "PCI: dw-rockchip: Prevent advertising L1 Substates support"

Best regards,

--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.

> On Mon, Oct 20, 2025 at 05:12:07PM -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 Substates, but that
>> caused regressions because these features depend on CLKREQ#, and not all
>> devices and form factors support it.
>>
>> 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>
>> Link: https://lore.kernel.org/r/22594781424C5C98+22cb5d61-19b1-4353-9818-3bb2b311da0b@radxa.com/
>> ---
>>
>> Mani, not sure what you think we should do here.  Here's a stab at it as a
>> strawman and in case anybody can test it.
>>
>> Not sure about the message log message.  Maybe OK for testing, but might be
>> overly verbose ultimately.
>>
>> ---
>>   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..dbc74cc85bcb 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: DT platform, enabling%s%s\n",
>> +				 FLAG(override, L0S, " L0s"),
>> +				 FLAG(override, L1, " L1"));
>>   	}
>>   }
>>   
>> -- 
>> 2.43.0
>>
> 

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

[FUKAUMI Naoki]

On 10/23/25 13:25, FUKAUMI Naoki wrote:
> # Fixes the ML address for linux-rockchip
> # Please resend the original patch to linux-rockchip@lists.infradead.org

and linuxppc-dev@lists.ozlabs.org?

> Hi Bjorn,
> 
> On 10/23/25 04:13, Bjorn Helgaas wrote:
>> Christian, Naoki, any chance you could test this patch on top of
>> v6.18-rc1 to see whether it resolves the problem you reported?
>>
>> I'd like to verify that it works before merging it.
> 
> I'll be testing now. May I test on v6.18-rc2 without the following patch?
> 
>   "PCI: dw-rockchip: Prevent advertising L1 Substates support"
> 
> Best regards,
> 
> -- 
> FUKAUMI Naoki
> Radxa Computer (Shenzhen) Co., Ltd.
> 
>> On Mon, Oct 20, 2025 at 05:12:07PM -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 Substates, but that
>>> caused regressions because these features depend on CLKREQ#, and not all
>>> devices and form factors support it.
>>>
>>> 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>
>>> Link: https://lore.kernel.org/ 
>>> r/22594781424C5C98+22cb5d61-19b1-4353-9818-3bb2b311da0b@radxa.com/

and https://lore.kernel.org/all/DDIW7ZP5K1VR.2I7VW56B9CZLF@cknow-tech.com/

maybe https://lore.kernel.org/all/20251015101304.3ec03e6b@bootlin.com/
(then +Cc: linux-arm-kernel@lists.infradead.org?)

Best regards,

--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.

>>> ---
>>>
>>> Mani, not sure what you think we should do here.  Here's a stab at it 
>>> as a
>>> strawman and in case anybody can test it.
>>>
>>> Not sure about the message log message.  Maybe OK for testing, but 
>>> might be
>>> overly verbose ultimately.
>>>
>>> ---
>>>   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..dbc74cc85bcb 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: DT platform, enabling%s%s\n",
>>> +                 FLAG(override, L0S, " L0s"),
>>> +                 FLAG(override, L1, " L1"));
>>>       }
>>>   }
>>> -- 
>>> 2.43.0
>>>
>>
> 
> 

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

[Manivannan Sadhasivam]

On Thu, Oct 23, 2025 at 01:25:17PM +0900, FUKAUMI Naoki wrote:
> # Fixes the ML address for linux-rockchip
> # Please resend the original patch to linux-rockchip@lists.infradead.org
> 
> Hi Bjorn,
> 
> On 10/23/25 04:13, Bjorn Helgaas wrote:
> > Christian, Naoki, any chance you could test this patch on top of
> > v6.18-rc1 to see whether it resolves the problem you reported?
> > 
> > I'd like to verify that it works before merging it.
> 
> I'll be testing now. May I test on v6.18-rc2 without the following patch?
> 
>  "PCI: dw-rockchip: Prevent advertising L1 Substates support"
> 

Yes, you should revert the above mentioned commit if you have applied it. It
will remove the L1ss CAP altogether making *this* patch irrelevant.

- Mani

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

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

[FUKAUMI Naoki]

Hi Bjorn,

On 10/21/25 07:12, 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 Substates, but that
> caused regressions because these features depend on CLKREQ#, and not all
> devices and form factors support it.
> 
> 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>
> Link: https://lore.kernel.org/r/22594781424C5C98+22cb5d61-19b1-4353-9818-3bb2b311da0b@radxa.com/

I've confirmed that this patch resolves the PCIe (M.2) Wi-Fi freezes or 
probe failures, as well as the NVMe SSD I/O errors occurring since 
v6.18-rc1.

Specifically, I verified this with the following configuration:

  ROCK 5A & M.2 RTL8852BE
  ROCK 5B & M.2 MT7921E, NVMe SSD
  ROCK 5T & on-board AX210, NVMe SSD x2
  ROCK 5 ITX+ & M.2 MT7922E, NVMe SSD x2

Therefore,

  Tested-by: FUKAUMI Naoki <naoki@radxa.com>

Best regards,

--
FUKAUMI Naoki
Radxa Computer (Shenzhen) Co., Ltd.

> ---
> 
> Mani, not sure what you think we should do here.  Here's a stab at it as a
> strawman and in case anybody can test it.
> 
> Not sure about the message log message.  Maybe OK for testing, but might be
> overly verbose ultimately.
> 
> ---
>   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..dbc74cc85bcb 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: DT platform, enabling%s%s\n",
> +				 FLAG(override, L0S, " L0s"),
> +				 FLAG(override, L1, " L1"));
>   	}
>   }
>   

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

[Herve Codina]

Hi,

On Thu, 23 Oct 2025 14:01:59 +0900
FUKAUMI Naoki <naoki@radxa.com> wrote:

...

> 
> maybe https://lore.kernel.org/all/20251015101304.3ec03e6b@bootlin.com/
> (then +Cc: linux-arm-kernel@lists.infradead.org?)
> 

For information, I tested the patch and I can confirm that my issue is
still there with the patch applied.

The only way to fix it seems to either fully disable ASPM or move to
performance ASPM policy.

Maybe my issue is simply a broken ASPM either on my host or my endpoint.

Best regards,
Hervé