Skip to content
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

STM32G0/G4 DMA + more devices + DAC DMA #632

Merged
merged 9 commits into from
Jun 16, 2021

Conversation

chris-durand
Copy link
Member

@chris-durand chris-durand commented May 17, 2021

This PR adds DMA support for controllers with the DMAMUX IP. Furthermore a DAC DMA driver is added, DAC support for all STM32 with DAC is provided and DAC examples with and without DMA are added (modm currently has zero),

Additionally, this PR completes DMA support for all previously unsupported F0, F1, F3, L0, L1 and L4.

This depends on modm-io/modm-devices#60.

The G4 DMA DAC example:

dac_g4_dma

The information how many channels a DAC instance has is pretty hard to get out of the CubeMX data. It's a huge mess with tons of conditional expressions in strings. Thus, the driver might provide more channels than are actually working in hardware. The same is true for information if channels have an output buffer.

@chris-durand chris-durand added the ci:hal Triggers the exhaustive HAL compile CI jobs label Jun 4, 2021
@chris-durand chris-durand force-pushed the feature/stm32g4_dma branch 2 times, most recently from 75197f5 to b655ad1 Compare June 4, 2021 17:11
@chris-durand chris-durand added ci:hal Triggers the exhaustive HAL compile CI jobs and removed ci:hal Triggers the exhaustive HAL compile CI jobs labels Jun 4, 2021
@chris-durand chris-durand force-pushed the feature/stm32g4_dma branch 2 times, most recently from cb7b19f to f3dd039 Compare June 6, 2021 17:02
@chris-durand chris-durand changed the title STM32G0/G4 DMA + DAC DMA STM32G0/G4 DMA + more devices + DAC DMA Jun 6, 2021
@chris-durand chris-durand added ci:hal Triggers the exhaustive HAL compile CI jobs and removed ci:hal Triggers the exhaustive HAL compile CI jobs labels Jun 6, 2021
@chris-durand chris-durand marked this pull request as ready for review June 6, 2021 20:40
Now all F0, F1, F3, L0, L1, L4, G0 and G4 are supported.
@chris-durand
Copy link
Member Author

Finally DMA should be supported for all F0, F1, F3, L0, L1, L4, G0 and G4 (if the CI agrees). Most likely H7 DMA1/2, WB and WL would also work if we supported the controllers.

@chris-durand chris-durand removed the ci:hal Triggers the exhaustive HAL compile CI jobs label Jun 6, 2021
@chris-durand chris-durand added the ci:hal Triggers the exhaustive HAL compile CI jobs label Jun 6, 2021
Copy link
Member

@salkinium salkinium left a comment

Choose a reason for hiding this comment

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

This is really well written code! No complaints from my side.

Copy link
Member

@rleh rleh left a comment

Choose a reason for hiding this comment

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

Nice 👍🏽

src/modm/platform/dac/stm32/dac.hpp.in Show resolved Hide resolved
* Set the output voltage for a DAC channel
*
* @param chan The channel to set
* @param value The 12-bit DAC output value, range 0-4095.
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to use the full range of uint16_t as input (and the left-aligned data register DHR12L1) to be able to use the same interface for different DACs in the future and better portability.

In any case, it could be useful to put the precision of the DAC somewhere in the code:
static constexpr uint8_t PrecisionBit = 12;

Copy link
Member Author

@chris-durand chris-durand Jun 12, 2021

Choose a reason for hiding this comment

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

I would prefer to use the full range of uint16_t as input (and the left-aligned data register DHR12L1) to be able to use the same interface for different DACs in the future and better portability.

It would be quite evil to change the data format to left-justified for an already existing driver. The benefit seems rather small and the choice seems a bit arbitrary as well. Let's say we have an 8 bit or 18 bit DAC, in both cases 16 bit left aligned data is not the most useful choice. It would be fine for me to state in some constexpr static variable what the data format is and the user code can deal with it.

In any case, it could be useful to put the precision of the DAC somewhere in the code:
static constexpr uint8_t PrecisionBit = 12;

👍

Copy link
Member

Choose a reason for hiding this comment

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

Ok 👍🏽

@salkinium salkinium merged commit 8896b5b into modm-io:develop Jun 16, 2021
@salkinium salkinium added this to the 2021q2 milestone Jun 20, 2021
@chris-durand chris-durand deleted the feature/stm32g4_dma branch January 2, 2022 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci:hal Triggers the exhaustive HAL compile CI jobs example 🔑 feature 🚧
Development

Successfully merging this pull request may close these issues.

3 participants