Re: [PATCH v5] PCI: Check for PCIe downtraining conditions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 7/31/2018 6:10 PM, Alex G. wrote:
On 07/31/2018 01:40 AM, Tal Gilboa wrote:
[snip]
@@ -2240,6 +2258,9 @@ static void pci_init_capabilities(struct pci_dev *dev)
      /* Advanced Error Reporting */
      pci_aer_init(dev);
+    /* Check link and detect downtrain errors */
+    pcie_check_upstream_link(dev);

This is called for every PCIe device right? Won't there be a duplicated print in case a device loads with lower PCIe bandwidth than needed?

Alex, can you comment on this please?

Of course I can.

There's one print at probe() time, which happens if bandwidth < max. I would think that's fine. There is a way to duplicate it, and that is if the driver also calls print_link_status(). A few driver maintainers who call it have indicated they'd be fine with removing it from the driver, and leaving it in the core PCI.

We would be fine with that as well. Please include the removal in your patches.


Should the device come back at low speed, go away, then come back at full speed we'd still only see one print from the first probe. Again, if driver doesn't also call the function. There's the corner case where the device come up at < max, goes away, then comes back faster, but still < max. There will be two prints, but they won't be true duplicates -- each one will indicate different BW.

This is fine.


Alex

+
      if (pci_probe_reset_function(dev) == 0)
          dev->reset_fn = 1;
  }
diff --git a/include/linux/pci.h b/include/linux/pci.h
index abd5d5e17aee..15bfab8f7a1b 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1088,6 +1088,7 @@ int pcie_set_mps(struct pci_dev *dev, int mps);
  u32 pcie_bandwidth_available(struct pci_dev *dev, struct pci_dev **limiting_dev,
                   enum pci_bus_speed *speed,
                   enum pcie_link_width *width);
+void __pcie_print_link_status(struct pci_dev *dev, bool verbose);
  void pcie_print_link_status(struct pci_dev *dev);
  int pcie_flr(struct pci_dev *dev);
  int __pci_reset_function_locked(struct pci_dev *dev);




[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux