[PATCH v2 0/3] ina238: Improvements and INA228 support

[PATCH v2 0/3] ina238: Improvements and INA228 support

[Jonas Rebmann]

This series includes:
 - Whitespace fixes
 - Support INA228 ultra-precise power monitor

Tested on the INA228 chips on a TI TMDS62LEVM.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
Changes in v2:
- Drop patch for label support for voltage inputs
- Sign-extend the twos complement ina228 20bit registers
- Upcast current readings to s64 earlier to prevent overflow
- Improve whitespace patch
- Link to v1: https://lore.kernel.org/r/20250715-ina228-v1-0-3302fae4434b@pengutronix.de

---
Jonas Rebmann (3):
      hwmon: ina238: Fix inconsistent whitespace
      dt-bindings: Add INA228 to ina2xx devicetree bindings
      hwmon: ina238: Add support for INA228

 .../devicetree/bindings/hwmon/ti,ina2xx.yaml       |   2 +
 drivers/hwmon/ina238.c                             | 134 ++++++++++++++++++---
 2 files changed, 117 insertions(+), 19 deletions(-)
---
base-commit: 27b297ca04813623d8df2f79d141d51e0856810c
change-id: 20250715-ina228-ddb5550dcb23

Best regards,
--  
Jonas Rebmann <jre@pengutronix.de>

[PATCH v2 1/3] hwmon: ina238: Fix inconsistent whitespace

[Jonas Rebmann]

Some purely cosmetic changes in ina238.c:

 - When aligning definitions, do so consistently with tab stop of 8.
 - Use spaces instead of tabs around operators.
 - Align wrapped lines.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 drivers/hwmon/ina238.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index 9a5fd16a4ec2..5e43bbe8817d 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -41,7 +41,7 @@
 
 #define INA238_CONFIG_ADCRANGE		BIT(4)
 #define SQ52206_CONFIG_ADCRANGE_HIGH	BIT(4)
-#define SQ52206_CONFIG_ADCRANGE_LOW		BIT(3)
+#define SQ52206_CONFIG_ADCRANGE_LOW	BIT(3)
 
 #define INA238_DIAG_ALERT_TMPOL		BIT(7)
 #define INA238_DIAG_ALERT_SHNTOL	BIT(6)
@@ -104,7 +104,7 @@
 
 #define INA238_SHUNT_VOLTAGE_LSB	5 /* 5 uV/lsb */
 #define INA238_BUS_VOLTAGE_LSB		3125 /* 3.125 mV/lsb */
-#define INA238_DIE_TEMP_LSB			1250000 /* 125.0000 mC/lsb */
+#define INA238_DIE_TEMP_LSB		1250000 /* 125.0000 mC/lsb */
 #define SQ52206_BUS_VOLTAGE_LSB		3750 /* 3.75 mV/lsb */
 #define SQ52206_DIE_TEMP_LSB		78125 /* 7.8125 mC/lsb */
 
@@ -118,12 +118,12 @@ enum ina238_ids { ina238, ina237, sq52206 };
 
 struct ina238_config {
 	bool has_power_highest;		/* chip detection power peak */
-	bool has_energy;			/* chip detection energy */
-	u8 temp_shift;				/* fixed parameters for temp calculate */
+	bool has_energy;		/* chip detection energy */
+	u8 temp_shift;			/* fixed parameters for temp calculate */
 	u32 power_calculate_factor;	/* fixed parameters for power calculate */
-	u16 config_default;			/* Power-on default state */
+	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_lsb;			/* use for temperature calculate */
 };
 
 struct ina238_data {
@@ -271,7 +271,7 @@ static int ina238_read_in(struct device *dev, u32 attr, int channel,
 		if (channel == 0)
 			/* gain of 1 -> LSB / 4 */
 			*val = (regval * INA238_SHUNT_VOLTAGE_LSB) *
-					data->gain / (1000 * 4);
+				data->gain / (1000 * 4);
 		else
 			*val = (regval * data->config->bus_voltage_lsb) / 1000;
 		break;
@@ -370,7 +370,7 @@ static int ina238_read_power(struct device *dev, u32 attr, long *val)
 			return err;
 
 		/* Fixed 1mA lsb, scaled by 1000000 to have result in uW */
-		power = div_u64(regval * 1000ULL * INA238_FIXED_SHUNT *	data->gain *
+		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);
@@ -381,7 +381,7 @@ static int ina238_read_power(struct device *dev, u32 attr, long *val)
 			return err;
 
 		/* Fixed 1mA lsb, scaled by 1000000 to have result in uW */
-		power = div_u64(regval * 1000ULL * INA238_FIXED_SHUNT *	data->gain *
+		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);
@@ -395,7 +395,7 @@ 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 *
+		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);
@@ -448,7 +448,7 @@ static int ina238_read_temp(struct device *dev, u32 attr, long *val)
 			return err;
 		/* Signed, result in mC */
 		*val = div_s64(((s64)((s16)regval) >> data->config->temp_shift) *
-						(s64)data->config->temp_lsb, 10000);
+			       (s64)data->config->temp_lsb, 10000);
 		break;
 	case hwmon_temp_max:
 		err = regmap_read(data->regmap, INA238_TEMP_LIMIT, &regval);
