Refactor the analyze() function to be less verbose
Split up the various fault checking sections of code to be there own
separate functions for checking the various fault bits/conditions and
logging of appropriate error with necessary metadata for debug.
Change-Id: I65e54f53b0089d82852bee398ebb7d2303f29b6c
Signed-off-by: Brandon Wyman <bjwyman@gmail.com>
diff --git a/power-supply/power_supply.cpp b/power-supply/power_supply.cpp
index cd7f87f..90f33ae 100644
--- a/power-supply/power_supply.cpp
+++ b/power-supply/power_supply.cpp
@@ -91,7 +91,6 @@
if (present)
{
std::uint16_t statusWord = 0;
- std::uint8_t statusInput = 0;
// Read the 2 byte STATUS_WORD value to check for faults.
statusWord = pmbusIntf.read(STATUS_WORD, Type::Debug);
@@ -103,127 +102,12 @@
// If count reaches 3, we have fault. If count reaches 0, fault is
// cleared.
- if ((statusWord & status_word::VIN_UV_FAULT) && !vinUVFault)
- {
- util::NamesValues nv;
- nv.add("STATUS_WORD", statusWord);
-
- using metadata = xyz::openbmc_project::Power::Fault::
- PowerSupplyUnderVoltageFault;
-
- report<PowerSupplyUnderVoltageFault>(
- metadata::RAW_STATUS(nv.get().c_str()));
-
- vinUVFault = true;
- }
- else
- {
- vinUVFault = false;
- log<level::INFO>("VIN_UV_FAULT cleared",
- entry("POWERSUPPLY=%s",
- inventoryPath.c_str()));
- }
-
- if ((statusWord & status_word::INPUT_FAULT_WARN) && !inputFault)
- {
- inputFault = true;
-
- statusInput = pmbusIntf.read(STATUS_INPUT, Type::Debug);
-
- util::NamesValues nv;
- nv.add("STATUS_WORD", statusWord);
- nv.add("STATUS_INPUT", statusInput);
-
- using metadata = xyz::openbmc_project::Power::Fault::
- PowerSupplyInputFault;
-
- report<PowerSupplyInputFault>(metadata::RAW_STATUS(
- nv.get().c_str()));
- }
- else
- {
- if ((inputFault) &&
- !(statusWord & status_word::INPUT_FAULT_WARN))
- {
- inputFault = false;
-
- statusInput = pmbusIntf.read(STATUS_INPUT, Type::Debug);
-
- log<level::INFO>("INPUT_FAULT_WARN cleared",
- entry("POWERSUPPLY=%s",
- inventoryPath.c_str()),
- entry("STATUS_WORD=0x%04X", statusWord),
- entry("STATUS_INPUT=0x%02X", statusInput));
- }
- }
+ checkInputFault(statusWord);
if (powerOn)
{
- std::uint8_t statusVout = 0;
- std::uint8_t statusIout = 0;
- std::uint8_t statusMFR = 0;
-
- // Check PG# and UNIT_IS_OFF
- if (((statusWord & status_word::POWER_GOOD_NEGATED) ||
- (statusWord & status_word::UNIT_IS_OFF)) &&
- !powerOnFault)
- {
- std::uint8_t statusVout = 0;
- std::uint8_t statusIout = 0;
- std::uint8_t statusMFR = 0;
-
- statusInput = pmbusIntf.read(STATUS_INPUT, Type::Debug);
- auto status0Vout = pmbusIntf.insertPageNum(STATUS_VOUT, 0);
- statusVout = pmbusIntf.read(status0Vout, Type::Debug);
- statusIout = pmbusIntf.read(STATUS_IOUT, Type::Debug);
- statusMFR = pmbusIntf.read(STATUS_MFR, Type::Debug);
-
- util::NamesValues nv;
- nv.add("STATUS_WORD", statusWord);
- nv.add("STATUS_INPUT", statusInput);
- nv.add("STATUS_VOUT", statusVout);
- nv.add("STATUS_IOUT", statusIout);
- nv.add("MFR_SPECIFIC", statusMFR);
-
- using metadata = xyz::openbmc_project::Power::Fault::
- PowerSupplyShouldBeOn;
-
- // A power supply is OFF (or pgood low) but should be on.
- report<PowerSupplyShouldBeOn>(
- metadata::RAW_STATUS(nv.get().c_str()),
- metadata::CALLOUT_INVENTORY_PATH(
- inventoryPath.c_str()));
-
- powerOnFault = true;
- }
-
- // Check for an output overcurrent fault.
- if ((statusWord & status_word::IOUT_OC_FAULT) &&
- !outputOCFault)
- {
- statusInput = pmbusIntf.read(STATUS_INPUT, Type::Debug);
- statusVout = pmbusIntf.read(STATUS_VOUT, Type::Debug);
- statusIout = pmbusIntf.read(STATUS_IOUT, Type::Debug);
- statusMFR = pmbusIntf.read(STATUS_MFR, Type::Debug);
-
- util::NamesValues nv;
- nv.add("STATUS_WORD", statusWord);
- nv.add("STATUS_INPUT", statusInput);
- nv.add("STATUS_VOUT", statusVout);
- nv.add("STATUS_IOUT", statusIout);
- nv.add("MFR_SPECIFIC", statusMFR);
-
- using metadata = xyz::openbmc_project::Power::Fault::
- PowerSupplyOutputOvercurrent;
-
- report<PowerSupplyOutputOvercurrent>(
- metadata::RAW_STATUS(nv.get().c_str()),
- metadata::CALLOUT_INVENTORY_PATH(
- inventoryPath.c_str()));
-
- outputOCFault = true;
- }
-
+ checkPGOrUnitOffFault(statusWord);
+ checkCurrentOutOverCurrentFault(statusWord);
}
}
}
@@ -233,8 +117,6 @@
{
commit<ReadFailure>();
readFailLogged = true;
- // TODO - Need to reset that to false at start of power on, or
- // presence change.
}
}
@@ -356,6 +238,146 @@
}
+void PowerSupply::checkInputFault(const uint16_t statusWord)
+{
+ using namespace witherspoon::pmbus;
+
+ std::uint8_t statusInput = 0;
+
+ if ((statusWord & status_word::VIN_UV_FAULT) && !vinUVFault)
+ {
+ vinUVFault = true;
+
+ util::NamesValues nv;
+ nv.add("STATUS_WORD", statusWord);
+
+ using metadata = xyz::openbmc_project::Power::Fault::
+ PowerSupplyUnderVoltageFault;
+
+ report<PowerSupplyUnderVoltageFault>(metadata::RAW_STATUS(
+ nv.get().c_str()));
+ }
+ else
+ {
+ if (vinUVFault)
+ {
+ vinUVFault = false;
+ log<level::INFO>("VIN_UV_FAULT cleared",
+ entry("POWERSUPPLY=%s",
+ inventoryPath.c_str()));
+ }
+ }
+
+ if ((statusWord & status_word::INPUT_FAULT_WARN) && !inputFault)
+ {
+ inputFault = true;
+
+ statusInput = pmbusIntf.read(STATUS_INPUT, Type::Debug);
+
+ util::NamesValues nv;
+ nv.add("STATUS_WORD", statusWord);
+ nv.add("STATUS_INPUT", statusInput);
+
+ using metadata = xyz::openbmc_project::Power::Fault::
+ PowerSupplyInputFault;
+
+ report<PowerSupplyInputFault>(
+ metadata::RAW_STATUS(nv.get().c_str()));
+ }
+ else
+ {
+ if ((inputFault) &&
+ !(statusWord & status_word::INPUT_FAULT_WARN))
+ {
+ inputFault = false;
+ statusInput = pmbusIntf.read(STATUS_INPUT, Type::Debug);
+
+ log<level::INFO>("INPUT_FAULT_WARN cleared",
+ entry("POWERSUPPLY=%s", inventoryPath.c_str()),
+ entry("STATUS_WORD=0x%04X", statusWord),
+ entry("STATUS_INPUT=0x%02X", statusInput));
+ }
+ }
+}
+
+void PowerSupply::checkPGOrUnitOffFault(const uint16_t statusWord)
+{
+ using namespace witherspoon::pmbus;
+
+ std::uint8_t statusInput = 0;
+ std::uint8_t statusVout = 0;
+ std::uint8_t statusIout = 0;
+ std::uint8_t statusMFR = 0;
+
+ // Check PG# and UNIT_IS_OFF
+ if (((statusWord & status_word::POWER_GOOD_NEGATED) ||
+ (statusWord & status_word::UNIT_IS_OFF)) &&
+ !powerOnFault)
+ {
+ statusInput = pmbusIntf.read(STATUS_INPUT, Type::Debug);
+ auto status0Vout = pmbusIntf.insertPageNum(STATUS_VOUT, 0);
+ statusVout = pmbusIntf.read(status0Vout, Type::Debug);
+ statusIout = pmbusIntf.read(STATUS_IOUT, Type::Debug);
+ statusMFR = pmbusIntf.read(STATUS_MFR, Type::Debug);
+
+ util::NamesValues nv;
+ nv.add("STATUS_WORD", statusWord);
+ nv.add("STATUS_INPUT", statusInput);
+ nv.add("STATUS_VOUT", statusVout);
+ nv.add("STATUS_IOUT", statusIout);
+ nv.add("MFR_SPECIFIC", statusMFR);
+
+ using metadata = xyz::openbmc_project::Power::Fault::
+ PowerSupplyShouldBeOn;
+
+ // A power supply is OFF (or pgood low) but should be on.
+ report<PowerSupplyShouldBeOn>(metadata::RAW_STATUS(nv.get().c_str()),
+ metadata::CALLOUT_INVENTORY_PATH(
+ inventoryPath.c_str()));
+
+ powerOnFault = true;
+ }
+
+}
+
+void PowerSupply::checkCurrentOutOverCurrentFault(const uint16_t statusWord)
+{
+ using namespace witherspoon::pmbus;
+
+ std::uint8_t statusInput = 0;
+ std::uint8_t statusVout = 0;
+ std::uint8_t statusIout = 0;
+ std::uint8_t statusMFR = 0;
+
+ // Check for an output overcurrent fault.
+ if ((statusWord & status_word::IOUT_OC_FAULT) &&
+ !outputOCFault)
+ {
+ statusInput = pmbusIntf.read(STATUS_INPUT, Type::Debug);
+ auto status0Vout = pmbusIntf.insertPageNum(STATUS_VOUT, 0);
+ statusVout = pmbusIntf.read(status0Vout, Type::Debug);
+ statusIout = pmbusIntf.read(STATUS_IOUT, Type::Debug);
+ statusMFR = pmbusIntf.read(STATUS_MFR, Type::Debug);
+
+ util::NamesValues nv;
+ nv.add("STATUS_WORD", statusWord);
+ nv.add("STATUS_INPUT", statusInput);
+ nv.add("STATUS_VOUT", statusVout);
+ nv.add("STATUS_IOUT", statusIout);
+ nv.add("MFR_SPECIFIC", statusMFR);
+
+ using metadata = xyz::openbmc_project::Power::Fault::
+ PowerSupplyOutputOvercurrent;
+
+ report<PowerSupplyOutputOvercurrent>(metadata::RAW_STATUS(
+ nv.get().c_str()),
+ metadata::CALLOUT_INVENTORY_PATH(
+ inventoryPath.c_str()));
+
+ outputOCFault = true;
+ }
+}
+
void PowerSupply::clearFaults()
{
//TODO - Clear faults at pre-poweron. openbmc/openbmc#1736
diff --git a/power-supply/power_supply.hpp b/power-supply/power_supply.hpp
index d47ee3c..ae03ee9 100644
--- a/power-supply/power_supply.hpp
+++ b/power-supply/power_supply.hpp
@@ -185,6 +185,32 @@
*/
void powerStateChanged(sdbusplus::message::message& msg);
+ /**
+ * @brief Checks for input voltage faults and logs error if needed.
+ *
+ * Check for voltage input under voltage fault (VIN_UV_FAULT) and/or
+ * input fault or warning (INPUT_FAULT), and logs appropriate error(s).
+ *
+ * @param[in] statusWord - 2 byte STATUS_WORD value read from sysfs
+ */
+ void checkInputFault(const uint16_t statusWord);
+
+ /**
+ * @brief Checks for power good negated or unit is off in wrong state
+ *
+ * @param[in] statusWord - 2 byte STATUS_WORD value read from sysfs
+ */
+ void checkPGOrUnitOffFault(const uint16_t statusWord);
+
+ /**
+ * @brief Checks for output current over current fault.
+ *
+ * IOUT_OC_FAULT is checked, if on, appropriate error is logged.
+ *
+ * @param[in] statusWord - 2 byte STATUS_WORD value read from sysfs
+ */
+ void checkCurrentOutOverCurrentFault(const uint16_t statusWord);
+
};
}