i2c bug in 3.0, works with older idf

[ardiehl]

tried to update to the current idf from github yesterday (June 23, 2018). After that, i had sporadic timeout retuned by i2c_master_cmd_begin. This always happend after i updated the firmware on my esp and restarted (via reset button or software). If this happens, i2c is dead and the only way to get it up again is a power cycle. Reset button or soft reset: no effect.

Here is where this happens in i2c.c:

Code: Select all

        portBASE_TYPE evt_res = xQueueReceive(p_i2c->cmd_evt_queue, &evt, wait_time);
        if (evt_res == pdTRUE) {
            if (evt.type == I2C_CMD_EVT_DONE) {
                if (p_i2c->status == I2C_STATUS_TIMEOUT) {
                    // If the I2C slave are powered off or the SDA/SCL are connected to ground, for example,
                    // I2C hw FSM would get stuck in wrong state, we have to reset the I2C module in this case.
                    ESP_LOGE(I2C_TAG,"Timeout after xQueueReceive");  // AD: this is the timeout i get
                    i2c_hw_fsm_reset(i2c_num);
                    clear_bus_cnt = 0;
                    ret = ESP_ERR_TIMEOUT;

I have reverted to an older version of idf (i2c.c / 53760 / Feb 3 17:07), updated the firware, done a software restart without cycling power and i2c works fine as it should.
For me this looks like idf 3.0 is not resetting the i2c ?

[WiFive]

Current master is technically "3.2-dev". It does look like i2c_master_clear_bus code is not quite right. Previous version was slightly better, still not totally right.

This may be better

Code: Select all

static esp_err_t i2c_master_clear_bus(i2c_port_t i2c_num)
{
    I2C_CHECK(i2c_num < I2C_NUM_MAX, I2C_NUM_ERROR_STR, ESP_ERR_INVALID_ARG);
    int sda_in_sig = 0, scl_in_sig = 0;
    if (i2c_num == I2C_NUM_0) {
        sda_in_sig = I2CEXT0_SDA_IN_IDX;
        scl_in_sig = I2CEXT0_SCL_IN_IDX;
    } else if (i2c_num == I2C_NUM_1) {
        sda_in_sig = I2CEXT1_SDA_IN_IDX;
        scl_in_sig = I2CEXT1_SCL_IN_IDX;
    }
    int scl_io = GPIO.func_in_sel_cfg[scl_in_sig].func_sel;
    int sda_io = GPIO.func_in_sel_cfg[sda_in_sig].func_sel;
    I2C_CHECK((GPIO_IS_VALID_OUTPUT_GPIO(scl_io)), I2C_SCL_IO_ERR_STR, ESP_ERR_INVALID_ARG);
    I2C_CHECK((GPIO_IS_VALID_GPIO(sda_io)), I2C_SDA_IO_ERR_STR, ESP_ERR_INVALID_ARG);
    // We do not check whether the SDA line is low
    // because after some serious interference, the bus may keep high all the time and the i2c bus is out of service.
    gpio_set_direction(scl_io, GPIO_MODE_OUTPUT_OD);
    gpio_set_direction(sda_io, GPIO_MODE_OUTPUT_OD);
    gpio_set_level(scl_io, 0);
    gpio_set_level(sda_io, 1);
    gpio_set_level(scl_io, 1); //clock transition
    gpio_set_level(sda_io, 0); //start condition
    for (int i = 0; i < 9; i++) { //9 clocks
        gpio_set_level(scl_io, 0);
        gpio_set_level(scl_io, 1);
        gpio_set_level(scl_io, 1); //allow for rise time
    }
    gpio_set_level(scl_io, 0);
    gpio_set_level(sda_io, 1);
    gpio_set_level(scl_io, 1); //clock transition
    gpio_set_level(sda_io, 0); //another start condition
    gpio_set_level(scl_io, 0);
    gpio_set_level(scl_io, 1); //clock transition
    gpio_set_level(sda_io, 1); //stop condition
    
    i2c_set_pin(i2c_num, sda_io, scl_io, 1, 1, I2C_MODE_MASTER);
    return ESP_OK;
}

[ardiehl]

Current master is technically "3.2-dev".

Sorry,
Tried as follows:
Recompiled and flashed using 3.2-dev, had to soft restart 2 times to freeze i2c. Recompiled 3.2-dev with your new "i2c_master_clear_bus", uploaded, did a soft restart and i2c works well. Thanks for the fix.

Will your fix be integrated into current 3.2-dev on github ?

[vonnieda]

Current master is technically "3.2-dev".

Sorry,
Tried as follows:
Recompiled and flashed using 3.2-dev, had to soft restart 2 times to freeze i2c. Recompiled 3.2-dev with your new "i2c_master_clear_bus", uploaded, did a soft restart and i2c works well. Thanks for the fix.

Will your fix be integrated into current 3.2-dev on github ?

+1

I am experiencing a similar issue, and WiFive's function above let me manually reset the bus when I detect the condition and have it work. So +1 to getting this merged.

[fly135]

I use i2c quite a bit. I also do software resets of the esp32. I've never experienced a complete failure of the i2c. I do get the occasional timeout. But the next transaction then seems fine. What should I be on the lookout for?

John A

[vonnieda]

I use i2c quite a bit. I also do software resets of the esp32. I've never experienced a complete failure of the i2c. I do get the occasional timeout. But the next transaction then seems fine. What should I be on the lookout for?

John A

In my case, the bus would get in a state where SDA was being held either high or low, can't remember which, but nothing else would happen on it. Only way to get past was a full power cycle. Software restart, and even toggling the ESP's enable line would not fix it. The code above *does* seem to fix it.

I am not able to say for sure if the ESP was the cause of the problem, or if it was one of the other devices on the bus. In my case I have an LM75 and a MCP3422 on the bus. I didn't go so far as to take them off to see who was misbehaving.

I don't *think* it has ever happened during normal operations. I think it only happened either on initial boot or during development when I was flashing the chip.

Jason

[meowsqueak]

I thought we'd been through all of this with 2.x? I spent countless hours debugging the i2c driver, investigating with brushing wires together, debugging the interrupt handler, finally concluding with a workaround for a hardware bug. Are we now saying that the 3.x driver is broken again? I haven't updated my application to 3.x yet, and now I'm thinking I probably will not...

[mrrosen]

May not be relevant to all of the I2C issues people are seeing, but with respect to occasional (or constant) timeout errors from i2c_master_cmd_begin, there is a change from IDF v2.x to v3.x that caused problems for me talking to a TI BQ27621 battery monitor. Changes in the I2C driver force the bit-timeout, the timeout the hardware uses to make sure it doesnt get locked up on each bit of the transaction (see ESP32 Technical Reference Manual Section 11.5; Register 11.4), to be much shorter (from about 2.5ms to about 20us fro 400Khz I2C SCL frequency!). This bit-timeout applies to the acknowledge bit as well, so from devices that use clock stretching before acknowledging a read/write to do some work, this timeout can be way too short and terminate the transaction prematurely. I noticed that the premature termination also leads to some strange behavior on the bus as its usually partway through a complete transaction when the transaction is cancelled. In my case, the BQ27621 would stall the acknowledge by ~90us after writing the register address to which you want to read/write; causing bit-timeouts in the new IDF with applications using older IDFs worked fine. Note that this timeout is completely independent from the one passed into i2c_master_cmd_begin, the former being a timeout on each bit of the transaction, the latter being a timeout on the entire transaction. Luckily, you can change this timeout to be more generous using i2c_set_timeout, making sure to note that this function takes in a count in APB cycles and is only 20-bits wide, so a maximum timeout of ~13ms is possible (but that should be plenty for even the slowest of I2C devices). So, if you are getting frustrated with weird I2C timeouts and bus behavior (like I was ), just trying setting this timeout to something really generous as it probably wont impact your system much otherwise.

[ESP_houwenxiang]

Hi,
Can you provide the IDF commit ID which version works fine and which version works incorrectly?
thanks !!

[mrrosen]

Hi,
Can you provide the IDF commit ID which version works fine and which version works incorrectly?
thanks !!

If you are referring to my issue with the TI BQ27621, I checked out the v3.1.1 tag and it didnt work, while checking out the v2.0 tag worked fine. However, since I was able to narrow down the problem to the I2C bit timeout, I can tell you that it should work before ed1e32f583980d2fefcd47690b77d5b0e1896667 and fail after, since this seems to be the commit in which the bit timeout was changed from ~2.5ms to a function of the I2C SCL frequency, and only ~8 SCL clocks before timeout. While this timeout value is fine for individual data bits in I2C transaction (which I dont think will typically be clock stretched much if at all), it can easily fail on the ACK bit for some devices (like BQ27621) that can be slow to handle new data; as I mentioned, it seems like most I2C devices add some delay, even if its small between the last bit of data and the ACK if they need to do some processing. I would recommend making the default delay much longer to accommodate this, but the i2c_set_timeout at least provides a clean API for applications to fix this issue for slow I2C devices (though it did take me a long time to figure it out )