| Message ID | 20240313-qcom-ucsi-fixes-v1-3-74d90cb48a00@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | usb: typec: ucsi: fix several issues manifesting on Qualcomm platforms | expand |
On Wed, Mar 13, 2024 at 05:54:13AM +0200, Dmitry Baryshkov wrote: > 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@linaro.org> Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com> > --- > 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
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); }
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@linaro.org> --- drivers/usb/typec/ucsi/ucsi.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)