Linux-PHY Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] phy: apple: atc: Prepare USB3-via-4 tunneling
@ 2026-08-21 13:50 Sven Peter
  2026-08-21 13:50 ` [PATCH 1/3] phy: apple: atc: Support DUMMY PIPEHANDLER state in configure_pipehandler Sven Peter
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Sven Peter @ 2026-08-21 13:50 UTC (permalink / raw)
  To: Janne Grunau, Neal Gompa, Vinod Koul, Neil Armstrong
  Cc: asahi, linux-arm-kernel, linux-phy, linux-kernel, Sven Peter,
	Paul Cristian, stable

Hi,

This short series first fixes an oversight that prevents USB2 from
working when DP AltMode with 4 lanes is used. Then it adds the required
sequence to point dwc3's PIPE interface to the USB4 Native Host
Interface which I plan to submit soon to support USB3-via-USB4 tunnels.

Best,

Sven

Signed-off-by: Sven Peter <sven@kernel.org>
---
Sven Peter (3):
      phy: apple: atc: Support DUMMY PIPEHANDLER state in configure_pipehandler
      phy: apple: atc: Factor out the PIPE mux sequence
      phy: apple: atc: Implement the USB4 pipehandler state

 drivers/phy/apple/atc.c | 91 ++++++++++++++++++++++++++++++++-----------------
 1 file changed, 59 insertions(+), 32 deletions(-)
---
base-commit: 818bebeb63dd6bf5f4e07e145f6cdbace520a34c
change-id: 20260821-b4-atcphy-usb4-f8f9008efb1d

Best regards,
--  
Sven Peter <sven@kernel.org>



-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

* [PATCH 1/3] phy: apple: atc: Support DUMMY PIPEHANDLER state in configure_pipehandler
  2026-08-21 13:50 [PATCH 0/3] phy: apple: atc: Prepare USB3-via-4 tunneling Sven Peter
@ 2026-08-21 13:50 ` Sven Peter
  2026-08-21 14:01   ` sashiko-bot
  2026-08-21 13:50 ` [PATCH 2/3] phy: apple: atc: Factor out the PIPE mux sequence Sven Peter
  2026-08-21 13:50 ` [PATCH 3/3] phy: apple: atc: Implement the USB4 pipehandler state Sven Peter
  2 siblings, 1 reply; 7+ messages in thread
From: Sven Peter @ 2026-08-21 13:50 UTC (permalink / raw)
  To: Janne Grunau, Neal Gompa, Vinod Koul, Neil Armstrong
  Cc: asahi, linux-arm-kernel, linux-phy, linux-kernel, Sven Peter,
	Paul Cristian, stable

For both Thunderbolt and DisplayPort atcphy_configure_pipehandler is
reached with a request to switch to the DUMMY state (i.e. usb2 only).
With the current code this breaks USB2 when all four SS lanes are used
for DisplayPort AltMode because the -EINVAL is passed all the way back
to the phy_set_mode() call which results in tearing down xhci and dwc3
again.
Let's actually handle that case correctly and also drop the default from
the switch such that we get a compiler warning if another pipehandler
state is ever added and forgotten here.

Reported-by: Paul Cristian <p4ulcristian@gmail.com>
Closes: https://github.com/AsahiLinux/linux/pull/515
Fixes: 8e98ca1e74db ("phy: apple: Add Apple Type-C PHY")
Cc: stable@vger.kernel.org
Signed-off-by: Sven Peter <sven@kernel.org>
---
 drivers/phy/apple/atc.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/apple/atc.c b/drivers/phy/apple/atc.c
index 4156fabad742..a3162f25e6cd 100644
--- a/drivers/phy/apple/atc.c
+++ b/drivers/phy/apple/atc.c
@@ -1121,7 +1121,7 @@ static int atcphy_configure_pipehandler_dummy(struct apple_atcphy *atcphy)
 
 static int atcphy_configure_pipehandler(struct apple_atcphy *atcphy, bool host)
 {
-	int ret;
+	int ret = -EINVAL;
 
 	lockdep_assert_held(&atcphy->lock);
 
@@ -1136,8 +1136,10 @@ static int atcphy_configure_pipehandler(struct apple_atcphy *atcphy, bool host)
 		ret = atcphy_configure_pipehandler_dummy(atcphy);
 		atcphy->pipehandler_up = false;
 		break;
-	default:
-		ret = -EINVAL;
+	case ATCPHY_PIPEHANDLER_STATE_DUMMY:
+		ret = atcphy_configure_pipehandler_dummy(atcphy);
+		atcphy->pipehandler_up = false;
+		break;
 	}
 
 	return ret;

-- 
2.55.0



-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

* [PATCH 2/3] phy: apple: atc: Factor out the PIPE mux sequence
  2026-08-21 13:50 [PATCH 0/3] phy: apple: atc: Prepare USB3-via-4 tunneling Sven Peter
  2026-08-21 13:50 ` [PATCH 1/3] phy: apple: atc: Support DUMMY PIPEHANDLER state in configure_pipehandler Sven Peter
