← index #14339PR #15585
Duplicate · high · value 2.264
QUERY · ISSUE

[RP2] machine.soft_reset() hangs when i2s is not deinitialized

openby mareclopened 2024-04-20updated 2024-08-07
bug

Checks

  • I agree to follow the MicroPython Code of Conduct to ensure a safe and respectful space for everyone.

  • I've searched for existing issues matching this bug, and didn't find any.

Port, board and/or hardware

RPI_PICO

MicroPython version

MicroPython v1.22.2 on 2024-02-22; Raspberry Pi Pico with RP2040

Reproduction

  1. Initialize I2S device:
from machine import I2S
from machine import Pin

audio_out = I2S(0,
	sck=Pin(0), ws=Pin(1), sd=(2),
	mode=I2S.TX,
	bits=16,
	format=I2S.MONO,
	rate=44100,
	ibuf=20000)
  1. Call machine.soft_reset()
  2. Repeat 1, call audio_out.deinit() and no. 2 again

Expected behaviour

machine.soft_reset() resets machine
obraz

Observed behaviour

device hangs, requiring hardware reset or reconnecting USB
obraz

Additional Information

Verified on latest git commit and official release 1.22.2. This issue is pin-independent.

CANDIDATE · PULL REQUEST

ports/rp2: Add i2s deinit on soft reset.

closedby Gadgetoidopened 2024-08-02updated 2024-08-07
port-rp2

Summary

Code using i2s required a try/finally in order to avoid a hard fault on soft reset.

Add machine_i2s_deinit_all to teardown any active i2s instances on reset.

Fixes #14339

Probably replaces #14345

Testing

I've "fixed" and tested the RP2 port, using our TinyFX/picofx libraries and confirmed that subsequent to this change, forgetting to explicitly deinit() your i2s instance will not result in a burst of static and a hardfault.

Trade-offs and Alternatives

As far as I'm aware, this bring i2s in line with other interfaces such that they're torn down automatically on soft reset.

I don't believe these are any cases where you wouldn't want to do this, but obviously doing so automatically comes at a small code size cost.

It's arguable that this should be done (as mentioned in #14339) with a finalizer, but there are some issues around that topic which would take much longer to work out than simply adding this method.

Even executing all finalizers first does not guarantee they happen in the right order, so since we're using MP_STATE_PORT to track active i2s instances, we might as well be nice and clean them up first.

Keyboard

j / / n
next pair
k / / p
previous pair
1 / / h
show query pane
2 / / l
show candidate pane
c
copy suggested comment
r
toggle reasoning
g i
go to index
?
show this help
esc
close overlays

press ? or esc to close

copied