On Fri, 2024-12-13 at 12:12 +0200, Ilpo Järvinen wrote: > On Thu, 12 Dec 2024, Lukas Wunner wrote: > > > On Thu, Dec 12, 2024 at 04:33:23PM +0200, Ilpo Järvinen wrote: > > > On Thu, 12 Dec 2024, Lukas Wunner wrote: > > > > --- a/drivers/pci/pci.c > > > > +++ b/drivers/pci/pci.c > > > > @@ -6240,12 +6240,14 @@ u8 pcie_get_supported_speeds(struct pci_dev *dev) > > > > pcie_capability_read_dword(dev, PCI_EXP_LNKCAP2, &lnkcap2); > > > > speeds = lnkcap2 & PCI_EXP_LNKCAP2_SLS; > > > > > > > > + /* Ignore speeds higher than Max Link Speed */ > > > > + pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap); > > > > + speeds &= GENMASK(lnkcap & PCI_EXP_LNKCAP_SLS, 0); > > > > ---8<--- > As in more broader terms there are other kinds of broken devices this > code doesn't handle. If PCI_EXP_LNKCAP2_SLS is empty of bits but the > device has >5GT/s in PCI_EXP_LNKCAP_SLS, this function will return 0. On second look I don't think it will. If lnkcap2 & PCI_EXP_LNKCAP2_SLS is 0 it will proceed to the synthesize part and rely on PCI_EXP_LNKCAP_SLS alone. The potentially broken part I see is when lnkcap2 has bits set but lnkcap doesn't which is also when the GENMASK(…, 1) would become weird. Not sure what the right handling for that is though.
