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>
---
 Documentation/hwmon/ina238.rst |  20 +++
 drivers/hwmon/ina238.c         | 258 ++++++++++++++++++++++++++-------
 2 files changed, 226 insertions(+), 52 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 a2cb615fa278..424efa99c541 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>
  */
@@ -32,6 +33,8 @@
 #define INA238_DIAG_ALERT		0xb
 #define INA238_SHUNT_OVER_VOLTAGE	0xc
 #define INA238_SHUNT_UNDER_VOLTAGE	0xd
+#define INA780_COL			0xc
+#define INA780_CUL			0xd
 #define INA238_BUS_OVER_VOLTAGE		0xe
 #define INA238_BUS_UNDER_VOLTAGE	0xf
 #define INA238_TEMP_LIMIT		0x10
@@ -49,6 +52,8 @@
 #define INA238_DIAG_ALERT_BUSOL		BIT(4)
 #define INA238_DIAG_ALERT_BUSUL		BIT(3)
 #define INA238_DIAG_ALERT_POL		BIT(2)
+#define INA780_DIAG_ALERT_CURRENTOL	BIT(6)
+#define INA780_DIAG_ALERT_CURRENTUL	BIT(5)
 
 #define INA238_REGISTERS		0x20
 
@@ -108,22 +113,29 @@
 #define SQ52206_BUS_VOLTAGE_LSB		3750 /* 3.75 mV/lsb */
 #define SQ52206_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 };
+enum ina238_ids { ina238, ina237, sq52206, ina780 };
 
 struct ina238_config {
+	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;
 };
 
 struct ina238_data {
@@ -137,6 +149,7 @@ struct ina238_data {
 
 static const struct ina238_config ina238_config[] = {
 	[ina238] = {
+		.has_shunt = true,
 		.has_energy = false,
 		.has_power_highest = false,
 		.temp_shift = 4,
@@ -144,8 +157,10 @@ static const struct ina238_config ina238_config[] = {
 		.config_default = INA238_CONFIG_DEFAULT,
 		.bus_voltage_lsb = INA238_BUS_VOLTAGE_LSB,
 		.temp_lsb = INA238_DIE_TEMP_LSB,
+		.temp_max = 125000,
 	},
 	[ina237] = {
+		.has_shunt = true,
 		.has_energy = false,
 		.has_power_highest = false,
 		.temp_shift = 4,
@@ -153,8 +168,10 @@ static const struct ina238_config ina238_config[] = {
 		.config_default = INA238_CONFIG_DEFAULT,
 		.bus_voltage_lsb = INA238_BUS_VOLTAGE_LSB,
 		.temp_lsb = INA238_DIE_TEMP_LSB,
+		.temp_max = 125000,
 	},
 	[sq52206] = {
+		.has_shunt = true,
 		.has_energy = true,
 		.has_power_highest = true,
 		.temp_shift = 0,
@@ -162,7 +179,20 @@ static const struct ina238_config ina238_config[] = {
 		.config_default = SQ52206_CONFIG_DEFAULT,
 		.bus_voltage_lsb = SQ52206_BUS_VOLTAGE_LSB,
 		.temp_lsb = SQ52206_DIE_TEMP_LSB,
+		.temp_max = 125000,
 	},
+	[ina780] = {
+		.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)
@@ -203,12 +233,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:
 			reg = INA238_SHUNT_VOLTAGE;
@@ -230,8 +260,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:
 			reg = INA238_BUS_VOLTAGE;
@@ -253,9 +282,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);
@@ -268,7 +294,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);
@@ -288,14 +314,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) /
@@ -312,7 +338,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;
@@ -328,8 +354,6 @@ static int ina238_write_in(struct device *dev, u32 attr, int channel,
 		default:
 			return -EOPNOTSUPP;
 		}
-	default:
-		return -EOPNOTSUPP;
 	}
 }
 
@@ -356,9 +380,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 = INA780_COL;
+		break;
+	case hwmon_curr_min:
+		reg = INA780_CUL;
+		break;
+	case hwmon_curr_max_alarm:
+		reg = INA238_DIAG_ALERT;
+		mask = INA780_DIAG_ALERT_CURRENTOL;
+		break;
+	case hwmon_curr_min_alarm:
+		reg = INA238_DIAG_ALERT;
+		mask = INA780_DIAG_ALERT_CURRENTUL;
+		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 = INA780_COL;
+		break;
+	case hwmon_curr_min:
+		reg = INA780_CUL;
+		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;
@@ -369,9 +470,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;
@@ -395,8 +501,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;
@@ -417,6 +527,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)
@@ -428,8 +539,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);
@@ -481,7 +596,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);
 
