I2C: Strange behaviour trying to read PA1010D GPS

[Gadgetoid]

I've been testing the PA1010D i2c GPS (happy to supply one to anyone who's got a head for mysteries) with the Pico, starting with CircuitPython and reducing down to a minimum amount of C++ SDK code to reproduce.

Long story short, it will stall hard after about one i2c read using Pico's hardware I2C.

Using either CircuitPython bitbangio or the I2C PIO driver from the Pico Examples it will read perfectly happily and - indeed - in the same setup for testing this I've had valid GPS data pouring out using a CircuitPython example and bitbangio in lieu of hardware i2c.

Works:

#include <stdio.h>
#include "pico/stdlib.h"
#include "pio_i2c.h"

#define PIN_SDA 4
#define PIN_SCL 5

int main() {
    stdio_init_all();

    PIO pio = pio0;
    uint sm = 0;
    uint offset = pio_add_program(pio, &i2c_program);
    i2c_program_init(pio, sm, offset, PIN_SDA, PIN_SCL);

    uint addr = 0x10;

    while (1) {
        int ret;
        uint8_t rxdata[11];
        ret = pio_i2c_read_blocking(pio, sm, addr, rxdata, 10);
        rxdata[10] = '\n';
        printf(rxdata);
        sleep_ms(1000);
    }
    return 0;
}

Stalls:

#include <stdio.h>
#include "pico/stdlib.h"
#include "hardware/i2c.h"

int main() {
    stdio_init_all();

    i2c_init(i2c0, 100 * 1000);
    gpio_set_function(4, GPIO_FUNC_I2C);
    gpio_set_function(5, GPIO_FUNC_I2C);
    gpio_pull_up(4);
    gpio_pull_up(5);

    int addr = 0x10;

    while (1) {
        int ret;
        uint8_t rxdata[11];
        ret = i2c_read_blocking(i2c0, addr, rxdata, 10, false);
        rxdata[10] = '\n';
        printf(rxdata);
        sleep_ms(1000);
    }
    return 0;
}

I've played with the i2c baud rate to no avail. The hardware i2c seems to inevitably get stuck in this loop:

        do {
            abort_reason = i2c->hw->tx_abrt_source;
            abort = (bool) i2c->hw->clr_tx_abrt;
            if (timeout_check) {
                timeout = timeout_check(ts);
                abort |= timeout;
            }
        } while (!abort && !i2c_get_read_available(i2c));

CircuitPython suggested that either Data or Clock was pulled low, preventing the detection of i2c pull-ups once this state is entered. Using C++ to sleuth this further suggests that SDA is held low. It would seem to be the GPS itself that's entered a bad state and is holding SDA low. Why Pico's hardware i2c would cause it to do this totally eludes me. Unplugging the GPS and re-inserting it seems to clear this state.

Weirder still. If I get the GPS into a bad state whereupon it's totally unresponsive to hardware I2C, running the PIO I2C example will get it working again. I've tried this a few times. It's... odd.

And. I'm baffled.

[fivdi]

Something similar was seen with a BME280. See https://www.raspberrypi.org/forums/viewtopic.php?f=144&t=306875.

[fivdi]

Could it have something to do with this:

    // TODO there are some subtleties to I2C timing which we are completely ignoring here

[dhalbert]

I could not duplicate the BME280 issues in CircuitPython, even turning off our bitbangio. I replied in that RPi Forum thread.

[Gadgetoid]

Likewise. I've recently completed a C++ BME280/BMP280 driver for our pimoroni-pico repository and have not run into the issues described on the forums. Full driver is here: https://github.com/pimoroni/pimoroni-pico/blob/patch-bme280-breakouts-dev/drivers/bme280/bme280.cpp

[stackjohn]

Likewise. I've recently completed a C++ BME280/BMP280 driver for our pimoroni-pico repository and have not run into the issues described on the forums. Full driver is here: https://github.com/pimoroni/pimoroni-pico/blob/patch-bme280-breakouts-dev/drivers/bme280/bme280.cpp

@Gadgetoid Absolutely brilliant thank you for this! :smiley:

The issue I was having on the forums seem to be solved now, when writing to the bme280 you need to make sure the parameter here is true to take control of the bus.

int rc = i2c_write_blocking(I2C_PORT, addr, &reg, 1, true);

Also, the bus scan still seems to hang and perhaps leave the bme280 in a strange state. You must disconnect power from the bme280 (and maybe the pico as well?) to reset it into a good state.

Is there a way to set this parameter to true when using the pio i2c implementation?

https://github.com/raspberrypi/pico-examples/blob/master/pio/i2c/pio_i2c.c

[fivdi]

@Gadgetoid I'm going to risk taking another guess here as I don't have a PA1010D to test with.

Is it possible to fix this problem by adding this line of code

    i2c->hw->sda_hold = 2;

