summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorVadim Bendebury <vbendeb@chromium.org>2017-06-29 17:59:44 -0700
committerChromeOS Commit Bot <chromeos-commit-bot@chromium.org>2017-09-14 16:56:06 +0000
commitc05e94813f8de6a4dee830dead09432fa9204e31 (patch)
treee6341124ca540715c76df27f7e997e89ea417bc6
parent12948bda110f1d1decf2a133666dba8ae994fec8 (diff)
downloadchrome-ec-c05e94813f8de6a4dee830dead09432fa9204e31.tar.gz
g: add 'recover hosed slave' i2cs capability
A common failure condition on the i2c bus is when the master unexpectedly stops clocking the bus while the slave is driving the SDA line low. In this case the master is not able to issue Stop or Start sequences, which makes the bus unusable. Good slave controllers are able to detect this condition and recover from it by removing the pull down from the SDA line. This patch adds this capability to the g chip i2c slave controller. A new timer function is created which samples the SDA line twice a second. If it detects that SDA is low in two consecutive invocations and the number of i2cs read interrupts has not advanced, it decides that the "hosed slave" condition is happening and reinitializes the i2c driver, which removes the hold from the SDA line. Even though the state of the SDA line is supposed to be accessible through the I2CS_READVAL register, it in fact is not, reads always return zero in the SDA bit. To work around this a GPIO (port 0, bit 14) is being allocated to allow to monitor the state of the line, it is multiplexed to the same pin the SDA line uses. When the AP is in low power modes the SDA line is held low, this state should not trigger i2c reinitializations. 9308.25 Cherry-pick note: ap_is_on() is not available in this branch, it was replaced with device_get_state(DEVICE_AP) == DEVICE_STATE_ON. CQ-DEPEND=CL:616300 BRANCH=none BUG=b:35648537 TEST=connected H1 on the test board to an I2c master capable of stopping clocking mid byte. Observed that the existing code would just sit in the "hosed" state indefinitely. The code with the fix recovers from the condition (drives the SDA line high) 500ms to 1s after the failure condition is created. Change-Id: Iafc7433bbae9e49975a72ef032a923274f8aab3b Signed-off-by: Vadim Bendebury <vbendeb@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/614391 Reviewed-by: Aseda Aboagye <aaboagye@chromium.org> (cherry picked from commit 430d55879dc0ce9567b4ee2eefed200e604091ee) Reviewed-on: https://chromium-review.googlesource.com/622277 (cherry picked from commit 70ac0da2a698a9a75edab2ad6a264cc798e09cc4) Reviewed-on: https://chromium-review.googlesource.com/666531
-rw-r--r--board/cr50/board.c6
-rw-r--r--board/cr50/gpio.inc11
-rw-r--r--chip/g/i2cs.c75
-rw-r--r--chip/g/i2cs.h11
-rw-r--r--common/i2cs_tpm.c8
5 files changed, 111 insertions, 0 deletions
diff --git a/board/cr50/board.c b/board/cr50/board.c
index 4537f2aab9..8cc0970586 100644
--- a/board/cr50/board.c
+++ b/board/cr50/board.c
@@ -1335,6 +1335,12 @@ void i2cs_set_pinmux(void)
GWRITE_FIELD(PINMUX, DIOA1_CTL, IE, 1); /* I2CS_SDA */
GWRITE_FIELD(PINMUX, DIOA9_CTL, IE, 1); /* I2CS_SCL */
+ /*
+ * Provide access to the SDA line to be able to detect 'hosed i2c
+ * slave' condition.
+ */
+ GWRITE(PINMUX, GPIO0_GPIO14_SEL, GC_PINMUX_DIOA1_SEL);
+
/* Allow I2CS_SCL to wake from sleep */
GWRITE_FIELD(PINMUX, EXITEDGE0, DIOA9, 1); /* edge sensitive */
GWRITE_FIELD(PINMUX, EXITINV0, DIOA9, 1); /* wake on low */
diff --git a/board/cr50/gpio.inc b/board/cr50/gpio.inc
index faa6ab09be..39686a1943 100644
--- a/board/cr50/gpio.inc
+++ b/board/cr50/gpio.inc
@@ -100,6 +100,17 @@ GPIO(EN_PP3300_INA_L, PIN(0, 11), GPIO_ODR_HIGH)
GPIO(I2C_SCL_INA, PIN(0, 12), GPIO_INPUT)
GPIO(I2C_SDA_INA, PIN(0, 13), GPIO_INPUT)
+/*
+ * Use this to poll the state of the I2CS SDA line. Note that this is not
+ * necessary if SPI interface is used to communicate with the AP, if needed,
+ * this GPIO could be reclaimed in that case.
+ *
+ * Actual attachment of this GPIO to the SDA line happens in board.c only when
+ * I2CS interface is required. Should this GPIO ever change, the code setting
+ * up the pinmux in board.c will have to change as well.
+ */
+GPIO(I2CS_SDA, PIN(0, 14), GPIO_INPUT)
+
/* Unimplemented signals which we need to emulate for now */
/* TODO(wfrichar): Half the boards don't use this signal. Take it out. */
UNIMPLEMENTED(ENTERING_RW)
diff --git a/chip/g/i2cs.c b/chip/g/i2cs.c
index 6efc891bdd..17183c7b7f 100644
--- a/chip/g/i2cs.c
+++ b/chip/g/i2cs.c
@@ -63,7 +63,10 @@
*/
#include "common.h"
+#include "board.h"
#include "console.h"
+#include "device_state.h"
+#include "gpio.h"
#include "hooks.h"
#include "i2cs.h"
#include "pmu.h"
@@ -96,6 +99,13 @@ static uint16_t last_write_pointer;
*/
static uint16_t last_read_pointer;
+/*
+ * Keep track of i2c interrupts and the number of times the "hosed slave"
+ * condition was encountered.
+ */
+static uint16_t i2cs_read_irq_count;
+static uint16_t i2cs_read_recovery_count;
+
static void i2cs_init(void)
{
/* First decide if i2c is even needed for this platform. */
@@ -118,6 +128,7 @@ static void i2cs_init(void)
memset(i2cs_buffer, 0, sizeof(i2cs_buffer));
last_write_pointer = 0;
last_read_pointer = 0;
+ i2cs_read_irq_count = 0;
GWRITE_FIELD(PMU, RST0, DI2CS0, 0);
@@ -132,6 +143,56 @@ static void i2cs_init(void)
GWRITE(I2CS, SLAVE_DEVADDRVAL, 0x50);
}
+/* Forward declaration of the hook function. */
+static void poll_read_state(void);
+DECLARE_DEFERRED(poll_read_state);
+
+/* Poll SDA line to detect the "hosed" condition. */
+#define READ_STATUS_CHECK_INTERVAL (500 * MSEC)
+
+/*
+ * Check for receive problems, if found - reinitialize the i2c slave
+ * interface.
+ */
+static void poll_read_state(void)
+{
+ /*
+ * Make sure there is no accidental match between
+ * last_i2cs_read_irq_count and i2cs_read_irq_count if the first run
+ * of this function happens when SDA is low.
+ */
+ static uint16_t last_i2cs_read_irq_count = ~0;
+
+ if (device_get_state(DEVICE_AP) == DEVICE_STATE_ON) {
+ if (!gpio_get_level(GPIO_I2CS_SDA)) {
+ if (last_i2cs_read_irq_count == i2cs_read_irq_count) {
+ /*
+ * SDA line is low and number of RX interrupts
+ * has not changed since last poll when it was
+ * low, it must be hosed. Reinitialize the i2c
+ * interface (which will also restart this
+ * polling function).
+ */
+ last_i2cs_read_irq_count = ~0;
+ i2cs_read_recovery_count++;
+ i2cs_register_write_complete_handler
+ (write_complete_handler_);
+ return;
+ }
+ last_i2cs_read_irq_count = i2cs_read_irq_count;
+ }
+ } else {
+ /*
+ * AP is off, let's make sure that in case this function
+ * happens to run right after AP wakes up and i2c is active,
+ * there is no false positive 'hosed' condition detection.
+ */
+ if (last_i2cs_read_irq_count == i2cs_read_irq_count)
+ last_i2cs_read_irq_count -= 1;
+ }
+ hook_call_deferred(&poll_read_state_data, READ_STATUS_CHECK_INTERVAL);
+}
+
/* Process the 'end of a write cycle' interrupt. */
static void _i2cs_write_complete_int(void)
{
@@ -185,6 +246,9 @@ static void _i2cs_write_complete_int(void)
/* Invoke the callback to process the message. */
write_complete_handler_(i2cs_buffer, bytes_processed);
+
+ if (bytes_processed == 1)
+ i2cs_read_irq_count++;
}
/*
@@ -287,6 +351,12 @@ int i2cs_register_write_complete_handler(wr_complete_handler_f wc_handler)
write_complete_handler_ = wc_handler;
task_enable_irq(GC_IRQNUM_I2CS0_INTR_WRITE_COMPLETE_INT);
+ /*
+ * Start a self perpetuating polling function to check for 'hosed'
+ * condition periodically.
+ */
+ hook_call_deferred(&poll_read_state_data, READ_STATUS_CHECK_INTERVAL);
+
return 0;
}
@@ -315,3 +385,8 @@ size_t i2cs_zero_read_fifo_buffer_depth(void)
*/
return depth;
}
+
+void i2cs_get_status(struct i2cs_status *status)
+{
+ status->read_recovery_count = i2cs_read_recovery_count;
+}
diff --git a/chip/g/i2cs.h b/chip/g/i2cs.h
index 809e4127a0..8fbc28187f 100644
--- a/chip/g/i2cs.h
+++ b/chip/g/i2cs.h
@@ -50,4 +50,15 @@ size_t i2cs_zero_read_fifo_buffer_depth(void);
*/
void i2cs_post_read_fill_fifo(uint8_t *buffer, size_t len);
+/*
+ * Provide upper layers with information with the I2CS interface
+ * status/statistics. The only piece of information currently provided is the
+ * counter of "hosed" i2c interface occurences, where i2c clocking stopped
+ * while slave was transmitting a zero.
+ */
+struct i2cs_status {
+ uint16_t read_recovery_count;
+};
+void i2cs_get_status(struct i2cs_status *status);
+
#endif /* ! __CHIP_G_I2CS_H */
diff --git a/common/i2cs_tpm.c b/common/i2cs_tpm.c
index e649be1675..42089681dd 100644
--- a/common/i2cs_tpm.c
+++ b/common/i2cs_tpm.c
@@ -229,14 +229,22 @@ DECLARE_HOOK(HOOK_INIT, i2cs_if_register, HOOK_PRIO_LAST);
static int command_i2cs(int argc, char **argv)
{
+ static uint16_t base_read_recovery_count;
+ struct i2cs_status status;
+
+ i2cs_get_status(&status);
+
ccprintf("rd fifo adjust cnt = %d\n", i2cs_fifo_adjust_count);
ccprintf("wr mismatch cnt = %d\n", i2cs_write_error_count);
+ ccprintf("read recovered cnt = %d\n", status.read_recovery_count
+ - base_read_recovery_count);
if (argc < 2)
return EC_SUCCESS;
if (!strcasecmp(argv[1], "reset")) {
i2cs_fifo_adjust_count = 0;
i2cs_write_error_count = 0;
+ base_read_recovery_count = status.read_recovery_count;
ccprintf("i2cs error counts reset\n");
} else
return EC_ERROR_PARAM1;