psu-ng: Updates to get VPD data to inventory
Add in code to retrieve the VPD data and populate that data to the D-Bus
inventory properties.
Use the PMBus utilities to retrieve the model (CCIN for IBM power
supplies), part number, serial number, and firmware version information
from the sysfs "files" provided via the device driver.
Only build in IBM VPD data collection and reporting if meson ran with
-Dibm-vpd=true.
Tested:
Copied SDK build of phospor-psu-monitor with -Dibm-vpd=true to
Rainier hardware system, verified inventory properties updated/added.
Signed-off-by: Brandon Wyman <bjwyman@gmail.com>
Change-Id: I61688b154ead570e9d9390342596bf7d840f4dce
diff --git a/meson.build b/meson.build
index d3312f4..eef6b82 100644
--- a/meson.build
+++ b/meson.build
@@ -86,6 +86,7 @@
'SEQUENCER', sequencer_class)
conf.set10(
'DEVICE_ACCESS', get_option('device-access'))
+conf.set10('IBM_VPD', get_option('ibm-vpd'))
configure_file(output: 'config.h', configuration: conf)
diff --git a/meson_options.txt b/meson_options.txt
index 1d587ee..418320a 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -26,6 +26,12 @@
'device-access', type: 'boolean', value: true,
description: 'Enable UCD90160 hardware access.',
)
+
+option(
+ 'ibm-vpd', type: 'boolean', value: false,
+ description: 'Setup for IBM VPD collection for inventory.',
+)
+
option(
'ucd90160-yaml', type: 'string', value: 'example/ucd90160.yaml',
description: 'The sequencer definition file to use.',
diff --git a/phosphor-power-supply/README.md b/phosphor-power-supply/README.md
index 38b6c83..a485508 100644
--- a/phosphor-power-supply/README.md
+++ b/phosphor-power-supply/README.md
@@ -2,6 +2,11 @@
Design document: https://github.com/openbmc/docs/blob/master/designs/psu-monitoring.md
+# Compile Options
+
+To enable reading VPD data via PMBus commands to IBM common form factor
+power supplies (ibm-cffps), run meson with `-Dibm-vpd=true`.
+
# JSON Configuration File
The JSON configuration file should contain:
diff --git a/phosphor-power-supply/power_supply.cpp b/phosphor-power-supply/power_supply.cpp
index 8c80ef3..2b77bcd 100644
--- a/phosphor-power-supply/power_supply.cpp
+++ b/phosphor-power-supply/power_supply.cpp
@@ -1,3 +1,5 @@
+#include "config.h"
+
#include "power_supply.hpp"
#include "types.hpp"
@@ -5,6 +7,10 @@
#include <xyz/openbmc_project/Common/Device/error.hpp>
+#include <chrono> // sleep_for()
+#include <cstdint> // uint8_t...
+#include <thread> // sleep_for()
+
namespace phosphor::power::psu
{
@@ -157,8 +163,13 @@
if (std::get<bool>(valPropMap->second))
{
present = true;
+ // TODO: Immediately trying to read or write the "files" causes read
+ // or write failures.
+ using namespace std::chrono_literals;
+ std::this_thread::sleep_for(20ms);
onOffConfig(phosphor::pmbus::ON_OFF_CONFIG_CONTROL_PIN_ONLY);
clearFaults();
+ updateInventory();
}
else
{
@@ -170,4 +181,150 @@
}
}
+void PowerSupply::updateInventory()
+{
+ using namespace phosphor::pmbus;
+
+#ifdef IBM_VPD
+ std::string ccin;
+ std::string pn;
+ std::string fn;
+ std::string header;
+ std::string sn;
+ std::string version;
+ using PropertyMap =
+ std::map<std::string, std::variant<std::string, std::vector<uint8_t>>>;
+ PropertyMap assetProps;
+ PropertyMap versionProps;
+ PropertyMap ipzvpdDINFProps;
+ PropertyMap ipzvpdVINIProps;
+ using InterfaceMap = std::map<std::string, PropertyMap>;
+ InterfaceMap interfaces;
+ using ObjectMap = std::map<sdbusplus::message::object_path, InterfaceMap>;
+ ObjectMap object;
+#endif
+
+ if (present)
+ {
+ // TODO: non-IBM inventory updates?
+
+#ifdef IBM_VPD
+ try
+ {
+ ccin = pmbusIntf->readString(CCIN, Type::HwmonDeviceDebug);
+ assetProps.emplace(MODEL_PROP, ccin);
+ }
+ catch (ReadFailure& e)
+ {
+ // Ignore the read failure, let pmbus code indicate failure, path...
+ // TODO - ibm918
+ // https://github.com/openbmc/docs/blob/master/designs/vpd-collection.md
+ // The BMC must log errors if any of the VPD cannot be properly
+ // parsed or fails ECC checks.
+ }
+
+ try
+ {
+ pn = pmbusIntf->readString(PART_NUMBER, Type::HwmonDeviceDebug);
+ assetProps.emplace(PN_PROP, pn);
+ }
+ catch (ReadFailure& e)
+ {
+ // Ignore the read failure, let pmbus code indicate failure, path...
+ }
+
+ try
+ {
+ fn = pmbusIntf->readString(FRU_NUMBER, Type::HwmonDeviceDebug);
+ }
+ catch (ReadFailure& e)
+ {
+ // Ignore the read failure, let pmbus code indicate failure, path...
+ }
+
+ try
+ {
+ header =
+ pmbusIntf->readString(SERIAL_HEADER, Type::HwmonDeviceDebug);
+ sn = pmbusIntf->readString(SERIAL_NUMBER, Type::HwmonDeviceDebug);
+ assetProps.emplace(SN_PROP, sn);
+ }
+ catch (ReadFailure& e)
+ {
+ // Ignore the read failure, let pmbus code indicate failure, path...
+ }
+
+ try
+ {
+ version = pmbusIntf->readString(FW_VERSION, Type::HwmonDeviceDebug);
+ versionProps.emplace(VERSION_PROP, version);
+ }
+ catch (ReadFailure& e)
+ {
+ // Ignore the read failure, let pmbus code indicate failure, path...
+ }
+
+ ipzvpdVINIProps.emplace("CC",
+ std::vector<uint8_t>(ccin.begin(), ccin.end()));
+ ipzvpdVINIProps.emplace("PN",
+ std::vector<uint8_t>(pn.begin(), pn.end()));
+ ipzvpdVINIProps.emplace("FN",
+ std::vector<uint8_t>(fn.begin(), fn.end()));
+ std::string header_sn = header + sn + '\0';
+ ipzvpdVINIProps.emplace(
+ "SN", std::vector<uint8_t>(header_sn.begin(), header_sn.end()));
+ std::string description = "IBM PS";
+ ipzvpdVINIProps.emplace(
+ "DR", std::vector<uint8_t>(description.begin(), description.end()));
+
+ // Update the Resource Identifier (RI) keyword
+ // 2 byte FRC: 0x0003
+ // 2 byte RID: 0x1000, 0x1001...
+ std::uint8_t num = std::stoul(
+ inventoryPath.substr(inventoryPath.size() - 1, 1), nullptr, 0);
+ std::vector<uint8_t> ri{0x00, 0x03, 0x10, num};
+ ipzvpdDINFProps.emplace("RI", ri);
+
+ // Fill in the FRU Label (FL) keyword.
+ std::string fl = "E";
+ fl.push_back(inventoryPath.back());
+ fl.resize(FL_KW_SIZE, ' ');
+ ipzvpdDINFProps.emplace("FL",
+ std::vector<uint8_t>(fl.begin(), fl.end()));
+
+ interfaces.emplace(ASSET_IFACE, std::move(assetProps));
+ interfaces.emplace(VERSION_IFACE, std::move(versionProps));
+ interfaces.emplace(DINF_IFACE, std::move(ipzvpdDINFProps));
+ interfaces.emplace(VINI_IFACE, std::move(ipzvpdVINIProps));
+
+ auto path = inventoryPath.substr(strlen(INVENTORY_OBJ_PATH));
+ object.emplace(path, std::move(interfaces));
+
+ try
+ {
+ auto service =
+ util::getService(INVENTORY_OBJ_PATH, INVENTORY_MGR_IFACE, bus);
+
+ if (service.empty())
+ {
+ log<level::ERR>("Unable to get inventory manager service");
+ return;
+ }
+
+ auto method =
+ bus.new_method_call(service.c_str(), INVENTORY_OBJ_PATH,
+ INVENTORY_MGR_IFACE, "Notify");
+
+ method.append(std::move(object));
+
+ auto reply = bus.call(method);
+ }
+ catch (std::exception& e)
+ {
+ log<level::ERR>(e.what(), entry("PATH=%s", inventoryPath.c_str()));
+ }
+#endif
+ }
+}
+
} // namespace phosphor::power::psu
diff --git a/phosphor-power-supply/power_supply.hpp b/phosphor-power-supply/power_supply.hpp
index 487fa53..eb310da 100644
--- a/phosphor-power-supply/power_supply.hpp
+++ b/phosphor-power-supply/power_supply.hpp
@@ -6,9 +6,30 @@
#include <sdbusplus/bus/match.hpp>
+#include <stdexcept>
+
namespace phosphor::power::psu
{
+#ifdef IBM_VPD
+// PMBus device driver "file name" to read for CCIN value.
+constexpr auto CCIN = "ccin";
+constexpr auto PART_NUMBER = "part_number";
+constexpr auto FRU_NUMBER = "fru";
+constexpr auto SERIAL_HEADER = "header";
+constexpr auto SERIAL_NUMBER = "serial_number";
+constexpr auto FW_VERSION = "fw_version";
+
+// The D-Bus property name to update with the CCIN value.
+constexpr auto MODEL_PROP = "Model";
+constexpr auto PN_PROP = "PartNumber";
+constexpr auto SN_PROP = "SerialNumber";
+constexpr auto VERSION_PROP = "Version";
+
+// ipzVPD Keyword sizes
+static constexpr auto FL_KW_SIZE = 20;
+#endif
+
/**
* @class PowerSupply
* Represents a PMBus power supply device.
@@ -34,6 +55,11 @@
inventoryPath(invpath),
pmbusIntf(phosphor::pmbus::createPMBus(i2cbus, i2caddr))
{
+ if (inventoryPath.empty())
+ {
+ throw std::invalid_argument{"Invalid empty inventoryPath"};
+ }
+
// Setup the functions to call when the D-Bus inventory path for the
// Present property changes.
presentMatch = std::make_unique<sdbusplus::bus::match_t>(
@@ -99,9 +125,7 @@
* - CCIN (Customer Card Identification Number) - added as the Model
* - Firmware version
*/
- void updateInventory()
- {
- }
+ void updateInventory();
/**
* @brief Accessor function to indicate present status
diff --git a/phosphor-power-supply/test/meson.build b/phosphor-power-supply/test/meson.build
index a71ac57..0350799 100644
--- a/phosphor-power-supply/test/meson.build
+++ b/phosphor-power-supply/test/meson.build
@@ -16,6 +16,9 @@
'../..'
],
link_args: dynamic_linker,
+ link_with: [
+ libpower,
+ ],
build_rpath: get_option('oe-sdk').enabled() ? rpath : '',
objects: power_supply,
)
diff --git a/phosphor-power-supply/test/mock.hpp b/phosphor-power-supply/test/mock.hpp
index c4c2c9d..63afffe 100644
--- a/phosphor-power-supply/test/mock.hpp
+++ b/phosphor-power-supply/test/mock.hpp
@@ -17,6 +17,8 @@
MOCK_METHOD(uint64_t, read, (const std::string& name, Type type),
(override));
+ MOCK_METHOD(std::string, readString, (const std::string& name, Type type),
+ (override));
MOCK_METHOD(void, writeBinary,
(const std::string& name, std::vector<uint8_t> data, Type type),
(override));
diff --git a/phosphor-power-supply/test/power_supply_tests.cpp b/phosphor-power-supply/test/power_supply_tests.cpp
index 5eb4b3e..55dc2a7 100644
--- a/phosphor-power-supply/test/power_supply_tests.cpp
+++ b/phosphor-power-supply/test/power_supply_tests.cpp
@@ -46,14 +46,40 @@
* @param[in] i2caddr - The 16-bit I2C address of the power supply
*/
auto bus = sdbusplus::bus::new_default();
- EXPECT_CALL(mockedUtil, getPresence(_, StrEq(PSUInventoryPath))).Times(1);
- auto psu = std::make_unique<PowerSupply>(bus, PSUInventoryPath, 3, "0068");
- EXPECT_EQ(psu->isPresent(), false);
- EXPECT_EQ(psu->isFaulted(), false);
- EXPECT_EQ(psu->hasInputFault(), false);
- EXPECT_EQ(psu->hasMFRFault(), false);
- EXPECT_EQ(psu->hasVINUVFault(), false);
+ // Try where inventory path is empty, constructor should fail.
+ try
+ {
+ auto psu = std::make_unique<PowerSupply>(bus, "", 3, "0068");
+ ADD_FAILURE() << "Should not have reached this line.";
+ }
+ catch (const std::invalid_argument& e)
+ {
+ EXPECT_STREQ(e.what(), "Invalid empty inventoryPath");
+ }
+ catch (...)
+ {
+ ADD_FAILURE() << "Should not have caught exception.";
+ }
+
+ // Test with valid arguments
+ try
+ {
+ EXPECT_CALL(mockedUtil, getPresence(_, StrEq(PSUInventoryPath)))
+ .Times(1);
+ auto psu =
+ std::make_unique<PowerSupply>(bus, PSUInventoryPath, 3, "0068");
+
+ EXPECT_EQ(psu->isPresent(), false);
+ EXPECT_EQ(psu->isFaulted(), false);
+ EXPECT_EQ(psu->hasInputFault(), false);
+ EXPECT_EQ(psu->hasMFRFault(), false);
+ EXPECT_EQ(psu->hasVINUVFault(), false);
+ }
+ catch (...)
+ {
+ ADD_FAILURE() << "Should not have caught exception.";
+ }
}
TEST_F(PowerSupplyTests, Analyze)
@@ -225,12 +251,47 @@
TEST_F(PowerSupplyTests, UpdateInventory)
{
auto bus = sdbusplus::bus::new_default();
- EXPECT_CALL(mockedUtil, getPresence(_, StrEq(PSUInventoryPath)))
- .Times(1)
- .WillOnce(Return(true)); // present
- PowerSupply psu{bus, PSUInventoryPath, 3, "0068"};
- psu.updateInventory();
- // TODO: Checks / Story #921
+
+ try
+ {
+ EXPECT_CALL(mockedUtil, getPresence(_, StrEq(PSUInventoryPath)))
+ .Times(1)
+ .WillOnce(Return(false)); // missing
+ PowerSupply psu{bus, PSUInventoryPath, 3, "0068"};
+ 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);
+ psu.updateInventory();
+ }
+ catch (...)
+ {
+ ADD_FAILURE() << "Should not have caught exception.";
+ }
+
+ try
+ {
+ EXPECT_CALL(mockedUtil, getPresence(_, StrEq(PSUInventoryPath)))
+ .Times(1)
+ .WillOnce(Return(true)); // present
+ PowerSupply psu{bus, PSUInventoryPath, 13, "0069"};
+ MockedPMBus& mockPMBus = static_cast<MockedPMBus&>(psu.getPMBus());
+ EXPECT_CALL(mockPMBus, readString(_, _)).WillRepeatedly(Return(""));
+ psu.updateInventory();
+
+ EXPECT_CALL(mockPMBus, readString(_, _))
+ .WillOnce(Return("CCIN"))
+ .WillOnce(Return("PN3456"))
+ .WillOnce(Return("FN3456"))
+ .WillOnce(Return("HEADER"))
+ .WillOnce(Return("SN3456"))
+ .WillOnce(Return("FW3456"));
+ psu.updateInventory();
+ // TODO: D-Bus mocking to verify values stored on D-Bus (???)
+ }
+ catch (...)
+ {
+ ADD_FAILURE() << "Should not have caught exception.";
+ }
}
TEST_F(PowerSupplyTests, IsPresent)
diff --git a/pmbus.hpp b/pmbus.hpp
index 123e4fd..1ca6536 100644
--- a/pmbus.hpp
+++ b/pmbus.hpp
@@ -142,6 +142,7 @@
virtual ~PMBusBase() = default;
virtual uint64_t read(const std::string& name, Type type) = 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;
};
@@ -269,7 +270,7 @@
*
* @return string - The data read from the file.
*/
- std::string readString(const std::string& name, Type type);
+ std::string readString(const std::string& name, Type type) override;
/**
* Read data from a binary file in sysfs.
diff --git a/types.hpp b/types.hpp
index e7287d4..df67013 100644
--- a/types.hpp
+++ b/types.hpp
@@ -10,6 +10,11 @@
constexpr auto ASSET_IFACE = "xyz.openbmc_project.Inventory.Decorator.Asset";
constexpr auto PSU_INVENTORY_IFACE =
"xyz.openbmc_project.Inventory.Item.PowerSupply";
+constexpr auto VERSION_IFACE = "xyz.openbmc_project.Software.Version";
+#ifdef IBM_VPD
+constexpr auto DINF_IFACE = "com.ibm.ipzvpd.DINF";
+constexpr auto VINI_IFACE = "com.ibm.ipzvpd.VINI";
+#endif
constexpr auto ENDPOINTS_PROP = "endpoints";
constexpr auto MESSAGE_PROP = "Message";