cpu/nrf52: add PWM implementation #9021
Conversation
| * @{ | ||
| */ | ||
| static const pwm_conf_t pwm_config[] = { | ||
| { NRF_PWM0, { 23, 24, 25, 26 } } |
There was a problem hiding this comment.
P0.23 doesn't seem to work here. Does it on your board?
There was a problem hiding this comment.
@PeterKietzmann Yes, it works for me on nrf52dk. Is it possible, that you are testing a different board with P0.23 already in use by another module?
There was a problem hiding this comment.
To be honest, I don't remember. But in my opinion any board that provides the PWM feature and makes use of this periph_conf.h should work with the provided configuration of at least be documented.
There was a problem hiding this comment.
I'll double check that, but i am confident 23 works for me, because i mostly used that pin for debugging.
There was a problem hiding this comment.
Would be great if you could check the manuals of the other nrf52 boards that we support. I think it's just 2-3...
|
|
||
| DEBUG("PWM started\n"); | ||
|
|
||
| return real_clk; |
There was a problem hiding this comment.
I assume this value has to be adapted in case of PWM_CENTER ?
init 0 1 10000 100
The pwm frequency is set to 10000
init 0 2 10000 100
The pwm frequency is set to 20000
However, frequency is 10k in both cases (as expected)
There was a problem hiding this comment.
@PeterKietzmann i intend on dropping center mode support. I already checked that there is no support in center mode for other CPUs, also center mode is rearly used anyway. If center mode has no special relevance to you, i will just drop it.
There was a problem hiding this comment.
I'm okay with dropping that feature.
| else { | ||
| pwm_seq[pwm][channel] = dev(pwm)->COUNTERTOP - value; | ||
| } | ||
|
|
There was a problem hiding this comment.
Can you please spend a note about the center mode here?
SemjonWilke
force-pushed
the
driver_periph_pwm_nrf52
branch
from
926e389
to
7cf60d8
Compare
|
@PeterKietzmann Okay, so i moved the pins and restricted the pwm pin configuration to both boards nrf52dk and nrf52840dk. I decided to use ifdefs, because it felt excessive to create new periph_conf files for both boards just to add 4 common pins each. |
| typedef enum { | ||
| PWM_LEFT = PWM_MODE(0, 1), /**< left aligned PWM */ | ||
| PWM_RIGHT = PWM_MODE(0, 0), /**< right aligned PWM */ | ||
| PWM_CENTER = PWM_MODE(1, 1), /**< not supported */ |
There was a problem hiding this comment.
I don't have a strong opinion about that. As i saw in other cpus (samd21,efm32) center mode is not supported but PWM_CENTER is set anyway. I did not question this design, but i think it gives easy access / guidance to anyone who wants to implement center mode in the future.
There was a problem hiding this comment.
Fine I leave it up to you. Please squash
SemjonWilke
force-pushed
the
driver_periph_pwm_nrf52
branch
from
7cf60d8
to
909bcef
Compare
PeterKietzmann
added
the
CI: ready for build
|
|
|
@SemjonKerner still around? Please fix Mudrocks complaints. My ACK holds |
SemjonWilke
force-pushed
the
driver_periph_pwm_nrf52
branch
from
909bcef
to
00ca499
Compare
SemjonWilke
force-pushed
the
driver_periph_pwm_nrf52
branch
from
00ca499
to
c0ee92e
Compare
|
@PeterKietzmann murdock is green |
This is an initial peripheral pwm implementation for nordic nrf52 cpu. This implementation provides all functions in pwm.h. It implements nordics hardware pwm. Also this PR adds nrf52dk board configs.