Re: [PATCH 2/2] devicetree: Document the max31760 device binding.

[Guenter Roeck]

On Tue, Apr 11, 2017 at 06:47:28AM -0700, John Muir wrote:
> > On Apr 10, 2017, at 8:42 AM, Rob Herring  wrote:
> > 
> > On Tue, Apr 04, 2017 at 12:20:34PM -0700, John Muir wrote:
> >> +MAX31760 fan controller
> >> +---
> >> +
> >> +This device supports I2C only. Many properties of this device are 
> >> configurable
> >> +thorugh the hwmon interface. See also Documentation/hwmon/max31760.
> > 
> > I really think we need to describe the fans as separate nodes and 
> > preferably with a common binding. This is the second fan controller 
> > binding recently[1].
> > 
> > Features of the "hwmon interface" are not relevant to the binding. 
> > Bindings describe h/w.
> 
> It seems to me that referring to the hwmon interface is only helpful. You are 
> suggesting removing those sentences? If so, can I add a link to the data 
> sheet?
> 

Devicetree properties are supposed to be operating system independent.
Any mention of how access to the device is implemented on a given
operating system is out of scope for this document.

Guenter

> > 
> >> +Optional node properties:
> >> +- maxim,fan1-enabled  - 1 to enable, 0 to disable. Default: 1.
> >> +- maxim,fan2-enabled  - 1 to enable, 0 to disable. Default: 1.
> >> +- maxim,fan1-label- String: Hwmon fan1_label.
> >> +- maxim,fan2-label- String: Hwmon fan2_label.
> > 
> > Perhaps 2 fan sub nodes. reg for fan number, status for enabled, and 
> > label for label.
> 
> OK.
> 
> Right now a fan’s number of pulses and the PWM frequency are configured using 
> the hwmon sysfs interface (which defines standard controls for those), but as 
> those are characteristics of the hardware, should they also be configured via 
> the device tree binding?
> 
> >> +- maxim,pwm-zero-fan-can-fail - 0: Fan failure detection disabled 
> >> when PWM is
> >> +   ramping to 0%.
> >> +1: Fan failure detection enabled for all PWM
> >> +   values.
> >> +Default: 0.
> > 
> > All these can be boolean…
> 
> OK. The only issue I see is when the default is ‘true’ in the device, but 
> I’ll try to avoid that. Sometimes I wish that you could set a boolean to 
> false in DTS files.
> 
> > 
> >> +- maxim,temp1-label   - String: Hwmon temp1_label.
> >> +- maxim,temp2-label   - String: Hwmon temp2_label.
> >> +- maxim,temp2-ideality- Set ideality factor for the remote 
> >> temperature
> >> +sensor. Integer with range 0 to 63,
> >> +representing a multiplication factor of 0.9844
> >> +to 1.0489. Default: 24 (1.0080).
> > 
> > No maxim,temp1-ideality?
> No - the device only lets you set the ideality of the ‘external' temperature 
> sensor. I guess if there is an ideality for the internal temperature sensor, 
> it would be hard-wired as a characteristic of the part that was used.
> 
> > Not sure what to do with these, but perhaps 
> > also as sub-nodes. Surely we have some bindings already for devices with 
> > multiple temp sensors. Don't invent something custom here.
> 
> I’ll look into it.
> 
> What is the best way to distinguish between ‘fan’ and ‘temp’ sub-nodes? Do I 
> require a ‘compatible’ string?
> 
> Thanks!
> 
> John.
> 
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] devicetree: Document the max31760 device binding.

[Rob Herring]

