Re: [PATCH v8] PCI: loongson: Override PCIe bridge supported speeds for Loongson-3C6000 series

[Bjorn Helgaas <helgaas@kernel.org>]

[+cc Mario]

On Sun, Apr 12, 2026 at 06:17:31PM +0800, Xi Ruoyao wrote:
> From: Ziyao Li <liziyao@uniontech.com>
> 
> Older steppings of the Loongson-3C6000 series incorrectly report the
> supported link speeds on their PCIe bridges (device IDs 0x3c19, 0x3c29)
> as only 2.5 GT/s, despite the upstream bus supporting speeds from
> 2.5 GT/s up to 16 GT/s.
> 
> As a result, since commit 774c71c52aa4 ("PCI/bwctrl: Enable only if more
> than one speed is supported"), bwctrl will be disabled if there's only
> one 2.5 GT/s value in vector `supported_speeds`.
> 
> Also, the amdgpu driver reads the value by pcie_get_speed_cap() in
> amdgpu_device_partner_bandwidth(), for its dynamic adjustment of PCIe
> clocks and lanes in power management. We hope this patch can prevent
> similar problems in future driver changes (similar checks may be
> implemented in other GPU, storage controller, NIC, etc. drivers).

Why is this paragraph here?  Is there code in
amdgpu_device_partner_bandwidth() that wouldn't be needed after this
patch?

This patch updates pdev->supported_speeds, which is used by
pcie_get_speed_cap(), which is in turn used by
amdgpu_device_partner_bandwidth().

Is the point just that users of pcie_get_speed_cap() (currently just
amdgpu, radeon, and sysfs) will now see the correct maximum link speed
for Loongson-3C6000 bridges?

And the "checks" you refer to would be the tests in
amdgpu_device_get_pcie_info() that use the results of
pcie_get_speed_cap()?

> Manually override the `supported_speeds` field for affected PCIe bridges
> with those found on the upstream bus to correctly reflect the supported
> link speeds.
> 
> This patch was originally found from AOSC OS[1].
> 
> Link: https://github.com/AOSC-Tracking/linux/pull/2 #1
> Tested-by: Lain Fearyncess Yang <fsf@live.com>
> Tested-by: Ayden Meng <aydenmeng@yeah.net>
> Signed-off-by: Ayden Meng <aydenmeng@yeah.net>
> Signed-off-by: Mingcong Bai <jeffbai@aosc.io>
> Link: https://github.com/AOSC-Tracking/linux/commit/4392f441363abdf6fa0a0433d73175a17f493454
> [Ziyao Li: move from drivers/pci/quirks.c to drivers/pci/controller/pci-loongson.c]
> Signed-off-by: Ziyao Li <liziyao@uniontech.com>
> Tested-by: Mingcong Bai <jeffbai@aosc.io>
> Reviewed-by: Huacai Chen <chenhuacai@loongson.cn>
> [Xi Ruoyao: Fix falling through logic and add kernel log output;
>  add Fixes tag and rebase to 7.0-rc7]
> Cc: stable@vger.kernel.org
> Fixes: cd89edda4002 ("PCI: loongson: Add ACPI init support")
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> ---
> 
> Changes in v8:
> - Add the Fixes tag.
> - Link to v7: https://lore.kernel.org/all/20260121-loongson-pci1-v7-1-fc79c85a574d@uniontech.com/
> 
> Ziyao Li's original commentary message follows below:
> 
> The reason of not just copying pdev->bus->self->supported_speeds is
> that we're concerned that this approach assumes the upstream port
> reports the same capabilities as bridge, which may not always be the
> case in future silicon revisions.
> 
> Our current conservative approach ensures we only enable speeds that
> are physically supported by checking the actual max_bus_speed. For
> example, if there's a future Loongson-3C9999 where the virtual bridge
> reports Gen4 support but the physical bridge only supports Gen3.
> 
> In this scenario, directly copying the upstream port's supported_speeds
> would incorrectly report Gen4 support for the downstream bridge. The
> current patch ensures we only set speed bits up to what the hardware
> actually supports, based on the measured max_bus_speed. This seems
> safer for future silicon.
> 
> Changes in v7:
> - adjust commit message
> - Link to v6: https://lore.kernel.org/r/20260114-loongson-pci1-v6-1-ee8a18f5d242@uniontech.com
> 
> Changes in v6:
> - adjust commit message
> - Link to v5: https://lore.kernel.org/r/20260113-loongson-pci1-v5-1-264c9b4a90ab@uniontech.com
> 
> Changes in v5:
> - style adjust
> - Link to v4: https://lore.kernel.org/r/20260113-loongson-pci1-v4-1-1921d6479fe4@uniontech.com
> 
> Changes in v4:
> - rename subject
> - use 0x3c19/0x3c29 instead of 3c19/3c29
> - Link to v3: https://lore.kernel.org/r/20260109-loongson-pci1-v3-1-5ddc5ae3ba93@uniontech.com
> 
> Changes in v3:
> - Adjust commit message
> - Make the program flow more intuitive
> - Link to v2: https://lore.kernel.org/r/20260104-loongson-pci1-v2-1-d151e57b6ef8@uniontech.com
> 
> Changes in v2:
> - Link to v1: https://lore.kernel.org/r/20250822-loongson-pci1-v1-1-39aabbd11fbd@uniontech.com
> - Move from arch/loongarch/pci/pci.c to drivers/pci/controller/pci-loongson.c
> - Fix falling through logic and add kernel log output by Xi Ruoyao
> 
>  drivers/pci/controller/pci-loongson.c | 36 +++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/drivers/pci/controller/pci-loongson.c b/drivers/pci/controller/pci-loongson.c
> index bc630ab8a283..a4250d7af1bf 100644
> --- a/drivers/pci/controller/pci-loongson.c
> +++ b/drivers/pci/controller/pci-loongson.c
> @@ -176,6 +176,42 @@ static void loongson_pci_msi_quirk(struct pci_dev *dev)
>  }
>  DECLARE_PCI_FIXUP_FINAL(PCI_VENDOR_ID_LOONGSON, DEV_LS7A_PCIE_PORT5, loongson_pci_msi_quirk);
>  
> +/*
> + * Older steppings of the Loongson-3C6000 series incorrectly report the
> + * supported link speeds on their PCIe bridges (device IDs 0x3c19,
> + * 0x3c29) as only 2.5 GT/s, despite the upstream bus supporting speeds
> + * from 2.5 GT/s up to 16 GT/s.
> + */
> +static void loongson_pci_bridge_speed_quirk(struct pci_dev *pdev)
> +{
> +	u8 old_supported_speeds = pdev->supported_speeds;
> +
> +	switch (pdev->bus->max_bus_speed) {
> +	case PCIE_SPEED_16_0GT:
> +		pdev->supported_speeds |= PCI_EXP_LNKCAP2_SLS_16_0GB;
> +		fallthrough;
> +	case PCIE_SPEED_8_0GT:
> +		pdev->supported_speeds |= PCI_EXP_LNKCAP2_SLS_8_0GB;
> +		fallthrough;
> +	case PCIE_SPEED_5_0GT:
> +		pdev->supported_speeds |= PCI_EXP_LNKCAP2_SLS_5_0GB;
> +		fallthrough;
> +	case PCIE_SPEED_2_5GT:
> +		pdev->supported_speeds |= PCI_EXP_LNKCAP2_SLS_2_5GB;
> +		break;
> +	default:
> +		pci_warn(pdev, "unexpected max bus speed");
> +
> +		return;
> +	}
> +
> +	if (pdev->supported_speeds != old_supported_speeds)
> +		pci_info(pdev, "fixing up supported link speeds: 0x%x => 0x%x",
> +			 old_supported_speeds, pdev->supported_speeds);
> +}
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_LOONGSON, 0x3c19, loongson_pci_bridge_speed_quirk);
> +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_LOONGSON, 0x3c29, loongson_pci_bridge_speed_quirk);
> +
>  static struct loongson_pci *pci_bus_to_loongson_pci(struct pci_bus *bus)
>  {
>  	struct pci_config_window *cfg;
> -- 
> 2.53.0
>