usb_pd: Remove pd_cable usage from common code

Previously, the Discovery Identity SOP' response for TCPMv1/2 was being
stored in pd_discovery and in pd_cable. This commit removes the storage
of Discover Identity SOP' response from the pd_cable structure.

BUG=b:158294748
    b:159504972
BRANCH=None
TEST=1. Able to get the cable characteristics
     2. Able to enter into Thunderbolt mode on TCPMv1
     3. Able to enter into USB4 mode on TCPMv1

Change-Id: I1e5112f9aa158c41abb6226a3819f1612ed906bd
Signed-off-by: Ayushee <ayushee.shah@intel.com>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2247211
Reviewed-by: Abe Levkoy <alevkoy@chromium.org>
Commit-Queue: Abe Levkoy <alevkoy@chromium.org>

6 files changed, 156 insertions, 198 deletions

diff --git a/common/usb_pd_alt_mode_dfp.c b/common/usb_pd_alt_mode_dfp.c
index cb34c547fb..dcca9dde47 100644
--- a/common/usb_pd_alt_mode_dfp.c
+++ b/common/usb_pd_alt_mode_dfp.c
@@ -313,12 +313,24 @@ void dfp_consume_attention(int port, uint32_t *payload)
 		modep->fx->attention(port, payload);
 }
 
-void dfp_consume_identity(int port, int cnt, uint32_t *payload)
+void dfp_consume_identity(int port, enum tcpm_transmit_type type, int cnt,
+		uint32_t *payload)
 {
-	int ptype = PD_IDH_PTYPE(payload[VDO_I(IDH)]);
-	struct pd_discovery *disc = pd_get_am_discovery(port, TCPC_TX_SOP);
-	size_t identity_size = MIN(sizeof(union disc_ident_ack),
+	int ptype;
+	struct pd_discovery *disc;
+	size_t identity_size;
+
+	if (type == TCPC_TX_SOP_PRIME &&
+	    !IS_ENABLED(CONFIG_USB_PD_DECODE_SOP)) {
+		CPRINTF("ERR:Unexpected cable response\n");
+		return;
+	}
+
+	ptype = PD_IDH_PTYPE(payload[VDO_I(IDH)]);
+	disc = pd_get_am_discovery(port, type);
+	identity_size = MIN(sizeof(union disc_ident_ack),
 				   (cnt - 1) * sizeof(uint32_t));
+
 	/* Note: only store VDOs, not the VDM header */
 	memcpy(disc->identity.raw_value, payload + 1, identity_size);
 
@@ -340,7 +352,7 @@ void dfp_consume_identity(int port, int cnt, uint32_t *payload)
 	default:
 		break;
 	}
-	pd_set_identity_discovery(port, TCPC_TX_SOP, PD_DISC_COMPLETE);
+	pd_set_identity_discovery(port, type, PD_DISC_COMPLETE);
 }
 
 void dfp_consume_svids(int port, enum tcpm_transmit_type type, int cnt,
@@ -644,6 +656,11 @@ bool is_vdo_present(int cnt, int index)
 	return cnt > index;
 }
 
+static inline bool is_rev3_vdo(int port, enum tcpm_transmit_type type)
+{
+	return pd_get_vdo_ver(port, type) == PD_REV30;
+}
+
 /*
  * ############################################################################
  *
@@ -653,30 +670,37 @@ bool is_vdo_present(int cnt, int index)
  */
 enum idh_ptype get_usb_pd_cable_type(int port)
 {
-	struct pd_cable *cable = pd_get_cable_attributes(port);
+	struct pd_discovery *disc =
+		pd_get_am_discovery(port, TCPC_TX_SOP_PRIME);
 
-	return cable->type;
+	return disc->identity.idh.product_type;
 }
 
 bool is_usb2_cable_support(int port)
 {
-	struct pd_cable *cable = pd_get_cable_attributes(port);
+	struct pd_discovery *disc =
+		pd_get_am_discovery(port, TCPC_TX_SOP_PRIME);
 
-	return cable->type == IDH_PTYPE_PCABLE ||
-		cable->attr2.a2_rev30.usb_20_support == USB2_SUPPORTED;
+	return disc->identity.idh.product_type == IDH_PTYPE_PCABLE ||
+	       disc->identity.product_t2.a2_rev30.usb_20_support ==
+							USB2_SUPPORTED;
 }
 
 bool is_cable_speed_gen2_capable(int port)
 {
-	struct pd_cable *cable = pd_get_cable_attributes(port);
+	struct pd_discovery *disc =
+		pd_get_am_discovery(port, TCPC_TX_SOP_PRIME);
 
-	switch (cable->rev) {
+	switch (pd_get_vdo_ver(port, TCPC_TX_SOP_PRIME)) {
 	case PD_REV20:
-		return cable->attr.p_rev20.ss == USB_R20_SS_U31_GEN1_GEN2;
+		return disc->identity.product_t1.p_rev20.ss ==
+						USB_R20_SS_U31_GEN1_GEN2;
 
 	case PD_REV30:
-		return cable->attr.p_rev30.ss == USB_R30_SS_U32_U40_GEN2 ||
-			cable->attr.p_rev30.ss == USB_R30_SS_U40_GEN3;
+		return disc->identity.product_t1.p_rev30.ss ==
+						USB_R30_SS_U32_U40_GEN2 ||
+		       disc->identity.product_t1.p_rev30.ss ==
+						USB_R30_SS_U40_GEN3;
 	default:
 		return false;
 	}
@@ -684,58 +708,16 @@ bool is_cable_speed_gen2_capable(int port)
 
 bool is_active_cable_element_retimer(int port)
 {
-	struct pd_cable *cable = pd_get_cable_attributes(port);
+	struct pd_discovery *disc =
+		pd_get_am_discovery(port, TCPC_TX_SOP_PRIME);
 
 	/* Ref: USB PD Spec 2.0 Table 6-29 Active Cable VDO
 	 * Revision 2 Active cables do not have Active element support.
 	 */
-	return cable->rev & PD_REV30 &&
-		get_usb_pd_cable_type(port) == IDH_PTYPE_ACABLE &&
-		cable->attr2.a2_rev30.active_elem == ACTIVE_RETIMER;
-}
-
-/*
- * TODO(b/152417597): Support SOP and SOP'; eliminate redundant code for port
- * partner and cable identity discovery.
- */
-void dfp_consume_cable_response(int port, int cnt, uint32_t *payload,
-				uint32_t head)
-{
-	struct pd_cable *cable = pd_get_cable_attributes(port);
-	struct pd_discovery *disc =
-		pd_get_am_discovery(port, TCPC_TX_SOP_PRIME);
-	size_t identity_size = MIN(sizeof(union disc_ident_ack),
-				   (cnt - 1) * sizeof(uint32_t));
-
-	if (!IS_ENABLED(CONFIG_USB_PD_DECODE_SOP))
-		return;
-
-	/* Note: only store VDOs, not the VDM header */
-	memcpy(disc->identity.raw_value, payload + 1, identity_size);
-
-	pd_set_identity_discovery(port, TCPC_TX_SOP_PRIME, PD_DISC_COMPLETE);
-
-	/* Get cable rev */
-	cable->rev = PD_HEADER_REV(head);
-
-	/* TODO: Move cable references to use discovery response */
-	if (is_vdo_present(cnt, VDO_INDEX_IDH)) {
-		cable->type = PD_IDH_PTYPE(payload[VDO_INDEX_IDH]);
-
-		if (is_vdo_present(cnt, VDO_INDEX_PTYPE_CABLE1))
-			cable->attr.raw_value =
-					payload[VDO_INDEX_PTYPE_CABLE1];
-
-		/*
-		 * Ref USB PD Spec 3.0  Pg 145. For active cable there are two
-		 * VDOs. Hence storing the second VDO.
-		 */
-		if (is_vdo_present(cnt, VDO_INDEX_PTYPE_CABLE2))
-			cable->attr2.raw_value =
-					payload[VDO_INDEX_PTYPE_CABLE2];
-
-		cable->is_identified = 1;
-	}
+	return is_rev3_vdo(port, TCPC_TX_SOP_PRIME) &&
+		disc->identity.idh.product_type == IDH_PTYPE_ACABLE &&
+		disc->identity.product_t2.a2_rev30.active_elem ==
+							ACTIVE_RETIMER;
 }
 
 /*
@@ -767,28 +749,34 @@ void set_tbt_compat_mode_ready(int port)
  */
 bool is_tbt_cable_superspeed(int port)
 {
-	struct pd_cable *cable;
+	struct pd_discovery *disc;
 
 	if (!IS_ENABLED(CONFIG_USB_PD_TBT_COMPAT_MODE) ||
 	    !IS_ENABLED(CONFIG_USB_PD_DECODE_SOP))
 		return false;
 
-	cable = pd_get_cable_attributes(port);
+	disc = pd_get_am_discovery(port, TCPC_TX_SOP_PRIME);
 
 	/* Product type is Active cable, hence don't check for speed */
-	if (cable->type == IDH_PTYPE_ACABLE)
+	if (disc->identity.idh.product_type == IDH_PTYPE_ACABLE)
 		return true;
 
-	if (cable->type != IDH_PTYPE_PCABLE)
+	if (disc->identity.idh.product_type != IDH_PTYPE_PCABLE)
 		return false;
 
-	if (IS_ENABLED(CONFIG_USB_PD_REV30) && cable->rev == PD_REV30)
-		return cable->attr.p_rev30.ss == USB_R30_SS_U32_U40_GEN1 ||
-			cable->attr.p_rev30.ss == USB_R30_SS_U32_U40_GEN2 ||
-			cable->attr.p_rev30.ss == USB_R30_SS_U40_GEN3;
+	if (IS_ENABLED(CONFIG_USB_PD_REV30) &&
+	   is_rev3_vdo(port, TCPC_TX_SOP_PRIME))
+		return  disc->identity.product_t1.p_rev30.ss ==
+						USB_R30_SS_U32_U40_GEN1 ||
+			disc->identity.product_t1.p_rev30.ss ==
+						USB_R30_SS_U32_U40_GEN2 ||
+			disc->identity.product_t1.p_rev30.ss ==
+						USB_R30_SS_U40_GEN3;
 
-	return cable->attr.p_rev20.ss == USB_R20_SS_U31_GEN1 ||
-		cable->attr.p_rev20.ss == USB_R20_SS_U31_GEN1_GEN2;
+	return  disc->identity.product_t1.p_rev20.ss ==
+						USB_R20_SS_U31_GEN1 ||
+		disc->identity.product_t1.p_rev20.ss ==
+						USB_R20_SS_U31_GEN1_GEN2;
 }
 
 bool is_modal(int port, int cnt, const uint32_t *payload)
@@ -797,30 +785,6 @@ bool is_modal(int port, int cnt, const uint32_t *payload)
 		PD_IDH_IS_MODAL(payload[VDO_INDEX_IDH]);
 }
 
-bool is_intel_svid(int port, int prev_svid_cnt)
-{
-	int i;
-	/* TODO(b/148528713): Use TCPMv2's separate storage for SOP'. */
-	struct pd_discovery *disc = pd_get_am_discovery(port, TCPC_TX_SOP);
-
-	/*
-	 * Ref: USB Type-C cable and connector specification, Table F-9
-	 * Check if SVID0 = USB_VID_INTEL. However,
-	 * errata: All the Thunderbolt certified cables and docks tested have
-	 * SVID1 = 0x8087.
-	 * Hence, check all the SVIDs for Intel SVID, if the response presents
-	 * SVIDs in any order.
-	 */
-	if (IS_ENABLED(CONFIG_USB_PD_TBT_COMPAT_MODE)) {
-		for (i = prev_svid_cnt;
-				i < pd_get_svid_count(port, TCPC_TX_SOP); i++) {
-			if (disc->svids[i].svid == USB_VID_INTEL)
-				return true;
-		}
-	}
-	return false;
-}
-
 bool is_tbt_compat_mode(int port, int cnt, const uint32_t *payload)
 {
 	/*
@@ -951,7 +915,8 @@ bool is_usb4_vdo(int port, int cnt, uint32_t *payload)
  */
 static enum usb_rev30_ss board_get_max_usb_cable_speed(int port)
 {
-	struct pd_cable *cable = pd_get_cable_attributes(port);
+	struct pd_discovery *disc =
+		pd_get_am_discovery(port, TCPC_TX_SOP_PRIME);
 	/*
 	 * Converting Thunderbolt-Compatible board speed to equivalent USB4
 	 * speed.
@@ -960,16 +925,15 @@ static enum usb_rev30_ss board_get_max_usb_cable_speed(int port)
 		board_get_max_tbt_speed(port) == TBT_SS_TBT_GEN3 ?
 		USB_R30_SS_U40_GEN3 : USB_R30_SS_U32_U40_GEN2;
 
-	return max_usb4_speed < cable->attr.p_rev30.ss ?
-	       max_usb4_speed : cable->attr.p_rev30.ss;
+	return max_usb4_speed <  disc->identity.product_t1.p_rev30.ss ?
+	       max_usb4_speed :  disc->identity.product_t1.p_rev30.ss;
 }
 
 enum usb_rev30_ss get_usb4_cable_speed(int port)
 {
-	struct pd_cable *cable = pd_get_cable_attributes(port);
 	enum usb_rev30_ss max_rev30_usb4_speed;
 
-	if (cable->rev == PD_REV30) {
+	if (is_rev3_vdo(port, TCPC_TX_SOP_PRIME)) {
 		max_rev30_usb4_speed = board_get_max_usb_cable_speed(port);
 		if (!IS_ENABLED(CONFIG_USB_PD_TBT_GEN3_CAPABLE) ||
 		     max_rev30_usb4_speed != USB_R30_SS_U32_U40_GEN2 ||
@@ -991,20 +955,21 @@ uint32_t get_enter_usb_msg_payload(int port)
 	 * Table 6-47 Enter_USB Data Object
 	 */
 	union enter_usb_data_obj eudo;
-	struct pd_cable *cable;
+	struct pd_discovery *disc;
 
 	if (!IS_ENABLED(CONFIG_USB_PD_USB4))
 		return 0;
 
-	cable = pd_get_cable_attributes(port);
+	disc = pd_get_am_discovery(port, TCPC_TX_SOP_PRIME);
 	eudo.mode = USB_PD_40;
 	eudo.usb4_drd_cap = IS_ENABLED(CONFIG_USB_PD_USB4_DRD);
 	eudo.usb3_drd_cap = IS_ENABLED(CONFIG_USB_PD_USB32_DRD);
 	eudo.cable_speed = get_usb4_cable_speed(port);
 
-	if ((cable->rev == PD_REV30) &&
-	    (get_usb_pd_cable_type(port) == IDH_PTYPE_ACABLE)) {
-		eudo.cable_type = (cable->attr2.a2_rev30.active_elem ==
+	if (is_rev3_vdo(port, TCPC_TX_SOP_PRIME) &&
+	    (disc->identity.idh.product_type == IDH_PTYPE_ACABLE)) {
+		eudo.cable_type =
+			(disc->identity.product_t2.a2_rev30.active_elem ==
 			ACTIVE_RETIMER) ? CABLE_TYPE_ACTIVE_RETIMER :
 			CABLE_TYPE_ACTIVE_REDRIVER;
 	/* TODO: Add eudo.cable_type for Revisiosn 2 active cables */
@@ -1012,7 +977,7 @@ uint32_t get_enter_usb_msg_payload(int port)
 		eudo.cable_type = CABLE_TYPE_PASSIVE;
 	}
 
-	switch (cable[port].attr.p_rev20.vbus_cur) {
+	switch (disc->identity.product_t1.p_rev20.vbus_cur) {
 	case USB_VBUS_CUR_3A:
 		eudo.cable_current = USB4_CABLE_CURRENT_3A;
 		break;
diff --git a/common/usb_pd_console_cmd.c b/common/usb_pd_console_cmd.c
index 3fc6a6f35f..c155a59077 100644
--- a/common/usb_pd_console_cmd.c
+++ b/common/usb_pd_console_cmd.c
@@ -111,7 +111,9 @@ static int command_cable(int argc, char **argv)
 {
 	int port;
 	char *e;
-	struct pd_cable *cable;
+	struct pd_discovery *disc;
+	enum idh_ptype ptype;
+	int cable_rev;
 
 	if (argc < 2)
 		return EC_ERROR_PARAM_COUNT;
@@ -120,34 +122,34 @@ static int command_cable(int argc, char **argv)
 	if (*e || port >= board_get_usb_pd_port_count())
 		return EC_ERROR_PARAM2;
 
-	cable = pd_get_cable_attributes(port);
-
-	if (!cable->is_identified) {
-		ccprintf("Cable not identified.\n");
-		return EC_SUCCESS;
-	}
+	ptype = get_usb_pd_cable_type(port);
 
 	ccprintf("Cable Type: ");
-	if (cable->type != IDH_PTYPE_PCABLE &&
-		cable->type != IDH_PTYPE_ACABLE) {
+	if (ptype != IDH_PTYPE_PCABLE &&
+		ptype != IDH_PTYPE_ACABLE) {
 		ccprintf("Not Emark Cable\n");
 		return EC_SUCCESS;
 	}
-	ccprintf("%s\n", cable_type[cable->type]);
+	ccprintf("%s\n", cable_type[ptype]);
+
+	cable_rev = pd_get_vdo_ver(port, TCPC_TX_SOP_PRIME);
+	disc = pd_get_am_discovery(port, TCPC_TX_SOP_PRIME);
 
 	/* Cable revision */
-	ccprintf("Cable Rev: %d.0\n", cable->rev + 1);
+	ccprintf("Cable Rev: %d.0\n", cable_rev + 1);
 
 	/*
 	 * For rev 2.0, rev 3.0 active and passive cables have same bits for
 	 * connector type (Bit 19:18) and current handling capability bit 6:5
 	 */
-	ccprintf("Connector Type: %d\n", cable->attr.p_rev20.connector);
+	ccprintf("Connector Type: %d\n",
+				disc->identity.product_t1.p_rev20.connector);
 
-	if (cable->attr.p_rev20.vbus_cur) {
+	if (disc->identity.product_t1.p_rev20.vbus_cur) {
 		ccprintf("Cable Current: %s\n",
-		   cable->attr.p_rev20.vbus_cur > ARRAY_SIZE(cable_curr) ?
-		   "Invalid" : cable_curr[cable->attr.p_rev20.vbus_cur]);
+		   disc->identity.product_t1.p_rev20.vbus_cur >
+		      ARRAY_SIZE(cable_curr) ? "Invalid" :
+		      cable_curr[disc->identity.product_t1.p_rev20.vbus_cur]);
 	} else
 		ccprintf("Cable Current: Invalid\n");
 
@@ -155,32 +157,33 @@ static int command_cable(int argc, char **argv)
 	 * For Rev 3.0 passive cables and Rev 2.0 active and passive cables,
 	 * USB Superspeed Signaling support have same bits 2:0
 	 */
-	if (cable->type == IDH_PTYPE_PCABLE)
+	if (ptype == IDH_PTYPE_PCABLE)
 		ccprintf("USB Superspeed Signaling support: %d\n",
-			cable[port].attr.p_rev20.ss);
+			disc->identity.product_t1.p_rev20.ss);
 
 	/*
 	 * For Rev 3.0 active cables and Rev 2.0 active and passive cables,
 	 * SOP" controller preset have same bit 3
 	 */
-	if (cable->type == IDH_PTYPE_ACABLE)
+	if (ptype == IDH_PTYPE_ACABLE)
 		ccprintf("SOP'' Controller: %s present\n",
-			cable->attr.a_rev20.sop_p_p ? "" : "Not");
+		      disc->identity.product_t1.a_rev20.sop_p_p ? "" : "Not");
 
-	if (cable->rev == PD_REV30) {
+	if (cable_rev == PD_REV30) {
 		/*
 		 * For Rev 3.0 active and passive cables, Max Vbus vtg have
 		 * same bits 10:9.
 		 */
 		ccprintf("Max vbus voltage: %d\n",
-			20 + 10 * cable->attr.p_rev30.vbus_max);
+			20 + 10 * disc->identity.product_t1.p_rev30.vbus_max);
 
 		/* For Rev 3.0 Active cables */
-		if (cable->type == IDH_PTYPE_ACABLE) {
+		if (ptype == IDH_PTYPE_ACABLE) {
 			ccprintf("SS signaling: USB_SS_GEN%u\n",
-				cable->attr2.a2_rev30.usb_gen ? 2 : 1);
+				disc->identity.product_t2.a2_rev30.usb_gen ?
+									2 : 1);
 			ccprintf("Number of SS lanes supported: %u\n",
-				cable->attr2.a2_rev30.usb_lanes);
+				disc->identity.product_t2.a2_rev30.usb_lanes);
 		}
 	}
 	return EC_SUCCESS;
diff --git a/common/usb_pd_policy.c b/common/usb_pd_policy.c
index b10b9cb8c7..2ab397c0e7 100644
--- a/common/usb_pd_policy.c
+++ b/common/usb_pd_policy.c
@@ -113,6 +113,11 @@ uint8_t pd_get_src_cap_cnt(int port)
 
 static struct pd_cable cable[CONFIG_USB_PD_PORT_MAX_COUNT];
 
+enum pd_rev_type get_usb_pd_cable_revision(int port)
+{
+	return cable[port].rev;
+}
+
 bool consume_sop_prime_repeat_msg(int port, uint8_t msg_id)
 {
 
@@ -205,7 +210,8 @@ void disable_enter_usb4_mode(int port)
 
 #ifdef CONFIG_USB_PD_ALT_MODE_DFP
 
-static struct pd_discovery discovery[CONFIG_USB_PD_PORT_MAX_COUNT];
+static struct pd_discovery
+	discovery[CONFIG_USB_PD_PORT_MAX_COUNT][DISCOVERY_TYPE_COUNT];
 static struct partner_active_modes partner_amodes[CONFIG_USB_PD_PORT_MAX_COUNT];
 
 static bool is_tbt_compat_enabled(int port)
@@ -237,6 +243,18 @@ static inline bool is_limit_tbt_cable_speed(int port)
 	return !!(cable[port].flags & CABLE_FLAGS_TBT_COMPAT_LIMIT_SPEED);
 }
 
+static bool is_intel_svid(int port, enum tcpm_transmit_type type)
+{
+	int i;
+
+	for (i = 0; i < discovery[port][type].svid_cnt; i++) {
+		if (pd_get_svid(port, i, type) == USB_VID_INTEL)
+			return true;
+	}
+
+	return false;
+}
+
 static inline bool is_usb4_mode_enabled(int port)
 {
 	return (IS_ENABLED(CONFIG_USB_PD_USB4) &&
@@ -292,13 +310,13 @@ static inline void disable_usb4_mode(int port)
 static bool is_cable_ready_to_enter_usb4(int port, int cnt)
 {
 	/* TODO: USB4 enter mode for Active cables */
-
+	struct pd_discovery *disc = &discovery[port][TCPC_TX_SOP_PRIME];
 	if (IS_ENABLED(CONFIG_USB_PD_USB4) &&
 	   (get_usb_pd_cable_type(port) == IDH_PTYPE_PCABLE) &&
 	    is_vdo_present(cnt, VDO_INDEX_PTYPE_CABLE1)) {
 		switch (cable[port].rev) {
 		case PD_REV30:
-			switch (cable[port].attr.p_rev30.ss) {
+			switch (disc->identity.product_t1.p_rev30.ss) {
 			case USB_R30_SS_U40_GEN3:
 			case USB_R30_SS_U32_U40_GEN1:
 				return true;
@@ -312,7 +330,7 @@ static bool is_cable_ready_to_enter_usb4(int port, int cnt)
 				return false;
 			}
 		case PD_REV20:
-			switch (cable[port].attr.p_rev20.ss) {
+			switch (disc->identity.product_t1.p_rev20.ss) {
 			case USB_R20_SS_U31_GEN1_GEN2:
 				/* Check if DFP is Gen 3 capable */
 				if (IS_ENABLED(CONFIG_USB_PD_TBT_GEN3_CAPABLE))
@@ -349,11 +367,7 @@ static int dfp_discover_svids(uint32_t *payload)
 
 struct pd_discovery *pd_get_am_discovery(int port, enum tcpm_transmit_type type)
 {
-	/*
-	 * TCPMv2 separates discovered data by partner (SOP vs. SOP'); TCPMv1
-	 * depends on both types being in the same structure.
-	 */
-	return &discovery[port];
+	return &discovery[port][type];
 }
 
 struct partner_active_modes *pd_get_partner_active_modes(int port,
@@ -377,10 +391,11 @@ static int process_am_discover_ident_sop(int port, int cnt,
 					uint32_t head, uint32_t *payload,
 					enum tcpm_transmit_type *rtype)
 {
+	pd_dfp_discovery_init(port);
+	dfp_consume_identity(port, TCPC_TX_SOP, cnt, payload);
+
 	if (IS_ENABLED(CONFIG_USB_PD_DECODE_SOP) && is_sop_prime_ready(port) &&
 	    board_is_tbt_usb4_port(port)) {
-		pd_dfp_discovery_init(port);
-		dfp_consume_identity(port, cnt, payload);
 
 		/* Enable USB4 mode if USB4 VDO present and port partner
 		 * supports USB Rev 3.0.
@@ -401,9 +416,6 @@ static int process_am_discover_ident_sop(int port, int cnt,
 			*rtype = TCPC_TX_SOP_PRIME;
 			return dfp_discover_ident(payload);
 		}
-	} else {
-		pd_dfp_discovery_init(port);
-		dfp_consume_identity(port, cnt, payload);
 	}
 
 	return dfp_discover_svids(payload);
@@ -412,8 +424,8 @@ static int process_am_discover_ident_sop(int port, int cnt,
 static int process_am_discover_ident_sop_prime(int port, int cnt,
 					uint32_t head, uint32_t *payload)
 {
-	/* Store cable type */
-	dfp_consume_cable_response(port, cnt, payload, head);
+	dfp_consume_identity(port, TCPC_TX_SOP_PRIME, cnt, payload);
+	cable[port].rev = PD_HEADER_REV(head);
 
 	/*
 	 * Enter USB4 mode if the cable supports USB4 operation and has USB4
@@ -446,14 +458,12 @@ static int process_am_discover_svids(int port, int cnt, uint32_t *payload,
 				enum tcpm_transmit_type sop,
 				enum tcpm_transmit_type *rtype)
 {
-	int prev_svid_cnt = discovery[port].svid_cnt;
-
 	/*
 	 * The pd_discovery structure stores SOP and SOP' discovery results
 	 * separately, but TCPMv1 depends on one-dimensional storage of SVIDs
 	 * and modes. Therefore, always use TCPC_TX_SOP in TCPMv1.
 	 */
-	dfp_consume_svids(port, TCPC_TX_SOP, cnt, payload);
+	dfp_consume_svids(port, sop, cnt, payload);
 
 	/*
 	 * Ref: USB Type-C Cable and Connector Specification,
@@ -470,8 +480,7 @@ static int process_am_discover_svids(int port, int cnt, uint32_t *payload,
 	 * passive Gen 2 cable.
 	 */
 	if (is_tbt_compat_enabled(port)) {
-		bool intel_svid = is_intel_svid(port, prev_svid_cnt);
-
+		bool intel_svid = is_intel_svid(port, sop);
 		if (!intel_svid) {
 			if (is_usb4_mode_enabled(port)) {
 				disable_tbt_compat_mode(port);
@@ -500,6 +509,7 @@ static int process_tbt_compat_discover_modes(int port,
 				enum tcpm_transmit_type *rtype)
 {
 	int rsize;
+	struct pd_discovery *disc;
 
 	/* Initialize transmit type to SOP */
 	*rtype = TCPC_TX_SOP;
@@ -540,6 +550,7 @@ static int process_tbt_compat_discover_modes(int port,
 	} else {
 		/* Store Discover Mode SOP response */
 		cable[port].dev_mode_resp.raw_value = payload[1];
+		disc = &discovery[port][TCPC_TX_SOP_PRIME];
 
 		if (is_limit_tbt_cable_speed(port)) {
 			/*
@@ -549,16 +560,16 @@ static int process_tbt_compat_discover_modes(int port,
 			 * Thunderbolt-compatible mode.
 			 */
 			cable[port].cable_mode_resp.tbt_cable_speed =
-				(cable[port].rev == PD_REV30 &&
-				cable[port].attr.p_rev30.ss >
+			     cable[port].rev == PD_REV30 &&
+			     (disc->identity.product_t1.p_rev30.ss >
 					USB_R30_SS_U32_U40_GEN2) ?
 				TBT_SS_U32_GEN1_GEN2 :
-				cable[port].attr.p_rev30.ss;
+				disc->identity.product_t1.p_rev30.ss;
 
 			rsize = enter_tbt_compat_mode(port, *rtype, payload);
 		} else {
 			/* Discover modes for SOP' */
-			discovery[port].svid_idx--;
+			discovery[port][TCPC_TX_SOP].svid_idx--;
 			rsize = dfp_discover_modes(port, payload);
 			*rtype = TCPC_TX_SOP_PRIME;
 		}
@@ -571,10 +582,12 @@ static int obj_cnt_enter_tbt_compat_mode(int port,
 		enum tcpm_transmit_type sop, uint32_t *payload,
 		enum tcpm_transmit_type *rtype)
 {
+	struct pd_discovery *disc = &discovery[port][TCPC_TX_SOP_PRIME];
+
 	/* Enter mode SOP' for active cables */
 	if (sop == TCPC_TX_SOP_PRIME) {
 		/* Check if the cable has a SOP'' controller */
-		if (cable[port].attr.a_rev20.sop_p_p)
+		if (disc->identity.product_t1.a_rev20.sop_p_p)
 			*rtype = TCPC_TX_SOP_PRIME_PRIME;
 		return enter_tbt_compat_mode(port, *rtype, payload);
 	}
@@ -692,13 +705,7 @@ int pd_svdm(int port, int cnt, uint32_t *payload, uint32_t **rpayload,
 							  sop, rtype);
 			break;
 		case CMD_DISCOVER_MODES:
-			/*
-			 * The pd_discovery structure stores SOP and SOP'
-			 * discovery results separately, but TCPMv1 depends on
-			 * one-dimensional storage of SVIDs and modes.
-			 * Therefore, always use TCPC_TX_SOP in TCPMv1.
-			 */
-			dfp_consume_modes(port, TCPC_TX_SOP, cnt, payload);
+			dfp_consume_modes(port, sop, cnt, payload);
 			if (is_tbt_compat_enabled(port) &&
 				is_tbt_compat_mode(port, cnt, payload)) {
 				rsize = process_tbt_compat_discover_modes(
diff --git a/common/usb_pd_protocol.c b/common/usb_pd_protocol.c
index 31093b9a2d..9d95b5b480 100644
--- a/common/usb_pd_protocol.c
+++ b/common/usb_pd_protocol.c
@@ -343,6 +343,9 @@ int pd_get_rev(int port)
 
 int pd_get_vdo_ver(int port, enum tcpm_transmit_type type)
 {
+	if (type == TCPC_TX_SOP_PRIME)
+		return get_usb_pd_cable_revision(port);
+
 	return vdo_ver[pd[port].rev];
 }
 #else
diff --git a/common/usbc/usb_pe_drp_sm.c b/common/usbc/usb_pe_drp_sm.c
index 1088d1ede8..053606f291 100644
--- a/common/usbc/usb_pe_drp_sm.c
+++ b/common/usbc/usb_pe_drp_sm.c
@@ -4352,8 +4352,7 @@ static void pe_vdm_identity_request_cbl_run(int port)
 		 * PE_INIT_PORT_VDM_Identity_ACKed and PE_SRC_VDM_Identity_ACKed
 		 * embedded here.
 		 */
-		dfp_consume_cable_response(port, cnt, payload,
-				rx_emsg[port].header);
+		dfp_consume_identity(port, sop, cnt, payload);
 
 		/*
 		 * Note: If port partner runs PD 2.0, we must use PD 2.0 to
@@ -4458,7 +4457,7 @@ static void pe_init_port_vdm_identity_request_run(int port)
 
 	if (response_result == PD_DISC_COMPLETE) {
 		/* PE_INIT_PORT_VDM_Identity_ACKed embedded here. */
-		dfp_consume_identity(port, cnt, payload);
+		dfp_consume_identity(port, sop, cnt, payload);
 	} else if (response_result == PD_DISC_FAIL) {
 		/* PE_INIT_PORT_VDM_IDENTITY_NAKed embedded here */
 		pd_set_identity_discovery(port, sop, PD_DISC_FAIL);
diff --git a/include/usb_pd.h b/include/usb_pd.h
index 09468911bf..9384a13295 100644
--- a/include/usb_pd.h
+++ b/include/usb_pd.h
@@ -573,14 +573,6 @@ struct pd_cable {
 	/* For storing Discover mode response from cable */
 	union tbt_mode_resp_cable cable_mode_resp;
 
-	/* Shared fields between TCPMv1 and TCPMv2 */
-	uint8_t is_identified;
-	/* Type of cable */
-	enum idh_ptype type;
-	/* Cable attributes */
-	union product_type_vdo1 attr;
-	/* For USB PD REV3, active cable has 2 VDOs */
-	union product_type_vdo2 attr2;
 	/* Cable revision */
 	enum pd_rev_type rev;
 
@@ -1651,10 +1643,12 @@ void dfp_consume_attention(int port, uint32_t *payload);
  * Consume the discover identity message
  *
  * @param port    USB-C port number
+ * @param type    Transmit type (SOP, SOP') for received modes
  * @param cnt     number of data objects in payload
  * @param payload payload data.
  */
-void dfp_consume_identity(int port, int cnt, uint32_t *payload);
+void dfp_consume_identity(int port, enum tcpm_transmit_type type, int cnt,
+		uint32_t *payload);
 
 /**
  * Consume the SVIDs
@@ -1694,7 +1688,6 @@ int dfp_discover_modes(int port, uint32_t *payload);
  */
 void pd_dfp_discovery_init(int port);
 
-
 /**
  * Set identity discovery state for this type and port
  *
@@ -1860,6 +1853,14 @@ bool pd_is_mode_discovered_for_svid(int port, enum tcpm_transmit_type type,
 struct svdm_amode_data *pd_get_amode_data(int port,
 		enum tcpm_transmit_type type, uint16_t svid);
 
+/*
+ * Returns cable revision
+ *
+ * @param port          USB-C port number
+ * @return              cable revision
+ */
+enum pd_rev_type get_usb_pd_cable_revision(int port);
+
 /**
  * Returns false if previous SOP' messageId count is different from received
  * messageId count.
@@ -1978,17 +1979,6 @@ bool is_vdo_present(int cnt, int index);
 enum idh_ptype get_usb_pd_cable_type(int port);
 
 /**
- * Stores the cable's response to discover Identity SOP' request
- *
- * @param port      USB-C port number
- * @param cnt       number of data objects in payload
- * @param payload   payload data
- * @param head      PD packet header
- */
-void dfp_consume_cable_response(int port, int cnt, uint32_t *payload,
-					uint32_t head);
-
-/**
  * Returns USB4 cable speed according to the port, if port supports lesser
  * USB4 cable speed than the cable.
  *
@@ -2103,15 +2093,6 @@ bool is_tbt_cable_superspeed(int port);
 bool is_modal(int port, int cnt, const uint32_t *payload);
 
 /**
- * Checks all the SVID for USB_VID_INTEL
- *
- * @param port	        USB-C port number
- * @param prev_svid_cnt Previous SVID count
- * @return              True is SVID = USB_VID_INTEL, false otherwise
- */
-bool is_intel_svid(int port, int prev_svid_cnt);
-
-/**
  * Checks if Device discover mode response contains Thunderbolt alternate mode
  *
  * @param port	   USB-C port number