Add a new tracing option, AVR_MMCU_TAG_VCD_IO_IRQ, that allows

any Input/Output IRQ to be logged in a VCD file.  Make it available
in run_avr.  Also add a new macro, AVR_MCU_VCD_REGISTER_BIT(),
to simplify the tracing of peripheral register bits.
Use these in atmega88_example.c, fixing the broken trace file reported
in Bug #486 - "Unable to reproduce tests/atmega88_example.axf results"
as an alternative to PR #547 - "bugfix: Fix Test VCD Output".

Author: ga

simavr/sim/avr/avr_mcu_section.h

@@ -64,6 +64,7 @@ enum {
 	AVR_MMCU_TAG_VCD_IRQ,
 	AVR_MMCU_TAG_VCD_SRAM_8,
 	AVR_MMCU_TAG_VCD_SRAM_16,
+	AVR_MMCU_TAG_VCD_IO_IRQ,
 	AVR_MMCU_TAG_PORT_EXTERNAL_PULL,
 };
 
@@ -109,6 +110,14 @@ struct avr_mmcu_vcd_trace_t {
 	char name[32];
 } __attribute__((__packed__));
 
+struct avr_mmcu_vcd_ioirq_t {
+	uint8_t tag;
+	uint8_t len;
+	uint8_t irqno;
+	char ioctl[5];	// Allow for null in string initialiser.
+	char name[32];
+} __attribute__((__packed__));
+
 #define AVR_MCU_STRING(_tag, _str) \
 	const struct avr_mmcu_string_t _##_tag _MMCU_ = {\
 		.tag = _tag,\
@@ -122,6 +131,7 @@ struct avr_mmcu_vcd_trace_t {
  */
 #define DO_CONCAT2(_a, _b) _a##_b
 #define DO_CONCAT(_a, _b) DO_CONCAT2(_a,_b)
+#define STRINGIFY(_a) #_a
 
 #define AVR_MCU_LONG(_tag, _val) \
 	const struct avr_mmcu_long_t DO_CONCAT(DO_CONCAT(_, _tag), __LINE__) _MMCU_ = {\
@@ -151,6 +161,25 @@ struct avr_mmcu_vcd_trace_t {
 	.len = sizeof(struct avr_mmcu_vcd_trace_t) - 2,\
 	.name = _name
 
+/*!
+ * Independent macro to trace a bit in an I/O register.
+ */
+
+#define AVR_MCU_VCD_REGISTER_BIT(_register, _bit) \
+	const struct avr_mmcu_vcd_trace_t \
+	  DO_CONCAT(_REGBIT_##_register##_##_bit##_, __LINE__) _MMCU_ = {	\
+		.tag = AVR_MMCU_TAG_VCD_TRACE, \
+		.len = sizeof(struct avr_mmcu_vcd_trace_t) - 2, \
+		.mask = (1 << _bit), \
+		.what = (void*)&_register, \
+		.name = STRINGIFY(_register##_##_bit),	\
+	}
+/*DO_CONCAT(DO_CONCAT(DO_CONCAT(_REGBIT, _register), DO_CONCAT(_, _bit)), \ */
+
+/*!
+ * Set traces on a RAM location, 8 or 15-bit.
+ */
+
 #define AVR_MCU_VCD_SRAM_8(_name) \
 	.tag = AVR_MMCU_TAG_VCD_SRAM_8, \
 	.len = sizeof(struct avr_mmcu_vcd_trace_t) - 2,\
@@ -213,7 +242,7 @@ struct avr_mmcu_vcd_trace_t {
 /*!
  * Add this port/pin to the VCD file. The syntax uses the name of the
  * port as a character, and not a pointer to a register.
- * AVR_MCU_VCD_PORT_PIN('B', 5);
+ * AVR_MCU_VCD_PORT_PIN('B', 5, "trace_name");
  */
 #define AVR_MCU_VCD_PORT_PIN(_port, _pin, _name) \
 	const struct avr_mmcu_vcd_trace_t DO_CONCAT(DO_CONCAT(_, _tag), __LINE__) _MMCU_ = {\
@@ -247,6 +276,21 @@ struct avr_mmcu_vcd_trace_t {
 #define AVR_MCU_VCD_ALL_IRQ_PENDING() \
 	AVR_MCU_VCD_IRQ_TRACE(0xff, 0, "IRQ_PENDING")
 
+/*!
+ * Above, "IRQ" refers to the IRQs associated with interrupts,
+ * but calls to any of Simavr's peripheral IRQs may be logged to a VCD file.
+ */
+
+#define AVR_MCU_VCD_IO_IRQ(_ioctl, _select, _trace_name) \
+	const struct avr_mmcu_vcd_ioirq_t \
+	  DO_CONCAT(_IO_IRQ_##_ioctl##_##_select##_, __LINE__) _MMCU_ = {	\
+		.tag = AVR_MMCU_TAG_VCD_IO_IRQ, \
+		.len = sizeof(struct avr_mmcu_vcd_ioirq_t) - 2,\
+		.irqno = _select, \
+		.ioctl = #_ioctl, \
+		.name = _trace_name, \
+	};
+
 /*!
  * This tag allows you to specify the voltages used by your board
  * It is optional in most cases, but you will need it if you use

simavr/sim/sim_elf.c

@@ -330,6 +330,10 @@ avr_load_firmware(
 						&bit[firmware->trace[ti].addr],
 						firmware->trace[ti].mask == 0xff ? 8 : 1,
 						firmware->trace[ti].name);
+		} else if (firmware->trace[ti].kind == AVR_MMCU_TAG_VCD_IO_IRQ) {
+			avr_irq_t * irq = avr_io_getirq(avr, firmware->trace[ti].addr, firmware->trace[ti].mask);
+			if (irq)
+				avr_vcd_add_signal(avr->vcd, irq, 8, firmware->trace[ti].name);
 		} else if ( (firmware->trace[ti].kind == AVR_MMCU_TAG_VCD_SRAM_8) ||
 		    (firmware->trace[ti].kind == AVR_MMCU_TAG_VCD_SRAM_16) ) {
 			if ((firmware->trace[ti].addr <= 31) || (firmware->trace[ti].addr > avr->ramend)) {
@@ -462,7 +466,6 @@ elf_parse_mmcu_section(
 				uint8_t mask = src[0];
 				uint16_t addr = src[1] | (src[2] << 8);
 				char * name = (char*)src + 3;
-
 #if 0
 				AVR_LOG(NULL, LOG_DEBUG,
 						"VCD_TRACE %d %04x:%02x - %s\n", tag,
@@ -475,6 +478,20 @@ elf_parse_mmcu_section(
 					sizeof(firmware->trace[firmware->tracecount].name));
 				firmware->tracecount++;
 			}	break;
+			case AVR_MMCU_TAG_VCD_IO_IRQ: {
+				uint8_t   mask = src[0];
+				uint32_t  ioctl;
+				char     *name;
+
+				ioctl = AVR_IOCTL_DEF(src[1], src[2], src[3], src[4]);
+				name = (char*)src + 6;
+				firmware->trace[firmware->tracecount].kind = tag;
+				firmware->trace[firmware->tracecount].mask = mask;
+				firmware->trace[firmware->tracecount].addr = ioctl;
+				strncpy(firmware->trace[firmware->tracecount].name, name,
+					sizeof(firmware->trace[firmware->tracecount].name));
+				firmware->tracecount++;
+			}	break;
 			case AVR_MMCU_TAG_VCD_FILENAME: {
 				strcpy(firmware->tracename, (char*)src);
 			}	break;

simavr/sim/sim_elf.h

@@ -69,7 +69,7 @@ typedef struct elf_firmware_t {
 	struct {
 		uint8_t kind;
 		uint8_t mask;
-		uint16_t addr;
+		uint32_t addr; // May be an ioctl value.
 		char	name[64];
 	} trace[32];
 

tests/atmega88_example.c

@@ -21,16 +21,17 @@
 AVR_MCU(F_CPU, "atmega88");
 
 /*
- * This small section tells simavr to generate a VCD trace dump with changes to these
- * registers.
- * Opening it with gtkwave will show you the data being pumped out into the data register
- * UDR0, and the UDRE0 bit being set, then cleared
+ * This small section tells simavr to generate a VCD trace dump.
+ * Opening it with gtkwave will show you the data being pumped out into
+ * the data register UDR0, and the UDRE0 ("empty") bit being set, then cleared
+ *
+ * The output bytes are logged as the value of an IRQ, because the special
+ * nature of UDRE0 (separate read and write buffers) means that logging
+ * the register UDRE0 would return any data received by the UART.
  */
-const struct avr_mmcu_vcd_trace_t _mytrace[]  _MMCU_ = {
-	{ AVR_MCU_VCD_SYMBOL("UDR0"), .what = (void*)&UDR0, },	
-	{ AVR_MCU_VCD_SYMBOL("UDRE0"), .mask = (1 << UDRE0), .what = (void*)&UCSR0A, },	
-};
 
+AVR_MCU_VCD_REGISTER_BIT(UCSR0A, UDRE0);
+AVR_MCU_VCD_IO_IRQ(uar0, 1 /* UART_IRQ_OUTPUT */, "UART_output");
 
 /* declare this in a .eeprom ELF section */
 uint32_t value EEMEM = 0xdeadbeef;

tests/tests.c

@@ -139,12 +139,20 @@ int tests_run_test(avr_t *avr, unsigned long run_usec) {
 		// setjmp() returned directly, run avr
 		while (1)
 			my_avr_run(avr);
-	} else if (reason == 1) {
-		// returned from longjmp(); cycle timer fired
-		return reason;
-	} else if (reason == 2) {
-		// returned from special deinit, avr stopped
-		return reason;
+	} else {
+		/* Normal cleanup before exit, includes VCD flush. */
+
+		avr->custom.deinit = NULL;
+		avr_terminate(avr);
+
+		if (reason == 1) {
+			// returned from longjmp(); cycle timer fired
+
+			return reason;
+		} else if (reason == 2) {
+			// returned from special deinit, avr stopped
+			return reason;
+		}
 	}
 	fail("Error in test case: Should never reach this.");
 	return 0;