| 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
| ||
|
Date: Mon, 14 Aug 2017 11:25:53 +0930
From: Andrew Jeffery <andrew@...id.au>
To: Rob Herring <robh@...nel.org>
Cc: linux@...ck-us.net, linux-hwmon@...r.kernel.org,
mark.rutland@....com, jdelvare@...e.com,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
joel@....id.au, mspinler@...ux.vnet.ibm.com,
msbarth@...ux.vnet.ibm.com, openbmc@...ts.ozlabs.org
Subject: Re: [PATCH v2 1/4] dt-bindings: hwmon: pmbus: Add Maxim MAX31785
documentation
On Thu, 2017-08-10 at 11:13 -0500, Rob Herring wrote:
> On Wed, Aug 02, 2017 at 04:45:38PM +0930, Andrew Jeffery wrote:
> > > > Signed-off-by: Andrew Jeffery <andrew@...id.au>
> > ---
> > .../devicetree/bindings/hwmon/pmbus/max31785.txt | 126 +++++++++++++++++++++
> > 1 file changed, 126 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt
> >
> > diff --git a/Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt b/Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt
> > new file mode 100644
> > index 000000000000..8ddc4ea1958d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/hwmon/pmbus/max31785.txt
> > @@ -0,0 +1,126 @@
> > +Bindings for the Maxim MAX31785 Intelligent Fan Controller
> > +==========================================================
>
> This is the second fan controller binding I've seen recently. The other
> being hwmon/aspeed-pwm-tacho.txt. I think some of this needs to be
> common. There's only so many types of fans, right?
Heh, you'd think so, I'll take a look. However much of this is driven by the
PMBus specification and the Aspeed PWM/Tacho isn't a PMBus-based device.
I was hesitant to start a generic PMBus bindings document without having more
PMBus devices with bindings, but maybe that's the way I should go? It would
make clear what's from the fan-control parts of the PMBus specification and
what's here for the purpose of supporting the MAX31785.
>
> And we have the thermal binding which you don't appear to tie into.
I'll look into that also.
>
> > +
> > +Reference:
> > +[1] https://datasheets.maximintegrated.com/en/ds/MAX31785.pdf
> > +
> > +Required properties:
> > +- compatible : One of "maxim,max31785" or "maxim,max31785a"
> > +- reg : I2C address, one of 0x52, 0x53, 0x54, 0x55.
> > +- #address-cells : Must be 1
> > +- #size-cells : Must be 0
> > +
> > +Optional properties:
> > +- use-stored-presence : Do not treat the devicetree description as canon for
>
> Is this maxim specific? If so, needs a vendor prefix.
PMBus specifies two 8-bit registers of the same structure: FAN_CONFIG_1_2
and FAN_CONFIG_3_4. It's not intended to be manufacturer-specific.
A bit in each nibble of FAN_CONFIG_* is specified as marking whether or not the
fan is installed. The intent of this property is to define whether the consumer
should consider the devicetree as canonical, or the device. In the absence of
the property consumer of the node should mark fans described in the devicetree
as installed (set the bit, and clear the bit for any fan pages that are not
described in the devicetree). Alternatively, the device may be pre-programmed
with a default register value that is suitable for the system the controller
resides in, in which case the devicetree consumer should itself consume the
installed bit from the device rather than set/clear it.
The two remaining fields in FAN_CONFIG_*, fan mode (RPM/PWM) and
tach-pulses-per-revolution (1-4), are covered by optional properties below.
>
> > + fan presence (the 'installed' bit of FAN_CONFIG_*).
> > + Instead, rely on the on the default value store of
> > + the device to populate it.
> > +
> > +Capabilities are configured through subnodes of the controller's node.
> > +
> > +Fans
> > +----
> > +
> > +Only fans with subnodes present will be considered as installed. If
> > +use-stored-presence is present in the parent node, then only fans that are both
> > +defined in the devicetree and have their installed bit set are considered
> > +installed.
> > +
> > +Required subnode properties:
> > +- compatible : Must be "pmbus-fan"
>
> What defines a pmbus-fan? Sounds generic, but then you have all these
> maxim specific properties.
It's driven by the two optional properties, fan-mode and tach-pulses, which
make up the remaining fields of the FAN_CONFIG_* registers from the PMBus
specification. PMBus reserves command ranges for manufacturer-specific use, and
Maxim chose to use one of these reserved commands as MFR_FAN_CONFIG
(Manufacturer-specific Fan Configuration). The vendor-prefixed properties deal
with the properties described in the 16-bit MFR_FAN_CONFIG register.
>
> > +- reg : The PMBus page the properties apply to.
> > +- maxim,fan-rotor-input : The type of rotor measurement provided to the
> > + controller. Must be either "tach" for tachometer
> > + pulses or "lock" for a locked-rotor signal.
> > +- maxim,fan-lock-polarity: Required iff maxim,fan-rotor-input is "lock". Valid
> > + values are "low" for active low, "high" for active
> > + high.
> > +
> > +Optional subnode properties:
> > +- fan-mode : "rpm" or "pwm". Default value is "pwm".
> > +- tach-pulses : Tachometer pulses per revolution. Valid values are
> > + 1, 2, 3 or 4. The default is 1.
> > +- maxim,fan-no-fault-ramp: Do not ramp the fan to 100% PWM duty on detecting a
> > + fan fault
> > +- maxim,fan-startup : The number of rotations required before taking
> > + emergency action for an unresponsive fan and driving
> > + it with 100% or 0% PWM duty, depending on the state
> > + of maxim,fan-no-fault-ramp. Valid values are 0
> > + (automatic spin-up disabled), 2, 4, or 8. Default
> > + value is 0.
> > +- maxim,fan-health : Enable automated fan health check
> > +- maxim,fan-ramp : Configures how fast the device ramps the PWM duty
> > + cycle from one value to another. Valid values are 0
> > + to 7 inclusive, with values 0 - 2 configuring a
> > + 1000ms update rate and 1 - 3% duty respective duty
> > + increase, and 3 - 7 a 200ms update rate with a 1 -
> > + 5% respective duty increase. Default value is 0.
> > +- maxim,fan-no-watchdog : Do not ramp fan to 100% PWM duty on failure to
> > + update desired fan rate inside 10s. This implies
> > + maxim,tmp-no-fault-ramp
> > +- maxim,tmp-no-fault-ramp: Do not ramp fan to 100% PWM duty on temperature
> > + sensor fault detection. This implies
> > + maxim,fan-no-watchdog
> > +- maxim,tmp-hysteresis : The temperature hysteresis used to determine
> > + transitions to lower fan speed bands in the
> > + temperature/fan rate lookup table. Valid values are
> > + 2, 4, 6 or 8 (degrees celcius). Default value is 2.
> > +- maxim,fan-dual-tach : Enable dual tachometer functionality
> > +- maxim,fan-pwm-freq : The PWM frequency. Valid values are 30, 50, 100, 150
> > + and 25000 (Hz). Default value is 30Hz.
> > +- maxim,fan-lookup-table : A 16-element cell array of alternating temperature
> > + and rate values representing the look up table. The
> > + rate units are set through the fan-mode property.
> > +- maxim,fan-fault-pin-mon: Ramp fans to 100% PWM duty when the FAULT pin is
> > + asserted
> > +
> > +Temperature
> > +-----------
> > +
> > +Required subnode properties:
> > +- compatible : Must be "pmbus-temperature"
> > +- reg : The PMBus page the properties apply to.
> > +
> > +Optional subnode properties:
> > +- maxim,tmp-offset : Valid values are 0 - 30 (degrees celcius) inclusive.
> > + Default value is 0.
> > +- maxim,tmp-fans : An array of fan indexes whose fans are controlled by
> > + the current temperature sensor. Fans are indexed from
> > + zero. The valid values are 0 - 5 inclusive.
>
> Why not use a phandle here.
I think that's a better approach. I'll rework it.
Thanks for the feedback.
Andrew
>
> > +
> > +Example:
> > > > > > + fan-max31785: max31785@52 {
> > > > + reg = <0x52>;
> > > > + compatible = "maxim,max31785";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > > > + fan@0 {
> > + compatible = "pmbus-fan";
> > + reg = <0>;
> > + mode = "rpm";
> > + tach-pulses = <1>;
> > + maxim,fan-rotor-input = "tach";
> > + maxim,fan-dual-tach;
> > + };
> > +
> > > > + fan@1 {
> > + compatible = "pmbus-fan";
> > + reg = <1>;
> > + mode = "rpm";
> > + tach-pulses = <1>;
> > + maxim,fan-rotor-input = "tach";
> > + maxim,tmp-hysteresis = <2>;
> > + maxim,fan-lookup-table = <
> > + /* Temperature RPM */
> > + 0 1000
> > + 10 2000
> > + 20 3000
> > + 30 4000
> > + 40 5000
> > + 50 6000
> > + 60 7000
> > + 70 8000
> > + >;
> > + };
> > > > + };
> > --
> > 2.11.0
> >
Download attachment "signature.asc" of type "application/pgp-signature" (802 bytes)
Powered by blists - more mailing lists