espressif / esp-idf

Espressif IoT Development Framework. Official development framework for Espressif SoCs.
Apache License 2.0
12.98k stars 7.12k forks source link

Problem RTC-I2C with Sensirion sensors (IDFGH-8536) #9990

Open remyhx opened 1 year ago

remyhx commented 1 year ago

Answers checklist.

General issue report

When I install my ESP-S3 devkitC-1 (latest updated master branch ESP-IDF) with a Sensirion SCD41, I have problems using the ulp_riscv_i2c_master_read_from_device function. Two reasons:

remyhx commented 1 year ago

It is solved with the help of @sudeep-mohanty (as a reply to my question during DevCon22)! He asked me to remove the delay of the receive loop in ulp_risc_i2c.c and now it works.

dobraMorda commented 1 year ago

@remyhx Hello! I am trying to use sensiron SHT30 in ULP but as you said, sensiron is two bytes addressing. How it works for you? I am trying to make it but it not works - i go faulty data.

remyhx commented 1 year ago

@dobraMorda if you write to the 2 byte register, just write as if you write to a one byte register and a second write byte. As if you write to that first register. And if you readout, I found with the SCD41 you can just write only the first register address. (And the mention above about the delay... I didn't touch it for two months now, maybe it was already updated)

dobraMorda commented 1 year ago

@sudeep-mohanty Is possible to write data or read data over i2c in your api without reg_addr?

I mean right now i am using it like below: ulp_riscv_i2c_master_set_slave_addr( slave_address ); ulp_riscv_i2c_master_set_slave_reg_addr( reg_address ); ulp_riscv_i2c_master_read_from_device( buffer, len );

but i want read data without reg_addr: ulp_riscv_i2c_master_set_slave_addr( slave_address ); ulp_riscv_i2c_master_read_from_device( data_to_write, len );

RIght now always api of ulp_riscv_i2c sending reg_address - i checked it with use osciloscope.

sudeep-mohanty commented 1 year ago

@dobraMorda Apologies for the delayed response. As of now it is not possible to write/read data using the RTC I2C peripheral without writing the reg_addr unfortunately. This is because of how the I2C peripheral controller is designed which always expects a reg_addr to be programmed. If no sub register address is programmed then the peripheral uses the value available in SENS_SAR_I2C_CTRL_REG[18:11] as the sub register address.

dobraMorda commented 1 year ago

@sudeep-mohanty Thank you for reply. I understand this. I am testing RTC-I2C a few days and i saw that the method:

ulp_riscv_i2c_master_write_to_device

can stay forever in while loop and never go back. I think there should be timeout in this while loop in case the condition is never true.

I am testing it on ESP32 S3 devkit.

dobraMorda commented 1 year ago

I added timeout to loop while in ulp_riscv_i2c.c in mehtods: void ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size) void ulp_riscv_i2c_master_write_to_device(uint8_t *data_wr, size_t size)

And i am testing it. Now it works well and never stay forver in loop. Before my fix it can stay in while after couple minutes of work.

Now this looks like here:

void ulp_riscv_i2c_master_read_from_device(uint8_t *data_rd, size_t size) {
    uint32_t i = 0;
    uint32_t cmd_idx = 0;

    if (size == 0) {
        // Quietly return
        return;
    }

    /* By default, RTC I2C controller is hard wired to use CMD2 register onwards for read operations */
    cmd_idx = 2;

    /* Write slave addr */
    ulp_riscv_i2c_format_cmd(cmd_idx++, ULP_I2C_CMD_WRITE, 0, 0, 1, 2);

    /* Repeated START */
    ulp_riscv_i2c_format_cmd(cmd_idx++, ULP_I2C_CMD_RESTART, 0, 0, 0, 0);

    /* Write slave register addr */
    ulp_riscv_i2c_format_cmd(cmd_idx++, ULP_I2C_CMD_WRITE, 0, 0, 1, 1);

    if (size > 1) {
        /* Read n - 1 bytes */
        ulp_riscv_i2c_format_cmd(cmd_idx++, ULP_I2C_CMD_READ, 0, 0, 1, size - 1);
    }

    /* Read last byte + NACK */
    ulp_riscv_i2c_format_cmd(cmd_idx++, ULP_I2C_CMD_READ, 1, 1, 1, 1);

    /* STOP */
    ulp_riscv_i2c_format_cmd(cmd_idx++, ULP_I2C_CMD_STOP, 0, 0, 0, 0);

    /* Configure the RTC I2C controller in read mode */
    SET_PERI_REG_BITS(SENS_SAR_I2C_CTRL_REG, 0x1, 0, 27);

    /* Enable Rx data interrupt */
    SET_PERI_REG_MASK(RTC_I2C_INT_ENA_REG, RTC_I2C_RX_DATA_INT_ENA);

    /* Start RTC I2C transmission */
    SET_PERI_REG_MASK(SENS_SAR_I2C_CTRL_REG, SENS_SAR_I2C_START_FORCE);
    SET_PERI_REG_MASK(SENS_SAR_I2C_CTRL_REG, SENS_SAR_I2C_START);

    for (i = 0; i < size; i++) {

        uint32_t timeout = ULP_RISCV_GET_CCOUNT() + 10000;
        /* Poll for RTC I2C Rx Data interrupt bit to be set */
        while (!REG_GET_FIELD(RTC_I2C_INT_ST_REG, RTC_I2C_RX_DATA_INT_ST)) { 
            /* Minimal delay to avoid hogging the CPU */
            ulp_riscv_delay_cycles( 10 );
            if ( timeout < ULP_RISCV_GET_CCOUNT() ) {
                break;
            }
        }

        /* Read the data
         *
         * Unfortunately, the RTC I2C has no fifo buffer to help us with reading and storing
         * multiple bytes of data. Therefore, we need to read one byte at a time and clear the
         * Rx interrupt to get ready for the next byte.
         */
#if CONFIG_IDF_TARGET_ESP32S2
        data_rd[i] = REG_GET_FIELD(RTC_I2C_DATA_REG, RTC_I2C_RDATA);
#elif CONFIG_IDF_TARGET_ESP32S3
        data_rd[i] = REG_GET_FIELD(RTC_I2C_DATA_REG, RTC_I2C_I2C_RDATA);
#endif // CONFIG_IDF_TARGET_ESP32S2

        /* Clear the Rx data interrupt bit */
        SET_PERI_REG_MASK(RTC_I2C_INT_CLR_REG, RTC_I2C_RX_DATA_INT_CLR);
    }

    /* Clear the RTC I2C transmission bits */
    CLEAR_PERI_REG_MASK(SENS_SAR_I2C_CTRL_REG, SENS_SAR_I2C_START_FORCE);
    CLEAR_PERI_REG_MASK(SENS_SAR_I2C_CTRL_REG, SENS_SAR_I2C_START);
}

void ulp_riscv_i2c_master_write_to_device(uint8_t *data_wr, size_t size){
    uint32_t i = 0;
    uint32_t cmd_idx = 0;

    if (size == 0) {
        // Quietly return
        return;
    }

    /* By default, RTC I2C controller is hard wired to use CMD0 and CMD1 registers for write operations */
    cmd_idx = 0;

    /* Write slave addr + reg addr + data */
    ulp_riscv_i2c_format_cmd(cmd_idx++, ULP_I2C_CMD_WRITE, 0, 0, 1, 2 + size);

    /* Stop */
    ulp_riscv_i2c_format_cmd(cmd_idx++, ULP_I2C_CMD_STOP, 0, 0, 0, 0);

    /* Configure the RTC I2C controller in write mode */
    SET_PERI_REG_BITS(SENS_SAR_I2C_CTRL_REG, 0x1, 1, 27);

    /* Enable Tx data interrupt */
    SET_PERI_REG_MASK(RTC_I2C_INT_ENA_REG, RTC_I2C_TX_DATA_INT_ENA);

    for (i = 0; i < size; i++) {
        /* Write the data to be transmitted */
        CLEAR_PERI_REG_MASK(SENS_SAR_I2C_CTRL_REG, I2C_CTRL_MASTER_TX_DATA_MASK);
        SET_PERI_REG_BITS(SENS_SAR_I2C_CTRL_REG, 0xFF, data_wr[i], 19);

        if (i == 0) {
            /* Start RTC I2C transmission. (Needn't do it for every byte) */
            SET_PERI_REG_MASK(SENS_SAR_I2C_CTRL_REG, SENS_SAR_I2C_START_FORCE);
            SET_PERI_REG_MASK(SENS_SAR_I2C_CTRL_REG, SENS_SAR_I2C_START);
        }

        uint32_t timeout = ULP_RISCV_GET_CCOUNT() + 10000;
        /* Poll for RTC I2C Tx Data interrupt bit to be set */
        while (!REG_GET_FIELD(RTC_I2C_INT_ST_REG, RTC_I2C_TX_DATA_INT_ST)) { 
            /* Minimal delay to avoid hogging the CPU */
            ulp_riscv_delay_cycles( 10 );
            if ( timeout < ULP_RISCV_GET_CCOUNT() ) {
                break;
            }
        }

        /* Clear the Tx data interrupt bit */
        SET_PERI_REG_MASK(RTC_I2C_INT_CLR_REG, RTC_I2C_TX_DATA_INT_CLR);
    }

    /* Clear the RTC I2C transmission bits */
    CLEAR_PERI_REG_MASK(SENS_SAR_I2C_CTRL_REG, SENS_SAR_I2C_START_FORCE);
    CLEAR_PERI_REG_MASK(SENS_SAR_I2C_CTRL_REG, SENS_SAR_I2C_START);
}

@sudeep-mohanty I think that you should consider making this mechanism permanent, as there is always the possibility of getting stuck in such a loop forever.

sudeep-mohanty commented 1 year ago

@dobraMorda Thank you for the valuable suggestion! Yes, the loop timeout certainly seems like a viable fallback option to avoid looping infinitely. We will incorporate it in the driver.