diff mbox series

[1/4] usb: typec: ucsi: Fix AB BA lock inversion

Message ID 20200809141904.4317-2-hdegoede@redhat.com (mailing list archive)
State New, archived
Headers show
Series usb: typec: ucsi: Fix various locking issues | expand

Commit Message

Hans de Goede Aug. 9, 2020, 2:19 p.m. UTC 
Lockdep reports an AB BA lock inversion between ucsi_init() and
ucsi_handle_connector_change():

AB order:

1. ucsi_init takes ucsi->ppm_lock (it runs with that locked for the
   duration of the function)
2. usci_init eventually end up calling ucsi_register_displayport,
   which takes ucsi_connector->lock

BA order:

1. ucsi_handle_connector_change work is started, takes ucsi_connector->lock
2. ucsi_handle_connector_change calls ucsi_send_command which takes
   ucsi->ppm_lock

The ppm_lock really only needs to be hold during 2 functions:
ucsi_reset_ppm() and ucsi_run_command().

This commit fixes the AB BA lock inversion by making ucsi_init drop the
ucsi->ppm_lock before it starts registering ports; and replacing any
ucsi_run_command() calls after this point with ucsi_send_command()
(which is a wrapper around run_command taking the lock while handling
the command).

Some of the replacing of ucsi_run_command with ucsi_send_command
in the helpers used during port registration also fixes a number of
code paths after registration which call ucsi_run_command() without
holding the ppm_lock:
1. ucsi_altmode_update_active() call in ucsi/displayport.c
2. ucsi_register_altmodes() call from ucsi_handle_connector_change()
   (through ucsi_partner_change())

Cc: stable@vger.kernel.org
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/usb/typec/ucsi/ucsi.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

Comments

Sasha Levin Aug. 19, 2020, 11:56 p.m. UTC | #1 
Hi

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v5.8.1, v5.7.15, v5.4.58, v4.19.139, v4.14.193, v4.9.232, v4.4.232.

v5.8.1: Build OK!
v5.7.15: Build OK!
v5.4.58: Failed to apply! Possible dependencies:
    170a6726d0e2 ("usb: typec: ucsi: add support for separate DP altmode devices")
    2ede55468ca8 ("usb: typec: ucsi: Remove the old API")
    470ce43a1a81 ("usb: typec: ucsi: Remove struct ucsi_control")
    6df475f804e6 ("usb: typec: ucsi: Start using struct typec_operations")
    71a1fa0df2a3 ("usb: typec: ucsi: Store the notification mask")
    9521e47e9ab8 ("usb: typec: ucsi: Actually enable all the interface notifications")
    bdc62f2bae8f ("usb: typec: ucsi: Simplified registration and I/O API")
    e32fd989ac1c ("usb: typec: ucsi: ccg: Move to the new API")
    e716bb38edb4 ("usb: typec: ucsi: New error codes")

v4.19.139: Failed to apply! Possible dependencies:
    247c554a14aa ("usb: typec: ucsi: add support for Cypress CCGx")
    2ede55468ca8 ("usb: typec: ucsi: Remove the old API")
    470ce43a1a81 ("usb: typec: ucsi: Remove struct ucsi_control")
    5c9ae5a87573 ("usb: typec: ucsi: ccg: add firmware flashing support")
    5d438e200215 ("usb: typec: ucsi: ccg: add get_fw_info function")
    6df475f804e6 ("usb: typec: ucsi: Start using struct typec_operations")
    81534d5fa973 ("usb: typec: ucsi: Remove debug.h file")
    a94ecde41f7e ("usb: typec: ucsi: ccg: enable runtime pm support")
    ad74b8649bea ("usb: typec: ucsi: Preliminary support for alternate modes")
    af8622f6a585 ("usb: typec: ucsi: Support for DisplayPort alt mode")
    bdc62f2bae8f ("usb: typec: ucsi: Simplified registration and I/O API")
    f2372b87c386 ("usb: typec: ucsi: displayport: Fix for the mode entering routine")

