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

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

[Bjorn Helgaas]

[+cc Andreas (author of 1df5172c5c25 ("PCI: Suspend/resume quirks for
Apple thunderbolt"), Mika, Yehezkel, Lukas, linux-usb]

On Thu, Sep 24, 2026 at 10:28:15AM -0300, Francisco Beltrán Millalén wrote:
> 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.

Thanks for all this detail.  I think it's too much for a commit log,
but it would be good to have it after the "---" where it's in the
email but not the git commit.  This is easily accessible via the
"Link: https://patch.msgid.link/" tag that we add when applying.

> 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.

Wrap these to fit in 80 columns like the rest of the file.  Ideally 75
or so; that allows minor changes and typo fixes without overflowing.

> + */
> +DECLARE_PCI_FIXUP_SUSPEND(PCI_VENDOR_ID_INTEL,
> +			  PCI_DEVICE_ID_INTEL_ALPINE_RIDGE_4C_BRIDGE,
> +			  quirk_apple_poweroff_thunderbolt);
>  #endif
>  
>  /*

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

[Francisco Beltrán Millalén]

Hi Bjorn,

Thanks for looking at this one as well, and for copying the Thunderbolt
folks.

On Thu, Oct 08, 2026 at 06:17:44PM -0500, Bjorn Helgaas wrote:
> Thanks for all this detail.  I think it's too much for a commit log,
> but it would be good to have it after the "---" where it's in the
> email but not the git commit.  This is easily accessible via the
> "Link: https://patch.msgid.link/" tag that we add when applying.

I'll move most of it below the "---" in v2 and keep the commit message
to the problem and the fix.

> Wrap these to fit in 80 columns like the rest of the file.  Ideally 75
> or so; that allows minor changes and typo fixes without overflowing.

I'll fix that in v2 too.

Before v2, though, I found something this week that changes the patch,
so I'd ask you not to apply this version.

The quirk works today partly by accident.  On resume, the firmware of
these Macs tries to write to the Thunderbolt controller through the
PCIe2CIO mailbox of its upstream bridge, and on Linux those accesses
currently go to the wrong device: ACPICA works out the PCI address of
the bridge's config region while the bridge's bus numbers are still
reset, and caches 00:00.0.  Each access then times out, which is where
most of the ~16 s noirq resume that Darrell reported comes from.  There
are two pull requests for this in ACPICA:

  https://github.com/open-acpica/acpica/pull/1235
  https://github.com/open-acpica/acpica/pull/1236

With that fixed, resume drops to about 4 s, but the firmware's writes
now succeed, and one of them sets bit 26 of dword 0x3c of the
controller's switch config space (VSC_CS_20 in the plug events
capability).  With that bit set, SXFP() really cuts power to the
controller, and on resume Linux does not bring it back.  With the
ACPICA change and this patch, and a USB disk attached, the controller
with the disk was lost in 3 of 3 suspend cycles, in two of them
together with the other controller, and resume took about 35 s.
Clearing the bit through the same mailbox before SXFP() runs fixes it:
15 of 15 cycles resumed in about 4 s with the disk still there.
Without the ACPICA change the firmware's write never lands, which is
why this version works as posted and in Darrell's tests.

I also checked whether the ACPICA change makes the quirk unnecessary.
It doesn't: in my test without the quirk, with the disk attached, the
link to that controller still did not come up after resume (LnkSta x0),
as described in the commit message.

So v2 would clear that bit before calling SXFP(), so that the quirk
keeps working once the ACPICA change lands.

Mika, I only know what this bit does from measuring it: with it set,
dropping the controller's power pin turns the NHI off; with it clear,
the NHI stays up.  Do you know what bit 26 of VSC_CS_20 is on Alpine
Ridge, and whether it is fine for the OS to clear it before cutting
power this way?  If it has a name, I'd like to use it in v2 instead of
a magic number.  And if you think this belongs in the thunderbolt
driver rather than in a PCI quirk, I'm happy to move it there.

Thanks again,
Francisco