Linux PCI subsystem development
 help / color / mirror / Atom feed
* [PATCH 0/2] PCI: Fix Renesas uPD720201 hang after the RCB Link Control write
@ 2026-09-30 14:46 Stefan Roese
  2026-09-30 14:46 ` [PATCH 1/2] PCI: Write RCB only when it changes Stefan Roese
  2026-09-30 14:46 ` [PATCH 2/2] PCI/ASPM: Clear ASPM Control on links without common ASPM support Stefan Roese
  0 siblings, 2 replies; 9+ messages in thread
From: Stefan Roese @ 2026-09-30 14:46 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, linux-kernel, Håkon Bugge, Ilpo Järvinen,
	Lukas Wunner, Manivannan Sadhasivam, Krishna Chaitanya Chundru

Since commit 1a6845aaa6de ("PCI: Initialize RCB from
pci_configure_device()"), which went into v6.18.14 as dbe723b480e4, a
Renesas uPD720201 xHCI (1912:0014) behind the CPM Root Port of an AMD
Versal SoC hangs the system on every boot. The first access to the xHCI
BAR after the firmware download runs into PCIe completion timeouts.

The chip comes out of reset with LnkCtl 0x0003 (ASPM L0s and L1
enabled), although the Root Port supports no ASPM. pcie_aspm_cap_init()
returns early for such a link and never clears these bits. This went
unnoticed so far because the chip clears ASPM Control itself during the
firmware download by xhci-pci-renesas. Once the host has written Link
Control, even with the unchanged value, the chip no longer does so, and
ASPM stays enabled on a link that cannot support it.
pci_configure_rcb() does exactly such a write: it read-modify-writes
Link Control of every endpoint at enumeration, even when RCB does not
change.

What was checked on the board:

- Bisecting v6.18.10..v6.18.40 ends at dbe723b480e4.
- v6.18.40 with that commit reverted: good.
- v6.18.10, which does not have it, booted with xhci_pci_renesas
  blacklisted, then a single "setpci CAP_EXP+0x10.w=0003" (the
  unchanged value) before loading the driver: bad. Without the setpci:
  good.

Patch 1 writes RCB only when it changes. On this board RCB is 0 on both
ends, so Link Control is no longer written, and this alone fixes the
hang. It is the minimal fix and marked for stable.

Patch 2 closes the underlying gap: on a link without common ASPM
support, clear ASPM Control where a device has it set. This does not
depend on patch 1 and also covers any other Link Control write that
might come before the driver.

Testing: both patches were tested on v6.18.40 (AMD linux-xlnx) on the
Versal board: USB comes up without completion timeouts in 3 of 3 cold
boots, and lspci shows "ASPM Disabled" for the xHCI. On pci/next the
only change is that patch 2 uses the existing "fn" iterator. The series
builds without warnings (W=1, arm64 and x86_64 defconfig), but I could
not boot test it on pci/next, as this board does not run a mainline
kernel.

Similar reports with this chip and ASPM, possibly related:
- Qualcomm RB3Gen2 needs pcie_aspm=off: https://lkml.iu.edu/2603.3/02364.html
- RPi CM5 "HC died": https://github.com/raspberrypi/linux/issues/6849

#regzbot introduced: 1a6845aaa6de

Stefan Roese (2):
  PCI: Write RCB only when it changes
  PCI/ASPM: Clear ASPM Control on links without common ASPM support

 drivers/pci/pcie/aspm.c | 22 ++++++++++++++++++++--
 drivers/pci/probe.c     | 17 ++++++++++++-----
 2 files changed, 32 insertions(+), 7 deletions(-)

-- 
2.56.0


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] PCI: Write RCB only when it changes
  2026-09-30 14:46 [PATCH 0/2] PCI: Fix Renesas uPD720201 hang after the RCB Link Control write Stefan Roese
@ 2026-09-30 14:46 ` Stefan Roese
  2026-09-30 14:52   ` sashiko-bot
                     ` (2 more replies)
  2026-09-30 14:46 ` [PATCH 2/2] PCI/ASPM: Clear ASPM Control on links without common ASPM support Stefan Roese
  1 sibling, 3 replies; 9+ messages in thread
