diff mbox series

[3/7] usb: typec: ucsi: make ACK_CC_CI rules more obvious

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

Commit Message

Dmitry Baryshkov March 13, 2024, 3:54 a.m. UTC
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(-)

Comments

Heikki Krogerus March 18, 2024, 10:48 a.m. UTC | #1
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 mbox series

Patch

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);
 	}