Skip to content

cpu/stm32_common/periph: only initialize defined PWM pins#6350

Closed
aabadie wants to merge 1 commit intoRIOT-OS:masterfrom
aabadie:fix_stm32_pwm
Closed

cpu/stm32_common/periph: only initialize defined PWM pins#6350
aabadie wants to merge 1 commit intoRIOT-OS:masterfrom
aabadie:fix_stm32_pwm

Conversation

@aabadie
Copy link
Copy Markdown
Contributor

@aabadie aabadie commented Jan 13, 2017

This fixes an issue when PWMs are defined with some of the first channels undefined (GPIO_UNDEF).
Config example in periph_conf.h:

[...]
.pins     = { GPIO_UNDEF, GPIO_PIN(PORT_B, 4),
                   GPIO_UNDEF, GPIO_UNDEF },
.chan    = 2
[...]

When running the tests/periph_pwm on a nucleo, I had a kernel panic:

*** RIOT kernel panic:
HARD FAULT HANDLER

The fix simply consists in ignoring the undefined pins at initialization.

@aabadie aabadie added Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors) labels Jan 13, 2017
@aabadie aabadie added this to the Release 2017.01 milestone Jan 13, 2017
@aabadie
Copy link
Copy Markdown
Contributor Author

aabadie commented Jan 13, 2017 •
edited
Loading

Just pushed an update that skip pwm_set if a channel is not defined.

@aabadie aabadie 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 Jan 13, 2017
@aabadie
Copy link
Copy Markdown
Contributor Author

aabadie commented Jan 16, 2017

@haukepetersen, this is a simple fix. It just miss a review (that should be fast).

@haukepetersen haukepetersen mentioned this pull request Jan 17, 2017
Merged
@haukepetersen
Copy link
Copy Markdown
Contributor

haukepetersen commented Jan 17, 2017

This fix does not suffice -> it will lead to a configuration of non-continuous (logical) channels, and that should not happen.

The fix should be, to include a mapping from logical channels (e.g. 0 to 1) to the actual timer channels (e.g. 1 and 3). This would be my approach: #6393

@aabadie
Copy link
Copy Markdown
Contributor Author

aabadie commented Jan 17, 2017

Agreed, this was the quick (and dirty?) solution. Looking at other cpu implementation I saw the same logic available for the samd21 so I thought it would be ok.

We can close this one if #6393 is the way to go.

@aabadie
Copy link
Copy Markdown
Contributor Author

aabadie commented Jan 17, 2017

then I'll have a few opened (and coming) PRs to update...

@aabadie aabadie closed this Jan 17, 2017
@PeterKietzmann
Copy link
Copy Markdown
Member

PeterKietzmann commented Jan 18, 2017

Closing in favour of #6393

@aabadie aabadie deleted the fix_stm32_pwm branch March 6, 2017 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@haukepetersen haukepetersen

Labels

CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR Platform: ARM Platform: This PR/issue effects ARM-based platforms Type: bug The issue reports a bug / The PR fixes a bug (including spelling errors)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@aabadie @haukepetersen @PeterKietzmann