cleanup: clean up reference to power role vs cable plug

The PD header specifies the power role for SOP packets and cable plug for
SOP' and SOP" packets. Refactor code to make this more obvious.

BRANCH=none
BUG=none
TEST=builds and new stack runs on hatch

Change-Id: I6cdb1561082d2142214ac65703ff42586b16d70b
Signed-off-by: Jett Rink <jettrink@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/1865986
Reviewed-by: Denis Brockus <dbrockus@chromium.org>

10 files changed, 91 insertions, 46 deletions

diff --git a/common/usb_pd_protocol.c b/common/usb_pd_protocol.c
index 266c83788b..1ce06d9260 100644
--- a/common/usb_pd_protocol.c
+++ b/common/usb_pd_protocol.c
@@ -2212,10 +2212,14 @@ static void pd_vdm_send_state_machine(int port)
 		if (is_sop_prime_ready(port, pd[port].data_role,
 				pd[port].flags)) {
 			/* Prepare SOP'/SOP'' header and send VDM */
-			header = PD_HEADER(PD_DATA_VENDOR_DEF, PD_PLUG_DFP_UFP,
-					   0, pd[port].msg_id,
-					   (int)pd[port].vdo_count,
-					   pd_get_rev(port), 0);
+			header = PD_HEADER(
+				PD_DATA_VENDOR_DEF,
+				PD_PLUG_FROM_DFP_UFP,
+				0,
+				pd[port].msg_id,
+				(int)pd[port].vdo_count,
+				pd_get_rev(port),
+				0);
 			res = pd_transmit(port, TCPC_TX_SOP_PRIME, header,
 					  pd[port].vdo_data);
 			/*
diff --git a/common/usbc/usb_prl_sm.c b/common/usbc/usb_prl_sm.c
index 1c4761e5f4..e95f2a3c9e 100644
--- a/common/usbc/usb_prl_sm.c
+++ b/common/usbc/usb_prl_sm.c
@@ -612,13 +612,18 @@ static void prl_tx_layer_reset_for_transmit_run(const int port)
 
 static uint32_t get_sop_star_header(const int port)
 {
+	const int is_sop_packet = pdmsg[port].xmit_type == TCPC_TX_SOP;
+
+	/* SOP vs SOP'/SOP" headers are different. Replace fields as needed */
 	return PD_HEADER(
 		pdmsg[port].msg_type,
-		tc_get_power_role(port),
-		tc_get_data_role(port),
+		is_sop_packet ?
+			tc_get_power_role(port) : tc_get_cable_plug(port),
+		is_sop_packet ?
+			tc_get_data_role(port) : 0,
 		prl_tx[port].msg_id_counter[pdmsg[port].xmit_type],
 		pdmsg[port].data_objs,
-		(pdmsg[port].xmit_type == TCPC_TX_SOP) ?
+		is_sop_packet ?
 			pdmsg[port].rev : pdmsg[port].cable_rev,
 		pdmsg[port].ext);
 }
@@ -1518,7 +1523,7 @@ static void prl_rx_wait_for_phy_message(const int port, int evt)
 	if (!IS_ENABLED(CONFIG_USB_TYPEC_CTVPD) &&
 	    !IS_ENABLED(CONFIG_USB_TYPEC_VPD) &&
 	    PD_HEADER_GET_SOP(header) != PD_MSG_SOP &&
-	    PD_HEADER_PROLE(header) == PD_PLUG_DFP_UFP)
+	    PD_HEADER_PROLE(header) == PD_PLUG_FROM_DFP_UFP)
 		return;
 
 	if (cnt == 0 && type == PD_CTRL_SOFT_RESET) {
diff --git a/common/usbc/usb_tc_ctvpd_sm.c b/common/usbc/usb_tc_ctvpd_sm.c
index de4bcedf66..c935be380f 100644
--- a/common/usbc/usb_tc_ctvpd_sm.c
+++ b/common/usbc/usb_tc_ctvpd_sm.c
@@ -39,10 +39,6 @@
 static struct type_c {
 	/* state machine context */
 	struct sm_ctx ctx;
-	/* current port power role (VPD, SOURCE or SINK) */
-	enum pd_power_role power_role;
-	/* current port data role (DFP or UFP) */
-	enum pd_data_role data_role;
 	/* Higher-level power deliver state machines are enabled if true. */
 	uint8_t pd_enable;
 	/* port flags, see TC_FLAGS_* */
@@ -143,14 +139,24 @@ static void set_state_tc(const int port, enum usb_tc_state new_state);
 
 enum pd_power_role tc_get_power_role(int port)
 {
-	return tc[port].power_role;
+	/* Vconn power device is always the sink */
+	return PD_ROLE_SINK;
+}
+
+enum pd_cable_plug tc_get_cable_plug(int port)
+{
+	/* Vconn power device is always the cable */
+	return PD_PLUG_FROM_CABLE;
 }
 
 enum pd_data_role tc_get_data_role(int port)
 {
-	return tc[port].data_role;
+	/* Vconn power device doesn't have a data role, but UFP matches SNK */
+	return PD_ROLE_UFP;
 }
 
+/* Note tc_set_power_role and tc_set_data_role are unimplemented */
+
 uint8_t tc_get_polarity(int port)
 {
 	/* Does not track polarity */
@@ -162,11 +168,6 @@ uint8_t tc_get_pd_enabled(int port)
 	return tc[port].pd_enable;
 }
 
-void tc_set_power_role(int port, enum pd_power_role role)
-{
-	tc[port].power_role = role;
-}
-
 void tc_reset_support_timer(int port)
 {
 	tc[port].support_timer_reset |= SUPPORT_TIMER_RESET_REQUEST;
@@ -185,8 +186,6 @@ void tc_state_init(int port)
 
 	/* Disable pd state machines */
 	tc[port].pd_enable = 0;
-	tc[port].power_role = PD_PLUG_CABLE_VPD;
-	tc[port].data_role = 0; /* Reserved for VPD */
 	tc[port].billboard_presented = 0;
 	tc[port].flags = 0;
 }
diff --git a/common/usbc/usb_tc_drp_acc_trysrc_sm.c b/common/usbc/usb_tc_drp_acc_trysrc_sm.c
index 569579535d..286fa3a660 100644
--- a/common/usbc/usb_tc_drp_acc_trysrc_sm.c
+++ b/common/usbc/usb_tc_drp_acc_trysrc_sm.c
@@ -678,6 +678,15 @@ enum pd_data_role tc_get_data_role(int port)
 	return tc[port].data_role;
 }
 
+enum pd_cable_plug tc_get_cable_plug(int port)
+{
+	/*
+	 * Messages sent by this state machine are always from a DFP/UFP,
+	 * i.e. the chromebook.
+	 */
+	return PD_PLUG_FROM_DFP_UFP;
+}
+
 uint8_t tc_get_polarity(int port)
 {
 	return tc[port].polarity;
diff --git a/common/usbc/usb_tc_vpd_sm.c b/common/usbc/usb_tc_vpd_sm.c
index cb30093be3..dedba97513 100644
--- a/common/usbc/usb_tc_vpd_sm.c
+++ b/common/usbc/usb_tc_vpd_sm.c
@@ -33,10 +33,6 @@
 static struct type_c {
 	/* state machine context */
 	struct sm_ctx ctx;
-	/* current port power role (VPD, SOURCE or SINK) */
-	enum pd_power_role power_role;
-	/* current port data role (DFP or UFP) */
-	enum pd_data_role data_role;
 	/* Higher-level power deliver state machines are enabled if true. */
 	uint8_t pd_enable;
 	/* port flags, see TC_FLAGS_* */
@@ -93,22 +89,29 @@ void tc_state_init(int port)
 
 	/* Disable pd state machines */
 	tc[port].pd_enable = 0;
-	tc[port].power_role = PD_PLUG_CABLE_VPD;
-	tc[port].data_role = 0; /* Reserved for VPD */
 	tc[port].flags = 0;
 }
 
-
 enum pd_power_role tc_get_power_role(int port)
 {
-	return tc[port].power_role;
+	/* Vconn power device is always the sink */
+	return PD_ROLE_SINK;
+}
+
+enum pd_cable_plug tc_get_cable_plug(int port)
+{
+	/* Vconn power device is always the cable */
+	return PD_PLUG_FROM_CABLE;
 }
 
 enum pd_data_role tc_get_data_role(int port)
 {
-	return tc[port].data_role;
+	/* Vconn power device doesn't have a data role, but UFP match SNK */
+	return PD_ROLE_UFP;
 }
 
+/* Note tc_set_power_role and tc_set_data_role are unimplemented */
+
 uint8_t tc_get_polarity(int port)
 {
 	/* Does not track polarity yet */
@@ -120,11 +123,6 @@ uint8_t tc_get_pd_enabled(int port)
 	return tc[port].pd_enable;
 }
 
-void tc_set_power_role(int port, enum pd_power_role role)
-{
-	tc[port].power_role = role;
-}
-
 void tc_event_check(int port, int evt)
 {
 	/* Do Nothing */
diff --git a/include/usb_pd.h b/include/usb_pd.h
index 67bdb0e6cf..df2d790cab 100644
--- a/include/usb_pd.h
+++ b/include/usb_pd.h
@@ -1237,21 +1237,32 @@ enum pd_rev_type {
 	PD_REV30
 };
 
-/* Power role */
+/*
+ * Power role. See 6.2.1.1.4 Port Power Role. Only applies to SOP packets.
+ * Replaced by pd_cable_plug for SOP' and SOP" packets.
+ */
 enum pd_power_role {
-	PD_ROLE_SINK,
-	PD_ROLE_SOURCE
+	PD_ROLE_SINK = 0,
+	PD_ROLE_SOURCE = 1
 };
 
-/* Data role */
+/*
+ * Data role. See 6.2.1.1.6 Port Data Role. Only applies to SOP.
+ * Replaced by reserved field for SOP' and SOP" packets.
+ */
 enum pd_data_role {
-	PD_ROLE_UFP,
-	PD_ROLE_DFP,
+	PD_ROLE_UFP = 0,
+	PD_ROLE_DFP = 1
 };
 
-/* Cable plug */
-#define PD_PLUG_DFP_UFP   0
-#define PD_PLUG_CABLE_VPD 1
+/*
+ * Cable plug. See 6.2.1.1.7 Cable Plug. Only applies to SOP' and SOP".
+ * Replaced by pd_power_role for SOP packets.
+ */
+enum pd_cable_plug {
+	PD_PLUG_FROM_DFP_UFP = 0,
+	PD_PLUG_FROM_CABLE = 1
+};
 
 /* Vconn role */
 #define PD_ROLE_VCONN_OFF 0
diff --git a/include/usb_tc_sm.h b/include/usb_tc_sm.h
index 2b9912bc7b..f6971b07e3 100644
--- a/include/usb_tc_sm.h
+++ b/include/usb_tc_sm.h
@@ -64,6 +64,15 @@ enum pd_data_role tc_get_data_role(int port);
 enum pd_power_role tc_get_power_role(int port);
 
 /**
+ * Get cable plug setting. This should be constant per build. This replaces
+ * the power role bit in PD header for SOP' and SOP" packets.
+ *
+ * @param port USB-C port number
+ * @return PD cable plug setting
+ */
+enum pd_cable_plug tc_get_cable_plug(int port);
+
+/**
  * Get current polarity
  *
  * @param port USB-C port number
diff --git a/test/fake_usbc.c b/test/fake_usbc.c
index 9f2d4c12ae..b0137cb893 100644
--- a/test/fake_usbc.c
+++ b/test/fake_usbc.c
@@ -46,6 +46,11 @@ void tc_set_power_role(int port, enum pd_power_role role)
 	power_role = role;
 }
 
+enum pd_cable_plug tc_get_cable_plug(int port)
+{
+	return PD_PLUG_FROM_DFP_UFP;
+}
+
 int tc_check_vconn_swap(int port)
 {
 	return 0;
diff --git a/test/usb_prl.c b/test/usb_prl.c
index 58bd452e60..e5692a8452 100644
--- a/test/usb_prl.c
+++ b/test/usb_prl.c
@@ -733,6 +733,11 @@ enum pd_data_role tc_get_data_role(int port)
 	return pd_port[port].data_role;
 }
 
+enum pd_cable_plug tc_get_cable_plug(int port)
+{
+	return PD_PLUG_FROM_DFP_UFP;
+}
+
 void pe_report_error(int port, enum pe_error e)
 {
 	pd_port[port].mock_pe_error = e;
diff --git a/test/usb_typec_ctvpd.c b/test/usb_typec_ctvpd.c
index b16ac37b25..75d620fd85 100644
--- a/test/usb_typec_ctvpd.c
+++ b/test/usb_typec_ctvpd.c
@@ -615,7 +615,7 @@ static int test_vpd_host_src_detection_message_reception(void)
 	/* Test Discover Identity Ack */
 	TEST_ASSERT(pd_test_tx_msg_verify_sop_prime(port));
 	TEST_ASSERT(pd_test_tx_msg_verify_short(port,
-			PD_HEADER(PD_DATA_VENDOR_DEF, PD_PLUG_CABLE_VPD, 0,
+			PD_HEADER(PD_DATA_VENDOR_DEF, PD_PLUG_FROM_CABLE, 0,
 			pd_port[port].msg_tx_id, 5, pd_port[port].rev, 0)));
 	TEST_ASSERT(pd_test_tx_msg_verify_word(port, expected_vdm_header));
 	TEST_ASSERT(pd_test_tx_msg_verify_word(port, expected_vdo_id_header));
@@ -1113,7 +1113,7 @@ static int test_ctvpd_behavior_case4(void)
 	/* Test Discover Identity Ack */
 	TEST_ASSERT(pd_test_tx_msg_verify_sop_prime(port));
 	TEST_ASSERT(pd_test_tx_msg_verify_short(port,
-			PD_HEADER(PD_DATA_VENDOR_DEF, PD_PLUG_CABLE_VPD, 0,
+			PD_HEADER(PD_DATA_VENDOR_DEF, PD_PLUG_FROM_CABLE, 0,
 			pd_port[port].msg_tx_id, 5, pd_port[port].rev, 0)));
 	TEST_ASSERT(pd_test_tx_msg_verify_word(port, expected_vdm_header));
 	TEST_ASSERT(pd_test_tx_msg_verify_word(port, expected_vdo_id_header));