RFC: Support pwm in phosphor-hwmon as fan target

[Lei YU]

On Wed, Dec 20, 2017 at 11:49 PM, Patrick Venture <venture at google.com> wrote:
> On Wed, Dec 20, 2017 at 6:09 AM, Lei YU <mine260309 at gmail.com> wrote:
>> The current phosphor-hwmon uses fanX_target for fan speed control. It expects
>> the fan driver to have `fanX_target` sysfs attribute, e.g. Witherspoon's
>> max31875 driver.
>> For the fans with `fanX_target`, phosphor-hwmon creates "Target" property on
>> the fan sensor Dbus object;
>> Then phosphor-fan-presence/control sets the "Target" property to do the
>> thermal control.
>>
>> Other systems like Romulus do not have this attribute, and it uses `pwmX` to
>> control the fan speed instead. This is not supported in phosphor-hwmon yet.
>>
>> To support such case, the proposal is to let phosphor-hwmon to create "Target"
>> property for the fans controlled by pwmX as well, via hwmon config file.
>>
>> E.g. for Romulus, fan9, fan11, fan13 are used, where fan9 and fan13 are
>> controlled by pwm1, and fan11 is controlled by pwm2, then the config looks
>> like:
>> ```
>> TARGET_fan9 = "pwm1"
>> TARGET_fan11 = "pwm2"
>> ```
>
> So we have a similar situation however, added a new interface to the
> sensor.value for FanPwm control, and then just add that target, but we
> were doing name matching identically to the fanspeed targets --
> instead of a configuration file setting up the matches.  However, it's
> a pretty trivial difference and easy to implement! :D  So, I like
> this.

Thanks for the comments!
I saw the FanPwm interface, adding a new interface for FanPwm is reasonable,
however, the current phosphor-fan-presence/control uses the interface
"xyz.openbmc_project.Control.FanSpeed" already.
If we have this new FanPwm interface, that part of code needs to be
updated as well.
Also I am not sure what happens if (in case) some fan object implements both
"FanSpeed" and "FanPwm" interface?

>
>>
>> * When phosphor-hwmon sees TARGET_fanX, it creates "Target" property for this
>> fan, and use the related pwmX to control the fan speed;
>> * For the fan control yaml configs, define the relted fan zone and control
>> logic, where the fan
>> * phosphor-fan-presence/control uses the same "Target" property to set the
>> fan speed target, only that the value is range from 0~255.
>>
>> The phosphor-hwmon changes are submitted at
>> https://gerrit.openbmc-project.xyz/#/c/8353/
>
> I reviewed your changes, and I'd actually like to merge our downstream
> patch and yours -- since I think the better way is to not overload
> fanspeed as something that can receive RPM or PWM, but rather have the
> different interfaces.  This is an important change because then you're
> not trying to interpret the value, or if someone wanted to add
> fanspeed and control via some other mechanism, they could - - warning,
> i haven't had caffeine yet so that might just be rambling.
>

Looking forward to your patches.
I wonder how do you config which fan to implement which FanPwm?

>>
>> Thanks
>>
>> --
>> BRs,
>> Lei YU
>
> I'm going to submit a couple patches that enable fanpwm partially to
> show another approach that I think should be merged with this
> approach. :D