bugfix: Fix Test VCD Output

[atluft]

Support VCD dumps for IO registers with write handlers (#486), implements changes suggested by @jamesrwaugh

Testing

Running these commands

make
cd simavr
./run_avr ../tests/atmega88_example.axf
gtkwave gtkwave_trace.vcd

Output

./run_avr ../tests/atmega88_example.axf 
Loaded 1722 bytes of Flash data at 0
Loaded 114 bytes of Flash data at 0x6ba
Read from eeprom 0xdeadbeef -- should be 0xdeadbeef..
Read from eeprom 0xcafef00d -- should be 0xcafef00d..

[gatk555]

This looks good in principle, but there are a couple of issues:

If _avr_set_r() is called for a register with a direct write callback set, the following IRQ call will pass the value written by the CPU, not the actual register value. (Important example: interrupt flag registers with write-1-to-clear behaviour.) If the IRQ is called with the final value, there is then no need to modify _call_register_irqs(). Will it still work for the VCD file?

test_atmega168_ioport.tst is failing. The fault may be in the recently-modified test code.

[atluft]

@gatk555, thank you for the helpful comments.

Have a look at the changes in 1d323bf , passing all tests and VCD working.
I removed a code comment without understanding, maybe you can advise on the consequences of this approach?

       // The byte to be sent should NOT be written there,
       // the value written could never be read back.
       // avr_core_watch_write(avr, addr, v);

[gatk555]

I see you reverted the first commit, I was about to ask about that. I think a better way would be to remake the branch with just one commit, and force-push it. But unfortunately, the new change seems not suitable for merge. The problem is that this is a special case: UDR0 is not one register but two, with independent values for read and write. That is what the comment seems to be about. Saving the byte written into the single simulated register means that it may later appear as the last byte received!

Also, the problem with the first commit is that register-write callbacks in peripheral code are expected to work out the new value and store it with avr_core_watch_write() and that raises the IRQ. By doing it directly the IRQ functions are called twice.

So what might the right answer look like? My guess is to change atmega88_example.c so that it logs the UART_IRQ_OUTPUT IRQ instead of the ambiguous register. The problem is that there is currently no way to request that. Logging any IRQ to VCD seems a missing feature, and not hard to add.

[atluft]

@gatk555, thanks for the comments and clearing up the UDR0 read / write aspects. I will rework the branch and begin looking at logging the UART_IRQ_OUTPUT IRQ.

[gatk555]

To avoid duplication, you may want to hold back. The ability to log any peripheral IRQ is something I have been wanting for another project, and seems an obvious gap in the VCD support. For example, there is currently no good way to log GPIO input for playback.

Since yesterday, I have a prototype working including a modified atmega88_example.c:

/*
 * This small section tells simavr to generate a VCD trace dump with
 * UART output and changes to the control register's "empty" bit.
 * Opening it with gtkwave will show you the data being pumped out
 * and the UDRE0 bit being set, then cleared.
 */
const struct avr_mmcu_vcd_trace_t _mytrace[]  _MMCU_ = {
        { AVR_MCU_VCD_SYMBOL("UDRE0"), .mask = (1 << UDRE0), .what = (void*)&UCSR0A, },
};
AVR_MCU_VCD_IO_IRQ("uar0", 1 /* UART_IRQ_OUTPUT*/, "Uart_output");

It needs more testing. but I can push it on a temporary Git branch if that would be useful.

[atluft]

Sounds good, I don't have any specific need or timeline. My interest is to have the readme steps produce the VCD results and sample code above looks great for that.

[gatk555]

The VCD output was fixed by e829f2b, with related fixes in 5d42c8d.