From: Stefan Roese @ 2026-09-30 14:46 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, linux-kernel, Håkon Bugge, Ilpo Järvinen,
	Lukas Wunner, Manivannan Sadhasivam, Krishna Chaitanya Chundru

pci_configure_rcb() does a read-modify-write of the Link Control
register of every endpoint at enumeration, even when RCB already has
the right value. Some devices react to any write of this register.

The Renesas uPD720201 xHCI (1912:0014) comes out of reset with ASPM L0s
and L1 enabled in Link Control. On a link whose Root Port supports no
ASPM, nothing else writes that register before the driver loads, and
the chip clears ASPM Control itself during the firmware download. After
a host write, even of the unchanged value 0x0003, it no longer does
so. ASPM stays enabled, and the first access to the xHCI BAR runs into
PCIe completion timeouts that hang the system.

Seen on an AMD Versal board (CPM Root Port without ASPM support): the
hang bisects to this commit, reverting it fixes it, and on a kernel
without it a single setpci write of the unchanged value reproduces it.

Read Link Control first and write it only when RCB has to change.

Fixes: 1a6845aaa6de ("PCI: Initialize RCB from pci_configure_device()")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Stefan Roese <stefan.roese@mailbox.org>
---
 drivers/pci/probe.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index 721daf5c5184..35e794df3be4 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2426,7 +2426,7 @@ static void pci_configure_serr(struct pci_dev *dev)
 static void pci_configure_rcb(struct pci_dev *dev)
 {
 	struct pci_dev *rp;
-	u16 rp_lnkctl;
+	u16 rp_lnkctl, lnkctl, rcb;
 
 	/*
 	 * Per PCIe r7.0, sec 7.5.3.7, RCB is only meaningful in Root Ports
@@ -2448,10 +2448,17 @@ static void pci_configure_rcb(struct pci_dev *dev)
 		return;
 
 	pcie_capability_read_word(rp, PCI_EXP_LNKCTL, &rp_lnkctl);
-	pcie_capability_clear_and_set_word(dev, PCI_EXP_LNKCTL,
-					   PCI_EXP_LNKCTL_RCB,
-					   (rp_lnkctl & PCI_EXP_LNKCTL_RCB) ?
-					   PCI_EXP_LNKCTL_RCB : 0);
+	rcb = rp_lnkctl & PCI_EXP_LNKCTL_RCB;
+
+	/*
+	 * Write Link Control only when RCB actually changes. Some devices
+	 * react to any write of this register, even one with an unchanged
+	 * value.
+	 */
+	pcie_capability_read_word(dev, PCI_EXP_LNKCTL, &lnkctl);
+	if ((lnkctl & PCI_EXP_LNKCTL_RCB) != rcb)
+		pcie_capability_clear_and_set_word(dev, PCI_EXP_LNKCTL,
+						   PCI_EXP_LNKCTL_RCB, rcb);
 }
 
 static void pci_configure_device(struct pci_dev *dev)
