adafruit / circuitpython

CircuitPython - a Python implementation for teaching coding with microcontrollers
https://circuitpython.org
Other
4.1k stars 1.22k forks source link

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

Open dhalbert opened 2 months ago

dhalbert commented 2 months ago

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.

tannewt commented 2 months ago

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 commented 1 month ago

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.