From 2f136762ea1e9d7fdd84413c09503ae9920b42cc Mon Sep 17 00:00:00 2001
From: ga <ga@oldell.fish>
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