[PATCH] PCI: Extend Apple Thunderbolt power quirk to Alpine Ridge

["Francisco Beltrán Millalén" <fbeltranmillalen@gmail.com>]

On Macs with an Alpine Ridge Thunderbolt 3 controller, suspending to RAM
with anything plugged into a USB-C port leaves the PCIe link between the
root port and the controller's upstream bridge permanently untrained on
resume.  The link does not merely fall back to a lower speed: it never
starts.  On a MacBookPro14,3 the root port reports

  LnkSta:  Speed 2.5GT/s, Width x0
  LnkSta2: EqualizationComplete- EqualizationPhase1- Phase2- Phase3-

with no correctable, non-fatal or fatal errors logged, and the link
training state machine stuck at its first state.  Both Thunderbolt
controllers, both integrated xHCIs and all four USB-C ports are lost
until reboot.  Measured 15 failures out of 15 with a device attached,
against 5 successes out of 5 with the ports empty; it follows the device,
not the port, and affects either controller.

Nothing recovers the link afterwards: neither the firmware's own eleven
retrain attempts, nor a secondary bus reset, nor bringing the controller
out of L2, nor cutting its power entirely once the link is already down.

macOS does not hit this because it powers the controller down on the way
into suspend.  Its AppleThunderboltNHIType3 driver calls SXFP() from
lateSleep, and the firmware of the affected machines defines exactly that
method: SXIO, SXLV, XRPE and XRIN, used by the drivers for older
controllers, are not present at all.

quirk_apple_poweroff_thunderbolt() already does this for Cactus Ridge, so
extend it rather than adding a second one.  Alpine Ridge needs only
SXFP(0); keying the short sequence off the device ID as well as off the
absence of SXIO/SXLV keeps Cactus Ridge behaviour unchanged.

It does need a different hook, though.  Cactus Ridge is handled as late
as possible, in the upstream bridge's suspend_noirq.  For Alpine Ridge
that does not prevent the failure: by the time the fixup runs there, the
bridges of both switches have already become inaccessible --
pci_save_state() on them has failed -- and the link still does not come
up on resume.  Hooking the
fixup one phase earlier, while every device below the switch is still in
D0, makes the branch resume intact.  That also matches where macOS does
it: SXFP() is called from lateSleep, ahead of the PCI teardown rather
than in the middle of it.

The earlier hook needs one extra guard.  pci_fixup_suspend also runs from
pci_pm_runtime_suspend(), and pm_suspend_via_firmware() does not
distinguish the two cases: PM_SUSPEND_FLAG_FW_SUSPEND is only cleared at
the beginning of the next system suspend, so it reads as set while the
system is running again.  Without pm_suspend_in_progress() the
controller would be powered down under a runtime-suspending bridge.
Cactus Ridge is unaffected, as pci_fixup_suspend_late has no runtime
counterpart, so the check is confined to the Alpine Ridge branch.

I cannot say with certainty what the platform does in between, only what
is observed: suspend_noirq is too late and suspend_late works, on 7 out
of 7 cycles.  For what it is worth, the firmware's own power protocol
does not appear to be involved: the ACPI methods that drop the rails are
all guarded on variables that only RTPC() ever writes, RTPC() is called
by macOS and never by Linux, and those variables still read as set after
several suspend/resume cycles on Linux.

With this, the branch comes back intact across suspend with a device
attached: both upstream bridges enumerate, the link comes up at 8 GT/s
x4 with equalisation complete, and USB devices are re-enumerated at
SuperSpeed by the integrated xHCI instead of falling back to the 2.0 path
wired straight to the PCH.  Twelve suspend/resume cycles in a single boot
without a failure -- six of them with a USB 3 SSD attached across the
suspend, one with it plugged in while suspended, and including lid-close
and idle-triggered suspends -- against fifteen failures out of fifteen
without the quirk.

Trade-off worth stating: cutting power to the controller also removes its
ability to wake the machine, so plugging something into a USB-C port no
longer wakes it from suspend.  Measured: with the machine suspended for
89 seconds, plugging the SSD into a free USB-C port did not wake it, and
the device was enumerated 0.3 s after the machine was woken by opening
the lid.  Opening the lid, the power button and the
internal keyboard (which is not behind these controllers) are unaffected,
and hotplug detection while the machine is awake is unaffected as well.
Gating the quirk on device_may_wakeup() would disable it outright on the
affected machines, so it is not conditional on that.

Cutting power a phase earlier has one visible consequence worth spelling
out, since the Cactus Ridge hook does not have it: devices below the
switch reach their own suspend_noirq after the controller is already
off, so pci_save_state() on them fails.  In practice this is limited to
the integrated xHCI of a branch with nothing plugged in: with a device
attached on one side only that side stays accessible, and with both sides
empty both xHCIs report it -- and xhci-hcd handles it as the
ordinary "root hub lost power or was reset" path and reinitialises the
controller on resume; the branch comes back complete.  It is also
strictly less than what happens without the quirk, where the whole
branch, bridges included, goes inaccessible instead of a single
endpoint.  It is also cleaner with

  https://lore.kernel.org/linux-pci/20260924124221.12374-1-fbeltranmillalen@gmail.com/

