Even thhough commit b891b4dc1eed claimed that original bit definitions were wrong, that's not really the case. After verifying PCI Specification Revisions 3.0, 3.1 and 4.0, Link Capabilites 2 Register's bit definitions were always starting from Bit 0.
This has been causing issues reporting correct link speeds on sysfs.
Fixes: b891b4dc1eed ("PCI: Fix bit definitions of PCI_EXP_LNKCAP2 register") Cc: stable@vger.kernel.org # v3.8+ Signed-off-by: Felipe Balbi felipe.balbi@linux.intel.com ---
PCI Spec References:
- 4.0 https://members.pcisig.com/wg/PCI-SIG/document/10912?downloadRevision=active - 3.1 https://members.pcisig.com/wg/PCI-SIG/document/download/8257 - 3.0 https://members.pcisig.com/wg/PCI-SIG/document/download/8265
include/uapi/linux/pci_regs.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h index 796d12910791..6ad597b3d082 100644 --- a/include/uapi/linux/pci_regs.h +++ b/include/uapi/linux/pci_regs.h @@ -652,10 +652,10 @@ #define PCI_EXP_DEVSTA2 42 /* Device Status 2 */ #define PCI_CAP_EXP_RC_ENDPOINT_SIZEOF_V2 44 /* v2 endpoints without link end here */ #define PCI_EXP_LNKCAP2 44 /* Link Capabilities 2 */ -#define PCI_EXP_LNKCAP2_SLS_2_5GB 0x00000002 /* Supported Speed 2.5GT/s */ -#define PCI_EXP_LNKCAP2_SLS_5_0GB 0x00000004 /* Supported Speed 5GT/s */ -#define PCI_EXP_LNKCAP2_SLS_8_0GB 0x00000008 /* Supported Speed 8GT/s */ -#define PCI_EXP_LNKCAP2_SLS_16_0GB 0x00000010 /* Supported Speed 16GT/s */ +#define PCI_EXP_LNKCAP2_SLS_2_5GB 0x00000001 /* Supported Speed 2.5GT/s */ +#define PCI_EXP_LNKCAP2_SLS_5_0GB 0x00000002 /* Supported Speed 5GT/s */ +#define PCI_EXP_LNKCAP2_SLS_8_0GB 0x00000004 /* Supported Speed 8GT/s */ +#define PCI_EXP_LNKCAP2_SLS_16_0GB 0x00000008 /* Supported Speed 16GT/s */ #define PCI_EXP_LNKCAP2_CROSSLINK 0x00000100 /* Crosslink supported */ #define PCI_EXP_LNKCTL2 48 /* Link Control 2 */ #define PCI_EXP_LNKCTL2_TLS 0x000f
[+cc Jingoo]
On Fri, Aug 03, 2018 at 09:51:20AM +0300, Felipe Balbi wrote:
Even thhough commit b891b4dc1eed claimed that original bit definitions were wrong, that's not really the case. After verifying PCI Specification Revisions 3.0, 3.1 and 4.0, Link Capabilites 2 Register's bit definitions were always starting from Bit 0.
This has been causing issues reporting correct link speeds on sysfs.
Can you elaborate on this a bit? b891b4dc1eed still looks correct to me. I'm looking at PCIe r4.0, sec 7.5.3.18, where it shows:
bit 0 RsvdP bits 7:1 Supported Link Speeds Vector bit 8 Crosslink Supported ...
The text in table 7-32 does say:
Bit definitions within this field are:
Bit 0 2.5 GT/s Bit 1 5.0 GT/s ...
It says "Bit 0" there, but it's referring to bit 0 of the *field*, which is bits 7:1 of the LNKCAP2 register, so bit 0 of the field is bit 1 of the 32-bit LNKCAP2 register.
I guess you must be looking at this path, since it's the only generic path that uses PCI_EXP_LNKCAP2_SLS_2_5GB:
max_link_speed_show() speed = pcie_get_speed_cap(pdev) pcie_capability_read_dword(dev, PCI_EXP_LNKCAP2, &lnkcap2) if (lnkcap2) ... else if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_2_5GB) return PCIE_SPEED_2_5GT PCIE_SPEED2STR(speed) ... (speed) == PCIE_SPEED_2_5GT ? "2.5 GT/s"
We don't extract the Supported Speeds Vector by masking and shifting, so PCI_EXP_LNKCAP2_SLS_2_5GB and related definitions should be based on the entire 32-bit register, not just the individual field within that register.
Can you attach the output of "sudo lspci -vvxxx" for a relevant device and the contents of the sysfs file that is wrong?
Fixes: b891b4dc1eed ("PCI: Fix bit definitions of PCI_EXP_LNKCAP2 register") Cc: stable@vger.kernel.org # v3.8+ Signed-off-by: Felipe Balbi felipe.balbi@linux.intel.com
PCI Spec References:
- 4.0 https://members.pcisig.com/wg/PCI-SIG/document/10912?downloadRevision=active
- 3.1 https://members.pcisig.com/wg/PCI-SIG/document/download/8257
- 3.0 https://members.pcisig.com/wg/PCI-SIG/document/download/8265
include/uapi/linux/pci_regs.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h index 796d12910791..6ad597b3d082 100644 --- a/include/uapi/linux/pci_regs.h +++ b/include/uapi/linux/pci_regs.h @@ -652,10 +652,10 @@ #define PCI_EXP_DEVSTA2 42 /* Device Status 2 */ #define PCI_CAP_EXP_RC_ENDPOINT_SIZEOF_V2 44 /* v2 endpoints without link end here */ #define PCI_EXP_LNKCAP2 44 /* Link Capabilities 2 */ -#define PCI_EXP_LNKCAP2_SLS_2_5GB 0x00000002 /* Supported Speed 2.5GT/s */ -#define PCI_EXP_LNKCAP2_SLS_5_0GB 0x00000004 /* Supported Speed 5GT/s */ -#define PCI_EXP_LNKCAP2_SLS_8_0GB 0x00000008 /* Supported Speed 8GT/s */ -#define PCI_EXP_LNKCAP2_SLS_16_0GB 0x00000010 /* Supported Speed 16GT/s */ +#define PCI_EXP_LNKCAP2_SLS_2_5GB 0x00000001 /* Supported Speed 2.5GT/s */ +#define PCI_EXP_LNKCAP2_SLS_5_0GB 0x00000002 /* Supported Speed 5GT/s */ +#define PCI_EXP_LNKCAP2_SLS_8_0GB 0x00000004 /* Supported Speed 8GT/s */ +#define PCI_EXP_LNKCAP2_SLS_16_0GB 0x00000008 /* Supported Speed 16GT/s */ #define PCI_EXP_LNKCAP2_CROSSLINK 0x00000100 /* Crosslink supported */ #define PCI_EXP_LNKCTL2 48 /* Link Control 2 */
#define PCI_EXP_LNKCTL2_TLS 0x000f
2.16.1
Hi,
Bjorn Helgaas helgaas@kernel.org writes:
On Fri, Aug 03, 2018 at 09:51:20AM +0300, Felipe Balbi wrote:
Even thhough commit b891b4dc1eed claimed that original bit definitions were wrong, that's not really the case. After verifying PCI Specification Revisions 3.0, 3.1 and 4.0, Link Capabilites 2 Register's bit definitions were always starting from Bit 0.
This has been causing issues reporting correct link speeds on sysfs.
Can you elaborate on this a bit? b891b4dc1eed still looks correct to me. I'm looking at PCIe r4.0, sec 7.5.3.18, where it shows:
bit 0 RsvdP bits 7:1 Supported Link Speeds Vector
I had missed this detail, actually. It was a misinterpretation of the spec. Sorry for the noise.
On Tuesday, August 7, 2018 4:21 AM, Felipe Balbi wrote:
Hi,
Bjorn Helgaas helgaas@kernel.org writes:
On Fri, Aug 03, 2018 at 09:51:20AM +0300, Felipe Balbi wrote:
Even thhough commit b891b4dc1eed claimed that original bit definitions were wrong, that's not really the case. After verifying PCI Specification Revisions 3.0, 3.1 and 4.0, Link Capabilites 2 Register's bit definitions were always starting from Bit 0.
This has been causing issues reporting correct link speeds on sysfs.
Can you elaborate on this a bit? b891b4dc1eed still looks correct to me. I'm looking at PCIe r4.0, sec 7.5.3.18, where it shows:
bit 0 RsvdP bits 7:1 Supported Link Speeds Vector
I had missed this detail, actually. It was a misinterpretation of the spec. Sorry for the noise.
Hi Balbi,
I can understand your situation. The detail of PCIe spec is very confusing. Actually, I was one of those who misunderstood. However, after reviewing the PCIe spec carefully, I found that the bit definition was wrong. At that time, I already checked that my patch works properly with Exynos SoCs. Thank you.
Best regards, Jingoo Han
-- balbi
On Tue, Aug 7, 2018 at 3:24 AM Felipe Balbi felipe.balbi@linux.intel.com wrote:
Hi,
Bjorn Helgaas helgaas@kernel.org writes:
On Fri, Aug 03, 2018 at 09:51:20AM +0300, Felipe Balbi wrote:
Even thhough commit b891b4dc1eed claimed that original bit definitions were wrong, that's not really the case. After verifying PCI Specification Revisions 3.0, 3.1 and 4.0, Link Capabilites 2 Register's bit definitions were always starting from Bit 0.
This has been causing issues reporting correct link speeds on sysfs.
Can you elaborate on this a bit? b891b4dc1eed still looks correct to me. I'm looking at PCIe r4.0, sec 7.5.3.18, where it shows:
bit 0 RsvdP bits 7:1 Supported Link Speeds Vector
I had missed this detail, actually. It was a misinterpretation of the spec. Sorry for the noise.
No problem. You were seeing something incorrect in sysfs, so if we can help diagnose or fix whatever the real problem is, don't hesitate to ask!
Hi,
Bjorn Helgaas bhelgaas@google.com writes:
Bjorn Helgaas helgaas@kernel.org writes:
On Fri, Aug 03, 2018 at 09:51:20AM +0300, Felipe Balbi wrote:
Even thhough commit b891b4dc1eed claimed that original bit definitions were wrong, that's not really the case. After verifying PCI Specification Revisions 3.0, 3.1 and 4.0, Link Capabilites 2 Register's bit definitions were always starting from Bit 0.
This has been causing issues reporting correct link speeds on sysfs.
Can you elaborate on this a bit? b891b4dc1eed still looks correct to me. I'm looking at PCIe r4.0, sec 7.5.3.18, where it shows:
bit 0 RsvdP bits 7:1 Supported Link Speeds Vector
I had missed this detail, actually. It was a misinterpretation of the spec. Sorry for the noise.
No problem. You were seeing something incorrect in sysfs, so if we can help diagnose or fix whatever the real problem is, don't hesitate to ask!
Sure thing, but I think this is a problem elsewhere :)
linux-stable-mirror@lists.linaro.org
-
Bjorn Helgaas -
Bjorn Helgaas -
Felipe Balbi -
Jingoo Han