-- 
2.56.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] PCI/ASPM: Clear ASPM Control on links without common ASPM support
  2026-09-30 14:46 [PATCH 0/2] PCI: Fix Renesas uPD720201 hang after the RCB Link Control write Stefan Roese
  2026-09-30 14:46 ` [PATCH 1/2] PCI: Write RCB only when it changes Stefan Roese
@ 2026-09-30 14:46 ` Stefan Roese
  2026-09-30 15:06   ` sashiko-bot
  1 sibling, 1 reply; 9+ messages in thread
From: Stefan Roese @ 2026-09-30 14:46 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: linux-pci, linux-kernel, Håkon Bugge, Ilpo Järvinen,
	Lukas Wunner, Manivannan Sadhasivam, Krishna Chaitanya Chundru

pcie_aspm_cap_init() returns early when the two ends of a link share
no ASPM state. It then never touches Link Control, so ASPM Control
keeps whatever the device came out of reset with. A device that
enables ASPM by default, against a port that cannot do it, keeps ASPM
enabled, which the spec does not allow.

The Renesas uPD720201 xHCI (1912:0014) is such a device: it resets
with LnkCtl 0x0003 (L0s and L1 enabled) and sits behind the CPM Root
Port of AMD Versal, which supports no ASPM. It only works because the
chip clears ASPM Control itself during its firmware download, and it
stops doing so once the host has written Link Control.

Clear ASPM Control on every function of such a link, downstream
component first, and only where it is set.

Before (lspci -vvv -s 01:00.0):
  LnkCtl: ASPM L0s L1 Enabled; RCB 64 bytes, LnkDisable- CommClk-

After:
  pci 0000:01:00.0: ASPM: link supports no ASPM, clearing ASPM Control
  LnkCtl: ASPM Disabled; RCB 64 bytes, LnkDisable- CommClk-

Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Stefan Roese <stefan.roese@mailbox.org>
---
 drivers/pci/pcie/aspm.c | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
index f8e64971c6c3..13d7a6d3f6ad 100644
--- a/drivers/pci/pcie/aspm.c
+++ b/drivers/pci/pcie/aspm.c
@@ -931,6 +931,18 @@ static void pcie_aspm_override_default_link_state(struct pcie_link_state *link)
 	}
 }
 
+static void pcie_aspm_clear_aspmc(struct pci_dev *pdev)
+{
+	u16 lnkctl;
+
+	pcie_capability_read_word(pdev, PCI_EXP_LNKCTL, &lnkctl);
+	if (!(lnkctl & PCI_EXP_LNKCTL_ASPMC))
+		return;
+
+	pci_info(pdev, "ASPM: link supports no ASPM, clearing ASPM Control\n");
+	pcie_capability_clear_word(pdev, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_ASPMC);
+}
+
 static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 {
 	struct pci_dev *child = link->downstream, *parent = link->pdev;
@@ -947,11 +959,17 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
 
 	/*
 	 * If ASPM not supported, don't mess with the clocks and link,
-	 * bail out now.
+	 * bail out now. A device may still come out of reset with ASPM
+	 * Control set, which the rest of this code never touches for
+	 * such a link. Clear it, downstream component first.
 	 */
 	if (!(parent->aspm_l0s_support && child->aspm_l0s_support) &&
-	    !(parent->aspm_l1_support && child->aspm_l1_support))
+	    !(parent->aspm_l1_support && child->aspm_l1_support)) {
+		list_for_each_entry(fn, &linkbus->devices, bus_list)
+			pcie_aspm_clear_aspmc(fn);
+		pcie_aspm_clear_aspmc(parent);
 		return;
+	}
 
 	/* Configure common clock before checking latencies */
 	pcie_aspm_configure_common_clock(link);
-- 
2.56.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] PCI: Write RCB only when it changes
  2026-09-30 14:46 ` [PATCH 1/2] PCI: Write RCB only when it changes Stefan Roese
@ 2026-09-30 14:52   ` sashiko-bot
  2026-09-30 16:41   ` Haakon Bugge
  2026-09-30 23:22   ` Bjorn Helgaas
  2 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-09-30 14:52 UTC (permalink / raw)
  To: Stefan Roese; +Cc: linux-pci

> pci_configure_rcb() does a read-modify-write of the Link Control
> register of every endpoint at enumeration, even when RCB already has
> the right value. Some devices react to any write of this register.
> 
> The Renesas uPD720201 xHCI (1912:0014) comes out of reset with ASPM L0s
> and L1 enabled in Link Control. On a link whose Root Port supports no
> ASPM, nothing else writes that register before the driver loads, and
> the chip clears ASPM Control itself during the firmware download. After
> a host write, even of the unchanged value 0x0003, it no longer does
> so. ASPM stays enabled, and the first access to the xHCI BAR runs into
> PCIe completion timeouts that hang the system.
> 
> Seen on an AMD Versal board (CPM Root Port without ASPM support): the
> hang bisects to this commit, reverting it fixes it, and on a kernel
> without it a single setpci write of the unchanged value reproduces it.
> 
> Read Link Control first and write it only when RCB has to change.
> 
> Fixes: 1a6845aaa6de ("PCI: Initialize RCB from pci_configure_device()")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Stefan Roese <stefan.roese@mailbox.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930144650.3701516-1-stefan.roese@mailbox.org?part=1


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] PCI/ASPM: Clear ASPM Control on links without common ASPM support
  2026-09-30 14:46 ` [PATCH 2/2] PCI/ASPM: Clear ASPM Control on links without common ASPM support Stefan Roese
