IDF 5.2 developer branch: i2c master => unable to read data

[cpehonk]

Hi All!

I tried out the IDF 5.2 branch as to the new I2C driver implementation. Unfortunately, I cant get it to work to read data from devices in master mode. Writing data to devices works like a charm.
Using reading functions (i2c_master_receive or i2c_master_transmit_receive) leads to a hang and a watchdog timeout in ISR.
After fail and restart, the I2C is left in an unusable state. Thus, calling i2c_new_master_bus leads to a hang and WDT timeout as well, unfortunately.
Please, could you investigate?

Here is my code snippet in order to communicate with a BME280 device:

Code: Select all

    
    i2c_master_bus_config_t conf = {
        .i2c_port = -1,
        .sda_io_num = GPIO_NUM_21,
        .scl_io_num = GPIO_NUM_22,
        .clk_source = I2C_CLK_SRC_DEFAULT,
        .glitch_ignore_cnt = 7,
    };

    ESP_LOGI(LOGTAG, "MasterInit call new master bus");
    i2c_master_bus_handle_t handle = NULL;
    esp_err_t err = i2c_new_master_bus(&conf, &handle);
    if (err != ESP_OK)
    {
        ESP_LOGE(LOGTAG, "i2c new master failed:%d", err);
    }

    if (i2c_master_probe(handle, 0x76, 10) == ESP_OK)
    {
        ESP_LOGI(LOGTAG, "found device at $%02x", 0x76);

        i2c_master_dev_handle_t hDev = NULL;
        i2c_device_config_t cfg = {
            .dev_addr_length = I2C_ADDR_BIT_LEN_7,
            .device_address = 0x76,
            .scl_speed_hz = 100000,
        };

        err = i2c_master_bus_add_device(handle, &cfg, &hDev);
        if (err == ESP_OK)
        {
            ESP_LOGI(LOGTAG, "read chip revision of BM*280");
            uint8_t reg = 208;
            uint8_t data = 0;

            err = i2c_master_transmit_receive(hDev, &reg, 1, &data, 1, -1);
            ESP_LOGI(LOGTAG, "transmit/receive: %d", err);
            if (err == ESP_OK)
                ESP_LOGI(LOGTAG, "Chip ID=%02x", data);
            else
                ESP_LOGE(LOGTAG, "failed %d", err);
        }

        // freigeben
        i2c_master_bus_rm_device(hDev);
    }

Thx,
Christoph

[matthias122]

Hi Christoph,
I already implemented the new i2C Driver and also get the same error. The solution is that you will need a array for read buffer.

So replace uint8_t data = 0; with uint8_t data [1] = 0;

Best regards
Matthias

[cpehonk]

Hi Matthias,

thanks for the hint! But unfortunately, even with "uint8_t data[1]" both read functions block until watchdog reset.
Do you use the 5.2 release branch or the 5.2 developer branch?

Greetings
Christoph

[matthias122]

Hi Christoph,
I tried the 5.2 Release and also the Master branch. Both are working.
Could you please activate i2C debug in Component config/ESP-Driver: I2C Configuration and post error message.

Here is my implementation of I2C read function. It is working. maybe you can try it. Sorry but you also need a array for write buffer.
Best regards
Matthias

Code: Select all

esp_err_t i2c_master_bus_read_byte(i2c_master_dev_handle_t dev_handle, uint8_t mem_address, uint8_t *data)
{
    uint8_t buf[1] = {mem_address};
    uint8_t buffer[1];
    esp_err_t ret = i2c_master_transmit_receive(dev_handle, buf, 1, buffer, 1, CONFIG_I2C_TIMEOUT); 
    *data=buffer[0];
    return ret;
}

[cpehonk]

Hi Matthias,

I am out

I tried it with the function you provided... nothing. Still blocks within read part, getting resetted by task watchdog and afterwards blocked in i2c_new_master_bus ... what am I missing?
LOG is set to "VERBOSE", check of i2c log is set, but there are no significant log messages from the i2c master driver ...

This is the complete program:

Code: Select all

esp_err_t i2c_master_bus_read_byte(i2c_master_dev_handle_t dev_handle, uint8_t mem_address, uint8_t *data)
{
    uint8_t buf[1] = {mem_address};
    uint8_t buffer[1];
    esp_err_t ret = i2c_master_transmit_receive(dev_handle, buf, 1, buffer, 1, CONFIG_I2C_TIMEOUT);
    *data = buffer[0];
    return ret;
}

void app_main(void)
{
    i2c_master_bus_config_t conf = {
        .i2c_port = -1,
        .sda_io_num = GPIO_NUM_21,
        .scl_io_num = GPIO_NUM_22,
        .clk_source = I2C_CLK_SRC_DEFAULT,
        .glitch_ignore_cnt = 7,
    };

    ESP_LOGI(LOGTAG, "MasterInit call new master bus");
    i2c_master_bus_handle_t handle = NULL;
    esp_err_t err = i2c_new_master_bus(&conf, &handle);
    if (err != ESP_OK)
    {
        ESP_LOGE(LOGTAG, "i2c new master failed:%d", err);
    }
    else if (i2c_master_probe(handle, 0x76, 10) == ESP_OK)
    {
        ESP_LOGI(LOGTAG, "found device at $%02x", 0x76);

        i2c_master_dev_handle_t hDev = NULL;
        i2c_device_config_t cfg = {
            .dev_addr_length = I2C_ADDR_BIT_LEN_7,
            .device_address = 0x76,
            .scl_speed_hz = 400000,
        };

        err = i2c_master_bus_add_device(handle, &cfg, &hDev);
        if (err == ESP_OK)
        {
            ESP_LOGI(LOGTAG, "read chip revision of BM*280");
            uint8_t reg = 208;
            uint8_t data = 0;

            err = i2c_master_bus_read_byte(hDev, reg, &data);
            ESP_LOGI(LOGTAG, "receive: %d", err);
            if (err == ESP_OK)
                ESP_LOGI(LOGTAG, "Chip ID=%02x", data);
            else
                ESP_LOGE(LOGTAG, "failed %d", err);
        }

        // freigeben
        i2c_master_bus_rm_device(hDev);
    }
    esp_restart();
}

The task gets resetted by watchdog before reaching ESP_LOGI after read... The device is good as the legacy driver works with it even with 1 MHz SCL clock.

Greetings
Christoph

[matthias122]

Hi Christoph,

ok I will try in Hardware.

Which Module or Board are you using?

I have already tested my code on ESP32 and ESP32-S3.

Best regards

Matthias Ahrens

[cpehonk]

Hi Matthias,

I opened an issue in Github. Simon Cao answered very quickly that it is a known bug. Which is already fixed internally. He posted the right code and I will do so here if anybody else encounters the same problem. You just have to replace the given function:

Code: Select all

static bool s_i2c_read_command(i2c_master_bus_handle_t i2c_master, i2c_operation_t *i2c_operation, uint8_t *fifo_fill, BaseType_t *do_yield)
{
    i2c_master->async_break = false;
    const size_t remaining_bytes = i2c_operation->total_bytes - i2c_operation->bytes_used;
    const i2c_ll_hw_cmd_t hw_end_cmd = {
        .op_code = I2C_LL_CMD_END
    };
    i2c_hal_context_t *hal = &i2c_master->base->hal;
    i2c_bus_handle_t handle = i2c_master->base;
    i2c_ll_hw_cmd_t hw_cmd = i2c_operation->hw_cmd;

    *fifo_fill = MIN(remaining_bytes, SOC_I2C_FIFO_LEN - i2c_master->read_len_static);
    i2c_master->rx_cnt = *fifo_fill;
    hw_cmd.byte_num = *fifo_fill;

    i2c_master->contains_read = true;
#if !SOC_I2C_STOP_INDEPENDENT
    if (remaining_bytes < SOC_I2C_FIFO_LEN - 1) {
        if (i2c_operation->hw_cmd.ack_val == ACK_VAL) {
            if (remaining_bytes != 0) {
                i2c_ll_master_write_cmd_reg(hal->dev, hw_cmd, i2c_master->cmd_idx);
                i2c_master->read_len_static = i2c_master->rx_cnt;
                i2c_master->cmd_idx++;
            }
            i2c_master->read_buf_pos = i2c_master->trans_idx;
        } else {
            i2c_ll_master_write_cmd_reg(hal->dev, hw_cmd, i2c_master->cmd_idx);
            i2c_master->cmd_idx++;
        }
        i2c_master->trans_idx++;
        i2c_master->i2c_trans.cmd_count--;
        if (i2c_master->async_trans == false) {
            if (xPortInIsrContext()) {
                xSemaphoreGiveFromISR(i2c_master->cmd_semphr, do_yield);
            } else {
                xSemaphoreGive(i2c_master->cmd_semphr);
            }
        }
    } else {
        atomic_store(&i2c_master->status, I2C_STATUS_READ);
        portENTER_CRITICAL_SAFE(&handle->spinlock);
        i2c_ll_master_write_cmd_reg(hal->dev, hw_cmd, i2c_master->cmd_idx);
        i2c_ll_master_write_cmd_reg(hal->dev, hw_end_cmd, i2c_master->cmd_idx + 1);
        if (i2c_master->async_trans == false) {
            i2c_hal_master_trans_start(hal);
        } else {
            i2c_master->async_break = true;
        }
        portEXIT_CRITICAL_SAFE(&handle->spinlock);
    }
#else
    portENTER_CRITICAL_SAFE(&handle->spinlock);
    // If the read command work with ack_val, but no bytes to read, we skip
    // this command, and run next command directly.
    if (hw_cmd.ack_val == ACK_VAL) {
        if (i2c_operation->total_bytes == 0) {
            i2c_master->trans_idx++;
            hw_cmd = i2c_master->i2c_trans.ops[i2c_master->trans_idx].hw_cmd;
            i2c_master->i2c_trans.cmd_count--;
        }
    }
    i2c_ll_master_write_cmd_reg(hal->dev, hw_cmd, i2c_master->cmd_idx);
    i2c_ll_master_write_cmd_reg(hal->dev, hw_end_cmd, i2c_master->cmd_idx + 1);
    portEXIT_CRITICAL_SAFE(&handle->spinlock);
    i2c_master->status = I2C_STATUS_READ;
    portENTER_CRITICAL_SAFE(&handle->spinlock);
    if (i2c_master->async_trans == false) {
        i2c_hal_master_trans_start(hal);
    } else {
        i2c_master->async_break = true;
    }
    portEXIT_CRITICAL_SAFE(&handle->spinlock);
#endif

    return i2c_master->async_break;
}

Nevertheless, he expects an update in Github soon.

Greetings,
Christoph