[PATCH v3] usb: typec: ucsi: recover from silent PPM completion in sync_control

AceLan Kao posted 1 patch 3 weeks, 4 days ago
drivers/usb/typec/ucsi/ucsi.c | 24 ++++++++++++++++++++++--
1 file changed, 22 insertions(+), 2 deletions(-)
[PATCH v3] usb: typec: ucsi: recover from silent PPM completion in sync_control
Posted by AceLan Kao 3 weeks, 4 days ago
From: "Chia-Lin Kao (AceLan)" <acelan.kao@canonical.com>

Some firmware completes UCSI commands and sets COMMAND_COMPLETE (or
ACK_COMPLETE for ACK_CC_CI) in CCI but never fires the ACPI notify that
would wake ucsi_sync_control_common().  The driver then times out after
5 seconds and returns -ETIMEDOUT, even though the EC finished the command
successfully.

Fix this by polling CCI once via poll_cci() on timeout.  If the relevant
completion bit is already set, the EC finished silently; fall through to
out_clear_bit so the normal read_cci()/read_message_in() path retrieves
the data and ucsi_run_command() issues ACK_CC_CI as usual.  Only return
-ETIMEDOUT when the EC has genuinely not completed the command.

Guard the poll_cci() call with a NULL check: if a backend does not
provide the op, skip the poll and keep reporting -ETIMEDOUT rather than
dereferencing a NULL function pointer.

Fixes: 584e8df58942 ("usb: typec: ucsi: extract common code for command handling")
Cc: <stable@vger.kernel.org> # 6.14+
Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@canonical.com>

---
v2 -> v3:
- rebase and resolve conflicts against v7.2
- Invert the post-timeout completion check so the empty "silent
  completion" branch falls straight through to out_clear_bit instead
  of being an empty if {} with the real logic in the else, as pointed
  out in Greg's review.
v1 -> v2:
- Add Cc: <stable@vger.kernel.org> # 6.14+ as flagged by Greg's patch bot:
  the Fixes: tag targets a commit already in released kernels, so the fix
  must be nominated for stable.  Scoped to 6.14+ because poll_cci only
  exists from that release (absent in 6.11-6.13).
- Guard the poll_cci() call with a NULL check to avoid dereferencing a
  NULL function pointer when a backend does not provide the op.
---
 drivers/usb/typec/ucsi/ucsi.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index bef3f9b71d718..0363ed127a567 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -92,8 +92,28 @@ int ucsi_sync_control_common(struct ucsi *ucsi, u64 command, u32 *cci,
 		goto out_clear_bit;
 
 	if (!wait_for_completion_timeout(&ucsi->complete,
-					 msecs_to_jiffies(UCSI_TIMEOUT_MS)))
-		ret = -ETIMEDOUT;
+					 msecs_to_jiffies(UCSI_TIMEOUT_MS))) {
+		u32 polled_cci = 0;
+
+		/*
+		 * Notification from EC did not arrive.  Poll once to check
+		 * whether the PPM actually finished without firing a notify.
+		 * If poll_cci() is missing or fails, polled_cci stays 0 and we
+		 * correctly report -ETIMEDOUT below.
+		 */
+		if (ucsi->ops->poll_cci)
+			ucsi->ops->poll_cci(ucsi, &polled_cci);
+
+		/*
+		 * If the relevant completion bit is not set, the EC has not
+		 * completed the command; report the timeout.  Otherwise fall
+		 * through to out_clear_bit, which reads CCI+data normally,
+		 * and ucsi_run_command() will issue ACK_CC_CI as usual.
+		 */
+		if (!((ack && (polled_cci & UCSI_CCI_ACK_COMPLETE)) ||
+		      (!ack && (polled_cci & UCSI_CCI_COMMAND_COMPLETE))))
+			ret = -ETIMEDOUT;
+	}
 
 out_clear_bit:
 	if (ack)
-- 
2.53.0