Skip to content

cpu/gd32v: add periph_dac support#19248

Merged
bors[bot] merged 4 commits intoRIOT-OS:masterfrom
gschorcht:cpu/gd32v/periph_dac
Feb 7, 2023
Merged

cpu/gd32v: add periph_dac support#19248
bors[bot] merged 4 commits intoRIOT-OS:masterfrom
gschorcht:cpu/gd32v/periph_dac

Conversation

@gschorcht
Copy link
Copy Markdown
Contributor

@gschorcht gschorcht commented Feb 6, 2023

Contribution description

This PR provides the periph_dac support for GD32VF103.

Testing procedure

tests/periph_dac should work on sipeed-longan-nano port on PA4 and PA5.

Issues/PRs references

@github-actions github-actions bot added Area: boards Area: Board ports Area: cpu Area: CPU/MCU ports Area: doc Area: Documentation Area: Kconfig Area: Kconfig integration Area: tools Area: Supplementary tools labels Feb 6, 2023
@gschorcht gschorcht added Type: new feature The issue requests / The PR implemements a new feature for RIOT CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: RISC-V Platform: This PR/issue effects RISC-V-based platforms labels Feb 6, 2023
@gschorcht gschorcht requested a review from benpicco February 6, 2023 06:48
@gschorcht gschorcht force-pushed the cpu/gd32v/periph_dac branch from ca4e9c0 to 507dff2 Compare February 6, 2023 06:51
@github-actions github-actions bot removed the Platform: RISC-V Platform: This PR/issue effects RISC-V-based platforms label Feb 6, 2023
@riot-ci
Copy link
Copy Markdown

riot-ci commented Feb 6, 2023 •
edited
Loading

Murdock results

✔️ PASSED

066b921 dist/toos/doccheck: add DAC configu to generic_exclude_pattern

Success Failures Total Runtime
6850 0 6851 10m:54s

Artifacts

benpicco
benpicco approved these changes Feb 6, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@benpicco benpicco left a comment

Choose a reason for hiding this comment

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

Looks good! There are some weird hangs (and a bug in the sine function) in tests/driver_dac_dds, but those are unrelated to this PR.

bmp_26_000

@benpicco
Copy link
Copy Markdown
Contributor

benpicco commented Feb 6, 2023

bors merge

bors bot added a commit that referenced this pull request Feb 6, 2023
@bors @benpicco @gschorcht
Merge #17045 #19248 #19251
95ce9fc 
17045: sys/coding: add XOR based coding module r=benpicco a=benpicco



19248: cpu/gd32v: add periph_dac support r=benpicco a=gschorcht

### Contribution description

This PR provides the `periph_dac` support for GD32VF103.

### Testing procedure

`tests/periph_dac` should work on `sipeed-longan-nano` port on PA4 and PA5.

### Issues/PRs references

19251: tests/driver_dac_dds: fix output of sine and saw functions r=benpicco a=benpicco



Co-authored-by: Benjamin Valentin <[email protected]>
Co-authored-by: Gunar Schorcht <[email protected]>
gschorcht
gschorcht commented Feb 6, 2023
View reviewed changes
cpu/gd32v/periph/dac.c Outdated
assert(dac_config[line].chan < DAC_CHANNEL_NUMOF);

/* disable the DAC channel */
DAC->CTL |= ~((dac_config[line].chan) ? DAC_CTL_DEN1_Msk : DAC_CTL_DEN0_Msk);
Copy link
Copy Markdown
Contributor Author

@gschorcht gschorcht Feb 6, 2023

Choose a reason for hiding this comment

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

I found a small mistake

Suggested change
DAC->CTL |= ~((dac_config[line].chan) ? DAC_CTL_DEN1_Msk : DAC_CTL_DEN0_Msk);
DAC->CTL &= ~((dac_config[line].chan) ? DAC_CTL_DEN1_Msk : DAC_CTL_DEN0_Msk);

Is it possible to cancel bors only for this PR or will it cancel the whole train?

