Linux PCI subsystem development
 help / color / mirror / Atom feed
* [PATCH] PCI: Extend Apple Thunderbolt power quirk to Alpine Ridge
@ 2026-09-24 13:28 Francisco Beltrán Millalén
  2026-09-24 13:34 ` sashiko-bot
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Francisco Beltrán Millalén @ 2026-09-24 13:28 UTC (permalink / raw)
  To: bhelgaas, linux-pci; +Cc: linux-kernel

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
 
 /*

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] PCI: Extend Apple Thunderbolt power quirk to Alpine Ridge
  2026-09-24 13:28 [PATCH] PCI: Extend Apple Thunderbolt power quirk to Alpine Ridge Francisco Beltrán Millalén
@ 2026-09-24 13:34 ` sashiko-bot
  2026-10-08 18:55 ` Darrell Gum
  2026-10-08 23:17 ` Bjorn Helgaas
  2 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-09-24 13:34 UTC (permalink / raw)
  To: Francisco Beltrán Millalén; +Cc: linux-pci

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

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260924132815.24430-1-fbeltranmillalen@gmail.com?part=1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] PCI: Extend Apple Thunderbolt power quirk to Alpine Ridge
  2026-09-24 13:28 [PATCH] PCI: Extend Apple Thunderbolt power quirk to Alpine Ridge Francisco Beltrán Millalén
  2026-09-24 13:34 ` sashiko-bot
@ 2026-10-08 18:55 ` Darrell Gum
  2026-10-09  2:51   ` Francisco Beltrán Millalén
  2026-10-08 23:17 ` Bjorn Helgaas
  2 siblings, 1 reply; 6+ messages in thread
From: Darrell Gum @ 2026-10-08 18:55 UTC (permalink / raw)
  To: Francisco Beltrán Millalén
  Cc: Bjorn Helgaas, linux-pci, linux-kernel

On Thu, 24 Sep 2026 10:28:15 -0300, Francisco Beltrán Millalén wrote:
> On Macs with an Alpine Ridge Thunderbolt 3 controller, suspending to RAM
[...]

Tested on a second MacBookPro14,3 on 7.2.5 (Omarchy's linux-omarchy
7.2.5-3). It has two Alpine Ridge 4C controllers, with upstream
bridges 8086:1578 at 04:00.0 and 7a:00.0. Your PCI/PM v2 series (with
e18d1abc3bff) and the Apple native PME patch were applied as well.
The full report is in reply to the series:
https://lore.kernel.org/all/20260930141914.6678-1-fbeltranmillalen@gmail.com/

For this patch:

- The quirk fired on both upstream bridges ("quirk: cutting power to
  Thunderbolt controller...").
- pm_test=platform passes. The stock kernel hard-hung there 3 out of
  3 times, with or without brcmfmac loaded.
- Real S3 resumed on every attempt, about half a dozen cycles
  including lid closes on battery. noirq resume is about 16 s, with
  about 11 s of it in each upstream bridge.
- With a USB 3 stick attached on the 7a:00.0 side (xHCI 7d:00.0),
  the quirk logged for both bridges during a lid-close S3. The stick
  was still there after resume: no disconnect logged, still at
  5000 Mbps, filesystem readable. A replug about 2 min later
  re-enumerated it at SuperSpeed.
- On that cycle the empty side's xHCI logged "xhci_hcd 0000:07:00.0:
  xHC error in resume, USBSTS 0x401, Reinit" and recovered.

The patches were tested together, not bisected. Only one S3 cycle
had a device attached (a USB stick, no real Thunderbolt device). The
no-wake-on-plug trade-off is untested here.

Tested-by: Darrell Gum <d@rrell.co>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] PCI: Extend Apple Thunderbolt power quirk to Alpine Ridge
  2026-09-24 13:28 [PATCH] PCI: Extend Apple Thunderbolt power quirk to Alpine Ridge Francisco Beltrán Millalén
  2026-09-24 13:34 ` sashiko-bot
  2026-10-08 18:55 ` Darrell Gum
@ 2026-10-08 23:17 ` Bjorn Helgaas
  2026-10-09  2:50   ` Francisco Beltrán Millalén
  2 siblings, 1 reply; 6+ messages in thread
From: Bjorn Helgaas @ 2026-10-08 23:17 UTC (permalink / raw)
  To: Francisco Beltrán Millalén
  Cc: bhelgaas, linux-pci, linux-kernel, Andreas Noever,
	Mika Westerberg, Yehezkel Bernat, Lukas Wunner, linux-usb

[+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
>  
>  /*

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] PCI: Extend Apple Thunderbolt power quirk to Alpine Ridge
  2026-10-08 23:17 ` Bjorn Helgaas
@ 2026-10-09  2:50   ` Francisco Beltrán Millalén
  0 siblings, 0 replies; 6+ messages in thread
From: Francisco Beltrán Millalén @ 2026-10-09  2:50 UTC (permalink / raw)
  To: helgaas
  Cc: bhelgaas, linux-pci, linux-kernel, andreas.noever, westeri,
	YehezkelShB, lukas, linux-usb, d

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] PCI: Extend Apple Thunderbolt power quirk to Alpine Ridge
  2026-10-08 18:55 ` Darrell Gum
@ 2026-10-09  2:51   ` Francisco Beltrán Millalén
  0 siblings, 0 replies; 6+ messages in thread
From: Francisco Beltrán Millalén @ 2026-10-09  2:51 UTC (permalink / raw)
  To: d; +Cc: bhelgaas, linux-pci, linux-kernel

Hi Darrell,

Thank you for testing all three patches on your MacBookPro14,3 and for
such a careful report.  It's very good to see the same results on a
second machine, and the details you sent were useful.

On Thu, Oct 08, 2026 at 11:55:42AM -0700, Darrell Gum wrote:
> - Real S3 resumed on every attempt, about half a dozen cycles
>   including lid closes on battery. noirq resume is about 16 s, with
>   about 11 s of it in each upstream bridge.

That 16 s is the ACPICA problem you mentioned in the other thread, and I
see the same thing here.  One warning before you try that ACPICA change,
though: together with this version of the quirk it makes things worse
when something is plugged in: the controller with the device attached,
and sometimes the other one too, is lost on resume until the next
reboot.  Even with nothing plugged in, the ports came back in my tests
but the thunderbolt driver logged timeouts.  I've explained why in my
reply to Bjorn in this thread; v2 of the quirk will handle it.  Until
then, I'd suggest not combining the ACPICA change with this version of
the quirk.

> - On that cycle the empty side's xHCI logged "xhci_hcd 0000:07:00.0:
>   xHC error in resume, USBSTS 0x401, Reinit" and recovered.

I see the same message here on the side with nothing attached.  With
the quirk, that controller loses power during suspend, and the xHCI
driver notices on resume and reinitialises it, so as far as I can tell
it's expected.

I'll copy you on v2.

Thanks again,
Francisco

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-10-09  2:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-24 13:28 [PATCH] PCI: Extend Apple Thunderbolt power quirk to Alpine Ridge Francisco Beltrán Millalén
2026-09-24 13:34 ` sashiko-bot
2026-10-08 18:55 ` Darrell Gum
2026-10-09  2:51   ` Francisco Beltrán Millalén
2026-10-08 23:17 ` Bjorn Helgaas
2026-10-09  2:50   ` Francisco Beltrán Millalén

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox