@@ -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);
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(-)