@ 2026-09-30 15:06   ` sashiko-bot
  0 siblings, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-09-30 15:06 UTC (permalink / raw)
  To: Stefan Roese; +Cc: linux-pci

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pci/aspm: cached saved state not updated after clearing ASPMC
--

commit d667f086616b3d0999815c8db89e04e4d40aea30
Author: Stefan Roese <stefan.roese@mailbox.org>

PCI/ASPM: Clear ASPM Control on links without common ASPM support

This commit ensures ASPM Control is cleared for all functions on a link that
lacks common ASPM support, processing downstream components first. This
prevents devices that enable ASPM by default from keeping it enabled when
connected to a port that does not support it.

> diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c
> index f8e64971c6c3d..13d7a6d3f6ad3 100644
> --- a/drivers/pci/pcie/aspm.c
> +++ b/drivers/pci/pcie/aspm.c

[ ... ]

> @@ -947,11 +959,17 @@ static void pcie_aspm_cap_init(struct pcie_link_state *link, int blacklist)
>  
>  	/*
>  	 * If ASPM not supported, don't mess with the clocks and link,
> -	 * bail out now.
> +	 * bail out now. A device may still come out of reset with ASPM
> +	 * Control set, which the rest of this code never touches for
> +	 * such a link. Clear it, downstream component first.
>  	 */
>  	if (!(parent->aspm_l0s_support && child->aspm_l0s_support) &&
> -	    !(parent->aspm_l1_support && child->aspm_l1_support))
> +	    !(parent->aspm_l1_support && child->aspm_l1_support)) {
> +		list_for_each_entry(fn, &linkbus->devices, bus_list)
> +			pcie_aspm_clear_aspmc(fn);
> +		pcie_aspm_clear_aspmc(parent);

[Severity: High]
Does this bypass updating the parent's cached saved state?

By clearing the ASPM Control in hardware but returning immediately without
updating the cache (e.g., via pci_update_aspm_saved_state()), it appears the
parent bridge's saved_config_space will retain the stale ASPMC enabled value.

If a non-ASPM-capable device is hot-added, and the system undergoes an AER or
DPC reset before any power management event forces a state save, could
pci_restore_state() use this stale cache to erroneously re-enable ASPM on the
parent port?

This might lead to completion timeouts or link hangs.

>  		return;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260930144650.3701516-1-stefan.roese@mailbox.org?part=2

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] PCI: Write RCB only when it changes
  2026-09-30 14:46 ` [PATCH 1/2] PCI: Write RCB only when it changes Stefan Roese
  2026-09-30 14:52   ` sashiko-bot
@ 2026-09-30 16:41   ` Haakon Bugge
  2026-10-02  9:10     ` Stefan Roese
  2026-09-30 23:22   ` Bjorn Helgaas
  2 siblings, 1 reply; 9+ messages in thread
From: Haakon Bugge @ 2026-09-30 16:41 UTC (permalink / raw)
  To: Stefan Roese
  Cc: Bjorn Helgaas, linux-pci@vger.kernel.org,
	linux-kernel@vger.kernel.org, Ilpo Järvinen, Lukas Wunner,
	Manivannan Sadhasivam, Krishna Chaitanya Chundru



> On 30 Sep 2026, at 16:46, Stefan Roese <stefan.roese@mailbox.org> wrote:
> 
> pci_configure_rcb() does a read-modify-write of the Link Control
> register of every endpoint at enumeration, even when RCB already has
> the right value. Some devices react to any write of this register.
> 
> The Renesas uPD720201 xHCI (1912:0014) comes out of reset with ASPM L0s
> and L1 enabled in Link Control. On a link whose Root Port supports no
> ASPM, nothing else writes that register before the driver loads, and
> the chip clears ASPM Control itself during the firmware download. After
> a host write, even of the unchanged value 0x0003, it no longer does
> so. ASPM stays enabled, and the first access to the xHCI BAR runs into
> PCIe completion timeouts that hang the system.
> 
> Seen on an AMD Versal board (CPM Root Port without ASPM support): the
> hang bisects to this commit, reverting it fixes it, and on a kernel
> without it a single setpci write of the unchanged value reproduces it.
> 
> Read Link Control first and write it only when RCB has to change.

