usb: typec: Add a sysfs node to manage port type

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 <[email protected]>

Change-Id Ia76877558c47271a34d912a54eea3459655dda3c

---
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

 drivers/usb/typec/typec.c | 127 ++++++++++++++++++++++++++++++++++++++++------
 include/linux/usb/typec.h |   4 ++
 2 files changed, 116 insertions(+), 15 deletions(-)

drivers/usb/typec/typec.c +0 -0

index 89e540bb7ff3..a0d8887ad560 100644

drivers/usb/typec/typec.c +5 -0

 struct typec_mode {
 	int				index;
 	u32				vdo;

@@ -69,6 +71,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 +793,19 @@ 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",

drivers/usb/typec/typec.c +0 -2

 static ssize_t
 preferred_role_store(struct device *dev, struct device_attribute *attr,
 		     const char *buf, size_t size)

@@ -846,25 +863,34 @@ 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");

drivers/usb/typec/typec.c +9 -4

 	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,32 +911,47 @@ 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;
 
+	mutex_lock(&port->port_type_lock);
 	if (!port->cap->pd_revision) {
 		dev_dbg(dev, "USB Power Delivery not supported\n");
-		return -EOPNOTSUPP;
+		ret = -EOPNOTSUPP;

drivers/usb/typec/typec.c +24 -4

+	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;
 	}
 
 	if (port->pwr_opmode != TYPEC_PWR_MODE_PD) {
 		dev_dbg(dev, "partner unable to swap power role\n");
-		return -EIO;
+		ret = -EIO;
+		goto unlock_and_ret;
 	}
 
 	ret = sysfs_match_string(typec_roles, buf);
 	if (ret < 0)
-		return ret;
+		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 +967,59 @@ 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;
+
+	mutex_lock(&port->port_type_lock);
+

drivers/usb/typec/typec.c +36 -0

+	type = ret;
+
+	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 size;
+}
+
+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 +1129,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 +1306,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;

include/linux/usb/typec.h +4 -0

index ec78204964ab..b174248af7c9 100644
@@ -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. */

Signature · 1 line

2.13.0.219.gdb65acc882-goog

Quoted 34 lines · Badhri Jagan Sridharan

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 <[email protected]>

Change-Id Ia76877558c47271a34d912a54eea3459655dda3c

---
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

 drivers/usb/typec/typec.c | 127 ++++++++++++++++++++++++++++++++++++++++------
 include/linux/usb/typec.h |   4 ++
 2 files changed, 116 insertions(+), 15 deletions(-)

drivers/usb/typec/typec.c +0 -0

index 89e540bb7ff3..a0d8887ad560 100644

drivers/usb/typec/typec.c line 18

Quoted 8 lines

drivers/usb/typec/typec.c +2 -0

@@ -11,9 +11,11 @@
 
 #include <linux/device.h>
 #include <linux/module.h>
+#include <linux/mutex.h>
 #include <linux/slab.h>
 #include <linux/usb/typec.h>
 
+

Single empty lines only, please.

Quoted 19 lines

drivers/usb/typec/typec.c +5 -0

 struct typec_mode {
      int                             index;
      u32                             vdo;

@@ -69,6 +71,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 +793,19 @@ 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",

drivers/usb/typec/typec.c static const char * const typec_data_roles[] = { · line 808

Quoted 10 lines

drivers/usb/typec/typec.c +10 -0

+     [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",
+};
+
+

Single empty line.

Quoted 9 lines

drivers/usb/typec/typec.c +0 -2

 static ssize_t
 preferred_role_store(struct device *dev, struct device_attribute *attr,
                   const char *buf, size_t size)

@@ -846,25 +863,34 @@ 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");

drivers/usb/typec/typec.c static ssize_t data_role_store(struct device *dev, · line 872

Quoted 10 lines

drivers/usb/typec/typec.c +4 -3

-             return -EOPNOTSUPP;
-     }
+     mutex_lock(&port->port_type_lock);
 
      if (!port->cap->dr_set) {
              dev_dbg(dev, "data role swapping not supported\n");
-             return -EOPNOTSUPP;
+             ret = -EOPNOTSUPP;
+             goto unlock_and_ret;
+     }

I think you can check this outside the lock; capabilities won't change.

drivers/usb/typec/typec.c static ssize_t data_role_store(struct device *dev, · line 883

Quoted 12 lines

drivers/usb/typec/typec.c +7 -1

+
+     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 = sysfs_match_string(typec_data_roles, buf);
      if (ret < 0)
-             return ret;
+             goto unlock_and_ret;

Move outside the lock ?

Quoted 27 lines

drivers/usb/typec/typec.c +9 -4

      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,32 +911,47 @@ 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;
 
+     mutex_lock(&port->port_type_lock);
      if (!port->cap->pd_revision) {
              dev_dbg(dev, "USB Power Delivery not supported\n");
-             return -EOPNOTSUPP;
+             ret = -EOPNOTSUPP;

drivers/usb/typec/typec.c static ssize_t power_role_store(struct device *dev, · line 928

Quoted 10 lines

drivers/usb/typec/typec.c +5 -1

+             goto unlock_and_ret;
      }
 
      if (!port->cap->pr_set) {
              dev_dbg(dev, "power role swapping not supported\n");
-             return -EOPNOTSUPP;
+             ret = -EOPNOTSUPP;
+             goto unlock_and_ret;
+     }
+

Capabilities can be checked outside the lock.

Quoted 50 lines

drivers/usb/typec/typec.c +27 -4

+     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;
      }
 
      if (port->pwr_opmode != TYPEC_PWR_MODE_PD) {
              dev_dbg(dev, "partner unable to swap power role\n");
-             return -EIO;
+             ret = -EIO;
+             goto unlock_and_ret;
      }
 
      ret = sysfs_match_string(typec_roles, buf);
      if (ret < 0)
-             return ret;
+             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 +967,59 @@ 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;
+
+     mutex_lock(&port->port_type_lock);
+
+     if (!port->cap->port_type_set || port->cap->type != TYPEC_PORT_DRP) {
+             dev_dbg(dev, "changing port type not supported\n");
+             ret = -EOPNOTSUPP;

drivers/usb/typec/typec.c static ssize_t power_role_show(struct device *dev, · line 989

Quoted 7 lines

drivers/usb/typec/typec.c +7 -0

+             goto unlock_and_ret;
+     }
+
+     ret = sysfs_match_string(typec_port_types, buf);
+     if (ret < 0)
+             goto unlock_and_ret;
+

I would suggest to check as much as possible outside the lock.

Quoted 79 lines

drivers/usb/typec/typec.c +36 -0

+     type = ret;
+
+     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 size;
+}
+
+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 +1129,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 +1306,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;

include/linux/usb/typec.h +4 -1

index ec78204964ab..b174248af7c9 100644
@@ -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. */
--

2.13.0.219.gdb65acc882-goog

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 <[email protected]>

Change-Id Ia76877558c47271a34d912a54eea3459655dda3c

Quoted 7 lines · Badhri Jagan Sridharan

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 <[email protected]>

Change-Id Ia76877558c47271a34d912a54eea3459655dda3c

Why is this line here? :)