diff mbox series

[pciutils] pcilmr: Fix margining for ports with Lane reversal

Message ID 20240522160819.30208-1-n.proshkin@yadro.com (mailing list archive)
State Superseded
Delegated to: Bjorn Helgaas
Headers show
Series [pciutils] pcilmr: Fix margining for ports with Lane reversal | expand

Commit Message

Nikita Proshkin May 22, 2024, 4:08 p.m. UTC 
Current implementation interacts only with first Negotiated Link Width
lanes even when Maximum Link Width for the port is bigger than that and
Lane reversal is used. Utility in such situation may try to margin lane
which is not used right now and erroneously fail with
'Error during caps reading' message. Fix that behaviour.

Signed-off-by: Nikita Proshkin <n.proshkin@yadro.com>
---
 lmr/lmr.h        |  5 +++--
 lmr/margin.c     |  8 ++++----
 lmr/margin_hw.c  | 20 +++++++++++---------
 lmr/margin_log.c |  2 +-
 4 files changed, 19 insertions(+), 16 deletions(-)

Comments

Martin Mareš May 27, 2024, 12:38 p.m. UTC | #1 
Hi!

> Current implementation interacts only with first Negotiated Link Width
> lanes even when Maximum Link Width for the port is bigger than that and
> Lane reversal is used. Utility in such situation may try to margin lane
> which is not used right now and erroneously fail with
> 'Error during caps reading' message. Fix that behaviour.

Sorry, this does not apply over your previous patch.

Could you re-diff it, please?

					Martin
diff mbox series

Patch

