Skip to content

Commit

Permalink
i2c: Add configurable I2C transfer timeout
Browse files Browse the repository at this point in the history
This patch introduces CONFIG_I2C_TRANSFER_TIMEOUT_US,
which controls how long to wait for an I2C devices to
produce/accept all the data bytes in a single transfer.
(The device can delay transfer by stretching the clock of
the ack bit.)

The default value of this new setting is 500ms.  Existing
code had timeouts anywhere from tens of milliseconds to a
full second beween various drivers.  Drivers can still have
their own shorter timeouts for setup/communication with the
I2C host controller (as opposed to transactions with I2C
devices on the bus.)

In general, the timeout is not meant to be reached except in
situations where there is already serious problem with the
boot, and serves to make sure that some useful diagnostic
output is produced on the console.

Change-Id: I6423122f32aad1dbcee0bfe240cdaa8cb512791f
Signed-off-by: Jes B. Klinke <jbk@chromium.org>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/62278
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
  • Loading branch information
Jes Klinke authored and jwerner-chromium committed Mar 15, 2022
1 parent ca82e61 commit 19baa9d
Show file tree
Hide file tree
Showing 15 changed files with 119 additions and 86 deletions.
8 changes: 8 additions & 0 deletions src/device/Kconfig
Original file line number Diff line number Diff line change
Expand Up @@ -906,6 +906,14 @@ config SOFTWARE_I2C
I2C controller is not (yet) available. The platform code needs to
provide bindings to manually toggle I2C lines.

config I2C_TRANSFER_TIMEOUT_US
int "I2C transfer timeout in microseconds"
default 500000
help
Timeout for a read/write transfers on the I2C bus, that is, the
maximum time a device could stretch clock bits before the transfer
is aborted and an error returned.

config RESOURCE_ALLOCATOR_V3
bool
default n
Expand Down
10 changes: 6 additions & 4 deletions src/drivers/i2c/designware/dw_i2c.c
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@

/* Use a ~10ms timeout for various operations */
#define DW_I2C_TIMEOUT_US 10000
/* Timeout for waiting for FIFO to flush */
#define DW_I2C_FLUSH_TIMEOUT_US 160000

/* High and low times in different speed modes (in ns) */
enum {
Expand Down Expand Up @@ -290,7 +292,7 @@ static enum cb_err dw_i2c_wait_for_bus_idle(struct dw_i2c_regs *regs)
struct stopwatch sw;

/* Start timeout for up to 16 bytes in FIFO */
stopwatch_init_usecs_expire(&sw, 16 * DW_I2C_TIMEOUT_US);
stopwatch_init_usecs_expire(&sw, DW_I2C_FLUSH_TIMEOUT_US);

while (!stopwatch_expired(&sw)) {
uint32_t status = read32(&regs->status);
Expand All @@ -316,7 +318,7 @@ static enum cb_err dw_i2c_transfer_byte(struct dw_i2c_regs *regs,
struct stopwatch sw;
uint32_t cmd = CMD_DATA_CMD; /* Read op */

stopwatch_init_usecs_expire(&sw, DW_I2C_TIMEOUT_US);
stopwatch_init_usecs_expire(&sw, CONFIG_I2C_TRANSFER_TIMEOUT_US);

if (!(segment->flags & I2C_M_RD)) {
/* Write op only: Wait for FIFO not full */
Expand Down Expand Up @@ -409,7 +411,7 @@ static enum cb_err _dw_i2c_transfer(unsigned int bus, const struct i2c_msg *segm
}

/* Wait for interrupt status to indicate transfer is complete */
stopwatch_init_usecs_expire(&sw, DW_I2C_TIMEOUT_US);
stopwatch_init_usecs_expire(&sw, CONFIG_I2C_TRANSFER_TIMEOUT_US);
while (!(read32(&regs->raw_intr_stat) & INTR_STAT_STOP_DET)) {
if (stopwatch_expired(&sw)) {
printk(BIOS_ERR, "I2C stop bit not received\n");
Expand All @@ -436,7 +438,7 @@ static enum cb_err _dw_i2c_transfer(unsigned int bus, const struct i2c_msg *segm
}

/* Flush the RX FIFO in case it is not empty */
stopwatch_init_usecs_expire(&sw, 16 * DW_I2C_TIMEOUT_US);
stopwatch_init_usecs_expire(&sw, DW_I2C_FLUSH_TIMEOUT_US);
while (read32(&regs->status) & STATUS_RX_FIFO_NOT_EMPTY) {
if (stopwatch_expired(&sw)) {
printk(BIOS_ERR, "I2C timeout flushing RX FIFO\n");
Expand Down
27 changes: 16 additions & 11 deletions src/soc/cavium/cn81xx/twsi.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include <delay.h>
#include <device/mmio.h>
#include <soc/addressmap.h>
#include <timer.h>

#define TWSI_THP 24

Expand Down Expand Up @@ -348,17 +349,15 @@ static u8 twsi_read_status(void *baseaddr)
*
* @return 0 for success, 1 if timeout
*/
static int twsi_wait(void *baseaddr)
static int twsi_wait(void *baseaddr, struct stopwatch *timeout)
{
unsigned long timeout = 500000;
u8 twsi_ctl;

printk(BIOS_SPEW, "%s(%p)\n", __func__, baseaddr);
do {
twsi_ctl = twsi_read_ctl(baseaddr);
twsi_ctl &= TWSI_CTL_IFLG;
timeout--;
} while (!twsi_ctl && timeout > 0);
} while (!twsi_ctl && !stopwatch_expired(timeout));

printk(BIOS_SPEW, " return: %u\n", !twsi_ctl);
return !twsi_ctl;
Expand Down Expand Up @@ -438,10 +437,12 @@ static int twsi_start(void *baseaddr)
{
int result;
u8 stat;
struct stopwatch timeout;

printk(BIOS_SPEW, "%s(%p)\n", __func__, baseaddr);
stopwatch_init_usecs_expire(&timeout, CONFIG_I2C_TRANSFER_TIMEOUT_US);
twsi_write_ctl(baseaddr, TWSI_CTL_STA | TWSI_CTL_ENAB);
result = twsi_wait(baseaddr);
result = twsi_wait(baseaddr, &timeout);
if (result) {
stat = twsi_read_status(baseaddr);
printk(BIOS_SPEW, "%s: result: 0x%x, status: 0x%x\n", __func__,
Expand Down Expand Up @@ -475,17 +476,19 @@ static int twsi_write_data(void *baseaddr, const u8 slave_addr,
union twsx_sw_twsi twsi_sw;
unsigned int curr = 0;
int result;
struct stopwatch timeout;

printk(BIOS_SPEW, "%s(%p, 0x%x, %p, 0x%x)\n", __func__, baseaddr,
slave_addr, buffer, length);
stopwatch_init_usecs_expire(&timeout, CONFIG_I2C_TRANSFER_TIMEOUT_US);
result = twsi_start(baseaddr);
if (result) {
printk(BIOS_ERR, "%s: Could not start BUS transaction\n",
__func__);
return -1;
}

result = twsi_wait(baseaddr);
result = twsi_wait(baseaddr, &timeout);
if (result) {
printk(BIOS_ERR, "%s: wait failed\n", __func__);
return result;
Expand All @@ -500,7 +503,7 @@ static int twsi_write_data(void *baseaddr, const u8 slave_addr,
twsi_write_ctl(baseaddr, TWSI_CTL_ENAB);

printk(BIOS_SPEW, "%s: Waiting\n", __func__);
result = twsi_wait(baseaddr);
result = twsi_wait(baseaddr, &timeout);
if (result) {
printk(BIOS_ERR, "%s: Timed out writing slave address 0x%x\n",
__func__, slave_addr);
Expand All @@ -521,7 +524,7 @@ static int twsi_write_data(void *baseaddr, const u8 slave_addr,
twsi_write_sw(baseaddr, twsi_sw);
twsi_write_ctl(baseaddr, TWSI_CTL_ENAB);

result = twsi_wait(baseaddr);
result = twsi_wait(baseaddr, &timeout);
if (result) {
printk(BIOS_ERR, "%s: Timed out writing data to 0x%x\n",
__func__, slave_addr);
Expand Down Expand Up @@ -549,16 +552,18 @@ static int twsi_read_data(void *baseaddr, const u8 slave_addr,
union twsx_sw_twsi twsi_sw;
unsigned int curr = 0;
int result;
struct stopwatch timeout;

printk(BIOS_SPEW, "%s(%p, 0x%x, %p, %u)\n", __func__, baseaddr,
slave_addr, buffer, length);
stopwatch_init_usecs_expire(&timeout, CONFIG_I2C_TRANSFER_TIMEOUT_US);
result = twsi_start(baseaddr);
if (result) {
printk(BIOS_ERR, "%s: start failed\n", __func__);
return result;
}

result = twsi_wait(baseaddr);
result = twsi_wait(baseaddr, &timeout);
if (result) {
printk(BIOS_ERR, "%s: wait failed\n", __func__);
return result;
Expand All @@ -574,7 +579,7 @@ static int twsi_read_data(void *baseaddr, const u8 slave_addr,
twsi_write_sw(baseaddr, twsi_sw);
twsi_write_ctl(baseaddr, TWSI_CTL_ENAB);

result = twsi_wait(baseaddr);
result = twsi_wait(baseaddr, &timeout);
if (result) {
printk(BIOS_ERR, "%s: waiting for sending addr failed\n", __func__);
return result;
Expand All @@ -590,7 +595,7 @@ static int twsi_read_data(void *baseaddr, const u8 slave_addr,
twsi_write_ctl(baseaddr, TWSI_CTL_ENAB |
((curr < length - 1) ? TWSI_CTL_AAK : 0));

result = twsi_wait(baseaddr);
result = twsi_wait(baseaddr, &timeout);
if (result) {
printk(BIOS_ERR, "%s: waiting for data failed\n",
__func__);
Expand Down
2 changes: 1 addition & 1 deletion src/soc/intel/quark/i2c.c
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ int platform_i2c_transfer(unsigned int bus, struct i2c_msg *segment,
status = regs->ic_clr_tx_abrt;

/* Start the timeout */
stopwatch_init_msecs_expire(&timeout, 1000);
stopwatch_init_usecs_expire(&timeout, CONFIG_I2C_TRANSFER_TIMEOUT_US);

/* Process each of the segments */
total_bytes = 0;
Expand Down
2 changes: 1 addition & 1 deletion src/soc/mediatek/common/i2c.c
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,7 @@ static int mtk_i2c_transfer(uint8_t bus, struct i2c_msg *seg,
/* start transfer transaction */
write32(&regs->start, 0x1);

stopwatch_init_msecs_expire(&sw, 100);
stopwatch_init_usecs_expire(&sw, CONFIG_I2C_TRANSFER_TIMEOUT_US);

/* polling mode : see if transaction complete */
while (1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,6 @@ u32 qup_wait_for_s_irq(unsigned int bus);
void qup_m_cancel_and_abort(unsigned int bus);
void qup_s_cancel_and_abort(unsigned int bus);
int qup_handle_transfer(unsigned int bus, const void *dout, void *din,
int size);
int size, struct stopwatch *timeout);

#endif /* __SOC_COMMON_QCOM_QUP_SE_H__ */
7 changes: 3 additions & 4 deletions src/soc/qualcomm/common/qup_se_handler.c
Original file line number Diff line number Diff line change
Expand Up @@ -149,15 +149,14 @@ void qup_s_cancel_and_abort(unsigned int bus)
}
}

int qup_handle_transfer(unsigned int bus, const void *dout, void *din, int size)
int qup_handle_transfer(unsigned int bus, const void *dout, void *din, int size,
struct stopwatch *timeout)
{
unsigned int m_irq;
struct stopwatch sw;
unsigned int rx_rem_bytes = din ? size : 0;
unsigned int tx_rem_bytes = dout ? size : 0;
struct qup_regs *regs = qup[bus].regs;

stopwatch_init_msecs_expire(&sw, 1000);
do {
m_irq = qup_wait_for_m_irq(bus);
if ((m_irq & M_RX_FIFO_WATERMARK_EN) ||
Expand All @@ -172,7 +171,7 @@ int qup_handle_transfer(unsigned int bus, const void *dout, void *din, int size)
break;
}
write32(&regs->geni_m_irq_clear, m_irq);
} while (!stopwatch_expired(&sw));
} while (!stopwatch_expired(timeout));

if (!(m_irq & M_CMD_DONE_EN) || tx_rem_bytes || rx_rem_bytes) {
printk(BIOS_INFO, "%s:Error: Transfer failed\n", __func__);
Expand Down
4 changes: 3 additions & 1 deletion src/soc/qualcomm/common/qupv3_i2c.c
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ static int i2c_do_xfer(unsigned int bus, struct i2c_msg segment,
unsigned int master_cmd_reg_val = (cmd << M_OPCODE_SHFT);
struct qup_regs *regs = qup[bus].regs;
void *dout = NULL, *din = NULL;
struct stopwatch timeout;

if (!(segment.flags & I2C_M_RD)) {
write32(&regs->i2c_tx_trans_len, segment.len);
Expand All @@ -130,7 +131,8 @@ static int i2c_do_xfer(unsigned int bus, struct i2c_msg segment,
master_cmd_reg_val |= (prams & M_PARAMS_MSK);
write32(&regs->geni_m_cmd0, master_cmd_reg_val);

return qup_handle_transfer(bus, dout, din, segment.len);
stopwatch_init_usecs_expire(&timeout, CONFIG_I2C_TRANSFER_TIMEOUT_US);
return qup_handle_transfer(bus, dout, din, segment.len, &timeout);
}

int platform_i2c_transfer(unsigned int bus, struct i2c_msg *segments,
Expand Down
4 changes: 3 additions & 1 deletion src/soc/qualcomm/common/qupv3_spi.c
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ int qup_spi_xfer(const struct spi_slave *slave, const void *dout,
int size;
unsigned int se_bus = slave->bus;
struct qup_regs *regs = qup[se_bus].regs;
struct stopwatch timeout;

if ((bytes_in == 0) && (bytes_out == 0))
return 0;
Expand Down Expand Up @@ -114,7 +115,8 @@ int qup_spi_xfer(const struct spi_slave *slave, const void *dout,

qup_setup_m_cmd(se_bus, m_cmd, m_param);

if (qup_handle_transfer(se_bus, dout, din, size))
stopwatch_init_msecs_expire(&timeout, 1000);
if (qup_handle_transfer(se_bus, dout, din, size, &timeout))
return -1;

qup_spi_xfer(slave, dout + size, MAX((int)bytes_out - size, 0),
Expand Down
32 changes: 18 additions & 14 deletions src/soc/qualcomm/ipq40xx/qup.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <device/mmio.h>
#include <console/console.h>
#include <delay.h>
#include <timer.h>
#include <soc/iomap.h>
#include <soc/qup.h>

Expand Down Expand Up @@ -93,35 +94,33 @@ static qup_return_t qup_reset_master_status(blsp_qup_id_t id)
return QUP_SUCCESS;
}

static qup_return_t qup_fifo_wait_for(blsp_qup_id_t id, uint32_t status)
static qup_return_t qup_fifo_wait_for(blsp_qup_id_t id, uint32_t status,
struct stopwatch *timeout)
{
qup_return_t ret = QUP_ERR_UNDEFINED;
unsigned int count = TIMEOUT_CNT;

while (!(read32(QUP_ADDR(id, QUP_OPERATIONAL)) & status)) {
ret = qup_i2c_master_status(id);
if (ret)
return ret;
if (count == 0)
if (stopwatch_expired(timeout))
return QUP_ERR_TIMEOUT;
count--;
}

return QUP_SUCCESS;
}

static qup_return_t qup_fifo_wait_while(blsp_qup_id_t id, uint32_t status)
static qup_return_t qup_fifo_wait_while(blsp_qup_id_t id, uint32_t status,
struct stopwatch *timeout)
{
qup_return_t ret = QUP_ERR_UNDEFINED;
unsigned int count = TIMEOUT_CNT;

while (read32(QUP_ADDR(id, QUP_OPERATIONAL)) & status) {
ret = qup_i2c_master_status(id);
if (ret)
return ret;
if (count == 0)
if (stopwatch_expired(timeout))
return QUP_ERR_TIMEOUT;
count--;
}

return QUP_SUCCESS;
Expand All @@ -139,15 +138,16 @@ static inline uint32_t qup_i2c_create_output_tag(int stop, u8 data)
return tag;
}

static inline qup_return_t qup_i2c_write_fifo_flush(blsp_qup_id_t id)
static inline qup_return_t qup_i2c_write_fifo_flush(blsp_qup_id_t id,
struct stopwatch *timeout)
{
qup_return_t ret = QUP_ERR_UNDEFINED;

qup_write32(QUP_ADDR(id, QUP_OPERATIONAL), OUTPUT_SERVICE_FLAG);

mdelay(4); /* TPM seems to need this */

ret = qup_fifo_wait_while(id, OUTPUT_FIFO_NOT_EMPTY);
ret = qup_fifo_wait_while(id, OUTPUT_FIFO_NOT_EMPTY, timeout);
if (ret)
return ret;

Expand All @@ -168,6 +168,7 @@ static qup_return_t qup_i2c_write_fifo(blsp_qup_id_t id, qup_data_t *p_tx_obj,
unsigned int data_len = p_tx_obj->p.iic.data_len;
unsigned int idx = 0;
uint32_t tag, *fifo = QUP_ADDR(id, QUP_OUTPUT_FIFO);
struct stopwatch timeout;

qup_reset_master_status(id);

Expand Down Expand Up @@ -196,6 +197,7 @@ static qup_return_t qup_i2c_write_fifo(blsp_qup_id_t id, qup_data_t *p_tx_obj,

qup_write32(fifo, tag);

stopwatch_init_usecs_expire(&timeout, CONFIG_I2C_TRANSFER_TIMEOUT_US);
while (data_len) {

tag = qup_i2c_create_output_tag(data_len == 1 && stop_seq,
Expand All @@ -213,15 +215,15 @@ static qup_return_t qup_i2c_write_fifo(blsp_qup_id_t id, qup_data_t *p_tx_obj,

qup_write32(fifo, tag);

ret = qup_i2c_write_fifo_flush(id);
ret = qup_i2c_write_fifo_flush(id, &timeout);

if (ret) {
printk(QUPDBG "%s: error\n", __func__);
return ret;
}
}

ret = qup_i2c_write_fifo_flush(id);
ret = qup_i2c_write_fifo_flush(id, &timeout);

qup_set_state(id, QUP_STATE_RESET);

Expand Down Expand Up @@ -285,6 +287,7 @@ static qup_return_t qup_i2c_read_fifo(blsp_qup_id_t id, qup_data_t *p_tx_obj)
unsigned int data_len = p_tx_obj->p.iic.data_len;
unsigned int idx = 0;
uint32_t *fifo = QUP_ADDR(id, QUP_OUTPUT_FIFO);
struct stopwatch timeout;

qup_reset_master_status(id);

Expand All @@ -303,13 +306,14 @@ static qup_return_t qup_i2c_read_fifo(blsp_qup_id_t id, qup_data_t *p_tx_obj)
(QUP_I2C_ADDR(addr) | QUP_I2C_SLAVE_READ)) |
((QUP_I2C_RECV_SEQ | data_len) << 16));

ret = qup_i2c_write_fifo_flush(id);
stopwatch_init_usecs_expire(&timeout, CONFIG_I2C_TRANSFER_TIMEOUT_US);
ret = qup_i2c_write_fifo_flush(id, &timeout);
if (ret) {
printk(QUPDBG "%s: OUTPUT_FIFO_NOT_EMPTY\n", __func__);
return ret;
}

ret = qup_fifo_wait_for(id, INPUT_SERVICE_FLAG);
ret = qup_fifo_wait_for(id, INPUT_SERVICE_FLAG, &timeout);
if (ret) {
printk(QUPDBG "%s: INPUT_SERVICE_FLAG\n", __func__);
return ret;
Expand Down
Loading

0 comments on commit 19baa9d

Please sign in to comment.