I2C: Add ability to do clock stretching

[TheButlah]

I'm working with the bno080 imu and I suspect that I'm getting timeouts due to clock stretching.
How would I control clock stretching with the hal?

[bjoernQ]

Currently we set the TO to 10 bus cycles - e.g. for C2, C3, S3 here

esp-hal/esp-hal-common/src/i2c.rs

Lines 584 to 586 in d29c4b1

	// default we set the timeout value to about 10 bus cycles
	// log(20*half_cycle)/log(2) = log(half_cycle)/log(2) + log(20)/log(2)
	let tout = (4 * 8 - (5 * half_cycle).leading_zeros()) + 2;

So, we should add a way to configure the TO (which is not yet possible).

You might want to test if that really helps in your case by just changing it locally and see if that helps - if it does, we need to add the functionality to optionally pass the timeout

[alexbohm]

I had a very similar issue with the BNO055 and locally increasing the timeout to 20 did solve the issue in my case.

[TheButlah]

What is the maximum that I can configure the timeout to be that is supported by the hardware? Could the hal support something in software (but still use the hardware i2c as much as possible) if I need to go longer than that?

[bjoernQ]

The max timeout is chip dependent but I think they can be configured to quite high values, e.g. on C3 "The value of I2C_SCL_ST_TO_I2C should be less than or equal to 22, which means SCL_FSM could remain unchanged for 222 I2C_SCLK clock cycles at most before the interrupt is generated."

I don't think there is a sane way to do it in a more software-controlled way

[TheButlah]

esp-idf supports values quite large, at least as best as I can tell:

How is it done in esp-idf? Maybe we could add similar functionality here. What would the maximum tick value in esp_idf equate to in us?

[TheButlah]

The max timeout is chip dependent but I think they can be configured to quite high values, e.g. on C3 "The value of I2C_SCL_ST_TO_I2C should be less than or equal to 22, which means SCL_FSM could remain unchanged for 222 I2C_SCLK clock cycles at most before the interrupt is generated."

I don't think there is a sane way to do it in a more software-controlled way

Oooh, I read this as 222 clock cycles, which didn't seem like enough. But the datasheet actually says this:

So yeah 2^22 clock cycles is a lot. Should be plenty.

I did some testing locally on arduino and using the Wire.setTimeOut() function to increase the clock stretching limit does seem to be an important part of getting the bno080 to work.

What are the next steps to add this functionality? I'm still probably not the best person to implement this, but if no one has bandwidth I can try :)

[bjoernQ]

The ticks_to_wait doesn't look like it's the same thing: https://github.com/espressif/esp-idf/blob/49551cc48cb3cdd5563059028749616de313f0ec/components/driver/i2c.c#L1544-L1547

I think in my pasted text GH made the 2^22 into 222 - so yes 2^22 is a lot

We could add an optional parameter to new but that is a breaking change (not that much of a problem for the next release since that's a breaking release anyway - don't know when we will publish it but probably soon because of that riscv crate which got yanked) or add a second constructor which allows specifying the timeout. Then pass the value down to set_fequency which is very chip specific unfortunately (so we have a few of them).

I probably would prefer the second constructor but don't have a very strong opinion on that.

Would be awesome if someone who actually runs into that problem would implement it (e.g. I myself don't own any I2C device that needs it - it's certainly possible to use a second MCU to test it but might not be ideal)

[TheButlah]

Part of the problem is that the clock stretching is only one factor in why the bno080 driver seems to fail in rust. But I could give it a try!

No promises though. And feel free if anyone sees this to work on it anyway, since I'm assuming I'm gonna struggle bus for a while here. This would be my first non-trivial contribution to the hal, I believe

[TheButlah]

@bjoernQ I was looking into this. The tout value in esp-hal and the i2c_set_timeout function in esp-idf both use the I2C_TO_REG (0x000C) not the I2C_SCL_ST_TIME_OUT_REG (0x0078).

Furthermore the datasheet shows that these two registers are doing slightly different things with either the SCL or SCL_FSM.

Im not sure which register is the correct one to use. What was the rationale for suggesting the use of the I2C_SCL_ST_TIME_OUT_REG instead of the one that the hal and esp-idf use? Should I switch from the the other register to using the one you suggested?

[TheButlah]

So I investigated more. It looks like there are two separate timers. Each timer has a different interrupt, and a different range of values.

The first timer is controlled by I2C_TIME_OUT_VALUE (0x0C), and can range from [0, 2**5) APB clock
cycles. It is set here. This triggers the I2C_TIME_OUT_INT interrupt, which is checked to abort the i2c operation if it is fired. I see where we disable the interrupt but couldn't find where we enable it though.

In esp-idf, the i2c_set_timeout() function calls i2c_ll_set_tout() to configure the timeout. The memory address is defined in this struct. Its the third union, so 3xu32 values before it means its the 12th==0x0C byte, which is indeed the address of I2C_TIME_OUT_VALUE relative to the base address of the register block.

What is interesting is that the comment on i2c_ll_set_tout() says that the tout parameter sets the timeout to 2**tout APB clock cycles. That this contradicts what I discovered above - the technical reference states that the time is not exponential in the value of the register. I.E. the max number of cycles is not 2**N but rather just N. I'm going to assume that the technical reference is correct, and esp-idf should revise the comment.

The other counter that is useful but to my knowledge not used by either esp-idf or esp-hal is the on that @bjoernQ mentioned is I2C_SCL_ST_TO_I2C (0x78) and can range from [0,23]. This will trigger an interrupt when 2**I2C_SCL_ST_TO_I2C I2C_SCLK clock cycles have elapsed. Presumably, this has a less precise but far longer wall time than the other interrupt timer.

My questions are:

1. Which timer should we be using? I think we must choose between the high range (I2C_SCL_ST_TO_I2C) or high precision (I2C_TIME_OUT_VALUE) timers, right?
2. Does esp32 have those same two timers? Or does it only have the high precision one, but it has more bits to work with? Are both timer types available on all esp32 family devices? I only read the datasheet for the esp32c3.
3. Is my understanding about the comment in esp-idf correct/should I open an issue there to have them confirm its wrong and have them change it?
4. Given this information about tout being non-exponential, does this code still make sense? It looks like it should just be set to a whole number, since its not exponential.