@@ -456,7 +456,7 @@ static int ina238_read_temp(struct device *dev, u32 attr, long *val)
 			return err;
 		/* Signed, result in mC */
 		*val = div_s64(((s64)((s16)regval) >> data->config->temp_shift) *
-						(s64)data->config->temp_lsb, 10000);
+			       (s64)data->config->temp_lsb, 10000);
 		break;
 	case hwmon_temp_max_alarm:
 		err = regmap_read(data->regmap, INA238_DIAG_ALERT, &regval);
@@ -501,8 +501,8 @@ 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);
+	energy = div_u64(regval * INA238_FIXED_SHUNT * data->gain * 16 * 10 *
+			 data->config->power_calculate_factor, 4 * data->rshunt);
 
 	return sysfs_emit(buf, "%llu\n", energy);
 }

-- 
2.50.1.319.g90c0775e97

Re: [PATCH v2 1/3] hwmon: ina238: Fix inconsistent whitespace

[Guenter Roeck]

On Fri, Jul 18, 2025 at 04:12:48PM +0200, Jonas Rebmann wrote:
> Some purely cosmetic changes in ina238.c:
> 
>  - When aligning definitions, do so consistently with tab stop of 8.
>  - Use spaces instead of tabs around operators.
>  - Align wrapped lines.
> 
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>

Applied.

Thanks,
Guenter

[PATCH v2 2/3] dt-bindings: Add INA228 to ina2xx devicetree bindings

[Jonas Rebmann]

Add the ina228 to ina2xx bindings.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
index d1fb7b9abda0..fa68b99ef2e2 100644
--- a/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
+++ b/Documentation/devicetree/bindings/hwmon/ti,ina2xx.yaml
@@ -25,6 +25,7 @@ properties:
       - ti,ina219
       - ti,ina220
       - ti,ina226
+      - ti,ina228
       - ti,ina230
       - ti,ina231
       - ti,ina233
@@ -107,6 +108,7 @@ allOf:
               - ti,ina219
               - ti,ina220
               - ti,ina226
+              - ti,ina228
               - ti,ina230
               - ti,ina231
               - ti,ina237

-- 
2.50.1.319.g90c0775e97

Re: [PATCH v2 2/3] dt-bindings: Add INA228 to ina2xx devicetree bindings

[Guenter Roeck]

On Fri, Jul 18, 2025 at 04:12:49PM +0200, Jonas Rebmann wrote:
> Add the ina228 to ina2xx bindings.
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>

Applied.

Thanks,
Guenter

[PATCH v2 3/3] hwmon: ina238: Add support for INA228

[Jonas Rebmann]

Add support for the Texas Instruments INA228 Ultra-Precise
Power/Energy/Charge Monitor.

The INA228 is very similar to the INA238 but offers four bits of extra
precision in the temperature, voltage and current measurement fields.
It also supports energy and charge monitoring, the latter of which is
not supported through this patch.

While it seems in the datasheet that some constants such as LSB values
differ between the 228 and the 238, they differ only for those registers
where four bits of precision have been added and they differ by a factor
of 16 (VBUS, VSHUNT, DIETEMP, CURRENT).

Therefore, the INA238 constants are still applicable with regard
to the bit of the same significance.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
 drivers/hwmon/ina238.c | 106 ++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 101 insertions(+), 5 deletions(-)

diff --git a/drivers/hwmon/ina238.c b/drivers/hwmon/ina238.c
index 5e43bbe8817d..5a394eeff676 100644
--- a/drivers/hwmon/ina238.c
+++ b/drivers/hwmon/ina238.c
@@ -6,6 +6,7 @@
  * Copyright (C) 2021 Nathan Rossi <nathan.rossi@digi.com>
  */
 
+#include <linux/bitops.h>
 #include <linux/err.h>
 #include <linux/hwmon.h>
 #include <linux/i2c.h>
@@ -107,6 +108,7 @@
 #define INA238_DIE_TEMP_LSB		1250000 /* 125.0000 mC/lsb */
 #define SQ52206_BUS_VOLTAGE_LSB		3750 /* 3.75 mV/lsb */
 #define SQ52206_DIE_TEMP_LSB		78125 /* 7.8125 mC/lsb */
+#define INA228_DIE_TEMP_LSB		78125 /* 7.8125 mC/lsb */
 
 static const struct regmap_config ina238_regmap_config = {
 	.max_register = INA238_REGISTERS,
@@ -114,9 +116,10 @@ static const struct regmap_config ina238_regmap_config = {
 	.val_bits = 16,
 };
 
-enum ina238_ids { ina238, ina237, sq52206 };
+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_power_highest;		/* chip detection power peak */
 	bool has_energy;		/* chip detection energy */
 	u8 temp_shift;			/* fixed parameters for temp calculate */
@@ -137,6 +140,7 @@ struct ina238_data {
 
 static const struct ina238_config ina238_config[] = {
 	[ina238] = {
+		.has_20bit_voltage_current = false,
 		.has_energy = false,
 		.has_power_highest = false,
 		.temp_shift = 4,
@@ -146,6 +150,7 @@ static const struct ina238_config ina238_config[] = {
 		.temp_lsb = INA238_DIE_TEMP_LSB,
 	},
 	[ina237] = {
+		.has_20bit_voltage_current = false,
 		.has_energy = false,
 		.has_power_highest = false,
 		.temp_shift = 4,
@@ -155,6 +160,7 @@ static const struct ina238_config ina238_config[] = {
 		.temp_lsb = INA238_DIE_TEMP_LSB,
 	},
 	[sq52206] = {
+		.has_20bit_voltage_current = false,
 		.has_energy = true,
 		.has_power_highest = true,
 		.temp_shift = 0,
@@ -163,6 +169,16 @@ static const struct ina238_config ina238_config[] = {
 		.bus_voltage_lsb = SQ52206_BUS_VOLTAGE_LSB,
 		.temp_lsb = SQ52206_DIE_TEMP_LSB,
 	},
+	[ina228] = {
+		.has_20bit_voltage_current = true,
+		.has_energy = true,
+		.has_power_highest = 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,
+	},
 };
 
 static int ina238_read_reg24(const struct i2c_client *client, u8 reg, u32 *val)
@@ -199,6 +215,65 @@ static int ina238_read_reg40(const struct i2c_client *client, u8 reg, u64 *val)
 	return 0;
 }
 
+static int ina238_read_field_s20(const struct i2c_client *client, u8 reg, s32 *val)
+{
+	u32 regval;
+	int err;
+
+	err = ina238_read_reg24(client, reg, &regval);
+	if (err)
+		return err;
+
+	/* bits 3-0 Reserved, always zero */
+	regval >>= 4;
+
+	*val = sign_extend32(regval, 19);
+
+	return 0;
+}
+
+static int ina228_read_shunt_voltage(struct device *dev, u32 attr, int channel,
+				     long *val)
+{
+	struct ina238_data *data = dev_get_drvdata(dev);
+	int regval;
+	int err;
+
+	err = ina238_read_field_s20(data->client, INA238_SHUNT_VOLTAGE, &regval);
+	if (err)
+		return err;
+
+	/*
+	 * gain of 1 -> LSB / 4
+	 * This field has 16 bit on ina238. ina228 adds another 4 bits of
+	 * precision. ina238 conversion factors can still be applied when
+	 * dividing by 16.
+	 */
+	*val = (regval * INA238_SHUNT_VOLTAGE_LSB) * data->gain / (1000 * 4) / 16;
+	return 0;
+}
+
+static int ina228_read_bus_voltage(struct device *dev, u32 attr, int channel,
+				   long *val)
+{
+	struct ina238_data *data = dev_get_drvdata(dev);
+	int regval;
+	int err;
+
+	err = ina238_read_field_s20(data->client, INA238_BUS_VOLTAGE, &regval);
+	if (err)
+		return err;
+
+	/*
+	 * gain of 1 -> LSB / 4
+	 * This field has 16 bit on ina238. ina228 adds another 4 bits of
+	 * precision. ina238 conversion factors can still be applied when
+	 * dividing by 16.
+	 */
+	*val = (regval * data->config->bus_voltage_lsb) / 1000 / 16;
+	return 0;
+}
+
 static int ina238_read_in(struct device *dev, u32 attr, int channel,
 			  long *val)
 {
@@ -211,6 +286,8 @@ static int ina238_read_in(struct device *dev, u32 attr, int channel,
 	case 0:
 		switch (attr) {
 		case hwmon_in_input:
+			if (data->config->has_20bit_voltage_current)
+				return ina228_read_shunt_voltage(dev, attr, channel, val);
 			reg = INA238_SHUNT_VOLTAGE;
 			break;
 		case hwmon_in_max:
@@ -234,6 +311,8 @@ static int ina238_read_in(struct device *dev, u32 attr, int channel,
 	case 1:
 		switch (attr) {
 		case hwmon_in_input:
+			if (data->config->has_20bit_voltage_current)
+				return ina228_read_bus_voltage(dev, attr, channel, val);
 			reg = INA238_BUS_VOLTAGE;
 			break;
 		case hwmon_in_max:
@@ -341,13 +420,25 @@ static int ina238_read_current(struct device *dev, u32 attr, long *val)
 
 	switch (attr) {
 	case hwmon_curr_input:
-		err = regmap_read(data->regmap, INA238_CURRENT, &regval);
-		if (err < 0)
-			return err;
+		if (data->config->has_20bit_voltage_current) {
+			err = ina238_read_field_s20(data->client, INA238_CURRENT, &regval);
+			if (err)
+				return err;
+		} else {
+			err = regmap_read(data->regmap, INA238_CURRENT, &regval);
+			if (err < 0)
+				return err;
+			/* sign-extend */
+			regval = (s16)regval;
+		}
 
 		/* Signed register, fixed 1mA current lsb. result in mA */
-		*val = div_s64((s16)regval * INA238_FIXED_SHUNT * data->gain,
+		*val = div_s64((s64)regval * INA238_FIXED_SHUNT * data->gain,
 			       data->rshunt * 4);
+
+		/* Account for 4 bit offset */
+		if (data->config->has_20bit_voltage_current)
+			*val /= 16;
 		break;
 	default:
 		return -EOPNOTSUPP;
@@ -750,6 +841,7 @@ static int ina238_probe(struct i2c_client *client)
 }
 
 static const struct i2c_device_id ina238_id[] = {
+	{ "ina228", ina228 },
 	{ "ina237", ina237 },
 	{ "ina238", ina238 },
 	{ "sq52206", sq52206 },
@@ -758,6 +850,10 @@ static const struct i2c_device_id ina238_id[] = {
 MODULE_DEVICE_TABLE(i2c, ina238_id);
 
 static const struct of_device_id __maybe_unused ina238_of_match[] = {
+	{
+		.compatible = "ti,ina228",
+		.data = (void *)ina228
+	},
 	{
 		.compatible = "ti,ina237",
 		.data = (void *)ina237

-- 
2.50.1.319.g90c0775e97

Re: [PATCH v2 3/3] hwmon: ina238: Add support for INA228

[Guenter Roeck]

On Fri, Jul 18, 2025 at 04:12:50PM +0200, Jonas Rebmann wrote:
> Add support for the Texas Instruments INA228 Ultra-Precise
> Power/Energy/Charge Monitor.
> 
> The INA228 is very similar to the INA238 but offers four bits of extra
> precision in the temperature, voltage and current measurement fields.
> It also supports energy and charge monitoring, the latter of which is
> not supported through this patch.
> 
> While it seems in the datasheet that some constants such as LSB values
> differ between the 228 and the 238, they differ only for those registers
> where four bits of precision have been added and they differ by a factor
> of 16 (VBUS, VSHUNT, DIETEMP, CURRENT).
> 
> Therefore, the INA238 constants are still applicable with regard
> to the bit of the same significance.
> 
> Signed-off-by: Jonas Rebmann <jre@pengutronix.de>

Applied.

Thanks,
Guenter