Message ID | 20200809141904.4317-5-hdegoede@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | usb: typec: ucsi: Fix various locking issues | expand |
Hi [This is an automated email] This commit has been processed because it contains a "Fixes:" tag fixing commit: 081da1325d35 ("usb: typec: ucsi: displayport: Fix a potential race during registration"). The bot has tested the following trees: v5.8, v5.7.14, v5.4.57. v5.8: Build OK! v5.7.14: Failed to apply! Possible dependencies: 4dbc6a4ef06d ("usb: typec: ucsi: save power data objects in PD mode") 992a60ed0d5e ("usb: typec: ucsi: register with power_supply class") v5.4.57: Failed to apply! Possible dependencies: 2ede55468ca8 ("usb: typec: ucsi: Remove the old API") 3cf657f07918 ("usb: typec: ucsi: Remove all bit-fields") 470ce43a1a81 ("usb: typec: ucsi: Remove struct ucsi_control") 4dbc6a4ef06d ("usb: typec: ucsi: save power data objects in PD mode") 6df475f804e6 ("usb: typec: ucsi: Start using struct typec_operations") 992a60ed0d5e ("usb: typec: ucsi: register with power_supply class") bdc62f2bae8f ("usb: typec: ucsi: Simplified registration and I/O API") NOTE: The patch will not be queued to stable trees until it is upstream. How should we proceed with this patch?
Hi [This is an automated email] This commit has been processed because it contains a "Fixes:" tag fixing commit: 081da1325d35 ("usb: typec: ucsi: displayport: Fix a potential race during registration"). The bot has tested the following trees: v5.8.1, v5.7.15, v5.4.58. v5.8.1: Build OK! v5.7.15: Failed to apply! Possible dependencies: 4dbc6a4ef06d ("usb: typec: ucsi: save power data objects in PD mode") 992a60ed0d5e ("usb: typec: ucsi: register with power_supply class") v5.4.58: Failed to apply! Possible dependencies: 2ede55468ca8 ("usb: typec: ucsi: Remove the old API") 3cf657f07918 ("usb: typec: ucsi: Remove all bit-fields") 470ce43a1a81 ("usb: typec: ucsi: Remove struct ucsi_control") 4dbc6a4ef06d ("usb: typec: ucsi: save power data objects in PD mode") 6df475f804e6 ("usb: typec: ucsi: Start using struct typec_operations") 992a60ed0d5e ("usb: typec: ucsi: register with power_supply class") bdc62f2bae8f ("usb: typec: ucsi: Simplified registration and I/O API") NOTE: The patch will not be queued to stable trees until it is upstream. How should we proceed with this patch?
Hi [This is an automated email] This commit has been processed because it contains a "Fixes:" tag fixing commit: 081da1325d35 ("usb: typec: ucsi: displayport: Fix a potential race during registration"). The bot has tested the following trees: v5.8.2, v5.7.16, v5.4.59. v5.8.2: Build OK! v5.7.16: Failed to apply! Possible dependencies: 4dbc6a4ef06d ("usb: typec: ucsi: save power data objects in PD mode") 992a60ed0d5e ("usb: typec: ucsi: register with power_supply class") v5.4.59: Failed to apply! Possible dependencies: 2ede55468ca8 ("usb: typec: ucsi: Remove the old API") 3cf657f07918 ("usb: typec: ucsi: Remove all bit-fields") 470ce43a1a81 ("usb: typec: ucsi: Remove struct ucsi_control") 4dbc6a4ef06d ("usb: typec: ucsi: save power data objects in PD mode") 6df475f804e6 ("usb: typec: ucsi: Start using struct typec_operations") 992a60ed0d5e ("usb: typec: ucsi: register with power_supply class") bdc62f2bae8f ("usb: typec: ucsi: Simplified registration and I/O API") NOTE: The patch will not be queued to stable trees until it is upstream. How should we proceed with this patch?
diff --git a/drivers/usb/typec/ucsi/displayport.c b/drivers/usb/typec/ucsi/displayport.c index 048381c058a5..261131c9e37c 100644 --- a/drivers/usb/typec/ucsi/displayport.c +++ b/drivers/usb/typec/ucsi/displayport.c @@ -288,8 +288,6 @@ struct typec_altmode *ucsi_register_displayport(struct ucsi_connector *con, struct typec_altmode *alt; struct ucsi_dp *dp; - mutex_lock(&con->lock); - /* We can't rely on the firmware with the capabilities. */ desc->vdo |= DP_CAP_DP_SIGNALING | DP_CAP_RECEPTACLE; @@ -298,15 +296,12 @@ struct typec_altmode *ucsi_register_displayport(struct ucsi_connector *con, desc->vdo |= all_assignments << 16; alt = typec_port_register_altmode(con->port, desc); - if (IS_ERR(alt)) { - mutex_unlock(&con->lock); + if (IS_ERR(alt)) return alt; - } dp = devm_kzalloc(&alt->dev, sizeof(*dp), GFP_KERNEL); if (!dp) { typec_unregister_altmode(alt); - mutex_unlock(&con->lock); return ERR_PTR(-ENOMEM); } @@ -319,7 +314,5 @@ struct typec_altmode *ucsi_register_displayport(struct ucsi_connector *con, alt->ops = &ucsi_displayport_ops; typec_altmode_set_drvdata(alt, dp); - mutex_unlock(&con->lock); - return alt; } diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c index 182e34aa65ed..2999217c8109 100644 --- a/drivers/usb/typec/ucsi/ucsi.c +++ b/drivers/usb/typec/ucsi/ucsi.c @@ -898,12 +898,15 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) con->num = index + 1; con->ucsi = ucsi; + /* Delay other interactions with the con until registration is complete */ + mutex_lock(&con->lock); + /* Get connector capability */ command = UCSI_GET_CONNECTOR_CAPABILITY; command |= UCSI_CONNECTOR_NUMBER(con->num); ret = ucsi_send_command(ucsi, command, &con->cap, sizeof(con->cap)); if (ret < 0) - return ret; + goto out; if (con->cap.op_mode & UCSI_CONCAP_OPMODE_DRP) cap->data = TYPEC_PORT_DRD; @@ -935,26 +938,32 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) ret = ucsi_register_port_psy(con); if (ret) - return ret; + goto out; /* Register the connector */ con->port = typec_register_port(ucsi->dev, cap); - if (IS_ERR(con->port)) - return PTR_ERR(con->port); + if (IS_ERR(con->port)) { + ret = PTR_ERR(con->port); + goto out; + } /* Alternate modes */ ret = ucsi_register_altmodes(con, UCSI_RECIPIENT_CON); - if (ret) + if (ret) { dev_err(ucsi->dev, "con%d: failed to register alt modes\n", con->num); + goto out; + } /* Get the status */ command = UCSI_GET_CONNECTOR_STATUS | UCSI_CONNECTOR_NUMBER(con->num); 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; + ret = 0; + goto out; } + ret = 0; /* ucsi_send_command() returns length on success */ switch (UCSI_CONSTAT_PARTNER_TYPE(con->status.flags)) { case UCSI_CONSTAT_PARTNER_TYPE_UFP: @@ -979,17 +988,21 @@ static int ucsi_register_port(struct ucsi *ucsi, int index) if (con->partner) { ret = ucsi_register_altmodes(con, UCSI_RECIPIENT_SOP); - if (ret) + if (ret) { dev_err(ucsi->dev, "con%d: failed to register alternate modes\n", con->num); - else + ret = 0; + } else { ucsi_altmode_update_active(con); + } } trace_ucsi_register_port(con->num, &con->status); - return 0; +out: + mutex_unlock(&con->lock); + return ret; } /**
Commit 081da1325d35 ("usb: typec: ucsi: displayport: Fix a potential race during registration") made the ucsi code hold con->lock in ucsi_register_displayport(). But we really don't want any interactions with the connector to run before the port-registration process is fully complete. This commit moves the taking of con->lock from ucsi_register_displayport() into ucsi_register_port() to achieve this. Cc: stable@vger.kernel.org Fixes: 081da1325d35 ("usb: typec: ucsi: displayport: Fix a potential race during registration") Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/usb/typec/ucsi/displayport.c | 9 +------- drivers/usb/typec/ucsi/ucsi.c | 31 ++++++++++++++++++++-------- 2 files changed, 23 insertions(+), 17 deletions(-)