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.

Sign up for GitHub

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

Jump to bottom

cpu/gd32v: add periph_dac support #19248

Merged
merged 4 commits into from Feb 7, 2023
Merged

cpu/gd32v: add periph_dac support #19248

merged 4 commits into from Feb 7, 2023

Conversation

gschorcht
Copy link
Contributor

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
@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

riot-ci commented Feb 6, 2023 •
edited

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
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
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 <benjamin.valentin@ml-pa.com>
Co-authored-by: Gunar Schorcht <gunar@schorcht.net>
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
Contributor Author

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
Contributor

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
Contributor Author

Choose a reason for hiding this comment

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

Done

@gschorcht
Copy link
Contributor Author

bors cancel

@bors
Copy link
Contributor

bors bot commented Feb 6, 2023

Canceled.

@gschorcht
Copy link
Contributor Author

gschorcht commented Feb 6, 2023 •
edited

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
Contributor Author

gschorcht commented Feb 6, 2023 •
edited

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
Contributor Author

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
Contributor

benpicco commented Feb 7, 2023 •
edited

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
Copy link
Contributor Author

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

@benpicco
Copy link
Contributor

benpicco commented Feb 7, 2023

bors merge

@bors
Copy link
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.

None yet
4 participants
@gschorcht @riot-ci @benpicco @MrKevinWeiss