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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux