[RFC PATCH 1/4] usb: typec: Drop locking from class code

[Guenter Roeck]

Drop locking from USB Type-C class code to avoid deadlocks between driver
and class code.

Signed-off-by: Guenter Roeck <groeck@xxxxxxxxxxxx>
---
 drivers/usb/typec/typec.c | 196 ++++++++++------------------------------------
 1 file changed, 41 insertions(+), 155 deletions(-)

diff --git a/drivers/usb/typec/typec.c b/drivers/usb/typec/typec.c
index bbfd6e5..0d0ebed 100644
--- a/drivers/usb/typec/typec.c
+++ b/drivers/usb/typec/typec.c
@@ -17,7 +17,6 @@
 struct typec_port {
 	unsigned int		id;
 	struct device		dev;
-	struct mutex		lock; /* device lock */
 
 	int			prefer_role;
 
@@ -480,38 +479,30 @@ EXPORT_SYMBOL_GPL(typec_disconnect);
 
 void typec_set_data_role(struct typec_port *port, enum typec_data_role role)
 {
-	mutex_lock(&port->lock);
 	port->data_role = role;
 	sysfs_notify(&port->dev.kobj, NULL, "current_data_role");
-	mutex_unlock(&port->lock);
 }
 EXPORT_SYMBOL(typec_set_data_role);
 
 void typec_set_pwr_role(struct typec_port *port, enum typec_role role)
 {
-	mutex_lock(&port->lock);
 	port->pwr_role = role;
 	sysfs_notify(&port->dev.kobj, NULL, "current_power_role");
-	mutex_unlock(&port->lock);
 }
 EXPORT_SYMBOL(typec_set_pwr_role);
 
 void typec_set_vconn_role(struct typec_port *port, enum typec_role role)
 {
-	mutex_lock(&port->lock);
 	port->vconn_role = role;
 	sysfs_notify(&port->dev.kobj, NULL, "current_vconn_role");
-	mutex_unlock(&port->lock);
 }
 EXPORT_SYMBOL(typec_set_vconn_role);
 
 void typec_set_pwr_opmode(struct typec_port *port,
 			  enum typec_pwr_opmode opmode)
 {
-	mutex_lock(&port->lock);
 	port->pwr_opmode = opmode;
 	sysfs_notify(&port->dev.kobj, NULL, "power_operation_mode");
-	mutex_unlock(&port->lock);
 }
 EXPORT_SYMBOL(typec_set_pwr_opmode);
 
@@ -527,15 +518,12 @@ EXPORT_SYMBOL(typec_set_pwr_opmode);
 void typec_altmode_update_active(struct typec_altmode *alt, int mode,
 				 bool active)
 {
-	struct typec_port *port = typec_altmode2port(alt);
 	struct typec_mode *m = alt->modes + mode;
 	char dir[6];
 
-	mutex_lock(&port->lock);
 	m->active = active;
 	sprintf(dir, "mode%d", mode);
 	sysfs_notify(&alt->dev.kobj, dir, "active");
-	mutex_unlock(&port->lock);
 }
 EXPORT_SYMBOL(typec_altmode_update_active);
 
@@ -573,14 +561,8 @@ typec_altmode_vdo_show(struct device *dev, struct device_attribute *attr,
 {
 	struct typec_mode *mode = container_of(attr, struct typec_mode,
 					       vdo_attr);
-	struct typec_port *port = typec_altmode2port(mode->alt_mode);
-	ssize_t ret;
 
-	mutex_lock(&port->lock);
-	ret = sprintf(buf, "0x%08x\n", mode->vdo);
-	mutex_unlock(&port->lock);
-
-	return ret;
+	return sprintf(buf, "0x%08x\n", mode->vdo);
 }
 
 static ssize_t
@@ -589,14 +571,8 @@ typec_altmode_desc_show(struct device *dev, struct device_attribute *attr,
 {
 	struct typec_mode *mode = container_of(attr, struct typec_mode,
 					       desc_attr);
-	struct typec_port *port = typec_altmode2port(mode->alt_mode);
-	ssize_t ret;
-
-	mutex_lock(&port->lock);
-	ret = sprintf(buf, "%s\n", mode->desc ? mode->desc : "");
-	mutex_unlock(&port->lock);
 
-	return ret;
+	return sprintf(buf, "%s\n", mode->desc ? mode->desc : "");
 }
 
 static ssize_t
@@ -605,14 +581,8 @@ typec_altmode_active_show(struct device *dev, struct device_attribute *attr,
 {
 	struct typec_mode *mode = container_of(attr, struct typec_mode,
 					       active_attr);
-	struct typec_port *port = typec_altmode2port(mode->alt_mode);
-	ssize_t ret;
-
-	mutex_lock(&port->lock);
-	ret = sprintf(buf, "%d\n", mode->active);
-	mutex_unlock(&port->lock);
 
-	return ret;
+	return sprintf(buf, "%d\n", mode->active);
 }
 
 static ssize_t
@@ -632,13 +602,11 @@ typec_altmode_active_store(struct device *dev, struct device_attribute *attr,
 	if (activate > 1)
 		return -EINVAL;
 
-	mutex_lock(&port->lock);
 	ret = port->cap->activate_mode(mode->alt_mode, mode->index, activate);
 	if (!ret) {
 		mode->active = activate;
 		ret = size;
 	}
-	mutex_unlock(&port->lock);
 
 	return ret;
 }
@@ -649,10 +617,8 @@ typec_altmode_roles_show(struct device *dev, struct device_attribute *attr,
 {
 	struct typec_mode *mode = container_of(attr, struct typec_mode,
 					       roles_attr);
-	struct typec_port *port = typec_altmode2port(mode->alt_mode);
 	ssize_t ret;
 
-	mutex_lock(&port->lock);
 	switch (mode->roles) {
 	case TYPEC_PORT_DFP:
 		ret =  sprintf(buf, "source\n");
@@ -665,7 +631,6 @@ typec_altmode_roles_show(struct device *dev, struct device_attribute *attr,
 		ret = sprintf(buf, "source, sink\n");
 		break;
 	}
-	mutex_unlock(&port->lock);
 
 	return ret;
 }
@@ -837,38 +802,30 @@ preferred_role_store(struct device *dev, struct device_attribute *attr,
 	enum typec_role role;
 	int ret;
 
-	mutex_lock(&port->lock);
-
 	if (port->cap->type != TYPEC_PORT_DRP) {
 		dev_dbg(dev, "Preferred role only supported with DRP ports\n");
-		ret = -EOPNOTSUPP;
-		goto out;
+		return -EOPNOTSUPP;
 	}
 
 	if (!port->cap->try_role) {
 		dev_dbg(dev, "Setting preferred role not supported\n");
-		ret = -EOPNOTSUPP;
-		goto out;
+		return -EOPNOTSUPP;
 	}
 
 	ret = match_string(typec_roles, ARRAY_SIZE(typec_roles), buf);
 	if (ret < 0) {
 		port->prefer_role = -1;
-		ret = size;
-		goto out;
+		return size;
 	}
 
 	role = ret;
 
 	ret = port->cap->try_role(port->cap, role);
 	if (ret)
-		goto out;
+		return ret;
 
 	port->prefer_role = role;
-	ret = size;
-out:
-	mutex_unlock(&port->lock);
-	return ret;
+	return size;
 }
 
 static ssize_t
@@ -878,15 +835,10 @@ preferred_role_show(struct device *dev, struct device_attribute *attr,
 	struct typec_port *port = to_typec_port(dev);
 	ssize_t ret = 0;
 
-	mutex_lock(&port->lock);
-
 	if (port->prefer_role < 0)
-		goto out;
+		return ret;
 
-	ret = sprintf(buf, "%s\n", typec_roles[port->prefer_role]);
-out:
-	mutex_unlock(&port->lock);
-	return ret;
+	return sprintf(buf, "%s\n", typec_roles[port->prefer_role]);
 }
 static DEVICE_ATTR_RW(preferred_role);
 
@@ -895,37 +847,30 @@ current_data_role_store(struct device *dev, struct device_attribute *attr,
 			const char *buf, size_t size)
 {
 	struct typec_port *port = to_typec_port(dev);
-	int ret = size;
-
-	mutex_lock(&port->lock);
+	int ret;
 
 	if (port->cap->type != TYPEC_PORT_DRP) {
 		dev_dbg(dev, "data role swap only supported with DRP ports\n");
-		ret = -EOPNOTSUPP;
-		goto out;
+		return -EOPNOTSUPP;
 	}
 
 	if (!port->cap->dr_set) {
 		dev_dbg(dev, "data role swapping not supported\n");
-		ret = -EOPNOTSUPP;
-		goto out;
+		return -EOPNOTSUPP;
 	}
 
 	if (!port->connected)
-		goto out;
+		return size;
 
 	ret = match_string(typec_data_roles, ARRAY_SIZE(typec_data_roles), buf);
 	if (ret < 0)
-		goto out;
+		return ret;
 
 	ret = port->cap->dr_set(port->cap, ret);
 	if (ret)
-		goto out;
+		return ret;
 
-	ret = size;
-out:
-	mutex_unlock(&port->lock);
-	return ret;
+	return size;
 }
 
 static ssize_t
@@ -933,13 +878,8 @@ current_data_role_show(struct device *dev, struct device_attribute *attr,
 		       char *buf)
 {
 	struct typec_port *port = to_typec_port(dev);
-	ssize_t ret;
-
-	mutex_lock(&port->lock);
-	ret = sprintf(buf, "%s\n", typec_data_roles[port->data_role]);
-	mutex_unlock(&port->lock);
 
-	return ret;
+	return sprintf(buf, "%s\n", typec_data_roles[port->data_role]);
 }
 static DEVICE_ATTR_RW(current_data_role);
 
@@ -948,19 +888,11 @@ static ssize_t supported_data_roles_show(struct device *dev,
 					 char *buf)
 {
 	struct typec_port *port = to_typec_port(dev);
-	size_t ret;
 
-	mutex_lock(&port->lock);
+	if (port->cap->type == TYPEC_PORT_DRP)
+		return sprintf(buf, "host, device\n");
 
-	if (port->cap->type == TYPEC_PORT_DRP) {
-		ret = sprintf(buf, "host, device\n");
-		goto out;
-	}
-
-	ret = sprintf(buf, "%s\n", typec_data_roles[port->data_role]);
-out:
-	mutex_unlock(&port->lock);
-	return ret;
+	return sprintf(buf, "%s\n", typec_data_roles[port->data_role]);
 }
 static DEVICE_ATTR_RO(supported_data_roles);
 
@@ -969,56 +901,43 @@ static ssize_t current_power_role_store(struct device *dev,
 					const char *buf, size_t size)
 {
 	struct typec_port *port = to_typec_port(dev);
-	int ret = size;
-
-	mutex_lock(&port->lock);
+	int ret;
 
 	if (!port->cap->usb_pd) {
 		dev_dbg(dev, "power role swap only supported with USB PD\n");
-		ret = -EOPNOTSUPP;
-		goto out;
+		return -EOPNOTSUPP;
 	}
 
 	if (!port->cap->pr_set) {
 		dev_dbg(dev, "power role swapping not supported\n");
-		ret = -EOPNOTSUPP;
-		goto out;
+		return -EOPNOTSUPP;
 	}
 
 	if (port->pwr_opmode != TYPEC_PWR_MODE_PD) {
 		dev_dbg(dev, "partner unable to swap power role\n");
-		ret = -EIO;
-		goto out;
+		return -EIO;
 	}
 
 	if (!port->connected)
-		goto out;
+		return size;
 
 	ret = match_string(typec_roles, ARRAY_SIZE(typec_roles), buf);
 	if (ret < 0)
-		goto out;
+		return ret;
 
 	ret = port->cap->pr_set(port->cap, ret);
 	if (ret)
-		goto out;
+		return ret;
 
-	ret = size;
-out:
-	mutex_unlock(&port->lock);
-	return ret;
+	return size;
 }
 
 static ssize_t current_power_role_show(struct device *dev,
 				       struct device_attribute *attr, char *buf)
 {
 	struct typec_port *port = to_typec_port(dev);
-	ssize_t ret;
 
-	mutex_lock(&port->lock);
-	ret = sprintf(buf, "%s\n", typec_roles[port->pwr_role]);
-	mutex_unlock(&port->lock);
-
-	return ret;
+	return sprintf(buf, "%s\n", typec_roles[port->pwr_role]);
 }
 static DEVICE_ATTR_RW(current_power_role);
 
@@ -1027,19 +946,11 @@ static ssize_t supported_power_roles_show(struct device *dev,
 					  char *buf)
 {
 	struct typec_port *port = to_typec_port(dev);
-	size_t ret;
 
-	mutex_lock(&port->lock);
-
-	if (port->cap->usb_pd || port->cap->type == TYPEC_PORT_DRP) {
-		ret = sprintf(buf, "source, sink\n");
-		goto out;
-	}
+	if (port->cap->usb_pd || port->cap->type == TYPEC_PORT_DRP)
+		return sprintf(buf, "source, sink\n");
 
-	ret = sprintf(buf, "%s\n", typec_roles[port->pwr_role]);
-out:
-	mutex_unlock(&port->lock);
-	return ret;
+	return sprintf(buf, "%s\n", typec_roles[port->pwr_role]);
 }
 static DEVICE_ATTR_RO(supported_power_roles);
 
@@ -1055,13 +966,8 @@ static ssize_t power_operation_mode_show(struct device *dev,
 					 char *buf)
 {
 	struct typec_port *port = to_typec_port(dev);
-	ssize_t ret;
 
-	mutex_lock(&port->lock);
-	ret = sprintf(buf, "%s\n", typec_pwr_opmodes[port->pwr_opmode]);
-	mutex_unlock(&port->lock);
-
-	return ret;
+	return sprintf(buf, "%s\n", typec_pwr_opmodes[port->pwr_opmode]);
 }
 static DEVICE_ATTR_RO(power_operation_mode);
 
@@ -1072,45 +978,33 @@ static ssize_t current_vconn_role_store(struct device *dev,
 	struct typec_port *port = to_typec_port(dev);
 	int ret;
 
-	mutex_lock(&port->lock);
-
 	if (!port->cap->usb_pd) {
 		dev_dbg(dev, "vconn swap only supported with USB PD\n");
-		ret = -EOPNOTSUPP;
-		goto out;
+		return -EOPNOTSUPP;
 	}
 
 	if (!port->cap->vconn_set) {
 		dev_dbg(dev, "vconn swapping not supported\n");
-		ret = -EOPNOTSUPP;
-		goto out;
+		return -EOPNOTSUPP;
 	}
 
 	ret = match_string(typec_roles, ARRAY_SIZE(typec_roles), buf);
 	if (ret < 0)
-		goto out;
+		return ret;
 
 	ret = port->cap->vconn_set(port->cap, ret);
 	if (ret)
-		goto out;
+		return ret;
 
-	ret = size;
-out:
-	mutex_unlock(&port->lock);
-	return ret;
+	return size;
 }
 
 static ssize_t current_vconn_role_show(struct device *dev,
 				       struct device_attribute *attr, char *buf)
 {
 	struct typec_port *port = to_typec_port(dev);
-	ssize_t ret;
 
-	mutex_lock(&port->lock);
-	ret = sprintf(buf, "%s\n", typec_roles[port->vconn_role]);
-	mutex_unlock(&port->lock);
-
-	return ret;
+	return sprintf(buf, "%s\n", typec_roles[port->vconn_role]);
 }
 static DEVICE_ATTR_RW(current_vconn_role);
 
@@ -1123,13 +1017,11 @@ static ssize_t supported_accessory_modes_show(struct device *dev,
 	ssize_t ret = 0;
 	int i;
 
-	mutex_lock(&port->lock);
 	if (port->cap->accessory)
 		for (accessory = port->cap->accessory, i = 0;
 		     i < port->cap->num_accessory; accessory++, i++)
 			ret += sprintf(buf, "%s\n",
 				       typec_accessory_modes[*accessory]);
-	mutex_unlock(&port->lock);
 
 	return ret;
 }
@@ -1140,13 +1032,8 @@ static ssize_t supports_usb_power_delivery_show(struct device *dev,
 						char *buf)
 {
 	struct typec_port *port = to_typec_port(dev);
-	ssize_t ret;
-
-	mutex_lock(&port->lock);
-	ret = sprintf(buf, "%d\n", port->cap->usb_pd);
-	mutex_unlock(&port->lock);
 
-	return ret;
+	return sprintf(buf, "%d\n", port->cap->usb_pd);
 }
 static DEVICE_ATTR_RO(supports_usb_power_delivery);
 
@@ -1224,7 +1111,6 @@ struct typec_port *typec_register_port(struct device *dev,
 	port->dev.class = &typec_class;
 	port->dev.parent = dev;
 	dev_set_name(&port->dev, "usbc%d", id);
-	mutex_init(&port->lock);
 
 	typec_init_roles(port);
 
-- 
2.6.6