Fix bug in how the PG is handled for the 3.8V firefly supply (#189)

* remove OK from 3.8V since it logically means something different than intended * Update Readme since FreeRTOS is now a submodule * add a compile-time check on the VALM masks * add more comments on 4v0 and how it is handled * remove all PS_OKS_*_MASK_L6 conflicting oks[] + only last PS w/ blade_power_ok * fix PS stuck at PS6 waiting for sem in enablev38 and OFF * fix MONI2C based on PR comments * get rid of some dead code --------- Co-authored-by: Peace Kotamnives <[email protected]>
apollo-lhc · Jun 28, 2023 · bc04d3f · bc04d3f
1 parent 2a56f47
commit bc04d3f
Show file tree

Hide file tree

Showing 7 changed files with 109 additions and 128 deletions.
diff --git a/README.md b/README.md
@@ -1,4 +1,4 @@
-#cm_mcu ![CI Status](https://github.com/apollo-lhc/cm_mcu/actions/workflows/c-cpp.yml/badge.svg)
+# cm_mcu ![CI Status](https://github.com/apollo-lhc/cm_mcu/actions/workflows/c-cpp.yml/badge.svg)
 Microcontroller source code, initially targeting the [TI Tiva TM4C1290NCPDT](https://www.ti.com/product/TM4C1290NCPDT) on the Apollo command module. This is a Cortex-M4F 32 bit processor.
 
 ## Project

diff --git a/common/power_ctl.c b/common/power_ctl.c
@@ -72,7 +72,6 @@ struct gpio_pin_t oks[] = {
 // REV 2
 //
 // ------------------------------------------
-// add here
 // if you update this you need to update N_PS_ENABLES
 static const struct gpio_pin_t enables[] = {
     {  EN_F1_INT, "EN_F1_INT", 1},
@@ -87,7 +86,11 @@ static const struct gpio_pin_t enables[] = {
     {  EN_F2_AVTT, "EN_F2_AVTT", 5},
 };
 
-//if you update this you need to update N_PS_OKS too
+// if you update this you need to update N_PS_OKS too
+// note we do _not_ include the PG for the 4V0 supply, though it exists.
+// this is because the supply is turned on automatically at L2
+// but only enabled for the fireflies at L6. At L6 we don't actually 
+// turn on the supplies, but instead enable them for the fireflies.
 const
 struct gpio_pin_t oks[N_PS_OKS] = {
     { PG_F1_INT_A, "PG_F1_INT_A", 1},
@@ -102,7 +105,7 @@ struct gpio_pin_t oks[N_PS_OKS] = {
     { PG_F2_AVCC, "PG_F2_AVCC", 4},
     { PG_F1_AVTT,  "PG_F1_AVTT", 5},
     { PG_F2_AVTT, "PG_F2_AVTT", 5},
-    { PG_4V0, "PG_4V0", 6},  // enable_3v8(true/false) won't change PG_4V0. Only within 10s after 4.0V off, PG_4V0 can be 0x0.
+    //{ PG_4V0, "PG_4V0", 6},  // enable_3v8(true/false) won't change PG_4V0. Only within 10s after 4.0V off, PG_4V0 can be 0x0.
 };
 
 #else
@@ -156,7 +159,7 @@ bool disable_ps(void)
       bool all_ready = true;
       for (int o = 0; o < N_PS_OKS; ++o) {
         if (oks[o].priority >= prio) {
-          int8_t val = read_gpio_pin(oks[o].pin_number);
+          uint8_t val = read_gpio_pin(oks[o].pin_number);
           if (val == 1) { // all supplies are supposed to be off now
             all_ready = false;
             states[o] = PWR_UNKNOWN;

diff --git a/common/power_ctl.h b/common/power_ctl.h
@@ -67,12 +67,10 @@ void setPSStatus(int i, enum ps_state theState);
 #define PS_OKS_F1_MASK_L2 0x0030U // these two pins are common to VU and KU
 #define PS_OKS_F1_MASK_L4 0x0300U
 #define PS_OKS_F1_MASK_L5 0x0C00U
-#define PS_OKS_F1_MASK_L6 0x0000U // no 4v0 pin in REV1
 #define PS_OKS_F2_MASK_L1 0x000CU
 #define PS_OKS_F2_MASK_L2 PS_OKS_F1_MASK_L2
 #define PS_OKS_F2_MASK_L4 0x00C0U
 #define PS_OKS_F2_MASK_L5 0x3000U
-#define PS_OKS_F2_MASK_L6 0x0000U // no 4v0 pin in REV1
 
 #elif defined(REV2) // Rev 2
 // -----------------------------------------------------
@@ -83,11 +81,11 @@ void setPSStatus(int i, enum ps_state theState);
 // Number of enable and power good/OK pins
 
 #define N_PS_ENABLES      10
-#define N_PS_OKS          13
+#define N_PS_OKS          12
 #define PS_OKS_MASK       ((1U << N_PS_OKS) - 1)
 #define PS_OKS_F1_MASK    0x543U
 #define PS_OKS_F2_MASK    0xA8CU
-#define PS_OKS_GEN_MASK   0x1030U // includes 4v0 pin
+#define PS_OKS_GEN_MASK   0x030U
 #define PS_ENS_MASK       ((1U << N_PS_ENABLES) - 1)
 #define PS_ENS_GEN_MASK   0x00CU
 #define PS_ENS_F1_MASK    0x151U
@@ -101,17 +99,11 @@ void setPSStatus(int i, enum ps_state theState);
 #define PS_OKS_F1_MASK_L3 0x040U
 #define PS_OKS_F1_MASK_L4 0x100U
 #define PS_OKS_F1_MASK_L5 0x400U
-#define PS_OKS_F1_MASK_L6 0x1000U // this one pin is common to F1 and F2
 #define PS_OKS_F2_MASK_L1 0x00CU
 #define PS_OKS_F2_MASK_L2 PS_OKS_F1_MASK_L2
 #define PS_OKS_F2_MASK_L3 0x080U
 #define PS_OKS_F2_MASK_L4 0x200U
 #define PS_OKS_F2_MASK_L5 0x800U
-#define PS_OKS_F2_MASK_L6 PS_OKS_F1_MASK_L6
-
-//#error "Missing Rev 2 PS masks"
-#else
-#error "Must define either Rev1 or Rev2"
 #endif // REV 2
 
 bool turn_on_ps(uint16_t);

diff --git a/projects/cm_mcu/AlarmUtilities.c b/projects/cm_mcu/AlarmUtilities.c
@@ -4,6 +4,8 @@
 
 #include "common/log.h"
 #include "common/pinsel.h"
+#include <assert.h>
+#include <math.h>
 
 ///////////////////////////////////////////////////////////
 //
@@ -172,90 +174,85 @@ uint32_t getVoltAlarmStatus(void)
 }
 // check the current voltage status.
 // returns +1 for warning, +2 or higher for error
+// these flags represent positions into thestruct ADC_Info_t ADCs[] array in
+// the ADCMonitorTask.c
+#ifdef REV1
+#define VALM_BASE_MASK    0x00025U // management powers, e.g. 12V and M3V3
+#define VALM_GEN_MASK     0x0001AU // common powers
+#define VALM_F1_MASK      0xCCC80U // F1-specific
+#define VALM_F2_MASK      0x33340U // F2-specific
+#define VALM_ALL_MASK     (VALM_BASE_MASK | VALM_GEN_MASK | VALM_F1_MASK | VALM_F2_MASK)
+#define VALM_HIGHEST_V_CH 19 // highest channel that contains a voltage, 0 based counting
+#elif REV2
+#define VALM_BASE_MASK    0x003U  // management powers, e.g. 12V and M3V3
+#define VALM_GEN_MASK     0x001CU // common powers
+#define VALM_F1_MASK      0x01E0U // F1-specific
+#define VALM_F2_MASK      0x1E00U // F2-specific
+#define VALM_ALL_MASK     (VALM_BASE_MASK | VALM_GEN_MASK | VALM_F1_MASK | VALM_F2_MASK)
+#define VALM_HIGHEST_V_CH 12 // highest channel that contains a voltage, 0 based counting
+#endif                       // REV2
 int VoltStatus(void)
 {
+
+  // compile-time sanity check on the flags being unique.
+  // I need the +1 in the 1<xx since the highest channel is 0-based counting.
+  static_assert((VALM_BASE_MASK ^ VALM_GEN_MASK ^ VALM_F1_MASK ^ VALM_F2_MASK) == ((1 << (VALM_HIGHEST_V_CH + 1)) - 1), "VALM masks not unique");
+
   bool f1_enable = isFPGAF1_PRESENT();
   bool f2_enable = isFPGAF2_PRESENT();
 
-#ifndef REV2                       // REV1
-  uint8_t GEN_VOLTAGE_MASK = 0x3f; // 0b111111 by default
-#else                              // REV2
-  uint8_t GEN_VOLTAGE_MASK = 0x1f; // 0b11111 by default
-#endif                             // REV 2
-
   int retval = 0;
   status_V = 0x0U;
 
-  int n_fpga_half_ch = (ADC_INFO_FPGA_VCC_FIN_CH - ADC_INFO_FPGA_VCC_INIT_CH + 1) / 2;
-  int ADC_INFO_FPGA2_VCC_INIT_CH = n_fpga_half_ch + ADC_INFO_FPGA_VCC_INIT_CH;
+  // change what we do, if power is on or not.
+  enum power_system_state currPsState = getPowerControlState();
 
-  const int adc_vcc_int_ch[3] = {ADC_INFO_GEN_VCC_INIT_CH, ADC_INFO_FPGA_VCC_INIT_CH, ADC_INFO_FPGA2_VCC_INIT_CH};
-  const int adc_vcc_fin_ch[3] = {ADC_INFO_GEN_VCC_FIN_CH, ADC_INFO_FPGA2_VCC_INIT_CH - 1, ADC_INFO_FPGA_VCC_FIN_CH};
-  uint8_t dev_bitmask[3] = {0, 0, 0};
-  uint8_t is_dev_alarm_volt[3] = {0, 0, 0};
-
-  // microcontroller and general power
-
-  if (getPowerControlState() != POWER_ON) {
-#ifndef REV2                // REV1
-    GEN_VOLTAGE_MASK = 0x5; // 0b000101 only allows other powers off except M3V3 and 12V
-#else                       // REV2
-    GEN_VOLTAGE_MASK = 0x3;        // 0b00011 only allows other powers off except M3V3 and 12V
-#endif                      // REV 2
+  if (!((currPsState == POWER_ON) || (currPsState == POWER_OFF))) { // in flux. Skip.
+    return 0;
   }
 
-  for (int n = 0; n < 3; ++n) {
-    for (int ch = adc_vcc_int_ch[n]; ch < adc_vcc_fin_ch[n] + 1; ++ch) {
-
-      if (n != 0) {
-        if ((!f1_enable) || (!f2_enable && ch > (ADC_INFO_FPGA2_VCC_INIT_CH - 1))) // check if fpga1/2 is on the board. currently fpga1 takes the first half of adc outputs in this indexing
-          break;
-      }
-      float threshold = getAlarmVoltageThres();
-      float target_value = getADCtargetValue(ch);
-
-      if (getADCvalue(ch) < 0.7f * target_value) // wait for delay from ADC outputs and actual reading
-        vTaskDelay(pdMS_TO_TICKS(500));          // delay 1000 ms
+  // set up mask for which channels to worry about
+  uint32_t ch_mask = VALM_BASE_MASK; // always true
+  if (currPsState == POWER_ON) {
+    ch_mask |= VALM_GEN_MASK; // common power
+    if (f1_enable) {
+      ch_mask |= VALM_F1_MASK;
+    }
+    if (f2_enable) {
+      ch_mask |= VALM_F2_MASK;
+    }
+  }
+  // Loop over ADC values.
+  const float threshold = getAlarmVoltageThres();
+  uint32_t ch_alm_mask = 0x0U;
+  for (int i = 0; i < VALM_HIGHEST_V_CH; ++i) {
+    // check if the current channel contains a voltage measurement we care about
+    if (!(ch_mask & (0x1U << i))) {
+      continue; // if not, continue to then ext loop iteration
+    }
+    float target_value = getADCtargetValue(i);
+    float now_value = getADCvalue(i);
+    float excess = (now_value - target_value) / target_value;
 
-      float now_value = getADCvalue(ch);
-      float excess = (now_value - target_value) / target_value;
+    if (ABS(excess) > threshold) {
+      ch_alm_mask |= (0x1U << i); // mark bit for failing supply
       int tens, frac;
       float_to_ints(excess * 100, &tens, &frac);
-      if (excess > 0.0f) {
-        is_dev_alarm_volt[n] = 1;
-        excess_volt = excess * 100;
-        excess_volt_which_ch = ch;
-      }
-
-      if ((excess > threshold && excess > 0.0f) || (excess * -1.0f > threshold && excess < 0.0f)) { // if this ADC voltage is greater/lower than a target value by getAlarmVoltageThres()*100%
-        dev_bitmask[n] += (1 << (ch - adc_vcc_int_ch[n]));                                          // first to last bit corresponds to status of low to high ADC voltage channel
-        is_dev_alarm_volt[n] = 2;
-        log_debug(LOG_ALM, "alarm volt at ADC ch : %02d now %02d.%02d %% off target\r\n", ch, tens, frac); // over voltage among one of power supplies by +/- getAlarmVoltageThres()*100% of its threshold
-      }
+      log_debug(LOG_ALM, "VoltAlm: %s: %02d.%02d %% off target\r\n", getADCname(i), tens, frac);
     }
-
-    if (n == 0)
-      currentVoltStatus[GEN] = dev_bitmask[n] & GEN_VOLTAGE_MASK; // applies a mask with power-off exceptions
-    else if (n == 1)
-      currentVoltStatus[FPGA1] = dev_bitmask[n];
-    else
-      currentVoltStatus[FPGA2] = dev_bitmask[n];
   }
-
-  if (is_dev_alarm_volt[0] > 0 || is_dev_alarm_volt[1] > 0 || is_dev_alarm_volt[2] > 0) {
-    retval++;
-    if (is_dev_alarm_volt[0] == 2) {
-      status_V |= ALM_STAT_GEN_OVERVOLT;
-      ++retval;
-    }
-    else if (is_dev_alarm_volt[1] == 2) {
-      status_V |= ALM_STAT_FPGA1_OVERVOLT;
-      ++retval;
-    }
-    else if (is_dev_alarm_volt[2] == 2) {
-      status_V |= ALM_STAT_FPGA2_OVERVOLT;
-      ++retval;
-    }
+  status_V = 0x0U;
+  if (ch_alm_mask & (VALM_BASE_MASK | VALM_GEN_MASK)) {
+    status_V |= ALM_STAT_GEN_OVERVOLT;
+    ++retval;
+  }
+  if (ch_alm_mask & VALM_F1_MASK) {
+    status_V |= ALM_STAT_FPGA1_OVERVOLT;
+    ++retval;
+  }
+  if (ch_alm_mask & VALM_F2_MASK) {
+    status_V |= ALM_STAT_FPGA2_OVERVOLT;
+    ++retval;
   }
 
   return retval;

diff --git a/projects/cm_mcu/MonitorI2CTask.c b/projects/cm_mcu/MonitorI2CTask.c
@@ -83,23 +83,43 @@ void MonitorI2CTask(void *parameters)
 
   bool good = false;
   for (;;) {
+
     log_debug(LOG_MONI2C, "%s: grab semaphore\r\n", args->name);
 
     // grab the semaphore to ensure unique access to I2C controller
-    if (acquireI2CSemaphore(args->xSem) == pdFAIL) {
-      log_warn(LOG_SERVICE, "%s could not get semaphore in time; continue\r\n", args->name);
-      continue;
+    if (args->xSem != NULL) {
+      if (acquireI2CSemaphore(args->xSem) == pdFAIL) {
+        log_warn(LOG_SERVICE, "%s could not get semaphore in time; delay & continue\r\n", args->name);
+        vTaskDelayUntil(&(args->updateTick), pdMS_TO_TICKS(10)); // wait
+        continue;
+      }
     }
 
     // -------------------------------
     // loop over devices in the device-type instance
     // -------------------------------
     for (int ps = 0; ps < args->n_devices; ++ps) {
-      log_debug(LOG_MONI2C, "%s: device %d\r\n", args->name, ps);
+      log_debug(LOG_MONI2C, "%s: device %d powercheck\r\n", args->name, ps);
 
-      if (ps == args->n_devices - 1 && getPowerControlState() != POWER_ON) { // avoid continues to infinite loops due to multi-threading when pwr is not on
+      if (getPowerControlState() != POWER_ON) {
+        if (good) {
+          log_info(LOG_MONI2C, "%s: PWR off. Disabling I2C monitoring.\r\n", args->name);
+          good = false;
+          task_watchdog_unregister_task(kWatchdogTaskID_MonitorI2CTask);
+        }
+        if (xSemaphoreGetMutexHolder(args->xSem) == xTaskGetCurrentTaskHandle()) {
+          xSemaphoreGive(args->xSem);
+        }
         break;
       }
+      else if (getPowerControlState() == POWER_ON) { // power is on, and ...
+        if (!good) {                                 // ... was not good, but is now good
+          task_watchdog_register_task(kWatchdogTaskID_MonitorI2CTask);
+          log_info(LOG_MONI2C, "%s: PWR on. (Re)starting I2C monitoring.\r\n", args->name);
+          good = true;
+        }
+      }
+
       if (!IsCLK) {                           // Fireflies need to be checked if the links are connected or not
         if (args->i2c_dev == I2C_DEVICE_F1) { // FPGA #1
 #ifdef REV1
@@ -125,31 +145,6 @@ void MonitorI2CTask(void *parameters)
 #endif
         }
       }
-      log_debug(LOG_MONI2C, "%s: powercheck\r\n", args->name);
-
-      if (getPowerControlState() != POWER_ON) {
-        if (good) {
-          log_info(LOG_MONI2C, "%s: PWR off. Disabling I2C monitoring.\r\n", args->name);
-          good = false;
-          task_watchdog_unregister_task(kWatchdogTaskID_MonitorI2CTask);
-        }
-        vTaskDelay(pdMS_TO_TICKS(500));
-        continue;
-      }
-      else if (getPowerControlState() == POWER_ON) { // power is on, and ...
-        if (!good) {                                 // ... was not good, but is now good
-          task_watchdog_register_task(kWatchdogTaskID_MonitorI2CTask);
-          log_info(LOG_MONI2C, "%s: PWR on. (Re)starting I2C monitoring.\r\n", args->name);
-          good = true;
-        }
-      }
-      // if the power state is unknown, don't do anything
-      else {
-        log_info(LOG_MONI2C, "%s: power state %d unknown\r\n", args->name,
-                 getPowerControlState());
-        vTaskDelay(10);
-        continue;
-      }
 
       if (!IsCLK) {
         // mux setting

diff --git a/projects/cm_mcu/PowerSupplyTask.c b/projects/cm_mcu/PowerSupplyTask.c
@@ -60,10 +60,10 @@ static uint16_t getPSFailMask(void)
   static uint32_t ps_ignore_mask;
   if (!configured) {
     ps_ignore_mask = read_eeprom_single(EEPROM_ID_PS_IGNORE_MASK);
-    if (ps_ignore_mask & ~(PS_OKS_F1_MASK_L4 | PS_OKS_F1_MASK_L5 | PS_OKS_F1_MASK_L6 | PS_OKS_F2_MASK_L4 | PS_OKS_F2_MASK_L5 | PS_OKS_F2_MASK_L6)) {
+    if (ps_ignore_mask & ~(PS_OKS_F1_MASK_L4 | PS_OKS_F1_MASK_L5 | PS_OKS_F2_MASK_L4 | PS_OKS_F2_MASK_L5)) {
       log_warn(LOG_PWRCTL, "Warning: mask 0x%x included masks at below L4; ignoring\r\n", ps_ignore_mask);
       // mask out supplies at startup L1, L2 or L3. We do not allow those to fail.
-      ps_ignore_mask &= (PS_OKS_F1_MASK_L4 | PS_OKS_F1_MASK_L5 | PS_OKS_F1_MASK_L6 | PS_OKS_F2_MASK_L4 | PS_OKS_F2_MASK_L5 | PS_OKS_F2_MASK_L6);
+      ps_ignore_mask &= (PS_OKS_F1_MASK_L4 | PS_OKS_F1_MASK_L5 | PS_OKS_F2_MASK_L4 | PS_OKS_F2_MASK_L5);
     }
     configured = true;
   }
@@ -143,7 +143,7 @@ void PowerSupplyTask(void *parameters)
     supply_ok_mask_L4 = supply_ok_mask_L2 | PS_OKS_F1_MASK_L4;
     supply_ok_mask_L5 = supply_ok_mask_L4 | PS_OKS_F1_MASK_L5;
 #ifdef REV2
-    supply_ok_mask_L6 = supply_ok_mask_L5 | PS_OKS_F1_MASK_L6;
+    supply_ok_mask_L6 = supply_ok_mask_L5;
 #endif // REV2
   }
   if (f2_enable) {
@@ -153,7 +153,7 @@ void PowerSupplyTask(void *parameters)
     supply_ok_mask_L4 |= supply_ok_mask_L2 | PS_OKS_F2_MASK_L4;
     supply_ok_mask_L5 |= supply_ok_mask_L4 | PS_OKS_F2_MASK_L5;
 #ifdef REV2
-    supply_ok_mask_L6 |= supply_ok_mask_L5 | PS_OKS_F2_MASK_L6;
+    supply_ok_mask_L6 |= supply_ok_mask_L5;
 #endif // REV2
   }
   // exceptions are stored in the internal EEPROM -- the IGNORE mask.
@@ -391,7 +391,6 @@ void PowerSupplyTask(void *parameters)
           nextState = POWER_FAILURE;
         }
         else {
-          blade_power_ok(true);
           nextState = POWER_L6ON;
         }