gh-raspberrypi/pico-sdk

Watch

Fork 0

i2c_read_timeout_us causing lockup #2311

New issue

Open

opened 2025-02-22 14:59:28 -06:00 by mitchellcairns · 8 comments

mitchellcairns commented

2025-02-22 14:59:28 -06:00

(Migrated from github.com)

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

I've updated [my gamepad library](https://github.com/HandHeldLegend/HOJA-LIB-RP2040) 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 commented

2025-03-03 05:09:55 -06:00

(Migrated from github.com)

What hardware do we need to reproduce these issues?

peterharperuk commented

2025-03-03 08:42:51 -06:00

(Migrated from github.com)

@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'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 commented

2025-03-03 14:45:40 -06:00

(Migrated from github.com)

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

mitchellcairns commented

2025-03-03 15:29:56 -06:00

(Migrated from github.com)

@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
github.com/raspberrypi/pico-sdk@ee68c78d0a/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)

@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 commented

2025-03-03 15:36:42 -06:00

(Migrated from github.com)

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 commented

2025-03-04 05:58:56 -06:00

(Migrated from github.com)

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!

> 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!

👍 1

mitchellcairns commented

2025-08-06 14:32:50 -05:00

(Migrated from github.com)

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

mitchellcairns commented

2026-07-06 23:51:47 -05:00

(Migrated from github.com)

On the next I2C write to the ESP32, it only pushes one byte, then spins until the write timeout occurs.

static void i2c_drain_abort(i2c_hw_t *hw)
{
    (void) hw->clr_tx_abrt;
    if (hw->raw_intr_stat & I2C_IC_RAW_INTR_STAT_STOP_DET_BITS) {
        (void) hw->clr_stop_det;
    }
}

If this code is called after the SDK write function as-is with the latest SDK version, this is cleared up.

I've done a bit more digging into this. It seems like, in the case of the ESP32 slave I2C driver, even if you are accepting, for example, 32 bytes, the final byte may be accepted, but NAK'd to the host. This leaves the RP2040 in a sort of fail state. On the next I2C write to the ESP32, it only pushes one byte, then spins until the write timeout occurs. ```c static void i2c_drain_abort(i2c_hw_t *hw) { (void) hw->clr_tx_abrt; if (hw->raw_intr_stat & I2C_IC_RAW_INTR_STAT_STOP_DET_BITS) { (void) hw->clr_stop_det; } } ``` If this code is called after the SDK write function as-is with the latest SDK version, this is cleared up.