This was debated during the review of 1a6845aaa6de. The conditional
logic to avoid a Link Control write when RCB already matched was
considered “over-engineered”:
https://lore.kernel.org/lkml/20260122130957.68757-2-haakon.bugge@oracle.com/

Since an unconditional write to Link Control while programming RCB
does not violate the PCIe specification, this appears to be
device-specific behavior and should be handled with a device quirk
instead.


Thxs, Håkon

> 
> Fixes: 1a6845aaa6de ("PCI: Initialize RCB from pci_configure_device()")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Stefan Roese <stefan.roese@mailbox.org>
> ---
> drivers/pci/probe.c | 17 ++++++++++++-----
> 1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 721daf5c5184..35e794df3be4 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2426,7 +2426,7 @@ static void pci_configure_serr(struct pci_dev *dev)
> static void pci_configure_rcb(struct pci_dev *dev)
> {
> struct pci_dev *rp;
> - u16 rp_lnkctl;
> + u16 rp_lnkctl, lnkctl, rcb;
> 
> /*
> * Per PCIe r7.0, sec 7.5.3.7, RCB is only meaningful in Root Ports
> @@ -2448,10 +2448,17 @@ static void pci_configure_rcb(struct pci_dev *dev)
> return;
> 
> pcie_capability_read_word(rp, PCI_EXP_LNKCTL, &rp_lnkctl);
> - pcie_capability_clear_and_set_word(dev, PCI_EXP_LNKCTL,
> - PCI_EXP_LNKCTL_RCB,
> - (rp_lnkctl & PCI_EXP_LNKCTL_RCB) ?
> - PCI_EXP_LNKCTL_RCB : 0);
> + rcb = rp_lnkctl & PCI_EXP_LNKCTL_RCB;
> +
> + /*
> + * Write Link Control only when RCB actually changes. Some devices
> + * react to any write of this register, even one with an unchanged
> + * value.
> + */
> + pcie_capability_read_word(dev, PCI_EXP_LNKCTL, &lnkctl);
> + if ((lnkctl & PCI_EXP_LNKCTL_RCB) != rcb)
> + pcie_capability_clear_and_set_word(dev, PCI_EXP_LNKCTL,
> + PCI_EXP_LNKCTL_RCB, rcb);
> }
> 
> static void pci_configure_device(struct pci_dev *dev)
> -- 
> 2.56.0
> 
> 
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] PCI: Write RCB only when it changes
  2026-09-30 14:46 ` [PATCH 1/2] PCI: Write RCB only when it changes Stefan Roese
  2026-09-30 14:52   ` sashiko-bot
  2026-09-30 16:41   ` Haakon Bugge
@ 2026-09-30 23:22   ` Bjorn Helgaas
  2026-10-02  9:09     ` Stefan Roese
  2 siblings, 1 reply; 9+ messages in thread
From: Bjorn Helgaas @ 2026-09-30 23:22 UTC (permalink / raw)
  To: Stefan Roese
  Cc: Bjorn Helgaas, linux-pci, linux-kernel, Håkon Bugge,
	Ilpo Järvinen, Lukas Wunner, Manivannan Sadhasivam,
	Krishna Chaitanya Chundru

On Wed, Sep 30, 2026 at 04:46:49PM +0200, Stefan Roese wrote:
> pci_configure_rcb() does a read-modify-write of the Link Control
> register of every endpoint at enumeration, even when RCB already has
> the right value. Some devices react to any write of this register.
> 
> The Renesas uPD720201 xHCI (1912:0014) comes out of reset with ASPM L0s
> and L1 enabled in Link Control. On a link whose Root Port supports no
> ASPM, nothing else writes that register before the driver loads, and
> the chip clears ASPM Control itself during the firmware download. After
> a host write, even of the unchanged value 0x0003, it no longer does
> so. ASPM stays enabled, and the first access to the xHCI BAR runs into
> PCIe completion timeouts that hang the system.

In "After a host write, even of the unchanged value 0x0003, it no
longer does so", what are you saying it no longer does?

Are you saying the chip no longer clears ASPM Control during firmware
download?

The PCIe Mini Card CEM and M.2 specs both say L0s and L1 should be
enabled by default, so I guess it makes sense that they're set when
coming out of reset.

But PCIe r7.0, sec 5.4.1.4, says the result is undefined if software
enables L0s when the other end of the link doesn't support it, and I
guess writing 0x0003 (ASPM L0s and L1 enabled) counts as enabling L0s,
and we certainly got undefined results.

> Seen on an AMD Versal board (CPM Root Port without ASPM support): the
> hang bisects to this commit, reverting it fixes it, and on a kernel
> without it a single setpci write of the unchanged value reproduces it.
> 
> Read Link Control first and write it only when RCB has to change.
> 
> Fixes: 1a6845aaa6de ("PCI: Initialize RCB from pci_configure_device()")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Stefan Roese <stefan.roese@mailbox.org>
> ---
>  drivers/pci/probe.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 721daf5c5184..35e794df3be4 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2426,7 +2426,7 @@ static void pci_configure_serr(struct pci_dev *dev)
>  static void pci_configure_rcb(struct pci_dev *dev)
>  {
>  	struct pci_dev *rp;
> -	u16 rp_lnkctl;
> +	u16 rp_lnkctl, lnkctl, rcb;
>  
>  	/*
>  	 * Per PCIe r7.0, sec 7.5.3.7, RCB is only meaningful in Root Ports
> @@ -2448,10 +2448,17 @@ static void pci_configure_rcb(struct pci_dev *dev)
>  		return;
>  
>  	pcie_capability_read_word(rp, PCI_EXP_LNKCTL, &rp_lnkctl);
> -	pcie_capability_clear_and_set_word(dev, PCI_EXP_LNKCTL,
> -					   PCI_EXP_LNKCTL_RCB,
> -					   (rp_lnkctl & PCI_EXP_LNKCTL_RCB) ?
> -					   PCI_EXP_LNKCTL_RCB : 0);
> +	rcb = rp_lnkctl & PCI_EXP_LNKCTL_RCB;
> +
> +	/*
> +	 * Write Link Control only when RCB actually changes. Some devices
> +	 * react to any write of this register, even one with an unchanged
> +	 * value.
> +	 */
> +	pcie_capability_read_word(dev, PCI_EXP_LNKCTL, &lnkctl);
> +	if ((lnkctl & PCI_EXP_LNKCTL_RCB) != rcb)
> +		pcie_capability_clear_and_set_word(dev, PCI_EXP_LNKCTL,
> +						   PCI_EXP_LNKCTL_RCB, rcb);

What if we just did this:

  if (rp_lnkctl & PCI_EXP_LNKCTL_RCB)
    pcie_capability_set_word(dev, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_RCB);

I don't know if it's ever necessary to *clear* RCB.  If RCB is set in
an Endpoint when it's not set in the Root Port, that would be a
firmware configuration error.

Either way, it's ugly magic to avoid the ASPM Control write here based
on the unrelated RCB settings.  But avoiding the read/modify/write is
probably worth doing just from a performance point of view.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] PCI: Write RCB only when it changes
  2026-09-30 23:22   ` Bjorn Helgaas
