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

WDIE is wronly cleared on interrupt in Interrupt Mode #456

Open
WGH- opened this issue Jul 19, 2021 · 4 comments
Open

WDIE is wronly cleared on interrupt in Interrupt Mode #456

WGH- opened this issue Jul 19, 2021 · 4 comments

Comments

@WGH-
Copy link
Contributor

WGH- commented Jul 19, 2021

WDIE should be only cleared in "Interrupt and System Reset Mode".

The fix is to add && avr_regbit_get(avr, p->wde) condition.

static void avr_watchdog_irq_notify(
struct avr_irq_t * irq,
uint32_t value,
void * param)
{
avr_watchdog_t * p = (avr_watchdog_t *)param;
avr_t * avr = p->io.avr;
/* interrupt handling calls this twice...
* first when raised (during queuing), value = 1
* again when cleared (after servicing), value = 0
*/
if (!value && avr_regbit_get(avr, p->watchdog.raised)) {
avr_regbit_clear(avr, p->watchdog.enable);
}
}

I think I'll submit a PR (unless someone does that first) once I get access to hardware to double-check the tests on.

@lcgamboa
Copy link

Hi @WGH-

I have applied your fix suggestion to my simavr fork and now I can use WDT and Arduino_freeRTOS without any problems.
I recommend you do a PR. Thanks for the info.

@WGH-
Copy link
Contributor Author

WGH- commented Dec 10, 2021

@lcgamboa just to clarify, Arduino_freeRTOS depends on the correct behaviour of WDIE auto-clearing, doesn't work correctly on simavr, and my suggested change fixes it?

@lcgamboa
Copy link

Yes, I tested some examples from the Arduino_freeRTOS library and they all worked after the fix you suggested.

WGH- added a commit to WGH-/simavr that referenced this issue Dec 10, 2021
WDIE must be automatically cleared only in "Interrupt and System
Reset Mode" (where it's used to transition to "System Reset Mode").

In other modes, watchdog interrupt must not clear WDIE.

Strictly speaking, the modified condition also clears it in
"System Reset Mode" as well, but the system is reset on
interrupt anyway, so it doesn't matter.

See buserror#456
@itopaloglu83
Copy link

I was having issues with the watchdog timer interrupts and I'm glad to find this issue. Here's a simple example code for testing purposes. it uses the watchdog timer to blink an LED.

The watchdog timer interrupt only works once and then stops. I updated the interrupt clearing line with the following line of code as an interim solution.

Current: WDTCSR |= (1 << WDIF);
Temporary Solution: WDTCSR |= (1 << WDIE) | (1 << WDIF);

#include <avr/io.h>
#include <avr/wdt.h>
#include <avr/interrupt.h>

ISR(WDT_vect)
{
    // Clear interrupt flag by writing 1 to it.
    WDTCSR |= (1 << WDIF);
    // Toggle LED.
    PORTB ^= _BV(PB5);
}

int main()
{
    // Set LED as output.
    DDRB |= _BV(DDB5);

    // Enable watchdog timer.
    WDTCSR |= (1 << WDCE) | (1 << WDE);
    WDTCSR = (1 << WDIE);

    // Toggle LED.
    PORTB ^= _BV(PB5);

    // Enable global interrupts.
    sei();

    // Loop forever.
    while (1)
    {
        _NOP();
    }
}

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

No branches or pull requests

3 participants