pseq: Remove output voltage comparison to OV limit
The pgood isolation algorithm for the phosphor-power-sequencer
application is being enhanced to handle more error scenarios.
One major enhancement is the option to determine the pgood status of a
voltage rail by comparing the output voltage to a fault limit.
The original design was to compare the output voltage to both the
overvoltage limit (VOUT_OV_FAULT_LIMIT) and the undervoltage limit
(VOUT_UV_FAULT_LIMIT).
After discussions with subject matter experts, the design has been
changed to remove the comparison with the overvoltage limit. While this
is important status information, it is not necessarily indicative of a
pgood fault and could lead to false positives.
Change-Id: I664e4b16fcaf32900798c59aeb4e4ed48db964bf
Signed-off-by: Shawn McCarney <shawnmm@us.ibm.com>
diff --git a/phosphor-power-sequencer/docs/config_file/rail.md b/phosphor-power-sequencer/docs/config_file/rail.md
index 7346c13..a65073d 100644
--- a/phosphor-power-sequencer/docs/config_file/rail.md
+++ b/phosphor-power-sequencer/docs/config_file/rail.md
@@ -4,26 +4,26 @@
A voltage rail that is enabled or monitored by the power sequencer device.
-The "check_status_vout", "compare_voltage_to_limits", and "gpio" properties
+The "check_status_vout", "compare_voltage_to_limit", and "gpio" properties
specify how to obtain the pgood status of the rail. You can specify more than
one of these properties if necessary.
## Properties
-| Name | Required | Type | Description |
-| :------------------------ | :-----------------: | :---------------------- | :----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
-| name | yes | string | Unique name for the rail. Can only contain letters (A-Z, a-z), numbers (0-9), period (.), and underscore (\_). |
-| presence | no | string | D-Bus inventory path of a system component which must be present in order for the rail to be present. If this property is not specified, the rail is assumed to always be present. |
-| page | see [notes](#notes) | number | PMBus PAGE number of the rail. |
-| is_power_supply_rail | no | boolean (true or false) | If true, this rail is produced by a power supply. Power supply rails require special error handling. If an error is detected in a power supply device, and the pgood status is false for a power supply rail, the power supply error is logged as the cause of the pgood failure. The default value of this property is false. |
-| check_status_vout | no | boolean (true or false) | If true, determine the pgood status of the rail by checking the value of the PMBus STATUS_VOUT command. If one of the error bits is set, the rail pgood will be considered false. The default value of this property is false. |
-| compare_voltage_to_limits | no | boolean (true or false) | If true, determine the pgood status of the rail by comparing the output voltage (READ_VOUT) to the undervoltage (VOUT_UV_FAULT_LIMIT) and overvoltage (VOUT_OV_FAULT_LIMIT) limits. If the output voltage is beyond those limits, the rail pgood will be considered false. The default value of this property is false. |
-| gpio | no | [gpio](gpio.md) | Determine the pgood status of the rail by reading a GPIO. |
+| Name | Required | Type | Description |
+| :----------------------- | :-----------------: | :---------------------- | :----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
+| name | yes | string | Unique name for the rail. Can only contain letters (A-Z, a-z), numbers (0-9), period (.), and underscore (\_). |
+| presence | no | string | D-Bus inventory path of a system component which must be present in order for the rail to be present. If this property is not specified, the rail is assumed to always be present. |
+| page | see [notes](#notes) | number | PMBus PAGE number of the rail. |
+| is_power_supply_rail | no | boolean (true or false) | If true, this rail is produced by a power supply. Power supply rails require special error handling. If an error is detected in a power supply device, and the pgood status is false for a power supply rail, the power supply error is logged as the cause of the pgood failure. The default value of this property is false. |
+| check_status_vout | no | boolean (true or false) | If true, determine the pgood status of the rail by checking the value of the PMBus STATUS_VOUT command. If one of the error bits is set, the rail pgood will be considered false. The default value of this property is false. |
+| compare_voltage_to_limit | no | boolean (true or false) | If true, determine the pgood status of the rail by comparing the output voltage (READ_VOUT) to the undervoltage fault limit (VOUT_UV_FAULT_LIMIT). If the output voltage is below this limit, the rail pgood will be considered false. The default value of this property is false. |
+| gpio | no | [gpio](gpio.md) | Determine the pgood status of the rail by reading a GPIO. |
### Notes
- The "page" property is required if the "check_status_vout" or
- "compare_voltage_to_limits" property is true.
+ "compare_voltage_to_limit" property is true.
## Examples
diff --git a/phosphor-power-sequencer/src/config_file_parser.cpp b/phosphor-power-sequencer/src/config_file_parser.cpp
index 37c6b04..fe9c661 100644
--- a/phosphor-power-sequencer/src/config_file_parser.cpp
+++ b/phosphor-power-sequencer/src/config_file_parser.cpp
@@ -118,12 +118,12 @@
++propertyCount;
}
- // Optional compare_voltage_to_limits property
- bool compareVoltageToLimits{false};
- auto compareVoltageToLimitsIt = element.find("compare_voltage_to_limits");
- if (compareVoltageToLimitsIt != element.end())
+ // Optional compare_voltage_to_limit property
+ bool compareVoltageToLimit{false};
+ auto compareVoltageToLimitIt = element.find("compare_voltage_to_limit");
+ if (compareVoltageToLimitIt != element.end())
{
- compareVoltageToLimits = parseBoolean(*compareVoltageToLimitsIt);
+ compareVoltageToLimit = parseBoolean(*compareVoltageToLimitIt);
++propertyCount;
}
@@ -136,9 +136,9 @@
++propertyCount;
}
- // If check_status_vout or compare_voltage_to_limits property is true,
- // the page property is required; verify page was specified
- if ((checkStatusVout || compareVoltageToLimits) && !page.has_value())
+ // If check_status_vout or compare_voltage_to_limit property is true, the
+ // page property is required; verify page was specified
+ if ((checkStatusVout || compareVoltageToLimit) && !page.has_value())
{
throw std::invalid_argument{"Required property missing: page"};
}
@@ -147,8 +147,7 @@
verifyPropertyCount(element, propertyCount);
return std::make_unique<Rail>(name, presence, page, isPowerSupplyRail,
- checkStatusVout, compareVoltageToLimits,
- gpio);
+ checkStatusVout, compareVoltageToLimit, gpio);
}
std::vector<std::unique_ptr<Rail>> parseRailArray(const json& element)
diff --git a/phosphor-power-sequencer/src/rail.hpp b/phosphor-power-sequencer/src/rail.hpp
index 6ac8610..a0115e5 100644
--- a/phosphor-power-sequencer/src/rail.hpp
+++ b/phosphor-power-sequencer/src/rail.hpp
@@ -70,31 +70,30 @@
* @param presence Optional D-Bus inventory path of a system component which
* must be present in order for the rail to be present
* @param page Optional PMBus PAGE number of the rail. Required if
- * checkStatusVout or compareVoltageToLimits is true.
+ * checkStatusVout or compareVoltageToLimit is true.
* @param isPowerSupplyRail Specifies whether the rail is produced by a
* power supply
* @param checkStatusVout Specifies whether to check the value of the PMBus
* STATUS_VOUT command when determining the pgood
* status of the rail
- * @param compareVoltageToLimits Specifies whether to compare the output
- * voltage to the undervoltage and
- * overvoltage limits when determining the
- * pgood status of the rail
+ * @param compareVoltageToLimit Specifies whether to compare the output
+ * voltage to the undervoltage fault limit when
+ * determining the pgood status of the rail
* @param gpio Optional GPIO to read to determine the pgood status of the
* rail
*/
explicit Rail(const std::string& name,
const std::optional<std::string>& presence,
const std::optional<uint8_t>& page, bool isPowerSupplyRail,
- bool checkStatusVout, bool compareVoltageToLimits,
+ bool checkStatusVout, bool compareVoltageToLimit,
const std::optional<GPIO>& gpio) :
name{name},
presence{presence}, page{page}, isPsuRail{isPowerSupplyRail},
checkStatusVout{checkStatusVout},
- compareVoltageToLimits{compareVoltageToLimits}, gpio{gpio}
+ compareVoltageToLimit{compareVoltageToLimit}, gpio{gpio}
{
// If checking STATUS_VOUT or output voltage, verify PAGE was specified
- if ((checkStatusVout || compareVoltageToLimits) && !page.has_value())
+ if ((checkStatusVout || compareVoltageToLimit) && !page.has_value())
{
throw std::invalid_argument{"PMBus PAGE is required"};
}
@@ -154,13 +153,13 @@
/**
* Returns whether the output voltage should be compared to the undervoltage
- * and overvoltage limits when determining the pgood status of the rail.
+ * fault limit when determining the pgood status of the rail.
*
- * @return true if output voltage is compared to limits, false otherwise
+ * @return true if output voltage is compared to limit, false otherwise
*/
- bool getCompareVoltageToLimits() const
+ bool getCompareVoltageToLimit() const
{
- return compareVoltageToLimits;
+ return compareVoltageToLimit;
}
/**
@@ -207,16 +206,15 @@
bool checkStatusVout{false};
/**
- * Specifies whether to compare the output voltage to the undervoltage and
- * overvoltage limits when determining the pgood status of the rail.
+ * Specifies whether to compare the output voltage to the undervoltage fault
+ * limit when determining the pgood status of the rail.
*
- * If the output voltage is beyond those limits, the rail pgood will be
+ * If the output voltage is below this limit, the rail pgood will be
* considered false.
*
- * Uses the values of the PMBus READ_VOUT, VOUT_UV_FAULT_LIMIT, and
- * VOUT_OV_FAULT_LIMIT commands.
+ * Uses the values of the PMBus READ_VOUT and VOUT_UV_FAULT_LIMIT commands.
*/
- bool compareVoltageToLimits{false};
+ bool compareVoltageToLimit{false};
/**
* GPIO to read to determine the pgood status of the rail.
diff --git a/phosphor-power-sequencer/test/config_file_parser_tests.cpp b/phosphor-power-sequencer/test/config_file_parser_tests.cpp
index 738d025..f28d827 100644
--- a/phosphor-power-sequencer/test/config_file_parser_tests.cpp
+++ b/phosphor-power-sequencer/test/config_file_parser_tests.cpp
@@ -308,7 +308,7 @@
EXPECT_FALSE(rail->getPage().has_value());
EXPECT_FALSE(rail->isPowerSupplyRail());
EXPECT_FALSE(rail->getCheckStatusVout());
- EXPECT_FALSE(rail->getCompareVoltageToLimits());
+ EXPECT_FALSE(rail->getCompareVoltageToLimit());
EXPECT_FALSE(rail->getGPIO().has_value());
}
@@ -321,7 +321,7 @@
"page": 11,
"is_power_supply_rail": true,
"check_status_vout": true,
- "compare_voltage_to_limits": true,
+ "compare_voltage_to_limit": true,
"gpio": { "line": 60, "active_low": true }
}
)"_json;
@@ -334,7 +334,7 @@
EXPECT_EQ(rail->getPage().value(), 11);
EXPECT_TRUE(rail->isPowerSupplyRail());
EXPECT_TRUE(rail->getCheckStatusVout());
- EXPECT_TRUE(rail->getCompareVoltageToLimits());
+ EXPECT_TRUE(rail->getCompareVoltageToLimit());
EXPECT_TRUE(rail->getGPIO().has_value());
EXPECT_EQ(rail->getGPIO().value().line, 60);
EXPECT_TRUE(rail->getGPIO().value().activeLow);
@@ -453,13 +453,13 @@
EXPECT_STREQ(e.what(), "Element is not a boolean");
}
- // Test where fails: compare_voltage_to_limits value is invalid
+ // Test where fails: compare_voltage_to_limit value is invalid
try
{
const json element = R"(
{
"name": "VCS_CPU1",
- "compare_voltage_to_limits": 23
+ "compare_voltage_to_limit": 23
}
)"_json;
parseRail(element);
@@ -504,14 +504,14 @@
EXPECT_STREQ(e.what(), "Required property missing: page");
}
- // Test where fails: compare_voltage_to_limits is true and page not
+ // Test where fails: compare_voltage_to_limit is true and page not
// specified
try
{
const json element = R"(
{
"name": "VCS_CPU1",
- "compare_voltage_to_limits": true
+ "compare_voltage_to_limit": true
}
)"_json;
parseRail(element);
diff --git a/phosphor-power-sequencer/test/rail_tests.cpp b/phosphor-power-sequencer/test/rail_tests.cpp
index bd1b51a..9b05b02 100644
--- a/phosphor-power-sequencer/test/rail_tests.cpp
+++ b/phosphor-power-sequencer/test/rail_tests.cpp
@@ -50,14 +50,14 @@
std::optional<uint8_t> page{};
bool isPowerSupplyRail{true};
bool checkStatusVout{false};
- bool compareVoltageToLimits{false};
+ bool compareVoltageToLimit{false};
std::optional<GPIO> gpio{};
Rail rail{name,
presence,
page,
isPowerSupplyRail,
checkStatusVout,
- compareVoltageToLimits,
+ compareVoltageToLimit,
gpio};
EXPECT_EQ(rail.getName(), "12.0V");
@@ -65,7 +65,7 @@
EXPECT_FALSE(rail.getPage().has_value());
EXPECT_TRUE(rail.isPowerSupplyRail());
EXPECT_FALSE(rail.getCheckStatusVout());
- EXPECT_FALSE(rail.getCompareVoltageToLimits());
+ EXPECT_FALSE(rail.getCompareVoltageToLimit());
EXPECT_FALSE(rail.getGPIO().has_value());
}
@@ -77,14 +77,14 @@
std::optional<uint8_t> page{11};
bool isPowerSupplyRail{false};
bool checkStatusVout{true};
- bool compareVoltageToLimits{true};
+ bool compareVoltageToLimit{true};
std::optional<GPIO> gpio{GPIO(60, true)};
Rail rail{name,
presence,
page,
isPowerSupplyRail,
checkStatusVout,
- compareVoltageToLimits,
+ compareVoltageToLimit,
gpio};
EXPECT_EQ(rail.getName(), "VCS_CPU1");
@@ -96,7 +96,7 @@
EXPECT_EQ(rail.getPage().value(), 11);
EXPECT_FALSE(rail.isPowerSupplyRail());
EXPECT_TRUE(rail.getCheckStatusVout());
- EXPECT_TRUE(rail.getCompareVoltageToLimits());
+ EXPECT_TRUE(rail.getCompareVoltageToLimit());
EXPECT_TRUE(rail.getGPIO().has_value());
EXPECT_EQ(rail.getGPIO().value().line, 60);
EXPECT_TRUE(rail.getGPIO().value().activeLow);
@@ -109,24 +109,24 @@
std::optional<uint8_t> page{};
bool isPowerSupplyRail{false};
bool checkStatusVout{true};
- bool compareVoltageToLimits{false};
+ bool compareVoltageToLimit{false};
std::optional<GPIO> gpio{};
EXPECT_THROW((Rail{name, presence, page, isPowerSupplyRail,
- checkStatusVout, compareVoltageToLimits, gpio}),
+ checkStatusVout, compareVoltageToLimit, gpio}),
std::invalid_argument);
}
- // Test where fails: compareVoltageToLimits is true and page has no value
+ // Test where fails: compareVoltageToLimit is true and page has no value
{
std::string name{"VDD1"};
std::optional<std::string> presence{};
std::optional<uint8_t> page{};
bool isPowerSupplyRail{false};
bool checkStatusVout{false};
- bool compareVoltageToLimits{true};
+ bool compareVoltageToLimit{true};
std::optional<GPIO> gpio{};
EXPECT_THROW((Rail{name, presence, page, isPowerSupplyRail,
- checkStatusVout, compareVoltageToLimits, gpio}),
+ checkStatusVout, compareVoltageToLimit, gpio}),
std::invalid_argument);
}
}
@@ -138,14 +138,14 @@
std::optional<uint8_t> page{};
bool isPowerSupplyRail{false};
bool checkStatusVout{false};
- bool compareVoltageToLimits{false};
+ bool compareVoltageToLimit{false};
std::optional<GPIO> gpio{};
Rail rail{name,
presence,
page,
isPowerSupplyRail,
checkStatusVout,
- compareVoltageToLimits,
+ compareVoltageToLimit,
gpio};
EXPECT_EQ(rail.getName(), "VDD2");
@@ -157,7 +157,7 @@
std::optional<uint8_t> page{};
bool isPowerSupplyRail{false};
bool checkStatusVout{false};
- bool compareVoltageToLimits{false};
+ bool compareVoltageToLimit{false};
std::optional<GPIO> gpio{};
// Test where presence has no value
@@ -168,7 +168,7 @@
page,
isPowerSupplyRail,
checkStatusVout,
- compareVoltageToLimits,
+ compareVoltageToLimit,
gpio};
EXPECT_FALSE(rail.getPresence().has_value());
}
@@ -182,7 +182,7 @@
page,
isPowerSupplyRail,
checkStatusVout,
- compareVoltageToLimits,
+ compareVoltageToLimit,
gpio};
EXPECT_TRUE(rail.getPresence().has_value());
EXPECT_EQ(
@@ -197,7 +197,7 @@
std::optional<std::string> presence{};
bool isPowerSupplyRail{false};
bool checkStatusVout{false};
- bool compareVoltageToLimits{false};
+ bool compareVoltageToLimit{false};
std::optional<GPIO> gpio{};
// Test where page has no value
@@ -208,7 +208,7 @@
page,
isPowerSupplyRail,
checkStatusVout,
- compareVoltageToLimits,
+ compareVoltageToLimit,
gpio};
EXPECT_FALSE(rail.getPage().has_value());
}
@@ -221,7 +221,7 @@
page,
isPowerSupplyRail,
checkStatusVout,
- compareVoltageToLimits,
+ compareVoltageToLimit,
gpio};
EXPECT_TRUE(rail.getPage().has_value());
EXPECT_EQ(rail.getPage().value(), 7);
@@ -235,14 +235,14 @@
std::optional<uint8_t> page{};
bool isPowerSupplyRail{true};
bool checkStatusVout{false};
- bool compareVoltageToLimits{false};
+ bool compareVoltageToLimit{false};
std::optional<GPIO> gpio{};
Rail rail{name,
presence,
page,
isPowerSupplyRail,
checkStatusVout,
- compareVoltageToLimits,
+ compareVoltageToLimit,
gpio};
EXPECT_TRUE(rail.isPowerSupplyRail());
@@ -255,37 +255,37 @@
std::optional<uint8_t> page{};
bool isPowerSupplyRail{false};
bool checkStatusVout{false};
- bool compareVoltageToLimits{false};
+ bool compareVoltageToLimit{false};
std::optional<GPIO> gpio{};
Rail rail{name,
presence,
page,
isPowerSupplyRail,
checkStatusVout,
- compareVoltageToLimits,
+ compareVoltageToLimit,
gpio};
EXPECT_FALSE(rail.getCheckStatusVout());
}
-TEST(RailTests, GetCompareVoltageToLimits)
+TEST(RailTests, GetCompareVoltageToLimit)
{
std::string name{"VDD2"};
std::optional<std::string> presence{};
std::optional<uint8_t> page{13};
bool isPowerSupplyRail{false};
bool checkStatusVout{false};
- bool compareVoltageToLimits{true};
+ bool compareVoltageToLimit{true};
std::optional<GPIO> gpio{};
Rail rail{name,
presence,
page,
isPowerSupplyRail,
checkStatusVout,
- compareVoltageToLimits,
+ compareVoltageToLimit,
gpio};
- EXPECT_TRUE(rail.getCompareVoltageToLimits());
+ EXPECT_TRUE(rail.getCompareVoltageToLimit());
}
TEST(RailTests, GetGPIO)
@@ -295,7 +295,7 @@
std::optional<uint8_t> page{};
bool isPowerSupplyRail{false};
bool checkStatusVout{false};
- bool compareVoltageToLimits{false};
+ bool compareVoltageToLimit{false};
// Test where gpio has no value
{
@@ -305,7 +305,7 @@
page,
isPowerSupplyRail,
checkStatusVout,
- compareVoltageToLimits,
+ compareVoltageToLimit,
gpio};
EXPECT_FALSE(rail.getGPIO().has_value());
}
@@ -318,7 +318,7 @@
page,
isPowerSupplyRail,
checkStatusVout,
- compareVoltageToLimits,
+ compareVoltageToLimit,
gpio};
EXPECT_TRUE(rail.getGPIO().has_value());
EXPECT_EQ(rail.getGPIO().value().line, 12);