diff --git a/lmr/lmr.h b/lmr/lmr.h
index 7375c33..2e5da7c 100644
--- a/lmr/lmr.h
+++ b/lmr/lmr.h
@@ -27,7 +27,8 @@  enum margin_hw { MARGIN_HW_DEFAULT, MARGIN_ICE_LAKE_RC };
 struct margin_dev {
   struct pci_dev *dev;
   int lmr_cap_addr;
-  u8 width;
+  u8 neg_width;
+  u8 max_width;
   u8 retimers_n;
   u8 link_speed;
 
@@ -202,7 +203,7 @@  void margin_log(char *format, ...);
 /* b:d.f -> b:d.f */
 void margin_log_bdfs(struct pci_dev *down_port, struct pci_dev *up_port);
 
-/* Print Link header (bdfs, width, speed) */
+/* Print Link header (bdfs, neg_width, speed) */
 void margin_log_link(struct margin_link *link);
 
 void margin_log_params(struct margin_params *params);
diff --git a/lmr/margin.c b/lmr/margin.c
index a8c6571..a432f68 100644
--- a/lmr/margin.c
+++ b/lmr/margin.c
@@ -161,7 +161,7 @@  read_params_internal(struct margin_dev *dev, u8 recvn, bool lane_reversal,
                      struct margin_params *params)
 {
   margin_cmd resp;
-  u8 lane = lane_reversal ? dev->width - 1 : 0;
+  u8 lane = lane_reversal ? dev->max_width - 1 : 0;
   margin_set_cmd(dev, lane, NO_COMMAND);
   bool status = margin_report_cmd(dev, lane, REPORT_CAPS(recvn), &resp);
   if (status)
@@ -361,7 +361,7 @@  margin_test_receiver(struct margin_dev *dev, u8 recvn, struct margin_args *args,
   for (int i = 0; i < lanes_n; i++)
     {
       results->lanes[i].lane
-        = recv.lane_reversal ? dev->width - lanes_to_margin[i] - 1 : lanes_to_margin[i];
+        = recv.lane_reversal ? dev->max_width - lanes_to_margin[i] - 1 : lanes_to_margin[i];
     }
 
   if (args->run_margin)
@@ -524,7 +524,7 @@  margin_process_args(struct margin_dev *dev, struct margin_args *args)
 
   if (!args->lanes_n)
     {
-      args->lanes_n = dev->width;
+      args->lanes_n = dev->neg_width;
       for (int i = 0; i < args->lanes_n; i++)
         args->lanes[i] = i;
     }
@@ -532,7 +532,7 @@  margin_process_args(struct margin_dev *dev, struct margin_args *args)
     {
       for (int i = 0; i < args->lanes_n; i++)
         {
-          if (args->lanes[i] >= dev->width)
+          if (args->lanes[i] >= dev->neg_width)
             {
               return MARGIN_TEST_ARGS_LANES;
             }
diff --git a/lmr/margin_hw.c b/lmr/margin_hw.c
index fc427c8..9ef8f1a 100644
--- a/lmr/margin_hw.c
+++ b/lmr/margin_hw.c
@@ -70,15 +70,17 @@  static struct margin_dev
 fill_dev_wrapper(struct pci_dev *dev)
 {
   struct pci_cap *cap = pci_find_cap(dev, PCI_CAP_ID_EXP, PCI_CAP_NORMAL);
-  struct margin_dev res
-    = { .dev = dev,
-        .lmr_cap_addr = pci_find_cap(dev, PCI_EXT_CAP_ID_LMR, PCI_CAP_EXTENDED)->addr,
-        .width = GET_REG_MASK(pci_read_word(dev, cap->addr + PCI_EXP_LNKSTA), PCI_EXP_LNKSTA_WIDTH),
-        .retimers_n
-        = (!!(pci_read_word(dev, cap->addr + PCI_EXP_LNKSTA2) & PCI_EXP_LINKSTA2_RETIMER))
-          + (!!(pci_read_word(dev, cap->addr + PCI_EXP_LNKSTA2) & PCI_EXP_LINKSTA2_2RETIMERS)),
-        .link_speed = (pci_read_word(dev, cap->addr + PCI_EXP_LNKSTA) & PCI_EXP_LNKSTA_SPEED),
-        .hw = detect_unique_hw(dev) };
+  struct margin_dev res = {
+    .dev = dev,
+    .lmr_cap_addr = pci_find_cap(dev, PCI_EXT_CAP_ID_LMR, PCI_CAP_EXTENDED)->addr,
+    .neg_width = GET_REG_MASK(pci_read_word(dev, cap->addr + PCI_EXP_LNKSTA), PCI_EXP_LNKSTA_WIDTH),
+    .max_width = GET_REG_MASK(pci_read_long(dev, cap->addr + PCI_EXP_LNKCAP), PCI_EXP_LNKCAP_WIDTH),
+    .retimers_n
+    = (!!(pci_read_word(dev, cap->addr + PCI_EXP_LNKSTA2) & PCI_EXP_LINKSTA2_RETIMER))
+      + (!!(pci_read_word(dev, cap->addr + PCI_EXP_LNKSTA2) & PCI_EXP_LINKSTA2_2RETIMERS)),
+    .link_speed = (pci_read_word(dev, cap->addr + PCI_EXP_LNKSTA) & PCI_EXP_LNKSTA_SPEED),
+    .hw = detect_unique_hw(dev)
+  };
   return res;
 }
 
diff --git a/lmr/margin_log.c b/lmr/margin_log.c
index b3c4bd5..b03d2b8 100644
--- a/lmr/margin_log.c
+++ b/lmr/margin_log.c
@@ -42,7 +42,7 @@  margin_log_link(struct margin_link *link)
 {
   margin_log("Link ");
   margin_log_bdfs(link->down_port.dev, link->up_port.dev);
-  margin_log("\nNegotiated Link Width: %d\n", link->down_port.width);
+  margin_log("\nNegotiated Link Width: %d\n", link->down_port.neg_width);
   margin_log("Link Speed: %d.0 GT/s = Gen %d\n", (link->down_port.link_speed - 3) * 16,
              link->down_port.link_speed);
   margin_log("Available receivers: ");