hwmontempsensor: Manage devices on host power-state events We now install handler callbacks for power event signals, deactivating sensors on power-off and activating them on power-on/POST-complete according to their PowerState configuration. This is needed to properly support sensor devices that are in the same power domain as the host, and hence may need their drivers bound only after the host is powered on so that the driver can properly probe and attach to the device (and likewise be unbound when the host is powered off). Tested: in combination with corresponding entity-manager patches to prevent it from trying to manage the same devices, hwmontempsensor successfully activates and deactivates host-power-domain nct6779 and w83773g sensors on romed8hm3. After the host is shut off, sensors remain on dbus with 'Available' set to false and 'Value' reading NaN. Also tested killing hwmontempsor after it instantiated host-power-domain sensors, shutting off the host, and then restarting hwmontempsensor to verify that it removed and re-created the sensor devices as necessary. Signed-off-by: Zev Weiss <zev@bewilderbeest.net> Change-Id: I2e272fd0870f3abfb06b6f3617f32721861bbc5b
diff --git a/include/HwmonTempSensor.hpp b/include/HwmonTempSensor.hpp index af275ec..276f37d 100644 --- a/include/HwmonTempSensor.hpp +++ b/include/HwmonTempSensor.hpp
@@ -1,5 +1,6 @@ #pragma once +#include <DeviceMgmt.hpp> #include <Thresholds.hpp> #include <boost/asio/random_access_file.hpp> #include <sdbusplus/asio/object_server.hpp> @@ -30,14 +31,20 @@ std::vector<thresholds::Threshold>&& thresholds, const struct SensorParams& thisSensorParameters, float pollRate, const std::string& sensorConfiguration, - PowerState powerState); + PowerState powerState, + const std::shared_ptr<I2CDevice>& i2cDevice); ~HwmonTempSensor() override; void setupRead(void); + void activate(const std::string& newPath, + const std::shared_ptr<I2CDevice>& newI2CDevice); + void deactivate(void); + bool isActive(void); private: // Ordering is important here; readBuf is first so that it's not destroyed // while async operations from other member fields might still be using it. std::array<char, 128> readBuf{}; + std::shared_ptr<I2CDevice> i2cDevice; sdbusplus::asio::object_server& objServer; boost::asio::random_access_file inputDev; boost::asio::steady_timer waitTimer;
diff --git a/src/HwmonTempMain.cpp b/src/HwmonTempMain.cpp index 79db947..e1b0d62 100644 --- a/src/HwmonTempMain.cpp +++ b/src/HwmonTempMain.cpp
@@ -240,22 +240,95 @@ return configMap; } +boost::container::flat_map<std::string, std::shared_ptr<I2CDevice>> + instantiateDevices( + const ManagedObjectType& sensorConfigs, + const boost::container::flat_map< + std::string, std::shared_ptr<HwmonTempSensor>>& sensors) +{ + boost::container::flat_map<std::string, std::shared_ptr<I2CDevice>> + newSensors; + for (const auto& [path, sensor] : sensorConfigs) + { + for (const auto& [name, cfg] : sensor) + { + PowerState powerState = getPowerState(cfg); + if (!readingStateGood(powerState)) + { + continue; + } + + auto findSensorName = cfg.find("Name"); + if (findSensorName == cfg.end()) + { + continue; + } + std::string sensorName = + std::get<std::string>(findSensorName->second); + + auto findSensor = sensors.find(sensorName); + if (findSensor != sensors.end() && findSensor->second != nullptr && + findSensor->second->isActive()) + { + continue; + } + + std::optional<I2CDeviceParams> params = + getI2CDeviceParams(sensorTypes, cfg); + if (params.has_value() && !params->deviceStatic()) + { + // There exist error cases in which a sensor device that we + // need is already instantiated, but needs to be destroyed and + // re-created in order to be useful (for example if we crash + // after instantiating a device and the sensor device's power + // is cut before we get restarted, leaving it "present" but + // not really usable). To be on the safe side, instantiate a + // temporary device that's immediately destroyed so as to + // ensure that we end up with a fresh instance of it. + if (params->devicePresent()) + { + std::cerr << "Clearing out previous instance for " + << path.str << "\n"; + I2CDevice tmp(*params); + } + + try + { + newSensors.emplace(path.str, + std::make_shared<I2CDevice>(*params)); + } + catch (std::runtime_error&) + { + std::cerr << "Failed to instantiate " << params->type->name + << " at address " << params->address << " on bus " + << params->bus << "\n"; + } + } + } + } + return newSensors; +} + void createSensors( boost::asio::io_service& io, sdbusplus::asio::object_server& objectServer, boost::container::flat_map<std::string, std::shared_ptr<HwmonTempSensor>>& sensors, std::shared_ptr<sdbusplus::asio::connection>& dbusConnection, const std::shared_ptr<boost::container::flat_set<std::string>>& - sensorsChanged) + sensorsChanged, + bool activateOnly) { auto getter = std::make_shared<GetSensorConfiguration>( dbusConnection, - [&io, &objectServer, &sensors, &dbusConnection, - sensorsChanged](const ManagedObjectType& sensorConfigurations) { + [&io, &objectServer, &sensors, &dbusConnection, sensorsChanged, + activateOnly](const ManagedObjectType& sensorConfigurations) { bool firstScan = sensorsChanged == nullptr; SensorConfigMap configMap = buildSensorConfigMap(sensorConfigurations); + boost::container::flat_map<std::string, std::shared_ptr<I2CDevice>> + newSensors = instantiateDevices(sensorConfigurations, sensors); + // IIO _raw devices look like this on sysfs: // /sys/bus/iio/devices/iio:device0/in_temp_raw // /sys/bus/iio/devices/iio:device0/in_temp_offset @@ -324,6 +397,18 @@ } const std::string& interfacePath = findSensorCfg->second.sensorPath; + auto findI2CDev = newSensors.find(interfacePath); + if (activateOnly && findI2CDev == newSensors.end()) + { + continue; + } + + std::shared_ptr<I2CDevice> i2cDev; + if (findI2CDev != newSensors.end()) + { + i2cDev = findI2CDev->second; + } + const SensorData& sensorData = findSensorCfg->second.sensorData; const std::string& sensorType = findSensorCfg->second.interface; const SensorBaseConfigMap& baseConfigMap = @@ -385,7 +470,10 @@ auto permitSet = getPermitSet(baseConfigMap); auto& sensor = sensors[sensorName]; - sensor = nullptr; + if (!activateOnly) + { + sensor = nullptr; + } auto hwmonFile = getFullHwmonFilePath(directory.string(), "temp1", permitSet); if (pathStr.starts_with("/sys/bus/iio/devices")) @@ -394,11 +482,19 @@ } if (hwmonFile) { - sensor = std::make_shared<HwmonTempSensor>( - *hwmonFile, sensorType, objectServer, dbusConnection, io, - sensorName, std::move(sensorThresholds), - thisSensorParameters, pollRate, interfacePath, readState); - sensor->setupRead(); + if (sensor != nullptr) + { + sensor->activate(*hwmonFile, i2cDev); + } + else + { + sensor = std::make_shared<HwmonTempSensor>( + *hwmonFile, sensorType, objectServer, dbusConnection, + io, sensorName, std::move(sensorThresholds), + thisSensorParameters, pollRate, interfacePath, + readState, i2cDev); + sensor->setupRead(); + } } hwmonName.erase( remove(hwmonName.begin(), hwmonName.end(), sensorName), @@ -440,13 +536,24 @@ } auto& sensor = sensors[sensorName]; - sensor = nullptr; - sensor = std::make_shared<HwmonTempSensor>( - *hwmonFile, sensorType, objectServer, dbusConnection, - io, sensorName, std::move(thresholds), - thisSensorParameters, pollRate, interfacePath, - readState); - sensor->setupRead(); + if (!activateOnly) + { + sensor = nullptr; + } + + if (sensor != nullptr) + { + sensor->activate(*hwmonFile, i2cDev); + } + else + { + sensor = std::make_shared<HwmonTempSensor>( + *hwmonFile, sensorType, objectServer, + dbusConnection, io, sensorName, + std::move(thresholds), thisSensorParameters, + pollRate, interfacePath, readState, i2cDev); + sensor->setupRead(); + } } hwmonName.erase( @@ -501,6 +608,29 @@ } } +static void powerStateChanged( + PowerState type, bool newState, + boost::container::flat_map<std::string, std::shared_ptr<HwmonTempSensor>>& + sensors, + boost::asio::io_service& io, sdbusplus::asio::object_server& objectServer, + std::shared_ptr<sdbusplus::asio::connection>& dbusConnection) +{ + if (newState) + { + createSensors(io, objectServer, sensors, dbusConnection, nullptr, true); + } + else + { + for (auto& [path, sensor] : sensors) + { + if (sensor != nullptr && sensor->readState == type) + { + sensor->deactivate(); + } + } + } +} + int main() { boost::asio::io_service io; @@ -514,8 +644,14 @@ auto sensorsChanged = std::make_shared<boost::container::flat_set<std::string>>(); + auto powerCallBack = [&sensors, &io, &objectServer, + &systemBus](PowerState type, bool state) { + powerStateChanged(type, state, sensors, io, objectServer, systemBus); + }; + setupPowerMatchCallback(systemBus, powerCallBack); + io.post([&]() { - createSensors(io, objectServer, sensors, systemBus, nullptr); + createSensors(io, objectServer, sensors, systemBus, nullptr, false); }); boost::asio::steady_timer filterTimer(io); @@ -541,7 +677,8 @@ std::cerr << "timer error\n"; return; } - createSensors(io, objectServer, sensors, systemBus, sensorsChanged); + createSensors(io, objectServer, sensors, systemBus, sensorsChanged, + false); }); };
diff --git a/src/HwmonTempSensor.cpp b/src/HwmonTempSensor.cpp index ee096ea..144f9d9 100644 --- a/src/HwmonTempSensor.cpp +++ b/src/HwmonTempSensor.cpp
@@ -27,6 +27,7 @@ #include <limits> #include <memory> #include <string> +#include <utility> #include <vector> // Temperatures are read in milli degrees Celsius, we need degrees Celsius. @@ -45,12 +46,13 @@ boost::asio::io_service& io, const std::string& sensorName, std::vector<thresholds::Threshold>&& thresholdsIn, const struct SensorParams& thisSensorParameters, const float pollRate, - const std::string& sensorConfiguration, const PowerState powerState) : + const std::string& sensorConfiguration, const PowerState powerState, + const std::shared_ptr<I2CDevice>& i2cDevice) : Sensor(boost::replace_all_copy(sensorName, " ", "_"), std::move(thresholdsIn), sensorConfiguration, objectType, false, false, thisSensorParameters.maxValue, thisSensorParameters.minValue, conn, powerState), - objServer(objectServer), + i2cDevice(i2cDevice), objServer(objectServer), inputDev(io, path, boost::asio::random_access_file::read_only), waitTimer(io), path(path), offsetValue(thisSensorParameters.offsetValue), scaleValue(thisSensorParameters.scaleValue), @@ -77,11 +79,35 @@ setInitialProperties(thisSensorParameters.units); } -HwmonTempSensor::~HwmonTempSensor() +bool HwmonTempSensor::isActive() { + return inputDev.is_open(); +} + +void HwmonTempSensor::activate(const std::string& newPath, + const std::shared_ptr<I2CDevice>& newI2CDevice) +{ + path = newPath; + i2cDevice = newI2CDevice; + inputDev.open(path, boost::asio::random_access_file::read_only); + markAvailable(true); + setupRead(); +} + +void HwmonTempSensor::deactivate() +{ + markAvailable(false); // close the input dev to cancel async operations inputDev.close(); waitTimer.cancel(); + i2cDevice = nullptr; + path = ""; +} + +HwmonTempSensor::~HwmonTempSensor() +{ + deactivate(); + for (const auto& iface : thresholdInterfaces) { objServer.remove_interface(iface);
diff --git a/src/meson.build b/src/meson.build index e420b27..e2af923 100644 --- a/src/meson.build +++ b/src/meson.build
@@ -82,6 +82,7 @@ 'HwmonTempSensor.cpp', dependencies: [ default_deps, + devicemgmt_dep, thresholds_dep, utils_dep, ],