On Mon, Apr 10, 2017 at 10:42 AM, Rob Herring  wrote:
> On Tue, Apr 04, 2017 at 12:20:34PM -0700, John Muir wrote:
>> Signed-off-by: John Muir 
>> ---
>>  .../devicetree/bindings/hwmon/max31760.txt | 58 
>> ++
>>  1 file changed, 58 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/hwmon/max31760.txt
>>
>> diff --git a/Documentation/devicetree/bindings/hwmon/max31760.txt 
>> b/Documentation/devicetree/bindings/hwmon/max31760.txt
>> new file mode 100644
>> index ..43787a77c322
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/hwmon/max31760.txt
>> @@ -0,0 +1,58 @@
>> +MAX31760 fan controller
>> +---
>> +
>> +This device supports I2C only. Many properties of this device are 
>> configurable
>> +thorugh the hwmon interface. See also Documentation/hwmon/max31760.
>
> I really think we need to describe the fans as separate nodes and
> preferably with a common binding. This is the second fan controller
> binding recently[1].

Forgot the link: https://patchwork.kernel.org/patch/9643643/
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] devicetree: Document the max31760 device binding.

[Rob Herring]

On Tue, Apr 04, 2017 at 12:20:34PM -0700, John Muir wrote:
> Signed-off-by: John Muir 
> ---
>  .../devicetree/bindings/hwmon/max31760.txt | 58 
> ++
>  1 file changed, 58 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/hwmon/max31760.txt
> 
> diff --git a/Documentation/devicetree/bindings/hwmon/max31760.txt 
> b/Documentation/devicetree/bindings/hwmon/max31760.txt
> new file mode 100644
> index ..43787a77c322
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/hwmon/max31760.txt
> @@ -0,0 +1,58 @@
> +MAX31760 fan controller
> +---
> +
> +This device supports I2C only. Many properties of this device are 
> configurable
> +thorugh the hwmon interface. See also Documentation/hwmon/max31760.

I really think we need to describe the fans as separate nodes and 
preferably with a common binding. This is the second fan controller 
binding recently[1].

Features of the "hwmon interface" are not relevant to the binding. 
Bindings describe h/w.

> +
> +Required node properties:
> +- compatible : "maxim,max31760"
> +- reg : The I2C address of the device. This is 0x50 - 0x57 depending on the
> + hardware configuration.
> +
> +Optional node properties:
> +- maxim,fan1-enabled - 1 to enable, 0 to disable. Default: 1.
> +- maxim,fan2-enabled - 1 to enable, 0 to disable. Default: 1.
> +- maxim,fan1-label   - String: Hwmon fan1_label.
> +- maxim,fan2-label   - String: Hwmon fan2_label.

Perhaps 2 fan sub nodes. reg for fan number, status for enabled, and 
label for label.

> +- maxim,fan-fail-full-only   - Set to 1 to assert a fan failure only when the
> +   PWM is at 100%. Default: 0.

Make this a boolean.

> +- maxim,fan-rd-signal- Set to 1 if fan(s) provide a rotation
> +   detection (RD) signal, or 0 if the fan
> +   generates square-wave pulses. Default: 0.

Make this a boolean.

> +- maxim,fan-rd-polarity  - 0: RD is low when the fan is running.
> +   1: RD is high when the fan is running.
> +   Only relevant when fan-rd-signal is 1.
> +   Default: 0.
> +- maxim,fan-signal-enabled   - Set to 1 if externally driving FF/FS low
> +   should force PWM output to 100%. Default: 0.
> +- maxim,fan-spin-up-enabled - For fan startup: Set to 1 to set the PWM to
> +   100% until tach is detected or two seconds
> +   have passed before reducing to the target
> +   value. Default: 0.
> +- maxim,pwm-polarity - 0: 100% PWM is when PWM is high.
> +   1: 100% PWM is when PWM is low.
> +   Default: 0.
> +- maxim,pwm-pulse-stretch-enabled
> + - 1 to enable PWM pulse stretching, 0 to
> +   disable. Default: 0.
> +- maxim,pwm-zero-fan-can-fail- 0: Fan failure detection disabled 
> when PWM is
> +  ramping to 0%.
> +   1: Fan failure detection enabled for all PWM
> +  values.
> +   Default: 0.

All these can be boolean...

