Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix I2C hang when power removed from pull-ups #8827

Merged
merged 3 commits into from
Jan 25, 2024

Conversation

furbrain
Copy link

@furbrain furbrain commented Jan 23, 2024

This PR fixes #2253 and fixes #8093.

Explanation

This works by creating the nrfx_twim device with a callback (twim_event_handler).
When the callback is triggered it marks that the transfer has finished and records any error/success code in the twim_peripheral data structure.
We replace the nrfx_twim_xfer function with our _twim_xfer_with_timeout which make a note of the start time, and waits until either the callback has triggered or 1s has elapsed.
If the timeout is triggered it generates an OSError: Timed out

Trying to use the i2c bus again after power restoration fails (generates an OSError), but it can be deinited and reinited and it then works

Sample Code

The following code now works (adapted from example in #8093:

import board
import busio
import time
from adafruit_lsm6ds.lsm6ds3 import LSM6DS3
import digitalio

# board.IMU_PWR provides the power to both the LSM6DS3 chip and also the pullups to the I2C bus
pwr = digitalio.DigitalInOut(board.IMU_PWR)
pwr.switch_to_output(True)
print("Starting")
time.sleep(5)
print("Connecting to IMU")
with busio.I2C(scl=board.IMU_SCL, sda=board.IMU_SDA) as i2c_bus:
    imu = LSM6DS3(i2c_bus, address=0x6A)
    print("This works")
    print(imu.acceleration)
    time.sleep(1)
    pwr.switch_to_output(False)
    print("This no longer crashes")
    try:
        print(imu.acceleration)
    except OSError:
        print("Error appropriately raised")
    else:
        print("Shouldn't get here")
        
    time.sleep(0.5)
    print("turn power back on")      
    pwr.switch_to_output(True)
    time.sleep(0.5)
    # trying to access the i2c bus again results in a further error - needs to deinit and reinit
    
with busio.I2C(scl=board.IMU_SCL, sda=board.IMU_SDA) as i2c_bus:
    print("Try again")
    imu = LSM6DS3(i2c_bus, address=0x6A)
    print("This works")
    print(imu.acceleration)

@furbrain
Copy link
Author

It also reduces the amount of data sent in one transaction to 1024 bytes. Longer transfers will still happen, they are just chunked.

@tannewt tannewt requested a review from dhalbert January 24, 2024 00:04
@dhalbert
Copy link
Collaborator

@furbrain I was going to test this, and first tried to reproduce the old problem from #2253. However, I can't seem to get the test suggestions there to work: unpowered LSM9DS1 on an Arduino Nano 33 BLE, or an unpowered LIS3DH attached to a Feather nRF52840 with a resistor between SCL and SDA. In both cases CircuitPython 8.2.9 complains that SCL/SDA pull-ups are missing.

Did you test this against some hw configuration you knew would cause the problem? Thanks.

@dhalbert
Copy link
Collaborator

Never mind -- I should have read your OP more carefully! I need to apply and then remove power.

@furbrain
Copy link
Author

I tested it against my current use case - i2c device is on the board, but is powered by a dedicated pin. I'd you turn the pin on and then connect the bus, everything works fine until you unpower the bus and it crashes

This is also the case if you are running an I2C display on separate per and later remove the power. Because the display gets repurposed as a console it is never properly shut down and you get a hang when you try to turn off the power to it

@furbrain
Copy link
Author

Just tested my code in the PR on stock 9.0.0-alpha.6-34 and it still hangs

Copy link
Collaborator

@dhalbert dhalbert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested with a Feather nRF52840 and an LIS3DH. I started collecting data from the LIS3DH and then disconnected its power. Before this fix, CircuitPython hangs. After the fix, I get an ETIMEDOUT, as expected.

Thanks very much for the fix. Using the callback is cleaner than having to modify nrfx.

@dhalbert dhalbert merged commit 53020f5 into adafruit:main Jan 25, 2024
65 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

I2C hangs when power removed from bus on nrf52840 nrf I2C hang
2 participants