On Sat, Feb 29, 2020 at 05:10:38PM +0800, Yicong Yang wrote: > On 2020/2/29 11:07, Bjorn Helgaas wrote: > > From: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > > > > Previously some PCI speed strings came from pci_speed_string(), some came > > from the PCIe-specific PCIE_SPEED2STR(), and some came from a PCIe-specific > > switch statement. These methods were inconsistent: > > > > pci_speed_string() PCIE_SPEED2STR() switch > > ------------------ ---------------- ------ > > 33 MHz PCI > > ... > > 2.5 GT/s PCIe 2.5 GT/s 2.5 GT/s > > 5.0 GT/s PCIe 5 GT/s 5 GT/s > > 8.0 GT/s PCIe 8 GT/s 8 GT/s > > 16.0 GT/s PCIe 16 GT/s 16 GT/s > > 32.0 GT/s PCIe 32 GT/s 32 GT/s > > > > Standardize on pci_speed_string() as the single source of these strings. > > > > Note that this adds ".0" and "PCIe" to some messages, including sysfs > > "max_link_speed" files, a brcmstb "link up" message, and the link status > > dmesg logging, e.g., > > > > nvme 0000:01:00.0: 16.000 Gb/s available PCIe bandwidth, limited by 5.0 GT/s PCIe x4 link at 0000:00:01.1 (capable of 31.504 Gb/s with 8.0 GT/s PCIe x4 link) > > > > I think it's better to standardize on a single version of the speed text. > > Previously we had strings like this: > > > > /sys/bus/pci/slots/0/cur_bus_speed: 8.0 GT/s PCIe > > /sys/bus/pci/slots/0/max_bus_speed: 8.0 GT/s PCIe > > /sys/devices/pci0000:00/0000:00:1c.0/current_link_speed: 8 GT/s > > /sys/devices/pci0000:00/0000:00:1c.0/max_link_speed: 8 GT/s > > > > This changes the latter two to match the slots files: > > > > /sys/devices/pci0000:00/0000:00:1c.0/current_link_speed: 8.0 GT/s PCIe > > /sys/devices/pci0000:00/0000:00:1c.0/max_link_speed: 8.0 GT/s PCIe > > > > Based-on-patch by: Yicong Yang <yangyicong@xxxxxxxxxxxxx> > > Signed-off-by: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > > --- > > drivers/pci/controller/pcie-brcmstb.c | 3 +-- > > drivers/pci/pci-sysfs.c | 27 +++++---------------------- > > drivers/pci/pci.c | 6 +++--- > > drivers/pci/pci.h | 9 --------- > > 4 files changed, 9 insertions(+), 36 deletions(-) > > > > diff --git a/drivers/pci/controller/pcie-brcmstb.c b/drivers/pci/controller/pcie-brcmstb.c > > index d20aabc26273..41e88f1667bf 100644 > > --- a/drivers/pci/controller/pcie-brcmstb.c > > +++ b/drivers/pci/controller/pcie-brcmstb.c > > @@ -823,8 +823,7 @@ static int brcm_pcie_setup(struct brcm_pcie *pcie) > > lnksta = readw(base + BRCM_PCIE_CAP_REGS + PCI_EXP_LNKSTA); > > cls = FIELD_GET(PCI_EXP_LNKSTA_CLS, lnksta); > > nlw = FIELD_GET(PCI_EXP_LNKSTA_NLW, lnksta); > > - dev_info(dev, "link up, %s x%u %s\n", > > - PCIE_SPEED2STR(cls + PCI_SPEED_133MHz_PCIX_533), > > + dev_info(dev, "link up, %s x%u %s\n", pci_speed_string(cls), > > nlw, ssc_good ? "(SSC)" : "(!SSC)"); > > Here comes the problem. cls is not a pci_bus_speed enumerate. The > PCIe link speed decodes from PCI_EXP_LNKSTA is from 0x000, we'll get > the *wrong* string if passing cls directly to pci_speed_string(). > pcie_link_speed[](drivers/pci/probe.c, line 662) array should be > used here to do the conversion. > > + dev_info(dev, "link up, %s x%u %s\n", pci_speed_string(pcie_link_speed[cls]), > nlw, ssc_good ? "(SSC)" : "(!SSC)"; Oh, right, thanks!
