From 7647b39adc9ba85f8677bd9bea6903c248c665e7 Mon Sep 17 00:00:00 2001 From: Alexandre Bourdiol Date: Tue, 20 Aug 2019 17:23:15 +0200 Subject: [PATCH 1/3] TARGET_STM: I2C sequential communication revert PR #3324 to original cube HAL --- .../TARGET_STM/TARGET_STM32F0/device/stm32f0xx_hal_i2c.c | 4 ++-- .../TARGET_STM/TARGET_STM32F3/device/stm32f3xx_hal_i2c.c | 4 ++-- .../TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_i2c.c | 4 ++-- .../TARGET_STM/TARGET_STM32L0/device/stm32l0xx_hal_i2c.c | 4 ++-- .../TARGET_STM/TARGET_STM32L4/device/stm32l4xx_hal_i2c.c | 6 ++---- 5 files changed, 10 insertions(+), 12 deletions(-) diff --git a/targets/TARGET_STM/TARGET_STM32F0/device/stm32f0xx_hal_i2c.c b/targets/TARGET_STM/TARGET_STM32F0/device/stm32f0xx_hal_i2c.c index 225b519bdb8..610fcf34018 100644 --- a/targets/TARGET_STM/TARGET_STM32F0/device/stm32f0xx_hal_i2c.c +++ b/targets/TARGET_STM/TARGET_STM32F0/device/stm32f0xx_hal_i2c.c @@ -2592,7 +2592,7 @@ HAL_StatusTypeDef HAL_I2C_Master_Sequential_Transmit_IT(I2C_HandleTypeDef *hi2c, /* Prepare transfer parameters */ hi2c->pBuffPtr = pData; hi2c->XferCount = Size; - hi2c->XferOptions = (XferOptions & (~I2C_RELOAD_MODE)); + hi2c->XferOptions = XferOptions; hi2c->XferISR = I2C_Master_ISR_IT; /* If size > MAX_NBYTE_SIZE, use reload mode */ @@ -2658,7 +2658,7 @@ HAL_StatusTypeDef HAL_I2C_Master_Sequential_Receive_IT(I2C_HandleTypeDef *hi2c, /* Prepare transfer parameters */ hi2c->pBuffPtr = pData; hi2c->XferCount = Size; - hi2c->XferOptions = (XferOptions & (~I2C_RELOAD_MODE)); + hi2c->XferOptions = XferOptions; hi2c->XferISR = I2C_Master_ISR_IT; /* If hi2c->XferCount > MAX_NBYTE_SIZE, use reload mode */ diff --git a/targets/TARGET_STM/TARGET_STM32F3/device/stm32f3xx_hal_i2c.c b/targets/TARGET_STM/TARGET_STM32F3/device/stm32f3xx_hal_i2c.c index 66afaed8cc9..c721823a7e9 100644 --- a/targets/TARGET_STM/TARGET_STM32F3/device/stm32f3xx_hal_i2c.c +++ b/targets/TARGET_STM/TARGET_STM32F3/device/stm32f3xx_hal_i2c.c @@ -2592,7 +2592,7 @@ HAL_StatusTypeDef HAL_I2C_Master_Sequential_Transmit_IT(I2C_HandleTypeDef *hi2c, /* Prepare transfer parameters */ hi2c->pBuffPtr = pData; hi2c->XferCount = Size; - hi2c->XferOptions = (XferOptions & (~I2C_RELOAD_MODE)); // MBED patch + hi2c->XferOptions = XferOptions; hi2c->XferISR = I2C_Master_ISR_IT; /* If size > MAX_NBYTE_SIZE, use reload mode */ @@ -2666,7 +2666,7 @@ HAL_StatusTypeDef HAL_I2C_Master_Sequential_Receive_IT(I2C_HandleTypeDef *hi2c, /* Prepare transfer parameters */ hi2c->pBuffPtr = pData; hi2c->XferCount = Size; - hi2c->XferOptions = (XferOptions & (~I2C_RELOAD_MODE)); // MBED patch + hi2c->XferOptions = XferOptions; hi2c->XferISR = I2C_Master_ISR_IT; /* If hi2c->XferCount > MAX_NBYTE_SIZE, use reload mode */ diff --git a/targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_i2c.c b/targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_i2c.c index f79ec0aa566..ebfb11125a0 100644 --- a/targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_i2c.c +++ b/targets/TARGET_STM/TARGET_STM32F7/device/stm32f7xx_hal_i2c.c @@ -2598,7 +2598,7 @@ HAL_StatusTypeDef HAL_I2C_Master_Sequential_Transmit_IT(I2C_HandleTypeDef *hi2c, /* Prepare transfer parameters */ hi2c->pBuffPtr = pData; hi2c->XferCount = Size; - hi2c->XferOptions = (XferOptions & (~I2C_RELOAD_MODE)); // MBED: changed + hi2c->XferOptions = XferOptions; hi2c->XferISR = I2C_Master_ISR_IT; /* If size > MAX_NBYTE_SIZE, use reload mode */ @@ -2672,7 +2672,7 @@ HAL_StatusTypeDef HAL_I2C_Master_Sequential_Receive_IT(I2C_HandleTypeDef *hi2c, /* Prepare transfer parameters */ hi2c->pBuffPtr = pData; hi2c->XferCount = Size; - hi2c->XferOptions = (XferOptions & (~I2C_RELOAD_MODE)); // MBED: changed + hi2c->XferOptions = XferOptions; hi2c->XferISR = I2C_Master_ISR_IT; /* If hi2c->XferCount > MAX_NBYTE_SIZE, use reload mode */ diff --git a/targets/TARGET_STM/TARGET_STM32L0/device/stm32l0xx_hal_i2c.c b/targets/TARGET_STM/TARGET_STM32L0/device/stm32l0xx_hal_i2c.c index d99695721c3..0f6f34f1dcb 100644 --- a/targets/TARGET_STM/TARGET_STM32L0/device/stm32l0xx_hal_i2c.c +++ b/targets/TARGET_STM/TARGET_STM32L0/device/stm32l0xx_hal_i2c.c @@ -2592,7 +2592,7 @@ HAL_StatusTypeDef HAL_I2C_Master_Sequential_Transmit_IT(I2C_HandleTypeDef *hi2c, /* Prepare transfer parameters */ hi2c->pBuffPtr = pData; hi2c->XferCount = Size; - hi2c->XferOptions = (XferOptions & (~I2C_RELOAD_MODE)); // MBED commit 23926a2418 + hi2c->XferOptions = XferOptions; hi2c->XferISR = I2C_Master_ISR_IT; /* If size > MAX_NBYTE_SIZE, use reload mode */ @@ -2666,7 +2666,7 @@ HAL_StatusTypeDef HAL_I2C_Master_Sequential_Receive_IT(I2C_HandleTypeDef *hi2c, /* Prepare transfer parameters */ hi2c->pBuffPtr = pData; hi2c->XferCount = Size; - hi2c->XferOptions = (XferOptions & (~I2C_RELOAD_MODE)); // MBED commit 23926a2418 + hi2c->XferOptions = XferOptions; hi2c->XferISR = I2C_Master_ISR_IT; /* If hi2c->XferCount > MAX_NBYTE_SIZE, use reload mode */ diff --git a/targets/TARGET_STM/TARGET_STM32L4/device/stm32l4xx_hal_i2c.c b/targets/TARGET_STM/TARGET_STM32L4/device/stm32l4xx_hal_i2c.c index a070a6545e7..f296d5e57cc 100644 --- a/targets/TARGET_STM/TARGET_STM32L4/device/stm32l4xx_hal_i2c.c +++ b/targets/TARGET_STM/TARGET_STM32L4/device/stm32l4xx_hal_i2c.c @@ -3173,8 +3173,7 @@ HAL_StatusTypeDef HAL_I2C_Master_Seq_Transmit_IT(I2C_HandleTypeDef *hi2c, uint16 /* Prepare transfer parameters */ hi2c->pBuffPtr = pData; hi2c->XferCount = Size; - // Added for MBED PR #3324 - hi2c->XferOptions = (XferOptions & (~I2C_RELOAD_MODE)); + hi2c->XferOptions = XferOptions; hi2c->XferISR = I2C_Master_ISR_IT; /* If hi2c->XferCount > MAX_NBYTE_SIZE, use reload mode */ @@ -3420,8 +3419,7 @@ HAL_StatusTypeDef HAL_I2C_Master_Seq_Receive_IT(I2C_HandleTypeDef *hi2c, uint16_ /* Prepare transfer parameters */ hi2c->pBuffPtr = pData; hi2c->XferCount = Size; - // Added for MBED PR #3324 - hi2c->XferOptions = (XferOptions & (~I2C_RELOAD_MODE)); + hi2c->XferOptions = XferOptions; hi2c->XferISR = I2C_Master_ISR_IT; /* If hi2c->XferCount > MAX_NBYTE_SIZE, use reload mode */ From 7910de2f38149bb52886b77e61a65b86aafeb4bf Mon Sep 17 00:00:00 2001 From: Alexandre Bourdiol Date: Wed, 21 Aug 2019 09:17:19 +0200 Subject: [PATCH 2/3] TARGET_STM: Fix I2C sequential communication Keep former behaviour for I2C V1. For I2C V2: Use only I2C_FIRST_FRAME, I2C_FIRST_AND_LAST_FRAME and I2C_LAST_FRAME, thus we avoid using reload bit. Reload suppose the next frame would be in the same direction, but we have no guarranty about this. So we cannot use reload bit. Note: in case of 2 consecutive I2C_FIRST_FRAME, a restart is automatically generated only if there is direction change in the direction. --- targets/TARGET_STM/i2c_api.c | 54 ++++++++++++++++++++++++++++++++++-- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/targets/TARGET_STM/i2c_api.c b/targets/TARGET_STM/i2c_api.c index 142e01b52a1..16ea84776d1 100644 --- a/targets/TARGET_STM/i2c_api.c +++ b/targets/TARGET_STM/i2c_api.c @@ -40,6 +40,7 @@ #include "pinmap.h" #include "PeripheralPins.h" #include "i2c_device.h" // family specific defines +#include "mbed_error.h" #ifndef DEBUG_STDIO # define DEBUG_STDIO 0 @@ -760,7 +761,7 @@ int i2c_read(i2c_t *obj, int address, char *data, int length, int stop) I2C_HandleTypeDef *handle = &(obj_s->handle); int count = I2C_ERROR_BUS_BUSY, ret = 0; uint32_t timeout = 0; - +#if defined(I2C_IP_VERSION_V1) // Trick to remove compiler warning "left and right operands are identical" in some cases uint32_t op1 = I2C_FIRST_AND_LAST_FRAME; uint32_t op2 = I2C_LAST_FRAME; @@ -778,6 +779,18 @@ int i2c_read(i2c_t *obj, int address, char *data, int length, int stop) obj_s->XferOperation = I2C_NEXT_FRAME; } } +#elif defined(I2C_IP_VERSION_V2) + if ((obj_s->XferOperation == I2C_FIRST_FRAME) || (obj_s->XferOperation == I2C_FIRST_AND_LAST_FRAME) || (obj_s->XferOperation == I2C_LAST_FRAME)) { + if (stop) { + obj_s->XferOperation = I2C_FIRST_AND_LAST_FRAME; + } else { + obj_s->XferOperation = I2C_FIRST_FRAME; + } + } else { + // should not happend + error("I2C: abnormal case should not happend"); + } +#endif obj_s->event = 0; @@ -818,6 +831,7 @@ int i2c_write(i2c_t *obj, int address, const char *data, int length, int stop) int count = I2C_ERROR_BUS_BUSY, ret = 0; uint32_t timeout = 0; +#if defined(I2C_IP_VERSION_V1) // Trick to remove compiler warning "left and right operands are identical" in some cases uint32_t op1 = I2C_FIRST_AND_LAST_FRAME; uint32_t op2 = I2C_LAST_FRAME; @@ -835,6 +849,18 @@ int i2c_write(i2c_t *obj, int address, const char *data, int length, int stop) obj_s->XferOperation = I2C_NEXT_FRAME; } } +#elif defined(I2C_IP_VERSION_V2) + if ((obj_s->XferOperation == I2C_FIRST_FRAME) || (obj_s->XferOperation == I2C_FIRST_AND_LAST_FRAME) || (obj_s->XferOperation == I2C_LAST_FRAME)) { + if (stop) { + obj_s->XferOperation = I2C_FIRST_AND_LAST_FRAME; + } else { + obj_s->XferOperation = I2C_FIRST_FRAME; + } + } else { + // should not happend + error("I2C: abnormal case should not happend"); + } +#endif obj_s->event = 0; @@ -874,11 +900,19 @@ void HAL_I2C_MasterTxCpltCallback(I2C_HandleTypeDef *hi2c) #if DEVICE_I2C_ASYNCH /* Handle potential Tx/Rx use case */ if ((obj->tx_buff.length) && (obj->rx_buff.length)) { +#if defined(I2C_IP_VERSION_V1) if (obj_s->stop) { obj_s->XferOperation = I2C_LAST_FRAME; } else { obj_s->XferOperation = I2C_NEXT_FRAME; } +#elif defined(I2C_IP_VERSION_V2) + if (obj_s->stop) { + obj_s->XferOperation = I2C_FIRST_AND_LAST_FRAME; + } else { + obj_s->XferOperation = I2C_FIRST_FRAME; + } +#endif HAL_I2C_Master_Sequential_Receive_IT(hi2c, obj_s->address, (uint8_t *)obj->rx_buff.buffer, obj->rx_buff.length, obj_s->XferOperation); } else @@ -1143,6 +1177,7 @@ void i2c_transfer_asynch(i2c_t *obj, const void *tx, size_t tx_length, void *rx, /* Set operation step depending if stop sending required or not */ if ((tx_length && !rx_length) || (!tx_length && rx_length)) { +#if defined(I2C_IP_VERSION_V1) // Trick to remove compiler warning "left and right operands are identical" in some cases uint32_t op1 = I2C_FIRST_AND_LAST_FRAME; uint32_t op2 = I2C_LAST_FRAME; @@ -1160,7 +1195,18 @@ void i2c_transfer_asynch(i2c_t *obj, const void *tx, size_t tx_length, void *rx, obj_s->XferOperation = I2C_NEXT_FRAME; } } - +#elif defined(I2C_IP_VERSION_V2) + if ((obj_s->XferOperation == I2C_FIRST_FRAME) || (obj_s->XferOperation == I2C_FIRST_AND_LAST_FRAME) || (obj_s->XferOperation == I2C_LAST_FRAME)) { + if (stop) { + obj_s->XferOperation = I2C_FIRST_AND_LAST_FRAME; + } else { + obj_s->XferOperation = I2C_FIRST_FRAME; + } + } else { + // should not happend + error("I2C: abnormal case should not happend"); + } +#endif if (tx_length > 0) { HAL_I2C_Master_Sequential_Transmit_IT(handle, address, (uint8_t *)tx, tx_length, obj_s->XferOperation); } @@ -1169,6 +1215,7 @@ void i2c_transfer_asynch(i2c_t *obj, const void *tx, size_t tx_length, void *rx, } } else if (tx_length && rx_length) { /* Two steps operation, don't modify XferOperation, keep it for next step */ +#if defined(I2C_IP_VERSION_V1) // Trick to remove compiler warning "left and right operands are identical" in some cases uint32_t op1 = I2C_FIRST_AND_LAST_FRAME; uint32_t op2 = I2C_LAST_FRAME; @@ -1178,6 +1225,9 @@ void i2c_transfer_asynch(i2c_t *obj, const void *tx, size_t tx_length, void *rx, (obj_s->XferOperation == I2C_NEXT_FRAME)) { HAL_I2C_Master_Sequential_Transmit_IT(handle, address, (uint8_t *)tx, tx_length, I2C_NEXT_FRAME); } +#elif defined(I2C_IP_VERSION_V2) + HAL_I2C_Master_Sequential_Transmit_IT(handle, address, (uint8_t *)tx, tx_length, I2C_FIRST_FRAME); +#endif } } From de121a308ae6c5137365315a195505d42ac61429 Mon Sep 17 00:00:00 2001 From: Alexandre Bourdiol Date: Mon, 26 Aug 2019 14:00:08 +0200 Subject: [PATCH 3/3] Fix I2C issue with test mbed_hal_fpga_ci_test_shield On last case #5 there was a last unexpected read. It happened when stop condition was generated --- targets/TARGET_STM/i2c_api.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/targets/TARGET_STM/i2c_api.c b/targets/TARGET_STM/i2c_api.c index 16ea84776d1..995bda07cbf 100644 --- a/targets/TARGET_STM/i2c_api.c +++ b/targets/TARGET_STM/i2c_api.c @@ -598,8 +598,6 @@ int i2c_stop(i2c_t *obj) return 0; } #endif - // Disable reload mode - handle->Instance->CR2 &= (uint32_t)~I2C_CR2_RELOAD; // Ensure the transmission is started before sending a stop if ((handle->Instance->CR2 & (uint32_t)I2C_CR2_RD_WRN) == 0) { @@ -612,7 +610,7 @@ int i2c_stop(i2c_t *obj) } // Generate the STOP condition - handle->Instance->CR2 |= I2C_CR2_STOP; + handle->Instance->CR2 = I2C_CR2_STOP; timeout = FLAG_TIMEOUT; while (!__HAL_I2C_GET_FLAG(handle, I2C_FLAG_STOPF)) { @@ -665,9 +663,16 @@ int i2c_byte_read(i2c_t *obj, int last) } } - /* Enable reload mode as we don't know how many bytes will be sent */ - /* and set transfer size to 1 */ - tmpreg |= I2C_CR2_RELOAD | (I2C_CR2_NBYTES & (1 << 16)); + if (last) { + /* Disable Address Acknowledge */ + tmpreg = tmpreg & (~I2C_CR2_RELOAD); + tmpreg |= I2C_CR2_NACK | (I2C_CR2_NBYTES & (1 << 16)); + } else { + /* Enable reload mode as we don't know how many bytes will be sent */ + /* and set transfer size to 1 */ + tmpreg |= I2C_CR2_RELOAD | (I2C_CR2_NBYTES & (1 << 16)); + } + /* Set the prepared configuration */ handle->Instance->CR2 = tmpreg; @@ -681,11 +686,6 @@ int i2c_byte_read(i2c_t *obj, int last) /* Then Get Byte */ data = handle->Instance->RXDR; - if (last) { - /* Disable Address Acknowledge */ - handle->Instance->CR2 |= I2C_CR2_NACK; - } - return data; }