Raspberry Pi Pico I2C issue with TCS34725

[caternuson]

Re this thread:
https://forums.adafruit.com/viewtopic.php?f=60&t=174668

Maybe some weird edge case for this hardware combo? Used an Itsy M4 as a sanity check and it works fine there. Tested via REPL doing essentially the same I2C transaction as the library.

RPi pico

Adafruit CircuitPython 6.2.0-beta.1 on 2021-01-27; Raspberry Pi Pico with rp2040
>>> import board
>>> import busio
>>> i2c = busio.I2C(board.GP27, board.GP26)
>>> i2c.try_lock()
True
>>> buffer = bytearray(2)
>>> buffer[0] = 0x01 | 0x80
>>> buffer[1] = 100
>>> i2c.writeto(0x29, buffer)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
OSError: [Errno 19] Unsupported operation
>>> 

Itsy M4

Adafruit CircuitPython 6.1.0 on 2021-01-21; Adafruit ItsyBitsy M4 Express with samd51g19
>>> import board
>>> i2c = board.I2C()
>>> i2c.try_lock()
True
>>> buffer = bytearray(2)
>>> buffer[0] = 0x01 | 0x80
>>> buffer[1] = 100
>>> i2c.writeto(0x29, buffer)
>>>  

[dhalbert]

I tested the Pico with a BME280, LIS3DH, and BNO055 (uses clock stretching), and don't see issues. So the problem seems fairly specific to this I2C device. I'll order one to check.

EDIT: also tested with TSL2561 and TSL2591, which are by the same manufacturer. No problems.

[ladyada]

@dhalbert ok you were able to replicate this issue? ill take it off my todo list if so

[dhalbert]

@ladyada I confirmed that there's an issue. TCS34725 does not respond, and also takes down the I2C bus so that other devices on the bus are prevented from responding: I added an LIS3DH, which could not be found until I removed the TCS34725.

Notes: @ladyada says this may very well be due to the unusual way we probe for the device on the RP2040. We do a zero-length read; normally we would do a zero-length write, but the pico SDK does not provide for that. (I believe that is right, based on the code, but I might be mis-remembering.)

[tannewt]

It's the peripheral itself that can't do a zero length write. To do it ourselves we'd need to bitbang it.

[kevinjwalters]

Possibly same thing discussed in #4235

[dhalbert]

Interim notes:

I am seeing this same NACK issue that the Saleae screenshots above show. I have a fix for bitbangio.I2C for #4221, and for busio short writes for #4202. When I use bitbangio.I2C for the TCS34725, the sensor works properly. The Saleae traces for bitbangio vs busio do not seem to show any notable differences, but, still, the busio traces show a NACK, and the bitbangio ones show an ACK for a simple write. It may be due to my fix for #4202 not being exactly right yet,but it seems more like something that the TCS34725 is doing.

[dhalbert]

Moving to 6.x.x bug fixes since using bitbangio.I2C is now a workaround.

[fivdi]

It's possible for me to fix this problem in the pico-sdk by adding the following line of code

    i2c->hw->sda_hold = 2;

directly after this line of code

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

This modification sets the SDA hold time during transmit (IC_SDA_TX_HOLD) to two ic_clk clock cycles rather than leaving it at the default of one ic_clk clock cycle.

I can't explain why this fixes the the problem because according to page 5 of the TCS34725 data sheet the minimum value for t(HDDAT), the SDA hold data time, is 0 microseconds.

After this modification, the bus_scan application in pico-examples functions correctly and can detect a TCS34725 at address 0x29. It's also possible to successfully read the Device ID register using the pico-sdk.

[jepler]

@fivdi A very interesting find! Would you be able to submit a Pull Request to make that change? I glanced at the datasheet and I'm not sure what negative consequence there could be to this change, except possibly at higher I2C data rates.

[dhalbert]

Thanks for the excellent sleuthing!

[fivdi]

@fivdi A very interesting find! Would you be able to submit a Pull Request to make that change? I glanced at the datasheet and I'm not sure what negative consequence there could be to this change, except possibly at higher I2C data rates.

@jepler the PR is at raspberrypi/pico-sdk#273

Thanks for the excellent sleuthing!

@dhalbert you're welcome

[kevinjwalters]

@caternuson Can you measure the frequency and duty cycles of both clocks? They both look like they are more like 95kHz than 100kHz and the Pico seems to spend more time high than low. Is the pull up story the same for both including any s/w enabled pull-ups in the respective processors?

Perhaps it's worth a look in the analogue world to see what's really going on.

And does this misbehave on other pin pairs on the pico, e.g. GP1 GP0 with matching nearby GND?

[tannewt]

@fivdi Want to PR here as well? https://github.com/adafruit/pico-sdk We can then merge this into CP sooner than upstream has it.

[caternuson]

@kevinjwalters I don't have this setup anymore. I could if it would help, but it looks like maybe a fix has been found.

[kevinjwalters]

@caternuson I captured a few traces of a Pi Pico talking to an SSD1306 and they look different to yours. I put in #4466 for it as the clock speed looks signficantly wrong and i2c "style" changes based on write length...

Might be worth testing the TCS34725 with native i2c at 100kHz before going ahead with proposed change as @fivdi noted that the change doesn't appear strictly necessary based on the data sheet. Or perhaps this has already been done if someone has some independent C code to test against TCS34725 ?

[fivdi]

@fivdi Want to PR here as well? https://github.com/adafruit/pico-sdk We can then merge this into CP sooner than upstream has it.

@tannewt ok. Against which branch of https://github.com/adafruit/pico-sdk?

[fivdi]

Might be worth testing the TCS34725 with native i2c at 100kHz before going ahead with proposed change as @fivdi noted that the change doesn't appear strictly necessary based on the data sheet.

@kevinjwalters The tests described in the PR were performws with native I2C at 10, 100 and 400kHz.

[tannewt]

@fivdi Want to PR here as well? https://github.com/adafruit/pico-sdk We can then merge this into CP sooner than upstream has it.

@tannewt ok. Against which branch of https://github.com/adafruit/pico-sdk?

Good question! I just made a circuitpython branch to track what we're using in CP. Thanks!