> +- maxim,temp1-label  - String: Hwmon temp1_label.
> +- maxim,temp2-label  - String: Hwmon temp2_label.
> +- maxim,temp2-ideality   - Set ideality factor for the remote 
> temperature
> +   sensor. Integer with range 0 to 63,
> +   representing a multiplication factor of 0.9844
> +   to 1.0489. Default: 24 (1.0080).

No maxim,temp1-ideality? Not sure what to do with these, but perhaps 
also as sub-nodes. Surely we have some bindings already for devices with 
multiple temp sensors. Don't invent something custom here.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] devicetree: Document the max31760 device binding.

[John Muir]

Signed-off-by: John Muir 
---
 .../devicetree/bindings/hwmon/max31760.txt | 58 ++
 1 file changed, 58 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/hwmon/max31760.txt

diff --git a/Documentation/devicetree/bindings/hwmon/max31760.txt 
b/Documentation/devicetree/bindings/hwmon/max31760.txt
new file mode 100644
index ..43787a77c322
--- /dev/null
+++ b/Documentation/devicetree/bindings/hwmon/max31760.txt
@@ -0,0 +1,58 @@
+MAX31760 fan controller
+---
+
+This device supports I2C only. Many properties of this device are configurable
+thorugh the hwmon interface. See also Documentation/hwmon/max31760.
+
+Required node properties:
+- compatible : "maxim,max31760"
+- reg : The I2C address of the device. This is 0x50 - 0x57 depending on the
+   hardware configuration.
+
+Optional node properties:
+- maxim,fan1-enabled   - 1 to enable, 0 to disable. Default: 1.
+- maxim,fan2-enabled   - 1 to enable, 0 to disable. Default: 1.
+- maxim,fan1-label - String: Hwmon fan1_label.
+- maxim,fan2-label - String: Hwmon fan2_label.
+- maxim,fan-fail-full-only - Set to 1 to assert a fan failure only when the
+ PWM is at 100%. Default: 0.
+- maxim,fan-rd-signal  - Set to 1 if fan(s) provide a rotation
+ detection (RD) signal, or 0 if the fan
+ generates square-wave pulses. Default: 0.
+- maxim,fan-rd-polarity- 0: RD is low when the fan is running.
+ 1: RD is high when the fan is running.
+ Only relevant when fan-rd-signal is 1.
+ Default: 0.
+- maxim,fan-signal-enabled - Set to 1 if externally driving FF/FS low
+ should force PWM output to 100%. Default: 0.
+- maxim,fan-spin-up-enabled - For fan startup: Set to 1 to set the PWM to
+ 100% until tach is detected or two seconds
+ have passed before reducing to the target
+ value. Default: 0.
+- maxim,pwm-polarity   - 0: 100% PWM is when PWM is high.
+ 1: 100% PWM is when PWM is low.
+ Default: 0.
+- maxim,pwm-pulse-stretch-enabled
+   - 1 to enable PWM pulse stretching, 0 to
+ disable. Default: 0.
+- maxim,pwm-zero-fan-can-fail  - 0: Fan failure detection disabled when PWM is
+ramping to 0%.
+ 1: Fan failure detection enabled for all PWM
+values.
+ Default: 0.
+- maxim,temp1-label- String: Hwmon temp1_label.
+- maxim,temp2-label- String: Hwmon temp2_label.
+- maxim,temp2-ideality - Set ideality factor for the remote temperature
+ sensor. Integer with range 0 to 63,
+ representing a multiplication factor of 0.9844
+ to 1.0489. Default: 24 (1.0080).
+
+Example:
+   max31760@50 {
+   compatible = "maxim,max31760";
+   reg = <0x50>;
+   maxim,fan1-label = "Left";
+   maxim,fan2-label = "Right";
+   maxim,fan-spin-up-enabled = <1>;
+   maxim,temp2-label = "CPU";
+   };
-- 
2.12.2.715.g7642488e1d-goog

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html