v4.14.193: Failed to apply! Possible dependencies:
    0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
    3c4fb9f16921 ("usb: typec: wcove: start using tcpm for USB PD support")
    44262fad12a7 ("staging: typec: tcpm: Drop commented out code")
    4b4e02c83167 ("typec: tcpm: Move out of staging")
    70cd90be3300 ("staging: typec: pd: Document struct pd_message")
    76f0c53d08b9 ("usb: typec: fusb302: Move out of staging")
    81534d5fa973 ("usb: typec: ucsi: Remove debug.h file")
    956c36c297a2 ("USB: typec: add SPDX identifiers to some files")
    98076fa64a05 ("staging: typec: tcpm: Document data structures")
    ad74b8649bea ("usb: typec: ucsi: Preliminary support for alternate modes")
    cf6e06cddf29 ("usb: typec: Start using ERR_PTR")

v4.9.232: Failed to apply! Possible dependencies:
    0c744ea4f77d ("Linux 4.10-rc2")
    2bd6bf03f4c1 ("Linux 4.14-rc1")
    2ea659a9ef48 ("Linux 4.12-rc1")
    49def1853334 ("Linux 4.10-rc4")
    566cf877a1fc ("Linux 4.10-rc6")
    5771a8c08880 ("Linux v4.13-rc1")
    7089db84e356 ("Linux 4.10-rc8")
    7a308bb3016f ("Linux 4.10-rc5")
    7ce7d89f4883 ("Linux 4.10-rc1")
    81534d5fa973 ("usb: typec: ucsi: Remove debug.h file")
    a121103c9228 ("Linux 4.10-rc3")
    ad74b8649bea ("usb: typec: ucsi: Preliminary support for alternate modes")
    b24413180f56 ("License cleanup: add SPDX GPL-2.0 license identifier to files with no license")
    c1ae3cfa0e89 ("Linux 4.11-rc1")
    c470abd4fde4 ("Linux 4.10")
    d5adbfcd5f7b ("Linux 4.10-rc7")

v4.4.232: Failed to apply! Possible dependencies:
    1001354ca341 ("Linux 4.9-rc1")
    18558cae0272 ("Linux 4.5-rc4")
    1a695a905c18 ("Linux 4.7-rc1")
    29b4817d4018 ("Linux 4.8-rc1")
    2bd6bf03f4c1 ("Linux 4.14-rc1")
    2dcd0af568b0 ("Linux 4.6")
    2ea659a9ef48 ("Linux 4.12-rc1")
    36f90b0a2ddd ("Linux 4.5-rc2")
    388f7b1d6e8c ("Linux 4.5-rc3")
    5771a8c08880 ("Linux v4.13-rc1")
    7ce7d89f4883 ("Linux 4.10-rc1")
    81534d5fa973 ("usb: typec: ucsi: Remove debug.h file")
    81f70ba233d5 ("Linux 4.5-rc5")
    92e963f50fc7 ("Linux 4.5-rc1")
    ad74b8649bea ("usb: typec: ucsi: Preliminary support for alternate modes")
    b24413180f56 ("License cleanup: add SPDX GPL-2.0 license identifier to files with no license")
    b562e44f507e ("Linux 4.5")
    c1ae3cfa0e89 ("Linux 4.11-rc1")
    f55532a0c0b8 ("Linux 4.6-rc1")
    f6cede5b49e8 ("Linux 4.5-rc7")
    fc77dbd34c5c ("Linux 4.5-rc6")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?
Sasha Levin Aug. 26, 2020, 1:54 p.m. UTC | #2 
Hi

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v5.8.2, v5.7.16, v5.4.59, v4.19.140, v4.14.193, v4.9.232, v4.4.232.

v5.8.2: Build OK!
v5.7.16: Build OK!
v5.4.59: Failed to apply! Possible dependencies:
    170a6726d0e2 ("usb: typec: ucsi: add support for separate DP altmode devices")
    2ede55468ca8 ("usb: typec: ucsi: Remove the old API")
    470ce43a1a81 ("usb: typec: ucsi: Remove struct ucsi_control")
    6df475f804e6 ("usb: typec: ucsi: Start using struct typec_operations")
    71a1fa0df2a3 ("usb: typec: ucsi: Store the notification mask")
    9521e47e9ab8 ("usb: typec: ucsi: Actually enable all the interface notifications")
    bdc62f2bae8f ("usb: typec: ucsi: Simplified registration and I/O API")
    e32fd989ac1c ("usb: typec: ucsi: ccg: Move to the new API")
    e716bb38edb4 ("usb: typec: ucsi: New error codes")

v4.19.140: Failed to apply! Possible dependencies:
    247c554a14aa ("usb: typec: ucsi: add support for Cypress CCGx")
    2ede55468ca8 ("usb: typec: ucsi: Remove the old API")
    470ce43a1a81 ("usb: typec: ucsi: Remove struct ucsi_control")
    5c9ae5a87573 ("usb: typec: ucsi: ccg: add firmware flashing support")
    5d438e200215 ("usb: typec: ucsi: ccg: add get_fw_info function")
    6df475f804e6 ("usb: typec: ucsi: Start using struct typec_operations")
    81534d5fa973 ("usb: typec: ucsi: Remove debug.h file")
    a94ecde41f7e ("usb: typec: ucsi: ccg: enable runtime pm support")
    ad74b8649bea ("usb: typec: ucsi: Preliminary support for alternate modes")
    af8622f6a585 ("usb: typec: ucsi: Support for DisplayPort alt mode")
    bdc62f2bae8f ("usb: typec: ucsi: Simplified registration and I/O API")
    f2372b87c386 ("usb: typec: ucsi: displayport: Fix for the mode entering routine")

v4.14.193: Failed to apply! Possible dependencies:
    0a4c005bd171 ("usb: typec: driver for TI TPS6598x USB Power Delivery controllers")
    3c4fb9f16921 ("usb: typec: wcove: start using tcpm for USB PD support")
    44262fad12a7 ("staging: typec: tcpm: Drop commented out code")
    4b4e02c83167 ("typec: tcpm: Move out of staging")
    70cd90be3300 ("staging: typec: pd: Document struct pd_message")
    76f0c53d08b9 ("usb: typec: fusb302: Move out of staging")
    81534d5fa973 ("usb: typec: ucsi: Remove debug.h file")
    956c36c297a2 ("USB: typec: add SPDX identifiers to some files")
    98076fa64a05 ("staging: typec: tcpm: Document data structures")
    ad74b8649bea ("usb: typec: ucsi: Preliminary support for alternate modes")
    cf6e06cddf29 ("usb: typec: Start using ERR_PTR")

v4.9.232: Failed to apply! Possible dependencies:
    0c744ea4f77d ("Linux 4.10-rc2")
    2bd6bf03f4c1 ("Linux 4.14-rc1")
    2ea659a9ef48 ("Linux 4.12-rc1")
    49def1853334 ("Linux 4.10-rc4")
    566cf877a1fc ("Linux 4.10-rc6")
    5771a8c08880 ("Linux v4.13-rc1")
    7089db84e356 ("Linux 4.10-rc8")
    7a308bb3016f ("Linux 4.10-rc5")
    7ce7d89f4883 ("Linux 4.10-rc1")
    81534d5fa973 ("usb: typec: ucsi: Remove debug.h file")
    a121103c9228 ("Linux 4.10-rc3")
    ad74b8649bea ("usb: typec: ucsi: Preliminary support for alternate modes")
    b24413180f56 ("License cleanup: add SPDX GPL-2.0 license identifier to files with no license")
    c1ae3cfa0e89 ("Linux 4.11-rc1")
    c470abd4fde4 ("Linux 4.10")
    d5adbfcd5f7b ("Linux 4.10-rc7")

v4.4.232: Failed to apply! Possible dependencies:
    1001354ca341 ("Linux 4.9-rc1")
    18558cae0272 ("Linux 4.5-rc4")
    1a695a905c18 ("Linux 4.7-rc1")
    29b4817d4018 ("Linux 4.8-rc1")
    2bd6bf03f4c1 ("Linux 4.14-rc1")
    2dcd0af568b0 ("Linux 4.6")
    2ea659a9ef48 ("Linux 4.12-rc1")
    36f90b0a2ddd ("Linux 4.5-rc2")
    388f7b1d6e8c ("Linux 4.5-rc3")
    5771a8c08880 ("Linux v4.13-rc1")
    7ce7d89f4883 ("Linux 4.10-rc1")
    81534d5fa973 ("usb: typec: ucsi: Remove debug.h file")
    81f70ba233d5 ("Linux 4.5-rc5")
    92e963f50fc7 ("Linux 4.5-rc1")
    ad74b8649bea ("usb: typec: ucsi: Preliminary support for alternate modes")
    b24413180f56 ("License cleanup: add SPDX GPL-2.0 license identifier to files with no license")
    b562e44f507e ("Linux 4.5")
    c1ae3cfa0e89 ("Linux 4.11-rc1")
    f55532a0c0b8 ("Linux 4.6-rc1")
    f6cede5b49e8 ("Linux 4.5-rc7")
    fc77dbd34c5c ("Linux 4.5-rc6")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?
