PCI: Check for PCIe downtraining conditions
PATCH 1/1 PCI: Check for PCIe downtraining conditions
PCIe downtraining happens when both the device and PCIe port are capable of a larger bus width or higher speed than negotiated. Downtraining might be indicative of other problems in the system, and identifying this from userspace is neither intuitive, nor straigh forward.
Instead, check for such conditions on device probe, and print an appropriate message.
--- drivers/pci/probe.c | 78 +++++++++++++++++++++++++++++++++++++++++++ include/uapi/linux/pci_regs.h | 1 + 2 files changed, 79 insertions(+)
drivers/pci/probe.c +78 -0
@@ -2146,6 +2146,82 @@ static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn)return dev; } +static void pcie_max_link_cap (struct pci_dev *dev,enum pci_bus_speed *speed, +enum pcie_link_width *width) +{ +uint32_t lnkcap; + +pcie_capability_read_dword (dev,PCI_EXP_LNKCAP , &lnkcap); + + *speed = pcie_link_speed[lnkcap &PCI_EXP_LNKCAP_SLS ]; + *width = (lnkcap &PCI_EXP_LNKCAP_MLW ) >>PCI_EXP_LNKCAP_MLW_SHIFT ; +} + +static void pcie_cur_link_sta (struct pci_dev *dev,enum pci_bus_speed *speed, +enum pcie_link_width *width) +{ +uint16_t lnksta; + +pcie_capability_read_word (dev,PCI_EXP_LNKSTA , &lnksta); + *speed = pcie_link_speed[lnksta &PCI_EXP_LNKSTA_CLS ]; + *width = (lnksta &PCI_EXP_LNKSTA_NLW ) >>PCI_EXP_LNKSTA_NLW_SHIFT ; +} + +static const char *pcie_bus_speed_name (enum pci_bus_speed speed) +{ +switch (speed) { +case PCIE_SPEED_2_5GT : +return "2.5 GT/s" ; +case PCIE_SPEED_5_0GT : +return "5.0 GT/s" ; +case PCIE_SPEED_8_0GT : +return "8.0 GT/s" ; +default : +return "unknown" ; + } +} + +static void pcie_check_downtrain_errors (struct pci_dev *dev) +{ +enum pci_bus_speed dev_max_speed, dev_cur_speed; +enum pci_bus_speed max_link_speed, bus_max_speed; +enum pcie_link_width dev_cur_width, dev_max_width; +enum pcie_link_width bus_max_width, max_link_width; +struct pci_dev *uport =pci_upstream_bridge (dev); + +if (!pci_is_pcie (dev) || !uport) +return ; + +/* Look from the device up to avoid downstream ports with no devices. */ +if ((pci_pcie_type (dev) !=PCI_EXP_TYPE_ENDPOINT ) && + (pci_pcie_type (dev) !=PCI_EXP_TYPE_LEG_END ) && + (pci_pcie_type (dev) !=PCI_EXP_TYPE_UPSTREAM )) +return ; + +/* Multi-function PCIe share the same link/status. */ +if (PCI_FUNC (dev->devfn) !=0 ) +return ; + +pcie_cur_link_sta (dev, &dev_cur_speed, &dev_cur_width); +pcie_max_link_cap (dev, &dev_max_speed, &dev_max_width); +pcie_max_link_cap (uport, &bus_max_speed, &bus_max_width); + + max_link_speed =min (bus_max_speed, dev_max_speed); + max_link_width =min (bus_max_width, dev_max_width); + + +if (dev_cur_speed < max_link_speed) +pci_warn (dev,"PCIe downtrain: link speed is %s (%s capable)" , +pcie_bus_speed_name (dev_cur_speed), +pcie_bus_speed_name (max_link_speed)); + +if (dev_cur_width < max_link_width) { +/* Lanes might not be routed, so use info instead of warn. */ +pci_info (dev,"PCIe downtrain: Port and device capable of x%d, but link running at x%d" , + max_link_width, dev_cur_width); + } +} +static void pci_init_capabilities (struct pci_dev *dev) {/* Enhanced Allocation */
@@ -2181,6 +2257,8 @@ static void pci_init_capabilities(struct pci_dev *dev)/* Advanced Error Reporting */ pci_aer_init (dev); +pcie_check_downtrain_errors (dev); +if (pci_probe_reset_function (dev) ==0 ) dev->reset_fn =1 ; }
include/uapi/linux/pci_regs.h +1 -0
@@ -522,6 +522,7 @@#define PCI_EXP_LNKCAP_SLS_8_0GB 0x00000003/* LNKCAP2 SLS Vector bit 2 */ #define PCI_EXP_LNKCAP_SLS_16_0GB 0x00000004/* LNKCAP2 SLS Vector bit 3 */ #define PCI_EXP_LNKCAP_MLW 0x000003f0/* Maximum Link Width */ +#define PCI_EXP_LNKCAP_MLW_SHIFT 4/* start of MLW mask in link status */ #define PCI_EXP_LNKCAP_ASPMS 0x00000c00/* ASPM Support */ #define PCI_EXP_LNKCAP_L0SEL 0x00007000/* L0s Exit Latency */ #define PCI_EXP_LNKCAP_L1EL 0x00038000/* L1 Exit Latency */
Signature · 1 line
2.14.3
Quoted 24 lines · Alexandru Gagniuc
drivers/pci/probe.c +24
-0+static void pcie_max_link_cap (struct pci_dev *dev,enum pci_bus_speed *speed, +enum pcie_link_width *width) +{ +uint32_t lnkcap; + +pcie_capability_read_dword (dev,PCI_EXP_LNKCAP , &lnkcap); + + *speed = pcie_link_speed[lnkcap &PCI_EXP_LNKCAP_SLS ]; + *width = (lnkcap &PCI_EXP_LNKCAP_MLW ) >>PCI_EXP_LNKCAP_MLW_SHIFT ; +} + +static void pcie_cur_link_sta (struct pci_dev *dev,enum pci_bus_speed *speed, +enum pcie_link_width *width) +{ +uint16_t lnksta; + +pcie_capability_read_word (dev,PCI_EXP_LNKSTA , &lnksta); + *speed = pcie_link_speed[lnksta &PCI_EXP_LNKSTA_CLS ]; + *width = (lnksta &PCI_EXP_LNKSTA_NLW ) >>PCI_EXP_LNKSTA_NLW_SHIFT ; +} + +static const char *pcie_bus_speed_name (enum pci_bus_speed speed) +{ +switch (speed) {
drivers/pci/probe.c static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn) · line 2182
Quoted 10 lines
drivers/pci/probe.c +10
-0+case PCIE_SPEED_2_5GT : +return "2.5 GT/s" ; +case PCIE_SPEED_5_0GT : +return "5.0 GT/s" ; +case PCIE_SPEED_8_0GT : +return "8.0 GT/s" ; +default : +return "unknown" ; + } +}
I thought Bjorn added some functions to retrieve this now.
Signature · 3 lines
Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
On 5/31/2018 10:28 AM, Sinan Kaya wrote:=0A=
Quoted 2 lines
On 5/31/2018 11:05 AM, Alexandru Gagniuc wrote:=0A=
Quoted 1 line
+static void pcie_max_link_cap(struct
pci_dev*dev, enumpci_bus_speed*=
speed,=0A=
Quoted 8 lines
Review
Quoted 8 lines
(quoted diff) +8
-0+ enum pcie_link_width *width)=0A= +{=0A= + uint32_t lnkcap;=0A= +=0A= + pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap);=0A= +=0A= + *speed =3D pcie_link_speed[lnkcap & PCI_EXP_LNKCAP_SLS];=0A= + *width =3D (lnkcap & PCI_EXP_LNKCAP_MLW) >> PCI_EXP_LNKCAP_MLW_SHIFT;=
=0A=
Quoted 3 lines
Review
Quoted 3 lines
(quoted diff) +3
-0+}=0A= +=0A= +static void pcie_cur_link_sta(struct pci_dev *dev, enum pci_bus_speed *=
speed,=0A=
Quoted 7 lines
Review
Quoted 7 lines
(quoted diff) +7
-0+ enum pcie_link_width *width)=0A= +{=0A= + uint16_t lnksta;=0A= +=0A= + pcie_capability_read_word(dev, PCI_EXP_LNKSTA, &lnksta);=0A= + *speed =3D pcie_link_speed[lnksta & PCI_EXP_LNKSTA_CLS];=0A= + *width =3D (lnksta & PCI_EXP_LNKSTA_NLW) >> PCI_EXP_LNKSTA_NLW_SHIFT;=
=0A=
Quoted 9 lines
Quoted 9 lines
(quoted diff) +9
-0+}=0A= +=0A= +static const char *pcie_bus_speed_name(enum pci_bus_speed speed)=0A= +{=0A= + switch (speed) {=0A= + case PCIE_SPEED_2_5GT:=0A= + return "2.5 GT/s";=0A= + case PCIE_SPEED_5_0GT:=0A= + return "5.0 GT/s";=0A=
Quoted 8 lines
Review
Quoted 6 lines
(quoted diff) +6
-0+ case PCIE_SPEED_8_0GT:=0A= + return "8.0 GT/s";=0A= + default:=0A= + return "unknown";=0A= + }=0A= +}=0A==0A=
I thought Bjorn added some functions to retrieve this now.=0A=
=0A=
Hmm. I couldn't find them.=0A=
=0A=
Alex=0A=
=0A=
Quoted 39 lines · On 5/31/2018 11:29 AM, [email protected] wrote:
Quoted 29 lines · Sinan Kaya
Quoted 28 lines · Alexandru Gagniuc
drivers/pci/probe.c +28
-0+static void pcie_max_link_cap (struct pci_dev *dev,enum pci_bus_speed *speed, +enum pcie_link_width *width) +{ +uint32_t lnkcap; + +pcie_capability_read_dword (dev,PCI_EXP_LNKCAP , &lnkcap); + + *speed = pcie_link_speed[lnkcap &PCI_EXP_LNKCAP_SLS ]; + *width = (lnkcap &PCI_EXP_LNKCAP_MLW ) >>PCI_EXP_LNKCAP_MLW_SHIFT ; +} + +static void pcie_cur_link_sta (struct pci_dev *dev,enum pci_bus_speed *speed, +enum pcie_link_width *width) +{ +uint16_t lnksta; + +pcie_capability_read_word (dev,PCI_EXP_LNKSTA , &lnksta); + *speed = pcie_link_speed[lnksta &PCI_EXP_LNKSTA_CLS ]; + *width = (lnksta &PCI_EXP_LNKSTA_NLW ) >>PCI_EXP_LNKSTA_NLW_SHIFT ; +} + +static const char *pcie_bus_speed_name (enum pci_bus_speed speed) +{ +switch (speed) { +case PCIE_SPEED_2_5GT : +return "2.5 GT/s" ; +case PCIE_SPEED_5_0GT : +return "5.0 GT/s" ;Quoted 8 lines
drivers/pci/probe.c static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn) · line 2182
Quoted 6 lines
drivers/pci/probe.c +6
-0+case PCIE_SPEED_8_0GT : +return "8.0 GT/s" ; +default : +return "unknown" ; + } +}I thought Bjorn added some functions to retrieve this now.
Quoted 1 line
Hmm. I couldn't find them.
https://lkml.org/lkml/2018/3/30/553
are you working on linux-next?
Quoted 4 lines
Alex
Signature · 3 lines
Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Quoted 40 lines · Sinan Kaya
Quoted 39 lines · On 5/31/2018 11:29 AM, [email protected] wrote:
Quoted 37 lines · Sinan Kaya
Quoted 24 lines · Alexandru Gagniuc
drivers/pci/probe.c +24
-0+static void pcie_max_link_cap (struct pci_dev *dev,enum pci_bus_speed *speed, +enum pcie_link_width *width) +{ +uint32_t lnkcap; + +pcie_capability_read_dword (dev,PCI_EXP_LNKCAP , &lnkcap); + + *speed = pcie_link_speed[lnkcap &PCI_EXP_LNKCAP_SLS ]; + *width = (lnkcap &PCI_EXP_LNKCAP_MLW ) >>PCI_EXP_LNKCAP_MLW_SHIFT ; +} + +static void pcie_cur_link_sta (struct pci_dev *dev,enum pci_bus_speed *speed, +enum pcie_link_width *width) +{ +uint16_t lnksta; + +pcie_capability_read_word (dev,PCI_EXP_LNKSTA , &lnksta); + *speed = pcie_link_speed[lnksta &PCI_EXP_LNKSTA_CLS ]; + *width = (lnksta &PCI_EXP_LNKSTA_NLW ) >>PCI_EXP_LNKSTA_NLW_SHIFT ; +} + +static const char *pcie_bus_speed_name (enum pci_bus_speed speed) +{ +switch (speed) {drivers/pci/probe.c static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn) · line 2182
Quoted 10 lines
drivers/pci/probe.c +10
-0+case PCIE_SPEED_2_5GT : +return "2.5 GT/s" ; +case PCIE_SPEED_5_0GT : +return "5.0 GT/s" ; +case PCIE_SPEED_8_0GT : +return "8.0 GT/s" ; +default : +return "unknown" ; + } +}I thought Bjorn added some functions to retrieve this now.
Quoted 3 lines
Quoted 1 line
Hmm. I couldn't find them.
Oh, pcie_get_speed_cap()/pcie_get_width_cap() seems to handle the capability. Not seeing one for status and speed name.
Quoted 1 line
are you working on linux-next?
v4.17-rc7
Quoted 6 lines
Quoted 4 lines
Alex
Quoted 7 lines · Alex G.
Quoted 1 line
Oh, pcie_get_speed_cap()/pcie_get_width_cap() seems to handle the capability. Not seeing one for status and speed name.
Quoted 1 line
are you working on linux-next?
v4.17-rc7
I think everything you need is in the series. I don't know which linux tree this landed or if it landed. Probably, it is shipping for 4.18. Need some help from Bjorn where to locate these.
Fri, 30 Mar 2018 16:04:40 -0500
Bjorn Helgaas (6):
bnx2x: Report PCIe link properties with pcie_print_link_status()
bnxt_en: Report PCIe link properties with pcie_print_link_status()
cxgb4: Report PCIe link properties with pcie_print_link_status()
fm10k: Report PCIe link properties with pcie_print_link_status()
ixgbe: Report PCIe link properties with pcie_print_link_status()
PCI: Remove unused pcie_get_minimum_link()
Tal Gilboa (8):
PCI: Add pcie_get_speed_cap() to find max supported link speed
PCI: Add pcie_get_width_cap() to find max supported link width
PCI: Add pcie_bandwidth_capable() to compute max supported link bandwidth
PCI: Add pcie_bandwidth_available() to compute bandwidth available to device
PCI: Add pcie_print_link_status() to log link speed and whether it's limited
Signature · 3 lines
Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Quoted 15 lines · Sinan Kaya
Quoted 7 lines · Alex G.
Quoted 1 line
Oh, pcie_get_speed_cap()/pcie_get_width_cap() seems to handle the capability. Not seeing one for status and speed name.
Quoted 1 line
are you working on linux-next?
v4.17-rc7
I think everything you need is in the series. I don't know which linux tree this landed or if it landed. Probably, it is shipping for 4.18. Need some help from Bjorn where to locate these.
Fri, 30 Mar 2018 16:04:40 -0500
Quoted 7 lines
Bjorn Helgaas (6):
bnx2x: Report PCIe link properties withpcie_print_link_status()
bnxt_en: Report PCIe link properties withpcie_print_link_status()
cxgb4: Report PCIe link properties withpcie_print_link_status()
fm10k: Report PCIe link properties withpcie_print_link_status()
ixgbe: Report PCIe link properties withpcie_print_link_status()
PCI: Remove unusedpcie_get_minimum_link()
I remember seeing some of these in my tree.
Quoted 5 lines
Tal Gilboa (8):
PCI: Addpcie_get_speed_cap()to find max supported link speed
PCI: Addpcie_get_width_cap()to find max supported link width
PCI: Addpcie_bandwidth_capable()to compute max supported link bandwidth
PCI: Addpcie_bandwidth_available()to compute bandwidth available to device
I definitely have these.
Quoted 1 line
PCI: Add
pcie_print_link_status()to log link speed and whether it's limited
This one, I have, but it's not what I need. This looks at the available bandwidth from root port to endpoint, whereas I'm only interested in downtraining between endpoint and upstream port.
Alex
Quoted 4 lines · Alex G.
Quoted 1 line
PCI: Add
pcie_print_link_status()to log link speed and whether it's limitedThis one, I have, but it's not what I need. This looks at the available bandwidth from root port to endpoint, whereas I'm only interested in downtraining between endpoint and upstream port.
I see what you are saying.
With a little bit of effort, you can reuse the same code.
Here is an attempt.
You can probably extend pcie_bandwidth_available() to put an optional parent bridge device for your own use case and terminate the loop around here.
https://elixir.bootlin.com/linux/v4.17-rc7/source/drivers/pci/pci.c#L5182
Then, you can use the existing code to achieve what you are looking for via pcie_print_link_status() by adding an optional parent parameter.
bw_cap = pcie_bandwidth_capable(dev, &speed_cap, &width_cap); bw_avail = pcie_bandwidth_available(dev, &limiting_dev, &speed, &width, *parent*);
If parent parameter is NULL, code can walk all the way to root as it is doing today. If it is not, then will terminate the loop on the first iteration.
Signature · 3 lines
Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Quoted 15 lines · Sinan Kaya
Quoted 4 lines · Alex G.
Quoted 1 line
PCI: Add
pcie_print_link_status()to log link speed and whether it's limitedThis one, I have, but it's not what I need. This looks at the available bandwidth from root port to endpoint, whereas I'm only interested in downtraining between endpoint and upstream port.
I see what you are saying.
With a little bit of effort, you can reuse the same code.
Here is an attempt.
You can probably extend
pcie_bandwidth_available()to put an optional parent bridge device for your own use case and terminate the loop around here.
Quoted 7 lines
https://elixir.bootlin.com/linux/v4.17-rc7/source/drivers/pci/pci.c#L5182
Then, you can use the existing code to achieve what you are looking for via
pcie_print_link_status()by adding an optional parent parameter.bw_cap = pcie_bandwidth_capable(dev, &speed_cap, &width_cap); bw_avail = pcie_bandwidth_available(dev, &limiting_dev, &speed, &width, *parent*);
That's confusing. I'd expect _capable() and _available() to be symmetrical. They either both look at one link only, or both go down to the root port. Though it seems _capable() is link-local, and _available() is down to root port.
Quoted 4 lines
If parent parameter is NULL, code can walk all the way to root as it is doing today. If it is not, then will terminate the loop on the first iteration.
Quoted 20 lines · Alex G.
Quoted 17 lines · Sinan Kaya
Quoted 4 lines · Alex G.
Quoted 1 line
PCI: Add
pcie_print_link_status()to log link speed and whether it's limitedThis one, I have, but it's not what I need. This looks at the available bandwidth from root port to endpoint, whereas I'm only interested in downtraining between endpoint and upstream port.
I see what you are saying.
With a little bit of effort, you can reuse the same code.
Here is an attempt.
You can probably extend
pcie_bandwidth_available()to put an optional parent bridge device for your own use case and terminate the loop around here.https://elixir.bootlin.com/linux/v4.17-rc7/source/drivers/pci/pci.c#L5182
Quoted 7 lines
Quoted 5 lines
Then, you can use the existing code to achieve what you are looking for via
pcie_print_link_status()by adding an optional parent parameter.bw_cap = pcie_bandwidth_capable(dev, &speed_cap, &width_cap); bw_avail = pcie_bandwidth_available(dev, &limiting_dev, &speed, &width, *parent*);That's confusing.
"confusing" refers to the way the code currently works. It doesn't refer to your proposal.
Alex
I'd expect _capable() and _available() to be
Quoted 8 lines
symmetrical. They either both look at one link only, or both go down to the root port. Though it seems
_capable()is link-local, and_available()is down to root port.Quoted 4 lines
If parent parameter is NULL, code can walk all the way to root as it is doing today. If it is not, then will terminate the loop on the first iteration.
Quoted 7 lines · Alex G.
Quoted 2 lines
bw_cap = pcie_bandwidth_capable(dev, &speed_cap, &width_cap); bw_avail = pcie_bandwidth_available(dev, &limiting_dev, &speed, &width, *parent*);That's confusing. I'd expect
_capable()and_available()to be symmetrical. They either both look at one link only, or both go down to the root port. Though it seems_capable()is link-local, and_available()is down to root port.
As you know, link speed is a qualification of two devices speed capability. Both speed and width parameters get negotiated by two devices during TS1 and TS2 ordered set exchange.
You need to see what your link partner can support in available function() vs. what this device can do in bandwidth() function.
Signature · 3 lines
Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Quoted 9 lines · Sinan Kaya
Quoted 7 lines · Alex G.
Quoted 2 lines
bw_cap = pcie_bandwidth_capable(dev, &speed_cap, &width_cap); bw_avail = pcie_bandwidth_available(dev, &limiting_dev, &speed, &width, *parent*);That's confusing. I'd expect
_capable()and_available()to be symmetrical. They either both look at one link only, or both go down to the root port. Though it seems_capable()is link-local, and_available()is down to root port.
Quoted 6 lines
As you know, link speed is a qualification of two devices speed capability. Both speed and width parameters get negotiated by two devices during TS1 and TS2 ordered set exchange.
You need to see what your link partner can support in available
function()vs. what this device can do inbandwidth()function.
I see. I'm not sure I can use pcie_print_link_status() without some major refactoring. I need to look at capability of device and it downstream port. There's no point complaining that an x16 device is running at x4 when the port is only x4 capable.
Let me think some more on this.
Alex
Quoted 17 lines · Alex G.
Quoted 9 lines · Sinan Kaya
Quoted 7 lines · Alex G.
Quoted 2 lines
bw_cap = pcie_bandwidth_capable(dev, &speed_cap, &width_cap); bw_avail = pcie_bandwidth_available(dev, &limiting_dev, &speed, &width, *parent*);That's confusing. I'd expect
_capable()and_available()to be symmetrical. They either both look at one link only, or both go down to the root port. Though it seems_capable()is link-local, and_available()is down to root port.Quoted 6 lines
As you know, link speed is a qualification of two devices speed capability. Both speed and width parameters get negotiated by two devices during TS1 and TS2 ordered set exchange.
You need to see what your link partner can support in available
function()vs. what this device can do inbandwidth()function.
Quoted 6 lines
I see. I'm not sure I can use
pcie_print_link_status()without some major refactoring. I need to look at capability of device and it downstream port. There's no point complaining that an x16 device is running at x4 when the port is only x4 capable.Let me think some more on this.
I did some thinking, and I don't like using the same message for both point-to-point links and full-tree links. From a userspace point of view having an arbitrary terminator in the tree parsing is confusing. So we're better of not modifying the behavior here.
On the other hand, just printing the link status on probe might be good enough. If we're downtraining, but there's a slower link somewhere else, it won't matter much that we're downtrained. Just calling the unmodified pcie_print_link_status() will most of the time find the exact segment that's also downtrained. So let's do this in v2
Alex
Quoted 2 lines
Alex
drivers/pci/probe.c static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn) · line 2217
Quoted 5 lines · Alexandru Gagniuc
drivers/pci/probe.c +5
-0+if (dev_cur_speed < max_link_speed) +pci_warn (dev,"PCIe downtrain: link speed is %s (%s capable)" , +pcie_bus_speed_name (dev_cur_speed), +pcie_bus_speed_name (max_link_speed)); +
Also this isn't quite correct. Target link speed is what the device tries to train. A device can try to train to much lower speed than the maximum on purpose.
It makes sense to print this if the speed that platform wants via target link speed is different from what is actually established though.
Signature · 3 lines
Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Quoted 10 lines · Sinan Kaya
drivers/pci/probe.c static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn) · line 2217
Quoted 5 lines · Alexandru Gagniuc
drivers/pci/probe.c +5
-0+if (dev_cur_speed < max_link_speed) +pci_warn (dev,"PCIe downtrain: link speed is %s (%s capable)" , +pcie_bus_speed_name (dev_cur_speed), +pcie_bus_speed_name (max_link_speed)); +Also this isn't quite correct. Target link speed is what the device tries to train. A device can try to train to much lower speed than the maximum on purpose.
Quoted 2 lines
It makes sense to print this if the speed that platform wants via target link speed is different from what is actually established though.
After seeing Gen 3 devices that train above the speed in the target link speed field, I talked to one the spec writers today. There is some ambiguity with the target link speed field. In PCIe 4.0 they are clarifying that to state that this field is "permitted to have no effect".
Alex
Quoted 11 lines · Alex G.
Quoted 10 lines · Sinan Kaya
drivers/pci/probe.c static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn) · line 2217
Quoted 5 lines · Alexandru Gagniuc
drivers/pci/probe.c +5
-0+if (dev_cur_speed < max_link_speed) +pci_warn (dev,"PCIe downtrain: link speed is %s (%s capable)" , +pcie_bus_speed_name (dev_cur_speed), +pcie_bus_speed_name (max_link_speed)); +Also this isn't quite correct. Target link speed is what the device tries to train. A device can try to train to much lower speed than the maximum on purpose.
Quoted 7 lines
Quoted 2 lines
It makes sense to print this if the speed that platform wants via target link speed is different from what is actually established though.
After seeing Gen 3 devices that train above the speed in the target link speed field, I talked to one the spec writers today. There is some ambiguity with the target link speed field. In PCIe 4.0 they are clarifying that to state that this field is "permitted to have no effect".
Good to know.
Probably, some company screwed up implementing this register... All these flexible terminology in the spec is an indication of such a failure.
Quoted 4 lines
Alex
Signature · 3 lines
Sinan Kaya Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc. Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Quoted 9 lines · Alexandru Gagniuc
PCIe downtraining happens when both the device and PCIe port are capable of a larger bus width or higher speed than negotiated. Downtraining might be indicative of other problems in the system, and identifying this from userspace is neither intuitive, nor straigh forward.
Instead, check for such conditions on device probe, and print an appropriate message.
drivers/pci/probe.c static struct pci_dev *pci_scan_device(struct pci_bus *bus, int devfn) · line 2222
Quoted 5 lines
drivers/pci/probe.c +5
-0+if (dev_cur_width < max_link_width) { +/* Lanes might not be routed, so use info instead of warn. */ +pci_info (dev,"PCIe downtrain: Port and device capable of x%d, but link running at x%d" , + max_link_width, dev_cur_width); + }
Would "warn" be right loglevel?
Pavel
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html