summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlec Berg <alecaberg@chromium.org>2014-09-24 17:50:47 -0700
committerchrome-internal-fetch <chrome-internal-fetch@google.com>2014-09-25 18:15:47 +0000
commit69238f9c661298bbf9d874a3ae927afcc718cf4a (patch)
tree8a6c1e90b445e95c00f5bf57f4657e9409f7f928
parent9c62920f96e844760aeef403da9d19f08060fd29 (diff)
downloadchrome-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.c1
-rw-r--r--common/usb_pd_protocol.c28
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)) {