Skip to content

Fix/atcphy pipehandler #503

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
milijan wants to merge 1 commit into
base: asahi
Choose a base branch
from
Open

Fix/atcphy pipehandler #503

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 38 additions & 16 deletions drivers/phy/apple/atc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@
#define ACIOPHY_CMN_SHM_STS_REG0_CMD_READY BIT(0)

#define CIO3PLL_CLK_CTRL 0x2a00

#define CIO3PLL_CLK_PCLK_EN BIT(1)
#define CIO3PLL_CLK_REFCLK_EN BIT(5)

Expand Down Expand Up @@ -922,23 +923,25 @@ static void atcphy_apply_tunables(struct apple_atcphy *atcphy, enum atcphy_mode

static int atcphy_pipehandler_lock(struct apple_atcphy *atcphy)
{
int ret;
int ret, retries = 10;
u32 reg;

if (readl(atcphy->regs.pipehandler + PIPEHANDLER_LOCK_REQ) & PIPEHANDLER_LOCK_EN) {
dev_warn(atcphy->dev, "Pipehandler already locked\n");
return 0;
}

set32(atcphy->regs.pipehandler + PIPEHANDLER_LOCK_REQ, PIPEHANDLER_LOCK_EN);

ret = readl_poll_timeout(atcphy->regs.pipehandler + PIPEHANDLER_LOCK_ACK, reg,
reg & PIPEHANDLER_LOCK_EN, 10, PIPEHANDLER_LOCK_ACK_TIMEOUT_US);
if (ret) {
clear32(atcphy->regs.pipehandler + PIPEHANDLER_LOCK_REQ, 1);
dev_warn(atcphy->dev, "Pipehandler lock not acked.\n");
}
do {
set32(atcphy->regs.pipehandler + PIPEHANDLER_LOCK_REQ, PIPEHANDLER_LOCK_EN);
ret = readl_poll_timeout(atcphy->regs.pipehandler + PIPEHANDLER_LOCK_ACK, reg,

@svenpeter42 svenpeter42 May 31, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

@milijan milijan Jun 1, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@svenpeter42 svenpeter42 Jun 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, that's what I was afraid of :( atcphy shouldn't have any firmware fwiw

@milijan milijan Jun 1, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

reg & PIPEHANDLER_LOCK_EN, 10, PIPEHANDLER_LOCK_ACK_TIMEOUT_US);
if (!ret)
return 0;
clear32(atcphy->regs.pipehandler + PIPEHANDLER_LOCK_REQ, PIPEHANDLER_LOCK_EN);
usleep_range(5000, 10000);
} while (--retries);

dev_warn(atcphy->dev, "Pipehandler lock not acked.\n");
return ret;
}

Expand Down Expand Up @@ -1000,6 +1003,8 @@ static int atcphy_configure_pipehandler_usb3(struct apple_atcphy *atcphy, bool h
ret = atcphy_pipehandler_lock(atcphy);
if (ret) {
dev_err(atcphy->dev, "Failed to lock pipehandler");
clear32(atcphy->regs.pipehandler + PIPEHANDLER_OVERRIDE,
PIPEHANDLER_OVERRIDE_RXVALID | PIPEHANDLER_OVERRIDE_RXDETECT);
return ret;
}

Expand Down Expand Up @@ -1835,6 +1840,7 @@ static int atcphy_usb3_power_off(struct phy *phy)
static int atcphy_usb3_set_mode(struct phy *phy, enum phy_mode mode, int submode)
{
struct apple_atcphy *atcphy = phy_get_drvdata(phy);
int ret;

guard(mutex)(&atcphy->lock);

Expand All @@ -1846,9 +1852,17 @@ static int atcphy_usb3_set_mode(struct phy *phy, enum phy_mode mode, int submode
if (atcphy->pipehandler_up)
return 0;

/* xhci can race tipd's debounced mux_set; the PHY may still be OFF */
if (atcphy->mode == APPLE_ATCPHY_MODE_OFF) {
ret = atcphy_configure(atcphy, APPLE_ATCPHY_MODE_USB3);
if (ret)
return ret;
}

switch (mode) {
case PHY_MODE_USB_HOST:
return atcphy_configure_pipehandler(atcphy, true);
/* BIST's RXDETECT override suppresses xhci link detection at boot */
return atcphy_configure_pipehandler(atcphy, false);
case PHY_MODE_USB_DEVICE:
return atcphy_configure_pipehandler(atcphy, false);
default:
Expand Down Expand Up @@ -2081,6 +2095,7 @@ static int atcphy_mux_set(struct typec_mux_dev *mux, struct typec_mux_state *sta
{
struct apple_atcphy *atcphy = typec_mux_get_drvdata(mux);
enum atcphy_mode target_mode;
int ret;

guard(mutex)(&atcphy->lock);

Expand Down Expand Up @@ -2137,12 +2152,19 @@ static int atcphy_mux_set(struct typec_mux_dev *mux, struct typec_mux_state *sta
if (atcphy->mode == target_mode)
return 0;

/*
* If the pipehandler is still/already up here there's a bug somewhere so make sure to
* complain loudly. We can still try to switch modes and hope for the best though,
* in the worst case the hardware will fall back to USB2-only.
*/
WARN_ON_ONCE(atcphy->pipehandler_up);
/* A DP alt-mode mux switch can race DWC3's phy_power_off on hotplug */
if (atcphy->pipehandler_up &&
atcphy_modes[target_mode].pipehandler_state == ATCPHY_PIPEHANDLER_STATE_DUMMY) {
ret = atcphy_configure_pipehandler_dummy(atcphy);
if (ret)
dev_warn(atcphy->dev,
"Failed to clear pipehandler before mode switch: %d\n", ret);
else
atcphy->pipehandler_up = false;
}

WARN_ON_ONCE(atcphy->pipehandler_up &&
atcphy_modes[target_mode].pipehandler_state == ATCPHY_PIPEHANDLER_STATE_DUMMY);
return atcphy_configure(atcphy, target_mode);
}

Expand Down