Re: [PATCH v2] PCI/portdrv: Disable bwctrl service if port is fixed at 2.5 GT/s

[Niklas Schnelle <schnelle@linux.ibm.com>]

On Tue, 2025-01-14 at 09:50 +0100, Geert Uytterhoeven wrote:
> On Fri, Dec 13, 2024 at 9:56 PM Niklas Schnelle <niks@kernel.org> wrote:
> > Trying to enable bwctrl on an Intel JHL7540 (Titan Ridge) based
> > Thunderbolt port causes a boot hang on at least some systems though the
> > exact reason is not yet understood. As per the spec Thunderbolt PCIe
> > Downstream Ports have a fake Max Link Speed of 2.5 GT/s (USB4 v2 sec
> > 11.2.1):
> > 
> >    "Max Link Speed field in the Link Capabilities Register set to 0001b
> >     (data rate of 2.5 GT/s only).
> >     Note: These settings do not represent actual throughput.
> >     Throughput is implementation specific and based on the USB4 Fabric
> >     performance."
> > 
> > More generally if 2.5 GT/s is the only supported link speed there is no
> > point in throtteling as this is already the lowest possible PCIe speed
> > so don't advertise the capability stopping bwctrl from being probed on
> > these ports.
> > 
> > The PCIe r6.2 specification section 7.5.3.18 recommends to primarily
> > utilize the Supported Link Speeds Vector instead of the Max Link Speeds
> > field to prevent confusion if future specifications allow devices not
> > to support lower speeds. This concern does not apply however when
> > specifically targeting devices claiming support only for 2.5 GT/s.
> > 
> > Link: https://lore.kernel.org/linux-pci/Z1R4VNwCOlh9Sg9n@wunner.de/
> > Fixes: 665745f27487 ("PCI/bwctrl: Re-add BW notification portdrv as PCIe BW controller")
> > Tested-by: Niklas Schnelle <niks@kernel.org>
> > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> > Signed-off-by: Niklas Schnelle <niks@kernel.org>
> > ---
> > Note: This issue causes a boot hang on my personal workstation.
> > 
> > While there is an ongoing discussion about generalizing this to all
> > devices with a single supported speed. It turns out however that in my
> > case dev->supported_speeds incorrectly claims 2.5-8 GT/s requiring
> > a seperate second fix. So in the interest of simplicity and because I'll
> > be out from the 19th until January, I'd like to propose to do this simple
> > fix to the boot hang now and take the time to figure out a more general
> > approach afterwards.
> > ---
> > Changes in v2:
> > - Improve commit message to mention the specific controller and
> >   why using the Max Link Speeds field should be fine here.
> > - Add a comment (Lukas)
> > - Add R-b's (no change to logic).
> > - Link to v1: https://lore.kernel.org/r/20241207-fix_bwctrl_thunderbolt-v1-1-b711f572a705@kernel.org
> 
> This is now commit e50e27a613db6f18 ("PCI/portdrv: Disable bwctrl
> service if port is fixed at 2.5 GT/s") in pci/next, which conflicts
> with  commit 774c71c52aa48700 ("PCI/bwctrl: Enable only if more than
> one speed is supported") in v6.13-rc4.
> 
> Gr{oetje,eeting}s,
> 
>                         Geert

Oh, that's not right, good catch. This patch is superseded by
"PCI/bwctrl: Enable only if more than one speed is supported" and
should be dropped. Sorry if that wasn't clear from the the limited
context of this patch discussion. @Bjorn I think this needs to be taken
care of by you?

Thanks,
Niklas