Skip to content

I2C IT mode #135

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from Nov 7, 2017
Merged

I2C IT mode #135

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c78f4fa
Revert "Revert "Use interrupt mode for I2C transfers""
Oct 17, 2017
acc2e60
Update I2C files to work properly with IT
Oct 17, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions cores/arduino/stm32/stm32_def.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@

// Here define some compatibility
#ifdef STM32F0xx
#define I2C1_EV_IRQn I2C1_IRQn

#elif defined(STM32F1xx)

Expand All @@ -65,8 +64,7 @@
#elif defined(STM32F7xx)

#elif defined(STM32L0xx)
#define I2C1_EV_IRQn I2C1_IRQn
#define I2C2_EV_IRQn I2C2_IRQn

#elif defined(STM32L1xx)

#elif defined(STM32L4xx)
Expand Down
178 changes: 119 additions & 59 deletions cores/arduino/stm32/twi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,16 @@
*/

/* Family specific description for I2C */
#define I2C_NUM (5)
#if defined(STM32F7xx) || defined(STM32L4xx)
#define I2C_NUM (4)
#elif defined(STM32F2xx) || defined(STM32F3xx) || defined(STM32F4xx) || defined(STM32L0xx)
#define I2C_NUM (3)
#elif defined(STM32F0xx) || defined(STM32F1xx) || defined(STM32L1xx)
#define I2C_NUM (2)
#else
#error "Unknown Family - unknown I2C_NUM"
#endif

static I2C_HandleTypeDef* i2c_handles[I2C_NUM];