@@ -492,6 +607,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;
@@ -501,8 +617,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);
 }
@@ -510,11 +629,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:
@@ -537,6 +662,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;
@@ -558,6 +686,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:
@@ -565,10 +695,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;
 		}
@@ -576,6 +710,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;
 		}
@@ -614,13 +755,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 |
@@ -679,21 +823,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 */
@@ -720,12 +866,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 */
@@ -743,8 +891,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;
 }
@@ -753,6 +902,7 @@ static const struct i2c_device_id ina238_id[] = {
 	{ "ina237", ina237 },
 	{ "ina238", ina238 },
 	{ "sq52206", sq52206 },
+	{ "ina780", ina780 },
 	{ }
 };
 MODULE_DEVICE_TABLE(i2c, ina238_id);
@@ -770,6 +920,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.50.1

On 8/7/25 20:05, Chris Packham wrote:
> 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>

Your patch does not apply, and I can't figure out its baseline. Please
reparent on top of the current mainline and resubmit.

To simplify review, the patch should be split into preparation patches
(such as adding .has_shunt and .temp_max options), followed by the actual
added chip support.

Other (not a complete review):

I don't see the value of adding INA780_COL and INA780_CUL defines;
those are really the same as the shunt voltage limits. Actually,
the current limits _are_ available for existing chips, only they
are expressed as voltage limits on the shunt voltages. For the ina_2xx
driver I was able to resolve that quite easily; we should do the same
for the ina238 driver. Maybe I have an evaluation board somewhere;
I'll need to check.

