Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

usb: typec: ucsi: ccg: Remove run_isr flag

The "run_isr" flag is used for preventing the driver from
calling the interrupt service routine in its runtime resume
callback when the driver is expecting completion to a
command, but what that basically does is that it hides the
real problem. The real problem is that the controller is
allowed to suspend in the middle of command execution.

As a more appropriate fix for the problem, using autosuspend
delay time that matches UCSI_TIMEOUT_MS (5s). That prevents
the controller from suspending while still in the middle of
executing a command.

This fixes a potential deadlock. Both ccg_read() and
ccg_write() are called with the mutex already taken at least
from ccg_send_command(). In ccg_read() and ccg_write, the
mutex is only acquired so that run_isr flag can be set.

Fixes: f0e4cd948b91 ("usb: typec: ucsi: ccg: add runtime pm workaround")
Cc: stable@vger.kernel.org
Signed-off-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Link: https://lore.kernel.org/r/20191004100219.71152-2-heikki.krogerus@linux.intel.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Heikki Krogerus and committed by
Greg Kroah-Hartman
8530e4e2 c9a2baa7

+4 -38
+4 -38
drivers/usb/typec/ucsi/ucsi_ccg.c
··· 195 195 196 196 /* fw build with vendor information */ 197 197 u16 fw_build; 198 - bool run_isr; /* flag to call ISR routine during resume */ 199 198 struct work_struct pm_work; 200 199 }; 201 200 ··· 222 223 /* check any max_read_len limitation on i2c adapter */ 223 224 if (quirks && quirks->max_read_len) 224 225 max_read_len = quirks->max_read_len; 225 - 226 - if (uc->fw_build == CCG_FW_BUILD_NVIDIA && 227 - uc->fw_version <= CCG_OLD_FW_VERSION) { 228 - mutex_lock(&uc->lock); 229 - /* 230 - * Do not schedule pm_work to run ISR in 231 - * ucsi_ccg_runtime_resume() after pm_runtime_get_sync() 232 - * since we are already in ISR path. 233 - */ 234 - uc->run_isr = false; 235 - mutex_unlock(&uc->lock); 236 - } 237 226 238 227 pm_runtime_get_sync(uc->dev); 239 228 while (rem_len > 0) { ··· 264 277 265 278 msgs[0].len = len + sizeof(rab); 266 279 msgs[0].buf = buf; 267 - 268 - if (uc->fw_build == CCG_FW_BUILD_NVIDIA && 269 - uc->fw_version <= CCG_OLD_FW_VERSION) { 270 - mutex_lock(&uc->lock); 271 - /* 272 - * Do not schedule pm_work to run ISR in 273 - * ucsi_ccg_runtime_resume() after pm_runtime_get_sync() 274 - * since we are already in ISR path. 275 - */ 276 - uc->run_isr = false; 277 - mutex_unlock(&uc->lock); 278 - } 279 280 280 281 pm_runtime_get_sync(uc->dev); 281 282 status = i2c_transfer(client->adapter, msgs, ARRAY_SIZE(msgs)); ··· 1105 1130 uc->ppm.sync = ucsi_ccg_sync; 1106 1131 uc->dev = dev; 1107 1132 uc->client = client; 1108 - uc->run_isr = true; 1109 1133 mutex_init(&uc->lock); 1110 1134 INIT_WORK(&uc->work, ccg_update_firmware); 1111 1135 INIT_WORK(&uc->pm_work, ccg_pm_workaround_work); ··· 1162 1188 1163 1189 pm_runtime_set_active(uc->dev); 1164 1190 pm_runtime_enable(uc->dev); 1191 + pm_runtime_use_autosuspend(uc->dev); 1192 + pm_runtime_set_autosuspend_delay(uc->dev, 5000); 1165 1193 pm_runtime_idle(uc->dev); 1166 1194 1167 1195 return 0; ··· 1205 1229 { 1206 1230 struct i2c_client *client = to_i2c_client(dev); 1207 1231 struct ucsi_ccg *uc = i2c_get_clientdata(client); 1208 - bool schedule = true; 1209 1232 1210 1233 /* 1211 1234 * Firmware version 3.1.10 or earlier, built for NVIDIA has known issue ··· 1212 1237 * Schedule a work to call ISR as a workaround. 1213 1238 */ 1214 1239 if (uc->fw_build == CCG_FW_BUILD_NVIDIA && 1215 - uc->fw_version <= CCG_OLD_FW_VERSION) { 1216 - mutex_lock(&uc->lock); 1217 - if (!uc->run_isr) { 1218 - uc->run_isr = true; 1219 - schedule = false; 1220 - } 1221 - mutex_unlock(&uc->lock); 1222 - 1223 - if (schedule) 1224 - schedule_work(&uc->pm_work); 1225 - } 1240 + uc->fw_version <= CCG_OLD_FW_VERSION) 1241 + schedule_work(&uc->pm_work); 1226 1242 1227 1243 return 0; 1228 1244 }