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

cpu/efm32: pwm_init errors are zeros #19405

Merged
merged 2 commits into from Mar 17, 2023

Conversation

chrysn
Copy link
Member

@chrysn chrysn commented Mar 17, 2023

Contribution description

pwm_init is documented to return 0 on errors, and has an unsigned return value.

EFM32's initialization function returned negative values, which through implicit casting become 0xffffffff or 0xfffffffe, which are successful by the documentation.

This makes all the EFM32 error paths return 0 as they should.

Also, it fixes a variable name and the corresponding error message that used to talk of "initiation" (which would be the start of a process) rather than "initialization" (which is a process that is completed before something else can happen).

Testing procedure

  • on stk3700, tests/periph_pwm, run init 0 0 10 1000 / set 0 0 500
  • The init used to respond with "The pwm frequency is set to 4294967294", and the set does nothing.
  • The init now responds with "Error: device is not initiatedinitialized". The set still does nothing, but then one doesn't expect it to any more.

(But really, looking at the patch and the docs should suffice).

Issues/PRs references

By-catch from testing the Rust wrappers provided by @Remmirad at RIOT-OS/rust-riot-wrappers#38

@chrysn chrysn added Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Area: cpu Area: CPU/MCU ports labels Mar 17, 2023
@github-actions github-actions bot added Area: tests Area: tests and testing framework Platform: ARM Platform: This PR/issue effects ARM-based platforms labels Mar 17, 2023
@riot-ci
Copy link

riot-ci commented Mar 17, 2023

Murdock results

✔️ PASSED

6c8d1eb tests/periph_pwm: wording fix, s/initiate/initialize/

Success Failures Total Runtime
6882 0 6882 10m:11s

Artifacts

@benpicco benpicco requested a review from jue89 March 17, 2023 12:36
@benpicco
Copy link
Contributor

bors merge

@chrysn
Copy link
Member Author

chrysn commented Mar 17, 2023

Thanks for the quick ACK.

By the way, I did also check the other pwm.c implementations: They're all good -- they either return 0, or they just assert that their parameters are right. (One might argue for either behavior to be better, but that's not this issue, and frankly, I don't think it matters much).

@bors
Copy link
Contributor

bors bot commented Mar 17, 2023

Build succeeded:

@bors bors bot merged commit 2fcedf9 into RIOT-OS:master Mar 17, 2023
27 checks passed
@chrysn chrysn deleted the pwm-has-zero-errors branch March 17, 2023 16:04
@MrKevinWeiss MrKevinWeiss added this to the Release 2023.04 milestone Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: cpu Area: CPU/MCU ports Area: tests Area: tests and testing framework CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Impact: minor The PR is small in size and might only require a quick look of a knowledgeable reviewer Platform: ARM Platform: This PR/issue effects ARM-based platforms
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants