openbmc / phosphor-pid-control

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

Cannot output 2 pwm signal in one zone #16

Closed minimada closed 4 years ago

minimada commented 4 years ago

Hi all, I write a config for entity manager which can be loaded by phosphor pid control like: { "Class": "fan", ... "Inputs": [ "Fan1", "Fan2" ], ... "Outputs": [ "Pwm_1", "Pwm2" ], ... "Type": "Pid", "Zones": [ "Zone1" ] } I need use single temperature sensor to control two pwm signal. But I found the sensor config would only set one pwm in one zone as following code dbus/dbusconfiguration.cpp : else if (sensorPathIfacePair.second == pwmInterface) { // copy so we can modify it for (std::string otherSensor : sensorNames) { std::replace(otherSensor.begin(), otherSensor.end(), ' ', ''); if (sensorPathIfacePair.first.find(otherSensor) != std::string::npos) { continue; }

                        auto& config = sensorConfig[otherSensor];
                        config.writePath = sensorPathIfacePair.first;
                        // todo: un-hardcode this if there are fans with
                        // different ranges
                        config.max = 255;
                        config.min = 0;
                    }
                }

Is this original design limitation?

feistjj commented 4 years ago

You can only have 1 output for a fan, do you have 2 pwms that both can drive fan 1 and fan 2? I assume you want 2 Fan classes in the one zone. Something like https://github.com/openbmc/entity-manager/blob/ac09fe44dba113aacee1d2fd995e137c0d5ea18e/configurations/R2000%20Chassis.json#L225

minimada commented 4 years ago

Yes, there are four fan socket on my test board. And four PWM signal and fanin for each socket. I think the PID controller can control all fans with each PWM signal. Maybe the problem is dbus configuration cannot set multiple PWM signal output in one zone?

feistjj commented 4 years ago

I don't believe 2 Pwm outputs was ever tested, this is probably something that just wasn't considered.

minimada commented 4 years ago

Do you think this case should be handled when dbus configuration set up sensor config?

feistjj commented 4 years ago

I'm still a bit confused why 1 tach would have 2 pwm that control it

minimada commented 4 years ago

Actually, there are two real fans in a zone. And I would like to control each fan with each PWM signal on the test board.

feistjj commented 4 years ago

Sure, that would be as the example I linked above. But each fan has only 1 pwm, they are part of the same zone though.