Noticed that I accidentally dropped the ABI documentation for the new port_type node during patchset revision. Adding that back and resubmitting the patch. Thanks, Badhri. On Mon, May 29, 2017 at 1:07 PM, Badhri Jagan Sridharan <badhri@xxxxxxxxxx> wrote: > Thanks for all the reviews Guenter & Heikki. > Heikki, Is there anything else that I need to address in this patch ? > Or is the patch ready to be merged ? > > Thanks, > Badhri > > On Mon, May 29, 2017 at 12:56 PM, Badhri Jagan Sridharan > <badhri@xxxxxxxxxx> 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 Guenter Roeck >> - added typec_port_types_drp for printing port_type when cap->type >> is TYPE_PORT_DRP as suggested by Guenter 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 Guenter Roeck >> >> Changelog since v3: >> - Moved as much as code outside port_type_lock as suggested by >> Guenter 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 port_type_lock in the port_type_store function as >> suggested by Guenter Roeck >> >> 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. */ >> -- >> 2.13.0.219.gdb65acc882-goog >> -- 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