From 2f136762ea1e9d7fdd84413c09503ae9920b42cc Mon Sep 17 00:00:00 2001 From: ga Date: Fri, 4 Mar 2022 14:09:57 +0000 Subject: [PATCH] Fix issue #478 - input changes not reflected in VCD output file. Change the way I/O register IRQs are called so that they report changes made by peripheral code as well as the core. Also, do not call them for reads. Add some test code. --- simavr/sim/avr_ioport.c | 6 +++--- simavr/sim/sim_core.c | 32 +++++++++++++++++++++++++------- tests/test_atmega168_ioport.c | 27 ++++++++++++++++++++++++--- 3 files changed, 52 insertions(+), 13 deletions(-) diff --git a/simavr/sim/avr_ioport.c b/simavr/sim/avr_ioport.c index ca43896..91cf106 100644 --- a/simavr/sim/avr_ioport.c +++ b/simavr/sim/avr_ioport.c @@ -173,9 +173,9 @@ avr_ioport_irq_notify( } 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; + avr_core_watch_write(avr, p->r_pin, + (avr->data[p->r_pin] & ~mask) | + (value ? mask : 0)); /* BUG: If DDR bit is set here, there should be no * interrupt. But a spurious IRQ call by the user diff --git a/simavr/sim/sim_core.c b/simavr/sim/sim_core.c index 4bb2543..cc7285b 100644 --- a/simavr/sim/sim_core.c +++ b/simavr/sim/sim_core.c @@ -121,6 +121,20 @@ _avr_flash_read16le( return(avr->flash[addr] | (avr->flash[addr + 1] << 8)); } +static inline void _call_register_irqs(avr_t * avr, uint16_t addr) +{ + if (addr > 31 && addr < 31 + MAX_IOs) { + avr_io_addr_t io = AVR_DATA_TO_IO(addr); + + if (avr->io[io].irq) { + uint8_t v = avr->data[addr]; + avr_raise_irq(avr->io[io].irq + AVR_IOMEM_IRQ_ALL, v); + for (int i = 0; i < 8; i++) + avr_raise_irq(avr->io[io].irq + i, (v >> i) & 1); + } + } +} + void avr_core_watch_write(avr_t *avr, uint16_t addr, uint8_t v) { if (addr > avr->ramend) { @@ -155,6 +169,7 @@ void avr_core_watch_write(avr_t *avr, uint16_t addr, uint8_t v) } avr->data[addr] = v; + _call_register_irqs(avr, addr); } uint8_t avr_core_watch_read(avr_t *avr, uint16_t addr) @@ -172,6 +187,7 @@ uint8_t avr_core_watch_read(avr_t *avr, uint16_t addr) avr_gdb_handle_watchpoints(avr, addr, AVR_GDB_WATCH_READ); } +// _call_register_irqs(avr, addr); return avr->data[addr]; } @@ -192,14 +208,15 @@ static inline void _avr_set_r(avr_t * avr, uint16_t r, uint8_t v) } if (r > 31) { avr_io_addr_t io = AVR_DATA_TO_IO(r); - if (avr->io[io].w.c) + if (avr->io[io].w.c) { avr->io[io].w.c(avr, r, v, avr->io[io].w.param); - else + } else { avr->data[r] = v; - if (avr->io[io].irq) { - avr_raise_irq(avr->io[io].irq + AVR_IOMEM_IRQ_ALL, v); - for (int i = 0; i < 8; i++) - avr_raise_irq(avr->io[io].irq + i, (v >> i) & 1); + if (avr->io[io].irq) { + avr_raise_irq(avr->io[io].irq + AVR_IOMEM_IRQ_ALL, v); + for (int i = 0; i < 8; i++) + avr_raise_irq(avr->io[io].irq + i, (v >> i) & 1); + } } } else avr->data[r] = v; @@ -266,13 +283,14 @@ static inline uint8_t _avr_get_ram(avr_t * avr, uint16_t addr) if (avr->io[io].r.c) avr->data[addr] = avr->io[io].r.c(avr, addr, avr->io[io].r.param); - +#if 0 if (avr->io[io].irq) { uint8_t v = avr->data[addr]; avr_raise_irq(avr->io[io].irq + AVR_IOMEM_IRQ_ALL, v); for (int i = 0; i < 8; i++) avr_raise_irq(avr->io[io].irq + i, (v >> i) & 1); } +#endif } return avr_core_watch_read(avr, addr); } diff --git a/tests/test_atmega168_ioport.c b/tests/test_atmega168_ioport.c index 24e8c25..d4da24a 100644 --- a/tests/test_atmega168_ioport.c +++ b/tests/test_atmega168_ioport.c @@ -3,6 +3,8 @@ #include "tests.h" #include "avr_ioport.h" +#define DDRB_ADDR (0x4 + 32) // From ATM168 header file. + /* Start of the IOPORT's IRQ list. */ static avr_irq_t *base_irq; @@ -22,6 +24,16 @@ static void monitor_5(struct avr_irq_t *irq, uint32_t value, void *param) LOG("5-%02X ", value); } +/* IRQ call-back for changes to the simulated DDRB register. + * This is to test an IRQ returned by avr_iomem_getirq() and not really + * specifically for GPIO. + */ + +static void monitor_ddrb(struct avr_irq_t *irq, uint32_t value, void *param) +{ + LOG("BD-%02X ", value); +} + /* This monitors the simulator's idea of the I/O pin states. * Changes this program makes to inputs are not reported, * presumably because the simulator "knows" we made them. @@ -97,10 +109,11 @@ static const char *expected = /* This string is expected in variable log. */ static const char *log_expected = - "d-0F P-00 o-0A P-0A I-0A 5-01 o-09 P-29 d-3C 5-00 P-09 o-F0 5-01 P-F0 " - "I-70 " // Interrupts off testing. + "BD-01 d-0F P-00 o-0A P-0A I-0A 5-01 o-09 P-29 d-3C 5-00 P-09 o-F0 5-01 " + "P-F0 I-70 " // Interrupts off testing. "o-E0 P-E0 I-E0 I-E0 " // External interrupt test. - "d-03 o-01 P-E1 o-03 P-E3 o-00 P-E8 I-E8 "; // Pin change interrupt test. + "d-03 o-01 P-E1 o-03 P-E3 o-00 P-E8 I-E8 " // Pin change interrupt test. + "BD-FF "; // iomem IRQ test. int main(int argc, char **argv) { @@ -114,6 +127,8 @@ int main(int argc, char **argv) { monitor_5, NULL); avr_irq_register_notify(base_irq + IOPORT_IRQ_PIN_ALL, monitor, NULL); + avr_irq_register_notify(avr_iomem_getirq(avr, DDRB_ADDR, NULL, 8), + monitor_ddrb, NULL); avr_irq_register_notify(base_irq + IOPORT_IRQ_DIRECTION_ALL, reg_write, NULL); avr_irq_register_notify(base_irq + IOPORT_IRQ_REG_PORT, @@ -121,6 +136,12 @@ int main(int argc, char **argv) { avr_irq_register_notify(base_irq + IOPORT_IRQ_REG_PIN, reg_read, NULL); + /* Tweak DDRB to confirm IO-memory based IRQs are working. */ + + avr_core_watch_write(avr, DDRB_ADDR, 1); + + /* Run ... */ + tests_assert_uart_receive_avr(avr, 100000, expected, '0'); if (strcmp(log, log_expected)) -- 2.39.5