Re: [PATCH v5] usb: typec: Add a sysfs node to manage port type

Guenter Roeck <linux@xxxxxxxxxxxx> · Sat, 27 May 2017 14:04:01 -0700

On 05/27/2017 01:52 PM, Badhri Jagan Sridharan wrote:
User space applications in some cases have the need to enforce a
specific port type(DFP/UFP/DRP). This change allows userspace to
attempt setting the desired port type. Low level drivers can
however reject the request if the specific port type is not supported.

Signed-off-by: Badhri Jagan Sridharan <Badhri@xxxxxxxxxx>

Reviewed-by: Guenter Roeck <linux@xxxxxxxxxxxx>

---
Changelog since v1:
- introduced a new variable port_type in struct typec_port to track
   the current port type instead of changing type member in
   typec_capability to address Heikki Krogerus comments.
- changed the output format and strings that would be displayed as
   suggested by Heikki Krogerus.

Changelog since v2:
- introduced a new mutex lock to protect port_type for addressing
   the race conditions identified by Geunter Roeck
- added typec_port_types_drp for printing port_type when cap->type
   is TYPE_PORT_DRP as suggested by Geunter Roeck
- Power role swap and data role swaps would be rejected unless
   port port_type == TYPE_PORT_DRP
- port_type_store would return immediately if the current port_type
   is same as the new port_type as suggested by Geunter Roeck

Changelog since v3:
- Moved as much as code outside port_type_lock as suggested by
   Geunter Roeck
- Removed Change-Id line from commit message identified by
   Greg Kroah-Hartman

Changelog since v4:
- Corrected return value and moved moved one more line of code
   outside log in the port_type_store function as suggested

not that it matters: s/log/lock/

   by Geunter Roeck


and s/Geunter/Guenter/ ;-)

  drivers/usb/typec/typec.c | 106 +++++++++++++++++++++++++++++++++++++++++-----
  include/linux/usb/typec.h |   4 ++
  2 files changed, 100 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/typec/typec.c b/drivers/usb/typec/typec.c
index 89e540bb7ff3..63644785ce31 100644
--- a/drivers/usb/typec/typec.c
+++ b/drivers/usb/typec/typec.c
@@ -11,6 +11,7 @@
  
  #include <linux/device.h>
  #include <linux/module.h>
+#include <linux/mutex.h>
  #include <linux/slab.h>
  #include <linux/usb/typec.h>
  
@@ -69,6 +70,8 @@ struct typec_port {
  	enum typec_role			pwr_role;
  	enum typec_role			vconn_role;
  	enum typec_pwr_opmode		pwr_opmode;
+	enum typec_port_type		port_type;
+	struct mutex			port_type_lock;
  
  	const struct typec_capability	*cap;
  };
@@ -789,6 +792,18 @@ static const char * const typec_data_roles[] = {
  	[TYPEC_HOST]	= "host",
  };
  
+static const char * const typec_port_types[] = {
+	[TYPEC_PORT_DFP] = "source",
+	[TYPEC_PORT_UFP] = "sink",
+	[TYPEC_PORT_DRP] = "dual",
+};
+
+static const char * const typec_port_types_drp[] = {
+	[TYPEC_PORT_DFP] = "dual [source] sink",
+	[TYPEC_PORT_UFP] = "dual source [sink]",
+	[TYPEC_PORT_DRP] = "[dual] source sink",
+};
+
  static ssize_t
  preferred_role_store(struct device *dev, struct device_attribute *attr,
  		     const char *buf, size_t size)
