[PATCH] usb: typec: Add sysfs node to show cc orientation

[PATCH] usb: typec: Add sysfs node to show cc orientation

[pumahsu]

Export the Type-C cc orientation so that user space can
get this information.

Signed-off-by: pumahsu <pumahsu@google.com>
---
 Documentation/ABI/testing/sysfs-class-typec |  7 +++++++
 drivers/usb/typec/class.c                   | 11 +++++++++++
 2 files changed, 18 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
index d7647b258c3c..419f952c991d 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -108,6 +108,13 @@ Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
 Description:
 		Revision number of the supported USB Type-C specification.
 
+What:		/sys/class/typec/<port>/cc_orientation
+Date:		September 2019
+Contact:	Puma Hsu <pumahsu@google.com>
+Description:
+		Indicates which cc orientation is active now, or 0 when
+		nothing is connected.
+
 
 USB Type-C partner devices (eg. /sys/class/typec/port0-partner/)
 
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 7d8805d9bf37..00edae63da8e 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -1238,6 +1238,16 @@ static ssize_t usb_power_delivery_revision_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(usb_power_delivery_revision);
 
+static ssize_t cc_orientation_show(struct device *dev,
+						struct device_attribute *attr,
+						char *buf)
+{
+	struct typec_port *p = to_typec_port(dev);
+
+	return sprintf(buf, "%d\n", typec_get_orientation(p));
+}
+static DEVICE_ATTR_RO(cc_orientation);
+
 static struct attribute *typec_attrs[] = {
 	&dev_attr_data_role.attr,
 	&dev_attr_power_operation_mode.attr,
@@ -1248,6 +1258,7 @@ static struct attribute *typec_attrs[] = {
 	&dev_attr_usb_typec_revision.attr,
 	&dev_attr_vconn_source.attr,
 	&dev_attr_port_type.attr,
+	&dev_attr_cc_orientation.attr,
 	NULL,
 };
 ATTRIBUTE_GROUPS(typec);
-- 
2.23.0.700.g56cf767bdb-goog

Re: [PATCH] usb: typec: Add sysfs node to show cc orientation

[Randy Dunlap]

On 10/15/19 8:43 PM, pumahsu wrote:
> Export the Type-C cc orientation so that user space can
> get this information.
> 
> Signed-off-by: pumahsu <pumahsu@google.com>

Hi,
what the * is cc orientation?

> ---
>  Documentation/ABI/testing/sysfs-class-typec |  7 +++++++
>  drivers/usb/typec/class.c                   | 11 +++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> index d7647b258c3c..419f952c991d 100644
> --- a/Documentation/ABI/testing/sysfs-class-typec
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -108,6 +108,13 @@ Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
>  Description:
>  		Revision number of the supported USB Type-C specification.
>  
> +What:		/sys/class/typec/<port>/cc_orientation
> +Date:		September 2019
> +Contact:	Puma Hsu <pumahsu@google.com>
> +Description:
> +		Indicates which cc orientation is active now, or 0 when
> +		nothing is connected.
> +
>  
>  USB Type-C partner devices (eg. /sys/class/typec/port0-partner/)
>  
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 7d8805d9bf37..00edae63da8e 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -1238,6 +1238,16 @@ static ssize_t usb_power_delivery_revision_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(usb_power_delivery_revision);
>  
> +static ssize_t cc_orientation_show(struct device *dev,
> +						struct device_attribute *attr,
> +						char *buf)
> +{
> +	struct typec_port *p = to_typec_port(dev);
> +
> +	return sprintf(buf, "%d\n", typec_get_orientation(p));
> +}
> +static DEVICE_ATTR_RO(cc_orientation);
> +
>  static struct attribute *typec_attrs[] = {
>  	&dev_attr_data_role.attr,
>  	&dev_attr_power_operation_mode.attr,
> @@ -1248,6 +1258,7 @@ static struct attribute *typec_attrs[] = {
>  	&dev_attr_usb_typec_revision.attr,
>  	&dev_attr_vconn_source.attr,
>  	&dev_attr_port_type.attr,
> +	&dev_attr_cc_orientation.attr,
>  	NULL,
>  };
>  ATTRIBUTE_GROUPS(typec);
> 


-- 
~Randy

Re: [PATCH] usb: typec: Add sysfs node to show cc orientation

[Puma Hsu]

Hi Randy,

According to include/linux/usb/typec.h, The value of
/sys/class/typec/<port>/cc_orientation is usually “1”, “2”, or “0”
corresponding to “configured in NORMAL side”, “configured in REVERSE
side”, or “nothing configured”.
Refer to the usage in tcpm.c, it claims CC1 is NORMAL and CC2 is REVERSE.
I will update this into Documentation description.

enum typec_orientation {
        TYPEC_ORIENTATION_NONE,
        TYPEC_ORIENTATION_NORMAL,
        TYPEC_ORIENTATION_REVERSE,
};

Thanks in advance.



  •  Puma Hsu 許誌宏
  •  Software Engineer, Pixel Phone
  •  Tel: +886 2 8729 0870
  •  pumahsu@google.com


Thanks in advance.




  •  Puma Hsu 許誌宏
  •  Software Engineer, Pixel Phone
  •  Tel: +886 2 8729 0870
  •  pumahsu@google.com





On Wed, Oct 16, 2019 at 11:47 AM Randy Dunlap <rdunlap@infradead.org> wrote:
>
> On 10/15/19 8:43 PM, pumahsu wrote:
> > Export the Type-C cc orientation so that user space can
> > get this information.
> >
> > Signed-off-by: pumahsu <pumahsu@google.com>
>
> Hi,
> what the * is cc orientation?
>
> > ---
> >  Documentation/ABI/testing/sysfs-class-typec |  7 +++++++
> >  drivers/usb/typec/class.c                   | 11 +++++++++++
> >  2 files changed, 18 insertions(+)
> >
> > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> > index d7647b258c3c..419f952c991d 100644
> > --- a/Documentation/ABI/testing/sysfs-class-typec
> > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > @@ -108,6 +108,13 @@ Contact: Heikki Krogerus <heikki.krogerus@linux.intel.com>
> >  Description:
> >               Revision number of the supported USB Type-C specification.
> >
> > +What:                /sys/class/typec/<port>/cc_orientation
> > +Date:                September 2019
> > +Contact:     Puma Hsu <pumahsu@google.com>
> > +Description:
> > +             Indicates which cc orientation is active now, or 0 when
> > +             nothing is connected.
> > +
> >
> >  USB Type-C partner devices (eg. /sys/class/typec/port0-partner/)
> >
> > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > index 7d8805d9bf37..00edae63da8e 100644
> > --- a/drivers/usb/typec/class.c
> > +++ b/drivers/usb/typec/class.c
> > @@ -1238,6 +1238,16 @@ static ssize_t usb_power_delivery_revision_show(struct device *dev,
> >  }
> >  static DEVICE_ATTR_RO(usb_power_delivery_revision);
> >
> > +static ssize_t cc_orientation_show(struct device *dev,
> > +                                             struct device_attribute *attr,
> > +                                             char *buf)
> > +{
> > +     struct typec_port *p = to_typec_port(dev);
> > +
> > +     return sprintf(buf, "%d\n", typec_get_orientation(p));
> > +}
> > +static DEVICE_ATTR_RO(cc_orientation);
> > +
> >  static struct attribute *typec_attrs[] = {
> >       &dev_attr_data_role.attr,
> >       &dev_attr_power_operation_mode.attr,
> > @@ -1248,6 +1258,7 @@ static struct attribute *typec_attrs[] = {
> >       &dev_attr_usb_typec_revision.attr,
> >       &dev_attr_vconn_source.attr,
> >       &dev_attr_port_type.attr,
> > +     &dev_attr_cc_orientation.attr,
> >       NULL,
> >  };
> >  ATTRIBUTE_GROUPS(typec);
> >
>
>
> --
> ~Randy

Re: [PATCH] usb: typec: Add sysfs node to show cc orientation

[Greg KH]

On Wed, Oct 16, 2019 at 11:43:14AM +0800, pumahsu wrote:
> Export the Type-C cc orientation so that user space can
> get this information.
> 
> Signed-off-by: pumahsu <pumahsu@google.com>
> ---
>  Documentation/ABI/testing/sysfs-class-typec |  7 +++++++
>  drivers/usb/typec/class.c                   | 11 +++++++++++
>  2 files changed, 18 insertions(+)

Hi,

This is the friendly patch-bot of Greg Kroah-Hartman.  You have sent him
a patch that has triggered this response.  He used to manually respond
to these common problems, but in order to save his sanity (he kept
writing the same thing over and over, yet to different people), I was
created.  Hopefully you will not take offence and will fix the problem
in your patch and resubmit it so that it can be accepted into the Linux
kernel tree.

You are receiving this message because of the following common error(s)
as indicated below:

- It looks like you did not use your "real" name for the patch on either
  the Signed-off-by: line, or the From: line (both of which have to
  match).  Please read the kernel file, Documentation/SubmittingPatches
  for how to do this correctly.

If you wish to discuss this problem further, or you have questions about
how to resolve this issue, please feel free to respond to this email and
Greg will reply once he has dug out from the pending patches received
from other developers.

thanks,

greg k-h's patch email bot

Re: [PATCH] usb: typec: Add sysfs node to show cc orientation

[Heikki Krogerus]

On Wed, Oct 16, 2019 at 11:43:14AM +0800, pumahsu wrote:
> Export the Type-C cc orientation so that user space can
> get this information.

For what do you need this information in user space? I'm guessing you
have something else in mind besides exposing this as just generic
information, or debugging purposes, no?

Please keep in mind that we do not always know the cable orientation.
UCSI for example does not give any clues about which way the cable
plug was connected to the connector. That means this sysfs file will
most likely need to be hidden in those cases, which I guess is
acceptable, but definitely not ideal.

> Signed-off-by: pumahsu <pumahsu@google.com>
> ---
>  Documentation/ABI/testing/sysfs-class-typec |  7 +++++++
>  drivers/usb/typec/class.c                   | 11 +++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> index d7647b258c3c..419f952c991d 100644
> --- a/Documentation/ABI/testing/sysfs-class-typec
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -108,6 +108,13 @@ Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
>  Description:
>  		Revision number of the supported USB Type-C specification.
>  
> +What:		/sys/class/typec/<port>/cc_orientation
> +Date:		September 2019
> +Contact:	Puma Hsu <pumahsu@google.com>
> +Description:
> +		Indicates which cc orientation is active now, or 0 when
> +		nothing is connected.

cc_orientation is a bit cryptic. I think if this is part of the port
ABI, then we should talk about something like "connector_orientation".

>  USB Type-C partner devices (eg. /sys/class/typec/port0-partner/)
>  
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 7d8805d9bf37..00edae63da8e 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -1238,6 +1238,16 @@ static ssize_t usb_power_delivery_revision_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(usb_power_delivery_revision);
>  
> +static ssize_t cc_orientation_show(struct device *dev,
> +						struct device_attribute *attr,
> +						char *buf)
> +{
> +	struct typec_port *p = to_typec_port(dev);
> +
> +	return sprintf(buf, "%d\n", typec_get_orientation(p));
> +}
> +static DEVICE_ATTR_RO(cc_orientation);

Now you are returning 0, 1 or 2 which to me is not ideal. This really
should return a string, something like "normal" / "reversed", and in
case the orientation is TYPEC_ORIENTATION_NONE, empty string.

>  static struct attribute *typec_attrs[] = {
>  	&dev_attr_data_role.attr,
>  	&dev_attr_power_operation_mode.attr,
> @@ -1248,6 +1258,7 @@ static struct attribute *typec_attrs[] = {
>  	&dev_attr_usb_typec_revision.attr,
>  	&dev_attr_vconn_source.attr,
>  	&dev_attr_port_type.attr,
> +	&dev_attr_cc_orientation.attr,
>  	NULL,
>  };
>  ATTRIBUTE_GROUPS(typec);
> -- 
> 2.23.0.700.g56cf767bdb-goog

thanks,

-- 
heikki

Re: [PATCH] usb: typec: Add sysfs node to show cc orientation

[Heikki Krogerus]

On Wed, Oct 16, 2019 at 03:58:50PM +0300, Heikki Krogerus wrote:
> On Wed, Oct 16, 2019 at 11:43:14AM +0800, pumahsu wrote:
> > Export the Type-C cc orientation so that user space can
> > get this information.
> 
> For what do you need this information in user space? I'm guessing you
> have something else in mind besides exposing this as just generic
> information, or debugging purposes, no?
> 
> Please keep in mind that we do not always know the cable orientation.
> UCSI for example does not give any clues about which way the cable
> plug was connected to the connector. That means this sysfs file will
> most likely need to be hidden in those cases, which I guess is
> acceptable, but definitely not ideal.
> 
> > Signed-off-by: pumahsu <pumahsu@google.com>
> > ---
> >  Documentation/ABI/testing/sysfs-class-typec |  7 +++++++
> >  drivers/usb/typec/class.c                   | 11 +++++++++++
> >  2 files changed, 18 insertions(+)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> > index d7647b258c3c..419f952c991d 100644
> > --- a/Documentation/ABI/testing/sysfs-class-typec
> > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > @@ -108,6 +108,13 @@ Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
> >  Description:
> >  		Revision number of the supported USB Type-C specification.
> >  
> > +What:		/sys/class/typec/<port>/cc_orientation
> > +Date:		September 2019
> > +Contact:	Puma Hsu <pumahsu@google.com>
> > +Description:
> > +		Indicates which cc orientation is active now, or 0 when
> > +		nothing is connected.
> 
> cc_orientation is a bit cryptic. I think if this is part of the port
> ABI, then we should talk about something like "connector_orientation".
> 
> >  USB Type-C partner devices (eg. /sys/class/typec/port0-partner/)
> >  
> > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > index 7d8805d9bf37..00edae63da8e 100644
> > --- a/drivers/usb/typec/class.c
> > +++ b/drivers/usb/typec/class.c
> > @@ -1238,6 +1238,16 @@ static ssize_t usb_power_delivery_revision_show(struct device *dev,
> >  }
> >  static DEVICE_ATTR_RO(usb_power_delivery_revision);
> >  
> > +static ssize_t cc_orientation_show(struct device *dev,
> > +						struct device_attribute *attr,
> > +						char *buf)
> > +{
> > +	struct typec_port *p = to_typec_port(dev);
> > +
> > +	return sprintf(buf, "%d\n", typec_get_orientation(p));
> > +}
> > +static DEVICE_ATTR_RO(cc_orientation);
> 
> Now you are returning 0, 1 or 2 which to me is not ideal. This really
> should return a string, something like "normal" / "reversed", and in
> case the orientation is TYPEC_ORIENTATION_NONE, empty string.

Or maybe TYPEC_ORIENTATION_NONE could be handle with "unknown" string.
That way we may not need to hide the file.

thanks,

-- 
heikki

Re: [PATCH] usb: typec: Add sysfs node to show cc orientation

[Puma Hsu]

Hi Heikki,

It’s not necessary to know the cable orientation for a normal user,
but we can have statistical analysis at Application layer. For example,
it may help investigating user behavior in the future if we can have the
count of plugging direction.

Besides, we also want to make the unified way to provide the connector
and connection information to user space. For the manufacturers, they
can easily record both orientations of the USB connector have been
verified working by an application tool. For a product, it’s system application
can diagnose itself that one orientation of the USB connector cannot work
anymore when there is hardware damage.

For the coding algorithm, I will upload the patch version 2 for reviewing.

Thanks in advance.
Puma Hsu






Thanks in advance.




  •  Puma Hsu 許誌宏
  •  Software Engineer, Pixel Phone
  •  Tel: +886 2 8729 0870
  •  pumahsu@google.com





On Wed, Oct 16, 2019 at 9:10 PM Heikki Krogerus
<heikki.krogerus@linux.intel.com> wrote:
>
> On Wed, Oct 16, 2019 at 03:58:50PM +0300, Heikki Krogerus wrote:
> > On Wed, Oct 16, 2019 at 11:43:14AM +0800, pumahsu wrote:
> > > Export the Type-C cc orientation so that user space can
> > > get this information.
> >
> > For what do you need this information in user space? I'm guessing you
> > have something else in mind besides exposing this as just generic
> > information, or debugging purposes, no?
> >
> > Please keep in mind that we do not always know the cable orientation.
> > UCSI for example does not give any clues about which way the cable
> > plug was connected to the connector. That means this sysfs file will
> > most likely need to be hidden in those cases, which I guess is
> > acceptable, but definitely not ideal.
> >
> > > Signed-off-by: pumahsu <pumahsu@google.com>
> > > ---
> > >  Documentation/ABI/testing/sysfs-class-typec |  7 +++++++
> > >  drivers/usb/typec/class.c                   | 11 +++++++++++
> > >  2 files changed, 18 insertions(+)
> > >
> > > diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> > > index d7647b258c3c..419f952c991d 100644
> > > --- a/Documentation/ABI/testing/sysfs-class-typec
> > > +++ b/Documentation/ABI/testing/sysfs-class-typec
> > > @@ -108,6 +108,13 @@ Contact:       Heikki Krogerus <heikki.krogerus@linux.intel.com>
> > >  Description:
> > >             Revision number of the supported USB Type-C specification.
> > >
> > > +What:              /sys/class/typec/<port>/cc_orientation
> > > +Date:              September 2019
> > > +Contact:   Puma Hsu <pumahsu@google.com>
> > > +Description:
> > > +           Indicates which cc orientation is active now, or 0 when
> > > +           nothing is connected.
> >
> > cc_orientation is a bit cryptic. I think if this is part of the port
> > ABI, then we should talk about something like "connector_orientation".
> >
> > >  USB Type-C partner devices (eg. /sys/class/typec/port0-partner/)
> > >
> > > diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> > > index 7d8805d9bf37..00edae63da8e 100644
> > > --- a/drivers/usb/typec/class.c
> > > +++ b/drivers/usb/typec/class.c
> > > @@ -1238,6 +1238,16 @@ static ssize_t usb_power_delivery_revision_show(struct device *dev,
> > >  }
> > >  static DEVICE_ATTR_RO(usb_power_delivery_revision);
> > >
> > > +static ssize_t cc_orientation_show(struct device *dev,
> > > +                                           struct device_attribute *attr,
> > > +                                           char *buf)
> > > +{
> > > +   struct typec_port *p = to_typec_port(dev);
> > > +
> > > +   return sprintf(buf, "%d\n", typec_get_orientation(p));
> > > +}
> > > +static DEVICE_ATTR_RO(cc_orientation);
> >
> > Now you are returning 0, 1 or 2 which to me is not ideal. This really
> > should return a string, something like "normal" / "reversed", and in
> > case the orientation is TYPEC_ORIENTATION_NONE, empty string.
>
> Or maybe TYPEC_ORIENTATION_NONE could be handle with "unknown" string.
> That way we may not need to hide the file.
>
> thanks,
>
> --
> heikki