From mboxrd@z Thu Jan 1 00:00:00 1970 From: Guenter Roeck Subject: Re: [v6,1/4] pmbus (core): Add fan control support Date: Wed, 29 Nov 2017 13:10:57 -0800 Message-ID: <20171129211056.GA21396@roeck-us.net> References: <3b782897affbb8ffe51d6b87d8c48fdcbb84e33e.1511152748.git-series.andrew@aj.id.au> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <3b782897affbb8ffe51d6b87d8c48fdcbb84e33e.1511152748.git-series.andrew@aj.id.au> Sender: linux-kernel-owner@vger.kernel.org To: Andrew Jeffery Cc: linux-hwmon@vger.kernel.org, jdelvare@suse.com, corbet@lwn.net, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, joel@jms.id.au, openbmc@lists.ozlabs.org List-Id: devicetree@vger.kernel.org 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 Appled (fixed whitespace problem). Thanks, Guenter