diff options
author | Aseda Aboagye <aaboagye@google.com> | 2019-05-07 16:38:29 -0700 |
---|---|---|
committer | chrome-bot <chrome-bot@chromium.org> | 2019-05-17 18:37:18 -0700 |
commit | 7495960f1f6f780fa63a57b9caaddbb82bdd2a32 (patch) | |
tree | 663d3a7ab93107b44671d07e1ab5a6bf60b4b205 | |
parent | 50f59b1d4e8ff077536c0f494e6a38635e90f6a2 (diff) | |
download | chrome-ec-7495960f1f6f780fa63a57b9caaddbb82bdd2a32.tar.gz |
pd_protocol: Add ready_state_holdoff_timer.
Recently a sink holdoff timer was added to the PD stack which allowed
the state machine to prevent initiating any messages for 200ms after
entering the SNK_READY state. This was to give time for some chatty
sources to send messages to avoid a collision. Apparently, the same
thing can happen when we are a source (collision with chatty sink).
This commit reuses the holdoff timer for resolution as a source as well,
which starts after an explicit contract is established.
In order to prevent any potential new collisions, some jitter based off
of the system timestamp is added to the holdoff timer.
BUG=b:132202148, chromium:925618
BRANCH=firmware-atlas-11827.B
TEST=Flash atlas, plug in a fully featured C-C cable between atlas and
the LG 27UK850-W, verify that no conflict occurs and external display
always works.
TEST=Verify that no messages are initiated by the source within 200ms of
sending PS_RDY.
TEST=Flash nocturne, verify Dell U3818DW still works over C-C cable.
TEST=Flash nocturne, verify CableMatters MST DP hub still works with
charge through.
TEST=Verify with Twinkie that messages are sent at varied timestamps
between 200-300ms in the SNK/SRC_READY state.
Change-Id: I195199de271950ae09c2b26194ddc5f271b296a0
Signed-off-by: Aseda Aboagye <aaboagye@google.com>
Reviewed-on: https://chromium-review.googlesource.com/1600510
Commit-Ready: Aseda Aboagye <aaboagye@chromium.org>
Tested-by: Aseda Aboagye <aaboagye@chromium.org>
Reviewed-by: Jett Rink <jettrink@chromium.org>
Reviewed-by: Diana Z <dzigterman@chromium.org>
-rw-r--r-- | common/usb_pd_protocol.c | 61 |
1 files changed, 44 insertions, 17 deletions
diff --git a/common/usb_pd_protocol.c b/common/usb_pd_protocol.c index 48d3c7afdd..92aecfce9f 100644 --- a/common/usb_pd_protocol.c +++ b/common/usb_pd_protocol.c @@ -95,13 +95,13 @@ static const int debug_level; #define PD_SNK_CAP_RETRIES 3 /* - * The time that we allow the source to send any messages after an explicit - * contract is established. 200ms was chosen somewhat arbitrarily as it should - * be long enough for sources to decide to send a message if they were going to, - * but not so long that a "low power charger connected" notification would be - * shown in the chrome OS UI. + * The time that we allow the port partner to send any messages after an + * explicit contract is established. 200ms was chosen somewhat arbitrarily as + * it should be long enough for sources to decide to send a message if they were + * going to, but not so long that a "low power charger connected" notification + * would be shown in the chrome OS UI. */ -#define SNK_READY_HOLD_OFF_US (200 * MSEC) +#define READY_HOLD_OFF_US (200 * MSEC) enum vdm_states { VDM_STATE_ERR_BUSY = -3, @@ -249,12 +249,12 @@ static struct pd_protocol { uint8_t rev; #endif /* - * Time at which the port acting as a sink can start to request for - * higher power and interrogate the port partner. Some port partners - * are really chatty after the initial request so we allow this time for - * the port partner to send any messages in order to avoid a collision. + * Some port partners are really chatty after an explicit contract is + * established. Therefore, we allow this time for the port partner to + * send any messages in order to avoid a collision of sending messages + * of our own. */ - uint64_t snk_ready_holdoff_timer; + uint64_t ready_state_holdoff_timer; } pd[CONFIG_USB_PD_PORT_COUNT]; #ifdef CONFIG_COMMON_RUNTIME @@ -711,8 +711,8 @@ static inline void set_state(int port, enum pd_states next_state) ppc_clear_oc_event_counter(port); } #endif /* CONFIG_USBC_PPC */ - /* Clear the SNK_READY holdoff timer. */ - pd[port].snk_ready_holdoff_timer = 0; + /* Clear the holdoff timer since the port is disconnected. */ + pd[port].ready_state_holdoff_timer = 0; /* * We should not clear any flags when transitioning back to the @@ -1773,11 +1773,14 @@ static void handle_ctrl_request(int port, uint16_t head, } else if (pd[port].power_role == PD_ROLE_SINK) { /* * Give the source some time to send any messages before - * we start our interrogation. + * we start our interrogation. Add some jitter of up to + * 100ms, taken from the current system time, to prevent + * multiple collisions. */ if (pd[port].task_state == PD_STATE_SNK_TRANSITION) - pd[port].snk_ready_holdoff_timer = - get_time().val + SNK_READY_HOLD_OFF_US; + pd[port].ready_state_holdoff_timer = + get_time().val + READY_HOLD_OFF_US + + (get_time().le.lo % (100 * MSEC)); set_state(port, PD_STATE_SNK_READY); pd_set_input_current_limit(port, pd[port].curr_limit, @@ -3384,6 +3387,18 @@ void pd_task(void *u) res = send_control(port, PD_CTRL_PS_RDY); if (res >= 0) { timeout = 10*MSEC; + + /* + * Give the sink some time to send any messages + * before we may send messages of our own. Add + * some jitter of up to 100ms, taken from the + * current system time, to prevent multiple + * collisions. + */ + pd[port].ready_state_holdoff_timer = + get_time().val + READY_HOLD_OFF_US + + (get_time().le.lo % (100 * MSEC)); + /* it'a time to ping regularly the sink */ set_state(port, PD_STATE_SRC_READY); } else { @@ -3395,6 +3410,17 @@ void pd_task(void *u) timeout = PD_T_SOURCE_ACTIVITY; /* + * Don't send any traffic yet until our holdoff timer + * has expired. Some devices are chatty once we reach + * the SRC_READY state and we may end up in a collision + * of messages if we try to immediately send our + * interrogations. + */ + if (get_time().val <= + pd[port].ready_state_holdoff_timer) + break; + + /* * Don't send any PD traffic if we woke up due to * incoming packet or if VDO response pending to avoid * collisions. @@ -3910,7 +3936,8 @@ void pd_task(void *u) * of messages if we try to immediately send our * interrogations. */ - if (get_time().val <= pd[port].snk_ready_holdoff_timer) + if (get_time().val <= + pd[port].ready_state_holdoff_timer) break; /* |