Merge pull request 'add SW IF for manipulating poll threshold' (#550) from ptme_poll_threshold_register into develop

Reviewed-on: #550

CHANGELOG.md

@@ -16,6 +16,15 @@ will consitute of a breaking change warranting a new major release:
 
 # [unreleased]
 
+## Fixed
+
+- PTME was not reset after configuration changes.
+
+## Changed
+
+- Poll threshold configuration of the PTME IP core is now configurable via a parameter command
+  and is set to 0b010 (4 polls) instead of 0b001 (1 poll) per default.
+
 # [v1.42.0] 2023-04-01
 
 - eive-tmtc: v2.20.1

linux/ipcore/AxiPtmeConfig.cpp

@@ -1,5 +1,7 @@
 #include "AxiPtmeConfig.h"
 
+#include <fsfw/ipc/MutexGuard.h>
+
 #include "fsfw/serviceinterface/ServiceInterface.h"
 #include "fsfw_hal/linux/uio/UioMapper.h"
 
@@ -39,97 +41,60 @@ ReturnValue_t AxiPtmeConfig::writeCaduRateReg(uint8_t rateVal) {
   return returnvalue::OK;
 }
 
-ReturnValue_t AxiPtmeConfig::enableTxclockManipulator() {
+void AxiPtmeConfig::enableTxclockManipulator() {
-  ReturnValue_t result = writeBit(COMMON_CONFIG_REG, true, BitPos::EN_TX_CLK_MANIPULATOR);
+  writeBit(COMMON_CONFIG_REG, true, BitPos::EN_TX_CLK_MANIPULATOR);
-  if (result != returnvalue::OK) {
-    return result;
-  }
-  return returnvalue::OK;
 }
 
-ReturnValue_t AxiPtmeConfig::disableTxclockManipulator() {
+void AxiPtmeConfig::disableTxclockManipulator() {
-  ReturnValue_t result = writeBit(COMMON_CONFIG_REG, false, BitPos::EN_TX_CLK_MANIPULATOR);
+  writeBit(COMMON_CONFIG_REG, false, BitPos::EN_TX_CLK_MANIPULATOR);
-  if (result != returnvalue::OK) {
-    return result;
-  }
-  return returnvalue::OK;
 }
 
-ReturnValue_t AxiPtmeConfig::enableTxclockInversion() {
+void AxiPtmeConfig::enableTxclockInversion() {
-  ReturnValue_t result = writeBit(COMMON_CONFIG_REG, true, BitPos::INVERT_CLOCK);
+  writeBit(COMMON_CONFIG_REG, true, BitPos::INVERT_CLOCK);
-  if (result != returnvalue::OK) {
-    return result;
-  }
-  return returnvalue::OK;
 }
 
-ReturnValue_t AxiPtmeConfig::disableTxclockInversion() {
+void AxiPtmeConfig::disableTxclockInversion() {
-  ReturnValue_t result = writeBit(COMMON_CONFIG_REG, false, BitPos::INVERT_CLOCK);
+  writeBit(COMMON_CONFIG_REG, false, BitPos::INVERT_CLOCK);
-  if (result != returnvalue::OK) {
-    return result;
-  }
-  return returnvalue::OK;
 }
 
-ReturnValue_t AxiPtmeConfig::enableBatPriorityBit() {
+void AxiPtmeConfig::enableBatPriorityBit() {
-  ReturnValue_t result = writeBit(COMMON_CONFIG_REG, true, BitPos::EN_BAT_PRIORITY);
+  writeBit(COMMON_CONFIG_REG, true, BitPos::EN_BAT_PRIORITY);
-  if (result != returnvalue::OK) {
-    return result;
-  }
-  return returnvalue::OK;
 }
 
-ReturnValue_t AxiPtmeConfig::disableBatPriorityBit() {
+void AxiPtmeConfig::disableBatPriorityBit() {
-  ReturnValue_t result = writeBit(COMMON_CONFIG_REG, false, BitPos::EN_BAT_PRIORITY);
+  writeBit(COMMON_CONFIG_REG, false, BitPos::EN_BAT_PRIORITY);
-  if (result != returnvalue::OK) {
-    return result;
-  }
-  return returnvalue::OK;
 }
 
-ReturnValue_t AxiPtmeConfig::writeReg(uint32_t regOffset, uint32_t writeVal) {
+void AxiPtmeConfig::writeReg(uint32_t regOffset, uint32_t writeVal) {
-  ReturnValue_t result = returnvalue::OK;
+  MutexGuard mg(mutex, timeoutType, mutexTimeout);
-  result = mutex->lockMutex(timeoutType, mutexTimeout);
-  if (result != returnvalue::OK) {
-    sif::warning << "AxiPtmeConfig::readReg: Failed to lock mutex" << std::endl;
-    return returnvalue::FAILED;
-  }
   *(baseAddress + regOffset / ADRESS_DIVIDER) = writeVal;
-  result = mutex->unlockMutex();
-  if (result != returnvalue::OK) {
-    sif::warning << "AxiPtmeConfig::readReg: Failed to unlock mutex" << std::endl;
-    return returnvalue::FAILED;
-  }
-  return returnvalue::OK;
 }
 
-ReturnValue_t AxiPtmeConfig::readReg(uint32_t regOffset, uint32_t* readVal) {
+uint32_t AxiPtmeConfig::readReg(uint32_t regOffset) {
-  ReturnValue_t result = returnvalue::OK;
+  MutexGuard mg(mutex, timeoutType, mutexTimeout);
-  result = mutex->lockMutex(timeoutType, mutexTimeout);
+  return *(baseAddress + regOffset / ADRESS_DIVIDER);
-  if (result != returnvalue::OK) {
-    sif::warning << "AxiPtmeConfig::readReg: Failed to lock mutex" << std::endl;
-    return returnvalue::FAILED;
-  }
-  *readVal = *(baseAddress + regOffset / ADRESS_DIVIDER);
-  result = mutex->unlockMutex();
-  if (result != returnvalue::OK) {
-    sif::warning << "AxiPtmeConfig::readReg: Failed to unlock mutex" << std::endl;
-    return returnvalue::FAILED;
-  }
-  return returnvalue::OK;
 }
 
-ReturnValue_t AxiPtmeConfig::writeBit(uint32_t regOffset, bool bitVal, BitPos bitPos) {
+void AxiPtmeConfig::writePollThreshold(AxiPtmeConfig::IdlePollThreshold pollThreshold) {
-  uint32_t readVal = 0;
+  MutexGuard mg(mutex, timeoutType, mutexTimeout);
-  ReturnValue_t result = readReg(regOffset, &readVal);
+  uint32_t regVal = readCommonCfgReg();
-  if (result != returnvalue::OK) {
+  // Clear bits first
-    return result;
+  regVal &= ~(0b111 << 3);
-  }
+  regVal |= (static_cast<uint8_t>(pollThreshold) << 3);
+  writeCommonCfgReg(regVal);
+}
+
+AxiPtmeConfig::IdlePollThreshold AxiPtmeConfig::readPollThreshold() {
+  uint32_t regVal = readCommonCfgReg();
+  return static_cast<AxiPtmeConfig::IdlePollThreshold>((regVal >> 3) & 0b111);
+}
+
+void AxiPtmeConfig::writeCommonCfgReg(uint32_t value) { writeReg(COMMON_CONFIG_REG, value); }
+uint32_t AxiPtmeConfig::readCommonCfgReg() { return readReg(COMMON_CONFIG_REG); }
+
+void AxiPtmeConfig::writeBit(uint32_t regOffset, bool bitVal, BitPos bitPos) {
+  uint32_t readVal = readReg(regOffset);
   uint32_t writeVal =
       (readVal & ~(1 << static_cast<uint32_t>(bitPos))) | bitVal << static_cast<uint32_t>(bitPos);
-  result = writeReg(regOffset, writeVal);
+  writeReg(regOffset, writeVal);
-  if (result != returnvalue::OK) {
-    return result;
-  }
-  return returnvalue::OK;
 }

linux/ipcore/AxiPtmeConfig.h

@@ -14,6 +14,16 @@
  */
 class AxiPtmeConfig : public SystemObject {
  public:
+  enum IdlePollThreshold : uint8_t {
+    ALWAYS = 0b000,
+    POLL_1 = 0b001,
+    POLL_4 = 0b010,
+    POLL_16 = 0b011,
+    POLL_64 = 0b100,
+    POLL_256 = 0b101,
+    POLL_1024 = 0b110,
+    NEVER = 0b111
+  };
   /**
    * @brief   Constructor
    * @param axiUio    Device file of UIO belonging to the AXI configuration interface.
@@ -40,8 +50,8 @@ class AxiPtmeConfig : public SystemObject {
    *            Default: Enables TX clock manipulator
    *
    */
-  ReturnValue_t enableTxclockManipulator();
+  void enableTxclockManipulator();
-  ReturnValue_t disableTxclockManipulator();
+  void disableTxclockManipulator();
 
   /**
    * @brief The next to functions control whether data will be updated on the rising or falling edge
@@ -51,11 +61,14 @@ class AxiPtmeConfig : public SystemObject {
    *        Disable clock inversion. Data updated on rising edge.
    *        Default: Inversion is disabled
    */
-  ReturnValue_t enableTxclockInversion();
+  void enableTxclockInversion();
-  ReturnValue_t disableTxclockInversion();
+  void disableTxclockInversion();
 
-  ReturnValue_t enableBatPriorityBit();
+  void enableBatPriorityBit();
-  ReturnValue_t disableBatPriorityBit();
+  void disableBatPriorityBit();
+
+  void writePollThreshold(IdlePollThreshold pollThreshold);
+  IdlePollThreshold readPollThreshold();
 
  private:
   // Address of register storing the bitrate configuration parameter
@@ -80,7 +93,7 @@ class AxiPtmeConfig : public SystemObject {
    *
    * @param writeVal    Value to write
    */
-  ReturnValue_t writeReg(uint32_t regOffset, uint32_t writeVal);
+  void writeReg(uint32_t regOffset, uint32_t writeVal);
 
   /**
    * @brief Reads value from configuration register
@@ -88,7 +101,10 @@ class AxiPtmeConfig : public SystemObject {
    * @param regOffset   Offset of register from base address to read from
    * Qparam readVal     Pointer to variable where read value will be written to
    */
-  ReturnValue_t readReg(uint32_t regOffset, uint32_t* readVal);
+  uint32_t readReg(uint32_t regOffset);
+
+  uint32_t readCommonCfgReg();
+  void writeCommonCfgReg(uint32_t value);
 
   /**
    * @brief Sets one bit in a register
@@ -99,7 +115,7 @@ class AxiPtmeConfig : public SystemObject {
    *
    * @return             returnvalue::OK if successful, otherwise returnvalue::FAILED
    */
-  ReturnValue_t writeBit(uint32_t regOffset, bool bitVal, BitPos bitPos);
+  void writeBit(uint32_t regOffset, bool bitVal, BitPos bitPos);
 };
 
 #endif /* LINUX_OBC_AXIPTMECONFIG_H_ */

linux/ipcore/PtmeConfig.cpp

@@ -26,33 +26,30 @@ ReturnValue_t PtmeConfig::setRate(uint32_t bitRate) {
   return axiPtmeConfig->writeCaduRateReg(static_cast<uint8_t>(rateVal));
 }
 
-ReturnValue_t PtmeConfig::invertTxClock(bool invert) {
+void PtmeConfig::invertTxClock(bool invert) {
-  ReturnValue_t result = returnvalue::OK;
   if (invert) {
-    result = axiPtmeConfig->enableTxclockInversion();
+    axiPtmeConfig->enableTxclockInversion();
   } else {
-    result = axiPtmeConfig->disableTxclockInversion();
+    axiPtmeConfig->disableTxclockInversion();
   }
-  if (result != returnvalue::OK) {
-    return CLK_INVERSION_FAILED;
-  }
-  return result;
 }
 
-ReturnValue_t PtmeConfig::configTxManipulator(bool enable) {
+void PtmeConfig::configTxManipulator(bool enable) {
-  ReturnValue_t result = returnvalue::OK;
   if (enable) {
-    result = axiPtmeConfig->enableTxclockManipulator();
+    axiPtmeConfig->enableTxclockManipulator();
   } else {
-    result = axiPtmeConfig->disableTxclockManipulator();
+    axiPtmeConfig->disableTxclockManipulator();
   }
-  return result;
 }
 
-ReturnValue_t PtmeConfig::enableBatPriorityBit(bool enable) {
+void PtmeConfig::enableBatPriorityBit(bool enable) {
   if (enable) {
-    return axiPtmeConfig->enableBatPriorityBit();
+    axiPtmeConfig->enableBatPriorityBit();
   } else {
-    return axiPtmeConfig->disableBatPriorityBit();
+    axiPtmeConfig->disableBatPriorityBit();
   }
 }
+
+void PtmeConfig::setPollThreshold(AxiPtmeConfig::IdlePollThreshold pollThreshold) {
+  axiPtmeConfig->writePollThreshold(pollThreshold);
+}

linux/ipcore/PtmeConfig.h

@@ -43,7 +43,7 @@ class PtmeConfig : public SystemObject {
    *
    * @return        REUTRN_OK if successful, otherwise error return value
    */
-  ReturnValue_t invertTxClock(bool invert);
+  void invertTxClock(bool invert);
 
   /**
    * @brief Controls the tx clock manipulator of the PTME wrapper component
@@ -53,7 +53,7 @@ class PtmeConfig : public SystemObject {
    *
    * @return        REUTRN_OK if successful, otherwise error return value
    */
-  ReturnValue_t configTxManipulator(bool enable);
+  void configTxManipulator(bool enable);
 
   /**
    * Enable the bat priority bit in the PTME wrapper component.
@@ -62,7 +62,9 @@ class PtmeConfig : public SystemObject {
    * @param enable
    * @return
    */
-  ReturnValue_t enableBatPriorityBit(bool enable);
+  void enableBatPriorityBit(bool enable);
+
+  void setPollThreshold(AxiPtmeConfig::IdlePollThreshold pollThreshold);
 
  private:
   static const uint8_t INTERFACE_ID = CLASS_ID::RATE_SETTER;

mission/com/CcsdsIpCoreHandler.cpp

@@ -75,11 +75,9 @@ ReturnValue_t CcsdsIpCoreHandler::initialize() {
   }
 
   // This also pulls the PTME out of reset state.
-  if (batPriorityParam == 0) {
+  updateBatPriorityFromParam();
-    disablePrioritySelectMode();
+  ptmeConfig.setPollThreshold(
-  } else {
+      static_cast<AxiPtmeConfig::IdlePollThreshold>(params.pollThresholdParam));
-    enablePrioritySelectMode();
-  }
   resetPtme();
   ptmeLocked = false;
 
@@ -123,7 +121,10 @@ ReturnValue_t CcsdsIpCoreHandler::getParameter(uint8_t domainId, uint8_t uniqueI
                                                ParameterWrapper* parameterWrapper,
                                                const ParameterWrapper* newValues,
                                                uint16_t startAtIndex) {
-  if ((domainId == 0) and (uniqueIdentifier == ParamId::BAT_PRIORITY)) {
+  if (domainId != 0) {
+    return HasParametersIF::INVALID_DOMAIN_ID;
+  }
+  if (uniqueIdentifier == ParamId::BAT_PRIORITY) {
     uint8_t newVal = 0;
     ReturnValue_t result = newValues->getElement(&newVal);
     if (result != returnvalue::OK) {
@@ -132,8 +133,8 @@ ReturnValue_t CcsdsIpCoreHandler::getParameter(uint8_t domainId, uint8_t uniqueI
     if (newVal > 1) {
       return HasParametersIF::INVALID_VALUE;
     }
-    parameterWrapper->set(batPriorityParam);
+    parameterWrapper->set(params.batPriorityParam);
-    if (newVal != batPriorityParam) {
+    if (newVal != params.batPriorityParam) {
       // This ensures that the BAT priority is updated at some point when an update of the PTME is
       // allowed
       updateContext.updateBatPrio = true;
@@ -144,6 +145,23 @@ ReturnValue_t CcsdsIpCoreHandler::getParameter(uint8_t domainId, uint8_t uniqueI
       }
     }
     return returnvalue::OK;
+  } else if (uniqueIdentifier == ParamId::POLL_THRESHOLD) {
+    uint8_t newVal = 0;
+    ReturnValue_t result = newValues->getElement(&newVal);
+    if (result != returnvalue::OK) {
+      return result;
+    }
+    if (newVal > static_cast<uint8_t>(AxiPtmeConfig::NEVER)) {
+      return HasParametersIF::INVALID_VALUE;
+    }
+    parameterWrapper->set(newVal);
+    if (newVal != params.pollThresholdParam) {
+      updateContext.updatePollThreshold = true;
+      if (mode == MODE_OFF) {
+        initPtmeUpdateAfterXCycles();
+      }
+    }
+    return returnvalue::OK;
   }
   return HasParametersIF::INVALID_IDENTIFIER_ID;
 }
@@ -161,24 +179,31 @@ ReturnValue_t CcsdsIpCoreHandler::executeAction(ActionId_t actionId, MessageQueu
   switch (actionId) {
     case ARBITRARY_RATE: {
       uint32_t bitrate = 0;
-      SerializeAdapter::deSerialize(&bitrate, &data, &size, SerializeIF::Endianness::BIG);
+      result = SerializeAdapter::deSerialize(&bitrate, &data, &size, SerializeIF::Endianness::BIG);
-      result = ptmeConfig.setRate(bitrate);
+      if (result != returnvalue::OK) {
+        return result;
+      }
+      ptmeConfig.setRate(bitrate);
+      updateContext.updateClockRate = true;
+      if (mode == MODE_OFF) {
+        initPtmeUpdateAfterXCycles();
+      }
       break;
     }
     case ENABLE_TX_CLK_MANIPULATOR: {
-      result = ptmeConfig.configTxManipulator(true);
+      ptmeConfig.configTxManipulator(true);
       break;
     }
     case DISABLE_TX_CLK_MANIPULATOR: {
-      result = ptmeConfig.configTxManipulator(false);
+      ptmeConfig.configTxManipulator(false);
       break;
     }
     case UPDATE_ON_RISING_EDGE: {
-      result = ptmeConfig.invertTxClock(false);
+      ptmeConfig.invertTxClock(false);
       break;
     }
     case UPDATE_ON_FALLING_EDGE: {
-      result = ptmeConfig.invertTxClock(true);
+      ptmeConfig.invertTxClock(true);
       break;
     }
     default:
@@ -271,7 +296,7 @@ void CcsdsIpCoreHandler::enablePrioritySelectMode() { ptmeConfig.enableBatPriori
 void CcsdsIpCoreHandler::disablePrioritySelectMode() { ptmeConfig.enableBatPriorityBit(false); }
 
 void CcsdsIpCoreHandler::updateBatPriorityFromParam() {
-  if (batPriorityParam == 0) {
+  if (params.batPriorityParam == 0) {
     disablePrioritySelectMode();
   } else {
     enablePrioritySelectMode();
@@ -290,9 +315,17 @@ void CcsdsIpCoreHandler::performPtmeUpdateWhenApplicable() {
     return;
   }
   if (updateContext.ptmeUpdateCycleCount >= 2) {
+    bool doResetPtme = false;
     if (updateContext.updateBatPrio) {
       updateBatPriorityFromParam();
       updateContext.updateBatPrio = false;
+      doResetPtme = true;
+    }
+    if (updateContext.updatePollThreshold) {
+      ptmeConfig.setPollThreshold(
+          static_cast<AxiPtmeConfig::IdlePollThreshold>(params.pollThresholdParam));
+      updateContext.updatePollThreshold = false;
+      doResetPtme = true;
     }
     ReturnValue_t result = returnvalue::OK;
     if (updateContext.updateClockRate) {
@@ -312,10 +345,7 @@ void CcsdsIpCoreHandler::performPtmeUpdateWhenApplicable() {
         sif::error << "CcsdsIpCoreHandler: Setting datarate failed" << std::endl;
       }
       updateContext.updateClockRate = false;
-    }
+      doResetPtme = true;
-    bool doResetPtme = true;
-    if (not updateContext.updateBatPrio and not updateContext.updateClockRate) {
-      doResetPtme = false;
     }
     finishPtmeUpdateAfterXCycles(doResetPtme);
     return;

mission/com/CcsdsIpCoreHandler.h

@@ -60,7 +60,7 @@ class CcsdsIpCoreHandler : public SystemObject,
                            public ReceivesParameterMessagesIF,
                            public HasActionsIF {
  public:
-  enum ParamId : uint8_t { BAT_PRIORITY = 0 };
+  enum ParamId : uint8_t { BAT_PRIORITY = 0, POLL_THRESHOLD = 1 };
 
   static const bool LINK_UP = true;
   static const bool LINK_DOWN = false;
@@ -156,12 +156,17 @@ class CcsdsIpCoreHandler : public SystemObject,
 
   PtmeConfig& ptmeConfig;
   PtmeGpios ptmeGpios;
-  // BAT priority bit on by default to enable priority selection mode for the PTME.
+  struct Parameters {
-  uint8_t batPriorityParam = 0;
+    // BAT priority bit on by default to enable priority selection mode for the PTME.
+    uint8_t batPriorityParam = 0;
+    uint8_t pollThresholdParam = static_cast<uint8_t>(AxiPtmeConfig::IdlePollThreshold::POLL_4);
+
+  } params;
 
   struct UpdateContext {
     bool updateBatPrio = false;
     bool updateClockRate = false;
+    bool updatePollThreshold = false;
     bool enableTransmitAfterPtmeUpdate = false;
     uint8_t ptmeUpdateCycleCount = 0;
     bool performPtmeUpdateAfterXCycles = false;