summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPeter Marheine <pmarheine@chromium.org>2020-08-04 11:24:16 +1000
committerCommit Bot <commit-bot@chromium.org>2020-08-06 07:27:49 +0000
commit0385893469bad2c00690268d112436086fa69d1a (patch)
tree80f2a73e6933d8aa3c76b12172659348e42a71bc
parentea334082eb7bcf690a54e2773ddaec2905075922 (diff)
downloadchrome-ec-0385893469bad2c00690268d112436086fa69d1a.tar.gz
usbc: correctly handle Get_Source_Cap as a sink
USB Power Delivery Specification Revision 3.0, version 2.0 section 6.3.7 states that a dual-role port shall respond to Get_Source_Cap with its source capabilities, but this was incorrectly handled by responding with a request for source capabilities. Per section 8.3.3.18.10, implement the PE_DR_SNK_Give_Source_Cap state to handle this correctly. To support the new test, some helper functions for the fake PE are added and the test code's copy of the PE state enum is updated to be in sync with the real one. BUG=b:161400825,b:161331630 TEST=New host test for this state, and verified on Dalboz that requesting a PRS via the EC console (`pd 1 swap power`) now sends source capabilities when the partner requests them. BRANCH=None Signed-off-by: Peter Marheine <pmarheine@chromium.org> Change-Id: I87c27d406e0a3f57cf2c25fa583bee51155b6b12 Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/ec/+/2336233 Reviewed-by: Jett Rink <jettrink@chromium.org> Reviewed-by: Diana Z <dzigterman@chromium.org>
-rw-r--r--common/usbc/usb_pe_drp_sm.c37
-rw-r--r--include/usb_prl_sm.h11
-rw-r--r--test/fake_prl.c16
-rw-r--r--test/usb_pe.h6
-rw-r--r--test/usb_pe_drp.c27
5 files changed, 92 insertions, 5 deletions
diff --git a/common/usbc/usb_pe_drp_sm.c b/common/usbc/usb_pe_drp_sm.c
index 30097dc2af..8663adecc2 100644
--- a/common/usbc/usb_pe_drp_sm.c
+++ b/common/usbc/usb_pe_drp_sm.c
@@ -251,6 +251,7 @@ enum usb_pe_state {
PE_BIST_RX,
PE_DEU_SEND_ENTER_USB,
PE_DR_SNK_GET_SINK_CAP,
+ PE_DR_SNK_GIVE_SOURCE_CAP,
/* AMS Start parent - runs SenderResponseTimer */
PE_SENDER_RESPONSE,
@@ -372,6 +373,7 @@ static const char * const pe_state_names[] = {
[PE_BIST_RX] = "PE_Bist_RX",
[PE_DEU_SEND_ENTER_USB] = "PE_DEU_Send_Enter_USB",
[PE_DR_SNK_GET_SINK_CAP] = "PE_DR_SNK_Get_Sink_Cap",
+ [PE_DR_SNK_GIVE_SOURCE_CAP] = "PE_DR_SNK_Give_Source_Cap",
[PE_SENDER_RESPONSE] = "PE_SENDER_RESPONSE",
@@ -2807,7 +2809,7 @@ static void pe_snk_ready_run(int port)
/* Do nothing */
break;
case PD_CTRL_GET_SOURCE_CAP:
- set_state_pe(port, PE_SNK_GET_SOURCE_CAP);
+ set_state_pe(port, PE_DR_SNK_GIVE_SOURCE_CAP);
return;
case PD_CTRL_GET_SINK_CAP:
set_state_pe(port, PE_SNK_GIVE_SINK_CAP);
@@ -5447,6 +5449,35 @@ static void pe_dr_snk_get_sink_cap_run(int port)
}
/*
+ * PE_DR_SNK_Give_Source_Cap
+ */
+static void pe_dr_snk_give_source_cap_entry(int port)
+{
+ print_current_state(port);
+
+ /* Send source capabilities. */
+ send_source_cap(port);
+}
+
+static void pe_dr_snk_give_source_cap_run(int port)
+{
+ /*
+ * Transition back to PE_SNK_Ready when the Source_Capabilities message
+ * has been successfully sent.
+ *
+ * Get Source Capabilities AMS is uninterruptible, but in case the
+ * partner violates the spec then send a soft reset rather than get
+ * stuck here.
+ */
+ if (PE_CHK_FLAG(port, PE_FLAGS_TX_COMPLETE)) {
+ PE_CLR_FLAG(port, PE_FLAGS_TX_COMPLETE);
+ set_state_pe(port, PE_SNK_READY);
+ } else if (PE_CHK_FLAG(port, PE_FLAGS_MSG_DISCARDED)) {
+ pe_send_soft_reset(port, TCPC_TX_SOP);
+ }
+}
+
+/*
* PE_SENDER_RESPONSE
*
* Parent state to run first message in an AMS and start SenderResponseTimer
@@ -5822,6 +5853,10 @@ static const struct usb_state pe_states[] = {
.run = pe_dr_snk_get_sink_cap_run,
.parent = &pe_states[PE_SENDER_RESPONSE],
},
+ [PE_DR_SNK_GIVE_SOURCE_CAP] = {
+ .entry = pe_dr_snk_give_source_cap_entry,
+ .run = pe_dr_snk_give_source_cap_run,
+ },
[PE_SENDER_RESPONSE] = {
.entry = pe_sender_response_entry,
.run = pe_sender_response_run,
diff --git a/include/usb_prl_sm.h b/include/usb_prl_sm.h
index 4b99d4c4c3..4f6f86e346 100644
--- a/include/usb_prl_sm.h
+++ b/include/usb_prl_sm.h
@@ -124,6 +124,17 @@ enum pd_ctrl_msg_type fake_prl_get_last_sent_ctrl_msg(int port);
* Fake to set the last sent control message to an invalid value.
*/
void fake_prl_clear_last_sent_ctrl_msg(int port);
+
+/**
+ * Get the type of the last sent data message on the given port.
+ */
+enum pd_data_msg_type fake_prl_get_last_sent_data_msg_type(int port);
+
+/**
+ * Clear the last sent data message on the given port to an invalid value,
+ * making it possible to check that two of the same message were sent in order.
+ */
+void fake_prl_clear_last_sent_data_msg(int port);
#endif
#endif /* __CROS_EC_USB_PRL_H */
diff --git a/test/fake_prl.c b/test/fake_prl.c
index dcfdce9606..3674953c45 100644
--- a/test/fake_prl.c
+++ b/test/fake_prl.c
@@ -37,6 +37,8 @@ void prl_reset_soft(int port)
{}
static enum pd_ctrl_msg_type last_ctrl_msg[CONFIG_USB_PD_PORT_MAX_COUNT];
+static enum pd_data_msg_type last_data_msg_type[CONFIG_USB_PD_PORT_MAX_COUNT];
+
void prl_send_ctrl_msg(int port, enum tcpm_transmit_type type,
enum pd_ctrl_msg_type msg)
{
@@ -45,7 +47,9 @@ void prl_send_ctrl_msg(int port, enum tcpm_transmit_type type,
void prl_send_data_msg(int port, enum tcpm_transmit_type type,
enum pd_data_msg_type msg)
-{}
+{
+ last_data_msg_type[port] = msg;
+}
void prl_send_ext_data_msg(int port, enum tcpm_transmit_type type,
enum pd_ext_msg_type msg)
@@ -65,3 +69,13 @@ void fake_prl_clear_last_sent_ctrl_msg(int port)
{
last_ctrl_msg[port] = 0;
}
+
+enum pd_data_msg_type fake_prl_get_last_sent_data_msg_type(int port)
+{
+ return last_data_msg_type[port];
+}
+
+void fake_prl_clear_last_sent_data_msg(int port)
+{
+ last_data_msg_type[port] = 0;
+}
diff --git a/test/usb_pe.h b/test/usb_pe.h
index f67146bd4d..3ba11ce040 100644
--- a/test/usb_pe.h
+++ b/test/usb_pe.h
@@ -116,20 +116,20 @@ enum usb_pe_state {
PE_VCS_TURN_ON_VCONN_SWAP,
PE_VCS_TURN_OFF_VCONN_SWAP,
PE_VCS_SEND_PS_RDY_SWAP,
- PE_DO_PORT_DISCOVERY,
PE_VDM_SEND_REQUEST,
PE_VDM_IDENTITY_REQUEST_CBL,
PE_INIT_PORT_VDM_IDENTITY_REQUEST,
PE_INIT_VDM_SVIDS_REQUEST,
PE_INIT_VDM_MODES_REQUEST,
- PE_VDM_REQUEST,
- PE_VDM_ACKED,
+ PE_VDM_REQUEST_DPM,
PE_VDM_RESPONSE,
PE_HANDLE_CUSTOM_VDM_REQUEST,
PE_WAIT_FOR_ERROR_RECOVERY,
PE_BIST_TX,
PE_BIST_RX,
+ PE_DEU_SEND_ENTER_USB,
PE_DR_SNK_GET_SINK_CAP,
+ PE_DR_SNK_GIVE_SOURCE_CAP,
#ifdef CONFIG_USB_PD_REV30
/* PD3.0 only states below here*/
diff --git a/test/usb_pe_drp.c b/test/usb_pe_drp.c
index 6cd1fb2bde..b57f0c09bd 100644
--- a/test/usb_pe_drp.c
+++ b/test/usb_pe_drp.c
@@ -178,6 +178,32 @@ static int test_pe_frs(void)
return EC_SUCCESS;
}
+static int test_snk_give_source_cap(void)
+{
+ setup_sink();
+
+ /*
+ * Receive a Get_Source_Cap message; respond with Source_Capabilities
+ * and return to PE_SNK_Ready once sent.
+ */
+ rx_emsg[PORT0].header =
+ PD_HEADER(PD_CTRL_GET_SOURCE_CAP, 0, 0, 0, 0, 0, 0);
+ pe_set_flag(PORT0, PE_FLAGS_MSG_RECEIVED);
+ task_wait_event(10 * MSEC);
+
+ TEST_ASSERT(!pe_chk_flag(PORT0, PE_FLAGS_MSG_RECEIVED));
+ TEST_ASSERT(!pe_chk_flag(PORT0, PE_FLAGS_TX_COMPLETE));
+ TEST_EQ(fake_prl_get_last_sent_data_msg_type(PORT0),
+ PD_DATA_SOURCE_CAP, "%d");
+ TEST_EQ(get_state_pe(PORT0), PE_DR_SNK_GIVE_SOURCE_CAP, "%d");
+
+ pe_set_flag(PORT0, PE_FLAGS_TX_COMPLETE);
+ task_wait_event(10 * MSEC);
+ TEST_EQ(get_state_pe(PORT0), PE_SNK_READY, "%d");
+
+ return EC_SUCCESS;
+}
+
static int test_vbus_gpio_discharge(void)
{
pd_set_vbus_discharge(PORT0, 1);
@@ -318,6 +344,7 @@ void run_test(int argc, char **argv)
test_reset();
RUN_TEST(test_pe_frs);
+ RUN_TEST(test_snk_give_source_cap);
RUN_TEST(test_vbus_gpio_discharge);
#ifndef CONFIG_USB_PD_EXTENDED_MESSAGES
RUN_TEST(test_extended_message_not_supported_src);