dma: stm32: Clean up around usage of STM32_DMA_STREAM_OFFSET #94869
Conversation
zephyrbot
added
platform: STM32
zephyrbot
requested review from
FRASTM,
GeorgeCGV,
anangl,
danieldegrasse,
de-nordic,
decsny,
djiatsaf-st,
etienne-lms,
gautierg-st,
jfischer-no,
josuah,
marwaiehm-st,
mathieuchopstm,
rghaddab and
teburd
erwango
force-pushed
the
dma_offset_cleanup
branch
2 times, most recently
from
30abb42 to
d6923cb
Compare
| #if CONFIG_DMA_STM32U5 | ||
| hdma.Instance = LL_DMA_GET_CHANNEL_INSTANCE(dev_data->dma.reg, | ||
| dev_data->dma.channel); | ||
| #elif defined(CONFIG_DMAMUX_STM32) |
There was a problem hiding this comment.
Is CONFIG_DMAMUX_STM32=y properly considered in the new STM32_DMA_GET_INSTANCE() macro?
There was a problem hiding this comment.
Actually, this macro only abstract LL_DMA_GET_CHANNEL_INSTANCE vs __LL_DMA_GET_STREAM_INSTANCE which doesn't depend on CONFIG_DMAMUX_STM32. Then the offset consideration, which can be impacted by CONFIG_DMAMUX_STM32 is taken care by systematic use of STM32_DMA_STREAM_OFFSET.
On STM32L4, similarly to other STM32 series, enable DMA directly based on dt configuration and avoid need to configure it at application level. Signed-off-by: Erwan Gouriou <[email protected]>
STM32_DMA_STREAM_OFFSET is defined as 0 in case "dma u5" is in use. Clean up code relating to this define. Signed-off-by: Erwan Gouriou <[email protected]>
To ease code understanding of offset handling within the driver, harmonize its treatment within impacted functions. Signed-off-by: Erwan Gouriou <[email protected]>
Makes it easier to grep. Signed-off-by: Erwan Gouriou <[email protected]>
In HAL based stm32 drivers, dma handling is done internally to HAL. Though, in order to avoid a dma_config() call is done to ensure stream will be set as busy in zephyr dma driver to avoid potential resource sharing conflict. This dma_config() call was done while taking into account STM32_DMA_STREAM_OFFSET, which is wrong as it will prevent zephyr dma driver to set the right stream as busy. Fix this in impacted drivers. Signed-off-by: Erwan Gouriou <[email protected]>
To simplify the work on client drivers, provide a STM32_DMA_GET_INSTANCE() macro which abstracts: - STM32_DMA_STREAM_OFFSET - __LL_DMA_GET_STREAM_INSTANCE() vs __LL_DMA_GET_CHANNEL_INSTANCE() Signed-off-by: Erwan Gouriou <[email protected]>
erwango
force-pushed
the
dma_offset_cleanup
branch
from
d6923cb to
102d302
Compare
Use the new macro and factorize code when possible. Signed-off-by: Erwan Gouriou <[email protected]>
erwango
force-pushed
the
dma_offset_cleanup
branch
from
102d302 to
d7e2581
Compare
|
|
@etienne-lms PTAL |
There was a problem hiding this comment.
My sole concern it that it seems to me that commits
"drivers: dma: stm32: Align channel offset handling in all API functions" and
"drivers: stm32: Keep DMA stream offset handling internal to driver"
are to be tested together. If testing (e.g. with gitbisect) after the 1st is applied but before the 2nd is, then something will go wrong.
Maybe this is not a real issue. If not, then LGTM, my +1, otherwise, I think both commits should be squashed.
Actually "drivers: dma: stm32: Align channel offset handling in all API functions" has no functional impact. Index shift is just moved so that it is done only once in the function for better readability. |
6 participants
STM32_DMA_STREAM_OFFSETis required to allow to deal with difference of channel numbering scheme between versions of STM32 DMA.Unfortunately, even if correct, its usage is confusing, so make some cleanup around it.