Skip to content

Commit

Permalink
usb: ucsi: stm32: fix command completion handling
Browse files Browse the repository at this point in the history
commit 8e1ec11 upstream.

Sometimes errors are seen, when doing DR swap, like:
[   24.672481] ucsi-stm32g0-i2c 0-0035: UCSI_GET_PDOS failed (-5)
[   24.720188] ucsi-stm32g0-i2c 0-0035: ucsi_handle_connector_change:
 GET_CONNECTOR_STATUS failed (-5)

There may be some race, which lead to read CCI, before the command complete
flag is set, hence returning -EIO. Similar fix has been done also in
ucsi_acpi [1].

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.

To fix this add the ACK_PENDING bit for ucsi_stm32g0 and only complete
commands if the completion bit matches.

[1] https://lore.kernel.org/lkml/20240121204123.275441-3-lk@c--e.de/

Fixes: 72849d4 ("usb: typec: ucsi: stm32g0: add support for stm32g0 controller")
Signed-off-by: Fabrice Gasnier <fabrice.gasnier@foss.st.com>
Link: https://lore.kernel.org/stable/20240612124656.2305603-1-fabrice.gasnier%40foss.st.com
Cc: stable <stable@kernel.org>
Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Link: https://lore.kernel.org/r/20240612124656.2305603-1-fabrice.gasnier@foss.st.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Fabrice Gasnier authored and gregkh committed Jul 5, 2024
1 parent a11b716 commit a47407a
Showing 1 changed file with 15 additions and 4 deletions.
19 changes: 15 additions & 4 deletions drivers/usb/typec/ucsi/ucsi_stm32g0.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ struct ucsi_stm32g0 {
struct completion complete;
struct device *dev;
unsigned long flags;
#define ACK_PENDING 2
const char *fw_name;
struct ucsi *ucsi;
bool suspended;
Expand Down Expand Up @@ -395,19 +396,28 @@ static int ucsi_stm32g0_sync_write(struct ucsi *ucsi, unsigned int offset, const
size_t len)
{
struct ucsi_stm32g0 *g0 = ucsi_get_drvdata(ucsi);
bool ack = UCSI_COMMAND(*(u64 *)val) == UCSI_ACK_CC_CI;
int ret;

set_bit(COMMAND_PENDING, &g0->flags);
if (ack)
set_bit(ACK_PENDING, &g0->flags);
else
set_bit(COMMAND_PENDING, &g0->flags);

ret = ucsi_stm32g0_async_write(ucsi, offset, val, len);
if (ret)
goto out_clear_bit;

if (!wait_for_completion_timeout(&g0->complete, msecs_to_jiffies(5000)))
ret = -ETIMEDOUT;
else
return 0;

out_clear_bit:
clear_bit(COMMAND_PENDING, &g0->flags);
if (ack)
clear_bit(ACK_PENDING, &g0->flags);
else
clear_bit(COMMAND_PENDING, &g0->flags);

return ret;
}
Expand All @@ -428,8 +438,9 @@ static irqreturn_t ucsi_stm32g0_irq_handler(int irq, void *data)
if (UCSI_CCI_CONNECTOR(cci))
ucsi_connector_change(g0->ucsi, UCSI_CCI_CONNECTOR(cci));

if (test_bit(COMMAND_PENDING, &g0->flags) &&
cci & (UCSI_CCI_ACK_COMPLETE | UCSI_CCI_COMMAND_COMPLETE))
if (cci & UCSI_CCI_ACK_COMPLETE && test_and_clear_bit(ACK_PENDING, &g0->flags))
complete(&g0->complete);
if (cci & UCSI_CCI_COMMAND_COMPLETE && test_and_clear_bit(COMMAND_PENDING, &g0->flags))
complete(&g0->complete);

return IRQ_HANDLED;
Expand Down

0 comments on commit a47407a

Please sign in to comment.