diff mbox

[v6,05/15] usb: typec: add API to get typec basic port power and data config

Message ID 20180611110847.GG17155@kuha.fi.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Heikki Krogerus June 11, 2018, 11:08 a.m. UTC
Hi Jun,

On Mon, May 28, 2018 at 10:52:37AM +0800, Li Jun wrote:
> This patch adds 3 APIs to get the typec port power and data type,
> and preferred power role by its name string.
> 
> Signed-off-by: Li Jun <jun.li@nxp.com>
> ---
>  drivers/usb/typec/class.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++
>  include/linux/usb/typec.h |  3 +++
>  2 files changed, 53 insertions(+)
> 
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 53df10d..4c7d18c 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -802,6 +802,12 @@ static const char * const typec_port_types[] = {
>  	[TYPEC_PORT_DRP] = "dual",
>  };
>  
> +static const char * const typec_data_caps[] = {
> +	[TYPEC_PORT_DFP] = "host",
> +	[TYPEC_PORT_UFP] = "device",
> +	[TYPEC_PORT_DRD] = "dual",
> +};

Since I guess you need to fix this patch in any case, could you rename
that to "typec_port_data_roles".

And while at it, how about using this as an opportunity to rename
typec_port_types to typec_port_power_roles?

So this just a suggestion, no need to actually change it :-) :


Thanks,

Comments

Jun Li June 13, 2018, 9:47 a.m. UTC | #1
Hi Heikki,
> -----Original Message-----

> From: Heikki Krogerus [mailto:heikki.krogerus@linux.intel.com]

> Sent: 2018年6月11日 19:09

> To: Jun Li <jun.li@nxp.com>

> Cc: robh+dt@kernel.org; gregkh@linuxfoundation.org; linux@roeck-us.net;

> cw00.choi@samsung.com; a.hajda@samsung.com; shufan_lee@richtek.com;

> Peter Chen <peter.chen@nxp.com>; garsilva@embeddedor.com;

> gsomlo@gmail.com; linux-usb@vger.kernel.org; devicetree@vger.kernel.org;

> dl-linux-imx <linux-imx@nxp.com>

> Subject: Re: [PATCH v6 05/15] usb: typec: add API to get typec basic port power

> and data config

> 

> Hi Jun,

> 

> On Mon, May 28, 2018 at 10:52:37AM +0800, Li Jun wrote:

> > This patch adds 3 APIs to get the typec port power and data type, and

> > preferred power role by its name string.

> >

> > Signed-off-by: Li Jun <jun.li@nxp.com>

> > ---

> >  drivers/usb/typec/class.c | 50

> > +++++++++++++++++++++++++++++++++++++++++++++++

> >  include/linux/usb/typec.h |  3 +++

> >  2 files changed, 53 insertions(+)

> >

> > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c

> > index 53df10d..4c7d18c 100644

> > --- a/drivers/usb/typec/class.c

> > +++ b/drivers/usb/typec/class.c

> > @@ -802,6 +802,12 @@ static const char * const typec_port_types[] = {

> >  	[TYPEC_PORT_DRP] = "dual",

> >  };

> >

> > +static const char * const typec_data_caps[] = {

> > +	[TYPEC_PORT_DFP] = "host",

> > +	[TYPEC_PORT_UFP] = "device",

> > +	[TYPEC_PORT_DRD] = "dual",

> > +};

> 

> Since I guess you need to fix this patch in any case, could you rename that to

> "typec_port_data_roles".


OK.

> 

> And while at it, how about using this as an opportunity to rename

> typec_port_types to typec_port_power_roles?

> 

> So this just a suggestion, no need to actually change it :-) :


Also OK for me, I can rename it by this chance.

> 

> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c index

> abbd33939109..97f7eb0e9879 100644

> --- a/drivers/usb/typec/class.c

> +++ b/drivers/usb/typec/class.c

