Re: RFC: Support pwm in phosphor-hwmon as fan target

[Patrick Venture <venture@google.com>]

On Wed, Dec 20, 2017 at 6:09 AM, Lei YU <mine260309@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.

>
> * 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.

>
> 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