From 23aadba528857ad70c22ea176d1ca0174b40406e Mon Sep 17 00:00:00 2001 From: Daniel Kao Date: Fri, 20 May 2022 22:08:36 -0700 Subject: [PATCH 1/2] avr i2c_master: Fix 1ms timeout i2c_start() produces a minimum time_slice of 1ms for use as timeout value. The timer granularity is 1ms, it is entirely possible for timer_count to tick up immediately after the last timer read and falsely trigger timeout with a '>= 1' comparison. --- platforms/avr/drivers/i2c_master.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/platforms/avr/drivers/i2c_master.c b/platforms/avr/drivers/i2c_master.c index c1a7b5f72d9d..2bb35647f4b4 100644 --- a/platforms/avr/drivers/i2c_master.c +++ b/platforms/avr/drivers/i2c_master.c @@ -64,7 +64,7 @@ static i2c_status_t i2c_start_impl(uint8_t address, uint16_t timeout) { uint16_t timeout_timer = timer_read(); while (!(TWCR & (1 << TWINT))) { - if ((timeout != I2C_TIMEOUT_INFINITE) && ((timer_read() - timeout_timer) >= timeout)) { + if ((timeout != I2C_TIMEOUT_INFINITE) && ((timer_read() - timeout_timer) > timeout)) { return I2C_STATUS_TIMEOUT; } } @@ -81,7 +81,7 @@ static i2c_status_t i2c_start_impl(uint8_t address, uint16_t timeout) { timeout_timer = timer_read(); while (!(TWCR & (1 << TWINT))) { - if ((timeout != I2C_TIMEOUT_INFINITE) && ((timer_read() - timeout_timer) >= timeout)) { + if ((timeout != I2C_TIMEOUT_INFINITE) && ((timer_read() - timeout_timer) > timeout)) { return I2C_STATUS_TIMEOUT; } } @@ -102,7 +102,7 @@ i2c_status_t i2c_start(uint8_t address, uint16_t timeout) { i2c_status_t status; do { status = i2c_start_impl(address, time_slice); - } while ((status < 0) && ((timeout == I2C_TIMEOUT_INFINITE) || (timer_elapsed(timeout_timer) < timeout))); + } while ((status < 0) && ((timeout == I2C_TIMEOUT_INFINITE) || (timer_elapsed(timeout_timer) <= timeout))); return status; } @@ -114,7 +114,7 @@ i2c_status_t i2c_write(uint8_t data, uint16_t timeout) { uint16_t timeout_timer = timer_read(); while (!(TWCR & (1 << TWINT))) { - if ((timeout != I2C_TIMEOUT_INFINITE) && ((timer_read() - timeout_timer) >= timeout)) { + if ((timeout != I2C_TIMEOUT_INFINITE) && ((timer_read() - timeout_timer) > timeout)) { return I2C_STATUS_TIMEOUT; } } @@ -132,7 +132,7 @@ int16_t i2c_read_ack(uint16_t timeout) { uint16_t timeout_timer = timer_read(); while (!(TWCR & (1 << TWINT))) { - if ((timeout != I2C_TIMEOUT_INFINITE) && ((timer_read() - timeout_timer) >= timeout)) { + if ((timeout != I2C_TIMEOUT_INFINITE) && ((timer_read() - timeout_timer) > timeout)) { return I2C_STATUS_TIMEOUT; } } @@ -147,7 +147,7 @@ int16_t i2c_read_nack(uint16_t timeout) { uint16_t timeout_timer = timer_read(); while (!(TWCR & (1 << TWINT))) { - if ((timeout != I2C_TIMEOUT_INFINITE) && ((timer_read() - timeout_timer) >= timeout)) { + if ((timeout != I2C_TIMEOUT_INFINITE) && ((timer_read() - timeout_timer) > timeout)) { return I2C_STATUS_TIMEOUT; } } From 90fed55d60e17f4da453f44f3af451328aaa42e6 Mon Sep 17 00:00:00 2001 From: Daniel Kao Date: Tue, 21 Jun 2022 14:27:56 -0700 Subject: [PATCH 2/2] avr/drivers/i2c_master: Use timer_elapsed() --- platforms/avr/drivers/i2c_master.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/platforms/avr/drivers/i2c_master.c b/platforms/avr/drivers/i2c_master.c index 2bb35647f4b4..524494c99d79 100644 --- a/platforms/avr/drivers/i2c_master.c +++ b/platforms/avr/drivers/i2c_master.c @@ -64,7 +64,7 @@ static i2c_status_t i2c_start_impl(uint8_t address, uint16_t timeout) { uint16_t timeout_timer = timer_read(); while (!(TWCR & (1 << TWINT))) { - if ((timeout != I2C_TIMEOUT_INFINITE) && ((timer_read() - timeout_timer) > timeout)) { + if ((timeout != I2C_TIMEOUT_INFINITE) && (timer_elapsed(timeout_timer) > timeout)) { return I2C_STATUS_TIMEOUT; } } @@ -81,7 +81,7 @@ static i2c_status_t i2c_start_impl(uint8_t address, uint16_t timeout) { timeout_timer = timer_read(); while (!(TWCR & (1 << TWINT))) { - if ((timeout != I2C_TIMEOUT_INFINITE) && ((timer_read() - timeout_timer) > timeout)) { + if ((timeout != I2C_TIMEOUT_INFINITE) && (timer_elapsed(timeout_timer) > timeout)) { return I2C_STATUS_TIMEOUT; } } @@ -114,7 +114,7 @@ i2c_status_t i2c_write(uint8_t data, uint16_t timeout) { uint16_t timeout_timer = timer_read(); while (!(TWCR & (1 << TWINT))) { - if ((timeout != I2C_TIMEOUT_INFINITE) && ((timer_read() - timeout_timer) > timeout)) { + if ((timeout != I2C_TIMEOUT_INFINITE) && (timer_elapsed(timeout_timer) > timeout)) { return I2C_STATUS_TIMEOUT; } } @@ -132,7 +132,7 @@ int16_t i2c_read_ack(uint16_t timeout) { uint16_t timeout_timer = timer_read(); while (!(TWCR & (1 << TWINT))) { - if ((timeout != I2C_TIMEOUT_INFINITE) && ((timer_read() - timeout_timer) > timeout)) { + if ((timeout != I2C_TIMEOUT_INFINITE) && (timer_elapsed(timeout_timer) > timeout)) { return I2C_STATUS_TIMEOUT; } } @@ -147,7 +147,7 @@ int16_t i2c_read_nack(uint16_t timeout) { uint16_t timeout_timer = timer_read(); while (!(TWCR & (1 << TWINT))) { - if ((timeout != I2C_TIMEOUT_INFINITE) && ((timer_read() - timeout_timer) > timeout)) { + if ((timeout != I2C_TIMEOUT_INFINITE) && (timer_elapsed(timeout_timer) > timeout)) { return I2C_STATUS_TIMEOUT; } }