Re: Add phosphor-fan configuration for the fan which added by dbus-sensor

[Thu Nguyen <thu@amperemail.onmicrosoft.com>]

On 07/05/2021 23:22, Matthew Barth wrote:
> On 5/6/21 5:34 PM, Thu Nguyen OS wrote:
>> On 06/05/2021, 23:30, "Matthew Barth" <msbarth@linux.ibm.com> wrote:
>>
>>
>>      On 5/6/21 10:59 AM, Thu Nguyen OS wrote:
>>      >
>>      > On 06/05/2021, 20:09, "Matthew Barth" <msbarth@linux.ibm.com> wrote:
>>      >
>>      >
>>      >     On 5/6/21 4:22 AM, Thu Nguyen OS wrote:
>>      >     > Hi,
>>      >     >
>>      >     > I’m supporting the fan control algorithms using phosphor-fan. Phosphor-fan monitoring and controlling require interface xyz.openbmc_project.Control.FanPwm in the fan inventory object.
>>      >     That is a sensor interface, not an interface that's added to inventory objects
>>      >     > https://github.com/openbmc/phosphor-fan-presence/blob/master/docs/monitor/inventory.md
>>      >     What you linked to here refers to where the fan inventory object is located and nothing to do with with the fan sensors that fan monitor is to be monitoring.
>>      >
>>      > [Thu Nguyen] The link detail about the inventory configuration in monitor.yaml for fan monitor  and fan.yaml for fan control feature. It should be " The relative path under inventory where the fan's inventory object exists ".
>>
>>      The README links refer to the use of JSON config files in place of YAML. YAML based configs for the fan presence and fan monitor applications has been deprecated, so please use JSON based configs instead. Fan control is in the process of getting converted to JSON based configs so that it too can have its YAML based config support deprecated. To enable the use of JSON, supply "json" to the PACKAGECONFIG and to continue using YAML based fan control, also provide "--disable-json-control" until fan control's support of JSON is completed. i.e.) https://github.com/openbmc/openbmc/blob/master/meta-ibm/recipes-phosphor/fans/phosphor-fan_%25.bbappend#L59-L60
>>
>>      > Look at the example of the fan control configuration  file named fan.yaml below you can see the yaml file includes "inventory" setting. And xyz.openbmc_project.Control.FanPwm interface is required interface for that inventory path.
>>      > https://github.com/openbmc/phosphor-fan-presence/blob/master/control/example/fans.yaml
>>      Now to address what you're seeing, the "inventory" entry for fans.yaml is just used to name a fan object created within fan control and not related to anything actually in dbus inventory. This will become "name" in the JSON based (fans.json) for fan control.
>>      >     >
>>      >     > But FanSensor daemon of dbus-sensor only adds /xyz/openbmc_project/sensors/fan_tach to inventory list, and this path don’t support xyz.openbmc_project.Control.FanPwm interface.
>>      >     I cant speak to the support of dbus-sensors and the use of the FanPwm interface, but phosphor-fan-monitor can support either the use of RPM or PWM fan sensors for monitoring and updating the corresponding OperationalStatus of a fan in inventory.
>>      >     https://github.com/openbmc/phosphor-fan-presence/blob/master/docs/monitor/sensors.md
>>      >
>>      >     Supplying the ` "target_interface": "xyz.openbmc_project.Control.FanPwm" ` directs fan monitor to monitor the fan sensor using the Target property from that interface. When the "target_interface" is not provided on a fan sensor, the default is to use the "xyz.openbmc_project.Control.FanSpeed" (RPM) interface.
>>      > [Thu Nguyen]
>>      > The point here is the phosphor-fan-monitor/control expect xyz.openbmc_project.Control.FanPwm or xyz.openbmc_project.Control.FanSpeed is included in /xyz/openbmc_project/sensors/fan_tach.
>>      > But dbus-sensor don't support that. In dbus sensor xyz.openbmc_project.Control.FanPwm is only available in /xyz/openbmc_project/control/fanpwm.
>>      Ok, that clarifies things...and I welcome a proposed change to support the different dbus object paths in which the xyz.openbmc_project.Control.* interfaces that the fan sensors can be be at. Before dbus-sensors existed in openbmc, the fan sensors were provided by phosphor-hwmon on the dbus object path of /xyz/openbmc_project/sensors/fan_tach regardless of being PWM or RPM so that's why they are looking at that path.
>>
>> [Thu Nguyen]
>> How about add target_path option in fan.yaml and monitor.yaml to set the dbus object path?
>> Example:
>> target_path: /xyz/openbmc_project/control/fanpwm.
>> The setting of target_path will override value of "FAN_SENSOR_PATH" in fan.cpp and tach_sensor.hpp.
> Does dbus-sensors place the sensor value of the fan on the same dbus object path as the target? Fan monitor looks for the interfaces of both the target and the sensor value(feedback) on the same dbus object path.

[Thu]

1. No. Dbus-sensor provided three difference path for one fan:

/xyz/openbmc_project/control/fanpwm/<Name> have 
xyz.openbmc_project.Control.FanPwm.Target to control fan.

/xyz/openbmc_project/sensors/fan_tach/<Name> have fan tach value in 
xyz.openbmc_project.Sensor.Value.Value.

/xyz/openbmc_project/sensors/fan_pwm<Name> have fan Pwm value in 
xyz.openbmc_project.Sensor.Value.Value.


2. I tried to update fan control to support "target_path" option, but 
still have issue with fan monitor.


3. I wonder when I set has_target=false in monitor.yaml, the fan 
functional in inventory is set as false.

If don't have the fanpwm value, fan monitor should bypass checking fan 
failure in programming speed.

Is this right?

>>      >     >
>>      >     > Do you have any solution to make phosphor-fan work with dbus-sensor?
>>      >     >
>>      >     > I’m currently update FanSensor in dbus-sensor to add xyz.openbmc_project.Control.FanPwm to /xyz/openbmc_project/sensors/fan_tach.
>>      >     Refer to the doc link I provided above on how to utilize monitoring fans that use a PWM target on that interface. Then phosphor-fan-monitor will update the fan inventory object's (as configured with https://github.com/openbmc/phosphor-fan-presence/blob/master/docs/monitor/inventory.md) xyz.openbmc_project.State.Decorator.OperationalStatus Functional property based on how you configure fan monitor to monitor the fan sensors.
>>      >     >
>>      >     > Regards.
>>      >     > Thu Nguyen.
>>      >
>>