Skip to content

Commit

Permalink
Rethink STM32 I2C v2 HAL (ARMmbed#78)
Browse files Browse the repository at this point in the history 
* Initial attempt at rethinking the STM32 I2C v2 HAL.  Makes single-byte work properly and adds a new 'state' variable to track what the hardware is doing.

* Fix some initial test failures

* Fix incorrect logic

* Fix more incorrect logic

* Tabs to spaces

* Fix repeated starts with single-byte API

* Fix race condition causing stop() after nacked address to sometimes break things

* Fix missed i2c structs that should have been removed

* Fix doing a repeated start from single-byte to transaction API causing I2C peripheral to lock up

* Fix xferOperation being set wrong for repeated starts, causing the peripheral to hang

* Fix race condition with repeated start after single-byte operation

* Fix compilation for targets that use I2C IP v1

* Fix initialization of XferOperation for API v1, optimize stop()

* Remove unneeded line

* Add docs for I2C events
  • Loading branch information
@multiplemonomials
multiplemonomials authored Nov 21, 2022
1 parent 961632a commit 5b28f5b
Show file tree
Hide file tree
Showing 17 changed files with 473 additions and 518 deletions.
2 changes: 1 addition & 1 deletion features/frameworks/unity/source/unity.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ int UNITY_OUTPUT_FLUSH(void);
#endif

/* Helpful macros for us to use here */
#define UNITY_FAIL_AND_BAIL { UNITY_OUTPUT_CHAR('\n'); utest_unity_assert_failure(); }
#define UNITY_FAIL_AND_BAIL { UNITY_OUTPUT_CHAR('\n'); utest_unity_assert_failure(); Unity.CurrentTestFailed = 1; }
#define UNITY_IGNORE_AND_BAIL { UNITY_OUTPUT_CHAR('\n'); utest_unity_ignore_failure(); }

/* return prematurely if we are already in failure or ignore state */
Expand Down
31 changes: 29 additions & 2 deletions hal/include/hal/i2c_api.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,36 @@
*
* @{
*/

/**
* Indicates that an unspecified error has occurred in the transfer. This usually means
* either an internal error in the Mbed MCU's I2C module, or something like an arbitration loss.
* Does not indicate a NACK.
*/
#define I2C_EVENT_ERROR (1 << 1)

/**
* Indicates that the slave did not respond to the address byte of the transfer.
*/
#define I2C_EVENT_ERROR_NO_SLAVE (1 << 2)

/**
* Indicates that the transfer completed successfully.
*/
#define I2C_EVENT_TRANSFER_COMPLETE (1 << 3)

/**
* Indicates that a NACK was received after the address byte, but before the requested number of bytes
* could be transferred.
*
* Note: Not every manufacturer HAL is able to make a distinction between this flag and #I2C_EVENT_ERROR_NO_SLAVE.
* On a NACK, you might conceivably get one or both of these flags.
*/
#define I2C_EVENT_TRANSFER_EARLY_NACK (1 << 4)

/**
* Use this macro to request all possible I2C events.
*/
#define I2C_EVENT_ALL (I2C_EVENT_ERROR | I2C_EVENT_TRANSFER_COMPLETE | I2C_EVENT_ERROR_NO_SLAVE | I2C_EVENT_TRANSFER_EARLY_NACK)

/**@}*/
Expand All @@ -61,7 +87,8 @@ typedef struct i2c_s i2c_t;

enum {
I2C_ERROR_NO_SLAVE = -1,
I2C_ERROR_BUS_BUSY = -2
I2C_ERROR_BUS_BUSY = -2,
I2C_ERROR_INVALID_USAGE = -3 // Invalid usage of the I2C API, e.g. by mixing single-byte and transactional function calls.
};

typedef struct {
Expand Down Expand Up @@ -229,7 +256,7 @@ int i2c_byte_read(i2c_t *obj, int last);
*
* @param obj The I2C object
* @param data Byte to be written
* @return 0 if NAK was received, 1 if ACK was received, 2 for timeout.
* @return 0 if NAK was received, 1 if ACK was received, 2 for timeout, or 3 for other error.
*/
int i2c_byte_write(i2c_t *obj, int data);

Expand Down
33 changes: 0 additions & 33 deletions targets/TARGET_STM/TARGET_STM32F0/objects.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,39 +77,6 @@ struct serial_s {
#endif
};

struct i2c_s {
/* The 1st 2 members I2CName i2c
* and I2C_HandleTypeDef handle should
* be kept as the first members of this struct
* to ensure i2c_get_obj to work as expected
*/
I2CName i2c;
I2C_HandleTypeDef handle;
uint8_t index;
int hz;
PinName sda;
PinName scl;
IRQn_Type event_i2cIRQ;
IRQn_Type error_i2cIRQ;
uint32_t XferOperation;
volatile uint8_t event;
volatile int pending_start;
int current_hz;
#if DEVICE_I2CSLAVE
uint8_t slave;
volatile uint8_t pending_slave_tx_master_rx;
volatile uint8_t pending_slave_rx_maxter_tx;
uint8_t *slave_rx_buffer;
volatile uint16_t slave_rx_buffer_size;
volatile uint16_t slave_rx_count;
#endif
#if DEVICE_I2C_ASYNCH
uint32_t address;
uint8_t stop;
uint8_t available_events;
#endif
};

struct analogin_s {
ADC_HandleTypeDef handle;
PinName pin;
Expand Down
35 changes: 0 additions & 35 deletions targets/TARGET_STM/TARGET_STM32F3/objects.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,41 +90,6 @@ struct serial_s {
#endif
};

struct i2c_s {
/* The 1st 2 members I2CName i2c
* and I2C_HandleTypeDef handle should
* be kept as the first members of this struct
* to ensure i2c_get_obj to work as expected
*/
I2CName i2c;
I2C_HandleTypeDef handle;
uint8_t index;
int hz;
PinName sda;
PinName scl;
int sda_func;
int scl_func;
IRQn_Type event_i2cIRQ;
IRQn_Type error_i2cIRQ;
uint32_t XferOperation;
volatile uint8_t event;
volatile int pending_start;
int current_hz;
#if DEVICE_I2CSLAVE
uint8_t slave;
volatile uint8_t pending_slave_tx_master_rx;
volatile uint8_t pending_slave_rx_maxter_tx;
uint8_t *slave_rx_buffer;
volatile uint16_t slave_rx_buffer_size;
volatile uint16_t slave_rx_count;
#endif
#if DEVICE_I2C_ASYNCH
uint32_t address;
uint8_t stop;
uint8_t available_events;
#endif
};

struct dac_s {
DACName dac;
PinName pin;
Expand Down
33 changes: 0 additions & 33 deletions targets/TARGET_STM/TARGET_STM32F7/objects.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -108,39 +108,6 @@ struct serial_s {
#endif
};

struct i2c_s {
/* The 1st 2 members I2CName i2c
* and I2C_HandleTypeDef handle should
* be kept as the first members of this struct
* to ensure i2c_get_obj to work as expected
*/
I2CName i2c;
I2C_HandleTypeDef handle;
uint8_t index;
int hz;
PinName sda;
PinName scl;
IRQn_Type event_i2cIRQ;
IRQn_Type error_i2cIRQ;
uint32_t XferOperation;
volatile uint8_t event;
volatile int pending_start;
int current_hz;
#if DEVICE_I2CSLAVE
uint8_t slave;
volatile uint8_t pending_slave_tx_master_rx;
volatile uint8_t pending_slave_rx_maxter_tx;
uint8_t *slave_rx_buffer;
volatile uint16_t slave_rx_buffer_size;
volatile uint16_t slave_rx_count;
#endif
#if DEVICE_I2C_ASYNCH
uint32_t address;
uint8_t stop;
uint8_t available_events;
#endif
};

struct analogin_s {
ADC_HandleTypeDef handle;
PinName pin;
Expand Down
35 changes: 0 additions & 35 deletions targets/TARGET_STM/TARGET_STM32G0/objects.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,41 +89,6 @@ struct serial_s {
#endif
};

struct i2c_s {
/* The 1st 2 members I2CName i2c
* and I2C_HandleTypeDef handle should
* be kept as the first members of this struct
* to ensure i2c_get_obj to work as expected
*/
I2CName i2c;
I2C_HandleTypeDef handle;
uint8_t index;
int hz;
PinName sda;
PinName scl;
int sda_func;
int scl_func;
IRQn_Type event_i2cIRQ;
IRQn_Type error_i2cIRQ;
uint32_t XferOperation;
volatile uint8_t event;
volatile int pending_start;
int current_hz;
#if DEVICE_I2CSLAVE
uint8_t slave;
volatile uint8_t pending_slave_tx_master_rx;
volatile uint8_t pending_slave_rx_maxter_tx;
uint8_t *slave_rx_buffer;
volatile uint16_t slave_rx_buffer_size;
volatile uint16_t slave_rx_count;
#endif
#if DEVICE_I2C_ASYNCH
uint32_t address;
uint8_t stop;
uint8_t available_events;
#endif
};

struct flash_s {
/* nothing to be stored for now */
uint32_t dummy;
Expand Down
35 changes: 0 additions & 35 deletions targets/TARGET_STM/TARGET_STM32G4/objects.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,41 +88,6 @@ struct serial_s {
#endif
};

struct i2c_s {
/* The 1st 2 members I2CName i2c
* and I2C_HandleTypeDef handle should
* be kept as the first members of this struct
* to ensure i2c_get_obj to work as expected
*/
I2CName i2c;
I2C_HandleTypeDef handle;
uint8_t index;
int hz;
PinName sda;
PinName scl;
int sda_func;
int scl_func;
IRQn_Type event_i2cIRQ;
IRQn_Type error_i2cIRQ;
uint32_t XferOperation;
volatile uint8_t event;
volatile int pending_start;
int current_hz;
#if DEVICE_I2CSLAVE
uint8_t slave;
volatile uint8_t pending_slave_tx_master_rx;
volatile uint8_t pending_slave_rx_maxter_tx;
uint8_t *slave_rx_buffer;
volatile uint16_t slave_rx_buffer_size;
volatile uint16_t slave_rx_count;
#endif
#if DEVICE_I2C_ASYNCH
uint32_t address;
uint8_t stop;
uint8_t available_events;
#endif
};

struct dac_s {
DACName dac;
PinName pin;
Expand Down
33 changes: 0 additions & 33 deletions targets/TARGET_STM/TARGET_STM32H7/objects.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,39 +97,6 @@ struct serial_s {
#endif
};

struct i2c_s {
/* The 1st 2 members I2CName i2c
* and I2C_HandleTypeDef handle should
* be kept as the first members of this struct
* to ensure i2c_get_obj to work as expected
*/
I2CName i2c;
I2C_HandleTypeDef handle;
uint8_t index;
int hz;
PinName sda;
PinName scl;
IRQn_Type event_i2cIRQ;
IRQn_Type error_i2cIRQ;
uint32_t XferOperation;
volatile uint8_t event;
volatile int pending_start;
int current_hz;
#if DEVICE_I2CSLAVE
uint8_t slave;
volatile uint8_t pending_slave_tx_master_rx;
volatile uint8_t pending_slave_rx_maxter_tx;
uint8_t *slave_rx_buffer;
volatile uint16_t slave_rx_buffer_size;
volatile uint16_t slave_rx_count;
#endif
#if DEVICE_I2C_ASYNCH
uint32_t address;
uint8_t stop;
uint8_t available_events;
#endif
};

struct analogin_s {
ADC_HandleTypeDef handle;
PinName pin;
Expand Down
35 changes: 0 additions & 35 deletions targets/TARGET_STM/TARGET_STM32L0/objects.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,41 +91,6 @@ struct serial_s {
#endif
};

struct i2c_s {
/* The 1st 2 members I2CName i2c
* and I2C_HandleTypeDef handle should
* be kept as the first members of this struct
* to ensure i2c_get_obj to work as expected
*/
I2CName i2c;
I2C_HandleTypeDef handle;
uint8_t index;
int hz;
PinName sda;
PinName scl;
int sda_func;
int scl_func;
IRQn_Type event_i2cIRQ;
IRQn_Type error_i2cIRQ;
uint32_t XferOperation;
volatile uint8_t event;
volatile int pending_start;
int current_hz;
#if DEVICE_I2CSLAVE
uint8_t slave;
volatile uint8_t pending_slave_tx_master_rx;
volatile uint8_t pending_slave_rx_maxter_tx;
uint8_t *slave_rx_buffer;
volatile uint16_t slave_rx_buffer_size;
volatile uint16_t slave_rx_count;
#endif
#if DEVICE_I2C_ASYNCH
uint32_t address;
uint8_t stop;
uint8_t available_events;
#endif
};

#if DEVICE_FLASH
struct flash_s {
/* nothing to be stored for now */
Expand Down
35 changes: 0 additions & 35 deletions targets/TARGET_STM/TARGET_STM32L4/objects.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -87,41 +87,6 @@ struct serial_s {
#endif
};

struct i2c_s {
/* The 1st 2 members I2CName i2c
* and I2C_HandleTypeDef handle should
* be kept as the first members of this struct
* to ensure i2c_get_obj to work as expected
*/
I2CName i2c;
I2C_HandleTypeDef handle;
uint8_t index;
int hz;
PinName sda;
PinName scl;
int sda_func;
int scl_func;
IRQn_Type event_i2cIRQ;
IRQn_Type error_i2cIRQ;
uint32_t XferOperation;
volatile uint8_t event;
volatile int pending_start;
int current_hz;
#if DEVICE_I2CSLAVE
uint8_t slave;
volatile uint8_t pending_slave_tx_master_rx;
volatile uint8_t pending_slave_rx_maxter_tx;
uint8_t *slave_rx_buffer;
volatile uint16_t slave_rx_buffer_size;
volatile uint16_t slave_rx_count;
#endif
#if DEVICE_I2C_ASYNCH
uint32_t address;
uint8_t stop;
uint8_t available_events;
#endif
};

struct flash_s {
/* nothing to be stored for now */
uint32_t dummy;
Expand Down
Loading

0 comments on commit 5b28f5b

Please sign in to comment.