On Sun, Jan 21, 2024 at 09:41:22PM +0100, Christian A. Ehrhardt wrote:
> In case of a spurious or otherwise delayed notification it is
> possible that CCI still reports the previous completion. The
> UCSI spec is aware of this and provides two completion bits in
> CCI, one for normal commands and one for acks. As acks and commands
> alternate the notification handler can determine if the completion
> bit is from the current command.
>
> The initial UCSI code correctly handled this but the distinction
> between the two completion bits was lost with the introduction of
> the new API.
>
> To fix this revive the ACK_PENDING bit for ucsi_acpi and only complete
> commands if the completion bit matches.
>
> Fixes: f56de278e8ec ("usb: typec: ucsi: acpi: Move to the new API")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Christian A. Ehrhardt <lk@xxxxxxx>
Acked-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> ---
> drivers/usb/typec/ucsi/ucsi_acpi.c | 17 +++++++++++++----
> 1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/typec/ucsi/ucsi_acpi.c b/drivers/usb/typec/ucsi/ucsi_acpi.c
> index 6bbf490ac401..fa222080887d 100644
> --- a/drivers/usb/typec/ucsi/ucsi_acpi.c
> +++ b/drivers/usb/typec/ucsi/ucsi_acpi.c
> @@ -73,9 +73,13 @@ static int ucsi_acpi_sync_write(struct ucsi *ucsi, unsigned int offset,
> const void *val, size_t val_len)
> {
> struct ucsi_acpi *ua = ucsi_get_drvdata(ucsi);
> + bool ack = UCSI_COMMAND(*(u64 *)val) == UCSI_ACK_CC_CI;
> int ret;
>
> - set_bit(COMMAND_PENDING, &ua->flags);
> + if (ack)
> + set_bit(ACK_PENDING, &ua->flags);
> + else
> + set_bit(COMMAND_PENDING, &ua->flags);
>
> ret = ucsi_acpi_async_write(ucsi, offset, val, val_len);
> if (ret)
> @@ -85,7 +89,10 @@ static int ucsi_acpi_sync_write(struct ucsi *ucsi, unsigned int offset,
> ret = -ETIMEDOUT;
>
> out_clear_bit:
> - clear_bit(COMMAND_PENDING, &ua->flags);
> + if (ack)
> + clear_bit(ACK_PENDING, &ua->flags);
> + else
> + clear_bit(COMMAND_PENDING, &ua->flags);
>
> return ret;
> }
> @@ -142,8 +149,10 @@ static void ucsi_acpi_notify(acpi_handle handle, u32 event, void *data)
> if (UCSI_CCI_CONNECTOR(cci))
> ucsi_connector_change(ua->ucsi, UCSI_CCI_CONNECTOR(cci));
>
> - if (test_bit(COMMAND_PENDING, &ua->flags) &&
> - cci & (UCSI_CCI_ACK_COMPLETE | UCSI_CCI_COMMAND_COMPLETE))
> + if (cci & UCSI_CCI_ACK_COMPLETE && test_bit(ACK_PENDING, &ua->flags))
> + complete(&ua->complete);
> + if (cci & UCSI_CCI_COMMAND_COMPLETE &&
> + test_bit(COMMAND_PENDING, &ua->flags))
> complete(&ua->complete);
> }
>
> --
> 2.40.1
--
heikki