directly after this line of code

    i2c->hw->fs_spklen = lcnt < 16 ? 1 : lcnt / 16;

For me, this fixes what might be the same problem with the TCS34725. See also here.

[Gadgetoid]

Interesting hypothesis but I can't see any difference. I really should get a scope on this. I really can't fathom what would be confusing the PA1010D into driving SDA low.

[fivdi]

@Gadgetoid oh well, thanks for checking.

[riozebratubo]

Hi, @Gadgetoid.

I'm having an issue that could be related to this one. I'm trying to mod some arduino libraries that use hardware i2c to work on the raspberry pi pico, and I'm getting read fails if I try to read too much at once, too much being more than 4 bytes.

Could you please try to make your line:

ret = i2c_read_blocking(i2c0, addr, rxdata, 10, false);

Into 3 reads like that:

ret = i2c_read_blocking(i2c0, addr, rxdata, 4, false);
ret = i2c_read_blocking(i2c0, addr + 4, rxdata + 4, 4, false);
ret = i2c_read_blocking(i2c0, addr + 8, rxdata + 8, 2, false);

And see if it has any impact?

Thank you!

[Gadgetoid]

No improvement @riozebratubo ... running step debugging shows my program never reaches the second read, stalling on the first. If anything this might be worse! I think the PA1010D might be uniquely weird in this case though.

Again my basis for comparison - using PIO i2c - works absolutely fine every time... even somehow rescuing the PA1010D from its "stuck" state where it forces SD low and refuses to cooperate.

[riozebratubo]

Thank you for trying it anyway!

[kevinjwalters]

@Gadgetoid The clock stretching feature as implemented by RP2040 (disussed in #4466 ) affects all clock pulses including at fast mode speeds based on logic analyser. This may be a difference to the PIO code? What do they look like on a logic trace?

Hmm, there's a wait in there

https://github.com/raspberrypi/pico-examples/blob/master/pio/i2c/i2c.pio#L53

[Gadgetoid]

@kevinjwalters I'll grab myself a logic analyser (might take a few days) and capture some comparisons.

[dhalbert]

IC_SDA_TX_HOLD_TIME = 5 helps

I was experimenting again with the PA1010D in CircuitPython, and found that if I increase the IC_SDA_TX_HOLD value to 5 or greater, then I can communicate with the PA1010D.

Proposed PR #273 by @fivdi increases IC_SDA_TX_HOLD from the default value of 1 to 2 to make the TCS34725 work. We have also seen that this increase fixes problems with SSD1306 and SH1107 OLED displays.

I originally had added a 1K series resistor (not pull-up) on the SDA line between the PA1010D and the RP2040 in order to see which side was holding SDA low, and with the addition of the resistor it started to work (!). The resistor increases the rise and fall times, so it seemed like increasing the IC_SDA_TX_HOLD would be a step in the same direction.

This testing was done at 100kHz I2C bus speed. The wires are about 20cm long.

This raises the question of what a "reasonable" value of IC_SDA_TX_HOLD is. The datasheet says:

The programmed SDA hold time during transmit (IC_SDA_TX_HOLD) cannot exceed at any time the duration of the low part of scl. Therefore the programmed value cannot be larger than N_SCL_LOW-2, where N_SCL_LOW is the duration of the low part of the scl period measured in ic_clk cycles.

I don't know exactly what the ic_clk value is, but I think it depends on the I2C bus speed, so the TX_HOLD value may need to vary by the bus speed. So I think i2c_set_baudrate() may need to calculate a reasonable value based on the supplied baudrate. I have not yet tried the PA1010D or the other I2C devices at 400kHz instead of the default 100kHz.

EDIT: Tried PA1010D at 400kHz, and 5 works and is necessary (4 still does not work).

[kevinjwalters]

I've been exploring the awesome power of the 1k (pull up) resistor too.

My original query about clock speed appears to relate mostly/entirely to a side-effect of clock stretching feature but I think tlow is out of spec, have a look at https://github.com/raspberrypi/pico-sdk/issues/283#issuecomment-808746340 .

[Gadgetoid]

was experimenting again with the PA1010D in CircuitPython, and found that if I increase the IC_SDA_TX_HOLD value to 5 or greater, then I can communicate with the PA1010D.

Can confirm, adding the following (which I opted to stick at the bottom of i2c_set_baudrate) gets data pouring out of the PA1010D for me:

hw_write_masked(&i2c->hw->sda_hold, 5, I2C_IC_SDA_HOLD_IC_SDA_TX_HOLD_BITS);

This works both at 100kHz and 400kHz. I guess since the value of IC_SDA_TX_HOLD is also measured in terms of ic_clk then a working value should work at any frequency.

[lurch]

I don't know exactly what the ic_clk value is

Section 4.3.1.2. of the RP2040 datasheet says "The I2C controller is connected to clk_sys. The I2C clock is generated by dividing down this clock, controlled by registers inside the block." and section 4.3.14. talks about "IC_CLK Frequency Configuration". And that's about as far as my knowledge goes :wink:

[dhalbert]

This raises the question of what a "reasonable" value of IC_SDA_TX_HOLD is. The datasheet says:

The programmed SDA hold time during transmit (IC_SDA_TX_HOLD) cannot exceed at any time the duration of the low part of scl. Therefore the programmed value cannot be larger than N_SCL_LOW-2, where N_SCL_LOW is the duration of the low part of the scl period measured in ic_clk cycles.

To answer my own question about this, I went through i2c.c by hand and also instrumented it. lcnt is the duration of SCL in ic_clk cycles. For a 125 MHz, clock, an ic_clk cycle is 8 nanoseconds.

For a 100kHz I2C baudrate:

freq_in: 125000000; period: 1250; hcnt: 750; lcnt: 500; spklen: 31

for 400 kHz:

freq_in: 125000000; period: 313; hcnt: 187; lcnt: 126; spklen: 7

So IC_SDA_TX_HOLD has a long way to go before it gets close to lcnt. At 5 is it forcing a 40ns hold, if I understand correctly.

[dhalbert]

I2C pin drive strength experimentation: higher drive did not help

On a whim, I tried increasing the drive strength on the I2C pins to 12ma after setting them to I2C functionality, and then lowering IC_SDA_TX_HOLD back down. That did not help. For the PA1010D, IC_SDA_TX_HOLD = 5 is still necessary even at this highest drive setting.

The change was in in CircuitPython's I2C setup, not in the pico-sdk:

    gpio_set_function(self->scl_pin, GPIO_FUNC_I2C);
    gpio_set_function(self->sda_pin, GPIO_FUNC_I2C);

    // Turn on "strong" pin driving (more current available).
    hw_write_masked(&padsbank0_hw->io[self->scl_pin],
        PADS_BANK0_GPIO0_DRIVE_VALUE_12MA << PADS_BANK0_GPIO0_DRIVE_LSB,
            PADS_BANK0_GPIO0_DRIVE_BITS);

    hw_write_masked(&padsbank0_hw->io[self->sda_pin],
        PADS_BANK0_GPIO0_DRIVE_VALUE_12MA << PADS_BANK0_GPIO0_DRIVE_LSB,
            PADS_BANK0_GPIO0_DRIVE_BITS);

[dhalbert]

I retested the PA1010D at 400kHz, and the increase of IC_SDA_TX_HOLD to 5 is still necessary and sufficient. (4 did not work)

[kevinjwalters]

@dhalbert Have you got traces for the 4 vs 5?

[dhalbert]

@dhalbert Have you got traces for the 4 vs 5?

I have Saleae traces for 1 vs 2 for TCS34725. They are indistinguishable visually (including the analog). I did not zoom in to single-ns resolution. I also have PA1010D traces for SAMD51 (successful) vs RP2040 2 (unsuccessful). Again, no eyeballing of the traces showed anything obvious.

Each increment is only 8ns in the hold time, so it would be hard to see.

[kevinjwalters]

@dhalbert So this value is controlling how long SDA maintains its value after SCL goes low for each data bit? I can only capture at 25Msps. For 100k requested speed my traces do show SDA changes happening 0-1 samples for high to low and 2 for low to high (1k pull-ups plus 10k onboard SSD1306) after SCL goes low. That doesn't seem very conservative. I'll compare with SAMD51 when I get a chance.

[fivdi]

Perhaps the hold time should be 300 ns. See here.

[dhalbert]

We're going to calculate the hold time at run-time in the Adafruit fork of pico-sdk: https://github.com/adafruit/pico-sdk/pull/3. As calculated for the current CPU clock speed, IC_SDA_TX_HOLD is 38.

[Gadgetoid]

It looks like merging #273 will fix this issue. Thank you for this @dhalbert @fivdi. Looks like you've got something working before I could even get my hands on test equipment to properly quantify the problem!

[lurch]

@Gadgetoid #273 has now been merged - does it fix this issue?

[Gadgetoid]

@lurch yes- when I actually check out the develop branch instead of master :roll_eyes:

Is there a way to get notified when develop is merged into master? I'd like to raise a PR or issue requesting MicroPython bump their SDK submodule to sweep in this fix.

[fivdi]

@Gadgetoid If you "Watch" the repository and select "Custom" and then select "Releases" you should get notified when there are new releases. This isn't exactly the same thing but may be sufficient for what you want to achieve.

[Gadgetoid]

@Gadgetoid If you "Watch" the repository and select "Custom" and then select "Releases" you should get notified when there are new releases. This isn't exactly the same thing but may be sufficient for what you want to achieve.

TIL! Thank you. (It's close enough)

[Wren6991]

Seems like this is fixed now, thank you @fivdi for your I2C timing improvements