Skip to content

drivers: dma_mcux_edma: Simple Refactoring #92869

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

decsny
Copy link
Member

@decsny decsny commented Jul 9, 2025

This edma driver is extremely important for NXP platforms as it is used by many different drivers itself, and what I am noticing is that is always causing a lot of problems to debug through it because it is a mess. So this PR is doing a basic refactor which should not change any logic of the program flow but just splitting things into helper functions to be more readable, etc.

@hakehuang
Copy link
Contributor

@decsny it does not well on my mimxrt1060_evk@c platform.
west build -b mimxrt1060_evk@C/mimxrt1062/qspi -T drivers.dma.scatter_gather tests/drivers/dma/scatter_gather/
west flash

*** Booting Zephyr OS build v4.2.0-rc2-41-g2320da176733 ***
Running TESTSUITE dma_m2m_sg
===================================================================
START - test_dma_m2m_sg
DMA memory to memory transfer started
Preparing DMA Controller
dma block 0 block_size 8192, source addr 0x80000000, dest addr 0x80002000
set next block pointer to 0x80011384
dma block 1 block_size 8192, source addr 0x80000000, dest addr 0x80004000
set next block pointer to 0x800113a4
dma block 2 block_size 8192, source addr 0x80000000, dest addr 0x80006000
set next block pointer to 0x800113c4
dma block 3 block_size 8192, source addr 0x80000000, dest addr 0x80008000
Configuring the scatter-gather transfer on channel 0
Starting the transfer on channel 0 and waiting completion
callback status -5
Timed out waiting for xfers

    Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/dma/scatter_gather/src/test_dma_sg.c:159: dma_m2m_sg_test_dma_m2m_sg: (test_sg() == TC_PASS) is false

 FAIL - test_dma_m2m_sg in 1.069 seconds
===================================================================
TESTSUITE dma_m2m_sg failed.

------ TESTSUITE SUMMARY START ------

SUITE FAIL -   0.00% [dma_m2m_sg]: pass = 0, fail = 1, skip = 0, total = 1 duration = 1.069 seconds
 - FAIL - [dma_m2m_sg.test_dma_m2m_sg] duration = 1.069 seconds

------ TESTSUITE SUMMARY END ------

===================================================================
RunID: e6cc0a0d383388b881518dafe6926e23
PROJECT EXECUTION FAILED

and for uart

tests/drivers/uart/uart_async_api/drivers.uart.async_api.lpuart.rt_nocache

*** Booting Zephyr OS build v4.2.0-rc2-41-g2320da176733 ***
Running TESTSUITE uart_async_chain_read
===================================================================
UART instance:uart@4018c000
E: No buffers to release from RX DMA!
START - test_chained_read

    Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/uart/uart_async_api/src/test_uart_async.c:431: uart_async_chain_read_test_chained_read: (k_sem_take(&tx_done, K_MSEC(100)) not equal to 0)
TX_DONE timeout
 FAIL - test_chained_read in 0.200 seconds
===================================================================
TESTSUITE uart_async_chain_read failed.
Running TESTSUITE uart_async_chain_write
===================================================================
UART instance:uart@4018c000
START - test_chained_write

    Assertion failed at WEST_TOPDIR/zephyr/tests/drivers/uart/uart_async_api/src/test_uart_async.c:887: uart_async_chain_write_test_chained_write: (k_sem_take(&tx_done, K_MSEC(100)) not equal to 0)
TX_DONE timeout
 FAIL - test_chained_write in 0.119 seconds
===================================================================
TESTSUITE uart_async_chain_write failed.
Running TESTSUITE uart_async_double_buf
===================================================================
UART instance:uart@4018c000
START - test_double_buffer

@decsny
Copy link
Member Author

decsny commented Jul 16, 2025

@hakehuang it looks like there was a typo in a #if condition which affected platform with cache like RT1060, should be fixed now

@hakehuang
Copy link
Contributor

hakehuang commented Jul 17, 2025

@hakehuang it looks like there was a typo in a #if condition which affected platform with cache like RT1060, should be fixed now

@decsny now regression pass on v4.2.0-rc3-26-g6aea2db65c32, but need fix issue with

- 1) drivers.adc.mcux.adc12.build on twr_ke18f/mke18f16 error (Build failure - error: #error Unexpected or disallowed cache situation for dma descriptors)
INFO    - 2) drivers.dac.mcux32.build on twr_ke18f/mke18f16 error (Build failure - error: #error Unexpected or disallowed cache situation for dma descriptors)

hakehuang
hakehuang previously approved these changes Jul 17, 2025
@hakehuang hakehuang self-requested a review July 17, 2025 09:28
@dleach02
Copy link
Member

@decsny please address the CI issues

@decsny
Copy link
Member Author

decsny commented Jul 24, 2025

i will when i have time

decsny added 7 commits August 8, 2025 10:29
The dependency on the chosen node for dtcm can be expressed in Kconfig
language.

Cache we care about is CPU DCACHE, not the meaningless "MCUX Cache"

The macros can be reordered to be simpler by having only one level of
conditional (no nesting) instead of three levels.

Move this code closer in the file to where this cache attribute macro is
actually going to be used (the init macro) instead of randomly splitting
up the struct definitions at the top.

Signed-off-by: Declan Snyder <[email protected]>
One #ifdef outside the functions is simpler to read than two #ifdefs
inside the functions.

Signed-off-by: Declan Snyder <[email protected]>
Instead of having preprocessor code, make a hidden kconfig
to indicate this and be smarter about the C code (these are all powers
of two)

Signed-off-by: Declan Snyder <[email protected]>
There is two completely different types of reload modes happening here,
therefore we should split this function into two completely separate
functions because it was getting large and hard to read. Removes
one level of indentation.

Signed-off-by: Declan Snyder <[email protected]>
There was actually three different types of configuration modes
happening here, and this function was getting extremely bulky and hard
to read due to the amount of nesting of conditionals. Split these into
separate functions and call them appropriately depending on the type of
transfer.

Signed-off-by: Declan Snyder <[email protected]>
Extract and group dmamux related code together for readability.

Signed-off-by: Declan Snyder <[email protected]>
Halve all this ifdef code by making a macro function for the memory
mapping.

Signed-off-by: Declan Snyder <[email protected]>
Copy link

sonarqubecloud bot commented Aug 8, 2025

@decsny
Copy link
Member Author

decsny commented Aug 8, 2025

@hakehuang can you help retesting this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: DMA Direct Memory Access platform: NXP Drivers NXP Semiconductors, drivers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants