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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171129211056.GA21396@roeck-us.net>
Date:   Wed, 29 Nov 2017 13:10:57 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     Andrew Jeffery <andrew@...id.au>
Cc:     linux-hwmon@...r.kernel.org, jdelvare@...e.com, corbet@....net,
        devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-doc@...r.kernel.org, joel@....id.au, openbmc@...ts.ozlabs.org
Subject: Re: [v6,1/4] pmbus (core): Add fan control support

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@...id.au>

Appled (fixed whitespace problem).

Thanks,
Guenter