On Mon, 26 Nov 2018, Bjorn Helgaas wrote:
> On Mon, Nov 19, 2018 at 06:47:04PM -0600, Bjorn Helgaas wrote:
> > On Tue, Oct 30, 2018 at 12:36:08PM -0400, Mikulas Patocka wrote:
> > > The macros PCI_EXP_LNKCAP_SLS_*GB are values, not bit masks. We must mask
> > > the register and compare it against them.
> > >
> > > This patch fixes errors "amdgpu: [powerplay] failed to send message 261
> > > ret is 0" errors when PCIe-v3 card is plugged into PCIe-v1 slot, because
> > > the slot is being incorrectly reported as PCIe-v3 capable.
> > >
> > > Signed-off-by: Mikulas Patocka <mpatocka@xxxxxxxxxx>
> > > Fixes: 6cf57be0f78e ("PCI: Add pcie_get_speed_cap() to find max supported link speed")
> > > Cc: stable@xxxxxxxxxxxxxxx # v4.17+
> > >
> > > ---
> > > drivers/pci/pci.c | 8 ++++----
> > > 1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > Index: linux-4.19/drivers/pci/pci.c
> > > ===================================================================
> > > --- linux-4.19.orig/drivers/pci/pci.c 2018-10-30 16:58:58.000000000 +0100
> > > +++ linux-4.19/drivers/pci/pci.c 2018-10-30 16:58:58.000000000 +0100
> > > @@ -5492,13 +5492,13 @@ enum pci_bus_speed pcie_get_speed_cap(st
> > >
> > > pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap);
> > > if (lnkcap) {
> > > - if (lnkcap & PCI_EXP_LNKCAP_SLS_16_0GB)
> > > + if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_16_0GB)
> > > return PCIE_SPEED_16_0GT;
> > > - else if (lnkcap & PCI_EXP_LNKCAP_SLS_8_0GB)
> > > + else if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_8_0GB)
> > > return PCIE_SPEED_8_0GT;
> > > - else if (lnkcap & PCI_EXP_LNKCAP_SLS_5_0GB)
> > > + else if ((lnkcap & PCI_EXP_LNKCAP_SLS) ==PCI_EXP_LNKCAP_SLS_5_0GB)
> > > return PCIE_SPEED_5_0GT;
> > > - else if (lnkcap & PCI_EXP_LNKCAP_SLS_2_5GB)
> > > + else if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_2_5GB)
> > > return PCIE_SPEED_2_5GT;
> > > }
>
> > We also need similar fixes in pci_set_bus_speed(), pcie_speeds()
> > (hfi1), cobalt_pcie_status_show(), hba_ioctl_callback(),
> > qla24xx_pci_info_str(), and maybe a couple other places.
>
> Does anybody want to volunteer to fix the places above as well? I
> found them by grepping for PCI_EXP_LNKCAP, and they're all broken in
> ways similar to pcie_get_speed_cap(). Possibly some of these places
> could use pcie_get_speed_cap() directly.
>
> Bjorn
>
They are not broken, they are masking the value with PCI_EXP_LNKCAP_SLS -
that is correct.
pci_set_bus_speed:
pcie_capability_read_dword(bridge, PCI_EXP_LNKCAP, &linkcap);
bus->max_bus_speed = pcie_link_speed[linkcap & PCI_EXP_LNKCAP_SLS];
pcie_speeds:
if ((linkcap & PCI_EXP_LNKCAP_SLS) != PCI_EXP_LNKCAP_SLS_8_0GB)
cobalt_pcie_status_show:
just prints the values without doing anything with them
hba_ioctl_callback:
gai->pci.link_speed_max = (u8)(caps & PCI_EXP_LNKCAP_SLS);
gai->pci.link_width_max = (u8)((caps & PCI_EXP_LNKCAP_MLW) >> 4);
qla24xx_pci_info_str:
lspeed = lstat & PCI_EXP_LNKCAP_SLS;
lwidth = (lstat & PCI_EXP_LNKCAP_MLW) >> 4;
Mikulas
