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

[pwm] Correction to heartbeat mode #25717

Merged
merged 1 commit into from
Dec 20, 2024
Merged

Conversation

alees24
Copy link
Contributor

@alees24 alees24 commented Dec 19, 2024

Heartbeat direction reversal was premature in the event of passing the target duty cycle (non-wrapping case). The heartbeat direction shall only be changed at the end of the 'X'+1 pulse cycles for the current duty cycle.

This was found using the improved DV which is ongoing; the current DV environment overlooked this deviation from the specification.

@alees24 alees24 requested a review from rswarbrick December 19, 2024 22:58
Copy link
Contributor

@rswarbrick rswarbrick left a comment

Choose a reason for hiding this comment

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

A nitty comment, but this looks really nice to me. I particularly like the use of XOR, "taking reverse seriously" :-D

hw/ip/pwm/rtl/pwm_chan.sv Outdated Show resolved Hide resolved
hw/ip/pwm/rtl/pwm_chan.sv Show resolved Hide resolved
Heartbeat direction reversal was premature in the event
of passing the target duty cycle (non-wrapping case).
The heartbeat direction shall only be changed at the end of
the 'X'+1 pulse cycles for the current duty cycle.

Signed-off-by: Adrian Lees <a.lees@lowrisc.org>
@rswarbrick
Copy link
Contributor

CHANGE AUTHORIZED: hw/ip/pwm/rtl/pwm_chan.sv

This causes a behaviour change, but only by fixing a bug! Looks like the right thing to me.

@alees24 alees24 merged commit 45b3a60 into lowRISC:master Dec 20, 2024
36 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants