From ed2b6ea0a8f4d55a373af032faeca128c0741317 Mon Sep 17 00:00:00 2001 From: Andrew Leech Date: Thu, 18 Apr 2019 10:31:44 +1000 Subject: [PATCH] stm32/i2c: Make timeout for hardware I2C configurable. Previously the hardware I2C timeout was hard coded to 50ms which isn't guaranteed to be enough depending on the clock stretching specs of the I2C device(s) in use. This patch ensures the hardware I2C implementation honors the existing timeout argument passed to the machine.I2C constructor. The default timeout for software and hardware I2C is now 50ms. --- ports/stm32/accel.c | 4 +++- ports/stm32/i2c.c | 28 ++++++++++++++++++++-------- ports/stm32/i2c.h | 2 +- ports/stm32/machine_i2c.c | 18 ++++++++++-------- ports/stm32/mpconfigboard_common.h | 3 +++ 5 files changed, 37 insertions(+), 18 deletions(-) diff --git a/ports/stm32/accel.c b/ports/stm32/accel.c index 0d7708c09..73ddcf8cf 100644 --- a/ports/stm32/accel.c +++ b/ports/stm32/accel.c @@ -46,6 +46,8 @@ /// /// Raw values are between -32 and 31. +#define I2C_TIMEOUT_MS (50) + #define MMA_ADDR (76) #define MMA_REG_X (0) #define MMA_REG_Y (1) @@ -62,7 +64,7 @@ void accel_init(void) { STATIC void accel_start(void) { // start the I2C bus in master mode - i2c_init(I2C1, MICROPY_HW_I2C1_SCL, MICROPY_HW_I2C1_SDA, 400000); + i2c_init(I2C1, MICROPY_HW_I2C1_SCL, MICROPY_HW_I2C1_SDA, 400000, I2C_TIMEOUT_MS); // turn off AVDD, wait 30ms, turn on AVDD, wait 30ms again mp_hal_pin_low(MICROPY_HW_MMA_AVDD_PIN); // turn off diff --git a/ports/stm32/i2c.c b/ports/stm32/i2c.c index 109b9418f..06e26d912 100644 --- a/ports/stm32/i2c.c +++ b/ports/stm32/i2c.c @@ -30,11 +30,11 @@ #if MICROPY_HW_ENABLE_HW_I2C -#define I2C_POLL_TIMEOUT_MS (50) - #if defined(STM32F4) -int i2c_init(i2c_t *i2c, mp_hal_pin_obj_t scl, mp_hal_pin_obj_t sda, uint32_t freq) { +STATIC uint16_t i2c_timeout_ms[MICROPY_HW_MAX_I2C]; + +int i2c_init(i2c_t *i2c, mp_hal_pin_obj_t scl, mp_hal_pin_obj_t sda, uint32_t freq, uint16_t timeout_ms) { uint32_t i2c_id = ((uint32_t)i2c - I2C1_BASE) / (I2C2_BASE - I2C1_BASE); // Init pins @@ -45,6 +45,9 @@ int i2c_init(i2c_t *i2c, mp_hal_pin_obj_t scl, mp_hal_pin_obj_t sda, uint32_t fr return -MP_EPERM; } + // Save timeout value + i2c_timeout_ms[i2c_id] = timeout_ms; + // Force reset I2C peripheral RCC->APB1RSTR |= RCC_APB1RSTR_I2C1RST << i2c_id; RCC->APB1RSTR &= ~(RCC_APB1RSTR_I2C1RST << i2c_id); @@ -88,9 +91,10 @@ int i2c_init(i2c_t *i2c, mp_hal_pin_obj_t scl, mp_hal_pin_obj_t sda, uint32_t fr } STATIC int i2c_wait_sr1_set(i2c_t *i2c, uint32_t mask) { + uint32_t i2c_id = ((uint32_t)i2c - I2C1_BASE) / (I2C2_BASE - I2C1_BASE); uint32_t t0 = HAL_GetTick(); while (!(i2c->SR1 & mask)) { - if (HAL_GetTick() - t0 >= I2C_POLL_TIMEOUT_MS) { + if (HAL_GetTick() - t0 >= i2c_timeout_ms[i2c_id]) { i2c->CR1 &= ~I2C_CR1_PE; return -MP_ETIMEDOUT; } @@ -99,9 +103,10 @@ STATIC int i2c_wait_sr1_set(i2c_t *i2c, uint32_t mask) { } STATIC int i2c_wait_stop(i2c_t *i2c) { + uint32_t i2c_id = ((uint32_t)i2c - I2C1_BASE) / (I2C2_BASE - I2C1_BASE); uint32_t t0 = HAL_GetTick(); while (i2c->CR1 & I2C_CR1_STOP) { - if (HAL_GetTick() - t0 >= I2C_POLL_TIMEOUT_MS) { + if (HAL_GetTick() - t0 >= i2c_timeout_ms[i2c_id]) { i2c->CR1 &= ~I2C_CR1_PE; return -MP_ETIMEDOUT; } @@ -264,7 +269,9 @@ int i2c_write(i2c_t *i2c, const uint8_t *src, size_t len, size_t next_len) { #elif defined(STM32F0) || defined(STM32F7) -int i2c_init(i2c_t *i2c, mp_hal_pin_obj_t scl, mp_hal_pin_obj_t sda, uint32_t freq) { +STATIC uint16_t i2c_timeout_ms[MICROPY_HW_MAX_I2C]; + +int i2c_init(i2c_t *i2c, mp_hal_pin_obj_t scl, mp_hal_pin_obj_t sda, uint32_t freq, uint16_t timeout_ms) { uint32_t i2c_id = ((uint32_t)i2c - I2C1_BASE) / (I2C2_BASE - I2C1_BASE); // Init pins @@ -275,6 +282,9 @@ int i2c_init(i2c_t *i2c, mp_hal_pin_obj_t scl, mp_hal_pin_obj_t sda, uint32_t fr return -MP_EPERM; } + // Save timeout value + i2c_timeout_ms[i2c_id] = timeout_ms; + // Enable I2C peripheral clock RCC->APB1ENR |= RCC_APB1ENR_I2C1EN << i2c_id; volatile uint32_t tmp = RCC->APB1ENR; // delay after RCC clock enable @@ -303,9 +313,10 @@ int i2c_init(i2c_t *i2c, mp_hal_pin_obj_t scl, mp_hal_pin_obj_t sda, uint32_t fr } STATIC int i2c_wait_cr2_clear(i2c_t *i2c, uint32_t mask) { + uint32_t i2c_id = ((uint32_t)i2c - I2C1_BASE) / (I2C2_BASE - I2C1_BASE); uint32_t t0 = HAL_GetTick(); while (i2c->CR2 & mask) { - if (HAL_GetTick() - t0 >= I2C_POLL_TIMEOUT_MS) { + if (HAL_GetTick() - t0 >= i2c_timeout_ms[i2c_id]) { i2c->CR1 &= ~I2C_CR1_PE; return -MP_ETIMEDOUT; } @@ -314,9 +325,10 @@ STATIC int i2c_wait_cr2_clear(i2c_t *i2c, uint32_t mask) { } STATIC int i2c_wait_isr_set(i2c_t *i2c, uint32_t mask) { + uint32_t i2c_id = ((uint32_t)i2c - I2C1_BASE) / (I2C2_BASE - I2C1_BASE); uint32_t t0 = HAL_GetTick(); while (!(i2c->ISR & mask)) { - if (HAL_GetTick() - t0 >= I2C_POLL_TIMEOUT_MS) { + if (HAL_GetTick() - t0 >= i2c_timeout_ms[i2c_id]) { i2c->CR1 &= ~I2C_CR1_PE; return -MP_ETIMEDOUT; } diff --git a/ports/stm32/i2c.h b/ports/stm32/i2c.h index 5599f4123..4846f1cf3 100644 --- a/ports/stm32/i2c.h +++ b/ports/stm32/i2c.h @@ -55,7 +55,7 @@ void i2c_er_irq_handler(mp_uint_t i2c_id); typedef I2C_TypeDef i2c_t; -int i2c_init(i2c_t *i2c, mp_hal_pin_obj_t scl, mp_hal_pin_obj_t sda, uint32_t freq); +int i2c_init(i2c_t *i2c, mp_hal_pin_obj_t scl, mp_hal_pin_obj_t sda, uint32_t freq, uint16_t timeout); int i2c_start_addr(i2c_t *i2c, int rd_wrn, uint16_t addr, size_t len, bool stop); int i2c_read(i2c_t *i2c, uint8_t *dest, size_t len, size_t next_len); int i2c_write(i2c_t *i2c, const uint8_t *src, size_t len, size_t next_len); diff --git a/ports/stm32/machine_i2c.c b/ports/stm32/machine_i2c.c index 037401d86..5acf9c7d6 100644 --- a/ports/stm32/machine_i2c.c +++ b/ports/stm32/machine_i2c.c @@ -37,6 +37,8 @@ STATIC const mp_obj_type_t machine_hard_i2c_type; +#define I2C_POLL_DEFAULT_TIMEOUT_US (50000) // 50ms + #if defined(STM32F0) || defined(STM32F4) || defined(STM32F7) typedef struct _machine_hard_i2c_obj_t { @@ -104,9 +106,9 @@ STATIC void machine_hard_i2c_print(const mp_print_t *print, mp_obj_t self_in, mp #endif } -void machine_hard_i2c_init(machine_hard_i2c_obj_t *self, uint32_t freq, uint32_t timeout) { - (void)timeout; - i2c_init(self->i2c, self->scl, self->sda, freq); +void machine_hard_i2c_init(machine_hard_i2c_obj_t *self, uint32_t freq, uint32_t timeout_us) { + uint32_t timeout_ms = (timeout_us + 999) / 1000; + i2c_init(self->i2c, self->scl, self->sda, freq, timeout_ms); } int machine_hard_i2c_transfer(mp_obj_base_t *self_in, uint16_t addr, size_t n, mp_machine_i2c_buf_t *bufs, unsigned int flags) { @@ -147,22 +149,22 @@ typedef mp_machine_soft_i2c_obj_t machine_hard_i2c_obj_t; STATIC machine_hard_i2c_obj_t machine_hard_i2c_obj[] = { #if defined(MICROPY_HW_I2C1_SCL) - {{&machine_hard_i2c_type}, 1, 500, MICROPY_HW_I2C1_SCL, MICROPY_HW_I2C1_SDA}, + {{&machine_hard_i2c_type}, 1, I2C_POLL_DEFAULT_TIMEOUT_US, MICROPY_HW_I2C1_SCL, MICROPY_HW_I2C1_SDA}, #else {{NULL}, 0, 0, NULL, NULL}, #endif #if defined(MICROPY_HW_I2C2_SCL) - {{&machine_hard_i2c_type}, 1, 500, MICROPY_HW_I2C2_SCL, MICROPY_HW_I2C2_SDA}, + {{&machine_hard_i2c_type}, 1, I2C_POLL_DEFAULT_TIMEOUT_US, MICROPY_HW_I2C2_SCL, MICROPY_HW_I2C2_SDA}, #else {{NULL}, 0, 0, NULL, NULL}, #endif #if defined(MICROPY_HW_I2C3_SCL) - {{&machine_hard_i2c_type}, 1, 500, MICROPY_HW_I2C3_SCL, MICROPY_HW_I2C3_SDA}, + {{&machine_hard_i2c_type}, 1, I2C_POLL_DEFAULT_TIMEOUT_US, MICROPY_HW_I2C3_SCL, MICROPY_HW_I2C3_SDA}, #else {{NULL}, 0, 0, NULL, NULL}, #endif #if defined(MICROPY_HW_I2C4_SCL) - {{&machine_hard_i2c_type}, 1, 500, MICROPY_HW_I2C4_SCL, MICROPY_HW_I2C4_SDA}, + {{&machine_hard_i2c_type}, 1, I2C_POLL_DEFAULT_TIMEOUT_US, MICROPY_HW_I2C4_SCL, MICROPY_HW_I2C4_SDA}, #else {{NULL}, 0, 0, NULL, NULL}, #endif @@ -209,7 +211,7 @@ mp_obj_t machine_hard_i2c_make_new(const mp_obj_type_t *type, size_t n_args, siz { MP_QSTR_scl, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} }, { MP_QSTR_sda, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = MP_OBJ_NULL} }, { MP_QSTR_freq, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 400000} }, - { MP_QSTR_timeout, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 1000} }, + { MP_QSTR_timeout, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = I2C_POLL_DEFAULT_TIMEOUT_US} }, }; mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)]; mp_arg_parse_all_kw_array(n_args, n_kw, all_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args); diff --git a/ports/stm32/mpconfigboard_common.h b/ports/stm32/mpconfigboard_common.h index 4348f50bd..f3a3d48e7 100644 --- a/ports/stm32/mpconfigboard_common.h +++ b/ports/stm32/mpconfigboard_common.h @@ -138,6 +138,7 @@ #define MP_HAL_UNIQUE_ID_ADDRESS (0x1ffff7ac) #define PYB_EXTI_NUM_VECTORS (23) +#define MICROPY_HW_MAX_I2C (2) #define MICROPY_HW_MAX_TIMER (17) #define MICROPY_HW_MAX_UART (8) @@ -146,6 +147,7 @@ #define MP_HAL_UNIQUE_ID_ADDRESS (0x1fff7a10) #define PYB_EXTI_NUM_VECTORS (23) +#define MICROPY_HW_MAX_I2C (3) #define MICROPY_HW_MAX_TIMER (14) #if defined(UART10) #define MICROPY_HW_MAX_UART (10) @@ -169,6 +171,7 @@ #endif #define PYB_EXTI_NUM_VECTORS (24) +#define MICROPY_HW_MAX_I2C (4) #define MICROPY_HW_MAX_TIMER (17) #define MICROPY_HW_MAX_UART (8)