/**
Expand Down Expand Up @@ -142,7 +151,6 @@ void i2c_init(i2c_t *obj)
*/
void i2c_custom_init(i2c_t *obj, i2c_timing_e timing, uint32_t addressingMode, uint32_t ownAddress, uint8_t master)
{
UNUSED(master);
if(obj == NULL)
return;

Expand Down Expand Up @@ -170,53 +178,56 @@ void i2c_custom_init(i2c_t *obj, i2c_timing_e timing, uint32_t addressingMode, u
#if defined I2C1_BASE
// Enable I2C1 clock if not done
if (obj->i2c == I2C1) {
__HAL_RCC_I2C1_CLK_ENABLE();
__HAL_RCC_I2C1_FORCE_RESET();
__HAL_RCC_I2C1_RELEASE_RESET();
obj->irq = I2C1_EV_IRQn;
#ifdef STM32F1xx
obj->irqER = I2C1_ER_IRQn;
#endif
i2c_handles[0] = handle;
__HAL_RCC_I2C1_CLK_ENABLE();
__HAL_RCC_I2C1_FORCE_RESET();
__HAL_RCC_I2C1_RELEASE_RESET();

obj->irq = I2C1_EV_IRQn;
#if !defined(STM32F0xx) && !defined(STM32L0xx)
obj->irqER = I2C1_ER_IRQn;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one new proposal : in every place where I2Cx_ER_IRQn is being used couldn't you use
#if defined (I2C1_ER_IRQn)
obj->irqER = I2C1_ER_IRQn;
#endif
this would be more future proof I think

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I2C1_ER_IRQn is a enum value this could not be done.

#endif // !defined(STM32F0xx) && !defined(STM32L0xx)
i2c_handles[0] = handle;
}
#endif
#endif // I2C1_BASE
#if defined I2C2_BASE
// Enable I2C2 clock if not done
if (obj->i2c == I2C2) {
__HAL_RCC_I2C2_CLK_ENABLE();
__HAL_RCC_I2C2_FORCE_RESET();
__HAL_RCC_I2C2_RELEASE_RESET();
#ifdef STM32F0xx
obj->irq = I2C2_IRQn;
#else
obj->irq = I2C2_EV_IRQn;
#ifdef STM32F1xx
obj->irqER = I2C2_ER_IRQn;
#endif
#endif
i2c_handles[1] = handle;
__HAL_RCC_I2C2_CLK_ENABLE();
__HAL_RCC_I2C2_FORCE_RESET();
__HAL_RCC_I2C2_RELEASE_RESET();
obj->irq = I2C2_EV_IRQn;
#if !defined(STM32F0xx) && !defined(STM32L0xx)
obj->irqER = I2C2_ER_IRQn;
#endif // !defined(STM32F0xx) && !defined(STM32L0xx)
i2c_handles[1] = handle;
}
#endif
#endif // I2C2_BASE
#if defined I2C3_BASE
// Enable I2C3 clock if not done
if (obj->i2c == I2C3) {
__HAL_RCC_I2C3_CLK_ENABLE();
__HAL_RCC_I2C3_FORCE_RESET();
__HAL_RCC_I2C3_RELEASE_RESET();
obj->irq = I2C3_EV_IRQn;
i2c_handles[2] = handle;
__HAL_RCC_I2C3_CLK_ENABLE();
__HAL_RCC_I2C3_FORCE_RESET();
__HAL_RCC_I2C3_RELEASE_RESET();
obj->irq = I2C3_EV_IRQn;
#if !defined(STM32F0xx) && !defined(STM32L0xx)
obj->irqER = I2C3_ER_IRQn;
#endif // !defined(STM32F0xx) && !defined(STM32L0xx)
i2c_handles[2] = handle;
}
#endif
#endif // I2C3_BASE
#if defined I2C4_BASE
// Enable I2C3 clock if not done
// Enable I2C4 clock if not done
if (obj->i2c == I2C4) {
__HAL_RCC_I2C4_CLK_ENABLE();
__HAL_RCC_I2C4_FORCE_RESET();
__HAL_RCC_I2C4_RELEASE_RESET();
obj->irq = I2C4_EV_IRQn;
i2c_handles[3] = handle;
__HAL_RCC_I2C4_CLK_ENABLE();
__HAL_RCC_I2C4_FORCE_RESET();
__HAL_RCC_I2C4_RELEASE_RESET();
obj->irq = I2C4_EV_IRQn;
#if !defined(STM32F0xx) && !defined(STM32L0xx)
obj->irqER = I2C4_ER_IRQn;
#endif // !defined(STM32F0xx) && !defined(STM32L0xx)
i2c_handles[3] = handle;
}
#endif
#endif // I2C4_BASE

//SCL
port = set_GPIO_Port_Clock(STM_PORT(obj->scl));
Expand Down Expand Up @@ -250,7 +261,7 @@ void i2c_custom_init(i2c_t *obj, i2c_timing_e timing, uint32_t addressingMode, u
handle->Init.Timing = timing;
#else
handle->Init.ClockSpeed = timing;
handle->Init.DutyCycle = I2C_DUTYCYCLE_2;//16_9;
handle->Init.DutyCycle = I2C_DUTYCYCLE_2;
#endif
handle->Init.OwnAddress1 = ownAddress;
handle->Init.OwnAddress2 = 0xFF;
Expand All @@ -261,17 +272,17 @@ void i2c_custom_init(i2c_t *obj, i2c_timing_e timing, uint32_t addressingMode, u

handle->State = HAL_I2C_STATE_RESET;

if(master == 0) {
HAL_NVIC_SetPriority(obj->irq, 0, 1);
HAL_NVIC_EnableIRQ(obj->irq);
#ifdef STM32F1xx
HAL_NVIC_SetPriority(obj->irqER, 0, 1);
HAL_NVIC_EnableIRQ(obj->irqER);
#endif
}
HAL_NVIC_SetPriority(obj->irq, 0, 1);
HAL_NVIC_EnableIRQ(obj->irq);
#if !defined(STM32F0xx) && !defined(STM32L0xx)
HAL_NVIC_SetPriority(obj->irqER, 0, 1);
HAL_NVIC_EnableIRQ(obj->irqER);
#endif // !defined(STM32F0xx) && !defined(STM32L0xx)

// Init the I2C
HAL_I2C_Init(handle);

obj->isMaster = master;
}

/**
Expand All @@ -282,9 +293,9 @@ void i2c_custom_init(i2c_t *obj, i2c_timing_e timing, uint32_t addressingMode, u
void i2c_deinit(i2c_t *obj)
{
HAL_NVIC_DisableIRQ(obj->irq);
#ifdef STM32F1xx
#if !defined(STM32F0xx) && !defined(STM32L0xx)
HAL_NVIC_DisableIRQ(obj->irqER);
#endif
#endif // !defined(STM32F0xx) && !defined(STM32L0xx)
HAL_I2C_DeInit(&(obj->handle));
}

Expand Down Expand Up @@ -341,17 +352,20 @@ i2c_status_e i2c_master_write(i2c_t *obj, uint8_t dev_address,

{
i2c_status_e ret = I2C_ERROR;
HAL_StatusTypeDef status = HAL_OK;
uint32_t tickstart = HAL_GetTick();

// Check the communication status
status = HAL_I2C_Master_Transmit(&(obj->handle), dev_address, data, size, I2C_TIMEOUT_TICK);

if(status == HAL_OK)
if(HAL_I2C_Master_Transmit_IT(&(obj->handle), dev_address, data, size) == HAL_OK){
ret = I2C_OK;
else if(status == HAL_TIMEOUT)
ret = I2C_TIMEOUT;
else
ret = I2C_ERROR;
// wait for transfer completion
while((HAL_I2C_GetState(&(obj->handle)) != HAL_I2C_STATE_READY)
&& (ret == I2C_OK)){
if((HAL_GetTick() - tickstart) > I2C_TIMEOUT_TICK) {
ret = I2C_TIMEOUT;
} else if(HAL_I2C_GetError(&(obj->handle)) != HAL_I2C_ERROR_NONE) {
ret = I2C_ERROR;
}
}
}

return ret;
}
Expand Down Expand Up @@ -385,9 +399,19 @@ void i2c_slave_write_IT(i2c_t *obj, uint8_t *data, uint8_t size)
i2c_status_e i2c_master_read(i2c_t *obj, uint8_t dev_address, uint8_t *data, uint8_t size)
{
i2c_status_e ret = I2C_ERROR;
uint32_t tickstart = HAL_GetTick();

if(HAL_I2C_Master_Receive(&(obj->handle), dev_address, data, size, I2C_TIMEOUT_TICK) == HAL_OK) {
if(HAL_I2C_Master_Receive_IT(&(obj->handle), dev_address, data, size) == HAL_OK) {
ret = I2C_OK;
// wait for transfer completion
while((HAL_I2C_GetState(&(obj->handle)) != HAL_I2C_STATE_READY)
&& (ret == I2C_OK)){
if((HAL_GetTick() - tickstart) > I2C_TIMEOUT_TICK) {
ret = I2C_TIMEOUT;
} else if(HAL_I2C_GetError(&(obj->handle)) != HAL_I2C_ERROR_NONE) {
ret = I2C_ERROR;
}
}
}

return ret;
Expand Down Expand Up @@ -506,15 +530,24 @@ void HAL_I2C_ListenCpltCallback(I2C_HandleTypeDef *hi2c)

/**
* @brief I2C error callback.
* @note In master mode, the callback is not used because the error is reported
* to the Arduino API from i2c_master_write() and i2c_master_read().
* In slave mode, there is no mechanism in Arduino API to report an error
* so the error callback forces the slave to listen again.
* @param hi2c Pointer to a I2C_HandleTypeDef structure that contains
* the configuration information for the specified I2C.
* @retval None
*/
void HAL_I2C_ErrorCallback(I2C_HandleTypeDef *hi2c)
{
HAL_I2C_EnableListen_IT(hi2c);
i2c_t *obj = get_i2c_obj(hi2c);

if(obj->isMaster == 0) {
HAL_I2C_EnableListen_IT(hi2c);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you comment or explain this ErrorCallback management ?
If there is an error, shouldn't we report the error to application ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In master mode, the callback is useless because the error is reported to the Arduino API with i2c_master_write() and i2c_master_read().
In slave mode, there is no mechanism in Arduino API to report an error so the error callback forces the slave to listen again.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok Good. So please explain this either with comments in the code, or in the commit message, this is useful information that we cannot guess when reading the code

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Copy link
Member

@LMESTM LMESTM Oct 17, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok - good - I see it now !

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Above in the header of the function.

}
}

#if defined(I2C1_BASE)
/**
* @brief This function handles I2C1 interrupt.
* @param None
Expand All @@ -524,8 +557,12 @@ void I2C1_EV_IRQHandler(void)
{
I2C_HandleTypeDef * handle = i2c_handles[0];
HAL_I2C_EV_IRQHandler(handle);
#if defined(STM32F0xx) || defined(STM32L0xx)
HAL_I2C_ER_IRQHandler(handle);
#endif // defined(STM32F0xx) || defined(STM32L0xx)
}

#if !defined(STM32F0xx) && !defined(STM32L0xx)
/**
* @brief This function handles I2C1 interrupt.
* @param None
Expand All @@ -536,7 +573,10 @@ void I2C1_ER_IRQHandler(void)
I2C_HandleTypeDef * handle = i2c_handles[0];
HAL_I2C_ER_IRQHandler(handle);
}
#endif // !defined(STM32F0xx) && !defined(STM32L0xx)
#endif // I2C1_BASE

#if defined(I2C2_BASE)
/**
* @brief This function handles I2C2 interrupt.
* @param None
Expand All @@ -546,8 +586,12 @@ void I2C2_EV_IRQHandler(void)
{
I2C_HandleTypeDef * handle = i2c_handles[1];
HAL_I2C_EV_IRQHandler(handle);
#if defined(STM32F0xx) || defined(STM32L0xx)
HAL_I2C_ER_IRQHandler(handle);
#endif // defined(STM32F0xx) || defined(STM32L0xx)
}

#if !defined(STM32F0xx) && !defined(STM32L0xx)
/**
* @brief This function handles I2C2 interrupt.
* @param None
Expand All @@ -558,7 +602,10 @@ void I2C2_ER_IRQHandler(void)
I2C_HandleTypeDef * handle = i2c_handles[1];
HAL_I2C_ER_IRQHandler(handle);
}
#endif // !defined(STM32F0xx) && !defined(STM32L0xx)
#endif // I2C2_BASE

#if defined(I2C3_BASE)
/**
* @brief This function handles I2C3 interrupt.
* @param None
Expand All @@ -568,8 +615,12 @@ void I2C3_EV_IRQHandler(void)
{
I2C_HandleTypeDef * handle = i2c_handles[2];
HAL_I2C_EV_IRQHandler(handle);
#if defined(STM32F0xx) || defined(STM32L0xx)
HAL_I2C_ER_IRQHandler(handle);
#endif // defined(STM32F0xx) || defined(STM32L0xx)
}

#if !defined(STM32F0xx) && !defined(STM32L0xx)
/**
* @brief This function handles I2C3 interrupt.
* @param None
Expand All @@ -580,7 +631,10 @@ void I2C3_ER_IRQHandler(void)
I2C_HandleTypeDef * handle = i2c_handles[2];
HAL_I2C_ER_IRQHandler(handle);
}
#endif // !defined(STM32F0xx) && !defined(STM32L0xx)
#endif // I2C3_BASE

#if defined(I2C4_BASE)
/**
* @brief This function handles I2C4 interrupt.
* @param None
Expand All @@ -590,8 +644,12 @@ void I2C4_EV_IRQHandler(void)
{
I2C_HandleTypeDef * handle = i2c_handles[3];
HAL_I2C_EV_IRQHandler(handle);
#if defined(STM32F0xx) || defined(STM32L0xx)
HAL_I2C_ER_IRQHandler(handle);
#endif // defined(STM32F0xx) || defined(STM32L0xx)
}

#if !defined(STM32F0xx) && !defined(STM32L0xx)
/**
* @brief This function handles I2C4 interrupt.
* @param None
Expand All @@ -602,6 +660,8 @@ void I2C4_ER_IRQHandler(void)
I2C_HandleTypeDef * handle = i2c_handles[3];
HAL_I2C_ER_IRQHandler(handle);
}
#endif // !defined(STM32F0xx) && !defined(STM32L0xx)
#endif // I2C4_BASE

/**
* @}
Expand Down
26 changes: 24 additions & 2 deletions cores/arduino/stm32/twi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,29 @@
/* offsetof is a gcc built-in function, this is the manual implementation */
#define OFFSETOF(type, member) ((uint32_t) (&(((type *)(0))->member)))

/* I2C Tx/Rx buffer size */
#define I2C_TXRX_BUFFER_SIZE 32

/* Redefinition of IRQ for F0 & L0 family */
#if defined(STM32F0xx) || defined(STM32L0xx)
#if defined(I2C1_BASE)
#define I2C1_EV_IRQn I2C1_IRQn
#define I2C1_EV_IRQHandler I2C1_IRQHandler
#endif // defined(I2C1_BASE)
#if defined(I2C2_BASE)
#define I2C2_EV_IRQn I2C2_IRQn
#define I2C2_EV_IRQHandler I2C2_IRQHandler
#endif // defined(I2C2_BASE)
#if defined(I2C3_BASE)
#define I2C3_EV_IRQn I2C3_IRQn
#define I2C3_EV_IRQHandler I2C3_IRQHandler
#endif // defined(I2C3_BASE)
#if defined(I2C4_BASE)
#define I2C4_EV_IRQn I2C4_IRQn
#define I2C4_EV_IRQHandler I2C4_IRQHandler
#endif // defined(I2C4_BASE)-
#endif // defined(STM32F0xx) || defined(STM32L0xx)

typedef struct i2c_s i2c_t;

struct i2c_s {
Expand All @@ -67,10 +88,11 @@ struct i2c_s {
PinName sda;
PinName scl;
IRQn_Type irq;
#ifdef STM32F1xx
#if !defined(STM32F0xx) && !defined(STM32L0xx)
IRQn_Type irqER;
#endif
#endif //!defined(STM32F0xx) && !defined(STM32L0xx)
uint8_t slaveMode;
uint8_t isMaster;
void (*i2c_onSlaveReceive)(uint8_t *, int);
void (*i2c_onSlaveTransmit)(void);
uint8_t i2cTxRxBuffer[I2C_TXRX_BUFFER_SIZE];
Expand Down