Skip to contentSkip to conversation

hwmon: ina3221: Add selective summation support

PATCH Superseded Jon Hunter 4 participants 12 messages Sep 21, 2023 – Sep 26, 2023

Subsystems (3)

v3 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Jon Hunter

author maintainer
#0 lore raw

v3 cover hwmon: ina3221: Add selective summation support

The current INA3221 driver always sums the shunt voltage for all enabled channels regardless of the shunt-resistor used for each channel. Summing the shunt-voltage for channels is only meaningful if the shunt resistor is the same for each channel. This series adds device-tree support to allow which channels are summed in device-tree.

Changes since V2:
- Added note to binding-doc to indicate that input channels must be explicitly disabled.
- Corrected ordering of properties in the binding-doc
- Updated license for the binding-doc to be dual licensed.
- Changed newly added property from 'summation-bypass' to summation-disable'.
- Documented type for the new 'summation-disable' property.
- Corrected spelling and comments as per the feedback received.
- Used debugfs instead of sysfs for exposing the 'summation-disable' status for each input channel.
- Populated missing instances for the ina3221 device for Tegra234 boards.
- Populated ina219 device for the NVIDIA IGX board (not strictly related to this series but related to populating all power-sensors for Tegra234 boards)

Changes since V1:
- Added yaml conversion patch for binding-doc
- Added binding-doc documentation patch for new property
- Added patch to populate ina3221 devices for Tegra234.

Jon Hunter (2):
dt-bindings: hwmon: ina3221: Add ti,summation-disable
arm64: tegra: Add power-sensors for Tegra234 boards

Ninad Malwade (2):
dt-bindings: hwmon: ina3221: Convert to json-schema
hwmon: ina3221: Add support for channel summation disable

 .../devicetree/bindings/hwmon/ina3221.txt     |  54 --------
 .../devicetree/bindings/hwmon/ti,ina3221.yaml | 117 ++++++++++++++++++
 .../boot/dts/nvidia/tegra234-p3701-0008.dtsi  |  33 +++++
 .../arm64/boot/dts/nvidia/tegra234-p3701.dtsi |  53 ++++++++
 .../arm64/boot/dts/nvidia/tegra234-p3767.dtsi |  29 +++++
 drivers/hwmon/ina3221.c                       |  30 ++++-
 6 files changed, 259 insertions(+), 57 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/hwmon/ina3221.txt
 create mode 100644 Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml
Signature · 1 line
2.34.1
author maintainer
#1 lore raw

PATCH v3 1/4 dt-bindings: hwmon: ina3221: Convert to json-schema

From: Ninad Malwade <[email protected]>

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.

---
 .../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
Documentation/devicetree/bindings/hwmon/ina3221.txt deleted +0 -54
deleted file mode 100644
index fa63b6171407..000000000000
@@ -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>;
-	};
-};
Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml new file +98 -0

98 lines not on this page · raw message

Signature · 1 line
2.34.1
maintainer
#2 lore raw

Re: [PATCH V3 1/4] dt-bindings: hwmon: ina3221: Convert to json-schema

Quoted 86 lines · Jon Hunter

From: Ninad Malwade <[email protected]>

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.

---
 .../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
Documentation/devicetree/bindings/hwmon/ina3221.txt deleted +0 -54
deleted file mode 100644
index fa63b6171407..000000000000
@@ -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>;
-	};
-};
Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml new file +1 -0
new file mode 100644
index 000000000000..d0e64a72af5b
@@ -0,0 +1,98 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)

Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml line 11

Quoted 10 lines
Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml new file +10 -0
+%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 <[email protected]>
+  - Guenter Roeck <[email protected]>

This should be someone with the h/w. You or the original author Nicolin (now Cc'ed)?

Quoted 13 lines
Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml new file +13 -0
+
+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

Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml line 34

Quoted 10 lines
Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml new file +10 -0
+      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

Quoted 40 lines
Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml new file +40 -0
+    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>;

Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml line 84

Quoted 10 lines
Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml new file +10 -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.

Quoted 17 lines
Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml new file +14 -0
+            };
+
+            input@1 {
+                reg = <0x1>;
+                shunt-resistor-micro-ohms = <5000>;
+            };
+
+            input@2 {
+                reg = <0x2>;
+                label = "VDD_5V";
+                shunt-resistor-micro-ohms = <5000>;
+            };
+        };
+    };
Signature · 1 line
2.34.1
author maintainer
#3 lore raw

