Fix occ-control terminations
1. Ensure PowerMode object created before attempting to use
2. Only call setChassisAssociation after getting successful temperature
reading.
Tested on Everest hw
Change-Id: I308c072cf5ab0235086c136ba7644125de0a8c6a
Signed-off-by: Chris Cain <cjcain@us.ibm.com>
diff --git a/occ_manager.cpp b/occ_manager.cpp
index 2245a3f..ee8575d 100644
--- a/occ_manager.cpp
+++ b/occ_manager.cpp
@@ -133,6 +133,13 @@
{
auto path = fs::path(OCC_CONTROL_ROOT) / occ;
+#ifdef POWER10
+ if (!pmode)
+ {
+ pmode = std::make_unique<open_power::occ::powermode::PowerMode>(*this);
+ }
+#endif
+
statusObjects.emplace_back(std::make_unique<Status>(
event, path.c_str(), *this,
#ifdef POWER10
@@ -163,12 +170,8 @@
}
#ifdef POWER10
- // Create the power mode object for master OCC
- if (!pmode)
- {
- pmode = std::make_unique<open_power::occ::powermode::PowerMode>(
- *this, path.c_str());
- }
+ // Set the master OCC on the PowerMode object
+ pmode->setMasterOcc(path);
#endif
}
@@ -289,8 +292,9 @@
pcap = std::make_unique<open_power::occ::powercap::PowerCap>(
*statusObjects.front(), occMasterName);
#ifdef POWER10
- pmode = std::make_unique<open_power::occ::powermode::PowerMode>(
- *this, path.c_str());
+ pmode = std::make_unique<open_power::occ::powermode::PowerMode>(*this);
+ // Set the master OCC on the PowerMode object
+ pmode->setMasterOcc(path);
#endif
}
#endif
@@ -651,13 +655,6 @@
continue;
}
- // At this point, the sensor will be created for sure.
- if (existingSensors.find(sensorPath) == existingSensors.end())
- {
- open_power::occ::dbus::OccDBusSensors::getOccDBus()
- .setChassisAssociation(sensorPath);
- }
-
if (faultValue != 0)
{
open_power::occ::dbus::OccDBusSensors::getOccDBus().setValue(
@@ -690,6 +687,13 @@
open_power::occ::dbus::OccDBusSensors::getOccDBus()
.setOperationalStatus(sensorPath, true);
+ // At this point, the sensor will be created for sure.
+ if (existingSensors.find(sensorPath) == existingSensors.end())
+ {
+ open_power::occ::dbus::OccDBusSensors::getOccDBus()
+ .setChassisAssociation(sensorPath);
+ }
+
existingSensors[sensorPath] = id;
}
return;
@@ -971,9 +975,9 @@
#ifdef POWER10
void Manager::occsNotAllRunning()
{
- // Function will also gets called when occ-control app gets restarted.
- // (occ active sensors do not change, so the Status object does not
- // call Manager back for all OCCs)
+ // Function will also gets called when occ-control app gets
+ // restarted. (occ active sensors do not change, so the Status
+ // object does not call Manager back for all OCCs)
if (activeCount != statusObjects.size())
{
diff --git a/occ_pass_through.cpp b/occ_pass_through.cpp
index 6cf6374..ea640ef 100644
--- a/occ_pass_through.cpp
+++ b/occ_pass_through.cpp
@@ -137,6 +137,12 @@
return false;
}
+ if (!pmode)
+ {
+ log<level::ERR>("PassThrough::setMode: PowerMode is not defined!");
+ return false;
+ }
+
log<level::INFO>(
fmt::format("PassThrough::setMode() Setting Power Mode {} (data: {})",
newMode, modeData)
diff --git a/occ_status.cpp b/occ_status.cpp
index 1da440c..0cb9b46 100644
--- a/occ_status.cpp
+++ b/occ_status.cpp
@@ -206,7 +206,9 @@
{
if (device.master())
{
- // Prevent mode changes
+ // Set the master OCC on the PowerMode object
+ pmode->setMasterOcc(path);
+ // Enable mode changes
pmode->setMasterActive();
// Special processing by master OCC when it goes active
diff --git a/powermode.cpp b/powermode.cpp
index 04a9787..9b9f3cb 100644
--- a/powermode.cpp
+++ b/powermode.cpp
@@ -20,11 +20,22 @@
using namespace phosphor::logging;
using Mode = sdbusplus::xyz::openbmc_project::Control::Power::server::Mode;
+// Set the Master OCC
+void PowerMode::setMasterOcc(const std::string& occPath)
+{
+ path = occPath;
+ occInstance = path.back() - '0';
+ log<level::DEBUG>(fmt::format("PowerMode::setMasterOcc(OCC{}, {})",
+ occInstance, path.c_str())
+ .c_str());
+ occCmd = std::make_unique<open_power::occ::OccCommand>(occInstance,
+ path.c_str());
+ masterOccSet = true;
+};
+
// Called when DBus power mode gets changed
void PowerMode::modeChanged(sdbusplus::message::message& msg)
{
- SysPwrMode newMode = SysPwrMode::NO_CHANGE;
-
std::map<std::string, std::variant<std::string>> properties{};
std::string interface;
std::string propVal;
@@ -34,7 +45,7 @@
{
auto modeEntryValue = modeEntry->second;
propVal = std::get<std::string>(modeEntryValue);
- newMode = convertStringToMode(propVal);
+ SysPwrMode newMode = convertStringToMode(propVal);
if (newMode != SysPwrMode::NO_CHANGE)
{
// DBus mode changed, get rid of any OEM mode if set
@@ -149,7 +160,7 @@
SysPwrMode PowerMode::getDbusMode()
{
using namespace open_power::occ::powermode;
- SysPwrMode currentMode = SysPwrMode::NO_CHANGE;
+ SysPwrMode currentMode;
// This will throw exception on failure
auto& bus = utils::getBus();
@@ -220,12 +231,12 @@
// Send mode change request to the master OCC
CmdStatus PowerMode::sendModeChange()
{
- CmdStatus status = CmdStatus::FAILURE;
+ CmdStatus status;
- if (!masterActive)
+ if (!masterActive || !masterOccSet)
{
// Nothing to do
- log<level::DEBUG>("PowerMode::sendModeChange: MODE CHANGE not enabled");
+ log<level::DEBUG>("PowerMode::sendModeChange: OCC master not active");
return CmdStatus::SUCCESS;
}
@@ -265,7 +276,7 @@
"PowerMode::sendModeChange: SET_MODE({},{}) command to OCC{} ({} bytes)",
newMode, modeData, occInstance, cmd.size())
.c_str());
- status = occCmd.send(cmd, rsp);
+ status = occCmd->send(cmd, rsp);
if (status == CmdStatus::SUCCESS)
{
if (rsp.size() == 5)
@@ -317,7 +328,7 @@
void PowerMode::ipsChanged(sdbusplus::message::message& msg)
{
- if (!masterActive)
+ if (!masterActive || !masterOccSet)
{
// Nothing to do
return;
@@ -511,9 +522,9 @@
// Send Idle Power Saver config data to the master OCC
CmdStatus PowerMode::sendIpsData()
{
- CmdStatus status = CmdStatus::FAILURE;
+ CmdStatus status;
- if (!masterActive)
+ if (!masterActive || !masterOccSet)
{
// Nothing to do
return CmdStatus::SUCCESS;
@@ -556,7 +567,7 @@
"command to OCC{} ({} bytes)",
occInstance, cmd.size())
.c_str());
- status = occCmd.send(cmd, rsp);
+ status = occCmd->send(cmd, rsp);
if (status == CmdStatus::SUCCESS)
{
if (rsp.size() == 5)
diff --git a/powermode.hpp b/powermode.hpp
index 5342c77..0ec22a5 100644
--- a/powermode.hpp
+++ b/powermode.hpp
@@ -159,9 +159,8 @@
* @param[in] managerRef -
* @param[in] path -
*/
- explicit PowerMode(Manager& managerRef, const char* path) :
- manager(managerRef), path(path), occInstance(this->path.back() - '0'),
- occCmd(occInstance, path),
+ explicit PowerMode(const Manager& managerRef) :
+ manager(managerRef), occInstance(0),
pmodeMatch(utils::getBus(),
sdbusplus::bus::match::rules::propertiesChanged(
PMODE_PATH, PMODE_INTERFACE),
@@ -170,7 +169,7 @@
sdbusplus::bus::match::rules::propertiesChanged(
PIPS_PATH, PIPS_INTERFACE),
[this](auto& msg) { this->ipsChanged(msg); }),
- masterActive(false){};
+ masterOccSet(false), masterActive(false){};
bool setMode(const SysPwrMode newMode, const uint16_t modedata);
@@ -184,6 +183,12 @@
*/
CmdStatus sendIpsData();
+ /** @brief Set the master OCC path
+ *
+ * @param[in] occPath - hwmon path for master OCC
+ */
+ void setMasterOcc(const std::string& occPath);
+
/** @brief Notify object of master OCC state. If not acitve, no
* commands will be sent to the master OCC
*
@@ -205,7 +210,7 @@
int occInstance;
/** @brief Object to send commands to the OCC */
- OccCommand occCmd;
+ std::unique_ptr<open_power::occ::OccCommand> occCmd;
/** @brief Used to subscribe to dbus pmode property changes **/
sdbusplus::bus::match_t pmodeMatch;
@@ -215,6 +220,10 @@
OccPersistData persistedData;
+ /** @brief True when the master OCC has been established */
+ bool masterOccSet;
+
+ /** @brief True when the master OCC is active */
bool masterActive;
/** @brief Callback for pmode setting changes