Messages in this thread | | | Subject | Re: [PATCH v5] PCI: Check for PCIe downtraining conditions | From | Tal Gilboa <> | Date | Sun, 5 Aug 2018 10:05:10 +0300 |
| |
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); >>>>
| |