Re: [PATCH V3 1/4] dt-bindings: hwmon: ina3221: Convert to json-schema

Quoted 19 lines · Rob Herring
Quoted 18 lines · Jon Hunter

From: Ninad Malwade <[email protected]>

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.

---
  .../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

...

Quoted 9 lines
Quoted 9 lines
Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml new file +9 -0
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        power-sensor@40 {
+            compatible = "ti,ina3221";
+            reg = <0x40>;
Quoted 8 lines

Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml line 84

Quoted 6 lines
Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml new file +6 -0
+            #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

Signature · 1 line
nvpublic
#4 lore raw

Re: [PATCH V3 1/4] dt-bindings: hwmon: ina3221: Convert to json-schema

Quoted 45 lines · Jon Hunter
Quoted 19 lines · Rob Herring
Quoted 18 lines · Jon Hunter

From: Ninad Malwade <[email protected]>

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.

---
  .../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

...

Quoted 9 lines
Quoted 9 lines
Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml new file +9 -0
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        power-sensor@40 {
+            compatible = "ti,ina3221";
+            reg = <0x40>;
Quoted 8 lines

Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml line 84

Quoted 6 lines
Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml new file +6 -0
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            input@0 {
+                reg = <0x0>;
+                status = "disabled";

Examples should be enabled.

Quoted 4 lines

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?

author maintainer
#5 lore raw

Re: [PATCH V3 1/4] dt-bindings: hwmon: ina3221: Convert to json-schema

Quoted 46 lines · Conor Dooley
Quoted 45 lines · Jon Hunter
Quoted 19 lines · Rob Herring
Quoted 18 lines · Jon Hunter

From: Ninad Malwade <[email protected]>

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.

---
   .../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

...

Quoted 17 lines
Quoted 5 lines
Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml new file +5 -0
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;

Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml line 84

Quoted 10 lines
Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml new file +10 -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.

Quoted 8 lines
Quoted 4 lines

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

Signature · 1 line
nvpublic
author maintainer
#6 lore raw

PATCH v3 2/4 dt-bindings: hwmon: ina3221: Add ti,summation-disable

In reply to Jon Hunter

The INA3221 has a critical alert pin that can be controlled by the summation control function. This function adds the single shunt-voltage conversions for the desired channels in order to compare the combined sum to the programmed limit. The Shunt-Voltage Sum Limit register contains the programmed value that is compared to the value in the Shunt-Voltage Sum register in order to determine if the total summed limit is exceeded. If the shunt-voltage sum limit value is exceeded, the critical alert pin pulls low.

For the summation limit to have a meaningful value, it is necessary to use the same shunt-resistor value on all included channels. Add a new vendor specific property, 'ti,summation-disable', to allow specific channels to be excluded from the summation control function if the shunt resistor is different to other channels or the channel should not be considered for triggering the critical alert pin.

---
 .../devicetree/bindings/hwmon/ti,ina3221.yaml | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
Documentation/devicetree/bindings/hwmon/ti,ina3221.yaml +19 -0
index d0e64a72af5b..8afbe729076f 100644
@@ -58,6 +58,25 @@ patternProperties:
       shunt-resistor-micro-ohms:
         description: shunt resistor value in micro-Ohm
 
+      ti,summation-disable:
+        description: |
+          The INA3221 has a critical alert pin that can be controlled by the
+          summation control function. This function adds the single
+          shunt-voltage conversions for the desired channels in order to
+          compare the combined sum to the programmed limit. The Shunt-Voltage
+          Sum Limit register contains the programmed value that is compared
+          to the value in the Shunt-Voltage Sum register in order to
+          determine if the total summed limit is exceeded. If the
+          shunt-voltage sum limit value is exceeded, the critical alert pin
+          is asserted.
+
+          For the summation limit to have a meaningful value, it is necessary
+          to use the same shunt-resistor value on all enabled channels. If
+          this is not the case or if a channel should not be used for
+          triggering the critical alert pin, then this property can be used
+          exclude specific channels from the summation control function.
+        type: boolean
+
     required:
       - reg
 
Signature · 1 line
2.34.1
maintainer
#7 lore raw

Re: [PATCH V3 2/4] dt-bindings: hwmon: ina3221: Add ti,summation-disable

Quoted 10 lines · Jon Hunter

The INA3221 has a critical alert pin that can be controlled by the summation control function. This function adds the single shunt-voltage conversions for the desired channels in order to compare the combined sum to the programmed limit. The Shunt-Voltage Sum Limit register contains the programmed value that is compared to the value in the Shunt-Voltage Sum register in order to determine if the total summed limit is exceeded. If the shunt-voltage sum limit value is exceeded, the critical alert pin pulls low.

Quoted 6 lines

For the summation limit to have a meaningful value, it is necessary to use the same shunt-resistor value on all included channels. Add a new vendor specific property, 'ti,summation-disable', to allow specific channels to be excluded from the summation control function if the shunt resistor is different to other channels or the channel should not be considered for triggering the critical alert pin.

You are adding this feature to the driver, but requiring a new property to disable it. So what happens with an existing user (old DT) and a kernel with the new feature?

Rob

author maintainer
#8 lore raw

Re: [PATCH V3 2/4] dt-bindings: hwmon: ina3221: Add ti,summation-disable

Quoted 18 lines · Rob Herring
Quoted 10 lines · Jon Hunter

The INA3221 has a critical alert pin that can be controlled by the summation control function. This function adds the single shunt-voltage conversions for the desired channels in order to compare the combined sum to the programmed limit. The Shunt-Voltage Sum Limit register contains the programmed value that is compared to the value in the Shunt-Voltage Sum register in order to determine if the total summed limit is exceeded. If the shunt-voltage sum limit value is exceeded, the critical alert pin pulls low.

Quoted 6 lines

For the summation limit to have a meaningful value, it is necessary to use the same shunt-resistor value on all included channels. Add a new vendor specific property, 'ti,summation-disable', to allow specific channels to be excluded from the summation control function if the shunt resistor is different to other channels or the channel should not be considered for triggering the critical alert pin.

Quoted 3 lines

You are adding this feature to the driver, but requiring a new property to disable it. So what happens with an existing user (old DT) and a kernel with the new feature?

Not exactly. The summation support has always been supported in the driver and is enabled (if the shunt resistors for all channels are the same). What we want to do is support summation but only for a subset of channels which is not supported today. So this new property allows us to explicitly tell the driver not to include a specific channel in the summation.

Jon

Signature · 1 line
nvpublic
author maintainer
#9 lore raw

PATCH v3 3/4 hwmon: ina3221: Add support for channel summation disable

In reply to Jon Hunter

From: Ninad Malwade <[email protected]>

The INA3221 allows the Critical alert pin to be controlled by the summation control function. This function adds the single shunt-voltage conversions for the desired channels in order to compare the combined sum to the programmed limit. The Shunt-Voltage Sum Limit register contains the programmed value that is compared to the value in the Shunt-Voltage Sum register in order to determine if the total summed limit is exceeded. If the shunt-voltage sum limit value is exceeded, the Critical alert pin pulls low.

For the summation limit to have a meaningful value, we have to use the same shunt-resistor value on all included channels. Unless equal shunt-resistor values are used for each channel, the summation control function cannot be used and it is not enabled by the driver.

To address this, add support to disable the summation of specific channels via device tree property "ti,summation-disable". The channel which has this property would be excluded from the calculation of summation control function.

For example, summation control function calculates Shunt-Voltage Sum as:

- input_shunt_voltage_summation = input_shunt_voltage_channel1
+ input_shunt_voltage_channel2
+ input_shunt_voltage_channel3

If we want the summation to only use channel1 and channel3, we can add 'ti,summation-disable' property in device tree node for channel2. Then the calculation will skip channel2.

- input_shunt_voltage_summation = input_shunt_voltage_channel1
+ input_shunt_voltage_channel3

Note that we only want the channel to be skipped for summation control function rather than completely disabled. Therefore, even if we add the property 'ti,summation-disable', the channel is still enabled and functional.

Finally, create debugfs entries that display if summation is disabled for each of the channels.

---
 drivers/hwmon/ina3221.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)
drivers/hwmon/ina3221.c +27 -3
index 5ab944056ec0..359b758e9f03 100644
@@ -6,6 +6,7 @@
  *	Andrew F. Davis <[email protected]>
  */
 
+#include <linux/debugfs.h>
 #include <linux/hwmon.h>
 #include <linux/hwmon-sysfs.h>
 #include <linux/i2c.h>
@@ -104,6 +105,7 @@ struct ina3221_input {
 	const char *label;
 	int shunt_resistor;
 	bool disconnected;
+	bool summation_disable;
 };
 
 /**
@@ -123,8 +125,10 @@ struct ina3221_data {
 	struct regmap_field *fields[F_MAX_FIELDS];
 	struct ina3221_input inputs[INA3221_NUM_CHANNELS];
 	struct mutex lock;
+	struct dentry *debugfs;
 	u32 reg_config;
 	int summation_shunt_resistor;
+	u32 summation_channel_control;
 
 	bool single_shot;
 };
@@ -154,7 +158,8 @@ static inline int ina3221_summation_shunt_resistor(struct ina3221_data *ina)
 	int i, shunt_resistor = 0;
 
 	for (i = 0; i < INA3221_NUM_CHANNELS; i++) {
-		if (input[i].disconnected || !input[i].shunt_resistor)
+		if (input[i].disconnected || !input[i].shunt_resistor ||
+		    input[i].summation_disable)
 			continue;
 		if (!shunt_resistor) {
 			/* Found the reference shunt resistor value */
@@ -786,6 +791,9 @@ static int ina3221_probe_child_from_dt(struct device *dev,
 	/* Save the connected input label if available */
 	of_property_read_string(child, "label", &input->label);
 
+	/* summation channel control */
+	input->summation_disable = of_property_read_bool(child, "ti,summation-disable");
+
 	/* Overwrite default shunt resistor value optionally */
 	if (!of_property_read_u32(child, "shunt-resistor-micro-ohms", &val)) {
 		if (val < 1 || val > INT_MAX) {
@@ -827,6 +835,7 @@ static int ina3221_probe(struct i2c_client *client)
 	struct device *dev = &client->dev;
 	struct ina3221_data *ina;
 	struct device *hwmon_dev;
+	char name[32];
 	int i, ret;
 
 	ina = devm_kzalloc(dev, sizeof(*ina), GFP_KERNEL);
@@ -873,6 +882,10 @@ static int ina3221_probe(struct i2c_client *client)
 
 	/* Initialize summation_shunt_resistor for summation channel control */
 	ina->summation_shunt_resistor = ina3221_summation_shunt_resistor(ina);
+	for (i = 0; i < INA3221_NUM_CHANNELS; i++) {
+		if (!ina->inputs[i].summation_disable)
+			ina->summation_channel_control |= BIT(14 - i);
+	}
 
 	ina->pm_dev = dev;
 	mutex_init(&ina->lock);
@@ -900,6 +913,15 @@ static int ina3221_probe(struct i2c_client *client)
 		goto fail;
 	}
 
+	scnprintf(name, sizeof(name), "%s-%s", INA3221_DRIVER_NAME, dev_name(dev));
+	ina->debugfs = debugfs_create_dir(name, NULL);
+
+	for (i = 0; i < INA3221_NUM_CHANNELS; i++) {
+		scnprintf(name, sizeof(name), "in%d_summation_disable", i);
+		debugfs_create_bool(name, 0400, ina->debugfs,
+				    &ina->inputs[i].summation_disable);
+	}
+
 	return 0;
 
 fail:
@@ -918,6 +940,8 @@ static void ina3221_remove(struct i2c_client *client)
 	struct ina3221_data *ina = dev_get_drvdata(&client->dev);
 	int i;
 
+	debugfs_remove_recursive(ina->debugfs);
+
 	pm_runtime_disable(ina->pm_dev);
 	pm_runtime_set_suspended(ina->pm_dev);
 
@@ -978,13 +1002,13 @@ static int ina3221_resume(struct device *dev)
 	/* Initialize summation channel control */
 	if (ina->summation_shunt_resistor) {
 		/*
-		 * Take all three channels into summation by default
+		 * Sum only channels that are not disabled for summation.
 		 * Shunt measurements of disconnected channels should
 		 * be 0, so it does not matter for summation.
 		 */
 		ret = regmap_update_bits(ina->regmap, INA3221_MASK_ENABLE,
 					 INA3221_MASK_ENABLE_SCC_MASK,
-					 INA3221_MASK_ENABLE_SCC_MASK);
+					 ina->summation_channel_control);
 		if (ret) {
 			dev_err(dev, "Unable to control summation channel\n");
 			return ret;
Signature · 1 line
2.34.1
#10 lore raw

Re: [PATCH V3 3/4] hwmon: ina3221: Add support for channel summation disable

kernel test robot noticed the following build warnings:

Hi Jon,

kernel test robot noticed the following build warnings:

[auto build test WARNING on groeck-staging/hwmon-next]
[also build test WARNING on robh/for-next linus/master v6.6-rc2 next-20230921] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jon-Hunter/dt-bindings-hwmon-ina3221-Add-ti-summation-disable/20230922-011119
base:   https://git.kernel.org/pub/scm/linux/kernel/git/groeck/linux-staging.git hwmon-next
patch link:    https://lore.kernel.org/r/20230921130818.21247-4-jonathanh%40nvidia.com
patch subject: [PATCH V3 3/4] hwmon: ina3221: Add support for channel summation disable
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20230922/[email protected]/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20230922/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All warnings (new ones prefixed by >>):

>> drivers/hwmon/ina3221.c:109: warning: Function parameter or member 'summation_disable' not described in 'ina3221_input'
>> drivers/hwmon/ina3221.c:134: warning: Function parameter or member 'debugfs' not described in 'ina3221_data'
>> drivers/hwmon/ina3221.c:134: warning: Function parameter or member 'summation_channel_control' not described in 'ina3221_data'

vim +109 drivers/hwmon/ina3221.c

7cb6dcff1956ec Andrew F. Davis 2016-06-10   97  
a9e9dd9c6de5d8 Nicolin Chen    2018-10-01   98  /**
a9e9dd9c6de5d8 Nicolin Chen    2018-10-01   99   * struct ina3221_input - channel input source specific information
a9e9dd9c6de5d8 Nicolin Chen    2018-10-01  100   * @label: label of channel input source
a9e9dd9c6de5d8 Nicolin Chen    2018-10-01  101   * @shunt_resistor: shunt resistor value of channel input source
a9e9dd9c6de5d8 Nicolin Chen    2018-10-01  102   * @disconnected: connection status of channel input source
a9e9dd9c6de5d8 Nicolin Chen    2018-10-01  103   */
a9e9dd9c6de5d8 Nicolin Chen    2018-10-01  104  struct ina3221_input {
a9e9dd9c6de5d8 Nicolin Chen    2018-10-01  105  	const char *label;
a9e9dd9c6de5d8 Nicolin Chen    2018-10-01  106  	int shunt_resistor;
a9e9dd9c6de5d8 Nicolin Chen    2018-10-01  107  	bool disconnected;
ac0a832fd3617c Ninad Malwade   2023-09-21  108  	bool summation_disable;
a9e9dd9c6de5d8 Nicolin Chen    2018-10-01 @109  };
a9e9dd9c6de5d8 Nicolin Chen    2018-10-01  110  
7cb6dcff1956ec Andrew F. Davis 2016-06-10  111  /**
7cb6dcff1956ec Andrew F. Davis 2016-06-10  112   * struct ina3221_data - device specific information
323aeb0eb5d9a6 Nicolin Chen    2018-11-05  113   * @pm_dev: Device pointer for pm runtime
7cb6dcff1956ec Andrew F. Davis 2016-06-10  114   * @regmap: Register map of the device
7cb6dcff1956ec Andrew F. Davis 2016-06-10  115   * @fields: Register fields of the device
a9e9dd9c6de5d8 Nicolin Chen    2018-10-01  116   * @inputs: Array of channel input source specific structures
87625b24986bc2 Nicolin Chen    2018-11-05  117   * @lock: mutex lock to serialize sysfs attribute accesses
59d608e152e582 Nicolin Chen    2018-09-29  118   * @reg_config: Register value of INA3221_CONFIG
2057bdfb7184e9 Nicolin Chen    2019-10-16  119   * @summation_shunt_resistor: equivalent shunt resistor value for summation
43dece162de047 Nicolin Chen    2019-01-17  120   * @single_shot: running in single-shot operating mode
7cb6dcff1956ec Andrew F. Davis 2016-06-10  121   */
7cb6dcff1956ec Andrew F. Davis 2016-06-10  122  struct ina3221_data {
323aeb0eb5d9a6 Nicolin Chen    2018-11-05  123  	struct device *pm_dev;
7cb6dcff1956ec Andrew F. Davis 2016-06-10  124  	struct regmap *regmap;
7cb6dcff1956ec Andrew F. Davis 2016-06-10  125  	struct regmap_field *fields[F_MAX_FIELDS];
a9e9dd9c6de5d8 Nicolin Chen    2018-10-01  126  	struct ina3221_input inputs[INA3221_NUM_CHANNELS];
87625b24986bc2 Nicolin Chen    2018-11-05  127  	struct mutex lock;
ac0a832fd3617c Ninad Malwade   2023-09-21  128  	struct dentry *debugfs;
59d608e152e582 Nicolin Chen    2018-09-29  129  	u32 reg_config;
2057bdfb7184e9 Nicolin Chen    2019-10-16  130  	int summation_shunt_resistor;
ac0a832fd3617c Ninad Malwade   2023-09-21  131  	u32 summation_channel_control;
43dece162de047 Nicolin Chen    2019-01-17  132  
43dece162de047 Nicolin Chen    2019-01-17  133  	bool single_shot;

7cb6dcff1956ec Andrew F. Davis 2016-06-10 @134 }; 7cb6dcff1956ec Andrew F. Davis 2016-06-10 135

Signature · 2 lines
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
author maintainer
#11 lore raw

PATCH v3 4/4 arm64: tegra: Add power-sensors for Tegra234 boards

In reply to Jon Hunter

Populate the ina219 and ina3221 power-sensors for the various Tegra234 boards. These sensors are located on the Tegra234 module boards and the configuration of some sensors is common across the different Tegra234 modules. Therefore, add any common sensor configurations to appropriate device tree source file so it can be re-used across modules.

---
 .../boot/dts/nvidia/tegra234-p3701-0008.dtsi  | 33 ++++++++++++
 .../arm64/boot/dts/nvidia/tegra234-p3701.dtsi | 53 +++++++++++++++++++
 .../arm64/boot/dts/nvidia/tegra234-p3767.dtsi | 29 ++++++++++
 3 files changed, 115 insertions(+)
arch/arm64/boot/dts/nvidia/tegra234-p3701-0008.dtsi +33 -0
index 62c4fdad0b60..553fa4ba1cd4 100644
@@ -44,6 +44,39 @@ i2c@c240000 {
 			status = "okay";
 		};
 
+		i2c@c250000 {
+			power-sensor@41 {
+				compatible = "ti,ina3221";
+				reg = <0x41>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				input@0 {
+					reg = <0x0>;
+					label = "CVB_ATX_12V";
+					shunt-resistor-micro-ohms = <2000>;
+				};
+
+				input@1 {
+					reg = <0x1>;
+					label = "CVB_ATX_3V3";
+					shunt-resistor-micro-ohms = <2000>;
+				};
+
+				input@2 {
+					reg = <0x2>;
+					label = "CVB_ATX_5V";
+					shunt-resistor-micro-ohms = <2000>;
+				};
+			};
+
+			power-sensor@44 {
+				compatible = "ti,ina219";
+				reg = <0x44>;
+				shunt-resistor = <2000>;
+			};
+		};
+
 		rtc@c2a0000 {
 			status = "okay";
 		};
arch/arm64/boot/dts/nvidia/tegra234-p3701.dtsi +53 -0

58 lines not on this page · raw message

arch/arm64/boot/dts/nvidia/tegra234-p3767.dtsi +29 -0
index fe08e131b7b9..59c14ded5e9f 100644
@@ -55,6 +55,35 @@ padctl@3520000 {
 			avdd-usb-supply = <&vdd_3v3_ao>;
 		};
 
+		i2c@c240000 {
+			status = "okay";
+
+			power-sensor@40 {
+				compatible = "ti,ina3221";
+				reg = <0x40>;
+				#address-cells = <1>;
+				#size-cells = <0>;
+
+				input@0 {
+					reg = <0x0>;
+					label = "VDD_IN";
+					shunt-resistor-micro-ohms = <5000>;
+				};
+
+				input@1 {
+					reg = <0x1>;
+					label = "VDD_CPU_GPU_CV";
+					shunt-resistor-micro-ohms = <5000>;
+				};
+
+				input@2 {
+					reg = <0x2>;
+					label = "VDD_SOC";
+					shunt-resistor-micro-ohms = <5000>;
+				};
+			};
+		};
+
 		rtc@c2a0000 {
 			status = "okay";
 		};
Signature · 1 line
2.34.1