| Message ID | 20230921130818.21247-2-jonathanh@nvidia.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | hwmon: ina3221: Add selective summation support | expand |
On Thu, Sep 21, 2023 at 02:08:15PM +0100, Jon Hunter wrote: > From: Ninad Malwade <nmalwade@nvidia.com> > > Convert the TI INA3221 bindings from the free-form text format to > json-schema. > > Note that the INA3221 input channels default to enabled in the chip. > Unless channels are explicitly disabled in device-tree, input > channels will be enabled. > > Signed-off-by: Thierry Reding <treding@nvidia.com> > Signed-off-by: Ninad Malwade <nmalwade@nvidia.com> > Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > --- > .../devicetree/bindings/hwmon/ina3221.txt | 54 ---------- > .../devicetree/bindings/hwmon/ti,ina3221.yaml | 98 +++++++++++++++++++ > 2 files changed, 98 insertions(+), 54 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/hwmon/ina3221.txt > create mode 100644 Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml > > diff --git a/Documentation/devicetree/bindings/hwmon/ina3221.txt b/Documentation/devicetree/bindings/hwmon/ina3221.txt > deleted file mode 100644 > index fa63b6171407..000000000000 > --- a/Documentation/devicetree/bindings/hwmon/ina3221.txt > +++ /dev/null > @@ -1,54 +0,0 @@ > -Texas Instruments INA3221 Device Tree Bindings > - > -1) ina3221 node > - Required properties: > - - compatible: Must be "ti,ina3221" > - - reg: I2C address > - > - Optional properties: > - - ti,single-shot: This chip has two power modes: single-shot (chip takes one > - measurement and then shuts itself down) and continuous ( > - chip takes continuous measurements). The continuous mode is > - more reliable and suitable for hardware monitor type device, > - but the single-shot mode is more power-friendly and useful > - for battery-powered device which cares power consumptions > - while still needs some measurements occasionally. > - If this property is present, the single-shot mode will be > - used, instead of the default continuous one for monitoring. > - > - = The node contains optional child nodes for three channels = > - = Each child node describes the information of input source = > - > - - #address-cells: Required only if a child node is present. Must be 1. > - - #size-cells: Required only if a child node is present. Must be 0. > - > -2) child nodes > - Required properties: > - - reg: Must be 0, 1 or 2, corresponding to IN1, IN2 or IN3 port of INA3221 > - > - Optional properties: > - - label: Name of the input source > - - shunt-resistor-micro-ohms: Shunt resistor value in micro-Ohm > - > -Example: > - > -ina3221@40 { > - compatible = "ti,ina3221"; > - reg = <0x40>; > - #address-cells = <1>; > - #size-cells = <0>; > - > - input@0 { > - reg = <0x0>; > - status = "disabled"; > - }; > - input@1 { > - reg = <0x1>; > - shunt-resistor-micro-ohms = <5000>; > - }; > - input@2 { > - reg = <0x2>; > - label = "VDD_5V"; > - shunt-resistor-micro-ohms = <5000>; > - }; > -}; > diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml > new file mode 100644 > index 000000000000..d0e64a72af5b > --- /dev/null > +++ b/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml > @@ -0,0 +1,98 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/hwmon/ti,ina3221.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Texas Instruments INA3221 Current and Voltage Monitor > + > +maintainers: > + - Jean Delvare <jdelvare@suse.com> > + - Guenter Roeck <linux@roeck-us.net> This should be someone with the h/w. You or the original author Nicolin (now Cc'ed)? > + > +properties: > + compatible: > + const: ti,ina3221 > + > + reg: > + maxItems: 1 > + > + ti,single-shot: > + description: | > + This chip has two power modes: single-shot (chip takes one measurement > + and then shuts itself down) and continuous (chip takes continuous > + measurements). The continuous mode is more reliable and suitable for > + hardware monitor type device, but the single-shot mode is more power- > + friendly and useful for battery-powered device which cares power > + consumptions while still needs some measurements occasionally. > + > + If this property is present, the single-shot mode will be used, instead > + of the default continuous one for monitoring. > + $ref: /schemas/types.yaml#/definitions/flag > + > + "#address-cells": > + description: Required only if a child node is present. That's always the case. Drop > + const: 1 > + > + "#size-cells": > + description: Required only if a child node is present. > + const: 0 > + > +patternProperties: > + "^input@[0-2]$": > + description: The node contains optional child nodes for three channels. > + Each child node describes the information of input source. Input channels > + default to enabled in the chip. Unless channels are explicitly disabled > + in device-tree, input channels will be enabled. > + type: object > + additionalProperties: false > + properties: > + reg: > + description: Must be 0, 1 and 2, corresponding to the IN1, IN2 or IN3 > + ports of the INA3221, respectively. > + enum: [ 0, 1, 2 ] > + > + label: > + description: name of the input source > + > + shunt-resistor-micro-ohms: > + description: shunt resistor value in micro-Ohm > + > + required: > + - reg > + > +required: > + - compatible > + - reg > + > +additionalProperties: false > + > +examples: > + - | > + i2c { > + #address-cells = <1>; > + #size-cells = <0>; > + > + power-sensor@40 { > + compatible = "ti,ina3221"; > + reg = <0x40>; > + #address-cells = <1>; > + #size-cells = <0>; > + > + input@0 { > + reg = <0x0>; > + status = "disabled"; Examples should be enabled. > + }; > + > + input@1 { > + reg = <0x1>; > + shunt-resistor-micro-ohms = <5000>; > + }; > + > + input@2 { > + reg = <0x2>; > + label = "VDD_5V"; > + shunt-resistor-micro-ohms = <5000>; > + }; > + }; > + }; > -- > 2.34.1 >
On 22/09/2023 22:01, Rob Herring wrote: > On Thu, Sep 21, 2023 at 02:08:15PM +0100, Jon Hunter wrote: >> From: Ninad Malwade <nmalwade@nvidia.com> >> >> Convert the TI INA3221 bindings from the free-form text format to >> json-schema. >> >> Note that the INA3221 input channels default to enabled in the chip. >> Unless channels are explicitly disabled in device-tree, input >> channels will be enabled. >> >> Signed-off-by: Thierry Reding <treding@nvidia.com> >> Signed-off-by: Ninad Malwade <nmalwade@nvidia.com> >> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> >> --- >> .../devicetree/bindings/hwmon/ina3221.txt | 54 ---------- >> .../devicetree/bindings/hwmon/ti,ina3221.yaml | 98 +++++++++++++++++++ >> 2 files changed, 98 insertions(+), 54 deletions(-) >> delete mode 100644 Documentation/devicetree/bindings/hwmon/ina3221.txt >> create mode 100644 Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml ... >> +examples: >> + - | >> + i2c { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + power-sensor@40 { >> + compatible = "ti,ina3221"; >> + reg = <0x40>; >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + input@0 { >> + reg = <0x0>; >> + status = "disabled"; > > Examples should be enabled. Yes normally that would be the case. However, per the discussion with Guenter and the comment in the changelog, for this device channels are enabled in the chip by default. And so to disable them, we need to explicitly disable in device-tree. Jon
On Mon, Sep 25, 2023 at 11:46:58AM +0100, Jon Hunter wrote: > > > On 22/09/2023 22:01, Rob Herring wrote: > > On Thu, Sep 21, 2023 at 02:08:15PM +0100, Jon Hunter wrote: > > > From: Ninad Malwade <nmalwade@nvidia.com> > > > > > > Convert the TI INA3221 bindings from the free-form text format to > > > json-schema. > > > > > > Note that the INA3221 input channels default to enabled in the chip. > > > Unless channels are explicitly disabled in device-tree, input > > > channels will be enabled. > > > > > > Signed-off-by: Thierry Reding <treding@nvidia.com> > > > Signed-off-by: Ninad Malwade <nmalwade@nvidia.com> > > > Signed-off-by: Jon Hunter <jonathanh@nvidia.com> > > > --- > > > .../devicetree/bindings/hwmon/ina3221.txt | 54 ---------- > > > .../devicetree/bindings/hwmon/ti,ina3221.yaml | 98 +++++++++++++++++++ > > > 2 files changed, 98 insertions(+), 54 deletions(-) > > > delete mode 100644 Documentation/devicetree/bindings/hwmon/ina3221.txt > > > create mode 100644 Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml > > > ... > > > > +examples: > > > + - | > > > + i2c { > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + power-sensor@40 { > > > + compatible = "ti,ina3221"; > > > + reg = <0x40>; > > > + #address-cells = <1>; > > > + #size-cells = <0>; > > > + > > > + input@0 { > > > + reg = <0x0>; > > > + status = "disabled"; > > > > Examples should be enabled. > > > Yes normally that would be the case. However, per the discussion with > Guenter and the comment in the changelog, for this device channels are > enabled in the chip by default. And so to disable them, we need to > explicitly disable in device-tree. Maybe a comment at this location would be good then, to point out that this is what you are trying to demonstrate with this example?
On 26/09/2023 10:18, Conor Dooley wrote: > On Mon, Sep 25, 2023 at 11:46:58AM +0100, Jon Hunter wrote: >> >> >> On 22/09/2023 22:01, Rob Herring wrote: >>> On Thu, Sep 21, 2023 at 02:08:15PM +0100, Jon Hunter wrote: >>>> From: Ninad Malwade <nmalwade@nvidia.com> >>>> >>>> Convert the TI INA3221 bindings from the free-form text format to >>>> json-schema. >>>> >>>> Note that the INA3221 input channels default to enabled in the chip. >>>> Unless channels are explicitly disabled in device-tree, input >>>> channels will be enabled. >>>> >>>> Signed-off-by: Thierry Reding <treding@nvidia.com> >>>> Signed-off-by: Ninad Malwade <nmalwade@nvidia.com> >>>> Signed-off-by: Jon Hunter <jonathanh@nvidia.com> >>>> --- >>>> .../devicetree/bindings/hwmon/ina3221.txt | 54 ---------- >>>> .../devicetree/bindings/hwmon/ti,ina3221.yaml | 98 +++++++++++++++++++ >>>> 2 files changed, 98 insertions(+), 54 deletions(-) >>>> delete mode 100644 Documentation/devicetree/bindings/hwmon/ina3221.txt >>>> create mode 100644 Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml >> >> >> ... >> >>>> +examples: >>>> + - | >>>> + i2c { >>>> + #address-cells = <1>; >>>> + #size-cells = <0>; >>>> + >>>> + power-sensor@40 { >>>> + compatible = "ti,ina3221"; >>>> + reg = <0x40>; >>>> + #address-cells = <1>; >>>> + #size-cells = <0>; >>>> + >>>> + input@0 { >>>> + reg = <0x0>; >>>> + status = "disabled"; >>> >>> Examples should be enabled. >> >> >> Yes normally that would be the case. However, per the discussion with >> Guenter and the comment in the changelog, for this device channels are >> enabled in the chip by default. And so to disable them, we need to >> explicitly disable in device-tree. > > Maybe a comment at this location would be good then, to point out that > this is what you are trying to demonstrate with this example? > OK yes I think that that will help convey that this is intentional in this case. Thanks Jon
diff --git a/Documentation/devicetree/bindings/hwmon/ina3221.txt b/Documentation/devicetree/bindings/hwmon/ina3221.txt deleted file mode 100644 index fa63b6171407..000000000000 --- a/Documentation/devicetree/bindings/hwmon/ina3221.txt +++ /dev/null @@ -1,54 +0,0 @@ -Texas Instruments INA3221 Device Tree Bindings - -1) ina3221 node - Required properties: - - compatible: Must be "ti,ina3221" - - reg: I2C address - - Optional properties: - - ti,single-shot: This chip has two power modes: single-shot (chip takes one - measurement and then shuts itself down) and continuous ( - chip takes continuous measurements). The continuous mode is - more reliable and suitable for hardware monitor type device, - but the single-shot mode is more power-friendly and useful - for battery-powered device which cares power consumptions - while still needs some measurements occasionally. - If this property is present, the single-shot mode will be - used, instead of the default continuous one for monitoring. - - = The node contains optional child nodes for three channels = - = Each child node describes the information of input source = - - - #address-cells: Required only if a child node is present. Must be 1. - - #size-cells: Required only if a child node is present. Must be 0. - -2) child nodes - Required properties: - - reg: Must be 0, 1 or 2, corresponding to IN1, IN2 or IN3 port of INA3221 - - Optional properties: - - label: Name of the input source - - shunt-resistor-micro-ohms: Shunt resistor value in micro-Ohm - -Example: - -ina3221@40 { - compatible = "ti,ina3221"; - reg = <0x40>; - #address-cells = <1>; - #size-cells = <0>; - - input@0 { - reg = <0x0>; - status = "disabled"; - }; - input@1 { - reg = <0x1>; - shunt-resistor-micro-ohms = <5000>; - }; - input@2 { - reg = <0x2>; - label = "VDD_5V"; - shunt-resistor-micro-ohms = <5000>; - }; -}; diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml new file mode 100644 index 000000000000..d0e64a72af5b --- /dev/null +++ b/Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml @@ -0,0 +1,98 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/hwmon/ti,ina3221.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Texas Instruments INA3221 Current and Voltage Monitor + +maintainers: + - Jean Delvare <jdelvare@suse.com> + - Guenter Roeck <linux@roeck-us.net> + +properties: + compatible: + const: ti,ina3221 + + reg: + maxItems: 1 + + ti,single-shot: + description: | + This chip has two power modes: single-shot (chip takes one measurement + and then shuts itself down) and continuous (chip takes continuous + measurements). The continuous mode is more reliable and suitable for + hardware monitor type device, but the single-shot mode is more power- + friendly and useful for battery-powered device which cares power + consumptions while still needs some measurements occasionally. + + If this property is present, the single-shot mode will be used, instead + of the default continuous one for monitoring. + $ref: /schemas/types.yaml#/definitions/flag + + "#address-cells": + description: Required only if a child node is present. + const: 1 + + "#size-cells": + description: Required only if a child node is present. + const: 0 + +patternProperties: + "^input@[0-2]$": + description: The node contains optional child nodes for three channels. + Each child node describes the information of input source. Input channels + default to enabled in the chip. Unless channels are explicitly disabled + in device-tree, input channels will be enabled. + type: object + additionalProperties: false + properties: + reg: + description: Must be 0, 1 and 2, corresponding to the IN1, IN2 or IN3 + ports of the INA3221, respectively. + enum: [ 0, 1, 2 ] + + label: + description: name of the input source + + shunt-resistor-micro-ohms: + description: shunt resistor value in micro-Ohm + + required: + - reg + +required: + - compatible + - reg + +additionalProperties: false + +examples: + - | + i2c { + #address-cells = <1>; + #size-cells = <0>; + + power-sensor@40 { + compatible = "ti,ina3221"; + reg = <0x40>; + #address-cells = <1>; + #size-cells = <0>; + + input@0 { + reg = <0x0>; + status = "disabled"; + }; + + input@1 { + reg = <0x1>; + shunt-resistor-micro-ohms = <5000>; + }; + + input@2 { + reg = <0x2>; + label = "VDD_5V"; + shunt-resistor-micro-ohms = <5000>; + }; + }; + };