Skip to content

Conversation

avolmat-st
Copy link

If hal_override is set, avoid reporting an error and clear the transfer error (TE) bit so that HAL code can properly handle it.

If hal_override is set, avoid reporting an error and clear the
transfer error (TE) bit so that HAL code can properly handle it.

Signed-off-by: Alain Volmat <[email protected]>
erwango
erwango previously approved these changes Oct 17, 2025
mathieuchopstm
mathieuchopstm previously approved these changes Oct 17, 2025
Copy link
Contributor

@mathieuchopstm mathieuchopstm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking:

Comment on lines +136 to +142
/* Let HAL DMA handle flags on its own */
if (!stream->hal_override) {
LOG_ERR("Transfer Error.");
stream->busy = false;
dma_stm32_dump_stream_irq(dev, id);
dma_stm32_clear_stream_irq(dev, id);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should log nonetheless?

Suggested change
/* Let HAL DMA handle flags on its own */
if (!stream->hal_override) {
LOG_ERR("Transfer Error.");
stream->busy = false;
dma_stm32_dump_stream_irq(dev, id);
dma_stm32_clear_stream_irq(dev, id);
}
LOG_ERR("Transfer Error.");
if (!stream->hal_override) {
/* Let HAL DMA handle flags on its own */
stream->busy = false;
dma_stm32_dump_stream_irq(dev, id);
dma_stm32_clear_stream_irq(dev, id);
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hum, considering that in case of HAL_OVERRIDE is being use, the Zephyr DMA framework is mainly use for resource handling rather than DMA functionaly speaking, I do not think that in such condition we should LOG anything here. If that's the HAL which is handling the DMA control, error checking etc, let's let it fully handle (aka configure, error handling etc).

Don't misunderstand me, I am not saying that we should bypass all and forever the Zephyr DMA framework ;) however in current implementation I do not see much merit to have the Zephyr DMA log this message. At least in case of DCMI, we have the callback in place to also get notification of transfer error. I suppose that's also the same thing for other drivers relying on HAL_OVERRIDE.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine by me 🙂

@avolmat-st
Copy link
Author

In case of hal_override, we might want to also avoid doing the flag clear in case of FiFo error done by the dma_stm32_v1.c.
What do you think ?

Cf #97491 (comment)

In case of using HAL_OVERRIDE, avoid clearing the FIFO ERROR flag
before calling the HAL DMA IrqHandler so that the HAL DMA code
handling can be used.

Signed-off-by: Alain Volmat <[email protected]>
@avolmat-st avolmat-st dismissed stale reviews from mathieuchopstm and erwango via 445302d October 18, 2025 11:25
@avolmat-st
Copy link
Author

I added a new commit to also avoid clear of the FIFO ERROR flag in case of using HAL_OVERRIDE.

Copy link

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: STM32 ST Micro STM32

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants