linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v4] usb: typec: Add a sysfs node to manage port type
@ 2017-05-26 23:07 Badhri Jagan Sridharan
  2017-05-27  2:24 ` Guenter Roeck
  0 siblings, 1 reply; 3+ messages in thread
From: Badhri Jagan Sridharan @ 2017-05-26 23:07 UTC (permalink / raw)
  To: Heikki Krogerus, Greg Kroah-Hartman, Guenter Roeck, Oliver Neukum
  Cc: linux-usb, linux-kernel, 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 <Badhri@google.com>
---
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

 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..bf1eb11c6646 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;
+
+	mutex_lock(&port->port_type_lock);
+	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 +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. */
-- 
2.13.0.219.gdb65acc882-goog

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] usb: typec: Add a sysfs node to manage port type
  2017-05-26 23:07 [PATCH v4] usb: typec: Add a sysfs node to manage port type Badhri Jagan Sridharan
@ 2017-05-27  2:24 ` Guenter Roeck
  2017-05-27 20:34   ` Badhri Jagan Sridharan
  0 siblings, 1 reply; 3+ messages in thread
From: Guenter Roeck @ 2017-05-27  2:24 UTC (permalink / raw)
  To: Badhri Jagan Sridharan, Heikki Krogerus, Greg Kroah-Hartman,
	Oliver Neukum
  Cc: linux-usb, linux-kernel

On 05/26/2017 04:07 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@google.com>
> ---
> 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
> 
>   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..bf1eb11c6646 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;
> +
> +	mutex_lock(&port->port_type_lock);
> +	type = ret;
> +

Can be outside 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 size;

	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. */
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v4] usb: typec: Add a sysfs node to manage port type
  2017-05-27  2:24 ` Guenter Roeck
@ 2017-05-27 20:34   ` Badhri Jagan Sridharan
  0 siblings, 0 replies; 3+ messages in thread
From: Badhri Jagan Sridharan @ 2017-05-27 20:34 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Heikki Krogerus, Greg Kroah-Hartman, Oliver Neukum, USB, LKML

Thanks Geunter for comments. Will fix them in my next patch.

On Fri, May 26, 2017 at 7:24 PM, Guenter Roeck <linux@roeck-us.net> wrote:
> On 05/26/2017 04:07 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@google.com>
>> ---
>> 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
>>
>>   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..bf1eb11c6646 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;
>> +
>> +       mutex_lock(&port->port_type_lock);
>> +       type = ret;
>> +
>
>
> Can be outside 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 size;
>
>
>         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. */
>>
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-05-27 20:34 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-26 23:07 [PATCH v4] usb: typec: Add a sysfs node to manage port type Badhri Jagan Sridharan
2017-05-27  2:24 ` Guenter Roeck
2017-05-27 20:34   ` Badhri Jagan Sridharan

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