linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] typec: Provide USB PD Specification Revision for cable and partner
@ 2020-10-30  4:42 Benson Leung
  2020-10-30 10:34 ` Greg KH
  2020-11-02 13:32 ` kernel test robot
  0 siblings, 2 replies; 3+ messages in thread
From: Benson Leung @ 2020-10-30  4:42 UTC (permalink / raw)
  To: heikki.krogerus
  Cc: heikki.krogerus, gregkh, bleung, bleung, linux-usb, linux-kernel,
	pmalani, bleung

The USB Power Delivery specification Section 6.2.1.1.5 outlines
revision backward compatibility requirements starting from Revision 3.0.

The Port, the Cable Plug, and the Port Partner may support either revision
2 or revision 3 individually, and communication between ports, partners,
and cables of different revisions are allowed under rules that the parties
agree to communicate between each other using the lowest common operating
revision.

This may mean that Port-to-Port operating revision comms may be different
than Port-to-CablePlug operating revision comms. For example, it is
possible for a R3.0 port to communicate with a R3.0 cable using R3.0
messages, while the R3.0 port (in the same session) must communicate with
a R2.0 partner using PD R2.0 messages only.

This change will introduce individual revision number tracking for cable
and port partner so that the port can track them independently.

This will also enable future changes which change cable identifier
decoding and visible sysfs nodes based on revision 2 or 3.

Signed-off-by: Benson Leung <bleung@chromium.org>
---
 Documentation/ABI/testing/sysfs-class-typec |  8 +++++
 drivers/usb/typec/class.c                   | 38 +++++++++++++++------
 include/linux/usb/typec.h                   | 10 ++++++
 3 files changed, 46 insertions(+), 10 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
index b834671522d6..740b226bb80e 100644
--- a/Documentation/ABI/testing/sysfs-class-typec
+++ b/Documentation/ABI/testing/sysfs-class-typec
@@ -102,6 +102,14 @@ Description:
 		Revision number of the supported USB Power Delivery
 		specification, or 0 when USB Power Delivery is not supported.
 
+What:		/sys/class/typec/<port>-{partner|cable}/usb_power_delivery_revision
+Date:		October 2020
+Contact:	Benson Leung <bleung@chromium.org>
+Description:
+		Revision number of the supported USB Power Delivery
+		specification of the port partner or cable, or 0 when USB Power
+		Delivery is not supported.
+
 What:		/sys/class/typec/<port>/usb_typec_revision
 Date:		April 2017
 Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index 35eec707cb51..abae4cbe66d5 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -25,6 +25,7 @@ struct typec_cable {
 	enum typec_plug_type		type;
 	struct usb_pd_identity		*identity;
 	unsigned int			active:1;
+	u16				pd_revision; /* 0300H = "3.0" */
 };
 
 struct typec_partner {
@@ -33,6 +34,7 @@ struct typec_partner {
 	struct usb_pd_identity		*identity;
 	enum typec_accessory		accessory;
 	struct ida			mode_ids;
+	u16				pd_revision; /* 0300H = "3.0" */
 };
 
 struct typec_port {
@@ -146,6 +148,28 @@ static void typec_report_identity(struct device *dev)
 	sysfs_notify(&dev->kobj, "identity", "product");
 }
 
+static ssize_t usb_power_delivery_revision_show(struct device *dev,
+						struct device_attribute *attr,
+						char *buf)
+{
+	if (is_typec_partner(dev)) {
+		struct typec_partner *partner = to_typec_partner(dev);
+
+		return sprintf(buf, "%d\n", (partner->pd_revision >> 8) & 0xff);
+	} else if (is_typec_cable(dev)) {
+		struct typec_cable *cable = to_typec_cable(dev);
+
+		return sprintf(buf, "%d\n", (cable->pd_revision >> 8) & 0xff);
+	} else if (is_typec_port(dev)) {
+		struct typec_port *p = to_typec_port(dev);
+
+		return sprintf(buf, "%d\n", (p->cap->pd_revision >> 8) & 0xff);
+	}
+}
+static DEVICE_ATTR_RO(usb_power_delivery_revision);
+
+
+
 /* ------------------------------------------------------------------------- */
 /* Alternate Modes */
 
@@ -535,6 +559,7 @@ static DEVICE_ATTR_RO(supports_usb_power_delivery);
 static struct attribute *typec_partner_attrs[] = {
 	&dev_attr_accessory_mode.attr,
 	&dev_attr_supports_usb_power_delivery.attr,
+	&dev_attr_usb_power_delivery_revision.attr,
 	NULL
 };
 ATTRIBUTE_GROUPS(typec_partner);
@@ -612,6 +637,7 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
 	ida_init(&partner->mode_ids);
 	partner->usb_pd = desc->usb_pd;
 	partner->accessory = desc->accessory;
+	partner->pd_revision = desc->pd_revision;
 
 	if (desc->identity) {
 		/*
@@ -773,6 +799,7 @@ static DEVICE_ATTR_RO(plug_type);
 static struct attribute *typec_cable_attrs[] = {
 	&dev_attr_type.attr,
 	&dev_attr_plug_type.attr,
+	&dev_attr_usb_power_delivery_revision.attr,
 	NULL
 };
 ATTRIBUTE_GROUPS(typec_cable);
@@ -875,6 +902,7 @@ struct typec_cable *typec_register_cable(struct typec_port *port,
 
 	cable->type = desc->type;
 	cable->active = desc->active;
+	cable->pd_revision = desc->pd_revision;
 
 	if (desc->identity) {
 		/*
@@ -1240,16 +1268,6 @@ static ssize_t usb_typec_revision_show(struct device *dev,
 }
 static DEVICE_ATTR_RO(usb_typec_revision);
 
-static ssize_t usb_power_delivery_revision_show(struct device *dev,
-						struct device_attribute *attr,
-						char *buf)
-{
-	struct typec_port *p = to_typec_port(dev);
-
-	return sprintf(buf, "%d\n", (p->cap->pd_revision >> 8) & 0xff);
-}
-static DEVICE_ATTR_RO(usb_power_delivery_revision);
-
 static ssize_t orientation_show(struct device *dev,
 				   struct device_attribute *attr,
 				   char *buf)
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index 6be558045942..2306d40e18a1 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -162,6 +162,7 @@ struct typec_plug_desc {
  * @type: The plug type from USB PD Cable VDO
  * @active: Is the cable active or passive
  * @identity: Result of Discover Identity command
+ * @pd_revision: USB Power Delivery Specification revision if supported
  *
  * Represents USB Type-C Cable attached to USB Type-C port.
  */
@@ -169,6 +170,8 @@ struct typec_cable_desc {
 	enum typec_plug_type	type;
 	unsigned int		active:1;
 	struct usb_pd_identity	*identity;
+	u16			pd_revision; /* 0300H = "3.0" */
+
 };
 
 /*
@@ -176,15 +179,22 @@ struct typec_cable_desc {
  * @usb_pd: USB Power Delivery support
  * @accessory: Audio, Debug or none.
  * @identity: Discover Identity command data
+ * @pd_revision: USB Power Delivery Specification Revision if supported
  *
  * Details about a partner that is attached to USB Type-C port. If @identity
  * member exists when partner is registered, a directory named "identity" is
  * created to sysfs for the partner device.
+ *
+ * @pd_revision is based on the setting of the "Specification Revision" field
+ * in the message header on the initial "Source Capabilities" message receieved
+ * from the partner, or a "Request" message received from the partner, depending
+ * on whether our port is a Sink or a Source.
  */
 struct typec_partner_desc {
 	unsigned int		usb_pd:1;
 	enum typec_accessory	accessory;
 	struct usb_pd_identity	*identity;
+	u16			pd_revision; /* 0300H = "3.0" */
 };
 
 /**
-- 
2.29.1.341.ge80a0c044ae-goog


-- 
Benson Leung
Staff Software Engineer
Chrome OS Kernel
Google Inc.
bleung@google.com
Chromium OS Project
bleung@chromium.org

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] typec: Provide USB PD Specification Revision for cable and partner
  2020-10-30  4:42 [PATCH] typec: Provide USB PD Specification Revision for cable and partner Benson Leung
@ 2020-10-30 10:34 ` Greg KH
  2020-11-02 13:32 ` kernel test robot
  1 sibling, 0 replies; 3+ messages in thread
From: Greg KH @ 2020-10-30 10:34 UTC (permalink / raw)
  To: Benson Leung
  Cc: heikki.krogerus, bleung, linux-usb, linux-kernel, pmalani, bleung

On Thu, Oct 29, 2020 at 09:42:24PM -0700, Benson Leung wrote:
> The USB Power Delivery specification Section 6.2.1.1.5 outlines
> revision backward compatibility requirements starting from Revision 3.0.
> 
> The Port, the Cable Plug, and the Port Partner may support either revision
> 2 or revision 3 individually, and communication between ports, partners,
> and cables of different revisions are allowed under rules that the parties
> agree to communicate between each other using the lowest common operating
> revision.
> 
> This may mean that Port-to-Port operating revision comms may be different
> than Port-to-CablePlug operating revision comms. For example, it is
> possible for a R3.0 port to communicate with a R3.0 cable using R3.0
> messages, while the R3.0 port (in the same session) must communicate with
> a R2.0 partner using PD R2.0 messages only.
> 
> This change will introduce individual revision number tracking for cable
> and port partner so that the port can track them independently.
> 
> This will also enable future changes which change cable identifier
> decoding and visible sysfs nodes based on revision 2 or 3.
> 
> Signed-off-by: Benson Leung <bleung@chromium.org>
> ---
>  Documentation/ABI/testing/sysfs-class-typec |  8 +++++
>  drivers/usb/typec/class.c                   | 38 +++++++++++++++------
>  include/linux/usb/typec.h                   | 10 ++++++
>  3 files changed, 46 insertions(+), 10 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-class-typec b/Documentation/ABI/testing/sysfs-class-typec
> index b834671522d6..740b226bb80e 100644
> --- a/Documentation/ABI/testing/sysfs-class-typec
> +++ b/Documentation/ABI/testing/sysfs-class-typec
> @@ -102,6 +102,14 @@ Description:
>  		Revision number of the supported USB Power Delivery
>  		specification, or 0 when USB Power Delivery is not supported.
>  
> +What:		/sys/class/typec/<port>-{partner|cable}/usb_power_delivery_revision
> +Date:		October 2020
> +Contact:	Benson Leung <bleung@chromium.org>
> +Description:
> +		Revision number of the supported USB Power Delivery
> +		specification of the port partner or cable, or 0 when USB Power
> +		Delivery is not supported.
> +
>  What:		/sys/class/typec/<port>/usb_typec_revision
>  Date:		April 2017
>  Contact:	Heikki Krogerus <heikki.krogerus@linux.intel.com>
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index 35eec707cb51..abae4cbe66d5 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -25,6 +25,7 @@ struct typec_cable {
>  	enum typec_plug_type		type;
>  	struct usb_pd_identity		*identity;
>  	unsigned int			active:1;
> +	u16				pd_revision; /* 0300H = "3.0" */
>  };
>  
>  struct typec_partner {
> @@ -33,6 +34,7 @@ struct typec_partner {
>  	struct usb_pd_identity		*identity;
>  	enum typec_accessory		accessory;
>  	struct ida			mode_ids;
> +	u16				pd_revision; /* 0300H = "3.0" */
>  };
>  
>  struct typec_port {
> @@ -146,6 +148,28 @@ static void typec_report_identity(struct device *dev)
>  	sysfs_notify(&dev->kobj, "identity", "product");
>  }
>  
> +static ssize_t usb_power_delivery_revision_show(struct device *dev,
> +						struct device_attribute *attr,
> +						char *buf)
> +{
> +	if (is_typec_partner(dev)) {
> +		struct typec_partner *partner = to_typec_partner(dev);
> +
> +		return sprintf(buf, "%d\n", (partner->pd_revision >> 8) & 0xff);
> +	} else if (is_typec_cable(dev)) {
> +		struct typec_cable *cable = to_typec_cable(dev);
> +
> +		return sprintf(buf, "%d\n", (cable->pd_revision >> 8) & 0xff);
> +	} else if (is_typec_port(dev)) {
> +		struct typec_port *p = to_typec_port(dev);
> +
> +		return sprintf(buf, "%d\n", (p->cap->pd_revision >> 8) & 0xff);

nit, you can use sysfs_emit() for the sprintf() calls in sysfs
attributes now.

> +	}
> +}
> +static DEVICE_ATTR_RO(usb_power_delivery_revision);
> +
> +
> +

2 extra blank lines are not needed :(

>  /* ------------------------------------------------------------------------- */
>  /* Alternate Modes */
>  
> @@ -535,6 +559,7 @@ static DEVICE_ATTR_RO(supports_usb_power_delivery);
>  static struct attribute *typec_partner_attrs[] = {
>  	&dev_attr_accessory_mode.attr,
>  	&dev_attr_supports_usb_power_delivery.attr,
> +	&dev_attr_usb_power_delivery_revision.attr,
>  	NULL
>  };
>  ATTRIBUTE_GROUPS(typec_partner);
> @@ -612,6 +637,7 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
>  	ida_init(&partner->mode_ids);
>  	partner->usb_pd = desc->usb_pd;
>  	partner->accessory = desc->accessory;
> +	partner->pd_revision = desc->pd_revision;
>  
>  	if (desc->identity) {
>  		/*
> @@ -773,6 +799,7 @@ static DEVICE_ATTR_RO(plug_type);
>  static struct attribute *typec_cable_attrs[] = {
>  	&dev_attr_type.attr,
>  	&dev_attr_plug_type.attr,
> +	&dev_attr_usb_power_delivery_revision.attr,
>  	NULL
>  };
>  ATTRIBUTE_GROUPS(typec_cable);
> @@ -875,6 +902,7 @@ struct typec_cable *typec_register_cable(struct typec_port *port,
>  
>  	cable->type = desc->type;
>  	cable->active = desc->active;
> +	cable->pd_revision = desc->pd_revision;
>  
>  	if (desc->identity) {
>  		/*
> @@ -1240,16 +1268,6 @@ static ssize_t usb_typec_revision_show(struct device *dev,
>  }
>  static DEVICE_ATTR_RO(usb_typec_revision);
>  
> -static ssize_t usb_power_delivery_revision_show(struct device *dev,
> -						struct device_attribute *attr,
> -						char *buf)
> -{
> -	struct typec_port *p = to_typec_port(dev);
> -
> -	return sprintf(buf, "%d\n", (p->cap->pd_revision >> 8) & 0xff);
> -}
> -static DEVICE_ATTR_RO(usb_power_delivery_revision);
> -
>  static ssize_t orientation_show(struct device *dev,
>  				   struct device_attribute *attr,
>  				   char *buf)
> diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
> index 6be558045942..2306d40e18a1 100644
> --- a/include/linux/usb/typec.h
> +++ b/include/linux/usb/typec.h
> @@ -162,6 +162,7 @@ struct typec_plug_desc {
>   * @type: The plug type from USB PD Cable VDO
>   * @active: Is the cable active or passive
>   * @identity: Result of Discover Identity command
> + * @pd_revision: USB Power Delivery Specification revision if supported
>   *
>   * Represents USB Type-C Cable attached to USB Type-C port.
>   */
> @@ -169,6 +170,8 @@ struct typec_cable_desc {
>  	enum typec_plug_type	type;
>  	unsigned int		active:1;
>  	struct usb_pd_identity	*identity;
> +	u16			pd_revision; /* 0300H = "3.0" */
> +
>  };
>  
>  /*
> @@ -176,15 +179,22 @@ struct typec_cable_desc {
>   * @usb_pd: USB Power Delivery support
>   * @accessory: Audio, Debug or none.
>   * @identity: Discover Identity command data
> + * @pd_revision: USB Power Delivery Specification Revision if supported
>   *
>   * Details about a partner that is attached to USB Type-C port. If @identity
>   * member exists when partner is registered, a directory named "identity" is
>   * created to sysfs for the partner device.
> + *
> + * @pd_revision is based on the setting of the "Specification Revision" field
> + * in the message header on the initial "Source Capabilities" message receieved
> + * from the partner, or a "Request" message received from the partner, depending
> + * on whether our port is a Sink or a Source.
>   */
>  struct typec_partner_desc {
>  	unsigned int		usb_pd:1;
>  	enum typec_accessory	accessory;
>  	struct usb_pd_identity	*identity;
> +	u16			pd_revision; /* 0300H = "3.0" */

No endian issues?  Where are you reading this data from?  How is it in
"native cpu" endian already?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] typec: Provide USB PD Specification Revision for cable and partner
  2020-10-30  4:42 [PATCH] typec: Provide USB PD Specification Revision for cable and partner Benson Leung
  2020-10-30 10:34 ` Greg KH
@ 2020-11-02 13:32 ` kernel test robot
  1 sibling, 0 replies; 3+ messages in thread
From: kernel test robot @ 2020-11-02 13:32 UTC (permalink / raw)
  To: Benson Leung, heikki.krogerus
  Cc: kbuild-all, clang-built-linux, gregkh, bleung, bleung, linux-usb,
	linux-kernel, pmalani, bleung

[-- Attachment #1: Type: text/plain, Size: 2818 bytes --]

Hi Benson,

I love your patch! Yet something to improve:

[auto build test ERROR on usb/usb-testing]
[also build test ERROR on chrome-platform-linux/for-next linus/master v5.10-rc2 next-20201102]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Benson-Leung/typec-Provide-USB-PD-Specification-Revision-for-cable-and-partner/20201030-124250
base:   https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
config: arm-randconfig-r033-20201030 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project fa5a13276764a2657b3571fa3c57b07ee5d2d661)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install arm cross compiling tool for clang build
        # apt-get install binutils-arm-linux-gnueabi
        # https://github.com/0day-ci/linux/commit/dca7134400af678fe4f5e4d94a7ec74487d9fcc1
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Benson-Leung/typec-Provide-USB-PD-Specification-Revision-for-cable-and-partner/20201030-124250
        git checkout dca7134400af678fe4f5e4d94a7ec74487d9fcc1
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=arm 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/usb/typec/class.c:168:1: error: non-void function does not return a value in all control paths [-Werror,-Wreturn-type]
   }
   ^
   1 error generated.

vim +168 drivers/usb/typec/class.c

   150	
   151	static ssize_t usb_power_delivery_revision_show(struct device *dev,
   152							struct device_attribute *attr,
   153							char *buf)
   154	{
   155		if (is_typec_partner(dev)) {
   156			struct typec_partner *partner = to_typec_partner(dev);
   157	
   158			return sprintf(buf, "%d\n", (partner->pd_revision >> 8) & 0xff);
   159		} else if (is_typec_cable(dev)) {
   160			struct typec_cable *cable = to_typec_cable(dev);
   161	
   162			return sprintf(buf, "%d\n", (cable->pd_revision >> 8) & 0xff);
   163		} else if (is_typec_port(dev)) {
   164			struct typec_port *p = to_typec_port(dev);
   165	
   166			return sprintf(buf, "%d\n", (p->cap->pd_revision >> 8) & 0xff);
   167		}
 > 168	}
   169	static DEVICE_ATTR_RO(usb_power_delivery_revision);
   170	
   171	
   172	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 31864 bytes --]

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-11-02 13:33 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-30  4:42 [PATCH] typec: Provide USB PD Specification Revision for cable and partner Benson Leung
2020-10-30 10:34 ` Greg KH
2020-11-02 13:32 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).