@@ -846,11 +861,6 @@ static ssize_t data_role_store(struct device *dev,
  	struct typec_port *port = to_typec_port(dev);
  	int ret;
  
-	if (port->cap->type != TYPEC_PORT_DRP) {
-		dev_dbg(dev, "data role swap only supported with DRP ports\n");
-		return -EOPNOTSUPP;
-	}
-
  	if (!port->cap->dr_set) {
  		dev_dbg(dev, "data role swapping not supported\n");
  		return -EOPNOTSUPP;
@@ -860,11 +870,22 @@ static ssize_t data_role_store(struct device *dev,
  	if (ret < 0)
  		return ret;
  
+	mutex_lock(&port->port_type_lock);
+	if (port->port_type != TYPEC_PORT_DRP) {
+		dev_dbg(dev, "port type fixed at \"%s\"",
+			     typec_port_types[port->port_type]);
+		ret = -EOPNOTSUPP;
+		goto unlock_and_ret;
+	}
+
  	ret = port->cap->dr_set(port->cap, ret);
  	if (ret)
-		return ret;
+		goto unlock_and_ret;
  
-	return size;
+	ret = size;
+unlock_and_ret:
+	mutex_unlock(&port->port_type_lock);
+	return ret;
  }
  
  static ssize_t data_role_show(struct device *dev,
@@ -885,7 +906,7 @@ static ssize_t power_role_store(struct device *dev,
  				const char *buf, size_t size)
  {
  	struct typec_port *port = to_typec_port(dev);
-	int ret = size;
+	int ret;
  
  	if (!port->cap->pd_revision) {
  		dev_dbg(dev, "USB Power Delivery not supported\n");
@@ -906,11 +927,22 @@ static ssize_t power_role_store(struct device *dev,
  	if (ret < 0)
  		return ret;
  
+	mutex_lock(&port->port_type_lock);
+	if (port->port_type != TYPEC_PORT_DRP) {
+		dev_dbg(dev, "port type fixed at \"%s\"",
+			     typec_port_types[port->port_type]);
+		ret = -EOPNOTSUPP;
+		goto unlock_and_ret;
+	}
+
  	ret = port->cap->pr_set(port->cap, ret);
  	if (ret)
-		return ret;
+		goto unlock_and_ret;
  
-	return size;
+	ret = size;
+unlock_and_ret:
+	mutex_unlock(&port->port_type_lock);
+	return ret;
  }
  
  static ssize_t power_role_show(struct device *dev,
@@ -926,6 +958,57 @@ static ssize_t power_role_show(struct device *dev,
  }
  static DEVICE_ATTR_RW(power_role);
  
+static ssize_t
+port_type_store(struct device *dev, struct device_attribute *attr,
+			const char *buf, size_t size)
+{
+	struct typec_port *port = to_typec_port(dev);
+	int ret;
+	enum typec_port_type type;
+
+	if (!port->cap->port_type_set || port->cap->type != TYPEC_PORT_DRP) {
+		dev_dbg(dev, "changing port type not supported\n");
+		return -EOPNOTSUPP;
+	}
+
+	ret = sysfs_match_string(typec_port_types, buf);
+	if (ret < 0)
+		return ret;
+
+	type = ret;
+	mutex_lock(&port->port_type_lock);
+
+	if (port->port_type == type) {
+		ret = size;
+		goto unlock_and_ret;
+	}
+
+	ret = port->cap->port_type_set(port->cap, type);
+	if (ret)
+		goto unlock_and_ret;
+
+	port->port_type = type;
+	ret = size;
+
+unlock_and_ret:
+	mutex_unlock(&port->port_type_lock);
+	return ret;
+}
+
+static ssize_t
+port_type_show(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	struct typec_port *port = to_typec_port(dev);
+
+	if (port->cap->type == TYPEC_PORT_DRP)
+		return sprintf(buf, "%s\n",
+			       typec_port_types_drp[port->port_type]);
+
+	return sprintf(buf, "[%s]\n", typec_port_types[port->cap->type]);
+}
+static DEVICE_ATTR_RW(port_type);
+
  static const char * const typec_pwr_opmodes[] = {
  	[TYPEC_PWR_MODE_USB]	= "default",
  	[TYPEC_PWR_MODE_1_5A]	= "1.5A",
@@ -1035,6 +1118,7 @@ static struct attribute *typec_attrs[] = {
  	&dev_attr_usb_power_delivery_revision.attr,
  	&dev_attr_usb_typec_revision.attr,
  	&dev_attr_vconn_source.attr,
+	&dev_attr_port_type.attr,
  	NULL,
  };
  ATTRIBUTE_GROUPS(typec);
@@ -1211,6 +1295,8 @@ struct typec_port *typec_register_port(struct device *parent,
  
  	port->id = id;
  	port->cap = cap;
+	port->port_type = cap->type;
+	mutex_init(&port->port_type_lock);
  	port->prefer_role = cap->prefer_role;
  
  	port->dev.class = typec_class;
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index ec78204964ab..b174248af7c9 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -190,6 +190,7 @@ struct typec_partner_desc {
   * @pr_set: Set Power Role
   * @vconn_set: Set VCONN Role
   * @activate_mode: Enter/exit given Alternate Mode
+ * @port_type_set: Set port type
   *
   * Static capabilities of a single USB Type-C port.
   */
@@ -214,6 +215,9 @@ struct typec_capability {
  
  	int		(*activate_mode)(const struct typec_capability *,
  					 int mode, int activate);
+	int		(*port_type_set)(const struct typec_capability *,
+					enum typec_port_type);
+
  };
  
  /* Specific to try_role(). Indicates the user want's to clear the preference. */


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html






