Skip to content

cpu/samd21: pwm: allow to use channels > 3#13797

Merged
dylad merged 2 commits intoRIOT-OS:masterfrom
benpicco:cpu/samd21-pwm
Apr 8, 2020
Merged

cpu/samd21: pwm: allow to use channels > 3#13797
dylad merged 2 commits intoRIOT-OS:masterfrom
benpicco:cpu/samd21-pwm

Conversation

@benpicco
Copy link
Contributor

@benpicco benpicco commented Apr 2, 2020

Contribution description

Each TCC device has 8 capture-compare channels, but RIOT will crash when accessing any channel > 3.
This is because the upper for channels are on the CCB set of registers.

There was also a bit of a hack in place to get the TCC ID (and from that the GCLK_ID & APBCMASK) by masking out some bits from the register address of the hardware blocks.

But TCC devices are not always adjacent in memory - this already breaks for samd21 devices that have a TCC3 and is certainly broken for non-samd21 MCUs.

Testing procedure

Run tests/periph_pwm on a samd21 board with pwm configured (e.g. samr21-xpro)
The osci test should blink the on-board LED when configured properly.

Issues/PRs references

Needed for the PWM config of the serpente board #13654

Channels 4…7 are on the CCB register.
@benpicco benpicco added Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) Area: cpu Area: CPU/MCU ports labels Apr 2, 2020
@benpicco benpicco requested a review from MrKevinWeiss as a code owner April 2, 2020 23:21
@benpicco benpicco requested review from dylad and haukepetersen April 2, 2020 23:21
@benpicco benpicco added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Apr 2, 2020
GCLK_ID and APBCMASK entries are not always uniform.
The previous hack would already break for TCC3.

Just explosively write down the cases, there are only 5 at most.
Copy link
Member

@dylad dylad left a comment

Choose a reason for hiding this comment

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

Tested work on an out of the tree board with 3 PWM channels. (2 of them were broken on master).
Changes look good so let's wait for Murdock.

@dylad dylad added Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines labels Apr 8, 2020
@dylad dylad added this to the Release 2020.04 milestone Apr 8, 2020
@dylad dylad merged commit f9a4c50 into RIOT-OS:master Apr 8, 2020
@benpicco benpicco deleted the cpu/samd21-pwm branch April 8, 2020 13:46
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 CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Reviewed: 1-fundamentals The fundamentals of the PR were reviewed according to the maintainer guidelines Reviewed: 2-code-design The code design of the PR was reviewed according to the maintainer guidelines Reviewed: 3-testing The PR was tested according to the maintainer guidelines Reviewed: 4-code-style The adherence to coding conventions by the PR were reviewed according to the maintainer guidelines Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants