linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/4] hwmon: Add support for INA780
@ 2025-08-29  3:05 Chris Packham
  2025-08-29  3:05 ` [PATCH v3 1/4] dt-bindings: hwmon: ti,ina2xx: Add INA780 device Chris Packham
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Chris Packham @ 2025-08-29  3:05 UTC (permalink / raw)
  To: jdelvare, linux, robh, krzk+dt, conor+dt, corbet, wenliang202407,
	jre
  Cc: linux-hwmon, devicetree, linux-kernel, linux-doc, Chris Packham

This series adds support for the INA780 to the existing ina238.c driver.

v2: https://lore.kernel.org/linux-hwmon/20250808030510.552724-1-chris.packham@alliedtelesis.co.nz/
v1: https://lore.kernel.org/linux-hwmon/20250806005127.542298-1-chris.packham@alliedtelesis.co.nz/

One important bit of feedback I've not addressed is this

> Follow-up: I ordered evaluation boards for INA228, INA237, INA238, and INA780A.
> I'll want to see support added for current limits on all chips, using a similar
> approach as the one in the ina2xx driver. After all, the shunt voltage limits
> are really current limits in disguise. With that and appropriate chip specific
> parameters the differences between the chips should become relatively minor.
>
> This should also simplify adding support for INA700 which seems similar to INA780A.

I wasn't sure if that meant you were going to look at adding current limits to
the existing chips and I should wait or if you wanted me to try based on the
code. Given our timezone differences I figured I'd send the series anyway.
Patch 2 is a bugfix that you might want to pick up sooner.

Chris Packham (4):
  dt-bindings: hwmon: ti,ina2xx: Add INA780 device
  hwmon: (ina238) Correctly clamp temperature
  hwmon: (ina238) Add ina238_config fields
  hwmon: (ina238) Add support for INA780

 .../devicetree/bindings/hwmon/ti,ina2xx.yaml  |   1 +
 Documentation/hwmon/ina238.rst                |  20 ++
 drivers/hwmon/ina238.c                        | 273 ++++++++++++++----
 3 files changed, 238 insertions(+), 56 deletions(-)

-- 
2.51.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v3 1/4] dt-bindings: hwmon: ti,ina2xx: Add INA780 device
  2025-08-29  3:05 [PATCH v3 0/4] hwmon: Add support for INA780 Chris Packham
@ 2025-08-29  3:05 ` Chris Packham
  2025-08-31 22:35   ` Guenter Roeck
  2025-08-29  3:05 ` [PATCH v3 2/4] hwmon: (ina238) Correctly clamp temperature Chris Packham
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Chris Packham @ 2025-08-29  3:05 UTC (permalink / raw)
  To: jdelvare, linux, robh, krzk+dt, conor+dt, corbet, wenliang202407,
	jre
  Cc: linux-hwmon, devicetree, linux-kernel, linux-doc, Chris Packham

Add a compatible string for the INA780 device.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
Acked-by: Conor Dooley <conor.dooley@microchip.com>
---

Notes:
    Changes in v3:
    - Collect ack from Conor

 Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
index fa68b99ef2e2..980ecba6d811 100644
--- a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
+++ b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
@@ -32,6 +32,7 @@ properties:
       - ti,ina237
       - ti,ina238
       - ti,ina260
+      - ti,ina780
 
   reg:
     maxItems: 1
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 2/4] hwmon: (ina238) Correctly clamp temperature
  2025-08-29  3:05 [PATCH v3 0/4] hwmon: Add support for INA780 Chris Packham
  2025-08-29  3:05 ` [PATCH v3 1/4] dt-bindings: hwmon: ti,ina2xx: Add INA780 device Chris Packham
@ 2025-08-29  3:05 ` Chris Packham
  2025-08-29  9:55   ` Guenter Roeck
  2025-08-31 22:34   ` Guenter Roeck
  2025-08-29  3:05 ` [PATCH v3 3/4] hwmon: (ina238) Add ina238_config fields Chris Packham
  2025-08-29  3:05 ` [PATCH v3 4/4] hwmon: (ina238) Add support for INA780 Chris Packham
  3 siblings, 2 replies; 11+ messages in thread
From: Chris Packham @ 2025-08-29  3:05 UTC (permalink / raw)
  To: jdelvare, linux, robh, krzk+dt, conor+dt, corbet, wenliang202407,
	jre
  Cc: linux-hwmon, devicetree, linux-kernel, linux-doc, Chris Packham

ina238_write_temp() was attempting to clamp the user input but was
throwing away the result. Ensure that we clamp the value to the
appropriate range before it is converted into a register value.

Fixes: 0d9f596b1fe3 ("hwmon: (ina238) Modify the calculation formula to adapt to different chips")
Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v3:
    - New. Split off bugfix from main patch

 drivers/hwmon/ina238.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index 5a394eeff676..4d3dc018ead9 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -572,7 +572,7 @@ static int ina238_write_temp(struct device *dev, u32 attr, long val)
 		return -EOPNOTSUPP;
 
 	/* Signed */
-	regval = clamp_val(val, -40000, 125000);
+	val = clamp_val(val, -40000, 125000);
 	regval = div_s64(val * 10000, data->config->temp_lsb) << data->config->temp_shift;
 	regval = clamp_val(regval, S16_MIN, S16_MAX) & (0xffff << data->config->temp_shift);
 
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 3/4] hwmon: (ina238) Add ina238_config fields
  2025-08-29  3:05 [PATCH v3 0/4] hwmon: Add support for INA780 Chris Packham
  2025-08-29  3:05 ` [PATCH v3 1/4] dt-bindings: hwmon: ti,ina2xx: Add INA780 device Chris Packham
  2025-08-29  3:05 ` [PATCH v3 2/4] hwmon: (ina238) Correctly clamp temperature Chris Packham
@ 2025-08-29  3:05 ` Chris Packham
  2025-08-29 15:56   ` Guenter Roeck
  2025-08-29  3:05 ` [PATCH v3 4/4] hwmon: (ina238) Add support for INA780 Chris Packham
  3 siblings, 1 reply; 11+ messages in thread
From: Chris Packham @ 2025-08-29  3:05 UTC (permalink / raw)
  To: jdelvare, linux, robh, krzk+dt, conor+dt, corbet, wenliang202407,
	jre
  Cc: linux-hwmon, devicetree, linux-kernel, linux-doc, Chris Packham

In preparation for adding INA780 support add some required fields to
ina238_config and set the appropriate values for the existing chips.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v3:
    - New. Split config struct changes from main patch

 drivers/hwmon/ina238.c | 22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index 4d3dc018ead9..930e12e64079 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -120,13 +120,17 @@ enum ina238_ids { ina238, ina237, sq52206, ina228 };
 
 struct ina238_config {
 	bool has_20bit_voltage_current; /* vshunt, vbus and current are 20-bit fields */
+	bool has_shunt;			/* has shunt resistor */
 	bool has_power_highest;		/* chip detection power peak */
 	bool has_energy;		/* chip detection energy */
+	bool has_curr_min_max;		/* supports COL/CUL */
 	u8 temp_shift;			/* fixed parameters for temp calculate */
 	u32 power_calculate_factor;	/* fixed parameters for power calculate */
 	u16 config_default;		/* Power-on default state */
 	int bus_voltage_lsb;		/* use for temperature calculate, uV/lsb */
 	int temp_lsb;			/* use for temperature calculate */
+	int temp_max;			/* maximum configurable temp limit in mC */
+	int fixed_power_lsb;		/* fixed power LSB value */
 };
 
 struct ina238_data {
@@ -141,43 +145,59 @@ struct ina238_data {
 static const struct ina238_config ina238_config[] = {
 	[ina238] = {
 		.has_20bit_voltage_current = false,
+		.has_shunt = true,
 		.has_energy = false,
 		.has_power_highest = false,
+		.has_curr_min_max = false,
 		.temp_shift = 4,
 		.power_calculate_factor = 20,
 		.config_default = INA238_CONFIG_DEFAULT,
 		.bus_voltage_lsb = INA238_BUS_VOLTAGE_LSB,
 		.temp_lsb = INA238_DIE_TEMP_LSB,
+		.temp_max = 125000,
+		.fixed_power_lsb = 0,
 	},
 	[ina237] = {
 		.has_20bit_voltage_current = false,
+		.has_shunt = true,
 		.has_energy = false,
 		.has_power_highest = false,
+		.has_curr_min_max = false,
 		.temp_shift = 4,
 		.power_calculate_factor = 20,
 		.config_default = INA238_CONFIG_DEFAULT,
 		.bus_voltage_lsb = INA238_BUS_VOLTAGE_LSB,
 		.temp_lsb = INA238_DIE_TEMP_LSB,
+		.temp_max = 125000,
+		.fixed_power_lsb = 0,
 	},
 	[sq52206] = {
 		.has_20bit_voltage_current = false,
+		.has_shunt = true,
 		.has_energy = true,
 		.has_power_highest = true,
+		.has_curr_min_max = false,
 		.temp_shift = 0,
 		.power_calculate_factor = 24,
 		.config_default = SQ52206_CONFIG_DEFAULT,
 		.bus_voltage_lsb = SQ52206_BUS_VOLTAGE_LSB,
 		.temp_lsb = SQ52206_DIE_TEMP_LSB,
+		.temp_max = 125000,
+		.fixed_power_lsb = 0,
 	},
 	[ina228] = {
 		.has_20bit_voltage_current = true,
+		.has_shunt = true,
 		.has_energy = true,
 		.has_power_highest = false,
+		.has_curr_min_max = false,
 		.temp_shift = 0,
 		.power_calculate_factor = 20,
 		.config_default = INA238_CONFIG_DEFAULT,
 		.bus_voltage_lsb = INA238_BUS_VOLTAGE_LSB,
 		.temp_lsb = INA228_DIE_TEMP_LSB,
+		.temp_max = 125000,
+		.fixed_power_lsb = 0,
 	},
 };
 
@@ -572,7 +592,7 @@ static int ina238_write_temp(struct device *dev, u32 attr, long val)
 		return -EOPNOTSUPP;
 
 	/* Signed */
-	val = clamp_val(val, -40000, 125000);
+	val = clamp_val(val, -40000, data->config->temp_max);
 	regval = div_s64(val * 10000, data->config->temp_lsb) << data->config->temp_shift;
 	regval = clamp_val(regval, S16_MIN, S16_MAX) & (0xffff << data->config->temp_shift);
 
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v3 4/4] hwmon: (ina238) Add support for INA780
  2025-08-29  3:05 [PATCH v3 0/4] hwmon: Add support for INA780 Chris Packham
                   ` (2 preceding siblings ...)
  2025-08-29  3:05 ` [PATCH v3 3/4] hwmon: (ina238) Add ina238_config fields Chris Packham
@ 2025-08-29  3:05 ` Chris Packham
  3 siblings, 0 replies; 11+ messages in thread
From: Chris Packham @ 2025-08-29  3:05 UTC (permalink / raw)
  To: jdelvare, linux, robh, krzk+dt, conor+dt, corbet, wenliang202407,
	jre
  Cc: linux-hwmon, devicetree, linux-kernel, linux-doc, Chris Packham

