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@xxxxxxxxxx>
Reviewed-by: Heikki Krogerus <heikki.krogerus@xxxxxxxxxxxxxxx>
> ---
> 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
--
heikki
