Re: [v6,1/4] pmbus (core): Add fan control support

[Guenter Roeck <linux@roeck-us.net>]

On Mon, Nov 20, 2017 at 03:12:03PM +1030, Andrew Jeffery wrote:
> Expose fanX_target, pwmX and pwmX_enable hwmon sysfs attributes.
> 
> Fans in a PMBus device are driven by the configuration of two registers,
> FAN_CONFIG_x_y and FAN_COMMAND_x: FAN_CONFIG_x_y dictates how the fan
> and the tacho operate (if installed), while FAN_COMMAND_x sets the
> desired fan rate. The unit of FAN_COMMAND_x is dependent on the
> operational fan mode, RPM or PWM percent duty, as determined by the
> corresponding configuration in FAN_CONFIG_x_y.
> 
> The mapping of fanX_target, pwmX and pwmX_enable onto FAN_CONFIG_x_y and
> FAN_COMMAND_x is implemented with the addition of virtual registers to
> facilitate the necessary side-effects of each access:
> 
> 1. PMBUS_VIRT_FAN_TARGET_x
> 2. PMBUS_VIRT_PWM_x
> 3. PMBUS_VIRT_PWM_ENABLE_x
> 
> Some complexity arises with the fanX_target and pwmX attributes both mapping
> onto FAN_COMMAND_x: There is no general mapping between PWM percent duty and
> RPM, so we can't display values in either attribute in terms of the other
> (which in my mind is the intuitive, if impossible, behaviour). This problem
> also affects the pwmX_enable attribute which allows userspace to switch between
> full speed, manual PWM and a number of automatic control modes, possibly
> including a switch to RPM behaviour (e.g. automatically adjusting PWM duty to
> reach a RPM target, the behaviour of fanX_target).
> 
> The next most intuitive behaviour is for fanX_target and pwmX to simply be
> independent, to retain their most recently set value even if that value is not
> active on the hardware (due to switching to the alternative control mode). This
> property of retaining the value independent of the hardware state has useful
> results for both userspace and the kernel: Userspace always sees a sensible
> value in the attribute (the last thing it was set to, as opposed to 0 or
> receiving an error on read), and the kernel can use the attributes as a value
> cache. This latter point eases the implementation of pwmX_enable, which can
> look up the associated pmbus_sensor object, take its cached value and apply it
> to hardware on changing control mode. This ensures we will not arbitrarily set
> a PWM value as an RPM value or vice versa, and we can assume that the RPM or
> PWM value set was sensible at least at some point in the past.
> 
> Finally, the DIRECT mode coefficients of some controllers is different between
> RPM and PWM percent duty control modes, so PSC_PWM is introduced to capture the
> necessary coefficients. As pmbus core had no PWM support previously PSC_FAN
> continues to be used to capture the RPM DIRECT coefficients, but in order to
> avoid falsely applying RPM scaling to PWM values I have introduced the
> PMBUS_HAVE_PWM12 and PMB_BUS_HAVE_PWM34 feature bits. These feature bits allow
> drivers to explicitly declare PWM support in order to have the attributes
> exposed.
> 
> Signed-off-by: Andrew Jeffery <andrew@aj.id.au>

Appled (fixed whitespace problem).

Thanks,
Guenter