Fan PWM settings via Redfish
Ed Tanous
edtanous at google.com
Sat Mar 13 04:40:20 AEDT 2021
On Thu, Mar 11, 2021 at 10:37 PM Bruce Lee (李昀峻) <Bruce_Lee at quantatw.com> wrote:
>
> Hi All,
>
>
>
> We are designing and implementing the Fan PWM settings via Redfish. The goal is that clients can set sensor value to bmc via Redfish.
>
>
>
> We divide the work into three phases.
>
>
>
> Phase 1 is to remove the definition “BMCWEB_INSECURE_UNRESTRICTED_SENSOR_OVERRIDE” and use new definition to “BMCWEB_SPECIAL_MODE_SENSOR_OVERRIDE”.
>
> The “BMCWEB_INSECURE_UNRESTRICTED_SENSOR_OVERRIDE” was added by Intel group, please refer to https://gerrit.openbmc-project.xyz/c/openbmc/bmcweb/+/30000,
>
> The Intel solution has 4 conditions needs to match one of them and that can be work to override sensor but actually not all project needs those conditions, so we want to propose to remove the insecure definition and use new definition to include the intel solution and execute when compile. It would be no compile time with option for common project. And the insecure issue we will discuss in phase 2.
>
>
>
> Example below:
>
> -----------------------------------------------------------------------------------------------------
>
> [Before modified]
>
> #ifdef BMCWEB_INSECURE_UNRESTRICTED_SENSOR_OVERRIDE
>
> // Proceed with sensor override
>
> setSensorsOverride(sensorAsyncResp, allCollections);
>
> return;
>
> #endif
>
> doIntelSpecialModeManager code …
>
> -----------------------------------------------------------------------------------------------------
>
> [After modified]
>
> #ifdef BMCWEB_SPECIAL_MODE_SENSOR_OVERRIDE
>
> doIntelSpecialModeManager code …
>
> return;
>
> #endif
>
> //Proceed with sensor override
>
> setSensorsOverride(sensorAsyncResp, allCollections);
>
> -----------------------------------------------------------------------------------------------------
>
>
>
>
I suspect this check and option needs to be moved into the individual
sensors, so that we can differentiate between "should be settable in a
test context" and "should be settable in a normal context".
>
> Phase 2 is to add a condition to check the sensor name’s Mutable value of EM if the value is true do the sensor override function else not do.
I suspect this patchset needs to be moved forward if you're hoping to
use the mutable param:
https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-dbus-interfaces/+/36333
>
> The Mutable value can be set in the sensor configuration of Entity-Manage, when using the patch command to override the sensor, it needs to check the EntityManager subtree’s sensor name and its interface “xyz.openbmc_project.Configuration.I2CFan.Connector” to check the corresponding property name’s mutable value to decide whether executing the override function.
See above. I suspect that the redfish code doesn't need to check the
mutability of the sensor, the interface should just have the correct
behavior. The only place I would expect to need to know the
mutability of a sensor is in the IPMI sdr, where we will need to set
the modifiable bit appropriately.
>
> This achieves feature parity with the ipmi::sensor::Mutability parameter of the old hardcoded YAML configuration files
Not sure what you're referring to. That may have been something done in a fork.
>
>
>
> Execute steps:
>
> 1. Patch command to override sensor.
>
> 2. Check the EM of sensor’s Mutable value
>
> 3. If Mutable value is true do sensor override action else not do.
>
>
>
>
>
> Phase 3 is to add a new get command to get the Zone_$id’s "Manual" value and patch command to change the fan mode from auto to manual mode ("Manual":true).
>
> Because the fan control is use package phosphor-pid-control, when we need to set fan pwm, it needs to set the fan mode from auto mode to manual mode, for now, the phosphor-pid-control has already provided ipmi-oem command to achieve this feature, so we need to implement this fan mode change via redfish command.
Doesn't this already work today? I thought we had all that sorted a
long time ago. For some reason I thought we intentionally didn't
expose the manual/automatic param, because that only applied to the
PID loops, and PWM sensor didn't expose that interface. I need to go
look at the code at some point.
>
>
>
> Example URLs |Method |Example Payload
>
> --------------------------------------- |-------------- |--
>
> /redfish/v1/Managers/bmc |GET |"Oem": {
>
> | | Fan": {
>
> | | "FanZones": {
>
> | | "@odata.id": "/redfish/v1/Managers/bmc#/Oem/OpenBmc/Fan/FanZones",
>
> | | "@odata.type": "#OemManager.FanZones",
>
> | | "Zone_0": {
>
> | | "@odata.id": "/redfish/v1/Managers/bmc#/Oem/OpenBmc/Fan/FanZones/Zone_0",
>
> | | "@odata.type": "#OemManager.FanZone",
>
> | | "Chassis": {
>
> | | "@odata.id": "/redfish/v1/Chassis/GSZ_EVT"
>
> | | },
>
> | | "FailSafePercent": 100.0,
>
> | | "MinThermalOutput": 0.0,
>
> | | "ZoneIndex": 0.0,
>
> | | "Manual":false
>
> | | },
>
> | | },
>
> | | },
>
> | |}
>
> --------------------------------------- |-------------- |----
>
> /redfish/v1/Managers/bmc | PATCH |"Oem": { "Fan": { "FanZones": { "Zone_0": { "Manual":true } } }
>
>
It should be noted, this schema needs some serious cleanup to make it
proper resources, paths, and collections, and should version the
schema files appropriately. If you're planning on extending it, I
would expect _some_ effort to be put into cleanup. There's several
github bugs that have more details, and I will leave it up to you to
decide how much you'd like to do as part of this work, but please plan
on some.
>
> If any thoughts on this topic, feel free to give your comments. Thanks!
>
>
>
> Sincerely,
>
> Bruce
More information about the openbmc
mailing list