Hi Heikki,
> -----Original Message-----
> From: Heikki Krogerus [mailto:heikki.kroge...@linux.intel.com]
> Sent: 2018年6月11日 19:09
> To: Jun Li <jun...@nxp.com>
> Cc: robh...@kernel.org; gre...@linuxfoundation.org; li...@roeck-us.net;
> cw00.c...@samsung.com; a.ha...@samsung.com; shufan_...@richtek.com;
> Peter Chen <peter.c...@nxp.com>; garsi...@embeddedor.com;
> gso...@gmail.com; linux-usb@vger.kernel.org; devicet...@vger.kernel.org;
> dl-linux-imx <linux-...@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...@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
