buserror / simavr

simavr is a lean, mean and hackable AVR simulator for linux & OSX
GNU General Public License v3.0
1.59k stars 369 forks source link

Pin change interrupts (PCINT) incorrectly fire when a timer compare event occurs #343

Open JarrettBillingsley opened 5 years ago

JarrettBillingsley commented 5 years ago

Once again, this code:

    ldi temp, _BV(DDB4) | _BV(DDB3)
    out IO(DDRB), temp

    ldi temp, 0
    out IO(OCR1B), temp
    out IO(OCR1A), temp
    ldi temp, 255
    out IO(OCR1C), temp

    ; Enable PWM mode on timer 1
    ldi temp, _BV(PWM1B) | _BV(COM1B1)
    out IO(GTCCR), temp

    ; Enable timer overflow interrupt
    in  temp, IO(TIMSK)
    ori temp, _BV(TOIE1)
    out IO(TIMSK), temp

    ; Hardware bug requires COM1A0 to be enabled as well.
    ; "3" means clock timer with CK/4 (16384 Hz sample rate)
    ldi temp, _BV(PWM1A) | _BV(COM1A1) | _BV(COM1A0) | 3
    out IO(TCCR1), temp

    ; turn on pin change interrupt for PB1
    ldi temp, _BV(PCINT1)
    out IO(PCMSK), temp
    ldi temp, _BV(PCIE)
    out IO(GIMSK), temp

The output compare A is required to be enabled in this code because of a hardware bug in real ATTiny85. It uses PB1, but PB1 is set as an input with a pin change interrupt.

On real hardware, the pin change interrupt only (correctly) triggers on an external edge. However in simavr, the behavior of OCR1A toggles PB1's output value, which should be ignored, but instead triggers the interrupt as well.

JarrettBillingsley commented 5 years ago

Quick patch:

diff --git a/simavr/sim/avr_ioport.c b/simavr/sim/avr_ioport.c
index e35b2de..287313d 100644
--- a/simavr/sim/avr_ioport.c
+++ b/simavr/sim/avr_ioport.c
@@ -154,7 +154,8 @@ avr_ioport_irq_notify(
        if (p->r_pcint) {
                // if the pcint bit is on, try to raise it
                int raise = avr->data[p->r_pcint] & mask;
-               if (raise)
+               int ddr = avr->data[p->r_ddr] & mask;
+               if (raise && ddr)
                        avr_raise_interrupt(avr, &p->pcint);
        }
 }