[PATCH] usb: typec: ucsi: retry init only while the PPM is not ready

[Jacob Riff <jacob@riff.dk>]

Commit 254261381538 ("usb: typec: ucsi: retry init when the PPM
answers early commands incorrectly") makes ucsi_init_work() retry
ucsi_init() on -ENODEV and -EINVAL, a hundred times over ten seconds,
to cover a PPM that is not ready to answer commands correctly for a
short window during boot.

It catches more than that. ucsi_register_port() also fails with
-ENODEV when no USB role switch is available, and logs that at error
level, so every retry runs it again and logs it again. Where a machine
without a usable role switch used to log

  ucsi_acpi USBC000:00: error -ENODEV: con1: failed to get usb role switch
  ucsi_acpi USBC000:00: error -ENODEV: PPM init failed

it now logs a hundred and one copies of the first line, followed by
"PPM init failed, stop trying", on every boot, and the retries can
never succeed there.

Have ucsi_init() report the two not-ready cases as -EAGAIN instead:
GET_CAPABILITY reporting zero connectors, and the PPM rejecting a
standard command (-EINVAL from ucsi_read_error(), the only -EINVAL
expected once the PPM reset has succeeded). Retry only -EAGAIN and, as
before that commit, -EPROBE_DEFER. Every other error is reported once
and init stops, as it did before.

Also give -EAGAIN its own budget. The hundred retries over ten seconds
belong to the USB role switch wait. On the machine the retry was
written for, every observed failure recovered on the first or second
retry, so ten retries over one second is plenty. A PPM that
permanently rejects a command now logs "possible UCSI driver bug" up
to eleven times in that second, on hardware where init already failed
before.

Fixes: 254261381538 ("usb: typec: ucsi: retry init when the PPM answers early commands incorrectly")
Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Closes: https://lore.kernel.org/r/CAHk-=wjGk9rBTphk1OAwcqGAE1__KJgMr3B-8X4hKFPX8Wm=BQ@mail.gmail.com
Signed-off-by: Jacob Riff <jacob@riff.dk>
---
Tested on the machine from the Fixes commit (Lenovo ThinkPad X1 Carbon
Gen 14, this patch applied to the 7.2.6 driver): 6 boots, 3 hit the
race, all 3 recovered through the -EAGAIN path (two on the first retry,
one on the second), none ended with UCSI unusable. The single-report
path for a permanent -ENODEV from ucsi_register_port() is verified by
inspection only; no machine here has that configuration.

 drivers/usb/typec/ucsi/ucsi.c | 37 +++++++++++++++++++++++++----------
 drivers/usb/typec/ucsi/ucsi.h |  1 +
 2 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index 2f9adf4..9e453d2 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -2106,7 +2106,7 @@ static int ucsi_init(struct ucsi *ucsi)
 		goto err_reset;
 
 	if (!ucsi->cap.num_connectors) {
-		ret = -ENODEV;
+		ret = -EAGAIN;
 		goto err_reset;
 	}
 	/* Check if reserved bit set. This is out of spec but happens in buggy FW */
@@ -2172,6 +2172,16 @@ static int ucsi_init(struct ucsi *ucsi)
 err_reset:
 	memset(&ucsi->cap, 0, sizeof(ucsi->cap));
 	ucsi_reset_ppm(ucsi);
+
+	/*
+	 * The only -EINVAL expected on this path is ucsi_read_error()'s:
+	 * the PPM rejected a standard command. Some PPMs do that, or
+	 * report zero connectors above, for a short while after boot
+	 * before they are ready. Report both as -EAGAIN so that
+	 * ucsi_init_work() can retry them, and only them.
+	 */
+	if (ret == -EINVAL)
+		ret = -EAGAIN;
 err:
 	return ret;
 }
@@ -2249,16 +2259,23 @@ static void ucsi_init_work(struct work_struct *work)
 	}
 
 	/*
-	 * On some platforms the PPM is not ready to answer commands
-	 * correctly for a short window during boot: standard commands are
-	 * rejected or GET_CAPABILITY reports zero connectors, seen as
-	 * -EINVAL or -ENODEV from ucsi_init(), and a retry moments later
-	 * succeeds (observed on Lenovo ThinkPad X1 Carbon Gen 14, where
-	 * this affects roughly half of all boots). Retry those like the
-	 * USB role switch wait instead of giving up on the first attempt.
+	 * -EPROBE_DEFER: the USB role switch is not registered yet. Wait
+	 * up to ten seconds for it.
+	 *
+	 * -EAGAIN: the PPM is not ready to answer commands correctly yet.
+	 * On some platforms that lasts for a short window during boot
+	 * (observed on Lenovo ThinkPad X1 Carbon Gen 14, where it affects
+	 * roughly half of all boots and a retry moments later succeeds).
+	 * One second is plenty for that.
+	 *
+	 * Anything else is not going to change with a retry, so report it
+	 * once and stop.
 	 */
-	if (ret == -EPROBE_DEFER || ret == -ENODEV || ret == -EINVAL) {
-		if (ucsi->work_count++ < UCSI_ROLE_SWITCH_WAIT_COUNT) {
+	if (ret == -EPROBE_DEFER || ret == -EAGAIN) {
+		int limit = ret == -EPROBE_DEFER ? UCSI_ROLE_SWITCH_WAIT_COUNT :
+						 UCSI_PPM_READY_WAIT_COUNT;
+
+		if (ucsi->work_count++ < limit) {
 			dev_dbg(ucsi->dev, "PPM init failed (%pe), retrying\n",
 				ERR_PTR(ret));
 			queue_delayed_work(system_dfl_long_wq, &ucsi->work,
diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
index 1996f04..140d34e 100644
--- a/drivers/usb/typec/ucsi/ucsi.h
+++ b/drivers/usb/typec/ucsi/ucsi.h
@@ -491,6 +491,7 @@ struct ucsi {
 #define UCSI_ROLE_SWITCH_RETRY_PER_HZ	10
 #define UCSI_ROLE_SWITCH_INTERVAL	(HZ / UCSI_ROLE_SWITCH_RETRY_PER_HZ)
 #define UCSI_ROLE_SWITCH_WAIT_COUNT	(10 * UCSI_ROLE_SWITCH_RETRY_PER_HZ)
+#define UCSI_PPM_READY_WAIT_COUNT	(1 * UCSI_ROLE_SWITCH_RETRY_PER_HZ)
 
 	/* PPM Communication lock */
 	struct mutex ppm_lock;

base-commit: e971f4f365bad51d01bc90db55b56fdb3124f963
-- 
2.55.0