Add support for the TI INA780 Digital Power Monitor. The INA780 uses
EZShunt(tm) technology, which means there are fixed LSB conversions for
a number of fields rather than needing to be calibrated.

Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
---

Notes:
    Changes in v3:
    - Rebase on top of master and resolve conflicts with commit fd470f4ed80c
      ("hwmon: (ina238) Add support for INA228")
    - Use INA238_SHUNT_OVER/UNDER_VOLTAGE register definitions with comment
      about COL/CUL.
    - Move some code to prepartory patches

 Documentation/hwmon/ina238.rst |  20 +++
 drivers/hwmon/ina238.c         | 251 +++++++++++++++++++++++++--------
 2 files changed, 216 insertions(+), 55 deletions(-)

diff --git a/Documentation/hwmon/ina238.rst b/Documentation/hwmon/ina238.rst
index 9a24da4786a4..220d36d5c947 100644
--- a/Documentation/hwmon/ina238.rst
+++ b/Documentation/hwmon/ina238.rst
@@ -14,6 +14,11 @@ Supported chips:
     Datasheet:
 	https://www.ti.com/lit/gpn/ina238
 
+  * Texas Instruments INA780
+
+    Datasheet:
+	https://www.ti.com/product/ina780a
+
   * Silergy SQ52206
 
     Prefix: 'SQ52206'
@@ -69,3 +74,18 @@ energy1_input		Energy measurement (uJ)
 
 power1_input_highest	Peak Power (uW)
 ======================= =======================================================
+
+Sysfs differences for ina780
+----------------------------
+
+======================= =======================================================
+in0_input		Bus voltage (mV)
+in0_min			Minimum bus voltage threshold (mV)
+in0_min_alarm		Minimum shunt voltage alarm
+in0_max			Maximum bus voltage threshold (mV)
+in0_max_alarm		Maximum shunt voltage alarm
+curr1_max		Maximum current threshold
+curr1_max_alarm		Maximum current alarm
+curr1_min		Minimum current threshold
+curr1_min_alarm		Minimum current alarm
+======================= =======================================================
diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index 930e12e64079..ad1b36762ca3 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -2,6 +2,7 @@
 /*
  * Driver for Texas Instruments INA238 power monitor chip
  * Datasheet: https://www.ti.com/product/ina238
+ *            https://www.ti.com/product/ina780a
  *
  * Copyright (C) 2021 Nathan Rossi <nathan.rossi@digi.com>
  */
@@ -31,8 +32,8 @@
 #define SQ52206_ENERGY			0x9
 #define SQ52206_CHARGE			0xa
 #define INA238_DIAG_ALERT		0xb
-#define INA238_SHUNT_OVER_VOLTAGE	0xc
-#define INA238_SHUNT_UNDER_VOLTAGE	0xd
+#define INA238_SHUNT_OVER_VOLTAGE	0xc /* COL on INA780 */
+#define INA238_SHUNT_UNDER_VOLTAGE	0xd /* CUL on INA780 */
 #define INA238_BUS_OVER_VOLTAGE		0xe
 #define INA238_BUS_UNDER_VOLTAGE	0xf
 #define INA238_TEMP_LIMIT		0x10
@@ -45,8 +46,8 @@
 #define SQ52206_CONFIG_ADCRANGE_LOW	BIT(3)
 
 #define INA238_DIAG_ALERT_TMPOL		BIT(7)
-#define INA238_DIAG_ALERT_SHNTOL	BIT(6)
-#define INA238_DIAG_ALERT_SHNTUL	BIT(5)
+#define INA238_DIAG_ALERT_SHNTOL	BIT(6) /* CURRENTOL on INA780 */
+#define INA238_DIAG_ALERT_SHNTUL	BIT(5) /* CURRENTUL on INA780 */
 #define INA238_DIAG_ALERT_BUSOL		BIT(4)
 #define INA238_DIAG_ALERT_BUSUL		BIT(3)
 #define INA238_DIAG_ALERT_POL		BIT(2)
@@ -110,13 +111,16 @@
 #define SQ52206_DIE_TEMP_LSB		78125 /* 7.8125 mC/lsb */
 #define INA228_DIE_TEMP_LSB		78125 /* 7.8125 mC/lsb */
 
+#define INA780_CURRENT_LSB		2400	/* 2.4 mA/lsb  */
+#define INA780_ENERGY_LSB		7680	/* 7.68 mJ/lsb */
+
 static const struct regmap_config ina238_regmap_config = {
 	.max_register = INA238_REGISTERS,
 	.reg_bits = 8,
 	.val_bits = 16,
 };
 
-enum ina238_ids { ina238, ina237, sq52206, ina228 };
+enum ina238_ids { ina238, ina237, sq52206, ina228, ina780 };
 
 struct ina238_config {
 	bool has_20bit_voltage_current; /* vshunt, vbus and current are 20-bit fields */
@@ -199,6 +203,19 @@ static const struct ina238_config ina238_config[] = {
 		.temp_max = 125000,
 		.fixed_power_lsb = 0,
 	},
+	[ina780] = {
+		.has_20bit_voltage_current = false,
+		.has_shunt = false,
+		.has_energy = true,
+		.has_power_highest = false,
+		.temp_shift = 4,
+		.config_default = INA238_CONFIG_DEFAULT,
+		.bus_voltage_lsb = INA238_BUS_VOLTAGE_LSB,
+		.temp_lsb = INA238_DIE_TEMP_LSB,
+		.temp_max = 150000,
+		.fixed_power_lsb = 480,
+		.has_curr_min_max = true,
+	}
 };
 
 static int ina238_read_reg24(const struct i2c_client *client, u8 reg, u32 *val)
