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

fix(PeriphDrivers): Fix Stuck Timer When PWM is set to 0 #818

Merged
merged 1 commit into from
Dec 14, 2023

Conversation

ozersa
Copy link
Contributor

@ozersa ozersa commented Dec 6, 2023

Pull Request Template

Description

For PWM mode when timer period end the tmr->count is reset to 0x00000001 and the timer resumes counting.
On edge case (pwm==0) flow stuck in while loop, tmr->cnt newer become bigger than pwm so that +8 added.

Checklist Before Requesting Review

  • PR Title follows correct guidelines.
  • Description of changes and all other relevant information.
  • (Optional) Link any related GitHub issues using a keyword
  • (Optional) Provide info on any relevant functional testing/validation. For API changes or significant features, this is not optional.

Test detail

If tolerance value be +8
image

If tolerance value be +2
image

As can be seen below due to if checks it is hard to catch exact 0x000001 value, there should be a tolerance value, so that i set it as 8 (TMR clock is PCLK, prescaler=1)
image

* But there should be a tolerance too, to it be catched
* So that +8 added as toleransa value.
*/
while (tmr->cnt >= (pwm + 8)) {}
Copy link
Contributor

@Jake-Carter Jake-Carter Dec 9, 2023

Choose a reason for hiding this comment

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

@ozersa What about monitoring the interrupt flag set by the hardware?

image

We only need to do this check if the timer is currently running. Something like:

int MXC_TMR_RevB_SetPWM(mxc_tmr_revb_regs_t *tmr, uint32_t pwm)
{
    int tmr_id = MXC_TMR_GET_IDX((mxc_tmr_regs_t *)tmr);
    (void)tmr_id;
    MXC_ASSERT(tmr_id >= 0);

    if (pwm > (tmr->cmp)) {
        return E_BAD_PARAM;
    }

    bool timera_is_running = tmr->ctrl0 & MXC_F_TMR_CTRL0_EN_A;
    bool timerb_is_running = tmr->ctrl0 & MXC_F_TMR_CTRL0_EN_B;

    if (timera_is_running || timerb_is_running) {
        MXC_TMR_RevB_ClearFlags(tmr); // Clear flags so we can catch the next one
        while (!MXC_TMR_RevB_GetFlags(tmr)) {} // Wait for next PWM transition
        MXC_TMR_RevB_Stop(tmr); // Pause timer
        MXC_TMR_RevB_SetCount(tmr, 0); // Reset the count
        MXC_TMR_RevB_ClearFlags(tmr); // Clear flags since app code wants the new PWM transitions set by this function
    }

    tmr->pwm = pwm;
    while (!(tmr->intfl & MXC_F_TMR_REVB_INTFL_WRDONE_A)) {}

    if (timera_is_running) {
        tmr->ctrl0 |= MXC_F_TMR_REVB_CTRL0_EN_A; // Unpause A
    }

    if (timerb_is_running) {
        tmr->ctrl0 |= MXC_F_TMR_REVB_CTRL0_EN_B; // Unpause B
    }

    return E_NO_ERROR;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The solution works.
Commit updated.

when timer period end the tmr->count is reset to 0x00000001
and the timer resumes counting. On edge case, when pwm==0 code stuck in while loop
Replace while loop with a smart logic that check current status of pwm
and reset counter/flags then start a new one

Co-authored-by: Jake Carter <[email protected]>
Signed-off-by: Sadik Ozer <[email protected]>
@sihyung-maxim sihyung-maxim changed the title fix(PeriphDrivers): Timer PWM mode issue (pwm==0) fixed fix(PeriphDrivers): Fix Timer PWM mode (pwm==0) edge case issue Dec 13, 2023
@Jake-Carter Jake-Carter changed the title fix(PeriphDrivers): Fix Timer PWM mode (pwm==0) edge case issue fix(PeriphDrivers): Fix Stuck Timer When PWM is set to 0 Dec 14, 2023
@Jake-Carter Jake-Carter merged commit 2c062d6 into main Dec 14, 2023
11 of 13 checks passed
@Jake-Carter Jake-Carter deleted the fix-timer_issue branch December 14, 2023 03:13
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.

3 participants