usb: typec: ucsi: Rework ppm_lock handling

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>
Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Reviewed-by: Guenter Roeck <linux@roeck-us.net>
Link: https://lore.kernel.org/r/20200809141904.4317-4-hdegoede@redhat.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Hans de Goede and committed by

Greg Kroah-Hartman 5 years ago 25794e30 7e90057f

+22 -34

1 changed file

expand all

drivers

usb

typec

ucsi

ucsi.c

+22 -34

drivers/usb/typec/ucsi/ucsi.c

··· 146 146 return UCSI_CCI_LENGTH(cci); 147 147 } 148 148 149 - static int ucsi_run_command(struct ucsi *ucsi, u64 command, 150 - void *data, size_t size) 149 + int ucsi_send_command(struct ucsi *ucsi, u64 command, 150 + void *data, size_t size) 151 151 { 152 152 u8 length; 153 153 int ret; 154 154 155 - WARN_ON(!mutex_is_locked(&ucsi->ppm_lock)); 155 + mutex_lock(&ucsi->ppm_lock); 156 156 157 157 ret = ucsi_exec_command(ucsi, command); 158 158 if (ret < 0) 159 - return ret; 159 + goto out; 160 160 161 161 length = ret; 162 162 163 163 if (data) { 164 164 ret = ucsi->ops->read(ucsi, UCSI_MESSAGE_IN, data, size); 165 165 if (ret) 166 - return ret; 166 + goto out; 167 167 } 168 168 169 169 ret = ucsi_acknowledge_command(ucsi); 170 170 if (ret) 171 - return ret; 171 + goto out; 172 172 173 - return length; 174 - } 175 - 176 - int ucsi_send_command(struct ucsi *ucsi, u64 command, 177 - void *retval, size_t size) 178 - { 179 - int ret; 180 - 181 - mutex_lock(&ucsi->ppm_lock); 182 - ret = ucsi_run_command(ucsi, command, retval, size); 173 + ret = length; 174 + out: 183 175 mutex_unlock(&ucsi->ppm_lock); 184 - 185 176 return ret; 186 177 } 187 178 EXPORT_SYMBOL_GPL(ucsi_send_command); ··· 729 738 u32 cci; 730 739 int ret; 731 740 741 + mutex_lock(&ucsi->ppm_lock); 742 + 732 743 ret = ucsi->ops->async_write(ucsi, UCSI_CONTROL, &command, 733 744 sizeof(command)); 734 745 if (ret < 0) 735 - return ret; 746 + goto out; 736 747 737 748 tmo = jiffies + msecs_to_jiffies(UCSI_TIMEOUT_MS); 738 749 739 750 do { 740 - if (time_is_before_jiffies(tmo)) 741 - return -ETIMEDOUT; 751 + if (time_is_before_jiffies(tmo)) { 752 + ret = -ETIMEDOUT; 753 + goto out; 754 + } 742 755 743 756 ret = ucsi->ops->read(ucsi, UCSI_CCI, &cci, sizeof(cci)); 744 757 if (ret) 745 - return ret; 758 + goto out; 746 759 747 760 /* If the PPM is still doing something else, reset it again. */ 748 761 if (cci & ~UCSI_CCI_RESET_COMPLETE) { ··· 754 759 &command, 755 760 sizeof(command)); 756 761 if (ret < 0) 757 - return ret; 762 + goto out; 758 763 } 759 764 760 765 msleep(20); 761 766 } while (!(cci & UCSI_CCI_RESET_COMPLETE)); 762 767 763 - return 0; 768 + out: 769 + mutex_unlock(&ucsi->ppm_lock); 770 + return ret; 764 771 } 765 772 766 773 static int ucsi_role_cmd(struct ucsi_connector *con, u64 command) ··· 774 777 u64 c; 775 778 776 779 /* PPM most likely stopped responding. Resetting everything. */ 777 - mutex_lock(&con->ucsi->ppm_lock); 778 780 ucsi_reset_ppm(con->ucsi); 779 - mutex_unlock(&con->ucsi->ppm_lock); 780 781 781 782 c = UCSI_SET_NOTIFICATION_ENABLE | con->ucsi->ntfy; 782 783 ucsi_send_command(con->ucsi, c, NULL, 0); ··· 1005 1010 int ret; 1006 1011 int i; 1007 1012 1008 - mutex_lock(&ucsi->ppm_lock); 1009 - 1010 1013 /* Reset the PPM */ 1011 1014 ret = ucsi_reset_ppm(ucsi); 1012 1015 if (ret) { ··· 1015 1022 /* Enable basic notifications */ 1016 1023 ucsi->ntfy = UCSI_ENABLE_NTFY_CMD_COMPLETE | UCSI_ENABLE_NTFY_ERROR; 1017 1024 command = UCSI_SET_NOTIFICATION_ENABLE | ucsi->ntfy; 1018 - ret = ucsi_run_command(ucsi, command, NULL, 0); 1025 + ret = ucsi_send_command(ucsi, command, NULL, 0); 1019 1026 if (ret < 0) 1020 1027 goto err_reset; 1021 1028 1022 1029 /* Get PPM capabilities */ 1023 1030 command = UCSI_GET_CAPABILITY; 1024 - ret = ucsi_run_command(ucsi, command, &ucsi->cap, sizeof(ucsi->cap)); 1031 + ret = ucsi_send_command(ucsi, command, &ucsi->cap, sizeof(ucsi->cap)); 1025 1032 if (ret < 0) 1026 1033 goto err_reset; 1027 1034 ··· 1037 1044 ret = -ENOMEM; 1038 1045 goto err_reset; 1039 1046 } 1040 - 1041 - mutex_unlock(&ucsi->ppm_lock); 1042 1047 1043 1048 /* Register all connectors */ 1044 1049 for (i = 0; i < ucsi->cap.num_connectors; i++) { ··· 1063 1072 con->port = NULL; 1064 1073 } 1065 1074 1066 - mutex_lock(&ucsi->ppm_lock); 1067 1075 err_reset: 1068 1076 ucsi_reset_ppm(ucsi); 1069 1077 err: 1070 - mutex_unlock(&ucsi->ppm_lock); 1071 - 1072 1078 return ret; 1073 1079 } 1074 1080

Configure Feed

Configure Feed