t8140: rearm the debug link reliably after a sleep

drivers/soc/apple/t8140-sleep.c

@@ -181,6 +181,12 @@ struct t8140_s2r_mailbox {
 #define HPM_CMD_COLD_RESET	0x64696147
 #define HPM_SSPS_S3		3
 #define HPM_RESET_MS		1000
+#define HPM_PAGE_POWER_STATUS	0x3f
+#define HPM_POWER_CONNECTED	BIT(0)
+#define HPM_POWER_TYPEC_MODE	GENMASK(3, 2)
+#define HPM_POWER_TYPEC_PD	3
+#define HPM_CONTRACT_MS		5000
+#define HPM_RESET_TRIES		3
 
 /* A PMU register write on the sleep's SPMI bus */
 #define SPMI_CMD_WRITE		(BIT(15) | (0xe << 8) | 0x30)
@@ -1736,18 +1742,46 @@ static int t8140_hpm_sleep(struct t8140_sleep *s)
 	return ret;
 }
 
+/*
+ * The partner takes the reset as an unplug, attaches again and renegotiates its contract.
+ * Left to finish across the commit, that sometimes failed: neither side had a contract or
+ * saw the other, and no VDM got through until a replug. Returns the ms it took.
+ */
+static int t8140_hpm_wait_contract(struct t8140_sleep *s)
+{
+	bool wake = true;
+	u32 val = 0;
+	int ms, ret;
+
+	for (ms = 0; ms < HPM_CONTRACT_MS; ms += 10) {
+		ret = wake ? t8140_hpm_cmd(s, SPMI_OP_WAKEUP, 0, NULL, NULL) : 0;
+		if (!ret)
+			ret = t8140_hpm_select(s, HPM_PAGE_POWER_STATUS);
+		if (!ret)
+			ret = t8140_hpm_cmd(s, SPMI_OP_EXT_READ(4), HPM_REG_DATA, NULL, &val);
+		if (!ret && (val & HPM_POWER_CONNECTED) &&
+		    FIELD_GET(HPM_POWER_TYPEC_MODE, val) == HPM_POWER_TYPEC_PD)
+			return ms;
+		wake = ret;
+		mdelay(10);
+	}
+
+	return -ETIMEDOUT;
+}
+
 /*
  * The debug link stays up with the ATC0 domains off, and it holds the debug domains
  * active, which macOS has idle. The only known way to end it is the port controller's
  * cold reset. To the SMC that's a cable pulled: it queues the port's events and records
  * them as wake reasons. So it has to come before the SMC's AP idle notice, not after, or
- * SMC.OutboxNotEmpty is up at the commit. tipd rearms the link after a refusal.
+ * SMC.OutboxNotEmpty is up at the commit, and so does the partner's new contract.
+ * tipd rearms the link after a refusal.
  */
 static bool t8140_sleep_usb_down(struct t8140_sleep *s)
 {
 	u32 cmd = HPM_CMD_COLD_RESET;
 	u32 enable;
-	int ret;
+	int i, ret, ms;
 
 	if (s->usb_down_ran)
 		return false;
@@ -1759,16 +1793,27 @@ static bool t8140_sleep_usb_down(struct t8140_sleep *s)
 	dev_info(s->dev, "debug link: cutting it for the sleep\n");
 	enable = readl(s->hpm_spmi + SPMI_NOTIFY_ENABLE);
 	writel(enable & ~SPMI_NOTIFY_REPLY, s->hpm_spmi + SPMI_NOTIFY_ENABLE);
-	ret = t8140_hpm_cmd(s, SPMI_OP_WAKEUP, 0, NULL, NULL);
-	if (!ret)
-		ret = t8140_hpm_select(s, HPM_PAGE_CMD1);
-	if (!ret)
-		ret = t8140_hpm_cmd(s, SPMI_OP_EXT_WRITE(4), HPM_REG_DATA, &cmd, NULL);
-	if (!ret) {
+	/* A reset that the partner doesn't come back from is done again. */
+	for (i = 1; i <= HPM_RESET_TRIES; i++) {
+		ret = t8140_hpm_cmd(s, SPMI_OP_WAKEUP, 0, NULL, NULL);
+		if (!ret)
+			ret = t8140_hpm_select(s, HPM_PAGE_CMD1);
+		if (!ret)
+			ret = t8140_hpm_cmd(s, SPMI_OP_EXT_WRITE(4), HPM_REG_DATA, &cmd, NULL);
+		if (ret)
+			break;
 		WRITE_ONCE(t8140_debug_link_cut, true);
 		mdelay(HPM_RESET_MS);
-		ret = t8140_hpm_sleep(s);
+		ms = t8140_hpm_wait_contract(s);
+		if (ms >= 0) {
+			dev_info(s->dev, "debug link: contract back %d ms after reset %d\n",
+				 HPM_RESET_MS + ms, i);
+			break;
+		}
+		dev_warn(s->dev, "debug link: no contract with the partner after reset %d\n", i);
 	}
+	if (!ret)
+		ret = t8140_hpm_sleep(s);
 	writel(1, s->hpm_spmi + SPMI_NOTIFY_ACK);
 	writel(enable, s->hpm_spmi + SPMI_NOTIFY_ENABLE);
 	s->usb_down_ret = ret;

drivers/usb/typec/tipd/core.c

@@ -9,6 +9,7 @@
 #include <kunit/static_stub.h>
 #include <kunit/visibility.h>
 #include <linux/i2c.h>
+#include <linux/iopoll.h>
 #include <linux/acpi.h>
 #include <linux/gpio/consumer.h>
 #include <linux/of.h>
@@ -1576,9 +1577,10 @@ static void cd321x_prepare(struct tps6598x *tps)
 		cd321x_pm_prepare(&cd321x->pm);
 		cd321x->state_valid = false;
 	}
-	/* Gate scheduling first; neither worker may be drained under its mutex. */
+	/* Gate scheduling first; no worker may be drained under its mutex. */
 	cancel_delayed_work_sync(&cd321x->resume_work);
 	cancel_delayed_work_sync(&cd321x->update_work);
+	cancel_delayed_work_sync(&cd321x->rearm_work);
 
 	/* As macOS does a second or so before the commit, measured on the same controller. */
 	if (pm_suspend_via_firmware() && device_property_read_bool(tps->dev, "apple,sleep-ssps")) {
@@ -1593,37 +1595,114 @@ static void cd321x_prepare(struct tps6598x *tps)
 	}
 }
 
