Fix/atcphy pipehandler#503
Conversation
At boot the Apple co-processor firmware does not acknowledge the pipehandler lock within the fixed 1ms timeout (PIPEHANDLER_LOCK_ACK_TIMEOUT_US = 1000). The function clears the lock bit and returns an error, leaving the PHY unconfigured for the rest of the session and preventing USB3 and DP alt mode from working. Replace the single poll attempt with a retry loop: up to 10 attempts with a 5-10ms sleep between each, giving the firmware approximately 100ms total to respond. Add an early return if the lock is already held to avoid double-locking. Tested on Apple M1 MacBook Air (J313), Fedora Asahi Remix 44, kernel 6.19.14. Boot with multiport hub connected no longer produces "Pipehandler lock not acked" errors and USB3 enumerates correctly. Signed-off-by: milijan <milijan@users.noreply.github.com>
…N on USB3_DP When DP alt mode is negotiated on hotplug, atcphy_mux_set() can be called while atcphy->pipehandler_up is still true. The existing WARN_ON fired unconditionally, but the situation has two distinct sub-cases that require different handling: 1. Target mode uses ATCPHY_PIPEHANDLER_STATE_DUMMY (DP-only, USB2, TBT, OFF): pipehandler_up being true is a teardown race between the TypeC mux call chain (cd321x_update_work -> typec_mux_set -> atcphy_mux_set) and the DWC3 PHY teardown (phy_power_off -> atcphy_usb3_power_off). By the time the kernel TypeC stack reaches atcphy_mux_set the USB PD Enter_Mode handshake is complete and the SS lanes are already being repurposed, so switching the pipehandler to dummy is safe. Tear it down explicitly here rather than relying on the race resolving itself. 2. Target mode uses ATCPHY_PIPEHANDLER_STATE_USB3 (USB3_DP, pin assignment D): pipehandler_up being true is correct and expected. The Apple USB-C Digital AV Multiport Adapter (05ac:1463) always negotiates pin assignment D, so the previous WARN_ON fired on every hotplug despite nothing being wrong with the pipehandler state. Retain the WARN_ON but scope it to case 1 only: warn when the target mode requires dummy state but the pipehandler is still up after the explicit teardown attempt (indicating atcphy_configure_pipehandler_dummy failed). Do not warn for USB3_DP transitions where USB3 pipehandler state is the correct precondition. Tested on Apple M1 MacBook Air (J313), Fedora Asahi Remix 44, kernel 6.19.14, with Apple USB-C Digital AV Multiport Adapter (05ac:1463). Hotplug no longer produces a spurious WARN_ON. Signed-off-by: milijan <milijan@users.noreply.github.com>
| } | ||
| do { | ||
| set32(atcphy->regs.pipehandler + PIPEHANDLER_LOCK_REQ, PIPEHANDLER_LOCK_EN); | ||
| ret = readl_poll_timeout(atcphy->regs.pipehandler + PIPEHANDLER_LOCK_ACK, reg, |
There was a problem hiding this comment.
I've never seen macos do this in traces and I highly suspect we're doing something else that's wrong here. have you seen this in traces or is this something you worked out by just playing with the hardware?
There was a problem hiding this comment.
Mostly try and error. My use case is using an M1 air in clamshell mode with fedora remix server (low idle power compute node for my homelab). So I need an Ethernet adapter. While it works fine when Hot Plugging the adapter, it fails on reboot (no phy detected/initialised). This was a defensive retry code attempt that seemed to work. But I agree with you something else is going on. Further testing shows that it is not reliable and if I removed some debug traces it would change the timing and fail again. Will probably move this issue to the forum before making further changes. Need to also understand some iboot behaviour differences between m1 and newer chips and assumptions made for how long it takes to load necessary firmwares. Thanks for reviewing.
There was a problem hiding this comment.
yeah, that's what I was afraid of :( atcphy shouldn't have any firmware fwiw
There was a problem hiding this comment.
Yes but some usb hubs also have an external display (dcpext) which requires some lane allocation, initialisation and firmware (video modes). Not sure how all this is orchestrated to work in a generic way for typeC hubs in non-macOS and macOS systems. Any idea?
| * If the pipehandler is still up and the target mode uses the dummy PIPE | ||
| * state (DP, USB2, TBT, OFF), tear it down before reconfiguring the lanes. | ||
| * Normally atcphy_usb3_power_off() does this, but a TypeC mux switch to | ||
| * DP alt mode can race the DWC3 PHY teardown on hotplug. By the time the |
There was a problem hiding this comment.
how does this race actually happen? can you describe this in more detail?
the fix here may be correct but I want to be sure we can't fix this at another level (e.g. the type c controller)
2 participants
Two fixes observed on J313 (M1 MacBook Air) with a USB-C multiport hub:
Retry pipehandler lock with backoff: Replace the single poll with up to 10 retries at 5–10 ms intervals (~100 ms total). Avoids having to hard cycle the usb-c connection.
Fix teardown race and spurious WARN on USB3_DP. Scope the WARN to the failure case only.
Tested on Fedora Asahi Remix 44, kernel 6.19.