firmware: handle voltage alerts on revC2

Implements a workaround for an i2c address clash of the SMBus Alert Request Address (required to clear the ~ALERT line) and the DAC A.
GlasgowEmbedded · Dec 5, 2020 · e472c43 · e472c43
1 parent 949b003
commit e472c43
Show file tree

Hide file tree

Showing 5 changed files with 738 additions and 509 deletions.
diff --git a/firmware/adc_ina233.c b/firmware/adc_ina233.c
@@ -5,14 +5,16 @@
 enum {
   // ADC registers
   INA233_REG_CLEAR_FAULTS        = 0x03,
+  INA233_REG_RESTORE_DEFAULT_ALL = 0x12,
   INA233_REG_VIN_OV_WARN_LIMIT   = 0x57,
   INA233_REG_VIN_UV_WARN_LIMIT   = 0x58,
   INA233_REG_STATUS_MFR_SPECIFIC = 0x80,
   INA233_REG_READ_VIN            = 0x88,
   INA233_REG_READ_IIN            = 0x89,
   INA233_REG_MFR_ALERT_MASK      = 0xD2,
   INA233_REG_MFR_CALIBRATION     = 0xD4,
-  // MFR_ALERT_MASK bits
+  INA233_REG_MFR_DEVICE_CONFIG   = 0xD5,
+  // MFR_ALERT_MASK and STATUS_MFR_SPECIFIC bits
   INA233_BIT_IN_UV_WARNING       = 1<<0,
   INA233_BIT_IN_OV_WARNING       = 1<<1,
   INA233_BIT_IN_OC_WARNING       = 1<<2,
@@ -21,30 +23,49 @@ enum {
   INA233_BIT_POR_EVENT           = 1<<5,
   INA233_BIT_ADC_OVERFLOW        = 1<<6,
   INA233_BIT_CONV_READY          = 1<<7,
-
 };
 
 struct buffer_desc {
   uint8_t selector;
+  __xdata uint8_t* status_cache_ptr;
   uint8_t address;
 };
 
+static __xdata uint8_t ina233_status_cache[2];
+
 static const struct buffer_desc buffers[] = {
-  { IO_BUF_A, I2C_ADDR_IOA_ADC_INA233 },
-  { IO_BUF_B, I2C_ADDR_IOB_ADC_INA233 },
+  { IO_BUF_A, &ina233_status_cache[0], I2C_ADDR_IOA_ADC_INA233 },
+  { IO_BUF_B, &ina233_status_cache[1], I2C_ADDR_IOB_ADC_INA233 },
   { 0, 0 }
 };
 
+static bool iobuf_reset_ina233(uint8_t i2c_addr) {
+  __pdata uint8_t regval;
+
+  // bring the INA233 to a known state, even if there was no reset (e.g. firmware reload)
+  // just send register/command code, no data, to execute the RESTORE_DEFAULT_ALL command
+  if(!i2c_reg8_write(i2c_addr, INA233_REG_RESTORE_DEFAULT_ALL, &regval, 0))
+    return false;
+
+  // mask all events from triggering an alert (=would switch off our port power)
+  // they will be unmasked selectively when the alerts are configured
+  regval = 0xFF;
+  if(!i2c_reg8_write(i2c_addr, INA233_REG_MFR_ALERT_MASK, &regval, 1))
+    return false;
+
+  // TODO: write cal register to allow current measurement
+
+  return true;
+}
+
 bool iobuf_init_adc_ina233() {
   __code const struct buffer_desc *buffer;
   for(buffer = buffers; buffer->selector; buffer++) {
-    // mask all events from triggering an alert (=would switch off our port power)
-    // they will be unmasked selectively when configured
-      __pdata uint8_t regval = 0xFF;
-    if(!i2c_reg8_write(buffer->address, INA233_REG_MFR_ALERT_MASK, &regval, 1))
+    // clear cache
+    *(buffer->status_cache_ptr) = 0;
+
+    if (!iobuf_reset_ina233(buffer->address))
       return false;
-
-    // TODO: write cal register
   }
 
   return true;
@@ -59,6 +80,13 @@ static uint16_t code_bytes_to_millivolts_ina233(__pdata const uint8_t *code_byte
   return millivolts;
 }
 
+static void millivolts_to_code_bytes_ina233(uint16_t millivolts, __pdata uint8_t *code_bytes) {
+  // See explanation above.
+  uint32_t code_word = (millivolts * 4) / 5;
+  code_bytes[0] = code_word & 0xff;
+  code_bytes[1] = code_word >> 8;
+}
+
 bool iobuf_measure_voltage_ina233(uint8_t selector, __xdata uint16_t *millivolts) {
   __code const struct buffer_desc *buffer;
   for(buffer = buffers; buffer->selector; buffer++) {
@@ -75,6 +103,57 @@ bool iobuf_measure_voltage_ina233(uint8_t selector, __xdata uint16_t *millivolts
   return false;
 }
 
+bool iobuf_set_alert_ina233(uint8_t mask,
+                     __xdata const uint16_t *low_millivolts,
+                     __xdata const uint16_t *high_millivolts) {
+  __code const struct buffer_desc *buffer;
+  __pdata uint8_t low_code_bytes[2] = { 0x00, 0x00 };
+  __pdata uint8_t high_code_bytes[2] = { 0xf8, 0x7f };
+  __pdata uint8_t mask_reg = 0xFF;
+
+  // TODO: we probably want to allow more than MAX_VOLTAGE for the INA233
+
+  if(*low_millivolts > MAX_VOLTAGE || *high_millivolts > MAX_VOLTAGE)
+    return false;
+
+  if(*low_millivolts != 0) {
+    // Alert enabled, unmask the alert
+    millivolts_to_code_bytes_ina233(*low_millivolts, low_code_bytes);
+    mask_reg &= ~(INA233_BIT_IN_UV_WARNING);
+  }
+
+  if(*high_millivolts != MAX_VOLTAGE) {
+    // Alert enabled, unmask the alert
+    millivolts_to_code_bytes_ina233(*high_millivolts, high_code_bytes);
+    mask_reg &= ~(INA233_BIT_IN_OV_WARNING);
+  }
+
+  for(buffer = buffers; buffer->selector; buffer++) {
+    if(mask & buffer->selector) {
+
+      if(!i2c_reg8_write(buffer->address, INA233_REG_VIN_UV_WARN_LIMIT, low_code_bytes, 2))
+        return false;
+
+      if(!i2c_reg8_write(buffer->address, INA233_REG_VIN_OV_WARN_LIMIT, high_code_bytes, 2))
+        return false;
+
+      if(!i2c_reg8_write(buffer->address, INA233_REG_MFR_ALERT_MASK, &mask_reg, 1))
+        return false;
+
+      // a CLEAR_FAULTS seems to be necessary after changing the alert mask.
+      // Experimentation shows that the alert mask is only evaluated when a fault occurs
+      // When a currently masked fault occured, a later change in the alert mask does not 
+      // cause the fault to trigger ~ALERT. A change in the limit vaules also doesn't cause
+      // a fault to be reevaluated.
+      if(!i2c_reg8_write(buffer->address, INA233_REG_CLEAR_FAULTS, &mask_reg, 0))
+        return false;      
+    }
+  }
+
+  return true;
+}
+
+
 bool iobuf_get_alert_ina233(uint8_t selector,
                      __xdata uint16_t *low_millivolts,
                      __xdata uint16_t *high_millivolts) {
@@ -84,33 +163,104 @@ bool iobuf_get_alert_ina233(uint8_t selector,
       __pdata uint8_t code_bytes[2];
       __pdata uint8_t mask_reg;
 
-      // check which kind of alerts are activated
-      if(!i2c_reg8_read(buffer->address, INA233_REG_MFR_ALERT_MASK, &mask_reg, 1))
+      if(!i2c_reg8_read(buffer->address, INA233_REG_VIN_UV_WARN_LIMIT, code_bytes, 2))
         return false;
-
-      if (mask_reg & INA233_BIT_IN_UV_WARNING) {
-        // undervoltage inactive
+
+      if (code_bytes[0] == 0 && code_bytes[1] == 0)
         *low_millivolts = 0;
-      } else {
-        if(!i2c_reg8_read(buffer->address, INA233_REG_VIN_UV_WARN_LIMIT, code_bytes, 2))
-            return false;
+      else
         *low_millivolts = code_bytes_to_millivolts_ina233(code_bytes);
-      }
 
-      if (mask_reg & INA233_BIT_IN_OV_WARNING) {
-        // overvoltage inactive
-        // TODO: MAX_VOLTAGE is a bad choice for the INA233, as it can measure up to 40V
-        // this suggests a small API change
+      if(!i2c_reg8_read(buffer->address, INA233_REG_VIN_OV_WARN_LIMIT, code_bytes, 2))
+        return false;
+
+      if (code_bytes[0] == 0xf8 && code_bytes[1] == 0x7f) 
         *high_millivolts = MAX_VOLTAGE;
-      } else {
-        if(!i2c_reg8_read(buffer->address, INA233_REG_VIN_OV_WARN_LIMIT, code_bytes, 2))
-            return false;
+      else
         *high_millivolts = code_bytes_to_millivolts_ina233(code_bytes);
-      }
 
       return true;
     }
   }
 
   return false;
 }
+
+// this just polls the INA233 for alerts and updates the status cache
+// it does not clear the ~ALERT line
+bool iobuf_poll_alert_ina233(__xdata uint8_t *mask) {
+  __code const struct buffer_desc *buffer;
+  for(*mask = 0, buffer = buffers; buffer->selector; buffer++) {
+    __pdata uint8_t status_byte;
+    if(!i2c_reg8_read(buffer->address, INA233_REG_STATUS_MFR_SPECIFIC, &status_byte, 1))
+      return false;
+
+    // just check the actual limit alert bits, ignoring the others
+    if(status_byte & 
+        (INA233_BIT_IN_UV_WARNING | INA233_BIT_IN_OV_WARNING | INA233_BIT_IN_OC_WARNING | INA233_BIT_IN_OP_WARNING))
+    {
+      // we got some kind of limit alert, return the port in the bitmask
+      *mask |= buffer->selector;
+
+      // store the full status byte in the status cache
+      *(buffer->status_cache_ptr) = status_byte;
+    }
+  }
+
+  return true;
+}
+
+void iobuf_read_alert_cache_ina233(__xdata uint8_t *mask, bool clear) {
+  __code const struct buffer_desc *buffer;
+  for(*mask = 0, buffer = buffers; buffer->selector; buffer++) {
+    uint8_t status_byte = *(buffer->status_cache_ptr);
+
+    // just check the actual limit alert bits, ignoring the others
+    if(status_byte & 
+        (INA233_BIT_IN_UV_WARNING | INA233_BIT_IN_OV_WARNING | INA233_BIT_IN_OC_WARNING | INA233_BIT_IN_OP_WARNING))
+    {
+      // we got some kind of limit alert, return the port in the bitmask
+      *mask |= buffer->selector;
+
+      if (clear)
+        *(buffer->status_cache_ptr) = 0;
+    }
+  }
+}
+
+
+bool iobuf_clear_alert_ina233(uint8_t mask) {
+  __code const struct buffer_desc *buffer;
+  __pdata uint8_t low_code_bytes[2];
+  __pdata uint8_t high_code_bytes[2];
+  for(buffer = buffers; buffer->selector; buffer++) {
+    if (mask & buffer->selector) {
+      // The INA233 seems to expect that you clear the ~ALERT line by reading the
+      // SMBus Alert Response Address (ARA) at 0001100.
+      // Unfortunately this clashes with the address of DAC A on revC2
+      // Experimentation showed only RESTORE_DEFAULT_ALL (aka software reset)
+      // as alternative way to clear ~ALERT. Especially CLEAR_FAULTS does not
+      // affect the ~ALERT line, despite the datasheet claiming otherwise
+
+      // So first read out the currently set limit values, reset, and write them back
+      if(!i2c_reg8_read(buffer->address, INA233_REG_VIN_UV_WARN_LIMIT, low_code_bytes, 2))
+        return false;
+
+      if(!i2c_reg8_read(buffer->address, INA233_REG_VIN_OV_WARN_LIMIT, high_code_bytes, 2))
+        return false;
+
+      if(!iobuf_reset_ina233(buffer->address))
+        return false;
+
+      // we masked all alerts after the reset, so the alert will not trigger again instantly
+
+      if(!i2c_reg8_write(buffer->address, INA233_REG_VIN_UV_WARN_LIMIT, low_code_bytes, 2))
+        return false;
+
+      if(!i2c_reg8_write(buffer->address, INA233_REG_VIN_OV_WARN_LIMIT, high_code_bytes, 2))
+        return false;
+    }
+  }
+
+  return true;
+}
diff --git a/firmware/glasgow.h b/firmware/glasgow.h
@@ -119,9 +119,15 @@ bool iobuf_poll_alert_adc081c(__xdata uint8_t *mask, bool clear);
 // ADC API (TI INA233)
 bool iobuf_init_adc_ina233();
 bool iobuf_measure_voltage_ina233(uint8_t selector, __xdata uint16_t *millivolts);
+bool iobuf_set_alert_ina233(uint8_t mask,
+                     __xdata const uint16_t *low_millivolts,
+                     __xdata const uint16_t *high_millivolts);
 bool iobuf_get_alert_ina233(uint8_t selector,
                      __xdata uint16_t *low_millivolts,
                      __xdata uint16_t *high_millivolts);
+bool iobuf_poll_alert_ina233(__xdata uint8_t *mask);
+bool iobuf_clear_alert_ina233(uint8_t mask);
+void iobuf_read_alert_cache_ina233(__xdata uint8_t *mask, bool clear);
 
 // Pull API
 bool iobuf_set_pull(uint8_t selector, uint8_t enable, uint8_t level);
@@ -133,6 +139,8 @@ void fifo_configure(bool two_ep);
 void fifo_reset(bool two_ep, uint8_t interfaces);
 
 // Util functions
+bool i2c_direct_read(uint8_t addr, __pdata uint8_t *value, uint8_t length);
+
 bool i2c_reg8_read(uint8_t addr, uint8_t reg,
                          __pdata uint8_t *value, uint8_t length);
 bool i2c_reg8_write(uint8_t addr, uint8_t reg,

diff --git a/firmware/main.c b/firmware/main.c
@@ -648,8 +648,7 @@ void handle_pending_usb_setup() {
       while(EP0CS & _BUSY);
 
       if(glasgow_config.revision == GLASGOW_REV_C2)
-        // TODO
-        result = true;
+        result = iobuf_set_alert_ina233(arg_mask, (__xdata uint16_t *)EP0BUF, (__xdata uint16_t *)EP0BUF + 1);
       else
         result = iobuf_set_alert_adc081c(arg_mask, (__xdata uint16_t *)EP0BUF, (__xdata uint16_t *)EP0BUF + 1);
 
@@ -666,13 +665,22 @@ void handle_pending_usb_setup() {
      req->bRequest == USB_REQ_POLL_ALERT &&
      req->wLength == 1) {
     pending_setup = false;
+    bool result = true;
 
     while(EP0CS & _BUSY);
-    iobuf_poll_alert_adc081c(EP0BUF, /*clear=*/true);
-    SETUP_EP0_BUF(1);
-
-    reset_status_bit(ST_ALERT);
-
+
+    if(glasgow_config.revision == GLASGOW_REV_C2)
+      iobuf_read_alert_cache_ina233(EP0BUF, /*clear=*/true);
+    else
+      result = iobuf_poll_alert_adc081c(EP0BUF, /*clear=*/true);
+
+    if(!result) {
+      STALL_EP0();
+    } else {
+      SETUP_EP0_BUF(1);
+      reset_status_bit(ST_ALERT);
+    }
+
     return;
   }
 
@@ -805,11 +813,24 @@ void handle_pending_alert() {
   pending_alert = false;
 
   latch_status_bit(ST_ALERT);
-  iobuf_poll_alert_adc081c(&mask, /*clear=*/false);
-  iobuf_set_voltage(mask, &millivolts);
+
+  if(glasgow_config.revision == GLASGOW_REV_C2) {
+    iobuf_poll_alert_ina233(&mask);
+  } else {
+    iobuf_poll_alert_adc081c(&mask, /*clear=*/false);
+  }
 
   // TODO: handle i2c comms errors of above calls
+
+  iobuf_set_voltage(mask, &millivolts);
 
+  if(glasgow_config.revision == GLASGOW_REV_C2) {
+    // only clear the ~ALERT line after the port vio has been disabled
+    // this prevents re-enabling the port voltage for a short time
+    // since on revC2 ~ALERT already disables the respective Vreg on a hw level
+    iobuf_clear_alert_ina233(mask);
+  }
+
   // the ADC that pulled the ~ALERT line should have released it by now
   // so we can re-enable the interrupt to catch the next alert
   EX0 = true;

diff --git a/firmware/util.c b/firmware/util.c
@@ -2,6 +2,18 @@
 #include <fx2i2c.h>
 #include "glasgow.h"
 
+bool i2c_direct_read(uint8_t addr, __pdata uint8_t *value, uint8_t length) {
+  if(!i2c_start((addr<<1)|1))
+    goto fail;
+  if(!i2c_read(value, length))
+    goto fail;
+  return true;
+
+fail:
+  i2c_stop();
+  return false;
+}
+
 bool i2c_reg8_read(uint8_t addr, uint8_t reg,
                          __pdata uint8_t *value, uint8_t length) {
   if(!i2c_start(addr<<1))
@@ -25,8 +37,9 @@ bool i2c_reg8_write(uint8_t addr, uint8_t reg,
     goto fail;
   if(!i2c_write(&reg, 1))
     goto fail;
-  if(!i2c_write(value, length))
-    goto fail;
+  if (length)
+    if(!i2c_write(value, length))
+      goto fail;
   if(!i2c_stop())
     return false;
   return true;