+/* A try every 250 ms for 10 s */
+#define CD321X_REARM_TRIES	40
+#define CD321X_REARM_RETRY_MS	250
+
+static int cd321x_debug_cmd(struct tps6598x *tps, const char *cmd, size_t len, const u8 *in)
+{
+	u8 out = 0;
+	int ret;
+
+	ret = tps6598x_exec_cmd(tps, cmd, len, in, 1, &out);
+	return ret ?: (out & 0xf ? -EIO : 0);
+}
+
+/* The firmware rejects a command for the mode it is switching to until it has switched. */
+static int cd321x_wait_dbma(struct tps6598x *tps, bool on)
+{
+	u32 mode;
+	int ret, err;
+
+	err = read_poll_timeout(tps6598x_read32, ret,
+				ret || (!memcmp(&mode, modes[TPS_MODE_DBMA], 4) == on),
+				1000, 100000, false, tps, TPS_REG_MODE, &mode);
+	return ret ?: err;
+}
+
 /*
- * A sleep cut this port's debug link. Rearm it the way a boot loader does: unlock with
- * the board's key, debug mode on, enable the debug VDM, debug mode off, lock.
+ * The boot loader's sequence: unlock with the board's key, debug mode on, enable the debug
+ * VDM, debug mode off, lock. Debug mode is left and the lock set again after a failure too.
  */
-static void cd321x_rearm_debug_link(struct tps6598x *tps)
+static int cd321x_arm_debug_link(struct tps6598x *tps, const char *key, const char **step)
 {
 	static const u8 vdm[] = { 0x06, 0x46, 0x82, 0x01 };
 	static const u8 no_key[4];
-	const char *key;
-	u8 on = 1, off = 0, out = 0;
+	u8 on = 1, off = 0;
+	int ret, err;
+
+	*step = "unlock";
+	ret = cd321x_debug_cmd(tps, "LOCK", sizeof(no_key), (const u8 *)key);
+	if (ret)
+		return ret;
+
+	*step = "debug mode";
+	ret = cd321x_debug_cmd(tps, "DBMa", 1, &on);
+	if (!ret)
+		ret = cd321x_wait_dbma(tps, true);
+	if (!ret) {
+		*step = "debug VDM";
+		ret = cd321x_debug_cmd(tps, "DVEn", sizeof(vdm), vdm);
+	}
+
+	err = cd321x_debug_cmd(tps, "DBMa", 1, &off);
+	if (!err)
+		err = cd321x_wait_dbma(tps, false);
+	if (err && !ret) {
+		*step = "leaving debug mode";
+		ret = err;
+	}
+	err = cd321x_debug_cmd(tps, "LOCK", sizeof(no_key), no_key);
+	if (err && !ret) {
+		*step = "lock";
+		ret = err;
+	}
+	return ret;
+}
+
+/*
+ * A sleep cut this port's debug link. Right after the wake the controller can reject the
+ * sequence, so a failed try is repeated until the link is back.
+ */
+static void cd321x_rearm_debug_link(struct cd321x *cd321x)
+{
+	struct tps6598x *tps = &cd321x->tps;
+	const char *key, *step;
 	int ret;
 
+	lockdep_assert_held(&tps->lock);
 	if (device_property_read_string(tps->dev, "apple,debug-lock-key", &key) ||
-	    strlen(key) != sizeof(no_key) || !apple_sleep_debug_link_was_cut(tps->dev))
+	    strlen(key) != 4) {
+		cd321x->rearm_pending = false;
 		return;
+	}
 
-	ret = tps6598x_exec_cmd(tps, "LOCK", sizeof(no_key), (const u8 *)key, 1, &out);
-	if (!ret && !(out & 0xf))
-		ret = tps6598x_exec_cmd(tps, "DBMa", 1, &on, 1, &out);
-	if (!ret && !(out & 0xf))
-		ret = tps6598x_exec_cmd(tps, "DVEn", sizeof(vdm), vdm, 1, &out);
-	if (!ret && (out & 0xf))
-		ret = -EIO;
-	if (!ret)
-		ret = tps6598x_exec_cmd(tps, "DBMa", 1, &off, 0, NULL);
-	if (!ret)
-		ret = tps6598x_exec_cmd(tps, "LOCK", sizeof(no_key), no_key, 0, NULL);
-	if (ret)
-		dev_warn(tps->dev, "debug link not rearmed: %d\n", ret);
-	else
-		dev_dbg(tps->dev, "debug link rearmed\n");
+	ret = cd321x_arm_debug_link(tps, key, &step);
+	cd321x->rearm_tries++;
+	if (!ret) {
+		if (cd321x->rearm_tries > 1)
+			dev_info(tps->dev, "debug link rearmed, try %u\n", cd321x->rearm_tries);
+		cd321x->rearm_pending = false;
+		return;
+	}
+	if (cd321x->rearm_tries < CD321X_REARM_TRIES) {
+		mod_delayed_work(system_freezable_wq, &cd321x->rearm_work,
+				 msecs_to_jiffies(CD321X_REARM_RETRY_MS));
+		return;
+	}
+	dev_warn(tps->dev, "debug link not rearmed after %u tries: %s: %d\n",
+		 cd321x->rearm_tries, step, ret);
+	cd321x->rearm_pending = false;
+}
+
+static void cd321x_rearm_work(struct work_struct *work)
+{
+	struct cd321x *cd321x = container_of(to_delayed_work(work), struct cd321x, rearm_work);
+
+	guard(mutex)(&cd321x->tps.lock);
+	if (cd321x->rearm_pending && cd321x->pm.phase != CD321X_PM_PREPARED &&
+	    cd321x->pm.phase != CD321X_PM_REMOVED)
+		cd321x_rearm_debug_link(cd321x);
 }
 
 static void cd321x_complete(struct tps6598x *tps)
