psu-ng: Continue reading after readFail
Update the PMBus::read() function to allow for not creating journal
trace and elog, but default to continuing to trace and elog, the
previous behavior.
If we reach the limit of read failures that results in a communication
error log, continue to read, but stop logging failures.
If communication restores, we may be able to detect what caused the read
failure, or otherwise detect or clear new faults.
Change-Id: If59b86211ab54c31248ede78f8f117b607298923
Signed-off-by: Brandon Wyman <bjwyman@gmail.com>
diff --git a/.gitignore b/.gitignore
index 8c94d06..9800e6b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,2 +1,3 @@
build/
*.sw*
+tags
diff --git a/phosphor-power-supply/power_supply.cpp b/phosphor-power-supply/power_supply.cpp
index cc846ac..10f1aca 100644
--- a/phosphor-power-supply/power_supply.cpp
+++ b/phosphor-power-supply/power_supply.cpp
@@ -475,11 +475,12 @@
updatePresenceGPIO();
}
- if ((present) && (readFail < LOG_LIMIT))
+ if (present)
{
try
{
- statusWord = pmbusIntf->read(STATUS_WORD, Type::Debug);
+ statusWord = pmbusIntf->read(STATUS_WORD, Type::Debug,
+ (readFail < LOG_LIMIT));
// Read worked, reset the fail count.
readFail = 0;
@@ -565,8 +566,14 @@
}
catch (const ReadFailure& e)
{
- readFail++;
- phosphor::logging::commit<ReadFailure>();
+ if (readFail < SIZE_MAX)
+ {
+ readFail++;
+ }
+ if (readFail == LOG_LIMIT)
+ {
+ phosphor::logging::commit<ReadFailure>();
+ }
}
}
}
diff --git a/phosphor-power-supply/test/mock.hpp b/phosphor-power-supply/test/mock.hpp
index 25f48c1..cf4d3ae 100644
--- a/phosphor-power-supply/test/mock.hpp
+++ b/phosphor-power-supply/test/mock.hpp
@@ -19,7 +19,8 @@
public:
virtual ~MockedPMBus() = default;
- MOCK_METHOD(uint64_t, read, (const std::string& name, Type type),
+ MOCK_METHOD(uint64_t, read,
+ (const std::string& name, Type type, bool errTrace),
(override));
MOCK_METHOD(std::string, readString, (const std::string& name, Type type),
(override));
diff --git a/phosphor-power-supply/test/power_supply_tests.cpp b/phosphor-power-supply/test/power_supply_tests.cpp
index 22c15e2..c556f82 100644
--- a/phosphor-power-supply/test/power_supply_tests.cpp
+++ b/phosphor-power-supply/test/power_supply_tests.cpp
@@ -38,7 +38,7 @@
void setPMBusExpectations(MockedPMBus& mockPMBus,
const PMBusExpectations& expectations)
{
- EXPECT_CALL(mockPMBus, read(STATUS_WORD, _))
+ EXPECT_CALL(mockPMBus, read(STATUS_WORD, _, _))
.Times(1)
.WillOnce(Return(expectations.statusWordValue));
@@ -47,29 +47,29 @@
// If fault bits are on in STATUS_WORD, there will also be a read of
// STATUS_INPUT, STATUS_MFR, STATUS_CML, STATUS_VOUT (page 0), and
// STATUS_TEMPERATURE.
- EXPECT_CALL(mockPMBus, read(STATUS_INPUT, _))
+ EXPECT_CALL(mockPMBus, read(STATUS_INPUT, _, _))
.Times(1)
.WillOnce(Return(expectations.statusInputValue));
- EXPECT_CALL(mockPMBus, read(STATUS_MFR, _))
+ EXPECT_CALL(mockPMBus, read(STATUS_MFR, _, _))
.Times(1)
.WillOnce(Return(expectations.statusMFRValue));
- EXPECT_CALL(mockPMBus, read(STATUS_CML, _))
+ EXPECT_CALL(mockPMBus, read(STATUS_CML, _, _))
.Times(1)
.WillOnce(Return(expectations.statusCMLValue));
// Page will need to be set to 0 to read STATUS_VOUT.
EXPECT_CALL(mockPMBus, insertPageNum(STATUS_VOUT, 0))
.Times(1)
.WillOnce(Return("status0_vout"));
- EXPECT_CALL(mockPMBus, read("status0_vout", _))
+ EXPECT_CALL(mockPMBus, read("status0_vout", _, _))
.Times(1)
.WillOnce(Return(expectations.statusVOUTValue));
- EXPECT_CALL(mockPMBus, read(STATUS_IOUT, _))
+ EXPECT_CALL(mockPMBus, read(STATUS_IOUT, _, _))
.Times(1)
.WillOnce(Return(expectations.statusIOUTValue));
- EXPECT_CALL(mockPMBus, read(STATUS_FANS_1_2, _))
+ EXPECT_CALL(mockPMBus, read(STATUS_FANS_1_2, _, _))
.Times(1)
.WillOnce(Return(expectations.statusFans12Value));
- EXPECT_CALL(mockPMBus, read(STATUS_TEMPERATURE, _))
+ EXPECT_CALL(mockPMBus, read(STATUS_TEMPERATURE, _, _))
.Times(1)
.WillOnce(Return(expectations.statusTempValue));
}
@@ -107,7 +107,7 @@
// voltage to the current reading, triggering another clearing of faults
// due to below minimum to within range voltage.
// This READ_VIN for CLEAR_FAULTS does not check the returned value.
- EXPECT_CALL(pmbus, read(READ_VIN, _))
+ EXPECT_CALL(pmbus, read(READ_VIN, _, _))
.Times(2)
.WillOnce(Return(1))
.WillOnce(Return(2));
@@ -311,7 +311,7 @@
}
}
- EXPECT_CALL(mockPMBus, read(READ_VIN, _)).Times(1).WillOnce(Return(1));
+ EXPECT_CALL(mockPMBus, read(READ_VIN, _, _)).Times(1).WillOnce(Return(1));
psu2.clearFaults();
// STATUS_WORD INPUT/UV fault.
@@ -364,7 +364,9 @@
.Times(1)
.WillOnce(Return("209000"));
// The call for CLEAR_FAULTS command
- EXPECT_CALL(mockPMBus, read(READ_VIN, _)).Times(1).WillOnce(Return(3));
+ EXPECT_CALL(mockPMBus, read(READ_VIN, _, _))
+ .Times(1)
+ .WillOnce(Return(3));
psu2.analyze();
// Should remain present, no longer be faulted, no input fault, no
// VIN_UV fault. Nothing else should change.
@@ -374,7 +376,7 @@
EXPECT_EQ(psu2.hasVINUVFault(), false);
}
- EXPECT_CALL(mockPMBus, read(READ_VIN, _)).Times(1).WillOnce(Return(1));
+ EXPECT_CALL(mockPMBus, read(READ_VIN, _, _)).Times(1).WillOnce(Return(1));
psu2.clearFaults();
// STATUS_WORD MFR fault.
@@ -416,7 +418,7 @@
}
}
- EXPECT_CALL(mockPMBus, read(READ_VIN, _)).Times(1).WillOnce(Return(1));
+ EXPECT_CALL(mockPMBus, read(READ_VIN, _, _)).Times(1).WillOnce(Return(1));
psu2.clearFaults();
// Temperature fault.
{
@@ -456,7 +458,7 @@
}
}
- EXPECT_CALL(mockPMBus, read(READ_VIN, _)).Times(1).WillOnce(Return(1));
+ EXPECT_CALL(mockPMBus, read(READ_VIN, _, _)).Times(1).WillOnce(Return(1));
psu2.clearFaults();
// CML fault
{
@@ -496,7 +498,7 @@
}
}
- EXPECT_CALL(mockPMBus, read(READ_VIN, _)).Times(1).WillOnce(Return(1));
+ EXPECT_CALL(mockPMBus, read(READ_VIN, _, _)).Times(1).WillOnce(Return(1));
psu2.clearFaults();
// VOUT_OV_FAULT fault
{
@@ -685,14 +687,7 @@
EXPECT_EQ(psu2.hasIoutOCFault(), false);
EXPECT_EQ(psu2.hasFanFault(), false);
EXPECT_EQ(psu2.hasTempFault(), false);
- if (x < DEGLITCH_LIMIT)
- {
- EXPECT_EQ(psu2.hasPgoodFault(), false);
- }
- else
- {
- EXPECT_EQ(psu2.hasPgoodFault(), true);
- }
+ EXPECT_EQ(psu2.hasPgoodFault(), x >= DEGLITCH_LIMIT);
}
}
@@ -830,7 +825,9 @@
EXPECT_EQ(psu.hasPSCS12VFault(), x >= DEGLITCH_LIMIT);
}
- EXPECT_CALL(mockPMBus, read(READ_VIN, _)).Times(1).WillOnce(Return(207000));
+ EXPECT_CALL(mockPMBus, read(READ_VIN, _, _))
+ .Times(1)
+ .WillOnce(Return(207000));
psu.clearFaults();
EXPECT_EQ(psu.isPresent(), true);
EXPECT_EQ(psu.isFaulted(), false);
@@ -904,7 +901,7 @@
EXPECT_CALL(mockPMBus, readString(READ_VIN, _))
.Times(1)
.WillOnce(Return("206000"));
- EXPECT_CALL(mockPMBus, read(READ_VIN, _)).Times(1).WillOnce(Return(0));
+ EXPECT_CALL(mockPMBus, read(READ_VIN, _, _)).Times(1).WillOnce(Return(0));
psu.analyze();
EXPECT_EQ(psu.isPresent(), true);
EXPECT_EQ(psu.isFaulted(), false);
@@ -1198,7 +1195,7 @@
.Times(1)
.WillOnce(Return("201300"));
// Went from below minimum to within range, expect CLEAR_FAULTS.
- EXPECT_CALL(mockPMBus, read(READ_VIN, _)).Times(1).WillOnce(Return(3));
+ EXPECT_CALL(mockPMBus, read(READ_VIN, _, _)).Times(1).WillOnce(Return(3));
psu.analyze();
EXPECT_EQ(psu.hasVINUVFault(), false);
}
diff --git a/pmbus.cpp b/pmbus.cpp
index 5483ca4..6ab6944 100644
--- a/pmbus.cpp
+++ b/pmbus.cpp
@@ -174,7 +174,7 @@
return fs::exists(path);
}
-uint64_t PMBus::read(const std::string& name, Type type)
+uint64_t PMBus::read(const std::string& name, Type type, bool errTrace)
{
uint64_t data = 0;
std::ifstream file;
@@ -192,16 +192,24 @@
catch (const std::exception& e)
{
auto rc = errno;
- log<level::ERR>((std::string("Failed to read sysfs file "
- "errno=") +
- std::to_string(rc) + " FILENAME=" + path.string())
- .c_str());
- using metadata = xyz::openbmc_project::Common::Device::ReadFailure;
+ if (errTrace)
+ {
+ log<level::ERR>((std::string("Failed to read sysfs file "
+ "errno=") +
+ std::to_string(rc) + " FILENAME=" + path.string())
+ .c_str());
- elog<ReadFailure>(
- metadata::CALLOUT_ERRNO(rc),
- metadata::CALLOUT_DEVICE_PATH(fs::canonical(basePath).c_str()));
+ using metadata = xyz::openbmc_project::Common::Device::ReadFailure;
+
+ elog<ReadFailure>(
+ metadata::CALLOUT_ERRNO(rc),
+ metadata::CALLOUT_DEVICE_PATH(fs::canonical(basePath).c_str()));
+ }
+ else
+ {
+ throw ReadFailure();
+ }
}
return data;
diff --git a/pmbus.hpp b/pmbus.hpp
index 4fd2880..d0fe747 100644
--- a/pmbus.hpp
+++ b/pmbus.hpp
@@ -165,7 +165,8 @@
public:
virtual ~PMBusBase() = default;
- virtual uint64_t read(const std::string& name, Type type) = 0;
+ virtual uint64_t read(const std::string& name, Type type,
+ bool errTrace = true) = 0;
virtual std::string readString(const std::string& name, Type type) = 0;
virtual void writeBinary(const std::string& name, std::vector<uint8_t> data,
Type type) = 0;
@@ -285,10 +286,12 @@
*
* @param[in] name - path concatenated to basePath to read
* @param[in] type - Path type
+ * @param[in] errTrace - true to enable tracing error (defaults to true)
*
* @return uint64_t - Up to 8 bytes of data read from file.
*/
- uint64_t read(const std::string& name, Type type) override;
+ uint64_t read(const std::string& name, Type type,
+ bool errTrace = true) override;
/**
* Read a string from file in sysfs.