Re: [PATCH] usb: typec: ucsi: split connector lock classes

[Heikki Krogerus <heikki.krogerus@linux.intel.com>]

On Fri, May 15, 2026 at 03:00:30PM +0900, Sergey Senozhatsky wrote:
> Lockdep detects a possible recursive locking scenario during
> ucsi init:
> 
> [    5.418616] ============================================
> [    5.418634] WARNING: possible recursive locking detected
> [    5.418706] --------------------------------------------
> [    5.418725] kworker/4:1/82 is trying to acquire lock:
> [    5.418759] ffff888119a34648 (&con->lock){+.+.}-{3:3}, at: ucsi_init_work+0x1a78/0x2eb0 [typec_ucsi]
> [    5.418801]
>                but task is already holding lock:
> [    5.418835] ffff888119a34080 (&con->lock){+.+.}-{3:3}, at: ucsi_init_work+0x1a78/0x2eb0 [typec_ucsi]
> [    5.418884]
>                other info that might help us debug this:
> [    5.418904]  Possible unsafe locking scenario:
> 
> [    5.418937]        CPU0
> [    5.418956]        ----
> [    5.418991]   lock(&con->lock);
> [    5.419013]   lock(&con->lock);
> [    5.419033]
>                 *** DEADLOCK ***
> 
> [    5.419387] Call Trace:
> [    5.419406]  <TASK>
> [    5.419425]  dump_stack_lvl+0x61/0xa0
> [    5.419448]  print_deadlock_bug+0x4a6/0x650
> [    5.419483]  __lock_acquire+0x62b6/0x7f50
> [    5.419507]  lock_acquire+0x11b/0x390
> [    5.419654]  __mutex_lock+0xbc/0xcd0
> [    5.419741]  ucsi_init_work+0x1a78/0x2eb0
> [    5.419785]  ? worker_thread+0xf53/0x2bc0
> [    5.419819]  worker_thread+0xff4/0x2bc0
> [    5.419842]  kthread+0x2a7/0x330
> [    5.419863]  ? __pfx_worker_thread+0x10/0x10
> [    5.419896]  ? __pfx_kthread+0x10/0x10
> [    5.419916]  ret_from_fork+0x38/0x70
> [    5.419936]  ? __pfx_kthread+0x10/0x10
> [    5.419969]  ret_from_fork_asm+0x1b/0x30
> [    5.419991]  </TASK>
> [    5.420009] ---[ end trace 0000000000000000 ]---
> 
> The problem is that all connector locks belong to the same
> lockdep lock class, so the following loop:
> 
> 	for (i = 0; i < ucsi->cap.num_connectors; i++)
> 		ucsi_register_port(connector[i])
> 			mutex_lock(&connector[i]->lock)
> 
> looks like a recursive acquire of the same mutex.  Put each connector
> lock into a dedicated lock class so that lockdep doesn't see it as a
> possible recursion.
> 
> Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org>

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

> ---
>  drivers/usb/typec/ucsi/ucsi.c | 8 ++++++++
>  drivers/usb/typec/ucsi/ucsi.h | 1 +
>  2 files changed, 9 insertions(+)
> 
> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> index 5b7ad9e99cb9..43da7512dea0 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c
> @@ -1642,6 +1642,7 @@ static int ucsi_register_port(struct ucsi *ucsi, struct ucsi_connector *con)
>  	INIT_WORK(&con->work, ucsi_handle_connector_change);
>  	init_completion(&con->complete);
>  	mutex_init(&con->lock);
> +	lockdep_set_class(&con->lock, &con->lock_key);
>  	INIT_LIST_HEAD(&con->partner_tasks);
>  	con->ucsi = ucsi;
>  
> @@ -1887,6 +1888,9 @@ static int ucsi_init(struct ucsi *ucsi)
>  		goto err_reset;
>  	}
>  
> +	for (i = 0; i < ucsi->cap.num_connectors; i++)
> +		lockdep_register_key(&connector[i].lock_key);
> +
>  	/* Register all connectors */
>  	for (i = 0; i < ucsi->cap.num_connectors; i++) {
>  		connector[i].num = i + 1;
> @@ -1916,6 +1920,9 @@ static int ucsi_init(struct ucsi *ucsi)
>  	return 0;
>  
>  err_unregister:
> +	for (i = 0; i < ucsi->cap.num_connectors; i++)
> +		lockdep_unregister_key(&connector[i].lock_key);
> +
>  	for (con = connector; con->port; con++) {
>  		if (con->wq)
>  			destroy_workqueue(con->wq);
> @@ -2166,6 +2173,7 @@ void ucsi_unregister(struct ucsi *ucsi)
>  		usb_power_delivery_unregister(ucsi->connector[i].pd);
>  		ucsi->connector[i].pd = NULL;
>  		typec_unregister_port(ucsi->connector[i].port);
> +		lockdep_unregister_key(&ucsi->connector[i].lock_key);
>  	}
>  
>  	kfree(ucsi->connector);
> diff --git a/drivers/usb/typec/ucsi/ucsi.h b/drivers/usb/typec/ucsi/ucsi.h
> index cff9ddc2ae21..51f6c3c0d365 100644
> --- a/drivers/usb/typec/ucsi/ucsi.h
> +++ b/drivers/usb/typec/ucsi/ucsi.h
> @@ -517,6 +517,7 @@ struct ucsi_connector {
>  
>  	struct ucsi *ucsi;
>  	struct mutex lock; /* port lock */
> +	struct lock_class_key lock_key;
>  	struct work_struct work;
>  	struct completion complete;
>  	struct workqueue_struct *wq;
> -- 
> 2.54.0.563.g4f69b47b94-goog

-- 
heikki