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

From: Guenter Roeck
Date: Tue Aug 02 2016 - 16:35:49 EST


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