linux-usb.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v6 0/7] common SVDM version and VDO from dt
@ 2021-02-05  3:34 Kyle Tso
  2021-02-05  3:34 ` [PATCHi v6 1/7] usb: typec: Manage SVDM version Kyle Tso
                   ` (7 more replies)
  0 siblings, 8 replies; 24+ messages in thread
From: Kyle Tso @ 2021-02-05  3:34 UTC (permalink / raw)
  To: linux, heikki.krogerus, gregkh, hdegoede, robh+dt
  Cc: badhri, linux-usb, linux-kernel, devicetree, Kyle Tso

v5 is here:
https://patchwork.kernel.org/project/linux-usb/cover/20210202161733.932215-1-kyletso@google.com/

Changes since v5:
=================
usb: typec: Manage SVDM version
- !! most changes are from Heikki
- location of the negotiated SVDM version is changed. Now the variable
  resides in typec_partner
- The setter and getter functions were modified according to the above
  changes
- the default SVDM version is stored upon calling to
  typec_register_partner

usb: pd: Make SVDM Version configurable in VDM header
- no change

usb: typec: tcpm: Determine common SVDM Version
- follow the changes of "usb: typec: Manage SVDM version"
- remove the "reset to default". Now the default SVDM version will be
  set when calling to typec_register_partner

usb: typec: ucsi: Determine common SVDM Version
- follow the changes of "usb: typec: Manage SVDM version"
- remove the "reset to default". Now the default SVDM version will be
  set when calling to typec_register_partner

usb: typec: displayport: Fill the negotiated SVDM Version in the header
- follow the changes of "usb: typec: Manage SVDM version"

dt-bindings: connector: Add SVDM VDO properties
- no change

usb: typec: tcpm: Get Sink VDO from fwnode
- no change

Kyle Tso (7):
  usb: typec: Manage SVDM version
  usb: pd: Make SVDM Version configurable in VDM header
  usb: typec: tcpm: Determine common SVDM Version
  usb: typec: ucsi: Determine common SVDM Version
  usb: typec: displayport: Fill the negotiated SVDM Version in the header
  dt-bindings: connector: Add SVDM VDO properties
  usb: typec: tcpm: Get Sink VDO from fwnode

 .../bindings/connector/usb-connector.yaml     |  11 +
 drivers/usb/typec/altmodes/displayport.c      |  17 +-
 drivers/usb/typec/class.c                     |  43 +++
 drivers/usb/typec/tcpm/tcpm.c                 |  85 ++++-
 drivers/usb/typec/ucsi/displayport.c          |  32 +-
 drivers/usb/typec/ucsi/ucsi.c                 |   1 +
 include/dt-bindings/usb/pd.h                  | 311 +++++++++++++++++-
 include/linux/usb/pd_vdo.h                    |   7 +-
 include/linux/usb/typec.h                     |  12 +
 include/linux/usb/typec_altmode.h             |  10 +
 10 files changed, 509 insertions(+), 20 deletions(-)

-- 
2.30.0.365.g02bc693789-goog


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

* [PATCHi v6 1/7] usb: typec: Manage SVDM version
  2021-02-05  3:34 [PATCH v6 0/7] common SVDM version and VDO from dt Kyle Tso
@ 2021-02-05  3:34 ` Kyle Tso
  2021-02-12  4:09   ` Guenter Roeck
  2021-02-05  3:34 ` [PATCH v6 2/7] usb: pd: Make SVDM Version configurable in VDM header Kyle Tso
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Kyle Tso @ 2021-02-05  3:34 UTC (permalink / raw)
  To: linux, heikki.krogerus, gregkh, hdegoede, robh+dt
  Cc: badhri, linux-usb, linux-kernel, devicetree, Kyle Tso

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_partner to cache the
negotiated SVDM version between the port and the partner.

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

Signed-off-by: Kyle Tso <kyletso@google.com>
---
Changes since v5
- !! most changes are from Heikki
- location of the negotiated SVDM version is changed. Now the variable
  resides in typec_partner
- The setter and getter functions were modified according to the above
  changes
- the default SVDM version is stored upon calling to
  typec_register_partner

 drivers/usb/typec/class.c         | 43 +++++++++++++++++++++++++++++++
 include/linux/usb/typec.h         | 12 +++++++++
 include/linux/usb/typec_altmode.h | 10 +++++++
 3 files changed, 65 insertions(+)

diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
index b4a5d9d4564f..45f0bf65e9ab 100644
--- a/drivers/usb/typec/class.c
+++ b/drivers/usb/typec/class.c
@@ -38,6 +38,7 @@ struct typec_partner {
 	struct ida			mode_ids;
 	int				num_altmodes;
 	u16				pd_revision; /* 0300H = "3.0" */
+	enum usb_pd_svdm_ver		svdm_version;
 };
 
 struct typec_port {
@@ -824,6 +825,20 @@ typec_partner_register_altmode(struct typec_partner *partner,
 }
 EXPORT_SYMBOL_GPL(typec_partner_register_altmode);
 
+/**
+ * typec_partner_set_svdm_version - Set negotiated Structured VDM (SVDM) Version
+ * @partner: USB Type-C Partner that supports SVDM
+ * @svdm_version: Negotiated SVDM Version
+ *
+ * This routine is used to save the negotiated SVDM Version.
+ */
+void typec_partner_set_svdm_version(struct typec_partner *partner,
+				   enum usb_pd_svdm_ver svdm_version)
+{
+	partner->svdm_version = svdm_version;
+}
+EXPORT_SYMBOL_GPL(typec_partner_set_svdm_version);
+
 /**
  * typec_register_partner - Register a USB Type-C Partner
  * @port: The USB Type-C Port the partner is connected to
@@ -848,6 +863,7 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
 	partner->accessory = desc->accessory;
 	partner->num_altmodes = -1;
 	partner->pd_revision = desc->pd_revision;
+	partner->svdm_version = port->cap->svdm_version;
 
 	if (desc->identity) {
 		/*
@@ -1894,6 +1910,33 @@ EXPORT_SYMBOL_GPL(typec_set_mode);
 
 /* --------------------------------------- */
 
+/**
+ * typec_get_negotiated_svdm_version - Get negotiated SVDM Version
+ * @port: USB Type-C Port.
+ *
+ * Get the negotiated SVDM Version. The Version is set to the port default
+ * value stored in typec_capability on partner registration, and updated after
+ * a successful Discover Identity if the negotiated value is less than the
+ * default value.
+ *
+ * Returns usb_pd_svdm_ver if the partner has been registered otherwise -ENODEV.
+ */
+int typec_get_negotiated_svdm_version(struct typec_port *port)
+{
+	enum usb_pd_svdm_ver svdm_version;
+	struct device *partner_dev;
+
+	partner_dev = device_find_child(&port->dev, NULL, partner_match);
+	if (!partner_dev)
+		return -ENODEV;
+
+	svdm_version = to_typec_partner(partner_dev)->svdm_version;
+	put_device(partner_dev);
+
+	return svdm_version;
+}
+EXPORT_SYMBOL_GPL(typec_get_negotiated_svdm_version);
+
 /**
  * typec_get_drvdata - Return private driver data pointer
  * @port: USB Type-C port
diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
index a94df82ab62f..91b4303ca305 100644
--- a/include/linux/usb/typec.h
+++ b/include/linux/usb/typec.h
@@ -217,12 +217,19 @@ struct typec_operations {
 			     enum typec_port_type type);
 };
 
+enum usb_pd_svdm_ver {
+	SVDM_VER_1_0 = 0,
+	SVDM_VER_2_0 = 1,
+	SVDM_VER_MAX = SVDM_VER_2_0,
+};
+
 /*
  * struct typec_capability - USB Type-C Port Capabilities
  * @type: Supported power role of the port
  * @data: Supported data role of the port
  * @revision: USB Type-C Specification release. Binary coded decimal
  * @pd_revision: USB Power Delivery Specification revision if supported
+ * @svdm_version: USB PD Structured VDM version if supported
  * @prefer_role: Initial role preference (DRP ports).
  * @accessory: Supported Accessory Modes
  * @fwnode: Optional fwnode of the port
@@ -236,6 +243,7 @@ struct typec_capability {
 	enum typec_port_data	data;
 	u16			revision; /* 0120H = "1.2" */
 	u16			pd_revision; /* 0300H = "3.0" */
+	enum usb_pd_svdm_ver	svdm_version;
 	int			prefer_role;
 	enum typec_accessory	accessory[TYPEC_MAX_ACCESSORY];
 	unsigned int		orientation_aware:1;
@@ -286,4 +294,8 @@ int typec_find_orientation(const char *name);
 int typec_find_port_power_role(const char *name);
 int typec_find_power_role(const char *name);
 int typec_find_port_data_role(const char *name);
+
+void typec_partner_set_svdm_version(struct typec_partner *partner,
+				    enum usb_pd_svdm_ver svdm_version);
+int typec_get_negotiated_svdm_version(struct typec_port *port);
 #endif /* __LINUX_USB_TYPEC_H */
diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index 5e0a7b7647c3..65933cbe9129 100644
--- a/include/linux/usb/typec_altmode.h
+++ b/include/linux/usb/typec_altmode.h
@@ -132,6 +132,16 @@ typec_altmode_get_orientation(struct typec_altmode *altmode)
 	return typec_get_orientation(typec_altmode2port(altmode));
 }
 
+/**
+ * typec_altmode_get_svdm_version - Get negotiated SVDM version
+ * @altmode: Handle to the alternate mode
+ */
+static inline int
+typec_altmode_get_svdm_version(struct typec_altmode *altmode)
+{
+	return typec_get_negotiated_svdm_version(typec_altmode2port(altmode));
+}
+
 /**
  * struct typec_altmode_driver - USB Type-C alternate mode device driver
  * @id_table: Null terminated array of SVIDs
-- 
2.30.0.365.g02bc693789-goog


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

* [PATCH v6 2/7] usb: pd: Make SVDM Version configurable in VDM header
  2021-02-05  3:34 [PATCH v6 0/7] common SVDM version and VDO from dt Kyle Tso
  2021-02-05  3:34 ` [PATCHi v6 1/7] usb: typec: Manage SVDM version Kyle Tso
@ 2021-02-05  3:34 ` Kyle Tso
  2021-02-12  4:09   ` Guenter Roeck
  2021-02-05  3:34 ` [PATCH v6 3/7] usb: typec: tcpm: Determine common SVDM Version Kyle Tso
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Kyle Tso @ 2021-02-05  3:34 UTC (permalink / raw)
  To: linux, heikki.krogerus, gregkh, hdegoede, robh+dt
  Cc: badhri, linux-usb, linux-kernel, devicetree, Kyle Tso

PD Rev 3.0 introduces SVDM Version 2.0. This patch makes the field
configuable in the header in order to be able to be compatible with
older SVDM version.

Signed-off-by: Kyle Tso <kyletso@google.com>
---
Changes since v5:
- no change

 drivers/usb/typec/altmodes/displayport.c |  2 +-
 drivers/usb/typec/tcpm/tcpm.c            | 16 ++++++++--------
 drivers/usb/typec/ucsi/displayport.c     |  6 +++---
 include/linux/usb/pd_vdo.h               |  7 +++++--
 4 files changed, 17 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
index e62e5e3da01e..0abc3121238f 100644
--- a/drivers/usb/typec/altmodes/displayport.c
+++ b/drivers/usb/typec/altmodes/displayport.c
@@ -15,7 +15,7 @@
 #include <linux/usb/typec_dp.h>
 #include "displayport.h"
 
-#define DP_HEADER(_dp, cmd)		(VDO((_dp)->alt->svid, 1, cmd) | \
+#define DP_HEADER(_dp, cmd)		(VDO((_dp)->alt->svid, 1, SVDM_VER_1_0, cmd) | \
 					 VDO_OPOS(USB_TYPEC_DP_MODE))
 
 enum {
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 8558ab006885..9aadb1e1bec5 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -1544,17 +1544,17 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
 		case CMD_DISCOVER_IDENT:
 			/* 6.4.4.3.1 */
 			svdm_consume_identity(port, p, cnt);
-			response[0] = VDO(USB_SID_PD, 1, CMD_DISCOVER_SVID);
+			response[0] = VDO(USB_SID_PD, 1, SVDM_VER_1_0, CMD_DISCOVER_SVID);
 			rlen = 1;
 			break;
 		case CMD_DISCOVER_SVID:
 			/* 6.4.4.3.2 */
 			if (svdm_consume_svids(port, p, cnt)) {
-				response[0] = VDO(USB_SID_PD, 1,
+				response[0] = VDO(USB_SID_PD, 1, SVDM_VER_1_0,
 						  CMD_DISCOVER_SVID);
 				rlen = 1;
 			} else if (modep->nsvids && supports_modal(port)) {
-				response[0] = VDO(modep->svids[0], 1,
+				response[0] = VDO(modep->svids[0], 1, SVDM_VER_1_0,
 						  CMD_DISCOVER_MODES);
 				rlen = 1;
 			}
@@ -1565,7 +1565,7 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
 			modep->svid_index++;
 			if (modep->svid_index < modep->nsvids) {
 				u16 svid = modep->svids[modep->svid_index];
-				response[0] = VDO(svid, 1, CMD_DISCOVER_MODES);
+				response[0] = VDO(svid, 1, SVDM_VER_1_0, CMD_DISCOVER_MODES);
 				rlen = 1;
 			} else {
 				tcpm_register_partner_altmodes(port);
@@ -1695,7 +1695,7 @@ static void tcpm_handle_vdm_request(struct tcpm_port *port,
 			break;
 		case ADEV_QUEUE_VDM_SEND_EXIT_MODE_ON_FAIL:
 			if (typec_altmode_vdm(adev, p[0], &p[1], cnt)) {
-				response[0] = VDO(adev->svid, 1, CMD_EXIT_MODE);
+				response[0] = VDO(adev->svid, 1, SVDM_VER_1_0, CMD_EXIT_MODE);
 				response[0] |= VDO_OPOS(adev->mode);
 				rlen = 1;
 			}
@@ -1729,7 +1729,7 @@ static void tcpm_send_vdm(struct tcpm_port *port, u32 vid, int cmd,
 
 	/* set VDM header with VID & CMD */
 	header = VDO(vid, ((vid & USB_SID_PD) == USB_SID_PD) ?
-			1 : (PD_VDO_CMD(cmd) <= CMD_ATTENTION), cmd);
+			1 : (PD_VDO_CMD(cmd) <= CMD_ATTENTION), SVDM_VER_1_0, cmd);
 	tcpm_queue_vdm(port, header, data, count);
 }
 
@@ -2024,7 +2024,7 @@ static int tcpm_altmode_enter(struct typec_altmode *altmode, u32 *vdo)
 	struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
 	u32 header;
 
-	header = VDO(altmode->svid, vdo ? 2 : 1, CMD_ENTER_MODE);
+	header = VDO(altmode->svid, vdo ? 2 : 1, SVDM_VER_1_0, CMD_ENTER_MODE);
 	header |= VDO_OPOS(altmode->mode);
 
 	tcpm_queue_vdm_unlocked(port, header, vdo, vdo ? 1 : 0);
@@ -2036,7 +2036,7 @@ static int tcpm_altmode_exit(struct typec_altmode *altmode)
 	struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
 	u32 header;
 
-	header = VDO(altmode->svid, 1, CMD_EXIT_MODE);
+	header = VDO(altmode->svid, 1, SVDM_VER_1_0, CMD_EXIT_MODE);
 	header |= VDO_OPOS(altmode->mode);
 
 	tcpm_queue_vdm_unlocked(port, header, NULL, 0);
diff --git a/drivers/usb/typec/ucsi/displayport.c b/drivers/usb/typec/ucsi/displayport.c
index 261131c9e37c..1d387bddefb9 100644
--- a/drivers/usb/typec/ucsi/displayport.c
+++ b/drivers/usb/typec/ucsi/displayport.c
@@ -83,7 +83,7 @@ static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo)
 	 * mode, and letting the alt mode driver continue.
 	 */
 
-	dp->header = VDO(USB_TYPEC_DP_SID, 1, CMD_ENTER_MODE);
+	dp->header = VDO(USB_TYPEC_DP_SID, 1, SVDM_VER_1_0, CMD_ENTER_MODE);
 	dp->header |= VDO_OPOS(USB_TYPEC_DP_MODE);
 	dp->header |= VDO_CMDT(CMDT_RSP_ACK);
 
@@ -120,7 +120,7 @@ static int ucsi_displayport_exit(struct typec_altmode *alt)
 	if (ret < 0)
 		goto out_unlock;
 
-	dp->header = VDO(USB_TYPEC_DP_SID, 1, CMD_EXIT_MODE);
+	dp->header = VDO(USB_TYPEC_DP_SID, 1, SVDM_VER_1_0, CMD_EXIT_MODE);
 	dp->header |= VDO_OPOS(USB_TYPEC_DP_MODE);
 	dp->header |= VDO_CMDT(CMDT_RSP_ACK);
 
@@ -200,7 +200,7 @@ static int ucsi_displayport_vdm(struct typec_altmode *alt,
 
 	switch (cmd_type) {
 	case CMDT_INIT:
-		dp->header = VDO(USB_TYPEC_DP_SID, 1, cmd);
+		dp->header = VDO(USB_TYPEC_DP_SID, 1, SVDM_VER_1_0, cmd);
 		dp->header |= VDO_OPOS(USB_TYPEC_DP_MODE);
 
 		switch (cmd) {
diff --git a/include/linux/usb/pd_vdo.h b/include/linux/usb/pd_vdo.h
index 5de7f550f93e..b057250704e8 100644
--- a/include/linux/usb/pd_vdo.h
+++ b/include/linux/usb/pd_vdo.h
@@ -21,22 +21,24 @@
  * ----------
  * <31:16>  :: SVID
  * <15>     :: VDM type ( 1b == structured, 0b == unstructured )
- * <14:13>  :: Structured VDM version (can only be 00 == 1.0 currently)
+ * <14:13>  :: Structured VDM version
  * <12:11>  :: reserved
  * <10:8>   :: object position (1-7 valid ... used for enter/exit mode only)
  * <7:6>    :: command type (SVDM only?)
  * <5>      :: reserved (SVDM), command type (UVDM)
  * <4:0>    :: command
  */
-#define VDO(vid, type, custom)				\
+#define VDO(vid, type, ver, custom)			\
 	(((vid) << 16) |				\
 	 ((type) << 15) |				\
+	 ((ver) << 13) |				\
 	 ((custom) & 0x7FFF))
 
 #define VDO_SVDM_TYPE		(1 << 15)
 #define VDO_SVDM_VERS(x)	((x) << 13)
 #define VDO_OPOS(x)		((x) << 8)
 #define VDO_CMDT(x)		((x) << 6)
+#define VDO_SVDM_VERS_MASK	VDO_SVDM_VERS(0x3)
 #define VDO_OPOS_MASK		VDO_OPOS(0x7)
 #define VDO_CMDT_MASK		VDO_CMDT(0x3)
 
@@ -74,6 +76,7 @@
 
 #define PD_VDO_VID(vdo)		((vdo) >> 16)
 #define PD_VDO_SVDM(vdo)	(((vdo) >> 15) & 1)
+#define PD_VDO_SVDM_VER(vdo)	(((vdo) >> 13) & 0x3)
 #define PD_VDO_OPOS(vdo)	(((vdo) >> 8) & 0x7)
 #define PD_VDO_CMD(vdo)		((vdo) & 0x1f)
 #define PD_VDO_CMDT(vdo)	(((vdo) >> 6) & 0x3)
-- 
2.30.0.365.g02bc693789-goog


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

* [PATCH v6 3/7] usb: typec: tcpm: Determine common SVDM Version
  2021-02-05  3:34 [PATCH v6 0/7] common SVDM version and VDO from dt Kyle Tso
  2021-02-05  3:34 ` [PATCHi v6 1/7] usb: typec: Manage SVDM version Kyle Tso
  2021-02-05  3:34 ` [PATCH v6 2/7] usb: pd: Make SVDM Version configurable in VDM header Kyle Tso
@ 2021-02-05  3:34 ` Kyle Tso
  2021-02-12  4:17   ` Guenter Roeck
  2021-02-12 15:20   ` Guenter Roeck
  2021-02-05  3:34 ` [PATCH v6 4/7] usb: typec: ucsi: " Kyle Tso
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 24+ messages in thread
From: Kyle Tso @ 2021-02-05  3:34 UTC (permalink / raw)
  To: linux, heikki.krogerus, gregkh, hdegoede, robh+dt
  Cc: badhri, linux-usb, linux-kernel, devicetree, Kyle Tso

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."

Also clear the fields newly defined in SVDM version 2.0 if the
negotiated SVDM version is 1.0.

Signed-off-by: Kyle Tso <kyletso@google.com>
---
Changes since v5:
- follow the changes of "usb: typec: Manage SVDM version"
- remove the "reset to default". Now the default SVDM version will be
  set when calling to typec_register_partner

 drivers/usb/typec/tcpm/tcpm.c | 71 ++++++++++++++++++++++++++++++-----
 1 file changed, 61 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 9aadb1e1bec5..b45cd191a8a4 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -1475,8 +1475,10 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
 			const u32 *p, int cnt, u32 *response,
 			enum adev_actions *adev_action)
 {
+	struct typec_port *typec = port->typec_port;
 	struct typec_altmode *pdev;
 	struct pd_mode_data *modep;
+	int svdm_version;
 	int rlen = 0;
 	int cmd_type;
 	int cmd;
@@ -1493,6 +1495,10 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
 	pdev = typec_match_altmode(port->partner_altmode, ALTMODE_DISCOVERY_MAX,
 				   PD_VDO_VID(p[0]), PD_VDO_OPOS(p[0]));
 
+	svdm_version = typec_get_negotiated_svdm_version(typec);
+	if (svdm_version < 0)
+		return 0;
+
 	switch (cmd_type) {
 	case CMDT_INIT:
 		switch (cmd) {
@@ -1500,10 +1506,22 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
 			if (PD_VDO_VID(p[0]) != USB_SID_PD)
 				break;
 
+			if (PD_VDO_SVDM_VER(p[0]) < svdm_version)
+				typec_partner_set_svdm_version(port->partner,
+							       PD_VDO_SVDM_VER(p[0]));
 			/* 6.4.4.3.1: Only respond as UFP (device) */
 			if (port->data_role == TYPEC_DEVICE &&
 			    port->nr_snk_vdo) {
-				for (i = 0; i <  port->nr_snk_vdo; i++)
+				/*
+				 * Product Type DFP and Connector Type are not defined in SVDM
+				 * version 1.0 and shall be set to zero.
+				 */
+				if (typec_get_negotiated_svdm_version(typec) < SVDM_VER_2_0)
+					response[1] = port->snk_vdo[0] & ~IDH_DFP_MASK
+						      & ~IDH_CONN_MASK;
+				else
+					response[1] = port->snk_vdo[0];
+				for (i = 1; i <  port->nr_snk_vdo; i++)
 					response[i + 1] = port->snk_vdo[i];
 				rlen = port->nr_snk_vdo + 1;
 			}
@@ -1532,6 +1550,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
 			response[0] = p[0] | VDO_CMDT(CMDT_RSP_BUSY);
 			rlen = 1;
 		}
+		response[0] = (response[0] & ~VDO_SVDM_VERS_MASK) |
+			      (VDO_SVDM_VERS(typec_get_negotiated_svdm_version(typec)));
 		break;
 	case CMDT_RSP_ACK:
 		/* silently drop message if we are not connected */
@@ -1542,19 +1562,22 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
 
 		switch (cmd) {
 		case CMD_DISCOVER_IDENT:
+			if (PD_VDO_SVDM_VER(p[0]) < svdm_version)
+				typec_partner_set_svdm_version(port->partner,
+							       PD_VDO_SVDM_VER(p[0]));
 			/* 6.4.4.3.1 */
 			svdm_consume_identity(port, p, cnt);
-			response[0] = VDO(USB_SID_PD, 1, SVDM_VER_1_0, CMD_DISCOVER_SVID);
+			response[0] = VDO(USB_SID_PD, 1, typec_get_negotiated_svdm_version(typec),
+					  CMD_DISCOVER_SVID);
 			rlen = 1;
 			break;
 		case CMD_DISCOVER_SVID:
 			/* 6.4.4.3.2 */
 			if (svdm_consume_svids(port, p, cnt)) {
-				response[0] = VDO(USB_SID_PD, 1, SVDM_VER_1_0,
-						  CMD_DISCOVER_SVID);
+				response[0] = VDO(USB_SID_PD, 1, svdm_version, CMD_DISCOVER_SVID);
 				rlen = 1;
 			} else if (modep->nsvids && supports_modal(port)) {
-				response[0] = VDO(modep->svids[0], 1, SVDM_VER_1_0,
+				response[0] = VDO(modep->svids[0], 1, svdm_version,
 						  CMD_DISCOVER_MODES);
 				rlen = 1;
 			}
@@ -1565,7 +1588,7 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
 			modep->svid_index++;
 			if (modep->svid_index < modep->nsvids) {
 				u16 svid = modep->svids[modep->svid_index];
-				response[0] = VDO(svid, 1, SVDM_VER_1_0, CMD_DISCOVER_MODES);
+				response[0] = VDO(svid, 1, svdm_version, CMD_DISCOVER_MODES);
 				rlen = 1;
 			} else {
 				tcpm_register_partner_altmodes(port);
@@ -1592,6 +1615,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
 			/* Unrecognized SVDM */
 			response[0] = p[0] | VDO_CMDT(CMDT_RSP_NAK);
 			rlen = 1;
+			response[0] = (response[0] & ~VDO_SVDM_VERS_MASK) |
+				      (VDO_SVDM_VERS(svdm_version));
 			break;
 		}
 		break;
@@ -1611,6 +1636,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
 			/* Unrecognized SVDM */
 			response[0] = p[0] | VDO_CMDT(CMDT_RSP_NAK);
 			rlen = 1;
+			response[0] = (response[0] & ~VDO_SVDM_VERS_MASK) |
+				      (VDO_SVDM_VERS(svdm_version));
 			break;
 		}
 		port->vdm_sm_running = false;
@@ -1618,6 +1645,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
 	default:
 		response[0] = p[0] | VDO_CMDT(CMDT_RSP_NAK);
 		rlen = 1;
+		response[0] = (response[0] & ~VDO_SVDM_VERS_MASK) |
+			      (VDO_SVDM_VERS(svdm_version));
 		port->vdm_sm_running = false;
 		break;
 	}
@@ -1695,7 +1724,13 @@ static void tcpm_handle_vdm_request(struct tcpm_port *port,
 			break;
 		case ADEV_QUEUE_VDM_SEND_EXIT_MODE_ON_FAIL:
 			if (typec_altmode_vdm(adev, p[0], &p[1], cnt)) {
-				response[0] = VDO(adev->svid, 1, SVDM_VER_1_0, CMD_EXIT_MODE);
+				int svdm_version = typec_get_negotiated_svdm_version(
+									port->typec_port);
+				if (svdm_version < 0)
+					break;
+
+				response[0] = VDO(adev->svid, 1, svdm_version,
+						  CMD_EXIT_MODE);
 				response[0] |= VDO_OPOS(adev->mode);
 				rlen = 1;
 			}
@@ -1722,14 +1757,19 @@ static void tcpm_handle_vdm_request(struct tcpm_port *port,
 static void tcpm_send_vdm(struct tcpm_port *port, u32 vid, int cmd,
 			  const u32 *data, int count)
 {
+	int svdm_version = typec_get_negotiated_svdm_version(port->typec_port);
 	u32 header;
 
+	if (svdm_version < 0)
+		return;
+
 	if (WARN_ON(count > VDO_MAX_SIZE - 1))
 		count = VDO_MAX_SIZE - 1;
 
 	/* set VDM header with VID & CMD */
 	header = VDO(vid, ((vid & USB_SID_PD) == USB_SID_PD) ?
-			1 : (PD_VDO_CMD(cmd) <= CMD_ATTENTION), SVDM_VER_1_0, cmd);
+			1 : (PD_VDO_CMD(cmd) <= CMD_ATTENTION),
+			svdm_version, cmd);
 	tcpm_queue_vdm(port, header, data, count);
 }
 
@@ -2022,9 +2062,14 @@ static int tcpm_validate_caps(struct tcpm_port *port, const u32 *pdo,
 static int tcpm_altmode_enter(struct typec_altmode *altmode, u32 *vdo)
 {
 	struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
+	int svdm_version;
 	u32 header;
 
-	header = VDO(altmode->svid, vdo ? 2 : 1, SVDM_VER_1_0, CMD_ENTER_MODE);
+	svdm_version = typec_get_negotiated_svdm_version(port->typec_port);
+	if (svdm_version < 0)
+		return svdm_version;
+
+	header = VDO(altmode->svid, vdo ? 2 : 1, svdm_version, CMD_ENTER_MODE);
 	header |= VDO_OPOS(altmode->mode);
 
 	tcpm_queue_vdm_unlocked(port, header, vdo, vdo ? 1 : 0);
@@ -2034,9 +2079,14 @@ static int tcpm_altmode_enter(struct typec_altmode *altmode, u32 *vdo)
 static int tcpm_altmode_exit(struct typec_altmode *altmode)
 {
 	struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
+	int svdm_version;
 	u32 header;
 
-	header = VDO(altmode->svid, 1, SVDM_VER_1_0, CMD_EXIT_MODE);
+	svdm_version = typec_get_negotiated_svdm_version(port->typec_port);
+	if (svdm_version < 0)
+		return svdm_version;
+
+	header = VDO(altmode->svid, 1, svdm_version, CMD_EXIT_MODE);
 	header |= VDO_OPOS(altmode->mode);
 
 	tcpm_queue_vdm_unlocked(port, header, NULL, 0);
@@ -5977,6 +6027,7 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
 	port->typec_caps.fwnode = tcpc->fwnode;
 	port->typec_caps.revision = 0x0120;	/* Type-C spec release 1.2 */
 	port->typec_caps.pd_revision = 0x0300;	/* USB-PD spec release 3.0 */
+	port->typec_caps.svdm_version = SVDM_VER_2_0;
 	port->typec_caps.driver_data = port;
 	port->typec_caps.ops = &tcpm_ops;
 	port->typec_caps.orientation_aware = 1;
-- 
2.30.0.365.g02bc693789-goog


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

* [PATCH v6 4/7] usb: typec: ucsi: Determine common SVDM Version
  2021-02-05  3:34 [PATCH v6 0/7] common SVDM version and VDO from dt Kyle Tso
                   ` (2 preceding siblings ...)
  2021-02-05  3:34 ` [PATCH v6 3/7] usb: typec: tcpm: Determine common SVDM Version Kyle Tso
@ 2021-02-05  3:34 ` Kyle Tso
  2021-02-12  4:18   ` Guenter Roeck
  2021-02-05  3:34 ` [PATCH v6 5/7] usb: typec: displayport: Fill the negotiated SVDM Version in the header Kyle Tso
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Kyle Tso @ 2021-02-05  3:34 UTC (permalink / raw)
  To: linux, heikki.krogerus, gregkh, hdegoede, robh+dt
  Cc: badhri, linux-usb, linux-kernel, devicetree, Kyle Tso

This patch implements the following requirement in the Spec.

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."

Signed-off-by: Kyle Tso <kyletso@google.com>
---
Changes since v5:
- follow the changes of "usb: typec: Manage SVDM version"
- remove the "reset to default". Now the default SVDM version will be
  set when calling to typec_register_partner

 drivers/usb/typec/ucsi/displayport.c | 32 +++++++++++++++++++++++++---
 drivers/usb/typec/ucsi/ucsi.c        |  1 +
 2 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/typec/ucsi/displayport.c b/drivers/usb/typec/ucsi/displayport.c
index 1d387bddefb9..73cd5bf35047 100644
--- a/drivers/usb/typec/ucsi/displayport.c
+++ b/drivers/usb/typec/ucsi/displayport.c
@@ -49,6 +49,7 @@ static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo)
 {
 	struct ucsi_dp *dp = typec_altmode_get_drvdata(alt);
 	struct ucsi *ucsi = dp->con->ucsi;
+	int svdm_version;
 	u64 command;
 	u8 cur = 0;
 	int ret;
@@ -83,7 +84,13 @@ static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo)
 	 * mode, and letting the alt mode driver continue.
 	 */
 
-	dp->header = VDO(USB_TYPEC_DP_SID, 1, SVDM_VER_1_0, CMD_ENTER_MODE);
+	svdm_version = typec_altmode_get_svdm_version(alt);
+	if (svdm_version < 0) {
+		ret = svdm_version;
+		goto err_unlock;
+	}
+
+	dp->header = VDO(USB_TYPEC_DP_SID, 1, svdm_version, CMD_ENTER_MODE);
 	dp->header |= VDO_OPOS(USB_TYPEC_DP_MODE);
 	dp->header |= VDO_CMDT(CMDT_RSP_ACK);
 
