whitequark · Mar 1, 2019
diff --git a/firmware/fpga.c b/firmware/fpga.c
@@ -10,13 +10,13 @@ void fpga_reset() {
 
   // Put FPGA in reset.
   switch(glasgow_config.revision) {
-    case 'A':
-    case 'B':
+    case GLASGOW_REV_A:
+    case GLASGOW_REV_B:
       OED |=  (1<<PIND_CRESET_N_REVAB);
       IOD &= ~(1<<PIND_CRESET_N_REVAB);
       break;
 
-    case 'C':
+    case GLASGOW_REV_C0:
       OEA |=  (1<<PINA_CRESET_N_REVC);
       IOA &= ~(1<<PINA_CRESET_N_REVC);
       break;
@@ -31,12 +31,12 @@ void fpga_reset() {
 
   // Release FPGA reset.
   switch(glasgow_config.revision) {
-    case 'A':
-    case 'B':
+    case GLASGOW_REV_A:
+    case GLASGOW_REV_B:
       IOD |=  (1<<PIND_CRESET_N_REVAB);
       break;
 
-    case 'C':
+    case GLASGOW_REV_C0:
       IOA |=  (1<<PINA_CRESET_N_REVC);
       break;
   }

diff --git a/firmware/glasgow.h b/firmware/glasgow.h
@@ -6,6 +6,15 @@
 #define VID_QIHW              0x20b7
 #define PID_GLASGOW           0x9db1
 
+enum {
+  // Board revisions
+  GLASGOW_REV_A  = 0x10,
+  GLASGOW_REV_B  = 0x20,
+  GLASGOW_REV_C0 = 0x30,
+
+  GLASGOW_REV_NA = 0xF9,
+};
+
 // PORTA pins
 #define PINA_ALERT_N          0
 #define PINA_CDONE            3

diff --git a/firmware/main.c b/firmware/main.c
@@ -135,7 +135,7 @@ usb_configuration_set_c usb_configs[] = {
 usb_ascii_string_c usb_strings[] = {
   [0] = "whitequark research",
   [1] = "Glasgow Debug Tool",
-  [2] = "X-XXXXXXXXXXXXXXXX",
+  [2] = "XX-XXXXXXXXXXXXXXXX",
   // Configurations
   [3] = "Pipe Q at {2x512B EP2OUT/EP6IN}, R at {2x512B EP4OUT/EP8IN}",
   [4] = "Pipe Q at {4x512B EP2OUT/EP6IN}",
@@ -206,20 +206,52 @@ static void config_init() {
 fail:
   // Configuration block is corrupted or missing, load default configuration so that
   // we don't hang or present nonsensical descriptors.
-  glasgow_config.revision = 'Z';
+  glasgow_config.revision = GLASGOW_REV_NA;
   xmemcpy((__xdata void *)glasgow_config.serial, (__xdata void *)"9999999999999999",
           sizeof(glasgow_config.serial));
   glasgow_config.bitstream_size = 0;
 }
 
+// Upgrade legacy revision encoding.
+static void config_fixup() {
+  __xdata uint8_t data;
+
+  switch(glasgow_config.revision) {
+    case 'A': glasgow_config.revision = GLASGOW_REV_A;  break;
+    case 'B': glasgow_config.revision = GLASGOW_REV_B;  break;
+    case 'C': glasgow_config.revision = GLASGOW_REV_C0; break;
+    default: return;
+  }
+
+  // Invalidate the old firmware (if any), since it will get confused if it sees new revision
+  // field contents.
+  data = 0x01; // I2C_400KHZ, no DISCON
+  eeprom_write(I2C_ADDR_FX2_MEM, 7,
+               (__xdata void *)&data, 1,
+               /*double_byte=*/true, /*page_size=*/8, /*timeout=*/255);
+  data = 0xC0; // C0 load
+  eeprom_write(I2C_ADDR_FX2_MEM, 0,
+               (__xdata void *)&data, 1,
+               /*double_byte=*/true, /*page_size=*/8, /*timeout=*/255);
+  // Update Device ID and revision fields.
+  data = glasgow_config.revision;
+  eeprom_write(I2C_ADDR_FX2_MEM, 5,
+               (__xdata void *)&data, 1,
+               /*double_byte=*/true, /*page_size=*/8, /*timeout=*/255);
+  eeprom_write(I2C_ADDR_FX2_MEM, 8 + 4 + __builtin_offsetof(struct glasgow_config, revision),
+               (__xdata void *)&data, 1,
+               /*double_byte=*/true, /*page_size=*/8, /*timeout=*/255);
+}
+
 // Populate descriptors from device configuration, if any.
 static void descriptors_init() {
   __xdata struct usb_desc_device *desc_device = (__xdata struct usb_desc_device *)usb_device;
   __xdata char *desc_serial = (__xdata char *)usb_strings[usb_device.iSerialNumber - 1];
 
-  desc_device->bcdDevice |= 1 + glasgow_config.revision - 'A';
-  desc_serial[0] = glasgow_config.revision;
-  xmemcpy(&desc_serial[2], (__xdata void *)glasgow_config.serial,
+  desc_device->bcdDevice |= glasgow_config.revision;
+  desc_serial[0] = 'A' + (glasgow_config.revision >> 4) - 1;
+  desc_serial[1] = '0' + (glasgow_config.revision & 0xF);
+  xmemcpy(&desc_serial[3], (__xdata void *)glasgow_config.serial,
           sizeof(glasgow_config.serial));
 }
 
@@ -667,7 +699,7 @@ void handle_pending_usb_setup() {
 
     if(arg_get) {
       while(EP0CS & _BUSY);
-      if(glasgow_config.revision < 'C' ||
+      if(glasgow_config.revision < GLASGOW_REV_C0 ||
          !iobuf_get_pull(arg_selector,
                          (__xdata uint8_t *)EP0BUF + 0,
                          (__xdata uint8_t *)EP0BUF + 1)) {
@@ -678,7 +710,7 @@ void handle_pending_usb_setup() {
     } else {
       SETUP_EP0_BUF(2);
       while(EP0CS & _BUSY);
-      if(glasgow_config.revision < 'C' ||
+      if(glasgow_config.revision < GLASGOW_REV_C0 ||
          !iobuf_set_pull(arg_selector,
                          *((__xdata uint8_t *)EP0BUF + 0),
                          *((__xdata uint8_t *)EP0BUF + 1))) {
@@ -759,6 +791,7 @@ int main() {
 
   // Initialize subsystems.
   config_init();
+  config_fixup();
   descriptors_init();
   leds_init();
   iobuf_init_dac_ldo();

diff --git a/software/glasgow/applet/__init__.py b/software/glasgow/applet/__init__.py
@@ -24,7 +24,7 @@ def __init_subclass__(cls, name, **kwargs):
     preview = False
     help = "applet help missing"
     description = "applet description missing"
-    required_revision = "A"
+    required_revision = "A0"
 
     @classmethod
     def add_build_arguments(cls, parser, access):
@@ -186,7 +186,7 @@ def setUp(self):
 
     def assertBuilds(self, access="direct", args=[]):
         if access == "direct":
-            target = GlasgowHardwareTarget(revision="A",
+            target = GlasgowHardwareTarget(revision="A0",
                                            multiplexer_cls=DirectMultiplexer)
             access_args = DirectArguments(applet_name=self.applet.name,
                                           default_port="AB", pin_count=16)
@@ -231,7 +231,7 @@ def _prepare_hardware_target(self, case, fixture, mode):
             revision = self.device.revision
         else:
             self.device = None
-            revision = "A"
+            revision = "A0"
 
         self.target = GlasgowHardwareTarget(revision=revision,
                                             multiplexer_cls=DirectMultiplexer)

diff --git a/software/glasgow/applet/i2c_master/__init__.py b/software/glasgow/applet/i2c_master/__init__.py
@@ -242,7 +242,7 @@ class I2CMasterApplet(GlasgowApplet, name="i2c-master"):
 
     Maximum transaction length is 65535 bytes.
     """
-    required_revision = "C"
+    required_revision = "C0"
 
     __pins = ("scl", "sda")
 

diff --git a/software/glasgow/cli.py b/software/glasgow/cli.py
@@ -114,7 +114,7 @@ def add_applet_arg(parser, mode, required=False):
                 help += " (PREVIEW QUALITY APPLET)"
                 description = "    This applet is PREVIEW QUALITY and may CORRUPT DATA or " \
                               "have missing features. Use at your own risk.\n" + description
-            if applet.required_revision > "A":
+            if applet.required_revision > "A0":
                 help += " (rev{}+)".format(applet.required_revision)
                 description += "\n    This applet requires Glasgow rev{} or later." \
                                .format(applet.required_revision)
@@ -233,7 +233,7 @@ def add_toolchain_args(parser):
     add_applet_arg(g_flash_bitstream, mode="build")
 
     def revision(arg):
-        if arg in "ABC":
+        if re.match(r"^A0|B0|C0$", string):
             return arg
         else:
             raise argparse.ArgumentTypeError("{} is not a valid revision letter".format(arg))
@@ -661,7 +661,7 @@ def startTest(test):
                     return 1
 
             fx2_config = FX2Config(vendor_id=VID_QIHW, product_id=PID_GLASGOW,
-                                   device_id=1 + ord(args.rev) - ord('A'),
+                                   device_id=GlasgowConfig.encode_revision(args.rev),
                                    i2c_400khz=True)
             glasgow_config = GlasgowConfig(args.rev, args.serial)
             fx2_config.append(0x4000 - GlasgowConfig.size, glasgow_config.encode())

diff --git a/software/glasgow/device/config.py b/software/glasgow/device/config.py
@@ -1,4 +1,5 @@
 import struct
+import re
 
 
 __all__ = ["GlasgowConfig"]
@@ -29,7 +30,7 @@ class GlasgowConfig:
         Maximum allowed I/O port voltage, in millivolts.
     """
     size = 64
-    _encoding = "<1s16sI16s2H"
+    _encoding = "<B16sI16s2H"
 
     def __init__(self, revision, serial, bitstream_size=0, bitstream_id=b"\x00"*16,
                  voltage_limit=None):
@@ -39,12 +40,30 @@ def __init__(self, revision, serial, bitstream_size=0, bitstream_id=b"\x00"*16,
         self.bitstream_id   = bitstream_id
         self.voltage_limit  = [5500, 5500] if voltage_limit is None else voltage_limit
 
+    @staticmethod
+    def encode_revision(string):
+        if re.match(r"^[A-Z][0-9]$", string):
+            major, minor = string
+            return ((ord(major) - ord("A") + 1) << 4) | (ord(minor) - ord("0"))
+        else:
+            raise ValueError("invalid revision string {!r}".format(string))
+
+    @staticmethod
+    def decode_revision(value):
+        major, minor = (value & 0xF0) >> 4, value & 0x0F
+        if major == 0:
+            return chr(ord("A") + minor - 1) + "0"
+        elif minor in range(10):
+            return chr(ord("A") + major - 1) + chr(ord("0") + minor)
+        else:
+            raise ValueError("invalid revision value {:#04x}".format(value))
+
     def encode(self):
         """
         Convert configuration to a byte array that can be loaded into memory or EEPROM.
         """
         data = struct.pack(self._encoding,
-                           self.revision.encode("ascii"),
+                           self.encode_revision(self.revision),
                            self.serial.encode("ascii"),
                            self.bitstream_size,
                            self.bitstream_id,
@@ -67,7 +86,7 @@ def decode(cls, data):
         revision, serial, bitstream_size, bitstream_id, \
             voltage_limit[0], voltage_limit[1] = \
             struct.unpack_from(cls._encoding, data, 0)
-        return cls(revision.decode("ascii"),
+        return cls(self.decode_revision(revision),
                    serial.decode("ascii"),
                    bitstream_size,
                    bitstream_id,

diff --git a/software/glasgow/device/hardware.py b/software/glasgow/device/hardware.py
@@ -10,6 +10,7 @@
 
 from ..support.logging import *
 from . import GlasgowDeviceError
+from .config import GlasgowConfig
 
 
 __all__ = ["GlasgowHardwareDevice"]
@@ -92,28 +93,25 @@ def __init__(self, firmware_file=None, vendor_id=VID_QIHW, product_id=PID_GLASGO
         self._open_device(vendor_id, product_id)
 
         device_id = self.usb.getDevice().getbcdDevice()
-        self.revision = chr(ord("A") + (device_id & 0xFF) - 1)
+        self.revision = GlasgowConfig.decode_revision(device_id & 0xFF)
 
         if device_id & 0xFF00 in (0x0000, 0xA000):
             logger.debug("found rev%s device without firmware", self.revision)
 
-            if self.revision not in "ABC":
-                raise GlasgowDeviceError("unknown device revision {}".format(self.revision))
-
             if firmware_file is None:
                 raise GlasgowDeviceError("firmware is not uploaded")
-            else:
-                logger.debug("loading firmware from %s", firmware_file)
-                with open(firmware_file, "rb") as f:
-                    self._download_firmware(input_data(f, fmt="ihex"))
 
-                # let the device re-enumerate and re-acquire it
-                time.sleep(1)
-                self._open_device(VID_QIHW, PID_GLASGOW)
+            logger.debug("loading firmware from %s", firmware_file)
+            with open(firmware_file, "rb") as f:
+                self._download_firmware(input_data(f, fmt="ihex"))
+
+            # let the device re-enumerate and re-acquire it
+            time.sleep(1)
+            self._open_device(VID_QIHW, PID_GLASGOW)
 
-                # still not the right firmware?
-                if self.usb.getDevice().getbcdDevice() & 0xFF00 in (0x0000, 0xA000):
-                    raise GlasgowDeviceError("firmware upload failed")
+            # still not the right firmware?
+            if self.usb.getDevice().getbcdDevice() & 0xFF00 in (0x0000, 0xA000):
+                raise GlasgowDeviceError("firmware upload failed")
 
         # https://github.com/vpelletier/python-libusb1/issues/39
         # serial = self.usb.getDevice().getSerialNumber()

diff --git a/software/glasgow/target/hardware.py b/software/glasgow/target/hardware.py
@@ -50,11 +50,9 @@ class GlasgowHardwareTarget(Module):
     sys_clk_freq = 30e6
 
     def __init__(self, revision, multiplexer_cls=None, with_analyzer=False):
-        if revision == "A":
+        if revision in ("A0", "B0"):
             self.platform = GlasgowPlatformRevAB()
-        elif revision == "B":
-            self.platform = GlasgowPlatformRevAB()
-        elif revision == "C":
+        elif revision in ("C0",):
             self.platform = GlasgowPlatformRevC()
         else:
             raise ValueError("Unknown revision")