stm32: i2s_stm32_sai driver does not release the last mem_block

[puliceworx]

Describe the bug

While implementing SAI, DRAIN, STOP, unable to refill men_slab completely causing a NOMEM error.
Diagnosis showed that the last men_block is not freed in HAL_SAI_TxCpltCallback. This occurs on last_block check and no more data check.

Only the previous block is free'd at the end of the callback. On last_block or no more blocks in the queue, free the streams mem_block.

Target: STM32U5xx
Board: nucleo_u575zi_q
Diagnose: Modify sample\drivers\i2s\output to print free blocks after waiting a time after triggering the drain.

Regression

• This is a regression.

Steps to reproduce

Modify the i2s\output sample to loop on free blocks to equal NUM_BLOCKS. This will loop forever.

while(k_mem_slab_num_free_get(&tx_0_mem_slab) != NUM_BLOCKS) {
		
	printf("Waiting for all blocks to be free. %d of %d \n",
		k_mem_slab_num_free_get(&tx_0_mem_slab), NUM_BLOCKS);

	k_msleep(500);
}

west build -b nucleo_u575zi_q samples\drivers\i2s\output

Relevant log output

*** Booting Zephyr OS build v4.2.0 ***
All I2S blocks written
Waiting for all blocks to be free. 16 of 20 
Waiting for all blocks to be free. 19 of 20 
Waiting for all blocks to be free. 19 of 20 
Waiting for all blocks to be free. 19 of 20 
Waiting for all blocks to be free. 19 of 20 
Waiting for all blocks to be free. 19 of 20 
Waiting for all blocks to be free. 19 of 20 
Waiting for all blocks to be free. 19 of 20

Impact

Functional Limitation – Some features not working as expected, but system usable.

Environment

OS: Windows 11
Toolchain: Zephyr SDK 0.17.2
Version: v4.2.0

Additional Context

No response

[github-actions]

Hi @puliceworx! We appreciate you submitting your first issue for our open-source project. 🌟

Even though I'm a bot, I can assure you that the whole community is genuinely grateful for your time and effort. 🤖💙

[erwango]

@mariopaja Would you mind having a look ?

[mariopaja]

Hi @puliceworx,

Is this issue only on DRAIN or also on STOP?
DRAIN I could replicate and I have a solution already for it :)

[puliceworx]

Hi @puliceworx,

Is this issue only on DRAIN or also on STOP? DRAIN I could replicate and I have a solution already for it :)

This happens on STOP as well.

When STOP is triggered last_block gets set.

stream->last_block = true;
stream->state = I2S_STATE_STOPPING;

Then in HAL_SAI_TxCpltCallback

if (stream->last_block) {
	LOG_DBG("TX Stopped ...");
	goto exit;
}

Does not free the last block and potentially leaves data in the queue. Unsure what should happen if there are more mem_blocks in the queue when stop is called.

[mariopaja]

@puliceworx Yes I just noticed.

I already found the issue for both cases. An initial test that I did seems like it fixes both the issues.

I will do some more tests and open tomorrow a PR :)

here you can find the fix branch: https://github.com/mariopaja/zephyr/tree/stm32_sai_tx_drain_stop_fix

I would really appreciate if you could do some more tests :)

STOP:

	ret = i2s_trigger(dev_i2s, I2S_DIR_TX, I2S_TRIGGER_STOP);
	if (ret < 0) {
		printf("Could not trigger I2S tx\n");
		return ret;
	}

	k_msleep(5000);
	
	ret = i2s_trigger(dev_i2s, I2S_DIR_TX, I2S_TRIGGER_START);
	if (ret < 0) {
		printf("Could not trigger I2S tx\n");
		return ret;
	}

LOG:

[00:00:00.001,000] <wrn> i2s_stm32_sai: Free mem slabs: 1
[00:00:00.002,000] <wrn> i2s_stm32_sai: Free mem slabs: 2
[00:00:00.004,000] <wrn> i2s_stm32_sai: Free mem slabs: 3
[00:00:00.005,000] <wrn> i2s_stm32_sai: Free mem slabs: 4
[00:00:00.007,000] <wrn> i2s_stm32_sai: Free mem slabs: 5
[00:00:00.008,000] <wrn> i2s_stm32_sai: Free mem slabs: 6
[00:00:00.010,000] <wrn> i2s_stm32_sai: Free mem slabs: 7
[00:00:00.011,000] <wrn> i2s_stm32_sai: Free mem slabs: 8
[00:00:00.013,000] <wrn> i2s_stm32_sai: Free mem slabs: 9
[00:00:00.014,000] <wrn> i2s_stm32_sai: Free mem slabs: 10
[00:00:00.015,000] <wrn> i2s_stm32_sai: Free mem slabs: 11
[00:00:00.017,000] <wrn> i2s_stm32_sai: Free mem slabs: 12
[00:00:00.018,000] <wrn> i2s_stm32_sai: Free mem slabs: 13
[00:00:00.020,000] <wrn> i2s_stm32_sai: Free mem slabs: 14
[00:00:00.021,000] <wrn> i2s_stm32_sai: Free mem slabs: 15
[00:00:00.023,000] <wrn> i2s_stm32_sai: TX Stopped ...
[00:00:00.023,000] <wrn> i2s_stm32_sai: Free mem slabs: 16
[00:00:05.023,000] <wrn> i2s_stm32_sai: Free mem slabs: 17
[00:00:05.024,000] <wrn> i2s_stm32_sai: Free mem slabs: 18
[00:00:05.026,000] <wrn> i2s_stm32_sai: Free mem slabs: 19
[00:00:05.027,000] <wrn> i2s_stm32_sai: Free mem slabs: 20

DRAIN:

	ret = i2s_trigger(dev_i2s, I2S_DIR_TX, I2S_TRIGGER_DRAIN);
	if (ret < 0) {
		printf("Could not trigger I2S tx\n");
		return ret;
	}

LOG:

[00:00:00.001,000] <wrn> i2s_stm32_sai: Free mem slabs: 1
[00:00:00.002,000] <wrn> i2s_stm32_sai: Free mem slabs: 2
[00:00:00.004,000] <wrn> i2s_stm32_sai: Free mem slabs: 3
[00:00:00.005,000] <wrn> i2s_stm32_sai: Free mem slabs: 4
[00:00:00.007,000] <wrn> i2s_stm32_sai: Free mem slabs: 5
[00:00:00.008,000] <wrn> i2s_stm32_sai: Free mem slabs: 6
[00:00:00.010,000] <wrn> i2s_stm32_sai: Free mem slabs: 7
[00:00:00.011,000] <wrn> i2s_stm32_sai: Free mem slabs: 8
[00:00:00.013,000] <wrn> i2s_stm32_sai: Free mem slabs: 9
[00:00:00.014,000] <wrn> i2s_stm32_sai: Free mem slabs: 10
[00:00:00.015,000] <wrn> i2s_stm32_sai: Free mem slabs: 11
[00:00:00.017,000] <wrn> i2s_stm32_sai: Free mem slabs: 12
[00:00:00.018,000] <wrn> i2s_stm32_sai: Free mem slabs: 13
[00:00:00.020,000] <wrn> i2s_stm32_sai: Free mem slabs: 14
[00:00:00.021,000] <wrn> i2s_stm32_sai: Free mem slabs: 15
[00:00:00.023,000] <wrn> i2s_stm32_sai: Free mem slabs: 16
[00:00:00.024,000] <wrn> i2s_stm32_sai: Free mem slabs: 17
[00:00:00.025,000] <wrn> i2s_stm32_sai: Free mem slabs: 18
[00:00:00.027,000] <wrn> i2s_stm32_sai: Free mem slabs: 19
[00:00:00.028,000] <wrn> i2s_stm32_sai: Free mem slabs: 20

[puliceworx]

@mariopaja yes! That should cover all exits to free the block.