Re: [PATCH 1/2] PCI: Write RCB only when it changes

[Haakon Bugge]

> On 30 Sep 2026, at 16:46, Stefan Roese <stefan.roese@xxxxxxxxxxx> 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@xxxxxxxxxx/

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@xxxxxxxxxxxxxxx
> Assisted-by: Claude:claude-opus-5-5
> Signed-off-by: Stefan Roese <stefan.roese@xxxxxxxxxxx>
> ---
> 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
> 
> 
>