Skip to content

Commit

Permalink
Implemented timeout handler for I2C driver
Browse files Browse the repository at this point in the history 
Signed-off-by: Matt Rossouw <matthew.rossouw@unsw.edu.au>
  • Loading branch information
@omeh-a
omeh-a committed Sep 18, 2024
1 parent a0d575b commit 65ef3fb
Show file tree
Hide file tree
Showing 2 changed files with 130 additions and 82 deletions.
7 changes: 7 additions & 0 deletions drivers/i2c/meson/driver.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,11 +44,18 @@ typedef struct _i2c_ifState {
enum data_direction data_direction;
/* I2C bus address of the current request being handled */
size_t addr;

unsigned long timeout;
} i2c_ifState_t;

#define DATA_DIRECTION_WRITE (0x0)
#define DATA_DIRECTION_READ (0x1)

// Max number of timeout IRQs before giving up. For some devices such as the pn532,
// these arrive constantly in normal operation. As a result we need a high threshold
// before acting.
#define TIMEOUT_MAX 100000

// Ctl register fields
#define REG_CTRL_START (BIT(0))
#define REG_CTRL_ACK_IGNORE (BIT(1))
Expand Down
205 changes: 123 additions & 82 deletions drivers/i2c/meson/i2c.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,38 @@ static inline uint8_t i2c_token_convert(i2c_token_t token)
}
}

/**
* Return current buffer to sender and reset interface state for next transaction.
*/
static inline void i2c_return_and_reset(void)
{
volatile struct i2c_regs *regs = (volatile struct i2c_regs *)i2c_regs;
LOG_DRIVER("returning to virtualiser!\n");
LOG_DRIVER("curr_response_len : 0x%lx, curr_request_len : 0x%lx, return address is 0x%lx\n",
i2c_ifState.curr_response_len, i2c_ifState.curr_request_len, i2c_ifState.addr);
LOG_DRIVER("enqueueing response with size: %d\n\n", i2c_ifState.curr_response_len + RESPONSE_DATA_OFFSET);

// response length is + 2 (RESPONSE_DATA_OFFSET = 2) because of the error tokens at the start
int ret = i2c_enqueue_response(queue_handle, i2c_ifState.addr,
(size_t) i2c_ifState.curr_data - DATA_REGIONS_START,
i2c_ifState.curr_response_len + RESPONSE_DATA_OFFSET);
if (ret) {
LOG_DRIVER_ERR("Failed to enqueue response\n");
}

// reset driver state for another request
// load tokens resets the interface registers so no need here
i2c_ifState.curr_response_len = 0;
i2c_ifState.curr_data = NULL;
i2c_ifState.curr_request_len = 0;
i2c_ifState.remaining = 0;
i2c_ifState.addr = 0;

microkit_notify(VIRTUALISER_CH);

i2c_halt(regs); // stop condition
}

/**
* Loads tokens onto specified i2c interface registers
* This function resets interface registers before uploading data
Expand All @@ -390,6 +422,9 @@ static inline void i2c_load_tokens(volatile struct i2c_regs *regs)
regs->wdata0 = 0x0;
regs->wdata1 = 0x0;

// Reset timeout counter
i2c_ifState.timeout = 0;

// Offset into token list registers
uint32_t tk_offset = 0;

Expand Down Expand Up @@ -455,8 +490,6 @@ static inline void i2c_load_tokens(volatile struct i2c_regs *regs)
i2c_ifState.rw_remaining--;
}



LOG_DRIVER("meson_token: 0x%lx, request_data_offset : 0x%lx, tk_offset: 0x%lx, wdata_offset: 0x%lx, rdata_offset: 0x%lx\n",
meson_token, request_data_offset, tk_offset, wdata_offset, rdata_offset);

Expand Down Expand Up @@ -514,6 +547,8 @@ void init(void)
i2c_ifState.remaining = 0;
i2c_ifState.notified = 0;
i2c_ifState.addr = 0;
i2c_ifState.rw_remaining = 0;
i2c_ifState.timeout = 0;

microkit_dbg_puts("Driver initialised.\n");
}
Expand Down Expand Up @@ -557,6 +592,7 @@ static inline void handle_request(void)
i2c_ifState.curr_request_len = size;
i2c_ifState.remaining = size;
i2c_ifState.notified = 0;
i2c_ifState.rw_remaining = 0;

i2c_load_tokens(regs);
} else {
Expand All @@ -566,102 +602,107 @@ static inline void handle_request(void)
}
}

/* Right now we do not do anything to handle a response timeout, even though we should
* be. More investigation is required.
* We observed very frequent timeout IRQs in our example system using an I2C card reader,
* we believe the source of this problem is a misconfiguration with the clocks, and will
* hopefully be resovled once we have an actual clock driver.
* See https://github.com/au-ts/sddf/issues/212 for more details.
*/
static void handle_response_timeout(void)
{
LOG_DRIVER("handling timeout IRQ\n");
return;
}
volatile struct i2c_regs *regs = (volatile struct i2c_regs *)i2c_regs;

