i2c_read_timeout_us causing lockup

[mitchellcairns]

I've updated my gamepad library to utilize Pico SDK 2.1.0, and today 2.1.1, and both have a serious issue with I2C making it unusable. I have a loop communicating with a battery PMIC as well as an ESP32. With the new library functions that underly these (i2c_write_blocking_internal && i2c_read_blocking_internal), they are hogging way more CPU time than would be determined by the set timeout. I wrote a custom wrapper which re-implements the 1.5.1 I2C functions and the issues went away.

I see a similar issue dating back to 2023 and I'm hoping that there is a fix on the horizon, as this would severely impact anyone else trying to use 2.1.0 and 2.1.1 https://github.com/raspberrypi/pico-sdk/issues/1471

[peterharperuk]

What hardware do we need to reproduce these issues?

[peterharperuk]

@mitchellcairns I'd appreciate any advice you can offer on reproducing the problems you're seeing with the sdk.

I've looked at the changes since 1.5.1 and the only thing that looks interesting is the following code added to the "for (byte_ctr = 0; byte_ctr < ilen; ++byte_ctr)" loop in i2c_write_blocking_internal and i2c_read_blocking_internal

        if (timeout_check) {
            timeout_check(ts, true); // for per iteration checks, this will reset the timeout
        }

I can't really see how that could cause the problems you're seeing however.

[mitchellcairns]

I will try to see if there's a basic setup I can provide which would help to reproduce this issue

[mitchellcairns]

@peterharperuk I have been able to narrow down the issue further.

The issue is isolated to i2c_read_blocking_internal and not i2c_write_blocking_internal.

The clr_tx_abrt is only referenced within a condition where the TX_ABRT_BIT is set https://github.com/raspberrypi/pico-sdk/blob/ee68c78d0afae2b69c03ae1a72bf5cc267a2d94c/src/rp2_common/hardware_i2c/i2c.c#L303

Adding this line abort = (bool) i2c->hw->clr_tx_abrt; outside of that if statement resolves the issue I'm having. Is it possible that the abort interrupt isn't firing under some condition?

Additionally, I noticed that the timeout_check is now reset for every byte that is being attempted. This basically changes the timeout_us to be timeout_us * transaction size (which doesn't seem intentional, or at least should be an option and not the default in my opinion)

[mitchellcairns]

In my context, I am writing data to an ESP32 in I2C slave mode using the RP2040. From what I understand, the ESP32 in I2C slave mode may never transmit a NAK packet (which is not normal behavior) but this should still not stop the i2c timeout from passing on as we would expect. For whatever reason, adding that original line back in causes this to work as intended.

[peterharperuk]

I noticed that the timeout_check is now reset for every byte that is being attempted.

My reading of the code is that this only happens when using i2c_read_timeout_per_char_us which ends up calling check_per_iteration_timeout_us. Other functions should call check_single_timeout_us which does not reset the timeout.

adding that original line back in causes this to work as intended.

Thanks - that gives me something to think about!

[mitchellcairns]

Is there any updates on this front? I saw 2.2.0 drop but I did not see any notes on this.