Document the quirks of Symon's ACIA emulation.

Fixes #7.
Writing 0 to the 6551 ACIA's control register is not a program reset.
2023-06-11 08:21:37 -07:00 · 2023-06-11 08:21:37 -07:00 · 2023-06-11 08:21:37 -07:00 · 2023-06-11 08:21:37 -07:00
3 changed files with 166 additions and 75 deletions
--- a/README.md
+++ b/README.md
@ -69,14 +69,29 @@ memory.
 ![Serial Console](https://github.com/sethm/symon/raw/master/screenshots/console.png)

 The main window of the simulator acts as the primary Input/Output
-system through a virtual serial terminal. The terminal is attached to
-a simulated ACIA, including a programmable baud rate generator that
-tries to approximate the correct "feel" of the programmed baud rate.
-(The sample Enhanced BASIC ROM image is programmed for 9600 baud)
+system through a virtual serial terminal. It also provides CPU status.
+Contents of the accumulator, index registers, processor status flags,
+disassembly of the instruction register, and stack pointer are all displayed.

-It also provides CPU status. Contents of the accumulator, index
-registers, processor status flags, disassembly of the instruction
-register, and stack pointer are all displayed.
+The terminal is attached to a simulated MOS 6551 ACIA. It behaves very much
+as described in the datasheet, with some exceptions:
+
+  - The simulated ACIA is permanently connected to the virtual terminal,
+    the Data Carrier Detect and Data Set Ready status bits always indicate
+    a connection is ready.
+  - The parity, stop-bits and bits-per-character settings are ignored. The
+    ACIA always sends and receives 8-bit characters, and parity errors
+    do not occur.
+  - The ACIA tries to honour the configured baud rate, but as a special case
+    the default "16x External Clock" rate is interpreted to mean "as fast as
+    possible" (The sample Enhanced BASIC ROM image is programmed for 9600 baud).
+  - The ACIA ignores the configured state of the Data Terminal Ready pin;
+    it is always ready to receive and transmit.
+
+For more information on the MOS 6551 ACIA and its programming model,
+see the official datasheet:
+
+  - [MOS 6551 ACIA](http://archive.6502.org/datasheets/mos_6551_acia.pdf)

 ![Font Selection](https://github.com/sethm/symon/raw/master/screenshots/font_selection.png)

--- a/src/main/java/com/loomcom/symon/devices/Acia6551.java
+++ b/src/main/java/com/loomcom/symon/devices/Acia6551.java
@ -53,6 +53,14 @@ public class Acia6551 extends Acia {

    public Acia6551(int address) throws MemoryRangeException {
        super(address, ACIA_SIZE, "ACIA");
+
+        // Figure 6 in the 6551 ACIA data sheet says the "hardware reset"
+        // state of the Control Register is all zeros.
+        setControlRegister(0b00000000);
+        // Figure 7 of the 6551 ACIA data sheet says the "hardware reset"
+        // state of the Command Register is zeros, but Transmitter Control
+        // is set to "interrupt disabled, ready to send".
+        setCommandRegister(0b00000010);
    }

    @Override
@ -110,67 +118,60 @@ public class Acia6551 extends Acia {
        controlRegister = data;
        int rate = 0;

-        // If the value of the data is 0, this is a request to reset,
-        // otherwise it's a control update.
-
-        if (data == 0) {
-            reset();
-        } else {
-            // Mask the lower four bits to get the baud rate.
-            int baudSelector = data & 0x0f;
-            switch (baudSelector) {
-                case 0:
-                    rate = 0;
-                    break;
-                case 1:
-                    rate = 50;
-                    break;
-                case 2:
-                    rate = 75;
-                    break;
-                case 3:
-                    rate = 110; // Real rate is actually 109.92
-                    break;
-                case 4:
-                    rate = 135; // Real rate is actually 134.58
-                    break;
-                case 5:
-                    rate = 150;
-                    break;
-                case 6:
-                    rate = 300;
-                    break;
-                case 7:
-                    rate = 600;
-                    break;
-                case 8:
-                    rate = 1200;
-                    break;
-                case 9:
-                    rate = 1800;
-                    break;
-                case 10:
-                    rate = 2400;
-                    break;
-                case 11:
-                    rate = 3600;
-                    break;
-                case 12:
-                    rate = 4800;
-                    break;
-                case 13:
-                    rate = 7200;
-                    break;
-                case 14:
-                    rate = 9600;
-                    break;
-                case 15:
-                    rate = 19200;
-                    break;
-            }
-
-            setBaudRate(rate);
+        // Mask the lower four bits to get the baud rate.
+        int baudSelector = data & 0x0f;
+        switch (baudSelector) {
+            case 0:
+                rate = 0;
+                break;
+            case 1:
+                rate = 50;
+                break;
+            case 2:
+                rate = 75;
+                break;
+            case 3:
+                rate = 110; // Real rate is actually 109.92
+                break;
+            case 4:
+                rate = 135; // Real rate is actually 134.58
+                break;
+            case 5:
+                rate = 150;
+                break;
+            case 6:
+                rate = 300;
+                break;
+            case 7:
+                rate = 600;
+                break;
+            case 8:
+                rate = 1200;
+                break;
+            case 9:
+                rate = 1800;
+                break;
+            case 10:
+                rate = 2400;
+                break;
+            case 11:
+                rate = 3600;
+                break;
+            case 12:
+                rate = 4800;
+                break;
+            case 13:
+                rate = 7200;
+                break;
+            case 14:
+                rate = 9600;
+                break;
+            case 15:
+                rate = 19200;
+                break;
        }
+
+        setBaudRate(rate);
    }


@ -203,13 +204,16 @@ public class Acia6551 extends Acia {


    private synchronized void reset() {
-        txChar = 0;
-        txEmpty = true;
-        rxChar = 0;
-        rxFull = false;
-        receiveIrqEnabled = false;
-        transmitIrqEnabled = false;
-        interrupt = false;
-    }
+        // Figure 6 in the 6551 ACIA data sheet says the "program reset"
+        // event does not modify the control register.

+        // Figure 7 in the 6551 ACIA data sheet says the "program reset"
+        // event keeps the "parity check" configuration in the command
+        // register, but resets the other bits to defaults.
+        setCommandRegister((commandRegister & 0xe0) | 0x02);
+
+        // Figure 8 in the 6551 ACIA data sheet says the "program reset"
+        // event clears the "overrun" flag but otherwise has no effect.
+        overrun = false;
+    }
 }
--- a/src/test/java/com/loomcom/symon/AciaTest.java
+++ b/src/test/java/com/loomcom/symon/AciaTest.java
@ -254,4 +254,76 @@ public class AciaTest {

        assertEquals(0x08, acia.read(0x0001, true));
    }
+
+    @Test
+    public void statusRegisterInitializedAtHardwareReset() throws Exception {
+        Acia6551 acia = new Acia6551(0x0000);
+
+        assertEquals(0x10, acia.read(0x0001, false));
+    }
+
+    @Test
+    public void commandRegisterInitializedAtHardwareReset() throws Exception {
+        Acia6551 acia = new Acia6551(0x0000);
+
+        assertEquals(0x02, acia.read(0x0002, false));
+    }
+
+    @Test
+    public void controlRegisterInitializedAtHardwareReset() throws Exception {
+        Acia6551 acia = new Acia6551(0x0000);
+
+        assertEquals(0x00, acia.read(0x0003, false));
+    }
+
+    @Test
+    public void programResetClearsOverrunStatus() throws Exception {
+        Acia6551 acia = new Acia6551(0x0000);
+        Bus bus = new Bus(acia.ACIA_SIZE);
+        acia.setBus(bus);
+
+        // Change as many status bits as we can.
+        acia.write(0x0002, 0x00); // enable receive interrupt
+        acia.rxWrite('a');
+        acia.rxWrite('b'); // overrun, receive full, interrupt signalled
+        acia.write(0x0000, 'c'); // Transmitter Data Register not empty
+
+        // Check that all the bits we expected to be set actually are
+        assertEquals(0x8C, acia.read(0x0001, false));
+
+        // Do a "program reset". The value is ignored.
+        acia.write(0x0001, 0xFF);
+
+        // Check that only bit 2 was cleared.
+        assertEquals(0x88, acia.read(0x0001, false));
+    }
+
+    @Test
+    public void programResetKeepsParitySettings() throws Exception {
+        Acia6551 acia = new Acia6551(0x0000);
+
+        // Set all the command register bits
+        acia.write(0x0002, 0xFF);
+
+        // Do a "program reset". The value is ignored.
+        acia.write(0x0001, 0xFF);
+
+        // The top 3 bits should be kept as-is,
+        // the bottom 5 bits should be reset to defaults.
+        assertEquals(0xE2, acia.read(0x0002, false));
+    }
+
+    @Test
+    public void programResetLeavesControlRegisterUnchanged() throws Exception {
+        Acia6551 acia = new Acia6551(0x0000);
+
+        // Set all the control register bits
+        acia.write(0x0003, 0xFF);
+
+        // Do a "program reset". The value is ignored.
+        acia.write(0x0001, 0xFF);
+
+        // No bits should have changed.
+        assertEquals(0xFF, acia.read(0x0003, false));
+    }
 }
Author	SHA1	Message	Date
Tim Allen	4a8a803472	Document the quirks of Symon's ACIA emulation. Fixes #7.	2023-06-11 08:21:37 -07:00
Tim Allen	4423623816	Writing 0 to the 6551 ACIA's control register is not a program reset. So far as I can tell, ever since the first version of the ACIA emulation in Symon, writing 0x00 to the control register has been interpreted as a request to reset, rather than to actually set the control register to 0x00. This is strange for a number of reasons: - All-zeros is actually a very sensible value for the control register, and is in fact the hardware-reset default. - I can't find any description of such behaviour in the 6551, W65C51S, or W65C51N data sheets. - The 6551 already has a way to trigger "program reset" by writing to the status register. So I've removed that quirk, and writing to the control register now just writes to the control register and nothing else.	2023-06-11 08:21:37 -07:00
Tim Allen	5df775bbb0	Make the emulated 6551's soft ("program") reset state match the MOS datasheet. The only description of the effects of "program reset" in the original MOS datasheet is in the section for each register. The W65C51S and W65C51N datasheets have a heading "PROGRAM RESET OPERATION", but it amounts to: - internal registers are modified as described in the section for each register - changes to the DTR, DCD, and DSR pins which Symon does not emulate - the overrun flag is cleared ...which is what this new implementation does. It would make sense for the reset to do things like "cancel transmission or reception in progress" and stop asserting an interrupt, as the old code did, but I can't find any evidence of such behaviour in the datasheets.	2023-06-11 08:21:37 -07:00
Tim Allen	d076046f57	Make the emulated 6551's hardware reset state match the MOS datasheet.	2023-06-11 08:21:37 -07:00