SuperHouse / esp-open-rtos

Open source FreeRTOS-based ESP8266 software framework
BSD 3-Clause "New" or "Revised" License
1.52k stars 491 forks source link

I2C Read Issue #641

Closed jreed509 closed 6 years ago

jreed509 commented 6 years ago

I've been trying to implement an I2C interface to a MAG3110 magnetometer on esp-open-rtos and have been running into issues.

It appears that fairly often (about 50% of the time) the I2C signal is corrupted. When this happens the I2C debug output prints "I2C: arbitration lost in i2c_write_bit from bus 0" or "I2C: Read Error".

Here's some sample code:

...

#define BUS     (0)
#define SCL_PIN (5)
#define SDA_PIN (4)

#define MAG31100_I2C_ADDRESS 0x0E
#define MAG3110_WHO_AM_I 0x07

void user_init(void) {
    uart_set_baud(0, 115200);
    uint8_t bus = BUS;
    uint8_t addr = MAG3110_I2C_ADDRESS;

    i2c_init(BUS, SCL_PIN, SDA_PIN, I2C_FREQ_100K);

    while(1) {
        uint8_t output;
        uint8_t reg = MAG3110_WHO_AM_I;
        i2c_slave_read(bus, addr, &reg, &output, 1);
        printf("ouput: %d\n", output);

        for (uint16_t i=0; i<1000; i++) {
            sdk_os_delay_us(1000);
        }

    }
}

The serial output looks like this (the expected output is 196 [0xC4]):

output: 196
output: 196
I2C: Read Error
output: 196
output: 196
I2C: arbitration lost in i2c_write_bit from bus 0
output: 252
I2C: Read Error
output: 252
I2C: arbitration lost in i2c_write_bit from bus 0
I2C: arbitration lost in i2c_stop from bus 0
output: 254
I2C: arbitration lost in i2c_start from bus 0
I2C: Read Error
output: 254
output: 196
output: 196
I2C: Read Error
output: 196

I hooked up a scope to the I2C lines and often the data looks like this: image

Here's a successful transmission for reference: image

I've tested this same circuit, with the same ESP board using an Arduino example and it works perfectly using the Arduino Wire library - so it seems that something is up with this I2C implementation.

Any thoughts on what could be causing this?

ourairquality commented 6 years ago

It looks like the slave is stretching the clock after receiving the address, and either the driver is not waiting or the wait time is far to short. Could you try increasing the clock stretch wait time with i2c_set_clock_stretch(bus, clk_stretch), otherwise try to debug the clock stretching code paths?

jreed509 commented 6 years ago

Thanks a lot for the quick response! It looks like this solved it.

I addedi2c_set_clock_strech(bus, 300); immediately after i2c_init() and everything appears to be working flawlessly now. (smaller values weren't enough)

It looks like I2C_DEFAULT_CLK_STRETCH was originally set to 10 by default. I'm curious as to why this is so low/is this meant to be adjusted by the user?

Thanks again! I spent way too much time debugging this...

flannelhead commented 6 years ago

@jreed509 I feel you, I2C debugging is never too easy... Indeed there have also recently been other reports of I2C not working properly

Would be nice to see waveforms after the change of I2C_DEFAULT_CLK_STRETCH, as in how much clock stretching is actually taking place now.

Perhaps the clock stretching limit has to be increased or the clock stretching code changed somehow. In https://github.com/SuperHouse/esp-open-rtos/pull/503 I landed some optimizations for GPIO access in the I2C driver. Probably due to read_scl() being slow earlier, the default clock stretch value has worked well enough. In its current state, the clock stretching code actually seems dependent of the CPU frequency.

I'll see if I can figure something out. Don't hold your breath for it though, hopefully increasing the limit helped in solving the most acute problem. :)

jreed509 commented 6 years ago

@flannelhead Unfortunately, after getting I2C working reliably for me I installed my device in a not-so-easy to access location so I can't grab another waveform at the moment.

I will say (from memory) that with the increased clock stretching value, the waveform looked very similar to the 2nd (successful) waveform above. The only notable difference was that often (but not always), the slave would stretch the clock for ~1-2 clock cycle periods at longest - the communication still worked fine when this happened.

At least according to this page a device could theoretically stretch the clock forever. Maybe it makes sense to have I2C_DEFAULT_CLK_STRETCH large by default and/or add a debug message letting the user know if the clock stretch timeout was reached?

At least for the short term though, increasing i2c_set_clock_strech(bus, 300); seems to have solved all I2C communications issues I had. Ill go ahead and close this issue.