On Fri, 2024-12-13 at 18:41 +0100, Niklas Schnelle wrote: > 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. Never mind, I missed the >5 GT/s bit. Though I think that returning 0 speeds is probably the safest bet for a broken device, no?
