diff options
author | Alec Berg <alecaberg@chromium.org> | 2014-09-24 17:50:47 -0700 |
---|---|---|
committer | chrome-internal-fetch <chrome-internal-fetch@google.com> | 2014-09-25 18:15:47 +0000 |
commit | 69238f9c661298bbf9d874a3ae927afcc718cf4a (patch) | |
tree | 8a6c1e90b445e95c00f5bf57f4657e9409f7f928 | |
parent | 9c62920f96e844760aeef403da9d19f08060fd29 (diff) | |
download | chrome-ec-69238f9c661298bbf9d874a3ae927afcc718cf4a.tar.gz |
pd: change pd_soft_reset() to use PD task to send command
Fix potential bug in pd_soft_reset() function. That function is
part of a global API and as such can be called by other tasks.
For example, a sysjump which takes place as part of host command
task. So, this function should not directly initiate PD communication
because if it is interrupted by the PD task, then there will
be unpredictable behavior since the send_validate_message() is not
designed to be re-entrant for a given port.
This changes pd_soft_reset() to simply change the PD state to
SOFT_RESET and then wake up the task to actually send the command.
BUG=none
BRANCH=none
TEST=you can test this with a type-C to A receptacle dongle. The
dongle has a pulldown on the CC line, but no device to respond to
PD comms. When you plug in C to A cable, samus should send source
cap repeatedly for 5 seconds. During that time, if you do a sysjump
from RO to RW, it will call pd_soft_reset(), which will send the
soft reset command. But, since there is no device it will timeout
and retry 3 times. During that period, the PD task will wake up
and try to do it's own thing, causing craziness and eventually a
hang and watchdog reset.
With this fix, I can plug in a C to A adapter, and sysjump to RW
cleanly.
Change-Id: Icab936ab8ab930e8e37b5a23825f7f054a50c177
Signed-off-by: Alec Berg <alecaberg@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/219893
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
Reviewed-by: Vic Yang <victoryang@chromium.org>
-rw-r--r-- | common/system.c | 1 | ||||
-rw-r--r-- | common/usb_pd_protocol.c | 28 |
2 files changed, 13 insertions, 16 deletions
diff --git a/common/system.c b/common/system.c index ee6d7ce97e..5db3681c26 100644 --- a/common/system.c +++ b/common/system.c @@ -409,6 +409,7 @@ static void jump_to_image(uintptr_t init_addr) * power. */ pd_soft_reset(); + usleep(5*MSEC); #endif /* Flush UART output unless the UART hasn't been initialized yet */ diff --git a/common/usb_pd_protocol.c b/common/usb_pd_protocol.c index 13cea00978..3835458db4 100644 --- a/common/usb_pd_protocol.c +++ b/common/usb_pd_protocol.c @@ -705,9 +705,8 @@ void pd_soft_reset(void) for (i = 0; i < PD_PORT_COUNT; ++i) if (pd_is_connected(i)) { - execute_soft_reset(i); - send_control(i, PD_CTRL_SOFT_RESET); set_state(i, PD_STATE_SOFT_RESET); + task_wake(PORT_TO_TASK_ID(i)); } } @@ -1360,19 +1359,9 @@ void pd_task(void) break; } - timeout = 10 * MSEC; - /* Ping dropped. Try soft reset. */ - execute_soft_reset(port); - res = send_control(port, PD_CTRL_SOFT_RESET); - - if (res >= 0) { - set_state(port, PD_STATE_SOFT_RESET); - break; - } - - /* Soft reset failed. Let's try hard reset. */ - set_state(port, PD_STATE_HARD_RESET); + set_state(port, PD_STATE_SOFT_RESET); + timeout = 10 * MSEC; break; #ifdef CONFIG_USB_PD_DUAL_ROLE case PD_STATE_SUSPENDED: @@ -1456,6 +1445,15 @@ void pd_task(void) #endif /* CONFIG_USB_PD_DUAL_ROLE */ case PD_STATE_SOFT_RESET: if (pd[port].last_state != pd[port].task_state) + execute_soft_reset(port); + res = send_control(port, PD_CTRL_SOFT_RESET); + + /* if soft reset failed, try hard reset. */ + if (res < 0) { + set_state(port, PD_STATE_HARD_RESET); + break; + } + set_state_timeout( port, get_time().val + PD_T_SENDER_RESPONSE, @@ -1785,8 +1783,6 @@ static int command_pd(int argc, char **argv) ccprintf("%08x ", pd[port].dev_rw_hash[i]); ccprintf("\n"); } else if (!strncasecmp(argv[2], "soft", 4)) { - execute_soft_reset(port); - send_control(port, PD_CTRL_SOFT_RESET); set_state(port, PD_STATE_SOFT_RESET); task_wake(PORT_TO_TASK_ID(port)); } else if (!strncasecmp(argv[2], "ping", 4)) { |