Skip to content

basic ch1 12bit driver done#55

Open
protodoatt wants to merge 1 commit into
masterfrom
dac_things
Open

basic ch1 12bit driver done#55
protodoatt wants to merge 1 commit into
masterfrom
dac_things

Conversation

@protodoatt

Copy link
Copy Markdown
Collaborator

Basic STM32U545 Ch1 12bit DAC driver that implements the existing(albeit very basic) HIL.

Pull Request Overview

This pull request adds very basic driver support for CH1 of the DAC of the stm32u5xx chips. Hardcoded for 12 bit right-aligned writes without DMA.

Testing Strategy

This pull request was tested by using the DAC test inside libtock-c (the written values do have to be multiplied by 4 to get the full range as the DAC test expects 8-bit values, but I implemented 12-bit support.

TODO or Help Wanted

This pull request still needs some way to take care of (or advertise) the difference between 8 and 12 bit input values. The DAC could be set to 8 bit mode as well. I also should add some bounds check on the value before its written to the register.
The HIL could also be modified to take care of more complex DACs such as the one on the u5xx series.

Documentation Updated

  • Updated the relevant files in /docs, or no updates are required.

Formatting

  • Ran make prepush.

AI Use

  • The PR description details my use of AI in the production of the
    code in this PR, if any, and I have manually checked and
    personally certify the entire contents of this PR.

@github-actions github-actions Bot added the stm32 label Jul 3, 2026
Comment on lines 84 to 118
impl InterruptService for Stm32u5xxDefaultPeripherals<'_> {
unsafe fn service_interrupt(&self, interrupt: u32) -> bool {
match interrupt {
TIM2_IRQ => {
// TIM2
self.tim2.handle_interrupt();
true
}
USART1_IRQ => {
// USART1
self.usart1.handle_interrupt();
true
}
EXTI13_IRQ => {
// EXTI13 (Button)
self.exti.handle_interrupt(crate::exti::LineId::Line13);
true
}
// Route all 16 GPDMA1 Channels to the DMA manager
GPDMA1_CH0_IRQ => {
self.dma1.handle_interrupt(ChannelId::Channel00);
true
}
GPDMA1_CH1_IRQ => {
self.dma1.handle_interrupt(ChannelId::Channel01);
true
}
GPDMA1_CH2_IRQ => {
self.dma1.handle_interrupt(ChannelId::Channel02);
true
}
GPDMA1_CH3_IRQ => {
self.dma1.handle_interrupt(ChannelId::Channel03);
true
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please add here the interrupt for the DAC.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Because the driver is super basic and doesn't use DMA, I don't think it needs an interrupt handler. Only writes the value to a register when called, doesn't need to wait for anything. If DMA is added in the future, then the interrupt of the DAC will be bound to one of the GPDMA channels which is already here.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please document this driver. We want to see some comments on how it is working.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Sure, I'll take care of that real quick.

Comment on lines +130 to +140

pub fn set_adcdacsel_source_hse(&self) {
self.registers.ccipr3.modify(CCIPR3::ADCDACSEL::HSE);
}

pub fn set_dac1sel_source_lsi(&self) {
self.registers.ccipr3.modify(CCIPR3::DAC1SEL::LSI);
}
pub fn enable_dac1(&self) {
self.registers.ahb3enr.modify(AHB3ENR::DAC1EN::SET);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why do we need these?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We apparently don't, as the default clock source seems to work fine. I implemented them thinking I would need to use them to route the ADCDACSEL clock MUX during initialization, but I never needed to. They can be removed.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

We apparently don't, as the default clock source seems to work fine. I implemented them thinking I would need to use them to route the ADCDACSEL clock MUX during initialization, but I never needed to. They can be removed.

If there are not used, I think we can remove them.

@protodoatt
protodoatt requested a review from genan2003 July 3, 2026 12:02
@protodoatt

Copy link
Copy Markdown
Collaborator Author

I saw clippy is throwing an error, I'll fix it

Comment on lines +127 to +132

//DAC pin
periphs
.gpio_a
.pin(PinId::Pin04)
.set_mode(stm32u545::gpio::Mode::Analog);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do we have only one pin from the Arduino ones that can act as a DAC?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, only one routed per Channel, because I only implemented Ch1, it can only live on that pin.

Comment thread chips/stm32u5xx/src/dac.rs Outdated
Comment on lines +127 to 133

//DAC pin
periphs
.gpio_a
.pin(PinId::Pin04)
.set_mode(stm32u545::gpio::Mode::Analog);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can you write in the comment what pin is this from the board?

Comment thread chips/stm32u5xx/src/dac.rs Outdated

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Please use /// instead of // for the comments about registers in this driver.

Comment thread chips/stm32u5xx/src/rcc.rs Outdated
Comment on lines +19 to +21
(0x090 => _reserved1: [u32; 1]),
(0x94 => ahb3enr: ReadWrite<u32, AHB3ENR::Register>),
(0x98 => _reserved4: [u32; 1]),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Are this offsets reserved for a reason?

control register done

basic ch1 driver complete

basic ch1 driver complete(again)

basic ch1 driver complete(again 2)

basic ch1 dac driver with ci errors fixed

basic ch1 dac driver with ci errors fixed fr this time

squashed all intermediary commits

Signed-off-by: Andrei-Tudor Tanase <protodoatt@gmail.com>

added bounds check and documentation

Signed-off-by: Andrei-Tudor Tanase <protodoatt@gmail.com>

fixed clippy

Signed-off-by: Andrei-Tudor Tanase <protodoatt@gmail.com>

Update chips/stm32u5xx/src/dac.rs

Co-authored-by: Omer Genan <81963672+genan2003@users.noreply.github.com>
Signed-off-by: Andrei-Tudor Tanase <protodoatt@gmail.com>

added pin comment

Signed-off-by: Andrei-Tudor Tanase <protodoatt@gmail.com>

changed register comments

Signed-off-by: Andrei-Tudor Tanase <protodoatt@gmail.com>

register reserved offsets explained

Signed-off-by: Andrei-Tudor Tanase <protodoatt@gmail.com>

Update chips/stm32u5xx/src/dac.rs

changed copyright code

Co-authored-by: Omer Genan <81963672+genan2003@users.noreply.github.com>
Signed-off-by: Andrei-Tudor Tanase <protodoatt@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants