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

Espressif: SPI.deinit() waits for SPI lock to unlock, and cannot be interrupted #9586

Open
dhalbert opened this issue Sep 1, 2024 · 2 comments
Labels
bug busio espressif applies to multiple Espressif chips
Milestone

Comments

@dhalbert
Copy link
Collaborator

dhalbert commented Sep 1, 2024

As of #9164 (MAX3421E support), SPI.deinit(), only on Espressif, waits for the lock to be unlocked before proceeding with the deinit(). If there is nothing else that is going to unlock the lock, CircuitPython hangs and cannot be interrupted.

    // Wait for any other users of this to finish.
    while (!common_hal_busio_spi_try_lock(self)) {
        RUN_BACKGROUND_TASKS;
    }

This caused adafruit_dotstar.DotStar.deinit() to hang forever. DotStar grabs the lock when constructed and never lets it go, since it's not sharing it with anyone else.

No other port implementation of SPI.deinit() waits for the lock to be unlocked.

@tannewt Was waiting for the lock necessary for MAX3421E, or should deinit() ignore the lock? If the former, should this at least be interruptible by ctrl-C? Should other ports wait for the lock also, since they don't right now? In comparison, no I2C implementations of deinit() , including espressif, wait for the lock.

I'll fix adafruit_dotstar so it is a better citizen about unlocking, but this is also a core problem.

@dhalbert dhalbert added espressif applies to multiple Espressif chips busio bug labels Sep 1, 2024
@dhalbert dhalbert added this to the 9.1.x milestone Sep 1, 2024
@tannewt
Copy link
Member

tannewt commented Sep 3, 2024

Was waiting for the lock necessary for MAX3421E, or should deinit() ignore the lock?

I switched to a proper lock for ESP32 because the TinyUSB task is run in a different FreeRTOS thread. I think it'd be ok to check ctrl-c but we need to think about the case where another task is using the bus when deinit is called.

@tannewt
Copy link
Member

tannewt commented Sep 11, 2024

Switching to 10.0. We should make all implementations of bus locking ensure that it is unlocked before deinit. For now, DotStar does the right thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug busio espressif applies to multiple Espressif chips
Projects
None yet
Development

No branches or pull requests

2 participants