diff mbox series

Patch

diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index d0c63afaf345..d9d93f83b2a6 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -205,7 +205,7 @@  void ucsi_altmode_update_active(struct ucsi_connector *con)
 	int i;
 
 	command = UCSI_GET_CURRENT_CAM | UCSI_CONNECTOR_NUMBER(con->num);
-	ret = ucsi_run_command(con->ucsi, command, &cur, sizeof(cur));
+	ret = ucsi_send_command(con->ucsi, command, &cur, sizeof(cur));
 	if (ret < 0) {
 		if (con->ucsi->version > 0x0100) {
 			dev_err(con->ucsi->dev,
@@ -354,7 +354,7 @@  ucsi_register_altmodes_nvidia(struct ucsi_connector *con, u8 recipient)
 		command |= UCSI_GET_ALTMODE_RECIPIENT(recipient);
 		command |= UCSI_GET_ALTMODE_CONNECTOR_NUMBER(con->num);
 		command |= UCSI_GET_ALTMODE_OFFSET(i);
-		len = ucsi_run_command(con->ucsi, command, &alt, sizeof(alt));
+		len = ucsi_send_command(con->ucsi, command, &alt, sizeof(alt));
 		/*
 		 * We are collecting all altmodes first and then registering.
 		 * Some type-C device will return zero length data beyond last
@@ -431,7 +431,7 @@  static int ucsi_register_altmodes(struct ucsi_connector *con, u8 recipient)
 		command |= UCSI_GET_ALTMODE_RECIPIENT(recipient);
 		command |= UCSI_GET_ALTMODE_CONNECTOR_NUMBER(con->num);
 		command |= UCSI_GET_ALTMODE_OFFSET(i);
-		len = ucsi_run_command(con->ucsi, command, alt, sizeof(alt));
+		len = ucsi_send_command(con->ucsi, command, alt, sizeof(alt));
 		if (len <= 0)
 			return len;
 
@@ -904,7 +904,7 @@  static int ucsi_register_port(struct ucsi *ucsi, int index)
 	/* Get connector capability */
 	command = UCSI_GET_CONNECTOR_CAPABILITY;
 	command |= UCSI_CONNECTOR_NUMBER(con->num);
-	ret = ucsi_run_command(ucsi, command, &con->cap, sizeof(con->cap));
+	ret = ucsi_send_command(ucsi, command, &con->cap, sizeof(con->cap));
 	if (ret < 0)
 		return ret;
 
@@ -953,8 +953,7 @@  static int ucsi_register_port(struct ucsi *ucsi, int index)
 
 	/* Get the status */
 	command = UCSI_GET_CONNECTOR_STATUS | UCSI_CONNECTOR_NUMBER(con->num);
-	ret = ucsi_run_command(ucsi, command, &con->status,
-			       sizeof(con->status));
+	ret = ucsi_send_command(ucsi, command, &con->status, sizeof(con->status));
 	if (ret < 0) {
 		dev_err(ucsi->dev, "con%d: failed to get status\n", con->num);
 		return 0;
@@ -1044,6 +1043,8 @@  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);
@@ -1054,12 +1055,10 @@  int ucsi_init(struct ucsi *ucsi)
 	/* Enable all notifications */
 	ucsi->ntfy = UCSI_ENABLE_NTFY_ALL;
 	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_unregister;
 
-	mutex_unlock(&ucsi->ppm_lock);
-
 	return 0;
 
 err_unregister:
@@ -1071,6 +1070,7 @@  int ucsi_init(struct ucsi *ucsi)
 		con->port = NULL;
 	}
 
+	mutex_lock(&ucsi->ppm_lock);
 err_reset:
 	ucsi_reset_ppm(ucsi);
 err: