Fix PSU status when BMC is in standby
Without this patch:
Plug in the power supplies in any order, the power supply that is
plugged in first will report a `PSU_KILL_Fault` in standby, but the
problem will be restored after the host is powered on.
With this patch:
Regardless of whether it is in standby or host powered on, this
problem disappears and it works fine.
Signed-off-by: George Liu <liuxiwei@inspur.com>
Change-Id: I18e0e26a4922dd710e042048625da1cc8b08dd3c
diff --git a/phosphor-power-supply/power_supply.cpp b/phosphor-power-supply/power_supply.cpp
index c965892..9d884f3 100644
--- a/phosphor-power-supply/power_supply.cpp
+++ b/phosphor-power-supply/power_supply.cpp
@@ -34,9 +34,11 @@
PowerSupply::PowerSupply(sdbusplus::bus_t& bus, const std::string& invpath,
std::uint8_t i2cbus, std::uint16_t i2caddr,
const std::string& driver,
- const std::string& gpioLineName) :
+ const std::string& gpioLineName,
+ std::function<bool()>&& callback) :
bus(bus),
- inventoryPath(invpath), bindPath("/sys/bus/i2c/drivers/" + driver)
+ inventoryPath(invpath), bindPath("/sys/bus/i2c/drivers/" + driver),
+ isPowerOn(std::move(callback))
{
if (inventoryPath.empty())
{
@@ -1164,8 +1166,8 @@
void PowerSupply::checkAvailability()
{
bool origAvailability = available;
- available = present && !hasInputFault() && !hasVINUVFault() &&
- !hasPSKillFault() && !hasIoutOCFault();
+ bool faulted = isPowerOn() && (hasPSKillFault() || hasIoutOCFault());
+ available = present && !hasInputFault() && !hasVINUVFault() && !faulted;
if (origAvailability != available)
{
diff --git a/phosphor-power-supply/power_supply.hpp b/phosphor-power-supply/power_supply.hpp
index 2c61f13..d823e68 100644
--- a/phosphor-power-supply/power_supply.hpp
+++ b/phosphor-power-supply/power_supply.hpp
@@ -70,10 +70,12 @@
* @param[in] driver - i2c driver name for power supply
* @param[in] gpioLineName - The gpio-line-name to read for presence. See
* https://github.com/openbmc/docs/blob/master/designs/device-tree-gpio-naming.md
+ * @param[in] callback - Get the power on status of the psu manager class
*/
PowerSupply(sdbusplus::bus_t& bus, const std::string& invpath,
std::uint8_t i2cbus, const std::uint16_t i2caddr,
- const std::string& driver, const std::string& gpioLineName);
+ const std::string& driver, const std::string& gpioLineName,
+ std::function<bool()>&& callback);
phosphor::pmbus::PMBusBase& getPMBus()
{
@@ -972,6 +974,14 @@
void updateHistory();
/**
+ * @brief Get the power on status of the psu manager class.
+ *
+ * This is a callback method used to get the power on status of the psu
+ * manager class.
+ */
+ std::function<bool()> isPowerOn;
+
+ /**
* @brief Set to true if INPUT_HISTORY command supported.
*
* Not all power supplies will support the INPUT_HISTORY command. The IBM
diff --git a/phosphor-power-supply/psu_manager.cpp b/phosphor-power-supply/psu_manager.cpp
index 13d9356..68ff7fb 100644
--- a/phosphor-power-supply/psu_manager.cpp
+++ b/phosphor-power-supply/psu_manager.cpp
@@ -250,8 +250,11 @@
"make PowerSupply bus: {} addr: {} driver: {} presline: {}",
*i2cbus, *i2caddr, driver, presline)
.c_str());
- auto psu = std::make_unique<PowerSupply>(bus, invpath, *i2cbus,
- *i2caddr, driver, presline);
+ auto psu = std::make_unique<PowerSupply>(
+ bus, invpath, *i2cbus, *i2caddr, driver, presline,
+ std::bind(
+ std::mem_fn(&phosphor::power::manager::PSUManager::isPowerOn),
+ this));
psus.emplace_back(std::move(psu));
// Subscribe to power supply presence changes
diff --git a/phosphor-power-supply/psu_manager.hpp b/phosphor-power-supply/psu_manager.hpp
index 2ee61a6..0749185 100644
--- a/phosphor-power-supply/psu_manager.hpp
+++ b/phosphor-power-supply/psu_manager.hpp
@@ -134,6 +134,14 @@
}
}
+ /**
+ * Get the status of Power on.
+ */
+ bool isPowerOn()
+ {
+ return powerOn;
+ }
+
private:
/**
* The D-Bus object
diff --git a/phosphor-power-supply/test/power_supply_tests.cpp b/phosphor-power-supply/test/power_supply_tests.cpp
index 278326d..0417b5a 100644
--- a/phosphor-power-supply/test/power_supply_tests.cpp
+++ b/phosphor-power-supply/test/power_supply_tests.cpp
@@ -22,6 +22,7 @@
static auto PSUInventoryPath = "/xyz/bmc/inv/sys/chassis/board/powersupply0";
static auto PSUGPIOLineName = "presence-ps0";
+static auto isPowerOn = []() { return true; };
struct PMBusExpectations
{
@@ -134,7 +135,7 @@
try
{
auto psu = std::make_unique<PowerSupply>(bus, "", 3, 0x68, "ibm-cffps",
- PSUGPIOLineName);
+ PSUGPIOLineName, isPowerOn);
ADD_FAILURE() << "Should not have reached this line.";
}
catch (const std::invalid_argument& e)
@@ -152,7 +153,7 @@
try
{
auto psu = std::make_unique<PowerSupply>(bus, PSUInventoryPath, 3, 0x68,
- "ibm-cffps", "");
+ "ibm-cffps", "", isPowerOn);
ADD_FAILURE()
<< "Should not have reached this line. Invalid gpioLineName.";
}
@@ -170,7 +171,8 @@
try
{
auto psu = std::make_unique<PowerSupply>(bus, PSUInventoryPath, 3, 0x68,
- "ibm-cffps", PSUGPIOLineName);
+ "ibm-cffps", PSUGPIOLineName,
+ isPowerOn);
EXPECT_EQ(psu->isPresent(), false);
EXPECT_EQ(psu->isFaulted(), false);
@@ -218,8 +220,8 @@
// If I default to reading the GPIO, I will NOT expect a call to
// getPresence().
- PowerSupply psu{bus, PSUInventoryPath, 4,
- 0x69, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 4, 0x69,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
MockedGPIOInterface* mockPresenceGPIO =
static_cast<MockedGPIOInterface*>(psu.getPresenceGPIO());
EXPECT_CALL(*mockPresenceGPIO, read()).Times(1).WillOnce(Return(0));
@@ -243,8 +245,8 @@
EXPECT_EQ(psu.hasPSCS12VFault(), false);
}
- PowerSupply psu2{bus, PSUInventoryPath, 5,
- 0x6a, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu2{bus, PSUInventoryPath, 5, 0x6a,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
// In order to get the various faults tested, the power supply needs to
// be present in order to read from the PMBus device(s).
MockedGPIOInterface* mockPresenceGPIO2 =
@@ -733,8 +735,8 @@
{
// Assume GPIO presence, not inventory presence?
EXPECT_CALL(mockedUtil, setAvailable(_, _, _)).Times(0);
- PowerSupply psu{bus, PSUInventoryPath, 4,
- 0x69, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 4, 0x69,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
MockedGPIOInterface* mockPresenceGPIO =
static_cast<MockedGPIOInterface*>(psu.getPresenceGPIO());
@@ -753,8 +755,8 @@
{
// Assume GPIO presence, not inventory presence?
EXPECT_CALL(mockedUtil, setAvailable(_, _, true));
- PowerSupply psu{bus, PSUInventoryPath, 5,
- 0x6a, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 5, 0x6a,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
MockedGPIOInterface* mockPresenceGPIO =
static_cast<MockedGPIOInterface*>(psu.getPresenceGPIO());
// There will potentially be multiple calls, we want it to continue
@@ -789,8 +791,8 @@
TEST_F(PowerSupplyTests, ClearFaults)
{
auto bus = sdbusplus::bus::new_default();
- PowerSupply psu{bus, PSUInventoryPath, 13,
- 0x68, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 13, 0x68,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
MockedGPIOInterface* mockPresenceGPIO =
static_cast<MockedGPIOInterface*>(psu.getPresenceGPIO());
// Always return 1 to indicate present.
@@ -1019,8 +1021,8 @@
try
{
- PowerSupply psu{bus, PSUInventoryPath, 3,
- 0x68, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 3, 0x68,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
MockedPMBus& mockPMBus = static_cast<MockedPMBus&>(psu.getPMBus());
// If it is not present, I should not be trying to read a string
EXPECT_CALL(mockPMBus, readString(_, _)).Times(0);
@@ -1033,8 +1035,8 @@
try
{
- PowerSupply psu{bus, PSUInventoryPath, 13,
- 0x69, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 13, 0x69,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
MockedGPIOInterface* mockPresenceGPIO =
static_cast<MockedGPIOInterface*>(psu.getPresenceGPIO());
// GPIO read return 1 to indicate present.
@@ -1081,8 +1083,8 @@
{
auto bus = sdbusplus::bus::new_default();
- PowerSupply psu{bus, PSUInventoryPath, 3,
- 0x68, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 3, 0x68,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
MockedGPIOInterface* mockPresenceGPIO =
static_cast<MockedGPIOInterface*>(psu.getPresenceGPIO());
EXPECT_EQ(psu.isPresent(), false);
@@ -1116,8 +1118,8 @@
{
auto bus = sdbusplus::bus::new_default();
- PowerSupply psu{bus, PSUInventoryPath, 11,
- 0x6f, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 11, 0x6f,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
MockedGPIOInterface* mockPresenceGPIO =
static_cast<MockedGPIOInterface*>(psu.getPresenceGPIO());
// Always return 1 to indicate present.
@@ -1176,8 +1178,8 @@
{
auto bus = sdbusplus::bus::new_default();
- PowerSupply psu{bus, PSUInventoryPath, 3,
- 0x68, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 3, 0x68,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
MockedGPIOInterface* mockPresenceGPIO =
static_cast<MockedGPIOInterface*>(psu.getPresenceGPIO());
// Always return 1 to indicate present.
@@ -1233,8 +1235,8 @@
{
auto bus = sdbusplus::bus::new_default();
- PowerSupply psu{bus, PSUInventoryPath, 3,
- 0x68, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 3, 0x68,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
MockedGPIOInterface* mockPresenceGPIO =
static_cast<MockedGPIOInterface*>(psu.getPresenceGPIO());
// Always return 1 to indicate present.
@@ -1284,8 +1286,8 @@
{
auto bus = sdbusplus::bus::new_default();
- PowerSupply psu{bus, PSUInventoryPath, 3,
- 0x68, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 3, 0x68,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
MockedGPIOInterface* mockPresenceGPIO =
static_cast<MockedGPIOInterface*>(psu.getPresenceGPIO());
// Always return 1 to indicate present.
@@ -1355,8 +1357,8 @@
{
auto bus = sdbusplus::bus::new_default();
- PowerSupply psu{bus, PSUInventoryPath, 3,
- 0x69, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 3, 0x69,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
MockedGPIOInterface* mockPresenceGPIO =
static_cast<MockedGPIOInterface*>(psu.getPresenceGPIO());
// Always return 1 to indicate present.
@@ -1406,8 +1408,8 @@
{
auto bus = sdbusplus::bus::new_default();
- PowerSupply psu{bus, PSUInventoryPath, 3,
- 0x6d, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 3, 0x6d,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
MockedGPIOInterface* mockPresenceGPIO =
static_cast<MockedGPIOInterface*>(psu.getPresenceGPIO());
// Always return 1 to indicate present.
@@ -1462,8 +1464,8 @@
{
auto bus = sdbusplus::bus::new_default();
- PowerSupply psu{bus, PSUInventoryPath, 3,
- 0x6a, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 3, 0x6a,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
MockedGPIOInterface* mockPresenceGPIO =
static_cast<MockedGPIOInterface*>(psu.getPresenceGPIO());
// Always return 1 to indicate present.
@@ -1516,8 +1518,8 @@
EXPECT_CALL(mockedUtil, setAvailable(_, _, true)).Times(1);
EXPECT_CALL(mockedUtil, setAvailable(_, _, false)).Times(0);
- PowerSupply psu{bus, PSUInventoryPath, 3,
- 0x6d, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 3, 0x6d,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
MockedGPIOInterface* mockPresenceGPIO =
static_cast<MockedGPIOInterface*>(psu.getPresenceGPIO());
// Always return 1 to indicate present.
@@ -1572,8 +1574,8 @@
EXPECT_CALL(mockedUtil, setAvailable(_, _, true)).Times(1);
EXPECT_CALL(mockedUtil, setAvailable(_, _, false)).Times(0);
- PowerSupply psu{bus, PSUInventoryPath, 3,
- 0x6a, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 3, 0x6a,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
MockedGPIOInterface* mockPresenceGPIO =
static_cast<MockedGPIOInterface*>(psu.getPresenceGPIO());
// Always return 1 to indicate present.
@@ -1625,8 +1627,8 @@
{
auto bus = sdbusplus::bus::new_default();
- PowerSupply psu{bus, PSUInventoryPath, 3,
- 0x6b, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 3, 0x6b,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
MockedGPIOInterface* mockPresenceGPIO =
static_cast<MockedGPIOInterface*>(psu.getPresenceGPIO());
// Always return 1 to indicate present.
@@ -1760,8 +1762,8 @@
TEST_F(PowerSupplyTests, HasPSKillFault)
{
auto bus = sdbusplus::bus::new_default();
- PowerSupply psu{bus, PSUInventoryPath, 4,
- 0x6d, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 4, 0x6d,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
MockedGPIOInterface* mockPresenceGPIO =
static_cast<MockedGPIOInterface*>(psu.getPresenceGPIO());
// Always return 1 to indicate present.
@@ -1850,8 +1852,8 @@
TEST_F(PowerSupplyTests, HasPS12VcsFault)
{
auto bus = sdbusplus::bus::new_default();
- PowerSupply psu{bus, PSUInventoryPath, 5,
- 0x6e, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 5, 0x6e,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
MockedGPIOInterface* mockPresenceGPIO =
static_cast<MockedGPIOInterface*>(psu.getPresenceGPIO());
// Always return 1 to indicate present.
@@ -1924,8 +1926,8 @@
TEST_F(PowerSupplyTests, HasPSCS12VFault)
{
auto bus = sdbusplus::bus::new_default();
- PowerSupply psu{bus, PSUInventoryPath, 6,
- 0x6f, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 6, 0x6f,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
MockedGPIOInterface* mockPresenceGPIO =
static_cast<MockedGPIOInterface*>(psu.getPresenceGPIO());
// Always return 1 to indicate present.
@@ -1998,8 +2000,8 @@
{
auto bus = sdbusplus::bus::new_default();
{
- PowerSupply psu{bus, PSUInventoryPath, 6,
- 0x6f, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 6, 0x6f,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
// Defaults to not present due to constructor and mock ordering.
psu.setupInputHistory();
EXPECT_EQ(psu.hasInputHistory(), false);
@@ -2030,8 +2032,8 @@
}
{
// Workaround - Disable INPUT_HISTORY collection if 1400W
- PowerSupply psu{bus, PSUInventoryPath, 3,
- 0x68, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 3, 0x68,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
// Defaults to not present due to constructor and mock ordering.
psu.setupInputHistory();
EXPECT_EQ(psu.hasInputHistory(), false);
@@ -2062,8 +2064,9 @@
EXPECT_EQ(psu.hasInputHistory(), false);
}
{
- PowerSupply psu{bus, PSUInventoryPath, 11,
- 0x58, "inspur-ipsps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 11,
+ 0x58, "inspur-ipsps", PSUGPIOLineName,
+ isPowerOn};
// Defaults to not present due to constructor and mock ordering.
psu.setupInputHistory();
EXPECT_EQ(psu.hasInputHistory(), false);
@@ -2092,8 +2095,8 @@
TEST_F(PowerSupplyTests, UpdateHistory)
{
auto bus = sdbusplus::bus::new_default();
- PowerSupply psu{bus, PSUInventoryPath, 7,
- 0x6e, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 7, 0x6e,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
EXPECT_EQ(psu.hasInputHistory(), false);
EXPECT_EQ(psu.getNumInputHistoryRecords(), 0);
MockedGPIOInterface* mockPresenceGPIO =
@@ -2175,8 +2178,8 @@
TEST_F(PowerSupplyTests, IsSyncHistoryRequired)
{
auto bus = sdbusplus::bus::new_default();
- PowerSupply psu{bus, PSUInventoryPath, 8,
- 0x6f, "ibm-cffps", PSUGPIOLineName};
+ PowerSupply psu{bus, PSUInventoryPath, 8, 0x6f,
+ "ibm-cffps", PSUGPIOLineName, isPowerOn};
EXPECT_EQ(psu.hasInputHistory(), false);
EXPECT_EQ(psu.isSyncHistoryRequired(), false);
MockedGPIOInterface* mockPresenceGPIO =