soc: stm32f7 remove cache memory with dma transfer

[FRASTM]

When using the DMA on stm32f7, the cache memory is disabled using the CONFIG_NOCACHE_MEMORY
This must be the case when running dma memory-to-memory transfers test cases
tests/driver/dma/chan_blen_transfer and tests/driver/dma/loop_transfer

on nucleo_f746zg (dma2 is already enabled in the dts of the board)

Signed-off-by: Francois Ramu [email protected]

Fixes #35220

[FRASTM]

rebase,
split commits

[erwango]

If commit "drivers: dma: stm32 dmamux has request 0 for mem-to-mem" is fixing a bug, this should be clearly identified.

[FRASTM]

tested with ./scripts/twister --device-testing --device-serial /dev/ttyACM0 -p nucleo_f746zg -j 2 -T tests/drivers/dma/chan_blen_transfer/
tested with ./scripts/twister --device-testing --device-serial /dev/ttyACM0 -p nucleo_f746zg -j 2 -T tests/drivers/dma/loop_transfer/

[FRASTM]

single commit for this fix so that it is not mix with other PRs

[galak]

I wonder if we need this fix for:

soc/arm/nxp_imx/rt/soc.c
soc/arm/atmel_sam/same70/soc.c
soc/arm/atmel_sam/samv71/soc.c

And if we should refactor that code into something come for M7's to do this in one place.

[galak]

@mnkp - wonder if this same fix would address #35107

[mnkp]

The implementation is incorrect. From the description of NOCACHE_MEMORY Kconfig option

Add a "nocache" read-write memory section that is configured to
not be cached. This memory section can be used to perform DMA
transfers when cache coherence issues are not optimal or can not
be solved using cache maintenance operations.

The option adds support for a new, dedicated linker section. Only data placed in this specific, limited RAM section via __nocache pragma/macro are not cached. This PR disables completely instruction and data cache if CONFIG_NOCACHE_MEMORY=y.

[aurel32]

The implementation is incorrect. From the description of NOCACHE_MEMORY Kconfig option

Add a "nocache" read-write memory section that is configured to
not be cached. This memory section can be used to perform DMA
transfers when cache coherence issues are not optimal or can not
be solved using cache maintenance operations.

The option adds support for a new, dedicated linker section. Only data placed in this specific, limited RAM section via __nocache pragma/macro are not cached. This PR disables completely instruction and data cache if CONFIG_NOCACHE_MEMORY=y.

I fully agree with that. Now if that PR fixes the issue, it means that there is a cache issue somewhere, which can actually be fixed by using no cache memory for that particular region. Alternatively it should be possible to use cache invalidate / clean, but Zephyr still lacks a proper infrastructure to do that.

Both DMA and cache are supposed to improve performance. They should not be exclusive.

[aurel32]

I have been able to reproduce the issue on a stm32f723e_disco board. This happens every other boot and is definitely the same issue than #35107 and #35089. Reverting commit d6b5023 fixes the issue.

[galak]

I have been able to reproduce the issue on a stm32f723e_disco board. This happens every other boot and is definitely the same issue than #35107 and #35089. Reverting commit d6b5023 fixes the issue.

Do we think there is a an ordering issue here between MPU init and cache init and that commit changed the order and thus broke M7?

[aurel32]

I have been able to reproduce the issue on a stm32f723e_disco board. This happens every other boot and is definitely the same issue than #35107 and #35089. Reverting commit d6b5023 fixes the issue.

Do we think there is a an ordering issue here between MPU init and cache init and that commit changed the order and thus broke M7?

I have the impression that it worked by chance before and that the cache clean and invalidation is not done the proper way. For instance enabling CONFIG_INIT_ARCH_HW_AT_BOOT moves that cache clean and invalidation even earlier in the boot process, and also fails.

[aurel32]

I think the problem is with the clean part of the operation. By reorganizing the code, the call to SCB_CleanInvalidateDCache() happens before the D-cache is enabled. When the cache is not enabled, it might contain plenty of outdated cache lines from before the reset, and trying to clean them means pushing random data to random addresses, triggering a bus fault.

We should change that part to something like:

        if (SCB->CCR & SCB_CCR_DC_Msk) {
                SCB_CleanInvalidateDCache();
        } else {
                SCB_InvalidateDCache();
        }   

Now if we are sure that the D-cache is not enabled at this point, we can just call SCB_InvalidateDCache() instead.

For the CONFIG_INIT_ARCH_HW_AT_BOOT case, we should just drop SCB_CleanInvalidateDCache(), as SCB_DisableDCache() already takes care of cleaning and invaliding the cache.

[FRASTM]

This patch is no more relevant since the #35258, isn't it

[aurel32]

This patch is no more relevant since the #35258, isn't it

Yes, I believe this PR can now be closed.