Skip to content

Conversation

@Gadgetoid
Copy link

I've split this changeset from #4378 so the two tangentially related things aren't muddied together, and so that #4378 is no longer blocked by this somewhat more controversial (and far reaching) change.

I know the i2c pullups on the RP2040 are a little on the weak side for i2c, but I've got tested prototypes that appear to be reliable with them. There's some related discussion on #4378.

This change may or may not be worthwhile or useful to someone, but with the issues bitbangio presents - not respecting the pull state of IO pins - it's currently incomplete and thus raised as a draft.

It was originally written to save a batch of boards, but I believe we've decided it's better to cut our losses and make the hardware work as designed.

The I2C.c for RP2040 included a special case for writes <=2 bytes to match the MicroPython implementation,
however RP2040 does support 1 and 2 byte reads, with only 0 bytes being the exception.

Signed-off-by: Philip Howard <phil@pimoroni.com>
* CIRCUITPY_BUSIO_I2C_INTERNAL_PULLUPS config option to enable/use internal i2c pull-ups.
* CIRCUITPY_BUSIO_I2C_INTERNAL_PULLUPS_AVAILABLE to indicate availability of internal pull-up resistors.
* Add a check to ensure onboard pullups are available when requested.
* Enable onboard pullups for pimoroni_keybow2040
* Indicate onboard pullups are available on Pico
* NB: Onboard pullups are weak and not ideal
@Gadgetoid Gadgetoid force-pushed the patch-rp2-onboard-i2c-pullups branch from 390ee79 to 29bf9e7 Compare March 25, 2021 22:36
@hierophect hierophect added the rp2040 Raspberry Pi RP2040 label Mar 27, 2021
@dhalbert
Copy link
Collaborator

@Gadgetoid Since you are not going to use this after all, I will just close this to keep our pending PR list clean. If you think it should remain open, I'm happy to reopen.

@dhalbert dhalbert closed this Mar 29, 2021
@Gadgetoid
Copy link
Author

Sorry I'd stopped juggling this since we'd resigned to including pull-up resistors on Keybow 2040 for i2c, but I believe it will still be required.

We'll be shipping some Breakout Garden products which - unlike Pico Explorer - do not include onboard pull-up resistors (or, indeed, any components other than headers) which might rely on this change.

Our in-house breakouts should be fine since they include onboard pull-up resistors, but since we've shipped Qw/St adapters there may be cases where our pullupless boards might be combined with pullupless sensors and I'd like to have an answer when that inevitably happens.

@tannewt
Copy link
Member

tannewt commented May 14, 2021

Our in-house breakouts should be fine since they include onboard pull-up resistors, but since we've shipped Qw/St adapters there may be cases where our pullupless boards might be combined with pullupless sensors and I'd like to have an answer when that inevitably happens.

Adafruit also puts pull ups on the breakouts. It comes up occasionally but most folks can handle adding pull ups themselves or getting a better breakout. I wouldn't worry about it. Though, if you do want to add it, we'd accept the PR.

@lesamouraipourpre
Copy link

I'd be in favour of adding this.

It's confusing moving from the Pimoroni Micropython firmware which doesn't need resistors to CircuitPython which does. I'm using the Pimoroni Scroll Pack which just plugs into the RP2040 Pico. Bodging two resistors when switching to CircuitPython feels kludgy.

@Gadgetoid Was this added by Pimoroni to their firmware or is it part of Micropython?

@Gadgetoid
Copy link
Author

@lesamouraipourpre our drivers on the MicroPython side use the Pico I2C directly, configuring the pins with pull-ups IIRC.

I need to revisit, rebase and rewrite this PR since it's clearly still an issue and I don't think it necessarily hurts to include it. Thank you for weighing in.

@thymjan
Copy link

thymjan commented Jul 16, 2022

Struggled 3h with this topic and landed here. It's the first disappointing experience with CircuitPython for me and I'm confused this topic still has no software solution.
Things mentioned above I discovered for myself as workarounds, each with no success.

I'm tinkering around with the new PiSquare board form sb.components which has an oled display on board.
It was a pleasure to give the dragino LoRa/GPS HAT a new live.
Now I tried to activate the oled display and I'm failing all the time.
CircuitPythons complains about the missing pullups and I can't find an easy workaround.
This issue should have an easy solution. The given error should give you an advise how to solve the problem easily, should it?

@dhalbert
Copy link
Collaborator

The internal pullups on an RP2040 are quite weak (50k-80k) and not very good for I2C. All of the I2C breakouts Adafruit makes have on-board pullups. We don't add pullups to the microcontroller boards.

It looks like (https://cdn.shopify.com/s/files/1/1217/2104/files/Schematic_9f818051-fdfc-43dd-9e5e-0999f1dc1d1c.pdf?v=1657287360) GPIO7 and GPIO6 are used for I2C. Since those pins are available on the 2x20 header, I think you could add some external pullups from those pins to 3.3V without too much work.

@thymjan
Copy link

thymjan commented Jul 17, 2022

I've already made a little shim:
IMG_0230
This is working. But I don't get the point why there is no option in CircuitPython to activate the internal pullups for I2C.
With MircoPython I can use the onboard display without the external pullups.

@dhalbert
Copy link
Collaborator

@thymjan This PR was for compile-time support. There was some other work for run-time specification of using internal pullups: #5718 and #5724. That work stopped because the user's need became moot.

There are several considerations about doing which have not pushed it over the threshold of inclusion.

  1. We'd want to make it available for all ports. So it's a noticeable amount of work.
  2. It takes extra code space, which is in very short supply on the small SAMD21 builds.
  3. Its use cases tend to be limited; usually in cases like this it would have been better to add some pull-ups on the hardware.
  4. The pullups available can be sub-optimal. As mentioned the pullup values on the RP2040 are substantially out of spec for most I2C devices.

@thymjan
Copy link

thymjan commented Jul 23, 2022

@dhalbert Thank you for your detailed reply. So there is no quick & dirty possibility to manipulate the registers in the RP2040 and switch on the pullups for I2C at runtime, am I right?

@dhalbert
Copy link
Collaborator

It would not be hard to add a couple of calls to enable internal I2C pullups on the RP2040. In fact it was already done in another PR I forgot to mention that was not merged, because the reason became moot. See https://github.com/adafruit/circuitpython/pull/4488/files#, lines 127-128 on the right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rp2040 Raspberry Pi RP2040

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants