Add --strict-failsafe-pwm compile flag
This build flag is used to set the fans strictly at the failsafe
percent when in failsafe mode, even when the calculated PWM is higher
than failsafe PWM. Without this enabled, the PWM is calculated and set
to the calculated PWM the failsafe PWM, whichever is higher.
Added a unit test that can test this new build flag code path if the
compile flag is defined.
Tested:
Verified on an internal machine that by adding the following to the
bbappend:
EXTRA_OECONF:append = " --enable-strict-failsafe-pwm=yes"
With flag:
ipmitool sensor list
fan_pwm | 89.768
Without flag:
ipmitool sensor list
fan_pwm | 99.960
We can see that the fan pwm was limited to the failsafe percentage when
in failsafe mode with the flag. Without the flag, it ran at 100%
Bug-Id: openbmc/phosphor-pid-control#17
Signed-off-by: Brandon Kim <brandonkim@google.com>
Change-Id: I72a1e5aab8d3e5b0e3716f0b3720d704a6f05008
diff --git a/configure.ac b/configure.ac
index d30af1f..622b091 100644
--- a/configure.ac
+++ b/configure.ac
@@ -204,6 +204,14 @@
)
)
+AC_ARG_ENABLE([strict-failsafe-pwm],
+ AS_HELP_STRING([--strict-failsafe-pwm],
+ [Set the fans strictly at the failsafe PWM when in failsafe mode]))
+AM_CONDITIONAL(ENABLE_STRICT_FAILSAFE_PWM, [test "x$enable_strict_failsafe_pwm" = "xyes"])
+AS_IF([test "x$enable_strict_failsafe_pwm" = "xyes"], [
+ AX_APPEND_COMPILE_FLAGS([-DSTRICT_FAILSAFE_PWM], [CXXFLAGS])
+])
+
AC_ARG_VAR(SYSTEMD_TARGET, "Target for starting this service")
AS_IF([test "x$SYSTEMD_TARGET" == "x"], [SYSTEMD_TARGET="multi-user.target"])
diff --git a/configure.md b/configure.md
index 3dbb320..44fd6ff 100644
--- a/configure.md
+++ b/configure.md
@@ -14,6 +14,15 @@
key names are not identical to JSON but similar enough to see the
correspondence.
+## Compile Flag Configuration
+
+### --strict-failsafe-pwm
+
+This build flag is used to set the fans strictly at the failsafe percent when
+in failsafe mode, even when the calculated PWM is higher than failsafe PWM.
+Without this enabled, the PWM is calculated and set to the calculated PWM
+**or** the failsafe PWM, whichever is higher.
+
## JSON Configuration
Default config file path `/usr/share/swampd/config.json` can be overridden by
diff --git a/pid/fancontroller.cpp b/pid/fancontroller.cpp
index d7ac0e6..33e2d9f 100644
--- a/pid/fancontroller.cpp
+++ b/pid/fancontroller.cpp
@@ -130,11 +130,22 @@
{
if (_owner->getFailSafeMode())
{
- /* In case it's being set to 100% */
- if (percent < _owner->getFailSafePercent())
+ double failsafePercent = _owner->getFailSafePercent();
+
+#ifdef STRICT_FAILSAFE_PWM
+ // Unconditionally replace the computed PWM with the
+ // failsafe PWM if STRICT_FAILSAFE_PWM is defined.
+ percent = failsafePercent;
+#else
+ // Ensure PWM is never lower than the failsafe PWM.
+ // The computed PWM is still allowed to rise higher than
+ // failsafe PWM if STRICT_FAILSAFE_PWM is NOT defined.
+ // This is the default behavior.
+ if (percent < failsafePercent)
{
- percent = _owner->getFailSafePercent();
+ percent = failsafePercent;
}
+#endif
}
}
diff --git a/test/pid_fancontroller_unittest.cpp b/test/pid_fancontroller_unittest.cpp
index 883ca84..f330202 100644
--- a/test/pid_fancontroller_unittest.cpp
+++ b/test/pid_fancontroller_unittest.cpp
@@ -150,7 +150,7 @@
{
// Verify that if failsafe mode is enabled and the input value for the fans
// is below the failsafe minimum value, the input is not used and the fans
- // are driven at failsafe RPM.
+ // are driven at failsafe RPM (this assumes STRICT_FAILSAFE_PWM is not set)
ZoneMock z;
@@ -162,7 +162,7 @@
EXPECT_FALSE(p == nullptr);
EXPECT_CALL(z, getFailSafeMode()).WillOnce(Return(true));
- EXPECT_CALL(z, getFailSafePercent()).Times(2).WillRepeatedly(Return(75.0));
+ EXPECT_CALL(z, getFailSafePercent()).WillOnce(Return(75.0));
int64_t timeout = 0;
std::unique_ptr<Sensor> s1 = std::make_unique<SensorMock>("fan0", timeout);
@@ -223,35 +223,44 @@
p->outputProc(50.0);
}
-TEST(FanControllerTest, OutputProc_VerifyFailSafeIgnoredIfInputHigher)
+TEST(FanControllerTest, OutputProc_VerifyFailSafeWhenInputHigher)
{
- // If the requested output is higher than the failsafe value, then use the
- // value provided to outputProc.
+ // If STRICT_FAILSAFE_PWM flag is NOT defined and the requested output is
+ // higher than the failsafe value, then use the value provided to outputProc
+ //
+ // If STRICT_FAILSAFE_PWM is defined, we expect the FailSafe PWM to be
+ // capped to the failsafe PWM, and not go higher than that.
ZoneMock z;
std::vector<std::string> inputs = {"fan0"};
ec::pidinfo initial;
+ const double failsafePWM = 75.0;
std::unique_ptr<PIDController> p =
FanController::createFanPid(&z, "fan1", inputs, initial);
EXPECT_FALSE(p == nullptr);
EXPECT_CALL(z, getFailSafeMode()).WillOnce(Return(true));
- EXPECT_CALL(z, getFailSafePercent()).WillOnce(Return(75.0));
+ EXPECT_CALL(z, getFailSafePercent()).WillOnce(Return(failsafePWM));
int64_t timeout = 0;
std::unique_ptr<Sensor> s1 = std::make_unique<SensorMock>("fan0", timeout);
// Grab pointer for mocking.
SensorMock* sm1 = reinterpret_cast<SensorMock*>(s1.get());
- // Converting from double to double for expectation.
double percent = 80;
- double value = percent / 100;
EXPECT_CALL(z, getRedundantWrite()).WillOnce(Return(false));
EXPECT_CALL(z, getSensor(StrEq("fan0"))).WillOnce(Return(s1.get()));
+#ifdef STRICT_FAILSAFE_PWM
+ double failsafeValue = failsafePWM / 100;
+ EXPECT_CALL(*sm1, write(failsafeValue, false, _));
+#else
+ // Converting from double to double for expectation.
+ double value = percent / 100;
EXPECT_CALL(*sm1, write(value, false, _));
+#endif
// This is a fan PID, so calling outputProc will try to write this value
// to the sensors.