@ 2026-08-21 13:50 ` Sven Peter
  2026-08-21 13:50 ` [PATCH 3/3] phy: apple: atc: Implement the USB4 pipehandler state Sven Peter
  2 siblings, 0 replies; 7+ messages in thread
From: Sven Peter @ 2026-08-21 13:50 UTC (permalink / raw)
  To: Janne Grunau, Neal Gompa, Vinod Koul, Neil Armstrong
  Cc: asahi, linux-arm-kernel, linux-phy, linux-kernel, Sven Peter

This sequence is already used in three places and we're about to add a
fourth copy so let's factor it out to a helper. No functional change.

Signed-off-by: Sven Peter <sven@kernel.org>
---
 drivers/phy/apple/atc.c | 46 +++++++++++++++++++---------------------------
 1 file changed, 19 insertions(+), 27 deletions(-)

diff --git a/drivers/phy/apple/atc.c b/drivers/phy/apple/atc.c
index a3162f25e6cd..96158d348656 100644
--- a/drivers/phy/apple/atc.c
+++ b/drivers/phy/apple/atc.c
@@ -972,6 +972,19 @@ static int atcphy_pipehandler_check(struct apple_atcphy *atcphy)
 	return 0;
 }
 
+static void atcphy_pipehandler_set_mux(struct apple_atcphy *atcphy, u32 data, u32 clk)
+{
+	mask32(atcphy->regs.pipehandler + PIPEHANDLER_MUX_CTRL, PIPEHANDLER_MUX_CTRL_CLK,
+	       FIELD_PREP(PIPEHANDLER_MUX_CTRL_CLK, PIPEHANDLER_MUX_CTRL_CLK_OFF));
+	udelay(10);
+	mask32(atcphy->regs.pipehandler + PIPEHANDLER_MUX_CTRL, PIPEHANDLER_MUX_CTRL_DATA,
+	       FIELD_PREP(PIPEHANDLER_MUX_CTRL_DATA, data));
+	udelay(10);
+	mask32(atcphy->regs.pipehandler + PIPEHANDLER_MUX_CTRL, PIPEHANDLER_MUX_CTRL_CLK,
+	       FIELD_PREP(PIPEHANDLER_MUX_CTRL_CLK, clk));
+	udelay(10);
+}
+
 static int atcphy_configure_pipehandler_usb3(struct apple_atcphy *atcphy, bool host)
 {
 	int ret;
@@ -1054,15 +1067,8 @@ static int atcphy_configure_pipehandler_usb3(struct apple_atcphy *atcphy, bool h
 	}
 
 	/* Configure PIPE mux to USB3 PHY */
-	mask32(atcphy->regs.pipehandler + PIPEHANDLER_MUX_CTRL, PIPEHANDLER_MUX_CTRL_CLK,
-	       FIELD_PREP(PIPEHANDLER_MUX_CTRL_CLK, PIPEHANDLER_MUX_CTRL_CLK_OFF));
-	udelay(10);
-	mask32(atcphy->regs.pipehandler + PIPEHANDLER_MUX_CTRL, PIPEHANDLER_MUX_CTRL_DATA,
-	       FIELD_PREP(PIPEHANDLER_MUX_CTRL_DATA, PIPEHANDLER_MUX_CTRL_DATA_USB3));
-	udelay(10);
-	mask32(atcphy->regs.pipehandler + PIPEHANDLER_MUX_CTRL, PIPEHANDLER_MUX_CTRL_CLK,
-	       FIELD_PREP(PIPEHANDLER_MUX_CTRL_CLK, PIPEHANDLER_MUX_CTRL_CLK_USB3));
-	udelay(10);
+	atcphy_pipehandler_set_mux(atcphy, PIPEHANDLER_MUX_CTRL_DATA_USB3,
+				   PIPEHANDLER_MUX_CTRL_CLK_USB3);
 
 	/* Remove link detection override */
 	clear32(atcphy->regs.pipehandler + PIPEHANDLER_OVERRIDE, PIPEHANDLER_OVERRIDE_RXVALID);
@@ -1097,15 +1103,8 @@ static int atcphy_configure_pipehandler_dummy(struct apple_atcphy *atcphy)
 		dev_warn(atcphy->dev, "Failed to lock pipehandler");
 
 	/* Switch to dummy PHY */
-	mask32(atcphy->regs.pipehandler + PIPEHANDLER_MUX_CTRL, PIPEHANDLER_MUX_CTRL_CLK,
-	       FIELD_PREP(PIPEHANDLER_MUX_CTRL_CLK, PIPEHANDLER_MUX_CTRL_CLK_OFF));
-	udelay(10);
-	mask32(atcphy->regs.pipehandler + PIPEHANDLER_MUX_CTRL, PIPEHANDLER_MUX_CTRL_DATA,
-	       FIELD_PREP(PIPEHANDLER_MUX_CTRL_DATA, PIPEHANDLER_MUX_CTRL_DATA_DUMMY));
-	udelay(10);
-	mask32(atcphy->regs.pipehandler + PIPEHANDLER_MUX_CTRL, PIPEHANDLER_MUX_CTRL_CLK,
-	       FIELD_PREP(PIPEHANDLER_MUX_CTRL_CLK, PIPEHANDLER_MUX_CTRL_CLK_DUMMY));
-	udelay(10);
+	atcphy_pipehandler_set_mux(atcphy, PIPEHANDLER_MUX_CTRL_DATA_DUMMY,
+				   PIPEHANDLER_MUX_CTRL_CLK_DUMMY);
 
 	ret = atcphy_pipehandler_unlock(atcphy);
 	if (ret)
@@ -1149,15 +1148,8 @@ static void atcphy_setup_pipehandler(struct apple_atcphy *atcphy)
 {
 	lockdep_assert_held(&atcphy->lock);
 
-	mask32(atcphy->regs.pipehandler + PIPEHANDLER_MUX_CTRL, PIPEHANDLER_MUX_CTRL_CLK,
-	       FIELD_PREP(PIPEHANDLER_MUX_CTRL_CLK, PIPEHANDLER_MUX_CTRL_CLK_OFF));
-	udelay(10);
-	mask32(atcphy->regs.pipehandler + PIPEHANDLER_MUX_CTRL, PIPEHANDLER_MUX_CTRL_DATA,
-	       FIELD_PREP(PIPEHANDLER_MUX_CTRL_DATA, PIPEHANDLER_MUX_CTRL_DATA_DUMMY));
-	udelay(10);
-	mask32(atcphy->regs.pipehandler + PIPEHANDLER_MUX_CTRL, PIPEHANDLER_MUX_CTRL_CLK,
-	       FIELD_PREP(PIPEHANDLER_MUX_CTRL_CLK, PIPEHANDLER_MUX_CTRL_CLK_DUMMY));
-	udelay(10);
+	atcphy_pipehandler_set_mux(atcphy, PIPEHANDLER_MUX_CTRL_DATA_DUMMY,
+				   PIPEHANDLER_MUX_CTRL_CLK_DUMMY);
 }
 
 static void atcphy_configure_lanes(struct apple_atcphy *atcphy, enum atcphy_mode mode)

-- 
2.55.0



-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

* [PATCH 3/3] phy: apple: atc: Implement the USB4 pipehandler state
  2026-08-21 13:50 [PATCH 0/3] phy: apple: atc: Prepare USB3-via-4 tunneling Sven Peter
  2026-08-21 13:50 ` [PATCH 1/3] phy: apple: atc: Support DUMMY PIPEHANDLER state in configure_pipehandler Sven Peter
  2026-08-21 13:50 ` [PATCH 2/3] phy: apple: atc: Factor out the PIPE mux sequence Sven Peter
@ 2026-08-21 13:50 ` Sven Peter
  2026-08-21 14:01   ` sashiko-bot
  2 siblings, 1 reply; 7+ messages in thread
From: Sven Peter @ 2026-08-21 13:50 UTC (permalink / raw)
  To: Janne Grunau, Neal Gompa, Vinod Koul, Neil Armstrong
  Cc: asahi, linux-arm-kernel, linux-phy, linux-kernel, Sven Peter

USB3 tunneled via USB4 requires dwc3's PIPE interface to be switched to
the USB4 NHI which will take care of the tunneling. Add the required
bringup sequence such that USB3 tunnels work once the USB4 NHI is
upstream.

Signed-off-by: Sven Peter <sven@kernel.org>
---
 drivers/phy/apple/atc.c | 41 +++++++++++++++++++++++++++++++++++++----
 1 file changed, 37 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/apple/atc.c b/drivers/phy/apple/atc.c
index 96158d348656..613bcd0ae886 100644
--- a/drivers/phy/apple/atc.c
+++ b/drivers/phy/apple/atc.c
@@ -1084,6 +1084,41 @@ static int atcphy_configure_pipehandler_usb3(struct apple_atcphy *atcphy, bool h
 	return 0;
 }
 
+static int atcphy_configure_pipehandler_usb4(struct apple_atcphy *atcphy)
+{
+	int ret;
+
+	ret = atcphy_pipehandler_check(atcphy);
+	if (ret)
+		return ret;
+
+	/* Force disable link detection */
+	clear32(atcphy->regs.pipehandler + PIPEHANDLER_OVERRIDE_VALUES,
+		PIPEHANDLER_OVERRIDE_VAL_RXDETECT0 | PIPEHANDLER_OVERRIDE_VAL_RXDETECT1);
+	set32(atcphy->regs.pipehandler + PIPEHANDLER_OVERRIDE, PIPEHANDLER_OVERRIDE_RXVALID);
+	set32(atcphy->regs.pipehandler + PIPEHANDLER_OVERRIDE, PIPEHANDLER_OVERRIDE_RXDETECT);
+
+	ret = atcphy_pipehandler_lock(atcphy);
+	if (ret) {
+		dev_err(atcphy->dev, "Failed to lock pipehandler\n");
+		return ret;
+	}
+
+	/* Configure PIPE mux to the USB4/Thunderbolt controller */
+	atcphy_pipehandler_set_mux(atcphy, PIPEHANDLER_MUX_CTRL_DATA_USB4,
+				   PIPEHANDLER_MUX_CTRL_CLK_USB4);
+
+	/* Remove link detection override */
+	clear32(atcphy->regs.pipehandler + PIPEHANDLER_OVERRIDE, PIPEHANDLER_OVERRIDE_RXVALID);
+	clear32(atcphy->regs.pipehandler + PIPEHANDLER_OVERRIDE, PIPEHANDLER_OVERRIDE_RXDETECT);
+
+	ret = atcphy_pipehandler_unlock(atcphy);
+	if (ret)
+		dev_warn(atcphy->dev, "Failed to unlock pipehandler\n");
+
+	return 0;
+}
+
 static int atcphy_configure_pipehandler_dummy(struct apple_atcphy *atcphy)
 {
 	int ret;
@@ -1130,10 +1165,8 @@ static int atcphy_configure_pipehandler(struct apple_atcphy *atcphy, bool host)
 		atcphy->pipehandler_up = true;
 		break;
 	case ATCPHY_PIPEHANDLER_STATE_USB4:
-		dev_warn(atcphy->dev,
-			 "ATCPHY_PIPEHANDLER_STATE_USB4 not implemented; falling back to USB2\n");
-		ret = atcphy_configure_pipehandler_dummy(atcphy);
-		atcphy->pipehandler_up = false;
+		ret = atcphy_configure_pipehandler_usb4(atcphy);
+		atcphy->pipehandler_up = true;
 		break;
 	case ATCPHY_PIPEHANDLER_STATE_DUMMY:
 		ret = atcphy_configure_pipehandler_dummy(atcphy);

-- 
2.55.0



-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

* Re: [PATCH 1/3] phy: apple: atc: Support DUMMY PIPEHANDLER state in configure_pipehandler
  2026-08-21 13:50 ` [PATCH 1/3] phy: apple: atc: Support DUMMY PIPEHANDLER state in configure_pipehandler Sven Peter
@ 2026-08-21 14:01   ` sashiko-bot
  0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-08-21 14:01 UTC (permalink / raw)
  To: Sven Peter; +Cc: neil.armstrong, olteanv, vkoul, linux-phy

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] `atcphy->pipehandler_up` is unconditionally set to true even if `atcphy_configure_pipehandler_usb3()` fails, masking errors on subsequent configuration attempts.
--

--- Patch [1]: [PATCH 1/3] phy: apple: atc: Support DUMMY PIPEHANDLER state in configure_pipehandler ---
commit 969f2e11a2c1dceb64c1ff0f1f1da36afa041518
Author: Sven Peter <sven@kernel.org>

phy: apple: atc: Support DUMMY PIPEHANDLER state in configure_pipehandler

For both Thunderbolt and DisplayPort atcphy_configure_pipehandler is
reached with a request to switch to the DUMMY state (i.e. usb2 only).
This actually handles that case correctly and drops the default from
the switch to enable compiler warnings for missing pipehandler states.

Closes: https://github.com/AsahiLinux/linux/pull/515
Fixes: 8e98ca1e74db ("phy: apple: Add Apple Type-C PHY")

> diff --git a/drivers/phy/apple/atc.c b/drivers/phy/apple/atc.c
> index 4156fabad742c..a3162f25e6cd6 100644
> --- a/drivers/phy/apple/atc.c
> +++ b/drivers/phy/apple/atc.c
> @@ -1121,7 +1121,7 @@ static int atcphy_configure_pipehandler_dummy(struct apple_atcphy *atcphy)
>  
>  static int atcphy_configure_pipehandler(struct apple_atcphy *atcphy, bool host)
>  {
> -	int ret;
> +	int ret = -EINVAL;
>  
>  	lockdep_assert_held(&atcphy->lock);
>  

[Severity: High]
This isn't a bug introduced by this patch, but looking at the USB3 state
handling in atcphy_configure_pipehandler() just below this:

drivers/phy/apple/atc.c:atcphy_configure_pipehandler() {
    ...
	case ATCPHY_PIPEHANDLER_STATE_USB3:
		ret = atcphy_configure_pipehandler_usb3(atcphy, host);
		atcphy->pipehandler_up = true;
		break;
    ...
}

Is it safe to unconditionally set atcphy->pipehandler_up to true even if
atcphy_configure_pipehandler_usb3() fails?

If the USB3 PHY initialization fails (for example, due to a lock timeout or
BIST failure), the PHY is not correctly configured but the state is still
marked as up.

Could this cause subsequent mode setting attempts (like from the generic
xHCI code) to falsely return success without re-attempting configuration,
leaving the USB port permanently broken?

[ ... ]

> @@ -1136,8 +1136,10 @@ static int atcphy_configure_pipehandler(struct apple_atcphy *atcphy, bool host)
>  		ret = atcphy_configure_pipehandler_dummy(atcphy);
>  		atcphy->pipehandler_up = false;
>  		break;
> -	default:
> -		ret = -EINVAL;
> +	case ATCPHY_PIPEHANDLER_STATE_DUMMY:
> +		ret = atcphy_configure_pipehandler_dummy(atcphy);
> +		atcphy->pipehandler_up = false;
> +		break;
>  	}
>  
>  	return ret;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-b4-atcphy-usb4-v1-0-45c0b741e0c0@kernel.org?part=1

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

* Re: [PATCH 3/3] phy: apple: atc: Implement the USB4 pipehandler state
  2026-08-21 13:50 ` [PATCH 3/3] phy: apple: atc: Implement the USB4 pipehandler state Sven Peter
@ 2026-08-21 14:01   ` sashiko-bot
  2026-08-21 14:05     ` Sven Peter
  0 siblings, 1 reply; 7+ messages in thread
From: sashiko-bot @ 2026-08-21 14:01 UTC (permalink / raw)
  To: Sven Peter; +Cc: linux-phy, vkoul, neil.armstrong, olteanv

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Missing cleanup of `PIPEHANDLER_OVERRIDE` registers on lock failure leaves link detection permanently disabled.

Pre-existing issues:
- [High] Unconditional state update of `atcphy->pipehandler_up = true` on the error path masks hardware configuration failures.
--

--- Patch [3]: [PATCH 3/3] phy: apple: atc: Implement the USB4 pipehandler state ---
commit 6b90c04f8cd4e7a3f818a4bc0eb1057996bb1845
Author: Sven Peter <sven@kernel.org>

phy: apple: atc: Implement the USB4 pipehandler state

This commit implements the USB4 pipehandler state for the Apple ATC PHY driver.
It switches the dwc3's PIPE interface to the USB4 NHI which takes care of
tunneling USB3 over USB4.

> diff --git a/drivers/phy/apple/atc.c b/drivers/phy/apple/atc.c
> index 96158d3486560..613bcd0ae8865 100644
> --- a/drivers/phy/apple/atc.c
> +++ b/drivers/phy/apple/atc.c
> @@ -1084,6 +1084,41 @@ static int atcphy_configure_pipehandler_usb3(struct apple_atcphy *atcphy, bool h
>  	return 0;
>  }
>  
> +static int atcphy_configure_pipehandler_usb4(struct apple_atcphy *atcphy)
> +{
> +	int ret;
> +
> +	ret = atcphy_pipehandler_check(atcphy);
> +	if (ret)
> +		return ret;
> +
> +	/* Force disable link detection */
> +	clear32(atcphy->regs.pipehandler + PIPEHANDLER_OVERRIDE_VALUES,
> +		PIPEHANDLER_OVERRIDE_VAL_RXDETECT0 | PIPEHANDLER_OVERRIDE_VAL_RXDETECT1);
> +	set32(atcphy->regs.pipehandler + PIPEHANDLER_OVERRIDE, PIPEHANDLER_OVERRIDE_RXVALID);
> +	set32(atcphy->regs.pipehandler + PIPEHANDLER_OVERRIDE, PIPEHANDLER_OVERRIDE_RXDETECT);
> +
> +	ret = atcphy_pipehandler_lock(atcphy);
> +	if (ret) {
> +		dev_err(atcphy->dev, "Failed to lock pipehandler\n");
> +		return ret;
> +	}

[Severity: High]
If atcphy_pipehandler_lock() fails and returns early, the link detection
override bits (PIPEHANDLER_OVERRIDE_RXVALID and
PIPEHANDLER_OVERRIDE_RXDETECT) are left set.

Does this permanently disable link detection on the PHY hardware for
subsequent retries until a hard reset occurs?

[ ... ]

> @@ -1130,10 +1165,8 @@ static int atcphy_configure_pipehandler(struct apple_atcphy *atcphy, bool host)
>  		atcphy->pipehandler_up = true;
>  		break;
>  	case ATCPHY_PIPEHANDLER_STATE_USB4:
> -		dev_warn(atcphy->dev,
> -			 "ATCPHY_PIPEHANDLER_STATE_USB4 not implemented; falling back to USB2\n");
> -		ret = atcphy_configure_pipehandler_dummy(atcphy);
> -		atcphy->pipehandler_up = false;
> +		ret = atcphy_configure_pipehandler_usb4(atcphy);
> +		atcphy->pipehandler_up = true;

[Severity: High]
This is a pre-existing issue, but if atcphy_configure_pipehandler_usb4()
returns an error, the software state atcphy->pipehandler_up is still
unconditionally set to true.

When a subsequent caller (such as the generic xhci code) invokes
atcphy_usb3_set_mode(), it checks atcphy->pipehandler_up, assumes the PHY
is successfully configured, and returns 0 without actually doing anything.

Could this state desynchronization mask hardware configuration failures and
prevent subsequent valid retries from successfully initializing the PHY?

>  		break;
>  	case ATCPHY_PIPEHANDLER_STATE_DUMMY:
>  		ret = atcphy_configure_pipehandler_dummy(atcphy);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260821-b4-atcphy-usb4-v1-0-45c0b741e0c0@kernel.org?part=3

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

* Re: [PATCH 3/3] phy: apple: atc: Implement the USB4 pipehandler state
  2026-08-21 14:01   ` sashiko-bot
@ 2026-08-21 14:05     ` Sven Peter
  0 siblings, 0 replies; 7+ messages in thread
From: Sven Peter @ 2026-08-21 14:05 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-phy, vkoul, neil.armstrong, olteanv



On 8/21/26 16:01, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>
> New issues:
> - [High] Missing cleanup of `PIPEHANDLER_OVERRIDE` registers on lock failure leaves link detection permanently disabled.
>
> Pre-existing issues:
> - [High] Unconditional state update of `atcphy->pipehandler_up = true` on the error path masks hardware configuration failures.
> --
>
> --- Patch [3]: [PATCH 3/3] phy: apple: atc: Implement the USB4 pipehandler state ---
> commit 6b90c04f8cd4e7a3f818a4bc0eb1057996bb1845
> Author: Sven Peter <sven@kernel.org>
>
> phy: apple: atc: Implement the USB4 pipehandler state
>
> This commit implements the USB4 pipehandler state for the Apple ATC PHY driver.
> It switches the dwc3's PIPE interface to the USB4 NHI which takes care of
> tunneling USB3 over USB4.
>
>> diff --git a/drivers/phy/apple/atc.c b/drivers/phy/apple/atc.c
>> index 96158d3486560..613bcd0ae8865 100644
>> --- a/drivers/phy/apple/atc.c
>> +++ b/drivers/phy/apple/atc.c
>> @@ -1084,6 +1084,41 @@ static int atcphy_configure_pipehandler_usb3(struct apple_atcphy *atcphy, bool h
>>   	return 0;
>>   }
>>   
>> +static int atcphy_configure_pipehandler_usb4(struct apple_atcphy *atcphy)
>> +{
>> +	int ret;
>> +
>> +	ret = atcphy_pipehandler_check(atcphy);
>> +	if (ret)
>> +		return ret;
>> +
>> +	/* Force disable link detection */
>> +	clear32(atcphy->regs.pipehandler + PIPEHANDLER_OVERRIDE_VALUES,
>> +		PIPEHANDLER_OVERRIDE_VAL_RXDETECT0 | PIPEHANDLER_OVERRIDE_VAL_RXDETECT1);
>> +	set32(atcphy->regs.pipehandler + PIPEHANDLER_OVERRIDE, PIPEHANDLER_OVERRIDE_RXVALID);
>> +	set32(atcphy->regs.pipehandler + PIPEHANDLER_OVERRIDE, PIPEHANDLER_OVERRIDE_RXDETECT);
>> +
>> +	ret = atcphy_pipehandler_lock(atcphy);
>> +	if (ret) {
>> +		dev_err(atcphy->dev, "Failed to lock pipehandler\n");
>> +		return ret;
>> +	}
> [Severity: High]
> If atcphy_pipehandler_lock() fails and returns early, the link detection
> override bits (PIPEHANDLER_OVERRIDE_RXVALID and
> PIPEHANDLER_OVERRIDE_RXDETECT) are left set.
>
> Does this permanently disable link detection on the PHY hardware for
> subsequent retries until a hard reset occurs?

No, if this fails the port will be broken until the next unplug/plug 
which resets everything anyway.
Same pattern has been working in the other paths and since we have no 
documentation for this PHY I won't touch it or do anything else here.



Sven


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

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

end of thread, other threads:[~2026-08-21 14:05 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-21 13:50 [PATCH 0/3] phy: apple: atc: Prepare USB3-via-4 tunneling Sven Peter
2026-08-21 13:50 ` [PATCH 1/3] phy: apple: atc: Support DUMMY PIPEHANDLER state in configure_pipehandler Sven Peter
2026-08-21 14:01   ` sashiko-bot
2026-08-21 13:50 ` [PATCH 2/3] phy: apple: atc: Factor out the PIPE mux sequence Sven Peter
2026-08-21 13:50 ` [PATCH 3/3] phy: apple: atc: Implement the USB4 pipehandler state Sven Peter
2026-08-21 14:01   ` sashiko-bot
2026-08-21 14:05     ` Sven Peter

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