> @@ -918,12 +918,18 @@ static const char * const typec_data_roles[] = {

>         [TYPEC_HOST]    = "host",

>  };

> 

> -static const char * const typec_port_types[] = {

> +static const char * const typec_port_power_roles[] = {

>         [TYPEC_PORT_SRC] = "source",

>         [TYPEC_PORT_SNK] = "sink",

>         [TYPEC_PORT_DRP] = "dual",

>  };

> 

> +static const char * const typec_port_data_roles[] = {

> +       [TYPEC_PORT_DFP] = "host",

> +       [TYPEC_PORT_UFP] = "device",

> +       [TYPEC_PORT_DRD] = "dual",

> +};

> +

>  static const char * const typec_port_types_drp[] = {

>         [TYPEC_PORT_SRC] = "dual [source] sink",

>         [TYPEC_PORT_SNK] = "dual source [sink]", @@ -1054,7 +1060,7 @@

> static ssize_t power_role_store(struct device *dev,

>         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]);

> +                       typec_port_power_roles[port->port_type]);

>                 ret = -EOPNOTSUPP;

>                 goto unlock_and_ret;

>         }

> @@ -1095,7 +1101,7 @@ port_type_store(struct device *dev, struct

> device_attribute *attr,

>                 return -EOPNOTSUPP;

>         }

> 

> -       ret = sysfs_match_string(typec_port_types, buf);

> +       ret = sysfs_match_string(typec_port_power_roles, buf);

>         if (ret < 0)

>                 return ret;

> 

> @@ -1129,7 +1135,7 @@ port_type_show(struct device *dev, struct

> device_attribute *attr,

>                 return sprintf(buf, "%s\n",

>                                typec_port_types_drp[port->port_type]);

> 

> -       return sprintf(buf, "[%s]\n", typec_port_types[port->cap->type]);

> +       return sprintf(buf, "[%s]\n",

> + typec_port_power_roles[port->cap->type]);

>  }

>  static DEVICE_ATTR_RW(port_type);

> 

> 

> Thanks,

> 

> --

> heikki
diff mbox

Patch

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index abbd33939109..97f7eb0e9879 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -918,12 +918,18 @@  static const char * const typec_data_roles[] = {
        [TYPEC_HOST]    = "host",
 };

-static const char * const typec_port_types[] = {
+static const char * const typec_port_power_roles[] = {
        [TYPEC_PORT_SRC] = "source",
        [TYPEC_PORT_SNK] = "sink",
        [TYPEC_PORT_DRP] = "dual",
 };

+static const char * const typec_port_data_roles[] = {
+       [TYPEC_PORT_DFP] = "host",
+       [TYPEC_PORT_UFP] = "device",
+       [TYPEC_PORT_DRD] = "dual",
+};
+
 static const char * const typec_port_types_drp[] = {
        [TYPEC_PORT_SRC] = "dual [source] sink",
        [TYPEC_PORT_SNK] = "dual source [sink]",
@@ -1054,7 +1060,7 @@  static ssize_t power_role_store(struct device *dev,
        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]);
+                       typec_port_power_roles[port->port_type]);
                ret = -EOPNOTSUPP;
                goto unlock_and_ret;
        }
@@ -1095,7 +1101,7 @@  port_type_store(struct device *dev, struct device_attribute *attr,
                return -EOPNOTSUPP;
        }

-       ret = sysfs_match_string(typec_port_types, buf);
+       ret = sysfs_match_string(typec_port_power_roles, buf);
        if (ret < 0)
                return ret;

@@ -1129,7 +1135,7 @@  port_type_show(struct device *dev, struct device_attribute *attr,
                return sprintf(buf, "%s\n",
                               typec_port_types_drp[port->port_type]);

-       return sprintf(buf, "[%s]\n", typec_port_types[port->cap->type]);
+       return sprintf(buf, "[%s]\n", typec_port_power_roles[port->cap->type]);
 }
 static DEVICE_ATTR_RW(port_type);