@@ -101,6 +108,7 @@ static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo)
 static int ucsi_displayport_exit(struct typec_altmode *alt)
 {
 	struct ucsi_dp *dp = typec_altmode_get_drvdata(alt);
+	int svdm_version;
 	u64 command;
 	int ret = 0;
 
@@ -120,7 +128,13 @@ static int ucsi_displayport_exit(struct typec_altmode *alt)
 	if (ret < 0)
 		goto out_unlock;
 
-	dp->header = VDO(USB_TYPEC_DP_SID, 1, SVDM_VER_1_0, CMD_EXIT_MODE);
+	svdm_version = typec_altmode_get_svdm_version(alt);
+	if (svdm_version < 0) {
+		ret = svdm_version;
+		goto out_unlock;
+	}
+
+	dp->header = VDO(USB_TYPEC_DP_SID, 1, svdm_version, CMD_EXIT_MODE);
 	dp->header |= VDO_OPOS(USB_TYPEC_DP_MODE);
 	dp->header |= VDO_CMDT(CMDT_RSP_ACK);
 
@@ -186,6 +200,7 @@ static int ucsi_displayport_vdm(struct typec_altmode *alt,
 	struct ucsi_dp *dp = typec_altmode_get_drvdata(alt);
 	int cmd_type = PD_VDO_CMDT(header);
 	int cmd = PD_VDO_CMD(header);
+	int svdm_version;
 
 	mutex_lock(&dp->con->lock);
 
@@ -198,9 +213,20 @@ static int ucsi_displayport_vdm(struct typec_altmode *alt,
 		return -EOPNOTSUPP;
 	}
 
+	svdm_version = typec_altmode_get_svdm_version(alt);
+	if (svdm_version < 0) {
+		mutex_unlock(&dp->con->lock);
+		return svdm_version;
+	}
+
 	switch (cmd_type) {
 	case CMDT_INIT:
-		dp->header = VDO(USB_TYPEC_DP_SID, 1, SVDM_VER_1_0, cmd);
+		if (PD_VDO_SVDM_VER(header) < svdm_version) {
+			typec_partner_set_svdm_version(dp->con->partner, PD_VDO_SVDM_VER(header));
+			svdm_version = PD_VDO_SVDM_VER(header);
+		}
+
+		dp->header = VDO(USB_TYPEC_DP_SID, 1, svdm_version, cmd);
 		dp->header |= VDO_OPOS(USB_TYPEC_DP_MODE);
 
 		switch (cmd) {
diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
index ca3f4194ad90..244270755ae6 100644
--- a/drivers/usb/typec/ucsi/ucsi.c
+++ b/drivers/usb/typec/ucsi/ucsi.c
@@ -1052,6 +1052,7 @@ static int ucsi_register_port(struct ucsi *ucsi, int index)
 
 	cap->revision = ucsi->cap.typec_version;
 	cap->pd_revision = ucsi->cap.pd_version;
+	cap->svdm_version = SVDM_VER_2_0;
 	cap->prefer_role = TYPEC_NO_PREFERRED_ROLE;
 
 	if (con->cap.op_mode & UCSI_CONCAP_OPMODE_AUDIO_ACCESSORY)
-- 
2.30.0.365.g02bc693789-goog


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

* [PATCH v6 5/7] usb: typec: displayport: Fill the negotiated SVDM Version in the header
  2021-02-05  3:34 [PATCH v6 0/7] common SVDM version and VDO from dt Kyle Tso
                   ` (3 preceding siblings ...)
  2021-02-05  3:34 ` [PATCH v6 4/7] usb: typec: ucsi: " Kyle Tso
@ 2021-02-05  3:34 ` Kyle Tso
  2021-02-12  4:19   ` Guenter Roeck
  2021-02-05  3:34 ` [PATCH v6 6/7] dt-bindings: connector: Add SVDM VDO properties Kyle Tso
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 24+ messages in thread
From: Kyle Tso @ 2021-02-05  3:34 UTC (permalink / raw)
  To: linux, heikki.krogerus, gregkh, hdegoede, robh+dt
  Cc: badhri, linux-usb, linux-kernel, devicetree, Kyle Tso

VDM header now requires SVDM Version. Get it from typec_partner.

Signed-off-by: Kyle Tso <kyletso@google.com>
---
Changes since v5:
- follow the changes of "usb: typec: Manage SVDM version"

 drivers/usb/typec/altmodes/displayport.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
index 0abc3121238f..b7f094435b00 100644
--- a/drivers/usb/typec/altmodes/displayport.c
+++ b/drivers/usb/typec/altmodes/displayport.c
@@ -15,8 +15,8 @@
 #include <linux/usb/typec_dp.h>
 #include "displayport.h"
 
-#define DP_HEADER(_dp, cmd)		(VDO((_dp)->alt->svid, 1, SVDM_VER_1_0, cmd) | \
-					 VDO_OPOS(USB_TYPEC_DP_MODE))
+#define DP_HEADER(_dp, ver, cmd)	(VDO((_dp)->alt->svid, 1, ver, cmd)	\
+					 | VDO_OPOS(USB_TYPEC_DP_MODE))
 
 enum {
 	DP_CONF_USB,
@@ -156,9 +156,14 @@ static int dp_altmode_configured(struct dp_altmode *dp)
 
 static int dp_altmode_configure_vdm(struct dp_altmode *dp, u32 conf)
 {
-	u32 header = DP_HEADER(dp, DP_CMD_CONFIGURE);
+	int svdm_version = typec_altmode_get_svdm_version(dp->alt);
+	u32 header;
 	int ret;
 
+	if (svdm_version < 0)
+		return svdm_version;
+
+	header = DP_HEADER(dp, svdm_version, DP_CMD_CONFIGURE);
 	ret = typec_altmode_notify(dp->alt, TYPEC_STATE_SAFE, &dp->data);
 	if (ret) {
 		dev_err(&dp->alt->dev,
@@ -181,6 +186,7 @@ static int dp_altmode_configure_vdm(struct dp_altmode *dp, u32 conf)
 static void dp_altmode_work(struct work_struct *work)
 {
 	struct dp_altmode *dp = container_of(work, struct dp_altmode, work);
+	int svdm_version;
 	u32 header;
 	u32 vdo;
 	int ret;
@@ -194,7 +200,10 @@ static void dp_altmode_work(struct work_struct *work)
 			dev_err(&dp->alt->dev, "failed to enter mode\n");
 		break;
 	case DP_STATE_UPDATE:
-		header = DP_HEADER(dp, DP_CMD_STATUS_UPDATE);
+		svdm_version = typec_altmode_get_svdm_version(dp->alt);
+		if (svdm_version < 0)
+			break;
+		header = DP_HEADER(dp, svdm_version, DP_CMD_STATUS_UPDATE);
 		vdo = 1;
 		ret = typec_altmode_vdm(dp->alt, header, &vdo, 2);
 		if (ret)
-- 
2.30.0.365.g02bc693789-goog


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

* [PATCH v6 6/7] dt-bindings: connector: Add SVDM VDO properties
  2021-02-05  3:34 [PATCH v6 0/7] common SVDM version and VDO from dt Kyle Tso
                   ` (4 preceding siblings ...)
  2021-02-05  3:34 ` [PATCH v6 5/7] usb: typec: displayport: Fill the negotiated SVDM Version in the header Kyle Tso
@ 2021-02-05  3:34 ` Kyle Tso
  2021-02-05 21:29   ` Rob Herring
  2021-02-12  4:21   ` Guenter Roeck
  2021-02-05  3:34 ` [PATCH v6 7/7] usb: typec: tcpm: Get Sink VDO from fwnode Kyle Tso
  2021-02-05 11:42 ` [PATCH v6 0/7] common SVDM version and VDO from dt Heikki Krogerus
  7 siblings, 2 replies; 24+ messages in thread
From: Kyle Tso @ 2021-02-05  3:34 UTC (permalink / raw)
  To: linux, heikki.krogerus, gregkh, hdegoede, robh+dt
  Cc: badhri, linux-usb, linux-kernel, devicetree, Kyle Tso

Add bindings of VDO properties of USB PD SVDM so that they can be
used in device tree.

Signed-off-by: Kyle Tso <kyletso@google.com>
---
Changes since v5:
- no change

 .../bindings/connector/usb-connector.yaml     |  11 +
 include/dt-bindings/usb/pd.h                  | 311 +++++++++++++++++-
 2 files changed, 321 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
index 4286ed767a0a..d385026944ec 100644
--- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
+++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
@@ -137,6 +137,17 @@ properties:
     maxItems: 7
     $ref: /schemas/types.yaml#/definitions/uint32-array
 
+  sink-vdos:
+    description: An array of u32 with each entry (VDM Objects) providing additional information
+      corresponding to the product, the detailed bit definitions and the order of each VDO can be
+      found in "USB Power Delivery Specification Revision 3.0, Version 2.0 + ECNs 2020-12-10"
+      chapter 6.4.4.3.1 Discover Identity. User can specify the VDO array via
+      VDO_IDH/_CERT/_PRODUCT/_UFP/_DFP/_PCABLE/_ACABLE(1/2)/_VPD() defined in
+      dt-bindings/usb/pd.h.
+    minItems: 3
+    maxItems: 6
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+
   op-sink-microwatt:
     description: Sink required operating power in microwatt, if source can't
       offer the power, Capability Mismatch is set. Required for power sink and
diff --git a/include/dt-bindings/usb/pd.h b/include/dt-bindings/usb/pd.h
index 0352893697f0..fef3ef65967f 100644
--- a/include/dt-bindings/usb/pd.h
+++ b/include/dt-bindings/usb/pd.h
@@ -93,4 +93,313 @@
 #define FRS_DEFAULT_POWER      1
 #define FRS_5V_1P5A            2
 #define FRS_5V_3A              3
- #endif /* __DT_POWER_DELIVERY_H */
+
+/*
+ * SVDM Identity Header
+ * --------------------
+ * <31>     :: data capable as a USB host
+ * <30>     :: data capable as a USB device
+ * <29:27>  :: product type (UFP / Cable / VPD)
+ * <26>     :: modal operation supported (1b == yes)
+ * <25:23>  :: product type (DFP) (SVDM version 2.0+ only; set to zero in version 1.0)
+ * <22:21>  :: connector type (SVDM version 2.0+ only; set to zero in version 1.0)
+ * <20:16>  :: Reserved, Shall be set to zero
+ * <15:0>   :: USB-IF assigned VID for this cable vendor
+ */
+/* SOP Product Type (UFP) */
+#define IDH_PTYPE_NOT_UFP       0
+#define IDH_PTYPE_HUB           1
+#define IDH_PTYPE_PERIPH        2
+#define IDH_PTYPE_PSD           3
+#define IDH_PTYPE_AMA           5
+
+/* SOP' Product Type (Cable Plug / VPD) */
+#define IDH_PTYPE_NOT_CABLE     0
+#define IDH_PTYPE_PCABLE        3
+#define IDH_PTYPE_ACABLE        4
+#define IDH_PTYPE_VPD           6
+
+/* SOP Product Type (DFP) */
+#define IDH_PTYPE_NOT_DFP       0
+#define IDH_PTYPE_DFP_HUB       1
+#define IDH_PTYPE_DFP_HOST      2
+#define IDH_PTYPE_DFP_PB        3
+
+#define VDO_IDH(usbh, usbd, ufp_cable, is_modal, dfp, conn, vid)                \
+	((usbh) << 31 | (usbd) << 30 | ((ufp_cable) & 0x7) << 27                \
+	 | (is_modal) << 26 | ((dfp) & 0x7) << 23 | ((conn) & 0x3) << 21        \
+	 | ((vid) & 0xffff))
+
+/*
+ * Cert Stat VDO
+ * -------------
+ * <31:0>  : USB-IF assigned XID for this cable
+ */
+#define VDO_CERT(xid)		((xid) & 0xffffffff)
+
+/*
+ * Product VDO
+ * -----------
+ * <31:16> : USB Product ID
+ * <15:0>  : USB bcdDevice
+ */
+#define VDO_PRODUCT(pid, bcd)   (((pid) & 0xffff) << 16 | ((bcd) & 0xffff))
+
+/*
+ * UFP VDO (PD Revision 3.0+ only)
+ * --------
+ * <31:29> :: UFP VDO version
+ * <28>    :: Reserved
+ * <27:24> :: Device capability
+ * <23:22> :: Connector type (10b == receptacle, 11b == captive plug)
+ * <21:11> :: Reserved
+ * <10:8>  :: Vconn power (AMA only)
+ * <7>     :: Vconn required (AMA only, 0b == no, 1b == yes)
+ * <6>     :: Vbus required (AMA only, 0b == yes, 1b == no)
+ * <5:3>   :: Alternate modes
+ * <2:0>   :: USB highest speed
+ */
+/* UFP VDO Version */
+#define UFP_VDO_VER1_2		2
+
+/* Device Capability */
+#define DEV_USB2_CAPABLE	BIT(0)
+#define DEV_USB2_BILLBOARD	BIT(1)
+#define DEV_USB3_CAPABLE	BIT(2)
+#define DEV_USB4_CAPABLE	BIT(3)
+
+/* Connector Type */
+#define UFP_RECEPTACLE		2
+#define UFP_CAPTIVE		3
+
+/* Vconn Power (AMA only, set to AMA_VCONN_NOT_REQ if Vconn is not required) */
+#define AMA_VCONN_PWR_1W	0
+#define AMA_VCONN_PWR_1W5	1
+#define AMA_VCONN_PWR_2W	2
+#define AMA_VCONN_PWR_3W	3
+#define AMA_VCONN_PWR_4W	4
+#define AMA_VCONN_PWR_5W	5
+#define AMA_VCONN_PWR_6W	6
+
+/* Vconn Required (AMA only) */
+#define AMA_VCONN_NOT_REQ	0
+#define AMA_VCONN_REQ		1
+
+/* Vbus Required (AMA only) */
+#define AMA_VBUS_REQ		0
+#define AMA_VBUS_NOT_REQ	1
+
+/* Alternate Modes */
+#define UFP_ALTMODE_NOT_SUPP	0
+#define UFP_ALTMODE_TBT3	BIT(0)
+#define UFP_ALTMODE_RECFG	BIT(1)
+#define UFP_ALTMODE_NO_RECFG	BIT(2)
+
+/* USB Highest Speed */
+#define UFP_USB2_ONLY		0
+#define UFP_USB32_GEN1		1
+#define UFP_USB32_4_GEN2	2
+#define UFP_USB4_GEN3		3
+
+#define VDO_UFP(ver, cap, conn, vcpwr, vcr, vbr, alt, spd)			\
+	(((ver) & 0x7) << 29 | ((cap) & 0xf) << 24 | ((conn) & 0x3) << 22	\
+	 | ((vcpwr) & 0x7) << 8 | (vcr) << 7 | (vbr) << 6 | ((alt) & 0x7) << 3	\
+	 | ((spd) & 0x7))
+
+/*
+ * DFP VDO (PD Revision 3.0+ only)
+ * --------
+ * <31:29> :: DFP VDO version
+ * <28:27> :: Reserved
+ * <26:24> :: Host capability
+ * <23:22> :: Connector type (10b == receptacle, 11b == captive plug)
+ * <21:5>  :: Reserved
+ * <4:0>   :: Port number
+ */
+#define DFP_VDO_VER1_1		1
+#define HOST_USB2_CAPABLE	BIT(0)
+#define HOST_USB3_CAPABLE	BIT(1)
+#define HOST_USB4_CAPABLE	BIT(2)
+#define DFP_RECEPTACLE		2
+#define DFP_CAPTIVE		3
+
+#define VDO_DFP(ver, cap, conn, pnum)						\
+	(((ver) & 0x7) << 29 | ((cap) & 0x7) << 24 | ((conn) & 0x3) << 22	\
+	 | ((pnum) & 0x1f))
+
+/*
+ * Passive Cable VDO
+ * ---------
+ * <31:28> :: Cable HW version
+ * <27:24> :: Cable FW version
+ * <23:21> :: VDO version
+ * <20>    :: Reserved, Shall be set to zero
+ * <19:18> :: Type-C to Type-C/Captive (10b == C, 11b == Captive)
+ * <17>    :: Reserved, Shall be set to zero
+ * <16:13> :: cable latency (0001 == <10ns(~1m length))
+ * <12:11> :: cable termination type (10b == Vconn not req, 01b == Vconn req)
+ * <10:9>  :: Maximum Vbus voltage (00b == 20V, 01b == 30V, 10b == 40V, 11b == 50V)
+ * <8:7>   :: Reserved, Shall be set to zero
+ * <6:5>   :: Vbus current handling capability (01b == 3A, 10b == 5A)
+ * <4:3>   :: Reserved, Shall be set to zero
+ * <2:0>   :: USB highest speed
+ *
+ * Active Cable VDO 1
+ * ---------
+ * <31:28> :: Cable HW version
+ * <27:24> :: Cable FW version
+ * <23:21> :: VDO version
+ * <20>    :: Reserved, Shall be set to zero
+ * <19:18> :: Connector type (10b == C, 11b == Captive)
+ * <17>    :: Reserved, Shall be set to zero
+ * <16:13> :: cable latency (0001 == <10ns(~1m length))
+ * <12:11> :: cable termination type (10b == one end active, 11b == both ends active VCONN req)
+ * <10:9>  :: Maximum Vbus voltage (00b == 20V, 01b == 30V, 10b == 40V, 11b == 50V)
+ * <8>     :: SBU supported (0b == supported, 1b == not supported)
+ * <7>     :: SBU type (0b == passive, 1b == active)
+ * <6:5>   :: Vbus current handling capability (01b == 3A, 10b == 5A)
+ * <4>     :: Vbus through cable (0b == no, 1b == yes)
+ * <3>     :: SOP" controller present? (0b == no, 1b == yes)
+ * <2:0>   :: USB highest speed
+ */
+/* Cable VDO Version */
+#define CABLE_VDO_VER1_0	0
+#define CABLE_VDO_VER1_3	3
+
+/* Connector Type */
+#define CABLE_CTYPE		2
+#define CABLE_CAPTIVE		3
+
+/* Cable Latency */
+#define CABLE_LATENCY_1M	1
+#define CABLE_LATENCY_2M	2
+#define CABLE_LATENCY_3M	3
+#define CABLE_LATENCY_4M	4
+#define CABLE_LATENCY_5M	5
+#define CABLE_LATENCY_6M	6
+#define CABLE_LATENCY_7M	7
+#define CABLE_LATENCY_7M_PLUS	8
+
+/* Cable Termination Type */
+#define PCABLE_VCONN_NOT_REQ	0
+#define PCABLE_VCONN_REQ	1
+#define ACABLE_ONE_END		2
+#define ACABLE_BOTH_END		3
+
+/* Maximum Vbus Voltage */
+#define CABLE_MAX_VBUS_20V	0
+#define CABLE_MAX_VBUS_30V	1
+#define CABLE_MAX_VBUS_40V	2
+#define CABLE_MAX_VBUS_50V	3
+
+/* Active Cable SBU Supported/Type */
+#define ACABLE_SBU_SUPP		0
+#define ACABLE_SBU_NOT_SUPP	1
+#define ACABLE_SBU_PASSIVE	0
+#define ACABLE_SBU_ACTIVE	1
+
+/* Vbus Current Handling Capability */
+#define CABLE_CURR_DEF		0
+#define CABLE_CURR_3A		1
+#define CABLE_CURR_5A		2
+
+/* USB Highest Speed */
+#define CABLE_USB2_ONLY		0
+#define CABLE_USB32_GEN1	1
+#define CABLE_USB32_4_GEN2	2
+#define CABLE_USB4_GEN3		3
+
+#define VDO_PCABLE(hw, fw, ver, conn, lat, term, vbm, cur, spd)			\
+	(((hw) & 0xf) << 28 | ((fw) & 0xf) << 24 | ((ver) & 0x7) << 21		\
+	 | ((conn) & 0x3) << 18 | ((lat) & 0xf) << 13 | ((term) & 0x3) << 11	\
+	 | ((vbm) & 0x3) << 9 | ((cur) & 0x3) << 5 | ((spd) & 0x7))
+#define VDO_ACABLE1(hw, fw, ver, conn, lat, term, vbm, sbu, sbut, cur, vbt, sopp, spd) \
+	(((hw) & 0xf) << 28 | ((fw) & 0xf) << 24 | ((ver) & 0x7) << 21		\
+	 | ((conn) & 0x3) << 18	| ((lat) & 0xf) << 13 | ((term) & 0x3) << 11	\
+	 | ((vbm) & 0x3) << 9 | (sbu) << 8 | (sbut) << 7 | ((cur) & 0x3) << 5	\
+	 | (vbt) << 4 | (sopp) << 3 | ((spd) & 0x7))
+
+/*
+ * Active Cable VDO 2
+ * ---------
+ * <31:24> :: Maximum operating temperature
+ * <23:16> :: Shutdown temperature
+ * <15>    :: Reserved, Shall be set to zero
+ * <14:12> :: U3/CLd power
+ * <11>    :: U3 to U0 transition mode (0b == direct, 1b == through U3S)
+ * <10>    :: Physical connection (0b == copper, 1b == optical)
+ * <9>     :: Active element (0b == redriver, 1b == retimer)
+ * <8>     :: USB4 supported (0b == yes, 1b == no)
+ * <7:6>   :: USB2 hub hops consumed
+ * <5>     :: USB2 supported (0b == yes, 1b == no)
+ * <4>     :: USB3.2 supported (0b == yes, 1b == no)
+ * <3>     :: USB lanes supported (0b == one lane, 1b == two lanes)
+ * <2>     :: Optically isolated active cable (0b == no, 1b == yes)
+ * <1>     :: Reserved, Shall be set to zero
+ * <0>     :: USB gen (0b == gen1, 1b == gen2+)
+ */
+/* U3/CLd Power*/
+#define ACAB2_U3_CLD_10MW_PLUS	0
+#define ACAB2_U3_CLD_10MW	1
+#define ACAB2_U3_CLD_5MW	2
+#define ACAB2_U3_CLD_1MW	3
+#define ACAB2_U3_CLD_500UW	4
+#define ACAB2_U3_CLD_200UW	5
+#define ACAB2_U3_CLD_50UW	6
+
+/* Other Active Cable VDO 2 Fields */
+#define ACAB2_U3U0_DIRECT	0
+#define ACAB2_U3U0_U3S		1
+#define ACAB2_PHY_COPPER	0
+#define ACAB2_PHY_OPTICAL	1
+#define ACAB2_REDRIVER		0
+#define ACAB2_RETIMER		1
+#define ACAB2_USB4_SUPP		0
+#define ACAB2_USB4_NOT_SUPP	1
+#define ACAB2_USB2_SUPP		0
+#define ACAB2_USB2_NOT_SUPP	1
+#define ACAB2_USB32_SUPP	0
+#define ACAB2_USB32_NOT_SUPP	1
+#define ACAB2_LANES_ONE		0
+#define ACAB2_LANES_TWO		1
+#define ACAB2_OPT_ISO_NO	0
+#define ACAB2_OPT_ISO_YES	1
+#define ACAB2_GEN_1		0
+#define ACAB2_GEN_2_PLUS	1
+
+#define VDO_ACABLE2(mtemp, stemp, u3p, trans, phy, ele, u4, hops, u2, u32, lane, iso, gen)	\
+	(((mtemp) & 0xff) << 24 | ((stemp) & 0xff) << 16 | ((u3p) & 0x7) << 12	\
+	 | (trans) << 11 | (phy) << 10 | (ele) << 9 | (u4) << 8			\
+	 | ((hops) & 0x3) << 6 | (u2) << 5 | (u32) << 4 | (lane) << 3		\
+	 | (iso) << 2 | (gen))
+
+/*
+ * VPD VDO
+ * ---------
+ * <31:28> :: HW version
+ * <27:24> :: FW version
+ * <23:21> :: VDO version
+ * <20:17> :: Reserved, Shall be set to zero
+ * <16:15> :: Maximum Vbus voltage (00b == 20V, 01b == 30V, 10b == 40V, 11b == 50V)
+ * <14>    :: Charge through current support (0b == 3A, 1b == 5A)
+ * <13>    :: Reserved, Shall be set to zero
+ * <12:7>  :: Vbus impedance
+ * <6:1>   :: Ground impedance
+ * <0>     :: Charge through support (0b == no, 1b == yes)
+ */
+#define VPD_VDO_VER1_0		0
+#define VPD_MAX_VBUS_20V	0
+#define VPD_MAX_VBUS_30V	1
+#define VPD_MAX_VBUS_40V	2
+#define VPD_MAX_VBUS_50V	3
+#define VPDCT_CURR_3A		0
+#define VPDCT_CURR_5A		1
+#define VPDCT_NOT_SUPP		0
+#define VPDCT_SUPP		1
+
+#define VDO_VPD(hw, fw, ver, vbm, curr, vbi, gi, ct)			\
+	(((hw) & 0xf) << 28 | ((fw) & 0xf) << 24 | ((ver) & 0x7) << 21	\
+	 | ((vbm) & 0x3) << 15 | (curr) << 14 | ((vbi) & 0x3f) << 7	\
+	 | ((gi) & 0x3f) << 1 | (ct))
+
+#endif /* __DT_POWER_DELIVERY_H */
-- 
2.30.0.365.g02bc693789-goog


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

* [PATCH v6 7/7] usb: typec: tcpm: Get Sink VDO from fwnode
  2021-02-05  3:34 [PATCH v6 0/7] common SVDM version and VDO from dt Kyle Tso
                   ` (5 preceding siblings ...)
  2021-02-05  3:34 ` [PATCH v6 6/7] dt-bindings: connector: Add SVDM VDO properties Kyle Tso
@ 2021-02-05  3:34 ` Kyle Tso
  2021-02-12  4:21   ` Guenter Roeck
  2021-02-05 11:42 ` [PATCH v6 0/7] common SVDM version and VDO from dt Heikki Krogerus
  7 siblings, 1 reply; 24+ messages in thread
From: Kyle Tso @ 2021-02-05  3:34 UTC (permalink / raw)
  To: linux, heikki.krogerus, gregkh, hdegoede, robh+dt
  Cc: badhri, linux-usb, linux-kernel, devicetree, Kyle Tso

Commit a079973f462a ("usb: typec: tcpm: Remove tcpc_config
configuration mechanism") removed the tcpc_config which includes the
Sink VDO and it is not yet added back with fwnode. Add it now.

Signed-off-by: Kyle Tso <kyletso@google.com>
---
Changes since v5:
- no change

 drivers/usb/typec/tcpm/tcpm.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index b45cd191a8a4..be0b6469dd3d 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -5722,6 +5722,20 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
 			port->new_source_frs_current = frs_current;
 	}
 
+	/* sink-vdos is optional */
+	ret = fwnode_property_count_u32(fwnode, "sink-vdos");
+	if (ret < 0)
+		ret = 0;
+
+	port->nr_snk_vdo = min(ret, VDO_MAX_OBJECTS);
+	if (port->nr_snk_vdo) {
+		ret = fwnode_property_read_u32_array(fwnode, "sink-vdos",
+						     port->snk_vdo,
+						     port->nr_snk_vdo);
+		if (ret < 0)
+			return ret;
+	}
+
 	return 0;
 }
 
-- 
2.30.0.365.g02bc693789-goog


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

* Re: [PATCH v6 0/7] common SVDM version and VDO from dt
  2021-02-05  3:34 [PATCH v6 0/7] common SVDM version and VDO from dt Kyle Tso
                   ` (6 preceding siblings ...)
  2021-02-05  3:34 ` [PATCH v6 7/7] usb: typec: tcpm: Get Sink VDO from fwnode Kyle Tso
@ 2021-02-05 11:42 ` Heikki Krogerus
  2021-02-05 12:20   ` Guenter Roeck
  7 siblings, 1 reply; 24+ messages in thread
From: Heikki Krogerus @ 2021-02-05 11:42 UTC (permalink / raw)
  To: Kyle Tso
  Cc: linux, gregkh, hdegoede, robh+dt, badhri, linux-usb,
	linux-kernel, devicetree

On Fri, Feb 05, 2021 at 11:34:08AM +0800, Kyle Tso wrote:
> v5 is here:
> https://patchwork.kernel.org/project/linux-usb/cover/20210202161733.932215-1-kyletso@google.com/
> 
> Changes since v5:
> =================
> usb: typec: Manage SVDM version
> - !! most changes are from Heikki
> - location of the negotiated SVDM version is changed. Now the variable
>   resides in typec_partner
> - The setter and getter functions were modified according to the above
>   changes
> - the default SVDM version is stored upon calling to
>   typec_register_partner
> 
> usb: pd: Make SVDM Version configurable in VDM header
> - no change
> 
> usb: typec: tcpm: Determine common SVDM Version
> - follow the changes of "usb: typec: Manage SVDM version"
> - remove the "reset to default". Now the default SVDM version will be
>   set when calling to typec_register_partner
> 
> usb: typec: ucsi: Determine common SVDM Version
> - follow the changes of "usb: typec: Manage SVDM version"
> - remove the "reset to default". Now the default SVDM version will be
>   set when calling to typec_register_partner
> 
> usb: typec: displayport: Fill the negotiated SVDM Version in the header
> - follow the changes of "usb: typec: Manage SVDM version"
> 
> dt-bindings: connector: Add SVDM VDO properties
> - no change
> 
> usb: typec: tcpm: Get Sink VDO from fwnode
> - no change
> 
> Kyle Tso (7):
>   usb: typec: Manage SVDM version
>   usb: pd: Make SVDM Version configurable in VDM header
>   usb: typec: tcpm: Determine common SVDM Version
>   usb: typec: ucsi: Determine common SVDM Version
>   usb: typec: displayport: Fill the negotiated SVDM Version in the header
>   dt-bindings: connector: Add SVDM VDO properties
>   usb: typec: tcpm: Get Sink VDO from fwnode

These are OK by me, but I think it would be great if Guenter could
give them the once-over, as usual. I hope he has time. FWIW, for all
of these:

Acked-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

thanks,

-- 
heikki

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

* Re: [PATCH v6 0/7] common SVDM version and VDO from dt
  2021-02-05 11:42 ` [PATCH v6 0/7] common SVDM version and VDO from dt Heikki Krogerus
@ 2021-02-05 12:20   ` Guenter Roeck
  0 siblings, 0 replies; 24+ messages in thread
From: Guenter Roeck @ 2021-02-05 12:20 UTC (permalink / raw)
  To: Heikki Krogerus, Kyle Tso
  Cc: gregkh, hdegoede, robh+dt, badhri, linux-usb, linux-kernel, devicetree

On 2/5/21 3:42 AM, Heikki Krogerus wrote:
[ ... ]
>> Kyle Tso (7):
>>   usb: typec: Manage SVDM version
>>   usb: pd: Make SVDM Version configurable in VDM header
>>   usb: typec: tcpm: Determine common SVDM Version
>>   usb: typec: ucsi: Determine common SVDM Version
>>   usb: typec: displayport: Fill the negotiated SVDM Version in the header
>>   dt-bindings: connector: Add SVDM VDO properties
>>   usb: typec: tcpm: Get Sink VDO from fwnode
> 
> These are OK by me, but I think it would be great if Guenter could
> give them the once-over, as usual. I hope he has time. FWIW, for all

I'll try my best, but I'll need a couple of days.

Guenter

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

* Re: [PATCH v6 6/7] dt-bindings: connector: Add SVDM VDO properties
  2021-02-05  3:34 ` [PATCH v6 6/7] dt-bindings: connector: Add SVDM VDO properties Kyle Tso
@ 2021-02-05 21:29   ` Rob Herring
  2021-02-06  0:29     ` Kyle Tso
  2021-02-12  4:21   ` Guenter Roeck
  1 sibling, 1 reply; 24+ messages in thread
From: Rob Herring @ 2021-02-05 21:29 UTC (permalink / raw)
  To: Kyle Tso
  Cc: linux, heikki.krogerus, gregkh, hdegoede, badhri, linux-usb,
	linux-kernel, devicetree

On Fri, Feb 05, 2021 at 11:34:14AM +0800, Kyle Tso wrote:
> Add bindings of VDO properties of USB PD SVDM so that they can be
> used in device tree.
> 
> Signed-off-by: Kyle Tso <kyletso@google.com>
> ---
> Changes since v5:
> - no change
> 
>  .../bindings/connector/usb-connector.yaml     |  11 +
>  include/dt-bindings/usb/pd.h                  | 311 +++++++++++++++++-
>  2 files changed, 321 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> index 4286ed767a0a..d385026944ec 100644
> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> @@ -137,6 +137,17 @@ properties:
>      maxItems: 7
>      $ref: /schemas/types.yaml#/definitions/uint32-array
>  
> +  sink-vdos:
> +    description: An array of u32 with each entry (VDM Objects) providing additional information

VDO stands for 'VDM Objects' and VDM stands for ???

Other than that,

Reviewed-by: Rob Herring <robh@kernel.org>

> +      corresponding to the product, the detailed bit definitions and the order of each VDO can be
> +      found in "USB Power Delivery Specification Revision 3.0, Version 2.0 + ECNs 2020-12-10"
> +      chapter 6.4.4.3.1 Discover Identity. User can specify the VDO array via
> +      VDO_IDH/_CERT/_PRODUCT/_UFP/_DFP/_PCABLE/_ACABLE(1/2)/_VPD() defined in
> +      dt-bindings/usb/pd.h.
> +    minItems: 3
> +    maxItems: 6
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +
>    op-sink-microwatt:
>      description: Sink required operating power in microwatt, if source can't
>        offer the power, Capability Mismatch is set. Required for power sink and
> diff --git a/include/dt-bindings/usb/pd.h b/include/dt-bindings/usb/pd.h
> index 0352893697f0..fef3ef65967f 100644
> --- a/include/dt-bindings/usb/pd.h
> +++ b/include/dt-bindings/usb/pd.h
> @@ -93,4 +93,313 @@
>  #define FRS_DEFAULT_POWER      1
>  #define FRS_5V_1P5A            2
>  #define FRS_5V_3A              3
> - #endif /* __DT_POWER_DELIVERY_H */
> +
> +/*
> + * SVDM Identity Header
> + * --------------------
> + * <31>     :: data capable as a USB host
> + * <30>     :: data capable as a USB device
> + * <29:27>  :: product type (UFP / Cable / VPD)
> + * <26>     :: modal operation supported (1b == yes)
> + * <25:23>  :: product type (DFP) (SVDM version 2.0+ only; set to zero in version 1.0)
> + * <22:21>  :: connector type (SVDM version 2.0+ only; set to zero in version 1.0)
> + * <20:16>  :: Reserved, Shall be set to zero
> + * <15:0>   :: USB-IF assigned VID for this cable vendor
> + */
> +/* SOP Product Type (UFP) */
> +#define IDH_PTYPE_NOT_UFP       0
> +#define IDH_PTYPE_HUB           1
> +#define IDH_PTYPE_PERIPH        2
> +#define IDH_PTYPE_PSD           3
> +#define IDH_PTYPE_AMA           5
> +
> +/* SOP' Product Type (Cable Plug / VPD) */
> +#define IDH_PTYPE_NOT_CABLE     0
> +#define IDH_PTYPE_PCABLE        3
> +#define IDH_PTYPE_ACABLE        4
> +#define IDH_PTYPE_VPD           6
> +
> +/* SOP Product Type (DFP) */
> +#define IDH_PTYPE_NOT_DFP       0
> +#define IDH_PTYPE_DFP_HUB       1
> +#define IDH_PTYPE_DFP_HOST      2
> +#define IDH_PTYPE_DFP_PB        3
> +
> +#define VDO_IDH(usbh, usbd, ufp_cable, is_modal, dfp, conn, vid)                \
> +	((usbh) << 31 | (usbd) << 30 | ((ufp_cable) & 0x7) << 27                \
> +	 | (is_modal) << 26 | ((dfp) & 0x7) << 23 | ((conn) & 0x3) << 21        \
> +	 | ((vid) & 0xffff))
> +
> +/*
> + * Cert Stat VDO
> + * -------------
> + * <31:0>  : USB-IF assigned XID for this cable
> + */
> +#define VDO_CERT(xid)		((xid) & 0xffffffff)
> +
> +/*
> + * Product VDO
> + * -----------
> + * <31:16> : USB Product ID
> + * <15:0>  : USB bcdDevice
> + */
> +#define VDO_PRODUCT(pid, bcd)   (((pid) & 0xffff) << 16 | ((bcd) & 0xffff))
> +
> +/*
> + * UFP VDO (PD Revision 3.0+ only)
> + * --------
> + * <31:29> :: UFP VDO version
> + * <28>    :: Reserved
> + * <27:24> :: Device capability
> + * <23:22> :: Connector type (10b == receptacle, 11b == captive plug)
> + * <21:11> :: Reserved
> + * <10:8>  :: Vconn power (AMA only)
> + * <7>     :: Vconn required (AMA only, 0b == no, 1b == yes)
> + * <6>     :: Vbus required (AMA only, 0b == yes, 1b == no)
> + * <5:3>   :: Alternate modes
> + * <2:0>   :: USB highest speed
> + */
> +/* UFP VDO Version */
> +#define UFP_VDO_VER1_2		2
> +
> +/* Device Capability */
> +#define DEV_USB2_CAPABLE	BIT(0)
> +#define DEV_USB2_BILLBOARD	BIT(1)
> +#define DEV_USB3_CAPABLE	BIT(2)
> +#define DEV_USB4_CAPABLE	BIT(3)
> +
> +/* Connector Type */
> +#define UFP_RECEPTACLE		2
> +#define UFP_CAPTIVE		3
> +
> +/* Vconn Power (AMA only, set to AMA_VCONN_NOT_REQ if Vconn is not required) */
> +#define AMA_VCONN_PWR_1W	0
> +#define AMA_VCONN_PWR_1W5	1
> +#define AMA_VCONN_PWR_2W	2
> +#define AMA_VCONN_PWR_3W	3
> +#define AMA_VCONN_PWR_4W	4
> +#define AMA_VCONN_PWR_5W	5
> +#define AMA_VCONN_PWR_6W	6
> +
> +/* Vconn Required (AMA only) */
> +#define AMA_VCONN_NOT_REQ	0
> +#define AMA_VCONN_REQ		1
> +
> +/* Vbus Required (AMA only) */
> +#define AMA_VBUS_REQ		0
> +#define AMA_VBUS_NOT_REQ	1
> +
> +/* Alternate Modes */
> +#define UFP_ALTMODE_NOT_SUPP	0
> +#define UFP_ALTMODE_TBT3	BIT(0)
> +#define UFP_ALTMODE_RECFG	BIT(1)
> +#define UFP_ALTMODE_NO_RECFG	BIT(2)
> +
> +/* USB Highest Speed */
> +#define UFP_USB2_ONLY		0
> +#define UFP_USB32_GEN1		1
> +#define UFP_USB32_4_GEN2	2
> +#define UFP_USB4_GEN3		3
> +
> +#define VDO_UFP(ver, cap, conn, vcpwr, vcr, vbr, alt, spd)			\
> +	(((ver) & 0x7) << 29 | ((cap) & 0xf) << 24 | ((conn) & 0x3) << 22	\
> +	 | ((vcpwr) & 0x7) << 8 | (vcr) << 7 | (vbr) << 6 | ((alt) & 0x7) << 3	\
> +	 | ((spd) & 0x7))
> +
> +/*
> + * DFP VDO (PD Revision 3.0+ only)
> + * --------
> + * <31:29> :: DFP VDO version
> + * <28:27> :: Reserved
> + * <26:24> :: Host capability
> + * <23:22> :: Connector type (10b == receptacle, 11b == captive plug)
> + * <21:5>  :: Reserved
> + * <4:0>   :: Port number
> + */
> +#define DFP_VDO_VER1_1		1
> +#define HOST_USB2_CAPABLE	BIT(0)
> +#define HOST_USB3_CAPABLE	BIT(1)
> +#define HOST_USB4_CAPABLE	BIT(2)
> +#define DFP_RECEPTACLE		2
> +#define DFP_CAPTIVE		3
> +
> +#define VDO_DFP(ver, cap, conn, pnum)						\
> +	(((ver) & 0x7) << 29 | ((cap) & 0x7) << 24 | ((conn) & 0x3) << 22	\
> +	 | ((pnum) & 0x1f))
> +
> +/*
> + * Passive Cable VDO
> + * ---------
> + * <31:28> :: Cable HW version
> + * <27:24> :: Cable FW version
> + * <23:21> :: VDO version
> + * <20>    :: Reserved, Shall be set to zero
> + * <19:18> :: Type-C to Type-C/Captive (10b == C, 11b == Captive)
> + * <17>    :: Reserved, Shall be set to zero
> + * <16:13> :: cable latency (0001 == <10ns(~1m length))
> + * <12:11> :: cable termination type (10b == Vconn not req, 01b == Vconn req)
> + * <10:9>  :: Maximum Vbus voltage (00b == 20V, 01b == 30V, 10b == 40V, 11b == 50V)
> + * <8:7>   :: Reserved, Shall be set to zero
> + * <6:5>   :: Vbus current handling capability (01b == 3A, 10b == 5A)
> + * <4:3>   :: Reserved, Shall be set to zero
> + * <2:0>   :: USB highest speed
> + *
> + * Active Cable VDO 1
> + * ---------
> + * <31:28> :: Cable HW version
> + * <27:24> :: Cable FW version
> + * <23:21> :: VDO version
> + * <20>    :: Reserved, Shall be set to zero
> + * <19:18> :: Connector type (10b == C, 11b == Captive)
> + * <17>    :: Reserved, Shall be set to zero
> + * <16:13> :: cable latency (0001 == <10ns(~1m length))
> + * <12:11> :: cable termination type (10b == one end active, 11b == both ends active VCONN req)
> + * <10:9>  :: Maximum Vbus voltage (00b == 20V, 01b == 30V, 10b == 40V, 11b == 50V)
> + * <8>     :: SBU supported (0b == supported, 1b == not supported)
> + * <7>     :: SBU type (0b == passive, 1b == active)
> + * <6:5>   :: Vbus current handling capability (01b == 3A, 10b == 5A)
> + * <4>     :: Vbus through cable (0b == no, 1b == yes)
> + * <3>     :: SOP" controller present? (0b == no, 1b == yes)
> + * <2:0>   :: USB highest speed
> + */
> +/* Cable VDO Version */
> +#define CABLE_VDO_VER1_0	0
> +#define CABLE_VDO_VER1_3	3
> +
> +/* Connector Type */
> +#define CABLE_CTYPE		2
> +#define CABLE_CAPTIVE		3
> +
> +/* Cable Latency */
> +#define CABLE_LATENCY_1M	1
> +#define CABLE_LATENCY_2M	2
> +#define CABLE_LATENCY_3M	3
> +#define CABLE_LATENCY_4M	4
> +#define CABLE_LATENCY_5M	5
> +#define CABLE_LATENCY_6M	6
> +#define CABLE_LATENCY_7M	7
> +#define CABLE_LATENCY_7M_PLUS	8
> +
> +/* Cable Termination Type */
> +#define PCABLE_VCONN_NOT_REQ	0
> +#define PCABLE_VCONN_REQ	1
> +#define ACABLE_ONE_END		2
> +#define ACABLE_BOTH_END		3
> +
> +/* Maximum Vbus Voltage */
> +#define CABLE_MAX_VBUS_20V	0
> +#define CABLE_MAX_VBUS_30V	1
> +#define CABLE_MAX_VBUS_40V	2
> +#define CABLE_MAX_VBUS_50V	3
> +
> +/* Active Cable SBU Supported/Type */
> +#define ACABLE_SBU_SUPP		0
> +#define ACABLE_SBU_NOT_SUPP	1
> +#define ACABLE_SBU_PASSIVE	0
> +#define ACABLE_SBU_ACTIVE	1
> +
> +/* Vbus Current Handling Capability */
> +#define CABLE_CURR_DEF		0
> +#define CABLE_CURR_3A		1
> +#define CABLE_CURR_5A		2
> +
> +/* USB Highest Speed */
> +#define CABLE_USB2_ONLY		0
> +#define CABLE_USB32_GEN1	1
> +#define CABLE_USB32_4_GEN2	2
> +#define CABLE_USB4_GEN3		3
> +
> +#define VDO_PCABLE(hw, fw, ver, conn, lat, term, vbm, cur, spd)			\
> +	(((hw) & 0xf) << 28 | ((fw) & 0xf) << 24 | ((ver) & 0x7) << 21		\
> +	 | ((conn) & 0x3) << 18 | ((lat) & 0xf) << 13 | ((term) & 0x3) << 11	\
> +	 | ((vbm) & 0x3) << 9 | ((cur) & 0x3) << 5 | ((spd) & 0x7))
> +#define VDO_ACABLE1(hw, fw, ver, conn, lat, term, vbm, sbu, sbut, cur, vbt, sopp, spd) \
> +	(((hw) & 0xf) << 28 | ((fw) & 0xf) << 24 | ((ver) & 0x7) << 21		\
> +	 | ((conn) & 0x3) << 18	| ((lat) & 0xf) << 13 | ((term) & 0x3) << 11	\
> +	 | ((vbm) & 0x3) << 9 | (sbu) << 8 | (sbut) << 7 | ((cur) & 0x3) << 5	\
> +	 | (vbt) << 4 | (sopp) << 3 | ((spd) & 0x7))
> +
> +/*
> + * Active Cable VDO 2
> + * ---------
> + * <31:24> :: Maximum operating temperature
> + * <23:16> :: Shutdown temperature
> + * <15>    :: Reserved, Shall be set to zero
> + * <14:12> :: U3/CLd power
> + * <11>    :: U3 to U0 transition mode (0b == direct, 1b == through U3S)
> + * <10>    :: Physical connection (0b == copper, 1b == optical)
> + * <9>     :: Active element (0b == redriver, 1b == retimer)
> + * <8>     :: USB4 supported (0b == yes, 1b == no)
> + * <7:6>   :: USB2 hub hops consumed
> + * <5>     :: USB2 supported (0b == yes, 1b == no)
> + * <4>     :: USB3.2 supported (0b == yes, 1b == no)
> + * <3>     :: USB lanes supported (0b == one lane, 1b == two lanes)
> + * <2>     :: Optically isolated active cable (0b == no, 1b == yes)
> + * <1>     :: Reserved, Shall be set to zero
> + * <0>     :: USB gen (0b == gen1, 1b == gen2+)
> + */
> +/* U3/CLd Power*/
> +#define ACAB2_U3_CLD_10MW_PLUS	0
> +#define ACAB2_U3_CLD_10MW	1
> +#define ACAB2_U3_CLD_5MW	2
> +#define ACAB2_U3_CLD_1MW	3
> +#define ACAB2_U3_CLD_500UW	4
> +#define ACAB2_U3_CLD_200UW	5
> +#define ACAB2_U3_CLD_50UW	6
> +
> +/* Other Active Cable VDO 2 Fields */
> +#define ACAB2_U3U0_DIRECT	0
> +#define ACAB2_U3U0_U3S		1
> +#define ACAB2_PHY_COPPER	0
> +#define ACAB2_PHY_OPTICAL	1
> +#define ACAB2_REDRIVER		0
> +#define ACAB2_RETIMER		1
> +#define ACAB2_USB4_SUPP		0
> +#define ACAB2_USB4_NOT_SUPP	1
> +#define ACAB2_USB2_SUPP		0
> +#define ACAB2_USB2_NOT_SUPP	1
> +#define ACAB2_USB32_SUPP	0
> +#define ACAB2_USB32_NOT_SUPP	1
> +#define ACAB2_LANES_ONE		0
> +#define ACAB2_LANES_TWO		1
> +#define ACAB2_OPT_ISO_NO	0
> +#define ACAB2_OPT_ISO_YES	1
> +#define ACAB2_GEN_1		0
> +#define ACAB2_GEN_2_PLUS	1
> +
> +#define VDO_ACABLE2(mtemp, stemp, u3p, trans, phy, ele, u4, hops, u2, u32, lane, iso, gen)	\
> +	(((mtemp) & 0xff) << 24 | ((stemp) & 0xff) << 16 | ((u3p) & 0x7) << 12	\
> +	 | (trans) << 11 | (phy) << 10 | (ele) << 9 | (u4) << 8			\
> +	 | ((hops) & 0x3) << 6 | (u2) << 5 | (u32) << 4 | (lane) << 3		\
> +	 | (iso) << 2 | (gen))
> +
> +/*
> + * VPD VDO
> + * ---------
> + * <31:28> :: HW version
> + * <27:24> :: FW version
> + * <23:21> :: VDO version
> + * <20:17> :: Reserved, Shall be set to zero
> + * <16:15> :: Maximum Vbus voltage (00b == 20V, 01b == 30V, 10b == 40V, 11b == 50V)
> + * <14>    :: Charge through current support (0b == 3A, 1b == 5A)
> + * <13>    :: Reserved, Shall be set to zero
> + * <12:7>  :: Vbus impedance
> + * <6:1>   :: Ground impedance
> + * <0>     :: Charge through support (0b == no, 1b == yes)
> + */
> +#define VPD_VDO_VER1_0		0
> +#define VPD_MAX_VBUS_20V	0
> +#define VPD_MAX_VBUS_30V	1
> +#define VPD_MAX_VBUS_40V	2
> +#define VPD_MAX_VBUS_50V	3
> +#define VPDCT_CURR_3A		0
> +#define VPDCT_CURR_5A		1
> +#define VPDCT_NOT_SUPP		0
> +#define VPDCT_SUPP		1
> +
> +#define VDO_VPD(hw, fw, ver, vbm, curr, vbi, gi, ct)			\
> +	(((hw) & 0xf) << 28 | ((fw) & 0xf) << 24 | ((ver) & 0x7) << 21	\
> +	 | ((vbm) & 0x3) << 15 | (curr) << 14 | ((vbi) & 0x3f) << 7	\
> +	 | ((gi) & 0x3f) << 1 | (ct))
> +
> +#endif /* __DT_POWER_DELIVERY_H */
> -- 
> 2.30.0.365.g02bc693789-goog
> 

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

* Re: [PATCH v6 6/7] dt-bindings: connector: Add SVDM VDO properties
  2021-02-05 21:29   ` Rob Herring
@ 2021-02-06  0:29     ` Kyle Tso
  0 siblings, 0 replies; 24+ messages in thread
From: Kyle Tso @ 2021-02-06  0:29 UTC (permalink / raw)
  To: Rob Herring
  Cc: Guenter Roeck, Heikki Krogerus, Greg KH, Hans de Goede,
	Badhri Jagan Sridharan, USB, LKML,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Sat, Feb 6, 2021 at 5:30 AM Rob Herring <robh@kernel.org> wrote:
>
> On Fri, Feb 05, 2021 at 11:34:14AM +0800, Kyle Tso wrote:
> > Add bindings of VDO properties of USB PD SVDM so that they can be
> > used in device tree.
> >
> > Signed-off-by: Kyle Tso <kyletso@google.com>
> > ---
> > Changes since v5:
> > - no change
> >
> >  .../bindings/connector/usb-connector.yaml     |  11 +
> >  include/dt-bindings/usb/pd.h                  | 311 +++++++++++++++++-
> >  2 files changed, 321 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > index 4286ed767a0a..d385026944ec 100644
> > --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > @@ -137,6 +137,17 @@ properties:
> >      maxItems: 7
> >      $ref: /schemas/types.yaml#/definitions/uint32-array
> >
> > +  sink-vdos:
> > +    description: An array of u32 with each entry (VDM Objects) providing additional information
>
> VDO stands for 'VDM Objects' and VDM stands for ???
>
> Other than that,
>
> Reviewed-by: Rob Herring <robh@kernel.org>
>

Thanks for the review.

The full name of VDM is "Vendor Defined Message"
If the patch set is accepted, I will have another patch for this.
If there will be a next version of this patchset, it will include the change.

thanks,
Kyle

> > +      corresponding to the product, the detailed bit definitions and the order of each VDO can be
> > +      found in "USB Power Delivery Specification Revision 3.0, Version 2.0 + ECNs 2020-12-10"
> > +      chapter 6.4.4.3.1 Discover Identity. User can specify the VDO array via
> > +      VDO_IDH/_CERT/_PRODUCT/_UFP/_DFP/_PCABLE/_ACABLE(1/2)/_VPD() defined in
> > +      dt-bindings/usb/pd.h.
> > +    minItems: 3
> > +    maxItems: 6
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > +
> >    op-sink-microwatt:
> >      description: Sink required operating power in microwatt, if source can't
> >        offer the power, Capability Mismatch is set. Required for power sink and
> > diff --git a/include/dt-bindings/usb/pd.h b/include/dt-bindings/usb/pd.h
> > index 0352893697f0..fef3ef65967f 100644
> > --- a/include/dt-bindings/usb/pd.h
> > +++ b/include/dt-bindings/usb/pd.h
> > @@ -93,4 +93,313 @@
> >  #define FRS_DEFAULT_POWER      1
> >  #define FRS_5V_1P5A            2
> >  #define FRS_5V_3A              3
> > - #endif /* __DT_POWER_DELIVERY_H */
> > +
> > +/*
> > + * SVDM Identity Header
> > + * --------------------
> > + * <31>     :: data capable as a USB host
> > + * <30>     :: data capable as a USB device
> > + * <29:27>  :: product type (UFP / Cable / VPD)
> > + * <26>     :: modal operation supported (1b == yes)
> > + * <25:23>  :: product type (DFP) (SVDM version 2.0+ only; set to zero in version 1.0)
> > + * <22:21>  :: connector type (SVDM version 2.0+ only; set to zero in version 1.0)
> > + * <20:16>  :: Reserved, Shall be set to zero
> > + * <15:0>   :: USB-IF assigned VID for this cable vendor
> > + */
> > +/* SOP Product Type (UFP) */
> > +#define IDH_PTYPE_NOT_UFP       0
> > +#define IDH_PTYPE_HUB           1
> > +#define IDH_PTYPE_PERIPH        2
> > +#define IDH_PTYPE_PSD           3
> > +#define IDH_PTYPE_AMA           5
> > +
> > +/* SOP' Product Type (Cable Plug / VPD) */
> > +#define IDH_PTYPE_NOT_CABLE     0
> > +#define IDH_PTYPE_PCABLE        3
> > +#define IDH_PTYPE_ACABLE        4
> > +#define IDH_PTYPE_VPD           6
> > +
> > +/* SOP Product Type (DFP) */
> > +#define IDH_PTYPE_NOT_DFP       0
> > +#define IDH_PTYPE_DFP_HUB       1
> > +#define IDH_PTYPE_DFP_HOST      2
> > +#define IDH_PTYPE_DFP_PB        3
> > +
> > +#define VDO_IDH(usbh, usbd, ufp_cable, is_modal, dfp, conn, vid)                \
> > +     ((usbh) << 31 | (usbd) << 30 | ((ufp_cable) & 0x7) << 27                \
> > +      | (is_modal) << 26 | ((dfp) & 0x7) << 23 | ((conn) & 0x3) << 21        \
> > +      | ((vid) & 0xffff))
> > +
> > +/*
> > + * Cert Stat VDO
> > + * -------------
> > + * <31:0>  : USB-IF assigned XID for this cable
> > + */
> > +#define VDO_CERT(xid)                ((xid) & 0xffffffff)
> > +
> > +/*
> > + * Product VDO
> > + * -----------
> > + * <31:16> : USB Product ID
> > + * <15:0>  : USB bcdDevice
> > + */
> > +#define VDO_PRODUCT(pid, bcd)   (((pid) & 0xffff) << 16 | ((bcd) & 0xffff))
> > +
> > +/*
> > + * UFP VDO (PD Revision 3.0+ only)
> > + * --------
> > + * <31:29> :: UFP VDO version
> > + * <28>    :: Reserved
> > + * <27:24> :: Device capability
> > + * <23:22> :: Connector type (10b == receptacle, 11b == captive plug)
> > + * <21:11> :: Reserved
> > + * <10:8>  :: Vconn power (AMA only)
> > + * <7>     :: Vconn required (AMA only, 0b == no, 1b == yes)
> > + * <6>     :: Vbus required (AMA only, 0b == yes, 1b == no)
> > + * <5:3>   :: Alternate modes
> > + * <2:0>   :: USB highest speed
> > + */
> > +/* UFP VDO Version */
> > +#define UFP_VDO_VER1_2               2
> > +
> > +/* Device Capability */
> > +#define DEV_USB2_CAPABLE     BIT(0)
> > +#define DEV_USB2_BILLBOARD   BIT(1)
> > +#define DEV_USB3_CAPABLE     BIT(2)
> > +#define DEV_USB4_CAPABLE     BIT(3)
> > +
> > +/* Connector Type */
> > +#define UFP_RECEPTACLE               2
> > +#define UFP_CAPTIVE          3
> > +
> > +/* Vconn Power (AMA only, set to AMA_VCONN_NOT_REQ if Vconn is not required) */
> > +#define AMA_VCONN_PWR_1W     0
> > +#define AMA_VCONN_PWR_1W5    1
> > +#define AMA_VCONN_PWR_2W     2
> > +#define AMA_VCONN_PWR_3W     3
> > +#define AMA_VCONN_PWR_4W     4
> > +#define AMA_VCONN_PWR_5W     5
> > +#define AMA_VCONN_PWR_6W     6
> > +
> > +/* Vconn Required (AMA only) */
> > +#define AMA_VCONN_NOT_REQ    0
> > +#define AMA_VCONN_REQ                1
> > +
> > +/* Vbus Required (AMA only) */
> > +#define AMA_VBUS_REQ         0
> > +#define AMA_VBUS_NOT_REQ     1
> > +
> > +/* Alternate Modes */
> > +#define UFP_ALTMODE_NOT_SUPP 0
> > +#define UFP_ALTMODE_TBT3     BIT(0)
> > +#define UFP_ALTMODE_RECFG    BIT(1)
> > +#define UFP_ALTMODE_NO_RECFG BIT(2)
> > +
> > +/* USB Highest Speed */
> > +#define UFP_USB2_ONLY                0
> > +#define UFP_USB32_GEN1               1
> > +#define UFP_USB32_4_GEN2     2
> > +#define UFP_USB4_GEN3                3
> > +
> > +#define VDO_UFP(ver, cap, conn, vcpwr, vcr, vbr, alt, spd)                   \
> > +     (((ver) & 0x7) << 29 | ((cap) & 0xf) << 24 | ((conn) & 0x3) << 22       \
> > +      | ((vcpwr) & 0x7) << 8 | (vcr) << 7 | (vbr) << 6 | ((alt) & 0x7) << 3  \
> > +      | ((spd) & 0x7))
> > +
> > +/*
> > + * DFP VDO (PD Revision 3.0+ only)
> > + * --------
> > + * <31:29> :: DFP VDO version
> > + * <28:27> :: Reserved
> > + * <26:24> :: Host capability
> > + * <23:22> :: Connector type (10b == receptacle, 11b == captive plug)
> > + * <21:5>  :: Reserved
> > + * <4:0>   :: Port number
> > + */
> > +#define DFP_VDO_VER1_1               1
> > +#define HOST_USB2_CAPABLE    BIT(0)
> > +#define HOST_USB3_CAPABLE    BIT(1)
> > +#define HOST_USB4_CAPABLE    BIT(2)
> > +#define DFP_RECEPTACLE               2
> > +#define DFP_CAPTIVE          3
> > +
> > +#define VDO_DFP(ver, cap, conn, pnum)                                                \
> > +     (((ver) & 0x7) << 29 | ((cap) & 0x7) << 24 | ((conn) & 0x3) << 22       \
> > +      | ((pnum) & 0x1f))
> > +
> > +/*
> > + * Passive Cable VDO
> > + * ---------
> > + * <31:28> :: Cable HW version
> > + * <27:24> :: Cable FW version
> > + * <23:21> :: VDO version
> > + * <20>    :: Reserved, Shall be set to zero
> > + * <19:18> :: Type-C to Type-C/Captive (10b == C, 11b == Captive)
> > + * <17>    :: Reserved, Shall be set to zero
> > + * <16:13> :: cable latency (0001 == <10ns(~1m length))
> > + * <12:11> :: cable termination type (10b == Vconn not req, 01b == Vconn req)
> > + * <10:9>  :: Maximum Vbus voltage (00b == 20V, 01b == 30V, 10b == 40V, 11b == 50V)
> > + * <8:7>   :: Reserved, Shall be set to zero
> > + * <6:5>   :: Vbus current handling capability (01b == 3A, 10b == 5A)
> > + * <4:3>   :: Reserved, Shall be set to zero
> > + * <2:0>   :: USB highest speed
> > + *
> > + * Active Cable VDO 1
> > + * ---------
> > + * <31:28> :: Cable HW version
> > + * <27:24> :: Cable FW version
> > + * <23:21> :: VDO version
> > + * <20>    :: Reserved, Shall be set to zero
> > + * <19:18> :: Connector type (10b == C, 11b == Captive)
> > + * <17>    :: Reserved, Shall be set to zero
> > + * <16:13> :: cable latency (0001 == <10ns(~1m length))
> > + * <12:11> :: cable termination type (10b == one end active, 11b == both ends active VCONN req)
> > + * <10:9>  :: Maximum Vbus voltage (00b == 20V, 01b == 30V, 10b == 40V, 11b == 50V)
> > + * <8>     :: SBU supported (0b == supported, 1b == not supported)
> > + * <7>     :: SBU type (0b == passive, 1b == active)
> > + * <6:5>   :: Vbus current handling capability (01b == 3A, 10b == 5A)
> > + * <4>     :: Vbus through cable (0b == no, 1b == yes)
> > + * <3>     :: SOP" controller present? (0b == no, 1b == yes)
> > + * <2:0>   :: USB highest speed
> > + */
> > +/* Cable VDO Version */
> > +#define CABLE_VDO_VER1_0     0
> > +#define CABLE_VDO_VER1_3     3
> > +
> > +/* Connector Type */
> > +#define CABLE_CTYPE          2
> > +#define CABLE_CAPTIVE                3
> > +
> > +/* Cable Latency */
> > +#define CABLE_LATENCY_1M     1
> > +#define CABLE_LATENCY_2M     2
> > +#define CABLE_LATENCY_3M     3
> > +#define CABLE_LATENCY_4M     4
> > +#define CABLE_LATENCY_5M     5
> > +#define CABLE_LATENCY_6M     6
> > +#define CABLE_LATENCY_7M     7
> > +#define CABLE_LATENCY_7M_PLUS        8
> > +
> > +/* Cable Termination Type */
> > +#define PCABLE_VCONN_NOT_REQ 0
> > +#define PCABLE_VCONN_REQ     1
> > +#define ACABLE_ONE_END               2
> > +#define ACABLE_BOTH_END              3
> > +
> > +/* Maximum Vbus Voltage */
> > +#define CABLE_MAX_VBUS_20V   0
> > +#define CABLE_MAX_VBUS_30V   1
> > +#define CABLE_MAX_VBUS_40V   2
> > +#define CABLE_MAX_VBUS_50V   3
> > +
> > +/* Active Cable SBU Supported/Type */
> > +#define ACABLE_SBU_SUPP              0
> > +#define ACABLE_SBU_NOT_SUPP  1
> > +#define ACABLE_SBU_PASSIVE   0
> > +#define ACABLE_SBU_ACTIVE    1
> > +
> > +/* Vbus Current Handling Capability */
> > +#define CABLE_CURR_DEF               0
> > +#define CABLE_CURR_3A                1
> > +#define CABLE_CURR_5A                2
> > +
> > +/* USB Highest Speed */
> > +#define CABLE_USB2_ONLY              0
> > +#define CABLE_USB32_GEN1     1
> > +#define CABLE_USB32_4_GEN2   2
> > +#define CABLE_USB4_GEN3              3
> > +
> > +#define VDO_PCABLE(hw, fw, ver, conn, lat, term, vbm, cur, spd)                      \
> > +     (((hw) & 0xf) << 28 | ((fw) & 0xf) << 24 | ((ver) & 0x7) << 21          \
> > +      | ((conn) & 0x3) << 18 | ((lat) & 0xf) << 13 | ((term) & 0x3) << 11    \
> > +      | ((vbm) & 0x3) << 9 | ((cur) & 0x3) << 5 | ((spd) & 0x7))
> > +#define VDO_ACABLE1(hw, fw, ver, conn, lat, term, vbm, sbu, sbut, cur, vbt, sopp, spd) \
> > +     (((hw) & 0xf) << 28 | ((fw) & 0xf) << 24 | ((ver) & 0x7) << 21          \
> > +      | ((conn) & 0x3) << 18 | ((lat) & 0xf) << 13 | ((term) & 0x3) << 11    \
> > +      | ((vbm) & 0x3) << 9 | (sbu) << 8 | (sbut) << 7 | ((cur) & 0x3) << 5   \
> > +      | (vbt) << 4 | (sopp) << 3 | ((spd) & 0x7))
> > +
> > +/*
> > + * Active Cable VDO 2
> > + * ---------
> > + * <31:24> :: Maximum operating temperature
> > + * <23:16> :: Shutdown temperature
> > + * <15>    :: Reserved, Shall be set to zero
> > + * <14:12> :: U3/CLd power
> > + * <11>    :: U3 to U0 transition mode (0b == direct, 1b == through U3S)
> > + * <10>    :: Physical connection (0b == copper, 1b == optical)
> > + * <9>     :: Active element (0b == redriver, 1b == retimer)
> > + * <8>     :: USB4 supported (0b == yes, 1b == no)
> > + * <7:6>   :: USB2 hub hops consumed
> > + * <5>     :: USB2 supported (0b == yes, 1b == no)
> > + * <4>     :: USB3.2 supported (0b == yes, 1b == no)
> > + * <3>     :: USB lanes supported (0b == one lane, 1b == two lanes)
> > + * <2>     :: Optically isolated active cable (0b == no, 1b == yes)
> > + * <1>     :: Reserved, Shall be set to zero
> > + * <0>     :: USB gen (0b == gen1, 1b == gen2+)
> > + */
> > +/* U3/CLd Power*/
> > +#define ACAB2_U3_CLD_10MW_PLUS       0
> > +#define ACAB2_U3_CLD_10MW    1
> > +#define ACAB2_U3_CLD_5MW     2
> > +#define ACAB2_U3_CLD_1MW     3
> > +#define ACAB2_U3_CLD_500UW   4
> > +#define ACAB2_U3_CLD_200UW   5
> > +#define ACAB2_U3_CLD_50UW    6
> > +
> > +/* Other Active Cable VDO 2 Fields */
> > +#define ACAB2_U3U0_DIRECT    0
> > +#define ACAB2_U3U0_U3S               1
> > +#define ACAB2_PHY_COPPER     0
> > +#define ACAB2_PHY_OPTICAL    1
> > +#define ACAB2_REDRIVER               0
> > +#define ACAB2_RETIMER                1
> > +#define ACAB2_USB4_SUPP              0
> > +#define ACAB2_USB4_NOT_SUPP  1
> > +#define ACAB2_USB2_SUPP              0
> > +#define ACAB2_USB2_NOT_SUPP  1
> > +#define ACAB2_USB32_SUPP     0
> > +#define ACAB2_USB32_NOT_SUPP 1
> > +#define ACAB2_LANES_ONE              0
> > +#define ACAB2_LANES_TWO              1
> > +#define ACAB2_OPT_ISO_NO     0
> > +#define ACAB2_OPT_ISO_YES    1
> > +#define ACAB2_GEN_1          0
> > +#define ACAB2_GEN_2_PLUS     1
> > +
> > +#define VDO_ACABLE2(mtemp, stemp, u3p, trans, phy, ele, u4, hops, u2, u32, lane, iso, gen)   \
> > +     (((mtemp) & 0xff) << 24 | ((stemp) & 0xff) << 16 | ((u3p) & 0x7) << 12  \
> > +      | (trans) << 11 | (phy) << 10 | (ele) << 9 | (u4) << 8                 \
> > +      | ((hops) & 0x3) << 6 | (u2) << 5 | (u32) << 4 | (lane) << 3           \
> > +      | (iso) << 2 | (gen))
> > +
> > +/*
> > + * VPD VDO
> > + * ---------
> > + * <31:28> :: HW version
> > + * <27:24> :: FW version
> > + * <23:21> :: VDO version
> > + * <20:17> :: Reserved, Shall be set to zero
> > + * <16:15> :: Maximum Vbus voltage (00b == 20V, 01b == 30V, 10b == 40V, 11b == 50V)
> > + * <14>    :: Charge through current support (0b == 3A, 1b == 5A)
> > + * <13>    :: Reserved, Shall be set to zero
> > + * <12:7>  :: Vbus impedance
> > + * <6:1>   :: Ground impedance
> > + * <0>     :: Charge through support (0b == no, 1b == yes)
> > + */
> > +#define VPD_VDO_VER1_0               0
> > +#define VPD_MAX_VBUS_20V     0
> > +#define VPD_MAX_VBUS_30V     1
> > +#define VPD_MAX_VBUS_40V     2
> > +#define VPD_MAX_VBUS_50V     3
> > +#define VPDCT_CURR_3A                0
> > +#define VPDCT_CURR_5A                1
> > +#define VPDCT_NOT_SUPP               0
> > +#define VPDCT_SUPP           1
> > +
> > +#define VDO_VPD(hw, fw, ver, vbm, curr, vbi, gi, ct)                 \
> > +     (((hw) & 0xf) << 28 | ((fw) & 0xf) << 24 | ((ver) & 0x7) << 21  \
> > +      | ((vbm) & 0x3) << 15 | (curr) << 14 | ((vbi) & 0x3f) << 7     \
> > +      | ((gi) & 0x3f) << 1 | (ct))
> > +
> > +#endif /* __DT_POWER_DELIVERY_H */
> > --
> > 2.30.0.365.g02bc693789-goog
> >

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

* Re: [PATCHi v6 1/7] usb: typec: Manage SVDM version
  2021-02-05  3:34 ` [PATCHi v6 1/7] usb: typec: Manage SVDM version Kyle Tso
@ 2021-02-12  4:09   ` Guenter Roeck
  0 siblings, 0 replies; 24+ messages in thread
From: Guenter Roeck @ 2021-02-12  4:09 UTC (permalink / raw)
  To: Kyle Tso
  Cc: heikki.krogerus, gregkh, hdegoede, robh+dt, badhri, linux-usb,
	linux-kernel, devicetree

On Fri, Feb 05, 2021 at 11:34:09AM +0800, Kyle Tso wrote:
> 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_partner to cache the
> negotiated SVDM version between the port and the partner.
> 
> Also add setter/getter functions for the negotiated SVDM version.
> 
> Signed-off-by: Kyle Tso <kyletso@google.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Changes since v5
> - !! most changes are from Heikki
> - location of the negotiated SVDM version is changed. Now the variable
>   resides in typec_partner
> - The setter and getter functions were modified according to the above
>   changes
> - the default SVDM version is stored upon calling to
>   typec_register_partner
> 
>  drivers/usb/typec/class.c         | 43 +++++++++++++++++++++++++++++++
>  include/linux/usb/typec.h         | 12 +++++++++
>  include/linux/usb/typec_altmode.h | 10 +++++++
>  3 files changed, 65 insertions(+)
> 
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index b4a5d9d4564f..45f0bf65e9ab 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -38,6 +38,7 @@ struct typec_partner {
>  	struct ida			mode_ids;
>  	int				num_altmodes;
>  	u16				pd_revision; /* 0300H = "3.0" */
> +	enum usb_pd_svdm_ver		svdm_version;
>  };
>  
>  struct typec_port {
> @@ -824,6 +825,20 @@ typec_partner_register_altmode(struct typec_partner *partner,
>  }
>  EXPORT_SYMBOL_GPL(typec_partner_register_altmode);
>  
> +/**
> + * typec_partner_set_svdm_version - Set negotiated Structured VDM (SVDM) Version
> + * @partner: USB Type-C Partner that supports SVDM
> + * @svdm_version: Negotiated SVDM Version
> + *
> + * This routine is used to save the negotiated SVDM Version.
> + */
> +void typec_partner_set_svdm_version(struct typec_partner *partner,
> +				   enum usb_pd_svdm_ver svdm_version)
> +{
> +	partner->svdm_version = svdm_version;
> +}
> +EXPORT_SYMBOL_GPL(typec_partner_set_svdm_version);
> +
>  /**
>   * typec_register_partner - Register a USB Type-C Partner
>   * @port: The USB Type-C Port the partner is connected to
> @@ -848,6 +863,7 @@ struct typec_partner *typec_register_partner(struct typec_port *port,
>  	partner->accessory = desc->accessory;
>  	partner->num_altmodes = -1;
>  	partner->pd_revision = desc->pd_revision;
> +	partner->svdm_version = port->cap->svdm_version;
>  
>  	if (desc->identity) {
>  		/*
> @@ -1894,6 +1910,33 @@ EXPORT_SYMBOL_GPL(typec_set_mode);
>  
>  /* --------------------------------------- */
>  
> +/**
> + * typec_get_negotiated_svdm_version - Get negotiated SVDM Version
> + * @port: USB Type-C Port.
> + *
> + * Get the negotiated SVDM Version. The Version is set to the port default
> + * value stored in typec_capability on partner registration, and updated after
> + * a successful Discover Identity if the negotiated value is less than the
> + * default value.
> + *
> + * Returns usb_pd_svdm_ver if the partner has been registered otherwise -ENODEV.
> + */
> +int typec_get_negotiated_svdm_version(struct typec_port *port)
> +{
> +	enum usb_pd_svdm_ver svdm_version;
> +	struct device *partner_dev;
> +
> +	partner_dev = device_find_child(&port->dev, NULL, partner_match);
> +	if (!partner_dev)
> +		return -ENODEV;
> +
> +	svdm_version = to_typec_partner(partner_dev)->svdm_version;
> +	put_device(partner_dev);
> +
> +	return svdm_version;
> +}
> +EXPORT_SYMBOL_GPL(typec_get_negotiated_svdm_version);
> +
>  /**
>   * typec_get_drvdata - Return private driver data pointer
>   * @port: USB Type-C port
> diff --git a/include/linux/usb/typec.h b/include/linux/usb/typec.h
> index a94df82ab62f..91b4303ca305 100644
> --- a/include/linux/usb/typec.h
> +++ b/include/linux/usb/typec.h
> @@ -217,12 +217,19 @@ struct typec_operations {
>  			     enum typec_port_type type);
>  };
>  
> +enum usb_pd_svdm_ver {
> +	SVDM_VER_1_0 = 0,
> +	SVDM_VER_2_0 = 1,
> +	SVDM_VER_MAX = SVDM_VER_2_0,
> +};
> +
>  /*
>   * struct typec_capability - USB Type-C Port Capabilities
>   * @type: Supported power role of the port
>   * @data: Supported data role of the port
>   * @revision: USB Type-C Specification release. Binary coded decimal
>   * @pd_revision: USB Power Delivery Specification revision if supported
> + * @svdm_version: USB PD Structured VDM version if supported
>   * @prefer_role: Initial role preference (DRP ports).
>   * @accessory: Supported Accessory Modes
>   * @fwnode: Optional fwnode of the port
> @@ -236,6 +243,7 @@ struct typec_capability {
>  	enum typec_port_data	data;
>  	u16			revision; /* 0120H = "1.2" */
>  	u16			pd_revision; /* 0300H = "3.0" */
> +	enum usb_pd_svdm_ver	svdm_version;
>  	int			prefer_role;
>  	enum typec_accessory	accessory[TYPEC_MAX_ACCESSORY];
>  	unsigned int		orientation_aware:1;
> @@ -286,4 +294,8 @@ int typec_find_orientation(const char *name);
>  int typec_find_port_power_role(const char *name);
>  int typec_find_power_role(const char *name);
>  int typec_find_port_data_role(const char *name);
> +
> +void typec_partner_set_svdm_version(struct typec_partner *partner,
> +				    enum usb_pd_svdm_ver svdm_version);
> +int typec_get_negotiated_svdm_version(struct typec_port *port);
>  #endif /* __LINUX_USB_TYPEC_H */
> diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
> index 5e0a7b7647c3..65933cbe9129 100644
> --- a/include/linux/usb/typec_altmode.h
> +++ b/include/linux/usb/typec_altmode.h
> @@ -132,6 +132,16 @@ typec_altmode_get_orientation(struct typec_altmode *altmode)
>  	return typec_get_orientation(typec_altmode2port(altmode));
>  }
>  
> +/**
> + * typec_altmode_get_svdm_version - Get negotiated SVDM version
> + * @altmode: Handle to the alternate mode
> + */
> +static inline int
> +typec_altmode_get_svdm_version(struct typec_altmode *altmode)
> +{
> +	return typec_get_negotiated_svdm_version(typec_altmode2port(altmode));
> +}
> +
>  /**
>   * struct typec_altmode_driver - USB Type-C alternate mode device driver
>   * @id_table: Null terminated array of SVIDs
> -- 
> 2.30.0.365.g02bc693789-goog
> 

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

* Re: [PATCH v6 2/7] usb: pd: Make SVDM Version configurable in VDM header
  2021-02-05  3:34 ` [PATCH v6 2/7] usb: pd: Make SVDM Version configurable in VDM header Kyle Tso
@ 2021-02-12  4:09   ` Guenter Roeck
  0 siblings, 0 replies; 24+ messages in thread
From: Guenter Roeck @ 2021-02-12  4:09 UTC (permalink / raw)
  To: Kyle Tso
  Cc: heikki.krogerus, gregkh, hdegoede, robh+dt, badhri, linux-usb,
	linux-kernel, devicetree

On Fri, Feb 05, 2021 at 11:34:10AM +0800, Kyle Tso wrote:
> PD Rev 3.0 introduces SVDM Version 2.0. This patch makes the field
> configuable in the header in order to be able to be compatible with
> older SVDM version.
> 
> Signed-off-by: Kyle Tso <kyletso@google.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Changes since v5:
> - no change
> 
>  drivers/usb/typec/altmodes/displayport.c |  2 +-
>  drivers/usb/typec/tcpm/tcpm.c            | 16 ++++++++--------
>  drivers/usb/typec/ucsi/displayport.c     |  6 +++---
>  include/linux/usb/pd_vdo.h               |  7 +++++--
>  4 files changed, 17 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
> index e62e5e3da01e..0abc3121238f 100644
> --- a/drivers/usb/typec/altmodes/displayport.c
> +++ b/drivers/usb/typec/altmodes/displayport.c
> @@ -15,7 +15,7 @@
>  #include <linux/usb/typec_dp.h>
>  #include "displayport.h"
>  
> -#define DP_HEADER(_dp, cmd)		(VDO((_dp)->alt->svid, 1, cmd) | \
> +#define DP_HEADER(_dp, cmd)		(VDO((_dp)->alt->svid, 1, SVDM_VER_1_0, cmd) | \
>  					 VDO_OPOS(USB_TYPEC_DP_MODE))
>  
>  enum {
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 8558ab006885..9aadb1e1bec5 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -1544,17 +1544,17 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  		case CMD_DISCOVER_IDENT:
>  			/* 6.4.4.3.1 */
>  			svdm_consume_identity(port, p, cnt);
> -			response[0] = VDO(USB_SID_PD, 1, CMD_DISCOVER_SVID);
> +			response[0] = VDO(USB_SID_PD, 1, SVDM_VER_1_0, CMD_DISCOVER_SVID);
>  			rlen = 1;
>  			break;
>  		case CMD_DISCOVER_SVID:
>  			/* 6.4.4.3.2 */
>  			if (svdm_consume_svids(port, p, cnt)) {
> -				response[0] = VDO(USB_SID_PD, 1,
> +				response[0] = VDO(USB_SID_PD, 1, SVDM_VER_1_0,
>  						  CMD_DISCOVER_SVID);
>  				rlen = 1;
>  			} else if (modep->nsvids && supports_modal(port)) {
> -				response[0] = VDO(modep->svids[0], 1,
> +				response[0] = VDO(modep->svids[0], 1, SVDM_VER_1_0,
>  						  CMD_DISCOVER_MODES);
>  				rlen = 1;
>  			}
> @@ -1565,7 +1565,7 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  			modep->svid_index++;
>  			if (modep->svid_index < modep->nsvids) {
>  				u16 svid = modep->svids[modep->svid_index];
> -				response[0] = VDO(svid, 1, CMD_DISCOVER_MODES);
> +				response[0] = VDO(svid, 1, SVDM_VER_1_0, CMD_DISCOVER_MODES);
>  				rlen = 1;
>  			} else {
>  				tcpm_register_partner_altmodes(port);
> @@ -1695,7 +1695,7 @@ static void tcpm_handle_vdm_request(struct tcpm_port *port,
>  			break;
>  		case ADEV_QUEUE_VDM_SEND_EXIT_MODE_ON_FAIL:
>  			if (typec_altmode_vdm(adev, p[0], &p[1], cnt)) {
> -				response[0] = VDO(adev->svid, 1, CMD_EXIT_MODE);
> +				response[0] = VDO(adev->svid, 1, SVDM_VER_1_0, CMD_EXIT_MODE);
>  				response[0] |= VDO_OPOS(adev->mode);
>  				rlen = 1;
>  			}
> @@ -1729,7 +1729,7 @@ static void tcpm_send_vdm(struct tcpm_port *port, u32 vid, int cmd,
>  
>  	/* set VDM header with VID & CMD */
>  	header = VDO(vid, ((vid & USB_SID_PD) == USB_SID_PD) ?
> -			1 : (PD_VDO_CMD(cmd) <= CMD_ATTENTION), cmd);
> +			1 : (PD_VDO_CMD(cmd) <= CMD_ATTENTION), SVDM_VER_1_0, cmd);
>  	tcpm_queue_vdm(port, header, data, count);
>  }
>  
> @@ -2024,7 +2024,7 @@ static int tcpm_altmode_enter(struct typec_altmode *altmode, u32 *vdo)
>  	struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
>  	u32 header;
>  
> -	header = VDO(altmode->svid, vdo ? 2 : 1, CMD_ENTER_MODE);
> +	header = VDO(altmode->svid, vdo ? 2 : 1, SVDM_VER_1_0, CMD_ENTER_MODE);
>  	header |= VDO_OPOS(altmode->mode);
>  
>  	tcpm_queue_vdm_unlocked(port, header, vdo, vdo ? 1 : 0);
> @@ -2036,7 +2036,7 @@ static int tcpm_altmode_exit(struct typec_altmode *altmode)
>  	struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
>  	u32 header;
>  
> -	header = VDO(altmode->svid, 1, CMD_EXIT_MODE);
> +	header = VDO(altmode->svid, 1, SVDM_VER_1_0, CMD_EXIT_MODE);
>  	header |= VDO_OPOS(altmode->mode);
>  
>  	tcpm_queue_vdm_unlocked(port, header, NULL, 0);
> diff --git a/drivers/usb/typec/ucsi/displayport.c b/drivers/usb/typec/ucsi/displayport.c
> index 261131c9e37c..1d387bddefb9 100644
> --- a/drivers/usb/typec/ucsi/displayport.c
> +++ b/drivers/usb/typec/ucsi/displayport.c
> @@ -83,7 +83,7 @@ static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo)
>  	 * mode, and letting the alt mode driver continue.
>  	 */
>  
> -	dp->header = VDO(USB_TYPEC_DP_SID, 1, CMD_ENTER_MODE);
> +	dp->header = VDO(USB_TYPEC_DP_SID, 1, SVDM_VER_1_0, CMD_ENTER_MODE);
>  	dp->header |= VDO_OPOS(USB_TYPEC_DP_MODE);
>  	dp->header |= VDO_CMDT(CMDT_RSP_ACK);
>  
> @@ -120,7 +120,7 @@ static int ucsi_displayport_exit(struct typec_altmode *alt)
>  	if (ret < 0)
>  		goto out_unlock;
>  
> -	dp->header = VDO(USB_TYPEC_DP_SID, 1, CMD_EXIT_MODE);
> +	dp->header = VDO(USB_TYPEC_DP_SID, 1, SVDM_VER_1_0, CMD_EXIT_MODE);
>  	dp->header |= VDO_OPOS(USB_TYPEC_DP_MODE);
>  	dp->header |= VDO_CMDT(CMDT_RSP_ACK);
>  
> @@ -200,7 +200,7 @@ static int ucsi_displayport_vdm(struct typec_altmode *alt,
>  
>  	switch (cmd_type) {
>  	case CMDT_INIT:
> -		dp->header = VDO(USB_TYPEC_DP_SID, 1, cmd);
> +		dp->header = VDO(USB_TYPEC_DP_SID, 1, SVDM_VER_1_0, cmd);
>  		dp->header |= VDO_OPOS(USB_TYPEC_DP_MODE);
>  
>  		switch (cmd) {
> diff --git a/include/linux/usb/pd_vdo.h b/include/linux/usb/pd_vdo.h
> index 5de7f550f93e..b057250704e8 100644
> --- a/include/linux/usb/pd_vdo.h
> +++ b/include/linux/usb/pd_vdo.h
> @@ -21,22 +21,24 @@
>   * ----------
>   * <31:16>  :: SVID
>   * <15>     :: VDM type ( 1b == structured, 0b == unstructured )
> - * <14:13>  :: Structured VDM version (can only be 00 == 1.0 currently)
> + * <14:13>  :: Structured VDM version
>   * <12:11>  :: reserved
>   * <10:8>   :: object position (1-7 valid ... used for enter/exit mode only)
>   * <7:6>    :: command type (SVDM only?)
>   * <5>      :: reserved (SVDM), command type (UVDM)
>   * <4:0>    :: command
>   */
> -#define VDO(vid, type, custom)				\
> +#define VDO(vid, type, ver, custom)			\
>  	(((vid) << 16) |				\
>  	 ((type) << 15) |				\
> +	 ((ver) << 13) |				\
>  	 ((custom) & 0x7FFF))
>  
>  #define VDO_SVDM_TYPE		(1 << 15)
>  #define VDO_SVDM_VERS(x)	((x) << 13)
>  #define VDO_OPOS(x)		((x) << 8)
>  #define VDO_CMDT(x)		((x) << 6)
> +#define VDO_SVDM_VERS_MASK	VDO_SVDM_VERS(0x3)
>  #define VDO_OPOS_MASK		VDO_OPOS(0x7)
>  #define VDO_CMDT_MASK		VDO_CMDT(0x3)
>  
> @@ -74,6 +76,7 @@
>  
>  #define PD_VDO_VID(vdo)		((vdo) >> 16)
>  #define PD_VDO_SVDM(vdo)	(((vdo) >> 15) & 1)
> +#define PD_VDO_SVDM_VER(vdo)	(((vdo) >> 13) & 0x3)
>  #define PD_VDO_OPOS(vdo)	(((vdo) >> 8) & 0x7)
>  #define PD_VDO_CMD(vdo)		((vdo) & 0x1f)
>  #define PD_VDO_CMDT(vdo)	(((vdo) >> 6) & 0x3)
> -- 
> 2.30.0.365.g02bc693789-goog
> 

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

* Re: [PATCH v6 3/7] usb: typec: tcpm: Determine common SVDM Version
  2021-02-05  3:34 ` [PATCH v6 3/7] usb: typec: tcpm: Determine common SVDM Version Kyle Tso
@ 2021-02-12  4:17   ` Guenter Roeck
  2021-02-12  7:10     ` Kyle Tso
  2021-02-12 15:20   ` Guenter Roeck
  1 sibling, 1 reply; 24+ messages in thread
From: Guenter Roeck @ 2021-02-12  4:17 UTC (permalink / raw)
  To: Kyle Tso
  Cc: heikki.krogerus, gregkh, hdegoede, robh+dt, badhri, linux-usb,
	linux-kernel, devicetree

On Fri, Feb 05, 2021 at 11:34:11AM +0800, Kyle Tso wrote:
> 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."
> 
> Also clear the fields newly defined in SVDM version 2.0 if the
> negotiated SVDM version is 1.0.
> 
> Signed-off-by: Kyle Tso <kyletso@google.com>
> ---
> Changes since v5:
> - follow the changes of "usb: typec: Manage SVDM version"
> - remove the "reset to default". Now the default SVDM version will be
>   set when calling to typec_register_partner
> 
>  drivers/usb/typec/tcpm/tcpm.c | 71 ++++++++++++++++++++++++++++++-----
>  1 file changed, 61 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 9aadb1e1bec5..b45cd191a8a4 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -1475,8 +1475,10 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  			const u32 *p, int cnt, u32 *response,
>  			enum adev_actions *adev_action)
>  {
> +	struct typec_port *typec = port->typec_port;
>  	struct typec_altmode *pdev;
>  	struct pd_mode_data *modep;
> +	int svdm_version;
>  	int rlen = 0;
>  	int cmd_type;
>  	int cmd;
> @@ -1493,6 +1495,10 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  	pdev = typec_match_altmode(port->partner_altmode, ALTMODE_DISCOVERY_MAX,
>  				   PD_VDO_VID(p[0]), PD_VDO_OPOS(p[0]));
>  
> +	svdm_version = typec_get_negotiated_svdm_version(typec);
> +	if (svdm_version < 0)
> +		return 0;
> +
>  	switch (cmd_type) {
>  	case CMDT_INIT:
>  		switch (cmd) {
> @@ -1500,10 +1506,22 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  			if (PD_VDO_VID(p[0]) != USB_SID_PD)
>  				break;
>  
> +			if (PD_VDO_SVDM_VER(p[0]) < svdm_version)
> +				typec_partner_set_svdm_version(port->partner,
> +							       PD_VDO_SVDM_VER(p[0]));
>  			/* 6.4.4.3.1: Only respond as UFP (device) */
>  			if (port->data_role == TYPEC_DEVICE &&
>  			    port->nr_snk_vdo) {
> -				for (i = 0; i <  port->nr_snk_vdo; i++)
> +				/*
> +				 * Product Type DFP and Connector Type are not defined in SVDM
> +				 * version 1.0 and shall be set to zero.
> +				 */
> +				if (typec_get_negotiated_svdm_version(typec) < SVDM_VER_2_0)

Why not
				if (svdm_version)
?

> +					response[1] = port->snk_vdo[0] & ~IDH_DFP_MASK
> +						      & ~IDH_CONN_MASK;
> +				else
> +					response[1] = port->snk_vdo[0];
> +				for (i = 1; i <  port->nr_snk_vdo; i++)
>  					response[i + 1] = port->snk_vdo[i];
>  				rlen = port->nr_snk_vdo + 1;
>  			}
> @@ -1532,6 +1550,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  			response[0] = p[0] | VDO_CMDT(CMDT_RSP_BUSY);
>  			rlen = 1;
>  		}
> +		response[0] = (response[0] & ~VDO_SVDM_VERS_MASK) |
> +			      (VDO_SVDM_VERS(typec_get_negotiated_svdm_version(typec)));

Unnecessary ( ) around VDO_SVDM_VERS. Also, why not svdm_version ?

>  		break;
>  	case CMDT_RSP_ACK:
>  		/* silently drop message if we are not connected */
> @@ -1542,19 +1562,22 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  
>  		switch (cmd) {
>  		case CMD_DISCOVER_IDENT:
> +			if (PD_VDO_SVDM_VER(p[0]) < svdm_version)
> +				typec_partner_set_svdm_version(port->partner,
> +							       PD_VDO_SVDM_VER(p[0]));
>  			/* 6.4.4.3.1 */
>  			svdm_consume_identity(port, p, cnt);
> -			response[0] = VDO(USB_SID_PD, 1, SVDM_VER_1_0, CMD_DISCOVER_SVID);
> +			response[0] = VDO(USB_SID_PD, 1, typec_get_negotiated_svdm_version(typec),

Guess I am a bit confused about the use of svdm_version vs.
typec_get_negotiated_svdm_version(typec). Is there some rationale
for using one vs. the other ?

> +					  CMD_DISCOVER_SVID);
>  			rlen = 1;
>  			break;
>  		case CMD_DISCOVER_SVID:
>  			/* 6.4.4.3.2 */
>  			if (svdm_consume_svids(port, p, cnt)) {
> -				response[0] = VDO(USB_SID_PD, 1, SVDM_VER_1_0,
> -						  CMD_DISCOVER_SVID);
> +				response[0] = VDO(USB_SID_PD, 1, svdm_version, CMD_DISCOVER_SVID);
>  				rlen = 1;
>  			} else if (modep->nsvids && supports_modal(port)) {
> -				response[0] = VDO(modep->svids[0], 1, SVDM_VER_1_0,
> +				response[0] = VDO(modep->svids[0], 1, svdm_version,
>  						  CMD_DISCOVER_MODES);
>  				rlen = 1;
>  			}
> @@ -1565,7 +1588,7 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  			modep->svid_index++;
>  			if (modep->svid_index < modep->nsvids) {
>  				u16 svid = modep->svids[modep->svid_index];
> -				response[0] = VDO(svid, 1, SVDM_VER_1_0, CMD_DISCOVER_MODES);
> +				response[0] = VDO(svid, 1, svdm_version, CMD_DISCOVER_MODES);
>  				rlen = 1;
>  			} else {
>  				tcpm_register_partner_altmodes(port);
> @@ -1592,6 +1615,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  			/* Unrecognized SVDM */
>  			response[0] = p[0] | VDO_CMDT(CMDT_RSP_NAK);
>  			rlen = 1;
> +			response[0] = (response[0] & ~VDO_SVDM_VERS_MASK) |
> +				      (VDO_SVDM_VERS(svdm_version));
>  			break;
>  		}
>  		break;
> @@ -1611,6 +1636,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  			/* Unrecognized SVDM */
>  			response[0] = p[0] | VDO_CMDT(CMDT_RSP_NAK);
>  			rlen = 1;
> +			response[0] = (response[0] & ~VDO_SVDM_VERS_MASK) |
> +				      (VDO_SVDM_VERS(svdm_version));
>  			break;
>  		}
>  		port->vdm_sm_running = false;
> @@ -1618,6 +1645,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  	default:
>  		response[0] = p[0] | VDO_CMDT(CMDT_RSP_NAK);
>  		rlen = 1;
> +		response[0] = (response[0] & ~VDO_SVDM_VERS_MASK) |
> +			      (VDO_SVDM_VERS(svdm_version));
>  		port->vdm_sm_running = false;
>  		break;
>  	}
> @@ -1695,7 +1724,13 @@ static void tcpm_handle_vdm_request(struct tcpm_port *port,
>  			break;
>  		case ADEV_QUEUE_VDM_SEND_EXIT_MODE_ON_FAIL:
>  			if (typec_altmode_vdm(adev, p[0], &p[1], cnt)) {
> -				response[0] = VDO(adev->svid, 1, SVDM_VER_1_0, CMD_EXIT_MODE);
> +				int svdm_version = typec_get_negotiated_svdm_version(
> +									port->typec_port);
> +				if (svdm_version < 0)
> +					break;
> +
> +				response[0] = VDO(adev->svid, 1, svdm_version,
> +						  CMD_EXIT_MODE);
>  				response[0] |= VDO_OPOS(adev->mode);
>  				rlen = 1;
>  			}
> @@ -1722,14 +1757,19 @@ static void tcpm_handle_vdm_request(struct tcpm_port *port,
>  static void tcpm_send_vdm(struct tcpm_port *port, u32 vid, int cmd,
>  			  const u32 *data, int count)
>  {
> +	int svdm_version = typec_get_negotiated_svdm_version(port->typec_port);
>  	u32 header;
>  
> +	if (svdm_version < 0)
> +		return;
> +
>  	if (WARN_ON(count > VDO_MAX_SIZE - 1))
>  		count = VDO_MAX_SIZE - 1;
>  
>  	/* set VDM header with VID & CMD */
>  	header = VDO(vid, ((vid & USB_SID_PD) == USB_SID_PD) ?
> -			1 : (PD_VDO_CMD(cmd) <= CMD_ATTENTION), SVDM_VER_1_0, cmd);
> +			1 : (PD_VDO_CMD(cmd) <= CMD_ATTENTION),
> +			svdm_version, cmd);
>  	tcpm_queue_vdm(port, header, data, count);
>  }
>  
> @@ -2022,9 +2062,14 @@ static int tcpm_validate_caps(struct tcpm_port *port, const u32 *pdo,
>  static int tcpm_altmode_enter(struct typec_altmode *altmode, u32 *vdo)
>  {
>  	struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
> +	int svdm_version;
>  	u32 header;
>  
> -	header = VDO(altmode->svid, vdo ? 2 : 1, SVDM_VER_1_0, CMD_ENTER_MODE);
> +	svdm_version = typec_get_negotiated_svdm_version(port->typec_port);
> +	if (svdm_version < 0)
> +		return svdm_version;
> +
> +	header = VDO(altmode->svid, vdo ? 2 : 1, svdm_version, CMD_ENTER_MODE);
>  	header |= VDO_OPOS(altmode->mode);
>  
>  	tcpm_queue_vdm_unlocked(port, header, vdo, vdo ? 1 : 0);
> @@ -2034,9 +2079,14 @@ static int tcpm_altmode_enter(struct typec_altmode *altmode, u32 *vdo)
>  static int tcpm_altmode_exit(struct typec_altmode *altmode)
>  {
>  	struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
> +	int svdm_version;
>  	u32 header;
>  
> -	header = VDO(altmode->svid, 1, SVDM_VER_1_0, CMD_EXIT_MODE);
> +	svdm_version = typec_get_negotiated_svdm_version(port->typec_port);
> +	if (svdm_version < 0)
> +		return svdm_version;
> +
> +	header = VDO(altmode->svid, 1, svdm_version, CMD_EXIT_MODE);
>  	header |= VDO_OPOS(altmode->mode);
>  
>  	tcpm_queue_vdm_unlocked(port, header, NULL, 0);
> @@ -5977,6 +6027,7 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
>  	port->typec_caps.fwnode = tcpc->fwnode;
>  	port->typec_caps.revision = 0x0120;	/* Type-C spec release 1.2 */
>  	port->typec_caps.pd_revision = 0x0300;	/* USB-PD spec release 3.0 */
> +	port->typec_caps.svdm_version = SVDM_VER_2_0;
>  	port->typec_caps.driver_data = port;
>  	port->typec_caps.ops = &tcpm_ops;
>  	port->typec_caps.orientation_aware = 1;
> -- 
> 2.30.0.365.g02bc693789-goog
> 

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

* Re: [PATCH v6 4/7] usb: typec: ucsi: Determine common SVDM Version
  2021-02-05  3:34 ` [PATCH v6 4/7] usb: typec: ucsi: " Kyle Tso
@ 2021-02-12  4:18   ` Guenter Roeck
  0 siblings, 0 replies; 24+ messages in thread
From: Guenter Roeck @ 2021-02-12  4:18 UTC (permalink / raw)
  To: Kyle Tso
  Cc: heikki.krogerus, gregkh, hdegoede, robh+dt, badhri, linux-usb,
	linux-kernel, devicetree

On Fri, Feb 05, 2021 at 11:34:12AM +0800, Kyle Tso wrote:
> This patch implements the following requirement in the Spec.
> 
> 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."
> 
> Signed-off-by: Kyle Tso <kyletso@google.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Changes since v5:
> - follow the changes of "usb: typec: Manage SVDM version"
> - remove the "reset to default". Now the default SVDM version will be
>   set when calling to typec_register_partner
> 
>  drivers/usb/typec/ucsi/displayport.c | 32 +++++++++++++++++++++++++---
>  drivers/usb/typec/ucsi/ucsi.c        |  1 +
>  2 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/typec/ucsi/displayport.c b/drivers/usb/typec/ucsi/displayport.c
> index 1d387bddefb9..73cd5bf35047 100644
> --- a/drivers/usb/typec/ucsi/displayport.c
> +++ b/drivers/usb/typec/ucsi/displayport.c
> @@ -49,6 +49,7 @@ static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo)
>  {
>  	struct ucsi_dp *dp = typec_altmode_get_drvdata(alt);
>  	struct ucsi *ucsi = dp->con->ucsi;
> +	int svdm_version;
>  	u64 command;
>  	u8 cur = 0;
>  	int ret;
> @@ -83,7 +84,13 @@ static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo)
>  	 * mode, and letting the alt mode driver continue.
>  	 */
>  
> -	dp->header = VDO(USB_TYPEC_DP_SID, 1, SVDM_VER_1_0, CMD_ENTER_MODE);
> +	svdm_version = typec_altmode_get_svdm_version(alt);
> +	if (svdm_version < 0) {
> +		ret = svdm_version;
> +		goto err_unlock;
> +	}
> +
> +	dp->header = VDO(USB_TYPEC_DP_SID, 1, svdm_version, CMD_ENTER_MODE);
>  	dp->header |= VDO_OPOS(USB_TYPEC_DP_MODE);
>  	dp->header |= VDO_CMDT(CMDT_RSP_ACK);
>  
> @@ -101,6 +108,7 @@ static int ucsi_displayport_enter(struct typec_altmode *alt, u32 *vdo)
>  static int ucsi_displayport_exit(struct typec_altmode *alt)
>  {
>  	struct ucsi_dp *dp = typec_altmode_get_drvdata(alt);
> +	int svdm_version;
>  	u64 command;
>  	int ret = 0;
>  
> @@ -120,7 +128,13 @@ static int ucsi_displayport_exit(struct typec_altmode *alt)
>  	if (ret < 0)
>  		goto out_unlock;
>  
> -	dp->header = VDO(USB_TYPEC_DP_SID, 1, SVDM_VER_1_0, CMD_EXIT_MODE);
> +	svdm_version = typec_altmode_get_svdm_version(alt);
> +	if (svdm_version < 0) {
> +		ret = svdm_version;
> +		goto out_unlock;
> +	}
> +
> +	dp->header = VDO(USB_TYPEC_DP_SID, 1, svdm_version, CMD_EXIT_MODE);
>  	dp->header |= VDO_OPOS(USB_TYPEC_DP_MODE);
>  	dp->header |= VDO_CMDT(CMDT_RSP_ACK);
>  
> @@ -186,6 +200,7 @@ static int ucsi_displayport_vdm(struct typec_altmode *alt,
>  	struct ucsi_dp *dp = typec_altmode_get_drvdata(alt);
>  	int cmd_type = PD_VDO_CMDT(header);
>  	int cmd = PD_VDO_CMD(header);
> +	int svdm_version;
>  
>  	mutex_lock(&dp->con->lock);
>  
> @@ -198,9 +213,20 @@ static int ucsi_displayport_vdm(struct typec_altmode *alt,
>  		return -EOPNOTSUPP;
>  	}
>  
> +	svdm_version = typec_altmode_get_svdm_version(alt);
> +	if (svdm_version < 0) {
> +		mutex_unlock(&dp->con->lock);
> +		return svdm_version;
> +	}
> +
>  	switch (cmd_type) {
>  	case CMDT_INIT:
> -		dp->header = VDO(USB_TYPEC_DP_SID, 1, SVDM_VER_1_0, cmd);
> +		if (PD_VDO_SVDM_VER(header) < svdm_version) {
> +			typec_partner_set_svdm_version(dp->con->partner, PD_VDO_SVDM_VER(header));
> +			svdm_version = PD_VDO_SVDM_VER(header);
> +		}
> +
> +		dp->header = VDO(USB_TYPEC_DP_SID, 1, svdm_version, cmd);
>  		dp->header |= VDO_OPOS(USB_TYPEC_DP_MODE);
>  
>  		switch (cmd) {
> diff --git a/drivers/usb/typec/ucsi/ucsi.c b/drivers/usb/typec/ucsi/ucsi.c
> index ca3f4194ad90..244270755ae6 100644
> --- a/drivers/usb/typec/ucsi/ucsi.c
> +++ b/drivers/usb/typec/ucsi/ucsi.c
> @@ -1052,6 +1052,7 @@ static int ucsi_register_port(struct ucsi *ucsi, int index)
>  
>  	cap->revision = ucsi->cap.typec_version;
>  	cap->pd_revision = ucsi->cap.pd_version;
> +	cap->svdm_version = SVDM_VER_2_0;
>  	cap->prefer_role = TYPEC_NO_PREFERRED_ROLE;
>  
>  	if (con->cap.op_mode & UCSI_CONCAP_OPMODE_AUDIO_ACCESSORY)
> -- 
> 2.30.0.365.g02bc693789-goog
> 

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

* Re: [PATCH v6 5/7] usb: typec: displayport: Fill the negotiated SVDM Version in the header
  2021-02-05  3:34 ` [PATCH v6 5/7] usb: typec: displayport: Fill the negotiated SVDM Version in the header Kyle Tso
@ 2021-02-12  4:19   ` Guenter Roeck
  0 siblings, 0 replies; 24+ messages in thread
From: Guenter Roeck @ 2021-02-12  4:19 UTC (permalink / raw)
  To: Kyle Tso
  Cc: heikki.krogerus, gregkh, hdegoede, robh+dt, badhri, linux-usb,
	linux-kernel, devicetree

On Fri, Feb 05, 2021 at 11:34:13AM +0800, Kyle Tso wrote:
> VDM header now requires SVDM Version. Get it from typec_partner.
> 
> Signed-off-by: Kyle Tso <kyletso@google.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Changes since v5:
> - follow the changes of "usb: typec: Manage SVDM version"
> 
>  drivers/usb/typec/altmodes/displayport.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/typec/altmodes/displayport.c b/drivers/usb/typec/altmodes/displayport.c
> index 0abc3121238f..b7f094435b00 100644
> --- a/drivers/usb/typec/altmodes/displayport.c
> +++ b/drivers/usb/typec/altmodes/displayport.c
> @@ -15,8 +15,8 @@
>  #include <linux/usb/typec_dp.h>
>  #include "displayport.h"
>  
> -#define DP_HEADER(_dp, cmd)		(VDO((_dp)->alt->svid, 1, SVDM_VER_1_0, cmd) | \
> -					 VDO_OPOS(USB_TYPEC_DP_MODE))
> +#define DP_HEADER(_dp, ver, cmd)	(VDO((_dp)->alt->svid, 1, ver, cmd)	\
> +					 | VDO_OPOS(USB_TYPEC_DP_MODE))
>  
>  enum {
>  	DP_CONF_USB,
> @@ -156,9 +156,14 @@ static int dp_altmode_configured(struct dp_altmode *dp)
>  
>  static int dp_altmode_configure_vdm(struct dp_altmode *dp, u32 conf)
>  {
> -	u32 header = DP_HEADER(dp, DP_CMD_CONFIGURE);
> +	int svdm_version = typec_altmode_get_svdm_version(dp->alt);
> +	u32 header;
>  	int ret;
>  
> +	if (svdm_version < 0)
> +		return svdm_version;
> +
> +	header = DP_HEADER(dp, svdm_version, DP_CMD_CONFIGURE);
>  	ret = typec_altmode_notify(dp->alt, TYPEC_STATE_SAFE, &dp->data);
>  	if (ret) {
>  		dev_err(&dp->alt->dev,
> @@ -181,6 +186,7 @@ static int dp_altmode_configure_vdm(struct dp_altmode *dp, u32 conf)
>  static void dp_altmode_work(struct work_struct *work)
>  {
>  	struct dp_altmode *dp = container_of(work, struct dp_altmode, work);
> +	int svdm_version;
>  	u32 header;
>  	u32 vdo;
>  	int ret;
> @@ -194,7 +200,10 @@ static void dp_altmode_work(struct work_struct *work)
>  			dev_err(&dp->alt->dev, "failed to enter mode\n");
>  		break;
>  	case DP_STATE_UPDATE:
> -		header = DP_HEADER(dp, DP_CMD_STATUS_UPDATE);
> +		svdm_version = typec_altmode_get_svdm_version(dp->alt);
> +		if (svdm_version < 0)
> +			break;
> +		header = DP_HEADER(dp, svdm_version, DP_CMD_STATUS_UPDATE);
>  		vdo = 1;
>  		ret = typec_altmode_vdm(dp->alt, header, &vdo, 2);
>  		if (ret)
> -- 
> 2.30.0.365.g02bc693789-goog
> 

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

* Re: [PATCH v6 6/7] dt-bindings: connector: Add SVDM VDO properties
  2021-02-05  3:34 ` [PATCH v6 6/7] dt-bindings: connector: Add SVDM VDO properties Kyle Tso
  2021-02-05 21:29   ` Rob Herring
@ 2021-02-12  4:21   ` Guenter Roeck
  2021-02-12  7:28     ` Kyle Tso
  1 sibling, 1 reply; 24+ messages in thread
From: Guenter Roeck @ 2021-02-12  4:21 UTC (permalink / raw)
  To: Kyle Tso
  Cc: heikki.krogerus, gregkh, hdegoede, robh+dt, badhri, linux-usb,
	linux-kernel, devicetree

On Fri, Feb 05, 2021 at 11:34:14AM +0800, Kyle Tso wrote:
> Add bindings of VDO properties of USB PD SVDM so that they can be
> used in device tree.
> 
> Signed-off-by: Kyle Tso <kyletso@google.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

Would it be possible to unify the dt definitions with the definitions
in include/linux/usb/pd_vdo.h ? I don't really like the duplication.

Thanks,
Guenter

> ---
> Changes since v5:
> - no change
> 
>  .../bindings/connector/usb-connector.yaml     |  11 +
>  include/dt-bindings/usb/pd.h                  | 311 +++++++++++++++++-
>  2 files changed, 321 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> index 4286ed767a0a..d385026944ec 100644
> --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> @@ -137,6 +137,17 @@ properties:
>      maxItems: 7
>      $ref: /schemas/types.yaml#/definitions/uint32-array
>  
> +  sink-vdos:
> +    description: An array of u32 with each entry (VDM Objects) providing additional information
> +      corresponding to the product, the detailed bit definitions and the order of each VDO can be
> +      found in "USB Power Delivery Specification Revision 3.0, Version 2.0 + ECNs 2020-12-10"
> +      chapter 6.4.4.3.1 Discover Identity. User can specify the VDO array via
> +      VDO_IDH/_CERT/_PRODUCT/_UFP/_DFP/_PCABLE/_ACABLE(1/2)/_VPD() defined in
> +      dt-bindings/usb/pd.h.
> +    minItems: 3
> +    maxItems: 6
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +
>    op-sink-microwatt:
>      description: Sink required operating power in microwatt, if source can't
>        offer the power, Capability Mismatch is set. Required for power sink and
> diff --git a/include/dt-bindings/usb/pd.h b/include/dt-bindings/usb/pd.h
> index 0352893697f0..fef3ef65967f 100644
> --- a/include/dt-bindings/usb/pd.h
> +++ b/include/dt-bindings/usb/pd.h
> @@ -93,4 +93,313 @@
>  #define FRS_DEFAULT_POWER      1
>  #define FRS_5V_1P5A            2
>  #define FRS_5V_3A              3
> - #endif /* __DT_POWER_DELIVERY_H */
> +
> +/*
> + * SVDM Identity Header
> + * --------------------
> + * <31>     :: data capable as a USB host
> + * <30>     :: data capable as a USB device
> + * <29:27>  :: product type (UFP / Cable / VPD)
> + * <26>     :: modal operation supported (1b == yes)
> + * <25:23>  :: product type (DFP) (SVDM version 2.0+ only; set to zero in version 1.0)
> + * <22:21>  :: connector type (SVDM version 2.0+ only; set to zero in version 1.0)
> + * <20:16>  :: Reserved, Shall be set to zero
> + * <15:0>   :: USB-IF assigned VID for this cable vendor
> + */
> +/* SOP Product Type (UFP) */
> +#define IDH_PTYPE_NOT_UFP       0
> +#define IDH_PTYPE_HUB           1
> +#define IDH_PTYPE_PERIPH        2
> +#define IDH_PTYPE_PSD           3
> +#define IDH_PTYPE_AMA           5
> +
> +/* SOP' Product Type (Cable Plug / VPD) */
> +#define IDH_PTYPE_NOT_CABLE     0
> +#define IDH_PTYPE_PCABLE        3
> +#define IDH_PTYPE_ACABLE        4
> +#define IDH_PTYPE_VPD           6
> +
> +/* SOP Product Type (DFP) */
> +#define IDH_PTYPE_NOT_DFP       0
> +#define IDH_PTYPE_DFP_HUB       1
> +#define IDH_PTYPE_DFP_HOST      2
> +#define IDH_PTYPE_DFP_PB        3
> +
> +#define VDO_IDH(usbh, usbd, ufp_cable, is_modal, dfp, conn, vid)                \
> +	((usbh) << 31 | (usbd) << 30 | ((ufp_cable) & 0x7) << 27                \
> +	 | (is_modal) << 26 | ((dfp) & 0x7) << 23 | ((conn) & 0x3) << 21        \
> +	 | ((vid) & 0xffff))
> +
> +/*
> + * Cert Stat VDO
> + * -------------
> + * <31:0>  : USB-IF assigned XID for this cable
> + */
> +#define VDO_CERT(xid)		((xid) & 0xffffffff)
> +
> +/*
> + * Product VDO
> + * -----------
> + * <31:16> : USB Product ID
> + * <15:0>  : USB bcdDevice
> + */
> +#define VDO_PRODUCT(pid, bcd)   (((pid) & 0xffff) << 16 | ((bcd) & 0xffff))
> +
> +/*
> + * UFP VDO (PD Revision 3.0+ only)
> + * --------
> + * <31:29> :: UFP VDO version
> + * <28>    :: Reserved
> + * <27:24> :: Device capability
> + * <23:22> :: Connector type (10b == receptacle, 11b == captive plug)
> + * <21:11> :: Reserved
> + * <10:8>  :: Vconn power (AMA only)
> + * <7>     :: Vconn required (AMA only, 0b == no, 1b == yes)
> + * <6>     :: Vbus required (AMA only, 0b == yes, 1b == no)
> + * <5:3>   :: Alternate modes
> + * <2:0>   :: USB highest speed
> + */
> +/* UFP VDO Version */
> +#define UFP_VDO_VER1_2		2
> +
> +/* Device Capability */
> +#define DEV_USB2_CAPABLE	BIT(0)
> +#define DEV_USB2_BILLBOARD	BIT(1)
> +#define DEV_USB3_CAPABLE	BIT(2)
> +#define DEV_USB4_CAPABLE	BIT(3)
> +
> +/* Connector Type */
> +#define UFP_RECEPTACLE		2
> +#define UFP_CAPTIVE		3
> +
> +/* Vconn Power (AMA only, set to AMA_VCONN_NOT_REQ if Vconn is not required) */
> +#define AMA_VCONN_PWR_1W	0
> +#define AMA_VCONN_PWR_1W5	1
> +#define AMA_VCONN_PWR_2W	2
> +#define AMA_VCONN_PWR_3W	3
> +#define AMA_VCONN_PWR_4W	4
> +#define AMA_VCONN_PWR_5W	5
> +#define AMA_VCONN_PWR_6W	6
> +
> +/* Vconn Required (AMA only) */
> +#define AMA_VCONN_NOT_REQ	0
> +#define AMA_VCONN_REQ		1
> +
> +/* Vbus Required (AMA only) */
> +#define AMA_VBUS_REQ		0
> +#define AMA_VBUS_NOT_REQ	1
> +
> +/* Alternate Modes */
> +#define UFP_ALTMODE_NOT_SUPP	0
> +#define UFP_ALTMODE_TBT3	BIT(0)
> +#define UFP_ALTMODE_RECFG	BIT(1)
> +#define UFP_ALTMODE_NO_RECFG	BIT(2)
> +
> +/* USB Highest Speed */
> +#define UFP_USB2_ONLY		0
> +#define UFP_USB32_GEN1		1
> +#define UFP_USB32_4_GEN2	2
> +#define UFP_USB4_GEN3		3
> +
> +#define VDO_UFP(ver, cap, conn, vcpwr, vcr, vbr, alt, spd)			\
> +	(((ver) & 0x7) << 29 | ((cap) & 0xf) << 24 | ((conn) & 0x3) << 22	\
> +	 | ((vcpwr) & 0x7) << 8 | (vcr) << 7 | (vbr) << 6 | ((alt) & 0x7) << 3	\
> +	 | ((spd) & 0x7))
> +
> +/*
> + * DFP VDO (PD Revision 3.0+ only)
> + * --------
> + * <31:29> :: DFP VDO version
> + * <28:27> :: Reserved
> + * <26:24> :: Host capability
> + * <23:22> :: Connector type (10b == receptacle, 11b == captive plug)
> + * <21:5>  :: Reserved
> + * <4:0>   :: Port number
> + */
> +#define DFP_VDO_VER1_1		1
> +#define HOST_USB2_CAPABLE	BIT(0)
> +#define HOST_USB3_CAPABLE	BIT(1)
> +#define HOST_USB4_CAPABLE	BIT(2)
> +#define DFP_RECEPTACLE		2
> +#define DFP_CAPTIVE		3
> +
> +#define VDO_DFP(ver, cap, conn, pnum)						\
> +	(((ver) & 0x7) << 29 | ((cap) & 0x7) << 24 | ((conn) & 0x3) << 22	\
> +	 | ((pnum) & 0x1f))
> +
> +/*
> + * Passive Cable VDO
> + * ---------
> + * <31:28> :: Cable HW version
> + * <27:24> :: Cable FW version
> + * <23:21> :: VDO version
> + * <20>    :: Reserved, Shall be set to zero
> + * <19:18> :: Type-C to Type-C/Captive (10b == C, 11b == Captive)
> + * <17>    :: Reserved, Shall be set to zero
> + * <16:13> :: cable latency (0001 == <10ns(~1m length))
> + * <12:11> :: cable termination type (10b == Vconn not req, 01b == Vconn req)
> + * <10:9>  :: Maximum Vbus voltage (00b == 20V, 01b == 30V, 10b == 40V, 11b == 50V)
> + * <8:7>   :: Reserved, Shall be set to zero
> + * <6:5>   :: Vbus current handling capability (01b == 3A, 10b == 5A)
> + * <4:3>   :: Reserved, Shall be set to zero
> + * <2:0>   :: USB highest speed
> + *
> + * Active Cable VDO 1
> + * ---------
> + * <31:28> :: Cable HW version
> + * <27:24> :: Cable FW version
> + * <23:21> :: VDO version
> + * <20>    :: Reserved, Shall be set to zero
> + * <19:18> :: Connector type (10b == C, 11b == Captive)
> + * <17>    :: Reserved, Shall be set to zero
> + * <16:13> :: cable latency (0001 == <10ns(~1m length))
> + * <12:11> :: cable termination type (10b == one end active, 11b == both ends active VCONN req)
> + * <10:9>  :: Maximum Vbus voltage (00b == 20V, 01b == 30V, 10b == 40V, 11b == 50V)
> + * <8>     :: SBU supported (0b == supported, 1b == not supported)
> + * <7>     :: SBU type (0b == passive, 1b == active)
> + * <6:5>   :: Vbus current handling capability (01b == 3A, 10b == 5A)
> + * <4>     :: Vbus through cable (0b == no, 1b == yes)
> + * <3>     :: SOP" controller present? (0b == no, 1b == yes)
> + * <2:0>   :: USB highest speed
> + */
> +/* Cable VDO Version */
> +#define CABLE_VDO_VER1_0	0
> +#define CABLE_VDO_VER1_3	3
> +
> +/* Connector Type */
> +#define CABLE_CTYPE		2
> +#define CABLE_CAPTIVE		3
> +
> +/* Cable Latency */
> +#define CABLE_LATENCY_1M	1
> +#define CABLE_LATENCY_2M	2
> +#define CABLE_LATENCY_3M	3
> +#define CABLE_LATENCY_4M	4
> +#define CABLE_LATENCY_5M	5
> +#define CABLE_LATENCY_6M	6
> +#define CABLE_LATENCY_7M	7
> +#define CABLE_LATENCY_7M_PLUS	8
> +
> +/* Cable Termination Type */
> +#define PCABLE_VCONN_NOT_REQ	0
> +#define PCABLE_VCONN_REQ	1
> +#define ACABLE_ONE_END		2
> +#define ACABLE_BOTH_END		3
> +
> +/* Maximum Vbus Voltage */
> +#define CABLE_MAX_VBUS_20V	0
> +#define CABLE_MAX_VBUS_30V	1
> +#define CABLE_MAX_VBUS_40V	2
> +#define CABLE_MAX_VBUS_50V	3
> +
> +/* Active Cable SBU Supported/Type */
> +#define ACABLE_SBU_SUPP		0
> +#define ACABLE_SBU_NOT_SUPP	1
> +#define ACABLE_SBU_PASSIVE	0
> +#define ACABLE_SBU_ACTIVE	1
> +
> +/* Vbus Current Handling Capability */
> +#define CABLE_CURR_DEF		0
> +#define CABLE_CURR_3A		1
> +#define CABLE_CURR_5A		2
> +
> +/* USB Highest Speed */
> +#define CABLE_USB2_ONLY		0
> +#define CABLE_USB32_GEN1	1
> +#define CABLE_USB32_4_GEN2	2
> +#define CABLE_USB4_GEN3		3
> +
> +#define VDO_PCABLE(hw, fw, ver, conn, lat, term, vbm, cur, spd)			\
> +	(((hw) & 0xf) << 28 | ((fw) & 0xf) << 24 | ((ver) & 0x7) << 21		\
> +	 | ((conn) & 0x3) << 18 | ((lat) & 0xf) << 13 | ((term) & 0x3) << 11	\
> +	 | ((vbm) & 0x3) << 9 | ((cur) & 0x3) << 5 | ((spd) & 0x7))
> +#define VDO_ACABLE1(hw, fw, ver, conn, lat, term, vbm, sbu, sbut, cur, vbt, sopp, spd) \
> +	(((hw) & 0xf) << 28 | ((fw) & 0xf) << 24 | ((ver) & 0x7) << 21		\
> +	 | ((conn) & 0x3) << 18	| ((lat) & 0xf) << 13 | ((term) & 0x3) << 11	\
> +	 | ((vbm) & 0x3) << 9 | (sbu) << 8 | (sbut) << 7 | ((cur) & 0x3) << 5	\
> +	 | (vbt) << 4 | (sopp) << 3 | ((spd) & 0x7))
> +
> +/*
> + * Active Cable VDO 2
> + * ---------
> + * <31:24> :: Maximum operating temperature
> + * <23:16> :: Shutdown temperature
> + * <15>    :: Reserved, Shall be set to zero
> + * <14:12> :: U3/CLd power
> + * <11>    :: U3 to U0 transition mode (0b == direct, 1b == through U3S)
> + * <10>    :: Physical connection (0b == copper, 1b == optical)
> + * <9>     :: Active element (0b == redriver, 1b == retimer)
> + * <8>     :: USB4 supported (0b == yes, 1b == no)
> + * <7:6>   :: USB2 hub hops consumed
> + * <5>     :: USB2 supported (0b == yes, 1b == no)
> + * <4>     :: USB3.2 supported (0b == yes, 1b == no)
> + * <3>     :: USB lanes supported (0b == one lane, 1b == two lanes)
> + * <2>     :: Optically isolated active cable (0b == no, 1b == yes)
> + * <1>     :: Reserved, Shall be set to zero
> + * <0>     :: USB gen (0b == gen1, 1b == gen2+)
> + */
> +/* U3/CLd Power*/
> +#define ACAB2_U3_CLD_10MW_PLUS	0
> +#define ACAB2_U3_CLD_10MW	1
> +#define ACAB2_U3_CLD_5MW	2
> +#define ACAB2_U3_CLD_1MW	3
> +#define ACAB2_U3_CLD_500UW	4
> +#define ACAB2_U3_CLD_200UW	5
> +#define ACAB2_U3_CLD_50UW	6
> +
> +/* Other Active Cable VDO 2 Fields */
> +#define ACAB2_U3U0_DIRECT	0
> +#define ACAB2_U3U0_U3S		1
> +#define ACAB2_PHY_COPPER	0
> +#define ACAB2_PHY_OPTICAL	1
> +#define ACAB2_REDRIVER		0
> +#define ACAB2_RETIMER		1
> +#define ACAB2_USB4_SUPP		0
> +#define ACAB2_USB4_NOT_SUPP	1
> +#define ACAB2_USB2_SUPP		0
> +#define ACAB2_USB2_NOT_SUPP	1
> +#define ACAB2_USB32_SUPP	0
> +#define ACAB2_USB32_NOT_SUPP	1
> +#define ACAB2_LANES_ONE		0
> +#define ACAB2_LANES_TWO		1
> +#define ACAB2_OPT_ISO_NO	0
> +#define ACAB2_OPT_ISO_YES	1
> +#define ACAB2_GEN_1		0
> +#define ACAB2_GEN_2_PLUS	1
> +
> +#define VDO_ACABLE2(mtemp, stemp, u3p, trans, phy, ele, u4, hops, u2, u32, lane, iso, gen)	\
> +	(((mtemp) & 0xff) << 24 | ((stemp) & 0xff) << 16 | ((u3p) & 0x7) << 12	\
> +	 | (trans) << 11 | (phy) << 10 | (ele) << 9 | (u4) << 8			\
> +	 | ((hops) & 0x3) << 6 | (u2) << 5 | (u32) << 4 | (lane) << 3		\
> +	 | (iso) << 2 | (gen))
> +
> +/*
> + * VPD VDO
> + * ---------
> + * <31:28> :: HW version
> + * <27:24> :: FW version
> + * <23:21> :: VDO version
> + * <20:17> :: Reserved, Shall be set to zero
> + * <16:15> :: Maximum Vbus voltage (00b == 20V, 01b == 30V, 10b == 40V, 11b == 50V)
> + * <14>    :: Charge through current support (0b == 3A, 1b == 5A)
> + * <13>    :: Reserved, Shall be set to zero
> + * <12:7>  :: Vbus impedance
> + * <6:1>   :: Ground impedance
> + * <0>     :: Charge through support (0b == no, 1b == yes)
> + */
> +#define VPD_VDO_VER1_0		0
> +#define VPD_MAX_VBUS_20V	0
> +#define VPD_MAX_VBUS_30V	1
> +#define VPD_MAX_VBUS_40V	2
> +#define VPD_MAX_VBUS_50V	3
> +#define VPDCT_CURR_3A		0
> +#define VPDCT_CURR_5A		1
> +#define VPDCT_NOT_SUPP		0
> +#define VPDCT_SUPP		1
> +
> +#define VDO_VPD(hw, fw, ver, vbm, curr, vbi, gi, ct)			\
> +	(((hw) & 0xf) << 28 | ((fw) & 0xf) << 24 | ((ver) & 0x7) << 21	\
> +	 | ((vbm) & 0x3) << 15 | (curr) << 14 | ((vbi) & 0x3f) << 7	\
> +	 | ((gi) & 0x3f) << 1 | (ct))
> +
> +#endif /* __DT_POWER_DELIVERY_H */
> -- 
> 2.30.0.365.g02bc693789-goog
> 

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

* Re: [PATCH v6 7/7] usb: typec: tcpm: Get Sink VDO from fwnode
  2021-02-05  3:34 ` [PATCH v6 7/7] usb: typec: tcpm: Get Sink VDO from fwnode Kyle Tso
@ 2021-02-12  4:21   ` Guenter Roeck
  0 siblings, 0 replies; 24+ messages in thread
From: Guenter Roeck @ 2021-02-12  4:21 UTC (permalink / raw)
  To: Kyle Tso
  Cc: heikki.krogerus, gregkh, hdegoede, robh+dt, badhri, linux-usb,
	linux-kernel, devicetree

On Fri, Feb 05, 2021 at 11:34:15AM +0800, Kyle Tso wrote:
> Commit a079973f462a ("usb: typec: tcpm: Remove tcpc_config
> configuration mechanism") removed the tcpc_config which includes the
> Sink VDO and it is not yet added back with fwnode. Add it now.
> 
> Signed-off-by: Kyle Tso <kyletso@google.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Changes since v5:
> - no change
> 
>  drivers/usb/typec/tcpm/tcpm.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index b45cd191a8a4..be0b6469dd3d 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -5722,6 +5722,20 @@ static int tcpm_fw_get_caps(struct tcpm_port *port,
>  			port->new_source_frs_current = frs_current;
>  	}
>  
> +	/* sink-vdos is optional */
> +	ret = fwnode_property_count_u32(fwnode, "sink-vdos");
> +	if (ret < 0)
> +		ret = 0;
> +
> +	port->nr_snk_vdo = min(ret, VDO_MAX_OBJECTS);
> +	if (port->nr_snk_vdo) {
> +		ret = fwnode_property_read_u32_array(fwnode, "sink-vdos",
> +						     port->snk_vdo,
> +						     port->nr_snk_vdo);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
>  	return 0;
>  }
>  
> -- 
> 2.30.0.365.g02bc693789-goog
> 

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

* Re: [PATCH v6 3/7] usb: typec: tcpm: Determine common SVDM Version
  2021-02-12  4:17   ` Guenter Roeck
@ 2021-02-12  7:10     ` Kyle Tso
  2021-02-12  7:24       ` Kyle Tso
  0 siblings, 1 reply; 24+ messages in thread
From: Kyle Tso @ 2021-02-12  7:10 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Heikki Krogerus, Greg KH, Hans de Goede, Rob Herring,
	Badhri Jagan Sridharan, USB, LKML,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Fri, Feb 12, 2021 at 12:17 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Fri, Feb 05, 2021 at 11:34:11AM +0800, Kyle Tso wrote:
> > 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."
> >
> > Also clear the fields newly defined in SVDM version 2.0 if the
> > negotiated SVDM version is 1.0.
> >
> > Signed-off-by: Kyle Tso <kyletso@google.com>
> > ---
> > Changes since v5:
> > - follow the changes of "usb: typec: Manage SVDM version"
> > - remove the "reset to default". Now the default SVDM version will be
> >   set when calling to typec_register_partner
> >
> >  drivers/usb/typec/tcpm/tcpm.c | 71 ++++++++++++++++++++++++++++++-----
> >  1 file changed, 61 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> > index 9aadb1e1bec5..b45cd191a8a4 100644
> > --- a/drivers/usb/typec/tcpm/tcpm.c
> > +++ b/drivers/usb/typec/tcpm/tcpm.c
> > @@ -1475,8 +1475,10 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
> >                       const u32 *p, int cnt, u32 *response,
> >                       enum adev_actions *adev_action)
> >  {
> > +     struct typec_port *typec = port->typec_port;
> >       struct typec_altmode *pdev;
> >       struct pd_mode_data *modep;
> > +     int svdm_version;
> >       int rlen = 0;
> >       int cmd_type;
> >       int cmd;
> > @@ -1493,6 +1495,10 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
> >       pdev = typec_match_altmode(port->partner_altmode, ALTMODE_DISCOVERY_MAX,
> >                                  PD_VDO_VID(p[0]), PD_VDO_OPOS(p[0]));
> >
> > +     svdm_version = typec_get_negotiated_svdm_version(typec);
> > +     if (svdm_version < 0)
> > +             return 0;
> > +
> >       switch (cmd_type) {
> >       case CMDT_INIT:
> >               switch (cmd) {
> > @@ -1500,10 +1506,22 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
> >                       if (PD_VDO_VID(p[0]) != USB_SID_PD)
> >                               break;
> >
> > +                     if (PD_VDO_SVDM_VER(p[0]) < svdm_version)
> > +                             typec_partner_set_svdm_version(port->partner,
> > +                                                            PD_VDO_SVDM_VER(p[0]));
> >                       /* 6.4.4.3.1: Only respond as UFP (device) */
> >                       if (port->data_role == TYPEC_DEVICE &&
> >                           port->nr_snk_vdo) {
> > -                             for (i = 0; i <  port->nr_snk_vdo; i++)
> > +                             /*
> > +                              * Product Type DFP and Connector Type are not defined in SVDM
> > +                              * version 1.0 and shall be set to zero.
> > +                              */
> > +                             if (typec_get_negotiated_svdm_version(typec) < SVDM_VER_2_0)
>
> Why not
>                                 if (svdm_version)
> ?
>

The "svdm_version" at this line is the cached value of
"partner->svdm_version" at the time from below lines.
In the case of the first calling to "tcpm_pd_svdm", this value is the
default value set when "typec_register_partner" is called.

>>>>>>>>>>>>>
       pdev = typec_match_altmode(port->partner_altmode, ALTMODE_DISCOVERY_MAX,
                                  PD_VDO_VID(p[0]), PD_VDO_OPOS(p[0]));

+     svdm_version = typec_get_negotiated_svdm_version(typec);
+     if (svdm_version < 0)
+             return 0;
<<<<<<<<<<<<<

"partner->svdm_version" is updated afterward If someone calls
"typec_partner_set_svdm_version" like these lines:
>>>>>>>>>>>>>
+                     if (PD_VDO_SVDM_VER(p[0]) < svdm_version)
+                             typec_partner_set_svdm_version(port->partner,
+
PD_VDO_SVDM_VER(p[0]));
<<<<<<<<<<<<<

However, this won't update the local variable "svdm_version". That's
why we need to get the value of "partner->svdm_version" again.
Unless every time the local variable "svdm_version" is updated when
"typec_partner_set_svdm_version" is called.


> > +                                     response[1] = port->snk_vdo[0] & ~IDH_DFP_MASK
> > +                                                   & ~IDH_CONN_MASK;
> > +                             else
> > +                                     response[1] = port->snk_vdo[0];
> > +                             for (i = 1; i <  port->nr_snk_vdo; i++)
> >                                       response[i + 1] = port->snk_vdo[i];
> >                               rlen = port->nr_snk_vdo + 1;
> >                       }
> > @@ -1532,6 +1550,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
> >                       response[0] = p[0] | VDO_CMDT(CMDT_RSP_BUSY);
> >                       rlen = 1;
> >               }
> > +             response[0] = (response[0] & ~VDO_SVDM_VERS_MASK) |
> > +                           (VDO_SVDM_VERS(typec_get_negotiated_svdm_version(typec)));
>
> Unnecessary ( ) around VDO_SVDM_VERS. Also, why not svdm_version ?
>
> >               break;
> >       case CMDT_RSP_ACK:
> >               /* silently drop message if we are not connected */
> > @@ -1542,19 +1562,22 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
> >
> >               switch (cmd) {
> >               case CMD_DISCOVER_IDENT:
> > +                     if (PD_VDO_SVDM_VER(p[0]) < svdm_version)
> > +                             typec_partner_set_svdm_version(port->partner,
> > +                                                            PD_VDO_SVDM_VER(p[0]));
> >                       /* 6.4.4.3.1 */
> >                       svdm_consume_identity(port, p, cnt);
> > -                     response[0] = VDO(USB_SID_PD, 1, SVDM_VER_1_0, CMD_DISCOVER_SVID);
> > +                     response[0] = VDO(USB_SID_PD, 1, typec_get_negotiated_svdm_version(typec),
>
> Guess I am a bit confused about the use of svdm_version vs.
> typec_get_negotiated_svdm_version(typec). Is there some rationale
> for using one vs. the other ?
>

The local variable "svdm_version" is get at the beginning of this function.
It cannot be trusted if someone calls "typec_partner_set_svdm_version"
before you use the local variable.
Unless, again,  it is updated everytime the
"typec_partner_set_svdm_version" is called.


> > +                                       CMD_DISCOVER_SVID);
> >                       rlen = 1;
> >                       break;
> >               case CMD_DISCOVER_SVID:
> >                       /* 6.4.4.3.2 */
> >                       if (svdm_consume_svids(port, p, cnt)) {
> > -                             response[0] = VDO(USB_SID_PD, 1, SVDM_VER_1_0,
> > -                                               CMD_DISCOVER_SVID);
> > +                             response[0] = VDO(USB_SID_PD, 1, svdm_version, CMD_DISCOVER_SVID);
> >                               rlen = 1;
> >                       } else if (modep->nsvids && supports_modal(port)) {
> > -                             response[0] = VDO(modep->svids[0], 1, SVDM_VER_1_0,
> > +                             response[0] = VDO(modep->svids[0], 1, svdm_version,
> >                                                 CMD_DISCOVER_MODES);
> >                               rlen = 1;
> >                       }
> > @@ -1565,7 +1588,7 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
> >                       modep->svid_index++;
> >                       if (modep->svid_index < modep->nsvids) {
> >                               u16 svid = modep->svids[modep->svid_index];
> > -                             response[0] = VDO(svid, 1, SVDM_VER_1_0, CMD_DISCOVER_MODES);
> > +                             response[0] = VDO(svid, 1, svdm_version, CMD_DISCOVER_MODES);
> >                               rlen = 1;
> >                       } else {
> >                               tcpm_register_partner_altmodes(port);
> > @@ -1592,6 +1615,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
> >                       /* Unrecognized SVDM */
> >                       response[0] = p[0] | VDO_CMDT(CMDT_RSP_NAK);
> >                       rlen = 1;
> > +                     response[0] = (response[0] & ~VDO_SVDM_VERS_MASK) |
> > +                                   (VDO_SVDM_VERS(svdm_version));
> >                       break;
> >               }
> >               break;
> > @@ -1611,6 +1636,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
> >                       /* Unrecognized SVDM */
> >                       response[0] = p[0] | VDO_CMDT(CMDT_RSP_NAK);
> >                       rlen = 1;
> > +                     response[0] = (response[0] & ~VDO_SVDM_VERS_MASK) |
> > +                                   (VDO_SVDM_VERS(svdm_version));
> >                       break;
> >               }
> >               port->vdm_sm_running = false;
> > @@ -1618,6 +1645,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
> >       default:
> >               response[0] = p[0] | VDO_CMDT(CMDT_RSP_NAK);
> >               rlen = 1;
> > +             response[0] = (response[0] & ~VDO_SVDM_VERS_MASK) |
> > +                           (VDO_SVDM_VERS(svdm_version));
> >               port->vdm_sm_running = false;
> >               break;
> >       }
> > @@ -1695,7 +1724,13 @@ static void tcpm_handle_vdm_request(struct tcpm_port *port,
> >                       break;
> >               case ADEV_QUEUE_VDM_SEND_EXIT_MODE_ON_FAIL:
> >                       if (typec_altmode_vdm(adev, p[0], &p[1], cnt)) {
> > -                             response[0] = VDO(adev->svid, 1, SVDM_VER_1_0, CMD_EXIT_MODE);
> > +                             int svdm_version = typec_get_negotiated_svdm_version(
> > +                                                                     port->typec_port);
> > +                             if (svdm_version < 0)
> > +                                     break;
> > +
> > +                             response[0] = VDO(adev->svid, 1, svdm_version,
> > +                                               CMD_EXIT_MODE);
> >                               response[0] |= VDO_OPOS(adev->mode);
> >                               rlen = 1;
> >                       }
> > @@ -1722,14 +1757,19 @@ static void tcpm_handle_vdm_request(struct tcpm_port *port,
> >  static void tcpm_send_vdm(struct tcpm_port *port, u32 vid, int cmd,
> >                         const u32 *data, int count)
> >  {
> > +     int svdm_version = typec_get_negotiated_svdm_version(port->typec_port);
> >       u32 header;
> >
> > +     if (svdm_version < 0)
> > +             return;
> > +
> >       if (WARN_ON(count > VDO_MAX_SIZE - 1))
> >               count = VDO_MAX_SIZE - 1;
> >
> >       /* set VDM header with VID & CMD */
> >       header = VDO(vid, ((vid & USB_SID_PD) == USB_SID_PD) ?
> > -                     1 : (PD_VDO_CMD(cmd) <= CMD_ATTENTION), SVDM_VER_1_0, cmd);
> > +                     1 : (PD_VDO_CMD(cmd) <= CMD_ATTENTION),
> > +                     svdm_version, cmd);
> >       tcpm_queue_vdm(port, header, data, count);
> >  }
> >
> > @@ -2022,9 +2062,14 @@ static int tcpm_validate_caps(struct tcpm_port *port, const u32 *pdo,
> >  static int tcpm_altmode_enter(struct typec_altmode *altmode, u32 *vdo)
> >  {
> >       struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
> > +     int svdm_version;
> >       u32 header;
> >
> > -     header = VDO(altmode->svid, vdo ? 2 : 1, SVDM_VER_1_0, CMD_ENTER_MODE);
> > +     svdm_version = typec_get_negotiated_svdm_version(port->typec_port);
> > +     if (svdm_version < 0)
> > +             return svdm_version;
> > +
> > +     header = VDO(altmode->svid, vdo ? 2 : 1, svdm_version, CMD_ENTER_MODE);
> >       header |= VDO_OPOS(altmode->mode);
> >
> >       tcpm_queue_vdm_unlocked(port, header, vdo, vdo ? 1 : 0);
> > @@ -2034,9 +2079,14 @@ static int tcpm_altmode_enter(struct typec_altmode *altmode, u32 *vdo)
> >  static int tcpm_altmode_exit(struct typec_altmode *altmode)
> >  {
> >       struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
> > +     int svdm_version;
> >       u32 header;
> >
> > -     header = VDO(altmode->svid, 1, SVDM_VER_1_0, CMD_EXIT_MODE);
> > +     svdm_version = typec_get_negotiated_svdm_version(port->typec_port);
> > +     if (svdm_version < 0)
> > +             return svdm_version;
> > +
> > +     header = VDO(altmode->svid, 1, svdm_version, CMD_EXIT_MODE);
> >       header |= VDO_OPOS(altmode->mode);
> >
> >       tcpm_queue_vdm_unlocked(port, header, NULL, 0);
> > @@ -5977,6 +6027,7 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
> >       port->typec_caps.fwnode = tcpc->fwnode;
> >       port->typec_caps.revision = 0x0120;     /* Type-C spec release 1.2 */
> >       port->typec_caps.pd_revision = 0x0300;  /* USB-PD spec release 3.0 */
> > +     port->typec_caps.svdm_version = SVDM_VER_2_0;
> >       port->typec_caps.driver_data = port;
> >       port->typec_caps.ops = &tcpm_ops;
> >       port->typec_caps.orientation_aware = 1;
> > --
> > 2.30.0.365.g02bc693789-goog
> >

thanks,
Kyle

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

* Re: [PATCH v6 3/7] usb: typec: tcpm: Determine common SVDM Version
  2021-02-12  7:10     ` Kyle Tso
@ 2021-02-12  7:24       ` Kyle Tso
  2021-02-12 15:18         ` Guenter Roeck
  0 siblings, 1 reply; 24+ messages in thread
From: Kyle Tso @ 2021-02-12  7:24 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Heikki Krogerus, Greg KH, Hans de Goede, Rob Herring,
	Badhri Jagan Sridharan, USB, LKML,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Fri, Feb 12, 2021 at 3:10 PM Kyle Tso <kyletso@google.com> wrote:
>
> On Fri, Feb 12, 2021 at 12:17 PM Guenter Roeck <linux@roeck-us.net> wrote:
> >
> > On Fri, Feb 05, 2021 at 11:34:11AM +0800, Kyle Tso wrote:
> > > 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."
> > >
> > > Also clear the fields newly defined in SVDM version 2.0 if the
> > > negotiated SVDM version is 1.0.
> > >
> > > Signed-off-by: Kyle Tso <kyletso@google.com>
> > > ---
> > > Changes since v5:
> > > - follow the changes of "usb: typec: Manage SVDM version"
> > > - remove the "reset to default". Now the default SVDM version will be
> > >   set when calling to typec_register_partner
> > >
> > >  drivers/usb/typec/tcpm/tcpm.c | 71 ++++++++++++++++++++++++++++++-----
> > >  1 file changed, 61 insertions(+), 10 deletions(-)
> > >
> > > diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> > > index 9aadb1e1bec5..b45cd191a8a4 100644
> > > --- a/drivers/usb/typec/tcpm/tcpm.c
> > > +++ b/drivers/usb/typec/tcpm/tcpm.c
> > > @@ -1475,8 +1475,10 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
> > >                       const u32 *p, int cnt, u32 *response,
> > >                       enum adev_actions *adev_action)
> > >  {
> > > +     struct typec_port *typec = port->typec_port;
> > >       struct typec_altmode *pdev;
> > >       struct pd_mode_data *modep;
> > > +     int svdm_version;
> > >       int rlen = 0;
> > >       int cmd_type;
> > >       int cmd;
> > > @@ -1493,6 +1495,10 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
> > >       pdev = typec_match_altmode(port->partner_altmode, ALTMODE_DISCOVERY_MAX,
> > >                                  PD_VDO_VID(p[0]), PD_VDO_OPOS(p[0]));
> > >
> > > +     svdm_version = typec_get_negotiated_svdm_version(typec);
> > > +     if (svdm_version < 0)
> > > +             return 0;
> > > +
> > >       switch (cmd_type) {
> > >       case CMDT_INIT:
> > >               switch (cmd) {
> > > @@ -1500,10 +1506,22 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
> > >                       if (PD_VDO_VID(p[0]) != USB_SID_PD)
> > >                               break;
> > >
> > > +                     if (PD_VDO_SVDM_VER(p[0]) < svdm_version)
> > > +                             typec_partner_set_svdm_version(port->partner,
> > > +                                                            PD_VDO_SVDM_VER(p[0]));
> > >                       /* 6.4.4.3.1: Only respond as UFP (device) */
> > >                       if (port->data_role == TYPEC_DEVICE &&
> > >                           port->nr_snk_vdo) {
> > > -                             for (i = 0; i <  port->nr_snk_vdo; i++)
> > > +                             /*
> > > +                              * Product Type DFP and Connector Type are not defined in SVDM
> > > +                              * version 1.0 and shall be set to zero.
> > > +                              */
> > > +                             if (typec_get_negotiated_svdm_version(typec) < SVDM_VER_2_0)
> >
> > Why not
> >                                 if (svdm_version)
> > ?
> >
>
> The "svdm_version" at this line is the cached value of
> "partner->svdm_version" at the time from below lines.
> In the case of the first calling to "tcpm_pd_svdm", this value is the
> default value set when "typec_register_partner" is called.
>
> >>>>>>>>>>>>>
>        pdev = typec_match_altmode(port->partner_altmode, ALTMODE_DISCOVERY_MAX,
>                                   PD_VDO_VID(p[0]), PD_VDO_OPOS(p[0]));
>
> +     svdm_version = typec_get_negotiated_svdm_version(typec);
> +     if (svdm_version < 0)
> +             return 0;
> <<<<<<<<<<<<<
>
> "partner->svdm_version" is updated afterward If someone calls
> "typec_partner_set_svdm_version" like these lines:
> >>>>>>>>>>>>>
> +                     if (PD_VDO_SVDM_VER(p[0]) < svdm_version)
> +                             typec_partner_set_svdm_version(port->partner,
> +
> PD_VDO_SVDM_VER(p[0]));
> <<<<<<<<<<<<<
>
> However, this won't update the local variable "svdm_version". That's
> why we need to get the value of "partner->svdm_version" again.


> Unless every time the local variable "svdm_version" is updated when
> "typec_partner_set_svdm_version" is called.
>

I can do that if it is clearer to do so.
It just needs two additional lines.

>
> > > +                                     response[1] = port->snk_vdo[0] & ~IDH_DFP_MASK
> > > +                                                   & ~IDH_CONN_MASK;
> > > +                             else
> > > +                                     response[1] = port->snk_vdo[0];
> > > +                             for (i = 1; i <  port->nr_snk_vdo; i++)
> > >                                       response[i + 1] = port->snk_vdo[i];
> > >                               rlen = port->nr_snk_vdo + 1;
> > >                       }
> > > @@ -1532,6 +1550,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
> > >                       response[0] = p[0] | VDO_CMDT(CMDT_RSP_BUSY);
> > >                       rlen = 1;
> > >               }
> > > +             response[0] = (response[0] & ~VDO_SVDM_VERS_MASK) |
> > > +                           (VDO_SVDM_VERS(typec_get_negotiated_svdm_version(typec)));
> >
> > Unnecessary ( ) around VDO_SVDM_VERS. Also, why not svdm_version ?
> >
> > >               break;
> > >       case CMDT_RSP_ACK:
> > >               /* silently drop message if we are not connected */
> > > @@ -1542,19 +1562,22 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
> > >
> > >               switch (cmd) {
> > >               case CMD_DISCOVER_IDENT:
> > > +                     if (PD_VDO_SVDM_VER(p[0]) < svdm_version)
> > > +                             typec_partner_set_svdm_version(port->partner,
> > > +                                                            PD_VDO_SVDM_VER(p[0]));
> > >                       /* 6.4.4.3.1 */
> > >                       svdm_consume_identity(port, p, cnt);
> > > -                     response[0] = VDO(USB_SID_PD, 1, SVDM_VER_1_0, CMD_DISCOVER_SVID);
> > > +                     response[0] = VDO(USB_SID_PD, 1, typec_get_negotiated_svdm_version(typec),
> >
> > Guess I am a bit confused about the use of svdm_version vs.
> > typec_get_negotiated_svdm_version(typec). Is there some rationale
> > for using one vs. the other ?
> >
>
> The local variable "svdm_version" is get at the beginning of this function.
> It cannot be trusted if someone calls "typec_partner_set_svdm_version"
> before you use the local variable.
> Unless, again,  it is updated everytime the
> "typec_partner_set_svdm_version" is called.
>
>
> > > +                                       CMD_DISCOVER_SVID);
> > >                       rlen = 1;
> > >                       break;
> > >               case CMD_DISCOVER_SVID:
> > >                       /* 6.4.4.3.2 */
> > >                       if (svdm_consume_svids(port, p, cnt)) {
> > > -                             response[0] = VDO(USB_SID_PD, 1, SVDM_VER_1_0,
> > > -                                               CMD_DISCOVER_SVID);
> > > +                             response[0] = VDO(USB_SID_PD, 1, svdm_version, CMD_DISCOVER_SVID);
> > >                               rlen = 1;
> > >                       } else if (modep->nsvids && supports_modal(port)) {
> > > -                             response[0] = VDO(modep->svids[0], 1, SVDM_VER_1_0,
> > > +                             response[0] = VDO(modep->svids[0], 1, svdm_version,
> > >                                                 CMD_DISCOVER_MODES);
> > >                               rlen = 1;
> > >                       }
> > > @@ -1565,7 +1588,7 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
> > >                       modep->svid_index++;
> > >                       if (modep->svid_index < modep->nsvids) {
> > >                               u16 svid = modep->svids[modep->svid_index];
> > > -                             response[0] = VDO(svid, 1, SVDM_VER_1_0, CMD_DISCOVER_MODES);
> > > +                             response[0] = VDO(svid, 1, svdm_version, CMD_DISCOVER_MODES);
> > >                               rlen = 1;
> > >                       } else {
> > >                               tcpm_register_partner_altmodes(port);
> > > @@ -1592,6 +1615,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
> > >                       /* Unrecognized SVDM */
> > >                       response[0] = p[0] | VDO_CMDT(CMDT_RSP_NAK);
> > >                       rlen = 1;
> > > +                     response[0] = (response[0] & ~VDO_SVDM_VERS_MASK) |
> > > +                                   (VDO_SVDM_VERS(svdm_version));
> > >                       break;
> > >               }
> > >               break;
> > > @@ -1611,6 +1636,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
> > >                       /* Unrecognized SVDM */
> > >                       response[0] = p[0] | VDO_CMDT(CMDT_RSP_NAK);
> > >                       rlen = 1;
> > > +                     response[0] = (response[0] & ~VDO_SVDM_VERS_MASK) |
> > > +                                   (VDO_SVDM_VERS(svdm_version));
> > >                       break;
> > >               }
> > >               port->vdm_sm_running = false;
> > > @@ -1618,6 +1645,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
> > >       default:
> > >               response[0] = p[0] | VDO_CMDT(CMDT_RSP_NAK);
> > >               rlen = 1;
> > > +             response[0] = (response[0] & ~VDO_SVDM_VERS_MASK) |
> > > +                           (VDO_SVDM_VERS(svdm_version));
> > >               port->vdm_sm_running = false;
> > >               break;
> > >       }
> > > @@ -1695,7 +1724,13 @@ static void tcpm_handle_vdm_request(struct tcpm_port *port,
> > >                       break;
> > >               case ADEV_QUEUE_VDM_SEND_EXIT_MODE_ON_FAIL:
> > >                       if (typec_altmode_vdm(adev, p[0], &p[1], cnt)) {
> > > -                             response[0] = VDO(adev->svid, 1, SVDM_VER_1_0, CMD_EXIT_MODE);
> > > +                             int svdm_version = typec_get_negotiated_svdm_version(
> > > +                                                                     port->typec_port);
> > > +                             if (svdm_version < 0)
> > > +                                     break;
> > > +
> > > +                             response[0] = VDO(adev->svid, 1, svdm_version,
> > > +                                               CMD_EXIT_MODE);
> > >                               response[0] |= VDO_OPOS(adev->mode);
> > >                               rlen = 1;
> > >                       }
> > > @@ -1722,14 +1757,19 @@ static void tcpm_handle_vdm_request(struct tcpm_port *port,
> > >  static void tcpm_send_vdm(struct tcpm_port *port, u32 vid, int cmd,
> > >                         const u32 *data, int count)
> > >  {
> > > +     int svdm_version = typec_get_negotiated_svdm_version(port->typec_port);
> > >       u32 header;
> > >
> > > +     if (svdm_version < 0)
> > > +             return;
> > > +
> > >       if (WARN_ON(count > VDO_MAX_SIZE - 1))
> > >               count = VDO_MAX_SIZE - 1;
> > >
> > >       /* set VDM header with VID & CMD */
> > >       header = VDO(vid, ((vid & USB_SID_PD) == USB_SID_PD) ?
> > > -                     1 : (PD_VDO_CMD(cmd) <= CMD_ATTENTION), SVDM_VER_1_0, cmd);
> > > +                     1 : (PD_VDO_CMD(cmd) <= CMD_ATTENTION),
> > > +                     svdm_version, cmd);
> > >       tcpm_queue_vdm(port, header, data, count);
> > >  }
> > >
> > > @@ -2022,9 +2062,14 @@ static int tcpm_validate_caps(struct tcpm_port *port, const u32 *pdo,
> > >  static int tcpm_altmode_enter(struct typec_altmode *altmode, u32 *vdo)
> > >  {
> > >       struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
> > > +     int svdm_version;
> > >       u32 header;
> > >
> > > -     header = VDO(altmode->svid, vdo ? 2 : 1, SVDM_VER_1_0, CMD_ENTER_MODE);
> > > +     svdm_version = typec_get_negotiated_svdm_version(port->typec_port);
> > > +     if (svdm_version < 0)
> > > +             return svdm_version;
> > > +
> > > +     header = VDO(altmode->svid, vdo ? 2 : 1, svdm_version, CMD_ENTER_MODE);
> > >       header |= VDO_OPOS(altmode->mode);
> > >
> > >       tcpm_queue_vdm_unlocked(port, header, vdo, vdo ? 1 : 0);
> > > @@ -2034,9 +2079,14 @@ static int tcpm_altmode_enter(struct typec_altmode *altmode, u32 *vdo)
> > >  static int tcpm_altmode_exit(struct typec_altmode *altmode)
> > >  {
> > >       struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
> > > +     int svdm_version;
> > >       u32 header;
> > >
> > > -     header = VDO(altmode->svid, 1, SVDM_VER_1_0, CMD_EXIT_MODE);
> > > +     svdm_version = typec_get_negotiated_svdm_version(port->typec_port);
> > > +     if (svdm_version < 0)
> > > +             return svdm_version;
> > > +
> > > +     header = VDO(altmode->svid, 1, svdm_version, CMD_EXIT_MODE);
> > >       header |= VDO_OPOS(altmode->mode);
> > >
> > >       tcpm_queue_vdm_unlocked(port, header, NULL, 0);
> > > @@ -5977,6 +6027,7 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
> > >       port->typec_caps.fwnode = tcpc->fwnode;
> > >       port->typec_caps.revision = 0x0120;     /* Type-C spec release 1.2 */
> > >       port->typec_caps.pd_revision = 0x0300;  /* USB-PD spec release 3.0 */
> > > +     port->typec_caps.svdm_version = SVDM_VER_2_0;
> > >       port->typec_caps.driver_data = port;
> > >       port->typec_caps.ops = &tcpm_ops;
> > >       port->typec_caps.orientation_aware = 1;
> > > --
> > > 2.30.0.365.g02bc693789-goog
> > >
>
> thanks,
> Kyle

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

* Re: [PATCH v6 6/7] dt-bindings: connector: Add SVDM VDO properties
  2021-02-12  4:21   ` Guenter Roeck
@ 2021-02-12  7:28     ` Kyle Tso
  0 siblings, 0 replies; 24+ messages in thread
From: Kyle Tso @ 2021-02-12  7:28 UTC (permalink / raw)
  To: Guenter Roeck
  Cc: Heikki Krogerus, Greg KH, Hans de Goede, Rob Herring,
	Badhri Jagan Sridharan, USB, LKML,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Fri, Feb 12, 2021 at 12:21 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Fri, Feb 05, 2021 at 11:34:14AM +0800, Kyle Tso wrote:
> > Add bindings of VDO properties of USB PD SVDM so that they can be
> > used in device tree.
> >
> > Signed-off-by: Kyle Tso <kyletso@google.com>
>
> Reviewed-by: Guenter Roeck <linux@roeck-us.net>
>
> Would it be possible to unify the dt definitions with the definitions
> in include/linux/usb/pd_vdo.h ? I don't really like the duplication.
>
> Thanks,
> Guenter
>

That's my question as well.
But I don't know how to do that.
Is it doable to include "include/linux/usb/pd_vdo.h" in the dt-bindings?

thanks,
Kyle

> > ---
> > Changes since v5:
> > - no change
> >
> >  .../bindings/connector/usb-connector.yaml     |  11 +
> >  include/dt-bindings/usb/pd.h                  | 311 +++++++++++++++++-
> >  2 files changed, 321 insertions(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/connector/usb-connector.yaml b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > index 4286ed767a0a..d385026944ec 100644
> > --- a/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > +++ b/Documentation/devicetree/bindings/connector/usb-connector.yaml
> > @@ -137,6 +137,17 @@ properties:
> >      maxItems: 7
> >      $ref: /schemas/types.yaml#/definitions/uint32-array
> >
> > +  sink-vdos:
> > +    description: An array of u32 with each entry (VDM Objects) providing additional information
> > +      corresponding to the product, the detailed bit definitions and the order of each VDO can be
> > +      found in "USB Power Delivery Specification Revision 3.0, Version 2.0 + ECNs 2020-12-10"
> > +      chapter 6.4.4.3.1 Discover Identity. User can specify the VDO array via
> > +      VDO_IDH/_CERT/_PRODUCT/_UFP/_DFP/_PCABLE/_ACABLE(1/2)/_VPD() defined in
> > +      dt-bindings/usb/pd.h.
> > +    minItems: 3
> > +    maxItems: 6
> > +    $ref: /schemas/types.yaml#/definitions/uint32-array
> > +
> >    op-sink-microwatt:
> >      description: Sink required operating power in microwatt, if source can't
> >        offer the power, Capability Mismatch is set. Required for power sink and
> > diff --git a/include/dt-bindings/usb/pd.h b/include/dt-bindings/usb/pd.h
> > index 0352893697f0..fef3ef65967f 100644
> > --- a/include/dt-bindings/usb/pd.h
> > +++ b/include/dt-bindings/usb/pd.h
> > @@ -93,4 +93,313 @@
> >  #define FRS_DEFAULT_POWER      1
> >  #define FRS_5V_1P5A            2
> >  #define FRS_5V_3A              3
> > - #endif /* __DT_POWER_DELIVERY_H */
> > +
> > +/*
> > + * SVDM Identity Header
> > + * --------------------
> > + * <31>     :: data capable as a USB host
> > + * <30>     :: data capable as a USB device
> > + * <29:27>  :: product type (UFP / Cable / VPD)
> > + * <26>     :: modal operation supported (1b == yes)
> > + * <25:23>  :: product type (DFP) (SVDM version 2.0+ only; set to zero in version 1.0)
> > + * <22:21>  :: connector type (SVDM version 2.0+ only; set to zero in version 1.0)
> > + * <20:16>  :: Reserved, Shall be set to zero
> > + * <15:0>   :: USB-IF assigned VID for this cable vendor
> > + */
> > +/* SOP Product Type (UFP) */
> > +#define IDH_PTYPE_NOT_UFP       0
> > +#define IDH_PTYPE_HUB           1
> > +#define IDH_PTYPE_PERIPH        2
> > +#define IDH_PTYPE_PSD           3
> > +#define IDH_PTYPE_AMA           5
> > +
> > +/* SOP' Product Type (Cable Plug / VPD) */
> > +#define IDH_PTYPE_NOT_CABLE     0
> > +#define IDH_PTYPE_PCABLE        3
> > +#define IDH_PTYPE_ACABLE        4
> > +#define IDH_PTYPE_VPD           6
> > +
> > +/* SOP Product Type (DFP) */
> > +#define IDH_PTYPE_NOT_DFP       0
> > +#define IDH_PTYPE_DFP_HUB       1
> > +#define IDH_PTYPE_DFP_HOST      2
> > +#define IDH_PTYPE_DFP_PB        3
> > +
> > +#define VDO_IDH(usbh, usbd, ufp_cable, is_modal, dfp, conn, vid)                \
> > +     ((usbh) << 31 | (usbd) << 30 | ((ufp_cable) & 0x7) << 27                \
> > +      | (is_modal) << 26 | ((dfp) & 0x7) << 23 | ((conn) & 0x3) << 21        \
> > +      | ((vid) & 0xffff))
> > +
> > +/*
> > + * Cert Stat VDO
> > + * -------------
> > + * <31:0>  : USB-IF assigned XID for this cable
> > + */
> > +#define VDO_CERT(xid)                ((xid) & 0xffffffff)
> > +
> > +/*
> > + * Product VDO
> > + * -----------
> > + * <31:16> : USB Product ID
> > + * <15:0>  : USB bcdDevice
> > + */
> > +#define VDO_PRODUCT(pid, bcd)   (((pid) & 0xffff) << 16 | ((bcd) & 0xffff))
> > +
> > +/*
> > + * UFP VDO (PD Revision 3.0+ only)
> > + * --------
> > + * <31:29> :: UFP VDO version
> > + * <28>    :: Reserved
> > + * <27:24> :: Device capability
> > + * <23:22> :: Connector type (10b == receptacle, 11b == captive plug)
> > + * <21:11> :: Reserved
> > + * <10:8>  :: Vconn power (AMA only)
> > + * <7>     :: Vconn required (AMA only, 0b == no, 1b == yes)
> > + * <6>     :: Vbus required (AMA only, 0b == yes, 1b == no)
> > + * <5:3>   :: Alternate modes
> > + * <2:0>   :: USB highest speed
> > + */
> > +/* UFP VDO Version */
> > +#define UFP_VDO_VER1_2               2
> > +
> > +/* Device Capability */
> > +#define DEV_USB2_CAPABLE     BIT(0)
> > +#define DEV_USB2_BILLBOARD   BIT(1)
> > +#define DEV_USB3_CAPABLE     BIT(2)
> > +#define DEV_USB4_CAPABLE     BIT(3)
> > +
> > +/* Connector Type */
> > +#define UFP_RECEPTACLE               2
> > +#define UFP_CAPTIVE          3
> > +
> > +/* Vconn Power (AMA only, set to AMA_VCONN_NOT_REQ if Vconn is not required) */
> > +#define AMA_VCONN_PWR_1W     0
> > +#define AMA_VCONN_PWR_1W5    1
> > +#define AMA_VCONN_PWR_2W     2
> > +#define AMA_VCONN_PWR_3W     3
> > +#define AMA_VCONN_PWR_4W     4
> > +#define AMA_VCONN_PWR_5W     5
> > +#define AMA_VCONN_PWR_6W     6
> > +
> > +/* Vconn Required (AMA only) */
> > +#define AMA_VCONN_NOT_REQ    0
> > +#define AMA_VCONN_REQ                1
> > +
> > +/* Vbus Required (AMA only) */
> > +#define AMA_VBUS_REQ         0
> > +#define AMA_VBUS_NOT_REQ     1
> > +
> > +/* Alternate Modes */
> > +#define UFP_ALTMODE_NOT_SUPP 0
> > +#define UFP_ALTMODE_TBT3     BIT(0)
> > +#define UFP_ALTMODE_RECFG    BIT(1)
> > +#define UFP_ALTMODE_NO_RECFG BIT(2)
> > +
> > +/* USB Highest Speed */
> > +#define UFP_USB2_ONLY                0
> > +#define UFP_USB32_GEN1               1
> > +#define UFP_USB32_4_GEN2     2
> > +#define UFP_USB4_GEN3                3
> > +
> > +#define VDO_UFP(ver, cap, conn, vcpwr, vcr, vbr, alt, spd)                   \
> > +     (((ver) & 0x7) << 29 | ((cap) & 0xf) << 24 | ((conn) & 0x3) << 22       \
> > +      | ((vcpwr) & 0x7) << 8 | (vcr) << 7 | (vbr) << 6 | ((alt) & 0x7) << 3  \
> > +      | ((spd) & 0x7))
> > +
> > +/*
> > + * DFP VDO (PD Revision 3.0+ only)
> > + * --------
> > + * <31:29> :: DFP VDO version
> > + * <28:27> :: Reserved
> > + * <26:24> :: Host capability
> > + * <23:22> :: Connector type (10b == receptacle, 11b == captive plug)
> > + * <21:5>  :: Reserved
> > + * <4:0>   :: Port number
> > + */
> > +#define DFP_VDO_VER1_1               1
> > +#define HOST_USB2_CAPABLE    BIT(0)
> > +#define HOST_USB3_CAPABLE    BIT(1)
> > +#define HOST_USB4_CAPABLE    BIT(2)
> > +#define DFP_RECEPTACLE               2
> > +#define DFP_CAPTIVE          3
> > +
> > +#define VDO_DFP(ver, cap, conn, pnum)                                                \
> > +     (((ver) & 0x7) << 29 | ((cap) & 0x7) << 24 | ((conn) & 0x3) << 22       \
> > +      | ((pnum) & 0x1f))
> > +
> > +/*
> > + * Passive Cable VDO
> > + * ---------
> > + * <31:28> :: Cable HW version
> > + * <27:24> :: Cable FW version
> > + * <23:21> :: VDO version
> > + * <20>    :: Reserved, Shall be set to zero
> > + * <19:18> :: Type-C to Type-C/Captive (10b == C, 11b == Captive)
> > + * <17>    :: Reserved, Shall be set to zero
> > + * <16:13> :: cable latency (0001 == <10ns(~1m length))
> > + * <12:11> :: cable termination type (10b == Vconn not req, 01b == Vconn req)
> > + * <10:9>  :: Maximum Vbus voltage (00b == 20V, 01b == 30V, 10b == 40V, 11b == 50V)
> > + * <8:7>   :: Reserved, Shall be set to zero
> > + * <6:5>   :: Vbus current handling capability (01b == 3A, 10b == 5A)
> > + * <4:3>   :: Reserved, Shall be set to zero
> > + * <2:0>   :: USB highest speed
> > + *
> > + * Active Cable VDO 1
> > + * ---------
> > + * <31:28> :: Cable HW version
> > + * <27:24> :: Cable FW version
> > + * <23:21> :: VDO version
> > + * <20>    :: Reserved, Shall be set to zero
> > + * <19:18> :: Connector type (10b == C, 11b == Captive)
> > + * <17>    :: Reserved, Shall be set to zero
> > + * <16:13> :: cable latency (0001 == <10ns(~1m length))
> > + * <12:11> :: cable termination type (10b == one end active, 11b == both ends active VCONN req)
> > + * <10:9>  :: Maximum Vbus voltage (00b == 20V, 01b == 30V, 10b == 40V, 11b == 50V)
> > + * <8>     :: SBU supported (0b == supported, 1b == not supported)
> > + * <7>     :: SBU type (0b == passive, 1b == active)
> > + * <6:5>   :: Vbus current handling capability (01b == 3A, 10b == 5A)
> > + * <4>     :: Vbus through cable (0b == no, 1b == yes)
> > + * <3>     :: SOP" controller present? (0b == no, 1b == yes)
> > + * <2:0>   :: USB highest speed
> > + */
> > +/* Cable VDO Version */
> > +#define CABLE_VDO_VER1_0     0
> > +#define CABLE_VDO_VER1_3     3
> > +
> > +/* Connector Type */
> > +#define CABLE_CTYPE          2
> > +#define CABLE_CAPTIVE                3
> > +
> > +/* Cable Latency */
> > +#define CABLE_LATENCY_1M     1
> > +#define CABLE_LATENCY_2M     2
> > +#define CABLE_LATENCY_3M     3
> > +#define CABLE_LATENCY_4M     4
> > +#define CABLE_LATENCY_5M     5
> > +#define CABLE_LATENCY_6M     6
> > +#define CABLE_LATENCY_7M     7
> > +#define CABLE_LATENCY_7M_PLUS        8
> > +
> > +/* Cable Termination Type */
> > +#define PCABLE_VCONN_NOT_REQ 0
> > +#define PCABLE_VCONN_REQ     1
> > +#define ACABLE_ONE_END               2
> > +#define ACABLE_BOTH_END              3
> > +
> > +/* Maximum Vbus Voltage */
> > +#define CABLE_MAX_VBUS_20V   0
> > +#define CABLE_MAX_VBUS_30V   1
> > +#define CABLE_MAX_VBUS_40V   2
> > +#define CABLE_MAX_VBUS_50V   3
> > +
> > +/* Active Cable SBU Supported/Type */
> > +#define ACABLE_SBU_SUPP              0
> > +#define ACABLE_SBU_NOT_SUPP  1
> > +#define ACABLE_SBU_PASSIVE   0
> > +#define ACABLE_SBU_ACTIVE    1
> > +
> > +/* Vbus Current Handling Capability */
> > +#define CABLE_CURR_DEF               0
> > +#define CABLE_CURR_3A                1
> > +#define CABLE_CURR_5A                2
> > +
> > +/* USB Highest Speed */
> > +#define CABLE_USB2_ONLY              0
> > +#define CABLE_USB32_GEN1     1
> > +#define CABLE_USB32_4_GEN2   2
> > +#define CABLE_USB4_GEN3              3
> > +
> > +#define VDO_PCABLE(hw, fw, ver, conn, lat, term, vbm, cur, spd)                      \
> > +     (((hw) & 0xf) << 28 | ((fw) & 0xf) << 24 | ((ver) & 0x7) << 21          \
> > +      | ((conn) & 0x3) << 18 | ((lat) & 0xf) << 13 | ((term) & 0x3) << 11    \
> > +      | ((vbm) & 0x3) << 9 | ((cur) & 0x3) << 5 | ((spd) & 0x7))
> > +#define VDO_ACABLE1(hw, fw, ver, conn, lat, term, vbm, sbu, sbut, cur, vbt, sopp, spd) \
> > +     (((hw) & 0xf) << 28 | ((fw) & 0xf) << 24 | ((ver) & 0x7) << 21          \
> > +      | ((conn) & 0x3) << 18 | ((lat) & 0xf) << 13 | ((term) & 0x3) << 11    \
> > +      | ((vbm) & 0x3) << 9 | (sbu) << 8 | (sbut) << 7 | ((cur) & 0x3) << 5   \
> > +      | (vbt) << 4 | (sopp) << 3 | ((spd) & 0x7))
> > +
> > +/*
> > + * Active Cable VDO 2
> > + * ---------
> > + * <31:24> :: Maximum operating temperature
> > + * <23:16> :: Shutdown temperature
> > + * <15>    :: Reserved, Shall be set to zero
> > + * <14:12> :: U3/CLd power
> > + * <11>    :: U3 to U0 transition mode (0b == direct, 1b == through U3S)
> > + * <10>    :: Physical connection (0b == copper, 1b == optical)
> > + * <9>     :: Active element (0b == redriver, 1b == retimer)
> > + * <8>     :: USB4 supported (0b == yes, 1b == no)
> > + * <7:6>   :: USB2 hub hops consumed
> > + * <5>     :: USB2 supported (0b == yes, 1b == no)
> > + * <4>     :: USB3.2 supported (0b == yes, 1b == no)
> > + * <3>     :: USB lanes supported (0b == one lane, 1b == two lanes)
> > + * <2>     :: Optically isolated active cable (0b == no, 1b == yes)
> > + * <1>     :: Reserved, Shall be set to zero
> > + * <0>     :: USB gen (0b == gen1, 1b == gen2+)
> > + */
> > +/* U3/CLd Power*/
> > +#define ACAB2_U3_CLD_10MW_PLUS       0
> > +#define ACAB2_U3_CLD_10MW    1
> > +#define ACAB2_U3_CLD_5MW     2
> > +#define ACAB2_U3_CLD_1MW     3
> > +#define ACAB2_U3_CLD_500UW   4
> > +#define ACAB2_U3_CLD_200UW   5
> > +#define ACAB2_U3_CLD_50UW    6
> > +
> > +/* Other Active Cable VDO 2 Fields */
> > +#define ACAB2_U3U0_DIRECT    0
> > +#define ACAB2_U3U0_U3S               1
> > +#define ACAB2_PHY_COPPER     0
> > +#define ACAB2_PHY_OPTICAL    1
> > +#define ACAB2_REDRIVER               0
> > +#define ACAB2_RETIMER                1
> > +#define ACAB2_USB4_SUPP              0
> > +#define ACAB2_USB4_NOT_SUPP  1
> > +#define ACAB2_USB2_SUPP              0
> > +#define ACAB2_USB2_NOT_SUPP  1
> > +#define ACAB2_USB32_SUPP     0
> > +#define ACAB2_USB32_NOT_SUPP 1
> > +#define ACAB2_LANES_ONE              0
> > +#define ACAB2_LANES_TWO              1
> > +#define ACAB2_OPT_ISO_NO     0
> > +#define ACAB2_OPT_ISO_YES    1
> > +#define ACAB2_GEN_1          0
> > +#define ACAB2_GEN_2_PLUS     1
> > +
> > +#define VDO_ACABLE2(mtemp, stemp, u3p, trans, phy, ele, u4, hops, u2, u32, lane, iso, gen)   \
> > +     (((mtemp) & 0xff) << 24 | ((stemp) & 0xff) << 16 | ((u3p) & 0x7) << 12  \
> > +      | (trans) << 11 | (phy) << 10 | (ele) << 9 | (u4) << 8                 \
> > +      | ((hops) & 0x3) << 6 | (u2) << 5 | (u32) << 4 | (lane) << 3           \
> > +      | (iso) << 2 | (gen))
> > +
> > +/*
> > + * VPD VDO
> > + * ---------
> > + * <31:28> :: HW version
> > + * <27:24> :: FW version
> > + * <23:21> :: VDO version
> > + * <20:17> :: Reserved, Shall be set to zero
> > + * <16:15> :: Maximum Vbus voltage (00b == 20V, 01b == 30V, 10b == 40V, 11b == 50V)
> > + * <14>    :: Charge through current support (0b == 3A, 1b == 5A)
> > + * <13>    :: Reserved, Shall be set to zero
> > + * <12:7>  :: Vbus impedance
> > + * <6:1>   :: Ground impedance
> > + * <0>     :: Charge through support (0b == no, 1b == yes)
> > + */
> > +#define VPD_VDO_VER1_0               0
> > +#define VPD_MAX_VBUS_20V     0
> > +#define VPD_MAX_VBUS_30V     1
> > +#define VPD_MAX_VBUS_40V     2
> > +#define VPD_MAX_VBUS_50V     3
> > +#define VPDCT_CURR_3A                0
> > +#define VPDCT_CURR_5A                1
> > +#define VPDCT_NOT_SUPP               0
> > +#define VPDCT_SUPP           1
> > +
> > +#define VDO_VPD(hw, fw, ver, vbm, curr, vbi, gi, ct)                 \
> > +     (((hw) & 0xf) << 28 | ((fw) & 0xf) << 24 | ((ver) & 0x7) << 21  \
> > +      | ((vbm) & 0x3) << 15 | (curr) << 14 | ((vbi) & 0x3f) << 7     \
> > +      | ((gi) & 0x3f) << 1 | (ct))
> > +
> > +#endif /* __DT_POWER_DELIVERY_H */
> > --
> > 2.30.0.365.g02bc693789-goog
> >

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

* Re: [PATCH v6 3/7] usb: typec: tcpm: Determine common SVDM Version
  2021-02-12  7:24       ` Kyle Tso
@ 2021-02-12 15:18         ` Guenter Roeck
  0 siblings, 0 replies; 24+ messages in thread
From: Guenter Roeck @ 2021-02-12 15:18 UTC (permalink / raw)
  To: Kyle Tso
  Cc: Heikki Krogerus, Greg KH, Hans de Goede, Rob Herring,
	Badhri Jagan Sridharan, USB, LKML,
	open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS

On Fri, Feb 12, 2021 at 03:24:22PM +0800, Kyle Tso wrote:
> On Fri, Feb 12, 2021 at 3:10 PM Kyle Tso <kyletso@google.com> wrote:
> 
> > Unless every time the local variable "svdm_version" is updated when
> > "typec_partner_set_svdm_version" is called.
> >
> 
> I can do that if it is clearer to do so.
> It just needs two additional lines.
> 

Don't bother; I don't want to hold up that series any further.

Guenter

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

* Re: [PATCH v6 3/7] usb: typec: tcpm: Determine common SVDM Version
  2021-02-05  3:34 ` [PATCH v6 3/7] usb: typec: tcpm: Determine common SVDM Version Kyle Tso
  2021-02-12  4:17   ` Guenter Roeck
@ 2021-02-12 15:20   ` Guenter Roeck
  1 sibling, 0 replies; 24+ messages in thread
From: Guenter Roeck @ 2021-02-12 15:20 UTC (permalink / raw)
  To: Kyle Tso, heikki.krogerus, gregkh, hdegoede, robh+dt
  Cc: badhri, linux-usb, linux-kernel, devicetree

On 2/4/21 7:34 PM, Kyle Tso wrote:
> 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."
> 
> Also clear the fields newly defined in SVDM version 2.0 if the
> negotiated SVDM version is 1.0.
> 
> Signed-off-by: Kyle Tso <kyletso@google.com>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
> Changes since v5:
> - follow the changes of "usb: typec: Manage SVDM version"
> - remove the "reset to default". Now the default SVDM version will be
>   set when calling to typec_register_partner
> 
>  drivers/usb/typec/tcpm/tcpm.c | 71 ++++++++++++++++++++++++++++++-----
>  1 file changed, 61 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 9aadb1e1bec5..b45cd191a8a4 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -1475,8 +1475,10 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  			const u32 *p, int cnt, u32 *response,
>  			enum adev_actions *adev_action)
>  {
> +	struct typec_port *typec = port->typec_port;
>  	struct typec_altmode *pdev;
>  	struct pd_mode_data *modep;
> +	int svdm_version;
>  	int rlen = 0;
>  	int cmd_type;
>  	int cmd;
> @@ -1493,6 +1495,10 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  	pdev = typec_match_altmode(port->partner_altmode, ALTMODE_DISCOVERY_MAX,
>  				   PD_VDO_VID(p[0]), PD_VDO_OPOS(p[0]));
>  
> +	svdm_version = typec_get_negotiated_svdm_version(typec);
> +	if (svdm_version < 0)
> +		return 0;
> +
>  	switch (cmd_type) {
>  	case CMDT_INIT:
>  		switch (cmd) {
> @@ -1500,10 +1506,22 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  			if (PD_VDO_VID(p[0]) != USB_SID_PD)
>  				break;
>  
> +			if (PD_VDO_SVDM_VER(p[0]) < svdm_version)
> +				typec_partner_set_svdm_version(port->partner,
> +							       PD_VDO_SVDM_VER(p[0]));
>  			/* 6.4.4.3.1: Only respond as UFP (device) */
>  			if (port->data_role == TYPEC_DEVICE &&
>  			    port->nr_snk_vdo) {
> -				for (i = 0; i <  port->nr_snk_vdo; i++)
> +				/*
> +				 * Product Type DFP and Connector Type are not defined in SVDM
> +				 * version 1.0 and shall be set to zero.
> +				 */
> +				if (typec_get_negotiated_svdm_version(typec) < SVDM_VER_2_0)
> +					response[1] = port->snk_vdo[0] & ~IDH_DFP_MASK
> +						      & ~IDH_CONN_MASK;
> +				else
> +					response[1] = port->snk_vdo[0];
> +				for (i = 1; i <  port->nr_snk_vdo; i++)
>  					response[i + 1] = port->snk_vdo[i];
>  				rlen = port->nr_snk_vdo + 1;
>  			}
> @@ -1532,6 +1550,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  			response[0] = p[0] | VDO_CMDT(CMDT_RSP_BUSY);
>  			rlen = 1;
>  		}
> +		response[0] = (response[0] & ~VDO_SVDM_VERS_MASK) |
> +			      (VDO_SVDM_VERS(typec_get_negotiated_svdm_version(typec)));
>  		break;
>  	case CMDT_RSP_ACK:
>  		/* silently drop message if we are not connected */
> @@ -1542,19 +1562,22 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  
>  		switch (cmd) {
>  		case CMD_DISCOVER_IDENT:
> +			if (PD_VDO_SVDM_VER(p[0]) < svdm_version)
> +				typec_partner_set_svdm_version(port->partner,
> +							       PD_VDO_SVDM_VER(p[0]));
>  			/* 6.4.4.3.1 */
>  			svdm_consume_identity(port, p, cnt);
> -			response[0] = VDO(USB_SID_PD, 1, SVDM_VER_1_0, CMD_DISCOVER_SVID);
> +			response[0] = VDO(USB_SID_PD, 1, typec_get_negotiated_svdm_version(typec),
> +					  CMD_DISCOVER_SVID);
>  			rlen = 1;
>  			break;
>  		case CMD_DISCOVER_SVID:
>  			/* 6.4.4.3.2 */
>  			if (svdm_consume_svids(port, p, cnt)) {
> -				response[0] = VDO(USB_SID_PD, 1, SVDM_VER_1_0,
> -						  CMD_DISCOVER_SVID);
> +				response[0] = VDO(USB_SID_PD, 1, svdm_version, CMD_DISCOVER_SVID);
>  				rlen = 1;
>  			} else if (modep->nsvids && supports_modal(port)) {
> -				response[0] = VDO(modep->svids[0], 1, SVDM_VER_1_0,
> +				response[0] = VDO(modep->svids[0], 1, svdm_version,
>  						  CMD_DISCOVER_MODES);
>  				rlen = 1;
>  			}
> @@ -1565,7 +1588,7 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  			modep->svid_index++;
>  			if (modep->svid_index < modep->nsvids) {
>  				u16 svid = modep->svids[modep->svid_index];
> -				response[0] = VDO(svid, 1, SVDM_VER_1_0, CMD_DISCOVER_MODES);
> +				response[0] = VDO(svid, 1, svdm_version, CMD_DISCOVER_MODES);
>  				rlen = 1;
>  			} else {
>  				tcpm_register_partner_altmodes(port);
> @@ -1592,6 +1615,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  			/* Unrecognized SVDM */
>  			response[0] = p[0] | VDO_CMDT(CMDT_RSP_NAK);
>  			rlen = 1;
> +			response[0] = (response[0] & ~VDO_SVDM_VERS_MASK) |
> +				      (VDO_SVDM_VERS(svdm_version));
>  			break;
>  		}
>  		break;
> @@ -1611,6 +1636,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  			/* Unrecognized SVDM */
>  			response[0] = p[0] | VDO_CMDT(CMDT_RSP_NAK);
>  			rlen = 1;
> +			response[0] = (response[0] & ~VDO_SVDM_VERS_MASK) |
> +				      (VDO_SVDM_VERS(svdm_version));
>  			break;
>  		}
>  		port->vdm_sm_running = false;
> @@ -1618,6 +1645,8 @@ static int tcpm_pd_svdm(struct tcpm_port *port, struct typec_altmode *adev,
>  	default:
>  		response[0] = p[0] | VDO_CMDT(CMDT_RSP_NAK);
>  		rlen = 1;
> +		response[0] = (response[0] & ~VDO_SVDM_VERS_MASK) |
> +			      (VDO_SVDM_VERS(svdm_version));
>  		port->vdm_sm_running = false;
>  		break;
>  	}
> @@ -1695,7 +1724,13 @@ static void tcpm_handle_vdm_request(struct tcpm_port *port,
>  			break;
>  		case ADEV_QUEUE_VDM_SEND_EXIT_MODE_ON_FAIL:
>  			if (typec_altmode_vdm(adev, p[0], &p[1], cnt)) {
> -				response[0] = VDO(adev->svid, 1, SVDM_VER_1_0, CMD_EXIT_MODE);
> +				int svdm_version = typec_get_negotiated_svdm_version(
> +									port->typec_port);
> +				if (svdm_version < 0)
> +					break;
> +
> +				response[0] = VDO(adev->svid, 1, svdm_version,
> +						  CMD_EXIT_MODE);
>  				response[0] |= VDO_OPOS(adev->mode);
>  				rlen = 1;
>  			}
> @@ -1722,14 +1757,19 @@ static void tcpm_handle_vdm_request(struct tcpm_port *port,
>  static void tcpm_send_vdm(struct tcpm_port *port, u32 vid, int cmd,
>  			  const u32 *data, int count)
>  {
> +	int svdm_version = typec_get_negotiated_svdm_version(port->typec_port);
>  	u32 header;
>  
> +	if (svdm_version < 0)
> +		return;
> +
>  	if (WARN_ON(count > VDO_MAX_SIZE - 1))
>  		count = VDO_MAX_SIZE - 1;
>  
>  	/* set VDM header with VID & CMD */
>  	header = VDO(vid, ((vid & USB_SID_PD) == USB_SID_PD) ?
> -			1 : (PD_VDO_CMD(cmd) <= CMD_ATTENTION), SVDM_VER_1_0, cmd);
> +			1 : (PD_VDO_CMD(cmd) <= CMD_ATTENTION),
> +			svdm_version, cmd);
>  	tcpm_queue_vdm(port, header, data, count);
>  }
>  
> @@ -2022,9 +2062,14 @@ static int tcpm_validate_caps(struct tcpm_port *port, const u32 *pdo,
>  static int tcpm_altmode_enter(struct typec_altmode *altmode, u32 *vdo)
>  {
>  	struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
> +	int svdm_version;
>  	u32 header;
>  
> -	header = VDO(altmode->svid, vdo ? 2 : 1, SVDM_VER_1_0, CMD_ENTER_MODE);
> +	svdm_version = typec_get_negotiated_svdm_version(port->typec_port);
> +	if (svdm_version < 0)
> +		return svdm_version;
> +
> +	header = VDO(altmode->svid, vdo ? 2 : 1, svdm_version, CMD_ENTER_MODE);
>  	header |= VDO_OPOS(altmode->mode);
>  
>  	tcpm_queue_vdm_unlocked(port, header, vdo, vdo ? 1 : 0);
> @@ -2034,9 +2079,14 @@ static int tcpm_altmode_enter(struct typec_altmode *altmode, u32 *vdo)
>  static int tcpm_altmode_exit(struct typec_altmode *altmode)
>  {
>  	struct tcpm_port *port = typec_altmode_get_drvdata(altmode);
> +	int svdm_version;
>  	u32 header;
>  
> -	header = VDO(altmode->svid, 1, SVDM_VER_1_0, CMD_EXIT_MODE);
> +	svdm_version = typec_get_negotiated_svdm_version(port->typec_port);
> +	if (svdm_version < 0)
> +		return svdm_version;
> +
> +	header = VDO(altmode->svid, 1, svdm_version, CMD_EXIT_MODE);
>  	header |= VDO_OPOS(altmode->mode);
>  
>  	tcpm_queue_vdm_unlocked(port, header, NULL, 0);
> @@ -5977,6 +6027,7 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
>  	port->typec_caps.fwnode = tcpc->fwnode;
>  	port->typec_caps.revision = 0x0120;	/* Type-C spec release 1.2 */
>  	port->typec_caps.pd_revision = 0x0300;	/* USB-PD spec release 3.0 */
> +	port->typec_caps.svdm_version = SVDM_VER_2_0;
>  	port->typec_caps.driver_data = port;
>  	port->typec_caps.ops = &tcpm_ops;
>  	port->typec_caps.orientation_aware = 1;
> 


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

end of thread, other threads:[~2021-02-12 15:24 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-05  3:34 [PATCH v6 0/7] common SVDM version and VDO from dt Kyle Tso
2021-02-05  3:34 ` [PATCHi v6 1/7] usb: typec: Manage SVDM version Kyle Tso
2021-02-12  4:09   ` Guenter Roeck
2021-02-05  3:34 ` [PATCH v6 2/7] usb: pd: Make SVDM Version configurable in VDM header Kyle Tso
2021-02-12  4:09   ` Guenter Roeck
2021-02-05  3:34 ` [PATCH v6 3/7] usb: typec: tcpm: Determine common SVDM Version Kyle Tso
2021-02-12  4:17   ` Guenter Roeck
2021-02-12  7:10     ` Kyle Tso
2021-02-12  7:24       ` Kyle Tso
2021-02-12 15:18         ` Guenter Roeck
2021-02-12 15:20   ` Guenter Roeck
2021-02-05  3:34 ` [PATCH v6 4/7] usb: typec: ucsi: " Kyle Tso
2021-02-12  4:18   ` Guenter Roeck
2021-02-05  3:34 ` [PATCH v6 5/7] usb: typec: displayport: Fill the negotiated SVDM Version in the header Kyle Tso
2021-02-12  4:19   ` Guenter Roeck
2021-02-05  3:34 ` [PATCH v6 6/7] dt-bindings: connector: Add SVDM VDO properties Kyle Tso
2021-02-05 21:29   ` Rob Herring
2021-02-06  0:29     ` Kyle Tso
2021-02-12  4:21   ` Guenter Roeck
2021-02-12  7:28     ` Kyle Tso
2021-02-05  3:34 ` [PATCH v6 7/7] usb: typec: tcpm: Get Sink VDO from fwnode Kyle Tso
2021-02-12  4:21   ` Guenter Roeck
2021-02-05 11:42 ` [PATCH v6 0/7] common SVDM version and VDO from dt Heikki Krogerus
2021-02-05 12:20   ` Guenter Roeck

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).