diff --git a/CHANGELOG.md b/CHANGELOG.md index fe5c2dec..ede02f9e 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/linux/ipcore/PdecHandler.cpp b/linux/ipcore/PdecHandler.cpp index 57d59841..0598cc95 100644 --- a/linux/ipcore/PdecHandler.cpp +++ b/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(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(dummy); return returnvalue::OK; } diff --git a/linux/ipcore/PdecHandler.h b/linux/ipcore/PdecHandler.h index 6ebd9ce6..8e08adc8 100644 --- a/linux/ipcore/PdecHandler.h +++ b/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; diff --git a/tmtc b/tmtc index 94a82b84..33da498e 160000 --- a/tmtc +++ b/tmtc @@ -1 +1 @@ -Subproject commit 94a82b84e86177d122f4ac12eff6e06528f6290b +Subproject commit 33da498ea8aeb2e8b328d040b3cd0720aa12f50c