esp-idf-hal icon indicating copy to clipboard operation
esp-idf-hal copied to clipboard

I2C timeout calculated wrong for MCU !esp32

Open empirephoenix opened this issue 3 months ago • 1 comments

The current timeout() for the i2c bus is calculated using

impl From<Duration> for APBTickType {
    fn from(duration: Duration) -> Self {
        APBTickType(
            ((duration.as_nanos() + APB_TICK_PERIOD_NS as u128 - 1) / APB_TICK_PERIOD_NS as u128)
                as ::core::ffi::c_int,
        )
    }
}

Basically the calculation for some newer chips is different and the valid values for the low level api are now 1-22 and the actual timeout is calculated as: 2^12 * SCLK_PERIOD In contrast to the idf api documenation the actual one for the chip correctly explains this:

Chips using the newer formula seems to be at least:

Using a 5 bit register with the new formular ESP32-C2 21.4.7 https://www.espressif.com/sites/default/files/documentation/esp8684_technical_reference_manual_en.pdf ESP32-C3 28.4.8 https://www.espressif.com/sites/default/files/documentation/esp32-c3_technical_reference_manual_en.pdf ESP32-C6 28.4.8 https://www.espressif.com/sites/default/files/documentation/esp32-c6_technical_reference_manual_en.pdf ESP32-S3 27.4.8 https://www.espressif.com/sites/default/files/documentation/esp32-s3_technical_reference_manual_en.pdf ESP32-H2 27.4.8 https://www.espressif.com/sites/default/files/documentation/esp32-h2_technical_reference_manual_en.pdf

Using a 10bit register with calculation as currently ESP32 11.5 https://www.espressif.com/sites/default/files/documentation/esp32_technical_reference_manual_en.pdf ESP32-S2 25.3.3 https://www.espressif.com/sites/default/files/documentation/esp32-s2_technical_reference_manual_en.pdf

Unclear: ESP32-P4 technical document linked on espressif website returns 404 https://www.espressif.com/sites/default/files/documentation/esp32-p4_technical_reference_manual_en.pdf

If someone else could check that I'm understanding this documents corrrectly, I would like to do a pull request for this. Possible changes I see a) Calculate the nearest valid timeout number for the api and print? if the wished for timeout is not exactly representable b) Add a enum containing the valid 22 timeouts and implement From<newEnum> for APBTickType ? what approach would be prefered?

Also how would one do the case difference based on the current MCU, is there somewhere a similar part of code, I can take a look at? (coming from a c world, I only know #define magic for this, but this is obviously not the rust way)

I also did take a look at the arduino wrapper how they do this... https://github.com/espressif/arduino-esp32/blob/cf448906b3836fbe9368934713b697469254c62f/cores/esp32/esp32-hal-i2c.c#L133 They just set it to the maximum possible timeout, so apart from waiting now >100ms instead of ~13 most probably will not have noticed

empirephoenix avatar Apr 28 '24 01:04 empirephoenix

The current timeout() for the i2c bus is calculated using

impl From<Duration> for APBTickType {
    fn from(duration: Duration) -> Self {
        APBTickType(
            ((duration.as_nanos() + APB_TICK_PERIOD_NS as u128 - 1) / APB_TICK_PERIOD_NS as u128)
                as ::core::ffi::c_int,
        )
    }
}

Basically the calculation for some newer chips is different and the valid values for the low level api are now 1-22 and the actual timeout is calculated as: 2^12 * SCLK_PERIOD In contrast to the idf api documenation the actual one for the chip correctly explains this:

Chips using the newer formula seems to be at least:

Using a 5 bit register with the new formular ESP32-C2 21.4.7 https://www.espressif.com/sites/default/files/documentation/esp8684_technical_reference_manual_en.pdf ESP32-C3 28.4.8 https://www.espressif.com/sites/default/files/documentation/esp32-c3_technical_reference_manual_en.pdf ESP32-C6 28.4.8 https://www.espressif.com/sites/default/files/documentation/esp32-c6_technical_reference_manual_en.pdf ESP32-S3 27.4.8 https://www.espressif.com/sites/default/files/documentation/esp32-s3_technical_reference_manual_en.pdf ESP32-H2 27.4.8 https://www.espressif.com/sites/default/files/documentation/esp32-h2_technical_reference_manual_en.pdf

Using a 10bit register with calculation as currently ESP32 11.5 https://www.espressif.com/sites/default/files/documentation/esp32_technical_reference_manual_en.pdf ESP32-S2 25.3.3 https://www.espressif.com/sites/default/files/documentation/esp32-s2_technical_reference_manual_en.pdf

Unclear: ESP32-P4 technical document linked on espressif website returns 404 https://www.espressif.com/sites/default/files/documentation/esp32-p4_technical_reference_manual_en.pdf

If someone else could check that I'm understanding this documents corrrectly, I would like to do a pull request for this.

PR welcome!

Possible changes I see a) Calculate the nearest valid timeout number for the api and print?

This one except for the "print" part.

Also how would one do the case difference based on the current MCU, is there somewhere a similar part of code, I can take a look at? (coming from a c world, I only know #define magic for this, but this is obviously not the rust way)

#cfg(esp32c3)] with the all, not and any combinators. Besides the esp32c3 config bool, you have similar constants defined for each MCU.

ivmarkov avatar Apr 30 '24 13:04 ivmarkov