| Message ID | 1579079063-5668-6-git-send-email-yangyicong@hisilicon.com |
|---|---|
| State | New |
| Headers | show |
| Series | Improve link speed presentation process | expand |
On Wed, Jan 15, 2020 at 05:04:22PM +0800, Yicong Yang wrote: > Add PCIE_LNKCAP2_SLS2SPEED macro for transforming raw link cap 2 > value to link speed. Use it in pcie_get_speed_cap() to reduce > redundancy. We'll not touch the functions when new link > speed comes. The patch seems OK to me, but I don't see where it reduces redundancy. There was one copy of "lnkcap2 & PCI_EXP_LNKCAP2_SLS_32_0GB" before, and there's one copy after. It's just moved from pci.c to pci.h. Or am I missing something? > Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> > --- > drivers/pci/pci.c | 17 ++++------------- > drivers/pci/pci.h | 9 +++++++++ > 2 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c > index dce32ce..2ef4030 100644 > --- a/drivers/pci/pci.c > +++ b/drivers/pci/pci.c > @@ -5780,19 +5780,10 @@ enum pci_bus_speed pcie_get_speed_cap(struct pci_dev *dev) > * where only 2.5 GT/s and 5.0 GT/s speeds were defined. > */ > pcie_capability_read_dword(dev, PCI_EXP_LNKCAP2, &lnkcap2); > - if (lnkcap2) { /* PCIe r3.0-compliant */ > - if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_32_0GB) > - return PCIE_SPEED_32_0GT; > - else if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_16_0GB) > - return PCIE_SPEED_16_0GT; > - else if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_8_0GB) > - return PCIE_SPEED_8_0GT; > - else if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_5_0GB) > - return PCIE_SPEED_5_0GT; > - else if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_2_5GB) > - return PCIE_SPEED_2_5GT; > - return PCI_SPEED_UNKNOWN; > - } > + > + /* PCIe r3.0-compliant */ > + if (lnkcap2) > + return PCIE_LNKCAP2_SLS2SPEED(lnkcap2); > > pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap); > if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_5_0GB) > diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h > index 5e1f810..3d988e9 100644 > --- a/drivers/pci/pci.h > +++ b/drivers/pci/pci.h > @@ -290,6 +290,15 @@ void pci_disable_bridge_window(struct pci_dev *dev); > struct pci_bus *pci_bus_get(struct pci_bus *bus); > void pci_bus_put(struct pci_bus *bus); > > +/* PCIe link information from Link Capabilities 2 */ > +#define PCIE_LNKCAP2_SLS2SPEED(lnkcap2) \ > + ((lnkcap2) & PCI_EXP_LNKCAP2_SLS_32_0GB ? PCIE_SPEED_32_0GT : \ > + (lnkcap2) & PCI_EXP_LNKCAP2_SLS_16_0GB ? PCIE_SPEED_16_0GT : \ > + (lnkcap2) & PCI_EXP_LNKCAP2_SLS_8_0GB ? PCIE_SPEED_8_0GT : \ > + (lnkcap2) & PCI_EXP_LNKCAP2_SLS_5_0GB ? PCIE_SPEED_5_0GT : \ > + (lnkcap2) & PCI_EXP_LNKCAP2_SLS_2_5GB ? PCIE_SPEED_2_5GT : \ > + PCI_SPEED_UNKNOWN) > + > /* PCIe link information */ > #define PCI_SPEED2STR(speed) \ > ((speed) == PCI_SPEED_UNKNOWN ? "Unknown speed" : \ > -- > 2.8.1 >
On 2020/2/6 2:54, Bjorn Helgaas wrote: > On Wed, Jan 15, 2020 at 05:04:22PM +0800, Yicong Yang wrote: >> Add PCIE_LNKCAP2_SLS2SPEED macro for transforming raw link cap 2 >> value to link speed. Use it in pcie_get_speed_cap() to reduce >> redundancy. We'll not touch the functions when new link >> speed comes. > The patch seems OK to me, but I don't see where it reduces redundancy. > There was one copy of "lnkcap2 & PCI_EXP_LNKCAP2_SLS_32_0GB" before, > and there's one copy after. It's just moved from pci.c to pci.h. > Or am I missing something? Seems I used improper description here. I'll correct it in next version. Thanks, Yang > >> Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> >> --- >> drivers/pci/pci.c | 17 ++++------------- >> drivers/pci/pci.h | 9 +++++++++ >> 2 files changed, 13 insertions(+), 13 deletions(-) >> >> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c >> index dce32ce..2ef4030 100644 >> --- a/drivers/pci/pci.c >> +++ b/drivers/pci/pci.c >> @@ -5780,19 +5780,10 @@ enum pci_bus_speed pcie_get_speed_cap(struct pci_dev *dev) >> * where only 2.5 GT/s and 5.0 GT/s speeds were defined. >> */ >> pcie_capability_read_dword(dev, PCI_EXP_LNKCAP2, &lnkcap2); >> - if (lnkcap2) { /* PCIe r3.0-compliant */ >> - if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_32_0GB) >> - return PCIE_SPEED_32_0GT; >> - else if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_16_0GB) >> - return PCIE_SPEED_16_0GT; >> - else if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_8_0GB) >> - return PCIE_SPEED_8_0GT; >> - else if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_5_0GB) >> - return PCIE_SPEED_5_0GT; >> - else if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_2_5GB) >> - return PCIE_SPEED_2_5GT; >> - return PCI_SPEED_UNKNOWN; >> - } >> + >> + /* PCIe r3.0-compliant */ >> + if (lnkcap2) >> + return PCIE_LNKCAP2_SLS2SPEED(lnkcap2); >> >> pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap); >> if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_5_0GB) >> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h >> index 5e1f810..3d988e9 100644 >> --- a/drivers/pci/pci.h >> +++ b/drivers/pci/pci.h >> @@ -290,6 +290,15 @@ void pci_disable_bridge_window(struct pci_dev *dev); >> struct pci_bus *pci_bus_get(struct pci_bus *bus); >> void pci_bus_put(struct pci_bus *bus); >> >> +/* PCIe link information from Link Capabilities 2 */ >> +#define PCIE_LNKCAP2_SLS2SPEED(lnkcap2) \ >> + ((lnkcap2) & PCI_EXP_LNKCAP2_SLS_32_0GB ? PCIE_SPEED_32_0GT : \ >> + (lnkcap2) & PCI_EXP_LNKCAP2_SLS_16_0GB ? PCIE_SPEED_16_0GT : \ >> + (lnkcap2) & PCI_EXP_LNKCAP2_SLS_8_0GB ? PCIE_SPEED_8_0GT : \ >> + (lnkcap2) & PCI_EXP_LNKCAP2_SLS_5_0GB ? PCIE_SPEED_5_0GT : \ >> + (lnkcap2) & PCI_EXP_LNKCAP2_SLS_2_5GB ? PCIE_SPEED_2_5GT : \ >> + PCI_SPEED_UNKNOWN) >> + >> /* PCIe link information */ >> #define PCI_SPEED2STR(speed) \ >> ((speed) == PCI_SPEED_UNKNOWN ? "Unknown speed" : \ >> -- >> 2.8.1 >> > . >
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index dce32ce..2ef4030 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -5780,19 +5780,10 @@ enum pci_bus_speed pcie_get_speed_cap(struct pci_dev *dev) * where only 2.5 GT/s and 5.0 GT/s speeds were defined. */ pcie_capability_read_dword(dev, PCI_EXP_LNKCAP2, &lnkcap2); - if (lnkcap2) { /* PCIe r3.0-compliant */ - if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_32_0GB) - return PCIE_SPEED_32_0GT; - else if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_16_0GB) - return PCIE_SPEED_16_0GT; - else if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_8_0GB) - return PCIE_SPEED_8_0GT; - else if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_5_0GB) - return PCIE_SPEED_5_0GT; - else if (lnkcap2 & PCI_EXP_LNKCAP2_SLS_2_5GB) - return PCIE_SPEED_2_5GT; - return PCI_SPEED_UNKNOWN; - } + + /* PCIe r3.0-compliant */ + if (lnkcap2) + return PCIE_LNKCAP2_SLS2SPEED(lnkcap2); pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap); if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_5_0GB) diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 5e1f810..3d988e9 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -290,6 +290,15 @@ void pci_disable_bridge_window(struct pci_dev *dev); struct pci_bus *pci_bus_get(struct pci_bus *bus); void pci_bus_put(struct pci_bus *bus); +/* PCIe link information from Link Capabilities 2 */ +#define PCIE_LNKCAP2_SLS2SPEED(lnkcap2) \ + ((lnkcap2) & PCI_EXP_LNKCAP2_SLS_32_0GB ? PCIE_SPEED_32_0GT : \ + (lnkcap2) & PCI_EXP_LNKCAP2_SLS_16_0GB ? PCIE_SPEED_16_0GT : \ + (lnkcap2) & PCI_EXP_LNKCAP2_SLS_8_0GB ? PCIE_SPEED_8_0GT : \ + (lnkcap2) & PCI_EXP_LNKCAP2_SLS_5_0GB ? PCIE_SPEED_5_0GT : \ + (lnkcap2) & PCI_EXP_LNKCAP2_SLS_2_5GB ? PCIE_SPEED_2_5GT : \ + PCI_SPEED_UNKNOWN) + /* PCIe link information */ #define PCI_SPEED2STR(speed) \ ((speed) == PCI_SPEED_UNKNOWN ? "Unknown speed" : \
Add PCIE_LNKCAP2_SLS2SPEED macro for transforming raw link cap 2 value to link speed. Use it in pcie_get_speed_cap() to reduce redundancy. We'll not touch the functions when new link speed comes. Signed-off-by: Yicong Yang <yangyicong@hisilicon.com> --- drivers/pci/pci.c | 17 ++++------------- drivers/pci/pci.h | 9 +++++++++ 2 files changed, 13 insertions(+), 13 deletions(-)