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

drivers: pwm: pwm_stm32.c: enable ARR preload #29619

Merged
merged 1 commit into from
Oct 30, 2020
Merged

drivers: pwm: pwm_stm32.c: enable ARR preload #29619

merged 1 commit into from
Oct 30, 2020

Conversation

ABOSTM
Copy link
Collaborator

@ABOSTM ABOSTM commented Oct 28, 2020

drivers: pwm: pwm_stm32.c: enable ARR preload

Enable ARR preload so that period or pulse updates are taken
into account synchronously with update event
(at the end of a ongoing period)
And thus avoid undetermined intermediate pulse.

Fixes #29446

Tested on Nucleo_f401re

@ABOSTM ABOSTM requested a review from gmarull as a code owner October 28, 2020 16:56
@ABOSTM
drivers: pwm: pwm_stm32.c: enable ARR preload
b71b831 
Enable ARR preload so that period or pulse updates are taken
into account synchronously with update event
(at the end of a ongoing period)
And thus avoid undetermined intermediate pulse.

Signed-off-by: Alexandre Bourdiol <[email protected]>
@ABOSTM ABOSTM added area: PWM Pulse Width Modulation platform: STM32 ST Micro STM32 labels Oct 28, 2020
@gmarull
Copy link
Member

gmarull commented Oct 28, 2020

Seems like a duplicate of #29470 (or at least related)

gmarull
gmarull approved these changes Oct 28, 2020
View reviewed changes
Copy link
Member

@gmarull gmarull left a comment

Choose a reason for hiding this comment

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

LGTM, this makes updates strictly synchronous to the update event.

As there was an already open PR trying to address the same observed issue, I think it would have been better to discuss the synchronous solution there to avoid 2 parallel PRs.

@gmarull gmarull mentioned this pull request Oct 28, 2020
Closed
@simonguinot
Copy link
Collaborator

I confirm I can't reproduce the #29446 bug with this patch. Thanks.

martinjaeger
martinjaeger approved these changes Oct 29, 2020
View reviewed changes
Copy link
Member

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

@ABOSTM
Copy link
Collaborator Author

ABOSTM commented Oct 29, 2020

As there was an already open PR trying to address the same observed issue, I think it would have been better to discuss the synchronous solution there to avoid 2 parallel PRs.

Yes, but to start the discussion, I wanted to shared my proposal so I needed to open a new PR.

@carlescufi carlescufi merged commit c47a5d4 into zephyrproject-rtos:master Oct 30, 2020
@bpbradley bpbradley mentioned this pull request Nov 13, 2020
Closed
@gmarull gmarull mentioned this pull request Apr 20, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmarull gmarull gmarull approved these changes

@martinjaeger martinjaeger martinjaeger approved these changes

@simonguinot simonguinot simonguinot approved these changes

@erwango erwango Awaiting requested review from erwango

Assignees
No one assigned
Labels
area: PWM Pulse Width Modulation platform: STM32 ST Micro STM32
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

pwm: stm32: output signal delayed
5 participants
@ABOSTM @gmarull @simonguinot @martinjaeger @carlescufi