| Message ID | 20170404192034.158901-3-john@jmuir.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
On Tue, Apr 04, 2017 at 12:20:34PM -0700, John Muir wrote: > Signed-off-by: John Muir <john@jmuir.com> > --- > .../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 000000000000..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-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Apr 10, 2017 at 10:42 AM, Rob Herring <robh@kernel.org> wrote: > On Tue, Apr 04, 2017 at 12:20:34PM -0700, John Muir wrote: >> Signed-off-by: John Muir <john@jmuir.com> >> --- >> .../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 000000000000..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-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> On Apr 10, 2017, at 8:42 AM, Rob Herring <robh@kernel.org> 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? > >> +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-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 11, 2017 at 06:47:28AM -0700, John Muir wrote: > > On Apr 10, 2017, at 8:42 AM, Rob Herring <robh@kernel.org> 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-hwmon" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/Documentation/devicetree/bindings/hwmon/max31760.txt b/Documentation/devicetree/bindings/hwmon/max31760.txt new file mode 100644 index 000000000000..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"; + };
Signed-off-by: John Muir <john@jmuir.com> --- .../devicetree/bindings/hwmon/max31760.txt | 58 ++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 Documentation/devicetree/bindings/hwmon/max31760.txt