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

[AceLan Kao]

A gentle ping

AceLan Kao <acelan.kao@xxxxxxxxxxxxx> 於 2026年9月1日週二 上午10:27寫道：
>
> From: "Chia-Lin Kao (AceLan)" <acelan.kao@xxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx> # 6.14+
> Signed-off-by: Chia-Lin Kao (AceLan) <acelan.kao@xxxxxxxxxxxxx>
>
> ---
> 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@xxxxxxxxxxxxxxx> # 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
>


-- 
Chia-Lin Kao(AceLan)
http://blog.acelan.idv.tw/
E-Mail: acelan.kaoATcanonical.com (s/AT/@/)