Add the missing pin change interrupts for 1280 and 1281.
In avr_ioport.[ch], fix multiple causes (but not all) of
mis-triggering of pin change interrupts including upstream issue #343:
Pin change interrupts incorrectly fire when a timer compare event occurs.
Other causes are initial setting of DDR and the AVR_IOPORT_OUTPUT bit.
Move the mask and shift data for ATmega2560 into the ioport structure.
Add a test for pin change interrupts.
AVR_EXTINT_MEGA_DECLARE(7, 'E', PE7, B),
},
AVR_IOPORT_DECLARE(a, 'A', A),
- .portb = {
- .name = 'B', .r_port = PORTB, .r_ddr = DDRB, .r_pin = PINB,
+ AVR_IOPORT_DECLARE_PC(b, 'B', B, 0), // PB0-7 have PCINT0-7
+ AVR_IOPORT_DECLARE(c, 'C', C),
+ AVR_IOPORT_DECLARE(d, 'D', D),
+ .porte = {
+ .name = 'E', .r_port = PORTE, .r_ddr = DDRE, .r_pin = PINE,
.pcint = {
- .enable = AVR_IO_REGBIT(PCICR, PCIE0),
- .raised = AVR_IO_REGBIT(PCIFR, PCIF0),
- .vector = PCINT0_vect,
+ .enable = AVR_IO_REGBIT(PCICR, PCIE1),
+ .raised = AVR_IO_REGBIT(PCIFR, PCIF1),
+ .vector = PCINT1_vect,
},
- .r_pcint = PCMSK0,
+ .r_pcint = PCMSK1,
+ .mask = 1, // PE0 has PCINT8
+ .shift = 0
},
- AVR_IOPORT_DECLARE(c, 'C', C),
- AVR_IOPORT_DECLARE(d, 'D', D),
- AVR_IOPORT_DECLARE(e, 'E', E),
AVR_IOPORT_DECLARE(f, 'F', F),
AVR_IOPORT_DECLARE(g, 'G', G),
AVR_IOPORT_DECLARE(h, 'H', H),
- AVR_IOPORT_DECLARE(j, 'J', J),
- AVR_IOPORT_DECLARE(k, 'K', K),
+ .portj = {
+ .name = 'J', .r_port = PORTJ, .r_ddr = DDRJ, .r_pin = PINJ,
+ .pcint = {
+ .enable = AVR_IO_REGBIT(PCICR, PCIE1),
+ .raised = AVR_IO_REGBIT(PCIFR, PCIF1),
+ .vector = PCINT1_vect,
+ },
+ .r_pcint = PCMSK1,
+ .mask = 0b11111110, // PJ0-6 have PCINT9-15
+ .shift = -1
+ },
+ AVR_IOPORT_DECLARE_PC(k, 'K', K, 2), // PK0-7 have PCINT16-23
AVR_IOPORT_DECLARE(l, 'L', L),
AVR_UARTX_DECLARE(0, PRR0, PRUSART0),
AVR_EXTINT_MEGA_DECLARE(7, 'E', PE7, B),
},
AVR_IOPORT_DECLARE(a, 'A', A),
- .portb = {
- .name = 'B', .r_port = PORTB, .r_ddr = DDRB, .r_pin = PINB,
+ AVR_IOPORT_DECLARE_PC(b, 'B', B, 0), // PB0-7 have PCINT0-7
+ AVR_IOPORT_DECLARE(c, 'C', C),
+ AVR_IOPORT_DECLARE(d, 'D', D),
+ .porte = {
+ .name = 'E', .r_port = PORTE, .r_ddr = DDRE, .r_pin = PINE,
.pcint = {
- .enable = AVR_IO_REGBIT(PCICR, PCIE0),
- .raised = AVR_IO_REGBIT(PCIFR, PCIF0),
- .vector = PCINT0_vect,
+ .enable = AVR_IO_REGBIT(PCICR, PCIE1),
+ .raised = AVR_IO_REGBIT(PCIFR, PCIF1),
+ .vector = PCINT1_vect,
},
- .r_pcint = PCMSK0,
+ .r_pcint = PCMSK1,
+ .mask = 1, // PE0 has PCINT8
+ .shift = 0
},
- AVR_IOPORT_DECLARE(c, 'C', C),
- AVR_IOPORT_DECLARE(d, 'D', D),
- AVR_IOPORT_DECLARE(e, 'E', E),
AVR_IOPORT_DECLARE(f, 'F', F),
AVR_IOPORT_DECLARE(g, 'G', G),
AVR_EXTINT_MEGA_DECLARE(7, 'E', PE7, B),
},
AVR_IOPORT_DECLARE(a, 'A', A),
- .portb = {
- .name = 'B', .r_port = PORTB, .r_ddr = DDRB, .r_pin = PINB,
- .pcint = {
- .enable = AVR_IO_REGBIT(PCICR, PCIE0),
- .raised = AVR_IO_REGBIT(PCIFR, PCIF0),
- .vector = PCINT0_vect,
- },
- .r_pcint = PCMSK0, // PB0-7 have PCINT0-7
- },
+ AVR_IOPORT_DECLARE_PC(b, 'B', B, 0), // PB0-7 have PCINT0-7
AVR_IOPORT_DECLARE(c, 'C', C),
AVR_IOPORT_DECLARE(d, 'D', D),
-#ifdef SPLIT_INTERRUPTS
.porte = {
.name = 'E', .r_port = PORTE, .r_ddr = DDRE, .r_pin = PINE,
.pcint = {
.enable = AVR_IO_REGBIT(PCICR, PCIE1),
.raised = AVR_IO_REGBIT(PCIFR, PCIF1),
.vector = PCINT1_vect,
- .mask = 1, // PE0 has PCINT8
- .shift = 0
},
.r_pcint = PCMSK1,
+ .mask = 1, // PE0 has PCINT8
+ .shift = 0
},
-#else
- AVR_IOPORT_DECLARE(e, 'E', E), // PE0 has PCINT8
-#endif
AVR_IOPORT_DECLARE(f, 'F', F),
AVR_IOPORT_DECLARE(g, 'G', G),
AVR_IOPORT_DECLARE(h, 'H', H),
.enable = AVR_IO_REGBIT(PCICR, PCIE1),
.raised = AVR_IO_REGBIT(PCIFR, PCIF1),
.vector = PCINT1_vect,
- .mask = 0b11111110, // PJ0-6 have PCINT9-15
- .shift = -1
},
.r_pcint = PCMSK1,
+ .mask = 0b11111110, // PJ0-6 have PCINT9-15
+ .shift = -1
},
- .portk = {
- .name = 'K', .r_port = PORTK, .r_ddr = DDRK, .r_pin = PINK,
- .pcint = {
- .enable = AVR_IO_REGBIT(PCICR, PCIE2),
- .raised = AVR_IO_REGBIT(PCIFR, PCIF2),
- .vector = PCINT2_vect,
- },
- .r_pcint = PCMSK2, // PK0-7 have PCINT16-23
- },
+ AVR_IOPORT_DECLARE_PC(k, 'K', K, 2), // PK0-7 have PCINT16-23
AVR_IOPORT_DECLARE(l, 'L', L),
AVR_UARTX_DECLARE(0, PRR0, PRUSART0),
int output = value & AVR_IOPORT_OUTPUT;
value &= 0xff;
uint8_t mask = 1 << irq->irq;
- // set the real PIN bit. ddr doesn't matter here as it's masked when read.
- avr->data[p->r_pin] &= ~mask;
- if (value)
- avr->data[p->r_pin] |= mask;
+ uint8_t ddr = avr->data[p->r_ddr];
- if (output) // if the IRQ was marked as Output, also do the IO write
- avr_ioport_write(avr, p->r_port, (avr->data[p->r_port] & ~mask) | (value ? mask : 0), p);
+ if (output) {
+ if ((mask & ddr) == 0)
+ return; // TODO: stop further processing of IRQ.
+
+ // If the IRQ was marked as Output, also do the IO write.
+
+ avr_ioport_write(avr,
+ p->r_port,
+ (avr->data[p->r_port] & ~mask) |
+ (value ? mask : 0),
+ p);
+ } else {
+ // Set the real PIN bit. Ignore DDR as it's masked when read.
+
+ avr->data[p->r_pin] &= ~mask;
+ if (value)
+ avr->data[p->r_pin] |= mask;
+
+ /* BUG: If DDR bit is set here, there should be no
+ * interrupt. But a spurious IRQ call by the user
+ * is indestinguishable from an internal one
+ * caused by writing the output port register and
+ * that should cause an interrupt. Doh!
+ */
+ }
if (p->r_pcint) {
+ // Ignore lingering copy of AVR_IOPORT_OUTPUT, or
+ // differing non-zero values.
+
+ if (!value == !(irq->value & 0xff))
+ return;
+
// if the pcint bit is on, try to raise it
+
int raisedata = avr->data[p->r_pcint];
- uint8_t uiRegMask = p->pcint.mask;
- int8_t iShift = p->pcint.shift;
+ uint8_t uiRegMask = p->mask;
+ int8_t iShift = p->shift;
+
if (uiRegMask) // If mask is 0, do nothing (backwards compat)
raisedata &= uiRegMask; // Mask off
// allocate this module's IRQ
avr_io_setirqs(&p->io, AVR_IOCTL_IOPORT_GETIRQ(p->name), IOPORT_IRQ_COUNT, NULL);
- for (int i = 0; i < IOPORT_IRQ_REG_PIN; i++)
+ for (int i = 0; i < IOPORT_IRQ_REG_PIN; i++) {
p->io.irq[i].flags |= IRQ_FLAG_FILTERED;
-
+ if (i < IOPORT_IRQ_PIN_ALL)
+ p->io.irq[i].flags &= ~IRQ_FLAG_INIT;
+ }
avr_register_io_write(avr, p->r_port, avr_ioport_write, p);
avr_register_io_read(avr, p->r_pin, avr_ioport_read, p);
avr_register_io_write(avr, p->r_pin, avr_ioport_pin_write, p);
avr_io_addr_t r_pin;
avr_int_vector_t pcint; // PCINT vector
- avr_io_addr_t r_pcint; // pcint 8 pins mask
+ avr_io_addr_t r_pcint; // pcint 8 pins mask
- // this represent the default IRQ value when
+ // Mask and shift for PCINTs. This is needed for chips like the 2560
+ // where PCINT do not align with IRQs.
+
+ uint8_t mask;
+ int8_t shift;
+
+ // This represent the default IRQ value when
// the port is set as input.
// If the mask is not set, no output value is sent
// on the output IRQ. If the mask is set, the specified
.name = _cname, .r_port = PORT ## _uname, .r_ddr = DDR ## _uname, .r_pin = PIN ## _uname, \
}
+#define AVR_IOPORT_DECLARE_PC(_lname, _cname, _uname, _pcnum) \
+ .port ## _lname = { \
+ .name = _cname, .r_port = PORT ## _uname, \
+ .r_ddr = DDR ## _uname, .r_pin = PIN ## _uname, \
+ .pcint = { \
+ .enable = AVR_IO_REGBIT(PCICR, PCIE ## _pcnum), \
+ .raised = AVR_IO_REGBIT(PCIFR, PCIF ## _pcnum), \
+ .vector = PCINT ## _pcnum ## _vect, \
+ }, \
+ .r_pcint = PCMSK ## _pcnum, \
+ }
+
#ifdef __cplusplus
};
#endif
--- /dev/null
+/*
+ atmega2560_pin_change.c
+
+ Test for pin_change interrupt simulation.
+ */
+
+#include <stdio.h>
+#include <avr/io.h>
+#include <avr/interrupt.h>
+#include <avr/sleep.h>
+
+#include "avr_mcu_section.h"
+
+/*
+ * This demonstrate how to use the avr_mcu_section.h file
+ * The macro adds a section to the ELF file with useful
+ * information for the simulator
+ */
+AVR_MCU(F_CPU, "atmega2560");
+
+static int uart_putchar(char c, FILE *stream)
+{
+ if (c == '\n')
+ uart_putchar('\r', stream);
+ loop_until_bit_is_set(UCSR3A, UDRE3);
+ UDR3 = c;
+ return 0;
+}
+
+static FILE mystdout = FDEV_SETUP_STREAM(uart_putchar, NULL,
+ _FDEV_SETUP_WRITE);
+
+volatile uint8_t interrupts;
+volatile char buffer[32];
+
+ISR(PCINT0_vect)
+{
+ buffer[interrupts++] = '0';
+}
+
+ISR(PCINT1_vect)
+{
+ buffer[interrupts++] = '1';
+}
+
+ISR(PCINT2_vect)
+{
+ buffer[interrupts++] = '2';
+}
+
+int main()
+{
+ stdout = &mystdout;
+ PCICR = (1 <<PCIE0) + (1 << PCIE1) + (1 <<PCIE2); // Enable interrupts.
+ PCMSK0 = 0xf0;
+
+ sei();
+
+ /* Test for interrupt caused external (timer) peripheral: no interrupt. */
+
+ PORTA = 0xff; // Triggers output to PORTB/4, no output.
+ PORTA = 0x0f; // Toogle output bit back to zero.
+ DDRB = 0xff; // Set ports for output.
+ buffer[interrupts++] = ' ';
+
+ /* Test for interrupt caused external (timer) peripheral: interrupts. */
+
+ PORTA = 0; // Triggers output to PORTB/4
+ buffer[interrupts++] = ' ';
+
+ DDRE = 0xff;
+ DDRJ = 0xff;
+ DDRK = 0xff;
+ PORTB = 0x20; // Interrupt
+ PORTE = 0xff; // No interrupt
+ PORTJ = 0x20; // No interrupt
+ PORTK = 0x20; // No interrupt
+ buffer[interrupts++] = ' ';
+ PORTB = 0x22; // No interrupt
+ PCMSK1 = 0x7f;
+ PCMSK2 = 0x7f;
+ PORTE = 0xfe; // Interrupt
+ PORTB = 0x32; // Interrupt
+ PORTK = 0x00; // Interrupt
+ PORTJ = 0x01; // Interrupt
+ cli();
+ buffer[interrupts++] = ' ';
+// PORTE = 0xff; // Fails! Double interrupt after sei().
+ PORTJ = 0xff;
+ sei(); // Only one interrupt
+ PORTK = 0x80; // No interrupt;
+ PORTB = 0x00; // Interrupt
+ PORTK = 0xC0; // Interrupt;
+
+ PCMSK1 = 0x7e;
+ PORTJ = 0x3F; // No interrupt;
+ PORTK = 0; // Interrupt;
+ PORTE = 0; // No interrupt;
+ PORTJ = 0xe0; // Interrupt;
+
+ /* Show that normal GPIO input works. */
+
+ DDRB = 0xff; // Set ports for output.
+ PORTA = 1; // Triggers input to PORTB/4
+
+ cli();
+ buffer[interrupts] = '\0';
+ fputs((const char *)buffer, stdout);
+ putchar('\n');
+
+ // this quits the simulator, since interupts are off
+ // this is a "feature" that allows running tests cases and exit
+
+ sleep_cpu();
+}
--- /dev/null
+#include "tests.h"
+#include "sim_avr.h"
+#include "avr_ioport.h"
+
+static avr_irq_t *twiddle_irq;
+
+/* Called on write to port A, twiddle PORTB/4 as peripheral might. */
+
+static void reg_write(struct avr_irq_t *irq, uint32_t value, void *param)
+{
+ static int count;
+ uint32_t flag;
+
+ /* Set the output flag on the first two calls.
+ *
+ * BUG: 3 should be 4 here, but it still works.
+ */
+
+ flag = (++count < 3) ? AVR_IOPORT_OUTPUT : 0;
+ avr_raise_irq(twiddle_irq, (twiddle_irq->value ^ 1) | flag);
+}
+
+int main(int argc, char **argv) {
+ static const char *expected = " 0 0 1021 102210\r\n";
+ avr_t *avr;
+
+ tests_init(argc, argv);
+ avr = tests_init_avr("atmega2560_pin_change.axf");
+
+ twiddle_irq = avr_io_getirq(avr,
+ AVR_IOCTL_IOPORT_GETIRQ('B'),
+ IOPORT_IRQ_PIN4);
+ avr_irq_register_notify(avr_io_getirq(avr,
+ AVR_IOCTL_IOPORT_GETIRQ('A'),
+ IOPORT_IRQ_REG_PORT),
+ reg_write, avr);
+
+ tests_assert_uart_receive_avr(avr, 10000000, expected, '3');
+ tests_success();
+ return 0;
+}
public / simavr.git / commitdiff