diff options
author | Charlie Mooney <charliemooney@chromium.org> | 2012-08-06 10:38:14 -0700 |
---|---|---|
committer | Gerrit <chrome-bot@google.com> | 2012-08-07 12:05:30 -0700 |
commit | a78bb5e560dbb68ae67855d1e2eb642b979e498a (patch) | |
tree | 20f7d59debf9297738a8a84ab247c3d41f9290bb | |
parent | 80e36e02549dc3bb121aee1619760198b706c406 (diff) | |
download | chrome-ec-a78bb5e560dbb68ae67855d1e2eb642b979e498a.tar.gz |
Fix stack overflow in i2c stack for EC
There were a number of problems resulting from i2c crashes,
particularly when trying to access the battery. The problem is that
the stack was overflowing on this particularly deep path, all the way
down to wait_status. This in itself was fine, but if there was a
timeout, debugging information would be printed to the uart, and that
function would cause an exception and restart the EC.
To fix it, I stripped the debugging CPRINTFs from wait_status. This
allows everything to work fine, but looses some information for
debugging. To allow future developers to still see what event the i2c
was waiting for, I added an additional variable to store it in, so that
it can be displayed/handled further up the stack.
BUG=chrome-os-partner:12245
TEST=Boot the machine using a Servo. On the AP's UART, run
"cros_test i2c" to start pounding the i2c bus. Then from the EC, run
"pmu 1000" and then "battery 1000" there should be no error messages,
exceptions, and the EC should not restart. Repeat this process with i2c
arbitration disabled (remove the flag in ./board/snow/board.h). You
should suffer no fatal errors. cros_test may report errors detected,
but the EC will never crash, restart, or throw exceptions. These other
errors are the EC and the AP stepping on each other's toes now that you
have disabled arbitration.
Change-Id: Idd2f017d3557652bf3e8536c4ac776c1f70319cb
Signed-off-by: Charlie Mooney <charliemooney@chromium.org>
Reviewed-on: https://gerrit.chromium.org/gerrit/29351
Reviewed-by: Simon Glass <sjg@chromium.org>
-rw-r--r-- | chip/stm32/i2c.c | 15 |
1 files changed, 8 insertions, 7 deletions
diff --git a/chip/stm32/i2c.c b/chip/stm32/i2c.c index 52c72bfc6e..4c084a0d29 100644 --- a/chip/stm32/i2c.c +++ b/chip/stm32/i2c.c @@ -392,13 +392,7 @@ static int wait_status(int port, uint32_t mask, enum wait_t wait) while (mask ? ((r & mask) != mask) : r) { t2 = get_time(); if (t2.val - t1.val > I2C_TX_TIMEOUT) { -#ifdef CONFIG_DEBUG_I2C - CPRINTF(" m %016b\n", mask); - CPRINTF(" - %016b\n", r); -#endif /* CONFIG_DEBUG_I2C */ - CPRINTF("i2c wait_status timeout type %d, %d us\n", - wait, (unsigned)t2.val - (unsigned)t1.val); - return EC_ERROR_TIMEOUT; + return EC_ERROR_TIMEOUT | (wait << 8); } else if (t2.val - t1.val > 150) { usleep(100); } @@ -470,6 +464,13 @@ static void handle_i2c_error(int port, int rv) if (rv == EC_ERROR_BUSY) return; + /* EC_ERROR_TIMEOUT may have a code specifying where the timeout was */ + if ((rv & 0xff) == EC_ERROR_TIMEOUT) { +#ifdef CONFIG_DEBUG_I2C + CPRINTF("Wait_status() timeout type: %d\n", (rv >> 8)); +#endif + rv = EC_ERROR_TIMEOUT; + } if (rv) dump_i2c_reg(port); |