[PATCH 3/4] usb: typec: ucsi: Rework ppm_lock handling - Linux-stable-mirror

9 Aug 2020

The ppm_lock really only needs to be hold during 2 functions:
ucsi_reset_ppm() and ucsi_run_command().
Push the taking of the lock down into these 2 functions, renaming
ucsi_run_command() to ucsi_send_command() which was an existing
wrapper already taking the lock for its callers.
This simplifies things for the callers and removes the difference
between ucsi_send_command() and ucsi_run_command() which has led
to various locking bugs in the past.
Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede hdegoede@redhat.com
---
 drivers/usb/typec/ucsi/ucsi.c | 56 ++++++++++++++---------------------
 1 file changed, 22 insertions(+), 34 deletions(-)

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index 2f586d6c54f4..182e34aa65ed 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -146,42 +146,33 @@ static int ucsi_exec_command(struct ucsi *ucsi, u64 cmd)
    return UCSI_CCI_LENGTH(cci);
 }
-static int ucsi_run_command(struct ucsi *ucsi, u64 command,
-			    void *data, size_t size)
+int ucsi_send_command(struct ucsi *ucsi, u64 command,
+		      void *data, size_t size)
 {
    u8 length;
    int ret;
-	WARN_ON(!mutex_is_locked(&ucsi->ppm_lock));
+	mutex_lock(&ucsi->ppm_lock);
ret = ucsi_exec_command(ucsi, command);
    if (ret < 0)
-		return ret;
+		goto out;
length = ret;
if (data) {
    	ret = ucsi->ops->read(ucsi, UCSI_MESSAGE_IN, data, size);
    	if (ret)
-			return ret;
+			goto out;
    }
ret = ucsi_acknowledge_command(ucsi);
    if (ret)
-		return ret;
-
-	return length;
-}
+		goto out;
-int ucsi_send_command(struct ucsi *ucsi, u64 command,
-		      void *retval, size_t size)
-{
-	int ret;
-
-	mutex_lock(&ucsi->ppm_lock);
-	ret = ucsi_run_command(ucsi, command, retval, size);
+	ret = length;
+out:
    mutex_unlock(&ucsi->ppm_lock);
-
    return ret;
 }
 EXPORT_SYMBOL_GPL(ucsi_send_command);
@@ -738,20 +729,24 @@ static int ucsi_reset_ppm(struct ucsi *ucsi)
    u32 cci;
    int ret;
+	mutex_lock(&ucsi->ppm_lock);
+
    ret = ucsi->ops->async_write(ucsi, UCSI_CONTROL, &command,
    			     sizeof(command));
    if (ret < 0)
-		return ret;
+		goto out;
tmo = jiffies + msecs_to_jiffies(UCSI_TIMEOUT_MS);
do {
-		if (time_is_before_jiffies(tmo))
-			return -ETIMEDOUT;
+		if (time_is_before_jiffies(tmo)) {
+			ret = -ETIMEDOUT;
+			goto out;
+		}
ret = ucsi->ops->read(ucsi, UCSI_CCI, &cci, sizeof(cci));
    	if (ret)
-			return ret;
+			goto out;
/* If the PPM is still doing something else, reset it again. */
    	if (cci & ~UCSI_CCI_RESET_COMPLETE) {
@@ -759,13 +754,15 @@ static int ucsi_reset_ppm(struct ucsi *ucsi)
    					     &command,
    					     sizeof(command));
    		if (ret < 0)
-				return ret;
+				goto out;
    	}
msleep(20);
    } while (!(cci & UCSI_CCI_RESET_COMPLETE));
-	return 0;
+out:
+	mutex_unlock(&ucsi->ppm_lock);
+	return ret;
 }
static int ucsi_role_cmd(struct ucsi_connector *con, u64 command)
@@ -777,9 +774,7 @@ static int ucsi_role_cmd(struct ucsi_connector *con, u64 command)
    	u64 c;
/* PPM most likely stopped responding. Resetting everything. */
-		mutex_lock(&con->ucsi->ppm_lock);
    	ucsi_reset_ppm(con->ucsi);
-		mutex_unlock(&con->ucsi->ppm_lock);
c = UCSI_SET_NOTIFICATION_ENABLE | con->ucsi->ntfy;
    	ucsi_send_command(con->ucsi, c, NULL, 0);
@@ -1010,8 +1005,6 @@ int ucsi_init(struct ucsi *ucsi)
    int ret;
    int i;
-	mutex_lock(&ucsi->ppm_lock);
-
    /* Reset the PPM */
    ret = ucsi_reset_ppm(ucsi);
    if (ret) {
@@ -1022,13 +1015,13 @@ int ucsi_init(struct ucsi *ucsi)
    /* Enable basic notifications */
    ucsi->ntfy = UCSI_ENABLE_NTFY_CMD_COMPLETE | UCSI_ENABLE_NTFY_ERROR;
    command = UCSI_SET_NOTIFICATION_ENABLE | ucsi->ntfy;
-	ret = ucsi_run_command(ucsi, command, NULL, 0);
+	ret = ucsi_send_command(ucsi, command, NULL, 0);
    if (ret < 0)
    	goto err_reset;
/* Get PPM capabilities */
    command = UCSI_GET_CAPABILITY;
-	ret = ucsi_run_command(ucsi, command, &ucsi->cap, sizeof(ucsi->cap));
+	ret = ucsi_send_command(ucsi, command, &ucsi->cap, sizeof(ucsi->cap));
    if (ret < 0)
    	goto err_reset;
@@ -1045,8 +1038,6 @@ int ucsi_init(struct ucsi *ucsi)
    	goto err_reset;
    }
-	mutex_unlock(&ucsi->ppm_lock);
-
    /* Register all connectors */
    for (i = 0; i < ucsi->cap.num_connectors; i++) {
    	ret = ucsi_register_port(ucsi, i);
@@ -1072,12 +1063,9 @@ int ucsi_init(struct ucsi *ucsi)
    	con->port = NULL;
    }
-	mutex_lock(&ucsi->ppm_lock);
 err_reset:
    ucsi_reset_ppm(ucsi);
 err:
-	mutex_unlock(&ucsi->ppm_lock);
-
    return ret;
 }
 EXPORT_SYMBOL_GPL(ucsi_init);
-- 
2.26.2


    