STM32: Add DMA+ADC support

[yonsch]

Is your enhancement proposal related to a problem? Please describe.
To sample relatively high frequency signals with an ADC, integration with a DMA is needed. This is not currently supported by the various ADC drivers.

Describe the solution you'd like
The ADC driver could be configured to use a DMA. A buffer is given to the driver, and is filled with samples. The driver will call a user callback when the buffer is (half) full.

Describe alternatives you've considered

Additional context

[Jeepgoing]

Strong request

[henrikbrixandersen]

Which ADC driver in particular?

[yonsch]

@henrikbrixandersen I needed this specifically for STM32 (I ended up implementing it with HAL), but grepping for "DMA" in the ADC driver directory yielded that no driver implements this currently

[henrikbrixandersen]

@henrikbrixandersen I needed this specifically for STM32 (I ended up implementing it with HAL), but grepping for "DMA" in the ADC driver directory yielded that no driver implements this currently

That's not entirely true. The drivers/adc/adc_mcux_adc16.c driver implements DMA. Given that we have 15+ ADC drivers, it is quite important which driver, you are asking about, though.

[Powilas21]

Any updates on this?

[pdietl]

Hi all. I have an implementation almost ready to go, but I have some questions.

1. Should the DMA version of the ADC driver be separate from the interrupt based (current) driver (i.e., an alternative), or should is replace the current driver?
2. In principle some of the DMA information contained in the DMA device tree is already known. For instance, here:

   zephyr/dts/bindings/dma/st,stm32-dma-v2.yaml

   Line 22 in 801fe1f

   -bit 6-7 : Direction (see dma.h)

   the direction bits, Peripheral Increment Address bit, the Memory Increment Address bit, the Peripheral data size bits, and the Memory data size bit, are all implied by this particular use case, as is the slot/DMA periph request ID. So how do we handle this? Require the user to provide this data which is unchanging and a possible source of confusion and error, or figure out that information for ourselves in the driver?

@henrikbrixandersen @ABOSTM @erwango

[erwango]

Hi all. I have an implementation almost ready to go, but I have some questions.

Great ! Thanks for working on this

1. Should the DMA version of the ADC driver be separate from the interrupt based (current) driver (i.e., an alternative), or should is replace the current driver?

It should be an alternative. Ideally both versions should coexist with the minimum overlap.
Later on, if we found that DMA version maturity is equivalent to the IRQ based one and there is no value in keeping the IRQ based, we can revisit this statement.

2. In principle some of the DMA information contained in the DMA device tree is already known. For instance, here:

   zephyr/dts/bindings/dma/st,stm32-dma-v2.yaml

   Line 22 in 801fe1f

   -bit 6-7 : Direction (see dma.h)

   
   the direction bits, Peripheral Increment Address bit, the Memory Increment Address bit, the Peripheral data size bits, and the Memory data size bit, are all implied by this particular use case, as is the slot/DMA periph request ID. So how do we handle this? Require the user to provide this data which is unchanging and a possible source of confusion and error, or figure out that information for ourselves in the driver?

Let's keep it simple: Go ahead and overwrite the device tree values

[pdietl]

Thanks for the feedback @erwango. What is your recommendation for organizing this? A separate file, conditional compilation of the current file, a combination of both? Sometime else?

[erwango]

Thanks for the feedback @erwango. What is your recommendation for organizing this? A separate file, conditional compilation of the current file, a combination of both? Sometime else?

My preference goes to the easier to maintain, ie:

• most readable
• less code duplication

These are generally going in opposite direction, so there's a trade off to be found, or use of implementation tricks to get best of both.
Issue with ADC driver is that it is already quite poor in term of readability and would deserve an initial clean up before adding any other feature.
In the end it's hard to provide a recommendation w/o having seen the code you intent to push.

Some questions :

• Is your solution compatible with series currently supported with the existing series.
• Are we sure IRQ/DMA should be exclusive ? (What about using IRQ on one ADC instance and DMA on another instance in the same application ?) > Being exclusive in a first iteration is acceptable, but if we're to consider they should be compatible in the end, this could help in the decision.

[pdietl]

@erwango is there a list of currently supported STM32s?

[henrikbrixandersen]

@erwango is there a list of currently supported STM32s?

I think the closest is the list of DTSI files under https://github.com/zephyrproject-rtos/zephyr/tree/main/dts/arm/st

[erwango]

is there a list of currently supported STM32s?

I think the closest is the list of DTSI files under https://github.com/zephyrproject-rtos/zephyr/tree/main/dts/arm/st

Device tree is indeed the reference, but my question was specifically around ADC, so look for adc nodes in this location.

[pdietl]

I have a working bit of code that needs a lot of cleanup and generalization away from STM32G4. It also makes a new interface for doing something like #32719 for continuous reading.

https://github.com/worldcoin/zephyr/tree/stm32_adc_dma

@erwango @henrikbrixandersen @Powilas21 @yonsch

[erwango]

I have a working bit of code that needs a lot of cleanup and generalization away from STM32G4. It also makes a new interface for doing something like #32719 for continuous reading.

https://github.com/worldcoin/zephyr/tree/stm32_adc_dma

@erwango @henrikbrixandersen @Powilas21 @yonsch

@pdietl Thanks, interesting. From what I see this removes IRQ mode, is that correct?

[pdietl]

@erwango yes, but we can add it back or make this configurable maybe.

[erwango]

@erwango yes, but we can add it back or make this configurable maybe.

It is indeed not possible to remove IRQ mode support. Behavior regarding IRQ mode should be equivalent with or without DMA.
If configurable, it should be default y.
I suggest to get the DMA mode as an opt-in, using dma property. If DMA property is configured, then DMA is used. IRQ mode is used otherwise. See for instance how it is done for SPI or UART STM32 drivers.

[pdietl]

Good idea. I’ll do this.

[pdietl]

@erwango I also want your opinion on the continuous reading interface. It would be nice to make a change to the ADC public API to support this use-case. Either by adding a new function (as I have) or adding more fields to the adc_options struct.

[erwango]

@erwango I also want your opinion on the continuous reading interface. It would be nice to make a change to the ADC public API to support this use-case. Either by adding a new function (as I have) or adding more fields to the adc_options struct.

This should rather be discussed with @anangl as ADC maintainer. I suggest to open a dedicated issue for this purpose.

[pdietl]

Discussion created here: #48478

[JamesBeckley-eaton]

Any updates on this?

[erwango]

For people interested in ADC+DMA:
#52965
Don't hesitate to test and provide feedback in this PR

[erwango]

#52965 is now merged. Closing