/* A timeout IRQ notionally occurs when a device has successfully ACKed a transaction
* but stops responding sometime thereafter while we are awaiting an R/W operation.
* The danger is that the device has not actually died and partially receives the
* transaction and we give up on it too quickly, leaving the device in a potentially
* incoherent state.
*
* Unfortunately, the datasheet describes nothing about this IRQ so it is hard to reason
* about the true conditions that make it occur. It happens often, so we opt to just wait
* until a very large number of IRQs have arrived before bailing.
*
* The timeout counter is reset to 0 upon every successful token load to ensure we only bail
* out when a huge amount of time is spent on a single batch.
*/

static void handle_response(void)
{
LOG_DRIVER("handling transfer complete IRQ\n");
volatile struct i2c_regs *regs = (volatile struct i2c_regs *)i2c_regs;
// IRQs still arrive when nothing is happening!
// @mattr: since the behaviour of this IRQ is unknown, this handler may be a vector for
// bugs in the future. If you ever find transactions being terminated with no
// clear electrical reason, inspect this!
if (i2c_ifState.timeout >= TIMEOUT_MAX) {
i2c_halt(regs);

i2c_dump(regs);
// Get state
i2c_token_t *return_buffer = i2c_ifState.curr_data;

// Get result
uint8_t curr_token = 0;
uint8_t bytes_read = 0;
bool write_error = i2c_get_error(regs, &bytes_read, &curr_token);

// Prepare to extract data from the interface.
// INVARIANT: request data is always smaller than returned data to allow
// reuse.
i2c_token_t *return_buffer = i2c_ifState.curr_data;

// If there was an error, cancel the rest of this transaction and load the
// error information into the return buffer.
if (write_error) {
LOG_DRIVER("error!\n");
// handle_response_timeout already has error logic for timeout
if (curr_token == I2C_TOKEN_ADDR_READ) {
return_buffer[RESPONSE_ERR] = I2C_ERR_NOREAD;
LOG_DRIVER("I2C_ERR_NOREAD!\n");
} else {
return_buffer[RESPONSE_ERR] = I2C_ERR_NACK;
LOG_DRIVER("I2C_ERR_NACK!\n");
}
return_buffer[RESPONSE_ERR] = I2C_ERR_TIMEOUT;
// Token that caused the error
return_buffer[RESPONSE_ERR_TOKEN] = curr_token;
LOG_DRIVER("token that caused error: %d!\n", curr_token);

i2c_return_and_reset();
sddf_printf("I2C | %lu timeouts occured for this token batch! Aborting.\n");
}
}


static void handle_response(void)
{
LOG_DRIVER("handling transfer complete IRQ\n");
volatile struct i2c_regs *regs = (volatile struct i2c_regs *)i2c_regs;
if (!i2c_ifState.curr_data) {
LOG_DRIVER("Bogus transfer complete - ignoring!\n");
return;
} else {
// Get bytes_read amount of read data

// Copy data into return buffer
for (int i = 0; i < bytes_read; i++) {
size_t index = RESPONSE_DATA_OFFSET + i2c_ifState.curr_response_len;
if (i < 4) {
uint8_t value = (regs->rdata0 >> (i * 8)) & 0xFF;
return_buffer[index] = value;
LOG_DRIVER("loading into return_buffer at %d value 0x%lx\n", index, value);
} else if (i < 8) {
uint8_t value = (regs->rdata1 >> ((i - 4) * 8)) & 0xFF;
return_buffer[index] = value;
LOG_DRIVER("loading into return_buffer at %d value 0x%lx\n", index, value);
i2c_dump(regs);

// Get result
uint8_t curr_token = 0;
uint8_t bytes_read = 0;
bool write_error = i2c_get_error(regs, &bytes_read, &curr_token);

// Prepare to extract data from the interface.
// INVARIANT: request data is always smaller than returned data to allow
// reuse.
i2c_token_t *return_buffer = i2c_ifState.curr_data;

// If there was an error, cancel the rest of this transaction and load the
// error information into the return buffer.
if (write_error) {
LOG_DRIVER("error!\n");
// handle_response_timeout already has error logic for timeout
if (curr_token == I2C_TOKEN_ADDR_READ) {
return_buffer[RESPONSE_ERR] = I2C_ERR_NOREAD;
LOG_DRIVER("I2C_ERR_NOREAD!\n");
} else {
return_buffer[RESPONSE_ERR] = I2C_ERR_NACK;
LOG_DRIVER("I2C_ERR_NACK!\n");
}
i2c_ifState.curr_response_len++;
}
// Token that caused the error
return_buffer[RESPONSE_ERR_TOKEN] = curr_token;
LOG_DRIVER("token that caused error: %d!\n", curr_token);

LOG_DRIVER("I2C_ERR_OK\n");
return_buffer[RESPONSE_ERR] = I2C_ERR_OK;
// Token that caused error (could be anything since we set error code to no error anyway)
return_buffer[RESPONSE_ERR_TOKEN] = 0x0;
}
} else {
// Copy data into return buffer
for (int i = 0; i < bytes_read; i++) {
size_t index = RESPONSE_DATA_OFFSET + i2c_ifState.curr_response_len;
if (i < 4) {
uint8_t value = (regs->rdata0 >> (i * 8)) & 0xFF;
return_buffer[index] = value;
LOG_DRIVER("loading into return_buffer at %d value 0x%lx\n", index, value);
} else if (i < 8) {
uint8_t value = (regs->rdata1 >> ((i - 4) * 8)) & 0xFF;
return_buffer[index] = value;
LOG_DRIVER("loading into return_buffer at %d value 0x%lx\n", index, value);
}
i2c_ifState.curr_response_len++;
}

// If request is completed or there was an error, return data to server and notify.
if (write_error || !i2c_ifState.remaining) {
LOG_DRIVER("request completed or error, hence returning response to server\n");
LOG_DRIVER("curr_response_len : 0x%lx, curr_request_len : 0x%lx, return address is 0x%lx\n",
i2c_ifState.curr_response_len, i2c_ifState.curr_request_len, i2c_ifState.addr);
LOG_DRIVER("enguing response with size: %d\n\n", i2c_ifState.curr_response_len + RESPONSE_DATA_OFFSET);
// response length is + 2 (RESPONSE_DATA_OFFSET = 2) because of the error tokens at the start
int ret = i2c_enqueue_response(queue_handle, i2c_ifState.addr,
(size_t) i2c_ifState.curr_data - DATA_REGIONS_START,
i2c_ifState.curr_response_len + RESPONSE_DATA_OFFSET);
if (ret) {
LOG_DRIVER_ERR("Failed to enqueue response\n");
LOG_DRIVER("I2C_ERR_OK\n");
return_buffer[RESPONSE_ERR] = I2C_ERR_OK;
// Token that caused error (could be anything since we set error code to no error anyway)
return_buffer[RESPONSE_ERR_TOKEN] = 0x0;
}

// reset driver state for another request
// load tokens resets the interface registers so no need here
i2c_ifState.curr_response_len = 0;
i2c_ifState.curr_data = NULL;
i2c_ifState.curr_request_len = 0;
i2c_ifState.remaining = 0;
i2c_ifState.addr = 0;

microkit_notify(VIRTUALISER_CH);

i2c_halt(regs); // stop condition
// If request is completed or there was an error, return data to server and notify.
if (write_error || !i2c_ifState.remaining) {
i2c_return_and_reset();
}
}

// If the driver was notified while this transaction was in progress, immediately start working on the next one.
Expand Down

0 comments on commit 65ef3fb

Please sign in to comment.