[PATCH v2] usb: typec: tipd: cd321x: replay the plug on resume

[PATCH v2] usb: typec: tipd: cd321x: replay the plug on resume

[Jacob Procee]

On Apple Silicon the Type-C PHY and the dwc3 controller behind a CD321x
port lose their state during s2idle while the plug stays connected, and
the CD321x does not raise an interrupt on resume. xhci notices the lost
state and re-initializes the host controller:

  xhci-hcd xhci-hcd.0.auto: xHC error in resume, USBSTS 0x401, Reinit
  usb usb1: root hub lost power or was reset
  usb 1-1: USB disconnect, device number 2

but the PHY is only ever programmed from typec_mux_set(), so a device
left plugged in (here a FIDO2 security key) never comes back until it is
unplugged and plugged in again.

Re-initializing only dwc3 after resume does not work: tearing dwc3 down
also powers off the PHY, and nothing turns it back on, so xhci then
times out ("Host halt failed, -110", "can't setup: -110").

Instead, make cd321x replay the plug on resume when a partner was
connected: mark the plug as changed and queue the update work, which
then goes through the same sequence as a real replug (role none, mux
safe, mux back to the partner's mode, role back on). Queue the update
work on system_freezable_wq, from the interrupt path as well, so that no
update runs before all devices have resumed and the work item is only
ever queued on one workqueue.

Tested on a 14" MacBook Pro (M1 Pro, j314s) with a backport of this
change to the Asahi 7.3.3 kernel: a USB-C security key left plugged in
re-enumerates right after "PM: suspend exit", where it previously stayed
gone until replugged. Unplugging the device while the system is asleep
also works: after resume the role goes to none, nothing is brought back
up, and replugging it later enumerates normally. Booting with the key
plugged in and replugging it while awake also still work. This mainline
version was build-tested only (W=1, checkpatch --strict clean).

Fixes: 82432bbfb9e8 ("usb: typec: tipd: Handle mode transitions for CD321x")
Assisted-by: LLM
Signed-off-by: Jacob Procee <sn125164@gmail.com>
---
Changes in v2:
- Queue update_work on system_freezable_wq from cd321x_connect() too, so
  an interrupt during resume (or a run still pending from before suspend)
  can't run the update before all devices have resumed, and the work item
  is no longer queued on two workqueues (sashiko-bot review).
- Retested on hardware: key left plugged in across suspend, key pulled
  while asleep and replugged, replug while awake, boot with the key in.
- Link to v1: https://lore.kernel.org/r/20261112113005.16418-1-sn125164@gmail.com

The pre-existing tps6598x_resume() issue sashiko-bot also pointed out
(I2C access before the reset GPIO is released) is not touched here: none
of the CD321x on this machine has a reset GPIO, so I can't test a fix.

 drivers/usb/typec/tipd/core.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/typec/tipd/core.c b/drivers/usb/typec/tipd/core.c
index 02539378ffed..bf2b08a5fad7 100644
--- a/drivers/usb/typec/tipd/core.c
+++ b/drivers/usb/typec/tipd/core.c
@@ -164,6 +164,7 @@ struct tipd_data {
 	bool (*read_power_status)(struct tps6598x *tps);
 	int (*reset)(struct tps6598x *tps);
 	int (*connect)(struct tps6598x *tps, u32 status);
+	void (*resume)(struct tps6598x *tps);
 };
 
 struct tps6598x {
@@ -906,11 +907,27 @@ static int cd321x_connect(struct tps6598x *tps, u32 status)
 	 * its last run anyway.
 	 */
 	cancel_delayed_work(&cd321x->update_work);
-	schedule_delayed_work(&cd321x->update_work, msecs_to_jiffies(CD321X_DEBOUNCE_DELAY_MS));
+	queue_delayed_work(system_freezable_wq, &cd321x->update_work,
+			   msecs_to_jiffies(CD321X_DEBOUNCE_DELAY_MS));
 
 	return 0;
 }
 
+static void cd321x_resume(struct tps6598x *tps)
+{
+	struct cd321x *cd321x = container_of(tps, struct cd321x, tps);
+
+	guard(mutex)(&tps->lock);
+
+	if (IS_ERR_OR_NULL(tps->partner))
+		return;
+
+	cd321x->update_status.status_changed |= TPS_STATUS_PLUG_PRESENT;
+	cancel_delayed_work(&cd321x->update_work);
+	queue_delayed_work(system_freezable_wq, &cd321x->update_work,
+			   msecs_to_jiffies(CD321X_DEBOUNCE_DELAY_MS));
+}
+
 static irqreturn_t cd321x_interrupt(int irq, void *data)
 {
 	struct tps6598x *tps = data;
@@ -2014,6 +2031,9 @@ static int __maybe_unused tps6598x_resume(struct device *dev)
 		queue_delayed_work(system_power_efficient_wq, &tps->wq_poll,
 				   msecs_to_jiffies(POLL_INTERVAL));
 
+	if (tps->data->resume)
+		tps->data->resume(tps);
+
 	return 0;
 }
 
@@ -2040,6 +2060,7 @@ static const struct tipd_data cd321x_data = {
 	.reset = cd321x_reset,
 	.switch_power_state = cd321x_switch_power_state,
 	.connect = cd321x_connect,
+	.resume = cd321x_resume,
 };
 
 static const struct tipd_data tps6598x_data = {
-- 
2.56.0

Re: [PATCH v2] usb: typec: tipd: cd321x: replay the plug on resume

[sashiko-bot]

> On Apple Silicon the Type-C PHY and the dwc3 controller behind a CD321x
> port lose their state during s2idle while the plug stays connected, and
> the CD321x does not raise an interrupt on resume. xhci notices the lost
> state and re-initializes the host controller:
> 
>   xhci-hcd xhci-hcd.0.auto: xHC error in resume, USBSTS 0x401, Reinit
>   usb usb1: root hub lost power or was reset
>   usb 1-1: USB disconnect, device number 2
> 
> but the PHY is only ever programmed from typec_mux_set(), so a device
> left plugged in (here a FIDO2 security key) never comes back until it is
> unplugged and plugged in again.
> 
> Re-initializing only dwc3 after resume does not work: tearing dwc3 down
> also powers off the PHY, and nothing turns it back on, so xhci then
> [ ... ]
> version was build-tested only (W=1, checkpatch --strict clean).
> 
> Fixes: 82432bbfb9e8 ("usb: typec: tipd: Handle mode transitions for CD321x")
> Assisted-by: LLM
> Signed-off-by: Jacob Procee <sn125164@gmail.com>

Sashiko has reviewed this patch and found no issues.

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