Repository navigation
Conversation
On the J713 the SuperSpeed port refuses system sleep for as long as any USB device is attached: atcphy_suspend() returns -EBUSY while the PHY owns its common block, so a laptop with a USB keyboard on that port never suspends: PM: dpm_run_callback(): atcphy_suspend returns -16 PM: Some devices failed to suspend, or early wake event detected Suspend-to-idle does not take the link down. While its core runs, DWC3 is a syscore device and keeps the shared power domain on across system sleep, and xHCI keeps running so that a USB device can wake the system. The PHY keeps its state with the domain, and resume restores it only if the domain went off. Allow suspend-to-idle with an active common epoch while a host runs on it. Keep refusing one without a host, as in a USB3 device role, where nothing holds the domain; an incomplete one; and any deeper sleep state, which would lose the link. Warn on resume if the domain went off under an active host link after all. A KUnit case covers these outcomes. Signed-off-by: Erik Bourget <erik@ebourget.net>
On the J713 a SuperSpeed link does not come back from U3 once the core has suspended the USB3 PHY with it (GUSB3PIPECTL.SUSPHY): resuming the port drops the link, and the whole tree behind it disconnects. This happens when a USB3 hub autosuspends, and on every resume from suspend-to-idle, where the USB core suspends all ports. A mounted disk behind a USB3 hub is removed under its filesystem. Clear SUSPHY for the USB3 PHY once the T8132 PIPE switch to USB3 is done, by setting dis_u3_susphy_quirk, which dwc3_enable_susphy() then honours when the xHCI host starts. The switch itself still runs with SUSPHY set, as it requires. Restore the device tree's setting before the host is stopped, which sets SUSPHY again: the PHY hands the PIPE back in P3, and fails to lock the PIPE handler otherwise. The core stop does the same for the error paths. USB2 PHY suspend is unchanged. The port routes the USB3 PIPE for any USB-C device in USB mode, so the USB3 PHY stays out of P3 while any USB device is attached, also a USB 2.0 one. The idle USB-C ports are powered down, so it costs nothing with nothing attached. The host test stubs of struct dwc3 and struct dwc3_apple gain dis_u3_susphy_quirk and dt_dis_u3_susphy, and the teardown test checks that the device tree's setting is back before the host exits. Signed-off-by: Erik Bourget <erik@ebourget.net>
The USB 2.0 hub of the VIA Labs VL822 (2109:2822), as in Anker USB-C hubs, does not pass on a remote wakeup from a device below it while it is suspended itself, like the VL122. A USB keyboard behind it cannot wake the system from suspend-to-idle. Give it USB_QUIRK_HUB_NO_SUSPENDED_WAKEUP, so that it is kept awake in suspend-to-idle while a device below it may wake the system. Its autosuspend is unchanged. A KUnit case checks the entry. Signed-off-by: Erik Bourget <erik@ebourget.net>
…a plug The resume revalidates the port and reports a cable removed in system sleep, but reads the power status only while a plug is present. A power source removed in sleep then leaves the cached power status, and with it the port's USB power supply, online until the next power event, and upower reports AC power while on battery. Read the power status unconditionally. Without a plug it reads as no connection. This matters once the events of a sleeping controller are acknowledged in sleep, before resume could handle them, which the following changes do. Signed-off-by: Erik Bourget <erik@ebourget.net>
As on ti,tps6598x, the controller may wake the system from sleep for a change on its port. Signed-off-by: Erik Bourget <erik@ebourget.net>
…link A USB device plugged into a J713 in suspend-to-idle cannot wake it, and cannot wake it later either: an idle USB-C port is powered down, so the device is never enumerated. Only the Type-C controller notices it: its event interrupt fires for the attach even while it sleeps. As a plain wakeup source that interrupt would also wake the system for a charger, on every plug, unplug and power negotiation. Filter the events instead. With wakeup-source set, the event interrupt stays live in system sleep (IRQF_NO_SUSPEND) and sn201202x_sleep_event() handles it there: it wakes the controller, acknowledges its events, reads its data status and status, and wakes the system with pm_wakeup_dev_event() only for a link that was not there before: - a USB2 or USB3 link with the port as the host: a port attaches as a USB device, as to a charger, before any data role swap; - from a power source, only one capable of USB communication, as its first source capability PDO says: a charger may still report a USB link as host for a moment after the swap; - DisplayPort, Thunderbolt or USB4. A charger plugged in or unplugged leaves the system asleep. A read that fails wakes it. The controller goes back to sleep either way, and the resume revalidates the whole port as before. A controller reset seen in sleep is set up again on resume, as cd321x_check_setup() latches it. The interrupt is not armed for wakeup, so IRQF_NO_SUSPEND is used as Documentation/power/suspend-and-interrupts.rst describes for a handler that decides itself whether to wake the system, with pm_system_wakeup() underneath. The watch is set up for suspend-to-idle only. In suspend-to-idle the tick is frozen, so a sleeping wait does not end until something else wakes the system: the controller is polled with busy waits there, within a fixed budget, and WAKEUP is repeated in case it reached a controller still going to sleep. A resume whose WAKEUP is not confirmed now falls back to polling as well. Its other interrupts are suspended, so this needs a controller without select notification, as on T8132; others keep their behaviour. A transfer the SPMI controller never answers still waits on its timer, so it ends only once the system is awake again. The port's power/wakeup setting turns the wakeup off. KUnit cases cover the link decision with the data status and status as read on a J713 for a charger as it attaches, swaps and settles, and for a USB keyboard. The tipd_init() host test stubs gain sleep_event and IRQF_NO_SUSPEND, and the SN201202x transport test covers the atomic selection too. Signed-off-by: Erik Bourget <erik@ebourget.net>
…ttach Mark both USB-C port controllers as wakeup sources. A USB device, dock or display plugged in while the J713 is in suspend-to-idle wakes it; a charger does not. Signed-off-by: Erik Bourget <erik@ebourget.net>
Author
|
Superseded by #832: the same seven commits, rebased onto integration/aurora. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On the J713 (MacBook Air 13" M4), USB devices did not reliably wake the machine from suspend-to-idle:
atcphy_suspend returns -16).This builds on Ryan's USB wake work for J700 (#340). Running xHCI in suspend-to-idle, the AIC wake flag and the hub wakeup quirk are what make USB wake possible at all. This series covers the parts that are specific to T8132 and the J713.
Depends on #449 and #450 for J713 suspend-to-idle itself. The series applies to
integration/auroraon its own and changes nothing on a J713 that doesn't suspend.Root causes
atcphy_suspend()returns-EBUSYwhile the PHY owns its common block. In suspend-to-idle that block keeps its state: DWC3 is a syscore device while its core runs, so theATCn_USBdomain stays on, and xHCI keeps running.GUSB3PIPECTL.SUSPHY, PIPE P3). This shows up for a runtime-autosuspended USB3 hub as well as in system sleep. Before (1) it was hidden: nothing reached U3, because the port didn't sleep anduasdisks don't autosuspend.Changes (one commit each)
phy: apple: atc: Let an active T8132 USB3 link through suspend-to-idle: allowPM_SUSPEND_TO_IDLEwith an active common epoch. It still refuses an incomplete epoch (mode_dirty) and any deeper sleep state, and warns on resume if the domain went off under an active link. KUnit case.usb: dwc3: apple: Keep the T8132 USB3 PHY out of P3 in host mode: setdis_u3_susphy_quirkonce the T8132 host PIPE switch to USB3 is done, so laterdwc3_enable_susphy()calls (xHCI start, system suspend) leave USB3 SUSPHY clear. The switch itself still runs with SUSPHY set. It is cleared again before the core stops, because the PHY hands the PIPE back in P3 and otherwise fails to lock the PIPE handler. USB2 PHY suspend is unchanged.usb: core: quirks: Keep the VIA VL822 USB2 hub awake for wakeups:USB_QUIRK_HUB_NO_SUSPENDED_WAKEUPfor 2109:2822, using the mechanism from J700: USB and Bluetooth keyboard wake from s2idle, including clamshell (supersedes #304) #340. KUnit case next to the VL122 one.usb: typec: tipd: cd321x: Refresh the power status on resume without a plug: the resume revalidation read the power status only with a plug present, so a charger removed in sleep left the port's USB power supply online, and upower reported AC power on battery. It now reads it unconditionally.dt-bindings: usb: apple,sn201202x: Allow wakeup-source, asti,tps6598xdoes.usb: typec: tipd: sn201202x: Wake from suspend-to-idle for a new USB link: withwakeup-sourceset, the controller's event interrupt stays live in system sleep (IRQF_NO_SUSPEND). Its handler wakes the controller, acknowledges the events, and wakes the system withpm_wakeup_dev_event()only for a link that wasn't there before:A failed read wakes the system. The handler busy-waits, because the tick is frozen in suspend-to-idle and a sleeping wait doesn't end until something else wakes the system. It is used only on controllers without select notification, as on T8132; everything else keeps its current behaviour. The per-port
power/wakeupswitch turns it off. KUnit cases use data and status values read on a J713.arm64: dts: apple: t8132-j713: Wake from suspend-to-idle on a USB-C attach:wakeup-sourceon both port controllers.Checked and left unchanged
IRQCHIP_SKIP_SET_WAKEon the SPMI irqchip. The filter doesn't arm the interrupt as a wake source, so it isn't needed.Hardware results (J713)
The display and GPU don't come up on
integration/auroraalone on a J713 yet, so the device tests ran on our J713 tree (which has #449 and #450) with these seven commits on top, unchanged:Before that, development testing on the same tree went through more combinations:
-EBUSY), wakesFailed to lock pipehandlerStatic checks: checkpatch
--strictis clean on all seven commits. Each commit builds its objects with W=1; the one warning, a-Wvoid-pointer-to-enum-castinspmi.c's probe, predates this series.dt_binding_checkis clean, and so isdtbs_checkfor all six T8132 boards.Power: idle ports are powered down, so keeping the USB3 PHY out of P3 costs nothing with nothing attached. It does apply to any device on a USB-C port in USB mode, including a USB 2.0 one, because the Type-C controller reports a USB3 connection for those too. Measured awake with a USB 2.0 keyboard on the port, alternating P3 allowed and not allowed four times, it costs about 25 mW (20 to 35 mW, close to the noise). Keeping P3 for USB 2.0 devices would mean switching SUSPHY after enumeration by link speed, which I left out. Awake, attaching the test hub raised total system power by 4.3 W, of which at most 0.37 W showed on the SoC-side 3.8 V rail; the rest is the hub's own draw through VBUS. The sleep filter costs nothing while no Type-C event happens. A charger plug-in raises about three events of a few milliseconds each.
Not covered
handle_bad_irqon its parent IRQ) when a Type-C event arrives while that controller's interrupt is disabled during the suspend transition. It's harmless (unhandled: 0) and predates this series; it's a separate follow-up.This series was developed and tested by Claude (Anthropic), working with me on the J713.