From: Guenter Roeck <groeck@chromium.org>
To: Felipe Balbi <felipe.balbi@linux.intel.com>
Cc: Chandra Sekhar Anagani <chandra.sekhar.anagani@intel.com>,
Bruce Ashfield <bruce.ashfield@windriver.com>,
Bin Gao <bin.gao@intel.com>,
Pranav Tipnis <pranav.tipnis@intel.com>,
Heikki Krogerus <heikki.krogerus@linux.intel.com>,
linux-kernel@vger.kernel.org, linux-usb@vger.kernel.org,
Guenter Roeck <groeck@chromium.org>
Subject: [RFC PATCH 1/4] usb: typec: Drop locking from class code
Date: Tue, 2 Aug 2016 13:32:52 -0700 [thread overview]
Message-ID: <1470169975-19941-2-git-send-email-groeck@chromium.org> (raw)
In-Reply-To: <1470169975-19941-1-git-send-email-groeck@chromium.org>
Drop locking from USB Type-C class code to avoid deadlocks between driver
and class code.
Signed-off-by: Guenter Roeck <groeck@chromium.org>
---
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
next prev parent reply other threads:[~2016-08-02 20:35 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-08-02 20:32 [RFC PATCH 0/4] Type-C Port Manager Guenter Roeck
2016-08-02 20:32 ` Guenter Roeck [this message]
2016-08-02 20:32 ` [RFC PATCH 2/4] usb: typec: Do not check if connected when setting roles Guenter Roeck
2016-08-02 20:32 ` [RFC PATCH 3/4] usb: typec: USB Type-C Port Manager (tcpm) Guenter Roeck
2016-08-03 9:18 ` Oliver Neukum
2016-08-03 18:03 ` Guenter Roeck
2016-08-03 9:28 ` Oliver Neukum
2016-08-03 18:46 ` Guenter Roeck
2016-08-02 20:32 ` [RFC PATCH 4/4] usb: typec: Type-C Port Controller Interface driver (tcpci) Guenter Roeck
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1470169975-19941-2-git-send-email-groeck@chromium.org \
--to=groeck@chromium.org \
--cc=bin.gao@intel.com \
--cc=bruce.ashfield@windriver.com \
--cc=chandra.sekhar.anagani@intel.com \
--cc=felipe.balbi@linux.intel.com \
--cc=heikki.krogerus@linux.intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=pranav.tipnis@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).