From b1cf7b79b3858a7860e8d73281fa6b40660f0420 Mon Sep 17 00:00:00 2001 From: Sergey Vlasov Date: Sun, 24 Oct 2021 23:44:33 +0300 Subject: [PATCH] Fix too small timeout in `i2c_start()` In #11930 the I2C master driver for AVR was changed to perform multiple retries (20 by default, configurable via `I2C_START_RETRY_COUNT`) in `i2c_start()`, apparently as a workaround for failures when the slave half had interrupts disabled for some time. Unfortunately, the implementation of those retries limited the minimum timeout for a single start attempt to 1 ms, but the timeout handling in the I2C master driver handles the partial millisecond before the next timer tick as if it was a full millisecond, therefore the timeout for a single attempt could be really short in some cases. These short timeouts resulted in some problems when various I2C APIs were invoked with timeouts not greater than 40 ms - e.g., #14935. As a minimal fix for this problem, limit the minimum timeout for a single start attempt to 2 ms instead of 1 (the actual timeout will be 1...2 ms, instead of 0...1 ms before the change). --- platforms/avr/drivers/i2c_master.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/platforms/avr/drivers/i2c_master.c b/platforms/avr/drivers/i2c_master.c index 2773e00778ef..620e40db21da 100644 --- a/platforms/avr/drivers/i2c_master.c +++ b/platforms/avr/drivers/i2c_master.c @@ -95,7 +95,7 @@ static i2c_status_t i2c_start_impl(uint8_t address, uint16_t timeout) { i2c_status_t i2c_start(uint8_t address, uint16_t timeout) { // Retry i2c_start_impl a bunch times in case the remote side has interrupts disabled. uint16_t timeout_timer = timer_read(); - uint16_t time_slice = MAX(1, (timeout == (I2C_TIMEOUT_INFINITE)) ? 5 : (timeout / (I2C_START_RETRY_COUNT))); // if it's infinite, wait 1ms between attempts, otherwise split up the entire timeout into the number of retries + uint16_t time_slice = MAX(2, (timeout == (I2C_TIMEOUT_INFINITE)) ? 5 : (timeout / (I2C_START_RETRY_COUNT))); // if it's infinite, wait 5ms between attempts, otherwise split up the entire timeout into the number of retries i2c_status_t status; do { status = i2c_start_impl(address, time_slice);