applied, which stops pci_save_state() from storing all-ones and marking
the state as saved when the device is already gone.  That series and this
quirk were developed together on the same machine: the series keeps the
resume from writing garbage back, and this patch keeps the controller
from disappearing in the first place.  Neither depends on the other to
build or to be correct.

Notes and limitations:

  * Only the 4C bridge (8086:1578) is added.  The 2C variant very likely
    needs the same treatment but I have no hardware to test it on, so I
    am not declaring it.
  * Tested only with USB devices behind the controller's integrated
    xHCI, not with a real Thunderbolt device.
  * Depends on the platform suspending via firmware (mem_sleep=deep);
    pm_suspend_via_firmware() already guards this.
  * pm_suspend_in_progress() is false during hibernation, so unlike the
    Cactus Ridge path this does not run on hibernate.  I have no way to
    test that path on this machine (the hibernate targets are masked),
    and leaving it out is the conservative choice.

Tested on 6.18.49 on a MacBookPro14,3.

Signed-off-by: Francisco Beltrán Millalén <fbeltranmillalen@gmail.com>
---
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -3879,7 +3879,7 @@
  */
 static void quirk_apple_poweroff_thunderbolt(struct pci_dev *dev)
 {
-	acpi_handle bridge, SXIO, SXFP, SXLV;
+	acpi_handle bridge, SXIO = NULL, SXFP = NULL, SXLV = NULL;
 
 	if (!x86_apple_machine)
 		return;
@@ -3906,8 +3906,34 @@
 	 * associated ACPI methods. This implicitly checks that we are at
 	 * the right bridge.
 	 */
+	if (ACPI_FAILURE(acpi_get_handle(bridge, "DSB0.NHI0.SXFP", &SXFP)))
+		return;
+
+	/*
+	 * Alpine Ridge uses a shorter sequence: macOS' AppleThunderboltNHIType3
+	 * calls only SXFP() from its lateSleep handler, and the firmware of the
+	 * affected machines does not even define SXIO or SXLV.  Keying this off
+	 * the device ID as well keeps the longer sequence for Cactus Ridge.
+	 */
+	if (dev->device == PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_4C_BRIDGE) {
+		/*
+		 * Unlike the suspend_late fixup used for Cactus Ridge, the
+		 * suspend fixup also runs on runtime suspend, and
+		 * pm_suspend_via_firmware() is not enough to tell the two
+		 * apart: that flag is only cleared at the beginning of the
+		 * next system suspend, so it stays set while the system is
+		 * running again.  Without this check the controller would be
+		 * powered down under a runtime-suspending bridge.
+		 */
+		if (!pm_suspend_in_progress())
+			return;
+
+		pci_info(dev, "quirk: cutting power to Thunderbolt controller...\n");
+		acpi_execute_simple_method(SXFP, NULL, 0);
+		return;
+	}
+
 	if (ACPI_FAILURE(acpi_get_handle(bridge, "DSB0.NHI0.SXIO", &SXIO))
-	    || ACPI_FAILURE(acpi_get_handle(bridge, "DSB0.NHI0.SXFP", &SXFP))
 	    || ACPI_FAILURE(acpi_get_handle(bridge, "DSB0.NHI0.SXLV", &SXLV)))
 		return;
 	pci_info(dev, "quirk: cutting power to Thunderbolt controller...\n");
@@ -3923,6 +3949,22 @@
 DECLARE_PCI_FIXUP_SUSPEND_LATE(PCI_VENDOR_ID_INTEL,
 			       PCI_DEVICE_ID_INTEL_CACTUS_RIDGE_4C,
 			       quirk_apple_poweroff_thunderbolt);
+
+/*
+ * Alpine Ridge needs the opposite of the above: SXFP() has to run while the
+ * switch is still up, not once it is being torn down.
+ *
+ * Hooked at suspend_noirq like Cactus Ridge it does not prevent the failure:
+ * by the time the upstream bridge is reached, the bridges of both switches have
+ * already become inaccessible -- pci_save_state() on them fails -- and the link
+ * still does not come up on resume.  Hooked one phase earlier, while every device below the
+ * switch is still in D0, the branch resumes intact.  That is also where macOS
+ * does it: AppleThunderboltNHIType3 calls SXFP() from lateSleep, ahead of the
+ * PCI teardown rather than in the middle of it.
+ */
+DECLARE_PCI_FIXUP_SUSPEND(PCI_VENDOR_ID_INTEL,
+			  PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_4C_BRIDGE,
+			  quirk_apple_poweroff_thunderbolt);
 #endif
 
 /*