On Thu, Dec 12, 2024 at 10:11:03AM -0600, Bjorn Helgaas wrote:
> On Thu, Dec 12, 2024 at 09:56:16AM +0100, Lukas Wunner wrote:
> > The Supported Link Speeds Vector in the Link Capabilities 2 Register
> > indicates the *supported* link speeds. The Max Link Speed field in
> > the Link Capabilities Register indicates the *maximum* of those speeds.
> >
> > Niklas reports that the Intel JHL7540 "Titan Ridge 2018" Thunderbolt
> > controller supports 2.5-8 GT/s speeds, but indicates 2.5 GT/s as maximum.
> > Ilpo recalls seeing this inconsistency on more devices.
> >
> > pcie_get_supported_speeds() neglects to honor the Max Link Speed field
> > and will thus incorrectly deem higher speeds as supported. Fix it.
> >
> > Fixes: d2bd39c0456b ("PCI: Store all PCIe Supported Link Speeds")
> > Reported-by: Niklas Schnelle <niks@xxxxxxxxxx>
> > Closes: https://lore.kernel.org/r/70829798889c6d779ca0f6cd3260a765780d1369.camel@xxxxxxxxxx/
> > Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
> > Cc: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
>
> Looks like you want this in v6.13? Can we make commit log more
> explicit as to why we need it there? Is this change enough to resolve
> the boot hang Niklas reported?
This is more like a "we're not conforming to the spec" kind of fix,
but also happens to be a prerequisite to fixing Niklas' boot hang.
Another user-visible issue addressed here is that we're exposing an
incorrect value in the sysfs "max_link_speed" attribute on devices
such as the JHL7540 Thunderbolt controller mentioned in the commit
message.
Thanks,
Lukas
> > ---
> > drivers/pci/pci.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 35dc9f2..b730560 100644
> > --- 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);
> > +
> > /* PCIe r3.0-compliant */
> > if (speeds)
> > return speeds;
> >
> > - pcie_capability_read_dword(dev, PCI_EXP_LNKCAP, &lnkcap);
> > -
> > /* Synthesize from the Max Link Speed field */
> > if ((lnkcap & PCI_EXP_LNKCAP_SLS) == PCI_EXP_LNKCAP_SLS_5_0GB)
> > speeds = PCI_EXP_LNKCAP2_SLS_5_0GB | PCI_EXP_LNKCAP2_SLS_2_5GB;
> > --
> > 2.43.0
