Merge pull request 'PDEC evil fix' (#374) from pdec_evil_fix into develop

Reviewed-on: #374

CHANGELOG.md

@@ -23,6 +23,13 @@ change warranting a new major release:
   that scheduling is done in big blocks to reduce risk of missed deadlines.
 - Replaced chained locks for polling new sensor data to the `AcsController`
 
+## Fixed
+
+- Bugfix for PDEC handler which causes the PIR register of the PDEC to never
+  be cleared on release builds. The dummy variable used to read the register
+  needs to be declared volatile to avoid compiler optimizations.
+  PR: https://egit.irs.uni-stuttgart.de/eive/eive-obsw/pulls/374
+
 # [v1.26.1] 2023-02-08
 
 - Initialize parameter helper in ACS controller.

linux/ipcore/PdecHandler.cpp

@@ -132,6 +132,7 @@ ReturnValue_t PdecHandler::polledOperation() {
   return returnvalue::OK;
 }
 
+// See https://yurovsky.github.io/2014/10/10/linux-uio-gpio-interrupt.html for more information.
 ReturnValue_t PdecHandler::irqOperation() {
   ReturnValue_t result = returnvalue::OK;
   int fd = open(uioNames.irq, O_RDWR);
@@ -141,15 +142,17 @@ ReturnValue_t PdecHandler::irqOperation() {
     return returnvalue::FAILED;
   }
 
-  struct pollfd fds = {.fd = fd, .events = POLLIN, .revents = 0};
   // Used to unmask IRQ
   uint32_t info = 1;
   ssize_t nb = 0;
   int ret = 0;
-  // Clear interrupts with dummy read before unmasking the interrupt
-  ret = *(registerBaseAddress + PDEC_PIR_OFFSET);
+  // Clear interrupts with dummy read before unmasking the interrupt. Use a volatile to prevent
+  // read being optimized away.
+  volatile uint32_t dummy = *(registerBaseAddress + PDEC_PIR_OFFSET);
 
   while (true) {
+    // Default value to unmask IRQ on the write call.
+    info = 1;
     readCommandQueue();
     switch (state) {
       case State::INIT:
@@ -166,9 +169,12 @@ ReturnValue_t PdecHandler::irqOperation() {
         nb = write(fd, &info, sizeof(info));
         if (nb != static_cast<ssize_t>(sizeof(info))) {
           sif::error << "PdecHandler::irqOperation: Unmasking IRQ failed" << std::endl;
+          triggerEvent(WRITE_SYSCALL_ERROR_PDEC, errno);
           close(fd);
+          state = State::INIT;
+          return returnvalue::FAILED;
         }
-
+        struct pollfd fds = {.fd = fd, .events = POLLIN, .revents = 0};
         ret = poll(&fds, 1, IRQ_TIMEOUT_MS);
         if (ret == 0) {
           // No TCs for timeout period
@@ -195,12 +201,15 @@ ReturnValue_t PdecHandler::irqOperation() {
               lockCheckCd.resetTimer();
             }
             // Clear interrupts with dummy read
-            ret = *(registerBaseAddress + PDEC_PIR_OFFSET);
+            dummy = *(registerBaseAddress + PDEC_PIR_OFFSET);
           }
         } else {
           sif::error << "PdecHandler::irqOperation: Poll error with errno " << errno << ": "
                      << strerror(errno) << std::endl;
-          triggerEvent(POLL_ERROR_PDEC, errno);
+          triggerEvent(POLL_SYSCALL_ERROR_PDEC, errno);
+          close(fd);
+          state = State::INIT;
+          return returnvalue::FAILED;
         }
         break;
       }
@@ -211,6 +220,8 @@ ReturnValue_t PdecHandler::irqOperation() {
         break;
     }
   }
+  // To avoid compiler warning
+  static_cast<void>(dummy);
   return returnvalue::OK;
 }
 

linux/ipcore/PdecHandler.h

@@ -87,7 +87,8 @@ class PdecHandler : public SystemObject, public ExecutableObjectIF, public HasAc
   static const Event LOST_CARRIER_LOCK_PDEC = MAKE_EVENT(5, severity::INFO);
   //! [EXPORT] : [COMMENT] Lost bit lock
   static const Event LOST_BIT_LOCK_PDEC = MAKE_EVENT(6, severity::INFO);
-  static constexpr Event POLL_ERROR_PDEC = event::makeEvent(SUBSYSTEM_ID, 7, severity::MEDIUM);
+  static constexpr Event POLL_SYSCALL_ERROR_PDEC = event::makeEvent(SUBSYSTEM_ID, 7, severity::MEDIUM);
+  static constexpr Event WRITE_SYSCALL_ERROR_PDEC = event::makeEvent(SUBSYSTEM_ID, 8, severity::MEDIUM);
 
  private:
   static const uint8_t INTERFACE_ID = CLASS_ID::PDEC_HANDLER;

tmtc

@@ -1 +1 @@
-Subproject commit 94a82b84e86177d122f4ac12eff6e06528f6290b
+Subproject commit 33da498ea8aeb2e8b328d040b3cd0720aa12f50c