@@ -298,12 +315,12 @@ static int ina238_read_in(struct device *dev, u32 attr, int channel,
 			  long *val)
 {
 	struct ina238_data *data = dev_get_drvdata(dev);
+	bool has_shunt = data->config->has_shunt;
 	int reg, mask;
 	int regval;
 	int err;
 
-	switch (channel) {
-	case 0:
+	if (has_shunt && channel == 0) {
 		switch (attr) {
 		case hwmon_in_input:
 			if (data->config->has_20bit_voltage_current)
@@ -327,8 +344,7 @@ static int ina238_read_in(struct device *dev, u32 attr, int channel,
 		default:
 			return -EOPNOTSUPP;
 		}
-		break;
-	case 1:
+	} else {
 		switch (attr) {
 		case hwmon_in_input:
 			if (data->config->has_20bit_voltage_current)
@@ -352,9 +368,6 @@ static int ina238_read_in(struct device *dev, u32 attr, int channel,
 		default:
 			return -EOPNOTSUPP;
 		}
-		break;
-	default:
-		return -EOPNOTSUPP;
 	}
 
 	err = regmap_read(data->regmap, reg, &regval);
@@ -367,7 +380,7 @@ static int ina238_read_in(struct device *dev, u32 attr, int channel,
 	case hwmon_in_min:
 		/* signed register, value in mV */
 		regval = (s16)regval;
-		if (channel == 0)
+		if (has_shunt && channel == 0)
 			/* gain of 1 -> LSB / 4 */
 			*val = (regval * INA238_SHUNT_VOLTAGE_LSB) *
 				data->gain / (1000 * 4);
@@ -387,14 +400,14 @@ static int ina238_write_in(struct device *dev, u32 attr, int channel,
 			   long val)
 {
 	struct ina238_data *data = dev_get_drvdata(dev);
+	bool has_shunt = data->config->has_shunt;
 	int regval;
 
 	if (attr != hwmon_in_max && attr != hwmon_in_min)
 		return -EOPNOTSUPP;
 
 	/* convert decimal to register value */
-	switch (channel) {
-	case 0:
+	if (has_shunt && channel == 0) {
 		/* signed value, clamp to max range +/-163 mV */
 		regval = clamp_val(val, -163, 163);
 		regval = (regval * 1000 * 4) /
@@ -411,7 +424,7 @@ static int ina238_write_in(struct device *dev, u32 attr, int channel,
 		default:
 			return -EOPNOTSUPP;
 		}
-	case 1:
+	} else {
 		/* signed value, positive values only. Clamp to max 102.396 V */
 		regval = clamp_val(val, 0, 102396);
 		regval = (regval * 1000) / data->config->bus_voltage_lsb;
@@ -427,8 +440,6 @@ static int ina238_write_in(struct device *dev, u32 attr, int channel,
 		default:
 			return -EOPNOTSUPP;
 		}
-	default:
-		return -EOPNOTSUPP;
 	}
 }
 
@@ -467,9 +478,86 @@ static int ina238_read_current(struct device *dev, u32 attr, long *val)
 	return 0;
 }
 
+static int ina780_read_current(struct device *dev, u32 attr, long *val)
+{
+	struct ina238_data *data = dev_get_drvdata(dev);
+	unsigned int regval;
+	int reg, mask;
+	int err;
+
+	switch (attr) {
+	case hwmon_curr_input:
+		reg = INA238_CURRENT;
+		break;
+	case hwmon_curr_max:
+		reg = INA238_SHUNT_OVER_VOLTAGE;
+		break;
+	case hwmon_curr_min:
+		reg = INA238_SHUNT_UNDER_VOLTAGE;
+		break;
+	case hwmon_curr_max_alarm:
+		reg = INA238_DIAG_ALERT;
+		mask = INA238_DIAG_ALERT_SHNTOL;
+		break;
+	case hwmon_curr_min_alarm:
+		reg = INA238_DIAG_ALERT;
+		mask = INA238_DIAG_ALERT_SHNTUL;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+
+	switch (attr) {
+	case hwmon_curr_input:
+	case hwmon_curr_max:
+	case hwmon_curr_min:
+		err = regmap_read(data->regmap, reg, &regval);
+		if (err)
+			return err;
+		*val = div_s64((s16)regval * INA780_CURRENT_LSB, 1000);
+		break;
+	case hwmon_curr_max_alarm:
+	case hwmon_curr_min_alarm:
+		err = regmap_read(data->regmap, reg, &regval);
+		if (err)
+			return err;
+		*val = !!(regval & mask);
+		break;
+	}
+
+	return 0;
+}
+
+static int ina780_write_current(struct device *dev, u32 attr, long val)
+{
+	struct ina238_data *data = dev_get_drvdata(dev);
+	bool has_curr_min_max = data->config->has_curr_min_max;
+	unsigned int regval;
+	int reg;
+
+	if (!has_curr_min_max)
+		return -EOPNOTSUPP;
+
+	switch (attr) {
+	case hwmon_curr_max:
+		reg = INA238_SHUNT_OVER_VOLTAGE;
+		break;
+	case hwmon_curr_min:
+		reg = INA238_SHUNT_UNDER_VOLTAGE;
+		break;
+	default:
+		return -EOPNOTSUPP;
+	}
+	clamp_val(val, -78643, 78640);
+	regval = div_s64(val * 1000ULL, INA780_CURRENT_LSB);
+
+	return regmap_write(data->regmap, reg, regval);
+}
+
 static int ina238_read_power(struct device *dev, u32 attr, long *val)
 {
 	struct ina238_data *data = dev_get_drvdata(dev);
+	long long fixed_power_lsb = data->config->fixed_power_lsb;
 	long long power;
 	int regval;
 	int err;
@@ -480,9 +568,14 @@ static int ina238_read_power(struct device *dev, u32 attr, long *val)
 		if (err)
 			return err;
 
-		/* Fixed 1mA lsb, scaled by 1000000 to have result in uW */
-		power = div_u64(regval * 1000ULL * INA238_FIXED_SHUNT * data->gain *
-				data->config->power_calculate_factor, 4 * 100 * data->rshunt);
+		if (fixed_power_lsb)
+			power = div_u64(regval * fixed_power_lsb, 1000);
+		else
+			/* Fixed 1mA lsb, scaled by 1000000 to have result in uW */
+			power = div_u64(regval * 1000ULL * INA238_FIXED_SHUNT * data->gain *
+					data->config->power_calculate_factor,
+					4 * 100 * data->rshunt);
+
 		/* Clamp value to maximum value of long */
 		*val = clamp_val(power, 0, LONG_MAX);
 		break;
@@ -506,8 +599,12 @@ static int ina238_read_power(struct device *dev, u32 attr, long *val)
 		 * Truncated 24-bit compare register, lower 8-bits are
 		 * truncated. Same conversion to/from uW as POWER register.
 		 */
-		power = div_u64((regval << 8) * 1000ULL * INA238_FIXED_SHUNT * data->gain *
-				data->config->power_calculate_factor, 4 * 100 * data->rshunt);
+		if (fixed_power_lsb)
+			power = div_u64((regval << 8) * fixed_power_lsb * 256, 1000);
+		else
+			power = div_u64((regval << 8) * 1000ULL * INA238_FIXED_SHUNT * data->gain *
+					data->config->power_calculate_factor,
+					4 * 100 * data->rshunt);
 		/* Clamp value to maximum value of long */
 		*val = clamp_val(power, 0, LONG_MAX);
 		break;
@@ -528,6 +625,7 @@ static int ina238_read_power(struct device *dev, u32 attr, long *val)
 static int ina238_write_power(struct device *dev, u32 attr, long val)
 {
 	struct ina238_data *data = dev_get_drvdata(dev);
+	int fixed_power_lsb = data->config->fixed_power_lsb;
 	long regval;
 
 	if (attr != hwmon_power_max)
@@ -539,8 +637,12 @@ static int ina238_write_power(struct device *dev, u32 attr, long val)
 	 * register.
 	 */
 	regval = clamp_val(val, 0, LONG_MAX);
-	regval = div_u64(val * 4 * 100 * data->rshunt, data->config->power_calculate_factor *
-			1000ULL * INA238_FIXED_SHUNT * data->gain);
+	if (fixed_power_lsb)
+		regval = div_u64(val * 1000ULL, fixed_power_lsb);
+	else
+		regval = div_u64(val * 4 * 100 * data->rshunt,
+				 data->config->power_calculate_factor *
+				 1000ULL * INA238_FIXED_SHUNT * data->gain);
 	regval = clamp_val(regval >> 8, 0, U16_MAX);
 
 	return regmap_write(data->regmap, INA238_POWER_LIMIT, regval);
@@ -603,6 +705,7 @@ static ssize_t energy1_input_show(struct device *dev,
 				  struct device_attribute *da, char *buf)
 {
 	struct ina238_data *data = dev_get_drvdata(dev);
+	bool has_shunt = data->config->has_shunt;
 	int ret;
 	u64 regval;
 	u64 energy;
@@ -612,8 +715,11 @@ static ssize_t energy1_input_show(struct device *dev,
 		return ret;
 
 	/* result in uJ */
-	energy = div_u64(regval * INA238_FIXED_SHUNT * data->gain * 16 * 10 *
-			 data->config->power_calculate_factor, 4 * data->rshunt);
+	if (has_shunt)
+		energy = div_u64(regval * INA238_FIXED_SHUNT *	data->gain * 16 * 10 *
+					data->config->power_calculate_factor, 4 * data->rshunt);
+	else
+		energy = div_u64(regval * INA780_ENERGY_LSB, 1000);
 
 	return sysfs_emit(buf, "%llu\n", energy);
 }
@@ -621,11 +727,17 @@ static ssize_t energy1_input_show(struct device *dev,
 static int ina238_read(struct device *dev, enum hwmon_sensor_types type,
 		       u32 attr, int channel, long *val)
 {
+	struct ina238_data *data = dev_get_drvdata(dev);
+	bool has_curr_min_max = data->config->has_curr_min_max;
+
 	switch (type) {
 	case hwmon_in:
 		return ina238_read_in(dev, attr, channel, val);
 	case hwmon_curr:
-		return ina238_read_current(dev, attr, val);
+		if (has_curr_min_max)
+			return ina780_read_current(dev, attr, val);
+		else
+			return ina238_read_current(dev, attr, val);
 	case hwmon_power:
 		return ina238_read_power(dev, attr, val);
 	case hwmon_temp:
@@ -648,6 +760,9 @@ static int ina238_write(struct device *dev, enum hwmon_sensor_types type,
 	case hwmon_in:
 		err = ina238_write_in(dev, attr, channel, val);
 		break;
+	case hwmon_curr:
+		err = ina780_write_current(dev, attr, val);
+		break;
 	case hwmon_power:
 		err = ina238_write_power(dev, attr, val);
 		break;
@@ -669,6 +784,8 @@ static umode_t ina238_is_visible(const void *drvdata,
 {
 	const struct ina238_data *data = drvdata;
 	bool has_power_highest = data->config->has_power_highest;
+	bool has_curr_min_max = data->config->has_curr_min_max;
+	bool has_shunt = data->config->has_shunt;
 
 	switch (type) {
 	case hwmon_in:
@@ -676,10 +793,14 @@ static umode_t ina238_is_visible(const void *drvdata,
 		case hwmon_in_input:
 		case hwmon_in_max_alarm:
 		case hwmon_in_min_alarm:
-			return 0444;
+			if (channel == 0 || has_shunt)
+				return 0444;
+			return 0;
 		case hwmon_in_max:
 		case hwmon_in_min:
-			return 0644;
+			if (channel == 0 || has_shunt)
+				return 0644;
+			return 0;
 		default:
 			return 0;
 		}
@@ -687,6 +808,13 @@ static umode_t ina238_is_visible(const void *drvdata,
 		switch (attr) {
 		case hwmon_curr_input:
 			return 0444;
+		case hwmon_curr_max:
+		case hwmon_curr_min:
+		case hwmon_curr_max_alarm:
+		case hwmon_curr_min_alarm:
+			if (has_curr_min_max)
+				return 0644;
+			return 0;
 		default:
 			return 0;
 		}
@@ -725,13 +853,16 @@ static umode_t ina238_is_visible(const void *drvdata,
 
 static const struct hwmon_channel_info * const ina238_info[] = {
 	HWMON_CHANNEL_INFO(in,
-			   /* 0: shunt voltage */
+			   /* 0: shunt voltage (bus voltage for ina780)*/
 			   INA238_HWMON_IN_CONFIG,
-			   /* 1: bus voltage */
+			   /* 1: bus voltage (not present on ina780) */
 			   INA238_HWMON_IN_CONFIG),
 	HWMON_CHANNEL_INFO(curr,
 			   /* 0: current through shunt */
-			   HWMON_C_INPUT),
+			   HWMON_C_INPUT |
+			   /* current limits avialble on ina780*/
+			   HWMON_C_MAX | HWMON_C_MAX_ALARM |
+			   HWMON_C_MIN | HWMON_C_MIN_ALARM),
 	HWMON_CHANNEL_INFO(power,
 			   /* 0: power */
 			   HWMON_P_INPUT | HWMON_P_MAX |
@@ -790,21 +921,23 @@ static int ina238_probe(struct i2c_client *client)
 		return PTR_ERR(data->regmap);
 	}
 
-	/* load shunt value */
-	data->rshunt = INA238_RSHUNT_DEFAULT;
-	if (device_property_read_u32(dev, "shunt-resistor", &data->rshunt) < 0 && pdata)
-		data->rshunt = pdata->shunt_uohms;
-	if (data->rshunt == 0) {
-		dev_err(dev, "invalid shunt resister value %u\n", data->rshunt);
-		return -EINVAL;
-	}
+	if (data->config->has_shunt) {
+		/* load shunt value */
+		data->rshunt = INA238_RSHUNT_DEFAULT;
+		if (device_property_read_u32(dev, "shunt-resistor", &data->rshunt) < 0 && pdata)
+			data->rshunt = pdata->shunt_uohms;
+		if (data->rshunt == 0) {
+			dev_err(dev, "invalid shunt resister value %u\n", data->rshunt);
+			return -EINVAL;
+		}
 
-	/* load shunt gain value */
-	if (device_property_read_u32(dev, "ti,shunt-gain", &data->gain) < 0)
-		data->gain = 4; /* Default of ADCRANGE = 0 */
-	if (data->gain != 1 && data->gain != 2 && data->gain != 4) {
-		dev_err(dev, "invalid shunt gain value %u\n", data->gain);
-		return -EINVAL;
+		/* load shunt gain value */
+		if (device_property_read_u32(dev, "ti,shunt-gain", &data->gain) < 0)
+			data->gain = 4; /* Default of ADCRANGE = 0 */
+		if (data->gain != 1 && data->gain != 2 && data->gain != 4) {
+			dev_err(dev, "invalid shunt gain value %u\n", data->gain);
+			return -EINVAL;
+		}
 	}
 
 	/* Setup CONFIG register */
@@ -831,12 +964,14 @@ static int ina238_probe(struct i2c_client *client)
 		return -ENODEV;
 	}
 
-	/* Setup SHUNT_CALIBRATION register with fixed value */
-	ret = regmap_write(data->regmap, INA238_SHUNT_CALIBRATION,
-			   INA238_CALIBRATION_VALUE);
-	if (ret < 0) {
-		dev_err(dev, "error configuring the device: %d\n", ret);
-		return -ENODEV;
+	if (data->config->has_shunt) {
+		/* Setup SHUNT_CALIBRATION register with fixed value */
+		ret = regmap_write(data->regmap, INA238_SHUNT_CALIBRATION,
+				   INA238_CALIBRATION_VALUE);
+		if (ret < 0) {
+			dev_err(dev, "error configuring the device: %d\n", ret);
+			return -ENODEV;
+		}
 	}
 
 	/* Setup alert/alarm configuration */
@@ -854,8 +989,9 @@ static int ina238_probe(struct i2c_client *client)
 	if (IS_ERR(hwmon_dev))
 		return PTR_ERR(hwmon_dev);
 
-	dev_info(dev, "power monitor %s (Rshunt = %u uOhm, gain = %u)\n",
-		 client->name, data->rshunt, data->gain);
+	if (data->config->has_shunt)
+		dev_info(dev, "power monitor %s (Rshunt = %u uOhm, gain = %u)\n",
+			 client->name, data->rshunt, data->gain);
 
 	return 0;
 }
@@ -865,6 +1001,7 @@ static const struct i2c_device_id ina238_id[] = {
 	{ "ina237", ina237 },
 	{ "ina238", ina238 },
 	{ "sq52206", sq52206 },
+	{ "ina780", ina780 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ina238_id);
@@ -886,6 +1023,10 @@ static const struct of_device_id __maybe_unused ina238_of_match[] = {
 		.compatible = "silergy,sq52206",
 		.data = (void *)sq52206
 	},
+	{
+		.compatible = "ti,ina780",
+		.data = (void *)ina780
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(of, ina238_of_match);
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/4] hwmon: (ina238) Correctly clamp temperature
  2025-08-29  3:05 ` [PATCH v3 2/4] hwmon: (ina238) Correctly clamp temperature Chris Packham
@ 2025-08-29  9:55   ` Guenter Roeck
  2025-08-31 21:12     ` Chris Packham
  2025-08-31 22:34   ` Guenter Roeck
  1 sibling, 1 reply; 11+ messages in thread
From: Guenter Roeck @ 2025-08-29  9:55 UTC (permalink / raw)
  To: Chris Packham, jdelvare, robh, krzk+dt, conor+dt, corbet,
	wenliang202407, jre
  Cc: linux-hwmon, devicetree, linux-kernel, linux-doc

On 8/28/25 20:05, Chris Packham wrote:
> ina238_write_temp() was attempting to clamp the user input but was
> throwing away the result. Ensure that we clamp the value to the
> appropriate range before it is converted into a register value.
> 
> Fixes: 0d9f596b1fe3 ("hwmon: (ina238) Modify the calculation formula to adapt to different chips")
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> 
> Notes:
>      Changes in v3:
>      - New. Split off bugfix from main patch
> 
>   drivers/hwmon/ina238.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
> index 5a394eeff676..4d3dc018ead9 100644
> --- a/drivers/hwmon/ina238.c
> +++ b/drivers/hwmon/ina238.c
> @@ -572,7 +572,7 @@ static int ina238_write_temp(struct device *dev, u32 attr, long val)
>   		return -EOPNOTSUPP;
>   
>   	/* Signed */
> -	regval = clamp_val(val, -40000, 125000);
> +	val = clamp_val(val, -40000, 125000);

That needs another correction: As it turns out, the default register value
is 0x7ff0, or 255875. That means we need to accept that range. The same is
probably true for negative temperatures, but I'll need to see the real chip
to be sure.

Yes, the chips only support a limited temperature range, but that is the
limit register, not the supported range. Other chips have a similar problem.
It is ok to limit the input range if the chip has a reasonable default set,
but if the actual chip default is 0x7ff0 or 255.875 degrees C we need to
support writing that value.

Guenter


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 3/4] hwmon: (ina238) Add ina238_config fields
  2025-08-29  3:05 ` [PATCH v3 3/4] hwmon: (ina238) Add ina238_config fields Chris Packham
@ 2025-08-29 15:56   ` Guenter Roeck
  0 siblings, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2025-08-29 15:56 UTC (permalink / raw)
  To: Chris Packham, jdelvare, robh, krzk+dt, conor+dt, corbet,
	wenliang202407, jre
  Cc: linux-hwmon, devicetree, linux-kernel, linux-doc

On 8/28/25 20:05, Chris Packham wrote:
> In preparation for adding INA780 support add some required fields to
> ina238_config and set the appropriate values for the existing chips.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> ---
> 
> Notes:
>      Changes in v3:
>      - New. Split config struct changes from main patch
> 

We should have fields for curent_lsb, power_lsb, and energy_lsb in both
struct ina238_config and struct ina238_data, and pre-calculate the values
for chips where it is dynamic. I started writing that code, but while
writing unit test code I found that the driver has more problems similar
to the one you fixed with an earlier patch of this series. We'll have to
address that first. I hope I can find and fix those issues over the weekend.

Guenter


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/4] hwmon: (ina238) Correctly clamp temperature
  2025-08-29  9:55   ` Guenter Roeck
@ 2025-08-31 21:12     ` Chris Packham
  2025-08-31 22:09       ` Guenter Roeck
  0 siblings, 1 reply; 11+ messages in thread
From: Chris Packham @ 2025-08-31 21:12 UTC (permalink / raw)
  To: Guenter Roeck, jdelvare@suse.com, robh@kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org, corbet@lwn.net,
	wenliang202407@163.com, jre@pengutronix.de
  Cc: linux-hwmon@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org


On 29/08/25 21:55, Guenter Roeck wrote:
> On 8/28/25 20:05, Chris Packham wrote:
>> ina238_write_temp() was attempting to clamp the user input but was
>> throwing away the result. Ensure that we clamp the value to the
>> appropriate range before it is converted into a register value.
>>
>> Fixes: 0d9f596b1fe3 ("hwmon: (ina238) Modify the calculation formula 
>> to adapt to different chips")
>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>> ---
>>
>> Notes:
>>      Changes in v3:
>>      - New. Split off bugfix from main patch
>>
>>   drivers/hwmon/ina238.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
>> index 5a394eeff676..4d3dc018ead9 100644
>> --- a/drivers/hwmon/ina238.c
>> +++ b/drivers/hwmon/ina238.c
>> @@ -572,7 +572,7 @@ static int ina238_write_temp(struct device *dev, 
>> u32 attr, long val)
>>           return -EOPNOTSUPP;
>>         /* Signed */
>> -    regval = clamp_val(val, -40000, 125000);
>> +    val = clamp_val(val, -40000, 125000);
>
> That needs another correction: As it turns out, the default register 
> value
> is 0x7ff0, or 255875. That means we need to accept that range. The 
> same is
> probably true for negative temperatures, but I'll need to see the real 
> chip
> to be sure.
>
> Yes, the chips only support a limited temperature range, but that is the
> limit register, not the supported range. Other chips have a similar 
> problem.
> It is ok to limit the input range if the chip has a reasonable default 
> set,
> but if the actual chip default is 0x7ff0 or 255.875 degrees C we need to
> support writing that value.
I'm guessing that might change my introduction of temp_max in the next 
patch. I'll re-order my series to put the bugfix first with the changes 
mentioned.
>
> Guenter
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/4] hwmon: (ina238) Correctly clamp temperature
  2025-08-31 21:12     ` Chris Packham
@ 2025-08-31 22:09       ` Guenter Roeck
  0 siblings, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2025-08-31 22:09 UTC (permalink / raw)
  To: Chris Packham, jdelvare@suse.com, robh@kernel.org,
	krzk+dt@kernel.org, conor+dt@kernel.org, corbet@lwn.net,
	wenliang202407@163.com, jre@pengutronix.de
  Cc: linux-hwmon@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org

On 8/31/25 14:12, Chris Packham wrote:
> 
> On 29/08/25 21:55, Guenter Roeck wrote:
>> On 8/28/25 20:05, Chris Packham wrote:
>>> ina238_write_temp() was attempting to clamp the user input but was
>>> throwing away the result. Ensure that we clamp the value to the
>>> appropriate range before it is converted into a register value.
>>>
>>> Fixes: 0d9f596b1fe3 ("hwmon: (ina238) Modify the calculation formula
>>> to adapt to different chips")
>>> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
>>> ---
>>>
>>> Notes:
>>>       Changes in v3:
>>>       - New. Split off bugfix from main patch
>>>
>>>    drivers/hwmon/ina238.c | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
>>> index 5a394eeff676..4d3dc018ead9 100644
>>> --- a/drivers/hwmon/ina238.c
>>> +++ b/drivers/hwmon/ina238.c
>>> @@ -572,7 +572,7 @@ static int ina238_write_temp(struct device *dev,
>>> u32 attr, long val)
>>>            return -EOPNOTSUPP;
>>>          /* Signed */
>>> -    regval = clamp_val(val, -40000, 125000);
>>> +    val = clamp_val(val, -40000, 125000);
>>
>> That needs another correction: As it turns out, the default register
>> value
>> is 0x7ff0, or 255875. That means we need to accept that range. The
>> same is
>> probably true for negative temperatures, but I'll need to see the real
>> chip
>> to be sure.
>>
>> Yes, the chips only support a limited temperature range, but that is the
>> limit register, not the supported range. Other chips have a similar
>> problem.
>> It is ok to limit the input range if the chip has a reasonable default
>> set,
>> but if the actual chip default is 0x7ff0 or 255.875 degrees C we need to
>> support writing that value.
> I'm guessing that might change my introduction of temp_max in the next
> patch. I'll re-order my series to put the bugfix first with the changes
> mentioned.

Please wait a bit before resending; I have a series almost ready to send out
with various other changes.

Guenter


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 2/4] hwmon: (ina238) Correctly clamp temperature
  2025-08-29  3:05 ` [PATCH v3 2/4] hwmon: (ina238) Correctly clamp temperature Chris Packham
  2025-08-29  9:55   ` Guenter Roeck
@ 2025-08-31 22:34   ` Guenter Roeck
  1 sibling, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2025-08-31 22:34 UTC (permalink / raw)
  To: Chris Packham
  Cc: jdelvare, robh, krzk+dt, conor+dt, corbet, wenliang202407, jre,
	linux-hwmon, devicetree, linux-kernel, linux-doc

On Fri, Aug 29, 2025 at 03:05:10PM +1200, Chris Packham wrote:
> ina238_write_temp() was attempting to clamp the user input but was
> throwing away the result. Ensure that we clamp the value to the
> appropriate range before it is converted into a register value.
> 
> Fixes: 0d9f596b1fe3 ("hwmon: (ina238) Modify the calculation formula to adapt to different chips")
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>

Applied.

Guenter

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v3 1/4] dt-bindings: hwmon: ti,ina2xx: Add INA780 device
  2025-08-29  3:05 ` [PATCH v3 1/4] dt-bindings: hwmon: ti,ina2xx: Add INA780 device Chris Packham
@ 2025-08-31 22:35   ` Guenter Roeck
  0 siblings, 0 replies; 11+ messages in thread
From: Guenter Roeck @ 2025-08-31 22:35 UTC (permalink / raw)
  To: Chris Packham
  Cc: jdelvare, robh, krzk+dt, conor+dt, corbet, wenliang202407, jre,
	linux-hwmon, devicetree, linux-kernel, linux-doc

On Fri, Aug 29, 2025 at 03:05:09PM +1200, Chris Packham wrote:
> Add a compatible string for the INA780 device.
> 
> Signed-off-by: Chris Packham <chris.packham@alliedtelesis.co.nz>
> Acked-by: Conor Dooley <conor.dooley@microchip.com>

Applied.

Guenter

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-08-31 22:35 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-29  3:05 [PATCH v3 0/4] hwmon: Add support for INA780 Chris Packham
2025-08-29  3:05 ` [PATCH v3 1/4] dt-bindings: hwmon: ti,ina2xx: Add INA780 device Chris Packham
2025-08-31 22:35   ` Guenter Roeck
2025-08-29  3:05 ` [PATCH v3 2/4] hwmon: (ina238) Correctly clamp temperature Chris Packham
2025-08-29  9:55   ` Guenter Roeck
2025-08-31 21:12     ` Chris Packham
2025-08-31 22:09       ` Guenter Roeck
2025-08-31 22:34   ` Guenter Roeck
2025-08-29  3:05 ` [PATCH v3 3/4] hwmon: (ina238) Add ina238_config fields Chris Packham
2025-08-29 15:56   ` Guenter Roeck
2025-08-29  3:05 ` [PATCH v3 4/4] hwmon: (ina238) Add support for INA780 Chris Packham

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).