Capture PSU STATUS_MFR_SPECIFIC during analysis
If there is a fault indicated in STATUS_WORD, add the
STATUS_MFR_SPECIFIC value to the error log additional data.
For additional diagnostic information, include the MFR status
for all log entries when status word is nonzero.
Change the journal entry for MFR fault to an error type,
suggested in code review.
Tested: Test by setting the bit in the PMBus status word in simulation
for MFR and a nonzero value in STATUS_MFR_SPECIFIC.
The chassis is on during this test.
This change also adds the MFR status in journal entries.
The journal contains an entry like this for the MFR fault:
MFR fault: status word = 0x01 MFR fault = 0x1
Other journal entries:
VIN_UV fault: status word = 0x08, MFR fault = 0x1
INPUT fault: status word = 0x02, MFR fault = 0x1
Note: definitions for INPUT_FAULT_WARN and MFR_SPECIFIC_FAULT
were changed for testing purposes because the simulator could
not set the upper bits of the status word.
Signed-off-by: Jay Meyer <jaymeyer@us.ibm.com>
Change-Id: Ib63ca6581c72f640aba01a95c6fe02b26ac8c1ee
diff --git a/phosphor-power-supply/power_supply.cpp b/phosphor-power-supply/power_supply.cpp
index 9c3af70..85248a5 100644
--- a/phosphor-power-supply/power_supply.cpp
+++ b/phosphor-power-supply/power_supply.cpp
@@ -50,14 +50,16 @@
if (statusWord)
{
+ statusMFR = pmbusIntf->read(STATUS_MFR, Type::Debug);
if (statusWord & status_word::INPUT_FAULT_WARN)
{
if (!inputFault)
{
- log<level::INFO>(
- "INPUT fault",
- entry("STATUS_WORD=0x%04X",
- static_cast<uint16_t>(statusWord)));
+ log<level::INFO>(fmt::format("INPUT fault: "
+ "status word = {:#04x}, "
+ "MFR fault = {:#02x}",
+ statusWord, statusMFR)
+ .c_str());
}
faultFound = true;
@@ -68,10 +70,11 @@
{
if (!mfrFault)
{
- log<level::INFO>(
- "MFRSPECIFIC fault",
- entry("STATUS_WORD=0x%04X",
- static_cast<uint16_t>(statusWord)));
+ log<level::ERR>(fmt::format("MFR fault: "
+ "status word = {:#04x} "
+ "MFR fault = {:#02x}",
+ statusWord, statusMFR)
+ .c_str());
}
faultFound = true;
mfrFault = true;
@@ -81,10 +84,11 @@
{
if (!vinUVFault)
{
- log<level::INFO>(
- "VIN_UV fault",
- entry("STATUS_WORD=0x%04X",
- static_cast<uint16_t>(statusWord)));
+ log<level::INFO>(fmt::format("VIN_UV fault: "
+ "status word = {:#04x}, "
+ "MFR fault = {:#02x}",
+ statusWord, statusMFR)
+ .c_str());
}
faultFound = true;
@@ -144,6 +148,7 @@
faultFound = false;
inputFault = false;
mfrFault = false;
+ statusMFR = 0;
vinUVFault = false;
readFail = 0;
faultLogged = false;
diff --git a/phosphor-power-supply/power_supply.hpp b/phosphor-power-supply/power_supply.hpp
index a73e14e..2aae6d6 100644
--- a/phosphor-power-supply/power_supply.hpp
+++ b/phosphor-power-supply/power_supply.hpp
@@ -146,6 +146,14 @@
}
/**
+ * @brief Returns the last read value from STATUS_MFR.
+ */
+ uint64_t getMFRFault() const
+ {
+ return statusMFR;
+ }
+
+ /**
* @brief Returns true if a fault was found.
*/
bool isFaulted() const
@@ -239,6 +247,9 @@
/** @brief Will be updated to the latest/lastvalue read from STATUS_WORD.*/
uint64_t statusWord = 0;
+ /** @brief Will be updated to the latest/lastvalue read from STATUS_MFR.*/
+ uint64_t statusMFR = 0;
+
/** @brief True if a fault has already been found and not cleared */
bool faultFound = false;
diff --git a/phosphor-power-supply/psu_manager.cpp b/phosphor-power-supply/psu_manager.cpp
index 4a42747..b83d77d 100644
--- a/phosphor-power-supply/psu_manager.cpp
+++ b/phosphor-power-supply/psu_manager.cpp
@@ -196,6 +196,8 @@
{
additionalData["STATUS_WORD"] =
std::to_string(psu->getStatusWord());
+ additionalData["STATUS_MFR"] =
+ std::to_string(psu->getMFRFault());
// If there are faults being reported, they possibly could be
// related to a bug in the firmware version running on the power
// supply. Capture that data into the error as well.
diff --git a/phosphor-power-supply/test/power_supply_tests.cpp b/phosphor-power-supply/test/power_supply_tests.cpp
index 55dc2a7..07b4b9e 100644
--- a/phosphor-power-supply/test/power_supply_tests.cpp
+++ b/phosphor-power-supply/test/power_supply_tests.cpp
@@ -116,8 +116,9 @@
// STATUS_WORD input fault/warn
EXPECT_CALL(mockPMBus, read(_, _))
- .Times(1)
- .WillOnce(Return(status_word::INPUT_FAULT_WARN));
+ .Times(2)
+ .WillOnce(Return(status_word::INPUT_FAULT_WARN))
+ .WillOnce(Return(0x0000));
psu2.analyze();
EXPECT_EQ(psu2.isPresent(), true);
EXPECT_EQ(psu2.isFaulted(), true);
@@ -129,7 +130,8 @@
// First need it to return good status, then the fault
EXPECT_CALL(mockPMBus, read(_, _))
.WillOnce(Return(0x0000))
- .WillOnce(Return(status_word::VIN_UV_FAULT));
+ .WillOnce(Return(status_word::VIN_UV_FAULT))
+ .WillOnce(Return(0x0000));
psu2.analyze();
psu2.analyze();
EXPECT_EQ(psu2.isPresent(), true);
@@ -141,7 +143,8 @@
// STATUS_WORD MFR fault.
EXPECT_CALL(mockPMBus, read(_, _))
.WillOnce(Return(0x0000))
- .WillOnce(Return(status_word::MFR_SPECIFIC_FAULT));
+ .WillOnce(Return(status_word::MFR_SPECIFIC_FAULT))
+ .WillOnce(Return(1)); // mock return for read(STATUS_MFR... )
psu2.analyze();
psu2.analyze();
EXPECT_EQ(psu2.isPresent(), true);
@@ -153,7 +156,8 @@
// Ignore Temperature fault.
EXPECT_CALL(mockPMBus, read(_, _))
.WillOnce(Return(0x0000))
- .WillOnce(Return(status_word::TEMPERATURE_FAULT_WARN));
+ .WillOnce(Return(status_word::TEMPERATURE_FAULT_WARN))
+ .WillOnce(Return(0x0000));
psu2.analyze();
psu2.analyze();
EXPECT_EQ(psu2.isPresent(), true);
@@ -165,7 +169,8 @@
// Ignore fan fault
EXPECT_CALL(mockPMBus, read(_, _))
.WillOnce(Return(0x0000))
- .WillOnce(Return(status_word::FAN_FAULT));
+ .WillOnce(Return(status_word::FAN_FAULT))
+ .WillOnce(Return(0x0000));
psu2.analyze();
psu2.analyze();
EXPECT_EQ(psu2.isPresent(), true);
@@ -230,7 +235,10 @@
EXPECT_EQ(psu.hasMFRFault(), false);
EXPECT_EQ(psu.hasVINUVFault(), false);
MockedPMBus& mockPMBus = static_cast<MockedPMBus&>(psu.getPMBus());
- EXPECT_CALL(mockPMBus, read(_, _)).Times(1).WillOnce(Return(0xFFFF));
+ EXPECT_CALL(mockPMBus, read(_, _))
+ .Times(2)
+ .WillOnce(Return(0xFFFF))
+ .WillOnce(Return(1)); // mock return for read(STATUS_MFR... )
psu.analyze();
EXPECT_EQ(psu.isPresent(), true);
EXPECT_EQ(psu.isFaulted(), true);
@@ -315,7 +323,10 @@
PowerSupply psu{bus, PSUInventoryPath, 11, "006f"};
EXPECT_EQ(psu.isFaulted(), false);
MockedPMBus& mockPMBus = static_cast<MockedPMBus&>(psu.getPMBus());
- EXPECT_CALL(mockPMBus, read(_, _)).Times(1).WillOnce(Return(0xFFFF));
+ EXPECT_CALL(mockPMBus, read(_, _))
+ .Times(2)
+ .WillOnce(Return(0xFFFF))
+ .WillOnce(Return(1)); // mock return for read(STATUS_MFR... )
psu.analyze();
EXPECT_EQ(psu.isFaulted(), true);
}
@@ -332,8 +343,9 @@
psu.analyze();
EXPECT_EQ(psu.hasInputFault(), false);
EXPECT_CALL(mockPMBus, read(_, _))
- .Times(1)
- .WillOnce(Return(status_word::INPUT_FAULT_WARN));
+ .Times(2)
+ .WillOnce(Return(status_word::INPUT_FAULT_WARN))
+ .WillOnce(Return(0));
psu.analyze();
EXPECT_EQ(psu.hasInputFault(), true);
EXPECT_CALL(mockPMBus, read(_, _)).Times(1).WillOnce(Return(0x0000));
@@ -353,8 +365,9 @@
psu.analyze();
EXPECT_EQ(psu.hasMFRFault(), false);
EXPECT_CALL(mockPMBus, read(_, _))
- .Times(1)
- .WillOnce(Return(status_word::MFR_SPECIFIC_FAULT));
+ .Times(2)
+ .WillOnce(Return(status_word::MFR_SPECIFIC_FAULT))
+ .WillOnce(Return(1)); // mock return for read(STATUS_MFR... )
psu.analyze();
EXPECT_EQ(psu.hasMFRFault(), true);
EXPECT_CALL(mockPMBus, read(_, _)).Times(1).WillOnce(Return(0x0000));
@@ -374,8 +387,9 @@
psu.analyze();
EXPECT_EQ(psu.hasVINUVFault(), false);
EXPECT_CALL(mockPMBus, read(_, _))
- .Times(1)
- .WillOnce(Return(status_word::VIN_UV_FAULT));
+ .Times(2)
+ .WillOnce(Return(status_word::VIN_UV_FAULT))
+ .WillOnce(Return(0));
psu.analyze();
EXPECT_EQ(psu.hasVINUVFault(), true);
EXPECT_CALL(mockPMBus, read(_, _)).Times(1).WillOnce(Return(0x0000));