[ Sorry for being so late with this; I am being swamped at work :-( ]

Thanks,
Guenter

On 29/08/2025 00:09, Guenter Roeck wrote:
> On 8/7/25 20:05, Chris Packham wrote:
>> 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>
>
> Your patch does not apply, and I can't figure out its baseline. Please
> reparent on top of the current mainline and resubmit.
Sure no problem. The ina238 changes were done on top of my initial 
ina780 stuff so the sha1 recorded in the patch will be a local sha1 that 
you don't have. I'll clean things up on top of master without any local 
junk.
>
> To simplify review, the patch should be split into preparation patches
> (such as adding .has_shunt and .temp_max options), followed by the actual
> added chip support.
Sure.
>
> Other (not a complete review):
>
> I don't see the value of adding INA780_COL and INA780_CUL defines;
> those are really the same as the shunt voltage limits. Actually,
> the current limits _are_ available for existing chips, only they
> are expressed as voltage limits on the shunt voltages.

My main motivation was trying to match the terms used in the INA780 
datasheet. INA780 uses COL/CUL, INA238 uses SOVL/SUVL. I can kind of 
squint and see how they are similar the INA238 is just more complicated 
because of the external shunt. I did kind of think it must be possible 
to express the INA780 behaviour with some fixed values but my math 
skills failed me.

> For the ina_2xx
> driver I was able to resolve that quite easily; we should do the same
> for the ina238 driver. Maybe I have an evaluation board somewhere;
> I'll need to check.
>
> [ Sorry for being so late with this; I am being swamped at work :-( ] 

No problem. Same thing for me.

On 8/28/25 14:22, Chris Packham wrote:
> 
> On 29/08/2025 00:09, Guenter Roeck wrote:
>> On 8/7/25 20:05, Chris Packham wrote:
>>> 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>
>>
>> Your patch does not apply, and I can't figure out its baseline. Please
>> reparent on top of the current mainline and resubmit.
> Sure no problem. The ina238 changes were done on top of my initial
> ina780 stuff so the sha1 recorded in the patch will be a local sha1 that
> you don't have. I'll clean things up on top of master without any local
> junk.
>>
>> To simplify review, the patch should be split into preparation patches
>> (such as adding .has_shunt and .temp_max options), followed by the actual
>> added chip support.
> Sure.
>>
>> Other (not a complete review):
>>
>> I don't see the value of adding INA780_COL and INA780_CUL defines;
>> those are really the same as the shunt voltage limits. Actually,
>> the current limits _are_ available for existing chips, only they
>> are expressed as voltage limits on the shunt voltages.
> 
> My main motivation was trying to match the terms used in the INA780
> datasheet. INA780 uses COL/CUL, INA238 uses SOVL/SUVL. I can kind of

Yeah, only those change all the time. Just try to match register names
(or pin names, for that matter) for the chips supported by the lm90 driver.
I'd rather just add a note explaining the differences in cases like this one,
where it isn't entirely obvious.

> squint and see how they are similar the INA238 is just more complicated
> because of the external shunt. I did kind of think it must be possible
> to express the INA780 behaviour with some fixed values but my math
> skills failed me.
> 

That is why I ordered those evaluation boards. Forget the math, just look
at the registers. Fortunately the TI evaluation boards are not that expensive.

Guenter

On 8/28/25 05:09, Guenter Roeck wrote:
> On 8/7/25 20:05, Chris Packham wrote:
>> 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>
> 
> Your patch does not apply, and I can't figure out its baseline. Please
> reparent on top of the current mainline and resubmit.
> 
> To simplify review, the patch should be split into preparation patches
> (such as adding .has_shunt and .temp_max options), followed by the actual
> added chip support.
> 
> Other (not a complete review):
> 
> I don't see the value of adding INA780_COL and INA780_CUL defines;
> those are really the same as the shunt voltage limits. Actually,
> the current limits _are_ available for existing chips, only they
> are expressed as voltage limits on the shunt voltages. For the ina_2xx
> driver I was able to resolve that quite easily; we should do the same
> for the ina238 driver. Maybe I have an evaluation board somewhere;
> I'll need to check.
> 
> [ Sorry for being so late with this; I am being swamped at work :-( ]
> 

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.

Guenter

Chis,

On Fri, Aug 08, 2025 at 03:05:10PM +1200, Chris Packham wrote:
> 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>

Please send me a register dump for the chip so I can add unit test code
for its support by the driver.

Thanks,
Guenter

Hi Gunter,

On 28/08/2025 04:04, Guenter Roeck wrote:
> Chis,
>
> On Fri, Aug 08, 2025 at 03:05:10PM +1200, Chris Packham wrote:
>> 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>
> Please send me a register dump for the chip so I can add unit test code
> for its support by the driver.

Sure. I used the following script to dump the registers

cat <<EOF | while read addr len; do printf "%2x: " $addr; i2cget -y -f 1 
0x40 $addr i $len; done
0x0 2
0x1 2
0x5 2
0x6 2
0x7 2
0x8 3
0x9 5
0xa 5
0xb 2
0xc 2
0xd 2
0xe 3
0xf 2
0x10 2
0x11 2
0x3e 2
EOF

On an unconfigured INA780A with no load just after reset

  0: 0x00 0x30
  1: 0xfb 0x68
  5: 0x00 0x00
  6: 0x0b 0x00
  7: 0x00 0x00
  8: 0x00 0x00 0x00
  9: 0x00 0x00 0x00 0x00 0x00
  a: 0xff 0xff 0xff 0xff 0x5b
  b: 0x00 0x03
  c: 0x7f 0xff
  d: 0x80 0x00
  e: 0x7f 0xff 0xff
  f: 0x00 0x00
10: 0x7f 0xf0
11: 0xff 0xff
3e: 0x54 0x49

On a INA780A with no load

  0: 0x00 0x30
  1: 0xfb 0x6a
  5: 0x00 0x00
  6: 0x0b 0x00
  7: 0x00 0x00
  8: 0x00 0x00 0x00
  9: 0x00 0x00 0x00 0x00 0x00
  a: 0xff 0xff 0xff 0xff 0xd8
  b: 0x20 0x03
  c: 0x7f 0xff
  d: 0x80 0x00
  e: 0x7f 0xff 0xff
  f: 0x00 0x00
10: 0x7f 0xf0
11: 0xff 0xff
3e: 0x54 0x49

corresponding lm-sensors output

ina780-i2c-1-40
Adapter: i2c-0-mux (chan_id 0)
in0:           0.00 V  (min =  +0.00 V, max = +102.40 V)
temp1:        +22.0 C  (high = +255.9 C)
power1:        0.00 W  (max =   2.06 kW)
energy1:       0.00 J
curr1:         0.00 A  (min = -78.64 A, max = +78.64

On a INA780A with 10V, 4A load

  0: 0x00 0x30
  1: 0xfb 0x6a
  5: 0x0d 0x75
  6: 0x0b 0x20
  7: 0x06 0x5f
  8: 0x01 0x57 0x0a
  9: 0x00 0x00 0x03 0x94 0xcd
  a: 0x00 0x00 0x11 0x4b 0xaa
  b: 0x20 0x03
  c: 0x7f 0xff
  d: 0x80 0x00
  e: 0x7f 0xff 0xff
  f: 0x00 0x00
10: 0x7f 0xf0
11: 0xff 0xff
3e: 0x54 0x49

corresponding lm-sensors output

ina780-i2c-1-40
Adapter: i2c-0-mux (chan_id 0)
in0:          10.77 V  (min =  +0.00 V, max = +102.40 V)
temp1:        +22.2 C  (high = +255.9 C)
power1:       42.06 mW (max =   2.06 kW)
energy1:       2.80 J
curr1:         3.91 A  (min = -78.64 A, max = +78.64 A)

And for good measure a word-wise dump (same config and load as above)

[root@linuxbox ~]# i2cdump -y -f 1 0x40 w
      0,8  1,9  2,a  3,b  4,c  5,d  6,e  7,f
00: 3000 6afb ffff ffff ffff 760d 300b 5a06
08: 5501 0000 0000 0320 ff7f 0080 ff7f 0000
10: f07f ffff ffff ffff ffff ffff ffff ffff
18: ffff ffff ffff ffff ffff ffff ffff ffff
20: ffff ffff ffff ffff ffff ffff ffff ffff
28: ffff ffff ffff ffff ffff ffff ffff ffff
30: ffff ffff ffff ffff ffff 0000 ffff ffff
38: ffff ffff ffff ffff ffff ffff 4954 6227
40: 3000 6afb ffff ffff ffff 760d 300b 5a06
48: 5501 0000 0000 0320 ff7f 0080 ff7f 0000
50: f07f ffff ffff ffff ffff ffff ffff ffff
58: ffff ffff ffff ffff ffff ffff ffff ffff
60: ffff ffff ffff ffff ffff ffff ffff ffff
68: ffff ffff ffff ffff ffff ffff ffff ffff
70: ffff ffff ffff ffff ffff 0000 ffff ffff
78: ffff ffff ffff ffff ffff ffff 4954 6227
80: 3000 6afb ffff ffff ffff 760d 300b 5a06
88: 5501 0000 0000 0320 ff7f 0080 ff7f 0000
90: f07f ffff ffff ffff ffff ffff ffff ffff
98: ffff ffff ffff ffff ffff ffff ffff ffff
a0: ffff ffff ffff ffff ffff ffff ffff ffff
a8: ffff ffff ffff ffff ffff ffff ffff ffff
b0: ffff ffff ffff ffff ffff 0000 ffff ffff
b8: ffff ffff ffff ffff ffff ffff 4954 6227
c0: 3000 6afb ffff ffff ffff 760d 300b 5a06
c8: 5501 0000 0000 0320 ff7f 0080 ff7f 0000
d0: f07f ffff ffff ffff ffff ffff ffff ffff
d8: ffff ffff ffff ffff ffff ffff ffff ffff
e0: ffff ffff ffff ffff ffff ffff ffff ffff
e8: ffff ffff ffff ffff ffff ffff ffff ffff
f0: ffff ffff ffff ffff ffff 0000 ffff ffff
f8: ffff ffff ffff ffff ffff ffff 4954 6227