@ 2026-10-02  9:09     ` Stefan Roese
  0 siblings, 0 replies; 9+ messages in thread
From: Stefan Roese @ 2026-10-02  9:09 UTC (permalink / raw)
  To: Bjorn Helgaas
  Cc: Bjorn Helgaas, linux-pci, linux-kernel, Håkon Bugge,
	Ilpo Järvinen, Lukas Wunner, Manivannan Sadhasivam,
	Krishna Chaitanya Chundru

On Wed, Sep 30, 2026 at 06:22:14PM -0500, Bjorn Helgaas wrote:
> In "After a host write, even of the unchanged value 0x0003, it no
> longer does so", what are you saying it no longer does?
>
> Are you saying the chip no longer clears ASPM Control during firmware
> download?

Yes, exactly. I watched Link Control with a kprobe on the config
write path. Without any host write to Link Control, it changes from
0x0003 to 0x0000 about 11-19 ms into the firmware download by
xhci-pci-renesas, between two config writes of the driver to its
download registers. Linux does not write Link Control there, so the
chip does it itself.

After a single host write of the unchanged value 0x0003, either from
pci_configure_rcb() or from a manual setpci on a kernel without
1a6845aaa6de, Link Control stays 0x0003 through the whole download.
The first MMIO access to the xHCI BAR afterwards ends in completion
timeouts, an AER error and link loss.

> But PCIe r7.0, sec 5.4.1.4, says the result is undefined if software
> enables L0s when the other end of the link doesn't support it, and I
> guess writing 0x0003 (ASPM L0s and L1 enabled) counts as enabling L0s,
> and we certainly got undefined results.

Agreed. The chip's own clearing has been hiding that ASPM is enabled
against a Root Port that supports no ASPM. So the real fix is in
aspm.c (v1 2/2, now 1/2), and the RCB write is only what exposed it.

> What if we just did this:
>
>   if (rp_lnkctl & PCI_EXP_LNKCTL_RCB)
>     pcie_capability_set_word(dev, PCI_EXP_LNKCTL, PCI_EXP_LNKCTL_RCB);

Fine with me. On this board RCB is clear in the Root Port, so this
also avoids the write here.

v2 makes the aspm.c change the first patch (with Fixes: and stable),
adds the saved state update that sashiko pointed out, and uses your
set-only variant as the second patch. I tested the combinations on the
board, v6.18.40 with the patches backported, 3 cold boots each:

- aspm.c patch alone: good, the "clearing ASPM Control" message shows
  up for the xHCI, after the RCB write.
- both patches: good.
- set-only patch alone, booted with pcie_aspm=off: good, the chip
  clears ASPM Control itself again.
- aspm.c patch alone, booted with pcie_aspm=off: bad, completion
  timeouts as before.

The last case is why the set-only patch now carries Fixes: and stable
as well: with pcie_aspm=off the aspm.c change never runs, and only
avoiding the Link Control write keeps the system alive. The same holds
for CONFIG_PCIEASPM=n. If you would rather take the set-only patch
without the stable tag, that is fine with me.

v2 follows.

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] PCI: Write RCB only when it changes
  2026-09-30 16:41   ` Haakon Bugge
