From b1302e6a8305f4cda9f3fa2da55fcc3f31fd14de Mon Sep 17 00:00:00 2001 From: Wang Jie Date: Thu, 11 Nov 2021 10:19:47 +0800 Subject: [PATCH] usb: typec: tcpm: add pd handler lock Quickly plug and unplug the Type-C device with DP function test, there is a low probability of unplugging the Type-C dongle, DP work cannot acquire the mutex(port->lock), and it will always be stuck in the tcpm_unregister_altmodes() function during the rest port process, causing the Type-C device to be inserted again to fail to be detected or the reboot system to be stuck. The exception process is as follows: (1) Thread#1: unplug the Type-C device to trigger cc status changes interrupt, in the tcpm_pd_event_handler() function, first acquire the mutex(port->lock), then start state_machine work, and finally release the mutex(port->lock); tcpm_cc_change() -->tcpm_pd_event_handler() -->mutex_lock(&port->lock) /* step 1 */ -->_tcpm_cc_change(port, cc1, cc2) /* step 2 */ -->tcpm_set_state(port, SRC_UNATTACHED, 0) -->kthread_queue_work(port->wq, &port->state_machine) /* step 3 */ --> mutex_unlock(&port->lock) /* step 4 */ (2) Thread#2: before the execution of "step 2" is completed, DP work calls the callback function registered by the tcpm framework to initiate a VDM Message, such as Enter Mode Message, in the tcpm_queue_vdm_unlocked() acquires the mutex(port->lock) fails and goes to sleep; dp_altmode_work() --> typec_altmode_enter(dp->alt) --> tcpm_altmode_enter() --> tcpm_queue_vdm_unlocked() --> mutex_lock(&port->lock) /* dp work enter sleep */ (3) After step 4, tcpm_queue_vdm_unlocked() did not acquire for the mutex (port->lock), but was preempted by the state_machine work to enter the reset port process. In the reset port process, call the tcpm_unregister_altmodes() function to release altmode and cancel dp work. Because tcpm_queue_vdm_unlocked() did not acquire the mutex(port->lock), causing it to be stuck in cancel dp work. tcpm_state_machine_work() -->mutex_lock(&port->lock) -->tcpm_reset_port(port) -->tcpm_unregister_altmodes(port) ...... --> dp_altmode_remove() --> cancel_work_sync(&dp->work) /* always stuck in cancel dp work */ ...... Therefore, after adding a new mutex to wait for the tcpm_pd_event_handler() function to complete the processing, if the port is in the disconnect state, there is no need to acquire the mutex(port->lock) in the tcpm_queue_vdm_unlocked() function. Change-Id: I364a035568ddc35ef7242b42f6d6d0ee3f5586fd Signed-off-by: Wang Jie --- drivers/usb/typec/tcpm/tcpm.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c index 98258c947ddb..6fa80042cd05 100644 --- a/drivers/usb/typec/tcpm/tcpm.c +++ b/drivers/usb/typec/tcpm/tcpm.c @@ -360,6 +360,7 @@ struct tcpm_port { unsigned long delay_ms; spinlock_t pd_event_lock; + struct mutex pd_handler_lock; u32 pd_events; struct kthread_work event_work; @@ -1463,9 +1464,16 @@ static void tcpm_queue_vdm(struct tcpm_port *port, const u32 header, static void tcpm_queue_vdm_unlocked(struct tcpm_port *port, const u32 header, const u32 *data, int cnt) { + mutex_lock(&port->pd_handler_lock); + if (tcpm_port_is_disconnected(port)) + goto unlock; + mutex_lock(&port->lock); tcpm_queue_vdm(port, header, data, cnt); mutex_unlock(&port->lock); + +unlock: + mutex_unlock(&port->pd_handler_lock); } static void svdm_consume_identity(struct tcpm_port *port, const u32 *p, int cnt) @@ -5388,6 +5396,7 @@ static void tcpm_pd_event_handler(struct kthread_work *work) event_work); u32 events; + mutex_lock(&port->pd_handler_lock); mutex_lock(&port->lock); spin_lock(&port->pd_event_lock); @@ -5450,6 +5459,7 @@ static void tcpm_pd_event_handler(struct kthread_work *work) } spin_unlock(&port->pd_event_lock); mutex_unlock(&port->lock); + mutex_unlock(&port->pd_handler_lock); } void tcpm_cc_change(struct tcpm_port *port) @@ -6457,6 +6467,7 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc) mutex_init(&port->lock); mutex_init(&port->swap_lock); + mutex_init(&port->pd_handler_lock); port->wq = kthread_create_worker(0, dev_name(dev)); if (IS_ERR(port->wq))