I2C fix READ of zero bytes hardware hang #2294
Conversation
**esp32-hal-i2c.c** * add callback for cpu frequency changes * adjust fifo thresholds based on cpu frequency and i2c bus frequency * reduce i2c bus frequency if differential is too small **Wire.h** * version to 1.1.0
The i2c peripheral will hang if a READ request is issued with a zero data length. The peripheral drops into a continuous timeout interrupt response. The STOP command can not be set out to the I2C bus. The SLAVE device correctly ACK'd the address byte, with READ bit set, it has control of the SDA pin. The ESP32 send out the next SCL HIGH pulse but, since the SLAVE is in READ Mode, and the First bit it is sending happened to be a ZERO, the ESP32 cannot send the STOP. When it releases SDA during the SCL HIGH, the pin does not change state. The pin stays low because the SLAVE is outputing a LOW! The ESP32 drops into a perminent WAIT state waiting for SDA to go HIGH (the STOP). **esp32-hal-i2c.c** * add databuff length checks to `i2cRead()` and `i2cWrite()` waiting for the ESP32 peripheral to send clock pulses on the SCL pin.
|
too many changes in this PR? |
|
I just fetched it against main i saw you merge cpu-clock-integration in, why are all your changes showing as new? I'll delete and manually copy. |
| @@ -1609,6 +1609,9 @@ i2c_err_t i2cFlush(i2c_t * i2c) | |||
| } | |||
|
|
|||
| i2c_err_t i2cWrite(i2c_t * i2c, uint16_t address, uint8_t* buff, uint16_t size, bool sendStop, uint16_t timeOutMillis){ | |||
| if((i2c==NULL)||((size>0)&&(buff==NULL))) { // need to have location to store requested data | |||
There was a problem hiding this comment.
this can possibly be simplified to:
if(!i2c||(size && !buff))
NULL is defined as zero..
There was a problem hiding this comment.
@atanisoft yes it could be obfuscated down to your concise example. I like to write code such that assumptions are minimized. I have found that I make more mistakes when I try to rely on assumptions.
The !i2c starts off wrong in my mind. !(not) starts right off describing a boolean operation, then up comes a pointer? So, my mind says "if not a pointer" What? What's NOT a pointer? Oh!, this construct means "if the value of i2c is not zero" well why don't you just write it that way, a few extra characters in a source file don't mean squat when the hard drive is measured in terabytes.
Chuck.
|
hi, regards, johann |
|
@johannruch Don't know of anything that would cause this slowdown, What clock speed are you setting the I2c bus too? Chuck. |
|
@chuck: i define 400kHz: i know there are other changes since dec 2018, but the nearest item was I2C :-) |
|
@johannruch i just uploaded a readme on i2c debugging #2325 try displaying the interrupts, it will show you exact timing. |
|
@chuck: i'm impressed! your hint about the i2c bus frequency let me thinking about 400 or 40000L for 400kHz. a short look in esp32-hal-i2c.c gaves the answer, it has to be 40000L! thanks for your (and others) work and support! |
|
The esp32 @ 240mhz has a minimum i2c freq of 10khz. New code sets freq at nearest edge when out of bounds.
Chuck. |
|
Make sure you use enough zeros. |
The i2c peripheral will hang if a READ request is issued with a zero data length. The peripheral
drops into a continuous timeout interrupt response. The STOP command can not be set out to the I2C
bus. The SLAVE device correctly ACK'd the address byte, with READ bit set, it has control of the SDA
pin. The ESP32 send out the next SCL HIGH pulse but, since the SLAVE is in READ Mode, and the First
bit it is sending happened to be a ZERO, the ESP32 cannot send the STOP. When it releases SDA during
the SCL HIGH, the pin does not change state. The pin stays low because the SLAVE is outputting a LOW!
The ESP32 drops into a permanent WAIT state waiting for SDA to go HIGH (the STOP).
esp32-hal-i2c.c
i2cRead()andi2cWrite()waiting for the ESP32 peripheral to send clock pulses on the SCL pin.