I2C endless SCL strobe until timeout on esp32-s3 wroom and esp-idf 4.4 (IDFGH-6923)

[bar-bog]

Environment

• Module or chip used: ESP32-S3-WROOM-1
• IDF version: release/v4.4
• Build System: idf.py
• Compiler version : xtensa-esp32-elf-gcc (crosstool-NG esp-2021r2-patch3) 8.4.0
• Operating System: Linux
• Using an IDE?: No
• Power Supply: Bench power supply with 5A limit

Problem Description

I have encountered an I2C issues when switching custom boards that had esp32 wroom modules on it with new esp32-s3 wroom. Issue happens only on esp32-s3 - when switching target to esp32 and running original board (with esp32) with the same code base the issue is not existing.
Problem occurs when communicating with a I2C chip (SLG46826) and only when sending a command with one byte of data. Before this error there is correct communication with the same chip.
There was a few issues mentioned that are similar, but couldn't find a solution there and it's not exactly the same problem in my opinion:
#8510

Expected Behavior

ESP32-S3 should stop clocking SCL line after STOP condition and return ESP_OK error.

Actual Behavior

ESP32-S3 is clocking SCL line after receiving a STOP condition until a Timeout occurs after one second. I2C driver returns TIMEOUT error to the application layer.

Logic analyzer images

Full communication with long clock pulse at the end:

Correct communication before an error:

Good communication in green and incorrect one in red:

Code to reproduce this issue

I am using this code as a function to write to sensor

    esp_err_t err = ESP_FAIL;
    uint8_t addr;

    if (mem_reg == REGISTER_DATA_BLOCK) {
        addr = SLG_I2C_BASE_ADDRESS;
    } else if (mem_reg == NVM_DATA_BLOCK) {
        addr = SLG_I2C_BASE_ADDRESS | NVM_DATA_BLOCK_I2C;
    } else if (mem_reg == EEPROM_DATA_BLOCK) {
        addr = SLG_I2C_BASE_ADDRESS | EEPROM_DATA_BLOCK_I2C;
    } else {
        return err;
    }

    i2c_cmd_handle_t cmd = i2c_cmd_link_create();
    i2c_master_start(cmd);
    i2c_master_write_byte(cmd, addr | WRITE_BIT, ACK_CHECK_EN);
    i2c_master_write_byte(cmd, reg, ACK_CHECK_EN);
    i2c_master_write(cmd, bufp, len, ACK_CHECK_EN);
    i2c_master_stop(cmd);
    err = i2c_master_cmd_begin(i2c_num, cmd, 1000 / portTICK_RATE_MS);
    i2c_cmd_link_delete(cmd);

    return err;

[bar-bog]

I think I was able to narrow down this issue to no interrupt after last hardware command. The i2c state machine expects this interrupt but in this case when it's not being sent the next event that comes to the state machine is a timeout event. I have made a short fix for this that once there is no next i2c command to execute I'm sending a done event and let the last command be executed:

@@ -1286,7 +1286,10 @@ static void IRAM_ATTR i2c_master_cmd_begin_static(i2c_port_t i2c_num)
             p_i2c->cmd_idx = 0;
         } else {
             p_i2c->cmd_link.head = p_i2c->cmd_link.head->next;
-            p_i2c->cmd_link.head->cmd.bytes_used = 0;
+            if(p_i2c->cmd_link.head)
+            {
+                p_i2c->cmd_link.head->cmd.bytes_used = 0;
+            }
         }
     } else if ((p_i2c->status == I2C_STATUS_ACK_ERROR)
                || (p_i2c->status == I2C_STATUS_TIMEOUT)) {
@@ -1367,6 +1370,13 @@ static void IRAM_ATTR i2c_master_cmd_begin_static(i2c_port_t i2c_num)
             p_i2c->cmd_idx = 0;
             break;
         }
+        if(p_i2c->cmd_link.head == NULL)
+        {
+            evt.type = I2C_CMD_EVT_DONE;
+            xQueueOverwriteFromISR(p_i2c->cmd_evt_queue, &evt, &HPTaskAwoken);
+            // Return to the IDLE status after cmd_eve_done signal were send out.
+            p_i2c->status = I2C_STATUS_IDLE;
+        }
     }
     i2c_hal_update_config(&(i2c_context[i2c_num].hal));
     i2c_hal_trans_start(&(i2c_context[i2c_num].hal));

Not really sure if that's a correct fix, but maybe it would be helpful.

[mythbuster5]

i2c_patch_1210.txt

Could you please try with this patch? To see whether this can help, thanks!

[bar-bog]

Definitely, will test it and let you know soon

[bar-bog]

I have checked out the fresh esp-idf v4.4 and tried to apply the patch but it seems like it does not apply:

esp-idf git:(8153bfe412) ✗ git apply i2c_patch_1210.txt 
error: patch failed: components/driver/i2c.c:531
error: components/driver/i2c.c: patch does not apply
error: patch failed: components/hal/esp32c3/include/hal/i2c_ll.h:799
error: components/hal/esp32c3/include/hal/i2c_ll.h: patch does not apply
error: patch failed: components/hal/i2c_hal_iram.c:16
error: components/hal/i2c_hal_iram.c: patch does not apply

Did you made the patch on 4.4 and for esp32-s3 ? I can still try to apply it manually if that would help you.

[mythbuster5]

i2c_patch_0317.txt

Sorry, there are too many new changes, please try this one? Thanks !

[bar-bog]

Hi, sorry for delay and thank you for your help . I did try to apply your patch on both v4.4 tag and release/v4.4 but this patch still didn't apply for me. Tried to apply it manually but it didn't work for me too. I think I got your intent with the change correctly i.e. guard the i2c isr with interrupt mask and force non-hardware bus reset. However I'm still stuck at the transaction.
What I have seen in my test is that I'm having one interrupt to little (no interrupt at last operation from the list) so I'm doing it manually.

[AxelLin]

i2c_patch_0317.txt

Sorry, there are too many new changes, please try this one? Thanks !

@mythbuster5
Above patch is against master, however this bug report is against v4.4. (It does not apply to v4.4 branch).

[mythbuster5]

@bar-bog Hi, we found another issue might cause this issue, could you please tell me the latest commit information of your release/v4.4?

[AxelLin]

@bar-bog @mythbuster5
How about let's sync to v4.4.1?
@mythbuster5 Then if you have patch for test, you can make the patch against v4.4.1.

[mythbuster5]

@AxelLin Emm, I'm trying to figure out the root cause of this problem, so I want to know the information of the latest commit the user used.

Might not just a I2C issue, also probably interrupt handler issue~

[aswinda29]

Hi, I was having exactly the same issue with v4.4. Then I upgraded to the master and the issue continued to occur.

What worked for me was applying the above mentioned patch i2c_patch_0317.txt coupled with the new i2c wrappers. (With the master, not V4.4)

i2c_master_write_to_device()
i2c_master_read_from_device()
i2c_master_write_read_device()

Here we can define the exact size of the read and write buffer length. This fixed the error I was getting. I hope this might help someone.

[Alvin1Zhang]

Thanks for reporting, fix on release/4.4 is available at f86be61, feel free to reopen if the issue still happens. Thanks.