scottk/OpenCellular
1
0
Fork 0
mirror of https://github.com/Telecominfraproject/OpenCellular.git synced 2026-01-09 17:11:42 +00:00
Code Issues Packages Projects Releases Wiki Activity

pd_suspend: coordinate with pd_task().

Browse Source 
looks like we had a bit of a race condition: set_state() was
effectively just an assignment opration to pd[port].task_state.  it's
called asynchronously from pd_set_suspend() in response to a
PD_SUSPEND message from the AP as well as from pd_task() before it
enters its main event loop.  this can take a long time because
tcpci_tcpm_init() has a 300ms timeout.  last one wins.

similarly, when pd_task() is running its main loop, pd_set_suspend()
really needs to wait for pd_task() to actually enter the
PD_STATE_SUSPENDED state before the caller can assume that the
pd_task() has stopped accessing the TCPC.

the particular failure case was when depthcharge would decide to do a
TCPC firmware update.  it starts by sending a PD_SUSPEND to the EC,
then accessing the TCPC.  unfortunately, the pd_task() hadn't gotten
out of the way yet, thus causing TCPC access chaos.

so, i'm adding a req_suspend_state flag to the pd_protocol struct so
we can tell pd_task() to suspend itself in a controlled manner.  when
pd_task() is ready to do a state change - basically at the top of the
main event loop -  it'll change to PD_STATE_SUSPENDED and clear the
req_suspend_state flag.

in any case, pd_set_suspend() still needs to wait around for pd_task()
to enter the suspended state as we don't have a fancy handshake
mechanism between these tasks.

TEST=in combination with some follow-on CLs, ps8751 firmware update
	works properly where previously it needed a ~2 second delay
	for the EC pd_task() to settle.  the way to trigger the
	failure was to insert or remove the power brick.

BRANCH=none

BUG=b:62356808

Change-Id: I363803ff60db31ccf84d592f8c9d1610fbe0f9ce
Signed-off-by: Caveh Jalali <caveh@google.com>
Reviewed-on: https://chromium-review.googlesource.com/544659
Reviewed-by: Shawn N <shawnn@chromium.org>
This commit is contained in:
Caveh Jalali
2017-06-20 17:44:55 -07:00
committed by chrome-bot
parent e85d3e9825
commit 7771c52368
1 changed files with 35 additions and 9 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
common/usb_pd_protocol.c
View File

@@ -118,6 +118,8 @@ static struct pd_protocol {
enum pd_states task_state;
/* PD state when we run state handler the last time */
enum pd_states last_state;
/* bool: request state change to SUSPENDED */
uint8_t req_suspend_state;
/* The state to go to after timeout */
enum pd_states timeout_state;
/* Timeout for the current state. Set to 0 for no timeout. */
@@ -1803,6 +1805,10 @@ void pd_task(void)
if (!tcpm_get_message(port, payload, &head))
handle_request(port, head, payload);
}
if (pd[port].req_suspend_state)
set_state(port, PD_STATE_SUSPENDED);
/* if nothing to do, verify the state of the world in 500ms */
this_state = pd[port].task_state;
timeout = 500*MSEC;
@@ -2299,6 +2305,7 @@ void pd_task(void)
break;
case PD_STATE_SUSPENDED:
CPRINTS("TCPC p%d suspended!", port);
pd[port].req_suspend_state = 0;
#ifdef CONFIG_USB_PD_TCPC
pd_rx_disable_monitoring(port);
pd_hw_release(port);
@@ -3108,19 +3115,38 @@ DECLARE_HOOK(HOOK_CHIPSET_SHUTDOWN, dual_role_force_sink, HOOK_PRIO_DEFAULT);
#endif /* CONFIG_USB_PD_DUAL_ROLE */
#ifdef CONFIG_COMMON_RUNTIME
/*
* (enable=1) request pd_task transition to the suspended state. hang
* around for a while until we observe the state change. this can
* take a while (like 300ms) on startup when pd_task is sleeping in
* tcpci_tcpm_init.
*
* (enable=0) force pd_task out of the suspended state and into the
* port's default state.
*/
void pd_set_suspend(int port, int enable)
{
int tries = 3;
int tries = 300;
do {
set_state(port, enable ? PD_STATE_SUSPENDED :
PD_DEFAULT_STATE(port));
if (enable) {
pd[port].req_suspend_state = 1;
do {
task_wake(PD_PORT_TO_TASK_ID(port));
if (pd[port].task_state == PD_STATE_SUSPENDED)
break;
msleep(1);
} while (--tries != 0);
if (!tries)
CPRINTS("TCPC p%d set_suspend failed!", port);
} else {
if (pd[port].task_state != PD_STATE_SUSPENDED)
CPRINTS("TCPC p%d suspend disable request "
"while not suspended!", port);
set_state(port, PD_DEFAULT_STATE(port));
task_wake(PD_PORT_TO_TASK_ID(port));
} while (enable && pd[port].task_state != PD_STATE_SUSPENDED
&& --tries);
if (!tries)
CPRINTS("TCPC p%d set_suspend failed!", port);
}
}
#if defined(CONFIG_CMD_PD) && defined(CONFIG_CMD_PD_FLASH)