Copy link
Copy Markdown
Contributor

@benpicco benpicco Feb 6, 2023

Choose a reason for hiding this comment

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

It should do it automatically when you do a push (just squash directly)

Copy link
Copy Markdown
Contributor Author

@gschorcht gschorcht Feb 6, 2023

Choose a reason for hiding this comment

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

Done

@gschorcht
Copy link
Copy Markdown
Contributor Author

gschorcht commented Feb 6, 2023

bors cancel

@bors
Copy link
Copy Markdown
Contributor

bors bot commented Feb 6, 2023

Canceled.

@gschorcht gschorcht force-pushed the cpu/gd32v/periph_dac branch from 507dff2 to c46382a Compare February 6, 2023 20:32
@gschorcht
Copy link
Copy Markdown
Contributor Author

gschorcht commented Feb 6, 2023 •
edited
Loading

Looks good! There are some weird hangs

What puzzles me a bit is that it seems to hang reproducibly when a command like saw 1000 1 or something like that is executed for the second time. I had not noticed this as I had only tested with tests/periph_dac. Could it have something to do with the periodic timer? Unfortunately I don't have another board at hand that supports the periph_dac function.

The driver itself is quite simple.

@gschorcht
Copy link
Copy Markdown
Contributor Author

gschorcht commented Feb 6, 2023 •
edited
Loading

Looks good! There are some weird hangs

Unfortunately I don't have another board at hand that supports the periph_dac function.

Ah, I found my STM32F4 Discovery board that supports periph_dac. I have the same problem for this board.

@gschorcht
Copy link
Copy Markdown
Contributor Author

gschorcht commented Feb 6, 2023

Ah, I found my STM32F4 Discovery board that supports periph_dac.

BTW, it is quite annoying that this board still uses stdio_uart by default although we could use it with stdio_cdc_acm.

@benpicco
Copy link
Copy Markdown
Contributor

benpicco commented Feb 7, 2023 •
edited
Loading

Could it have something to do with the periodic timer?

That would be my first guess too. On same54-xpro there is no hang.

it is quite annoying that this board still uses stdio_uart by default although we could use it with stdio_cdc_acm.

ah I guess it's never been updated since we have stdio_cdc_acm, I didn't even know this also comes with one of the old non-UART capable ST-Links.

@gschorcht gschorcht force-pushed the cpu/gd32v/periph_dac branch from c46382a to 066b921 Compare February 7, 2023 01:42
@gschorcht
Copy link
Copy Markdown
Contributor Author

gschorcht commented Feb 7, 2023

I had to rebase after the merge of PR #19249 to resolve a conflict in doc.

@benpicco
Copy link
Copy Markdown
Contributor

benpicco commented Feb 7, 2023

bors merge

@bors
Copy link
Copy Markdown
Contributor

bors bot commented Feb 7, 2023

Build succeeded:

@bors bors bot merged commit dd2d336 into RIOT-OS:master Feb 7, 2023
@benpicco benpicco mentioned this pull request Feb 7, 2023
Merged
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023
@gschorcht gschorcht deleted the cpu/gd32v/periph_dac branch April 26, 2023 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@benpicco benpicco benpicco approved these changes

@leandrolanzieri leandrolanzieri Awaiting requested review from leandrolanzieri leandrolanzieri is a code owner

@jia200x jia200x Awaiting requested review from jia200x jia200x is a code owner

@MrKevinWeiss MrKevinWeiss Awaiting requested review from MrKevinWeiss MrKevinWeiss is a code owner

Assignees

No one assigned

Labels

Area: boards Area: Board ports Area: cpu Area: CPU/MCU ports Area: doc Area: Documentation Area: Kconfig Area: Kconfig integration Area: tools Area: Supplementary tools CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Type: new feature The issue requests / The PR implemements a new feature for RIOT

Projects

None yet

Milestone

Release 2023.04

Development

Successfully merging this pull request may close these issues.

4 participants

@gschorcht @riot-ci @benpicco @MrKevinWeiss