@@ -1641,7 +1720,13 @@ static void cd321x_complete(struct tps6598x *tps)
 	}
 	if (pm_suspend_via_firmware() && device_property_read_bool(tps->dev, "apple,sleep-ssps"))
 		tps->data->switch_power_state(tps, TPS_SYSTEM_POWER_STATE_S0);
-	cd321x_rearm_debug_link(tps);
+	/* A link still down from an earlier sleep isn't cut again, so it stays pending. */
+	if (apple_sleep_debug_link_was_cut(tps->dev))
+		cd321x->rearm_pending = true;
+	if (cd321x->rearm_pending) {
+		cd321x->rearm_tries = 0;
+		cd321x_rearm_debug_link(cd321x);
+	}
 	if (cd321x_pm_resume(&cd321x->pm))
 		mod_delayed_work(system_freezable_wq, &cd321x->resume_work, 0);
 }
@@ -2266,6 +2351,7 @@ VISIBLE_IF_KUNIT void cd321x_stop_work(struct cd321x *cd321x)
 	}
 	cancel_delayed_work_sync(&cd321x->resume_work);
 	cancel_delayed_work_sync(&cd321x->update_work);
+	cancel_delayed_work_sync(&cd321x->rearm_work);
 }
 EXPORT_SYMBOL_IF_KUNIT(cd321x_stop_work);
 
@@ -2382,6 +2468,7 @@ VISIBLE_IF_KUNIT void cd321x_init_work(struct cd321x *cd321x)
 {
 	INIT_DELAYED_WORK(&cd321x->update_work, cd321x_update_work);
 	INIT_DELAYED_WORK(&cd321x->resume_work, cd321x_resume_work);
+	INIT_DELAYED_WORK(&cd321x->rearm_work, cd321x_rearm_work);
 	INIT_WORK(&cd321x->tbt_attach_work, cd321x_tbt_attach_work);
 	atomic_set(&cd321x->link_event, 0);
 	atomic_set(&cd321x->ready_event, 0);

drivers/usb/typec/tipd/tps6598x.h

@@ -470,12 +470,16 @@ struct cd321x {
 	bool display_route_active;
 	/* The sleep's debug link cut reset this controller: its setup is expected. */
 	bool reset_by_cut;
+	/* The debug link is still to be rearmed after a cut, and the tries so far */
+	bool rearm_pending;
+	unsigned int rearm_tries;
 	bool dp_sink_reported;
 	bool dp_irq_hpd;
 
 	struct cd321x_status update_status;
 	struct delayed_work update_work;
 	struct delayed_work resume_work;
+	struct delayed_work rearm_work;
 	struct cd321x_pm_state pm;
 	struct usb_pd_identity cur_partner_identity;
 	bool cur_partner_is_pd;

tools/testing/selftests/drivers/gpu/apple-dcp/pm.py

@@ -497,7 +497,7 @@ def function(text, name):
     mocks = r'''
 struct cd321x {
  struct tps6598x tps;
- struct work update_work,resume_work;
+ struct work update_work,resume_work,rearm_work;
  int link_event,ready_event,restore_event;
  struct cd321x_pm_state pm;
  void *mux,*tbt_switch,*port_altmode_dp,*port_altmode_tbt;