Re: [PATCH v2] PCI: tegra264: Fix Link Capabilities register offset

[Bjorn Helgaas]

On Wed, Sep 23, 2026 at 02:57:20PM -0500, Bjorn Helgaas wrote:
> The PCI Express Capability begins at 0x48.  Link Capabilities is a 32-bit
> register at offset 0xc, and Link Status is a 16-bit register at offset
> 0x12:
> 
>   Link Capabilities is at 0x48 + 0xc  = 0x54
>   Link Status       is at 0x48 + 0x12 = 0x5a
> 
> Previously the driver read Link Capabilities with a 16-bit read from
> XTL_RC_PCIE_CFG_LINK_CAPS (0x56), which incorrectly read just the upper
> half of the register.  When a hotplug-capable port has no link during
> probe, tegra264_pcie_icc_set() consequently derives the maximum speed and
> width from unrelated bits and requests the wrong interconnect bandwidth.
> 
> Correct the Link Capabilities usage by adding a XTL_RC_PCIE_CAP definition
> for the base of the PCIe Capability, using the existing PCI_EXP_LNKCAP
> (0xc) and PCI_EXP_LNKSTA (0x12) offsets so they're easily searchable, and
> reading the entire 32 bits of Link Capabilities.
> 
> Fixes: 01c3c27a0ef6 ("PCI: tegra264: Add Tegra264 support")
> Based-on-patch-by: Linmao Li <lilinmao@xxxxxxxxxx>
> Link: https://lore.kernel.org/20260827093919.2825467-1-lilinmao@xxxxxxxxxx
> Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> ---
> 
> This fixes a real bug, so I'd like to get this in pci/next for v7.4.  I
> can't test this though, so need a review and test from somebody with the
> hardware.

I haven't seen any review or testing, but I replaced the patch from
https://lore.kernel.org/20260827093919.2825467-1-lilinmao@xxxxxxxxxx
on pci/controller/tegra264 with this one, headed for v7.4.

>  drivers/pci/controller/pcie-tegra264.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/pci/controller/pcie-tegra264.c b/drivers/pci/controller/pcie-tegra264.c
> index 653136db401e..c97576ab4a31 100644
> --- a/drivers/pci/controller/pcie-tegra264.c
> +++ b/drivers/pci/controller/pcie-tegra264.c
> @@ -49,8 +49,7 @@
>  #define XAL_RC_BAR_CNTL_STANDARD_64B_BAR_EN	BIT(2)
>  
>  /* XTL registers */
> -#define XTL_RC_PCIE_CFG_LINK_CAPS		0x56
> -#define XTL_RC_PCIE_CFG_LINK_STATUS		0x5a
> +#define XTL_RC_PCIE_CAP				0x48	/* PCIe Capability */
>  
>  #define XTL_RC_MGMT_PERST_CONTROL		0x218
>  #define XTL_RC_MGMT_PERST_CONTROL_PERST_O_N	BIT(0)
> @@ -118,11 +117,11 @@ static void tegra264_pcie_icc_set(struct tegra264_pcie *pcie)
>  	 * possible, so this is as good as it gets for now.
>  	 */
>  	if (pcie->link_up) {
> -		value = readw(pcie->ecam + XTL_RC_PCIE_CFG_LINK_STATUS);
> +		value = readw(pcie->ecam + XTL_RC_PCIE_CAP + PCI_EXP_LNKSTA);
>  		speed = FIELD_GET(PCI_EXP_LNKSTA_CLS, value);
>  		width = FIELD_GET(PCI_EXP_LNKSTA_NLW, value);
>  	} else {
> -		value = readw(pcie->ecam + XTL_RC_PCIE_CFG_LINK_CAPS);
> +		value = readl(pcie->ecam + XTL_RC_PCIE_CAP + PCI_EXP_LNKCAP);
>  		speed = FIELD_GET(PCI_EXP_LNKCAP_SLS, value);
>  		width = FIELD_GET(PCI_EXP_LNKCAP_MLW, value);
>  	}
> @@ -263,7 +262,7 @@ static bool tegra264_pcie_supports_hotplug(struct tegra264_pcie *pcie)
>  static bool tegra264_pcie_link_up(struct tegra264_pcie *pcie,
>  				  enum pci_bus_speed *speed)
>  {
> -	u16 value = readw(pcie->ecam + XTL_RC_PCIE_CFG_LINK_STATUS);
> +	u16 value = readw(pcie->ecam + XTL_RC_PCIE_CAP + PCI_EXP_LNKSTA);
>  
>  	if (value & PCI_EXP_LNKSTA_DLLLA) {
>  		if (speed)
> -- 
> 2.53.0
>