openbmc / phosphor-pid-control

OpenBMC PID-based Thermal Control Daemon
Apache License 2.0
16 stars 21 forks source link

The problem when a fan set as input to multi zones, and the problem about read path configuration #19

Closed harveyquta closed 1 year ago

harveyquta commented 2 years ago

If using swampd/config.json setting, the sensorConfig map index is set by here, and when createDbusPassive class, the dbus propertiesChanged signal setting is related to type and id. This means the "name" in json config have to same as dbus sensor name. If the name setting is different from dbus sensor name, phosphor-pid-control will not do anything when readPath sensor propertiesChanged because of the wrong match.

If using entity-manager setting, there is another a little similar problem. In EM setting, the sensorConfig map index is set by dbus sensor name, so it is impossible to happen the wrong match. But if a fan is set as input to multi zones, the sensorConfig map has some problem. For example: fan0 set as input to zone0, zone1, and output pwm in zone0 is fan0_pwm, zone1 is fan2_pwm. The EM setting like below,

        {
            "Class": "fan",
            "FFGainCoefficient": 0.006,
            "FFOffCoefficient": 0.0,
            "ICoefficient": 0.0,
            "ILimitMax": 0.0,
            "ILimitMin": 0.0,
            "Inputs": [
                "fan0_tach"
            ],
            "Name": "pwm 0",
            "OutLimitMax": 100.0,
            "OutLimitMin": 25.0,
            "Outputs": [
                "fan0_pwm"
            ],
            "PCoefficient": 0.0,
            "SlewNeg": 0.0,
            "SlewPos": 0.0,
            "Type": "Pid",
            "Zones": [
                "Zone 0"
            ]
        },
        {
            "Class": "fan",
            "FFGainCoefficient": 0.006,
            "FFOffCoefficient": 0.0,
            "ICoefficient": 0.0,
            "ILimitMax": 0.0,
            "ILimitMin": 0.0,
            "Inputs": [
                "fan0_tach"
            ],
            "Name": "pwm 2",
            "OutLimitMax": 100.0,
            "OutLimitMin": 25.0,
            "Outputs": [
                "fan2_pwm"
            ],
            "PCoefficient": 0.0,
            "SlewNeg": 0.0,
            "SlewPos": 0.0,
            "Type": "Pid",
            "Zones": [
                "Zone 1"
            ]
        }

if fan setting transform into using swampd/config.json, will like below

        {
            "name": "fan0_tach",
            "type": "fan",
            "readPath": "/xyz/openbmc_project/sensors/fan_tach/fan0_tach",
            "writePath": "/xyz/openbmc_project/sensors/fan_tach/fan0_pwm",
            "min": 0,
            "max": 255
        },
        {
            "name": "fan0_tach",
            "type": "fan",
            "readPath": "/xyz/openbmc_project/sensors/fan_tach/fan0_tach",
            "writePath": "/xyz/openbmc_project/sensors/fan_tach/fan2_pwm",
            "min": 0,
            "max": 255
        }

Which means after building the sensorConfig map, only exist the second setting(fan0_tach -> fan2_pwm), the first setting(fan0_tach -> fan0_pwm) has been disappeared, the phosphor-pid-control only can set duty cycle to pwm2_tach, cannot set duty cycle to pwm0_tach.

harveyquta commented 1 year ago

fix in PR-51062 and PR-51063