I2C: Add clock stretching support. #142
Conversation
r12f
changed the title
src/drv/stm32cube/bsp_i2c_master.c
Outdated
| @@ -192,6 +200,40 @@ bsp_status_t bsp_i2c_stop(bsp_dev_i2c_t dev_num) | |||
| return BSP_OK; | |||
| } | |||
|
|
|||
| /** \brief Set SCL to float and wait for slave device to be ready | |||
| */ | |||
| static void i2c_master_set_scl_float_and_wait_ready(void) | |||
There was a problem hiding this comment.
Add a return value bsp_status_t BSP_OK or an error in case of Timeout BSP_TIMEOUT
There was a problem hiding this comment.
Sounds good! and this is the result after the update.
The error message is shown only when clock stretch is enabled:
i2c1> clock-stretch 1
i2c1> show
GPIO resistor: pull-up
Mode: master
Frequency: 100khz (50khz, 400khz, 1mhz)
Clock stretch timeout: 1 ticks / 10.00 us (0 = Disabled)
i2c1> [w 0x50] &:10 [w 0x50 0x00] &:10 [0x51 r:1]
I2C START
WRITE: 0x50 ACK
i2c clock stretching timed out. Please consider increase the timeout with clock-stretch command to see if it helps.
WRITE error:3
I2C STOP
I2C START
WRITE: 0x50 NACK 0x00 NACK
i2c clock stretching timed out. Please consider increase the timeout with clock-stretch command to see if it helps.
WRITE error:3
I2C STOP
I2C START
WRITE: 0x51 ACK
i2c clock stretching timed out. Please consider increase the timeout with clock-stretch command to see if it helps.
WRITE error:3
READ: 0x07
i2c clock stretching timed out. Please consider increase the timeout with clock-stretch command to see if it helps.
READ error:3
I2C STOP
When it is disabled, it does the same as what we are doing now:
i2c
GPIO resistor: pull-up
Mode: master
Frequency: 100khz (50khz, 400khz, 1mhz)
Clock stretch timeout: 0 ticks / 0.00 us (0 = Disabled)
i2c1> [w 0x50] &:10 [w 0x50 0x00] &:10 [0x51 r:1]
I2C START
WRITE: 0x50 ACK
I2C STOP
I2C START
WRITE: 0x50 NACK 0x00 ACK
I2C STOP
I2C START
WRITE: 0x51 ACK
READ: 0x07 NACK
I2C STOP
There was a problem hiding this comment.
1 small note: I add a function to print the i2c only error inside i2c mode. I am aware of hydrabus_mode.c has code to print out the error, but I feel that is way to generic and probably not a good idea to put mode and error checking code there, especially, BSP_TIMEOUT could mean different things, e.g. clock stretch timeout in IO, while no data returned for sniff. So maybe within i2c mode is a better place.
I didn't choose to pass in the con pointer into the dev module, because I didn't see any other ones doing it, hence I believe this is a code layer thing. bsp_* functions should only have knowledge of the device but not the upper logic, which makes sense to me.
| @@ -61,11 +61,13 @@ static void init_proto_default(t_hydra_console *con) | |||
| proto->config.i2c.dev_speed = 1; | |||
| proto->config.i2c.ack_pending = 0; | |||
| proto->config.i2c.dev_mode = DEV_MASTER; | |||
| proto->config.i2c.dev_clock_stretch_timeout = 0; | |||
There was a problem hiding this comment.
I think it will be a good idea to use it by default with a short default timeout set to 10 or more it will help to detect problems with some I2C devices
To be discussed with @Baldanos
There was a problem hiding this comment.
sounds perfect! once we have a conclusion. I will make the code change.
There was a problem hiding this comment.
Keep it like that we will change it later if needed
There was a problem hiding this comment.
Sounds perfect! Please let me know if any other change is needed. I will make it accordingly.
|
Thanks for your contribution |
|
woot! thank you for this awesome project too! |
This change adds the ability of handling i2c clock stretching. Clock stretching could happen on certain devices, such as BNO055 and PN532. When it happens, we could not sending the I2C read/writes blindly, but have to wait until the SCL coming back to high before sending the next bits. Otherwise, the SDA signals will be ignored by the device and ends up with incorrect communication.
However, clock stretching can cause I2C bus hang indefinitely by faulty devices, so many devices doesn't do clock stretching at all. So due to the rareness of clock stretching, this feature is turned off by default. And we need to explicitly enable it with the following command when running in i2c mode:
If we specify 0, it will disable the feature, otherwise it means how many full clock ticks we wait. This also means the final time that we waits depends on the frequency that we use. If the frequency is 100Khz, it means every clock tick will be 10us. If we specify
clock-stretch 100, we will be waiting at maximum 1ms. But if the frequency is 1Mhz, we will be waiting 100us.We choose the ticks instead of time in the setting, because it match more on the actual behavior and not every time interval can be mapped to a set of complete full clock ticks.