It is pretty easy to miss a call to usb_acknowledge_command() in
the error handling inside ucsi_exec_command(). For example
UCSI_CCI_ERROR had this call hidden inside ucsi_read_error().
Move this call and add a comment to make the rules regarding
usb_acknowledge_command() calls more obvious.
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---
drivers/usb/typec/ucsi/ucsi.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index bde4f03b9aa2..05a44e346e85 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -92,11 +92,6 @@ static int ucsi_read_error(struct ucsi *ucsi)
u16 error;
int ret;
- /* Acknowledge the command that failed */
- ret = ucsi_acknowledge_command(ucsi);
- if (ret)
- return ret;
-
ret = ucsi_exec_command(ucsi, UCSI_GET_ERROR_STATUS);
if (ret < 0)
return ret;
@@ -167,14 +162,27 @@ static int ucsi_exec_command(struct ucsi *ucsi, u64 cmd)
if (!(cci & UCSI_CCI_COMMAND_COMPLETE))
return -EIO;
+ /*
+ * All error cases below must acknowledge the command completion,
+ * otherwise PPM will be stuck and won't process commands anymore.
+ *
+ * In non-error case the command is acknowledged after reading Data
+ * from the controller.
+ */
+
if (cci & UCSI_CCI_NOT_SUPPORTED) {
ret = ucsi_acknowledge_command(ucsi);
return ret ? ret : -EOPNOTSUPP;
}
if (cci & UCSI_CCI_ERROR) {
+ ret = ucsi_acknowledge_command(ucsi);
+ if (ret)
+ return ret;
+
if (cmd == UCSI_GET_ERROR_STATUS)
return -EIO;
+
return ucsi_read_error(ucsi);
}
--
2.39.2