@ 2026-10-02  9:10     ` Stefan Roese
  0 siblings, 0 replies; 9+ messages in thread
From: Stefan Roese @ 2026-10-02  9:10 UTC (permalink / raw)
  To: Haakon Bugge
  Cc: Bjorn Helgaas, linux-pci, linux-kernel, Ilpo Järvinen,
	Lukas Wunner, Manivannan Sadhasivam, Krishna Chaitanya Chundru

Hi Håkon,

On 30 Sep 2026, at 16:41, Haakon Bugge wrote:
> Since an unconditional write to Link Control while programming RCB
> does not violate the PCIe specification, this appears to be
> device-specific behavior and should be handled with a device quirk
> instead.

You're right that the RCB write itself is legal. What is not legal is
the state it leaves behind: ASPM L0s/L1 enabled on a link whose Root
Port supports no ASPM (r7.0, sec 5.4.1.4, as Bjorn pointed out).
pcie_aspm_cap_init() returns early for such a link and never clears
ASPM Control, so this is not specific to one device.

I did look at a quirk. The existing helper, pcie_aspm_remove_cap(),
only clears the support flags, which leads into the same early return
and leaves ASPM Control set. A quirk that clears it would have to
repeat the aspm.c link checks. So v2 will fix this in aspm.c, and
keep only Bjorn's set-only RCB change in probe.c.

Thanks,
Stefan

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-10-02  9:10 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-30 14:46 [PATCH 0/2] PCI: Fix Renesas uPD720201 hang after the RCB Link Control write Stefan Roese
2026-09-30 14:46 ` [PATCH 1/2] PCI: Write RCB only when it changes Stefan Roese
2026-09-30 14:52   ` sashiko-bot
2026-09-30 16:41   ` Haakon Bugge
2026-10-02  9:10     ` Stefan Roese
2026-09-30 23:22   ` Bjorn Helgaas
2026-10-02  9:09     ` Stefan Roese
2026-09-30 14:46 ` [PATCH 2/2] PCI/ASPM: Clear ASPM Control on links without common ASPM support Stefan Roese
2026-09-30 15:06   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox