drivers: i2s: esp32: a failed TX start loses the mem-slab block it already dequeued
bugpriority: lowplatform: ESP32area: I2S
## Describe the bug
`i2s_esp32_tx_start_transfer()` takes a block off the TX queue and records it in `stream->data->mem_block` before it starts the DMA:
https://github.com/zephyrproject-rtos/zephyr/blob/697b32395414c8f11a3199479dccf49b9338a1aa/drivers/i2s/i2s_esp32.c#L544-L550
If starting the DMA then fails, it returns `-EIO` without returning the block to the mem-slab, without putting it back on the queue, and without clearing `mem_block`:
https://github.com/zephyrproject-rtos/zephyr/blob/697b32395414c8f11a3199479dccf49b9338a1aa/drivers/i2s/i2s_esp32.c#L559-L563
`transferring` is never set either, so nothing downstream treats the stream as owning anything. The next `I2S_TRIGGER_START` calls this function again, which dequeues a fresh item and overwrites `mem_block`, and the first block is gone for good. Repeat the failure and the slab drains one block at a time.
In `I2S_DIR_BOTH` the rollback does not catch it. When RX started and TX failed, `rx.transferring` is true and `tx.transferring` is still false, so only the first arm runs and the TX side is never stopped or unwound:
https://github.com/zephyrproject-rtos/zephyr/blob/697b32395414c8f11a3199479dccf49b9338a1aa/drivers/i2s/i2s_esp32.c#L627-L637
## Steps to reproduce
This comes from reading the driver, not from a captured run. I have not reproduced it on hardware, so the sequence below is the path through the code rather than a measurement.
1. Queue a block and call `i2s_trigger(dev, I2S_DIR_TX, I2S_TRIGGER_START)` under a condition that makes `i2s_esp32_start_dma()` fail, for example a `dma_config()` rejection.
2. `START` returns `-EIO` and the block is neither freed nor requeued.
3. Read `k_mem_slab_num_free_get()` on the TX slab and it is one lower than before.
4. Retry `START` and the count drops again.
## Impact
Functional Limitation
## Environment
- Zephyr `main` at `697b3239`
- ESP32-S3, the `SOC_GDMA_SUPPORTED` path, though the code is common to both paths
## Additional Context
Found while preparing #113311, which does not change any of the code above.
One interaction worth recording. With #113311 applied, an application that calls `I2S_TRIGGER_DROP` after a failed start does get the block back, because the stop path there frees whatever `mem_block` still holds once `dma_stop()` has succeeded. An application that simply retries `START` still loses it, so that is a mitigation rather than a fix.
I first wrote that freeing the block or pushing it back onto the queue was a design choice. It is not, and I should have looked one function up before saying so. `i2s_esp32_rx_start_transfer()` already meets the same failure and frees the block, clears `mem_block` and `mem_block_len`, and returns `-EIO`:
https://github.com/zephyrproject-rtos/zephyr/blob/697b32395414c8f11a3199479dccf49b9338a1aa/drivers/i2s/i2s_esp32.c#L353-L360
So this is an asymmetry rather than an open question, and TX only has to do what RX already does. Pushing the block back onto the queue would not be equivalent anyway, since `k_msgq_put()` appends and would reorder the stream.
Correcting one more thing I overstated above: once TX cleans up after itself, the `I2S_DIR_BOTH` rollback needs no change. With RX started and TX failed, `rx.transferring` is true and `tx.transferring` is false, so the arm that stops RX is exactly the one that runs. The reverse case cannot occur, because `i2s_esp32_start_transfer()` starts RX first and skips TX entirely if RX fails.
0 条评论