Thread (20 messages) 20 messages, 5 authors, 2021-02-04

Re: [PATCH v5 1/8] usb: typec: Manage SVDM version

From: Heikki Krogerus <heikki.krogerus@linux.intel.com>
Date: 2021-02-03 12:52:39
Also in: linux-usb, lkml

Hi Kyle,

On Wed, Feb 03, 2021 at 12:17:26AM +0800, Kyle Tso wrote:
quoted hunk ↗ jump to hunk
PD Spec Revision 3.0 Version 2.0 + ECNs 2020-12-10
  6.4.4.2.3 Structured VDM Version
  "The Structured VDM Version field of the Discover Identity Command
  sent and received during VDM discovery Shall be used to determine the
  lowest common Structured VDM Version supported by the Port Partners or
  Cable Plug and Shall continue to operate using this Specification
  Revision until they are Detached."

Add a variable in typec_capability to specify the highest SVDM version
supported by the port and another variable in typec_port to cache the
negotiated SVDM version between the port partners.

Also add setter/getter functions for the negotiated SVDM version.

Signed-off-by: Kyle Tso <redacted>
---
 drivers/usb/typec/class.c | 13 +++++++++++++
 include/linux/usb/typec.h | 10 ++++++++++
 2 files changed, 23 insertions(+)
diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index b6ceab3dc16b..42d1be1eece9 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -51,6 +51,7 @@ struct typec_port {
 	enum typec_role			vconn_role;
 	enum typec_pwr_opmode		pwr_opmode;
 	enum typec_port_type		port_type;
+	enum usb_pd_svdm_ver		svdm_version;
 	struct mutex			port_type_lock;
I just realized that you are storing that in the port object. I guess
we don't have to change this right now, but it would have been more
clear to store that in the partner object IMO.
quoted hunk ↗ jump to hunk
 	enum typec_orientation		orientation;
@@ -1841,6 +1842,18 @@ int typec_find_port_data_role(const char *name)
 }
 EXPORT_SYMBOL_GPL(typec_find_port_data_role);
 
+void typec_set_svdm_version(struct typec_port *port, enum usb_pd_svdm_ver ver)
+{
+	port->svdm_version = ver;
+}
+EXPORT_SYMBOL_GPL(typec_set_svdm_version);
+
+enum usb_pd_svdm_ver typec_get_svdm_version(struct typec_port *port)
+{
+	return port->svdm_version;
+}
+EXPORT_SYMBOL_GPL(typec_get_svdm_version);
You need to document those exported functions! You need to do that in
any case, but in this case it's very important, because the purpose of
these functions is not clear from the ctx.

I'm sorry for noticing that so late. Since you do need to fix that,
please see if you can also store that detail in the partner device
object instead of the port object.

thanks,

-- 
heikki
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help