mirror of
https://github.com/Telecominfraproject/OpenCellular.git
synced 2026-01-01 21:02:27 +00:00
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>
This commit is contained in:
@@ -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);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user